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.

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

Engine API: Cancun specification #420

Merged
merged 13 commits into from
Jun 26, 2023
Merged

Engine API: Cancun specification #420

merged 13 commits into from
Jun 26, 2023

Conversation

mkalinin
Copy link
Collaborator

@mkalinin mkalinin commented Jun 12, 2023

Introduces Cancun specification with the scope of changes defined in blob-extension.md.

TODO:

  • Add engine_exchangeTransitionConfigurationV1 deprecation notice
  • Add new methods to schema
  • Add PayloadAttributesV3 and engine_forkchoiceUpdatedV3 to provide payload build process with parentBeaconBlockRoot

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

- `withdrawals`: `Array of WithdrawalV1` - Array of withdrawals, each object is an `OBJECT` containing the fields of a `WithdrawalV1` structure.
- `dataGasUsed`: `QUANTITY`, 64 bits
- `excessDataGas`: `QUANTITY`, 64 Bits
- `parentBeaconBlockRoot`: `DATA`, 32 Bytes
Copy link
Member

@ralexstokes ralexstokes Jun 21, 2023

Choose a reason for hiding this comment

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

I'm having trouble checking out this PR locally, so I can follow up with the following changes:

  • updating to PayloadAttributesV3 with the parent_beacon_block_root

Copy link
Member

@ralexstokes ralexstokes Jun 21, 2023

Choose a reason for hiding this comment

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

having to make changes in another PR #423

Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

Couple nits, looks good generally though!

src/engine/cancun.md Outdated Show resolved Hide resolved
src/engine/cancun.md Outdated Show resolved Hide resolved
src/engine/cancun.md Outdated Show resolved Hide resolved
src/engine/cancun.md Outdated Show resolved Hide resolved
* method: `engine_newPayloadV3`
* params:
1. [`ExecutionPayloadV3`](#ExecutionPayloadV3).
2. `Array of DATA`, 32 Bytes - Array of blob versioned hashes to validate.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
2. `Array of DATA`, 32 Bytes - Array of blob versioned hashes to validate.
2. `Array of DATA`, 32 Bytes - Array of blob versioned hashes to validate.
3. `parentBeaconBlockRoot`: DATA`, 32 Bytes - Parent beacon block root to validate according to EIP-4788.

src/engine/cancun.md Outdated Show resolved Hide resolved
src/engine/cancun.md Outdated Show resolved Hide resolved
src/engine/cancun.md Outdated Show resolved Hide resolved
Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

generally looks good!

left some suggestions we should apply to most directly follow the latest CL spec: ethereum/consensus-specs#3421

where root is passed as a "sidecar" along with the ExecutionPayload rather than in the payload itself

@mkalinin mkalinin merged commit ee3df5b into ethereum:main Jun 26, 2023
tersec added a commit to status-im/nim-web3 that referenced this pull request Jun 29, 2023
tersec added a commit to status-im/nim-web3 that referenced this pull request Jun 29, 2023
tersec added a commit to status-im/nimbus-eth2 that referenced this pull request Jun 29, 2023
tersec added a commit to status-im/nimbus-eth2 that referenced this pull request Jun 29, 2023
tersec added a commit to status-im/nimbus-eth2 that referenced this pull request Jun 30, 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.

5 participants