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 <[email protected]>

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::<T>(Event::<T>::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::<T>::put(SignersSize {
+        total_signers: MAX_SIGNERS,
+        threshold: 3,
+        last_session_change: 0,
+    });
+
+    let validator_id = <T as pallet_session::Config>::ValidatorId::try_from(caller.clone())
+        .or(Err(Error::<T>::InvalidValidatorId))
+        .unwrap();
+
+    let signers = vec![validator_id.clone(); s as usize];
+    Signers::<T>::put(signers);
+  }:  {
+    // Note that here we only add one validator, where as `Signers` already contains two as a
+    // minimum.
+    let _ = Staking::<T>::new_session_handler(&vec![validator_id]);
+  }
+  verify {
+    assert!(NextSigners::<T>::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::<T>::put(SignersSize {
+        total_signers: MAX_SIGNERS,
+        threshold: 3,
+        last_session_change: 0,
+    });
+
+    let validator_id = <T as pallet_session::Config>::ValidatorId::try_from(caller.clone())
+        .or(Err(Error::<T>::InvalidValidatorId))
+        .unwrap();
+
+    let second_signer: T::AccountId = account("second_signer", 0, SEED);
+    let second_signer_id =
+        <T as pallet_session::Config>::ValidatorId::try_from(second_signer.clone())
+            .or(Err(Error::<T>::InvalidValidatorId))
+            .unwrap();
+
+    // full signer list leaving room for one extra validator
+    let mut signers = vec![second_signer_id.clone(); c as usize];
+
+    Signers::<T>::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::<T>::new_session_handler(&signers);
+  }
+  verify {
+    assert!(NextSigners::<T>::get().is_some());
+  }
 }
 
 impl_benchmark_test_suite!(Staking, crate::mock::new_test_ext(), crate::mock::Test);

pallets/staking/src/lib.rs

@@ -538,28 +538,36 @@ pub mod pallet {
 
         pub fn new_session_handler(
             validators: &[<T as pallet_session::Config>::ValidatorId],
-        ) -> Result<(), DispatchError> {
+        ) -> Result<Weight, DispatchError> {
             let mut current_signers = Self::signers();
             let current_signers_length = current_signers.len();
+            let signers_info = pallet_parameters::Pallet::<T>::signers_info();
+
+            let mut weight: Weight =
+                <T as Config>::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::<T>::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 = &current_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::<T>::put(NextSignerInfo {
-                next_signers: current_signers,
+                next_signers: current_signers.clone(),
                 confirmations: vec![],
             });
+
             // trigger reshare at next block
             let current_block_number = <frame_system::Pallet<T>>::block_number();
             let reshare_info = ReshareInfo {
                 block_number: current_block_number + sp_runtime::traits::One::one(),
                 new_signer,
             };
+
             ReshareData::<T>::put(reshare_info);
             JumpStartProgress::<T>::mutate(|jump_start_details| {
                 jump_start_details.parent_key_threshold = signers_info.threshold
             });
-            Ok(())
+
+            weight = <T as Config>::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<Vec<ValidatorId>> {
             let new_session = I::new_session(new_index);
             if let Some(validators) = &new_session {
-                let result = Pallet::<T>::new_session_handler(validators);
-                if result.is_err() {
-                    log::warn!("Error splitting validators, Session: {:?}", new_index)
+                let weight = Pallet::<T>::new_session_handler(validators);
+
+                match weight {
+                    Ok(weight) => {
+                        frame_system::Pallet::<T>::register_extra_weight_unchecked(
+                            weight,
+                            DispatchClass::Mandatory,
+                        );
+                    },
+                    Err(why) => {
+                        log::warn!(
+                            "Error splitting validators, Session: {:?}, reason: {:?}",
+                            new_index,
+                            why
+                        )
+                    },
                 }
             }
             new_session

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<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 			.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()))
+	}
 }