feat: governance-pre-authorised-calls (#3964)

Co-authored-by: Daniel <daniel@chainflip.io>
Co-authored-by: dandanlen <3168260+dandanlen@users.noreply.github.com>

api/lib/src/lib.rs

@@ -10,6 +10,7 @@ use cf_chains::{
 };
 use cf_primitives::{AccountRole, Asset, BasisPoints, ChannelId};
 use futures::FutureExt;
+use pallet_cf_governance::ExecutionMode;
 use pallet_cf_validator::MAX_LENGTH_FOR_VANITY_NAME;
 use serde::Serialize;
 use sp_consensus_aura::sr25519::AuthorityId as AuraId;
@@ -265,6 +266,7 @@ pub trait GovernanceApi: SignedExtrinsicApi {
 		println!("Submitting governance proposal for rotation.");
 		self.submit_signed_extrinsic(pallet_cf_governance::Call::propose_governance_extrinsic {
 			call: Box::new(pallet_cf_validator::Call::force_rotation {}.into()),
+			execution: ExecutionMode::Automatic,
 		})
 		.await
 		.until_finalized()

bouncer/shared/cf_governance.ts

@@ -15,10 +15,13 @@ const keyring = new Keyring({ type: 'sr25519' });
 
 export const snowWhite = keyring.createFromUri(snowWhiteUri);
 
-export async function submitGovernanceExtrinsic(extrinsic: SubmittableExtrinsic<'promise'>) {
+export async function submitGovernanceExtrinsic(
+  extrinsic: SubmittableExtrinsic<'promise'>,
+  preAuthorise = 0,
+) {
   return snowWhiteMutex.runExclusive(async () =>
     chainflip.tx.governance
-      .proposeGovernanceExtrinsic(extrinsic)
+      .proposeGovernanceExtrinsic(extrinsic, preAuthorise)
       .signAndSend(snowWhite, { nonce: -1 }, handleSubstrateError(chainflip)),
   );
 }

state-chain/pallets/cf-governance/Cargo.toml

@@ -50,7 +50,6 @@ std = [
   'frame-system/std',
   'scale-info/std',
   'sp-std/std',
-  'sp-version/std',
 ]
 runtime-benchmarks = [
   'cf-primitives/runtime-benchmarks',

state-chain/pallets/cf-governance/src/benchmarking.rs

@@ -16,15 +16,15 @@ benchmarks! {
 		let caller: T::AccountId = whitelisted_caller();
 		let call = Box::new(frame_system::Call::remark{remark: vec![]}.into());
 		<Members<T>>::put(BTreeSet::from([caller.clone()]));
-	}: _(RawOrigin::Signed(caller.clone()), call)
+	}: _(RawOrigin::Signed(caller.clone()), call, ExecutionMode::Automatic)
 	verify {
 		assert_eq!(ProposalIdCounter::<T>::get(), 1);
 	}
 	approve {
 		let call: <T as Config>::RuntimeCall = frame_system::Call::remark{remark: vec![]}.into();
 		let caller: T::AccountId = whitelisted_caller();
 		<Members<T>>::put(BTreeSet::from([caller.clone()]));
-		Pallet::<T>::push_proposal(Box::new(call));
+		Pallet::<T>::push_proposal(Box::new(call), ExecutionMode::Automatic);
 	}: _(RawOrigin::Signed(caller.clone()), 1)
 	verify {
 		assert_eq!(ProposalIdCounter::<T>::get(), 1);
@@ -48,7 +48,7 @@ benchmarks! {
 		let b in 1 .. 100u32;
 		for _n in 1 .. b {
 			let call = Box::new(frame_system::Call::remark{remark: vec![]}.into());
-			Pallet::<T>::push_proposal(call);
+			Pallet::<T>::push_proposal(call, ExecutionMode::Automatic);
 		}
 	}: {
 		Pallet::<T>::on_initialize(2u32.into());
@@ -61,7 +61,7 @@ benchmarks! {
 		let b in 1 .. 100u32;
 		for _n in 1 .. b {
 			let call = Box::new(frame_system::Call::remark{remark: vec![]}.into());
-			Pallet::<T>::push_proposal(call);
+			Pallet::<T>::push_proposal(call, ExecutionMode::Automatic);
 		}
 	} : {
 		Pallet::<T>::expire_proposals(<ActiveProposals<T>>::get());
@@ -106,5 +106,15 @@ benchmarks! {
 		assert!(GovKeyWhitelistedCallHash::<T>::get().is_none());
 	}
 
+	dispatch_whitelisted_call {
+		let caller: T::AccountId = whitelisted_caller();
+		<Members<T>>::put(BTreeSet::from([caller.clone()]));
+		let call: <T as Config>::RuntimeCall = Call::<T>::new_membership_set {
+			accounts: vec![]
+		}.into();
+		Pallet::<T>::push_proposal(Box::new(call.clone()), ExecutionMode::Manual);
+		PreAuthorisedGovCalls::<T>::insert(1, call.encode());
+	}: _(RawOrigin::Signed(caller.clone()), 1)
+
 	impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test,);
 }

state-chain/pallets/cf-governance/src/lib.rs

@@ -52,6 +52,13 @@ pub mod pallet {
 
 	use super::{GovCallHash, WeightInfo};
 
+	#[derive(Default, Encode, Decode, TypeInfo, Clone, RuntimeDebug, PartialEq, Eq)]
+	pub enum ExecutionMode {
+		#[default]
+		Automatic,
+		Manual,
+	}
+
 	#[derive(Encode, Decode, TypeInfo, Clone, Copy, RuntimeDebug, PartialEq, Eq)]
 	pub struct ActiveProposal {
 		pub proposal_id: ProposalId,
@@ -65,12 +72,8 @@ pub mod pallet {
 		pub call: OpaqueCall,
 		/// Accounts who have already approved the proposal.
 		pub approved: BTreeSet<AccountId>,
-	}
-
-	impl<T> Default for Proposal<T> {
-		fn default() -> Self {
-			Self { call: Default::default(), approved: Default::default() }
-		}
+		/// Proposal is pre authorised.
+		pub execution: ExecutionMode,
 	}
 
 	type AccountId<T> = <T as frame_system::Config>::AccountId;
@@ -115,7 +118,7 @@ pub mod pallet {
 	#[pallet::storage]
 	#[pallet::getter(fn proposals)]
 	pub(super) type Proposals<T: Config> =
-		StorageMap<_, Blake2_128Concat, ProposalId, Proposal<T::AccountId>, ValueQuery>;
+		StorageMap<_, Blake2_128Concat, ProposalId, Proposal<T::AccountId>>;
 
 	/// Active proposals.
 	#[pallet::storage]
@@ -127,6 +130,11 @@ pub mod pallet {
 	#[pallet::getter(fn gov_key_whitelisted_call_hash)]
 	pub(super) type GovKeyWhitelistedCallHash<T> = StorageValue<_, GovCallHash, OptionQuery>;
 
+	/// Pre authorised governance calls.
+	#[pallet::storage]
+	pub(super) type PreAuthorisedGovCalls<T> =
+		StorageMap<_, Twox64Concat, u32, OpaqueCall, OptionQuery>;
+
 	/// Any nonces before this have been consumed.
 	#[pallet::storage]
 	#[pallet::getter(fn next_gov_key_call_hash_nonce)]
@@ -242,11 +250,12 @@ pub mod pallet {
 		pub fn propose_governance_extrinsic(
 			origin: OriginFor<T>,
 			call: Box<<T as Config>::RuntimeCall>,
+			execution: ExecutionMode,
 		) -> DispatchResultWithPostInfo {
 			let who = ensure_signed(origin)?;
 			ensure!(Members::<T>::get().contains(&who), Error::<T>::NotMember);
 
-			let id = Self::push_proposal(call);
+			let id = Self::push_proposal(call, execution);
 			Self::deposit_event(Event::Proposed(id));
 
 			Self::inner_approve(who, id)?;
@@ -425,6 +434,29 @@ pub mod pallet {
 				_ => Err(Error::<T>::CallHashNotWhitelisted.into()),
 			}
 		}
+
+		#[pallet::call_index(7)]
+		#[pallet::weight(T::WeightInfo::dispatch_whitelisted_call())]
+		pub fn dispatch_whitelisted_call(
+			origin: OriginFor<T>,
+			approved_id: ProposalId,
+		) -> DispatchResult {
+			let who = ensure_signed(origin)?;
+			ensure!(Members::<T>::get().contains(&who), Error::<T>::NotMember);
+			if let Some(call) = PreAuthorisedGovCalls::<T>::take(approved_id) {
+				if let Ok(call) = <T as Config>::RuntimeCall::decode(&mut &(*call)) {
+					Self::deposit_event(match Self::dispatch_governance_call(call) {
+						Ok(_) => Event::Executed(approved_id),
+						Err(err) => Event::FailedExecution(err.error),
+					});
+					Ok(())
+				} else {
+					Err(Error::<T>::DecodeOfCallFailed.into())
+				}
+			} else {
+				Err(Error::<T>::ProposalNotFound.into())
+			}
+		}
 	}
 
 	/// Genesis definition
@@ -491,7 +523,9 @@ impl<T: Config> Pallet<T> {
 		ensure!(Proposals::<T>::contains_key(approved_id), Error::<T>::ProposalNotFound);
 
 		// Try to approve the proposal
-		let proposal = Proposals::<T>::mutate(approved_id, |proposal| {
+		let proposal = Proposals::<T>::try_mutate(approved_id, |proposal| {
+			let proposal = proposal.as_mut().ok_or(Error::<T>::ProposalNotFound)?;
+
 			if !proposal.approved.insert(who) {
 				return Err(Error::<T>::AlreadyApproved)
 			}
@@ -502,7 +536,11 @@ impl<T: Config> Pallet<T> {
 		if proposal.approved.len() >
 			(Members::<T>::decode_len().ok_or(Error::<T>::DecodeMembersLenFailed)? / 2)
 		{
-			ExecutionPipeline::<T>::append((proposal.call, approved_id));
+			if proposal.execution == ExecutionMode::Manual {
+				PreAuthorisedGovCalls::<T>::insert(approved_id, proposal.call);
+			} else {
+				ExecutionPipeline::<T>::append((proposal.call, approved_id));
+			}
 			Proposals::<T>::remove(approved_id);
 			ActiveProposals::<T>::mutate(|proposals| {
 				proposals.retain(|ActiveProposal { proposal_id, .. }| *proposal_id != approved_id)
@@ -560,11 +598,11 @@ impl<T: Config> Pallet<T> {
 		T::WeightInfo::expire_proposals(expired.len() as u32)
 	}
 
-	fn push_proposal(call: Box<<T as Config>::RuntimeCall>) -> u32 {
+	fn push_proposal(call: Box<<T as Config>::RuntimeCall>, execution: ExecutionMode) -> u32 {
 		let proposal_id = ProposalIdCounter::<T>::get().add(1);
 		Proposals::<T>::insert(
 			proposal_id,
-			Proposal { call: call.encode(), approved: Default::default() },
+			Proposal { call: call.encode(), approved: Default::default(), execution },
 		);
 		ProposalIdCounter::<T>::put(proposal_id);
 		ActiveProposals::<T>::append(ActiveProposal {

state-chain/pallets/cf-governance/src/tests.rs

@@ -1,5 +1,6 @@
 use crate::{
-	mock::*, ActiveProposals, Error, ExecutionPipeline, ExpiryTime, Members, ProposalIdCounter,
+	mock::*, ActiveProposals, Error, ExecutionMode, ExecutionPipeline, ExpiryTime, Members,
+	PreAuthorisedGovCalls, ProposalIdCounter,
 };
 use cf_test_utilities::last_event;
 use cf_traits::mocks::time_source;
@@ -33,7 +34,11 @@ fn genesis_config() {
 fn not_a_member() {
 	new_test_ext().execute_with(|| {
 		assert_noop!(
-			Governance::propose_governance_extrinsic(RuntimeOrigin::signed(EVE), mock_extrinsic()),
+			Governance::propose_governance_extrinsic(
+				RuntimeOrigin::signed(EVE),
+				mock_extrinsic(),
+				ExecutionMode::Automatic,
+			),
 			<Error<Test>>::NotMember
 		);
 	});
@@ -46,7 +51,8 @@ fn propose_a_governance_extrinsic_and_expect_execution() {
 			// Propose a governance extrinsic
 			assert_ok!(Governance::propose_governance_extrinsic(
 				RuntimeOrigin::signed(ALICE),
-				mock_extrinsic()
+				mock_extrinsic(),
+				ExecutionMode::Automatic,
 			));
 			assert_eq!(
 				last_event::<Test>(),
@@ -77,7 +83,8 @@ fn already_executed() {
 		// Propose a governance extrinsic
 		assert_ok!(Governance::propose_governance_extrinsic(
 			RuntimeOrigin::signed(ALICE),
-			mock_extrinsic()
+			mock_extrinsic(),
+			ExecutionMode::Automatic,
 		));
 		// Assert the proposed event was fired
 		assert_eq!(
@@ -119,7 +126,8 @@ fn propose_a_governance_extrinsic_and_expect_it_to_expire() {
 			// Propose governance extrinsic
 			assert_ok!(Governance::propose_governance_extrinsic(
 				RuntimeOrigin::signed(ALICE),
-				mock_extrinsic()
+				mock_extrinsic(),
+				ExecutionMode::Automatic,
 			));
 		})
 		.then_execute_at_next_block(|_| {
@@ -142,7 +150,8 @@ fn can_not_vote_twice() {
 		// Propose a governance extrinsic
 		assert_ok!(Governance::propose_governance_extrinsic(
 			RuntimeOrigin::signed(ALICE),
-			mock_extrinsic()
+			mock_extrinsic(),
+			ExecutionMode::Automatic,
 		));
 		// Try to approve it again. Proposing implies approving.
 		assert_noop!(
@@ -157,15 +166,17 @@ fn several_open_proposals() {
 	new_test_ext().execute_with(|| {
 		assert_ok!(Governance::propose_governance_extrinsic(
 			RuntimeOrigin::signed(ALICE),
-			mock_extrinsic()
+			mock_extrinsic(),
+			ExecutionMode::Automatic,
 		));
 		assert_eq!(
 			last_event::<Test>(),
 			crate::mock::RuntimeEvent::Governance(crate::Event::Approved(1)),
 		);
 		assert_ok!(Governance::propose_governance_extrinsic(
 			RuntimeOrigin::signed(BOB),
-			mock_extrinsic()
+			mock_extrinsic(),
+			ExecutionMode::Automatic,
 		));
 		assert_eq!(
 			last_event::<Test>(),
@@ -190,7 +201,8 @@ fn sudo_extrinsic() {
 			// Propose the governance extrinsic
 			assert_ok!(Governance::propose_governance_extrinsic(
 				RuntimeOrigin::signed(ALICE),
-				governance_extrinsic
+				governance_extrinsic,
+				ExecutionMode::Automatic,
 			));
 			assert_eq!(
 				last_event::<Test>(),
@@ -309,3 +321,18 @@ fn runtime_upgrade_can_have_no_cfes_version_requirement() {
 		));
 	});
 }
+
+#[test]
+fn whitelisted_gov_call() {
+	new_test_ext().execute_with(|| {
+		assert_ok!(Governance::propose_governance_extrinsic(
+			RuntimeOrigin::signed(ALICE),
+			mock_extrinsic(),
+			ExecutionMode::Manual,
+		));
+		assert_ok!(Governance::approve(RuntimeOrigin::signed(BOB), 1));
+		assert!(PreAuthorisedGovCalls::<Test>::contains_key(1));
+		assert_ok!(Governance::dispatch_whitelisted_call(RuntimeOrigin::signed(CHARLES), 1));
+		assert!(!PreAuthorisedGovCalls::<Test>::contains_key(1));
+	});
+}