Updates to elections-phragmen and some runtime docs. (#3940)

* minor changes

* Refactors for phragmen-election

* Bump.

* Fix genesis stuff

* Fix rest of the errors

substrate/core/phragmen/src/lib.rs

@@ -290,7 +290,9 @@ pub fn elect<AccountId, Balance, FS, C>(
 		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<AccountId, Balance, FS, C>(
 	})
 }
 
+/// Build the support map from the given phragmen result.
+pub fn build_support_map<Balance, AccountId, FS, C>(
+	elected_stashes: &Vec<AccountId>,
+	assignments: &Vec<(AccountId, Vec<PhragmenAssignment<AccountId>>)>,
+	stake_of: FS,
+	assume_self_vote: bool,
+) -> SupportMap<AccountId> where
+	AccountId: Default + Ord + Member,
+	Balance: Default + Copy + SimpleArithmetic,
+	C: Convert<Balance, u64> + Convert<u128, Balance>,
+	for<'r> FS: Fn(&'r AccountId) -> Balance,
+{
+	let to_votes = |b: Balance| <C as Convert<Balance, u64>>::convert(b) as ExtendedBalance;
+	// Initialize the support of each candidate.
+	let mut supports = <SupportMap<AccountId>>::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.

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 <http://www.gnu.org/licenses/>.
 
-//! 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<T>` and `WeighData<T>` 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<T> {
+	/// 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<T> {
+	/// 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<T> {
-	/// Weigh the data `T` given by `target`.
-	fn weigh_data(&self, target: T) -> Weight;
-}
-
-/// Means of classifying a dispatchable function.
-pub trait ClassifyDispatch<T> {
-	/// 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<T> {
 ///   - 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)]

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

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<CouncilCollective> 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<T>},
 		Council: collective::<Instance1>::{Module, Call, Storage, Origin<T>, Event<T>, Config<T>},
 		TechnicalCommittee: collective::<Instance2>::{Module, Call, Storage, Origin<T>, Event<T>, Config<T>},
-		Elections: elections_phragmen::{Module, Call, Storage, Event<T>, Config<T>},
+		Elections: elections_phragmen::{Module, Call, Storage, Event<T>},
 		TechnicalMembership: membership::<Instance1>::{Module, Call, Storage, Event<T>, Config<T>},
 		FinalityTracker: finality_tracker::{Module, Call, Inherent},
 		Grandpa: grandpa::{Module, Call, Storage, Config, Event},

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()),
 	}

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<NegativeImbalanceOf<Self>>;
+
+	/// Number of members to elect.
+	type DesiredMembers: Get<u32>;
+
+	/// Number of runners_up to keep.
+	type DesiredRunnersUp: Get<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.
+	type TermDuration: Get<Self::BlockNumber>;
 }
 
 decl_storage! {
 	trait Store for Module<T: Trait> 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<T::AccountId>;
+		pub Members get(fn members): Vec<(T::AccountId, BalanceOf<T>)>;
 		/// The current runners_up. Sorted based on low to high merit (worse to best runner).
-		pub RunnersUp get(fn runners_up): Vec<T::AccountId>;
+		pub RunnersUp get(fn runners_up): Vec<(T::AccountId, BalanceOf<T>)>;
 		/// 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> = T::CandidacyBond::get();
 		const VotingBond: BalanceOf<T> = 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! {
 			<Candidates<T>>::mutate(|c| c.insert(index, who));
 		}
 
-		/// Set the desired member count. Changes will be effective at the beginning of next round.
-		///
-		/// # <weight>
-		/// #### State
-		/// Reads: O(1)
-		/// Writes: O(1)
-		/// # </weight>
-		#[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::<Vec<T::AccountId>>(),
+						);
 					}
 					// 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.
 
