From 3491c8e086d9f119e9746f68826ffd3ed004d5e3 Mon Sep 17 00:00:00 2001 From: shamb0 Date: Thu, 18 Feb 2021 17:11:07 +0530 Subject: [PATCH 01/10] port to frame v2 | basic pallet compilation up --- frame/collective/src/lib.rs | 1346 ++++++++++------------------------- 1 file changed, 366 insertions(+), 980 deletions(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 50beb8607d61d..30c8743723b61 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -42,244 +42,89 @@ #![cfg_attr(not(feature = "std"), no_std)] #![recursion_limit = "128"] +#[macro_use] +// mod tests; +// mod mock; +// mod benchmarking; +pub mod weights; + use sp_std::{prelude::*, result}; use sp_core::u32_trait::Value as U32; use sp_io::storage; -use sp_runtime::{RuntimeDebug, traits::Hash}; use frame_support::{ codec::{Decode, Encode}, - debug, decl_error, decl_event, decl_module, decl_storage, - dispatch::{ - DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable, Parameter, + debug, dispatch::{ + DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable, PostDispatchInfo, }, ensure, traits::{ChangeMembers, EnsureOrigin, Get, InitializeMembers}, - weights::{DispatchClass, GetDispatchInfo, Weight, Pays}, + weights::{GetDispatchInfo, Weight}, }; -use frame_system::{self as system, ensure_signed, ensure_root}; - -#[cfg(feature = "runtime-benchmarks")] -mod benchmarking; - -pub mod weights; +#[cfg(feature = "std")] +use frame_support::traits::GenesisBuild; + +// use sp_runtime::{RuntimeDebug, traits::Hash}; +use sp_runtime::{ RuntimeDebug, traits::{ + Hash, +}}; +use frame_system::{self as system}; pub use weights::WeightInfo; -/// Simple index type for proposal counting. -pub type ProposalIndex = u32; - -/// A number of members. -/// -/// This also serves as a number of voting members, and since for motions, each member may -/// vote exactly once, therefore also the number of votes for any given motion. -pub type MemberCount = u32; - -/// Default voting strategy when a member is inactive. -pub trait DefaultVote { - /// Get the default voting strategy, given: - /// - /// - Whether the prime member voted Aye. - /// - Raw number of yes votes. - /// - Raw number of no votes. - /// - Total number of member count. - fn default_vote( - prime_vote: Option, - yes_votes: MemberCount, - no_votes: MemberCount, - len: MemberCount, - ) -> bool; -} - -/// Set the prime member's vote as the default vote. -pub struct PrimeDefaultVote; - -impl DefaultVote for PrimeDefaultVote { - fn default_vote( - prime_vote: Option, - _yes_votes: MemberCount, - _no_votes: MemberCount, - _len: MemberCount, - ) -> bool { - prime_vote.unwrap_or(false) - } -} - -/// First see if yes vote are over majority of the whole collective. If so, set the default vote -/// as yes. Otherwise, use the prime meber's vote as the default vote. -pub struct MoreThanMajorityThenPrimeDefaultVote; - -impl DefaultVote for MoreThanMajorityThenPrimeDefaultVote { - fn default_vote( - prime_vote: Option, - yes_votes: MemberCount, - _no_votes: MemberCount, - len: MemberCount, - ) -> bool { - let more_than_majority = yes_votes * 2 > len; - more_than_majority || prime_vote.unwrap_or(false) - } -} - -pub trait Config: frame_system::Config { - /// The outer origin type. - type Origin: From>; +pub use pallet::*; - /// The outer call dispatch type. - type Proposal: Parameter - + Dispatchable>::Origin, PostInfo=PostDispatchInfo> - + From> - + GetDispatchInfo; - - /// The outer event type. - type Event: From> + Into<::Event>; - - /// The time-out for council motions. - type MotionDuration: Get; +#[frame_support::pallet] +pub mod pallet { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + use super::*; - /// Maximum number of proposals allowed to be active in parallel. - type MaxProposals: Get; + #[pallet::config] + pub trait Config: frame_system::Config { - /// The maximum number of members supported by the pallet. Used for weight estimation. - /// - /// NOTE: - /// + Benchmarks will need to be re-run and weights adjusted if this changes. - /// + This pallet assumes that dependents keep to the limit without enforcing it. - type MaxMembers: Get; + /// The outer origin type. + type Origin: From>; - /// Default vote strategy of this collective. - type DefaultVote: DefaultVote; + /// The outer call dispatch type. + type Proposal: Parameter + + Dispatchable>::Origin, PostInfo=PostDispatchInfo> + + From> + + GetDispatchInfo; - /// Weight information for extrinsics in this pallet. - type WeightInfo: WeightInfo; -} + /// The overarching event type. + type Event: From> + IsType<::Event>; -/// Origin for the collective module. -#[derive(PartialEq, Eq, Clone, RuntimeDebug, Encode, Decode)] -pub enum RawOrigin { - /// It has been condoned by a given number of members of the collective from a given total. - Members(MemberCount, MemberCount), - /// It has been condoned by a single member of the collective. - Member(AccountId), - /// Dummy to manage the fact we have instancing. - _Phantom(sp_std::marker::PhantomData), -} + /// The time-out for council motions. + type MotionDuration: Get; -/// Origin for the collective module. -pub type Origin = RawOrigin<::AccountId, I>; + /// Maximum number of proposals allowed to be active in parallel. + type MaxProposals: Get; -#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)] -/// Info for keeping track of a motion being voted on. -pub struct Votes { - /// The proposal's unique index. - index: ProposalIndex, - /// The number of approval votes that are needed to pass the motion. - threshold: MemberCount, - /// The current set of voters that approved it. - ayes: Vec, - /// The current set of voters that rejected it. - nays: Vec, - /// The hard end time of this vote. - end: BlockNumber, -} + /// The maximum number of members supported by the pallet. Used for weight estimation. + /// + /// NOTE: + /// + Benchmarks will need to be re-run and weights adjusted if this changes. + /// + This pallet assumes that dependents keep to the limit without enforcing it. + type MaxMembers: Get; -decl_storage! { - trait Store for Module, I: Instance=DefaultInstance> as Collective { - /// The hashes of the active proposals. - pub Proposals get(fn proposals): Vec; - /// Actual proposal for a given hash, if it's current. - pub ProposalOf get(fn proposal_of): - map hasher(identity) T::Hash => Option<>::Proposal>; - /// Votes on a given proposal, if it is ongoing. - pub Voting get(fn voting): - map hasher(identity) T::Hash => Option>; - /// Proposals so far. - pub ProposalCount get(fn proposal_count): u32; - /// The current members of the collective. This is stored sorted (just by value). - pub Members get(fn members): Vec; - /// The prime member that helps determine the default vote behavior in case of absentations. - pub Prime get(fn prime): Option; - } - add_extra_genesis { - config(phantom): sp_std::marker::PhantomData; - config(members): Vec; - build(|config| Module::::initialize_members(&config.members)) - } -} + /// Default vote strategy of this collective. + type DefaultVote: DefaultVote; -decl_event! { - pub enum Event where - ::Hash, - ::AccountId, - { - /// A motion (given hash) has been proposed (by given account) with a threshold (given - /// `MemberCount`). - /// \[account, proposal_index, proposal_hash, threshold\] - Proposed(AccountId, ProposalIndex, Hash, MemberCount), - /// A motion (given hash) has been voted on by given account, leaving - /// a tally (yes votes and no votes given respectively as `MemberCount`). - /// \[account, proposal_hash, voted, yes, no\] - Voted(AccountId, Hash, bool, MemberCount, MemberCount), - /// A motion was approved by the required threshold. - /// \[proposal_hash\] - Approved(Hash), - /// A motion was not approved by the required threshold. - /// \[proposal_hash\] - Disapproved(Hash), - /// A motion was executed; result will be `Ok` if it returned without error. - /// \[proposal_hash, result\] - Executed(Hash, DispatchResult), - /// A single member did some action; result will be `Ok` if it returned without error. - /// \[proposal_hash, result\] - MemberExecuted(Hash, DispatchResult), - /// A proposal was closed because its threshold was reached or after its duration was up. - /// \[proposal_hash, yes, no\] - Closed(Hash, MemberCount, MemberCount), + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; } -} -decl_error! { - pub enum Error for Module, I: Instance> { - /// Account is not a member - NotMember, - /// Duplicate proposals not allowed - DuplicateProposal, - /// Proposal must exist - ProposalMissing, - /// Mismatched index - WrongIndex, - /// Duplicate vote ignored - DuplicateVote, - /// Members are already initialized! - AlreadyInitialized, - /// The close call was made too early, before the end of the voting. - TooEarly, - /// There can only be a maximum of `MaxProposals` active proposals. - TooManyProposals, - /// The given weight bound for the proposal was too low. - WrongProposalWeight, - /// The given length bound for the proposal was too low. - WrongProposalLength, - } -} + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(PhantomData<(T, I)>); -/// Return the weight of a dispatch call result as an `Option`. -/// -/// Will return the weight regardless of what the state of the result is. -fn get_result_weight(result: DispatchResultWithPostInfo) -> Option { - match result { - Ok(post_info) => post_info.actual_weight, - Err(err) => err.post_info.actual_weight, + #[pallet::hooks] + impl, I: 'static> Hooks> for Pallet { } -} - - -// Note that councillor operations are assigned to the operational class. -decl_module! { - pub struct Module, I: Instance=DefaultInstance> for enum Call where origin: ::Origin { - type Error = Error; - fn deposit_event() = default; + #[pallet::call] + impl, I: 'static> Pallet { /// Set the collective's membership. /// @@ -292,28 +137,14 @@ decl_module! { /// /// NOTE: Does not enforce the expected `MaxMembers` limit on the amount of members, but /// the weight estimations rely on it to estimate dispatchable weight. - /// - /// # - /// ## Weight - /// - `O(MP + N)` where: - /// - `M` old-members-count (code- and governance-bounded) - /// - `N` new-members-count (code- and governance-bounded) - /// - `P` proposals-count (code-bounded) - /// - DB: - /// - 1 storage mutation (codec `O(M)` read, `O(N)` write) for reading and writing the members - /// - 1 storage read (codec `O(P)`) for reading the proposals - /// - `P` storage mutations (codec `O(M)`) for updating the votes for each proposal - /// - 1 storage write (codec `O(1)`) for deleting the old `prime` and setting the new one - /// # - #[weight = ( + #[pallet::weight( T::WeightInfo::set_members( *old_count, // M new_members.len() as u32, // N T::MaxProposals::get() // P - ), - DispatchClass::Operational - )] - fn set_members(origin, + ))] + fn set_members( + origin: OriginFor, new_members: Vec, prime: Option, old_count: MemberCount, @@ -350,34 +181,27 @@ decl_module! { /// Dispatch a proposal from a member using the `Member` origin. /// /// Origin must be a member of the collective. - /// - /// # - /// ## Weight - /// - `O(M + P)` where `M` members-count (code-bounded) and `P` complexity of dispatching `proposal` - /// - DB: 1 read (codec `O(M)`) + DB access of `proposal` - /// - 1 event - /// # - #[weight = ( + #[pallet::weight( T::WeightInfo::execute( *length_bound, // B T::MaxMembers::get(), // M - ).saturating_add(proposal.get_dispatch_info().weight), // P - DispatchClass::Operational + ) )] - fn execute(origin, + fn execute( + origin: OriginFor, proposal: Box<>::Proposal>, - #[compact] length_bound: u32, + #[pallet::compact] length_bound: u32, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let members = Self::members(); - ensure!(members.contains(&who), Error::::NotMember); + ensure!(members.contains(&who), >::NotMember); let proposal_len = proposal.using_encoded(|x| x.len()); - ensure!(proposal_len <= length_bound as usize, Error::::WrongProposalLength); + ensure!(proposal_len <= length_bound as usize, >::WrongProposalLength); let proposal_hash = T::Hashing::hash_of(&proposal); let result = proposal.dispatch(RawOrigin::Member(who).into()); Self::deposit_event( - RawEvent::MemberExecuted(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) + Event::MemberExecuted(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) ); Ok(get_result_weight(result).map(|w| { @@ -415,7 +239,7 @@ decl_module! { /// - 1 storage write `Voting` (codec `O(M)`) /// - 1 event /// # - #[weight = ( + #[pallet::weight( if *threshold < 2 { T::WeightInfo::propose_execute( *length_bound, // B @@ -427,28 +251,28 @@ decl_module! { T::MaxMembers::get(), // M T::MaxProposals::get(), // P2 ) - }, - DispatchClass::Operational + } )] - fn propose(origin, - #[compact] threshold: MemberCount, - proposal: Box<>::Proposal>, - #[compact] length_bound: u32 + fn propose( + origin: OriginFor, + #[pallet::compact] threshold: MemberCount, + proposal: Box, + #[pallet::compact] length_bound: u32 ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let members = Self::members(); - ensure!(members.contains(&who), Error::::NotMember); + ensure!(members.contains(&who), >::NotMember); let proposal_len = proposal.using_encoded(|x| x.len()); - ensure!(proposal_len <= length_bound as usize, Error::::WrongProposalLength); + ensure!(proposal_len <= length_bound as usize, >::WrongProposalLength); let proposal_hash = T::Hashing::hash_of(&proposal); - ensure!(!>::contains_key(proposal_hash), Error::::DuplicateProposal); + ensure!(!>::contains_key(proposal_hash), >::DuplicateProposal); if threshold < 2 { let seats = Self::members().len() as MemberCount; let result = proposal.dispatch(RawOrigin::Members(1, seats).into()); Self::deposit_event( - RawEvent::Executed(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) + Event::Executed(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) ); Ok(get_result_weight(result).map(|w| { @@ -463,18 +287,19 @@ decl_module! { proposals.push(proposal_hash); ensure!( proposals.len() <= T::MaxProposals::get() as usize, - Error::::TooManyProposals + >::TooManyProposals ); Ok(proposals.len()) })?; let index = Self::proposal_count(); - >::mutate(|i| *i += 1); - >::insert(proposal_hash, *proposal); + + >::mutate(|i| *i += 1); + >::insert(proposal_hash, Some(*proposal)); let end = system::Module::::block_number() + T::MotionDuration::get(); let votes = Votes { index, threshold, ayes: vec![who.clone()], nays: vec![], end }; - >::insert(proposal_hash, votes); + >::insert(proposal_hash, Some(votes)); - Self::deposit_event(RawEvent::Proposed(who, index, proposal_hash, threshold)); + Self::deposit_event(Event::Proposed(who, index, proposal_hash, threshold)); Ok(Some(T::WeightInfo::propose_proposed( proposal_len as u32, // B @@ -490,29 +315,19 @@ decl_module! { /// /// Transaction fees will be waived if the member is voting on any particular proposal /// for the first time and the call is successful. Subsequent vote changes will charge a fee. - /// # - /// ## Weight - /// - `O(M)` where `M` is members-count (code- and governance-bounded) - /// - DB: - /// - 1 storage read `Members` (codec `O(M)`) - /// - 1 storage mutation `Voting` (codec `O(M)`) - /// - 1 event - /// # - #[weight = ( - T::WeightInfo::vote(T::MaxMembers::get()), - DispatchClass::Operational - )] - fn vote(origin, + #[pallet::weight(T::WeightInfo::vote(T::MaxMembers::get()))] + fn vote( + origin: OriginFor, proposal: T::Hash, - #[compact] index: ProposalIndex, + #[pallet::compact] index: ProposalIndex, approve: bool, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let members = Self::members(); - ensure!(members.contains(&who), Error::::NotMember); + ensure!(members.contains(&who), >::NotMember); - let mut voting = Self::voting(&proposal).ok_or(Error::::ProposalMissing)?; - ensure!(voting.index == index, Error::::WrongIndex); + let mut voting = Self::voting(&proposal).ok_or(>::ProposalMissing)?; + ensure!(voting.index == index, >::WrongIndex); let position_yes = voting.ayes.iter().position(|a| a == &who); let position_no = voting.nays.iter().position(|a| a == &who); @@ -524,7 +339,7 @@ decl_module! { if position_yes.is_none() { voting.ayes.push(who.clone()); } else { - Err(Error::::DuplicateVote)? + Err(>::DuplicateVote)? } if let Some(pos) = position_no { voting.nays.swap_remove(pos); @@ -533,7 +348,7 @@ decl_module! { if position_no.is_none() { voting.nays.push(who.clone()); } else { - Err(Error::::DuplicateVote)? + Err(>::DuplicateVote)? } if let Some(pos) = position_yes { voting.ayes.swap_remove(pos); @@ -542,9 +357,9 @@ decl_module! { let yes_votes = voting.ayes.len() as MemberCount; let no_votes = voting.nays.len() as MemberCount; - Self::deposit_event(RawEvent::Voted(who, proposal, approve, yes_votes, no_votes)); + Self::deposit_event(Event::Voted(who, proposal, approve, yes_votes, no_votes)); - Voting::::insert(&proposal, voting); + >::insert(&proposal, Some(voting)); if is_account_voting_first_time { Ok(( @@ -576,20 +391,7 @@ decl_module! { /// + `length_bound`: The upper bound for the length of the proposal in storage. Checked via /// `storage::read` so it is `size_of::() == 4` larger than the pure length. /// - /// # - /// ## Weight - /// - `O(B + M + P1 + P2)` where: - /// - `B` is `proposal` size in bytes (length-fee-bounded) - /// - `M` is members-count (code- and governance-bounded) - /// - `P1` is the complexity of `proposal` preimage. - /// - `P2` is proposal-count (code-bounded) - /// - DB: - /// - 2 storage reads (`Members`: codec `O(M)`, `Prime`: codec `O(1)`) - /// - 3 mutations (`Voting`: codec `O(M)`, `ProposalOf`: codec `O(B)`, `Proposals`: codec `O(P2)`) - /// - any mutations done while executing `proposal` (`P1`) - /// - up to 3 events - /// # - #[weight = ( + #[pallet::weight( { let b = *length_bound; let m = T::MaxMembers::get(); @@ -600,19 +402,19 @@ decl_module! { .max(T::WeightInfo::close_approved(b, m, p2)) .max(T::WeightInfo::close_disapproved(m, p2)) .saturating_add(p1) - }, - DispatchClass::Operational + } )] - fn close(origin, + fn close( + origin: OriginFor, proposal_hash: T::Hash, - #[compact] index: ProposalIndex, - #[compact] proposal_weight_bound: Weight, - #[compact] length_bound: u32 + #[pallet::compact] index: ProposalIndex, + #[pallet::compact] proposal_weight_bound: Weight, + #[pallet::compact] length_bound: u32 ) -> DispatchResultWithPostInfo { let _ = ensure_signed(origin)?; - let voting = Self::voting(&proposal_hash).ok_or(Error::::ProposalMissing)?; - ensure!(voting.index == index, Error::::WrongIndex); + let voting = Self::voting(&proposal_hash).ok_or(>::ProposalMissing)?; + ensure!(voting.index == index, >::WrongIndex); let mut no_votes = voting.nays.len() as MemberCount; let mut yes_votes = voting.ayes.len() as MemberCount; @@ -626,7 +428,7 @@ decl_module! { length_bound, proposal_weight_bound, )?; - Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); + Self::deposit_event(Event::Closed(proposal_hash, yes_votes, no_votes)); let (proposal_weight, proposal_count) = Self::do_approve_proposal(seats, voting, proposal_hash, proposal); return Ok(( @@ -636,7 +438,7 @@ decl_module! { ).into()); } else if disapproved { - Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); + Self::deposit_event(Event::Closed(proposal_hash, yes_votes, no_votes)); let proposal_count = Self::do_disapprove_proposal(proposal_hash); return Ok(( Some(T::WeightInfo::close_early_disapproved(seats, proposal_count)), @@ -645,7 +447,7 @@ decl_module! { } // Only allow actual closing of the proposal after the voting period has ended. - ensure!(system::Module::::block_number() >= voting.end, Error::::TooEarly); + ensure!(system::Module::::block_number() >= voting.end, >::TooEarly); let prime_vote = Self::prime().map(|who| voting.ayes.iter().any(|a| a == &who)); @@ -665,7 +467,7 @@ decl_module! { length_bound, proposal_weight_bound, )?; - Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); + Self::deposit_event(Event::Closed(proposal_hash, yes_votes, no_votes)); let (proposal_weight, proposal_count) = Self::do_approve_proposal(seats, voting, proposal_hash, proposal); return Ok(( @@ -674,7 +476,7 @@ decl_module! { Pays::Yes, ).into()); } else { - Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); + Self::deposit_event(Event::Closed(proposal_hash, yes_votes, no_votes)); let proposal_count = Self::do_disapprove_proposal(proposal_hash); return Ok(( Some(T::WeightInfo::close_disapproved(seats, proposal_count)), @@ -689,23 +491,259 @@ decl_module! { /// /// Parameters: /// * `proposal_hash`: The hash of the proposal that should be disapproved. - /// - /// # - /// Complexity: O(P) where P is the number of max proposals - /// DB Weight: - /// * Reads: Proposals - /// * Writes: Voting, Proposals, ProposalOf - /// # - #[weight = T::WeightInfo::disapprove_proposal(T::MaxProposals::get())] - fn disapprove_proposal(origin, proposal_hash: T::Hash) -> DispatchResultWithPostInfo { + #[pallet::weight( + T::WeightInfo::disapprove_proposal(T::MaxProposals::get()) + )] + fn disapprove_proposal( + origin: OriginFor, + proposal_hash: T::Hash + ) -> DispatchResultWithPostInfo { ensure_root(origin)?; let proposal_count = Self::do_disapprove_proposal(proposal_hash); Ok(Some(T::WeightInfo::disapprove_proposal(proposal_count)).into()) } } + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + #[pallet::metadata(T::AccountId = "AccountId", T::Hash = "Hash")] + pub enum Event, I: 'static = ()> { + /// A motion (given hash) has been proposed (by given account) with a threshold (given + /// `MemberCount`). + /// \[account, proposal_index, proposal_hash, threshold\] + Proposed(T::AccountId, ProposalIndex, T::Hash, MemberCount), + /// A motion (given hash) has been voted on by given account, leaving + /// a tally (yes votes and no votes given respectively as `MemberCount`). + /// \[account, proposal_hash, voted, yes, no\] + Voted(T::AccountId, T::Hash, bool, MemberCount, MemberCount), + /// A motion was approved by the required threshold. + /// \[proposal_hash\] + Approved(T::Hash), + /// A motion was not approved by the required threshold. + /// \[proposal_hash\] + Disapproved(T::Hash), + /// A motion was executed; result will be `Ok` if it returned without error. + /// \[proposal_hash, result\] + Executed(T::Hash, DispatchResult), + /// A single member did some action; result will be `Ok` if it returned without error. + /// \[proposal_hash, result\] + MemberExecuted(T::Hash, DispatchResult), + /// A proposal was closed because its threshold was reached or after its duration was up. + /// \[proposal_hash, yes, no\] + Closed(T::Hash, MemberCount, MemberCount), + } + + /// Old name generated by `decl_event`. + #[deprecated(note = "use `Event` instead")] + pub type RawEvent = Event; + + #[pallet::error] + pub enum Error { + /// Account is not a member + NotMember, + /// Duplicate proposals not allowed + DuplicateProposal, + /// Proposal must exist + ProposalMissing, + /// Mismatched index + WrongIndex, + /// Duplicate vote ignored + DuplicateVote, + /// Members are already initialized! + AlreadyInitialized, + /// The close call was made too early, before the end of the voting. + TooEarly, + /// There can only be a maximum of `MaxProposals` active proposals. + TooManyProposals, + /// The given weight bound for the proposal was too low. + WrongProposalWeight, + /// The given length bound for the proposal was too low. + WrongProposalLength, + } + + /// The hashes of the active proposals. + #[pallet::storage] + #[pallet::getter(fn proposals)] + pub type Proposals, I: 'static = ()> = StorageValue<_, Vec, ValueQuery>; + + /// Actual proposal for a given hash, if it's current. + #[pallet::storage] + #[pallet::getter(fn proposal_of)] + pub type ProposalOf, I: 'static = ()> = StorageMap< + _, + Identity, + T::Hash, + Option, + ValueQuery + >; + + /// Votes on a given proposal, if it is ongoing. + #[pallet::storage] + #[pallet::getter(fn voting)] + pub type Voting, I: 'static = ()> = StorageMap< + _, + Identity, + T::Hash, + Option>, + ValueQuery + >; + + /// Proposals so far. + #[pallet::storage] + #[pallet::getter(fn proposal_count)] + pub type ProposalCount, I: 'static = ()> = StorageValue<_, u32, ValueQuery>; + + /// The current members of the collective. This is stored sorted (just by value). + #[pallet::storage] + #[pallet::getter(fn members)] + pub type Members, I: 'static = ()> = StorageValue<_, Vec, ValueQuery>; + + /// The prime member that helps determine the default vote behavior in case of absentations. + #[pallet::storage] + #[pallet::getter(fn prime)] + pub type Prime, I: 'static = ()> = StorageValue<_, Option, ValueQuery>; + + #[pallet::genesis_config] + pub struct GenesisConfig, I: 'static = ()>{ + pub members: Vec, + // TODO :: Uglly fix, have to recheck + _inner: sp_std::marker::PhantomData<(T, I)>, + } + + #[cfg(feature = "std")] + impl, I: 'static> Default for GenesisConfig { + fn default() -> Self { + Self { + members: Default::default(), + _inner: Default::default(), + } + } + } + + #[pallet::genesis_build] + impl, I: 'static> GenesisBuild for GenesisConfig { + fn build(&self) { + Pallet::::initialize_members(&self.members); + } + } + +} + +#[cfg(feature = "std")] +impl, I: 'static> GenesisConfig { + /// Direct implementation of `GenesisBuild::build_storage`. + /// + /// Kept in order not to break dependency. + pub fn build_storage(&self) -> Result { + >::build_storage(self) + } + + /// Direct implementation of `GenesisBuild::assimilate_storage`. + /// + /// Kept in order not to break dependency. + pub fn assimilate_storage( + &self, + storage: &mut sp_runtime::Storage + ) -> Result<(), String> { + >::assimilate_storage(self, storage) + } } -impl, I: Instance> Module { +/// Simple index type for proposal counting. +pub type ProposalIndex = u32; + +/// A number of members. +/// +/// This also serves as a number of voting members, and since for motions, each member may +/// vote exactly once, therefore also the number of votes for any given motion. +pub type MemberCount = u32; + +/// Default voting strategy when a member is inactive. +pub trait DefaultVote { + /// Get the default voting strategy, given: + /// + /// - Whether the prime member voted Aye. + /// - Raw number of yes votes. + /// - Raw number of no votes. + /// - Total number of member count. + fn default_vote( + prime_vote: Option, + yes_votes: MemberCount, + no_votes: MemberCount, + len: MemberCount, + ) -> bool; +} + +/// Set the prime member's vote as the default vote. +pub struct PrimeDefaultVote; + +impl DefaultVote for PrimeDefaultVote { + fn default_vote( + prime_vote: Option, + _yes_votes: MemberCount, + _no_votes: MemberCount, + _len: MemberCount, + ) -> bool { + prime_vote.unwrap_or(false) + } +} + +/// First see if yes vote are over majority of the whole collective. If so, set the default vote +/// as yes. Otherwise, use the prime meber's vote as the default vote. +pub struct MoreThanMajorityThenPrimeDefaultVote; + +impl DefaultVote for MoreThanMajorityThenPrimeDefaultVote { + fn default_vote( + prime_vote: Option, + yes_votes: MemberCount, + _no_votes: MemberCount, + len: MemberCount, + ) -> bool { + let more_than_majority = yes_votes * 2 > len; + more_than_majority || prime_vote.unwrap_or(false) + } +} + +/// Origin for the collective module. +#[derive(PartialEq, Eq, Clone, RuntimeDebug, Encode, Decode)] +pub enum RawOrigin { + /// It has been condoned by a given number of members of the collective from a given total. + Members(MemberCount, MemberCount), + /// It has been condoned by a single member of the collective. + Member(AccountId), + /// Dummy to manage the fact we have instancing. + _Phantom(sp_std::marker::PhantomData), +} + +/// Origin for the collective module. +pub type Origin = RawOrigin<::AccountId, I>; + +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)] +/// Info for keeping track of a motion being voted on. +pub struct Votes { + /// The proposal's unique index. + index: ProposalIndex, + /// The number of approval votes that are needed to pass the motion. + threshold: MemberCount, + /// The current set of voters that approved it. + ayes: Vec, + /// The current set of voters that rejected it. + nays: Vec, + /// The hard end time of this vote. + end: BlockNumber, +} + +/// Return the weight of a dispatch call result as an `Option`. +/// +/// Will return the weight regardless of what the state of the result is. +fn get_result_weight(result: DispatchResultWithPostInfo) -> Option { + match result { + Ok(post_info) => post_info.actual_weight, + Err(err) => err.post_info.actual_weight, + } +} + +impl, I: 'static> Pallet { /// Check whether `who` is a member of the collective. pub fn is_member(who: &T::AccountId) -> bool { // Note: The dispatchables *do not* use this to check membership so make sure @@ -753,13 +791,13 @@ impl, I: Instance> Module { proposal_hash: T::Hash, proposal: >::Proposal, ) -> (Weight, u32) { - Self::deposit_event(RawEvent::Approved(proposal_hash)); + Self::deposit_event(Event::Approved(proposal_hash)); let dispatch_weight = proposal.get_dispatch_info().weight; let origin = RawOrigin::Members(voting.threshold, seats).into(); let result = proposal.dispatch(origin); Self::deposit_event( - RawEvent::Executed(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) + Event::Executed(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) ); // default to the dispatch info weight for safety let proposal_weight = get_result_weight(result).unwrap_or(dispatch_weight); // P1 @@ -770,7 +808,7 @@ impl, I: Instance> Module { fn do_disapprove_proposal(proposal_hash: T::Hash) -> u32 { // disapproved - Self::deposit_event(RawEvent::Disapproved(proposal_hash)); + Self::deposit_event(Event::Disapproved(proposal_hash)); Self::remove_proposal(proposal_hash) } @@ -787,7 +825,7 @@ impl, I: Instance> Module { } } -impl, I: Instance> ChangeMembers for Module { +impl, I: 'static> ChangeMembers for Pallet { /// Update the members of the collective. Votes are updated and the prime is reset. /// /// NOTE: Does not enforce the expected `MaxMembers` limit on the amount of members, but @@ -846,7 +884,7 @@ impl, I: Instance> ChangeMembers for Module { } } -impl, I: Instance> InitializeMembers for Module { +impl, I: 'static> InitializeMembers for Pallet { fn initialize_members(members: &[T::AccountId]) { if !members.is_empty() { assert!(>::get().is_empty(), "Members are already initialized!"); @@ -868,7 +906,7 @@ where } } -pub struct EnsureMember(sp_std::marker::PhantomData<(AccountId, I)>); +pub struct EnsureMember(sp_std::marker::PhantomData<(AccountId, I)>); impl< O: Into, O>> + From>, AccountId: Default, @@ -888,7 +926,7 @@ impl< } } -pub struct EnsureMembers(sp_std::marker::PhantomData<(N, AccountId, I)>); +pub struct EnsureMembers(sp_std::marker::PhantomData<(N, AccountId, I)>); impl< O: Into, O>> + From>, N: U32, @@ -909,7 +947,7 @@ impl< } } -pub struct EnsureProportionMoreThan( +pub struct EnsureProportionMoreThan( sp_std::marker::PhantomData<(N, D, AccountId, I)> ); impl< @@ -933,7 +971,7 @@ impl< } } -pub struct EnsureProportionAtLeast( +pub struct EnsureProportionAtLeast( sp_std::marker::PhantomData<(N, D, AccountId, I)> ); impl< @@ -1043,6 +1081,7 @@ mod tests { UncheckedExtrinsic = UncheckedExtrinsic { System: system::{Module, Call, Event}, + // Collective: collective::::{Module, Call, Event, Config}, Collective: collective::::{Module, Call, Event, Origin, Config}, CollectiveMajority: collective::::{Module, Call, Event, Origin, Config}, DefaultCollective: collective::{Module, Call, Event, Origin, Config}, @@ -1076,657 +1115,4 @@ mod tests { assert_eq!(Collective::proposals(), Vec::::new()); }); } - - #[test] - fn close_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(3); - assert_noop!( - Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len), - Error::::TooEarly - ); - - System::set_block_number(4); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), - record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) - ]); - }); - } - - #[test] - fn proposal_weight_limit_works_on_approve() { - new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - // Set 1 as prime voter - Prime::::set(Some(1)); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - // With 1's prime vote, this should pass - System::set_block_number(4); - assert_noop!( - Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len), - Error::::WrongProposalWeight - ); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - }) - } - - #[test] - fn proposal_weight_limit_ignored_on_disapprove() { - new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - // No votes, this proposal wont pass - System::set_block_number(4); - assert_ok!( - Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len) - ); - }) - } - - #[test] - fn close_with_prime_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(3), MaxMembers::get())); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), - record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) - ]); - }); - } - - #[test] - fn close_with_voting_prime_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(1), MaxMembers::get())); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 3, 0))), - record(Event::collective_Instance1(RawEvent::Approved(hash.clone()))), - record(Event::collective_Instance1(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) - ]); - }); - } - - #[test] - fn close_with_no_prime_but_majority_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(CollectiveMajority::set_members(Origin::root(), vec![1, 2, 3, 4, 5], Some(5), MaxMembers::get())); - - assert_ok!(CollectiveMajority::propose(Origin::signed(1), 5, Box::new(proposal.clone()), proposal_len)); - assert_ok!(CollectiveMajority::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_ok!(CollectiveMajority::vote(Origin::signed(3), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(CollectiveMajority::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance2(RawEvent::Proposed(1, 0, hash.clone(), 5))), - record(Event::collective_Instance2(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance2(RawEvent::Voted(3, hash.clone(), true, 3, 0))), - record(Event::collective_Instance2(RawEvent::Closed(hash.clone(), 5, 0))), - record(Event::collective_Instance2(RawEvent::Approved(hash.clone()))), - record(Event::collective_Instance2(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) - ]); - }); - } - - #[test] - fn removal_of_old_voters_votes_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) - ); - Collective::change_members_sorted(&[4], &[1], &[2, 3, 4]); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) - ); - - let proposal = make_proposal(69); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) - ); - Collective::change_members_sorted(&[], &[3], &[2, 4]); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) - ); - }); - } - - #[test] - fn removal_of_old_voters_votes_works_with_set_members() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) - ); - assert_ok!(Collective::set_members(Origin::root(), vec![2, 3, 4], None, MaxMembers::get())); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) - ); - - let proposal = make_proposal(69); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) - ); - assert_ok!(Collective::set_members(Origin::root(), vec![2, 4], None, MaxMembers::get())); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) - ); - }); - } - - #[test] - fn propose_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = proposal.blake2_256().into(); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_eq!(Collective::proposals(), vec![hash]); - assert_eq!(Collective::proposal_of(&hash), Some(proposal)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![1], nays: vec![], end }) - ); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 3, - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn limit_active_proposals() { - new_test_ext().execute_with(|| { - for i in 0..MaxProposals::get() { - let proposal = make_proposal(i as u64); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - } - let proposal = make_proposal(MaxProposals::get() as u64 + 1); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_noop!( - Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len), - Error::::TooManyProposals - ); - }) - } - - #[test] - fn correct_validate_and_get_proposal() { - new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let length = proposal.encode().len() as u32; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), length)); - - let hash = BlakeTwo256::hash_of(&proposal); - let weight = proposal.get_dispatch_info().weight; - assert_noop!( - Collective::validate_and_get_proposal(&BlakeTwo256::hash_of(&vec![3; 4]), length, weight), - Error::::ProposalMissing - ); - assert_noop!( - Collective::validate_and_get_proposal(&hash, length - 2, weight), - Error::::WrongProposalLength - ); - assert_noop!( - Collective::validate_and_get_proposal(&hash, length, weight - 10), - Error::::WrongProposalWeight - ); - let res = Collective::validate_and_get_proposal(&hash, length, weight); - assert_ok!(res.clone()); - let (retrieved_proposal, len) = res.unwrap(); - assert_eq!(length as usize, len); - assert_eq!(proposal, retrieved_proposal); - }) - } - - #[test] - fn motions_ignoring_non_collective_proposals_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_noop!( - Collective::propose(Origin::signed(42), 3, Box::new(proposal.clone()), proposal_len), - Error::::NotMember - ); - }); - } - - #[test] - fn motions_ignoring_non_collective_votes_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_noop!( - Collective::vote(Origin::signed(42), hash.clone(), 0, true), - Error::::NotMember, - ); - }); - } - - #[test] - fn motions_ignoring_bad_index_collective_vote_works() { - new_test_ext().execute_with(|| { - System::set_block_number(3); - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_noop!( - Collective::vote(Origin::signed(2), hash.clone(), 1, true), - Error::::WrongIndex, - ); - }); - } - - #[test] - fn motions_revoting_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) - ); - assert_noop!( - Collective::vote(Origin::signed(1), hash.clone(), 0, true), - Error::::DuplicateVote, - ); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![1], end }) - ); - assert_noop!( - Collective::vote(Origin::signed(1), hash.clone(), 0, false), - Error::::DuplicateVote, - ); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 2, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Voted( - 1, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - false, - 0, - 1, - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn motions_all_first_vote_free_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - let end = 4; - assert_ok!( - Collective::propose( - Origin::signed(1), - 2, - Box::new(proposal.clone()), - proposal_len, - ) - ); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) - ); - - // For the motion, acc 2's first vote, expecting Ok with Pays::No. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(2), - hash.clone(), - 0, - true, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); - - // Duplicate vote, expecting error with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(2), - hash.clone(), - 0, - true, - ); - assert_eq!(vote_rval.unwrap_err().post_info.pays_fee, Pays::Yes); - - // Modifying vote, expecting ok with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(2), - hash.clone(), - 0, - false, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); - - // For the motion, acc 3's first vote, expecting Ok with Pays::No. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(3), - hash.clone(), - 0, - true, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); - - // acc 3 modify the vote, expecting Ok with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(3), - hash.clone(), - 0, - false, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); - - // Test close() Extrincis | Check DispatchResultWithPostInfo with Pay Info - - let proposal_weight = proposal.get_dispatch_info().weight; - let close_rval: DispatchResultWithPostInfo = Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len, - ); - assert_eq!(close_rval.unwrap().pays_fee, Pays::No); - - // trying to close the proposal, which is already closed. - // Expecting error "ProposalMissing" with Pays::Yes - let close_rval: DispatchResultWithPostInfo = Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len, - ); - assert_eq!(close_rval.unwrap_err().post_info.pays_fee, Pays::Yes); - }); - } - - #[test] - fn motions_reproposing_disapproved_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); - assert_eq!(Collective::proposals(), vec![]); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - assert_eq!(Collective::proposals(), vec![hash]); - }); - } - - #[test] - fn motions_disapproval_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1( - RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 3, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Voted( - 2, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - false, - 1, - 1, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Closed( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 1, 1, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Disapproved( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn motions_approval_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 2, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Voted( - 2, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - true, - 2, - 0, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Closed( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 2, 0, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Approved( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Executed( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - Err(DispatchError::BadOrigin), - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn close_disapprove_does_not_care_about_weight_or_len() { - // This test confirms that if you close a proposal that would be disapproved, - // we do not care about the proposal length or proposal weight since it will - // not be read from storage or executed. - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - // First we make the proposal succeed - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - // It will not close with bad weight/len information - assert_noop!( - Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0), - Error::::WrongProposalLength, - ); - assert_noop!( - Collective::close(Origin::signed(2), hash.clone(), 0, 0, proposal_len), - Error::::WrongProposalWeight, - ); - // Now we make the proposal fail - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - // It can close even if the weight/len information is bad - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0)); - }) - } - - #[test] - fn disapprove_proposal_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - // Proposal would normally succeed - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - // But Root can disapprove and remove it anyway - assert_ok!(Collective::disapprove_proposal(Origin::root(), hash.clone())); - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 2))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))), - ]); - }) - } -} +} \ No newline at end of file From 6334f1a3ff4d8fe48572ea21d2d0dae879455647 Mon Sep 17 00:00:00 2001 From: shamb0 Date: Thu, 18 Feb 2021 22:11:09 +0530 Subject: [PATCH 02/10] port to frame v2 | unit test up --- frame/collective/src/lib.rs | 677 +++++++++++++++++++++++++++++++++++- 1 file changed, 666 insertions(+), 11 deletions(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 30c8743723b61..1927e3236881a 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -119,6 +119,10 @@ pub mod pallet { #[pallet::generate_store(pub(super) trait Store)] pub struct Pallet(PhantomData<(T, I)>); + /// Origin for the collective module. + #[pallet::origin] + pub type Origin = RawOrigin<::AccountId, I>; + #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { } @@ -143,7 +147,7 @@ pub mod pallet { new_members.len() as u32, // N T::MaxProposals::get() // P ))] - fn set_members( + pub fn set_members( origin: OriginFor, new_members: Vec, prime: Option, @@ -187,7 +191,7 @@ pub mod pallet { T::MaxMembers::get(), // M ) )] - fn execute( + pub fn execute( origin: OriginFor, proposal: Box<>::Proposal>, #[pallet::compact] length_bound: u32, @@ -253,7 +257,7 @@ pub mod pallet { ) } )] - fn propose( + pub fn propose( origin: OriginFor, #[pallet::compact] threshold: MemberCount, proposal: Box, @@ -316,7 +320,7 @@ pub mod pallet { /// Transaction fees will be waived if the member is voting on any particular proposal /// for the first time and the call is successful. Subsequent vote changes will charge a fee. #[pallet::weight(T::WeightInfo::vote(T::MaxMembers::get()))] - fn vote( + pub fn vote( origin: OriginFor, proposal: T::Hash, #[pallet::compact] index: ProposalIndex, @@ -404,7 +408,7 @@ pub mod pallet { .saturating_add(p1) } )] - fn close( + pub fn close( origin: OriginFor, proposal_hash: T::Hash, #[pallet::compact] index: ProposalIndex, @@ -494,7 +498,7 @@ pub mod pallet { #[pallet::weight( T::WeightInfo::disapprove_proposal(T::MaxProposals::get()) )] - fn disapprove_proposal( + pub fn disapprove_proposal( origin: OriginFor, proposal_hash: T::Hash ) -> DispatchResultWithPostInfo { @@ -606,8 +610,7 @@ pub mod pallet { #[pallet::genesis_config] pub struct GenesisConfig, I: 'static = ()>{ pub members: Vec, - // TODO :: Uglly fix, have to recheck - _inner: sp_std::marker::PhantomData<(T, I)>, + pub phantom: sp_std::marker::PhantomData<(T, I)>, } #[cfg(feature = "std")] @@ -615,7 +618,7 @@ pub mod pallet { fn default() -> Self { Self { members: Default::default(), - _inner: Default::default(), + phantom: Default::default(), } } } @@ -715,8 +718,6 @@ pub enum RawOrigin { _Phantom(sp_std::marker::PhantomData), } -/// Origin for the collective module. -pub type Origin = RawOrigin<::AccountId, I>; #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)] /// Info for keeping track of a motion being voted on. @@ -999,6 +1000,7 @@ impl< mod tests { use super::*; use frame_support::{Hashable, assert_ok, assert_noop, parameter_types}; + use frame_support::{weights::{Pays}}; use frame_system::{self as system, EventRecord, Phase}; use hex_literal::hex; use sp_core::H256; @@ -1115,4 +1117,657 @@ mod tests { assert_eq!(Collective::proposals(), Vec::::new()); }); } + + #[test] + fn close_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(3); + assert_noop!( + Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len), + Error::::TooEarly + ); + + System::set_block_number(4); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), + record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) + ]); + }); + } + + #[test] + fn proposal_weight_limit_works_on_approve() { + new_test_ext().execute_with(|| { + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + // Set 1 as prime voter + Prime::::set(Some(1)); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + // With 1's prime vote, this should pass + System::set_block_number(4); + assert_noop!( + Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len), + Error::::WrongProposalWeight + ); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + }) + } + + #[test] + fn proposal_weight_limit_ignored_on_disapprove() { + new_test_ext().execute_with(|| { + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + // No votes, this proposal wont pass + System::set_block_number(4); + assert_ok!( + Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len) + ); + }) + } + + #[test] + fn close_with_prime_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(3), MaxMembers::get())); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), + record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) + ]); + }); + } + + #[test] + fn close_with_voting_prime_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(1), MaxMembers::get())); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 3, 0))), + record(Event::collective_Instance1(RawEvent::Approved(hash.clone()))), + record(Event::collective_Instance1(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) + ]); + }); + } + + #[test] + fn close_with_no_prime_but_majority_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(CollectiveMajority::set_members(Origin::root(), vec![1, 2, 3, 4, 5], Some(5), MaxMembers::get())); + + assert_ok!(CollectiveMajority::propose(Origin::signed(1), 5, Box::new(proposal.clone()), proposal_len)); + assert_ok!(CollectiveMajority::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(CollectiveMajority::vote(Origin::signed(3), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(CollectiveMajority::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance2(RawEvent::Proposed(1, 0, hash.clone(), 5))), + record(Event::collective_Instance2(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance2(RawEvent::Voted(3, hash.clone(), true, 3, 0))), + record(Event::collective_Instance2(RawEvent::Closed(hash.clone(), 5, 0))), + record(Event::collective_Instance2(RawEvent::Approved(hash.clone()))), + record(Event::collective_Instance2(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) + ]); + }); + } + + #[test] + fn removal_of_old_voters_votes_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) + ); + Collective::change_members_sorted(&[4], &[1], &[2, 3, 4]); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) + ); + + let proposal = make_proposal(69); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) + ); + Collective::change_members_sorted(&[], &[3], &[2, 4]); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) + ); + }); + } + + #[test] + fn removal_of_old_voters_votes_works_with_set_members() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) + ); + assert_ok!(Collective::set_members(Origin::root(), vec![2, 3, 4], None, MaxMembers::get())); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) + ); + + let proposal = make_proposal(69); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) + ); + assert_ok!(Collective::set_members(Origin::root(), vec![2, 4], None, MaxMembers::get())); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) + ); + }); + } + + #[test] + fn propose_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = proposal.blake2_256().into(); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_eq!(Collective::proposals(), vec![hash]); + assert_eq!(Collective::proposal_of(&hash), Some(proposal)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![1], nays: vec![], end }) + ); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 3, + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn limit_active_proposals() { + new_test_ext().execute_with(|| { + for i in 0..MaxProposals::get() { + let proposal = make_proposal(i as u64); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + } + let proposal = make_proposal(MaxProposals::get() as u64 + 1); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_noop!( + Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len), + Error::::TooManyProposals + ); + }) + } + + #[test] + fn correct_validate_and_get_proposal() { + new_test_ext().execute_with(|| { + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let length = proposal.encode().len() as u32; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), length)); + + let hash = BlakeTwo256::hash_of(&proposal); + let weight = proposal.get_dispatch_info().weight; + assert_noop!( + Collective::validate_and_get_proposal(&BlakeTwo256::hash_of(&vec![3; 4]), length, weight), + Error::::ProposalMissing + ); + assert_noop!( + Collective::validate_and_get_proposal(&hash, length - 2, weight), + Error::::WrongProposalLength + ); + assert_noop!( + Collective::validate_and_get_proposal(&hash, length, weight - 10), + Error::::WrongProposalWeight + ); + let res = Collective::validate_and_get_proposal(&hash, length, weight); + assert_ok!(res.clone()); + let (retrieved_proposal, len) = res.unwrap(); + assert_eq!(length as usize, len); + assert_eq!(proposal, retrieved_proposal); + }) + } + + #[test] + fn motions_ignoring_non_collective_proposals_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_noop!( + Collective::propose(Origin::signed(42), 3, Box::new(proposal.clone()), proposal_len), + Error::::NotMember + ); + }); + } + + #[test] + fn motions_ignoring_non_collective_votes_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_noop!( + Collective::vote(Origin::signed(42), hash.clone(), 0, true), + Error::::NotMember, + ); + }); + } + + #[test] + fn motions_ignoring_bad_index_collective_vote_works() { + new_test_ext().execute_with(|| { + System::set_block_number(3); + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_noop!( + Collective::vote(Origin::signed(2), hash.clone(), 1, true), + Error::::WrongIndex, + ); + }); + } + + #[test] + fn motions_revoting_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) + ); + assert_noop!( + Collective::vote(Origin::signed(1), hash.clone(), 0, true), + Error::::DuplicateVote, + ); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![1], end }) + ); + assert_noop!( + Collective::vote(Origin::signed(1), hash.clone(), 0, false), + Error::::DuplicateVote, + ); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 2, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Voted( + 1, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + false, + 0, + 1, + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn motions_all_first_vote_free_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + let end = 4; + assert_ok!( + Collective::propose( + Origin::signed(1), + 2, + Box::new(proposal.clone()), + proposal_len, + ) + ); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) + ); + + // For the motion, acc 2's first vote, expecting Ok with Pays::No. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(2), + hash.clone(), + 0, + true, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); + + // Duplicate vote, expecting error with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(2), + hash.clone(), + 0, + true, + ); + assert_eq!(vote_rval.unwrap_err().post_info.pays_fee, Pays::Yes); + + // Modifying vote, expecting ok with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(2), + hash.clone(), + 0, + false, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); + + // For the motion, acc 3's first vote, expecting Ok with Pays::No. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(3), + hash.clone(), + 0, + true, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); + + // acc 3 modify the vote, expecting Ok with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(3), + hash.clone(), + 0, + false, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); + + // Test close() Extrincis | Check DispatchResultWithPostInfo with Pay Info + + let proposal_weight = proposal.get_dispatch_info().weight; + let close_rval: DispatchResultWithPostInfo = Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len, + ); + assert_eq!(close_rval.unwrap().pays_fee, Pays::No); + + // trying to close the proposal, which is already closed. + // Expecting error "ProposalMissing" with Pays::Yes + let close_rval: DispatchResultWithPostInfo = Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len, + ); + assert_eq!(close_rval.unwrap_err().post_info.pays_fee, Pays::Yes); + }); + } + + #[test] + fn motions_reproposing_disapproved_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); + assert_eq!(Collective::proposals(), vec![]); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + assert_eq!(Collective::proposals(), vec![hash]); + }); + } + + #[test] + fn motions_disapproval_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1( + RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 3, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Voted( + 2, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + false, + 1, + 1, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Closed( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 1, 1, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Disapproved( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn motions_approval_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 2, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Voted( + 2, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + true, + 2, + 0, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Closed( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 2, 0, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Approved( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Executed( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + Err(DispatchError::BadOrigin), + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn close_disapprove_does_not_care_about_weight_or_len() { + // This test confirms that if you close a proposal that would be disapproved, + // we do not care about the proposal length or proposal weight since it will + // not be read from storage or executed. + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + // First we make the proposal succeed + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + // It will not close with bad weight/len information + assert_noop!( + Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0), + Error::::WrongProposalLength, + ); + assert_noop!( + Collective::close(Origin::signed(2), hash.clone(), 0, 0, proposal_len), + Error::::WrongProposalWeight, + ); + // Now we make the proposal fail + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + // It can close even if the weight/len information is bad + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0)); + }) + } + + #[test] + fn disapprove_proposal_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + // Proposal would normally succeed + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + // But Root can disapprove and remove it anyway + assert_ok!(Collective::disapprove_proposal(Origin::root(), hash.clone())); + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 2))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))), + ]); + }) + } } \ No newline at end of file From a464af69e02c2077697eade7ca5c79dcab47077d Mon Sep 17 00:00:00 2001 From: shamb0 Date: Fri, 19 Feb 2021 22:52:26 +0530 Subject: [PATCH 03/10] port to frame v2 | unit test up --- frame/collective/src/lib.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 1927e3236881a..340b2c9dc97e4 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -48,6 +48,7 @@ // mod benchmarking; pub mod weights; +use sp_std::if_std; use sp_std::{prelude::*, result}; use sp_core::u32_trait::Value as U32; use sp_io::storage; @@ -765,7 +766,15 @@ impl, I: 'static> Pallet { // read the length of the proposal storage entry directly let proposal_len = storage::read(&key, &mut [0; 0], 0) .ok_or(Error::::ProposalMissing)?; + + // TODO :: Why len - 1 ? Why 1 byte extra from storage::read() ?. + let proposal_len = proposal_len - 1; ensure!(proposal_len <= length_bound, Error::::WrongProposalLength); + + if_std!{ + println!("Valid prop_len :: {:#?}", proposal_len); + } + let proposal = ProposalOf::::get(hash).ok_or(Error::::ProposalMissing)?; let proposal_weight = proposal.get_dispatch_info().weight; ensure!(proposal_weight <= weight_bound, Error::::WrongProposalWeight); @@ -1107,7 +1116,12 @@ mod tests { } fn make_proposal(value: u64) -> Call { - Call::System(frame_system::Call::remark(value.encode())) + let rval = Call::System(frame_system::Call::remark(value.encode())); + let proposal_len: u32 = rval.using_encoded(|p| p.len() as u32); + if_std!{ + println!("prop_len :: {:#?}", proposal_len); + } + return rval } #[test] From 647460715d9873b36f0801a0556ad1f50bfb2464 Mon Sep 17 00:00:00 2001 From: shamb0 Date: Sat, 20 Feb 2021 18:28:04 +0530 Subject: [PATCH 04/10] wk2107 | bringup | unit test & benchmarking --- frame/collective/src/benchmarking.rs | 31 +- frame/collective/src/lib.rs | 982 ++------------------------- frame/collective/src/mock.rs | 174 +++++ frame/collective/src/tests.rs | 689 +++++++++++++++++++ 4 files changed, 941 insertions(+), 935 deletions(-) create mode 100644 frame/collective/src/mock.rs create mode 100644 frame/collective/src/tests.rs diff --git a/frame/collective/src/benchmarking.rs b/frame/collective/src/benchmarking.rs index 1afdd14b1ad38..1ca4856c895e7 100644 --- a/frame/collective/src/benchmarking.rs +++ b/frame/collective/src/benchmarking.rs @@ -17,12 +17,14 @@ //! Staking pallet benchmarking. +#![cfg(feature = "runtime-benchmarks")] + use super::*; use frame_system::RawOrigin as SystemOrigin; use frame_system::EventRecord; use frame_benchmarking::{ - benchmarks_instance, + benchmarks, account, whitelisted_caller, impl_benchmark_test_suite, @@ -35,10 +37,9 @@ use frame_system::Module as System; use crate::Module as Collective; const SEED: u32 = 0; - const MAX_BYTES: u32 = 1_024; -fn assert_last_event, I: Instance>(generic_event: >::Event) { +fn assert_last_event, I: 'static>(generic_event: >::Event) { let events = System::::events(); let system_event: ::Event = generic_event.into(); // compare to the last event record @@ -46,7 +47,7 @@ fn assert_last_event, I: Instance>(generic_event: >: assert_eq!(event, &system_event); } -benchmarks_instance! { +benchmarks! { set_members { let m in 1 .. T::MaxMembers::get(); let n in 1 .. T::MaxMembers::get(); @@ -137,7 +138,7 @@ benchmarks_instance! { verify { let proposal_hash = T::Hashing::hash_of(&proposal); // Note that execution fails due to mis-matched origin - assert_last_event::( + assert_last_event::( RawEvent::MemberExecuted(proposal_hash, Err(DispatchError::BadOrigin)).into() ); } @@ -168,7 +169,7 @@ benchmarks_instance! { verify { let proposal_hash = T::Hashing::hash_of(&proposal); // Note that execution fails due to mis-matched origin - assert_last_event::( + assert_last_event::( RawEvent::Executed(proposal_hash, Err(DispatchError::BadOrigin)).into() ); } @@ -213,7 +214,7 @@ benchmarks_instance! { // New proposal is recorded assert_eq!(Collective::::proposals().len(), p as usize); let proposal_hash = T::Hashing::hash_of(&proposal); - assert_last_event::(RawEvent::Proposed(caller, p - 1, proposal_hash, threshold).into()); + assert_last_event::(RawEvent::Proposed(caller, p - 1, proposal_hash, threshold).into()); } vote { @@ -287,7 +288,7 @@ benchmarks_instance! { verify { // All proposals exist and the last proposal has just been updated. assert_eq!(Collective::::proposals().len(), p as usize); - let voting = Collective::::voting(&last_hash).ok_or(Error::::ProposalMissing)?; + let voting = Collective::::voting(&last_hash).ok_or(Error::::ProposalMissing)?; assert_eq!(voting.ayes.len(), (m - 3) as usize); assert_eq!(voting.nays.len(), 1); } @@ -369,7 +370,7 @@ benchmarks_instance! { verify { // The last proposal is removed. assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Disapproved(last_hash).into()); + assert_last_event::(RawEvent::Disapproved(last_hash).into()); } close_early_approved { @@ -450,7 +451,7 @@ benchmarks_instance! { verify { // The last proposal is removed. assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Executed(last_hash, Err(DispatchError::BadOrigin)).into()); + assert_last_event::(RawEvent::Executed(last_hash, Err(DispatchError::BadOrigin)).into()); } close_disapproved { @@ -522,7 +523,7 @@ benchmarks_instance! { }: close(SystemOrigin::Signed(caller), last_hash, index, Weight::max_value(), bytes_in_storage) verify { assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Disapproved(last_hash).into()); + assert_last_event::(RawEvent::Disapproved(last_hash).into()); } close_approved { @@ -586,7 +587,7 @@ benchmarks_instance! { }: close(SystemOrigin::Signed(caller), last_hash, p - 1, Weight::max_value(), bytes_in_storage) verify { assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Executed(last_hash, Err(DispatchError::BadOrigin)).into()); + assert_last_event::(RawEvent::Executed(last_hash, Err(DispatchError::BadOrigin)).into()); } disapprove_proposal { @@ -634,12 +635,12 @@ benchmarks_instance! { }: _(SystemOrigin::Root, last_hash) verify { assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Disapproved(last_hash).into()); + assert_last_event::(RawEvent::Disapproved(last_hash).into()); } } impl_benchmark_test_suite!( Collective, - crate::tests::new_test_ext(), - crate::tests::Test, + crate::mock::new_test_ext(), + crate::mock::Test, ); diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 340b2c9dc97e4..a8c74241693ef 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -43,12 +43,11 @@ #![recursion_limit = "128"] #[macro_use] -// mod tests; -// mod mock; -// mod benchmarking; +mod tests; +mod mock; +mod benchmarking; pub mod weights; -use sp_std::if_std; use sp_std::{prelude::*, result}; use sp_core::u32_trait::Value as U32; use sp_io::storage; @@ -66,15 +65,38 @@ use frame_support::{ #[cfg(feature = "std")] use frame_support::traits::GenesisBuild; -// use sp_runtime::{RuntimeDebug, traits::Hash}; -use sp_runtime::{ RuntimeDebug, traits::{ - Hash, -}}; +use sp_runtime::{ RuntimeDebug, traits::{Hash}}; use frame_system::{self as system}; + pub use weights::WeightInfo; pub use pallet::*; +/// Simple index type for proposal counting. +pub type ProposalIndex = u32; + +/// A number of members. +/// +/// This also serves as a number of voting members, and since for motions, each member may +/// vote exactly once, therefore also the number of votes for any given motion. +pub type MemberCount = u32; + +/// Default voting strategy when a member is inactive. +pub trait DefaultVote { + /// Get the default voting strategy, given: + /// + /// - Whether the prime member voted Aye. + /// - Raw number of yes votes. + /// - Raw number of no votes. + /// - Total number of member count. + fn default_vote( + prime_vote: Option, + yes_votes: MemberCount, + no_votes: MemberCount, + len: MemberCount, + ) -> bool; +} + #[frame_support::pallet] pub mod pallet { use frame_support::pallet_prelude::*; @@ -97,9 +119,11 @@ pub mod pallet { type Event: From> + IsType<::Event>; /// The time-out for council motions. + #[pallet::constant] type MotionDuration: Get; /// Maximum number of proposals allowed to be active in parallel. + #[pallet::constant] type MaxProposals: Get; /// The maximum number of members supported by the pallet. Used for weight estimation. @@ -107,6 +131,7 @@ pub mod pallet { /// NOTE: /// + Benchmarks will need to be re-run and weights adjusted if this changes. /// + This pallet assumes that dependents keep to the limit without enforcing it. + #[pallet::constant] type MaxMembers: Get; /// Default vote strategy of this collective. @@ -157,7 +182,7 @@ pub mod pallet { ensure_root(origin)?; if new_members.len() > T::MaxMembers::get() as usize { debug::error!( - "New members count exceeds maximum amount of members expected. (expected: {}, actual: {})", + "New members count exceeds MAX members expected.(Expct: {}, Act: {})", T::MaxMembers::get(), new_members.len() ); @@ -166,7 +191,7 @@ pub mod pallet { let old = Members::::get(); if old.len() > old_count as usize { debug::warn!( - "Wrong count used to estimate set_members weight. (expected: {}, actual: {})", + "Wrong count used to estimate set_members weight.(Expct: {}, Act: {})", old_count, old.len() ); @@ -209,7 +234,7 @@ pub mod pallet { Event::MemberExecuted(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) ); - Ok(get_result_weight(result).map(|w| { + Ok(Self::get_result_weight(result).map(|w| { T::WeightInfo::execute( proposal_len as u32, // B members.len() as u32, // M @@ -224,26 +249,6 @@ pub mod pallet { /// `threshold` determines whether `proposal` is executed directly (`threshold < 2`) /// or put up for voting. /// - /// # - /// ## Weight - /// - `O(B + M + P1)` or `O(B + M + P2)` where: - /// - `B` is `proposal` size in bytes (length-fee-bounded) - /// - `M` is members-count (code- and governance-bounded) - /// - branching is influenced by `threshold` where: - /// - `P1` is proposal execution complexity (`threshold < 2`) - /// - `P2` is proposals-count (code-bounded) (`threshold >= 2`) - /// - DB: - /// - 1 storage read `is_member` (codec `O(M)`) - /// - 1 storage read `ProposalOf::contains_key` (codec `O(1)`) - /// - DB accesses influenced by `threshold`: - /// - EITHER storage accesses done by `proposal` (`threshold < 2`) - /// - OR proposal insertion (`threshold <= 2`) - /// - 1 storage mutation `Proposals` (codec `O(P2)`) - /// - 1 storage mutation `ProposalCount` (codec `O(1)`) - /// - 1 storage write `ProposalOf` (codec `O(B)`) - /// - 1 storage write `Voting` (codec `O(M)`) - /// - 1 event - /// # #[pallet::weight( if *threshold < 2 { T::WeightInfo::propose_execute( @@ -271,7 +276,11 @@ pub mod pallet { let proposal_len = proposal.using_encoded(|x| x.len()); ensure!(proposal_len <= length_bound as usize, >::WrongProposalLength); let proposal_hash = T::Hashing::hash_of(&proposal); - ensure!(!>::contains_key(proposal_hash), >::DuplicateProposal); + + ensure!( + !>::contains_key(proposal_hash), + >::DuplicateProposal + ); if threshold < 2 { let seats = Self::members().len() as MemberCount; @@ -280,7 +289,7 @@ pub mod pallet { Event::Executed(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) ); - Ok(get_result_weight(result).map(|w| { + Ok(Self::get_result_weight(result).map(|w| { T::WeightInfo::propose_execute( proposal_len as u32, // B members.len() as u32, // M @@ -299,7 +308,7 @@ pub mod pallet { let index = Self::proposal_count(); >::mutate(|i| *i += 1); - >::insert(proposal_hash, Some(*proposal)); + >::insert(proposal_hash, *proposal); let end = system::Module::::block_number() + T::MotionDuration::get(); let votes = Votes { index, threshold, ayes: vec![who.clone()], nays: vec![], end }; >::insert(proposal_hash, Some(votes)); @@ -490,7 +499,8 @@ pub mod pallet { } } - /// Disapprove a proposal, close, and remove it from the system, regardless of its current state. + /// Disapprove a proposal, close, and remove it from the system, + /// regardless of its current state. /// /// Must be called by the Root origin. /// @@ -539,7 +549,8 @@ pub mod pallet { } /// Old name generated by `decl_event`. - #[deprecated(note = "use `Event` instead")] + // TODO :: Have to re-check is "Note message" required ? + // #[deprecated(note = "use `Event` instead")] pub type RawEvent = Event; #[pallet::error] @@ -578,8 +589,8 @@ pub mod pallet { _, Identity, T::Hash, - Option, - ValueQuery + T::Proposal, + OptionQuery, >; /// Votes on a given proposal, if it is ongoing. @@ -630,7 +641,6 @@ pub mod pallet { Pallet::::initialize_members(&self.members); } } - } #[cfg(feature = "std")] @@ -653,61 +663,6 @@ impl, I: 'static> GenesisConfig { } } -/// Simple index type for proposal counting. -pub type ProposalIndex = u32; - -/// A number of members. -/// -/// This also serves as a number of voting members, and since for motions, each member may -/// vote exactly once, therefore also the number of votes for any given motion. -pub type MemberCount = u32; - -/// Default voting strategy when a member is inactive. -pub trait DefaultVote { - /// Get the default voting strategy, given: - /// - /// - Whether the prime member voted Aye. - /// - Raw number of yes votes. - /// - Raw number of no votes. - /// - Total number of member count. - fn default_vote( - prime_vote: Option, - yes_votes: MemberCount, - no_votes: MemberCount, - len: MemberCount, - ) -> bool; -} - -/// Set the prime member's vote as the default vote. -pub struct PrimeDefaultVote; - -impl DefaultVote for PrimeDefaultVote { - fn default_vote( - prime_vote: Option, - _yes_votes: MemberCount, - _no_votes: MemberCount, - _len: MemberCount, - ) -> bool { - prime_vote.unwrap_or(false) - } -} - -/// First see if yes vote are over majority of the whole collective. If so, set the default vote -/// as yes. Otherwise, use the prime meber's vote as the default vote. -pub struct MoreThanMajorityThenPrimeDefaultVote; - -impl DefaultVote for MoreThanMajorityThenPrimeDefaultVote { - fn default_vote( - prime_vote: Option, - yes_votes: MemberCount, - _no_votes: MemberCount, - len: MemberCount, - ) -> bool { - let more_than_majority = yes_votes * 2 > len; - more_than_majority || prime_vote.unwrap_or(false) - } -} - /// Origin for the collective module. #[derive(PartialEq, Eq, Clone, RuntimeDebug, Encode, Decode)] pub enum RawOrigin { @@ -719,7 +674,6 @@ pub enum RawOrigin { _Phantom(sp_std::marker::PhantomData), } - #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)] /// Info for keeping track of a motion being voted on. pub struct Votes { @@ -735,16 +689,6 @@ pub struct Votes { end: BlockNumber, } -/// Return the weight of a dispatch call result as an `Option`. -/// -/// Will return the weight regardless of what the state of the result is. -fn get_result_weight(result: DispatchResultWithPostInfo) -> Option { - match result { - Ok(post_info) => post_info.actual_weight, - Err(err) => err.post_info.actual_weight, - } -} - impl, I: 'static> Pallet { /// Check whether `who` is a member of the collective. pub fn is_member(who: &T::AccountId) -> bool { @@ -767,34 +711,15 @@ impl, I: 'static> Pallet { let proposal_len = storage::read(&key, &mut [0; 0], 0) .ok_or(Error::::ProposalMissing)?; - // TODO :: Why len - 1 ? Why 1 byte extra from storage::read() ?. - let proposal_len = proposal_len - 1; ensure!(proposal_len <= length_bound, Error::::WrongProposalLength); - if_std!{ - println!("Valid prop_len :: {:#?}", proposal_len); - } - let proposal = ProposalOf::::get(hash).ok_or(Error::::ProposalMissing)?; let proposal_weight = proposal.get_dispatch_info().weight; ensure!(proposal_weight <= weight_bound, Error::::WrongProposalWeight); Ok((proposal, proposal_len as usize)) } - /// Weight: - /// If `approved`: - /// - the weight of `proposal` preimage. - /// - two events deposited. - /// - two removals, one mutation. - /// - computation and i/o `O(P + L)` where: - /// - `P` is number of active proposals, - /// - `L` is the encoded length of `proposal` preimage. - /// - /// If not `approved`: - /// - one event deposited. - /// Two removals, one mutation. - /// Computation and i/o `O(P)` where: - /// - `P` is number of active proposals + // Approve proposal fn do_approve_proposal( seats: MemberCount, voting: Votes, @@ -810,12 +735,13 @@ impl, I: 'static> Pallet { Event::Executed(proposal_hash, result.map(|_| ()).map_err(|e| e.error)) ); // default to the dispatch info weight for safety - let proposal_weight = get_result_weight(result).unwrap_or(dispatch_weight); // P1 + let proposal_weight = Self::get_result_weight(result).unwrap_or(dispatch_weight); // P1 let proposal_count = Self::remove_proposal(proposal_hash); (proposal_weight, proposal_count) } + // disapprove proposal from the pallet fn do_disapprove_proposal(proposal_hash: T::Hash) -> u32 { // disapproved Self::deposit_event(Event::Disapproved(proposal_hash)); @@ -833,6 +759,16 @@ impl, I: 'static> Pallet { }); num_proposals as u32 } + + /// Return the weight of a dispatch call result as an `Option`. + /// + /// Will return the weight regardless of what the state of the result is. + fn get_result_weight(result: DispatchResultWithPostInfo) -> Option { + match result { + Ok(post_info) => post_info.actual_weight, + Err(err) => err.post_info.actual_weight, + } + } } impl, I: 'static> ChangeMembers for Pallet { @@ -840,19 +776,6 @@ impl, I: 'static> ChangeMembers for Pallet { /// /// NOTE: Does not enforce the expected `MaxMembers` limit on the amount of members, but /// the weight estimations rely on it to estimate dispatchable weight. - /// - /// # - /// ## Weight - /// - `O(MP + N)` - /// - where `M` old-members-count (governance-bounded) - /// - where `N` new-members-count (governance-bounded) - /// - where `P` proposals-count - /// - DB: - /// - 1 storage read (codec `O(P)`) for reading the proposals - /// - `P` storage mutations for updating the votes (codec `O(M)`) - /// - 1 storage write (codec `O(N)`) for storing the new members - /// - 1 storage write (codec `O(1)`) for deleting the old prime - /// # fn change_members_sorted( _incoming: &[T::AccountId], outgoing: &[T::AccountId], @@ -1003,785 +926,4 @@ impl< fn successful_origin() -> O { O::from(RawOrigin::Members(0u32, 0u32)) } -} - -#[cfg(test)] -mod tests { - use super::*; - use frame_support::{Hashable, assert_ok, assert_noop, parameter_types}; - use frame_support::{weights::{Pays}}; - use frame_system::{self as system, EventRecord, Phase}; - use hex_literal::hex; - use sp_core::H256; - use sp_runtime::{ - traits::{BlakeTwo256, IdentityLookup}, testing::Header, - BuildStorage, - }; - use crate as collective; - - parameter_types! { - pub const BlockHashCount: u64 = 250; - pub const MotionDuration: u64 = 3; - pub const MaxProposals: u32 = 100; - pub const MaxMembers: u32 = 100; - pub BlockWeights: frame_system::limits::BlockWeights = - frame_system::limits::BlockWeights::simple_max(1024); - } - impl frame_system::Config for Test { - type BaseCallFilter = (); - type BlockWeights = (); - type BlockLength = (); - type DbWeight = (); - type Origin = Origin; - type Index = u64; - type BlockNumber = u64; - type Call = Call; - type Hash = H256; - type Hashing = BlakeTwo256; - type AccountId = u64; - type Lookup = IdentityLookup; - type Header = Header; - type Event = Event; - type BlockHashCount = BlockHashCount; - type Version = (); - type PalletInfo = PalletInfo; - type AccountData = (); - type OnNewAccount = (); - type OnKilledAccount = (); - type SystemWeightInfo = (); - type SS58Prefix = (); - } - impl Config for Test { - type Origin = Origin; - type Proposal = Call; - type Event = Event; - type MotionDuration = MotionDuration; - type MaxProposals = MaxProposals; - type MaxMembers = MaxMembers; - type DefaultVote = PrimeDefaultVote; - type WeightInfo = (); - } - impl Config for Test { - type Origin = Origin; - type Proposal = Call; - type Event = Event; - type MotionDuration = MotionDuration; - type MaxProposals = MaxProposals; - type MaxMembers = MaxMembers; - type DefaultVote = MoreThanMajorityThenPrimeDefaultVote; - type WeightInfo = (); - } - impl Config for Test { - type Origin = Origin; - type Proposal = Call; - type Event = Event; - type MotionDuration = MotionDuration; - type MaxProposals = MaxProposals; - type MaxMembers = MaxMembers; - type DefaultVote = PrimeDefaultVote; - type WeightInfo = (); - } - - pub type Block = sp_runtime::generic::Block; - pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; - - frame_support::construct_runtime!( - pub enum Test where - Block = Block, - NodeBlock = Block, - UncheckedExtrinsic = UncheckedExtrinsic - { - System: system::{Module, Call, Event}, - // Collective: collective::::{Module, Call, Event, Config}, - Collective: collective::::{Module, Call, Event, Origin, Config}, - CollectiveMajority: collective::::{Module, Call, Event, Origin, Config}, - DefaultCollective: collective::{Module, Call, Event, Origin, Config}, - } - ); - - pub fn new_test_ext() -> sp_io::TestExternalities { - let mut ext: sp_io::TestExternalities = GenesisConfig { - collective_Instance1: Some(collective::GenesisConfig { - members: vec![1, 2, 3], - phantom: Default::default(), - }), - collective_Instance2: Some(collective::GenesisConfig { - members: vec![1, 2, 3, 4, 5], - phantom: Default::default(), - }), - collective: None, - }.build_storage().unwrap().into(); - ext.execute_with(|| System::set_block_number(1)); - ext - } - - fn make_proposal(value: u64) -> Call { - let rval = Call::System(frame_system::Call::remark(value.encode())); - let proposal_len: u32 = rval.using_encoded(|p| p.len() as u32); - if_std!{ - println!("prop_len :: {:#?}", proposal_len); - } - return rval - } - - #[test] - fn motions_basic_environment_works() { - new_test_ext().execute_with(|| { - assert_eq!(Collective::members(), vec![1, 2, 3]); - assert_eq!(Collective::proposals(), Vec::::new()); - }); - } - - #[test] - fn close_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(3); - assert_noop!( - Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len), - Error::::TooEarly - ); - - System::set_block_number(4); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), - record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) - ]); - }); - } - - #[test] - fn proposal_weight_limit_works_on_approve() { - new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - // Set 1 as prime voter - Prime::::set(Some(1)); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - // With 1's prime vote, this should pass - System::set_block_number(4); - assert_noop!( - Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len), - Error::::WrongProposalWeight - ); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - }) - } - - #[test] - fn proposal_weight_limit_ignored_on_disapprove() { - new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - // No votes, this proposal wont pass - System::set_block_number(4); - assert_ok!( - Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len) - ); - }) - } - - #[test] - fn close_with_prime_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(3), MaxMembers::get())); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), - record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) - ]); - }); - } - - #[test] - fn close_with_voting_prime_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(1), MaxMembers::get())); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 3, 0))), - record(Event::collective_Instance1(RawEvent::Approved(hash.clone()))), - record(Event::collective_Instance1(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) - ]); - }); - } - - #[test] - fn close_with_no_prime_but_majority_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(CollectiveMajority::set_members(Origin::root(), vec![1, 2, 3, 4, 5], Some(5), MaxMembers::get())); - - assert_ok!(CollectiveMajority::propose(Origin::signed(1), 5, Box::new(proposal.clone()), proposal_len)); - assert_ok!(CollectiveMajority::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_ok!(CollectiveMajority::vote(Origin::signed(3), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(CollectiveMajority::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance2(RawEvent::Proposed(1, 0, hash.clone(), 5))), - record(Event::collective_Instance2(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance2(RawEvent::Voted(3, hash.clone(), true, 3, 0))), - record(Event::collective_Instance2(RawEvent::Closed(hash.clone(), 5, 0))), - record(Event::collective_Instance2(RawEvent::Approved(hash.clone()))), - record(Event::collective_Instance2(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) - ]); - }); - } - - #[test] - fn removal_of_old_voters_votes_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) - ); - Collective::change_members_sorted(&[4], &[1], &[2, 3, 4]); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) - ); - - let proposal = make_proposal(69); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) - ); - Collective::change_members_sorted(&[], &[3], &[2, 4]); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) - ); - }); - } - - #[test] - fn removal_of_old_voters_votes_works_with_set_members() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) - ); - assert_ok!(Collective::set_members(Origin::root(), vec![2, 3, 4], None, MaxMembers::get())); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) - ); - - let proposal = make_proposal(69); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) - ); - assert_ok!(Collective::set_members(Origin::root(), vec![2, 4], None, MaxMembers::get())); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) - ); - }); - } - - #[test] - fn propose_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = proposal.blake2_256().into(); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_eq!(Collective::proposals(), vec![hash]); - assert_eq!(Collective::proposal_of(&hash), Some(proposal)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![1], nays: vec![], end }) - ); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 3, - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn limit_active_proposals() { - new_test_ext().execute_with(|| { - for i in 0..MaxProposals::get() { - let proposal = make_proposal(i as u64); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - } - let proposal = make_proposal(MaxProposals::get() as u64 + 1); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_noop!( - Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len), - Error::::TooManyProposals - ); - }) - } - - #[test] - fn correct_validate_and_get_proposal() { - new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let length = proposal.encode().len() as u32; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), length)); - - let hash = BlakeTwo256::hash_of(&proposal); - let weight = proposal.get_dispatch_info().weight; - assert_noop!( - Collective::validate_and_get_proposal(&BlakeTwo256::hash_of(&vec![3; 4]), length, weight), - Error::::ProposalMissing - ); - assert_noop!( - Collective::validate_and_get_proposal(&hash, length - 2, weight), - Error::::WrongProposalLength - ); - assert_noop!( - Collective::validate_and_get_proposal(&hash, length, weight - 10), - Error::::WrongProposalWeight - ); - let res = Collective::validate_and_get_proposal(&hash, length, weight); - assert_ok!(res.clone()); - let (retrieved_proposal, len) = res.unwrap(); - assert_eq!(length as usize, len); - assert_eq!(proposal, retrieved_proposal); - }) - } - - #[test] - fn motions_ignoring_non_collective_proposals_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_noop!( - Collective::propose(Origin::signed(42), 3, Box::new(proposal.clone()), proposal_len), - Error::::NotMember - ); - }); - } - - #[test] - fn motions_ignoring_non_collective_votes_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_noop!( - Collective::vote(Origin::signed(42), hash.clone(), 0, true), - Error::::NotMember, - ); - }); - } - - #[test] - fn motions_ignoring_bad_index_collective_vote_works() { - new_test_ext().execute_with(|| { - System::set_block_number(3); - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_noop!( - Collective::vote(Origin::signed(2), hash.clone(), 1, true), - Error::::WrongIndex, - ); - }); - } - - #[test] - fn motions_revoting_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) - ); - assert_noop!( - Collective::vote(Origin::signed(1), hash.clone(), 0, true), - Error::::DuplicateVote, - ); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![1], end }) - ); - assert_noop!( - Collective::vote(Origin::signed(1), hash.clone(), 0, false), - Error::::DuplicateVote, - ); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 2, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Voted( - 1, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - false, - 0, - 1, - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn motions_all_first_vote_free_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - let end = 4; - assert_ok!( - Collective::propose( - Origin::signed(1), - 2, - Box::new(proposal.clone()), - proposal_len, - ) - ); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) - ); - - // For the motion, acc 2's first vote, expecting Ok with Pays::No. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(2), - hash.clone(), - 0, - true, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); - - // Duplicate vote, expecting error with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(2), - hash.clone(), - 0, - true, - ); - assert_eq!(vote_rval.unwrap_err().post_info.pays_fee, Pays::Yes); - - // Modifying vote, expecting ok with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(2), - hash.clone(), - 0, - false, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); - - // For the motion, acc 3's first vote, expecting Ok with Pays::No. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(3), - hash.clone(), - 0, - true, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); - - // acc 3 modify the vote, expecting Ok with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(3), - hash.clone(), - 0, - false, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); - - // Test close() Extrincis | Check DispatchResultWithPostInfo with Pay Info - - let proposal_weight = proposal.get_dispatch_info().weight; - let close_rval: DispatchResultWithPostInfo = Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len, - ); - assert_eq!(close_rval.unwrap().pays_fee, Pays::No); - - // trying to close the proposal, which is already closed. - // Expecting error "ProposalMissing" with Pays::Yes - let close_rval: DispatchResultWithPostInfo = Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len, - ); - assert_eq!(close_rval.unwrap_err().post_info.pays_fee, Pays::Yes); - }); - } - - #[test] - fn motions_reproposing_disapproved_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); - assert_eq!(Collective::proposals(), vec![]); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - assert_eq!(Collective::proposals(), vec![hash]); - }); - } - - #[test] - fn motions_disapproval_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1( - RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 3, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Voted( - 2, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - false, - 1, - 1, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Closed( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 1, 1, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Disapproved( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn motions_approval_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 2, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Voted( - 2, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - true, - 2, - 0, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Closed( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 2, 0, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Approved( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Executed( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - Err(DispatchError::BadOrigin), - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn close_disapprove_does_not_care_about_weight_or_len() { - // This test confirms that if you close a proposal that would be disapproved, - // we do not care about the proposal length or proposal weight since it will - // not be read from storage or executed. - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - // First we make the proposal succeed - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - // It will not close with bad weight/len information - assert_noop!( - Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0), - Error::::WrongProposalLength, - ); - assert_noop!( - Collective::close(Origin::signed(2), hash.clone(), 0, 0, proposal_len), - Error::::WrongProposalWeight, - ); - // Now we make the proposal fail - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - // It can close even if the weight/len information is bad - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0)); - }) - } - - #[test] - fn disapprove_proposal_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - // Proposal would normally succeed - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - // But Root can disapprove and remove it anyway - assert_ok!(Collective::disapprove_proposal(Origin::root(), hash.clone())); - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 2))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))), - ]); - }) - } } \ No newline at end of file diff --git a/frame/collective/src/mock.rs b/frame/collective/src/mock.rs new file mode 100644 index 0000000000000..b478b33d800ec --- /dev/null +++ b/frame/collective/src/mock.rs @@ -0,0 +1,174 @@ +// This file is part of Substrate. + +// Copyright (C) 2018-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. + +//! Test utilities + +#![cfg(test)] + +use sp_std::{prelude::*}; + +use sp_runtime::{ + traits::{Hash, BlakeTwo256, IdentityLookup}, testing::Header, + BuildStorage, +}; +use sp_core::H256; + +use frame_support::{ + Hashable, assert_ok, assert_noop, parameter_types, + traits::{ChangeMembers}, + weights::{Pays, GetDispatchInfo}, +}; + +use frame_system::{self as system, EventRecord, Phase}; +use hex_literal::hex; +use crate::{ + self as collective, + Config, decl_tests, +}; +parameter_types! { + pub const BlockHashCount: u64 = 250; + pub BlockWeights: frame_system::limits::BlockWeights = + frame_system::limits::BlockWeights::simple_max(1024); +} +impl frame_system::Config for Test { + type BaseCallFilter = (); + type BlockWeights = (); + type BlockLength = (); + type DbWeight = (); + type Origin = Origin; + type Index = u64; + type BlockNumber = u64; + type Call = Call; + type Hash = H256; + type Hashing = BlakeTwo256; + type AccountId = u64; + type Lookup = IdentityLookup; + type Header = Header; + type Event = Event; + type BlockHashCount = BlockHashCount; + type Version = (); + type PalletInfo = PalletInfo; + type AccountData = (); + type OnNewAccount = (); + type OnKilledAccount = (); + type SystemWeightInfo = (); + type SS58Prefix = (); +} + +/// Set the prime member's vote as the default vote. +pub struct PrimeDefaultVote; + +impl collective::DefaultVote for PrimeDefaultVote { + fn default_vote( + prime_vote: Option, + _yes_votes: collective::MemberCount, + _no_votes: collective::MemberCount, + _len: collective::MemberCount, + ) -> bool { + prime_vote.unwrap_or(false) + } +} + +/// First see if yes vote are over majority of the whole collective. If so, set the default vote +/// as yes. Otherwise, use the prime meber's vote as the default vote. +pub struct MoreThanMajorityThenPrimeDefaultVote; + +impl collective::DefaultVote for MoreThanMajorityThenPrimeDefaultVote { + fn default_vote( + prime_vote: Option, + yes_votes: collective::MemberCount, + _no_votes: collective::MemberCount, + len: collective::MemberCount, + ) -> bool { + let more_than_majority = yes_votes * 2 > len; + more_than_majority || prime_vote.unwrap_or(false) + } +} + +parameter_types! { + pub const MotionDuration: u64 = 3; + pub const MaxProposals: u32 = 100; + pub const MaxMembers: u32 = 100; +} +impl Config for Test { + type Origin = Origin; + type Proposal = Call; + type Event = Event; + type MotionDuration = MotionDuration; + type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; + type DefaultVote = PrimeDefaultVote; + type WeightInfo = (); +} +impl Config for Test { + type Origin = Origin; + type Proposal = Call; + type Event = Event; + type MotionDuration = MotionDuration; + type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; + type DefaultVote = MoreThanMajorityThenPrimeDefaultVote; + type WeightInfo = (); +} +impl collective::Config for Test { + type Origin = Origin; + type Proposal = Call; + type Event = Event; + type MotionDuration = MotionDuration; + type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; + type DefaultVote = PrimeDefaultVote; + type WeightInfo = (); +} + +type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; +type Block = frame_system::mocking::MockBlock; + +frame_support::construct_runtime!( + pub enum Test where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic + { + System: system::{Module, Call, Event}, + Collective: collective::::{Module, Call, Event, Origin, Config}, + CollectiveMajority: collective::::{Module, Call, Event, Origin, Config}, + DefaultCollective: collective::{Module, Call, Event, Origin, Config}, + } +); + +pub fn new_test_ext() -> sp_io::TestExternalities { + let mut ext: sp_io::TestExternalities = GenesisConfig { + collective_Instance1: Some(collective::GenesisConfig { + members: vec![1, 2, 3], + phantom: Default::default(), + }), + collective_Instance2: Some(collective::GenesisConfig { + members: vec![1, 2, 3, 4, 5], + phantom: Default::default(), + }), + collective: None, + }.build_storage().unwrap().into(); + ext.execute_with(|| System::set_block_number(1)); + ext +} + +fn make_proposal(value: u64) -> Call { + Call::System(frame_system::Call::remark(value.encode())) +} + +decl_tests!{ Test } \ No newline at end of file diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs new file mode 100644 index 0000000000000..79f27e3fc0b8f --- /dev/null +++ b/frame/collective/src/tests.rs @@ -0,0 +1,689 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-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. + +//! Macro for creating the tests for the module. + +#![cfg(test)] + +#[macro_export] +macro_rules! decl_tests { + ($test:ty) => { + + use crate::*; + + #[test] + fn motions_basic_environment_works() { + new_test_ext().execute_with(|| { + assert_eq!(Collective::members(), vec![1, 2, 3]); + assert_eq!(Collective::proposals(), Vec::::new()); + }); + } + + #[test] + fn close_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(3); + assert_noop!( + Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len), + Error::::TooEarly + ); + + System::set_block_number(4); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), + record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) + ]); + }); + } + + #[test] + fn proposal_weight_limit_works_on_approve() { + new_test_ext().execute_with(|| { + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + // Set 1 as prime voter + Prime::::set(Some(1)); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + // With 1's prime vote, this should pass + System::set_block_number(4); + assert_noop!( + Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len), + Error::::WrongProposalWeight + ); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + }) + } + + #[test] + fn proposal_weight_limit_ignored_on_disapprove() { + new_test_ext().execute_with(|| { + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + // No votes, this proposal wont pass + System::set_block_number(4); + assert_ok!( + Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len) + ); + }) + } + + #[test] + fn close_with_prime_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(3), MaxMembers::get())); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), + record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) + ]); + }); + } + + #[test] + fn close_with_voting_prime_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(1), MaxMembers::get())); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 3, 0))), + record(Event::collective_Instance1(RawEvent::Approved(hash.clone()))), + record(Event::collective_Instance1(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) + ]); + }); + } + + #[test] + fn close_with_no_prime_but_majority_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(CollectiveMajority::set_members(Origin::root(), vec![1, 2, 3, 4, 5], Some(5), MaxMembers::get())); + + assert_ok!(CollectiveMajority::propose(Origin::signed(1), 5, Box::new(proposal.clone()), proposal_len)); + assert_ok!(CollectiveMajority::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(CollectiveMajority::vote(Origin::signed(3), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(CollectiveMajority::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance2(RawEvent::Proposed(1, 0, hash.clone(), 5))), + record(Event::collective_Instance2(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance2(RawEvent::Voted(3, hash.clone(), true, 3, 0))), + record(Event::collective_Instance2(RawEvent::Closed(hash.clone(), 5, 0))), + record(Event::collective_Instance2(RawEvent::Approved(hash.clone()))), + record(Event::collective_Instance2(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) + ]); + }); + } + + #[test] + fn removal_of_old_voters_votes_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) + ); + Collective::change_members_sorted(&[4], &[1], &[2, 3, 4]); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) + ); + + let proposal = make_proposal(69); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) + ); + Collective::change_members_sorted(&[], &[3], &[2, 4]); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) + ); + }); + } + + #[test] + fn removal_of_old_voters_votes_works_with_set_members() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) + ); + assert_ok!(Collective::set_members(Origin::root(), vec![2, 3, 4], None, MaxMembers::get())); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) + ); + + let proposal = make_proposal(69); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) + ); + assert_ok!(Collective::set_members(Origin::root(), vec![2, 4], None, MaxMembers::get())); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) + ); + }); + } + + #[test] + fn propose_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = proposal.blake2_256().into(); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_eq!(Collective::proposals(), vec![hash]); + assert_eq!(Collective::proposal_of(&hash), Some(proposal)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![1], nays: vec![], end }) + ); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 3, + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn limit_active_proposals() { + new_test_ext().execute_with(|| { + for i in 0..MaxProposals::get() { + let proposal = make_proposal(i as u64); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + } + let proposal = make_proposal(MaxProposals::get() as u64 + 1); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_noop!( + Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len), + Error::::TooManyProposals + ); + }) + } + + #[test] + fn correct_validate_and_get_proposal() { + new_test_ext().execute_with(|| { + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let length = proposal.encode().len() as u32; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), length)); + + let hash = BlakeTwo256::hash_of(&proposal); + let weight = proposal.get_dispatch_info().weight; + assert_noop!( + Collective::validate_and_get_proposal(&BlakeTwo256::hash_of(&vec![3; 4]), length, weight), + Error::::ProposalMissing + ); + assert_noop!( + Collective::validate_and_get_proposal(&hash, length - 2, weight), + Error::::WrongProposalLength + ); + assert_noop!( + Collective::validate_and_get_proposal(&hash, length, weight - 10), + Error::::WrongProposalWeight + ); + let res = Collective::validate_and_get_proposal(&hash, length, weight); + assert_ok!(res.clone()); + let (retrieved_proposal, len) = res.unwrap(); + assert_eq!(length as usize, len); + assert_eq!(proposal, retrieved_proposal); + }) + } + + #[test] + fn motions_ignoring_non_collective_proposals_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_noop!( + Collective::propose(Origin::signed(42), 3, Box::new(proposal.clone()), proposal_len), + Error::::NotMember + ); + }); + } + + #[test] + fn motions_ignoring_non_collective_votes_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_noop!( + Collective::vote(Origin::signed(42), hash.clone(), 0, true), + Error::::NotMember, + ); + }); + } + + #[test] + fn motions_ignoring_bad_index_collective_vote_works() { + new_test_ext().execute_with(|| { + System::set_block_number(3); + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_noop!( + Collective::vote(Origin::signed(2), hash.clone(), 1, true), + Error::::WrongIndex, + ); + }); + } + + #[test] + fn motions_revoting_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) + ); + assert_noop!( + Collective::vote(Origin::signed(1), hash.clone(), 0, true), + Error::::DuplicateVote, + ); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![1], end }) + ); + assert_noop!( + Collective::vote(Origin::signed(1), hash.clone(), 0, false), + Error::::DuplicateVote, + ); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 2, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Voted( + 1, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + false, + 0, + 1, + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn motions_all_first_vote_free_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + let end = 4; + assert_ok!( + Collective::propose( + Origin::signed(1), + 2, + Box::new(proposal.clone()), + proposal_len, + ) + ); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) + ); + + // For the motion, acc 2's first vote, expecting Ok with Pays::No. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(2), + hash.clone(), + 0, + true, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); + + // Duplicate vote, expecting error with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(2), + hash.clone(), + 0, + true, + ); + assert_eq!(vote_rval.unwrap_err().post_info.pays_fee, Pays::Yes); + + // Modifying vote, expecting ok with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(2), + hash.clone(), + 0, + false, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); + + // For the motion, acc 3's first vote, expecting Ok with Pays::No. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(3), + hash.clone(), + 0, + true, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); + + // acc 3 modify the vote, expecting Ok with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(3), + hash.clone(), + 0, + false, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); + + // Test close() Extrincis | Check DispatchResultWithPostInfo with Pay Info + + let proposal_weight = proposal.get_dispatch_info().weight; + let close_rval: DispatchResultWithPostInfo = Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len, + ); + assert_eq!(close_rval.unwrap().pays_fee, Pays::No); + + // trying to close the proposal, which is already closed. + // Expecting error "ProposalMissing" with Pays::Yes + let close_rval: DispatchResultWithPostInfo = Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len, + ); + assert_eq!(close_rval.unwrap_err().post_info.pays_fee, Pays::Yes); + }); + } + + #[test] + fn motions_reproposing_disapproved_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); + assert_eq!(Collective::proposals(), vec![]); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + assert_eq!(Collective::proposals(), vec![hash]); + }); + } + + #[test] + fn motions_disapproval_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1( + RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 3, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Voted( + 2, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + false, + 1, + 1, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Closed( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 1, 1, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Disapproved( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn motions_approval_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 2, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Voted( + 2, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + true, + 2, + 0, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Closed( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 2, 0, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Approved( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Executed( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + Err(DispatchError::BadOrigin), + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn close_disapprove_does_not_care_about_weight_or_len() { + // This test confirms that if you close a proposal that would be disapproved, + // we do not care about the proposal length or proposal weight since it will + // not be read from storage or executed. + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + // First we make the proposal succeed + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + // It will not close with bad weight/len information + assert_noop!( + Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0), + Error::::WrongProposalLength, + ); + assert_noop!( + Collective::close(Origin::signed(2), hash.clone(), 0, 0, proposal_len), + Error::::WrongProposalWeight, + ); + // Now we make the proposal fail + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + // It can close even if the weight/len information is bad + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0)); + }) + } + + #[test] + fn disapprove_proposal_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + // Proposal would normally succeed + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + // But Root can disapprove and remove it anyway + assert_ok!(Collective::disapprove_proposal(Origin::root(), hash.clone())); + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 2))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))), + ]); + }) + } + } +} \ No newline at end of file From f34c82ce4514f4678034727bdf8fbbb2c4d7b8f6 Mon Sep 17 00:00:00 2001 From: RK Date: Wed, 24 Feb 2021 10:31:25 +0530 Subject: [PATCH 05/10] Update frame/collective/src/mock.rs Co-authored-by: Guillaume Thiolliere --- frame/collective/src/mock.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/collective/src/mock.rs b/frame/collective/src/mock.rs index b478b33d800ec..4181469dad649 100644 --- a/frame/collective/src/mock.rs +++ b/frame/collective/src/mock.rs @@ -145,9 +145,9 @@ frame_support::construct_runtime!( UncheckedExtrinsic = UncheckedExtrinsic { System: system::{Module, Call, Event}, - Collective: collective::::{Module, Call, Event, Origin, Config}, - CollectiveMajority: collective::::{Module, Call, Event, Origin, Config}, - DefaultCollective: collective::{Module, Call, Event, Origin, Config}, + Collective: collective::::{Module, Call, Storage, Event, Origin, Config}, + CollectiveMajority: collective::::{Module, Call, Storage, Event, Origin, Config}, + DefaultCollective: collective::{Module, Call, Storage, Event, Origin, Config}, } ); @@ -171,4 +171,4 @@ fn make_proposal(value: u64) -> Call { Call::System(frame_system::Call::remark(value.encode())) } -decl_tests!{ Test } \ No newline at end of file +decl_tests!{ Test } From a74c3105c179decfdd7796e3b53cc7c8d4d0c2f9 Mon Sep 17 00:00:00 2001 From: RK Date: Wed, 24 Feb 2021 10:41:06 +0530 Subject: [PATCH 06/10] Update frame/collective/src/lib.rs Co-authored-by: Guillaume Thiolliere --- frame/collective/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index a8c74241693ef..9a06ef2f8b8a6 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -617,7 +617,7 @@ pub mod pallet { /// The prime member that helps determine the default vote behavior in case of absentations. #[pallet::storage] #[pallet::getter(fn prime)] - pub type Prime, I: 'static = ()> = StorageValue<_, Option, ValueQuery>; + pub type Prime, I: 'static = ()> = StorageValue<_, T::AccountId, OptionQuery>; #[pallet::genesis_config] pub struct GenesisConfig, I: 'static = ()>{ @@ -926,4 +926,4 @@ impl< fn successful_origin() -> O { O::from(RawOrigin::Members(0u32, 0u32)) } -} \ No newline at end of file +} From 4fbce2c25aa338ecd670fb8dc186b97a7ba53243 Mon Sep 17 00:00:00 2001 From: RK Date: Wed, 24 Feb 2021 10:41:17 +0530 Subject: [PATCH 07/10] Update frame/collective/src/lib.rs Co-authored-by: Guillaume Thiolliere --- frame/collective/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 9a06ef2f8b8a6..cb055d37599d8 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -600,8 +600,8 @@ pub mod pallet { _, Identity, T::Hash, - Option>, - ValueQuery + Votes, + OptionQuery >; /// Proposals so far. From cd06c81c33d31326f14659804ad910cbc405d15e Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 24 Feb 2021 09:59:45 +0100 Subject: [PATCH 08/10] impl and use in balances --- frame/balances/src/benchmarking.rs | 40 ++++++------ frame/benchmarking/src/lib.rs | 98 ++++++++++++++++++------------ 2 files changed, 79 insertions(+), 59 deletions(-) diff --git a/frame/balances/src/benchmarking.rs b/frame/balances/src/benchmarking.rs index 14732b44b4fc2..c7cb67403d749 100644 --- a/frame/balances/src/benchmarking.rs +++ b/frame/balances/src/benchmarking.rs @@ -22,7 +22,7 @@ use super::*; use frame_system::RawOrigin; -use frame_benchmarking::{benchmarks, account, whitelisted_caller, impl_benchmark_test_suite}; +use frame_benchmarking::{benchmarks_instance_pallet, account, whitelisted_caller, impl_benchmark_test_suite}; use sp_runtime::traits::Bounded; use crate::Module as Balances; @@ -32,7 +32,7 @@ const SEED: u32 = 0; const ED_MULTIPLIER: u32 = 10; -benchmarks! { +benchmarks_instance_pallet! { // Benchmark `transfer` extrinsic with the worst possible conditions: // * Transfer will kill the sender account. // * Transfer will create the recipient account. @@ -42,7 +42,7 @@ benchmarks! { // Give some multiple of the existential deposit + creation fee + transfer fee let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); - let _ = as Currency<_>>::make_free_balance_be(&caller, balance); + let _ = as Currency<_>>::make_free_balance_be(&caller, balance); // Transfer `e - 1` existential deposits + 1 unit, which guarantees to create one account, and reap this user. let recipient: T::AccountId = account("recipient", 0, SEED); @@ -50,8 +50,8 @@ benchmarks! { let transfer_amount = existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1u32.into(); }: transfer(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount) verify { - assert_eq!(Balances::::free_balance(&caller), Zero::zero()); - assert_eq!(Balances::::free_balance(&recipient), transfer_amount); + assert_eq!(Balances::::free_balance(&caller), Zero::zero()); + assert_eq!(Balances::::free_balance(&recipient), transfer_amount); } // Benchmark `transfer` with the best possible condition: @@ -63,16 +63,16 @@ benchmarks! { let recipient_lookup: ::Source = T::Lookup::unlookup(recipient.clone()); // Give the sender account max funds for transfer (their account will never reasonably be killed). - let _ = as Currency<_>>::make_free_balance_be(&caller, T::Balance::max_value()); + let _ = as Currency<_>>::make_free_balance_be(&caller, T::Balance::max_value()); // Give the recipient account existential deposit (thus their account already exists). let existential_deposit = T::ExistentialDeposit::get(); - let _ = as Currency<_>>::make_free_balance_be(&recipient, existential_deposit); + let _ = as Currency<_>>::make_free_balance_be(&recipient, existential_deposit); let transfer_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); }: transfer(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount) verify { - assert!(!Balances::::free_balance(&caller).is_zero()); - assert!(!Balances::::free_balance(&recipient).is_zero()); + assert!(!Balances::::free_balance(&caller).is_zero()); + assert!(!Balances::::free_balance(&recipient).is_zero()); } // Benchmark `transfer_keep_alive` with the worst possible condition: @@ -83,13 +83,13 @@ benchmarks! { let recipient_lookup: ::Source = T::Lookup::unlookup(recipient.clone()); // Give the sender account max funds, thus a transfer will not kill account. - let _ = as Currency<_>>::make_free_balance_be(&caller, T::Balance::max_value()); + let _ = as Currency<_>>::make_free_balance_be(&caller, T::Balance::max_value()); let existential_deposit = T::ExistentialDeposit::get(); let transfer_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); }: _(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount) verify { - assert!(!Balances::::free_balance(&caller).is_zero()); - assert_eq!(Balances::::free_balance(&recipient), transfer_amount); + assert!(!Balances::::free_balance(&caller).is_zero()); + assert_eq!(Balances::::free_balance(&recipient), transfer_amount); } // Benchmark `set_balance` coming from ROOT account. This always creates an account. @@ -100,11 +100,11 @@ benchmarks! { // Give the user some initial balance. let existential_deposit = T::ExistentialDeposit::get(); let balance_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); - let _ = as Currency<_>>::make_free_balance_be(&user, balance_amount); + let _ = as Currency<_>>::make_free_balance_be(&user, balance_amount); }: set_balance(RawOrigin::Root, user_lookup, balance_amount, balance_amount) verify { - assert_eq!(Balances::::free_balance(&user), balance_amount); - assert_eq!(Balances::::reserved_balance(&user), balance_amount); + assert_eq!(Balances::::free_balance(&user), balance_amount); + assert_eq!(Balances::::reserved_balance(&user), balance_amount); } // Benchmark `set_balance` coming from ROOT account. This always kills an account. @@ -115,10 +115,10 @@ benchmarks! { // Give the user some initial balance. let existential_deposit = T::ExistentialDeposit::get(); let balance_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); - let _ = as Currency<_>>::make_free_balance_be(&user, balance_amount); + let _ = as Currency<_>>::make_free_balance_be(&user, balance_amount); }: set_balance(RawOrigin::Root, user_lookup, Zero::zero(), Zero::zero()) verify { - assert!(Balances::::free_balance(&user).is_zero()); + assert!(Balances::::free_balance(&user).is_zero()); } // Benchmark `force_transfer` extrinsic with the worst possible conditions: @@ -131,7 +131,7 @@ benchmarks! { // Give some multiple of the existential deposit + creation fee + transfer fee let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); - let _ = as Currency<_>>::make_free_balance_be(&source, balance); + let _ = as Currency<_>>::make_free_balance_be(&source, balance); // Transfer `e - 1` existential deposits + 1 unit, which guarantees to create one account, and reap this user. let recipient: T::AccountId = account("recipient", 0, SEED); @@ -139,8 +139,8 @@ benchmarks! { let transfer_amount = existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1u32.into(); }: force_transfer(RawOrigin::Root, source_lookup, recipient_lookup, transfer_amount) verify { - assert_eq!(Balances::::free_balance(&source), Zero::zero()); - assert_eq!(Balances::::free_balance(&recipient), transfer_amount); + assert_eq!(Balances::::free_balance(&source), Zero::zero()); + assert_eq!(Balances::::free_balance(&recipient), transfer_amount); } } diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs index 94803b88b93f0..219d74c107890 100644 --- a/frame/benchmarking/src/lib.rs +++ b/frame/benchmarking/src/lib.rs @@ -175,13 +175,33 @@ macro_rules! benchmarks { } /// Same as [`benchmarks`] but for instantiable module. +/// +/// NOTE: For pallet declared with [`frame_support::pallet`], use [`benchmarks_instance_pallet`]. #[macro_export] macro_rules! benchmarks_instance { ( $( $rest:tt )* ) => { $crate::benchmarks_iter!( - { I } + { I: Instance } + { } + ( ) + ( ) + $( $rest )* + ); + } +} + +/// Same as [`benchmarks`] but for instantiable pallet declared [`frame_support::pallet`]. +/// +/// NOTE: For pallet declared with `decl_module!`, use [`benchmarks_instnace`]. +#[macro_export] +macro_rules! benchmarks_instance_pallet { + ( + $( $rest:tt )* + ) => { + $crate::benchmarks_iter!( + { I: 'static } { } ( ) ( ) @@ -195,7 +215,7 @@ macro_rules! benchmarks_instance { macro_rules! benchmarks_iter { // detect and extract where clause: ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } { $( $where_clause:tt )* } ( $( $names:tt )* ) ( $( $names_extra:tt )* ) @@ -203,7 +223,7 @@ macro_rules! benchmarks_iter { $( $rest:tt )* ) => { $crate::benchmarks_iter! { - { $( $instance)? } + { $( $instance: $instance_bound)? } { $( $where_bound )* } ( $( $names )* ) ( $( $names_extra )* ) @@ -212,7 +232,7 @@ macro_rules! benchmarks_iter { }; // detect and extract extra tag: ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } { $( $where_clause:tt )* } ( $( $names:tt )* ) ( $( $names_extra:tt )* ) @@ -221,7 +241,7 @@ macro_rules! benchmarks_iter { $( $rest:tt )* ) => { $crate::benchmarks_iter! { - { $( $instance)? } + { $( $instance: $instance_bound )? } { $( $where_clause )* } ( $( $names )* ) ( $( $names_extra )* $name ) @@ -231,7 +251,7 @@ macro_rules! benchmarks_iter { }; // mutation arm: ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } { $( $where_clause:tt )* } ( $( $names:tt )* ) // This contains $( $( { $instance } )? $name:ident )* ( $( $names_extra:tt )* ) @@ -240,7 +260,7 @@ macro_rules! benchmarks_iter { $( $rest:tt )* ) => { $crate::benchmarks_iter! { - { $( $instance)? } + { $( $instance: $instance_bound )? } { $( $where_clause )* } ( $( $names )* ) ( $( $names_extra )* ) @@ -251,7 +271,7 @@ macro_rules! benchmarks_iter { }; // mutation arm: ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } { $( $where_clause:tt )* } ( $( $names:tt )* ) ( $( $names_extra:tt )* ) @@ -260,7 +280,7 @@ macro_rules! benchmarks_iter { $( $rest:tt )* ) => { $crate::benchmarks_iter! { - { $( $instance)? } + { $( $instance: $instance_bound )? } { $( $where_clause )* } ( $( $names )* ) ( $( $names_extra )* ) @@ -277,7 +297,7 @@ macro_rules! benchmarks_iter { }; // iteration arm: ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } { $( $where_clause:tt )* } ( $( $names:tt )* ) ( $( $names_extra:tt )* ) @@ -286,7 +306,7 @@ macro_rules! benchmarks_iter { $( $rest:tt )* ) => { $crate::benchmark_backend! { - { $( $instance)? } + { $( $instance: $instance_bound )? } $name { $( $where_clause )* } { } @@ -298,12 +318,12 @@ macro_rules! benchmarks_iter { #[cfg(test)] $crate::impl_benchmark_test!( { $( $where_clause )* } - { $( $instance)? } + { $( $instance: $instance_bound )? } $name ); $crate::benchmarks_iter!( - { $( $instance)? } + { $( $instance: $instance_bound )? } { $( $where_clause )* } ( $( $names )* { $( $instance )? } $name ) ( $( $names_extra )* ) @@ -312,26 +332,26 @@ macro_rules! benchmarks_iter { }; // iteration-exit arm ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } { $( $where_clause:tt )* } ( $( $names:tt )* ) ( $( $names_extra:tt )* ) ) => { $crate::selected_benchmark!( { $( $where_clause)* } - { $( $instance)? } + { $( $instance: $instance_bound )? } $( $names )* ); $crate::impl_benchmark!( { $( $where_clause )* } - { $( $instance)? } + { $( $instance: $instance_bound )? } ( $( $names )* ) ( $( $names_extra ),* ) ); }; // add verify block to _() format ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } { $( $where_clause:tt )* } ( $( $names:tt )* ) ( $( $names_extra:tt )* ) @@ -339,7 +359,7 @@ macro_rules! benchmarks_iter { $( $rest:tt )* ) => { $crate::benchmarks_iter! { - { $( $instance)? } + { $( $instance: $instance_bound )? } { $( $where_clause )* } ( $( $names )* ) ( $( $names_extra )* ) @@ -350,7 +370,7 @@ macro_rules! benchmarks_iter { }; // add verify block to name() format ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } { $( $where_clause:tt )* } ( $( $names:tt )* ) ( $( $names_extra:tt )* ) @@ -358,7 +378,7 @@ macro_rules! benchmarks_iter { $( $rest:tt )* ) => { $crate::benchmarks_iter! { - { $( $instance)? } + { $( $instance: $instance_bound )? } { $( $where_clause )* } ( $( $names )* ) ( $( $names_extra )* ) @@ -369,7 +389,7 @@ macro_rules! benchmarks_iter { }; // add verify block to {} format ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } { $( $where_clause:tt )* } ( $( $names:tt )* ) ( $( $names_extra:tt )* ) @@ -377,7 +397,7 @@ macro_rules! benchmarks_iter { $( $rest:tt )* ) => { $crate::benchmarks_iter!( - { $( $instance)? } + { $( $instance: $instance_bound )? } { $( $where_clause )* } ( $( $names )* ) ( $( $names_extra )* ) @@ -393,7 +413,7 @@ macro_rules! benchmarks_iter { macro_rules! benchmark_backend { // parsing arms ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } $name:ident { $( $where_clause:tt )* } { $( PRE { $( $pre_parsed:tt )* } )* } @@ -405,7 +425,7 @@ macro_rules! benchmark_backend { $postcode:block ) => { $crate::benchmark_backend! { - { $( $instance)? } + { $( $instance: $instance_bound )? } $name { $( $where_clause )* } { @@ -418,7 +438,7 @@ macro_rules! benchmark_backend { } }; ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } $name:ident { $( $where_clause:tt )* } { $( $parsed:tt )* } @@ -430,7 +450,7 @@ macro_rules! benchmark_backend { $postcode:block ) => { $crate::benchmark_backend! { - { $( $instance)? } + { $( $instance: $instance_bound )? } $name { $( $where_clause )* } { @@ -444,7 +464,7 @@ macro_rules! benchmark_backend { }; // mutation arm to look after a single tt for param_from. ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } $name:ident { $( $where_clause:tt )* } { $( $parsed:tt )* } @@ -456,7 +476,7 @@ macro_rules! benchmark_backend { $postcode:block ) => { $crate::benchmark_backend! { - { $( $instance)? } + { $( $instance: $instance_bound )? } $name { $( $where_clause )* } { $( $parsed )* } @@ -470,7 +490,7 @@ macro_rules! benchmark_backend { }; // mutation arm to look after the default tail of `=> ()` ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } $name:ident { $( $where_clause:tt )* } { $( $parsed:tt )* } @@ -482,7 +502,7 @@ macro_rules! benchmark_backend { $postcode:block ) => { $crate::benchmark_backend! { - { $( $instance)? } + { $( $instance: $instance_bound )? } $name { $( $where_clause )* } { $( $parsed )* } @@ -496,7 +516,7 @@ macro_rules! benchmark_backend { }; // mutation arm to look after `let _ =` ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } $name:ident { $( $where_clause:tt )* } { $( $parsed:tt )* } @@ -508,7 +528,7 @@ macro_rules! benchmark_backend { $postcode:block ) => { $crate::benchmark_backend! { - { $( $instance)? } + { $( $instance: $instance_bound )? } $name { $( $where_clause )* } { $( $parsed )* } @@ -522,7 +542,7 @@ macro_rules! benchmark_backend { }; // actioning arm ( - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } $name:ident { $( $where_clause:tt )* } { @@ -536,7 +556,7 @@ macro_rules! benchmark_backend { #[allow(non_camel_case_types)] struct $name; #[allow(unused_variables)] - impl, I: Instance)? > + impl, $instance: $instance_bound )? > $crate::BenchmarkingSetup for $name where $( $where_clause )* { @@ -597,7 +617,7 @@ macro_rules! benchmark_backend { macro_rules! selected_benchmark { ( { $( $where_clause:tt )* } - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } $( { $( $bench_inst:ident )? } $bench:ident )* ) => { // The list of available benchmarks for this pallet. @@ -607,7 +627,7 @@ macro_rules! selected_benchmark { } // Allow us to select a benchmark from the list of available benchmarks. - impl, I: Instance )? > + impl, $instance: $instance_bound )? > $crate::BenchmarkingSetup for SelectedBenchmark where $( $where_clause )* { @@ -643,11 +663,11 @@ macro_rules! selected_benchmark { macro_rules! impl_benchmark { ( { $( $where_clause:tt )* } - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } ( $( { $( $name_inst:ident )? } $name:ident )* ) ( $( $name_extra:ident ),* ) ) => { - impl, I: Instance)? > + impl, $instance: $instance_bound )? > $crate::Benchmarking<$crate::BenchmarkResults> for Module where T: frame_system::Config, $( $where_clause )* { @@ -866,7 +886,7 @@ macro_rules! impl_benchmark { macro_rules! impl_benchmark_test { ( { $( $where_clause:tt )* } - { $( $instance:ident )? } + { $( $instance:ident: $instance_bound:tt )? } $name:ident ) => { $crate::paste::item! { From 141db8a89f6288962c9a30be67757d8ce83b2f00 Mon Sep 17 00:00:00 2001 From: shamb0 Date: Wed, 24 Feb 2021 14:43:56 +0530 Subject: [PATCH 09/10] wk2108 | rework on review comments --- frame/collective/src/benchmarking.rs | 31 +- frame/collective/src/lib.rs | 831 ++++++++++++++++++++++++++- frame/collective/src/mock.rs | 174 ------ frame/collective/src/tests.rs | 689 ---------------------- 4 files changed, 832 insertions(+), 893 deletions(-) delete mode 100644 frame/collective/src/mock.rs delete mode 100644 frame/collective/src/tests.rs diff --git a/frame/collective/src/benchmarking.rs b/frame/collective/src/benchmarking.rs index 1ca4856c895e7..1afdd14b1ad38 100644 --- a/frame/collective/src/benchmarking.rs +++ b/frame/collective/src/benchmarking.rs @@ -17,14 +17,12 @@ //! Staking pallet benchmarking. -#![cfg(feature = "runtime-benchmarks")] - use super::*; use frame_system::RawOrigin as SystemOrigin; use frame_system::EventRecord; use frame_benchmarking::{ - benchmarks, + benchmarks_instance, account, whitelisted_caller, impl_benchmark_test_suite, @@ -37,9 +35,10 @@ use frame_system::Module as System; use crate::Module as Collective; const SEED: u32 = 0; + const MAX_BYTES: u32 = 1_024; -fn assert_last_event, I: 'static>(generic_event: >::Event) { +fn assert_last_event, I: Instance>(generic_event: >::Event) { let events = System::::events(); let system_event: ::Event = generic_event.into(); // compare to the last event record @@ -47,7 +46,7 @@ fn assert_last_event, I: 'static>(generic_event: >:: assert_eq!(event, &system_event); } -benchmarks! { +benchmarks_instance! { set_members { let m in 1 .. T::MaxMembers::get(); let n in 1 .. T::MaxMembers::get(); @@ -138,7 +137,7 @@ benchmarks! { verify { let proposal_hash = T::Hashing::hash_of(&proposal); // Note that execution fails due to mis-matched origin - assert_last_event::( + assert_last_event::( RawEvent::MemberExecuted(proposal_hash, Err(DispatchError::BadOrigin)).into() ); } @@ -169,7 +168,7 @@ benchmarks! { verify { let proposal_hash = T::Hashing::hash_of(&proposal); // Note that execution fails due to mis-matched origin - assert_last_event::( + assert_last_event::( RawEvent::Executed(proposal_hash, Err(DispatchError::BadOrigin)).into() ); } @@ -214,7 +213,7 @@ benchmarks! { // New proposal is recorded assert_eq!(Collective::::proposals().len(), p as usize); let proposal_hash = T::Hashing::hash_of(&proposal); - assert_last_event::(RawEvent::Proposed(caller, p - 1, proposal_hash, threshold).into()); + assert_last_event::(RawEvent::Proposed(caller, p - 1, proposal_hash, threshold).into()); } vote { @@ -288,7 +287,7 @@ benchmarks! { verify { // All proposals exist and the last proposal has just been updated. assert_eq!(Collective::::proposals().len(), p as usize); - let voting = Collective::::voting(&last_hash).ok_or(Error::::ProposalMissing)?; + let voting = Collective::::voting(&last_hash).ok_or(Error::::ProposalMissing)?; assert_eq!(voting.ayes.len(), (m - 3) as usize); assert_eq!(voting.nays.len(), 1); } @@ -370,7 +369,7 @@ benchmarks! { verify { // The last proposal is removed. assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Disapproved(last_hash).into()); + assert_last_event::(RawEvent::Disapproved(last_hash).into()); } close_early_approved { @@ -451,7 +450,7 @@ benchmarks! { verify { // The last proposal is removed. assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Executed(last_hash, Err(DispatchError::BadOrigin)).into()); + assert_last_event::(RawEvent::Executed(last_hash, Err(DispatchError::BadOrigin)).into()); } close_disapproved { @@ -523,7 +522,7 @@ benchmarks! { }: close(SystemOrigin::Signed(caller), last_hash, index, Weight::max_value(), bytes_in_storage) verify { assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Disapproved(last_hash).into()); + assert_last_event::(RawEvent::Disapproved(last_hash).into()); } close_approved { @@ -587,7 +586,7 @@ benchmarks! { }: close(SystemOrigin::Signed(caller), last_hash, p - 1, Weight::max_value(), bytes_in_storage) verify { assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Executed(last_hash, Err(DispatchError::BadOrigin)).into()); + assert_last_event::(RawEvent::Executed(last_hash, Err(DispatchError::BadOrigin)).into()); } disapprove_proposal { @@ -635,12 +634,12 @@ benchmarks! { }: _(SystemOrigin::Root, last_hash) verify { assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Disapproved(last_hash).into()); + assert_last_event::(RawEvent::Disapproved(last_hash).into()); } } impl_benchmark_test_suite!( Collective, - crate::mock::new_test_ext(), - crate::mock::Test, + crate::tests::new_test_ext(), + crate::tests::Test, ); diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index cb055d37599d8..82319b8c57c72 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -42,32 +42,30 @@ #![cfg_attr(not(feature = "std"), no_std)] #![recursion_limit = "128"] -#[macro_use] -mod tests; -mod mock; -mod benchmarking; -pub mod weights; - use sp_std::{prelude::*, result}; use sp_core::u32_trait::Value as U32; use sp_io::storage; +use sp_runtime::{RuntimeDebug, traits::Hash}; use frame_support::{ - codec::{Decode, Encode}, - debug, dispatch::{ - DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable, + codec::{Decode, Encode}, debug, + dispatch::{ + DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable, Parameter, PostDispatchInfo, }, ensure, traits::{ChangeMembers, EnsureOrigin, Get, InitializeMembers}, - weights::{GetDispatchInfo, Weight}, + weights::{DispatchClass, GetDispatchInfo, Weight, Pays}, }; #[cfg(feature = "std")] use frame_support::traits::GenesisBuild; -use sp_runtime::{ RuntimeDebug, traits::{Hash}}; -use frame_system::{self as system}; +use frame_system::{self as system, ensure_signed, ensure_root}; + +#[cfg(feature = "runtime-benchmarks")] +mod benchmarking; +pub mod weights; pub use weights::WeightInfo; pub use pallet::*; @@ -97,6 +95,36 @@ pub trait DefaultVote { ) -> bool; } +/// Set the prime member's vote as the default vote. +pub struct PrimeDefaultVote; + +impl DefaultVote for PrimeDefaultVote { + fn default_vote( + prime_vote: Option, + _yes_votes: MemberCount, + _no_votes: MemberCount, + _len: MemberCount, + ) -> bool { + prime_vote.unwrap_or(false) + } +} + +/// First see if yes vote are over majority of the whole collective. If so, set the default vote +/// as yes. Otherwise, use the prime meber's vote as the default vote. +pub struct MoreThanMajorityThenPrimeDefaultVote; + +impl DefaultVote for MoreThanMajorityThenPrimeDefaultVote { + fn default_vote( + prime_vote: Option, + yes_votes: MemberCount, + _no_votes: MemberCount, + len: MemberCount, + ) -> bool { + let more_than_majority = yes_votes * 2 > len; + more_than_majority || prime_vote.unwrap_or(false) + } +} + #[frame_support::pallet] pub mod pallet { use frame_support::pallet_prelude::*; @@ -311,7 +339,7 @@ pub mod pallet { >::insert(proposal_hash, *proposal); let end = system::Module::::block_number() + T::MotionDuration::get(); let votes = Votes { index, threshold, ayes: vec![who.clone()], nays: vec![], end }; - >::insert(proposal_hash, Some(votes)); + >::insert(proposal_hash, votes); Self::deposit_event(Event::Proposed(who, index, proposal_hash, threshold)); @@ -373,7 +401,7 @@ pub mod pallet { let no_votes = voting.nays.len() as MemberCount; Self::deposit_event(Event::Voted(who, proposal, approve, yes_votes, no_votes)); - >::insert(&proposal, Some(voting)); + >::insert(&proposal, voting); if is_account_voting_first_time { Ok(( @@ -927,3 +955,778 @@ impl< O::from(RawOrigin::Members(0u32, 0u32)) } } + + +#[cfg(test)] +mod tests { + use super::*; + use frame_support::{Hashable, assert_ok, assert_noop, parameter_types}; + use frame_system::{self as system, EventRecord, Phase}; + use hex_literal::hex; + use sp_core::H256; + use sp_runtime::{ + traits::{BlakeTwo256, IdentityLookup}, testing::Header, + BuildStorage, + }; + use crate as collective; + + parameter_types! { + pub const BlockHashCount: u64 = 250; + pub const MotionDuration: u64 = 3; + pub const MaxProposals: u32 = 100; + pub const MaxMembers: u32 = 100; + pub BlockWeights: frame_system::limits::BlockWeights = + frame_system::limits::BlockWeights::simple_max(1024); + } + impl frame_system::Config for Test { + type BaseCallFilter = (); + type BlockWeights = (); + type BlockLength = (); + type DbWeight = (); + type Origin = Origin; + type Index = u64; + type BlockNumber = u64; + type Call = Call; + type Hash = H256; + type Hashing = BlakeTwo256; + type AccountId = u64; + type Lookup = IdentityLookup; + type Header = Header; + type Event = Event; + type BlockHashCount = BlockHashCount; + type Version = (); + type PalletInfo = PalletInfo; + type AccountData = (); + type OnNewAccount = (); + type OnKilledAccount = (); + type SystemWeightInfo = (); + type SS58Prefix = (); + } + impl Config for Test { + type Origin = Origin; + type Proposal = Call; + type Event = Event; + type MotionDuration = MotionDuration; + type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; + type DefaultVote = PrimeDefaultVote; + type WeightInfo = (); + } + impl Config for Test { + type Origin = Origin; + type Proposal = Call; + type Event = Event; + type MotionDuration = MotionDuration; + type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; + type DefaultVote = MoreThanMajorityThenPrimeDefaultVote; + type WeightInfo = (); + } + impl Config for Test { + type Origin = Origin; + type Proposal = Call; + type Event = Event; + type MotionDuration = MotionDuration; + type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; + type DefaultVote = PrimeDefaultVote; + type WeightInfo = (); + } + + pub type Block = sp_runtime::generic::Block; + pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; + + frame_support::construct_runtime!( + pub enum Test where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic + { + System: system::{Module, Call, Event}, + Collective: collective::::{Module, Call, Storage, Event, Origin, Config}, + CollectiveMajority: collective::::{Module, Call, Storage, Event, Origin, Config}, + DefaultCollective: collective::{Module, Call, Storage, Event, Origin, Config}, + } + ); + + pub fn new_test_ext() -> sp_io::TestExternalities { + let mut ext: sp_io::TestExternalities = GenesisConfig { + collective_Instance1: Some(collective::GenesisConfig { + members: vec![1, 2, 3], + phantom: Default::default(), + }), + collective_Instance2: Some(collective::GenesisConfig { + members: vec![1, 2, 3, 4, 5], + phantom: Default::default(), + }), + collective: None, + }.build_storage().unwrap().into(); + ext.execute_with(|| System::set_block_number(1)); + ext + } + + fn make_proposal(value: u64) -> Call { + Call::System(frame_system::Call::remark(value.encode())) + } + + #[test] + fn motions_basic_environment_works() { + new_test_ext().execute_with(|| { + assert_eq!(Collective::members(), vec![1, 2, 3]); + assert_eq!(Collective::proposals(), Vec::::new()); + }); + } + + #[test] + fn close_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(3); + assert_noop!( + Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len), + Error::::TooEarly + ); + + System::set_block_number(4); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), + record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) + ]); + }); + } + + #[test] + fn proposal_weight_limit_works_on_approve() { + new_test_ext().execute_with(|| { + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + // Set 1 as prime voter + Prime::::set(Some(1)); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + // With 1's prime vote, this should pass + System::set_block_number(4); + assert_noop!( + Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len), + Error::::WrongProposalWeight + ); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + }) + } + + #[test] + fn proposal_weight_limit_ignored_on_disapprove() { + new_test_ext().execute_with(|| { + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + // No votes, this proposal wont pass + System::set_block_number(4); + assert_ok!( + Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len) + ); + }) + } + + #[test] + fn close_with_prime_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(3), MaxMembers::get())); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), + record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) + ]); + }); + } + + #[test] + fn close_with_voting_prime_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(1), MaxMembers::get())); + + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 3, 0))), + record(Event::collective_Instance1(RawEvent::Approved(hash.clone()))), + record(Event::collective_Instance1(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) + ]); + }); + } + + #[test] + fn close_with_no_prime_but_majority_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(CollectiveMajority::set_members(Origin::root(), vec![1, 2, 3, 4, 5], Some(5), MaxMembers::get())); + + assert_ok!(CollectiveMajority::propose(Origin::signed(1), 5, Box::new(proposal.clone()), proposal_len)); + assert_ok!(CollectiveMajority::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(CollectiveMajority::vote(Origin::signed(3), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(CollectiveMajority::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); + + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance2(RawEvent::Proposed(1, 0, hash.clone(), 5))), + record(Event::collective_Instance2(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance2(RawEvent::Voted(3, hash.clone(), true, 3, 0))), + record(Event::collective_Instance2(RawEvent::Closed(hash.clone(), 5, 0))), + record(Event::collective_Instance2(RawEvent::Approved(hash.clone()))), + record(Event::collective_Instance2(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) + ]); + }); + } + + #[test] + fn removal_of_old_voters_votes_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) + ); + Collective::change_members_sorted(&[4], &[1], &[2, 3, 4]); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) + ); + + let proposal = make_proposal(69); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) + ); + Collective::change_members_sorted(&[], &[3], &[2, 4]); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) + ); + }); + } + + #[test] + fn removal_of_old_voters_votes_works_with_set_members() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) + ); + assert_ok!(Collective::set_members(Origin::root(), vec![2, 3, 4], None, MaxMembers::get())); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) + ); + + let proposal = make_proposal(69); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) + ); + assert_ok!(Collective::set_members(Origin::root(), vec![2, 4], None, MaxMembers::get())); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) + ); + }); + } + + #[test] + fn propose_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = proposal.blake2_256().into(); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_eq!(Collective::proposals(), vec![hash]); + assert_eq!(Collective::proposal_of(&hash), Some(proposal)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![1], nays: vec![], end }) + ); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 3, + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn limit_active_proposals() { + new_test_ext().execute_with(|| { + for i in 0..MaxProposals::get() { + let proposal = make_proposal(i as u64); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + } + let proposal = make_proposal(MaxProposals::get() as u64 + 1); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_noop!( + Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len), + Error::::TooManyProposals + ); + }) + } + + #[test] + fn correct_validate_and_get_proposal() { + new_test_ext().execute_with(|| { + let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let length = proposal.encode().len() as u32; + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), length)); + + let hash = BlakeTwo256::hash_of(&proposal); + let weight = proposal.get_dispatch_info().weight; + assert_noop!( + Collective::validate_and_get_proposal(&BlakeTwo256::hash_of(&vec![3; 4]), length, weight), + Error::::ProposalMissing + ); + assert_noop!( + Collective::validate_and_get_proposal(&hash, length - 2, weight), + Error::::WrongProposalLength + ); + assert_noop!( + Collective::validate_and_get_proposal(&hash, length, weight - 10), + Error::::WrongProposalWeight + ); + let res = Collective::validate_and_get_proposal(&hash, length, weight); + assert_ok!(res.clone()); + let (retrieved_proposal, len) = res.unwrap(); + assert_eq!(length as usize, len); + assert_eq!(proposal, retrieved_proposal); + }) + } + + #[test] + fn motions_ignoring_non_collective_proposals_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_noop!( + Collective::propose(Origin::signed(42), 3, Box::new(proposal.clone()), proposal_len), + Error::::NotMember + ); + }); + } + + #[test] + fn motions_ignoring_non_collective_votes_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_noop!( + Collective::vote(Origin::signed(42), hash.clone(), 0, true), + Error::::NotMember, + ); + }); + } + + #[test] + fn motions_ignoring_bad_index_collective_vote_works() { + new_test_ext().execute_with(|| { + System::set_block_number(3); + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_noop!( + Collective::vote(Origin::signed(2), hash.clone(), 1, true), + Error::::WrongIndex, + ); + }); + } + + #[test] + fn motions_revoting_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + let end = 4; + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) + ); + assert_noop!( + Collective::vote(Origin::signed(1), hash.clone(), 0, true), + Error::::DuplicateVote, + ); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![1], end }) + ); + assert_noop!( + Collective::vote(Origin::signed(1), hash.clone(), 0, false), + Error::::DuplicateVote, + ); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 2, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Voted( + 1, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + false, + 0, + 1, + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn motions_all_first_vote_free_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + let end = 4; + assert_ok!( + Collective::propose( + Origin::signed(1), + 2, + Box::new(proposal.clone()), + proposal_len, + ) + ); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) + ); + + // For the motion, acc 2's first vote, expecting Ok with Pays::No. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(2), + hash.clone(), + 0, + true, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); + + // Duplicate vote, expecting error with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(2), + hash.clone(), + 0, + true, + ); + assert_eq!(vote_rval.unwrap_err().post_info.pays_fee, Pays::Yes); + + // Modifying vote, expecting ok with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(2), + hash.clone(), + 0, + false, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); + + // For the motion, acc 3's first vote, expecting Ok with Pays::No. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(3), + hash.clone(), + 0, + true, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); + + // acc 3 modify the vote, expecting Ok with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = Collective::vote( + Origin::signed(3), + hash.clone(), + 0, + false, + ); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); + + // Test close() Extrincis | Check DispatchResultWithPostInfo with Pay Info + + let proposal_weight = proposal.get_dispatch_info().weight; + let close_rval: DispatchResultWithPostInfo = Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len, + ); + assert_eq!(close_rval.unwrap().pays_fee, Pays::No); + + // trying to close the proposal, which is already closed. + // Expecting error "ProposalMissing" with Pays::Yes + let close_rval: DispatchResultWithPostInfo = Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len, + ); + assert_eq!(close_rval.unwrap_err().post_info.pays_fee, Pays::Yes); + }); + } + + #[test] + fn motions_reproposing_disapproved_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); + assert_eq!(Collective::proposals(), vec![]); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + assert_eq!(Collective::proposals(), vec![hash]); + }); + } + + #[test] + fn motions_disapproval_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1( + RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 3, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Voted( + 2, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + false, + 1, + 1, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Closed( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 1, 1, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Disapproved( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn motions_approval_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + 2, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Voted( + 2, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + true, + 2, + 0, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Closed( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 2, 0, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Approved( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::collective_Instance1(RawEvent::Executed( + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), + Err(DispatchError::BadOrigin), + )), + topics: vec![], + } + ]); + }); + } + + #[test] + fn close_disapprove_does_not_care_about_weight_or_len() { + // This test confirms that if you close a proposal that would be disapproved, + // we do not care about the proposal length or proposal weight since it will + // not be read from storage or executed. + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + // First we make the proposal succeed + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + // It will not close with bad weight/len information + assert_noop!( + Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0), + Error::::WrongProposalLength, + ); + assert_noop!( + Collective::close(Origin::signed(2), hash.clone(), 0, 0, proposal_len), + Error::::WrongProposalWeight, + ); + // Now we make the proposal fail + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + // It can close even if the weight/len information is bad + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0)); + }) + } + + #[test] + fn disapprove_proposal_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); + // Proposal would normally succeed + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + // But Root can disapprove and remove it anyway + assert_ok!(Collective::disapprove_proposal(Origin::root(), hash.clone())); + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!(System::events(), vec![ + record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 2))), + record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))), + ]); + }) + } +} diff --git a/frame/collective/src/mock.rs b/frame/collective/src/mock.rs deleted file mode 100644 index 4181469dad649..0000000000000 --- a/frame/collective/src/mock.rs +++ /dev/null @@ -1,174 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2018-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. - -//! Test utilities - -#![cfg(test)] - -use sp_std::{prelude::*}; - -use sp_runtime::{ - traits::{Hash, BlakeTwo256, IdentityLookup}, testing::Header, - BuildStorage, -}; -use sp_core::H256; - -use frame_support::{ - Hashable, assert_ok, assert_noop, parameter_types, - traits::{ChangeMembers}, - weights::{Pays, GetDispatchInfo}, -}; - -use frame_system::{self as system, EventRecord, Phase}; -use hex_literal::hex; -use crate::{ - self as collective, - Config, decl_tests, -}; -parameter_types! { - pub const BlockHashCount: u64 = 250; - pub BlockWeights: frame_system::limits::BlockWeights = - frame_system::limits::BlockWeights::simple_max(1024); -} -impl frame_system::Config for Test { - type BaseCallFilter = (); - type BlockWeights = (); - type BlockLength = (); - type DbWeight = (); - type Origin = Origin; - type Index = u64; - type BlockNumber = u64; - type Call = Call; - type Hash = H256; - type Hashing = BlakeTwo256; - type AccountId = u64; - type Lookup = IdentityLookup; - type Header = Header; - type Event = Event; - type BlockHashCount = BlockHashCount; - type Version = (); - type PalletInfo = PalletInfo; - type AccountData = (); - type OnNewAccount = (); - type OnKilledAccount = (); - type SystemWeightInfo = (); - type SS58Prefix = (); -} - -/// Set the prime member's vote as the default vote. -pub struct PrimeDefaultVote; - -impl collective::DefaultVote for PrimeDefaultVote { - fn default_vote( - prime_vote: Option, - _yes_votes: collective::MemberCount, - _no_votes: collective::MemberCount, - _len: collective::MemberCount, - ) -> bool { - prime_vote.unwrap_or(false) - } -} - -/// First see if yes vote are over majority of the whole collective. If so, set the default vote -/// as yes. Otherwise, use the prime meber's vote as the default vote. -pub struct MoreThanMajorityThenPrimeDefaultVote; - -impl collective::DefaultVote for MoreThanMajorityThenPrimeDefaultVote { - fn default_vote( - prime_vote: Option, - yes_votes: collective::MemberCount, - _no_votes: collective::MemberCount, - len: collective::MemberCount, - ) -> bool { - let more_than_majority = yes_votes * 2 > len; - more_than_majority || prime_vote.unwrap_or(false) - } -} - -parameter_types! { - pub const MotionDuration: u64 = 3; - pub const MaxProposals: u32 = 100; - pub const MaxMembers: u32 = 100; -} -impl Config for Test { - type Origin = Origin; - type Proposal = Call; - type Event = Event; - type MotionDuration = MotionDuration; - type MaxProposals = MaxProposals; - type MaxMembers = MaxMembers; - type DefaultVote = PrimeDefaultVote; - type WeightInfo = (); -} -impl Config for Test { - type Origin = Origin; - type Proposal = Call; - type Event = Event; - type MotionDuration = MotionDuration; - type MaxProposals = MaxProposals; - type MaxMembers = MaxMembers; - type DefaultVote = MoreThanMajorityThenPrimeDefaultVote; - type WeightInfo = (); -} -impl collective::Config for Test { - type Origin = Origin; - type Proposal = Call; - type Event = Event; - type MotionDuration = MotionDuration; - type MaxProposals = MaxProposals; - type MaxMembers = MaxMembers; - type DefaultVote = PrimeDefaultVote; - type WeightInfo = (); -} - -type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; -type Block = frame_system::mocking::MockBlock; - -frame_support::construct_runtime!( - pub enum Test where - Block = Block, - NodeBlock = Block, - UncheckedExtrinsic = UncheckedExtrinsic - { - System: system::{Module, Call, Event}, - Collective: collective::::{Module, Call, Storage, Event, Origin, Config}, - CollectiveMajority: collective::::{Module, Call, Storage, Event, Origin, Config}, - DefaultCollective: collective::{Module, Call, Storage, Event, Origin, Config}, - } -); - -pub fn new_test_ext() -> sp_io::TestExternalities { - let mut ext: sp_io::TestExternalities = GenesisConfig { - collective_Instance1: Some(collective::GenesisConfig { - members: vec![1, 2, 3], - phantom: Default::default(), - }), - collective_Instance2: Some(collective::GenesisConfig { - members: vec![1, 2, 3, 4, 5], - phantom: Default::default(), - }), - collective: None, - }.build_storage().unwrap().into(); - ext.execute_with(|| System::set_block_number(1)); - ext -} - -fn make_proposal(value: u64) -> Call { - Call::System(frame_system::Call::remark(value.encode())) -} - -decl_tests!{ Test } diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs deleted file mode 100644 index 79f27e3fc0b8f..0000000000000 --- a/frame/collective/src/tests.rs +++ /dev/null @@ -1,689 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2017-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. - -//! Macro for creating the tests for the module. - -#![cfg(test)] - -#[macro_export] -macro_rules! decl_tests { - ($test:ty) => { - - use crate::*; - - #[test] - fn motions_basic_environment_works() { - new_test_ext().execute_with(|| { - assert_eq!(Collective::members(), vec![1, 2, 3]); - assert_eq!(Collective::proposals(), Vec::::new()); - }); - } - - #[test] - fn close_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(3); - assert_noop!( - Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len), - Error::::TooEarly - ); - - System::set_block_number(4); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), - record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) - ]); - }); - } - - #[test] - fn proposal_weight_limit_works_on_approve() { - new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - // Set 1 as prime voter - Prime::::set(Some(1)); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - // With 1's prime vote, this should pass - System::set_block_number(4); - assert_noop!( - Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len), - Error::::WrongProposalWeight - ); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - }) - } - - #[test] - fn proposal_weight_limit_ignored_on_disapprove() { - new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - // No votes, this proposal wont pass - System::set_block_number(4); - assert_ok!( - Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight - 100, proposal_len) - ); - }) - } - - #[test] - fn close_with_prime_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(3), MaxMembers::get())); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 2, 1))), - record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))) - ]); - }); - } - - #[test] - fn close_with_voting_prime_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(1), MaxMembers::get())); - - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(Collective::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Closed(hash.clone(), 3, 0))), - record(Event::collective_Instance1(RawEvent::Approved(hash.clone()))), - record(Event::collective_Instance1(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) - ]); - }); - } - - #[test] - fn close_with_no_prime_but_majority_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(CollectiveMajority::set_members(Origin::root(), vec![1, 2, 3, 4, 5], Some(5), MaxMembers::get())); - - assert_ok!(CollectiveMajority::propose(Origin::signed(1), 5, Box::new(proposal.clone()), proposal_len)); - assert_ok!(CollectiveMajority::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_ok!(CollectiveMajority::vote(Origin::signed(3), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(CollectiveMajority::close(Origin::signed(4), hash.clone(), 0, proposal_weight, proposal_len)); - - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance2(RawEvent::Proposed(1, 0, hash.clone(), 5))), - record(Event::collective_Instance2(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance2(RawEvent::Voted(3, hash.clone(), true, 3, 0))), - record(Event::collective_Instance2(RawEvent::Closed(hash.clone(), 5, 0))), - record(Event::collective_Instance2(RawEvent::Approved(hash.clone()))), - record(Event::collective_Instance2(RawEvent::Executed(hash.clone(), Err(DispatchError::BadOrigin)))) - ]); - }); - } - - #[test] - fn removal_of_old_voters_votes_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) - ); - Collective::change_members_sorted(&[4], &[1], &[2, 3, 4]); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) - ); - - let proposal = make_proposal(69); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) - ); - Collective::change_members_sorted(&[], &[3], &[2, 4]); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) - ); - }); - } - - #[test] - fn removal_of_old_voters_votes_works_with_set_members() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) - ); - assert_ok!(Collective::set_members(Origin::root(), vec![2, 3, 4], None, MaxMembers::get())); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) - ); - - let proposal = make_proposal(69); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::propose(Origin::signed(2), 2, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) - ); - assert_ok!(Collective::set_members(Origin::root(), vec![2, 4], None, MaxMembers::get())); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) - ); - }); - } - - #[test] - fn propose_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = proposal.blake2_256().into(); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_eq!(Collective::proposals(), vec![hash]); - assert_eq!(Collective::proposal_of(&hash), Some(proposal)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![1], nays: vec![], end }) - ); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 3, - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn limit_active_proposals() { - new_test_ext().execute_with(|| { - for i in 0..MaxProposals::get() { - let proposal = make_proposal(i as u64); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - } - let proposal = make_proposal(MaxProposals::get() as u64 + 1); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_noop!( - Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len), - Error::::TooManyProposals - ); - }) - } - - #[test] - fn correct_validate_and_get_proposal() { - new_test_ext().execute_with(|| { - let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let length = proposal.encode().len() as u32; - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), length)); - - let hash = BlakeTwo256::hash_of(&proposal); - let weight = proposal.get_dispatch_info().weight; - assert_noop!( - Collective::validate_and_get_proposal(&BlakeTwo256::hash_of(&vec![3; 4]), length, weight), - Error::::ProposalMissing - ); - assert_noop!( - Collective::validate_and_get_proposal(&hash, length - 2, weight), - Error::::WrongProposalLength - ); - assert_noop!( - Collective::validate_and_get_proposal(&hash, length, weight - 10), - Error::::WrongProposalWeight - ); - let res = Collective::validate_and_get_proposal(&hash, length, weight); - assert_ok!(res.clone()); - let (retrieved_proposal, len) = res.unwrap(); - assert_eq!(length as usize, len); - assert_eq!(proposal, retrieved_proposal); - }) - } - - #[test] - fn motions_ignoring_non_collective_proposals_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_noop!( - Collective::propose(Origin::signed(42), 3, Box::new(proposal.clone()), proposal_len), - Error::::NotMember - ); - }); - } - - #[test] - fn motions_ignoring_non_collective_votes_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_noop!( - Collective::vote(Origin::signed(42), hash.clone(), 0, true), - Error::::NotMember, - ); - }); - } - - #[test] - fn motions_ignoring_bad_index_collective_vote_works() { - new_test_ext().execute_with(|| { - System::set_block_number(3); - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_noop!( - Collective::vote(Origin::signed(2), hash.clone(), 1, true), - Error::::WrongIndex, - ); - }); - } - - #[test] - fn motions_revoting_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - let end = 4; - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) - ); - assert_noop!( - Collective::vote(Origin::signed(1), hash.clone(), 0, true), - Error::::DuplicateVote, - ); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![1], end }) - ); - assert_noop!( - Collective::vote(Origin::signed(1), hash.clone(), 0, false), - Error::::DuplicateVote, - ); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 2, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Voted( - 1, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - false, - 0, - 1, - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn motions_all_first_vote_free_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - let end = 4; - assert_ok!( - Collective::propose( - Origin::signed(1), - 2, - Box::new(proposal.clone()), - proposal_len, - ) - ); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) - ); - - // For the motion, acc 2's first vote, expecting Ok with Pays::No. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(2), - hash.clone(), - 0, - true, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); - - // Duplicate vote, expecting error with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(2), - hash.clone(), - 0, - true, - ); - assert_eq!(vote_rval.unwrap_err().post_info.pays_fee, Pays::Yes); - - // Modifying vote, expecting ok with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(2), - hash.clone(), - 0, - false, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); - - // For the motion, acc 3's first vote, expecting Ok with Pays::No. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(3), - hash.clone(), - 0, - true, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); - - // acc 3 modify the vote, expecting Ok with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = Collective::vote( - Origin::signed(3), - hash.clone(), - 0, - false, - ); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); - - // Test close() Extrincis | Check DispatchResultWithPostInfo with Pay Info - - let proposal_weight = proposal.get_dispatch_info().weight; - let close_rval: DispatchResultWithPostInfo = Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len, - ); - assert_eq!(close_rval.unwrap().pays_fee, Pays::No); - - // trying to close the proposal, which is already closed. - // Expecting error "ProposalMissing" with Pays::Yes - let close_rval: DispatchResultWithPostInfo = Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len, - ); - assert_eq!(close_rval.unwrap_err().post_info.pays_fee, Pays::Yes); - }); - } - - #[test] - fn motions_reproposing_disapproved_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); - assert_eq!(Collective::proposals(), vec![]); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - assert_eq!(Collective::proposals(), vec![hash]); - }); - } - - #[test] - fn motions_disapproval_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1( - RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 3, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Voted( - 2, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - false, - 1, - 1, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Closed( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 1, 1, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Disapproved( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn motions_approval_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); - - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - 2, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Voted( - 2, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - true, - 2, - 0, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Closed( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), 2, 0, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Approved( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::collective_Instance1(RawEvent::Executed( - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"].into(), - Err(DispatchError::BadOrigin), - )), - topics: vec![], - } - ]); - }); - } - - #[test] - fn close_disapprove_does_not_care_about_weight_or_len() { - // This test confirms that if you close a proposal that would be disapproved, - // we do not care about the proposal length or proposal weight since it will - // not be read from storage or executed. - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - // First we make the proposal succeed - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - // It will not close with bad weight/len information - assert_noop!( - Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0), - Error::::WrongProposalLength, - ); - assert_noop!( - Collective::close(Origin::signed(2), hash.clone(), 0, 0, proposal_len), - Error::::WrongProposalWeight, - ); - // Now we make the proposal fail - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - // It can close even if the weight/len information is bad - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0)); - }) - } - - #[test] - fn disapprove_proposal_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - // Proposal would normally succeed - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - // But Root can disapprove and remove it anyway - assert_ok!(Collective::disapprove_proposal(Origin::root(), hash.clone())); - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!(System::events(), vec![ - record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 2))), - record(Event::collective_Instance1(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::collective_Instance1(RawEvent::Disapproved(hash.clone()))), - ]); - }) - } - } -} \ No newline at end of file From a390d8fca337b18295201f3d4767df7419c043f3 Mon Sep 17 00:00:00 2001 From: shamb0 Date: Wed, 24 Feb 2021 15:30:37 +0530 Subject: [PATCH 10/10] wk2108 | update on benchmarks.rs --- frame/collective/src/benchmarking.rs | 6 +++--- frame/collective/src/lib.rs | 11 +++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/frame/collective/src/benchmarking.rs b/frame/collective/src/benchmarking.rs index 1afdd14b1ad38..6406c503fe154 100644 --- a/frame/collective/src/benchmarking.rs +++ b/frame/collective/src/benchmarking.rs @@ -22,7 +22,7 @@ use super::*; use frame_system::RawOrigin as SystemOrigin; use frame_system::EventRecord; use frame_benchmarking::{ - benchmarks_instance, + benchmarks_instance_pallet, account, whitelisted_caller, impl_benchmark_test_suite, @@ -38,7 +38,7 @@ const SEED: u32 = 0; const MAX_BYTES: u32 = 1_024; -fn assert_last_event, I: Instance>(generic_event: >::Event) { +fn assert_last_event, I: 'static>(generic_event: >::Event) { let events = System::::events(); let system_event: ::Event = generic_event.into(); // compare to the last event record @@ -46,7 +46,7 @@ fn assert_last_event, I: Instance>(generic_event: >: assert_eq!(event, &system_event); } -benchmarks_instance! { +benchmarks_instance_pallet! { set_members { let m in 1 .. T::MaxMembers::get(); let n in 1 .. T::MaxMembers::get(); diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 82319b8c57c72..456cd19f0c52f 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -50,17 +50,17 @@ use sp_runtime::{RuntimeDebug, traits::Hash}; use frame_support::{ codec::{Decode, Encode}, debug, dispatch::{ - DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable, Parameter, + DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable, PostDispatchInfo, }, ensure, traits::{ChangeMembers, EnsureOrigin, Get, InitializeMembers}, - weights::{DispatchClass, GetDispatchInfo, Weight, Pays}, + weights::{GetDispatchInfo, Weight}, }; #[cfg(feature = "std")] use frame_support::traits::GenesisBuild; -use frame_system::{self as system, ensure_signed, ensure_root}; +use frame_system::{self as system}; #[cfg(feature = "runtime-benchmarks")] mod benchmarking; @@ -960,7 +960,10 @@ impl< #[cfg(test)] mod tests { use super::*; - use frame_support::{Hashable, assert_ok, assert_noop, parameter_types}; + use frame_support::{ + Hashable, assert_ok, assert_noop, parameter_types, + weights::{Pays}, + }; use frame_system::{self as system, EventRecord, Phase}; use hex_literal::hex; use sp_core::H256;