Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass warp sync target block to parachain node's SyncingEngine during initialization #3537

Closed
dmitry-markin opened this issue Mar 1, 2024 · 1 comment · Fixed by #5431
Closed
Labels
I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue.

Comments

@dmitry-markin
Copy link
Contributor

Currently, warp sync target block header is passed to parachain node syncing via oneshot channel, which is later polled in SyncingEngine:

warp_target_block_header = &mut self.warp_sync_target_block_header_rx_fused => {
if let Err(_) = self.pass_warp_sync_target_block_header(warp_target_block_header) {
error!(
target: LOG_TARGET,
"Failed to set warp sync target block header, terminating `SyncingEngine`.",
);
return
}
},

and passed all the way down to WarpSync:
pub fn set_target_block(&mut self, header: B::Header) {
let Phase::PendingTargetBlock = self.phase else {
error!(
target: LOG_TARGET,
"Attempt to set warp sync target block in invalid phase.",
);
debug_assert!(false);
return
};
self.phase = Phase::TargetBlock(header);
}

This is cumbersome, requires checks for the correct phase of syncing, leads to bugs like #3496, and should be reworked to pass the target block header as a parameter during the initialization of SyncingEngine.

@dmitry-markin dmitry-markin added the I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue. label Mar 1, 2024
dmitry-markin added a commit that referenced this issue Mar 1, 2024
@nazar-pc
Copy link
Contributor

nazar-pc commented Aug 21, 2024

This PR should resolve it I think: #5431

github-merge-queue bot pushed a commit that referenced this issue Aug 23, 2024
…tation (#5431)

I'm not sure if this is exactly what
#3537 meant, but I
think it should be fine to wait for relay chain before initializing
parachain node fully, which removed the need for background task and
extra hacks throughout the stack just to know where warp sync should
start.

Previously there were both `WarpSyncParams` and `WarpSyncConfig`, but
there was no longer any point in having two data structures, so I
simplified it to just `WarpSyncConfig`.

Fixes #3537
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue.
Projects
Status: Blocked ⛔️
Development

Successfully merging a pull request may close this issue.

2 participants