Skip to content

Commit af07f5a

Browse files
ntn-x2ntn-x2
authored
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
1 parent a27c8b6 commit af07f5a

34 files changed

+3120
-2798
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,9 @@ impl pallet_web3_names::Config for Runtime {
453453
type Web3Name = Web3Name;
454454
type Web3NameOwner = DidIdentifier;
455455
type WeightInfo = weights::pallet_web3_names::WeightInfo<Runtime>;
456+
457+
#[cfg(feature = "runtime-benchmarks")]
458+
type BenchmarkHelper = ();
456459
}
457460

458461
#[cfg(feature = "runtime-benchmarks")]

‎pallets/pallet-migration/src/mock.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,9 @@ pub mod runtime {
340340
type Web3NameOwner = TestWeb3NameOwner;
341341
type WeightInfo = ();
342342
type BalanceMigrationManager = Migration;
343+
344+
#[cfg(feature = "runtime-benchmarks")]
345+
type BenchmarkHelper = ();
343346
}
344347

345348
#[derive(

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

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use frame_support::{
3131
};
3232
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
3333
use sp_runtime::app_crypto::sr25519;
34-
use sp_std::{vec, vec::Vec};
34+
use sp_std::vec::Vec;
3535

3636
use kilt_support::{traits::GenerateBenchmarkOrigin, Deposit};
3737

@@ -40,6 +40,16 @@ use crate::{
4040
Web3NameOwnerOf,
4141
};
4242

43+
pub trait BenchmarkHelper {
44+
fn generate_name_input_with_length(length: usize) -> Vec<u8>;
45+
}
46+
47+
impl BenchmarkHelper for () {
48+
fn generate_name_input_with_length(length: usize) -> Vec<u8> {
49+
sp_std::vec![b'a'; length]
50+
}
51+
}
52+
4353
const CALLER_SEED: u32 = 0;
4454
const OWNER_SEED: u32 = 1;
4555

@@ -55,28 +65,24 @@ where
5565
CurrencyOf::<T, I>::set_balance(account, balance);
5666
}
5767

