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 trait methods for constructing alloy_rpc_types_eth::Transaction to alloy_consensus::Transaction #1172

Merged
merged 14 commits into from
Aug 22, 2024

Conversation

emhane
Copy link
Contributor

@emhane emhane commented Aug 21, 2024

Motivation

Removing the optimism feature from reth-rpc-types-compat in a simple way, requires implementing the conversion from a signed transaction into an rpc transaction object, into the crates that define the associated types of alloy_network::Network. Otherwise, we are not able to access the additional OP fields for example.

Moving into reth-optimsim-rpc is not an option, since neither the trait for conversion nor the <op_alloy_network::Optimism as Network>::TransactionResponse type are defined in the reth-optimsim-rpc crate

Solution

Adds trait methods to alloy_consensus::Transaction, and implements for respective types following implementation of https://github.com/paradigmxyz/reth/blob/302ce7f482d5c3189457fe43d8155676d01766ff/crates/transaction-pool/src/traits.rs#L766-L876

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all of these we definitely want, one nit re blob versioned hashes

crates/consensus/src/transaction/mod.rs Outdated Show resolved Hide resolved
@emhane emhane requested a review from mattsse August 22, 2024 13:56
///
/// This is different than the `max_priority_fee_per_gas` method, which returns `None` for
/// non-EIP-1559 transactions.
fn priority_fee_or_price(&self) -> u128;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can have a default impl like self.max_priority_fee_per_gas().unwrap_or(self.max_fee_per_gas())

though I'm wondering what's a usecase for this method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah got it, its basically how execution layer treats legacy txses wrt priority fee

crates/consensus/src/transaction/eip7702.rs Outdated Show resolved Hide resolved
Co-authored-by: Arsenii Kulikov <[email protected]>
@mattsse mattsse merged commit e037394 into main Aug 22, 2024
26 checks passed
@mattsse mattsse deleted the emhane/tx-trait branch August 22, 2024 20:05
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

Successfully merging this pull request may close these issues.

3 participants