refactor(test): DRY up interop test harness

Address review feedback on verbosity and duplication:

- Extract common tests (10 scenarios) into `interop_basic_tests!` macro,
  reducing CLN/LND/Eclair test files from 743 to 343 lines (-54%)
- Move `make_error()` from 3 implementations to ExternalNode trait default
- Extract reconnection polling into `reconnect_and_wait()` helper
- Add Windows exclusion for bitcoind/electrs env vars (binaries not downloaded)
- Add newline in channel.rs per review nit
- Update Eclair ignore messages with issue links and clearer wording

Author: febyeji

.github/workflows/rust.yml

@@ -68,6 +68,7 @@ jobs:
           mv "$BITCOIND_EXE" bin/bitcoind-${{ runner.os }}-${{ runner.arch }}
           mv "$ELECTRS_EXE" bin/electrs-${{ runner.os }}-${{ runner.arch }}
       - name: Set bitcoind/electrs environment variables
+        if: "matrix.platform != 'windows-latest'"
         run: |
           echo "BITCOIND_EXE=$( pwd )/bin/bitcoind-${{ runner.os }}-${{ runner.arch }}" >> "$GITHUB_ENV"
           echo "ELECTRS_EXE=$( pwd )/bin/electrs-${{ runner.os }}-${{ runner.arch }}" >> "$GITHUB_ENV"

tests/common/cln.rs

@@ -52,10 +52,6 @@ impl TestClnNode {
 		tokio::task::spawn_blocking(move || f(&*client)).await.expect("CLN RPC task panicked")
 	}
 
-	fn make_error(&self, detail: String) -> TestFailure {
-		TestFailure::ExternalNodeError { node: "CLN".to_string(), detail }
-	}
-
 	/// Repeatedly call `splice_update` until `commitments_secured` is true.
 	/// Returns the final PSBT. Gives up after 10 attempts.
 	async fn splice_update_loop(

tests/common/eclair.rs

@@ -78,10 +78,6 @@ impl TestEclairNode {
 		})
 	}
 
-	fn make_error(&self, detail: String) -> TestFailure {
-		TestFailure::ExternalNodeError { node: "Eclair".to_string(), detail }
-	}
-
 	/// Poll /getsentinfo until the payment settles or fails.
 	/// Eclair's pay/keysend APIs are fire-and-forget, so without polling
 	/// the caller would hang on the LDK event timeout when a payment fails.

tests/common/external_node.rs

@@ -106,6 +106,11 @@ pub(crate) trait ExternalNode: Send + Sync {
 	/// List all channels known to this node.
 	async fn list_channels(&self) -> Result<Vec<ExternalChannel>, TestFailure>;
 
+	/// Construct a `TestFailure::ExternalNodeError` for this node.
+	fn make_error(&self, detail: String) -> TestFailure {
+		TestFailure::ExternalNodeError { node: self.name().to_string(), detail }
+	}
+
 	/// Wait until this node has synced to at least `min_height`.
 	///
 	/// The default is a no-op — most implementations (LND, CLN) sync blocks

tests/common/lnd.rs

@@ -55,10 +55,6 @@ impl TestLndNode {
 			.unwrap();
 		Self::new(cert_path, macaroon_path, endpoint, listen_addr).await
 	}
-
-	fn make_error(&self, detail: String) -> TestFailure {
-		TestFailure::ExternalNodeError { node: "LND".to_string(), detail }
-	}
 }
 
 #[async_trait]

tests/common/mod.rs

@@ -1793,6 +1793,163 @@ impl TestSyncStoreInner {
 /// ```ignore
 /// interop_combo_tests!(test_lnd, setup_clients, setup_ldk_node);
 /// ```
+///
+/// ## `interop_basic_tests!`
+///
+/// Generates the standard named tests that are identical across all external
+/// node implementations: basic channel cycle, disconnect/reconnect, force-close
+/// (by LDK and external), inbound channel, bidirectional payments, expired
+/// invoice, concurrent payments, and fee-change close scenarios.
+///
+/// Tests that differ per-implementation (e.g. keysend, splicing) should remain
+/// as hand-written functions in the individual test files.
+///
+/// Usage:
+/// ```ignore
+/// interop_basic_tests!(test_lnd, setup_clients, setup_ldk_node);
+/// ```
+#[macro_export]
+macro_rules! interop_basic_tests {
+	($prefix:ident, $setup_clients:ident, $setup_ldk_node:ident) => {
+		paste::paste! {
+			#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+			async fn [<$prefix _basic_channel_cycle>]() {
+				let (bitcoind, electrs, peer) = $setup_clients().await;
+				let node = $setup_ldk_node();
+				setup_interop_test(&node, &peer, &bitcoind, &electrs).await;
+
+				let (user_channel_id, _ext_channel_id) =
+					open_channel_to_external(
+						&node, &peer, &bitcoind, &electrs, 1_000_000, Some(500_000_000),
+					).await;
+
+				let invoice = peer.create_invoice(10_000_000, "basic-send").await.unwrap();
+				let parsed = lightning_invoice::Bolt11Invoice::from_str(&invoice).unwrap();
+				node.bolt11_payment().send(&parsed, None).unwrap();
+				$crate::common::expect_event!(node, PaymentSuccessful);
+
+				receive_bolt11_payment(&node, &peer, 10_000_000).await;
+
+				cooperative_close_by_ldk(
+					&node, &peer, &bitcoind, &electrs, &user_channel_id,
+				).await;
+				node.stop().unwrap();
+			}
+
+			#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+			async fn [<$prefix _disconnect_reconnect>]() {
+				let (bitcoind, electrs, peer) = $setup_clients().await;
+				let node = $setup_ldk_node();
+				setup_interop_test(&node, &peer, &bitcoind, &electrs).await;
+				let (_user_ch, _ext_ch) = open_channel_to_external(
+					&node, &peer, &bitcoind, &electrs, 1_000_000, Some(500_000_000),
+				).await;
+
+				disconnect_reconnect_idle(
+					&node, &peer, &bitcoind, &electrs, &Side::Ldk,
+				).await;
+				disconnect_reconnect_idle(
+					&node, &peer, &bitcoind, &electrs, &Side::External,
+				).await;
+
+				node.stop().unwrap();
+			}
+
+			#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+			async fn [<$prefix _force_close_by_ldk>]() {
+				let (bitcoind, electrs, peer) = $setup_clients().await;
+				let node = $setup_ldk_node();
+				setup_interop_test(&node, &peer, &bitcoind, &electrs).await;
+				let (user_ch, _ext_ch) = open_channel_to_external(
+					&node, &peer, &bitcoind, &electrs, 1_000_000, Some(500_000_000),
+				).await;
+
+				force_close_by_ldk(&node, &peer, &bitcoind, &electrs, &user_ch).await;
+				node.stop().unwrap();
+			}
+
+			#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+			async fn [<$prefix _force_close_by_external>]() {
+				let (bitcoind, electrs, peer) = $setup_clients().await;
+				let node = $setup_ldk_node();
+				setup_interop_test(&node, &peer, &bitcoind, &electrs).await;
+				let (_user_ch, ext_ch) = open_channel_to_external(
+					&node, &peer, &bitcoind, &electrs, 1_000_000, Some(500_000_000),
+				).await;
+
+				force_close_by_external(&node, &peer, &bitcoind, &electrs, &ext_ch).await;
+				node.stop().unwrap();
+			}
+
+			#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+			async fn [<$prefix _inbound_channel>]() {
+				let (bitcoind, electrs, peer) = $setup_clients().await;
+				let node = $setup_ldk_node();
+				run_inbound_channel_test(
+					&node, &peer, &bitcoind, &electrs,
+					CloseType::Cooperative, Side::External,
+				).await;
+				node.stop().unwrap();
+			}
+
+			#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+			async fn [<$prefix _bidirectional_payments>]() {
+				let (bitcoind, electrs, peer) = $setup_clients().await;
+				let node = $setup_ldk_node();
+				setup_interop_test(&node, &peer, &bitcoind, &electrs).await;
+				let (_user_ch, ext_ch) = open_channel_to_external(
+					&node, &peer, &bitcoind, &electrs, 1_000_000, Some(500_000_000),
+				).await;
+
+				bidirectional_payments(&node, &peer, &ext_ch, 5_000_000).await;
+				node.stop().unwrap();
+			}
+
+			#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+			async fn [<$prefix _pay_expired_invoice>]() {
+				let (bitcoind, electrs, peer) = $setup_clients().await;
+				let node = $setup_ldk_node();
+				setup_interop_test(&node, &peer, &bitcoind, &electrs).await;
+				let (_user_ch, _ext_ch) = open_channel_to_external(
+					&node, &peer, &bitcoind, &electrs, 1_000_000, Some(500_000_000),
+				).await;
+
+				pay_expired_invoice(&node, &peer).await;
+				node.stop().unwrap();
+			}
+
+			#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+			async fn [<$prefix _concurrent_payments>]() {
+				let (bitcoind, electrs, peer) = $setup_clients().await;
+				let node = $setup_ldk_node();
+				setup_interop_test(&node, &peer, &bitcoind, &electrs).await;
+				let (_user_ch, _ext_ch) = open_channel_to_external(
+					&node, &peer, &bitcoind, &electrs, 1_000_000, Some(500_000_000),
+				).await;
+
+				concurrent_payments(&node, &peer, 5, 1_000_000).await;
+				node.stop().unwrap();
+			}
+
+			#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+			async fn [<$prefix _cooperative_close_after_fee_change>]() {
+				let (bitcoind, electrs, peer) = $setup_clients().await;
+				let node = $setup_ldk_node();
+				cooperative_close_after_fee_change(&node, &peer, &bitcoind, &electrs).await;
+				node.stop().unwrap();
+			}
+
+			#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+			async fn [<$prefix _force_close_after_fee_change>]() {
+				let (bitcoind, electrs, peer) = $setup_clients().await;
+				let node = $setup_ldk_node();
+				force_close_after_fee_change(&node, &peer, &bitcoind, &electrs).await;
+				node.stop().unwrap();
+			}
+		}
+	};
+}
+
 #[macro_export]
 macro_rules! interop_combo_tests {
 	($prefix:ident, $setup_clients:ident, $setup_ldk_node:ident) => {

tests/common/scenarios/channel.rs

@@ -27,6 +27,7 @@ async fn expect_channel_closed(node: &Node, expected_local_initiated: bool) {
 	)
 	.await
 	.unwrap_or_else(|_| panic!("{} timed out waiting for ChannelClosed event", node.node_id()));
+
 	match event {
 		Event::ChannelClosed { ref reason, .. } => {
 			println!("{} got ChannelClosed: reason={:?}", node.node_id(), reason);

tests/common/scenarios/disconnect.rs

@@ -10,12 +10,30 @@ use std::time::Duration;
 
 use electrsd::corepc_node::Client as BitcoindClient;
 use electrum_client::ElectrumApi;
+use ldk_node::bitcoin::secp256k1::PublicKey;
+use ldk_node::lightning::ln::msgs::SocketAddress;
 use ldk_node::{Event, Node};
 use lightning_invoice::Bolt11Invoice;
 
 use super::super::external_node::ExternalNode;
 use super::Side;
 
+/// Reconnect to a peer and wait until the connection is established.
+async fn reconnect_and_wait(node: &Node, peer_id: PublicKey, addr: SocketAddress, context: &str) {
+	node.connect(peer_id, addr, true).unwrap();
+	for i in 0..30 {
+		if node.list_peers().iter().any(|p| p.node_id == peer_id && p.is_connected) {
+			// Allow channel reestablishment to complete
+			tokio::time::sleep(Duration::from_secs(2)).await;
+			return;
+		}
+		if i == 29 {
+			panic!("Peer did not reconnect within 30s ({})", context);
+		}
+		tokio::time::sleep(Duration::from_secs(1)).await;
+	}
+}
+
 /// Disconnect during idle, reconnect, verify channel still works.
 pub(crate) async fn disconnect_reconnect_idle<E: ElectrumApi>(
 	node: &Node, peer: &(impl ExternalNode + ?Sized), _bitcoind: &BitcoindClient, _electrs: &E,
@@ -35,21 +53,7 @@ pub(crate) async fn disconnect_reconnect_idle<E: ElectrumApi>(
 
 	tokio::time::sleep(Duration::from_secs(1)).await;
 
-	// Reconnect and wait for channel reestablishment
-	node.connect(ext_node_id, ext_addr, true).unwrap();
-	for i in 0..30 {
-		let connected =
-			node.list_peers().iter().any(|p| p.node_id == ext_node_id && p.is_connected);
-		if connected {
-			break;
-		}
-		if i == 29 {
-			panic!("Peer did not reconnect within 30s after idle disconnect");
-		}
-		tokio::time::sleep(Duration::from_secs(1)).await;
-	}
-	// Allow channel reestablishment to complete
-	tokio::time::sleep(Duration::from_secs(2)).await;
+	reconnect_and_wait(node, ext_node_id, ext_addr, "idle disconnect").await;
 
 	// Verify channel still works with a payment
 	let invoice_str = peer.create_invoice(10_000_000, "disconnect-idle-test").await.unwrap();
@@ -86,8 +90,7 @@ pub(crate) async fn disconnect_during_payment<E: ElectrumApi>(
 	tokio::time::sleep(Duration::from_secs(2)).await;
 
 	// Reconnect
-	node.connect(ext_node_id, ext_addr, true).unwrap();
-	tokio::time::sleep(Duration::from_secs(2)).await;
+	reconnect_and_wait(node, ext_node_id, ext_addr, "disconnect during payment").await;
 
 	// If the payment was initiated, wait for it to resolve.
 	if send_ok {