Skip to content

feat(consensus): start with safe block hash#1932

Merged
rkrasiuk merged 1 commit intorkrasiuk/sync-controllerfrom
rkrasiuk/start-with-safe-hash
Mar 23, 2023
Merged

feat(consensus): start with safe block hash#1932
rkrasiuk merged 1 commit intorkrasiuk/sync-controllerfrom
rkrasiuk/start-with-safe-hash

Conversation

@rkrasiuk
Copy link
Copy Markdown
Contributor

Introducing change to #1845 as a PR, because it's mostly finalized.

Prefer safe_block_hash as tip to start the sync from. Minimizes the chance of starting from reorged block and should increase the chance of finding good peers (especially on testnets).

@rkrasiuk rkrasiuk added the C-enhancement New feature or request label Mar 23, 2023
@rkrasiuk rkrasiuk requested review from mattsse and rakita March 23, 2023 13:35
@rkrasiuk rkrasiuk requested a review from gakonst as a code owner March 23, 2023 13:35
@rkrasiuk rkrasiuk marked this pull request as draft March 23, 2023 13:36
Comment on lines 142 to 143
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sgtm!

@rkrasiuk rkrasiuk force-pushed the rkrasiuk/start-with-safe-hash branch from 9b65c94 to f39da52 Compare March 23, 2023 13:52
@rkrasiuk rkrasiuk requested a review from mattsse March 23, 2023 13:52
@rkrasiuk rkrasiuk marked this pull request as ready for review March 23, 2023 13:55
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 23, 2023

Codecov Report

Merging #1932 (f39da52) into rkrasiuk/sync-controller (08e5ed8) will decrease coverage by 0.08%.
The diff coverage is 94.73%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@                     Coverage Diff                      @@
##           rkrasiuk/sync-controller    #1932      +/-   ##
============================================================
- Coverage                     73.34%   73.26%   -0.08%     
============================================================
  Files                           424      424              
  Lines                         51750    51755       +5     
============================================================
- Hits                          37954    37920      -34     
- Misses                        13796    13835      +39     
Flag Coverage Δ
integration-tests 19.46% <0.00%> (+<0.01%) ⬆️
unit-tests 67.59% <94.73%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
crates/consensus/beacon/src/engine/mod.rs 97.14% <94.73%> (-0.13%) ⬇️

... and 7 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@rkrasiuk rkrasiuk merged commit 043a0c4 into rkrasiuk/sync-controller Mar 23, 2023
@rkrasiuk rkrasiuk deleted the rkrasiuk/start-with-safe-hash branch March 23, 2023 14:51
@rkrasiuk rkrasiuk changed the title consensus engine: start with safe block hash feat(consensus): start with safe block hash Mar 24, 2023
@rkrasiuk rkrasiuk added the A-consensus Related to the consensus engine label Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-consensus Related to the consensus engine C-enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants