Skip to content

Commit 98323b4

Browse files
committed
Allow binding to port 0 for OS-assigned ports
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 a new last_bound_addresses field on ConnectionManager, 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() never expose port-0 or OS-assigned addresses, since those are ephemeral and change on restart. This eliminates the need for the deterministic port picker in tests, which was fragile due to potential port collisions. Tests now use 127.0.0.1:0 and query the actual port after start(). AI tools were used in preparing this commit.
1 parent 3aef2b3 commit 98323b4

File tree

5 files changed

+90
-34
lines changed

5 files changed

+90
-34
lines changed

src/config.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,16 @@ pub(crate) fn may_announce_channel(config: &Config) -> Result<(), AnnounceError>
331331
}
332332
}
333333

334+
pub(crate) fn has_port_zero(addr: &SocketAddress) -> bool {
335+
match addr {
336+
SocketAddress::TcpIpV4 { port, .. }
337+
| SocketAddress::TcpIpV6 { port, .. }
338+
| SocketAddress::OnionV3 { port, .. }
339+
| SocketAddress::Hostname { port, .. } => *port == 0,
340+
_ => false,
341+
}
342+
}
343+
334344
pub(crate) fn default_user_config(config: &Config) -> UserConfig {
335345
// Initialize the default config values.
336346
//

src/connection.rs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
use std::collections::hash_map::{self, HashMap};
99
use std::net::ToSocketAddrs;
1010
use std::ops::Deref;
11-
use std::sync::{Arc, Mutex};
11+
use std::sync::{Arc, Mutex, RwLock};
1212
use std::time::Duration;
1313

1414
use bitcoin::secp256k1::PublicKey;
@@ -29,6 +29,7 @@ where
2929
tor_proxy_config: Option<TorConfig>,
3030
keys_manager: Arc<KeysManager>,
3131
logger: L,
32+
last_bound_addresses: RwLock<Option<Vec<SocketAddress>>>,
3233
}
3334

3435
impl<L: Deref + Clone + Sync + Send> ConnectionManager<L>
@@ -40,8 +41,24 @@ where
4041
keys_manager: Arc<KeysManager>, logger: L,
4142
) -> Self {
4243
let pending_connections = Mutex::new(HashMap::new());
44+
let last_bound_addresses = RwLock::new(None);
4345

44-
Self { pending_connections, peer_manager, tor_proxy_config, keys_manager, logger }
46+
Self {
47+
pending_connections,
48+
peer_manager,
49+
tor_proxy_config,
50+
keys_manager,
51+
logger,
52+
last_bound_addresses,
53+
}
54+
}
55+
56+
pub(crate) fn set_last_bound_addresses(&self, addrs: Vec<SocketAddress>) {
57+
*self.last_bound_addresses.write().unwrap() = Some(addrs);
58+
}
59+
60+
pub(crate) fn last_bound_addresses(&self) -> Option<Vec<SocketAddress>> {
61+
self.last_bound_addresses.read().unwrap().clone()
4562
}
4663

4764
pub(crate) async fn connect_peer_if_necessary(

src/lib.rs

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ pub use builder::BuildError;
130130
pub use builder::NodeBuilder as Builder;
131131
use chain::ChainSource;
132132
use config::{
133-
default_user_config, may_announce_channel, AsyncPaymentsRole, ChannelConfig, Config,
134-
LNURL_AUTH_TIMEOUT_SECS, NODE_ANN_BCAST_INTERVAL, PEER_RECONNECTION_INTERVAL,
133+
default_user_config, has_port_zero, may_announce_channel, AsyncPaymentsRole, ChannelConfig,
134+
Config, LNURL_AUTH_TIMEOUT_SECS, NODE_ANN_BCAST_INTERVAL, PEER_RECONNECTION_INTERVAL,
135135
RGS_SYNC_INTERVAL,
136136
};
137137
use connection::ConnectionManager;
@@ -356,7 +356,12 @@ impl Node {
356356
);
357357
}
358358

