feat: Improve OP txpool batch validation #13918
Conversation
emhane
added
A-op-reth
|
|
||
| outcomes | ||
| .into_iter() | ||
| .map(|outcome| self.validate_gas_fee(self.validate_transaction_type(outcome))) |
There was a problem hiding this comment.
note: i validate the transaction type after performing the transaction validation (validate_all), which is in a different order than done previously. if you think this is problematic, lmk.
| if transaction.is_eip4844() { | ||
| return TransactionValidationOutcome::Invalid( | ||
| transaction, | ||
| InvalidTransactionError::TxTypeNotSupported.into(), | ||
| ); | ||
| } |
There was a problem hiding this comment.
this can call the new method validate_transaction_type right?
| } | ||
|
|
||
| return TransactionValidationOutcome::Valid { | ||
| balance, | ||
| state_nonce, | ||
| transaction: valid_tx, | ||
| propagate, | ||
| } | ||
| }; |
There was a problem hiding this comment.
pls revert all these semicolons, and take care to use cargo +nightly fmt instead of cargo fmt in the future
| fn validate_batch( | ||
| &self, | ||
| origin: TransactionOrigin, | ||
| transaction: Tx, | ||
| transactions: Vec<(TransactionOrigin, Tx)>, | ||
| ) -> Vec<TransactionValidationOutcome<Tx>> { | ||
| let outcomes = self.inner.validate_all(transactions); | ||
|
|
||
| outcomes | ||
| .into_iter() | ||
| .map(|outcome| self.validate_gas_fee(self.validate_transaction_type(outcome))) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
it makes sense to do the tx type validation first! it is much cheaper in the case of an eip4844 transaction, as then it will bail on first check as opposed to if you do batch validation on inner EthTransactionValidator first, which would allow the eip4844 tx to pass through and then carry out all the other checks on it for no reason
Closes #13902
(someone is already assigned, did it for the exercise :)