From 80f31ec3df0303261680c98a44e5006813e2515e Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Mon, 7 Aug 2023 14:35:05 +0200 Subject: [PATCH 01/13] Test for `pallet_xcm::reserve_transfer_assets` with paid XcmRouter --- xcm/pallet-xcm/src/mock.rs | 57 ++++++++++++++++++++++++---- xcm/pallet-xcm/src/tests.rs | 74 +++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 7 deletions(-) diff --git a/xcm/pallet-xcm/src/mock.rs b/xcm/pallet-xcm/src/mock.rs index b56b1af82def..5ca3371b4499 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; @@ -143,6 +145,7 @@ construct_runtime!( thread_local! { pub static SENT_XCM: RefCell)>> = RefCell::new(Vec::new()); + pub static XCM_ROUTER_FEES: RefCell> = RefCell::new(BTreeMap::new()); } pub(crate) fn sent_xcm() -> Vec<(MultiLocation, Xcm<()>)> { SENT_XCM.with(|q| (*q.borrow()).clone()) @@ -154,7 +157,24 @@ pub(crate) fn take_sent_xcm() -> Vec<(MultiLocation, Xcm<()>)> { r }) } -/// Sender that never returns error, always sends +pub(crate) fn set_router_fee_for_destination(dest: MultiLocation, fees: Option) { + XCM_ROUTER_FEES.with(|q| { + if let Some(fees) = fees { + q.borrow_mut() + .entry(dest) + .and_modify(|old_value| *old_value = fees.clone()) + .or_insert(fees); + } else { + let _ = q.borrow_mut().remove_entry(&dest); + } + }) +} +pub(crate) fn get_router_fee_for_destination(dest: &MultiLocation) -> Option { + XCM_ROUTER_FEES.with(|q| q.borrow().get(dest).map(Clone::clone)) +} + +/// Sender that never returns error, always sends. +/// By default does not return **fees**, **fees** can be managed per destination with `set_router_fee_for_destination`. pub struct TestSendXcm; impl SendXcm for TestSendXcm { type Ticket = (MultiLocation, Xcm<()>); @@ -163,7 +183,17 @@ impl SendXcm for TestSendXcm { msg: &mut Option>, ) -> SendResult<(MultiLocation, Xcm<()>)> { let pair = (dest.take().unwrap(), msg.take().unwrap()); - Ok((pair, MultiAssets::new())) + + // Check if fees are configured for dest + let maybe_payment = match get_router_fee_for_destination(&pair.0) { + Some(fees) => fees, + None => { + // can be change to None, but this was here before. + MultiAssets::new() + }, + }; + + Ok((pair, maybe_payment)) } fn deliver(pair: (MultiLocation, Xcm<()>)) -> Result { let hash = fake_message_hash(&pair.1); @@ -271,6 +301,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 = ( @@ -300,7 +338,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; diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index 2ad13dced936..f1d9798d6f13 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -525,6 +525,80 @@ 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() { + // set up router fees for destination + let xcm_router_fee_amount = 1; + set_router_fee_for_destination( + Parachain(PARA_ID).into(), + Some(MultiAssets::from((Here, xcm_router_fee_amount))), + ); + + let user_account = AccountId::from(XCM_FEES_NOT_WAIVED_USER_ACCOUNT); + + let balances = vec![ + (user_account.clone(), INITIAL_BALANCE), + (ParaId::from(PARA_ID).into_account_truncating(), INITIAL_BALANCE), + (XcmFeesTargetAccount::get(), INITIAL_BALANCE), + ]; + new_test_ext_with_balances(balances).execute_with(|| { + 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(PARA_ID).into()), + Box::new(dest.clone().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(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(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 From c1f882d22693fe99734cd9ac30cdc689bd9fbe02 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Mon, 7 Aug 2023 14:42:22 +0200 Subject: [PATCH 02/13] Fix another assert --- xcm/pallet-xcm/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index f1d9798d6f13..d560c9099e73 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -583,9 +583,9 @@ fn reserve_transfer_assets_with_paid_router_works() { vec![( Parachain(PARA_ID).into(), Xcm(vec![ - ReserveAssetDeposited((Parent, SEND_AMOUNT).into()), + ReserveAssetDeposited((Parent, SEND_AMOUNT - xcm_router_fee_amount).into()), ClearOrigin, - buy_limited_execution((Parent, SEND_AMOUNT), Weight::from_parts(4000, 4000)), + buy_limited_execution((Parent, SEND_AMOUNT - xcm_router_fee_amount), Weight::from_parts(4000, 4000)), DepositAsset { assets: AllCounted(1).into(), beneficiary: dest }, ]), )] From 28459c03d739b994c83230847ad616228dfb5f18 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Mon, 7 Aug 2023 15:15:17 +0200 Subject: [PATCH 03/13] Potential fix? --- xcm/pallet-xcm/src/tests.rs | 4 ++-- xcm/xcm-executor/src/lib.rs | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index d560c9099e73..f1d9798d6f13 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -583,9 +583,9 @@ fn reserve_transfer_assets_with_paid_router_works() { vec![( Parachain(PARA_ID).into(), Xcm(vec![ - ReserveAssetDeposited((Parent, SEND_AMOUNT - xcm_router_fee_amount).into()), + ReserveAssetDeposited((Parent, SEND_AMOUNT).into()), ClearOrigin, - buy_limited_execution((Parent, SEND_AMOUNT - xcm_router_fee_amount), Weight::from_parts(4000, 4000)), + buy_limited_execution((Parent, SEND_AMOUNT), Weight::from_parts(4000, 4000)), DepositAsset { assets: AllCounted(1).into(), beneficiary: dest }, ]), )] diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index ae99c6adac2d..c45dc8981ad5 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -405,8 +405,7 @@ impl XcmExecutor { ) -> 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) } From 7f876565dc02880ab2de1d083f60bbd67ff1e9fd Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Mon, 7 Aug 2023 15:30:14 +0200 Subject: [PATCH 04/13] Even easier --- xcm/xcm-executor/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index c45dc8981ad5..c7045bb4a9f9 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -404,9 +404,7 @@ impl XcmExecutor { reason: FeeReason, ) -> Result { let (ticket, fee) = validate_send::(dest, msg)?; - if !Config::FeeManager::is_waived(self.origin_ref(), reason) { - self.take_fee(fee, reason)?; - } + self.take_fee(fee, reason)?; Config::XcmSender::deliver(ticket).map_err(Into::into) } From 3774a14a58d7d22d27d721e21e5e005eded48dee Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Mon, 7 Aug 2023 17:33:06 +0200 Subject: [PATCH 05/13] Potential fix for `fn respond(` --- xcm/xcm-executor/src/lib.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index c7045bb4a9f9..7f1b1e9af690 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -982,12 +982,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( From 319956b2aefd16e0824e98712053ad9b7f438947 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Mon, 7 Aug 2023 18:02:34 +0200 Subject: [PATCH 06/13] `set_router_fee_for_destination` inside `new_test_ext` --- xcm/pallet-xcm/src/tests.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index f1d9798d6f13..a256d59b39a5 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -533,21 +533,20 @@ fn reserve_transfer_assets_works() { /// and deposited to a correct target account (`XcmFeesTargetAccount`). #[test] fn reserve_transfer_assets_with_paid_router_works() { - // set up router fees for destination - let xcm_router_fee_amount = 1; - set_router_fee_for_destination( - Parachain(PARA_ID).into(), - Some(MultiAssets::from((Here, xcm_router_fee_amount))), - ); - let user_account = AccountId::from(XCM_FEES_NOT_WAIVED_USER_ACCOUNT); - let balances = vec![ (user_account.clone(), INITIAL_BALANCE), (ParaId::from(PARA_ID).into_account_truncating(), INITIAL_BALANCE), (XcmFeesTargetAccount::get(), INITIAL_BALANCE), ]; new_test_ext_with_balances(balances).execute_with(|| { + // set up router fees for destination + let xcm_router_fee_amount = 1; + set_router_fee_for_destination( + Parachain(PARA_ID).into(), + Some(MultiAssets::from((Here, xcm_router_fee_amount))), + ); + let weight = BaseXcmWeight::get() * 2; let dest: MultiLocation = Junction::AccountId32 { network: None, id: user_account.clone().into() }.into(); From b694ce8dd9330dfc041d0c79731ca5fda380962d Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Tue, 8 Aug 2023 13:06:33 +0200 Subject: [PATCH 07/13] Added TestPaidForPara3000SendXcm to test --- xcm/pallet-xcm/src/mock.rs | 68 ++++++++++++++++++++----------------- xcm/pallet-xcm/src/tests.rs | 17 ++++------ 2 files changed, 42 insertions(+), 43 deletions(-) diff --git a/xcm/pallet-xcm/src/mock.rs b/xcm/pallet-xcm/src/mock.rs index 5ca3371b4499..3dd134061517 100644 --- a/xcm/pallet-xcm/src/mock.rs +++ b/xcm/pallet-xcm/src/mock.rs @@ -145,7 +145,6 @@ construct_runtime!( thread_local! { pub static SENT_XCM: RefCell)>> = RefCell::new(Vec::new()); - pub static XCM_ROUTER_FEES: RefCell> = RefCell::new(BTreeMap::new()); } pub(crate) fn sent_xcm() -> Vec<(MultiLocation, Xcm<()>)> { SENT_XCM.with(|q| (*q.borrow()).clone()) @@ -157,24 +156,7 @@ pub(crate) fn take_sent_xcm() -> Vec<(MultiLocation, Xcm<()>)> { r }) } -pub(crate) fn set_router_fee_for_destination(dest: MultiLocation, fees: Option) { - XCM_ROUTER_FEES.with(|q| { - if let Some(fees) = fees { - q.borrow_mut() - .entry(dest) - .and_modify(|old_value| *old_value = fees.clone()) - .or_insert(fees); - } else { - let _ = q.borrow_mut().remove_entry(&dest); - } - }) -} -pub(crate) fn get_router_fee_for_destination(dest: &MultiLocation) -> Option { - XCM_ROUTER_FEES.with(|q| q.borrow().get(dest).map(Clone::clone)) -} - -/// Sender that never returns error, always sends. -/// By default does not return **fees**, **fees** can be managed per destination with `set_router_fee_for_destination`. +/// Sender that never returns error. pub struct TestSendXcm; impl SendXcm for TestSendXcm { type Ticket = (MultiLocation, Xcm<()>); @@ -183,17 +165,7 @@ impl SendXcm for TestSendXcm { msg: &mut Option>, ) -> SendResult<(MultiLocation, Xcm<()>)> { let pair = (dest.take().unwrap(), msg.take().unwrap()); - - // Check if fees are configured for dest - let maybe_payment = match get_router_fee_for_destination(&pair.0) { - Some(fees) => fees, - None => { - // can be change to None, but this was here before. - MultiAssets::new() - }, - }; - - Ok((pair, maybe_payment)) + Ok((pair, MultiAssets::new())) } fn deliver(pair: (MultiLocation, Xcm<()>)) -> Result { let hash = fake_message_hash(&pair.1); @@ -223,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; } @@ -321,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 = (); @@ -365,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 a256d59b39a5..cf9d2cae2e29 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -534,26 +534,21 @@ fn reserve_transfer_assets_works() { #[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(PARA_ID).into_account_truncating(), INITIAL_BALANCE), + (ParaId::from(paid_para_id).into_account_truncating(), INITIAL_BALANCE), (XcmFeesTargetAccount::get(), INITIAL_BALANCE), ]; new_test_ext_with_balances(balances).execute_with(|| { - // set up router fees for destination - let xcm_router_fee_amount = 1; - set_router_fee_for_destination( - Parachain(PARA_ID).into(), - Some(MultiAssets::from((Here, xcm_router_fee_amount))), - ); - + 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(PARA_ID).into()), + Box::new(Parachain(paid_para_id).into()), Box::new(dest.clone().into()), Box::new((Here, SEND_AMOUNT).into()), 0, @@ -570,7 +565,7 @@ fn reserve_transfer_assets_with_paid_router_works() { INITIAL_BALANCE - SEND_AMOUNT - xcm_router_fee_amount ); // Destination account (parachain account) has amount - let para_acc: AccountId = ParaId::from(PARA_ID).into_account_truncating(); + 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!( @@ -580,7 +575,7 @@ fn reserve_transfer_assets_with_paid_router_works() { assert_eq!( sent_xcm(), vec![( - Parachain(PARA_ID).into(), + Parachain(paid_para_id).into(), Xcm(vec![ ReserveAssetDeposited((Parent, SEND_AMOUNT).into()), ClearOrigin, From 0a249a031d1b7c5f970d35f4c6ff707ece586d34 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Tue, 8 Aug 2023 16:44:47 +0200 Subject: [PATCH 08/13] TODO: remove dependency of `is_waived` on `runtime-benchmarks` --- xcm/xcm-builder/src/fee_handling.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) 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>) { From edefa352968f7c23e862e523ae652a7913207e00 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Wed, 9 Aug 2023 16:06:00 +0200 Subject: [PATCH 09/13] Fixed benchmarks to be `FeeManager/XcmFeesToAccount` aware --- runtime/kusama/src/lib.rs | 39 +++++++++++++++++++ runtime/kusama/src/xcm_config.rs | 6 ++- runtime/polkadot/src/lib.rs | 39 +++++++++++++++++++ runtime/polkadot/src/xcm_config.rs | 6 ++- runtime/rococo/src/lib.rs | 39 +++++++++++++++++++ runtime/rococo/src/xcm_config.rs | 6 ++- runtime/westend/src/lib.rs | 6 +++ .../src/fungible/benchmarking.rs | 11 +++++- .../src/fungible/mock.rs | 10 +++++ xcm/pallet-xcm-benchmarks/src/generic/mock.rs | 9 +++++ xcm/pallet-xcm-benchmarks/src/lib.rs | 13 ++++++- xcm/xcm-executor/src/lib.rs | 8 ++++ xcm/xcm-executor/src/traits/fee_manager.rs | 2 +- 13 files changed, 187 insertions(+), 7 deletions(-) diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 3a7a031fd55a..fee7ea491e54 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -2108,6 +2108,8 @@ sp_api::impl_runtime_apis! { use xcm_config::{ LocalCheckAccount, SovereignAccountOf, AssetHub, TokenLocation, XcmConfig, }; + use xcm_executor::FeesMode; + use xcm_executor::traits::FeeReason; impl pallet_session_benchmarking::Config for Runtime {} impl pallet_offences_benchmarking::Config for Runtime {} @@ -2130,6 +2132,43 @@ sp_api::impl_runtime_apis! { fun: Fungible(1_000_000 * UNITS), }].into() } + fn ensure_for_send(origin_ref: &MultiLocation, _dest: &MultiLocation, fee_reason: FeeReason) -> Option { + use xcm_executor::traits::{FeeManager, TransactAsset}; + + let mut fees_mode = None; + if !::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 + let ed = MultiAsset { + id: Concrete(TokenLocation::get()), + fun: Fungible(EXISTENTIAL_DEPOSIT.into()) + }; + ::AssetTransactor::deposit_asset(&ed, &origin_ref, None).unwrap(); + + // overestimate delivery fee + use runtime_common::xcm_sender::PriceForParachainDelivery; + let overestimated_xcm = vec![ClearOrigin; 128].into(); + let overestimated_fees = runtime_common::xcm_sender::ExponentialPrice::< + xcm_config::FeeAssetId, + xcm_config::BaseDeliveryFee, + TransactionByteFee, + Dmp + >::price_for_parachain_delivery( + kusama_runtime_constants::system_parachain::ASSET_HUB_ID.into(), + &overestimated_xcm + ); + + // mint overestimated fee to origin + for fee in overestimated_fees.inner() { + ::AssetTransactor::deposit_asset(&fee, &origin_ref, None).unwrap(); + } + + // expected worst case + fees_mode = Some(FeesMode { jit_withdraw: true }); + } + fees_mode + } } parameter_types! { diff --git a/runtime/kusama/src/xcm_config.rs b/runtime/kusama/src/xcm_config.rs index 3e7e51aad571..51045be8db5a 100644 --- a/runtime/kusama/src/xcm_config.rs +++ b/runtime/kusama/src/xcm_config.rs @@ -146,6 +146,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 +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/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index fc8c905f0877..1979c07ff757 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -2085,6 +2085,8 @@ sp_api::impl_runtime_apis! { use frame_benchmarking::baseline::Pallet as Baseline; use xcm::latest::prelude::*; use xcm_config::{XcmConfig, AssetHubLocation, TokenLocation, LocalCheckAccount, SovereignAccountOf}; + use xcm_executor::FeesMode; + use xcm_executor::traits::FeeReason; impl pallet_session_benchmarking::Config for Runtime {} impl pallet_offences_benchmarking::Config for Runtime {} @@ -2108,6 +2110,43 @@ sp_api::impl_runtime_apis! { // Polkadot only knows about DOT vec![MultiAsset { id: Concrete(TokenLocation::get()), fun: Fungible(1_000_000 * UNITS) }].into() } + fn ensure_for_send(origin_ref: &MultiLocation, _dest: &MultiLocation, fee_reason: FeeReason) -> Option { + use xcm_executor::traits::{FeeManager, TransactAsset}; + + let mut fees_mode = None; + if !::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 + let ed = MultiAsset { + id: Concrete(TokenLocation::get()), + fun: Fungible(EXISTENTIAL_DEPOSIT.into()) + }; + ::AssetTransactor::deposit_asset(&ed, &origin_ref, None).unwrap(); + + // overestimate delivery fee + use runtime_common::xcm_sender::PriceForParachainDelivery; + let overestimated_xcm = vec![ClearOrigin; 128].into(); + let overestimated_fees = runtime_common::xcm_sender::ExponentialPrice::< + xcm_config::FeeAssetId, + xcm_config::BaseDeliveryFee, + TransactionByteFee, + Dmp + >::price_for_parachain_delivery( + polkadot_runtime_constants::system_parachain::ASSET_HUB_ID.into(), + &overestimated_xcm + ); + + // mint overestimated fee to origin + for fee in overestimated_fees.inner() { + ::AssetTransactor::deposit_asset(&fee, &origin_ref, None).unwrap(); + } + + // expected worst case + fees_mode = Some(FeesMode { jit_withdraw: true }); + } + fees_mode + } } parameter_types! { diff --git a/runtime/polkadot/src/xcm_config.rs b/runtime/polkadot/src/xcm_config.rs index 5ae13cb89d16..6a921cf6896c 100644 --- a/runtime/polkadot/src/xcm_config.rs +++ b/runtime/polkadot/src/xcm_config.rs @@ -157,6 +157,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 +350,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 3ff32de89e64..7a9881ae3a96 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -2094,6 +2094,8 @@ sp_api::impl_runtime_apis! { use xcm_config::{ LocalCheckAccount, LocationConverter, AssetHub, TokenLocation, XcmConfig, }; + use xcm_executor::FeesMode; + use xcm_executor::traits::FeeReason; impl frame_system_benchmarking::Config for Runtime {} impl frame_benchmarking::baseline::Config for Runtime {} @@ -2110,6 +2112,43 @@ sp_api::impl_runtime_apis! { fun: Fungible(1_000_000 * UNITS), }].into() } + fn ensure_for_send(origin_ref: &MultiLocation, _dest: &MultiLocation, fee_reason: FeeReason) -> Option { + use xcm_executor::traits::{FeeManager, TransactAsset}; + + let mut fees_mode = None; + if !::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 + let ed = MultiAsset { + id: Concrete(TokenLocation::get()), + fun: Fungible(EXISTENTIAL_DEPOSIT.into()) + }; + ::AssetTransactor::deposit_asset(&ed, &origin_ref, None).unwrap(); + + // overestimate delivery fee + use runtime_common::xcm_sender::PriceForParachainDelivery; + let overestimated_xcm = vec![ClearOrigin; 128].into(); + let overestimated_fees = runtime_common::xcm_sender::ExponentialPrice::< + xcm_config::FeeAssetId, + xcm_config::BaseDeliveryFee, + TransactionByteFee, + Dmp + >::price_for_parachain_delivery( + rococo_runtime_constants::system_parachain::ASSET_HUB_ID.into(), + &overestimated_xcm + ); + + // mint overestimated fee to origin + for fee in overestimated_fees.inner() { + ::AssetTransactor::deposit_asset(&fee, &origin_ref, None).unwrap(); + } + + // expected worst case + fees_mode = Some(FeesMode { jit_withdraw: true }); + } + fees_mode + } } parameter_types! { diff --git a/runtime/rococo/src/xcm_config.rs b/runtime/rococo/src/xcm_config.rs index aa72dd3c9fc3..20348452d6c1 100644 --- a/runtime/rococo/src/xcm_config.rs +++ b/runtime/rococo/src/xcm_config.rs @@ -140,6 +140,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 +322,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 bf0db060b507..37d9e62f1b18 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1858,6 +1858,8 @@ sp_api::impl_runtime_apis! { MultiAsset, MultiAssets, MultiLocation, NetworkId, Response, }; use xcm_config::{AssetHub, TokenLocation}; + use xcm_executor::FeesMode; + use xcm_executor::traits::FeeReason; impl pallet_xcm_benchmarks::Config for Runtime { type XcmConfig = xcm_config::XcmConfig; @@ -1872,6 +1874,10 @@ sp_api::impl_runtime_apis! { fun: Fungible(1_000_000 * UNITS), }].into() } + fn ensure_for_send(_origin_ref: &MultiLocation, _dest: &MultiLocation, _fee_reason: FeeReason) -> Option { + // doing nothing + None + } } parameter_types! { diff --git a/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs b/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs index 9e51fa83de90..3fc6fcf27812 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,19 @@ benchmarks_instance_pallet! { let dest_location = T::valid_destination()?; let dest_account = T::AccountIdConverter::convert_location(&dest_location).unwrap(); + let expected_fees_mode = T::ensure_for_send(&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); + } let instruction = Instruction::TransferReserveAsset { assets, dest: dest_location, @@ -102,7 +109,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..0fa7e56f7614 100644 --- a/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs +++ b/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs @@ -27,6 +27,7 @@ use sp_core::H256; use sp_runtime::traits::{BlakeTwo256, IdentityLookup}; use xcm::latest::prelude::*; use xcm_builder::{AllowUnpaidExecutionFrom, MintLocation}; +use xcm_executor::{traits::FeeReason, FeesMode}; type Block = frame_system::mocking::MockBlock; @@ -169,6 +170,15 @@ impl crate::Config for Test { ::MaxAssetsIntoHolding::get(), ) } + + fn ensure_for_send( + _origin_ref: &MultiLocation, + _dest: &MultiLocation, + _fee_reason: FeeReason, + ) -> Option { + // doing nothing + None + } } pub type TrustedTeleporters = xcm_builder::Case; diff --git a/xcm/pallet-xcm-benchmarks/src/generic/mock.rs b/xcm/pallet-xcm-benchmarks/src/generic/mock.rs index 6f9d42df553d..fd3dff051616 100644 --- a/xcm/pallet-xcm-benchmarks/src/generic/mock.rs +++ b/xcm/pallet-xcm-benchmarks/src/generic/mock.rs @@ -152,6 +152,15 @@ impl crate::Config for Test { ::MaxAssetsIntoHolding::get(), ) } + + fn ensure_for_send( + _origin_ref: &MultiLocation, + _dest: &MultiLocation, + _fee_reason: FeeReason, + ) -> Option { + // doing nothing + None + } } impl generic::Config for Test { diff --git a/xcm/pallet-xcm-benchmarks/src/lib.rs b/xcm/pallet-xcm-benchmarks/src/lib.rs index c6a963435953..49ab2d710fb8 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; @@ -47,6 +50,14 @@ pub trait Config: frame_system::Config { /// Worst case scenario for a holding account in this runtime. fn worst_case_holding(depositable_count: u32) -> MultiAssets; + + /// Prepare all requirements for successful `XcmSender: SendXcm` passing (accounts, balances ...). + /// Returns possible `FeesMode` which is expected to be set to executor. + fn ensure_for_send( + origin_ref: &MultiLocation, + dest: &MultiLocation, + fee_reason: FeeReason, + ) -> Option; } const SEED: u32 = 0; diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index 7f1b1e9af690..4381f5ac67fd 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -941,6 +941,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() { 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, From cf462a7e52b071136f54227c5b9eddaa2c98f652 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Sun, 13 Aug 2023 14:19:48 +0200 Subject: [PATCH 10/13] Extract logic to common place to be reused by different runtimes (relaychains or parachains) --- Cargo.lock | 2 + runtime/common/Cargo.toml | 7 +- runtime/common/src/xcm_sender.rs | 90 +++++++++++++++++++ runtime/kusama/src/lib.rs | 54 ++++------- runtime/kusama/src/xcm_config.rs | 9 +- runtime/polkadot/src/lib.rs | 54 ++++------- runtime/polkadot/src/xcm_config.rs | 9 +- runtime/rococo/src/lib.rs | 54 ++++------- runtime/rococo/src/xcm_config.rs | 9 +- runtime/westend/src/lib.rs | 7 +- .../src/fungible/benchmarking.rs | 3 +- .../src/fungible/mock.rs | 11 +-- xcm/pallet-xcm-benchmarks/src/generic/mock.rs | 10 +-- xcm/pallet-xcm-benchmarks/src/lib.rs | 34 +++++-- 14 files changed, 186 insertions(+), 167 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 31b05ec5d3df..528e5abdfbee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7804,6 +7804,7 @@ dependencies = [ "pallet-transaction-payment", "pallet-treasury", "pallet-vesting", + "pallet-xcm-benchmarks", "parity-scale-codec", "polkadot-primitives", "polkadot-primitives-test-helpers", @@ -7827,6 +7828,7 @@ dependencies = [ "sp-std", "static_assertions", "xcm", + "xcm-executor", ] [[package]] diff --git a/runtime/common/Cargo.toml b/runtime/common/Cargo.toml index c9812d806733..93e2e354f064 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" @@ -108,7 +111,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 3573ec3dc42b..2342ab86170e 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 { + 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 + fees_mode = Some(FeesMode { jit_withdraw: true }); + } + fees_mode + } +} + +/// 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 13d62f27b88b..4d9f29878180 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -2108,8 +2108,6 @@ sp_api::impl_runtime_apis! { use xcm_config::{ LocalCheckAccount, SovereignAccountOf, AssetHub, TokenLocation, XcmConfig, }; - use xcm_executor::FeesMode; - use xcm_executor::traits::FeeReason; impl pallet_session_benchmarking::Config for Runtime {} impl pallet_offences_benchmarking::Config for Runtime {} @@ -2119,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()) } @@ -2132,43 +2145,6 @@ sp_api::impl_runtime_apis! { fun: Fungible(1_000_000 * UNITS), }].into() } - fn ensure_for_send(origin_ref: &MultiLocation, _dest: &MultiLocation, fee_reason: FeeReason) -> Option { - use xcm_executor::traits::{FeeManager, TransactAsset}; - - let mut fees_mode = None; - if !::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 - let ed = MultiAsset { - id: Concrete(TokenLocation::get()), - fun: Fungible(EXISTENTIAL_DEPOSIT.into()) - }; - ::AssetTransactor::deposit_asset(&ed, &origin_ref, None).unwrap(); - - // overestimate delivery fee - use runtime_common::xcm_sender::PriceForParachainDelivery; - let overestimated_xcm = vec![ClearOrigin; 128].into(); - let overestimated_fees = runtime_common::xcm_sender::ExponentialPrice::< - xcm_config::FeeAssetId, - xcm_config::BaseDeliveryFee, - TransactionByteFee, - Dmp - >::price_for_parachain_delivery( - kusama_runtime_constants::system_parachain::ASSET_HUB_ID.into(), - &overestimated_xcm - ); - - // mint overestimated fee to origin - for fee in overestimated_fees.inner() { - ::AssetTransactor::deposit_asset(&fee, &origin_ref, None).unwrap(); - } - - // expected worst case - fees_mode = Some(FeesMode { jit_withdraw: true }); - } - fees_mode - } } parameter_types! { diff --git a/runtime/kusama/src/xcm_config.rs b/runtime/kusama/src/xcm_config.rs index 51045be8db5a..32a18f498127 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! { diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 345ff98f7135..3acce374d6fe 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -2063,8 +2063,6 @@ sp_api::impl_runtime_apis! { use frame_benchmarking::baseline::Pallet as Baseline; use xcm::latest::prelude::*; use xcm_config::{XcmConfig, AssetHubLocation, TokenLocation, LocalCheckAccount, SovereignAccountOf}; - use xcm_executor::FeesMode; - use xcm_executor::traits::FeeReason; impl pallet_session_benchmarking::Config for Runtime {} impl pallet_offences_benchmarking::Config for Runtime {} @@ -2078,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()) } @@ -2088,43 +2101,6 @@ sp_api::impl_runtime_apis! { // Polkadot only knows about DOT vec![MultiAsset { id: Concrete(TokenLocation::get()), fun: Fungible(1_000_000 * UNITS) }].into() } - fn ensure_for_send(origin_ref: &MultiLocation, _dest: &MultiLocation, fee_reason: FeeReason) -> Option { - use xcm_executor::traits::{FeeManager, TransactAsset}; - - let mut fees_mode = None; - if !::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 - let ed = MultiAsset { - id: Concrete(TokenLocation::get()), - fun: Fungible(EXISTENTIAL_DEPOSIT.into()) - }; - ::AssetTransactor::deposit_asset(&ed, &origin_ref, None).unwrap(); - - // overestimate delivery fee - use runtime_common::xcm_sender::PriceForParachainDelivery; - let overestimated_xcm = vec![ClearOrigin; 128].into(); - let overestimated_fees = runtime_common::xcm_sender::ExponentialPrice::< - xcm_config::FeeAssetId, - xcm_config::BaseDeliveryFee, - TransactionByteFee, - Dmp - >::price_for_parachain_delivery( - polkadot_runtime_constants::system_parachain::ASSET_HUB_ID.into(), - &overestimated_xcm - ); - - // mint overestimated fee to origin - for fee in overestimated_fees.inner() { - ::AssetTransactor::deposit_asset(&fee, &origin_ref, None).unwrap(); - } - - // expected worst case - fees_mode = Some(FeesMode { jit_withdraw: true }); - } - fees_mode - } } parameter_types! { diff --git a/runtime/polkadot/src/xcm_config.rs b/runtime/polkadot/src/xcm_config.rs index 6a921cf6896c..4efa18fb8a9e 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! { diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 7a9881ae3a96..3d3c4c44b0d1 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -2094,14 +2094,27 @@ sp_api::impl_runtime_apis! { use xcm_config::{ LocalCheckAccount, LocationConverter, AssetHub, TokenLocation, XcmConfig, }; - use xcm_executor::FeesMode; - use xcm_executor::traits::FeeReason; + + 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()) } @@ -2112,43 +2125,6 @@ sp_api::impl_runtime_apis! { fun: Fungible(1_000_000 * UNITS), }].into() } - fn ensure_for_send(origin_ref: &MultiLocation, _dest: &MultiLocation, fee_reason: FeeReason) -> Option { - use xcm_executor::traits::{FeeManager, TransactAsset}; - - let mut fees_mode = None; - if !::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 - let ed = MultiAsset { - id: Concrete(TokenLocation::get()), - fun: Fungible(EXISTENTIAL_DEPOSIT.into()) - }; - ::AssetTransactor::deposit_asset(&ed, &origin_ref, None).unwrap(); - - // overestimate delivery fee - use runtime_common::xcm_sender::PriceForParachainDelivery; - let overestimated_xcm = vec![ClearOrigin; 128].into(); - let overestimated_fees = runtime_common::xcm_sender::ExponentialPrice::< - xcm_config::FeeAssetId, - xcm_config::BaseDeliveryFee, - TransactionByteFee, - Dmp - >::price_for_parachain_delivery( - rococo_runtime_constants::system_parachain::ASSET_HUB_ID.into(), - &overestimated_xcm - ); - - // mint overestimated fee to origin - for fee in overestimated_fees.inner() { - ::AssetTransactor::deposit_asset(&fee, &origin_ref, None).unwrap(); - } - - // expected worst case - fees_mode = Some(FeesMode { jit_withdraw: true }); - } - fees_mode - } } parameter_types! { diff --git a/runtime/rococo/src/xcm_config.rs b/runtime/rococo/src/xcm_config.rs index 20348452d6c1..d3cb66b4cfcd 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! { diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 69633bb66342..bbb4c83f1163 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1857,12 +1857,11 @@ sp_api::impl_runtime_apis! { MultiAsset, MultiAssets, MultiLocation, NetworkId, Response, }; use xcm_config::{AssetHub, TokenLocation}; - use xcm_executor::FeesMode; - use xcm_executor::traits::FeeReason; 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()) } @@ -1873,10 +1872,6 @@ sp_api::impl_runtime_apis! { fun: Fungible(1_000_000 * UNITS), }].into() } - fn ensure_for_send(_origin_ref: &MultiLocation, _dest: &MultiLocation, _fee_reason: FeeReason) -> Option { - // doing nothing - None - } } parameter_types! { diff --git a/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs b/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs index 3fc6fcf27812..e5e77ea88216 100644 --- a/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs +++ b/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs @@ -87,7 +87,8 @@ benchmarks_instance_pallet! { let dest_location = T::valid_destination()?; let dest_account = T::AccountIdConverter::convert_location(&dest_location).unwrap(); - let expected_fees_mode = T::ensure_for_send(&sender_location, &dest_location, FeeReason::TransferReserveAsset); + use crate::EnsureDelivery; + let expected_fees_mode = 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(); diff --git a/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs b/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs index 0fa7e56f7614..dd23f9222772 100644 --- a/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs +++ b/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs @@ -27,7 +27,6 @@ use sp_core::H256; use sp_runtime::traits::{BlakeTwo256, IdentityLookup}; use xcm::latest::prelude::*; use xcm_builder::{AllowUnpaidExecutionFrom, MintLocation}; -use xcm_executor::{traits::FeeReason, FeesMode}; type Block = frame_system::mocking::MockBlock; @@ -158,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(); @@ -170,15 +170,6 @@ impl crate::Config for Test { ::MaxAssetsIntoHolding::get(), ) } - - fn ensure_for_send( - _origin_ref: &MultiLocation, - _dest: &MultiLocation, - _fee_reason: FeeReason, - ) -> Option { - // doing nothing - None - } } pub type TrustedTeleporters = xcm_builder::Case; diff --git a/xcm/pallet-xcm-benchmarks/src/generic/mock.rs b/xcm/pallet-xcm-benchmarks/src/generic/mock.rs index fd3dff051616..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(); @@ -152,15 +153,6 @@ impl crate::Config for Test { ::MaxAssetsIntoHolding::get(), ) } - - fn ensure_for_send( - _origin_ref: &MultiLocation, - _dest: &MultiLocation, - _fee_reason: FeeReason, - ) -> Option { - // doing nothing - None - } } impl generic::Config for Test { diff --git a/xcm/pallet-xcm-benchmarks/src/lib.rs b/xcm/pallet-xcm-benchmarks/src/lib.rs index 49ab2d710fb8..e9455145165e 100644 --- a/xcm/pallet-xcm-benchmarks/src/lib.rs +++ b/xcm/pallet-xcm-benchmarks/src/lib.rs @@ -44,20 +44,15 @@ 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; /// Worst case scenario for a holding account in this runtime. fn worst_case_holding(depositable_count: u32) -> MultiAssets; - - /// Prepare all requirements for successful `XcmSender: SendXcm` passing (accounts, balances ...). - /// Returns possible `FeesMode` which is expected to be set to executor. - fn ensure_for_send( - origin_ref: &MultiLocation, - dest: &MultiLocation, - fee_reason: FeeReason, - ) -> Option; } const SEED: u32 = 0; @@ -119,3 +114,26 @@ 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. + fn ensure_successful_delivery( + origin_ref: &MultiLocation, + dest: &MultiLocation, + fee_reason: FeeReason, + ) -> 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 { + // doing nothing + None + } +} From 3866842dfe50194a535fb3f7cdd7b48193f81d8d Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Mon, 14 Aug 2023 10:27:51 +0200 Subject: [PATCH 11/13] Fix rococo xcm benchmarks --- runtime/rococo/src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 3d3c4c44b0d1..60b2106dfd7f 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -2132,10 +2132,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 { From 5661f28b488ff7986c44c9302161915f2b380856 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Wed, 16 Aug 2023 20:28:35 +0000 Subject: [PATCH 12/13] ".git/.scripts/commands/fmt/fmt.sh" --- runtime/common/src/xcm_sender.rs | 4 ++-- xcm/pallet-xcm-benchmarks/src/lib.rs | 7 ++++--- xcm/pallet-xcm/src/tests.rs | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/runtime/common/src/xcm_sender.rs b/runtime/common/src/xcm_sender.rs index 449c3927166f..4e560516f8ff 100644 --- a/runtime/common/src/xcm_sender.rs +++ b/runtime/common/src/xcm_sender.rs @@ -109,8 +109,8 @@ impl(index: u32) -> (T::AccountId, MultiLocati (account, location) } -/// Trait for a type which ensures all requirements for successful delivery with XCM transport layers. +/// 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. + /// Prepare all requirements for successful `XcmSender: SendXcm` passing (accounts, balances, + /// channels ...). Returns possible `FeesMode` which is expected to be set to executor. fn ensure_successful_delivery( origin_ref: &MultiLocation, dest: &MultiLocation, diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index c80260b49391..0f067110bed1 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -526,8 +526,8 @@ fn reserve_transfer_assets_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`). +/// 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); From a11a0c685ba1d81f7d4deee391cd5154856ca770 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Wed, 16 Aug 2023 23:09:41 +0200 Subject: [PATCH 13/13] Nits --- runtime/common/src/xcm_sender.rs | 6 +++--- xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs | 10 +++++++++- xcm/pallet-xcm-benchmarks/src/lib.rs | 10 ++++++---- xcm/pallet-xcm/src/tests.rs | 2 +- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/runtime/common/src/xcm_sender.rs b/runtime/common/src/xcm_sender.rs index 4e560516f8ff..1dfa83f57deb 100644 --- a/runtime/common/src/xcm_sender.rs +++ b/runtime/common/src/xcm_sender.rs @@ -150,7 +150,7 @@ impl< origin_ref: &MultiLocation, _dest: &MultiLocation, fee_reason: xcm_executor::traits::FeeReason, - ) -> Option { + ) -> (Option, Option) { use xcm_executor::{ traits::{FeeManager, TransactAsset}, FeesMode, @@ -180,10 +180,10 @@ impl< // allow more initialization for target parachain ToParachainHelper::ensure(Parachain::get()); - // expected worst case + // expected worst case - direct withdraw fees_mode = Some(FeesMode { jit_withdraw: true }); } - fees_mode + (fees_mode, None) } } diff --git a/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs b/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs index e5e77ea88216..89c6e5e4325c 100644 --- a/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs +++ b/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs @@ -88,7 +88,11 @@ benchmarks_instance_pallet! { let dest_account = T::AccountIdConverter::convert_location(&dest_location).unwrap(); use crate::EnsureDelivery; - let expected_fees_mode = T::DeliveryHelper::ensure_successful_delivery(&sender_location, &dest_location, FeeReason::TransferReserveAsset); + 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(); @@ -101,6 +105,10 @@ benchmarks_instance_pallet! { 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, diff --git a/xcm/pallet-xcm-benchmarks/src/lib.rs b/xcm/pallet-xcm-benchmarks/src/lib.rs index 60786a3ede13..3bf4aea1b25e 100644 --- a/xcm/pallet-xcm-benchmarks/src/lib.rs +++ b/xcm/pallet-xcm-benchmarks/src/lib.rs @@ -119,12 +119,14 @@ pub fn account_and_location(index: u32) -> (T::AccountId, MultiLocati /// 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. + /// 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, Option); } /// `()` implementation does nothing which means no special requirements for environment. @@ -133,8 +135,8 @@ impl EnsureDelivery for () { _origin_ref: &MultiLocation, _dest: &MultiLocation, _fee_reason: FeeReason, - ) -> Option { + ) -> (Option, Option) { // doing nothing - None + (None, None) } } diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index 0f067110bed1..88454c1758a6 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -546,7 +546,7 @@ fn reserve_transfer_assets_with_paid_router_works() { assert_ok!(XcmPallet::reserve_transfer_assets( RuntimeOrigin::signed(user_account.clone()), Box::new(Parachain(paid_para_id).into()), - Box::new(dest.clone().into()), + Box::new(dest.into()), Box::new((Here, SEND_AMOUNT).into()), 0, ));