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

feat: add OptimismExecutionPayloadV3 #242

Merged
merged 1 commit into from
Mar 2, 2024

Conversation

fgimenez
Copy link
Contributor

@fgimenez fgimenez commented Mar 1, 2024

Motivation

Towards paradigmxyz/reth#6871

Solution

Adds specific type for OptimismExecutionPayloadV3 to be used in BuiltPayload. Very similar to ExecutionPayloadV3, adds optimism-specific field parent_beacon_block_root

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@prestwich
Copy link
Member

prestwich commented Mar 1, 2024

alloy project does not intend to house any non-Ethereum structs. this whole file belongs in a separate repo

@mattsse
Copy link
Member

mattsse commented Mar 1, 2024

this should be in there for ease of use, at least for the time being because we already have some other op types, but once stable we should remove them yes

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

doc nit, otherwise lgtm

/// This structure maps on the ExecutionPayloadV3 structure of the beacon chain spec.
///
/// See also: <https://github.com/ethereum/execution-apis/blob/6709c2a795b707202e93c4f2867fa0bf2640a84f/src/engine/shanghai.md#executionpayloadv2>
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should also reference the op spec:

sure, proposed here #244

@mattsse mattsse marked this pull request as ready for review March 1, 2024 22:48
Copy link
Contributor

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

lgtm codewise. as prestwich said, this should only be temporary—I think we should make an issue to track moving OP types to another repo once we have stability

@prestwich
Copy link
Member

@DaniPopes is the clippy relevant? otherwise good to merge

@Evalir
Copy link
Contributor

Evalir commented Mar 2, 2024

@prestwich nope, clippy is unrelated—see proptest fix: proptest-rs/proptest#427

@Evalir Evalir merged commit 146d6a8 into alloy-rs:main Mar 2, 2024
13 of 14 checks passed
@Evalir
Copy link
Contributor

Evalir commented Mar 2, 2024

Tracking wrt OP types #243

@fgimenez fgimenez deleted the fgimenez/add-optimism-payload-v3 branch March 2, 2024 09:53
mattsse pushed a commit to paradigmxyz/revm-inspectors that referenced this pull request Mar 3, 2024
Updates `alloy-rpc-types` and `alloy-rpc-trace-types` to
`52bf7125d944e8a389371be503035724dfde5657`, which contains
alloy-rs/alloy#242
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.

4 participants