Refactor: fixed point arithmetic for SRML. (#3456)

* Macro-ify perthings. * Refactor fixed64 * Half-workign phragmen refactor. * Finalize phragmen refactor. * Fix creation of perquintill * Fix build errors * Line-width * Fix more build errors. * Line-width * Fix offence test * Resolve all TODOs. * Apply suggestions from code review Co-Authored-By: Gavin Wood <gavin@parity.io> Co-Authored-By: thiolliere <gui.thiolliere@gmail.com> * Fix most of the review comments. * Updates to multiply by rational * Fxi build * Fix abs issue with Fixed64 * Fix tests and improvements. * Fix build * Remove more tests from staking. * Review comments. * Add fuzzing stuff. * Better fuzzing * Better doc. * Bump. * Master.into() * A bit more hardening. * Final nits. * Update lock * Fix indent. * Revert lock file. * Bump.
2026-06-15 04:31:08 +00:00 · 2019-09-25 11:21:05 +02:00
parent 87688aadaa
commit 1c15ca6ad1
19 changed files with 1909 additions and 961 deletions
@@ -34,15 +34,12 @@
 #![cfg_attr(not(feature = "std"), no_std)]

 use rstd::{prelude::*, collections::btree_map::BTreeMap};
-use sr_primitives::PerU128;
-use sr_primitives::traits::{Zero, Convert, Member, SimpleArithmetic};
+use sr_primitives::{helpers_128bit::multiply_by_rational_best_effort, Perbill, Rational128};
+use sr_primitives::traits::{Zero, Convert, Member, SimpleArithmetic, Saturating};

 mod mock;
 mod tests;

-/// Type used as the fraction.
-type Fraction = PerU128;
-
 /// A type in which performing operations on balances and stakes of candidates and voters are safe.
 ///
 /// This module's functions expect a `Convert` type to convert all balances to u64. Hence, u128 is
@@ -51,16 +48,10 @@ type Fraction = PerU128;
 /// Balance types converted to `ExtendedBalance` are referred to as `Votes`.
 pub type ExtendedBalance = u128;

-// this is only used while creating the candidate score. Due to reasons explained below
-// The more accurate this is, the less likely we choose a wrong candidate.
-// TODO: can be removed with proper use of per-things #2908
-const SCALE_FACTOR: ExtendedBalance = u32::max_value() as ExtendedBalance + 1;
-
-/// These are used to expose a fixed accuracy to the caller function. The bigger they are,
-/// the more accurate we get, but the more likely it is for us to overflow. The case of overflow
-/// is handled but accuracy will be lost. 32 or 16 are reasonable values.
-// TODO: can be removed with proper use of per-things #2908
-pub const ACCURACY: ExtendedBalance = u32::max_value() as ExtendedBalance + 1;
+/// The denominator used for loads. Since votes are collected as u64, the smallest ratio that we
+/// might collect is `1/approval_stake` where approval stake is the sum of votes. Hence, some number
+/// bigger than u64::max_value() is needed. For maximum accuracy we simply use u128;
+const DEN: u128 = u128::max_value();

 /// A candidate entity for phragmen election.
 #[derive(Clone, Default)]
@@ -69,7 +60,7 @@ pub struct Candidate<AccountId> {
 	/// Identifier.
 	pub who: AccountId,
 	/// Intermediary value used to sort candidates.
-	pub score: Fraction,
+	pub score: Rational128,
 	/// Sum of the stake of this candidate based on received votes.
 	approval_stake: ExtendedBalance,
 	/// Flag for being elected.
@@ -87,7 +78,7 @@ pub struct Voter<AccountId> {
 	/// The stake of this voter.
 	budget: ExtendedBalance,
 	/// Incremented each time a candidate that this voter voted for has been elected.
-	load: Fraction,
+	load: Rational128,
 }

 /// A candidate being backed by a voter.
@@ -97,13 +88,16 @@ pub struct Edge<AccountId> {
 	/// Identifier.
 	who: AccountId,
 	/// Load of this vote.
-	load: Fraction,
+	load: Rational128,
 	/// Index of the candidate stored in the 'candidates' vector.
 	candidate_index: usize,
 }

-/// Means a particular `AccountId` was backed by a ratio of `ExtendedBalance / ACCURACY`.
-pub type PhragmenAssignment<AccountId> = (AccountId, ExtendedBalance);
+/// Means a particular `AccountId` was backed by `Perbill`th of a nominator's stake.
+pub type PhragmenAssignment<AccountId> = (AccountId, Perbill);
+
+/// Means a particular `AccountId` was backed by `ExtendedBalance` of a nominator's stake.
+pub type PhragmenStakedAssignment<AccountId> = (AccountId, ExtendedBalance);

 /// Final result of the phragmen election.
 #[cfg_attr(feature = "std", derive(Debug))]
@@ -131,7 +125,7 @@ pub struct Support<AccountId> {
 	/// Total support.
 	pub total: ExtendedBalance,
 	/// Support from voters.
-	pub others: Vec<PhragmenAssignment<AccountId>>,
+	pub others: Vec<PhragmenStakedAssignment<AccountId>>,
 }

 /// A linkage from a candidate and its [`Support`].
@@ -164,8 +158,7 @@ pub fn elect<AccountId, Balance, FS, C>(
 	for<'r> FS: Fn(&'r AccountId) -> Balance,
 	C: Convert<Balance, u64> + Convert<u128, Balance>,
 {
-	let to_votes = |b: Balance|
-		<C as Convert<Balance, u64>>::convert(b) as ExtendedBalance;
+	let to_votes = |b: Balance| <C as Convert<Balance, u64>>::convert(b) as ExtendedBalance;

 	// return structures
 	let mut elected_candidates: Vec<(AccountId, ExtendedBalance)>;
@@ -192,7 +185,7 @@ pub fn elect<AccountId, Balance, FS, C>(
 				who: c.who.clone(),
 				edges: vec![Edge { who: c.who.clone(), candidate_index: i, ..Default::default() }],
 				budget: c.approval_stake,
-				load: Fraction::zero(),
+				load: Rational128::zero(),
 			});
 			c_idx_cache.insert(c.who.clone(), i);
 			c
@@ -229,7 +222,7 @@ pub fn elect<AccountId, Balance, FS, C>(
 			who,
 			edges: edges,
 			budget: to_votes(voter_stake),
-			load: Fraction::zero(),
+			load: Rational128::zero(),
 		}
 	}));

