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

refactor: integrate BuiltPayload::Primitives #13484

Merged
merged 3 commits into from
Dec 23, 2024
Merged

Conversation

klkvr
Copy link
Member

@klkvr klkvr commented Dec 20, 2024

This PR integrates NodePrimitives to BuiltPayload. It doesn't make a lot of sense as a generic because every built payload is required to hold ExecutedBlock which is generic over primitives, thus the type will always know the primitives type.

Additionally I've extracted block -> ExecutionPayload conversion to EngineTypes. For now both implementations just call the same fn as we were using for this earlier

@klkvr klkvr added A-consensus Related to the consensus engine A-sdk Related to reth's use as a library labels Dec 20, 2024
Copy link
Collaborator

@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.

lgtm

@@ -24,7 +24,6 @@ reth-payload-builder-primitives.workspace = true
reth-payload-primitives.workspace = true
reth-provider.workspace = true
reth-prune.workspace = true
reth-rpc-types-compat.workspace = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice

Comment on lines +84 to +89
/// Converts a [`BuiltPayload`] into an [`ExecutionPayload`] and [`ExecutionPayloadSidecar`].
fn block_to_payload(
block: SealedBlockFor<
<<Self::BuiltPayload as BuiltPayload>::Primitives as NodePrimitives>::Block,
>,
) -> (ExecutionPayload, ExecutionPayloadSidecar);
Copy link
Collaborator

Choose a reason for hiding this comment

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

this makes sense, we can likely do something about these casts

@klkvr klkvr enabled auto-merge December 23, 2024 22:43
@klkvr klkvr added this pull request to the merge queue Dec 23, 2024
Merged via the queue into main with commit af1c9b7 Dec 23, 2024
43 checks passed
@klkvr klkvr deleted the klkvr/primitives-built-payload branch December 23, 2024 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Related to the consensus engine A-sdk Related to reth's use as a library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants