← Watch feed
test: move abortprivatebroadcast test at the end
Public commit record
90/100 · Strong
What the developer wrote
Authored by Vasil Dimov
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})
```
✓ Descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides detailed explanatory context✓ Mentions testing or verification
What changed, and why it matters
This commit only moves two test blocks to the end of a single functional test file. It fixes a flaky test failure caused by mock time advancing and prematurely disconnecting private broadcast connections. There is no change to production code, no security fix, and no vulnerability.
Recommended action
No security action required. This is a test-only reliability improvement.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch relocates the abortprivatebroadcast RPC tests in test/functional/p2p_private_broadcast.py to the end of the test sequence. The sendrawtransaction call in those tests opens up to three private broadcast connections whose CNode::m_connected timestamp is set early. Later in the test, mock time is advanced by 20 minutes, which triggers PRIVATE_BROADCAST_MAX_CONNECTION_LIFETIME (180 seconds) on those still-opening connections and causes an assertion failure. Moving the abort tests after all mock-time advancement avoids the race. No C++ or consensus code is modified.
Changed components
test/functional/p2p_private_broadcast.pyInspect captured patch +19 / −19
diff --git a/test/functional/p2p_private_broadcast.py b/test/functional/p2p_private_broadcast.py
index f43863ee..9a1a7991 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.
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.