Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: governance-pre-authorised-calls #3964

Merged
merged 13 commits into from
Sep 11, 2023
Merged
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions api/lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down
7 changes: 5 additions & 2 deletions bouncer/shared/cf_governance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
);
}
2 changes: 0 additions & 2 deletions state-chain/pallets/cf-governance/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ frame-benchmarking = { git = "https://github.com/chainflip-io/substrate.git", ta
frame-support = { git = "https://github.com/chainflip-io/substrate.git", tag = "chainflip-monthly-2023-08+1", default-features = false }
frame-system = { git = "https://github.com/chainflip-io/substrate.git", tag = "chainflip-monthly-2023-08+1", default-features = false }
sp-std = { git = "https://github.com/chainflip-io/substrate.git", tag = "chainflip-monthly-2023-08+1", default-features = false }
sp-version = { git = "https://github.com/chainflip-io/substrate.git", tag = "chainflip-monthly-2023-08+1", default-features = false }

cf-runtime-upgrade-utilities = { path = '../../runtime-upgrade-utilities', default-features = false }

Expand All @@ -50,7 +49,6 @@ std = [
'frame-system/std',
'scale-info/std',
'sp-std/std',
'sp-version/std',
]
runtime-benchmarks = [
'cf-primitives/runtime-benchmarks',
Expand Down
18 changes: 14 additions & 4 deletions state-chain/pallets/cf-governance/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());
Expand All @@ -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());
Expand Down Expand Up @@ -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,);
}
62 changes: 50 additions & 12 deletions state-chain/pallets/cf-governance/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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]
Expand All @@ -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)]
Expand Down Expand Up @@ -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)?;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
45 changes: 36 additions & 9 deletions state-chain/pallets/cf-governance/src/tests.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
);
});
Expand All @@ -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>(),
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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(|_| {
Expand All @@ -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!(
Expand All @@ -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>(),
Expand All @@ -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>(),
Expand Down Expand Up @@ -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));
});
}
Loading