diff --git a/substrate/core/phragmen/src/lib.rs b/substrate/core/phragmen/src/lib.rs index 7377bac2f8..8c8401bed6 100644 --- a/substrate/core/phragmen/src/lib.rs +++ b/substrate/core/phragmen/src/lib.rs @@ -290,7 +290,9 @@ pub fn elect( let mut assignment = (n.who.clone(), vec![]); for e in &mut n.edges { if let Some(c) = elected_candidates.iter().cloned().find(|(c, _)| *c == e.who) { - if c.0 != n.who { + // if self_vote == false, this branch should always be executed as we want to + // collect all nominations + if c.0 != n.who || !self_vote { let per_bill_parts = { if n.load == e.load { @@ -360,6 +362,47 @@ pub fn elect( }) } +/// Build the support map from the given phragmen result. +pub fn build_support_map( + elected_stashes: &Vec, + assignments: &Vec<(AccountId, Vec>)>, + stake_of: FS, + assume_self_vote: bool, +) -> SupportMap where + AccountId: Default + Ord + Member, + Balance: Default + Copy + SimpleArithmetic, + C: Convert + Convert, + for<'r> FS: Fn(&'r AccountId) -> Balance, +{ + let to_votes = |b: Balance| >::convert(b) as ExtendedBalance; + // Initialize the support of each candidate. + let mut supports = >::new(); + elected_stashes + .iter() + .map(|e| (e, if assume_self_vote { to_votes(stake_of(e)) } else { Zero::zero() } )) + .for_each(|(e, s)| { + let item = Support { own: s, total: s, ..Default::default() }; + supports.insert(e.clone(), item); + }); + + // build support struct. + for (n, assignment) in assignments.iter() { + for (c, per_thing) in assignment.iter() { + let nominator_stake = to_votes(stake_of(n)); + // AUDIT: it is crucially important for the `Mul` implementation of all + // per-things to be sound. + let other_stake = *per_thing * nominator_stake; + if let Some(support) = supports.get_mut(c) { + // For an astronomically rich validator with more astronomically rich + // set of nominators, this might saturate. + support.total = support.total.saturating_add(other_stake); + support.others.push((n.clone(), other_stake)); + } + } + } + supports +} + /// Performs equalize post-processing to the output of the election algorithm. This happens in /// rounds. The number of rounds and the maximum diff-per-round tolerance can be tuned through input /// parameters. diff --git a/substrate/core/sr-primitives/src/weights.rs b/substrate/core/sr-primitives/src/weights.rs index b245f90247..89852eb595 100644 --- a/substrate/core/sr-primitives/src/weights.rs +++ b/substrate/core/sr-primitives/src/weights.rs @@ -14,11 +14,23 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! Primitives for transaction weighting. +//! # Primitives for transaction weighting. //! -//! Each dispatch function within `decl_module!` can have an optional `#[weight = $x]` attribute. -//! `$x` can be any type that implements the `ClassifyDispatch` and `WeighData` traits. By -//! default, All transactions are annotated with `#[weight = SimpleDispatchInfo::default()]`. +//! All dispatchable functions defined in `decl_module!` must provide two trait implementations: +//! - [`WeightData`]: To determine the weight of the dispatch. +//! - [`ClassifyDispatch`]: To determine the class of the dispatch. See the enum definition for +//! more information on dispatch classes. +//! +//! Every dispatchable function is responsible for providing this data via an optional `#[weight = +//! $x]` attribute. In this snipped, `$x` can be any user provided struct that implements the +//! two aforementioned traits. +//! +//! Substrate then bundles then output information of the two traits into [`DispatchInfo`] struct +//! and provides it by implementing the [`GetDispatchInfo`] for all `Call` variants, and opaque +//! extrinsic types. +//! +//! If no `#[weight]` is defined, the macro automatically injects the `Default` implementation of +//! the [`SimpleDispatchInfo`]. //! //! Note that the decl_module macro _cannot_ enforce this and will simply fail if an invalid struct //! (something that does not implement `Weighable`) is passed in. @@ -31,8 +43,24 @@ pub use crate::transaction_validity::TransactionPriority; /// Numeric range of a transaction weight. pub type Weight = u32; -/// A generalized group of dispatch types. This is only distinguishing normal, user-triggered transactions -/// (`Normal`) and anything beyond which serves a higher purpose to the system (`Operational`). +/// Means of weighing some particular kind of data (`T`). +pub trait WeighData { + /// Weigh the data `T` given by `target`. When implementing this for a dispatchable, `T` will be + /// a tuple of all arguments given to the function (except origin). + fn weigh_data(&self, target: T) -> Weight; +} + +/// Means of classifying a dispatchable function. +pub trait ClassifyDispatch { + /// Classify the dispatch function based on input data `target` of type `T`. When implementing + /// this for a dispatchable, `T` will be a tuple of all arguments given to the function (except + /// origin). + fn classify_dispatch(&self, target: T) -> DispatchClass; +} + +/// A generalized group of dispatch types. This is only distinguishing normal, user-triggered +/// transactions (`Normal`) and anything beyond which serves a higher purpose to the system +/// (`Operational`). #[derive(PartialEq, Eq, Clone, Copy, RuntimeDebug)] pub enum DispatchClass { /// A normal dispatch. @@ -82,8 +110,8 @@ impl DispatchInfo { } } -/// A `Dispatchable` function (aka transaction) that can carry some static information along with it, using the -/// `#[weight]` attribute. +/// A `Dispatchable` function (aka transaction) that can carry some static information along with +/// it, using the `#[weight]` attribute. pub trait GetDispatchInfo { /// Return a `DispatchInfo`, containing relevant information of this dispatch. /// @@ -91,18 +119,6 @@ pub trait GetDispatchInfo { fn get_dispatch_info(&self) -> DispatchInfo; } -/// Means of weighing some particular kind of data (`T`). -pub trait WeighData { - /// Weigh the data `T` given by `target`. - fn weigh_data(&self, target: T) -> Weight; -} - -/// Means of classifying a dispatchable function. -pub trait ClassifyDispatch { - /// Classify the dispatch function based on input data `target` of type `T`. - fn classify_dispatch(&self, target: T) -> DispatchClass; -} - /// Default type used with the `#[weight = x]` attribute in a substrate chain. /// /// A user may pass in any other type that implements the correct traits. If not, the `Default` @@ -114,13 +130,9 @@ pub trait ClassifyDispatch { /// - A `Free` variant is equal to `::Fixed(0)`. Note that this does not guarantee inclusion. /// - A `Max` variant is equal to `::Fixed(Weight::max_value())`. /// -/// Based on the final weight value, based on the above variants: -/// - A _weight-fee_ is deducted. -/// - The block weight is consumed proportionally. -/// /// As for the generalized groups themselves: /// - `Normal` variants will be assigned a priority proportional to their weight. They can only -/// consume a portion (1/4) of the maximum block resource limits. +/// consume a portion (defined in the system module) of the maximum block resource limits. /// - `Operational` variants will be assigned the maximum priority. They can potentially consume /// the entire block resource limit. #[derive(Clone, Copy)] diff --git a/substrate/node/cli/src/chain_spec.rs b/substrate/node/cli/src/chain_spec.rs index 80c74455e3..04fb41c211 100644 --- a/substrate/node/cli/src/chain_spec.rs +++ b/substrate/node/cli/src/chain_spec.rs @@ -20,12 +20,12 @@ use chain_spec::ChainSpecExtension; use primitives::{Pair, Public, crypto::UncheckedInto, sr25519}; use serde::{Serialize, Deserialize}; use node_runtime::{ - BabeConfig, BalancesConfig, ContractsConfig, CouncilConfig, DemocracyConfig, ElectionsConfig, GrandpaConfig, - ImOnlineConfig, IndicesConfig, SessionConfig, SessionKeys, StakerStatus, StakingConfig, SudoConfig, SystemConfig, - TechnicalCommitteeConfig, WASM_BINARY, + BabeConfig, BalancesConfig, ContractsConfig, CouncilConfig, DemocracyConfig, GrandpaConfig, + ImOnlineConfig, IndicesConfig, SessionConfig, SessionKeys, StakerStatus, StakingConfig, + SudoConfig, SystemConfig, TechnicalCommitteeConfig, WASM_BINARY, }; use node_runtime::Block; -use node_runtime::constants::{time::*, currency::*}; +use node_runtime::constants::currency::*; use substrate_service; use hex_literal::hex; use substrate_telemetry::TelemetryEndpoints; @@ -243,12 +243,6 @@ pub fn testnet_genesis( members: vec![], phantom: Default::default(), }), - elections_phragmen: Some(ElectionsConfig { - members: endowed_accounts.iter().take(2).cloned().collect(), - term_duration: 28 * DAYS, - desired_members: 4, - desired_runners_up: 1, - }), contracts: Some(ContractsConfig { current_schedule: contracts::Schedule { enable_println, // this should only be enabled on development chains diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs index b56e015d77..a6bd25a73e 100644 --- a/substrate/node/runtime/src/lib.rs +++ b/substrate/node/runtime/src/lib.rs @@ -82,7 +82,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. spec_version: 189, - impl_version: 189, + impl_version: 190, apis: RUNTIME_API_VERSIONS, }; @@ -328,6 +328,9 @@ impl collective::Trait for Runtime { parameter_types! { pub const CandidacyBond: Balance = 10 * DOLLARS; pub const VotingBond: Balance = 1 * DOLLARS; + pub const TermDuration: BlockNumber = 7 * DAYS; + pub const DesiredMembers: u32 = 13; + pub const DesiredRunnersUp: u32 = 7; } impl elections_phragmen::Trait for Runtime { @@ -336,6 +339,9 @@ impl elections_phragmen::Trait for Runtime { type CurrencyToVote = CurrencyToVoteHandler; type CandidacyBond = CandidacyBond; type VotingBond = VotingBond; + type TermDuration = TermDuration; + type DesiredMembers = DesiredMembers; + type DesiredRunnersUp = DesiredRunnersUp; type LoserCandidate = (); type BadReport = (); type KickedMember = (); @@ -520,7 +526,7 @@ construct_runtime!( Democracy: democracy::{Module, Call, Storage, Config, Event}, Council: collective::::{Module, Call, Storage, Origin, Event, Config}, TechnicalCommittee: collective::::{Module, Call, Storage, Origin, Event, Config}, - Elections: elections_phragmen::{Module, Call, Storage, Event, Config}, + Elections: elections_phragmen::{Module, Call, Storage, Event}, TechnicalMembership: membership::::{Module, Call, Storage, Event, Config}, FinalityTracker: finality_tracker::{Module, Call, Inherent}, Grandpa: grandpa::{Module, Call, Storage, Config, Event}, diff --git a/substrate/node/testing/src/genesis.rs b/substrate/node/testing/src/genesis.rs index b6ee28cf0f..0b99052f25 100644 --- a/substrate/node/testing/src/genesis.rs +++ b/substrate/node/testing/src/genesis.rs @@ -93,7 +93,6 @@ pub fn config(support_changes_trie: bool, code: Option<&[u8]>) -> GenesisConfig collective_Instance1: Some(Default::default()), collective_Instance2: Some(Default::default()), membership_Instance1: Some(Default::default()), - elections_phragmen: Some(Default::default()), sudo: Some(Default::default()), treasury: Some(Default::default()), } diff --git a/substrate/srml/elections-phragmen/src/lib.rs b/substrate/srml/elections-phragmen/src/lib.rs index e74ecd8521..f22c05ca45 100644 --- a/substrate/srml/elections-phragmen/src/lib.rs +++ b/substrate/srml/elections-phragmen/src/lib.rs @@ -86,6 +86,7 @@ use srml_support::{ ChangeMembers, OnUnbalanced, WithdrawReason } }; +use phragmen::ExtendedBalance; use system::{self, ensure_signed, ensure_root}; const MODULE_ID: LockIdentifier = *b"phrelect"; @@ -127,25 +128,26 @@ pub trait Trait: system::Trait { /// Handler for the unbalanced reduction when a member has been kicked. type KickedMember: OnUnbalanced>; + + /// Number of members to elect. + type DesiredMembers: Get; + + /// Number of runners_up to keep. + type DesiredRunnersUp: Get; + + /// How long each seat is kept. This defines the next block number at which an election + /// round will happen. If set to zero, no elections are ever triggered and the module will + /// be in passive mode. + type TermDuration: Get; } decl_storage! { trait Store for Module as PhragmenElection { - // ---- parameters - /// Number of members to elect. - pub DesiredMembers get(fn desired_members) config(): u32; - /// Number of runners_up to keep. - pub DesiredRunnersUp get(fn desired_runners_up) config(): u32; - /// How long each seat is kept. This defines the next block number at which an election - /// round will happen. If set to zero, no elections are ever triggered and the module will - /// be in passive mode. In that case only a member set defined in at genesis can exist. - pub TermDuration get(fn term_duration) config(): T::BlockNumber; - // ---- State /// The current elected membership. Sorted based on account id. - pub Members get(fn members) config(): Vec; + pub Members get(fn members): Vec<(T::AccountId, BalanceOf)>; /// The current runners_up. Sorted based on low to high merit (worse to best runner). - pub RunnersUp get(fn runners_up): Vec; + pub RunnersUp get(fn runners_up): Vec<(T::AccountId, BalanceOf)>; /// The total number of vote rounds that have happened, excluding the upcoming one. pub ElectionRounds get(fn election_rounds): u32 = Zero::zero(); @@ -166,6 +168,9 @@ decl_module! { const CandidacyBond: BalanceOf = T::CandidacyBond::get(); const VotingBond: BalanceOf = T::VotingBond::get(); + const DesiredMembers: u32 = T::DesiredMembers::get(); + const DesiredRunnersUp: u32 = T::DesiredRunnersUp::get(); + const TermDuration: T::BlockNumber = T::TermDuration::get(); /// Vote for a set of candidates for the upcoming round of election. /// @@ -194,8 +199,8 @@ decl_module! { ensure!(!allowed_votes.is_zero(), "cannot vote when no candidates or members exist"); ensure!(votes.len() <= allowed_votes, "cannot vote more than candidates"); ensure!(votes.len() <= MAXIMUM_VOTE, "cannot vote more than maximum allowed"); - ensure!(!votes.is_empty(), "must vote for at least one candidate."); + ensure!( value > T::Currency::minimum_balance(), "cannot vote with stake less than minimum balance" @@ -310,19 +315,6 @@ decl_module! { >::mutate(|c| c.insert(index, who)); } - /// Set the desired member count. Changes will be effective at the beginning of next round. - /// - /// # - /// #### State - /// Reads: O(1) - /// Writes: O(1) - /// # - #[weight = SimpleDispatchInfo::FixedOperational(10_000)] - fn set_desired_member_count(origin, #[compact] count: u32) { - ensure_root(origin)?; - DesiredMembers::put(count); - } - /// Remove a particular member from the set. This is effective immediately. /// /// If a runner-up is available, then the best runner-up will be removed and replaces the @@ -340,49 +332,45 @@ decl_module! { ensure_root(origin)?; let who = T::Lookup::lookup(who)?; - let mut members = Self::members(); - if let Ok(index) = members.binary_search(&who) { + let mut members_with_stake = Self::members(); + if let Ok(index) = members_with_stake.binary_search_by(|(ref m, ref _s)| m.cmp(&who)) { // remove, slash, emit event. - members.remove(index); + members_with_stake.remove(index); let (imbalance, _) = T::Currency::slash_reserved(&who, T::CandidacyBond::get()); T::KickedMember::on_unbalanced(imbalance); Self::deposit_event(RawEvent::MemberKicked(who.clone())); let mut runners_up = Self::runners_up(); - if let Some(replacement) = runners_up.pop() { + if let Some((replacement, stake)) = runners_up.pop() { // replace the outgoing with the best runner up. - if let Err(index) = members.binary_search(&replacement) { - members.insert(index, replacement.clone()); + if let Err(index) = members_with_stake + .binary_search_by(|(ref m, ref _s)| m.cmp(&replacement)) + { + members_with_stake.insert(index, (replacement.clone(), stake)); ElectionRounds::mutate(|v| *v += 1); - T::ChangeMembers::change_members_sorted(&[replacement], &[who], &members); + T::ChangeMembers::change_members_sorted( + &[replacement], + &[who], + &members_with_stake + .iter() + .map(|(m, _)| m.clone()) + .collect::>(), + ); } // else it would mean that the runner up was already a member. This cannot // happen. If it does, not much that we can do about it. - >::put(members); + >::put(members_with_stake); >::put(runners_up); } else { // update `Members` storage -- `do_phragmen` adds this to the candidate list. - >::put(members); + >::put(members_with_stake); // trigger a new phragmen. grab a cup of coffee. This might take a while. Self::do_phragmen(); } } } - /// Set the duration of each term. This will affect the next election's block number. - /// - /// # - /// #### State - /// Reads: O(1) - /// Writes: O(1) - /// # - #[weight = SimpleDispatchInfo::FixedOperational(10_000)] - fn set_term_duration(origin, #[compact] count: T::BlockNumber) { - ensure_root(origin)?; - >::put(count); - } - /// What to do at the end of each block. Checks if an election needs to happen or not. fn on_initialize(n: T::BlockNumber) { if let Err(e) = Self::end_block(n) { @@ -394,10 +382,13 @@ decl_module! { } decl_event!( - pub enum Event where ::AccountId { + pub enum Event where + Balance = BalanceOf, + ::AccountId, + { /// A new term with new members. This indicates that enough candidates existed, not that /// enough have has been elected. The inner value must be examined for this purpose. - NewTerm(Vec), + NewTerm(Vec<(AccountId, Balance)>), /// No (or not enough) candidates existed for this round. EmptyTerm, /// A member has been removed. This should always be followed by either `NewTerm` ot @@ -429,7 +420,32 @@ impl Module { /// /// Limited number of members. Binary search. Constant time factor. O(1) fn is_member(who: &T::AccountId) -> bool { - Self::members().binary_search(who).is_ok() + Self::members_ids().binary_search(who).is_ok() + } + + /// Returns number of desired members. + fn desired_members() -> u32 { + T::DesiredMembers::get() + } + + /// Returns number of desired runners up. + fn desired_runners_up() -> u32 { + T::DesiredRunnersUp::get() + } + + /// Returns the term duration + fn term_duration() -> T::BlockNumber { + T::TermDuration::get() + } + + /// Get the members' account ids. + fn members_ids() -> Vec { + Self::members().into_iter().map(|(m, _)| m).collect::>() + } + + /// The the runners' up account ids. + fn runners_up_ids() -> Vec { + Self::runners_up().into_iter().map(|(r, _)| r).collect::>() } /// Check if `who` is a defunct voter. @@ -500,11 +516,11 @@ impl Module { let mut exposed_candidates = candidates.clone(); // current members are always a candidate for the next round as well. // this is guaranteed to not create any duplicates. - candidates.append(&mut Self::members()); + candidates.append(&mut Self::members_ids()); // previous runners_up are also always candidates for the next round. - candidates.append(&mut Self::runners_up()); + candidates.append(&mut Self::runners_up_ids()); // and exposed to being an outgoing in case they are no longer elected. - exposed_candidates.append(&mut Self::runners_up()); + exposed_candidates.append(&mut Self::runners_up_ids()); let voters_and_votes = >::enumerate() .map(|(v, i)| (v, i)) @@ -532,14 +548,35 @@ impl Module { .filter_map(|(m, a)| if a.is_zero() { None } else { Some(m) } ) .collect::>(); + let support_map = phragmen::build_support_map::<_, _, _, T::CurrencyToVote>( + &new_set, + &phragmen_result.assignments, + Self::locked_stake_of, + false, + ); + + let to_balance = |e: ExtendedBalance| + >>::convert(e); + let new_set_with_stake = new_set + .into_iter() + .map(|ref m| { + let support = support_map.get(m) + .expect( + "entire new_set was given to build_support_map; en entry must be \ + created for each item; qed" + ); + (m.clone(), to_balance(support.total)) + }) + .collect::)>>(); + // split new set into winners and runner ups. - let split_point = desired_seats.min(new_set.len()); - let mut new_members = (&new_set[..split_point]).to_vec(); - let runners_up = &new_set[split_point..] + let split_point = desired_seats.min(new_set_with_stake.len()); + let mut new_members = (&new_set_with_stake[..split_point]).to_vec(); + let runners_up = &new_set_with_stake[split_point..] .into_iter() .cloned() .rev() - .collect::>(); + .collect::)>>(); // sort and save the members. new_members.sort(); @@ -550,13 +587,13 @@ impl Module { // report member changes. We compute diff because we need the outgoing list. let (incoming, outgoing) = T::ChangeMembers::compute_members_diff( - &new_members, - &old_members + &Self::members_ids(), + &old_members.into_iter().map(|(m, _)| m).collect::>(), ); T::ChangeMembers::change_members_sorted( &incoming, &outgoing.clone(), - &new_members + &Self::members_ids(), ); // unlike exposed_candidates, these are members who were in the list and no longer @@ -567,7 +604,8 @@ impl Module { // runner up list is not sorted. O(K*N) given K runner ups. Overall: O(NLogM + N*K) exposed_candidates.into_iter().for_each(|c| { // any candidate who is not a member and not a runner up. - if new_members.binary_search(&c).is_err() && !runners_up.contains(&c) + if new_members.binary_search_by_key(&c, |(m, _)| m.clone()).is_err() + && !Self::runners_up_ids().contains(&c) { let (imbalance, _) = T::Currency::slash_reserved(&c, T::CandidacyBond::get()); T::LoserCandidate::on_unbalanced(imbalance); @@ -651,7 +689,9 @@ mod tests { thread_local! { static VOTING_BOND: RefCell = RefCell::new(2); - static MEMBERS: RefCell> = RefCell::new(vec![]); + static DESIRED_MEMBERS: RefCell = RefCell::new(2); + static DESIRED_RUNNERS_UP: RefCell = RefCell::new(2); + static TERM_DURATION: RefCell = RefCell::new(5); } pub struct VotingBond; @@ -659,6 +699,21 @@ mod tests { fn get() -> u64 { VOTING_BOND.with(|v| *v.borrow()) } } + pub struct DesiredMembers; + impl Get for DesiredMembers { + fn get() -> u32 { DESIRED_MEMBERS.with(|v| *v.borrow()) } + } + + pub struct DesiredRunnersUp; + impl Get for DesiredRunnersUp { + fn get() -> u32 { DESIRED_RUNNERS_UP.with(|v| *v.borrow()) } + } + + pub struct TermDuration; + impl Get for TermDuration { + fn get() -> u64 { TERM_DURATION.with(|v| *v.borrow()) } + } + pub struct TestChangeMembers; impl ChangeMembers for TestChangeMembers { fn change_members_sorted(_: &[u64], _: &[u64], _: &[u64]) {} @@ -682,6 +737,9 @@ mod tests { type ChangeMembers = TestChangeMembers; type CandidacyBond = CandidacyBond; type VotingBond = VotingBond; + type TermDuration = TermDuration; + type DesiredMembers = DesiredMembers; + type DesiredRunnersUp = DesiredRunnersUp; type LoserCandidate = (); type KickedMember = (); type BadReport = (); @@ -698,7 +756,7 @@ mod tests { { System: system::{Module, Call, Event}, Balances: balances::{Module, Call, Event, Config}, - Elections: elections::{Module, Call, Event, Config}, + Elections: elections::{Module, Call, Event}, } ); @@ -707,7 +765,6 @@ mod tests { voter_bond: u64, term_duration: u64, desired_runners_up: u32, - members: Vec, } impl Default for ExtBuilder { @@ -717,7 +774,6 @@ mod tests { voter_bond: 2, desired_runners_up: 0, term_duration: 5, - members: vec![], } } } @@ -735,12 +791,10 @@ mod tests { self.term_duration = duration; self } - pub fn members(mut self, members: Vec) -> Self { - self.members = members; - self - } pub fn build(self) -> runtime_io::TestExternalities { VOTING_BOND.with(|v| *v.borrow_mut() = self.voter_bond); + TERM_DURATION.with(|v| *v.borrow_mut() = self.term_duration); + DESIRED_RUNNERS_UP.with(|v| *v.borrow_mut() = self.desired_runners_up); GenesisConfig { balances: Some(balances::GenesisConfig::{ balances: vec![ @@ -753,12 +807,6 @@ mod tests { ], vesting: vec![], }), - elections: Some(elections::GenesisConfig::{ - members: self.members, - desired_members: 2, - desired_runners_up: self.desired_runners_up, - term_duration: self.term_duration, - }), }.build_storage().unwrap().into() } } @@ -798,10 +846,9 @@ mod tests { } #[test] - fn passive_module_should_work() { + fn term_duration_zero_is_passive() { ExtBuilder::default() .term_duration(0) - .members(vec![1, 2, 3]) .build() .execute_with(|| { @@ -810,17 +857,16 @@ mod tests { assert_eq!(Elections::desired_members(), 2); assert_eq!(Elections::election_rounds(), 0); - assert_eq!(Elections::members(), vec![1, 2, 3]); + assert_eq!(Elections::members_ids(), vec![]); assert_eq!(Elections::runners_up(), vec![]); - assert_eq!(Elections::candidates(), vec![]); - assert_eq!(all_voters(), vec![]); System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![1, 2, 3]); + assert_eq!(Elections::members_ids(), vec![]); assert_eq!(Elections::runners_up(), vec![]); + assert_eq!(Elections::candidates(), vec![]); }); } @@ -863,7 +909,7 @@ mod tests { assert!(Elections::is_candidate(&2).is_ok()); assert_eq!(Elections::candidates(), vec![1, 2]); - assert_eq!(Elections::members(), vec![]); + assert_eq!(Elections::members_ids(), vec![]); assert_eq!(Elections::runners_up(), vec![]); System::set_block_number(5); @@ -873,7 +919,7 @@ mod tests { assert!(Elections::is_candidate(&2).is_err()); assert_eq!(Elections::candidates(), vec![]); - assert_eq!(Elections::members(), vec![]); + assert_eq!(Elections::members_ids(), vec![]); assert_eq!(Elections::runners_up(), vec![]); }); } @@ -901,7 +947,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![5]); + assert_eq!(Elections::members_ids(), vec![5]); assert_eq!(Elections::runners_up(), vec![]); assert_eq!(Elections::candidates(), vec![]); @@ -994,7 +1040,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::candidates(), vec![]); assert_ok!(Elections::vote(Origin::signed(3), vec![4, 5], 10)); @@ -1016,7 +1062,7 @@ mod tests { #[test] fn cannot_vote_for_less_than_ed() { - ExtBuilder::default().voter_bond(8).build().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_ok!(Elections::submit_candidacy(Origin::signed(5))); assert_ok!(Elections::submit_candidacy(Origin::signed(4))); @@ -1101,7 +1147,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![3, 5]); + assert_eq!(Elections::members_ids(), vec![3, 5]); }); } @@ -1130,7 +1176,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::candidates(), vec![]); // all of them have a member that they voted for. @@ -1165,7 +1211,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::candidates(), vec![]); assert_eq!(balances(&3), (28, 2)); @@ -1194,7 +1240,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::candidates(), vec![]); assert_eq!(balances(&4), (35, 5)); @@ -1237,7 +1283,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![3, 5]); + assert_eq!(Elections::members(), vec![(3, 30), (5, 20)]); assert_eq!(Elections::runners_up(), vec![]); assert_eq_uvec!(all_voters(), vec![2, 3, 4]); assert_eq!(Elections::candidates(), vec![]); @@ -1259,7 +1305,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![5]); + assert_eq!(Elections::members(), vec![(5, 50)]); assert_eq!(Elections::election_rounds(), 1); // but now it has a valid target. @@ -1269,7 +1315,7 @@ mod tests { assert_ok!(Elections::end_block(System::block_number())); // candidate 4 is affected by an old vote. - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members(), vec![(4, 30), (5, 50)]); assert_eq!(Elections::election_rounds(), 2); assert_eq_uvec!(all_voters(), vec![3, 5]); }); @@ -1292,7 +1338,7 @@ mod tests { assert_ok!(Elections::end_block(System::block_number())); assert_eq!(Elections::election_rounds(), 1); - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members_ids(), vec![4, 5]); }); } @@ -1307,7 +1353,7 @@ mod tests { assert_eq!(Elections::candidates(), vec![]); assert_eq!(Elections::election_rounds(), 1); - assert_eq!(Elections::members(), vec![]); + assert_eq!(Elections::members_ids(), vec![]); }); } @@ -1327,9 +1373,9 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); // sorted based on account id. - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members_ids(), vec![4, 5]); // sorted based on merit (least -> most) - assert_eq!(Elections::runners_up(), vec![3, 2]); + assert_eq!(Elections::runners_up_ids(), vec![3, 2]); // runner ups are still locked. assert_eq!(balances(&4), (35, 5)); @@ -1353,15 +1399,15 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![4, 5]); - assert_eq!(Elections::runners_up(), vec![2, 3]); + assert_eq!(Elections::members(), vec![(4, 40), (5, 50)]); + assert_eq!(Elections::runners_up(), vec![(2, 20), (3, 30)]); assert_ok!(Elections::vote(Origin::signed(5), vec![5], 15)); System::set_block_number(10); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![3, 4]); - assert_eq!(Elections::runners_up(), vec![5, 2]); + assert_eq!(Elections::members(), vec![(3, 30), (4, 40)]); + assert_eq!(Elections::runners_up(), vec![(5, 15), (2, 20)]); }); } @@ -1378,9 +1424,9 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members_ids(), vec![4, 5]); - assert_eq!(Elections::runners_up(), vec![2]); + assert_eq!(Elections::runners_up_ids(), vec![2]); assert_eq!(balances(&2), (15, 5)); assert_ok!(Elections::submit_candidacy(Origin::signed(3))); @@ -1388,9 +1434,9 @@ mod tests { System::set_block_number(10); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members_ids(), vec![4, 5]); - assert_eq!(Elections::runners_up(), vec![3]); + assert_eq!(Elections::runners_up_ids(), vec![3]); assert_eq!(balances(&3), (25, 5)); assert_eq!(balances(&2), (15, 2)); }); @@ -1408,7 +1454,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::election_rounds(), 1); assert_ok!(Elections::submit_candidacy(Origin::signed(2))); @@ -1426,7 +1472,7 @@ mod tests { assert_ok!(Elections::end_block(System::block_number())); // 4 removed; 5 and 3 are the new best. - assert_eq!(Elections::members(), vec![3, 5]); + assert_eq!(Elections::members_ids(), vec![3, 5]); }); } @@ -1449,8 +1495,8 @@ mod tests { System::set_block_number(b.into()); assert_ok!(Elections::end_block(System::block_number())); // we keep re-electing the same folks. - assert_eq!(Elections::members(), vec![4, 5]); - assert_eq!(Elections::runners_up(), vec![2, 3]); + assert_eq!(Elections::members(), vec![(4, 40), (5, 50)]); + assert_eq!(Elections::runners_up(), vec![(2, 20), (3, 30)]); // no new candidates but old members and runners-up are always added. assert_eq!(Elections::candidates(), vec![]); assert_eq!(Elections::election_rounds(), b / 5); @@ -1476,7 +1522,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::election_rounds(), 1); // a new candidate @@ -1487,7 +1533,7 @@ mod tests { assert_eq!(balances(&4), (35, 2)); // slashed assert_eq!(Elections::election_rounds(), 2); // new election round - assert_eq!(Elections::members(), vec![3, 5]); // new members + assert_eq!(Elections::members_ids(), vec![3, 5]); // new members }); } @@ -1509,7 +1555,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![3, 5]); + assert_eq!(Elections::members_ids(), vec![3, 5]); assert_eq!(Elections::election_rounds(), 1); assert_ok!(Elections::remove_voter(Origin::signed(2))); @@ -1520,7 +1566,7 @@ mod tests { // meanwhile, no one cares to become a candidate again. System::set_block_number(10); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![]); + assert_eq!(Elections::members_ids(), vec![]); assert_eq!(Elections::election_rounds(), 2); }); } @@ -1538,14 +1584,14 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![5]); + assert_eq!(Elections::members_ids(), vec![5]); assert_ok!(Elections::remove_voter(Origin::signed(5))); assert_eq!(balances(&5), (47, 3)); System::set_block_number(10); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![]); + assert_eq!(Elections::members_ids(), vec![]); assert_eq!(balances(&5), (50, 0)); }); @@ -1565,7 +1611,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![5]); + assert_eq!(Elections::members_ids(), vec![5]); // winner assert_eq!(balances(&5), (47, 3)); @@ -1585,7 +1631,7 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq!(Elections::members(), vec![4, 5]); + assert_eq!(Elections::members_ids(), vec![4, 5]); assert_ok!(Elections::submit_candidacy(Origin::signed(1))); assert_ok!(Elections::submit_candidacy(Origin::signed(2))); @@ -1605,7 +1651,7 @@ mod tests { assert_ok!(Elections::end_block(System::block_number())); // 3, 4 are new members, must still be bonded, nothing slashed. - assert_eq!(Elections::members(), vec![3, 4]); + assert_eq!(Elections::members(), vec![(3, 30), (4, 48)]); assert_eq!(balances(&3), (25, 5)); assert_eq!(balances(&4), (35, 5)); @@ -1615,7 +1661,10 @@ mod tests { // 5 is an outgoing loser, it will get their bond back. assert_eq!(balances(&5), (48, 2)); - assert_eq!(System::events()[0].event, Event::elections(RawEvent::NewTerm(vec![4, 5]))); + assert_eq!( + System::events()[0].event, + Event::elections(RawEvent::NewTerm(vec![(4, 40), (5, 50)])), + ); }) } @@ -1632,8 +1681,30 @@ mod tests { System::set_block_number(5); assert_ok!(Elections::end_block(System::block_number())); - assert_eq_uvec!(Elections::members(), vec![3, 4]); + assert_eq_uvec!(Elections::members_ids(), vec![3, 4]); assert_eq!(Elections::election_rounds(), 1); }); } + + #[test] + fn members_are_sorted_based_on_id_runners_on_merit() { + ExtBuilder::default().desired_runners_up(2).build().execute_with(|| { + assert_ok!(Elections::submit_candidacy(Origin::signed(5))); + assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(Elections::submit_candidacy(Origin::signed(3))); + assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + + assert_ok!(Elections::vote(Origin::signed(2), vec![3], 20)); + assert_ok!(Elections::vote(Origin::signed(3), vec![2], 30)); + assert_ok!(Elections::vote(Origin::signed(4), vec![5], 40)); + assert_ok!(Elections::vote(Origin::signed(5), vec![4], 50)); + + System::set_block_number(5); + assert_ok!(Elections::end_block(System::block_number())); + // id: low -> high. + assert_eq!(Elections::members(), vec![(4, 50), (5, 40)]); + // merit: low -> high. + assert_eq!(Elections::runners_up(), vec![(3, 20), (2, 30)]); + }); + } } diff --git a/substrate/srml/example/src/lib.rs b/substrate/srml/example/src/lib.rs index c9d9d93cc7..fb05e731bd 100644 --- a/substrate/srml/example/src/lib.rs +++ b/substrate/srml/example/src/lib.rs @@ -269,13 +269,13 @@ use sr_primitives::{ // the arguments and makes a decision based upon them. // // The `WeightData` trait has access to the arguments of the dispatch that it wants to assign a -// weight to. Nonetheless, the trait itself can not make any assumptions about what that type -// generic type of the arguments, `T`, is. Based on our needs, we could replace `T` with a more -// concrete type while implementing the trait. The `decl_module!` expects whatever implements -// `WeighData` to replace `T` with a tuple of the dispatch arguments. This is exactly how we will -// craft the implementation below. +// weight to. Nonetheless, the trait itself can not make any assumptions about what the generic type +// of the arguments (`T`) is. Based on our needs, we could replace `T` with a more concrete type +// while implementing the trait. The `decl_module!` expects whatever implements `WeighData` to +// replace `T` with a tuple of the dispatch arguments. This is exactly how we will craft the +// implementation below. // -// The rules of `WeightForSetDummy` is as follows: +// The rules of `WeightForSetDummy` are as follows: // - The final weight of each dispatch is calculated as the argument of the call multiplied by the // parameter given to the `WeightForSetDummy`'s constructor. // - assigns a dispatch class `operational` if the argument of the call is more than 1000. @@ -449,9 +449,8 @@ decl_module! { // The _right-hand-side_ value of the `#[weight]` attribute can be any type that implements // a set of traits, namely [`WeighData`] and [`ClassifyDispatch`]. The former conveys the // weight (a numeric representation of pure execution time and difficulty) of the - // transaction and the latter demonstrates the `DispatchClass` of the call. A higher weight - // means a larger transaction (less of which can be placed in a single block). See the - // `CheckWeight` signed extension struct in the `system` module for more information. + // transaction and the latter demonstrates the [`DispatchClass`] of the call. A higher + // weight means a larger transaction (less of which can be placed in a single block). #[weight = SimpleDispatchInfo::FixedNormal(10_000)] fn accumulate_dummy(origin, increase_by: T::Balance) -> Result { // This is a public call, so we ensure that the origin is some signed account. diff --git a/substrate/srml/staking/src/lib.rs b/substrate/srml/staking/src/lib.rs index f6d123bd13..edbf7c245f 100644 --- a/substrate/srml/staking/src/lib.rs +++ b/substrate/srml/staking/src/lib.rs @@ -278,7 +278,7 @@ use sr_staking_primitives::{ use sr_primitives::{Serialize, Deserialize}; use system::{ensure_signed, ensure_root}; -use phragmen::{elect, equalize, ExtendedBalance, Support, SupportMap, PhragmenStakedAssignment}; +use phragmen::{elect, equalize, build_support_map, ExtendedBalance, PhragmenStakedAssignment}; const DEFAULT_MINIMUM_VALIDATOR_COUNT: u32 = 4; const MAX_NOMINATIONS: usize = 16; @@ -1268,31 +1268,12 @@ impl Module { let to_balance = |e: ExtendedBalance| >>::convert(e); - // Initialize the support of each candidate. - let mut supports = >::new(); - elected_stashes - .iter() - .map(|e| (e, to_votes(Self::slashable_balance_of(e)))) - .for_each(|(e, s)| { - let item = Support { own: s, total: s, ..Default::default() }; - supports.insert(e.clone(), item); - }); - - // build support struct. - for (n, assignment) in assignments.iter() { - for (c, per_thing) in assignment.iter() { - let nominator_stake = to_votes(Self::slashable_balance_of(n)); - // AUDIT: it is crucially important for the `Mul` implementation of all - // per-things to be sound. - let other_stake = *per_thing * nominator_stake; - if let Some(support) = supports.get_mut(c) { - // For an astronomically rich validator with more astronomically rich - // set of nominators, this might saturate. - support.total = support.total.saturating_add(other_stake); - support.others.push((n.clone(), other_stake)); - } - } - } + let mut supports = build_support_map::<_, _, _, T::CurrencyToVote>( + &elected_stashes, + &assignments, + Self::slashable_balance_of, + true, + ); if cfg!(feature = "equalize") { let mut staked_assignments diff --git a/substrate/srml/system/src/lib.rs b/substrate/srml/system/src/lib.rs index 87c38096ab..e07b937751 100644 --- a/substrate/srml/system/src/lib.rs +++ b/substrate/srml/system/src/lib.rs @@ -772,7 +772,6 @@ impl CheckWeight { fn get_dispatch_limit_ratio(class: DispatchClass) -> Perbill { match class { DispatchClass::Operational => Perbill::one(), - // TODO: this must be some sort of a constant. DispatchClass::Normal => T::AvailableBlockRatio::get(), } }