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 engine_getPayloadV4 and engine_newPayloadV4 for Electra #8115

Merged
merged 6 commits into from
Mar 20, 2024
Merged

Add engine_getPayloadV4 and engine_newPayloadV4 for Electra #8115

merged 6 commits into from
Mar 20, 2024

Conversation

StefanBratanov
Copy link
Contributor

@StefanBratanov StefanBratanov commented Mar 20, 2024 •
edited
Loading

PR Description

As per ethereum/execution-apis#528 and ethereum/execution-apis#531

Fixed Issue(s)

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@StefanBratanov StefanBratanov changed the title Added engine_getPayloadV4 and engine_newPayloadV4 for Electra Add engine_getPayloadV4 and engine_newPayloadV4 for Electra Mar 20, 2024
@lucassaldanha lucassaldanha self-requested a review March 20, 2024 20:35
lucassaldanha
lucassaldanha reviewed Mar 20, 2024
View reviewed changes
Copy link
Member

@lucassaldanha lucassaldanha left a comment

Choose a reason for hiding this comment

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

PR is looking good. However I believe one of the fields has the wrong name (using snake case instead of came case).

...client/src/main/java/tech/pegasys/teku/ethereum/executionclient/schema/DepositReceiptV1.java Outdated

public DepositReceiptV1(
@JsonProperty("pubkey") final Bytes48 pubkey,
@JsonProperty("withdrawal_credentials") final Bytes32 withdrawalCredentials,
Copy link
Member

Choose a reason for hiding this comment

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

I believe this is supposed to be withdrawalCredentials

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. Good catch!

lucassaldanha
lucassaldanha approved these changes Mar 20, 2024
View reviewed changes
Copy link
Member

@lucassaldanha lucassaldanha left a comment

Choose a reason for hiding this comment

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

LGTM :)

@StefanBratanov StefanBratanov enabled auto-merge (squash) March 20, 2024 21:35
@StefanBratanov StefanBratanov merged commit c93c873 into Consensys:master Mar 20, 2024
15 of 16 checks passed
@StefanBratanov StefanBratanov deleted the 8084 branch March 21, 2024 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucassaldanha lucassaldanha lucassaldanha approved these changes

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

Successfully merging this pull request may close these issues.

Add/Update EngineGetPayloadV4 to support exits
2 participants
@StefanBratanov @lucassaldanha