Do not read parachain heads from ancient relay headers (paritytech#1827)

* do not read parachain heads from ancient relay headers

* revert test change

Author: svyatonik

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<N: From<u32> + PartialOrd + Saturating>(block: N, best: N) -> bool {
+	best.saturating_sub(block) >= N::from(ANCIENT_BLOCK_THRESHOLD)
+}
+
 /// Opaque justifications subscription type.
 pub struct Subscription<T>(pub(crate) Mutex<futures::channel::mpsc::Receiver<Option<T>>>);
 

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},

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`

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