diff options
| -rw-r--r-- | doc/release-notes-19988.md | 9 | ||||
| -rw-r--r-- | src/net_permissions.cpp | 2 | ||||
| -rw-r--r-- | src/net_permissions.h | 1 | ||||
| -rw-r--r-- | src/net_processing.cpp | 14 | ||||
| -rwxr-xr-x | test/functional/p2p_tx_download.py | 18 |
5 files changed, 38 insertions, 6 deletions
diff --git a/doc/release-notes-19988.md b/doc/release-notes-19988.md new file mode 100644 index 0000000000..ef26eb3032 --- /dev/null +++ b/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 `-whitelist=relay@127.0.0.1`. diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp index 53648deb40..d40fdfb113 100644 --- a/src/net_permissions.cpp +++ b/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)" diff --git a/src/net_permissions.h b/src/net_permissions.h index 5b68f635a7..bba0ea1695 100644 --- a/src/net_permissions.h +++ b/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 diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 163c9c1f72..ee50bc1805 100644 --- a/src/net_processing.cpp +++ b/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); } diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py index a3f5f88922..16d9302db8 100755 --- a/test/functional/p2p_tx_download.py +++ b/test/functional/p2p_tx_download.py @@ -46,6 +46,7 @@ INBOUND_PEER_TX_DELAY = 2 # seconds 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 @@ class TxDownloadTest(BitcoinTestFramework): 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=['-whitelist=relay@127.0.0.1']) + 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 @@ class TxDownloadTest(BitcoinTestFramework): 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 |