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

[Feature] Constructor in primitive data type traits (TxReceipt, TransactionResponse, etc.) #1315

Open
emhane opened this issue Sep 19, 2024 · 0 comments
Labels
debt Tech debt which needs to be addressed

Comments

@emhane
Copy link
Member

emhane commented Sep 19, 2024

Component

node-bindings

Describe the feature you would like

Op-reth, will always have the problem with the primitive data types, that it won't be able to implement any conversions from reth/op-reth types to op-alloy types, on the op-alloy type directly. Another type must always be defined in op-reth, and added to the node at sdk level, i.e. generalised (for reth too). That means, each primitive data type added at sdk level, must be paired with a new conversion type. This gets quite heavy to maintain! And is only useful for rollups that have its primitive data types implemented in a different crate than the node components (like op-reth and op-alloy).

This also applies for conversions from op-alloy types to other op-alloy types in reth/op-reth, when the only view of them that reth/op-reth sees, is through the alloy primitive traits (e.g. TxReceipt, TransactionResponse, etc.)

The goal:

We want to add a trait bound at sdk level for conversion from reth/op-reth type: NodePrimitives::Transaction: From<TransactionSigned<T>>.

The problem:

  • TransactionSigned<T> is a reth type. We can't import it into op-alloy to implement the conversion.
  • If we tried to hack this by defining a new trait in reth, and instead using this at sdk level: NodePrimitives::Transaction: FromTransactionSigned, then we have the next problem.
  • FromTransactionSigned can only be implemented on foreign types (op-alloy) in the crate it is defined - but then we would be importing op-alloy types into reth, that's the opposite of what we want!
  • We can't have the trait bound on NodePrimitives::Transaction like this. We would need it on a new associated type NodePrimitives::TransactionCompat: TransactionSignedToRpcConverter (or as associated type on some reth component).
  • The type used for NodePrimitives::TransactionCompat can be defined in op-reth. However, the decoupling of a type from its constructor, gets very complex, since the reth/op-reth node builder already has a bunch of components as associated types to keep track of.

The solution:

Move primitive data type construction into alloy primitive traits.

For example TransactionResponse, can be updated to

pub trait TransactionResponse<T>: From<T> { .. }

or, more granular generics for guidance, without loss of generality (ref #1173 (review))

pub trait TransactionResponse<Tx, Sig, BlockCtx>: From<(Tx, Sig, BlockCtx)>

Additional context

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Tech debt which needs to be addressed
Projects
None yet
Development

No branches or pull requests

1 participant