From a51c2bddf8e95265a721b139caad1ca236d97d8b Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 26 Mar 2026 10:08:21 +0100 Subject: [PATCH] Use deterministic port allocation in integration tests Integration tests allocate listening ports for test nodes by binding to 127.0.0.1:0, reading the OS-assigned port, then immediately dropping the socket. Later, Node::start() tries to bind to that same port. This has two problems: Parallel test collisions: with 35 tests running concurrently, each creating 2-4 nodes with 2 ports each, the OS can reassign a freed port to another test's node before the original node calls start(). This is a classic TOCTOU race. In CI, it caused ~50% of Rust test runs to fail with InvalidSocketAddress, always exactly one random test per run. Restart instability: when a test stops and restarts a node, the port is released during stop() and must be re-acquired during start(). Another test's node can grab it in between. The peer store still has the old address, so auto-reconnection also fails. Both problems are inherent to any scheme that allocates a port and later releases it. The only way to guarantee a port stays yours is to never release it, or to never share the port space. A deterministic atomic counter starting at a fixed base port (20000) solves both problems: each fetch_add returns a unique value, so no two nodes in the same process ever get the same port, and ports are stable across restarts because the same node keeps the same config. There is no external contention because CI runners are isolated. AI tools were used in preparing this commit. --- tests/common/mod.rs | 24 +++++++++++------------- tests/integration_tests_rust.rs | 10 +++++----- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 7854a77f2..69f9cc8d5 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -14,6 +14,7 @@ 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; @@ -268,17 +269,14 @@ pub(crate) fn random_storage_path() -> PathBuf { temp_path } -pub(crate) fn random_listening_addresses() -> Vec { - let num_addresses = 2; - let mut listening_addresses = HashSet::new(); +static NEXT_PORT: AtomicU16 = AtomicU16::new(20000); - while listening_addresses.len() < num_addresses { - let socket = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - let address: SocketAddress = socket.local_addr().unwrap().into(); - listening_addresses.insert(address); - } - - listening_addresses.into_iter().collect() +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 { @@ -304,9 +302,9 @@ 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 rand_listening_addresses = random_listening_addresses(); - println!("Setting random LDK listening addresses: {:?}", rand_listening_addresses); - node_config.listening_addresses = Some(rand_listening_addresses); + let listening_addresses = generate_listening_addresses(); + println!("Setting LDK listening addresses: {:?}", listening_addresses); + node_config.listening_addresses = Some(listening_addresses); let alias = random_node_alias(); println!("Setting random LDK node alias: {:?}", alias); diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 3fde52dc4..25f243c41 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -21,8 +21,8 @@ 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, - open_channel, open_channel_push_amt, open_channel_with_all, premine_and_distribute_funds, - premine_blocks, prepare_rbf, random_chain_source, random_config, random_listening_addresses, + 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, }; @@ -1429,9 +1429,9 @@ 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 = random_listening_addresses(); + let node_a_announcement_addresses = generate_listening_addresses(); config_a.node_config.node_alias = node_a_node_alias.clone(); - config_a.node_config.listening_addresses = Some(random_listening_addresses()); + 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 @@ -1441,7 +1441,7 @@ async fn test_node_announcement_propagation() { 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 = random_listening_addresses(); + let node_b_listening_addresses = generate_listening_addresses(); 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;