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

spec 1.1.6 - Applying proposer boost to fork-choice #3540

Merged
merged 14 commits into from
Jan 22, 2022

Conversation

g11tech
Copy link
Contributor

@g11tech g11tech commented Dec 19, 2021

Motivation
In order to prevent some attack scenarios, timely receipt of the block needs to be rewarded with proper boost as per CL spec 1.1.6 (the calculation of which was fixed in spec 1.1.7)

This PR implements following CL spec 1.1.6 changes along with spec 1.1.7 calc fixes

  • proposer boost
  • enables the proposer boost checks in fork-choice spec tests version 1.1.8
  • putting the proposer boost behind the flag --chain.proposerBoostEnabled

Closes #3506, #3438,
Partially closes #3483

@codeclimate
Copy link

codeclimate bot commented Dec 19, 2021

Code Climate has analyzed commit 067debc and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@codecov
Copy link

codecov bot commented Dec 19, 2021

Codecov Report

Merging #3540 (553aeaa) into master (1bd730a) will decrease coverage by 0.23%.
The diff coverage is n/a.

❗ Current head 553aeaa differs from pull request most recent head 067debc. Consider uploading reports for the commit 067debc to get more accurate results

@@            Coverage Diff             @@
##           master    #3540      +/-   ##
==========================================
- Coverage   37.40%   37.16%   -0.24%     
==========================================
  Files         311      321      +10     
  Lines        8374     8696     +322     
  Branches     1299     1348      +49     
==========================================
+ Hits         3132     3232     +100     
- Misses       5093     5319     +226     
+ Partials      149      145       -4     

@g11tech g11tech marked this pull request as ready for review January 16, 2022 09:33
@g11tech
Copy link
Contributor Author

g11tech commented Jan 17, 2022

since lighthouse's proposer score computation is still pretty cheap, reverted to that approach in order to keep the forkChoice closer to lighthouse's implementation.

✓ computeProposerBoostScoreFromBalances                               6907.508 ops/s    144.7700 us/op        -     206061 runs   30.0 s

if (proposerIndex == undefined) throw Error("InvalidProposerIndex");
const proposerBoostScore =
this.proposerBoost.score ??
computeProposerBoostScoreFromBalances(this.justifiedBalances, {
Copy link
Contributor

Choose a reason for hiding this comment

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

this function should be called once per justified balances at most, we can even precompute it in getEffectiveBalances() to avoid an extra for loop over justified balances, and send both proposerBoostScore and justifiedBalances to forkchoice over onBlock()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, good idea 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually it could require a bit more bookeeping as there is one place where justified is updated with the bestJustifiedBalances, so will also have to track bestProposerBoostScore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tuyennhv now score is calculated atmost once per justified balances

twoeths
twoeths previously approved these changes Jan 18, 2022
Copy link
Contributor

@dapplion dapplion left a comment

Choose a reason for hiding this comment

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

Excellent! Looks great 🔥 🔥 I've committed on top to make the feature flag visible

@g11tech g11tech merged commit f9cb593 into ChainSafe:master Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CL spec 1.1.6: proposer boost Merge spec 1.1.6
3 participants