diff --git a/substrate/frame/election-provider-multi-phase/src/benchmarking.rs b/substrate/frame/election-provider-multi-phase/src/benchmarking.rs index d37b9451b7..cc322cbb4d 100644 --- a/substrate/frame/election-provider-multi-phase/src/benchmarking.rs +++ b/substrate/frame/election-provider-multi-phase/src/benchmarking.rs @@ -148,7 +148,10 @@ fn solution_with_size( let score = solution.clone().score(stake_of, voter_at, target_at).unwrap(); let round = >::round(); - assert!(score[0] > 0, "score is zero, this probably means that the stakes are not set."); + assert!( + score.minimal_stake > 0, + "score is zero, this probably means that the stakes are not set." + ); Ok(RawSolution { solution, score, round }) } @@ -312,7 +315,7 @@ frame_benchmarking::benchmarks! { // the solution will be worse than all of them meaning the score need to be checked against // ~ log2(c) let solution = RawSolution { - score: [(10_000_000u128 - 1).into(), 0, 0], + score: ElectionScore { minimal_stake: 10_000_000u128 - 1, ..Default::default() }, ..Default::default() }; @@ -323,7 +326,7 @@ frame_benchmarking::benchmarks! { let mut signed_submissions = SignedSubmissions::::get(); for i in 0..c { let raw_solution = RawSolution { - score: [(10_000_000 + i).into(), 0, 0], + score: ElectionScore { minimal_stake: 10_000_000u128 + (i as u128), ..Default::default() }, ..Default::default() }; let signed_submission = SignedSubmission { diff --git a/substrate/frame/election-provider-multi-phase/src/lib.rs b/substrate/frame/election-provider-multi-phase/src/lib.rs index cad3d12605..58df7d5a9d 100644 --- a/substrate/frame/election-provider-multi-phase/src/lib.rs +++ b/substrate/frame/election-provider-multi-phase/src/lib.rs @@ -944,8 +944,11 @@ pub mod pallet { // Note: we don't `rotate_round` at this point; the next call to // `ElectionProvider::elect` will succeed and take care of that. - let solution = - ReadySolution { supports, score: [0, 0, 0], compute: ElectionCompute::Emergency }; + let solution = ReadySolution { + supports, + score: Default::default(), + compute: ElectionCompute::Emergency, + }; >::put(solution); Ok(()) @@ -1059,8 +1062,11 @@ pub mod pallet { }, )?; - let solution = - ReadySolution { supports, score: [0, 0, 0], compute: ElectionCompute::Fallback }; + let solution = ReadySolution { + supports, + score: Default::default(), + compute: ElectionCompute::Fallback, + }; >::put(solution); Ok(()) @@ -1138,10 +1144,10 @@ pub mod pallet { .map_err(dispatch_error_to_invalid)?; ValidTransaction::with_tag_prefix("OffchainElection") - // The higher the score[0], the better a solution is. + // The higher the score.minimal_stake, the better a solution is. .priority( T::MinerTxPriority::get() - .saturating_add(raw_solution.score[0].saturated_into()), + .saturating_add(raw_solution.score.minimal_stake.saturated_into()), ) // Used to deduplicate unsigned solutions: each validator should produce one // solution per round at most, and solutions are not propagate. @@ -1430,7 +1436,7 @@ impl Pallet { let submitted_score = raw_solution.score.clone(); ensure!( Self::minimum_untrusted_score().map_or(true, |min_score| { - sp_npos_elections::is_score_better(submitted_score, min_score, Perbill::zero()) + submitted_score.strict_threshold_better(min_score, Perbill::zero()) }), FeasibilityError::UntrustedScoreTooLow ); @@ -1750,7 +1756,7 @@ mod feasibility_check { assert_eq!(MultiPhase::snapshot().unwrap().voters.len(), 8); // Simply faff with the score. - solution.score[0] += 1; + solution.score.minimal_stake += 1; assert_noop!( MultiPhase::feasibility_check(solution, COMPUTE), @@ -1960,7 +1966,10 @@ mod tests { // fill the queue with signed submissions for s in 0..SignedMaxSubmissions::get() { - let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() }, + ..Default::default() + }; assert_ok!(MultiPhase::submit( crate::mock::Origin::signed(99), Box::new(solution), @@ -2087,13 +2096,19 @@ mod tests { crate::mock::Balancing::set(Some((2, 0))); let (solution, _) = MultiPhase::mine_solution::<::Solver>().unwrap(); - // Default solution has a score of [50, 100, 5000]. - assert_eq!(solution.score, [50, 100, 5000]); + // Default solution's score. + assert!(matches!(solution.score, ElectionScore { minimal_stake: 50, .. })); - >::put([49, 0, 0]); + >::put(ElectionScore { + minimal_stake: 49, + ..Default::default() + }); assert_ok!(MultiPhase::feasibility_check(solution.clone(), ElectionCompute::Signed)); - >::put([51, 0, 0]); + >::put(ElectionScore { + minimal_stake: 51, + ..Default::default() + }); assert_noop!( MultiPhase::feasibility_check(solution, ElectionCompute::Signed), FeasibilityError::UntrustedScoreTooLow, diff --git a/substrate/frame/election-provider-multi-phase/src/signed.rs b/substrate/frame/election-provider-multi-phase/src/signed.rs index 3b314bce80..4362fb5127 100644 --- a/substrate/frame/election-provider-multi-phase/src/signed.rs +++ b/substrate/frame/election-provider-multi-phase/src/signed.rs @@ -28,7 +28,7 @@ use frame_support::{ traits::{defensive_prelude::*, Currency, Get, OnUnbalanced, ReservableCurrency}, }; use sp_arithmetic::traits::SaturatedConversion; -use sp_npos_elections::{is_score_better, ElectionScore, NposSolution}; +use sp_npos_elections::{ElectionScore, NposSolution}; use sp_runtime::{ traits::{Saturating, Zero}, RuntimeDebug, @@ -293,7 +293,7 @@ impl SignedSubmissions { let threshold = T::SolutionImprovementThreshold::get(); // if we haven't improved on the weakest score, don't change anything. - if !is_score_better(insert_score, weakest_score, threshold) { + if !insert_score.strict_threshold_better(weakest_score, threshold) { return InsertResult::NotInserted } @@ -592,7 +592,7 @@ mod tests { assert_eq!(balances(&99), (100, 0)); // make the solution invalid. - solution.score[0] += 1; + solution.score.minimal_stake += 1; assert_ok!(submit_with_witness(Origin::signed(99), solution)); assert_eq!(balances(&99), (95, 5)); @@ -618,7 +618,7 @@ mod tests { assert_ok!(submit_with_witness(Origin::signed(99), solution.clone())); // make the solution invalid and weaker. - solution.score[0] -= 1; + solution.score.minimal_stake -= 1; assert_ok!(submit_with_witness(Origin::signed(999), solution)); assert_eq!(balances(&99), (95, 5)); assert_eq!(balances(&999), (95, 5)); @@ -641,12 +641,18 @@ mod tests { for s in 0..SignedMaxSubmissions::get() { // score is always getting better - let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() }, + ..Default::default() + }; assert_ok!(submit_with_witness(Origin::signed(99), solution)); } // weaker. - let solution = RawSolution { score: [4, 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: 4, ..Default::default() }, + ..Default::default() + }; assert_noop!( submit_with_witness(Origin::signed(99), solution), @@ -663,27 +669,33 @@ mod tests { for s in 0..SignedMaxSubmissions::get() { // score is always getting better - let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() }, + ..Default::default() + }; assert_ok!(submit_with_witness(Origin::signed(99), solution)); } assert_eq!( MultiPhase::signed_submissions() .iter() - .map(|s| s.raw_solution.score[0]) + .map(|s| s.raw_solution.score.minimal_stake) .collect::>(), vec![5, 6, 7, 8, 9] ); // better. - let solution = RawSolution { score: [20, 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: 20, ..Default::default() }, + ..Default::default() + }; assert_ok!(submit_with_witness(Origin::signed(99), solution)); // the one with score 5 was rejected, the new one inserted. assert_eq!( MultiPhase::signed_submissions() .iter() - .map(|s| s.raw_solution.score[0]) + .map(|s| s.raw_solution.score.minimal_stake) .collect::>(), vec![6, 7, 8, 9, 20] ); @@ -698,30 +710,39 @@ mod tests { for s in 1..SignedMaxSubmissions::get() { // score is always getting better - let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() }, + ..Default::default() + }; assert_ok!(submit_with_witness(Origin::signed(99), solution)); } - let solution = RawSolution { score: [4, 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: 4, ..Default::default() }, + ..Default::default() + }; assert_ok!(submit_with_witness(Origin::signed(99), solution)); assert_eq!( MultiPhase::signed_submissions() .iter() - .map(|s| s.raw_solution.score[0]) + .map(|s| s.raw_solution.score.minimal_stake) .collect::>(), vec![4, 6, 7, 8, 9], ); // better. - let solution = RawSolution { score: [5, 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: 5, ..Default::default() }, + ..Default::default() + }; assert_ok!(submit_with_witness(Origin::signed(99), solution)); // the one with score 5 was rejected, the new one inserted. assert_eq!( MultiPhase::signed_submissions() .iter() - .map(|s| s.raw_solution.score[0]) + .map(|s| s.raw_solution.score.minimal_stake) .collect::>(), vec![5, 6, 7, 8, 9], ); @@ -736,7 +757,10 @@ mod tests { for s in 0..SignedMaxSubmissions::get() { // score is always getting better - let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() }, + ..Default::default() + }; assert_ok!(submit_with_witness(Origin::signed(99), solution)); } @@ -744,7 +768,10 @@ mod tests { assert_eq!(balances(&999).1, 0); // better. - let solution = RawSolution { score: [20, 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: 20, ..Default::default() }, + ..Default::default() + }; assert_ok!(submit_with_witness(Origin::signed(999), solution)); // got one bond back. @@ -760,19 +787,25 @@ mod tests { assert!(MultiPhase::current_phase().is_signed()); for i in 0..SignedMaxSubmissions::get() { - let solution = RawSolution { score: [(5 + i).into(), 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: (5 + i).into(), ..Default::default() }, + ..Default::default() + }; assert_ok!(submit_with_witness(Origin::signed(99), solution)); } assert_eq!( MultiPhase::signed_submissions() .iter() - .map(|s| s.raw_solution.score[0]) + .map(|s| s.raw_solution.score.minimal_stake) .collect::>(), vec![5, 6, 7] ); // 5 is not accepted. This will only cause processing with no benefit. - let solution = RawSolution { score: [5, 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: 5, ..Default::default() }, + ..Default::default() + }; assert_noop!( submit_with_witness(Origin::signed(99), solution), Error::::SignedQueueFull, @@ -800,13 +833,13 @@ mod tests { // make the solution invalidly better and submit. This ought to be slashed. let mut solution_999 = solution.clone(); - solution_999.score[0] += 1; + solution_999.score.minimal_stake += 1; assert_ok!(submit_with_witness(Origin::signed(999), solution_999)); // make the solution invalidly worse and submit. This ought to be suppressed and // returned. let mut solution_9999 = solution.clone(); - solution_9999.score[0] -= 1; + solution_9999.score.minimal_stake -= 1; assert_ok!(submit_with_witness(Origin::signed(9999), solution_9999)); assert_eq!( @@ -889,13 +922,19 @@ mod tests { for s in 0..SignedMaxSubmissions::get() { // score is always getting better - let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + let solution = RawSolution { + score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() }, + ..Default::default() + }; assert_ok!(submit_with_witness(Origin::signed(99), solution)); } // this solution has a higher score than any in the queue let solution = RawSolution { - score: [(5 + SignedMaxSubmissions::get()).into(), 0, 0], + score: ElectionScore { + minimal_stake: (5 + SignedMaxSubmissions::get()).into(), + ..Default::default() + }, ..Default::default() }; diff --git a/substrate/frame/election-provider-multi-phase/src/unsigned.rs b/substrate/frame/election-provider-multi-phase/src/unsigned.rs index 196147f8a4..510fc9fcdc 100644 --- a/substrate/frame/election-provider-multi-phase/src/unsigned.rs +++ b/substrate/frame/election-provider-multi-phase/src/unsigned.rs @@ -26,10 +26,9 @@ use codec::Encode; use frame_election_provider_support::{NposSolver, PerThing128}; use frame_support::{dispatch::DispatchResult, ensure, traits::Get}; use frame_system::offchain::SubmitTransaction; -use sp_arithmetic::Perbill; use sp_npos_elections::{ - assignment_ratio_to_staked_normalized, assignment_staked_to_ratio_normalized, is_score_better, - ElectionResult, NposSolution, + assignment_ratio_to_staked_normalized, assignment_staked_to_ratio_normalized, ElectionResult, + NposSolution, }; use sp_runtime::{ offchain::storage::{MutateStorageError, StorageValueRef}, @@ -624,11 +623,9 @@ impl Pallet { // ensure score is being improved. Panic henceforth. ensure!( - Self::queued_solution().map_or(true, |q: ReadySolution<_>| is_score_better::( - raw_solution.score, - q.score, - T::SolutionImprovementThreshold::get() - )), + Self::queued_solution().map_or(true, |q: ReadySolution<_>| raw_solution + .score + .strict_threshold_better(q.score, T::SolutionImprovementThreshold::get())), Error::::PreDispatchWeakSubmission, ); @@ -748,11 +745,11 @@ mod tests { use frame_support::{ assert_noop, assert_ok, bounded_vec, dispatch::Dispatchable, traits::OffchainWorker, }; - use sp_npos_elections::IndexAssignment; + use sp_npos_elections::{ElectionScore, IndexAssignment}; use sp_runtime::{ offchain::storage_lock::{BlockAndTime, StorageLock}, traits::ValidateUnsigned, - ModuleError, PerU16, + ModuleError, PerU16, Perbill, }; type Assignment = crate::unsigned::Assignment; @@ -760,8 +757,10 @@ mod tests { #[test] fn validate_unsigned_retracts_wrong_phase() { ExtBuilder::default().desired_targets(0).build_and_execute(|| { - let solution = - RawSolution:: { score: [5, 0, 0], ..Default::default() }; + let solution = RawSolution:: { + score: ElectionScore { minimal_stake: 5, ..Default::default() }, + ..Default::default() + }; let call = Call::submit_unsigned { raw_solution: Box::new(solution.clone()), witness: witness(), @@ -833,8 +832,10 @@ mod tests { roll_to(25); assert!(MultiPhase::current_phase().is_unsigned()); - let solution = - RawSolution:: { score: [5, 0, 0], ..Default::default() }; + let solution = RawSolution:: { + score: ElectionScore { minimal_stake: 5, ..Default::default() }, + ..Default::default() + }; let call = Call::submit_unsigned { raw_solution: Box::new(solution.clone()), witness: witness(), @@ -849,7 +850,10 @@ mod tests { assert!(::pre_dispatch(&call).is_ok()); // set a better score - let ready = ReadySolution { score: [10, 0, 0], ..Default::default() }; + let ready = ReadySolution { + score: ElectionScore { minimal_stake: 10, ..Default::default() }, + ..Default::default() + }; >::put(ready); // won't work anymore. @@ -874,7 +878,10 @@ mod tests { roll_to(25); assert!(MultiPhase::current_phase().is_unsigned()); - let raw = RawSolution:: { score: [5, 0, 0], ..Default::default() }; + let raw = RawSolution:: { + score: ElectionScore { minimal_stake: 5, ..Default::default() }, + ..Default::default() + }; let call = Call::submit_unsigned { raw_solution: Box::new(raw.clone()), witness: witness() }; assert_eq!(raw.solution.unique_targets().len(), 0); @@ -900,8 +907,10 @@ mod tests { roll_to(25); assert!(MultiPhase::current_phase().is_unsigned()); - let solution = - RawSolution:: { score: [5, 0, 0], ..Default::default() }; + let solution = RawSolution:: { + score: ElectionScore { minimal_stake: 5, ..Default::default() }, + ..Default::default() + }; let call = Call::submit_unsigned { raw_solution: Box::new(solution.clone()), witness: witness(), @@ -930,8 +939,10 @@ mod tests { assert!(MultiPhase::current_phase().is_unsigned()); // This is in itself an invalid BS solution. - let solution = - RawSolution:: { score: [5, 0, 0], ..Default::default() }; + let solution = RawSolution:: { + score: ElectionScore { minimal_stake: 5, ..Default::default() }, + ..Default::default() + }; let call = Call::submit_unsigned { raw_solution: Box::new(solution.clone()), witness: witness(), @@ -950,8 +961,10 @@ mod tests { assert!(MultiPhase::current_phase().is_unsigned()); // This solution is unfeasible as well, but we won't even get there. - let solution = - RawSolution:: { score: [5, 0, 0], ..Default::default() }; + let solution = RawSolution:: { + score: ElectionScore { minimal_stake: 5, ..Default::default() }, + ..Default::default() + }; let mut correct_witness = witness(); correct_witness.voters += 1; @@ -1070,7 +1083,7 @@ mod tests { Box::new(solution), witness )); - assert_eq!(MultiPhase::queued_solution().unwrap().score[0], 10); + assert_eq!(MultiPhase::queued_solution().unwrap().score.minimal_stake, 10); // trial 1: a solution who's score is only 2, i.e. 20% better in the first element. let result = ElectionResult { @@ -1086,7 +1099,7 @@ mod tests { }; let (solution, _) = MultiPhase::prepare_election_result(result).unwrap(); // 12 is not 50% more than 10 - assert_eq!(solution.score[0], 12); + assert_eq!(solution.score.minimal_stake, 12); assert_noop!( MultiPhase::unsigned_pre_dispatch_checks(&solution), Error::::PreDispatchWeakSubmission, @@ -1107,7 +1120,7 @@ mod tests { ], }; let (solution, witness) = MultiPhase::prepare_election_result(result).unwrap(); - assert_eq!(solution.score[0], 17); + assert_eq!(solution.score.minimal_stake, 17); // and it is fine assert_ok!(MultiPhase::unsigned_pre_dispatch_checks(&solution)); diff --git a/substrate/primitives/arithmetic/src/lib.rs b/substrate/primitives/arithmetic/src/lib.rs index 92f8a70831..273608a3d1 100644 --- a/substrate/primitives/arithmetic/src/lib.rs +++ b/substrate/primitives/arithmetic/src/lib.rs @@ -55,7 +55,7 @@ use traits::{BaseArithmetic, One, SaturatedConversion, Unsigned, Zero}; /// - `Ordering::Equal` otherwise. pub trait ThresholdOrd { /// Compare if `self` is `threshold` greater or less than `other`. - fn tcmp(&self, other: &T, epsilon: T) -> Ordering; + fn tcmp(&self, other: &T, threshold: T) -> Ordering; } impl ThresholdOrd for T diff --git a/substrate/primitives/npos-elections/fuzzer/src/phragmen_balancing.rs b/substrate/primitives/npos-elections/fuzzer/src/phragmen_balancing.rs index 8f782405df..76641fc2c7 100644 --- a/substrate/primitives/npos-elections/fuzzer/src/phragmen_balancing.rs +++ b/substrate/primitives/npos-elections/fuzzer/src/phragmen_balancing.rs @@ -23,8 +23,8 @@ use common::*; use honggfuzz::fuzz; use rand::{self, SeedableRng}; use sp_npos_elections::{ - assignment_ratio_to_staked_normalized, is_score_better, seq_phragmen, to_supports, - ElectionResult, EvaluateSupport, VoteWeight, + assignment_ratio_to_staked_normalized, seq_phragmen, to_supports, ElectionResult, + EvaluateSupport, VoteWeight, }; use sp_runtime::Perbill; @@ -60,7 +60,7 @@ fn main() { .unwrap(); let score = to_supports(staked.as_ref()).evaluate(); - if score[0] == 0 { + if score.minimal_stake == 0 { // such cases cannot be improved by balancing. return } @@ -80,7 +80,8 @@ fn main() { to_supports(staked.as_ref()).evaluate() }; - let enhance = is_score_better(balanced_score, unbalanced_score, Perbill::zero()); + let enhance = + balanced_score.strict_threshold_better(unbalanced_score, Perbill::zero()); println!( "iter = {} // {:?} -> {:?} [{}]", @@ -90,9 +91,9 @@ fn main() { // The only guarantee of balancing is such that the first and third element of the // score cannot decrease. assert!( - balanced_score[0] >= unbalanced_score[0] && - balanced_score[1] == unbalanced_score[1] && - balanced_score[2] <= unbalanced_score[2] + balanced_score.minimal_stake >= unbalanced_score.minimal_stake && + balanced_score.sum_stake == unbalanced_score.sum_stake && + balanced_score.sum_stake_squared <= unbalanced_score.sum_stake_squared ); } }); diff --git a/substrate/primitives/npos-elections/fuzzer/src/phragmms_balancing.rs b/substrate/primitives/npos-elections/fuzzer/src/phragmms_balancing.rs index f2b12b1378..09daf3f34d 100644 --- a/substrate/primitives/npos-elections/fuzzer/src/phragmms_balancing.rs +++ b/substrate/primitives/npos-elections/fuzzer/src/phragmms_balancing.rs @@ -23,8 +23,8 @@ use common::*; use honggfuzz::fuzz; use rand::{self, SeedableRng}; use sp_npos_elections::{ - assignment_ratio_to_staked_normalized, is_score_better, phragmms, to_supports, ElectionResult, - EvaluateSupport, VoteWeight, + assignment_ratio_to_staked_normalized, phragmms, to_supports, ElectionResult, EvaluateSupport, + VoteWeight, }; use sp_runtime::Perbill; @@ -60,7 +60,7 @@ fn main() { .unwrap(); let score = to_supports(&staked).evaluate(); - if score[0] == 0 { + if score.minimal_stake == 0 { // such cases cannot be improved by balancing. return } @@ -77,7 +77,7 @@ fn main() { to_supports(staked.as_ref()).evaluate() }; - let enhance = is_score_better(balanced_score, unbalanced_score, Perbill::zero()); + let enhance = balanced_score.strict_threshold_better(unbalanced_score, Perbill::zero()); println!( "iter = {} // {:?} -> {:?} [{}]", @@ -87,9 +87,9 @@ fn main() { // The only guarantee of balancing is such that the first and third element of the score // cannot decrease. assert!( - balanced_score[0] >= unbalanced_score[0] && - balanced_score[1] == unbalanced_score[1] && - balanced_score[2] <= unbalanced_score[2] + balanced_score.minimal_stake >= unbalanced_score.minimal_stake && + balanced_score.sum_stake == unbalanced_score.sum_stake && + balanced_score.sum_stake_squared <= unbalanced_score.sum_stake_squared ); }); } diff --git a/substrate/primitives/npos-elections/src/lib.rs b/substrate/primitives/npos-elections/src/lib.rs index 7b3b09a4c7..7bd1a4b7f6 100644 --- a/substrate/primitives/npos-elections/src/lib.rs +++ b/substrate/primitives/npos-elections/src/lib.rs @@ -74,11 +74,12 @@ #![cfg_attr(not(feature = "std"), no_std)] +use scale_info::TypeInfo; use sp_arithmetic::{traits::Zero, Normalizable, PerThing, Rational128, ThresholdOrd}; use sp_core::RuntimeDebug; use sp_std::{cell::RefCell, cmp::Ordering, collections::btree_map::BTreeMap, prelude::*, rc::Rc}; -use codec::{Decode, Encode}; +use codec::{Decode, Encode, MaxEncodedLen}; #[cfg(feature = "std")] use serde::{Deserialize, Serialize}; @@ -144,9 +145,86 @@ pub type VoteWeight = u64; /// A type in which performing operations on vote weights are safe. pub type ExtendedBalance = u128; -/// The score of an assignment. This can be computed from the support map via -/// [`EvaluateSupport::evaluate`]. -pub type ElectionScore = [ExtendedBalance; 3]; +/// The score of an election. This is the main measure of an election's quality. +/// +/// By definition, the order of significance in [`ElectionScore`] is: +/// +/// 1. `minimal_stake`. +/// 2. `sum_stake`. +/// 3. `sum_stake_squared`. +#[derive(Clone, Copy, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo, Debug, Default)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +pub struct ElectionScore { + /// The minimal winner, in terms of total backing stake. + /// + /// This parameter should be maximized. + pub minimal_stake: ExtendedBalance, + /// The sum of the total backing of all winners. + /// + /// This parameter should maximized + pub sum_stake: ExtendedBalance, + /// The sum squared of the total backing of all winners, aka. the variance. + /// + /// Ths parameter should be minimized. + pub sum_stake_squared: ExtendedBalance, +} + +impl ElectionScore { + /// Iterate over the inner items, first visiting the most significant one. + fn iter_by_significance(self) -> impl Iterator { + [self.minimal_stake, self.sum_stake, self.sum_stake_squared].into_iter() + } + + /// Compares two sets of election scores based on desirability, returning true if `self` is + /// strictly `threshold` better than `other`. In other words, each element of `self` must be + /// `self * threshold` better than `other`. + /// + /// Evaluation is done based on the order of significance of the fields of [`ElectionScore`]. + pub fn strict_threshold_better(self, other: Self, threshold: impl PerThing) -> bool { + match self + .iter_by_significance() + .zip(other.iter_by_significance()) + .map(|(this, that)| (this.ge(&that), this.tcmp(&that, threshold.mul_ceil(that)))) + .collect::>() + .as_slice() + { + // threshold better in the `score.minimal_stake`, accept. + [(x, Ordering::Greater), _, _] => { + debug_assert!(x); + true + }, + + // less than threshold better in `score.minimal_stake`, but more than threshold better + // in `score.sum_stake`. + [(true, Ordering::Equal), (_, Ordering::Greater), _] => true, + + // less than threshold better in `score.minimal_stake` and `score.sum_stake`, but more + // than threshold better in `score.sum_stake_squared`. + [(true, Ordering::Equal), (true, Ordering::Equal), (_, Ordering::Less)] => true, + + // anything else is not a good score. + _ => false, + } + } +} + +impl sp_std::cmp::Ord for ElectionScore { + fn cmp(&self, other: &Self) -> Ordering { + // we delegate this to the lexicographic cmp of slices`, and to incorporate that we want the + // third element to be minimized, we swap them. + [self.minimal_stake, self.sum_stake, other.sum_stake_squared].cmp(&[ + other.minimal_stake, + other.sum_stake, + self.sum_stake_squared, + ]) + } +} + +impl sp_std::cmp::PartialOrd for ElectionScore { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} /// A pointer to a candidate struct with interior mutability. pub type CandidatePtr = Rc>>; @@ -353,7 +431,7 @@ pub struct ElectionResult { /// /// This, at the current version, resembles the `Exposure` defined in the Staking pallet, yet they /// do not necessarily have to be the same. -#[derive(RuntimeDebug, Encode, Decode, Clone, Eq, PartialEq, scale_info::TypeInfo)] +#[derive(RuntimeDebug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] pub struct Support { /// Total support. @@ -418,49 +496,22 @@ pub trait EvaluateSupport { impl EvaluateSupport for Supports { fn evaluate(&self) -> ElectionScore { - let mut min_support = ExtendedBalance::max_value(); - let mut sum: ExtendedBalance = Zero::zero(); + let mut minimal_stake = ExtendedBalance::max_value(); + let mut sum_stake: ExtendedBalance = Zero::zero(); // NOTE: The third element might saturate but fine for now since this will run on-chain and // need to be fast. - let mut sum_squared: ExtendedBalance = Zero::zero(); + let mut sum_stake_squared: ExtendedBalance = Zero::zero(); + for (_, support) in self { - sum = sum.saturating_add(support.total); + sum_stake = sum_stake.saturating_add(support.total); let squared = support.total.saturating_mul(support.total); - sum_squared = sum_squared.saturating_add(squared); - if support.total < min_support { - min_support = support.total; + sum_stake_squared = sum_stake_squared.saturating_add(squared); + if support.total < minimal_stake { + minimal_stake = support.total; } } - [min_support, sum, sum_squared] - } -} -/// Compares two sets of election scores based on desirability and returns true if `this` is better -/// than `that`. -/// -/// Evaluation is done in a lexicographic manner, and if each element of `this` is `that * epsilon` -/// greater or less than `that`. -/// -/// Note that the third component should be minimized. -pub fn is_score_better(this: ElectionScore, that: ElectionScore, epsilon: P) -> bool { - match this - .iter() - .zip(that.iter()) - .map(|(thi, tha)| (thi.ge(&tha), thi.tcmp(&tha, epsilon.mul_ceil(*tha)))) - .collect::>() - .as_slice() - { - // epsilon better in the score[0], accept. - [(_, Ordering::Greater), _, _] => true, - - // less than epsilon better in score[0], but more than epsilon better in the second. - [(true, Ordering::Equal), (_, Ordering::Greater), _] => true, - - // less than epsilon better in score[0, 1], but more than epsilon better in the third - [(true, Ordering::Equal), (true, Ordering::Equal), (_, Ordering::Less)] => true, - - // anything else is not a good score. - _ => false, + ElectionScore { minimal_stake, sum_stake, sum_stake_squared } } } diff --git a/substrate/primitives/npos-elections/src/tests.rs b/substrate/primitives/npos-elections/src/tests.rs index c6748b29e9..b199fdd1af 100644 --- a/substrate/primitives/npos-elections/src/tests.rs +++ b/substrate/primitives/npos-elections/src/tests.rs @@ -18,9 +18,9 @@ //! Tests for npos-elections. use crate::{ - balancing, helpers::*, is_score_better, mock::*, seq_phragmen, seq_phragmen_core, setup_inputs, - to_support_map, Assignment, ElectionResult, ExtendedBalance, IndexAssignment, NposSolution, - StakedAssignment, Support, Voter, + balancing, helpers::*, mock::*, seq_phragmen, seq_phragmen_core, setup_inputs, to_support_map, + Assignment, ElectionResult, ExtendedBalance, IndexAssignment, NposSolution, StakedAssignment, + Support, Voter, }; use rand::{self, SeedableRng}; use sp_arithmetic::{PerU16, Perbill, Percent, Permill}; @@ -792,6 +792,21 @@ mod assignment_convert_normalize { mod score { use super::*; + use crate::ElectionScore; + use sp_arithmetic::PerThing; + + /// NOTE: in tests, we still use the legacy [u128; 3] since it is more compact. Each `u128` + /// corresponds to element at the respective field index of `ElectionScore`. + impl From<[ExtendedBalance; 3]> for ElectionScore { + fn from(t: [ExtendedBalance; 3]) -> Self { + Self { minimal_stake: t[0], sum_stake: t[1], sum_stake_squared: t[2] } + } + } + + fn is_score_better(this: [u128; 3], that: [u128; 3], p: impl PerThing) -> bool { + ElectionScore::from(this).strict_threshold_better(ElectionScore::from(that), p) + } + #[test] fn score_comparison_is_lexicographical_no_epsilon() { let epsilon = Perbill::zero(); @@ -883,6 +898,26 @@ mod score { false, ); } + + #[test] + fn ord_works() { + // equal only when all elements are equal + assert!(ElectionScore::from([10, 5, 15]) == ElectionScore::from([10, 5, 15])); + assert!(ElectionScore::from([10, 5, 15]) != ElectionScore::from([9, 5, 15])); + assert!(ElectionScore::from([10, 5, 15]) != ElectionScore::from([10, 5, 14])); + + // first element greater, rest don't matter + assert!(ElectionScore::from([10, 5, 15]) > ElectionScore::from([8, 5, 25])); + assert!(ElectionScore::from([10, 5, 15]) > ElectionScore::from([9, 20, 5])); + + // second element greater, rest don't matter + assert!(ElectionScore::from([10, 5, 15]) > ElectionScore::from([10, 4, 25])); + assert!(ElectionScore::from([10, 5, 15]) > ElectionScore::from([10, 4, 5])); + + // second element is less, rest don't matter. Note that this is swapped. + assert!(ElectionScore::from([10, 5, 15]) > ElectionScore::from([10, 5, 16])); + assert!(ElectionScore::from([10, 5, 15]) > ElectionScore::from([10, 5, 25])); + } } mod solution_type {