diff --git a/Cargo.lock b/Cargo.lock index 0877a5628de6..7ac25ecae412 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8370,6 +8370,7 @@ dependencies = [ "pallet-transaction-payment", "pallet-treasury", "pallet-vesting", + "pallet-xcm-benchmarks", "parity-scale-codec", "polkadot-primitives", "polkadot-primitives-test-helpers", @@ -8393,6 +8394,7 @@ dependencies = [ "sp-std", "static_assertions", "xcm", + "xcm-executor", ] [[package]] diff --git a/runtime/common/Cargo.toml b/runtime/common/Cargo.toml index dda7c2e92368..832f51e4ec9b 100644 --- a/runtime/common/Cargo.toml +++ b/runtime/common/Cargo.toml @@ -50,6 +50,9 @@ runtime-parachains = { package = "polkadot-runtime-parachains", path = "../parac slot-range-helper = { path = "slot_range_helper", default-features = false } xcm = { path = "../../xcm", default-features = false } +xcm-executor = { path = "../../xcm/xcm-executor", default-features = false, optional = true } + +pallet-xcm-benchmarks = { path = "../../xcm/pallet-xcm-benchmarks", default-features = false, optional = true } [dev-dependencies] hex-literal = "0.4.1" @@ -111,7 +114,9 @@ runtime-benchmarks = [ "frame-system/runtime-benchmarks", "runtime-parachains/runtime-benchmarks", "pallet-babe/runtime-benchmarks", - "pallet-fast-unstake/runtime-benchmarks" + "pallet-fast-unstake/runtime-benchmarks", + "pallet-xcm-benchmarks/runtime-benchmarks", + "xcm-executor", ] try-runtime = [ "runtime-parachains/try-runtime", diff --git a/runtime/common/src/xcm_sender.rs b/runtime/common/src/xcm_sender.rs index ff529143c509..1dfa83f57deb 100644 --- a/runtime/common/src/xcm_sender.rs +++ b/runtime/common/src/xcm_sender.rs @@ -109,6 +109,96 @@ impl( + sp_std::marker::PhantomData<( + XcmConfig, + ExistentialDeposit, + PriceForDelivery, + ParaId, + ToParaIdHelper, + )>, +); + +#[cfg(feature = "runtime-benchmarks")] +impl< + XcmConfig: xcm_executor::Config, + ExistentialDeposit: Get>, + PriceForDelivery: PriceForParachainDelivery, + Parachain: Get, + ToParachainHelper: EnsureForParachain, + > pallet_xcm_benchmarks::EnsureDelivery + for ToParachainDeliveryHelper< + XcmConfig, + ExistentialDeposit, + PriceForDelivery, + Parachain, + ToParachainHelper, + > +{ + fn ensure_successful_delivery( + origin_ref: &MultiLocation, + _dest: &MultiLocation, + fee_reason: xcm_executor::traits::FeeReason, + ) -> (Option, Option) { + use xcm_executor::{ + traits::{FeeManager, TransactAsset}, + FeesMode, + }; + + let mut fees_mode = None; + if !XcmConfig::FeeManager::is_waived(Some(origin_ref), fee_reason) { + // if not waived, we need to set up accounts for paying and receiving fees + + // mint ED to origin if needed + if let Some(ed) = ExistentialDeposit::get() { + XcmConfig::AssetTransactor::deposit_asset(&ed, &origin_ref, None).unwrap(); + } + + // overestimate delivery fee + let overestimated_xcm = vec![ClearOrigin; 128].into(); + let overestimated_fees = PriceForDelivery::price_for_parachain_delivery( + Parachain::get(), + &overestimated_xcm, + ); + + // mint overestimated fee to origin + for fee in overestimated_fees.inner() { + XcmConfig::AssetTransactor::deposit_asset(&fee, &origin_ref, None).unwrap(); + } + + // allow more initialization for target parachain + ToParachainHelper::ensure(Parachain::get()); + + // expected worst case - direct withdraw + fees_mode = Some(FeesMode { jit_withdraw: true }); + } + (fees_mode, None) + } +} + +/// Ensure more initialization for `ParaId`. (e.g. open HRMP channels, ...) +#[cfg(feature = "runtime-benchmarks")] +pub trait EnsureForParachain { + fn ensure(para_id: ParaId); +} +#[cfg(feature = "runtime-benchmarks")] +impl EnsureForParachain for () { + fn ensure(_para_id: ParaId) { + // doing nothing + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 6c4ba35c665d..4d9f29878180 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -2117,9 +2117,24 @@ sp_api::impl_runtime_apis! { impl pallet_nomination_pools_benchmarking::Config for Runtime {} impl runtime_parachains::disputes::slashing::benchmarking::Config for Runtime {} + parameter_types! { + pub ExistentialDepositMultiAsset: Option = Some(( + TokenLocation::get(), + ExistentialDeposit::get() + ).into()); + pub ToParachain: ParaId = kusama_runtime_constants::system_parachain::ASSET_HUB_ID.into(); + } + impl pallet_xcm_benchmarks::Config for Runtime { type XcmConfig = XcmConfig; type AccountIdConverter = SovereignAccountOf; + type DeliveryHelper = runtime_common::xcm_sender::ToParachainDeliveryHelper< + XcmConfig, + ExistentialDepositMultiAsset, + xcm_config::PriceForChildParachainDelivery, + ToParachain, + (), + >; fn valid_destination() -> Result { Ok(AssetHub::get()) } diff --git a/runtime/kusama/src/xcm_config.rs b/runtime/kusama/src/xcm_config.rs index df58f9196656..a1a02af71cee 100644 --- a/runtime/kusama/src/xcm_config.rs +++ b/runtime/kusama/src/xcm_config.rs @@ -115,15 +115,14 @@ parameter_types! { pub const BaseDeliveryFee: u128 = CENTS.saturating_mul(3); } +pub type PriceForChildParachainDelivery = + ExponentialPrice; + /// The XCM router. When we want to send an XCM message, we use this type. It amalgamates all of our /// individual routers. pub type XcmRouter = WithUniqueTopic<( // Only one router so far - use DMP to communicate with child parachains. - ChildParachainRouter< - Runtime, - XcmPallet, - ExponentialPrice, - >, + ChildParachainRouter, )>; parameter_types! { @@ -146,6 +145,9 @@ match_types! { pub type OnlyParachains: impl Contains = { MultiLocation { parents: 0, interior: X1(Parachain(_)) } }; + pub type HereLocation: impl Contains = { + MultiLocation { parents: 0, interior: Here } + }; } /// The barriers one of which must be passed for an XCM message to be executed. @@ -346,7 +348,8 @@ impl xcm_executor::Config for XcmConfig { type SubscriptionService = XcmPallet; type PalletInstancesInfo = AllPalletsWithSystem; type MaxAssetsIntoHolding = MaxAssetsIntoHolding; - type FeeManager = XcmFeesToAccount; + type FeeManager = + XcmFeesToAccount; // No bridges yet... type MessageExporter = (); type UniversalAliases = Nothing; diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 5f7d4dec8f19..3acce374d6fe 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -2076,9 +2076,24 @@ sp_api::impl_runtime_apis! { let treasury_key = frame_system::Account::::hashed_key_for(Treasury::account_id()); whitelist.push(treasury_key.to_vec().into()); + parameter_types! { + pub ExistentialDepositMultiAsset: Option = Some(( + TokenLocation::get(), + ExistentialDeposit::get() + ).into()); + pub ToParachain: ParaId = polkadot_runtime_constants::system_parachain::ASSET_HUB_ID.into(); + } + impl pallet_xcm_benchmarks::Config for Runtime { type XcmConfig = XcmConfig; type AccountIdConverter = SovereignAccountOf; + type DeliveryHelper = runtime_common::xcm_sender::ToParachainDeliveryHelper< + XcmConfig, + ExistentialDepositMultiAsset, + xcm_config::PriceForChildParachainDelivery, + ToParachain, + (), + >; fn valid_destination() -> Result { Ok(AssetHubLocation::get()) } diff --git a/runtime/polkadot/src/xcm_config.rs b/runtime/polkadot/src/xcm_config.rs index 9481a971ff90..3175780b384d 100644 --- a/runtime/polkadot/src/xcm_config.rs +++ b/runtime/polkadot/src/xcm_config.rs @@ -120,15 +120,14 @@ parameter_types! { pub const BaseDeliveryFee: u128 = CENTS.saturating_mul(3); } +pub type PriceForChildParachainDelivery = + ExponentialPrice; + /// The XCM router. When we want to send an XCM message, we use this type. It amalgamates all of our /// individual routers. pub type XcmRouter = ( // Only one router so far - use DMP to communicate with child parachains. - ChildParachainRouter< - Runtime, - XcmPallet, - ExponentialPrice, - >, + ChildParachainRouter, ); parameter_types! { @@ -157,6 +156,9 @@ match_types! { MultiLocation { parents: 0, interior: X1(Parachain(COLLECTIVES_ID)) } | MultiLocation { parents: 0, interior: X2(Parachain(COLLECTIVES_ID), Plurality { id: BodyId::Technical, .. }) } }; + pub type HereLocation: impl Contains = { + MultiLocation { parents: 0, interior: Here } + }; } /// The barriers one of which must be passed for an XCM message to be executed. @@ -347,7 +349,8 @@ impl xcm_executor::Config for XcmConfig { type SubscriptionService = XcmPallet; type PalletInstancesInfo = AllPalletsWithSystem; type MaxAssetsIntoHolding = MaxAssetsIntoHolding; - type FeeManager = XcmFeesToAccount; + type FeeManager = + XcmFeesToAccount; // No bridges yet... type MessageExporter = (); type UniversalAliases = Nothing; diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index e3e3776b603e..86f07053eaaa 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -2094,11 +2094,26 @@ sp_api::impl_runtime_apis! { LocalCheckAccount, LocationConverter, AssetHub, TokenLocation, XcmConfig, }; + parameter_types! { + pub ExistentialDepositMultiAsset: Option = Some(( + TokenLocation::get(), + ExistentialDeposit::get() + ).into()); + pub ToParachain: ParaId = rococo_runtime_constants::system_parachain::ASSET_HUB_ID.into(); + } + impl frame_system_benchmarking::Config for Runtime {} impl frame_benchmarking::baseline::Config for Runtime {} impl pallet_xcm_benchmarks::Config for Runtime { type XcmConfig = XcmConfig; type AccountIdConverter = LocationConverter; + type DeliveryHelper = runtime_common::xcm_sender::ToParachainDeliveryHelper< + XcmConfig, + ExistentialDepositMultiAsset, + xcm_config::PriceForChildParachainDelivery, + ToParachain, + (), + >; fn valid_destination() -> Result { Ok(AssetHub::get()) } @@ -2116,10 +2131,7 @@ sp_api::impl_runtime_apis! { AssetHub::get(), MultiAsset { fun: Fungible(1 * UNITS), id: Concrete(TokenLocation::get()) }, )); - pub const TrustedReserve: Option<(MultiLocation, MultiAsset)> = Some(( - AssetHub::get(), - MultiAsset { fun: Fungible(1 * UNITS), id: Concrete(TokenLocation::get()) }, - )); + pub const TrustedReserve: Option<(MultiLocation, MultiAsset)> = None; } impl pallet_xcm_benchmarks::fungible::Config for Runtime { diff --git a/runtime/rococo/src/xcm_config.rs b/runtime/rococo/src/xcm_config.rs index f0b6e509dda0..33d48ad32cbd 100644 --- a/runtime/rococo/src/xcm_config.rs +++ b/runtime/rococo/src/xcm_config.rs @@ -96,15 +96,14 @@ parameter_types! { pub const BaseDeliveryFee: u128 = CENTS.saturating_mul(3); } +pub type PriceForChildParachainDelivery = + ExponentialPrice; + /// The XCM router. When we want to send an XCM message, we use this type. It amalgamates all of our /// individual routers. pub type XcmRouter = WithUniqueTopic<( // Only one router so far - use DMP to communicate with child parachains. - ChildParachainRouter< - Runtime, - XcmPallet, - ExponentialPrice, - >, + ChildParachainRouter, )>; parameter_types! { @@ -140,6 +139,9 @@ match_types! { pub type OnlyParachains: impl Contains = { MultiLocation { parents: 0, interior: X1(Parachain(_)) } }; + pub type HereLocation: impl Contains = { + MultiLocation { parents: 0, interior: Here } + }; } /// The barriers one of which must be passed for an XCM message to be executed. @@ -319,7 +321,8 @@ impl xcm_executor::Config for XcmConfig { type SubscriptionService = XcmPallet; type PalletInstancesInfo = AllPalletsWithSystem; type MaxAssetsIntoHolding = MaxAssetsIntoHolding; - type FeeManager = XcmFeesToAccount; + type FeeManager = + XcmFeesToAccount; type MessageExporter = (); type UniversalAliases = Nothing; type CallDispatcher = WithOriginFilter; diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 29273ce4059f..f8e53560624e 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1860,6 +1860,7 @@ sp_api::impl_runtime_apis! { impl pallet_xcm_benchmarks::Config for Runtime { type XcmConfig = xcm_config::XcmConfig; type AccountIdConverter = xcm_config::LocationConverter; + type DeliveryHelper = (); fn valid_destination() -> Result { Ok(AssetHub::get()) } diff --git a/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs b/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs index 9e51fa83de90..89c6e5e4325c 100644 --- a/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs +++ b/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs @@ -24,7 +24,7 @@ use frame_support::{ use sp_runtime::traits::{Bounded, Zero}; use sp_std::{prelude::*, vec}; use xcm::latest::prelude::*; -use xcm_executor::traits::{ConvertLocation, TransactAsset}; +use xcm_executor::traits::{ConvertLocation, FeeReason, TransactAsset}; benchmarks_instance_pallet! { where_clause { where @@ -87,12 +87,28 @@ benchmarks_instance_pallet! { let dest_location = T::valid_destination()?; let dest_account = T::AccountIdConverter::convert_location(&dest_location).unwrap(); + use crate::EnsureDelivery; + let (expected_fees_mode, expected_assets_in_holding) = T::DeliveryHelper::ensure_successful_delivery( + &sender_location, + &dest_location, + FeeReason::TransferReserveAsset + ); + let sender_account_balance_before = T::TransactAsset::balance(&sender_account); + let asset = T::get_multi_asset(); >::deposit_asset(&asset, &sender_location, None).unwrap(); + assert!(T::TransactAsset::balance(&sender_account) > sender_account_balance_before); let assets: MultiAssets = vec![ asset ].into(); assert!(T::TransactAsset::balance(&dest_account).is_zero()); let mut executor = new_executor::(sender_location); + if let Some(expected_fees_mode) = expected_fees_mode { + executor.set_fees_mode(expected_fees_mode); + } + if let Some(expected_assets_in_holding) = expected_assets_in_holding { + executor.set_holding(expected_assets_in_holding.into()); + } + let instruction = Instruction::TransferReserveAsset { assets, dest: dest_location, @@ -102,7 +118,7 @@ benchmarks_instance_pallet! { }: { executor.bench_process(xcm)?; } verify { - assert!(T::TransactAsset::balance(&sender_account).is_zero()); + assert!(T::TransactAsset::balance(&sender_account) <= sender_account_balance_before); assert!(!T::TransactAsset::balance(&dest_account).is_zero()); // TODO: Check sender queue is not empty. #4426 } diff --git a/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs b/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs index f5759afc0646..dd23f9222772 100644 --- a/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs +++ b/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs @@ -157,6 +157,7 @@ impl xcm_executor::Config for XcmConfig { impl crate::Config for Test { type XcmConfig = XcmConfig; type AccountIdConverter = AccountIdConverter; + type DeliveryHelper = (); fn valid_destination() -> Result { let valid_destination: MultiLocation = X1(AccountId32 { network: None, id: [0u8; 32] }).into(); diff --git a/xcm/pallet-xcm-benchmarks/src/generic/mock.rs b/xcm/pallet-xcm-benchmarks/src/generic/mock.rs index 6f9d42df553d..7099542dd415 100644 --- a/xcm/pallet-xcm-benchmarks/src/generic/mock.rs +++ b/xcm/pallet-xcm-benchmarks/src/generic/mock.rs @@ -140,6 +140,7 @@ impl xcm_executor::Config for XcmConfig { impl crate::Config for Test { type XcmConfig = XcmConfig; type AccountIdConverter = AccountIdConverter; + type DeliveryHelper = (); fn valid_destination() -> Result { let valid_destination: MultiLocation = Junction::AccountId32 { network: None, id: [0u8; 32] }.into(); diff --git a/xcm/pallet-xcm-benchmarks/src/lib.rs b/xcm/pallet-xcm-benchmarks/src/lib.rs index c6a963435953..3bf4aea1b25e 100644 --- a/xcm/pallet-xcm-benchmarks/src/lib.rs +++ b/xcm/pallet-xcm-benchmarks/src/lib.rs @@ -22,7 +22,10 @@ use codec::Encode; use frame_benchmarking::{account, BenchmarkError}; use sp_std::prelude::*; use xcm::latest::prelude::*; -use xcm_executor::{traits::ConvertLocation, Config as XcmConfig}; +use xcm_executor::{ + traits::{ConvertLocation, FeeReason}, + Config as XcmConfig, FeesMode, +}; pub mod fungible; pub mod generic; @@ -41,6 +44,9 @@ pub trait Config: frame_system::Config { /// A converter between a multi-location to a sovereign account. type AccountIdConverter: ConvertLocation; + /// Helper that ensures successful delivery for XCM instructions which need `SendXcm`. + type DeliveryHelper: EnsureDelivery; + /// Does any necessary setup to create a valid destination for XCM messages. /// Returns that destination's multi-location to be used in benchmarks. fn valid_destination() -> Result; @@ -108,3 +114,29 @@ pub fn account_and_location(index: u32) -> (T::AccountId, MultiLocati (account, location) } + +/// Trait for a type which ensures all requirements for successful delivery with XCM transport +/// layers. +pub trait EnsureDelivery { + /// Prepare all requirements for successful `XcmSender: SendXcm` passing (accounts, balances, + /// channels ...). Returns: + /// - possible `FeesMode` which is expected to be set to executor + /// - possible `MultiAssets` which are expected to be subsume to the Holding Register + fn ensure_successful_delivery( + origin_ref: &MultiLocation, + dest: &MultiLocation, + fee_reason: FeeReason, + ) -> (Option, Option); +} + +/// `()` implementation does nothing which means no special requirements for environment. +impl EnsureDelivery for () { + fn ensure_successful_delivery( + _origin_ref: &MultiLocation, + _dest: &MultiLocation, + _fee_reason: FeeReason, + ) -> (Option, Option) { + // doing nothing + (None, None) + } +} diff --git a/xcm/pallet-xcm/src/mock.rs b/xcm/pallet-xcm/src/mock.rs index b56b1af82def..3dd134061517 100644 --- a/xcm/pallet-xcm/src/mock.rs +++ b/xcm/pallet-xcm/src/mock.rs @@ -16,8 +16,8 @@ use codec::Encode; use frame_support::{ - construct_runtime, parameter_types, - traits::{ConstU32, Everything, Nothing}, + construct_runtime, match_types, parameter_types, + traits::{ConstU32, Everything, EverythingBut, Nothing}, weights::Weight, }; use frame_system::EnsureRoot; @@ -25,14 +25,16 @@ use polkadot_parachain::primitives::Id as ParaId; use polkadot_runtime_parachains::origin; use sp_core::H256; use sp_runtime::{traits::IdentityLookup, AccountId32, BuildStorage}; -pub use sp_std::{cell::RefCell, fmt::Debug, marker::PhantomData}; +pub use sp_std::{ + cell::RefCell, collections::btree_map::BTreeMap, fmt::Debug, marker::PhantomData, +}; use xcm::prelude::*; use xcm_builder::{ AccountId32Aliases, AllowKnownQueryResponses, AllowSubscriptionsFrom, AllowTopLevelPaidExecutionFrom, Case, ChildParachainAsNative, ChildParachainConvertsVia, ChildSystemParachainAsSuperuser, CurrencyAdapter as XcmCurrencyAdapter, FixedRateOfFungible, FixedWeightBounds, IsConcrete, SignedAccountId32AsNative, SignedToAccountId32, - SovereignSignedViaLocation, TakeWeightCredit, + SovereignSignedViaLocation, TakeWeightCredit, XcmFeesToAccount, }; use xcm_executor::XcmExecutor; @@ -154,7 +156,7 @@ pub(crate) fn take_sent_xcm() -> Vec<(MultiLocation, Xcm<()>)> { r }) } -/// Sender that never returns error, always sends +/// Sender that never returns error. pub struct TestSendXcm; impl SendXcm for TestSendXcm { type Ticket = (MultiLocation, Xcm<()>); @@ -193,6 +195,38 @@ impl SendXcm for TestSendXcmErrX8 { } } +parameter_types! { + pub Para3000: u32 = 3000; + pub Para3000Location: MultiLocation = Parachain(Para3000::get()).into(); + pub Para3000PaymentAmount: u128 = 1; + pub Para3000PaymentMultiAssets: MultiAssets = MultiAssets::from(MultiAsset::from((Here, Para3000PaymentAmount::get()))); +} +/// Sender only sends to `Parachain(3000)` destination requiring payment. +pub struct TestPaidForPara3000SendXcm; +impl SendXcm for TestPaidForPara3000SendXcm { + type Ticket = (MultiLocation, Xcm<()>); + fn validate( + dest: &mut Option, + msg: &mut Option>, + ) -> SendResult<(MultiLocation, Xcm<()>)> { + if let Some(dest) = dest.as_ref() { + if !dest.eq(&Para3000Location::get()) { + return Err(SendError::NotApplicable) + } + } else { + return Err(SendError::NotApplicable) + } + + let pair = (dest.take().unwrap(), msg.take().unwrap()); + Ok((pair, Para3000PaymentMultiAssets::get())) + } + fn deliver(pair: (MultiLocation, Xcm<()>)) -> Result { + let hash = fake_message_hash(&pair.1); + SENT_XCM.with(|q| q.borrow_mut().push(pair)); + Ok(hash) + } +} + parameter_types! { pub const BlockHashCount: u64 = 250; } @@ -271,6 +305,14 @@ parameter_types! { pub TrustedAssets: (MultiAssetFilter, MultiLocation) = (All.into(), Here.into()); pub const MaxInstructions: u32 = 100; pub const MaxAssetsIntoHolding: u32 = 64; + pub XcmFeesTargetAccount: AccountId = AccountId::new([167u8; 32]); +} + +pub const XCM_FEES_NOT_WAIVED_USER_ACCOUNT: [u8; 32] = [37u8; 32]; +match_types! { + pub type XcmFeesNotWaivedLocations: impl Contains = { + MultiLocation { parents: 0, interior: X1(Junction::AccountId32 {network: None, id: XCM_FEES_NOT_WAIVED_USER_ACCOUNT})} + }; } pub type Barrier = ( @@ -283,7 +325,7 @@ pub type Barrier = ( pub struct XcmConfig; impl xcm_executor::Config for XcmConfig { type RuntimeCall = RuntimeCall; - type XcmSender = TestSendXcm; + type XcmSender = (TestPaidForPara3000SendXcm, TestSendXcm); type AssetTransactor = LocalAssetTransactor; type OriginConverter = LocalOriginConverter; type IsReserve = (); @@ -300,7 +342,12 @@ impl xcm_executor::Config for XcmConfig { type SubscriptionService = XcmPallet; type PalletInstancesInfo = AllPalletsWithSystem; type MaxAssetsIntoHolding = MaxAssetsIntoHolding; - type FeeManager = (); + type FeeManager = XcmFeesToAccount< + Self, + EverythingBut, + AccountId, + XcmFeesTargetAccount, + >; type MessageExporter = (); type UniversalAliases = Nothing; type CallDispatcher = RuntimeCall; @@ -322,7 +369,7 @@ parameter_types! { impl pallet_xcm::Config for Test { type RuntimeEvent = RuntimeEvent; type SendXcmOrigin = xcm_builder::EnsureXcmOrigin; - type XcmRouter = (TestSendXcmErrX8, TestSendXcm); + type XcmRouter = (TestSendXcmErrX8, TestPaidForPara3000SendXcm, TestSendXcm); type ExecuteXcmOrigin = xcm_builder::EnsureXcmOrigin; type XcmExecuteFilter = Everything; type XcmExecutor = XcmExecutor; diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index 6ff9f1d893c8..88454c1758a6 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -522,6 +522,74 @@ fn reserve_transfer_assets_works() { }); } +/// Test `reserve_transfer_assets_with_paid_router_works` +/// +/// Asserts that the sender's balance is decreased and the beneficiary's balance +/// is increased. Verifies the correct message is sent and event is emitted. +/// Verifies that XCM router fees (`SendXcm::validate` -> `MultiAssets`) are withdrawn from correct +/// user account and deposited to a correct target account (`XcmFeesTargetAccount`). +#[test] +fn reserve_transfer_assets_with_paid_router_works() { + let user_account = AccountId::from(XCM_FEES_NOT_WAIVED_USER_ACCOUNT); + let paid_para_id = Para3000::get(); + let balances = vec![ + (user_account.clone(), INITIAL_BALANCE), + (ParaId::from(paid_para_id).into_account_truncating(), INITIAL_BALANCE), + (XcmFeesTargetAccount::get(), INITIAL_BALANCE), + ]; + new_test_ext_with_balances(balances).execute_with(|| { + let xcm_router_fee_amount = Para3000PaymentAmount::get(); + let weight = BaseXcmWeight::get() * 2; + let dest: MultiLocation = + Junction::AccountId32 { network: None, id: user_account.clone().into() }.into(); + assert_eq!(Balances::total_balance(&user_account), INITIAL_BALANCE); + assert_ok!(XcmPallet::reserve_transfer_assets( + RuntimeOrigin::signed(user_account.clone()), + Box::new(Parachain(paid_para_id).into()), + Box::new(dest.into()), + Box::new((Here, SEND_AMOUNT).into()), + 0, + )); + // check event + assert_eq!( + last_event(), + RuntimeEvent::XcmPallet(crate::Event::Attempted { outcome: Outcome::Complete(weight) }) + ); + + // XCM_FEES_NOT_WAIVED_USER_ACCOUNT spent amount + assert_eq!( + Balances::free_balance(user_account), + INITIAL_BALANCE - SEND_AMOUNT - xcm_router_fee_amount + ); + // Destination account (parachain account) has amount + let para_acc: AccountId = ParaId::from(paid_para_id).into_account_truncating(); + assert_eq!(Balances::free_balance(para_acc), INITIAL_BALANCE + SEND_AMOUNT); + // XcmFeesTargetAccount where should lend xcm_router_fee_amount + assert_eq!( + Balances::free_balance(XcmFeesTargetAccount::get()), + INITIAL_BALANCE + xcm_router_fee_amount + ); + assert_eq!( + sent_xcm(), + vec![( + Parachain(paid_para_id).into(), + Xcm(vec![ + ReserveAssetDeposited((Parent, SEND_AMOUNT).into()), + ClearOrigin, + buy_limited_execution((Parent, SEND_AMOUNT), Weight::from_parts(4000, 4000)), + DepositAsset { assets: AllCounted(1).into(), beneficiary: dest }, + ]), + )] + ); + let versioned_sent = VersionedXcm::from(sent_xcm().into_iter().next().unwrap().1); + let _check_v2_ok: xcm::v2::Xcm<()> = versioned_sent.try_into().unwrap(); + assert_eq!( + last_event(), + RuntimeEvent::XcmPallet(crate::Event::Attempted { outcome: Outcome::Complete(weight) }) + ); + }); +} + /// Test `limited_reserve_transfer_assets` /// /// Asserts that the sender's balance is decreased and the beneficiary's balance diff --git a/xcm/xcm-builder/src/fee_handling.rs b/xcm/xcm-builder/src/fee_handling.rs index 207d58d1fa00..049055471d32 100644 --- a/xcm/xcm-builder/src/fee_handling.rs +++ b/xcm/xcm-builder/src/fee_handling.rs @@ -37,16 +37,8 @@ impl< > FeeManager for XcmFeesToAccount { fn is_waived(origin: Option<&MultiLocation>, _: FeeReason) -> bool { - #[cfg(feature = "runtime-benchmarks")] - { - let _ = origin; - true - } - #[cfg(not(feature = "runtime-benchmarks"))] - { - let Some(loc) = origin else { return false }; - WaivedLocations::contains(loc) - } + let Some(loc) = origin else { return false }; + WaivedLocations::contains(loc) } fn handle_fee(fees: MultiAssets, context: Option<&XcmContext>) { diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index bddfe0884dab..e11ec2630e43 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -405,10 +405,7 @@ impl XcmExecutor { reason: FeeReason, ) -> Result { let (ticket, fee) = validate_send::(dest, msg)?; - if !Config::FeeManager::is_waived(self.origin_ref(), reason) { - let paid = self.holding.try_take(fee.into()).map_err(|_| XcmError::NotHoldingFees)?; - Config::FeeManager::handle_fee(paid.into(), Some(&self.context)); - } + self.take_fee(fee, reason)?; Config::XcmSender::deliver(ticket).map_err(Into::into) } @@ -948,6 +945,14 @@ impl XcmExecutor { if Config::FeeManager::is_waived(self.origin_ref(), reason) { return Ok(()) } + log::trace!( + target: "xcm::fees", + "taking fee: {:?} from origin_ref: {:?} in fees_mode: {:?} for a reason: {:?}", + fee, + self.origin_ref(), + self.fees_mode, + reason, + ); let paid = if self.fees_mode.jit_withdraw { let origin = self.origin_ref().ok_or(XcmError::BadOrigin)?; for asset in fee.inner() { @@ -989,12 +994,7 @@ impl XcmExecutor { let QueryResponseInfo { destination, query_id, max_weight } = info; let instruction = QueryResponse { query_id, response, max_weight, querier }; let message = Xcm(vec![instruction]); - let (ticket, fee) = validate_send::(destination, message)?; - if !Config::FeeManager::is_waived(self.origin_ref(), fee_reason) { - let paid = self.holding.try_take(fee.into()).map_err(|_| XcmError::NotHoldingFees)?; - Config::FeeManager::handle_fee(paid.into(), Some(&self.context)); - } - Config::XcmSender::deliver(ticket).map_err(Into::into) + self.send(destination, message, fee_reason) } fn try_reanchor( diff --git a/xcm/xcm-executor/src/traits/fee_manager.rs b/xcm/xcm-executor/src/traits/fee_manager.rs index 588a246ce17f..495f021e7252 100644 --- a/xcm/xcm-executor/src/traits/fee_manager.rs +++ b/xcm/xcm-executor/src/traits/fee_manager.rs @@ -27,7 +27,7 @@ pub trait FeeManager { } /// Context under which a fee is paid. -#[derive(Copy, Clone, Eq, PartialEq)] +#[derive(Copy, Clone, Debug, Eq, PartialEq)] pub enum FeeReason { /// When a reporting instruction is called. Report,