From 0845044454c005b577eab7afaea18583bd7e3dd3 Mon Sep 17 00:00:00 2001 From: clangenb <37865735+clangenb@users.noreply.github.com> Date: Mon, 2 Dec 2024 17:07:21 +0100 Subject: [PATCH 1/2] migrate pallet-session-benchmarking to bench V2 syntax (#6294) Migrates pallet-session-benchmarking to bench V2 syntax. Part of: * #6202 --------- Co-authored-by: Shawn Tabrizi Co-authored-by: Giuseppe Re --- .../frame/session/benchmarking/src/inner.rs | 68 ++++++++++++------- .../frame/session/benchmarking/src/mock.rs | 6 +- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/substrate/frame/session/benchmarking/src/inner.rs b/substrate/frame/session/benchmarking/src/inner.rs index 9ba47b34ed7a..9789b6bb593d 100644 --- a/substrate/frame/session/benchmarking/src/inner.rs +++ b/substrate/frame/session/benchmarking/src/inner.rs @@ -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, *}; @@ -45,8 +45,12 @@ impl OnInitialize> for Pallet { } } -benchmarks! { - set_keys { +#[benchmarks] +mod benchmarks { + use super::*; + + #[benchmark] + fn set_keys() -> Result<(), BenchmarkError> { let n = MaxNominationsOf::::get(); let (v_stash, _) = create_validator_with_nominators::( n, @@ -58,13 +62,19 @@ benchmarks! { let v_controller = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; let keys = T::Keys::decode(&mut TrailingZeroInput::zeroes()).unwrap(); - let proof: Vec = vec![0,1,2,3]; + let proof: Vec = vec![0, 1, 2, 3]; // Whitelist controller account from further DB operations. let v_controller_key = frame_system::Account::::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::::get(); let (v_stash, _) = create_validator_with_nominators::( n, @@ -75,30 +85,33 @@ benchmarks! { )?; let v_controller = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; let keys = T::Keys::decode(&mut TrailingZeroInput::zeroes()).unwrap(); - let proof: Vec = vec![0,1,2,3]; + let proof: Vec = vec![0, 1, 2, 3]; Session::::set_keys(RawOrigin::Signed(v_controller.clone()).into(), keys, proof)?; // Whitelist controller account from further DB operations. let v_controller_key = frame_system::Account::::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::(n); let key_owner_proof2 = key_owner_proof1.clone(); - }: { - Historical::::check_proof(key, key_owner_proof1); - } - verify { + + #[block] + { + Historical::::check_proof(key, key_owner_proof1); + } + assert!(Historical::::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::(n); // skip to the next session so that the session is historical @@ -106,14 +119,21 @@ benchmarks! { Session::::rotate_session(); let key_owner_proof2 = key_owner_proof1.clone(); - }: { - Historical::::check_proof(key, key_owner_proof1); - } - verify { + + #[block] + { + Historical::::check_proof(key, key_owner_proof1); + } + assert!(Historical::::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 diff --git a/substrate/frame/session/benchmarking/src/mock.rs b/substrate/frame/session/benchmarking/src/mock.rs index 2aec58cceded..346cd04c0fa9 100644 --- a/substrate/frame/session/benchmarking/src/mock.rs +++ b/substrate/frame/session/benchmarking/src/mock.rs @@ -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; @@ -42,6 +42,7 @@ frame_support::construct_runtime!( Balances: pallet_balances, Staking: pallet_staking, Session: pallet_session, + Historical: pallet_session::historical } ); @@ -79,7 +80,8 @@ sp_runtime::impl_opaque_keys! { pub struct TestSessionHandler; impl pallet_session::SessionHandler 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])]; fn on_genesis_session(_validators: &[(AccountId, Ks)]) {} From 3d8da815ecd12b8f04daf87d6ffba5ec4a181806 Mon Sep 17 00:00:00 2001 From: clangenb <37865735+clangenb@users.noreply.github.com> Date: Mon, 2 Dec 2024 17:36:52 +0100 Subject: [PATCH 2/2] migrate pallet-offences-benchmarking to benchmark v2 syntax (#6300) Migrates pallet-offences-benchmarking to benchmark v2 syntax. Part of: * #6202 --------- Co-authored-by: Giuseppe Re --- .../frame/offences/benchmarking/src/inner.rs | 107 +++++++++++------- .../frame/offences/benchmarking/src/mock.rs | 5 +- 2 files changed, 66 insertions(+), 46 deletions(-) diff --git a/substrate/frame/offences/benchmarking/src/inner.rs b/substrate/frame/offences/benchmarking/src/inner.rs index 573114de0742..75f3e9931e34 100644 --- a/substrate/frame/offences/benchmarking/src/inner.rs +++ b/substrate/frame/offences/benchmarking/src/inner.rs @@ -19,7 +19,7 @@ use alloc::{vec, vec::Vec}; -use frame_benchmarking::v1::{account, benchmarks}; +use frame_benchmarking::v2::*; use frame_support::traits::Get; use frame_system::{Config as SystemConfig, Pallet as System, RawOrigin}; @@ -144,7 +144,7 @@ fn create_offender(n: u32, nominators: u32) -> Result, &' fn make_offenders( num_offenders: u32, num_nominators: u32, -) -> Result<(Vec>, Vec>), &'static str> { +) -> Result>, &'static str> { Staking::::new_session(0); let mut offenders = vec![]; @@ -167,21 +167,50 @@ fn make_offenders( .expect("failed to convert validator id to full identification") }) .collect::>>(); - Ok((id_tuples, offenders)) + Ok(id_tuples) } -benchmarks! { - where_clause { - where +#[cfg(test)] +fn assert_all_slashes_applied(offender_count: usize) +where + T: Config, + ::RuntimeEvent: TryInto>, + ::RuntimeEvent: TryInto>, + ::RuntimeEvent: TryInto, + ::RuntimeEvent: TryInto>, +{ + // make sure that all slashes have been applied + // (n nominators + one validator) * (slashed + unlocked) + deposit to reporter + + // reporter account endowed + some funds rescinded from issuance. + assert_eq!( + System::::read_events_for_pallet::>().len(), + 2 * (offender_count + 1) + 3 + ); + // (n nominators + one validator) * slashed + Slash Reported + assert_eq!( + System::::read_events_for_pallet::>().len(), + 1 * (offender_count + 1) + 1 + ); + // offence + assert_eq!(System::::read_events_for_pallet::().len(), 1); + // reporter new account + assert_eq!(System::::read_events_for_pallet::>().len(), 1); +} + +#[benchmarks( + where ::RuntimeEvent: TryInto>, ::RuntimeEvent: TryInto>, ::RuntimeEvent: TryInto, ::RuntimeEvent: TryInto>, - } - - report_offence_grandpa { - let n in 0 .. MAX_NOMINATORS.min(MaxNominationsOf::::get()); - +)] +mod benchmarks { + use super::*; + + #[benchmark] + pub fn report_offence_grandpa( + n: Linear<0, { MAX_NOMINATORS.min(MaxNominationsOf::::get()) }>, + ) -> Result<(), BenchmarkError> { // for grandpa equivocation reports the number of reporters // and offenders is always 1 let reporters = vec![account("reporter", 1, SEED)]; @@ -189,7 +218,7 @@ benchmarks! { // make sure reporters actually get rewarded Staking::::set_slash_reward_fraction(Perbill::one()); - let (mut offenders, raw_offenders) = make_offenders::(1, n)?; + let mut offenders = make_offenders::(1, n)?; let validator_set_count = Session::::validators().len() as u32; let offence = GrandpaEquivocationOffence { @@ -199,28 +228,24 @@ benchmarks! { offender: T::convert(offenders.pop().unwrap()), }; assert_eq!(System::::event_count(), 0); - }: { - let _ = Offences::::report_offence(reporters, offence); - } - verify { + + #[block] + { + let _ = Offences::::report_offence(reporters, offence); + } + #[cfg(test)] { - // make sure that all slashes have been applied - // (n nominators + one validator) * (slashed + unlocked) + deposit to reporter + reporter - // account endowed + some funds rescinded from issuance. - assert_eq!(System::::read_events_for_pallet::>().len(), 2 * (n + 1) as usize + 3); - // (n nominators + one validator) * slashed + Slash Reported - assert_eq!(System::::read_events_for_pallet::>().len(), 1 * (n + 1) as usize + 1); - // offence - assert_eq!(System::::read_events_for_pallet::().len(), 1); - // reporter new account - assert_eq!(System::::read_events_for_pallet::>().len(), 1); + assert_all_slashes_applied::(n as usize); } - } - report_offence_babe { - let n in 0 .. MAX_NOMINATORS.min(MaxNominationsOf::::get()); + Ok(()) + } + #[benchmark] + fn report_offence_babe( + n: Linear<0, { MAX_NOMINATORS.min(MaxNominationsOf::::get()) }>, + ) -> Result<(), BenchmarkError> { // for babe equivocation reports the number of reporters // and offenders is always 1 let reporters = vec![account("reporter", 1, SEED)]; @@ -228,7 +253,7 @@ benchmarks! { // make sure reporters actually get rewarded Staking::::set_slash_reward_fraction(Perbill::one()); - let (mut offenders, raw_offenders) = make_offenders::(1, n)?; + let mut offenders = make_offenders::(1, n)?; let validator_set_count = Session::::validators().len() as u32; let offence = BabeEquivocationOffence { @@ -238,23 +263,17 @@ benchmarks! { offender: T::convert(offenders.pop().unwrap()), }; assert_eq!(System::::event_count(), 0); - }: { - let _ = Offences::::report_offence(reporters, offence); - } - verify { + + #[block] + { + let _ = Offences::::report_offence(reporters, offence); + } #[cfg(test)] { - // make sure that all slashes have been applied - // (n nominators + one validator) * (slashed + unlocked) + deposit to reporter + reporter - // account endowed + some funds rescinded from issuance. - assert_eq!(System::::read_events_for_pallet::>().len(), 2 * (n + 1) as usize + 3); - // (n nominators + one validator) * slashed + Slash Reported - assert_eq!(System::::read_events_for_pallet::>().len(), 1 * (n + 1) as usize + 1); - // offence - assert_eq!(System::::read_events_for_pallet::().len(), 1); - // reporter new account - assert_eq!(System::::read_events_for_pallet::>().len(), 1); + assert_all_slashes_applied::(n as usize); } + + Ok(()) } impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test); diff --git a/substrate/frame/offences/benchmarking/src/mock.rs b/substrate/frame/offences/benchmarking/src/mock.rs index efaec49a65b3..c5c178aa4443 100644 --- a/substrate/frame/offences/benchmarking/src/mock.rs +++ b/substrate/frame/offences/benchmarking/src/mock.rs @@ -29,7 +29,7 @@ use frame_system as system; use pallet_session::historical as pallet_session_historical; use sp_runtime::{ testing::{Header, UintAuthorityId}, - BuildStorage, Perbill, + BuildStorage, KeyTypeId, Perbill, }; type AccountId = u64; @@ -66,7 +66,8 @@ sp_runtime::impl_opaque_keys! { pub struct TestSessionHandler; impl pallet_session::SessionHandler 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])]; fn on_genesis_session(_validators: &[(AccountId, Ks)]) {}