diff --git a/bin/node/runtime/voter-bags/src/main.rs b/bin/node/runtime/voter-bags/src/main.rs index 1340285c29a1a..a92af37fb5bf8 100644 --- a/bin/node/runtime/voter-bags/src/main.rs +++ b/bin/node/runtime/voter-bags/src/main.rs @@ -17,7 +17,7 @@ //! Make the set of voting bag thresholds to be used in `voter_bags.rs`. -use pallet_staking::{voter_bags::make_bags::generate_thresholds_module}; +use pallet_staking::voter_bags::make_bags::generate_thresholds_module; use std::path::PathBuf; use structopt::StructOpt; @@ -34,7 +34,5 @@ struct Opt { fn main() -> Result<(), std::io::Error> { let Opt { n_bags, output } = Opt::from_args(); let mut ext = sp_io::TestExternalities::new_empty(); - ext.execute_with(|| { - generate_thresholds_module::(n_bags, &output) - }) + ext.execute_with(|| generate_thresholds_module::(n_bags, &output)) } diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 905492d6ca04c..3841817d04f8b 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -632,6 +632,15 @@ pub mod pallet { #[pallet::constant] type SignedDepositWeight: Get>; + /// The number of snapshot voters to fetch per block. + /// + /// In the future, this value will make more sense with multi-block snapshot. + /// + /// Also, note the data type: If the voters are represented by a `u32` in `type + /// CompactSolution`, the same `u32` is used here to ensure bounds are respected. + #[pallet::constant] + type VoterSnapshotPerBlock: Get>; + /// Handler for the slashed deposits. type SlashHandler: OnUnbalanced>; @@ -1285,8 +1294,11 @@ impl Pallet { /// /// Returns `Ok(consumed_weight)` if operation is okay. pub fn create_snapshot() -> Result { + // we don't impose any limits on the targets for now, the assumption is that + // `T::DataProvider` will sensibly return small values to use. let target_limit = >::max_value().saturated_into::(); - let voter_limit = >::max_value().saturated_into::(); + // for now we have just a single block snapshot. + let voter_limit = T::VoterSnapshotPerBlock::get().saturated_into::(); let (targets, w1) = T::DataProvider::targets(Some(target_limit)).map_err(ElectionError::DataProvider)?; @@ -1970,8 +1982,8 @@ mod tests { }) } - #[test] - fn snapshot_creation_fails_if_too_big() { + fn snapshot_too_big_failure_onchain_fallback() { + // the `MockStaking` is designed such that if it has too many targets, it simply fails. ExtBuilder::default().build_and_execute(|| { Targets::set((0..(TargetIndex::max_value() as AccountId) + 1).collect::>()); @@ -1987,6 +1999,49 @@ mod tests { roll_to(29); let (supports, _) = MultiPhase::elect().unwrap(); assert!(supports.len() > 0); + }); + } + + #[test] + fn snapshot_too_big_failure_no_fallback() { + // and if the backup mode is nothing, we go into the emergency mode.. + ExtBuilder::default().fallback(FallbackStrategy::Nothing).build_and_execute(|| { + crate::mock::Targets::set( + (0..(TargetIndex::max_value() as AccountId) + 1).collect::>(), + ); + + // Signed phase failed to open. + roll_to(15); + assert_eq!(MultiPhase::current_phase(), Phase::Off); + + // Unsigned phase failed to open. + roll_to(25); + assert_eq!(MultiPhase::current_phase(), Phase::Off); + + roll_to(29); + let err = MultiPhase::elect().unwrap_err(); + assert_eq!(err, ElectionError::NoFallbackConfigured); + assert_eq!(MultiPhase::current_phase(), Phase::Emergency); + }); + } + + #[test] + fn snapshot_too_big_truncate() { + // but if there are too many voters, we simply truncate them. + ExtBuilder::default().build_and_execute(|| { + // we have 8 voters in total. + assert_eq!(crate::mock::Voters::get().len(), 8); + // but we want to take 4. + crate::mock::VoterSnapshotPerBlock::set(2); + + // Signed phase opens just fine. + roll_to(15); + assert_eq!(MultiPhase::current_phase(), Phase::Signed); + + assert_eq!( + MultiPhase::snapshot_metadata().unwrap(), + SolutionOrSnapshotSize { voters: 2, targets: 4 } + ); }) } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index c5007733c1e33..8e6424844f1e4 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -270,6 +270,7 @@ parameter_types! { pub static MinerMaxWeight: Weight = BlockWeights::get().max_block; pub static MinerMaxLength: u32 = 256; pub static MockWeightInfo: bool = false; + pub static VoterSnapshotPerBlock: VoterIndex = u32::max_value(); pub static EpochLength: u64 = 30; } @@ -379,6 +380,7 @@ impl crate::Config for Runtime { type Fallback = Fallback; type ForceOrigin = frame_system::EnsureRoot; type CompactSolution = TestCompact; + type VoterSnapshotPerBlock = VoterSnapshotPerBlock; } impl frame_system::offchain::SendTransactionTypes for Runtime @@ -410,9 +412,9 @@ impl ElectionDataProvider for StakingMock { fn voters( maybe_max_len: Option, ) -> data_provider::Result<(Vec<(AccountId, VoteWeight, Vec)>, Weight)> { - let voters = Voters::get(); - if maybe_max_len.map_or(false, |max_len| voters.len() > max_len) { - return Err("Voters too big") + let mut voters = Voters::get(); + if let Some(max_len) = maybe_max_len { + voters.truncate(max_len) } Ok((voters, 0)) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 4ac2b7404d0f3..3574c34602bbb 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -273,10 +273,10 @@ #![cfg_attr(not(feature = "std"), no_std)] -#[cfg(any(feature = "runtime-benchmarks", test))] -pub mod testing_utils; #[cfg(any(feature = "runtime-benchmarks", test))] pub mod benchmarking; +#[cfg(any(feature = "runtime-benchmarks", test))] +pub mod testing_utils; #[cfg(test)] pub(crate) mod mock; @@ -3111,12 +3111,6 @@ impl Pallet { maybe_max_len: Option, voter_count: usize, ) -> Vec> { - debug_assert_eq!( - voter_count, - VoterList::::decode_len().unwrap_or_default(), - "voter_count must be accurate", - ); - let wanted_voters = maybe_max_len.unwrap_or(voter_count).min(voter_count); let weight_of = Self::weight_of_fn(); @@ -3240,8 +3234,15 @@ impl let nominator_count = CounterForNominators::::get(); let validator_count = CounterForValidators::::get(); let voter_count = nominator_count.saturating_add(validator_count) as usize; + + // check a few counters one last time... debug_assert!(>::iter().count() as u32 == CounterForNominators::::get()); debug_assert!(>::iter().count() as u32 == CounterForValidators::::get()); + debug_assert_eq!( + voter_count, + VoterList::::decode_len().unwrap_or_default(), + "voter_count must be accurate", + ); let slashing_span_count = >::iter().count(); let weight = T::WeightInfo::get_npos_voters( @@ -3249,6 +3250,7 @@ impl validator_count, slashing_span_count as u32, ); + Ok((Self::get_npos_voters(maybe_max_len, voter_count), weight)) } diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index cec490cdbfb67..4ae38611492e5 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4006,9 +4006,21 @@ mod election_data_provider { } #[test] - fn respects_len_limits() { - ExtBuilder::default().build_and_execute(|| { + fn respects_snapshot_len_limits() { + ExtBuilder::default().validator_pool(true).build_and_execute(|| { + // sum of all validators and nominators who'd be voters. + assert_eq!(VoterList::::decode_len().unwrap(), 5); + + // if limits is less.. assert_eq!(Staking::voters(Some(1)).unwrap().0.len(), 1); + + // if limit is equal.. + assert_eq!(Staking::voters(Some(5)).unwrap().0.len(), 5); + + // if limit is more. + assert_eq!(Staking::voters(Some(55)).unwrap().0.len(), 5); + + // if target limit is less, then we return an error. assert_eq!(Staking::targets(Some(1)).unwrap_err(), "Target snapshot too big"); }); } @@ -4068,12 +4080,22 @@ mod election_data_provider { #[test] #[should_panic] - fn count_check_works() { + fn count_check_prevents_validator_insert() { ExtBuilder::default().build_and_execute(|| { // We should never insert into the validators or nominators map directly as this will // not keep track of the count. This test should panic as we verify the count is accurate - // after every test using the `post_checks` in `mock`. + // after every test using the `post_conditions` in `mock`. Validators::::insert(987654321, ValidatorPrefs::default()); + }) + } + + #[test] + #[should_panic] + fn count_check_prevents_nominator_insert() { + ExtBuilder::default().build_and_execute(|| { + // We should never insert into the validators or nominators map directly as this will + // not keep track of the count. This test should panic as we verify the count is accurate + // after every test using the `post_conditions` in `mock`. Nominators::::insert( 987654321, Nominations { diff --git a/frame/staking/src/voter_bags.rs b/frame/staking/src/voter_bags.rs index d5bae70b29f46..1927c4c6c049c 100644 --- a/frame/staking/src/voter_bags.rs +++ b/frame/staking/src/voter_bags.rs @@ -620,15 +620,13 @@ impl Node { weight_of: impl Fn(&T::AccountId) -> VoteWeight, slashing_spans: &BTreeMap, SlashingSpans>, ) -> Option> { + let voter_weight = weight_of(&self.voter.id); match self.voter.voter_type { - VoterType::Validator => Some(( - self.voter.id.clone(), - weight_of(&self.voter.id), - sp_std::vec![self.voter.id.clone()], - )), + VoterType::Validator => + Some((self.voter.id.clone(), voter_weight, sp_std::vec![self.voter.id.clone()])), VoterType::Nominator => { let Nominations { submitted_in, mut targets, .. } = - Nominators::::get(self.voter.id.clone())?; + Nominators::::get(&self.voter.id)?; // Filter out nomination targets which were nominated before the most recent // slashing span. targets.retain(|stash| { @@ -637,8 +635,7 @@ impl Node { .map_or(true, |spans| submitted_in >= spans.last_nonzero_slash()) }); - (!targets.is_empty()) - .then(move || (self.voter.id.clone(), weight_of(&self.voter.id), targets)) + (!targets.is_empty()).then(move || (self.voter.id.clone(), voter_weight, targets)) }, } } diff --git a/primitives/arithmetic/src/traits.rs b/primitives/arithmetic/src/traits.rs index a441a0dcbc08d..53341117b1fee 100644 --- a/primitives/arithmetic/src/traits.rs +++ b/primitives/arithmetic/src/traits.rs @@ -20,8 +20,8 @@ use codec::HasCompact; pub use integer_sqrt::IntegerSquareRoot; pub use num_traits::{ - checked_pow, Bounded, CheckedAdd, CheckedDiv, CheckedMul, CheckedNeg, CheckedShl, CheckedShr, - CheckedSub, One, Signed, Unsigned, Zero, + checked_pow, Bounded, CheckedAdd, CheckedDiv, CheckedMul, CheckedNeg, CheckedRem, CheckedShl, + CheckedShr, CheckedSub, One, Signed, Unsigned, Zero, }; use sp_std::{ self, @@ -55,6 +55,7 @@ pub trait BaseArithmetic: + CheckedSub + CheckedMul + CheckedDiv + + CheckedRem + Saturating + PartialOrd + Ord @@ -109,6 +110,7 @@ impl< + CheckedSub + CheckedMul + CheckedDiv + + CheckedRem + Saturating + PartialOrd + Ord diff --git a/primitives/npos-elections/src/lib.rs b/primitives/npos-elections/src/lib.rs index ad2b3229a0881..ff4919876aff1 100644 --- a/primitives/npos-elections/src/lib.rs +++ b/primitives/npos-elections/src/lib.rs @@ -155,7 +155,8 @@ where + Debug + Copy + Clone - + Bounded; + + Bounded + + Encode; /// The target type. Needs to be an index (convert to usize). type Target: UniqueSaturatedInto @@ -164,7 +165,8 @@ where + Debug + Copy + Clone - + Bounded; + + Bounded + + Encode; /// The weight/accuracy type of each vote. type Accuracy: PerThing128;