optimism: OpPayloadBuilder, specify PoolTransaction to OpPooledTransaction

[hamdiallam]

• Specify TransactionPool::Transaction to OpPooledTransaction to OpPooledTransaction. This will allow execute_best_transaction to reference the conditional to then validate
• Introduce reth_optimism_transaction_pool that pulls out txpool.rs so that there's no cyclical dep between payload <> node

Open Q: Should we update OpPayloadPrimitives / OpPrimitives to hold type references for the Pool?

[hamdiallam]

needed to implement Transaction such that PayloadTransactionsChain could operate on the MockTransaction type.

Need to update some of these stubbed out methods

[hamdiallam]

@mattsse

lmk what you think about the changes to BestPayloadTransactions. If it's helpful, I can split those code changes from the secondary change to the OpPayloadBuilder

[hamdiallam]

@mattsse soft bump on this, seem to be getting more conflicts as time passes. Have a pending PR adding the conditional to the pooled struct and checking it in the payload builder

[hamdiallam]

@klkvr @mattsse

Just saw the changes from #14249. In order for OpPayloadBuilder to enforce TransactionPool<Transaction: OpPooledTranscation>, I created the optimism/transaction-pool package to avoid the circular dependency between the node <> payload crates.

Can you let me know if this is directionally correct? Since this has a ton of conflicts now, i'll just start over from there. Would rather not do the work if there are more structural changes happening to these crates and there's a different approach in mind

In the draft I had waiting on this, I see the Pool is now available the OpPayloadBuilder. I had a similar change such that on eviction in execute_best_transaction, I could simple invoke Pool::remove_transactions_and_descendants() to evict the tx from the pool.

• Then from this, it is straightforward to have the api extension also do the first-level check at the rpc layer then submit OpPooledTransaction with the attached conditional. No need for a custom Pool trait to implement the feature

[hamdiallam]

Lol now i see #14264 updates the Pooled tx type with the conditional.

In order for the payload builder to reference this type. optimism-transaction-pool still needs to be created like in this PR so that the payload builder can validate the conditional in execute_best_transaction without a cyclical dependency with node.

Going to again rebase this PR with the latest changes. @fgimenez are you also working on this? Please let me know so I dont waste my time

[mattsse]

I'm very sorry @hamdiallam, I dropped the ball here...

need a bit to see what to do here

[hamdiallam]

I really don't mind if you have folks pick it up especially since it's a lot easier for you guys to mess around the type parameters in the right way. Can see where some of the same stuff I was trying to get working was broken

mainly goal is just to get the feature implemented

[hamdiallam]

@fgimenez rebased this PRs. Have lint stuff to fix but I think some of the type bounds can be made cleaner through NodePrimitives.

Also if it's straight forward, feel free to just take this and fix it up if that's faster for you, just lmk.

From this, validating the conditional should be straight forward and a reference to the pool via the OpPayloadBuilderCtx allows for eviction without needing a pool wrapper

[hamdiallam]

for example, can we make SignedTx = OpTransactionSigned? Any reason this needs to be generic in OpPayloadPrimitives

[emhane]

superseded by #14280, thanks anyway @hamdiallam

[hamdiallam]

@mattsse what is going man?

[hamdiallam]

This is extremely unprofessional i'm no longer supporting this workstream. good luck