From 65334f51c73ab2d15412da2f56615d3a9de4323b Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 18 Jan 2022 17:07:06 +0300 Subject: [PATCH] when GRANDPA pallet is halted, relay shall not submit finality transactions (#1288) --- bridges/modules/grandpa/src/lib.rs | 5 +++ .../header-chain/src/storage_keys.rs | 32 +++++++++++++++++-- bridges/relays/client-substrate/src/error.rs | 3 ++ .../src/finality_pipeline.rs | 6 ++-- .../src/finality_target.rs | 21 ++++++++++-- .../lib-substrate-relay/src/messages_lane.rs | 16 ++++++++-- 6 files changed, 71 insertions(+), 12 deletions(-) diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index 1dadc58fe8688..31b5280bb039a 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -1151,6 +1151,11 @@ mod tests { #[test] fn storage_keys_computed_properly() { + assert_eq!( + IsHalted::::storage_value_final_key().to_vec(), + bp_header_chain::storage_keys::is_halted_key("Grandpa").0, + ); + assert_eq!( BestFinalized::::storage_value_final_key().to_vec(), bp_header_chain::storage_keys::best_finalized_hash_key("Grandpa").0, diff --git a/bridges/primitives/header-chain/src/storage_keys.rs b/bridges/primitives/header-chain/src/storage_keys.rs index 460dbb8dc4d0e..e123703eed50e 100644 --- a/bridges/primitives/header-chain/src/storage_keys.rs +++ b/bridges/primitives/header-chain/src/storage_keys.rs @@ -16,17 +16,30 @@ //! Storage keys of bridge GRANDPA pallet. -/// Name of the `BestFinalized` storage map. -pub const BEST_FINALIZED_MAP_NAME: &str = "BestFinalized"; +/// Name of the `IsHalted` storage value. +pub const IS_HALTED_VALUE_NAME: &str = "IsHalted"; +/// Name of the `BestFinalized` storage value. +pub const BEST_FINALIZED_VALUE_NAME: &str = "BestFinalized"; use sp_core::storage::StorageKey; +/// Storage key of the `IsHalted` flag in the runtime storage. +pub fn is_halted_key(pallet_prefix: &str) -> StorageKey { + StorageKey( + bp_runtime::storage_value_final_key( + pallet_prefix.as_bytes(), + IS_HALTED_VALUE_NAME.as_bytes(), + ) + .to_vec(), + ) +} + /// Storage key of the best finalized header hash value in the runtime storage. pub fn best_finalized_hash_key(pallet_prefix: &str) -> StorageKey { StorageKey( bp_runtime::storage_value_final_key( pallet_prefix.as_bytes(), - BEST_FINALIZED_MAP_NAME.as_bytes(), + BEST_FINALIZED_VALUE_NAME.as_bytes(), ) .to_vec(), ) @@ -37,6 +50,19 @@ mod tests { use super::*; use hex_literal::hex; + #[test] + fn is_halted_key_computed_properly() { + // If this test fails, then something has been changed in module storage that is breaking + // compatibility with previous pallet. + let storage_key = is_halted_key("BridgeGrandpa").0; + assert_eq!( + storage_key, + hex!("0b06f475eddb98cf933a12262e0388de9611a984bbd04e2fd39f97bbc006115f").to_vec(), + "Unexpected storage key: {}", + hex::encode(&storage_key), + ); + } + #[test] fn best_finalized_hash_key_computed_properly() { // If this test fails, then something has been changed in module storage that is breaking diff --git a/bridges/relays/client-substrate/src/error.rs b/bridges/relays/client-substrate/src/error.rs index 33b9b22a03efe..309531dd26099 100644 --- a/bridges/relays/client-substrate/src/error.rs +++ b/bridges/relays/client-substrate/src/error.rs @@ -51,6 +51,9 @@ pub enum Error { /// The client we're connected to is not synced, so we can't rely on its state. #[error("Substrate client is not synced {0}.")] ClientNotSynced(Health), + /// The bridge pallet is halted and all transactions will be rejected. + #[error("Bridge pallet is halted.")] + BridgePalletIsHalted, /// An error has happened when we have tried to parse storage proof. #[error("Error when parsing storage proof: {0:?}.")] StorageProofError(bp_runtime::StorageProofError), diff --git a/bridges/relays/lib-substrate-relay/src/finality_pipeline.rs b/bridges/relays/lib-substrate-relay/src/finality_pipeline.rs index 07a1279ef4603..84fb8661016b0 100644 --- a/bridges/relays/lib-substrate-relay/src/finality_pipeline.rs +++ b/bridges/relays/lib-substrate-relay/src/finality_pipeline.rs @@ -27,8 +27,8 @@ use bp_header_chain::justification::GrandpaJustification; use finality_relay::FinalitySyncPipeline; use pallet_bridge_grandpa::{Call as BridgeGrandpaCall, Config as BridgeGrandpaConfig}; use relay_substrate_client::{ - transaction_stall_timeout, AccountIdOf, AccountKeyPairOf, BlockNumberOf, CallOf, Chain, Client, - HashOf, HeaderOf, SyncHeader, TransactionSignScheme, + transaction_stall_timeout, AccountIdOf, AccountKeyPairOf, BlockNumberOf, CallOf, Chain, + ChainWithGrandpa, Client, HashOf, HeaderOf, SyncHeader, TransactionSignScheme, }; use relay_utils::metrics::MetricsParams; use sp_core::Pair; @@ -44,7 +44,7 @@ pub(crate) const RECENT_FINALITY_PROOFS_LIMIT: usize = 4096; #[async_trait] pub trait SubstrateFinalitySyncPipeline: 'static + Clone + Debug + Send + Sync { /// Headers of this chain are submitted to the `TargetChain`. - type SourceChain: Chain; + type SourceChain: ChainWithGrandpa; /// Headers of the `SourceChain` are submitted to this chain. type TargetChain: Chain; diff --git a/bridges/relays/lib-substrate-relay/src/finality_target.rs b/bridges/relays/lib-substrate-relay/src/finality_target.rs index 918c633d7420a..9a92c88a234a9 100644 --- a/bridges/relays/lib-substrate-relay/src/finality_target.rs +++ b/bridges/relays/lib-substrate-relay/src/finality_target.rs @@ -26,12 +26,12 @@ use crate::{ }; use async_trait::async_trait; -use bp_header_chain::justification::GrandpaJustification; +use bp_header_chain::{justification::GrandpaJustification, storage_keys::is_halted_key}; use codec::Encode; use finality_relay::TargetClient; use relay_substrate_client::{ - AccountIdOf, AccountKeyPairOf, BlockNumberOf, Chain, Client, Error, HashOf, HeaderOf, - SignParam, SyncHeader, TransactionEra, TransactionSignScheme, UnsignedTransaction, + AccountIdOf, AccountKeyPairOf, BlockNumberOf, Chain, ChainWithGrandpa, Client, Error, HashOf, + HeaderOf, SignParam, SyncHeader, TransactionEra, TransactionSignScheme, UnsignedTransaction, }; use relay_utils::relay_loop::Client as RelayClient; use sp_core::{Bytes, Pair}; @@ -50,6 +50,19 @@ impl SubstrateFinalityTarget

{ ) -> Self { SubstrateFinalityTarget { client, transaction_params } } + + /// Ensure that the GRANDPA pallet at target chain is active. + async fn ensure_pallet_active(&self) -> Result<(), Error> { + let is_halted = self + .client + .storage_value(is_halted_key(P::SourceChain::WITH_CHAIN_GRANDPA_PALLET_NAME), None) + .await?; + if is_halted.unwrap_or(false) { + Err(Error::BridgePalletIsHalted) + } else { + Ok(()) + } + } } impl Clone for SubstrateFinalityTarget

{ @@ -83,6 +96,8 @@ where // we can't continue to relay finality if target node is out of sync, because // it may have already received (some of) headers that we're going to relay self.client.ensure_synced().await?; + // we can't relay finality if GRANDPA pallet at target chain is halted + self.ensure_pallet_active().await?; Ok(crate::messages_source::read_client_state::< P::TargetChain, diff --git a/bridges/relays/lib-substrate-relay/src/messages_lane.rs b/bridges/relays/lib-substrate-relay/src/messages_lane.rs index 87146be4896cc..380d1c9624f32 100644 --- a/bridges/relays/lib-substrate-relay/src/messages_lane.rs +++ b/bridges/relays/lib-substrate-relay/src/messages_lane.rs @@ -34,8 +34,8 @@ use frame_support::weights::{GetDispatchInfo, Weight}; use messages_relay::{message_lane::MessageLane, relay_strategy::RelayStrategy}; use pallet_bridge_messages::{Call as BridgeMessagesCall, Config as BridgeMessagesConfig}; use relay_substrate_client::{ - AccountKeyPairOf, BalanceOf, BlockNumberOf, CallOf, Chain, ChainWithMessages, Client, HashOf, - TransactionSignScheme, + transaction_stall_timeout, AccountKeyPairOf, BalanceOf, BlockNumberOf, CallOf, Chain, + ChainWithMessages, Client, HashOf, TransactionSignScheme, }; use relay_utils::metrics::MetricsParams; use sp_core::Pair; @@ -173,7 +173,7 @@ where Max messages in single transaction: {}\n\t\ Max messages size in single transaction: {}\n\t\ Max messages weight in single transaction: {}\n\t\ - Tx mortality: {:?}/{:?}\n\t\ + Tx mortality: {:?} (~{}m)/{:?} (~{}m)\n\t\ Stall timeout: {:?}", P::SourceChain::NAME, P::TargetChain::NAME, @@ -183,7 +183,17 @@ where max_messages_size_in_single_batch, max_messages_weight_in_single_batch, params.source_transaction_params.mortality, + transaction_stall_timeout( + params.source_transaction_params.mortality, + P::SourceChain::AVERAGE_BLOCK_INTERVAL, + STALL_TIMEOUT, + ).as_secs_f64() / 60.0f64, params.target_transaction_params.mortality, + transaction_stall_timeout( + params.target_transaction_params.mortality, + P::TargetChain::AVERAGE_BLOCK_INTERVAL, + STALL_TIMEOUT, + ).as_secs_f64() / 60.0f64, stall_timeout, );