diff --git a/Cargo.lock b/Cargo.lock index ffc0f5b940a9c..1930628146cef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4547,7 +4547,7 @@ dependencies = [ [[package]] name = "pallet-elections-phragmen" -version = "2.0.1" +version = "3.0.0" dependencies = [ "frame-benchmarking", "frame-support", diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index d3cc0101e082b..6ff98e5b3aa2f 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -54,7 +54,7 @@ pallet-contracts = { version = "2.0.0", default-features = false, path = "../../ pallet-contracts-primitives = { version = "2.0.0", default-features = false, path = "../../../frame/contracts/common/" } pallet-contracts-rpc-runtime-api = { version = "0.8.0", default-features = false, path = "../../../frame/contracts/rpc/runtime-api/" } pallet-democracy = { version = "2.0.0", default-features = false, path = "../../../frame/democracy" } -pallet-elections-phragmen = { version = "2.0.0", default-features = false, path = "../../../frame/elections-phragmen" } +pallet-elections-phragmen = { version = "3.0.0", default-features = false, path = "../../../frame/elections-phragmen" } pallet-grandpa = { version = "2.0.0", default-features = false, path = "../../../frame/grandpa" } pallet-im-online = { version = "2.0.0", default-features = false, path = "../../../frame/im-online" } pallet-indices = { version = "2.0.0", default-features = false, path = "../../../frame/indices" } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 92e5dfa7830af..e74c61a9c0eb7 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -579,7 +579,10 @@ impl pallet_collective::Config for Runtime { parameter_types! { pub const CandidacyBond: Balance = 10 * DOLLARS; - pub const VotingBond: Balance = 1 * DOLLARS; + // 1 storage item created, key size is 32 bytes, value size is 16+16. + pub const VotingBondBase: Balance = deposit(1, 64); + // additional data per vote is 32 bytes (account id). + pub const VotingBondFactor: Balance = deposit(0, 32); pub const TermDuration: BlockNumber = 7 * DAYS; pub const DesiredMembers: u32 = 13; pub const DesiredRunnersUp: u32 = 7; @@ -599,9 +602,9 @@ impl pallet_elections_phragmen::Config for Runtime { type InitializeMembers = Council; type CurrencyToVote = U128CurrencyToVote; type CandidacyBond = CandidacyBond; - type VotingBond = VotingBond; + type VotingBondBase = VotingBondBase; + type VotingBondFactor = VotingBondFactor; type LoserCandidate = (); - type BadReport = (); type KickedMember = (); type DesiredMembers = DesiredMembers; type DesiredRunnersUp = DesiredRunnersUp; diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 7c41b97996a68..b2993fd45eb3a 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -40,7 +40,7 @@ //! If there are not, or if no prime is set, then the motion is dropped without being executed. #![cfg_attr(not(feature = "std"), no_std)] -#![recursion_limit="128"] +#![recursion_limit = "128"] use sp_std::{prelude::*, result}; use sp_core::u32_trait::Value as U32; @@ -840,6 +840,10 @@ impl, I: Instance> ChangeMembers for Module { fn set_prime(prime: Option) { Prime::::set(prime); } + + fn get_prime() -> Option { + Prime::::get() + } } impl, I: Instance> InitializeMembers for Module { diff --git a/frame/elections-phragmen/CHANGELOG.md b/frame/elections-phragmen/CHANGELOG.md new file mode 100644 index 0000000000000..3d48448fa55ec --- /dev/null +++ b/frame/elections-phragmen/CHANGELOG.md @@ -0,0 +1,24 @@ +# Changelog +All notable changes to this crate will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this crate adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [3.0.0] - UNRELEASED + +### Added +[Add slashing events to elections-phragmen](https://github.com/paritytech/substrate/pull/7543) + +### Changed + +### Fixed +[Don't slash all outgoing members](https://github.com/paritytech/substrate/pull/7394) +[Fix wrong outgoing calculation in election](https://github.com/paritytech/substrate/pull/7384) + +### Security +\[**Needs Migration**\] [Fix elections-phragmen and proxy issue + Record deposits on-chain](https://github.com/paritytech/substrate/pull/7040) + +## [2.0.0] - 2020-09-2020 + +Initial version from which version tracking has begun. + diff --git a/frame/elections-phragmen/Cargo.toml b/frame/elections-phragmen/Cargo.toml index 55cbcfe985d5b..2571dff7c8904 100644 --- a/frame/elections-phragmen/Cargo.toml +++ b/frame/elections-phragmen/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-elections-phragmen" -version = "2.0.1" +version = "3.0.0" authors = ["Parity Technologies "] edition = "2018" license = "Apache-2.0" diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index 3ed4af2487df3..511d2751a5d77 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -69,20 +69,6 @@ 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> @@ -104,7 +90,7 @@ fn submit_candidates_with_self_vote(c: u32, prefix: &'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) + submit_voter::(c.clone(), vec![c.clone()], stake).map(|_| ()) ).collect::>()?; Ok(candidates) } @@ -112,7 +98,7 @@ fn submit_candidates_with_self_vote(c: u32, prefix: &'static str) /// Submit one voter. fn submit_voter(caller: T::AccountId, votes: Vec, stake: BalanceOf) - -> Result<(), sp_runtime::DispatchError> + -> frame_support::dispatch::DispatchResult { >::vote(RawOrigin::Signed(caller).into(), votes, stake) } @@ -152,8 +138,8 @@ fn fill_seats_up_to(m: u32) -> Result, &'static str Ok( >::members() .into_iter() - .map(|(x, _)| x) - .chain(>::runners_up().into_iter().map(|(x, _)| x)) + .map(|m| m.who) + .chain(>::runners_up().into_iter().map(|r| r.who)) .collect() ) } @@ -163,12 +149,12 @@ fn clean() { >::kill(); >::kill(); >::kill(); - let _ = >::drain(); + >::remove_all(); } benchmarks! { // -- Signed ones - vote { + vote_equal { let v in 1 .. (MAXIMUM_VOTE as u32); clean::(); @@ -178,14 +164,39 @@ benchmarks! { let caller = endowed_account::("caller", 0); let stake = default_stake::(BALANCE_FACTOR); - // vote for all of them. - let votes = all_candidates; + // original votes. + let mut votes = all_candidates; + submit_voter::(caller.clone(), votes.clone(), stake)?; + + // new votes. + votes.rotate_left(1); whitelist!(caller); - }: _(RawOrigin::Signed(caller), votes, stake) + }: vote(RawOrigin::Signed(caller), votes, stake) - vote_update { - let v in 1 .. (MAXIMUM_VOTE as u32); + vote_more { + let v in 2 .. (MAXIMUM_VOTE as u32); + clean::(); + + // create a bunch of candidates. + let all_candidates = submit_candidates::(v, "candidates")?; + + let caller = endowed_account::("caller", 0); + let stake = default_stake::(BALANCE_FACTOR); + + // original votes. + let mut votes = all_candidates.iter().skip(1).cloned().collect::>(); + submit_voter::(caller.clone(), votes.clone(), stake / >::from(10u32))?; + + // new votes. + votes = all_candidates; + assert!(votes.len() > >::get(caller.clone()).votes.len()); + + whitelist!(caller); + }: vote(RawOrigin::Signed(caller), votes, stake / >::from(10u32)) + + vote_less { + let v in 2 .. (MAXIMUM_VOTE as u32); clean::(); // create a bunch of candidates. @@ -199,7 +210,8 @@ benchmarks! { submit_voter::(caller.clone(), votes.clone(), stake)?; // new votes. - votes.rotate_left(1); + votes = votes.into_iter().skip(1).collect::>(); + assert!(votes.len() < >::get(caller.clone()).votes.len()); whitelist!(caller); }: vote(RawOrigin::Signed(caller), votes, stake) @@ -220,123 +232,6 @@ benchmarks! { whitelist!(caller); }: _(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 the number of desired members and runners-up. We'll be in - // this state almost always. - let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get(); - - clean::(); - 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 must be whitelisted, and 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. NOTE: we can simplify this. There's no need to create all these - // candidates and remove them. The defunct voter can just vote for random accounts as long - // as there are enough members (potential candidates). - 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_info = defunct_for::(account_2.clone()); - whitelist!(account_1); - - assert!(>::is_voter(&account_2)); - }: report_defunct_voter(RawOrigin::Signed(account_1.clone()), defunct_info) - verify { - 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 the number of desired members and runners-up. We'll be in - // this state almost always. - let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get(); - - clean::(); - 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 need to be whitelisted, and 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 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_info = defunct_for::(account_2.clone()); - whitelist!(account_1); - }: report_defunct_voter(RawOrigin::Signed(account_1.clone()), defunct_info) - verify { - // account 2 is still a voter. - 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; @@ -519,20 +414,52 @@ benchmarks! { } } - #[extra] - 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. + clean_defunct_voters { + // total number of voters. + let v in (MAX_VOTERS / 2) .. MAX_VOTERS; + // those that are defunct and need removal. + let d in 1 .. (MAX_VOTERS / 2); + + // remove any previous stuff. + clean::(); + + let all_candidates = submit_candidates::(v, "candidates")?; + distribute_voters::(all_candidates, v, MAXIMUM_VOTE)?; + + // all candidates leave. + >::kill(); + + // now everyone is defunct + assert!(>::iter().all(|(_, v)| >::is_defunct_voter(&v.votes))); + assert_eq!(>::iter().count() as u32, v); + let root = RawOrigin::Root; + }: _(root, v, d) + verify { + assert_eq!(>::iter().count() as u32, 0); + } + + election_phragmen { + // This is just to focus on phragmen in the context of this module. We always select 20 + // members, this is hard-coded in the runtime and cannot be trivially changed at this stage. + // Yet, change the number of voters, candidates and edge per voter to see the impact. Note + // that we give all candidates a self vote to make sure they are all considered. let c in 1 .. MAX_CANDIDATES; + let v in 1 .. MAX_VOTERS; + let e in MAX_VOTERS .. MAX_VOTERS * MAXIMUM_VOTE as u32; clean::(); - // create c candidates. + // so we have a situation with v and e. we want e to basically always be in the range of `e + // -> e * MAXIMUM_VOTE`, but we cannot express that now with the benchmarks. So what we do + // is: when c is being iterated, v, and e are max and fine. when v is being iterated, e is + // being set to max and this is a problem. In these cases, we cap e to a lower value, namely + // v * MAXIMUM_VOTE. when e is being iterated, v is at max, and again fine. all in all, + // votes_per_voter can never be more than MAXIMUM_VOTE. Note that this might cause `v` to be + // an overestimate. + let votes_per_voter = (e / v).min(MAXIMUM_VOTE as u32); + 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)?; + let _ = distribute_voters::(all_candidates, v, votes_per_voter as usize)?; }: { - // elect >::on_initialize(T::TermDuration::get()); } verify { @@ -551,18 +478,16 @@ benchmarks! { } #[extra] - 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. + election_phragmen_c_e { let c in 1 .. MAX_CANDIDATES; - let v in 1 .. MAX_VOTERS; - let e in 1 .. (MAXIMUM_VOTE as u32); + let e in MAX_VOTERS .. MAX_VOTERS * MAXIMUM_VOTE as u32; + let fixed_v = MAX_VOTERS; clean::(); + let votes_per_voter = e / fixed_v; + let all_candidates = submit_candidates_with_self_vote::(c, "candidates")?; - let _ = distribute_voters::(all_candidates, v, e as usize)?; + let _ = distribute_voters::(all_candidates, fixed_v, votes_per_voter as usize)?; }: { >::on_initialize(T::TermDuration::get()); } @@ -580,6 +505,35 @@ benchmarks! { MEMBERS.with(|m| *m.borrow_mut() = vec![]); } } + + #[extra] + election_phragmen_v { + let v in 4 .. 16; + let fixed_c = MAX_CANDIDATES; + let fixed_e = 64; + clean::(); + + let votes_per_voter = fixed_e / v; + + let all_candidates = submit_candidates_with_self_vote::(fixed_c, "candidates")?; + let _ = distribute_voters::(all_candidates, v, votes_per_voter as usize)?; + }: { + >::on_initialize(T::TermDuration::get()); + } + verify { + assert_eq!(>::members().len() as u32, T::DesiredMembers::get().min(fixed_c)); + assert_eq!( + >::runners_up().len() as u32, + T::DesiredRunnersUp::get().min(fixed_c.saturating_sub(T::DesiredMembers::get())), + ); + + #[cfg(test)] + { + // reset members in between benchmark tests. + use crate::tests::MEMBERS; + MEMBERS.with(|m| *m.borrow_mut() = vec![]); + } + } } #[cfg(test)] @@ -590,52 +544,76 @@ mod tests { #[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_vote_equal::()); + }); + + ExtBuilder::default() + .desired_members(13) + .desired_runners_up(7) + .build_and_execute(|| { + assert_ok!(test_benchmark_vote_more::()); + }); + + ExtBuilder::default() + .desired_members(13) + .desired_runners_up(7) + .build_and_execute(|| { + assert_ok!(test_benchmark_vote_less::()); + }); + + 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_remove_voter::()); + assert_ok!(test_benchmark_submit_candidacy::()); }); ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { - assert_ok!(test_benchmark_report_defunct_voter_correct::()); + assert_ok!(test_benchmark_renounce_candidacy_candidate::()); }); ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { - assert_ok!(test_benchmark_report_defunct_voter_incorrect::()); + assert_ok!(test_benchmark_renounce_candidacy_runners_up::()); }); ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { - assert_ok!(test_benchmark_submit_candidacy::()); + assert_ok!(test_benchmark_renounce_candidacy_members::()); }); ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { - assert_ok!(test_benchmark_renounce_candidacy_candidate::()); + assert_ok!(test_benchmark_remove_member_without_replacement::()); }); ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { - assert_ok!(test_benchmark_renounce_candidacy_runners_up::()); + assert_ok!(test_benchmark_remove_member_with_replacement::()); }); ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { - assert_ok!(test_benchmark_renounce_candidacy_members::()); + assert_ok!(test_benchmark_clean_defunct_voters::()); }); ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { - assert_ok!(test_benchmark_remove_member_without_replacement::()); + assert_ok!(test_benchmark_election_phragmen::()); }); ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { - assert_ok!(test_benchmark_remove_member_with_replacement::()); + assert_ok!(test_benchmark_election_phragmen::()); }); ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { - assert_ok!(test_benchmark_on_initialize::()); + assert_ok!(test_benchmark_election_phragmen_c_e::()); }); ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| { - assert_ok!(test_benchmark_phragmen::()); + assert_ok!(test_benchmark_election_phragmen_v::()); }); } } diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 5027840aef3c7..1bef73831e65d 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -23,51 +23,66 @@ //! //! The election happens in _rounds_: every `N` blocks, all previous members are retired and a new //! set is elected (which may or may not have an intersection with the previous set). Each round -//! lasts for some number of blocks defined by `TermDuration` storage item. The words _term_ and +//! lasts for some number of blocks defined by [`Config::TermDuration`]. The words _term_ and //! _round_ can be used interchangeably in this context. //! -//! `TermDuration` might change during a round. This can shorten or extend the length of the round. -//! The next election round's block number is never stored but rather always checked on the fly. -//! Based on the current block number and `TermDuration`, the condition `BlockNumber % TermDuration -//! == 0` being satisfied will always trigger a new election round. +//! [`Config::TermDuration`] might change during a round. This can shorten or extend the length of +//! the round. The next election round's block number is never stored but rather always checked on +//! the fly. Based on the current block number and [`Config::TermDuration`], the condition +//! `BlockNumber % TermDuration == 0` being satisfied will always trigger a new election round. +//! +//! ### Bonds and Deposits +//! +//! Both voting and being a candidate requires deposits to be taken, in exchange for the data that +//! needs to be kept on-chain. The terms *bond* and *deposit* can be used interchangeably in this +//! context. +//! +//! Bonds will be unreserved only upon adhering to the protocol laws. Failing to do so will cause in +//! the bond to slashed. //! //! ### Voting //! -//! Voters can vote for any set of the candidates by providing a list of account ids. Invalid votes -//! (voting for non-candidates) are ignored during election. Yet, a voter _might_ vote for a future -//! candidate. Voters reserve a bond as they vote. Each vote defines a `value`. This amount is -//! locked from the account of the voter and indicates the weight of the vote. Voters can update -//! their votes at any time by calling `vote()` again. This keeps the bond untouched but can -//! optionally change the locked `value`. After a round, votes are kept and might still be valid for +//! Voters can vote for a limited number of the candidates by providing a list of account ids, +//! bounded by [`MAXIMUM_VOTE`]. Invalid votes (voting for non-candidates) and duplicate votes are +//! ignored during election. Yet, a voter _might_ vote for a future candidate. Voters reserve a bond +//! as they vote. Each vote defines a `value`. This amount is locked from the account of the voter +//! and indicates the weight of the vote. Voters can update their votes at any time by calling +//! `vote()` again. This can update the vote targets (which might update the deposit) or update the +//! vote's stake ([`Voter::stake`]). After a round, votes are kept and might still be valid for //! further rounds. A voter is responsible for calling `remove_voter` once they are done to have //! their bond back and remove the lock. //! -//! Voters also report other voters as being defunct to earn their bond. A voter is defunct once all -//! of the candidates that they have voted for are neither a valid candidate anymore nor a member. -//! Upon reporting, if the target voter is actually defunct, the reporter will be rewarded by the -//! voting bond of the target. The target will lose their bond and get removed. If the target is not -//! defunct, the reporter is slashed and removed. To prevent being reported, voters should manually -//! submit a `remove_voter()` as soon as they are in the defunct state. +//! See [`Call::vote`], [`Call::remove_voter`]. +//! +//! ### Defunct Voter +//! +//! A voter is defunct once all of the candidates that they have voted for are not a valid candidate +//! (as seen further below, members and runners-up are also always candidates). Defunct voters can +//! be removed via a root call ([`Call::clean_defunct_voters`]). Upon being removed, their bond is +//! returned. This is an administrative operation and can be called only by the root origin in the +//! case of state bloat. //! //! ### Candidacy and Members //! -//! Candidates also reserve a bond as they submit candidacy. A candidate cannot take their candidacy -//! back. A candidate can end up in one of the below situations: -//! - **Winner**: A winner is kept as a _member_. They must still have a bond in reserve and they -//! are automatically counted as a candidate for the next election. +//! Candidates also reserve a bond as they submit candidacy. A candidate can end up in one of the +//! below situations: +//! - **Members**: A winner is kept as a _member_. They must still have a bond in reserve and they +//! are automatically counted as a candidate for the next election. The number of desired +//! members is set by [`Config::DesiredMembers`]. //! - **Runner-up**: Runners-up are the best candidates immediately after the winners. The number -//! of runners_up to keep is configurable. Runners-up are used, in order that they are elected, -//! as replacements when a candidate is kicked by `[remove_member]`, or when an active member -//! renounces their candidacy. Runners are automatically counted as a candidate for the next -//! election. -//! - **Loser**: Any of the candidate who are not a winner are left as losers. A loser might be an -//! _outgoing member or runner_, meaning that they are an active member who failed to keep their -//! spot. An outgoing will always lose their bond. +//! of runners up to keep is set by [`Config::DesiredRunnersUp`]. Runners-up are used, in the +//! same order as they are elected, as replacements when a candidate is kicked by +//! [`Call::remove_member`], or when an active member renounces their candidacy. Runners are +//! automatically counted as a candidate for the next election. +//! - **Loser**: Any of the candidate who are not member/runner-up are left as losers. A loser +//! might be an _outgoing member or runner-up_, meaning that they are an active member who +//! failed to keep their spot. **An outgoing candidate/member/runner-up will always lose their +//! bond**. //! -//! ##### Renouncing candidacy. +//! #### Renouncing candidacy. //! -//! All candidates, elected or not, can renounce their candidacy. A call to [`Module::renounce_candidacy`] -//! will always cause the candidacy bond to be refunded. +//! All candidates, elected or not, can renounce their candidacy. A call to +//! [`Call::renounce_candidacy`] will always cause the candidacy bond to be refunded. //! //! Note that with the members being the default candidates for the next round and votes persisting //! in storage, the election system is entirely stable given no further input. This means that if @@ -90,7 +105,7 @@ use frame_support::{ ensure, storage::{IterableStorageMap, StorageMap}, traits::{ - BalanceStatus, ChangeMembers, Contains, ContainsLengthBound, Currency, CurrencyToVote, Get, + ChangeMembers, Contains, ContainsLengthBound, Currency, CurrencyToVote, Get, InitializeMembers, LockIdentifier, LockableCurrency, OnUnbalanced, ReservableCurrency, WithdrawReasons, }, @@ -102,12 +117,14 @@ use sp_runtime::{ traits::{Saturating, StaticLookup, Zero}, DispatchError, Perbill, RuntimeDebug, }; -use sp_std::prelude::*; +use sp_std::{prelude::*, cmp::Ordering}; mod benchmarking; pub mod weights; pub use weights::WeightInfo; +pub mod migrations_3_0_0; + /// The maximum votes allowed per voter. pub const MAXIMUM_VOTE: usize = 16; @@ -127,17 +144,30 @@ pub enum Renouncing { 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 +/// An active voter. +#[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)] +pub struct Voter { + /// The members being backed. + pub votes: Vec, + /// The amount of stake placed on this vote. + pub stake: Balance, + /// The amount of deposit reserved for this vote. + /// + /// To be unreserved upon removal. + pub deposit: Balance, +} + +/// A holder of a seat as either a member or a runner-up. +#[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)] +pub struct SeatHolder { + /// The holder. 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 + /// The total backing stake. + pub stake: Balance, + /// The amount of deposit held on-chain. + /// + /// To be unreserved upon renouncing, or slashed upon being a loser. + pub deposit: Balance, } pub trait Config: frame_system::Config { @@ -165,15 +195,18 @@ pub trait Config: frame_system::Config { /// How much should be locked up in order to submit one's candidacy. type CandidacyBond: Get>; - /// How much should be locked up in order to be able to submit votes. - type VotingBond: Get>; + /// Base deposit associated with voting. + /// + /// This should be sensibly high to economically ensure the pallet cannot be attacked by + /// creating a gigantic number of votes. + type VotingBondBase: Get>; + + /// The amount of bond that need to be locked for each vote (32 bytes). + type VotingBondFactor: Get>; /// Handler for the unbalanced reduction when a candidate has lost (and is not a runner-up) type LoserCandidate: OnUnbalanced>; - /// Handler for the unbalanced reduction when a reporter has submitted a bad defunct report. - type BadReport: OnUnbalanced>; - /// Handler for the unbalanced reduction when a member has been kicked. type KickedMember: OnUnbalanced>; @@ -194,22 +227,32 @@ pub trait Config: frame_system::Config { decl_storage! { trait Store for Module as PhragmenElection { - // ---- State - /// The current elected membership. Sorted based on account id. - pub Members get(fn members): Vec<(T::AccountId, BalanceOf)>; - /// The current runners_up. Sorted based on low to high merit (worse to best). - pub RunnersUp get(fn runners_up): Vec<(T::AccountId, BalanceOf)>; + /// The current elected members. + /// + /// Invariant: Always sorted based on account id. + pub Members get(fn members): Vec>>; + + /// The current reserved runners-up. + /// + /// Invariant: Always sorted based on rank (worse to best). Upon removal of a member, the + /// last (i.e. _best_) runner-up will be replaced. + pub RunnersUp get(fn runners_up): Vec>>; + + /// The present candidate list. A current member or runner-up can never enter this vector + /// and is always implicitly assumed to be a candidate. + /// + /// Second element is the deposit. + /// + /// Invariant: Always sorted based on account id. + pub Candidates get(fn candidates): Vec<(T::AccountId, BalanceOf)>; + /// The total number of vote rounds that have happened, excluding the upcoming one. pub ElectionRounds get(fn election_rounds): u32 = Zero::zero(); /// Votes and locked stake of a particular voter. /// - /// TWOX-NOTE: SAFE as `AccountId` is a crypto hash - pub Voting get(fn voting): map hasher(twox_64_concat) T::AccountId => (BalanceOf, Vec); - - /// The present candidate list. Sorted based on account-id. A current member or runner-up - /// can never enter this vector and is always implicitly assumed to be a candidate. - pub Candidates get(fn candidates): Vec; + /// TWOX-NOTE: SAFE as `AccountId` is a crypto hash. + pub Voting get(fn voting): map hasher(twox_64_concat) T::AccountId => Voter>; } add_extra_genesis { config(members): Vec<(T::AccountId, BalanceOf)>; build(|config: &GenesisConfig| { @@ -218,32 +261,33 @@ decl_storage! { "Cannot accept more than DesiredMembers genesis member", ); let members = config.members.iter().map(|(ref member, ref stake)| { - // make sure they have enough stake + // make sure they have enough stake. assert!( T::Currency::free_balance(member) >= *stake, - "Genesis member does not have enough stake", + "Genesis member does not have enough stake.", ); - // reserve candidacy bond and set as members. - T::Currency::reserve(&member, T::CandidacyBond::get()) - .expect("Genesis member does not have enough balance to be a candidate"); - // Note: all members will only vote for themselves, hence they must be given exactly // their own stake as total backing. Any sane election should behave as such. // Nonetheless, stakes will be updated for term 1 onwards according to the election. Members::::mutate(|members| { - match members.binary_search_by(|(a, _b)| a.cmp(member)) { - Ok(_) => panic!("Duplicate member in elections phragmen genesis: {}", member), - Err(pos) => members.insert(pos, (member.clone(), *stake)), + match members.binary_search_by(|m| m.who.cmp(member)) { + Ok(_) => panic!("Duplicate member in elections-phragmen genesis: {}", member), + Err(pos) => members.insert( + pos, + SeatHolder { who: member.clone(), stake: *stake, deposit: Zero::zero() }, + ), } }); - // set self-votes to make persistent. - >::vote( - T::Origin::from(Some(member.clone()).into()), - vec![member.clone()], - *stake, - ).expect("Genesis member could not vote."); + // set self-votes to make persistent. Genesis voters don't have any bond, nor do + // they have any lock. NOTE: this means that we will still try to remove a lock once + // this genesis voter is removed, and for now it is okay because remove_lock is noop + // if lock is not there. + >::insert( + &member, + Voter { votes: vec![member.clone()], stake: *stake, deposit: Zero::zero() }, + ); member.clone() }).collect::>(); @@ -277,13 +321,13 @@ decl_error! { /// Member cannot re-submit candidacy. MemberSubmit, /// Runner cannot re-submit candidacy. - RunnerSubmit, + RunnerUpSubmit, /// Candidate does not have enough funds. InsufficientCandidateFunds, /// Not a member. NotMember, /// The provided count of number of candidates is incorrect. - InvalidCandidateCount, + InvalidWitnessData, /// The provided count of number of votes is incorrect. InvalidVoteCount, /// The renouncing origin presented a wrong `Renouncing` parameter. @@ -293,46 +337,74 @@ decl_error! { } } +decl_event!( + pub enum Event where Balance = BalanceOf, ::AccountId { + /// A new term with \[new_members\]. This indicates that enough candidates existed to run the + /// election, not that enough have has been elected. The inner value must be examined for + /// this purpose. A `NewTerm(\[\])` indicates that some candidates got their bond slashed and + /// none were elected, whilst `EmptyTerm` means that no candidates existed to begin with. + NewTerm(Vec<(AccountId, Balance)>), + /// No (or not enough) candidates existed for this round. This is different from + /// `NewTerm(\[\])`. See the description of `NewTerm`. + EmptyTerm, + /// Internal error happened while trying to perform election. + ElectionError, + /// A \[member\] has been removed. This should always be followed by either `NewTerm` or + /// `EmptyTerm`. + MemberKicked(AccountId), + /// Someone has renounced their candidacy. + Renounced(AccountId), + /// A \[candidate\] was slashed by \[amount\] due to failing to obtain a seat as member or + /// runner-up. + /// + /// Note that old members and runners-up are also candidates. + CandidateSlashed(AccountId, Balance), + /// A \[seat holder\] was slashed by \[amount\] by being forcefully removed from the set. + SeatHolderSlashed(AccountId, Balance), + } +); + decl_module! { pub struct Module for enum Call where origin: T::Origin { type Error = Error; - fn deposit_event() = default; const CandidacyBond: BalanceOf = T::CandidacyBond::get(); - const VotingBond: BalanceOf = T::VotingBond::get(); + const VotingBondBase: BalanceOf = T::VotingBondBase::get(); + const VotingBondFactor: BalanceOf = T::VotingBondFactor::get(); const DesiredMembers: u32 = T::DesiredMembers::get(); const DesiredRunnersUp: u32 = T::DesiredRunnersUp::get(); const TermDuration: T::BlockNumber = T::TermDuration::get(); - const ModuleId: LockIdentifier = T::ModuleId::get(); + const ModuleId: LockIdentifier = T::ModuleId::get(); /// 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. + /// Upon initial voting, `value` units of `who`'s balance is locked and a deposit amount is + /// reserved. The deposit is based on the number of votes and can be updated over time. /// /// The `votes` should: /// - not be empty. /// - be less than the number of possible candidates. Note that all current members and /// runners-up are also automatically candidates for the next round. /// - /// It is the responsibility of the caller to not place all of their balance into the lock - /// and keep some for further transactions. + /// If `value` is more than `who`'s total balance, then the maximum of the two is used. + /// + /// The dispatch origin of this call must be signed. + /// + /// ### Warning + /// + /// It is the responsibility of the caller to **NOT** place all of their balance into the + /// lock and keep some for further operations. /// /// # - /// Base weight: 47.93 µs - /// State reads: - /// - Candidates.len() + Members.len() + RunnersUp.len() - /// - Voting (is_voter) - /// - Lock - /// - [AccountBalance(who) (unreserve + total_balance)] - /// State writes: - /// - Voting - /// - Lock - /// - [AccountBalance(who) (unreserve -- only when creating a new voter)] + /// We assume the maximum weight among all 3 cases: vote_equal, vote_more and vote_less. /// # - #[weight = T::WeightInfo::vote(votes.len() as u32)] + #[weight = + T::WeightInfo::vote_more(votes.len() as u32) + .max(T::WeightInfo::vote_less(votes.len() as u32)) + .max(T::WeightInfo::vote_equal(votes.len() as u32)) + ] fn vote( origin, votes: Vec, @@ -340,6 +412,7 @@ decl_module! { ) { let who = ensure_signed(origin)?; + // votes should not be empty and more than `MAXIMUM_VOTE` in any case. ensure!(votes.len() <= MAXIMUM_VOTE, Error::::MaximumVotesExceeded); ensure!(!votes.is_empty(), Error::::NoVotes); @@ -347,156 +420,73 @@ decl_module! { let members_count = >::decode_len().unwrap_or(0); let runners_up_count = >::decode_len().unwrap_or(0); + // can never submit a vote of there are no members, and cannot submit more votes than + // all potential vote targets. // addition is valid: candidates, members and runners-up will never overlap. - let allowed_votes = candidates_count + members_count + runners_up_count; - + let allowed_votes = candidates_count + .saturating_add(members_count) + .saturating_add(runners_up_count); ensure!(!allowed_votes.is_zero(), Error::::UnableToVote); ensure!(votes.len() <= allowed_votes, Error::::TooManyVotes); ensure!(value > T::Currency::minimum_balance(), Error::::LowBalance); - // first time voter. Reserve bond. - if !Self::is_voter(&who) { - T::Currency::reserve(&who, T::VotingBond::get()) - .map_err(|_| Error::::UnableToPayBond)?; - } + // Reserve bond. + let new_deposit = Self::deposit_of(votes.len()); + let Voter { deposit: old_deposit, .. } = >::get(&who); + match new_deposit.cmp(&old_deposit) { + Ordering::Greater => { + // Must reserve a bit more. + let to_reserve = new_deposit - old_deposit; + T::Currency::reserve(&who, to_reserve).map_err(|_| Error::::UnableToPayBond)?; + }, + Ordering::Equal => {}, + Ordering::Less => { + // Must unreserve a bit. + let to_unreserve = old_deposit - new_deposit; + let _remainder = T::Currency::unreserve(&who, to_unreserve); + debug_assert!(_remainder.is_zero()); + }, + }; // Amount to be locked up. - let locked_balance = value.min(T::Currency::total_balance(&who)); - - // lock + let locked_stake = value.min(T::Currency::total_balance(&who)); T::Currency::set_lock( T::ModuleId::get(), &who, - locked_balance, - WithdrawReasons::except(WithdrawReasons::TRANSACTION_PAYMENT), + locked_stake, + WithdrawReasons::all(), ); - Voting::::insert(&who, (locked_balance, votes)); + Voting::::insert(&who, Voter { votes, deposit: new_deposit, stake: locked_stake }); } - /// Remove `origin` as a voter. This removes the lock and returns the bond. + /// Remove `origin` as a voter. /// - /// # - /// Base weight: 36.8 µs - /// All state access is from do_remove_voter. - /// State reads: - /// - Voting - /// - [AccountData(who)] - /// State writes: - /// - Voting - /// - Locks - /// - [AccountData(who)] - /// # + /// This removes the lock and returns the deposit. + /// + /// The dispatch origin of this call must be signed and be a voter. #[weight = T::WeightInfo::remove_voter()] fn remove_voter(origin) { let who = ensure_signed(origin)?; ensure!(Self::is_voter(&who), Error::::MustBeVoter); - - Self::do_remove_voter(&who, true); + Self::do_remove_voter(&who); } - /// Report `target` for being an defunct voter. In case of a valid report, the reporter is - /// rewarded by the bond amount of `target`. Otherwise, the reporter itself is removed and - /// their bond is slashed. + /// Submit oneself for candidacy. A fixed amount of deposit is recorded. /// - /// 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 or a runner-up. + /// All candidates are wiped at the end of the term. They either become a member/runner-up, + /// or leave the system while their deposit is slashed. /// + /// The dispatch origin of this call must be signed. /// - /// The origin must provide the number of current candidates and votes of the reported target - /// for the purpose of accurate weight calculation. - /// - /// # - /// 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 = T::WeightInfo::report_defunct_voter_correct( - defunct.candidate_count, - defunct.vote_count, - )] - fn report_defunct_voter( - origin, - defunct: DefunctVoter<::Source>, - ) -> DispatchResultWithPostInfo { - let reporter = ensure_signed(origin)?; - let target = T::Lookup::lookup(defunct.who)?; - - ensure!(reporter != target, Error::::ReportSelf); - ensure!(Self::is_voter(&reporter), Error::::MustBeVoter); - - 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); - let maybe_refund = if valid { - // reporter will get the voting bond of the target - T::Currency::repatriate_reserved(&target, &reporter, T::VotingBond::get(), BalanceStatus::Free)?; - // remove the target. They are defunct. - Self::do_remove_voter(&target, false); - None - } else { - // slash the bond of the reporter. - let imbalance = T::Currency::slash_reserved(&reporter, T::VotingBond::get()).0; - T::BadReport::on_unbalanced(imbalance); - // remove the reporter. - Self::do_remove_voter(&reporter, false); - Some(T::WeightInfo::report_defunct_voter_incorrect( - defunct.candidate_count, - defunct.vote_count, - )) - }; - Self::deposit_event(RawEvent::VoterReported(target, reporter, valid)); - Ok(maybe_refund.into()) - } - - /// Submit oneself for candidacy. + /// ### Warning /// - /// A candidate will either: - /// - Lose at the end of the term and forfeit their deposit. - /// - Win and become a member. Members will eventually get their stash back. - /// - Become a runner-up. Runners-ups are reserved members in case one gets forcefully - /// removed. + /// Even if a candidate ends up being a member, they must call [`Call::renounce_candidacy`] + /// to get their deposit back. Losing the spot in an election will always lead to a slash. /// /// # - /// Base weight = 33.33 µs - /// Complexity of candidate_count: 0.375 µs - /// State reads: - /// - Candidates - /// - Members - /// - RunnersUp - /// - [AccountBalance(who)] - /// State writes: - /// - [AccountBalance(who)] - /// - Candidates + /// The number of current candidates must be provided as witness data. /// # #[weight = T::WeightInfo::submit_candidacy(*candidate_count)] fn submit_candidacy(origin, #[compact] candidate_count: u32) { @@ -505,60 +495,37 @@ decl_module! { let actual_count = >::decode_len().unwrap_or(0); ensure!( actual_count as u32 <= candidate_count, - Error::::InvalidCandidateCount, + Error::::InvalidWitnessData, ); - 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(); + let index = Self::is_candidate(&who).err().ok_or(Error::::DuplicatedCandidate)?; ensure!(!Self::is_member(&who), Error::::MemberSubmit); - ensure!(!Self::is_runner_up(&who), Error::::RunnerSubmit); + ensure!(!Self::is_runner_up(&who), Error::::RunnerUpSubmit); T::Currency::reserve(&who, T::CandidacyBond::get()) .map_err(|_| Error::::InsufficientCandidateFunds)?; - >::mutate(|c| c.insert(index, who)); + >::mutate(|c| c.insert(index, (who, T::CandidacyBond::get()))); } /// Renounce one's intention to be a candidate for the next election round. 3 potential /// outcomes exist: - /// - `origin` is a candidate and not elected in any set. In this case, the bond is + /// + /// - `origin` is a candidate and not elected in any set. In this case, the deposit 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 a current runner-up. In this case, the deposit 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 + /// - `origin` is a current member. In this case, the deposit 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. - /// - /// 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)] - /// + /// Similar to [`remove_members`], if replacement runners exists, they are immediately used. + /// If the prime is renouncing, then no prime will exist until the next round. + /// + /// The dispatch origin of this call must be signed, and have one of the above roles. + /// + /// # + /// The type of renouncing must be provided as witness data. + /// # #[weight = match *renouncing { Renouncing::Candidate(count) => T::WeightInfo::renounce_candidacy_candidate(count), Renouncing::Member => T::WeightInfo::renounce_candidacy_members(), @@ -568,38 +535,36 @@ decl_module! { let who = ensure_signed(origin)?; 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)); + let _ = Self::remove_and_replace_member(&who, false) + .map_err(|_| Error::::InvalidRenouncing)?; + Self::deposit_event(RawEvent::Renounced(who)); }, 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)?; - } + >::try_mutate::<_, Error, _>(|runners_up| { + let index = runners_up + .iter() + .position(|SeatHolder { who: r, .. }| r == &who) + .ok_or(Error::::InvalidRenouncing)?; + // can't fail anymore. + let SeatHolder { deposit, .. } = runners_up.remove(index); + let _remainder = T::Currency::unreserve(&who, deposit); + debug_assert!(_remainder.is_zero()); + Self::deposit_event(RawEvent::Renounced(who)); + Ok(()) + })?; } 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)?; - } + >::try_mutate::<_, Error, _>(|candidates| { + ensure!(count >= candidates.len() as u32, Error::::InvalidWitnessData); + let index = candidates + .binary_search_by(|(c, _)| c.cmp(&who)) + .map_err(|_| Error::::InvalidRenouncing)?; + let (_removed, deposit) = candidates.remove(index); + let _remainder = T::Currency::unreserve(&who, deposit); + debug_assert!(_remainder.is_zero()); + Self::deposit_event(RawEvent::Renounced(who)); + Ok(()) + })?; } }; } @@ -610,17 +575,13 @@ decl_module! { /// If a runner-up is available, then the best runner-up will be removed and replaces the /// outgoing member. Otherwise, a new phragmen election is started. /// + /// The dispatch origin of this call must be root. + /// /// Note that this does not affect the designated block number of the next election. /// /// # - /// 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. + /// If we have a replacement, we use a small weight. Else, since this is a root call and + /// will go into phragmen, we assume full block for now. /// # #[weight = if *has_replacement { T::WeightInfo::remove_member_with_replacement() @@ -635,164 +596,196 @@ decl_module! { ensure_root(origin)?; let who = T::Lookup::lookup(who)?; - let will_have_replacement = >::decode_len().unwrap_or(0) > 0; + let will_have_replacement = >::decode_len().map_or(false, |l| l > 0); if will_have_replacement != has_replacement { - // In both cases, we will change more weight than neede. Refund and abort. + // In both cases, we will change more weight than need. 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 T::WeightInfo::remove_member_wrong_refund() )); - } // 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())); + let had_replacement = Self::remove_and_replace_member(&who, true)?; + debug_assert_eq!(has_replacement, had_replacement); + 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(); - } + if !had_replacement { + Self::do_phragmen(); + } - // no refund needed. - None.into() - }).map_err(|e| e.into()) + // no refund needed. + Ok(None.into()) } - /// What to do at the end of each block. Checks if an election needs to happen or not. + /// Clean all voters who are defunct (i.e. they do not serve any purpose at all). The + /// deposit of the removed voters are returned. + /// + /// This is an root function to be used only for cleaning the state. + /// + /// The dispatch origin of this call must be root. + /// + /// # + /// The total number of voters and those that are defunct must be provided as witness data. + /// # + #[weight = T::WeightInfo::clean_defunct_voters(*_num_voters, *_num_defunct)] + fn clean_defunct_voters(origin, _num_voters: u32, _num_defunct: u32) { + let _ = ensure_root(origin)?; + >::iter() + .filter(|(_, x)| Self::is_defunct_voter(&x.votes)) + .for_each(|(dv, _)| { + Self::do_remove_voter(&dv) + }) + } + + /// 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 { - // returns the correct weight. - Self::end_block(n) + let term_duration = T::TermDuration::get(); + if !term_duration.is_zero() && (n % term_duration).is_zero() { + Self::do_phragmen() + } else { + 0 + } } } } -decl_event!( - pub enum Event where - Balance = BalanceOf, - ::AccountId, - { - /// A new term with \[new_members\]. This indicates that enough candidates existed to run the - /// election, not that enough have has been elected. The inner value must be examined for - /// this purpose. A `NewTerm(\[\])` indicates that some candidates got their bond slashed and - /// none were elected, whilst `EmptyTerm` means that no candidates existed to begin with. - NewTerm(Vec<(AccountId, Balance)>), - /// No (or not enough) candidates existed for this round. This is different from - /// `NewTerm(\[\])`. See the description of `NewTerm`. - EmptyTerm, - /// Internal error happened while trying to perform election. - ElectionError, - /// A \[member\] has been removed. This should always be followed by either `NewTerm` or - /// `EmptyTerm`. - MemberKicked(AccountId), - /// A candidate was slashed due to failing to obtain a seat as member or runner-up - CandidateSlashed(AccountId, Balance), - /// A seat holder (member or runner-up) was slashed due to failing to retaining their position. - SeatHolderSlashed(AccountId, Balance), - /// A \[member\] has renounced their candidacy. - MemberRenounced(AccountId), - /// A voter was reported with the the report being successful or not. - /// \[voter, reporter, success\] - VoterReported(AccountId, AccountId, bool), +impl Module { + /// The deposit value of `count` votes. + fn deposit_of(count: usize) -> BalanceOf { + T::VotingBondBase::get().saturating_add( + T::VotingBondFactor::get().saturating_mul((count as u32).into()) + ) } -); -impl Module { - /// Attempts to remove a member `who`. If a runner-up exists, it is used as the replacement and - /// Ok(true). is returned. + /// Attempts to remove a member `who`. If a runner-up exists, it is used as the replacement. /// - /// Otherwise, `Ok(false)` is returned to signal the caller. + /// Returns: /// - /// 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. + /// - `Ok(true)` if the member was removed and a replacement was found. + /// - `Ok(false)` if the member was removed and but no replacement was found. + /// - `Err(_)` if the member was no found. /// - /// Note that this function _will_ call into `T::ChangeMembers` in case any change happens - /// (`Ok(true)`). + /// Both `Members` and `RunnersUp` storage is updated accordingly. `T::ChangeMember` is called + /// if needed. If `slash` is true, the deposit of the potentially removed member is slashed, + /// else, it is unreserved. /// - /// 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)) { - members_with_stake.remove(index); - - let next_up = >::mutate(|runners_up| runners_up.pop()); - let maybe_replacement = next_up.and_then(|(replacement, stake)| - members_with_stake.binary_search_by(|(ref m, ref _s)| m.cmp(&replacement)) - .err() - .map(|index| { - members_with_stake.insert(index, (replacement.clone(), stake)); - replacement - }) - ); + /// ### Note: Prime preservation + /// + /// This function attempts to preserve the prime. If the removed members is not the prime, it is + /// set again via [`Config::ChangeMembers`]. + fn remove_and_replace_member(who: &T::AccountId, slash: bool) -> Result { + // closure will return: + // - `Ok(Option(replacement))` if member was removed and replacement was replaced. + // - `Ok(None)` if member was removed but no replacement was found + // - `Err(_)` if who is not a member. + let maybe_replacement = >::try_mutate::<_, Error, _>(|members| { + let remove_index = + members.binary_search_by(|m| m.who.cmp(who)).map_err(|_| Error::::NotMember)?; + // we remove the member anyhow, regardless of having a runner-up or not. + let removed = members.remove(remove_index); + + // slash or unreserve + if slash { + let (imbalance, _remainder) = T::Currency::slash_reserved(who, removed.deposit); + debug_assert!(_remainder.is_zero()); + T::LoserCandidate::on_unbalanced(imbalance); + Self::deposit_event(RawEvent::SeatHolderSlashed(who.clone(), removed.deposit)); + } else { + T::Currency::unreserve(who, removed.deposit); + } - >::put(&members_with_stake); - let members = members_with_stake.into_iter().map(|m| m.0).collect::>(); - let result = Ok(maybe_replacement.is_some()); - let old = [who.clone()]; - match maybe_replacement { - Some(new) => T::ChangeMembers::change_members_sorted(&[new], &old, &members), - None => T::ChangeMembers::change_members_sorted(&[], &old, &members), + let maybe_next_best = >::mutate(|r| r.pop()).map(|next_best| { + // defensive-only: Members and runners-up are disjoint. This will always be err and + // give us an index to insert. + if let Err(index) = members.binary_search_by(|m| m.who.cmp(&next_best.who)) { + members.insert(index, next_best.clone()); + } else { + // overlap. This can never happen. If so, it seems like our intended replacement + // is already a member, so not much more to do. + frame_support::debug::error!( + "pallet-elections-phragmen: a member seems to also be a runner-up." + ); + } + next_best + }); + Ok(maybe_next_best) + })?; + + let remaining_member_ids_sorted = Self::members() + .into_iter() + .map(|x| x.who.clone()) + .collect::>(); + let outgoing = &[who.clone()]; + let maybe_current_prime = T::ChangeMembers::get_prime(); + let return_value = match maybe_replacement { + // member ids are already sorted, other two elements have one item. + Some(incoming) => { + T::ChangeMembers::change_members_sorted( + &[incoming.who], + outgoing, + &remaining_member_ids_sorted[..] + ); + true + } + None => { + T::ChangeMembers::change_members_sorted( + &[], + outgoing, + &remaining_member_ids_sorted[..] + ); + false + } + }; + + // if there was a prime before and they are not the one being removed, then set them + // again. + if let Some(current_prime) = maybe_current_prime { + if ¤t_prime != who { + T::ChangeMembers::set_prime(Some(current_prime)); } - result - } else { - Err(Error::::NotMember)? } + + Ok(return_value) } /// Check if `who` is a candidate. It returns the insert index if the element does not exists as /// an error. - /// - /// O(LogN) given N candidates. fn is_candidate(who: &T::AccountId) -> Result<(), usize> { - Self::candidates().binary_search(who).map(|_| ()) + Self::candidates().binary_search_by(|c| c.0.cmp(who)).map(|_| ()) } /// Check if `who` is a voter. It may or may not be a _current_ one. - /// - /// State: O(1). fn is_voter(who: &T::AccountId) -> bool { Voting::::contains_key(who) } /// Check if `who` is currently an active member. - /// - /// 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() + Self::members().binary_search_by(|m| m.who.cmp(who)).is_ok() } /// Check if `who` is currently an active runner-up. - /// - /// 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() - } - - /// Returns number of desired members. - fn desired_members() -> u32 { - T::DesiredMembers::get() - } - - /// Returns number of desired runners up. - fn desired_runners_up() -> u32 { - T::DesiredRunnersUp::get() - } - - /// Returns the term duration - fn term_duration() -> T::BlockNumber { - T::TermDuration::get() + Self::runners_up().iter().position(|r| &r.who == who).is_some() } /// Get the members' account ids. fn members_ids() -> Vec { - Self::members().into_iter().map(|(m, _)| m).collect::>() + Self::members().into_iter().map(|m| m.who).collect::>() } - /// The the runners' up account ids. - fn runners_up_ids() -> Vec { - Self::runners_up().into_iter().map(|(r, _)| r).collect::>() + /// Get a concatenation of previous members and runners-up and their deposits. + /// + /// These accounts are essentially treated as candidates. + fn implicit_candidates_with_deposit() -> Vec<(T::AccountId, BalanceOf)> { + // invariant: these two are always without duplicates. + Self::members() + .into_iter() + .map(|m| (m.who, m.deposit)) + .chain(Self::runners_up().into_iter().map(|r| (r.who, r.deposit))) + .collect::>() } /// Check if `votes` will correspond to a defunct voter. As no origin is part of the inputs, @@ -809,118 +802,115 @@ impl Module { } /// 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); + fn do_remove_voter(who: &T::AccountId) { + let Voter { deposit, .. } = >::take(who); + + // remove storage, lock and unreserve. T::Currency::remove_lock(T::ModuleId::get(), who); - if unreserve { - T::Currency::unreserve(who, T::VotingBond::get()); - } - } - - /// 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) -> Weight { - if !Self::term_duration().is_zero() { - if (block_number % Self::term_duration()).is_zero() { - Self::do_phragmen(); - return T::BlockWeights::get().max_block; - } - } - 0 + // NOTE: we could check the deposit amount before removing and skip if zero, but it will be + // a noop anyhow. + let _remainder = T::Currency::unreserve(who, deposit); + debug_assert!(_remainder.is_zero()); } /// Run the phragmen election with all required side processes and state updates, if election /// succeeds. Else, it will emit an `ElectionError` event. /// /// Calls the appropriate [`ChangeMembers`] function variant internally. - /// - /// Reads: O(C + V*E) where C = candidates, V voters and E votes per voter exits. - /// Writes: O(M + R) with M desired members and R runners_up. - fn do_phragmen() { - let desired_seats = Self::desired_members() as usize; - let desired_runners_up = Self::desired_runners_up() as usize; + fn do_phragmen() -> Weight { + let desired_seats = T::DesiredMembers::get() as usize; + let desired_runners_up = T::DesiredRunnersUp::get() as usize; let num_to_elect = desired_runners_up + desired_seats; - let mut candidates = Self::candidates(); - // candidates who explicitly called `submit_candidacy`. Only these folks are at risk of - // losing their bond. - let exposed_candidates = candidates.clone(); - // current members are always a candidate for the next round as well. - // this is guaranteed to not create any duplicates. - candidates.append(&mut Self::members_ids()); - // previous runners_up are also always candidates for the next round. - candidates.append(&mut Self::runners_up_ids()); - - if candidates.len().is_zero() { + let mut candidates_and_deposit = Self::candidates(); + // add all the previous members and runners-up as candidates as well. + candidates_and_deposit.append(&mut Self::implicit_candidates_with_deposit()); + + if candidates_and_deposit.len().is_zero() { Self::deposit_event(RawEvent::EmptyTerm); - return; + return T::DbWeight::get().reads(5); } + // All of the new winners that come out of phragmen will thus have a deposit recorded. + let candidate_ids = candidates_and_deposit + .iter() + .map(|(x, _)| x) + .cloned() + .collect::>(); + // helper closures to deal with balance/stake. let total_issuance = T::Currency::total_issuance(); let to_votes = |b: BalanceOf| T::CurrencyToVote::to_vote(b, total_issuance); let to_balance = |e: ExtendedBalance| T::CurrencyToVote::to_currency(e, total_issuance); + let mut num_edges: u32 = 0; // used for prime election. let voters_and_stakes = Voting::::iter() - .map(|(voter, (stake, votes))| (voter, stake, votes)) + .map(|(voter, Voter { stake, votes, .. })| { (voter, stake, votes) }) .collect::>(); // used for phragmen. let voters_and_votes = voters_and_stakes.iter() .cloned() - .map(|(voter, stake, votes)| { (voter, to_votes(stake), votes)} ) + .map(|(voter, stake, votes)| { + num_edges = num_edges.saturating_add(votes.len() as u32); + (voter, to_votes(stake), votes) + }) .collect::>(); + let weight_candidates = candidates_and_deposit.len() as u32; + let weight_voters = voters_and_votes.len() as u32; + let weight_edges = num_edges; let _ = sp_npos_elections::seq_phragmen::( num_to_elect, - candidates, + candidate_ids, voters_and_votes.clone(), None, - ).map(|ElectionResult { winners, assignments: _ }| { + ).map(|ElectionResult { winners, assignments: _, }| { // this is already sorted by id. let old_members_ids_sorted = >::take().into_iter() - .map(|(m, _)| m) + .map(|m| m.who) .collect::>(); // this one needs a sort by id. let mut old_runners_up_ids_sorted = >::take().into_iter() - .map(|(r, _)| r) + .map(|r| r.who) .collect::>(); old_runners_up_ids_sorted.sort(); // filter out those who end up with no backing stake. - let new_set_with_stake = winners + let mut new_set_with_stake = winners .into_iter() .filter_map(|(m, b)| if b.is_zero() { None } else { Some((m, to_balance(b))) }) .collect::)>>(); - // OPTIMISATION NOTE: we could bail out here if `new_set.len() == 0`. There isn't much - // left to do. Yet, re-arranging the code would require duplicating the slashing of - // exposed candidates, cleaning any previous members, and so on. For now, in favour of - // readability and veracity, we keep it simple. + // OPTIMIZATION NOTE: we could bail out here if `new_set.len() == 0`. There isn't + // much left to do. Yet, re-arranging the code would require duplicating the + // slashing of exposed candidates, cleaning any previous members, and so on. For + // now, in favor of readability and veracity, we keep it simple. // split new set into winners and runners up. let split_point = desired_seats.min(new_set_with_stake.len()); - let mut new_members_sorted_by_id = (&new_set_with_stake[..split_point]).to_vec(); - - // save the runners up as-is. They are sorted based on desirability. - // save the members, sorted based on account id. + let mut new_members_sorted_by_id = new_set_with_stake.drain(..split_point).collect::>(); new_members_sorted_by_id.sort_by(|i, j| i.0.cmp(&j.0)); - // Now we select a prime member using a [Borda count](https://en.wikipedia.org/wiki/Borda_count). - // We weigh everyone's vote for that new member by a multiplier based on the order - // of the votes. i.e. the first person a voter votes for gets a 16x multiplier, - // the next person gets a 15x multiplier, an so on... (assuming `MAXIMUM_VOTE` = 16) - let mut prime_votes: Vec<_> = new_members_sorted_by_id.iter().map(|c| (&c.0, BalanceOf::::zero())).collect(); + // all the rest will be runners-up + new_set_with_stake.reverse(); + let new_runners_up_sorted_by_rank = new_set_with_stake; + let mut new_runners_up_ids_sorted = new_runners_up_sorted_by_rank + .iter() + .map(|(r, _)| r.clone()) + .collect::>(); + new_runners_up_ids_sorted.sort(); + + // Now we select a prime member using a [Borda + // count](https://en.wikipedia.org/wiki/Borda_count). We weigh everyone's vote for + // that new member by a multiplier based on the order of the votes. i.e. the first + // person a voter votes for gets a 16x multiplier, the next person gets a 15x + // multiplier, an so on... (assuming `MAXIMUM_VOTE` = 16) + let mut prime_votes = new_members_sorted_by_id + .iter() + .map(|c| (&c.0, BalanceOf::::zero())) + .collect::>(); for (_, stake, votes) in voters_and_stakes.into_iter() { for (vote_multiplier, who) in votes.iter() .enumerate() @@ -933,9 +923,9 @@ impl Module { } } } - // We then select the new member with the highest weighted stake. In the case of - // a tie, the last person in the list with the tied score is selected. This is - // the person with the "highest" account id based on the sort above. + // We then select the new member with the highest weighted stake. In the case of a tie, + // the last person in the list with the tied score is selected. This is the person with + // the "highest" account id based on the sort above. let prime = prime_votes.into_iter().max_by_key(|x| x.1).map(|x| x.0.clone()); // new_members_sorted_by_id is sorted by account id. @@ -944,20 +934,8 @@ impl Module { .map(|(m, _)| m.clone()) .collect::>(); - let new_runners_up_sorted_by_rank = &new_set_with_stake[split_point..] - .into_iter() - .cloned() - .rev() - .collect::)>>(); - // new_runners_up remains sorted by desirability. - let mut new_runners_up_ids_sorted = new_runners_up_sorted_by_rank - .iter() - .map(|(r, _)| r.clone()) - .collect::>(); - new_runners_up_ids_sorted.sort(); - // report member changes. We compute diff because we need the outgoing list. - let (incoming, outgoing) = T::ChangeMembers::compute_members_diff( + let (incoming, outgoing) = T::ChangeMembers::compute_members_diff_sorted( &new_members_ids_sorted, &old_members_ids_sorted, ); @@ -968,66 +946,63 @@ impl Module { ); T::ChangeMembers::set_prime(prime); - // outgoing members who are no longer a runner-up lose their bond. - let mut to_burn_bond = outgoing - .iter() - .filter(|o| new_runners_up_ids_sorted.binary_search(o).is_err()) - .cloned() - .collect::>(); - - // compute the outgoing of runners up as well and append them to the `to_burn_bond`, if - // they are not members. - { - let (_, outgoing) = T::ChangeMembers::compute_members_diff( - &new_runners_up_ids_sorted, - &old_runners_up_ids_sorted, - ); - // none of the ones computed to be outgoing must still be in the list. - debug_assert!(outgoing.iter().all(|o| !new_runners_up_ids_sorted.contains(o))); - to_burn_bond.extend( - outgoing - .iter() - .filter(|o| new_members_ids_sorted.binary_search(o).is_err()) - .cloned() - .collect::>() - ); - } - - // Burn loser bond. members list is sorted. O(NLogM) (N candidates, M members) - // runner up list is also sorted. O(NLogK) given K runner ups. Overall: O(NLogM + N*K) - // both the member and runner counts are bounded. - exposed_candidates.into_iter().for_each(|c| { - // any candidate who is not a member and not a runner up. + // All candidates/members/runners-up who are no longer retaining a position as a + // seat holder will lose their bond. + candidates_and_deposit.iter().for_each(|(c, d)| { if - new_members_ids_sorted.binary_search(&c).is_err() && - new_runners_up_ids_sorted.binary_search(&c).is_err() + new_members_ids_sorted.binary_search(c).is_err() && + new_runners_up_ids_sorted.binary_search(c).is_err() { - let (imbalance, _) = T::Currency::slash_reserved(&c, T::CandidacyBond::get()); - Self::deposit_event(RawEvent::CandidateSlashed(c, T::CandidacyBond::get())); + let (imbalance, _) = T::Currency::slash_reserved(c, *d); T::LoserCandidate::on_unbalanced(imbalance); + Self::deposit_event(RawEvent::CandidateSlashed(c.clone(), *d)); } }); - // Burn outgoing bonds - to_burn_bond.into_iter().for_each(|x| { - let (imbalance, _) = T::Currency::slash_reserved(&x, T::CandidacyBond::get()); - Self::deposit_event(RawEvent::SeatHolderSlashed(x, T::CandidacyBond::get())); - T::LoserCandidate::on_unbalanced(imbalance); - }); - - >::put(&new_members_sorted_by_id); - >::put(new_runners_up_sorted_by_rank); - - Self::deposit_event(RawEvent::NewTerm(new_members_sorted_by_id.clone().to_vec())); + // write final values to storage. + let deposit_of_candidate = |x: &T::AccountId| -> BalanceOf { + // defensive-only. This closure is used against the new members and new runners-up, + // both of which are phragmen winners and thus must have deposit. + candidates_and_deposit + .iter() + .find_map(|(c, d)| if c == x { Some(*d) } else { None }) + .unwrap_or_default() + }; + // fetch deposits from the one recorded one. This will make sure that a candidate who + // submitted candidacy before a change to candidacy deposit will have the correct amount + // recorded. + >::put( + new_members_sorted_by_id + .iter() + .map(|(who, stake)| SeatHolder { + deposit: deposit_of_candidate(&who), + who: who.clone(), + stake: stake.clone(), + }) + .collect::>(), + ); + >::put( + new_runners_up_sorted_by_rank + .into_iter() + .map(|(who, stake)| SeatHolder { + deposit: deposit_of_candidate(&who), + who, + stake, + }) + .collect::>(), + ); // clean candidates. >::kill(); + Self::deposit_event(RawEvent::NewTerm(new_members_sorted_by_id)); ElectionRounds::mutate(|v| *v += 1); }).map_err(|e| { frame_support::debug::error!("elections-phragmen: failed to run election [{:?}].", e); Self::deposit_event(RawEvent::ElectionError); }); + + T::WeightInfo::election_phragmen(weight_candidates, weight_voters, weight_edges) } } @@ -1035,16 +1010,19 @@ impl Contains for Module { fn contains(who: &T::AccountId) -> bool { Self::is_member(who) } - fn sorted_members() -> Vec { Self::members_ids() } + + fn sorted_members() -> Vec { + Self::members_ids() + } // A special function to populate members in this pallet for passing Origin // checks in runtime benchmarking. #[cfg(feature = "runtime-benchmarks")] fn add(who: &T::AccountId) { Members::::mutate(|members| { - match members.binary_search_by(|(a, _b)| a.cmp(who)) { + match members.binary_search_by(|m| m.who.cmp(who)) { Ok(_) => (), - Err(pos) => members.insert(pos, (who.clone(), BalanceOf::::default())), + Err(pos) => members.insert(pos, SeatHolder { who: who.clone(), ..Default::default() }), } }) } @@ -1055,14 +1033,16 @@ impl ContainsLengthBound for Module { /// Implementation uses a parameter type so calling is cost-free. fn max_len() -> usize { - Self::desired_members() as usize + T::DesiredMembers::get() as usize } } #[cfg(test)] mod tests { use super::*; - use frame_support::{assert_ok, assert_noop, assert_err_with_weight, parameter_types}; + use frame_support::{assert_ok, assert_noop, parameter_types, + traits::OnInitialize, + }; use substrate_test_utils::assert_eq_uvec; use sp_core::H256; use sp_runtime::{ @@ -1079,7 +1059,7 @@ mod tests { impl frame_system::Config for Test { type BaseCallFilter = (); - type BlockWeights = (); + type BlockWeights = BlockWeights; type BlockLength = (); type DbWeight = (); type Origin = Origin; @@ -1116,14 +1096,12 @@ mod tests { type WeightInfo = (); } - parameter_types! { - pub const CandidacyBond: u64 = 3; - } - frame_support::parameter_types! { - pub static VotingBond: u64 = 2; + pub static VotingBondBase: u64 = 2; + pub static VotingBondFactor: u64 = 0; + pub static CandidacyBond: u64 = 3; pub static DesiredMembers: u32 = 2; - pub static DesiredRunnersUp: u32 = 2; + pub static DesiredRunnersUp: u32 = 0; pub static TermDuration: u64 = 5; pub static Members: Vec = vec![]; pub static Prime: Option = None; @@ -1167,9 +1145,13 @@ mod tests { fn set_prime(who: Option) { PRIME.with(|p| *p.borrow_mut() = who); } + + fn get_prime() -> Option { + PRIME.with(|p| *p.borrow()) + } } - parameter_types!{ + parameter_types! { pub const ElectionsPhragmenModuleId: LockIdentifier = *b"phrelect"; } @@ -1181,13 +1163,13 @@ mod tests { type ChangeMembers = TestChangeMembers; type InitializeMembers = (); type CandidacyBond = CandidacyBond; - type VotingBond = VotingBond; + type VotingBondBase = VotingBondBase; + type VotingBondFactor = VotingBondFactor; type TermDuration = TermDuration; type DesiredMembers = DesiredMembers; type DesiredRunnersUp = DesiredRunnersUp; type LoserCandidate = (); type KickedMember = (); - type BadReport = (); type WeightInfo = (); } @@ -1207,61 +1189,56 @@ mod tests { ); pub struct ExtBuilder { - genesis_members: Vec<(u64, u64)>, balance_factor: u64, - voter_bond: u64, - term_duration: u64, - desired_runners_up: u32, - desired_members: u32, + genesis_members: Vec<(u64, u64)>, } impl Default for ExtBuilder { fn default() -> Self { Self { - genesis_members: vec![], balance_factor: 1, - voter_bond: 2, - term_duration: 5, - desired_runners_up: 0, - desired_members: 2, + genesis_members: vec![], } } } impl ExtBuilder { - pub fn voter_bond(mut self, fee: u64) -> Self { - self.voter_bond = fee; + pub fn voter_bond(self, bond: u64) -> Self { + VOTING_BOND_BASE.with(|v| *v.borrow_mut() = bond); + self + } + pub fn voter_bond_factor(self, bond: u64) -> Self { + VOTING_BOND_FACTOR.with(|v| *v.borrow_mut() = bond); self } - pub fn desired_runners_up(mut self, count: u32) -> Self { - self.desired_runners_up = count; + pub fn desired_runners_up(self, count: u32) -> Self { + DESIRED_RUNNERS_UP.with(|v| *v.borrow_mut() = count); self } - pub fn term_duration(mut self, duration: u64) -> Self { - self.term_duration = duration; + pub fn term_duration(self, duration: u64) -> Self { + TERM_DURATION.with(|v| *v.borrow_mut() = duration); self } pub fn genesis_members(mut self, members: Vec<(u64, u64)>) -> Self { + MEMBERS.with(|m| { + *m.borrow_mut() = members + .iter() + .map(|(m, _)| m.clone()) + .collect::>() + }); self.genesis_members = members; self } - pub fn desired_members(mut self, count: u32) -> Self { - self.desired_members = count; + pub fn desired_members(self, count: u32) -> Self { + DESIRED_MEMBERS.with(|m| *m.borrow_mut() = count); self } pub fn balance_factor(mut self, factor: u64) -> Self { self.balance_factor = factor; self } - 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(); + MEMBERS.with(|m| *m.borrow_mut() = self.genesis_members.iter().map(|(m, _)| m.clone()).collect::>()); let mut ext: sp_io::TestExternalities = GenesisConfig { pallet_balances: Some(pallet_balances::GenesisConfig::{ balances: vec![ @@ -1283,6 +1260,40 @@ mod tests { } } + fn candidate_ids() -> Vec { + Elections::candidates() + .into_iter() + .map(|(c, _)| c) + .collect::>() + } + + fn candidate_deposit(who: &u64) -> u64 { + Elections::candidates() + .into_iter() + .find_map(|(c, d)| if c == *who { Some(d) } else { None }) + .unwrap_or_default() + } + + fn voter_deposit(who: &u64) -> u64 { + Elections::voting(who).deposit + } + + fn runners_up_ids() -> Vec { + Elections::runners_up().into_iter().map(|r| r.who).collect::>() + } + + fn members_ids() -> Vec { + Elections::members_ids() + } + + fn members_and_stake() -> Vec<(u64, u64)> { + Elections::members().into_iter().map(|m| (m.who, m.stake)).collect::>() + } + + fn runners_up_and_stake() -> Vec<(u64, u64)> { + Elections::runners_up().into_iter().map(|r| (r.who, r.stake)).collect::>() + } + fn all_voters() -> Vec { Voting::::iter().map(|(v, _)| v).collect::>() } @@ -1292,9 +1303,15 @@ mod tests { } fn has_lock(who: &u64) -> u64 { - let lock = Balances::locks(who)[0].clone(); - assert_eq!(lock.id, ElectionsPhragmenModuleId::get()); - lock.amount + dbg!(Balances::locks(who)); + Balances::locks(who) + .get(0) + .cloned() + .map(|lock| { + assert_eq!(lock.id, ElectionsPhragmenModuleId::get()); + lock.amount + }) + .unwrap_or_default() } fn intersects(a: &[T], b: &[T]) -> bool { @@ -1303,41 +1320,41 @@ mod tests { fn ensure_members_sorted() { let mut members = Elections::members().clone(); - members.sort(); + members.sort_by_key(|m| m.who); assert_eq!(Elections::members(), members); } fn ensure_candidates_sorted() { let mut candidates = Elections::candidates().clone(); - candidates.sort(); + candidates.sort_by_key(|(c, _)| *c); assert_eq!(Elections::candidates(), candidates); } fn locked_stake_of(who: &u64) -> u64 { - Voting::::get(who).0 + Voting::::get(who).stake } fn ensure_members_has_approval_stake() { // we filter members that have no approval state. This means that even we have more seats // than candidates, we will never ever chose a member with no votes. - assert!( - Elections::members().iter().chain( - Elections::runners_up().iter() - ).all(|(_, s)| *s != u64::zero()) - ); + assert!(Elections::members() + .iter() + .chain(Elections::runners_up().iter()) + .all(|s| s.stake != u64::zero())); } fn ensure_member_candidates_runners_up_disjoint() { // members, candidates and runners-up must always be disjoint sets. - assert!(!intersects(&Elections::members_ids(), &Elections::candidates())); - assert!(!intersects(&Elections::members_ids(), &Elections::runners_up_ids())); - assert!(!intersects(&Elections::candidates(), &Elections::runners_up_ids())); + assert!(!intersects(&members_ids(), &candidate_ids())); + assert!(!intersects(&members_ids(), &runners_up_ids())); + assert!(!intersects(&candidate_ids(), &runners_up_ids())); } fn pre_conditions() { System::set_block_number(1); ensure_members_sorted(); ensure_candidates_sorted(); + ensure_member_candidates_runners_up_disjoint(); } fn post_conditions() { @@ -1360,28 +1377,24 @@ mod tests { } 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 - } + Voting::::get(who).votes } #[test] fn params_should_work() { ExtBuilder::default().build_and_execute(|| { - assert_eq!(Elections::desired_members(), 2); - assert_eq!(Elections::term_duration(), 5); + assert_eq!(::DesiredMembers::get(), 2); + assert_eq!(::DesiredRunnersUp::get(), 0); + assert_eq!(::VotingBondBase::get(), 2); + assert_eq!(::VotingBondFactor::get(), 0); + assert_eq!(::CandidacyBond::get(), 3); + assert_eq!(::TermDuration::get(), 5); assert_eq!(Elections::election_rounds(), 0); assert!(Elections::members().is_empty()); assert!(Elections::runners_up().is_empty()); - assert!(Elections::candidates().is_empty()); + assert!(candidate_ids().is_empty()); assert_eq!(>::decode_len(), None); assert!(Elections::is_candidate(&1).is_err()); @@ -1394,16 +1407,38 @@ mod tests { fn genesis_members_should_work() { ExtBuilder::default().genesis_members(vec![(1, 10), (2, 20)]).build_and_execute(|| { System::set_block_number(1); - assert_eq!(Elections::members(), vec![(1, 10), (2, 20)]); + assert_eq!( + Elections::members(), + vec![ + SeatHolder { who: 1, stake: 10, deposit: 0 }, + SeatHolder { who: 2, stake: 20, deposit: 0 } + ] + ); - assert_eq!(Elections::voting(1), (10, vec![1])); - assert_eq!(Elections::voting(2), (20, vec![2])); + assert_eq!(Elections::voting(1), Voter { stake: 10u64, votes: vec![1], deposit: 0 }); + assert_eq!(Elections::voting(2), Voter { stake: 20u64, votes: vec![2], deposit: 0 }); // they will persist since they have self vote. System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); + + assert_eq!(members_ids(), vec![1, 2]); + }) + } + + #[test] + fn genesis_voters_can_remove_lock() { + ExtBuilder::default().genesis_members(vec![(1, 10), (2, 20)]).build_and_execute(|| { + System::set_block_number(1); + + assert_eq!(Elections::voting(1), Voter { stake: 10u64, votes: vec![1], deposit: 0 }); + assert_eq!(Elections::voting(2), Voter { stake: 20u64, votes: vec![2], deposit: 0 }); + + assert_ok!(Elections::remove_voter(Origin::signed(1))); + assert_ok!(Elections::remove_voter(Origin::signed(2))); - assert_eq!(Elections::members_ids(), vec![1, 2]); + assert_eq!(Elections::voting(1), Default::default()); + assert_eq!(Elections::voting(2), Default::default()); }) } @@ -1411,16 +1446,22 @@ mod tests { fn genesis_members_unsorted_should_work() { ExtBuilder::default().genesis_members(vec![(2, 20), (1, 10)]).build_and_execute(|| { System::set_block_number(1); - assert_eq!(Elections::members(), vec![(1, 10), (2, 20)]); + assert_eq!( + Elections::members(), + vec![ + SeatHolder { who: 1, stake: 10, deposit: 0 }, + SeatHolder { who: 2, stake: 20, deposit: 0 }, + ] + ); - assert_eq!(Elections::voting(1), (10, vec![1])); - assert_eq!(Elections::voting(2), (20, vec![2])); + assert_eq!(Elections::voting(1), Voter { stake: 10u64, votes: vec![1], deposit: 0 }); + assert_eq!(Elections::voting(2), Voter { stake: 20u64, votes: vec![2], deposit: 0 }); // they will persist since they have self vote. System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![1, 2]); + assert_eq!(members_ids(), vec![1, 2]); }) } @@ -1434,17 +1475,7 @@ mod tests { } #[test] - #[should_panic] - fn genesis_members_cannot_over_stake_1() { - // 10 cannot reserve 20 as voting bond and extra genesis will panic. - ExtBuilder::default() - .voter_bond(20) - .genesis_members(vec![(1, 10), (2, 20)]) - .build_and_execute(|| {}); - } - - #[test] - #[should_panic = "Duplicate member in elections phragmen genesis: 2"] + #[should_panic = "Duplicate member in elections-phragmen genesis: 2"] fn genesis_members_cannot_be_duplicate() { ExtBuilder::default() .desired_members(3) @@ -1467,27 +1498,27 @@ mod tests { .term_duration(0) .build_and_execute(|| { - assert_eq!(Elections::term_duration(), 0); - assert_eq!(Elections::desired_members(), 2); + assert_eq!(::TermDuration::get(), 0); + assert_eq!(::DesiredMembers::get(), 2); assert_eq!(Elections::election_rounds(), 0); - assert!(Elections::members_ids().is_empty()); + assert!(members_ids().is_empty()); assert!(Elections::runners_up().is_empty()); - assert!(Elections::candidates().is_empty()); + assert!(candidate_ids().is_empty()); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert!(Elections::members_ids().is_empty()); + assert!(members_ids().is_empty()); assert!(Elections::runners_up().is_empty()); - assert!(Elections::candidates().is_empty()); + assert!(candidate_ids().is_empty()); }); } #[test] fn simple_candidate_submission_should_work() { ExtBuilder::default().build_and_execute(|| { - assert_eq!(Elections::candidates(), Vec::::new()); + assert_eq!(candidate_ids(), Vec::::new()); assert!(Elections::is_candidate(&1).is_err()); assert!(Elections::is_candidate(&2).is_err()); @@ -1495,7 +1526,7 @@ mod tests { assert_ok!(submit_candidacy(Origin::signed(1))); assert_eq!(balances(&1), (7, 3)); - assert_eq!(Elections::candidates(), vec![1]); + assert_eq!(candidate_ids(), vec![1]); assert!(Elections::is_candidate(&1).is_ok()); assert!(Elections::is_candidate(&2).is_err()); @@ -1504,46 +1535,67 @@ mod tests { assert_ok!(submit_candidacy(Origin::signed(2))); assert_eq!(balances(&2), (17, 3)); - assert_eq!(Elections::candidates(), vec![1, 2]); + assert_eq!(candidate_ids(), vec![1, 2]); assert!(Elections::is_candidate(&1).is_ok()); assert!(Elections::is_candidate(&2).is_ok()); + + assert_eq!(candidate_deposit(&1), 3); + assert_eq!(candidate_deposit(&2), 3); + assert_eq!(candidate_deposit(&3), 0); }); } #[test] - fn simple_candidate_submission_with_no_votes_should_work() { + fn updating_candidacy_bond_works() { ExtBuilder::default().build_and_execute(|| { - assert_eq!(Elections::candidates(), Vec::::new()); - - assert_ok!(submit_candidacy(Origin::signed(1))); - assert_ok!(submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + assert_eq!(Elections::candidates(), vec![(5, 3)]); - assert!(Elections::is_candidate(&1).is_ok()); - assert!(Elections::is_candidate(&2).is_ok()); - assert_eq!(Elections::candidates(), vec![1, 2]); + // a runtime upgrade changes the bond. + CANDIDACY_BOND.with(|v| *v.borrow_mut() = 4); - assert!(Elections::members_ids().is_empty()); - assert!(Elections::runners_up().is_empty()); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_eq!(Elections::candidates(), vec![(4, 4), (5, 3)]); + // once elected, they each hold their candidacy bond, no more. System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert!(Elections::is_candidate(&1).is_err()); - assert!(Elections::is_candidate(&2).is_err()); - assert!(Elections::candidates().is_empty()); + assert_eq!( + Elections::members(), + vec![ + SeatHolder { who: 4, stake: 40, deposit: 4 }, + SeatHolder { who: 5, stake: 50, deposit: 3 }, + ] + ); + }) + } - assert!(Elections::members_ids().is_empty()); - assert!(Elections::runners_up().is_empty()); + #[test] + fn candidates_are_always_sorted() { + ExtBuilder::default().build_and_execute(|| { + assert_eq!(candidate_ids(), Vec::::new()); + + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_eq!(candidate_ids(), vec![3]); + assert_ok!(submit_candidacy(Origin::signed(1))); + assert_eq!(candidate_ids(), vec![1, 3]); + assert_ok!(submit_candidacy(Origin::signed(2))); + assert_eq!(candidate_ids(), vec![1, 2, 3]); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_eq!(candidate_ids(), vec![1, 2, 3, 4]); }); } #[test] fn dupe_candidate_submission_should_not_work() { ExtBuilder::default().build_and_execute(|| { - assert_eq!(Elections::candidates(), Vec::::new()); + assert_eq!(candidate_ids(), Vec::::new()); assert_ok!(submit_candidacy(Origin::signed(1))); - assert_eq!(Elections::candidates(), vec![1]); + assert_eq!(candidate_ids(), vec![1]); assert_noop!( submit_candidacy(Origin::signed(1)), Error::::DuplicatedCandidate, @@ -1559,11 +1611,11 @@ mod tests { assert_ok!(vote(Origin::signed(2), vec![5], 20)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![5]); + assert_eq!(members_ids(), vec![5]); assert!(Elections::runners_up().is_empty()); - assert!(Elections::candidates().is_empty()); + assert!(candidate_ids().is_empty()); assert_noop!( submit_candidacy(Origin::signed(5)), @@ -1583,14 +1635,14 @@ mod tests { assert_ok!(vote(Origin::signed(1), vec![3], 10)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4, 5]); - assert_eq!(Elections::runners_up_ids(), vec![3]); + assert_eq!(members_ids(), vec![4, 5]); + assert_eq!(runners_up_ids(), vec![3]); assert_noop!( submit_candidacy(Origin::signed(3)), - Error::::RunnerSubmit, + Error::::RunnerUpSubmit, ); }); } @@ -1598,7 +1650,7 @@ mod tests { #[test] fn poor_candidate_submission_should_not_work() { ExtBuilder::default().build_and_execute(|| { - assert_eq!(Elections::candidates(), Vec::::new()); + assert_eq!(candidate_ids(), Vec::::new()); assert_noop!( submit_candidacy(Origin::signed(7)), Error::::InsufficientCandidateFunds, @@ -1609,7 +1661,7 @@ mod tests { #[test] fn simple_voting_should_work() { ExtBuilder::default().build_and_execute(|| { - assert_eq!(Elections::candidates(), Vec::::new()); + assert_eq!(candidate_ids(), Vec::::new()); assert_eq!(balances(&2), (20, 0)); assert_ok!(submit_candidacy(Origin::signed(5))); @@ -1623,7 +1675,7 @@ mod tests { #[test] fn can_vote_with_custom_stake() { ExtBuilder::default().build_and_execute(|| { - assert_eq!(Elections::candidates(), Vec::::new()); + assert_eq!(candidate_ids(), Vec::::new()); assert_eq!(balances(&2), (20, 0)); assert_ok!(submit_candidacy(Origin::signed(5))); @@ -1655,6 +1707,74 @@ mod tests { }); } + #[test] + fn updated_voting_bond_works() { + ExtBuilder::default().build_and_execute(|| { + assert_ok!(submit_candidacy(Origin::signed(5))); + + assert_eq!(balances(&2), (20, 0)); + assert_ok!(vote(Origin::signed(2), vec![5], 5)); + assert_eq!(balances(&2), (18, 2)); + assert_eq!(voter_deposit(&2), 2); + + // a runtime upgrade lowers the voting bond to 1. This guy still un-reserves 2 when + // leaving. + VOTING_BOND_BASE.with(|v| *v.borrow_mut() = 1); + + // proof that bond changed. + assert_eq!(balances(&1), (10, 0)); + assert_ok!(vote(Origin::signed(1), vec![5], 5)); + assert_eq!(balances(&1), (9, 1)); + assert_eq!(voter_deposit(&1), 1); + + assert_ok!(Elections::remove_voter(Origin::signed(2))); + assert_eq!(balances(&2), (20, 0)); + }) + } + + #[test] + fn voting_reserves_bond_per_vote() { + ExtBuilder::default().voter_bond_factor(1).build_and_execute(|| { + assert_eq!(balances(&2), (20, 0)); + + assert_ok!(submit_candidacy(Origin::signed(5))); + assert_ok!(submit_candidacy(Origin::signed(4))); + + // initial vote. + assert_ok!(vote(Origin::signed(2), vec![5], 10)); + + // 2 + 1 + assert_eq!(balances(&2), (17, 3)); + assert_eq!(Elections::voting(&2).deposit, 3); + assert_eq!(has_lock(&2), 10); + assert_eq!(locked_stake_of(&2), 10); + + // can update; different stake; different lock and reserve. + assert_ok!(vote(Origin::signed(2), vec![5, 4], 15)); + // 2 + 2 + assert_eq!(balances(&2), (16, 4)); + assert_eq!(Elections::voting(&2).deposit, 4); + assert_eq!(has_lock(&2), 15); + assert_eq!(locked_stake_of(&2), 15); + + // stay at two votes with different stake. + assert_ok!(vote(Origin::signed(2), vec![5, 3], 18)); + // 2 + 2 + assert_eq!(balances(&2), (16, 4)); + assert_eq!(Elections::voting(&2).deposit, 4); + assert_eq!(has_lock(&2), 18); + assert_eq!(locked_stake_of(&2), 18); + + // back to 1 vote. + assert_ok!(vote(Origin::signed(2), vec![4], 12)); + // 2 + 1 + assert_eq!(balances(&2), (17, 3)); + assert_eq!(Elections::voting(&2).deposit, 3); + assert_eq!(has_lock(&2), 12); + assert_eq!(locked_stake_of(&2), 12); + }); + } + #[test] fn cannot_vote_for_no_candidate() { ExtBuilder::default().build_and_execute(|| { @@ -1674,10 +1794,10 @@ mod tests { assert_ok!(vote(Origin::signed(2), vec![4, 5], 20)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4, 5]); - assert!(Elections::candidates().is_empty()); + assert_eq!(members_ids(), vec![4, 5]); + assert!(candidate_ids().is_empty()); assert_ok!(vote(Origin::signed(3), vec![4, 5], 10)); }); @@ -1697,10 +1817,10 @@ mod tests { assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4, 5]); - assert!(Elections::candidates().is_empty()); + assert_eq!(members_ids(), vec![4, 5]); + assert!(candidate_ids().is_empty()); assert_ok!(vote(Origin::signed(3), vec![4, 5], 10)); assert_eq!(PRIME.with(|p| *p.borrow()), Some(4)); @@ -1708,28 +1828,73 @@ mod tests { } #[test] - fn prime_votes_for_exiting_members_are_removed() { + fn prime_votes_for_exiting_members_are_removed() { + ExtBuilder::default().build_and_execute(|| { + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + + 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), Renouncing::Candidate(3))); + + System::set_block_number(5); + Elections::on_initialize(System::block_number()); + + assert_eq!(members_ids(), vec![3, 5]); + assert!(candidate_ids().is_empty()); + + assert_eq!(PRIME.with(|p| *p.borrow()), Some(5)); + }); + } + + #[test] + fn prime_is_kept_if_other_members_leave() { + ExtBuilder::default().build_and_execute(|| { + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(5))); + + assert_ok!(vote(Origin::signed(4), vec![4], 40)); + assert_ok!(vote(Origin::signed(5), vec![5], 50)); + + System::set_block_number(5); + Elections::on_initialize(System::block_number()); + + assert_eq!(members_ids(), vec![4, 5]); + assert_eq!(PRIME.with(|p| *p.borrow()), Some(5)); + assert_ok!(Elections::renounce_candidacy( + Origin::signed(4), + Renouncing::Member + )); + + assert_eq!(members_ids(), vec![5]); + assert_eq!(PRIME.with(|p| *p.borrow()), Some(5)); + }) + } + + #[test] + fn prime_is_gone_if_renouncing() { ExtBuilder::default().build_and_execute(|| { - assert_ok!(submit_candidacy(Origin::signed(3))); assert_ok!(submit_candidacy(Origin::signed(4))); assert_ok!(submit_candidacy(Origin::signed(5))); - 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), Renouncing::Candidate(3))); - System::set_block_number(5); - Elections::end_block(System::block_number()); - - assert_eq!(Elections::members_ids(), vec![3, 5]); - assert!(Elections::candidates().is_empty()); + Elections::on_initialize(System::block_number()); + assert_eq!(members_ids(), vec![4, 5]); assert_eq!(PRIME.with(|p| *p.borrow()), Some(5)); - }); + assert_ok!(Elections::renounce_candidacy(Origin::signed(5), Renouncing::Member)); + + assert_eq!(members_ids(), vec![4]); + assert_eq!(PRIME.with(|p| *p.borrow()), None); + }) } #[test] @@ -1755,7 +1920,7 @@ mod tests { assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); // now we have 2 members, 1 runner-up, and 1 new candidate assert_ok!(submit_candidacy(Origin::signed(2))); @@ -1853,172 +2018,9 @@ mod tests { assert_ok!(Elections::remove_voter(Origin::signed(4))); System::set_block_number(5); - Elections::end_block(System::block_number()); - - assert_eq!(Elections::members_ids(), vec![3, 5]); - }); - } - - #[test] - fn reporter_must_be_voter() { - ExtBuilder::default().build_and_execute(|| { - assert_noop!( - 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!(submit_candidacy(Origin::signed(4))); - assert_ok!(submit_candidacy(Origin::signed(5))); - assert_ok!(submit_candidacy(Origin::signed(6))); - - 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!(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![6]); - assert!(Elections::candidates().is_empty()); - - // all of them have a member or runner-up that they voted for. - 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(&votes_of(&3)), true); - - 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(&votes_of(&1)), false); - - }); - } - - #[test] - fn report_voter_should_work_and_earn_reward() { - ExtBuilder::default().build_and_execute(|| { - assert_ok!(submit_candidacy(Origin::signed(5))); - assert_ok!(submit_candidacy(Origin::signed(4))); - - 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!(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!(Elections::candidates().is_empty()); - - assert_eq!(balances(&3), (28, 2)); - assert_eq!(balances(&5), (45, 5)); - - 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)) - })); - - assert_eq!(balances(&3), (28, 0)); - assert_eq!(balances(&5), (47, 5)); - }); - } - - #[test] - fn report_voter_should_slash_when_bad_report() { - ExtBuilder::default().build_and_execute(|| { - assert_ok!(submit_candidacy(Origin::signed(5))); - assert_ok!(submit_candidacy(Origin::signed(4))); - - assert_ok!(vote(Origin::signed(5), vec![5], 50)); - assert_ok!(vote(Origin::signed(4), vec![4], 40)); - - System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4, 5]); - assert!(Elections::candidates().is_empty()); - - assert_eq!(balances(&4), (35, 5)); - assert_eq!(balances(&5), (45, 5)); - - 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)) - })); - - assert_eq!(balances(&4), (35, 5)); - assert_eq!(balances(&5), (45, 3)); + assert_eq!(members_ids(), vec![3, 5]); }); } @@ -2039,18 +2041,19 @@ mod tests { assert_eq!(votes_of(&3), vec![3]); assert_eq!(votes_of(&4), vec![4]); - assert_eq!(Elections::candidates(), vec![3, 4, 5]); + assert_eq!(candidate_ids(), vec![3, 4, 5]); assert_eq!(>::decode_len().unwrap(), 3); assert_eq!(Elections::election_rounds(), 0); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members(), vec![(3, 30), (5, 20)]); + assert_eq!(members_and_stake(), vec![(3, 30), (5, 20)]); assert!(Elections::runners_up().is_empty()); + assert_eq_uvec!(all_voters(), vec![2, 3, 4]); - assert!(Elections::candidates().is_empty()); + assert!(candidate_ids().is_empty()); assert_eq!(>::decode_len(), None); assert_eq!(Elections::election_rounds(), 1); @@ -2062,7 +2065,7 @@ mod tests { ExtBuilder::default().build_and_execute(|| { // no candidates, no nothing. System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); assert_eq!( System::events().iter().last().unwrap().event, @@ -2081,21 +2084,21 @@ mod tests { assert_ok!(vote(Origin::signed(4), vec![4], 40)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); assert_eq!( System::events().iter().last().unwrap().event, Event::elections_phragmen(RawEvent::NewTerm(vec![(4, 40), (5, 50)])), ); - assert_eq!(Elections::members(), vec![(4, 40), (5, 50)]); - assert_eq!(Elections::runners_up(), vec![]); + assert_eq!(members_and_stake(), vec![(4, 40), (5, 50)]); + assert_eq!(runners_up_and_stake(), vec![]); assert_ok!(Elections::remove_voter(Origin::signed(5))); assert_ok!(Elections::remove_voter(Origin::signed(4))); System::set_block_number(10); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); assert_eq!( System::events().iter().last().unwrap().event, @@ -2118,19 +2121,19 @@ mod tests { assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members(), vec![(5, 50)]); + assert_eq!(members_and_stake(), vec![(5, 50)]); assert_eq!(Elections::election_rounds(), 1); // but now it has a valid target. assert_ok!(submit_candidacy(Origin::signed(4))); System::set_block_number(10); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); // candidate 4 is affected by an old vote. - assert_eq!(Elections::members(), vec![(4, 30), (5, 50)]); + assert_eq!(members_and_stake(), vec![(4, 30), (5, 50)]); assert_eq!(Elections::election_rounds(), 2); assert_eq_uvec!(all_voters(), vec![3, 5]); }); @@ -2150,10 +2153,10 @@ mod tests { assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); assert_eq!(Elections::election_rounds(), 1); - assert_eq!(Elections::members_ids(), vec![4, 5]); + assert_eq!(members_ids(), vec![4, 5]); }); } @@ -2164,11 +2167,11 @@ mod tests { assert_ok!(submit_candidacy(Origin::signed(4))); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert!(Elections::candidates().is_empty()); + assert!(candidate_ids().is_empty()); assert_eq!(Elections::election_rounds(), 1); - assert!(Elections::members_ids().is_empty()); + assert!(members_ids().is_empty()); assert_eq!( System::events().iter().last().unwrap().event, @@ -2191,11 +2194,11 @@ mod tests { assert_ok!(vote(Origin::signed(5), vec![4], 50)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); // sorted based on account id. - assert_eq!(Elections::members_ids(), vec![4, 5]); + assert_eq!(members_ids(), vec![4, 5]); // sorted based on merit (least -> most) - assert_eq!(Elections::runners_up_ids(), vec![3, 2]); + assert_eq!(runners_up_ids(), vec![3, 2]); // runner ups are still locked. assert_eq!(balances(&4), (35, 5)); @@ -2218,16 +2221,17 @@ mod tests { assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - 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)]); + Elections::on_initialize(System::block_number()); + assert_eq!(members_and_stake(), vec![(4, 40), (5, 50)]); + assert_eq!(runners_up_and_stake(), vec![(2, 20), (3, 30)]); assert_ok!(vote(Origin::signed(5), vec![5], 15)); System::set_block_number(10); - 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)]); + Elections::on_initialize(System::block_number()); + + assert_eq!(members_and_stake(), vec![(3, 30), (4, 40)]); + assert_eq!(runners_up_and_stake(), vec![(5, 15), (2, 20)]); }); } @@ -2243,18 +2247,18 @@ mod tests { 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![2]); + Elections::on_initialize(System::block_number()); + assert_eq!(members_ids(), vec![4, 5]); + assert_eq!(runners_up_ids(), vec![2]); assert_eq!(balances(&2), (15, 5)); assert_ok!(submit_candidacy(Origin::signed(3))); assert_ok!(vote(Origin::signed(3), vec![3], 30)); System::set_block_number(10); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::runners_up_ids(), vec![3]); + assert_eq!(runners_up_ids(), vec![3]); assert_eq!(balances(&2), (15, 2)); }); } @@ -2271,22 +2275,22 @@ mod tests { assert_eq!(balances(&5), (45, 5)); System::set_block_number(5); - Elections::end_block(System::block_number()); - assert_eq!(Elections::members_ids(), vec![5]); + Elections::on_initialize(System::block_number()); + assert_eq!(members_ids(), vec![5]); assert_ok!(Elections::remove_voter(Origin::signed(5))); assert_eq!(balances(&5), (47, 3)); System::set_block_number(10); - Elections::end_block(System::block_number()); - assert!(Elections::members_ids().is_empty()); + Elections::on_initialize(System::block_number()); + assert!(members_ids().is_empty()); assert_eq!(balances(&5), (47, 0)); }); } #[test] - fn losers_will_lose_the_bond() { + fn candidates_lose_the_bond_when_outgoing() { ExtBuilder::default().build_and_execute(|| { assert_ok!(submit_candidacy(Origin::signed(5))); assert_ok!(submit_candidacy(Origin::signed(3))); @@ -2297,9 +2301,9 @@ mod tests { assert_eq!(balances(&3), (27, 3)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![5]); + assert_eq!(members_ids(), vec![5]); // winner assert_eq!(balances(&5), (47, 3)); @@ -2318,9 +2322,9 @@ mod tests { assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4, 5]); + assert_eq!(members_ids(), vec![4, 5]); assert_eq!(Elections::election_rounds(), 1); assert_ok!(submit_candidacy(Origin::signed(2))); @@ -2332,13 +2336,13 @@ mod tests { assert_ok!(Elections::remove_voter(Origin::signed(4))); // 5 will persist as candidates despite not being in the list. - assert_eq!(Elections::candidates(), vec![2, 3]); + assert_eq!(candidate_ids(), vec![2, 3]); System::set_block_number(10); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); // 4 removed; 5 and 3 are the new best. - assert_eq!(Elections::members_ids(), vec![3, 5]); + assert_eq!(members_ids(), vec![3, 5]); }); } @@ -2359,12 +2363,12 @@ mod tests { let check_at_block = |b: u32| { System::set_block_number(b.into()); - Elections::end_block(System::block_number()); + Elections::on_initialize(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)]); + assert_eq!(members_and_stake(), vec![(4, 40), (5, 50)]); + assert_eq!(runners_up_and_stake(), vec![(2, 20), (3, 30)]); // no new candidates but old members and runners-up are always added. - assert!(Elections::candidates().is_empty()); + assert!(candidate_ids().is_empty()); assert_eq!(Elections::election_rounds(), b / 5); assert_eq_uvec!(all_voters(), vec![2, 3, 4, 5]); }; @@ -2387,8 +2391,8 @@ mod tests { 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]); + Elections::on_initialize(System::block_number()); + assert_eq!(members_ids(), vec![4, 5]); assert_eq!(Elections::election_rounds(), 1); // a new candidate @@ -2399,7 +2403,7 @@ mod tests { assert_eq!(balances(&4), (35, 2)); // slashed assert_eq!(Elections::election_rounds(), 2); // new election round - assert_eq!(Elections::members_ids(), vec![3, 5]); // new members + assert_eq!(members_ids(), vec![3, 5]); // new members }); } @@ -2413,14 +2417,21 @@ mod tests { 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]); + Elections::on_initialize(System::block_number()); + assert_eq!(members_ids(), vec![4, 5]); // no replacement yet. - assert_err_with_weight!( - Elections::remove_member(Origin::root(), 4, true), - Error::::InvalidReplacement, - Some(33489000), // only thing that matters for now is that it is NOT the full block. + let unwrapped_error = Elections::remove_member(Origin::root(), 4, true).unwrap_err(); + matches!( + unwrapped_error.error, + DispatchError::Module { + message: Some("InvalidReplacement"), + .. + } + ); + matches!( + unwrapped_error.post_info.actual_weight, + Some(x) if x < ::BlockWeights::get().max_block ); }); @@ -2434,15 +2445,22 @@ mod tests { 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]); + Elections::on_initialize(System::block_number()); + assert_eq!(members_ids(), vec![4, 5]); + assert_eq!(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(33489000) // only thing that matters for now is that it is NOT the full block. + let unwrapped_error = Elections::remove_member(Origin::root(), 4, false).unwrap_err(); + matches!( + unwrapped_error.error, + DispatchError::Module { + message: Some("InvalidReplacement"), + .. + } + ); + matches!( + unwrapped_error.post_info.actual_weight, + Some(x) if x < ::BlockWeights::get().max_block ); }); } @@ -2464,8 +2482,8 @@ mod tests { assert_eq!(Elections::election_rounds(), 0); System::set_block_number(5); - Elections::end_block(System::block_number()); - assert_eq!(Elections::members_ids(), vec![3, 5]); + Elections::on_initialize(System::block_number()); + assert_eq!(members_ids(), vec![3, 5]); assert_eq!(Elections::election_rounds(), 1); assert_ok!(Elections::remove_voter(Origin::signed(2))); @@ -2475,8 +2493,8 @@ mod tests { // meanwhile, no one cares to become a candidate again. System::set_block_number(10); - Elections::end_block(System::block_number()); - assert!(Elections::members_ids().is_empty()); + Elections::on_initialize(System::block_number()); + assert!(members_ids().is_empty()); assert_eq!(Elections::election_rounds(), 2); }); } @@ -2491,8 +2509,8 @@ mod tests { 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]); + Elections::on_initialize(System::block_number()); + assert_eq!(members_ids(), vec![4, 5]); assert_ok!(submit_candidacy(Origin::signed(1))); assert_ok!(submit_candidacy(Origin::signed(2))); @@ -2509,10 +2527,10 @@ mod tests { assert_ok!(vote(Origin::signed(1), vec![1], 10)); System::set_block_number(10); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); // 3, 4 are new members, must still be bonded, nothing slashed. - assert_eq!(Elections::members(), vec![(3, 30), (4, 48)]); + assert_eq!(members_and_stake(), vec![(3, 30), (4, 48)]); assert_eq!(balances(&3), (25, 5)); assert_eq!(balances(&4), (35, 5)); @@ -2539,9 +2557,9 @@ mod tests { assert_ok!(vote(Origin::signed(5), vec![10], 50)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq_uvec!(Elections::members_ids(), vec![3, 4]); + assert_eq_uvec!(members_ids(), vec![3, 4]); assert_eq!(Elections::election_rounds(), 1); }); } @@ -2560,48 +2578,34 @@ mod tests { assert_ok!(vote(Origin::signed(5), vec![4], 50)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); // id: low -> high. - assert_eq!(Elections::members(), vec![(4, 50), (5, 40)]); + assert_eq!(members_and_stake(), vec![(4, 50), (5, 40)]); // merit: low -> high. - assert_eq!(Elections::runners_up(), vec![(3, 20), (2, 30)]); + assert_eq!(runners_up_and_stake(), vec![(3, 20), (2, 30)]); }); } - #[test] - fn candidates_are_sorted() { - ExtBuilder::default().build_and_execute(|| { - assert_ok!(submit_candidacy(Origin::signed(5))); - assert_ok!(submit_candidacy(Origin::signed(3))); - - assert_eq!(Elections::candidates(), vec![3, 5]); - - 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]); - }) - } - #[test] fn runner_up_replacement_maintains_members_order() { - ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - assert_ok!(submit_candidacy(Origin::signed(5))); - assert_ok!(submit_candidacy(Origin::signed(4))); + 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(2))); 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); - Elections::end_block(System::block_number()); + System::set_block_number(5); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![2, 4]); - assert_ok!(Elections::remove_member(Origin::root(), 2, true)); - assert_eq!(Elections::members_ids(), vec![4, 5]); - }); + assert_eq!(members_ids(), vec![2, 4]); + assert_ok!(Elections::remove_member(Origin::root(), 2, true)); + assert_eq!(members_ids(), vec![4, 5]); + }); } #[test] @@ -2618,16 +2622,16 @@ mod tests { assert_ok!(vote(Origin::signed(2), vec![2], 20)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4, 5]); - assert_eq!(Elections::runners_up_ids(), vec![2, 3]); + assert_eq!(members_ids(), vec![4, 5]); + assert_eq!(runners_up_ids(), vec![2, 3]); 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]); - assert_eq!(Elections::runners_up_ids(), vec![2]); + assert_eq!(members_ids(), vec![3, 5]); + assert_eq!(runners_up_ids(), vec![2]); }) } @@ -2641,26 +2645,28 @@ mod tests { assert_ok!(vote(Origin::signed(4), vec![4], 40)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4, 5]); - assert!(Elections::runners_up_ids().is_empty()); + assert_eq!(members_ids(), vec![4, 5]); + assert!(runners_up_ids().is_empty()); 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!(Elections::runners_up_ids().is_empty()); + assert_eq!(members_ids(), vec![5]); + assert!(runners_up_ids().is_empty()); }) } #[test] - fn can_renounce_candidacy_runner() { - 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))); + fn can_renounce_candidacy_runner_up() { + 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(5), vec![4], 50)); @@ -2668,18 +2674,21 @@ mod tests { assert_ok!(vote(Origin::signed(3), vec![3], 30)); assert_ok!(vote(Origin::signed(2), vec![2], 20)); - System::set_block_number(5); - Elections::end_block(System::block_number()); + System::set_block_number(5); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4, 5]); - assert_eq!(Elections::runners_up_ids(), vec![2, 3]); + assert_eq!(members_ids(), vec![4, 5]); + assert_eq!(runners_up_ids(), vec![2, 3]); - assert_ok!(Elections::renounce_candidacy(Origin::signed(3), Renouncing::RunnerUp)); - assert_eq!(balances(&3), (28, 2)); // 2 is voting bond. + 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]); - assert_eq!(Elections::runners_up_ids(), vec![2]); - }) + assert_eq!(members_ids(), vec![4, 5]); + assert_eq!(runners_up_ids(), vec![2]); + }) } #[test] @@ -2696,13 +2705,13 @@ mod tests { assert_ok!(vote(Origin::signed(5), vec![2], 50)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![2, 4]); - assert_eq!(Elections::runners_up_ids(), vec![5, 3]); + assert_eq!(members_ids(), vec![2, 4]); + assert_eq!(runners_up_ids(), vec![5, 3]); 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]); + assert_eq!(members_ids(), vec![2, 4]); + assert_eq!(runners_up_ids(), vec![5]); }); } @@ -2711,11 +2720,11 @@ mod tests { ExtBuilder::default().build_and_execute(|| { assert_ok!(submit_candidacy(Origin::signed(5))); assert_eq!(balances(&5), (47, 3)); - assert_eq!(Elections::candidates(), vec![5]); + assert_eq!(candidate_ids(), vec![5]); assert_ok!(Elections::renounce_candidacy(Origin::signed(5), Renouncing::Candidate(1))); assert_eq!(balances(&5), (50, 0)); - assert!(Elections::candidates().is_empty()); + assert!(candidate_ids().is_empty()); }) } @@ -2728,7 +2737,7 @@ mod tests { ); assert_noop!( Elections::renounce_candidacy(Origin::signed(5), Renouncing::Member), - Error::::NotMember, + Error::::InvalidRenouncing, ); assert_noop!( Elections::renounce_candidacy(Origin::signed(5), Renouncing::RunnerUp), @@ -2749,14 +2758,14 @@ mod tests { assert_ok!(vote(Origin::signed(3), vec![3], 30)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4, 5]); - assert_eq!(Elections::runners_up_ids(), vec![3]); + assert_eq!(members_ids(), vec![4, 5]); + assert_eq!(runners_up_ids(), vec![3]); assert_noop!( Elections::renounce_candidacy(Origin::signed(3), Renouncing::Member), - Error::::NotMember, + Error::::InvalidRenouncing, ); }) } @@ -2773,10 +2782,10 @@ mod tests { assert_ok!(vote(Origin::signed(3), vec![3], 30)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4, 5]); - assert_eq!(Elections::runners_up_ids(), vec![3]); + assert_eq!(members_ids(), vec![4, 5]); + assert_eq!(runners_up_ids(), vec![3]); assert_noop!( Elections::renounce_candidacy(Origin::signed(4), Renouncing::RunnerUp), @@ -2794,7 +2803,7 @@ mod tests { assert_noop!( Elections::renounce_candidacy(Origin::signed(4), Renouncing::Candidate(2)), - Error::::InvalidRenouncing, + Error::::InvalidWitnessData, ); assert_ok!(Elections::renounce_candidacy(Origin::signed(4), Renouncing::Candidate(3))); @@ -2812,25 +2821,6 @@ mod tests { }) } - #[test] - fn behavior_with_dupe_candidate() { - ExtBuilder::default().desired_runners_up(2).build_and_execute(|| { - >::put(vec![1, 1, 2, 3, 4]); - - 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); - Elections::end_block(System::block_number()); - - assert_eq!(Elections::members_ids(), vec![1, 4]); - assert_eq!(Elections::runners_up_ids(), vec![2, 3]); - assert!(Elections::candidates().is_empty()); - }) - } - #[test] fn unsorted_runners_up_are_detected() { ExtBuilder::default().desired_runners_up(2).desired_members(1).build_and_execute(|| { @@ -2838,25 +2828,24 @@ mod tests { 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], 5)); assert_ok!(vote(Origin::signed(3), vec![3], 15)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![5]); - assert_eq!(Elections::runners_up_ids(), vec![4, 3]); + assert_eq!(members_ids(), vec![5]); + assert_eq!(runners_up_ids(), vec![4, 3]); assert_ok!(submit_candidacy(Origin::signed(2))); assert_ok!(vote(Origin::signed(2), vec![2], 10)); System::set_block_number(10); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![5]); - assert_eq!(Elections::runners_up_ids(), vec![2, 3]); + assert_eq!(members_ids(), vec![5]); + assert_eq!(runners_up_ids(), vec![2, 3]); // 4 is outgoing runner-up. Slash candidacy bond. assert_eq!(balances(&4), (35, 2)); @@ -2878,10 +2867,10 @@ mod tests { assert_ok!(vote(Origin::signed(2), vec![2], 20)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4]); - assert_eq!(Elections::runners_up_ids(), vec![2, 3]); + assert_eq!(members_ids(), vec![4]); + assert_eq!(runners_up_ids(), vec![2, 3]); assert_eq!(balances(&4), (35, 5)); assert_eq!(balances(&3), (25, 5)); @@ -2892,10 +2881,10 @@ mod tests { assert_ok!(vote(Origin::signed(5), vec![5], 50)); System::set_block_number(10); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![5]); - assert_eq!(Elections::runners_up_ids(), vec![3, 4]); + assert_eq!(members_ids(), vec![5]); + assert_eq!(runners_up_ids(), vec![3, 4]); // 4 went from member to runner-up -- don't slash. assert_eq!(balances(&4), (35, 5)); @@ -2919,10 +2908,10 @@ mod tests { assert_ok!(vote(Origin::signed(2), vec![2], 20)); System::set_block_number(5); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![4]); - assert_eq!(Elections::runners_up_ids(), vec![2, 3]); + assert_eq!(members_ids(), vec![4]); + assert_eq!(runners_up_ids(), vec![2, 3]); assert_eq!(balances(&4), (35, 5)); assert_eq!(balances(&3), (25, 5)); @@ -2933,10 +2922,10 @@ mod tests { assert_ok!(vote(Origin::signed(2), vec![4], 20)); System::set_block_number(10); - Elections::end_block(System::block_number()); + Elections::on_initialize(System::block_number()); - assert_eq!(Elections::members_ids(), vec![2]); - assert_eq!(Elections::runners_up_ids(), vec![4, 3]); + assert_eq!(members_ids(), vec![2]); + assert_eq!(runners_up_ids(), vec![4, 3]); // 2 went from runner to member, don't slash assert_eq!(balances(&2), (15, 5)); @@ -2946,4 +2935,166 @@ mod tests { assert_eq!(balances(&3), (25, 5)); }); } + + #[test] + fn remove_and_replace_member_works() { + let setup = || { + 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::on_initialize(System::block_number()); + + assert_eq!(members_ids(), vec![4, 5]); + assert_eq!(runners_up_ids(), vec![3]); + }; + + // member removed, replacement found. + ExtBuilder::default().desired_runners_up(1).build_and_execute(|| { + setup(); + assert_eq!(Elections::remove_and_replace_member(&4, false), Ok(true)); + + assert_eq!(members_ids(), vec![3, 5]); + assert_eq!(runners_up_ids().len(), 0); + }); + + // member removed, no replacement found. + ExtBuilder::default().desired_runners_up(1).build_and_execute(|| { + setup(); + assert_ok!(Elections::renounce_candidacy(Origin::signed(3), Renouncing::RunnerUp)); + assert_eq!(Elections::remove_and_replace_member(&4, false), Ok(false)); + + assert_eq!(members_ids(), vec![5]); + assert_eq!(runners_up_ids().len(), 0); + }); + + // wrong member to remove. + ExtBuilder::default().desired_runners_up(1).build_and_execute(|| { + setup(); + assert!(matches!(Elections::remove_and_replace_member(&2, false), Err(_))); + }); + } + + #[test] + fn no_desired_members() { + // not interested in anything + ExtBuilder::default().desired_members(0).desired_runners_up(0).build_and_execute(|| { + assert_eq!(Elections::candidates().len(), 0); + + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(2))); + + assert_eq!(Elections::candidates().len(), 3); + + 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); + Elections::on_initialize(System::block_number()); + + assert_eq!(members_ids().len(), 0); + assert_eq!(runners_up_ids().len(), 0); + assert_eq!(all_voters().len(), 3); + assert_eq!(Elections::candidates().len(), 0); + }); + + // not interested in members + ExtBuilder::default().desired_members(0).desired_runners_up(2).build_and_execute(|| { + assert_eq!(Elections::candidates().len(), 0); + + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(2))); + + assert_eq!(Elections::candidates().len(), 3); + + 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); + Elections::on_initialize(System::block_number()); + + assert_eq!(members_ids().len(), 0); + assert_eq!(runners_up_ids(), vec![3, 4]); + assert_eq!(all_voters().len(), 3); + assert_eq!(Elections::candidates().len(), 0); + }); + + // not interested in runners-up + ExtBuilder::default().desired_members(2).desired_runners_up(0).build_and_execute(|| { + assert_eq!(Elections::candidates().len(), 0); + + assert_ok!(submit_candidacy(Origin::signed(4))); + assert_ok!(submit_candidacy(Origin::signed(3))); + assert_ok!(submit_candidacy(Origin::signed(2))); + + assert_eq!(Elections::candidates().len(), 3); + + 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); + Elections::on_initialize(System::block_number()); + + assert_eq!(members_ids(), vec![3, 4]); + assert_eq!(runners_up_ids().len(), 0); + assert_eq!(all_voters().len(), 3); + assert_eq!(Elections::candidates().len(), 0); + }); + } + + #[test] + fn dupe_vote_is_moot() { + ExtBuilder::default().desired_members(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!(submit_candidacy(Origin::signed(2))); + assert_ok!(submit_candidacy(Origin::signed(1))); + + // all these duplicate votes will not cause 2 to win. + assert_ok!(vote(Origin::signed(1), vec![2, 2, 2, 2], 5)); + assert_ok!(vote(Origin::signed(2), vec![2, 2, 2, 2], 20)); + + assert_ok!(vote(Origin::signed(3), vec![3], 30)); + + System::set_block_number(5); + Elections::on_initialize(System::block_number()); + + assert_eq!(members_ids(), vec![3]); + }) + } + + #[test] + fn remove_defunct_voter_works() { + 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))); + + // defunct + assert_ok!(vote(Origin::signed(5), vec![5, 4], 5)); + // defunct + assert_ok!(vote(Origin::signed(4), vec![4], 5)); + // ok + assert_ok!(vote(Origin::signed(3), vec![3], 5)); + // ok + assert_ok!(vote(Origin::signed(2), vec![3, 4], 5)); + + assert_ok!(Elections::renounce_candidacy(Origin::signed(5), Renouncing::Candidate(3))); + assert_ok!(Elections::renounce_candidacy(Origin::signed(4), Renouncing::Candidate(2))); + assert_ok!(Elections::renounce_candidacy(Origin::signed(3), Renouncing::Candidate(1))); + + assert_ok!(Elections::clean_defunct_voters(Origin::root(), 4, 2)); + }) + } } diff --git a/frame/elections-phragmen/src/migrations_3_0_0.rs b/frame/elections-phragmen/src/migrations_3_0_0.rs new file mode 100644 index 0000000000000..0737a12207c11 --- /dev/null +++ b/frame/elections-phragmen/src/migrations_3_0_0.rs @@ -0,0 +1,195 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Migrations to version [`3.0.0`], as denoted by the changelog. + +use codec::{Encode, Decode, FullCodec}; +use sp_std::prelude::*; +use frame_support::{ + RuntimeDebug, weights::Weight, Twox64Concat, + storage::types::{StorageMap, StorageValue}, + traits::{GetPalletVersion, PalletVersion}, +}; + +#[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)] +struct SeatHolder { + who: AccountId, + stake: Balance, + deposit: Balance, +} + +#[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)] +struct Voter { + votes: Vec, + stake: Balance, + deposit: Balance, +} + +/// Trait to implement to give information about types used for migration +pub trait V2ToV3 { + /// elections-phragmen module, used to check storage version. + type Module: GetPalletVersion; + + /// System config account id + type AccountId: 'static + FullCodec; + + /// Elections-phragmen currency balance. + type Balance: 'static + FullCodec + Copy; +} + +struct __Candidates; +impl frame_support::traits::StorageInstance for __Candidates { + fn pallet_prefix() -> &'static str { "PhragmenElection" } + const STORAGE_PREFIX: &'static str = "Candidates"; +} + +#[allow(type_alias_bounds)] +type Candidates = StorageValue<__Candidates, Vec<(T::AccountId, T::Balance)>>; + +struct __Members; +impl frame_support::traits::StorageInstance for __Members { + fn pallet_prefix() -> &'static str { "PhragmenElection" } + const STORAGE_PREFIX: &'static str = "Members"; +} +#[allow(type_alias_bounds)] +type Members = StorageValue<__Members, Vec>>; + +struct __RunnersUp; +impl frame_support::traits::StorageInstance for __RunnersUp { + fn pallet_prefix() -> &'static str { "PhragmenElection" } + const STORAGE_PREFIX: &'static str = "RunnersUp"; +} +#[allow(type_alias_bounds)] +type RunnersUp = StorageValue<__RunnersUp, Vec>>; + +struct __Voting; +impl frame_support::traits::StorageInstance for __Voting { + fn pallet_prefix() -> &'static str { "PhragmenElection" } + const STORAGE_PREFIX: &'static str = "Voting"; +} +#[allow(type_alias_bounds)] +type Voting = StorageMap<__Voting, Twox64Concat, T::AccountId, Voter>; + +/// Apply all of the migrations from 2_0_0 to 3_0_0. +/// +/// ### Warning +/// +/// This code will **ONLY** check that the storage version is less than or equal to 2_0_0. +/// Further check might be needed at the user runtime. +/// +/// Be aware that this migration is intended to be used only for the mentioned versions. Use +/// with care and run at your own risk. +pub fn apply(old_voter_bond: T::Balance, old_candidacy_bond: T::Balance) -> Weight { + let maybe_storage_version = ::storage_version(); + frame_support::debug::info!( + "Running migration for elections-phragmen with storage version {:?}", + maybe_storage_version + ); + match maybe_storage_version { + Some(storage_version) if storage_version <= PalletVersion::new(2, 0, 0) => { + migrate_voters_to_recorded_deposit::(old_voter_bond); + migrate_candidates_to_recorded_deposit::(old_candidacy_bond); + migrate_runners_up_to_recorded_deposit::(old_candidacy_bond); + migrate_members_to_recorded_deposit::(old_candidacy_bond); + Weight::max_value() + } + _ => { + frame_support::debug::warn!( + "Attempted to apply migration to V3 but failed because storage version is {:?}", + maybe_storage_version + ); + 0 + }, + } +} + +/// Migrate from the old legacy voting bond (fixed) to the new one (per-vote dynamic). +pub fn migrate_voters_to_recorded_deposit(old_deposit: T::Balance) { + >::translate::<(T::Balance, Vec), _>( + |_who, (stake, votes)| { + Some(Voter { + votes, + stake, + deposit: old_deposit, + }) + }, + ); + + frame_support::debug::info!( + "migrated {} voter accounts.", + >::iter().count(), + ); +} + +/// Migrate all candidates to recorded deposit. +pub fn migrate_candidates_to_recorded_deposit(old_deposit: T::Balance) { + let _ = >::translate::, _>( + |maybe_old_candidates| { + maybe_old_candidates.map(|old_candidates| { + frame_support::debug::info!( + "migrated {} candidate accounts.", + old_candidates.len() + ); + old_candidates + .into_iter() + .map(|c| (c, old_deposit)) + .collect::>() + }) + }, + ); +} + +/// Migrate all members to recorded deposit. +pub fn migrate_members_to_recorded_deposit(old_deposit: T::Balance) { + let _ = >::translate::, _>( + |maybe_old_members| { + maybe_old_members.map(|old_members| { + frame_support::debug::info!("migrated {} member accounts.", old_members.len()); + old_members + .into_iter() + .map(|(who, stake)| SeatHolder { + who, + stake, + deposit: old_deposit, + }) + .collect::>() + }) + }, + ); +} + +/// Migrate all runners-up to recorded deposit. +pub fn migrate_runners_up_to_recorded_deposit(old_deposit: T::Balance) { + let _ = >::translate::, _>( + |maybe_old_runners_up| { + maybe_old_runners_up.map(|old_runners_up| { + frame_support::debug::info!( + "migrated {} runner-up accounts.", + old_runners_up.len() + ); + old_runners_up + .into_iter() + .map(|(who, stake)| SeatHolder { + who, + stake, + deposit: old_deposit, + }) + .collect::>() + }) + }, + ); +} diff --git a/frame/elections-phragmen/src/weights.rs b/frame/elections-phragmen/src/weights.rs index baecda6180062..25c2091408361 100644 --- a/frame/elections-phragmen/src/weights.rs +++ b/frame/elections-phragmen/src/weights.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2020-2021 Parity Technologies (UK) Ltd. +// Copyright (C) 2021 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -15,9 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Weights for pallet_elections_phragmen -//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0 -//! DATE: 2020-10-27, STEPS: [50, ], REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] +//! Autogenerated weights for pallet_elections_phragmen +//! +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.1 +//! DATE: 2021-01-20, STEPS: [50, ], REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128 // Executed Command: @@ -43,173 +44,187 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_elections_phragmen. pub trait WeightInfo { - fn vote(_v: u32, ) -> Weight; - fn vote_update(_v: u32, ) -> Weight; + fn vote_equal(v: u32, ) -> Weight; + fn vote_more(v: u32, ) -> Weight; + fn vote_less(v: u32, ) -> Weight; fn remove_voter() -> Weight; - fn report_defunct_voter_correct(_c: u32, _v: u32, ) -> Weight; - fn report_defunct_voter_incorrect(_c: u32, _v: u32, ) -> Weight; - fn submit_candidacy(_c: u32, ) -> Weight; - fn renounce_candidacy_candidate(_c: u32, ) -> Weight; + fn submit_candidacy(c: u32, ) -> Weight; + fn renounce_candidacy_candidate(c: u32, ) -> Weight; fn renounce_candidacy_members() -> Weight; fn renounce_candidacy_runners_up() -> Weight; fn remove_member_with_replacement() -> Weight; fn remove_member_wrong_refund() -> Weight; - + fn clean_defunct_voters(v: u32, d: u32, ) -> Weight; + fn election_phragmen(c: u32, v: u32, e: u32, ) -> Weight; } /// Weights for pallet_elections_phragmen using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - fn vote(v: u32, ) -> Weight { - (89_627_000 as Weight) - .saturating_add((197_000 as Weight).saturating_mul(v as Weight)) + fn vote_equal(v: u32, ) -> Weight { + (45_157_000 as Weight) + // Standard Error: 6_000 + .saturating_add((399_000 as Weight).saturating_mul(v as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) - } - fn vote_update(v: u32, ) -> Weight { - (54_724_000 as Weight) - .saturating_add((213_000 as Weight).saturating_mul(v as Weight)) + fn vote_more(v: u32, ) -> Weight { + (69_738_000 as Weight) + // Standard Error: 14_000 + .saturating_add((450_000 as Weight).saturating_mul(v as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) - } - fn remove_voter() -> Weight { - (73_774_000 as Weight) - .saturating_add(T::DbWeight::get().reads(2 as Weight)) + fn vote_less(v: u32, ) -> Weight { + (73_955_000 as Weight) + // Standard Error: 38_000 + .saturating_add((227_000 as Weight).saturating_mul(v as Weight)) + .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) - - } - fn report_defunct_voter_correct(c: u32, v: u32, ) -> Weight { - (0 as Weight) - .saturating_add((1_746_000 as Weight).saturating_mul(c as Weight)) - .saturating_add((31_383_000 as Weight).saturating_mul(v as Weight)) - .saturating_add(T::DbWeight::get().reads(7 as Weight)) - .saturating_add(T::DbWeight::get().writes(3 as Weight)) - } - fn report_defunct_voter_incorrect(c: u32, v: u32, ) -> Weight { - (0 as Weight) - .saturating_add((1_725_000 as Weight).saturating_mul(c as Weight)) - .saturating_add((31_293_000 as Weight).saturating_mul(v as Weight)) - .saturating_add(T::DbWeight::get().reads(6 as Weight)) + fn remove_voter() -> Weight { + (68_398_000 as Weight) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) - } fn submit_candidacy(c: u32, ) -> Weight { - (73_403_000 as Weight) - .saturating_add((314_000 as Weight).saturating_mul(c as Weight)) + (59_291_000 as Weight) + // Standard Error: 2_000 + .saturating_add((412_000 as Weight).saturating_mul(c as Weight)) .saturating_add(T::DbWeight::get().reads(3 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) - } fn renounce_candidacy_candidate(c: u32, ) -> Weight { - (48_834_000 as Weight) - .saturating_add((187_000 as Weight).saturating_mul(c as Weight)) + (55_026_000 as Weight) + // Standard Error: 2_000 + .saturating_add((207_000 as Weight).saturating_mul(c as Weight)) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) - } fn renounce_candidacy_members() -> Weight { - (78_402_000 as Weight) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) + (77_840_000 as Weight) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) - } fn renounce_candidacy_runners_up() -> Weight { - (49_054_000 as Weight) + (54_559_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) - } fn remove_member_with_replacement() -> Weight { - (75_421_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) + (84_311_000 as Weight) + .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(5 as Weight)) - } fn remove_member_wrong_refund() -> Weight { - (8_489_000 as Weight) + (7_677_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) - } - + fn clean_defunct_voters(v: u32, d: u32, ) -> Weight { + (0 as Weight) + // Standard Error: 55_000 + .saturating_add((114_815_000 as Weight).saturating_mul(v as Weight)) + // Standard Error: 53_000 + .saturating_add((49_000 as Weight).saturating_mul(d as Weight)) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().reads((3 as Weight).saturating_mul(v as Weight))) + .saturating_add(T::DbWeight::get().writes((3 as Weight).saturating_mul(v as Weight))) + } + fn election_phragmen(c: u32, v: u32, e: u32, ) -> Weight { + (0 as Weight) + // Standard Error: 1_940_000 + .saturating_add((43_557_000 as Weight).saturating_mul(c as Weight)) + // Standard Error: 807_000 + .saturating_add((65_849_000 as Weight).saturating_mul(v as Weight)) + // Standard Error: 55_000 + .saturating_add((4_206_000 as Weight).saturating_mul(e as Weight)) + .saturating_add(T::DbWeight::get().reads((2 as Weight).saturating_mul(c as Weight))) + .saturating_add(T::DbWeight::get().reads((1 as Weight).saturating_mul(v as Weight))) + .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(c as Weight))) + } } // For backwards compatibility and tests impl WeightInfo for () { - fn vote(v: u32, ) -> Weight { - (89_627_000 as Weight) - .saturating_add((197_000 as Weight).saturating_mul(v as Weight)) + fn vote_equal(v: u32, ) -> Weight { + (45_157_000 as Weight) + // Standard Error: 6_000 + .saturating_add((399_000 as Weight).saturating_mul(v as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) - } - fn vote_update(v: u32, ) -> Weight { - (54_724_000 as Weight) - .saturating_add((213_000 as Weight).saturating_mul(v as Weight)) + fn vote_more(v: u32, ) -> Weight { + (69_738_000 as Weight) + // Standard Error: 14_000 + .saturating_add((450_000 as Weight).saturating_mul(v as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) - } - fn remove_voter() -> Weight { - (73_774_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + fn vote_less(v: u32, ) -> Weight { + (73_955_000 as Weight) + // Standard Error: 38_000 + .saturating_add((227_000 as Weight).saturating_mul(v as Weight)) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) - - } - fn report_defunct_voter_correct(c: u32, v: u32, ) -> Weight { - (0 as Weight) - .saturating_add((1_746_000 as Weight).saturating_mul(c as Weight)) - .saturating_add((31_383_000 as Weight).saturating_mul(v as Weight)) - .saturating_add(RocksDbWeight::get().reads(7 as Weight)) - .saturating_add(RocksDbWeight::get().writes(3 as Weight)) - } - fn report_defunct_voter_incorrect(c: u32, v: u32, ) -> Weight { - (0 as Weight) - .saturating_add((1_725_000 as Weight).saturating_mul(c as Weight)) - .saturating_add((31_293_000 as Weight).saturating_mul(v as Weight)) - .saturating_add(RocksDbWeight::get().reads(6 as Weight)) + fn remove_voter() -> Weight { + (68_398_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) - } fn submit_candidacy(c: u32, ) -> Weight { - (73_403_000 as Weight) - .saturating_add((314_000 as Weight).saturating_mul(c as Weight)) + (59_291_000 as Weight) + // Standard Error: 2_000 + .saturating_add((412_000 as Weight).saturating_mul(c as Weight)) .saturating_add(RocksDbWeight::get().reads(3 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) - } fn renounce_candidacy_candidate(c: u32, ) -> Weight { - (48_834_000 as Weight) - .saturating_add((187_000 as Weight).saturating_mul(c as Weight)) + (55_026_000 as Weight) + // Standard Error: 2_000 + .saturating_add((207_000 as Weight).saturating_mul(c as Weight)) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) - } fn renounce_candidacy_members() -> Weight { - (78_402_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) + (77_840_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) - } fn renounce_candidacy_runners_up() -> Weight { - (49_054_000 as Weight) + (54_559_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) - } fn remove_member_with_replacement() -> Weight { - (75_421_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + (84_311_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(5 as Weight)) - } fn remove_member_wrong_refund() -> Weight { - (8_489_000 as Weight) + (7_677_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) - } - + fn clean_defunct_voters(v: u32, d: u32, ) -> Weight { + (0 as Weight) + // Standard Error: 55_000 + .saturating_add((114_815_000 as Weight).saturating_mul(v as Weight)) + // Standard Error: 53_000 + .saturating_add((49_000 as Weight).saturating_mul(d as Weight)) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + .saturating_add(RocksDbWeight::get().reads((3 as Weight).saturating_mul(v as Weight))) + .saturating_add(RocksDbWeight::get().writes((3 as Weight).saturating_mul(v as Weight))) + } + fn election_phragmen(c: u32, v: u32, e: u32, ) -> Weight { + (0 as Weight) + // Standard Error: 1_940_000 + .saturating_add((43_557_000 as Weight).saturating_mul(c as Weight)) + // Standard Error: 807_000 + .saturating_add((65_849_000 as Weight).saturating_mul(v as Weight)) + // Standard Error: 55_000 + .saturating_add((4_206_000 as Weight).saturating_mul(e as Weight)) + .saturating_add(RocksDbWeight::get().reads((2 as Weight).saturating_mul(c as Weight))) + .saturating_add(RocksDbWeight::get().reads((1 as Weight).saturating_mul(v as Weight))) + .saturating_add(RocksDbWeight::get().writes((1 as Weight).saturating_mul(c as Weight))) + } } diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 1d3048cc9c0b6..2888abc306b3d 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -1263,16 +1263,16 @@ pub trait ChangeMembers { /// /// This resets any previous value of prime. fn set_members_sorted(new_members: &[AccountId], old_members: &[AccountId]) { - let (incoming, outgoing) = Self::compute_members_diff(new_members, old_members); + let (incoming, outgoing) = Self::compute_members_diff_sorted(new_members, old_members); Self::change_members_sorted(&incoming[..], &outgoing[..], &new_members); } /// Compute diff between new and old members; they **must already be sorted**. /// /// Returns incoming and outgoing members. - fn compute_members_diff( + fn compute_members_diff_sorted( new_members: &[AccountId], - old_members: &[AccountId] + old_members: &[AccountId], ) -> (Vec, Vec) { let mut old_iter = old_members.iter(); let mut new_iter = new_members.iter(); @@ -1306,6 +1306,11 @@ pub trait ChangeMembers { /// Set the prime member. fn set_prime(_prime: Option) {} + + /// Get the current prime. + fn get_prime() -> Option { + None + } } impl ChangeMembers for () {