feat: add reth traits to alloy for op-alloy dependency removal#3649
feat: add reth traits to alloy for op-alloy dependency removal#3649theochap wants to merge 2 commits intoalloy-rs:mainfrom
Conversation
|
Btw I implemented some new type wrappers for the DB traits in op-reth directly (Compact and such). Here are all the traits that could be cleanly pulled out of op-reth |
theochap
requested review from
DaniPopes,
grandizzy,
klkvr,
onbjerg and
zerosnacks
as code owners
Move 10 trait definitions from reth core crates into alloy so that op-alloy can implement them natively without depending on reth: - alloy-consensus: InMemorySize, SerdeBincodeCompat, SignedTransaction - alloy-rpc-types-engine: PayloadAttributes, ExecutionPayload - alloy-network: FromConsensusTx, TryFromTransactionResponse, TryFromReceiptResponse, SignableTxRequest, TryIntoSimTx, RpcTypes Co-Authored-By: Claude Opus 4.6 <[email protected]>
theochap
force-pushed
the
theo/add-reth-traits
branch
from
008019b to
8f96b37
Compare
onbjerg
left a comment
onbjerg
left a comment
There was a problem hiding this comment.
generally supportive, but it seems like a lot of these are because reth is not fully using alloy and instead uses a bunch of wrapper traits/duplicated functionality
| /// | ||
| /// Use unchecked methods only when dealing with historical pre-EIP-2 transactions. | ||
| #[auto_impl::auto_impl(&, Arc)] | ||
| pub trait SignedTransaction: |
There was a problem hiding this comment.
we already have Signed<T>, I'm not sure why this is different in reth
There was a problem hiding this comment.
Signed<T> looks pretty different from SignedTransaction. One is a trait, the other is a struct. I think reth wants to make it possible to generically override transactions with custom types that don't come from alloy
| /// | ||
| /// The recommended way to add bincode compatible serialization is via the | ||
| /// [`serde_with`] crate and the `serde_as` macro. | ||
| pub trait SerdeBincodeCompat: Sized + 'static { |
There was a problem hiding this comment.
I don't think this belongs in alloy tbh
There was a problem hiding this comment.
Where do you think it would be better to integrate?
| /// Converts `self` into `T`. | ||
| /// | ||
| /// Should create a fake transaction for simulation using [`TransactionRequest`]. | ||
| pub trait TryIntoSimTx<T> |
There was a problem hiding this comment.
these probably belong in alloy-evm?
There was a problem hiding this comment.
Ack, will move it there
There was a problem hiding this comment.
Just to confirm, "these" mean all the traits in this file?
crates/network/src/lib.rs
Outdated
| /// RPC types used by the `eth_` RPC API. | ||
| /// | ||
| /// This is a subset of [`Network`] trait with only RPC response types kept. | ||
| pub trait RpcTypes: Send + Sync + Clone + Unpin + Debug + 'static { |
There was a problem hiding this comment.
these already exist in Network, no need to duplicate them.
crates/network/src/lib.rs
Outdated
| pub type RpcTransaction<T> = <T as RpcTypes>::TransactionResponse; | ||
|
|
||
| /// Adapter for network specific receipt response. | ||
| pub type RpcReceipt<T> = <T as RpcTypes>::Receipt; | ||
|
|
||
| /// Adapter for network specific header response. | ||
| pub type RpcHeader<T> = <T as RpcTypes>::Header; | ||
|
|
||
| /// Adapter for network specific block type. | ||
| pub type RpcBlock<T> = | ||
| alloy_rpc_types_eth::Block<RpcTransaction<T>, RpcHeader<T>>; | ||
|
|
||
| /// Adapter for network specific transaction request. | ||
| pub type RpcTxReq<T> = <T as RpcTypes>::TransactionRequest; |
crates/rpc-types-engine/src/lib.rs
Outdated
| // Re-export trait names without the `Trait` suffix inside the traits module for direct use. | ||
| // Users can also import from `alloy_rpc_types_engine::traits::{PayloadAttributes, ExecutionPayload}`. |
There was a problem hiding this comment.
unsure what this comment is doing here?
3 participants
Summary
alloy-consensus:InMemorySize,SerdeBincodeCompat,SignedTransactiontraits + impls for Ethereum typesalloy-rpc-types-engine:PayloadAttributes,ExecutionPayloadtraits + impls for alloy's own structsalloy-network:FromConsensusTx,TryFromTransactionResponse,TryFromReceiptResponse,SignableTxRequest,TryIntoSimTx,RpcTypestraits + Ethereum implsMotivation
This is part of a multi-repo effort to remove
op-alloy-*dependencies from core (non-optimism) reth crates. By moving trait definitions into alloy, op-alloy can implement them natively for OP types without needing to depend on reth, and reth core crates can drop their op-alloy dependencies entirely.Test plan
cargo check --workspace --all-featurescargo nextest run --workspacelib.rs🤖 Generated with Claude Code