359-
if let Some(listening_addresses) = &self.config.listening_addresses {
359+
let effective_listening_addresses = self
360+
.connection_manager
361+
.last_bound_addresses()
362+
.or_else(|| self.config.listening_addresses.clone());
363+
364+
if let Some(listening_addresses) = &effective_listening_addresses {
360365
// Setup networking
361366
let peer_manager_connection_handler = Arc::clone(&self.peer_manager);
362367
let listening_logger = Arc::clone(&self.logger);
@@ -378,14 +383,31 @@ impl Node {
378383
}
379384

380385
let logger = Arc::clone(&listening_logger);
381-
let listeners = self.runtime.block_on(async move {
386+
let (listeners, bound_addrs) = self.runtime.block_on(async move {
382387
let mut listeners = Vec::new();
388+
let mut bound_addrs = Vec::new();
383389

384-
// Try to bind to all addresses
385390
for addr in &*bind_addrs {
386391
match tokio::net::TcpListener::bind(addr).await {
387392
Ok(listener) => {
388-
log_trace!(logger, "Listener bound to {}", addr);
393+
let local_addr = listener.local_addr().map_err(|e| {
394+
log_error!(
395+
logger,
396+
"Failed to retrieve local address from listener: {}",
397+
e
398+
);
399+
Error::InvalidSocketAddress
400+
})?;
401+
let socket_address = match local_addr {
402+
std::net::SocketAddr::V4(a) => {
403+
SocketAddress::TcpIpV4 { addr: a.ip().octets(), port: a.port() }
404+
},
405+
std::net::SocketAddr::V6(a) => {
406+
SocketAddress::TcpIpV6 { addr: a.ip().octets(), port: a.port() }
407+
},
408+
};
409+
log_info!(logger, "Listening on {}", socket_address);
410+
bound_addrs.push(socket_address);
389411
listeners.push(listener);
390412
},
391413
Err(e) => {
@@ -400,9 +422,11 @@ impl Node {
400422
}
401423
}
402424

403-
Ok(listeners)
425+
Ok((listeners, bound_addrs))
404426
})?;
405427

428+
self.connection_manager.set_last_bound_addresses(bound_addrs);
429+
406430
for listener in listeners {
407431
let logger = Arc::clone(&listening_logger);
408432
let peer_mgr = Arc::clone(&peer_manager_connection_handler);
@@ -526,6 +550,11 @@ impl Node {
526550
let addresses = if let Some(announcement_addresses) = bcast_config.announcement_addresses.clone() {
527551
announcement_addresses
528552
} else if let Some(listening_addresses) = bcast_config.listening_addresses.clone() {
553+
if listening_addresses.iter().any(has_port_zero) {
554+
// Don't announce addresses that include port 0
555+
// since the OS-assigned port changes on restart.
556+
continue;
557+
}
529558
listening_addresses
530559
} else {
531560
debug_assert!(false, "We checked whether the node may announce, so listening addresses should always be set");
@@ -842,16 +871,25 @@ impl Node {
842871
}
843872

844873
/// Returns our own listening addresses.
874+
///
875+
/// If the node has been started, this returns the actual bound addresses (which may differ
876+
/// from the configured addresses if port 0 was used). Otherwise, this returns the configured
877+
/// addresses.
845878
pub fn listening_addresses(&self) -> Option<Vec<SocketAddress>> {
846-
self.config.listening_addresses.clone()
879+
self.connection_manager
880+
.last_bound_addresses()
881+
.or_else(|| self.config.listening_addresses.clone())
847882
}
848883

849884
/// Returns the addresses that the node will announce to the network.
885+
///
886+
/// Returns the configured announcement addresses if set, otherwise falls back to the
887+
/// configured listening addresses. Does not return OS-assigned addresses from port 0
888+
/// bindings, since those are ephemeral and change on restart.
850889
pub fn announcement_addresses(&self) -> Option<Vec<SocketAddress>> {
851-
self.config
852-
.announcement_addresses
853-
.clone()
854-
.or_else(|| self.config.listening_addresses.clone())
890+
self.config.announcement_addresses.clone().or_else(|| {
891+
self.config.listening_addresses.clone().filter(|a| !a.iter().any(has_port_zero))
892+
})
855893
}
856894

857895
/// Returns our node alias.

tests/common/mod.rs

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use std::collections::{HashMap, HashSet};
1414
use std::env;
1515
use std::future::Future;
1616
use std::path::PathBuf;
17-
use std::sync::atomic::{AtomicU16, Ordering};
1817
use std::sync::{Arc, RwLock};
1918
use std::time::Duration;
2019

@@ -269,16 +268,6 @@ pub(crate) fn random_storage_path() -> PathBuf {
269268
temp_path
270269
}
271270

272-
static NEXT_PORT: AtomicU16 = AtomicU16::new(20000);
273-
274-
pub(crate) fn generate_listening_addresses() -> Vec<SocketAddress> {
275-
let port = NEXT_PORT.fetch_add(2, Ordering::Relaxed);
276-
vec![
277-
SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port },
278-
SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: port + 1 },
279-
]
280-
}
281-
282271
pub(crate) fn random_node_alias() -> Option<NodeAlias> {
283272
let mut rng = rng();
284273
let rand_val = rng.random_range(0..1000);
@@ -302,7 +291,7 @@ pub(crate) fn random_config(anchor_channels: bool) -> TestConfig {
302291
println!("Setting random LDK storage dir: {}", rand_dir.display());
303292
node_config.storage_dir_path = rand_dir.to_str().unwrap().to_owned();
304293

305-
let listening_addresses = generate_listening_addresses();
294+
let listening_addresses = vec![SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: 0 }];
306295
println!("Setting LDK listening addresses: {:?}", listening_addresses);
307296
node_config.listening_addresses = Some(listening_addresses);
308297

tests/integration_tests_rust.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ use common::{
2121
expect_channel_pending_event, expect_channel_ready_event, expect_channel_ready_events,
2222
expect_event, expect_payment_claimable_event, expect_payment_received_event,
2323
expect_payment_successful_event, expect_splice_pending_event, generate_blocks_and_wait,
24-
generate_listening_addresses, open_channel, open_channel_push_amt, open_channel_with_all,
25-
premine_and_distribute_funds, premine_blocks, prepare_rbf, random_chain_source, random_config,
26-
setup_bitcoind_and_electrsd, setup_builder, setup_node, setup_two_nodes, splice_in_with_all,
27-
wait_for_tx, TestChainSource, TestStoreType, TestSyncStore,
24+
open_channel, open_channel_push_amt, open_channel_with_all, premine_and_distribute_funds,
25+
premine_blocks, prepare_rbf, random_chain_source, random_config, setup_bitcoind_and_electrsd,
26+
setup_builder, setup_node, setup_two_nodes, splice_in_with_all, wait_for_tx, TestChainSource,
27+
TestStoreType, TestSyncStore,
2828
};
2929
use electrsd::corepc_node::Node as BitcoinD;
3030
use electrsd::ElectrsD;
@@ -37,6 +37,7 @@ use ldk_node::payment::{
3737
};
3838
use ldk_node::{Builder, Event, NodeError};
3939
use lightning::ln::channelmanager::PaymentId;
40+
use lightning::ln::msgs::SocketAddress;
4041
use lightning::routing::gossip::{NodeAlias, NodeId};
4142
use lightning::routing::router::RouteParametersConfig;
4243
use lightning_invoice::{Bolt11InvoiceDescription, Description};
@@ -1431,9 +1432,11 @@ async fn test_node_announcement_propagation() {
14311432
node_a_alias_bytes[..node_a_alias_string.as_bytes().len()]
14321433
.copy_from_slice(node_a_alias_string.as_bytes());
14331434
let node_a_node_alias = Some(NodeAlias(node_a_alias_bytes));
1434-
let node_a_announcement_addresses = generate_listening_addresses();
1435+
let node_a_announcement_addresses = vec![
1436+
SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: 10001 },
1437+
SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: 10002 },
1438+
];
14351439
config_a.node_config.node_alias = node_a_node_alias.clone();
1436-
config_a.node_config.listening_addresses = Some(generate_listening_addresses());
14371440
config_a.node_config.announcement_addresses = Some(node_a_announcement_addresses.clone());
14381441

14391442
// Node B will only use listening addresses
@@ -1443,9 +1446,7 @@ async fn test_node_announcement_propagation() {
14431446
node_b_alias_bytes[..node_b_alias_string.as_bytes().len()]
14441447
.copy_from_slice(node_b_alias_string.as_bytes());
14451448
let node_b_node_alias = Some(NodeAlias(node_b_alias_bytes));
1446-
let node_b_listening_addresses = generate_listening_addresses();
14471449
config_b.node_config.node_alias = node_b_node_alias.clone();
1448-
config_b.node_config.listening_addresses = Some(node_b_listening_addresses.clone());
14491450
config_b.node_config.announcement_addresses = None;
14501451

14511452
let node_a = setup_node(&chain_source, config_a);
@@ -1505,6 +1506,7 @@ async fn test_node_announcement_propagation() {
15051506
#[cfg(feature = "uniffi")]
15061507
assert_eq!(node_b_announcement_info.alias, node_b_alias_string);
15071508

1509+
let node_b_listening_addresses = node_b.listening_addresses().unwrap();
15081510
#[cfg(not(feature = "uniffi"))]
15091511
assert_eq!(node_b_announcement_info.addresses(), &node_b_listening_addresses);
15101512
#[cfg(feature = "uniffi")]

0 commit comments

Comments
 (0)