diff --git a/substrate/frame/collective/Cargo.toml b/substrate/frame/collective/Cargo.toml index 9fc5c7a3de..8828c6a61c 100644 --- a/substrate/frame/collective/Cargo.toml +++ b/substrate/frame/collective/Cargo.toml @@ -13,34 +13,35 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = [ - "derive", -] } +codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] } +log = { version = "0.4.14", default-features = false } + sp-core = { version = "4.0.0-dev", default-features = false, path = "../../primitives/core" } -sp-std = { version = "4.0.0-dev", default-features = false, path = "../../primitives/std" } sp-io = { version = "4.0.0-dev", default-features = false, path = "../../primitives/io" } sp-runtime = { version = "4.0.0-dev", default-features = false, path = "../../primitives/runtime" } +sp-std = { version = "4.0.0-dev", default-features = false, path = "../../primitives/std" } + frame-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../benchmarking", optional = true } frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } -log = { version = "0.4.14", default-features = false } [features] default = ["std"] std = [ "codec/std", - "sp-core/std", - "sp-std/std", - "sp-io/std", - "frame-support/std", - "sp-runtime/std", - "frame-system/std", "log/std", + "sp-core/std", + "sp-io/std", + "sp-runtime/std", + "sp-std/std", "frame-benchmarking/std", + "frame-support/std", + "frame-system/std", ] runtime-benchmarks = [ "frame-benchmarking", "sp-runtime/runtime-benchmarks", + "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", ] try-runtime = ["frame-support/try-runtime"] diff --git a/substrate/frame/collective/src/benchmarking.rs b/substrate/frame/collective/src/benchmarking.rs index b966279a42..1ce7750278 100644 --- a/substrate/frame/collective/src/benchmarking.rs +++ b/substrate/frame/collective/src/benchmarking.rs @@ -18,26 +18,25 @@ //! Staking pallet benchmarking. use super::*; +use crate::Pallet as Collective; -use frame_benchmarking::{ - account, benchmarks_instance, impl_benchmark_test_suite, whitelisted_caller, -}; -use frame_system::RawOrigin as SystemOrigin; use sp_runtime::traits::Bounded; use sp_std::mem::size_of; -use crate::Module as Collective; -use frame_system::{Call as SystemCall, Pallet as System}; +use frame_benchmarking::{ + account, benchmarks_instance_pallet, impl_benchmark_test_suite, whitelisted_caller, +}; +use frame_system::{Call as SystemCall, Pallet as System, RawOrigin as SystemOrigin}; 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) { frame_system::Pallet::::assert_last_event(generic_event.into()); } -benchmarks_instance! { +benchmarks_instance_pallet! { set_members { let m in 1 .. T::MaxMembers::get(); let n in 1 .. T::MaxMembers::get(); @@ -53,7 +52,7 @@ benchmarks_instance! { } let old_members_count = old_members.len() as u32; - Collective::::set_members( + Collective::::set_members( SystemOrigin::Root.into(), old_members.clone(), Some(last_old_member.clone()), @@ -67,7 +66,7 @@ benchmarks_instance! { for i in 0 .. p { // Proposals should be different so that different proposal hashes are generated let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; length]).into(); - Collective::::propose( + Collective::::propose( SystemOrigin::Signed(last_old_member.clone()).into(), threshold, Box::new(proposal.clone()), @@ -80,7 +79,7 @@ benchmarks_instance! { for j in 2 .. m - 1 { let voter = &old_members[j as usize]; let approve = true; - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(voter.clone()).into(), hash, i, @@ -101,7 +100,7 @@ benchmarks_instance! { }: _(SystemOrigin::Root, new_members.clone(), Some(last_member), T::MaxMembers::get()) verify { new_members.sort(); - assert_eq!(Collective::::members(), new_members); + assert_eq!(Collective::::members(), new_members); } execute { @@ -120,7 +119,7 @@ benchmarks_instance! { let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members, None, T::MaxMembers::get())?; + Collective::::set_members(SystemOrigin::Root.into(), members, None, T::MaxMembers::get())?; let proposal: T::Proposal = SystemCall::::remark(vec![1; b as usize]).into(); @@ -129,7 +128,7 @@ benchmarks_instance! { let proposal_hash = T::Hashing::hash_of(&proposal); // Note that execution fails due to mis-matched origin assert_last_event::( - RawEvent::MemberExecuted(proposal_hash, Err(DispatchError::BadOrigin)).into() + Event::MemberExecuted(proposal_hash, Err(DispatchError::BadOrigin)).into() ); } @@ -150,7 +149,7 @@ benchmarks_instance! { let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members, None, T::MaxMembers::get())?; + Collective::::set_members(SystemOrigin::Root.into(), members, None, T::MaxMembers::get())?; let proposal: T::Proposal = SystemCall::::remark(vec![1; b as usize]).into(); let threshold = 1; @@ -160,7 +159,7 @@ benchmarks_instance! { let proposal_hash = T::Hashing::hash_of(&proposal); // Note that execution fails due to mis-matched origin assert_last_event::( - RawEvent::Executed(proposal_hash, Err(DispatchError::BadOrigin)).into() + Event::Executed(proposal_hash, Err(DispatchError::BadOrigin)).into() ); } @@ -180,14 +179,14 @@ benchmarks_instance! { } let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members, None, T::MaxMembers::get())?; + Collective::::set_members(SystemOrigin::Root.into(), members, None, T::MaxMembers::get())?; let threshold = m; // Add previous proposals. for i in 0 .. p - 1 { // Proposals should be different so that different proposal hashes are generated let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; b as usize]).into(); - Collective::::propose( + Collective::::propose( SystemOrigin::Signed(caller.clone()).into(), threshold, Box::new(proposal), @@ -195,16 +194,16 @@ benchmarks_instance! { )?; } - assert_eq!(Collective::::proposals().len(), (p - 1) as usize); + assert_eq!(Collective::::proposals().len(), (p - 1) as usize); let proposal: T::Proposal = SystemCall::::remark(vec![p as u8; b as usize]).into(); }: propose(SystemOrigin::Signed(caller.clone()), threshold, Box::new(proposal.clone()), bytes_in_storage) verify { // New proposal is recorded - assert_eq!(Collective::::proposals().len(), p as usize); + 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::(Event::Proposed(caller, p - 1, proposal_hash, threshold).into()); } vote { @@ -225,7 +224,7 @@ benchmarks_instance! { } let voter: T::AccountId = account("voter", 0, SEED); members.push(voter.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, T::MaxMembers::get())?; + Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, T::MaxMembers::get())?; // Threshold is 1 less than the number of members so that one person can vote nay let threshold = m - 1; @@ -235,7 +234,7 @@ benchmarks_instance! { for i in 0 .. p { // Proposals should be different so that different proposal hashes are generated let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; b as usize]).into(); - Collective::::propose( + Collective::::propose( SystemOrigin::Signed(proposer.clone()).into(), threshold, Box::new(proposal.clone()), @@ -249,7 +248,7 @@ benchmarks_instance! { for j in 0 .. m - 3 { let voter = &members[j as usize]; let approve = true; - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), index, @@ -258,14 +257,14 @@ benchmarks_instance! { } // Voter votes aye without resolving the vote. let approve = true; - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), index, approve, )?; - assert_eq!(Collective::::proposals().len(), p as usize); + assert_eq!(Collective::::proposals().len(), p as usize); // Voter switches vote to nay, but does not kill the vote, just updates + inserts let approve = false; @@ -276,8 +275,8 @@ benchmarks_instance! { }: _(SystemOrigin::Signed(voter), last_hash.clone(), index, approve) 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("Proposal Missing")?; + assert_eq!(Collective::::proposals().len(), p as usize); + let voting = Collective::::voting(&last_hash).ok_or("Proposal Missing")?; assert_eq!(voting.ayes.len(), (m - 3) as usize); assert_eq!(voting.nays.len(), 1); } @@ -300,7 +299,7 @@ benchmarks_instance! { } let voter: T::AccountId = account("voter", 0, SEED); members.push(voter.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, T::MaxMembers::get())?; + Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, T::MaxMembers::get())?; // Threshold is total members so that one nay will disapprove the vote let threshold = m; @@ -310,7 +309,7 @@ benchmarks_instance! { for i in 0 .. p { // Proposals should be different so that different proposal hashes are generated let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; bytes as usize]).into(); - Collective::::propose( + Collective::::propose( SystemOrigin::Signed(proposer.clone()).into(), threshold, Box::new(proposal.clone()), @@ -324,7 +323,7 @@ benchmarks_instance! { for j in 0 .. m - 2 { let voter = &members[j as usize]; let approve = true; - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), index, @@ -333,18 +332,18 @@ benchmarks_instance! { } // Voter votes aye without resolving the vote. let approve = true; - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), index, approve, )?; - assert_eq!(Collective::::proposals().len(), p as usize); + assert_eq!(Collective::::proposals().len(), p as usize); // Voter switches vote to nay, which kills the vote let approve = false; - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), index, @@ -357,8 +356,8 @@ benchmarks_instance! { }: close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::max_value(), bytes_in_storage) verify { // The last proposal is removed. - assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Disapproved(last_hash).into()); + assert_eq!(Collective::::proposals().len(), (p - 1) as usize); + assert_last_event::(Event::Disapproved(last_hash).into()); } close_early_approved { @@ -377,7 +376,7 @@ benchmarks_instance! { } let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); - Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, T::MaxMembers::get())?; + Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, T::MaxMembers::get())?; // Threshold is 2 so any two ayes will approve the vote let threshold = 2; @@ -387,7 +386,7 @@ benchmarks_instance! { for i in 0 .. p { // Proposals should be different so that different proposal hashes are generated let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; b as usize]).into(); - Collective::::propose( + Collective::::propose( SystemOrigin::Signed(caller.clone()).into(), threshold, Box::new(proposal.clone()), @@ -397,7 +396,7 @@ benchmarks_instance! { } // Caller switches vote to nay on their own proposal, allowing them to be the deciding approval vote - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(caller.clone()).into(), last_hash.clone(), p - 1, @@ -408,7 +407,7 @@ benchmarks_instance! { for j in 2 .. m - 1 { let voter = &members[j as usize]; let approve = false; - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), p - 1, @@ -417,19 +416,19 @@ benchmarks_instance! { } // Member zero is the first aye - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(members[0].clone()).into(), last_hash.clone(), p - 1, true, )?; - assert_eq!(Collective::::proposals().len(), p as usize); + assert_eq!(Collective::::proposals().len(), p as usize); // Caller switches vote to aye, which passes the vote let index = p - 1; let approve = true; - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(caller.clone()).into(), last_hash.clone(), index, approve, @@ -438,8 +437,8 @@ benchmarks_instance! { }: close(SystemOrigin::Signed(caller), last_hash.clone(), index, Weight::max_value(), bytes_in_storage) 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_eq!(Collective::::proposals().len(), (p - 1) as usize); + assert_last_event::(Event::Executed(last_hash, Err(DispatchError::BadOrigin)).into()); } close_disapproved { @@ -458,7 +457,7 @@ benchmarks_instance! { } let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); - Collective::::set_members( + Collective::::set_members( SystemOrigin::Root.into(), members.clone(), Some(caller.clone()), @@ -473,7 +472,7 @@ benchmarks_instance! { for i in 0 .. p { // Proposals should be different so that different proposal hashes are generated let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; bytes as usize]).into(); - Collective::::propose( + Collective::::propose( SystemOrigin::Signed(caller.clone()).into(), threshold, Box::new(proposal.clone()), @@ -488,7 +487,7 @@ benchmarks_instance! { for j in 2 .. m - 1 { let voter = &members[j as usize]; let approve = true; - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), index, @@ -497,7 +496,7 @@ benchmarks_instance! { } // caller is prime, prime votes nay - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(caller.clone()).into(), last_hash.clone(), index, @@ -505,13 +504,13 @@ benchmarks_instance! { )?; System::::set_block_number(T::BlockNumber::max_value()); - assert_eq!(Collective::::proposals().len(), p as usize); + assert_eq!(Collective::::proposals().len(), p as usize); // Prime nay will close it as disapproved }: 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_eq!(Collective::::proposals().len(), (p - 1) as usize); + assert_last_event::(Event::Disapproved(last_hash).into()); } close_approved { @@ -530,7 +529,7 @@ benchmarks_instance! { } let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); - Collective::::set_members( + Collective::::set_members( SystemOrigin::Root.into(), members.clone(), Some(caller.clone()), @@ -545,7 +544,7 @@ benchmarks_instance! { for i in 0 .. p { // Proposals should be different so that different proposal hashes are generated let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; b as usize]).into(); - Collective::::propose( + Collective::::propose( SystemOrigin::Signed(caller.clone()).into(), threshold, Box::new(proposal.clone()), @@ -567,7 +566,7 @@ benchmarks_instance! { for j in 2 .. m - 1 { let voter = &members[j as usize]; let approve = false; - Collective::::vote( + Collective::::vote( SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), p - 1, @@ -577,13 +576,13 @@ benchmarks_instance! { // caller is prime, prime already votes aye by creating the proposal System::::set_block_number(T::BlockNumber::max_value()); - assert_eq!(Collective::::proposals().len(), p as usize); + assert_eq!(Collective::::proposals().len(), p as usize); // Prime aye will close it as approved }: 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_eq!(Collective::::proposals().len(), (p - 1) as usize); + assert_last_event::(Event::Executed(last_hash, Err(DispatchError::BadOrigin)).into()); } disapprove_proposal { @@ -601,7 +600,7 @@ benchmarks_instance! { } let caller: T::AccountId = account("caller", 0, SEED); members.push(caller.clone()); - Collective::::set_members( + Collective::::set_members( SystemOrigin::Root.into(), members.clone(), Some(caller.clone()), @@ -616,7 +615,7 @@ benchmarks_instance! { for i in 0 .. p { // Proposals should be different so that different proposal hashes are generated let proposal: T::Proposal = SystemCall::::remark(vec![i as u8; b as usize]).into(); - Collective::::propose( + Collective::::propose( SystemOrigin::Signed(caller.clone()).into(), threshold, Box::new(proposal.clone()), @@ -626,12 +625,12 @@ benchmarks_instance! { } System::::set_block_number(T::BlockNumber::max_value()); - assert_eq!(Collective::::proposals().len(), p as usize); + assert_eq!(Collective::::proposals().len(), p as usize); }: _(SystemOrigin::Root, last_hash) verify { - assert_eq!(Collective::::proposals().len(), (p - 1) as usize); - assert_last_event::(RawEvent::Disapproved(last_hash).into()); + assert_eq!(Collective::::proposals().len(), (p - 1) as usize); + assert_last_event::(Event::Disapproved(last_hash).into()); } } diff --git a/substrate/frame/collective/src/lib.rs b/substrate/frame/collective/src/lib.rs index fc40fd5541..fa537a0a43 100644 --- a/substrate/frame/collective/src/lib.rs +++ b/substrate/frame/collective/src/lib.rs @@ -45,29 +45,27 @@ use sp_core::u32_trait::Value as U32; use sp_io::storage; use sp_runtime::{traits::Hash, RuntimeDebug}; -use sp_std::{prelude::*, result}; +use sp_std::{marker::PhantomData, prelude::*, result}; use frame_support::{ codec::{Decode, Encode}, - decl_error, decl_event, decl_module, decl_storage, - dispatch::{ - DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable, Parameter, - PostDispatchInfo, - }, + dispatch::{DispatchError, DispatchResultWithPostInfo, Dispatchable, PostDispatchInfo}, ensure, - traits::{Backing, ChangeMembers, EnsureOrigin, Get, GetBacking, InitializeMembers}, - weights::{DispatchClass, GetDispatchInfo, Pays, Weight}, - BoundedVec, + traits::{ + Backing, ChangeMembers, EnsureOrigin, Get, GetBacking, InitializeMembers, StorageVersion, + }, + weights::{GetDispatchInfo, Weight}, }; -use frame_system::{self as system, ensure_root, ensure_signed}; #[cfg(test)] mod tests; #[cfg(feature = "runtime-benchmarks")] mod benchmarking; - +pub mod migrations; pub mod weights; + +pub use pallet::*; pub use weights::WeightInfo; /// Simple index type for proposal counting. @@ -125,39 +123,6 @@ 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; - - /// The outer event type. - type Event: From> + Into<::Event>; - - /// The time-out for council motions. - type MotionDuration: Get; - - /// Maximum number of proposals allowed to be active in parallel. - type MaxProposals: Get; - - /// 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; - - /// Default vote strategy of this collective. - type DefaultVote: DefaultVote; - - /// Weight information for extrinsics in this pallet. - type WeightInfo: WeightInfo; -} - /// Origin for the collective module. #[derive(PartialEq, Eq, Clone, RuntimeDebug, Encode, Decode)] pub enum RawOrigin { @@ -166,7 +131,7 @@ pub enum RawOrigin { /// 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), + _Phantom(PhantomData), } impl GetBacking for RawOrigin { @@ -178,11 +143,8 @@ impl GetBacking for RawOrigin { } } -/// 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. +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)] pub struct Votes { /// The proposal's unique index. index: ProposalIndex, @@ -196,69 +158,155 @@ pub struct Votes { end: BlockNumber, } -decl_storage! { - trait Store for Module, I: Instance=DefaultInstance> as Collective { - /// The hashes of the active proposals. - pub Proposals get(fn proposals): BoundedVec; - /// 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; +#[frame_support::pallet] +pub mod pallet { + use super::*; + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + + /// The current storage version. + const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + #[pallet::storage_version(STORAGE_VERSION)] + pub struct Pallet(PhantomData<(T, I)>); + + #[pallet::config] + 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; + + /// The outer event type. + type Event: From> + IsType<::Event>; + + /// The time-out for council motions. + type MotionDuration: Get; + + /// Maximum number of proposals allowed to be active in parallel. + type MaxProposals: Get; + + /// 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; + + /// Default vote strategy of this collective. + type DefaultVote: DefaultVote; + + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; } - add_extra_genesis { - config(phantom): sp_std::marker::PhantomData; - config(members): Vec; - build(|config| { + + #[pallet::genesis_config] + pub struct GenesisConfig, I: 'static = ()> { + pub phantom: PhantomData, + pub members: Vec, + } + + #[cfg(feature = "std")] + impl, I: 'static> Default for GenesisConfig { + fn default() -> Self { + Self { phantom: Default::default(), members: Default::default() } + } + } + + #[pallet::genesis_build] + impl, I: 'static> GenesisBuild for GenesisConfig { + fn build(&self) { use sp_std::collections::btree_set::BTreeSet; - let members_set: BTreeSet<_> = config.members.iter().collect(); - assert!(members_set.len() == config.members.len(), "Members cannot contain duplicate accounts."); + let members_set: BTreeSet<_> = self.members.iter().collect(); + assert_eq!( + members_set.len(), + self.members.len(), + "Members cannot contain duplicate accounts." + ); - Module::::initialize_members(&config.members) - }); + Pallet::::initialize_members(&self.members) + } } -} -decl_event! { - pub enum Event where - ::Hash, - ::AccountId, - { + /// Origin for the collective pallet. + #[pallet::origin] + pub type Origin = RawOrigin<::AccountId, I>; + + /// The hashes of the active proposals. + #[pallet::storage] + #[pallet::getter(fn proposals)] + pub type Proposals, I: 'static = ()> = + StorageValue<_, BoundedVec, 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, >::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::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(AccountId, ProposalIndex, Hash, MemberCount), + 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(AccountId, Hash, bool, MemberCount, MemberCount), + Voted(T::AccountId, T::Hash, bool, MemberCount, MemberCount), /// A motion was approved by the required threshold. /// \[proposal_hash\] - Approved(Hash), + Approved(T::Hash), /// A motion was not approved by the required threshold. /// \[proposal_hash\] - Disapproved(Hash), + Disapproved(T::Hash), /// A motion was executed; result will be `Ok` if it returned without error. /// \[proposal_hash, result\] - Executed(Hash, DispatchResult), + Executed(T::Hash, DispatchResult), /// A single member did some action; result will be `Ok` if it returned without error. /// \[proposal_hash, result\] - MemberExecuted(Hash, DispatchResult), + MemberExecuted(T::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), + Closed(T::Hash, MemberCount, MemberCount), } -} -decl_error! { - pub enum Error for Module, I: Instance> { + /// 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 @@ -280,31 +328,16 @@ decl_error! { /// The given length bound for the proposal was too low. WrongProposalLength, } -} - -/// 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, - } -} - -// 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; + // Note that councillor operations are assigned to the operational class. + #[pallet::call] + impl, I: 'static> Pallet { /// Set the collective's membership. /// /// - `new_members`: The new member list. Be nice to the chain and provide it sorted. /// - `prime`: The prime member whose vote sets the default. - /// - `old_count`: The upper bound for the previous number of members in storage. - /// Used for weight estimation. + /// - `old_count`: The upper bound for the previous number of members in storage. Used for + /// weight estimation. /// /// Requires root origin. /// @@ -318,20 +351,22 @@ decl_module! { /// - `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 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, @@ -361,10 +396,11 @@ decl_module! { Prime::::set(prime); Ok(Some(T::WeightInfo::set_members( - old.len() as u32, // M + old.len() as u32, // M new_members.len() as u32, // N - T::MaxProposals::get(), // P - )).into()) + T::MaxProposals::get(), // P + )) + .into()) } /// Dispatch a proposal from a member using the `Member` origin. @@ -373,20 +409,22 @@ decl_module! { /// /// # /// ## Weight - /// - `O(M + P)` where `M` members-count (code-bounded) and `P` complexity of dispatching `proposal` + /// - `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(); @@ -396,16 +434,20 @@ decl_module! { 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)) - ); + Self::deposit_event(Event::MemberExecuted( + proposal_hash, + result.map(|_| ()).map_err(|e| e.error), + )); - Ok(get_result_weight(result).map(|w| { - T::WeightInfo::execute( - proposal_len as u32, // B - members.len() as u32, // M - ).saturating_add(w) // P - }).into()) + Ok(get_result_weight(result) + .map(|w| { + T::WeightInfo::execute( + proposal_len as u32, // B + members.len() as u32, // M + ) + .saturating_add(w) // P + }) + .into()) } /// Add a new proposal to either be voted on or executed directly. @@ -435,7 +477,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 @@ -449,11 +491,12 @@ decl_module! { ) }, DispatchClass::Operational - )] - fn propose(origin, - #[compact] threshold: MemberCount, + ))] + pub fn propose( + origin: OriginFor, + #[pallet::compact] threshold: MemberCount, proposal: Box<>::Proposal>, - #[compact] length_bound: u32 + #[pallet::compact] length_bound: u32, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let members = Self::members(); @@ -462,43 +505,53 @@ decl_module! { let proposal_len = proposal.using_encoded(|x| x.len()); ensure!(proposal_len <= length_bound as usize, Error::::WrongProposalLength); let proposal_hash = T::Hashing::hash_of(&proposal); - ensure!(!>::contains_key(proposal_hash), Error::::DuplicateProposal); + ensure!( + !>::contains_key(proposal_hash), + Error::::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)) - ); + Self::deposit_event(Event::Executed( + proposal_hash, + result.map(|_| ()).map_err(|e| e.error), + )); - Ok(get_result_weight(result).map(|w| { - T::WeightInfo::propose_execute( - proposal_len as u32, // B - members.len() as u32, // M - ).saturating_add(w) // P1 - }).into()) + Ok(get_result_weight(result) + .map(|w| { + T::WeightInfo::propose_execute( + proposal_len as u32, // B + members.len() as u32, // M + ) + .saturating_add(w) // P1 + }) + .into()) } else { let active_proposals = >::try_mutate(|proposals| -> Result { - proposals.try_push(proposal_hash).map_err(|_| Error::::TooManyProposals)?; + proposals + .try_push(proposal_hash) + .map_err(|_| Error::::TooManyProposals)?; Ok(proposals.len()) })?; let index = Self::proposal_count(); - >::mutate(|i| *i += 1); + >::mutate(|i| *i += 1); >::insert(proposal_hash, *proposal); let votes = { - let end = system::Pallet::::block_number() + T::MotionDuration::get(); + let end = frame_system::Pallet::::block_number() + T::MotionDuration::get(); Votes { index, threshold, ayes: vec![], 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 - members.len() as u32, // M + proposal_len as u32, // B + members.len() as u32, // M active_proposals as u32, // P2 - )).into()) + )) + .into()) } } @@ -507,7 +560,8 @@ decl_module! { /// Requires the sender to be a member. /// /// 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. + /// 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) @@ -516,13 +570,11 @@ decl_module! { /// - 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()), DispatchClass::Operational))] + pub fn vote( + origin: OriginFor, proposal: T::Hash, - #[compact] index: ProposalIndex, + #[pallet::compact] index: ProposalIndex, approve: bool, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; @@ -542,7 +594,7 @@ decl_module! { if position_yes.is_none() { voting.ayes.push(who.clone()); } else { - Err(Error::::DuplicateVote)? + return Err(Error::::DuplicateVote.into()) } if let Some(pos) = position_no { voting.nays.swap_remove(pos); @@ -551,7 +603,7 @@ decl_module! { if position_no.is_none() { voting.nays.push(who.clone()); } else { - Err(Error::::DuplicateVote)? + return Err(Error::::DuplicateVote.into()) } if let Some(pos) = position_yes { voting.ayes.swap_remove(pos); @@ -560,20 +612,14 @@ 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); if is_account_voting_first_time { - Ok(( - Some(T::WeightInfo::vote(members.len() as u32)), - Pays::No, - ).into()) + Ok((Some(T::WeightInfo::vote(members.len() as u32)), Pays::No).into()) } else { - Ok(( - Some(T::WeightInfo::vote(members.len() as u32)), - Pays::Yes, - ).into()) + Ok((Some(T::WeightInfo::vote(members.len() as u32)), Pays::Yes).into()) } } @@ -590,9 +636,10 @@ decl_module! { /// If the close operation completes successfully with disapproval, the transaction fee will /// be waived. Otherwise execution of the approved operation will be charged to the caller. /// - /// + `proposal_weight_bound`: The maximum amount of weight consumed by executing the closed proposal. + /// + `proposal_weight_bound`: The maximum amount of weight consumed by executing the closed + /// proposal. /// + `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. + /// `storage::read` so it is `size_of::() == 4` larger than the pure length. /// /// # /// ## Weight @@ -603,11 +650,12 @@ decl_module! { /// - `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)`) + /// - 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(); @@ -620,12 +668,13 @@ decl_module! { .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)?; @@ -644,26 +693,32 @@ 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, yes_votes, proposal_hash, proposal); return Ok(( - Some(T::WeightInfo::close_early_approved(len as u32, seats, proposal_count) - .saturating_add(proposal_weight)), + Some( + T::WeightInfo::close_early_approved(len as u32, seats, proposal_count) + .saturating_add(proposal_weight), + ), Pays::Yes, - ).into()); - + ) + .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)), Pays::No, - ).into()); + ) + .into()) } // Only allow actual closing of the proposal after the voting period has ended. - ensure!(system::Pallet::::block_number() >= voting.end, Error::::TooEarly); + ensure!( + frame_system::Pallet::::block_number() >= voting.end, + Error::::TooEarly + ); let prime_vote = Self::prime().map(|who| voting.ayes.iter().any(|a| a == &who)); @@ -683,25 +738,26 @@ 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, yes_votes, proposal_hash, proposal); - return Ok(( - Some(T::WeightInfo::close_approved(len as u32, seats, proposal_count) - .saturating_add(proposal_weight)), + Ok(( + Some( + T::WeightInfo::close_approved(len as u32, seats, proposal_count) + .saturating_add(proposal_weight), + ), Pays::Yes, - ).into()); + ) + .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)), - Pays::No, - ).into()); + Ok((Some(T::WeightInfo::close_disapproved(seats, proposal_count)), Pays::No).into()) } } - /// 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. /// @@ -714,8 +770,11 @@ decl_module! { /// * 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()) @@ -723,7 +782,17 @@ decl_module! { } } -impl, I: Instance> Module { +/// 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 @@ -771,12 +840,12 @@ 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(yes_votes, seats).into(); let result = proposal.dispatch(origin); - Self::deposit_event(RawEvent::Executed( + Self::deposit_event(Event::Executed( proposal_hash, result.map(|_| ()).map_err(|e| e.error), )); @@ -789,7 +858,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) } @@ -806,7 +875,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 @@ -870,7 +939,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!"); @@ -894,9 +963,7 @@ where } } -pub struct EnsureMember( - sp_std::marker::PhantomData<(AccountId, I)>, -); +pub struct EnsureMember(PhantomData<(AccountId, I)>); impl< O: Into, O>> + From>, AccountId: Default, @@ -917,9 +984,7 @@ impl< } } -pub struct EnsureMembers( - sp_std::marker::PhantomData<(N, AccountId, I)>, -); +pub struct EnsureMembers(PhantomData<(N, AccountId, I)>); impl< O: Into, O>> + From>, N: U32, @@ -941,8 +1006,8 @@ impl< } } -pub struct EnsureProportionMoreThan( - sp_std::marker::PhantomData<(N, D, AccountId, I)>, +pub struct EnsureProportionMoreThan( + PhantomData<(N, D, AccountId, I)>, ); impl< O: Into, O>> + From>, @@ -966,8 +1031,8 @@ impl< } } -pub struct EnsureProportionAtLeast( - sp_std::marker::PhantomData<(N, D, AccountId, I)>, +pub struct EnsureProportionAtLeast( + PhantomData<(N, D, AccountId, I)>, ); impl< O: Into, O>> + From>, diff --git a/substrate/frame/collective/src/migrations/mod.rs b/substrate/frame/collective/src/migrations/mod.rs new file mode 100644 index 0000000000..26d07a0cd5 --- /dev/null +++ b/substrate/frame/collective/src/migrations/mod.rs @@ -0,0 +1,19 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-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. + +/// Version 4. +pub mod v4; diff --git a/substrate/frame/collective/src/migrations/v4.rs b/substrate/frame/collective/src/migrations/v4.rs new file mode 100644 index 0000000000..68284ba4df --- /dev/null +++ b/substrate/frame/collective/src/migrations/v4.rs @@ -0,0 +1,147 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-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. + +use sp_io::hashing::twox_128; + +use frame_support::{ + traits::{ + Get, GetStorageVersion, PalletInfoAccess, StorageVersion, + STORAGE_VERSION_STORAGE_KEY_POSTFIX, + }, + weights::Weight, +}; + +/// Migrate the entire storage of this pallet to a new prefix. +/// +/// This new prefix must be the same as the one set in construct_runtime. For safety, use +/// `PalletInfo` to get it, as: +/// `::PalletInfo::name::`. +/// +/// The migration will look into the storage version in order not to trigger a migration on an up +/// to date storage. Thus the on chain storage version must be less than 4 in order to trigger the +/// migration. +pub fn migrate>( + old_pallet_name: N, +) -> Weight { + let old_pallet_name = old_pallet_name.as_ref(); + let new_pallet_name =

::name(); + + if new_pallet_name == old_pallet_name { + log::info!( + target: "runtime::collective", + "New pallet name is equal to the old pallet name. No migration needs to be done.", + ); + return 0 + } + + let on_chain_storage_version =

::on_chain_storage_version(); + log::info!( + target: "runtime::collective", + "Running migration to v4 for collective with storage version {:?}", + on_chain_storage_version, + ); + + if on_chain_storage_version < 4 { + frame_support::storage::migration::move_pallet( + old_pallet_name.as_bytes(), + new_pallet_name.as_bytes(), + ); + log_migration("migration", old_pallet_name, new_pallet_name); + + StorageVersion::new(4).put::

(); + ::BlockWeights::get().max_block + } else { + log::warn!( + target: "runtime::collective", + "Attempted to apply migration to v4 but failed because storage version is {:?}", + on_chain_storage_version, + ); + 0 + } +} + +/// Some checks prior to migration. This can be linked to +/// [`frame_support::traits::OnRuntimeUpgrade::pre_upgrade`] for further testing. +/// +/// Panics if anything goes wrong. +pub fn pre_migrate>(old_pallet_name: N) { + let old_pallet_name = old_pallet_name.as_ref(); + let new_pallet_name =

