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

feat: abstract OpPooledTransaction and OpPool over consensus tx #14256

Merged
merged 6 commits into from
Feb 6, 2025

Conversation

klkvr
Copy link
Member

@klkvr klkvr commented Feb 6, 2025

Changes in this PR:

  1. PoolTransaction trait is simplified to not have any conversion traits as supertraits. Core conversions are now defined as Self::from_pooled, Self::into_consensus and Pooled: TryFrom<Consensus> where the latter is responsible for stripping all tx variants that can't be pooled.
  2. OpPooledTransaction is made generic over both consensus and pooled tx. This is a bit ugly but it seems that we can't do much better unfortunately. Ideally PoolTransaction::Pooled should not exist, and the conversion into Pooled variant should be done somewhere closer to networking (which is the only consumer of the conversion)
  3. OpPool and OpPoolBuilder are made generic over pool transaction, allowing them to no longer lock tx to OpPooledTransaction

@klkvr klkvr force-pushed the klkvr/generic-op-pooled branch from 7dd7319 to d3bd76d Compare February 6, 2025 02:30
Copy link
Collaborator

@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.

great

@mattsse mattsse added this pull request to the merge queue Feb 6, 2025
@mattsse mattsse added the A-sdk Related to reth's use as a library label Feb 6, 2025
Merged via the queue into main with commit 823d065 Feb 6, 2025
44 checks passed
@mattsse mattsse deleted the klkvr/generic-op-pooled branch February 6, 2025 11:37
18aaddy pushed a commit to 18aaddy/reth that referenced this pull request Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sdk Related to reth's use as a library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants