From 78caff04461d5220d70fdb31a9b2db42e259f034 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 20 Jul 2020 20:44:26 +0200 Subject: [PATCH 1/2] Prevent duplicate voter --- frame/staking/src/tests.rs | 95 ++++++++++++++++++++++++++ primitives/npos-elections/src/lib.rs | 4 ++ primitives/npos-elections/src/tests.rs | 60 ++++++++++++++++ 3 files changed, 159 insertions(+) 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..a8299c14dcc07 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().position(|e| e.who == v).is_some() { + // 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] From d5497d587332deff9e92a3641ea817b5a59a5b74 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Mon, 20 Jul 2020 21:00:26 +0200 Subject: [PATCH 2/2] Update primitives/npos-elections/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- primitives/npos-elections/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/npos-elections/src/lib.rs b/primitives/npos-elections/src/lib.rs index a8299c14dcc07..76c011902126e 100644 --- a/primitives/npos-elections/src/lib.rs +++ b/primitives/npos-elections/src/lib.rs @@ -371,7 +371,7 @@ 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().position(|e| e.who == v).is_some() { + if edges.iter().any(|e| e.who == v) { // duplicate edge. continue; }