net: use the proxy if overriden when doing v2->v1 reconnections (with functional test) #35410

vasild commented at 10:24 AM on May 29, 2026: contributor

This PR includes #35319 and on top of that adds a regression functional test.

The functional test exercises the relevant code paths without modifying non-test code. To do that it does:

Add a bunch of IPv4 addresses to the node's addrman (they will be added without P2P_V2 flag).
Get them to report P2P_V2 in their service flags and connect to each one, so that the flags in addrman are updated to contain P2P_V2.
Get one successful connection to a Tor peer (.onion) so that bitcoind assumes the configured Tor proxy works and is indeed a proxy to the Tor network. This will make it open private broadcast connections also to IPv4 addresses via that proxy.
Start some private broadcast connections.
Remember the destination IPv4 address of the first connection and get it to fail the v2 transport.
Wait for a subsequent connection also through the Tor proxy to the same IPv4 and expect it to be v1, i.e. the v2->v1 downgrade retry.

The test fails without the fix - the v1 retry never arrives to the Tor proxy. And passes with the fix. The fix is in the first commit here and in #35319, can remove it by git show fd230f942d | git apply -R.

net: use the proxy if overriden when doing v2->v1 reconnections

`OpenNetworkConnection()` supports overriding the proxy to use for
connecting. However when v2 connection is attempted and it fails a v1
connection is tried without that proxy.

Store the override proxy in `CNode` and pass it to
`CConnman::m_reconnections` to be used for v1 retries.

fd230f942d

net: ensure no direct private broadcast connections

Private broadcast connections use either Tor or I2P, which require a
proxy intrinsically or IPv4 or IPv6 which must use a proxy in the
context of private broadcast to avoid leaking the originator's IP
address.

Add a safety check to guard against future mistakes.

Co-authored-by: Andrew Toth <andrewstoth@gmail.com>

d01b461f71

doc: add release notes for #35319 32d072a49f

DrahtBot added the label P2P on May 29, 2026

DrahtBot commented at 10:24 AM on May 29, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35410.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Stale ACK	Crypt-iQ

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:

#35221 (BIP 434 Support: Peer feature negotiation by ajtowns)
#35027 (net: add -outboundbind option for outgoing source address by 8144225309)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

transport versions (v2 or1) -> transport versions (v2 or 1) [missing space makes the phrase harder to read]

2026-05-30 04:33:08

test: make reusable SOCKS5 server starting

Extract the part of `p2p_private_broadcast.py` that configures and
starts the SOCKS5 server into a reusable function and put it into
`test_framework/socks5.py`.

Use bind port 0 to let the OS pick an available port instead of
hackishly assuming that `p2p_port(N)` is available where N is more
than the number of the nodes the test uses.

2ffa81fac4

test: make reusable starting a standalone P2P listener

Extract the part of `p2p_private_broadcast.py` that starts
listening on a `P2PConnection` object (or its children classes)
and put it into `test_framework/p2p.py`.

2333be9cbc

test: make reusable filling of a node's addrman

Extract the part of `p2p_private_broadcast.py` that fills a given node's
addrman and put it into `test_framework/test_framework.py`.

ab35a028ed

vasild force-pushed on May 29, 2026

vasild commented at 10:53 AM on May 29, 2026: contributor

ed038aa33d5a8c8f612cb7ef852fab85308198fb...dd6c8d51c6a9a308657665612740ede4062d0c9e: pylint

DrahtBot added the label CI failed on May 29, 2026

DrahtBot commented at 10:53 AM on May 29, 2026: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/26631964140/job/78482797321 LLM reason (✨ experimental): Python lint (py_lint/ruff) failed due to 3 fixable F401 unused-import errors in test/functional/p2p_private_broadcast.py (ruff returned exit code 1).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

fanquake commented at 11:42 AM on May 29, 2026: member

https://github.com/bitcoin/bitcoin/actions/runs/26633178388/job/78486867509?pr=35410#step:11:4084:

p2p_private_broadcast_retry_v1.py                                                | ✖ Failed  | 4 s

