From 5d05169e9d46d0683d3a0ad372621ad67d990e1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Fri, 11 Oct 2024 01:39:04 +0100 Subject: [PATCH 01/10] Refactor staking pallet benchmarks to `v2` --- substrate/frame/staking/src/benchmarking.rs | 493 +++++++++++++------- 1 file changed, 337 insertions(+), 156 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index a25085a18036..85b4bff24fd2 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -17,6 +17,8 @@ //! Staking pallet benchmarking. +#![cfg(feature = "runtime-benchmarks")] + use super::*; use crate::{asset, ConfigOp, Pallet as Staking}; use testing_utils::*; @@ -34,8 +36,9 @@ use sp_runtime::{ }; use sp_staking::{currency_to_vote::CurrencyToVote, SessionIndex}; -pub use frame_benchmarking::v1::{ - account, benchmarks, impl_benchmark_test_suite, whitelist_account, whitelisted_caller, +pub use frame_benchmarking::{ + BenchmarkError, + impl_benchmark_test_suite, whitelist_account, whitelisted_caller, v2::* }; use frame_system::RawOrigin; @@ -219,19 +222,26 @@ impl ListScenario { const USER_SEED: u32 = 999666; -benchmarks! { - bond { +#[benchmarks] +mod benchmarks { + use super::*; + + #[benchmark] + fn bond() { let stash = create_funded_user::("stash", USER_SEED, 100); let reward_destination = RewardDestination::Staked; let amount = asset::existential_deposit::() * 10u32.into(); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash.clone()), amount, reward_destination) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(stash.clone()), amount, reward_destination); + assert!(Bonded::::contains_key(stash.clone())); assert!(Ledger::::contains_key(stash)); } - bond_extra { + #[benchmark] + fn bond_extra() -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); @@ -252,14 +262,19 @@ benchmarks! { let _ = asset::mint_existing::(&stash, max_additional).unwrap(); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash), max_additional) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(stash), max_additional); + let ledger = Ledger::::get(&controller).ok_or("ledger not created after")?; let new_bonded: BalanceOf = ledger.active; assert!(original_bonded < new_bonded); + + Ok(()) } - unbond { + #[benchmark] + fn unbond() -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); @@ -279,17 +294,23 @@ benchmarks! { let original_bonded: BalanceOf = ledger.active; whitelist_account!(controller); - }: _(RawOrigin::Signed(controller.clone()), amount) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(controller.clone()), amount); + let ledger = Ledger::::get(&controller).ok_or("ledger not created after")?; let new_bonded: BalanceOf = ledger.active; assert!(original_bonded > new_bonded); + + Ok(()) } + #[benchmark] // Withdraw only updates the ledger - withdraw_unbonded_update { + fn withdraw_unbonded_update( // Slashing Spans - let s in 0 .. MAX_SPANS; + s: Linear<0, MAX_SPANS>, + ) -> Result<(), BenchmarkError> { let (stash, controller) = create_stash_controller::(0, 100, RewardDestination::Staked)?; add_slashing_spans::(&stash, s); let amount = asset::existential_deposit::() * 5u32.into(); // Half of total @@ -298,17 +319,23 @@ benchmarks! { let ledger = Ledger::::get(&controller).ok_or("ledger not created before")?; let original_total: BalanceOf = ledger.total; whitelist_account!(controller); - }: withdraw_unbonded(RawOrigin::Signed(controller.clone()), s) - verify { + + #[extrinsic_call] + withdraw_unbonded(RawOrigin::Signed(controller.clone()), s); + let ledger = Ledger::::get(&controller).ok_or("ledger not created after")?; let new_total: BalanceOf = ledger.total; assert!(original_total > new_total); + + Ok(()) } + #[benchmark] // Worst case scenario, everything is removed after the bonding duration - withdraw_unbonded_kill { + fn withdraw_unbonded_kill( // Slashing Spans - let s in 0 .. MAX_SPANS; + s: Linear<0, MAX_SPANS> + ) -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); @@ -329,13 +356,18 @@ benchmarks! { CurrentEra::::put(EraIndex::max_value()); whitelist_account!(controller); - }: withdraw_unbonded(RawOrigin::Signed(controller.clone()), s) - verify { + + #[extrinsic_call] + withdraw_unbonded(RawOrigin::Signed(controller.clone()), s); + assert!(!Ledger::::contains_key(controller)); assert!(!T::VoterList::contains(&stash)); + + Ok(()) } - validate { + #[benchmark] + fn validate() -> Result<(), BenchmarkError> { let (stash, controller) = create_stash_controller::( MaxNominationsOf::::get() - 1, 100, @@ -346,19 +378,24 @@ benchmarks! { let prefs = ValidatorPrefs::default(); whitelist_account!(controller); - }: _(RawOrigin::Signed(controller), prefs) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(controller), prefs); + assert!(Validators::::contains_key(&stash)); assert!(T::VoterList::contains(&stash)); + + Ok(()) } - kick { + #[benchmark] + fn kick( // scenario: we want to kick `k` nominators from nominating us (we are a validator). // we'll assume that `k` is under 128 for the purposes of determining the slope. // each nominator should have `T::MaxNominations::get()` validators nominated, and our validator // should be somewhere in there. - let k in 1 .. 128; - + k: Linear<1, 128>, + )-> Result<(), BenchmarkError> { // these are the other validators; there are `T::MaxNominations::get() - 1` of them, so // there are a total of `T::MaxNominations::get()` validators in the system. let rest_of_validators = create_validators_with_seed::(MaxNominationsOf::::get() - 1, 100, 415)?; @@ -407,18 +444,23 @@ benchmarks! { .collect::>(); whitelist_account!(controller); - }: _(RawOrigin::Signed(controller), kicks) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(controller), kicks); + // all nominators now should *not* be nominating our validator... for n in nominator_stashes.iter() { assert!(!Nominators::::get(n).unwrap().targets.contains(&stash)); } + + Ok(()) } + #[benchmark] // Worst case scenario, T::MaxNominations::get() - nominate { - let n in 1 .. MaxNominationsOf::::get(); - + fn nominate( + n: Linear<1, { MaxNominationsOf::::get() } > + ) -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); @@ -438,13 +480,18 @@ benchmarks! { let validators = create_validators::(n, 100).unwrap(); whitelist_account!(controller); - }: _(RawOrigin::Signed(controller), validators) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(controller), validators); + assert!(Nominators::::contains_key(&stash)); - assert!(T::VoterList::contains(&stash)) + assert!(T::VoterList::contains(&stash)); + + Ok(()) } - chill { + #[benchmark] + fn chill() -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); @@ -458,33 +505,48 @@ benchmarks! { assert!(T::VoterList::contains(&stash)); whitelist_account!(controller); - }: _(RawOrigin::Signed(controller)) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(controller)); + assert!(!T::VoterList::contains(&stash)); + + Ok(()) } - set_payee { + #[benchmark] + fn set_payee() -> Result<(), BenchmarkError> { let (stash, controller) = create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; assert_eq!(Payee::::get(&stash), Some(RewardDestination::Staked)); whitelist_account!(controller); - }: _(RawOrigin::Signed(controller.clone()), RewardDestination::Account(controller.clone())) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(controller.clone()), RewardDestination::Account(controller.clone())); + assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(controller))); + + Ok(()) } - update_payee { + #[benchmark] + fn update_payee() -> Result<(), BenchmarkError> { let (stash, controller) = create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; Payee::::insert(&stash, { #[allow(deprecated)] RewardDestination::Controller }); whitelist_account!(controller); - }: _(RawOrigin::Signed(controller.clone()), controller.clone()) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(controller.clone()), controller.clone()); + assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(controller))); + + Ok(()) } - set_controller { + #[benchmark] + fn set_controller() -> Result<(), BenchmarkError> { let (stash, ctlr) = create_unique_stash_controller::(9000, 100, RewardDestination::Staked, false)?; // ensure `ctlr` is the currently stored controller. assert!(!Ledger::::contains_key(&stash)); @@ -492,49 +554,76 @@ benchmarks! { assert_eq!(Bonded::::get(&stash), Some(ctlr.clone())); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash.clone())) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(stash.clone())); + assert!(Ledger::::contains_key(&stash)); + + Ok(()) } - set_validator_count { + #[benchmark] + fn set_validator_count() { let validator_count = MaxValidators::::get(); - }: _(RawOrigin::Root, validator_count) - verify { + + #[extrinsic_call] + _(RawOrigin::Root, validator_count); + assert_eq!(ValidatorCount::::get(), validator_count); } - force_no_eras {}: _(RawOrigin::Root) - verify { assert_eq!(ForceEra::::get(), Forcing::ForceNone); } + #[benchmark] + fn force_no_eras() { + #[extrinsic_call] + _(RawOrigin::Root); + + assert_eq!(ForceEra::::get(), Forcing::ForceNone); + } - force_new_era {}: _(RawOrigin::Root) - verify { assert_eq!(ForceEra::::get(), Forcing::ForceNew); } + #[benchmark] + fn force_new_era() { + #[extrinsic_call] + _(RawOrigin::Root); - force_new_era_always {}: _(RawOrigin::Root) - verify { assert_eq!(ForceEra::::get(), Forcing::ForceAlways); } + assert_eq!(ForceEra::::get(), Forcing::ForceNew); + } + + #[benchmark] + fn force_new_era_always() { + #[extrinsic_call] + _(RawOrigin::Root); + + assert_eq!(ForceEra::::get(), Forcing::ForceAlways); + } + #[benchmark] // Worst case scenario, the list of invulnerables is very long. - set_invulnerables { - let v in 0 .. MaxValidators::::get(); + fn set_invulnerables( + v: Linear<0, { MaxValidators::::get() } >, + ) { let mut invulnerables = Vec::new(); for i in 0 .. v { invulnerables.push(account("invulnerable", i, SEED)); } - }: _(RawOrigin::Root, invulnerables) - verify { + + #[extrinsic_call] + _(RawOrigin::Root, invulnerables); + assert_eq!(Invulnerables::::get().len(), v as usize); } - deprecate_controller_batch { + #[benchmark] + fn deprecate_controller_batch( // We pass a dynamic number of controllers to the benchmark, up to // `MaxControllersInDeprecationBatch`. - let i in 0 .. T::MaxControllersInDeprecationBatch::get(); - + u: Linear<0, { T::MaxControllersInDeprecationBatch::get() } >, + ) -> Result<(), BenchmarkError> { let mut controllers: Vec<_> = vec![]; let mut stashes: Vec<_> = vec![]; - for n in 0..i as u32 { + for i in 0..u as u32 { let (stash, controller) = create_unique_stash_controller::( - n, + i, 100, RewardDestination::Staked, false @@ -544,11 +633,13 @@ benchmarks! { } let bounded_controllers: BoundedVec<_, T::MaxControllersInDeprecationBatch> = BoundedVec::try_from(controllers.clone()).unwrap(); - }: _(RawOrigin::Root, bounded_controllers) - verify { - for n in 0..i as u32 { - let stash = &stashes[n as usize]; - let controller = &controllers[n as usize]; + + #[extrinsic_call] + _(RawOrigin::Root, bounded_controllers); + + for i in 0..u as u32 { + let stash = &stashes[i as usize]; + let controller = &controllers[i as usize]; // Ledger no longer keyed by controller. assert_eq!(Ledger::::get(controller), None); // Bonded now maps to the stash. @@ -556,11 +647,15 @@ benchmarks! { // Ledger is now keyed by stash. assert_eq!(Ledger::::get(stash).unwrap().stash, *stash); } + + Ok(()) } - force_unstake { + #[benchmark] + fn force_unstake( // Slashing Spans - let s in 0 .. MAX_SPANS; + s: Linear<0, MAX_SPANS>, + ) -> Result<(), BenchmarkError> { // Clean up any existing state. clear_validators_and_nominators::(); @@ -574,14 +669,19 @@ benchmarks! { assert!(T::VoterList::contains(&stash)); add_slashing_spans::(&stash, s); - }: _(RawOrigin::Root, stash.clone(), s) - verify { + #[extrinsic_call] + _(RawOrigin::Root, stash.clone(), s); + assert!(!Ledger::::contains_key(&controller)); assert!(!T::VoterList::contains(&stash)); + + Ok(()) } - cancel_deferred_slash { - let s in 1 .. MAX_SLASHES; + #[benchmark] + fn cancel_deferred_slash( + s: Linear<1, MAX_SLASHES>, + ) { let mut unapplied_slashes = Vec::new(); let era = EraIndex::one(); let dummy = || T::AccountId::decode(&mut TrailingZeroInput::zeroes()).unwrap(); @@ -591,13 +691,17 @@ benchmarks! { UnappliedSlashes::::insert(era, &unapplied_slashes); let slash_indices: Vec = (0 .. s).collect(); - }: _(RawOrigin::Root, era, slash_indices) - verify { + + #[extrinsic_call] + _(RawOrigin::Root, era, slash_indices); + assert_eq!(UnappliedSlashes::::get(&era).len(), (MAX_SLASHES - s) as usize); } - payout_stakers_alive_staked { - let n in 0 .. T::MaxExposurePageSize::get() as u32; + #[benchmark] + fn payout_stakers_alive_staked( + n: Linear<0, { T::MaxExposurePageSize::get() as u32 } >, + ) -> Result<(), BenchmarkError> { let (validator, nominators) = create_validator_with_nominators::( n, T::MaxExposurePageSize::get() as u32, @@ -617,8 +721,10 @@ benchmarks! { let balance = asset::stakeable_balance::(stash); nominator_balances_before.push(balance); } - }: payout_stakers(RawOrigin::Signed(caller), validator.clone(), current_era) - verify { + + #[extrinsic_call] + payout_stakers(RawOrigin::Signed(caller), validator.clone(), current_era); + let balance_after = asset::stakeable_balance::(&validator); ensure!( balance_before < balance_after, @@ -631,11 +737,14 @@ benchmarks! { "Balance of nominator stash should have increased after payout.", ); } - } - rebond { - let l in 1 .. T::MaxUnlockingChunks::get() as u32; + Ok(()) + } + #[benchmark] + fn rebond( + l: Linear<1, { T::MaxUnlockingChunks::get() as u32 }>, + ) -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); @@ -674,15 +783,21 @@ benchmarks! { let original_bonded: BalanceOf = staking_ledger.active; whitelist_account!(controller); - }: _(RawOrigin::Signed(controller.clone()), rebond_amount) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(controller.clone()), rebond_amount); + let ledger = Ledger::::get(&controller).ok_or("ledger not created after")?; let new_bonded: BalanceOf = ledger.active; assert!(original_bonded < new_bonded); + + Ok(()) } - reap_stash { - let s in 1 .. MAX_SPANS; + #[benchmark] + fn reap_stash( + s: Linear<1, MAX_SPANS>, + ) -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); @@ -705,16 +820,21 @@ benchmarks! { assert!(T::VoterList::contains(&stash)); whitelist_account!(controller); - }: _(RawOrigin::Signed(controller), stash.clone(), s) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(controller), stash.clone(), s); + assert!(!Bonded::::contains_key(&stash)); assert!(!T::VoterList::contains(&stash)); - } - new_era { - let v in 1 .. 10; - let n in 0 .. 100; + Ok(()) + } + #[benchmark] + fn new_era( + v: Linear<1, 10>, + n: Linear<0, 100>, + ) -> Result<(), BenchmarkError> { create_validators_with_nominators_for_era::( v, n, @@ -723,16 +843,24 @@ benchmarks! { None, )?; let session_index = SessionIndex::one(); - }: { - let validators = Staking::::try_trigger_new_era(session_index, true) + + let validators; + #[block] + { + validators = Staking::::try_trigger_new_era(session_index, true) .ok_or("`new_era` failed")?; + } + assert!(validators.len() == v as usize); + + Ok(()) } - #[extra] - payout_all { - let v in 1 .. 10; - let n in 0 .. 100; + #[benchmark(extra)] + fn payout_all( + v: Linear<1, 10>, + n: Linear<0, 100>, + ) -> Result<(), BenchmarkError> { create_validators_with_nominators_for_era::( v, n, @@ -772,17 +900,23 @@ benchmarks! { let calls: Vec<_> = payout_calls_arg.iter().map(|arg| Call::::payout_stakers_by_page { validator_stash: arg.0.clone(), era: arg.1, page: 0 }.encode() ).collect(); - }: { - for call in calls { - as Decode>::decode(&mut &*call) - .expect("call is encoded above, encoding must be correct") - .dispatch_bypass_filter(origin.clone().into())?; + + #[block] + { + for call in calls { + as Decode>::decode(&mut &*call) + .expect("call is encoded above, encoding must be correct") + .dispatch_bypass_filter(origin.clone().into())?; + } } + + Ok(()) } - #[extra] - do_slash { - let l in 1 .. T::MaxUnlockingChunks::get() as u32; + #[benchmark(extra)] + fn do_slash( + l: Linear<1, { T::MaxUnlockingChunks::get() as u32 }>, + ) -> Result<(), BenchmarkError> { let (stash, controller) = create_stash_controller::(0, 100, RewardDestination::Staked)?; let mut staking_ledger = Ledger::::get(controller.clone()).unwrap(); let unlock_chunk = UnlockChunk::> { @@ -795,7 +929,8 @@ benchmarks! { Ledger::::insert(controller, staking_ledger); let slash_amount = asset::existential_deposit::() * 10u32.into(); let balance_before = asset::stakeable_balance::(&stash); - }: { + + #[extrinsic_call] crate::slashing::do_slash::( &stash, slash_amount, @@ -803,17 +938,21 @@ benchmarks! { &mut NegativeImbalanceOf::::zero(), EraIndex::zero() ); - } verify { + let balance_after = asset::stakeable_balance::(&stash); assert!(balance_before > balance_after); + + Ok(()) } - get_npos_voters { + #[benchmark] + fn get_npos_voters( // number of validator intention. we will iterate all of them. - let v in (MaxValidators::::get() / 2) .. MaxValidators::::get(); - // number of nominator intention. we will iterate all of them. - let n in (MaxNominators::::get() / 2) .. MaxNominators::::get(); + v: Linear< { MaxValidators::::get() / 2 }, { MaxValidators::::get() } >, + // number of nominator intention. we will iterate all of them. + n: Linear< { MaxNominators::::get() / 2 }, { MaxNominators::::get() } >, + ) -> Result<(), BenchmarkError> { let validators = create_validators_with_nominators_for_era::( v, n, MaxNominationsOf::::get() as usize, false, None )? @@ -825,38 +964,57 @@ benchmarks! { assert_eq!(Nominators::::count(), n); let num_voters = (v + n) as usize; - }: { + // default bounds are unbounded. - let voters = >::get_npos_voters(DataProviderBounds::default()); + let voters; + #[block] + { + voters = >::get_npos_voters(DataProviderBounds::default()); + } + assert_eq!(voters.len(), num_voters); + + Ok(()) } - get_npos_targets { + #[benchmark] + fn get_npos_targets( // number of validator intention. - let v in (MaxValidators::::get() / 2) .. MaxValidators::::get(); + v: Linear< { MaxValidators::::get() / 2 }, { MaxValidators::::get() } >, + ) -> Result<(), BenchmarkError> { // number of nominator intention. let n = MaxNominators::::get(); - let _ = create_validators_with_nominators_for_era::( - v, n, MaxNominationsOf::::get() as usize, false, None - )?; - }: { + #[block] + { + let _ = create_validators_with_nominators_for_era::( + v, n, MaxNominationsOf::::get() as usize, false, None + )?; + } + // default bounds are unbounded. let targets = >::get_npos_targets(DataProviderBounds::default()); assert_eq!(targets.len() as u32, v); + + Ok(()) } - set_staking_configs_all_set { - }: set_staking_configs( - RawOrigin::Root, - ConfigOp::Set(BalanceOf::::max_value()), - ConfigOp::Set(BalanceOf::::max_value()), - ConfigOp::Set(u32::MAX), - ConfigOp::Set(u32::MAX), - ConfigOp::Set(Percent::max_value()), - ConfigOp::Set(Perbill::max_value()), - ConfigOp::Set(Percent::max_value()) - ) verify { + #[benchmark] + fn set_staking_configs_all_set() + { + #[block] { + set_staking_configs( + RawOrigin::Root, + ConfigOp::Set(BalanceOf::::max_value()), + ConfigOp::Set(BalanceOf::::max_value()), + ConfigOp::Set(u32::MAX), + ConfigOp::Set(u32::MAX), + ConfigOp::Set(Percent::max_value()), + ConfigOp::Set(Perbill::max_value()), + ConfigOp::Set(Percent::max_value()) + ); + } + assert_eq!(MinNominatorBond::::get(), BalanceOf::::max_value()); assert_eq!(MinValidatorBond::::get(), BalanceOf::::max_value()); assert_eq!(MaxNominatorsCount::::get(), Some(u32::MAX)); @@ -866,17 +1024,22 @@ benchmarks! { assert_eq!(MaxStakedRewards::::get(), Some(Percent::from_percent(100))); } - set_staking_configs_all_remove { - }: set_staking_configs( - RawOrigin::Root, - ConfigOp::Remove, - ConfigOp::Remove, - ConfigOp::Remove, - ConfigOp::Remove, - ConfigOp::Remove, - ConfigOp::Remove, - ConfigOp::Remove - ) verify { + #[benchmark] + fn set_staking_configs_all_remove() { + + #[block] { + set_staking_configs( + RawOrigin::Root, + ConfigOp::Remove, + ConfigOp::Remove, + ConfigOp::Remove, + ConfigOp::Remove, + ConfigOp::Remove, + ConfigOp::Remove, + ConfigOp::Remove + ); + } + assert!(!MinNominatorBond::::exists()); assert!(!MinValidatorBond::::exists()); assert!(!MaxNominatorsCount::::exists()); @@ -886,7 +1049,8 @@ benchmarks! { assert!(!MaxStakedRewards::::exists()); } - chill_other { + #[benchmark] + fn chill_other() -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); @@ -911,12 +1075,17 @@ benchmarks! { )?; let caller = whitelisted_caller(); - }: _(RawOrigin::Signed(caller), stash.clone()) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller), stash.clone()); + assert!(!T::VoterList::contains(&stash)); + + Ok(()) } - force_apply_min_commission { + #[benchmark] + fn force_apply_min_commission() -> Result<(), BenchmarkError> { // Clean up any existing state clear_validators_and_nominators::(); @@ -936,29 +1105,41 @@ benchmarks! { // Set the min commission to 75% MinCommission::::set(Perbill::from_percent(75)); let caller = whitelisted_caller(); - }: _(RawOrigin::Signed(caller), stash.clone()) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller), stash.clone()); + // The validators commission has been bumped to 75% assert_eq!( Validators::::get(&stash), ValidatorPrefs { commission: Perbill::from_percent(75), ..Default::default() } ); + + Ok(()) } - set_min_commission { + #[benchmark] + fn set_min_commission() { let min_commission = Perbill::max_value(); - }: _(RawOrigin::Root, min_commission) - verify { + + #[extrinsic_call] + _(RawOrigin::Root, min_commission); + assert_eq!(MinCommission::::get(), Perbill::from_percent(100)); } - restore_ledger { + #[benchmark] + fn restore_ledger() -> Result<(), BenchmarkError> { let (stash, controller) = create_stash_controller::(0, 100, RewardDestination::Staked)?; // corrupt ledger. Ledger::::remove(controller); - }: _(RawOrigin::Root, stash.clone(), None, None, None) - verify { + + #[extrinsic_call] + _(RawOrigin::Root, stash.clone(), None, None, None); + assert_eq!(Staking::::inspect_bond_state(&stash), Ok(LedgerIntegrityState::Ok)); + + Ok(()) } impl_benchmark_test_suite!( From a9cca7ce1ae7b3e4c70c3ec5ed4adf420506be68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Fri, 11 Oct 2024 14:52:07 +0100 Subject: [PATCH 02/10] Correct mixups --- substrate/frame/staking/src/benchmarking.rs | 44 ++++++++++----------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 85b4bff24fd2..d6f6d48d1259 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -930,14 +930,16 @@ mod benchmarks { let slash_amount = asset::existential_deposit::() * 10u32.into(); let balance_before = asset::stakeable_balance::(&stash); - #[extrinsic_call] - crate::slashing::do_slash::( - &stash, - slash_amount, - &mut BalanceOf::::zero(), - &mut NegativeImbalanceOf::::zero(), - EraIndex::zero() - ); + #[block] + { + crate::slashing::do_slash::( + &stash, + slash_amount, + &mut BalanceOf::::zero(), + &mut NegativeImbalanceOf::::zero(), + EraIndex::zero() + ); + } let balance_after = asset::stakeable_balance::(&stash); assert!(balance_before > balance_after); @@ -1002,18 +1004,17 @@ mod benchmarks { #[benchmark] fn set_staking_configs_all_set() { - #[block] { - set_staking_configs( - RawOrigin::Root, - ConfigOp::Set(BalanceOf::::max_value()), - ConfigOp::Set(BalanceOf::::max_value()), - ConfigOp::Set(u32::MAX), - ConfigOp::Set(u32::MAX), - ConfigOp::Set(Percent::max_value()), - ConfigOp::Set(Perbill::max_value()), - ConfigOp::Set(Percent::max_value()) - ); - } + #[extrinsic_call] + set_staking_configs( + RawOrigin::Root, + ConfigOp::Set(BalanceOf::::max_value()), + ConfigOp::Set(BalanceOf::::max_value()), + ConfigOp::Set(u32::MAX), + ConfigOp::Set(u32::MAX), + ConfigOp::Set(Percent::max_value()), + ConfigOp::Set(Perbill::max_value()), + ConfigOp::Set(Percent::max_value()) + ); assert_eq!(MinNominatorBond::::get(), BalanceOf::::max_value()); assert_eq!(MinValidatorBond::::get(), BalanceOf::::max_value()); @@ -1027,7 +1028,7 @@ mod benchmarks { #[benchmark] fn set_staking_configs_all_remove() { - #[block] { + #[extrinsic_call] set_staking_configs( RawOrigin::Root, ConfigOp::Remove, @@ -1038,7 +1039,6 @@ mod benchmarks { ConfigOp::Remove, ConfigOp::Remove ); - } assert!(!MinNominatorBond::::exists()); assert!(!MinValidatorBond::::exists()); From 7d2ab0777086446406b9f422aac23dd45ea82ce9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Fri, 11 Oct 2024 19:26:20 +0100 Subject: [PATCH 03/10] Remove test using `SelectedBenchmarks` from `v1` --- substrate/frame/staking/src/benchmarking.rs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index d6f6d48d1259..85ddc611fdc4 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -1246,25 +1246,4 @@ mod tests { } }); } - - #[test] - fn test_payout_all() { - ExtBuilder::default().build_and_execute(|| { - let v = 10; - let n = 100; - - let selected_benchmark = SelectedBenchmark::payout_all; - let c = vec![ - (frame_benchmarking::BenchmarkParameter::v, v), - (frame_benchmarking::BenchmarkParameter::n, n), - ]; - - assert_ok!( - >::unit_test_instance( - &selected_benchmark, - &c, - ) - ); - }); - } } From d563045659694f4ae00db36f49edd53398128a15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Sun, 13 Oct 2024 13:40:32 +0100 Subject: [PATCH 04/10] Underscore unused variables in benchmarks --- substrate/frame/staking/src/benchmarking.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 85ddc611fdc4..7de1a3a52344 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -279,7 +279,7 @@ mod benchmarks { clear_validators_and_nominators::(); // setup the worst case list scenario. - let total_issuance = asset::total_issuance::(); + let _total_issuance = asset::total_issuance::(); // the weight the nominator will start at. The value used here is expected to be // significantly higher than the first position in a list (e.g. the first bag threshold). let origin_weight = BalanceOf::::try_from(952_994_955_240_703u128) @@ -287,7 +287,7 @@ mod benchmarks { .unwrap(); let scenario = ListScenario::::new(origin_weight, false)?; - let stash = scenario.origin_stash1.clone(); + let _stash = scenario.origin_stash1.clone(); let controller = scenario.origin_controller1.clone(); let amount = origin_weight - scenario.dest_weight; let ledger = Ledger::::get(&controller).ok_or("ledger not created before")?; @@ -468,7 +468,7 @@ mod benchmarks { // setup a worst case list scenario. Note we don't care about the destination position, because // we are just doing an insert into the origin position. - let scenario = ListScenario::::new(origin_weight, true)?; + let _scenario = ListScenario::::new(origin_weight, true)?; let (stash, controller) = create_stash_controller_with_balance::( SEED + MaxNominationsOf::::get() + 1, // make sure the account does not conflict with others origin_weight, @@ -772,7 +772,7 @@ mod benchmarks { era: EraIndex::zero(), }; - let stash = scenario.origin_stash1.clone(); + let _stash = scenario.origin_stash1.clone(); let controller = scenario.origin_controller1; let mut staking_ledger = Ledger::::get(controller.clone()).unwrap(); @@ -955,7 +955,7 @@ mod benchmarks { // number of nominator intention. we will iterate all of them. n: Linear< { MaxNominators::::get() / 2 }, { MaxNominators::::get() } >, ) -> Result<(), BenchmarkError> { - let validators = create_validators_with_nominators_for_era::( + let _validators = create_validators_with_nominators_for_era::( v, n, MaxNominationsOf::::get() as usize, false, None )? .into_iter() @@ -1059,7 +1059,7 @@ mod benchmarks { // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. let scenario = ListScenario::::new(origin_weight, true)?; - let controller = scenario.origin_controller1.clone(); + let _controller = scenario.origin_controller1.clone(); let stash = scenario.origin_stash1; assert!(T::VoterList::contains(&stash)); From ca7a39fcbafff428e1a12a8aa3c52fe29ab2dce8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Sun, 13 Oct 2024 13:51:06 +0100 Subject: [PATCH 05/10] Run `cargo fmt` on benchmarking module --- substrate/frame/staking/src/benchmarking.rs | 179 ++++++++++---------- 1 file changed, 89 insertions(+), 90 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 7de1a3a52344..d0465ad39cc2 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -37,8 +37,7 @@ use sp_runtime::{ use sp_staking::{currency_to_vote::CurrencyToVote, SessionIndex}; pub use frame_benchmarking::{ - BenchmarkError, - impl_benchmark_test_suite, whitelist_account, whitelisted_caller, v2::* + impl_benchmark_test_suite, v2::*, whitelist_account, whitelisted_caller, BenchmarkError, }; use frame_system::RawOrigin; @@ -53,7 +52,7 @@ type MaxNominators = <::BenchmarkingConfig as BenchmarkingConfig // read and write operations. pub fn add_slashing_spans(who: &T::AccountId, spans: u32) { if spans == 0 { - return + return; } // For the first slashing span, we initialize @@ -256,8 +255,9 @@ mod benchmarks { let stash = scenario.origin_stash1.clone(); let controller = scenario.origin_controller1; - let original_bonded: BalanceOf - = Ledger::::get(&controller).map(|l| l.active).ok_or("ledger not created after")?; + let original_bonded: BalanceOf = Ledger::::get(&controller) + .map(|l| l.active) + .ok_or("ledger not created after")?; let _ = asset::mint_existing::(&stash, max_additional).unwrap(); @@ -334,7 +334,7 @@ mod benchmarks { // Worst case scenario, everything is removed after the bonding duration fn withdraw_unbonded_kill( // Slashing Spans - s: Linear<0, MAX_SPANS> + s: Linear<0, MAX_SPANS>, ) -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); @@ -378,7 +378,7 @@ mod benchmarks { let prefs = ValidatorPrefs::default(); whitelist_account!(controller); - + #[extrinsic_call] _(RawOrigin::Signed(controller), prefs); @@ -392,13 +392,14 @@ mod benchmarks { fn kick( // scenario: we want to kick `k` nominators from nominating us (we are a validator). // we'll assume that `k` is under 128 for the purposes of determining the slope. - // each nominator should have `T::MaxNominations::get()` validators nominated, and our validator - // should be somewhere in there. + // each nominator should have `T::MaxNominations::get()` validators nominated, and our + // validator should be somewhere in there. k: Linear<1, 128>, - )-> Result<(), BenchmarkError> { + ) -> Result<(), BenchmarkError> { // these are the other validators; there are `T::MaxNominations::get() - 1` of them, so // there are a total of `T::MaxNominations::get()` validators in the system. - let rest_of_validators = create_validators_with_seed::(MaxNominationsOf::::get() - 1, 100, 415)?; + let rest_of_validators = + create_validators_with_seed::(MaxNominationsOf::::get() - 1, 100, 415)?; // this is the validator that will be kicking. let (stash, controller) = create_stash_controller::( @@ -414,7 +415,7 @@ mod benchmarks { // we now create the nominators. there will be `k` of them; each will nominate all // validators. we will then kick each of the `k` nominators from the main validator. let mut nominator_stashes = Vec::with_capacity(k as usize); - for i in 0 .. k { + for i in 0..k { // create a nominator stash. let (n_stash, n_controller) = create_stash_controller::( MaxNominationsOf::::get() + i, @@ -439,7 +440,8 @@ mod benchmarks { } // we need the unlookuped version of the nominator stash for the kick. - let kicks = nominator_stashes.iter() + let kicks = nominator_stashes + .iter() .map(|n| T::Lookup::unlookup(n.clone())) .collect::>(); @@ -458,22 +460,22 @@ mod benchmarks { #[benchmark] // Worst case scenario, T::MaxNominations::get() - fn nominate( - n: Linear<1, { MaxNominationsOf::::get() } > - ) -> Result<(), BenchmarkError> { + fn nominate(n: Linear<1, { MaxNominationsOf::::get() }>) -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); let origin_weight = MinNominatorBond::::get().max(asset::existential_deposit::()); - // setup a worst case list scenario. Note we don't care about the destination position, because - // we are just doing an insert into the origin position. + // setup a worst case list scenario. Note we don't care about the destination position, + // because we are just doing an insert into the origin position. let _scenario = ListScenario::::new(origin_weight, true)?; let (stash, controller) = create_stash_controller_with_balance::( - SEED + MaxNominationsOf::::get() + 1, // make sure the account does not conflict with others + SEED + MaxNominationsOf::::get() + 1, /* make sure the account does not conflict + * with others */ origin_weight, RewardDestination::Staked, - ).unwrap(); + ) + .unwrap(); assert!(!Nominators::::contains_key(&stash)); assert!(!T::VoterList::contains(&stash)); @@ -516,7 +518,8 @@ mod benchmarks { #[benchmark] fn set_payee() -> Result<(), BenchmarkError> { - let (stash, controller) = create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; + let (stash, controller) = + create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; assert_eq!(Payee::::get(&stash), Some(RewardDestination::Staked)); whitelist_account!(controller); @@ -530,7 +533,8 @@ mod benchmarks { #[benchmark] fn update_payee() -> Result<(), BenchmarkError> { - let (stash, controller) = create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; + let (stash, controller) = + create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; Payee::::insert(&stash, { #[allow(deprecated)] RewardDestination::Controller @@ -547,7 +551,8 @@ mod benchmarks { #[benchmark] fn set_controller() -> Result<(), BenchmarkError> { - let (stash, ctlr) = create_unique_stash_controller::(9000, 100, RewardDestination::Staked, false)?; + let (stash, ctlr) = + create_unique_stash_controller::(9000, 100, RewardDestination::Staked, false)?; // ensure `ctlr` is the currently stored controller. assert!(!Ledger::::contains_key(&stash)); assert!(Ledger::::contains_key(&ctlr)); @@ -599,11 +604,9 @@ mod benchmarks { #[benchmark] // Worst case scenario, the list of invulnerables is very long. - fn set_invulnerables( - v: Linear<0, { MaxValidators::::get() } >, - ) { + fn set_invulnerables(v: Linear<0, { MaxValidators::::get() }>) { let mut invulnerables = Vec::new(); - for i in 0 .. v { + for i in 0..v { invulnerables.push(account("invulnerable", i, SEED)); } @@ -617,17 +620,13 @@ mod benchmarks { fn deprecate_controller_batch( // We pass a dynamic number of controllers to the benchmark, up to // `MaxControllersInDeprecationBatch`. - u: Linear<0, { T::MaxControllersInDeprecationBatch::get() } >, + u: Linear<0, { T::MaxControllersInDeprecationBatch::get() }>, ) -> Result<(), BenchmarkError> { let mut controllers: Vec<_> = vec![]; let mut stashes: Vec<_> = vec![]; for i in 0..u as u32 { - let (stash, controller) = create_unique_stash_controller::( - i, - 100, - RewardDestination::Staked, - false - )?; + let (stash, controller) = + create_unique_stash_controller::(i, 100, RewardDestination::Staked, false)?; controllers.push(controller); stashes.push(stash); } @@ -679,18 +678,17 @@ mod benchmarks { } #[benchmark] - fn cancel_deferred_slash( - s: Linear<1, MAX_SLASHES>, - ) { + fn cancel_deferred_slash(s: Linear<1, MAX_SLASHES>) { let mut unapplied_slashes = Vec::new(); let era = EraIndex::one(); let dummy = || T::AccountId::decode(&mut TrailingZeroInput::zeroes()).unwrap(); - for _ in 0 .. MAX_SLASHES { - unapplied_slashes.push(UnappliedSlash::>::default_from(dummy())); + for _ in 0..MAX_SLASHES { + unapplied_slashes + .push(UnappliedSlash::>::default_from(dummy())); } UnappliedSlashes::::insert(era, &unapplied_slashes); - let slash_indices: Vec = (0 .. s).collect(); + let slash_indices: Vec = (0..s).collect(); #[extrinsic_call] _(RawOrigin::Root, era, slash_indices); @@ -700,7 +698,7 @@ mod benchmarks { #[benchmark] fn payout_stakers_alive_staked( - n: Linear<0, { T::MaxExposurePageSize::get() as u32 } >, + n: Linear<0, { T::MaxExposurePageSize::get() as u32 }>, ) -> Result<(), BenchmarkError> { let (validator, nominators) = create_validator_with_nominators::( n, @@ -712,7 +710,11 @@ mod benchmarks { let current_era = CurrentEra::::get().unwrap(); // set the commission for this particular era as well. - >::insert(current_era, validator.clone(), >::validators(&validator)); + >::insert( + current_era, + validator.clone(), + >::validators(&validator), + ); let caller = whitelisted_caller(); let balance_before = asset::stakeable_balance::(&validator); @@ -730,7 +732,8 @@ mod benchmarks { balance_before < balance_after, "Balance of validator stash should have increased after payout.", ); - for ((stash, _), balance_before) in nominators.iter().zip(nominator_balances_before.iter()) { + for ((stash, _), balance_before) in nominators.iter().zip(nominator_balances_before.iter()) + { let balance_after = asset::stakeable_balance::(stash); ensure!( balance_before < &balance_after, @@ -742,9 +745,7 @@ mod benchmarks { } #[benchmark] - fn rebond( - l: Linear<1, { T::MaxUnlockingChunks::get() as u32 }>, - ) -> Result<(), BenchmarkError> { + fn rebond(l: Linear<1, { T::MaxUnlockingChunks::get() as u32 }>) -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); @@ -767,16 +768,13 @@ mod benchmarks { // so the sum of unlocking chunks puts voter into the dest bag. assert!(value * l.into() + origin_weight > origin_weight); assert!(value * l.into() + origin_weight <= dest_weight); - let unlock_chunk = UnlockChunk::> { - value, - era: EraIndex::zero(), - }; + let unlock_chunk = UnlockChunk::> { value, era: EraIndex::zero() }; let _stash = scenario.origin_stash1.clone(); let controller = scenario.origin_controller1; let mut staking_ledger = Ledger::::get(controller.clone()).unwrap(); - for _ in 0 .. l { + for _ in 0..l { staking_ledger.unlocking.try_push(unlock_chunk.clone()).unwrap() } Ledger::::insert(controller.clone(), staking_ledger.clone()); @@ -795,9 +793,7 @@ mod benchmarks { } #[benchmark] - fn reap_stash( - s: Linear<1, MAX_SPANS>, - ) -> Result<(), BenchmarkError> { + fn reap_stash(s: Linear<1, MAX_SPANS>) -> Result<(), BenchmarkError> { // clean up any existing state. clear_validators_and_nominators::(); @@ -810,10 +806,8 @@ mod benchmarks { let stash = scenario.origin_stash1; add_slashing_spans::(&stash, s); - let l = StakingLedger::::new( - stash.clone(), - asset::existential_deposit::() - One::one(), - ); + let l = + StakingLedger::::new(stash.clone(), asset::existential_deposit::() - One::one()); Ledger::::insert(&controller, l); assert!(Bonded::::contains_key(&stash)); @@ -831,10 +825,7 @@ mod benchmarks { } #[benchmark] - fn new_era( - v: Linear<1, 10>, - n: Linear<0, 100>, - ) -> Result<(), BenchmarkError> { + fn new_era(v: Linear<1, 10>, n: Linear<0, 100>) -> Result<(), BenchmarkError> { create_validators_with_nominators_for_era::( v, n, @@ -847,20 +838,17 @@ mod benchmarks { let validators; #[block] { - validators = Staking::::try_trigger_new_era(session_index, true) - .ok_or("`new_era` failed")?; + validators = + Staking::::try_trigger_new_era(session_index, true).ok_or("`new_era` failed")?; } - + assert!(validators.len() == v as usize); Ok(()) } #[benchmark(extra)] - fn payout_all( - v: Linear<1, 10>, - n: Linear<0, 100>, - ) -> Result<(), BenchmarkError> { + fn payout_all(v: Linear<1, 10>, n: Linear<0, 100>) -> Result<(), BenchmarkError> { create_validators_with_nominators_for_era::( v, n, @@ -897,9 +885,17 @@ mod benchmarks { let caller: T::AccountId = whitelisted_caller(); let origin = RawOrigin::Signed(caller); - let calls: Vec<_> = payout_calls_arg.iter().map(|arg| - Call::::payout_stakers_by_page { validator_stash: arg.0.clone(), era: arg.1, page: 0 }.encode() - ).collect(); + let calls: Vec<_> = payout_calls_arg + .iter() + .map(|arg| { + Call::::payout_stakers_by_page { + validator_stash: arg.0.clone(), + era: arg.1, + page: 0, + } + .encode() + }) + .collect(); #[block] { @@ -919,11 +915,9 @@ mod benchmarks { ) -> Result<(), BenchmarkError> { let (stash, controller) = create_stash_controller::(0, 100, RewardDestination::Staked)?; let mut staking_ledger = Ledger::::get(controller.clone()).unwrap(); - let unlock_chunk = UnlockChunk::> { - value: 1u32.into(), - era: EraIndex::zero(), - }; - for _ in 0 .. l { + let unlock_chunk = + UnlockChunk::> { value: 1u32.into(), era: EraIndex::zero() }; + for _ in 0..l { staking_ledger.unlocking.try_push(unlock_chunk.clone()).unwrap(); } Ledger::::insert(controller, staking_ledger); @@ -937,7 +931,7 @@ mod benchmarks { slash_amount, &mut BalanceOf::::zero(), &mut NegativeImbalanceOf::::zero(), - EraIndex::zero() + EraIndex::zero(), ); } @@ -950,13 +944,17 @@ mod benchmarks { #[benchmark] fn get_npos_voters( // number of validator intention. we will iterate all of them. - v: Linear< { MaxValidators::::get() / 2 }, { MaxValidators::::get() } >, + v: Linear<{ MaxValidators::::get() / 2 }, { MaxValidators::::get() }>, // number of nominator intention. we will iterate all of them. - n: Linear< { MaxNominators::::get() / 2 }, { MaxNominators::::get() } >, + n: Linear<{ MaxNominators::::get() / 2 }, { MaxNominators::::get() }>, ) -> Result<(), BenchmarkError> { let _validators = create_validators_with_nominators_for_era::( - v, n, MaxNominationsOf::::get() as usize, false, None + v, + n, + MaxNominationsOf::::get() as usize, + false, + None, )? .into_iter() .map(|v| T::Lookup::lookup(v).unwrap()) @@ -982,7 +980,7 @@ mod benchmarks { #[benchmark] fn get_npos_targets( // number of validator intention. - v: Linear< { MaxValidators::::get() / 2 }, { MaxValidators::::get() } >, + v: Linear<{ MaxValidators::::get() / 2 }, { MaxValidators::::get() }>, ) -> Result<(), BenchmarkError> { // number of nominator intention. let n = MaxNominators::::get(); @@ -990,7 +988,11 @@ mod benchmarks { #[block] { let _ = create_validators_with_nominators_for_era::( - v, n, MaxNominationsOf::::get() as usize, false, None + v, + n, + MaxNominationsOf::::get() as usize, + false, + None, )?; } @@ -1002,8 +1004,7 @@ mod benchmarks { } #[benchmark] - fn set_staking_configs_all_set() - { + fn set_staking_configs_all_set() { #[extrinsic_call] set_staking_configs( RawOrigin::Root, @@ -1013,7 +1014,7 @@ mod benchmarks { ConfigOp::Set(u32::MAX), ConfigOp::Set(Percent::max_value()), ConfigOp::Set(Perbill::max_value()), - ConfigOp::Set(Percent::max_value()) + ConfigOp::Set(Percent::max_value()), ); assert_eq!(MinNominatorBond::::get(), BalanceOf::::max_value()); @@ -1027,7 +1028,6 @@ mod benchmarks { #[benchmark] fn set_staking_configs_all_remove() { - #[extrinsic_call] set_staking_configs( RawOrigin::Root, @@ -1037,7 +1037,7 @@ mod benchmarks { ConfigOp::Remove, ConfigOp::Remove, ConfigOp::Remove, - ConfigOp::Remove + ConfigOp::Remove, ); assert!(!MinNominatorBond::::exists()); @@ -1090,8 +1090,7 @@ mod benchmarks { clear_validators_and_nominators::(); // Create a validator with a commission of 50% - let (stash, controller) = - create_stash_controller::(1, 1, RewardDestination::Staked)?; + let (stash, controller) = create_stash_controller::(1, 1, RewardDestination::Staked)?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; Staking::::validate(RawOrigin::Signed(controller).into(), validator_prefs)?; From 8020d89239aab549eb5c5287546cda39df3ef6ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Sun, 13 Oct 2024 15:09:38 +0100 Subject: [PATCH 06/10] Add prdoc file for 6025 --- prdoc/pr_6025.prdoc | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 prdoc/pr_6025.prdoc diff --git a/prdoc/pr_6025.prdoc b/prdoc/pr_6025.prdoc new file mode 100644 index 000000000000..84d61ee08bec --- /dev/null +++ b/prdoc/pr_6025.prdoc @@ -0,0 +1,13 @@ +# 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: Refactor staking pallet benchmarks to `v2` + +doc: + - audience: Runtime Dev + description: | + Update benchmarks in staking pallet to the second version of the `frame_benchmarking` runtime benchmarking framework. + +crates: + - name: pallet-staking + bump: none \ No newline at end of file From c3e5a9d7482ac8221213967d4f79dcaa561e2c06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Mon, 14 Oct 2024 11:50:08 +0100 Subject: [PATCH 07/10] Rerun `cargo fmt` with appropriate toolchain --- substrate/frame/staking/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index d0465ad39cc2..3b6b6bf953dc 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -52,7 +52,7 @@ type MaxNominators = <::BenchmarkingConfig as BenchmarkingConfig // read and write operations. pub fn add_slashing_spans(who: &T::AccountId, spans: u32) { if spans == 0 { - return; + return } // For the first slashing span, we initialize From 2606390545b1adf66f3147630dfcc8324870cf67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Mon, 14 Oct 2024 13:50:44 +0100 Subject: [PATCH 08/10] Remove unused code in benchmarks --- substrate/frame/staking/src/benchmarking.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 3b6b6bf953dc..d2c790cfebba 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -278,8 +278,6 @@ mod benchmarks { // clean up any existing state. clear_validators_and_nominators::(); - // setup the worst case list scenario. - let _total_issuance = asset::total_issuance::(); // the weight the nominator will start at. The value used here is expected to be // significantly higher than the first position in a list (e.g. the first bag threshold). let origin_weight = BalanceOf::::try_from(952_994_955_240_703u128) @@ -287,7 +285,6 @@ mod benchmarks { .unwrap(); let scenario = ListScenario::::new(origin_weight, false)?; - let _stash = scenario.origin_stash1.clone(); let controller = scenario.origin_controller1.clone(); let amount = origin_weight - scenario.dest_weight; let ledger = Ledger::::get(&controller).ok_or("ledger not created before")?; @@ -468,7 +465,7 @@ mod benchmarks { // setup a worst case list scenario. Note we don't care about the destination position, // because we are just doing an insert into the origin position. - let _scenario = ListScenario::::new(origin_weight, true)?; + ListScenario::::new(origin_weight, true)?; let (stash, controller) = create_stash_controller_with_balance::( SEED + MaxNominationsOf::::get() + 1, /* make sure the account does not conflict * with others */ @@ -770,7 +767,6 @@ mod benchmarks { assert!(value * l.into() + origin_weight <= dest_weight); let unlock_chunk = UnlockChunk::> { value, era: EraIndex::zero() }; - let _stash = scenario.origin_stash1.clone(); let controller = scenario.origin_controller1; let mut staking_ledger = Ledger::::get(controller.clone()).unwrap(); @@ -949,7 +945,7 @@ mod benchmarks { // number of nominator intention. we will iterate all of them. n: Linear<{ MaxNominators::::get() / 2 }, { MaxNominators::::get() }>, ) -> Result<(), BenchmarkError> { - let _validators = create_validators_with_nominators_for_era::( + let _ = create_validators_with_nominators_for_era::( v, n, MaxNominationsOf::::get() as usize, @@ -1059,7 +1055,6 @@ mod benchmarks { // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. let scenario = ListScenario::::new(origin_weight, true)?; - let _controller = scenario.origin_controller1.clone(); let stash = scenario.origin_stash1; assert!(T::VoterList::contains(&stash)); From e2509a004b9c69fece8d9fb109c4729d9aeaecee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Mon, 14 Oct 2024 14:23:26 +0100 Subject: [PATCH 09/10] Remove unneeded `![cfg]`, fix `prdoc` --- prdoc/pr_6025.prdoc | 2 +- substrate/frame/staking/src/benchmarking.rs | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/prdoc/pr_6025.prdoc b/prdoc/pr_6025.prdoc index 84d61ee08bec..64072c0ae632 100644 --- a/prdoc/pr_6025.prdoc +++ b/prdoc/pr_6025.prdoc @@ -10,4 +10,4 @@ doc: crates: - name: pallet-staking - bump: none \ No newline at end of file + bump: patch \ No newline at end of file diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index d2c790cfebba..5c1b48e7ed84 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -17,8 +17,6 @@ //! Staking pallet benchmarking. -#![cfg(feature = "runtime-benchmarks")] - use super::*; use crate::{asset, ConfigOp, Pallet as Staking}; use testing_utils::*; From 08f9251c65bfd43e6b57b5deaa643086318d4e63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Mon, 14 Oct 2024 14:31:58 +0100 Subject: [PATCH 10/10] Remove `let _` when creating validators in benches --- substrate/frame/staking/src/benchmarking.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 5c1b48e7ed84..96bd3860542f 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -943,16 +943,13 @@ mod benchmarks { // number of nominator intention. we will iterate all of them. n: Linear<{ MaxNominators::::get() / 2 }, { MaxNominators::::get() }>, ) -> Result<(), BenchmarkError> { - let _ = create_validators_with_nominators_for_era::( + create_validators_with_nominators_for_era::( v, n, MaxNominationsOf::::get() as usize, false, None, - )? - .into_iter() - .map(|v| T::Lookup::lookup(v).unwrap()) - .collect::>(); + )?; assert_eq!(Validators::::count(), v); assert_eq!(Nominators::::count(), n); @@ -981,7 +978,7 @@ mod benchmarks { #[block] { - let _ = create_validators_with_nominators_for_era::( + create_validators_with_nominators_for_era::( v, n, MaxNominationsOf::::get() as usize,