net: Fix Discover() not running when using -bind=0.0.0.0:port #32757

b-l-u-e · 2025-06-16T04:28:44Z

This PR fixes two related issues with address discovery when using explicit bind addresses in Bitcoin Core:

When using -bind=0.0.0.0:port (or -bind=::), the Discover() function was not being executed because the code only checked the bind_on_any flag. This led to two problems:

The node would not discover its own local addresses if an explicit "any" bind was used.
The functional test feature_bind_port_discover.py would fail, as it expects local addresses to be discovered in these cases.

This PR:

Checks both bind_on_any and any bind addresses using IsBindAny()
Ensures Discover() runs when binding to 0.0.0.0 or ::, even if specified explicitly.
Ensures correct address discovery
The node will now discover its own addresses when using explicit "any" binds, matching user expectations and fixing the test.
Maintains backward compatibility
The semantic meaning of bind_on_any is preserved as defined in net.h:

"True if the user did not specify -bind= or -whitebind= and thus we should bind on 0.0.0.0 (IPv4) and :: (IPv6)"
Updates the test to use dynamic ports
The functional test feature_bind_port_discover.py is updated to use dynamic ports instead of hardcoded ones, improving reliability.

How this PR Differs from #31492

Preserves the semantic meaning of bind_on_any (see net.h).
Uses a simpler approach: checks IsBindAny() on bind addresses, without modifying GetListenPort().
Avoids code duplication with DefaultOnionServiceTarget().

References

Implementation follows the approach proposed in my review comment on #31492.

Closes #31293

The #31336 has the overall test failure and requires both this PR and #33362 to be fully resolved.

DrahtBot · 2025-06-16T04:28:47Z

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Stale ACK	PeterWrighten, yuvicc, pinheadmz, mzumsande, vasild, frankomosh

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

Conflicts

No conflicts as of last run.

yuvicc · 2025-06-16T15:40:07Z

Concept ACK

Removing duplicate code from DefaultOnionServiceTarget() is better IMO

PeterWrighten

utACK 776b7ee
I consider you could report error message when running in original codebase, and in comparison with this PR one. Or you can find related issue and declare to fix them, it could be clearer for reviewers.

yuvicc

lgtm ACK 776b7ee5855f7572798eff3014030bcc53aa7393

src/init.cpp

vasild

Approach ACK 776b7ee585

src/init.cpp

test/functional/feature_bind_port_discover.py

b-l-u-e · 2025-07-04T06:50:04Z

vasild thanks for the suggestions...about the self.extra_args configuration.

In your patch, node 2 only has -whitebind=0.0.0.0 without any onion bind:

['-discover', f'-whitebind=0.0.0.0:{self.bind_ports[2]}'], # Explicit -whitebind=0.0.0.0

However, when I tried this configuration, I encountered port conflicts because the test framework automatically adds an onion bind to node 2, which conflicts with the port already used by node 1.

Test Results:

Without dummy onion bind (your suggested configuration):

Error: Unable to bind to 127.0.0.1:12866 on this computer. Bitcoin Core is probably already running.
Error: Failed to listen on any port. Use -listen=0 if you want this.

With dummy onion bind current implementation:

TestFramework (INFO): Tests successful

The issue is that when using only -whitebind=0.0.0.0, the test framework automatically adds an onion bind to the same node, causing a port conflict with other nodes in the test.

To avoid this, I added a dummy onion bind:

['-discover', f'-whitebind=0.0.0.0:{self.bind_ports[2]}', f'-bind=127.0.0.1:{self.bind_ports[2]+100}=onion']

This ensures the test runs successfully while still testing the intended -whitebind=0.0.0.0 functionality.

DrahtBot · 2025-07-04T06:56:41Z

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/runs/45342616996}
_{LLM reason (✨ experimental): The CI failure is caused by lint errors due to trailing whitespace and missing trailing newline in the code.}

Hints

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.

pinheadmz

ACK ad3f60b0ea59dbbe7aedcfdcca1caddefffe6976

Built and tested on macos/arm64 and debian/x86. Reviewed code changes which are minimal, this PR searches bind settings for 0.0.0.0 specifically to call Discover() which attempts to add local addresses for peer advertising. Also adds test coverage using 1.1.1.1 and 2.2.2.2 which I was able to set up and check locally on both platforms. Also tested on main.

I would really like to know if its possible to run this test in CI as vasild suggested here and if so, should probably be added to this PR

Show Signature

pinheadmz's public key is on openpgp.org

test/functional/feature_bind_port_discover.py

pinheadmz

Thanks for fixing the nit in the test.

In general, nit-fixes should be squashed (not added as an extra commit) so the final commit history is clean and perfect ;-)

Also I think you rebased on master which is ok, but it makes review slightly harder because my local copy of your branch is now dozens of commits different from the current state of the PR. I used git range-diff to isolate the changes which is why its not like, a super hard rule, but again in general, try not to rebase on master unless you have to (to fix a conflict, include other merged commits, etc)

test/functional/feature_bind_port_discover.py

vasild

Almost ACK a9b7542b6ea1af770d93cbbf5bb55dae6841334f

I encountered port conflicts because the test framework automatically adds an onion bind to node 2,

Hmm, I think it is not the test framework but bitcoind itself that adds the onion bind. To figure out how bitcoind is started, e.g. for nodes[2] I look at /tmp/bitcoin_func_test_.../node2/regtest/debug.log for the lines like:

...
Config file arg: [regtest] shrinkdebugfile="0"
Config file arg: [regtest] unsafesqlitesync="1"
Command-line arg: bind="127.0.0.1:23642=onion"
Command-line arg: datadir="/tmp/bitcoin_func_test_h7qvveos/node2"
...

which conflicts with the port already used by node 1

Are you sure it is nodes[1] and nodes[2] conflicting? What is the error message? What is the output of ./test/functional/combine_logs.py |grep 'Bound to'? I got nodes[2] to bind the whitebind and tor on the same port, see my comment below.

test/functional/feature_bind_port_discover.py

vasild

ACK 1dc6b1e8b04363107b4b44cbf42f8a25f2efabfe

Thank you!

pinheadmz

ACK 1dc6b1e8b04363107b4b44cbf42f8a25f2efabfe

Built and tested on macos/arm64. Reviewed code, minimal changes since last ACK, addressing clean-ups in the test. Tested on mainnet and compared against master to check that local address was discovered when -bind=0.0.0.0

Show Signature

pinheadmz's public key is on openpgp.org

mzumsande

Does the changed test succeed for others? I tried to follow the instructions and get

 test  2025-07-23T21:50:01.787971Z TestFramework (ERROR): Address 1.1.1.1 not found in node0's local addresses: [] 
 test  2025-07-23T21:50:01.788044Z TestFramework (ERROR): Assertion failed 
                                   Traceback (most recent call last):
                                     File "XXX/bitcoin/32757_bind0000/test/functional/test_framework/test_framework.py", line 195, in main
                                       self.run_test()
                                     File "XXX/32757_bind0000/build/test/functional/feature_bind_port_discover.py", line 97, in run_test
                                       assert False

It also fails on master with the different error assert found_addr1 like #31336 describes.
As an aside, ifconfig is deprecated by many linux versions and should probably be replaced by ip (though not necessary to do it here).

[Edit]: Using Ubuntu 24.04.2 LTS

pinheadmz · 2025-07-23T21:58:36Z

Test worked for me on mac and failed as expected without 1.1.1.1 and 2.2.2.2

On my system the command was
ifconfig en0 alias 1.1.1.1

mzumsande · 2025-07-24T15:58:06Z

ifconfig en0 alias 1.1.1.1

ok, you don't add it to the loopback interface.
But the instructions for Linux
ifconfig lo:0 1.1.1.1/32 up && ifconfig lo:1 2.2.2.2/32 up # to set up
do that, while IFF_LOOPBACK addresses are being filtered out here. So I don't think the problem from #31336 is fixed by this as the OP says.

achow101 · 2025-07-24T22:36:18Z

Also seeing @mzumsande's observation if I add the addresses to loopback, but not if I add them to a different interface.

I think it's reasonable to filter out all loopback addresses, so the resolution is probably to just update the instructions to have the addresses bound to a non-loopback interface.

pinheadmz · 2025-07-24T22:51:15Z

If @b-l-u-e is going to retouch to update the instructions in the test comment I'd also really like to see those instructions executed in a CI test as well.

mzumsande · 2025-07-24T22:59:27Z

For me it would also be ok to leave the resolution of #31336 for a separate PR - but this PR shouldn't claim to solve the issue then.

b-l-u-e · 2025-07-25T05:23:34Z

found the issues raised and found the cause.

The test was failing because the instructions were wrong. The original instructions told users to use loopback interfaces (lo:0, lo:1), but the code filters out loopback addresses (line 340 in netif.cpp).

This is what issue #31336 reported - users following the instructions got empty local addresses.

mzumsande: identified that the test was failing with "Address 1.1.1.1 not found in node0's local addresses: []" and noticed that ifconfig is deprecated. he pointed out that the loopback filtering means this doesn't actually fix #31336.

pinheadmz: confirmed the test worked on macOS with non-loopback interfaces (ifconfig en0 alias 1.1.1.1) and failed as expected without the addresses.

achow101: confirmed mzumsande observation that loopback addresses don't work, but non-loopback addresses do work and suggested updating the instructions to use non-loopback interfaces.

I tested both scenarios

Non-loopback interface (wlo1): Test passes successfully
Loopback interface (lo): Test fails as expected (validates your concerns)

This makes the loopback filtering working as intended.

updated the test instructions to be more educational and flexible

Uses ip commands for Linux
Uses placeholder names instead of assuming specific interfaces
Provides instructions for Linux, macOS, and FreeBSD
Explains why loopback interfaces don't work

regarding #31336
This doesn't fix the core loopback filtering issue in #31336. However, it does fix the immediate problem where users following the test instructions get confused. The test works perfectly when users follow the correct instructions.

The test logic itself is correct - the problem was jthe instructions were wrong.

Zeegaths · 2025-07-27T22:14:41Z

I ran these commands on Ubuntu 22.04:

sudo ip addr add 1.1.1.1/32 dev wlo1
sudo ip addr add 2.2.2.2/32 dev wlo1

./build/bin/bitcoind -regtest -bind=0.0.0.0:18444 -discover -daemon
./build/bin/bitcoin-cli -regtest getnetworkinfo | grep -A 10 "localaddresses"

Result on master:
"localaddresses": [
],
"warnings": [
"This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"
]

Result on PR:
"localaddresses": [
{
"address": "1.1.1.1",
"port": 18444,
"score": 1
},
{
"address": "2.2.2.2",
"port": 18444,
"score": 1
}

Conclusion:
PR successfully fixes the issue described in #31293. When using explicit -bind=0.0.0.0, the Discover() function now correctly runs and discovers local addresses, whereas on master it returns empty localaddresses.

vasild

ACK 210dd40

It suffices to prefix only the first line of the commit message with "test:".

test/functional/feature_bind_port_discover.py

vasild · 2025-09-11T07:58:56Z

I would really like to know if its possible to run this test in CI ...

IFF_LOOPBACK addresses are being filtered out here. So I don't think the problem from #31336 is fixed by this as the OP says.

Both resolved in #33362

b-l-u-e · 2025-11-03T19:02:14Z

Are you still working on this?

Yes am still working on this
Caught up on something else

vasild

ACK e0463f3

frankomosh

Code Review ACK e0463f3

Maybe you could clarify the PR description to reflect that it closes #31293, and #31336 would only be fully resolved with #33362. I think this would accurately reflect that #31336 requires both PRs.

src/init.cpp

Signed-off-by: b-l-u-e <[email protected]>

…dd comprehensive test coverage Signed-off-by: b-l-u-e <[email protected]>

DrahtBot added the P2P label Jun 16, 2025

DrahtBot mentioned this pull request Jun 16, 2025

docs: adds correct updated documentation links #32699

Closed

b-l-u-e force-pushed the net-fix-discover-bind-any branch from 5d5bf98 to b17208d Compare June 16, 2025 07:19

b-l-u-e requested a review from maflcko June 16, 2025 08:15

This was referenced Jun 16, 2025

index: store per-block transaction locations for efficient lookups #32541

Closed

Bugfix: Correct first-run free space checks #29678

Open

index: initial sync speedup, parallelize process #26966

Open

b-l-u-e force-pushed the net-fix-discover-bind-any branch from b17208d to 776b7ee Compare June 16, 2025 16:23

PeterWrighten reviewed Jun 20, 2025

View reviewed changes

yuvicc mentioned this pull request Jun 20, 2025

Execute Discover() when bind=0.0.0.0 or :: is set #31492

Closed

yuvicc reviewed Jun 20, 2025

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

vasild reviewed Jun 30, 2025

View reviewed changes

src/init.cpp Show resolved Hide resolved

test/functional/feature_bind_port_discover.py Show resolved Hide resolved

b-l-u-e force-pushed the net-fix-discover-bind-any branch from 776b7ee to 99f8a6b Compare July 4, 2025 05:45

DrahtBot added the CI failed label Jul 4, 2025

DrahtBot removed the CI failed label Jul 4, 2025

b-l-u-e force-pushed the net-fix-discover-bind-any branch from 4fdc0b1 to ad3f60b Compare July 4, 2025 10:43

b-l-u-e requested a review from vasild July 4, 2025 10:55

pinheadmz approved these changes Jul 15, 2025

View reviewed changes

test/functional/feature_bind_port_discover.py Show resolved Hide resolved

DrahtBot requested review from PeterWrighten and yuvicc July 15, 2025 14:57

b-l-u-e force-pushed the net-fix-discover-bind-any branch from ad3f60b to 894fc0c Compare July 16, 2025 06:11

pinheadmz reviewed Jul 16, 2025

View reviewed changes

test/functional/feature_bind_port_discover.py Outdated Show resolved Hide resolved

b-l-u-e force-pushed the net-fix-discover-bind-any branch 2 times, most recently from 57588f7 to a9b7542 Compare July 17, 2025 00:23

b-l-u-e requested a review from pinheadmz July 17, 2025 10:14

vasild reviewed Jul 22, 2025

View reviewed changes

test/functional/feature_bind_port_discover.py Outdated Show resolved Hide resolved

b-l-u-e force-pushed the net-fix-discover-bind-any branch from 8613ebd to 1dc6b1e Compare July 22, 2025 16:39

vasild approved these changes Jul 23, 2025

View reviewed changes

pinheadmz approved these changes Jul 23, 2025

View reviewed changes

mzumsande reviewed Jul 23, 2025

View reviewed changes

b-l-u-e force-pushed the net-fix-discover-bind-any branch 2 times, most recently from 51f8d08 to 210dd40 Compare July 25, 2025 05:19

vasild approved these changes Sep 9, 2025

View reviewed changes

DrahtBot requested a review from pinheadmz September 9, 2025 08:47

vasild reviewed Sep 9, 2025

View reviewed changes

test/functional/feature_bind_port_discover.py Outdated Show resolved Hide resolved

vasild mentioned this pull request Sep 11, 2025

Run feature_bind_port_(discover|externalip).py in CI #33362

Open

DrahtBot requested a review from mzumsande December 7, 2025 22:06

b-l-u-e force-pushed the net-fix-discover-bind-any branch from 210dd40 to e0463f3 Compare December 7, 2025 22:53

b-l-u-e requested a review from vasild December 7, 2025 22:56

vasild approved these changes Dec 9, 2025

View reviewed changes

frankomosh reviewed Dec 29, 2025

View reviewed changes

src/init.cpp Show resolved Hide resolved

b-l-u-e force-pushed the net-fix-discover-bind-any branch from e0463f3 to 8c5f2b4 Compare January 5, 2026 17:59

b-l-u-e added 2 commits January 5, 2026 21:57

net: Fix Discover() not running when using -bind=0.0.0.0:port

2577ab0

Signed-off-by: b-l-u-e <[email protected]>

test: Update feature_bind_port_discover.py to use dynamic ports and a…

37855ae

…dd comprehensive test coverage Signed-off-by: b-l-u-e <[email protected]>

b-l-u-e force-pushed the net-fix-discover-bind-any branch from 8c5f2b4 to 37855ae Compare January 5, 2026 19:00

b-l-u-e requested a review from frankomosh January 5, 2026 19:06

DrahtBot added the CI failed label Jan 11, 2026

net: Fix Discover() not running when using -bind=0.0.0.0:port #32757

Are you sure you want to change the base?

net: Fix Discover() not running when using -bind=0.0.0.0:port #32757

Conversation

b-l-u-e commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

yuvicc commented Jun 16, 2025

Uh oh!

PeterWrighten left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuvicc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

b-l-u-e commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 4, 2025

Uh oh!

pinheadmz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pinheadmz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

pinheadmz left a comment

Choose a reason for hiding this comment

Uh oh!

mzumsande left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pinheadmz commented Jul 23, 2025

Uh oh!

mzumsande commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

achow101 commented Jul 24, 2025

Uh oh!

pinheadmz commented Jul 24, 2025

Uh oh!

mzumsande commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

b-l-u-e commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zeegaths commented Jul 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vasild commented Sep 11, 2025

Uh oh!

b-l-u-e commented Nov 3, 2025

b-l-u-e commented Jun 16, 2025 •

edited

Loading

DrahtBot commented Jun 16, 2025 •

edited

Loading

PeterWrighten left a comment •

edited

Loading

b-l-u-e commented Jul 4, 2025 •

edited

Loading

mzumsande left a comment •

edited

Loading

mzumsande commented Jul 24, 2025 •

edited

Loading

mzumsande commented Jul 24, 2025 •

edited

Loading

b-l-u-e commented Jul 25, 2025 •

edited

Loading

Zeegaths commented Jul 27, 2025 •

edited

Loading