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

Misc fork choice improvements #4744

Merged
merged 4 commits into from Feb 4, 2020
Merged

Misc fork choice improvements #4744

merged 4 commits into from Feb 4, 2020

Conversation

terencechain
Copy link
Member

Added a few more checks after rereading the spec. This should improve new fork choice health

  • Update justified check point with best justified check point before calling head
  • New finalized check point could imply new justification. Added a check for that

@terencechain terencechain self-assigned this Feb 4, 2020
@codecov
Copy link

codecov bot commented Feb 4, 2020

Codecov Report

Merging #4744 into master will increase coverage by 30.67%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #4744       +/-   ##
===========================================
+ Coverage   12.26%   42.94%   +30.67%     
===========================================
  Files          77      204      +127     
  Lines        6260    15341     +9081     
===========================================
+ Hits          768     6588     +5820     
- Misses       5360     7638     +2278     
- Partials      132     1115      +983     

// or get_ancestor(store, store.justified_checkpoint.root, finalized_slot) != store.finalized_checkpoint.root
// ):
// store.justified_checkpoint = state.current_justified_checkpoint
func (s *Service) finalizedNewJustified(ctx context.Context, state *stateTrie.BeaconState) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we rename to something more informative? finalizedImpliesNewJustifiedCheckpoint or something like that

@prylabs-bulldozer prylabs-bulldozer bot merged commit 9c1a294 into master Feb 4, 2020
@delete-merged-branch delete-merged-branch bot deleted the update-justified branch February 4, 2020 17:35
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 24, 2020
* Added missing spec implementation
* Use it
* Rename
* Merge branch 'master' into update-justified
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 28, 2020
* Added missing spec implementation
* Use it
* Rename
* Merge branch 'master' into update-justified
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.

None yet

2 participants