From 4fb1956e1ad35db49b700a5be46a5e936281b219 Mon Sep 17 00:00:00 2001 From: steven Date: Wed, 15 Jan 2025 00:11:14 -0600 Subject: [PATCH 1/3] feat: Track DA cost in Op Pooled transactions --- crates/optimism/node/src/txpool.rs | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/crates/optimism/node/src/txpool.rs b/crates/optimism/node/src/txpool.rs index 0cce61fb7a01..bcfd4fb9f6b8 100644 --- a/crates/optimism/node/src/txpool.rs +++ b/crates/optimism/node/src/txpool.rs @@ -37,12 +37,22 @@ pub type OpTransactionPool = Pool< /// Pool transaction for OP. #[derive(Debug, Clone, derive_more::Deref)] -pub struct OpPooledTransaction(EthPooledTransaction); +pub struct OpPooledTransaction { + inner: EthPooledTransaction, + da_cost: U256, +} impl OpPooledTransaction { /// Create new instance of [Self]. pub fn new(transaction: RecoveredTx, encoded_length: usize) -> Self { - Self(EthPooledTransaction::new(transaction, encoded_length)) + Self { inner: EthPooledTransaction::new(transaction, encoded_length), da_cost: U256::ZERO } + } + + /// Estimate DA costs + pub fn estimate_da_cost(&self) -> U256 { + let tx = self.transaction().clone_into_consensus(); + let cost = reth_revm::estimate_tx(&tx, None, None).await.unwrap(); + U256::from(cost) } } @@ -296,7 +306,7 @@ where /// 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. + /// that the account has enough balance to cover the L1 gas cost and DA cost. pub fn validate_one( &self, origin: TransactionOrigin, @@ -316,7 +326,7 @@ where return outcome } - // ensure that the account has enough balance to cover the L1 gas cost + // ensure that the account has enough balance to cover the L1 gas cost and DA cost if let TransactionValidationOutcome::Valid { balance, state_nonce, @@ -341,14 +351,17 @@ where return TransactionValidationOutcome::Error(*valid_tx.hash(), Box::new(err)) } }; - let cost = valid_tx.transaction().cost().saturating_add(cost_addition); + + let da_cost = valid_tx.estimate_da_cost(); + let total_cost = + valid_tx.transaction().cost().saturating_add(cost_addition).saturating_add(da_cost); // Checks for max cost - if cost > balance { + if total_cost > balance { return TransactionValidationOutcome::Invalid( valid_tx.into_transaction(), InvalidTransactionError::InsufficientFunds( - GotExpected { got: balance, expected: cost }.into(), + GotExpected { got: balance, expected: total_cost }.into(), ) .into(), ) From 4c418e93c382264887310953870f800a7c23a5ff Mon Sep 17 00:00:00 2001 From: steven Date: Fri, 17 Jan 2025 00:38:10 -0600 Subject: [PATCH 2/3] lazylock --- crates/optimism/node/src/txpool.rs | 91 +++++++++++++++++------------- 1 file changed, 51 insertions(+), 40 deletions(-) diff --git a/crates/optimism/node/src/txpool.rs b/crates/optimism/node/src/txpool.rs index bcfd4fb9f6b8..bf664cc6d51c 100644 --- a/crates/optimism/node/src/txpool.rs +++ b/crates/optimism/node/src/txpool.rs @@ -25,7 +25,7 @@ use reth_transaction_pool::{ use revm::primitives::{AccessList, KzgSettings}; use std::sync::{ atomic::{AtomicU64, Ordering}, - Arc, + Arc, LazyLock, }; /// Type alias for default optimism transaction pool @@ -36,23 +36,34 @@ pub type OpTransactionPool = Pool< >; /// Pool transaction for OP. -#[derive(Debug, Clone, derive_more::Deref)] +#[derive(Debug, derive_more::Deref)] pub struct OpPooledTransaction { + #[deref] inner: EthPooledTransaction, - da_cost: U256, + estimated_tx_compressed_size: LazyLock, } impl OpPooledTransaction { /// Create new instance of [Self]. pub fn new(transaction: RecoveredTx, encoded_length: usize) -> Self { - Self { inner: EthPooledTransaction::new(transaction, encoded_length), da_cost: U256::ZERO } + Self { + inner: EthPooledTransaction::new(transaction, encoded_length), + estimated_tx_compressed_size: LazyLock::new(|| Default::default()), + } } - /// Estimate DA costs - pub fn estimate_da_cost(&self) -> U256 { - let tx = self.transaction().clone_into_consensus(); - let cost = reth_revm::estimate_tx(&tx, None, None).await.unwrap(); - U256::from(cost) + /// Returns the estimated compressed size of a transaction. + pub fn compressed_size(&self) -> u64 { + *self.estimated_tx_compressed_size + } +} + +impl Clone for OpPooledTransaction { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + estimated_tx_compressed_size: LazyLock::new(|| Default::default()), + } } } @@ -60,7 +71,10 @@ impl From> for OpPooledTran fn from(tx: RecoveredTx) -> Self { let encoded_len = tx.encode_2718_len(); let tx = tx.map_transaction(|tx| tx.into()); - Self(EthPooledTransaction::new(tx, encoded_len)) + Self { + inner: EthPooledTransaction::new(tx, encoded_len), + estimated_tx_compressed_size: LazyLock::new(|| Default::default()), + } } } @@ -77,7 +91,7 @@ impl TryFrom> for OpPooledTransaction { impl From for RecoveredTx { fn from(value: OpPooledTransaction) -> Self { - value.0.transaction + value.inner.transaction } } @@ -87,7 +101,7 @@ impl PoolTransaction for OpPooledTransaction { type Pooled = op_alloy_consensus::OpPooledTransaction; fn clone_into_consensus(&self) -> RecoveredTx { - self.transaction().clone() + self.inner.transaction().clone() } fn try_consensus_into_pooled( @@ -98,79 +112,79 @@ impl PoolTransaction for OpPooledTransaction { } fn hash(&self) -> &TxHash { - self.transaction.tx_hash() + self.inner.transaction.tx_hash() } fn sender(&self) -> Address { - self.transaction.signer() + self.inner.transaction.signer() } fn sender_ref(&self) -> &Address { - self.transaction.signer_ref() + self.inner.transaction.signer_ref() } fn nonce(&self) -> u64 { - self.transaction.nonce() + self.inner.transaction.nonce() } fn cost(&self) -> &U256 { - &self.cost + &self.inner.cost } fn gas_limit(&self) -> u64 { - self.transaction.gas_limit() + self.inner.transaction.gas_limit() } fn max_fee_per_gas(&self) -> u128 { - self.transaction.transaction.max_fee_per_gas() + self.inner.transaction.transaction.max_fee_per_gas() } fn access_list(&self) -> Option<&AccessList> { - self.transaction.access_list() + self.inner.transaction.access_list() } fn max_priority_fee_per_gas(&self) -> Option { - self.transaction.transaction.max_priority_fee_per_gas() + self.inner.transaction.transaction.max_priority_fee_per_gas() } fn max_fee_per_blob_gas(&self) -> Option { - self.transaction.max_fee_per_blob_gas() + self.inner.transaction.max_fee_per_blob_gas() } fn effective_tip_per_gas(&self, base_fee: u64) -> Option { - self.transaction.effective_tip_per_gas(base_fee) + self.inner.transaction.effective_tip_per_gas(base_fee) } fn priority_fee_or_price(&self) -> u128 { - self.transaction.priority_fee_or_price() + self.inner.transaction.priority_fee_or_price() } fn kind(&self) -> TxKind { - self.transaction.kind() + self.inner.transaction.kind() } fn is_create(&self) -> bool { - self.transaction.is_create() + self.inner.transaction.is_create() } fn input(&self) -> &[u8] { - self.transaction.input() + self.inner.transaction.input() } fn size(&self) -> usize { - self.transaction.transaction.input().len() + self.inner.transaction.transaction.input().len() } fn tx_type(&self) -> u8 { - self.transaction.ty() + self.inner.transaction.ty() } fn encoded_length(&self) -> usize { - self.encoded_length + self.inner.encoded_length } fn chain_id(&self) -> Option { - self.transaction.chain_id() + self.inner.transaction.chain_id() } } @@ -206,7 +220,7 @@ impl EthPoolTransaction for OpPooledTransaction { } fn authorization_count(&self) -> usize { - match &self.transaction.transaction { + match &self.inner.transaction.transaction { OpTypedTransaction::Eip7702(tx) => tx.authorization_list.len(), _ => 0, } @@ -306,7 +320,7 @@ where /// 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 and DA cost. + /// that the account has enough balance to cover the L1 gas cost. pub fn validate_one( &self, origin: TransactionOrigin, @@ -326,7 +340,7 @@ where return outcome } - // ensure that the account has enough balance to cover the L1 gas cost and DA cost + // ensure that the account has enough balance to cover the L1 gas cost if let TransactionValidationOutcome::Valid { balance, state_nonce, @@ -351,17 +365,14 @@ where return TransactionValidationOutcome::Error(*valid_tx.hash(), Box::new(err)) } }; - - let da_cost = valid_tx.estimate_da_cost(); - let total_cost = - valid_tx.transaction().cost().saturating_add(cost_addition).saturating_add(da_cost); + let cost = valid_tx.transaction().cost().saturating_add(cost_addition); // Checks for max cost - if total_cost > balance { + if cost > balance { return TransactionValidationOutcome::Invalid( valid_tx.into_transaction(), InvalidTransactionError::InsufficientFunds( - GotExpected { got: balance, expected: total_cost }.into(), + GotExpected { got: balance, expected: cost }.into(), ) .into(), ) From 0b319707aea51f7886a199da41b4f494e174c46d Mon Sep 17 00:00:00 2001 From: steven Date: Fri, 17 Jan 2025 00:52:07 -0600 Subject: [PATCH 3/3] clippy --- crates/optimism/node/src/txpool.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/optimism/node/src/txpool.rs b/crates/optimism/node/src/txpool.rs index bf664cc6d51c..33b1818bec6c 100644 --- a/crates/optimism/node/src/txpool.rs +++ b/crates/optimism/node/src/txpool.rs @@ -48,7 +48,7 @@ impl OpPooledTransaction { pub fn new(transaction: RecoveredTx, encoded_length: usize) -> Self { Self { inner: EthPooledTransaction::new(transaction, encoded_length), - estimated_tx_compressed_size: LazyLock::new(|| Default::default()), + estimated_tx_compressed_size: LazyLock::new(Default::default), } } @@ -62,7 +62,7 @@ impl Clone for OpPooledTransaction { fn clone(&self) -> Self { Self { inner: self.inner.clone(), - estimated_tx_compressed_size: LazyLock::new(|| Default::default()), + estimated_tx_compressed_size: LazyLock::new(Default::default), } } } @@ -73,7 +73,7 @@ impl From> for OpPooledTran let tx = tx.map_transaction(|tx| tx.into()); Self { inner: EthPooledTransaction::new(tx, encoded_len), - estimated_tx_compressed_size: LazyLock::new(|| Default::default()), + estimated_tx_compressed_size: LazyLock::new(Default::default), } } }