From 41b65c28cf3dbfc171bae76fdffecc87064a98fe Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 7 Jun 2021 16:47:23 +0300 Subject: [PATCH] Flag for rejecting all outbound messages (#982) * flag for rejecting all outbound messages * update weights * Revert "update weights" This reverts commit 992b866edc7c9fd97013cee9cd68b9d783f8681e. * Revert "flag for rejecting all outbound messages" This reverts commit d094964bb4f8800ab6978b9d4b12aade2f80d266. * OperatingMode * Update modules/messages/src/lib.rs Co-authored-by: Hernando Castano * Poke CI * RustFmt Co-authored-by: Hernando Castano Co-authored-by: Hernando Castano --- bridges/modules/messages/src/lib.rs | 148 +++++++++++++++++++------ bridges/primitives/messages/Cargo.toml | 2 + bridges/primitives/messages/src/lib.rs | 24 ++++ 3 files changed, 142 insertions(+), 32 deletions(-) diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index 03f4b0ed74..4e9800e0c7 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -50,7 +50,7 @@ use bp_messages::{ source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, RelayersRewards, TargetHeaderChain}, target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, total_unrewarded_messages, InboundLaneData, LaneId, MessageData, MessageKey, MessageNonce, MessagePayload, - OutboundLaneData, Parameter as MessagesParameter, UnrewardedRelayersState, + OperatingMode, OutboundLaneData, Parameter as MessagesParameter, UnrewardedRelayersState, }; use bp_runtime::Size; use codec::{Decode, Encode}; @@ -198,8 +198,10 @@ decl_storage! { /// runtime methods may still be used to do that (i.e. democracy::referendum to update halt /// flag directly or call the `halt_operations`). pub PalletOwner get(fn module_owner): Option; - /// If true, all pallet transactions are failed immediately. - pub IsHalted get(fn is_halted) config(): bool; + /// The current operating mode of the pallet. + /// + /// Depending on the mode either all, some, or no transactions will be allowed. + pub PalletOperatingMode get(fn operating_mode) config(): OperatingMode; /// Map of lane id => inbound lane data. pub InboundLanes: map hasher(blake2_128_concat) LaneId => InboundLaneData; /// Map of lane id => outbound lane data. @@ -266,19 +268,18 @@ decl_module! { } } - /// Halt or resume all pallet operations. + /// Halt or resume all/some pallet operations. /// /// May only be called either by root, or by `PalletOwner`. #[weight = (T::DbWeight::get().reads_writes(1, 1), DispatchClass::Operational)] - pub fn set_operational(origin, operational: bool) { + pub fn set_operating_mode(origin, operating_mode: OperatingMode) { ensure_owner_or_root::(origin)?; - >::put(operational); - - if operational { - log::info!(target: "runtime::bridge-messages", "Resuming pallet operations."); - } else { - log::warn!(target: "runtime::bridge-messages", "Stopping pallet operations."); - } + >::put(operating_mode); + log::info!( + target: "runtime::bridge-messages", + "Setting messages pallet operating mode to {:?}.", + operating_mode, + ); } /// Update pallet parameter. @@ -301,7 +302,7 @@ decl_module! { payload: T::OutboundPayload, delivery_and_dispatch_fee: T::OutboundMessageFee, ) -> DispatchResult { - ensure_operational::()?; + ensure_normal_operating_mode::()?; let submitter = origin.into().map_err(|_| BadOrigin)?; // let's first check if message can be delivered to target chain @@ -384,6 +385,7 @@ decl_module! { nonce: MessageNonce, additional_fee: T::OutboundMessageFee, ) -> DispatchResult { + ensure_not_halted::()?; // if someone tries to pay for already-delivered message, we're rejecting this intention // (otherwise this additional fee will be locked forever in relayers fund) // @@ -441,7 +443,7 @@ decl_module! { messages_count: u32, dispatch_weight: Weight, ) -> DispatchResult { - ensure_operational::()?; + ensure_not_halted::()?; let _ = ensure_signed(origin)?; // reject transactions that are declaring too many messages @@ -532,7 +534,7 @@ decl_module! { proof: MessagesDeliveryProofOf, relayers_state: UnrewardedRelayersState, ) -> DispatchResult { - ensure_operational::()?; + ensure_not_halted::()?; let confirmation_relayer = ensure_signed(origin)?; let (lane_id, lane_data) = T::TargetHeaderChain::verify_messages_delivery_proof(proof).map_err(|err| { @@ -697,9 +699,18 @@ fn ensure_owner_or_root, I: Instance>(origin: T::Origin) -> Result< } } -/// Ensure that the pallet is in operational mode (not halted). -fn ensure_operational, I: Instance>() -> Result<(), Error> { - if IsHalted::::get() { +/// Ensure that the pallet is in normal operational mode. +fn ensure_normal_operating_mode, I: Instance>() -> Result<(), Error> { + if PalletOperatingMode::::get() != OperatingMode::Normal { + Err(Error::::Halted) + } else { + Ok(()) + } +} + +/// Ensure that the pallet is not halted. +fn ensure_not_halted, I: Instance>() -> Result<(), Error> { + if PalletOperatingMode::::get() == OperatingMode::Halted { Err(Error::::Halted) } else { Ok(()) @@ -922,29 +933,41 @@ mod tests { assert_ok!(Pallet::::set_owner(Origin::root(), Some(1))); assert_noop!( - Pallet::::set_operational(Origin::signed(2), false), + Pallet::::set_operating_mode(Origin::signed(2), OperatingMode::Halted), DispatchError::BadOrigin, ); - assert_ok!(Pallet::::set_operational(Origin::root(), false)); + assert_ok!(Pallet::::set_operating_mode( + Origin::root(), + OperatingMode::Halted + )); assert_ok!(Pallet::::set_owner(Origin::signed(1), None)); assert_noop!( - Pallet::::set_operational(Origin::signed(1), true), + Pallet::::set_operating_mode(Origin::signed(1), OperatingMode::Normal), DispatchError::BadOrigin, ); assert_noop!( - Pallet::::set_operational(Origin::signed(2), true), + Pallet::::set_operating_mode(Origin::signed(2), OperatingMode::Normal), DispatchError::BadOrigin, ); - assert_ok!(Pallet::::set_operational(Origin::root(), true)); + assert_ok!(Pallet::::set_operating_mode( + Origin::root(), + OperatingMode::Normal + )); }); } #[test] fn pallet_may_be_halted_by_root() { run_test(|| { - assert_ok!(Pallet::::set_operational(Origin::root(), false)); - assert_ok!(Pallet::::set_operational(Origin::root(), true)); + assert_ok!(Pallet::::set_operating_mode( + Origin::root(), + OperatingMode::Halted + )); + assert_ok!(Pallet::::set_operating_mode( + Origin::root(), + OperatingMode::Normal + )); }); } @@ -953,21 +976,30 @@ mod tests { run_test(|| { PalletOwner::::put(2); - assert_ok!(Pallet::::set_operational(Origin::signed(2), false)); - assert_ok!(Pallet::::set_operational(Origin::signed(2), true)); + assert_ok!(Pallet::::set_operating_mode( + Origin::signed(2), + OperatingMode::Halted + )); + assert_ok!(Pallet::::set_operating_mode( + Origin::signed(2), + OperatingMode::Normal + )); assert_noop!( - Pallet::::set_operational(Origin::signed(1), false), + Pallet::::set_operating_mode(Origin::signed(1), OperatingMode::Halted), DispatchError::BadOrigin, ); assert_noop!( - Pallet::::set_operational(Origin::signed(1), true), + Pallet::::set_operating_mode(Origin::signed(1), OperatingMode::Normal), DispatchError::BadOrigin, ); - assert_ok!(Pallet::::set_operational(Origin::signed(2), false)); + assert_ok!(Pallet::::set_operating_mode( + Origin::signed(2), + OperatingMode::Halted + )); assert_noop!( - Pallet::::set_operational(Origin::signed(1), true), + Pallet::::set_operating_mode(Origin::signed(1), OperatingMode::Normal), DispatchError::BadOrigin, ); }); @@ -1074,7 +1106,7 @@ mod tests { // send message first to be able to check that delivery_proof fails later send_regular_message(); - IsHalted::::put(true); + PalletOperatingMode::::put(OperatingMode::Halted); assert_noop!( Pallet::::send_message( @@ -1086,6 +1118,11 @@ mod tests { Error::::Halted, ); + assert_noop!( + Pallet::::increase_message_fee(Origin::signed(1), TEST_LANE_ID, 1, 1,), + Error::::Halted, + ); + assert_noop!( Pallet::::receive_messages_proof( Origin::signed(1), @@ -1114,6 +1151,53 @@ mod tests { }); } + #[test] + fn pallet_rejects_new_messages_in_rejecting_outbound_messages_operating_mode() { + run_test(|| { + // send message first to be able to check that delivery_proof fails later + send_regular_message(); + + PalletOperatingMode::::put(OperatingMode::RejectingOutboundMessages); + + assert_noop!( + Pallet::::send_message( + Origin::signed(1), + TEST_LANE_ID, + REGULAR_PAYLOAD, + REGULAR_PAYLOAD.1, + ), + Error::::Halted, + ); + + assert_ok!(Pallet::::increase_message_fee( + Origin::signed(1), + TEST_LANE_ID, + 1, + 1, + )); + + assert_ok!(Pallet::::receive_messages_proof( + Origin::signed(1), + TEST_RELAYER_A, + Ok(vec![message(1, REGULAR_PAYLOAD)]).into(), + 1, + REGULAR_PAYLOAD.1, + ),); + + assert_ok!(Pallet::::receive_messages_delivery_proof( + Origin::signed(1), + TestMessagesDeliveryProof(Ok(( + TEST_LANE_ID, + InboundLaneData { + last_confirmed_nonce: 1, + ..Default::default() + }, + ))), + Default::default(), + )); + }); + } + #[test] fn send_message_works() { run_test(|| { diff --git a/bridges/primitives/messages/Cargo.toml b/bridges/primitives/messages/Cargo.toml index 9cb037a34c..9b79a3bdb5 100644 --- a/bridges/primitives/messages/Cargo.toml +++ b/bridges/primitives/messages/Cargo.toml @@ -8,6 +8,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] } +serde = { version = "1.0.101", optional = true, features = ["derive"] } # Bridge dependencies @@ -26,5 +27,6 @@ std = [ "codec/std", "frame-support/std", "frame-system/std", + "serde", "sp-std/std" ] diff --git a/bridges/primitives/messages/src/lib.rs b/bridges/primitives/messages/src/lib.rs index c3ffce8baa..2bc3372329 100644 --- a/bridges/primitives/messages/src/lib.rs +++ b/bridges/primitives/messages/src/lib.rs @@ -32,6 +32,30 @@ pub mod target_chain; // Weight is reexported to avoid additional frame-support dependencies in related crates. pub use frame_support::weights::Weight; +/// Messages pallet operating mode. +#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug)] +#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] +pub enum OperatingMode { + /// Normal mode, when all operations are allowed. + Normal, + /// The pallet is not accepting outbound messages. Inbound messages and receival proofs + /// are still accepted. + /// + /// This mode may be used e.g. when bridged chain expects upgrade. Then to avoid dispatch + /// failures, the pallet owner may stop accepting new messages, while continuing to deliver + /// queued messages to the bridged chain. Once upgrade is completed, the mode may be switched + /// back to `Normal`. + RejectingOutboundMessages, + /// The pallet is halted. All operations (except operating mode change) are prohibited. + Halted, +} + +impl Default for OperatingMode { + fn default() -> Self { + OperatingMode::Normal + } +} + /// Messages pallet parameter. pub trait Parameter: frame_support::Parameter { /// Save parameter value in the runtime storage.