diff --git a/node/src/chain_spec.rs b/node/src/chain_spec.rs index b05c32f933..32a4152ef0 100644 --- a/node/src/chain_spec.rs +++ b/node/src/chain_spec.rs @@ -62,6 +62,12 @@ pub fn pint_development_config(id: ParaId) -> ChainSpec { get_account_id_from_seed::("Alice//stash"), get_account_id_from_seed::("Bob//stash"), ], + vec![ + get_account_id_from_seed::("Alice"), + get_account_id_from_seed::("Bob"), + get_account_id_from_seed::("Charlie"), + get_account_id_from_seed::("Dave"), + ], id, ) }, @@ -100,6 +106,12 @@ pub fn pint_local_config(id: ParaId) -> ChainSpec { get_account_id_from_seed::("Eve//stash"), get_account_id_from_seed::("Ferdie//stash"), ], + vec![ + get_account_id_from_seed::("Alice"), + get_account_id_from_seed::("Bob"), + get_account_id_from_seed::("Charlie"), + get_account_id_from_seed::("Dave"), + ], id, ) }, @@ -117,6 +129,7 @@ pub fn pint_local_config(id: ParaId) -> ChainSpec { fn pint_testnet_genesis( root_key: AccountId, endowed_accounts: Vec, + council_members: Vec, id: ParaId, ) -> parachain_runtime::GenesisConfig { parachain_runtime::GenesisConfig { @@ -126,6 +139,10 @@ fn pint_testnet_genesis( .to_vec(), changes_trie_config: Default::default(), }, + pallet_committee: parachain_runtime::CommitteeConfig { + council_members, + ..Default::default() + }, pallet_balances: parachain_runtime::BalancesConfig { balances: endowed_accounts .iter() diff --git a/pallets/committee/src/benchmarking.rs b/pallets/committee/src/benchmarking.rs index 42246bdcf1..b70b94a754 100644 --- a/pallets/committee/src/benchmarking.rs +++ b/pallets/committee/src/benchmarking.rs @@ -1,35 +1,41 @@ // Copyright 2021 ChainSafe Systems // SPDX-License-Identifier: LGPL-3.0-only use super::*; -use frame_benchmarking::{account, benchmarks, vec, whitelisted_caller, Box}; -use frame_support::{assert_noop, assert_ok, traits::Get}; -use frame_system::{Call as SystemCall, Pallet as System, RawOrigin as SystemOrigin}; +use frame_benchmarking::{account, benchmarks, vec, Box}; +use frame_support::{ + assert_noop, assert_ok, + traits::{EnsureOrigin, Get, UnfilteredDispatchable}, +}; +use frame_system::{ + ensure_signed, Call as SystemCall, Pallet as System, RawOrigin as SystemOrigin, +}; -fn submit_proposal(caller: T::AccountId) -> pallet::Proposal { +fn submit_proposal(origin: ::Origin) -> pallet::Proposal { let action: T::Action = >::remark(vec![0; 0]).into(); let expected_nonce = pallet::ProposalCount::::get(); - assert_ok!(>::propose( - SystemOrigin::Signed(caller).into(), - Box::new(action.clone()) + assert_ok!(>::add_constituent( + SystemOrigin::Root.into(), + ensure_signed(origin.clone()).unwrap(), )); + let call = >::propose(Box::new(action.clone())); + assert_ok!(call.dispatch_bypass_filter(origin)); pallet::Proposal::::new(expected_nonce, action) } benchmarks! { propose { - let caller: T::AccountId = whitelisted_caller(); - let proposal = submit_proposal::(caller.clone()); - }: _( - SystemOrigin::Signed(caller.clone()), - Box::new(>::remark(vec![0; 0]).into()) - ) verify { + let origin = T::ProposalSubmissionOrigin::successful_origin(); + let proposal = submit_proposal::(origin.clone()); + let call = >::propose(Box::new(>::remark(vec![0; 0]).into())); + }: { + call.dispatch_bypass_filter(origin)? + } verify { assert!(>::get_proposal(&proposal.hash()) == Some(proposal)); } vote { - let caller: T::AccountId = whitelisted_caller(); - let proposal = submit_proposal::(caller.clone()); - assert_ok!(>::add_constituent(SystemOrigin::Root.into(), caller.clone())); + let origin = T::ProposalSubmissionOrigin::successful_origin(); + let proposal = submit_proposal::(origin.clone()); // run to voting period >::set_block_number( @@ -37,11 +43,12 @@ benchmarks! { + ::VotingPeriod::get() + ::ProposalSubmissionPeriod::get() + 1_u32.into(), ); - }: _( - SystemOrigin::Signed(caller.clone()), - proposal.hash(), - Vote::Abstain - ) verify { + + // construct call + let call = >::vote(proposal.hash(), Vote::Abstain); + }: { + call.dispatch_bypass_filter(origin)? + } verify { assert_eq!( >::get_votes_for(&proposal.hash()).unwrap().votes.len(), 1, @@ -49,25 +56,21 @@ benchmarks! { } close { - let caller: T::AccountId = whitelisted_caller(); - let proposal: pallet::Proposal = submit_proposal::(caller.clone()); - assert_ok!(>::add_constituent(SystemOrigin::Root.into(), caller.clone())); - let voters = ["a", "b", "c", "d", "e"]; + let proposal: pallet::Proposal = submit_proposal::(T::ProposalSubmissionOrigin::successful_origin()); - // run to voting period - >::set_block_number(>::block_number() + ::VotingPeriod::get() + ::ProposalSubmissionPeriod::get() + 1_u32.into()); - - // generate members - for i in &voters { - let voter: T::AccountId = account(i, 0, 0); - >::insert(voter.clone(), MemberType::Council); - - // vote aye - assert_ok!(>::vote( - SystemOrigin::Signed(voter).into(), - proposal.hash(), - Vote::Aye, - )); + // vote + for i in 0..5 { + let voter: T::AccountId = account("voter", i, 0); + assert_ok!(Votes::::try_mutate(&proposal.hash(), |votes| { + if let Some(votes) = votes { + votes.cast_vote( + MemberVote::new(CommitteeMember::new(voter, MemberType::Council), Vote::Aye), + ); + Ok(()) + } else { + Err(Error::::NoProposalWithHash) + } + })); } // run out of voting period @@ -77,12 +80,14 @@ benchmarks! { + ::ProposalSubmissionPeriod::get() + 1_u32.into() ); - }: _( - SystemOrigin::Signed(caller.clone()), - proposal.hash() - ) verify { + + // construct call + let call = >::close(proposal.hash()); + }: { + call.dispatch_bypass_filter(T::ProposalExecutionOrigin::successful_origin())? + } verify { assert_noop!( - >::close(SystemOrigin::Signed(caller.clone()).into(), proposal.hash()), + >::close(T::ProposalExecutionOrigin::successful_origin(), proposal.hash()), >::ProposalAlreadyExecuted ); } diff --git a/pallets/committee/src/lib.rs b/pallets/committee/src/lib.rs index e91d9202a4..69b0f6f535 100644 --- a/pallets/committee/src/lib.rs +++ b/pallets/committee/src/lib.rs @@ -76,10 +76,16 @@ pub mod pallet { type MinCouncilVotes: Get; /// Origin that is permitted to create proposals - type ProposalSubmissionOrigin: EnsureOrigin<::Origin>; + type ProposalSubmissionOrigin: EnsureOrigin< + ::Origin, + Success = ::AccountId, + >; /// Origin that is permitted to execute approved proposals - type ProposalExecutionOrigin: EnsureOrigin<::Origin>; + type ProposalExecutionOrigin: EnsureOrigin< + ::Origin, + Success = ::AccountId, + >; /// Origin that is permitted to execute `add_constituent` type ApprovedByCommitteeOrigin: EnsureOrigin<::Origin>; @@ -265,19 +271,6 @@ pub mod pallet { Votes::::get(hash) } - /// Used to check if an origin is signed and the signer is a member of - /// the committee - pub fn ensure_member( - origin: OriginFor, - ) -> Result>, DispatchError> { - let who = ensure_signed(origin)?; - if let Some(member_type) = Members::::get(who.clone()) { - Ok(CommitteeMember::new(who, member_type)) - } else { - Err(Error::::NotMember.into()) - } - } - /// Returns the block at the end of the next voting period pub fn get_next_voting_period_end( block_number: &BlockNumberFor, @@ -313,6 +306,19 @@ pub mod pallet { }) }) } + + /// Used to check if an origin is signed and the signer is a member of + /// the committee + pub fn ensure_member( + origin: OriginFor, + ) -> Result>, DispatchError> { + let who = ensure_signed(origin)?; + if let Some(member_type) = >::get(who.clone()) { + Ok(CommitteeMember::new(who, member_type)) + } else { + Err(>::NotMember.into()) + } + } } #[pallet::call] @@ -322,8 +328,7 @@ pub mod pallet { /// The provided action will be turned into a proposal and added to the list of current active proposals /// to be voted on in the next voting period. pub fn propose(origin: OriginFor, action: Box) -> DispatchResultWithPostInfo { - let proposer = ensure_signed(origin.clone())?; - T::ProposalSubmissionOrigin::ensure_origin(origin)?; + let proposer = T::ProposalSubmissionOrigin::ensure_origin(origin.clone())?; // Create a new proposal with a unique nonce let nonce = Self::take_and_increment_nonce()?; @@ -390,8 +395,7 @@ pub mod pallet { origin: OriginFor, proposal_hash: HashFor, ) -> DispatchResultWithPostInfo { - let closer = ensure_signed(origin.clone())?; - T::ProposalExecutionOrigin::ensure_origin(origin)?; + let closer = T::ProposalExecutionOrigin::ensure_origin(origin.clone())?; // ensure proposal has not already been executed ensure!( diff --git a/pallets/committee/src/mock.rs b/pallets/committee/src/mock.rs index 31cf4ec143..471f35d64a 100644 --- a/pallets/committee/src/mock.rs +++ b/pallets/committee/src/mock.rs @@ -4,14 +4,14 @@ // Required as construct_runtime! produces code that violates this lint #![allow(clippy::from_over_into)] -use crate as pallet_committee; +use crate::{self as pallet_committee, EnsureMember}; #[cfg(feature = "std")] use frame_support::traits::GenesisBuild; use frame_support::{ ord_parameter_types, parameter_types, traits::{OnFinalize, OnInitialize}, }; -use frame_system as system; +use frame_system::{self as system, EnsureSignedBy}; use sp_core::H256; use sp_runtime::{ @@ -75,7 +75,7 @@ parameter_types! { pub const VotingPeriod: ::BlockNumber = VOTING_PERIOD; } pub(crate) const PROPOSER_ACCOUNT_ID: AccountId = 88; -pub(crate) const EXECUTER_ACCOUNT_ID: AccountId = 88; +pub(crate) const EXECUTER_ACCOUNT_ID: AccountId = PROPOSER_ACCOUNT_ID; pub(crate) const MIN_COUNCIL_VOTES: usize = 4; ord_parameter_types! { @@ -88,15 +88,15 @@ ord_parameter_types! { type EnsureApprovedByCommittee = frame_system::EnsureOneOf< AccountId, frame_system::EnsureRoot, - crate::EnsureApprovedByCommittee, + crate::EnsureApprovedByCommittee, >; impl pallet_committee::Config for Test { type ProposalSubmissionPeriod = ProposalSubmissionPeriod; type VotingPeriod = VotingPeriod; type MinCouncilVotes = MinCouncilVotes; - type ProposalSubmissionOrigin = frame_system::EnsureSignedBy; - type ProposalExecutionOrigin = frame_system::EnsureSignedBy; + type ProposalSubmissionOrigin = EnsureSignedBy; + type ProposalExecutionOrigin = EnsureMember; type ApprovedByCommitteeOrigin = EnsureApprovedByCommittee; type ProposalNonce = u32; type Origin = Origin; @@ -125,8 +125,10 @@ where .build_storage::() .unwrap(); + let mut council_members = vec![PROPOSER_ACCOUNT_ID]; + council_members.append(&mut members.into_iter().collect()); pallet_committee::GenesisConfig:: { - council_members: members.into_iter().collect(), + council_members, constituent_members: Default::default(), } .assimilate_storage(&mut t) diff --git a/pallets/committee/src/tests.rs b/pallets/committee/src/tests.rs index 72aec6f3df..d6651bbf0a 100644 --- a/pallets/committee/src/tests.rs +++ b/pallets/committee/src/tests.rs @@ -3,7 +3,7 @@ use crate as pallet; use crate::mock::*; -use crate::{CommitteeMember, CommitteeOrigin, MemberType, Vote, VoteAggregate}; +use crate::{CommitteeMember, MemberType, Vote, VoteAggregate}; use frame_support::{assert_noop, assert_ok, codec::Encode}; use frame_system as system; use sp_runtime::traits::BadOrigin; @@ -130,7 +130,7 @@ fn non_member_cannot_vote() { let expected_votes = VoteAggregate::new_with_end(START_OF_V1); assert_noop!( Committee::vote(Origin::signed(ASHLEY), proposal.hash(), Vote::Aye), - pallet::Error::::NotMember + >::NotMember, ); assert_eq!( Committee::get_votes_for(&proposal.hash()), @@ -326,12 +326,15 @@ where #[test] fn non_execution_origin_cannot_close() { new_test_ext(0..4).execute_with(|| { + let non_execution_origin = 5; let proposal = submit_proposal(123); run_to_block(START_OF_S1); vote_with_each(0..4, proposal.hash(), Vote::Aye); + + run_to_block(START_OF_V1 + 1); assert_noop!( - Committee::close(Origin::signed(ASHLEY), proposal.hash()), + Committee::close(Origin::signed(non_execution_origin), proposal.hash()), BadOrigin ); }); @@ -451,22 +454,11 @@ fn cannot_execute_proposal_twice() { // Constituent Committee Council Selection // -/// An `ApprovedByCommittee` origin -fn approved_by_committee() -> CommitteeOrigin { - CommitteeOrigin::ApprovedByCommittee( - PROPOSER_ACCOUNT_ID, - VoteAggregate { - votes: Vec::new(), - end: START_OF_V1, - }, - ) -} - #[test] fn cannot_add_constituent_if_already_is_council() { new_test_ext(PROPOSER_ACCOUNT_ID..PROPOSER_ACCOUNT_ID + 1).execute_with(|| { assert_noop!( - Committee::add_constituent(approved_by_committee().into(), PROPOSER_ACCOUNT_ID), + Committee::add_constituent(Origin::root().into(), PROPOSER_ACCOUNT_ID), >::AlreadyCouncilMember ); }); @@ -475,13 +467,10 @@ fn cannot_add_constituent_if_already_is_council() { #[test] fn cannot_add_constituent_if_already_is_constituent() { new_test_ext(PROPOSER_ACCOUNT_ID..PROPOSER_ACCOUNT_ID + 1).execute_with(|| { - assert_ok!(Committee::add_constituent( - approved_by_committee().into(), - 42 - )); + assert_ok!(Committee::add_constituent(Origin::root().into(), 42)); assert_noop!( - Committee::add_constituent(approved_by_committee().into(), 42), + Committee::add_constituent(Origin::root().into(), 42), >::AlreadyConstituentMember ); }); diff --git a/pallets/committee/src/types.rs b/pallets/committee/src/types.rs index 2978edbc83..025c12bfdf 100644 --- a/pallets/committee/src/types.rs +++ b/pallets/committee/src/types.rs @@ -1,11 +1,13 @@ // Copyright 2021 ChainSafe Systems // SPDX-License-Identifier: LGPL-3.0-only -use crate::Config; +use crate::{Config, Members, Origin}; use frame_support::{ pallet_prelude::*, sp_std::{self, prelude::Vec}, + traits::EnsureOrigin, }; +use frame_system::RawOrigin; use sp_runtime::traits::Hash; #[cfg(feature = "std")] @@ -76,6 +78,8 @@ impl MemberVote { pub enum CommitteeOrigin { /// Action is executed by the committee. Contains the closer account and the members that voted Aye ApprovedByCommittee(AccountId, VoteAggregate), + /// It has been condoned by a single member of the committee. + CommitteeMember(AccountId), } #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, Default)] @@ -183,21 +187,23 @@ pub enum Vote { /// // This is for the extrinsics only can be called after the /// approval of the committee -pub struct EnsureApprovedByCommittee( - sp_std::marker::PhantomData<(AccountId, BlockNumber)>, -); -impl< - O: Into, O>> - + From>, - AccountId: PartialEq + Default, - BlockNumber: Default, - > EnsureOrigin for EnsureApprovedByCommittee +pub struct EnsureApprovedByCommittee(sp_std::marker::PhantomData); + +impl, O>> + From> + Clone, T: Config> EnsureOrigin + for EnsureApprovedByCommittee { - type Success = AccountId; + type Success = ::AccountId; fn try_origin(o: O) -> Result { - o.into().and_then(|o| match o { - CommitteeOrigin::ApprovedByCommittee(i, _) => Ok(i), - }) + let origin = o.clone().into()?; + match origin { + CommitteeOrigin::ApprovedByCommittee(i, votes) => { + votes + .is_accepted(T::MinCouncilVotes::get()) + .map_err(|_| o)?; + Ok(i) + } + _ => Err(o), + } } #[cfg(feature = "runtime-benchmarks")] @@ -208,3 +214,34 @@ impl< )) } } + +/// Ensure committee member +pub struct EnsureMember(sp_std::marker::PhantomData); + +impl< + O: Into::AccountId>, O>> + + From::AccountId>> + + Clone, + T: Config, + > EnsureOrigin for EnsureMember +{ + type Success = ::AccountId; + fn try_origin(o: O) -> Result { + let origin = o.clone().into()?; + match origin { + RawOrigin::Signed(i) => { + if >::contains_key(i.clone()) { + Ok(i) + } else { + Err(o) + } + } + _ => Err(o), + } + } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> O { + O::from(RawOrigin::Signed(Default::default())) + } +} diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 46d7dea0a2..7fffd47c03 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -25,7 +25,10 @@ use sp_std::prelude::*; use sp_version::NativeVersion; use sp_version::RuntimeVersion; -use frame_system::limits::{BlockLength, BlockWeights}; +use frame_system::{ + limits::{BlockLength, BlockWeights}, + EnsureSigned, +}; // Polkadot imports use cumulus_primitives_core::ParaId; @@ -52,6 +55,7 @@ pub use frame_support::{ }; use pallet_asset_index::{MultiAssetAdapter, MultiAssetRegistry}; pub use pallet_balances::Call as BalancesCall; +use pallet_committee::EnsureMember; pub use pallet_timestamp::Call as TimestampCall; #[cfg(any(feature = "std", test))] pub use sp_runtime::BuildStorage; @@ -477,16 +481,15 @@ ord_parameter_types! { type EnsureApprovedByCommittee = frame_system::EnsureOneOf< AccountId, frame_system::EnsureRoot, - pallet_committee::EnsureApprovedByCommittee, + pallet_committee::EnsureApprovedByCommittee, >; impl pallet_committee::Config for Runtime { type ProposalSubmissionPeriod = ProposalSubmissionPeriod; type VotingPeriod = VotingPeriod; type MinCouncilVotes = MinCouncilVotes; - // Using signed as the admin origin for now - type ProposalSubmissionOrigin = frame_system::EnsureSigned; - type ProposalExecutionOrigin = frame_system::EnsureSigned; + type ProposalSubmissionOrigin = EnsureSigned; + type ProposalExecutionOrigin = EnsureMember; type ApprovedByCommitteeOrigin = EnsureApprovedByCommittee; type ProposalNonce = u32; type Origin = Origin; @@ -705,7 +708,7 @@ construct_runtime!( // PINT pallets AssetIndex: pallet_asset_index::{Pallet, Call, Storage, Event}, AssetDepository: pallet_asset_depository::{Pallet, Call, Storage, Event}, - Committee: pallet_committee::{Pallet, Call, Storage, Origin, Event}, + Committee: pallet_committee::{Pallet, Call, Storage, Origin, Event, Config}, LocalTreasury: pallet_local_treasury::{Pallet, Call, Storage, Event}, SaftRegistry: pallet_saft_registry::{Pallet, Call, Storage, Event}, RemoteAssetManager: pallet_remote_asset_manager::{Pallet, Call, Storage, Event},