From 1278a5970d5ada0979052a5bad899e896b8ab40b Mon Sep 17 00:00:00 2001 From: Gregory Sanders Date: Tue, 14 Jul 2026 13:43:48 -0400 Subject: [PATCH] net_processing: process unique tx INVs only There is no reason we should process conflicting advertisements for transactions, as they cannot be both accepted into our mempool. Avoid processing these and doing spurious work. --- src/net_processing.cpp | 9 ++++ test/functional/p2p_tx_download.py | 68 ++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3524dd2fa10..501b14eaee4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -57,6 +57,7 @@ #include #include #include +#include #include #include #include @@ -85,6 +86,7 @@ #include #include #include +#include #include using kernel::ChainstateRole; @@ -842,6 +844,8 @@ private: FastRandomContext m_rng GUARDED_BY(NetEventsInterface::g_msgproc_mutex); + /** Copied into short-lived tx INV deduplication sets to avoid generating salts per message. */ + const SaltedUint256Hasher m_txhash_hasher; FeeFilterRounder m_fee_filter_rounder GUARDED_BY(NetEventsInterface::g_msgproc_mutex); const CChainParams& m_chainparams; @@ -4337,6 +4341,8 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string } const bool reject_tx_invs{RejectIncomingTxs(pfrom)}; + std::unordered_set seen_txids{0, m_txhash_hasher}; + std::unordered_set seen_wtxids{0, m_txhash_hasher}; LOCK2(cs_main, m_tx_download_mutex); @@ -4375,6 +4381,9 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string pfrom.fDisconnect = true; return; } + // MSG_WITNESS_TX is treated as a txid, despite only being specified for getdata. + auto& seen_hashes{inv.IsMsgWtx() ? seen_wtxids : seen_txids}; + if (!seen_hashes.insert(inv.hash).second) continue; const GenTxid gtxid = ToGenTxid(inv); AddKnownTx(peer, inv.hash); diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py index b910bda39b2..aa43cc18e23 100755 --- a/test/functional/p2p_tx_download.py +++ b/test/functional/p2p_tx_download.py @@ -16,6 +16,7 @@ from test_framework.messages import ( CInv, MSG_TX, MSG_TYPE_MASK, + MSG_WITNESS_TX, MSG_WTX, msg_inv, msg_notfound, @@ -324,6 +325,72 @@ class TxDownloadTest(BitcoinTestFramework): peer.wait_until(lambda: peer.tx_getdata_count == MAX_PEER_TX_ANNOUNCEMENTS) peer.sync_with_ping() + def test_duplicate_tx_inv(self): + self.log.info('Check that duplicate transaction identifiers in one inv message are processed once') + node = self.nodes[0] + node.logging(include=['net']) + + def send_invs_and_read_log(peer, invs): + log_start = node.debug_log_size(encoding='utf-8') + peer.send_and_ping(msg_inv(invs)) + with open(node.debug_log_path, encoding='utf-8', errors='replace') as debug_log: + debug_log.seek(log_start) + return debug_log.read() + + for wtxidrelay, inv_type, inv_name, mismatched_type, mismatched_name, hash_a, hash_b in [ + (False, MSG_TX, 'tx', MSG_WTX, 'wtx', 0xaabbcc, 0xddeeff), + (True, MSG_WTX, 'wtx', MSG_TX, 'tx', 0x112233, 0x445566), + ]: + peer = node.add_p2p_connection(TestP2PConn(wtxidrelay=wtxidrelay)) + inv_a_log = f"got inv: {inv_name} {hash_a:064x}" + inv_b_log = f"got inv: {inv_name} {hash_b:064x}" + mismatched_inv_log = f"got inv: {mismatched_name} {hash_a:064x}" + + log = send_invs_and_read_log(peer, [ + CInv(t=mismatched_type, h=hash_a), + CInv(t=inv_type, h=hash_a), + CInv(t=inv_type, h=hash_b), + CInv(t=inv_type, h=hash_a), + CInv(t=inv_type, h=hash_b), + ]) + assert_equal(log.count(inv_a_log), 1) + assert_equal(log.count(inv_b_log), 1) + assert_equal(log.count(mismatched_inv_log), 0) + + # The duplicate filter is scoped to a single INV message. + log = send_invs_and_read_log(peer, [ + CInv(t=inv_type, h=hash_a), + CInv(t=inv_type, h=hash_a), + ]) + assert_equal(log.count(inv_a_log), 1) + assert_equal(log.count(inv_b_log), 0) + + self.log.info('Check that MSG_TX and MSG_WITNESS_TX are deduplicated as txids') + peer = node.add_p2p_connection(TestP2PConn(wtxidrelay=False)) + for first_type, first_name, second_type, second_name, hash_a in [ + (MSG_TX, 'tx', MSG_WITNESS_TX, 'witness-tx', 0x667788), + (MSG_WITNESS_TX, 'witness-tx', MSG_TX, 'tx', 0x778899), + ]: + log = send_invs_and_read_log(peer, [ + CInv(t=first_type, h=hash_a), + CInv(t=second_type, h=hash_a), + ]) + assert_equal(log.count(f"got inv: {first_name} {hash_a:064x}"), 1) + assert_equal(log.count(f"got inv: {second_name} {hash_a:064x}"), 0) + + self.log.info('Check that txids and wtxids are deduplicated separately') + peer = node.add_p2p_connection(TestP2PConn(wtxidrelay=True)) + for first_type, second_type, hash_a in [ + (MSG_WITNESS_TX, MSG_WTX, 0x8899aa), + (MSG_WTX, MSG_WITNESS_TX, 0x99aabb), + ]: + log = send_invs_and_read_log(peer, [ + CInv(t=first_type, h=hash_a), + CInv(t=second_type, h=hash_a), + ]) + assert_equal(log.count(f"got inv: witness-tx {hash_a:064x}"), 1) + assert_equal(log.count(f"got inv: wtx {hash_a:064x}"), 1) + def test_spurious_notfound(self): self.log.info('Check that spurious notfound is ignored') self.nodes[0].p2ps[0].send_without_ping(msg_notfound(vec=[CInv(MSG_TX, 1)])) @@ -395,6 +462,7 @@ class TxDownloadTest(BitcoinTestFramework): self.test_txid_inv_delay() self.test_txid_inv_delay(True) self.test_large_inv_batch() + self.test_duplicate_tx_inv() self.test_spurious_notfound() # Run each test against new bitcoind instances, as setting mocktimes has long-term effects on when