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

Add GetPayloadResponse dataclass for get_payload API #3341

Merged
merged 1 commit into from
May 10, 2023
Merged

Add GetPayloadResponse dataclass for get_payload API #3341

merged 1 commit into from
May 10, 2023

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Apr 28, 2023

The stub get_blobs_and_kzg_commitments API was cleaned up by Engine API changes (ethereum/execution-apis#402). Now engine_getPayloadV3 returns multiple fields include blobsBundle.

For CL specs, changing the function signature means we have to deal with backward compatibility. IMO it is easier to define a GetPayloadResponse dataclass for different versions.

This PR adds GetPayloadResponse and BlobsBundle dataclasses that reflect the Engine API changes.

@hwwhww hwwhww requested a review from mkalinin April 28, 2023 15:29
djrtwo
djrtwo approved these changes May 3, 2023
View reviewed changes
Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

This path makes sense to me (rather than modifying the function return signature)

@StefanBratanov StefanBratanov mentioned this pull request May 4, 2023
Merged
2 tasks
@hwwhww hwwhww merged commit f7352d1 into dev May 10, 2023
@hwwhww hwwhww deleted the add-GetPayloadResponse branch May 10, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djrtwo djrtwo djrtwo approved these changes

@mkalinin mkalinin Awaiting requested review from mkalinin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hwwhww @djrtwo