diff --git a/src/config.rs b/src/config.rs index 71e4d2314..04f562a9f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -331,6 +331,16 @@ pub(crate) fn may_announce_channel(config: &Config) -> Result<(), AnnounceError> } } +pub(crate) fn has_port_zero(addr: &SocketAddress) -> bool { + match addr { + SocketAddress::TcpIpV4 { port, .. } + | SocketAddress::TcpIpV6 { port, .. } + | SocketAddress::OnionV3 { port, .. } + | SocketAddress::Hostname { port, .. } => *port == 0, + _ => false, + } +} + pub(crate) fn default_user_config(config: &Config) -> UserConfig { // Initialize the default config values. // diff --git a/src/connection.rs b/src/connection.rs index 9110ed0d9..16f8713f3 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -8,7 +8,7 @@ use std::collections::hash_map::{self, HashMap}; use std::net::ToSocketAddrs; use std::ops::Deref; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, RwLock}; use std::time::Duration; use bitcoin::secp256k1::PublicKey; @@ -29,6 +29,7 @@ where tor_proxy_config: Option, keys_manager: Arc, logger: L, + last_bound_addresses: RwLock>>, } impl ConnectionManager @@ -40,8 +41,24 @@ where keys_manager: Arc, logger: L, ) -> Self { let pending_connections = Mutex::new(HashMap::new()); + let last_bound_addresses = RwLock::new(None); - Self { pending_connections, peer_manager, tor_proxy_config, keys_manager, logger } + Self { + pending_connections, + peer_manager, + tor_proxy_config, + keys_manager, + logger, + last_bound_addresses, + } + } + + pub(crate) fn set_last_bound_addresses(&self, addrs: Vec) { + *self.last_bound_addresses.write().unwrap() = Some(addrs); + } + + pub(crate) fn last_bound_addresses(&self) -> Option> { + self.last_bound_addresses.read().unwrap().clone() } pub(crate) async fn connect_peer_if_necessary( diff --git a/src/lib.rs b/src/lib.rs index 2e02e996c..609169a4d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -130,8 +130,8 @@ pub use builder::BuildError; pub use builder::NodeBuilder as Builder; use chain::ChainSource; use config::{ - default_user_config, may_announce_channel, AsyncPaymentsRole, ChannelConfig, Config, - LNURL_AUTH_TIMEOUT_SECS, NODE_ANN_BCAST_INTERVAL, PEER_RECONNECTION_INTERVAL, + default_user_config, has_port_zero, may_announce_channel, AsyncPaymentsRole, ChannelConfig, + Config, LNURL_AUTH_TIMEOUT_SECS, NODE_ANN_BCAST_INTERVAL, PEER_RECONNECTION_INTERVAL, RGS_SYNC_INTERVAL, }; use connection::ConnectionManager; @@ -356,7 +356,12 @@ impl Node { ); } - if let Some(listening_addresses) = &self.config.listening_addresses { + let effective_listening_addresses = self + .connection_manager + .last_bound_addresses() + .or_else(|| self.config.listening_addresses.clone()); + + if let Some(listening_addresses) = &effective_listening_addresses { // Setup networking let peer_manager_connection_handler = Arc::clone(&self.peer_manager); let listening_logger = Arc::clone(&self.logger); @@ -378,14 +383,31 @@ impl Node { } let logger = Arc::clone(&listening_logger); - let listeners = self.runtime.block_on(async move { + let (listeners, bound_addrs) = self.runtime.block_on(async move { let mut listeners = Vec::new(); + let mut bound_addrs = Vec::new(); - // Try to bind to all addresses for addr in &*bind_addrs { match tokio::net::TcpListener::bind(addr).await { Ok(listener) => { - log_trace!(logger, "Listener bound to {}", addr); + let local_addr = listener.local_addr().map_err(|e| { + log_error!( + logger, + "Failed to retrieve local address from listener: {}", + e + ); + Error::InvalidSocketAddress + })?; + let socket_address = match local_addr { + std::net::SocketAddr::V4(a) => { + SocketAddress::TcpIpV4 { addr: a.ip().octets(), port: a.port() } + }, + std::net::SocketAddr::V6(a) => { + SocketAddress::TcpIpV6 { addr: a.ip().octets(), port: a.port() } + }, + }; + log_info!(logger, "Listening on {}", socket_address); + bound_addrs.push(socket_address); listeners.push(listener); }, Err(e) => { @@ -400,9 +422,11 @@ impl Node { } } - Ok(listeners) + Ok((listeners, bound_addrs)) })?; + self.connection_manager.set_last_bound_addresses(bound_addrs); + for listener in listeners { let logger = Arc::clone(&listening_logger); let peer_mgr = Arc::clone(&peer_manager_connection_handler); @@ -526,6 +550,11 @@ impl Node { let addresses = if let Some(announcement_addresses) = bcast_config.announcement_addresses.clone() { announcement_addresses } else if let Some(listening_addresses) = bcast_config.listening_addresses.clone() { + if listening_addresses.iter().any(has_port_zero) { + // Don't announce addresses that include port 0 + // since the OS-assigned port changes on restart. + continue; + } listening_addresses } else { debug_assert!(false, "We checked whether the node may announce, so listening addresses should always be set"); @@ -842,16 +871,25 @@ impl Node { } /// Returns our own listening addresses. + /// + /// If the node has been started, this returns the actual bound addresses (which may differ + /// from the configured addresses if port 0 was used). Otherwise, this returns the configured + /// addresses. pub fn listening_addresses(&self) -> Option> { - self.config.listening_addresses.clone() + self.connection_manager + .last_bound_addresses() + .or_else(|| self.config.listening_addresses.clone()) } /// Returns the addresses that the node will announce to the network. + /// + /// Returns the configured announcement addresses if set, otherwise falls back to the + /// configured listening addresses. Does not return OS-assigned addresses from port 0 + /// bindings, since those are ephemeral and change on restart. pub fn announcement_addresses(&self) -> Option> { - self.config - .announcement_addresses - .clone() - .or_else(|| self.config.listening_addresses.clone()) + self.config.announcement_addresses.clone().or_else(|| { + self.config.listening_addresses.clone().filter(|a| !a.iter().any(has_port_zero)) + }) } /// Returns our node alias. diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 4f68f9825..0f06bdc25 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -14,7 +14,6 @@ use std::collections::{HashMap, HashSet}; use std::env; use std::future::Future; use std::path::PathBuf; -use std::sync::atomic::{AtomicU16, Ordering}; use std::sync::{Arc, RwLock}; use std::time::Duration; @@ -269,16 +268,6 @@ pub(crate) fn random_storage_path() -> PathBuf { temp_path } -static NEXT_PORT: AtomicU16 = AtomicU16::new(20000); - -pub(crate) fn generate_listening_addresses() -> Vec { - let port = NEXT_PORT.fetch_add(2, Ordering::Relaxed); - vec![ - SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port }, - SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: port + 1 }, - ] -} - pub(crate) fn random_node_alias() -> Option { let mut rng = rng(); let rand_val = rng.random_range(0..1000); @@ -302,7 +291,7 @@ pub(crate) fn random_config(anchor_channels: bool) -> TestConfig { println!("Setting random LDK storage dir: {}", rand_dir.display()); node_config.storage_dir_path = rand_dir.to_str().unwrap().to_owned(); - let listening_addresses = generate_listening_addresses(); + let listening_addresses = vec![SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: 0 }]; println!("Setting LDK listening addresses: {:?}", listening_addresses); node_config.listening_addresses = Some(listening_addresses); diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 413b2d44a..70a029af1 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -21,10 +21,10 @@ use common::{ expect_channel_pending_event, expect_channel_ready_event, expect_channel_ready_events, expect_event, expect_payment_claimable_event, expect_payment_received_event, expect_payment_successful_event, expect_splice_pending_event, generate_blocks_and_wait, - generate_listening_addresses, open_channel, open_channel_push_amt, open_channel_with_all, - premine_and_distribute_funds, premine_blocks, prepare_rbf, random_chain_source, random_config, - setup_bitcoind_and_electrsd, setup_builder, setup_node, setup_two_nodes, splice_in_with_all, - wait_for_tx, TestChainSource, TestStoreType, TestSyncStore, + open_channel, open_channel_push_amt, open_channel_with_all, premine_and_distribute_funds, + premine_blocks, prepare_rbf, random_chain_source, random_config, setup_bitcoind_and_electrsd, + setup_builder, setup_node, setup_two_nodes, splice_in_with_all, wait_for_tx, TestChainSource, + TestStoreType, TestSyncStore, }; use electrsd::corepc_node::Node as BitcoinD; use electrsd::ElectrsD; @@ -37,6 +37,7 @@ use ldk_node::payment::{ }; use ldk_node::{Builder, Event, NodeError}; use lightning::ln::channelmanager::PaymentId; +use lightning::ln::msgs::SocketAddress; use lightning::routing::gossip::{NodeAlias, NodeId}; use lightning::routing::router::RouteParametersConfig; use lightning_invoice::{Bolt11InvoiceDescription, Description}; @@ -1431,22 +1432,25 @@ async fn test_node_announcement_propagation() { node_a_alias_bytes[..node_a_alias_string.as_bytes().len()] .copy_from_slice(node_a_alias_string.as_bytes()); let node_a_node_alias = Some(NodeAlias(node_a_alias_bytes)); - let node_a_announcement_addresses = generate_listening_addresses(); + let node_a_announcement_addresses = vec![ + SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: 10001 }, + SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: 10002 }, + ]; config_a.node_config.node_alias = node_a_node_alias.clone(); - config_a.node_config.listening_addresses = Some(generate_listening_addresses()); config_a.node_config.announcement_addresses = Some(node_a_announcement_addresses.clone()); - // Node B will only use listening addresses let mut config_b = random_config(true); let node_b_alias_string = "ldk-node-b".to_string(); let mut node_b_alias_bytes = [0u8; 32]; node_b_alias_bytes[..node_b_alias_string.as_bytes().len()] .copy_from_slice(node_b_alias_string.as_bytes()); let node_b_node_alias = Some(NodeAlias(node_b_alias_bytes)); - let node_b_listening_addresses = generate_listening_addresses(); + let node_b_announcement_addresses = vec![ + SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: 20001 }, + SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: 20002 }, + ]; config_b.node_config.node_alias = node_b_node_alias.clone(); - config_b.node_config.listening_addresses = Some(node_b_listening_addresses.clone()); - config_b.node_config.announcement_addresses = None; + config_b.node_config.announcement_addresses = Some(node_b_announcement_addresses.clone()); let node_a = setup_node(&chain_source, config_a); let node_b = setup_node(&chain_source, config_b); @@ -1506,9 +1510,9 @@ async fn test_node_announcement_propagation() { assert_eq!(node_b_announcement_info.alias, node_b_alias_string); #[cfg(not(feature = "uniffi"))] - assert_eq!(node_b_announcement_info.addresses(), &node_b_listening_addresses); + assert_eq!(node_b_announcement_info.addresses(), &node_b_announcement_addresses); #[cfg(feature = "uniffi")] - assert_eq!(node_b_announcement_info.addresses, node_b_listening_addresses); + assert_eq!(node_b_announcement_info.addresses, node_b_announcement_addresses); } #[tokio::test(flavor = "multi_thread", worker_threads = 1)]