Add another condition to the reject-obsolete-parachain-heads extension (#1505)

* add another condition to the reject-obsolete-parachain-heads extension

* add tracing to obsolete-tx-extensions

* fix tests

* extension_rejects_header_from_new_relay_block_with_same_hash

* fmt

* fix benchmarks

bridges/relays/lib-substrate-relay/src/parachains/mod.rs

@@ -18,7 +18,7 @@
 //! parachain finality proofs synchronization pipelines.
 
 use async_trait::async_trait;
-use bp_polkadot_core::parachains::{ParaHeadsProof, ParaId};
+use bp_polkadot_core::parachains::{ParaHash, ParaHeadsProof, ParaId};
 use pallet_bridge_parachains::{
 	Call as BridgeParachainsCall, Config as BridgeParachainsConfig, RelayBlockHash,
 	RelayBlockHasher, RelayBlockNumber,
@@ -71,7 +71,7 @@ pub trait SubmitParachainHeadsCallBuilder<P: SubstrateParachainsPipeline>:
 	/// function of bridge parachains module at the target chain.
 	fn build_submit_parachain_heads_call(
 		at_relay_block: HeaderIdOf<P::SourceRelayChain>,
-		parachains: Vec<ParaId>,
+		parachains: Vec<(ParaId, ParaHash)>,
 		parachain_heads_proof: ParaHeadsProof,
 	) -> CallOf<P::TargetChain>;
 }
@@ -97,7 +97,7 @@ where
 {
 	fn build_submit_parachain_heads_call(
 		at_relay_block: HeaderIdOf<P::SourceRelayChain>,
-		parachains: Vec<ParaId>,
+		parachains: Vec<(ParaId, ParaHash)>,
 		parachain_heads_proof: ParaHeadsProof,
 	) -> CallOf<P::TargetChain> {
 		BridgeParachainsCall::<R, I>::submit_parachain_heads {

bridges/relays/lib-substrate-relay/src/parachains/source.rs

@@ -160,23 +160,46 @@ where
 		&self,
 		at_block: HeaderIdOf<P::SourceRelayChain>,
 		parachains: &[ParaId],
-	) -> Result<ParaHeadsProof, Self::Error> {
-		let storage_keys = parachains
-			.iter()
-			.map(|para_id| {
-				parachain_head_storage_key_at_source(
-					P::SourceRelayChain::PARAS_PALLET_NAME,
-					*para_id,
-				)
-			})
-			.collect();
+	) -> Result<(ParaHeadsProof, Vec<ParaHash>), Self::Error> {
+		if parachains.len() != 1 || parachains[0].0 != P::SOURCE_PARACHAIN_PARA_ID {
+			return Err(SubstrateError::Custom(format!(
+				"Trying to prove unexpected parachains {:?}. Expected {:?}",
+				parachains,
+				P::SOURCE_PARACHAIN_PARA_ID,
+			)))
+		}
+
+		let parachain = parachains[0];
+		let storage_key =
+			parachain_head_storage_key_at_source(P::SourceRelayChain::PARAS_PALLET_NAME, parachain);
 		let parachain_heads_proof = self
 			.client
-			.prove_storage(storage_keys, at_block.1)
+			.prove_storage(vec![storage_key.clone()], at_block.1)
 			.await?
 			.iter_nodes()
 			.collect();
 
-		Ok(ParaHeadsProof(parachain_heads_proof))
+		// why we're reading parachain head here once again (it has already been read at the
+		// `parachain_head`)? that's because `parachain_head` sometimes returns obsolete parachain
+		// head and loop sometimes asks to prove this obsolete head and gets other (actual) head
+		// instead
+		//
+		// => since we want to provide proper hashes in our `submit_parachain_heads` call, we're
+		// rereading actual value here
+		let parachain_head = self
+			.client
+			.raw_storage_value(storage_key, Some(at_block.1))
+			.await?
+			.map(|h| ParaHead::decode(&mut &h.0[..]))
+			.transpose()?
+			.ok_or_else(|| {
+				SubstrateError::Custom(format!(
+					"Failed to read expected parachain {:?} head at {:?}",
+					parachain, at_block
+				))
+			})?;
+		let parachain_head_hash = parachain_head.hash();
+
+		Ok((ParaHeadsProof(parachain_heads_proof), vec![parachain_head_hash]))
 	}
 }

bridges/relays/lib-substrate-relay/src/parachains/target.rs

@@ -28,7 +28,7 @@ use bp_parachains::{
 	best_parachain_head_hash_storage_key_at_target, imported_parachain_head_storage_key_at_target,
 	BestParaHeadHash,
 };
-use bp_polkadot_core::parachains::{ParaHead, ParaHeadsProof, ParaId};
+use bp_polkadot_core::parachains::{ParaHash, ParaHead, ParaHeadsProof, ParaId};
 use codec::{Decode, Encode};
 use parachains_relay::{
 	parachains_loop::TargetClient, parachains_loop_metrics::ParachainsLoopMetrics,
@@ -166,7 +166,7 @@ where
 	async fn submit_parachain_heads_proof(
 		&self,
 		at_relay_block: HeaderIdOf<P::SourceRelayChain>,
-		updated_parachains: Vec<ParaId>,
+		updated_parachains: Vec<(ParaId, ParaHash)>,
 		proof: ParaHeadsProof,
 	) -> Result<(), Self::Error> {
 		let genesis_hash = *self.client.genesis_hash();

bridges/relays/parachains/src/parachains_loop.rs

@@ -69,6 +69,16 @@ pub enum ParaHashAtSource {
 	Unavailable,
 }
 
+impl ParaHashAtSource {
+	/// Return parachain head hash, if available.
+	pub fn hash(&self) -> Option<&ParaHash> {
+		match *self {
+			ParaHashAtSource::Some(ref para_hash) => Some(para_hash),
+			_ => None,
+		}
+	}
+}
+
 /// Source client used in parachain heads synchronization loop.
 #[async_trait]
 pub trait SourceClient<P: ParachainsPipeline>: RelayClient {
@@ -87,11 +97,15 @@ pub trait SourceClient<P: ParachainsPipeline>: RelayClient {
 	) -> Result<ParaHashAtSource, Self::Error>;
 
 	/// Get parachain heads proof.
+	///
+	/// The number and order of entries in the resulting parachain head hashes vector must match the
+	/// number and order of parachains in the `parachains` vector. The incorrect implementation will
+	/// result in panic.
 	async fn prove_parachain_heads(
 		&self,
 		at_block: HeaderIdOf<P::SourceChain>,
 		parachains: &[ParaId],
-	) -> Result<ParaHeadsProof, Self::Error>;
+	) -> Result<(ParaHeadsProof, Vec<ParaHash>), Self::Error>;
 }
 
 /// Target client used in parachain heads synchronization loop.
@@ -121,7 +135,7 @@ pub trait TargetClient<P: ParachainsPipeline>: RelayClient {
 	async fn submit_parachain_heads_proof(
 		&self,
 		at_source_block: HeaderIdOf<P::SourceChain>,
-		updated_parachains: Vec<ParaId>,
+		updated_parachains: Vec<(ParaId, ParaHash)>,
 		proof: ParaHeadsProof,
 	) -> Result<(), Self::Error>;
 }
@@ -274,7 +288,7 @@ where
 		);
 
 		if is_update_required {
-			let heads_proofs = source_client
+			let (heads_proofs, head_hashes) = source_client
 				.prove_parachain_heads(best_finalized_relay_block, &updated_ids)
 				.await
 				.map_err(|e| {
@@ -292,10 +306,17 @@ where
 				P::SourceChain::NAME,
 				P::TargetChain::NAME,
 			);
+
+			assert_eq!(
+				head_hashes.len(),
+				updated_ids.len(),
+				"Incorrect parachains SourceClient implementation"
+			);
+
 			target_client
 				.submit_parachain_heads_proof(
 					best_finalized_relay_block,
-					updated_ids.clone(),
+					updated_ids.iter().cloned().zip(head_hashes).collect(),
 					heads_proofs,
 				)
 				.await
@@ -394,7 +415,7 @@ where
 
 			needs_update
 		})
-		.map(|((para_id, _), _)| para_id)
+		.map(|((para, _), _)| para)
 		.collect()
 }
 
@@ -666,7 +687,7 @@ mod tests {
 			&self,
 			_at_block: HeaderIdOf<TestChain>,
 			parachains: &[ParaId],
-		) -> Result<ParaHeadsProof, TestError> {
+		) -> Result<(ParaHeadsProof, Vec<ParaHash>), TestError> {
 			let mut proofs = Vec::new();
 			for para_id in parachains {
 				proofs.push(
@@ -680,7 +701,7 @@ mod tests {
 						.ok_or(TestError::MissingParachainHeadProof)?,
 				);
 			}
-			Ok(ParaHeadsProof(proofs))
+			Ok((ParaHeadsProof(proofs), vec![Default::default(); parachains.len()]))
 		}
 	}
 
@@ -709,7 +730,7 @@ mod tests {
 		async fn submit_parachain_heads_proof(
 			&self,
 			_at_source_block: HeaderIdOf<TestChain>,
-			_updated_parachains: Vec<ParaId>,
+			_updated_parachains: Vec<(ParaId, ParaHash)>,
 			_proof: ParaHeadsProof,
 		) -> Result<(), Self::Error> {
 			self.data.lock().await.target_submit_result.clone()?;