Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
make MAX_VOTERS and MAX_CANDIDATES in elections-phragmen configurable.
Browse files Browse the repository at this point in the history
…Fix: #11092 (#11908)

* make MAX_VOTERS and MAX_CANDIDATES in elections-phragmen configurable

* Configure election-phragmen in node bin configuring max candidates & voters

* Add document comment for added Config parameter

* Incorporate suggestion

* fix benchmarks

* Update frame/elections-phragmen/src/lib.rs

* Update frame/elections-phragmen/src/lib.rs

* fix wrong values

* fix typo

* docs

* more detailed docs

* fmt

* ".git/.scripts/bench-bot.sh" pallet dev pallet_elections_phragmen

Co-authored-by: Szegoo <[email protected]>
Co-authored-by: Sergej Sakac <[email protected]>
Co-authored-by: command-bot <>
  • Loading branch information
3 people authored Aug 14, 2022
1 parent 055453e commit 34a0621
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 78 deletions.
4 changes: 4 additions & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,8 @@ parameter_types! {
pub const TermDuration: BlockNumber = 7 * DAYS;
pub const DesiredMembers: u32 = 13;
pub const DesiredRunnersUp: u32 = 7;
pub const MaxVoters: u32 = 10 * 1000;
pub const MaxCandidates: u32 = 1000;
pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect";
}

Expand All @@ -989,6 +991,8 @@ impl pallet_elections_phragmen::Config for Runtime {
type DesiredMembers = DesiredMembers;
type DesiredRunnersUp = DesiredRunnersUp;
type TermDuration = TermDuration;
type MaxVoters = MaxVoters;
type MaxCandidates = MaxCandidates;
type WeightInfo = pallet_elections_phragmen::weights::SubstrateWeight<Runtime>;
}

