fix(tx-mgr): report peers that send known bad transactions (#7400)

This commit is contained in:
Emilia Hane
2024-03-31 14:53:51 +02:00
committed by GitHub
parent 283d51c2f1
commit 0c363ea010
2 changed files with 44 additions and 22 deletions

View File

@ -118,7 +118,9 @@ pub struct TransactionsManagerMetrics {
/* ================ POOL IMPORTS ================ */
/// Number of transactions about to be imported into the pool.
pub(crate) pending_pool_imports: Gauge,
/// Total number of bad imports.
/// Total number of bad imports, imports that fail because the transaction is badly formed
/// (i.e. have no chance of passing validation, unlike imports that fail due to e.g. nonce
/// gaps).
pub(crate) bad_imports: Counter,
/// Number of inflight requests at which the
/// [`TransactionPool`](reth_transaction_pool::TransactionPool) is considered to be at

View File

@ -31,8 +31,9 @@ use reth_primitives::{
B256,
};
use reth_transaction_pool::{
error::PoolResult, GetPooledTransactionLimit, PoolTransaction, PropagateKind,
PropagatedTransactions, TransactionPool, ValidPoolTransaction,
error::{PoolError, PoolResult},
GetPooledTransactionLimit, PoolTransaction, PropagateKind, PropagatedTransactions,
TransactionPool, ValidPoolTransaction,
};
use std::{
collections::{hash_map::Entry, HashMap, HashSet},
@ -1013,9 +1014,9 @@ where
peer_id=format!("{peer_id:#}"),
hash=%tx.hash(),
client_version=%peer.client_version,
"received an invalid transaction from peer"
"received a known bad transaction from peer"
);
self.metrics.bad_imports.increment(1);
has_bad_transactions = true;
}
}
}
@ -1079,18 +1080,7 @@ where
self.on_good_import(hash);
}
Err(err) => {
// If we're _currently_ syncing and the transaction is bad we
// ignore it, otherwise we penalize the peer that sent the bad
// transaction with the assumption that the peer should have
// known that this transaction is bad. (e.g. consensus rules).
// Note: nonce gaps are not considered bad transactions.
//
if err.is_bad_transaction() && !self.network.is_syncing() {
debug!(target: "net::tx", %err, "bad pool transaction import");
self.on_bad_import(err.hash);
continue
}
self.on_good_import(err.hash);
self.on_bad_import(err);
}
}
}
@ -1157,15 +1147,45 @@ where
self.transactions_by_peers.remove(&hash);
}
/// Penalize the peers that sent the bad transaction and cache it to avoid fetching or
/// importing it again.
fn on_bad_import(&mut self, hash: TxHash) {
if let Some(peers) = self.transactions_by_peers.remove(&hash) {
/// Penalize the peers that intentionally sent the bad transaction, and cache it to avoid
/// fetching or importing it again.
///
/// Errors that count as bad transactions are:
///
/// - intrinsic gas too low
/// - exceeds gas limit
/// - gas uint overflow
/// - exceeds max init code size
/// - oversized data
/// - signer account has bytecode
/// - chain id mismatch
/// - old legacy chain id
/// - tx type not supported
///
/// (and additionally for blobs txns...)
///
/// - no blobs
/// - too many blobs
/// - invalid kzg proof
/// - kzg error
/// - not blob transaction (tx type mismatch)
/// - wrong versioned kzg commitment hash
fn on_bad_import(&mut self, err: PoolError) {
let peers = self.transactions_by_peers.remove(&err.hash);
// if we're _currently_ syncing, we ignore a bad transaction
if !err.is_bad_transaction() || self.network.is_syncing() {
return
}
// otherwise we penalize the peer that sent the bad transaction, with the assumption that
// the peer should have known that this transaction is bad (e.g. violating consensus rules)
if let Some(peers) = peers {
for peer_id in peers {
self.report_peer_bad_transactions(peer_id);
}
}
self.bad_imports.insert(hash);
self.metrics.bad_imports.increment(1);
self.bad_imports.insert(err.hash);
}
/// Returns `true` if [`TransactionsManager`] has capacity to request pending hashes. Returns