Revert "Adds max backers per winner bounds check in the phragmen impl…

…ementation (#6482)"

This reverts commit 69b38e5.

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

@@ -153,8 +153,7 @@ 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, None)
-			.unwrap();
+		seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None).unwrap();
 
 	// sort by decreasing order of stake
 	assignments.sort_by_key(|assignment| {
@@ -181,8 +180,7 @@ 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, None)
-			.unwrap();
+		seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None).unwrap();
 
 	// closures
 	let cache = helpers::generate_voter_cache::<Runtime>(&voters);
@@ -310,8 +308,7 @@ parameter_types! {
 pub struct OnChainSeqPhragmen;
 impl onchain::Config for OnChainSeqPhragmen {
 	type System = Runtime;
-	type Solver =
-		SequentialPhragmen<AccountId, SolutionAccuracyOf<Runtime>, MaxBackersPerWinner, Balancing>;
+	type Solver = SequentialPhragmen<AccountId, SolutionAccuracyOf<Runtime>, Balancing>;
 	type DataProvider = StakingMock;
 	type WeightInfo = ();
 	type MaxWinnersPerPage = MaxWinners;
@@ -423,8 +420,7 @@ impl crate::Config for Runtime {
 	type MaxWinners = MaxWinners;
 	type MaxBackersPerWinner = MaxBackersPerWinner;
 	type MinerConfig = Self;
-	type Solver =
-		SequentialPhragmen<AccountId, SolutionAccuracyOf<Runtime>, MaxBackersPerWinner, Balancing>;
+	type Solver = SequentialPhragmen<AccountId, SolutionAccuracyOf<Runtime>, Balancing>;
 	type ElectionBounds = ElectionsBounds;
 }
 
@@ -611,10 +607,6 @@ 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 a solution.
+	/// The maximum number of backers per winner in the last solution.
 	type MaxBackersPerWinner: Get<u32>;
 	/// Something that can compute the weight of a solution.
 	///
@@ -1865,99 +1865,6 @@ 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,16 +670,12 @@ 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, MaxBackersPerWinner = (), Balancing = ()>(
-	core::marker::PhantomData<(AccountId, Accuracy, MaxBackersPerWinner, Balancing)>,
+pub struct SequentialPhragmen<AccountId, Accuracy, Balancing = ()>(
+	core::marker::PhantomData<(AccountId, Accuracy, Balancing)>,
 );
 
-impl<
-		AccountId: IdentifierT,
-		Accuracy: PerThing128,
-		MaxBackersPerWinner: Get<Option<u32>>,
-		Balancing: Get<Option<BalancingConfig>>,
-	> NposSolver for SequentialPhragmen<AccountId, Accuracy, MaxBackersPerWinner, Balancing>
+impl<AccountId: IdentifierT, Accuracy: PerThing128, Balancing: Get<Option<BalancingConfig>>>
+	NposSolver for SequentialPhragmen<AccountId, Accuracy, Balancing>
 {
 	type AccountId = AccountId;
 	type Accuracy = Accuracy;
@@ -689,13 +685,7 @@ impl<
 		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,
-			MaxBackersPerWinner::get(),
-			Balancing::get(),
-		)
+		sp_npos_elections::seq_phragmen(winners, targets, voters, Balancing::get())
 	}
 
 	fn weight<T: WeightInfo>(voters: u32, targets: u32, vote_degree: u32) -> Weight {

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

@@ -145,9 +145,8 @@ impl<T: Config> OnChainExecution<T> {
 			DispatchClass::Mandatory,
 		);
 
-		// 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.
+		// 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.
 		let supports: BoundedSupportsOf<Self> =
 			to_supports(&staked).try_into().map_err(|_| Error::TooManyWinners)?;
 

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

@@ -247,9 +247,6 @@ 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> {
@@ -272,23 +269,21 @@ 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(), skip: false }
+		Self { weight, who, candidate, load: Default::default() }
 	}
 }
 
 #[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 = {:?}, skip = {})", self.who, self.weight, self.skip)
+		write!(f, "Edge({:?}, weight = {:?})", self.who, self.weight)
 	}
 }
 
@@ -561,7 +556,6 @@ pub fn setup_inputs<AccountId: IdentifierT>(
 				backed_stake: Default::default(),
 				elected: Default::default(),
 				round: Default::default(),
-				bounded_backers: Default::default(),
 			}
 			.to_ptr()
 		})
@@ -586,7 +580,6 @@ 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,7 +311,6 @@ 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,
@@ -324,7 +323,6 @@ 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,13 +71,11 @@ 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, max_backers_per_candidate)?;
+	let (candidates, mut voters) = seq_phragmen_core::<AccountId>(to_elect, candidates, voters)?;
 
 	if let Some(ref config) = balancing {
 		// NOTE: might create zero-edges, but we will strip them again when we convert voter into
@@ -120,7 +118,6 @@ 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());
@@ -141,21 +138,10 @@ pub fn seq_phragmen_core<AccountId: IdentifierT>(
 			}
 		}
 
-		// loop 2: increment score and the included backers of a candidate.
-		for voter in &mut voters {
-			for edge in &mut voter.edges {
+		// loop 2: increment score
+		for voter in &voters {
+			for edge in &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(),
@@ -167,7 +153,6 @@ 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());
 				}
 			}
 		}
@@ -198,11 +183,6 @@ 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,8 +294,6 @@ 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()
 		})
@@ -326,7 +324,6 @@ fn prepare_pjr_input<AccountId: IdentifierT>(
 						candidate: Rc::clone(&candidates[*idx]),
 						weight,
 						load: Default::default(),
-						skip: false,
 					});
 				}
 			}
@@ -405,7 +402,6 @@ mod tests {
 					score: Default::default(),
 					approval_stake: Default::default(),
 					round: Default::default(),
-					bounded_backers: Default::default(),
 				}
 			})
 			.collect::<Vec<_>>();
@@ -416,7 +412,6 @@ mod tests {
 				weight: c.backed_stake,
 				candidate: c.to_ptr(),
 				load: Default::default(),
-				skip: false,
 			})
 			.collect::<Vec<_>>();
 		voter.edges = edges;
@@ -459,7 +454,6 @@ 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);