diff --git a/core/phragmen/src/lib.rs b/core/phragmen/src/lib.rs index a4287773b5aed..67ea6077adf58 100644 --- a/core/phragmen/src/lib.rs +++ b/core/phragmen/src/lib.rs @@ -140,16 +140,17 @@ pub type SupportMap = BTreeMap>; /// * `initial_candidates`: candidates list to be elected from. /// * `initial_voters`: voters list. /// * `stake_of`: something that can return the stake stake of a particular candidate or voter. -/// * `self_vote`. If true, then each candidate will automatically vote for themselves with the a -/// weight indicated by their stake. Note that when this is `true` candidates are filtered by -/// having at least some backed stake from themselves. +/// +/// This function does not strip out candidates who do not have any backing stake. It is the +/// responsibility of the caller to make sure only those candidates who have a sensible economic +/// value are passed in. From the perspective of this function, a candidate can easily be among the +/// winner with no backing stake. pub fn elect( candidate_count: usize, minimum_candidate_count: usize, initial_candidates: Vec, initial_voters: Vec<(AccountId, Vec)>, stake_of: FS, - self_vote: bool, ) -> Option> where AccountId: Default + Ord + Member, Balance: Default + Copy + SimpleArithmetic, @@ -169,36 +170,16 @@ pub fn elect( let num_voters = initial_candidates.len() + initial_voters.len(); let mut voters: Vec> = Vec::with_capacity(num_voters); - // collect candidates. self vote or filter might apply - let mut candidates = if self_vote { - // self vote. filter. - initial_candidates.into_iter().map(|who| { - let stake = stake_of(&who); - Candidate { who, approval_stake: to_votes(stake), ..Default::default() } - }) - .filter(|c| !c.approval_stake.is_zero()) + // Iterate once to create a cache of candidates indexes. This could be optimized by being + // provided by the call site. + let mut candidates = initial_candidates + .into_iter() .enumerate() - .map(|(i, c)| { - voters.push(Voter { - who: c.who.clone(), - edges: vec![Edge { who: c.who.clone(), candidate_index: i, ..Default::default() }], - budget: c.approval_stake, - load: Rational128::zero(), - }); - c_idx_cache.insert(c.who.clone(), i); - c + .map(|(idx, who)| { + c_idx_cache.insert(who.clone(), idx); + Candidate { who, ..Default::default() } }) - .collect::>>() - } else { - // no self vote. just collect. - initial_candidates.into_iter() - .enumerate() - .map(|(idx, who)| { - c_idx_cache.insert(who.clone(), idx); - Candidate { who, ..Default::default() } - }) - .collect::>>() - }; + .collect::>>(); // early return if we don't have enough candidates if candidates.len() < minimum_candidate_count { return None; } @@ -290,37 +271,33 @@ pub fn elect( for n in &mut voters { 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 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 { - // Full support. No need to calculate. - Perbill::accuracy().into() + if elected_candidates.iter().position(|(ref c, _)| *c == e.who).is_some() { + 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( + desired_scale, + e.load.n(), + n.load.n(), + ).unwrap_or(Bounded::max_value()) } else { - if e.load.d() == n.load.d() { - // return e.load / n.load. - let desired_scale: u128 = Perbill::accuracy().into(); - multiply_by_rational( - desired_scale, - e.load.n(), - n.load.n(), - ).unwrap_or(Bounded::max_value()) - } else { - // defensive only. Both edge and nominator loads are built from - // scores, hence MUST have the same denominator. - Zero::zero() - } + // defensive only. Both edge and nominator loads are built from + // scores, hence MUST have the same denominator. + Zero::zero() } - }; - // 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)); - } + } + }; + // 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)); } } @@ -368,7 +345,6 @@ pub fn build_support_map( elected_stashes: &Vec, assignments: &Vec<(AccountId, Vec>)>, stake_of: FS, - assume_self_vote: bool, ) -> SupportMap where AccountId: Default + Ord + Member, Balance: Default + Copy + SimpleArithmetic, @@ -380,11 +356,7 @@ pub fn build_support_map( let mut supports = >::new(); elected_stashes .iter() - .map(|e| (e, if assume_self_vote { to_votes(stake_of(e)) } else { Zero::zero() } )) - .for_each(|(e, s)| { - let item = Support { own: s, total: s, ..Default::default() }; - supports.insert(e.clone(), item); - }); + .for_each(|e| { supports.insert(e.clone(), Default::default()); }); // build support struct. for (n, assignment) in assignments.iter() { @@ -394,10 +366,20 @@ pub fn build_support_map( // 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)); + 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 = support.own.saturating_add(other_stake); + support.total = support.total.saturating_add(other_stake); + } else { + // This is a nomination from `n` to someone else. Increase `total` and add an entry + // inside `others`. + // 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)); + } } } } diff --git a/core/phragmen/src/mock.rs b/core/phragmen/src/mock.rs index ee4e1eb4bbb32..a0ab23db34040 100644 --- a/core/phragmen/src/mock.rs +++ b/core/phragmen/src/mock.rs @@ -75,13 +75,16 @@ pub(crate) struct _PhragmenResult { pub assignments: Vec<(A, Vec<_PhragmenAssignment>)> } +pub(crate) fn auto_generate_self_voters(candidates: &[A]) -> Vec<(A, Vec)> { + candidates.iter().map(|c| (c.clone(), vec![c.clone()])).collect() +} + pub(crate) fn elect_float( candidate_count: usize, minimum_candidate_count: usize, initial_candidates: Vec, initial_voters: Vec<(A, Vec)>, stake_of: FS, - self_vote: bool, ) -> Option<_PhragmenResult> where A: Default + Ord + Member + Copy, for<'r> FS: Fn(&'r A) -> Balance, @@ -92,36 +95,14 @@ pub(crate) fn elect_float( let num_voters = initial_candidates.len() + initial_voters.len(); let mut voters: Vec<_Voter> = Vec::with_capacity(num_voters); - let mut candidates = if self_vote { - initial_candidates.into_iter().map(|who| { - let stake = stake_of(&who) as f64; - _Candidate { who, approval_stake: stake, ..Default::default() } - }) - .filter(|c| c.approval_stake != 0f64) + let mut candidates = initial_candidates + .into_iter() .enumerate() - .map(|(i, c)| { - let who = c.who; - voters.push(_Voter { - who: who.clone(), - edges: vec![ - _Edge { who: who.clone(), candidate_index: i, ..Default::default() } - ], - budget: c.approval_stake, - load: 0f64, - }); - c_idx_cache.insert(c.who.clone(), i); - c + .map(|(idx, who)| { + c_idx_cache.insert(who.clone(), idx); + _Candidate { who, ..Default::default() } }) - .collect::>>() - } else { - initial_candidates.into_iter() - .enumerate() - .map(|(idx, who)| { - c_idx_cache.insert(who.clone(), idx); - _Candidate { who, ..Default::default() } - }) - .collect::>>() - }; + .collect::>>(); if candidates.len() < minimum_candidate_count { return None; @@ -359,7 +340,6 @@ pub(crate) fn run_and_compare( stake_of: Box Balance>, to_elect: usize, min_to_elect: usize, - self_vote: bool, ) { // run fixed point code. let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote>( @@ -368,7 +348,6 @@ pub(crate) fn run_and_compare( candidates.clone(), voters.clone(), &stake_of, - self_vote, ).unwrap(); // run float poc code. @@ -378,7 +357,6 @@ pub(crate) fn run_and_compare( candidates, voters, &stake_of, - self_vote, ).unwrap(); assert_eq!(winners, truth_value.winners); diff --git a/core/phragmen/src/tests.rs b/core/phragmen/src/tests.rs index d3c5b1168cc4e..aaeac27c1411c 100644 --- a/core/phragmen/src/tests.rs +++ b/core/phragmen/src/tests.rs @@ -32,7 +32,7 @@ fn float_phragmen_poc_works() { (30, vec![2, 3]), ]; let stake_of = create_stake_of(&[(10, 10), (20, 20), (30, 30), (1, 0), (2, 0), (3, 0)]); - let mut phragmen_result = elect_float(2, 2, candidates, voters, &stake_of, false).unwrap(); + let mut phragmen_result = elect_float(2, 2, candidates, voters, &stake_of).unwrap(); let winners = phragmen_result.clone().winners; let assignments = phragmen_result.clone().assignments; @@ -84,7 +84,6 @@ fn phragmen_poc_works() { candidates, voters, create_stake_of(&[(10, 10), (20, 20), (30, 30)]), - false, ).unwrap(); assert_eq_uvec!(winners, vec![(2, 40), (3, 50)]); @@ -114,7 +113,7 @@ fn phragmen_poc_2_works() { (4, 500), ]); - run_and_compare(candidates, voters, stake_of, 2, 2, true); + run_and_compare(candidates, voters, stake_of, 2, 2); } #[test] @@ -132,7 +131,7 @@ fn phragmen_poc_3_works() { (4, 1000), ]); - run_and_compare(candidates, voters, stake_of, 2, 2, true); + run_and_compare(candidates, voters, stake_of, 2, 2); } #[test] @@ -151,24 +150,24 @@ fn phragmen_accuracy_on_large_scale_only_validators() { let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote>( 2, 2, - candidates, - vec![], + candidates.clone(), + auto_generate_self_voters(&candidates), stake_of, - true, ).unwrap(); assert_eq_uvec!(winners, vec![(1, 18446744073709551614u128), (5, 18446744073709551613u128)]); - assert_eq!(assignments.len(), 0); + assert_eq!(assignments.len(), 2); check_assignments(assignments); } #[test] fn phragmen_accuracy_on_large_scale_validators_and_nominators() { let candidates = vec![1, 2, 3, 4, 5]; - let voters = vec![ + let mut voters = vec![ (13, vec![1, 3, 5]), (14, vec![2, 4]), ]; + voters.extend(auto_generate_self_voters(&candidates)); let stake_of = create_stake_of(&[ (1, (u64::max_value() - 1).into()), (2, (u64::max_value() - 4).into()), @@ -185,13 +184,17 @@ fn phragmen_accuracy_on_large_scale_validators_and_nominators() { 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())])] + vec![ + (13, vec![(1, Perbill::one())]), + (14, vec![(2, Perbill::one())]), + (1, vec![(1, Perbill::one())]), + (2, vec![(2, Perbill::one())]), + ] ); check_assignments(assignments); } @@ -199,7 +202,7 @@ fn phragmen_accuracy_on_large_scale_validators_and_nominators() { #[test] fn phragmen_accuracy_on_small_scale_self_vote() { let candidates = vec![40, 10, 20, 30]; - let voters = vec![]; + let voters = auto_generate_self_voters(&candidates); let stake_of = create_stake_of(&[ (40, 0), (10, 1), @@ -213,7 +216,6 @@ fn phragmen_accuracy_on_small_scale_self_vote() { candidates, voters, stake_of, - true, ).unwrap(); assert_eq_uvec!(winners, vec![(20, 2), (10, 1), (30, 1)]); @@ -245,7 +247,6 @@ fn phragmen_accuracy_on_small_scale_no_self_vote() { candidates, voters, stake_of, - false, ).unwrap(); assert_eq_uvec!(winners, vec![(20, 2), (10, 1), (30, 1)]); @@ -254,9 +255,10 @@ fn phragmen_accuracy_on_small_scale_no_self_vote() { #[test] fn phragmen_large_scale_test() { let candidates = vec![2, 4, 6, 8, 10, 12, 14, 16 ,18, 20, 22, 24]; - let voters = vec![ + let mut voters = vec![ (50, vec![2, 4, 6, 8, 10, 12, 14, 16 ,18, 20, 22, 24]), ]; + voters.extend(auto_generate_self_voters(&candidates)); let stake_of = create_stake_of(&[ (2, 1), (4, 100), @@ -279,7 +281,6 @@ fn phragmen_large_scale_test() { candidates, voters, stake_of, - true, ).unwrap(); assert_eq_uvec!(winners, vec![(24, 1490000000000200000u128), (22, 1490000000000100000u128)]); @@ -292,7 +293,8 @@ fn phragmen_large_scale_test_2() { let c_budget: u64 = 4_000_000; let candidates = vec![2, 4]; - let voters = vec![(50, vec![2, 4])]; + let mut voters = vec![(50, vec![2, 4])]; + voters.extend(auto_generate_self_voters(&candidates)); let stake_of = create_stake_of(&[ (2, c_budget.into()), @@ -306,13 +308,16 @@ fn phragmen_large_scale_test_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))])], + vec![ + (50, vec![(2, Perbill::from_parts(500000001)), (4, Perbill::from_parts(499999999))]), + (2, vec![(2, Perbill::one())]), + (4, vec![(4, Perbill::one())]), + ], ); check_assignments(assignments); } @@ -347,5 +352,5 @@ fn phragmen_linear_equalize() { (130, 1000), ]); - run_and_compare(candidates, voters, stake_of, 2, 2, true); + run_and_compare(candidates, voters, stake_of, 2, 2); } diff --git a/srml/elections-phragmen/src/lib.rs b/srml/elections-phragmen/src/lib.rs index 9b277815f385a..1ddd890b8e02f 100644 --- a/srml/elections-phragmen/src/lib.rs +++ b/srml/elections-phragmen/src/lib.rs @@ -542,7 +542,6 @@ impl Module { candidates, voters_and_votes, Self::locked_stake_of, - false, ); let mut to_release_bond: Vec = Vec::with_capacity(desired_seats); @@ -563,7 +562,6 @@ impl Module { &new_set, &phragmen_result.assignments, Self::locked_stake_of, - false, ); let to_balance = |e: ExtendedBalance| diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 2b58be1f68d60..e5613a57be69e 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -1265,13 +1265,21 @@ impl Module { /// /// Returns the new `SlotStake` value and a set of newly selected _stash_ IDs. fn select_validators() -> (BalanceOf, Option>) { + let mut all_nominators: Vec<(T::AccountId, Vec)> = Vec::new(); + let all_validator_candidates_iter = >::enumerate(); + let all_validators = all_validator_candidates_iter.map(|(who, _pref)| { + let self_vote = (who.clone(), vec![who.clone()]); + all_nominators.push(self_vote); + who + }).collect::>(); + all_nominators.extend(>::enumerate()); + let maybe_phragmen_result = elect::<_, _, _, T::CurrencyToVote>( Self::validator_count() as usize, Self::minimum_validator_count().max(1) as usize, - >::enumerate().map(|(who, _)| who).collect::>(), - >::enumerate().collect(), + all_validators, + all_nominators, Self::slashable_balance_of, - true, ); if let Some(phragmen_result) = maybe_phragmen_result { @@ -1289,7 +1297,6 @@ impl Module { &elected_stashes, &assignments, Self::slashable_balance_of, - true, ); if cfg!(feature = "equalize") { @@ -1300,6 +1307,13 @@ impl Module { let mut staked_assignment : Vec> = Vec::with_capacity(assignment.len()); + + // If this is a self vote, then we don't need to equalise it at all. While the + // staking system does not allow nomination and validation at the same time, + // this must always be 100% support. + if assignment.len() == 1 && assignment[0].0 == *n { + continue; + } for (c, per_thing) in assignment.iter() { let nominator_stake = to_votes(Self::slashable_balance_of(n)); let other_stake = *per_thing * nominator_stake; diff --git a/srml/treasury/src/lib.rs b/srml/treasury/src/lib.rs index 504e374430d45..e07efc396dda3 100644 --- a/srml/treasury/src/lib.rs +++ b/srml/treasury/src/lib.rs @@ -579,7 +579,7 @@ mod tests { >::on_finalize(2); assert_eq!(Treasury::pot(), 100); // Pot hasn't changed - Balances::deposit_into_existing(&Treasury::account_id(), 100); + let _ = Balances::deposit_into_existing(&Treasury::account_id(), 100).unwrap(); >::on_finalize(4); assert_eq!(Balances::free_balance(&3), 150); // Fund has been spent assert_eq!(Treasury::pot(), 25); // Pot has finally changed