diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index 1527de80fc..726de9c507 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -37,12 +37,12 @@ #![allow(clippy::large_enum_variant)] use bp_header_chain::{justification::GrandpaJustification, InitializationData}; -use bp_runtime::{BlockNumberOf, Chain, HashOf, HasherOf, HeaderOf}; +use bp_runtime::{BlockNumberOf, Chain, HashOf, HasherOf, HeaderOf, OwnedBridgeModule}; use finality_grandpa::voter_set::VoterSet; use frame_support::{ensure, fail}; -use frame_system::{ensure_signed, RawOrigin}; +use frame_system::ensure_signed; use sp_finality_grandpa::{ConsensusLog, GRANDPA_ENGINE_ID}; -use sp_runtime::traits::{BadOrigin, Header as HeaderT, Zero}; +use sp_runtime::traits::{Header as HeaderT, Zero}; use sp_std::{boxed::Box, convert::TryInto}; mod extension; @@ -117,6 +117,18 @@ pub mod pallet { } } + impl, I: 'static> OwnedBridgeModule for Pallet { + const LOG_TARGET: &'static str = "runtime::bridge-grandpa"; + const OPERATING_MODE_KEY: &'static str = "IsHalted"; + type OwnerStorage = PalletOwner; + type OperatingMode = bool; + type OperatingModeStorage = IsHalted; + + fn is_halted() -> bool { + Self::OperatingModeStorage::get() + } + } + #[pallet::call] impl, I: 'static> Pallet { /// Verify a target header is finalized according to the given finality proof. @@ -135,7 +147,7 @@ pub mod pallet { finality_target: Box>, justification: GrandpaJustification>, ) -> DispatchResultWithPostInfo { - ensure_operational::()?; + Self::ensure_not_halted().map_err(Error::::BridgeModule)?; let _ = ensure_signed(origin)?; ensure!(Self::request_count() < T::MaxRequests::get(), >::TooManyRequests); @@ -198,7 +210,7 @@ pub mod pallet { origin: OriginFor, init_data: super::InitializationData>, ) -> DispatchResultWithPostInfo { - ensure_owner_or_root::(origin)?; + Self::ensure_owner_or_root(origin)?; let init_allowed = !>::exists(); ensure!(init_allowed, >::AlreadyInitialized); @@ -217,43 +229,16 @@ pub mod pallet { /// /// May only be called either by root, or by `PalletOwner`. #[pallet::weight((T::DbWeight::get().reads_writes(1, 1), DispatchClass::Operational))] - pub fn set_owner( - origin: OriginFor, - new_owner: Option, - ) -> DispatchResultWithPostInfo { - ensure_owner_or_root::(origin)?; - match new_owner { - Some(new_owner) => { - PalletOwner::::put(&new_owner); - log::info!(target: "runtime::bridge-grandpa", "Setting pallet Owner to: {:?}", new_owner); - }, - None => { - PalletOwner::::kill(); - log::info!(target: "runtime::bridge-grandpa", "Removed Owner of pallet."); - }, - } - - Ok(().into()) + pub fn set_owner(origin: OriginFor, new_owner: Option) -> DispatchResult { + >::set_owner(origin, new_owner) } /// Halt or resume all pallet operations. /// /// May only be called either by root, or by `PalletOwner`. #[pallet::weight((T::DbWeight::get().reads_writes(1, 1), DispatchClass::Operational))] - pub fn set_operational( - origin: OriginFor, - operational: bool, - ) -> DispatchResultWithPostInfo { - ensure_owner_or_root::(origin)?; - >::put(!operational); - - if operational { - log::info!(target: "runtime::bridge-grandpa", "Resuming pallet operations."); - } else { - log::warn!(target: "runtime::bridge-grandpa", "Stopping pallet operations."); - } - - Ok(().into()) + pub fn set_operational(origin: OriginFor, operational: bool) -> DispatchResult { + Self::set_operating_mode(origin, !operational) } } @@ -310,7 +295,7 @@ pub mod pallet { /// If true, all pallet transactions are failed immediately. #[pallet::storage] - pub(super) type IsHalted, I: 'static = ()> = StorageValue<_, bool, ValueQuery>; + pub type IsHalted, I: 'static = ()> = StorageValue<_, bool, ValueQuery>; #[pallet::genesis_config] pub struct GenesisConfig, I: 'static = ()> { @@ -364,10 +349,10 @@ pub mod pallet { NotInitialized, /// The pallet has already been initialized. AlreadyInitialized, - /// All pallet operations are halted. - Halted, /// The storage proof doesn't contains storage root. So it is invalid for given header. StorageRootMismatch, + /// Error generated by the `OwnedBridgeModule` trait. + BridgeModule(bp_runtime::OwnedBridgeModuleError), } /// Check the given header for a GRANDPA scheduled authority set change. If a change @@ -513,26 +498,6 @@ pub mod pallet { insert_header::(header, hash); } } - - /// Ensure that the origin is either root, or `PalletOwner`. - fn ensure_owner_or_root, I: 'static>(origin: T::Origin) -> Result<(), BadOrigin> { - match origin.into() { - Ok(RawOrigin::Root) => Ok(()), - Ok(RawOrigin::Signed(ref signer)) - if Some(signer) == >::get().as_ref() => - Ok(()), - _ => Err(BadOrigin), - } - } - - /// Ensure that the pallet is in operational mode (not halted). - fn ensure_operational, I: 'static>() -> Result<(), Error> { - if >::get() { - Err(>::Halted) - } else { - Ok(()) - } - } } impl, I: 'static> Pallet { @@ -799,7 +764,10 @@ mod tests { initialize_substrate_bridge(); assert_ok!(Pallet::::set_operational(Origin::root(), false)); - assert_noop!(submit_finality_proof(1), Error::::Halted); + assert_noop!( + submit_finality_proof(1), + Error::::BridgeModule(bp_runtime::OwnedBridgeModuleError::Halted) + ); assert_ok!(Pallet::::set_operational(Origin::root(), true)); assert_ok!(submit_finality_proof(1)); diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index 0f287a9df4..78f44e7502 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -61,17 +61,16 @@ use bp_messages::{ OutboundMessageDetails, Parameter as MessagesParameter, UnrewardedRelayer, UnrewardedRelayersState, }; -use bp_runtime::{ChainId, Size}; +use bp_runtime::{ChainId, OwnedBridgeModule, Size}; use codec::{Decode, Encode}; use frame_support::{ fail, traits::Get, weights::{Pays, PostDispatchInfo}, }; -use frame_system::RawOrigin; use num_traits::{SaturatingAdd, Zero}; use sp_core::H256; -use sp_runtime::traits::{BadOrigin, Convert}; +use sp_runtime::traits::Convert; use sp_std::{ cell::RefCell, cmp::PartialOrd, collections::vec_deque::VecDeque, marker::PhantomData, ops::RangeInclusive, prelude::*, @@ -217,6 +216,18 @@ pub mod pallet { #[pallet::without_storage_info] pub struct Pallet(PhantomData<(T, I)>); + impl, I: 'static> OwnedBridgeModule for Pallet { + const LOG_TARGET: &'static str = "runtime::bridge-messages"; + const OPERATING_MODE_KEY: &'static str = "PalletOperatingMode"; + type OwnerStorage = PalletOwner; + type OperatingMode = OperatingMode; + type OperatingModeStorage = PalletOperatingMode; + + fn is_halted() -> bool { + Self::OperatingModeStorage::get() == OperatingMode::Halted + } + } + #[pallet::call] impl, I: 'static> Pallet { /// Change `PalletOwner`. @@ -224,18 +235,7 @@ pub mod pallet { /// May only be called either by root, or by `PalletOwner`. #[pallet::weight((T::DbWeight::get().reads_writes(1, 1), DispatchClass::Operational))] pub fn set_owner(origin: OriginFor, new_owner: Option) -> DispatchResult { - ensure_owner_or_root::(origin)?; - match new_owner { - Some(new_owner) => { - PalletOwner::::put(&new_owner); - log::info!(target: "runtime::bridge-messages", "Setting pallet Owner to: {:?}", new_owner); - }, - None => { - PalletOwner::::kill(); - log::info!(target: "runtime::bridge-messages", "Removed Owner of pallet."); - }, - } - Ok(()) + >::set_owner(origin, new_owner) } /// Halt or resume all/some pallet operations. @@ -246,14 +246,7 @@ pub mod pallet { origin: OriginFor, operating_mode: OperatingMode, ) -> DispatchResult { - ensure_owner_or_root::(origin)?; - PalletOperatingMode::::put(operating_mode); - log::info!( - target: "runtime::bridge-messages", - "Setting messages pallet operating mode to {:?}.", - operating_mode, - ); - Ok(()) + >::set_operating_mode(origin, operating_mode) } /// Update pallet parameter. @@ -267,7 +260,7 @@ pub mod pallet { origin: OriginFor, parameter: T::Parameter, ) -> DispatchResult { - ensure_owner_or_root::(origin)?; + Self::ensure_owner_or_root(origin)?; parameter.save(); Self::deposit_event(Event::ParameterUpdated(parameter)); Ok(()) @@ -297,7 +290,7 @@ pub mod pallet { nonce: MessageNonce, additional_fee: T::OutboundMessageFee, ) -> DispatchResultWithPostInfo { - ensure_not_halted::()?; + Self::ensure_not_halted().map_err(Error::::BridgeModule)?; // 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) // @@ -368,7 +361,7 @@ pub mod pallet { messages_count: u32, dispatch_weight: Weight, ) -> DispatchResultWithPostInfo { - ensure_not_halted::()?; + Self::ensure_not_halted().map_err(Error::::BridgeModule)?; let relayer_id_at_this_chain = ensure_signed(origin)?; // reject transactions that are declaring too many messages @@ -512,7 +505,7 @@ pub mod pallet { proof: MessagesDeliveryProofOf, relayers_state: UnrewardedRelayersState, ) -> DispatchResultWithPostInfo { - ensure_not_halted::()?; + Self::ensure_not_halted().map_err(Error::::BridgeModule)?; // why do we need to know the weight of this (`receive_messages_delivery_proof`) call? // Because we may want to return some funds for messages that are not processed by the @@ -669,8 +662,8 @@ pub mod pallet { #[pallet::error] pub enum Error { - /// All pallet operations are halted. - Halted, + /// Pallet is not in Normal operating mode. + NotOperatingNormally, /// Message has been treated as invalid by chain verifier. MessageRejectedByChainVerifier, /// Message has been treated as invalid by lane verifier. @@ -695,6 +688,8 @@ pub mod pallet { /// The number of actually confirmed messages is going to be larger than the number of /// messages in the proof. This may mean that this or bridged chain storage is corrupted. TryingToConfirmMoreMessagesThanExpected, + /// Error generated by the `OwnedBridgeModule` trait. + BridgeModule(bp_runtime::OwnedBridgeModuleError), } /// Optional pallet owner. @@ -975,30 +970,10 @@ where relayers_rewards } -/// Ensure that the origin is either root, or `PalletOwner`. -fn ensure_owner_or_root, I: 'static>(origin: T::Origin) -> Result<(), BadOrigin> { - match origin.into() { - Ok(RawOrigin::Root) => Ok(()), - Ok(RawOrigin::Signed(ref signer)) - if Some(signer) == Pallet::::module_owner().as_ref() => - Ok(()), - _ => Err(BadOrigin), - } -} - /// Ensure that the pallet is in normal operational mode. fn ensure_normal_operating_mode, I: 'static>() -> Result<(), Error> { if PalletOperatingMode::::get() != OperatingMode::Normal { - Err(Error::::Halted) - } else { - Ok(()) - } -} - -/// Ensure that the pallet is not halted. -fn ensure_not_halted, I: 'static>() -> Result<(), Error> { - if PalletOperatingMode::::get() == OperatingMode::Halted { - Err(Error::::Halted) + Err(Error::::NotOperatingNormally) } else { Ok(()) } @@ -1442,12 +1417,12 @@ mod tests { REGULAR_PAYLOAD, REGULAR_PAYLOAD.declared_weight, ), - Error::::Halted, + Error::::NotOperatingNormally, ); assert_noop!( Pallet::::increase_message_fee(Origin::signed(1), TEST_LANE_ID, 1, 1,), - Error::::Halted, + Error::::BridgeModule(bp_runtime::OwnedBridgeModuleError::Halted), ); assert_noop!( @@ -1458,7 +1433,7 @@ mod tests { 1, REGULAR_PAYLOAD.declared_weight, ), - Error::::Halted, + Error::::BridgeModule(bp_runtime::OwnedBridgeModuleError::Halted), ); assert_noop!( @@ -1480,7 +1455,7 @@ mod tests { last_delivered_nonce: 1, }, ), - Error::::Halted, + Error::::BridgeModule(bp_runtime::OwnedBridgeModuleError::Halted), ); }); } @@ -1500,7 +1475,7 @@ mod tests { REGULAR_PAYLOAD, REGULAR_PAYLOAD.declared_weight, ), - Error::::Halted, + Error::::NotOperatingNormally, ); assert_ok!(Pallet::::increase_message_fee( diff --git a/bridges/primitives/runtime/Cargo.toml b/bridges/primitives/runtime/Cargo.toml index 20208fd5fa..691426aaf0 100644 --- a/bridges/primitives/runtime/Cargo.toml +++ b/bridges/primitives/runtime/Cargo.toml @@ -15,6 +15,7 @@ scale-info = { version = "2.1.1", default-features = false, features = ["derive" # Substrate Dependencies frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +frame-system = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } sp-core = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } sp-io = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } @@ -30,6 +31,7 @@ default = ["std"] std = [ "codec/std", "frame-support/std", + "frame-system/std", "hash-db/std", "num-traits/std", "scale-info/std", diff --git a/bridges/primitives/runtime/src/lib.rs b/bridges/primitives/runtime/src/lib.rs index bd09a651c7..ee5e6b5d24 100644 --- a/bridges/primitives/runtime/src/lib.rs +++ b/bridges/primitives/runtime/src/lib.rs @@ -18,11 +18,16 @@ #![cfg_attr(not(feature = "std"), no_std)] -use codec::Encode; -use frame_support::{RuntimeDebug, StorageHasher}; +use codec::{Decode, Encode, FullCodec}; +use frame_support::{ + log, pallet_prelude::DispatchResult, PalletError, RuntimeDebug, StorageHasher, StorageValue, +}; +use frame_system::RawOrigin; +use scale_info::TypeInfo; use sp_core::{hash::H256, storage::StorageKey}; use sp_io::hashing::blake2_256; -use sp_std::{convert::TryFrom, vec, vec::Vec}; +use sp_runtime::traits::BadOrigin; +use sp_std::{convert::TryFrom, fmt::Debug, vec, vec::Vec}; pub use chain::{ AccountIdOf, AccountPublicOf, BalanceOf, BlockNumberOf, Chain, EncodedOrDecodedCall, HashOf, @@ -257,6 +262,79 @@ pub fn storage_value_key(pallet_prefix: &str, value_name: &str) -> StorageKey { StorageKey(final_key) } +/// Error generated by the `OwnedBridgeModule` trait. +#[derive(Encode, Decode, TypeInfo, PalletError)] +pub enum OwnedBridgeModuleError { + /// All pallet operations are halted. + Halted, +} + +/// Bridge module that has owner and operating mode +pub trait OwnedBridgeModule { + /// The target that will be used when publishing logs related to this module. + const LOG_TARGET: &'static str; + const OPERATING_MODE_KEY: &'static str; + + type OwnerStorage: StorageValue>; + type OperatingMode: Copy + Debug + FullCodec; + type OperatingModeStorage: StorageValue; + + /// Check if the module is halted. + fn is_halted() -> bool; + + /// Ensure that the origin is either root, or `PalletOwner`. + fn ensure_owner_or_root(origin: T::Origin) -> Result<(), BadOrigin> { + match origin.into() { + Ok(RawOrigin::Root) => Ok(()), + Ok(RawOrigin::Signed(ref signer)) + if Self::OwnerStorage::get().as_ref() == Some(signer) => + Ok(()), + _ => Err(BadOrigin), + } + } + + /// Ensure that the module is not halted. + fn ensure_not_halted() -> Result<(), OwnedBridgeModuleError> { + match Self::is_halted() { + true => Err(OwnedBridgeModuleError::Halted), + false => Ok(()), + } + } + + /// Change the owner of the module. + fn set_owner(origin: T::Origin, maybe_owner: Option) -> DispatchResult { + Self::ensure_owner_or_root(origin)?; + match maybe_owner { + Some(owner) => { + Self::OwnerStorage::put(&owner); + log::info!(target: Self::LOG_TARGET, "Setting pallet Owner to: {:?}", owner); + }, + None => { + Self::OwnerStorage::kill(); + log::info!(target: Self::LOG_TARGET, "Removed Owner of pallet."); + }, + } + + Ok(()) + } + + /// Halt or resume all/some module operations. + fn set_operating_mode( + origin: T::Origin, + operating_mode: Self::OperatingMode, + ) -> DispatchResult { + Self::ensure_owner_or_root(origin)?; + Self::OperatingModeStorage::put(operating_mode); + log::info!( + target: Self::LOG_TARGET, + "Setting operating mode ( {} = {:?}).", + Self::OPERATING_MODE_KEY, + operating_mode + ); + Ok(()) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/bridges/relays/lib-substrate-relay/src/lib.rs b/bridges/relays/lib-substrate-relay/src/lib.rs index 51edf85783..1af4f67a7c 100644 --- a/bridges/relays/lib-substrate-relay/src/lib.rs +++ b/bridges/relays/lib-substrate-relay/src/lib.rs @@ -97,12 +97,15 @@ impl TaggedAccount { pub fn tag(&self) -> String { match *self { TaggedAccount::Headers { ref bridged_chain, .. } => format!("{}Headers", bridged_chain), - TaggedAccount::Parachains { ref bridged_chain, .. } => - format!("{}Parachains", bridged_chain), - TaggedAccount::Messages { ref bridged_chain, .. } => - format!("{}Messages", bridged_chain), - TaggedAccount::MessagesPalletOwner { ref bridged_chain, .. } => - format!("{}MessagesPalletOwner", bridged_chain), + TaggedAccount::Parachains { ref bridged_chain, .. } => { + format!("{}Parachains", bridged_chain) + }, + TaggedAccount::Messages { ref bridged_chain, .. } => { + format!("{}Messages", bridged_chain) + }, + TaggedAccount::MessagesPalletOwner { ref bridged_chain, .. } => { + format!("{}MessagesPalletOwner", bridged_chain) + }, } } }