net: advertise -externalip addresses by willcl-ark · Pull Request #34538 · bitcoin/bitcoin

willcl-ark · 2026-02-09T12:16:46Z

-onlynet is documented to restrict automatic outbound connections, but it also currently prevents -externalip addresses from being advertised when their network is not in the -onlynet set. This happens because AddLocal() rejects addresses outside g_reachable_nets, regardless of whether the address was explicitly configured by the user.

Previous attempts to fix this (#24835 and #25690) removed the g_reachable_nets check from AddLocal().

This PR instead adds an explicit add_even_if_unreachable argument to AddLocal(). The argument defaults to false, and is set to true only when adding addresses from -externalip.

As a result, explicitly configured -externalip addresses can still be advertised even when their network is excluded by -onlynet, while discovered, mapped, bound, Tor control, and I2P SAM addresses continue to use the existing reachable-network filter.

This keeps the fix scoped to -externalip and addresses the concern raised in #25690:

I think it might also be weird for a user to activate -onlynet and keep on advertising their clearnet address to the network

The branch adds unit coverage for AddLocal() and functional coverage in p2p_addr_selfannouncement.py for -onlynet=ipv4 -externalip=<onion>.

Fixes: #25336
Fixes: #25669

DrahtBot · 2026-02-09T12:17:08Z

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/34538.

Reviews

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

Type	Reviewers
ACK	mzumsande, w0xlt, achow101
Concept ACK	chriszeng1010
Stale ACK	seduless, vasild

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

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

willcl-ark · 2026-02-09T12:18:38Z

cc @mzumsande @vasild

I've taken a very slightly different approach at fixing this, and would value your opinion(s) on :)

chriszeng1010

Tested ACK

Built and rantest_bitcoin --run_test=net_testson Windows/MSYS2. New addlocal_onlynet_externalip test passes.

seduless

Tested ACK 383763f

Reproduced #25336 on regtest: started a node with -onlynet=ipv4 -externalip=<onion_address>, confirmed getnetworkinfo shows empty localaddresses on master. With the fix applied, the onion address appears at score 4.

Traced all LOCAL_MANUAL callers and found no inconsistency with IsPeerAddrLocalGood.

seduless

re-ACK 6e73dcf

vasild

ACK 6e73dcf

Note that AddLocal(..., LOCAL_MANUAL) is called from 3 places:

For addresses given to -externalip=..., the focus of this PR.
For our listening Tor address that is automatically created with the Tor router when -listenonion=1. So this setup will be affected by this PR: -onlynet=ipv4 -listenonion=1. Before this PR we would not advertise the Tor address and now we will.
For our listening I2P address that is created with the I2P SAM proxy when -i2psam=... -i2pacceptincoming=1 is used. Similarly to Tor this would affect -onlynet=ipv4 -i2psam=... -i2pacceptincoming=1.

I think those 2. and 3. are welcomed changes. Maybe deserve a release note.

Will resolve #25669

vasild · 2026-03-05T11:09:42Z

+    RemoveLocal(addr_onion);
+    g_reachable_nets.Add(NET_ONION);
+    fDiscover = true;


I guess this is an attempt to restore the global state to what it was at the beginning of the test. Here are more robust ways to that:

For g_reachable_nets:

... test ... // Assumes that g_reachable_nets was in its default-constructed state at the start of the test. g_reachable_nets.Reset();

or (no assumptions)

const auto reachable_nets_at_start{g_reachable_nets.All()}; ... test ... g_reachable_nets.RemoveAll(); for (const auto& net : reachable_nets_at_start) { g_reachable_nets.Add(net); }

For fDiscover:

const bool discover_orig{fDiscover}; ... test ... fDiscover = discover_orig;

Good point, thanks, I agree. I changed the test to restore both global states as you suggested, in e459f65

willcl-ark · 2026-03-05T12:19:25Z

I think those 2. and 3. are welcomed changes. Maybe deserve a release note.

😃 Release note added in 3c63971

vasild

ACK 3c63971

vasild · 2026-03-05T12:36:11Z

Might want to add "Fixes: #25669" to the OP.

seduless

re-ACK 3c63971

ajtowns · 2026-03-20T09:36:22Z

Might be better to set a bit somewhere for -externalip addresses rather than reusing score? Otherwise it seems possible that setting up a private node and then having some controlled peers connect to it would result in the score eventually being bumped above the LOCAL_MANUAL level and you'd start unexpectedly advertising your ip automatically, which might be a footgun for some.

The configuration and expected/implicit behaviours here seem very confusing and fragile in general here; eg I think this code is (or reads as-if it is) treating IsRoutable as "will packets sent to/from here have any chance of arriving?" while other parts of the code are using it as the -onlynet "should we make outbound connections to this part of the internet?"

vasild

ACK 7b28ef4

Show Signature

vasild's public key is on openpgp.org

vasild · 2026-04-13T13:58:00Z


 // learn a new local address
-bool AddLocal(const CService& addr_, int nScore)
+bool AddLocal(const CService& addr_, int nScore, bool fExternalIP)


Naming, feel free to ignore:

The linkage between this argument and the -externalip configuration option is only outside of the AddLocal() function. So, looking at the function alone and not its callers, the name fExternalIP is odd.

From the point of view of the AddLocal() function, the meaning of its new argument is "ignore reachable nets" or "add even if unreachable". It is better to name it after that.

This would show in the future if there is a new caller of AddLocal() who wants to pass true to "add even if unreachable" for a reason other than that the address comes from -externalip. Then the caller would have to use /*fExternalIP=*/true even if he has nothing to do with -externalip.

Thanks. I addressed both of your review points at once by renaming the new param to add_even_if_unreachable in b187434

vasild · 2026-04-13T14:01:49Z

 }

