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 EngineApiBuilder to RpcAddons #13831

Closed
Tracked by #8987 ...
emhane opened this issue Jan 16, 2025 · 12 comments · Fixed by #14426
Closed
Tracked by #8987 ...

Add EngineApiBuilder to RpcAddons #13831

emhane opened this issue Jan 16, 2025 · 12 comments · Fixed by #14426
Assignees
Labels
A-op-reth Related to Optimism and op-reth A-rpc Related to the RPC implementation A-sdk Related to reth's use as a library C-debt A clean up/refactor of existing code

Comments

@emhane
Copy link
Member

emhane commented Jan 16, 2025

Describe the feature

Define EngineApiBuilder and add to RpcAddons. Follow how it has been done for EthApiBuilder and EngineValidator

/// Builder for `EthApi`
eth_api_builder: Box<dyn FnOnce(&EthApiBuilderCtx<Node>) -> EthApi + Send + Sync>,
/// Engine validator
engine_validator_builder: EV,

Then, instead of calling EngineApi constructor here, call EngineApiBuilder::build

let engine_api = EngineApi::new(
node.provider().clone(),
config.chain.clone(),
beacon_engine_handle,
PayloadStore::new(node.payload_builder().clone()),
node.pool().clone(),
Box::new(node.task_executor().clone()),
client,
EngineCapabilities::default(),
engine_validator.clone(),
);

Additional context

No response

@emhane emhane added A-op-reth Related to Optimism and op-reth A-rpc Related to the RPC implementation C-debt A clean up/refactor of existing code labels Jan 16, 2025
@emhane emhane added the A-sdk Related to reth's use as a library label Jan 16, 2025
@mattsse
Copy link
Collaborator

mattsse commented Jan 16, 2025

if we need to modify engine api then this should become part of the launch logic I think

@emhane
Copy link
Member Author

emhane commented Jan 17, 2025

not sure I follow? rn this trait isn't used, implementing this issue would make way for using it
https://github.com/alloy-rs/op-alloy/blob/b506e16d6e6bb0f3cf416c965092b693fea4cb71/crates/provider/src/ext/engine.rs#L13-L180

@klkvr
Copy link
Member

klkvr commented Jan 17, 2025

@emhane isn't this trait only relevant for alloy provider and not reth rpc? i.e op-node would still expect eth-compatible engine API

@emhane
Copy link
Member Author

emhane commented Jan 17, 2025

@emhane
Copy link
Member Author

emhane commented Jan 17, 2025

@cody-wang-cb are you keen on this to unblock ethereum-optimism/pm#20

@mattsse
Copy link
Collaborator

mattsse commented Jan 17, 2025

unclear what this is issue is about tbh

what's the usecase here

@emhane
Copy link
Member Author

emhane commented Jan 18, 2025

actually op Engine API shouldn't support v1 requests, it breaks spec. eventually we want to use the op_alloy_provider::OpEngineApi in op-reth. to do so, an Engine API builder must be exposed at sdk level.

ref https://specs.optimism.io/protocol/exec-engine.html

@mattsse
Copy link
Collaborator

mattsse commented Jan 24, 2025

eventually we want to use the op_alloy_provider::OpEngineApi in op-reth

we don't

closing for now, unclear what this is about

@mattsse mattsse closed this as not planned Won't fix, can't repro, duplicate, stale Jan 24, 2025
@github-project-automation github-project-automation bot moved this from Todo to Done in Reth Tracker Jan 24, 2025
@emhane
Copy link
Member Author

emhane commented Feb 3, 2025

pre-req for using op-alloy OpEngineApi which is needed for using Engine API v4 in isthmus which diverges from l1 Engine API

@emhane
Copy link
Member Author

emhane commented Feb 10, 2025

@klkvr I will implement OpEngineApiBuilder now that #14354 and #14359 are merged - or are you already working on this?

@klkvr
Copy link
Member

klkvr commented Feb 10, 2025

@emhane wasn't working on this yet, feel free to pick this up

@emhane
Copy link
Member Author

emhane commented Feb 10, 2025

ok nice! will finish this issue off first thing tomorrow then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-op-reth Related to Optimism and op-reth A-rpc Related to the RPC implementation A-sdk Related to reth's use as a library C-debt A clean up/refactor of existing code
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants