net: advertise -externalip addresses #34538

willcl-ark commented at 12:16 PM on February 9, 2026: member

-onlynet is documented to only affect automatic outbound connections, but it also currently prevents -externalip addresses from being advertised when their network isn't in the -onlynet set. This is because of the g_reachable_nets.Contains() check in AddLocal(), which rejects all addresses outside the reachable set, regardless of how they were specified.

Previous attempts to fix this (#24835 and #25690) both tried removal of the g_reachable_nets check from AddLocal().

This approach uses a bool flag, fExternalIP, and pass it through AddLocal(), set only in the -externalip startup path. The g_reachable_nets check is bypassed only when this flag is true.

Discovered addresses on non-onlynet networks are still not advertised. The bypass applies only to addresses explicitly provided via -externalip. This addresses the concern raised in #25690 (comment)

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

Note: -torcontrol and I2P SAM also call AddLocal() with LOCAL_MANUAL but do not set the flag. Whether those paths should also bypass -onlynet is a separate question and this PR therefore intentionally scopes the fix to -externalip only.

Fixes: #25336 Fixes: #25669

DrahtBot added the label P2P on Feb 9, 2026

DrahtBot commented at 12:17 PM on February 9, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	vasild
Concept ACK	chriszeng1010
Stale ACK	seduless

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:

#35032 (net_processing: don't modify addrman for private broadcast connections by vasild)
#35027 (net: use -bind address for outgoing connections 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 places where comparison-specific test macros should replace generic comparisons:

[test/functional/p2p_addr_selfannouncement.py] assert not onion_net["reachable"], "onion should not be reachable under -onlynet=ipv4" -> Prefer assert_equal(onion_net["reachable"], False, "onion should not be reachable under -onlynet=ipv4").

willcl-ark commented at 12:18 PM on February 9, 2026: member

cc @mzumsande @vasild

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

chriszeng1010 commented at 6:48 PM on March 2, 2026: none

Tested ACK

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

in src/test/net_tests.cpp:1569 in 383763f9ee

1564 | +    CAddress addr_onion;
1565 | +    BOOST_REQUIRE(addr_onion.SetSpecial("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"));
1566 | +    BOOST_REQUIRE(addr_onion.IsValid());
1567 | +    BOOST_REQUIRE(addr_onion.IsTor());
1568 | +
1569 | +    // Simulate using -onlynet=ipv4

seduless commented at 3:57 AM on March 3, 2026:

nit: This simulates -onlynet=ipv4 but doesn't replicate -externalip's side effect of setting fDiscover=false. With fDiscover left as true (the test default), only the g_reachable_nets gate is exercised. Adding fDiscover = false; here would catch a regression on the fDiscover gate too.

willcl-ark commented at 10:03 AM on March 3, 2026:

Thanks for the review. I agree, and have taken this suggestion in 6e73dcf700796146b33f3788b2ff7f659244f909

seduless commented at 4:03 AM on March 3, 2026: contributor

Tested ACK 383763f9ee3ca307c2b881f0fcf4de660073aa84

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.

DrahtBot requested review from chriszeng1010 on Mar 3, 2026

willcl-ark force-pushed on Mar 3, 2026

seduless commented at 11:24 PM on March 3, 2026: contributor

re-ACK 6e73dcf700796146b33f3788b2ff7f659244f909

in src/test/net_tests.cpp:1584 in 6e73dcf700

1579 | +    BOOST_CHECK(AddLocal(addr_onion, LOCAL_MANUAL));
1580 | +    BOOST_CHECK(IsLocal(addr_onion));
1581 | +
1582 | +    RemoveLocal(addr_onion);
1583 | +    g_reachable_nets.Add(NET_ONION);
1584 | +    fDiscover = true;

vasild commented at 11:09 AM on March 5, 2026:

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;

willcl-ark commented at 12:18 PM on March 5, 2026:

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

vasild approved

vasild commented at 11:27 AM on March 5, 2026: contributor

ACK 6e73dcf700796146b33f3788b2ff7f659244f909

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 https://github.com/bitcoin/bitcoin/issues/25669

willcl-ark force-pushed on Mar 5, 2026

willcl-ark commented at 12:19 PM on March 5, 2026: member

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

😃 Release note added in 3c63971454722a9ab399d49b1195fdd640867e31

vasild approved

vasild commented at 12:33 PM on March 5, 2026: contributor

ACK 3c63971454722a9ab399d49b1195fdd640867e31

DrahtBot requested review from seduless on Mar 5, 2026

vasild commented at 12:36 PM on March 5, 2026: contributor

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

seduless commented at 2:58 AM on March 10, 2026: contributor

re-ACK 3c63971454722a9ab399d49b1195fdd640867e31

sedited removed review request from chriszeng1010 on Mar 19, 2026

sedited requested review from ajtowns on Mar 19, 2026

sedited removed review request from chriszeng1010 on Mar 19, 2026

sedited requested review from ajtowns on Mar 19, 2026

ajtowns commented at 9:36 AM on March 20, 2026: contributor

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

DrahtBot requested review from chriszeng1010 on Mar 20, 2026

net: plumb an explicit -externalip flag through AddLocal

Prepare for letting `-externalip` bypass the reachable-net filter
without tying that behavior to `LOCAL_MANUAL` score.

Do that by passing an explicit `fExternalIP` bool through `AddLocal()`,
without changing behavior.

4587cbf39b

net: let -externalip bypass -onlynet

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.

Now that `AddLocal()` accepts an explicit `fExternalIP` flag, use that
to let only those addresses bypass the reachable-net filter.

db46d0527b

test: cover -externalip bypassing -onlynet

Add a unit test covering the `-onlynet`/`-externalip` interaction.

Check that an unreachable address with `LOCAL_MANUAL` still fails, while
the same address succeeds when marked as coming from `-externalip`.

c0ed7cc84d

test: cover -externalip bypassing -onlynet in functional test

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.

7b28ef4149

willcl-ark force-pushed on Mar 20, 2026

DrahtBot added the label Needs rebase on Apr 9, 2026

DrahtBot commented at 8:31 PM on April 9, 2026: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

in src/net.cpp:277 in 7b28ef4149

 273 | @@ -274,7 +274,7 @@ void ClearLocal()
 274 |  }
 275 |  
 276 |  // learn a new local address
 277 | -bool AddLocal(const CService& addr_, int nScore)
 278 | +bool AddLocal(const CService& addr_, int nScore, bool fExternalIP)

vasild commented at 1:58 PM on April 13, 2026:

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.

in src/net.cpp:305 in 7b28ef4149

 301 | @@ -302,9 +302,9 @@ bool AddLocal(const CService& addr_, int nScore)
 302 |      return true;
 303 |  }
 304 |  
 305 | -bool AddLocal(const CNetAddr &addr, int nScore)
 306 | +bool AddLocal(const CNetAddr& addr, int nScore, bool fExternalIP)

vasild commented at 2:01 PM on April 13, 2026:

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 approved

vasild commented at 2:02 PM on April 13, 2026: contributor

ACK 7b28ef41495f57d5b8f3fa67d467c16b0660b331

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

vasild's public key is on openpgp.org

</details>

DrahtBot requested review from seduless on Apr 13, 2026