diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index 747bc71c5007c..6fd57e31e466e 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -23,8 +23,8 @@ use hex_literal::hex; use node_runtime::{ constants::currency::*, wasm_binary_unwrap, AuthorityDiscoveryConfig, BabeConfig, BalancesConfig, Block, CouncilConfig, DemocracyConfig, ElectionsConfig, GrandpaConfig, - ImOnlineConfig, IndicesConfig, SessionConfig, SessionKeys, SocietyConfig, StakerStatus, - StakingConfig, SudoConfig, SystemConfig, TechnicalCommitteeConfig, MAX_NOMINATIONS, + ImOnlineConfig, IndicesConfig, MaxNominations, SessionConfig, SessionKeys, SocietyConfig, + StakerStatus, StakingConfig, SudoConfig, SystemConfig, TechnicalCommitteeConfig, }; use pallet_im_online::sr25519::AuthorityId as ImOnlineId; use sc_chain_spec::ChainSpecExtension; @@ -278,7 +278,7 @@ pub fn testnet_genesis( .map(|x| (x.0.clone(), x.1.clone(), STASH, StakerStatus::Validator)) .chain(initial_nominators.iter().map(|x| { use rand::{seq::SliceRandom, Rng}; - let limit = (MAX_NOMINATIONS as usize).min(initial_authorities.len()); + let limit = (MaxNominations::get() as usize).min(initial_authorities.len()); let count = rng.gen::() % limit; let nominations = initial_authorities .as_slice() diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 6034b86987ffb..8f00dd8f64b4c 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -541,7 +541,7 @@ impl pallet_staking::BenchmarkingConfig for StakingBenchmarkingConfig { } impl pallet_staking::Config for Runtime { - const MAX_NOMINATIONS: u32 = MAX_NOMINATIONS; + type MaxNominations = MaxNominations; type Currency = Balances; type UnixTime = Timestamp; type CurrencyToVote = U128CurrencyToVote; @@ -605,7 +605,9 @@ sp_npos_elections::generate_solution_type!( >(16) ); -pub const MAX_NOMINATIONS: u32 = ::LIMIT as u32; +parameter_types! { + pub MaxNominations: u32 = ::LIMIT as u32; +} /// The numbers configured here could always be more than the the maximum limits of staking pallet /// to ensure election snapshot will not run out of memory. For now, we set them to smaller values @@ -1792,7 +1794,7 @@ mod tests { #[test] fn perbill_as_onchain_accuracy() { type OnChainAccuracy = ::Accuracy; - let maximum_chain_accuracy: Vec> = (0..MAX_NOMINATIONS) + let maximum_chain_accuracy: Vec> = (0..MaxNominations::get()) .map(|_| >::from(OnChainAccuracy::one().deconstruct())) .collect(); let _: UpperOf = diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index fa7a7b185b342..2e4b6023f1e5f 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -178,7 +178,7 @@ impl onchain::Config for Test { } impl pallet_staking::Config for Test { - const MAX_NOMINATIONS: u32 = 16; + type MaxNominations = ConstU32<16>; type RewardRemainder = (); type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote; type Event = Event; diff --git a/frame/election-provider-multi-phase/src/benchmarking.rs b/frame/election-provider-multi-phase/src/benchmarking.rs index d605c5131cf77..d37b9451b770f 100644 --- a/frame/election-provider-multi-phase/src/benchmarking.rs +++ b/frame/election-provider-multi-phase/src/benchmarking.rs @@ -20,7 +20,11 @@ use super::*; use crate::{unsigned::IndexAssignmentOf, Pallet as MultiPhase}; use frame_benchmarking::account; -use frame_support::{assert_ok, traits::Hooks}; +use frame_support::{ + assert_ok, + traits::{Hooks, TryCollect}, + BoundedVec, +}; use frame_system::RawOrigin; use rand::{prelude::SliceRandom, rngs::SmallRng, SeedableRng}; use sp_arithmetic::{per_things::Percent, traits::One}; @@ -69,11 +73,12 @@ fn solution_with_size( let active_voters = (0..active_voters_count) .map(|i| { // chose a random subset of winners. - let winner_votes = winners + let winner_votes: BoundedVec<_, _> = winners .as_slice() .choose_multiple(&mut rng, >::LIMIT) .cloned() - .collect::>(); + .try_collect() + .expect(">::LIMIT is the correct bound; qed."); let voter = frame_benchmarking::account::("Voter", i, SEED); (voter, stake, winner_votes) }) @@ -87,10 +92,11 @@ fn solution_with_size( .collect::>(); let rest_voters = (active_voters_count..size.voters) .map(|i| { - let votes = (&non_winners) + let votes: BoundedVec<_, _> = (&non_winners) .choose_multiple(&mut rng, >::LIMIT) .cloned() - .collect::>(); + .try_collect() + .expect(">::LIMIT is the correct bound; qed."); let voter = frame_benchmarking::account::("Voter", i, SEED); (voter, stake, votes) }) @@ -152,7 +158,7 @@ fn set_up_data_provider(v: u32, t: u32) { info, "setting up with voters = {} [degree = {}], targets = {}", v, - T::DataProvider::MAXIMUM_VOTES_PER_VOTER, + ::MaxVotesPerVoter::get(), t ); @@ -165,14 +171,16 @@ fn set_up_data_provider(v: u32, t: u32) { }) .collect::>(); // we should always have enough voters to fill. - assert!(targets.len() > T::DataProvider::MAXIMUM_VOTES_PER_VOTER as usize); - targets.truncate(T::DataProvider::MAXIMUM_VOTES_PER_VOTER as usize); + assert!( + targets.len() > ::MaxVotesPerVoter::get() as usize + ); + targets.truncate(::MaxVotesPerVoter::get() as usize); // fill voters. (0..v).for_each(|i| { let voter = frame_benchmarking::account::("Voter", i, SEED); let weight = T::Currency::minimum_balance().saturated_into::() * 1000; - T::DataProvider::add_voter(voter, weight, targets.clone()); + T::DataProvider::add_voter(voter, weight, targets.clone().try_into().unwrap()); }); } diff --git a/frame/election-provider-multi-phase/src/helpers.rs b/frame/election-provider-multi-phase/src/helpers.rs index 9bd5b5dbabd25..48da194cc65d9 100644 --- a/frame/election-provider-multi-phase/src/helpers.rs +++ b/frame/election-provider-multi-phase/src/helpers.rs @@ -17,7 +17,7 @@ //! Some helper functions/macros for this crate. -use super::{Config, SolutionTargetIndexOf, SolutionVoterIndexOf, VoteWeight}; +use crate::{unsigned::VoterOf, Config, SolutionTargetIndexOf, SolutionVoterIndexOf, VoteWeight}; use sp_std::{collections::btree_map::BTreeMap, prelude::*}; #[macro_export] @@ -34,7 +34,7 @@ macro_rules! log { /// /// This can be used to efficiently build index getter closures. pub fn generate_voter_cache( - snapshot: &Vec<(T::AccountId, VoteWeight, Vec)>, + snapshot: &Vec>, ) -> BTreeMap { let mut cache: BTreeMap = BTreeMap::new(); snapshot.iter().enumerate().for_each(|(i, (x, _, _))| { @@ -97,7 +97,7 @@ pub fn voter_index_fn_usize( /// Not meant to be used in production. #[cfg(test)] pub fn voter_index_fn_linear( - snapshot: &Vec<(T::AccountId, VoteWeight, Vec)>, + snapshot: &Vec>, ) -> impl Fn(&T::AccountId) -> Option> + '_ { move |who| { snapshot @@ -148,7 +148,7 @@ pub fn target_index_fn_linear( /// Create a function that can map a voter index ([`SolutionVoterIndexOf`]) to the actual voter /// account using a linearly indexible snapshot. pub fn voter_at_fn( - snapshot: &Vec<(T::AccountId, VoteWeight, Vec)>, + snapshot: &Vec>, ) -> impl Fn(SolutionVoterIndexOf) -> Option + '_ { move |i| { as TryInto>::try_into(i) @@ -174,7 +174,7 @@ pub fn target_at_fn( /// This is not optimized and uses a linear search. #[cfg(test)] pub fn stake_of_fn_linear( - snapshot: &Vec<(T::AccountId, VoteWeight, Vec)>, + snapshot: &Vec>, ) -> impl Fn(&T::AccountId) -> VoteWeight + '_ { move |who| { snapshot @@ -192,7 +192,7 @@ pub fn stake_of_fn_linear( /// The cache need must be derived from the same snapshot. Zero is returned if a voter is /// non-existent. pub fn stake_of_fn<'a, T: Config>( - snapshot: &'a Vec<(T::AccountId, VoteWeight, Vec)>, + snapshot: &'a Vec>, cache: &'a BTreeMap, ) -> impl Fn(&T::AccountId) -> VoteWeight + 'a { move |who| { diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 4716bcd6c859b..f1d1d78912c46 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -269,6 +269,7 @@ const LOG_TARGET: &'static str = "runtime::election-provider"; pub mod signed; pub mod unsigned; pub mod weights; +use unsigned::VoterOf; pub use weights::WeightInfo; pub use signed::{ @@ -448,11 +449,13 @@ pub struct ReadySolution { /// /// These are stored together because they are often accessed together. #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, Default, TypeInfo)] -pub struct RoundSnapshot { +#[codec(mel_bound(T: Config))] +#[scale_info(skip_type_params(T))] +pub struct RoundSnapshot { /// All of the voters. - pub voters: Vec<(A, VoteWeight, Vec)>, + pub voters: Vec>, /// All of the targets. - pub targets: Vec, + pub targets: Vec, } /// Encodes the length of a solution or a snapshot. @@ -820,7 +823,7 @@ pub mod pallet { // NOTE that this pallet does not really need to enforce this in runtime. The // solution cannot represent any voters more than `LIMIT` anyhow. assert_eq!( - ::MAXIMUM_VOTES_PER_VOTER, + ::MaxVotesPerVoter::get(), as NposSolution>::LIMIT as u32, ); } @@ -1140,7 +1143,7 @@ pub mod pallet { /// This is created at the beginning of the signed phase and cleared upon calling `elect`. #[pallet::storage] #[pallet::getter(fn snapshot)] - pub type Snapshot = StorageValue<_, RoundSnapshot>; + pub type Snapshot = StorageValue<_, RoundSnapshot>; /// Desired number of targets to elect for this round. /// @@ -1257,7 +1260,7 @@ impl Pallet { /// Extracted for easier weight calculation. fn create_snapshot_internal( targets: Vec, - voters: Vec>, + voters: Vec>, desired_targets: u32, ) { let metadata = @@ -1270,7 +1273,7 @@ impl Pallet { // instead of using storage APIs, we do a manual encoding into a fixed-size buffer. // `encoded_size` encodes it without storing it anywhere, this should not cause any // allocation. - let snapshot = RoundSnapshot { voters, targets }; + let snapshot = RoundSnapshot:: { voters, targets }; let size = snapshot.encoded_size(); log!(debug, "snapshot pre-calculated size {:?}", size); let mut buffer = Vec::with_capacity(size); @@ -1288,7 +1291,7 @@ impl Pallet { /// /// Extracted for easier weight calculation. fn create_snapshot_external( - ) -> Result<(Vec, Vec>, u32), ElectionError> { + ) -> Result<(Vec, Vec>, u32), ElectionError> { let target_limit = >::max_value().saturated_into::(); // for now we have just a single block snapshot. let voter_limit = T::VoterSnapshotPerBlock::get().saturated_into::(); diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 409ebd3f1e10d..1be93c363e321 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -22,11 +22,12 @@ use frame_election_provider_support::{ }; pub use frame_support::{assert_noop, assert_ok}; use frame_support::{ - parameter_types, + bounded_vec, parameter_types, traits::{ConstU32, Hooks}, weights::Weight, + BoundedVec, }; -use multi_phase::unsigned::{IndexAssignmentOf, Voter}; +use multi_phase::unsigned::{IndexAssignmentOf, VoterOf}; use parking_lot::RwLock; use sp_core::{ offchain::{ @@ -100,7 +101,7 @@ pub fn roll_to_with_ocw(n: BlockNumber) { } pub struct TrimHelpers { - pub voters: Vec>, + pub voters: Vec>, pub assignments: Vec>, pub encoded_size_of: Box]) -> Result>, @@ -131,13 +132,8 @@ pub fn trim_helpers() -> TrimHelpers { let desired_targets = MultiPhase::desired_targets().unwrap(); - let ElectionResult { mut assignments, .. } = seq_phragmen::<_, SolutionAccuracyOf>( - desired_targets as usize, - targets.clone(), - voters.clone(), - None, - ) - .unwrap(); + let ElectionResult::<_, SolutionAccuracyOf> { mut assignments, .. } = + seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None).unwrap(); // sort by decreasing order of stake assignments.sort_unstable_by_key(|assignment| { @@ -163,14 +159,8 @@ pub fn raw_solution() -> RawSolution> { let RoundSnapshot { voters, targets } = MultiPhase::snapshot().unwrap(); let desired_targets = MultiPhase::desired_targets().unwrap(); - let ElectionResult { winners: _, assignments } = - seq_phragmen::<_, SolutionAccuracyOf>( - desired_targets as usize, - targets.clone(), - voters.clone(), - None, - ) - .unwrap(); + let ElectionResult::<_, SolutionAccuracyOf> { winners: _, assignments } = + seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None).unwrap(); // closures let cache = helpers::generate_voter_cache::(&voters); @@ -246,16 +236,16 @@ impl pallet_balances::Config for Runtime { parameter_types! { pub static Targets: Vec = vec![10, 20, 30, 40]; - pub static Voters: Vec<(AccountId, VoteWeight, Vec)> = vec![ - (1, 10, vec![10, 20]), - (2, 10, vec![30, 40]), - (3, 10, vec![40]), - (4, 10, vec![10, 20, 30, 40]), + pub static Voters: Vec> = vec![ + (1, 10, bounded_vec![10, 20]), + (2, 10, bounded_vec![30, 40]), + (3, 10, bounded_vec![40]), + (4, 10, bounded_vec![10, 20, 30, 40]), // self votes. - (10, 10, vec![10]), - (20, 20, vec![20]), - (30, 30, vec![30]), - (40, 40, vec![40]), + (10, 10, bounded_vec![10]), + (20, 20, bounded_vec![20]), + (30, 30, bounded_vec![30]), + (40, 40, bounded_vec![40]), ]; pub static DesiredTargets: u32 = 2; @@ -436,6 +426,10 @@ where pub type Extrinsic = sp_runtime::testing::TestXt; +parameter_types! { + pub MaxNominations: u32 = ::LIMIT as u32; +} + #[derive(Default)] pub struct ExtBuilder {} @@ -443,7 +437,7 @@ pub struct StakingMock; impl ElectionDataProvider for StakingMock { type AccountId = AccountId; type BlockNumber = u64; - const MAXIMUM_VOTES_PER_VOTER: u32 = ::LIMIT as u32; + type MaxVotesPerVoter = MaxNominations; fn targets(maybe_max_len: Option) -> data_provider::Result> { let targets = Targets::get(); @@ -454,9 +448,7 @@ impl ElectionDataProvider for StakingMock { Ok(targets) } - fn voters( - maybe_max_len: Option, - ) -> data_provider::Result)>> { + fn voters(maybe_max_len: Option) -> data_provider::Result>> { let mut voters = Voters::get(); if let Some(max_len) = maybe_max_len { voters.truncate(max_len) @@ -475,7 +467,7 @@ impl ElectionDataProvider for StakingMock { #[cfg(feature = "runtime-benchmarks")] fn put_snapshot( - voters: Vec<(AccountId, VoteWeight, Vec)>, + voters: Vec>, targets: Vec, _target_stake: Option, ) { @@ -490,7 +482,11 @@ impl ElectionDataProvider for StakingMock { } #[cfg(feature = "runtime-benchmarks")] - fn add_voter(voter: AccountId, weight: VoteWeight, targets: Vec) { + fn add_voter( + voter: AccountId, + weight: VoteWeight, + targets: frame_support::BoundedVec, + ) { let mut current = Voters::get(); current.push((voter, weight, targets)); Voters::set(current); @@ -505,7 +501,7 @@ impl ElectionDataProvider for StakingMock { // to be on-par with staking, we add a self vote as well. the stake is really not that // important. let mut current = Voters::get(); - current.push((target, ExistentialDeposit::get() as u64, vec![target])); + current.push((target, ExistentialDeposit::get() as u64, bounded_vec![target])); Voters::set(current); } } @@ -540,7 +536,12 @@ impl ExtBuilder { ::set(t); self } - pub fn add_voter(self, who: AccountId, stake: Balance, targets: Vec) -> Self { + pub fn add_voter( + self, + who: AccountId, + stake: Balance, + targets: BoundedVec, + ) -> Self { VOTERS.with(|v| v.borrow_mut().push((who, stake, targets))); self } diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index da56dd4d073df..936993b41fb6c 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -47,11 +47,7 @@ pub(crate) const OFFCHAIN_CACHED_CALL: &[u8] = b"parity/multi-phase-unsigned-ele /// A voter's fundamental data: their ID, their stake, and the list of candidates for whom they /// voted. -pub type Voter = ( - ::AccountId, - sp_npos_elections::VoteWeight, - Vec<::AccountId>, -); +pub type VoterOf = frame_election_provider_support::VoterOf<::DataProvider>; /// The relative distribution of a voter's stake among the winning targets. pub type Assignment = @@ -749,7 +745,9 @@ mod tests { }; use codec::Decode; use frame_benchmarking::Zero; - use frame_support::{assert_noop, assert_ok, dispatch::Dispatchable, traits::OffchainWorker}; + use frame_support::{ + assert_noop, assert_ok, bounded_vec, dispatch::Dispatchable, traits::OffchainWorker, + }; use sp_npos_elections::IndexAssignment; use sp_runtime::{ offchain::storage_lock::{BlockAndTime, StorageLock}, @@ -1048,8 +1046,8 @@ mod tests { fn unsigned_per_dispatch_checks_can_only_submit_threshold_better() { ExtBuilder::default() .desired_targets(1) - .add_voter(7, 2, vec![10]) - .add_voter(8, 5, vec![10]) + .add_voter(7, 2, bounded_vec![10]) + .add_voter(8, 5, bounded_vec![10]) .solution_improvement_threshold(Perbill::from_percent(50)) .build_and_execute(|| { roll_to(25); diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 889606da3428a..3374e1e97b8be 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -80,6 +80,7 @@ //! ```rust //! # use frame_election_provider_support::{*, data_provider}; //! # use sp_npos_elections::{Support, Assignment}; +//! # use frame_support::traits::ConstU32; //! //! type AccountId = u64; //! type Balance = u64; @@ -101,13 +102,13 @@ //! impl ElectionDataProvider for Pallet { //! type AccountId = AccountId; //! type BlockNumber = BlockNumber; -//! const MAXIMUM_VOTES_PER_VOTER: u32 = 1; +//! type MaxVotesPerVoter = ConstU32<1>; //! //! fn desired_targets() -> data_provider::Result { //! Ok(1) //! } //! fn voters(maybe_max_len: Option) -//! -> data_provider::Result)>> +//! -> data_provider::Result>> //! { //! Ok(Default::default()) //! } @@ -166,7 +167,7 @@ #![cfg_attr(not(feature = "std"), no_std)] pub mod onchain; -use frame_support::traits::Get; +use frame_support::{traits::Get, BoundedVec}; use sp_std::{fmt::Debug, prelude::*}; /// Re-export some type as they are used in the interface. @@ -191,7 +192,7 @@ pub trait ElectionDataProvider { type BlockNumber; /// Maximum number of votes per voter that this data provider is providing. - const MAXIMUM_VOTES_PER_VOTER: u32; + type MaxVotesPerVoter: Get; /// All possible targets for the election, i.e. the candidates. /// @@ -211,9 +212,7 @@ pub trait ElectionDataProvider { /// /// This should be implemented as a self-weighing function. The implementor should register its /// appropriate weight at the end of execution with the system pallet directly. - fn voters( - maybe_max_len: Option, - ) -> data_provider::Result)>>; + fn voters(maybe_max_len: Option) -> data_provider::Result>>; /// The number of targets to elect. /// @@ -233,7 +232,7 @@ pub trait ElectionDataProvider { /// else a noop. #[cfg(any(feature = "runtime-benchmarks", test))] fn put_snapshot( - _voters: Vec<(Self::AccountId, VoteWeight, Vec)>, + _voters: Vec>, _targets: Vec, _target_stake: Option, ) { @@ -244,7 +243,12 @@ pub trait ElectionDataProvider { /// /// Same as `put_snapshot`, but can add a single voter one by one. #[cfg(any(feature = "runtime-benchmarks", test))] - fn add_voter(_voter: Self::AccountId, _weight: VoteWeight, _targets: Vec) {} + fn add_voter( + _voter: Self::AccountId, + _weight: VoteWeight, + _targets: BoundedVec, + ) { + } /// Utility function only to be used in benchmarking scenarios, to be implemented optionally, /// else a noop. @@ -266,19 +270,20 @@ pub struct TestDataProvider(sp_std::marker::PhantomData); impl ElectionDataProvider for TestDataProvider<(AccountId, BlockNumber)> { type AccountId = AccountId; type BlockNumber = BlockNumber; + type MaxVotesPerVoter = (); - const MAXIMUM_VOTES_PER_VOTER: u32 = 0; fn targets(_maybe_max_len: Option) -> data_provider::Result> { Ok(Default::default()) } - fn voters( - _maybe_max_len: Option, - ) -> data_provider::Result)>> { + + fn voters(_maybe_max_len: Option) -> data_provider::Result>> { Ok(Default::default()) } + fn desired_targets() -> data_provider::Result { Ok(Default::default()) } + fn next_election_prediction(now: BlockNumber) -> BlockNumber { now } @@ -421,7 +426,7 @@ pub trait NposSolver { fn solve( to_elect: usize, targets: Vec, - voters: Vec<(Self::AccountId, VoteWeight, Vec)>, + voters: Vec<(Self::AccountId, VoteWeight, impl IntoIterator)>, ) -> Result, Self::Error>; } @@ -443,7 +448,7 @@ impl< fn solve( winners: usize, targets: Vec, - voters: Vec<(Self::AccountId, VoteWeight, Vec)>, + voters: Vec<(Self::AccountId, VoteWeight, impl IntoIterator)>, ) -> Result, Self::Error> { sp_npos_elections::seq_phragmen(winners, targets, voters, Balancing::get()) } @@ -467,8 +472,15 @@ impl< fn solve( winners: usize, targets: Vec, - voters: Vec<(Self::AccountId, VoteWeight, Vec)>, + voters: Vec<(Self::AccountId, VoteWeight, impl IntoIterator)>, ) -> Result, Self::Error> { sp_npos_elections::phragmms(winners, targets, voters, Balancing::get()) } } + +/// A voter, at the level of abstraction of this crate. +pub type Voter = (AccountId, VoteWeight, BoundedVec); + +/// Same as [`Voter`], but parameterized by an [`ElectionDataProvider`]. +pub type VoterOf = + Voter<::AccountId, ::MaxVotesPerVoter>; diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index d325daf514757..808b49ba6234d 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -87,9 +87,8 @@ impl ElectionProvider for OnChainSequentialPhragmen { let stake_of = |w: &T::AccountId| -> VoteWeight { stake_map.get(w).cloned().unwrap_or_default() }; - let ElectionResult { winners: _, assignments } = - seq_phragmen::<_, T::Accuracy>(desired_targets as usize, targets, voters, None) - .map_err(Error::from)?; + let ElectionResult::<_, T::Accuracy> { winners: _, assignments } = + seq_phragmen(desired_targets as usize, targets, voters, None).map_err(Error::from)?; let staked = assignment_ratio_to_staked_normalized(assignments, &stake_of)?; @@ -161,18 +160,22 @@ mod tests { type OnChainPhragmen = OnChainSequentialPhragmen; mod mock_data_provider { + use frame_support::{bounded_vec, traits::ConstU32}; + use super::*; - use crate::data_provider; + use crate::{data_provider, VoterOf}; pub struct DataProvider; impl ElectionDataProvider for DataProvider { type AccountId = AccountId; type BlockNumber = BlockNumber; - const MAXIMUM_VOTES_PER_VOTER: u32 = 2; - fn voters( - _: Option, - ) -> data_provider::Result)>> { - Ok(vec![(1, 10, vec![10, 20]), (2, 20, vec![30, 20]), (3, 30, vec![10, 30])]) + type MaxVotesPerVoter = ConstU32<2>; + fn voters(_: Option) -> data_provider::Result>> { + Ok(vec![ + (1, 10, bounded_vec![10, 20]), + (2, 20, bounded_vec![30, 20]), + (3, 30, bounded_vec![10, 30]), + ]) } fn targets(_: Option) -> data_provider::Result> { diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 2c53bb2222b76..bc6a81125e8bd 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -926,13 +926,13 @@ impl Pallet { let weight_candidates = candidates_and_deposit.len() as u32; let weight_voters = voters_and_votes.len() as u32; let weight_edges = num_edges; - let _ = sp_npos_elections::seq_phragmen::( + let _ = sp_npos_elections::seq_phragmen( num_to_elect, candidate_ids, voters_and_votes.clone(), None, ) - .map(|ElectionResult { winners, assignments: _ }| { + .map(|ElectionResult:: { winners, assignments: _ }| { // this is already sorted by id. let old_members_ids_sorted = >::take().into_iter().map(|m| m.who).collect::>(); diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index 34787c93bb9ce..d07f3136d9a0d 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -186,7 +186,7 @@ impl onchain::Config for Test { } impl pallet_staking::Config for Test { - const MAX_NOMINATIONS: u32 = 16; + type MaxNominations = ConstU32<16>; type RewardRemainder = (); type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote; type Event = Event; diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 4d6a182034c9e..c2e29f0d0fdd3 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -24,7 +24,7 @@ mod mock; use sp_std::{prelude::*, vec}; use frame_benchmarking::{account, benchmarks}; -use frame_support::traits::{Currency, ValidatorSet, ValidatorSetWithIdentification}; +use frame_support::traits::{Currency, Get, ValidatorSet, ValidatorSetWithIdentification}; use frame_system::{Config as SystemConfig, Pallet as System, RawOrigin}; use sp_runtime::{ @@ -275,7 +275,7 @@ benchmarks! { let r in 1 .. MAX_REPORTERS; // we skip 1 offender, because in such case there is no slashing let o in 2 .. MAX_OFFENDERS; - let n in 0 .. MAX_NOMINATORS.min(::MAX_NOMINATIONS); + let n in 0 .. MAX_NOMINATORS.min(::MaxNominations::get()); // Make r reporters let mut reporters = vec![]; @@ -381,7 +381,7 @@ benchmarks! { } report_offence_grandpa { - let n in 0 .. MAX_NOMINATORS.min(::MAX_NOMINATIONS); + let n in 0 .. MAX_NOMINATORS.min(::MaxNominations::get()); // for grandpa equivocation reports the number of reporters // and offenders is always 1 @@ -416,7 +416,7 @@ benchmarks! { } report_offence_babe { - let n in 0 .. MAX_NOMINATORS.min(::MAX_NOMINATIONS); + let n in 0 .. MAX_NOMINATORS.min(::MaxNominations::get()); // for babe equivocation reports the number of reporters // and offenders is always 1 diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index 598ba650dfffb..3b5e640867c5a 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -156,7 +156,7 @@ impl onchain::Config for Test { } impl pallet_staking::Config for Test { - const MAX_NOMINATIONS: u32 = 16; + type MaxNominations = ConstU32<16>; type Currency = Balances; type UnixTime = pallet_timestamp::Pallet; type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote; diff --git a/frame/session/benchmarking/src/lib.rs b/frame/session/benchmarking/src/lib.rs index 7beb4631e0518..265c35cbe4908 100644 --- a/frame/session/benchmarking/src/lib.rs +++ b/frame/session/benchmarking/src/lib.rs @@ -28,7 +28,7 @@ use sp_std::{prelude::*, vec}; use frame_benchmarking::benchmarks; use frame_support::{ codec::Decode, - traits::{KeyOwnerProofSystem, OnInitialize}, + traits::{Get, KeyOwnerProofSystem, OnInitialize}, }; use frame_system::RawOrigin; use pallet_session::{historical::Pallet as Historical, Pallet as Session, *}; @@ -53,10 +53,10 @@ impl OnInitialize for Pallet { benchmarks! { set_keys { - let n = ::MAX_NOMINATIONS; + let n = ::MaxNominations::get(); let (v_stash, _) = create_validator_with_nominators::( n, - ::MAX_NOMINATIONS, + ::MaxNominations::get(), false, RewardDestination::Staked, )?; @@ -70,10 +70,10 @@ benchmarks! { }: _(RawOrigin::Signed(v_controller), keys, proof) purge_keys { - let n = ::MAX_NOMINATIONS; + let n = ::MaxNominations::get(); let (v_stash, _) = create_validator_with_nominators::( n, - ::MAX_NOMINATIONS, + ::MaxNominations::get(), false, RewardDestination::Staked )?; diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs index 1f00236605130..37305437ca095 100644 --- a/frame/session/benchmarking/src/mock.rs +++ b/frame/session/benchmarking/src/mock.rs @@ -162,7 +162,7 @@ impl onchain::Config for Test { } impl pallet_staking::Config for Test { - const MAX_NOMINATIONS: u32 = 16; + type MaxNominations = ConstU32<16>; type Currency = Balances; type UnixTime = pallet_timestamp::Pallet; type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote; diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index 8328adc00a978..564172d912413 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -355,17 +355,17 @@ benchmarks! { kick { // scenario: we want to kick `k` nominators from nominating us (we are a validator). // we'll assume that `k` is under 128 for the purposes of determining the slope. - // each nominator should have `T::MAX_NOMINATIONS` validators nominated, and our validator + // each nominator should have `T::MaxNominations::get()` validators nominated, and our validator // should be somewhere in there. let k in 1 .. 128; - // these are the other validators; there are `T::MAX_NOMINATIONS - 1` of them, so - // there are a total of `T::MAX_NOMINATIONS` validators in the system. - let rest_of_validators = create_validators_with_seed::(T::MAX_NOMINATIONS - 1, 100, 415)?; + // these are the other validators; there are `T::MaxNominations::get() - 1` of them, so + // there are a total of `T::MaxNominations::get()` validators in the system. + let rest_of_validators = create_validators_with_seed::(T::MaxNominations::get() - 1, 100, 415)?; // this is the validator that will be kicking. let (stash, controller) = create_stash_controller::( - T::MAX_NOMINATIONS - 1, + T::MaxNominations::get() - 1, 100, Default::default(), )?; @@ -380,7 +380,7 @@ benchmarks! { for i in 0 .. k { // create a nominator stash. let (n_stash, n_controller) = create_stash_controller::( - T::MAX_NOMINATIONS + i, + T::MaxNominations::get() + i, 100, Default::default(), )?; @@ -415,9 +415,9 @@ benchmarks! { } } - // Worst case scenario, T::MAX_NOMINATIONS + // Worst case scenario, T::MaxNominations::get() nominate { - let n in 1 .. T::MAX_NOMINATIONS; + let n in 1 .. T::MaxNominations::get(); // clean up any existing state. clear_validators_and_nominators::(); @@ -428,7 +428,7 @@ benchmarks! { // we are just doing an insert into the origin position. let scenario = ListScenario::::new(origin_weight, true)?; let (stash, controller) = create_stash_controller_with_balance::( - SEED + T::MAX_NOMINATIONS + 1, // make sure the account does not conflict with others + SEED + T::MaxNominations::get() + 1, // make sure the account does not conflict with others origin_weight, Default::default(), ).unwrap(); @@ -724,7 +724,7 @@ benchmarks! { create_validators_with_nominators_for_era::( v, n, - ::MAX_NOMINATIONS as usize, + ::MaxNominations::get() as usize, false, None, )?; @@ -742,7 +742,7 @@ benchmarks! { create_validators_with_nominators_for_era::( v, n, - ::MAX_NOMINATIONS as usize, + ::MaxNominations::get() as usize, false, None, )?; @@ -822,7 +822,7 @@ benchmarks! { let s in 1 .. 20; let validators = create_validators_with_nominators_for_era::( - v, n, T::MAX_NOMINATIONS as usize, false, None + v, n, T::MaxNominations::get() as usize, false, None )? .into_iter() .map(|v| T::Lookup::lookup(v).unwrap()) @@ -845,7 +845,7 @@ benchmarks! { let n = MaxNominators::::get(); let _ = create_validators_with_nominators_for_era::( - v, n, T::MAX_NOMINATIONS as usize, false, None + v, n, T::MaxNominations::get() as usize, false, None )?; }: { let targets = >::get_npos_targets(); @@ -923,7 +923,7 @@ mod tests { create_validators_with_nominators_for_era::( v, n, - ::MAX_NOMINATIONS as usize, + ::MaxNominations::get() as usize, false, None, ) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index efe67c8b00c85..f1d4cd1e07c2a 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -303,6 +303,7 @@ use codec::{Decode, Encode, HasCompact}; use frame_support::{ traits::{ConstU32, Currency, Get}, weights::Weight, + BoundedVec, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; use scale_info::TypeInfo; use sp_runtime::{ @@ -574,10 +575,12 @@ where } /// A record of the nominations made by a specific account. -#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)] -pub struct Nominations { +#[derive(PartialEqNoBound, EqNoBound, Clone, Encode, Decode, RuntimeDebugNoBound, TypeInfo)] +#[codec(mel_bound(T: Config))] +#[scale_info(skip_type_params(T))] +pub struct Nominations { /// The targets of nomination. - pub targets: Vec, + pub targets: BoundedVec, /// The era the nominations were submitted. /// /// Except for initial nominations which are considered submitted at era 0. diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 3bf46588044a6..95f305dfdd22a 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -234,6 +234,7 @@ const THRESHOLDS: [sp_npos_elections::VoteWeight; 9] = parameter_types! { pub static BagThresholds: &'static [sp_npos_elections::VoteWeight] = &THRESHOLDS; + pub static MaxNominations: u32 = 16; } impl pallet_bags_list::Config for Test { @@ -249,7 +250,7 @@ impl onchain::Config for Test { } impl crate::pallet::pallet::Config for Test { - const MAX_NOMINATIONS: u32 = 16; + type MaxNominations = MaxNominations; type Currency = Balances; type UnixTime = Timestamp; type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote; @@ -533,7 +534,7 @@ fn post_conditions() { } fn check_count() { - let nominator_count = Nominators::::iter().count() as u32; + let nominator_count = Nominators::::iter_keys().count() as u32; let validator_count = Validators::::iter().count() as u32; assert_eq!(nominator_count, Nominators::::count()); assert_eq!(validator_count, Validators::::count()); diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index a72d2774dee2a..ae20550cd40b6 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -19,7 +19,7 @@ use frame_election_provider_support::{ data_provider, ElectionDataProvider, ElectionProvider, SortedListProvider, Supports, - VoteWeight, VoteWeightProvider, + VoteWeight, VoteWeightProvider, VoterOf, }; use frame_support::{ pallet_prelude::*, @@ -661,9 +661,7 @@ impl Pallet { /// /// All nominations that have been submitted before the last non-zero slash of the validator are /// auto-chilled, but still count towards the limit imposed by `maybe_max_len`. - pub fn get_npos_voters( - maybe_max_len: Option, - ) -> Vec<(T::AccountId, VoteWeight, Vec)> { + pub fn get_npos_voters(maybe_max_len: Option) -> Vec> { let max_allowed_len = { let nominator_count = Nominators::::count() as usize; let validator_count = Validators::::count() as usize; @@ -677,8 +675,13 @@ impl Pallet { let mut validators_taken = 0u32; for (validator, _) in >::iter().take(max_allowed_len) { // Append self vote. - let self_vote = - (validator.clone(), Self::weight_of(&validator), vec![validator.clone()]); + let self_vote = ( + validator.clone(), + Self::weight_of(&validator), + vec![validator.clone()] + .try_into() + .expect("`MaxVotesPerVoter` must be greater than or equal to 1"), + ); all_voters.push(self_vote); validators_taken.saturating_inc(); } @@ -724,7 +727,12 @@ impl Pallet { nominators_taken.saturating_inc(); } } else { - log!(error, "DEFENSIVE: invalid item in `SortedListProvider`: {:?}", nominator) + // this can only happen if: 1. there a pretty bad bug in the bags-list (or whatever + // is the sorted list) logic and the state of the two pallets is no longer + // compatible, or because the nominators is not decodable since they have more + // nomination than `T::MaxNominations`. This can rarely happen, and is not really an + // emergency or bug if it does. + log!(warn, "DEFENSIVE: invalid item in `SortedListProvider`: {:?}, this nominator probably has too many nominations now", nominator) } } @@ -772,7 +780,7 @@ impl Pallet { /// NOTE: you must ALWAYS use this function to add nominator or update their targets. Any access /// to `Nominators` or `VoterList` outside of this function is almost certainly /// wrong. - pub fn do_add_nominator(who: &T::AccountId, nominations: Nominations) { + pub fn do_add_nominator(who: &T::AccountId, nominations: Nominations) { if !Nominators::::contains_key(who) { // maybe update sorted list. Error checking is defensive-only - this should never fail. let _ = T::SortedListProvider::on_insert(who.clone(), Self::weight_of(who)) @@ -845,16 +853,14 @@ impl Pallet { impl ElectionDataProvider for Pallet { type AccountId = T::AccountId; type BlockNumber = BlockNumberFor; - const MAXIMUM_VOTES_PER_VOTER: u32 = T::MAX_NOMINATIONS; + type MaxVotesPerVoter = T::MaxNominations; fn desired_targets() -> data_provider::Result { Self::register_weight(T::DbWeight::get().reads(1)); Ok(Self::validator_count()) } - fn voters( - maybe_max_len: Option, - ) -> data_provider::Result)>> { + 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)); @@ -907,7 +913,11 @@ impl ElectionDataProvider for Pallet { } #[cfg(feature = "runtime-benchmarks")] - fn add_voter(voter: T::AccountId, weight: VoteWeight, targets: Vec) { + fn add_voter( + voter: T::AccountId, + weight: VoteWeight, + targets: BoundedVec, + ) { let stake = >::try_from(weight).unwrap_or_else(|_| { panic!("cannot convert a VoteWeight into BalanceOf, benchmark needs reconfiguring.") }); @@ -922,6 +932,7 @@ impl ElectionDataProvider for Pallet { claimed_rewards: vec![], }, ); + Self::do_add_nominator(&voter, Nominations { targets, submitted_in: 0, suppressed: false }); } @@ -957,7 +968,7 @@ impl ElectionDataProvider for Pallet { #[cfg(feature = "runtime-benchmarks")] fn put_snapshot( - voters: Vec<(T::AccountId, VoteWeight, Vec)>, + voters: Vec>, targets: Vec, target_stake: Option, ) { @@ -999,7 +1010,7 @@ impl ElectionDataProvider for Pallet { ); Self::do_add_nominator( &v, - Nominations { targets: t, submitted_in: 0, suppressed: false }, + Nominations { targets: t.try_into().unwrap(), submitted_in: 0, suppressed: false }, ); }); } diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 20864e4829e66..2a870fda063d3 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -32,7 +32,7 @@ use sp_runtime::{ DispatchError, Perbill, Percent, }; use sp_staking::{EraIndex, SessionIndex}; -use sp_std::{convert::From, prelude::*, result}; +use sp_std::{convert::From, prelude::*}; mod impls; @@ -50,6 +50,8 @@ const STAKING_ID: LockIdentifier = *b"staking "; #[frame_support::pallet] pub mod pallet { + use frame_election_provider_support::ElectionDataProvider; + use crate::BenchmarkingConfig; use super::*; @@ -94,7 +96,7 @@ pub mod pallet { >; /// Maximum number of nominations per nominator. - const MAX_NOMINATIONS: u32; + type MaxNominations: Get; /// Tokens have been minted and are unused for validator-reward. /// See [Era payout](./index.html#era-payout). @@ -161,15 +163,6 @@ pub mod pallet { type WeightInfo: WeightInfo; } - #[pallet::extra_constants] - impl Pallet { - // TODO: rename to snake case after https://github.com/paritytech/substrate/issues/8826 fixed. - #[allow(non_snake_case)] - fn MaxNominations() -> u32 { - T::MAX_NOMINATIONS - } - } - #[pallet::type_value] pub(crate) fn HistoryDepthOnEmpty() -> u32 { 84u32 @@ -246,11 +239,26 @@ pub mod pallet { #[pallet::storage] pub type MaxValidatorsCount = StorageValue<_, u32, OptionQuery>; - /// The map from nominator stash key to the set of stash keys of all validators to nominate. + /// The map from nominator stash key to their nomination preferences, namely the validators that + /// they wish to support. + /// + /// Note that the keys of this storage map might become non-decodable in case the + /// [`Config::MaxNominations`] configuration is decreased. In this rare case, these nominators + /// are still existent in storage, their key is correct and retrievable (i.e. `contains_key` + /// indicates that they exist), but their value cannot be decoded. Therefore, the non-decodable + /// nominators will effectively not-exist, until they re-submit their preferences such that it + /// is within the bounds of the newly set `Config::MaxNominations`. + /// + /// This implies that `::iter_keys().count()` and `::iter().count()` might return different + /// values for this map. Moreover, the main `::count()` is aligned with the former, namely the + /// number of keys that exist. + /// + /// Lastly, if any of the nominators become non-decodable, they can be chilled immediately via + /// [`Call::chill_other`] dispatchable by anyone. #[pallet::storage] #[pallet::getter(fn nominators)] pub type Nominators = - CountedStorageMap<_, Twox64Concat, T::AccountId, Nominations>; + CountedStorageMap<_, Twox64Concat, T::AccountId, Nominations>; /// The maximum nominator count before we stop allowing new validators to join. /// @@ -681,6 +689,14 @@ pub mod pallet { } fn integrity_test() { + // ensure that we funnel the correct value to the `DataProvider::MaxVotesPerVoter`; + assert_eq!( + T::MaxNominations::get(), + ::MaxVotesPerVoter::get() + ); + // and that MaxNominations is always greater than 1, since we count on this. + assert!(!T::MaxNominations::get().is_zero()); + sp_std::if_std! { sp_io::TestExternalities::new_empty().execute_with(|| assert!( @@ -978,7 +994,7 @@ pub mod pallet { /// /// # /// - The transaction's complexity is proportional to the size of `targets` (N) - /// which is capped at CompactAssignments::LIMIT (MAX_NOMINATIONS). + /// which is capped at CompactAssignments::LIMIT (T::MaxNominations). /// - Both the reads and writes follow a similar pattern. /// # #[pallet::weight(T::WeightInfo::nominate(targets.len() as u32))] @@ -1006,11 +1022,11 @@ pub mod pallet { } ensure!(!targets.is_empty(), Error::::EmptyTargets); - ensure!(targets.len() <= T::MAX_NOMINATIONS as usize, Error::::TooManyTargets); + ensure!(targets.len() <= T::MaxNominations::get() as usize, Error::::TooManyTargets); - let old = Nominators::::get(stash).map_or_else(Vec::new, |x| x.targets); + let old = Nominators::::get(stash).map_or_else(Vec::new, |x| x.targets.into_inner()); - let targets = targets + let targets: BoundedVec<_, _> = targets .into_iter() .map(|t| T::Lookup::lookup(t).map_err(DispatchError::from)) .map(|n| { @@ -1022,11 +1038,13 @@ pub mod pallet { } }) }) - .collect::, _>>()?; + .collect::, _>>()? + .try_into() + .map_err(|_| Error::::TooManyNominators)?; let nominations = Nominations { targets, - // Initial nominations are considered submitted at era 0. See `Nominations` doc + // Initial nominations are considered submitted at era 0. See `Nominations` doc. submitted_in: Self::current_era().unwrap_or(0), suppressed: false, }; @@ -1216,11 +1234,6 @@ pub mod pallet { /// Set the validators who cannot be slashed (if any). /// /// The dispatch origin must be Root. - /// - /// # - /// - O(V) - /// - Write: Invulnerables - /// # #[pallet::weight(T::WeightInfo::set_invulnerables(invulnerables.len() as u32))] pub fn set_invulnerables( origin: OriginFor, @@ -1234,13 +1247,6 @@ pub mod pallet { /// Force a current staker to become completely unstaked, immediately. /// /// The dispatch origin must be Root. - /// - /// # - /// O(S) where S is the number of slashing spans to be removed - /// Reads: Bonded, Slashing Spans, Account, Locks - /// Writes: Bonded, Slashing Spans (if S > 0), Ledger, Payee, Validators, Nominators, - /// Account, Locks Writes Each: SpanSlash * S - /// # #[pallet::weight(T::WeightInfo::force_unstake(*num_slashing_spans))] pub fn force_unstake( origin: OriginFor, @@ -1266,11 +1272,6 @@ pub mod pallet { /// The election process starts multiple blocks before the end of the era. /// If this is called just before a new era is triggered, the election process may not /// have enough blocks to get a result. - /// - /// # - /// - Weight: O(1) - /// - Write: ForceEra - /// # #[pallet::weight(T::WeightInfo::force_new_era_always())] pub fn force_new_era_always(origin: OriginFor) -> DispatchResult { ensure_root(origin)?; @@ -1283,14 +1284,6 @@ pub mod pallet { /// Can be called by the `T::SlashCancelOrigin`. /// /// Parameters: era and indices of the slashes for that era to kill. - /// - /// # - /// Complexity: O(U + S) - /// with U unapplied slashes weighted with U=1000 - /// and S is the number of slash indices to be canceled. - /// - Read: Unapplied Slashes - /// - Write: Unapplied Slashes - /// # #[pallet::weight(T::WeightInfo::cancel_deferred_slash(slash_indices.len() as u32))] pub fn cancel_deferred_slash( origin: OriginFor, @@ -1550,6 +1543,11 @@ pub mod pallet { /// /// If the caller is different than the controller being targeted, the following conditions /// must be met: + /// + /// * `controller` must belong to a nominator who has become non-decodable, + /// + /// Or: + /// /// * A `ChillThreshold` must be set and checked which defines how close to the max /// nominators or validators we must reach before users can start chilling one-another. /// * A `MaxNominatorCount` and `MaxValidatorCount` must be set which is used to determine @@ -1568,6 +1566,11 @@ pub mod pallet { let stash = ledger.stash; // In order for one user to chill another user, the following conditions must be met: + // + // * `controller` belongs to a nominator who has become non-decodable, + // + // Or + // // * A `ChillThreshold` is set which defines how close to the max nominators or // validators we must reach before users can start chilling one-another. // * A `MaxNominatorCount` and `MaxValidatorCount` which is used to determine how close @@ -1577,6 +1580,12 @@ pub mod pallet { // threshold bond required. // // Otherwise, if caller is the same as the controller, this is just like `chill`. + + if Nominators::::contains_key(&stash) && Nominators::::get(&stash).is_none() { + Self::chill_stash(&stash); + return Ok(()) + } + if caller != controller { let threshold = ChillThreshold::::get().ok_or(Error::::CannotChillOther)?; let min_active_bond = if Nominators::::contains_key(&stash) { diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 8d465c8c93dc4..538b75ead340b 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4248,7 +4248,11 @@ fn count_check_works() { Validators::::insert(987654321, ValidatorPrefs::default()); Nominators::::insert( 987654321, - Nominations { targets: vec![], submitted_in: Default::default(), suppressed: false }, + Nominations { + targets: Default::default(), + submitted_in: Default::default(), + suppressed: false, + }, ); }) } @@ -4589,6 +4593,100 @@ fn min_commission_works() { }) } +#[test] +fn change_of_max_nominations() { + use frame_election_provider_support::ElectionDataProvider; + ExtBuilder::default() + .add_staker(60, 61, 10, StakerStatus::Nominator(vec![1])) + .add_staker(70, 71, 10, StakerStatus::Nominator(vec![1, 2, 3])) + .balance_factor(10) + .build_and_execute(|| { + // pre-condition + assert_eq!(MaxNominations::get(), 16); + + assert_eq!( + Nominators::::iter() + .map(|(k, n)| (k, n.targets.len())) + .collect::>(), + vec![(70, 3), (101, 2), (60, 1)] + ); + // 3 validators and 3 nominators + assert_eq!(Staking::voters(None).unwrap().len(), 3 + 3); + + // abrupt change from 16 to 4, everyone should be fine. + MaxNominations::set(4); + + assert_eq!( + Nominators::::iter() + .map(|(k, n)| (k, n.targets.len())) + .collect::>(), + vec![(70, 3), (101, 2), (60, 1)] + ); + assert_eq!(Staking::voters(None).unwrap().len(), 3 + 3); + + // abrupt change from 4 to 3, everyone should be fine. + MaxNominations::set(3); + + assert_eq!( + Nominators::::iter() + .map(|(k, n)| (k, n.targets.len())) + .collect::>(), + vec![(70, 3), (101, 2), (60, 1)] + ); + assert_eq!(Staking::voters(None).unwrap().len(), 3 + 3); + + // abrupt change from 3 to 2, this should cause some nominators to be non-decodable, and + // thus non-existent unless if they update. + MaxNominations::set(2); + + assert_eq!( + Nominators::::iter() + .map(|(k, n)| (k, n.targets.len())) + .collect::>(), + vec![(101, 2), (60, 1)] + ); + // 70 is still in storage.. + assert!(Nominators::::contains_key(70)); + // but its value cannot be decoded and default is returned. + assert!(Nominators::::get(70).is_none()); + + assert_eq!(Staking::voters(None).unwrap().len(), 3 + 2); + assert!(Nominators::::contains_key(101)); + + // abrupt change from 2 to 1, this should cause some nominators to be non-decodable, and + // thus non-existent unless if they update. + MaxNominations::set(1); + + assert_eq!( + Nominators::::iter() + .map(|(k, n)| (k, n.targets.len())) + .collect::>(), + vec![(60, 1)] + ); + assert!(Nominators::::contains_key(70)); + assert!(Nominators::::contains_key(60)); + assert!(Nominators::::get(70).is_none()); + assert!(Nominators::::get(60).is_some()); + assert_eq!(Staking::voters(None).unwrap().len(), 3 + 1); + + // now one of them can revive themselves by re-nominating to a proper value. + assert_ok!(Staking::nominate(Origin::signed(71), vec![1])); + assert_eq!( + Nominators::::iter() + .map(|(k, n)| (k, n.targets.len())) + .collect::>(), + vec![(70, 1), (60, 1)] + ); + + // or they can be chilled by any account. + assert!(Nominators::::contains_key(101)); + assert!(Nominators::::get(101).is_none()); + assert_ok!(Staking::chill_other(Origin::signed(70), 100)); + assert!(!Nominators::::contains_key(101)); + assert!(Nominators::::get(101).is_none()); + }) +} + mod sorted_list_provider { use super::*; use frame_election_provider_support::SortedListProvider; diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index fc2754c5c555f..ef60729f6d861 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -114,6 +114,29 @@ impl TypeId for PalletId { const TYPE_ID: [u8; 4] = *b"modl"; } +/// Build a bounded vec from the given literals. +/// +/// The type of the outcome must be known. +/// +/// Will not handle any errors and just panic if the given literals cannot fit in the corresponding +/// bounded vec type. Thus, this is only suitable for testing and non-consensus code. +#[macro_export] +#[cfg(feature = "std")] +macro_rules! bounded_vec { + ($ ($values:expr),* ) => { + { + use $crate::sp_std::convert::TryInto as _; + $crate::sp_std::vec![$($values),*].try_into().unwrap() + } + }; + ( $value:expr ; $repetition:expr ) => { + { + use $crate::sp_std::convert::TryInto as _; + $crate::sp_std::vec![$value ; $repetition].try_into().unwrap() + } + } +} + /// Generate a new type alias for [`storage::types::StorageValue`], /// [`storage::types::StorageMap`], [`storage::types::StorageDoubleMap`] /// and [`storage::types::StorageNMap`]. diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index 6b20ed2bba90f..9298a5d98b003 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -664,6 +664,19 @@ pub mod test { assert_eq!(*bounded, vec![1, 0, 2, 3]); } + #[test] + fn constructor_macro_works() { + use frame_support::bounded_vec; + + // With values. Use some brackets to make sure the macro doesn't expand. + let bv: BoundedVec<(u32, u32), ConstU32<3>> = bounded_vec![(1, 2), (1, 2), (1, 2)]; + assert_eq!(bv, vec![(1, 2), (1, 2), (1, 2)]); + + // With repetition. + let bv: BoundedVec<(u32, u32), ConstU32<3>> = bounded_vec![(1, 2); 3]; + assert_eq!(bv, vec![(1, 2), (1, 2), (1, 2)]); + } + #[test] #[should_panic(expected = "insertion index (is 9) should be <= len (is 3)")] fn try_inert_panics_if_oob() { diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs index 0f98b13282a74..7f53b196c8151 100644 --- a/frame/support/src/storage/types/counted_map.rs +++ b/frame/support/src/storage/types/counted_map.rs @@ -401,6 +401,13 @@ where ) -> crate::storage::PrefixIterator<(Key, Value), OnRemovalCounterUpdate> { ::Map::iter_from(starting_raw_key).convert_on_removal() } + + /// Enumerate all keys in the counted map. + /// + /// If you alter the map while doing this, you'll get undefined results. + pub fn iter_keys() -> crate::storage::KeyPrefixIterator { + ::Map::iter_keys() + } } impl StorageEntryMetadataBuilder diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 800ffbfd20dc2..878edd7840e74 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -479,11 +479,6 @@ pub mod pallet { } /// Make some on-chain remark and emit event. - /// - /// # - /// - `O(b)` where b is the length of the remark. - /// - 1 event. - /// # #[pallet::weight(T::SystemWeightInfo::remark_with_event(remark.len() as u32))] pub fn remark_with_event( origin: OriginFor, diff --git a/primitives/npos-elections/fuzzer/src/common.rs b/primitives/npos-elections/fuzzer/src/common.rs index 7ca66f72dd92b..89c635c4b4f68 100644 --- a/primitives/npos-elections/fuzzer/src/common.rs +++ b/primitives/npos-elections/fuzzer/src/common.rs @@ -158,20 +158,10 @@ pub fn generate_random_npos_result( ( match election_type { - ElectionType::Phragmen(conf) => seq_phragmen::( - to_elect, - candidates.clone(), - voters.clone(), - conf, - ) - .unwrap(), - ElectionType::Phragmms(conf) => phragmms::( - to_elect, - candidates.clone(), - voters.clone(), - conf, - ) - .unwrap(), + ElectionType::Phragmen(conf) => + seq_phragmen(to_elect, candidates.clone(), voters.clone(), conf).unwrap(), + ElectionType::Phragmms(conf) => + phragmms(to_elect, candidates.clone(), voters.clone(), conf).unwrap(), }, candidates, voters, diff --git a/primitives/npos-elections/fuzzer/src/phragmen_balancing.rs b/primitives/npos-elections/fuzzer/src/phragmen_balancing.rs index 2af6a4c0f8151..8f782405df527 100644 --- a/primitives/npos-elections/fuzzer/src/phragmen_balancing.rs +++ b/primitives/npos-elections/fuzzer/src/phragmen_balancing.rs @@ -24,7 +24,7 @@ use honggfuzz::fuzz; use rand::{self, SeedableRng}; use sp_npos_elections::{ assignment_ratio_to_staked_normalized, is_score_better, seq_phragmen, to_supports, - EvaluateSupport, VoteWeight, + ElectionResult, EvaluateSupport, VoteWeight, }; use sp_runtime::Perbill; @@ -68,13 +68,8 @@ fn main() { }; if iterations > 0 { - let balanced = seq_phragmen::( - to_elect, - candidates, - voters, - Some((iterations, 0)), - ) - .unwrap(); + let balanced: ElectionResult = + seq_phragmen(to_elect, candidates, voters, Some((iterations, 0))).unwrap(); let balanced_score = { let staked = assignment_ratio_to_staked_normalized( diff --git a/primitives/npos-elections/fuzzer/src/phragmms_balancing.rs b/primitives/npos-elections/fuzzer/src/phragmms_balancing.rs index 0cd49c3f80442..f2b12b137883c 100644 --- a/primitives/npos-elections/fuzzer/src/phragmms_balancing.rs +++ b/primitives/npos-elections/fuzzer/src/phragmms_balancing.rs @@ -23,8 +23,8 @@ use common::*; use honggfuzz::fuzz; use rand::{self, SeedableRng}; use sp_npos_elections::{ - assignment_ratio_to_staked_normalized, is_score_better, phragmms, to_supports, EvaluateSupport, - VoteWeight, + assignment_ratio_to_staked_normalized, is_score_better, phragmms, to_supports, ElectionResult, + EvaluateSupport, VoteWeight, }; use sp_runtime::Perbill; @@ -67,13 +67,8 @@ fn main() { score }; - let balanced = phragmms::( - to_elect, - candidates, - voters, - Some((iterations, 0)), - ) - .unwrap(); + let balanced: ElectionResult = + phragmms(to_elect, candidates, voters, Some((iterations, 0))).unwrap(); let balanced_score = { let staked = diff --git a/primitives/npos-elections/src/lib.rs b/primitives/npos-elections/src/lib.rs index bb1c38d3077c4..7b3b09a4c7346 100644 --- a/primitives/npos-elections/src/lib.rs +++ b/primitives/npos-elections/src/lib.rs @@ -471,7 +471,7 @@ pub fn is_score_better(this: ElectionScore, that: ElectionScore, ep /// - It drops duplicate targets within a voter. pub fn setup_inputs( initial_candidates: Vec, - initial_voters: Vec<(AccountId, VoteWeight, Vec)>, + initial_voters: Vec<(AccountId, VoteWeight, impl IntoIterator)>, ) -> (Vec>, Vec>) { // used to cache and access candidates index. let mut c_idx_cache = BTreeMap::::new(); @@ -496,7 +496,7 @@ pub fn setup_inputs( let voters = initial_voters .into_iter() .filter_map(|(who, voter_stake, votes)| { - let mut edges: Vec> = Vec::with_capacity(votes.len()); + let mut edges: Vec> = Vec::new(); for v in votes { if edges.iter().any(|e| e.who == v) { // duplicate edge. diff --git a/primitives/npos-elections/src/mock.rs b/primitives/npos-elections/src/mock.rs index 8e8e7ebc1c0c6..85c970d7b418f 100644 --- a/primitives/npos-elections/src/mock.rs +++ b/primitives/npos-elections/src/mock.rs @@ -344,7 +344,7 @@ pub(crate) fn run_and_compare( FS: Fn(&AccountId) -> VoteWeight, { // run fixed point code. - let ElectionResult { winners, assignments } = seq_phragmen::<_, Output>( + let ElectionResult::<_, Output> { winners, assignments } = seq_phragmen( to_elect, candidates.clone(), voters diff --git a/primitives/npos-elections/src/phragmen.rs b/primitives/npos-elections/src/phragmen.rs index c582c5910d69a..e8e925935f774 100644 --- a/primitives/npos-elections/src/phragmen.rs +++ b/primitives/npos-elections/src/phragmen.rs @@ -70,7 +70,7 @@ const DEN: ExtendedBalance = ExtendedBalance::max_value(); pub fn seq_phragmen( to_elect: usize, candidates: Vec, - voters: Vec<(AccountId, VoteWeight, Vec)>, + voters: Vec<(AccountId, VoteWeight, impl IntoIterator)>, balancing: Option<(usize, ExtendedBalance)>, ) -> Result, crate::Error> { let (candidates, voters) = setup_inputs(candidates, voters); diff --git a/primitives/npos-elections/src/phragmms.rs b/primitives/npos-elections/src/phragmms.rs index 7c51da9ee92e0..6220cacd157b2 100644 --- a/primitives/npos-elections/src/phragmms.rs +++ b/primitives/npos-elections/src/phragmms.rs @@ -44,7 +44,7 @@ use sp_std::{prelude::*, rc::Rc}; pub fn phragmms( to_elect: usize, candidates: Vec, - voters: Vec<(AccountId, VoteWeight, Vec)>, + voters: Vec<(AccountId, VoteWeight, impl IntoIterator)>, balancing: Option<(usize, ExtendedBalance)>, ) -> Result, crate::Error> { let (candidates, mut voters) = setup_inputs(candidates, voters); @@ -351,8 +351,8 @@ mod tests { let candidates = vec![1, 2, 3]; let voters = vec![(10, 10, vec![1, 2]), (20, 20, vec![1, 3]), (30, 30, vec![2, 3])]; - let ElectionResult { winners, assignments } = - phragmms::<_, Perbill>(2, candidates, voters, Some((2, 0))).unwrap(); + let ElectionResult::<_, Perbill> { winners, assignments } = + phragmms(2, candidates, voters, Some((2, 0))).unwrap(); assert_eq!(winners, vec![(3, 30), (2, 30)]); assert_eq!( assignments, @@ -383,8 +383,8 @@ mod tests { (130, 1000, vec![61, 71]), ]; - let ElectionResult { winners, assignments: _ } = - phragmms::<_, Perbill>(4, candidates, voters, Some((2, 0))).unwrap(); + let ElectionResult::<_, Perbill> { winners, assignments: _ } = + phragmms(4, candidates, voters, Some((2, 0))).unwrap(); assert_eq!(winners, vec![(11, 3000), (31, 2000), (51, 1500), (61, 1500),]); } @@ -396,8 +396,8 @@ mod tests { // give a bit more to 1 and 3. voters.push((2, u64::MAX, vec![1, 3])); - let ElectionResult { winners, assignments: _ } = - phragmms::<_, Perbill>(2, candidates, voters, Some((2, 0))).unwrap(); + let ElectionResult::<_, Perbill> { winners, assignments: _ } = + phragmms(2, candidates, voters, Some((2, 0))).unwrap(); assert_eq!(winners.into_iter().map(|(w, _)| w).collect::>(), vec![1u32, 3]); } } diff --git a/primitives/npos-elections/src/tests.rs b/primitives/npos-elections/src/tests.rs index e7d0078b1fbe0..c6748b29e9851 100644 --- a/primitives/npos-elections/src/tests.rs +++ b/primitives/npos-elections/src/tests.rs @@ -230,7 +230,7 @@ fn phragmen_poc_works() { let voters = vec![(10, vec![1, 2]), (20, vec![1, 3]), (30, vec![2, 3])]; let stake_of = create_stake_of(&[(10, 10), (20, 20), (30, 30)]); - let ElectionResult { winners, assignments } = seq_phragmen::<_, Perbill>( + let ElectionResult::<_, Perbill> { winners, assignments } = seq_phragmen( 2, candidates, voters @@ -285,7 +285,7 @@ fn phragmen_poc_works_with_balancing() { let voters = vec![(10, vec![1, 2]), (20, vec![1, 3]), (30, vec![2, 3])]; let stake_of = create_stake_of(&[(10, 10), (20, 20), (30, 30)]); - let ElectionResult { winners, assignments } = seq_phragmen::<_, Perbill>( + let ElectionResult::<_, Perbill> { winners, assignments } = seq_phragmen( 2, candidates, voters @@ -372,7 +372,7 @@ fn phragmen_accuracy_on_large_scale_only_candidates() { (5, (u64::MAX - 2).into()), ]); - let ElectionResult { winners, assignments } = seq_phragmen::<_, Perbill>( + let ElectionResult::<_, Perbill> { winners, assignments } = seq_phragmen( 2, candidates.clone(), auto_generate_self_voters(&candidates) @@ -403,7 +403,7 @@ fn phragmen_accuracy_on_large_scale_voters_and_candidates() { (14, u64::MAX.into()), ]); - let ElectionResult { winners, assignments } = seq_phragmen::<_, Perbill>( + let ElectionResult::<_, Perbill> { winners, assignments } = seq_phragmen( 2, candidates, voters @@ -435,7 +435,7 @@ fn phragmen_accuracy_on_small_scale_self_vote() { let voters = auto_generate_self_voters(&candidates); let stake_of = create_stake_of(&[(40, 0), (10, 1), (20, 2), (30, 1)]); - let ElectionResult { winners, assignments } = seq_phragmen::<_, Perbill>( + let ElectionResult::<_, Perbill> { winners, assignments } = seq_phragmen( 3, candidates, voters @@ -465,7 +465,7 @@ fn phragmen_accuracy_on_small_scale_no_self_vote() { (3, 1), ]); - let ElectionResult { winners, assignments } = seq_phragmen::<_, Perbill>( + let ElectionResult::<_, Perbill> { winners, assignments } = seq_phragmen( 3, candidates, voters @@ -501,7 +501,7 @@ fn phragmen_large_scale_test() { (50, 990000000000000000), ]); - let ElectionResult { winners, assignments } = seq_phragmen::<_, Perbill>( + let ElectionResult::<_, Perbill> { winners, assignments } = seq_phragmen( 2, candidates, voters @@ -528,7 +528,7 @@ fn phragmen_large_scale_test_2() { let stake_of = create_stake_of(&[(2, c_budget.into()), (4, c_budget.into()), (50, nom_budget.into())]); - let ElectionResult { winners, assignments } = seq_phragmen::<_, Perbill>( + let ElectionResult::<_, Perbill> { winners, assignments } = seq_phragmen( 2, candidates, voters @@ -597,7 +597,7 @@ fn elect_has_no_entry_barrier() { let voters = vec![(1, vec![10]), (2, vec![20])]; let stake_of = create_stake_of(&[(1, 10), (2, 10)]); - let ElectionResult { winners, assignments: _ } = seq_phragmen::<_, Perbill>( + let ElectionResult::<_, Perbill> { winners, assignments: _ } = seq_phragmen( 3, candidates, voters @@ -618,7 +618,7 @@ fn phragmen_self_votes_should_be_kept() { let voters = vec![(5, vec![5]), (10, vec![10]), (20, vec![20]), (1, vec![10, 20])]; let stake_of = create_stake_of(&[(5, 5), (10, 10), (20, 20), (1, 8)]); - let result = seq_phragmen::<_, Perbill>( + let result: ElectionResult<_, Perbill> = seq_phragmen( 2, candidates, voters @@ -664,8 +664,8 @@ fn duplicate_target_is_ignored() { let candidates = vec![1, 2, 3]; let voters = vec![(10, 100, vec![1, 1, 2, 3]), (20, 100, vec![2, 3]), (30, 50, vec![1, 1, 2])]; - let ElectionResult { winners, assignments } = - seq_phragmen::<_, Perbill>(2, candidates, voters, None).unwrap(); + let ElectionResult::<_, Perbill> { winners, assignments } = + seq_phragmen(2, candidates, voters, None).unwrap(); assert_eq!(winners, vec![(2, 140), (3, 110)]); assert_eq!( @@ -682,8 +682,8 @@ fn duplicate_target_is_ignored_when_winner() { let candidates = vec![1, 2, 3]; let voters = vec![(10, 100, vec![1, 1, 2, 3]), (20, 100, vec![1, 2])]; - let ElectionResult { winners, assignments } = - seq_phragmen::<_, Perbill>(2, candidates, voters, None).unwrap(); + let ElectionResult::<_, Perbill> { winners, assignments } = + seq_phragmen(2, candidates, voters, None).unwrap(); assert_eq!(winners, vec![(1, 100), (2, 100)]); assert_eq!(