diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 0aff3d8046eef..d1aea6575ff67 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -49,6 +49,7 @@ use pallet_grandpa::{ }; use pallet_im_online::sr25519::AuthorityId as ImOnlineId; use pallet_session::historical as pallet_session_historical; +use pallet_staking::UseValidatorsMap; pub use pallet_transaction_payment::{CurrencyAdapter, Multiplier, TargetedFeeAdjustment}; use pallet_transaction_payment::{FeeDetails, RuntimeDispatchInfo}; use sp_api::impl_runtime_apis; @@ -555,9 +556,8 @@ 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 TargetList = UseValidatorsMap; type WeightInfo = pallet_staking::weights::SubstrateWeight; type BenchmarkingConfig = StakingBenchmarkingConfig; } diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index cd59ea881739d..91cbec2383871 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -24,12 +24,11 @@ //! ## Overview //! //! This pallet contains functionality for multi-signature dispatch, a (potentially) stateful -//! operation, allowing multiple signed -//! origins (accounts) to coordinate and dispatch a call from a well-known origin, derivable -//! deterministically from the set of account IDs and the threshold number of accounts from the -//! set that must approve it. In the case that the threshold is just one then this is a stateless -//! operation. This is useful for multisig wallets where cryptographic threshold signatures are -//! not available or desired. +//! operation, allowing multiple signed origins (accounts) to coordinate and dispatch a call from a +//! well-known origin, derivable deterministically from the set of account IDs and the threshold +//! number of accounts from the set that must approve it. In the case that the threshold is just one +//! then this is a stateless operation. This is useful for multisig wallets where cryptographic +//! threshold signatures are not available or desired. //! //! ## Interface //! diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index 115e4db54143c..300f27c2578c4 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -189,7 +189,7 @@ impl ListScenario { // find a destination weight that will trigger the worst case scenario let dest_weight_as_vote = - T::SortedListProvider::weight_update_worst_case(&origin_stash1, is_increase); + T::VoterList::weight_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 { @@ -844,7 +844,7 @@ benchmarks! { v, n, T::MaxNominations::get() as usize, false, None )?; }: { - let targets = >::get_npos_targets(); + let targets = >::get_npos_targets(None); assert_eq!(targets.len() as u32, v); } @@ -878,7 +878,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(), @@ -893,7 +893,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 beb814384f227..a38939edb195b 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -767,11 +767,12 @@ enum Releases { V2_0_0, V3_0_0, V4_0_0, - 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. + 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 `VoterList`. + V9_0_0, // inject validators into `NPoSVoteProvider` as well. + V10_0_0, // inject validator into `NPoSTargetList`. } impl Default for Releases { diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 029a6a380fee3..87c95965a572b 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -22,21 +22,22 @@ 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 { 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) + let _ = T::VoterList::on_insert(v.clone(), weight).map_err(|err| { + log!(warn, "failed to insert {:?} into VoterList: {:?}", v, err) }); } + StorageVersion::::put(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, "InjectValidatorsIntoVoterList being executed on the wrong storage version, expected Releases::V8_0_0"); T::DbWeight::get().reads(1) } } @@ -44,12 +45,9 @@ impl OnRuntimeUpgrade for InjectValidatorsIntoSortedListProvider { #[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" - ); + ensure!(StorageVersion::::get() == crate::Releases::V8_0_0, "must upgrade linearly"); - let prev_count = T::SortedListProvider::count(); + let prev_count = T::VoterList::count(); Self::set_temp_storage(prev_count, "prev"); Ok(()) } @@ -57,10 +55,46 @@ 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); + ensure!(post_count == prev_count + validators, "incorrect count"); + ensure!(StorageVersion::::get(), Releases::V9_0_0, "version not set"); + Ok(()) + } +} + +/// 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 InjectValidatorsIntoTargetList(sp_std::marker::PhantomData); +impl OnRuntimeUpgrade for InjectValidatorsIntoTargetList { + fn on_runtime_upgrade() -> Weight { + if StorageVersion::::get() == Releases::V9_0_0 { + for (v, _) in Validators::::iter() { + let weight = Pallet::::vote_weight(&v); + let _ = T::TargetList::on_insert(v.clone(), weight).map_err(|err| { + log!(warn, "failed to insert {:?} into TargetList: {:?}", v, err) + }); + } + + StorageVersion::::put(Releases::V10_0_0); + T::BlockWeights::get().max_block + } else { + log!(warn, "InjectValidatorsIntoTargetList being executed on the wrong storage version, expected Releases::V9_0_0"); + T::DbWeight::get().reads(1) + } + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + ensure!(StorageVersion::::get() == Releases::V9_0_0, "must upgrade linearly"); + Ok(()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade() -> Result<(), &'static str> { + ensure!(StorageVersion::::get(), Releases::V10_0_0, "must upgrade linearly"); Ok(()) } } @@ -87,11 +121,11 @@ pub mod v8 { 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!( @@ -108,8 +142,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 16b16dba22d57..c9791317c1c5b 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -270,7 +270,8 @@ 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 TargetList = UseValidatorsMap; type BenchmarkingConfig = TestBenchmarkingConfig; type WeightInfo = (); } @@ -539,8 +540,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 0e603b59e986f..1dbad666bdb33 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -22,6 +22,7 @@ use frame_election_provider_support::{ VoteWeight, VoteWeightProvider, VoterOf, }; use frame_support::{ + defensive, pallet_prelude::*, traits::{ Currency, CurrencyToVote, Defensive, EstimateNextNewSession, Get, Imbalance, @@ -659,7 +660,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) }; @@ -673,7 +674,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 < (2 * max_allowed_len as u32) { let voter = match sorted_voters.next() { Some(voter) => { @@ -715,7 +716,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 ) } @@ -744,22 +745,18 @@ impl Pallet { /// Get the targets for an upcoming npos election. /// /// This function is self-weighing as [`DispatchClass::Mandatory`]. - pub fn get_npos_targets() -> Vec { - let mut validator_count = 0u32; - let targets = Validators::::iter() - .map(|(v, _)| { - validator_count.saturating_inc(); - v - }) + pub fn get_npos_targets(maybe_max_len: Option) -> Vec { + let targets = T::TargetList::iter() + .take(maybe_max_len.unwrap_or_else(Bounded::max_value)) .collect::>(); - Self::register_weight(T::WeightInfo::get_npos_targets(validator_count)); + Self::register_weight(T::WeightInfo::get_npos_targets(targets.len() as u32)); targets } /// This function will add a nominator to the `Nominators` storage map, - /// and [`SortedListProvider`]. + /// and [`Config::VoterList`]. /// /// If the nominator already exists, their nominations will be updated. /// @@ -769,20 +766,17 @@ 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() - ); - debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); + #[cfg(debug_assertions)] + Self::sanity_check_list_providers(); } /// This function will remove a nominator from the `Nominators` storage map, - /// and [`SortedListProvider`]. + /// and [`Config::VoterList`]. /// /// Returns true if `who` was removed from `Nominators`, otherwise false. /// @@ -792,17 +786,14 @@ 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!( - Nominators::::count() + Validators::::count(), - T::SortedListProvider::count() - ); + #[cfg(debug_assertions)] + Self::sanity_check_list_providers(); outcome } @@ -817,16 +808,15 @@ 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(); + let _ = T::TargetList::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(())); + #[cfg(debug_assertions)] + Self::sanity_check_list_providers(); } /// This function will remove a validator from the `Validators` storage map. @@ -839,17 +829,15 @@ 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); + T::TargetList::on_remove(who); true } else { false }; - debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); - debug_assert_eq!( - Nominators::::count() + Validators::::count(), - T::SortedListProvider::count() - ); + #[cfg(debug_assertions)] + Self::sanity_check_list_providers(); outcome } @@ -863,6 +851,18 @@ impl Pallet { DispatchClass::Mandatory, ); } + + /// Perform all checks related to both [`Config::TargetList`] and [`Config::VoterList`]. + #[cfg(debug_assertions)] + fn sanity_check_list_providers() { + debug_assert_eq!( + Nominators::::count() + Validators::::count(), + T::VoterList::count() + ); + debug_assert_eq!(Validators::::count(), T::TargetList::count()); + debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); + } } impl ElectionDataProvider for Pallet { @@ -876,22 +876,19 @@ impl ElectionDataProvider for Pallet { } fn 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)); - + if maybe_max_len.map_or(false, |m| voters.len() > m) { + defensive!("get_npos_voters is corrupt"); + } Ok(voters) } fn targets(maybe_max_len: Option) -> data_provider::Result> { - let target_count = Validators::::count(); - - // We can't handle this case yet -- return an error. - if maybe_max_len.map_or(false, |max_len| target_count > max_len as u32) { - return Err("Target snapshot too big") + let targets = Self::get_npos_targets(maybe_max_len); + if maybe_max_len.map_or(false, |m| targets.len() > m) { + defensive!("get_npos_targets is corrupt"); } - - Ok(Self::get_npos_targets()) + Ok(targets) } fn next_election_prediction(now: T::BlockNumber) -> T::BlockNumber { @@ -978,7 +975,7 @@ impl ElectionDataProvider for Pallet { >::remove_all(); >::remove_all(); - T::SortedListProvider::unsafe_clear(); + T::VoterList::unsafe_clear(); } #[cfg(feature = "runtime-benchmarks")] @@ -1288,7 +1285,7 @@ impl VoteWeightProvider for Pallet { } } -/// A simple voter list implementation that does not require any additional pallets. Note, this +/// A simple sorted 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 UseNominatorsAndValidatorsMap(sp_std::marker::PhantomData); @@ -1331,9 +1328,50 @@ impl SortedListProvider for UseNominatorsAndValidatorsM } fn unsafe_clear() { - // 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(); } } + +/// A simple sorted list implementation that does not require any additional pallets. Note, this +/// does not provided validators in sorted ordered. If you desire nominators in a sorted order take +/// a look at [`pallet-bags-list]. +pub struct UseValidatorsMap(sp_std::marker::PhantomData); +impl SortedListProvider for UseValidatorsMap { + type Error = (); + + /// Returns iterator over voter list, which can have `take` called on it. + fn iter() -> Box> { + Box::new(Validators::::iter().map(|(v, _)| v)) + } + fn count() -> u32 { + Validators::::count() + } + fn contains(id: &T::AccountId) -> bool { + Validators::::contains_key(id) + } + fn on_insert(_: T::AccountId, _weight: VoteWeight) -> Result<(), Self::Error> { + // nothing to do on insert. + Ok(()) + } + fn on_update(_: &T::AccountId, _weight: VoteWeight) { + // nothing to do on update. + } + fn on_remove(_: &T::AccountId) { + // nothing to do on remove. + } + fn unsafe_regenerate( + _: impl IntoIterator, + _: Box VoteWeight>, + ) -> u32 { + // nothing to do upon regenerate. + 0 + } + fn sanity_check() -> Result<(), &'static str> { + Ok(()) + } + + fn unsafe_clear() { + Validators::::remove_all(); + } +} diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 134d3a9a65bc5..70cb229870941 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -150,11 +150,14 @@ 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::UseNominatorsAndValidatorsMap`] is likely the - /// desired option. - type SortedListProvider: SortedListProvider; + /// Something that can provide a list of voters in a somewhat sorted way. The original use + /// case for this was designed with `pallet_bags_list::Pallet` in mind. Otherwise, + /// [`impls::UseNominatorsAndValidatorsMap`] is likely the second option. + type VoterList: SortedListProvider; + /// Something that can provide a list of targets in a somewhat sorted way. The original use + /// case for this was designed with `pallet_bags_list::Pallet` in mind. Otherwise, + /// [`impls::UseValidatorsMap`] is likely the second option. + type TargetList: SortedListProvider; /// Some parameters of the benchmarking. type BenchmarkingConfig: BenchmarkingConfig; @@ -564,12 +567,17 @@ 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." ); + assert_eq!( + T::TargetList::count(), + Validators::::count(), + "not all genesis stakers were inserted into sorted list provider, something is wrong." + ); } } @@ -816,10 +824,15 @@ 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(())); + } + if T::TargetList::contains(&stash) { + T::TargetList::on_update(&stash, Self::weight_of(&ledger.stash)); + debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); } Self::deposit_event(Event::::Bonded(stash.clone(), extra)); @@ -885,8 +898,11 @@ 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)); + } + if T::TargetList::contains(&ledger.stash) { + T::TargetList::on_update(&ledger.stash, Self::weight_of(&ledger.stash)); } Self::deposit_event(Event::::Unbonded(ledger.stash, value)); @@ -1368,8 +1384,12 @@ 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)); + } + if T::TargetList::contains(&ledger.stash) { + T::TargetList::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 50d95f2847cb1..26e74cb1a7e57 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4056,7 +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(), 5); + assert_eq!(::VoterList::count(), 5); // if limits is less.. assert_eq!(Staking::voters(Some(1)).unwrap().len(), 1); @@ -4090,7 +4090,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] ); @@ -4132,7 +4132,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] ); @@ -4679,10 +4679,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] ); @@ -4690,10 +4690,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] ); }); @@ -4705,23 +4705,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]); }); } } diff --git a/frame/support/src/traits/misc.rs b/frame/support/src/traits/misc.rs index d2fd438d3a802..8c61874003bce 100644 --- a/frame/support/src/traits/misc.rs +++ b/frame/support/src/traits/misc.rs @@ -38,18 +38,18 @@ macro_rules! defensive { frame_support::log::error!( target: "runtime", "{}", - $crate::traits::misc::DEFENSIVE_OP_PUBLIC_ERROR + $crate::traits::DEFENSIVE_OP_PUBLIC_ERROR ); - debug_assert!(false, "{}", $crate::traits::misc::DEFENSIVE_OP_INTERNAL_ERROR); + debug_assert!(false, "{}", $crate::traits::DEFENSIVE_OP_INTERNAL_ERROR); }; ($error:tt) => { frame_support::log::error!( target: "runtime", "{}: {:?}", - $crate::traits::misc::DEFENSIVE_OP_PUBLIC_ERROR, + $crate::traits::DEFENSIVE_OP_PUBLIC_ERROR, $error ); - debug_assert!(false, "{}: {:?}", $crate::traits::misc::DEFENSIVE_OP_INTERNAL_ERROR, $error); + debug_assert!(false, "{}: {:?}", $crate::traits::DEFENSIVE_OP_INTERNAL_ERROR, $error); } }