Skip to content

Fix builder API electra json response #7285

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
Merged
merged 3 commits into from
Apr 9, 2025
Merged

Fix builder API electra json response #7285

merged 3 commits into from
Apr 9, 2025

Conversation

eserilev
Copy link
Member

@eserilev eserilev commented Apr 8, 2025

Issue Addressed

#7277

Implement ForkVersionDeserialize for ExecutionPayloadAndBlobs so we get fork hinting when deserializing

@eserilev eserilev added ready-for-review The code is ready for review builder API electra Required for the Electra/Prague fork v7.0.0 New release c. Q1 2025 labels Apr 8, 2025
pawanjay176
pawanjay176 approved these changes Apr 8, 2025
View reviewed changes
Copy link
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Tested this with kurtosis and its deserializing the electra variant as expected

@eserilev
Copy link
Member Author

eserilev commented Apr 8, 2025

I added some roundtrip tests

@eserilev
Copy link
Member Author

eserilev commented Apr 8, 2025

I talked to Pawan and we plan to add more generic roundtrip tests that cover all types that implement deserialize_by_fork in a separate issue

@michaelsproul
Copy link
Member

Awesome, the tests are a great idea. I'm just pushing a small cleanup now

@michaelsproul
Copy link
Member

Another idea for cleaning up fork version deserialize:

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Apr 9, 2025
@mergify mergify bot merged commit d24a4ff into sigp:release-v7.0.0 Apr 9, 2025
31 checks passed
@michaelsproul michaelsproul mentioned this pull request Apr 9, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pawanjay176 pawanjay176 pawanjay176 approved these changes

Assignees
No one assigned
Labels
builder API electra Required for the Electra/Prague fork ready-for-merge This PR is ready to merge. v7.0.0 New release c. Q1 2025
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eserilev @michaelsproul @pawanjay176