@@ -245,24 +238,29 @@ pub fn elect<AccountId, Balance, FS, C>(
 		// loop 1: initialize score
 		for c in &mut candidates {
 			if !c.elected {
-				c.score = Fraction::from_xth(c.approval_stake);
+				// 1 / approval_stake == (DEN / approval_stake) / DEN. If approval_stake is zero,
+				// then the ratio should be as large as possible, essentially `infinity`.
+				if c.approval_stake.is_zero() {
+					c.score = Rational128::from_unchecked(DEN, 0);
+				} else {
+					c.score = Rational128::from(DEN / c.approval_stake, DEN);
+				}
 			}
 		}
+
 		// loop 2: increment score
 		for n in &voters {
 			for e in &n.edges {
 				let c = &mut candidates[e.candidate_index];
 				if !c.elected && !c.approval_stake.is_zero() {
-					// Basic fixed-point shifting by 32.
-					// `n.budget.saturating_mul(SCALE_FACTOR)` will never saturate
-					// since n.budget cannot exceed u64,despite being stored in u128. yet,
-					// `*n.load / SCALE_FACTOR` might collapse to zero. Hence, 32 or 16 bits are
-					// better scale factors. Note that left-associativity in operators precedence is
-					//  crucially important here.
-					let temp =
-						n.budget.saturating_mul(SCALE_FACTOR) / c.approval_stake
-						* (*n.load / SCALE_FACTOR);
-					c.score = Fraction::from_parts((*c.score).saturating_add(temp));
+					let temp_n = multiply_by_rational_best_effort(
+						n.load.n(),
+						n.budget,
+						c.approval_stake,
+					);
+					let temp_d = n.load.d();
+					let temp = Rational128::from(temp_n, temp_d);
+					c.score = c.score.lazy_saturating_add(temp);
 				}
 			}
 		}
@@ -271,14 +269,14 @@ pub fn elect<AccountId, Balance, FS, C>(
 		if let Some(winner) = candidates
 			.iter_mut()
 			.filter(|c| !c.elected)
-			.min_by_key(|c| *c.score)
+			.min_by_key(|c| c.score)
 		{
 			// loop 3: update voter and edge load
 			winner.elected = true;
 			for n in &mut voters {
 				for e in &mut n.edges {
 					if e.who == winner.who {
-						e.load = Fraction::from_parts(*winner.score - *n.load);
+						e.load = winner.score.lazy_saturating_sub(n.load);
 						n.load = winner.score;
 					}
 				}
@@ -296,48 +294,64 @@ pub fn elect<AccountId, Balance, FS, C>(
 		for e in &mut n.edges {
 			if let Some(c) = elected_candidates.iter().cloned().find(|(c, _)| *c == e.who) {
 				if c.0 != n.who {
-					let ratio = {
-						// Full support. No need to calculate.
-						if *n.load == *e.load { ACCURACY }
-						else {
-							// This should not saturate. Safest is to just check
-							if let Some(r) = ACCURACY.checked_mul(*e.load) {
-								r / n.load.max(1)
+					let per_bill_parts =
+					{
+						if n.load == e.load {
+							// Full support. No need to calculate.
+							Perbill::accuracy().into()
+						} else {
+							if e.load.d() == n.load.d() {
+								// return e.load / n.load.
+								let desired_scale: u128 = Perbill::accuracy().into();
+								multiply_by_rational_best_effort(
+									desired_scale,
+									e.load.n(),
+									n.load.n(),
+								)
 							} else {
-								// Just a simple trick.
-								*e.load / (n.load.max(1) / ACCURACY)
+								// defensive only. Both edge and nominator loads are built from
+								// scores, hence MUST have the same denominator.
+								Zero::zero()
 							}
 						}
 					};
-					assignment.1.push((e.who.clone(), ratio));
+					// safer to .min() inside as well to argue as u32 is safe.
+					let per_thing = Perbill::from_parts(
+						per_bill_parts.min(Perbill::accuracy().into()) as u32
+					);
+					assignment.1.push((e.who.clone(), per_thing));
 				}
 			}
 		}

 		if assignment.1.len() > 0 {
-			// To ensure an assertion indicating: no stake from the voter going to waste, we add
-			//  a minimal post-processing to equally assign all of the leftover stake ratios.
-			let vote_count = assignment.1.len() as ExtendedBalance;
-			let l = assignment.1.len();
-			let sum = assignment.1.iter().map(|a| a.1).sum::<ExtendedBalance>();
-			let diff = ACCURACY.checked_sub(sum).unwrap_or(0);
-			let diff_per_vote= diff / vote_count;
+			// To ensure an assertion indicating: no stake from the nominator going to waste,
+			// we add a minimal post-processing to equally assign all of the leftover stake ratios.
+			let vote_count = assignment.1.len() as u32;
+			let len = assignment.1.len();
+			let sum = assignment.1.iter()
+				.map(|a| a.1.deconstruct())
+				.sum::<u32>();
+			let accuracy = Perbill::accuracy();
+			let diff = accuracy.checked_sub(sum).unwrap_or(0);
+			let diff_per_vote = (diff / vote_count).min(accuracy);

 			if diff_per_vote > 0 {
-				for i in 0..l {
-					assignment.1[i%l].1 =
-						assignment.1[i%l].1
-						.saturating_add(diff_per_vote);
+				for i in 0..len {
+					let current_ratio = assignment.1[i % len].1;
+					let next_ratio = current_ratio
+						.saturating_add(Perbill::from_parts(diff_per_vote));
+					assignment.1[i % len].1 = next_ratio;
 				}
 			}

-			// `remainder` is set to be less than maximum votes of a voter (currently 16).
+			// `remainder` is set to be less than maximum votes of a nominator (currently 16).
 			// safe to cast it to usize.
 			let remainder = diff - diff_per_vote * vote_count;
 			for i in 0..remainder as usize {
-				assignment.1[i%l].1 =
-					assignment.1[i%l].1
-					.saturating_add(1);
+				let current_ratio = assignment.1[i % len].1;
+				let next_ratio = current_ratio.saturating_add(Perbill::from_parts(1));
+				assignment.1[i % len].1 = next_ratio;
 			}
 			assigned.push(assignment);
 		}
@@ -360,8 +374,8 @@ pub fn elect<AccountId, Balance, FS, C>(
 /// * `tolerance`: maximum difference that can occur before an early quite happens.
 /// * `iterations`: maximum number of iterations that will be processed.
 /// * `stake_of`: something that can return the stake stake of a particular candidate or voter.
-pub fn equalize<Balance, AccountId, FS, C>(
-	mut assignments: Vec<(AccountId, Vec<PhragmenAssignment<AccountId>>)>,
+pub fn equalize<Balance, AccountId, C, FS>(
+	mut assignments: Vec<(AccountId, Vec<PhragmenStakedAssignment<AccountId>>)>,
 	supports: &mut SupportMap<AccountId>,
 	tolerance: ExtendedBalance,
 	iterations: usize,
@@ -399,7 +413,7 @@ pub fn equalize<Balance, AccountId, FS, C>(
 fn do_equalize<Balance, AccountId, C>(
 	voter: &AccountId,
 	budget_balance: Balance,
-	elected_edges: &mut Vec<(AccountId, ExtendedBalance)>,
+	elected_edges: &mut Vec<PhragmenStakedAssignment<AccountId>>,
 	support_map: &mut SupportMap<AccountId>,
 	tolerance: ExtendedBalance
 ) -> ExtendedBalance where
@@ -18,10 +18,12 @@

 #![cfg(test)]

-use crate::{elect, ACCURACY, PhragmenResult};
-use sr_primitives::traits::{Convert, Member, SaturatedConversion};
+use crate::{elect, PhragmenResult, PhragmenAssignment};
+use sr_primitives::{
+	assert_eq_error_rate, Perbill,
+	traits::{Convert, Member, SaturatedConversion}
+};
 use rstd::collections::btree_map::BTreeMap;
-use support::assert_eq_error_rate;

 pub(crate) struct TestCurrencyToVote;
 impl Convert<Balance, u64> for TestCurrencyToVote {
@@ -343,6 +345,14 @@ pub(crate) fn create_stake_of(stakes: &[(AccountId, Balance)])
 	Box::new(stake_of)
 }

+
+pub fn check_assignments(assignments: Vec<(AccountId, Vec<PhragmenAssignment<AccountId>>)>) {
+	for (_, a) in assignments {
+		let sum: u32 = a.iter().map(|(_, p)| p.deconstruct()).sum();
+		assert_eq_error_rate!(sum, Perbill::accuracy(), 5);
+	}
+}
+
 pub(crate) fn run_and_compare(
 	candidates: Vec<AccountId>,
 	voters: Vec<(AccountId, Vec<AccountId>)>,
@@ -375,9 +385,13 @@ pub(crate) fn run_and_compare(

 	for (nominator, assigned) in assignments.clone() {
 		if let Some(float_assignments) = truth_value.assignments.iter().find(|x| x.0 == nominator) {
-			for (candidate, ratio) in assigned {
+			for (candidate, per_thingy) in assigned {
 				if let Some(float_assignment) = float_assignments.1.iter().find(|x| x.0 == candidate ) {
-					assert_eq_error_rate!((float_assignment.1 * ACCURACY as f64).round() as u128, ratio, 1);
+					assert_eq_error_rate!(
+						Perbill::from_fraction(float_assignment.1).deconstruct(),
+						per_thingy.deconstruct(),
+						1,
+					);
 				} else {
 					panic!("candidate mismatch. This should never happen.")
 				}
@@ -386,6 +400,8 @@ pub(crate) fn run_and_compare(
 			panic!("nominator mismatch. This should never happen.")
 		}
 	}
+
+	check_assignments(assignments);
 }

 pub(crate) fn build_support_map<FS>(
@@ -414,6 +430,5 @@ pub(crate) fn build_support_map<FS>(
 			*r = other_stake;
 		}
 	}
-
 	supports
 }
@@ -19,8 +19,9 @@
 #![cfg(test)]

 use crate::mock::*;
-use crate::{elect, ACCURACY, PhragmenResult};
+use crate::{elect, PhragmenResult};
 use support::assert_eq_uvec;
+use sr_primitives::Perbill;

 #[test]
 fn float_phragmen_poc_works() {
@@ -90,9 +91,9 @@ fn phragmen_poc_works() {
 	assert_eq_uvec!(
 		assignments,
 		vec![
-			(10, vec![(2, ACCURACY)]),
-			(20, vec![(3, ACCURACY)]),
-			(30, vec![(2, ACCURACY/2), (3, ACCURACY/2)]),
+			(10, vec![(2, Perbill::from_percent(100))]),
+			(20, vec![(3, Perbill::from_percent(100))]),
+			(30, vec![(2, Perbill::from_percent(100/2)), (3, Perbill::from_percent(100/2))]),
 		]
 	);
 }
@@ -133,3 +134,218 @@ fn phragmen_poc_3_works() {

 	run_and_compare(candidates, voters, stake_of, 2, 2, true);
 }
+
+#[test]
+fn phragmen_accuracy_on_large_scale_only_validators() {
+	// because of this particular situation we had per_u128 and now rational128. In practice, a
+	// candidate can have the maximum amount of tokens, and also supported by the maximum.
+	let candidates = vec![1, 2, 3, 4, 5];
+	let stake_of = create_stake_of(&[
+		(1, (u64::max_value() - 1).into()),
+		(2, (u64::max_value() - 4).into()),
+		(3, (u64::max_value() - 5).into()),
+		(4, (u64::max_value() - 3).into()),
+		(5, (u64::max_value() - 2).into()),
+	]);
+
+	let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote>(
+		2,
+		2,
+		candidates,
+		vec![],
+		stake_of,
+		true,
+	).unwrap();
+
+	assert_eq_uvec!(winners, vec![(1, 18446744073709551614u128), (5, 18446744073709551613u128)]);
+	assert_eq!(assignments.len(), 0);
+	check_assignments(assignments);
+}
+
+#[test]
+fn phragmen_accuracy_on_large_scale_validators_and_nominators() {
+	let candidates = vec![1, 2, 3, 4, 5];
+	let voters = vec![
+		(13, vec![1, 3, 5]),
+		(14, vec![2, 4]),
+	];
+	let stake_of = create_stake_of(&[
+		(1,  (u64::max_value() - 1).into()),
+		(2,  (u64::max_value() - 4).into()),
+		(3,  (u64::max_value() - 5).into()),
+		(4,  (u64::max_value() - 3).into()),
+		(5,  (u64::max_value() - 2).into()),
+		(13, (u64::max_value() - 10).into()),
+		(14, u64::max_value().into()),
+	]);
+
+	let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote>(
+		2,
+		2,
+		candidates,
+		voters,
+		stake_of,
+		true,
+	).unwrap();
+
+	assert_eq_uvec!(winners, vec![(2, 36893488147419103226u128), (1, 36893488147419103219u128)]);
+	assert_eq!(
+		assignments,
+		vec![(13, vec![(1, Perbill::one())]), (14, vec![(2, Perbill::one())])]
+	);
+	check_assignments(assignments);
+}
+
+#[test]
+fn phragmen_accuracy_on_small_scale_self_vote() {
+	let candidates = vec![40, 10, 20, 30];
+	let voters = vec![];
+	let stake_of = create_stake_of(&[
+		(40, 0),
+		(10, 1),
+		(20, 2),
+		(30, 1),
+	]);
+
+	let PhragmenResult { winners, assignments: _ } = elect::<_, _, _, TestCurrencyToVote>(
+		3,
+		3,
+		candidates,
+		voters,
+		stake_of,
+		true,
+	).unwrap();
+
+	assert_eq_uvec!(winners, vec![(20, 2), (10, 1), (30, 1)]);
+}
+
+#[test]
+fn phragmen_accuracy_on_small_scale_no_self_vote() {
+	let candidates = vec![40, 10, 20, 30];
+	let voters = vec![
+		(1, vec![10]),
+		(2, vec![20]),
+		(3, vec![30]),
+		(4, vec![40]),
+	];
+	let stake_of = create_stake_of(&[
+		(40, 1000), // don't care
+		(10, 1000), // don't care
+		(20, 1000), // don't care
+		(30, 1000), // don't care
+		(4, 0),
+		(1, 1),
+		(2, 2),
+		(3, 1),
+	]);
+
+	let PhragmenResult { winners, assignments: _ } = elect::<_, _, _, TestCurrencyToVote>(
+		3,
+		3,
+		candidates,
+		voters,
+		stake_of,
+		false,
+	).unwrap();
+
+	assert_eq_uvec!(winners, vec![(20, 2), (10, 1), (30, 1)]);
+}
+
+#[test]
+fn phragmen_large_scale_test() {
+	let candidates = vec![2, 4, 6, 8, 10, 12, 14, 16 ,18, 20, 22, 24];
+	let voters = vec![
+		(50, vec![2, 4, 6, 8, 10, 12, 14, 16 ,18, 20, 22, 24]),
+	];
+	let stake_of = create_stake_of(&[
+		(2,  1),
+		(4,  100),
+		(6,  1000000),
+		(8,  100000000001000),
+		(10, 100000000002000),
+		(12, 100000000003000),
+		(14, 400000000000000),
+		(16, 400000000001000),
+		(18, 18000000000000000),
+		(20, 20000000000000000),
+		(22, 500000000000100000),
+		(24, 500000000000200000),
+		(50, 990000000000000000),
+	]);
+
+	let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote>(
+		2,
+		2,
+		candidates,
+		voters,
+		stake_of,
+		true,
+	).unwrap();
+
+	assert_eq_uvec!(winners, vec![(24, 1490000000000200000u128), (22, 1490000000000100000u128)]);
+	check_assignments(assignments);
+}
+
+#[test]
+fn phragmen_large_scale_test_2() {
+	let nom_budget: u64 = 1_000_000_000_000_000_000;
+	let c_budget: u64 = 4_000_000;
+
+	let candidates = vec![2, 4];
+	let voters = vec![(50, vec![2, 4])];
+
+	let stake_of = create_stake_of(&[
+		(2,  c_budget.into()),
+		(4,  c_budget.into()),
+		(50, nom_budget.into()),
+	]);
+
+	let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote>(
+		2,
+		2,
+		candidates,
+		voters,
+		stake_of,
+		true,
+	).unwrap();
+
+	assert_eq_uvec!(winners, vec![(2, 1000000000004000000u128), (4, 1000000000004000000u128)]);
+	assert_eq!(
+		assignments,
+		vec![(50, vec![(2, Perbill::from_parts(500000001)), (4, Perbill::from_parts(499999999))])],
+	);
+	check_assignments(assignments);
+}
+
+#[test]
+fn phragmen_linear_equalize() {
+	let candidates = vec![11, 21, 31, 41, 51, 61, 71];
+	let voters = vec![
+		(2, vec![11]),
+		(4, vec![11, 21]),
+		(6, vec![21, 31]),
+		(8, vec![31, 41]),
+		(110, vec![41, 51]),
+		(120, vec![51, 61]),
+		(130, vec![61, 71]),
+	];
+	let stake_of = create_stake_of(&[
+		(11, 1000),
+		(21, 1000),
+		(31, 1000),
+		(41, 1000),
+		(51, 1000),
+		(61, 1000),
+		(71, 1000),
+
+		(2, 2000),
+		(4, 1000),
+		(6, 1000),
+		(8, 1000),
+		(110, 1000),
+		(120, 1000),
+		(130, 1000),
+	]);
+
+	run_and_compare(candidates, voters, stake_of, 2, 2, true);
+}