Skip to content

Commit 1b2e170

Browse files
José Molina Colmeneropgherveou
authored andcommitted
Do not make pallet-identity benchmarks signature-dependent (#8179)
Similar to #4756 for `pallet-nfts`, the changes proposed in this PR intend to make benchmarks for `pallet-identity` signature-agnostic by the inclusion of a benchmark helper with sane defaults to handle the signing operations.
1 parent 11c7297 commit 1b2e170

12 files changed

Lines changed: 91 additions & 17 deletions

File tree

cumulus/parachains/runtimes/people/people-rococo/src/people.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ impl pallet_identity::Config for Runtime {
6262
type UsernameGracePeriod = ConstU32<{ 3 * DAYS }>;
6363
type MaxSuffixLength = ConstU32<7>;
6464
type MaxUsernameLength = ConstU32<32>;
65+
#[cfg(feature = "runtime-benchmarks")]
66+
type BenchmarkHelper = ();
6567
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
6668
}
6769

cumulus/parachains/runtimes/people/people-westend/src/people.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ impl pallet_identity::Config for Runtime {
6262
type UsernameGracePeriod = ConstU32<{ 3 * DAYS }>;
6363
type MaxSuffixLength = ConstU32<7>;
6464
type MaxUsernameLength = ConstU32<32>;
65+
#[cfg(feature = "runtime-benchmarks")]
66+
type BenchmarkHelper = ();
6567
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
6668
}
6769

polkadot/runtime/common/src/integration_tests.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,8 @@ impl pallet_identity::Config for Test {
305305
type UsernameGracePeriod = ConstU32<10>;
306306
type MaxSuffixLength = ConstU32<7>;
307307
type MaxUsernameLength = ConstU32<32>;
308+
#[cfg(feature = "runtime-benchmarks")]
309+
type BenchmarkHelper = ();
308310
type WeightInfo = ();
309311
}
310312

polkadot/runtime/rococo/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,8 @@ impl pallet_identity::Config for Runtime {
780780
type UsernameGracePeriod = ConstU32<{ 30 * DAYS }>;
781781
type MaxSuffixLength = ConstU32<7>;
782782
type MaxUsernameLength = ConstU32<32>;
783+
#[cfg(feature = "runtime-benchmarks")]
784+
type BenchmarkHelper = ();
783785
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
784786
}
785787

polkadot/runtime/westend/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,8 @@ impl pallet_identity::Config for Runtime {
11271127
type UsernameGracePeriod = ConstU32<{ 30 * DAYS }>;
11281128
type MaxSuffixLength = ConstU32<7>;
11291129
type MaxUsernameLength = ConstU32<32>;
1130+
#[cfg(feature = "runtime-benchmarks")]
1131+
type BenchmarkHelper = ();
11301132
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
11311133
}
11321134

prdoc/pr_8179.prdoc

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
2+
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
3+
4+
title: Do not make pallet-identity benchmarks signature-dependent
5+
6+
doc:
7+
- audience: Runtime Dev
8+
description: |
9+
- Includes a `BenchmarkHelper` configuration item in `pallet-identity` to handle signing operations.
10+
- Abstracts away the explicit link with Sr25519 schema in the benchmarks, allowing chains with a different one to be able to run them and calculate the weights.
11+
- Adds a default implementation for the empty tuple that leaves the code equivalent.
12+
13+
Adding the following to your implementation of the `frame_identity::Config` should be enough:
14+
```diff
15+
#[cfg(feature = "runtime-benchmarks")]
16+
type BenchmarkHelper = ();
17+
```
18+
19+
crates:
20+
- name: pallet-identity
21+
bump: major
22+
- name: polkadot-runtime-common
23+
bump: patch
24+
- name: westend-runtime
25+
bump: patch
26+
- name: rococo-runtime
27+
bump: patch
28+
- name: pallet-alliance
29+
bump: patch
30+
- name: people-rococo-runtime
31+
bump: patch
32+
- name: people-westend-runtime
33+
bump: patch

substrate/bin/node/runtime/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1683,6 +1683,8 @@ impl pallet_identity::Config for Runtime {
16831683
type UsernameGracePeriod = ConstU32<{ 30 * DAYS }>;
16841684
type MaxSuffixLength = ConstU32<7>;
16851685
type MaxUsernameLength = ConstU32<32>;
1686+
#[cfg(feature = "runtime-benchmarks")]
1687+
type BenchmarkHelper = ();
16861688
type WeightInfo = pallet_identity::weights::SubstrateWeight<Runtime>;
16871689
}
16881690

substrate/frame/alliance/src/mock.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,16 @@ ord_parameter_types! {
103103
type EnsureOneOrRoot = EitherOfDiverse<EnsureRoot<AccountId>, EnsureSignedBy<One, AccountId>>;
104104
type EnsureTwoOrRoot = EitherOfDiverse<EnsureRoot<AccountId>, EnsureSignedBy<Two, AccountId>>;
105105

106+
#[cfg(feature = "runtime-benchmarks")]
107+
pub struct BenchmarkHelper;
108+
109+
#[cfg(feature = "runtime-benchmarks")]
110+
impl pallet_identity::BenchmarkHelper<AccountU64, AccountU64> for BenchmarkHelper {
111+
fn sign_message(_message: &[u8]) -> (AccountU64, AccountU64) {
112+
(AccountU64(0), AccountU64(0))
113+
}
114+
}
115+
106116
impl pallet_identity::Config for Test {
107117
type RuntimeEvent = RuntimeEvent;
108118
type Currency = Balances;
@@ -123,6 +133,8 @@ impl pallet_identity::Config for Test {
123133
type UsernameGracePeriod = UsernameGracePeriod;
124134
type MaxSuffixLength = ConstU32<7>;
125135
type MaxUsernameLength = ConstU32<32>;
136+
#[cfg(feature = "runtime-benchmarks")]
137+
type BenchmarkHelper = BenchmarkHelper;
126138
type WeightInfo = ();
127139
}
128140

substrate/frame/identity/src/benchmarking.rs

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,7 @@ use frame_support::{
2929
traits::{EnsureOrigin, Get, OnFinalize, OnInitialize},
3030
};
3131
use frame_system::RawOrigin;
32-
use sp_io::crypto::{sr25519_generate, sr25519_sign};
33-
use sp_runtime::{
34-
traits::{Bounded, IdentifyAccount, One},
35-
MultiSignature, MultiSigner,
36-
};
32+
use sp_runtime::traits::{Bounded, One};
3733

3834
const SEED: u32 = 0;
3935

@@ -131,11 +127,7 @@ fn bounded_username<T: Config>(username: Vec<u8>, suffix: Vec<u8>) -> Username<T
131127
Username::<T>::try_from(full_username).expect("test usernames should fit within bounds")
132128
}
133129

134-
#[benchmarks(
135-
where
136-
<T as frame_system::Config>::AccountId: From<sp_runtime::AccountId32>,
137-
T::OffchainSignature: From<MultiSignature>,
138-
)]
130+
#[benchmarks]
139131
mod benchmarks {
140132
use super::*;
141133

@@ -625,16 +617,12 @@ mod benchmarks {
625617
let username = bench_username();
626618
let bounded_username = bounded_username::<T>(username.clone(), suffix.clone());
627619

628-
let public = sr25519_generate(0.into(), None);
629-
let who_account: T::AccountId = MultiSigner::Sr25519(public).into_account().into();
620+
let (public, signature) = T::BenchmarkHelper::sign_message(&bounded_username[..]);
621+
let who_account = public.into_account();
630622
let who_lookup = T::Lookup::unlookup(who_account.clone());
631623

632-
let signature = MultiSignature::Sr25519(
633-
sr25519_sign(0.into(), &public, &bounded_username[..]).unwrap(),
634-
);
635-
636624
// Verify signature here to avoid surprise errors at runtime
637-
assert!(signature.verify(&bounded_username[..], &public.into()));
625+
assert!(signature.verify(&bounded_username[..], &who_account));
638626
let use_allocation = match p {
639627
0 => false,
640628
1 => true,

substrate/frame/identity/src/lib.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,26 @@ pub mod pallet {
150150
use super::*;
151151
use frame_support::pallet_prelude::*;
152152

153+
#[cfg(feature = "runtime-benchmarks")]
154+
pub trait BenchmarkHelper<Public, Signature> {
155+
fn sign_message(message: &[u8]) -> (Public, Signature);
156+
}
157+
#[cfg(feature = "runtime-benchmarks")]
158+
impl BenchmarkHelper<sp_runtime::MultiSigner, sp_runtime::MultiSignature> for () {
159+
fn sign_message(message: &[u8]) -> (sp_runtime::MultiSigner, sp_runtime::MultiSignature) {
160+
let public = sp_io::crypto::sr25519_generate(0.into(), None);
161+
let signature = sp_runtime::MultiSignature::Sr25519(
162+
sp_io::crypto::sr25519_sign(
163+
0.into(),
164+
&public.into_account().try_into().unwrap(),
165+
message,
166+
)
167+
.unwrap(),
168+
);
169+
(public.into(), signature)
170+
}
171+
}
172+
153173
#[pallet::config]
154174
pub trait Config: frame_system::Config {
155175
/// The overarching event type.
@@ -227,6 +247,11 @@ pub mod pallet {
227247
#[pallet::constant]
228248
type MaxUsernameLength: Get<u32>;
229249

250+
/// A set of helper functions for benchmarking.
251+
/// The default configuration `()` uses the `SR25519` signature schema.
252+
#[cfg(feature = "runtime-benchmarks")]
253+
type BenchmarkHelper: BenchmarkHelper<Self::SigningPublicKey, Self::OffchainSignature>;
254+
230255
/// Weight information for extrinsics in this pallet.
231256
type WeightInfo: WeightInfo;
232257
}

0 commit comments

Comments
 (0)