From b057b2c7b23d67725bf315be3316a476d0668b45 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 16 Feb 2024 11:07:03 +0300 Subject: [PATCH] Fix relay submitting extra parachain headers during reorg (#2839) * fix on-demand parachain relay behavior during target chain reorgs * fix compilation --- .../src/parachains/source.rs | 7 ++- .../relays/parachains/src/parachains_loop.rs | 45 ++++++++++++++++--- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/bridges/relays/lib-substrate-relay/src/parachains/source.rs b/bridges/relays/lib-substrate-relay/src/parachains/source.rs index ba59171c8e01a..32d70cf425f0b 100644 --- a/bridges/relays/lib-substrate-relay/src/parachains/source.rs +++ b/bridges/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/bridges/relays/parachains/src/parachains_loop.rs b/bridges/relays/parachains/src/parachains_loop.rs index 390b02b859459..41ebbf5aadede 100644 --- a/bridges/relays/parachains/src/parachains_loop.rs +++ b/bridges/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::

(head_at_source, head_at_target); + let is_update_required = is_update_required::

( + 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,6 +311,8 @@ where fn is_update_required( head_at_source: AvailableHeader>, head_at_target: Option>, + best_finalized_relay_block_at_source: HeaderIdOf, + best_target_block: HeaderIdOf, ) -> bool where P::SourceRelayChain: Chain, @@ -311,14 +320,16 @@ where 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,16 +919,28 @@ mod tests { #[test] fn parachain_is_not_updated_if_it_is_unavailable() { - assert!(!is_update_required::(AvailableHeader::Unavailable, None)); assert!(!is_update_required::( AvailableHeader::Unavailable, - Some(HeaderId(10, PARA_10_HASH)) + None, + Default::default(), + Default::default(), + )); + assert!(!is_update_required::( + 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::(AvailableHeader::Missing, None),); + assert!(!is_update_required::( + AvailableHeader::Missing, + None, + Default::default(), + Default::default(), + ),); } #[test] @@ -925,6 +948,8 @@ mod tests { assert!(!is_update_required::( 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::( AvailableHeader::Missing, Some(HeaderId(20, Default::default())), + Default::default(), + Default::default(), ),); } @@ -941,6 +968,8 @@ mod tests { assert!(is_update_required::( AvailableHeader::Available(HeaderId(30, Default::default())), None, + Default::default(), + Default::default(), ),); } @@ -949,6 +978,8 @@ mod tests { assert!(is_update_required::( AvailableHeader::Available(HeaderId(40, Default::default())), Some(HeaderId(30, Default::default())), + Default::default(), + Default::default(), ),); } }