From 0719f950311b97bdd83b15a452601a8a2b5146d6 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 4 Feb 2021 10:27:04 +0000 Subject: [PATCH 01/16] All done --- frame/elections-phragmen/src/lib.rs | 594 +++++++++++++++------------- 1 file changed, 326 insertions(+), 268 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index d566975e2e7a9..7394e761c1418 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -100,10 +100,7 @@ use codec::{Decode, Encode}; use frame_support::{ - decl_error, decl_event, decl_module, decl_storage, - dispatch::{DispatchResultWithPostInfo, WithPostDispatchInfo}, - ensure, - storage::{IterableStorageMap, StorageMap}, + dispatch::{WithPostDispatchInfo}, traits::{ ChangeMembers, Contains, ContainsLengthBound, Currency, CurrencyToVote, Get, InitializeMembers, LockIdentifier, LockableCurrency, OnUnbalanced, ReservableCurrency, @@ -111,7 +108,6 @@ use frame_support::{ }, weights::Weight, }; -use frame_system::{ensure_root, ensure_signed}; use sp_npos_elections::{ElectionResult, ExtendedBalance}; use sp_runtime::{ traits::{Saturating, StaticLookup, Zero}, @@ -170,213 +166,98 @@ pub struct SeatHolder { pub deposit: Balance, } -pub trait Config: frame_system::Config { - /// The overarching event type.c - type Event: From> + Into<::Event>; +pub use pallet::*; - /// Identifier for the elections-phragmen pallet's lock - type ModuleId: Get; - - /// The currency that people are electing with. - type Currency: - LockableCurrency + - ReservableCurrency; - - /// What to do when the members change. - type ChangeMembers: ChangeMembers; - - /// What to do with genesis members - type InitializeMembers: InitializeMembers; - - /// Convert a balance into a number used for election calculation. - /// This must fit into a `u64` but is allowed to be sensibly lossy. - type CurrencyToVote: CurrencyToVote>; - - /// How much should be locked up in order to submit one's candidacy. - type CandidacyBond: 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>; +#[frame_support::pallet] +pub mod pallet { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + use super::*; - /// The amount of bond that need to be locked for each vote (32 bytes). - type VotingBondFactor: Get>; + #[pallet::config] + pub trait Config: frame_system::Config { + type Event: From> + + Into<::Event> + + IsType<::Event>; - /// Handler for the unbalanced reduction when a candidate has lost (and is not a runner-up) - type LoserCandidate: OnUnbalanced>; + /// Identifier for the elections-phragmen pallet's lock + #[pallet::constant] + type ModuleId: Get; - /// Handler for the unbalanced reduction when a member has been kicked. - type KickedMember: OnUnbalanced>; + /// The currency that people are electing with. + type Currency: LockableCurrency + + ReservableCurrency; - /// Number of members to elect. - type DesiredMembers: Get; + /// What to do when the members change. + type ChangeMembers: ChangeMembers; - /// Number of runners_up to keep. - type DesiredRunnersUp: Get; + /// What to do with genesis members + type InitializeMembers: InitializeMembers; - /// How long each seat is kept. This defines the next block number at which an election - /// round will happen. If set to zero, no elections are ever triggered and the module will - /// be in passive mode. - type TermDuration: Get; + /// Convert a balance into a number used for election calculation. + /// This must fit into a `u64` but is allowed to be sensibly lossy. + type CurrencyToVote: CurrencyToVote>; - /// Weight information for extrinsics in this pallet. - type WeightInfo: WeightInfo; -} + /// How much should be locked up in order to submit one's candidacy. + #[pallet::constant] + type CandidacyBond: Get>; -decl_storage! { - trait Store for Module as PhragmenElection { - /// The current elected members. + /// Base deposit associated with voting. /// - /// Invariant: Always sorted based on account id. - pub Members get(fn members): Vec>>; + /// This should be sensibly high to economically ensure the pallet cannot be attacked by + /// creating a gigantic number of votes. + #[pallet::constant] + type VotingBondBase: Get>; - /// 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 amount of bond that need to be locked for each vote (32 bytes). + #[pallet::constant] + type VotingBondFactor: Get>; - /// 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)>; + /// Handler for the unbalanced reduction when a candidate has lost (and is not a runner-up) + type LoserCandidate: OnUnbalanced>; - /// The total number of vote rounds that have happened, excluding the upcoming one. - pub ElectionRounds get(fn election_rounds): u32 = Zero::zero(); + /// Handler for the unbalanced reduction when a member has been kicked. + type KickedMember: OnUnbalanced>; - /// 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 => Voter>; - } add_extra_genesis { - config(members): Vec<(T::AccountId, BalanceOf)>; - build(|config: &GenesisConfig| { - assert!( - config.members.len() as u32 <= T::DesiredMembers::get(), - "Cannot accept more than DesiredMembers genesis member", - ); - let members = config.members.iter().map(|(ref member, ref stake)| { - // make sure they have enough stake. - assert!( - T::Currency::free_balance(member) >= *stake, - "Genesis member does not have enough stake.", - ); + /// Number of members to elect. + #[pallet::constant] + type DesiredMembers: Get; - // 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(|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. 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() }, - ); + /// Number of runners_up to keep. + #[pallet::constant] + type DesiredRunnersUp: Get; - member.clone() - }).collect::>(); + /// How long each seat is kept. This defines the next block number at which an election + /// round will happen. If set to zero, no elections are ever triggered and the module will + /// be in passive mode. + #[pallet::constant] + type TermDuration: Get; - // report genesis members to upstream, if any. - T::InitializeMembers::initialize_members(&members); - }) + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; } -} -decl_error! { - pub enum Error for Module { - /// Cannot vote when no candidates or members exist. - UnableToVote, - /// Must vote for at least one candidate. - NoVotes, - /// Cannot vote more than candidates. - TooManyVotes, - /// Cannot vote more than maximum allowed. - MaximumVotesExceeded, - /// Cannot vote with stake less than minimum balance. - LowBalance, - /// Voter can not pay voting bond. - UnableToPayBond, - /// Must be a voter. - MustBeVoter, - /// Cannot report self. - ReportSelf, - /// Duplicated candidate submission. - DuplicatedCandidate, - /// Member cannot re-submit candidacy. - MemberSubmit, - /// Runner cannot re-submit candidacy. - RunnerUpSubmit, - /// Candidate does not have enough funds. - InsufficientCandidateFunds, - /// Not a member. - NotMember, - /// The provided count of number of candidates is incorrect. - InvalidWitnessData, - /// The provided count of number of votes is incorrect. - InvalidVoteCount, - /// The renouncing origin presented a wrong `Renouncing` parameter. - InvalidRenouncing, - /// Prediction regarding replacement after member removal is wrong. - InvalidReplacement, - } -} + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(PhantomData); -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. + #[pallet::hooks] + impl Hooks> for Pallet { + /// What to do at the end of each block. /// - /// 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), + /// Checks if an election needs to happen or not. + fn on_initialize(n: T::BlockNumber) -> Weight { + let term_duration = T::TermDuration::get(); + if !term_duration.is_zero() && (n % term_duration).is_zero() { + Self::do_phragmen() + } else { + 0 + } + } } -); - -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 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(); + #[pallet::call] + impl Pallet { /// 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. /// @@ -400,16 +281,16 @@ decl_module! { /// # /// We assume the maximum weight among all 3 cases: vote_equal, vote_more and vote_less. /// # - #[weight = + #[pallet::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, + )] + pub(crate) fn vote( + origin: OriginFor, votes: Vec, - #[compact] value: BalanceOf, - ) { + #[pallet::compact] value: BalanceOf, + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; // votes should not be empty and more than `MAXIMUM_VOTE` in any case. @@ -423,9 +304,8 @@ decl_module! { // 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 - .saturating_add(members_count) - .saturating_add(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); @@ -438,27 +318,24 @@ decl_module! { 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 => {}, + 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_stake = value.min(T::Currency::total_balance(&who)); - T::Currency::set_lock( - T::ModuleId::get(), - &who, - locked_stake, - WithdrawReasons::all(), - ); + T::Currency::set_lock(T::ModuleId::get(), &who, locked_stake, WithdrawReasons::all()); Voting::::insert(&who, Voter { votes, deposit: new_deposit, stake: locked_stake }); + Ok(None.into()) } /// Remove `origin` as a voter. @@ -466,11 +343,12 @@ decl_module! { /// 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) { + #[pallet::weight(T::WeightInfo::remove_voter())] + pub(crate) fn remove_voter(origin: OriginFor) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; ensure!(Self::is_voter(&who), Error::::MustBeVoter); Self::do_remove_voter(&who); + Ok(None.into()) } /// Submit oneself for candidacy. A fixed amount of deposit is recorded. @@ -488,15 +366,15 @@ decl_module! { /// # /// 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) { + #[pallet::weight(T::WeightInfo::submit_candidacy(*candidate_count))] + pub(crate) fn submit_candidacy( + origin: OriginFor, + #[pallet::compact] candidate_count: u32, + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let actual_count = >::decode_len().unwrap_or(0); - ensure!( - actual_count as u32 <= candidate_count, - Error::::InvalidWitnessData, - ); + ensure!(actual_count as u32 <= candidate_count, Error::::InvalidWitnessData,); let index = Self::is_candidate(&who).err().ok_or(Error::::DuplicatedCandidate)?; @@ -507,6 +385,7 @@ decl_module! { .map_err(|_| Error::::InsufficientCandidateFunds)?; >::mutate(|c| c.insert(index, (who, T::CandidacyBond::get()))); + Ok(None.into()) } /// Renounce one's intention to be a candidate for the next election round. 3 potential @@ -518,27 +397,30 @@ decl_module! { /// origin is removed as a runner-up. /// - `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_members`], if replacement runners exists, they are immediately used. - /// If the prime is renouncing, then no prime will exist until the next round. + /// 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 { + #[pallet::weight(match *renouncing { Renouncing::Candidate(count) => T::WeightInfo::renounce_candidacy_candidate(count), Renouncing::Member => T::WeightInfo::renounce_candidacy_members(), Renouncing::RunnerUp => T::WeightInfo::renounce_candidacy_runners_up(), - }] - fn renounce_candidacy(origin, renouncing: Renouncing) { + })] + pub(crate) fn renounce_candidacy( + origin: OriginFor, + renouncing: Renouncing, + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; match renouncing { Renouncing::Member => { let _ = Self::remove_and_replace_member(&who, false) .map_err(|_| Error::::InvalidRenouncing)?; - Self::deposit_event(RawEvent::Renounced(who)); - }, + Self::deposit_event(Event::Renounced(who)); + } Renouncing::RunnerUp => { >::try_mutate::<_, Error, _>(|runners_up| { let index = runners_up @@ -549,7 +431,7 @@ decl_module! { 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)); + Self::deposit_event(Event::Renounced(who)); Ok(()) })?; } @@ -562,11 +444,12 @@ decl_module! { let (_removed, deposit) = candidates.remove(index); let _remainder = T::Currency::unreserve(&who, deposit); debug_assert!(_remainder.is_zero()); - Self::deposit_event(RawEvent::Renounced(who)); + Self::deposit_event(Event::Renounced(who)); Ok(()) })?; } }; + Ok(None.into()) } /// Remove a particular member from the set. This is effective immediately and the bond of @@ -583,13 +466,13 @@ decl_module! { /// 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 { + #[pallet::weight(if *has_replacement { T::WeightInfo::remove_member_with_replacement() } else { T::BlockWeights::get().max_block - }] - fn remove_member( - origin, + })] + pub(crate) fn remove_member( + origin: OriginFor, who: ::Source, has_replacement: bool, ) -> DispatchResultWithPostInfo { @@ -601,13 +484,13 @@ decl_module! { // 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. - T::WeightInfo::remove_member_wrong_refund() + T::WeightInfo::remove_member_wrong_refund(), )); } let had_replacement = Self::remove_and_replace_member(&who, true)?; debug_assert_eq!(has_replacement, had_replacement); - Self::deposit_event(RawEvent::MemberKicked(who.clone())); + Self::deposit_event(Event::MemberKicked(who.clone())); if !had_replacement { Self::do_phragmen(); @@ -627,36 +510,206 @@ decl_module! { /// # /// 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) { + #[pallet::weight(T::WeightInfo::clean_defunct_voters(*_num_voters, *_num_defunct))] + pub(crate) fn clean_defunct_voters( + origin: OriginFor, + _num_voters: u32, + _num_defunct: u32, + ) -> DispatchResultWithPostInfo { let _ = ensure_root(origin)?; >::iter() .filter(|(_, x)| Self::is_defunct_voter(&x.votes)) - .for_each(|(dv, _)| { - Self::do_remove_voter(&dv) - }) + .for_each(|(dv, _)| Self::do_remove_voter(&dv)); + + Ok(None.into()) } + } - /// What to do at the end of each block. + #[pallet::event] + #[pallet::metadata(::AccountId = "AccountId", BalanceOf = "Balance")] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// 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, BalanceOf)>), + /// 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. /// - /// Checks if an election needs to happen or not. - fn on_initialize(n: T::BlockNumber) -> Weight { - let term_duration = T::TermDuration::get(); - if !term_duration.is_zero() && (n % term_duration).is_zero() { - Self::do_phragmen() - } else { - 0 - } + /// Note that old members and runners-up are also candidates. + CandidateSlashed(::AccountId, BalanceOf), + /// A \[seat holder\] was slashed by \[amount\] by being forcefully removed from the set. + SeatHolderSlashed(::AccountId, BalanceOf), + } + + #[deprecated(note = "use `Event` instead")] + pub type RawEvent = Event; + + #[pallet::error] + pub enum Error { + /// Cannot vote when no candidates or members exist. + UnableToVote, + /// Must vote for at least one candidate. + NoVotes, + /// Cannot vote more than candidates. + TooManyVotes, + /// Cannot vote more than maximum allowed. + MaximumVotesExceeded, + /// Cannot vote with stake less than minimum balance. + LowBalance, + /// Voter can not pay voting bond. + UnableToPayBond, + /// Must be a voter. + MustBeVoter, + /// Cannot report self. + ReportSelf, + /// Duplicated candidate submission. + DuplicatedCandidate, + /// Member cannot re-submit candidacy. + MemberSubmit, + /// Runner cannot re-submit candidacy. + RunnerUpSubmit, + /// Candidate does not have enough funds. + InsufficientCandidateFunds, + /// Not a member. + NotMember, + /// The provided count of number of candidates is incorrect. + InvalidWitnessData, + /// The provided count of number of votes is incorrect. + InvalidVoteCount, + /// The renouncing origin presented a wrong `Renouncing` parameter. + InvalidRenouncing, + /// Prediction regarding replacement after member removal is wrong. + InvalidReplacement, + } + + #[pallet::origin] + pub struct Origin(PhantomData); + + /// The current elected members. + /// + /// Invariant: Always sorted based on account id. + #[pallet::storage] + #[pallet::getter(fn members)] + pub type Members = + StorageValue<_, Vec>>, ValueQuery>; + + /// 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. + #[pallet::storage] + #[pallet::getter(fn runners_up)] + pub type RunnersUp = + StorageValue<_, Vec>>, ValueQuery>; + + /// 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. + #[pallet::storage] + #[pallet::getter(fn candidates)] + pub type Candidates = StorageValue<_, Vec<(T::AccountId, BalanceOf)>, ValueQuery>; + + #[pallet::type_value] + pub fn DefaultForElectionRounds() -> u32 { + Zero::zero() + } + + /// The total number of vote rounds that have happened, excluding the upcoming one. + #[pallet::storage] + #[pallet::getter(fn election_rounds)] + pub type ElectionRounds = StorageValue<_, u32, ValueQuery, DefaultForElectionRounds>; + + /// Votes and locked stake of a particular voter. + /// + /// TWOX-NOTE: SAFE as `AccountId` is a crypto hash. + #[pallet::storage] + #[pallet::getter(fn voting)] + pub type Voting = + StorageMap<_, Twox64Concat, T::AccountId, Voter>, ValueQuery>; + + #[pallet::genesis_config] + pub struct GenesisConfig { + pub members: Vec<(T::AccountId, BalanceOf)>, + } + + #[cfg(feature = "std")] + impl Default for GenesisConfig { + fn default() -> Self { + Self { members: Default::default() } + } + } + + #[pallet::genesis_build] + impl GenesisBuild for GenesisConfig { + fn build(&self) { + let extra_genesis_builder: fn(&Self) = |config: &GenesisConfig| { + assert!( + config.members.len() as u32 <= T::DesiredMembers::get(), + "Cannot accept more than DesiredMembers genesis member", + ); + let members = config + .members + .iter() + .map(|(ref member, ref stake)| { + assert!( + T::Currency::free_balance(member) >= *stake, + "Genesis member does not have enough stake.", + ); + Members::::mutate(|members| { + 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(), + }, + ), + } + }); + >::insert( + &member, + Voter { + votes: vec![member.clone()], + stake: *stake, + deposit: Zero::zero(), + }, + ); + member.clone() + }) + .collect::>(); + T::InitializeMembers::initialize_members(&members); + }; + extra_genesis_builder(self); } } } -impl Module { +impl Pallet { /// 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()) - ) + T::VotingBondBase::get() + .saturating_add(T::VotingBondFactor::get().saturating_mul((count as u32).into())) } /// Attempts to remove a member `who`. If a runner-up exists, it is used as the replacement. @@ -691,7 +744,7 @@ impl Module { 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)); + Self::deposit_event(Event::SeatHolderSlashed(who.clone(), removed.deposit)); } else { T::Currency::unreserve(who, removed.deposit); } @@ -828,7 +881,7 @@ impl Module { candidates_and_deposit.append(&mut Self::implicit_candidates_with_deposit()); if candidates_and_deposit.len().is_zero() { - Self::deposit_event(RawEvent::EmptyTerm); + Self::deposit_event(Event::EmptyTerm); return T::DbWeight::get().reads(5); } @@ -955,7 +1008,7 @@ impl Module { { let (imbalance, _) = T::Currency::slash_reserved(c, *d); T::LoserCandidate::on_unbalanced(imbalance); - Self::deposit_event(RawEvent::CandidateSlashed(c.clone(), *d)); + Self::deposit_event(Event::CandidateSlashed(c.clone(), *d)); } }); @@ -995,18 +1048,18 @@ impl Module { // clean candidates. >::kill(); - Self::deposit_event(RawEvent::NewTerm(new_members_sorted_by_id)); - ElectionRounds::mutate(|v| *v += 1); + Self::deposit_event(Event::NewTerm(new_members_sorted_by_id)); + >::mutate(|v| *v += 1); }).map_err(|e| { frame_support::debug::error!("elections-phragmen: failed to run election [{:?}].", e); - Self::deposit_event(RawEvent::ElectionError); + Self::deposit_event(Event::ElectionError); }); T::WeightInfo::election_phragmen(weight_candidates, weight_voters, weight_edges) } } -impl Contains for Module { +impl Contains for Pallet { fn contains(who: &T::AccountId) -> bool { Self::is_member(who) } @@ -1028,8 +1081,10 @@ impl Contains for Module { } } -impl ContainsLengthBound for Module { - fn min_len() -> usize { 0 } +impl ContainsLengthBound for Pallet { + fn min_len() -> usize { + 0 + } /// Implementation uses a parameter type so calling is cost-free. fn max_len() -> usize { @@ -1040,15 +1095,18 @@ impl ContainsLengthBound for Module { #[cfg(test)] mod tests { use super::*; - use frame_support::{assert_ok, assert_noop, parameter_types, - traits::OnInitialize, + use frame_support::{ + assert_ok, assert_noop, parameter_types, traits::OnInitialize, + dispatch::DispatchResultWithPostInfo, }; use substrate_test_utils::assert_eq_uvec; use sp_core::H256; use sp_runtime::{ - testing::Header, BuildStorage, DispatchResult, + BuildStorage, + testing::Header, traits::{BlakeTwo256, IdentityLookup}, }; + use frame_system::ensure_signed; use crate as elections_phragmen; parameter_types! { @@ -1364,11 +1422,11 @@ mod tests { ensure_members_has_approval_stake(); } - fn submit_candidacy(origin: Origin) -> DispatchResult { + fn submit_candidacy(origin: Origin) -> DispatchResultWithPostInfo { Elections::submit_candidacy(origin, Elections::candidates().len() as u32) } - fn vote(origin: Origin, votes: Vec, stake: u64) -> DispatchResult { + fn vote(origin: Origin, votes: Vec, stake: u64) -> DispatchResultWithPostInfo { // historical note: helper function was created in a period of time in which the API of vote // call was changing. Currently it is a wrapper for the original call and does not do much. // Nonetheless, totally harmless. @@ -2069,7 +2127,7 @@ mod tests { assert_eq!( System::events().iter().last().unwrap().event, - Event::elections_phragmen(RawEvent::EmptyTerm), + Event::elections_phragmen(super::Event::EmptyTerm), ) }) } @@ -2088,7 +2146,7 @@ mod tests { assert_eq!( System::events().iter().last().unwrap().event, - Event::elections_phragmen(RawEvent::NewTerm(vec![(4, 40), (5, 50)])), + Event::elections_phragmen(super::Event::NewTerm(vec![(4, 40), (5, 50)])), ); assert_eq!(members_and_stake(), vec![(4, 40), (5, 50)]); @@ -2102,7 +2160,7 @@ mod tests { assert_eq!( System::events().iter().last().unwrap().event, - Event::elections_phragmen(RawEvent::NewTerm(vec![])), + Event::elections_phragmen(super::Event::NewTerm(vec![])), ); // outgoing have lost their bond. @@ -2175,7 +2233,7 @@ mod tests { assert_eq!( System::events().iter().last().unwrap().event, - Event::elections_phragmen(RawEvent::NewTerm(vec![])), + Event::elections_phragmen(super::Event::NewTerm(vec![])), ) }); } @@ -2541,7 +2599,7 @@ mod tests { assert_eq!(balances(&5), (45, 2)); assert!(System::events().iter().any(|event| { - event.event == Event::elections_phragmen(RawEvent::NewTerm(vec![(4, 40), (5, 50)])) + event.event == Event::elections_phragmen(super::Event::NewTerm(vec![(4, 40), (5, 50)])) })); }) } From 1f28b3273e2399cec2692ca87602694bc77c6925 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 4 Feb 2021 11:43:04 +0000 Subject: [PATCH 02/16] Fix benchmarks --- frame/elections-phragmen/src/benchmarking.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index 511d2751a5d77..6a0178c49cb87 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -23,7 +23,7 @@ use super::*; use frame_system::RawOrigin; use frame_benchmarking::{benchmarks, account}; -use frame_support::traits::OnInitialize; +use frame_support::{dispatch::DispatchResultWithPostInfo, traits::OnInitialize}; use crate::Module as Elections; @@ -95,11 +95,12 @@ fn submit_candidates_with_self_vote(c: u32, prefix: &'static str) Ok(candidates) } - /// Submit one voter. -fn submit_voter(caller: T::AccountId, votes: Vec, stake: BalanceOf) - -> frame_support::dispatch::DispatchResult -{ +fn submit_voter( + caller: T::AccountId, + votes: Vec, + stake: BalanceOf, +) -> DispatchResultWithPostInfo { >::vote(RawOrigin::Signed(caller).into(), votes, stake) } From efc56236aa17a4e68be6bd4bb0761a30bcb5f635 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Fri, 19 Feb 2021 14:07:36 +0000 Subject: [PATCH 03/16] Apply suggestions from code review Co-authored-by: Guillaume Thiolliere --- frame/elections-phragmen/src/lib.rs | 87 +++++++++++++---------------- 1 file changed, 38 insertions(+), 49 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 7394e761c1418..f1af8a24cb426 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -177,7 +177,6 @@ pub mod pallet { #[pallet::config] pub trait Config: frame_system::Config { type Event: From> - + Into<::Event> + IsType<::Event>; /// Identifier for the elections-phragmen pallet's lock @@ -239,7 +238,7 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] - pub struct Pallet(PhantomData); + pub struct Pallet(_); #[pallet::hooks] impl Hooks> for Pallet { @@ -625,15 +624,10 @@ pub mod pallet { #[pallet::getter(fn candidates)] pub type Candidates = StorageValue<_, Vec<(T::AccountId, BalanceOf)>, ValueQuery>; - #[pallet::type_value] - pub fn DefaultForElectionRounds() -> u32 { - Zero::zero() - } - /// The total number of vote rounds that have happened, excluding the upcoming one. #[pallet::storage] #[pallet::getter(fn election_rounds)] - pub type ElectionRounds = StorageValue<_, u32, ValueQuery, DefaultForElectionRounds>; + pub type ElectionRounds = StorageValue<_, u32, ValueQuery>; /// Votes and locked stake of a particular voter. /// @@ -658,49 +652,44 @@ pub mod pallet { #[pallet::genesis_build] impl GenesisBuild for GenesisConfig { fn build(&self) { - let extra_genesis_builder: fn(&Self) = |config: &GenesisConfig| { + assert!( + self.members.len() as u32 <= T::DesiredMembers::get(), + "Cannot accept more than DesiredMembers genesis member", + ); + let members = self.members.iter().map(|(ref member, ref stake)| { + // make sure they have enough stake. assert!( - config.members.len() as u32 <= T::DesiredMembers::get(), - "Cannot accept more than DesiredMembers genesis member", + T::Currency::free_balance(member) >= *stake, + "Genesis member does not have enough stake.", ); - let members = config - .members - .iter() - .map(|(ref member, ref stake)| { - assert!( - T::Currency::free_balance(member) >= *stake, - "Genesis member does not have enough stake.", - ); - Members::::mutate(|members| { - 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(), - }, - ), - } - }); - >::insert( - &member, - Voter { - votes: vec![member.clone()], - stake: *stake, - deposit: Zero::zero(), - }, - ); - member.clone() - }) - .collect::>(); - T::InitializeMembers::initialize_members(&members); - }; - extra_genesis_builder(self); + + // 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(|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. 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::>(); + + // report genesis members to upstream, if any. + T::InitializeMembers::initialize_members(&members); } } } From 0ddd835262e155786c4efd4441e518b3ff4c5f69 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 19 Feb 2021 14:09:44 +0000 Subject: [PATCH 04/16] Fix metadata. --- frame/elections-phragmen/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 7394e761c1418..31f575c067c04 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -526,7 +526,11 @@ pub mod pallet { } #[pallet::event] - #[pallet::metadata(::AccountId = "AccountId", BalanceOf = "Balance")] + #[pallet::metadata( + ::AccountId = "AccountId", + BalanceOf = "Balance", + Vec<(::AccountId, BalanceOf)> = "Vec<(AccountId, Balance)>", + )] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { /// A new term with \[new_members\]. This indicates that enough candidates existed to run From 397932ebcbaafa704fd679a6e95a9b106f993b53 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 19 Feb 2021 14:12:55 +0000 Subject: [PATCH 05/16] Fix build --- frame/elections-phragmen/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 6683eec5e612a..683b9aeec06a1 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -238,7 +238,7 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] - pub struct Pallet(_); + pub struct Pallet(PhantomData); #[pallet::hooks] impl Hooks> for Pallet { From 86ec8dfd75379a2f18581709069539065607eb44 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 30 Mar 2021 11:22:45 +0200 Subject: [PATCH 06/16] Add migrations --- Cargo.lock | 2 +- frame/elections-phragmen/Cargo.toml | 3 +- frame/elections-phragmen/src/lib.rs | 5 +- .../elections-phragmen/src/migrations/mod.rs | 23 ++++++ .../src/{migrations_3.rs => migrations/v3.rs} | 0 frame/elections-phragmen/src/migrations/v4.rs | 75 +++++++++++++++++++ frame/elections-phragmen/src/migrations_4.rs | 41 ---------- 7 files changed, 104 insertions(+), 45 deletions(-) create mode 100644 frame/elections-phragmen/src/migrations/mod.rs rename frame/elections-phragmen/src/{migrations_3.rs => migrations/v3.rs} (100%) create mode 100644 frame/elections-phragmen/src/migrations/v4.rs delete mode 100644 frame/elections-phragmen/src/migrations_4.rs diff --git a/Cargo.lock b/Cargo.lock index 338bafbc87af1..2536af14c3786 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4933,7 +4933,7 @@ dependencies = [ [[package]] name = "pallet-elections-phragmen" -version = "3.0.0" +version = "4.0.0" dependencies = [ "frame-benchmarking", "frame-support", diff --git a/frame/elections-phragmen/Cargo.toml b/frame/elections-phragmen/Cargo.toml index 1fef728a3662d..58a602da79ffd 100644 --- a/frame/elections-phragmen/Cargo.toml +++ b/frame/elections-phragmen/Cargo.toml @@ -17,6 +17,7 @@ codec = { package = "parity-scale-codec", version = "2.0.0", default-features = serde = { version = "1.0.101", optional = true } sp-runtime = { version = "3.0.0", default-features = false, path = "../../primitives/runtime" } sp-npos-elections = { version = "3.0.0", default-features = false, path = "../../primitives/npos-elections" } +sp-io = { version = "3.0.0", default-features = false, path = "../../primitives/io" } frame-support = { version = "3.0.0", default-features = false, path = "../support" } frame-system = { version = "3.0.0", default-features = false, path = "../system" } sp-std = { version = "3.0.0", default-features = false, path = "../../primitives/std" } @@ -24,7 +25,6 @@ frame-benchmarking = { version = "3.1.0", default-features = false, path = "../b log = { version = "0.4.14", default-features = false } [dev-dependencies] -sp-io = { version = "3.0.0", path = "../../primitives/io" } hex-literal = "0.3.1" pallet-balances = { version = "3.0.0", path = "../balances" } sp-core = { version = "3.0.0", path = "../../primitives/core" } @@ -40,6 +40,7 @@ std = [ "sp-npos-elections/std", "frame-system/std", "sp-std/std", + "sp-io/std", "log/std", ] runtime-benchmarks = [ diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 6d7c67e2013d0..034035d6a017c 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -119,7 +119,8 @@ mod benchmarking; pub mod weights; pub use weights::WeightInfo; -pub mod migrations_3_0_0; +/// All migrations. +pub mod migrations; /// The maximum votes allowed per voter. pub const MAXIMUM_VOTE: usize = 16; @@ -1050,7 +1051,7 @@ impl Pallet { "Failed to run election [{:?}].", e, ); - Self::deposit_event(RawEvent::ElectionError); + Self::deposit_event(Event::ElectionError); }); T::WeightInfo::election_phragmen(weight_candidates, weight_voters, weight_edges) diff --git a/frame/elections-phragmen/src/migrations/mod.rs b/frame/elections-phragmen/src/migrations/mod.rs new file mode 100644 index 0000000000000..9a1f86a1ad7ce --- /dev/null +++ b/frame/elections-phragmen/src/migrations/mod.rs @@ -0,0 +1,23 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 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. + +//! All migrations of this pallet. + +/// Version 3. +pub mod v3; +/// Version 4. +pub mod v4; diff --git a/frame/elections-phragmen/src/migrations_3.rs b/frame/elections-phragmen/src/migrations/v3.rs similarity index 100% rename from frame/elections-phragmen/src/migrations_3.rs rename to frame/elections-phragmen/src/migrations/v3.rs diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs new file mode 100644 index 0000000000000..14b34c5a4aaa4 --- /dev/null +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -0,0 +1,75 @@ +// 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 [`4.0.0`], as denoted by the changelog. + +use frame_support::{ + weights::Weight, + traits::{GetPalletVersion, PalletVersion, Get}, +}; + +/// The old prefix. +pub const OLD_PREFIX: &[u8] = b"PhragmenElection"; + +/// Migrate the entire storage of this pallet to a new prefix. +/// +/// This new prefix must be the same as the one set in construct_runtime. For safety, use +/// `PalletInfo` to get it, as: +/// `::PalletInfo::name::`. +/// +/// The old storage prefix, `PhragmenElection` is hardcoded in the migration code. +pub fn migrate< + T: frame_system::Config + GetPalletVersion, + N: AsRef, +>(new_pallet_name: N) -> Weight { + let maybe_storage_version = ::storage_version(); + log::info!( + target: "runtime::elections-phragmen", + "Running migration to v4 for elections-phragmen with storage version {:?}", + maybe_storage_version, + ); + match maybe_storage_version { + Some(storage_version) if storage_version <= PalletVersion::new(3, 0, 0) => { + frame_support::storage::migration::move_pallet( + OLD_PREFIX, + new_pallet_name.as_ref().as_bytes(), + ); + ::BlockWeights::get().max_block + } + _ => { + log::warn!( + target: "runtime::elections-phragmen", + "Attempted to apply migration to v4 but failed because storage version is {:?}", + maybe_storage_version, + ); + 0 + }, + } +} + +/// Some checks prior to migration. This can be linked to +/// [`frame_support::traits::OnRuntimeUpgrade::pre_upgrade`] for further testing. +/// +/// Panics if anything goes wrong. +pub fn pre_migration>(new: N) { + // ensure some stuff exist in the old prefix. + assert!(sp_io::storage::next_key(OLD_PREFIX).is_some()); + // ensure nothing is stored in the new prefix. + assert!(sp_io::storage::next_key(new.as_ref().as_bytes()).is_none()); + // ensure storage version is 3. + assert!(::storage_version().unwrap().major == 3); +} diff --git a/frame/elections-phragmen/src/migrations_4.rs b/frame/elections-phragmen/src/migrations_4.rs deleted file mode 100644 index 41260d70aaa03..0000000000000 --- a/frame/elections-phragmen/src/migrations_4.rs +++ /dev/null @@ -1,41 +0,0 @@ -// 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 [`4.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}, -}; - -/// Migrate the entire storage of this pallet to a new prefix. -/// -/// This new prefix must be the same as the one set in construct_runtime. For safety, use -/// `PalletInfo` to get it, as: -/// `::PalletInfo::name::`. -/// -/// The old storage prefix, `PhragmenElection` is hardcoded in the migration code. -pub fn migrate>(new_pallet_name: N) -> Weight { - frame_support::migrations::move_pallet( - b"PhragmenElection", - new_pallet_name.as_ref().as_bytes(), - ); - ::BlockWeights::get().max_block -} From f82cca5f9257c1861f92050bc31b982d33a1c6d4 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 30 Mar 2021 12:13:39 +0200 Subject: [PATCH 07/16] Fix --- frame/elections-phragmen/src/migrations/v4.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs index 14b34c5a4aaa4..ff43e2e176644 100644 --- a/frame/elections-phragmen/src/migrations/v4.rs +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -33,17 +33,20 @@ pub const OLD_PREFIX: &[u8] = b"PhragmenElection"; /// /// The old storage prefix, `PhragmenElection` is hardcoded in the migration code. pub fn migrate< - T: frame_system::Config + GetPalletVersion, + T: frame_system::Config, + P: GetPalletVersion, N: AsRef, >(new_pallet_name: N) -> Weight { - let maybe_storage_version = ::storage_version(); + let maybe_storage_version =

::storage_version(); log::info!( target: "runtime::elections-phragmen", "Running migration to v4 for elections-phragmen with storage version {:?}", maybe_storage_version, ); + match maybe_storage_version { Some(storage_version) if storage_version <= PalletVersion::new(3, 0, 0) => { + log::info!("new prefix: {}", new_pallet_name.as_ref()); frame_support::storage::migration::move_pallet( OLD_PREFIX, new_pallet_name.as_ref().as_bytes(), @@ -65,11 +68,11 @@ pub fn migrate< /// [`frame_support::traits::OnRuntimeUpgrade::pre_upgrade`] for further testing. /// /// Panics if anything goes wrong. -pub fn pre_migration>(new: N) { +pub fn pre_migration>(new: N) { // ensure some stuff exist in the old prefix. assert!(sp_io::storage::next_key(OLD_PREFIX).is_some()); // ensure nothing is stored in the new prefix. assert!(sp_io::storage::next_key(new.as_ref().as_bytes()).is_none()); // ensure storage version is 3. - assert!(::storage_version().unwrap().major == 3); + assert!(

::storage_version().unwrap().major == 3); } From 28a7d166f00739e4a1cb04b21104358ec7c7ec11 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 31 Mar 2021 06:54:21 +0200 Subject: [PATCH 08/16] Update frame/elections-phragmen/src/migrations/v4.rs --- frame/elections-phragmen/src/migrations/v4.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs index ff43e2e176644..5bbda61add9a1 100644 --- a/frame/elections-phragmen/src/migrations/v4.rs +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2019-2020 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"); From a14743eeff5f62d87d230d9947141a20e5e7d612 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 31 Mar 2021 07:00:44 +0200 Subject: [PATCH 09/16] Better migeation test --- frame/elections-phragmen/Cargo.toml | 2 ++ frame/elections-phragmen/src/migrations/v4.rs | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/frame/elections-phragmen/Cargo.toml b/frame/elections-phragmen/Cargo.toml index 58a602da79ffd..c5da81b0b4425 100644 --- a/frame/elections-phragmen/Cargo.toml +++ b/frame/elections-phragmen/Cargo.toml @@ -21,6 +21,7 @@ sp-io = { version = "3.0.0", default-features = false, path = "../../primitives/ frame-support = { version = "3.0.0", default-features = false, path = "../support" } frame-system = { version = "3.0.0", default-features = false, path = "../system" } sp-std = { version = "3.0.0", default-features = false, path = "../../primitives/std" } +sp-core = { version = "3.0.0", default-features = false, path = "../../primitives/core" } frame-benchmarking = { version = "3.1.0", default-features = false, path = "../benchmarking", optional = true } log = { version = "0.4.14", default-features = false } @@ -41,6 +42,7 @@ std = [ "frame-system/std", "sp-std/std", "sp-io/std", + "sp-core/std", "log/std", ] runtime-benchmarks = [ diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs index ff43e2e176644..09a97bcddbf16 100644 --- a/frame/elections-phragmen/src/migrations/v4.rs +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -72,7 +72,12 @@ pub fn pre_migration>(new: N) { // ensure some stuff exist in the old prefix. assert!(sp_io::storage::next_key(OLD_PREFIX).is_some()); // ensure nothing is stored in the new prefix. - assert!(sp_io::storage::next_key(new.as_ref().as_bytes()).is_none()); + assert!( + sp_io::storage::next_key(new.as_ref().as_bytes()).is_none(), + "unexpected next_key({}) = {:?}", + new.as_ref(), + sp_core::hexdisplay::HexDisplay::from(&sp_io::storage::next_key(new.as_ref().as_bytes()).unwrap()) + ); // ensure storage version is 3. assert!(

::storage_version().unwrap().major == 3); } From 69f1020fb5d7585b68b2b77207c9b0449cef302c Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 31 Mar 2021 07:36:35 +0200 Subject: [PATCH 10/16] More test --- frame/elections-phragmen/src/migrations/v4.rs | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs index 5abaa886cfc67..2c4e7103151ad 100644 --- a/frame/elections-phragmen/src/migrations/v4.rs +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -69,15 +69,33 @@ pub fn migrate< /// /// Panics if anything goes wrong. pub fn pre_migration>(new: N) { + let new = new.as_ref(); + log::info!("pre-migration elections-phragmen test with new = {}", new); + // ensure some stuff exist in the old prefix. assert!(sp_io::storage::next_key(OLD_PREFIX).is_some()); // ensure nothing is stored in the new prefix. assert!( - sp_io::storage::next_key(new.as_ref().as_bytes()).is_none(), + sp_io::storage::next_key(new.as_bytes()).map_or( + // either nothing is there + true, + // or we ensure that it has no common prefix with twox_128(new). + |next_key| !next_key.starts_with(&sp_io::hashing::twox_128(new.as_bytes())) + ), "unexpected next_key({}) = {:?}", - new.as_ref(), - sp_core::hexdisplay::HexDisplay::from(&sp_io::storage::next_key(new.as_ref().as_bytes()).unwrap()) + new, + sp_core::hexdisplay::HexDisplay::from(&sp_io::storage::next_key(new.as_bytes()).unwrap()) ); // ensure storage version is 3. assert!(

::storage_version().unwrap().major == 3); } + +/// Some checks for after migration. This can be linked to +/// [`frame_support::traits::OnRuntimeUpgrade::post_upgrade`] for further testing. +/// +/// Panics if anything goes wrong. +pub fn post_migration

() { + log::info!("post-migration elections-phragmen"); + // ensure we've been updated to v4 by the automatic write of crate version -> storage version. + assert!(

::storage_version().unwrap().major == 4); +} From 1f4984bb03c6d20a5e8fdb9e9a0d065b9c37e2a0 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 31 Mar 2021 09:56:25 +0200 Subject: [PATCH 11/16] Fix warn --- frame/elections-phragmen/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index c5bc6da197d17..406f97f50b66a 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -25,7 +25,7 @@ use frame_system::RawOrigin; use frame_benchmarking::{benchmarks, account, impl_benchmark_test_suite}; use frame_support::{dispatch::DispatchResultWithPostInfo, traits::OnInitialize}; -use crate::Module as Elections; +use crate::Pallet as Elections; const BALANCE_FACTOR: u32 = 250; const MAX_VOTERS: u32 = 500; From 8bea3bed0b154de2474d4c4ea9194b654a582a99 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Thu, 15 Apr 2021 09:14:47 +0200 Subject: [PATCH 12/16] Update frame/elections-phragmen/src/lib.rs Co-authored-by: Guillaume Thiolliere --- frame/elections-phragmen/src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 034035d6a017c..89759aa6fd691 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -599,9 +599,6 @@ pub mod pallet { InvalidReplacement, } - #[pallet::origin] - pub struct Origin(PhantomData); - /// The current elected members. /// /// Invariant: Always sorted based on account id. From 9700fc836304cf23657987d3c5ec8a6bcf6aadb7 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 15 Apr 2021 09:42:16 +0200 Subject: [PATCH 13/16] Fix test --- frame/elections-phragmen/src/lib.rs | 2 +- frame/elections-phragmen/src/migrations/v4.rs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 1b7c965c5e080..dafcc3dd5910e 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -1066,7 +1066,7 @@ impl Contains for Pallet { } } -impl SortedMembers for Module { +impl SortedMembers for Pallet { fn contains(who: &T::AccountId) -> bool { Self::is_member(who) } diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs index 2c4e7103151ad..16e7c442fe876 100644 --- a/frame/elections-phragmen/src/migrations/v4.rs +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -72,8 +72,10 @@ pub fn pre_migration>(new: N) { let new = new.as_ref(); log::info!("pre-migration elections-phragmen test with new = {}", new); - // ensure some stuff exist in the old prefix. - assert!(sp_io::storage::next_key(OLD_PREFIX).is_some()); + // the next key must exist, and start with the hash of `OLD_PREFIX`. + let next_key = sp_io::storage::next_key(OLD_PREFIX).unwrap(); + assert!(next_key.starts_with(&sp_io::hashing::twox_128(OLD_PREFIX))); + // ensure nothing is stored in the new prefix. assert!( sp_io::storage::next_key(new.as_bytes()).map_or( From 56d77d470d26171a4b1eef0f8f21a07edeb3071a Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 15 Apr 2021 14:21:36 +0200 Subject: [PATCH 14/16] early exit --- frame/elections-phragmen/src/migrations/v4.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs index 16e7c442fe876..7cd09081c6836 100644 --- a/frame/elections-phragmen/src/migrations/v4.rs +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -37,6 +37,14 @@ pub fn migrate< P: GetPalletVersion, N: AsRef, >(new_pallet_name: N) -> Weight { + if new_pallet_name.as_ref() == OLD_PREFIX { + log::info!( + target: "runtime::elections-phragmen", + "New pallet name is equal to the old prefix. No migration needs to be done.", + maybe_storage_version, + ); + return 0; + } let maybe_storage_version =

::storage_version(); log::info!( target: "runtime::elections-phragmen", From 971af54ccf71690ac178f15cf33ab7fabea75cee Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 15 Apr 2021 14:22:54 +0200 Subject: [PATCH 15/16] Fix --- frame/elections-phragmen/src/migrations/v4.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs index 7cd09081c6836..f704b203d34cf 100644 --- a/frame/elections-phragmen/src/migrations/v4.rs +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -37,11 +37,10 @@ pub fn migrate< P: GetPalletVersion, N: AsRef, >(new_pallet_name: N) -> Weight { - if new_pallet_name.as_ref() == OLD_PREFIX { + if new_pallet_name.as_ref().as_bytes() == OLD_PREFIX { log::info!( target: "runtime::elections-phragmen", "New pallet name is equal to the old prefix. No migration needs to be done.", - maybe_storage_version, ); return 0; } From cd908edf12d9f8a0b0afda6410d16565e375c1a4 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 23 Apr 2021 08:26:52 +0200 Subject: [PATCH 16/16] Fix build --- frame/elections-phragmen/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index f187aca181a2c..3534a62ac3ce0 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -23,7 +23,7 @@ use super::*; use frame_system::RawOrigin; use frame_benchmarking::{benchmarks, account, whitelist, impl_benchmark_test_suite}; -use frame_support::traits::OnInitialize; +use frame_support::{traits::OnInitialize, dispatch::DispatchResultWithPostInfo}; use crate::Pallet as Elections;