Skip to content

Conversation

ralexstokes
Copy link
Member

Following discussion from here ethereum/pm#702, protocol devs want to explore harmonization in the withdrawals format from CL to EL.

This PR updates the Engine API with the first part of this change to use the same units (Gwei) for the amount in each withdrawal.

@ralexstokes
Copy link
Member Author

See EIP update here: ethereum/EIPs#6325

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.

looks good! not sure why line 146 is in the diff though

Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm

@tersec
Copy link
Contributor

tersec commented Jan 13, 2023

lgtm

Copy link
Contributor

@etan-status etan-status left a comment

Choose a reason for hiding this comment

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

Thanks for this. This will allow eventually migrating the EL's withdrawals_root and txs_root to SSZ, eliminating the inconsistency with CL ExecutionPayloadHeader values.

etan-status added a commit to etan-status/consensus-specs that referenced this pull request Jan 15, 2023
Updates the EL block hash computation for pytests to match latest spec.
Notably, use `Gwei` for `Withdrawal` amount consistently.
Also fix `excess_data_gas` not being correctly accounted for.

ethereum/execution-apis#354
ethereum/EIPs#6325
Copy link
Contributor

@potuz potuz left a comment

Choose a reason for hiding this comment

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

Can we get this merged today so that we can merge in production and use production code in the devnet?

@lightclient lightclient merged commit 5561353 into ethereum:main Jan 17, 2023
@ralexstokes ralexstokes deleted the withdrawals-gwei-arc branch January 17, 2023 05:00
yperbasis added a commit to erigontech/erigon that referenced this pull request Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.