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

Remove getByBlockNumber for CheckpointSync #5173

Merged

Conversation

siladu
Copy link
Contributor

@siladu siladu commented Mar 6, 2023

Pull up and simplify data passed to CheckpointSource, which then allows checkpointBlockHeader to be used by protocolSchedule.getByBlockHeader.

Fixes #5167

Alternative approach explored: siladu@bbaa80d
but I didn't like the way ScheduleBasedHeaderBlockHeaderFunctions requires an explicit delegation override (otherwise it implicitly gets the wrong value).

CheckpointSync testing:

  • Sepolia (prysm)
  • Sepolia (lighthouse)
  • Goerli (lighthouse)
    • got in sync but required a restarted to transition from initial sync to backwards sync
    • resync completed without intervention
  • Mainnet (teku)

Pull up and simplify data passed to CheckpointSource, which then allows checkpointBlockHeader to be used by protocolSchedule.getByBlockHeader.
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
@non-fungible-nelson non-fungible-nelson added the TeamGroot GH issues worked on by Groot Team label Mar 7, 2023
…mber-checkpointsync

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
@siladu siladu marked this pull request as ready for review March 8, 2023 03:51
Copy link
Contributor

@gfukushima gfukushima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ltgm

…mber-checkpointsync

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
@siladu siladu enabled auto-merge (squash) March 9, 2023 04:00
@siladu siladu merged commit 192c0bb into hyperledger:main Mar 9, 2023
@siladu siladu deleted the remove-getByBlockNumber-checkpointsync branch March 9, 2023 04:15
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
Pull up and simplify data passed to CheckpointSource, which then allows checkpointBlockHeader to be used by protocolSchedule.getByBlockHeader.
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
Pull up and simplify data passed to CheckpointSource, which then allows checkpointBlockHeader to be used by protocolSchedule.getByBlockHeader.
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TeamGroot GH issues worked on by Groot Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CheckpointSyncDownloadPipelineFactory getBlockHeaderFunctions usage of getByBlockNumber
4 participants