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! { 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 50beb8607d61d..456cd19f0c52f 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -48,17 +48,19 @@ 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, + 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}, }; -use frame_system::{self as system, ensure_signed, ensure_root}; +#[cfg(feature = "std")] +use frame_support::traits::GenesisBuild; + +use frame_system::{self as system}; #[cfg(feature = "runtime-benchmarks")] mod benchmarking; @@ -66,6 +68,8 @@ mod benchmarking; pub mod weights; pub use weights::WeightInfo; +pub use pallet::*; + /// Simple index type for proposal counting. pub type ProposalIndex = u32; @@ -121,165 +125,64 @@ impl DefaultVote for MoreThanMajorityThenPrimeDefaultVote { } } -pub trait Config: frame_system::Config { - /// The outer origin type. - type Origin: From>; - - /// The outer call dispatch type. - type Proposal: Parameter - + Dispatchable>::Origin, PostInfo=PostDispatchInfo> - + From> - + GetDispatchInfo; +#[frame_support::pallet] +pub mod pallet { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + use super::*; - /// The outer event type. - type Event: From> + Into<::Event>; + #[pallet::config] + pub trait Config: frame_system::Config { - /// The time-out for council motions. - type MotionDuration: Get; + /// The outer origin type. + type Origin: From>; - /// Maximum number of proposals allowed to be active in parallel. - type MaxProposals: Get; + /// The outer call dispatch type. + type Proposal: Parameter + + Dispatchable>::Origin, PostInfo=PostDispatchInfo> + + From> + + GetDispatchInfo; - /// 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 overarching event type. + type Event: From> + IsType<::Event>; - /// Default vote strategy of this collective. - type DefaultVote: DefaultVote; + /// The time-out for council motions. + #[pallet::constant] + type MotionDuration: Get; - /// Weight information for extrinsics in this pallet. - type WeightInfo: WeightInfo; -} + /// Maximum number of proposals allowed to be active in parallel. + #[pallet::constant] + type MaxProposals: Get; -/// 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 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. + #[pallet::constant] + type MaxMembers: Get; -/// Origin for the collective module. -pub type Origin = RawOrigin<::AccountId, I>; + /// Default vote strategy of this collective. + type DefaultVote: DefaultVote; -#[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, -} - -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; + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; } - add_extra_genesis { - config(phantom): sp_std::marker::PhantomData; - config(members): Vec; - build(|config| Module::::initialize_members(&config.members)) - } -} -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), - } -} + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(PhantomData<(T, I)>); -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, - } -} + /// Origin for the collective module. + #[pallet::origin] + pub type Origin = RawOrigin<::AccountId, 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 +195,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, + ))] + pub fn set_members( + origin: OriginFor, new_members: Vec, prime: Option, old_count: MemberCount, @@ -321,7 +210,7 @@ decl_module! { 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() ); @@ -330,7 +219,7 @@ decl_module! { 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() ); @@ -350,37 +239,30 @@ 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, + pub 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| { + Ok(Self::get_result_weight(result).map(|w| { T::WeightInfo::execute( proposal_len as u32, // B members.len() as u32, // M @@ -395,27 +277,7 @@ decl_module! { /// `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 - /// # - #[weight = ( + #[pallet::weight( if *threshold < 2 { T::WeightInfo::propose_execute( *length_bound, // B @@ -427,31 +289,35 @@ 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 + pub 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| { + Ok(Self::get_result_weight(result).map(|w| { T::WeightInfo::propose_execute( proposal_len as u32, // B members.len() as u32, // M @@ -463,18 +329,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); + + >::mutate(|i| *i += 1); >::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, 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 +357,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()))] + pub 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 +381,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 +390,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 +399,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, voting); if is_account_voting_first_time { Ok(( @@ -576,20 +433,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 +444,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, + pub 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 +470,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 +480,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 +489,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 +509,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 +518,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)), @@ -683,29 +527,197 @@ decl_module! { } } - /// 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. /// /// 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()) + )] + pub 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`. + // TODO :: Have to re-check is "Note message" required ? + // #[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, + T::Proposal, + OptionQuery, + >; + + /// Votes on a given proposal, if it is ongoing. + #[pallet::storage] + #[pallet::getter(fn voting)] + pub type Voting, I: 'static = ()> = StorageMap< + _, + Identity, + T::Hash, + Votes, + OptionQuery + >; + + /// 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<_, T::AccountId, OptionQuery>; + + #[pallet::genesis_config] + pub struct GenesisConfig, I: 'static = ()>{ + pub members: Vec, + pub phantom: sp_std::marker::PhantomData<(T, I)>, + } + + #[cfg(feature = "std")] + impl, I: 'static> Default for GenesisConfig { + fn default() -> Self { + Self { + members: Default::default(), + phantom: 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) + } +} + +/// 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), +} + +#[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, } -impl, I: Instance> Module { +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 @@ -726,51 +738,41 @@ impl, I: Instance> Module { // read the length of the proposal storage entry directly let proposal_len = storage::read(&key, &mut [0; 0], 0) .ok_or(Error::::ProposalMissing)?; + ensure!(proposal_len <= length_bound, Error::::WrongProposalLength); + 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, 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 + 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(RawEvent::Disapproved(proposal_hash)); + Self::deposit_event(Event::Disapproved(proposal_hash)); Self::remove_proposal(proposal_hash) } @@ -785,26 +787,23 @@ impl, I: Instance> Module { }); 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: 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 /// 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], @@ -846,7 +845,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 +867,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 +887,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 +908,7 @@ impl< } } -pub struct EnsureProportionMoreThan( +pub struct EnsureProportionMoreThan( sp_std::marker::PhantomData<(N, D, AccountId, I)> ); impl< @@ -933,7 +932,7 @@ impl< } } -pub struct EnsureProportionAtLeast( +pub struct EnsureProportionAtLeast( sp_std::marker::PhantomData<(N, D, AccountId, I)> ); impl< @@ -957,10 +956,14 @@ 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; @@ -1043,9 +1046,9 @@ mod tests { 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}, } );