Skip to content

Conversation

@ethDreamer
Copy link
Member

Finished implementing the gossip block validation conditions.

Copy link
Member

@paulhauner paulhauner left a 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

@michaelsproul michaelsproul added the merge-f2f Relates to the Oct 2021 Merge F2F label Sep 28, 2021
/// 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> {
Copy link
Member

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

Copy link
Member Author

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?

Copy link
Member

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 🏅

@ethDreamer ethDreamer merged commit 0a0deb7 into sigp:merge-f2f Sep 28, 2021
paulhauner pushed a commit that referenced this pull request Oct 1, 2021
* Gossip Block Validation is Much More Efficient

Co-authored-by: realbigsean <[email protected]>
paulhauner pushed a commit that referenced this pull request Oct 12, 2021
* Gossip Block Validation is Much More Efficient

Co-authored-by: realbigsean <[email protected]>
paulhauner pushed a commit that referenced this pull request Oct 27, 2021
* Gossip Block Validation is Much More Efficient

Co-authored-by: realbigsean <[email protected]>
paulhauner pushed a commit to paulhauner/lighthouse that referenced this pull request Nov 3, 2021
* Gossip Block Validation is Much More Efficient

Co-authored-by: realbigsean <[email protected]>
paulhauner pushed a commit that referenced this pull request Nov 11, 2021
* Gossip Block Validation is Much More Efficient

Co-authored-by: realbigsean <[email protected]>
paulhauner pushed a commit that referenced this pull request Nov 28, 2021
* Gossip Block Validation is Much More Efficient

Co-authored-by: realbigsean <[email protected]>
paulhauner pushed a commit that referenced this pull request Nov 28, 2021
* Gossip Block Validation is Much More Efficient

Co-authored-by: realbigsean <[email protected]>
paulhauner pushed a commit that referenced this pull request Dec 2, 2021
* Gossip Block Validation is Much More Efficient

Co-authored-by: realbigsean <[email protected]>
@ethDreamer ethDreamer deleted the block-validation branch December 19, 2022 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-f2f Relates to the Oct 2021 Merge F2F

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants