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

pull vasild wants to merge 8 commits into bitcoin:master from vasild:keep_using_override_proxy_for_v1_reconnects_with_functional_test changing 11 files +449 −162
  1. 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.

  2. 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
  3. 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
  4. doc: add release notes for #35319 32d072a49f
  5. DrahtBot added the label P2P on May 29, 2026
  6. DrahtBot commented at 10:24 AM on May 29, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK Crypt-iQ, andrewtoth, instagibbs, sedited

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #35267 (rpc: make getprivatebroadcastinfo fail if privatebroadcast is not enabled by polespinasa)
    • #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.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

    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]

    <sup>2026-05-30 04:33:08</sup>

  7. 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
  8. 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
  9. 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
  10. vasild force-pushed on May 29, 2026
  11. vasild commented at 10:53 AM on May 29, 2026: contributor

    ed038aa33d5a8c8f612cb7ef852fab85308198fb...dd6c8d51c6a9a308657665612740ede4062d0c9e: pylint

  12. DrahtBot added the label CI failed on May 29, 2026
  13. DrahtBot commented at 10:53 AM on May 29, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task lint: https://github.com/bitcoin/bitcoin/actions/runs/26631964140/job/78482797321</sub> <sub>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).</sub>

    <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>

  14. 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()]>>
  15. vasild force-pushed on May 29, 2026
  16. fanquake added the label Private Broadcast on May 29, 2026
  17. Crypt-iQ commented at 3:28 PM on May 29, 2026: contributor

    ACK abd5f305ec0b816596f17be23995eea51232c116 pending CI

  18. vasild force-pushed on May 29, 2026
  19. 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.

  20. 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:

  21. test: add a regression test for private broadcast v1 retries 5a3756d150
  22. net: un-default the OpenNetworkConnection()'s proxy_override argument
    This way callers will not forget to set it.
    bf0d257c11
  23. vasild force-pushed on May 30, 2026
  24. vasild commented at 4:33 AM on May 30, 2026: contributor

    a71c168130067fb1538e3d21fed3aac719f6974d...bf0d257c11bebc42b88b7e96368ed2be48bc0ad1: address suggestions

  25. DrahtBot removed the label CI failed on May 30, 2026
  26. Crypt-iQ commented at 1:13 PM on June 1, 2026: contributor

    reACK bf0d257c11bebc42b88b7e96368ed2be48bc0ad1

  27. in test/functional/p2p_private_broadcast_retry_v1.py:160 in 5a3756d150 
     155 | +        node0 = self.nodes[0]
 156 | +
 157 | +        self.log.info("Filling node0's addrman with addresses")
 158 | +        self.fill_node_addrman(node_index=0, address_types_to_add=[CAddress.NET_IPV4])
 159 | +
 160 | +        self.log.info("Opening manual connections to all IPv4 addresses to add P2P_V2 flag to addrman entries")
    instagibbs commented at 3:30 PM on June 1, 2026:

    5a3756d150fc095d0038592c6338225db48fd4b5

    Found it a bit confusing: the node is connecting via v1 re:addnode, but advertising v2.

    Might be worth noting.

  28. instagibbs commented at 3:49 PM on June 1, 2026: member

    Test looks pretty good, and I think opens the door for more coverage

    Some good vibes test suggestions for consideration:

    https://github.com/instagibbs/bitcoin/commits/test-private-broadcast-retry-hardening/

    1. checks that the fix also fixes ipv6 rebroadcasting
    2. checks that the default proxy is never sent anything, to avoid the situation where other we're making other connections erroneously.
  29. fanquake renamed this:
    net: use the proxy if overriden when doing v2->v1 reconnections (with functional test)
    net: use the proxy if overriden when doing v2->v1 reconnections
    on Jun 1, 2026
  30. fanquake added the label Needs Backport (31.x) on Jun 1, 2026
  31. fanquake added this to the milestone 31.1 on Jun 1, 2026
  32. andrewtoth approved
  33. andrewtoth commented at 2:10 AM on June 2, 2026: contributor

    ACK bf0d257c11bebc42b88b7e96368ed2be48bc0ad1

  34. danielabrozzoni commented at 1:50 PM on June 3, 2026: member

    Hey, I wanted to flag that sometimes I get these noisy error logs while running the test (which always passes). I'm currently investigating, wondering if it happens to someone else too!

    ❯ ./build/test/functional/p2p_private_broadcast_retry_v1.py 
2026-06-03T13:23:56.291364Z TestFramework (INFO): PRNG seed is: 8427039551081571994
2026-06-03T13:23:56.291799Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_c2ojkqrw
2026-06-03T13:23:56.580066Z TestFramework (INFO): Filling node0's addrman with addresses
2026-06-03T13:23:56.621629Z TestFramework (INFO): Opening manual connections to all IPv4 addresses to add P2P_V2 flag to addrman entries
2026-06-03T13:23:56.761248Z TestFramework (INFO): Waiting for all IPv4 addresses to get P2P_V2 as a result of peers advertising support
2026-06-03T13:23:56.962812Z TestFramework.socks5 (ERROR): socks5 request handling failed (running True)
Traceback (most recent call last):
  File "/home/daniela/Developer/BitcoinCore/bitcoin/test/functional/test_framework/socks5.py", line 199, in handle
    forward_sockets(self.conn, conn_to, self.wakeup_socket_pair[1], self.serv)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/daniela/Developer/BitcoinCore/bitcoin/test/functional/test_framework/socks5.py", line 76, in forward_sockets
    data = s.recv(4096)
ConnectionResetError: [Errno 104] Connection reset by peer
2026-06-03T13:23:56.965510Z TestFramework.socks5 (ERROR): socks5 request handling failed (running True)
Traceback (most recent call last):
  File "/home/daniela/Developer/BitcoinCore/bitcoin/test/functional/test_framework/socks5.py", line 199, in handle
    forward_sockets(self.conn, conn_to, self.wakeup_socket_pair[1], self.serv)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/daniela/Developer/BitcoinCore/bitcoin/test/functional/test_framework/socks5.py", line 76, in forward_sockets
    data = s.recv(4096)
ConnectionResetError: [Errno 104] Connection reset by peer
2026-06-03T13:23:56.966612Z TestFramework.socks5 (ERROR): socks5 request handling failed (running True)
Traceback (most recent call last):
  File "/home/daniela/Developer/BitcoinCore/bitcoin/test/functional/test_framework/socks5.py", line 199, in handle
    forward_sockets(self.conn, conn_to, self.wakeup_socket_pair[1], self.serv)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/daniela/Developer/BitcoinCore/bitcoin/test/functional/test_framework/socks5.py", line 76, in forward_sockets
    data = s.recv(4096)
ConnectionResetError: [Errno 104] Connection reset by peer
2026-06-03T13:23:56.966859Z TestFramework.socks5 (ERROR): socks5 request handling failed (running True)
Traceback (most recent call last):
  File "/home/daniela/Developer/BitcoinCore/bitcoin/test/functional/test_framework/socks5.py", line 199, in handle
    forward_sockets(self.conn, conn_to, self.wakeup_socket_pair[1], self.serv)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/daniela/Developer/BitcoinCore/bitcoin/test/functional/test_framework/socks5.py", line 76, in forward_sockets
    data = s.recv(4096)
ConnectionResetError: [Errno 104] Connection reset by peer
2026-06-03T13:23:56.967086Z TestFramework.socks5 (ERROR): socks5 request handling failed (running True)
Traceback (most recent call last):
  File "/home/daniela/Developer/BitcoinCore/bitcoin/test/functional/test_framework/socks5.py", line 199, in handle
    forward_sockets(self.conn, conn_to, self.wakeup_socket_pair[1], self.serv)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/daniela/Developer/BitcoinCore/bitcoin/test/functional/test_framework/socks5.py", line 76, in forward_sockets
    data = s.recv(4096)
