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.

Sign up for GitHub

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

Jump to bottom

Remove some easy Electra TODOs #5928

Merged
merged 1 commit into from
Jun 17, 2024
Merged

Remove some easy Electra TODOs #5928

merged 1 commit into from
Jun 17, 2024

Conversation

michaelsproul
Copy link
Member

Proposed Changes

Remove 3 TODOs related to Electra:

  • Delete legacy payload reconstruction for Electra. We don't even have proper payload reconstruction specced yet for Electra (see Electra engine api #5743), and I'm doubtful we will need or want the legacy reconstruction. I'm actually tempted to delete legacy payload reconstruction entirely, but it is a little bit useful for sanity-checking the EL genesis block and preventing mistakes similar to Holesky v1.0 (see bellatrix_readiness.rs which does genesis block checks).
  • Confirm that non-optional fields are OK in PartialBeaconState. This is going away soon with tree-states anyway.
  • Remove comment from ExecutionPayloadHeader::ssz_max_var_len_for_fork. The extra data is still the only variable-length field in ExecutionPayloadHeader, so this function is still correct. If the spec changes, the light client tests should catch any discrepancy here too.

@michaelsproul michaelsproul added ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! electra Required for the Electra/Prague fork labels Jun 14, 2024
@realbigsean
Copy link
Member

@mergify queue

@mergify Mergify
Copy link

mergify bot commented Jun 17, 2024 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 1eb8694

@mergify mergify bot merged commit 1eb8694 into unstable Jun 17, 2024
28 checks passed
@mergify mergify bot deleted the remove-electra-todos branch June 17, 2024 13:32
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dapplion dapplion dapplion approved these changes

Assignees
No one assigned
Labels
electra Required for the Electra/Prague fork low-hanging-fruit Easy to resolve, get it before someone else does! ready-for-review The code is ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@michaelsproul @realbigsean @dapplion