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: relax more tx manager bounds #12744

Merged
merged 3 commits into from
Nov 21, 2024
Merged

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Nov 21, 2024

relaxes more trait bounds in txmanager

since TxSignedRecovered != TxSigned I had to add an additional Into bound.

this will suck for a bit, but next step would be to gradually remove this, this will require some additional changes first.

no functional changes

@mattsse mattsse added A-networking Related to networking in general A-sdk Related to reth's use as a library labels Nov 21, 2024
impl<Pool> TransactionsManager<Pool>
where
Pool: TransactionPool + 'static,
<<Pool as TransactionPool>::Transaction as PoolTransaction>::Consensus: Into<TransactionSigned>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

these are currently redundant, but I'd like to keep them as a reminder that we need to enforce this conversion

@mattsse mattsse requested a review from klkvr November 21, 2024 15:11
Copy link
Member

@klkvr klkvr left a comment

Choose a reason for hiding this comment

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

lgtm, we can clean this up once we start getting rid of concrete bounds on pool transactions

I think the TransactionSignedEcRecovered just needs to be made generic over transaction? like Recovered<SignedTransaction>

@mattsse mattsse added this pull request to the merge queue Nov 21, 2024
Merged via the queue into main with commit 54ff4c7 Nov 21, 2024
40 checks passed
@mattsse mattsse deleted the matt/relax-more-tx-manager-bounds branch November 21, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-networking Related to networking in general 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