feat: use network tx for Pool::Pooled
#13159
Conversation
klkvr
requested review from
mattsse,
Rjected,
emhane,
fgimenez and
onbjerg
as code owners
| .into_iter() | ||
| .map(|tx| tx.into_signed()) | ||
| .collect::<Vec<_>>(); |
There was a problem hiding this comment.
this collect is a bit redundant and get_pooled_transaction_elements was a workaround to avoid the collect here
| { | ||
| self.pool.get_pooled_transactions_as(tx_hashes, limit) |
There was a problem hiding this comment.
this existed only so we can avoid a double-collect,
we can either find a way to enforce the
From<RecoveredTx<N::PooledTransaction>> again which would be a bit weird because this would always be into_signed.
or we add another helper fn to this trait that strips the Recovered layer away and returns just the pooled type
There was a problem hiding this comment.
looks like this fn is only used here, so I've just made it return transactions with already stripped signer e6c0f18
| Pool::Transaction: | ||
| PoolTransaction<Consensus = N::BroadcastedTransaction, Pooled = N::PooledTransaction>, |
There was a problem hiding this comment.
hmm, I think this would work for both OP and eth,
but for OP's case this would mean Consensus must be a different type that what we currently have for TransactionSigned (minus the deposit variant)
so Conensus really just means: All consensus variants that are allowed over p2p
There was a problem hiding this comment.
couldn't Consensus be same as Pooled for OP?
There was a problem hiding this comment.
yes, for op these are the same type, basically TransactionSigned - Deposit
| @@ -981,7 +967,7 @@ pub trait PoolTransaction: | |||
| type Consensus; | |||
|
|
|||
| /// Associated type representing the recovered pooled variant of the transaction. | |||
| type Pooled: Encodable2718 + Into<Self>; | |||
| type Pooled: SignedTransaction; | |||
There was a problem hiding this comment.
makes sense, we don't expect non signed txs here
There was a problem hiding this comment.
yeah, it's also more like a RecoverableTransaction
| PoolTransaction< | ||
| Pooled: From<PooledTransactionsElementEcRecovered> | ||
| + Into<PooledTransactionsElementEcRecovered> | ||
| + Into<PooledTransactionsElement>, | ||
| > |
Similar to #13086, but for
Pooledtransaction AT.This relaxes bound on
EthPoolTransaction, so they no longer require from/into traits for concretePooledTransactionsEleement, and the conversion (Consensus, sidecar) -> Pooled is now abstracted asEthPoolTransaction::try_from_eip4844