Skip to content

Commit

Permalink
Configurable voting-degree in council elections pallet (paritytech#13305
Browse files Browse the repository at this point in the history 
)

* configurable council elections pallet

* configurable council elections pallet

* add warning

* reduce sizes

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_elections_phragmen

* fix stuff

* make assert

* fix docs

* fix docs again

* fix docs again

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

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

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

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

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

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

* fix docs

---------

Co-authored-by: command-bot <>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
  • Loading branch information
@kianenigma @gpestana @melekes
2 people authored and melekes committed Feb 12, 2023
1 parent ae85e8b commit 0f794b4
Show file tree
Hide file tree
Showing 5 changed files with 216 additions and 240 deletions.
6 changes: 4 additions & 2 deletions bin/node/runtime/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1003,8 +1003,9 @@ 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 MaxVotesPerVoter: u32 = 16;
pub const MaxVoters: u32 = 512;
pub const MaxCandidates: u32 = 64;
pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect";
}

Expand All @@ -1029,6 +1030,7 @@ impl pallet_elections_phragmen::Config for Runtime {
type DesiredRunnersUp = DesiredRunnersUp;
type TermDuration = TermDuration;
type MaxVoters = MaxVoters;
type MaxVotesPerVoter = MaxVotesPerVoter;
type MaxCandidates = MaxCandidates;
type WeightInfo = pallet_elections_phragmen::weights::SubstrateWeight<Runtime>;
}
Expand Down
88 changes: 13 additions & 75 deletions frame/elections-phragmen/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ fn clean<T: Config>() {
benchmarks! {
// -- Signed ones
vote_equal {
let v in 1 .. (MAXIMUM_VOTE as u32);
let v in 1 .. T::MaxVotesPerVoter::get();
clean::<T>();

// create a bunch of candidates.
Expand All @@ -168,7 +168,7 @@ benchmarks! {
}: vote(RawOrigin::Signed(caller), votes, stake)

vote_more {
let v in 2 .. (MAXIMUM_VOTE as u32);
let v in 2 .. T::MaxVotesPerVoter::get();
clean::<T>();

// create a bunch of candidates.
Expand All @@ -190,7 +190,7 @@ benchmarks! {
}: vote(RawOrigin::Signed(caller), votes, stake / <BalanceOf<T>>::from(10u32))

vote_less {
let v in 2 .. (MAXIMUM_VOTE as u32);
let v in 2 .. T::MaxVotesPerVoter::get();
clean::<T>();

// create a bunch of candidates.
Expand All @@ -212,7 +212,7 @@ benchmarks! {

remove_voter {
// we fix the number of voted candidates to max
let v = MAXIMUM_VOTE as u32;
let v = T::MaxVotesPerVoter::get();
clean::<T>();

// create a bunch of candidates.
Expand Down Expand Up @@ -368,7 +368,7 @@ benchmarks! {
clean::<T>();

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

// all candidates leave.
<Candidates<T>>::kill();
Expand All @@ -389,17 +389,17 @@ benchmarks! {
// that we give all candidates a self vote to make sure they are all considered.
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;
let e in (T::MaxVoters::get()) .. T::MaxVoters::get() * T::MaxVotesPerVoter::get();
clean::<T>();

// so we have a situation with v and e. we want e to basically always be in the range of `e
// -> e * MAXIMUM_VOTE`, but we cannot express that now with the benchmarks. So what we do
// is: when c is being iterated, v, and e are max and fine. when v is being iterated, e is
// being set to max and this is a problem. In these cases, we cap e to a lower value, namely
// v * MAXIMUM_VOTE. when e is being iterated, v is at max, and again fine. all in all,
// votes_per_voter can never be more than MAXIMUM_VOTE. Note that this might cause `v` to be
// an overestimate.
let votes_per_voter = (e / v).min(MAXIMUM_VOTE as u32);
// -> e * T::MaxVotesPerVoter::get()`, but we cannot express that now with the benchmarks.
// So what we do is: when c is being iterated, v, and e are max and fine. when v is being
// iterated, e is being set to max and this is a problem. In these cases, we cap e to a
// lower value, namely v * T::MaxVotesPerVoter::get(). when e is being iterated, v is at
// max, and again fine. all in all, votes_per_voter can never be more than
// T::MaxVotesPerVoter::get(). Note that this might cause `v` to be an overestimate.
let votes_per_voter = (e / v).min(T::MaxVotesPerVoter::get());

let all_candidates = submit_candidates_with_self_vote::<T>(c, "candidates")?;
let _ = distribute_voters::<T>(all_candidates, v.saturating_sub(c), votes_per_voter as usize)?;
Expand All @@ -421,68 +421,6 @@ benchmarks! {
}
}

#[extra]
election_phragmen_c_e {
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;

let all_candidates = submit_candidates_with_self_vote::<T>(c, "candidates")?;
let _ = distribute_voters::<T>(
all_candidates,
fixed_v - c,
votes_per_voter as usize,
)?;
}: {
<Elections<T>>::on_initialize(T::TermDuration::get());
}
verify {
assert_eq!(<Elections<T>>::members().len() as u32, T::DesiredMembers::get().min(c));
assert_eq!(
<Elections<T>>::runners_up().len() as u32,
T::DesiredRunnersUp::get().min(c.saturating_sub(T::DesiredMembers::get())),
);

#[cfg(test)]
{
// reset members in between benchmark tests.
use crate::tests::MEMBERS;
MEMBERS.with(|m| *m.borrow_mut() = vec![]);
}
}

#[extra]
election_phragmen_v {
let v in 4 .. 16;
let fixed_c = T::MaxCandidates::get() / 10;
let fixed_e = 64;
clean::<T>();

let votes_per_voter = fixed_e / v;

let all_candidates = submit_candidates_with_self_vote::<T>(fixed_c, "candidates")?;
let _ = distribute_voters::<T>(all_candidates, v, votes_per_voter as usize)?;
}: {
<Elections<T>>::on_initialize(T::TermDuration::get());
}
verify {
assert_eq!(<Elections<T>>::members().len() as u32, T::DesiredMembers::get().min(fixed_c));
assert_eq!(
<Elections<T>>::runners_up().len() as u32,
T::DesiredRunnersUp::get().min(fixed_c.saturating_sub(T::DesiredMembers::get())),
);

#[cfg(test)]
{
// reset members in between benchmark tests.
use crate::tests::MEMBERS;
MEMBERS.with(|m| *m.borrow_mut() = vec![]);
}
}

impl_benchmark_test_suite!(
Elections,
crate::tests::ExtBuilder::default().desired_members(13).desired_runners_up(7),
Expand Down
96 changes: 65 additions & 31 deletions frame/elections-phragmen/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@
//! ### Voting
//!
//! Voters can vote for a limited number of the candidates by providing a list of account ids,
//! bounded by [`MAXIMUM_VOTE`]. Invalid votes (voting for non-candidates) and duplicate votes are
//! ignored during election. Yet, a voter _might_ vote for a future candidate. Voters reserve a bond
//! as they vote. Each vote defines a `value`. This amount is locked from the account of the voter
//! and indicates the weight of the vote. Voters can update their votes at any time by calling
//! `vote()` again. This can update the vote targets (which might update the deposit) or update the
//! vote's stake ([`Voter::stake`]). After a round, votes are kept and might still be valid for
//! further rounds. A voter is responsible for calling `remove_voter` once they are done to have
//! their bond back and remove the lock.
//! bounded by [`Config::MaxVotesPerVoter`]. Invalid votes (voting for non-candidates) and duplicate
//! votes are ignored during election. Yet, a voter _might_ vote for a future candidate. Voters
//! reserve a bond as they vote. Each vote defines a `value`. This amount is locked from the account
//! of the voter and indicates the weight of the vote. Voters can update their votes at any time by
//! calling `vote()` again. This can update the vote targets (which might update the deposit) or
//! update the vote's stake ([`Voter::stake`]). After a round, votes are kept and might still be
//! valid for further rounds. A voter is responsible for calling `remove_voter` once they are done
//! to have their bond back and remove the lock.
//!
//! See [`Call::vote`], [`Call::remove_voter`].
//!
Expand Down Expand Up @@ -124,9 +124,6 @@ pub mod migrations;

const LOG_TARGET: &str = "runtime::elections-phragmen";

/// The maximum votes allowed per voter.
pub const MAXIMUM_VOTE: usize = 16;

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 @@ -254,19 +251,29 @@ pub mod pallet {

/// 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.
/// Warning: This impacts the size of the election which is run onchain. Chose wisely, and
/// consider how it will impact `T::WeightInfo::election_phragmen`.
///
/// 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.
/// Warning: This impacts the size of the election which is run onchain. Chose wisely, and
/// consider how it will impact `T::WeightInfo::election_phragmen`.
///
/// When the limit is reached the new voters are ignored.
#[pallet::constant]
type MaxVoters: Get<u32>;

/// Maximum numbers of votes per voter.
///
/// Warning: This impacts the size of the election which is run onchain. Chose wisely, and
/// consider how it will impact `T::WeightInfo::election_phragmen`.
#[pallet::constant]
type MaxVotesPerVoter: Get<u32>;

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;
}
Expand All @@ -284,6 +291,41 @@ pub mod pallet {
Weight::zero()
}
}

fn integrity_test() {
let block_weight = T::BlockWeights::get().max_block;
// mind the order.
let election_weight = T::WeightInfo::election_phragmen(
T::MaxCandidates::get(),
T::MaxVoters::get(),
T::MaxVotesPerVoter::get() * T::MaxVoters::get(),
);

let to_seconds = |w: &Weight| {
w.ref_time() as f32 /
frame_support::weights::constants::WEIGHT_REF_TIME_PER_SECOND as f32
};

frame_support::log::debug!(
target: LOG_TARGET,
"election weight {}s ({:?}) // chain's block weight {}s ({:?})",
to_seconds(&election_weight),
election_weight,
to_seconds(&block_weight),
block_weight,
);
assert!(
election_weight.all_lt(block_weight),
"election weight {}s ({:?}) will exceed a {}s chain's block weight ({:?}) (MaxCandidates {}, MaxVoters {}, MaxVotesPerVoter {} -- tweak these parameters)",
election_weight,
to_seconds(&election_weight),
to_seconds(&block_weight),
block_weight,
T::MaxCandidates::get(),
T::MaxVoters::get(),
T::MaxVotesPerVoter::get(),
);
}
}

#[pallet::call]
Expand All @@ -307,10 +349,6 @@ pub mod pallet {
///
/// It is the responsibility of the caller to **NOT** place all of their balance into the
/// lock and keep some for further operations.
///
/// # <weight>
/// We assume the maximum weight among all 3 cases: vote_equal, vote_more and vote_less.
/// # </weight>
#[pallet::call_index(0)]
#[pallet::weight(
T::WeightInfo::vote_more(votes.len() as u32)
Expand All @@ -324,8 +362,10 @@ pub mod pallet {
) -> DispatchResultWithPostInfo {
let who = ensure_signed(origin)?;

// votes should not be empty and more than `MAXIMUM_VOTE` in any case.
ensure!(votes.len() <= MAXIMUM_VOTE, Error::<T>::MaximumVotesExceeded);
ensure!(
votes.len() <= T::MaxVotesPerVoter::get() as usize,
Error::<T>::MaximumVotesExceeded
);
ensure!(!votes.is_empty(), Error::<T>::NoVotes);

let candidates_count = <Candidates<T>>::decode_len().unwrap_or(0);
Expand Down Expand Up @@ -1006,15 +1046,15 @@ impl<T: Config> Pallet<T> {
// count](https://en.wikipedia.org/wiki/Borda_count). We weigh everyone's vote for
// that new member by a multiplier based on the order of the votes. i.e. the
// first person a voter votes for gets a 16x multiplier, the next person gets a
// 15x multiplier, an so on... (assuming `MAXIMUM_VOTE` = 16)
// 15x multiplier, an so on... (assuming `T::MaxVotesPerVoter` = 16)
let mut prime_votes = new_members_sorted_by_id
.iter()
.map(|c| (&c.0, BalanceOf::<T>::zero()))
.collect::<Vec<_>>();
for (_, stake, votes) in voters_and_stakes.into_iter() {
for (vote_multiplier, who) in
votes.iter().enumerate().map(|(vote_position, who)| {
((MAXIMUM_VOTE - vote_position) as u32, who)
((T::MaxVotesPerVoter::get() as usize - vote_position) as u32, who)
}) {
if let Ok(i) = prime_votes.binary_search_by_key(&who, |k| k.0) {
prime_votes[i].1 = prime_votes[i]
Expand Down Expand Up @@ -1173,16 +1213,9 @@ mod tests {
};
use substrate_test_utils::assert_eq_uvec;

parameter_types! {
pub BlockWeights: frame_system::limits::BlockWeights =
frame_system::limits::BlockWeights::simple_max(
frame_support::weights::Weight::from_ref_time(1024).set_proof_size(u64::MAX),
);
}

impl frame_system::Config for Test {
type BaseCallFilter = frame_support::traits::Everything;
type BlockWeights = BlockWeights;
type BlockWeights = ();
type BlockLength = ();
type DbWeight = ();
type RuntimeOrigin = RuntimeOrigin;
Expand Down Expand Up @@ -1297,6 +1330,7 @@ mod tests {
type KickedMember = ();
type WeightInfo = ();
type MaxVoters = PhragmenMaxVoters;
type MaxVotesPerVoter = ConstU32<16>;
type MaxCandidates = PhragmenMaxCandidates;
}

Expand Down
Loading

0 comments on commit 0f794b4

Please sign in to comment.