[#113] Benchmark drop_assets() of TrappistDropAssets

README.md

@@ -62,16 +62,14 @@ cargo b -r --no-default-features --features with-stout-runtime --target-dir targ
 ```
 
 Alternatively, run
-`bash build_runtimes.sh` .
+`bash ./scripts/build_runtimes.sh` .
 
 
 ### XCM Playground via Zombienet
 
 Create a `bin` directory into the root of this repository and place the following binaries inside of it:
 - `polkadot` (which you can download from [the releases](https://github.com/paritytech/polkadot/releases))
 - `polkadot-parachain` (which you will build from [cumulus](https://github.com/paritytech/cumulus))
-- `trappist-collator` (which you will build from this repository)
-- `stout-collator` (which you will build from this repository)
 
 Download the [latest release of zombienet](https://github.com/paritytech/zombienet/releases/) into the root of this repository and make it executable:
 ```

node/Cargo.toml

@@ -7,6 +7,7 @@ license = "Unlicense"
 homepage = "https://trappist.io"
 repository = "https://github.com/TrappistNetwork/trappist"
 edition = "2021"
+default-run = "trappist-collator"
 
 [package.metadata.docs.rs]
 targets = ["x86_64-unknown-linux-gnu"]
@@ -41,7 +42,7 @@ tempfile = "3.2.0"
 [features]
 default = ["with-trappist-runtime"]
 runtime-benchmarks = [
-    "trappist-cli/runtime-benchmarks"
+	"trappist-cli/runtime-benchmarks"
 ]
 with-trappist-runtime = [
 	"trappist-service/with-trappist-runtime",

node/cli/src/cli.rs

@@ -14,7 +14,6 @@
 // You should have received a copy of the GNU General Public License
 // along with Cumulus.  If not, see <http://www.gnu.org/licenses/>.
 
-use service::chain_spec;
 #[cfg(feature = "with-stout-runtime")]
 use service::chain_spec::stout::Extensions;
 #[cfg(feature = "with-trappist-runtime")]

node/cli/src/lib.rs

@@ -1,4 +1,5 @@
-#![warn(missing_docs)]
+// TODO: Write missing docs.
+//#![warn(missing_docs)]
 #![warn(unused_extern_crates)]
 
 #[cfg(feature = "cli")]

node/service/src/chain_spec/stout.rs

@@ -1,9 +1,8 @@
 use cumulus_primitives_core::ParaId;
-use hex_literal::hex;
 use sc_chain_spec::{ChainSpecExtension, ChainSpecGroup};
 use sc_service::ChainType;
 use serde::{Deserialize, Serialize};
-use sp_core::{crypto::UncheckedInto, sr25519, Pair, Public};
+use sp_core::{sr25519, Pair, Public};
 use sp_runtime::traits::{IdentifyAccount, Verify};
 use stout_runtime::{
 	constants::currency::EXISTENTIAL_DEPOSIT, AccountId, AssetsConfig, AuraId, BalancesConfig,

node/service/src/chain_spec/trappist.rs

@@ -1,7 +1,6 @@
 use cumulus_primitives_core::ParaId;
 use hex_literal::hex;
 use sc_chain_spec::{ChainSpecExtension, ChainSpecGroup};
-use sc_cli::DefaultConfigurationValues;
 use sc_service::ChainType;
 use serde::{Deserialize, Serialize};
 use sp_core::{crypto::UncheckedInto, sr25519, Pair, Public};

node/service/src/lib.rs

@@ -30,6 +30,9 @@ use substrate_prometheus_endpoint::Registry;
 
 pub mod chain_spec;
 
+#[cfg(all(feature = "with-trappist-runtime", feature = "with-stout-runtime"))]
+compile_error!("features `with-trappist-runtime` and `with-stout-runtime` are mutually exclusive");
+
 #[cfg(feature = "with-stout-runtime")]
 pub mod stout_executor {
 	pub use stout_runtime;

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

@@ -4,7 +4,7 @@ use frame_support::{
 	sp_runtime::SaturatedConversion,
 	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,88 @@ 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 += 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 += 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 += 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());
-		}
+		});
 
+		// we have filtered out non-compliant assets
+		// insert valid assets into the asset trap implemented by XcmPallet
+		weight += XcmPallet::drop_assets(origin, assets);
 		weight
 	}
 }