-
Notifications
You must be signed in to change notification settings - Fork 934
Finished Gossip Block Validation Conditions #2640
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
Conversation
paulhauner
left a comment
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.
Looks good, just one TODO to remove
| /// An implementation of the method described in the consensus spec here: | ||
| /// | ||
| /// https://github.com/ethereum/consensus-specs/blob/dev/specs/merge/beacon-chain.md#compute_timestamp_at_slot | ||
| fn compute_timestamp_at_slot(&self, slot: Slot) -> Option<u64> { |
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.
This PR adds a second implementation of compute_timestamp_at_slot in the SlotClock to avoid accessing the head during block verification just to get the genesis_time. The original implementation still exists and is used in the consensus code, because we don't pass the SlotClock around the consensus code (and to keep the consensus code more similar to the spec). Wondering if people think that seems reasonable? Or should we try to keep a single implementation everywhere
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.
@paulhauner how does this sound to you?
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.
Yep, I think this is a good idea. It's a simple enough function that I think it's worth the trade-off.
Very neat and correct impl too 🏅
a11c92c to
e559bd9
Compare
8283199 to
e4445e1
Compare
* Gossip Block Validation is Much More Efficient Co-authored-by: realbigsean <[email protected]>
* Gossip Block Validation is Much More Efficient Co-authored-by: realbigsean <[email protected]>
* Gossip Block Validation is Much More Efficient Co-authored-by: realbigsean <[email protected]>
* Gossip Block Validation is Much More Efficient Co-authored-by: realbigsean <[email protected]>
* Gossip Block Validation is Much More Efficient Co-authored-by: realbigsean <[email protected]>
* Gossip Block Validation is Much More Efficient Co-authored-by: realbigsean <[email protected]>
* Gossip Block Validation is Much More Efficient Co-authored-by: realbigsean <[email protected]>
* Gossip Block Validation is Much More Efficient Co-authored-by: realbigsean <[email protected]>
Finished implementing the gossip block validation conditions.