diff --git a/prdoc/pr_6314.prdoc b/prdoc/pr_6314.prdoc
new file mode 100644
index 0000000000000000000000000000000000000000..2ebbc68158d590d6b8d33abe77752139f1e2db06
--- /dev/null
+++ b/prdoc/pr_6314.prdoc
@@ -0,0 +1,10 @@
+title: Migrate pallet-elections-phragmen benchmark to v2 and improve doc
+doc:
+- audience: Runtime Dev
+ description: |-
+ Part of:
+
+ - #6202.
+crates:
+- name: pallet-elections-phragmen
+ bump: patch
diff --git a/substrate/frame/elections-phragmen/src/benchmarking.rs b/substrate/frame/elections-phragmen/src/benchmarking.rs
index 8e762f667b2a6a0d3b58916e93e91bb8fc56a67b..60771fa89ad7ea6378bd1e97f8d47054bca58674 100644
--- a/substrate/frame/elections-phragmen/src/benchmarking.rs
+++ b/substrate/frame/elections-phragmen/src/benchmarking.rs
@@ -19,47 +19,47 @@
#![cfg(feature = "runtime-benchmarks")]
-use super::*;
-
-use frame_benchmarking::v1::{account, benchmarks, whitelist, BenchmarkError, BenchmarkResult};
+use frame_benchmarking::v2::*;
use frame_support::{dispatch::DispatchResultWithPostInfo, traits::OnInitialize};
use frame_system::RawOrigin;
-use crate::Pallet as Elections;
+#[cfg(test)]
+use crate::tests::MEMBERS;
+use crate::*;
const BALANCE_FACTOR: u32 = 250;
-/// grab new account with infinite balance.
+// grab new account with infinite balance.
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 their stake but still a sane amount as to not mess up
// the vote calculation.
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.
+ // Important to increase the total issuance since `T::CurrencyToVote` will need it to be sane
+ // for phragmen to work.
let _ = T::Currency::issue(amount);
account
}
-/// Account to lookup type of system trait.
+// Account to lookup type of system trait.
fn as_lookup<T: Config>(account: T::AccountId) -> AccountIdLookupOf<T> {
T::Lookup::unlookup(account)
}
-/// Get a reasonable amount of stake based on the execution trait's configuration
+// Get a reasonable amount of stake based on the execution trait's configuration.
fn default_stake<T: Config>(num_votes: u32) -> BalanceOf<T> {
let min = T::Currency::minimum_balance();
- Elections::<T>::deposit_of(num_votes as usize).max(min)
+ Pallet::<T>::deposit_of(num_votes as usize).max(min)
}
-/// Get the current number of candidates.
+// Get the current number of candidates.
fn candidate_count<T: Config>() -> u32 {
Candidates::<T>::decode_len().unwrap_or(0usize) as u32
}
-/// Add `c` new candidates.
+// Add `c` new candidates.
fn submit_candidates<T: Config>(
c: u32,
prefix: &'static str,
@@ -67,7 +67,7 @@ fn submit_candidates<T: Config>(
(0..c)
.map(|i| {
let account = endowed_account::<T>(prefix, i);
- Elections::<T>::submit_candidacy(
+ Pallet::<T>::submit_candidacy(
RawOrigin::Signed(account.clone()).into(),
candidate_count::<T>(),
)
@@ -77,7 +77,7 @@ fn submit_candidates<T: Config>(
.collect::<Result<_, _>>()
}
-/// Add `c` new candidates with self vote.
+// Add `c` new candidates with self vote.
fn submit_candidates_with_self_vote<T: Config>(
c: u32,
prefix: &'static str,
@@ -90,17 +90,17 @@ fn submit_candidates_with_self_vote<T: Config>(
Ok(candidates)
}
-/// Submit one voter.
+// Submit one voter.
fn submit_voter<T: Config>(
caller: T::AccountId,
votes: Vec<T::AccountId>,
stake: BalanceOf<T>,
) -> DispatchResultWithPostInfo {
- Elections::<T>::vote(RawOrigin::Signed(caller).into(), votes, stake)
+ Pallet::<T>::vote(RawOrigin::Signed(caller).into(), votes, stake)
}
-/// create `num_voter` voters who randomly vote for at most `votes` of `all_candidates` if
-/// available.
+// Create `num_voter` voters who randomly vote for at most `votes` of `all_candidates` if
+// available.
fn distribute_voters<T: Config>(
mut all_candidates: Vec<T::AccountId>,
num_voters: u32,
@@ -117,12 +117,12 @@ fn distribute_voters<T: Config>(
Ok(())
}
-/// Fill the seats of members and runners-up up until `m`. Note that this might include either only
-/// members, or members and runners-up.
+// Fill the seats of members and runners-up up until `m`. Note that this might include either only
+// members, or members and runners-up.
fn fill_seats_up_to<T: Config>(m: u32) -> Result<Vec<T::AccountId>, &'static str> {
let _ = submit_candidates_with_self_vote::<T>(m, "fill_seats_up_to")?;
assert_eq!(Candidates::<T>::get().len() as u32, m, "wrong number of candidates.");
- Elections::<T>::do_phragmen();
+ Pallet::<T>::do_phragmen();
assert_eq!(Candidates::<T>::get().len(), 0, "some candidates remaining.");
assert_eq!(
Members::<T>::get().len() + RunnersUp::<T>::get().len(),
@@ -136,7 +136,7 @@ fn fill_seats_up_to<T: Config>(m: u32) -> Result<Vec<T::AccountId>, &'static str
.collect())
}
-/// removes all the storage items to reverse any genesis state.
+// Removes all the storage items to reverse any genesis state.
fn clean<T: Config>() {
Members::<T>::kill();
Candidates::<T>::kill();
@@ -145,10 +145,13 @@ fn clean<T: Config>() {
Voting::<T>::remove_all(None);
}
-benchmarks! {
+#[benchmarks]
+mod benchmarks {
+ use super::*;
+
// -- Signed ones
- vote_equal {
- let v in 1 .. T::MaxVotesPerVoter::get();
+ #[benchmark]
+ fn vote_equal(v: Linear<1, { T::MaxVotesPerVoter::get() }>) -> Result<(), BenchmarkError> {
clean::<T>();
// create a bunch of candidates.
@@ -157,65 +160,81 @@ benchmarks! {
let caller = endowed_account::<T>("caller", 0);
let stake = default_stake::<T>(v);
- // original votes.
+ // Original votes.
let mut votes = all_candidates;
submit_voter::<T>(caller.clone(), votes.clone(), stake)?;
- // new votes.
+ // New votes.
votes.rotate_left(1);
whitelist!(caller);
- }: vote(RawOrigin::Signed(caller), votes, stake)
- vote_more {
- let v in 2 .. T::MaxVotesPerVoter::get();
+ #[extrinsic_call]
+ vote(RawOrigin::Signed(caller), votes, stake);
+
+ Ok(())
+ }
+
+ #[benchmark]
+ fn vote_more(v: Linear<2, { T::MaxVotesPerVoter::get() }>) -> Result<(), BenchmarkError> {
clean::<T>();
- // create a bunch of candidates.
+ // Create a bunch of candidates.
let all_candidates = submit_candidates::<T>(v, "candidates")?;
let caller = endowed_account::<T>("caller", 0);
// Multiply the stake with 10 since we want to be able to divide it by 10 again.
- let stake = default_stake::<T>(v) * BalanceOf::<T>::from(10u32);
+ let stake = default_stake::<T>(v) * BalanceOf::<T>::from(10_u32);
- // original votes.
+ // Original votes.
let mut votes = all_candidates.iter().skip(1).cloned().collect::<Vec<_>>();
- submit_voter::<T>(caller.clone(), votes.clone(), stake / BalanceOf::<T>::from(10u32))?;
+ submit_voter::<T>(caller.clone(), votes.clone(), stake / BalanceOf::<T>::from(10_u32))?;
- // new votes.
+ // New votes.
votes = all_candidates;
assert!(votes.len() > Voting::<T>::get(caller.clone()).votes.len());
whitelist!(caller);
- }: vote(RawOrigin::Signed(caller), votes, stake / BalanceOf::<T>::from(10u32))
- vote_less {
- let v in 2 .. T::MaxVotesPerVoter::get();
+ #[extrinsic_call]
+ vote(RawOrigin::Signed(caller), votes, stake / BalanceOf::<T>::from(10_u32));
+
+ Ok(())
+ }
+
+ #[benchmark]
+ fn vote_less(v: Linear<2, { T::MaxVotesPerVoter::get() }>) -> Result<(), BenchmarkError> {
clean::<T>();
- // create a bunch of candidates.
+ // Create a bunch of candidates.
let all_candidates = submit_candidates::<T>(v, "candidates")?;
let caller = endowed_account::<T>("caller", 0);
let stake = default_stake::<T>(v);
- // original votes.
+ // Original votes.
let mut votes = all_candidates;
submit_voter::<T>(caller.clone(), votes.clone(), stake)?;
- // new votes.
+ // New votes.
votes = votes.into_iter().skip(1).collect::<Vec<_>>();
assert!(votes.len() < Voting::<T>::get(caller.clone()).votes.len());
whitelist!(caller);
- }: vote(RawOrigin::Signed(caller), votes, stake)
- remove_voter {
- // we fix the number of voted candidates to max
+ #[extrinsic_call]
+ vote(RawOrigin::Signed(caller), votes, stake);
+
+ Ok(())
+ }
+
+ #[benchmark]
+ fn remove_voter() -> Result<(), BenchmarkError> {
+ // We fix the number of voted candidates to max.
let v = T::MaxVotesPerVoter::get();
clean::<T>();
- // create a bunch of candidates.
+ // Create a bunch of candidates.
let all_candidates = submit_candidates::<T>(v, "candidates")?;
let caller = endowed_account::<T>("caller", 0);
@@ -224,207 +243,245 @@ benchmarks! {
submit_voter::<T>(caller.clone(), all_candidates, stake)?;
whitelist!(caller);
- }: _(RawOrigin::Signed(caller))
- submit_candidacy {
- // number of already existing 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.
+ #[extrinsic_call]
+ _(RawOrigin::Signed(caller));
+
+ Ok(())
+ }
+
+ #[benchmark]
+ fn submit_candidacy(
+ // Number of already existing candidates.
+ c: Linear<1, { T::MaxCandidates::get() }>,
+ ) -> Result<(), BenchmarkError> {
+ // 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();
clean::<T>();
- let stake = default_stake::<T>(c);
- // create m members and runners combined.
+ // Create `m` members and runners combined.
let _ = fill_seats_up_to::<T>(m)?;
- // create previous candidates;
+ // Create previous candidates.
let _ = submit_candidates::<T>(c, "candidates")?;
- // we assume worse case that: extrinsic is successful and candidate is not duplicate.
+ // We assume worse case that: extrinsic is successful and candidate is not duplicate.
let candidate_account = endowed_account::<T>("caller", 0);
whitelist!(candidate_account);
- }: _(RawOrigin::Signed(candidate_account.clone()), candidate_count::<T>())
- verify {
+
+ #[extrinsic_call]
+ _(RawOrigin::Signed(candidate_account), candidate_count::<T>());
+
+ // Reset members in between benchmark tests.
#[cfg(test)]
- {
- // reset members in between benchmark tests.
- use crate::tests::MEMBERS;
- MEMBERS.with(|m| *m.borrow_mut() = vec![]);
- }
+ MEMBERS.with(|m| *m.borrow_mut() = vec![]);
+
+ Ok(())
}
- renounce_candidacy_candidate {
- // 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 .. 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.
+ #[benchmark]
+ fn renounce_candidacy_candidate(
+ // 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.
+ c: Linear<1, { T::MaxCandidates::get() }>,
+ ) -> Result<(), BenchmarkError> {
+ // 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();
clean::<T>();
- // create m members and runners combined.
+ // Create `m` members and runners combined.
let _ = fill_seats_up_to::<T>(m)?;
let all_candidates = submit_candidates::<T>(c, "caller")?;
let bailing = all_candidates[0].clone(); // Should be ("caller", 0)
let count = candidate_count::<T>();
whitelist!(bailing);
- }: renounce_candidacy(RawOrigin::Signed(bailing), Renouncing::Candidate(count))
- verify {
+
+ #[extrinsic_call]
+ renounce_candidacy(RawOrigin::Signed(bailing), Renouncing::Candidate(count));
+
+ // Reset members in between benchmark tests.
#[cfg(test)]
- {
- // reset members in between benchmark tests.
- use crate::tests::MEMBERS;
- MEMBERS.with(|m| *m.borrow_mut() = vec![]);
- }
+ MEMBERS.with(|m| *m.borrow_mut() = vec![]);
+
+ Ok(())
}
- renounce_candidacy_members {
- // removing members and runners will be cheaper than a candidate.
- // we fix the number of members to when members and runners-up to the desired. We'll be in
- // this state almost always.
+ #[benchmark]
+ fn renounce_candidacy_members() -> Result<(), BenchmarkError> {
+ // Removing members and runners will be cheaper than a candidate.
+ // We fix the number of members to when members and runners-up to the desired.
+ // We'll be in this state almost always.
let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();
clean::<T>();
- // create m members and runners combined.
+ // Create `m` members and runners combined.
let members_and_runners_up = fill_seats_up_to::<T>(m)?;
let bailing = members_and_runners_up[0].clone();
- assert!(Elections::<T>::is_member(&bailing));
+ assert!(Pallet::<T>::is_member(&bailing));
whitelist!(bailing);
- }: renounce_candidacy(RawOrigin::Signed(bailing.clone()), Renouncing::Member)
- verify {
+
+ #[extrinsic_call]
+ renounce_candidacy(RawOrigin::Signed(bailing.clone()), Renouncing::Member);
+
+ // Reset members in between benchmark tests.
#[cfg(test)]
- {
- // reset members in between benchmark tests.
- use crate::tests::MEMBERS;
- MEMBERS.with(|m| *m.borrow_mut() = vec![]);
- }
+ MEMBERS.with(|m| *m.borrow_mut() = vec![]);
+
+ Ok(())
}
- renounce_candidacy_runners_up {
- // removing members and runners will be cheaper than a candidate.
- // we fix the number of members to when members and runners-up to the desired. We'll be in
- // this state almost always.
+ #[benchmark]
+ fn renounce_candidacy_runners_up() -> Result<(), BenchmarkError> {
+ // Removing members and runners will be cheaper than a candidate.
+ // We fix the number of members to when members and runners-up to the desired.
+ // We'll be in this state almost always.
let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();
clean::<T>();
- // create m members and runners combined.
+ // Create `m` members and runners combined.
let members_and_runners_up = fill_seats_up_to::<T>(m)?;
let bailing = members_and_runners_up[T::DesiredMembers::get() as usize + 1].clone();
- assert!(Elections::<T>::is_runner_up(&bailing));
+ assert!(Pallet::<T>::is_runner_up(&bailing));
whitelist!(bailing);
- }: renounce_candidacy(RawOrigin::Signed(bailing.clone()), Renouncing::RunnerUp)
- verify {
+
+ #[extrinsic_call]
+ renounce_candidacy(RawOrigin::Signed(bailing.clone()), Renouncing::RunnerUp);
+
+ // Reset members in between benchmark tests.
#[cfg(test)]
- {
- // reset members in between benchmark tests.
- use crate::tests::MEMBERS;
- MEMBERS.with(|m| *m.borrow_mut() = vec![]);
- }
+ MEMBERS.with(|m| *m.borrow_mut() = vec![]);
+
+ Ok(())
}
// We use the max block weight for this extrinsic for now. See below.
- remove_member_without_replacement {}: {
- Err(BenchmarkError::Override(
- BenchmarkResult::from_weight(T::BlockWeights::get().max_block)
- ))?;
+ #[benchmark]
+ fn remove_member_without_replacement() -> Result<(), BenchmarkError> {
+ #[block]
+ {
+ Err(BenchmarkError::Override(BenchmarkResult::from_weight(
+ T::BlockWeights::get().max_block,
+ )))?;
+ }
+
+ Ok(())
}
- remove_member_with_replacement {
- // easy case. We have a runner up. Nothing will have that much of an impact. m will be
- // number of members and runners. There is always at least one runner.
+ #[benchmark]
+ fn remove_member_with_replacement() -> Result<(), BenchmarkError> {
+ // Easy case.
+ // We have a runner up.
+ // Nothing will have that much of an impact.
+ // `m` will be number of members and runners.
+ // There is always at least one runner.
let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();
clean::<T>();
let _ = fill_seats_up_to::<T>(m)?;
- let removing = as_lookup::<T>(Elections::<T>::members_ids()[0].clone());
- }: remove_member(RawOrigin::Root, removing, true, false)
- verify {
- // must still have enough members.
+ let removing = as_lookup::<T>(Pallet::<T>::members_ids()[0].clone());
+
+ #[extrinsic_call]
+ remove_member(RawOrigin::Root, removing, true, false);
+
+ // Must still have enough members.
assert_eq!(Members::<T>::get().len() as u32, T::DesiredMembers::get());
+
+ // Reset members in between benchmark tests.
#[cfg(test)]
- {
- // reset members in between benchmark tests.
- use crate::tests::MEMBERS;
- MEMBERS.with(|m| *m.borrow_mut() = vec![]);
- }
- }
+ MEMBERS.with(|m| *m.borrow_mut() = vec![]);
- clean_defunct_voters {
- // total number of voters.
- let v in (T::MaxVoters::get() / 2) .. T::MaxVoters::get();
- // those that are defunct and need removal.
- let d in 0 .. (T::MaxVoters::get() / 2);
+ Ok(())
+ }
- // remove any previous stuff.
+ #[benchmark]
+ fn clean_defunct_voters(
+ // Total number of voters.
+ v: Linear<{ T::MaxVoters::get() / 2 }, { T::MaxVoters::get() }>,
+ // Those that are defunct and need removal.
+ d: Linear<0, { T::MaxVoters::get() / 2 }>,
+ ) -> Result<(), BenchmarkError> {
+ // Remove any previous stuff.
clean::<T>();
let all_candidates = submit_candidates::<T>(T::MaxCandidates::get(), "candidates")?;
distribute_voters::<T>(all_candidates, v, T::MaxVotesPerVoter::get() as usize)?;
- // all candidates leave.
+ // All candidates leave.
Candidates::<T>::kill();
- // now everyone is defunct
- assert!(Voting::<T>::iter().all(|(_, v)| Elections::<T>::is_defunct_voter(&v.votes)));
+ // Now everyone is defunct.
+ assert!(Voting::<T>::iter().all(|(_, v)| Pallet::<T>::is_defunct_voter(&v.votes)));
assert_eq!(Voting::<T>::iter().count() as u32, v);
- let root = RawOrigin::Root;
- }: _(root, v, d)
- verify {
+
+ #[extrinsic_call]
+ _(RawOrigin::Root, v, d);
+
assert_eq!(Voting::<T>::iter().count() as u32, v - d);
+
+ Ok(())
}
- election_phragmen {
- // This is just to focus on phragmen in the context of this module. We always select 20
- // 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 .. T::MaxCandidates::get();
- let v in 1 .. T::MaxVoters::get();
- let e in (T::MaxVoters::get()) .. T::MaxVoters::get() * T::MaxVotesPerVoter::get();
+ #[benchmark]
+ fn election_phragmen(
+ // This is just to focus on phragmen in the context of this module.
+ // We always select 20 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.
+ c: Linear<1, { T::MaxCandidates::get() }>,
+ v: Linear<1, { T::MaxVoters::get() }>,
+ e: Linear<{ T::MaxVoters::get() }, { T::MaxVoters::get() * T::MaxVotesPerVoter::get() }>,
+ ) -> Result<(), BenchmarkError> {
clean::<T>();
- // so we have a situation with v and e. we want e to basically always be in the range of `e
- // -> 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.
+ // So we have a situation with `v` and `e`.
+ // We want `e` to basically always be in the range of
+ // `e -> 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)?;
- }: {
- Elections::<T>::on_initialize(T::TermDuration::get());
- }
- verify {
+ let _ =
+ distribute_voters::<T>(all_candidates, v.saturating_sub(c), votes_per_voter as usize)?;
+
+ #[block]
+ {
+ Pallet::<T>::on_initialize(T::TermDuration::get());
+ }
+
assert_eq!(Members::<T>::get().len() as u32, T::DesiredMembers::get().min(c));
assert_eq!(
RunnersUp::<T>::get().len() as u32,
T::DesiredRunnersUp::get().min(c.saturating_sub(T::DesiredMembers::get())),
);
+ // reset members in between benchmark tests.
#[cfg(test)]
- {
- // reset members in between benchmark tests.
- use crate::tests::MEMBERS;
- MEMBERS.with(|m| *m.borrow_mut() = vec![]);
- }
+ MEMBERS.with(|m| *m.borrow_mut() = vec![]);
+
+ Ok(())
}
- impl_benchmark_test_suite!(
- Elections,
- crate::tests::ExtBuilder::default().desired_members(13).desired_runners_up(7),
- crate::tests::Test,
+ impl_benchmark_test_suite! {
+ Pallet,
+ tests::ExtBuilder::default().desired_members(13).desired_runners_up(7),
+ tests::Test,
exec_name = build_and_execute,
- );
+ }
}