diff --git a/crates/net/network/src/transactions.rs b/crates/net/network/src/transactions.rs index 0dbf7a10af..dd8548bee4 100644 --- a/crates/net/network/src/transactions.rs +++ b/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()); + } } } } diff --git a/crates/rpc/rpc/src/eth/error.rs b/crates/rpc/rpc/src/eth/error.rs index c1a81d7497..fbaa916982 100644 --- a/crates/rpc/rpc/src/eth/error.rs +++ b/crates/rpc/rpc/src/eth/error.rs @@ -341,7 +341,7 @@ impl From 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(), diff --git a/crates/transaction-pool/src/error.rs b/crates/transaction-pool/src/error.rs index 1a9a1e6b04..82b5832ebe 100644 --- a/crates/transaction-pool/src/error.rs +++ b/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 diff --git a/crates/transaction-pool/src/pool/txpool.rs b/crates/transaction-pool/src/pool/txpool.rs index 69c190c8c0..ddaafb7699 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -247,9 +247,9 @@ impl TxPool { 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 AllTransactions { // 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 { /// The transactions feeCap is lower than the chain's minimum fee requirement. /// /// See also [`MIN_PROTOCOL_BASE_FEE`] - ProtocolFeeCapTooLow { transaction: Arc>, fee_cap: u128 }, + FeeCapBelowMinimumProtocolFeeCap { transaction: Arc>, fee_cap: u128 }, /// Sender currently exceeds the configured limit for max account slots. /// /// The sender can be considered a spammer at this point.