tx-pool: rm into tx constraint for PoolTransaction

[tcoratger]

Related #8668

[emhane]

make sense if you review @klkvr since your work to make generic over primitives in other crates collides/overlaps. otherwise also @joshieDo currently looking at best way to do this in provider.

[tcoratger]

make sense if you review @klkvr since your work to make generic over primitives in other crates collides/overlaps. otherwise also @joshieDo currently looking at best way to do this in provider.

To give a bit more context to this PR, it follows a discussion with @mattsse about #8668 and its resolution in several steps to avoid doing everything at once.

• The first steps were tx-pool: Make txpool independent of primitive tx types #9916 and tx-pool: rm constraint for PoolTransaction #10024
• This PR almost completes the issue, all that's left is to remove the last constraint on the trait to completely complete the issue in a follow up PR that I'll do once this one is merged so as not to cause confusion and a review that's too long :)

[klkvr]

This currently introduces pretty strict requirements of Tx: T: PoolTransaction<Consensus = TransactionSignedEcRecovered> on many of transaction-pool types which don't rely on this conversion.

Given that most of the conversion consumers are outside of tx-pool crate I am wondering if we should relax those trait bounds to keep tx-pool types more flexible and instead move them to types explicitly relying on those conversions?

[klkvr]

I've tried relaxing the bounds locally and it seems that a lot of them can either be removed or only kept at a small subset of impl blocks

Wondering if the root issue is that you're adding T: PoolTransaction<Consensus = TransactionSignedEcRecovered> to struct definitions? T: PoolTransaction should be mostly enough, and impls can restrict Consensus AT if needed

Though I am wondering if I am missing something about the chosen approach here and those bounds on structs are actually desired? (cc @emhane @mattsse)

[emhane]

I've tried relaxing the bounds locally and it seems that a lot of them can either be removed or only kept at a small subset of impl blocks

Wondering if the root issue is that you're adding T: PoolTransaction<Consensus = TransactionSignedEcRecovered> to struct definitions? T: PoolTransaction should be mostly enough, and impls can restrict Consensus AT if needed

Though I am wondering if I am missing something about the chosen approach here and those bounds on structs are actually desired? (cc @emhane @mattsse)

I agree that it makes sense to relax bounds on ATs here. since we are not yet completely sure how we will inject NodeTypes::Primitives into reth-transaction-pool crate, narrowing down constraints on ATs to the smallest scope possible is helping us impl that later in the most logic way.

if you already have the code locally @klkvr, feel free to push to this branch.

[tcoratger]

@klkvr yes please feel free to push here. I wanted to relax as much as possible but I certainly missed a point here. If you don't push I'll try myself tonight but feel free to do so to make it faster :)

[tcoratger]

Nice @klkvr seems logical to me this way! Once this PR is merged, I'll do the last one to close the issue!