Reduce MAX_PEER_TX_ANNOUNCEMENTS for non-PF_RELAY peers

Maintaining up to 100000 INVs per peer is excessive, as that is far more
than fits in a typical mempool.

Also disable the "overload" penalty for PF_RELAY peers.

Author: sipa

doc/release-notes-19988.md

@@ -0,0 +1,9 @@
+P2P changes
+-----------
+
+The size of the set of transactions that peers have announced and we consider
+for requests has been reduced from 100000 to 5000 (per peer), and further
+announcements will be ignored when that limit is reached. If you need to
+dump (very) large batches of transactions, exceptions can be made for trusted
+peers using the "relay" network permission. For localhost for example it can
+be enabled using the command line option `[email protected]`.

src/net_permissions.cpp

@@ -12,7 +12,7 @@ const std::vector<std::string> NET_PERMISSIONS_DOC{
     "bloomfilter (allow requesting BIP37 filtered blocks and transactions)",
     "noban (do not ban for misbehavior; implies download)",
     "forcerelay (relay transactions that are already in the mempool; implies relay)",
-    "relay (relay even in -blocksonly mode)",
+    "relay (relay even in -blocksonly mode, and unlimited transaction announcements)",
     "mempool (allow requesting BIP35 mempool contents)",
     "download (allow getheaders during IBD, no disconnect after maxuploadtarget limit)",
     "addr (responses to GETADDR avoid hitting the cache and contain random records with the most up-to-date info)"

src/net_permissions.h

@@ -19,6 +19,7 @@ enum NetPermissionFlags {
     // Can query bloomfilter even if -peerbloomfilters is false
     PF_BLOOMFILTER = (1U << 1),
     // Relay and accept transactions from this peer, even if -blocksonly is true
+    // This peer is also not subject to limits on how many transaction INVs are tracked
     PF_RELAY = (1U << 3),
     // Always relay transactions from this peer, even if already in mempool
     // Keep parameter interaction: forcerelay implies relay

src/net_processing.cpp

@@ -75,8 +75,11 @@ static const unsigned int MAX_INV_SZ = 50000;
 /** Maximum number of in-flight transaction requests from a peer. It is not a hard limit, but the threshold at which
  *  point the OVERLOADED_PEER_TX_DELAY kicks in. */
 static constexpr int32_t MAX_PEER_TX_REQUEST_IN_FLIGHT = 100;
-/** Maximum number of announced transactions from a peer */
-static constexpr int32_t MAX_PEER_TX_ANNOUNCEMENTS = 2 * MAX_INV_SZ;
+/** Maximum number of transactions to consider for requesting, per peer. It provides a reasonable DoS limit to
+ *  per-peer memory usage spent on announcements, while covering peers continuously sending INVs at the maximum
+ *  rate (by our own policy, see INVENTORY_BROADCAST_PER_SECOND) for several minutes, while not receiving
+ *  the actual transaction (from any peer) in response to requests for them. */
+static constexpr int32_t MAX_PEER_TX_ANNOUNCEMENTS = 5000;
 /** How long to delay requesting transactions via txids, if we have wtxid-relaying peers */
 static constexpr auto TXID_RELAY_DELAY = std::chrono::seconds{2};
 /** How long to delay requesting transactions from non-preferred peers */
@@ -754,7 +757,7 @@ void PeerManager::AddTxAnnouncement(const CNode& node, const GenTxid& gtxid, std
 {
     AssertLockHeld(::cs_main); // For m_txrequest
     NodeId nodeid = node.GetId();
-    if (m_txrequest.Count(nodeid) >= MAX_PEER_TX_ANNOUNCEMENTS) {
+    if (!node.HasPermission(PF_RELAY) && m_txrequest.Count(nodeid) >= MAX_PEER_TX_ANNOUNCEMENTS) {
         // Too many queued announcements from this peer
         return;
     }
@@ -766,12 +769,13 @@ void PeerManager::AddTxAnnouncement(const CNode& node, const GenTxid& gtxid, std
     //   - NONPREF_PEER_TX_DELAY for announcements from non-preferred connections
     //   - TXID_RELAY_DELAY for announcements from txid peers while wtxid peers are available
     //   - OVERLOADED_PEER_TX_DELAY for announcements from peers which have at least
-    //     MAX_PEER_TX_REQUEST_IN_FLIGHT requests in flight.
+    //     MAX_PEER_TX_REQUEST_IN_FLIGHT requests in flight (and don't have PF_RELAY).
     auto delay = std::chrono::microseconds{0};
     const bool preferred = state->fPreferredDownload;
     if (!preferred) delay += NONPREF_PEER_TX_DELAY;
     if (!state->m_wtxid_relay && g_wtxid_relay_peers > 0) delay += TXID_RELAY_DELAY;
-    const bool overloaded = m_txrequest.CountInFlight(nodeid) >= MAX_PEER_TX_REQUEST_IN_FLIGHT;
+    const bool overloaded = !node.HasPermission(PF_RELAY) &&
+        m_txrequest.CountInFlight(nodeid) >= MAX_PEER_TX_REQUEST_IN_FLIGHT;
     if (overloaded) delay += OVERLOADED_PEER_TX_DELAY;
     m_txrequest.ReceivedInv(nodeid, gtxid, preferred, current_time + delay);
 }

test/functional/p2p_tx_download.py

@@ -46,6 +46,7 @@ def on_getdata(self, message):
 TXID_RELAY_DELAY = 2 # seconds
 OVERLOADED_PEER_DELAY = 2 # seconds
 MAX_GETDATA_IN_FLIGHT = 100
+MAX_PEER_TX_ANNOUNCEMENTS = 5000
 
 # Python test constants
 NUM_INBOUND = 10
@@ -215,6 +216,22 @@ def test_preferred_inv(self):
         with p2p_lock:
             assert_equal(peer.tx_getdata_count, 1)
 
+    def test_large_inv_batch(self):
+        self.log.info('Test how large inv batches are handled with relay permission')
+        self.restart_node(0, extra_args=['[email protected]'])
+        peer = self.nodes[0].add_p2p_connection(TestP2PConn())
+        peer.send_message(msg_inv([CInv(t=MSG_WTX, h=wtxid) for wtxid in range(MAX_PEER_TX_ANNOUNCEMENTS + 1)]))
+        peer.wait_until(lambda: peer.tx_getdata_count == MAX_PEER_TX_ANNOUNCEMENTS + 1)
+
+        self.log.info('Test how large inv batches are handled without relay permission')
+        self.restart_node(0)
+        peer = self.nodes[0].add_p2p_connection(TestP2PConn())
+        peer.send_message(msg_inv([CInv(t=MSG_WTX, h=wtxid) for wtxid in range(MAX_PEER_TX_ANNOUNCEMENTS + 1)]))
+        peer.wait_until(lambda: peer.tx_getdata_count == MAX_PEER_TX_ANNOUNCEMENTS)
+        peer.sync_with_ping()
+        with p2p_lock:
+            assert_equal(peer.tx_getdata_count, MAX_PEER_TX_ANNOUNCEMENTS)
+
     def test_spurious_notfound(self):
         self.log.info('Check that spurious notfound is ignored')
         self.nodes[0].p2ps[0].send_message(msg_notfound(vec=[CInv(MSG_TX, 1)]))
@@ -225,6 +242,7 @@ def run_test(self):
         self.test_disconnect_fallback()
         self.test_notfound_fallback()
         self.test_preferred_inv()
+        self.test_large_inv_batch()
         self.test_spurious_notfound()
 
         # Run each test against new bitcoind instances, as setting mocktimes has long-term effects on when