fix(txpool): differentiate pool error variants (#2008)

crates/net/network/src/transactions.rs

@@ -451,13 +451,16 @@ where
     }
 
     fn report_bad_message(&self, peer_id: PeerId) {
+        trace!(target: "net::tx", ?peer_id, "Penalizing peer for bad transaction");
         self.network.reputation_change(peer_id, ReputationChangeKind::BadTransactions);
     }
 
+    /// Clear the transaction
     fn on_good_import(&mut self, hash: TxHash) {
         self.transactions_by_peers.remove(&hash);
     }
 
+    /// Penalize the peers that sent the bad transaction
     fn on_bad_import(&mut self, hash: TxHash) {
         if let Some(peers) = self.transactions_by_peers.remove(&hash) {
             for peer_id in peers {
@@ -522,7 +525,11 @@ where
                     this.on_good_import(hash);
                 }
                 Err(err) => {
-                    this.on_bad_import(*err.hash());
+                    if err.is_bad_transaction() {
+                        this.on_bad_import(*err.hash());
+                    } else {
+                        this.on_good_import(*err.hash());
+                    }
                 }
             }
         }

crates/rpc/rpc/src/eth/error.rs

@@ -341,7 +341,7 @@ impl From<PoolError> for RpcPoolError {
     fn from(err: PoolError) -> RpcPoolError {
         match err {
             PoolError::ReplacementUnderpriced(_) => RpcPoolError::ReplaceUnderpriced,
-            PoolError::ProtocolFeeCapTooLow(_, _) => RpcPoolError::Underpriced,
+            PoolError::FeeCapBelowMinimumProtocolFeeCap(_, _) => RpcPoolError::Underpriced,
             PoolError::SpammerExceededCapacity(_, _) => RpcPoolError::TxPoolOverflow,
             PoolError::DiscardedOnInsert(_) => RpcPoolError::TxPoolOverflow,
             PoolError::InvalidTransaction(_, err) => err.into(),

crates/transaction-pool/src/error.rs

@@ -11,9 +11,9 @@ pub enum PoolError {
     /// Thrown if a replacement transaction's gas price is below the already imported transaction
     #[error("[{0:?}]: insufficient gas price to replace existing transaction.")]
     ReplacementUnderpriced(TxHash),
-    /// Encountered a transaction that was already added into the poll
+    /// The fee cap of the transaction is below the minimum fee cap determined by the protocol
     #[error("[{0:?}] Transaction feeCap {1} below chain minimum.")]
-    ProtocolFeeCapTooLow(TxHash, u128),
+    FeeCapBelowMinimumProtocolFeeCap(TxHash, u128),
     /// Thrown when the number of unique transactions of a sender exceeded the slot capacity.
     #[error("{0:?} identified as spammer. Transaction {1:?} rejected.")]
     SpammerExceededCapacity(Address, TxHash),
@@ -37,13 +37,56 @@ impl PoolError {
     pub fn hash(&self) -> &TxHash {
         match self {
             PoolError::ReplacementUnderpriced(hash) => hash,
-            PoolError::ProtocolFeeCapTooLow(hash, _) => hash,
+            PoolError::FeeCapBelowMinimumProtocolFeeCap(hash, _) => hash,
             PoolError::SpammerExceededCapacity(_, hash) => hash,
             PoolError::DiscardedOnInsert(hash) => hash,
             PoolError::InvalidTransaction(hash, _) => hash,
             PoolError::Other(hash, _) => hash,
         }
     }
+
+    /// Returns `true` if the error was caused by a transaction that is considered bad in the
+    /// context of the transaction pool.
+    ///
+    /// Not all error variants are caused by the incorrect composition of the transaction (See also
+    /// [InvalidPoolTransactionError]) and can be caused by the current state of the transaction
+    /// pool. For example the transaction pool is already full or the error was caused my an
+    /// internal error, such as database errors.
+    ///
+    /// This function returns true only if the transaction will never make it into the pool because
+    /// its composition is invalid and the original sender should have detected this as well. This
+    /// is used to determine whether the original sender should be penalized for sending an
+    /// erroneous transaction.
+    #[inline]
+    pub fn is_bad_transaction(&self) -> bool {
+        match self {
+            PoolError::ReplacementUnderpriced(_) => {
+                // already imported but not bad
+                false
+            }
+            PoolError::FeeCapBelowMinimumProtocolFeeCap(_, _) => {
+                // fee cap of the tx below the technical minimum determined by the protocol: always
+                // invalid
+                true
+            }
+            PoolError::SpammerExceededCapacity(_, _) => {
+                // spammer detected
+                true
+            }
+            PoolError::DiscardedOnInsert(_) => {
+                // valid tx but dropped due to size constraints
+                false
+            }
+            PoolError::InvalidTransaction(_, _) => {
+                // transaction rejected because it violates constraints
+                true
+            }
+            PoolError::Other(_, _) => {
+                // internal error unrelated to the transaction
+                false
+            }
+        }
+    }
 }
 
 /// Represents errors that can happen when validating transactions for the pool

crates/transaction-pool/src/pool/txpool.rs

@@ -247,9 +247,9 @@ impl<T: TransactionOrdering> TxPool<T> {
                     InsertErr::Underpriced { existing, .. } => {
                         Err(PoolError::ReplacementUnderpriced(existing))
                     }
-                    InsertErr::ProtocolFeeCapTooLow { transaction, fee_cap } => {
-                        Err(PoolError::ProtocolFeeCapTooLow(*transaction.hash(), fee_cap))
-                    }
+                    InsertErr::FeeCapBelowMinimumProtocolFeeCap { transaction, fee_cap } => Err(
+                        PoolError::FeeCapBelowMinimumProtocolFeeCap(*transaction.hash(), fee_cap),
+                    ),
                     InsertErr::ExceededSenderTransactionsCapacity { transaction } => {
                         Err(PoolError::SpammerExceededCapacity(
                             transaction.sender(),
@@ -846,7 +846,7 @@ impl<T: PoolTransaction> AllTransactions<T> {
         // Check dynamic fee
         if let Some(fee_cap) = transaction.max_fee_per_gas() {
             if fee_cap < self.minimal_protocol_basefee {
-                return Err(InsertErr::ProtocolFeeCapTooLow { transaction, fee_cap })
+                return Err(InsertErr::FeeCapBelowMinimumProtocolFeeCap { transaction, fee_cap })
             }
             if fee_cap >= self.pending_basefee {
                 state.insert(TxState::ENOUGH_FEE_CAP_BLOCK);
@@ -1028,7 +1028,7 @@ pub(crate) enum InsertErr<T: PoolTransaction> {
     /// The transactions feeCap is lower than the chain's minimum fee requirement.
     ///
     /// See also [`MIN_PROTOCOL_BASE_FEE`]
-    ProtocolFeeCapTooLow { transaction: Arc<ValidPoolTransaction<T>>, fee_cap: u128 },
+    FeeCapBelowMinimumProtocolFeeCap { transaction: Arc<ValidPoolTransaction<T>>, fee_cap: u128 },
     /// Sender currently exceeds the configured limit for max account slots.
     ///
     /// The sender can be considered a spammer at this point.