da7f70a5322843b70f29456a8bc2227209a0718b test: use port 0 for I2P addresses in p2p_private_broadcast.py (Vasil Dimov)
a8ebcfd34c63f142064b4f5ef7d52299739d4cd6 test: let connections happen in any order in p2p_private_broadcast.py (Vasil Dimov)
67696b207f370e902c8d5fb765e4ff10f6c9e1b4 net: extend log message to include attempted connection type (Vasil Dimov)
Pull request description:
If the following two events happen:
* (likely) the automatic 10 initial connections are not made to all
networks
* (unlikely) the network-specific logic kicks in almost immediately.
It is using exponential distribution with a mean of 5 minutes
(`rng.rand_exp_duration(EXTRA_NETWORK_PEER_INTERVAL)`).
So if both happen, then the 11th connection may not be the expected
private broadcast, but a network-specific connection.
Fix this by retrieving the connection type from
`destinations_factory()`. This is more flexible because it allows
connections to happen in any order and does not break if e.g. the 11th
connection is not the expected first private broadcast.
This also makes the test run faster:
before: 19-44 sec
now: 10-25 sec
because for example there is no need to wait for the initial 10
automatic outbound connections to be made in order to proceed.
Fixes: https://github.com/bitcoin/bitcoin/issues/34387
ACKs for top commit:
achow101:
ACK da7f70a5322843b70f29456a8bc2227209a0718b
andrewtoth:
ACK da7f70a5322843b70f29456a8bc2227209a0718b
mzumsande:
Code Review ACK da7f70a5322843b70f29456a8bc2227209a0718b
Tree-SHA512: 7c293e59c15c148a438e0119343b05eb278205640658c99336d4caf4848c5bae92b48e15f325fa616cbc9d5f394649abfa02406a76e802cffbd3d312a22a6885
I2P addresses must use port=0, otherwise `bitcoind` refuses to connect.
The test `p2p_private_broadcast.py` cannot simulate connections to I2P
peers, so the I2P proxy is set to a dummy `127.0.0.1:1`. Still it is
good to pick I2P addresses and attempt connections to increase coverage.
However the test uses port=8333 for I2P addresses and thus the
connection attempts fail for the "wrong" reason:
```
Error connecting to ...i2p:8333, connection refused due to arbitrary port 8333
```
Using the proper port=0 makes the failures:
```
Error connecting to ...i2p:0: Cannot connect to 127.0.0.1:1
```
which will make possible simulated I2P connections once we have a test
I2P proxy.
If the following two events happen:
* (likely) the automatic 10 initial connections are not made to all
networks
* (unlikely) the network-specific logic kicks in almost immediately.
It is using exponential distribution with a mean of 5 minutes
(`rng.rand_exp_duration(EXTRA_NETWORK_PEER_INTERVAL)`).
So if both happen, then the 11th connection may not be the expected
private broadcast, but a network-specific connection.
Fix this by retrieving the connection type from
`destinations_factory()`. This is more flexible because it allows
connections to happen in any order and does not break if e.g. the 11th
connection is not the expected first private broadcast.
This also makes the test run faster:
before: 19-44 sec
now: 10-25 sec
because for example there is no need to wait for the initial 10
automatic outbound connections to be made in order to proceed.
Fixes: https://github.com/bitcoin/bitcoin/issues/34387
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
```
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})
```
3f5211cba8e73e8eb03781e6ec32ba9c4a263782 test: remove child_one/child_two (w)txid variables (naiyoma)
7cfe790820cf247e8a27bb8091defc54c74d6aec test: replace ValidWitnessMalleatedTx class with function (naiyoma)
81675a781f3ab62a0576a9739d13b4997b63230d test: use pre-generated chain (naiyoma)
Pull request description:
This PR refactors ` ValidWitnessMalleatedTx` class into a `build_malleated_tx_package` function. As a result, two tests are updated: `mempool_accept_wtxid` and `p2p_p2p_private_broadcast`. Also included are a few small refactors in mempool_accept_wtxid , (switching to MiniWallet, using a pre-mined chain, using txid directly.)
Together, these changes reduce complexity and improve test runtime.
ACKs for top commit:
stratospher:
reACK 3f5211c.
cedwies:
reACK 3f5211c
maflcko:
review ACK 3f5211cba8e73e8eb03781e6ec32ba9c4a263782 👥
rkrux:
ACK 3f5211cba8e73e8eb03781e6ec32ba9c4a263782
Tree-SHA512: 1fd02be3432fef6b68e54fbe8b15ed56d2699580bb13d0777b21f9cbe4c6d33bbb710541e3ca2fc93eab771d17bf1c427e4b08fa216d561bdb320cc6b36ac8fc
Simplify the witness malleation test helper by converting the
ValidWitnessMalleatedTx class to a standalone function
build_malleated_tx_package() and updating call sites.
Co-authored-by: rkrux <rkrux.connect@gmail.com>
The test `p2p_private_broadcast.py` gets some Python P2P nodes to listen
and instructs the SOCKS5 proxy to redirect connections to them instead
of to the requested addresses. This way the `bitcoind` which uses the
proxy is tricked to think it has connected to real routable internet
IP addresses or `.onion` addresses.
Picking the ports where to Python P2P nodes to listen however is tricky
to be done in a non-conflicting way, given that other tests may run in
parallel. https://github.com/bitcoin/bitcoin/pull/34186 made it possible
to let the OS select a free port, so use that in
`p2p_private_broadcast.py`.
