From 7a8d59199eeda3c1d62fed6588555aa11e4fb3bc Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Fri, 15 May 2020 14:07:12 +0200 Subject: [PATCH] Benchmarks for elections-phragmen pallet (#5845) * Fist benchmark barely working * Debug checkpoint * add rest of benchmarks * Add to runtime * Fix build * Update frame/elections-phragmen/src/benchmarking.rs Co-authored-by: Shawn Tabrizi * Update frame/elections-phragmen/src/benchmarking.rs Co-authored-by: Shawn Tabrizi * major imp * Make them run on release * Help finish phragmen benchmarks (#5886) * update caller, account, and member/runner-up creation * remove stuff * ocd * make it work with real run * relax the numbers a bit * New and improved version * Make elections-phragmen weighable and secure. (#5949) * Make elections-phragmen weighable. * Update frame/elections-phragmen/src/lib.rs Co-authored-by: Alexander Popiak * Update frame/elections-phragmen/src/lib.rs Co-authored-by: Alexander Popiak * Fix all tests * Fix everything * Add note Co-authored-by: Alexander Popiak * Doc update * Fix some complexity params * Once more ready to benchmark * ready for bench * final tunes * Update frame/elections-phragmen/src/lib.rs * Fix fix * Update frame/elections-phragmen/src/lib.rs * Update frame/elections-phragmen/src/benchmarking.rs Co-authored-by: Alexander Popiak * Update frame/elections-phragmen/src/benchmarking.rs Co-authored-by: Alexander Popiak * Update to latest weights * Some fixes * Fix dual voter read from @thiolliere * Remove todos * review from @shawntabrizi * Fix bench tests. Co-authored-by: Shawn Tabrizi Co-authored-by: Alexander Popiak --- substrate/Cargo.lock | 2 +- substrate/bin/node/runtime/src/lib.rs | 1 + substrate/frame/benchmarking/src/lib.rs | 4 + substrate/frame/elections-phragmen/Cargo.toml | 8 +- .../elections-phragmen/src/benchmarking.rs | 600 ++++++++ substrate/frame/elections-phragmen/src/lib.rs | 1316 +++++++++++------ substrate/frame/support/src/lib.rs | 15 +- substrate/frame/timestamp/src/benchmarking.rs | 2 + substrate/primitives/runtime/src/lib.rs | 22 +- 9 files changed, 1490 insertions(+), 480 deletions(-) create mode 100644 substrate/frame/elections-phragmen/src/benchmarking.rs diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index f0bdcef29b..d4dc8ac49b 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -4171,11 +4171,11 @@ dependencies = [ name = "pallet-elections-phragmen" version = "2.0.0-dev" dependencies = [ + "frame-benchmarking", "frame-support", "frame-system", "hex-literal", "pallet-balances", - "pallet-scheduler", "parity-scale-codec", "serde", "sp-core", diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 32ba8c94db..12c105e940 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -982,6 +982,7 @@ impl_runtime_apis! { add_benchmark!(params, batches, b"balances", Balances); add_benchmark!(params, batches, b"collective", Council); add_benchmark!(params, batches, b"democracy", Democracy); + add_benchmark!(params, batches, b"elections", Elections); add_benchmark!(params, batches, b"identity", Identity); add_benchmark!(params, batches, b"im-online", ImOnline); add_benchmark!(params, batches, b"offences", OffencesBench::); diff --git a/substrate/frame/benchmarking/src/lib.rs b/substrate/frame/benchmarking/src/lib.rs index 27966545f7..ae9ef90764 100644 --- a/substrate/frame/benchmarking/src/lib.rs +++ b/substrate/frame/benchmarking/src/lib.rs @@ -922,6 +922,10 @@ macro_rules! impl_benchmark_tests { let selected_benchmark = SelectedBenchmark::$name; let components = >::components(&selected_benchmark); + assert!( + components.len() != 0, + "You need to add components to your benchmark!", + ); for (_, (name, low, high)) in components.iter().enumerate() { // Test only the low and high value, assuming values in the middle won't break for component_value in vec![low, high] { diff --git a/substrate/frame/elections-phragmen/Cargo.toml b/substrate/frame/elections-phragmen/Cargo.toml index 4fe85e7733..f9a3ec0b21 100644 --- a/substrate/frame/elections-phragmen/Cargo.toml +++ b/substrate/frame/elections-phragmen/Cargo.toml @@ -19,12 +19,12 @@ sp-phragmen = { version = "2.0.0-dev", default-features = false, path = "../../p frame-support = { version = "2.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "2.0.0-dev", default-features = false, path = "../system" } sp-std = { version = "2.0.0-dev", default-features = false, path = "../../primitives/std" } +frame-benchmarking = { version = "2.0.0-dev", default-features = false, path = "../benchmarking", optional = true } [dev-dependencies] sp-io = { version = "2.0.0-dev", path = "../../primitives/io" } hex-literal = "0.2.1" pallet-balances = { version = "2.0.0-dev", path = "../balances" } -pallet-scheduler = { version = "2.0.0-dev", path = "../scheduler" } sp-core = { version = "2.0.0-dev", path = "../../primitives/core" } substrate-test-utils = { version = "2.0.0-dev", path = "../../test-utils" } @@ -39,4 +39,8 @@ std = [ "frame-system/std", "sp-std/std", ] -runtime-benchmarks = ["frame-support/runtime-benchmarks"] +runtime-benchmarks = [ + "frame-benchmarking", + "frame-support/runtime-benchmarks", + "frame-system/runtime-benchmarks", +] diff --git a/substrate/frame/elections-phragmen/src/benchmarking.rs b/substrate/frame/elections-phragmen/src/benchmarking.rs new file mode 100644 index 0000000000..6de9ad57e2 --- /dev/null +++ b/substrate/frame/elections-phragmen/src/benchmarking.rs @@ -0,0 +1,600 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Elections-Phragmen pallet benchmarking. + +#![cfg(feature = "runtime-benchmarks")] + +use super::*; + +use frame_system::RawOrigin; +use frame_benchmarking::{benchmarks, account}; +use frame_support::traits::OnInitialize; + +use crate::Module as Elections; + +const BALANCE_FACTOR: u32 = 250; +const MAX_VOTERS: u32 = 500; +const MAX_CANDIDATES: u32 = 100; + +type Lookup = <::Lookup as StaticLookup>::Source; + +/// grab new account with infinite balance. +fn endowed_account(name: &'static str, index: u32) -> T::AccountId { + let account: T::AccountId = account(name, index, 0); + let amount = default_stake::(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. + T::Currency::issue(amount); + account +} + +/// Account to lookup type of system trait. +fn as_lookup(account: T::AccountId) -> Lookup { + T::Lookup::unlookup(account) +} + +/// Get a reasonable amount of stake based on the execution trait's configuration +fn default_stake(factor: u32) -> BalanceOf { + let factor = BalanceOf::::from(factor); + T::Currency::minimum_balance() * factor +} + +/// Get the current number of candidates. +fn candidate_count() -> u32 { + >::decode_len().unwrap_or(0usize) as u32 +} + +/// Get the number of votes of a voter. +fn vote_count_of(who: &T::AccountId) -> u32 { + >::get(who).1.len() as u32 +} + +/// A `DefunctVoter` struct with correct value +fn defunct_for(who: T::AccountId) -> DefunctVoter> { + DefunctVoter { + who: as_lookup::(who.clone()), + candidate_count: candidate_count::(), + vote_count: vote_count_of::(&who), + } +} + +/// Add `c` new candidates. +fn submit_candidates(c: u32, prefix: &'static str) + -> Result, &'static str> +{ + (0..c).map(|i| { + let account = endowed_account::(prefix, i); + >::submit_candidacy( + RawOrigin::Signed(account.clone()).into(), + candidate_count::(), + ).map_err(|_| "failed to submit candidacy")?; + Ok(account) + }).collect::>() +} + +/// Add `c` new candidates with self vote. +fn submit_candidates_with_self_vote(c: u32, prefix: &'static str) + -> Result, &'static str> +{ + let candidates = submit_candidates::(c, prefix)?; + let stake = default_stake::(BALANCE_FACTOR); + let _ = candidates.iter().map(|c| + submit_voter::(c.clone(), vec![c.clone()], stake) + ).collect::>()?; + Ok(candidates) +} + + +/// Submit one voter. +fn submit_voter(caller: T::AccountId, votes: Vec, stake: BalanceOf) + -> Result<(), &'static str> +{ + >::vote(RawOrigin::Signed(caller).into(), votes, stake) + .map_err(|_| "failed to submit vote") +} + +/// create `num_voter` voters who randomly vote for at most `votes` of `all_candidates` if +/// available. +fn distribute_voters(mut all_candidates: Vec, num_voters: u32, votes: usize) + -> Result<(), &'static str> +{ + let stake = default_stake::(BALANCE_FACTOR); + for i in 0..num_voters { + // to ensure that votes are different + all_candidates.rotate_left(1); + let votes = all_candidates + .iter() + .cloned() + .take(votes) + .collect::>(); + let voter = endowed_account::("voter", i); + submit_voter::(voter, votes, stake)?; + } + 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. +fn fill_seats_up_to(m: u32) -> Result, &'static str> { + let candidates = submit_candidates_with_self_vote::(m, "fill_seats_up_to")?; + assert_eq!(>::candidates().len() as u32, m, "wrong number of candidates."); + >::do_phragmen(); + assert_eq!(>::candidates().len(), 0, "some candidates remaining."); + assert_eq!( + >::members().len() + >::runners_up().len(), + m as usize, + "wrong number of members and runners-up", + ); + Ok(candidates) +} + +/// removes all the storage items to reverse any genesis state. +fn clean() { + >::kill(); + >::kill(); + >::kill(); + let _ = >::drain(); +} + +benchmarks! { + _ { + // User account seed + let u in 0 .. 1000 => (); + } + + // -- Signed ones + vote { + let u in ...; + // we fix the number of voted candidates to max + let v = MAXIMUM_VOTE; + clean::(); + + // create a bunch of candidates. + let all_candidates = submit_candidates::(MAXIMUM_VOTE as u32, "candidates")?; + + let caller = endowed_account::("caller", u); + let stake = default_stake::(BALANCE_FACTOR); + + // vote for all of them. + let votes = all_candidates.into_iter().take(v).collect(); + + }: _(RawOrigin::Signed(caller), votes, stake) + + vote_update { + let u in ...; + // we fix the number of voted candidates to max + let v = MAXIMUM_VOTE; + clean::(); + + // create a bunch of candidates. + let all_candidates = submit_candidates::(MAXIMUM_VOTE as u32, "candidates")?; + + let caller = endowed_account::("caller", u); + let stake = default_stake::(BALANCE_FACTOR); + + // original votes. + let mut votes = all_candidates.into_iter().take(v).collect::>(); + submit_voter::(caller.clone(), votes.clone(), stake)?; + // new votes. + votes.rotate_left(1); + }: vote(RawOrigin::Signed(caller), votes, stake) + + remove_voter { + let u in ...; + // we fix the number of voted candidates to max + let v = MAXIMUM_VOTE as u32; + clean::(); + + // create a bunch of candidates. + let all_candidates = submit_candidates::(v, "candidates")?; + + let caller = endowed_account::("caller", u); + + let stake = default_stake::(BALANCE_FACTOR); + submit_voter::(caller.clone(), all_candidates, stake)?; + + }: _(RawOrigin::Signed(caller)) + + report_defunct_voter_correct { + // number of already existing candidates that may or may not be voted by the reported + // account. + let c in 1 .. MAX_CANDIDATES; + // number of candidates that the reported voter voted for. The worse case of search here is + // basically `c * v`. + let v in 1 .. (MAXIMUM_VOTE as u32); + // 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::(); + + let stake = default_stake::(BALANCE_FACTOR); + + // create m members and runners combined. + let _ = fill_seats_up_to::(m)?; + + // create a bunch of candidates as well. + let bailing_candidates = submit_candidates::(v, "bailing_candidates")?; + let all_candidates = submit_candidates::(c, "all_candidates")?; + + // account 1 is the reporter and it doesn't matter how many it votes. But it has to be a + // voter. + let account_1 = endowed_account::("caller", 0); + submit_voter::( + account_1.clone(), + all_candidates.iter().take(1).cloned().collect(), + stake, + )?; + + // account 2 votes for all of the mentioned candidates. + let account_2 = endowed_account::("caller_2", 1); + submit_voter::( + account_2.clone(), + bailing_candidates.clone(), + stake, + )?; + + // all the bailers go away. + bailing_candidates.into_iter().for_each(|b| { + let count = candidate_count::(); + assert!(>::renounce_candidacy( + RawOrigin::Signed(b).into(), + Renouncing::Candidate(count), + ).is_ok()); + }); + let defunct = defunct_for::(account_2.clone()); + }: report_defunct_voter(RawOrigin::Signed(account_1.clone()), defunct) + verify { + assert!(>::is_voter(&account_1)); + assert!(!>::is_voter(&account_2)); + #[cfg(test)] + { + // reset members in between benchmark tests. + use crate::tests::MEMBERS; + MEMBERS.with(|m| *m.borrow_mut() = vec![]); + } + } + + report_defunct_voter_incorrect { + // number of already existing candidates that may or may not be voted by the reported + // account. + let c in 1 .. MAX_CANDIDATES; + // number of candidates that the reported voter voted for. The worse case of search here is + // basically `c * v`. + let v in 1 .. (MAXIMUM_VOTE as u32); + // 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::(); + let stake = default_stake::(BALANCE_FACTOR); + + // create m members and runners combined. + let _ = fill_seats_up_to::(m)?; + + // create a bunch of candidates as well. + let all_candidates = submit_candidates::(c, "candidates")?; + + // account 1 is the reporter and it doesn't matter how many it votes. + let account_1 = endowed_account::("caller", 0); + submit_voter::( + account_1.clone(), + all_candidates.iter().take(1).cloned().collect(), + stake, + )?; + + // account 2 votes for a bunch of crap, and finally a correct candidate. + let account_2 = endowed_account::("caller_2", 1); + let mut invalid: Vec = + (0..(v-1)).map(|seed| account::("invalid", 0, seed).clone()).collect(); + invalid.push(all_candidates.last().unwrap().clone()); + submit_voter::( + account_2.clone(), + invalid, + stake, + )?; + + let defunct = defunct_for::(account_2.clone()); + // no one bails out. account_1 is slashed and removed as voter now. + }: report_defunct_voter(RawOrigin::Signed(account_1.clone()), defunct) + verify { + assert!(!>::is_voter(&account_1)); + assert!(>::is_voter(&account_2)); + #[cfg(test)] + { + // reset members in between benchmark tests. + use crate::tests::MEMBERS; + MEMBERS.with(|m| *m.borrow_mut() = vec![]); + } + } + + submit_candidacy { + // number of already existing candidates. + let c in 1 .. MAX_CANDIDATES; + // 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::(); + let stake = default_stake::(BALANCE_FACTOR); + + // create m members and runners combined. + let _ = fill_seats_up_to::(m)?; + + // create previous candidates; + let _ = submit_candidates::(c, "candidates")?; + + // we assume worse case that: extrinsic is successful and candidate is not duplicate. + let candidate_account = endowed_account::("caller", 0); + }: _(RawOrigin::Signed(candidate_account.clone()), candidate_count::()) + verify { + #[cfg(test)] + { + // reset members in between benchmark tests. + use crate::tests::MEMBERS; + MEMBERS.with(|m| *m.borrow_mut() = vec![]); + } + } + + 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 .. MAX_CANDIDATES; + // 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::(); + + // create m members and runners combined. + let _ = fill_seats_up_to::(m)?; + let all_candidates = submit_candidates::(c, "caller")?; + + let bailing = all_candidates[0].clone(); // Should be ("caller", 0) + let count = candidate_count::(); + }: renounce_candidacy(RawOrigin::Signed(bailing), Renouncing::Candidate(count)) + verify { + #[cfg(test)] + { + // reset members in between benchmark tests. + use crate::tests::MEMBERS; + MEMBERS.with(|m| *m.borrow_mut() = vec![]); + } + } + + renounce_candidacy_member_runner_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. + let u in ...; + let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get(); + clean::(); + + // create m members and runners combined. + let members_and_runners_up = fill_seats_up_to::(m)?; + + let bailing = members_and_runners_up[0].clone(); + let renouncing = if >::is_member(&bailing) { + Renouncing::Member + } else if >::is_runner_up(&bailing) { + Renouncing::RunnerUp + } else { + panic!("Bailing must be a member or runner-up for this bench to be sane."); + }; + }: renounce_candidacy(RawOrigin::Signed(bailing.clone()), renouncing) + verify { + #[cfg(test)] + { + // reset members in between benchmark tests. + use crate::tests::MEMBERS; + MEMBERS.with(|m| *m.borrow_mut() = vec![]); + } + } + + // -- Root ones + remove_member_without_replacement { + // worse case is when we remove a member and we have no runner as a replacement. This + // triggers phragmen again. The only parameter is how many candidates will compete for the + // new slot. + let c in 1 .. MAX_CANDIDATES; + clean::(); + + // fill only desired members. no runners-up. + let all_members = fill_seats_up_to::(T::DesiredMembers::get())?; + assert_eq!(>::members().len() as u32, T::DesiredMembers::get()); + + // submit a new one to compensate, with self-vote. + let replacements = submit_candidates_with_self_vote::(c, "new_candidate")?; + + // create some voters for these replacements. + distribute_voters::(replacements, MAX_VOTERS, MAXIMUM_VOTE)?; + + let to_remove = as_lookup::(all_members[0].clone()); + }: remove_member(RawOrigin::Root, to_remove, false) + verify { + // must still have the desired number of members members. + assert_eq!(>::members().len() as u32, T::DesiredMembers::get()); + #[cfg(test)] + { + // reset members in between benchmark tests. + use crate::tests::MEMBERS; + MEMBERS.with(|m| *m.borrow_mut() = vec![]); + } + } + + 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. + let u in ...; + let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get(); + clean::(); + + let _ = fill_seats_up_to::(m)?; + let removing = as_lookup::(>::members_ids()[0].clone()); + }: remove_member(RawOrigin::Root, removing, true) + verify { + // must still have enough members. + assert_eq!(>::members().len() as u32, T::DesiredMembers::get()); + #[cfg(test)] + { + // reset members in between benchmark tests. + use crate::tests::MEMBERS; + MEMBERS.with(|m| *m.borrow_mut() = vec![]); + } + } + + remove_member_wrong_refund { + // The root call by mistake indicated that this will have no replacement, while it has! + // this has now consumed a lot of weight and need to refund. + let u in ...; + let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get(); + clean::(); + + let _ = fill_seats_up_to::(m)?; + let removing = as_lookup::(>::members_ids()[0].clone()); + }: { + assert_eq!( + >::remove_member(RawOrigin::Root.into(), removing, false).unwrap_err().error, + Error::::InvalidReplacement.into(), + ); + } + verify { + // must still have enough members. + assert_eq!(>::members().len() as u32, T::DesiredMembers::get()); + #[cfg(test)] + { + // reset members in between benchmark tests. + use crate::tests::MEMBERS; + MEMBERS.with(|m| *m.borrow_mut() = vec![]); + } + } + + on_initialize { + // if n % TermDuration is zero, then we run phragmen. The weight function must and should + // check this as it is cheap to do so. TermDuration is not a storage item, it is a constant + // encoded in the runtime. + let c in 1 .. MAX_CANDIDATES; + clean::(); + + // create c candidates. + let all_candidates = submit_candidates_with_self_vote::(c, "candidates")?; + // create 500 voters, each voting the maximum 16 + distribute_voters::(all_candidates, MAX_VOTERS, MAXIMUM_VOTE)?; + }: { + // elect + >::on_initialize(T::TermDuration::get()); + } + verify { + assert_eq!(>::members().len() as u32, T::DesiredMembers::get().min(c)); + assert_eq!( + >::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![]); + } + } + + 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 .. MAX_CANDIDATES; + let v in 1 .. MAX_VOTERS; + let e in 1 .. (MAXIMUM_VOTE as u32); + clean::(); + + let all_candidates = submit_candidates_with_self_vote::(c, "candidates")?; + let _ = distribute_voters::(all_candidates, v, e as usize)?; + }: { + >::on_initialize(T::TermDuration::get()); + } + verify { + assert_eq!(>::members().len() as u32, T::DesiredMembers::get().min(c)); + assert_eq!( + >::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![]); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests::{ExtBuilder, Test}; + use frame_support::assert_ok; + + #[test] + fn test_benchmarks_elections_phragmen() { + ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { + assert_ok!(test_benchmark_vote::()); + }); + + ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { + assert_ok!(test_benchmark_remove_voter::()); + }); + + ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { + assert_ok!(test_benchmark_report_defunct_voter_correct::()); + }); + + ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { + assert_ok!(test_benchmark_report_defunct_voter_incorrect::()); + }); + + ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { + assert_ok!(test_benchmark_submit_candidacy::()); + }); + + ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { + assert_ok!(test_benchmark_renounce_candidacy_candidate::()); + }); + + ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { + assert_ok!(test_benchmark_renounce_candidacy_member_runner_up::()); + }); + + ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { + assert_ok!(test_benchmark_remove_member_without_replacement::()); + }); + + ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { + assert_ok!(test_benchmark_remove_member_with_replacement::()); + }); + + ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { + assert_ok!(test_benchmark_on_initialize::()); + }); + + ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { + assert_ok!(test_benchmark_phragmen::()); + }); + } +} diff --git a/substrate/frame/elections-phragmen/src/lib.rs b/substrate/frame/elections-phragmen/src/lib.rs index 0133abc648..4e2ae09afa 100644 --- a/substrate/frame/elections-phragmen/src/lib.rs +++ b/substrate/frame/elections-phragmen/src/lib.rs @@ -83,14 +83,17 @@ #![cfg_attr(not(feature = "std"), no_std)] +use codec::{Encode, Decode}; use sp_std::prelude::*; use sp_runtime::{ - print, DispatchResult, DispatchError, Perbill, traits::{Zero, StaticLookup, Convert}, + DispatchError, RuntimeDebug, Perbill, + traits::{Zero, StaticLookup, Convert}, }; use frame_support::{ decl_storage, decl_event, ensure, decl_module, decl_error, - weights::{Weight, DispatchClass}, + weights::{Weight, constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}}, storage::{StorageMap, IterableStorageMap}, + dispatch::{DispatchResultWithPostInfo, WithPostDispatchInfo}, traits::{ Currency, Get, LockableCurrency, LockIdentifier, ReservableCurrency, WithdrawReasons, ChangeMembers, OnUnbalanced, WithdrawReason, Contains, BalanceStatus, InitializeMembers, @@ -100,13 +103,40 @@ use frame_support::{ use sp_phragmen::{build_support_map, ExtendedBalance, VoteWeight, PhragmenResult}; use frame_system::{self as system, ensure_signed, ensure_root}; +mod benchmarking; + /// The maximum votes allowed per voter. pub const MAXIMUM_VOTE: usize = 16; -type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; +type BalanceOf = + <::Currency as Currency<::AccountId>>::Balance; type NegativeImbalanceOf = <::Currency as Currency<::AccountId>>::NegativeImbalance; +/// An indication that the renouncing account currently has which of the below roles. +#[derive(Encode, Decode, Clone, PartialEq, RuntimeDebug)] +pub enum Renouncing { + /// A member is renouncing. + Member, + /// A runner-up is renouncing. + RunnerUp, + /// A candidate is renouncing, while the given total number of candidates exists. + Candidate(#[codec(compact)] u32), +} + +/// Information needed to prove the defunct-ness of a voter. +#[derive(Encode, Decode, Clone, PartialEq, RuntimeDebug)] +pub struct DefunctVoter { + /// the voter's who's being challenged for being defunct + pub who: AccountId, + /// The number of votes that `who` has placed. + #[codec(compact)] + pub vote_count: u32, + /// The number of current active candidates. + #[codec(compact)] + pub candidate_count: u32 +} + pub trait Trait: frame_system::Trait { /// The overarching event type.c type Event: From> + Into<::Event>; @@ -239,10 +269,16 @@ decl_error! { RunnerSubmit, /// Candidate does not have enough funds. InsufficientCandidateFunds, - /// Origin is not a candidate, member or a runner up. - InvalidOrigin, /// Not a member. NotMember, + /// The provided count of number of candidates is incorrect. + InvalidCandidateCount, + /// The provided count of number of votes is incorrect. + InvalidVoteCount, + /// The renouncing origin presented a wrong `Renouncing` parameter. + InvalidRenouncing, + /// Prediction regarding replacement after member removal is wrong. + InvalidReplacement, } } @@ -259,46 +295,60 @@ decl_module! { const TermDuration: T::BlockNumber = T::TermDuration::get(); const ModuleId: LockIdentifier = T::ModuleId::get(); - /// Vote for a set of candidates for the upcoming round of election. + /// Vote for a set of candidates for the upcoming round of election. This can be called to + /// set the initial votes, or update already existing votes. + /// + /// Upon initial voting, `value` units of `who`'s balance is locked and a bond amount is + /// reserved. /// /// The `votes` should: /// - not be empty. - /// - be less than the number of candidates. + /// - be less than the number of possible candidates. Note that all current members and + /// runners-up are also automatically candidates for the next round. /// - /// Upon voting, `value` units of `who`'s balance is locked and a bond amount is reserved. /// It is the responsibility of the caller to not place all of their balance into the lock /// and keep some for further transactions. /// /// # - /// #### State - /// Reads: O(1) - /// Writes: O(V) given `V` votes. V is bounded by 16. + /// Base weight: 47.93 µs + /// State reads: + /// - Candidates.len() + Members.len() + RunnersUp.len() + /// - Voting (is_voter) + /// - [AccountBalance(who) (unreserve + total_balance)] + /// State writes: + /// - Voting + /// - Lock + /// - [AccountBalance(who) (unreserve -- only when creating a new voter)] /// # - #[weight = 100_000_000] - fn vote(origin, votes: Vec, #[compact] value: BalanceOf) { + #[weight = 50 * WEIGHT_PER_MICROS + T::DbWeight::get().reads_writes(4, 2)] + fn vote( + origin, + votes: Vec, + #[compact] value: BalanceOf, + ) { let who = ensure_signed(origin)?; + ensure!(votes.len() <= MAXIMUM_VOTE, Error::::MaximumVotesExceeded); + ensure!(!votes.is_empty(), Error::::NoVotes); + let candidates_count = >::decode_len().unwrap_or(0); let members_count = >::decode_len().unwrap_or(0); let runners_up_count = >::decode_len().unwrap_or(0); + // addition is valid: candidates, members and runners-up will never overlap. let allowed_votes = candidates_count + members_count + runners_up_count; ensure!(!allowed_votes.is_zero(), Error::::UnableToVote); ensure!(votes.len() <= allowed_votes, Error::::TooManyVotes); - ensure!(votes.len() <= MAXIMUM_VOTE, Error::::MaximumVotesExceeded); - ensure!(!votes.is_empty(), Error::::NoVotes); - ensure!( - value > T::Currency::minimum_balance(), - Error::::LowBalance, - ); + ensure!(value > T::Currency::minimum_balance(), Error::::LowBalance); + // first time voter. Reserve bond. if !Self::is_voter(&who) { - // first time voter. Reserve bond. T::Currency::reserve(&who, T::VotingBond::get()) .map_err(|_| Error::::UnableToPayBond)?; } + // Amount to be locked up. let locked_balance = value.min(T::Currency::total_balance(&who)); @@ -316,14 +366,19 @@ decl_module! { /// Remove `origin` as a voter. This removes the lock and returns the bond. /// /// # - /// #### State - /// Reads: O(1) - /// Writes: O(1) + /// Base weight: 36.8 µs + /// All state access is from do_remove_voter. + /// State reads: + /// - Voting + /// - [AccountData(who)] + /// State writes: + /// - Voting + /// - Locks + /// - [AccountData(who)] /// # - #[weight = 0] + #[weight = 35 * WEIGHT_PER_MICROS + T::DbWeight::get().reads_writes(1, 2)] fn remove_voter(origin) { let who = ensure_signed(origin)?; - ensure!(Self::is_voter(&who), Error::::MustBeVoter); Self::do_remove_voter(&who, true); @@ -335,27 +390,62 @@ decl_module! { /// /// A defunct voter is defined to be: /// - a voter whose current submitted votes are all invalid. i.e. all of them are no - /// longer a candidate nor an active member. + /// longer a candidate nor an active member or a runner-up. + /// + /// + /// The origin must provide the number of current candidates and votes of the reported target + /// for the purpose of accurate weight calculation. /// /// # - /// #### State - /// Reads: O(NLogM) given M current candidates and N votes for `target`. - /// Writes: O(1) + /// No Base weight based on min square analysis. + /// Complexity of candidate_count: 1.755 µs + /// Complexity of vote_count: 18.51 µs + /// State reads: + /// - Voting(reporter) + /// - Candidate.len() + /// - Voting(Target) + /// - Candidates, Members, RunnersUp (is_defunct_voter) + /// State writes: + /// - Lock(reporter || target) + /// - [AccountBalance(reporter)] + AccountBalance(target) + /// - Voting(reporter || target) + /// Note: the db access is worse with respect to db, which is when the report is correct. /// # - #[weight = 1_000_000_000] - fn report_defunct_voter(origin, target: ::Source) { + #[weight = + Weight::from(defunct.candidate_count).saturating_mul(2 * WEIGHT_PER_MICROS) + .saturating_add(Weight::from(defunct.vote_count).saturating_mul(19 * WEIGHT_PER_MICROS)) + .saturating_add(T::DbWeight::get().reads_writes(6, 3)) + ] + fn report_defunct_voter( + origin, + defunct: DefunctVoter<::Source>, + ) { let reporter = ensure_signed(origin)?; - let target = T::Lookup::lookup(target)?; + let target = T::Lookup::lookup(defunct.who)?; ensure!(reporter != target, Error::::ReportSelf); ensure!(Self::is_voter(&reporter), Error::::MustBeVoter); - // Checking if someone is a candidate and a member here is O(LogN), making the whole - // function O(MLonN) with N candidates in total and M of them being voted by `target`. - // We could easily add another mapping to be able to check if someone is a candidate in - // `O(1)` but that would make the process of removing candidates at the end of each - // round slightly harder. Note that for now we have a bound of number of votes (`N`). - let valid = Self::is_defunct_voter(&target); + let DefunctVoter { candidate_count, vote_count, .. } = defunct; + + ensure!( + >::decode_len().unwrap_or(0) as u32 <= candidate_count, + Error::::InvalidCandidateCount, + ); + + let (_, votes) = >::get(&target); + // indirect way to ensure target is a voter. We could call into `::contains()`, but it + // would have the same effect with one extra db access. Note that votes cannot be + // submitted with length 0. Hence, a non-zero length means that the target is a voter. + ensure!(votes.len() > 0, Error::::MustBeVoter); + + // ensure that the size of votes that need to be searched is correct. + ensure!( + votes.len() as u32 <= vote_count, + Error::::InvalidVoteCount, + ); + + let valid = Self::is_defunct_voter(&votes); if valid { // reporter will get the voting bond of the target T::Currency::repatriate_reserved(&target, &reporter, T::VotingBond::get(), BalanceStatus::Free)?; @@ -371,7 +461,6 @@ decl_module! { Self::deposit_event(RawEvent::VoterReported(target, reporter, valid)); } - /// Submit oneself for candidacy. /// /// A candidate will either: @@ -381,21 +470,40 @@ decl_module! { /// removed. /// /// # - /// #### State - /// Reads: O(LogN) Given N candidates. - /// Writes: O(1) + /// Base weight = 33.33 µs + /// Complexity of candidate_count: 0.375 µs + /// State reads: + /// - Candidates.len() + /// - Candidates + /// - Members + /// - RunnersUp + /// - [AccountBalance(who)] + /// State writes: + /// - [AccountBalance(who)] + /// - Candidates /// # - #[weight = 500_000_000] - fn submit_candidacy(origin) { + #[weight = + (35 * WEIGHT_PER_MICROS) + .saturating_add(Weight::from(*candidate_count).saturating_mul(375 * WEIGHT_PER_NANOS)) + .saturating_add(T::DbWeight::get().reads_writes(4, 1)) + ] + fn submit_candidacy(origin, #[compact] candidate_count: u32) { let who = ensure_signed(origin)?; + let actual_count = >::decode_len().unwrap_or(0); + ensure!( + actual_count as u32 <= candidate_count, + Error::::InvalidCandidateCount, + ); + let is_candidate = Self::is_candidate(&who); ensure!(is_candidate.is_err(), Error::::DuplicatedCandidate); + // assured to be an error, error always contains the index. let index = is_candidate.unwrap_err(); ensure!(!Self::is_member(&who), Error::::MemberSubmit); - ensure!(!Self::is_runner(&who), Error::::RunnerSubmit); + ensure!(!Self::is_runner_up(&who), Error::::RunnerSubmit); T::Currency::reserve(&who, T::CandidacyBond::get()) .map_err(|_| Error::::InsufficientCandidateFunds)?; @@ -407,55 +515,93 @@ decl_module! { /// outcomes exist: /// - `origin` is a candidate and not elected in any set. In this case, the bond is /// unreserved, returned and origin is removed as a candidate. - /// - `origin` is a current runner up. In this case, the bond is unreserved, returned and - /// origin is removed as a runner. + /// - `origin` is a current runner-up. In this case, the bond is unreserved, returned and + /// origin is removed as a runner-up. /// - `origin` is a current member. In this case, the bond is unreserved and origin is /// removed as a member, consequently not being a candidate for the next round anymore. /// Similar to [`remove_voter`], if replacement runners exists, they are immediately used. - #[weight = (2_000_000_000, DispatchClass::Operational)] - fn renounce_candidacy(origin) { + /// + /// If a candidate is renouncing: + /// Base weight: 17.28 µs + /// Complexity of candidate_count: 0.235 µs + /// State reads: + /// - Candidates + /// - [AccountBalance(who) (unreserve)] + /// State writes: + /// - Candidates + /// - [AccountBalance(who) (unreserve)] + /// If member is renouncing: + /// Base weight: 46.25 µs + /// State reads: + /// - Members, RunnersUp (remove_and_replace_member), + /// - [AccountData(who) (unreserve)] + /// State writes: + /// - Members, RunnersUp (remove_and_replace_member), + /// - [AccountData(who) (unreserve)] + /// If runner is renouncing: + /// Base weight: 46.25 µs + /// State reads: + /// - RunnersUp (remove_and_replace_member), + /// - [AccountData(who) (unreserve)] + /// State writes: + /// - RunnersUp (remove_and_replace_member), + /// - [AccountData(who) (unreserve)] + /// + /// Weight note: The call into changeMembers need to be accounted for. + /// + #[weight = match *renouncing { + Renouncing::Candidate(count) => { + (18 * WEIGHT_PER_MICROS) + .saturating_add(Weight::from(count).saturating_mul(235 * WEIGHT_PER_NANOS)) + .saturating_add(T::DbWeight::get().reads_writes(1, 1)) + }, + Renouncing::Member => { + 46 * WEIGHT_PER_MICROS + + T::DbWeight::get().reads_writes(2, 2) + }, + Renouncing::RunnerUp => { + 46 * WEIGHT_PER_MICROS + + T::DbWeight::get().reads_writes(1, 1) + } + }] + fn renounce_candidacy(origin, renouncing: Renouncing) { let who = ensure_signed(origin)?; - - // NOTE: this function attempts the 3 conditions (being a candidate, member, runner) and - // fails if none are matched. Unlike other Palette functions and modules where checks - // happen first and then execution happens, this function is written the other way - // around. The main intention is that reading all of the candidates, members and runners - // from storage is expensive. Furthermore, we know (soft proof) that they are always - // mutually exclusive. Hence, we try one, and only then decode more storage. - - if let Ok(_replacement) = Self::remove_and_replace_member(&who) { - T::Currency::unreserve(&who, T::CandidacyBond::get()); - Self::deposit_event(RawEvent::MemberRenounced(who.clone())); - - // safety guard to make sure we do only one arm. Better to read runners later. - return Ok(()); - } - - let mut runners_up_with_stake = Self::runners_up(); - if let Some(index) = runners_up_with_stake.iter() - .position(|(ref r, ref _s)| r == &who) - { - runners_up_with_stake.remove(index); - // unreserve the bond - T::Currency::unreserve(&who, T::CandidacyBond::get()); - // update storage. - >::put(runners_up_with_stake); - // safety guard to make sure we do only one arm. Better to read runners later. - return Ok(()); - } - - let mut candidates = Self::candidates(); - if let Ok(index) = candidates.binary_search(&who) { - candidates.remove(index); - // unreserve the bond - T::Currency::unreserve(&who, T::CandidacyBond::get()); - // update storage. - >::put(candidates); - // safety guard to make sure we do only one arm. Better to read runners later. - return Ok(()); - } - - Err(Error::::InvalidOrigin)? + match renouncing { + Renouncing::Member => { + // returns NoMember error in case of error. + let _ = Self::remove_and_replace_member(&who)?; + T::Currency::unreserve(&who, T::CandidacyBond::get()); + Self::deposit_event(RawEvent::MemberRenounced(who.clone())); + }, + Renouncing::RunnerUp => { + let mut runners_up_with_stake = Self::runners_up(); + if let Some(index) = runners_up_with_stake + .iter() + .position(|(ref r, ref _s)| r == &who) + { + runners_up_with_stake.remove(index); + // unreserve the bond + T::Currency::unreserve(&who, T::CandidacyBond::get()); + // update storage. + >::put(runners_up_with_stake); + } else { + Err(Error::::InvalidRenouncing)?; + } + } + Renouncing::Candidate(count) => { + let mut candidates = Self::candidates(); + ensure!(count >= candidates.len() as u32, Error::::InvalidRenouncing); + if let Some(index) = candidates.iter().position(|x| *x == who) { + candidates.remove(index); + // unreserve the bond + T::Currency::unreserve(&who, T::CandidacyBond::get()); + // update storage. + >::put(candidates); + } else { + Err(Error::::InvalidRenouncing)?; + } + } + }; } /// Remove a particular member from the set. This is effective immediately and the bond of @@ -467,34 +613,57 @@ decl_module! { /// Note that this does not affect the designated block number of the next election. /// /// # - /// #### State - /// Reads: O(do_phragmen) - /// Writes: O(do_phragmen) + /// If we have a replacement: + /// - Base weight: 50.93 µs + /// - State reads: + /// - RunnersUp.len() + /// - Members, RunnersUp (remove_and_replace_member) + /// - State writes: + /// - Members, RunnersUp (remove_and_replace_member) + /// Else, since this is a root call and will go into phragmen, we assume full block for now. /// # - #[weight = (2_000_000_000, DispatchClass::Operational)] - fn remove_member(origin, who: ::Source) -> DispatchResult { + #[weight = if *has_replacement { + 50 * WEIGHT_PER_MICROS + T::DbWeight::get().reads_writes(3, 2) + } else { + T::MaximumBlockWeight::get() + }] + fn remove_member( + origin, + who: ::Source, + has_replacement: bool, + ) -> DispatchResultWithPostInfo { ensure_root(origin)?; let who = T::Lookup::lookup(who)?; + let will_have_replacement = >::decode_len().unwrap_or(0) > 0; + if will_have_replacement != has_replacement { + // In both cases, we will change more weight than neede. Refund and abort. + return Err(Error::::InvalidReplacement.with_weight( + // refund. The weight value comes from a benchmark which is special to this. + // 5.751 µs + 6 * WEIGHT_PER_MICROS + T::DbWeight::get().reads_writes(1, 0) + )); + } // else, prediction was correct. + Self::remove_and_replace_member(&who).map(|had_replacement| { let (imbalance, _) = T::Currency::slash_reserved(&who, T::CandidacyBond::get()); T::KickedMember::on_unbalanced(imbalance); Self::deposit_event(RawEvent::MemberKicked(who.clone())); if !had_replacement { + // if we end up here, we will charge a full block weight. Self::do_phragmen(); } - }) + + // no refund needed. + None.into() + }).map_err(|e| e.into()) } /// What to do at the end of each block. Checks if an election needs to happen or not. fn on_initialize(n: T::BlockNumber) -> Weight { - if let Err(e) = Self::end_block(n) { - print("Guru meditation"); - print(e); - } - - 0 + // returns the correct weight. + Self::end_block(n) } } } @@ -524,13 +693,19 @@ decl_event!( ); impl Module { - /// Attempts to remove a member `who`. If a runner up exists, it is used as the replacement. + /// Attempts to remove a member `who`. If a runner-up exists, it is used as the replacement and + /// Ok(true). is returned. + /// /// Otherwise, `Ok(false)` is returned to signal the caller. /// - /// In both cases, [`Members`], [`ElectionRounds`] and [`RunnersUp`] storage are updated - /// accordingly. Furthermore, the membership change is reported. + /// If a replacement exists, `Members` and `RunnersUp` storage is updated, where the first + /// element of `RunnersUp` is used as the replacement and `Ok(true)` is returned. Else, + /// `Ok(false)` is returned with no storage updated. /// - /// O(phragmen) in the worse case. + /// Note that this function _will_ call into `T::ChangeMembers` in case any change happens + /// (`Ok(true)`). + /// + /// If replacement exists, this will read and write from/into both `Members` and `RunnersUp`. fn remove_and_replace_member(who: &T::AccountId) -> Result { let mut members_with_stake = Self::members(); if let Ok(index) = members_with_stake.binary_search_by(|(ref m, ref _s)| m.cmp(who)) { @@ -563,7 +738,7 @@ impl Module { /// Check if `who` is a candidate. It returns the insert index if the element does not exists as /// an error. /// - /// State: O(LogN) given N candidates. + /// O(LogN) given N candidates. fn is_candidate(who: &T::AccountId) -> Result<(), usize> { Self::candidates().binary_search(who).map(|_| ()) } @@ -577,15 +752,15 @@ impl Module { /// Check if `who` is currently an active member. /// - /// Limited number of members. Binary search. Constant time factor. O(1) + /// O(LogN) given N members. Since members are limited, O(1). fn is_member(who: &T::AccountId) -> bool { Self::members().binary_search_by(|(a, _b)| a.cmp(who)).is_ok() } - /// Check if `who` is currently an active runner. + /// Check if `who` is currently an active runner-up. /// - /// Limited number of runners-up. Binary search. Constant time factor. O(1) - fn is_runner(who: &T::AccountId) -> bool { + /// O(LogN) given N runners-up. Since runners-up are limited, O(1). + fn is_runner_up(who: &T::AccountId) -> bool { Self::runners_up().iter().position(|(a, _b)| a == who).is_some() } @@ -614,25 +789,24 @@ impl Module { Self::runners_up().into_iter().map(|(r, _)| r).collect::>() } - /// Check if `who` is a defunct voter. - /// - /// Note that false is returned if `who` is not a voter at all. + /// Check if `votes` will correspond to a defunct voter. As no origin is part of the inputs, + /// this function does not check the origin at all. /// /// O(NLogM) with M candidates and `who` having voted for `N` of them. - fn is_defunct_voter(who: &T::AccountId) -> bool { - if Self::is_voter(who) { - Self::votes_of(who) - .iter() - .all(|v| !Self::is_member(v) && !Self::is_runner(v) && !Self::is_candidate(v).is_ok()) - } else { - false - } + /// Reads Members, RunnersUp, Candidates and Voting(who) from database. + fn is_defunct_voter(votes: &[T::AccountId]) -> bool { + votes.iter().all(|v| + !Self::is_member(v) && !Self::is_runner_up(v) && !Self::is_candidate(v).is_ok() + ) } /// Remove a certain someone as a voter. /// /// This will clean always clean the storage associated with the voter, and remove the balance /// lock. Optionally, it would also return the reserved voting bond if indicated by `unreserve`. + /// If unreserve is true, has 3 storage reads and 1 reads. + /// + /// DB access: Voting and Lock are always written to, if unreserve, then 1 read and write added. fn do_remove_voter(who: &T::AccountId, unreserve: bool) { // remove storage and lock. Voting::::remove(who); @@ -648,22 +822,18 @@ impl Module { Voting::::get(who).0 } - /// The locked stake of a voter. - fn votes_of(who: &T::AccountId) -> Vec { - Voting::::get(who).1 - } - /// Check there's nothing to do this block. /// /// Runs phragmen election and cleans all the previous candidate state. The voter state is NOT /// cleaned and voters must themselves submit a transaction to retract. - fn end_block(block_number: T::BlockNumber) -> DispatchResult { + fn end_block(block_number: T::BlockNumber) -> Weight { if !Self::term_duration().is_zero() { if (block_number % Self::term_duration()).is_zero() { Self::do_phragmen(); + return T::MaximumBlockWeight::get() } } - Ok(()) + 0 } /// Run the phragmen election with all required side processes and state updates. @@ -882,11 +1052,13 @@ impl ContainsLengthBound for Module { mod tests { use super::*; use std::cell::RefCell; - use frame_support::{assert_ok, assert_noop, parameter_types, weights::Weight}; + use frame_support::{assert_ok, assert_noop, assert_err_with_weight, parameter_types, + weights::Weight, + }; use substrate_test_utils::assert_eq_uvec; use sp_core::H256; use sp_runtime::{ - Perbill, testing::Header, BuildStorage, + Perbill, testing::Header, BuildStorage, DispatchResult, traits::{BlakeTwo256, IdentityLookup, Block as BlockT}, }; use crate as elections_phragmen; @@ -1001,7 +1173,7 @@ mod tests { new_plus_outgoing.extend_from_slice(outgoing); new_plus_outgoing.sort(); - assert_eq!(old_plus_incoming, new_plus_outgoing); + assert_eq!(old_plus_incoming, new_plus_outgoing, "change members call is incorrect!"); MEMBERS.with(|m| *m.borrow_mut() = new.to_vec()); PRIME.with(|p| *p.borrow_mut() = None); @@ -1065,6 +1237,7 @@ mod tests { voter_bond: u64, term_duration: u64, desired_runners_up: u32, + desired_members: u32, } impl Default for ExtBuilder { @@ -1073,8 +1246,9 @@ mod tests { genesis_members: vec![], balance_factor: 1, voter_bond: 2, - desired_runners_up: 0, term_duration: 5, + desired_runners_up: 0, + desired_members: 2, } } } @@ -1096,15 +1270,24 @@ mod tests { self.genesis_members = members; self } + #[cfg(feature = "runtime-benchmarks")] + pub fn desired_members(mut self, count: u32) -> Self { + self.desired_members = count; + self + } pub fn balance_factor(mut self, factor: u64) -> Self { self.balance_factor = factor; self } - pub fn build_and_execute(self, test: impl FnOnce() -> ()) { + fn set_constants(&self) { VOTING_BOND.with(|v| *v.borrow_mut() = self.voter_bond); TERM_DURATION.with(|v| *v.borrow_mut() = self.term_duration); DESIRED_RUNNERS_UP.with(|v| *v.borrow_mut() = self.desired_runners_up); + DESIRED_MEMBERS.with(|m| *m.borrow_mut() = self.desired_members); MEMBERS.with(|m| *m.borrow_mut() = self.genesis_members.iter().map(|(m, _)| m.clone()).collect::>()); + } + pub fn build_and_execute(self, test: impl FnOnce() -> ()) { + self.set_constants(); let mut ext: sp_io::TestExternalities = GenesisConfig { pallet_balances: Some(pallet_balances::GenesisConfig::{ balances: vec![ @@ -1186,6 +1369,33 @@ mod tests { ensure_members_has_approval_stake(); } + fn submit_candidacy(origin: Origin) -> DispatchResult { + Elections::submit_candidacy(origin, Elections::candidates().len() as u32) + } + + fn vote(origin: Origin, votes: Vec, stake: u64) -> DispatchResult { + // historical note: helper function was created in a period of time in which the API of vote + // call was changing. Currently it is a wrapper for the original call and does not do much. + // Nonetheless, totally harmless. + if let Origin::system(frame_system::RawOrigin::Signed(_account)) = origin { + Elections::vote(origin, votes, stake) + } else { + panic!("vote origin must be signed"); + } + } + + fn votes_of(who: &u64) -> Vec { + Voting::::get(who).1 + } + + fn defunct_for(who: u64) -> DefunctVoter { + DefunctVoter { + who, + candidate_count: Elections::candidates().len() as u32, + vote_count: votes_of(&who).len() as u32 + } + } + #[test] fn params_should_work() { ExtBuilder::default().build_and_execute(|| { @@ -1201,7 +1411,7 @@ mod tests { assert!(Elections::is_candidate(&1).is_err()); assert_eq!(all_voters(), vec![]); - assert_eq!(Elections::votes_of(&1), vec![]); + assert_eq!(votes_of(&1), vec![]); }); } @@ -1216,7 +1426,7 @@ mod tests { // they will persist since they have self vote. System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![1, 2]); }) @@ -1233,7 +1443,7 @@ mod tests { // they will persist since they have self vote. System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![1, 2]); }) @@ -1281,7 +1491,7 @@ mod tests { assert_eq!(Elections::candidates(), vec![]); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![]); assert_eq!(Elections::runners_up(), vec![]); @@ -1297,7 +1507,7 @@ mod tests { assert!(Elections::is_candidate(&2).is_err()); assert_eq!(balances(&1), (10, 0)); - assert_ok!(Elections::submit_candidacy(Origin::signed(1))); + assert_ok!(submit_candidacy(Origin::signed(1))); assert_eq!(balances(&1), (7, 3)); assert_eq!(Elections::candidates(), vec![1]); @@ -1306,7 +1516,7 @@ mod tests { assert!(Elections::is_candidate(&2).is_err()); assert_eq!(balances(&2), (20, 0)); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(2))); assert_eq!(balances(&2), (17, 3)); assert_eq!(Elections::candidates(), vec![1, 2]); @@ -1321,8 +1531,8 @@ mod tests { ExtBuilder::default().build_and_execute(|| { assert_eq!(Elections::candidates(), Vec::::new()); - assert_ok!(Elections::submit_candidacy(Origin::signed(1))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(1))); + assert_ok!(submit_candidacy(Origin::signed(2))); assert!(Elections::is_candidate(&1).is_ok()); assert!(Elections::is_candidate(&2).is_ok()); @@ -1332,7 +1542,7 @@ mod tests { assert_eq!(Elections::runners_up(), vec![]); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert!(Elections::is_candidate(&1).is_err()); assert!(Elections::is_candidate(&2).is_err()); @@ -1347,10 +1557,10 @@ mod tests { fn dupe_candidate_submission_should_not_work() { ExtBuilder::default().build_and_execute(|| { assert_eq!(Elections::candidates(), Vec::::new()); - assert_ok!(Elections::submit_candidacy(Origin::signed(1))); + assert_ok!(submit_candidacy(Origin::signed(1))); assert_eq!(Elections::candidates(), vec![1]); assert_noop!( - Elections::submit_candidacy(Origin::signed(1)), + submit_candidacy(Origin::signed(1)), Error::::DuplicatedCandidate, ); }); @@ -1360,18 +1570,18 @@ mod tests { fn member_candidacy_submission_should_not_work() { // critically important to make sure that outgoing candidates and losers are not mixed up. ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::vote(Origin::signed(2), vec![5], 20)); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(vote(Origin::signed(2), vec![5], 20)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![5]); assert_eq!(Elections::runners_up(), vec![]); assert_eq!(Elections::candidates(), vec![]); assert_noop!( - Elections::submit_candidacy(Origin::signed(5)), + submit_candidacy(Origin::signed(5)), Error::::MemberSubmit, ); }); @@ -1380,21 +1590,21 @@ mod tests { #[test] fn runner_candidate_submission_should_not_work() { ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::vote(Origin::signed(2), vec![5, 4], 20)); - assert_ok!(Elections::vote(Origin::signed(1), vec![3], 10)); + assert_ok!(vote(Origin::signed(2), vec![5, 4], 20)); + assert_ok!(vote(Origin::signed(1), vec![3], 10)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::runners_up_ids(), vec![3]); assert_noop!( - Elections::submit_candidacy(Origin::signed(3)), + submit_candidacy(Origin::signed(3)), Error::::RunnerSubmit, ); }); @@ -1405,7 +1615,7 @@ mod tests { ExtBuilder::default().build_and_execute(|| { assert_eq!(Elections::candidates(), Vec::::new()); assert_noop!( - Elections::submit_candidacy(Origin::signed(7)), + submit_candidacy(Origin::signed(7)), Error::::InsufficientCandidateFunds, ); }); @@ -1417,8 +1627,8 @@ mod tests { assert_eq!(Elections::candidates(), Vec::::new()); assert_eq!(balances(&2), (20, 0)); - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::vote(Origin::signed(2), vec![5], 20)); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(vote(Origin::signed(2), vec![5], 20)); assert_eq!(balances(&2), (18, 2)); assert_eq!(has_lock(&2), 20); @@ -1431,8 +1641,8 @@ mod tests { assert_eq!(Elections::candidates(), Vec::::new()); assert_eq!(balances(&2), (20, 0)); - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::vote(Origin::signed(2), vec![5], 12)); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(vote(Origin::signed(2), vec![5], 12)); assert_eq!(balances(&2), (18, 2)); assert_eq!(has_lock(&2), 12); @@ -1444,16 +1654,16 @@ mod tests { ExtBuilder::default().build_and_execute(|| { assert_eq!(balances(&2), (20, 0)); - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::vote(Origin::signed(2), vec![5], 20)); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(vote(Origin::signed(2), vec![5], 20)); assert_eq!(balances(&2), (18, 2)); assert_eq!(has_lock(&2), 20); assert_eq!(Elections::locked_stake_of(&2), 20); // can update; different stake; different lock and reserve. - assert_ok!(Elections::vote(Origin::signed(2), vec![5, 4], 15)); + assert_ok!(vote(Origin::signed(2), vec![5, 4], 15)); assert_eq!(balances(&2), (18, 2)); assert_eq!(has_lock(&2), 15); assert_eq!(Elections::locked_stake_of(&2), 15); @@ -1464,8 +1674,8 @@ mod tests { fn cannot_vote_for_no_candidate() { ExtBuilder::default().build_and_execute(|| { assert_noop!( - Elections::vote(Origin::signed(2), vec![], 20), - Error::::UnableToVote, + vote(Origin::signed(2), vec![], 20), + Error::::NoVotes, ); }); } @@ -1473,41 +1683,41 @@ mod tests { #[test] fn can_vote_for_old_members_even_when_no_new_candidates() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::vote(Origin::signed(2), vec![4, 5], 20)); + assert_ok!(vote(Origin::signed(2), vec![4, 5], 20)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::candidates(), vec![]); - assert_ok!(Elections::vote(Origin::signed(3), vec![4, 5], 10)); + assert_ok!(vote(Origin::signed(3), vec![4, 5], 10)); }); } #[test] fn prime_works() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::vote(Origin::signed(1), vec![4, 3], 10)); - assert_ok!(Elections::vote(Origin::signed(2), vec![4], 20)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(1), vec![4, 3], 10)); + assert_ok!(vote(Origin::signed(2), vec![4], 20)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::candidates(), vec![]); - assert_ok!(Elections::vote(Origin::signed(3), vec![4, 5], 10)); + assert_ok!(vote(Origin::signed(3), vec![4, 5], 10)); assert_eq!(PRIME.with(|p| *p.borrow()), Some(4)); }); } @@ -1515,20 +1725,20 @@ mod tests { #[test] fn prime_votes_for_exiting_members_are_removed() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::vote(Origin::signed(1), vec![4, 3], 10)); - assert_ok!(Elections::vote(Origin::signed(2), vec![4], 20)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(1), vec![4, 3], 10)); + assert_ok!(vote(Origin::signed(2), vec![4], 20)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); - assert_ok!(Elections::renounce_candidacy(Origin::signed(4))); + assert_ok!(Elections::renounce_candidacy(Origin::signed(4), Renouncing::Candidate(3))); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![3, 5]); assert_eq!(Elections::candidates(), vec![]); @@ -1545,29 +1755,29 @@ mod tests { .build_and_execute( || { // when we have only candidates - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); assert_noop!( // content of the vote is irrelevant. - Elections::vote(Origin::signed(1), vec![9, 99, 999, 9999], 5), + vote(Origin::signed(1), vec![9, 99, 999, 9999], 5), Error::::TooManyVotes, ); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); // now we have 2 members, 1 runner-up, and 1 new candidate - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(1), vec![9, 99, 999, 9999], 5)); + assert_ok!(vote(Origin::signed(1), vec![9, 99, 999, 9999], 5)); assert_noop!( - Elections::vote(Origin::signed(1), vec![9, 99, 999, 9_999, 99_999], 5), + vote(Origin::signed(1), vec![9, 99, 999, 9_999, 99_999], 5), Error::::TooManyVotes, ); }); @@ -1576,11 +1786,11 @@ mod tests { #[test] fn cannot_vote_for_less_than_ed() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); assert_noop!( - Elections::vote(Origin::signed(2), vec![4], 1), + vote(Origin::signed(2), vec![4], 1), Error::::LowBalance, ); }) @@ -1589,10 +1799,10 @@ mod tests { #[test] fn can_vote_for_more_than_total_balance_but_moot() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::vote(Origin::signed(2), vec![4, 5], 30)); + assert_ok!(vote(Origin::signed(2), vec![4, 5], 30)); // you can lie but won't get away with it. assert_eq!(Elections::locked_stake_of(&2), 20); assert_eq!(has_lock(&2), 20); @@ -1602,21 +1812,21 @@ mod tests { #[test] fn remove_voter_should_work() { ExtBuilder::default().voter_bond(8).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::vote(Origin::signed(2), vec![5], 20)); - assert_ok!(Elections::vote(Origin::signed(3), vec![5], 30)); + assert_ok!(vote(Origin::signed(2), vec![5], 20)); + assert_ok!(vote(Origin::signed(3), vec![5], 30)); assert_eq_uvec!(all_voters(), vec![2, 3]); assert_eq!(Elections::locked_stake_of(&2), 20); assert_eq!(Elections::locked_stake_of(&3), 30); - assert_eq!(Elections::votes_of(&2), vec![5]); - assert_eq!(Elections::votes_of(&3), vec![5]); + assert_eq!(votes_of(&2), vec![5]); + assert_eq!(votes_of(&3), vec![5]); assert_ok!(Elections::remove_voter(Origin::signed(2))); assert_eq_uvec!(all_voters(), vec![3]); - assert_eq!(Elections::votes_of(&2), vec![]); + assert_eq!(votes_of(&2), vec![]); assert_eq!(Elections::locked_stake_of(&2), 0); assert_eq!(balances(&2), (20, 0)); @@ -1634,8 +1844,8 @@ mod tests { #[test] fn dupe_remove_should_fail() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::vote(Origin::signed(2), vec![5], 20)); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(vote(Origin::signed(2), vec![5], 20)); assert_ok!(Elections::remove_voter(Origin::signed(2))); assert_eq!(all_voters(), vec![]); @@ -1647,18 +1857,18 @@ mod tests { #[test] fn removed_voter_should_not_be_counted() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); assert_ok!(Elections::remove_voter(Origin::signed(4))); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![3, 5]); }); @@ -1668,47 +1878,102 @@ mod tests { fn reporter_must_be_voter() { ExtBuilder::default().build_and_execute(|| { assert_noop!( - Elections::report_defunct_voter(Origin::signed(1), 2), + Elections::report_defunct_voter(Origin::signed(1), defunct_for(2)), Error::::MustBeVoter, ); }); } + #[test] + fn reporter_must_provide_lengths() { + ExtBuilder::default().build_and_execute(|| { + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + + // both are defunct. + assert_ok!(vote(Origin::signed(5), vec![99, 999, 9999], 50)); + assert_ok!(vote(Origin::signed(4), vec![999], 40)); + + // 3 candidates! incorrect candidate length. + assert_noop!( + Elections::report_defunct_voter(Origin::signed(4), DefunctVoter { + who: 5, + candidate_count: 2, + vote_count: 3, + }), + Error::::InvalidCandidateCount, + ); + + // 3 votes! incorrect vote length + assert_noop!( + Elections::report_defunct_voter(Origin::signed(4), DefunctVoter { + who: 5, + candidate_count: 3, + vote_count: 2, + }), + Error::::InvalidVoteCount, + ); + + // correct. + assert_ok!(Elections::report_defunct_voter(Origin::signed(4), DefunctVoter { + who: 5, + candidate_count: 3, + vote_count: 3, + })); + }); + } + + #[test] + fn reporter_can_overestimate_length() { + ExtBuilder::default().build_and_execute(|| { + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + + // both are defunct. + assert_ok!(vote(Origin::signed(5), vec![99], 50)); + assert_ok!(vote(Origin::signed(4), vec![999], 40)); + + // 2 candidates! overestimation is okay. + assert_ok!(Elections::report_defunct_voter(Origin::signed(4), defunct_for(5))); + }); + } + #[test] fn can_detect_defunct_voter() { ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(6))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(6))); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(2), vec![4, 5], 20)); - assert_ok!(Elections::vote(Origin::signed(6), vec![6], 30)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(2), vec![4, 5], 20)); + assert_ok!(vote(Origin::signed(6), vec![6], 30)); // will be soon a defunct voter. - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::runners_up_ids(), vec![6]); assert_eq!(Elections::candidates(), vec![]); // all of them have a member or runner-up that they voted for. - assert_eq!(Elections::is_defunct_voter(&5), false); - assert_eq!(Elections::is_defunct_voter(&4), false); - assert_eq!(Elections::is_defunct_voter(&2), false); - assert_eq!(Elections::is_defunct_voter(&6), false); + assert_eq!(Elections::is_defunct_voter(&votes_of(&5)), false); + assert_eq!(Elections::is_defunct_voter(&votes_of(&4)), false); + assert_eq!(Elections::is_defunct_voter(&votes_of(&2)), false); + assert_eq!(Elections::is_defunct_voter(&votes_of(&6)), false); // defunct - assert_eq!(Elections::is_defunct_voter(&3), true); + assert_eq!(Elections::is_defunct_voter(&votes_of(&3)), true); - assert_ok!(Elections::submit_candidacy(Origin::signed(1))); - assert_ok!(Elections::vote(Origin::signed(1), vec![1], 10)); + assert_ok!(submit_candidacy(Origin::signed(1))); + assert_ok!(vote(Origin::signed(1), vec![1], 10)); // has a candidate voted for. - assert_eq!(Elections::is_defunct_voter(&1), false); + assert_eq!(Elections::is_defunct_voter(&votes_of(&1)), false); }); } @@ -1716,17 +1981,17 @@ mod tests { #[test] fn report_voter_should_work_and_earn_reward() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(2), vec![4, 5], 20)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(2), vec![4, 5], 20)); // will be soon a defunct voter. - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::candidates(), vec![]); @@ -1734,7 +1999,7 @@ mod tests { assert_eq!(balances(&3), (28, 2)); assert_eq!(balances(&5), (45, 5)); - assert_ok!(Elections::report_defunct_voter(Origin::signed(5), 3)); + assert_ok!(Elections::report_defunct_voter(Origin::signed(5), defunct_for(3))); assert!(System::events().iter().any(|event| { event.event == Event::elections_phragmen(RawEvent::VoterReported(3, 5, true)) })); @@ -1747,14 +2012,14 @@ mod tests { #[test] fn report_voter_should_slash_when_bad_report() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::candidates(), vec![]); @@ -1762,7 +2027,7 @@ mod tests { assert_eq!(balances(&4), (35, 5)); assert_eq!(balances(&5), (45, 5)); - assert_ok!(Elections::report_defunct_voter(Origin::signed(5), 4)); + assert_ok!(Elections::report_defunct_voter(Origin::signed(5), defunct_for(4))); assert!(System::events().iter().any(|event| { event.event == Event::elections_phragmen(RawEvent::VoterReported(4, 5, false)) })); @@ -1775,19 +2040,19 @@ mod tests { #[test] fn simple_voting_rounds_should_work() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::vote(Origin::signed(2), vec![5], 20)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 15)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(2), vec![5], 20)); + assert_ok!(vote(Origin::signed(4), vec![4], 15)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); assert_eq_uvec!(all_voters(), vec![2, 3, 4]); - assert_eq!(Elections::votes_of(&2), vec![5]); - assert_eq!(Elections::votes_of(&3), vec![3]); - assert_eq!(Elections::votes_of(&4), vec![4]); + assert_eq!(votes_of(&2), vec![5]); + assert_eq!(votes_of(&3), vec![3]); + assert_eq!(votes_of(&4), vec![4]); assert_eq!(Elections::candidates(), vec![3, 4, 5]); assert_eq!(>::decode_len().unwrap(), 3); @@ -1795,7 +2060,7 @@ mod tests { assert_eq!(Elections::election_rounds(), 0); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members(), vec![(3, 30), (5, 20)]); assert_eq!(Elections::runners_up(), vec![]); @@ -1810,23 +2075,23 @@ mod tests { #[test] fn defunct_voter_will_be_counted() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(5))); // This guy's vote is pointless for this round. - assert_ok!(Elections::vote(Origin::signed(3), vec![4], 30)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(3), vec![4], 30)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members(), vec![(5, 50)]); assert_eq!(Elections::election_rounds(), 1); // but now it has a valid target. - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(4))); System::set_block_number(10); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); // candidate 4 is affected by an old vote. assert_eq!(Elections::members(), vec![(4, 30), (5, 50)]); @@ -1838,18 +2103,18 @@ mod tests { #[test] fn only_desired_seats_are_chosen() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(2), vec![2], 20)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(2), vec![2], 20)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::election_rounds(), 1); assert_eq!(Elections::members_ids(), vec![4, 5]); @@ -1859,11 +2124,11 @@ mod tests { #[test] fn phragmen_should_not_self_vote() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::candidates(), vec![]); assert_eq!(Elections::election_rounds(), 1); @@ -1879,18 +2144,18 @@ mod tests { #[test] fn runners_up_should_be_kept() { ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(2), vec![3], 20)); - assert_ok!(Elections::vote(Origin::signed(3), vec![2], 30)); - assert_ok!(Elections::vote(Origin::signed(4), vec![5], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![4], 50)); + assert_ok!(vote(Origin::signed(2), vec![3], 20)); + assert_ok!(vote(Origin::signed(3), vec![2], 30)); + assert_ok!(vote(Origin::signed(4), vec![5], 40)); + assert_ok!(vote(Origin::signed(5), vec![4], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); // sorted based on account id. assert_eq!(Elections::members_ids(), vec![4, 5]); // sorted based on merit (least -> most) @@ -1906,25 +2171,25 @@ mod tests { #[test] fn runners_up_should_be_next_candidates() { ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(2), vec![2], 20)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(2), vec![2], 20)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members(), vec![(4, 40), (5, 50)]); assert_eq!(Elections::runners_up(), vec![(2, 20), (3, 30)]); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 15)); + assert_ok!(vote(Origin::signed(5), vec![5], 15)); System::set_block_number(10); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members(), vec![(3, 30), (4, 40)]); assert_eq!(Elections::runners_up(), vec![(5, 15), (2, 20)]); }); @@ -1933,25 +2198,25 @@ mod tests { #[test] fn runners_up_lose_bond_once_outgoing() { ExtBuilder::default().desired_runners_up(1).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(2), vec![2], 20)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(2), vec![2], 20)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::runners_up_ids(), vec![2]); assert_eq!(balances(&2), (15, 5)); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); System::set_block_number(10); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::runners_up_ids(), vec![3]); assert_eq!(balances(&2), (15, 2)); @@ -1963,21 +2228,21 @@ mod tests { ExtBuilder::default().build_and_execute(|| { assert_eq!(balances(&5), (50, 0)); - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(5))); assert_eq!(balances(&5), (47, 3)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); assert_eq!(balances(&5), (45, 5)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![5]); assert_ok!(Elections::remove_voter(Origin::signed(5))); assert_eq!(balances(&5), (47, 3)); System::set_block_number(10); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![]); assert_eq!(balances(&5), (47, 0)); @@ -1987,16 +2252,16 @@ mod tests { #[test] fn losers_will_lose_the_bond() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::vote(Origin::signed(4), vec![5], 40)); + assert_ok!(vote(Origin::signed(4), vec![5], 40)); assert_eq!(balances(&5), (47, 3)); assert_eq!(balances(&3), (27, 3)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![5]); @@ -2010,23 +2275,23 @@ mod tests { #[test] fn current_members_are_always_next_candidate() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::election_rounds(), 1); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(2), vec![2], 20)); + assert_ok!(submit_candidacy(Origin::signed(2))); + assert_ok!(vote(Origin::signed(2), vec![2], 20)); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); assert_ok!(Elections::remove_voter(Origin::signed(4))); @@ -2034,7 +2299,7 @@ mod tests { assert_eq!(Elections::candidates(), vec![2, 3]); System::set_block_number(10); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); // 4 removed; 5 and 3 are the new best. assert_eq!(Elections::members_ids(), vec![3, 5]); @@ -2046,19 +2311,19 @@ mod tests { // what I mean by uninterrupted: // given no input or stimulants the same members are re-elected. ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(2), vec![2], 20)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(2), vec![2], 20)); let check_at_block = |b: u32| { System::set_block_number(b.into()); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); // we keep re-electing the same folks. assert_eq!(Elections::members(), vec![(4, 40), (5, 50)]); assert_eq!(Elections::runners_up(), vec![(2, 20), (3, 30)]); @@ -2079,22 +2344,22 @@ mod tests { #[test] fn remove_members_triggers_election() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::election_rounds(), 1); // a new candidate - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::remove_member(Origin::ROOT, 4)); + assert_ok!(Elections::remove_member(Origin::ROOT, 4, false)); assert_eq!(balances(&4), (35, 2)); // slashed assert_eq!(Elections::election_rounds(), 2); // new election round @@ -2102,24 +2367,68 @@ mod tests { }); } + #[test] + fn remove_member_should_indicate_replacement() { + ExtBuilder::default().build_and_execute(|| { + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + + System::set_block_number(5); + Elections::end_block(System::block_number()); + assert_eq!(Elections::members_ids(), vec![4, 5]); + + // no replacement yet. + assert_err_with_weight!( + Elections::remove_member(Origin::ROOT, 4, true), + Error::::InvalidReplacement, + Some(6000000), + ); + }); + + ExtBuilder::default().desired_runners_up(1).build_and_execute(|| { + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + + System::set_block_number(5); + Elections::end_block(System::block_number()); + assert_eq!(Elections::members_ids(), vec![4, 5]); + assert_eq!(Elections::runners_up_ids(), vec![3]); + + // there is a replacement! and this one needs a weight refund. + assert_err_with_weight!( + Elections::remove_member(Origin::ROOT, 4, false), + Error::::InvalidReplacement, + Some(6000000) // only thing that matters for now is that it is NOT the full block. + ); + }); + } + #[test] fn seats_should_be_released_when_no_vote() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::vote(Origin::signed(2), vec![3], 20)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(2), vec![3], 20)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); assert_eq!(>::decode_len().unwrap(), 3); assert_eq!(Elections::election_rounds(), 0); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![3, 5]); assert_eq!(Elections::election_rounds(), 1); @@ -2130,7 +2439,7 @@ mod tests { // meanwhile, no one cares to become a candidate again. System::set_block_number(10); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![]); assert_eq!(Elections::election_rounds(), 2); }); @@ -2139,32 +2448,32 @@ mod tests { #[test] fn incoming_outgoing_are_reported() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); - assert_ok!(Elections::submit_candidacy(Origin::signed(1))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(1))); + assert_ok!(submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(3))); // 5 will change their vote and becomes an `outgoing` - assert_ok!(Elections::vote(Origin::signed(5), vec![4], 8)); + assert_ok!(vote(Origin::signed(5), vec![4], 8)); // 4 will stay in the set - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); // 3 will become a winner - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); // these two are losers. - assert_ok!(Elections::vote(Origin::signed(2), vec![2], 20)); - assert_ok!(Elections::vote(Origin::signed(1), vec![1], 10)); + assert_ok!(vote(Origin::signed(2), vec![2], 20)); + assert_ok!(vote(Origin::signed(1), vec![1], 10)); System::set_block_number(10); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); // 3, 4 are new members, must still be bonded, nothing slashed. assert_eq!(Elections::members(), vec![(3, 30), (4, 48)]); @@ -2186,15 +2495,15 @@ mod tests { #[test] fn invalid_votes_are_moot() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![10], 50)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![10], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq_uvec!(Elections::members_ids(), vec![3, 4]); assert_eq!(Elections::election_rounds(), 1); @@ -2204,18 +2513,18 @@ mod tests { #[test] fn members_are_sorted_based_on_id_runners_on_merit() { ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(2), vec![3], 20)); - assert_ok!(Elections::vote(Origin::signed(3), vec![2], 30)); - assert_ok!(Elections::vote(Origin::signed(4), vec![5], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![4], 50)); + assert_ok!(vote(Origin::signed(2), vec![3], 20)); + assert_ok!(vote(Origin::signed(3), vec![2], 30)); + assert_ok!(vote(Origin::signed(4), vec![5], 40)); + assert_ok!(vote(Origin::signed(5), vec![4], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); // id: low -> high. assert_eq!(Elections::members(), vec![(4, 50), (5, 40)]); // merit: low -> high. @@ -2226,14 +2535,14 @@ mod tests { #[test] fn candidates_are_sorted() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(3))); assert_eq!(Elections::candidates(), vec![3, 5]); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::renounce_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(Elections::renounce_candidacy(Origin::signed(3), Renouncing::Candidate(4))); assert_eq!(Elections::candidates(), vec![2, 4, 5]); }) @@ -2242,64 +2551,43 @@ mod tests { #[test] fn runner_up_replacement_maintains_members_order() { ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(2), vec![5], 20)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![2], 50)); + assert_ok!(vote(Origin::signed(2), vec![5], 20)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![2], 50)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![2, 4]); - assert_ok!(Elections::remove_member(Origin::ROOT, 2)); + assert_ok!(Elections::remove_member(Origin::ROOT, 2, true)); assert_eq!(Elections::members_ids(), vec![4, 5]); }); } - #[test] - fn runner_up_replacement_works_when_out_of_order() { - ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); - - assert_ok!(Elections::vote(Origin::signed(2), vec![5], 20)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(5), vec![2], 50)); - - System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); - - assert_eq!(Elections::members_ids(), vec![2, 4]); - assert_ok!(Elections::renounce_candidacy(Origin::signed(3))); - }); - } - #[test] fn can_renounce_candidacy_member_with_runners_bond_is_refunded() { ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(2), vec![2], 20)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(2), vec![2], 20)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::runners_up_ids(), vec![2, 3]); - assert_ok!(Elections::renounce_candidacy(Origin::signed(4))); + assert_ok!(Elections::renounce_candidacy(Origin::signed(4), Renouncing::Member)); assert_eq!(balances(&4), (38, 2)); // 2 is voting bond. assert_eq!(Elections::members_ids(), vec![3, 5]); @@ -2310,55 +2598,47 @@ mod tests { #[test] fn can_renounce_candidacy_member_without_runners_bond_is_refunded() { ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); - - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(2), vec![2], 20)); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::runners_up_ids(), vec![]); - assert_eq!(Elections::candidates(), vec![2, 3]); - assert_ok!(Elections::renounce_candidacy(Origin::signed(4))); + assert_ok!(Elections::renounce_candidacy(Origin::signed(4), Renouncing::Member)); assert_eq!(balances(&4), (38, 2)); // 2 is voting bond. // no replacement assert_eq!(Elections::members_ids(), vec![5]); assert_eq!(Elections::runners_up_ids(), vec![]); - // still candidate - assert_eq!(Elections::candidates(), vec![2, 3]); }) } #[test] fn can_renounce_candidacy_runner() { ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); - assert_ok!(Elections::submit_candidacy(Origin::signed(4))); - assert_ok!(Elections::submit_candidacy(Origin::signed(3))); - assert_ok!(Elections::submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(2))); - assert_ok!(Elections::vote(Origin::signed(5), vec![4], 50)); - assert_ok!(Elections::vote(Origin::signed(4), vec![5], 40)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(2), vec![2], 20)); + assert_ok!(vote(Origin::signed(5), vec![4], 50)); + assert_ok!(vote(Origin::signed(4), vec![5], 40)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(2), vec![2], 20)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![4, 5]); assert_eq!(Elections::runners_up_ids(), vec![2, 3]); - assert_ok!(Elections::renounce_candidacy(Origin::signed(3))); + assert_ok!(Elections::renounce_candidacy(Origin::signed(3), Renouncing::RunnerUp)); assert_eq!(balances(&3), (28, 2)); // 2 is voting bond. assert_eq!(Elections::members_ids(), vec![4, 5]); @@ -2366,14 +2646,38 @@ mod tests { }) } + // #[test] + // fn runner_up_replacement_works_when_out_of_order() { + // ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { + // assert_ok!(submit_candidacy(Origin::signed(5))); + // assert_ok!(submit_candidacy(Origin::signed(4))); + // assert_ok!(submit_candidacy(Origin::signed(3))); + // assert_ok!(submit_candidacy(Origin::signed(2))); + + // assert_ok!(vote(Origin::signed(2), vec![5], 20)); + // assert_ok!(vote(Origin::signed(3), vec![3], 30)); + // assert_ok!(vote(Origin::signed(4), vec![4], 40)); + // assert_ok!(vote(Origin::signed(5), vec![2], 50)); + + // System::set_block_number(5); + // Elections::end_block(System::block_number()); + + // assert_eq!(Elections::members_ids(), vec![2, 4]); + // assert_eq!(ELections::runners_up_ids(), vec![3, 5]); + // assert_ok!(Elections::renounce_candidacy(Origin::signed(3), Renouncing::RunnerUp)); + // assert_eq!(Elections::members_ids(), vec![2, 4]); + // assert_eq!(ELections::runners_up_ids(), vec![5]); + // }); + // } + #[test] fn can_renounce_candidacy_candidate() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(5))); assert_eq!(balances(&5), (47, 3)); assert_eq!(Elections::candidates(), vec![5]); - assert_ok!(Elections::renounce_candidacy(Origin::signed(5))); + assert_ok!(Elections::renounce_candidacy(Origin::signed(5), Renouncing::Candidate(1))); assert_eq!(balances(&5), (50, 0)); assert_eq!(Elections::candidates(), vec![]); }) @@ -2383,25 +2687,107 @@ mod tests { fn wrong_renounce_candidacy_should_fail() { ExtBuilder::default().build_and_execute(|| { assert_noop!( - Elections::renounce_candidacy(Origin::signed(5)), - Error::::InvalidOrigin, + Elections::renounce_candidacy(Origin::signed(5), Renouncing::Candidate(0)), + Error::::InvalidRenouncing, ); + assert_noop!( + Elections::renounce_candidacy(Origin::signed(5), Renouncing::Member), + Error::::NotMember, + ); + assert_noop!( + Elections::renounce_candidacy(Origin::signed(5), Renouncing::RunnerUp), + Error::::InvalidRenouncing, + ); + }) + } + + #[test] + fn non_member_renounce_member_should_fail() { + ExtBuilder::default().desired_runners_up(1).build_and_execute(|| { + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + + System::set_block_number(5); + Elections::end_block(System::block_number()); + + assert_eq!(Elections::members_ids(), vec![4, 5]); + assert_eq!(Elections::runners_up_ids(), vec![3]); + + assert_noop!( + Elections::renounce_candidacy(Origin::signed(3), Renouncing::Member), + Error::::NotMember, + ); + }) + } + + #[test] + fn non_runner_up_renounce_runner_up_should_fail() { + ExtBuilder::default().desired_runners_up(1).build_and_execute(|| { + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + + System::set_block_number(5); + Elections::end_block(System::block_number()); + + assert_eq!(Elections::members_ids(), vec![4, 5]); + assert_eq!(Elections::runners_up_ids(), vec![3]); + + assert_noop!( + Elections::renounce_candidacy(Origin::signed(4), Renouncing::RunnerUp), + Error::::InvalidRenouncing, + ); + }) + } + + #[test] + fn wrong_candidate_count_renounce_should_fail() { + ExtBuilder::default().build_and_execute(|| { + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + + assert_noop!( + Elections::renounce_candidacy(Origin::signed(4), Renouncing::Candidate(2)), + Error::::InvalidRenouncing, + ); + + assert_ok!(Elections::renounce_candidacy(Origin::signed(4), Renouncing::Candidate(3))); + }) + } + + #[test] + fn renounce_candidacy_count_can_overestimate() { + ExtBuilder::default().build_and_execute(|| { + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + // while we have only 3 candidates. + assert_ok!(Elections::renounce_candidacy(Origin::signed(4), Renouncing::Candidate(4))); }) } #[test] fn behavior_with_dupe_candidate() { ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - // TODD: this is a demonstration and should be fixed with #4593 >::put(vec![1, 1, 2, 3, 4]); - assert_ok!(Elections::vote(Origin::signed(5), vec![1], 50)); - assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); - assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); - assert_ok!(Elections::vote(Origin::signed(2), vec![2], 20)); + assert_ok!(vote(Origin::signed(5), vec![1], 50)); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + assert_ok!(vote(Origin::signed(2), vec![2], 20)); System::set_block_number(5); - assert_ok!(Elections::end_block(System::block_number())); + Elections::end_block(System::block_number()); assert_eq!(Elections::members_ids(), vec![1, 4]); assert_eq!(Elections::runners_up_ids(), vec![2, 3]); diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index a05561173b..11109fb177 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -214,7 +214,20 @@ macro_rules! assert_err { #[cfg(feature = "std")] macro_rules! assert_err_ignore_postinfo { ( $x:expr , $y:expr $(,)? ) => { - assert_err!($x.map(|_| ()).map_err(|e| e.error), $y); + $crate::assert_err!($x.map(|_| ()).map_err(|e| e.error), $y); + } +} + +#[macro_export] +#[cfg(feature = "std")] +macro_rules! assert_err_with_weight { + ($call:expr, $err:expr, $weight:expr $(,)? ) => { + if let Err(dispatch_err_with_post) = $call { + $crate::assert_err!($call.map(|_| ()).map_err(|e| e.error), $err); + assert_eq!(dispatch_err_with_post.post_info.actual_weight, $weight.into()); + } else { + panic!("expected Err(_), got Ok(_).") + } } } diff --git a/substrate/frame/timestamp/src/benchmarking.rs b/substrate/frame/timestamp/src/benchmarking.rs index fd2d285698..9b1c976229 100644 --- a/substrate/frame/timestamp/src/benchmarking.rs +++ b/substrate/frame/timestamp/src/benchmarking.rs @@ -35,6 +35,7 @@ benchmarks! { set { let t in 1 .. MAX_TIME; }: _(RawOrigin::None, t.into()) + verify { ensure!(Timestamp::::now() == t.into(), "Time was not set."); } @@ -44,6 +45,7 @@ benchmarks! { Timestamp::::set(RawOrigin::None.into(), t.into())?; ensure!(DidUpdate::exists(), "Time was not set."); }: { Timestamp::::on_finalize(t.into()); } + verify { ensure!(!DidUpdate::exists(), "Time was not removed."); } diff --git a/substrate/primitives/runtime/src/lib.rs b/substrate/primitives/runtime/src/lib.rs index 41181ff439..361da3ee57 100644 --- a/substrate/primitives/runtime/src/lib.rs +++ b/substrate/primitives/runtime/src/lib.rs @@ -374,16 +374,16 @@ impl From for DispatchOutcome { } } -/// This is the legacy return type of `Dispatchable`. It is still exposed for compatibilty -/// reasons. The new return type is `DispatchResultWithInfo`. -/// FRAME runtimes should use frame_support::dispatch::DispatchResult +/// This is the legacy return type of `Dispatchable`. It is still exposed for compatibility reasons. +/// The new return type is `DispatchResultWithInfo`. FRAME runtimes should use +/// `frame_support::dispatch::DispatchResult`. pub type DispatchResult = sp_std::result::Result<(), DispatchError>; /// Return type of a `Dispatchable` which contains the `DispatchResult` and additional information /// about the `Dispatchable` that is only known post dispatch. pub type DispatchResultWithInfo = sp_std::result::Result>; -/// Reason why a dispatch call failed +/// Reason why a dispatch call failed. #[derive(Eq, PartialEq, Clone, Copy, Encode, Decode, RuntimeDebug)] #[cfg_attr(feature = "std", derive(Serialize))] pub enum DispatchError { @@ -393,11 +393,11 @@ pub enum DispatchError { CannotLookup, /// A bad origin. BadOrigin, - /// A custom error in a module + /// A custom error in a module. Module { - /// Module index, matching the metadata module index + /// Module index, matching the metadata module index. index: u8, - /// Module specific error value + /// Module specific error value. error: u8, /// Optional error message. #[codec(skip)] @@ -405,15 +405,15 @@ pub enum DispatchError { }, } -/// Result of a `Dispatchable` which contains the `DispatchResult` and additional information -/// about the `Dispatchable` that is only known post dispatch. +/// Result of a `Dispatchable` which contains the `DispatchResult` and additional information about +/// the `Dispatchable` that is only known post dispatch. #[derive(Eq, PartialEq, Clone, Copy, Encode, Decode, RuntimeDebug)] pub struct DispatchErrorWithPostInfo where Info: Eq + PartialEq + Clone + Copy + Encode + Decode + traits::Printable { - /// Addditional information about the `Dispatchable` which is only known post dispatch. + /// Additional information about the `Dispatchable` which is only known post dispatch. pub post_info: Info, - /// The actual `DispatchResult` indicating whether the dispatch was succesfull. + /// The actual `DispatchResult` indicating whether the dispatch was successful. pub error: DispatchError, }