From c12f9e4ba55a9cf38f78de5f37f5ad0d99b3038d Mon Sep 17 00:00:00 2001 From: David Salami <31099392+Wizdave97@users.noreply.github.com> Date: Fri, 8 Nov 2024 06:24:43 +0100 Subject: [PATCH] Require two fishermen to veto state machine updates (#338) --- modules/ismp/pallets/fishermen/src/lib.rs | 46 ++++++++++++++----- .../testsuite/src/tests/pallet_fishermen.rs | 29 ++++++++++++ parachain/runtimes/nexus/src/lib.rs | 19 ++++---- 3 files changed, 73 insertions(+), 21 deletions(-) diff --git a/modules/ismp/pallets/fishermen/src/lib.rs b/modules/ismp/pallets/fishermen/src/lib.rs index 1efe98492..585b68ad3 100644 --- a/modules/ismp/pallets/fishermen/src/lib.rs +++ b/modules/ismp/pallets/fishermen/src/lib.rs @@ -51,6 +51,12 @@ pub mod pallet { #[pallet::getter(fn whitelist)] pub type Fishermen = StorageMap<_, Twox64Concat, T::AccountId, (), OptionQuery>; + /// Set of whitelisted fishermen accounts + #[pallet::storage] + #[pallet::getter(fn pending_vetoes)] + pub type PendingVetoes = + StorageMap<_, Blake2_128Concat, StateMachineHeight, T::AccountId, OptionQuery>; + #[pallet::error] pub enum Error { /// Account Already Whitelisted @@ -61,6 +67,8 @@ pub mod pallet { UnauthorizedAction, /// State commitment was not found VetoFailed, + /// Invalid veto request + InvalidVeto, } #[pallet::event] @@ -72,6 +80,8 @@ pub mod pallet { Removed { account: T::AccountId }, /// The provided state commitment was vetoed `state_machine` is by account StateCommitmentVetoed { height: StateMachineHeight, commitment: StateCommitment }, + /// The vetoe has been noted by the runtime + VetoNoted { height: StateMachineHeight, fisherman: T::AccountId }, } #[pallet::call] @@ -109,6 +119,7 @@ pub mod pallet { /// challenge period) is infact fraudulent and misrepresentative of the state /// changes at the provided height. This allows them to veto the state commitment. /// They aren't required to provide any proofs for this. + /// Successful veto requires two fishermen #[pallet::call_index(2)] #[pallet::weight(::DbWeight::get().reads_writes(2, 3))] pub fn veto_state_commitment( @@ -118,18 +129,29 @@ pub mod pallet { let account = ensure_signed(origin.clone())?; ensure!(Fishermen::::contains_key(&account), Error::::UnauthorizedAction); - let ismp_host = ::IsmpHost::default(); - let commitment = - ismp_host.state_machine_commitment(height).map_err(|_| Error::::VetoFailed)?; - ismp_host.delete_state_commitment(height).map_err(|_| Error::::VetoFailed)?; - - Self::deposit_event(Event::StateCommitmentVetoed { height, commitment }); - pallet_ismp::Pallet::::deposit_pallet_event( - ismp::events::Event::StateCommitmentVetoed(StateCommitmentVetoed { - height, - fisherman: account.as_ref().to_vec(), - }), - ); + if let Some(prev_veto) = PendingVetoes::::get(height) { + if account == prev_veto { + Err(Error::::InvalidVeto)? + } + let ismp_host = ::IsmpHost::default(); + let commitment = ismp_host + .state_machine_commitment(height) + .map_err(|_| Error::::VetoFailed)?; + ismp_host.delete_state_commitment(height).map_err(|_| Error::::VetoFailed)?; + PendingVetoes::::remove(height); + + Self::deposit_event(Event::StateCommitmentVetoed { height, commitment }); + pallet_ismp::Pallet::::deposit_pallet_event( + ismp::events::Event::StateCommitmentVetoed(StateCommitmentVetoed { + height, + fisherman: account.as_ref().to_vec(), + }), + ); + } else { + PendingVetoes::::insert(height, account.clone()); + Self::deposit_event(Event::VetoNoted { height, fisherman: account }); + } + Ok(()) } } diff --git a/modules/ismp/pallets/testsuite/src/tests/pallet_fishermen.rs b/modules/ismp/pallets/testsuite/src/tests/pallet_fishermen.rs index 29999fd41..dd93f2d56 100644 --- a/modules/ismp/pallets/testsuite/src/tests/pallet_fishermen.rs +++ b/modules/ismp/pallets/testsuite/src/tests/pallet_fishermen.rs @@ -73,6 +73,12 @@ fn test_can_veto_state_commitments() { })) ); + // Add another fisherman + + let account_2: AccountId32 = H256::random().0.into(); + pallet_fishermen::Pallet::::add(RuntimeOrigin::root(), account_2.clone()).unwrap(); + assert_eq!(pallet_fishermen::Fishermen::::get(account_2.clone()), Some(())); + // actual veto let result = pallet_fishermen::Pallet::::veto_state_commitment( RuntimeOrigin::signed(account.clone()), @@ -80,6 +86,29 @@ fn test_can_veto_state_commitments() { ); assert_eq!(result, Ok(())); + assert_eq!(pallet_fishermen::PendingVetoes::::get(height), Some(account.clone())); + + // veto with same account + let result = pallet_fishermen::Pallet::::veto_state_commitment( + RuntimeOrigin::signed(account.clone()), + height, + ); + + assert!(matches!( + result, + Err(sp_runtime::DispatchError::Module(ModuleError { + index: 9, + error: [4, 0, 0, 0,], + message: Some("InvalidVeto",), + })) + )); + + let result = pallet_fishermen::Pallet::::veto_state_commitment( + RuntimeOrigin::signed(account_2.clone()), + height, + ); + assert_eq!(result, Ok(())); + // should have been deleted let result = host.state_machine_commitment(height); assert!(matches!(result, Err(Error::StateCommitmentNotFound { .. }))); diff --git a/parachain/runtimes/nexus/src/lib.rs b/parachain/runtimes/nexus/src/lib.rs index c4a52c4c8..c853dae71 100644 --- a/parachain/runtimes/nexus/src/lib.rs +++ b/parachain/runtimes/nexus/src/lib.rs @@ -217,7 +217,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("nexus"), impl_name: create_runtime_str!("nexus"), authoring_version: 1, - spec_version: 1400, + spec_version: 1600, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, @@ -748,19 +748,20 @@ impl InstanceFilter for ProxyType { fn filter(&self, c: &RuntimeCall) -> bool { match self { ProxyType::Any => true, - ProxyType::NonTransfer => - !matches!(c, RuntimeCall::Balances { .. } | RuntimeCall::Assets { .. }), + ProxyType::NonTransfer => { + !matches!(c, RuntimeCall::Balances { .. } | RuntimeCall::Assets { .. }) + }, ProxyType::CancelProxy => matches!( c, - RuntimeCall::Proxy(pallet_proxy::Call::reject_announcement { .. }) | - RuntimeCall::Utility { .. } | - RuntimeCall::Multisig { .. } + RuntimeCall::Proxy(pallet_proxy::Call::reject_announcement { .. }) + | RuntimeCall::Utility { .. } + | RuntimeCall::Multisig { .. } ), ProxyType::Collator => matches!( c, - RuntimeCall::CollatorSelection { .. } | - RuntimeCall::Utility { .. } | - RuntimeCall::Multisig { .. } + RuntimeCall::CollatorSelection { .. } + | RuntimeCall::Utility { .. } + | RuntimeCall::Multisig { .. } ), } }