diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index a3cfed9e2f260..a957b6ef33a79 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -1715,6 +1715,101 @@ fn bond_with_little_staked_value_bounded() { }); } +#[test] +fn bond_with_duplicate_vote_should_be_ignored_by_npos_election() { + ExtBuilder::default() + .validator_count(2) + .nominate(false) + .minimum_validator_count(1) + .build() + .execute_with(|| { + // disable the nominator + assert_ok!(Staking::chill(Origin::signed(100))); + // make stakes equal. + assert_ok!(Staking::bond_extra(Origin::signed(31), 999)); + + assert_eq!( + >::iter() + .map(|(v, _)| (v, Staking::ledger(v - 1).unwrap().total)) + .collect::>(), + vec![(31, 1000), (21, 1000), (11, 1000)], + ); + assert_eq!(>::iter().map(|(n, _)| n).collect::>(), vec![]); + + // give the man some money + let initial_balance = 1000; + for i in [1, 2, 3, 4,].iter() { + let _ = Balances::make_free_balance_be(i, initial_balance); + } + + assert_ok!(Staking::bond(Origin::signed(1), 2, 1000, RewardDestination::Controller)); + assert_ok!(Staking::nominate(Origin::signed(2), vec![11, 11, 11, 21, 31,])); + + assert_ok!(Staking::bond(Origin::signed(3), 4, 1000, RewardDestination::Controller)); + assert_ok!(Staking::nominate(Origin::signed(4), vec![21, 31])); + + // winners should be 21 and 31. Otherwise this election is taking duplicates into account. + + let sp_npos_elections::ElectionResult { + winners, + assignments, + } = Staking::do_phragmen::().unwrap(); + let winners = sp_npos_elections::to_without_backing(winners); + + assert_eq!(winners, vec![31, 21]); + // only distribution to 21 and 31. + assert_eq!(assignments.iter().find(|a| a.who == 1).unwrap().distribution.len(), 2); + }); +} + +#[test] +fn bond_with_duplicate_vote_should_be_ignored_by_npos_election_elected() { + // same as above but ensures that even when the duple is being elected, everything is sane. + ExtBuilder::default() + .validator_count(2) + .nominate(false) + .minimum_validator_count(1) + .build() + .execute_with(|| { + // disable the nominator + assert_ok!(Staking::chill(Origin::signed(100))); + // make stakes equal. + assert_ok!(Staking::bond_extra(Origin::signed(31), 99)); + + assert_eq!( + >::iter() + .map(|(v, _)| (v, Staking::ledger(v - 1).unwrap().total)) + .collect::>(), + vec![(31, 100), (21, 1000), (11, 1000)], + ); + assert_eq!(>::iter().map(|(n, _)| n).collect::>(), vec![]); + + // give the man some money + let initial_balance = 1000; + for i in [1, 2, 3, 4,].iter() { + let _ = Balances::make_free_balance_be(i, initial_balance); + } + + assert_ok!(Staking::bond(Origin::signed(1), 2, 1000, RewardDestination::Controller)); + assert_ok!(Staking::nominate(Origin::signed(2), vec![11, 11, 11, 21, 31,])); + + assert_ok!(Staking::bond(Origin::signed(3), 4, 1000, RewardDestination::Controller)); + assert_ok!(Staking::nominate(Origin::signed(4), vec![21, 31])); + + // winners should be 21 and 31. Otherwise this election is taking duplicates into account. + + let sp_npos_elections::ElectionResult { + winners, + assignments, + } = Staking::do_phragmen::().unwrap(); + + let winners = sp_npos_elections::to_without_backing(winners); + assert_eq!(winners, vec![21, 11]); + // only distribution to 21 and 31. + assert_eq!(assignments.iter().find(|a| a.who == 1).unwrap().distribution.len(), 2); + }); +} + #[test] fn new_era_elects_correct_number_of_validators() { ExtBuilder::default() diff --git a/primitives/npos-elections/src/lib.rs b/primitives/npos-elections/src/lib.rs index 592ed3b717350..76c011902126e 100644 --- a/primitives/npos-elections/src/lib.rs +++ b/primitives/npos-elections/src/lib.rs @@ -371,6 +371,10 @@ pub fn seq_phragmen( voters.extend(initial_voters.into_iter().map(|(who, voter_stake, votes)| { let mut edges: Vec> = Vec::with_capacity(votes.len()); for v in votes { + if edges.iter().any(|e| e.who == v) { + // duplicate edge. + continue; + } if let Some(idx) = c_idx_cache.get(&v) { // This candidate is valid + already cached. candidates[*idx].approval_stake = candidates[*idx].approval_stake diff --git a/primitives/npos-elections/src/tests.rs b/primitives/npos-elections/src/tests.rs index 80c742117d979..c630f0ae35984 100644 --- a/primitives/npos-elections/src/tests.rs +++ b/primitives/npos-elections/src/tests.rs @@ -588,6 +588,66 @@ fn self_votes_should_be_kept() { ); } +#[test] +fn duplicate_target_is_ignored() { + let candidates = vec![1, 2, 3]; + let voters = vec![ + (10, 100, vec![1, 1, 2, 3]), + (20, 100, vec![2, 3]), + (30, 50, vec![1, 1, 2]), + ]; + + let ElectionResult { winners, assignments } = seq_phragmen::<_, Perbill>( + 2, + 2, + candidates, + voters, + ).unwrap(); + let winners = to_without_backing(winners); + + assert_eq!(winners, vec![(2), (3)]); + assert_eq!( + assignments + .into_iter() + .map(|x| (x.who, x.distribution.into_iter().map(|(w, _)| w).collect::>())) + .collect::>(), + vec![ + (10, vec![2, 3]), + (20, vec![2, 3]), + (30, vec![2]), + ], + ); +} + +#[test] +fn duplicate_target_is_ignored_when_winner() { + let candidates = vec![1, 2, 3]; + let voters = vec![ + (10, 100, vec![1, 1, 2, 3]), + (20, 100, vec![1, 2]), + ]; + + let ElectionResult { winners, assignments } = seq_phragmen::<_, Perbill>( + 2, + 2, + candidates, + voters, + ).unwrap(); + let winners = to_without_backing(winners); + + assert_eq!(winners, vec![1, 2]); + assert_eq!( + assignments + .into_iter() + .map(|x| (x.who, x.distribution.into_iter().map(|(w, _)| w).collect::>())) + .collect::>(), + vec![ + (10, vec![1, 2]), + (20, vec![1, 2]), + ], + ); +} + mod assignment_convert_normalize { use super::*; #[test]