::name(); + log_migration("pre-migration", old_pallet_name, new_pallet_name); + + if new_pallet_name == old_pallet_name { + return + } + + let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); + let storage_version_key = twox_128(STORAGE_VERSION_STORAGE_KEY_POSTFIX); + + let mut new_pallet_prefix_iter = frame_support::storage::KeyPrefixIterator::new( + new_pallet_prefix.to_vec(), + new_pallet_prefix.to_vec(), + |key| Ok(key.to_vec()), + ); + + // Ensure nothing except the storage_version_key is stored in the new prefix. + assert!(new_pallet_prefix_iter.all(|key| key == storage_version_key)); + + assert!(

::on_chain_storage_version() < 4); +} + +/// Some checks for after migration. This can be linked to +/// [`frame_support::traits::OnRuntimeUpgrade::post_upgrade`] for further testing. +/// +/// Panics if anything goes wrong. +pub fn post_migrate>(old_pallet_name: N) { + let old_pallet_name = old_pallet_name.as_ref(); + let new_pallet_name =

::name(); + log_migration("post-migration", old_pallet_name, new_pallet_name); + + if new_pallet_name == old_pallet_name { + return + } + + // Assert that nothing remains at the old prefix. + let old_pallet_prefix = twox_128(old_pallet_name.as_bytes()); + let old_pallet_prefix_iter = frame_support::storage::KeyPrefixIterator::new( + old_pallet_prefix.to_vec(), + old_pallet_prefix.to_vec(), + |_| Ok(()), + ); + assert_eq!(old_pallet_prefix_iter.count(), 0); + + // NOTE: storage_version_key is already in the new prefix. + let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); + let new_pallet_prefix_iter = frame_support::storage::KeyPrefixIterator::new( + new_pallet_prefix.to_vec(), + new_pallet_prefix.to_vec(), + |_| Ok(()), + ); + assert!(new_pallet_prefix_iter.count() >= 1); + + assert_eq!(