-bool AddLocal(const CNetAddr &addr, int nScore)
+bool AddLocal(const CNetAddr& addr, int nScore, bool fExternalIP)


My understanding and preference is that we follow the naming convention regardless of surrounding code that might use the old convention. That is, name the new parameter external_ip instead of fExternalIP. Then, if you want, in another commit, for consistency rename nScore to score.

vasild

ACK d63f6a9

Show Signature

vasild's public key is on openpgp.org

willcl-ark · 2026-05-24T20:31:08Z

Might be better to set a bit somewhere for -externalip addresses rather than reusing score? Otherwise it seems possible that setting up a private node and then having some controlled peers connect to it would result in the score eventually being bumped above the LOCAL_MANUAL level and you'd start unexpectedly advertising your ip automatically, which might be a footgun for some.

The configuration and expected/implicit behaviours here seem very confusing and fragile in general here; eg I think this code is (or reads as-if it is) treating IsRoutable as "will packets sent to/from here have any chance of arriving?" while other parts of the code are using it as the -onlynet "should we make outbound connections to this part of the internet?"

I should have updated after my last push here, sorry. I think the current approach in d63f6a9 addresses the (correct) concerns you had: the reachable-net bypass is no longer derived from the stored local-address score, but is passed explicitly only from the -externalip init path. So peer observations can still change nScore, but cannot cause a non-externalip address to start bypassing -onlynet.

I considered adding persistent state to LocalServiceInfo, but it makes the AddLocal path a (tiny) bit more complicated and I don't see another user of that state. Do you know of one outside of this reachable-net decision? The diff wouldn't be large, but I think this is fine as it is here too.

mzumsande

ACK d63f6a9

I think the behavior of the current version may match user expectations best.

Though it still doesn't match the documentation of -onlynet ("Inbound and manual connections are not affected by this option.") when the automatic discovery is supppressed for non-reachable networks, including cases like -onlynet=ipv4 -listenonion=1.

Prepare for letting explicitly configured local addresses bypass the reachable-net filter without tying that behavior to LOCAL_MANUAL score. Pass an `add_even_if_unreachable` bool through `AddLocal()`, defaulting to `false`.

When -onlynet excludes a network, AddLocal() rejects local addresses from that network. This also rejects addresses explicitly configured via -externalip, so a configuration like -onlynet=ipv4 -externalip=<onion> never advertises the onion address. Set the AddLocal() unreachable-net override only when adding -externalip addresses so the bypass remains limited to explicit user configuration.

Add unit coverage for the -onlynet/-externalip interaction. Check that an unreachable address still fails with normal AddLocal() arguments, while the same address succeeds when the unreachable-net override is set for explicit local-address configuration.

Extend p2p_addr_selfannouncement to restart the node with -onlynet=ipv4 -externalip=<onion> and verify the onion address appears in localaddresses despite its network being unreachable.

willcl-ark · 2026-07-07T09:54:15Z

Though it still doesn't match the documentation of -onlynet ("Inbound and manual connections are not affected by this option.") when the automatic discovery is supppressed for non-reachable networks, including cases like -onlynet=ipv4 -listenonion=1.

I agree somewhat, but IMO listenonion=1 is a fair bit more ambiguous as it is automatically creating a service which, if advertised with a configuration like -onlynet=ipv4, might do something the user doesn't expect, if they consider themselves using onlynet as a privacy boundary.

