diff --git a/frame/bags-list/remote-tests/src/snapshot.rs b/frame/bags-list/remote-tests/src/snapshot.rs index 0e68a4495edfc..f05c00cd9a244 100644 --- a/frame/bags-list/remote-tests/src/snapshot.rs +++ b/frame/bags-list/remote-tests/src/snapshot.rs @@ -27,6 +27,7 @@ pub async fn execute ws_url: String, ) { use frame_support::storage::generator::StorageMap; + let mut ext = Builder::::new() .mode(Mode::Online(OnlineConfig { transport: ws_url.to_string().into(), @@ -38,10 +39,10 @@ pub async fn execute })) .inject_hashed_prefix(&>::prefix_hash()) .inject_hashed_prefix(&>::prefix_hash()) - .inject_hashed_prefix(&>::prefix_hash()) - .inject_hashed_prefix(&>::prefix_hash()) - .inject_hashed_key(&>::hashed_key()) - .inject_hashed_key(&>::hashed_key()) + .inject_hashed_prefix(&>::map_storage_final_prefix()) + .inject_hashed_prefix(&>::map_storage_final_prefix()) + .inject_hashed_key(&>::counter_storage_final_key()) + .inject_hashed_key(&>::counter_storage_final_key()) .build() .await .unwrap(); diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index 69a73c51fdc7e..5b242e1f223a8 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -112,8 +112,8 @@ pub fn create_validator_with_nominators( assert_eq!(new_validators.len(), 1); assert_eq!(new_validators[0], v_stash, "Our validator was not selected!"); - assert_ne!(CounterForValidators::::get(), 0); - assert_ne!(CounterForNominators::::get(), 0); + assert_ne!(Validators::::count(), 0); + assert_ne!(Nominators::::count(), 0); // Give Era Points let reward = EraRewardPoints:: { @@ -922,8 +922,8 @@ mod tests { let count_validators = Validators::::iter().count(); let count_nominators = Nominators::::iter().count(); - assert_eq!(count_validators, CounterForValidators::::get() as usize); - assert_eq!(count_nominators, CounterForNominators::::get() as usize); + assert_eq!(count_validators, Validators::::count() as usize); + assert_eq!(count_nominators, Nominators::::count() as usize); assert_eq!(count_validators, v as usize); assert_eq!(count_nominators, n as usize); diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 6f1d3953f8f17..33f45d80870a3 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -70,10 +70,22 @@ pub mod v8 { pub mod v7 { use super::*; + use frame_support::generate_storage_alias; + + generate_storage_alias!(Staking, CounterForValidators => Value); + generate_storage_alias!(Staking, CounterForNominators => Value); pub fn pre_migrate() -> Result<(), &'static str> { - assert!(CounterForValidators::::get().is_zero(), "CounterForValidators already set."); - assert!(CounterForNominators::::get().is_zero(), "CounterForNominators already set."); + assert!( + CounterForValidators::get().unwrap().is_zero(), + "CounterForValidators already set." + ); + assert!( + CounterForNominators::get().unwrap().is_zero(), + "CounterForNominators already set." + ); + assert!(Validators::::count().is_zero(), "Validators already set."); + assert!(Nominators::::count().is_zero(), "Nominators already set."); assert!(StorageVersion::::get() == Releases::V6_0_0); Ok(()) } @@ -83,8 +95,8 @@ pub mod v7 { let validator_count = Validators::::iter().count() as u32; let nominator_count = Nominators::::iter().count() as u32; - CounterForValidators::::put(validator_count); - CounterForNominators::::put(nominator_count); + CounterForValidators::put(validator_count); + CounterForNominators::put(nominator_count); StorageVersion::::put(Releases::V7_0_0); log!(info, "Completed staking migration to Releases::V7_0_0"); diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 0b1d6a06d9c7f..8a8ab1f1e4d04 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -536,8 +536,8 @@ fn post_conditions() { fn check_count() { let nominator_count = Nominators::::iter().count() as u32; let validator_count = Validators::::iter().count() as u32; - assert_eq!(nominator_count, CounterForNominators::::get()); - assert_eq!(validator_count, CounterForValidators::::get()); + 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(); diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index cd26ff3b729c5..b3158c4ef6b3d 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -665,8 +665,8 @@ impl Pallet { maybe_max_len: Option, ) -> Vec<(T::AccountId, VoteWeight, Vec)> { let max_allowed_len = { - let nominator_count = CounterForNominators::::get() as usize; - let validator_count = CounterForValidators::::get() as usize; + let nominator_count = Nominators::::count() as usize; + let validator_count = Validators::::count() as usize; let all_voter_count = validator_count.saturating_add(nominator_count); maybe_max_len.unwrap_or(all_voter_count).min(all_voter_count) }; @@ -765,18 +765,15 @@ impl Pallet { } /// This function will add a nominator to the `Nominators` storage map, - /// [`SortedListProvider`] and keep track of the `CounterForNominators`. + /// and [`SortedListProvider`]. /// /// 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`, its counter, or `VoterList` outside of this function is almost certainly + /// 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 the counter. - CounterForNominators::::mutate(|x| x.saturating_inc()); - // maybe update sorted list. Error checking is defensive-only - this should never fail. if T::SortedListProvider::on_insert(who.clone(), Self::weight_of(who)).is_err() { log!(warn, "attempt to insert duplicate nominator ({:#?})", who); @@ -790,53 +787,46 @@ impl Pallet { } /// This function will remove a nominator from the `Nominators` storage map, - /// [`SortedListProvider`] and keep track of the `CounterForNominators`. + /// and [`SortedListProvider`]. /// /// Returns true if `who` was removed from `Nominators`, otherwise false. /// /// NOTE: you must ALWAYS use this function to remove a nominator from the system. Any access to - /// `Nominators`, its counter, or `VoterList` outside of this function is almost certainly + /// `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) { Nominators::::remove(who); - CounterForNominators::::mutate(|x| x.saturating_dec()); T::SortedListProvider::on_remove(who); debug_assert_eq!(T::SortedListProvider::sanity_check(), Ok(())); - debug_assert_eq!(CounterForNominators::::get(), T::SortedListProvider::count()); + debug_assert_eq!(Nominators::::count(), T::SortedListProvider::count()); true } else { false } } - /// This function will add a validator to the `Validators` storage map, and keep track of the - /// `CounterForValidators`. + /// This function will add a validator to the `Validators` storage map. /// /// If the validator already exists, their preferences will be updated. /// /// NOTE: you must ALWAYS use this function to add a validator to the system. Any access to - /// `Validators`, its counter, or `VoterList` outside of this function is almost certainly + /// `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) { - CounterForValidators::::mutate(|x| x.saturating_inc()) - } Validators::::insert(who, prefs); } - /// This function will remove a validator from the `Validators` storage map, - /// and keep track of the `CounterForValidators`. + /// This function will remove a validator from the `Validators` storage map. /// /// Returns true if `who` was removed from `Validators`, otherwise false. /// /// NOTE: you must ALWAYS use this function to remove a validator from the system. Any access to - /// `Validators`, its counter, or `VoterList` outside of this function is almost certainly + /// `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) { Validators::::remove(who); - CounterForValidators::::mutate(|x| x.saturating_dec()); true } else { false @@ -865,14 +855,6 @@ impl ElectionDataProvider> for Pallet fn voters( maybe_max_len: Option, ) -> data_provider::Result)>> { - debug_assert!(>::iter().count() as u32 == CounterForNominators::::get()); - debug_assert!(>::iter().count() as u32 == CounterForValidators::::get()); - debug_assert_eq!( - CounterForNominators::::get(), - T::SortedListProvider::count(), - "voter_count must be accurate", - ); - // 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)); @@ -881,7 +863,7 @@ impl ElectionDataProvider> for Pallet } fn targets(maybe_max_len: Option) -> data_provider::Result> { - let target_count = CounterForValidators::::get(); + 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) { @@ -967,10 +949,9 @@ impl ElectionDataProvider> for Pallet fn clear() { >::remove_all(None); >::remove_all(None); - >::remove_all(None); - >::remove_all(None); - >::kill(); - >::kill(); + >::remove_all(); + >::remove_all(); + T::SortedListProvider::unsafe_clear(); } @@ -1284,7 +1265,7 @@ impl SortedListProvider for UseNominatorsMap { Box::new(Nominators::::iter().map(|(n, _)| n)) } fn count() -> u32 { - CounterForNominators::::get() + Nominators::::count() } fn contains(id: &T::AccountId) -> bool { Nominators::::contains_key(id) @@ -1309,10 +1290,10 @@ impl SortedListProvider for UseNominatorsMap { fn sanity_check() -> Result<(), &'static str> { Ok(()) } + 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(None); - CounterForNominators::::take(); + Nominators::::remove_all(); } } diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index f7e96ce0cf765..091df82e28172 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -234,16 +234,10 @@ pub mod pallet { StorageMap<_, Twox64Concat, T::AccountId, RewardDestination, ValueQuery>; /// The map from (wannabe) validator stash key to the preferences of that validator. - /// - /// When updating this storage item, you must also update the `CounterForValidators`. #[pallet::storage] #[pallet::getter(fn validators)] pub type Validators = - StorageMap<_, Twox64Concat, T::AccountId, ValidatorPrefs, ValueQuery>; - - /// A tracker to keep count of the number of items in the `Validators` map. - #[pallet::storage] - pub type CounterForValidators = StorageValue<_, u32, ValueQuery>; + CountedStorageMap<_, Twox64Concat, T::AccountId, ValidatorPrefs, ValueQuery>; /// The maximum validator count before we stop allowing new validators to join. /// @@ -252,16 +246,10 @@ pub mod pallet { pub type MaxValidatorsCount = StorageValue<_, u32, OptionQuery>; /// The map from nominator stash key to the set of stash keys of all validators to nominate. - /// - /// When updating this storage item, you must also update the `CounterForNominators`. #[pallet::storage] #[pallet::getter(fn nominators)] pub type Nominators = - StorageMap<_, Twox64Concat, T::AccountId, Nominations>; - - /// A tracker to keep count of the number of items in the `Nominators` map. - #[pallet::storage] - pub type CounterForNominators = StorageValue<_, u32, ValueQuery>; + CountedStorageMap<_, Twox64Concat, T::AccountId, Nominations>; /// The maximum nominator count before we stop allowing new validators to join. /// @@ -570,7 +558,7 @@ pub mod pallet { // all voters are reported to the `SortedListProvider`. assert_eq!( T::SortedListProvider::count(), - CounterForNominators::::get(), + Nominators::::count(), "not all genesis stakers were inserted into sorted list provider, something is wrong." ); } @@ -987,7 +975,7 @@ pub mod pallet { // the runtime. if let Some(max_validators) = MaxValidatorsCount::::get() { ensure!( - CounterForValidators::::get() < max_validators, + Validators::::count() < max_validators, Error::::TooManyValidators ); } @@ -1027,7 +1015,7 @@ pub mod pallet { // the runtime. if let Some(max_nominators) = MaxNominatorsCount::::get() { ensure!( - CounterForNominators::::get() < max_nominators, + Nominators::::count() < max_nominators, Error::::TooManyNominators ); } @@ -1610,7 +1598,7 @@ pub mod pallet { let min_active_bond = if Nominators::::contains_key(&stash) { let max_nominator_count = MaxNominatorsCount::::get().ok_or(Error::::CannotChillOther)?; - let current_nominator_count = CounterForNominators::::get(); + let current_nominator_count = Nominators::::count(); ensure!( threshold * max_nominator_count < current_nominator_count, Error::::CannotChillOther @@ -1619,7 +1607,7 @@ pub mod pallet { } else if Validators::::contains_key(&stash) { let max_validator_count = MaxValidatorsCount::::get().ok_or(Error::::CannotChillOther)?; - let current_validator_count = CounterForValidators::::get(); + let current_validator_count = Validators::::count(); ensure!( threshold * max_validator_count < current_validator_count, Error::::CannotChillOther diff --git a/frame/staking/src/testing_utils.rs b/frame/staking/src/testing_utils.rs index 611773fc0ccf2..1c9163bd56402 100644 --- a/frame/staking/src/testing_utils.rs +++ b/frame/staking/src/testing_utils.rs @@ -36,12 +36,11 @@ const SEED: u32 = 0; /// This function removes all validators and nominators from storage. pub fn clear_validators_and_nominators() { - Validators::::remove_all(None); - CounterForValidators::::kill(); + Validators::::remove_all(); // whenever we touch nominators counter we should update `T::SortedListProvider` as well. - Nominators::::remove_all(None); - CounterForNominators::::kill(); + Nominators::::remove_all(); + // NOTE: safe to call outside block production T::SortedListProvider::unsafe_clear(); } diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 4604351b52305..ec8be43f02841 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4300,8 +4300,8 @@ fn chill_other_works() { .min_nominator_bond(1_000) .min_validator_bond(1_500) .build_and_execute(|| { - let initial_validators = CounterForValidators::::get(); - let initial_nominators = CounterForNominators::::get(); + let initial_validators = Validators::::count(); + let initial_nominators = Nominators::::count(); for i in 0..15 { let a = 4 * i; let b = 4 * i + 1; @@ -4424,8 +4424,8 @@ fn chill_other_works() { )); // 16 people total because tests start with 2 active one - assert_eq!(CounterForNominators::::get(), 15 + initial_nominators); - assert_eq!(CounterForValidators::::get(), 15 + initial_validators); + assert_eq!(Nominators::::count(), 15 + initial_nominators); + assert_eq!(Validators::::count(), 15 + initial_validators); // Users can now be chilled down to 7 people, so we try to remove 9 of them (starting // with 16) @@ -4437,13 +4437,13 @@ fn chill_other_works() { } // chill a nominator. Limit is not reached, not chill-able - assert_eq!(CounterForNominators::::get(), 7); + assert_eq!(Nominators::::count(), 7); assert_noop!( Staking::chill_other(Origin::signed(1337), 1), Error::::CannotChillOther ); // chill a validator. Limit is reached, chill-able. - assert_eq!(CounterForValidators::::get(), 9); + assert_eq!(Validators::::count(), 9); assert_ok!(Staking::chill_other(Origin::signed(1337), 3)); }) } @@ -4451,9 +4451,9 @@ fn chill_other_works() { #[test] fn capped_stakers_works() { ExtBuilder::default().build_and_execute(|| { - let validator_count = CounterForValidators::::get(); + let validator_count = Validators::::count(); assert_eq!(validator_count, 3); - let nominator_count = CounterForNominators::::get(); + let nominator_count = Nominators::::count(); assert_eq!(nominator_count, 1); // Change the maximums diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 5211453fd09b4..01414d7019ae1 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -98,6 +98,17 @@ where OnEmpty: Get + 'static, MaxValues: Get>, { + /// The key used to store the counter of the map. + pub fn counter_storage_final_key() -> [u8; 32] { + CounterFor::::hashed_key() + } + + /// The prefix used to generate the key of the map. + pub fn map_storage_final_prefix() -> Vec { + use crate::storage::generator::StorageMap; + ::Map::prefix_hash() + } + /// Get the storage key used to fetch a value corresponding to a specific key. pub fn hashed_key_for>(key: KeyArg) -> Vec { ::Map::hashed_key_for(key)