Allow binding to port 0 for OS-assigned ports#860
Allow binding to port 0 for OS-assigned ports#860joostjager wants to merge 1 commit intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
joostjager
force-pushed
the
bind-port-zero
branch
from
526375e to
37defe0
Compare
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
While I know about the flakiness issue, I can't say I'm loving all this additional complexity for very little actual (read: production) gain. Do we really need this?
src/lib.rs
Outdated
| payment_store: Arc<PaymentStore>, | ||
| lnurl_auth: Arc<LnurlAuth>, | ||
| /// Addresses the node is actively listening on. Set on `start()`, cleared on `stop()`. | ||
| active_listening_addresses: Arc<RwLock<Option<Vec<SocketAddress>>>>, |
There was a problem hiding this comment.
If we add all of that extra logic, etc, could we move these fields and the logic to ConnectionManager or at least connection.rs? Seems it would be the appropriate place?
There was a problem hiding this comment.
Moved to ConnectionManager, but it doesn't seem completely logical.
There was a problem hiding this comment.
Moved it back. ConnectionManager didn't do anything with the field except for offering get and set.
We've discussed various options offline. Another one is to do a loop around node creation in tests only. |
joostjager
force-pushed
the
bind-port-zero
branch
2 times, most recently
from
ba72299 to
98323b4
Compare
joostjager
force-pushed
the
bind-port-zero
branch
5 times, most recently
from
a9c2ea6 to
0b9e04b
Compare
Add support for configuring listening addresses with port 0, letting the OS pick a free port. After binding, the actual port is resolved via local_addr() and stored in last_bound_addresses on Node, preserved across restarts so the node rebinds the same ports. Node::listening_addresses() returns the last bound addresses when available, falling back to configured addresses. The gossip broadcast task and announcement_addresses() prefer actual bound addresses over configured ones, so OS-assigned ports are correctly announced. Port 0 is only allowed under cfg(test). In production, the builder rejects it to prevent accidentally announcing ephemeral ports. Tests now use 127.0.0.1:0 instead of a deterministic port picker, eliminating potential port collisions between concurrent test runs. AI tools were used in preparing this commit.
joostjager
force-pushed
the
bind-port-zero
branch
from
0b9e04b to
bc6507d
Compare
|
Made the :0 test only, to prevent user errors. |
3 participants
Test port collision fix.