Fix relay submitting extra parachain headers during reorg (paritytech…

…#2839) * fix on-demand parachain relay behavior during target chain reorgs * fix compilation
serban300 · Feb 16, 2024 · 8e7b471 · 8e7b471
1 parent 8a0ed23
commit 8e7b471
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 8 deletions.
diff --git a/relays/lib-substrate-relay/src/parachains/source.rs b/relays/lib-substrate-relay/src/parachains/source.rs
@@ -120,10 +120,15 @@ where
 					// `max_header_id` is not set. There is no limit.
 					AvailableHeader::Available(on_chain_para_head_id)
 				},
-				AvailableHeader::Available(max_head_id) => {
+				AvailableHeader::Available(max_head_id) if on_chain_para_head_id >= max_head_id => {
 					// We report at most `max_header_id`.
 					AvailableHeader::Available(std::cmp::min(on_chain_para_head_id, max_head_id))
 				},
+				AvailableHeader::Available(_) => {
+					// the `max_head_id` is not yet available at the source chain => wait and avoid
+					// syncing extra headers
+					AvailableHeader::Unavailable
+				},
 			}
 		}
 

diff --git a/relays/parachains/src/parachains_loop.rs b/relays/parachains/src/parachains_loop.rs
@@ -256,7 +256,12 @@ where
 		let head_at_source =
 			read_head_at_source(&source_client, metrics.as_ref(), &best_finalized_relay_block)
 				.await?;
-		let is_update_required = is_update_required::<P>(head_at_source, head_at_target);
+		let is_update_required = is_update_required::<P>(
+			head_at_source,
+			head_at_target,
+			best_finalized_relay_block,
+			best_target_block,
+		);
 
 		if is_update_required {
 			let (head_proof, head_hash) = source_client
@@ -274,10 +279,12 @@ where
 				})?;
 			log::info!(
 				target: "bridge",
-				"Submitting {} parachain ParaId({}) head update transaction to {}",
+				"Submitting {} parachain ParaId({}) head update transaction to {}. Para hash at source relay {:?}: {:?}",
 				P::SourceRelayChain::NAME,
 				P::SourceParachain::PARACHAIN_ID,
 				P::TargetChain::NAME,
+				best_finalized_relay_block,
+				head_hash,
 			);
 
 			let transaction_tracker = target_client
@@ -304,21 +311,25 @@ where
 fn is_update_required<P: ParachainsPipeline>(
 	head_at_source: AvailableHeader<HeaderIdOf<P::SourceParachain>>,
 	head_at_target: Option<HeaderIdOf<P::SourceParachain>>,
+	best_finalized_relay_block_at_source: HeaderIdOf<P::SourceRelayChain>,
+	best_target_block: HeaderIdOf<P::TargetChain>,
 ) -> bool
 where
 	P::SourceRelayChain: Chain<BlockNumber = RelayBlockNumber>,
 {
 	log::trace!(
 		target: "bridge",
 		"Checking if {} parachain ParaId({}) needs update at {}:\n\t\
-			At {}: {:?}\n\t\
-			At {}: {:?}",
+			At {} ({:?}): {:?}\n\t\
+			At {} ({:?}): {:?}",
 		P::SourceRelayChain::NAME,
 		P::SourceParachain::PARACHAIN_ID,
 		P::TargetChain::NAME,
 		P::SourceRelayChain::NAME,
+		best_finalized_relay_block_at_source,
 		head_at_source,
 		P::TargetChain::NAME,
+		best_target_block,
 		head_at_target,
 	);
 
@@ -908,23 +919,37 @@ mod tests {
 
 	#[test]
 	fn parachain_is_not_updated_if_it_is_unavailable() {
-		assert!(!is_update_required::<TestParachainsPipeline>(AvailableHeader::Unavailable, None));
 		assert!(!is_update_required::<TestParachainsPipeline>(
 			AvailableHeader::Unavailable,
-			Some(HeaderId(10, PARA_10_HASH))
+			None,
+			Default::default(),
+			Default::default(),
+		));
+		assert!(!is_update_required::<TestParachainsPipeline>(
+			AvailableHeader::Unavailable,
+			Some(HeaderId(10, PARA_10_HASH)),
+			Default::default(),
+			Default::default(),
 		));
 	}
 
 	#[test]
 	fn parachain_is_not_updated_if_it_is_unknown_to_both_clients() {
-		assert!(!is_update_required::<TestParachainsPipeline>(AvailableHeader::Missing, None),);
+		assert!(!is_update_required::<TestParachainsPipeline>(
+			AvailableHeader::Missing,
+			None,
+			Default::default(),
+			Default::default(),
+		),);
 	}
 
 	#[test]
 	fn parachain_is_not_updated_if_target_has_better_head() {
 		assert!(!is_update_required::<TestParachainsPipeline>(
 			AvailableHeader::Available(HeaderId(10, Default::default())),
 			Some(HeaderId(20, Default::default())),
+			Default::default(),
+			Default::default(),
 		),);
 	}
 
@@ -933,6 +958,8 @@ mod tests {
 		assert!(is_update_required::<TestParachainsPipeline>(
 			AvailableHeader::Missing,
 			Some(HeaderId(20, Default::default())),
+			Default::default(),
+			Default::default(),
 		),);
 	}
 
@@ -941,6 +968,8 @@ mod tests {
 		assert!(is_update_required::<TestParachainsPipeline>(
 			AvailableHeader::Available(HeaderId(30, Default::default())),
 			None,
+			Default::default(),
+			Default::default(),
 		),);
 	}
 
@@ -949,6 +978,8 @@ mod tests {
 		assert!(is_update_required::<TestParachainsPipeline>(
 			AvailableHeader::Available(HeaderId(40, Default::default())),
 			Some(HeaderId(30, Default::default())),
+			Default::default(),
+			Default::default(),
 		),);
 	}
 }