mzumsande

re-ACK dab7f2c

only commit messages changed

w0xlt

ACK dab7f2c

achow101 · 2026-07-14T22:31:42Z

ACK dab7f2c

DrahtBot added the P2P label Feb 9, 2026

chriszeng1010 reviewed Mar 2, 2026

View reviewed changes

seduless reviewed Mar 3, 2026

View reviewed changes

Comment thread src/test/net_tests.cpp Outdated

DrahtBot requested a review from chriszeng1010 March 3, 2026 04:04

willcl-ark force-pushed the onlynet-advertisments branch from 383763f to 6e73dcf Compare March 3, 2026 10:01

seduless reviewed Mar 3, 2026

View reviewed changes

vasild approved these changes Mar 5, 2026

View reviewed changes

willcl-ark force-pushed the onlynet-advertisments branch from 6e73dcf to 3c63971 Compare March 5, 2026 12:18

vasild approved these changes Mar 5, 2026

View reviewed changes

DrahtBot requested a review from seduless March 5, 2026 12:33

seduless reviewed Mar 10, 2026

View reviewed changes

sedited requested review from ajtowns and removed request for chriszeng1010 March 19, 2026 17:14

DrahtBot requested a review from chriszeng1010 March 20, 2026 09:36

willcl-ark force-pushed the onlynet-advertisments branch from 3c63971 to 7b28ef4 Compare March 20, 2026 12:18

This was referenced Apr 8, 2026

net: add -outboundbind option for outgoing source address #35027

Open

net_processing: don't modify addrman for private broadcast connections #35032

Merged

DrahtBot added the Needs rebase label Apr 9, 2026

vasild approved these changes Apr 13, 2026

View reviewed changes

DrahtBot requested a review from seduless April 13, 2026 14:02

willcl-ark force-pushed the onlynet-advertisments branch from 7b28ef4 to d63f6a9 Compare April 30, 2026 11:13

vasild approved these changes Apr 30, 2026

View reviewed changes

DrahtBot removed the Needs rebase label Apr 30, 2026

w0xlt reviewed Jun 12, 2026

View reviewed changes

Comment thread src/net.cpp

DrahtBot mentioned this pull request Jun 23, 2026

Remove boost as a unit test runner #35587

Closed

mzumsande reviewed Jul 6, 2026

View reviewed changes

Comment thread src/net.cpp

willcl-ark added 4 commits July 7, 2026 10:48

net: add an add_even_if_unreachable argument to AddLocal

f4af02e

Prepare for letting explicitly configured local addresses bypass the reachable-net filter without tying that behavior to LOCAL_MANUAL score. Pass an `add_even_if_unreachable` bool through `AddLocal()`, defaulting to `false`.

test: cover -externalip/onlynet interaction in functional test

dab7f2c

Extend p2p_addr_selfannouncement to restart the node with -onlynet=ipv4 -externalip=<onion> and verify the onion address appears in localaddresses despite its network being unreachable.

willcl-ark force-pushed the onlynet-advertisments branch from d63f6a9 to dab7f2c Compare July 7, 2026 09:50

mzumsande reviewed Jul 7, 2026

View reviewed changes

DrahtBot requested a review from vasild July 7, 2026 13:28

w0xlt reviewed Jul 8, 2026

View reviewed changes

achow101 merged commit 70d9ec7 into bitcoin:master Jul 14, 2026
28 checks passed

Uh oh!

Conversation

willcl-ark commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

willcl-ark commented Feb 9, 2026

Uh oh!

chriszeng1010 left a comment

Choose a reason for hiding this comment

Uh oh!

seduless left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

seduless left a comment

Choose a reason for hiding this comment

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

vasild Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

willcl-ark Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

willcl-ark commented Mar 5, 2026

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

vasild commented Mar 5, 2026

Uh oh!

seduless left a comment

Choose a reason for hiding this comment

Uh oh!

ajtowns commented Mar 20, 2026

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

vasild Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

willcl-ark Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

vasild Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

willcl-ark commented May 24, 2026

Uh oh!

Uh oh!

mzumsande left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

willcl-ark commented Jul 7, 2026

Uh oh!

mzumsande left a comment

Choose a reason for hiding this comment

Uh oh!

w0xlt left a comment

Choose a reason for hiding this comment

Uh oh!

achow101 commented Jul 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

willcl-ark commented Feb 9, 2026 •

edited

Loading

DrahtBot commented Feb 9, 2026 •

edited

Loading