Simplify pallet operating modes (paritytech#969)

Co-authored-by: Vincent Geddes <vincent@snowfork.com> Co-authored-by: Clara van Staden <claravanstaden64@gmail.com>
Snowfork · Oct 27, 2023 · 0a2baf7 · 0a2baf7
1 parent 187c3ff
commit 0a2baf7
Show file tree

Hide file tree

Showing 20 changed files with 246 additions and 248 deletions.
diff --git a/cumulus b/cumulus
diff --git a/parachain/Cargo.lock b/parachain/Cargo.lock
diff --git a/parachain/pallets/control/src/lib.rs b/parachain/pallets/control/src/lib.rs
@@ -45,10 +45,10 @@ use frame_system::pallet_prelude::*;
 use snowbridge_core::{
 	outbound::{
 		Command, Initializer, Message, OperatingMode, OutboundQueue as OutboundQueueTrait, ParaId,
+		SendError,
 	},
 	sibling_sovereign_account, AgentId,
 };
-use sp_runtime::Saturating;
 
 #[cfg(feature = "runtime-benchmarks")]
 use frame_support::traits::OriginTrait;
@@ -158,14 +158,14 @@ pub mod pallet {
 
 	#[pallet::error]
 	pub enum Error<T> {
-		SubmissionFailed,
 		LocationConversionFailed,
 		AgentAlreadyCreated,
 		NoAgent,
 		ChannelAlreadyCreated,
 		NoChannel,
 		UnsupportedLocationVersion,
 		InvalidLocation,
+		Send(SendError),
 	}
 
 	/// The set of registered agents
@@ -409,10 +409,10 @@ pub mod pallet {
 		fn send(origin: ParaId, command: Command, pays_fee: PaysFee<T>) -> DispatchResult {
 			let message = Message { origin, command };
 			let (ticket, fee) =
-				T::OutboundQueue::validate(&message).map_err(|_| Error::<T>::SubmissionFailed)?;
+				T::OutboundQueue::validate(&message).map_err(|err| Error::<T>::Send(err))?;
 
 			let payment = match pays_fee {
-				PaysFee::Yes(account) => Some((account, fee.base.saturating_add(fee.delivery))),
+				PaysFee::Yes(account) => Some((account, fee.total())),
 				PaysFee::Partial(account) => Some((account, fee.base)),
 				PaysFee::No => None,
 			};
@@ -426,7 +426,7 @@ pub mod pallet {
 				)?;
 			}
 
-			T::OutboundQueue::submit(ticket).map_err(|_| Error::<T>::SubmissionFailed)?;
+			T::OutboundQueue::submit(ticket).map_err(|err| Error::<T>::Send(err))?;
 			Ok(())
 		}
 

diff --git a/parachain/pallets/ethereum-beacon-client/Cargo.toml b/parachain/pallets/ethereum-beacon-client/Cargo.toml
@@ -32,7 +32,6 @@ snowbridge-core = { path = "../../primitives/core", default-features = false }
 snowbridge-ethereum = { path = "../../primitives/ethereum", default-features = false }
 primitives = { package = "snowbridge-beacon-primitives", path = "../../primitives/beacon", default-features = false }
 static_assertions = { version = "1.1.0" }
-bp-runtime = { git = "https://github.com/Snowfork/cumulus.git", branch = "snowbridge", default-features = false }
 pallet-timestamp = { git = "https://github.com/paritytech/substrate.git", branch = "master", optional = true }
 sp-io = { git = "https://github.com/paritytech/substrate.git", branch = "master", optional = true }
 
@@ -69,7 +68,6 @@ std = [
     "primitives/std",
     "ssz_rs/std",
     "byte-slice-cast/std",
-    "bp-runtime/std",
 ]
 runtime-benchmarks = [
     "beacon-spec-mainnet",

diff --git a/parachain/pallets/ethereum-beacon-client/src/lib.rs b/parachain/pallets/ethereum-beacon-client/src/lib.rs
@@ -29,7 +29,7 @@ use primitives::{
 };
 use snowbridge_core::{
 	inbound::{Message, Proof, Verifier},
-	RingBufferMap,
+	BasicOperatingMode, RingBufferMap,
 };
 use sp_core::H256;
 use sp_std::prelude::*;
@@ -57,8 +57,6 @@ pub mod pallet {
 	use frame_support::pallet_prelude::*;
 	use frame_system::pallet_prelude::*;
 
-	use bp_runtime::{BasicOperatingMode, OwnedBridgeModule};
-
 	#[derive(scale_info::TypeInfo, codec::Encode, codec::Decode, codec::MaxEncodedLen)]
 	#[codec(mel_bound(T: Config))]
 	#[scale_info(skip_type_params(T))]
@@ -90,9 +88,21 @@ pub mod pallet {
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
 	pub enum Event<T: Config> {
-		BeaconHeaderImported { block_hash: H256, slot: u64 },
-		ExecutionHeaderImported { block_hash: H256, block_number: u64 },
-		SyncCommitteeUpdated { period: u64 },
+		BeaconHeaderImported {
+			block_hash: H256,
+			slot: u64,
+		},
+		ExecutionHeaderImported {
+			block_hash: H256,
+			block_number: u64,
+		},
+		SyncCommitteeUpdated {
+			period: u64,
+		},
+		/// Set OperatingMode
+		OperatingModeChanged {
+			mode: BasicOperatingMode,
+		},
 	}
 
 	#[pallet::error]
@@ -125,7 +135,7 @@ pub mod pallet {
 		InvalidSyncCommitteeUpdate,
 		ExecutionHeaderTooFarBehind,
 		ExecutionHeaderSkippedBlock,
-		BridgeModule(bp_runtime::OwnedBridgeModuleError),
+		Halted,
 	}
 
 	/// Latest imported checkpoint root
@@ -186,27 +196,10 @@ pub mod pallet {
 	#[pallet::storage]
 	pub type ExecutionHeaderMapping<T: Config> = StorageMap<_, Identity, u32, H256, ValueQuery>;
 
-	/// Optional pallet owner.
-	///
-	/// Pallet owner has a right to halt all pallet operations and then resume them. If it is
-	/// `None`, then there are no direct ways to halt/resume pallet operations, but other
-	/// runtime methods may still be used to do that (i.e. democracy::referendum to update halt
-	/// flag directly or call the `halt_operations`).
-	#[pallet::storage]
-	pub type PalletOwner<T: Config> = StorageValue<_, T::AccountId, OptionQuery>;
-
 	/// The current operating mode of the pallet.
-	///
-	/// Depending on the mode either all, or no transactions will be allowed.
 	#[pallet::storage]
-	pub type PalletOperatingMode<T: Config> = StorageValue<_, BasicOperatingMode, ValueQuery>;
-
-	impl<T: Config> OwnedBridgeModule<T> for Pallet<T> {
-		const LOG_TARGET: &'static str = LOG_TARGET;
-		type OwnerStorage = PalletOwner<T>;
-		type OperatingMode = BasicOperatingMode;
-		type OperatingModeStorage = PalletOperatingMode<T>;
-	}
+	#[pallet::getter(fn operating_mode)]
+	pub type OperatingMode<T: Config> = StorageValue<_, BasicOperatingMode, ValueQuery>;
 
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
@@ -235,8 +228,8 @@ pub mod pallet {
 		/// Submits a new finalized beacon header update. The update may contain the next
 		/// sync committee.
 		pub fn submit(origin: OriginFor<T>, update: Box<Update>) -> DispatchResult {
-			Self::ensure_not_halted().map_err(Error::<T>::BridgeModule)?;
 			ensure_signed(origin)?;
+			ensure!(!Self::operating_mode().is_halted(), Error::<T>::Halted);
 			Self::process_update(&update)?;
 			Ok(())
 		}
@@ -250,29 +243,23 @@ pub mod pallet {
 			origin: OriginFor<T>,
 			update: Box<ExecutionHeaderUpdate>,
 		) -> DispatchResult {
-			Self::ensure_not_halted().map_err(Error::<T>::BridgeModule)?;
 			ensure_signed(origin)?;
+			ensure!(!Self::operating_mode().is_halted(), Error::<T>::Halted);
 			Self::process_execution_header_update(&update)?;
 			Ok(())
 		}
 
-		/// Change `PalletOwner`.
-		/// May only be called either by root, or by `PalletOwner`.
+		/// Halt or resume all pallet operations. May only be called by root.
 		#[pallet::call_index(3)]
 		#[pallet::weight((T::DbWeight::get().reads_writes(1, 1), DispatchClass::Operational))]
-		pub fn set_owner(origin: OriginFor<T>, new_owner: Option<T::AccountId>) -> DispatchResult {
-			<Self as OwnedBridgeModule<_>>::set_owner(origin, new_owner)
-		}
-
-		/// Halt or resume all pallet operations.
-		/// May only be called either by root, or by `PalletOwner`.
-		#[pallet::call_index(4)]
-		#[pallet::weight((T::DbWeight::get().reads_writes(1, 1), DispatchClass::Operational))]
 		pub fn set_operating_mode(
 			origin: OriginFor<T>,
-			operating_mode: BasicOperatingMode,
+			mode: BasicOperatingMode,
 		) -> DispatchResult {
-			<Self as OwnedBridgeModule<_>>::set_operating_mode(origin, operating_mode)
+			ensure_root(origin)?;
+			OperatingMode::<T>::set(mode);
+			Self::deposit_event(Event::OperatingModeChanged { mode });
+			Ok(())
 		}
 	}
 

diff --git a/parachain/pallets/ethereum-beacon-client/src/tests.rs b/parachain/pallets/ethereum-beacon-client/src/tests.rs
@@ -10,7 +10,7 @@ use crate::{
 	NextSyncCommittee, SyncCommitteePrepared,
 };
 
-use frame_support::{assert_err, assert_ok};
+use frame_support::{assert_err, assert_noop, assert_ok, dispatch::DispatchError};
 use hex_literal::hex;
 use primitives::{
 	CompactExecutionHeader, ExecutionHeaderState, Fork, ForkVersions, NextSyncCommitteeUpdate,
@@ -976,3 +976,45 @@ fn verify_message_receipt_does_not_contain_log() {
 		assert_err!(EthereumBeaconClient::verify(&message), Error::<Test>::InvalidProof);
 	});
 }
+
+#[test]
+fn set_operating_mode() {
+	let checkpoint = load_checkpoint_update_fixture();
+	let update = load_finalized_header_update_fixture();
+	let execution_header_update = load_execution_header_update_fixture();
+
+	new_tester().execute_with(|| {
+		assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint));
+
+		assert_ok!(EthereumBeaconClient::set_operating_mode(
+			RuntimeOrigin::root(),
+			snowbridge_core::BasicOperatingMode::Halted
+		));
+
+		assert_noop!(
+			EthereumBeaconClient::submit(RuntimeOrigin::signed(1), Box::new(update)),
+			Error::<Test>::Halted
+		);
+
+		assert_noop!(
+			EthereumBeaconClient::submit_execution_header(
+				RuntimeOrigin::signed(1),
+				Box::new(execution_header_update)
+			),
+			Error::<Test>::Halted
+		);
+	});
+}
+
+#[test]
+fn set_operating_mode_root_only() {
+	new_tester().execute_with(|| {
+		assert_noop!(
+			EthereumBeaconClient::set_operating_mode(
+				RuntimeOrigin::signed(1),
+				snowbridge_core::BasicOperatingMode::Halted
+			),
+			DispatchError::BadOrigin
+		);
+	});
+}
diff --git a/parachain/pallets/inbound-queue/Cargo.toml b/parachain/pallets/inbound-queue/Cargo.toml
@@ -3,19 +3,18 @@ name = "snowbridge-inbound-queue"
 description = "Snowbridge Inbound Queue"
 version = "0.1.1"
 edition = "2021"
-authors = [ "Snowfork <contact@snowfork.com>" ]
+authors = ["Snowfork <contact@snowfork.com>"]
 repository = "https://github.com/Snowfork/snowbridge"
 
 [package.metadata.docs.rs]
-targets = [ "x86_64-unknown-linux-gnu" ]
+targets = ["x86_64-unknown-linux-gnu"]
 
 [dependencies]
 serde = { version = "1.0.164", optional = true }
-codec = { version = "3.1.5", package = "parity-scale-codec", default-features = false, features = [ "derive" ] }
-scale-info = { version = "2.7.0", default-features = false, features = [ "derive" ] }
+codec = { version = "3.1.5", package = "parity-scale-codec", default-features = false, features = ["derive"] }
+scale-info = { version = "2.7.0", default-features = false, features = ["derive"] }
 hex-literal = { version = "0.4.1", optional = true }
 rlp = { version = "0.5", default-features = false, optional = true }
-
 frame-benchmarking = { git = "https://github.com/paritytech/substrate.git", branch = "master", default-features = false, optional = true }
 frame-support = { git = "https://github.com/paritytech/substrate.git", branch = "master", default-features = false }
 frame-system = { git = "https://github.com/paritytech/substrate.git", branch = "master", default-features = false }
@@ -24,16 +23,13 @@ sp-core = { git = "https://github.com/paritytech/substrate.git", branch = "maste
 sp-std = { git = "https://github.com/paritytech/substrate.git", branch = "master", default-features = false }
 sp-io = { git = "https://github.com/paritytech/substrate.git", branch = "master", default-features = false }
 sp-runtime = { git = "https://github.com/paritytech/substrate.git", branch = "master", default-features = false }
-
 xcm = { git = "https://github.com/paritytech/polkadot.git", branch = "master", default-features = false }
 xcm-builder = { git = "https://github.com/paritytech/polkadot.git", branch = "master", default-features = false }
-bp-runtime = { git = "https://github.com/Snowfork/cumulus.git", branch = "snowbridge", default-features = false }
 
 snowbridge-core = { path = "../../primitives/core", default-features = false }
 snowbridge-ethereum = { path = "../../primitives/ethereum", default-features = false }
 snowbridge-router-primitives = { path = "../../primitives/router", default-features = false }
 ethabi = { git = "https://github.com/Snowfork/ethabi-decode.git", package = "ethabi-decode", branch = "master", default-features = false }
-
 snowbridge-beacon-primitives = { path = "../../primitives/beacon", default-features = false, optional = true }
 
 [dev-dependencies]
@@ -46,7 +42,7 @@ hex-literal = { version = "0.4.1" }
 rlp = { version = "0.5" }
 
 [features]
-default = [ "std" ]
+default = ["std"]
 std = [
     "serde",
     "codec/std",
@@ -64,8 +60,6 @@ std = [
     "snowbridge-router-primitives/std",
     "ethabi/std",
     "xcm/std",
-    "xcm-builder/std",
-    "bp-runtime/std",
 ]
 runtime-benchmarks = [
     "snowbridge-core/runtime-benchmarks",
+0 −1		Cargo.lock
+3 −7		parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs
+19 −30		parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/snowbridge_outbound_queue.rs
+1 −3		parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs