From 43123e7312e16cf16d158c71a8c11e741dd5d501 Mon Sep 17 00:00:00 2001 From: JesseAbram <33698952+JesseAbram@users.noreply.github.com> Date: Thu, 22 Aug 2024 10:53:30 -0400 Subject: [PATCH] Add benchmark for `new_session` hook (#1016) * Benchmark for new session * add new_session_not_adding_new_signer * new_session weight * lint * Suggestions for `new_session` benches (#1019) * Format benchmarking code * Update `*_less_then_signers` bench to scale signers in storage Since we end up reading from this storage vec we want to make sure it is populated with up to `MAX_SIGNERS` for the purposes of benchmarking. * Change benchmarking for session handler to only use two benches We add an extra storage read each time, but we can simplify the benchmarking a bit * Always update session weight If the signer length was more than the total signers we wouldn't update the session weight, even though we would still do a few storage operations. * Remove `expect` by using `match` statement * Remove unused `new_session_not_adding_new_signer` bench * Update the name for one of the benches * Update benchmark results * Add missing parameter to base bench --------- Co-authored-by: Hernando Castano --- pallets/staking/src/benchmarking.rs | 69 +++++++++++ pallets/staking/src/lib.rs | 42 +++++-- pallets/staking/src/weights.rs | 88 ++++++++++++++ .../src/weights/pallet_staking_extension.rs | 115 ++++++++++++------ 4 files changed, 272 insertions(+), 42 deletions(-) diff --git a/pallets/staking/src/benchmarking.rs b/pallets/staking/src/benchmarking.rs index b79c6c029..5fce1157b 100644 --- a/pallets/staking/src/benchmarking.rs +++ b/pallets/staking/src/benchmarking.rs @@ -23,6 +23,7 @@ use frame_support::{ traits::{Currency, Get}, }; use frame_system::{EventRecord, RawOrigin}; +use pallet_parameters::{SignersInfo, SignersSize}; use pallet_staking::{Pallet as FrameStaking, RewardDestination, ValidatorPrefs}; use sp_std::{vec, vec::Vec}; @@ -226,6 +227,74 @@ benchmarks! { verify { assert_last_event::(Event::::SignersRotation(signers.clone()).into()); } + + new_session_base_weight { + let s in 2 .. MAX_SIGNERS as u32; + + let caller: T::AccountId = whitelisted_caller(); + + // For the purpose of the bench these values don't actually matter, we just care that there's a + // storage entry available + SignersInfo::::put(SignersSize { + total_signers: MAX_SIGNERS, + threshold: 3, + last_session_change: 0, + }); + + let validator_id = ::ValidatorId::try_from(caller.clone()) + .or(Err(Error::::InvalidValidatorId)) + .unwrap(); + + let signers = vec![validator_id.clone(); s as usize]; + Signers::::put(signers); + }: { + // Note that here we only add one validator, where as `Signers` already contains two as a + // minimum. + let _ = Staking::::new_session_handler(&vec![validator_id]); + } + verify { + assert!(NextSigners::::get().is_none()); + } + + new_session { + let c in 1 .. MAX_SIGNERS as u32 - 1; + let l in 0 .. MAX_SIGNERS as u32; + + let caller: T::AccountId = whitelisted_caller(); + + // For the purpose of the bench these values don't actually matter, we just care that there's a + // storage entry available + SignersInfo::::put(SignersSize { + total_signers: MAX_SIGNERS, + threshold: 3, + last_session_change: 0, + }); + + let validator_id = ::ValidatorId::try_from(caller.clone()) + .or(Err(Error::::InvalidValidatorId)) + .unwrap(); + + let second_signer: T::AccountId = account("second_signer", 0, SEED); + let second_signer_id = + ::ValidatorId::try_from(second_signer.clone()) + .or(Err(Error::::InvalidValidatorId)) + .unwrap(); + + // full signer list leaving room for one extra validator + let mut signers = vec![second_signer_id.clone(); c as usize]; + + Signers::::put(signers.clone()); + signers.push(second_signer_id.clone()); + + // place new signer in the signers struct in different locations to calculate random selection + // re-run + signers[l as usize % c as usize] = validator_id.clone(); + }: { + let _ = Staking::::new_session_handler(&signers); + } + verify { + assert!(NextSigners::::get().is_some()); + } } impl_benchmark_test_suite!(Staking, crate::mock::new_test_ext(), crate::mock::Test); diff --git a/pallets/staking/src/lib.rs b/pallets/staking/src/lib.rs index 38d357f1f..76af74d3b 100644 --- a/pallets/staking/src/lib.rs +++ b/pallets/staking/src/lib.rs @@ -538,28 +538,36 @@ pub mod pallet { pub fn new_session_handler( validators: &[::ValidatorId], - ) -> Result<(), DispatchError> { + ) -> Result { let mut current_signers = Self::signers(); let current_signers_length = current_signers.len(); + let signers_info = pallet_parameters::Pallet::::signers_info(); + + let mut weight: Weight = + ::WeightInfo::new_session_base_weight(current_signers_length as u32); + // Since not enough validators do not allow rotation // TODO: https://github.com/entropyxyz/entropy-core/issues/943 if validators.len() <= current_signers_length { - return Ok(()); + return Ok(weight); } - let signers_info = pallet_parameters::Pallet::::signers_info(); let mut new_signer = vec![]; + let mut count = 0u32; if current_signers_length <= signers_info.total_signers as usize { let mut randomness = Self::get_randomness(); // grab a current signer to initiate value let mut next_signer_up = ¤t_signers[0].clone(); let mut index; + // loops to find signer in validator that is not already signer while current_signers.contains(next_signer_up) { index = randomness.next_u32() % validators.len() as u32; next_signer_up = &validators[index as usize]; + count += 1; } + current_signers.push(next_signer_up.clone()); new_signer = next_signer_up.encode(); } @@ -570,20 +578,25 @@ pub mod pallet { } NextSigners::::put(NextSignerInfo { - next_signers: current_signers, + next_signers: current_signers.clone(), confirmations: vec![], }); + // trigger reshare at next block let current_block_number = >::block_number(); let reshare_info = ReshareInfo { block_number: current_block_number + sp_runtime::traits::One::one(), new_signer, }; + ReshareData::::put(reshare_info); JumpStartProgress::::mutate(|jump_start_details| { jump_start_details.parent_key_threshold = signers_info.threshold }); - Ok(()) + + weight = ::WeightInfo::new_session(current_signers.len() as u32, count); + + Ok(weight) } } @@ -600,9 +613,22 @@ pub mod pallet { fn new_session(new_index: SessionIndex) -> Option> { let new_session = I::new_session(new_index); if let Some(validators) = &new_session { - let result = Pallet::::new_session_handler(validators); - if result.is_err() { - log::warn!("Error splitting validators, Session: {:?}", new_index) + let weight = Pallet::::new_session_handler(validators); + + match weight { + Ok(weight) => { + frame_system::Pallet::::register_extra_weight_unchecked( + weight, + DispatchClass::Mandatory, + ); + }, + Err(why) => { + log::warn!( + "Error splitting validators, Session: {:?}, reason: {:?}", + new_index, + why + ) + }, } } new_session diff --git a/pallets/staking/src/weights.rs b/pallets/staking/src/weights.rs index a78e552f0..25be1c558 100644 --- a/pallets/staking/src/weights.rs +++ b/pallets/staking/src/weights.rs @@ -59,6 +59,8 @@ pub trait WeightInfo { fn declare_synced() -> Weight; fn confirm_key_reshare_confirmed(c: u32) -> Weight; fn confirm_key_reshare_completed() -> Weight; + fn new_session_base_weight(s: u32) -> Weight; + fn new_session(c: u32, l: u32) -> Weight; } /// Weights for pallet_staking_extension using the Substrate node and recommended hardware. @@ -190,6 +192,49 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(2)) } + /// Storage: `StakingExtension::Signers` (r:1 w:0) + /// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn new_session_base_weight(s: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `254 + s * (32 ±0)` + // Estimated: `1739 + s * (32 ±0)` + // Minimum execution time: 7_000_000 picoseconds. + Weight::from_parts(7_682_879, 0) + .saturating_add(Weight::from_parts(0, 1739)) + .saturating_add(T::DbWeight::get().reads(2)) + .saturating_add(Weight::from_parts(0, 32).saturating_mul(s.into())) + } + /// Storage: `StakingExtension::Signers` (r:1 w:0) + /// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `Parameters::SignersInfo` (r:1 w:0) + /// Proof: `Parameters::SignersInfo` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `Babe::NextRandomness` (r:1 w:0) + /// Proof: `Babe::NextRandomness` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) + /// Storage: `Babe::EpochStart` (r:1 w:0) + /// Proof: `Babe::EpochStart` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`) + /// Storage: `StakingExtension::JumpStartProgress` (r:1 w:1) + /// Proof: `StakingExtension::JumpStartProgress` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `StakingExtension::ReshareData` (r:0 w:1) + /// Proof: `StakingExtension::ReshareData` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `StakingExtension::NextSigners` (r:0 w:1) + /// Proof: `StakingExtension::NextSigners` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// The range of component `c` is `[1, 14]`. + /// The range of component `l` is `[0, 15]`. + fn new_session(c: u32, l: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `482 + c * (32 ±0)` + // Estimated: `1966 + c * (32 ±0)` + // Minimum execution time: 13_000_000 picoseconds. + Weight::from_parts(12_791_889, 0) + .saturating_add(Weight::from_parts(0, 1966)) + // Standard Error: 22_917 + .saturating_add(Weight::from_parts(65_067, 0).saturating_mul(c.into())) + // Standard Error: 19_636 + .saturating_add(Weight::from_parts(30_071, 0).saturating_mul(l.into())) + .saturating_add(T::DbWeight::get().reads(5)) + .saturating_add(T::DbWeight::get().writes(3)) + .saturating_add(Weight::from_parts(0, 32).saturating_mul(c.into())) + } } // For backwards compatibility and tests @@ -320,4 +365,47 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(2)) .saturating_add(RocksDbWeight::get().writes(2)) } + /// Storage: `StakingExtension::Signers` (r:1 w:0) + /// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn new_session_base_weight(s: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `254 + s * (32 ±0)` + // Estimated: `1739 + s * (32 ±0)` + // Minimum execution time: 7_000_000 picoseconds. + Weight::from_parts(7_682_879, 0) + .saturating_add(Weight::from_parts(0, 1739)) + .saturating_add(RocksDbWeight::get().reads(2)) + .saturating_add(Weight::from_parts(0, 32).saturating_mul(s.into())) + } + /// Storage: `StakingExtension::Signers` (r:1 w:0) + /// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `Parameters::SignersInfo` (r:1 w:0) + /// Proof: `Parameters::SignersInfo` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `Babe::NextRandomness` (r:1 w:0) + /// Proof: `Babe::NextRandomness` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) + /// Storage: `Babe::EpochStart` (r:1 w:0) + /// Proof: `Babe::EpochStart` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`) + /// Storage: `StakingExtension::JumpStartProgress` (r:1 w:1) + /// Proof: `StakingExtension::JumpStartProgress` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `StakingExtension::ReshareData` (r:0 w:1) + /// Proof: `StakingExtension::ReshareData` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `StakingExtension::NextSigners` (r:0 w:1) + /// Proof: `StakingExtension::NextSigners` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// The range of component `c` is `[1, 14]`. + /// The range of component `l` is `[0, 15]`. + fn new_session(c: u32, l: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `482 + c * (32 ±0)` + // Estimated: `1966 + c * (32 ±0)` + // Minimum execution time: 13_000_000 picoseconds. + Weight::from_parts(12_791_889, 0) + .saturating_add(Weight::from_parts(0, 1966)) + // Standard Error: 22_917 + .saturating_add(Weight::from_parts(65_067, 0).saturating_mul(c.into())) + // Standard Error: 19_636 + .saturating_add(Weight::from_parts(30_071, 0).saturating_mul(l.into())) + .saturating_add(RocksDbWeight::get().reads(5)) + .saturating_add(RocksDbWeight::get().writes(3)) + .saturating_add(Weight::from_parts(0, 32).saturating_mul(c.into())) + } } diff --git a/runtime/src/weights/pallet_staking_extension.rs b/runtime/src/weights/pallet_staking_extension.rs index 87698051d..f3232611b 100644 --- a/runtime/src/weights/pallet_staking_extension.rs +++ b/runtime/src/weights/pallet_staking_extension.rs @@ -16,9 +16,9 @@ //! Autogenerated weights for `pallet_staking_extension` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 33.0.0 -//! DATE: 2024-07-26, STEPS: `5`, REPEAT: `2`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-08-21, STEPS: `5`, REPEAT: `2`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `Jesses-MacBook-Pro.local`, CPU: `` +//! HOSTNAME: `hcastano`, CPU: `` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: 1024 // Executed Command: @@ -53,11 +53,11 @@ impl pallet_staking_extension::WeightInfo for WeightInf /// Proof: `StakingExtension::ThresholdServers` (`max_values`: None, `max_size`: None, mode: `Measured`) fn change_endpoint() -> Weight { // Proof Size summary in bytes: - // Measured: `1342` - // Estimated: `4807` - // Minimum execution time: 25_000_000 picoseconds. - Weight::from_parts(29_000_000, 0) - .saturating_add(Weight::from_parts(0, 4807)) + // Measured: `1309` + // Estimated: `4774` + // Minimum execution time: 28_000_000 picoseconds. + Weight::from_parts(30_000_000, 0) + .saturating_add(Weight::from_parts(0, 4774)) .saturating_add(T::DbWeight::get().reads(3)) .saturating_add(T::DbWeight::get().writes(1)) } @@ -71,11 +71,11 @@ impl pallet_staking_extension::WeightInfo for WeightInf /// Proof: `StakingExtension::ThresholdServers` (`max_values`: None, `max_size`: None, mode: `Measured`) fn change_threshold_accounts() -> Weight { // Proof Size summary in bytes: - // Measured: `1463` - // Estimated: `4928` - // Minimum execution time: 28_000_000 picoseconds. - Weight::from_parts(29_000_000, 0) - .saturating_add(Weight::from_parts(0, 4928)) + // Measured: `1430` + // Estimated: `4895` + // Minimum execution time: 31_000_000 picoseconds. + Weight::from_parts(32_000_000, 0) + .saturating_add(Weight::from_parts(0, 4895)) .saturating_add(T::DbWeight::get().reads(4)) .saturating_add(T::DbWeight::get().writes(2)) } @@ -95,8 +95,8 @@ impl pallet_staking_extension::WeightInfo for WeightInf // Proof Size summary in bytes: // Measured: `1262` // Estimated: `4764` - // Minimum execution time: 42_000_000 picoseconds. - Weight::from_parts(43_000_000, 0) + // Minimum execution time: 45_000_000 picoseconds. + Weight::from_parts(48_000_000, 0) .saturating_add(Weight::from_parts(0, 4764)) .saturating_add(T::DbWeight::get().reads(6)) .saturating_add(T::DbWeight::get().writes(3)) @@ -129,10 +129,10 @@ impl pallet_staking_extension::WeightInfo for WeightInf /// Proof: `StakingExtension::ThresholdServers` (`max_values`: None, `max_size`: None, mode: `Measured`) fn validate() -> Weight { // Proof Size summary in bytes: - // Measured: `1951` + // Measured: `1918` // Estimated: `6248` - // Minimum execution time: 64_000_000 picoseconds. - Weight::from_parts(70_000_000, 0) + // Minimum execution time: 76_000_000 picoseconds. + Weight::from_parts(184_000_000, 0) .saturating_add(Weight::from_parts(0, 6248)) .saturating_add(T::DbWeight::get().reads(13)) .saturating_add(T::DbWeight::get().writes(8)) @@ -143,11 +143,11 @@ impl pallet_staking_extension::WeightInfo for WeightInf /// Proof: `StakingExtension::IsValidatorSynced` (`max_values`: None, `max_size`: None, mode: `Measured`) fn declare_synced() -> Weight { // Proof Size summary in bytes: - // Measured: `353` - // Estimated: `3818` - // Minimum execution time: 10_000_000 picoseconds. - Weight::from_parts(11_000_000, 0) - .saturating_add(Weight::from_parts(0, 3818)) + // Measured: `320` + // Estimated: `3785` + // Minimum execution time: 11_000_000 picoseconds. + Weight::from_parts(13_000_000, 0) + .saturating_add(Weight::from_parts(0, 3785)) .saturating_add(T::DbWeight::get().reads(1)) .saturating_add(T::DbWeight::get().writes(1)) } @@ -155,16 +155,19 @@ impl pallet_staking_extension::WeightInfo for WeightInf /// Proof: `StakingExtension::ThresholdToStash` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `StakingExtension::NextSigners` (r:1 w:1) /// Proof: `StakingExtension::NextSigners` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - /// The range of component `c` is `[0, 2]`. - fn confirm_key_reshare_confirmed(_c: u32, ) -> Weight { + /// The range of component `c` is `[0, 15]`. + fn confirm_key_reshare_confirmed(c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `445` - // Estimated: `3910` - // Minimum execution time: 11_000_000 picoseconds. - Weight::from_parts(11_916_666, 0) - .saturating_add(Weight::from_parts(0, 3910)) + // Measured: `797 + c * (32 ±0)` + // Estimated: `4298 + c * (29 ±1)` + // Minimum execution time: 13_000_000 picoseconds. + Weight::from_parts(14_248_618, 0) + .saturating_add(Weight::from_parts(0, 4298)) + // Standard Error: 122_082 + .saturating_add(Weight::from_parts(90_469, 0).saturating_mul(c.into())) .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(1)) + .saturating_add(Weight::from_parts(0, 29).saturating_mul(c.into())) } /// Storage: `StakingExtension::ThresholdToStash` (r:1 w:0) /// Proof: `StakingExtension::ThresholdToStash` (`max_values`: None, `max_size`: None, mode: `Measured`) @@ -174,12 +177,56 @@ impl pallet_staking_extension::WeightInfo for WeightInf /// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) fn confirm_key_reshare_completed() -> Weight { // Proof Size summary in bytes: - // Measured: `477` - // Estimated: `3942` - // Minimum execution time: 12_000_000 picoseconds. - Weight::from_parts(13_000_000, 0) - .saturating_add(Weight::from_parts(0, 3942)) + // Measured: `1309` + // Estimated: `4774` + // Minimum execution time: 14_000_000 picoseconds. + Weight::from_parts(14_000_000, 0) + .saturating_add(Weight::from_parts(0, 4774)) .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(2)) } + /// Storage: `StakingExtension::Signers` (r:1 w:0) + /// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `Parameters::SignersInfo` (r:1 w:0) + /// Proof: `Parameters::SignersInfo` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// The range of component `s` is `[2, 15]`. + fn new_session_base_weight(s: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `254 + s * (32 ±0)` + // Estimated: `1739 + s * (32 ±0)` + // Minimum execution time: 7_000_000 picoseconds. + Weight::from_parts(7_682_879, 0) + .saturating_add(Weight::from_parts(0, 1739)) + .saturating_add(T::DbWeight::get().reads(2)) + .saturating_add(Weight::from_parts(0, 32).saturating_mul(s.into())) + } + /// Storage: `StakingExtension::Signers` (r:1 w:0) + /// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `Parameters::SignersInfo` (r:1 w:0) + /// Proof: `Parameters::SignersInfo` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `Babe::NextRandomness` (r:1 w:0) + /// Proof: `Babe::NextRandomness` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) + /// Storage: `Babe::EpochStart` (r:1 w:0) + /// Proof: `Babe::EpochStart` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`) + /// Storage: `StakingExtension::JumpStartProgress` (r:1 w:1) + /// Proof: `StakingExtension::JumpStartProgress` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `StakingExtension::ReshareData` (r:0 w:1) + /// Proof: `StakingExtension::ReshareData` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `StakingExtension::NextSigners` (r:0 w:1) + /// Proof: `StakingExtension::NextSigners` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// The range of component `c` is `[1, 14]`. + /// The range of component `l` is `[0, 15]`. + fn new_session(c: u32, _l: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `482 + c * (32 ±0)` + // Estimated: `1966 + c * (32 ±0)` + // Minimum execution time: 15_000_000 picoseconds. + Weight::from_parts(16_252_259, 0) + .saturating_add(Weight::from_parts(0, 1966)) + // Standard Error: 101_760 + .saturating_add(Weight::from_parts(74_486, 0).saturating_mul(c.into())) + .saturating_add(T::DbWeight::get().reads(5)) + .saturating_add(T::DbWeight::get().writes(3)) + .saturating_add(Weight::from_parts(0, 32).saturating_mul(c.into())) + } }