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