Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ impl pallet_identity::Config for Runtime {
type UsernameGracePeriod = ConstU32<{ 3 * DAYS }>;
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ impl pallet_identity::Config for Runtime {
type UsernameGracePeriod = ConstU32<{ 3 * DAYS }>;
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
}

Expand Down
2 changes: 2 additions & 0 deletions polkadot/runtime/common/src/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,8 @@ impl pallet_identity::Config for Test {
type UsernameGracePeriod = ConstU32<10>;
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
type WeightInfo = ();
}

Expand Down
2 changes: 2 additions & 0 deletions polkadot/runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,8 @@ impl pallet_identity::Config for Runtime {
type UsernameGracePeriod = ConstU32<{ 30 * DAYS }>;
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
}

Expand Down
2 changes: 2 additions & 0 deletions polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1127,6 +1127,8 @@ impl pallet_identity::Config for Runtime {
type UsernameGracePeriod = ConstU32<{ 30 * DAYS }>;
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
}

Expand Down
33 changes: 33 additions & 0 deletions prdoc/pr_8179.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Do not make pallet-identity benchmarks signature-dependent

doc:
- audience: Runtime Dev
description: |
- Includes a `BenchmarkHelper` configuration item in `pallet-identity` to handle signing operations.
- 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.
- Adds a default implementation for the empty tuple that leaves the code equivalent.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Adds a default implementation for the empty tuple that leaves the code equivalent.
- Adds a default implementation for the empty tuple that leaves the code equivalent.
Adding the following to your implementation of the `frame_identity::Config` should be enough:
```diff
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
```

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add once we address my previous comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 63c1e13.


Adding the following to your implementation of the `frame_identity::Config` should be enough:
```diff
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
```

crates:
- name: pallet-identity
bump: major
- name: polkadot-runtime-common
bump: patch
- name: westend-runtime
bump: patch
- name: rococo-runtime
bump: patch
- name: pallet-alliance
bump: patch
- name: people-rococo-runtime
bump: patch
- name: people-westend-runtime
bump: patch
2 changes: 2 additions & 0 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1683,6 +1683,8 @@ impl pallet_identity::Config for Runtime {
type UsernameGracePeriod = ConstU32<{ 30 * DAYS }>;
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
type WeightInfo = pallet_identity::weights::SubstrateWeight<Runtime>;
}

Expand Down
12 changes: 12 additions & 0 deletions substrate/frame/alliance/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,16 @@ ord_parameter_types! {
type EnsureOneOrRoot = EitherOfDiverse<EnsureRoot<AccountId>, EnsureSignedBy<One, AccountId>>;
type EnsureTwoOrRoot = EitherOfDiverse<EnsureRoot<AccountId>, EnsureSignedBy<Two, AccountId>>;

#[cfg(feature = "runtime-benchmarks")]
pub struct BenchmarkHelper;

#[cfg(feature = "runtime-benchmarks")]
impl pallet_identity::BenchmarkHelper<AccountU64, AccountU64> for BenchmarkHelper {
fn sign_message(_message: &[u8]) -> (AccountU64, AccountU64) {
(AccountU64(0), AccountU64(0))
}
}

impl pallet_identity::Config for Test {
type RuntimeEvent = RuntimeEvent;
type Currency = Balances;
Expand All @@ -123,6 +133,8 @@ impl pallet_identity::Config for Test {
type UsernameGracePeriod = UsernameGracePeriod;
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = BenchmarkHelper;
type WeightInfo = ();
}

Expand Down
22 changes: 5 additions & 17 deletions substrate/frame/identity/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@ use frame_support::{
traits::{EnsureOrigin, Get, OnFinalize, OnInitialize},
};
use frame_system::RawOrigin;
use sp_io::crypto::{sr25519_generate, sr25519_sign};
use sp_runtime::{
traits::{Bounded, IdentifyAccount, One},
MultiSignature, MultiSigner,
};
use sp_runtime::traits::{Bounded, One};

const SEED: u32 = 0;

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

#[benchmarks(
where
<T as frame_system::Config>::AccountId: From<sp_runtime::AccountId32>,
T::OffchainSignature: From<MultiSignature>,
)]
#[benchmarks]
mod benchmarks {
use super::*;

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

let public = sr25519_generate(0.into(), None);
let who_account: T::AccountId = MultiSigner::Sr25519(public).into_account().into();
let (public, signature) = T::BenchmarkHelper::sign_message(&bounded_username[..]);
let who_account = public.into_account();
let who_lookup = T::Lookup::unlookup(who_account.clone());

let signature = MultiSignature::Sr25519(
sr25519_sign(0.into(), &public, &bounded_username[..]).unwrap(),
);

// Verify signature here to avoid surprise errors at runtime
assert!(signature.verify(&bounded_username[..], &public.into()));
assert!(signature.verify(&bounded_username[..], &who_account));
let use_allocation = match p {
0 => false,
1 => true,
Expand Down
25 changes: 25 additions & 0 deletions substrate/frame/identity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,26 @@ pub mod pallet {
use super::*;
use frame_support::pallet_prelude::*;

#[cfg(feature = "runtime-benchmarks")]
pub trait BenchmarkHelper<Public, Signature> {
fn sign_message(message: &[u8]) -> (Public, Signature);
}
#[cfg(feature = "runtime-benchmarks")]
impl BenchmarkHelper<sp_runtime::MultiSigner, sp_runtime::MultiSignature> for () {
fn sign_message(message: &[u8]) -> (sp_runtime::MultiSigner, sp_runtime::MultiSignature) {
let public = sp_io::crypto::sr25519_generate(0.into(), None);
let signature = sp_runtime::MultiSignature::Sr25519(
sp_io::crypto::sr25519_sign(
0.into(),
&public.into_account().try_into().unwrap(),
message,
)
.unwrap(),
);
(public.into(), signature)
}
}

#[pallet::config]
pub trait Config: frame_system::Config {
/// The overarching event type.
Expand Down Expand Up @@ -227,6 +247,11 @@ pub mod pallet {
#[pallet::constant]
type MaxUsernameLength: Get<u32>;

/// A set of helper functions for benchmarking.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment normally goes above the feature gate.

Can you also please include a line what the default config () means?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was copy-pasted from elsewhere in the repository, so for sure there will be more cases around in other parts not covered in this PR. Just the heads up.

Addressed in 7bb7da3.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also please include a line what the default config () means?

Missed that part. Done in 926e317.

/// The default configuration `()` uses the `SR25519` signature schema.
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper: BenchmarkHelper<Self::SigningPublicKey, Self::OffchainSignature>;

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;
}
Expand Down
2 changes: 2 additions & 0 deletions substrate/frame/identity/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ impl pallet_identity::Config for Test {
type UsernameGracePeriod = ConstU64<2>;
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
type WeightInfo = ();
}

Expand Down
2 changes: 2 additions & 0 deletions substrate/frame/staking-async/runtimes/rc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,8 @@ impl pallet_identity::Config for Runtime {
type UsernameGracePeriod = ConstU32<{ 30 * DAYS }>;
type MaxSuffixLength = ConstU32<7>;
type MaxUsernameLength = ConstU32<32>;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
type WeightInfo = weights::pallet_identity::WeightInfo<Runtime>;
}

Expand Down
Loading