net: fix startup failures from stale Tor examples #36177

l0rinc commented at 10:32 PM on September 5, 2026: contributor

Problem: #34031 removed tor as a network name, but the -proxy help and Tor documentation still advertise it, and following those examples fails at startup. The proxy option also maintains its own network-name mapping while -onlynet and getnodeaddresses use the shared parser.

Fix: Reuse the shared parser and canonical network names for -proxy, and document onion consistently. Unsupported network names are now echoed as supplied, matching the other interfaces, while accepted names remain case-insensitive.

DrahtBot added the label P2P on Sep 5, 2026

DrahtBot commented at 10:32 PM on September 5, 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/36177.

Reviews

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

Type	Reviewers
Concept ACK	jeanpablojp
Stale ACK	vasild

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:

#35578 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35578.svg"></sub> (net: don’t self advertise tor exit node ip addresses in outbound connections by stratospher)

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.

jeanpablojp commented at 9:23 PM on September 6, 2026: contributor

Concept ACK

Built and ran feature_proxy.py and rpc_net.py, and checked against the base that the refactor commit keeps the same accepted names and the same per-network proxies.

in doc/tor.md:16 in 723bf379ea

  10 | @@ -11,6 +11,11 @@ The following directions assume you have a Tor proxy running on port 9050. Many
  11 |  
  12 |  - Tor removed v2 support beginning with version 0.4.6.
  13 |  
  14 | +- Since version 31.0, the network name for onion services is `onion` (as in
  15 | +  `-onlynet=onion` or `-proxy=addr:port=onion`); the former `tor` name is no
  16 | +  longer accepted ([#34031](https://github.com/bitcoin/bitcoin/pull/34031)).

jeanpablojp commented at 9:23 PM on September 6, 2026:

This reads as if onion were new in 31.0, but v30.0 already accepted tor and onion for -proxy (ca5781e23a). Naming Core also keeps this 31.0 apart from the Tor 0.4.6 above. Something like this?

- Bitcoin Core 31.0 removed `tor` as a network name. The name to use for onion
  services is `onion`, as in `-onlynet=onion` or `-proxy=addr:port=onion`
  ([#34031](https://github.com/bitcoin/bitcoin/pull/34031)).

l0rinc commented at 11:33 PM on September 8, 2026:

Simplified similarly

in test/functional/rpc_net.py:337 in 723bf379ea

 334 | @@ -335,12 +335,13 @@ def test_getnodeaddresses(self):
 335 |          assert_equal(res[0]["services"], P2P_SERVICES)
 336 |  
 337 |          # Test for the absence of onion, I2P and CJDNS addresses.

jeanpablojp commented at 9:23 PM on September 6, 2026:

nit: OnIoN isn't a fourth network here, it's the same one covering case-insensitive matching, and the comment above still describes only the three. Worth widening it?

        # Test for the absence of onion, I2P and CJDNS addresses, and case-insensitive names.

l0rinc commented at 11:33 PM on September 8, 2026:

removed the whole comment instead

in test/functional/feature_proxy.py:473 in 723bf379ea outdated

 471 | @@ -470,8 +472,8 @@ def networks_dict(d):
 472 |          assert_equal(nets["ipv6"]["proxy"], "127.6.6.6:6666")
 473 |          self.stop_node(1)

jeanpablojp commented at 9:23 PM on September 6, 2026:

I found this pre-existing coverage gap while reviewing the refactor. Keeping the name proxy when clearing the IPv4 proxy still passes feature_proxy.py. Could we add this case, better placed next to the CJDNS removal below?

        self.stop_node(1)

        self.log.info("Test that clearing the IPv4 proxy also clears the name proxy")
        self.start_node(1, extra_args=[f"-proxy={self.conf1.addr[0]}:{self.conf1.addr[1]}", "-proxy=0=ipv4", "-dns=0"])
        with self.nodes[1].assert_debug_log(
            expected_msgs=["trying v1 connection (manual) to example.com:8333"],
            unexpected_msgs=["SOCKS5 connecting example.com"],
        ):
            self.nodes[1].addnode("example.com:8333", "onetry", v2transport=False)
        self.stop_node(1)

l0rinc commented at 11:33 PM on September 8, 2026:

While it's not strictly related, we do need the coverage in this area - added the test and you as coauthor

l0rinc force-pushed on Sep 8, 2026

l0rinc commented at 11:34 PM on September 8, 2026: contributor

Thanks @jeanpablojp, rebased, added new test, updated doc and removed redundant code comment

in test/functional/feature_proxy.py:499 in b960b1e3e9 outdated

 494 | +        with self.nodes[1].assert_debug_log(
 495 | +            expected_msgs=["trying v1 connection (manual) to example.com:8333"],
 496 | +            unexpected_msgs=["SOCKS5 connecting example.com"],
 497 | +        ):
 498 | +            self.nodes[1].addnode("example.com:8333", "onetry", v2transport=False)
 499 | +        self.stop_node(1)

vasild commented at 10:15 AM on September 30, 2026:

It seems to me that this will actually attempt to open a connection to example.com:8333. That would be undesirable, see #31349.

To avoid that we usually provide a bogus proxy setting, like 127.0.0.1 on port 1. However here the idea is to explicitly go directly, without a proxy. So, to avoid non-localhost traffic, what about using e.g. 127.8.3.3:3 instead of example.com:8333?

vasild commented at 5:36 PM on September 30, 2026:

-dns=0 prevents the connections to example.com:8333

l0rinc commented at 6:25 PM on September 30, 2026:

yes, we set -dns=0 explicitly in https://github.com/bitcoin/bitcoin/blob/b960b1e3e99290937fa2e0b39b69953d59906bfe/test/functional/feature_proxy.py#L493. Do you still think we need to change anything here?

vasild commented at 7:54 AM on October 1, 2026:

Given -dns=0 this is "safe". Still, using an address from 127.0.0.0/8 with some bogus port seems more direct and robust to me, but definitely not a blocker anymore (initially I didn't realize -dns=0 would stop that). Up to you.

fanquake commented at 9:05 AM on October 1, 2026:

Do you still think we need to change anything here?

I think it would be better just just avoid "real" domains entirely, rather than rely on other settings.

l0rinc commented at 7:10 PM on October 1, 2026:

Valid point, rebased and used https://example.invalid instead, thanks for the feedback.

vasild commented at 10:34 AM on September 30, 2026: contributor

Almost ACK b960b1e3e99290937fa2e0b39b69953d59906bfe, except example.com:8333

DrahtBot requested review from jeanpablojp on Sep 30, 2026

vasild approved

vasild commented at 7:50 AM on October 1, 2026: contributor

ACK b960b1e3e99290937fa2e0b39b69953d59906bfe

<details> <summary>Show Signature</summary>

vasild's public key is on openpgp.org

</details>

test: characterize proxy network name errors

The `-proxy` error lowercases unsupported network names, unlike `-onlynet` and `getnodeaddresses`.
Also cover rejection of the `tor` name removed in #34031, mixed-case `onion` acceptance, I2P exclusion, and clearing the IPv4 and name proxies together.

Co-authored-by: jeanpablo <me@jeanpablo.jp>

332c3e8e3e

refactor: share proxy network parsing

Use the shared network parser to select each per-network proxy, matching `-onlynet` and `getnodeaddresses`.
Keep accepted names, proxy assignments, and error messages unchanged.

b0375749d4

net: preserve network name case in proxy errors

`-proxy` lowercases unsupported network names in its error message, unlike `-onlynet` and `getnodeaddresses`.
Let the shared parser handle case-insensitive matching so the diagnostic retains the user's spelling.

fd65406b39

doc: use onion in proxy help and examples

`tor` was removed as a network name in #34031, but the proxy help and Tor examples still advertise it.
Use canonical network names in the help and document `onion` with the removal reference.

6c7c27f615

l0rinc force-pushed on Oct 1, 2026