Skip to content

Commit

Permalink
Fix relay submitting extra parachain headers during reorg (paritytech…
Browse files Browse the repository at this point in the history
…#2839)

* fix on-demand parachain relay behavior during target chain reorgs

* fix compilation
  • Loading branch information
svyatonik authored Feb 16, 2024
1 parent 8a0ed23 commit 8e7b471
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 8 deletions.
7 changes: 6 additions & 1 deletion relays/lib-substrate-relay/src/parachains/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}
}

Expand Down
45 changes: 38 additions & 7 deletions relays/parachains/src/parachains_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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,
);

Expand Down Expand Up @@ -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(),
),);
}

Expand All @@ -933,6 +958,8 @@ mod tests {
assert!(is_update_required::<TestParachainsPipeline>(
AvailableHeader::Missing,
Some(HeaderId(20, Default::default())),
Default::default(),
Default::default(),
),);
}

Expand All @@ -941,6 +968,8 @@ mod tests {
assert!(is_update_required::<TestParachainsPipeline>(
AvailableHeader::Available(HeaderId(30, Default::default())),
None,
Default::default(),
Default::default(),
),);
}

Expand All @@ -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(),
),);
}
}

0 comments on commit 8e7b471

Please sign in to comment.