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

Improve Engine API abstraction #6734

Closed
mattsse opened this issue Feb 22, 2024 · 2 comments · Fixed by #6871
Closed

Improve Engine API abstraction #6734

mattsse opened this issue Feb 22, 2024 · 2 comments · Fixed by #6871
Assignees
Labels
A-rpc Related to the RPC implementation C-enhancement New feature or request

Comments

@mattsse
Copy link
Collaborator

mattsse commented Feb 22, 2024

Describe the feature

Currently we provide a way to abstract parts of the engine API (PayloadAttributes) because these are different eth vs OP

/// The types that are used by the engine API.
pub trait EngineTypes:
serde::de::DeserializeOwned + fmt::Debug + Unpin + Send + Sync + Clone
{
/// The RPC payload attributes type the CL node emits via the engine API.
type PayloadAttributes: PayloadAttributes + Unpin;

with alloy-rs/alloy#227 we now have the case where the return type: ExecutionPayload is also no longer consistent which means those trait conversion functions are no longer entirely consistent:

/// Represents a built payload type that contains a built [SealedBlock] and can be converted into
/// engine API execution payloads.
pub trait BuiltPayload: Send + Sync + std::fmt::Debug {
/// Returns the built block (sealed)
fn block(&self) -> &SealedBlock;
/// Returns the fees collected for the built block
fn fees(&self) -> U256;
/// Converts the type into the response expected by `engine_getPayloadV1`
fn into_v1_payload(self) -> ExecutionPayloadV1;
/// Converts the type into the response expected by `engine_getPayloadV2`
fn into_v2_payload(self) -> ExecutionPayloadEnvelopeV2;
/// Converts the type into the response expected by `engine_getPayloadV2`
fn into_v3_payload(self) -> ExecutionPayloadEnvelopeV3;
}

The way it currently works is, the EngineTypes configure

  • The PayloadAttributes type: type we get from the CL
  • BuiltPayload: the type that payload builder service builds, which currently must be convertible into the versioned execution payload envelope

I think we now also the ExecutionPayloadV* types to be configurable, so that we can have clean types for all networks and don't need additional Options for OP for example, see alloy PR

TODO

we can solve this in a number of ways, but I think we want an additional associated type for the ExecutionPayload on the EngineTypes trait and then a requirement that the configured BuiltPayload can be converted into this type.

perhaps we want an associated type for each version or an enum, I assume 3 types might be easier to handle from engine impl POV:

pub async fn get_payload_v3(
&self,
payload_id: PayloadId,
) -> EngineApiResult<ExecutionPayloadEnvelopeV3> {

This would also require to introduce OP specific V3 type on alloy-rpc

@fgimenez would you be interested in this?

Additional context

No response

@mattsse mattsse added C-enhancement New feature or request S-needs-triage This issue needs to be labelled A-rpc Related to the RPC implementation and removed S-needs-triage This issue needs to be labelled labels Feb 22, 2024
@fgimenez
Copy link
Member

@fgimenez would you be interested in this?

absolutely! thx a lot @mattsse will start with it asap

@mattsse
Copy link
Collaborator Author

mattsse commented Feb 22, 2024

fyi @fgimenez this is also relevant here

#6331

We should do TryFrom or similar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation C-enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants