Skip to content

Commit 4736ce2

Browse files
joostjagerjoostjager
committed
test: Use random ports with retry loop instead of deterministic allocation
Revert the deterministic port allocation approach from PR lightningdevkit#847 and instead use random ports with a retry loop around node.start(). This avoids collisions with ports allocated by electrsd/corepc_node via get_available_port(), which use the OS ephemeral port allocator and can land in any range. On InvalidSocketAddress, new random ports are selected and the node is rebuilt, up to 5 attempts. AI tools were used in preparing this commit.
1 parent 1bd1672 commit 4736ce2

3 files changed

Lines changed: 98 additions & 85 deletions

File tree

‎.github/workflows/rust.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ jobs:
135135
for i in $(seq 1 5); do
136136
echo "=== Iteration $i (shard ${{ matrix.shard }}) ==="
137137
for j in 1 2 3; do
138-
LDK_NODE_TEST_BASE_PORT=$((20000 + (i - 1) * 3000 + (j - 1) * 1000)) RUSTFLAGS="--cfg no_download --cfg cycle_tests" cargo test --test integration_tests_rust -- --nocapture > /tmp/stress-${j}.log 2>&1 &
138+
RUSTFLAGS="--cfg no_download --cfg cycle_tests" cargo test --test integration_tests_rust -- --nocapture > /tmp/stress-${j}.log 2>&1 &
139139
pids[${j}]=$!
140140
done
141141
failed=0

‎tests/common/mod.rs‎

Lines changed: 92 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ 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};
18-
use std::sync::{Arc, LazyLock, RwLock};
17+
use std::sync::{Arc, RwLock};
1918
use std::time::Duration;
2019

2120
use bitcoin::hashes::hex::FromHex;
@@ -274,13 +273,9 @@ pub(crate) fn random_storage_path() -> PathBuf {
274273
temp_path
275274
}
276275

277-
static BASE_PORT: LazyLock<u16> = LazyLock::new(|| {
278-
env::var("LDK_NODE_TEST_BASE_PORT").ok().and_then(|v| v.parse().ok()).unwrap_or(20000)
279-
});
280-
static NEXT_PORT: LazyLock<AtomicU16> = LazyLock::new(|| AtomicU16::new(*BASE_PORT));
281-
282-
pub(crate) fn generate_listening_addresses() -> Vec<SocketAddress> {
283-
let port = NEXT_PORT.fetch_add(2, Ordering::Relaxed);
276+
pub(crate) fn random_listening_addresses() -> Vec<SocketAddress> {
277+
let mut rng = rng();
278+
let port = rng.random_range(10000..65000u16);
284279
vec![
285280
SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port },
286281
SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: port + 1 },
@@ -310,8 +305,8 @@ pub(crate) fn random_config(anchor_channels: bool) -> TestConfig {
310305
println!("Setting random LDK storage dir: {}", rand_dir.display());
311306
node_config.storage_dir_path = rand_dir.to_str().unwrap().to_owned();
312307

313-
let listening_addresses = generate_listening_addresses();
314-
println!("Setting LDK listening addresses: {:?}", listening_addresses);
308+
let listening_addresses = random_listening_addresses();
309+
println!("Setting random LDK listening addresses: {:?}", listening_addresses);
315310
node_config.listening_addresses = Some(listening_addresses);
316311

317312
let alias = random_node_alias();
@@ -430,81 +425,99 @@ pub(crate) fn setup_two_nodes_with_store(
430425
}
431426

432427
pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) -> TestNode {
433-
setup_builder!(builder, config.node_config);
434-
match chain_source {
435-
TestChainSource::Esplora(electrsd) => {
436-
let esplora_url = format!("http://{}", electrsd.esplora_url.as_ref().unwrap());
437-
let mut sync_config = EsploraSyncConfig::default();
438-
sync_config.background_sync_config = None;
439-
builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
440-
},
441-
TestChainSource::Electrum(electrsd) => {
442-
let electrum_url = format!("tcp://{}", electrsd.electrum_url);
443-
let mut sync_config = ElectrumSyncConfig::default();
444-
sync_config.background_sync_config = None;
445-
builder.set_chain_source_electrum(electrum_url.clone(), Some(sync_config));
446-
},
447-
TestChainSource::BitcoindRpcSync(bitcoind) => {
448-
let rpc_host = bitcoind.params.rpc_socket.ip().to_string();
449-
let rpc_port = bitcoind.params.rpc_socket.port();
450-
let values = bitcoind.params.get_cookie_values().unwrap().unwrap();
451-
let rpc_user = values.user;
452-
let rpc_password = values.password;
453-
builder.set_chain_source_bitcoind_rpc(rpc_host, rpc_port, rpc_user, rpc_password);
454-
},
455-
TestChainSource::BitcoindRestSync(bitcoind) => {
456-
let rpc_host = bitcoind.params.rpc_socket.ip().to_string();
457-
let rpc_port = bitcoind.params.rpc_socket.port();
458-
let values = bitcoind.params.get_cookie_values().unwrap().unwrap();
459-
let rpc_user = values.user;
460-
let rpc_password = values.password;
461-
let rest_host = bitcoind.params.rpc_socket.ip().to_string();
462-
let rest_port = bitcoind.params.rpc_socket.port();
463-
builder.set_chain_source_bitcoind_rest(
464-
rest_host,
465-
rest_port,
466-
rpc_host,
467-
rpc_port,
468-
rpc_user,
469-
rpc_password,
428+
for attempt in 0..5 {
429+
let mut node_config = config.node_config.clone();
430+
if attempt > 0 {
431+
let new_addrs = random_listening_addresses();
432+
println!(
433+
"Retrying with new listening addresses (attempt {}): {:?}",
434+
attempt + 1,
435+
new_addrs
470436
);
471-
},
472-
}
437+
node_config.listening_addresses = Some(new_addrs);
438+
}
473439

474-
match &config.log_writer {
475-
TestLogWriter::FileWriter => {
476-
builder.set_filesystem_logger(None, None);
477-
},
478-
TestLogWriter::LogFacade => {
479-
builder.set_log_facade_logger();
480-
},
481-
TestLogWriter::Custom(custom_log_writer) => {
482-
builder.set_custom_logger(Arc::clone(custom_log_writer));
483-
},
484-
}
440+
setup_builder!(builder, node_config);
441+
match chain_source {
442+
TestChainSource::Esplora(electrsd) => {
443+
let esplora_url = format!("http://{}", electrsd.esplora_url.as_ref().unwrap());
444+
let mut sync_config = EsploraSyncConfig::default();
445+
sync_config.background_sync_config = None;
446+
builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
447+
},
448+
TestChainSource::Electrum(electrsd) => {
449+
let electrum_url = format!("tcp://{}", electrsd.electrum_url);
450+
let mut sync_config = ElectrumSyncConfig::default();
451+
sync_config.background_sync_config = None;
452+
builder.set_chain_source_electrum(electrum_url.clone(), Some(sync_config));
453+
},
454+
TestChainSource::BitcoindRpcSync(bitcoind) => {
455+
let rpc_host = bitcoind.params.rpc_socket.ip().to_string();
456+
let rpc_port = bitcoind.params.rpc_socket.port();
457+
let values = bitcoind.params.get_cookie_values().unwrap().unwrap();
458+
let rpc_user = values.user;
459+
let rpc_password = values.password;
460+
builder.set_chain_source_bitcoind_rpc(rpc_host, rpc_port, rpc_user, rpc_password);
461+
},
462+
TestChainSource::BitcoindRestSync(bitcoind) => {
463+
let rpc_host = bitcoind.params.rpc_socket.ip().to_string();
464+
let rpc_port = bitcoind.params.rpc_socket.port();
465+
let values = bitcoind.params.get_cookie_values().unwrap().unwrap();
466+
let rpc_user = values.user;
467+
let rpc_password = values.password;
468+
let rest_host = bitcoind.params.rpc_socket.ip().to_string();
469+
let rest_port = bitcoind.params.rpc_socket.port();
470+
builder.set_chain_source_bitcoind_rest(
471+
rest_host,
472+
rest_port,
473+
rpc_host,
474+
rpc_port,
475+
rpc_user,
476+
rpc_password,
477+
);
478+
},
479+
}
485480

486-
builder.set_async_payments_role(config.async_payments_role).unwrap();
481+
match &config.log_writer {
482+
TestLogWriter::FileWriter => {
483+
builder.set_filesystem_logger(None, None);
484+
},
485+
TestLogWriter::LogFacade => {
486+
builder.set_log_facade_logger();
487+
},
488+
TestLogWriter::Custom(custom_log_writer) => {
489+
builder.set_custom_logger(Arc::clone(custom_log_writer));
490+
},
491+
}
487492

488-
if config.recovery_mode {
489-
builder.set_wallet_recovery_mode();
490-
}
493+
builder.set_async_payments_role(config.async_payments_role).unwrap();
491494

492-
let node = match config.store_type {
493-
TestStoreType::TestSyncStore => {
494-
let kv_store = TestSyncStore::new(config.node_config.storage_dir_path.into());
495-
builder.build_with_store(config.node_entropy.into(), kv_store).unwrap()
496-
},
497-
TestStoreType::Sqlite => builder.build(config.node_entropy.into()).unwrap(),
498-
};
495+
if config.recovery_mode {
496+
builder.set_wallet_recovery_mode();
497+
}
499498

500-
if config.recovery_mode {
501-
builder.set_wallet_recovery_mode();
502-
}
499+
let node = match config.store_type {
500+
TestStoreType::TestSyncStore => {
501+
let kv_store = TestSyncStore::new(node_config.storage_dir_path.into());
502+
builder.build_with_store(config.node_entropy.clone().into(), kv_store).unwrap()
503+
},
504+
TestStoreType::Sqlite => builder.build(config.node_entropy.clone().into()).unwrap(),
505+
};
503506

504-
node.start().unwrap();
505-
assert!(node.status().is_running);
506-
assert!(node.status().latest_fee_rate_cache_update_timestamp.is_some());
507-
node
507+
match node.start() {
508+
Ok(()) => {
509+
assert!(node.status().is_running);
510+
assert!(node.status().latest_fee_rate_cache_update_timestamp.is_some());
511+
return node;
512+
},
513+
Err(NodeError::InvalidSocketAddress) => {
514+
eprintln!("node.start() failed with InvalidSocketAddress, retrying...");
515+
continue;
516+
},
517+
Err(e) => panic!("node.start() failed: {:?}", e),
518+
}
519+
}
520+
panic!("Failed to start node after 5 attempts due to port collisions")
508521
}
509522

510523
pub(crate) async fn generate_blocks_and_wait<E: ElectrumApi>(

‎tests/integration_tests_rust.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ 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,
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, random_listening_addresses,
2626
setup_bitcoind_and_electrsd, setup_builder, setup_node, setup_two_nodes, splice_in_with_all,
2727
wait_for_tx, TestChainSource, TestStoreType, TestSyncStore,
2828
};
@@ -1431,9 +1431,9 @@ async fn test_node_announcement_propagation() {
14311431
node_a_alias_bytes[..node_a_alias_string.as_bytes().len()]
14321432
.copy_from_slice(node_a_alias_string.as_bytes());
14331433
let node_a_node_alias = Some(NodeAlias(node_a_alias_bytes));
1434-
let node_a_announcement_addresses = generate_listening_addresses();
1434+
let node_a_announcement_addresses = random_listening_addresses();
14351435
config_a.node_config.node_alias = node_a_node_alias.clone();
1436-
config_a.node_config.listening_addresses = Some(generate_listening_addresses());
1436+
config_a.node_config.listening_addresses = Some(random_listening_addresses());
14371437
config_a.node_config.announcement_addresses = Some(node_a_announcement_addresses.clone());
14381438

14391439
// Node B will only use listening addresses
@@ -1443,7 +1443,7 @@ async fn test_node_announcement_propagation() {
14431443
node_b_alias_bytes[..node_b_alias_string.as_bytes().len()]
14441444
.copy_from_slice(node_b_alias_string.as_bytes());
14451445
let node_b_node_alias = Some(NodeAlias(node_b_alias_bytes));
1446-
let node_b_listening_addresses = generate_listening_addresses();
1446+
let node_b_listening_addresses = random_listening_addresses();
14471447
config_b.node_config.node_alias = node_b_node_alias.clone();
14481448
config_b.node_config.listening_addresses = Some(node_b_listening_addresses.clone());
14491449
config_b.node_config.announcement_addresses = None;

0 commit comments

Comments
 (0)