Do not include voters that have zero voter weight in the election snapshot

frame/election-provider-multi-phase/src/benchmarking.rs

@@ -169,10 +169,12 @@ fn set_up_data_provider<T: Config>(v: u32, t: u32) {
 	let mut targets = (0..t)
 		.map(|i| {
 			let target = frame_benchmarking::account::<T::AccountId>("Target", i, SEED);
+
 			T::DataProvider::add_target(target.clone());
 			target
 		})
 		.collect::<Vec<_>>();
+
 	// we should always have enough voters to fill.
 	assert!(
 		targets.len() > <T::DataProvider as ElectionDataProvider>::MaxVotesPerVoter::get() as usize
@@ -276,7 +278,7 @@ frame_benchmarking::benchmarks! {
 		<MultiPhase::<T>>::create_snapshot_internal(targets, voters, desired_targets)
 	} verify {
 		assert!(<MultiPhase<T>>::snapshot().is_some());
-		assert_eq!(<MultiPhase<T>>::snapshot_metadata().ok_or("metadata missing")?.voters, v + t);
+		assert_eq!(<MultiPhase<T>>::snapshot_metadata().ok_or("metadata missing")?.voters, v);
 		assert_eq!(<MultiPhase<T>>::snapshot_metadata().ok_or("metadata missing")?.targets, t);
 	}
 
@@ -450,7 +452,7 @@ frame_benchmarking::benchmarks! {
 		<MultiPhase::<T>>::create_snapshot().map_err(|_| "could not create snapshot")?;
 	} verify {
 		assert!(<MultiPhase<T>>::snapshot().is_some());
-		assert_eq!(<MultiPhase<T>>::snapshot_metadata().ok_or("snapshot missing")?.voters, v + t);
+		assert_eq!(<MultiPhase<T>>::snapshot_metadata().ok_or("snapshot missing")?.voters, v);
 		assert_eq!(<MultiPhase<T>>::snapshot_metadata().ok_or("snapshot missing")?.targets, t);
 	}
 

frame/election-provider-multi-phase/src/mock.rs

@@ -505,12 +505,6 @@ impl ElectionDataProvider for StakingMock {
 		let mut current = Targets::get();
 		current.push(target);
 		Targets::set(current);
-
-		// to be on-par with staking, we add a self vote as well. the stake is really not that
-		// important.
-		let mut current = Voters::get();
-		current.push((target, ExistentialDeposit::get() as u64, bounded_vec![target]));
-		Voters::set(current);
 	}
 }
 

frame/staking/src/lib.rs

@@ -159,7 +159,7 @@
 //! ```
 //! use pallet_staking::{self as staking};
 //!
-//! #[frame_support::pallet]
+//! #[frame_support::pallet(dev_mode)]
 //! pub mod pallet {
 //! 	use super::*;
 //! 	use frame_support::pallet_prelude::*;

frame/staking/src/pallet/impls.rs

@@ -788,8 +788,14 @@ impl<T: Config> Pallet<T> {
 				None => break,
 			};
 
+			let voter_weight = weight_of(&voter);
+			// if voter weight is zero, do not consider this voter for the snapshot.
+			if voter_weight.is_zero() {
+				log!(debug, "voter's active balance is 0. skip this voter.");
+				continue
+			}
+
 			if let Some(Nominations { targets, .. }) = <Nominators<T>>::get(&voter) {
-				let voter_weight = weight_of(&voter);
 				if !targets.is_empty() {
 					all_voters.push((voter.clone(), voter_weight, targets));
 					nominators_taken.saturating_inc();
@@ -802,7 +808,7 @@ impl<T: Config> Pallet<T> {
 				// if this voter is a validator:
 				let self_vote = (
 					voter.clone(),
-					weight_of(&voter),
+					voter_weight,
 					vec![voter.clone()]
 						.try_into()
 						.expect("`MaxVotesPerVoter` must be greater than or equal to 1"),
@@ -829,7 +835,7 @@ impl<T: Config> Pallet<T> {
 		Self::register_weight(T::WeightInfo::get_npos_voters(validators_taken, nominators_taken));
 
 		let min_active_stake: T::CurrencyBalance =
-			if all_voters.len() == 0 { 0u64.into() } else { min_active_stake.into() };
+			if all_voters.is_empty() { Zero::zero() } else { min_active_stake.into() };
 
 		MinimumActiveStake::<T>::put(min_active_stake);
 

frame/staking/src/tests.rs

@@ -4519,11 +4519,75 @@ mod election_data_provider {
 	}
 
 	#[test]
-	fn set_minimum_active_stake_zero_correct() {
+	fn set_minimum_active_stake_lower_bond_works() {
+		// if there are no voters, minimum active stake is zero (should not happen).
 		ExtBuilder::default().has_stakers(false).build_and_execute(|| {
+			assert_eq!(<Test as Config>::VoterList::count(), 0);
 			assert_ok!(<Staking as ElectionDataProvider>::electing_voters(None));
 			assert_eq!(MinimumActiveStake::<Test>::get(), 0);
 		});
+
+		// lower non-zero active stake below `MinNominatorBond` is the minimum active stake if
+		// it is selected as part of the npos voters.
+		ExtBuilder::default().has_stakers(true).nominate(true).build_and_execute(|| {
+			assert_eq!(MinNominatorBond::<Test>::get(), 1);
+			assert_eq!(<Test as Config>::VoterList::count(), 4);
+
+			assert_ok!(Staking::bond(RuntimeOrigin::signed(4), 5, Default::default(),));
+			assert_ok!(Staking::nominate(RuntimeOrigin::signed(4), vec![1]));
+			assert_eq!(<Test as Config>::VoterList::count(), 5);
+
+			let voters_before = <Staking as ElectionDataProvider>::electing_voters(None).unwrap();
+			assert_eq!(MinimumActiveStake::<Test>::get(), 5);
+
+			// update minimum nominator bond.
+			MinNominatorBond::<Test>::set(10);
+			assert_eq!(MinNominatorBond::<Test>::get(), 10);
+			// voter list still considers nominator 4 for voting, even though its active stake is
+			// lower than `MinNominatorBond`.
+			assert_eq!(<Test as Config>::VoterList::count(), 5);
+
+			let voters = <Staking as ElectionDataProvider>::electing_voters(None).unwrap();
+			assert_eq!(voters_before, voters);
+
+			// minimum active stake is lower than `MinNominatorBond`.
+			assert_eq!(MinimumActiveStake::<Test>::get(), 5);
+		});
+	}
+
+	#[test]
+	fn set_minimum_active_bond_corrupt_state() {
+		ExtBuilder::default()
+			.has_stakers(true)
+			.nominate(true)
+			.add_staker(61, 61, 2_000, StakerStatus::<AccountId>::Nominator(vec![21]))
+			.build_and_execute(|| {
+				assert_eq!(Staking::weight_of(&101), 500);
+				let voters = <Staking as ElectionDataProvider>::electing_voters(None).unwrap();
+				assert_eq!(voters.len(), 5);
+				assert_eq!(MinimumActiveStake::<Test>::get(), 500);
+
+				assert_ok!(Staking::unbond(RuntimeOrigin::signed(101), 200));
+				start_active_era(10);
+				assert_ok!(Staking::unbond(RuntimeOrigin::signed(101), 100));
+				start_active_era(20);
+
+				// corrupt ledger state by lowering max unlocking chunks bounds.
+				MaxUnlockingChunks::set(1);
+
+				let voters = <Staking as ElectionDataProvider>::electing_voters(None).unwrap();
+				// number of returned voters decreases since ledger entry of stash 101 is now
+				// corrupt.
+				assert_eq!(voters.len(), 4);
+				// minimum active stake does not take into consideration the corrupt entry.
+				assert_eq!(MinimumActiveStake::<Test>::get(), 2_000);
+
+				// voter weight of corrupted ledger entry is 0.
+				assert_eq!(Staking::weight_of(&101), 0);
+
+				// reset max unlocking chunks for try_state to pass.
+				MaxUnlockingChunks::set(32);
+			})
 	}
 
 	#[test]