Allow Sufficient Assets for XCM Fee Payment on Statemint #1884 (#1910)

* Allow Sufficient Assets for XCM Fee Payment on Statemint #1884

* Revert of removed payment.clone().into()

* Fixes

* Bring AssetFeeAsExistentialDepositMultiplier to test_asset_xcm_trader

* Fixes

cumulus/parachains/common/src/xcm_config.rs

@@ -64,7 +64,7 @@ impl ShouldExecute for DenyReserveTransferToRelayChain {
 			message.0.iter().any(|inst| matches!(inst, ReserveAssetDeposited { .. }))
 		{
 			log::warn!(
-				target: "xcm::barrier",
+				target: "xcm::barriers",
 				"Unexpected ReserveAssetDeposited from the Relay Chain",
 			);
 		}

cumulus/parachains/runtimes/assets/statemine/src/xcm_config.rs

@@ -157,6 +157,12 @@ pub type Barrier = DenyThenTry<
 	),
 >;
 
+pub type AssetFeeAsExistentialDepositMultiplierFeeCharger = AssetFeeAsExistentialDepositMultiplier<
+	Runtime,
+	WeightToFee,
+	pallet_assets::BalanceToAssetBalance<Balances, Runtime, ConvertInto>,
+>;
+
 pub struct XcmConfig;
 impl xcm_executor::Config for XcmConfig {
 	type RuntimeCall = RuntimeCall;
@@ -179,11 +185,7 @@ impl xcm_executor::Config for XcmConfig {
 		UsingComponents<WeightToFee, KsmLocation, AccountId, Balances, ToStakingPot<Runtime>>,
 		cumulus_primitives_utility::TakeFirstAssetTrader<
 			AccountId,
-			AssetFeeAsExistentialDepositMultiplier<
+			AssetFeeAsExistentialDepositMultiplierFeeCharger,
-				Runtime,
-				WeightToFee,
-				pallet_assets::BalanceToAssetBalance<Balances, Runtime, ConvertInto>,
-			>,
 			ConvertedConcreteAssetId<
 				AssetId,
 				Balance,

cumulus/parachains/runtimes/assets/statemine/tests/tests.rs

@@ -1,16 +1,19 @@
 use asset_test_utils::{ExtBuilder, RuntimeHelper};
+use cumulus_primitives_utility::ChargeWeightInFungibles;
 use frame_support::{
 	assert_noop, assert_ok,
 	traits::PalletInfo,
 	weights::{Weight, WeightToFee as WeightToFeeT},
 };
 use parachains_common::{AccountId, AuraId};
+use statemine_runtime::xcm_config::AssetFeeAsExistentialDepositMultiplierFeeCharger;
 pub use statemine_runtime::{
 	constants::fee::WeightToFee, xcm_config::XcmConfig, Assets, Balances, ExistentialDeposit,
 	Runtime, SessionKeys, System,
 };
 use xcm::latest::prelude::*;
 use xcm_executor::traits::WeightTrader;
+
 pub const ALICE: [u8; 32] = [1u8; 32];
 
 #[test]
@@ -25,34 +28,25 @@ fn test_asset_xcm_trader() {
 		.build()
 		.execute_with(|| {
 			// We need root origin to create a sufficient asset
-			// We set existential deposit to be identical to the one for Balances first
+			let minimum_asset_balance = 3333333_u128;
+			let local_asset_id = 1;
 			assert_ok!(Assets::force_create(
 				RuntimeHelper::<Runtime>::root_origin(),
-				1,
+				local_asset_id,
 				AccountId::from(ALICE).into(),
 				true,
-				ExistentialDeposit::get()
+				minimum_asset_balance
 			));
 
 			// We first mint enough asset for the account to exist for assets
 			assert_ok!(Assets::mint(
 				RuntimeHelper::<Runtime>::origin_of(AccountId::from(ALICE)),
-				1,
+				local_asset_id,
 				AccountId::from(ALICE).into(),
-				ExistentialDeposit::get()
+				minimum_asset_balance
 			));
 
-			let mut trader = <XcmConfig as xcm_executor::Config>::Trader::new();
+			// get asset id as multilocation
-
-			// Set Alice as block author, who will receive fees
-			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
-
-			// We are going to buy 4e9 weight
-			let bought = 4_000_000_000u64;
-
-			// lets calculate amount needed
-			let amount_needed = WeightToFee::weight_to_fee(&Weight::from_ref_time(bought));
-
 			let asset_multilocation = MultiLocation::new(
 				0,
 				X2(
@@ -60,14 +54,40 @@ fn test_asset_xcm_trader() {
 						<Runtime as frame_system::Config>::PalletInfo::index::<Assets>().unwrap()
 							as u8,
 					),
-					GeneralIndex(1),
+					GeneralIndex(local_asset_id.into()),
 				),
 			);
 
-			let asset: MultiAsset = (asset_multilocation, amount_needed).into();
+			// Set Alice as block author, who will receive fees
+			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
 
-			// Make sure buy_weight does not return an error
+			// We are going to buy 4e9 weight
-			assert_ok!(trader.buy_weight(bought, asset.into()));
+			let bought = 4_000_000_000u64;
+
+			// Lets calculate amount needed
+			let asset_amount_needed =
+				AssetFeeAsExistentialDepositMultiplierFeeCharger::charge_weight_in_fungibles(
+					local_asset_id,
+					Weight::from_ref_time(bought),
+				)
+				.expect("failed to compute");
+
+			// Lets pay with: asset_amount_needed + asset_amount_extra
+			let asset_amount_extra = 100_u128;
+			let asset: MultiAsset =
+				(asset_multilocation.clone(), asset_amount_needed + asset_amount_extra).into();
+
+			let mut trader = <XcmConfig as xcm_executor::Config>::Trader::new();
+
+			// Lets buy_weight and make sure buy_weight does not return an error
+			match trader.buy_weight(bought, asset.into()) {
+				Ok(unused_assets) => {
+					// Check whether a correct amount of unused assets is returned
+					assert_ok!(unused_assets
+						.ensure_contains(&(asset_multilocation, asset_amount_extra).into()));
+				},
+				Err(e) => assert!(false, "Expected Ok(_). Got {:#?}", e),
+			}
 
 			// Drop trader
 			drop(trader);
@@ -75,11 +95,11 @@ fn test_asset_xcm_trader() {
 			// Make sure author(Alice) has received the amount
 			assert_eq!(
 				Assets::balance(1, AccountId::from(ALICE)),
-				ExistentialDeposit::get() + amount_needed
+				minimum_asset_balance + asset_amount_needed
 			);
 
 			// We also need to ensure the total supply increased
-			assert_eq!(Assets::total_supply(1), ExistentialDeposit::get() + amount_needed);
+			assert_eq!(Assets::total_supply(1), minimum_asset_balance + asset_amount_needed);
 		});
 }
 
@@ -302,3 +322,70 @@ fn test_that_buying_ed_refund_does_not_refund() {
 			assert_eq!(Assets::total_supply(1), ExistentialDeposit::get());
 		});
 }
+
+#[test]
+fn test_asset_xcm_trader_not_possible_for_non_sufficient_assets() {
+	ExtBuilder::<Runtime>::default()
+		.with_collators(vec![AccountId::from(ALICE)])
+		.with_session_keys(vec![(
+			AccountId::from(ALICE),
+			AccountId::from(ALICE),
+			SessionKeys { aura: AuraId::from(sp_core::sr25519::Public::from_raw(ALICE)) },
+		)])
+		.build()
+		.execute_with(|| {
+			// Create a non-sufficient asset with specific existential deposit
+			let minimum_asset_balance = 1_000_000_u128;
+			assert_ok!(Assets::force_create(
+				RuntimeHelper::<Runtime>::root_origin(),
+				1,
+				AccountId::from(ALICE).into(),
+				false,
+				minimum_asset_balance
+			));
+
+			// We first mint enough asset for the account to exist for assets
+			assert_ok!(Assets::mint(
+				RuntimeHelper::<Runtime>::origin_of(AccountId::from(ALICE)),
+				1,
+				AccountId::from(ALICE).into(),
+				minimum_asset_balance
+			));
+
+			let mut trader = <XcmConfig as xcm_executor::Config>::Trader::new();
+
+			// Set Alice as block author, who will receive fees
+			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
+
+			// We are going to buy 4e9 weight
+			let bought = 4_000_000_000u64;
+
+			// lets calculate amount needed
+			let asset_amount_needed = WeightToFee::weight_to_fee(&Weight::from_ref_time(bought));
+
+			let asset_multilocation = MultiLocation::new(
+				0,
+				X2(
+					PalletInstance(
+						<Runtime as frame_system::Config>::PalletInfo::index::<Assets>().unwrap()
+							as u8,
+					),
+					GeneralIndex(1),
+				),
+			);
+
+			let asset: MultiAsset = (asset_multilocation, asset_amount_needed).into();
+
+			// Make sure again buy_weight does return an error
+			assert_noop!(trader.buy_weight(bought, asset.into()), XcmError::TooExpensive);
+
+			// Drop trader
+			drop(trader);
+
+			// Make sure author(Alice) has NOT received the amount
+			assert_eq!(Assets::balance(1, AccountId::from(ALICE)), minimum_asset_balance);
+
+			// We also need to ensure the total supply NOT increased
+			assert_eq!(Assets::total_supply(1), minimum_asset_balance);
+		});
+}

cumulus/parachains/runtimes/assets/statemint/src/xcm_config.rs

@@ -24,9 +24,12 @@ use frame_support::{
 use pallet_xcm::XcmPassthrough;
 use parachains_common::{
 	impls::ToStakingPot,
-	xcm_config::{DenyReserveTransferToRelayChain, DenyThenTry},
+	xcm_config::{
+		AssetFeeAsExistentialDepositMultiplier, DenyReserveTransferToRelayChain, DenyThenTry,
+	},
 };
 use polkadot_parachain::primitives::Sibling;
+use sp_runtime::traits::ConvertInto;
 use xcm::latest::prelude::*;
 use xcm_builder::{
 	AccountId32Aliases, AllowKnownQueryResponses, AllowSubscriptionsFrom,
@@ -154,6 +157,12 @@ pub type Barrier = DenyThenTry<
 	),
 >;
 
+pub type AssetFeeAsExistentialDepositMultiplierFeeCharger = AssetFeeAsExistentialDepositMultiplier<
+	Runtime,
+	WeightToFee,
+	pallet_assets::BalanceToAssetBalance<Balances, Runtime, ConvertInto>,
+>;
+
 pub struct XcmConfig;
 impl xcm_executor::Config for XcmConfig {
 	type RuntimeCall = RuntimeCall;
@@ -172,8 +181,25 @@ impl xcm_executor::Config for XcmConfig {
 		RuntimeCall,
 		MaxInstructions,
 	>;
-	type Trader =
+	type Trader = (
-		UsingComponents<WeightToFee, DotLocation, AccountId, Balances, ToStakingPot<Runtime>>;
+		UsingComponents<WeightToFee, DotLocation, AccountId, Balances, ToStakingPot<Runtime>>,
+		cumulus_primitives_utility::TakeFirstAssetTrader<
+			AccountId,
+			AssetFeeAsExistentialDepositMultiplierFeeCharger,
+			ConvertedConcreteAssetId<
+				AssetId,
+				Balance,
+				AsPrefixedGeneralIndex<AssetsPalletLocation, AssetId, JustTry>,
+				JustTry,
+			>,
+			Assets,
+			cumulus_primitives_utility::XcmFeesTo32ByteAccount<
+				FungiblesTransactor,
+				AccountId,
+				XcmAssetFeesReceiver,
+			>,
+		>,
+	);
 	type ResponseHandler = PolkadotXcm;
 	type AssetTrap = PolkadotXcm;
 	type AssetClaims = PolkadotXcm;

cumulus/parachains/runtimes/assets/statemint/tests/tests.rs

@@ -1,20 +1,203 @@
 use asset_test_utils::{ExtBuilder, RuntimeHelper};
+use cumulus_primitives_utility::ChargeWeightInFungibles;
 use frame_support::{
 	assert_noop, assert_ok,
 	traits::PalletInfo,
 	weights::{Weight, WeightToFee as WeightToFeeT},
 };
 use parachains_common::{AccountId, StatemintAuraId as AuraId};
+use statemint_runtime::xcm_config::AssetFeeAsExistentialDepositMultiplierFeeCharger;
 pub use statemint_runtime::{
 	constants::fee::WeightToFee, xcm_config::XcmConfig, Assets, Balances, ExistentialDeposit,
 	Runtime, SessionKeys, System,
 };
 use xcm::latest::prelude::*;
 use xcm_executor::traits::WeightTrader;
+
 pub const ALICE: [u8; 32] = [1u8; 32];
 
 #[test]
-fn test_asset_xcm_trader_does_not_work_in_statemine() {
+fn test_asset_xcm_trader() {
+	ExtBuilder::<Runtime>::default()
+		.with_collators(vec![AccountId::from(ALICE)])
+		.with_session_keys(vec![(
+			AccountId::from(ALICE),
+			AccountId::from(ALICE),
+			SessionKeys { aura: AuraId::from(sp_core::ed25519::Public::from_raw(ALICE)) },
+		)])
+		.build()
+		.execute_with(|| {
+			// We need root origin to create a sufficient asset
+			let minimum_asset_balance = 333333333_u128;
+			let local_asset_id = 1;
+			assert_ok!(Assets::force_create(
+				RuntimeHelper::<Runtime>::root_origin(),
+				local_asset_id,
+				AccountId::from(ALICE).into(),
+				true,
+				minimum_asset_balance
+			));
+
+			// We first mint enough asset for the account to exist for assets
+			assert_ok!(Assets::mint(
+				RuntimeHelper::<Runtime>::origin_of(AccountId::from(ALICE)),
+				local_asset_id,
+				AccountId::from(ALICE).into(),
+				minimum_asset_balance
+			));
+
+			// get asset id as multilocation
+			let asset_multilocation = MultiLocation::new(
+				0,
+				X2(
+					PalletInstance(
+						<Runtime as frame_system::Config>::PalletInfo::index::<Assets>().unwrap()
+							as u8,
+					),
+					GeneralIndex(local_asset_id.into()),
+				),
+			);
+
+			// Set Alice as block author, who will receive fees
+			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
+
+			// We are going to buy 400e9 weight
+			// Because of the ED being higher in statemine
+			// and not to complicate things, we use a little
+			// bit more of weight
+			let bought = 400_000_000_000u64;
+
+			// Lets calculate amount needed
+			let asset_amount_needed =
+				AssetFeeAsExistentialDepositMultiplierFeeCharger::charge_weight_in_fungibles(
+					local_asset_id,
+					Weight::from_ref_time(bought),
+				)
+				.expect("failed to compute");
+
+			// Lets pay with: asset_amount_needed + asset_amount_extra
+			let asset_amount_extra = 100_u128;
+			let asset: MultiAsset =
+				(asset_multilocation.clone(), asset_amount_needed + asset_amount_extra).into();
+
+			let mut trader = <XcmConfig as xcm_executor::Config>::Trader::new();
+
+			// Lets buy_weight and make sure buy_weight does not return an error
+			match trader.buy_weight(bought, asset.into()) {
+				Ok(unused_assets) => {
+					// Check whether a correct amount of unused assets is returned
+					assert_ok!(unused_assets
+						.ensure_contains(&(asset_multilocation, asset_amount_extra).into()));
+				},
+				Err(e) => assert!(false, "Expected Ok(_). Got {:#?}", e),
+			}
+
+			// Drop trader
+			drop(trader);
+
+			// Make sure author(Alice) has received the amount
+			assert_eq!(
+				Assets::balance(1, AccountId::from(ALICE)),
+				minimum_asset_balance + asset_amount_needed
+			);
+
+			// We also need to ensure the total supply increased
+			assert_eq!(Assets::total_supply(1), minimum_asset_balance + asset_amount_needed);
+		});
+}
+
+#[test]
+fn test_asset_xcm_trader_with_refund() {
+	ExtBuilder::<Runtime>::default()
+		.with_collators(vec![AccountId::from(ALICE)])
+		.with_session_keys(vec![(
+			AccountId::from(ALICE),
+			AccountId::from(ALICE),
+			SessionKeys { aura: AuraId::from(sp_core::ed25519::Public::from_raw(ALICE)) },
+		)])
+		.build()
+		.execute_with(|| {
+			// We need root origin to create a sufficient asset
+			// We set existential deposit to be identical to the one for Balances first
+			assert_ok!(Assets::force_create(
+				RuntimeHelper::<Runtime>::root_origin(),
+				1,
+				AccountId::from(ALICE).into(),
+				true,
+				ExistentialDeposit::get()
+			));
+
+			// We first mint enough asset for the account to exist for assets
+			assert_ok!(Assets::mint(
+				RuntimeHelper::<Runtime>::origin_of(AccountId::from(ALICE)),
+				1,
+				AccountId::from(ALICE).into(),
+				ExistentialDeposit::get()
+			));
+
+			let mut trader = <XcmConfig as xcm_executor::Config>::Trader::new();
+
+			// Set Alice as block author, who will receive fees
+			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
+
+			// We are going to buy 400e9 weight
+			// Because of the ED being higher in statemine
+			// and not to complicate things, we use a little
+			// bit more of weight
+			let bought = 400_000_000_000u64;
+
+			let asset_multilocation = MultiLocation::new(
+				0,
+				X2(
+					PalletInstance(
+						<Runtime as frame_system::Config>::PalletInfo::index::<Assets>().unwrap()
+							as u8,
+					),
+					GeneralIndex(1),
+				),
+			);
+
+			// lets calculate amount needed
+			let amount_bought = WeightToFee::weight_to_fee(&Weight::from_ref_time(bought));
+
+			let asset: MultiAsset = (asset_multilocation.clone(), amount_bought).into();
+
+			// Make sure buy_weight does not return an error
+			assert_ok!(trader.buy_weight(bought, asset.clone().into()));
+
+			// Make sure again buy_weight does return an error
+			assert_noop!(trader.buy_weight(bought, asset.into()), XcmError::NotWithdrawable);
+
+			// We actually use half of the weight
+			let weight_used = bought / 2;
+
+			// Make sure refurnd works.
+			let amount_refunded =
+				WeightToFee::weight_to_fee(&Weight::from_ref_time(bought - weight_used));
+
+			assert_eq!(
+				trader.refund_weight(bought - weight_used),
+				Some((asset_multilocation, amount_refunded).into())
+			);
+
+			// Drop trader
+			drop(trader);
+
+			// We only should have paid for half of the bought weight
+			let fees_paid = WeightToFee::weight_to_fee(&Weight::from_ref_time(weight_used));
+
+			assert_eq!(
+				Assets::balance(1, AccountId::from(ALICE)),
+				ExistentialDeposit::get() + fees_paid
+			);
+
+			// We also need to ensure the total supply increased
+			assert_eq!(Assets::total_supply(1), ExistentialDeposit::get() + fees_paid);
+		});
+}
+
+#[test]
+fn test_asset_xcm_trader_refund_not_possible_since_amount_less_than_ed() {
 	ExtBuilder::<Runtime>::default()
 		.with_collators(vec![AccountId::from(ALICE)])
 		.with_session_keys(vec![(
@@ -39,14 +222,11 @@ fn test_asset_xcm_trader_does_not_work_in_statemine() {
 			// Set Alice as block author, who will receive fees
 			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
 
-			// We are going to buy 400e9 weight
+			// We are going to buy 50e9 weight
 			// Because of the ED being higher in statemine
 			// and not to complicate things, we use a little
 			// bit more of weight
-			let bought = 400_000_000_000u64;
+			let bought = 50_000_000_000u64;
-
-			// lets calculate amount needed
-			let amount_needed = WeightToFee::weight_to_fee(&Weight::from_ref_time(bought));
 
 			let asset_multilocation = MultiLocation::new(
 				0,
@@ -59,9 +239,16 @@ fn test_asset_xcm_trader_does_not_work_in_statemine() {
 				),
 			);
 
-			let asset: MultiAsset = (asset_multilocation, amount_needed).into();
+			let amount_bought = WeightToFee::weight_to_fee(&Weight::from_ref_time(bought));
 
-			// Buy weight should return an error, since asset trader not installed
+			assert!(
+				amount_bought < ExistentialDeposit::get(),
+				"we are testing what happens when the amount does not exceed ED"
+			);
+
+			let asset: MultiAsset = (asset_multilocation.clone(), amount_bought).into();
+
+			// Buy weight should return an error
 			assert_noop!(trader.buy_weight(bought, asset.into()), XcmError::TooExpensive);
 
 			// not credited since the ED is higher than this value
@@ -71,3 +258,146 @@ fn test_asset_xcm_trader_does_not_work_in_statemine() {
 			assert_eq!(Assets::total_supply(1), 0);
 		});
 }
+
+#[test]
+fn test_that_buying_ed_refund_does_not_refund() {
+	ExtBuilder::<Runtime>::default()
+		.with_collators(vec![AccountId::from(ALICE)])
+		.with_session_keys(vec![(
+			AccountId::from(ALICE),
+			AccountId::from(ALICE),
+			SessionKeys { aura: AuraId::from(sp_core::ed25519::Public::from_raw(ALICE)) },
+		)])
+		.build()
+		.execute_with(|| {
+			// We need root origin to create a sufficient asset
+			// We set existential deposit to be identical to the one for Balances first
+			assert_ok!(Assets::force_create(
+				RuntimeHelper::<Runtime>::root_origin(),
+				1,
+				AccountId::from(ALICE).into(),
+				true,
+				ExistentialDeposit::get()
+			));
+
+			let mut trader = <XcmConfig as xcm_executor::Config>::Trader::new();
+
+			// Set Alice as block author, who will receive fees
+			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
+
+			// We are gonna buy ED
+			let bought: u64 = ExistentialDeposit::get().try_into().unwrap();
+
+			let asset_multilocation = MultiLocation::new(
+				0,
+				X2(
+					PalletInstance(
+						<Runtime as frame_system::Config>::PalletInfo::index::<Assets>().unwrap()
+							as u8,
+					),
+					GeneralIndex(1),
+				),
+			);
+
+			let amount_bought = WeightToFee::weight_to_fee(&Weight::from_ref_time(bought));
+
+			assert!(
+				amount_bought < ExistentialDeposit::get(),
+				"we are testing what happens when the amount does not exceed ED"
+			);
+
+			// We know we will have to buy at least ED, so lets make sure first it will
+			// fail with a payment of less than ED
+			let asset: MultiAsset = (asset_multilocation.clone(), amount_bought).into();
+			assert_noop!(trader.buy_weight(bought, asset.into()), XcmError::TooExpensive);
+
+			// Now lets buy ED at least
+			let asset: MultiAsset = (asset_multilocation.clone(), ExistentialDeposit::get()).into();
+
+			// Buy weight should work
+			assert_ok!(trader.buy_weight(bought, asset.into()));
+
+			// Should return None. We have a specific check making sure we dont go below ED for
+			// drop payment
+			assert_eq!(trader.refund_weight(bought), None);
+
+			// Drop trader
+			drop(trader);
+
+			// Make sure author(Alice) has received the amount
+			assert_eq!(Assets::balance(1, AccountId::from(ALICE)), ExistentialDeposit::get());
+
+			// We also need to ensure the total supply increased
+			assert_eq!(Assets::total_supply(1), ExistentialDeposit::get());
+		});
+}
+
+#[test]
+fn test_asset_xcm_trader_not_possible_for_non_sufficient_assets() {
+	ExtBuilder::<Runtime>::default()
+		.with_collators(vec![AccountId::from(ALICE)])
+		.with_session_keys(vec![(
+			AccountId::from(ALICE),
+			AccountId::from(ALICE),
+			SessionKeys { aura: AuraId::from(sp_core::ed25519::Public::from_raw(ALICE)) },
+		)])
+		.build()
+		.execute_with(|| {
+			// Create a non-sufficient asset
+			let minimum_asset_balance = 1_000_000_u128;
+			assert_ok!(Assets::force_create(
+				RuntimeHelper::<Runtime>::root_origin(),
+				1,
+				AccountId::from(ALICE).into(),
+				false,
+				minimum_asset_balance
+			));
+
+			// We first mint enough asset for the account to exist for assets
+			assert_ok!(Assets::mint(
+				RuntimeHelper::<Runtime>::origin_of(AccountId::from(ALICE)),
+				1,
+				AccountId::from(ALICE).into(),
+				minimum_asset_balance
+			));
+
+			let mut trader = <XcmConfig as xcm_executor::Config>::Trader::new();
+
+			// Set Alice as block author, who will receive fees
+			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
+
+			// We are going to buy 400e9 weight
+			// Because of the ED being higher in statemine
+			// and not to complicate things, we use a little
+			// bit more of weight
+			let bought = 400_000_000_000u64;
+
+			// lets calculate amount needed
+			let asset_amount_needed = WeightToFee::weight_to_fee(&Weight::from_ref_time(bought));
+
+			let asset_multilocation = MultiLocation::new(
+				0,
+				X2(
+					PalletInstance(
+						<Runtime as frame_system::Config>::PalletInfo::index::<Assets>().unwrap()
+							as u8,
+					),
+					GeneralIndex(1),
+				),
+			);
+
+			let asset: MultiAsset = (asset_multilocation, asset_amount_needed).into();
+
+			// Make sure again buy_weight does return an error
+			assert_noop!(trader.buy_weight(bought, asset.into()), XcmError::TooExpensive);
+
+			// Drop trader
+			drop(trader);
+
+			// Make sure author(Alice) has NOT received the amount
+			assert_eq!(Assets::balance(1, AccountId::from(ALICE)), minimum_asset_balance);
+
+			// We also need to ensure the total supply NOT increased
+			assert_eq!(Assets::total_supply(1), minimum_asset_balance);
+		});
+}

cumulus/parachains/runtimes/assets/westmint/src/xcm_config.rs

@@ -154,6 +154,12 @@ pub type Barrier = DenyThenTry<
 	),
 >;
 
+pub type AssetFeeAsExistentialDepositMultiplierFeeCharger = AssetFeeAsExistentialDepositMultiplier<
+	Runtime,
+	WeightToFee,
+	pallet_assets::BalanceToAssetBalance<Balances, Runtime, ConvertInto>,
+>;
+
 pub struct XcmConfig;
 impl xcm_executor::Config for XcmConfig {
 	type RuntimeCall = RuntimeCall;
@@ -176,11 +182,7 @@ impl xcm_executor::Config for XcmConfig {
 		UsingComponents<WeightToFee, WestendLocation, AccountId, Balances, ToStakingPot<Runtime>>,
 		cumulus_primitives_utility::TakeFirstAssetTrader<
 			AccountId,
-			AssetFeeAsExistentialDepositMultiplier<
+			AssetFeeAsExistentialDepositMultiplierFeeCharger,
-				Runtime,
-				WeightToFee,
-				pallet_assets::BalanceToAssetBalance<Balances, Runtime, ConvertInto>,
-			>,
 			ConvertedConcreteAssetId<
 				AssetId,
 				Balance,

cumulus/parachains/runtimes/assets/westmint/tests/tests.rs

@@ -1,10 +1,12 @@
 use asset_test_utils::{ExtBuilder, RuntimeHelper};
+use cumulus_primitives_utility::ChargeWeightInFungibles;
 use frame_support::{
 	assert_noop, assert_ok,
 	traits::PalletInfo,
 	weights::{Weight, WeightToFee as WeightToFeeT},
 };
 use parachains_common::{AccountId, AuraId};
+use westmint_runtime::xcm_config::AssetFeeAsExistentialDepositMultiplierFeeCharger;
 pub use westmint_runtime::{
 	constants::fee::WeightToFee, xcm_config::XcmConfig, Assets, Balances, ExistentialDeposit,
 	Runtime, SessionKeys, System,
@@ -26,34 +28,25 @@ fn test_asset_xcm_trader() {
 		.build()
 		.execute_with(|| {
 			// We need root origin to create a sufficient asset
-			// We set existential deposit to be identical to the one for Balances first
+			let minimum_asset_balance = 3333333_u128;
+			let local_asset_id = 1;
 			assert_ok!(Assets::force_create(
 				RuntimeHelper::<Runtime>::root_origin(),
-				1,
+				local_asset_id,
 				AccountId::from(ALICE).into(),
 				true,
-				ExistentialDeposit::get()
+				minimum_asset_balance
 			));
 
 			// We first mint enough asset for the account to exist for assets
 			assert_ok!(Assets::mint(
 				RuntimeHelper::<Runtime>::origin_of(AccountId::from(ALICE)),
-				1,
+				local_asset_id,
 				AccountId::from(ALICE).into(),
-				ExistentialDeposit::get()
+				minimum_asset_balance
 			));
 
-			let mut trader = <XcmConfig as xcm_executor::Config>::Trader::new();
+			// get asset id as multilocation
-
-			// Set Alice as block author, who will receive fees
-			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
-
-			// We are going to buy 4e9 weight
-			let bought = 4_000_000_000u64;
-
-			// lets calculate amount needed
-			let amount_needed = WeightToFee::weight_to_fee(&Weight::from_ref_time(bought));
-
 			let asset_multilocation = MultiLocation::new(
 				0,
 				X2(
@@ -61,14 +54,40 @@ fn test_asset_xcm_trader() {
 						<Runtime as frame_system::Config>::PalletInfo::index::<Assets>().unwrap()
 							as u8,
 					),
-					GeneralIndex(1),
+					GeneralIndex(local_asset_id.into()),
 				),
 			);
 
-			let asset: MultiAsset = (asset_multilocation, amount_needed).into();
+			// Set Alice as block author, who will receive fees
+			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
 
-			// Make sure buy_weight does not return an error
+			// We are going to buy 4e9 weight
-			assert_ok!(trader.buy_weight(bought, asset.into()));
+			let bought = 4_000_000_000u64;
+
+			// Lets calculate amount needed
+			let asset_amount_needed =
+				AssetFeeAsExistentialDepositMultiplierFeeCharger::charge_weight_in_fungibles(
+					local_asset_id,
+					Weight::from_ref_time(bought),
+				)
+				.expect("failed to compute");
+
+			// Lets pay with: asset_amount_needed + asset_amount_extra
+			let asset_amount_extra = 100_u128;
+			let asset: MultiAsset =
+				(asset_multilocation.clone(), asset_amount_needed + asset_amount_extra).into();
+
+			let mut trader = <XcmConfig as xcm_executor::Config>::Trader::new();
+
+			// Lets buy_weight and make sure buy_weight does not return an error
+			match trader.buy_weight(bought, asset.into()) {
+				Ok(unused_assets) => {
+					// Check whether a correct amount of unused assets is returned
+					assert_ok!(unused_assets
+						.ensure_contains(&(asset_multilocation, asset_amount_extra).into()));
+				},
+				Err(e) => assert!(false, "Expected Ok(_). Got {:#?}", e),
+			}
 
 			// Drop trader
 			drop(trader);
@@ -76,11 +95,11 @@ fn test_asset_xcm_trader() {
 			// Make sure author(Alice) has received the amount
 			assert_eq!(
 				Assets::balance(1, AccountId::from(ALICE)),
-				ExistentialDeposit::get() + amount_needed
+				minimum_asset_balance + asset_amount_needed
 			);
 
 			// We also need to ensure the total supply increased
-			assert_eq!(Assets::total_supply(1), ExistentialDeposit::get() + amount_needed);
+			assert_eq!(Assets::total_supply(1), minimum_asset_balance + asset_amount_needed);
 		});
 }
 
@@ -196,7 +215,7 @@ fn test_asset_xcm_trader_refund_not_possible_since_amount_less_than_ed() {
 			// Set Alice as block author, who will receive fees
 			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
 
-			// We are going to buy 4e9 weight
+			// We are going to buy 5e9 weight
 			let bought = 500_000_000u64;
 
 			let asset_multilocation = MultiLocation::new(
@@ -301,3 +320,70 @@ fn test_that_buying_ed_refund_does_not_refund() {
 			assert_eq!(Assets::total_supply(1), ExistentialDeposit::get());
 		});
 }
+
+#[test]
+fn test_asset_xcm_trader_not_possible_for_non_sufficient_assets() {
+	ExtBuilder::<Runtime>::default()
+		.with_collators(vec![AccountId::from(ALICE)])
+		.with_session_keys(vec![(
+			AccountId::from(ALICE),
+			AccountId::from(ALICE),
+			SessionKeys { aura: AuraId::from(sp_core::sr25519::Public::from_raw(ALICE)) },
+		)])
+		.build()
+		.execute_with(|| {
+			// Create a non-sufficient asset with specific existential deposit
+			let minimum_asset_balance = 1_000_000_u128;
+			assert_ok!(Assets::force_create(
+				RuntimeHelper::<Runtime>::root_origin(),
+				1,
+				AccountId::from(ALICE).into(),
+				false,
+				minimum_asset_balance
+			));
+
+			// We first mint enough asset for the account to exist for assets
+			assert_ok!(Assets::mint(
+				RuntimeHelper::<Runtime>::origin_of(AccountId::from(ALICE)),
+				1,
+				AccountId::from(ALICE).into(),
+				minimum_asset_balance
+			));
+
+			let mut trader = <XcmConfig as xcm_executor::Config>::Trader::new();
+
+			// Set Alice as block author, who will receive fees
+			RuntimeHelper::<Runtime>::run_to_block(2, Some(AccountId::from(ALICE)));
+
+			// We are going to buy 4e9 weight
+			let bought = 4_000_000_000u64;
+
+			// lets calculate amount needed
+			let asset_amount_needed = WeightToFee::weight_to_fee(&Weight::from_ref_time(bought));
+
+			let asset_multilocation = MultiLocation::new(
+				0,
+				X2(
+					PalletInstance(
+						<Runtime as frame_system::Config>::PalletInfo::index::<Assets>().unwrap()
+							as u8,
+					),
+					GeneralIndex(1),
+				),
+			);
+
+			let asset: MultiAsset = (asset_multilocation, asset_amount_needed).into();
+
+			// Make sure again buy_weight does return an error
+			assert_noop!(trader.buy_weight(bought, asset.into()), XcmError::TooExpensive);
+
+			// Drop trader
+			drop(trader);
+
+			// Make sure author(Alice) has NOT received the amount
+			assert_eq!(Assets::balance(1, AccountId::from(ALICE)), minimum_asset_balance);
+
+			// We also need to ensure the total supply NOT increased
+			assert_eq!(Assets::total_supply(1), minimum_asset_balance);
+		});
+}

cumulus/primitives/utility/src/lib.rs

@@ -71,7 +71,7 @@ struct AssetTraderRefunder {
 	outstanding_concrete_asset: MultiAsset,
 }
 
-/// Charges for exercution in the first multiasset of those selected for fee payment
+/// Charges for execution in the first multiasset of those selected for fee payment
 /// Only succeeds for Concrete Fungible Assets
 /// First tries to convert the this MultiAsset into a local assetId
 /// Then charges for this assetId as described by FeeCharger
@@ -121,6 +121,7 @@ impl<
 		let weight = Weight::from_ref_time(weight);
 
 		// We take the very first multiasset from payment
+		// (assets are sorted by fungibility/amount after this conversion)
 		let multiassets: MultiAssets = payment.clone().into();
 
 		// Take the first multiasset from the selected MultiAssets