diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index e2903c0b314da..8c7a20af15683 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -690,9 +690,10 @@ parameter_types! { impl pallet_bags_list::Config for Runtime { type Event = Event; - type VoteWeightProvider = Staking; + type ScoreProvider = Staking; type WeightInfo = pallet_bags_list::weights::SubstrateWeight; type BagThresholds = BagThresholds; + type Score = sp_npos_elections::VoteWeight; } parameter_types! { diff --git a/frame/bags-list/remote-tests/src/lib.rs b/frame/bags-list/remote-tests/src/lib.rs index 9a88ff24f2f3b..83c322f93134e 100644 --- a/frame/bags-list/remote-tests/src/lib.rs +++ b/frame/bags-list/remote-tests/src/lib.rs @@ -17,6 +17,7 @@ //! Utilities for remote-testing pallet-bags-list. +use frame_election_provider_support::ScoreProvider; use sp_std::prelude::*; /// A common log target to use. @@ -55,8 +56,12 @@ pub fn display_and_check_bags(currency_unit: u64, currency_na let mut rebaggable = 0; let mut active_bags = 0; for vote_weight_thresh in ::BagThresholds::get() { + let vote_weight_thresh_u64: u64 = (*vote_weight_thresh) + .try_into() + .map_err(|_| "runtime must configure score to at most u64 to use this test") + .unwrap(); // threshold in terms of UNITS (e.g. KSM, DOT etc) - let vote_weight_thresh_as_unit = *vote_weight_thresh as f64 / currency_unit as f64; + let vote_weight_thresh_as_unit = vote_weight_thresh_u64 as f64 / currency_unit as f64; let pretty_thresh = format!("Threshold: {}. {}", vote_weight_thresh_as_unit, currency_name); let bag = match pallet_bags_list::Pallet::::list_bags_get(*vote_weight_thresh) { @@ -70,9 +75,13 @@ pub fn display_and_check_bags(currency_unit: u64, currency_na active_bags += 1; for id in bag.std_iter().map(|node| node.std_id().clone()) { - let vote_weight = pallet_staking::Pallet::::weight_of(&id); + let vote_weight = ::ScoreProvider::score(&id); + let vote_weight_thresh_u64: u64 = (*vote_weight_thresh) + .try_into() + .map_err(|_| "runtime must configure score to at most u64 to use this test") + .unwrap(); let vote_weight_as_balance: pallet_staking::BalanceOf = - vote_weight.try_into().map_err(|_| "can't convert").unwrap(); + vote_weight_thresh_u64.try_into().map_err(|_| "can't convert").unwrap(); if vote_weight_as_balance < min_nominator_bond { log::trace!( @@ -87,13 +96,17 @@ pub fn display_and_check_bags(currency_unit: u64, currency_na pallet_bags_list::Node::::get(&id).expect("node in bag must exist."); if node.is_misplaced(vote_weight) { rebaggable += 1; + let notional_bag = pallet_bags_list::notional_bag_for::(vote_weight); + let notional_bag_as_u64: u64 = notional_bag + .try_into() + .map_err(|_| "runtime must configure score to at most u64 to use this test") + .unwrap(); log::trace!( target: LOG_TARGET, "Account {:?} can be rebagged from {:?} to {:?}", id, vote_weight_thresh_as_unit, - pallet_bags_list::notional_bag_for::(vote_weight) as f64 / - currency_unit as f64 + notional_bag_as_u64 as f64 / currency_unit as f64 ); } } diff --git a/frame/bags-list/src/benchmarks.rs b/frame/bags-list/src/benchmarks.rs index cc575d7d1efff..b94a97093d001 100644 --- a/frame/bags-list/src/benchmarks.rs +++ b/frame/bags-list/src/benchmarks.rs @@ -20,9 +20,10 @@ use super::*; use crate::list::List; use frame_benchmarking::{account, whitelist_account, whitelisted_caller}; -use frame_election_provider_support::VoteWeightProvider; +use frame_election_provider_support::ScoreProvider; use frame_support::{assert_ok, traits::Get}; use frame_system::RawOrigin as SystemOrigin; +use sp_runtime::traits::One; frame_benchmarking::benchmarks! { rebag_non_terminal { @@ -36,7 +37,7 @@ frame_benchmarking::benchmarks! { // clear any pre-existing storage. // NOTE: safe to call outside block production - List::::unsafe_clear(); + List::::unsafe_clear(); // define our origin and destination thresholds. let origin_bag_thresh = T::BagThresholds::get()[0]; @@ -44,21 +45,21 @@ frame_benchmarking::benchmarks! { // seed items in the origin bag. let origin_head: T::AccountId = account("origin_head", 0, 0); - assert_ok!(List::::insert(origin_head.clone(), origin_bag_thresh)); + assert_ok!(List::::insert(origin_head.clone(), origin_bag_thresh)); let origin_middle: T::AccountId = account("origin_middle", 0, 0); // the node we rebag (_R_) - assert_ok!(List::::insert(origin_middle.clone(), origin_bag_thresh)); + assert_ok!(List::::insert(origin_middle.clone(), origin_bag_thresh)); let origin_tail: T::AccountId = account("origin_tail", 0, 0); - assert_ok!(List::::insert(origin_tail.clone(), origin_bag_thresh)); + assert_ok!(List::::insert(origin_tail.clone(), origin_bag_thresh)); // seed items in the destination bag. let dest_head: T::AccountId = account("dest_head", 0, 0); - assert_ok!(List::::insert(dest_head.clone(), dest_bag_thresh)); + assert_ok!(List::::insert(dest_head.clone(), dest_bag_thresh)); // the bags are in the expected state after initial setup. assert_eq!( - List::::get_bags(), + List::::get_bags(), vec![ (origin_bag_thresh, vec![origin_head.clone(), origin_middle.clone(), origin_tail.clone()]), (dest_bag_thresh, vec![dest_head.clone()]) @@ -67,12 +68,12 @@ frame_benchmarking::benchmarks! { let caller = whitelisted_caller(); // update the weight of `origin_middle` to guarantee it will be rebagged into the destination. - T::VoteWeightProvider::set_vote_weight_of(&origin_middle, dest_bag_thresh); + T::ScoreProvider::set_score_of(&origin_middle, dest_bag_thresh); }: rebag(SystemOrigin::Signed(caller), origin_middle.clone()) verify { // check the bags have updated as expected. assert_eq!( - List::::get_bags(), + List::::get_bags(), vec![ ( origin_bag_thresh, @@ -104,18 +105,18 @@ frame_benchmarking::benchmarks! { // seed items in the origin bag. let origin_head: T::AccountId = account("origin_head", 0, 0); - assert_ok!(List::::insert(origin_head.clone(), origin_bag_thresh)); + assert_ok!(List::::insert(origin_head.clone(), origin_bag_thresh)); let origin_tail: T::AccountId = account("origin_tail", 0, 0); // the node we rebag (_R_) - assert_ok!(List::::insert(origin_tail.clone(), origin_bag_thresh)); + assert_ok!(List::::insert(origin_tail.clone(), origin_bag_thresh)); // seed items in the destination bag. let dest_head: T::AccountId = account("dest_head", 0, 0); - assert_ok!(List::::insert(dest_head.clone(), dest_bag_thresh)); + assert_ok!(List::::insert(dest_head.clone(), dest_bag_thresh)); // the bags are in the expected state after initial setup. assert_eq!( - List::::get_bags(), + List::::get_bags(), vec![ (origin_bag_thresh, vec![origin_head.clone(), origin_tail.clone()]), (dest_bag_thresh, vec![dest_head.clone()]) @@ -124,12 +125,12 @@ frame_benchmarking::benchmarks! { let caller = whitelisted_caller(); // update the weight of `origin_tail` to guarantee it will be rebagged into the destination. - T::VoteWeightProvider::set_vote_weight_of(&origin_tail, dest_bag_thresh); + T::ScoreProvider::set_score_of(&origin_tail, dest_bag_thresh); }: rebag(SystemOrigin::Signed(caller), origin_tail.clone()) verify { // check the bags have updated as expected. assert_eq!( - List::::get_bags(), + List::::get_bags(), vec![ (origin_bag_thresh, vec![origin_head.clone()]), (dest_bag_thresh, vec![dest_head.clone(), origin_tail.clone()]) @@ -147,22 +148,22 @@ frame_benchmarking::benchmarks! { // insert the nodes in order let lighter: T::AccountId = account("lighter", 0, 0); - assert_ok!(List::::insert(lighter.clone(), bag_thresh)); + assert_ok!(List::::insert(lighter.clone(), bag_thresh)); let heavier_prev: T::AccountId = account("heavier_prev", 0, 0); - assert_ok!(List::::insert(heavier_prev.clone(), bag_thresh)); + assert_ok!(List::::insert(heavier_prev.clone(), bag_thresh)); let heavier: T::AccountId = account("heavier", 0, 0); - assert_ok!(List::::insert(heavier.clone(), bag_thresh)); + assert_ok!(List::::insert(heavier.clone(), bag_thresh)); let heavier_next: T::AccountId = account("heavier_next", 0, 0); - assert_ok!(List::::insert(heavier_next.clone(), bag_thresh)); + assert_ok!(List::::insert(heavier_next.clone(), bag_thresh)); - T::VoteWeightProvider::set_vote_weight_of(&lighter, bag_thresh - 1); - T::VoteWeightProvider::set_vote_weight_of(&heavier, bag_thresh); + T::ScoreProvider::set_score_of(&lighter, bag_thresh - One::one()); + T::ScoreProvider::set_score_of(&heavier, bag_thresh); assert_eq!( - List::::iter().map(|n| n.id().clone()).collect::>(), + List::::iter().map(|n| n.id().clone()).collect::>(), vec![lighter.clone(), heavier_prev.clone(), heavier.clone(), heavier_next.clone()] ); @@ -170,7 +171,7 @@ frame_benchmarking::benchmarks! { }: _(SystemOrigin::Signed(heavier.clone()), lighter.clone()) verify { assert_eq!( - List::::iter().map(|n| n.id().clone()).collect::>(), + List::::iter().map(|n| n.id().clone()).collect::>(), vec![heavier, lighter, heavier_prev, heavier_next] ) } diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 89c54db87023f..c502245409fdb 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -17,13 +17,14 @@ //! # Bags-List Pallet //! -//! A semi-sorted list, where items hold an `AccountId` based on some `VoteWeight`. The `AccountId` -//! (`id` for short) might be synonym to a `voter` or `nominator` in some context, and `VoteWeight` -//! signifies the chance of each id being included in the final [`SortedListProvider::iter`]. +//! A semi-sorted list, where items hold an `AccountId` based on some `Score`. The +//! `AccountId` (`id` for short) might be synonym to a `voter` or `nominator` in some context, and +//! `Score` signifies the chance of each id being included in the final +//! [`SortedListProvider::iter`]. //! //! It implements [`frame_election_provider_support::SortedListProvider`] to provide a semi-sorted //! list of accounts to another pallet. It needs some other pallet to give it some information about -//! the weights of accounts via [`frame_election_provider_support::VoteWeightProvider`]. +//! the weights of accounts via [`frame_election_provider_support::ScoreProvider`]. //! //! This pallet is not configurable at genesis. Whoever uses it should call appropriate functions of //! the `SortedListProvider` (e.g. `on_insert`, or `unsafe_regenerate`) at their genesis. @@ -33,12 +34,12 @@ //! The data structure exposed by this pallet aims to be optimized for: //! //! - insertions and removals. -//! - iteration over the top* N items by weight, where the precise ordering of items doesn't +//! - iteration over the top* N items by score, where the precise ordering of items doesn't //! particularly matter. //! //! # Details //! -//! - items are kept in bags, which are delineated by their range of weight (See +//! - items are kept in bags, which are delineated by their range of score (See //! [`Config::BagThresholds`]). //! - for iteration, bags are chained together from highest to lowest and elements within the bag //! are iterated from head to tail. @@ -46,14 +47,16 @@ //! it will worsen its position in list iteration; this reduces incentives for some types of spam //! that involve consistently removing and inserting for better position. Further, ordering //! granularity is thus dictated by range between each bag threshold. -//! - if an item's weight changes to a value no longer within the range of its current bag the -//! item's position will need to be updated by an external actor with rebag (update), or removal -//! and insertion. +//! - if an item's score changes to a value no longer within the range of its current bag the item's +//! position will need to be updated by an external actor with rebag (update), or removal and +//! insertion. #![cfg_attr(not(feature = "std"), no_std)] -use frame_election_provider_support::{SortedListProvider, VoteWeight, VoteWeightProvider}; +use codec::FullCodec; +use frame_election_provider_support::{ScoreProvider, SortedListProvider}; use frame_system::ensure_signed; +use sp_runtime::traits::{AtLeast32BitUnsigned, Bounded}; use sp_std::prelude::*; #[cfg(any(feature = "runtime-benchmarks", test))] @@ -92,38 +95,38 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(crate) trait Store)] - pub struct Pallet(_); + pub struct Pallet(_); #[pallet::config] - pub trait Config: frame_system::Config { + pub trait Config: frame_system::Config { /// The overarching event type. - type Event: From> + IsType<::Event>; + type Event: From> + IsType<::Event>; /// Weight information for extrinsics in this pallet. type WeightInfo: weights::WeightInfo; - /// Something that provides the weights of ids. - type VoteWeightProvider: VoteWeightProvider; + /// Something that provides the scores of ids. + type ScoreProvider: ScoreProvider; /// The list of thresholds separating the various bags. /// - /// Ids are separated into unsorted bags according to their vote weight. This specifies the - /// thresholds separating the bags. An id's bag is the largest bag for which the id's weight + /// Ids are separated into unsorted bags according to their score. This specifies the + /// thresholds separating the bags. An id's bag is the largest bag for which the id's score /// is less than or equal to its upper threshold. /// /// When ids are iterated, higher bags are iterated completely before lower bags. This means - /// that iteration is _semi-sorted_: ids of higher weight tend to come before ids of lower - /// weight, but peer ids within a particular bag are sorted in insertion order. + /// that iteration is _semi-sorted_: ids of higher score tend to come before ids of lower + /// score, but peer ids within a particular bag are sorted in insertion order. /// /// # Expressing the constant /// /// This constant must be sorted in strictly increasing order. Duplicate items are not /// permitted. /// - /// There is an implied upper limit of `VoteWeight::MAX`; that value does not need to be + /// There is an implied upper limit of `Score::MAX`; that value does not need to be /// specified within the bag. For any two threshold lists, if one ends with - /// `VoteWeight::MAX`, the other one does not, and they are otherwise equal, the two lists - /// will behave identically. + /// `Score::MAX`, the other one does not, and they are otherwise equal, the two + /// lists will behave identically. /// /// # Calculation /// @@ -141,52 +144,68 @@ pub mod pallet { /// the procedure given above, then the constant ratio is equal to 2. /// - If `BagThresholds::get().len() == 200`, and the thresholds are determined according to /// the procedure given above, then the constant ratio is approximately equal to 1.248. - /// - If the threshold list begins `[1, 2, 3, ...]`, then an id with weight 0 or 1 will fall - /// into bag 0, an id with weight 2 will fall into bag 1, etc. + /// - If the threshold list begins `[1, 2, 3, ...]`, then an id with score 0 or 1 will fall + /// into bag 0, an id with score 2 will fall into bag 1, etc. /// /// # Migration /// /// In the event that this list ever changes, a copy of the old bags list must be retained. /// With that `List::migrate` can be called, which will perform the appropriate migration. #[pallet::constant] - type BagThresholds: Get<&'static [VoteWeight]>; + type BagThresholds: Get<&'static [Self::Score]>; + + /// The type used to dictate a node position relative to other nodes. + type Score: Clone + + Default + + PartialEq + + Eq + + Ord + + PartialOrd + + sp_std::fmt::Debug + + Copy + + AtLeast32BitUnsigned + + Bounded + + TypeInfo + + FullCodec + + MaxEncodedLen; } /// A single node, within some bag. /// /// Nodes store links forward and back within their respective bags. #[pallet::storage] - pub(crate) type ListNodes = - CountedStorageMap<_, Twox64Concat, T::AccountId, list::Node>; + pub(crate) type ListNodes, I: 'static = ()> = + CountedStorageMap<_, Twox64Concat, T::AccountId, list::Node>; /// A bag stored in storage. /// /// Stores a `Bag` struct, which stores head and tail pointers to itself. #[pallet::storage] - pub(crate) type ListBags = StorageMap<_, Twox64Concat, VoteWeight, list::Bag>; + pub(crate) type ListBags, I: 'static = ()> = + StorageMap<_, Twox64Concat, T::Score, list::Bag>; #[pallet::event] #[pallet::generate_deposit(pub(crate) fn deposit_event)] - pub enum Event { + pub enum Event, I: 'static = ()> { /// Moved an account from one bag to another. - Rebagged { who: T::AccountId, from: VoteWeight, to: VoteWeight }, + Rebagged { who: T::AccountId, from: T::Score, to: T::Score }, } #[pallet::error] #[cfg_attr(test, derive(PartialEq))] - pub enum Error { + pub enum Error { /// Attempted to place node in front of a node in another bag. NotInSameBag, /// Id not found in list. IdNotFound, - /// An Id does not have a greater vote weight than another Id. + /// An Id does not have a greater score than another Id. NotHeavier, } #[pallet::call] - impl Pallet { + impl, I: 'static> Pallet { /// Declare that some `dislocated` account has, through rewards or penalties, sufficiently - /// changed its weight that it should properly fall into a different bag than its current + /// changed its score that it should properly fall into a different bag than its current /// one. /// /// Anyone can call this function about any potentially dislocated account. @@ -196,8 +215,8 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::rebag_non_terminal().max(T::WeightInfo::rebag_terminal()))] pub fn rebag(origin: OriginFor, dislocated: T::AccountId) -> DispatchResult { ensure_signed(origin)?; - let current_weight = T::VoteWeightProvider::vote_weight(&dislocated); - let _ = Pallet::::do_rebag(&dislocated, current_weight); + let current_score = T::ScoreProvider::score(&dislocated); + let _ = Pallet::::do_rebag(&dislocated, current_score); Ok(()) } @@ -208,16 +227,16 @@ pub mod pallet { /// /// Only works if /// - both nodes are within the same bag, - /// - and `origin` has a greater `VoteWeight` than `lighter`. + /// - and `origin` has a greater `Score` than `lighter`. #[pallet::weight(T::WeightInfo::put_in_front_of())] pub fn put_in_front_of(origin: OriginFor, lighter: T::AccountId) -> DispatchResult { let heavier = ensure_signed(origin)?; - List::::put_in_front_of(&lighter, &heavier).map_err(Into::into) + List::::put_in_front_of(&lighter, &heavier).map_err(Into::into) } } #[pallet::hooks] - impl Hooks> for Pallet { + impl, I: 'static> Hooks> for Pallet { fn integrity_test() { // ensure they are strictly increasing, this also implies that duplicates are detected. assert!( @@ -228,71 +247,70 @@ pub mod pallet { } } -impl Pallet { +impl, I: 'static> Pallet { /// Move an account from one bag to another, depositing an event on success. /// /// If the account changed bags, returns `Some((from, to))`. - pub fn do_rebag( - account: &T::AccountId, - new_weight: VoteWeight, - ) -> Option<(VoteWeight, VoteWeight)> { + pub fn do_rebag(account: &T::AccountId, new_weight: T::Score) -> Option<(T::Score, T::Score)> { // if no voter at that node, don't do anything. // the caller just wasted the fee to call this. - let maybe_movement = list::Node::::get(&account) + let maybe_movement = list::Node::::get(&account) .and_then(|node| List::update_position_for(node, new_weight)); if let Some((from, to)) = maybe_movement { - Self::deposit_event(Event::::Rebagged { who: account.clone(), from, to }); + Self::deposit_event(Event::::Rebagged { who: account.clone(), from, to }); }; maybe_movement } /// Equivalent to `ListBags::get`, but public. Useful for tests in outside of this crate. #[cfg(feature = "std")] - pub fn list_bags_get(weight: VoteWeight) -> Option> { - ListBags::get(weight) + pub fn list_bags_get(score: T::Score) -> Option> { + ListBags::get(score) } } -impl SortedListProvider for Pallet { +impl, I: 'static> SortedListProvider for Pallet { type Error = Error; + type Score = T::Score; + fn iter() -> Box> { - Box::new(List::::iter().map(|n| n.id().clone())) + Box::new(List::::iter().map(|n| n.id().clone())) } fn count() -> u32 { - ListNodes::::count() + ListNodes::::count() } fn contains(id: &T::AccountId) -> bool { - List::::contains(id) + List::::contains(id) } - fn on_insert(id: T::AccountId, weight: VoteWeight) -> Result<(), Error> { - List::::insert(id, weight) + fn on_insert(id: T::AccountId, score: T::Score) -> Result<(), Error> { + List::::insert(id, score) } - fn on_update(id: &T::AccountId, new_weight: VoteWeight) { - Pallet::::do_rebag(id, new_weight); + fn on_update(id: &T::AccountId, new_score: T::Score) { + Pallet::::do_rebag(id, new_score); } fn on_remove(id: &T::AccountId) { - List::::remove(id) + List::::remove(id) } fn unsafe_regenerate( all: impl IntoIterator, - weight_of: Box VoteWeight>, + score_of: Box T::Score>, ) -> u32 { // NOTE: This call is unsafe for the same reason as SortedListProvider::unsafe_regenerate. // I.e. because it can lead to many storage accesses. // So it is ok to call it as caller must ensure the conditions. - List::::unsafe_regenerate(all, weight_of) + List::::unsafe_regenerate(all, score_of) } #[cfg(feature = "std")] fn sanity_check() -> Result<(), &'static str> { - List::::sanity_check() + List::::sanity_check() } #[cfg(not(feature = "std"))] @@ -304,17 +322,17 @@ impl SortedListProvider for Pallet { // NOTE: This call is unsafe for the same reason as SortedListProvider::unsafe_clear. // I.e. because it can lead to many storage accesses. // So it is ok to call it as caller must ensure the conditions. - List::::unsafe_clear() + List::::unsafe_clear() } #[cfg(feature = "runtime-benchmarks")] - fn weight_update_worst_case(who: &T::AccountId, is_increase: bool) -> VoteWeight { + fn score_update_worst_case(who: &T::AccountId, is_increase: bool) -> Self::Score { use frame_support::traits::Get as _; let thresholds = T::BagThresholds::get(); - let node = list::Node::::get(who).unwrap(); + let node = list::Node::::get(who).unwrap(); let current_bag_idx = thresholds .iter() - .chain(sp_std::iter::once(&VoteWeight::MAX)) + .chain(sp_std::iter::once(&T::Score::max_value())) .position(|w| w == &node.bag_upper()) .unwrap(); diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 8cd89b8fff1cc..4921817c7e146 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -26,9 +26,10 @@ use crate::Config; use codec::{Decode, Encode, MaxEncodedLen}; -use frame_election_provider_support::{VoteWeight, VoteWeightProvider}; +use frame_election_provider_support::ScoreProvider; use frame_support::{traits::Get, DefaultNoBound}; use scale_info::TypeInfo; +use sp_runtime::traits::{Bounded, Zero}; use sp_std::{ boxed::Box, collections::{btree_map::BTreeMap, btree_set::BTreeSet}, @@ -46,36 +47,36 @@ pub enum Error { #[cfg(test)] mod tests; -/// Given a certain vote weight, to which bag does it belong to? +/// Given a certain score, to which bag does it belong to? /// /// Bags are identified by their upper threshold; the value returned by this function is guaranteed /// to be a member of `T::BagThresholds`. /// -/// Note that even if the thresholds list does not have `VoteWeight::MAX` as its final member, this -/// function behaves as if it does. -pub fn notional_bag_for(weight: VoteWeight) -> VoteWeight { +/// Note that even if the thresholds list does not have `T::Score::max_value()` as its final member, +/// this function behaves as if it does. +pub fn notional_bag_for, I: 'static>(score: T::Score) -> T::Score { let thresholds = T::BagThresholds::get(); - let idx = thresholds.partition_point(|&threshold| weight > threshold); - thresholds.get(idx).copied().unwrap_or(VoteWeight::MAX) + let idx = thresholds.partition_point(|&threshold| score > threshold); + thresholds.get(idx).copied().unwrap_or(T::Score::max_value()) } /// The **ONLY** entry point of this module. All operations to the bags-list should happen through /// this interface. It is forbidden to access other module members directly. // -// Data structure providing efficient mostly-accurate selection of the top N id by `VoteWeight`. +// Data structure providing efficient mostly-accurate selection of the top N id by `Score`. // // It's implemented as a set of linked lists. Each linked list comprises a bag of ids of -// arbitrary and unbounded length, all having a vote weight within a particular constant range. +// arbitrary and unbounded length, all having a score within a particular constant range. // This structure means that ids can be added and removed in `O(1)` time. // // Iteration is accomplished by chaining the iteration of each bag, from greatest to least. While -// the users within any particular bag are sorted in an entirely arbitrary order, the overall vote -// weight decreases as successive bags are reached. This means that it is valid to truncate +// the users within any particular bag are sorted in an entirely arbitrary order, the overall score +// decreases as successive bags are reached. This means that it is valid to truncate // iteration at any desired point; only those ids in the lowest bag can be excluded. This // satisfies both the desire for fairness and the requirement for efficiency. -pub struct List(PhantomData); +pub struct List, I: 'static = ()>(PhantomData<(T, I)>); -impl List { +impl, I: 'static> List { /// Remove all data associated with the list from storage. /// /// ## WARNING @@ -83,8 +84,8 @@ impl List { /// this function should generally not be used in production as it could lead to a very large /// number of storage accesses. pub(crate) fn unsafe_clear() { - crate::ListBags::::remove_all(None); - crate::ListNodes::::remove_all(); + crate::ListBags::::remove_all(None); + crate::ListNodes::::remove_all(); } /// Regenerate all of the data from the given ids. @@ -98,13 +99,13 @@ impl List { /// Returns the number of ids migrated. pub fn unsafe_regenerate( all: impl IntoIterator, - weight_of: Box VoteWeight>, + score_of: Box T::Score>, ) -> u32 { // NOTE: This call is unsafe for the same reason as SortedListProvider::unsafe_regenerate. // I.e. because it can lead to many storage accesses. // So it is ok to call it as caller must ensure the conditions. Self::unsafe_clear(); - Self::insert_many(all, weight_of) + Self::insert_many(all, score_of) } /// Migrate the list from one set of thresholds to another. @@ -127,7 +128,7 @@ impl List { /// - ids whose bags change at all are implicitly rebagged into the appropriate bag in the new /// threshold set. #[allow(dead_code)] - pub fn migrate(old_thresholds: &[VoteWeight]) -> u32 { + pub fn migrate(old_thresholds: &[T::Score]) -> u32 { let new_thresholds = T::BagThresholds::get(); if new_thresholds == old_thresholds { return 0 @@ -135,11 +136,13 @@ impl List { // we can't check all preconditions, but we can check one debug_assert!( - crate::ListBags::::iter().all(|(threshold, _)| old_thresholds.contains(&threshold)), + crate::ListBags::::iter() + .all(|(threshold, _)| old_thresholds.contains(&threshold)), "not all `bag_upper` currently in storage are members of `old_thresholds`", ); debug_assert!( - crate::ListNodes::::iter().all(|(_, node)| old_thresholds.contains(&node.bag_upper)), + crate::ListNodes::::iter() + .all(|(_, node)| old_thresholds.contains(&node.bag_upper)), "not all `node.bag_upper` currently in storage are members of `old_thresholds`", ); @@ -158,7 +161,7 @@ impl List { let affected_bag = { // this recreates `notional_bag_for` logic, but with the old thresholds. let idx = old_thresholds.partition_point(|&threshold| inserted_bag > threshold); - old_thresholds.get(idx).copied().unwrap_or(VoteWeight::MAX) + old_thresholds.get(idx).copied().unwrap_or(T::Score::max_value()) }; if !affected_old_bags.insert(affected_bag) { // If the previous threshold list was [10, 20], and we insert [3, 5], then there's @@ -166,7 +169,7 @@ impl List { continue } - if let Some(bag) = Bag::::get(affected_bag) { + if let Some(bag) = Bag::::get(affected_bag) { affected_accounts.extend(bag.iter().map(|node| node.id)); } } @@ -178,17 +181,17 @@ impl List { continue } - if let Some(bag) = Bag::::get(removed_bag) { + if let Some(bag) = Bag::::get(removed_bag) { affected_accounts.extend(bag.iter().map(|node| node.id)); } } // migrate the voters whose bag has changed let num_affected = affected_accounts.len() as u32; - let weight_of = T::VoteWeightProvider::vote_weight; + let score_of = T::ScoreProvider::score; let _removed = Self::remove_many(&affected_accounts); debug_assert_eq!(_removed, num_affected); - let _inserted = Self::insert_many(affected_accounts.into_iter(), weight_of); + let _inserted = Self::insert_many(affected_accounts.into_iter(), score_of); debug_assert_eq!(_inserted, num_affected); // we couldn't previously remove the old bags because both insertion and removal assume that @@ -199,10 +202,10 @@ impl List { // lookups. for removed_bag in removed_bags { debug_assert!( - !crate::ListNodes::::iter().any(|(_, node)| node.bag_upper == removed_bag), + !crate::ListNodes::::iter().any(|(_, node)| node.bag_upper == removed_bag), "no id should be present in a removed bag", ); - crate::ListBags::::remove(removed_bag); + crate::ListBags::::remove(removed_bag); } debug_assert_eq!(Self::sanity_check(), Ok(())); @@ -212,14 +215,14 @@ impl List { /// Returns `true` if the list contains `id`, otherwise returns `false`. pub(crate) fn contains(id: &T::AccountId) -> bool { - crate::ListNodes::::contains_key(id) + crate::ListNodes::::contains_key(id) } /// Iterate over all nodes in all bags in the list. /// /// Full iteration can be expensive; it's recommended to limit the number of items with /// `.take(n)`. - pub(crate) fn iter() -> impl Iterator> { + pub(crate) fn iter() -> impl Iterator> { // We need a touch of special handling here: because we permit `T::BagThresholds` to // omit the final bound, we need to ensure that we explicitly include that threshold in the // list. @@ -228,12 +231,14 @@ impl List { // easier; they can just configure `type BagThresholds = ()`. let thresholds = T::BagThresholds::get(); let iter = thresholds.iter().copied(); - let iter: Box> = if thresholds.last() == Some(&VoteWeight::MAX) { + let iter: Box> = if thresholds.last() == + Some(&T::Score::max_value()) + { // in the event that they included it, we can just pass the iterator through unchanged. Box::new(iter.rev()) } else { // otherwise, insert it here. - Box::new(iter.chain(iter::once(VoteWeight::MAX)).rev()) + Box::new(iter.chain(iter::once(T::Score::max_value())).rev()) }; iter.filter_map(Bag::get).flat_map(|bag| bag.iter()) @@ -245,12 +250,12 @@ impl List { /// Returns the final count of number of ids inserted. fn insert_many( ids: impl IntoIterator, - weight_of: impl Fn(&T::AccountId) -> VoteWeight, + score_of: impl Fn(&T::AccountId) -> T::Score, ) -> u32 { let mut count = 0; ids.into_iter().for_each(|v| { - let weight = weight_of(&v); - if Self::insert(v, weight).is_ok() { + let score = score_of(&v); + if Self::insert(v, score).is_ok() { count += 1; } }); @@ -261,13 +266,13 @@ impl List { /// Insert a new id into the appropriate bag in the list. /// /// Returns an error if the list already contains `id`. - pub(crate) fn insert(id: T::AccountId, weight: VoteWeight) -> Result<(), Error> { + pub(crate) fn insert(id: T::AccountId, score: T::Score) -> Result<(), Error> { if Self::contains(&id) { return Err(Error::Duplicate) } - let bag_weight = notional_bag_for::(weight); - let mut bag = Bag::::get_or_make(bag_weight); + let bag_score = notional_bag_for::(score); + let mut bag = Bag::::get_or_make(bag_score); // unchecked insertion is okay; we just got the correct `notional_bag_for`. bag.insert_unchecked(id.clone()); @@ -276,11 +281,12 @@ impl List { crate::log!( debug, - "inserted {:?} with weight {} into bag {:?}, new count is {}", + "inserted {:?} with score {:? + } into bag {:?}, new count is {}", id, - weight, - bag_weight, - crate::ListNodes::::count(), + score, + bag_score, + crate::ListNodes::::count(), ); Ok(()) @@ -301,7 +307,7 @@ impl List { let mut count = 0; for id in ids.into_iter() { - let node = match Node::::get(id) { + let node = match Node::::get(id) { Some(node) => node, None => continue, }; @@ -314,7 +320,7 @@ impl List { // this node is a head or tail, so the bag needs to be updated let bag = bags .entry(node.bag_upper) - .or_insert_with(|| Bag::::get_or_make(node.bag_upper)); + .or_insert_with(|| Bag::::get_or_make(node.bag_upper)); // node.bag_upper must be correct, therefore this bag will contain this node. bag.remove_node_unchecked(&node); } @@ -341,17 +347,17 @@ impl List { /// [`self.insert`]. However, given large quantities of nodes to move, it may be more efficient /// to call [`self.remove_many`] followed by [`self.insert_many`]. pub(crate) fn update_position_for( - node: Node, - new_weight: VoteWeight, - ) -> Option<(VoteWeight, VoteWeight)> { - node.is_misplaced(new_weight).then(move || { + node: Node, + new_score: T::Score, + ) -> Option<(T::Score, T::Score)> { + node.is_misplaced(new_score).then(move || { let old_bag_upper = node.bag_upper; if !node.is_terminal() { // this node is not a head or a tail, so we can just cut it out of the list. update // and put the prev and next of this node, we do `node.put` inside `insert_note`. node.excise(); - } else if let Some(mut bag) = Bag::::get(node.bag_upper) { + } else if let Some(mut bag) = Bag::::get(node.bag_upper) { // this is a head or tail, so the bag must be updated. bag.remove_node_unchecked(&node); bag.put(); @@ -365,8 +371,8 @@ impl List { } // put the node into the appropriate new bag. - let new_bag_upper = notional_bag_for::(new_weight); - let mut bag = Bag::::get_or_make(new_bag_upper); + let new_bag_upper = notional_bag_for::(new_score); + let mut bag = Bag::::get_or_make(new_bag_upper); // prev, next, and bag_upper of the node are updated inside `insert_node`, also // `node.put` is in there. bag.insert_node_unchecked(node); @@ -377,23 +383,22 @@ impl List { } /// Put `heavier_id` to the position directly in front of `lighter_id`. Both ids must be in the - /// same bag and the `weight_of` `lighter_id` must be less than that of `heavier_id`. + /// same bag and the `score_of` `lighter_id` must be less than that of `heavier_id`. pub(crate) fn put_in_front_of( lighter_id: &T::AccountId, heavier_id: &T::AccountId, - ) -> Result<(), crate::pallet::Error> { + ) -> Result<(), crate::pallet::Error> { use crate::pallet; use frame_support::ensure; - let lighter_node = Node::::get(&lighter_id).ok_or(pallet::Error::IdNotFound)?; - let heavier_node = Node::::get(&heavier_id).ok_or(pallet::Error::IdNotFound)?; + let lighter_node = Node::::get(&lighter_id).ok_or(pallet::Error::IdNotFound)?; + let heavier_node = Node::::get(&heavier_id).ok_or(pallet::Error::IdNotFound)?; ensure!(lighter_node.bag_upper == heavier_node.bag_upper, pallet::Error::NotInSameBag); // this is the most expensive check, so we do it last. ensure!( - T::VoteWeightProvider::vote_weight(&heavier_id) > - T::VoteWeightProvider::vote_weight(&lighter_id), + T::ScoreProvider::score(&heavier_id) > T::ScoreProvider::score(&lighter_id), pallet::Error::NotHeavier ); @@ -403,7 +408,7 @@ impl List { // re-fetch `lighter_node` from storage since it may have been updated when `heavier_node` // was removed. - let lighter_node = Node::::get(&lighter_id).ok_or_else(|| { + let lighter_node = Node::::get(&lighter_id).ok_or_else(|| { debug_assert!(false, "id that should exist cannot be found"); crate::log!(warn, "id that should exist cannot be found"); pallet::Error::IdNotFound @@ -422,7 +427,7 @@ impl List { /// - this is a naive function in that it does not check if `node` belongs to the same bag as /// `at`. It is expected that the call site will check preconditions. /// - this will panic if `at.bag_upper` is not a bag that already exists in storage. - fn insert_at_unchecked(mut at: Node, mut node: Node) { + fn insert_at_unchecked(mut at: Node, mut node: Node) { // connect `node` to its new `prev`. node.prev = at.prev.clone(); if let Some(mut prev) = at.prev() { @@ -439,7 +444,7 @@ impl List { // since `node` is always in front of `at` we know that 1) there is always at least 2 // nodes in the bag, and 2) only `node` could be the head and only `at` could be the // tail. - let mut bag = Bag::::get(at.bag_upper) + let mut bag = Bag::::get(at.bag_upper) .expect("given nodes must always have a valid bag. qed."); if node.prev == None { @@ -473,8 +478,8 @@ impl List { ); let iter_count = Self::iter().count() as u32; - let stored_count = crate::ListNodes::::count(); - let nodes_count = crate::ListNodes::::iter().count() as u32; + let stored_count = crate::ListNodes::::count(); + let nodes_count = crate::ListNodes::::iter().count() as u32; ensure!(iter_count == stored_count, "iter_count != stored_count"); ensure!(stored_count == nodes_count, "stored_count != nodes_count"); @@ -482,14 +487,15 @@ impl List { let active_bags = { let thresholds = T::BagThresholds::get().iter().copied(); - let thresholds: Vec = if thresholds.clone().last() == Some(VoteWeight::MAX) { - // in the event that they included it, we don't need to make any changes - thresholds.collect() - } else { - // otherwise, insert it here. - thresholds.chain(iter::once(VoteWeight::MAX)).collect() - }; - thresholds.into_iter().filter_map(|t| Bag::::get(t)) + let thresholds: Vec = + if thresholds.clone().last() == Some(T::Score::max_value()) { + // in the event that they included it, we don't need to make any changes + thresholds.collect() + } else { + // otherwise, insert it here. + thresholds.chain(iter::once(T::Score::max_value())).collect() + }; + thresholds.into_iter().filter_map(|t| Bag::::get(t)) }; let _ = active_bags.clone().map(|b| b.sanity_check()).collect::>()?; @@ -502,7 +508,7 @@ impl List { // check that all nodes are sane. We check the `ListNodes` storage item directly in case we // have some "stale" nodes that are not in a bag. - for (_id, node) in crate::ListNodes::::iter() { + for (_id, node) in crate::ListNodes::::iter() { node.sanity_check()? } @@ -517,21 +523,24 @@ impl List { /// Returns the nodes of all non-empty bags. For testing and benchmarks. #[cfg(any(feature = "std", feature = "runtime-benchmarks"))] #[allow(dead_code)] - pub(crate) fn get_bags() -> Vec<(VoteWeight, Vec)> { + pub(crate) fn get_bags() -> Vec<(T::Score, Vec)> { use frame_support::traits::Get as _; let thresholds = T::BagThresholds::get(); let iter = thresholds.iter().copied(); - let iter: Box> = if thresholds.last() == Some(&VoteWeight::MAX) { + let iter: Box> = if thresholds.last() == + Some(&T::Score::max_value()) + { // in the event that they included it, we can just pass the iterator through unchanged. Box::new(iter) } else { // otherwise, insert it here. - Box::new(iter.chain(sp_std::iter::once(VoteWeight::MAX))) + Box::new(iter.chain(sp_std::iter::once(T::Score::max_value()))) }; iter.filter_map(|t| { - Bag::::get(t).map(|bag| (t, bag.iter().map(|n| n.id().clone()).collect::>())) + Bag::::get(t) + .map(|bag| (t, bag.iter().map(|n| n.id().clone()).collect::>())) }) .collect::>() } @@ -546,37 +555,39 @@ impl List { /// appearing within the ids set. #[derive(DefaultNoBound, Encode, Decode, MaxEncodedLen, TypeInfo)] #[codec(mel_bound())] -#[scale_info(skip_type_params(T))] +#[scale_info(skip_type_params(T, I))] #[cfg_attr(feature = "std", derive(frame_support::DebugNoBound, Clone, PartialEq))] -pub struct Bag { +pub struct Bag, I: 'static = ()> { head: Option, tail: Option, #[codec(skip)] - bag_upper: VoteWeight, + bag_upper: T::Score, + #[codec(skip)] + _phantom: PhantomData, } -impl Bag { +impl, I: 'static> Bag { #[cfg(test)] pub(crate) fn new( head: Option, tail: Option, - bag_upper: VoteWeight, + bag_upper: T::Score, ) -> Self { - Self { head, tail, bag_upper } + Self { head, tail, bag_upper, _phantom: PhantomData } } - /// Get a bag by its upper vote weight. - pub(crate) fn get(bag_upper: VoteWeight) -> Option> { - crate::ListBags::::try_get(bag_upper).ok().map(|mut bag| { + /// Get a bag by its upper score. + pub(crate) fn get(bag_upper: T::Score) -> Option> { + crate::ListBags::::try_get(bag_upper).ok().map(|mut bag| { bag.bag_upper = bag_upper; bag }) } - /// Get a bag by its upper vote weight or make it, appropriately initialized. Does not check if + /// Get a bag by its upper score or make it, appropriately initialized. Does not check if /// if `bag_upper` is a valid threshold. - fn get_or_make(bag_upper: VoteWeight) -> Bag { + fn get_or_make(bag_upper: T::Score) -> Bag { Self::get(bag_upper).unwrap_or(Bag { bag_upper, ..Default::default() }) } @@ -588,24 +599,24 @@ impl Bag { /// Put the bag back into storage. fn put(self) { if self.is_empty() { - crate::ListBags::::remove(self.bag_upper); + crate::ListBags::::remove(self.bag_upper); } else { - crate::ListBags::::insert(self.bag_upper, self); + crate::ListBags::::insert(self.bag_upper, self); } } /// Get the head node in this bag. - fn head(&self) -> Option> { + fn head(&self) -> Option> { self.head.as_ref().and_then(|id| Node::get(id)) } /// Get the tail node in this bag. - fn tail(&self) -> Option> { + fn tail(&self) -> Option> { self.tail.as_ref().and_then(|id| Node::get(id)) } /// Iterate over the nodes in this bag. - pub(crate) fn iter(&self) -> impl Iterator> { + pub(crate) fn iter(&self) -> impl Iterator> { sp_std::iter::successors(self.head(), |prev| prev.next()) } @@ -620,7 +631,13 @@ impl Bag { // insert_node will overwrite `prev`, `next` and `bag_upper` to the proper values. As long // as this bag is the correct one, we're good. All calls to this must come after getting the // correct [`notional_bag_for`]. - self.insert_node_unchecked(Node:: { id, prev: None, next: None, bag_upper: 0 }); + self.insert_node_unchecked(Node:: { + id, + prev: None, + next: None, + bag_upper: Zero::zero(), + _phantom: PhantomData, + }); } /// Insert a node into this bag. @@ -630,7 +647,7 @@ impl Bag { /// /// Storage note: this modifies storage, but only for the node. You still need to call /// `self.put()` after use. - fn insert_node_unchecked(&mut self, mut node: Node) { + fn insert_node_unchecked(&mut self, mut node: Node) { if let Some(tail) = &self.tail { if *tail == node.id { // this should never happen, but this check prevents one path to a worst case @@ -674,7 +691,7 @@ impl Bag { /// /// Storage note: this modifies storage, but only for adjacent nodes. You still need to call /// `self.put()` and `ListNodes::remove(id)` to update storage for the bag and `node`. - fn remove_node_unchecked(&mut self, node: &Node) { + fn remove_node_unchecked(&mut self, node: &Node) { // reassign neighboring nodes. node.excise(); @@ -735,7 +752,7 @@ impl Bag { /// Iterate over the nodes in this bag (public for tests). #[cfg(feature = "std")] #[allow(dead_code)] - pub fn std_iter(&self) -> impl Iterator> { + pub fn std_iter(&self) -> impl Iterator> { sp_std::iter::successors(self.head(), |prev| prev.next()) } @@ -749,24 +766,26 @@ impl Bag { /// A Node is the fundamental element comprising the doubly-linked list described by `Bag`. #[derive(Encode, Decode, MaxEncodedLen, TypeInfo)] #[codec(mel_bound())] -#[scale_info(skip_type_params(T))] +#[scale_info(skip_type_params(T, I))] #[cfg_attr(feature = "std", derive(frame_support::DebugNoBound, Clone, PartialEq))] -pub struct Node { +pub struct Node, I: 'static = ()> { id: T::AccountId, prev: Option, next: Option, - bag_upper: VoteWeight, + bag_upper: T::Score, + #[codec(skip)] + _phantom: PhantomData, } -impl Node { +impl, I: 'static> Node { /// Get a node by id. - pub fn get(id: &T::AccountId) -> Option> { - crate::ListNodes::::try_get(id).ok() + pub fn get(id: &T::AccountId) -> Option> { + crate::ListNodes::::try_get(id).ok() } /// Put the node back into storage. fn put(self) { - crate::ListNodes::::insert(self.id.clone(), self); + crate::ListNodes::::insert(self.id.clone(), self); } /// Update neighboring nodes to point to reach other. @@ -790,22 +809,22 @@ impl Node { /// /// It is naive because it does not check if the node has first been removed from its bag. fn remove_from_storage_unchecked(&self) { - crate::ListNodes::::remove(&self.id) + crate::ListNodes::::remove(&self.id) } /// Get the previous node in the bag. - fn prev(&self) -> Option> { + fn prev(&self) -> Option> { self.prev.as_ref().and_then(|id| Node::get(id)) } /// Get the next node in the bag. - fn next(&self) -> Option> { + fn next(&self) -> Option> { self.next.as_ref().and_then(|id| Node::get(id)) } /// `true` when this voter is in the wrong bag. - pub fn is_misplaced(&self, current_weight: VoteWeight) -> bool { - notional_bag_for::(current_weight) != self.bag_upper + pub fn is_misplaced(&self, current_score: T::Score) -> bool { + notional_bag_for::(current_score) != self.bag_upper } /// `true` when this voter is a bag head or tail. @@ -828,13 +847,13 @@ impl Node { /// The bag this nodes belongs to (public for benchmarks). #[cfg(feature = "runtime-benchmarks")] #[allow(dead_code)] - pub fn bag_upper(&self) -> VoteWeight { + pub fn bag_upper(&self) -> T::Score { self.bag_upper } #[cfg(feature = "std")] fn sanity_check(&self) -> Result<(), &'static str> { - let expected_bag = Bag::::get(self.bag_upper).ok_or("bag not found for node")?; + let expected_bag = Bag::::get(self.bag_upper).ok_or("bag not found for node")?; let id = self.id(); diff --git a/frame/bags-list/src/list/tests.rs b/frame/bags-list/src/list/tests.rs index aaa215b0af1ca..9b7a078b44284 100644 --- a/frame/bags-list/src/list/tests.rs +++ b/frame/bags-list/src/list/tests.rs @@ -20,14 +20,20 @@ use crate::{ mock::{test_utils::*, *}, ListBags, ListNodes, }; -use frame_election_provider_support::SortedListProvider; +use frame_election_provider_support::{SortedListProvider, VoteWeight}; use frame_support::{assert_ok, assert_storage_noop}; #[test] fn basic_setup_works() { ExtBuilder::default().build_and_execute(|| { // syntactic sugar to create a raw node - let node = |id, prev, next, bag_upper| Node:: { id, prev, next, bag_upper }; + let node = |id, prev, next, bag_upper| Node:: { + id, + prev, + next, + bag_upper, + _phantom: PhantomData, + }; assert_eq!(ListNodes::::count(), 4); assert_eq!(ListNodes::::iter().count(), 4); @@ -38,11 +44,11 @@ fn basic_setup_works() { // the state of the bags is as expected assert_eq!( ListBags::::get(10).unwrap(), - Bag:: { head: Some(1), tail: Some(1), bag_upper: 0 } + Bag:: { head: Some(1), tail: Some(1), bag_upper: 0, _phantom: PhantomData } ); assert_eq!( ListBags::::get(1_000).unwrap(), - Bag:: { head: Some(2), tail: Some(4), bag_upper: 0 } + Bag:: { head: Some(2), tail: Some(4), bag_upper: 0, _phantom: PhantomData } ); assert_eq!(ListNodes::::get(2).unwrap(), node(2, None, Some(3), 1_000)); @@ -65,24 +71,24 @@ fn basic_setup_works() { #[test] fn notional_bag_for_works() { // under a threshold gives the next threshold. - assert_eq!(notional_bag_for::(0), 10); - assert_eq!(notional_bag_for::(9), 10); + assert_eq!(notional_bag_for::(0), 10); + assert_eq!(notional_bag_for::(9), 10); // at a threshold gives that threshold. - assert_eq!(notional_bag_for::(10), 10); + assert_eq!(notional_bag_for::(10), 10); // above the threshold, gives the next threshold. - assert_eq!(notional_bag_for::(11), 20); + assert_eq!(notional_bag_for::(11), 20); let max_explicit_threshold = *::BagThresholds::get().last().unwrap(); assert_eq!(max_explicit_threshold, 10_000); - // if the max explicit threshold is less than VoteWeight::MAX, + // if the max explicit threshold is less than T::Value::max_value(), assert!(VoteWeight::MAX > max_explicit_threshold); - // then anything above it will belong to the VoteWeight::MAX bag. - assert_eq!(notional_bag_for::(max_explicit_threshold), max_explicit_threshold); - assert_eq!(notional_bag_for::(max_explicit_threshold + 1), VoteWeight::MAX); + // then anything above it will belong to the T::Value::max_value() bag. + assert_eq!(notional_bag_for::(max_explicit_threshold), max_explicit_threshold); + assert_eq!(notional_bag_for::(max_explicit_threshold + 1), VoteWeight::MAX); } #[test] @@ -388,14 +394,26 @@ mod list { #[should_panic = "given nodes must always have a valid bag. qed."] fn put_in_front_of_panics_if_bag_not_found() { ExtBuilder::default().skip_genesis_ids().build_and_execute_no_post_check(|| { - let node_10_no_bag = Node:: { id: 10, prev: None, next: None, bag_upper: 15 }; - let node_11_no_bag = Node:: { id: 11, prev: None, next: None, bag_upper: 15 }; + let node_10_no_bag = Node:: { + id: 10, + prev: None, + next: None, + bag_upper: 15, + _phantom: PhantomData, + }; + let node_11_no_bag = Node:: { + id: 11, + prev: None, + next: None, + bag_upper: 15, + _phantom: PhantomData, + }; // given ListNodes::::insert(10, node_10_no_bag); ListNodes::::insert(11, node_11_no_bag); - StakingMock::set_vote_weight_of(&10, 14); - StakingMock::set_vote_weight_of(&11, 15); + StakingMock::set_score_of(&10, 14); + StakingMock::set_score_of(&11, 15); assert!(!ListBags::::contains_key(15)); assert_eq!(List::::get_bags(), vec![]); @@ -414,8 +432,13 @@ mod list { assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); // implicitly also test that `node`'s `prev`/`next` are correctly re-assigned. - let node_42 = - Node:: { id: 42, prev: Some(1), next: Some(2), bag_upper: 1_000 }; + let node_42 = Node:: { + id: 42, + prev: Some(1), + next: Some(2), + bag_upper: 1_000, + _phantom: PhantomData, + }; assert!(!crate::ListNodes::::contains_key(42)); let node_1 = crate::ListNodes::::get(&1).unwrap(); @@ -438,7 +461,13 @@ mod list { assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); // implicitly also test that `node`'s `prev`/`next` are correctly re-assigned. - let node_42 = Node:: { id: 42, prev: Some(4), next: None, bag_upper: 1_000 }; + let node_42 = Node:: { + id: 42, + prev: Some(4), + next: None, + bag_upper: 1_000, + _phantom: PhantomData, + }; assert!(!crate::ListNodes::::contains_key(42)); let node_2 = crate::ListNodes::::get(&2).unwrap(); @@ -461,7 +490,13 @@ mod list { assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); // implicitly also test that `node`'s `prev`/`next` are correctly re-assigned. - let node_42 = Node:: { id: 42, prev: None, next: Some(2), bag_upper: 1_000 }; + let node_42 = Node:: { + id: 42, + prev: None, + next: Some(2), + bag_upper: 1_000, + _phantom: PhantomData, + }; assert!(!crate::ListNodes::::contains_key(42)); let node_3 = crate::ListNodes::::get(&3).unwrap(); @@ -484,8 +519,13 @@ mod list { assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); // implicitly also test that `node`'s `prev`/`next` are correctly re-assigned. - let node_42 = - Node:: { id: 42, prev: Some(42), next: Some(42), bag_upper: 1_000 }; + let node_42 = Node:: { + id: 42, + prev: Some(42), + next: Some(42), + bag_upper: 1_000, + _phantom: PhantomData, + }; assert!(!crate::ListNodes::::contains_key(42)); let node_4 = crate::ListNodes::::get(&4).unwrap(); @@ -512,7 +552,7 @@ mod bags { let bag = Bag::::get(bag_upper).unwrap(); let bag_ids = bag.iter().map(|n| *n.id()).collect::>(); - assert_eq!(bag, Bag:: { head, tail, bag_upper }); + assert_eq!(bag, Bag:: { head, tail, bag_upper, _phantom: PhantomData }); assert_eq!(bag_ids, ids); }; @@ -543,7 +583,13 @@ mod bags { #[test] fn insert_node_sets_proper_bag() { ExtBuilder::default().build_and_execute_no_post_check(|| { - let node = |id, bag_upper| Node:: { id, prev: None, next: None, bag_upper }; + let node = |id, bag_upper| Node:: { + id, + prev: None, + next: None, + bag_upper, + _phantom: PhantomData, + }; assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); @@ -552,7 +598,7 @@ mod bags { assert_eq!( ListNodes::::get(&42).unwrap(), - Node { bag_upper: 10, prev: Some(1), next: None, id: 42 } + Node { bag_upper: 10, prev: Some(1), next: None, id: 42, _phantom: PhantomData } ); }); } @@ -560,7 +606,13 @@ mod bags { #[test] fn insert_node_happy_paths_works() { ExtBuilder::default().build_and_execute_no_post_check(|| { - let node = |id, bag_upper| Node:: { id, prev: None, next: None, bag_upper }; + let node = |id, bag_upper| Node:: { + id, + prev: None, + next: None, + bag_upper, + _phantom: PhantomData, + }; // when inserting into a bag with 1 node let mut bag_10 = Bag::::get(10).unwrap(); @@ -581,15 +633,26 @@ mod bags { assert_eq!(bag_as_ids(&bag_20), vec![62]); // when inserting a node pointing to the accounts not in the bag - let node_61 = - Node:: { id: 61, prev: Some(21), next: Some(101), bag_upper: 20 }; + let node_61 = Node:: { + id: 61, + prev: Some(21), + next: Some(101), + bag_upper: 20, + _phantom: PhantomData, + }; bag_20.insert_node_unchecked(node_61); // then ids are in order assert_eq!(bag_as_ids(&bag_20), vec![62, 61]); // and when the node is re-fetched all the info is correct assert_eq!( Node::::get(&61).unwrap(), - Node:: { id: 61, prev: Some(62), next: None, bag_upper: 20 } + Node:: { + id: 61, + prev: Some(62), + next: None, + bag_upper: 20, + _phantom: PhantomData, + } ); // state of all bags is as expected @@ -604,7 +667,13 @@ mod bags { // Document improper ways `insert_node` may be getting used. #[test] fn insert_node_bad_paths_documented() { - let node = |id, prev, next, bag_upper| Node:: { id, prev, next, bag_upper }; + let node = |id, prev, next, bag_upper| Node:: { + id, + prev, + next, + bag_upper, + _phantom: PhantomData, + }; ExtBuilder::default().build_and_execute_no_post_check(|| { // when inserting a node with both prev & next pointing at an account in an incorrect // bag. @@ -657,7 +726,10 @@ mod bags { ); // ^^^ despite being the bags head, it has a prev - assert_eq!(bag_1000, Bag { head: Some(2), tail: Some(2), bag_upper: 1_000 }) + assert_eq!( + bag_1000, + Bag { head: Some(2), tail: Some(2), bag_upper: 1_000, _phantom: PhantomData } + ) }); } @@ -669,7 +741,13 @@ mod bags { )] fn insert_node_duplicate_tail_panics_with_debug_assert() { ExtBuilder::default().build_and_execute(|| { - let node = |id, prev, next, bag_upper| Node:: { id, prev, next, bag_upper }; + let node = |id, prev, next, bag_upper| Node:: { + id, + prev, + next, + bag_upper, + _phantom: PhantomData, + }; // given assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])],); @@ -801,6 +879,7 @@ mod bags { prev: None, next: Some(3), bag_upper: 10, // should be 1_000 + _phantom: PhantomData, }; let mut bag_1000 = Bag::::get(1_000).unwrap(); diff --git a/frame/bags-list/src/mock.rs b/frame/bags-list/src/mock.rs index aa3f549e12dec..fce1431054174 100644 --- a/frame/bags-list/src/mock.rs +++ b/frame/bags-list/src/mock.rs @@ -27,19 +27,21 @@ pub type AccountId = u32; pub type Balance = u32; parameter_types! { - // Set the vote weight for any id who's weight has _not_ been set with `set_vote_weight_of`. + // Set the vote weight for any id who's weight has _not_ been set with `set_score_of`. pub static NextVoteWeight: VoteWeight = 0; pub static NextVoteWeightMap: HashMap = Default::default(); } pub struct StakingMock; -impl frame_election_provider_support::VoteWeightProvider for StakingMock { - fn vote_weight(id: &AccountId) -> VoteWeight { +impl frame_election_provider_support::ScoreProvider for StakingMock { + type Score = VoteWeight; + + fn score(id: &AccountId) -> Self::Score { *NextVoteWeightMap::get().get(id).unwrap_or(&NextVoteWeight::get()) } #[cfg(any(feature = "runtime-benchmarks", test))] - fn set_vote_weight_of(id: &AccountId, weight: VoteWeight) { + fn set_score_of(id: &AccountId, weight: Self::Score) { NEXT_VOTE_WEIGHT_MAP.with(|m| m.borrow_mut().insert(id.clone(), weight)); } } @@ -79,7 +81,8 @@ impl bags_list::Config for Runtime { type Event = Event; type WeightInfo = (); type BagThresholds = BagThresholds; - type VoteWeightProvider = StakingMock; + type ScoreProvider = StakingMock; + type Score = VoteWeight; } type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; @@ -134,7 +137,7 @@ impl ExtBuilder { ext.execute_with(|| { for (id, weight) in ids_with_weight { frame_support::assert_ok!(List::::insert(*id, *weight)); - StakingMock::set_vote_weight_of(id, *weight); + StakingMock::set_score_of(id, *weight); } }); diff --git a/frame/bags-list/src/tests.rs b/frame/bags-list/src/tests.rs index 43397b3c120f5..99396c9cbb3e3 100644 --- a/frame/bags-list/src/tests.rs +++ b/frame/bags-list/src/tests.rs @@ -18,7 +18,7 @@ use frame_support::{assert_noop, assert_ok, assert_storage_noop, traits::IntegrityTest}; use super::*; -use frame_election_provider_support::SortedListProvider; +use frame_election_provider_support::{SortedListProvider, VoteWeight}; use list::Bag; use mock::{test_utils::*, *}; @@ -35,7 +35,7 @@ mod pallet { ); // when increasing vote weight to the level of non-existent bag - StakingMock::set_vote_weight_of(&42, 2_000); + StakingMock::set_score_of(&42, 2_000); assert_ok!(BagsList::rebag(Origin::signed(0), 42)); // then a new bag is created and the id moves into it @@ -45,7 +45,7 @@ mod pallet { ); // when decreasing weight within the range of the current bag - StakingMock::set_vote_weight_of(&42, 1_001); + StakingMock::set_score_of(&42, 1_001); assert_ok!(BagsList::rebag(Origin::signed(0), 42)); // then the id does not move @@ -55,7 +55,7 @@ mod pallet { ); // when reducing weight to the level of a non-existent bag - StakingMock::set_vote_weight_of(&42, 30); + StakingMock::set_score_of(&42, 30); assert_ok!(BagsList::rebag(Origin::signed(0), 42)); // then a new bag is created and the id moves into it @@ -65,7 +65,7 @@ mod pallet { ); // when increasing weight to the level of a pre-existing bag - StakingMock::set_vote_weight_of(&42, 500); + StakingMock::set_score_of(&42, 500); assert_ok!(BagsList::rebag(Origin::signed(0), 42)); // then the id moves into that bag @@ -85,7 +85,7 @@ mod pallet { assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); // when - StakingMock::set_vote_weight_of(&4, 10); + StakingMock::set_score_of(&4, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 4)); // then @@ -93,7 +93,7 @@ mod pallet { assert_eq!(Bag::::get(1_000).unwrap(), Bag::new(Some(2), Some(3), 1_000)); // when - StakingMock::set_vote_weight_of(&3, 10); + StakingMock::set_score_of(&3, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 3)); // then @@ -104,7 +104,7 @@ mod pallet { assert_eq!(get_list_as_ids(), vec![2u32, 1, 4, 3]); // when - StakingMock::set_vote_weight_of(&2, 10); + StakingMock::set_score_of(&2, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 2)); // then @@ -119,7 +119,7 @@ mod pallet { fn rebag_head_works() { ExtBuilder::default().build_and_execute(|| { // when - StakingMock::set_vote_weight_of(&2, 10); + StakingMock::set_score_of(&2, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 2)); // then @@ -127,7 +127,7 @@ mod pallet { assert_eq!(Bag::::get(1_000).unwrap(), Bag::new(Some(3), Some(4), 1_000)); // when - StakingMock::set_vote_weight_of(&3, 10); + StakingMock::set_score_of(&3, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 3)); // then @@ -135,7 +135,7 @@ mod pallet { assert_eq!(Bag::::get(1_000).unwrap(), Bag::new(Some(4), Some(4), 1_000)); // when - StakingMock::set_vote_weight_of(&4, 10); + StakingMock::set_score_of(&4, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 4)); // then @@ -241,7 +241,7 @@ mod pallet { // given assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4, 5])]); - StakingMock::set_vote_weight_of(&3, 999); + StakingMock::set_score_of(&3, 999); // when assert_ok!(BagsList::put_in_front_of(Origin::signed(4), 3)); @@ -262,7 +262,7 @@ mod pallet { vec![(10, vec![1]), (1_000, vec![2, 3, 4, 5, 6])] ); - StakingMock::set_vote_weight_of(&5, 999); + StakingMock::set_score_of(&5, 999); // when assert_ok!(BagsList::put_in_front_of(Origin::signed(3), 5)); @@ -281,7 +281,7 @@ mod pallet { // given assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); - StakingMock::set_vote_weight_of(&2, 999); + StakingMock::set_score_of(&2, 999); // when assert_ok!(BagsList::put_in_front_of(Origin::signed(3), 2)); @@ -297,7 +297,7 @@ mod pallet { // given assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); - StakingMock::set_vote_weight_of(&3, 999); + StakingMock::set_score_of(&3, 999); // when assert_ok!(BagsList::put_in_front_of(Origin::signed(4), 3)); @@ -313,7 +313,7 @@ mod pallet { // given assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4, 5])]); - StakingMock::set_vote_weight_of(&2, 999); + StakingMock::set_score_of(&2, 999); // when assert_ok!(BagsList::put_in_front_of(Origin::signed(5), 2)); @@ -329,7 +329,7 @@ mod pallet { // given assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4, 5])]); - StakingMock::set_vote_weight_of(&4, 999); + StakingMock::set_score_of(&4, 999); // when BagsList::put_in_front_of(Origin::signed(2), 4).unwrap(); @@ -359,7 +359,7 @@ mod pallet { // given assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); - StakingMock::set_vote_weight_of(&4, 999); + StakingMock::set_score_of(&4, 999); // when BagsList::put_in_front_of(Origin::signed(2), 4).unwrap(); @@ -375,7 +375,7 @@ mod pallet { // given assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); - StakingMock::set_vote_weight_of(&3, 999); + StakingMock::set_score_of(&3, 999); // then assert_noop!( diff --git a/frame/election-provider-support/Cargo.toml b/frame/election-provider-support/Cargo.toml index e66774c6b5e71..b95bd994fa70a 100644 --- a/frame/election-provider-support/Cargo.toml +++ b/frame/election-provider-support/Cargo.toml @@ -18,12 +18,12 @@ scale-info = { version = "2.0.1", default-features = false, features = ["derive" sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } sp-arithmetic = { version = "5.0.0", default-features = false, path = "../../primitives/arithmetic" } sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../primitives/npos-elections" } +sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } [dev-dependencies] sp-npos-elections = { version = "4.0.0-dev", path = "../../primitives/npos-elections" } -sp-runtime = { version = "6.0.0", path = "../../primitives/runtime" } sp-core = { version = "6.0.0", path = "../../primitives/core" } sp-io = { version = "6.0.0", path = "../../primitives/io" } diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index ca22e3093855c..ff57eacf68fa0 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -168,6 +168,7 @@ pub mod onchain; use frame_support::{traits::Get, BoundedVec}; +use sp_runtime::traits::Bounded; use sp_std::{fmt::Debug, prelude::*}; /// Re-export some type as they are used in the interface. @@ -333,9 +334,7 @@ where /// This is generic over `AccountId` and it can represent a validator, a nominator, or any other /// entity. /// -/// To simplify the trait, the `VoteWeight` is hardcoded as the weight of each entity. The weights -/// are ascending, the higher, the better. In the long term, if this trait ends up having use cases -/// outside of the election context, it is easy enough to make it generic over the `VoteWeight`. +/// The scores (see [`Self::Score`]) are ascending, the higher, the better. /// /// Something that implements this trait will do a best-effort sort over ids, and thus can be /// used on the implementing side of [`ElectionDataProvider`]. @@ -343,6 +342,9 @@ pub trait SortedListProvider { /// The list's error type. type Error: sp_std::fmt::Debug; + /// The type used by the list to compare nodes for ordering. + type Score: Bounded; + /// An iterator over the list, which can have `take` called on it. fn iter() -> Box>; @@ -353,10 +355,10 @@ pub trait SortedListProvider { fn contains(id: &AccountId) -> bool; /// Hook for inserting a new id. - fn on_insert(id: AccountId, weight: VoteWeight) -> Result<(), Self::Error>; + fn on_insert(id: AccountId, score: Self::Score) -> Result<(), Self::Error>; /// Hook for updating a single id. - fn on_update(id: &AccountId, weight: VoteWeight); + fn on_update(id: &AccountId, score: Self::Score); /// Hook for removing am id from the list. fn on_remove(id: &AccountId); @@ -371,7 +373,7 @@ pub trait SortedListProvider { /// new list, which can lead to too many storage accesses, exhausting the block weight. fn unsafe_regenerate( all: impl IntoIterator, - weight_of: Box VoteWeight>, + score_of: Box Self::Score>, ) -> u32; /// Remove all items from the list. @@ -388,21 +390,23 @@ pub trait SortedListProvider { /// If `who` changes by the returned amount they are guaranteed to have a worst case change /// in their list position. #[cfg(feature = "runtime-benchmarks")] - fn weight_update_worst_case(_who: &AccountId, _is_increase: bool) -> VoteWeight { - VoteWeight::MAX + fn score_update_worst_case(_who: &AccountId, _is_increase: bool) -> Self::Score { + Self::Score::max_value() } } /// Something that can provide the `VoteWeight` of an account. Similar to [`ElectionProvider`] and /// [`ElectionDataProvider`], this should typically be implementing by whoever is supposed to *use* /// `SortedListProvider`. -pub trait VoteWeightProvider { - /// Get the current `VoteWeight` of `who`. - fn vote_weight(who: &AccountId) -> VoteWeight; +pub trait ScoreProvider { + type Score; + + /// Get the current `Score` of `who`. + fn score(who: &AccountId) -> Self::Score; /// For tests and benchmarks, set the `VoteWeight`. #[cfg(any(feature = "runtime-benchmarks", test))] - fn set_vote_weight_of(_: &AccountId, _: VoteWeight) {} + fn set_score_of(_: &AccountId, _: Self::Score) {} } /// Something that can compute the result to an NPoS solution. diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index ec7d36c820f3e..3e57b647ac80d 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::SortedListProvider::score_update_worst_case(&origin_stash1, is_increase); let total_issuance = T::Currency::total_issuance(); diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 12d3804b4e303..843791a46ade2 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -18,7 +18,7 @@ //! Test utilities use crate::{self as pallet_staking, *}; -use frame_election_provider_support::{onchain, SortedListProvider}; +use frame_election_provider_support::{onchain, SortedListProvider, VoteWeight}; use frame_support::{ assert_ok, parameter_types, traits::{ @@ -240,8 +240,9 @@ parameter_types! { impl pallet_bags_list::Config for Test { type Event = Event; type WeightInfo = (); - type VoteWeightProvider = Staking; + type ScoreProvider = Staking; type BagThresholds = BagThresholds; + type Score = VoteWeight; } impl onchain::Config for Test { diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 5cd0d0107f015..a4aadb16ab1b9 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -18,8 +18,8 @@ //! Implementations for the Staking FRAME Pallet. use frame_election_provider_support::{ - data_provider, ElectionDataProvider, ElectionProvider, SortedListProvider, Supports, - VoteWeight, VoteWeightProvider, VoterOf, + data_provider, ElectionDataProvider, ElectionProvider, ScoreProvider, SortedListProvider, + Supports, VoteWeight, VoterOf, }; use frame_support::{ pallet_prelude::*, @@ -1244,13 +1244,15 @@ where } } -impl VoteWeightProvider for Pallet { - fn vote_weight(who: &T::AccountId) -> VoteWeight { +impl ScoreProvider for Pallet { + type Score = VoteWeight; + + fn score(who: &T::AccountId) -> Self::Score { Self::weight_of(who) } #[cfg(feature = "runtime-benchmarks")] - fn set_vote_weight_of(who: &T::AccountId, weight: VoteWeight) { + fn set_score_of(who: &T::AccountId, weight: Self::Score) { // this will clearly results in an inconsistent state, but it should not matter for a // benchmark. let active: BalanceOf = weight.try_into().map_err(|_| ()).unwrap(); @@ -1279,6 +1281,7 @@ impl VoteWeightProvider for Pallet { pub struct UseNominatorsMap(sp_std::marker::PhantomData); impl SortedListProvider for UseNominatorsMap { type Error = (); + type Score = VoteWeight; /// Returns iterator over voter list, which can have `take` called on it. fn iter() -> Box> { @@ -1290,11 +1293,11 @@ impl SortedListProvider for UseNominatorsMap { fn contains(id: &T::AccountId) -> bool { Nominators::::contains_key(id) } - fn on_insert(_: T::AccountId, _weight: VoteWeight) -> Result<(), Self::Error> { + fn on_insert(_: T::AccountId, _weight: Self::Score) -> Result<(), Self::Error> { // nothing to do on insert. Ok(()) } - fn on_update(_: &T::AccountId, _weight: VoteWeight) { + fn on_update(_: &T::AccountId, _weight: Self::Score) { // nothing to do on update. } fn on_remove(_: &T::AccountId) { @@ -1302,7 +1305,7 @@ impl SortedListProvider for UseNominatorsMap { } fn unsafe_regenerate( _: impl IntoIterator, - _: Box VoteWeight>, + _: Box Self::Score>, ) -> u32 { // nothing to do upon regenerate. 0 diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 58f9fd237263b..b50ce5ed4502c 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -166,7 +166,10 @@ 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. - type SortedListProvider: SortedListProvider; + type SortedListProvider: SortedListProvider< + Self::AccountId, + Score = frame_election_provider_support::VoteWeight, + >; /// The maximum number of `unlocking` chunks a [`StakingLedger`] can have. Effectively /// determines how many unique eras a staker may be unbonding in.