test: look for the wtxid in p2p_blocksonly #36405

pull fametrano wants to merge 1 commits into bitcoin:master from fametrano:test-blocksonly-wtxid-inv changing 1 files +5 −2

fametrano commented at 8:32 PM on October 1, 2026: none

In blocks_relay_conn_tests, the check that a transaction submitted over RPC is not announced to a block-relay-only peer looks for its txid among the peer's invs. The peer is a P2PTxInvStore, which negotiates wtxidrelay, so an announcement would carry the wtxid. The MiniWallet transaction has a witness, so its txid and wtxid differ, and the check passes whether or not the node announces it.

This checks the wtxid instead. To see the difference, open that connection as outbound-full-relay: the old check still passes, the new one fails. It also adds an outbound-full-relay peer that must receive the wtxid announcement first, so the check cannot pass silently again.

#35920 also edits blocks_relay_conn_tests; the two do not conflict.

Found while porting these tests to https://github.com/btclib-org/bitcoin-node-tests.
DrahtBot added the label Tests on Oct 1, 2026
DrahtBot commented at 8:32 PM on October 1, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36405
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36405.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36405#author-ralph


Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK maflcko, AndreaDiazCorreia

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #36257 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36257.svg"></sub> (qa: assert_equals -> assert_true/assert_false by hodlinator)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
mercie-ux commented at 2:23 PM on October 7, 2026: contributor

Concept Ack
in test/functional/p2p_blocksonly.py:116 in cdb8d399bc outdated
```
 111 | @@ -112,7 +112,8 @@ def blocks_relay_conn_tests(self):
 112 |          self.nodes[0].setmocktime(int(time.time()) + 60)
 113 |  
 114 |          conn.sync_with_ping()
 115 | -        assert int(txid, 16) not in conn.get_invs()
 116 | +        # The peer negotiated wtxidrelay, so an announcement would carry the wtxid
 117 | +        assert int(wtxid, 16) not in conn.get_invs()
```
maflcko commented at 12:33 PM on October 12, 2026:

maybe there should be another peer to check it is received for it? Otherwise the test will likely silently break again in the future?

fametrano commented at 10:31 AM on October 13, 2026:

Done in c765571. I added an outbound-full-relay peer that must receive the wtxid announcement before the block-relay-only peer is checked, so the test fails if the node stops announcing the transaction or announces it under another id.

Type	Reviewers
ACK	maflcko, AndreaDiazCorreia

test: look for the wtxid in p2p_blocksonly

The connection that checks a transaction submitted over RPC is not
announced to a block-relay-only peer uses P2PTxInvStore, which negotiates
wtxidrelay. The node would therefore announce the transaction by its
wtxid, and the assertion looked for the txid, which cannot be in the
store for a transaction carrying a witness: it passed whether or not the
transaction was announced.

Assert on the wtxid instead. With the connection opened as
outbound-full-relay rather than block-relay-only, the old assertion still
passes and the new one fails.

Also open an outbound-full-relay connection and assert that the wtxid
was announced on it. The two assertions differ only by `not`, so if the
announcement or its id changes, the test fails instead of the absence
check passing silently.

22535b4605

in test/functional/p2p_blocksonly.py:115 in c765571e83
```
 110 |          self.nodes[0].sendrawtransaction(tx_hex)
 111 |  
 112 |          # Bump time forward to ensure m_next_inv_send_time timer pops
 113 |          self.nodes[0].setmocktime(int(time.time()) + 60)
 114 |  
 115 | +        tx_relay_peer.wait_for_broadcast([wtxid])
```
maflcko commented at 10:45 AM on October 13, 2026:

There is no need to wait for a broadcast, when not using the unbroadcast set for this test. Also, with mocktime, this should probably be a sync_with_ping instead?

Finally, the point was that the test has the relevant assert directly inside, so that it is somewhat "self-checking". So the difference between the two asserts is just the not.

fametrano commented at 5:24 AM on October 14, 2026:

Thanks, done in 22535b4. I dropped the wait and sync both peers with a ping; the two asserts now differ only by not.
maflcko commented at 5:31 AM on October 14, 2026: member

lgtm ACK 22535b460568be47e57aadfbe50d489becb77577
AndreaDiazCorreia commented at 1:42 AM on October 26, 2026: none

ACK 22535b460568be47e57aadfbe50d489becb77577

Ran p2p_blocksonly.py with v1 and v2 transport. With conn changed to outbound-full-relay, the new assertion fails while the old txid check still passes. Using txid in both assertions also fails at the positive control.

Contributors

Labels

Linked (view graph)

#35920 net_processing: Ignore MSG_WITNESS_TX entries from INV messages

test: look for the wtxid in p2p_blocksonly #36405

External sites

Reviews

Conflicts