58-
fn generate_web3_name_input(length: usize) -> Vec<u8> {
59-
vec![b'1'; length]
60-
}
61-
6268
benchmarks_instance_pallet! {
6369
where_clause {
6470
where
6571
T::AccountId: From<sr25519::Public>,
66-
T::Web3NameOwner: From<T::AccountId>,
67-
T::OwnerOrigin: GenerateBenchmarkOrigin<T::RuntimeOrigin, T::AccountId, T::Web3NameOwner>,
68-
T::BanOrigin: EnsureOrigin<T::RuntimeOrigin>,
72+
<T as Config<I>>::Web3NameOwner: From<T::AccountId>,
73+
<T as Config<I>>::OwnerOrigin: GenerateBenchmarkOrigin<T::RuntimeOrigin, T::AccountId, <T as Config<I>>::Web3NameOwner>,
74+
<T as Config<I>>::BanOrigin: EnsureOrigin<T::RuntimeOrigin>,
6975
<<T as Config<I>>::Web3Name as TryFrom<Vec<u8>>>::Error: Into<Error<T, I>>,
7076
<T as Config<I>>::Currency: Mutate<T::AccountId>,
7177
}
7278

7379
claim {
74-
let n in (T::MinNameLength::get()) .. (T::MaxNameLength::get());
80+
let n in (<T as Config<I>>::MinNameLength::get()) .. (<T as Config<I>>::MaxNameLength::get());
7581
let caller: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
7682
let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
77-
let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(generate_web3_name_input(n.saturated_into())).expect("BoundedVec creation should not fail.");
83+
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.");
7884
let web3_name_input_clone = web3_name_input.clone();
79-
let origin = T::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
85+
let origin = <T as Config<I>>::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
8086

8187
make_free_for_did::<T, I>(&caller);
8288
}: _<T::RuntimeOrigin>(origin, web3_name_input_clone)
@@ -91,8 +97,8 @@ benchmarks_instance_pallet! {
9197
release_by_owner {
9298
let caller: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
9399
let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
94-
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.");
95-
let origin = T::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
100+
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.");
101+
let origin = <T as Config<I>>::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
96102

97103
make_free_for_did::<T, I>(&caller);
98104
Pallet::<T, I>::claim(origin.clone(), web3_name_input.clone()).expect("Should register the claimed web3 name.");
@@ -106,12 +112,12 @@ benchmarks_instance_pallet! {
106112
}
107113

108114
reclaim_deposit {
109-
let n in (T::MinNameLength::get()) .. (T::MaxNameLength::get());
115+
let n in (<T as Config<I>>::MinNameLength::get()) .. (<T as Config<I>>::MaxNameLength::get());
110116
let caller: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
111117
let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
112-
let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(generate_web3_name_input(n.saturated_into())).expect("BoundedVec creation should not fail.");
118+
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.");
113119
let web3_name_input_clone = web3_name_input.clone();
114-
let did_origin = T::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
120+
let did_origin = <T as Config<I>>::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
115121
let signed_origin = RawOrigin::Signed(caller.clone());
116122

117123
make_free_for_did::<T, I>(&caller);
@@ -126,12 +132,12 @@ benchmarks_instance_pallet! {
126132
}
127133

128134
ban {
129-
let n in (T::MinNameLength::get()) .. (T::MaxNameLength::get());
135+
let n in (<T as Config<I>>::MinNameLength::get()) .. (<T as Config<I>>::MaxNameLength::get());
130136
let caller: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
131137
let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
132-
let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(generate_web3_name_input(n.saturated_into())).expect("BoundedVec creation should not fail.");
138+
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.");
133139
let web3_name_input_clone = web3_name_input.clone();
134-
let did_origin = T::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
140+
let did_origin = <T as Config<I>>::OwnerOrigin::generate_origin(caller.clone(), owner.clone());
135141
let ban_origin = RawOrigin::Root;
136142

137143
make_free_for_did::<T, I>(&caller);
@@ -147,10 +153,10 @@ benchmarks_instance_pallet! {
147153
}
148154

149155
unban {
150-
let n in (T::MinNameLength::get()) .. (T::MaxNameLength::get());
156+
let n in (<T as Config<I>>::MinNameLength::get()) .. (<T as Config<I>>::MaxNameLength::get());
151157
let caller: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
152158
let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
153-
let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(generate_web3_name_input(n.saturated_into())).expect("BoundedVec creation should not fail.");
159+
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.");
154160
let web3_name_input_clone = web3_name_input.clone();
155161
let ban_origin = RawOrigin::Root;
156162

@@ -170,17 +176,17 @@ benchmarks_instance_pallet! {
170176
let deposit_owner_old: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
171177
let deposit_owner_new: AccountIdOf<T> = account("caller", 1, CALLER_SEED);
172178
let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
173-
let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(
174-
generate_web3_name_input(T::MaxNameLength::get().saturated_into())
179+
let web3_name_input: BoundedVec<u8, <T as Config<I>>::MaxNameLength> = BoundedVec::try_from(
180+
<T as Config<I>>::BenchmarkHelper::generate_name_input_with_length(<T as Config<I>>::MaxNameLength::get().saturated_into())
175181
).expect("BoundedVec creation should not fail.");
176182
let web3_name_input_clone = web3_name_input.clone();
177-
let origin_create = T::OwnerOrigin::generate_origin(deposit_owner_old.clone(), owner.clone());
183+
let origin_create = <T as Config<I>>::OwnerOrigin::generate_origin(deposit_owner_old.clone(), owner.clone());
178184

179185
make_free_for_did::<T, I>(&deposit_owner_old);
180186
make_free_for_did::<T, I>(&deposit_owner_new);
181187
Pallet::<T, I>::claim(origin_create, web3_name_input.clone()).expect("Should register the claimed web3 name.");
182188

183-
let origin = T::OwnerOrigin::generate_origin(deposit_owner_new.clone(), owner);
189+
let origin = <T as Config<I>>::OwnerOrigin::generate_origin(deposit_owner_new.clone(), owner);
184190
}: _<T::RuntimeOrigin>(origin)
185191
verify {
186192
let Ok(web3_name) = Web3NameOf::<T, I>::try_from(web3_name_input.to_vec()) else {
@@ -195,8 +201,8 @@ benchmarks_instance_pallet! {
195201
update_deposit {
196202
let deposit_owner: AccountIdOf<T> = account("caller", 0, CALLER_SEED);
197203
let owner: Web3NameOwnerOf<T, I> = account("owner", 0, OWNER_SEED);
198-
let web3_name_input: BoundedVec<u8, T::MaxNameLength> = BoundedVec::try_from(
199-
generate_web3_name_input(T::MaxNameLength::get().saturated_into())
204+
let web3_name_input: BoundedVec<u8, <T as Config<I>>::MaxNameLength> = BoundedVec::try_from(
205+
<T as Config<I>>::BenchmarkHelper::generate_name_input_with_length(<T as Config<I>>::MaxNameLength::get().saturated_into())
200206
).expect("BoundedVec creation should not fail.");
201207
let Ok(web3_name) = Web3NameOf::<T, I>::try_from(web3_name_input.to_vec()) else {
202208
panic!();

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ mod tests;
3636

3737
#[cfg(feature = "runtime-benchmarks")]
3838
mod benchmarking;
39+
#[cfg(feature = "runtime-benchmarks")]
40+
pub use benchmarking::BenchmarkHelper;
3941

4042
pub use crate::{default_weights::WeightInfo, pallet::*};
4143

@@ -149,6 +151,9 @@ pub mod pallet {
149151

150152
/// Migration manager to handle new created entries
151153
type BalanceMigrationManager: BalanceMigrationManager<AccountIdOf<Self>, BalanceOf<Self, I>>;
154+
155+
#[cfg(feature = "runtime-benchmarks")]
156+
type BenchmarkHelper: crate::benchmarking::BenchmarkHelper;
152157
}
153158

154159
#[pallet::event]

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,9 @@ pub(crate) mod runtime {
166166
type Web3NameOwner = TestWeb3NameOwner;
167167
type WeightInfo = ();
168168
type BalanceMigrationManager = ();
169+
170+
#[cfg(feature = "runtime-benchmarks")]
171+
type BenchmarkHelper = ();
169172
}
170173

171174
impl mock_origin::Config for Test {

‎runtimes/common/src/constants.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,8 +512,8 @@ pub mod dot_names {
512512

513513
const MIN_NAME_LENGTH: u32 = 3;
514514
const MAX_NAME_LENGTH: u32 = 28;
515-
const DOT_NAME_SUFFIX: &str = ".dot";
516515

516+
pub const DOT_NAME_SUFFIX: &str = ".dot";
517517
#[allow(clippy::as_conversions)]
518518
pub const MIN_LENGTH: u32 = MIN_NAME_LENGTH + DOT_NAME_SUFFIX.len() as u32;
519519
#[allow(clippy::as_conversions)]

‎runtimes/common/src/dip/mock.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,9 @@ impl pallet_web3_names::Config for TestRuntime {
158158
type Web3Name = AsciiWeb3Name<Self>;
159159
type Web3NameOwner = DidIdentifier;
160160
type WeightInfo = ();
161+
162+
#[cfg(feature = "runtime-benchmarks")]
163+
type BenchmarkHelper = ();
161164
}
162165

163166
impl pallet_did_lookup::Config for TestRuntime {

‎runtimes/common/src/dot_names/mod.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ use sp_std::vec::Vec;
2525

2626
use pallet_web3_names::{Config, Error};
2727

28+
use crate::constants::dot_names::DOT_NAME_SUFFIX;
29+
2830
#[cfg(test)]
2931
mod tests;
3032

@@ -64,7 +66,7 @@ impl<const MIN_LENGTH: u32, const MAX_LENGTH: u32> TryFrom<Vec<u8>> for DotName<
6466
}
6567
}
6668
fn is_valid_dot_name(input: &[u8]) -> bool {
67-
let Some(dot_name_without_suffix) = input.strip_suffix(b".dot") else {
69+
let Some(dot_name_without_suffix) = input.strip_suffix(DOT_NAME_SUFFIX.as_bytes()) else {
6870
return false;
6971
};
7072
// 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‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,9 @@ impl pallet_web3_names::Config for Runtime {
456456
type Web3NameOwner = DidIdentifier;
457457
type WeightInfo = ();
458458
type BalanceMigrationManager = ();
459+
460+
#[cfg(feature = "runtime-benchmarks")]
461+
type BenchmarkHelper = ();
459462
}
460463

461464
parameter_types! {

0 commit comments

Comments
 (0)