From 04fdac7d8692530bad89ed8413498321cf1eabf6 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Thu, 18 Apr 2024 22:22:11 +0200 Subject: [PATCH 1/8] Move `validate_xcm_nesting` directly to the `VersionedXcm` for better reusability --- cumulus/pallets/xcmp-queue/src/lib.rs | 17 +---- polkadot/xcm/src/lib.rs | 93 ++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 16 deletions(-) diff --git a/cumulus/pallets/xcmp-queue/src/lib.rs b/cumulus/pallets/xcmp-queue/src/lib.rs index deced13a9e81..7de2fd809421 100644 --- a/cumulus/pallets/xcmp-queue/src/lib.rs +++ b/cumulus/pallets/xcmp-queue/src/lib.rs @@ -916,7 +916,8 @@ impl SendXcm for Pallet { let price = T::PriceForSiblingDelivery::price_for_delivery(id, &xcm); let versioned_xcm = T::VersionWrapper::wrap_version(&d, xcm) .map_err(|()| SendError::DestinationUnsupported)?; - validate_xcm_nesting(&versioned_xcm) + versioned_xcm + .validate_xcm_nesting() .map_err(|()| SendError::ExceedsMaxMessageSize)?; Ok(((id, versioned_xcm), price)) @@ -932,10 +933,6 @@ impl SendXcm for Pallet { fn deliver((id, xcm): (ParaId, VersionedXcm<()>)) -> Result { let hash = xcm.using_encoded(sp_io::hashing::blake2_256); - defensive_assert!( - validate_xcm_nesting(&xcm).is_ok(), - "Tickets are valid prior to delivery by trait XCM; qed" - ); match Self::send_fragment(id, XcmpMessageFormat::ConcatenatedVersionedXcm, xcm) { Ok(_) => { @@ -950,16 +947,6 @@ impl SendXcm for Pallet { } } -/// Checks that the XCM is decodable with `MAX_XCM_DECODE_DEPTH`. -/// -/// Note that this uses the limit of the sender - not the receiver. It it best effort. -pub(crate) fn validate_xcm_nesting(xcm: &VersionedXcm<()>) -> Result<(), ()> { - xcm.using_encoded(|mut enc| { - VersionedXcm::<()>::decode_all_with_depth_limit(MAX_XCM_DECODE_DEPTH, &mut enc).map(|_| ()) - }) - .map_err(|_| ()) -} - impl FeeTracker for Pallet { type Id = ParaId; diff --git a/polkadot/xcm/src/lib.rs b/polkadot/xcm/src/lib.rs index e836486e86e3..198020ea1261 100644 --- a/polkadot/xcm/src/lib.rs +++ b/polkadot/xcm/src/lib.rs @@ -25,7 +25,7 @@ extern crate alloc; use derivative::Derivative; -use parity_scale_codec::{Decode, Encode, Error as CodecError, Input, MaxEncodedLen}; +use parity_scale_codec::{Decode, DecodeLimit, Encode, Error as CodecError, Input, MaxEncodedLen}; use scale_info::TypeInfo; pub mod v2; @@ -456,6 +456,23 @@ impl IdentifyVersion for VersionedXcm { } } +impl VersionedXcm { + /// Checks that the XCM is decodable with `MAX_XCM_DECODE_DEPTH`. Consequently, it also checks + /// all decode implementations and limits, such as MAX_ITEMS_IN_ASSETS or + /// MAX_INSTRUCTIONS_TO_DECODE. + /// + /// Note that this uses the limit of the sender - not the receiver. It is a best effort. + pub fn validate_xcm_nesting(&self) -> Result<(), ()> { + self.using_encoded(|mut enc| { + Self::decode_all_with_depth_limit(MAX_XCM_DECODE_DEPTH, &mut enc).map(|_| ()) + }) + .map_err(|e| { + log::error!(target: "xcm::validate_xcm_nesting", "Decode error: {e:?} for xcm: {self:?}!"); + () + }) + } +} + impl From> for VersionedXcm { fn from(x: v2::Xcm) -> Self { VersionedXcm::V2(x) @@ -704,3 +721,77 @@ fn size_limits() { } assert!(!test_failed); } + +#[test] +fn validate_xcm_nesting_works() { + use crate::latest::{ + prelude::{GeneralIndex, ReserveAssetDeposited, SetAppendix}, + Assets, Xcm, MAX_INSTRUCTIONS_TO_DECODE, MAX_ITEMS_IN_ASSETS, + }; + + // closure generates assets of `count` + let assets = |count| { + let mut assets = Assets::new(); + for i in 0..count { + assets.push((GeneralIndex(i as u128), 100).into()); + } + assets + }; + + // closer generates `Xcm` with nested instructions of `depth` + let with_instr = |depth| { + let mut xcm = Xcm::<()>(vec![]); + for _ in 0..depth - 1 { + xcm = Xcm::<()>(vec![SetAppendix(xcm)]); + } + xcm + }; + + // `MAX_INSTRUCTIONS_TO_DECODE` check + assert!(VersionedXcm::<()>::from(Xcm(vec![ + ReserveAssetDeposited(assets(1)); + (MAX_INSTRUCTIONS_TO_DECODE - 1) as usize + ])) + .validate_xcm_nesting() + .is_ok()); + assert!(VersionedXcm::<()>::from(Xcm(vec![ + ReserveAssetDeposited(assets(1)); + MAX_INSTRUCTIONS_TO_DECODE as usize + ])) + .validate_xcm_nesting() + .is_ok()); + assert!(VersionedXcm::<()>::from(Xcm(vec![ + ReserveAssetDeposited(assets(1)); + (MAX_INSTRUCTIONS_TO_DECODE + 1) as usize + ])) + .validate_xcm_nesting() + .is_err()); + + // `MAX_XCM_DECODE_DEPTH` check + assert!(VersionedXcm::<()>::from(with_instr(MAX_XCM_DECODE_DEPTH - 1)) + .validate_xcm_nesting() + .is_ok()); + assert!(VersionedXcm::<()>::from(with_instr(MAX_XCM_DECODE_DEPTH)) + .validate_xcm_nesting() + .is_ok()); + assert!(VersionedXcm::<()>::from(with_instr(MAX_XCM_DECODE_DEPTH + 1)) + .validate_xcm_nesting() + .is_err()); + + // `MAX_ITEMS_IN_ASSETS` check + assert!(VersionedXcm::<()>::from(Xcm(vec![ReserveAssetDeposited(assets( + MAX_ITEMS_IN_ASSETS + ))])) + .validate_xcm_nesting() + .is_ok()); + assert!(VersionedXcm::<()>::from(Xcm(vec![ReserveAssetDeposited(assets( + MAX_ITEMS_IN_ASSETS - 1 + ))])) + .validate_xcm_nesting() + .is_ok()); + assert!(VersionedXcm::<()>::from(Xcm(vec![ReserveAssetDeposited(assets( + MAX_ITEMS_IN_ASSETS + 1 + ))])) + .validate_xcm_nesting() + .is_err()); +} From 78eff219da8d28d421cf6eaec8d21687b921ddb5 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Fri, 19 Apr 2024 16:00:52 +0200 Subject: [PATCH 2/8] Re-use `validate_xcm_nesting` for `ParentAsUmp` --- cumulus/primitives/utility/src/lib.rs | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/cumulus/primitives/utility/src/lib.rs b/cumulus/primitives/utility/src/lib.rs index d5d411356dc3..54f40bd01097 100644 --- a/cumulus/primitives/utility/src/lib.rs +++ b/cumulus/primitives/utility/src/lib.rs @@ -69,6 +69,9 @@ where let price = P::price_for_delivery((), &xcm); let versioned_xcm = W::wrap_version(&d, xcm).map_err(|()| SendError::DestinationUnsupported)?; + versioned_xcm + .validate_xcm_nesting() + .map_err(|()| SendError::ExceedsMaxMessageSize)?; let data = versioned_xcm.encode(); Ok((data, price)) @@ -526,6 +529,8 @@ impl< mod test_xcm_router { use super::*; use cumulus_primitives_core::UpwardMessage; + use frame_support::assert_ok; + use xcm::MAX_XCM_DECODE_DEPTH; /// Validates [`validate`] for required Some(destination) and Some(message) struct OkFixedXcmHashWithAssertingRequiredInputsSender; @@ -621,6 +626,29 @@ mod test_xcm_router { )>(dest.into(), message) ); } + + #[test] + fn parent_as_ump_validate_nested_xcm_works() { + let dest = Parent; + + type Router = ParentAsUmp<(), (), ()>; + + // Message that is not too deeply nested: + let mut good = Xcm(vec![ClearOrigin]); + for _ in 0..MAX_XCM_DECODE_DEPTH - 1 { + good = Xcm(vec![SetAppendix(good)]); + } + + // Check that the good message is validated: + assert_ok!(::validate(&mut Some(dest.into()), &mut Some(good.clone()))); + + // Nesting the message one more time should reject it: + let bad = Xcm(vec![SetAppendix(good)]); + assert_eq!( + Err(SendError::ExceedsMaxMessageSize), + ::validate(&mut Some(dest.into()), &mut Some(bad)) + ); + } } #[cfg(test)] mod test_trader { From d5a78ab8dce8df9a9f30e73ea2a07b78afe24121 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Sat, 20 Apr 2024 14:53:14 +0200 Subject: [PATCH 3/8] Re-use `validate_xcm_nesting` for `ChildParachainRouter` --- .../runtime/common/src/integration_tests.rs | 5 ++- polkadot/runtime/common/src/xcm_sender.rs | 44 ++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/polkadot/runtime/common/src/integration_tests.rs b/polkadot/runtime/common/src/integration_tests.rs index 91b64ef7259c..3e9ac1fc1b15 100644 --- a/polkadot/runtime/common/src/integration_tests.rs +++ b/polkadot/runtime/common/src/integration_tests.rs @@ -39,7 +39,7 @@ use primitives::{ MAX_CODE_SIZE, }; use runtime_parachains::{ - configuration, origin, paras, shared, Origin as ParaOrigin, ParaLifecycle, + configuration, dmp, origin, paras, shared, Origin as ParaOrigin, ParaLifecycle, }; use sp_core::H256; use sp_io::TestExternalities; @@ -84,6 +84,7 @@ frame_support::construct_runtime!( Paras: paras, ParasShared: shared, ParachainsOrigin: origin, + Dmp: dmp, // Para Onboarding Pallets Registrar: paras_registrar, @@ -201,6 +202,8 @@ impl shared::Config for Test { type DisabledValidators = (); } +impl dmp::Config for Test {} + impl origin::Config for Test {} parameter_types! { diff --git a/polkadot/runtime/common/src/xcm_sender.rs b/polkadot/runtime/common/src/xcm_sender.rs index 0cbc2e603c8e..a712d4381f75 100644 --- a/polkadot/runtime/common/src/xcm_sender.rs +++ b/polkadot/runtime/common/src/xcm_sender.rs @@ -119,7 +119,9 @@ where let config = configuration::ActiveConfig::::get(); let para = id.into(); let price = P::price_for_delivery(para, &xcm); - let blob = W::wrap_version(&d, xcm).map_err(|()| DestinationUnsupported)?.encode(); + let versioned_xcm = W::wrap_version(&d, xcm).map_err(|()| DestinationUnsupported)?; + versioned_xcm.validate_xcm_nesting().map_err(|()| ExceedsMaxMessageSize)?; + let blob = versioned_xcm.encode(); dmp::Pallet::::can_queue_downward_message(&config, ¶, &blob) .map_err(Into::::into)?; @@ -236,9 +238,11 @@ impl EnsureForParachain for () { #[cfg(test)] mod tests { use super::*; - use frame_support::parameter_types; + use crate::integration_tests::new_test_ext; + use frame_support::{assert_ok, parameter_types}; use runtime_parachains::FeeTracker; use sp_runtime::FixedU128; + use xcm::MAX_XCM_DECODE_DEPTH; parameter_types! { pub const BaseDeliveryFee: u128 = 300_000_000; @@ -297,4 +301,40 @@ mod tests { (FeeAssetId::get(), result).into() ); } + + #[test] + fn child_parachain_router_validate_nested_xcm_works() { + let dest = Parachain(5555); + + type Router = ChildParachainRouter< + crate::integration_tests::Test, + (), + NoPriceForMessageDelivery, + >; + + // Message that is not too deeply nested: + let mut good = Xcm(vec![ClearOrigin]); + for _ in 0..MAX_XCM_DECODE_DEPTH - 1 { + good = Xcm(vec![SetAppendix(good)]); + } + + new_test_ext().execute_with(|| { + configuration::ActiveConfig::::mutate(|c| { + c.max_downward_message_size = u32::MAX; + }); + + // Check that the good message is validated: + assert_ok!(::validate( + &mut Some(dest.into()), + &mut Some(good.clone()) + )); + + // Nesting the message one more time should reject it: + let bad = Xcm(vec![SetAppendix(good)]); + assert_eq!( + Err(ExceedsMaxMessageSize), + ::validate(&mut Some(dest.into()), &mut Some(bad)) + ); + }); + } } From b6ad32b87c41fcf6621794d154e199b6b43c08f0 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Mon, 22 Apr 2024 17:42:17 +0200 Subject: [PATCH 4/8] Fix benchmark --- .../xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs index 8c6ed4b5d0e0..d58e6b5d23da 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs @@ -21,7 +21,7 @@ use frame_benchmarking::{benchmarks, BenchmarkError}; use frame_support::{dispatch::GetDispatchInfo, traits::fungible::Inspect}; use sp_std::vec; use xcm::{ - latest::{prelude::*, MaxDispatchErrorLen, MaybeErrorCode, Weight}, + latest::{prelude::*, MaxDispatchErrorLen, MaybeErrorCode, Weight, MAX_ITEMS_IN_ASSETS}, DoubleEncoded, }; use xcm_executor::{ @@ -57,8 +57,8 @@ benchmarks! { query_id: Default::default(), max_weight: Weight::MAX, }, - // Worst case is looking through all holdings for every asset explicitly. - assets: Definite(holding), + // Worst case is looking through all holdings for every asset explicitly - respecting the limit `MAX_ITEMS_IN_ASSETS`. + assets: Definite(holding.into_inner().into_iter().take(MAX_ITEMS_IN_ASSETS).collect::>().into()), }; let xcm = Xcm(vec![instruction]); From 1d4a9fab08f9d4ddf0e754331509af78ba553ed2 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Mon, 22 Apr 2024 17:50:54 +0200 Subject: [PATCH 5/8] compile --- polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs index d58e6b5d23da..6fb313dc1bca 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs @@ -19,7 +19,7 @@ use crate::{account_and_location, new_executor, EnsureDelivery, XcmCallOf}; use codec::Encode; use frame_benchmarking::{benchmarks, BenchmarkError}; use frame_support::{dispatch::GetDispatchInfo, traits::fungible::Inspect}; -use sp_std::vec; +use sp_std::{prelude::*, vec}; use xcm::{ latest::{prelude::*, MaxDispatchErrorLen, MaybeErrorCode, Weight, MAX_ITEMS_IN_ASSETS}, DoubleEncoded, From ba6317aafae018c235c2c3ff0475e57a9535c8ef Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Thu, 18 Apr 2024 22:17:18 +0200 Subject: [PATCH 6/8] Fixed XCM benchmarks to generate holding register data import --- .../src/fungible/benchmarking.rs | 26 ++++++++++++----- .../src/fungible/mock.rs | 7 ++--- .../src/generic/benchmarking.rs | 28 +++++++++++++------ .../pallet-xcm-benchmarks/src/generic/mock.rs | 5 ++-- polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs | 13 ++++++--- 5 files changed, 53 insertions(+), 26 deletions(-) diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs index 4b77199069d3..d99da9184b5d 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs @@ -146,8 +146,6 @@ benchmarks_instance_pallet! { initiate_reserve_withdraw { let (sender_account, sender_location) = account_and_location::(1); - let holding = T::worst_case_holding(1); - let assets_filter = AssetFilter::Definite(holding.clone().into_inner().into_iter().take(MAX_ITEMS_IN_ASSETS).collect::>().into()); let reserve = T::valid_destination().map_err(|_| BenchmarkError::Skip)?; let (expected_fees_mode, expected_assets_in_holding) = T::DeliveryHelper::ensure_successful_delivery( @@ -157,15 +155,29 @@ benchmarks_instance_pallet! { ); let sender_account_balance_before = T::TransactAsset::balance(&sender_account); + // generate holding and add possible required fees + let holding = if let Some(expected_assets_in_holding) = expected_assets_in_holding { + let mut holding = T::worst_case_holding(1 + expected_assets_in_holding.len() as u32); + for a in expected_assets_in_holding.into_inner() { + holding.push(a); + } + holding + } else { + T::worst_case_holding(1) + }; + let mut executor = new_executor::(sender_location); - executor.set_holding(holding.into()); + executor.set_holding(holding.clone().into()); 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::InitiateReserveWithdraw { assets: assets_filter, reserve, xcm: Xcm(vec![]) }; + + let instruction = Instruction::InitiateReserveWithdraw { + // Worst case is looking through all holdings for every asset explicitly - respecting the limit `MAX_ITEMS_IN_ASSETS`. + assets: Definite(holding.into_inner().into_iter().take(MAX_ITEMS_IN_ASSETS).collect::>().into()), + reserve, + xcm: Xcm(vec![]) + }; let xcm = Xcm(vec![instruction]); }: { executor.bench_process(xcm)?; diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs index d11f64e74944..bf7d4e589de3 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs @@ -16,7 +16,7 @@ //! A mock runtime for XCM benchmarking. -use crate::{fungible as xcm_balances_benchmark, mock::*}; +use crate::{fungible as xcm_balances_benchmark, generate_holding_assets, mock::*}; use frame_benchmarking::BenchmarkError; use frame_support::{ derive_impl, parameter_types, @@ -130,9 +130,8 @@ impl crate::Config for Test { Ok(valid_destination) } fn worst_case_holding(depositable_count: u32) -> Assets { - crate::mock_worst_case_holding( - depositable_count, - ::MaxAssetsIntoHolding::get(), + generate_holding_assets( + ::MaxAssetsIntoHolding::get() - depositable_count, ) } } diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs index 6fb313dc1bca..760b21f93566 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs @@ -32,7 +32,6 @@ use xcm_executor::{ benchmarks! { report_holding { let (sender_account, sender_location) = account_and_location::(1); - let holding = T::worst_case_holding(0); let destination = T::valid_destination().map_err(|_| BenchmarkError::Skip)?; let (expected_fees_mode, expected_assets_in_holding) = T::DeliveryHelper::ensure_successful_delivery( @@ -42,14 +41,22 @@ benchmarks! { ); let sender_account_balance_before = T::TransactAsset::balance(&sender_account); + // generate holding and add possible required fees + let holding = if let Some(expected_assets_in_holding) = expected_assets_in_holding { + let mut holding = T::worst_case_holding(expected_assets_in_holding.len() as u32); + for a in expected_assets_in_holding.into_inner() { + holding.push(a); + } + holding + } else { + T::worst_case_holding(0) + }; + let mut executor = new_executor::(sender_location); executor.set_holding(holding.clone().into()); 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::>::ReportHolding { response_info: QueryResponseInfo { @@ -612,14 +619,19 @@ benchmarks! { let sender_account = T::AccountIdConverter::convert_location(&owner).unwrap(); let sender_account_balance_before = T::TransactAsset::balance(&sender_account); + // generate holding and add possible required fees + let mut holding: Assets = asset.clone().into(); + if let Some(expected_assets_in_holding) = expected_assets_in_holding { + for a in expected_assets_in_holding.into_inner() { + holding.push(a); + } + }; + let mut executor = new_executor::(owner); - executor.set_holding(asset.clone().into()); + executor.set_holding(holding.into()); 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::LockAsset { asset, unlocker }; let xcm = Xcm(vec![instruction]); diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs index f41df017b9db..da0f28ccf28d 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs @@ -132,9 +132,8 @@ impl crate::Config for Test { Ok(valid_destination) } fn worst_case_holding(depositable_count: u32) -> Assets { - crate::mock_worst_case_holding( - depositable_count, - ::MaxAssetsIntoHolding::get(), + generate_holding_assets( + ::MaxAssetsIntoHolding::get() - depositable_count, ) } } diff --git a/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs b/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs index 63ed0ac0ca73..a43f27bf47e7 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs +++ b/polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs @@ -50,6 +50,8 @@ pub trait Config: frame_system::Config { fn valid_destination() -> Result; /// Worst case scenario for a holding account in this runtime. + /// - `depositable_count` specifies the count of assets we plan to add to the holding on top of + /// those generated by the `worst_case_holding` implementation. fn worst_case_holding(depositable_count: u32) -> Assets; } @@ -64,19 +66,22 @@ pub type AssetTransactorOf = <::XcmConfig as XcmConfig>::AssetTr /// The call type of executor's config. Should eventually resolve to the same overarching call type. pub type XcmCallOf = <::XcmConfig as XcmConfig>::RuntimeCall; -pub fn mock_worst_case_holding(depositable_count: u32, max_assets: u32) -> Assets { +pub fn generate_holding_assets(max_assets: u32) -> Assets { let fungibles_amount: u128 = 100; - let holding_fungibles = max_assets / 2 - depositable_count; - let holding_non_fungibles = holding_fungibles; + let holding_fungibles = max_assets / 2; + let holding_non_fungibles = max_assets - holding_fungibles - 1; // -1 because of adding `Here` asset + // add count of `holding_fungibles` (0..holding_fungibles) .map(|i| { Asset { id: AssetId(GeneralIndex(i as u128).into()), - fun: Fungible(fungibles_amount * i as u128), + fun: Fungible(fungibles_amount * (i + 1) as u128), // non-zero amount } .into() }) + // add one more `Here` asset .chain(core::iter::once(Asset { id: AssetId(Here.into()), fun: Fungible(u128::MAX) })) + // add count of `holding_non_fungibles` .chain((0..holding_non_fungibles).map(|i| Asset { id: AssetId(GeneralIndex(i as u128).into()), fun: NonFungible(asset_instance_from(i)), From 0901f25e75d19de80e3e969d982eb4b8edd04a34 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Tue, 23 Apr 2024 09:06:37 +0200 Subject: [PATCH 7/8] nit for AHs benchmarks --- .../runtimes/assets/asset-hub-rococo/src/lib.rs | 14 +++++--------- .../runtimes/assets/asset-hub-westend/src/lib.rs | 14 +++++--------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index 5cb29343a1cf..5d72dda3e778 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -1519,9 +1519,9 @@ impl_runtime_apis! { fn worst_case_holding(depositable_count: u32) -> xcm::v4::Assets { // A mix of fungible, non-fungible, and concrete assets. let holding_non_fungibles = MaxAssetsIntoHolding::get() / 2 - depositable_count; - let holding_fungibles = holding_non_fungibles.saturating_sub(1); + let holding_fungibles = holding_non_fungibles.saturating_sub(2); // -2 for two `iter::once` bellow let fungibles_amount: u128 = 100; - let mut assets = (0..holding_fungibles) + (0..holding_fungibles) .map(|i| { Asset { id: GeneralIndex(i as u128).into(), @@ -1529,17 +1529,13 @@ impl_runtime_apis! { } }) .chain(core::iter::once(Asset { id: Here.into(), fun: Fungible(u128::MAX) })) + .chain(core::iter::once(Asset { id: AssetId(TokenLocation::get()), fun: Fungible(1_000_000 * UNITS) })) .chain((0..holding_non_fungibles).map(|i| Asset { id: GeneralIndex(i as u128).into(), fun: NonFungible(asset_instance_from(i)), })) - .collect::>(); - - assets.push(Asset { - id: AssetId(TokenLocation::get()), - fun: Fungible(1_000_000 * UNITS), - }); - assets.into() + .collect::>() + .into() } } 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 366fb91723ae..f07d8f117677 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -1610,9 +1610,9 @@ impl_runtime_apis! { fn worst_case_holding(depositable_count: u32) -> xcm::v4::Assets { // A mix of fungible, non-fungible, and concrete assets. let holding_non_fungibles = MaxAssetsIntoHolding::get() / 2 - depositable_count; - let holding_fungibles = holding_non_fungibles - 1; + let holding_fungibles = holding_non_fungibles - 2; // -2 for two `iter::once` bellow let fungibles_amount: u128 = 100; - let mut assets = (0..holding_fungibles) + (0..holding_fungibles) .map(|i| { Asset { id: AssetId(GeneralIndex(i as u128).into()), @@ -1620,17 +1620,13 @@ impl_runtime_apis! { } }) .chain(core::iter::once(Asset { id: AssetId(Here.into()), fun: Fungible(u128::MAX) })) + .chain(core::iter::once(Asset { id: AssetId(WestendLocation::get()), fun: Fungible(1_000_000 * UNITS) })) .chain((0..holding_non_fungibles).map(|i| Asset { id: AssetId(GeneralIndex(i as u128).into()), fun: NonFungible(asset_instance_from(i)), })) - .collect::>(); - - assets.push(Asset { - id: AssetId(WestendLocation::get()), - fun: Fungible(1_000_000 * UNITS), - }); - assets.into() + .collect::>() + .into() } } From c5177a4dc2acc8eeb774882f9d459a4424b338f6 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Tue, 23 Apr 2024 10:13:24 +0200 Subject: [PATCH 8/8] 0 amount is invalid --- cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs | 2 +- cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index 5d72dda3e778..201647ac2ebf 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -1525,7 +1525,7 @@ impl_runtime_apis! { .map(|i| { Asset { id: GeneralIndex(i as u128).into(), - fun: Fungible(fungibles_amount * i as u128), + fun: Fungible(fungibles_amount * (i + 1) as u128), // non-zero amount } }) .chain(core::iter::once(Asset { id: Here.into(), fun: Fungible(u128::MAX) })) 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 f07d8f117677..78c83cf6922a 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -1616,7 +1616,7 @@ impl_runtime_apis! { .map(|i| { Asset { id: AssetId(GeneralIndex(i as u128).into()), - fun: Fungible(fungibles_amount * i as u128), + fun: Fungible(fungibles_amount * (i + 1) as u128), // non-zero amount } }) .chain(core::iter::once(Asset { id: AssetId(Here.into()), fun: Fungible(u128::MAX) }))