ConnectionResetError: [Errno 104] Connection reset by peer
2026-06-03T13:23:57.282943Z TestFramework (INFO): Opening a connection to a Tor addresses, so bitcoind considers -onion= is a real Tor proxy
2026-06-03T13:23:57.287777Z TestFramework (INFO): Waiting for at least one Tor connection
2026-06-03T13:23:57.291204Z TestFramework (INFO): Starting private broadcast connections
2026-06-03T13:23:57.325388Z TestFramework (INFO): Tor proxy: waiting for connection to an IPv4 address
2026-06-03T13:24:01.833484Z TestFramework (INFO): Tor proxy: got 190.0.0.1:8333, waiting for v2
2026-06-03T13:24:01.884335Z TestFramework (INFO): Tor proxy: got 190.0.0.1:8333 v2, waiting for v1
2026-06-03T13:24:03.242139Z TestFramework (INFO): Tor proxy: got 190.0.0.1:8333 v2, v1
2026-06-03T13:24:08.311255Z TestFramework (INFO): Stopping nodes
2026-06-03T13:24:08.311714Z TestFramework (INFO): Cleaning up /tmp/bitcoin_func_test_c2ojkqrw on exit
2026-06-03T13:24:08.311955Z TestFramework (INFO): Tests successful

    I found this comment in p2p_private_broadcast.py explaining that the socks proxy needs to be stopped before the node restart: https://github.com/bitcoin/bitcoin/blob/7c2718a4b82df709ee06380209747577514acbf0/test/functional/p2p_private_broadcast.py#L518-L521

    I think it's the same problem, the solution might just be to stop all_proxy before restarting... It's not even used after restarting as far as I can tell, so maybe it can be dropped from extra_args?

    Edit: I just saw https://github.com/bitcoin/bitcoin/pull/35410/changes#r3326416139, maybe instead of stopping all_proxy we can call setnetworkactive(False) on node, wait for getpeerinfo to return 0 peers, then restart the node?

  35. in doc/release-notes-35319.md:4 in 32d072a49f 
       0 | @@ -0,0 +1,20 @@
   1 | +P2P and network changes
   2 | +-----------------------
   3 | +
   4 | +- Fix a possible leak of the originator's IP address for transactions sent with
    instagibbs commented at 5:00 PM on June 3, 2026:

    Suggested alternative:

    "Fix ip address leak when using the -privatebroadcast feature. Under certain circumstances connections were being made over clearnet rather than the enabled privacy network."

    I think long explanation could go on the website.

    fanquake commented at 10:01 AM on June 4, 2026:

    Took something like this for the rel note in #35331. Can take further suggestion / additions to it there.

  36. instagibbs approved
  37. instagibbs commented at 5:01 PM on June 3, 2026: member

    ACK bf0d257c11bebc42b88b7e96368ed2be48bc0ad1

    Tests suffice for now, more can follow, a number of which I've run on my own setup with success.

  38. instagibbs commented at 5:28 PM on June 3, 2026: member

    @danielabrozzoni IIUC we need to handle connection resets upon abrupt shutdown. We can turn them into debug logs or whatever instead of loudly throwing an exception. i.e. in forward_sockets: "except (ConnectionResetError, BrokenPipeError) as e:"

  39. sedited commented at 8:53 PM on June 3, 2026: contributor

    utACK bf0d257c11bebc42b88b7e96368ed2be48bc0ad1

  40. fanquake merged this on Jun 4, 2026
  41. fanquake closed this on Jun 4, 2026
  42. fanquake removed the label Needs Backport (31.x) on Jun 4, 2026
  43. fanquake commented at 10:00 AM on June 4, 2026: member

    Backported to 31.x (minus the release note) in #35331.

  44. fanquake referenced this in commit 0fee0c2c49 on Jun 4, 2026
  45. fanquake referenced this in commit 7f5b249bae on Jun 4, 2026
  46. fanquake referenced this in commit cbcdc6ebb0 on Jun 4, 2026
  47. fanquake referenced this in commit eef53ad20a on Jun 4, 2026
  48. fanquake referenced this in commit 31ccc9165f on Jun 4, 2026
  49. fanquake referenced this in commit 0e1f3a7d15 on Jun 4, 2026
  50. fanquake referenced this in commit bf728e6f6b on Jun 4, 2026
  51. rustaceanrob referenced this in commit c33a13bc1d on Jun 5, 2026
  52. danielabrozzoni commented at 11:31 AM on June 8, 2026: member

    Post merge ACK bf0d257c11bebc42b88b7e96368ed2be48bc0ad1

  53. vasild deleted the branch on Jun 11, 2026
  54. vasild commented at 8:11 AM on June 12, 2026: contributor

    data = s.recv(4096) ConnectionResetError: [Errno 104] Connection reset by peer

    Attempt to address this in #35510 (it is not due to this PR).

Contributors
vasildDrahtBotfanquakeCrypt-iQandrewtothinstagibbsdanielabrozzonisedited
Labels
P2PPrivate Broadcast

Milestone
31.1

Linked (view graph)
#34476 Private broadcast tracking#35319 net: use the proxy if overriden when doing v2->v1 reconnections#35331 [31.x] Backports
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-06-21 09:50 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me