From cd7e1135d88bf3fb3dd23b1778d7d7c204d77119 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 5 Sep 2023 21:26:01 +0200 Subject: [PATCH] feat: enforce no nonce gaps for eip-4844 (#4487) Co-authored-by: Dan Cline <6798349+Rjected@users.noreply.github.com> --- crates/rpc/rpc/src/eth/error.rs | 3 + crates/transaction-pool/src/error.rs | 4 ++ crates/transaction-pool/src/pool/txpool.rs | 71 +++++++++++++++++++-- crates/transaction-pool/src/validate/mod.rs | 6 ++ 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/crates/rpc/rpc/src/eth/error.rs b/crates/rpc/rpc/src/eth/error.rs index 6a79f59e19..a2b0e38126 100644 --- a/crates/rpc/rpc/src/eth/error.rs +++ b/crates/rpc/rpc/src/eth/error.rs @@ -514,6 +514,9 @@ impl From for RpcPoolError { InvalidPoolTransactionError::Underpriced => RpcPoolError::Underpriced, InvalidPoolTransactionError::Other(err) => RpcPoolError::PoolTransactionError(err), InvalidPoolTransactionError::Eip4844(err) => RpcPoolError::Eip4844(err), + InvalidPoolTransactionError::Overdraft => { + RpcPoolError::Invalid(RpcInvalidTransactionError::InsufficientFunds) + } } } } diff --git a/crates/transaction-pool/src/error.rs b/crates/transaction-pool/src/error.rs index 8b57b61164..1f0442833d 100644 --- a/crates/transaction-pool/src/error.rs +++ b/crates/transaction-pool/src/error.rs @@ -169,6 +169,9 @@ pub enum InvalidPoolTransactionError { /// Thrown if the transaction's fee is below the minimum fee #[error("transaction underpriced")] Underpriced, + /// Thrown if the transaction's would require an account to be overdrawn + #[error("transaction overdraws from account")] + Overdraft, /// Eip-4844 related errors #[error(transparent)] Eip4844(#[from] Eip4844PoolTransactionError), @@ -228,6 +231,7 @@ impl InvalidPoolTransactionError { // local setting false } + InvalidPoolTransactionError::Overdraft => false, InvalidPoolTransactionError::Other(err) => err.is_bad_transaction(), InvalidPoolTransactionError::Eip4844(eip4844_err) => { match eip4844_err { diff --git a/crates/transaction-pool/src/pool/txpool.rs b/crates/transaction-pool/src/pool/txpool.rs index 98e07d39ca..d3e84ffafb 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -1,7 +1,7 @@ //! The internal transaction pool implementation. use crate::{ config::TXPOOL_MAX_ACCOUNT_SLOTS_PER_SENDER, - error::{InvalidPoolTransactionError, PoolError}, + error::{Eip4844PoolTransactionError, InvalidPoolTransactionError, PoolError}, identifier::{SenderId, TransactionId}, metrics::TxPoolMetrics, pool::{ @@ -396,10 +396,10 @@ impl TxPool { Ok(res) } - Err(e) => { + Err(err) => { // Update invalid transactions metric self.metrics.invalid_transactions.increment(1); - match e { + match err { InsertErr::Underpriced { existing, transaction: _ } => { Err(PoolError::ReplacementUnderpriced(existing)) } @@ -420,6 +420,16 @@ impl TxPool { *transaction.hash(), InvalidPoolTransactionError::ExceedsGasLimit(block_gas_limit, tx_gas_limit), )), + InsertErr::BlobTxHasNonceGap { transaction } => { + Err(PoolError::InvalidTransaction( + *transaction.hash(), + Eip4844PoolTransactionError::Eip4844NonceGap.into(), + )) + } + InsertErr::Overdraft { transaction } => Err(PoolError::InvalidTransaction( + *transaction.hash(), + InvalidPoolTransactionError::Overdraft, + )), } } } @@ -1065,6 +1075,39 @@ impl AllTransactions { Ok(transaction) } + /// Enforces additional constraints for blob transactions before attempting to insert: + /// - new blob transactions must not have any nonce gaps + /// - blob transactions cannot go into overdraft + fn ensure_valid_blob_transaction( + &self, + transaction: ValidPoolTransaction, + on_chain_balance: U256, + ancestor: Option, + ) -> Result, InsertErr> { + if let Some(ancestor) = ancestor { + let Some(tx) = self.txs.get(&ancestor) else { + // ancestor tx is missing, so we can't insert the new blob + return Err(InsertErr::BlobTxHasNonceGap { transaction: Arc::new(transaction) }) + }; + if tx.state.has_nonce_gap() { + // the ancestor transaction already has a nonce gap, so we can't insert the new + // blob + return Err(InsertErr::BlobTxHasNonceGap { transaction: Arc::new(transaction) }) + } + + // check if the new blob would go into overdraft + if tx.next_cumulative_cost() + transaction.cost() > on_chain_balance { + // the transaction would go into overdraft + return Err(InsertErr::Overdraft { transaction: Arc::new(transaction) }) + } + } else if transaction.cost() > on_chain_balance { + // the transaction would go into overdraft + return Err(InsertErr::Overdraft { transaction: Arc::new(transaction) }) + } + + Ok(transaction) + } + /// Returns true if the replacement candidate is underpriced and can't replace the existing /// transaction. #[inline] @@ -1122,6 +1165,10 @@ impl AllTransactions { /// These can include: /// - closing nonce gaps of descendant transactions /// - enough balance updates + /// + /// Note: For EIP-4844 blob transactions additional constraints are enforced: + /// - new blob transactions must not have any nonce gaps + /// - blob transactions cannot go into overdraft pub(crate) fn insert_tx( &mut self, transaction: ValidPoolTransaction, @@ -1130,18 +1177,29 @@ impl AllTransactions { ) -> InsertResult { assert!(on_chain_nonce <= transaction.nonce(), "Invalid transaction"); - let transaction = Arc::new(self.ensure_valid(transaction)?); + let mut transaction = self.ensure_valid(transaction)?; + let inserted_tx_id = *transaction.id(); let mut state = TxState::default(); let mut cumulative_cost = U256::ZERO; let mut updates = Vec::new(); + // identifier of the ancestor transaction, will be None if the transaction is the next tx of + // the sender let ancestor = TransactionId::ancestor( transaction.transaction.nonce(), on_chain_nonce, inserted_tx_id.sender, ); + // before attempting to insert a blob transaction, we need to ensure that additional + // constraints are met + if transaction.is_eip4844() { + transaction = + self.ensure_valid_blob_transaction(transaction, on_chain_balance, ancestor)?; + } + let transaction = Arc::new(transaction); + // If there's no ancestor tx then this is the next transaction. if ancestor.is_none() { state.insert(TxState::NO_NONCE_GAPS); @@ -1341,6 +1399,11 @@ pub(crate) enum InsertErr { transaction: Arc>, existing: TxHash, }, + /// Attempted to insert a blob transaction with a nonce gap + BlobTxHasNonceGap { transaction: Arc> }, + /// Attempted to insert a transaction that would overdraft the sender's balance at the time of + /// insertion. + Overdraft { transaction: Arc> }, /// The transactions feeCap is lower than the chain's minimum fee requirement. /// /// See also [`MIN_PROTOCOL_BASE_FEE`] diff --git a/crates/transaction-pool/src/validate/mod.rs b/crates/transaction-pool/src/validate/mod.rs index d99100961a..1a302d14e6 100644 --- a/crates/transaction-pool/src/validate/mod.rs +++ b/crates/transaction-pool/src/validate/mod.rs @@ -285,6 +285,12 @@ impl ValidPoolTransaction { self.origin.is_local() } + /// Whether the transaction is an EIP-4844 blob transaction. + #[inline] + pub fn is_eip4844(&self) -> bool { + self.transaction.is_eip4844() + } + /// The heap allocated size of this transaction. pub(crate) fn size(&self) -> usize { self.transaction.size()