From 57bc0acc38f8a580aff7562ffb799d796d546c4a Mon Sep 17 00:00:00 2001 From: yohkaz Date: Wed, 22 Jan 2025 15:13:35 +0200 Subject: [PATCH 1/2] OP txpool batch validation using eth validator --- crates/optimism/node/src/txpool.rs | 115 ++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 18 deletions(-) diff --git a/crates/optimism/node/src/txpool.rs b/crates/optimism/node/src/txpool.rs index a0e6c6119c54..1765a80ae1d1 100644 --- a/crates/optimism/node/src/txpool.rs +++ b/crates/optimism/node/src/txpool.rs @@ -325,29 +325,49 @@ where } } - /// Validates a single transaction. - /// - /// See also [`TransactionValidator::validate_transaction`] - /// - /// This behaves the same as [`EthTransactionValidator::validate_one`], but in addition, ensures - /// that the account has enough balance to cover the L1 gas cost. - pub fn validate_one( + fn validate_batch( &self, - origin: TransactionOrigin, - transaction: Tx, + transactions: Vec<(TransactionOrigin, Tx)>, + ) -> Vec> { + let outcomes = self.inner.validate_all(transactions); + + outcomes + .into_iter() + .map(|outcome| self.validate_gas_fee(self.validate_transaction_type(outcome))) + .collect() + } + + fn validate_transaction_type( + &self, + outcome: TransactionValidationOutcome, ) -> TransactionValidationOutcome { - if transaction.is_eip4844() { - return TransactionValidationOutcome::Invalid( + match outcome { + TransactionValidationOutcome::Valid { + balance: _, + state_nonce: _, transaction, + propagate: _, + } if transaction.transaction().is_eip4844() => TransactionValidationOutcome::Invalid( + transaction.into_transaction(), InvalidTransactionError::TxTypeNotSupported.into(), - ) + ), + TransactionValidationOutcome::Invalid(tx, _) if tx.is_eip4844() => { + TransactionValidationOutcome::Invalid( + tx, + InvalidTransactionError::TxTypeNotSupported.into(), + ) + } + _ => outcome, } + } - let outcome = self.inner.validate_one(origin, transaction); - + fn validate_gas_fee( + &self, + outcome: TransactionValidationOutcome, + ) -> TransactionValidationOutcome { if !self.requires_l1_data_gas_fee() { // no need to check L1 gas fee - return outcome + return outcome; } // ensure that the account has enough balance to cover the L1 gas cost @@ -385,7 +405,7 @@ where GotExpected { got: balance, expected: cost }.into(), ) .into(), - ) + ); } return TransactionValidationOutcome::Valid { @@ -393,12 +413,34 @@ where state_nonce, transaction: valid_tx, propagate, - } + }; } outcome } + /// Validates a single transaction. + /// + /// See also [`TransactionValidator::validate_transaction`] + /// + /// This behaves the same as [`EthTransactionValidator::validate_one`], but in addition, ensures + /// that the account has enough balance to cover the L1 gas cost. + pub fn validate_one( + &self, + origin: TransactionOrigin, + transaction: Tx, + ) -> TransactionValidationOutcome { + if transaction.is_eip4844() { + return TransactionValidationOutcome::Invalid( + transaction, + InvalidTransactionError::TxTypeNotSupported.into(), + ); + } + + let outcome = self.inner.validate_one(origin, transaction); + self.validate_gas_fee(outcome) + } + /// Validates all given transactions. /// /// Returns all outcomes for the given transactions in the same order. @@ -408,7 +450,7 @@ where &self, transactions: Vec<(TransactionOrigin, Tx)>, ) -> Vec> { - transactions.into_iter().map(|(origin, tx)| self.validate_one(origin, tx)).collect() + self.validate_batch(transactions) } } @@ -503,4 +545,41 @@ mod tests { }; assert_eq!(err.to_string(), "transaction type not supported"); } + + #[test] + fn validate_optimism_transaction_all() { + let client = MockEthProvider::default(); + let validator = EthTransactionValidatorBuilder::new(MAINNET.clone()) + .no_shanghai() + .no_cancun() + .build(client, InMemoryBlobStore::default()); + let validator = OpTransactionValidator::new(validator); + + let origin = TransactionOrigin::External; + let signer = Default::default(); + let deposit_tx = OpTypedTransaction::Deposit(TxDeposit { + source_hash: Default::default(), + from: signer, + to: TxKind::Create, + mint: None, + value: U256::ZERO, + gas_limit: 0, + is_system_transaction: false, + input: Default::default(), + }); + let signature = Signature::test_signature(); + let signed_tx = OpTransactionSigned::new_unhashed(deposit_tx, signature); + let signed_recovered = Recovered::new_unchecked(signed_tx, signer); + let len = signed_recovered.encode_2718_len(); + let pooled_tx = OpPooledTransaction::new(signed_recovered, len); + let outcomes = validator.validate_all(vec![(origin, pooled_tx)]); + + for outcome in outcomes { + let err = match outcome { + TransactionValidationOutcome::Invalid(_, err) => err, + _ => panic!("Expected invalid transaction"), + }; + assert_eq!(err.to_string(), "transaction type not supported"); + } + } } From fba31c30e4e69e06fe825bad7a0614cff0404c1d Mon Sep 17 00:00:00 2001 From: yohkaz Date: Sat, 25 Jan 2025 01:18:57 +0200 Subject: [PATCH 2/2] Perform tx type validation first, also fix formatting --- crates/optimism/node/src/txpool.rs | 82 +++++++++++++++++------------- 1 file changed, 48 insertions(+), 34 deletions(-) diff --git a/crates/optimism/node/src/txpool.rs b/crates/optimism/node/src/txpool.rs index 1765a80ae1d1..6792148e979c 100644 --- a/crates/optimism/node/src/txpool.rs +++ b/crates/optimism/node/src/txpool.rs @@ -329,35 +329,52 @@ where &self, transactions: Vec<(TransactionOrigin, Tx)>, ) -> Vec> { - let outcomes = self.inner.validate_all(transactions); + let length = transactions.len(); + let mut transactions_to_validate = Vec::with_capacity(length); + let mut invalid_tx_type_outcomes = Vec::new(); + for (i, (tx_origin, tx)) in transactions.into_iter().enumerate() { + match Self::validate_transaction_type(tx) { + Ok(tx) => transactions_to_validate.push((tx_origin, tx)), + Err(outcome) => invalid_tx_type_outcomes.push((i, outcome)), + } + } + let validated_outcomes = self.inner.validate_all(transactions_to_validate); + + // final vector containing all outcomes + let mut outcomes = Vec::with_capacity(length); + + // merge outcomes vectors, keeping original order + let mut invalid_type_outcomes_it = invalid_tx_type_outcomes.into_iter(); + let mut validated_outcomes_it = validated_outcomes.into_iter(); + let mut curr_invalid_type_outcome = invalid_type_outcomes_it.next(); + let mut curr_validated_outcome = validated_outcomes_it.next(); + for i in 0..length { + match curr_invalid_type_outcome { + Some((j, outcome)) if i == j => { + outcomes.push(outcome); + curr_invalid_type_outcome = invalid_type_outcomes_it.next(); + continue; + } + _ => {} + } + + if let Some(outcome) = curr_validated_outcome { + outcomes.push(self.validate_gas_fee(outcome)); + curr_validated_outcome = validated_outcomes_it.next(); + } + } outcomes - .into_iter() - .map(|outcome| self.validate_gas_fee(self.validate_transaction_type(outcome))) - .collect() } - fn validate_transaction_type( - &self, - outcome: TransactionValidationOutcome, - ) -> TransactionValidationOutcome { - match outcome { - TransactionValidationOutcome::Valid { - balance: _, - state_nonce: _, - transaction, - propagate: _, - } if transaction.transaction().is_eip4844() => TransactionValidationOutcome::Invalid( - transaction.into_transaction(), + fn validate_transaction_type(tx: Tx) -> Result> { + if tx.is_eip4844() { + Err(TransactionValidationOutcome::Invalid( + tx, InvalidTransactionError::TxTypeNotSupported.into(), - ), - TransactionValidationOutcome::Invalid(tx, _) if tx.is_eip4844() => { - TransactionValidationOutcome::Invalid( - tx, - InvalidTransactionError::TxTypeNotSupported.into(), - ) - } - _ => outcome, + )) + } else { + Ok(tx) } } @@ -367,7 +384,7 @@ where ) -> TransactionValidationOutcome { if !self.requires_l1_data_gas_fee() { // no need to check L1 gas fee - return outcome; + return outcome } // ensure that the account has enough balance to cover the L1 gas cost @@ -405,7 +422,7 @@ where GotExpected { got: balance, expected: cost }.into(), ) .into(), - ); + ) } return TransactionValidationOutcome::Valid { @@ -413,7 +430,7 @@ where state_nonce, transaction: valid_tx, propagate, - }; + } } outcome @@ -430,13 +447,10 @@ where origin: TransactionOrigin, transaction: Tx, ) -> TransactionValidationOutcome { - if transaction.is_eip4844() { - return TransactionValidationOutcome::Invalid( - transaction, - InvalidTransactionError::TxTypeNotSupported.into(), - ); - } - + let transaction = match Self::validate_transaction_type(transaction) { + Ok(tx) => tx, + Err(outcome) => return outcome, + }; let outcome = self.inner.validate_one(origin, transaction); self.validate_gas_fee(outcome) }