Skip to content

Commit

Permalink
when GRANDPA pallet is halted, relay shall not submit finality transa…
Browse files Browse the repository at this point in the history
…ctions (paritytech#1288)
  • Loading branch information
svyatonik authored and serban300 committed Apr 8, 2024
1 parent ff4c14e commit 65334f5
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 12 deletions.
5 changes: 5 additions & 0 deletions bridges/modules/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1151,6 +1151,11 @@ mod tests {

#[test]
fn storage_keys_computed_properly() {
assert_eq!(
IsHalted::<TestRuntime>::storage_value_final_key().to_vec(),
bp_header_chain::storage_keys::is_halted_key("Grandpa").0,
);

assert_eq!(
BestFinalized::<TestRuntime>::storage_value_final_key().to_vec(),
bp_header_chain::storage_keys::best_finalized_hash_key("Grandpa").0,
Expand Down
32 changes: 29 additions & 3 deletions bridges/primitives/header-chain/src/storage_keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions bridges/relays/client-substrate/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
6 changes: 3 additions & 3 deletions bridges/relays/lib-substrate-relay/src/finality_pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down
21 changes: 18 additions & 3 deletions bridges/relays/lib-substrate-relay/src/finality_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -50,6 +50,19 @@ impl<P: SubstrateFinalitySyncPipeline> SubstrateFinalityTarget<P> {
) -> 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<P: SubstrateFinalitySyncPipeline> Clone for SubstrateFinalityTarget<P> {
Expand Down Expand Up @@ -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,
Expand Down
16 changes: 13 additions & 3 deletions bridges/relays/lib-substrate-relay/src/messages_lane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
);

Expand Down

0 comments on commit 65334f5

Please sign in to comment.