Skip to content

Commit

Permalink
Suggestions for new_session benches (#1019)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@HCastano
HCastano authored Aug 21, 2024
1 parent 9555684 commit 7b4254b
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 154 deletions.
70 changes: 38 additions & 32 deletions pallets/staking/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,61 +228,67 @@ benchmarks! {
assert_last_event::<T>(Event::<T>::SignersRotation(signers.clone()).into());
}

new_session_validators_less_then_signers {
let caller: T::AccountId = whitelisted_caller();
let validator_id_res = <T as pallet_session::Config>::ValidatorId::try_from(caller.clone()).or(Err(Error::<T>::InvalidValidatorId)).unwrap();
Signers::<T>::put(vec![validator_id_res.clone(), validator_id_res.clone()]);

}: {
let _ = Staking::<T>::new_session_handler(&vec![validator_id_res]);
}
verify {
assert!(NextSigners::<T>::get().is_none());
}
new_session_base_weight {
let s in 2 .. MAX_SIGNERS as u32;

new_session_not_adding_new_signer {
let caller: T::AccountId = whitelisted_caller();
let validator_id_res = <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(); 5];
Signers::<T>::put(signers.clone());
signers.push(validator_id_res.clone());
// 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);
}: {
let _ = Staking::<T>::new_session_handler(&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_eq!(NextSigners::<T>::get().unwrap().next_signers.len(), signers.len() - 2);
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();
let validator_id_res = <T as pallet_session::Config>::ValidatorId::try_from(caller.clone()).or(Err(Error::<T>::InvalidValidatorId)).unwrap();

// 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();
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_res.clone();

SignersInfo::<T>::put(SignersSize {
total_signers: MAX_SIGNERS,
threshold: 3,
last_session_change: 0
});


// 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);
}
Expand Down
48 changes: 30 additions & 18 deletions pallets/staking/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,31 +541,35 @@ pub mod pallet {
) -> 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(<T as Config>::WeightInfo::new_session_validators_less_then_signers());
return Ok(weight);
}

let signers_info = pallet_parameters::Pallet::<T>::signers_info();
let mut new_signer = vec![];
let mut weight: Weight = <T as Config>::WeightInfo::new_session_not_adding_new_signer();
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;
let mut count = 0u32;

// 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();
weight = <T as Config>::WeightInfo::new_session(current_signers.len() as u32, count)
}

// removes first signer and pushes new signer to back if total signers not increased
Expand All @@ -574,20 +578,24 @@ 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
});

weight = <T as Config>::WeightInfo::new_session(current_signers.len() as u32, count);

Ok(weight)
}
}
Expand All @@ -605,18 +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_weight = Pallet::<T>::new_session_handler(validators);
if let Err(why) = result_weight {
log::warn!(
"Error splitting validators, Session: {:?}, reason: {:?}",
new_index,
why
)
} else {
frame_system::Pallet::<T>::register_extra_weight_unchecked(
result_weight.expect("Error unwraping non error value"),
DispatchClass::Mandatory,
);
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
Expand Down
73 changes: 17 additions & 56 deletions pallets/staking/src/weights.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ pub trait WeightInfo {
fn declare_synced() -> Weight;
fn confirm_key_reshare_confirmed(c: u32) -> Weight;
fn confirm_key_reshare_completed() -> Weight;
fn new_session_validators_less_then_signers() -> Weight;
fn new_session_not_adding_new_signer() -> Weight;
fn new_session_base_weight(s: u32) -> Weight;
fn new_session(c: u32, l: u32) -> Weight;
}

Expand Down Expand Up @@ -195,14 +194,15 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
}
/// Storage: `StakingExtension::Signers` (r:1 w:0)
/// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
fn new_session_validators_less_then_signers() -> Weight {
fn new_session_base_weight(s: u32, ) -> Weight {
// Proof Size summary in bytes:
// Measured: `234`
// Estimated: `1719`
// Minimum execution time: 4_000_000 picoseconds.
Weight::from_parts(4_000_000, 0)
.saturating_add(Weight::from_parts(0, 1719))
.saturating_add(T::DbWeight::get().reads(1))
// 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`)
Expand Down Expand Up @@ -235,26 +235,6 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
.saturating_add(T::DbWeight::get().writes(3))
.saturating_add(Weight::from_parts(0, 32).saturating_mul(c.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: `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`)
fn new_session_not_adding_new_signer() -> Weight {
// Proof Size summary in bytes:
// Measured: `439`
// Estimated: `1924`
// Minimum execution time: 9_000_000 picoseconds.
Weight::from_parts(10_000_000, 0)
.saturating_add(Weight::from_parts(0, 1924))
.saturating_add(T::DbWeight::get().reads(3))
.saturating_add(T::DbWeight::get().writes(3))
}
}

// For backwards compatibility and tests
Expand Down Expand Up @@ -387,14 +367,15 @@ impl WeightInfo for () {
}
/// Storage: `StakingExtension::Signers` (r:1 w:0)
/// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
fn new_session_validators_less_then_signers() -> Weight {
fn new_session_base_weight(s: u32, ) -> Weight {
// Proof Size summary in bytes:
// Measured: `234`
// Estimated: `1719`
// Minimum execution time: 4_000_000 picoseconds.
Weight::from_parts(4_000_000, 0)
.saturating_add(Weight::from_parts(0, 1719))
.saturating_add(RocksDbWeight::get().reads(1))
// 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`)
Expand Down Expand Up @@ -427,24 +408,4 @@ impl WeightInfo for () {
.saturating_add(RocksDbWeight::get().writes(3))
.saturating_add(Weight::from_parts(0, 32).saturating_mul(c.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: `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`)
fn new_session_not_adding_new_signer() -> Weight {
// Proof Size summary in bytes:
// Measured: `439`
// Estimated: `1924`
// Minimum execution time: 9_000_000 picoseconds.
Weight::from_parts(10_000_000, 0)
.saturating_add(Weight::from_parts(0, 1924))
.saturating_add(RocksDbWeight::get().reads(3))
.saturating_add(RocksDbWeight::get().writes(3))
}
}
Loading

0 comments on commit 7b4254b

Please sign in to comment.