Properly filter out duplicate voters in elections. (#6693)

* 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

@@ -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()

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

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]