From 500bc9d3bb71af18f39d1b0532984a61165db174 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 8 Feb 2022 15:25:28 +0000 Subject: [PATCH 01/19] Implement the new validator-in-bags-list scenario + migration --- frame/staking/src/benchmarking.rs | 20 ++-- frame/staking/src/migrations.rs | 35 +++++++ frame/staking/src/mock.rs | 2 +- frame/staking/src/pallet/impls.rs | 161 +++++++++++++++++------------- frame/staking/src/pallet/mod.rs | 2 +- frame/staking/src/tests.rs | 114 +++++++++++---------- 6 files changed, 201 insertions(+), 133 deletions(-) diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index 564172d912413..29fa5e34c7f1e 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -332,24 +332,20 @@ benchmarks! { } validate { - // clean up any existing state. - clear_validators_and_nominators::(); - - let origin_weight = MinNominatorBond::::get().max(T::Currency::minimum_balance()); - - // setup a worst case scenario where the user calling validate was formerly a nominator so - // they must be removed from the list. - let scenario = ListScenario::::new(origin_weight, true)?; - let controller = scenario.origin_controller1.clone(); - let stash = scenario.origin_stash1.clone(); - assert!(T::SortedListProvider::contains(&stash)); + let (stash, controller) = create_stash_controller::( + T::MaxNominations::get() - 1, + 100, + Default::default(), + )?; + // because it is chilled. + assert!(!T::SortedListProvider::contains(&stash)); let prefs = ValidatorPrefs::default(); whitelist_account!(controller); }: _(RawOrigin::Signed(controller), prefs) verify { assert!(Validators::::contains_key(&stash)); - assert!(!T::SortedListProvider::contains(&stash)); + assert!(T::SortedListProvider::contains(&stash)); } kick { diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 3991c4a66076f..cd7f7c633184f 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -17,6 +17,41 @@ //! Storage migrations for the Staking pallet. use super::*; +use frame_election_provider_support::{SortedListProvider, VoteWeightProvider}; +use frame_support::traits::OnRuntimeUpgrade; + +/// Migration implementation that injects all validators into sorted list. +/// +/// This is only useful for chains that started their `SortedListProvider` just based on nominators. +pub struct InjectValidatorsIntoSortedListProvider(sp_std::marker::PhantomData); +impl OnRuntimeUpgrade for InjectValidatorsIntoSortedListProvider { + fn on_runtime_upgrade() -> Weight { + for (v, _) in Validators::::iter() { + let weight = Pallet::::vote_weight(&v); + let _ = T::SortedListProvider::on_insert(v.clone(), weight).map_err(|err| { + log!(warn, "failed to insert {:?} into SortedListProvider: {:?}", v, err) + }); + } + + T::BlockWeights::get().max_block + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + use frame_support::traits::OnRuntimeUpgradeHelpersExt; + let prev_count = T::SortedListProvider::count(); + Self::set_temp_storage(prev_count, "prev"); + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade() -> Result<(), &'static str> { + use frame_support::traits::OnRuntimeUpgradeHelpersExt; + let post_count = T::SortedListProvider::count(); + let prev_count = Self::get_temp_storage::("prev"); + let validators = Validators::::count(); + assert!(post_count == prev_count + validators); + } +} pub mod v8 { use frame_election_provider_support::SortedListProvider; diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 95f305dfdd22a..cb3240769f1f0 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -541,7 +541,7 @@ fn check_count() { // the voters that the `SortedListProvider` list is storing for us. let external_voters = ::SortedListProvider::count(); - assert_eq!(external_voters, nominator_count); + assert_eq!(external_voters, nominator_count + validator_count); } fn check_ledgers() { diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index ae20550cd40b6..091c941cb5985 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -649,90 +649,78 @@ impl Pallet { /// Get all of the voters that are eligible for the npos election. /// - /// `maybe_max_len` can imposes a cap on the number of voters returned; First all the validator - /// are included in no particular order, then remainder is taken from the nominators, as - /// returned by [`Config::SortedListProvider`]. - /// - /// This will use nominators, and all the validators will inject a self vote. + /// `maybe_max_len` can imposes a cap on the number of voters returned; /// /// This function is self-weighing as [`DispatchClass::Mandatory`]. /// /// ### Slashing /// - /// All nominations that have been submitted before the last non-zero slash of the validator are - /// auto-chilled, but still count towards the limit imposed by `maybe_max_len`. + /// All voter have been submitted before the last non-zero slash of the corresponding target are + /// *auto-chilled*, but still count towards the limit imposed by `maybe_max_len`. pub fn get_npos_voters(maybe_max_len: Option) -> Vec> { let max_allowed_len = { - let nominator_count = Nominators::::count() as usize; - let validator_count = Validators::::count() as usize; - let all_voter_count = validator_count.saturating_add(nominator_count); + let all_voter_count = T::SortedListProvider::count() as usize; maybe_max_len.unwrap_or(all_voter_count).min(all_voter_count) }; let mut all_voters = Vec::<_>::with_capacity(max_allowed_len); - // first, grab all validators in no particular order, capped by the maximum allowed length. - let mut validators_taken = 0u32; - for (validator, _) in >::iter().take(max_allowed_len) { - // Append self vote. - let self_vote = ( - validator.clone(), - Self::weight_of(&validator), - vec![validator.clone()] - .try_into() - .expect("`MaxVotesPerVoter` must be greater than or equal to 1"), - ); - all_voters.push(self_vote); - validators_taken.saturating_inc(); - } - - // .. and grab whatever we have left from nominators. - let nominators_quota = (max_allowed_len as u32).saturating_sub(validators_taken); + // cache a few things. + let weight_of = Self::weight_of_fn(); let slashing_spans = >::iter().collect::>(); - // track the count of nominators added to `all_voters + let mut voters_taken = 0u32; + let mut voters_seen = 0u32; + let mut validators_taken = 0u32; let mut nominators_taken = 0u32; - // track every nominator iterated over, but not necessarily added to `all_voters` - let mut nominators_seen = 0u32; - // cache the total-issuance once in this function - let weight_of = Self::weight_of_fn(); - - let mut nominators_iter = T::SortedListProvider::iter(); - while nominators_taken < nominators_quota && nominators_seen < nominators_quota * 2 { - let nominator = match nominators_iter.next() { - Some(nominator) => { - nominators_seen.saturating_inc(); - nominator + while voters_taken < (max_allowed_len as u32) && voters_seen < (2 * max_allowed_len as u32) + { + let voter = match T::SortedListProvider::iter().next() { + Some(voter) => { + voters_seen.saturating_inc(); + voter }, None => break, }; if let Some(Nominations { submitted_in, mut targets, suppressed: _ }) = - >::get(&nominator) + >::get(&voter) { - log!( - trace, - "fetched nominator {:?} with weight {:?}", - nominator, - weight_of(&nominator) - ); + // if this voter is a nominator: targets.retain(|stash| { slashing_spans .get(stash) .map_or(true, |spans| submitted_in >= spans.last_nonzero_slash()) }); if !targets.len().is_zero() { - all_voters.push((nominator.clone(), weight_of(&nominator), targets)); + all_voters.push((voter.clone(), weight_of(&voter), targets)); + voters_taken.saturating_inc(); nominators_taken.saturating_inc(); } + } else if Validators::::contains_key(&voter) { + // if this voter is a validator: + let self_vote = ( + voter.clone(), + Self::weight_of(&voter), + vec![voter.clone()] + .try_into() + .expect("`MaxVotesPerVoter` must be greater than or equal to 1"), + ); + all_voters.push(self_vote); + voters_taken.saturating_inc(); + validators_taken.saturating_inc(); } else { - // this can only happen if: 1. there a pretty bad bug in the bags-list (or whatever - // is the sorted list) logic and the state of the two pallets is no longer - // compatible, or because the nominators is not decodable since they have more - // nomination than `T::MaxNominations`. This can rarely happen, and is not really an - // emergency or bug if it does. - log!(warn, "DEFENSIVE: invalid item in `SortedListProvider`: {:?}, this nominator probably has too many nominations now", nominator) + // this can only happen if: 1. there a bug in the bags-list (or whatever is the + // sorted list) logic and the state of the two pallets is no longer compatible, or + // because the nominators is not decodable since they have more nomination than + // `T::MaxNominations`. This can rarely happen, and is not really an emergency or + // bug if it does. + log!( + warn, + "DEFENSIVE: invalid item in `SortedListProvider`: {:?}, this nominator probably has too many nominations now", + voter + ) } } @@ -748,10 +736,11 @@ impl Pallet { log!( info, "generated {} npos voters, {} from validators and {} nominators", - all_voters.len(), + voters_taken, validators_taken, nominators_taken ); + all_voters } @@ -782,14 +771,17 @@ impl Pallet { /// wrong. pub fn do_add_nominator(who: &T::AccountId, nominations: Nominations) { if !Nominators::::contains_key(who) { - // maybe update sorted list. Error checking is defensive-only - this should never fail. + // maybe update sorted list. let _ = T::SortedListProvider::on_insert(who.clone(), Self::weight_of(who)) .defensive_unwrap_or_default(); - - debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); } - Nominators::::insert(who, nominations); + + debug_assert_eq!( + Nominators::::count() + Validators::::count(), + T::SortedListProvider::count() + ); + debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); } /// This function will remove a nominator from the `Nominators` storage map, @@ -801,15 +793,21 @@ impl Pallet { /// `Nominators` or `VoterList` outside of this function is almost certainly /// wrong. pub fn do_remove_nominator(who: &T::AccountId) -> bool { - if Nominators::::contains_key(who) { + let outcome = if Nominators::::contains_key(who) { Nominators::::remove(who); T::SortedListProvider::on_remove(who); - debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); - debug_assert_eq!(Nominators::::count(), T::SortedListProvider::count()); true } else { false - } + }; + + debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); + debug_assert_eq!( + Nominators::::count() + Validators::::count(), + T::SortedListProvider::count() + ); + + outcome } /// This function will add a validator to the `Validators` storage map. @@ -820,7 +818,18 @@ impl Pallet { /// `Validators` or `VoterList` outside of this function is almost certainly /// wrong. pub fn do_add_validator(who: &T::AccountId, prefs: ValidatorPrefs) { + if !Validators::::contains_key(who) { + // maybe update sorted list. + let _ = T::SortedListProvider::on_insert(who.clone(), Self::weight_of(who)) + .defensive_unwrap_or_default(); + } Validators::::insert(who, prefs); + + debug_assert_eq!( + Nominators::::count() + Validators::::count(), + T::SortedListProvider::count() + ); + debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); } /// This function will remove a validator from the `Validators` storage map. @@ -831,12 +840,21 @@ impl Pallet { /// `Validators` or `VoterList` outside of this function is almost certainly /// wrong. pub fn do_remove_validator(who: &T::AccountId) -> bool { - if Validators::::contains_key(who) { + let outcome = if Validators::::contains_key(who) { Validators::::remove(who); + T::SortedListProvider::on_remove(who); true } else { false - } + }; + + debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); + debug_assert_eq!( + Nominators::::count() + Validators::::count(), + T::SortedListProvider::count() + ); + + outcome } /// Register some amount of weight directly with the system pallet. @@ -1276,19 +1294,23 @@ impl VoteWeightProvider for Pallet { /// A simple voter list implementation that does not require any additional pallets. Note, this /// does not provided nominators in sorted ordered. If you desire nominators in a sorted order take /// a look at [`pallet-bags-list]. -pub struct UseNominatorsMap(sp_std::marker::PhantomData); -impl SortedListProvider for UseNominatorsMap { +pub struct UseNominatorsAndValidatorsMap(sp_std::marker::PhantomData); +impl SortedListProvider for UseNominatorsAndValidatorsMap { type Error = (); /// Returns iterator over voter list, which can have `take` called on it. fn iter() -> Box> { - Box::new(Nominators::::iter().map(|(n, _)| n)) + Box::new( + Validators::::iter() + .map(|(v, _)| v) + .chain(Nominators::::iter().map(|(n, _)| n)), + ) } fn count() -> u32 { - Nominators::::count() + Nominators::::count().saturating_add(Validators::::count()) } fn contains(id: &T::AccountId) -> bool { - Nominators::::contains_key(id) + Nominators::::contains_key(id) || Validators::::contains_key(id) } fn on_insert(_: T::AccountId, _weight: VoteWeight) -> Result<(), Self::Error> { // nothing to do on insert. @@ -1315,5 +1337,6 @@ impl SortedListProvider for UseNominatorsMap { // NOTE: Caller must ensure this doesn't lead to too many storage accesses. This is a // condition of SortedListProvider::unsafe_clear. Nominators::::remove_all(); + Validators::::remove_all(); } } diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 2a870fda063d3..7fea562ffd14f 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -567,7 +567,7 @@ pub mod pallet { // all voters are reported to the `SortedListProvider`. assert_eq!( T::SortedListProvider::count(), - Nominators::::count(), + Nominators::::count() + Validators::::count(), "not all genesis stakers were inserted into sorted list provider, something is wrong." ); } diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 538b75ead340b..4df1378bc059f 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4056,11 +4056,7 @@ mod election_data_provider { .set_status(41, StakerStatus::Validator) .build_and_execute(|| { // sum of all nominators who'd be voters (1), plus the self-votes (4). - assert_eq!( - ::SortedListProvider::count() + - >::iter().count() as u32, - 5 - ); + assert_eq!(::SortedListProvider::count(), 5); // if limits is less.. assert_eq!(Staking::voters(Some(1)).unwrap().len(), 1); @@ -4080,43 +4076,43 @@ mod election_data_provider { }); } + // Tests the criteria that in `ElectionDataProvider::voters` function, we try to get at most + // `maybe_max_len` voters, and if some of them end up being skipped, we iterate at most `2 * + // maybe_max_len`. #[test] fn only_iterates_max_2_times_nominators_quota() { ExtBuilder::default() - .nominate(true) // add nominator 101, who nominates [11, 21] + .nominate(false) // the other nominators only nominate 21 .add_staker(61, 60, 2_000, StakerStatus::::Nominator(vec![21])) .add_staker(71, 70, 2_000, StakerStatus::::Nominator(vec![21])) .add_staker(81, 80, 2_000, StakerStatus::::Nominator(vec![21])) .build_and_execute(|| { - // given our nominators ordered by stake, + // all voters ordered by stake, assert_eq!( ::SortedListProvider::iter().collect::>(), - vec![61, 71, 81, 101] - ); - - // and total voters - assert_eq!( - ::SortedListProvider::count() + - >::iter().count() as u32, - 7 + vec![61, 71, 81, 11, 21, 31] ); - // roll to session 5 run_to_block(25); // slash 21, the only validator nominated by our first 3 nominators add_slash(&21); - // we take 4 voters: 2 validators and 2 nominators (so nominators quota = 2) + // we want 2 voters now, and in maximum we allow 4 iterations. This is what happens: + // 61 is pruned; + // 71 is pruned; + // 81 is pruned; + // 11 is taken; + // we finish since the 2x limit is reached. assert_eq!( - Staking::voters(Some(3)) + Staking::voters(Some(2)) .unwrap() .iter() .map(|(stash, _, _)| stash) .copied() .collect::>(), - vec![31, 11], // 2 validators, but no nominators because we hit the quota + vec![11], ); }); } @@ -4129,46 +4125,35 @@ mod election_data_provider { #[test] fn get_max_len_voters_even_if_some_nominators_are_slashed() { ExtBuilder::default() - .nominate(true) // add nominator 101, who nominates [11, 21] + .nominate(false) .add_staker(61, 60, 20, StakerStatus::::Nominator(vec![21])) - // 61 only nominates validator 21 ^^ .add_staker(71, 70, 10, StakerStatus::::Nominator(vec![11, 21])) + .add_staker(81, 80, 10, StakerStatus::::Nominator(vec![11, 21])) .build_and_execute(|| { - // given our nominators ordered by stake, + // given our voters ordered by stake, assert_eq!( ::SortedListProvider::iter().collect::>(), - vec![101, 61, 71] - ); - - // and total voters - assert_eq!( - ::SortedListProvider::count() + - >::iter().count() as u32, - 6 + vec![11, 21, 31, 61, 71, 81] ); - // we take 5 voters + // we take 4 voters assert_eq!( - Staking::voters(Some(5)) + Staking::voters(Some(4)) .unwrap() .iter() .map(|(stash, _, _)| stash) .copied() .collect::>(), - // then - vec![ - 31, 21, 11, // 3 nominators - 101, 61 // 2 validators, and 71 is excluded - ], + vec![11, 21, 31, 61], ); // roll to session 5 run_to_block(25); - // slash 21, the only validator nominated by 61 + // slash 21, the only validator nominated by 61 and 21. add_slash(&21); - // we take 4 voters + // we take 4 voters; 71 and 81 are replacing the ejected ones. assert_eq!( Staking::voters(Some(4)) .unwrap() @@ -4176,10 +4161,7 @@ mod election_data_provider { .map(|(stash, _, _)| stash) .copied() .collect::>(), - vec![ - 31, 11, // 2 validators (21 was slashed) - 101, 71 // 2 nominators, excluding 61 - ], + vec![11, 31, 71, 81], ); }); } @@ -4695,19 +4677,51 @@ mod sorted_list_provider { fn re_nominate_does_not_change_counters_or_list() { ExtBuilder::default().nominate(true).build_and_execute(|| { // given - let pre_insert_nominator_count = Nominators::::iter().count() as u32; - assert_eq!(::SortedListProvider::count(), pre_insert_nominator_count); - assert!(Nominators::::contains_key(101)); - assert_eq!(::SortedListProvider::iter().collect::>(), vec![101]); + let pre_insert_voter_count = + (Nominators::::count() + Validators::::count()) as u32; + assert_eq!(::SortedListProvider::count(), pre_insert_voter_count); + + assert_eq!( + ::SortedListProvider::iter().collect::>(), + vec![11, 21, 31, 101] + ); // when account 101 renominates assert_ok!(Staking::nominate(Origin::signed(100), vec![41])); // then counts don't change - assert_eq!(::SortedListProvider::count(), pre_insert_nominator_count); - assert_eq!(Nominators::::iter().count() as u32, pre_insert_nominator_count); + assert_eq!(::SortedListProvider::count(), pre_insert_voter_count); + // and the list is the same + assert_eq!( + ::SortedListProvider::iter().collect::>(), + vec![11, 21, 31, 101] + ); + }); + } + + #[test] + fn re_validate_does_not_change_counters_or_list() { + ExtBuilder::default().nominate(false).build_and_execute(|| { + // given + let pre_insert_voter_count = + (Nominators::::count() + Validators::::count()) as u32; + assert_eq!(::SortedListProvider::count(), pre_insert_voter_count); + + assert_eq!( + ::SortedListProvider::iter().collect::>(), + vec![11, 21, 31] + ); + + // when account 101 re-validates + assert_ok!(Staking::validate(Origin::signed(10), Default::default())); + + // then counts don't change + assert_eq!(::SortedListProvider::count(), pre_insert_voter_count); // and the list is the same - assert_eq!(::SortedListProvider::iter().collect::>(), vec![101]); + assert_eq!( + ::SortedListProvider::iter().collect::>(), + vec![11, 21, 31] + ); }); } } From ddcc81b92f221c61ada1a787e8ee9c0c5357255c Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Tue, 15 Feb 2022 12:32:50 +0000 Subject: [PATCH 02/19] Apply suggestions from code review Co-authored-by: Zeke Mostov --- frame/staking/src/pallet/impls.rs | 4 ++-- frame/staking/src/tests.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 091c941cb5985..9986c160c2b93 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -655,7 +655,7 @@ impl Pallet { /// /// ### Slashing /// - /// All voter have been submitted before the last non-zero slash of the corresponding target are + /// All votes that have been submitted before the last non-zero slash of the corresponding target are /// *auto-chilled*, but still count towards the limit imposed by `maybe_max_len`. pub fn get_npos_voters(maybe_max_len: Option) -> Vec> { let max_allowed_len = { @@ -714,7 +714,7 @@ impl Pallet { // this can only happen if: 1. there a bug in the bags-list (or whatever is the // sorted list) logic and the state of the two pallets is no longer compatible, or // because the nominators is not decodable since they have more nomination than - // `T::MaxNominations`. This can rarely happen, and is not really an emergency or + // `T::MaxNominations`. The latter can rarely happen, and is not really an emergency or // bug if it does. log!( warn, diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 4df1378bc059f..6ae46aed4ad49 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4080,7 +4080,7 @@ mod election_data_provider { // `maybe_max_len` voters, and if some of them end up being skipped, we iterate at most `2 * // maybe_max_len`. #[test] - fn only_iterates_max_2_times_nominators_quota() { + fn only_iterates_max_2_times_max_allowed_len() { ExtBuilder::default() .nominate(false) // the other nominators only nominate 21 @@ -4712,7 +4712,7 @@ mod sorted_list_provider { vec![11, 21, 31] ); - // when account 101 re-validates + // when account 11 re-validates assert_ok!(Staking::validate(Origin::signed(10), Default::default())); // then counts don't change From 41540f25af0e3aaccfb2cc5523c085936ad6d5b7 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 15 Feb 2022 12:36:00 +0000 Subject: [PATCH 03/19] some review comments --- frame/babe/src/mock.rs | 2 +- frame/grandpa/src/mock.rs | 2 +- frame/offences/benchmarking/src/mock.rs | 2 +- frame/session/benchmarking/src/mock.rs | 2 +- frame/staking/src/mock.rs | 2 +- frame/staking/src/pallet/impls.rs | 6 +----- frame/staking/src/pallet/mod.rs | 3 ++- 7 files changed, 8 insertions(+), 11 deletions(-) diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index 2e4b6023f1e5f..208a14f91d16e 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -197,7 +197,7 @@ impl pallet_staking::Config for Test { type NextNewSession = Session; type ElectionProvider = onchain::OnChainSequentialPhragmen; type GenesisElectionProvider = Self::ElectionProvider; - type SortedListProvider = pallet_staking::UseNominatorsMap; + type SortedListProvider = pallet_staking::UseNominatorsAndValidatorsMap; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; type WeightInfo = (); } diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index d07f3136d9a0d..04e71445ecd34 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -205,7 +205,7 @@ impl pallet_staking::Config for Test { type NextNewSession = Session; type ElectionProvider = onchain::OnChainSequentialPhragmen; type GenesisElectionProvider = Self::ElectionProvider; - type SortedListProvider = pallet_staking::UseNominatorsMap; + type SortedListProvider = pallet_staking::UseNominatorsAndValidatorsMap; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; type WeightInfo = (); } diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index 3b5e640867c5a..1b78c00d66dda 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -175,7 +175,7 @@ impl pallet_staking::Config for Test { type OffendingValidatorsThreshold = (); type ElectionProvider = onchain::OnChainSequentialPhragmen; type GenesisElectionProvider = Self::ElectionProvider; - type SortedListProvider = pallet_staking::UseNominatorsMap; + type SortedListProvider = pallet_staking::UseNominatorsAndValidatorsMap; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; type WeightInfo = (); } diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs index 37305437ca095..771839b2bf45b 100644 --- a/frame/session/benchmarking/src/mock.rs +++ b/frame/session/benchmarking/src/mock.rs @@ -181,7 +181,7 @@ impl pallet_staking::Config for Test { type OffendingValidatorsThreshold = (); type ElectionProvider = onchain::OnChainSequentialPhragmen; type GenesisElectionProvider = Self::ElectionProvider; - type SortedListProvider = pallet_staking::UseNominatorsMap; + type SortedListProvider = pallet_staking::UseNominatorsAndValidatorsMap; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; type WeightInfo = (); } diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index cb3240769f1f0..16b16dba22d57 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -269,7 +269,7 @@ impl crate::pallet::pallet::Config for Test { type OffendingValidatorsThreshold = OffendingValidatorsThreshold; type ElectionProvider = onchain::OnChainSequentialPhragmen; type GenesisElectionProvider = Self::ElectionProvider; - // NOTE: consider a macro and use `UseNominatorsMap` as well. + // NOTE: consider a macro and use `UseNominatorsAndValidatorsMap` as well. type SortedListProvider = BagsList; type BenchmarkingConfig = TestBenchmarkingConfig; type WeightInfo = (); diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 091c941cb5985..abed1c416d9cc 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -669,13 +669,11 @@ impl Pallet { let weight_of = Self::weight_of_fn(); let slashing_spans = >::iter().collect::>(); - let mut voters_taken = 0u32; let mut voters_seen = 0u32; let mut validators_taken = 0u32; let mut nominators_taken = 0u32; - while voters_taken < (max_allowed_len as u32) && voters_seen < (2 * max_allowed_len as u32) - { + while all_voters.len() < max_allowed_len && voters_seen < (2 * max_allowed_len as u32) { let voter = match T::SortedListProvider::iter().next() { Some(voter) => { voters_seen.saturating_inc(); @@ -695,7 +693,6 @@ impl Pallet { }); if !targets.len().is_zero() { all_voters.push((voter.clone(), weight_of(&voter), targets)); - voters_taken.saturating_inc(); nominators_taken.saturating_inc(); } } else if Validators::::contains_key(&voter) { @@ -708,7 +705,6 @@ impl Pallet { .expect("`MaxVotesPerVoter` must be greater than or equal to 1"), ); all_voters.push(self_vote); - voters_taken.saturating_inc(); validators_taken.saturating_inc(); } else { // this can only happen if: 1. there a bug in the bags-list (or whatever is the diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 7fea562ffd14f..51045d1538f8f 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -153,7 +153,8 @@ pub mod pallet { /// Something that can provide a sorted list of voters in a somewhat sorted way. The /// original use case for this was designed with `pallet_bags_list::Pallet` in mind. If - /// the bags-list is not desired, [`impls::UseNominatorsMap`] is likely the desired option. + /// the bags-list is not desired, [`impls::UseNominatorsAndValidatorsMap`] is likely the + /// desired option. type SortedListProvider: SortedListProvider; /// Some parameters of the benchmarking. From 398cf7026e47b8bce715bbfb01aae98d7c5cbe3e Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 16 Feb 2022 14:57:11 +0000 Subject: [PATCH 04/19] guard the migration --- frame/staking/src/lib.rs | 1 + frame/staking/src/migrations.rs | 24 +++++++++++++++++------- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 17af4829c0ea8..11d840d42f67c 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -771,6 +771,7 @@ enum Releases { V6_0_0, // removal of all storage associated with offchain phragmen. V7_0_0, // keep track of number of nominators / validators in map V8_0_0, // populate `SortedListProvider`. + V9_0_0, // inject validators into `SortedListProvider` as well. } impl Default for Releases { diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index cd7f7c633184f..04b5817ff4cc9 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -26,19 +26,29 @@ use frame_support::traits::OnRuntimeUpgrade; pub struct InjectValidatorsIntoSortedListProvider(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for InjectValidatorsIntoSortedListProvider { fn on_runtime_upgrade() -> Weight { - for (v, _) in Validators::::iter() { - let weight = Pallet::::vote_weight(&v); - let _ = T::SortedListProvider::on_insert(v.clone(), weight).map_err(|err| { - log!(warn, "failed to insert {:?} into SortedListProvider: {:?}", v, err) - }); - } + if StorageVersion::::get() == Releases::V8_0_0 { + for (v, _) in Validators::::iter() { + let weight = Pallet::::vote_weight(&v); + let _ = T::SortedListProvider::on_insert(v.clone(), weight).map_err(|err| { + log!(warn, "failed to insert {:?} into SortedListProvider: {:?}", v, err) + }); + } - T::BlockWeights::get().max_block + T::BlockWeights::get().max_block + } else { + log!(warn, "InjectValidatorsIntoSortedListProvider being executed on the wrong storage version, expected Releases::V8_0_0"); + T::DbWeight::get().reads(1) + } } #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { use frame_support::traits::OnRuntimeUpgradeHelpersExt; + frame_support::ensure!( + StorageVersion::::get() == crate::Releases::V8_0_0, + "must upgrade linearly" + ); + let prev_count = T::SortedListProvider::count(); Self::set_temp_storage(prev_count, "prev"); } From 66fc7d2ce9214ca6fab726c682386a0f8ac9d57a Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 16 Feb 2022 15:30:18 +0000 Subject: [PATCH 05/19] some review comments --- frame/staking/src/pallet/impls.rs | 10 +++++----- frame/staking/src/tests.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 3cab0d9487717..0a83b560163ae 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -655,8 +655,8 @@ impl Pallet { /// /// ### Slashing /// - /// All votes that have been submitted before the last non-zero slash of the corresponding target are - /// *auto-chilled*, but still count towards the limit imposed by `maybe_max_len`. + /// All votes that have been submitted before the last non-zero slash of the corresponding + /// target are *auto-chilled*, but still count towards the limit imposed by `maybe_max_len`. pub fn get_npos_voters(maybe_max_len: Option) -> Vec> { let max_allowed_len = { let all_voter_count = T::SortedListProvider::count() as usize; @@ -710,8 +710,8 @@ impl Pallet { // this can only happen if: 1. there a bug in the bags-list (or whatever is the // sorted list) logic and the state of the two pallets is no longer compatible, or // because the nominators is not decodable since they have more nomination than - // `T::MaxNominations`. The latter can rarely happen, and is not really an emergency or - // bug if it does. + // `T::MaxNominations`. The latter can rarely happen, and is not really an emergency + // or bug if it does. log!( warn, "DEFENSIVE: invalid item in `SortedListProvider`: {:?}, this nominator probably has too many nominations now", @@ -732,7 +732,7 @@ impl Pallet { log!( info, "generated {} npos voters, {} from validators and {} nominators", - voters_taken, + all_voters.len(), validators_taken, nominators_taken ); diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index faf3b9d90c377..50d95f2847cb1 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4150,7 +4150,7 @@ mod election_data_provider { // roll to session 5 run_to_block(25); - // slash 21, the only validator nominated by 61 and 21. + // slash 21, the only validator nominated by 61. add_slash(&21); // we take 4 voters; 71 and 81 are replacing the ejected ones. From 75989dec72ed5e6ffb55c1add74d2ddc21ac0d73 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 16 Feb 2022 15:40:57 +0000 Subject: [PATCH 06/19] =?UTF-8?q?Fix=20tests=20=F0=9F=A4=A6=E2=80=8D?= =?UTF-8?q?=E2=99=82=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- frame/staking/src/pallet/impls.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 0a83b560163ae..0e603b59e986f 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -673,8 +673,9 @@ impl Pallet { let mut validators_taken = 0u32; let mut nominators_taken = 0u32; + let mut sorted_voters = T::SortedListProvider::iter(); while all_voters.len() < max_allowed_len && voters_seen < (2 * max_allowed_len as u32) { - let voter = match T::SortedListProvider::iter().next() { + let voter = match sorted_voters.next() { Some(voter) => { voters_seen.saturating_inc(); voter From db2a7b81a6c8487463af7929aa1ccef7df885972 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 16 Feb 2022 16:11:36 +0000 Subject: [PATCH 07/19] Fix build --- frame/staking/src/lib.rs | 6 +++--- frame/staking/src/migrations.rs | 4 +++- frame/staking/src/pallet/mod.rs | 9 ++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 11d840d42f67c..fa71a50962f6c 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -301,7 +301,7 @@ mod pallet; use codec::{Decode, Encode, HasCompact}; use frame_support::{ - traits::{ConstU32, Currency, Get}, + traits::{Currency, Get}, weights::Weight, BoundedVec, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; @@ -852,6 +852,6 @@ pub struct TestBenchmarkingConfig; #[cfg(feature = "std")] impl BenchmarkingConfig for TestBenchmarkingConfig { - type MaxValidators = ConstU32<100>; - type MaxNominators = ConstU32<100>; + type MaxValidators = frame_support::traits::ConstU32<100>; + type MaxNominators = frame_support::traits::ConstU32<100>; } diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 04b5817ff4cc9..029a6a380fee3 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -51,15 +51,17 @@ impl OnRuntimeUpgrade for InjectValidatorsIntoSortedListProvider { let prev_count = T::SortedListProvider::count(); Self::set_temp_storage(prev_count, "prev"); + Ok(()) } #[cfg(feature = "try-runtime")] fn post_upgrade() -> Result<(), &'static str> { use frame_support::traits::OnRuntimeUpgradeHelpersExt; let post_count = T::SortedListProvider::count(); - let prev_count = Self::get_temp_storage::("prev"); + let prev_count = Self::get_temp_storage::("prev").unwrap(); let validators = Validators::::count(); assert!(post_count == prev_count + validators); + Ok(()) } } diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 36056c4f9db6d..134d3a9a65bc5 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -39,10 +39,9 @@ mod impls; pub use impls::*; use crate::{ - log, slashing, weights::WeightInfo, ActiveEraInfo, BalanceOf, EraPayout, EraRewardPoints, - Exposure, Forcing, NegativeImbalanceOf, Nominations, PositiveImbalanceOf, Releases, - RewardDestination, SessionInterface, StakingLedger, UnappliedSlash, UnlockChunk, - ValidatorPrefs, + slashing, weights::WeightInfo, ActiveEraInfo, BalanceOf, EraPayout, EraRewardPoints, Exposure, + Forcing, NegativeImbalanceOf, Nominations, PositiveImbalanceOf, Releases, RewardDestination, + SessionInterface, StakingLedger, UnappliedSlash, UnlockChunk, ValidatorPrefs, }; pub const MAX_UNLOCKING_CHUNKS: usize = 32; @@ -535,7 +534,7 @@ pub mod pallet { } for &(ref stash, ref controller, balance, ref status) in &self.stakers { - log!( + crate::log!( trace, "inserting genesis staker: {:?} => {:?} => {:?}", stash, From f892d042f8d100d02ca85b656c42d34616dac824 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 11 Mar 2022 14:19:41 +0000 Subject: [PATCH 08/19] fix weight_of_fn --- frame/staking/src/migrations.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 029a6a380fee3..89eb313f00313 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -17,7 +17,7 @@ //! Storage migrations for the Staking pallet. use super::*; -use frame_election_provider_support::{SortedListProvider, VoteWeightProvider}; +use frame_election_provider_support::{SortedListProvider, ScoreProvider}; use frame_support::traits::OnRuntimeUpgrade; /// Migration implementation that injects all validators into sorted list. @@ -27,13 +27,15 @@ pub struct InjectValidatorsIntoSortedListProvider(sp_std::marker::PhantomData impl OnRuntimeUpgrade for InjectValidatorsIntoSortedListProvider { fn on_runtime_upgrade() -> Weight { if StorageVersion::::get() == Releases::V8_0_0 { + let weight_of_cached = Pallet::::weight_of_fn(); for (v, _) in Validators::::iter() { - let weight = Pallet::::vote_weight(&v); + let weight = weight_of_cached(&v); let _ = T::SortedListProvider::on_insert(v.clone(), weight).map_err(|err| { log!(warn, "failed to insert {:?} into SortedListProvider: {:?}", v, err) }); } + StorageVersion::::put(crate::Releases::V9_0_0); T::BlockWeights::get().max_block } else { log!(warn, "InjectValidatorsIntoSortedListProvider being executed on the wrong storage version, expected Releases::V8_0_0"); From 5640da73cc4599fdbf863229706165aeea6ef04f Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 11 Mar 2022 14:21:19 +0000 Subject: [PATCH 09/19] reformat line width --- frame/staking/src/migrations.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 89eb313f00313..32e4a139bc32a 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -17,7 +17,7 @@ //! Storage migrations for the Staking pallet. use super::*; -use frame_election_provider_support::{SortedListProvider, ScoreProvider}; +use frame_election_provider_support::{ScoreProvider, SortedListProvider}; use frame_support::traits::OnRuntimeUpgrade; /// Migration implementation that injects all validators into sorted list. @@ -38,7 +38,11 @@ impl OnRuntimeUpgrade for InjectValidatorsIntoSortedListProvider { StorageVersion::::put(crate::Releases::V9_0_0); T::BlockWeights::get().max_block } else { - log!(warn, "InjectValidatorsIntoSortedListProvider being executed on the wrong storage version, expected Releases::V8_0_0"); + log!( + warn, + "InjectValidatorsIntoSortedListProvider being executed on the wrong storage \ + version, expected Releases::V8_0_0" + ); T::DbWeight::get().reads(1) } } From 3aefa002b04139100ad50f43e19f81d091f0e3e7 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 11 Mar 2022 14:27:02 +0000 Subject: [PATCH 10/19] make const --- frame/staking/src/migrations.rs | 2 +- frame/staking/src/pallet/impls.rs | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 32e4a139bc32a..96c5e2da89367 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -17,7 +17,7 @@ //! Storage migrations for the Staking pallet. use super::*; -use frame_election_provider_support::{ScoreProvider, SortedListProvider}; +use frame_election_provider_support::SortedListProvider; use frame_support::traits::OnRuntimeUpgrade; /// Migration implementation that injects all validators into sorted list. diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 372460ab7a620..0bffae529a4e7 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -49,6 +49,14 @@ use crate::{ use super::{pallet::*, STAKING_ID}; +/// The maximum number of iterations that we do whilst iterating over `T::VoterList` in +/// `get_npos_voters`. +/// +/// In most cases, if we want n items, we iterate exactly n times. In rare cases, if a voter is +/// invalid (for any reason) the iteration continues. With this constant, we iterate at most 2 * n +/// times and then give up. +const NPOS_MAX_ITERATIONS_COEFFICIENT: u32 = 2; + impl Pallet { /// The total balance that can be slashed from a stash account as of right now. pub fn slashable_balance_of(stash: &T::AccountId) -> BalanceOf { @@ -674,7 +682,9 @@ impl Pallet { let mut nominators_taken = 0u32; let mut sorted_voters = T::SortedListProvider::iter(); - while all_voters.len() < max_allowed_len && voters_seen < (2 * max_allowed_len as u32) { + while all_voters.len() < max_allowed_len && + voters_seen < (NPOS_MAX_ITERATIONS_COEFFICIENT * max_allowed_len as u32) + { let voter = match sorted_voters.next() { Some(voter) => { voters_seen.saturating_inc(); From 0923d5b6511fab7ceec411a72cd83f10556c8492 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 11 Mar 2022 14:30:31 +0000 Subject: [PATCH 11/19] use weight of fn cached --- frame/staking/src/pallet/impls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 0bffae529a4e7..a26207547f656 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -710,7 +710,7 @@ impl Pallet { // if this voter is a validator: let self_vote = ( voter.clone(), - Self::weight_of(&voter), + weight_of(&voter), vec![voter.clone()] .try_into() .expect("`MaxVotesPerVoter` must be greater than or equal to 1"), From 2ea076924f1322d25098f81cba996385659902bf Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 11 Mar 2022 14:37:25 +0000 Subject: [PATCH 12/19] SortedListProvider -> VoterList --- frame/staking/src/benchmarking.rs | 34 ++++++++++++++-------------- frame/staking/src/lib.rs | 4 ++-- frame/staking/src/migrations.rs | 25 ++++++++++----------- frame/staking/src/mock.rs | 6 ++--- frame/staking/src/pallet/impls.rs | 36 +++++++++++++++--------------- frame/staking/src/pallet/mod.rs | 33 +++++++++++++-------------- frame/staking/src/testing_utils.rs | 4 ++-- frame/staking/src/tests.rs | 28 +++++++++-------------- 8 files changed, 81 insertions(+), 89 deletions(-) diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index ca4b2388e8e42..983f1bd54deb7 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -155,8 +155,8 @@ impl ListScenario { /// - the destination bag has at least one node, which will need its next pointer updated. /// /// NOTE: while this scenario specifically targets a worst case for the bags-list, it should - /// also elicit a worst case for other known `SortedListProvider` implementations; although - /// this may not be true against unknown `SortedListProvider` implementations. + /// also elicit a worst case for other known `VoterList` implementations; although + /// this may not be true against unknown `VoterList` implementations. fn new(origin_weight: BalanceOf, is_increase: bool) -> Result { ensure!(!origin_weight.is_zero(), "origin weight must be greater than 0"); @@ -189,7 +189,7 @@ impl ListScenario { // find a destination weight that will trigger the worst case scenario let dest_weight_as_vote = - T::SortedListProvider::score_update_worst_case(&origin_stash1, is_increase); + T::VoterList::score_update_worst_case(&origin_stash1, is_increase); let total_issuance = T::Currency::total_issuance(); @@ -316,7 +316,7 @@ benchmarks! { let scenario = ListScenario::::new(origin_weight, true)?; let controller = scenario.origin_controller1.clone(); let stash = scenario.origin_stash1.clone(); - assert!(T::SortedListProvider::contains(&stash)); + assert!(T::VoterList::contains(&stash)); let ed = T::Currency::minimum_balance(); let mut ledger = Ledger::::get(&controller).unwrap(); @@ -328,7 +328,7 @@ benchmarks! { }: withdraw_unbonded(RawOrigin::Signed(controller.clone()), s) verify { assert!(!Ledger::::contains_key(controller)); - assert!(!T::SortedListProvider::contains(&stash)); + assert!(!T::VoterList::contains(&stash)); } validate { @@ -338,14 +338,14 @@ benchmarks! { Default::default(), )?; // because it is chilled. - assert!(!T::SortedListProvider::contains(&stash)); + assert!(!T::VoterList::contains(&stash)); let prefs = ValidatorPrefs::default(); whitelist_account!(controller); }: _(RawOrigin::Signed(controller), prefs) verify { assert!(Validators::::contains_key(&stash)); - assert!(T::SortedListProvider::contains(&stash)); + assert!(T::VoterList::contains(&stash)); } kick { @@ -430,14 +430,14 @@ benchmarks! { ).unwrap(); assert!(!Nominators::::contains_key(&stash)); - assert!(!T::SortedListProvider::contains(&stash)); + assert!(!T::VoterList::contains(&stash)); let validators = create_validators::(n, 100).unwrap(); whitelist_account!(controller); }: _(RawOrigin::Signed(controller), validators) verify { assert!(Nominators::::contains_key(&stash)); - assert!(T::SortedListProvider::contains(&stash)) + assert!(T::VoterList::contains(&stash)) } chill { @@ -451,12 +451,12 @@ benchmarks! { let scenario = ListScenario::::new(origin_weight, true)?; let controller = scenario.origin_controller1.clone(); let stash = scenario.origin_stash1.clone(); - assert!(T::SortedListProvider::contains(&stash)); + assert!(T::VoterList::contains(&stash)); whitelist_account!(controller); }: _(RawOrigin::Signed(controller)) verify { - assert!(!T::SortedListProvider::contains(&stash)); + assert!(!T::VoterList::contains(&stash)); } set_payee { @@ -519,13 +519,13 @@ benchmarks! { let scenario = ListScenario::::new(origin_weight, true)?; let controller = scenario.origin_controller1.clone(); let stash = scenario.origin_stash1.clone(); - assert!(T::SortedListProvider::contains(&stash)); + assert!(T::VoterList::contains(&stash)); add_slashing_spans::(&stash, s); }: _(RawOrigin::Root, stash.clone(), s) verify { assert!(!Ledger::::contains_key(&controller)); - assert!(!T::SortedListProvider::contains(&stash)); + assert!(!T::VoterList::contains(&stash)); } cancel_deferred_slash { @@ -704,13 +704,13 @@ benchmarks! { Ledger::::insert(&controller, l); assert!(Bonded::::contains_key(&stash)); - assert!(T::SortedListProvider::contains(&stash)); + assert!(T::VoterList::contains(&stash)); whitelist_account!(controller); }: _(RawOrigin::Signed(controller), stash.clone(), s) verify { assert!(!Bonded::::contains_key(&stash)); - assert!(!T::SortedListProvider::contains(&stash)); + assert!(!T::VoterList::contains(&stash)); } new_era { @@ -895,7 +895,7 @@ benchmarks! { let scenario = ListScenario::::new(origin_weight, true)?; let controller = scenario.origin_controller1.clone(); let stash = scenario.origin_stash1.clone(); - assert!(T::SortedListProvider::contains(&stash)); + assert!(T::VoterList::contains(&stash)); Staking::::set_staking_configs( RawOrigin::Root.into(), @@ -910,7 +910,7 @@ benchmarks! { let caller = whitelisted_caller(); }: _(RawOrigin::Signed(caller), controller.clone()) verify { - assert!(!T::SortedListProvider::contains(&stash)); + assert!(!T::VoterList::contains(&stash)); } force_apply_min_commission { diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 2885de66598e7..2a0716721dd51 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -780,8 +780,8 @@ enum Releases { V5_0_0, // blockable validators. V6_0_0, // removal of all storage associated with offchain phragmen. V7_0_0, // keep track of number of nominators / validators in map - V8_0_0, // populate `SortedListProvider`. - V9_0_0, // inject validators into `SortedListProvider` as well. + V8_0_0, // populate `VoterList`. + V9_0_0, // inject validators into `VoterList` as well. } impl Default for Releases { diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 96c5e2da89367..d3c4dd9809911 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -22,16 +22,16 @@ use frame_support::traits::OnRuntimeUpgrade; /// Migration implementation that injects all validators into sorted list. /// -/// This is only useful for chains that started their `SortedListProvider` just based on nominators. -pub struct InjectValidatorsIntoSortedListProvider(sp_std::marker::PhantomData); -impl OnRuntimeUpgrade for InjectValidatorsIntoSortedListProvider { +/// This is only useful for chains that started their `VoterList` just based on nominators. +pub struct InjectValidatorsIntoVoterList(sp_std::marker::PhantomData); +impl OnRuntimeUpgrade for InjectValidatorsIntoVoterList { fn on_runtime_upgrade() -> Weight { if StorageVersion::::get() == Releases::V8_0_0 { let weight_of_cached = Pallet::::weight_of_fn(); for (v, _) in Validators::::iter() { let weight = weight_of_cached(&v); - let _ = T::SortedListProvider::on_insert(v.clone(), weight).map_err(|err| { - log!(warn, "failed to insert {:?} into SortedListProvider: {:?}", v, err) + let _ = T::VoterList::on_insert(v.clone(), weight).map_err(|err| { + log!(warn, "failed to insert {:?} into VoterList: {:?}", v, err) }); } @@ -40,7 +40,7 @@ impl OnRuntimeUpgrade for InjectValidatorsIntoSortedListProvider { } else { log!( warn, - "InjectValidatorsIntoSortedListProvider being executed on the wrong storage \ + "InjectValidatorsIntoVoterList being executed on the wrong storage \ version, expected Releases::V8_0_0" ); T::DbWeight::get().reads(1) @@ -55,7 +55,7 @@ impl OnRuntimeUpgrade for InjectValidatorsIntoSortedListProvider { "must upgrade linearly" ); - let prev_count = T::SortedListProvider::count(); + let prev_count = T::VoterList::count(); Self::set_temp_storage(prev_count, "prev"); Ok(()) } @@ -63,7 +63,7 @@ impl OnRuntimeUpgrade for InjectValidatorsIntoSortedListProvider { #[cfg(feature = "try-runtime")] fn post_upgrade() -> Result<(), &'static str> { use frame_support::traits::OnRuntimeUpgradeHelpersExt; - let post_count = T::SortedListProvider::count(); + let post_count = T::VoterList::count(); let prev_count = Self::get_temp_storage::("prev").unwrap(); let validators = Validators::::count(); assert!(post_count == prev_count + validators); @@ -88,16 +88,16 @@ pub mod v8 { Ok(()) } - /// Migration to sorted [`SortedListProvider`]. + /// Migration to sorted [`VoterList`]. pub fn migrate() -> Weight { if StorageVersion::::get() == crate::Releases::V7_0_0 { crate::log!(info, "migrating staking to Releases::V8_0_0"); - let migrated = T::SortedListProvider::unsafe_regenerate( + let migrated = T::VoterList::unsafe_regenerate( Nominators::::iter().map(|(id, _)| id), Pallet::::weight_of_fn(), ); - debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); StorageVersion::::put(crate::Releases::V8_0_0); crate::log!( @@ -114,8 +114,7 @@ pub mod v8 { #[cfg(feature = "try-runtime")] pub fn post_migrate() -> Result<(), &'static str> { - T::SortedListProvider::sanity_check() - .map_err(|_| "SortedListProvider is not in a sane state.")?; + T::VoterList::sanity_check().map_err(|_| "VoterList is not in a sane state.")?; crate::log!(info, "👜 staking bags-list migration passes POST migrate checks ✅",); Ok(()) } diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index effd4bc0c5ace..bb71232c34673 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -271,7 +271,7 @@ impl crate::pallet::pallet::Config for Test { type ElectionProvider = onchain::OnChainSequentialPhragmen; type GenesisElectionProvider = Self::ElectionProvider; // NOTE: consider a macro and use `UseNominatorsAndValidatorsMap` as well. - type SortedListProvider = BagsList; + type VoterList = BagsList; type MaxUnlockingChunks = ConstU32<32>; type BenchmarkingConfig = TestBenchmarkingConfig; type WeightInfo = (); @@ -541,8 +541,8 @@ fn check_count() { assert_eq!(nominator_count, Nominators::::count()); assert_eq!(validator_count, Validators::::count()); - // the voters that the `SortedListProvider` list is storing for us. - let external_voters = ::SortedListProvider::count(); + // the voters that the `VoterList` list is storing for us. + let external_voters = ::VoterList::count(); assert_eq!(external_voters, nominator_count + validator_count); } diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index a26207547f656..6ff125060a70f 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -667,7 +667,7 @@ impl Pallet { /// target are *auto-chilled*, but still count towards the limit imposed by `maybe_max_len`. pub fn get_npos_voters(maybe_max_len: Option) -> Vec> { let max_allowed_len = { - let all_voter_count = T::SortedListProvider::count() as usize; + let all_voter_count = T::VoterList::count() as usize; maybe_max_len.unwrap_or(all_voter_count).min(all_voter_count) }; @@ -681,7 +681,7 @@ impl Pallet { let mut validators_taken = 0u32; let mut nominators_taken = 0u32; - let mut sorted_voters = T::SortedListProvider::iter(); + let mut sorted_voters = T::VoterList::iter(); while all_voters.len() < max_allowed_len && voters_seen < (NPOS_MAX_ITERATIONS_COEFFICIENT * max_allowed_len as u32) { @@ -725,7 +725,7 @@ impl Pallet { // or bug if it does. log!( warn, - "DEFENSIVE: invalid item in `SortedListProvider`: {:?}, this nominator probably has too many nominations now", + "DEFENSIVE: invalid item in `VoterList`: {:?}, this nominator probably has too many nominations now", voter ) } @@ -769,7 +769,7 @@ impl Pallet { } /// This function will add a nominator to the `Nominators` storage map, - /// and [`SortedListProvider`]. + /// and [`VoterList`]. /// /// If the nominator already exists, their nominations will be updated. /// @@ -779,20 +779,20 @@ impl Pallet { pub fn do_add_nominator(who: &T::AccountId, nominations: Nominations) { if !Nominators::::contains_key(who) { // maybe update sorted list. - let _ = T::SortedListProvider::on_insert(who.clone(), Self::weight_of(who)) + let _ = T::VoterList::on_insert(who.clone(), Self::weight_of(who)) .defensive_unwrap_or_default(); } Nominators::::insert(who, nominations); debug_assert_eq!( Nominators::::count() + Validators::::count(), - T::SortedListProvider::count() + T::VoterList::count() ); - debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); } /// This function will remove a nominator from the `Nominators` storage map, - /// and [`SortedListProvider`]. + /// and [`VoterList`]. /// /// Returns true if `who` was removed from `Nominators`, otherwise false. /// @@ -802,16 +802,16 @@ impl Pallet { pub fn do_remove_nominator(who: &T::AccountId) -> bool { let outcome = if Nominators::::contains_key(who) { Nominators::::remove(who); - T::SortedListProvider::on_remove(who); + T::VoterList::on_remove(who); true } else { false }; - debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); debug_assert_eq!( Nominators::::count() + Validators::::count(), - T::SortedListProvider::count() + T::VoterList::count() ); outcome @@ -827,16 +827,16 @@ impl Pallet { pub fn do_add_validator(who: &T::AccountId, prefs: ValidatorPrefs) { if !Validators::::contains_key(who) { // maybe update sorted list. - let _ = T::SortedListProvider::on_insert(who.clone(), Self::weight_of(who)) + let _ = T::VoterList::on_insert(who.clone(), Self::weight_of(who)) .defensive_unwrap_or_default(); } Validators::::insert(who, prefs); debug_assert_eq!( Nominators::::count() + Validators::::count(), - T::SortedListProvider::count() + T::VoterList::count() ); - debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); } /// This function will remove a validator from the `Validators` storage map. @@ -849,16 +849,16 @@ impl Pallet { pub fn do_remove_validator(who: &T::AccountId) -> bool { let outcome = if Validators::::contains_key(who) { Validators::::remove(who); - T::SortedListProvider::on_remove(who); + T::VoterList::on_remove(who); true } else { false }; - debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); debug_assert_eq!( Nominators::::count() + Validators::::count(), - T::SortedListProvider::count() + T::VoterList::count() ); outcome @@ -988,7 +988,7 @@ impl ElectionDataProvider for Pallet { >::remove_all(); >::remove_all(); - T::SortedListProvider::unsafe_clear(); + T::VoterList::unsafe_clear(); } #[cfg(feature = "runtime-benchmarks")] diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index c7b848540c362..306bd34390d82 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -17,7 +17,7 @@ //! Staking FRAME Pallet. -use frame_election_provider_support::SortedListProvider; +use frame_election_provider_support::{SortedListProvider, VoteWeight}; use frame_support::{ dispatch::Codec, pallet_prelude::*, @@ -163,13 +163,12 @@ pub mod pallet { /// After the threshold is reached a new era will be forced. type OffendingValidatorsThreshold: Get; - /// Something that can provide a sorted list of voters in a somewhat sorted way. The - /// original use case for this was designed with `pallet_bags_list::Pallet` in mind. If - /// the bags-list is not desired, [`impls::UseNominatorsMap`] is likely the desired option. - type SortedListProvider: SortedListProvider< - Self::AccountId, - Score = frame_election_provider_support::VoteWeight, - >; + /// Something that provides a best-effort sorted list of voters aka electing nominators, + /// used for NPoS election. + /// + /// The changes to nominators are reported to this. Moreover, each validator's self-vote is + /// also reported as one independent vote. + type VoterList: SortedListProvider; /// The maximum number of `unlocking` chunks a [`StakingLedger`] can have. Effectively /// determines how many unique eras a staker may be unbonding in. @@ -584,9 +583,9 @@ pub mod pallet { }); } - // all voters are reported to the `SortedListProvider`. + // all voters are reported to the `VoterList`. assert_eq!( - T::SortedListProvider::count(), + T::VoterList::count(), Nominators::::count() + Validators::::count(), "not all genesis stakers were inserted into sorted list provider, something is wrong." ); @@ -837,9 +836,9 @@ pub mod pallet { // NOTE: ledger must be updated prior to calling `Self::weight_of`. Self::update_ledger(&controller, &ledger); // update this staker in the sorted list, if they exist in it. - if T::SortedListProvider::contains(&stash) { - T::SortedListProvider::on_update(&stash, Self::weight_of(&ledger.stash)); - debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); + if T::VoterList::contains(&stash) { + T::VoterList::on_update(&stash, Self::weight_of(&ledger.stash)); + debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); } Self::deposit_event(Event::::Bonded(stash.clone(), extra)); @@ -920,8 +919,8 @@ pub mod pallet { Self::update_ledger(&controller, &ledger); // update this staker in the sorted list, if they exist in it. - if T::SortedListProvider::contains(&ledger.stash) { - T::SortedListProvider::on_update(&ledger.stash, Self::weight_of(&ledger.stash)); + if T::VoterList::contains(&ledger.stash) { + T::VoterList::on_update(&ledger.stash, Self::weight_of(&ledger.stash)); } Self::deposit_event(Event::::Unbonded(ledger.stash, value)); @@ -1403,8 +1402,8 @@ pub mod pallet { // NOTE: ledger must be updated prior to calling `Self::weight_of`. Self::update_ledger(&controller, &ledger); - if T::SortedListProvider::contains(&ledger.stash) { - T::SortedListProvider::on_update(&ledger.stash, Self::weight_of(&ledger.stash)); + if T::VoterList::contains(&ledger.stash) { + T::VoterList::on_update(&ledger.stash, Self::weight_of(&ledger.stash)); } let removed_chunks = 1u32 // for the case where the last iterated chunk is not removed diff --git a/frame/staking/src/testing_utils.rs b/frame/staking/src/testing_utils.rs index 8e6bd88468930..5f9f378b10619 100644 --- a/frame/staking/src/testing_utils.rs +++ b/frame/staking/src/testing_utils.rs @@ -38,11 +38,11 @@ const SEED: u32 = 0; pub fn clear_validators_and_nominators() { Validators::::remove_all(); - // whenever we touch nominators counter we should update `T::SortedListProvider` as well. + // whenever we touch nominators counter we should update `T::VoterList` as well. Nominators::::remove_all(); // NOTE: safe to call outside block production - T::SortedListProvider::unsafe_clear(); + T::VoterList::unsafe_clear(); } /// Grab a funded user. diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index fd5ffc9c5791c..13b46c869ba73 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4111,7 +4111,7 @@ mod election_data_provider { .set_status(41, StakerStatus::Validator) .build_and_execute(|| { // sum of all nominators who'd be voters (1), plus the self-votes (4). - assert_eq!(::SortedListProvider::count(), 5); + assert_eq!(::VoterList::count(), 5); // if limits is less.. assert_eq!(Staking::voters(Some(1)).unwrap().len(), 1); @@ -4145,7 +4145,7 @@ mod election_data_provider { .build_and_execute(|| { // all voters ordered by stake, assert_eq!( - ::SortedListProvider::iter().collect::>(), + ::VoterList::iter().collect::>(), vec![61, 71, 81, 11, 21, 31] ); @@ -4187,7 +4187,7 @@ mod election_data_provider { .build_and_execute(|| { // given our voters ordered by stake, assert_eq!( - ::SortedListProvider::iter().collect::>(), + ::VoterList::iter().collect::>(), vec![11, 21, 31, 61, 71, 81] ); @@ -4734,10 +4734,10 @@ mod sorted_list_provider { // given let pre_insert_voter_count = (Nominators::::count() + Validators::::count()) as u32; - assert_eq!(::SortedListProvider::count(), pre_insert_voter_count); + assert_eq!(::VoterList::count(), pre_insert_voter_count); assert_eq!( - ::SortedListProvider::iter().collect::>(), + ::VoterList::iter().collect::>(), vec![11, 21, 31, 101] ); @@ -4745,10 +4745,10 @@ mod sorted_list_provider { assert_ok!(Staking::nominate(Origin::signed(100), vec![41])); // then counts don't change - assert_eq!(::SortedListProvider::count(), pre_insert_voter_count); + assert_eq!(::VoterList::count(), pre_insert_voter_count); // and the list is the same assert_eq!( - ::SortedListProvider::iter().collect::>(), + ::VoterList::iter().collect::>(), vec![11, 21, 31, 101] ); }); @@ -4760,23 +4760,17 @@ mod sorted_list_provider { // given let pre_insert_voter_count = (Nominators::::count() + Validators::::count()) as u32; - assert_eq!(::SortedListProvider::count(), pre_insert_voter_count); + assert_eq!(::VoterList::count(), pre_insert_voter_count); - assert_eq!( - ::SortedListProvider::iter().collect::>(), - vec![11, 21, 31] - ); + assert_eq!(::VoterList::iter().collect::>(), vec![11, 21, 31]); // when account 11 re-validates assert_ok!(Staking::validate(Origin::signed(10), Default::default())); // then counts don't change - assert_eq!(::SortedListProvider::count(), pre_insert_voter_count); + assert_eq!(::VoterList::count(), pre_insert_voter_count); // and the list is the same - assert_eq!( - ::SortedListProvider::iter().collect::>(), - vec![11, 21, 31] - ); + assert_eq!(::VoterList::iter().collect::>(), vec![11, 21, 31]); }); } } From 4c9ef4b8b087a52166ac0414e3002b14b53abd80 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 11 Mar 2022 14:52:51 +0000 Subject: [PATCH 13/19] Fix all build and docs --- bin/node/runtime/src/lib.rs | 4 +- frame/babe/src/mock.rs | 2 +- frame/bags-list/remote-tests/src/lib.rs | 2 +- frame/bags-list/remote-tests/src/migration.rs | 11 +- frame/bags-list/remote-tests/src/snapshot.rs | 5 +- frame/grandpa/src/mock.rs | 2 +- frame/offences/benchmarking/src/mock.rs | 2 +- frame/session/benchmarking/src/mock.rs | 2 +- frame/session/src/migrations/v1.rs | 4 +- frame/staking/src/migrations.rs | 101 ++++++++++-------- frame/staking/src/pallet/impls.rs | 4 +- 11 files changed, 74 insertions(+), 65 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 8c7a20af15683..6f4af10a13a89 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -555,9 +555,7 @@ impl pallet_staking::Config for Runtime { type OffendingValidatorsThreshold = OffendingValidatorsThreshold; type ElectionProvider = ElectionProviderMultiPhase; type GenesisElectionProvider = onchain::OnChainSequentialPhragmen; - // Alternatively, use pallet_staking::UseNominatorsMap to just use the nominators map. - // Note that the aforementioned does not scale to a very large number of nominators. - type SortedListProvider = BagsList; + type VoterList = BagsList; type MaxUnlockingChunks = ConstU32<32>; type WeightInfo = pallet_staking::weights::SubstrateWeight; type BenchmarkingConfig = StakingBenchmarkingConfig; diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index 802a39c1f1bf8..e74288577c9ef 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -197,7 +197,7 @@ impl pallet_staking::Config for Test { type NextNewSession = Session; type ElectionProvider = onchain::OnChainSequentialPhragmen; type GenesisElectionProvider = Self::ElectionProvider; - type SortedListProvider = pallet_staking::UseNominatorsAndValidatorsMap; + type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; type MaxUnlockingChunks = ConstU32<32>; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; type WeightInfo = (); diff --git a/frame/bags-list/remote-tests/src/lib.rs b/frame/bags-list/remote-tests/src/lib.rs index 83c322f93134e..caf7a2a547e09 100644 --- a/frame/bags-list/remote-tests/src/lib.rs +++ b/frame/bags-list/remote-tests/src/lib.rs @@ -48,7 +48,7 @@ pub fn display_and_check_bags(currency_unit: u64, currency_na let min_nominator_bond = >::get(); log::info!(target: LOG_TARGET, "min nominator bond is {:?}", min_nominator_bond); - let voter_list_count = ::SortedListProvider::count(); + let voter_list_count = ::VoterList::count(); // go through every bag to track the total number of voters within bags and log some info about // how voters are distributed within the bags. diff --git a/frame/bags-list/remote-tests/src/migration.rs b/frame/bags-list/remote-tests/src/migration.rs index 4d5169fcc6dfa..c4cd73c45d377 100644 --- a/frame/bags-list/remote-tests/src/migration.rs +++ b/frame/bags-list/remote-tests/src/migration.rs @@ -17,7 +17,6 @@ //! Test to check the migration of the voter bag. use crate::{RuntimeT, LOG_TARGET}; -use frame_election_provider_support::SortedListProvider; use frame_support::traits::PalletInfoAccess; use pallet_staking::Nominators; use remote_externalities::{Builder, Mode, OnlineConfig}; @@ -45,16 +44,16 @@ pub async fn execute( let pre_migrate_nominator_count = >::iter().count() as u32; log::info!(target: LOG_TARGET, "Nominator count: {}", pre_migrate_nominator_count); - // run the actual migration, - let moved = ::SortedListProvider::unsafe_regenerate( + use frame_election_provider_support::SortedListProvider; + // run the actual migration + let moved = ::VoterList::unsafe_regenerate( pallet_staking::Nominators::::iter().map(|(n, _)| n), pallet_staking::Pallet::::weight_of_fn(), ); log::info!(target: LOG_TARGET, "Moved {} nominators", moved); - let voter_list_len = - ::SortedListProvider::iter().count() as u32; - let voter_list_count = ::SortedListProvider::count(); + let voter_list_len = ::VoterList::iter().count() as u32; + let voter_list_count = ::VoterList::count(); // and confirm it is equal to the length of the `VoterList`. assert_eq!(pre_migrate_nominator_count, voter_list_len); assert_eq!(pre_migrate_nominator_count, voter_list_count); diff --git a/frame/bags-list/remote-tests/src/snapshot.rs b/frame/bags-list/remote-tests/src/snapshot.rs index 241b64b366117..e9af726e78dc6 100644 --- a/frame/bags-list/remote-tests/src/snapshot.rs +++ b/frame/bags-list/remote-tests/src/snapshot.rs @@ -16,6 +16,7 @@ //! Test to execute the snapshot using the voter bag. +use frame_election_provider_support::SortedListProvider; use frame_support::traits::PalletInfoAccess; use remote_externalities::{Builder, Mode, OnlineConfig}; use sp_runtime::{traits::Block as BlockT, DeserializeOwned}; @@ -48,11 +49,11 @@ pub async fn execute .unwrap(); ext.execute_with(|| { - use frame_election_provider_support::{ElectionDataProvider, SortedListProvider}; + use frame_election_provider_support::ElectionDataProvider; log::info!( target: crate::LOG_TARGET, "{} nodes in bags list.", - ::SortedListProvider::count(), + ::VoterList::count(), ); let voters = diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index 98f95f13ac23f..6490a2b6992bf 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -205,7 +205,7 @@ impl pallet_staking::Config for Test { type NextNewSession = Session; type ElectionProvider = onchain::OnChainSequentialPhragmen; type GenesisElectionProvider = Self::ElectionProvider; - type SortedListProvider = pallet_staking::UseNominatorsAndValidatorsMap; + type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; type MaxUnlockingChunks = ConstU32<32>; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; type WeightInfo = (); diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index 5858dfe548e63..4359b7745ddd6 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -175,7 +175,7 @@ impl pallet_staking::Config for Test { type OffendingValidatorsThreshold = (); type ElectionProvider = onchain::OnChainSequentialPhragmen; type GenesisElectionProvider = Self::ElectionProvider; - type SortedListProvider = pallet_staking::UseNominatorsAndValidatorsMap; + type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; type MaxUnlockingChunks = ConstU32<32>; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; type WeightInfo = (); diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs index e77f6d5a4af9b..5ebc75245630c 100644 --- a/frame/session/benchmarking/src/mock.rs +++ b/frame/session/benchmarking/src/mock.rs @@ -182,7 +182,7 @@ impl pallet_staking::Config for Test { type ElectionProvider = onchain::OnChainSequentialPhragmen; type GenesisElectionProvider = Self::ElectionProvider; type MaxUnlockingChunks = ConstU32<32>; - type SortedListProvider = pallet_staking::UseNominatorsAndValidatorsMap; + type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; type WeightInfo = (); } diff --git a/frame/session/src/migrations/v1.rs b/frame/session/src/migrations/v1.rs index 2a69cd6d6a550..3c687ea7d9d66 100644 --- a/frame/session/src/migrations/v1.rs +++ b/frame/session/src/migrations/v1.rs @@ -87,7 +87,7 @@ pub fn migrate(sp_std::marker::PhantomData); -impl OnRuntimeUpgrade for InjectValidatorsIntoVoterList { - fn on_runtime_upgrade() -> Weight { - if StorageVersion::::get() == Releases::V8_0_0 { - let weight_of_cached = Pallet::::weight_of_fn(); - for (v, _) in Validators::::iter() { - let weight = weight_of_cached(&v); - let _ = T::VoterList::on_insert(v.clone(), weight).map_err(|err| { - log!(warn, "failed to insert {:?} into VoterList: {:?}", v, err) - }); - } +pub mod v9 { + use super::*; - StorageVersion::::put(crate::Releases::V9_0_0); - T::BlockWeights::get().max_block - } else { - log!( - warn, - "InjectValidatorsIntoVoterList being executed on the wrong storage \ + /// Migration implementation that injects all validators into sorted list. + /// + /// This is only useful for chains that started their `VoterList` just based on nominators. + pub struct InjectValidatorsIntoVoterList(sp_std::marker::PhantomData); + impl OnRuntimeUpgrade for InjectValidatorsIntoVoterList { + fn on_runtime_upgrade() -> Weight { + if StorageVersion::::get() == Releases::V8_0_0 { + let prev_count = T::VoterList::count(); + let weight_of_cached = Pallet::::weight_of_fn(); + for (v, _) in Validators::::iter() { + let weight = weight_of_cached(&v); + let _ = T::VoterList::on_insert(v.clone(), weight).map_err(|err| { + log!(warn, "failed to insert {:?} into VoterList: {:?}", v, err) + }); + } + + log!( + info, + "injected a total of {} new voters, prev count: {} next count: {}", + Validators::::count(), + prev_count, + T::VoterList::count(), + ); + StorageVersion::::put(crate::Releases::V9_0_0); + T::BlockWeights::get().max_block + } else { + log!( + warn, + "InjectValidatorsIntoVoterList being executed on the wrong storage \ version, expected Releases::V8_0_0" - ); - T::DbWeight::get().reads(1) + ); + T::DbWeight::get().reads(1) + } } - } - #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<(), &'static str> { - use frame_support::traits::OnRuntimeUpgradeHelpersExt; - frame_support::ensure!( - StorageVersion::::get() == crate::Releases::V8_0_0, - "must upgrade linearly" - ); + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + use frame_support::traits::OnRuntimeUpgradeHelpersExt; + frame_support::ensure!( + StorageVersion::::get() == crate::Releases::V8_0_0, + "must upgrade linearly" + ); - let prev_count = T::VoterList::count(); - Self::set_temp_storage(prev_count, "prev"); - Ok(()) - } + let prev_count = T::VoterList::count(); + Self::set_temp_storage(prev_count, "prev"); + Ok(()) + } - #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { - use frame_support::traits::OnRuntimeUpgradeHelpersExt; - let post_count = T::VoterList::count(); - let prev_count = Self::get_temp_storage::("prev").unwrap(); - let validators = Validators::::count(); - assert!(post_count == prev_count + validators); - Ok(()) + #[cfg(feature = "try-runtime")] + fn post_upgrade() -> Result<(), &'static str> { + use frame_support::traits::OnRuntimeUpgradeHelpersExt; + let post_count = T::VoterList::count(); + let prev_count = Self::get_temp_storage::("prev").unwrap(); + let validators = Validators::::count(); + assert!(post_count == prev_count + validators); + Ok(()) + } } } pub mod v8 { + use crate::{Config, Nominators, Pallet, StorageVersion, Weight}; use frame_election_provider_support::SortedListProvider; use frame_support::traits::Get; - use crate::{Config, Nominators, Pallet, StorageVersion, Weight}; - #[cfg(feature = "try-runtime")] pub fn pre_migrate() -> Result<(), &'static str> { frame_support::ensure!( @@ -88,7 +99,7 @@ pub mod v8 { Ok(()) } - /// Migration to sorted [`VoterList`]. + /// Migration to sorted `VoterList`. pub fn migrate() -> Weight { if StorageVersion::::get() == crate::Releases::V7_0_0 { crate::log!(info, "migrating staking to Releases::V8_0_0"); diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 6ff125060a70f..d976ff6e2dcc0 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -769,7 +769,7 @@ impl Pallet { } /// This function will add a nominator to the `Nominators` storage map, - /// and [`VoterList`]. + /// and `VoterList`. /// /// If the nominator already exists, their nominations will be updated. /// @@ -792,7 +792,7 @@ impl Pallet { } /// This function will remove a nominator from the `Nominators` storage map, - /// and [`VoterList`]. + /// and `VoterList`. /// /// Returns true if `who` was removed from `Nominators`, otherwise false. /// From b311d039fd245710503ad1831c7f433de9662971 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 11 Mar 2022 14:58:42 +0000 Subject: [PATCH 14/19] check post migration --- frame/staking/src/migrations.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index a8d11df80b8fb..96c905f4e5942 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -41,11 +41,12 @@ pub mod v9 { log!( info, - "injected a total of {} new voters, prev count: {} next count: {}", + "injected a total of {} new voters, prev count: {} next count: {}, updating to version 9", Validators::::count(), prev_count, T::VoterList::count(), ); + StorageVersion::::put(crate::Releases::V9_0_0); T::BlockWeights::get().max_block } else { @@ -78,6 +79,11 @@ pub mod v9 { let prev_count = Self::get_temp_storage::("prev").unwrap(); let validators = Validators::::count(); assert!(post_count == prev_count + validators); + + frame_support::ensure!( + StorageVersion::::get() == crate::Releases::V9_0_0, + "must upgrade " + ); Ok(()) } } From 8dcc94cfce9cf0fd3353bae12c3435e31c060055 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 23 Mar 2022 22:49:30 +0000 Subject: [PATCH 15/19] get everything going --- frame/bags-list/src/lib.rs | 7 + frame/bags-list/src/list/mod.rs | 29 +++ frame/bags-list/src/tests.rs | 20 ++ .../election-provider-multi-phase/src/mock.rs | 12 +- frame/election-provider-support/src/lib.rs | 58 ++++- .../election-provider-support/src/onchain.rs | 11 +- frame/staking/src/mock.rs | 4 + frame/staking/src/pallet/impls.rs | 208 ++++++++++++------ frame/staking/src/pallet/mod.rs | 34 ++- frame/staking/src/slashing.rs | 8 +- frame/staking/src/tests.rs | 196 ++++++++--------- frame/support/src/storage/bounded_vec.rs | 13 +- 12 files changed, 401 insertions(+), 199 deletions(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index c502245409fdb..f6c9cde1469ea 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -278,6 +278,13 @@ impl, I: 'static> SortedListProvider for Pallet Box::new(List::::iter().map(|n| n.id().clone())) } + fn iter_from( + start: &T::AccountId, + ) -> Result>, Self::Error> { + let iter = List::::iter_from(start)?; + Ok(Box::new(iter.map(|n| n.id().clone()))) + } + fn count() -> u32 { ListNodes::::count() } diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 4921817c7e146..530e3502aa7c2 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -42,6 +42,8 @@ use sp_std::{ pub enum Error { /// A duplicate id has been detected. Duplicate, + /// Given node id was not found. + NodeNotFound, } #[cfg(test)] @@ -244,6 +246,33 @@ impl, I: 'static> List { iter.filter_map(Bag::get).flat_map(|bag| bag.iter()) } + /// Same as `iter`, but we start from a specific node. + /// + /// All items after this node are returned, excluding `start` itself. + pub(crate) fn iter_from( + start: &T::AccountId, + ) -> Result>, Error> { + // We chain two iterators: + // 1. from the given `start` till the end of the bag + // 2. all the bags that come after `start`'s bag. + + let start_node = Node::::get(start).ok_or(Error::NodeNotFound)?; + let start_node_upper = start_node.bag_upper; + let start_bag = sp_std::iter::successors(start_node.next(), |prev| prev.next()); + + let thresholds = T::BagThresholds::get(); + let idx = thresholds.partition_point(|&threshold| start_node_upper > threshold); + let leftover_bags = thresholds + .into_iter() + .take(idx) + .copied() + .rev() + .filter_map(Bag::get) + .flat_map(|bag| bag.iter()); + + Ok(start_bag.chain(leftover_bags)) + } + /// Insert several ids into the appropriate bags in the list. Continues with insertions /// if duplicates are detected. /// diff --git a/frame/bags-list/src/tests.rs b/frame/bags-list/src/tests.rs index 99396c9cbb3e3..f2ca7296e8a3b 100644 --- a/frame/bags-list/src/tests.rs +++ b/frame/bags-list/src/tests.rs @@ -458,6 +458,26 @@ mod sorted_list_provider { }); } + #[test] + fn iter_from_works() { + ExtBuilder::default().add_ids(vec![(5, 5), (6, 15)]).build_and_execute(|| { + // given + assert_eq!( + List::::get_bags(), + vec![(10, vec![1, 5]), (20, vec![6]), (1000, vec![2, 3, 4])] + ); + + assert_eq!(BagsList::iter_from(&2).unwrap().collect::>(), vec![3, 4, 6, 1, 5]); + assert_eq!(BagsList::iter_from(&3).unwrap().collect::>(), vec![4, 6, 1, 5]); + assert_eq!(BagsList::iter_from(&4).unwrap().collect::>(), vec![6, 1, 5]); + assert_eq!(BagsList::iter_from(&6).unwrap().collect::>(), vec![1, 5]); + assert_eq!(BagsList::iter_from(&1).unwrap().collect::>(), vec![5]); + assert!(BagsList::iter_from(&5).unwrap().collect::>().is_empty()); + + assert_storage_noop!(assert!(BagsList::iter_from(&8).is_err())); + }); + } + #[test] fn count_works() { ExtBuilder::default().build_and_execute(|| { diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 89b5b72565dcb..2486a97e2705f 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -18,7 +18,7 @@ use super::*; use crate as multi_phase; use frame_election_provider_support::{ - data_provider, onchain, ElectionDataProvider, NposSolution, SequentialPhragmen, + data_provider, onchain, ElectionDataProvider, NposSolution, PageIndex, SequentialPhragmen, }; pub use frame_support::{assert_noop, assert_ok}; use frame_support::{ @@ -442,7 +442,11 @@ impl ElectionDataProvider for StakingMock { type BlockNumber = u64; type MaxVotesPerVoter = MaxNominations; - fn electable_targets(maybe_max_len: Option) -> data_provider::Result> { + fn electable_targets_paged( + maybe_max_len: Option, + remaining: PageIndex, + ) -> data_provider::Result> { + assert_eq!(remaining, 0, "this pallet only works with single page snapshots"); let targets = Targets::get(); if maybe_max_len.map_or(false, |max_len| targets.len() > max_len) { @@ -452,9 +456,11 @@ impl ElectionDataProvider for StakingMock { Ok(targets) } - fn electing_voters( + fn electing_voters_paged( maybe_max_len: Option, + remaining: PageIndex, ) -> data_provider::Result>> { + assert_eq!(remaining, 0, "this pallet only works with single page snapshots"); let mut voters = Voters::get(); if let Some(max_len) = maybe_max_len { voters.truncate(max_len) diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 81fd841a6dc47..37876bd4fa8bb 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -103,6 +103,7 @@ //! type AccountId = AccountId; //! type BlockNumber = BlockNumber; //! type MaxVotesPerVoter = ConstU32<1>; +//! type Pages = ConstU32<1>; //! //! fn desired_targets() -> data_provider::Result { //! Ok(1) @@ -112,7 +113,7 @@ //! { //! Ok(Default::default()) //! } -//! fn electable_targets(maybe_max_len: Option) -> data_provider::Result> { +//! fn electable_targets_paged(maybe_max_len: Option, _: PageIndex) -> data_provider::Result> { //! Ok(vec![10, 20, 30]) //! } //! fn next_election_prediction(now: BlockNumber) -> BlockNumber { @@ -169,7 +170,10 @@ pub mod onchain; pub mod traits; use codec::{Decode, Encode}; -use frame_support::{traits::Get, BoundedVec, RuntimeDebug}; +use frame_support::{ + traits::{DefensiveSaturating, Get}, + BoundedVec, RuntimeDebug, +}; use sp_runtime::traits::Bounded; use sp_std::{fmt::Debug, prelude::*}; @@ -183,6 +187,13 @@ pub use sp_npos_elections::{ }; pub use traits::NposSolution; +/// The index used to indicate the page number. +/// +/// A u8 would have probably been enough until the end of universe, but since we use this as the +/// bound of `BoundedVec` and similar, using `u32` will save us some hassle. +// TODO: https://github.com/paritytech/substrate/issues/11076 +pub type PageIndex = u32; + // re-export for the solution macro, with the dependencies of the macro. #[doc(hidden)] pub use codec; @@ -265,6 +276,10 @@ pub trait ElectionDataProvider { /// Maximum number of votes per voter that this data provider is providing. type MaxVotesPerVoter: Get; + /// The number of pages that this data provider is expected to be able to provide. An + /// implementation could use this to verify the value of `remaining: PageIndex`. + type Pages: Get; + /// All possible targets for the election, i.e. the targets that could become elected, thus /// "electable". /// @@ -273,8 +288,9 @@ pub trait ElectionDataProvider { /// /// This should be implemented as a self-weighing function. The implementor should register its /// appropriate weight at the end of execution with the system pallet directly. - fn electable_targets( + fn electable_targets_paged( maybe_max_len: Option, + remaining_pages: PageIndex, ) -> data_provider::Result>; /// All the voters that participate in the election, thus "electing". @@ -286,7 +302,22 @@ pub trait ElectionDataProvider { /// /// This should be implemented as a self-weighing function. The implementor should register its /// appropriate weight at the end of execution with the system pallet directly. - fn electing_voters(maybe_max_len: Option) -> data_provider::Result>>; + fn electing_voters_paged( + maybe_max_len: Option, + remaining_pages: PageIndex, + ) -> data_provider::Result>>; + + /// Same as [`Self::electable_targets_paged`], but the page 0 is assumed. + fn electable_targets( + maybe_max_len: Option, + ) -> data_provider::Result> { + Self::electable_targets_paged(maybe_max_len, 0) + } + + /// Same as [`Self::electing_voters_paged`], but the page 0 is assumed. + fn electing_voters(maybe_max_len: Option) -> data_provider::Result>> { + Self::electing_voters_paged(maybe_max_len, 0) + } /// The number of targets to elect. /// @@ -308,6 +339,20 @@ pub trait ElectionDataProvider { /// This is only useful for stateful election providers. fn next_election_prediction(now: Self::BlockNumber) -> Self::BlockNumber; + /// Shorthand for "most significant page". + /// + /// The least significant, aka "first" page that this election provider is expected to receive. + fn msp() -> PageIndex { + Self::Pages::get().defensive_saturating_sub(1) + } + + /// Shorthand for "least significant page". + /// + /// The most significant, aka "first" page that this election provider is expected to receive. + fn lsp() -> PageIndex { + 0 + } + /// Utility function only to be used in benchmarking scenarios, to be implemented optionally, /// else a noop. #[cfg(any(feature = "runtime-benchmarks", test))] @@ -427,6 +472,11 @@ pub trait SortedListProvider { /// An iterator over the list, which can have `take` called on it. fn iter() -> Box>; + /// Returns an iterator over the list, starting from the given voter. + /// + /// May return an error if `start` is invalid. + fn iter_from(start: &AccountId) -> Result>, Self::Error>; + /// The current count of ids in the list. fn count() -> u32; diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 7d845c2dc5ab3..d38b629f63800 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -191,14 +191,18 @@ mod tests { use frame_support::{bounded_vec, traits::ConstU32}; use super::*; - use crate::{data_provider, VoterOf}; + use crate::{data_provider, PageIndex, VoterOf}; pub struct DataProvider; impl ElectionDataProvider for DataProvider { type AccountId = AccountId; type BlockNumber = BlockNumber; type MaxVotesPerVoter = ConstU32<2>; - fn electing_voters(_: Option) -> data_provider::Result>> { + fn electing_voters_paged( + _: Option, + remaining: PageIndex, + ) -> data_provider::Result>> { + assert_eq!(remaining, 0); Ok(vec![ (1, 10, bounded_vec![10, 20]), (2, 20, bounded_vec![30, 20]), @@ -206,7 +210,8 @@ mod tests { ]) } - fn electable_targets(_: Option) -> data_provider::Result> { + fn electable_targets_paged(_: Option, remaining: PageIndex) -> data_provider::Result> { + assert_eq!(remaining, 0); Ok(vec![10, 20, 30]) } diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index bb71232c34673..f95a6fb1c96fe 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -235,6 +235,7 @@ const THRESHOLDS: [sp_npos_elections::VoteWeight; 9] = parameter_types! { pub static BagThresholds: &'static [sp_npos_elections::VoteWeight] = &THRESHOLDS; pub static MaxNominations: u32 = 16; + pub static Pages: u32 = 1; } impl pallet_bags_list::Config for Test { @@ -272,6 +273,7 @@ impl crate::pallet::pallet::Config for Test { type GenesisElectionProvider = Self::ElectionProvider; // NOTE: consider a macro and use `UseNominatorsAndValidatorsMap` as well. type VoterList = BagsList; + type Pages = Pages; type MaxUnlockingChunks = ConstU32<32>; type BenchmarkingConfig = TestBenchmarkingConfig; type WeightInfo = (); @@ -430,6 +432,8 @@ impl ExtBuilder { (71, self.balance_factor * 2000), (80, self.balance_factor), (81, self.balance_factor * 2000), + (90, self.balance_factor), + (91, self.balance_factor * 2000), // This allows us to have a total_payout different from 0. (999, 1_000_000_000_000), ], diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 9d5a3ed484184..0ecda03cf9177 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -18,14 +18,14 @@ //! Implementations for the Staking FRAME Pallet. use frame_election_provider_support::{ - data_provider, ElectionDataProvider, ElectionProvider, ScoreProvider, SortedListProvider, - Supports, VoteWeight, VoterOf, + data_provider, ElectionDataProvider, ElectionProvider, PageIndex, ScoreProvider, + SortedListProvider, Supports, VoteWeight, VoterOf, }; use frame_support::{ pallet_prelude::*, traits::{ - Currency, CurrencyToVote, Defensive, EstimateNextNewSession, Get, Imbalance, - LockableCurrency, OnUnbalanced, UnixTime, WithdrawReasons, + Currency, CurrencyToVote, Defensive, DefensiveSaturating, EstimateNextNewSession, Get, + Imbalance, LockableCurrency, OnUnbalanced, UnixTime, WithdrawReasons, }, weights::{Weight, WithPostDispatchInfo}, }; @@ -218,12 +218,17 @@ impl Pallet { } /// Chill a stash account. - pub(crate) fn chill_stash(stash: &T::AccountId) { - let chilled_as_validator = Self::do_remove_validator(stash); - let chilled_as_nominator = Self::do_remove_nominator(stash); - if chilled_as_validator || chilled_as_nominator { + /// + /// Returns `Ok(())` if it was actually chilled (either from a validator or a nominator), and + /// `Error` otherwise. + pub(crate) fn try_chill_stash(stash: &T::AccountId) -> Result<(), Error> { + let chilled_as_validator = Self::try_remove_validator(stash); + let chilled_as_nominator = Self::try_remove_nominator(stash); + if chilled_as_validator.is_ok() || chilled_as_nominator.is_ok() { Self::deposit_event(Event::::Chilled(stash.clone())); } + + chilled_as_validator.or(chilled_as_nominator) } /// Actually make a payment to a staker. This uses the currency's reward function @@ -574,8 +579,10 @@ impl Pallet { >::remove(&controller); >::remove(stash); - Self::do_remove_validator(stash); - Self::do_remove_nominator(stash); + // dropping result is justified: we just want to make sure they are no longer + // nominator/validator. + let _ = Self::try_remove_validator(stash); + let _ = Self::try_remove_nominator(stash); frame_system::Pallet::::dec_consumers(stash); @@ -655,35 +662,43 @@ impl Pallet { SlashRewardFraction::::put(fraction); } - /// Get all of the voters that are eligible for the npos election. - /// - /// `maybe_max_len` can imposes a cap on the number of voters returned; - /// - /// This function is self-weighing as [`DispatchClass::Mandatory`]. + /// Get at most `maybe_max_len` voters ready for the npos election, assuming that `remaining` + /// more calls will be made in the same 'unit of election'. /// - /// ### Slashing - /// - /// All votes that have been submitted before the last non-zero slash of the corresponding - /// target are *auto-chilled*, but still count towards the limit imposed by `maybe_max_len`. - pub fn get_npos_voters(maybe_max_len: Option) -> Vec> { + /// If `remaining > 0`, `LastIteratedNominator` is set to the last item that is returned. + /// Consequently, regardless of the value of `remaining`, if `LastIteratedNominator` exists, it + /// is used as the starting point of the iteration. + pub fn get_npos_voters_page( + maybe_max_len: Option, + maybe_last: Option, + slashing_spans: &BTreeMap, + ) -> (Vec>, u32, u32) { let max_allowed_len = { let all_voter_count = T::VoterList::count() as usize; maybe_max_len.unwrap_or(all_voter_count).min(all_voter_count) }; - let mut all_voters = Vec::<_>::with_capacity(max_allowed_len); + let mut voters = Vec::<_>::with_capacity(max_allowed_len); - // cache a few things. + // cache the total-issuance once in this function let weight_of = Self::weight_of_fn(); - let slashing_spans = >::iter().collect::>(); - let mut voters_seen = 0u32; let mut validators_taken = 0u32; let mut nominators_taken = 0u32; - let mut sorted_voters = T::VoterList::iter(); - while all_voters.len() < max_allowed_len && - voters_seen < (NPOS_MAX_ITERATIONS_COEFFICIENT * max_allowed_len as u32) + let mut sorted_voters = if let Some(last) = maybe_last { + // Defensive: a correctly working pallet-staking will store a last node that is already + // part of of the voter list. If otherwise, we start from the beginning and return + // duplicates, hopefully our beloved `ElectionProvider` can handle duplicate voters + // (substrate implementations do). This is guaranteed by making sure while + // `LastIteratedNominator` is set, no nominator can chill. + T::VoterList::iter_from(&last).defensive_unwrap_or_else(T::VoterList::iter) + } else { + T::VoterList::iter() + }; + + while voters.len() < max_allowed_len && + voters_seen < (max_allowed_len as u32 * NPOS_MAX_ITERATIONS_COEFFICIENT) { let voter = match sorted_voters.next() { Some(voter) => { @@ -696,14 +711,13 @@ impl Pallet { if let Some(Nominations { submitted_in, mut targets, suppressed: _ }) = >::get(&voter) { - // if this voter is a nominator: targets.retain(|stash| { slashing_spans .get(stash) .map_or(true, |spans| submitted_in >= spans.last_nonzero_slash()) }); if !targets.len().is_zero() { - all_voters.push((voter.clone(), weight_of(&voter), targets)); + voters.push((voter.clone(), weight_of(&voter), targets)); nominators_taken.saturating_inc(); } } else if Validators::::contains_key(&voter) { @@ -715,24 +729,24 @@ impl Pallet { .try_into() .expect("`MaxVotesPerVoter` must be greater than or equal to 1"), ); - all_voters.push(self_vote); + voters.push(self_vote); validators_taken.saturating_inc(); } else { - // this can only happen if: 1. there a bug in the bags-list (or whatever is the - // sorted list) logic and the state of the two pallets is no longer compatible, or - // because the nominators is not decodable since they have more nomination than - // `T::MaxNominations`. The latter can rarely happen, and is not really an emergency - // or bug if it does. + // this can only happen if: 1. there a pretty bad bug in the bags-list (or whatever + // is the sorted list) logic and the state of the two pallets is no longer + // compatible, or because the nominators is not decodable since they have more + // nomination than `T::MaxNominations`. This can rarely happen, and is not really an + // emergency or bug if it does. log!( warn, - "DEFENSIVE: invalid item in `VoterList`: {:?}, this nominator probably has too many nominations now", + "DEFENSIVE: invalid item in `SortedListProvider`: {:?}, this nominator probably has too many nominations now", voter - ) + ); } } - // all_voters should have not re-allocated. - debug_assert!(all_voters.capacity() == max_allowed_len); + // voters should have never re-allocated. + debug_assert!(voters.capacity() >= voters.len()); Self::register_weight(T::WeightInfo::get_npos_voters( validators_taken, @@ -740,15 +754,7 @@ impl Pallet { slashing_spans.len() as u32, )); - log!( - info, - "generated {} npos voters, {} from validators and {} nominators", - all_voters.len(), - validators_taken, - nominators_taken - ); - - all_voters + (voters, validators_taken, nominators_taken) } /// Get the targets for an upcoming npos election. @@ -768,14 +774,12 @@ impl Pallet { targets } - /// This function will add a nominator to the `Nominators` storage map, - /// and `VoterList`. + /// This function will add a nominator to the `Nominators` storage map, and `VoterList`. /// /// If the nominator already exists, their nominations will be updated. /// /// NOTE: you must ALWAYS use this function to add nominator or update their targets. Any access - /// to `Nominators` or `VoterList` outside of this function is almost certainly - /// wrong. + /// to `Nominators` or `VoterList` outside of this function is almost certainly wrong. pub fn do_add_nominator(who: &T::AccountId, nominations: Nominations) { if !Nominators::::contains_key(who) { // maybe update sorted list. @@ -794,18 +798,22 @@ impl Pallet { /// This function will remove a nominator from the `Nominators` storage map, /// and `VoterList`. /// - /// Returns true if `who` was removed from `Nominators`, otherwise false. + /// Returns `Ok(())` if `who` was removed from `Nominators`, otherwise `Err(_)`. /// /// NOTE: you must ALWAYS use this function to remove a nominator from the system. Any access to /// `Nominators` or `VoterList` outside of this function is almost certainly /// wrong. - pub fn do_remove_nominator(who: &T::AccountId) -> bool { + pub fn try_remove_nominator(who: &T::AccountId) -> Result<(), Error> { let outcome = if Nominators::::contains_key(who) { + ensure!( + LastIteratedNominator::::get().map_or(true, |last| &last != who), + Error::::TemporarilyNotAllowed + ); Nominators::::remove(who); T::VoterList::on_remove(who); - true + Ok(()) } else { - false + Err(Error::::NotNominator) }; debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); @@ -846,13 +854,13 @@ impl Pallet { /// NOTE: you must ALWAYS use this function to remove a validator from the system. Any access to /// `Validators` or `VoterList` outside of this function is almost certainly /// wrong. - pub fn do_remove_validator(who: &T::AccountId) -> bool { + pub fn try_remove_validator(who: &T::AccountId) -> Result<(), Error> { let outcome = if Validators::::contains_key(who) { Validators::::remove(who); T::VoterList::on_remove(who); - true + Ok(()) } else { - false + Err(Error::::NotValidator) }; debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); @@ -879,21 +887,73 @@ impl ElectionDataProvider for Pallet { type AccountId = T::AccountId; type BlockNumber = BlockNumberFor; type MaxVotesPerVoter = T::MaxNominations; + type Pages = T::Pages; fn desired_targets() -> data_provider::Result { Self::register_weight(T::DbWeight::get().reads(1)); Ok(Self::validator_count()) } - fn electing_voters(maybe_max_len: Option) -> data_provider::Result>> { - // This can never fail -- if `maybe_max_len` is `Some(_)` we handle it. - let voters = Self::get_npos_voters(maybe_max_len); - debug_assert!(maybe_max_len.map_or(true, |max| voters.len() <= max)); + /// implementation notes: Given the loose dynamics of the `ElectionProvider`, namely the fact + /// that number of entries returned per page is dynamic, this implementation is best-effort at + /// best. We try and use the `LastIteratedNominator`, which is always kept as long call with + /// `remaining > 0` are made. Any call with `remaining = 0` will clear this history item, and + /// the next call will be fresh. + /// + /// This function is written defensively: + fn electing_voters_paged( + maybe_max_len: Option, + mut remaining: PageIndex, + ) -> data_provider::Result>> { + remaining = remaining.min(T::Pages::get().defensive_saturating_sub(1)); + // either this is the first call, or `LastIteratedNominator` should exist. + let mut maybe_last = LastIteratedNominator::::get(); + + let is_valid_state = if T::Pages::get() > 1 { + // in the first page, the `LastIteratedNominator` should not exist, in the rest of the + // pages it should + (remaining == Self::msp()) ^ maybe_last.is_some() + } else { + maybe_last.is_none() + }; + + // defensive: since, at least as of now, this function is also used for governance fallback, + // we really really prefer it to NEVER fail. Thus, we play defensive and return just the + // first page if the state of `LastIteratedNominator` is corrupt. + if !is_valid_state { + frame_support::defensive!("corrupt state in staking election data provider"); + maybe_last = None; + } + let slashing_spans = >::iter().collect::>(); + let (voters, _validators_taken, _nominators_taken) = + Self::get_npos_voters_page(maybe_max_len, maybe_last, &slashing_spans); + + log!( + debug, + "generated {} npos voters, {} from validators and {} nominators", + voters.len(), + _validators_taken, + _nominators_taken, + ); + + match remaining { + 0 => LastIteratedNominator::::kill(), + _ => + if let Some(last) = voters.last().map(|(x, _, _)| x) { + LastIteratedNominator::::put(last.clone()) + }, + }; Ok(voters) } - fn electable_targets(maybe_max_len: Option) -> data_provider::Result> { + fn electable_targets_paged( + maybe_max_len: Option, + remaining: PageIndex, + ) -> data_provider::Result> { + if remaining > 0 { + return Err("Cannot support more than a single page of targets") + } let target_count = Validators::::count(); // We can't handle this case yet -- return an error. @@ -1308,7 +1368,6 @@ impl SortedListProvider for UseNominatorsAndValidatorsM type Error = (); type Score = VoteWeight; - /// Returns iterator over voter list, which can have `take` called on it. fn iter() -> Box> { Box::new( Validators::::iter() @@ -1316,6 +1375,25 @@ impl SortedListProvider for UseNominatorsAndValidatorsM .chain(Nominators::::iter().map(|(n, _)| n)), ) } + + fn iter_from( + start: &T::AccountId, + ) -> Result>, Self::Error> { + if Validators::::contains_key(start) { + let start_key = Validators::::hashed_key_for(start); + Ok(Box::new( + Validators::::iter_from(start_key) + .map(|(n, _)| n) + .chain(Nominators::::iter().map(|(x, _)| x)), + )) + } else if Nominators::::contains_key(start) { + let start_key = Nominators::::hashed_key_for(start); + Ok(Box::new(Nominators::::iter_from(start_key).map(|(n, _)| n))) + } else { + Err(()) + } + } + fn count() -> u32 { Nominators::::count().saturating_add(Validators::::count()) } diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 306bd34390d82..d087b290abb47 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -50,7 +50,7 @@ const STAKING_ID: LockIdentifier = *b"staking "; #[frame_support::pallet] pub mod pallet { - use frame_election_provider_support::ElectionDataProvider; + use frame_election_provider_support::{ElectionDataProvider, PageIndex}; use crate::BenchmarkingConfig; @@ -170,6 +170,10 @@ pub mod pallet { /// also reported as one independent vote. type VoterList: SortedListProvider; + /// The number of pages that we expect to return as part of our role as + /// [`ElectionDataProvider`]. + type Pages: Get; + /// The maximum number of `unlocking` chunks a [`StakingLedger`] can have. Effectively /// determines how many unique eras a staker may be unbonding in. #[pallet::constant] @@ -495,6 +499,11 @@ pub mod pallet { #[pallet::storage] pub(crate) type ChillThreshold = StorageValue<_, Percent, OptionQuery>; + /// Last nominator that was being iterated. This is only useful for multi-page snapshot, and + /// does not affect anything else. + #[pallet::storage] + pub(crate) type LastIteratedNominator = StorageValue<_, T::AccountId, OptionQuery>; + #[pallet::genesis_config] pub struct GenesisConfig { pub history_depth: u32, @@ -636,14 +645,16 @@ pub mod pallet { NotController, /// Not a stash account. NotStash, + /// Not a validators account. + NotValidator, + /// Not a nominator account. + NotNominator, /// Stash is already bonded. AlreadyBonded, /// Controller is already paired. AlreadyPaired, /// Targets cannot be empty. EmptyTargets, - /// Duplicate index. - DuplicateIndex, /// Slash record index out of bounds. InvalidSlashIndex, /// Cannot have a validator or nominator role, with value less than the minimum defined by @@ -684,6 +695,8 @@ pub mod pallet { TooManyValidators, /// Commission is too low. Must be at least `MinCommission`. CommissionTooLow, + /// Operation is not permitted at this time. Try again later. + TemporarilyNotAllowed, } #[pallet::hooks] @@ -716,6 +729,8 @@ pub mod pallet { // and that MaxNominations is always greater than 1, since we count on this. assert!(!T::MaxNominations::get().is_zero()); + assert!(T::Pages::get() > 0, "number of pages can be 1 or more"); + sp_std::if_std! { sp_io::TestExternalities::new_empty().execute_with(|| assert!( @@ -1015,7 +1030,7 @@ pub mod pallet { } } - Self::do_remove_nominator(stash); + let _ = Self::try_remove_nominator(stash); Self::do_add_validator(stash, prefs); Ok(()) } @@ -1083,7 +1098,7 @@ pub mod pallet { suppressed: false, }; - Self::do_remove_validator(stash); + let _ = Self::try_remove_validator(stash); Self::do_add_nominator(stash, nominations); Ok(()) } @@ -1103,8 +1118,7 @@ pub mod pallet { pub fn chill(origin: OriginFor) -> DispatchResult { let controller = ensure_signed(origin)?; let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; - Self::chill_stash(&ledger.stash); - Ok(()) + Self::try_chill_stash(&ledger.stash).map_err(Into::into) } /// (Re-)set the payment target for a controller. @@ -1632,8 +1646,7 @@ pub mod pallet { // Otherwise, if caller is the same as the controller, this is just like `chill`. if Nominators::::contains_key(&stash) && Nominators::::get(&stash).is_none() { - Self::chill_stash(&stash); - return Ok(()) + return Self::try_chill_stash(&stash).map_err(Into::into) } if caller != controller { @@ -1663,8 +1676,7 @@ pub mod pallet { ensure!(ledger.active < min_active_bond, Error::::CannotChillOther); } - Self::chill_stash(&stash); - Ok(()) + Self::try_chill_stash(&stash).map_err(Into::into) } /// Force a validator to have at least the minimum commission. This will not affect a diff --git a/frame/staking/src/slashing.rs b/frame/staking/src/slashing.rs index 2f381ad631fe5..862fae9330669 100644 --- a/frame/staking/src/slashing.rs +++ b/frame/staking/src/slashing.rs @@ -281,7 +281,9 @@ pub(crate) fn compute_slash( // chill the validator - it misbehaved in the current span and should // not continue in the next election. also end the slashing span. spans.end_span(params.now); - >::chill_stash(params.stash); + // dropping result is justified: we don't care if they were a validator or not, rather + // that they are certainly no longer a validator. + let _ = >::try_chill_stash(params.stash); } } @@ -316,7 +318,9 @@ fn kick_out_if_recent(params: SlashParams) { if spans.era_span(params.slash_era).map(|s| s.index) == Some(spans.span_index()) { spans.end_span(params.now); - >::chill_stash(params.stash); + // dropping result is justified: we don't care if they were a validator or not, rather + // that they are certainly no longer a validator. + let _ = >::try_chill_stash(params.stash); } let disable_without_slash = params.disable_strategy == DisableStrategy::Always; diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 11dfe3e4777f8..f8e5c5ee21429 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4018,6 +4018,7 @@ fn on_finalize_weight_is_nonzero() { mod election_data_provider { use super::*; use frame_election_provider_support::ElectionDataProvider; + use frame_support::bounded_vec; #[test] fn targets_2sec_block() { @@ -4107,6 +4108,75 @@ mod election_data_provider { }) } + #[test] + fn can_paginate() { + ExtBuilder::default() + .add_staker(61, 60, 500, StakerStatus::Nominator(vec![11])) + .add_staker(71, 70, 500, StakerStatus::Nominator(vec![11])) + .add_staker(81, 80, 500, StakerStatus::Nominator(vec![11])) + .add_staker(91, 90, 500, StakerStatus::Nominator(vec![11])) + .set_status(31, StakerStatus::Idle) + .set_status(41, StakerStatus::Idle) + .build_and_execute(|| { + // we shall have 2 validators and 5 nominators. + assert_eq!(Nominators::::count(), 5); + assert_eq!(Validators::::count(), 2); + + // a correct, 2 page snapshot. + Pages::set(2); + // we have 6 voters in total, get them over two pages, each 4 max. + assert_eq!( + Staking::electing_voters_paged(Some(4), 1).unwrap(), + vec![ + // everything sorted based on stake. + (11, 1000, bounded_vec![11]), + (21, 1000, bounded_vec![21]), + (101, 500, bounded_vec![11, 21]), + (61, 500, bounded_vec![11]), + ] + ); + + assert_eq!(LastIteratedNominator::::get(), Some(61)); + + // tandem: node 61 cannot be chilled in any humanly possible way now. + frame_support::storage::with_transaction(|| { + // normal chilling + assert_noop!( + Staking::chill(Origin::signed(60)), + Error::::TemporarilyNotAllowed + ); + // chill-other with the virtue of being less than min-bond + MinNominatorBond::::put(501); + ChillThreshold::::put(Percent::default()); + MaxNominatorsCount::::put(1); + assert_noop!( + Staking::chill_other(Origin::signed(100), 60), + Error::::TemporarilyNotAllowed + ); + storage::TransactionOutcome::Rollback(()) + }); + // and making this nominator un-decodable. + MaxNominations::set(0); + assert_noop!( + Staking::chill_other(Origin::signed(100), 60), + Error::::TemporarilyNotAllowed + ); + // this needs to reverted, despite being transactional scope, because it is + // thread-local, not storage. + MaxNominations::set(16); + + assert_eq!( + Staking::electing_voters_paged(Some(4), 0).unwrap(), + vec![ + (71, 500, bounded_vec![11]), + (81, 500, bounded_vec![11]), + (91, 500, bounded_vec![11]) + ] + ); + assert_eq!(LastIteratedNominator::::get(), None); + }); + } + #[test] fn respects_snapshot_len_limits() { ExtBuilder::default() @@ -4123,8 +4193,10 @@ mod election_data_provider { // if limit is more. assert_eq!(Staking::electing_voters(Some(55)).unwrap().len(), 5); + assert_eq!(Staking::electing_voters(None).unwrap().len(), 5); // if target limit is more.. + assert_eq!(Staking::electable_targets(None).unwrap().len(), 4); assert_eq!(Staking::electable_targets(Some(6)).unwrap().len(), 4); assert_eq!(Staking::electable_targets(Some(4)).unwrap().len(), 4); @@ -4133,151 +4205,55 @@ mod election_data_provider { Staking::electable_targets(Some(1)).unwrap_err(), "Target snapshot too big" ); - }); + }) } - // Tests the criteria that in `ElectionDataProvider::voters` function, we try to get at most - // `maybe_max_len` voters, and if some of them end up being skipped, we iterate at most `2 * - // maybe_max_len`. #[test] fn only_iterates_max_2_times_max_allowed_len() { ExtBuilder::default() - .nominate(false) + .nominate(true) // add nominator 101, who nominates [11, 21] // the other nominators only nominate 21 .add_staker(61, 60, 2_000, StakerStatus::::Nominator(vec![21])) .add_staker(71, 70, 2_000, StakerStatus::::Nominator(vec![21])) .add_staker(81, 80, 2_000, StakerStatus::::Nominator(vec![21])) + .add_staker(91, 90, 2_000, StakerStatus::::Nominator(vec![21])) .build_and_execute(|| { - // all voters ordered by stake, + // given our nominators and validators, ordered by stake, assert_eq!( ::VoterList::iter().collect::>(), - vec![61, 71, 81, 11, 21, 31] + vec![61, 71, 81, 91, 11, 21, 31, 101] ); + // roll to session 5 run_to_block(25); // slash 21, the only validator nominated by our first 3 nominators add_slash(&21); - // we want 2 voters now, and in maximum we allow 4 iterations. This is what happens: - // 61 is pruned; - // 71 is pruned; - // 81 is pruned; - // 11 is taken; - // we finish since the 2x limit is reached. - assert_eq!( - Staking::electing_voters(Some(2)) - .unwrap() - .iter() - .map(|(stash, _, _)| stash) - .copied() - .collect::>(), - vec![11], - ); - }); - } - - // Even if some of the higher staked nominators are slashed, we still get up to max len voters - // by adding more lower staked nominators. In other words, we assert that we keep on adding - // valid nominators until we reach max len voters; which is opposed to simply stopping after we - // have iterated max len voters, but not adding all of them to voters due to some nominators not - // having valid targets. - #[test] - fn get_max_len_voters_even_if_some_nominators_are_slashed() { - ExtBuilder::default() - .nominate(false) - .add_staker(61, 60, 20, StakerStatus::::Nominator(vec![21])) - .add_staker(71, 70, 10, StakerStatus::::Nominator(vec![11, 21])) - .add_staker(81, 80, 10, StakerStatus::::Nominator(vec![11, 21])) - .build_and_execute(|| { - // given our voters ordered by stake, - assert_eq!( - ::VoterList::iter().collect::>(), - vec![11, 21, 31, 61, 71, 81] - ); - - // we take 4 voters - assert_eq!( - Staking::electing_voters(Some(4)) + assert_eq_uvec!( + Staking::electing_voters(Some(3)) .unwrap() .iter() .map(|(stash, _, _)| stash) .copied() .collect::>(), - vec![11, 21, 31, 61], + // 61, 71, 81, 91 21 are all trimmed out, and since we are allowed to see only + // 3 * 2, we don't reach 101, and we return less than ideal. + vec![11, 31], ); - // roll to session 5 - run_to_block(25); - - // slash 21, the only validator nominated by 61. - add_slash(&21); - - // we take 4 voters; 71 and 81 are replacing the ejected ones. - assert_eq!( + // now, if we ask for 4, we have 8 items to iterate, and will fetch 101 as well. + assert_eq_uvec!( Staking::electing_voters(Some(4)) .unwrap() .iter() .map(|(stash, _, _)| stash) .copied() .collect::>(), - vec![11, 31, 71, 81], + vec![11, 31, 101], ); }); } - - #[test] - fn estimate_next_election_works() { - ExtBuilder::default().session_per_era(5).period(5).build_and_execute(|| { - // first session is always length 0. - for b in 1..20 { - run_to_block(b); - assert_eq!(Staking::next_election_prediction(System::block_number()), 20); - } - - // election - run_to_block(20); - assert_eq!(Staking::next_election_prediction(System::block_number()), 45); - assert_eq!(staking_events().len(), 1); - assert_eq!(*staking_events().last().unwrap(), Event::StakersElected); - - for b in 21..45 { - run_to_block(b); - assert_eq!(Staking::next_election_prediction(System::block_number()), 45); - } - - // election - run_to_block(45); - assert_eq!(Staking::next_election_prediction(System::block_number()), 70); - assert_eq!(staking_events().len(), 3); - assert_eq!(*staking_events().last().unwrap(), Event::StakersElected); - - Staking::force_no_eras(Origin::root()).unwrap(); - assert_eq!(Staking::next_election_prediction(System::block_number()), u64::MAX); - - Staking::force_new_era_always(Origin::root()).unwrap(); - assert_eq!(Staking::next_election_prediction(System::block_number()), 45 + 5); - - Staking::force_new_era(Origin::root()).unwrap(); - assert_eq!(Staking::next_election_prediction(System::block_number()), 45 + 5); - - // Do a fail election - MinimumValidatorCount::::put(1000); - run_to_block(50); - // Election: failed, next session is a new election - assert_eq!(Staking::next_election_prediction(System::block_number()), 50 + 5); - // The new era is still forced until a new era is planned. - assert_eq!(ForceEra::::get(), Forcing::ForceNew); - - MinimumValidatorCount::::put(2); - run_to_block(55); - assert_eq!(Staking::next_election_prediction(System::block_number()), 55 + 25); - assert_eq!(staking_events().len(), 6); - assert_eq!(*staking_events().last().unwrap(), Event::StakersElected); - // The new era has been planned, forcing is changed from `ForceNew` to `NotForcing`. - assert_eq!(ForceEra::::get(), Forcing::NotForcing); - }) - } } #[test] diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index 4e513258f9684..ae51f416214f0 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -585,7 +585,11 @@ where #[cfg(test)] pub mod test { use super::*; - use crate::{bounded_vec, traits::ConstU32, Twox128}; + use crate::{ + bounded_vec, + traits::{ConstU16, ConstU32, ConstU8}, + Twox128, + }; use sp_io::TestExternalities; crate::generate_storage_alias! { Prefix, Foo => Value>> } @@ -887,4 +891,11 @@ pub mod test { assert!(b.try_extend(vec![4, 5, 6].into_iter()).is_err()); assert_eq!(*b, vec![1, 2, 3]); } + + #[test] + fn works_with_u8_u16_u32_as_bound() { + let mut b1: BoundedVec> = bounded_vec![1, 2, 3]; + let mut b2: BoundedVec> = bounded_vec![1, 2, 3]; + let mut b3: BoundedVec> = bounded_vec![1, 2, 3]; + } } From 0d8b1b38b53fee66f0f8c81a94c8ceca84e08015 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 19 Apr 2022 14:41:12 +0200 Subject: [PATCH 16/19] a good round of self-review --- Cargo.lock | 356 +++++++++--------- frame/election-provider-support/Cargo.toml | 1 + frame/election-provider-support/src/lib.rs | 27 +- .../election-provider-support/src/onchain.rs | 6 +- frame/staking/src/pallet/impls.rs | 30 +- 5 files changed, 220 insertions(+), 200 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 84872ce141da4..a9fafd1bbafd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -226,7 +226,7 @@ dependencies = [ "fastrand", "futures-lite", "libc", - "log 0.4.14", + "log 0.4.16", "nb-connect", "once_cell", "parking", @@ -272,7 +272,7 @@ dependencies = [ "futures-lite", "gloo-timers", "kv-log-macro", - "log 0.4.14", + "log 0.4.16", "memchr", "num_cpus", "once_cell", @@ -474,7 +474,7 @@ dependencies = [ "futures 0.3.21", "futures-timer", "hex", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parking_lot 0.12.0", "sc-chain-spec", @@ -519,7 +519,7 @@ dependencies = [ "jsonrpc-core-client", "jsonrpc-derive", "jsonrpc-pubsub", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parking_lot 0.12.0", "sc-rpc", @@ -539,7 +539,7 @@ dependencies = [ "env_logger 0.9.0", "hex", "hex-literal", - "log 0.4.14", + "log 0.4.16", "tiny-keccak", ] @@ -1171,7 +1171,7 @@ dependencies = [ "cranelift-codegen-shared 0.76.0", "cranelift-entity 0.76.0", "gimli 0.25.0", - "log 0.4.14", + "log 0.4.16", "regalloc 0.0.31", "smallvec 1.8.0", "target-lexicon", @@ -1188,7 +1188,7 @@ dependencies = [ "cranelift-codegen-shared 0.82.3", "cranelift-entity 0.82.3", "gimli 0.26.1", - "log 0.4.14", + "log 0.4.16", "regalloc 0.0.34", "smallvec 1.8.0", "target-lexicon", @@ -1247,7 +1247,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "279afcc0d3e651b773f94837c3d581177b348c8d69e928104b2e9fccb226f921" dependencies = [ "cranelift-codegen 0.76.0", - "log 0.4.14", + "log 0.4.16", "smallvec 1.8.0", "target-lexicon", ] @@ -1259,7 +1259,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a006e3e32d80ce0e4ba7f1f9ddf66066d052a8c884a110b91d05404d6ce26dce" dependencies = [ "cranelift-codegen 0.82.3", - "log 0.4.14", + "log 0.4.16", "smallvec 1.8.0", "target-lexicon", ] @@ -1285,7 +1285,7 @@ dependencies = [ "cranelift-entity 0.82.3", "cranelift-frontend 0.82.3", "itertools", - "log 0.4.14", + "log 0.4.16", "smallvec 1.8.0", "wasmparser 0.83.0", "wasmtime-types", @@ -1937,7 +1937,7 @@ checksum = "44533bbbb3bb3c1fa17d9f2e4e38bbbaf8396ba82193c4cb1b6445d711445d36" dependencies = [ "atty", "humantime 1.3.0", - "log 0.4.14", + "log 0.4.16", "regex", "termcolor", ] @@ -1948,7 +1948,7 @@ version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a19187fea3ac7e84da7dacf48de0c45d63c6a76f9490dae389aead16c243fce3" dependencies = [ - "log 0.4.14", + "log 0.4.16", "regex", ] @@ -1960,7 +1960,7 @@ checksum = "0b2cf0344971ee6c64c31be0d530793fba457d322dfec2810c453d0ef228f9c3" dependencies = [ "atty", "humantime 2.1.0", - "log 0.4.14", + "log 0.4.16", "regex", "termcolor", ] @@ -2054,7 +2054,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4fdbe0d94371f9ce939b555dd342d0686cc4c0cadbcd4b61d70af5ff97eb4126" dependencies = [ "env_logger 0.7.1", - "log 0.4.14", + "log 0.4.16", ] [[package]] @@ -2066,7 +2066,7 @@ dependencies = [ "either", "futures 0.3.21", "futures-timer", - "log 0.4.14", + "log 0.4.16", "num-traits", "parity-scale-codec", "parking_lot 0.11.2", @@ -2151,7 +2151,7 @@ dependencies = [ "frame-system", "hex-literal", "linregress", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "paste 1.0.6", "scale-info", @@ -2182,7 +2182,7 @@ dependencies = [ "itertools", "kvdb", "linked-hash-map", - "log 0.4.14", + "log 0.4.16", "memory-db", "parity-scale-codec", "prettytable-rs", @@ -2236,6 +2236,7 @@ dependencies = [ "frame-election-provider-solution-type", "frame-support", "frame-system", + "log 0.4.16", "parity-scale-codec", "rand 0.7.3", "scale-info", @@ -2307,7 +2308,7 @@ dependencies = [ "frame-system", "impl-trait-for-tuples", "k256", - "log 0.4.14", + "log 0.4.16", "once_cell", "parity-scale-codec", "parity-util-mem", @@ -2411,7 +2412,7 @@ version = "4.0.0-dev" dependencies = [ "criterion", "frame-support", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "scale-info", "serde", @@ -2746,7 +2747,7 @@ dependencies = [ "bitflags", "libc", "libgit2-sys", - "log 0.4.14", + "log 0.4.16", "url 2.2.1", ] @@ -2765,7 +2766,7 @@ dependencies = [ "aho-corasick", "bstr", "fnv", - "log 0.4.14", + "log 0.4.16", "regex", ] @@ -2824,7 +2825,7 @@ version = "4.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "99d6a30320f094710245150395bc763ad23128d6a1ebbad7594dc4164b62c56b" dependencies = [ - "log 0.4.14", + "log 0.4.16", "pest", "pest_derive", "quick-error 2.0.0", @@ -3061,7 +3062,7 @@ dependencies = [ "ct-logs", "futures-util", "hyper 0.14.16", - "log 0.4.14", + "log 0.4.16", "rustls 0.19.1", "rustls-native-certs 0.5.0", "tokio", @@ -3143,7 +3144,7 @@ dependencies = [ "if-addrs", "ipnet", "libc", - "log 0.4.14", + "log 0.4.16", "winapi 0.3.9", ] @@ -3295,7 +3296,7 @@ dependencies = [ "hyper-tls", "jsonrpc-core", "jsonrpc-pubsub", - "log 0.4.14", + "log 0.4.16", "serde", "serde_json", "tokio", @@ -3312,7 +3313,7 @@ dependencies = [ "futures 0.3.21", "futures-executor", "futures-util", - "log 0.4.14", + "log 0.4.16", "serde", "serde_derive", "serde_json", @@ -3350,7 +3351,7 @@ dependencies = [ "hyper 0.14.16", "jsonrpc-core", "jsonrpc-server-utils", - "log 0.4.14", + "log 0.4.16", "net2", "parking_lot 0.11.2", "unicase 2.6.0", @@ -3365,7 +3366,7 @@ dependencies = [ "futures 0.3.21", "jsonrpc-core", "jsonrpc-server-utils", - "log 0.4.14", + "log 0.4.16", "parity-tokio-ipc", "parking_lot 0.11.2", "tower-service", @@ -3380,7 +3381,7 @@ dependencies = [ "futures 0.3.21", "jsonrpc-core", "lazy_static", - "log 0.4.14", + "log 0.4.16", "parking_lot 0.11.2", "rand 0.7.3", "serde", @@ -3397,7 +3398,7 @@ dependencies = [ "globset", "jsonrpc-core", "lazy_static", - "log 0.4.14", + "log 0.4.16", "tokio", "tokio-stream", "tokio-util 0.6.7", @@ -3413,7 +3414,7 @@ dependencies = [ "futures 0.3.21", "jsonrpc-core", "jsonrpc-server-utils", - "log 0.4.14", + "log 0.4.16", "parity-ws", "parking_lot 0.11.2", "slab", @@ -3557,7 +3558,7 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0de8b303297635ad57c9f5059fd9cee7a47f8e8daa09df0fcd07dd39fb22977f" dependencies = [ - "log 0.4.14", + "log 0.4.16", ] [[package]] @@ -3589,7 +3590,7 @@ checksum = "ca7fbdfd71cd663dceb0faf3367a99f8cf724514933e9867cec4995b6027cbc1" dependencies = [ "fs-swap", "kvdb", - "log 0.4.14", + "log 0.4.16", "num_cpus", "owning_ref", "parity-util-mem", @@ -3723,7 +3724,7 @@ dependencies = [ "futures-timer", "lazy_static", "libsecp256k1", - "log 0.4.14", + "log 0.4.16", "multiaddr", "multihash 0.14.0", "multistream-select", @@ -3762,7 +3763,7 @@ dependencies = [ "async-std-resolver", "futures 0.3.21", "libp2p-core", - "log 0.4.14", + "log 0.4.16", "smallvec 1.8.0", "trust-dns-resolver", ] @@ -3778,7 +3779,7 @@ dependencies = [ "futures 0.3.21", "libp2p-core", "libp2p-swarm", - "log 0.4.14", + "log 0.4.16", "prost", "prost-build", "rand 0.7.3", @@ -3800,7 +3801,7 @@ dependencies = [ "hex_fmt", "libp2p-core", "libp2p-swarm", - "log 0.4.14", + "log 0.4.16", "prost", "prost-build", "rand 0.7.3", @@ -3820,7 +3821,7 @@ dependencies = [ "futures 0.3.21", "libp2p-core", "libp2p-swarm", - "log 0.4.14", + "log 0.4.16", "lru 0.6.6", "prost", "prost-build", @@ -3842,7 +3843,7 @@ dependencies = [ "futures 0.3.21", "libp2p-core", "libp2p-swarm", - "log 0.4.14", + "log 0.4.16", "prost", "prost-build", "rand 0.7.3", @@ -3868,7 +3869,7 @@ dependencies = [ "lazy_static", "libp2p-core", "libp2p-swarm", - "log 0.4.14", + "log 0.4.16", "rand 0.8.4", "smallvec 1.8.0", "socket2 0.4.4", @@ -3899,7 +3900,7 @@ dependencies = [ "bytes 1.1.0", "futures 0.3.21", "libp2p-core", - "log 0.4.14", + "log 0.4.16", "nohash-hasher", "parking_lot 0.11.2", "rand 0.7.3", @@ -3918,7 +3919,7 @@ dependencies = [ "futures 0.3.21", "lazy_static", "libp2p-core", - "log 0.4.14", + "log 0.4.16", "prost", "prost-build", "rand 0.8.4", @@ -3938,7 +3939,7 @@ dependencies = [ "futures 0.3.21", "libp2p-core", "libp2p-swarm", - "log 0.4.14", + "log 0.4.16", "rand 0.7.3", "void", "wasm-timer", @@ -3954,7 +3955,7 @@ dependencies = [ "bytes 1.1.0", "futures 0.3.21", "libp2p-core", - "log 0.4.14", + "log 0.4.16", "prost", "prost-build", "unsigned-varint 0.7.0", @@ -3968,7 +3969,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0f1a458bbda880107b5b36fcb9b5a1ef0c329685da0e203ed692a8ebe64cc92c" dependencies = [ "futures 0.3.21", - "log 0.4.14", + "log 0.4.16", "pin-project 1.0.10", "rand 0.7.3", "salsa20", @@ -3987,7 +3988,7 @@ dependencies = [ "futures-timer", "libp2p-core", "libp2p-swarm", - "log 0.4.14", + "log 0.4.16", "pin-project 1.0.10", "prost", "prost-build", @@ -4009,7 +4010,7 @@ dependencies = [ "futures 0.3.21", "libp2p-core", "libp2p-swarm", - "log 0.4.14", + "log 0.4.16", "prost", "prost-build", "rand 0.8.4", @@ -4031,7 +4032,7 @@ dependencies = [ "futures 0.3.21", "libp2p-core", "libp2p-swarm", - "log 0.4.14", + "log 0.4.16", "lru 0.7.5", "rand 0.7.3", "smallvec 1.8.0", @@ -4048,7 +4049,7 @@ dependencies = [ "either", "futures 0.3.21", "libp2p-core", - "log 0.4.14", + "log 0.4.16", "rand 0.7.3", "smallvec 1.8.0", "void", @@ -4078,7 +4079,7 @@ dependencies = [ "ipnet", "libc", "libp2p-core", - "log 0.4.14", + "log 0.4.16", "socket2 0.4.4", ] @@ -4091,7 +4092,7 @@ dependencies = [ "async-std", "futures 0.3.21", "libp2p-core", - "log 0.4.14", + "log 0.4.16", ] [[package]] @@ -4118,7 +4119,7 @@ dependencies = [ "futures 0.3.21", "futures-rustls", "libp2p-core", - "log 0.4.14", + "log 0.4.16", "quicksink", "rw-stream-sink", "soketto", @@ -4287,14 +4288,14 @@ version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e19e8d5c34a3e0e2223db8e060f9e8264aeeb5c5fc64a4ee9965c062211c024b" dependencies = [ - "log 0.4.14", + "log 0.4.16", ] [[package]] name = "log" -version = "0.4.14" +version = "0.4.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "51b9bbe6c47d51fc3e1a9b945965946b4c44142ab8792c50835a980d362c2710" +checksum = "6389c490849ff5bc16be905ae24bc913a9c8892e19b2341dbc175e14c341c2b8" dependencies = [ "cfg-if 1.0.0", "value-bag", @@ -4528,7 +4529,7 @@ dependencies = [ "iovec", "kernel32-sys", "libc", - "log 0.4.14", + "log 0.4.16", "miow 0.2.2", "net2", "slab", @@ -4542,7 +4543,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba272f85fa0b41fc91872be579b3bbe0f56b792aa361a380eb669469f68dafb2" dependencies = [ "libc", - "log 0.4.14", + "log 0.4.16", "miow 0.3.6", "ntapi", "winapi 0.3.9", @@ -4555,7 +4556,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "52403fe290012ce777c4626790c8951324a2b9e3316b3143779c72b029742f19" dependencies = [ "lazycell", - "log 0.4.14", + "log 0.4.16", "mio 0.6.23", "slab", ] @@ -4675,7 +4676,7 @@ checksum = "7d91ec0a2440aaff5f78ec35631a7027d50386c6163aa975f7caa0d5da4b6ff8" dependencies = [ "bytes 1.1.0", "futures 0.3.21", - "log 0.4.14", + "log 0.4.16", "pin-project 1.0.10", "smallvec 1.8.0", "unsigned-varint 0.7.0", @@ -4727,7 +4728,7 @@ checksum = "48ba9f7719b5a0f42f338907614285fb5fd70e53858141f69898a1fb7203b24d" dependencies = [ "lazy_static", "libc", - "log 0.4.14", + "log 0.4.16", "openssl", "openssl-probe", "openssl-sys", @@ -4784,7 +4785,7 @@ dependencies = [ "kvdb", "kvdb-rocksdb", "lazy_static", - "log 0.4.14", + "log 0.4.16", "node-primitives", "node-runtime", "node-testing", @@ -4822,7 +4823,7 @@ dependencies = [ "frame-system-rpc-runtime-api", "futures 0.3.21", "hex-literal", - "log 0.4.14", + "log 0.4.16", "nix", "node-executor", "node-inspect", @@ -4996,7 +4997,7 @@ dependencies = [ "frame-system-rpc-runtime-api", "frame-try-runtime", "hex-literal", - "log 0.4.14", + "log 0.4.16", "node-primitives", "pallet-asset-tx-payment", "pallet-assets", @@ -5167,7 +5168,7 @@ dependencies = [ "frame-system", "fs_extra", "futures 0.3.21", - "log 0.4.14", + "log 0.4.16", "node-executor", "node-primitives", "node-runtime", @@ -5550,7 +5551,7 @@ dependencies = [ "frame-election-provider-support", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-authorship", "pallet-balances", "pallet-offences", @@ -5579,7 +5580,7 @@ dependencies = [ "frame-election-provider-support", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "parity-scale-codec", "scale-info", @@ -5607,7 +5608,7 @@ dependencies = [ "frame-election-provider-support", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-bags-list", "pallet-staking", "remote-externalities", @@ -5626,7 +5627,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-transaction-payment", "parity-scale-codec", "scale-info", @@ -5664,7 +5665,7 @@ dependencies = [ "frame-system", "hex", "hex-literal", - "log 0.4.14", + "log 0.4.16", "pallet-beefy", "pallet-mmr", "pallet-session", @@ -5685,7 +5686,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "pallet-treasury", "parity-scale-codec", @@ -5703,7 +5704,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "pallet-bounties", "pallet-treasury", @@ -5722,7 +5723,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "scale-info", "sp-core", @@ -5742,7 +5743,7 @@ dependencies = [ "frame-support", "frame-system", "hex-literal", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "pallet-contracts-primitives", "pallet-contracts-proc-macro", @@ -5866,7 +5867,7 @@ dependencies = [ "frame-election-provider-support", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "pallet-election-provider-support-benchmarking", "parity-scale-codec", @@ -5903,7 +5904,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "parity-scale-codec", "scale-info", @@ -5922,7 +5923,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "parity-scale-codec", "scale-info", @@ -5939,7 +5940,7 @@ dependencies = [ "frame-support", "frame-system", "lite-json", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "scale-info", "sp-core", @@ -5990,7 +5991,7 @@ dependencies = [ "frame-election-provider-support", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-authorship", "pallet-balances", "pallet-offences", @@ -6035,7 +6036,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-authorship", "pallet-session", "parity-scale-codec", @@ -6089,7 +6090,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "scale-info", "sp-core", @@ -6171,7 +6172,7 @@ version = "4.0.0-dev" dependencies = [ "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "scale-info", "sp-core", @@ -6186,7 +6187,7 @@ version = "4.0.0-dev" dependencies = [ "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "parity-scale-codec", "scale-info", @@ -6330,7 +6331,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-preimage", "parity-scale-codec", "scale-info", @@ -6363,7 +6364,7 @@ dependencies = [ "frame-support", "frame-system", "impl-trait-for-tuples", - "log 0.4.14", + "log 0.4.16", "pallet-timestamp", "parity-scale-codec", "scale-info", @@ -6424,7 +6425,7 @@ dependencies = [ "frame-election-provider-support", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-authorship", "pallet-bags-list", "pallet-balances", @@ -6461,7 +6462,7 @@ dependencies = [ name = "pallet-staking-reward-fn" version = "4.0.0-dev" dependencies = [ - "log 0.4.14", + "log 0.4.16", "sp-arithmetic", ] @@ -6472,7 +6473,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "parity-scale-codec", "parking_lot 0.12.0", @@ -6525,7 +6526,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "scale-info", "sp-core", @@ -6543,7 +6544,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "pallet-treasury", "parity-scale-codec", @@ -6645,7 +6646,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "parity-scale-codec", "scale-info", @@ -6678,7 +6679,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "log 0.4.14", + "log 0.4.16", "pallet-balances", "parity-scale-codec", "scale-info", @@ -6717,7 +6718,7 @@ dependencies = [ "fs2", "hex", "libc", - "log 0.4.14", + "log 0.4.16", "lz4", "memmap2 0.2.1", "parking_lot 0.11.2", @@ -6765,7 +6766,7 @@ checksum = "9981e32fb75e004cc148f5fb70342f393830e0a4aa62e3cc93b50976218d42b6" dependencies = [ "futures 0.3.21", "libc", - "log 0.4.14", + "log 0.4.16", "rand 0.7.3", "tokio", "winapi 0.3.9", @@ -6822,7 +6823,7 @@ dependencies = [ "byteorder", "bytes 0.4.12", "httparse", - "log 0.4.14", + "log 0.4.16", "mio 0.6.23", "mio-extras", "rand 0.7.3", @@ -7131,7 +7132,7 @@ checksum = "a2a7bc6b2a29e632e45451c941832803a18cce6781db04de8a04696cdca8bde4" dependencies = [ "cfg-if 0.1.10", "libc", - "log 0.4.14", + "log 0.4.16", "wepoll-sys", "winapi 0.3.9", ] @@ -7323,7 +7324,7 @@ dependencies = [ "heck 0.3.2", "itertools", "lazy_static", - "log 0.4.14", + "log 0.4.16", "multimap", "petgraph", "prost", @@ -7404,7 +7405,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "588f6378e4dd99458b60ec275b4477add41ce4fa9f64dcba6f15adccb19b50d6" dependencies = [ "env_logger 0.8.4", - "log 0.4.14", + "log 0.4.16", "rand 0.8.4", ] @@ -7752,7 +7753,7 @@ version = "0.0.31" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "571f7f397d61c4755285cd37853fe8e03271c243424a907415909379659381c5" dependencies = [ - "log 0.4.14", + "log 0.4.16", "rustc-hash", "smallvec 1.8.0", ] @@ -7763,7 +7764,7 @@ version = "0.0.34" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "62446b1d3ebf980bdc68837700af1d77b37bc430e524bf95319c6eada2a4cc02" dependencies = [ - "log 0.4.14", + "log 0.4.16", "rustc-hash", "smallvec 1.8.0", ] @@ -7826,7 +7827,7 @@ dependencies = [ "env_logger 0.9.0", "frame-support", "jsonrpsee", - "log 0.4.14", + "log 0.4.16", "pallet-elections-phragmen", "parity-scale-codec", "serde", @@ -8012,7 +8013,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "35edb675feee39aec9c99fa5ff985081995a06d594114ae14cbe797ad7b7a6d7" dependencies = [ "base64 0.13.0", - "log 0.4.14", + "log 0.4.16", "ring", "sct 0.6.0", "webpki 0.21.4", @@ -8024,7 +8025,7 @@ version = "0.20.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d37e5e2290f3e040b594b1a9e04377c2c671f1a1cfd9bfdef82106ac1c113f84" dependencies = [ - "log 0.4.14", + "log 0.4.16", "ring", "sct 0.7.0", "webpki 0.22.0", @@ -8123,7 +8124,7 @@ dependencies = [ name = "sc-allocator" version = "4.1.0-dev" dependencies = [ - "log 0.4.14", + "log 0.4.16", "sp-core", "sp-wasm-interface", "thiserror", @@ -8138,7 +8139,7 @@ dependencies = [ "futures-timer", "ip_network", "libp2p", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "prost", "prost-build", @@ -8164,7 +8165,7 @@ version = "0.10.0-dev" dependencies = [ "futures 0.3.21", "futures-timer", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parking_lot 0.12.0", "sc-block-builder", @@ -8235,7 +8236,7 @@ dependencies = [ "futures 0.3.21", "hex", "libp2p", - "log 0.4.14", + "log 0.4.16", "names", "parity-scale-codec", "rand 0.7.3", @@ -8270,7 +8271,7 @@ dependencies = [ "fnv", "futures 0.3.21", "hash-db", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parking_lot 0.12.0", "sc-executor", @@ -8302,7 +8303,7 @@ dependencies = [ "kvdb-memorydb", "kvdb-rocksdb", "linked-hash-map", - "log 0.4.14", + "log 0.4.16", "parity-db", "parity-scale-codec", "parking_lot 0.12.0", @@ -8329,7 +8330,7 @@ dependencies = [ "futures 0.3.21", "futures-timer", "libp2p", - "log 0.4.14", + "log 0.4.16", "parking_lot 0.12.0", "sc-client-api", "sc-utils", @@ -8351,7 +8352,7 @@ version = "0.10.0-dev" dependencies = [ "async-trait", "futures 0.3.21", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parking_lot 0.12.0", "sc-block-builder", @@ -8389,7 +8390,7 @@ dependencies = [ "async-trait", "fork-tree", "futures 0.3.21", - "log 0.4.14", + "log 0.4.16", "merlin", "num-bigint", "num-rational 0.2.4", @@ -8483,7 +8484,7 @@ dependencies = [ "jsonrpc-core", "jsonrpc-core-client", "jsonrpc-derive", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sc-basic-authorship", "sc-client-api", @@ -8519,7 +8520,7 @@ dependencies = [ "async-trait", "futures 0.3.21", "futures-timer", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parking_lot 0.12.0", "sc-client-api", @@ -8543,7 +8544,7 @@ dependencies = [ "async-trait", "futures 0.3.21", "futures-timer", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sc-client-api", "sc-consensus", @@ -8631,7 +8632,7 @@ dependencies = [ name = "sc-executor-wasmi" version = "0.10.0-dev" dependencies = [ - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sc-allocator", "sc-executor-common", @@ -8648,7 +8649,7 @@ version = "0.10.0-dev" dependencies = [ "cfg-if 1.0.0", "libc", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parity-wasm 0.42.2", "sc-allocator", @@ -8675,7 +8676,7 @@ dependencies = [ "futures 0.3.21", "futures-timer", "hex", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parking_lot 0.12.0", "rand 0.8.4", @@ -8719,7 +8720,7 @@ dependencies = [ "jsonrpc-core-client", "jsonrpc-derive", "jsonrpc-pubsub", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sc-block-builder", "sc-client-api", @@ -8743,7 +8744,7 @@ dependencies = [ "ansi_term", "futures 0.3.21", "futures-timer", - "log 0.4.14", + "log 0.4.16", "parity-util-mem", "sc-client-api", "sc-network", @@ -8788,7 +8789,7 @@ dependencies = [ "libp2p", "linked-hash-map", "linked_hash_set", - "log 0.4.14", + "log 0.4.16", "lru 0.7.5", "parity-scale-codec", "parking_lot 0.12.0", @@ -8832,7 +8833,7 @@ dependencies = [ "futures 0.3.21", "futures-timer", "libp2p", - "log 0.4.14", + "log 0.4.16", "lru 0.7.5", "quickcheck", "sc-network", @@ -8851,7 +8852,7 @@ dependencies = [ "futures 0.3.21", "futures-timer", "libp2p", - "log 0.4.14", + "log 0.4.16", "parking_lot 0.12.0", "rand 0.7.3", "sc-block-builder", @@ -8911,7 +8912,7 @@ version = "4.0.0-dev" dependencies = [ "futures 0.3.21", "libp2p", - "log 0.4.14", + "log 0.4.16", "rand 0.7.3", "sc-utils", "serde_json", @@ -8922,7 +8923,7 @@ dependencies = [ name = "sc-proposer-metrics" version = "0.10.0-dev" dependencies = [ - "log 0.4.14", + "log 0.4.16", "substrate-prometheus-endpoint", ] @@ -8936,7 +8937,7 @@ dependencies = [ "jsonrpc-core", "jsonrpc-pubsub", "lazy_static", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parking_lot 0.12.0", "sc-block-builder", @@ -8972,7 +8973,7 @@ dependencies = [ "jsonrpc-core-client", "jsonrpc-derive", "jsonrpc-pubsub", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parking_lot 0.12.0", "sc-chain-spec", @@ -8998,7 +8999,7 @@ dependencies = [ "jsonrpc-ipc-server", "jsonrpc-pubsub", "jsonrpc-ws-server", - "log 0.4.14", + "log 0.4.16", "serde_json", "substrate-prometheus-endpoint", "tokio", @@ -9031,7 +9032,7 @@ dependencies = [ "hash-db", "jsonrpc-core", "jsonrpc-pubsub", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parity-util-mem", "parking_lot 0.12.0", @@ -9093,7 +9094,7 @@ dependencies = [ "futures 0.3.21", "hex", "hex-literal", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parking_lot 0.12.0", "sc-block-builder", @@ -9125,7 +9126,7 @@ dependencies = [ name = "sc-state-db" version = "0.10.0-dev" dependencies = [ - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parity-util-mem", "parity-util-mem-derive", @@ -9160,7 +9161,7 @@ version = "6.0.0-dev" dependencies = [ "futures 0.3.21", "libc", - "log 0.4.14", + "log 0.4.16", "rand 0.7.3", "rand_pcg 0.2.1", "regex", @@ -9179,7 +9180,7 @@ dependencies = [ "chrono", "futures 0.3.21", "libp2p", - "log 0.4.14", + "log 0.4.16", "parking_lot 0.12.0", "pin-project 1.0.10", "rand 0.7.3", @@ -9199,7 +9200,7 @@ dependencies = [ "criterion", "lazy_static", "libc", - "log 0.4.14", + "log 0.4.16", "once_cell", "parking_lot 0.12.0", "regex", @@ -9240,7 +9241,7 @@ dependencies = [ "futures-timer", "hex", "linked-hash-map", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parity-util-mem", "parking_lot 0.12.0", @@ -9269,7 +9270,7 @@ name = "sc-transaction-pool-api" version = "4.0.0-dev" dependencies = [ "futures 0.3.21", - "log 0.4.14", + "log 0.4.16", "serde", "sp-blockchain", "sp-runtime", @@ -9283,7 +9284,7 @@ dependencies = [ "futures 0.3.21", "futures-timer", "lazy_static", - "log 0.4.14", + "log 0.4.16", "parking_lot 0.12.0", "prometheus", "tokio-test", @@ -9766,7 +9767,7 @@ dependencies = [ "flate2", "futures 0.3.21", "httparse", - "log 0.4.14", + "log 0.4.16", "rand 0.8.4", "sha-1 0.9.4", ] @@ -9776,7 +9777,7 @@ name = "sp-api" version = "4.0.0-dev" dependencies = [ "hash-db", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sp-api-proc-macro", "sp-core", @@ -9805,7 +9806,7 @@ version = "2.0.1" dependencies = [ "criterion", "futures 0.3.21", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "rustversion", "sc-block-builder", @@ -9910,7 +9911,7 @@ name = "sp-blockchain" version = "4.0.0-dev" dependencies = [ "futures 0.3.21", - "log 0.4.14", + "log 0.4.16", "lru 0.7.5", "parity-scale-codec", "parking_lot 0.12.0", @@ -9929,7 +9930,7 @@ dependencies = [ "async-trait", "futures 0.3.21", "futures-timer", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sp-core", "sp-inherents", @@ -10034,7 +10035,7 @@ dependencies = [ "impl-serde", "lazy_static", "libsecp256k1", - "log 0.4.14", + "log 0.4.16", "merlin", "num-traits", "parity-scale-codec", @@ -10120,7 +10121,7 @@ name = "sp-finality-grandpa" version = "4.0.0-dev" dependencies = [ "finality-grandpa", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "scale-info", "serde", @@ -10153,7 +10154,7 @@ dependencies = [ "futures 0.3.21", "hash-db", "libsecp256k1", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parking_lot 0.12.0", "secp256k1", @@ -10211,7 +10212,7 @@ name = "sp-mmr-primitives" version = "4.0.0-dev" dependencies = [ "hex-literal", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "serde", "sp-api", @@ -10284,7 +10285,7 @@ dependencies = [ "either", "hash256-std-hasher", "impl-trait-for-tuples", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "parity-util-mem", "paste 1.0.6", @@ -10380,7 +10381,7 @@ name = "sp-sandbox" version = "0.10.0-dev" dependencies = [ "assert_matches", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sp-core", "sp-io", @@ -10427,7 +10428,7 @@ version = "0.12.0" dependencies = [ "hash-db", "hex-literal", - "log 0.4.14", + "log 0.4.16", "num-traits", "parity-scale-codec", "parking_lot 0.12.0", @@ -10465,7 +10466,7 @@ dependencies = [ name = "sp-tasks" version = "4.0.0-dev" dependencies = [ - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sp-core", "sp-externalities", @@ -10492,7 +10493,7 @@ version = "4.0.0-dev" dependencies = [ "async-trait", "futures-timer", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sp-api", "sp-inherents", @@ -10525,7 +10526,7 @@ name = "sp-transaction-storage-proof" version = "4.0.0-dev" dependencies = [ "async-trait", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "scale-info", "sp-core", @@ -10587,7 +10588,7 @@ name = "sp-wasm-interface" version = "6.0.0" dependencies = [ "impl-trait-for-tuples", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sp-std", "wasmi", @@ -10732,7 +10733,7 @@ dependencies = [ "jsonrpc-core", "jsonrpc-core-client", "jsonrpc-derive", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sc-client-api", "sc-rpc-api", @@ -10753,7 +10754,7 @@ version = "0.10.0-dev" dependencies = [ "futures-util", "hyper 0.14.16", - "log 0.4.14", + "log 0.4.16", "prometheus", "thiserror", "tokio", @@ -10766,7 +10767,7 @@ dependencies = [ "jsonrpc-core", "jsonrpc-core-client", "jsonrpc-derive", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "sc-client-api", "sc-rpc-api", @@ -10817,7 +10818,7 @@ dependencies = [ "frame-system", "frame-system-rpc-runtime-api", "futures 0.3.21", - "log 0.4.14", + "log 0.4.16", "memory-db", "pallet-babe", "pallet-timestamp", @@ -11190,7 +11191,7 @@ checksum = "57fc868aae093479e3131e3d165c93b1c7474109d13c90ec0dda2a1bbfff0674" dependencies = [ "bytes 0.4.12", "futures 0.1.31", - "log 0.4.14", + "log 0.4.16", ] [[package]] @@ -11223,7 +11224,7 @@ dependencies = [ "crossbeam-utils 0.7.2", "futures 0.1.31", "lazy_static", - "log 0.4.14", + "log 0.4.16", "mio 0.6.23", "num_cpus", "parking_lot 0.9.0", @@ -11323,7 +11324,7 @@ dependencies = [ "bytes 1.1.0", "futures-core", "futures-sink", - "log 0.4.14", + "log 0.4.16", "pin-project-lite 0.2.6", "tokio", ] @@ -11364,7 +11365,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "375a639232caf30edfc78e8d89b2d4c375515393e7af7e16f01cd96917fb2105" dependencies = [ "cfg-if 1.0.0", - "log 0.4.14", + "log 0.4.16", "pin-project-lite 0.2.6", "tracing-attributes", "tracing-core", @@ -11407,7 +11408,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6923477a48e41c1951f1999ef8bb5a3023eb723ceadafe78ffb65dc366761e3" dependencies = [ "lazy_static", - "log 0.4.14", + "log 0.4.16", "tracing-core", ] @@ -11480,7 +11481,7 @@ checksum = "d32d034c0d3db64b43c31de38e945f15b40cd4ca6d2dcfc26d4798ce8de4ab83" dependencies = [ "hash-db", "hashbrown 0.12.0", - "log 0.4.14", + "log 0.4.16", "rustc-hex", "smallvec 1.8.0", ] @@ -11520,7 +11521,7 @@ dependencies = [ "idna 0.2.2", "ipnet", "lazy_static", - "log 0.4.14", + "log 0.4.16", "rand 0.8.4", "smallvec 1.8.0", "thiserror", @@ -11538,7 +11539,7 @@ dependencies = [ "futures-util", "ipconfig", "lazy_static", - "log 0.4.14", + "log 0.4.16", "lru-cache", "parking_lot 0.11.2", "resolv-conf", @@ -11559,7 +11560,7 @@ version = "0.10.0-dev" dependencies = [ "clap 3.1.6", "jsonrpsee", - "log 0.4.14", + "log 0.4.16", "parity-scale-codec", "remote-externalities", "sc-chain-spec", @@ -11765,11 +11766,12 @@ dependencies = [ [[package]] name = "value-bag" -version = "1.0.0-alpha.6" +version = "1.0.0-alpha.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b676010e055c99033117c2343b33a40a30b91fecd6c49055ac9cd2d6c305ab1" +checksum = "79923f7731dc61ebfba3633098bf3ac533bbd35ccd8c57e7088d9a5eebe0263f" dependencies = [ "ctor", + "version_check 0.9.2", ] [[package]] @@ -11834,7 +11836,7 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1ce8a968cb1cd110d136ff8b819a556d6fb6d919363c61534f6860c7eb172ba0" dependencies = [ - "log 0.4.14", + "log 0.4.16", "try-lock", ] @@ -11868,7 +11870,7 @@ checksum = "f34c405b4f0658583dba0c1c7c9b694f3cac32655db463b56c254a1c75269523" dependencies = [ "bumpalo", "lazy_static", - "log 0.4.14", + "log 0.4.16", "proc-macro2", "quote", "syn", @@ -11922,7 +11924,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d0c32691b6c7e6c14e7f8fd55361a9088b507aa49620fcd06c09b3a1082186b9" dependencies = [ - "log 0.4.14", + "log 0.4.16", "parity-wasm 0.32.0", "rustc-demangle", ] @@ -12215,7 +12217,7 @@ dependencies = [ "indexmap", "lazy_static", "libc", - "log 0.4.14", + "log 0.4.16", "object 0.27.1", "once_cell", "paste 1.0.6", @@ -12244,7 +12246,7 @@ dependencies = [ "bincode", "directories-next", "file-per-thread-logger", - "log 0.4.14", + "log 0.4.16", "rustix", "serde", "sha2 0.9.8", @@ -12266,7 +12268,7 @@ dependencies = [ "cranelift-native", "cranelift-wasm", "gimli 0.26.1", - "log 0.4.14", + "log 0.4.16", "more-asserts", "object 0.27.1", "target-lexicon", @@ -12285,7 +12287,7 @@ dependencies = [ "cranelift-entity 0.82.3", "gimli 0.26.1", "indexmap", - "log 0.4.14", + "log 0.4.16", "more-asserts", "object 0.27.1", "serde", @@ -12307,7 +12309,7 @@ dependencies = [ "cfg-if 1.0.0", "cpp_demangle", "gimli 0.26.1", - "log 0.4.14", + "log 0.4.16", "object 0.27.1", "region 2.2.0", "rustc-demangle", @@ -12344,7 +12346,7 @@ dependencies = [ "cfg-if 1.0.0", "indexmap", "libc", - "log 0.4.14", + "log 0.4.16", "mach", "memoffset", "more-asserts", @@ -12633,7 +12635,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e7d9028f208dd5e63c614be69f115c1b53cacc1111437d4c765185856666c107" dependencies = [ "futures 0.3.21", - "log 0.4.14", + "log 0.4.16", "nohash-hasher", "parking_lot 0.11.2", "rand 0.8.4", diff --git a/frame/election-provider-support/Cargo.toml b/frame/election-provider-support/Cargo.toml index be0c05e46df32..550302eae9f03 100644 --- a/frame/election-provider-support/Cargo.toml +++ b/frame/election-provider-support/Cargo.toml @@ -22,6 +22,7 @@ sp-runtime = { version = "6.0.0", default-features = false, path = "../../primit frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } frame-election-provider-solution-type = { version = "4.0.0-dev", path = "solution-type" } +log = { version = "0.4.16" } [dev-dependencies] rand = "0.7.3" diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 59d17089ad32c..a9b73cd1178e8 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -286,6 +286,8 @@ pub trait ElectionDataProvider { /// The number of pages that this data provider is expected to be able to provide. An /// implementation could use this to verify the value of `remaining: PageIndex`. + // TODO: this should maybe be a generic, because a single type might want implement it with + // different bounds. type Pages: Get; /// All possible targets for the election, i.e. the targets that could become elected, thus @@ -315,16 +317,33 @@ pub trait ElectionDataProvider { remaining_pages: PageIndex, ) -> data_provider::Result>>; - /// Same as [`Self::electable_targets_paged`], but the page 0 is assumed. + /// Same as [`Self::electable_targets_paged`], but the most significant page is assumed. + /// + /// This should only be used in the case where `Self::Pages` is `1`. fn electable_targets( maybe_max_len: Option, ) -> data_provider::Result> { - Self::electable_targets_paged(maybe_max_len, 0) + if Self::Pages::get() != 1 { + frame_support::defensive!( + "using the backward compatibility single page election data \ + in a multi-page context. This is most likely a mis-config" + ); + } + Self::electable_targets_paged(maybe_max_len, Self::msp()) } /// Same as [`Self::electing_voters_paged`], but the page 0 is assumed. + /// + /// + /// This should only be used in the case where `Self::Pages` is `1`. fn electing_voters(maybe_max_len: Option) -> data_provider::Result>> { - Self::electing_voters_paged(maybe_max_len, 0) + if Self::Pages::get() != 1 { + frame_support::defensive!( + "using the backward compatibility single page election data \ + in a multi-page context. This is most likely a mis-config" + ); + } + Self::electing_voters_paged(maybe_max_len, Self::msp()) } /// The number of targets to elect. @@ -335,8 +354,6 @@ pub trait ElectionDataProvider { /// A sensible implementation should use the minimum between this value and /// [`Self::targets().len()`], since desiring a winner set larger than candidates is not /// feasible. - /// - /// This is documented further in issue: fn desired_targets() -> data_provider::Result; /// Provide a best effort prediction about when the next election is about to happen. diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 3d67ab9505827..c90e3bc8e074d 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -22,7 +22,10 @@ use crate::{ Debug, ElectionDataProvider, ElectionProvider, InstantElectionProvider, NposSolver, WeightInfo, }; -use frame_support::{traits::Get, weights::DispatchClass}; +use frame_support::{ + traits::{ConstU32, Get}, + weights::DispatchClass, +}; use sp_npos_elections::*; use sp_std::{collections::btree_map::BTreeMap, marker::PhantomData, prelude::*}; @@ -81,6 +84,7 @@ pub trait Config { type DataProvider: ElectionDataProvider< AccountId = ::AccountId, BlockNumber = ::BlockNumber, + Pages = ConstU32<1>, >; /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 4ed46d2273751..263d73e577853 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -662,12 +662,11 @@ impl Pallet { SlashRewardFraction::::put(fraction); } - /// Get at most `maybe_max_len` voters ready for the npos election, assuming that `remaining` - /// more calls will be made in the same 'unit of election'. + /// Get at most `maybe_max_len` voters ready for the npos election, starting from `maybe_last` + /// if specified. /// - /// If `remaining > 0`, `LastIteratedNominator` is set to the last item that is returned. - /// Consequently, regardless of the value of `remaining`, if `LastIteratedNominator` exists, it - /// is used as the starting point of the iteration. + /// Returns the vector of voters (which is implicitly at most `maybe_max_len`), grouped with the + /// number of validators taken and the number of nominators taken respectively. pub fn get_npos_voters_page( maybe_max_len: Option, maybe_last: Option, @@ -774,7 +773,8 @@ impl Pallet { targets } - /// This function will add a nominator to the `Nominators` storage map, and `VoterList`. + /// This function will add a nominator to the `Nominators` storage map, and reflect it in + /// `VoterList`. /// /// If the nominator already exists, their nominations will be updated. /// @@ -796,7 +796,7 @@ impl Pallet { } /// This function will remove a nominator from the `Nominators` storage map, - /// and `VoterList`. + /// and reflect it in the `VoterList`. /// /// Returns `Ok(())` if `who` was removed from `Nominators`, otherwise `Err(_)`. /// @@ -849,7 +849,7 @@ impl Pallet { /// This function will remove a validator from the `Validators` storage map. /// - /// Returns true if `who` was removed from `Validators`, otherwise false. + /// Returns `Ok(_)` if `who` was removed from `Validators`, `Err(_)` otherwise. /// /// NOTE: you must ALWAYS use this function to remove a validator from the system. Any access to /// `Validators` or `VoterList` outside of this function is almost certainly @@ -895,20 +895,17 @@ impl ElectionDataProvider for Pallet { } /// implementation notes: Given the loose dynamics of the `ElectionProvider`, namely the fact - /// that number of entries returned per page is dynamic, this implementation is best-effort at - /// best. We try and use the `LastIteratedNominator`, which is always kept as long call with - /// `remaining > 0` are made. Any call with `remaining = 0` will clear this history item, and - /// the next call will be fresh. - /// - /// This function is written defensively: + /// that number of entries returned per page is dynamic, this implementation is best-effort. We + /// try and use the `LastIteratedNominator`, which is always kept as long call with `remaining > + /// 0` are made. Any call with `remaining = 0` will clear this history item, and the next call + /// will be a fresh start. fn electing_voters_paged( maybe_max_len: Option, mut remaining: PageIndex, ) -> data_provider::Result>> { remaining = remaining.min(Self::msp()); - // either this is the first call, or `LastIteratedNominator` should exist. - let mut maybe_last = LastIteratedNominator::::get(); + let mut maybe_last = LastIteratedNominator::::get(); let is_valid_state = if T::Pages::get() > 1 { // in the first page, the `LastIteratedNominator` should not exist, in the rest of the // pages it should. @@ -916,7 +913,6 @@ impl ElectionDataProvider for Pallet { } else { maybe_last.is_none() }; - // defensive: since, at least as of now, this function is also used for governance fallback, // we really really prefer it to NEVER fail. Thus, we play defensive and return just the // first page if the state of `LastIteratedNominator` is corrupt. From 56b02c63a8f1ab5fe5b2e7d3ff6b882852ad2631 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 19 Apr 2022 16:36:51 +0200 Subject: [PATCH 17/19] remote tests --- frame/bags-list/remote-tests/src/snapshot.rs | 69 +++++++++++-------- frame/election-provider-support/src/lib.rs | 26 ++++--- .../election-provider-support/src/onchain.rs | 11 +-- frame/staking/src/pallet/mod.rs | 1 + 4 files changed, 59 insertions(+), 48 deletions(-) diff --git a/frame/bags-list/remote-tests/src/snapshot.rs b/frame/bags-list/remote-tests/src/snapshot.rs index 408f5f2bd8aa2..3ccef6e4eee9c 100644 --- a/frame/bags-list/remote-tests/src/snapshot.rs +++ b/frame/bags-list/remote-tests/src/snapshot.rs @@ -16,19 +16,19 @@ //! Test to execute the snapshot using the voter bag. -use frame_election_provider_support::SortedListProvider; -use frame_support::traits::PalletInfoAccess; +use frame_election_provider_support::{ElectionDataProvider, SortedListProvider}; +use frame_support::{storage::generator::StorageMap, traits::PalletInfoAccess}; +use pallet_staking::Pallet as Staking; use remote_externalities::{Builder, Mode, OnlineConfig}; use sp_runtime::{traits::Block as BlockT, DeserializeOwned}; /// Execute create a snapshot from pallet-staking. pub async fn execute( - voter_limit: Option, + page_limit: Option, + pages: usize, currency_unit: u64, ws_url: String, ) { - use frame_support::storage::generator::StorageMap; - let mut ext = Builder::::new() .mode(Mode::Online(OnlineConfig { transport: ws_url.to_string().into(), @@ -49,38 +49,49 @@ pub async fn execute .unwrap(); ext.execute_with(|| { - use frame_election_provider_support::ElectionDataProvider; log::info!( target: crate::LOG_TARGET, "{} nodes in bags list.", ::VoterList::count(), ); + log::info!( + target: crate::LOG_TARGET, + "generating {} pages of snapshot, ranging {:?}", + pages, + >::range(pages as u32).collect::>(), + ); + + assert!(Staking::::last_iterated_nominator().is_none()); - let voters = - as ElectionDataProvider>::electing_voters(voter_limit) + for page in >::range(pages as u32) { + let page_voters = + as ElectionDataProvider>::electing_voters_paged( + page_limit, page, + ) .unwrap(); - let mut voters_nominator_only = voters - .iter() - .filter(|(v, _, _)| pallet_staking::Nominators::::contains_key(v)) - .cloned() - .collect::>(); - voters_nominator_only.sort_by_key(|(_, w, _)| *w); + let currency_unit = currency_unit as f64; + let min_voter = + page_voters.last().map(|(x, y, _)| (x.clone(), *y as f64 / currency_unit)); + let max_voter = + page_voters.first().map(|(x, y, _)| (x.clone(), *y as f64 / currency_unit)); + assert!( + page == 0 || + Staking::::last_iterated_nominator() == + page_voters.last().map(|(x, _, _)| x.clone()), + ); - let currency_unit = currency_unit as f64; - let min_voter = voters_nominator_only - .first() - .map(|(x, y, _)| (x.clone(), *y as f64 / currency_unit)); - let max_voter = voters_nominator_only - .last() - .map(|(x, y, _)| (x.clone(), *y as f64 / currency_unit)); - log::info!( - target: crate::LOG_TARGET, - "a snapshot with limit {:?} has been created, {} voters are taken. min nominator: {:?}, max: {:?}", - voter_limit, - voters.len(), - min_voter, - max_voter - ); + log::info!( + target: crate::LOG_TARGET, + "took snapshot page {:?} with limit {:?}, {} voters are taken. min voter: {:?}, max: {:?}", + page, + page_limit, + page_voters.len(), + min_voter, + max_voter + ); + } + + assert!(Staking::::last_iterated_nominator().is_none()); }); } diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index a9b73cd1178e8..418bd226737fc 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -319,30 +319,18 @@ pub trait ElectionDataProvider { /// Same as [`Self::electable_targets_paged`], but the most significant page is assumed. /// - /// This should only be used in the case where `Self::Pages` is `1`. + /// This should almost only be used in the case where `Self::Pages` is `1`. fn electable_targets( maybe_max_len: Option, ) -> data_provider::Result> { - if Self::Pages::get() != 1 { - frame_support::defensive!( - "using the backward compatibility single page election data \ - in a multi-page context. This is most likely a mis-config" - ); - } Self::electable_targets_paged(maybe_max_len, Self::msp()) } /// Same as [`Self::electing_voters_paged`], but the page 0 is assumed. /// /// - /// This should only be used in the case where `Self::Pages` is `1`. + /// This should almost only be used in the case where `Self::Pages` is `1`. fn electing_voters(maybe_max_len: Option) -> data_provider::Result>> { - if Self::Pages::get() != 1 { - frame_support::defensive!( - "using the backward compatibility single page election data \ - in a multi-page context. This is most likely a mis-config" - ); - } Self::electing_voters_paged(maybe_max_len, Self::msp()) } @@ -378,8 +366,18 @@ pub trait ElectionDataProvider { 0 } + /// The range of `take` pages, from most significant to least significant. + /// + /// ``` + /// todo!(); + /// ``` + fn range(take: PageIndex) -> Box> { + Box::new((Self::lsp()..Self::Pages::get()).take(take as usize).rev()) + } + /// Utility function only to be used in benchmarking scenarios, to be implemented optionally, /// else a noop. + #[cfg(any(feature = "runtime-benchmarks", test))] fn put_snapshot( _voters: Vec>, diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index c90e3bc8e074d..c43fde114eac8 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -22,16 +22,15 @@ use crate::{ Debug, ElectionDataProvider, ElectionProvider, InstantElectionProvider, NposSolver, WeightInfo, }; -use frame_support::{ - traits::{ConstU32, Get}, - weights::DispatchClass, -}; +use frame_support::{traits::Get, weights::DispatchClass}; use sp_npos_elections::*; use sp_std::{collections::btree_map::BTreeMap, marker::PhantomData, prelude::*}; /// Errors of the on-chain election. #[derive(Eq, PartialEq, Debug)] pub enum Error { + /// The number of pages is incorrect. + WrongPageSize, /// An internal error in the NPoS elections crate. NposElections(sp_npos_elections::Error), /// Errors from the data provider. @@ -84,7 +83,6 @@ pub trait Config { type DataProvider: ElectionDataProvider< AccountId = ::AccountId, BlockNumber = ::BlockNumber, - Pages = ConstU32<1>, >; /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; @@ -101,6 +99,9 @@ fn elect_with( maybe_max_voters: Option, maybe_max_targets: Option, ) -> Result::AccountId>, Error> { + if ::Pages::get() != 1 { + return Err(Error::WrongPageSize) + } let voters = T::DataProvider::electing_voters(maybe_max_voters).map_err(Error::DataProvider)?; let targets = T::DataProvider::electable_targets(maybe_max_targets).map_err(Error::DataProvider)?; diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 4080fe252d1c3..caa7b9e93b803 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -502,6 +502,7 @@ pub mod pallet { /// Last nominator that was being iterated. This is only useful for multi-page snapshot, and /// does not affect anything else. #[pallet::storage] + #[pallet::getter(fn last_iterated_nominator)] pub(crate) type LastIteratedNominator = StorageValue<_, T::AccountId, OptionQuery>; #[pallet::genesis_config] From bc5ca19fc165466441b39e93f7f283bbf66591eb Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 19 Apr 2022 16:44:47 +0200 Subject: [PATCH 18/19] fix --- frame/election-provider-support/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 418bd226737fc..ec43bb64fff78 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -284,7 +284,7 @@ pub trait ElectionDataProvider { /// Maximum number of votes per voter that this data provider is providing. type MaxVotesPerVoter: Get; - /// The number of pages that this data provider is expected to be able to provide. An + /// The maximum number of pages that this data provider is expected to be able to provide. An /// implementation could use this to verify the value of `remaining: PageIndex`. // TODO: this should maybe be a generic, because a single type might want implement it with // different bounds. From 5ad46ccdeed5321228a3e43b9fbf098c9d472839 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 19 Apr 2022 16:53:28 +0200 Subject: [PATCH 19/19] remove todos --- frame/election-provider-support/src/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index ec43bb64fff78..8ebf5ab48553a 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -286,7 +286,7 @@ pub trait ElectionDataProvider { /// The maximum number of pages that this data provider is expected to be able to provide. An /// implementation could use this to verify the value of `remaining: PageIndex`. - // TODO: this should maybe be a generic, because a single type might want implement it with + // NOTE: this should maybe be a generic, because a single type might want implement it with // different bounds. type Pages: Get; @@ -367,10 +367,6 @@ pub trait ElectionDataProvider { } /// The range of `take` pages, from most significant to least significant. - /// - /// ``` - /// todo!(); - /// ``` fn range(take: PageIndex) -> Box> { Box::new((Self::lsp()..Self::Pages::get()).take(take as usize).rev()) }