From 3710566305e569bed8458809f0dedc83420b7de2 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Sat, 21 Feb 2026 16:02:34 +0100 Subject: [PATCH 1/2] test: move abortprivatebroadcast test at the end The piece of `p2p_private_broadcast.py` which tests the correctness of `abortprivatebroadcast` issues a new `sendrawtransaction` call. That call schedules up to 3 new connections: peer=13, peer=14 and possibly peer=15 before it gets aborted. These up to 3 in-the-process-of-opening private broadcast connections have `CNode::m_connected` set early - when the `CNode` object is created. Later in the test the mock time is advanced by 20 minutes and those "old" connections pick a transaction for rebroadcast but that triggers `PRIVATE_BROADCAST_MAX_CONNECTION_LIFETIME` immediately: ``` 2026-02-21T13:28:14.209766Z [privbcast] [net.cpp:4006] [CNode] [net] Added connection peer=20 2026-02-21T13:28:14.309792Z (mocktime: 2026-02-21T13:48:14Z) [msghand] [net.cpp:4074] [PushMessage] [net] sending inv (37 bytes) peer=20 2026-02-21T13:28:14.309801Z (mocktime: 2026-02-21T13:48:14Z) [msghand] [net_processing.cpp:5745] [SendMessages] [privatebroadcast] Disconnecting: did not complete the transaction send within 180 seconds, peer=20 ``` This prematurely stops the private broadcast connection and results in a failure like: ``` AssertionError: ... not({} == {'ping': 1, 'tx': 1}) ``` --- test/functional/p2p_private_broadcast.py | 38 ++++++++++++------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/test/functional/p2p_private_broadcast.py b/test/functional/p2p_private_broadcast.py index f43863eeb5f..9a1a7991edf 100755 --- a/test/functional/p2p_private_broadcast.py +++ b/test/functional/p2p_private_broadcast.py @@ -382,25 +382,6 @@ class P2PPrivateBroadcast(BitcoinTestFramework): pending = [t for t in pbinfo["transactions"] if t["txid"] == txs[0]["txid"] and t["wtxid"] == txs[0]["wtxid"]] assert_equal(len(pending), 0) - self.log.info("Checking abortprivatebroadcast removes a pending private-broadcast transaction") - tx_abort = wallet.create_self_transfer() - tx_originator.sendrawtransaction(hexstring=tx_abort["hex"], maxfeerate=0.1) - assert any(t["wtxid"] == tx_abort["wtxid"] for t in tx_originator.getprivatebroadcastinfo()["transactions"]) - abort_res = tx_originator.abortprivatebroadcast(tx_abort["txid"]) - assert_equal(len(abort_res["removed_transactions"]), 1) - assert_equal(abort_res["removed_transactions"][0]["txid"], tx_abort["txid"]) - assert_equal(abort_res["removed_transactions"][0]["wtxid"], tx_abort["wtxid"]) - assert_equal(abort_res["removed_transactions"][0]["hex"].lower(), tx_abort["hex"].lower()) - assert all(t["wtxid"] != tx_abort["wtxid"] for t in tx_originator.getprivatebroadcastinfo()["transactions"]) - - self.log.info("Checking abortprivatebroadcast fails for non-existent transaction") - assert_raises_rpc_error( - -5, - "Transaction not in private broadcast queue", - tx_originator.abortprivatebroadcast, - "0" * 64, - ) - self.log.info("Sending a transaction that is already in the mempool") skip_destinations = len(self.destinations) tx_originator.sendrawtransaction(hexstring=txs[0]["hex"], maxfeerate=0) @@ -453,6 +434,25 @@ class P2PPrivateBroadcast(BitcoinTestFramework): tx_originator.sendrawtransaction(hexstring=sibling2.serialize_with_witness().hex(), maxfeerate=0.1) self.log.info(" - sent sibling2: ok") + self.log.info("Checking abortprivatebroadcast removes a pending private-broadcast transaction") + tx_abort = wallet.create_self_transfer() + tx_originator.sendrawtransaction(hexstring=tx_abort["hex"], maxfeerate=0.1) + assert any(t["wtxid"] == tx_abort["wtxid"] for t in tx_originator.getprivatebroadcastinfo()["transactions"]) + abort_res = tx_originator.abortprivatebroadcast(tx_abort["txid"]) + assert_equal(len(abort_res["removed_transactions"]), 1) + assert_equal(abort_res["removed_transactions"][0]["txid"], tx_abort["txid"]) + assert_equal(abort_res["removed_transactions"][0]["wtxid"], tx_abort["wtxid"]) + assert_equal(abort_res["removed_transactions"][0]["hex"].lower(), tx_abort["hex"].lower()) + assert all(t["wtxid"] != tx_abort["wtxid"] for t in tx_originator.getprivatebroadcastinfo()["transactions"]) + + self.log.info("Checking abortprivatebroadcast fails for non-existent transaction") + assert_raises_rpc_error( + -5, + "Transaction not in private broadcast queue", + tx_originator.abortprivatebroadcast, + "0" * 64, + ) + # Stop the SOCKS5 proxy server to avoid it being upset by the bitcoin # node disconnecting in the middle of the SOCKS5 handshake when we # restart below. From c462e54f9df431434e6480d8293060645468d3ab Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Sat, 21 Feb 2026 17:13:11 +0100 Subject: [PATCH 2/2] test: don't always assert NUM_PRIVATE_BROADCAST_PER_TX broadcasts In `p2p_private_broadcast.py` in the function `check_broadcasts()` we should assert that the broadcast was done to `broadcasts_to_expect` peers, not to `NUM_PRIVATE_BROADCAST_PER_TX`. This is because in the "Basic" test we check the first broadcast manually because it is done to `nodes[1]` and then check the other two by `check_broadcasts(..., NUM_PRIVATE_BROADCAST_PER_TX - 1, ...)`. The first broadcast might not have fully concluded by the time we call `check_broadcasts()` to check the remaining 2. Demanding always `NUM_PRIVATE_BROADCAST_PER_TX` can lead to: ``` Traceback (most recent call last): File "/home/vd/gh/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 142, in main self.run_test() ~~~~~~~~~~~~~^^ File "/tmp/build/clang22/test/functional/p2p_private_broadcast.py", line 347, in run_test self.check_broadcasts("Basic", txs[0], NUM_PRIVATE_BROADCAST_PER_TX - 1, NUM_INITIAL_CONNECTIONS + 1) ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/tmp/build/clang22/test/functional/p2p_private_broadcast.py", line 313, in check_broadcasts assert_greater_than_or_equal(sum(1 for p in peers if "received" in p), NUM_PRIVATE_BROADCAST_PER_TX) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/vd/gh/bitcoin/bitcoin/test/functional/test_framework/util.py", line 94, in assert_greater_than_or_equal raise AssertionError("%s < %s" % (str(thing1), str(thing2))) AssertionError: 2 < 3 ``` --- test/functional/p2p_private_broadcast.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/p2p_private_broadcast.py b/test/functional/p2p_private_broadcast.py index 9a1a7991edf..b81b5aa483d 100755 --- a/test/functional/p2p_private_broadcast.py +++ b/test/functional/p2p_private_broadcast.py @@ -310,7 +310,7 @@ class P2PPrivateBroadcast(BitcoinTestFramework): peers = pending[0]["peers"] assert len(peers) >= NUM_PRIVATE_BROADCAST_PER_TX assert all("address" in p and "sent" in p for p in peers) - assert_greater_than_or_equal(sum(1 for p in peers if "received" in p), NUM_PRIVATE_BROADCAST_PER_TX) + assert_greater_than_or_equal(sum(1 for p in peers if "received" in p), broadcasts_to_expect) def run_test(self): tx_originator = self.nodes[0]