Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate pallet-session-benchmarking to bench V2 syntax #6294

Merged
merged 6 commits into from
Dec 2, 2024
+48 −26
Merged

migrate pallet-session-benchmarking to bench V2 syntax #6294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0d1ead0
migrate pallet-session-benchmarking to frame v2
clangenb Oct 30, 2024
c81bdb9
fix testing benchmarks
clangenb Oct 30, 2024
df232f9
Merge remote-tracking branch 'upstream/master' into cl/pallet-session…
clangenb Nov 29, 2024
26ef444
fmt
clangenb Nov 29, 2024
9aa51eb
Merge branch 'master' into cl/pallet-sessions-bench-v2
shawntabrizi Nov 29, 2024
2de515a
Merge branch 'master' into cl/pallet-sessions-bench-v2
re-gius Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 44 additions & 24 deletions substrate/frame/session/benchmarking/src/inner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use alloc::{vec, vec::Vec};
use sp_runtime::traits::{One, StaticLookup, TrailingZeroInput};

use codec::Decode;
use frame_benchmarking::v1::benchmarks;
use frame_benchmarking::v2::*;
use frame_support::traits::{Get, KeyOwnerProofSystem, OnInitialize};
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
use pallet_session::{historical::Pallet as Historical, Pallet as Session, *};
Expand All @@ -45,8 +45,12 @@ impl<T: Config> OnInitialize<BlockNumberFor<T>> for Pallet<T> {
}
}

benchmarks! {
set_keys {
#[benchmarks]
mod benchmarks {
use super::*;

#[benchmark]
fn set_keys() -> Result<(), BenchmarkError> {
let n = MaxNominationsOf::<T>::get();
let (v_stash, _) = create_validator_with_nominators::<T>(
n,
Expand All @@ -58,13 +62,19 @@ benchmarks! {
let v_controller = pallet_staking::Pallet::<T>::bonded(&v_stash).ok_or("not stash")?;

let keys = T::Keys::decode(&mut TrailingZeroInput::zeroes()).unwrap();
let proof: Vec<u8> = vec![0,1,2,3];
let proof: Vec<u8> = vec![0, 1, 2, 3];
// Whitelist controller account from further DB operations.
let v_controller_key = frame_system::Account::<T>::hashed_key_for(&v_controller);
frame_benchmarking::benchmarking::add_to_whitelist(v_controller_key.into());
}: _(RawOrigin::Signed(v_controller), keys, proof)

purge_keys {
#[extrinsic_call]
_(RawOrigin::Signed(v_controller), keys, proof);

Ok(())
}

#[benchmark]
fn purge_keys() -> Result<(), BenchmarkError> {
let n = MaxNominationsOf::<T>::get();
let (v_stash, _) = create_validator_with_nominators::<T>(
n,
Expand All @@ -75,45 +85,55 @@ benchmarks! {
)?;
let v_controller = pallet_staking::Pallet::<T>::bonded(&v_stash).ok_or("not stash")?;
let keys = T::Keys::decode(&mut TrailingZeroInput::zeroes()).unwrap();
let proof: Vec<u8> = vec![0,1,2,3];
let proof: Vec<u8> = vec![0, 1, 2, 3];
Session::<T>::set_keys(RawOrigin::Signed(v_controller.clone()).into(), keys, proof)?;
// Whitelist controller account from further DB operations.
let v_controller_key = frame_system::Account::<T>::hashed_key_for(&v_controller);
frame_benchmarking::benchmarking::add_to_whitelist(v_controller_key.into());
}: _(RawOrigin::Signed(v_controller))

#[extra]
check_membership_proof_current_session {
let n in 2 .. MAX_VALIDATORS as u32;
#[extrinsic_call]
_(RawOrigin::Signed(v_controller));

Ok(())
}

#[benchmark(extra)]
fn check_membership_proof_current_session(n: Linear<2, MAX_VALIDATORS>) {
let (key, key_owner_proof1) = check_membership_proof_setup::<T>(n);
let key_owner_proof2 = key_owner_proof1.clone();
}: {
Historical::<T>::check_proof(key, key_owner_proof1);
}
verify {

#[block]
{
Historical::<T>::check_proof(key, key_owner_proof1);
}

assert!(Historical::<T>::check_proof(key, key_owner_proof2).is_some());
}

#[extra]
check_membership_proof_historical_session {
let n in 2 .. MAX_VALIDATORS as u32;

#[benchmark(extra)]
fn check_membership_proof_historical_session(n: Linear<2, MAX_VALIDATORS>) {
let (key, key_owner_proof1) = check_membership_proof_setup::<T>(n);

// skip to the next session so that the session is historical
// and the membership merkle proof must be checked.
Session::<T>::rotate_session();

let key_owner_proof2 = key_owner_proof1.clone();
}: {
Historical::<T>::check_proof(key, key_owner_proof1);
}
verify {

#[block]
{
Historical::<T>::check_proof(key, key_owner_proof1);
}

assert!(Historical::<T>::check_proof(key, key_owner_proof2).is_some());
}

impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test, extra = false);
impl_benchmark_test_suite!(
Pallet,
crate::mock::new_test_ext(),
crate::mock::Test,
extra = false
);
}

/// Sets up the benchmark for checking a membership proof. It creates the given
Expand Down
6 changes: 4 additions & 2 deletions substrate/frame/session/benchmarking/src/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use frame_support::{
derive_impl, parameter_types,
traits::{ConstU32, ConstU64},
};
use sp_runtime::{traits::IdentityLookup, BuildStorage};
use sp_runtime::{traits::IdentityLookup, BuildStorage, KeyTypeId};

type AccountId = u64;
type Nonce = u32;
Expand All @@ -42,6 +42,7 @@ frame_support::construct_runtime!(
Balances: pallet_balances,
Staking: pallet_staking,
Session: pallet_session,
Historical: pallet_session::historical
}
Comment on lines +45 to 46
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This failed on master:

---- mock::test_genesis_config_builds stdout ----
thread 'mock::test_genesis_config_builds' panicked at /home/clang/code/polkadot-sdk/substrate/frame/session/src/historical/mod.rs:93:15:
No name_hash found for the pallet in the runtime! This usually means that the pallet wasn't added to `construct_runtime!`.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

);

Expand Down Expand Up @@ -79,7 +80,8 @@ sp_runtime::impl_opaque_keys! {

pub struct TestSessionHandler;
impl pallet_session::SessionHandler<AccountId> for TestSessionHandler {
const KEY_TYPE_IDS: &'static [sp_runtime::KeyTypeId] = &[];
// corresponds to the opaque key id above
const KEY_TYPE_IDS: &'static [KeyTypeId] = &[KeyTypeId([100u8, 117u8, 109u8, 121u8])];
Comment on lines +83 to +84
Copy link
Contributor Author

@clangenb clangenb Oct 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This also failed on master.

---- mock::test_genesis_config_builds stdout ----
thread 'mock::test_genesis_config_builds' panicked at /home/clang/code/polkadot-sdk/substrate/frame/session/src/lib.rs:440:17:
Number of keys in session handler and session keys does not match
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I was a bit surprised that cargo test -p pallet-session-benchmarking --features runtime-benchmarks failed on master, is this not CI tested?

Copy link
Contributor

Choose a reason for hiding this comment

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

I opened an issue: #6726


fn on_genesis_session<Ks: sp_runtime::traits::OpaqueKeys>(_validators: &[(AccountId, Ks)]) {}

Expand Down
Loading