-					<Members<T>>::put(members);
+					<Members<T>>::put(members_with_stake);
 					<RunnersUp<T>>::put(runners_up);
 				} else {
 					// update `Members` storage -- `do_phragmen` adds this to the candidate list.
-					<Members<T>>::put(members);
+					<Members<T>>::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.
-		///
-		/// # <weight>
-		/// #### State
-		/// Reads: O(1)
-		/// Writes: O(1)
-		/// # </weight>
-		#[weight = SimpleDispatchInfo::FixedOperational(10_000)]
-		fn set_term_duration(origin, #[compact] count: T::BlockNumber) {
-			ensure_root(origin)?;
-			<TermDuration<T>>::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<T> where <T as system::Trait>::AccountId {
+	pub enum Event<T> where
+		Balance = BalanceOf<T>,
+		<T as system::Trait>::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<AccountId>),
+		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<T: Trait> Module<T> {
 	///
 	/// 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<T::AccountId> {
+		Self::members().into_iter().map(|(m, _)| m).collect::<Vec<T::AccountId>>()
+	}
+
+	/// The the runners' up account ids.
+	fn runners_up_ids() -> Vec<T::AccountId> {
+		Self::runners_up().into_iter().map(|(r, _)| r).collect::<Vec<T::AccountId>>()
 	}
 
 	/// Check if `who` is a defunct voter.
@@ -500,11 +516,11 @@ impl<T: Trait> Module<T> {
 		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 = <VotesOf<T>>::enumerate()
 			.map(|(v, i)| (v, i))
@@ -532,14 +548,35 @@ impl<T: Trait> Module<T> {
 				.filter_map(|(m, a)| if a.is_zero() { None } else { Some(m) } )
 				.collect::<Vec<T::AccountId>>();
 
+			let support_map = phragmen::build_support_map::<_, _, _, T::CurrencyToVote>(
+				&new_set,
+				&phragmen_result.assignments,
+				Self::locked_stake_of,
+				false,
+			);
+
+			let to_balance = |e: ExtendedBalance|
+				<T::CurrencyToVote as Convert<ExtendedBalance, BalanceOf<T>>>::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::<Vec<(T::AccountId, BalanceOf<T>)>>();
+
 			// 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::<Vec<T::AccountId>>();
+				.collect::<Vec<(T::AccountId, BalanceOf<T>)>>();
 
 			// sort and save the members.
 			new_members.sort();
@@ -550,13 +587,13 @@ impl<T: Trait> Module<T> {
 
 			// 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::<Vec<T::AccountId>>(),
 			);
 			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<T: Trait> Module<T> {
 			// 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<u64> = RefCell::new(2);
-		static MEMBERS: RefCell<Vec<u64>> = RefCell::new(vec![]);
+		static DESIRED_MEMBERS: RefCell<u32> = RefCell::new(2);
+		static DESIRED_RUNNERS_UP: RefCell<u32> = RefCell::new(2);
+		static TERM_DURATION: RefCell<u64> = 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<u32> for DesiredMembers {
+		fn get() -> u32 { DESIRED_MEMBERS.with(|v| *v.borrow()) }
+	}
+
+	pub struct DesiredRunnersUp;
+	impl Get<u32> for DesiredRunnersUp {
+		fn get() -> u32 { DESIRED_RUNNERS_UP.with(|v| *v.borrow()) }
+	}
+
+	pub struct TermDuration;
+	impl Get<u64> for TermDuration {
+		fn get() -> u64 { TERM_DURATION.with(|v| *v.borrow()) }
+	}
+
 	pub struct TestChangeMembers;
 	impl ChangeMembers<u64> 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<T>, Config<T>},
-			Elections: elections::{Module, Call, Event<T>, Config<T>},
+			Elections: elections::{Module, Call, Event<T>},
 		}
 	);
 
@@ -707,7 +765,6 @@ mod tests {
 		voter_bond: u64,
 		term_duration: u64,
 		desired_runners_up: u32,
-		members: Vec<u64>,
 	}
 
 	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<u64>) -> 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::<Test>{
 					balances: vec![
@@ -753,12 +807,6 @@ mod tests {
 					],
 					vesting: vec![],
 				}),
-				elections: Some(elections::GenesisConfig::<Test>{
-					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)]);
+		});
+	}
 }

substrate/srml/example/src/lib.rs

@@ -269,13 +269,13 @@ use sr_primitives::{
 // the arguments and makes a decision based upon them.
 //
 // The `WeightData<T>` 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<T>` 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<T>` 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.

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<T: Trait> Module<T> {
 			let to_balance = |e: ExtendedBalance|
 				<T::CurrencyToVote as Convert<ExtendedBalance, BalanceOf<T>>>::convert(e);
 
-			// Initialize the support of each candidate.
-			let mut supports = <SupportMap<T::AccountId>>::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

substrate/srml/system/src/lib.rs

@@ -772,7 +772,6 @@ impl<T: Trait + Send + Sync> CheckWeight<T> {
 	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(),
 		}
 	}