feat(committee): add CommitteeMember origin for pallet committee

node/src/chain_spec.rs

@@ -62,6 +62,12 @@ pub fn pint_development_config(id: ParaId) -> ChainSpec {
                     get_account_id_from_seed::<sr25519::Public>("Alice//stash"),
                     get_account_id_from_seed::<sr25519::Public>("Bob//stash"),
                 ],
+                vec![
+                    get_account_id_from_seed::<sr25519::Public>("Alice"),
+                    get_account_id_from_seed::<sr25519::Public>("Bob"),
+                    get_account_id_from_seed::<sr25519::Public>("Charlie"),
+                    get_account_id_from_seed::<sr25519::Public>("Dave"),
+                ],
                 id,
             )
         },
@@ -100,6 +106,12 @@ pub fn pint_local_config(id: ParaId) -> ChainSpec {
                     get_account_id_from_seed::<sr25519::Public>("Eve//stash"),
                     get_account_id_from_seed::<sr25519::Public>("Ferdie//stash"),
                 ],
+                vec![
+                    get_account_id_from_seed::<sr25519::Public>("Alice"),
+                    get_account_id_from_seed::<sr25519::Public>("Bob"),
+                    get_account_id_from_seed::<sr25519::Public>("Charlie"),
+                    get_account_id_from_seed::<sr25519::Public>("Dave"),
+                ],
                 id,
             )
         },
