Skip to content

Refactor RpcConverter #17545

@klkvr

Description

@klkvr

Describe the feature

We have recently abstracted all of the RPC conversions and intorudced RpcConverter type:

/// Generic RPC response object converter for `Evm` and network `E`.
///
/// The main purpose of this struct is to provide an implementation of [`RpcConvert`] for generic
/// associated types. This struct can then be used for conversions in RPC method handlers.
///
/// An [`RpcConvert`] implementation is generated if the following traits are implemented for the
/// network and EVM associated primitives:
/// * [`FromConsensusTx`]: from signed transaction into RPC response object.
/// * [`TryIntoSimTx`]: from RPC transaction request into a simulated transaction.
/// * [`TryIntoTxEnv`]: from RPC transaction request into an executable transaction.
/// * [`TxInfoMapper`]: from [`TransactionInfo`] into [`FromConsensusTx::TxInfo`]. Should be
/// implemented for a dedicated struct that is assigned to `Map`. If [`FromConsensusTx::TxInfo`]
/// is [`TransactionInfo`] then `()` can be used as `Map` which trivially passes over the input
/// object.
#[derive(Debug)]
pub struct RpcConverter<E, Evm, Receipt, Header = (), Map = ()> {
phantom: PhantomData<(E, Evm)>,
receipt_converter: Receipt,
header_converter: Header,
mapper: Map,
}

It is still somewhat messy and is not super easy to build. What we want is to have 2 ways to define each of the conversions (header, receipt, tx, sim tx, txenv):
a. by directly implementing some trait on a primitive type (e.g impl TryIntoTxEnv on TransactionRequest)
b. by having a closure or fully custom type that does the conversion and potentially contains some useful context

What we can do is have something like RpcConverter<Tx = (), TxEnv = (), SimTx = (), Receipt = (), Header = ()> where each of the generics is a certain converter, for example:

Tx generic should implement trait RpcTxConverter<PrimitiveTx, RpcTx> { ... } which has 2 blanket impls

  1. for () where RpcTx: FromConsensusTx<PrimitiveTx>
  2. for impl Fn(PrimitiveTx) -> RpcTx

then we should also have a helper method with_tx_converter<NewTx>(self, new: NewTx) -> RpcConverter<NewTx, ...> on RpcConverter to set custom values.

That way, RpcConverter::default() would be a fully valid type as long as you implement all of the needed traits on your rpc/primitive types, and if you don't do that because of foreign type rules or needing more context, you can still override all of the ()'s in a more verbose way via a nice builder pattern.

Also, worth exploring whether the converter traits should get N: RpcNodeCore or provider as input to extend their basic capabilities

Additional context

No response

Metadata

Metadata

Assignees

Labels

A-rpcRelated to the RPC implementationA-sdkRelated to reth's use as a libraryC-enhancementNew feature or requestD-good-first-issueNice and easy! A great choice to get started

Type

No type

Projects

Status

Backlog

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions