[#113] Benchmark drop_assets() of TrappistDropAssets

pallets/asset-registry/src/lib.rs

@@ -137,7 +137,7 @@ pub mod pallet {
 			AssetIdMultiLocation::<T>::get(asset_id)
 		}
 
-		fn get_asset_id(asset_type: MultiLocation) -> Option<AssetIdOf<T>> {
+		fn get_asset_id(asset_type: &MultiLocation) -> Option<AssetIdOf<T>> {
 			AssetMultiLocationId::<T>::get(asset_type)
 		}
 	}

pallets/benchmarks/Cargo.toml

@@ -0,0 +1,35 @@
+[package]
+name = "pallet-benchmarks"
+version = "0.1.0"
+edition = "2021"
+
+[package.metadata.docs.rs]
+targets = ["x86_64-unknown-linux-gnu"]
+
+[dependencies]
+codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive", ] }
+scale-info = { version = "2.3.1", default-features = false, features = ["derive"] }
+sp-runtime = { default-features = false, git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.37" }
+sp-std = { default-features = false, git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.37" }
+frame-benchmarking = { default-features = false, optional = true, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.37" }
+frame-support = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.37" }
+frame-system = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.37" }
+
+xcm = { default-features = false, git = "https://github.com/paritytech/polkadot", branch = "release-v0.9.37" }
+xcm-executor = { default-features = false, git = "https://github.com/paritytech/polkadot", branch = "release-v0.9.37" }
+
+[dev-dependencies]
+sp-core = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.37" }
+
+[features]
+default = ["std"]
+std = [
+	"codec/std",
+	"frame-benchmarking/std",
+	"frame-support/std",
+	"frame-system/std",
+	"sp-runtime/std",
+	"sp-std/std",
+	"xcm-executor/std"
+]
+runtime-benchmarks = ["frame-benchmarking/runtime-benchmarks"]

pallets/benchmarks/src/benchmarking.rs

@@ -0,0 +1,33 @@
+use frame_benchmarking::benchmarks;
+use sp_runtime::SaturatedConversion;
+use xcm::prelude::AssetId as XcmAssetId;
+
+use crate::*;
+
+benchmarks! {
+	drop_assets_fungible {
+		let origin = MultiLocation::default();
+		let asset_id = 1;
+		let location = Parachain(asset_id).into();
+		T::register_asset(asset_id.into(), location.clone());
+		let asset = MultiAsset { id: XcmAssetId::Concrete(location), fun: Fungibility::Fungible(100) };
+	} : {
+		T::DropAssets::drop_assets(&origin, asset.into());
+	}
+
+	drop_assets_native {
+		let origin = MultiLocation::default();
+		let location = MultiLocation { parents: 0, interior: Here };
+		let amount = T::ExistentialDeposit::get().saturated_into();
+		let asset = MultiAsset { id: XcmAssetId::Concrete(location), fun: Fungibility::Fungible(amount) };
+	} : {
+		T::DropAssets::drop_assets(&origin, asset.into());
+	}
+
+	drop_assets_default {
+		let origin = MultiLocation::default();
+		let asset = MultiAsset { id: XcmAssetId::Abstract(Default::default()), fun: Fungibility::Fungible(0) };
+	} : {
+		T::DropAssets::drop_assets(&origin, asset.into());
+	}
+}

pallets/benchmarks/src/lib.rs

@@ -0,0 +1,43 @@
+//! Pallet for benchmarking.
+
+#![cfg_attr(not(feature = "std"), no_std)]
+
+use codec::Codec;
+use frame_support::{pallet_prelude::*, traits::tokens::AssetId};
+use sp_runtime::traits::AtLeast32BitUnsigned;
+use xcm::prelude::*;
+use xcm_executor::traits::DropAssets;
+
+pub use pallet::*;
+pub use weights::*;
+
+#[cfg(feature = "runtime-benchmarks")]
+pub mod benchmarking;
+pub mod weights;
+
+#[frame_support::pallet]
+pub mod pallet {
+	use super::*;
+
+	#[pallet::config]
+	pub trait Config: frame_system::Config {
+		/// Identifier for the class of asset.
+		type AssetId: AssetId + From<u32>;
+
+		/// The balance of an account.
+		type Balance: Parameter + Member + AtLeast32BitUnsigned + Codec + TypeInfo;
+
+		/// The minimum amount required to keep an account open.
+		#[pallet::constant]
+		type ExistentialDeposit: Get<Self::Balance>;
+
+		/// Handler for when some non-empty `Assets` value should be dropped.
+		type DropAssets: DropAssets;
+
+		/// Handler to register an asset.
+		fn register_asset(asset_id: Self::AssetId, location: MultiLocation);
+	}
+
+	#[pallet::pallet]
+	pub struct Pallet<T>(_);
+}

pallets/benchmarks/src/weights.rs

@@ -0,0 +1,7 @@
+use frame_support::weights::Weight;
+
+pub trait WeightInfo {
+	fn drop_assets_fungible() -> Weight;
+	fn drop_assets_native() -> Weight;
+	fn drop_assets_default() -> Weight;
+}

primitives/xcm/src/lib.rs

@@ -1,10 +1,10 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
 use frame_support::{
-	sp_runtime::SaturatedConversion,
+	sp_runtime::{SaturatedConversion, Saturating},
 	traits::{fungibles::Inspect, Currency},
 };
-use sp_std::{borrow::Borrow, marker::PhantomData, vec::Vec};
+use sp_std::{borrow::Borrow, marker::PhantomData};
 use xcm::latest::{
 	AssetId::Concrete, Fungibility::Fungible, Junctions::Here, MultiAsset, MultiLocation,
 };
@@ -23,7 +23,7 @@ where
 	AssetIdInfoGetter: AssetMultiLocationGetter<AssetId>,
 {
 	fn convert_ref(asset_multi_location: impl Borrow<MultiLocation>) -> Result<AssetId, ()> {
-		AssetIdInfoGetter::get_asset_id(asset_multi_location.borrow().clone()).ok_or(())
+		AssetIdInfoGetter::get_asset_id(asset_multi_location.borrow()).ok_or(())
 	}
 
 	fn reverse_ref(asset_id: impl Borrow<AssetId>) -> Result<MultiLocation, ()> {
@@ -33,7 +33,7 @@ where
 
 pub trait AssetMultiLocationGetter<AssetId> {
 	fn get_asset_multi_location(asset_id: AssetId) -> Option<MultiLocation>;
-	fn get_asset_id(asset_multi_location: MultiLocation) -> Option<AssetId>;
+	fn get_asset_id(asset_multi_location: &MultiLocation) -> Option<AssetId>;
 }
 
 pub struct ConvertedRegisteredAssetId<AssetId, Balance, ConvertAssetId, ConvertBalance>(
@@ -59,68 +59,87 @@ impl<
 	}
 }
 
+pub trait DropAssetsWeigher {
+	fn fungible() -> u64;
+	fn native() -> u64;
+	fn default() -> u64;
+}
+
 pub struct TrappistDropAssets<
 	AssetId,
 	AssetIdInfoGetter,
 	AssetsPallet,
 	BalancesPallet,
 	XcmPallet,
-	AccoundId,
->(PhantomData<(AssetId, AssetIdInfoGetter, AssetsPallet, BalancesPallet, XcmPallet, AccoundId)>);
-impl<AssetId, AssetIdInfoGetter, AssetsPallet, BalancesPallet, XcmPallet, AccountId> DropAssets
+	AccountId,
+	Weigher,
+>(
+	PhantomData<(
+		AssetId,
+		AssetIdInfoGetter,
+		AssetsPallet,
+		BalancesPallet,
+		XcmPallet,
+		AccountId,
+		Weigher,
+	)>,
+);
+
+impl<AssetId, AssetIdInfoGetter, AssetsPallet, BalancesPallet, XcmPallet, AccountId, Weigher>
+	DropAssets
 	for TrappistDropAssets<
 		AssetId,
 		AssetIdInfoGetter,
 		AssetsPallet,
 		BalancesPallet,
 		XcmPallet,
 		AccountId,
+		Weigher,
 	> where
-	AssetId: Clone,
 	AssetIdInfoGetter: AssetMultiLocationGetter<AssetId>,
 	AssetsPallet: Inspect<AccountId, AssetId = AssetId>,
 	BalancesPallet: Currency<AccountId>,
 	XcmPallet: DropAssets,
+	Weigher: DropAssetsWeigher,
 {
 	// assets are whatever the Holding Register had when XCVM halts
-	fn drop_assets(origin: &MultiLocation, assets: Assets) -> u64 {
-		let multi_assets: Vec<MultiAsset> = assets.into();
-		let mut trap: Vec<MultiAsset> = Vec::new();
+	fn drop_assets(origin: &MultiLocation, mut assets: Assets) -> u64 {
+		const NATIVE_LOCATION: MultiLocation = MultiLocation { parents: 0, interior: Here };
 
-		for asset in multi_assets {
-			if let MultiAsset { id: Concrete(location), fun: Fungible(amount) } = asset.clone() {
-				// is location a fungible on AssetRegistry?
-				if let Some(asset_id) = AssetIdInfoGetter::get_asset_id(location.clone()) {
-					let min_balance = AssetsPallet::minimum_balance(asset_id);
+		let mut weight = {
+			assets.non_fungible.clear();
+			Weigher::default()
+		};
 
-					// only trap if amount ≥ min_balance
-					// do nothing otherwise (asset is lost)
-					if min_balance <= amount.saturated_into::<AssetsPallet::Balance>() {
-						trap.push(asset);
-					}
+		assets.fungible.retain(|id, &mut amount| {
+			if let Concrete(location) = id {
+				match AssetIdInfoGetter::get_asset_id(location) {
+					Some(asset_id) => {
+						weight.saturating_accrue(Weigher::fungible());
 
-				// is location the native token?
-				} else if location == (MultiLocation { parents: 0, interior: Here }) {
-					let min_balance = BalancesPallet::minimum_balance();
+						// only trap if amount ≥ min_balance
+						// do nothing otherwise (asset is lost)
+						amount.saturated_into::<AssetsPallet::Balance>() >=
+							AssetsPallet::minimum_balance(asset_id)
+					},
+					None => {
+						weight.saturating_accrue(Weigher::native());
 
-					// only trap if amount ≥ min_balance
-					// do nothing otherwise (asset is lost)
-					if min_balance <= amount.saturated_into::<BalancesPallet::Balance>() {
-						trap.push(asset);
-					}
+						// only trap if native token and amount ≥ min_balance
+						// do nothing otherwise (asset is lost)
+						*location == NATIVE_LOCATION &&
+							amount.saturated_into::<BalancesPallet::Balance>() >=
+								BalancesPallet::minimum_balance()
+					},
 				}
+			} else {
+				weight.saturating_accrue(Weigher::default());
+				false
 			}
-		}
-
-		// TODO: put real weight of execution up until this point here
-		let mut weight = 0;
-
-		if !trap.is_empty() {
-			// we have filtered out non-compliant assets
-			// insert valid assets into the asset trap implemented by XcmPallet
-			weight += XcmPallet::drop_assets(origin, trap.into());
-		}
+		});
 
-		weight
+		// we have filtered out non-compliant assets
+		// insert valid assets into the asset trap implemented by XcmPallet
+		weight.saturating_add(XcmPallet::drop_assets(origin, assets))
 	}
 }

runtime/trappist/Cargo.toml

@@ -104,7 +104,8 @@ pallet-dex-rpc-runtime-api = { version = "0.0.1", git = "https://github.com/pari
 pallet-chess = { git = "https://github.com/SubstrateChess/pallet-chess.git", default-features = false, branch = "polkadot-v0.9.37" }
 
 # Trappist Pallets
-pallet-asset-registry = { version = "0.0.1", default-features = false, path = "../../pallets/asset-registry" }
+pallet-asset-registry = { default-features = false, path = "../../pallets/asset-registry" }
+pallet-benchmarks = { default-features = false, path = "../../pallets/benchmarks" }
 
 [features]
 default = ["std"]
@@ -183,6 +184,7 @@ runtime-benchmarks = [
 	"pallet-assets/runtime-benchmarks",
 	"pallet-asset-registry/runtime-benchmarks",
 	"pallet-balances/runtime-benchmarks",
+	"pallet-benchmarks/runtime-benchmarks",
 	"pallet-collator-selection/runtime-benchmarks",
 	"pallet-collective/runtime-benchmarks",
 	"pallet-contracts/runtime-benchmarks",