diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs index 9d2444f76df58..510df00b55109 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs @@ -22,7 +22,7 @@ use super::{ }; use assets_common::{ matching::{FromNetwork, FromSiblingParachain, IsForeignConcreteAsset, ParentLocation}, - TrustBackedAssetsAsLocation, + ForeignAssetsFilter, TrustBackedAssetsAsLocation, }; use frame_support::{ parameter_types, @@ -43,7 +43,7 @@ use parachains_common::{ }; use polkadot_parachain_primitives::primitives::Sibling; use polkadot_runtime_common::xcm_sender::ExponentialPrice; -use sp_runtime::traits::{AccountIdConversion, ConvertInto, TryConvertInto}; +use sp_runtime::traits::{AccountIdConversion, ConvertInto, Identity, TryConvertInto}; use testnet_parachains_constants::rococo::snowbridge::{ EthereumNetwork, INBOUND_QUEUE_PALLET_INDEX, }; @@ -163,18 +163,22 @@ pub type UniquesTransactor = NonFungiblesAdapter< CheckingAccount, >; +pub type ExcludeFromForeignAssets = ( + // Ignore `TrustBackedAssets` explicitly + StartsWith, + // Ignore asset which starts explicitly with our `GlobalConsensus(NetworkId)`, means: + // - foreign assets from our consensus should be: `Location {parents: 1, X*(Parachain(xyz), + // ..)} + // - foreign assets outside our consensus with the same `GlobalConsensus(NetworkId)` wont be + // accepted here + StartsWithExplicitGlobalConsensus, +); + +pub type ForeignAssetsOnlyFilter = ForeignAssetsFilter; + /// `AssetId`/`Balance` converter for `ForeignAssets`. pub type ForeignAssetsConvertedConcreteId = assets_common::ForeignAssetsConvertedConcreteId< - ( - // Ignore `TrustBackedAssets` explicitly - StartsWith, - // Ignore assets that start explicitly with our `GlobalConsensus(NetworkId)`, means: - // - foreign assets from our consensus should be: `Location {parents: 1, X*(Parachain(xyz), - // ..)}` - // - foreign assets outside our consensus with the same `GlobalConsensus(NetworkId)` won't - // be accepted here - StartsWithExplicitGlobalConsensus, - ), + ExcludeFromForeignAssets, Balance, xcm::v5::Location, >; @@ -389,29 +393,21 @@ impl xcm_executor::Config for XcmConfig { Balances, ResolveTo, >, - cumulus_primitives_utility::SwapFirstAssetTrader< + cumulus_primitives_utility::SwapAssetTrader< TokenLocation, - crate::AssetConversion, + Identity, + (StartsWith, ForeignAssetsOnlyFilter), WeightToFee, - crate::NativeAndNonPoolAssets, - ( - TrustBackedAssetsAsLocation< - TrustBackedAssetsPalletLocation, - Balance, - xcm::v5::Location, - >, - ForeignAssetsConvertedConcreteId, - ), - ResolveAssetTo, - AccountId, + crate::AssetConversion, >, // This trader allows to pay with `is_sufficient=true` "Trust Backed" assets from dedicated // `pallet_assets` instance - `Assets`. - cumulus_primitives_utility::TakeFirstAssetTrader< + cumulus_primitives_utility::ConcreteAssetTrader< AccountId, - AssetFeeAsExistentialDepositMultiplierFeeCharger, - TrustBackedAssetsConvertedConcreteId, + assets_common::AssetIdForTrustBackedAssetsConvert, + StartsWith, Assets, + AssetFeeAsExistentialDepositMultiplierFeeCharger, cumulus_primitives_utility::XcmFeesTo32ByteAccount< FungiblesTransactor, AccountId, @@ -420,11 +416,14 @@ impl xcm_executor::Config for XcmConfig { >, // This trader allows to pay with `is_sufficient=true` "Foreign" assets from dedicated // `pallet_assets` instance - `ForeignAssets`. - cumulus_primitives_utility::TakeFirstAssetTrader< + cumulus_primitives_utility::ConcreteAssetTrader< AccountId, - ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, - ForeignAssetsConvertedConcreteId, + WithLatestLocationConverter< + >::AssetId, + >, + ForeignAssetsOnlyFilter, ForeignAssets, + ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, cumulus_primitives_utility::XcmFeesTo32ByteAccount< ForeignFungiblesTransactor, AccountId, @@ -507,6 +506,7 @@ impl pallet_xcm::Config for Runtime { RuntimeCall, MaxInstructions, >; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index e1a9b7faf67a0..7628b45f9886f 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -1692,33 +1692,7 @@ impl_runtime_apis! { } fn query_weight_to_asset_fee(weight: Weight, asset: VersionedAssetId) -> Result { - let native_asset = xcm_config::WestendLocation::get(); - let fee_in_native = WeightToFee::weight_to_fee(&weight); - let latest_asset_id: Result = asset.clone().try_into(); - match latest_asset_id { - Ok(asset_id) if asset_id.0 == native_asset => { - // for native asset - Ok(fee_in_native) - }, - Ok(asset_id) => { - // Try to get current price of `asset_id` in `native_asset`. - if let Ok(Some(swapped_in_native)) = assets_common::PoolAdapter::::quote_price_tokens_for_exact_tokens( - asset_id.0.clone(), - native_asset, - fee_in_native, - true, // We include the fee. - ) { - Ok(swapped_in_native) - } else { - log::trace!(target: "xcm::xcm_runtime_apis", "query_weight_to_asset_fee - unhandled asset_id: {asset_id:?}!"); - Err(XcmPaymentApiError::AssetNotFound) - } - }, - Err(_) => { - log::trace!(target: "xcm::xcm_runtime_apis", "query_weight_to_asset_fee - failed to convert asset: {asset:?}!"); - Err(XcmPaymentApiError::VersionedConversionFailed) - } - } + PolkadotXcm::query_weight_to_asset_fee(weight, asset) } fn query_xcm_weight(message: VersionedXcm<()>) -> Result { diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs index 388fecde57d1b..1b2178c7ddb67 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs @@ -22,7 +22,7 @@ use super::{ }; use assets_common::{ matching::{FromSiblingParachain, IsForeignConcreteAsset, ParentLocation}, - TrustBackedAssetsAsLocation, + ForeignAssetsFilter, TrustBackedAssetsAsLocation, }; use frame_support::{ parameter_types, @@ -44,7 +44,7 @@ use parachains_common::{ use polkadot_parachain_primitives::primitives::Sibling; use polkadot_runtime_common::xcm_sender::ExponentialPrice; use snowbridge_outbound_queue_primitives::v2::exporter::PausableExporter; -use sp_runtime::traits::{AccountIdConversion, ConvertInto, TryConvertInto}; +use sp_runtime::traits::{AccountIdConversion, ConvertInto, Identity, TryConvertInto}; use westend_runtime_constants::system_parachain::COLLECTIVES_ID; use xcm::latest::{prelude::*, ROCOCO_GENESIS_HASH, WESTEND_GENESIS_HASH}; use xcm_builder::{ @@ -158,18 +158,22 @@ pub type UniquesTransactor = NonFungiblesAdapter< CheckingAccount, >; +pub type ExcludeFromForeignAssets = ( + // Ignore `TrustBackedAssets` explicitly + StartsWith, + // Ignore asset which starts explicitly with our `GlobalConsensus(NetworkId)`, means: + // - foreign assets from our consensus should be: `Location {parents: 1, X*(Parachain(xyz), + // ..)} + // - foreign assets outside our consensus with the same `GlobalConsensus(NetworkId)` wont be + // accepted here + StartsWithExplicitGlobalConsensus, +); + +pub type ForeignAssetsOnlyFilter = ForeignAssetsFilter; + /// `AssetId`/`Balance` converter for `ForeignAssets`. pub type ForeignAssetsConvertedConcreteId = assets_common::ForeignAssetsConvertedConcreteId< - ( - // Ignore `TrustBackedAssets` explicitly - StartsWith, - // Ignore asset which starts explicitly with our `GlobalConsensus(NetworkId)`, means: - // - foreign assets from our consensus should be: `Location {parents: 1, X*(Parachain(xyz), - // ..)} - // - foreign assets outside our consensus with the same `GlobalConsensus(NetworkId)` wont - // be accepted here - StartsWithExplicitGlobalConsensus, - ), + ExcludeFromForeignAssets, Balance, xcm::v5::Location, >; @@ -410,29 +414,21 @@ impl xcm_executor::Config for XcmConfig { Balances, ResolveTo, >, - cumulus_primitives_utility::SwapFirstAssetTrader< + cumulus_primitives_utility::SwapAssetTrader< WestendLocation, - crate::AssetConversion, + Identity, + (StartsWith, ForeignAssetsOnlyFilter), WeightToFee, - crate::NativeAndNonPoolAssets, - ( - TrustBackedAssetsAsLocation< - TrustBackedAssetsPalletLocation, - Balance, - xcm::v5::Location, - >, - ForeignAssetsConvertedConcreteId, - ), - ResolveAssetTo, - AccountId, + crate::AssetConversion, >, // This trader allows to pay with `is_sufficient=true` "Trust Backed" assets from dedicated // `pallet_assets` instance - `Assets`. - cumulus_primitives_utility::TakeFirstAssetTrader< + cumulus_primitives_utility::ConcreteAssetTrader< AccountId, - AssetFeeAsExistentialDepositMultiplierFeeCharger, - TrustBackedAssetsConvertedConcreteId, + assets_common::AssetIdForTrustBackedAssetsConvert, + StartsWith, Assets, + AssetFeeAsExistentialDepositMultiplierFeeCharger, cumulus_primitives_utility::XcmFeesTo32ByteAccount< FungiblesTransactor, AccountId, @@ -441,11 +437,14 @@ impl xcm_executor::Config for XcmConfig { >, // This trader allows to pay with `is_sufficient=true` "Foreign" assets from dedicated // `pallet_assets` instance - `ForeignAssets`. - cumulus_primitives_utility::TakeFirstAssetTrader< + cumulus_primitives_utility::ConcreteAssetTrader< AccountId, - ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, - ForeignAssetsConvertedConcreteId, + WithLatestLocationConverter< + >::AssetId, + >, + ForeignAssetsOnlyFilter, ForeignAssets, + ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, cumulus_primitives_utility::XcmFeesTo32ByteAccount< ForeignFungiblesTransactor, AccountId, @@ -533,6 +532,7 @@ impl pallet_xcm::Config for Runtime { RuntimeCall, MaxInstructions, >; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs index 564228d307476..42166ad5ae205 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs @@ -60,7 +60,10 @@ use xcm::latest::{ ROCOCO_GENESIS_HASH, }; use xcm_builder::WithLatestLocationConverter; -use xcm_executor::traits::{ConvertLocation, JustTry, WeightTrader}; +use xcm_executor::{ + traits::{weight_testing::TraderTest, ConvertLocation, JustTry, WeightFee, WeightTrader}, + XcmExecutor, +}; use xcm_runtime_apis::conversions::LocationToAccountHelper; const ALICE: [u8; 32] = [1u8; 32]; @@ -141,9 +144,8 @@ fn setup_pool_for_paying_fees_with_foreign_assets( } #[test] -fn test_buy_and_refund_weight_in_native() { +fn test_weight_fee_in_native() { ExtBuilder::::default() - .with_tracing() .with_collators(vec![AccountId::from(ALICE)]) .with_session_keys(vec![( AccountId::from(ALICE), @@ -154,7 +156,7 @@ fn test_buy_and_refund_weight_in_native() { .execute_with(|| { let bob: AccountId = SOME_ASSET_ADMIN.into(); let staking_pot = CollatorSelection::account_id(); - let native_location = WestendLocation::get(); + let native_asset_id: AssetId = WestendLocation::get().into(); let initial_balance = 200 * UNITS; assert_ok!(Balances::mint_into(&bob, initial_balance)); @@ -165,44 +167,32 @@ fn test_buy_and_refund_weight_in_native() { // prepare input to buy weight. let weight = Weight::from_parts(4_000_000_000, 0); - let fee = WeightToFee::weight_to_fee(&weight); - let extra_amount = 100; + let required_amount = WeightToFee::weight_to_fee(&weight); let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; - let payment: Asset = (native_location.clone(), fee + extra_amount).into(); - - // init trader and buy weight. - let mut trader = ::Trader::new(); - let unused_asset = - trader.buy_weight(weight, payment.into(), &ctx).expect("Expected Ok"); - - // assert. - let unused_amount = - unused_asset.fungible.get(&native_location.clone().into()).map_or(0, |a| *a); - assert_eq!(unused_amount, extra_amount); - assert_eq!(Balances::total_issuance(), total_issuance); - - // prepare input to refund weight. - let refund_weight = Weight::from_parts(1_000_000_000, 0); - let refund = WeightToFee::weight_to_fee(&refund_weight); - // refund. - let actual_refund = trader.refund_weight(refund_weight, &ctx).unwrap(); - assert_eq!(actual_refund, (native_location, refund).into()); + let weight_fee = ::Trader::weight_fee( + &weight, + &native_asset_id, + Some(&ctx), + ) + .expect("native asset should be accepted as weight fee"); + assert_eq!(weight_fee, WeightFee::Desired(required_amount)); - // assert. assert_eq!(Balances::balance(&staking_pot), initial_balance); - // only after `trader` is dropped we expect the fee to be resolved into the treasury - // account. - drop(trader); - assert_eq!(Balances::balance(&staking_pot), initial_balance + fee - refund); - assert_eq!(Balances::total_issuance(), total_issuance + fee - refund); + + let is_fee_taken = ::Trader::take_fee( + &native_asset_id, + required_amount, + ); + assert!(is_fee_taken); + assert_eq!(Balances::balance(&staking_pot), initial_balance + required_amount); + assert_eq!(Balances::total_issuance(), total_issuance + required_amount); }) } #[test] -fn test_buy_and_refund_weight_with_swap_local_asset_xcm_trader() { +fn test_buy_and_refund_weight_in_native() { ExtBuilder::::default() - .with_tracing() .with_collators(vec![AccountId::from(ALICE)]) .with_session_keys(vec![( AccountId::from(ALICE), @@ -213,99 +203,44 @@ fn test_buy_and_refund_weight_with_swap_local_asset_xcm_trader() { .execute_with(|| { let bob: AccountId = SOME_ASSET_ADMIN.into(); let staking_pot = CollatorSelection::account_id(); - let asset_1: u32 = 1; let native_location = WestendLocation::get(); - let asset_1_location = - AssetIdForTrustBackedAssetsConvert::convert_back(&asset_1).unwrap(); - // bob's initial balance for native and `asset1` assets. let initial_balance = 200 * UNITS; - // liquidity for both arms of (native, asset1) pool. - let pool_liquidity = 100 * UNITS; - - // init asset, balances and pool. - assert_ok!(>::create(asset_1, bob.clone(), true, 10)); - assert_ok!(Assets::mint_into(asset_1, &bob, initial_balance)); assert_ok!(Balances::mint_into(&bob, initial_balance)); assert_ok!(Balances::mint_into(&staking_pot, initial_balance)); - assert_ok!(AssetConversion::create_pool( - RuntimeHelper::origin_of(bob.clone()), - Box::new( - xcm::v5::Location::try_from(native_location.clone()).expect("conversion works") - ), - Box::new( - xcm::v5::Location::try_from(asset_1_location.clone()) - .expect("conversion works") - ) - )); - - assert_ok!(AssetConversion::add_liquidity( - RuntimeHelper::origin_of(bob.clone()), - Box::new( - xcm::v5::Location::try_from(native_location.clone()).expect("conversion works") - ), - Box::new( - xcm::v5::Location::try_from(asset_1_location.clone()) - .expect("conversion works") - ), - pool_liquidity, - pool_liquidity, - 1, - 1, - bob, - )); - // keep initial total issuance to assert later. - let asset_total_issuance = Assets::total_issuance(asset_1); - let native_total_issuance = Balances::total_issuance(); + let total_issuance = Balances::total_issuance(); // prepare input to buy weight. let weight = Weight::from_parts(4_000_000_000, 0); let fee = WeightToFee::weight_to_fee(&weight); - let asset_fee = - AssetConversion::get_amount_in(&fee, &pool_liquidity, &pool_liquidity).unwrap(); let extra_amount = 100; - let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; - let payment: Asset = (asset_1_location.clone(), asset_fee + extra_amount).into(); + let mut executor = XcmExecutor::::new(Here, XcmHash::default()); + let payment: Asset = (native_location.clone(), fee + extra_amount).into(); - // init trader and buy weight. - let mut trader = ::Trader::new(); - let unused_asset = - trader.buy_weight(weight, payment.into(), &ctx).expect("Expected Ok"); + // buy weight. + let (_, unused_amount) = + executor.test_buy_weight(weight, payment).expect("Expected Ok"); - // assert. - let unused_amount = - unused_asset.fungible.get(&asset_1_location.clone().into()).map_or(0, |a| *a); assert_eq!(unused_amount, extra_amount); - assert_eq!(Assets::total_issuance(asset_1), asset_total_issuance + asset_fee); + assert_eq!(Balances::total_issuance(), total_issuance); // prepare input to refund weight. let refund_weight = Weight::from_parts(1_000_000_000, 0); let refund = WeightToFee::weight_to_fee(&refund_weight); - let (reserve1, reserve2) = AssetConversion::get_reserves( - xcm::v5::Location::try_from(native_location).expect("conversion works"), - xcm::v5::Location::try_from(asset_1_location.clone()).expect("conversion works"), - ) - .unwrap(); - let asset_refund = - AssetConversion::get_amount_out(&refund, &reserve1, &reserve2).unwrap(); // refund. - let actual_refund = trader.refund_weight(refund_weight, &ctx).unwrap(); - assert_eq!(actual_refund, (asset_1_location, asset_refund).into()); + let actual_refund = executor.test_refund_weight(refund_weight).unwrap(); + assert_eq!(actual_refund, (AssetId(native_location), refund)); // assert. assert_eq!(Balances::balance(&staking_pot), initial_balance); - // only after `trader` is dropped we expect the fee to be resolved into the treasury - // account. - drop(trader); + + executor.test_take_fee(); + assert_eq!(Balances::balance(&staking_pot), initial_balance + fee - refund); - assert_eq!( - Assets::total_issuance(asset_1), - asset_total_issuance + asset_fee - asset_refund - ); - assert_eq!(Balances::total_issuance(), native_total_issuance); + assert_eq!(Balances::total_issuance(), total_issuance + fee - refund); }) } @@ -377,17 +312,13 @@ fn test_buy_and_refund_weight_with_swap_foreign_asset_xcm_trader() { let asset_fee = AssetConversion::get_amount_in(&fee, &pool_liquidity, &pool_liquidity).unwrap(); let extra_amount = 100; - let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; + let mut executor = XcmExecutor::::new(Here, XcmHash::default()); let payment: Asset = (foreign_location.clone(), asset_fee + extra_amount).into(); - // init trader and buy weight. - let mut trader = ::Trader::new(); - let unused_asset = - trader.buy_weight(weight, payment.into(), &ctx).expect("Expected Ok"); + // buy weight. + let (_, unused_amount) = + executor.test_buy_weight(weight, payment).expect("Expected Ok"); - // assert. - let unused_amount = - unused_asset.fungible.get(&foreign_location.clone().into()).map_or(0, |a| *a); assert_eq!(unused_amount, extra_amount); assert_eq!( ForeignAssets::total_issuance(foreign_location.clone()), @@ -403,14 +334,14 @@ fn test_buy_and_refund_weight_with_swap_foreign_asset_xcm_trader() { AssetConversion::get_amount_out(&refund, &reserve1, &reserve2).unwrap(); // refund. - let actual_refund = trader.refund_weight(refund_weight, &ctx).unwrap(); - assert_eq!(actual_refund, (foreign_location.clone(), asset_refund).into()); + let actual_refund = executor.test_refund_weight(refund_weight).unwrap(); + assert_eq!(actual_refund, (AssetId(foreign_location.clone()), asset_refund).into()); // assert. assert_eq!(Balances::balance(&staking_pot), initial_balance); - // only after `trader` is dropped we expect the fee to be resolved into the treasury - // account. - drop(trader); + + executor.test_take_fee(); + assert_eq!(Balances::balance(&staking_pot), initial_balance + fee - refund); assert_eq!( ForeignAssets::total_issuance(foreign_location), @@ -474,27 +405,133 @@ fn test_asset_xcm_take_first_trader() { let asset: Asset = (asset_location.clone(), asset_amount_needed + asset_amount_extra).into(); - let mut trader = ::Trader::new(); - let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; + let mut executor = XcmExecutor::::new(Here, XcmHash::default()); // Lets buy_weight and make sure buy_weight does not return an error - let unused_assets = trader.buy_weight(bought, asset.into(), &ctx).expect("Expected Ok"); + let unused_assets = executor.test_buy_weight(bought, asset).expect("Expected Ok"); // Check whether a correct amount of unused assets is returned - assert_ok!(unused_assets.ensure_contains(&(asset_location, asset_amount_extra).into())); + assert_eq!(unused_assets, (AssetId(asset_location.clone()), asset_amount_extra)); - // Drop trader - drop(trader); + // prepare input to refund weight. + let refund_weight = Weight::from_parts(1_000_000_000, 0); + + let asset_amount_refund = + AssetFeeAsExistentialDepositMultiplierFeeCharger::charge_weight_in_fungibles( + local_asset_id, + refund_weight, + ) + .expect("failed to compute"); + let actual_refund = executor.test_refund_weight(refund_weight).unwrap(); + assert_eq!(actual_refund, (AssetId(asset_location), asset_amount_refund).into()); + + executor.test_take_fee(); // Make sure author(Alice) has received the amount assert_eq!( Assets::balance(local_asset_id, AccountId::from(ALICE)), - minimum_asset_balance + asset_amount_needed + minimum_asset_balance + asset_amount_needed - asset_amount_refund ); // We also need to ensure the total supply increased assert_eq!( Assets::total_supply(local_asset_id), - minimum_asset_balance + asset_amount_needed + minimum_asset_balance + asset_amount_needed - asset_amount_refund + ); + }); +} + +#[test] +fn test_asset_xcm_at_least_minimum_balance_fee() { + ExtBuilder::::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(|| { + // We need root origin to create a sufficient asset + let minimum_asset_balance = 3333333_u128; + let local_asset_id = 1; + assert_ok!(Assets::force_create( + RuntimeHelper::root_origin(), + local_asset_id.into(), + 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::origin_of(AccountId::from(ALICE)), + local_asset_id.into(), + AccountId::from(ALICE).into(), + minimum_asset_balance + )); + + // get asset id as location + let asset_location = + AssetIdForTrustBackedAssetsConvert::convert_back(&local_asset_id).unwrap(); + + // Set Alice as block author, who will receive fees + RuntimeHelper::run_to_block(2, AccountId::from(ALICE)); + + // We are going to buy 4e9 weight + let bought = Weight::from_parts(4_000_000_000u64, 0); + + // Lets calculate amount needed + let asset_amount_needed = + AssetFeeAsExistentialDepositMultiplierFeeCharger::charge_weight_in_fungibles( + local_asset_id, + bought, + ) + .expect("failed to compute"); + + // Lets pay with: asset_amount_needed + asset_amount_extra + let asset: Asset = (asset_location.clone(), asset_amount_needed).into(); + let mut executor = XcmExecutor::::new(Here, XcmHash::default()); + executor.test_buy_weight(bought, asset).expect("Expected Ok"); + + // We will refund a lot of weight so that only a small portion of it is left. + // This test checks if the fee for a small weight (which costs less than + // minimum_balance) is at least minimum_balance + + // Ensure the specified small weight costs less than minimum_balance + let small_weight = Weight::from_parts(30000, 0); + let small_weight_price = + AssetFeeAsExistentialDepositMultiplierFeeCharger::charge_weight_in_fungibles( + local_asset_id, + small_weight, + ) + .expect("failed to compute"); + assert!(small_weight_price < minimum_asset_balance); + + // prepare input to refund weight. + let refund_weight = bought.saturating_sub(small_weight); + + let asset_amount_refund = asset_amount_needed - minimum_asset_balance; + let actual_refund = executor.test_refund_weight(refund_weight).unwrap(); + assert_eq!(actual_refund, (AssetId(asset_location), asset_amount_refund).into()); + + // Try to refund a bit more. + // This shouldn't result in any refund since the weight fee is already minimum_balance + let smaller_weight = small_weight.saturating_div(2); + let second_refund = executor.test_refund_weight(smaller_weight); + assert!(second_refund.is_none()); + + executor.test_take_fee(); + + // Make sure author(Alice) has received the minimum_balance + assert_eq!( + Assets::balance(local_asset_id, AccountId::from(ALICE)), + minimum_asset_balance + minimum_asset_balance + ); + + // We also need to ensure the total supply increased + assert_eq!( + Assets::total_supply(local_asset_id), + minimum_asset_balance + minimum_asset_balance ); }); } @@ -554,18 +591,16 @@ fn test_foreign_asset_xcm_take_first_trader() { let asset: Asset = (asset_location_v5.clone(), asset_amount_needed + asset_amount_extra).into(); - let mut trader = ::Trader::new(); - let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; + let mut executor = XcmExecutor::::new(Here, XcmHash::default()); // Lets buy_weight and make sure buy_weight does not return an error - let unused_assets = trader.buy_weight(bought, asset.into(), &ctx).expect("Expected Ok"); + let unused_assets = executor.test_buy_weight(bought, asset).expect("Expected Ok"); // Check whether a correct amount of unused assets is returned - assert_ok!( - unused_assets.ensure_contains(&(asset_location_v5, asset_amount_extra).into()) + assert_eq!( + unused_assets, (AssetId(asset_location_v5), asset_amount_extra) ); - // Drop trader - drop(trader); + executor.test_take_fee(); // Make sure author(Alice) has received the amount assert_eq!( @@ -611,8 +646,7 @@ fn test_asset_xcm_take_first_trader_with_refund() { ExistentialDeposit::get() )); - let mut trader = ::Trader::new(); - let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; + let mut executor = XcmExecutor::::new(Here, XcmHash::default()); // Set Alice as block author, who will receive fees RuntimeHelper::run_to_block(2, AccountId::from(ALICE)); @@ -627,12 +661,7 @@ fn test_asset_xcm_take_first_trader_with_refund() { let asset: Asset = (asset_location.clone(), amount_bought).into(); // Make sure buy_weight does not return an error - assert_ok!(trader.buy_weight(bought, asset.clone().into(), &ctx)); - - // Make sure again buy_weight does return an error - // This assert relies on the fact, that we use `TakeFirstAssetTrader` in `WeightTrader` - // tuple chain, which cannot be called twice - assert_noop!(trader.buy_weight(bought, asset.into(), &ctx), XcmError::TooExpensive); + assert_ok!(executor.test_buy_weight(bought, asset.clone())); // We actually use half of the weight let weight_used = bought / 2; @@ -641,12 +670,11 @@ fn test_asset_xcm_take_first_trader_with_refund() { let amount_refunded = WeightToFee::weight_to_fee(&(bought - weight_used)); assert_eq!( - trader.refund_weight(bought - weight_used, &ctx), - Some((asset_location, amount_refunded).into()) + executor.test_refund_weight(bought - weight_used), + Some((AssetId(asset_location), amount_refunded).into()) ); - // Drop trader - drop(trader); + executor.test_take_fee(); // We only should have paid for half of the bought weight let fees_paid = WeightToFee::weight_to_fee(&weight_used); @@ -683,8 +711,7 @@ fn test_asset_xcm_take_first_trader_refund_not_possible_since_amount_less_than_e ExistentialDeposit::get() )); - let mut trader = ::Trader::new(); - let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; + let mut executor = XcmExecutor::::new(Here, XcmHash::default()); // Set Alice as block author, who will receive fees RuntimeHelper::run_to_block(2, AccountId::from(ALICE)); @@ -704,7 +731,7 @@ fn test_asset_xcm_take_first_trader_refund_not_possible_since_amount_less_than_e let asset: Asset = (asset_location, amount_bought).into(); // Buy weight should return an error - assert_noop!(trader.buy_weight(bought, asset.into(), &ctx), XcmError::TooExpensive); + assert_noop!(executor.test_buy_weight(bought, asset), XcmError::TooExpensive); // not credited since the ED is higher than this value assert_eq!(Assets::balance(1, AccountId::from(ALICE)), 0); @@ -736,8 +763,7 @@ fn test_that_buying_ed_refund_does_not_refund_for_take_first_trader() { ExistentialDeposit::get() )); - let mut trader = ::Trader::new(); - let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; + let mut executor = XcmExecutor::::new(Here, XcmHash::default()); // Set Alice as block author, who will receive fees RuntimeHelper::run_to_block(2, AccountId::from(ALICE)); @@ -756,20 +782,19 @@ fn test_that_buying_ed_refund_does_not_refund_for_take_first_trader() { // 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: Asset = (asset_location.clone(), amount_bought).into(); - assert_noop!(trader.buy_weight(bought, asset.into(), &ctx), XcmError::TooExpensive); + assert_noop!(executor.test_buy_weight(bought, asset), XcmError::TooExpensive); // Now lets buy ED at least let asset: Asset = (asset_location.clone(), ExistentialDeposit::get()).into(); // Buy weight should work - assert_ok!(trader.buy_weight(bought, asset.into(), &ctx)); + assert_ok!(executor.test_buy_weight(bought, asset)); // Should return None. We have a specific check making sure we don't go below ED for // drop payment - assert_eq!(trader.refund_weight(bought, &ctx), None); + assert_eq!(executor.test_refund_weight(bought), None); - // Drop trader - drop(trader); + executor.test_take_fee(); // Make sure author(Alice) has received the amount assert_eq!(Assets::balance(1, AccountId::from(ALICE)), ExistentialDeposit::get()); @@ -809,8 +834,7 @@ fn test_asset_xcm_take_first_trader_not_possible_for_non_sufficient_assets() { minimum_asset_balance )); - let mut trader = ::Trader::new(); - let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; + let mut executor = XcmExecutor::::new(Here, XcmHash::default()); // Set Alice as block author, who will receive fees RuntimeHelper::run_to_block(2, AccountId::from(ALICE)); @@ -826,10 +850,9 @@ fn test_asset_xcm_take_first_trader_not_possible_for_non_sufficient_assets() { let asset: Asset = (asset_location, asset_amount_needed).into(); // Make sure again buy_weight does return an error - assert_noop!(trader.buy_weight(bought, asset.into(), &ctx), XcmError::TooExpensive); + assert_noop!(executor.test_buy_weight(bought, asset), XcmError::TooExpensive); - // Drop trader - drop(trader); + executor.test_take_fee(); // Make sure author(Alice) has NOT received the amount assert_eq!(Assets::balance(1, AccountId::from(ALICE)), minimum_asset_balance); diff --git a/cumulus/parachains/runtimes/assets/common/src/lib.rs b/cumulus/parachains/runtimes/assets/common/src/lib.rs index da024b1c94562..df4fb6c436d42 100644 --- a/cumulus/parachains/runtimes/assets/common/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/common/src/lib.rs @@ -92,6 +92,18 @@ pub type TrustBackedAssetsAsLocation< TryConvertInto, >; +pub type ForeignAssetsFilter = EverythingBut<( + // Excludes relay/parent chain currency + Equals, + // Here we rely on fact that something like this works: + // assert!(Location::new(1, + // [Parachain(100)]).starts_with(&Location::parent())); + // assert!([Parachain(100)].into().starts_with(&Here)); + StartsWith, + // Here we can exclude more stuff or leave it as `()` + AdditionalLocationExclusionFilter, +)>; + /// [`MatchedConvertedConcreteId`] converter dedicated for storing `ForeignAssets` with `AssetId` as /// `Location`. /// @@ -109,17 +121,7 @@ pub type ForeignAssetsConvertedConcreteId< > = MatchedConvertedConcreteId< AssetId, Balance, - EverythingBut<( - // Excludes relay/parent chain currency - Equals, - // Here we rely on fact that something like this works: - // assert!(Location::new(1, - // [Parachain(100)]).starts_with(&Location::parent())); - // assert!([Parachain(100)].into().starts_with(&Here)); - StartsWith, - // Here we can exclude more stuff or leave it as `()` - AdditionalLocationExclusionFilter, - )>, + ForeignAssetsFilter, LocationToAssetIdConverter, BalanceConverter, >; diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs index dab4ff8b7779c..b412959a871e6 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs @@ -288,6 +288,7 @@ impl pallet_xcm::Config for Runtime { RuntimeCall, MaxInstructions, >; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; @@ -326,7 +327,7 @@ impl, FeeHandler: HandleFee> FeeManager let Some(loc) = origin else { return false }; if let Export { network, destination: Here } = fee_reason { if network == EthereumNetwork::get().into() { - return false + return false; } } WaivedLocations::contains(loc) diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs index 5a00ae852886b..8cdf4b2636ea9 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs @@ -298,6 +298,7 @@ impl pallet_xcm::Config for Runtime { RuntimeCall, MaxInstructions, >; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/xcm_config.rs index 21d76113eb842..d94244ea78c03 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/xcm_config.rs @@ -298,6 +298,7 @@ impl pallet_xcm::Config for Runtime { RuntimeCall, MaxInstructions, >; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs index 3dcc258abb654..4dc716207b193 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs @@ -270,6 +270,7 @@ impl pallet_xcm::Config for Runtime { RuntimeCall, MaxInstructions, >; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs index 0c5d1ef47907c..f5cfc550c7872 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs @@ -304,6 +304,7 @@ impl pallet_xcm::Config for Runtime { RuntimeCall, MaxInstructions, >; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/cumulus/parachains/runtimes/people/people-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/people/people-rococo/src/xcm_config.rs index 4adbf02d9e586..7b4fcef34e112 100644 --- a/cumulus/parachains/runtimes/people/people-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/people/people-rococo/src/xcm_config.rs @@ -268,6 +268,7 @@ impl pallet_xcm::Config for Runtime { RuntimeCall, MaxInstructions, >; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs index 62a84d5f2d858..013255993d3a2 100644 --- a/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs @@ -306,6 +306,7 @@ impl pallet_xcm::Config for Runtime { RuntimeCall, MaxInstructions, >; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs b/cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs index 2f41c118d7a89..ea2d52f26c1ad 100644 --- a/cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs @@ -389,21 +389,12 @@ impl xcm_executor::Config for XcmConfig { type Weigher = FixedWeightBounds; type Trader = ( UsingComponents>, - cumulus_primitives_utility::SwapFirstAssetTrader< + cumulus_primitives_utility::SwapAssetTrader< RelayLocation, - crate::AssetConversion, + Identity, + StartsWith, WeightToFee, - crate::NativeAndAssets, - ( - TrustBackedAssetsAsLocation< - TrustBackedAssetsPalletLocation, - Balance, - xcm::latest::Location, - >, - ForeignAssetsConvertedConcreteId, - ), - ResolveAssetTo, - AccountId, + crate::AssetConversion, >, ); type ResponseHandler = PolkadotXcm; @@ -471,6 +462,7 @@ impl pallet_xcm::Config for Runtime { type XcmTeleportFilter = Everything; type XcmReserveTransferFilter = Everything; type Weigher = FixedWeightBounds; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs b/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs index 78a3df8349bc0..d99699a44b8ac 100644 --- a/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs +++ b/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs @@ -526,6 +526,7 @@ impl pallet_xcm::Config for Runtime { type XcmTeleportFilter = Everything; type XcmReserveTransferFilter = Nothing; type Weigher = FixedWeightBounds; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/cumulus/primitives/utility/src/lib.rs b/cumulus/primitives/utility/src/lib.rs index dd80335caaaab..9460478a9f233 100644 --- a/cumulus/primitives/utility/src/lib.rs +++ b/cumulus/primitives/utility/src/lib.rs @@ -27,20 +27,20 @@ use core::marker::PhantomData; use cumulus_primitives_core::{MessageSendError, UpwardMessageSender}; use frame_support::{ defensive, - traits::{tokens::fungibles, Get, OnUnbalanced as OnUnbalancedT}, + traits::{tokens::fungibles, Contains, Get, OnUnbalanced as OnUnbalancedT}, weights::{Weight, WeightToFee as WeightToFeeT}, CloneNoBound, }; -use pallet_asset_conversion::SwapCredit as SwapCreditT; +use pallet_asset_conversion::{QuotePrice as QuotePriceT, SwapCredit as SwapCreditT}; use polkadot_runtime_common::xcm_sender::PriceForMessageDelivery; use sp_runtime::{ - traits::{Saturating, Zero}, + traits::{CheckedSub, MaybeEquivalence, Zero}, SaturatedConversion, }; use xcm::{latest::prelude::*, VersionedLocation, VersionedXcm, WrapVersion}; use xcm_builder::{InspectMessageQueues, TakeRevenue}; use xcm_executor::{ - traits::{MatchesFungibles, TransactAsset, WeightTrader}, + traits::{MatchesFungibles, TransactAsset, WeightFee, WeightTrader}, AssetsInHolding, }; @@ -117,71 +117,66 @@ struct AssetTraderRefunder { outstanding_concrete_asset: Asset, } -/// Charges for execution in the first asset of those selected for fee payment -/// Only succeeds for Concrete Fungible Assets -/// First tries to convert the this Asset into a local assetId -/// Then charges for this assetId as described by FeeCharger -/// Weight, paid balance, local asset Id and the location is stored for -/// later refund purposes -/// Important: Errors if the Trader is being called twice by 2 BuyExecution instructions -/// Alternatively we could just return payment in the aforementioned case -#[derive(CloneNoBound)] -pub struct TakeFirstAssetTrader< - AccountId: Eq, +// // FIXME docs +// /// Charges for execution in the first asset of those selected for fee payment +// /// Only succeeds for Concrete Fungible Assets +// /// First tries to convert the this Asset into a local assetId +// /// Then charges for this assetId as described by FeeCharger +// /// Weight, paid balance, local asset Id and the location is stored for +// /// later refund purposes +// /// Important: Errors if the Trader is being called twice by 2 BuyExecution instructions +// /// Alternatively we could just return payment in the aforementioned case +pub struct ConcreteAssetTrader< + AccountId, + AssetIdConversion: MaybeEquivalence, + AcceptableAssets: Contains, + ConcreteAssets: fungibles::Inspect, FeeCharger: ChargeWeightInFungibles, - Matcher: MatchesFungibles, - ConcreteAssets: fungibles::Mutate + fungibles::Balanced, - HandleRefund: TakeRevenue, + TakeFee: TakeRevenue, >( - Option, - PhantomData<(AccountId, FeeCharger, Matcher, ConcreteAssets, HandleRefund)>, + PhantomData<( + AccountId, + AssetIdConversion, + AcceptableAssets, + ConcreteAssets, + FeeCharger, + TakeFee, + )>, ); impl< - AccountId: Eq, + AccountId, + AssetIdConversion: MaybeEquivalence, + AcceptableAssets: Contains, + ConcreteAssets: fungibles::Inspect, FeeCharger: ChargeWeightInFungibles, - Matcher: MatchesFungibles, - ConcreteAssets: fungibles::Mutate + fungibles::Balanced, - HandleRefund: TakeRevenue, + TakeFee: TakeRevenue, > WeightTrader - for TakeFirstAssetTrader + for ConcreteAssetTrader< + AccountId, + AssetIdConversion, + AcceptableAssets, + ConcreteAssets, + FeeCharger, + TakeFee, + > { - fn new() -> Self { - Self(None, PhantomData) - } - // We take first asset - // Check whether we can convert fee to asset_fee (is_sufficient, min_deposit) - // If everything goes well, we charge. - fn buy_weight( - &mut self, - weight: Weight, - payment: xcm_executor::AssetsInHolding, - context: &XcmContext, - ) -> Result { - log::trace!(target: "xcm::weight", "TakeFirstAssetTrader::buy_weight weight: {:?}, payment: {:?}, context: {:?}", weight, payment, context); - - // Make sure we don't enter twice - if self.0.is_some() { - return Err(XcmError::NotWithdrawable) + fn weight_fee( + weight: &Weight, + desired_asset_id: &AssetId, + context: Option<&XcmContext>, + ) -> Result { + // TODO logs + if !AcceptableAssets::contains(&desired_asset_id.0) { + return Err(XcmError::FeesNotMet); } - // We take the very first asset from payment - // (assets are sorted by fungibility/amount after this conversion) - let assets: Assets = payment.clone().into(); + let concrete_asset_id = + AssetIdConversion::convert(&desired_asset_id.0).ok_or(XcmError::FeesNotMet)?; - // Take the first asset from the selected Assets - let first = assets.get(0).ok_or(XcmError::AssetNotFound)?; - - // Get the local asset id in which we can pay for fees - let (local_asset_id, _) = - Matcher::matches_fungibles(first).map_err(|_| XcmError::AssetNotFound)?; - - // Calculate how much we should charge in the asset_id for such amount of weight - // Require at least a payment of minimum_balance - // Necessary for fully collateral-backed assets - let asset_balance: u128 = - FeeCharger::charge_weight_in_fungibles(local_asset_id.clone(), weight) + let required_amount = + FeeCharger::charge_weight_in_fungibles(concrete_asset_id.clone(), weight.clone()) .map(|amount| { - let minimum_balance = ConcreteAssets::minimum_balance(local_asset_id); + let minimum_balance = ConcreteAssets::minimum_balance(concrete_asset_id); if amount < minimum_balance { minimum_balance } else { @@ -191,91 +186,54 @@ impl< .try_into() .map_err(|_| XcmError::Overflow)?; - // Convert to the same kind of asset, with the required fungible balance - let required = first.id.clone().into_asset(asset_balance.into()); + Ok(WeightFee::Desired(required_amount)) + } - // Subtract payment - let unused = payment.checked_sub(required.clone()).map_err(|_| XcmError::TooExpensive)?; + fn refund_amount( + weight: &Weight, + used_asset_id: &AssetId, + paid_amount: u128, + context: Option<&XcmContext>, + ) -> Option { + // TODO logs + if !AcceptableAssets::contains(&used_asset_id.0) { + return None; + } - // record weight and asset - self.0 = Some(AssetTraderRefunder { - weight_outstanding: weight, - outstanding_concrete_asset: required, - }); + let concrete_asset_id = AssetIdConversion::convert(&used_asset_id.0)?; - Ok(unused) - } + let refund_amount = + FeeCharger::charge_weight_in_fungibles(concrete_asset_id.clone(), weight.clone()) + .map(|amount| { + // TODO explain why this should always succeed + let paid_amount: ConcreteAssets::Balance = paid_amount.try_into().ok()?; - fn refund_weight(&mut self, weight: Weight, context: &XcmContext) -> Option { - log::trace!(target: "xcm::weight", "TakeFirstAssetTrader::refund_weight weight: {:?}, context: {:?}", weight, context); - if let Some(AssetTraderRefunder { - mut weight_outstanding, - outstanding_concrete_asset: Asset { id, fun }, - }) = self.0.clone() - { - // Get the local asset id in which we can refund fees - let (local_asset_id, outstanding_balance) = - Matcher::matches_fungibles(&(id.clone(), fun).into()).ok()?; - - let minimum_balance = ConcreteAssets::minimum_balance(local_asset_id.clone()); - - // Calculate asset_balance - // This read should have already be cached in buy_weight - let (asset_balance, outstanding_minus_subtracted) = - FeeCharger::charge_weight_in_fungibles(local_asset_id, weight).ok().map( - |asset_balance| { - // Require at least a drop of minimum_balance - // Necessary for fully collateral-backed assets - if outstanding_balance.saturating_sub(asset_balance) > minimum_balance { - (asset_balance, outstanding_balance.saturating_sub(asset_balance)) - } - // If the amount to be refunded leaves the remaining balance below ED, - // we just refund the exact amount that guarantees at least ED will be - // dropped - else { - (outstanding_balance.saturating_sub(minimum_balance), minimum_balance) - } - }, - )?; - - // Convert balances into u128 - let outstanding_minus_subtracted: u128 = outstanding_minus_subtracted.saturated_into(); - let asset_balance: u128 = asset_balance.saturated_into(); - - // Construct outstanding_concrete_asset with the same location id and subtracted - // balance - let outstanding_concrete_asset: Asset = - (id.clone(), outstanding_minus_subtracted).into(); - - // Subtract from existing weight and balance - weight_outstanding = weight_outstanding.saturating_sub(weight); - - // Override AssetTraderRefunder - self.0 = Some(AssetTraderRefunder { weight_outstanding, outstanding_concrete_asset }); - - // Only refund if positive - if asset_balance > 0 { - Some((id, asset_balance).into()) - } else { - None - } - } else { - None - } + let resulting_paid_amount = paid_amount.checked_sub(&amount)?; + + let minimum_balance = ConcreteAssets::minimum_balance(concrete_asset_id); + + if resulting_paid_amount >= minimum_balance { + Some(amount) + } else { + // ensure refund results in at least minimum_balance weight fee + let correction = minimum_balance - resulting_paid_amount; + Some(amount - correction) + } + }) + .ok() + .flatten()?; + + let refund_amount: u128 = refund_amount.try_into().ok()?; + + (refund_amount != 0).then_some(refund_amount) } -} -impl< - AccountId: Eq, - FeeCharger: ChargeWeightInFungibles, - Matcher: MatchesFungibles, - ConcreteAssets: fungibles::Mutate + fungibles::Balanced, - HandleRefund: TakeRevenue, - > Drop for TakeFirstAssetTrader -{ - fn drop(&mut self) { - if let Some(asset_trader) = self.0.clone() { - HandleRefund::take_revenue(asset_trader.outstanding_concrete_asset); + fn take_fee(asset_id: &AssetId, amount: u128) -> bool { + if AcceptableAssets::contains(&asset_id.0) { + TakeFee::take_revenue((asset_id.clone(), amount).into()); + true + } else { + false } } } @@ -317,6 +275,7 @@ pub trait ChargeWeightInFungibles Result<>::Balance, XcmError>; } +// FIXME docs /// Provides an implementation of [`WeightTrader`] to charge for weight using the first asset /// specified in the `payment` argument. /// @@ -332,230 +291,109 @@ pub trait ChargeWeightInFungibles, - SwapCredit: SwapCreditT< - AccountId, - Balance = Fungibles::Balance, - AssetKind = Fungibles::AssetId, - Credit = fungibles::Credit, - >, - WeightToFee: WeightToFeeT, - Fungibles: fungibles::Balanced, - FungiblesAssetMatcher: MatchesFungibles, - OnUnbalanced: OnUnbalancedT>, - AccountId, -> where - Fungibles::Balance: Into, -{ - /// Accumulated fee paid for XCM execution. - total_fee: fungibles::Credit, - /// Last asset utilized by a client to settle a fee. - last_fee_asset: Option, - _phantom_data: PhantomData<( - Target, - SwapCredit, - WeightToFee, - Fungibles, - FungiblesAssetMatcher, - OnUnbalanced, - AccountId, - )>, -} +pub struct SwapAssetTrader< + Target: Get, + AssetIdConversion: MaybeEquivalence, + Swappable: Contains, + WeightToFee: WeightToFeeT, + QuotePrice: QuotePriceT, +>(PhantomData<(Target, AssetIdConversion, Swappable, WeightToFee, QuotePrice)>); impl< - Target: Get, - SwapCredit: SwapCreditT< - AccountId, - Balance = Fungibles::Balance, - AssetKind = Fungibles::AssetId, - Credit = fungibles::Credit, - >, - WeightToFee: WeightToFeeT, - Fungibles: fungibles::Balanced, - FungiblesAssetMatcher: MatchesFungibles, - OnUnbalanced: OnUnbalancedT>, - AccountId, - > WeightTrader - for SwapFirstAssetTrader< - Target, - SwapCredit, - WeightToFee, - Fungibles, - FungiblesAssetMatcher, - OnUnbalanced, - AccountId, - > -where - Fungibles::Balance: Into, + Target: Get, + AssetIdConversion: MaybeEquivalence, + Swappable: Contains, + WeightToFee: WeightToFeeT, + QuotePrice: QuotePriceT, + > WeightTrader for SwapAssetTrader { - fn new() -> Self { - Self { - total_fee: fungibles::Credit::::zero(Target::get()), - last_fee_asset: None, - _phantom_data: PhantomData, - } - } - - fn buy_weight( - &mut self, - weight: Weight, - mut payment: AssetsInHolding, - _context: &XcmContext, - ) -> Result { + fn weight_fee( + weight: &Weight, + asset_id: &AssetId, + context: Option<&XcmContext>, + ) -> Result { log::trace!( target: "xcm::weight", - "SwapFirstAssetTrader::buy_weight weight: {:?}, payment: {:?}", + "SwapAssetTrader::weight_price weight: {:?}, asset_id: {:?}, context: {:?}", weight, - payment, + asset_id, + context, ); - let first_asset: Asset = - payment.fungible.pop_first().ok_or(XcmError::AssetNotFound)?.into(); - let (fungibles_asset, balance) = FungiblesAssetMatcher::matches_fungibles(&first_asset) - .map_err(|error| { - log::trace!( - target: "xcm::weight", - "SwapFirstAssetTrader::buy_weight asset {:?} didn't match. Error: {:?}", - first_asset, - error, - ); - XcmError::AssetNotFound - })?; - let swap_asset = fungibles_asset.clone().into(); - if Target::get().eq(&swap_asset) { + let required_asset_id = AssetId(Target::get()); + if required_asset_id.eq(asset_id) { log::trace!( target: "xcm::weight", - "SwapFirstAssetTrader::buy_weight Asset was same as Target, swap not needed.", + "SwapAssetTrader::weight_price asset is same as the Target, won't replace, skipping.", ); // current trader is not applicable. - return Err(XcmError::FeesNotMet) + return Err(XcmError::FeesNotMet); } - let credit_in = Fungibles::issue(fungibles_asset, balance); - let fee = WeightToFee::weight_to_fee(&weight); - - // swap the user's asset for the `Target` asset. - let (credit_out, credit_change) = SwapCredit::swap_tokens_for_exact_tokens( - vec![swap_asset, Target::get()], - credit_in, - fee, - ) - .map_err(|(credit_in, error)| { + if !Swappable::contains(&asset_id.0) { log::trace!( target: "xcm::weight", - "SwapFirstAssetTrader::buy_weight swap couldn't be done. Error was: {:?}", - error, + "SwapAssetTrader::weight_price asset isn't swappable", ); - drop(credit_in); - XcmError::FeesNotMet - })?; + return Err(XcmError::FeesNotMet); + } - match self.total_fee.subsume(credit_out) { - Err(credit_out) => { - // error may occur if `total_fee.asset` differs from `credit_out.asset`, which does - // not apply in this context. - defensive!( - "`total_fee.asset` must be equal to `credit_out.asset`", - (self.total_fee.asset(), credit_out.asset()) - ); - return Err(XcmError::FeesNotMet) - }, - _ => (), - }; - self.last_fee_asset = Some(first_asset.id.clone()); - - payment.fungible.insert(first_asset.id, credit_change.peek().into()); - drop(credit_change); - Ok(payment) - } + let required_asset_kind = AssetIdConversion::convert(&required_asset_id.0) + .ok_or(XcmError::FeesNotMet) + .inspect_err(|_| { + log::trace!( + target: "xcm::weight", + "SwapAssetTrader::weight_price unable to convert required asset id to asset kind" + ) + })?; - fn refund_weight(&mut self, weight: Weight, _context: &XcmContext) -> Option { - log::trace!( - target: "xcm::weight", - "SwapFirstAssetTrader::refund_weight weight: {:?}, self.total_fee: {:?}", - weight, - self.total_fee, - ); - if self.total_fee.peek().is_zero() { - // noting yet paid to refund. - return None - } - let mut refund_asset = if let Some(asset) = &self.last_fee_asset { - // create an initial zero refund in the asset used in the last `buy_weight`. - (asset.clone(), Fungible(0)).into() - } else { - return None - }; - let refund_amount = WeightToFee::weight_to_fee(&weight); - if refund_amount >= self.total_fee.peek() { - // not enough was paid to refund the `weight`. - return None - } + let desired_asset_kind = AssetIdConversion::convert(&asset_id.0) + .ok_or(XcmError::FeesNotMet) + .inspect_err(|_| { + log::trace!( + target: "xcm::weight", + "SwapAssetTrader::weight_price unable to convert desired asset id to asset kind" + ) + })?; - let refund_swap_asset = FungiblesAssetMatcher::matches_fungibles(&refund_asset) - .map(|(a, _)| a.into()) - .ok()?; - - let refund = self.total_fee.extract(refund_amount); - let refund = match SwapCredit::swap_exact_tokens_for_tokens( - vec![Target::get(), refund_swap_asset], - refund, - None, - ) { - Ok(refund_in_target) => refund_in_target, - Err((refund, _)) => { - // return an attempted refund back to the `total_fee`. - let _ = self.total_fee.subsume(refund).map_err(|refund| { - // error may occur if `total_fee.asset` differs from `refund.asset`, which does - // not apply in this context. - defensive!( - "`total_fee.asset` must be equal to `refund.asset`", - (self.total_fee.asset(), refund.asset()) - ); - }); - return None - }, - }; - - refund_asset.fun = refund.peek().into().into(); - drop(refund); - Some(refund_asset) + let required_amount = WeightToFee::weight_to_fee(weight); + let required_amount_u128 = required_amount.try_into().map_err(|_| XcmError::Overflow)?; + + let swap_amount = QuotePrice::quote_price_tokens_for_exact_tokens( + desired_asset_kind, + required_asset_kind, + required_amount, + true, + ) + .ok_or(XcmError::FeesNotMet) + .inspect_err(|_| { + log::trace!( + target: "xcm::weight", + "SwapAssetTrader::weight_price unable to quote the swap price" + ) + })? + .try_into() + .map_err(|_| XcmError::Overflow)?; + + Ok(WeightFee::Swap { + required_fee: (required_asset_id, required_amount_u128).into(), + swap_amount, + }) } -} -impl< - Target: Get, - SwapCredit: SwapCreditT< - AccountId, - Balance = Fungibles::Balance, - AssetKind = Fungibles::AssetId, - Credit = fungibles::Credit, - >, - WeightToFee: WeightToFeeT, - Fungibles: fungibles::Balanced, - FungiblesAssetMatcher: MatchesFungibles, - OnUnbalanced: OnUnbalancedT>, - AccountId, - > Drop - for SwapFirstAssetTrader< - Target, - SwapCredit, - WeightToFee, - Fungibles, - FungiblesAssetMatcher, - OnUnbalanced, - AccountId, - > -where - Fungibles::Balance: Into, -{ - fn drop(&mut self) { - if self.total_fee.peek().is_zero() { - return - } - let total_fee = self.total_fee.extract(self.total_fee.peek()); - OnUnbalanced::on_unbalanced(total_fee); + fn refund_amount( + _weight: &Weight, + _used_asset_id: &AssetId, + _paid_amount: u128, + _context: Option<&XcmContext>, + ) -> Option { + // FIXME explain + None + } + + fn take_fee(_asset_id: &AssetId, _amount: u128) -> bool { + // FIXME explain + false } } @@ -684,143 +522,6 @@ mod test_xcm_router { ); } } -#[cfg(test)] -mod test_trader { - use super::*; - use frame_support::{ - assert_ok, - traits::tokens::{ - DepositConsequence, Fortitude, Preservation, Provenance, WithdrawConsequence, - }, - }; - use sp_runtime::DispatchError; - use xcm_executor::{traits::Error, AssetsInHolding}; - - #[test] - fn take_first_asset_trader_buy_weight_called_twice_throws_error() { - const AMOUNT: u128 = 100; - - // prepare prerequisites to instantiate `TakeFirstAssetTrader` - type TestAccountId = u32; - type TestAssetId = u32; - type TestBalance = u128; - struct TestAssets; - impl MatchesFungibles for TestAssets { - fn matches_fungibles(a: &Asset) -> Result<(TestAssetId, TestBalance), Error> { - match a { - Asset { fun: Fungible(amount), id: AssetId(_id) } => Ok((1, *amount)), - _ => Err(Error::AssetNotHandled), - } - } - } - impl fungibles::Inspect for TestAssets { - type AssetId = TestAssetId; - type Balance = TestBalance; - - fn total_issuance(_: Self::AssetId) -> Self::Balance { - todo!() - } - - fn minimum_balance(_: Self::AssetId) -> Self::Balance { - 0 - } - - fn balance(_: Self::AssetId, _: &TestAccountId) -> Self::Balance { - todo!() - } - - fn total_balance(_: Self::AssetId, _: &TestAccountId) -> Self::Balance { - todo!() - } - - fn reducible_balance( - _: Self::AssetId, - _: &TestAccountId, - _: Preservation, - _: Fortitude, - ) -> Self::Balance { - todo!() - } - - fn can_deposit( - _: Self::AssetId, - _: &TestAccountId, - _: Self::Balance, - _: Provenance, - ) -> DepositConsequence { - todo!() - } - - fn can_withdraw( - _: Self::AssetId, - _: &TestAccountId, - _: Self::Balance, - ) -> WithdrawConsequence { - todo!() - } - - fn asset_exists(_: Self::AssetId) -> bool { - todo!() - } - } - impl fungibles::Mutate for TestAssets {} - impl fungibles::Balanced for TestAssets { - type OnDropCredit = fungibles::DecreaseIssuance; - type OnDropDebt = fungibles::IncreaseIssuance; - } - impl fungibles::Unbalanced for TestAssets { - fn handle_dust(_: fungibles::Dust) { - todo!() - } - fn write_balance( - _: Self::AssetId, - _: &TestAccountId, - _: Self::Balance, - ) -> Result, DispatchError> { - todo!() - } - - fn set_total_issuance(_: Self::AssetId, _: Self::Balance) { - todo!() - } - } - - struct FeeChargerAssetsHandleRefund; - impl ChargeWeightInFungibles for FeeChargerAssetsHandleRefund { - fn charge_weight_in_fungibles( - _: >::AssetId, - _: Weight, - ) -> Result<>::Balance, XcmError> { - Ok(AMOUNT) - } - } - impl TakeRevenue for FeeChargerAssetsHandleRefund { - fn take_revenue(_: Asset) {} - } - - // create new instance - type Trader = TakeFirstAssetTrader< - TestAccountId, - FeeChargerAssetsHandleRefund, - TestAssets, - TestAssets, - FeeChargerAssetsHandleRefund, - >; - let mut trader = ::new(); - let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; - - // prepare test data - let asset: Asset = (Here, AMOUNT).into(); - let payment = AssetsInHolding::from(asset); - let weight_to_buy = Weight::from_parts(1_000, 1_000); - - // lets do first call (success) - assert_ok!(trader.buy_weight(weight_to_buy, payment.clone(), &ctx)); - - // lets do second call (error) - assert_eq!(trader.buy_weight(weight_to_buy, payment, &ctx), Err(XcmError::NotWithdrawable)); - } -} /// Implementation of `xcm_builder::EnsureDelivery` which helps to ensure delivery to the /// parent relay chain. Deposits existential deposit for origin (if needed). diff --git a/polkadot/runtime/rococo/src/xcm_config.rs b/polkadot/runtime/rococo/src/xcm_config.rs index 87fc99eb32ad7..01711f7c1f002 100644 --- a/polkadot/runtime/rococo/src/xcm_config.rs +++ b/polkadot/runtime/rococo/src/xcm_config.rs @@ -286,6 +286,7 @@ impl pallet_xcm::Config for Runtime { // transfer. type XcmReserveTransferFilter = Everything; type Weigher = FixedWeightBounds; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/polkadot/runtime/test-runtime/src/xcm_config.rs b/polkadot/runtime/test-runtime/src/xcm_config.rs index 8d7e351d0d5be..0be6f2f61ecba 100644 --- a/polkadot/runtime/test-runtime/src/xcm_config.rs +++ b/polkadot/runtime/test-runtime/src/xcm_config.rs @@ -28,7 +28,7 @@ use xcm_builder::{ SignedAccountId32AsNative, SignedToAccountId32, WithUniqueTopic, }; use xcm_executor::{ - traits::{TransactAsset, WeightTrader}, + traits::{TransactAsset, WeightFee, WeightTrader}, AssetsInHolding, }; @@ -107,17 +107,16 @@ impl TransactAsset for DummyAssetTransactor { #[derive(Clone)] pub struct DummyWeightTrader; impl WeightTrader for DummyWeightTrader { - fn new() -> Self { - DummyWeightTrader + fn weight_fee( + _weight: &Weight, + _asset_id: &AssetId, + _context: Option<&XcmContext>, + ) -> Result { + Ok(WeightFee::Desired(0)) } - fn buy_weight( - &mut self, - _weight: Weight, - _payment: AssetsInHolding, - _context: &XcmContext, - ) -> Result { - Ok(AssetsInHolding::default()) + fn take_fee(_asset_id: &AssetId, _amount: u128) -> bool { + true } } diff --git a/polkadot/runtime/westend/src/xcm_config.rs b/polkadot/runtime/westend/src/xcm_config.rs index e93da33b204f2..cc3d4dd21c4d8 100644 --- a/polkadot/runtime/westend/src/xcm_config.rs +++ b/polkadot/runtime/westend/src/xcm_config.rs @@ -301,6 +301,7 @@ impl pallet_xcm::Config for Runtime { RuntimeCall, MaxInstructions, >; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index fdd52784b036c..49073450c77b6 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -64,7 +64,7 @@ use xcm_executor::{ AssetTransferError, CheckSuspension, ClaimAssets, ConvertLocation, ConvertOrigin, DropAssets, EventEmitter, FeeManager, FeeReason, MatchesFungible, OnResponse, Properties, QueryHandler, QueryResponseStatus, RecordXcm, TransactAsset, TransferType, - VersionChangeNotifier, WeightBounds, XcmAssetTransfers, + VersionChangeNotifier, WeightBounds, WeightFee, WeightTrader, XcmAssetTransfers, }, AssetsInHolding, }; @@ -281,6 +281,8 @@ pub mod pallet { /// Means of measuring the weight consumed by an XCM message locally. type Weigher: WeightBounds<::RuntimeCall>; + type Trader: WeightTrader; + /// This chain's Universal Location. type UniversalLocation: Get; @@ -2953,6 +2955,21 @@ impl Pallet { }) } + pub fn query_weight_to_asset_fee( + weight: Weight, + asset_id: VersionedAssetId, + ) -> Result { + let asset_id: AssetId = + asset_id.try_into().map_err(|_| XcmPaymentApiError::VersionedConversionFailed)?; + + let weight_fee = T::Trader::weight_fee(&weight, &asset_id, None) + .map_err(|_| XcmPaymentApiError::AssetNotFound)?; + match weight_fee { + WeightFee::Desired(amount) => Ok(amount), + WeightFee::Swap { swap_amount, .. } => Ok(swap_amount), + } + } + /// Given a `destination` and XCM `message`, return assets to be charged as XCM delivery fees. pub fn query_delivery_fees( destination: VersionedLocation, diff --git a/polkadot/xcm/pallet-xcm/src/mock.rs b/polkadot/xcm/pallet-xcm/src/mock.rs index f7ba2cdc91d4a..d616de700018a 100644 --- a/polkadot/xcm/pallet-xcm/src/mock.rs +++ b/polkadot/xcm/pallet-xcm/src/mock.rs @@ -564,6 +564,7 @@ impl pallet_xcm::Config for Test { type XcmTeleportFilter = EverythingBut; type XcmReserveTransferFilter = Everything; type Weigher = FixedWeightBounds; + type Trader = ::Trader; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; diff --git a/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs b/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs index c578e7667864f..5cd1d6c012909 100644 --- a/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs +++ b/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs @@ -27,7 +27,10 @@ use frame_support::{ use frame_system::{EnsureRoot, EnsureSigned}; use polkadot_primitives::{AccountIndex, BlakeTwo256, Signature}; use sp_runtime::{generic, traits::MaybeEquivalence, AccountId32, BuildStorage}; -use xcm_executor::{traits::ConvertLocation, XcmExecutor}; +use xcm_executor::{ + traits::{ConvertLocation, WeightFee}, + XcmExecutor, +}; pub type TxExtension = ( frame_system::CheckNonZeroSender, @@ -188,17 +191,16 @@ type Barrier = AllowUnpaidExecutionFrom; #[derive(Clone)] pub struct DummyWeightTrader; impl WeightTrader for DummyWeightTrader { - fn new() -> Self { - DummyWeightTrader + fn weight_fee( + weight: &Weight, + asset_id: &AssetId, + context: Option<&XcmContext>, + ) -> Result { + Ok(WeightFee::Desired(0)) } - fn buy_weight( - &mut self, - _weight: Weight, - _payment: xcm_executor::AssetsInHolding, - _context: &XcmContext, - ) -> Result { - Ok(xcm_executor::AssetsInHolding::default()) + fn take_fee(_asset_id: &AssetId, _amount: u128) -> bool { + true } } diff --git a/polkadot/xcm/xcm-builder/src/tests/weight.rs b/polkadot/xcm/xcm-builder/src/tests/weight.rs index 637e30cce998b..80a0fd4ebdd70 100644 --- a/polkadot/xcm/xcm-builder/src/tests/weight.rs +++ b/polkadot/xcm/xcm-builder/src/tests/weight.rs @@ -14,6 +14,9 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +use frame_support::assert_ok; +use xcm_executor::traits::WeightFee; + use super::*; #[test] @@ -23,44 +26,29 @@ fn fixed_rate_of_fungible_should_work() { (Here.into(), WEIGHT_REF_TIME_PER_SECOND.into(), WEIGHT_PROOF_SIZE_PER_MB.into()); } - let mut trader = FixedRateOfFungible::::new(); + type Trader = FixedRateOfFungible; let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; - // supplies 100 unit of asset, 80 still remains after purchasing weight - assert_eq!( - trader.buy_weight( - Weight::from_parts(10, 10), - fungible_multi_asset(Here.into(), 100).into(), - &ctx, - ), - Ok(fungible_multi_asset(Here.into(), 80).into()), - ); - // should have nothing left, as 5 + 5 = 10, and we supplied 10 units of asset. - assert_eq!( - trader.buy_weight( - Weight::from_parts(5, 5), - fungible_multi_asset(Here.into(), 10).into(), - &ctx, - ), - Ok(vec![].into()), - ); - // should have 5 left, as there are no proof size components - assert_eq!( - trader.buy_weight( - Weight::from_parts(5, 0), - fungible_multi_asset(Here.into(), 10).into(), - &ctx, - ), - Ok(fungible_multi_asset(Here.into(), 5).into()), - ); - // not enough to purchase the combined weights + // Correctly computes the fee + assert_ok!( + Trader::weight_fee(&Weight::from_parts(10, 10), &Here.into(), Some(&ctx),), + WeightFee::Desired(20), + ); + + assert_ok!( + Trader::weight_fee(&Weight::from_parts(5, 5), &Here.into(), Some(&ctx),), + WeightFee::Desired(10), + ); + + assert_ok!( + Trader::weight_fee(&Weight::from_parts(5, 0), &Here.into(), Some(&ctx),), + WeightFee::Desired(5), + ); + + // Won't accept unknown token assert_err!( - trader.buy_weight( - Weight::from_parts(5, 5), - fungible_multi_asset(Here.into(), 5).into(), - &ctx, - ), - XcmError::TooExpensive, + Trader::weight_fee(&Weight::from_parts(10, 10), &Parachain(1).into(), Some(&ctx)), + XcmError::FeesNotMet, ); } @@ -188,7 +176,7 @@ fn weight_trader_tuple_should_work() { pub static HereWeightPrice: (AssetId, u128, u128) = (Here.into(), WEIGHT_REF_TIME_PER_SECOND.into(), WEIGHT_PROOF_SIZE_PER_MB.into()); pub static Para1WeightPrice: (AssetId, u128, u128) = - (Parachain(1).into(), WEIGHT_REF_TIME_PER_SECOND.into(), WEIGHT_PROOF_SIZE_PER_MB.into()); + (Parachain(1).into(), (5 * WEIGHT_REF_TIME_PER_SECOND).into(), (5 * WEIGHT_PROOF_SIZE_PER_MB).into()); } type Traders = ( @@ -198,46 +186,23 @@ fn weight_trader_tuple_should_work() { FixedRateOfFungible, ); - let mut traders = Traders::new(); let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None }; - // trader one buys weight - assert_eq!( - traders.buy_weight( - Weight::from_parts(5, 5), - fungible_multi_asset(Here.into(), 10).into(), - &ctx - ), - Ok(vec![].into()), - ); - // trader one refunds - assert_eq!( - traders.refund_weight(Weight::from_parts(2, 2), &ctx), - Some(fungible_multi_asset(Here.into(), 4)) + // the first trader computes the weight fee + assert_ok!( + Traders::weight_fee(&Weight::from_parts(5, 5), &Here.into(), Some(&ctx)), + WeightFee::Desired(10), ); - let mut traders = Traders::new(); - // trader one failed; trader two buys weight - assert_eq!( - traders.buy_weight( - Weight::from_parts(5, 5), - fungible_multi_asset(para_1.clone(), 10).into(), - &ctx - ), - Ok(vec![].into()), - ); - // trader two refunds - assert_eq!( - traders.refund_weight(Weight::from_parts(2, 2), &ctx), - Some(fungible_multi_asset(para_1, 4)) + // the second trader computes the weight fee + assert_ok!( + Traders::weight_fee(&Weight::from_parts(5, 5), ¶_1.into(), Some(&ctx)), + WeightFee::Desired(50), ); - let mut traders = Traders::new(); - // all traders fails + // unknown asset, all traders fail to compute the weight fee assert_err!( - traders.buy_weight(Weight::from_parts(5, 5), fungible_multi_asset(para_2, 10).into(), &ctx), + Traders::weight_fee(&Weight::from_parts(5, 5), ¶_2.into(), Some(&ctx)), XcmError::TooExpensive, ); - // and no refund - assert_eq!(traders.refund_weight(Weight::from_parts(2, 2), &ctx), None); } diff --git a/polkadot/xcm/xcm-builder/src/weight.rs b/polkadot/xcm/xcm-builder/src/weight.rs index 9e90053bf27fc..4650834b717a5 100644 --- a/polkadot/xcm/xcm-builder/src/weight.rs +++ b/polkadot/xcm/xcm-builder/src/weight.rs @@ -30,7 +30,7 @@ use frame_support::{ use sp_runtime::traits::{SaturatedConversion, Saturating, Zero}; use xcm::latest::{prelude::*, GetWeight, Weight}; use xcm_executor::{ - traits::{WeightBounds, WeightTrader}, + traits::{WeightBounds, WeightFee, WeightTrader}, AssetsInHolding, }; @@ -138,65 +138,42 @@ impl TakeRevenue for () { /// /// The constant `Get` type parameter should be the fungible ID, the amount of it required for one /// second of weight and the amount required for 1 MB of proof. -pub struct FixedRateOfFungible, R: TakeRevenue>( - Weight, - u128, - PhantomData<(T, R)>, -); +pub struct FixedRateOfFungible, R: TakeRevenue>(PhantomData<(T, R)>); impl, R: TakeRevenue> WeightTrader for FixedRateOfFungible { - fn new() -> Self { - Self(Weight::zero(), 0, PhantomData) - } - - fn buy_weight( - &mut self, - weight: Weight, - payment: AssetsInHolding, - context: &XcmContext, - ) -> Result { + fn weight_fee( + weight: &Weight, + asset_id: &AssetId, + context: Option<&XcmContext>, + ) -> Result { let (id, units_per_second, units_per_mb) = T::get(); tracing::trace!( target: "xcm::weight", - ?id, ?weight, ?payment, ?context, - "FixedRateOfFungible::buy_weight", + ?id, ?weight, ?asset_id, ?context, + "FixedRateOfFungible::weight_price", ); - let amount = (units_per_second * (weight.ref_time() as u128) / - (WEIGHT_REF_TIME_PER_SECOND as u128)) + - (units_per_mb * (weight.proof_size() as u128) / (WEIGHT_PROOF_SIZE_PER_MB as u128)); - if amount == 0 { - return Ok(payment) + + if id.ne(asset_id) { + return Err(XcmError::FeesNotMet); } - let unused = payment.checked_sub((id, amount).into()).map_err(|error| { - tracing::error!(target: "xcm::weight", ?amount, ?error, "FixedRateOfFungible::buy_weight Failed to substract from payment"); - XcmError::TooExpensive - })?; - self.0 = self.0.saturating_add(weight); - self.1 = self.1.saturating_add(amount); - Ok(unused) - } - fn refund_weight(&mut self, weight: Weight, context: &XcmContext) -> Option { - let (id, units_per_second, units_per_mb) = T::get(); - tracing::trace!(target: "xcm::weight", ?id, ?weight, ?context, "FixedRateOfFungible::refund_weight"); - let weight = weight.min(self.0); let amount = (units_per_second * (weight.ref_time() as u128) / (WEIGHT_REF_TIME_PER_SECOND as u128)) + (units_per_mb * (weight.proof_size() as u128) / (WEIGHT_PROOF_SIZE_PER_MB as u128)); - self.0 -= weight; - self.1 = self.1.saturating_sub(amount); - if amount > 0 { - Some((id, amount).into()) - } else { - None - } + Ok(WeightFee::Desired(amount)) } -} -impl, R: TakeRevenue> Drop for FixedRateOfFungible { - fn drop(&mut self) { - if self.1 > 0 { - R::take_revenue((T::get().0, self.1).into()); + fn take_fee(asset_id: &AssetId, amount: u128) -> bool { + let (id, _, _) = T::get(); + + if id.ne(asset_id) { + return false; } + + if amount != 0 { + R::take_revenue((id, amount).into()); + } + + true } } @@ -204,73 +181,52 @@ impl, R: TakeRevenue> Drop for FixedRateOfFungible /// places any weight bought into the right account. pub struct UsingComponents< WeightToFee: WeightToFeeT>::Balance>, - AssetIdValue: Get, + AssetLocation: Get, AccountId, Fungible: Balanced + Inspect, OnUnbalanced: OnUnbalancedT>, ->( - Weight, - Fungible::Balance, - PhantomData<(WeightToFee, AssetIdValue, AccountId, Fungible, OnUnbalanced)>, -); +>(PhantomData<(WeightToFee, AssetLocation, AccountId, Fungible, OnUnbalanced)>); impl< WeightToFee: WeightToFeeT>::Balance>, - AssetIdValue: Get, + AssetLocation: Get, AccountId, Fungible: Balanced + Inspect, OnUnbalanced: OnUnbalancedT>, - > WeightTrader for UsingComponents + > WeightTrader for UsingComponents { - fn new() -> Self { - Self(Weight::zero(), Zero::zero(), PhantomData) - } + fn weight_fee( + weight: &Weight, + asset_id: &AssetId, + context: Option<&XcmContext>, + ) -> Result { + let required_asset_id = AssetId(AssetLocation::get()); + if required_asset_id.ne(asset_id) { + return Err(XcmError::FeesNotMet); + } - fn buy_weight( - &mut self, - weight: Weight, - payment: AssetsInHolding, - context: &XcmContext, - ) -> Result { - tracing::trace!(target: "xcm::weight", ?weight, ?payment, ?context, "UsingComponents::buy_weight"); + tracing::trace!(target: "xcm::weight", ?weight, ?asset_id, ?context, "UsingComponents::weight_price"); let amount = WeightToFee::weight_to_fee(&weight); - let u128_amount: u128 = amount.try_into().map_err(|_| { + let required_amount: u128 = amount.try_into().map_err(|_| { tracing::debug!(target: "xcm::weight", ?amount, "Weight fee could not be converted"); XcmError::Overflow })?; - let required = Asset { id: AssetId(AssetIdValue::get()), fun: Fungible(u128_amount) }; - let unused = payment.checked_sub(required).map_err(|error| { - tracing::debug!(target: "xcm::weight", ?error, "Failed to substract from payment"); - XcmError::TooExpensive - })?; - self.0 = self.0.saturating_add(weight); - self.1 = self.1.saturating_add(amount); - Ok(unused) + + Ok(WeightFee::Desired(required_amount)) } - fn refund_weight(&mut self, weight: Weight, context: &XcmContext) -> Option { - tracing::trace!(target: "xcm::weight", ?weight, ?context, available_weight = ?self.0, available_amount = ?self.1, "UsingComponents::refund_weight"); - let weight = weight.min(self.0); - let amount = WeightToFee::weight_to_fee(&weight); - self.0 -= weight; - self.1 = self.1.saturating_sub(amount); - let amount: u128 = amount.saturated_into(); - tracing::trace!(target: "xcm::weight", ?amount, "UsingComponents::refund_weight"); - if amount > 0 { - Some((AssetIdValue::get(), amount).into()) - } else { - None + fn take_fee(asset_id: &AssetId, amount: u128) -> bool { + if AssetLocation::get().ne(&asset_id.0) { + return false; } - } -} -impl< - WeightToFee: WeightToFeeT>::Balance>, - AssetId: Get, - AccountId, - Fungible: Balanced + Inspect, - OnUnbalanced: OnUnbalancedT>, - > Drop for UsingComponents -{ - fn drop(&mut self) { - OnUnbalanced::on_unbalanced(Fungible::issue(self.1)); + + let Ok(amount) = amount.try_into() else { + // FIXME log + // TODO justify why this should be impossible and use defensive! + tracing::debug!(target: "xcm::weight", ?amount, "Weight fee could not be converted for depositing"); + return false; + }; + + OnUnbalanced::on_unbalanced(Fungible::issue(amount)); + true } } diff --git a/polkadot/xcm/xcm-executor/src/lib.rs b/polkadot/xcm/xcm-executor/src/lib.rs index ffaafcce28246..7fa0433dbf018 100644 --- a/polkadot/xcm/xcm-executor/src/lib.rs +++ b/polkadot/xcm/xcm-executor/src/lib.rs @@ -22,6 +22,7 @@ use alloc::{vec, vec::Vec}; use codec::{Decode, Encode}; use core::{fmt::Debug, marker::PhantomData}; use frame_support::{ + defensive, dispatch::GetDispatchInfo, ensure, traits::{Contains, ContainsPair, Defensive, Get, PalletsInfoAccess}, @@ -36,8 +37,8 @@ use traits::{ validate_export, AssetExchange, AssetLock, CallDispatcher, ClaimAssets, ConvertOrigin, DropAssets, Enact, EventEmitter, ExportXcm, FeeManager, FeeReason, HandleHrmpChannelAccepted, HandleHrmpChannelClosing, HandleHrmpNewChannelOpenRequest, OnResponse, ProcessTransaction, - Properties, ShouldExecute, TransactAsset, VersionChangeNotifier, WeightBounds, WeightTrader, - XcmAssetTransfers, + Properties, ShouldExecute, TransactAsset, VersionChangeNotifier, WeightBounds, WeightFee, + WeightTrader, XcmAssetTransfers, }; pub use traits::RecordXcm; @@ -76,7 +77,6 @@ pub struct XcmExecutor { holding_limit: usize, context: XcmContext, original_origin: Location, - trader: Config::Trader, /// The most recent error result and instruction index into the fragment in which it occurred, /// if any. error: Option<(u32, XcmError)>, @@ -93,16 +93,20 @@ pub struct XcmExecutor { transact_status: MaybeErrorCode, fees_mode: FeesMode, fees: AssetsInHolding, - /// Asset provided in last `BuyExecution` instruction (if any) in current XCM program. Same - /// asset type will be used for paying any potential delivery fees incurred by the program. - asset_used_in_buy_execution: Option, + fee_payment: Option, /// Stores the current message's weight. message_weight: Weight, asset_claimer: Option, - already_paid_fees: bool, _config: PhantomData, } +#[derive(Debug, Clone)] +struct FeePayment { + desired_fee_asset_id: AssetId, + used_fee_asset_id: AssetId, + paid_amount: u128, +} + #[cfg(any(test, feature = "runtime-benchmarks"))] impl XcmExecutor { pub fn holding(&self) -> &AssetsInHolding { @@ -129,12 +133,6 @@ impl XcmExecutor { pub fn set_original_origin(&mut self, v: Location) { self.original_origin = v } - pub fn trader(&self) -> &Config::Trader { - &self.trader - } - pub fn set_trader(&mut self, v: Config::Trader) { - self.trader = v - } pub fn error(&self) -> &Option<(u32, XcmError)> { &self.error } @@ -208,7 +206,7 @@ impl XcmExecutor { self.message_weight = weight; } pub fn already_paid_fees(&self) -> bool { - self.already_paid_fees + self.fee_payment.is_some() } } @@ -356,7 +354,6 @@ impl XcmExecutor { holding_limit: Config::MaxAssetsIntoHolding::get() as usize, context: XcmContext { origin: Some(origin.clone()), message_id, topic: None }, original_origin: origin, - trader: Config::Trader::new(), error: None, total_surplus: Weight::zero(), total_refunded: Weight::zero(), @@ -367,10 +364,9 @@ impl XcmExecutor { transact_status: Default::default(), fees_mode: FeesMode { jit_withdraw: false }, fees: AssetsInHolding::new(), - asset_used_in_buy_execution: None, + fee_payment: None, message_weight: Weight::zero(), asset_claimer: None, - already_paid_fees: false, _config: PhantomData, } } @@ -382,7 +378,12 @@ impl XcmExecutor { // We silently drop any error from our attempt to refund the surplus as it's a charitable // thing so best-effort is all we will do. let _ = self.refund_surplus(); - drop(self.trader); + + if let Some(fee_payment) = self.fee_payment { + if !Config::Trader::take_fee(&fee_payment.used_fee_asset_id, fee_payment.paid_amount) { + defensive!("The trader must take the fee"); + } + } let mut weight_used = xcm_weight.saturating_sub(self.total_surplus); @@ -517,27 +518,14 @@ impl XcmExecutor { ?current_surplus, "Refunding surplus", ); - if current_surplus.any_gt(Weight::zero()) { - if let Some(w) = self.trader.refund_weight(current_surplus, &self.context) { - if !self.holding.contains_asset(&(w.id.clone(), 1).into()) && - self.ensure_can_subsume_assets(1).is_err() - { - let _ = self - .trader - .buy_weight(current_surplus, w.into(), &self.context) - .defensive_proof( - "refund_weight returned an asset capable of buying weight; qed", - ); - tracing::error!( - target: "xcm::refund_surplus", - "error: HoldingWouldOverflow", - ); - return Err(XcmError::HoldingWouldOverflow) - } - self.total_refunded.saturating_accrue(current_surplus); - self.holding.subsume_assets(w.into()); + + if let Some(refund @ (_, amount)) = self.refund_weight(current_surplus) { + if amount != 0 { + // != 0 to avoid inserting zero amount into holding if asset is not present there + self.holding.subsume(refund.into()); } } + // If there are any leftover `fees`, merge them with `holding`. if !self.fees.is_empty() { let leftover_fees = self.fees.saturating_take(Wild(All)); @@ -550,6 +538,73 @@ impl XcmExecutor { Ok(()) } + fn refund_weight(&mut self, xcm_weight: Weight) -> Option<(AssetId, u128)> { + if !xcm_weight.any_gt(Weight::zero()) { + // Nothing to refund + return None; + } + + let Some(mut fee_payment) = self.fee_payment.as_ref().cloned() else { + // No asset was used to pay, nothing to refund + return None; + }; + + if !self + .holding + .contains_asset(&(fee_payment.desired_fee_asset_id.clone(), 1).into()) && + self.ensure_can_subsume_assets(1).is_err() + { + // The Holding Register can't subsume the refund asset + return None; + } + + let Some(refund_amount) = Config::Trader::refund_amount( + &xcm_weight, + &fee_payment.used_fee_asset_id, + fee_payment.paid_amount, + Some(&self.context), + ) else { + // Best effort + return None; + }; + + let Some(decreased_paid) = fee_payment.paid_amount.checked_sub(refund_amount) else { + // Can't refund more than was paid + return None; + }; + + if decreased_paid == 0 { + defensive!("refund shouldn't result in free execution"); + return None; + } + + let desired_asset_refund_amount; + if fee_payment.desired_fee_asset_id != fee_payment.used_fee_asset_id { + if let Some(desired_refund) = self.quote_and_exchange_exact_give( + (fee_payment.used_fee_asset_id.clone(), refund_amount).into(), + fee_payment.desired_fee_asset_id.clone(), + ) { + desired_asset_refund_amount = desired_refund; + } else { + // Can't exchange to get the desired refund asset + return None; + } + } else { + desired_asset_refund_amount = refund_amount; + }; + + if desired_asset_refund_amount != 0 {} + + let refund = (fee_payment.desired_fee_asset_id.clone(), desired_asset_refund_amount); + + self.total_refunded.saturating_accrue(xcm_weight); + + fee_payment.paid_amount = decreased_paid; + self.fee_payment = Some(fee_payment); + + Some(refund) + } + fn take_fee(&mut self, fees: Assets, reason: FeeReason) -> XcmResult { if Config::FeeManager::is_waived(self.origin_ref(), reason.clone()) { return Ok(()) @@ -644,14 +699,11 @@ impl XcmExecutor { /// If neither `PayFees` or `BuyExecution` were not used, or no swap is required, /// it will just return `asset_needed_for_fees`. fn calculate_asset_for_delivery_fees(&self, asset_needed_for_fees: Asset) -> Asset { - let Some(asset_wanted_for_fees) = - // we try to swap first asset in the fees register (should only ever be one), - self.fees.fungible.first_key_value().map(|(id, _)| id).or_else(|| { - // or the one used in BuyExecution - self.asset_used_in_buy_execution.as_ref() - }) - // if it is different than what we need - .filter(|&id| asset_needed_for_fees.id.ne(id)) + let Some(asset_wanted_for_fees) = self + .fee_payment + .as_ref() + .map(|p| p.desired_fee_asset_id.clone()) + .filter(|id| asset_needed_for_fees.id.ne(&id)) else { // either nothing to swap or we're already holding the right asset return asset_needed_for_fees @@ -830,7 +882,7 @@ impl XcmExecutor { let inst_res = recursion_count::using_once(&mut 1, || { recursion_count::with(|count| { if *count > RECURSION_LIMIT { - return None + return None; } *count = count.saturating_add(1); Some(()) @@ -1351,43 +1403,44 @@ impl XcmExecutor { Ok(()) }, BuyExecution { fees, weight_limit } => { + // If we've already paid for fees, do nothing. + if self.fee_payment.is_some() { + return Ok(()); + } + // There is no need to buy any weight if `weight_limit` is `Unlimited` since it // would indicate that `AllowTopLevelPaidExecutionFrom` was unused for execution // and thus there is some other reason why it has been determined that this XCM // should be executed. let Some(weight) = Option::::from(weight_limit) else { return Ok(()) }; let old_holding = self.holding.clone(); - // Save the asset being used for execution fees, so we later know what should be - // used for delivery fees. - self.asset_used_in_buy_execution = Some(fees.id.clone()); - tracing::trace!( - target: "xcm::executor::BuyExecution", - asset_used_in_buy_execution = ?self.asset_used_in_buy_execution - ); // pay for `weight` using up to `fees` of the holding register. - let max_fee = + let max_fee_assets = self.holding.try_take(fees.clone().into()).map_err(|e| { tracing::error!(target: "xcm::process_instruction::buy_execution", ?e, ?fees, "Failed to take fees from holding"); XcmError::NotHoldingFees })?; + + // `fees` contain only ony asset id, so we should get exactly one asset from the holding + let max_fee = max_fee_assets.into_assets_iter().next().ok_or(XcmError::TooExpensive)?; let result = Config::TransactionalProcessor::process(|| { - let unspent = self.trader.buy_weight(weight, max_fee, &self.context)?; - self.holding.subsume_assets(unspent); + let unspent = self.buy_weight(weight, max_fee)?; + self.holding.subsume(unspent.into()); Ok(()) }); - if result.is_err() { + if Config::TransactionalProcessor::IS_TRANSACTIONAL && result.is_err() { self.holding = old_holding; + self.fee_payment = None; } result }, PayFees { asset } => { // If we've already paid for fees, do nothing. - if self.already_paid_fees { + if self.fee_payment.is_some() { return Ok(()); } - // Make sure `PayFees` won't be processed again. - self.already_paid_fees = true; + // Record old holding in case we need to rollback. let old_holding = self.holding.clone(); // The max we're willing to pay for fees is decided by the `asset` operand. @@ -1398,19 +1451,23 @@ impl XcmExecutor { ); // Pay for execution fees. let result = Config::TransactionalProcessor::process(|| { - let max_fee = + let max_fee_assets = self.holding.try_take(asset.into()).map_err(|_| XcmError::NotHoldingFees)?; - let unspent = - self.trader.buy_weight(self.message_weight, max_fee, &self.context)?; + + // `fees` contain only ony asset id, so we should get exactly one asset from the holding + let max_fee = max_fee_assets.into_assets_iter().next().ok_or(XcmError::TooExpensive)?; + + let unspent = self.buy_weight(self.message_weight.clone(), max_fee)?; + // Move unspent to the `fees` register, it can later be moved to holding // by calling `RefundSurplus`. - self.fees.subsume_assets(unspent); + self.fees.subsume(unspent.into()); Ok(()) }); if Config::TransactionalProcessor::IS_TRANSACTIONAL && result.is_err() { // Rollback on error. self.holding = old_holding; - self.already_paid_fees = false; + self.fee_payment = None; } result }, @@ -1720,6 +1777,103 @@ impl XcmExecutor { } } + fn buy_weight( + &mut self, + xcm_weight: Weight, + max_desired_fees: Asset, + ) -> Result<(AssetId, u128), XcmError> { + if let Fungible(max_amount) = max_desired_fees.fun { + let desired_fee_asset_id = max_desired_fees.id.clone(); + let weight_fee = Config::Trader::weight_fee( + &xcm_weight, + &desired_fee_asset_id, + Some(&self.context), + )?; + + let used_fee_asset_id; + let paid_amount; + let unspent; + match weight_fee { + WeightFee::Desired(required_fee_amount) => { + used_fee_asset_id = desired_fee_asset_id.clone(); + paid_amount = required_fee_amount; + unspent = max_amount.checked_sub(paid_amount).ok_or(XcmError::TooExpensive)?; + }, + WeightFee::Swap { + required_fee: (required_asset_id, required_amount), + swap_amount, + } => { + used_fee_asset_id = required_asset_id; + paid_amount = required_amount; + + // Do not swap unless we have the needed amount + unspent = max_amount.checked_sub(swap_amount).ok_or(XcmError::TooExpensive)?; + + let give: Asset = (desired_fee_asset_id.clone(), swap_amount).into(); + let want: Asset = (used_fee_asset_id.clone(), required_amount).into(); + Config::AssetExchanger::exchange_asset( + self.origin_ref(), + give.clone().into(), + &want.clone().into(), + false, + ) + .map_err(|given_assets| { + tracing::error!( + target: "xcm::fees", + ?given_assets, "buy_weight: couldn't swap the asset {:?} for {:?}", give, want, + ); + + XcmError::FeesNotMet + })?; + }, + } + + self.fee_payment = Some(FeePayment { + desired_fee_asset_id: desired_fee_asset_id.clone(), + used_fee_asset_id: used_fee_asset_id.clone(), + paid_amount, + }); + + tracing::trace!( + target: "xcm::executor::buy_weight", + fee_payment = ?self.fee_payment + ); + + Ok((desired_fee_asset_id, unspent)) + } else { + Err(XcmError::FeesNotMet) + } + } + + fn quote_and_exchange_exact_give(&self, give: Asset, want_id: AssetId) -> Option { + let give_assets: Assets = Asset::from(give).into(); + let exchange_assets = Config::AssetExchanger::quote_exchange_price( + &give_assets, + &(want_id.clone(), Fungible(0)).into(), + true, + )? + .into_inner(); + + let Some(Fungible(want_amount)) = exchange_assets.get(0).map(|a| a.fun.clone()) else { + return None; + }; + + let to_exchange: Assets = Asset::from((want_id, want_amount)).into(); + Config::AssetExchanger::exchange_asset( + self.origin_ref(), + give_assets.clone().into(), + &to_exchange, + true, + ).inspect_err(|given_assets| { + tracing::error!( + target: "xcm::fees", + ?given_assets, "quote_and_exchange_exact_give: couldn't swap the asset {:?} for {:?}", give_assets, to_exchange, + ); + }).ok()?; + + Some(want_amount) + } + fn do_descend_origin(&mut self, who: InteriorLocation) -> XcmResult { self.context .origin @@ -1812,8 +1966,8 @@ impl XcmExecutor { let maybe_delivery_fee = fee.get(0).map(|asset_needed_for_fees| { tracing::trace!( target: "xcm::fees::take_delivery_fee_from_assets", - "Asset provided to pay for fees {:?}, asset required for delivery fees: {:?}", - self.asset_used_in_buy_execution, asset_needed_for_fees, + "Fee payment: {:?}, asset required for delivery fees: {:?}", + self.fee_payment, asset_needed_for_fees, ); let asset_to_pay_for_fees = self.calculate_asset_for_delivery_fees(asset_needed_for_fees.clone()); @@ -1825,3 +1979,25 @@ impl XcmExecutor { Ok(maybe_delivery_fee) } } + +impl traits::weight_testing::TraderTest for XcmExecutor { + fn test_buy_weight( + &mut self, + weight: Weight, + max_payment: Asset, + ) -> Result<(AssetId, u128), XcmError> { + self.buy_weight(weight, max_payment) + } + + fn test_refund_weight(&mut self, weight: Weight) -> Option<(AssetId, u128)> { + self.refund_weight(weight) + } + + fn test_take_fee(self) { + if let Some(fee_payment) = self.fee_payment { + if !Config::Trader::take_fee(&fee_payment.used_fee_asset_id, fee_payment.paid_amount) { + defensive!("The trader must take the fee"); + } + } + } +} diff --git a/polkadot/xcm/xcm-executor/src/tests/mock.rs b/polkadot/xcm/xcm-executor/src/tests/mock.rs index db84e9cabe282..40de3b5453bdf 100644 --- a/polkadot/xcm/xcm-executor/src/tests/mock.rs +++ b/polkadot/xcm/xcm-executor/src/tests/mock.rs @@ -31,7 +31,7 @@ use xcm::prelude::*; use crate::{ traits::{ DropAssets, FeeManager, ProcessTransaction, Properties, ShouldExecute, TransactAsset, - WeightBounds, WeightTrader, + WeightBounds, WeightFee, WeightTrader, }, AssetsInHolding, Config, FeeReason, XcmExecutor, }; @@ -172,36 +172,28 @@ impl WeightToFee { /// Test weight trader that just buys weight with the native asset (`Here`) and /// uses the test `WeightToFee`. -pub struct TestTrader { - weight_bought_so_far: Weight, -} +pub struct TestTrader; impl WeightTrader for TestTrader { - fn new() -> Self { - Self { weight_bought_so_far: Weight::zero() } - } + fn weight_fee( + weight: &Weight, + asset_id: &AssetId, + _context: Option<&XcmContext>, + ) -> Result { + if asset_id.0 != Here.into() { + return Err(XcmError::FeesNotMet); + } - fn buy_weight( - &mut self, - weight: Weight, - payment: AssetsInHolding, - _context: &XcmContext, - ) -> Result { let amount = WeightToFee::weight_to_fee(&weight); - let required: Asset = (Here, amount).into(); - let unused = payment.checked_sub(required).map_err(|_| XcmError::TooExpensive)?; - self.weight_bought_so_far.saturating_add(weight); - Ok(unused) + Ok(WeightFee::Desired(amount)) } - fn refund_weight(&mut self, weight: Weight, _context: &XcmContext) -> Option { - let weight = weight.min(self.weight_bought_so_far); - let amount = WeightToFee::weight_to_fee(&weight); - self.weight_bought_so_far -= weight; - if amount > 0 { - Some((Here, amount).into()) - } else { - None + fn take_fee(asset_id: &AssetId, _amount: u128) -> bool { + if asset_id.0 != Here.into() { + return false; } + + // Just burn the asset + true } } diff --git a/polkadot/xcm/xcm-executor/src/traits/mod.rs b/polkadot/xcm/xcm-executor/src/traits/mod.rs index e70d2c2f4f3af..9bf53453615e6 100644 --- a/polkadot/xcm/xcm-executor/src/traits/mod.rs +++ b/polkadot/xcm/xcm-executor/src/traits/mod.rs @@ -57,7 +57,7 @@ pub use event_emitter::EventEmitter; pub use record_xcm::RecordXcm; #[deprecated = "Use `sp_runtime::traits::` instead"] pub use sp_runtime::traits::{Identity, TryConvertInto as JustTry}; -pub use weight::{WeightBounds, WeightTrader}; +pub use weight::{testing as weight_testing, WeightBounds, WeightFee, WeightTrader}; pub mod prelude { pub use super::{ diff --git a/polkadot/xcm/xcm-executor/src/traits/weight.rs b/polkadot/xcm/xcm-executor/src/traits/weight.rs index 4e41aa5b47530..953561fe392fa 100644 --- a/polkadot/xcm/xcm-executor/src/traits/weight.rs +++ b/polkadot/xcm/xcm-executor/src/traits/weight.rs @@ -14,7 +14,6 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -use crate::AssetsInHolding; use core::result::Result; use xcm::latest::{prelude::*, Weight}; @@ -29,98 +28,160 @@ pub trait WeightBounds { fn instr_weight(instruction: &mut Instruction) -> Result; } -/// Charge for weight in order to execute XCM. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum WeightFee { + Desired(u128), + Swap { required_fee: (AssetId, u128), swap_amount: u128 }, +} + +// FIXME docs +/// Get the weight price in order to buy it to execute XCM. /// /// A `WeightTrader` may also be put into a tuple, in which case the default behavior of -/// `buy_weight` and `refund_weight` would be to attempt to call each tuple element's own +/// `weight_price` would be to attempt to call each tuple element's own /// implementation of these two functions, in the order of which they appear in the tuple, /// returning early when a successful result is returned. -pub trait WeightTrader: Sized { - /// Create a new trader instance. - fn new() -> Self; - - /// Purchase execution weight credit in return for up to a given `payment`. If less of the - /// payment is required then the surplus is returned. If the `payment` cannot be used to pay - /// for the `weight`, then an error is returned. - fn buy_weight( - &mut self, - weight: Weight, - payment: AssetsInHolding, - context: &XcmContext, - ) -> Result; - - /// Attempt a refund of `weight` into some asset. The caller does not guarantee that the weight - /// was purchased using `buy_weight`. - /// - /// Default implementation refunds nothing. - fn refund_weight(&mut self, _weight: Weight, _context: &XcmContext) -> Option { - None +pub trait WeightTrader { + fn weight_fee( + weight: &Weight, + desired_asset_id: &AssetId, + context: Option<&XcmContext>, + ) -> Result; + + fn refund_amount( + weight: &Weight, + used_asset_id: &AssetId, + _paid_amount: u128, + context: Option<&XcmContext>, + ) -> Option { + Self::weight_fee(weight, used_asset_id, context) + .ok() + .map(|wf| if let WeightFee::Desired(amount) = wf { Some(amount) } else { None }) + .flatten() } + + fn take_fee(asset_id: &AssetId, amount: u128) -> bool; } #[impl_trait_for_tuples::impl_for_tuples(30)] impl WeightTrader for Tuple { - fn new() -> Self { - for_tuples!( ( #( Tuple::new() ),* ) ) - } - - fn buy_weight( - &mut self, - weight: Weight, - payment: AssetsInHolding, - context: &XcmContext, - ) -> Result { - let mut too_expensive_error_found = false; - let mut last_error = None; + fn weight_fee( + weight: &Weight, + desired_asset_id: &AssetId, + context: Option<&XcmContext>, + ) -> Result { for_tuples!( #( let weight_trader = core::any::type_name::(); - match Tuple.buy_weight(weight, payment.clone(), context) { - Ok(assets) => { + match Tuple::weight_fee(weight, desired_asset_id, context) { + Ok(fee) => { tracing::trace!( - target: "xcm::buy_weight", + target: "xcm::weight_trader", %weight_trader, - "Buy weight succeeded", + "Getting weight price succeeded", ); - return Ok(assets) - }, + return Ok(fee); + } Err(error) => { - if let XcmError::TooExpensive = error { - too_expensive_error_found = true; - } - last_error = Some(error); - tracing::trace!( - target: "xcm::buy_weight", + target: "xcm::weight_trader", ?error, %weight_trader, - "Weight trader failed", + "Getting weight price failed", ); } } )* ); tracing::trace!( - target: "xcm::buy_weight", - "Buy weight failed", + target: "xcm::weight_trader", + "Getting weight price failed", ); - // if we have multiple traders, and first one returns `TooExpensive` and others fail e.g. - // `AssetNotFound` then it is more accurate to return `TooExpensive` then `AssetNotFound` - Err(if too_expensive_error_found { - XcmError::TooExpensive - } else { - last_error.unwrap_or(XcmError::TooExpensive) - }) + Err(XcmError::TooExpensive) } - fn refund_weight(&mut self, weight: Weight, context: &XcmContext) -> Option { + fn refund_amount( + weight: &Weight, + used_asset_id: &AssetId, + paid_amount: u128, + context: Option<&XcmContext>, + ) -> Option { for_tuples!( #( - if let Some(asset) = Tuple.refund_weight(weight, context) { - return Some(asset); + let weight_trader = core::any::type_name::(); + + match Tuple::refund_amount(weight, used_asset_id, paid_amount, context) { + Some(refund_amount) => { + tracing::trace!( + target: "xcm::weight_trader", + %weight_trader, + "Getting refund amount succeeded", + ); + + return Some(refund_amount); + }, + None => { + tracing::trace!( + target: "xcm::weight_trader", + %weight_trader, + "Getting refund amount failed" + ); + } } )* ); + + tracing::trace!( + target: "xcm::weight_trader", + "Getting refund amount failed", + ); + None } + + fn take_fee(asset_id: &AssetId, amount: u128) -> bool { + for_tuples!( #( + let weight_trader = core::any::type_name::(); + + if Tuple::take_fee(asset_id, amount) { + tracing::trace!( + target: "xcm::weight_trader", + %weight_trader, + "Asset is taken", + ); + return true; + } else { + tracing::trace!( + target: "xcm::weight_trader", + %weight_trader, + "Asset is skipped", + ); + } + )* ); + + tracing::trace!( + target: "xcm::weight_trader", + "All assets are skipped", + ); + + false + } +} + +// FIXME docs +/// Must not be used in production +pub mod testing { + use super::*; + + pub trait TraderTest { + fn test_buy_weight( + &mut self, + weight: Weight, + max_payment: Asset, + ) -> Result<(AssetId, u128), XcmError>; + + fn test_refund_weight(&mut self, weight: Weight) -> Option<(AssetId, u128)>; + + fn test_take_fee(self); + } }