From d517da8a2766613e9830dc14e5f36fdd16f75b50 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 30 Jan 2023 12:34:52 +0300 Subject: [PATCH] Do not read parachain heads from ancient relay headers (#1827) * do not read parachain heads from ancient relay headers * revert test change --- relays/client-substrate/src/client.rs | 7 ++++- relays/client-substrate/src/lib.rs | 4 +-- .../src/on_demand/parachains.rs | 29 ++++++++++++------- .../src/parachains/source.rs | 11 ++++++- 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/relays/client-substrate/src/client.rs b/relays/client-substrate/src/client.rs index 62d6a7c599e..1a4aeb583cd 100644 --- a/relays/client-substrate/src/client.rs +++ b/relays/client-substrate/src/client.rs @@ -37,7 +37,7 @@ use jsonrpsee::{ core::DeserializeOwned, ws_client::{WsClient as RpcClient, WsClientBuilder as RpcClientBuilder}, }; -use num_traits::{Bounded, Zero}; +use num_traits::{Bounded, Saturating, Zero}; use pallet_balances::AccountData; use pallet_transaction_payment::InclusionFee; use relay_utils::{relay_loop::RECONNECT_DELAY, STALL_TIMEOUT}; @@ -69,6 +69,11 @@ const MAX_SUBSCRIPTION_CAPACITY: usize = 4096; /// half of this value. pub const ANCIENT_BLOCK_THRESHOLD: u32 = 128; +/// Returns `true` if we think that the state is already discarded for given block. +pub fn is_ancient_block + PartialOrd + Saturating>(block: N, best: N) -> bool { + best.saturating_sub(block) >= N::from(ANCIENT_BLOCK_THRESHOLD) +} + /// Opaque justifications subscription type. pub struct Subscription(pub(crate) Mutex>>); diff --git a/relays/client-substrate/src/lib.rs b/relays/client-substrate/src/lib.rs index ef75e304246..c1a96c487c4 100644 --- a/relays/client-substrate/src/lib.rs +++ b/relays/client-substrate/src/lib.rs @@ -40,8 +40,8 @@ pub use crate::{ UnsignedTransaction, UtilityPallet, }, client::{ - ChainRuntimeVersion, Client, OpaqueGrandpaAuthoritiesSet, SimpleRuntimeVersion, - Subscription, ANCIENT_BLOCK_THRESHOLD, + is_ancient_block, ChainRuntimeVersion, Client, OpaqueGrandpaAuthoritiesSet, + SimpleRuntimeVersion, Subscription, ANCIENT_BLOCK_THRESHOLD, }, error::{Error, Result}, rpc::{SubstrateBeefyFinalityClient, SubstrateFinalityClient, SubstrateGrandpaFinalityClient}, diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index c36eef99823..b6c30d56ee1 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -40,8 +40,8 @@ use parachains_relay::parachains_loop::{ AvailableHeader, ParachainSyncParams, SourceClient, TargetClient, }; use relay_substrate_client::{ - AccountIdOf, AccountKeyPairOf, BlockNumberOf, CallOf, Chain, Client, Error as SubstrateError, - HashOf, HeaderIdOf, ParachainBase, ANCIENT_BLOCK_THRESHOLD, + is_ancient_block, AccountIdOf, AccountKeyPairOf, BlockNumberOf, CallOf, Chain, Client, + Error as SubstrateError, HashOf, HeaderIdOf, ParachainBase, }; use relay_utils::{ metrics::MetricsParams, relay_loop::Client as RelayClient, BlockNumberBase, FailedClient, @@ -501,10 +501,18 @@ where .await .map_err(map_target_err)?; - let para_header_at_relay_header_at_target = source - .on_chain_para_head_id(relay_header_at_target, P::SourceParachain::PARACHAIN_ID.into()) - .await - .map_err(map_source_err)?; + // if relay header at target is too old, then its state may already be discarded at the source + // => just use `None` in this case + let is_relay_header_at_target_ancient = + is_ancient_block(relay_header_at_target.number(), relay_header_at_source); + let para_header_at_relay_header_at_target = if is_relay_header_at_target_ancient { + None + } else { + source + .on_chain_para_head_id(relay_header_at_target, P::SourceParachain::PARACHAIN_ID.into()) + .await + .map_err(map_source_err)? + }; Ok(RelayData { required_para_header: required_header_number, @@ -677,11 +685,10 @@ where // we don't require source node to be archive, so we can't craft storage proofs using // ancient headers. So if the `best_finalized_relay_block_at_target` is too ancient, we // can't craft storage proofs using it - let may_use_state_at_best_finalized_relay_block_at_target = - best_finalized_relay_block_at_source - .number() - .saturating_sub(best_finalized_relay_block_at_target.number()) <= - RBN::from(ANCIENT_BLOCK_THRESHOLD); + let may_use_state_at_best_finalized_relay_block_at_target = !is_ancient_block( + best_finalized_relay_block_at_target.number(), + best_finalized_relay_block_at_source.number(), + ); // now let's check if `required_header` may be proved using // `best_finalized_relay_block_at_target` diff --git a/relays/lib-substrate-relay/src/parachains/source.rs b/relays/lib-substrate-relay/src/parachains/source.rs index afacd18e292..90776902dd6 100644 --- a/relays/lib-substrate-relay/src/parachains/source.rs +++ b/relays/lib-substrate-relay/src/parachains/source.rs @@ -29,7 +29,8 @@ use parachains_relay::{ parachains_loop_metrics::ParachainsLoopMetrics, }; use relay_substrate_client::{ - Chain, Client, Error as SubstrateError, HeaderIdOf, HeaderOf, ParachainBase, RelayChain, + is_ancient_block, Chain, Client, Error as SubstrateError, HeaderIdOf, HeaderOf, ParachainBase, + RelayChain, }; use relay_utils::relay_loop::Client as RelayClient; @@ -115,6 +116,14 @@ where ))) } + // if requested relay header is ancient, then we don't even want to try to read the + // parachain head - we simply return `Unavailable` + let best_block_number = self.client.best_finalized_header_number().await?; + if is_ancient_block(at_block.number(), best_block_number) { + return Ok(AvailableHeader::Unavailable) + } + + // else - try to read head from the source client let mut para_head_id = AvailableHeader::Missing; if let Some(on_chain_para_head_id) = self.on_chain_para_head_id(at_block, para_id).await? { // Never return head that is larger than requested. This way we'll never sync