Fixed XCM benchmarks to generate holding register data

import

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::<T>(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::<Vec<_>>().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::<T>(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::<Vec<_>>().into()),
+			reserve,
+			xcm: Xcm(vec![])
+		};
 		let xcm = Xcm(vec![instruction]);
 	}: {
 		executor.bench_process(xcm)?;

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,
@@ -162,9 +162,8 @@ impl crate::Config for Test {
 		Ok(valid_destination)
 	}
 	fn worst_case_holding(depositable_count: u32) -> Assets {
-		crate::mock_worst_case_holding(
-			depositable_count,
-			<XcmConfig as xcm_executor::Config>::MaxAssetsIntoHolding::get(),
+		generate_holding_assets(
+			<XcmConfig as xcm_executor::Config>::MaxAssetsIntoHolding::get() - depositable_count,
 		)
 	}
 }

polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs

@@ -19,9 +19,9 @@ 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},
+	latest::{prelude::*, MaxDispatchErrorLen, MaybeErrorCode, Weight, MAX_ITEMS_IN_ASSETS},
 	DoubleEncoded,
 };
 use xcm_executor::{
@@ -32,7 +32,6 @@ use xcm_executor::{
 benchmarks! {
 	report_holding {
 		let (sender_account, sender_location) = account_and_location::<T>(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,23 +41,31 @@ 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::<T>(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::<XcmCallOf<T>>::ReportHolding {
 			response_info: QueryResponseInfo {
 				destination,
 				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::<Vec<_>>().into()),
 		};
 
 		let xcm = Xcm(vec![instruction]);
@@ -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::<T>(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]);

polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs

@@ -162,9 +162,8 @@ impl crate::Config for Test {
 		Ok(valid_destination)
 	}
 	fn worst_case_holding(depositable_count: u32) -> Assets {
-		crate::mock_worst_case_holding(
-			depositable_count,
-			<XcmConfig as xcm_executor::Config>::MaxAssetsIntoHolding::get(),
+		generate_holding_assets(
+			<XcmConfig as xcm_executor::Config>::MaxAssetsIntoHolding::get() - depositable_count,
 		)
 	}
 }

polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs

@@ -50,6 +50,8 @@ pub trait Config: frame_system::Config {
 	fn valid_destination() -> Result<Location, BenchmarkError>;
 
 	/// 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<T> = <<T as Config>::XcmConfig as XcmConfig>::AssetTr
 /// The call type of executor's config. Should eventually resolve to the same overarching call type.
 pub type XcmCallOf<T> = <<T as Config>::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)),