-
Notifications
You must be signed in to change notification settings - Fork 283
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
Transition finalized state until the first slot after the finalized epoch #8782
base: master
Are you sure you want to change the base?
Transition finalized state until the first slot after the finalized epoch #8782
Conversation
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
…pty blocks Signed-off-by: Gabriel Fukushima <[email protected]>
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.
I think it's a hacky way to go. The that I'd prefer is to have transitioned anchor in Store and just return it here as is. Because If we start with such anchor, it's what we returns if someone queries our node via beacon API
true))); | ||
finalized -> { | ||
final UInt64 slot = finalized.getSlot(); | ||
final Spec spec = finalized.getSpec(); |
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.
we have spec already at StateSelectorFactory
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.
how did I miss that?
final BeaconState finalizedState; | ||
//if this isn't the first slot after the finalized epoch than | ||
// we'll compute empty slots until we get there | ||
if (!slot.dividedBy(spec.getSlotsPerEpoch(slot)).isZero()) { |
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.
I'd avoid checks like this especially we consider changes in slots time or maybe slots per epoch in the future. Use spec helpers like computeStartSlotAtEpoch
and compare actual and expected slot
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.
yeah, that is much better, thanks for the heads up.
PR Description
This PR aims to return the finalized state applying processing empty slot to make the transtion of the epoch fixing the case when we return a different state when the first slot of the new epoch is an empty block.
Missing some unit tests but tests with kurtosis have shown good results.
Fixed Issue(s)
Fixes #7955
Documentation
doc-change-required
label to this PR if updates are required.Changelog