-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Conversation
There was a problem hiding this 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
/// Converts a [`BuiltPayload`] into an [`ExecutionPayload`] and [`ExecutionPayloadSidecar`]. | ||
fn block_to_payload( | ||
block: SealedBlockFor< | ||
<<Self::BuiltPayload as BuiltPayload>::Primitives as NodePrimitives>::Block, | ||
>, | ||
) -> (ExecutionPayload, ExecutionPayloadSidecar); |
There was a problem hiding this comment.
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
Co-authored-by: Matthias Seitz <[email protected]>
This PR integrates
NodePrimitives
toBuiltPayload
. It doesn't make a lot of sense as a generic because every built payload is required to holdExecutedBlock
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