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

Add executionPayload abstraction in PayloadTypes #12670

Open
mattsse opened this issue Nov 19, 2024 · 2 comments
Open

Add executionPayload abstraction in PayloadTypes #12670

mattsse opened this issue Nov 19, 2024 · 2 comments
Assignees
Labels
A-sdk Related to reth's use as a library C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started

Comments

@mattsse
Copy link
Collaborator

mattsse commented Nov 19, 2024

Describe the feature

currently we have hardcoded the executionpayload type and sidecar:

/// The execution payload received by Engine API.
payload: ExecutionPayload,
/// The execution payload sidecar with additional version-specific fields received by
/// engine API.
sidecar: ExecutionPayloadSidecar,

ideally we want this to be configurable.

especially when we want to make custom block types work.

first step would be introducing an associated in the EngineTypes

pub trait EngineTypes:

that encapsulates both, for example type ExecutionPayloadRequest

and add a helper type that has the payload and sidecar as fields

Additional context

No response

@mattsse mattsse added C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started labels Nov 19, 2024
@mattsse mattsse added the A-sdk Related to reth's use as a library label Nov 19, 2024
@nils-mathieu
Copy link
Contributor

I'd like to work on this.

@nils-mathieu
Copy link
Contributor

nils-mathieu commented Nov 19, 2024

I'm running into a roadblock.

The create_reorg_head function relies on the internals of the ExecutionPayloadRequest (which is being abstracted out). Specifically, it requires the ExecutionPayloadValidator<Spec> to be coupled with the ExecutionPayloadRequest.

fn create_reorg_head<Provider, Evm, Spec>(
provider: &Provider,
evm_config: &Evm,
payload_validator: &ExecutionPayloadValidator<Spec>,
mut depth: usize,
next_payload: ExecutionPayload,
next_sidecar: ExecutionPayloadSidecar,
) -> RethResult<(ExecutionPayload, ExecutionPayloadSidecar)>

Currently, I have added an associated type to the EngineTypes trait.

trait EngineTypes {
    type ExecutionPayloadRequest;
}

One thing I could do is to add a dependency on Spec for EngineTypes. This way, we would be able to do the following:

trait EngineTypes<Spec> {
    type ExecutionPayloadRequest: ExecutionPayloadRequest<Spec>;
}

trait ExecutionPayloadRequest<Spec> {
    fn create_reorg_head(
        self,
        provider: &Provider,
        evm_config: &Evm,
        payload_validator: &ExecutionPayloadValidator<Spec>,
        depth: usize,
    ) -> RethResult<Self>;
}

Or one alternative would be:

trait EngineTypes {
    type Spec;
    type ExecutionPayloadRequest: ExecutionPayloadRequest<Self::Spec>;
}

Would that be fine? Or am I missing an obvious thing.


Actually I havn't looked too much into it but it seems like Provider and Evm would both need to be coupled as well. Not sure what would the right thing be for this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sdk Related to reth's use as a library C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started
Projects
Status: Todo
Development

No branches or pull requests

2 participants