Fix #282

bin/node/runtime/src/lib.rs

@@ -523,7 +523,6 @@ construct_runtime!(
 		TransactionPayment: pallet_transaction_payment::{Module, Storage},
 		Session: pallet_session::{Module, Call, Storage, Event, Config<T>},
 		Council: pallet_collective::<Instance1>::{Module, Call, Storage, Origin<T>, Event<T>, Config<T>},
-		Elections: pallet_elections_phragmen::{Module, Call, Storage, Event<T>},
 		TechnicalCommittee: pallet_collective::<Instance2>::{Module, Call, Storage, Origin<T>, Event<T>, Config<T>},
 		TechnicalMembership: pallet_membership  ::<Instance1>,
 		FinalityTracker: pallet_finality_tracker::{Module, Call, Inherent},
@@ -536,6 +535,7 @@ construct_runtime!(
 		RandomnessCollectiveFlip: pallet_randomness_collective_flip::{Module, Call, Storage},
 		Nicks: pallet_nicks::{Module, Call, Storage, Event<T>},
 
+		Elections: pallet_elections_phragmen::{Module, Call, Storage, Event<T>},
 		EthBacking: pallet_eth_backing,
 		EthRelay: pallet_eth_relay,
 		Kton: pallet_kton,

frame/staking/src/lib.rs

@@ -626,10 +626,6 @@ where
 	pub own_kton_balance: KtonBalance,
 	pub own_power: Power,
 	/// The total balance backing this validator.
-	#[codec(compact)]
-	pub total_ring_balance: RingBalance,
-	#[codec(compact)]
-	pub total_kton_balance: KtonBalance,
 	pub total_power: Power,
 	/// The portions of nominators stashes that are exposed.
 	pub others: Vec<IndividualExposure<AccountId, RingBalance, KtonBalance>>,
@@ -1618,7 +1614,8 @@ impl<T: Trait> Module<T> {
 	// PUBLIC IMMUTABLES
 
 	// power is a mixture of ring and kton
-	// power = ring_ratio * POWER_COUNT / 2 + kton_ratio * POWER_COUNT / 2
+	// For *RING* power = ring_ratio * POWER_COUNT / 2
+	// For *KTON* power = kton_ratio * POWER_COUNT / 2
 	pub fn currency_to_power<S: TryInto<Balance>>(active: S, pool: S) -> Power {
 		(Perquintill::from_rational_approximation(
 			active.saturated_into::<Balance>(),
@@ -2093,8 +2090,6 @@ impl<T: Trait> Module<T> {
 					own_ring_balance: s.own_ring_balance,
 					own_kton_balance: s.own_kton_balance,
 					own_power: to_power(s.own_votes),
-					total_ring_balance: s.total_ring_balance,
-					total_kton_balance: s.total_kton_balance,
 					total_power: to_power(s.total_votes),
 					others: s
 						.others

primitives/phragmen/src/lib.rs

@@ -23,13 +23,13 @@ mod mock;
 mod tests;
 
 use sp_runtime::{
-	traits::{Member, Saturating, SimpleArithmetic, Zero},
+	helpers_128bit::multiply_by_rational,
+	traits::{Member, SaturatedConversion, Saturating, SimpleArithmetic, Zero},
 	Perbill, RuntimeDebug,
 };
 use sp_std::{collections::btree_map::BTreeMap, prelude::*};
 
 use darwinia_support::Rational64;
-use sp_runtime::traits::SaturatedConversion;
 
 /// Power of an account.
 pub type Power = u32;
@@ -116,13 +116,11 @@ pub struct PhragmenResult<AccountId> {
 #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
 pub struct Support<AccountId, RingBalance, KtonBalance> {
 	/// The amount of support as the effect of self-vote.
-	pub own_votes: Votes,
 	pub own_ring_balance: RingBalance,
 	pub own_kton_balance: KtonBalance,
+	pub own_votes: Votes,
 	/// Total support.
 	pub total_votes: Votes,
-	pub total_ring_balance: RingBalance,
-	pub total_kton_balance: KtonBalance,
 	/// Support from voters.
 	pub others: Vec<PhragmenStakedAssignment<AccountId, RingBalance, KtonBalance>>,
 }
@@ -355,23 +353,21 @@ where
 		for (c, per_thing) in assignment.iter() {
 			if let Some(support) = supports.get_mut(c) {
 				let nominator_stake = to_votes(power_of(n));
+				// AUDIT: it is crucially important for the `Mul` implementation of all
+				// per-things to be sound.
 				let (ring_balance, kton_balance) = {
 					let (r, k) = stake_of(n);
 					(*per_thing * r, *per_thing * k)
 				};
-				// AUDIT: it is crucially important for the `Mul` implementation of all
-				// per-things to be sound.
 				let other_stake = *per_thing * nominator_stake;
 				if c == n {
 					// This is a nomination from `n` to themselves. This will increase both the
 					// `own` and `total` field.
 					debug_assert!(*per_thing == Perbill::one()); // TODO: deal with this: do we want it?
 
 					support.own_ring_balance = support.own_ring_balance.saturating_add(ring_balance);
-					support.total_ring_balance = support.total_ring_balance.saturating_add(ring_balance);
 
 					support.own_kton_balance = support.own_kton_balance.saturating_add(kton_balance);
-					support.total_kton_balance = support.total_kton_balance.saturating_add(kton_balance);
 
 					support.own_votes = support.own_votes.saturating_add(other_stake);
 					support.total_votes = support.total_votes.saturating_add(other_stake);
@@ -380,8 +376,6 @@ where
 					// inside `others`.
 					// For an astronomically rich validator with more astronomically rich
 					// set of nominators, this might saturate.
-					support.total_ring_balance = support.total_ring_balance.saturating_add(ring_balance);
-					support.total_kton_balance = support.total_kton_balance.saturating_add(kton_balance);
 					support.total_votes = support.total_votes.saturating_add(other_stake);
 
 					support.others.push(PhragmenStakedAssignment {
@@ -532,25 +526,21 @@ where
 		idx += 1;
 	}
 
-	let PhragmenStakedAssignment {
-		ring_balance: last_ring_balance,
-		kton_balance: last_kton_balance,
-		votes: last_votes,
-		..
-	} = elected_edges[last_index];
+	let last_votes = elected_edges[last_index].votes;
 	let split_ways = last_index + 1;
 	let excess = budget
 		.saturating_add(cumulative_stake)
 		.saturating_sub(last_votes.saturating_mul(split_ways as Votes));
 	elected_edges.iter_mut().take(split_ways).for_each(|e| {
 		if let Some(support) = support_map.get_mut(&e.account_id) {
-			e.ring_balance = ((excess.saturated_into::<RingBalance>() / split_ways.saturated_into::<RingBalance>())
-				+ last_ring_balance)
-				.saturating_sub(support.total_ring_balance);
-			e.kton_balance = ((excess.saturated_into::<KtonBalance>() / split_ways.saturated_into::<KtonBalance>())
-				+ last_kton_balance)
-				.saturating_sub(support.total_kton_balance);
-			e.votes = ((excess / split_ways as Votes) + last_votes).saturating_sub(support.total_votes);
+			let votes = ((excess / split_ways as Votes) + last_votes).saturating_sub(support.total_votes);
+			e.ring_balance = multiply_by_rational(e.ring_balance.saturated_into(), votes as _, e.votes as _)
+				.unwrap_or(0)
+				.saturated_into();
+			e.kton_balance = multiply_by_rational(e.kton_balance.saturated_into(), votes as _, e.votes as _)
+				.unwrap_or(0)
+				.saturated_into();
+			e.votes = votes;
 
 			support.total_votes = support.total_votes.saturating_add(e.votes);
 			support.others.push(PhragmenStakedAssignment {

primitives/phragmen/src/mock.rs

@@ -53,8 +53,8 @@ pub(crate) fn create_stake_of(stakes: &[(AccountId, Power)]) -> Box<dyn Fn(&Acco
 	stakes.iter().for_each(|s| {
 		storage.insert(s.0, s.1);
 	});
-	let stake_of = move |who: &AccountId| -> Power { storage.get(who).unwrap().to_owned() };
-	Box::new(stake_of)
+	let power_of = move |who: &AccountId| -> Power { storage.get(who).unwrap().to_owned() };
+	Box::new(power_of)
 }
 
 pub(crate) fn auto_generate_self_voters<A: Clone>(candidates: &[A]) -> Vec<(A, Vec<A>)> {
@@ -71,16 +71,16 @@ pub(crate) fn check_assignments(assignments: Vec<(AccountId, Vec<PhragmenAssignm
 pub(crate) fn run_and_compare(
 	candidates: Vec<AccountId>,
 	voters: Vec<(AccountId, Vec<AccountId>)>,
-	stake_of: Box<dyn Fn(&AccountId) -> Power>,
+	power_of: Box<dyn Fn(&AccountId) -> Power>,
 	to_elect: usize,
 	min_to_elect: usize,
 ) {
 	// run fixed point code.
 	let PhragmenResult { winners, assignments } =
-		elect::<_, _>(to_elect, min_to_elect, candidates.clone(), voters.clone(), &stake_of).unwrap();
+		elect::<_, _>(to_elect, min_to_elect, candidates.clone(), voters.clone(), &power_of).unwrap();
 
 	// run float poc code.
-	let truth_value = elect_float(to_elect, min_to_elect, candidates, voters, &stake_of).unwrap();
+	let truth_value = elect_float(to_elect, min_to_elect, candidates, voters, &power_of).unwrap();
 
 	assert_eq!(winners, truth_value.winners);
 
@@ -110,7 +110,7 @@ pub(crate) fn elect_float<A, FS>(
 	minimum_candidate_count: usize,
 	initial_candidates: Vec<A>,
 	initial_voters: Vec<(A, Vec<A>)>,
-	stake_of: FS,
+	power_of: FS,
 ) -> Option<_PhragmenResult<A>>
 where
 	A: Default + Ord + Member + Copy,
@@ -139,7 +139,7 @@ where
 	}
 
 	voters.extend(initial_voters.into_iter().map(|(who, votes)| {
-		let voter_stake = stake_of(&who) as f64;
+		let voter_stake = power_of(&who) as f64;
 		let mut edges: Vec<_Edge<A>> = Vec::with_capacity(votes.len());
 		for v in votes {
 			if let Some(idx) = c_idx_cache.get(&v) {
@@ -220,15 +220,15 @@ where
 	})
 }
 
-pub(crate) fn build_support_map<FS>(result: &mut _PhragmenResult<AccountId>, stake_of: FS) -> _SupportMap<AccountId>
+pub(crate) fn build_support_map<FS>(result: &mut _PhragmenResult<AccountId>, power_of: FS) -> _SupportMap<AccountId>
 where
 	for<'r> FS: Fn(&'r AccountId) -> Power,
 {
 	let mut supports = <_SupportMap<AccountId>>::new();
 	result
 		.winners
 		.iter()
-		.map(|(e, _)| (e, stake_of(e) as f64))
+		.map(|(e, _)| (e, power_of(e) as f64))
 		.for_each(|(e, s)| {
 			let item = _Support {
 				own: s,
@@ -240,7 +240,7 @@ where
 
 	for (n, assignment) in result.assignments.iter_mut() {
 		for (c, r) in assignment.iter_mut() {
-			let nominator_stake = stake_of(n) as f64;
+			let nominator_stake = power_of(n) as f64;
 			let other_stake = nominator_stake * *r;
 			if let Some(support) = supports.get_mut(c) {
 				support.total = support.total + other_stake;
@@ -257,15 +257,15 @@ pub(crate) fn equalize_float<A, FS>(
 	supports: &mut _SupportMap<A>,
 	tolerance: f64,
 	iterations: usize,
-	stake_of: FS,
+	power_of: FS,
 ) where
 	for<'r> FS: Fn(&'r A) -> Power,
 	A: Ord + Clone + std::fmt::Debug,
 {
 	for _i in 0..iterations {
 		let mut max_diff = 0.0;
 		for (voter, assignment) in assignments.iter_mut() {
-			let voter_budget = stake_of(&voter);
+			let voter_budget = power_of(&voter);
 			let diff = do_equalize_float(voter, voter_budget, assignment, supports, tolerance);
 			if diff > max_diff {
 				max_diff = diff;