::on_chain_storage_version(), 4); +} + +fn log_migration(stage: &str, old_pallet_name: &str, new_pallet_name: &str) { + log::info!( + target: "runtime::collective", + "{}, prefix: '{}' ==> '{}'", + stage, + old_pallet_name, + new_pallet_name, + ); +} diff --git a/substrate/frame/collective/src/tests.rs b/substrate/frame/collective/src/tests.rs index aa6ea090f4..5c662428fd 100644 --- a/substrate/frame/collective/src/tests.rs +++ b/substrate/frame/collective/src/tests.rs @@ -15,10 +15,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -use super::*; -use crate as collective; -use frame_support::{assert_noop, assert_ok, parameter_types, Hashable}; -use frame_system::{self as system, EventRecord, Phase}; +use super::{Event as CollectiveEvent, *}; +use crate as pallet_collective; +use frame_support::{ + assert_noop, assert_ok, parameter_types, traits::GenesisBuild, weights::Pays, Hashable, +}; +use frame_system::{EventRecord, Phase}; use sp_core::{ u32_trait::{_3, _4}, H256, @@ -38,10 +40,10 @@ frame_support::construct_runtime!( NodeBlock = Block, UncheckedExtrinsic = UncheckedExtrinsic { - System: system::{Pallet, Call, Event}, - Collective: collective::::{Pallet, Call, Event, Origin, Config}, - CollectiveMajority: collective::::{Pallet, Call, Event, Origin, Config}, - DefaultCollective: collective::{Pallet, Call, Event, Origin, Config}, + System: frame_system::{Pallet, Call, Event}, + Collective: pallet_collective::::{Pallet, Call, Event, Origin, Config}, + CollectiveMajority: pallet_collective::::{Pallet, Call, Event, Origin, Config}, + DefaultCollective: pallet_collective::{Pallet, Call, Event, Origin, Config}, Democracy: mock_democracy::{Pallet, Call, Event}, } ); @@ -152,11 +154,11 @@ impl Config for Test { pub fn new_test_ext() -> sp_io::TestExternalities { let mut ext: sp_io::TestExternalities = GenesisConfig { - collective: collective::GenesisConfig { + collective: pallet_collective::GenesisConfig { members: vec![1, 2, 3], phantom: Default::default(), }, - collective_majority: collective::GenesisConfig { + collective_majority: pallet_collective::GenesisConfig { members: vec![1, 2, 3, 4, 5], phantom: Default::default(), }, @@ -214,11 +216,11 @@ fn close_works() { assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3))), - record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), - record(Event::Collective(RawEvent::Closed(hash, 2, 1))), - record(Event::Collective(RawEvent::Disapproved(hash))) + record(Event::Collective(CollectiveEvent::Proposed(1, 0, hash, 3))), + record(Event::Collective(CollectiveEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(CollectiveEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(CollectiveEvent::Closed(hash, 2, 1))), + record(Event::Collective(CollectiveEvent::Disapproved(hash))) ] ); }); @@ -307,11 +309,11 @@ fn close_with_prime_works() { assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3))), - record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), - record(Event::Collective(RawEvent::Closed(hash, 2, 1))), - record(Event::Collective(RawEvent::Disapproved(hash))) + record(Event::Collective(CollectiveEvent::Proposed(1, 0, hash, 3))), + record(Event::Collective(CollectiveEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(CollectiveEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(CollectiveEvent::Closed(hash, 2, 1))), + record(Event::Collective(CollectiveEvent::Disapproved(hash))) ] ); }); @@ -346,12 +348,15 @@ fn close_with_voting_prime_works() { assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3))), - record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), - record(Event::Collective(RawEvent::Closed(hash, 3, 0))), - record(Event::Collective(RawEvent::Approved(hash))), - record(Event::Collective(RawEvent::Executed(hash, Err(DispatchError::BadOrigin)))) + record(Event::Collective(CollectiveEvent::Proposed(1, 0, hash, 3))), + record(Event::Collective(CollectiveEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(CollectiveEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(CollectiveEvent::Closed(hash, 3, 0))), + record(Event::Collective(CollectiveEvent::Approved(hash))), + record(Event::Collective(CollectiveEvent::Executed( + hash, + Err(DispatchError::BadOrigin) + ))) ] ); }); @@ -393,13 +398,13 @@ fn close_with_no_prime_but_majority_works() { assert_eq!( System::events(), vec![ - record(Event::CollectiveMajority(RawEvent::Proposed(1, 0, hash, 5))), - record(Event::CollectiveMajority(RawEvent::Voted(1, hash, true, 1, 0))), - record(Event::CollectiveMajority(RawEvent::Voted(2, hash, true, 2, 0))), - record(Event::CollectiveMajority(RawEvent::Voted(3, hash, true, 3, 0))), - record(Event::CollectiveMajority(RawEvent::Closed(hash, 5, 0))), - record(Event::CollectiveMajority(RawEvent::Approved(hash))), - record(Event::CollectiveMajority(RawEvent::Executed( + record(Event::CollectiveMajority(CollectiveEvent::Proposed(1, 0, hash, 5))), + record(Event::CollectiveMajority(CollectiveEvent::Voted(1, hash, true, 1, 0))), + record(Event::CollectiveMajority(CollectiveEvent::Voted(2, hash, true, 2, 0))), + record(Event::CollectiveMajority(CollectiveEvent::Voted(3, hash, true, 3, 0))), + record(Event::CollectiveMajority(CollectiveEvent::Closed(hash, 5, 0))), + record(Event::CollectiveMajority(CollectiveEvent::Approved(hash))), + record(Event::CollectiveMajority(CollectiveEvent::Executed( hash, Err(DispatchError::BadOrigin) ))) @@ -526,7 +531,7 @@ fn propose_works() { assert_eq!( System::events(), - vec![record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3)))] + vec![record(Event::Collective(CollectiveEvent::Proposed(1, 0, hash, 3)))] ); }); } @@ -682,9 +687,9 @@ fn motions_vote_after_works() { assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash, 2))), - record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), - record(Event::Collective(RawEvent::Voted(1, hash, false, 0, 1))), + record(Event::Collective(CollectiveEvent::Proposed(1, 0, hash, 2))), + record(Event::Collective(CollectiveEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(CollectiveEvent::Voted(1, hash, false, 0, 1))), ] ); }); @@ -798,12 +803,15 @@ fn motions_approval_with_enough_votes_and_lower_voting_threshold_works() { assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash, 2))), - record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), - record(Event::Collective(RawEvent::Closed(hash, 2, 0))), - record(Event::Collective(RawEvent::Approved(hash))), - record(Event::Collective(RawEvent::Executed(hash, Err(DispatchError::BadOrigin)))), + record(Event::Collective(CollectiveEvent::Proposed(1, 0, hash, 2))), + record(Event::Collective(CollectiveEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(CollectiveEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(CollectiveEvent::Closed(hash, 2, 0))), + record(Event::Collective(CollectiveEvent::Approved(hash))), + record(Event::Collective(CollectiveEvent::Executed( + hash, + Err(DispatchError::BadOrigin) + ))), ] ); @@ -823,14 +831,14 @@ fn motions_approval_with_enough_votes_and_lower_voting_threshold_works() { assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 1, hash, 2))), - record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), - record(Event::Collective(RawEvent::Voted(3, hash, true, 3, 0))), - record(Event::Collective(RawEvent::Closed(hash, 3, 0))), - record(Event::Collective(RawEvent::Approved(hash))), + record(Event::Collective(CollectiveEvent::Proposed(1, 1, hash, 2))), + record(Event::Collective(CollectiveEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(CollectiveEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(CollectiveEvent::Voted(3, hash, true, 3, 0))), + record(Event::Collective(CollectiveEvent::Closed(hash, 3, 0))), + record(Event::Collective(CollectiveEvent::Approved(hash))), record(Event::Democracy(mock_democracy::pallet::Event::::ExternalProposed)), - record(Event::Collective(RawEvent::Executed(hash, Ok(())))), + record(Event::Collective(CollectiveEvent::Executed(hash, Ok(())))), ] ); }); @@ -856,11 +864,11 @@ fn motions_disapproval_works() { assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3))), - record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash, false, 1, 1))), - record(Event::Collective(RawEvent::Closed(hash, 1, 1))), - record(Event::Collective(RawEvent::Disapproved(hash))), + record(Event::Collective(CollectiveEvent::Proposed(1, 0, hash, 3))), + record(Event::Collective(CollectiveEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(CollectiveEvent::Voted(2, hash, false, 1, 1))), + record(Event::Collective(CollectiveEvent::Closed(hash, 1, 1))), + record(Event::Collective(CollectiveEvent::Disapproved(hash))), ] ); }); @@ -886,12 +894,15 @@ fn motions_approval_works() { assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash, 2))), - record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), - record(Event::Collective(RawEvent::Closed(hash, 2, 0))), - record(Event::Collective(RawEvent::Approved(hash))), - record(Event::Collective(RawEvent::Executed(hash, Err(DispatchError::BadOrigin)))), + record(Event::Collective(CollectiveEvent::Proposed(1, 0, hash, 2))), + record(Event::Collective(CollectiveEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(CollectiveEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(CollectiveEvent::Closed(hash, 2, 0))), + record(Event::Collective(CollectiveEvent::Approved(hash))), + record(Event::Collective(CollectiveEvent::Executed( + hash, + Err(DispatchError::BadOrigin) + ))), ] ); }); @@ -912,7 +923,7 @@ fn motion_with_no_votes_closes_with_disapproval() { )); assert_eq!( System::events()[0], - record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3))) + record(Event::Collective(CollectiveEvent::Proposed(1, 0, hash, 3))) ); // Closing the motion too early is not possible because it has neither @@ -929,8 +940,14 @@ fn motion_with_no_votes_closes_with_disapproval() { assert_ok!(Collective::close(Origin::signed(2), hash, 0, proposal_weight, proposal_len)); // Events show that the close ended in a disapproval. - assert_eq!(System::events()[1], record(Event::Collective(RawEvent::Closed(hash, 0, 3)))); - assert_eq!(System::events()[2], record(Event::Collective(RawEvent::Disapproved(hash)))); + assert_eq!( + System::events()[1], + record(Event::Collective(CollectiveEvent::Closed(hash, 0, 3))) + ); + assert_eq!( + System::events()[2], + record(Event::Collective(CollectiveEvent::Disapproved(hash))) + ); }) } @@ -989,10 +1006,10 @@ fn disapprove_proposal_works() { assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash, 2))), - record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), - record(Event::Collective(RawEvent::Disapproved(hash))), + record(Event::Collective(CollectiveEvent::Proposed(1, 0, hash, 2))), + record(Event::Collective(CollectiveEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(CollectiveEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(CollectiveEvent::Disapproved(hash))), ] ); }) @@ -1001,7 +1018,53 @@ fn disapprove_proposal_works() { #[test] #[should_panic(expected = "Members cannot contain duplicate accounts.")] fn genesis_build_panics_with_duplicate_members() { - collective::GenesisConfig:: { members: vec![1, 2, 3, 1], phantom: Default::default() } - .build_storage() - .unwrap(); + pallet_collective::GenesisConfig:: { + members: vec![1, 2, 3, 1], + phantom: Default::default(), + } + .build_storage() + .unwrap(); +} + +#[test] +fn migration_v4() { + new_test_ext().execute_with(|| { + use frame_support::traits::PalletInfoAccess; + + let old_pallet = "OldCollective"; + let new_pallet = ::name(); + frame_support::storage::migration::move_pallet( + new_pallet.as_bytes(), + old_pallet.as_bytes(), + ); + StorageVersion::new(0).put::(); + + crate::migrations::v4::pre_migrate::(old_pallet); + crate::migrations::v4::migrate::(old_pallet); + crate::migrations::v4::post_migrate::(old_pallet); + + let old_pallet = "OldCollectiveMajority"; + let new_pallet = ::name(); + frame_support::storage::migration::move_pallet( + new_pallet.as_bytes(), + old_pallet.as_bytes(), + ); + StorageVersion::new(0).put::(); + + crate::migrations::v4::pre_migrate::(old_pallet); + crate::migrations::v4::migrate::(old_pallet); + crate::migrations::v4::post_migrate::(old_pallet); + + let old_pallet = "OldDefaultCollective"; + let new_pallet = ::name(); + frame_support::storage::migration::move_pallet( + new_pallet.as_bytes(), + old_pallet.as_bytes(), + ); + StorageVersion::new(0).put::(); + + crate::migrations::v4::pre_migrate::(old_pallet); + crate::migrations::v4::migrate::(old_pallet); + crate::migrations::v4::post_migrate::(old_pallet); + }); }