[5](https://github.com/bitcoin/bitcoin/actions/runs/26633178388/job/78486867509?pr=35410#step:11:4096)
2026-05-29T11:16:26.043881Z TestFramework (INFO): Tor proxy: got 190.0.0.1:8333, waiting for v2
2026-05-29T11:16:26.144163Z TestFramework (INFO): Tor proxy: got 190.0.0.1:8333 v2, waiting for v1
2026-05-29T11:16:27.496975Z TestFramework (INFO): Tor proxy: got 190.0.0.1:8333 v2, v1
2026-05-29T11:16:27.562422Z TestFramework (INFO): Stopping nodes
2026-05-29T11:16:28.564949Z TestFramework (INFO): Cleaning up /home/runner/work/_temp/ci/scratch_ ₿🧪_/test_runner/test_runner_₿_🏃_20260529_110827/p2p_private_broadcast_retry_v1_20 on exit
2026-05-29T11:16:28.564999Z TestFramework (INFO): Tests successful


stderr:
Task was destroyed but it is pending!
task: <Task pending name='Task-54' coro=<BaseSelectorEventLoop._accept_connection2() done, defined at /usr/lib/python3.13/asyncio/selector_events.py:213> wait_for=<Future pending cb=[Task.task_wakeup()]>>

vasild force-pushed on May 29, 2026

fanquake added the label Private Broadcast on May 29, 2026

Crypt-iQ commented at 3:28 PM on May 29, 2026: contributor

ACK abd5f305ec0b816596f17be23995eea51232c116 pending CI

vasild force-pushed on May 29, 2026

in src/net.cpp:3025 in a71c168130

3021 | @@ -3022,7 +3022,7 @@ void CConnman::ThreadOpenAddedConnections()
3022 |              }
3023 |              tried = true;
3024 |              CAddress addr(CService(), NODE_NONE);
3025 | -            OpenNetworkConnection(addr, false, std::move(grant), info.m_params.m_added_node.c_str(), ConnectionType::MANUAL, info.m_params.m_use_v2transport);
3026 | +            OpenNetworkConnection(addr, false, std::move(grant), info.m_params.m_added_node.c_str(), ConnectionType::MANUAL, info.m_params.m_use_v2transport, std::nullopt);

andrewtoth commented at 6:13 PM on May 29, 2026:

nit: should prefix std::nullopt with /*proxy_override=*/ for all uses in the last commit.

vasild commented at 4:34 AM on May 30, 2026:

Done, plus prefixed all arguments and because lines became too long, wrapped them.

in test/functional/p2p_private_broadcast_retry_v1.py:193 in a71c168130 outdated

 188 | +        self.wait_until(lambda: 2 in self.ipv4_via_tor_proxy_conn_versions)
 189 | +        self.log.info(f"Tor proxy: got {self.ipv4_via_tor_proxy_addr_port} v2, waiting for v1")
 190 | +        self.wait_until(lambda: 1 in self.ipv4_via_tor_proxy_conn_versions)
 191 | +        self.log.info(f"Tor proxy: got {self.ipv4_via_tor_proxy_addr_port} v2, v1")
 192 | +
 193 | +        self.all_proxy.stop()

andrewtoth commented at 7:13 PM on May 29, 2026:

This will fix the CI error. The shutdown() closes the asyncio loop, before node 0 is shutdown. A pending private broadcast connection on the listener side may be left hanging.

        self.stop_node(0)
        self.all_proxy.stop()

vasild commented at 4:34 AM on May 30, 2026:

Done. CI :pray: :crossed_fingers:

test: add a regression test for private broadcast v1 retries 5a3756d150

net: un-default the OpenNetworkConnection()'s proxy_override argument

This way callers will not forget to set it.

bf0d257c11

vasild force-pushed on May 30, 2026

vasild commented at 4:33 AM on May 30, 2026: contributor

a71c168130067fb1538e3d21fed3aac719f6974d...bf0d257c11bebc42b88b7e96368ed2be48bc0ad1: address suggestions

DrahtBot removed the label CI failed on May 30, 2026