Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions substrate/frame/election-provider-multi-phase/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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 =
Expand Down
95 changes: 94 additions & 1 deletion substrate/frame/election-provider-multi-phase/src/unsigned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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(|| {
Expand Down
20 changes: 15 additions & 5 deletions substrate/frame/election-provider-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions substrate/frame/election-provider-support/src/onchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand Down
11 changes: 9 additions & 2 deletions substrate/primitives/npos-elections/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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()
})
Expand All @@ -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.
}
Expand Down
2 changes: 2 additions & 0 deletions substrate/primitives/npos-elections/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
Expand Down
28 changes: 24 additions & 4 deletions substrate/primitives/npos-elections/src/phragmen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand All @@ -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(),
Expand All @@ -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());
}
}
}
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 6 additions & 0 deletions substrate/primitives/npos-elections/src/pjr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
Expand Down Expand Up @@ -324,6 +326,7 @@ fn prepare_pjr_input<AccountId: IdentifierT>(
candidate: Rc::clone(&candidates[*idx]),
weight,
load: Default::default(),
skip: false,
});
}
}
Expand Down Expand Up @@ -402,6 +405,7 @@ mod tests {
score: Default::default(),
approval_stake: Default::default(),
round: Default::default(),
bounded_backers: Default::default(),
}
})
.collect::<Vec<_>>();
Expand All @@ -412,6 +416,7 @@ mod tests {
weight: c.backed_stake,
candidate: c.to_ptr(),
load: Default::default(),
skip: false,
})
.collect::<Vec<_>>();
voter.edges = edges;
Expand Down Expand Up @@ -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);
Expand Down
Loading