feat: add new already seen transactions reputation variant (#2468)

crates/net/network-api/src/reputation.rs

@@ -12,6 +12,16 @@ pub enum ReputationChangeKind {
     BadBlock,
     /// Peer sent a bad transaction messages. E.g. Transactions which weren't recoverable.
     BadTransactions,
+    /// Peer sent a message that included a hash or transaction that we already received from the
+    /// peer.
+    ///
+    /// According to the [Eth spec](https://github.com/ethereum/devp2p/blob/master/caps/eth.md):
+    ///
+    /// > A node should never send a transaction back to a peer that it can determine already knows
+    /// > of it (either because it was previously sent or because it was informed from this peer
+    /// > originally). This is usually achieved by remembering a set of transaction hashes recently
+    /// > relayed by the peer.
+    AlreadySeenTransaction,
     /// Peer failed to respond in time.
     Timeout,
     /// Peer does not adhere to network protocol rules.

crates/net/network/src/peers/reputation.rs

@@ -23,6 +23,13 @@ const TIMEOUT_REPUTATION_CHANGE: i32 = 4 * REPUTATION_UNIT;
 /// The reputation change to apply to a peer that sent a bad message.
 const BAD_MESSAGE_REPUTATION_CHANGE: i32 = 16 * REPUTATION_UNIT;
 
+/// The reputation change applies to a peer that has sent a transaction (full or hash) that we
+/// already know about and have already previously received from that peer.
+///
+/// Note: We weight this very low because it is generally not a problem to get a transaction that we
+/// already know about, so only spammers are affected.
+const ALREADY_SEEN_TRANSACTION_REPUTATION_CHANGE: i32 = REPUTATION_UNIT / 2;
+
 /// The reputation change to apply to a peer which violates protocol rules: minimal reputation
 const BAD_PROTOCOL_REPUTATION_CHANGE: i32 = i32::MIN;
 
@@ -42,6 +49,8 @@ pub struct ReputationChangeWeights {
     pub bad_block: Reputation,
     /// Weight for [`ReputationChangeKind::BadTransactions`]
     pub bad_transactions: Reputation,
+    /// Weight for [`ReputationChangeKind::AlreadySeenTransaction`]
+    pub already_seen_transactions: Reputation,
     /// Weight for [`ReputationChangeKind::Timeout`]
     pub timeout: Reputation,
     /// Weight for [`ReputationChangeKind::BadProtocol`]
@@ -62,6 +71,7 @@ impl ReputationChangeWeights {
             ReputationChangeKind::BadMessage => self.bad_message.into(),
             ReputationChangeKind::BadBlock => self.bad_block.into(),
             ReputationChangeKind::BadTransactions => self.bad_transactions.into(),
+            ReputationChangeKind::AlreadySeenTransaction => self.already_seen_transactions.into(),
             ReputationChangeKind::Timeout => self.timeout.into(),
             ReputationChangeKind::BadProtocol => self.bad_protocol.into(),
             ReputationChangeKind::FailedToConnect => self.failed_to_connect.into(),
@@ -77,6 +87,7 @@ impl Default for ReputationChangeWeights {
         Self {
             bad_block: BAD_MESSAGE_REPUTATION_CHANGE,
             bad_transactions: BAD_MESSAGE_REPUTATION_CHANGE,
+            already_seen_transactions: ALREADY_SEEN_TRANSACTION_REPUTATION_CHANGE,
             bad_message: BAD_MESSAGE_REPUTATION_CHANGE,
             timeout: TIMEOUT_REPUTATION_CHANGE,
             bad_protocol: BAD_PROTOCOL_REPUTATION_CHANGE,

crates/net/network/src/transactions.rs

@@ -319,8 +319,7 @@ where
         }
 
         if num_already_seen > 0 {
-            self.metrics.messages_with_already_seen_hashes.increment(1);
-            self.report_bad_message(peer_id);
+            self.report_already_seen(peer_id);
         }
     }
 
@@ -461,7 +460,7 @@ where
         }
 
         if has_bad_transactions || num_already_seen > 0 {
-            self.report_bad_message(peer_id);
+            self.report_already_seen(peer_id);
         }
     }
 
@@ -471,6 +470,11 @@ where
         self.network.reputation_change(peer_id, ReputationChangeKind::BadTransactions);
     }
 
+    fn report_already_seen(&self, peer_id: PeerId) {
+        trace!(target: "net::tx", ?peer_id, "Penalizing peer for already seen transaction");
+        self.network.reputation_change(peer_id, ReputationChangeKind::AlreadySeenTransaction);
+    }
+
     /// Clear the transaction
     fn on_good_import(&mut self, hash: TxHash) {
         self.transactions_by_peers.remove(&hash);