diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -5,3 +5,13 @@ This release includes the following features and fixes: + +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 `-whitelist=relay@127.0.0.1`. diff --git a/src/net_permissions.h b/src/net_permissions.h --- a/src/net_permissions.h +++ b/src/net_permissions.h @@ -19,6 +19,8 @@ // 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 or // rejected from policy Keep parameter interaction: forcerelay implies relay diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp --- a/src/net_permissions.cpp +++ b/src/net_permissions.cpp @@ -14,7 +14,8 @@ "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)", diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -108,8 +108,15 @@ * 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 from non-preferred peers */ static constexpr auto NONPREF_PEER_TX_DELAY = std::chrono::seconds{2}; /** @@ -921,7 +928,8 @@ // For m_txrequest AssertLockHeld(::cs_main); 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; } @@ -934,7 +942,8 @@ // - NONPREF_PEER_TX_DELAY for announcements from non-preferred // connections // - OVERLOADED_PEER_TX_DELAY for announcements from peers which have at - // least MAX_PEER_TX_REQUEST_IN_FLIGHT requests in flight. + // least 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) { @@ -942,6 +951,7 @@ } const bool overloaded = + !node.HasPermission(PF_RELAY) && m_txrequest.CountInFlight(nodeid) >= MAX_PEER_TX_REQUEST_IN_FLIGHT; if (overloaded) { delay += OVERLOADED_PEER_TX_DELAY; diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py --- a/test/functional/p2p_tx_download.py +++ b/test/functional/p2p_tx_download.py @@ -45,6 +45,7 @@ INBOUND_PEER_TX_DELAY = 2 # seconds OVERLOADED_PEER_DELAY = 2 # seconds MAX_GETDATA_IN_FLIGHT = 100 +MAX_PEER_TX_ANNOUNCEMENTS = 5000 # Python test constants NUM_INBOUND = 10 @@ -260,6 +261,28 @@ with mininode_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=['-whitelist=relay@127.0.0.1']) + peer = self.nodes[0].add_p2p_connection(TestP2PConn()) + peer.send_message(msg_inv([CInv(t=MSG_TX, h=txid) + for txid 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_TX, h=txid) + for txid in range(MAX_PEER_TX_ANNOUNCEMENTS + 1)])) + peer.wait_until(lambda: peer.tx_getdata_count == + MAX_PEER_TX_ANNOUNCEMENTS) + peer.sync_with_ping() + with mininode_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)])) @@ -271,6 +294,7 @@ 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