Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Electra: upgrade #13933
Electra: upgrade #13933
Changes from 28 commits
3b58756
e02475a
7d0c2bc
2f7f899
c594df8
45ee712
0a2dae0
5287f56
b5560be
1f7bb36
49cacc4
38bca88
de12137
4dd8861
eae67f7
dd36ec6
6080275
8ea9034
abbdd1a
0b34cf9
5cfe8d2
a4607c7
3cf41ee
53552ad
4cee2cb
78af849
0897bea
266c7ec
969ae73
174c6f6
ecc6e6a
7f69c2d
62c0cee
2b57557
a523f30
37dbcbd
7ddae4a
1c9a6ae
cf89f83
44993f6
429b474
a915fbe
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You dont need a list here. You can save on memory. Just track a single variable of
primitives.Epoch
, first set it totime.CurrentEpoch(beaconState)
, then override it if there's a higher one. Finally, increment it by oneThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: I think this comment should come before
exitEpochs
declarationThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment feels out of place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the spec these are
[]
which I understand as empty slices. Having this nil might cause a panic the first time we access the fieldThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to sort here? I know spec says it but why? We should avoid wasteful compute if we can
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validators might have different ActivationEligibilityEpoch and should be sorted based on that value, there is also a tie breaker situation. i don't have a good solution to replace this right now.
Check failure on line 319 in beacon-chain/core/electra/upgrade.go
GitHub Actions / Lint