Expand Down
26 changes: 13 additions & 13 deletions frame/elections-phragmen/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fn endowed_account<T: Config>(name: &'static str, index: u32) -> T::AccountId {
let account: T::AccountId = account(name, index, 0);
// Fund each account with at-least his stake but still a sane amount as to not mess up
// the vote calculation.
let amount = default_stake::<T>(MAX_VOTERS) * BalanceOf::<T>::from(BALANCE_FACTOR);
let amount = default_stake::<T>(T::MaxVoters::get()) * BalanceOf::<T>::from(BALANCE_FACTOR);
let _ = T::Currency::make_free_balance_be(&account, amount);
// important to increase the total issuance since T::CurrencyToVote will need it to be sane for
// phragmen to work.
Expand Down Expand Up @@ -230,7 +230,7 @@ benchmarks! {

submit_candidacy {
// number of already existing candidates.
let c in 1 .. MAX_CANDIDATES;
let c in 1 .. T::MaxCandidates::get();
// we fix the number of members to the number of desired members and runners-up. We'll be in
// this state almost always.
let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();
Expand Down Expand Up @@ -261,7 +261,7 @@ benchmarks! {
// this will check members, runners-up and candidate for removal. Members and runners-up are
// limited by the runtime bound, nonetheless we fill them by `m`.
// number of already existing candidates.
let c in 1 .. MAX_CANDIDATES;
let c in 1 .. T::MaxCandidates::get();
// we fix the number of members to the number of desired members and runners-up. We'll be in
// this state almost always.
let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();
Expand Down Expand Up @@ -362,14 +362,14 @@ benchmarks! {

clean_defunct_voters {
// total number of voters.
let v in (MAX_VOTERS / 2) .. MAX_VOTERS;
let v in (T::MaxVoters::get() / 2) .. T::MaxVoters::get();
// those that are defunct and need removal.
let d in 1 .. (MAX_VOTERS / 2);
let d in 1 .. (T::MaxVoters::get() / 2);

// remove any previous stuff.
clean::<T>();

let all_candidates = submit_candidates::<T>(MAX_CANDIDATES, "candidates")?;
let all_candidates = submit_candidates::<T>(T::MaxCandidates::get(), "candidates")?;
distribute_voters::<T>(all_candidates, v, MAXIMUM_VOTE)?;

// all candidates leave.
Expand All @@ -389,9 +389,9 @@ benchmarks! {
// members, this is hard-coded in the runtime and cannot be trivially changed at this stage.
// Yet, change the number of voters, candidates and edge per voter to see the impact. Note
// that we give all candidates a self vote to make sure they are all considered.
let c in 1 .. MAX_CANDIDATES;
let v in 1 .. MAX_VOTERS;
let e in MAX_VOTERS .. MAX_VOTERS * MAXIMUM_VOTE as u32;
let c in 1 .. T::MaxCandidates::get();
let v in 1 .. T::MaxVoters::get();
let e in (T::MaxVoters::get()) .. T::MaxVoters::get() as u32 * MAXIMUM_VOTE as u32;
clean::<T>();

// so we have a situation with v and e. we want e to basically always be in the range of `e
Expand Down Expand Up @@ -425,9 +425,9 @@ benchmarks! {

#[extra]
election_phragmen_c_e {
let c in 1 .. MAX_CANDIDATES;
let e in MAX_VOTERS .. MAX_VOTERS * MAXIMUM_VOTE as u32;
let fixed_v = MAX_VOTERS;
let c in 1 .. T::MaxCandidates::get();
let e in (T::MaxVoters::get()) .. T::MaxVoters::get() * MAXIMUM_VOTE as u32;
let fixed_v = T::MaxVoters::get();
clean::<T>();

let votes_per_voter = e / fixed_v;
Expand Down Expand Up @@ -459,7 +459,7 @@ benchmarks! {
#[extra]
election_phragmen_v {
let v in 4 .. 16;
let fixed_c = MAX_CANDIDATES / 10;
let fixed_c = T::MaxCandidates::get() / 10;
let fixed_e = 64;
clean::<T>();

Expand Down
40 changes: 26 additions & 14 deletions frame/elections-phragmen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,17 +125,6 @@ pub mod migrations;
/// The maximum votes allowed per voter.
pub const MAXIMUM_VOTE: usize = 16;

// Some safe temp values to make the wasm execution sane while we still use this pallet.
#[cfg(test)]
pub(crate) const MAX_CANDIDATES: u32 = 100;
#[cfg(not(test))]
pub(crate) const MAX_CANDIDATES: u32 = 1000;

#[cfg(test)]
pub(crate) const MAX_VOTERS: u32 = 1000;
#[cfg(not(test))]
pub(crate) const MAX_VOTERS: u32 = 10 * 1000;

type BalanceOf<T> =
<<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;
type NegativeImbalanceOf<T> = <<T as Config>::Currency as Currency<
Expand Down Expand Up @@ -259,6 +248,21 @@ pub mod pallet {
#[pallet::constant]
type TermDuration: Get<Self::BlockNumber>;

/// The maximum number of candidates in a phragmen election.
///
/// Warning: The election happens onchain, and this value will determine
/// the size of the election. When this limit is reached no more
/// candidates are accepted in the election.
#[pallet::constant]
type MaxCandidates: Get<u32>;

/// The maximum number of voters to allow in a phragmen election.
///
/// Warning: This impacts the size of the election which is run onchain.
/// When the limit is reached the new voters are ignored.
#[pallet::constant]
type MaxVoters: Get<u32>;

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;
}
Expand Down Expand Up @@ -397,7 +401,10 @@ pub mod pallet {

let actual_count = <Candidates<T>>::decode_len().unwrap_or(0) as u32;
ensure!(actual_count <= candidate_count, Error::<T>::InvalidWitnessData);
ensure!(actual_count <= MAX_CANDIDATES, Error::<T>::TooManyCandidates);
ensure!(
actual_count <= <T as Config>::MaxCandidates::get(),
Error::<T>::TooManyCandidates
);

let index = Self::is_candidate(&who).err().ok_or(Error::<T>::DuplicatedCandidate)?;

Expand Down Expand Up @@ -913,10 +920,11 @@ impl<T: Config> Pallet<T> {

let mut num_edges: u32 = 0;

let max_voters = <T as Config>::MaxVoters::get() as usize;
// used for prime election.
let mut voters_and_stakes = Vec::new();
match Voting::<T>::iter().try_for_each(|(voter, Voter { stake, votes, .. })| {
if voters_and_stakes.len() < MAX_VOTERS as usize {
if voters_and_stakes.len() < max_voters {
voters_and_stakes.push((voter, stake, votes));
Ok(())
} else {
Expand All @@ -930,7 +938,7 @@ impl<T: Config> Pallet<T> {
"Failed to run election. Number of voters exceeded",
);
Self::deposit_event(Event::ElectionError);
return T::DbWeight::get().reads(3 + MAX_VOTERS as u64)
return T::DbWeight::get().reads(3 + max_voters as u64)
},
}

Expand Down Expand Up @@ -1266,6 +1274,8 @@ mod tests {

parameter_types! {
pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect";
pub const PhragmenMaxVoters: u32 = 1000;
pub const PhragmenMaxCandidates: u32 = 100;
}

impl Config for Test {
Expand All @@ -1284,6 +1294,8 @@ mod tests {
type LoserCandidate = ();
type KickedMember = ();
type WeightInfo = ();
type MaxVoters = PhragmenMaxVoters;
type MaxCandidates = PhragmenMaxCandidates;
}

pub type Block = sp_runtime::generic::Block<Header, UncheckedExtrinsic>;
Expand Down
Loading

0 comments on commit 34a0621

Please sign in to comment.