fix: update benchmarks (#790)

Part of KILTprotocol/ticket#3650, built on top
of #787.

What started as a simple fix for the web3name pallet benchmarks, turned
out to be a bigger changeset due to a couple of bugs found in the
Substrate code, which are mentioned in the self-review I gave myself
[below](#790 (review)).

In a gist, what I did is:

1. Update the benchmarking logic for the web3name pallet to delegate the
name creation to a `BenchmarkHelper` type.
2. Implement the trait above for both web3names (which uses the old
implementation) and the dotnames
3. Work around the Substrate bug for our pallets as well as the
`pallet_collective` and `pallet_membership` pallets which we deploy
multiple times
4. Add the benchmarking of the `pallet_membership` deployments of the
`TipsMembership` pallet

dip-template/runtimes/dip-provider/src/lib.rs

@@ -453,6 +453,9 @@ impl pallet_web3_names::Config for Runtime {
 	type Web3Name = Web3Name;
 	type Web3NameOwner = DidIdentifier;
 	type WeightInfo = weights::pallet_web3_names::WeightInfo<Runtime>;
+
+	#[cfg(feature = "runtime-benchmarks")]
+	type BenchmarkHelper = ();
 }
 
 #[cfg(feature = "runtime-benchmarks")]

pallets/pallet-migration/src/mock.rs

@@ -340,6 +340,9 @@ pub mod runtime {
 		type Web3NameOwner = TestWeb3NameOwner;
 		type WeightInfo = ();
 		type BalanceMigrationManager = Migration;
+
+		#[cfg(feature = "runtime-benchmarks")]
+		type BenchmarkHelper = ();
 	}
 
 	#[derive(

pallets/pallet-web3-names/src/benchmarking.rs

@@ -31,7 +31,7 @@ use frame_support::{
 };
 use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
 use sp_runtime::app_crypto::sr25519;
-use sp_std::{vec, vec::Vec};
+use sp_std::vec::Vec;
 
 use kilt_support::{traits::GenerateBenchmarkOrigin, Deposit};
 
@@ -40,6 +40,16 @@ use crate::{
 	Web3NameOwnerOf,
 };
 
+pub trait BenchmarkHelper {
+	fn generate_name_input_with_length(length: usize) -> Vec<u8>;
+}
+
+impl BenchmarkHelper for () {
+	fn generate_name_input_with_length(length: usize) -> Vec<u8> {
+		sp_std::vec![b'a'; length]
+	}
+}
+
 const CALLER_SEED: u32 = 0;
 const OWNER_SEED: u32 = 1;
 
@@ -55,28 +65,24 @@ where
 	CurrencyOf::<T, I>::set_balance(account, balance);
 }
 
-fn generate_web3_name_input(length: usize) -> Vec<u8> {
-	vec![b'1'; length]
-}
-
 benchmarks_instance_pallet! {
 	where_clause {
 		where
 		T::AccountId: From<sr25519::Public>,
-		T::Web3NameOwner: From<T::AccountId>,
-		T::OwnerOrigin: GenerateBenchmarkOrigin<T::RuntimeOrigin, T::AccountId, T::Web3NameOwner>,
-		T::BanOrigin: EnsureOrigin<T::RuntimeOrigin>,
+		<T as Config<I>>::Web3NameOwner: From<T::AccountId>,
+		<T as Config<I>>::OwnerOrigin: GenerateBenchmarkOrigin<T::RuntimeOrigin, T::AccountId, <T as Config<I>>::Web3NameOwner>,
+		<T as Config<I>>::BanOrigin: EnsureOrigin<T::RuntimeOrigin>,
 		<<T as Config<I>>::Web3Name as TryFrom<Vec<u8>>>::Error: Into<Error<T, I>>,
 		<T as Config<I>>::Currency: Mutate<T::AccountId>,
 	}
 
 	claim {
-		let n in (T::MinNameLength::get()) .. (T::MaxNameLength::get());
+		let n in (<T as Config<I>>::MinNameLength::get()) .. (<T as Config<I>>::MaxNameLength::get());
 		let caller: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
 		let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
-		let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(generate_web3_name_input(n.saturated_into())).expect("BoundedVec creation should not fail.");
+		let web3_name_input: BoundedVec<u8, <T as Config<I>>::MaxNameLength> = BoundedVec::try_from(<T as Config<I>>::BenchmarkHelper::generate_name_input_with_length(n.saturated_into())).expect("BoundedVec creation should not fail.");
 		let web3_name_input_clone = web3_name_input.clone();
-		let origin = T::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
+		let origin = <T as Config<I>>::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
 
 		make_free_for_did::<T, I>(&caller);
 	}: _<T::RuntimeOrigin>(origin, web3_name_input_clone)
@@ -91,8 +97,8 @@ benchmarks_instance_pallet! {
 	release_by_owner {
 		let caller: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
 		let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
-		let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(generate_web3_name_input(T::MaxNameLength::get().saturated_into())).expect("BoundedVec creation should not fail.");
-		let origin = T::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
+		let web3_name_input: BoundedVec<u8, <T as Config<I>>::MaxNameLength> = BoundedVec::try_from(<T as Config<I>>::BenchmarkHelper::generate_name_input_with_length(<T as Config<I>>::MaxNameLength::get().saturated_into())).expect("BoundedVec creation should not fail.");
+		let origin = <T as Config<I>>::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
 
 		make_free_for_did::<T, I>(&caller);
 		Pallet::<T, I>::claim(origin.clone(), web3_name_input.clone()).expect("Should register the claimed web3 name.");
@@ -106,12 +112,12 @@ benchmarks_instance_pallet! {
 	}
 
 	reclaim_deposit {
-		let n in (T::MinNameLength::get()) .. (T::MaxNameLength::get());
+		let n in (<T as Config<I>>::MinNameLength::get()) .. (<T as Config<I>>::MaxNameLength::get());
 		let caller: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
 		let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
-		let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(generate_web3_name_input(n.saturated_into())).expect("BoundedVec creation should not fail.");
+		let web3_name_input: BoundedVec<u8, <T as Config<I>>::MaxNameLength> = BoundedVec::try_from(<T as Config<I>>::BenchmarkHelper::generate_name_input_with_length(n.saturated_into())).expect("BoundedVec creation should not fail.");
 		let web3_name_input_clone = web3_name_input.clone();
-		let did_origin = T::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
+		let did_origin = <T as Config<I>>::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
 		let signed_origin = RawOrigin::Signed(caller.clone());
 
 		make_free_for_did::<T, I>(&caller);
@@ -126,12 +132,12 @@ benchmarks_instance_pallet! {
 	}
 
 	ban {
-		let n in (T::MinNameLength::get()) .. (T::MaxNameLength::get());
+		let n in (<T as Config<I>>::MinNameLength::get()) .. (<T as Config<I>>::MaxNameLength::get());
 		let caller: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
 		let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
-		let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(generate_web3_name_input(n.saturated_into())).expect("BoundedVec creation should not fail.");
+		let web3_name_input: BoundedVec<u8, <T as Config<I>>::MaxNameLength> = BoundedVec::try_from(<T as Config<I>>::BenchmarkHelper::generate_name_input_with_length(n.saturated_into())).expect("BoundedVec creation should not fail.");
 		let web3_name_input_clone = web3_name_input.clone();
-		let did_origin = T::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
+		let did_origin = <T as Config<I>>::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
 		let ban_origin = RawOrigin::Root;
 
 		make_free_for_did::<T, I>(&caller);
@@ -147,10 +153,10 @@ benchmarks_instance_pallet! {
 	}
 
 	unban {
-		let n in (T::MinNameLength::get()) .. (T::MaxNameLength::get());
+		let n in (<T as Config<I>>::MinNameLength::get()) .. (<T as Config<I>>::MaxNameLength::get());
 		let caller: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
 		let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
-		let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(generate_web3_name_input(n.saturated_into())).expect("BoundedVec creation should not fail.");
+		let web3_name_input: BoundedVec<u8, <T as Config<I>>::MaxNameLength> = BoundedVec::try_from(<T as Config<I>>::BenchmarkHelper::generate_name_input_with_length(n.saturated_into())).expect("BoundedVec creation should not fail.");
 		let web3_name_input_clone = web3_name_input.clone();
 		let ban_origin = RawOrigin::Root;
 
@@ -170,17 +176,17 @@ benchmarks_instance_pallet! {
 		let deposit_owner_old: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
 		let deposit_owner_new: AccountIdOf<T> = account("caller", 1, CALLER_SEED);
 		let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
-		let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(
-			generate_web3_name_input(T::MaxNameLength::get().saturated_into())
+		let web3_name_input: BoundedVec<u8, <T as Config<I>>::MaxNameLength> = BoundedVec::try_from(
+			<T as Config<I>>::BenchmarkHelper::generate_name_input_with_length(<T as Config<I>>::MaxNameLength::get().saturated_into())
 		).expect("BoundedVec creation should not fail.");
 		let web3_name_input_clone = web3_name_input.clone();
-		let origin_create = T::OwnerOrigin::generate_origin(deposit_owner_old.clone(), owner.clone());
+		let origin_create = <T as Config<I>>::OwnerOrigin::generate_origin(deposit_owner_old.clone(), owner.clone());
 
 		make_free_for_did::<T, I>(&deposit_owner_old);
 		make_free_for_did::<T, I>(&deposit_owner_new);
 		Pallet::<T, I>::claim(origin_create, web3_name_input.clone()).expect("Should register the claimed web3 name.");
 
-		let origin = T::OwnerOrigin::generate_origin(deposit_owner_new.clone(), owner);
+		let origin = <T as Config<I>>::OwnerOrigin::generate_origin(deposit_owner_new.clone(), owner);
 	}: _<T::RuntimeOrigin>(origin)
 	verify {
 		let Ok(web3_name) = Web3NameOf::<T, I>::try_from(web3_name_input.to_vec()) else {
@@ -195,8 +201,8 @@ benchmarks_instance_pallet! {
 	update_deposit {
 		let deposit_owner: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
 		let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
-		let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(
-			generate_web3_name_input(T::MaxNameLength::get().saturated_into())
+		let web3_name_input: BoundedVec<u8, <T as Config<I>>::MaxNameLength> = BoundedVec::try_from(
+			<T as Config<I>>::BenchmarkHelper::generate_name_input_with_length(<T as Config<I>>::MaxNameLength::get().saturated_into())
 		).expect("BoundedVec creation should not fail.");
 		let Ok(web3_name) = Web3NameOf::<T, I>::try_from(web3_name_input.to_vec()) else {
 			panic!();

pallets/pallet-web3-names/src/lib.rs

@@ -36,6 +36,8 @@ mod tests;
 
 #[cfg(feature = "runtime-benchmarks")]
 mod benchmarking;
+#[cfg(feature = "runtime-benchmarks")]
+pub use benchmarking::BenchmarkHelper;
 
 pub use crate::{default_weights::WeightInfo, pallet::*};
 
@@ -149,6 +151,9 @@ pub mod pallet {
 
 		/// Migration manager to handle new created entries
 		type BalanceMigrationManager: BalanceMigrationManager<AccountIdOf<Self>, BalanceOf<Self, I>>;
+
+		#[cfg(feature = "runtime-benchmarks")]
+		type BenchmarkHelper: crate::benchmarking::BenchmarkHelper;
 	}
 
 	#[pallet::event]

pallets/pallet-web3-names/src/mock.rs

@@ -166,6 +166,9 @@ pub(crate) mod runtime {
 		type Web3NameOwner = TestWeb3NameOwner;
 		type WeightInfo = ();
 		type BalanceMigrationManager = ();
+
+		#[cfg(feature = "runtime-benchmarks")]
+		type BenchmarkHelper = ();
 	}
 
 	impl mock_origin::Config for Test {

runtimes/common/src/constants.rs

@@ -512,8 +512,8 @@ pub mod dot_names {
 
 	const MIN_NAME_LENGTH: u32 = 3;
 	const MAX_NAME_LENGTH: u32 = 28;
-	const DOT_NAME_SUFFIX: &str = ".dot";
 
+	pub const DOT_NAME_SUFFIX: &str = ".dot";
 	#[allow(clippy::as_conversions)]
 	pub const MIN_LENGTH: u32 = MIN_NAME_LENGTH + DOT_NAME_SUFFIX.len() as u32;
 	#[allow(clippy::as_conversions)]

runtimes/common/src/dip/mock.rs

@@ -158,6 +158,9 @@ impl pallet_web3_names::Config for TestRuntime {
 	type Web3Name = AsciiWeb3Name<Self>;
 	type Web3NameOwner = DidIdentifier;
 	type WeightInfo = ();
+
+	#[cfg(feature = "runtime-benchmarks")]
+	type BenchmarkHelper = ();
 }
 
 impl pallet_did_lookup::Config for TestRuntime {

runtimes/common/src/dot_names/mod.rs

@@ -25,6 +25,8 @@ use sp_std::vec::Vec;
 
 use pallet_web3_names::{Config, Error};
 
+use crate::constants::dot_names::DOT_NAME_SUFFIX;
+
 #[cfg(test)]
 mod tests;
 
@@ -64,7 +66,7 @@ impl<const MIN_LENGTH: u32, const MAX_LENGTH: u32> TryFrom<Vec<u8>> for DotName<
 	}
 }
 fn is_valid_dot_name(input: &[u8]) -> bool {
-	let Some(dot_name_without_suffix) = input.strip_suffix(b".dot") else {
+	let Some(dot_name_without_suffix) = input.strip_suffix(DOT_NAME_SUFFIX.as_bytes()) else {
 		return false;
 	};
 	// Char validation logic taken from https://github.com/paritytech/polkadot-sdk/blob/657b5503a04e97737696fa7344641019350fb521/substrate/frame/identity/src/lib.rs#L1435

runtimes/kestrel/src/lib.rs

@@ -456,6 +456,9 @@ impl pallet_web3_names::Config for Runtime {
 	type Web3NameOwner = DidIdentifier;
 	type WeightInfo = ();
 	type BalanceMigrationManager = ();
+
+	#[cfg(feature = "runtime-benchmarks")]
+	type BenchmarkHelper = ();
 }
 
 parameter_types! {