@@ -117,6 +129,7 @@ pub fn pint_local_config(id: ParaId) -> ChainSpec {
 fn pint_testnet_genesis(
     root_key: AccountId,
     endowed_accounts: Vec<AccountId>,
+    council_members: Vec<AccountId>,
     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()

pallets/committee/src/benchmarking.rs

@@ -1,73 +1,76 @@
 // 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<T: Config>(caller: T::AccountId) -> pallet::Proposal<T> {
+fn submit_proposal<T: Config>(origin: <T as frame_system::Config>::Origin) -> pallet::Proposal<T> {
     let action: T::Action = <SystemCall<T>>::remark(vec![0; 0]).into();
     let expected_nonce = pallet::ProposalCount::<T>::get();
-    assert_ok!(<Pallet<T>>::propose(
-        SystemOrigin::Signed(caller).into(),
-        Box::new(action.clone())
+    assert_ok!(<Pallet<T>>::add_constituent(
+        SystemOrigin::Root.into(),
+        ensure_signed(origin.clone()).unwrap(),
     ));
+    let call = <Call<T>>::propose(Box::new(action.clone()));
+    assert_ok!(call.dispatch_bypass_filter(origin));
     pallet::Proposal::<T>::new(expected_nonce, action)
 }
 
 benchmarks! {
     propose {
-        let caller: T::AccountId = whitelisted_caller();
-        let proposal = submit_proposal::<T>(caller.clone());
-    }: _(
-        SystemOrigin::Signed(caller.clone()),
-        Box::new(<SystemCall<T>>::remark(vec![0; 0]).into())
-    ) verify {
+        let origin = T::ProposalSubmissionOrigin::successful_origin();
+        let proposal = submit_proposal::<T>(origin.clone());
+        let call = <Call<T>>::propose(Box::new(<SystemCall<T>>::remark(vec![0; 0]).into()));
+    }: {
+        call.dispatch_bypass_filter(origin)?
+    } verify {
         assert!(<Pallet<T>>::get_proposal(&proposal.hash()) == Some(proposal));
     }
 
     vote {
-        let caller: T::AccountId = whitelisted_caller();
-        let proposal = submit_proposal::<T>(caller.clone());
-        assert_ok!(<Pallet<T>>::add_constituent(SystemOrigin::Root.into(), caller.clone()));
+        let origin = T::ProposalSubmissionOrigin::successful_origin();
+        let proposal = submit_proposal::<T>(origin.clone());
 
         // run to voting period
         <System<T>>::set_block_number(
             <System<T>>::block_number()
                 + <T as Config>::VotingPeriod::get()
                 + <T as Config>::ProposalSubmissionPeriod::get() + 1_u32.into(),
         );
-    }: _(
-        SystemOrigin::Signed(caller.clone()),
-        proposal.hash(),
-        Vote::Abstain
-    ) verify {
+
+        // construct call
+        let call = <Call<T>>::vote(proposal.hash(), Vote::Abstain);
+    }: {
+        call.dispatch_bypass_filter(origin)?
+    } verify {
         assert_eq!(
             <Pallet<T>>::get_votes_for(&proposal.hash()).unwrap().votes.len(),
             1,
         );
     }
 
     close {
-        let caller: T::AccountId = whitelisted_caller();
-        let proposal: pallet::Proposal<T> = submit_proposal::<T>(caller.clone());
-        assert_ok!(<Pallet<T>>::add_constituent(SystemOrigin::Root.into(), caller.clone()));
-        let voters = ["a", "b", "c", "d", "e"];
+        let proposal: pallet::Proposal<T> = submit_proposal::<T>(T::ProposalSubmissionOrigin::successful_origin());
 
-        // run to voting period
-        <System<T>>::set_block_number(<System<T>>::block_number() + <T as Config>::VotingPeriod::get() + <T as Config>::ProposalSubmissionPeriod::get() + 1_u32.into());
-
-        // generate members
-        for i in &voters {
-            let voter: T::AccountId = account(i, 0, 0);
-            <Members<T>>::insert(voter.clone(), MemberType::Council);
-
-            // vote aye
-            assert_ok!(<Pallet<T>>::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::<T>::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::<T>::NoProposalWithHash)
+                }
+            }));
         }
 
         // run out of voting period
@@ -77,12 +80,14 @@ benchmarks! {
                 + <T as Config>::ProposalSubmissionPeriod::get()
                 + 1_u32.into()
         );
-    }: _(
-        SystemOrigin::Signed(caller.clone()),
-        proposal.hash()
-    ) verify {
+
+        // construct call
+        let call = <Call<T>>::close(proposal.hash());
+    }: {
+        call.dispatch_bypass_filter(T::ProposalExecutionOrigin::successful_origin())?
+    } verify {
         assert_noop!(
-            <Pallet<T>>::close(SystemOrigin::Signed(caller.clone()).into(), proposal.hash()),
+            <Pallet<T>>::close(T::ProposalExecutionOrigin::successful_origin(), proposal.hash()),
             <Error<T>>::ProposalAlreadyExecuted
         );
     }

pallets/committee/src/lib.rs

@@ -76,10 +76,16 @@ pub mod pallet {
         type MinCouncilVotes: Get<usize>;
 
         /// Origin that is permitted to create proposals
-        type ProposalSubmissionOrigin: EnsureOrigin<<Self as frame_system::Config>::Origin>;
+        type ProposalSubmissionOrigin: EnsureOrigin<
+            <Self as frame_system::Config>::Origin,
+            Success = <Self as frame_system::Config>::AccountId,
+        >;
 
         /// Origin that is permitted to execute approved proposals
-        type ProposalExecutionOrigin: EnsureOrigin<<Self as frame_system::Config>::Origin>;
+        type ProposalExecutionOrigin: EnsureOrigin<
+            <Self as frame_system::Config>::Origin,
+            Success = <Self as frame_system::Config>::AccountId,
+        >;
 
         /// Origin that is permitted to execute `add_constituent`
         type ApprovedByCommitteeOrigin: EnsureOrigin<<Self as frame_system::Config>::Origin>;
@@ -265,19 +271,6 @@ pub mod pallet {
             Votes::<T>::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<T>,
-        ) -> Result<CommitteeMember<AccountIdFor<T>>, DispatchError> {
-            let who = ensure_signed(origin)?;
-            if let Some(member_type) = Members::<T>::get(who.clone()) {
-                Ok(CommitteeMember::new(who, member_type))
-            } else {
-                Err(Error::<T>::NotMember.into())
-            }
-        }
-
         /// Returns the block at the end of the next voting period
         pub fn get_next_voting_period_end(
             block_number: &BlockNumberFor<T>,
@@ -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<T>,
+        ) -> Result<CommitteeMember<AccountIdFor<T>>, DispatchError> {
+            let who = ensure_signed(origin)?;
+            if let Some(member_type) = <Members<T>>::get(who.clone()) {
+                Ok(CommitteeMember::new(who, member_type))
+            } else {
+                Err(<Error<T>>::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<T>, action: Box<T::Action>) -> 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<T>,
             proposal_hash: HashFor<T>,
         ) -> 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!(

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: <Test as system::Config>::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<AccountId>,
-    crate::EnsureApprovedByCommittee<AccountId, u64>,
+    crate::EnsureApprovedByCommittee<Test>,
 >;
 
 impl pallet_committee::Config for Test {
     type ProposalSubmissionPeriod = ProposalSubmissionPeriod;
     type VotingPeriod = VotingPeriod;
     type MinCouncilVotes = MinCouncilVotes;
-    type ProposalSubmissionOrigin = frame_system::EnsureSignedBy<AdminAccountId, AccountId>;
-    type ProposalExecutionOrigin = frame_system::EnsureSignedBy<ExecuterAccountId, AccountId>;
+    type ProposalSubmissionOrigin = EnsureSignedBy<AdminAccountId, AccountId>;
+    type ProposalExecutionOrigin = EnsureMember<Self>;
     type ApprovedByCommitteeOrigin = EnsureApprovedByCommittee;
     type ProposalNonce = u32;
     type Origin = Origin;
@@ -125,8 +125,10 @@ where
         .build_storage::<Test>()
         .unwrap();
 
+    let mut council_members = vec![PROPOSER_ACCOUNT_ID];
+    council_members.append(&mut members.into_iter().collect());
     pallet_committee::GenesisConfig::<Test> {
-        council_members: members.into_iter().collect(),
+        council_members,
         constituent_members: Default::default(),
     }
     .assimilate_storage(&mut t)

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::<Test>::NotMember
+            <pallet::Error<Test>>::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<AccountId, u64> {
-    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),
             <pallet::Error<Test>>::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),
             <pallet::Error<Test>>::AlreadyConstituentMember
         );
     });