Adds max backers per winner bounds check in the phragmen implementati…

…on (#6482)

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

@@ -153,7 +153,8 @@ pub fn trim_helpers() -> TrimHelpers {
 	let desired_targets = crate::DesiredTargets::<Runtime>::get().unwrap();
 
 	let ElectionResult::<_, SolutionAccuracyOf<Runtime>> { mut assignments, .. } =
-		seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None).unwrap();
+		seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None, None)
+			.unwrap();
 
 	// sort by decreasing order of stake
 	assignments.sort_by_key(|assignment| {
@@ -180,7 +181,8 @@ pub fn raw_solution() -> RawSolution<SolutionOf<Runtime>> {
 	let desired_targets = crate::DesiredTargets::<Runtime>::get().unwrap();
 
 	let ElectionResult::<_, SolutionAccuracyOf<Runtime>> { winners: _, assignments } =
-		seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None).unwrap();
+		seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None, None)
+			.unwrap();
 
 	// closures
 	let cache = helpers::generate_voter_cache::<Runtime>(&voters);
@@ -308,7 +310,8 @@ parameter_types! {
 pub struct OnChainSeqPhragmen;
 impl onchain::Config for OnChainSeqPhragmen {
 	type System = Runtime;
-	type Solver = SequentialPhragmen<AccountId, SolutionAccuracyOf<Runtime>, Balancing>;
+	type Solver =
+		SequentialPhragmen<AccountId, SolutionAccuracyOf<Runtime>, MaxBackersPerWinner, Balancing>;
 	type DataProvider = StakingMock;
 	type WeightInfo = ();
 	type MaxWinnersPerPage = MaxWinners;
@@ -420,7 +423,8 @@ impl crate::Config for Runtime {
 	type MaxWinners = MaxWinners;
 	type MaxBackersPerWinner = MaxBackersPerWinner;
 	type MinerConfig = Self;
-	type Solver = SequentialPhragmen<AccountId, SolutionAccuracyOf<Runtime>, Balancing>;
+	type Solver =
+		SequentialPhragmen<AccountId, SolutionAccuracyOf<Runtime>, MaxBackersPerWinner, Balancing>;
 	type ElectionBounds = ElectionsBounds;
 }
 
@@ -607,6 +611,10 @@ impl ExtBuilder {
 		<SignedMaxWeight>::set(weight);
 		self
 	}
+	pub fn max_backers_per_winner(self, max: u32) -> Self {
+		<MaxBackersPerWinner>::set(max);
+		self
+	}
 	pub fn build(self) -> sp_io::TestExternalities {
 		sp_tracing::try_init_simple();
 		let mut storage =

substrate/frame/election-provider-multi-phase/src/unsigned.rs

@@ -430,7 +430,7 @@ pub trait MinerConfig {
 	/// The maximum number of winners that can be elected in the single page supported by this
 	/// pallet.
 	type MaxWinners: Get<u32>;
-	/// The maximum number of backers per winner in the last solution.
+	/// The maximum number of backers per winner in a solution.
 	type MaxBackersPerWinner: Get<u32>;
 	/// Something that can compute the weight of a solution.
 	///
@@ -1865,6 +1865,99 @@ mod tests {
 		})
 	}
 
+	#[test]
+	fn mine_solution_always_respects_max_backers_per_winner() {
+		use crate::mock::MaxBackersPerWinner;
+		use frame_election_provider_support::BoundedSupport;
+
+		let targets = vec![10, 20, 30, 40];
+		let voters = vec![
+			(1, 10, bounded_vec![10, 20, 30]),
+			(2, 10, bounded_vec![10, 20, 30]),
+			(3, 10, bounded_vec![10, 20, 30]),
+			(4, 10, bounded_vec![10, 20, 30]),
+			(5, 10, bounded_vec![10, 20, 40]),
+		];
+		let snapshot = RoundSnapshot { voters: voters.clone(), targets: targets.clone() };
+		let (round, desired_targets) = (1, 3);
+
+		let expected_score_unbounded =
+			ElectionScore { minimal_stake: 12, sum_stake: 50, sum_stake_squared: 874 };
+		let expected_score_bounded =
+			ElectionScore { minimal_stake: 2, sum_stake: 10, sum_stake_squared: 44 };
+
+		// solution without max_backers_per_winner set will be higher than the score when bounds
+		// are set, confirming the trimming when using the same snapshot state.
+		assert!(expected_score_unbounded > expected_score_bounded);
+
+		// election with unbounded max backers per winnner.
+		ExtBuilder::default().max_backers_per_winner(u32::MAX).build_and_execute(|| {
+			assert_eq!(MaxBackersPerWinner::get(), u32::MAX);
+
+			let solution = Miner::<Runtime>::mine_solution_with_snapshot::<
+				<Runtime as Config>::Solver,
+			>(voters.clone(), targets.clone(), desired_targets)
+			.unwrap()
+			.0;
+
+			let ready_solution = Miner::<Runtime>::feasibility_check(
+				RawSolution { solution, score: expected_score_unbounded, round },
+				Default::default(),
+				desired_targets,
+				snapshot.clone(),
+				round,
+				Default::default(),
+			)
+			.unwrap();
+
+			assert_eq!(
+				ready_solution.supports.into_iter().collect::<Vec<_>>(),
+				vec![
+					(
+						10,
+						BoundedSupport { total: 21, voters: bounded_vec![(1, 10), (4, 8), (5, 3)] }
+					),
+					(20, BoundedSupport { total: 17, voters: bounded_vec![(2, 10), (5, 7)] }),
+					(30, BoundedSupport { total: 12, voters: bounded_vec![(3, 10), (4, 2)] }),
+				]
+			);
+		});
+
+		// election with max 1 backer per winnner.
+		ExtBuilder::default().max_backers_per_winner(1).build_and_execute(|| {
+			assert_eq!(MaxBackersPerWinner::get(), 1);
+
+			let solution = Miner::<Runtime>::mine_solution_with_snapshot::<
+				<Runtime as Config>::Solver,
+			>(voters, targets, desired_targets)
+			.unwrap()
+			.0;
+
+			let ready_solution = Miner::<Runtime>::feasibility_check(
+				RawSolution { solution, score: expected_score_bounded, round },
+				Default::default(),
+				desired_targets,
+				snapshot,
+				round,
+				Default::default(),
+			)
+			.unwrap();
+
+			for (_, supports) in ready_solution.supports.iter() {
+				assert!((supports.voters.len() as u32) <= MaxBackersPerWinner::get());
+			}
+
+			assert_eq!(
+				ready_solution.supports.into_iter().collect::<Vec<_>>(),
+				vec![
+					(10, BoundedSupport { total: 6, voters: bounded_vec![(1, 6)] }),
+					(20, BoundedSupport { total: 2, voters: bounded_vec![(1, 2)] }),
+					(30, BoundedSupport { total: 2, voters: bounded_vec![(1, 2)] }),
+				]
+			);
+		});
+	}
+
 	#[test]
 	fn trim_assignments_length_does_not_modify_when_short_enough() {
 		ExtBuilder::default().build_and_execute(|| {

substrate/frame/election-provider-support/src/lib.rs

@@ -670,12 +670,16 @@ pub trait NposSolver {
 
 /// A wrapper for [`sp_npos_elections::seq_phragmen`] that implements [`NposSolver`]. See the
 /// documentation of [`sp_npos_elections::seq_phragmen`] for more info.
-pub struct SequentialPhragmen<AccountId, Accuracy, Balancing = ()>(
-	core::marker::PhantomData<(AccountId, Accuracy, Balancing)>,
+pub struct SequentialPhragmen<AccountId, Accuracy, MaxBackersPerWinner = (), Balancing = ()>(
+	core::marker::PhantomData<(AccountId, Accuracy, MaxBackersPerWinner, Balancing)>,
 );
 
-impl<AccountId: IdentifierT, Accuracy: PerThing128, Balancing: Get<Option<BalancingConfig>>>
-	NposSolver for SequentialPhragmen<AccountId, Accuracy, Balancing>
+impl<
+		AccountId: IdentifierT,
+		Accuracy: PerThing128,
+		MaxBackersPerWinner: Get<Option<u32>>,
+		Balancing: Get<Option<BalancingConfig>>,
+	> NposSolver for SequentialPhragmen<AccountId, Accuracy, MaxBackersPerWinner, Balancing>
 {
 	type AccountId = AccountId;
 	type Accuracy = Accuracy;
@@ -685,7 +689,13 @@ impl<AccountId: IdentifierT, Accuracy: PerThing128, Balancing: Get<Option<Balanc
 		targets: Vec<Self::AccountId>,
 		voters: Vec<(Self::AccountId, VoteWeight, impl IntoIterator<Item = Self::AccountId>)>,
 	) -> Result<ElectionResult<Self::AccountId, Self::Accuracy>, Self::Error> {
-		sp_npos_elections::seq_phragmen(winners, targets, voters, Balancing::get())
+		sp_npos_elections::seq_phragmen(
+			winners,
+			targets,
+			voters,
+			MaxBackersPerWinner::get(),
+			Balancing::get(),
+		)
 	}
 
 	fn weight<T: WeightInfo>(voters: u32, targets: u32, vote_degree: u32) -> Weight {

substrate/frame/election-provider-support/src/onchain.rs

@@ -145,8 +145,9 @@ impl<T: Config> OnChainExecution<T> {
 			DispatchClass::Mandatory,
 		);
 
-		// defensive: Since npos solver returns a result always bounded by `desired_targets`, this
-		// is never expected to happen as long as npos solver does what is expected for it to do.
+		// defensive: Since npos solver returns a result always bounded by `desired_targets`, and
+		// ensures the maximum backers per winner, this is never expected to happen as long as npos
+		// solver does what is expected for it to do.
 		let supports: BoundedSupportsOf<Self> =
 			to_supports(&staked).try_into().map_err(|_| Error::TooManyWinners)?;
 

substrate/primitives/npos-elections/src/lib.rs

@@ -247,6 +247,9 @@ pub struct Candidate<AccountId> {
 	elected: bool,
 	/// The round index at which this candidate was elected.
 	round: usize,
+	/// A list of included backers for this candidate. This can be used to control the bounds of
+	/// maximum backers per candidate.
+	bounded_backers: Vec<AccountId>,
 }
 
 impl<AccountId> Candidate<AccountId> {
@@ -269,21 +272,23 @@ pub struct Edge<AccountId> {
 	candidate: CandidatePtr<AccountId>,
 	/// The weight (i.e. stake given to `who`) of this edge.
 	weight: ExtendedBalance,
+	/// Skips this edge.
+	skip: bool,
 }
 
 #[cfg(test)]
 impl<AccountId: Clone> Edge<AccountId> {
 	fn new(candidate: Candidate<AccountId>, weight: ExtendedBalance) -> Self {
 		let who = candidate.who.clone();
 		let candidate = Rc::new(RefCell::new(candidate));
-		Self { weight, who, candidate, load: Default::default() }
+		Self { weight, who, candidate, load: Default::default(), skip: false }
 	}
 }
 
 #[cfg(feature = "std")]
 impl<A: IdentifierT> core::fmt::Debug for Edge<A> {
 	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-		write!(f, "Edge({:?}, weight = {:?})", self.who, self.weight)
+		write!(f, "Edge({:?}, weight = {:?}, skip = {})", self.who, self.weight, self.skip)
 	}
 }
 
@@ -556,6 +561,7 @@ pub fn setup_inputs<AccountId: IdentifierT>(
 				backed_stake: Default::default(),
 				elected: Default::default(),
 				round: Default::default(),
+				bounded_backers: Default::default(),
 			}
 			.to_ptr()
 		})
@@ -580,6 +586,7 @@ pub fn setup_inputs<AccountId: IdentifierT>(
 						candidate: Rc::clone(&candidates[*idx]),
 						load: Default::default(),
 						weight: Default::default(),
+						skip: false,
 					});
 				} // else {} would be wrong votes. We don't really care about it.
 			}

substrate/primitives/npos-elections/src/mock.rs

@@ -311,6 +311,7 @@ pub(crate) fn run_and_compare<Output: PerThing128, FS>(
 	voters: Vec<(AccountId, Vec<AccountId>)>,
 	stake_of: FS,
 	to_elect: usize,
+	max_backers_candidate: Option<u32>,
 ) where
 	Output: PerThing128,
 	FS: Fn(&AccountId) -> VoteWeight,
@@ -323,6 +324,7 @@ pub(crate) fn run_and_compare<Output: PerThing128, FS>(
 			.iter()
 			.map(|(ref v, ref vs)| (*v, stake_of(v), vs.clone()))
 			.collect::<Vec<_>>(),
+		max_backers_candidate,
 		None,
 	)
 	.unwrap();

substrate/primitives/npos-elections/src/phragmen.rs

@@ -71,11 +71,13 @@ pub fn seq_phragmen<AccountId: IdentifierT, P: PerThing128>(
 	to_elect: usize,
 	candidates: Vec<AccountId>,
 	voters: Vec<(AccountId, VoteWeight, impl IntoIterator<Item = AccountId>)>,
+	max_backers_per_candidate: Option<u32>,
 	balancing: Option<BalancingConfig>,
 ) -> Result<ElectionResult<AccountId, P>, crate::Error> {
 	let (candidates, voters) = setup_inputs(candidates, voters);
 
-	let (candidates, mut voters) = seq_phragmen_core::<AccountId>(to_elect, candidates, voters)?;
+	let (candidates, mut voters) =
+		seq_phragmen_core::<AccountId>(to_elect, candidates, voters, max_backers_per_candidate)?;
 
 	if let Some(ref config) = balancing {
 		// NOTE: might create zero-edges, but we will strip them again when we convert voter into
@@ -118,6 +120,7 @@ pub fn seq_phragmen_core<AccountId: IdentifierT>(
 	to_elect: usize,
 	candidates: Vec<CandidatePtr<AccountId>>,
 	mut voters: Vec<Voter<AccountId>>,
+	max_backers_per_candidate: Option<u32>,
 ) -> Result<(Vec<CandidatePtr<AccountId>>, Vec<Voter<AccountId>>), crate::Error> {
 	// we have already checked that we have more candidates than minimum_candidate_count.
 	let to_elect = to_elect.min(candidates.len());
@@ -138,10 +141,21 @@ pub fn seq_phragmen_core<AccountId: IdentifierT>(
 			}
 		}
 
-		// loop 2: increment score
-		for voter in &voters {
-			for edge in &voter.edges {
+		// loop 2: increment score and the included backers of a candidate.
+		for voter in &mut voters {
+			for edge in &mut voter.edges {
 				let mut candidate = edge.candidate.borrow_mut();
+
+				if (candidate.bounded_backers.len() as u32) >=
+					max_backers_per_candidate.unwrap_or(Bounded::max_value()) &&
+					!candidate.bounded_backers.contains(&voter.who)
+				{
+					// if the candidate has reached max backers and the voter is not part of the
+					// bounded backers, taint the edge with skip and continue.
+					edge.skip = true;
+					continue
+				}
+
 				if !candidate.elected && !candidate.approval_stake.is_zero() {
 					let temp_n = multiply_by_rational_with_rounding(
 						voter.load.n(),
@@ -153,6 +167,7 @@ pub fn seq_phragmen_core<AccountId: IdentifierT>(
 					let temp_d = voter.load.d();
 					let temp = Rational128::from(temp_n, temp_d);
 					candidate.score = candidate.score.lazy_saturating_add(temp);
+					candidate.bounded_backers.push(voter.who.clone());
 				}
 			}
 		}
@@ -183,6 +198,11 @@ pub fn seq_phragmen_core<AccountId: IdentifierT>(
 	// update backing stake of candidates and voters
 	for voter in &mut voters {
 		for edge in &mut voter.edges {
+			if edge.skip {
+				// skip this edge as its candidate has already reached max backers.
+				continue
+			}
+
 			if edge.candidate.borrow().elected {
 				// update internal state.
 				edge.weight = multiply_by_rational_with_rounding(

substrate/primitives/npos-elections/src/pjr.rs

@@ -294,6 +294,8 @@ fn prepare_pjr_input<AccountId: IdentifierT>(
 				score: Default::default(),
 				approval_stake: Default::default(),
 				round: Default::default(),
+				// TODO: check if we need to pass the bounds here.
+				bounded_backers: supports.iter().map(|(a, _)| a).cloned().collect(),
 			}
 			.to_ptr()
 		})
@@ -324,6 +326,7 @@ fn prepare_pjr_input<AccountId: IdentifierT>(
 						candidate: Rc::clone(&candidates[*idx]),
 						weight,
 						load: Default::default(),
+						skip: false,
 					});
 				}
 			}
@@ -402,6 +405,7 @@ mod tests {
 					score: Default::default(),
 					approval_stake: Default::default(),
 					round: Default::default(),
+					bounded_backers: Default::default(),
 				}
 			})
 			.collect::<Vec<_>>();
@@ -412,6 +416,7 @@ mod tests {
 				weight: c.backed_stake,
 				candidate: c.to_ptr(),
 				load: Default::default(),
+				skip: false,
 			})
 			.collect::<Vec<_>>();
 		voter.edges = edges;
@@ -454,6 +459,7 @@ mod tests {
 			approval_stake: Default::default(),
 			backed_stake: Default::default(),
 			round: Default::default(),
+			bounded_backers: Default::default(),
 		}
 		.to_ptr();
 		let score = pre_score(unelected, &vec![v1, v2, v3], 15);