From c9fd0c964a385ae20995fd185e70569a73aaee6b Mon Sep 17 00:00:00 2001
From: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Date: Tue, 21 Jul 2020 17:33:33 +0200
Subject: [PATCH] Properly filter out duplicate voters in elections. (#6693)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Prevent duplicate voter
* Update primitives/npos-elections/src/lib.rs
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
---
substrate/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/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs
index a3cfed9e2f2..a957b6ef33a 100644
--- a/substrate/frame/staking/src/tests.rs
+++ b/substrate/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!(
+ <Validators<Test>>::iter()
+ .map(|(v, _)| (v, Staking::ledger(v - 1).unwrap().total))
+ .collect::<Vec<_>>(),
+ vec![(31, 1000), (21, 1000), (11, 1000)],
+ );
+ assert_eq!(<Nominators<Test>>::iter().map(|(n, _)| n).collect::<Vec<_>>(), 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::<Perbill>().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!(
+ <Validators<Test>>::iter()
+ .map(|(v, _)| (v, Staking::ledger(v - 1).unwrap().total))
+ .collect::<Vec<_>>(),
+ vec![(31, 100), (21, 1000), (11, 1000)],
+ );
+ assert_eq!(<Nominators<Test>>::iter().map(|(n, _)| n).collect::<Vec<_>>(), 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::<Perbill>().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/substrate/primitives/npos-elections/src/lib.rs b/substrate/primitives/npos-elections/src/lib.rs
index b3eb3ed6cc7..9ac058f8c3e 100644
--- a/substrate/primitives/npos-elections/src/lib.rs
+++ b/substrate/primitives/npos-elections/src/lib.rs
@@ -371,6 +371,10 @@ pub fn seq_phragmen<AccountId, R>(
voters.extend(initial_voters.into_iter().map(|(who, voter_stake, votes)| {
let mut edges: Vec<Edge<AccountId>> = 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/substrate/primitives/npos-elections/src/tests.rs b/substrate/primitives/npos-elections/src/tests.rs
index 80c742117d9..c630f0ae359 100644
--- a/substrate/primitives/npos-elections/src/tests.rs
+++ b/substrate/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::<Vec<_>>()))
+ .collect::<Vec<_>>(),
+ 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::<Vec<_>>()))
+ .collect::<Vec<_>>(),
+ vec![
+ (10, vec![1, 2]),
+ (20, vec![1, 2]),
+ ],
+ );
+}
+
mod assignment_convert_normalize {
use super::*;
#[test]
--
GitLab