Merge pull request #972 from tnull/2026-07-fix-flaky-reorg-tests

Fix flaky reorg tests

Author: tnull

tests/common/mod.rs

@@ -753,7 +753,7 @@ pub(crate) async fn generate_blocks_and_wait<E: ElectrumApi>(
 	let address = bitcoind.new_address().expect("failed to get new address");
 	// TODO: expect this Result once the WouldBlock issue is resolved upstream.
 	let _block_hashes_res = bitcoind.generate_to_address(num, &address);
-	wait_for_block(electrs, cur_height as usize + num).await;
+	wait_for_block(bitcoind, electrs, cur_height as usize + num).await;
 	print!(" Done!");
 	println!("\n");
 }
@@ -773,27 +773,25 @@ pub(crate) fn invalidate_blocks(bitcoind: &BitcoindClient, num_blocks: usize) {
 	assert!(new_cur_height + num_blocks == cur_height);
 }
 
-pub(crate) async fn wait_for_block<E: ElectrumApi>(electrs: &E, min_height: usize) {
-	let mut header = match electrs.block_headers_subscribe() {
-		Ok(header) => header,
-		Err(_) => {
-			// While subscribing should succeed the first time around, we ran into some cases where
-			// it didn't. Since we can't proceed without subscribing, we try again after a delay
-			// and panic if it still fails.
-			tokio::time::sleep(Duration::from_secs(3)).await;
-			electrs.block_headers_subscribe().expect("failed to subscribe to block headers")
-		},
-	};
-	loop {
-		if header.height >= min_height {
-			break;
+pub(crate) async fn wait_for_block<E: ElectrumApi>(
+	bitcoind: &BitcoindClient, electrs: &E, min_height: usize,
+) {
+	let expected_block_hash = exponential_backoff_poll(|| {
+		let bitcoind_height =
+			bitcoind.get_blockchain_info().expect("failed to get blockchain info").blocks as usize;
+		if bitcoind_height < min_height {
+			return None;
 		}
-		header = exponential_backoff_poll(|| {
-			electrs.ping().expect("failed to ping electrs");
-			electrs.block_headers_pop().expect("failed to pop block header")
-		})
-		.await;
-	}
+		bitcoind.get_block_hash(min_height as u64).ok()?.block_hash().ok()
+	})
+	.await;
+	// A height-only wait can return the old header during a same-height reorg. Require the
+	// replacement hash so callers cannot sync against the stale chain by mistake.
+	exponential_backoff_poll(|| {
+		let header = electrs.block_header(min_height).ok()?;
+		(header.block_hash() == expected_block_hash).then_some(())
+	})
+	.await;
 }
 
 pub(crate) async fn wait_for_tx<E: ElectrumApi>(electrs: &E, txid: Txid) {
@@ -812,15 +810,14 @@ pub(crate) async fn wait_for_outpoint_spend<E: ElectrumApi>(electrs: &E, outpoin
 	let tx = electrs.transaction_get(&outpoint.txid).unwrap();
 	let txout_script = tx.output.get(outpoint.vout as usize).unwrap().clone().script_pubkey;
 
-	let is_spent = !electrs.script_get_history(&txout_script).unwrap().is_empty();
-	if is_spent {
-		return;
-	}
-
+	// Script history already contains the funding transaction itself, so wait until the exact
+	// funding outpoint leaves the unspent set instead of treating any history as a spend.
 	exponential_backoff_poll(|| {
 		electrs.ping().unwrap();
 
-		let is_spent = !electrs.script_get_history(&txout_script).unwrap().is_empty();
+		let is_spent = !electrs.script_list_unspent(&txout_script).unwrap().iter().any(|output| {
+			output.tx_hash == outpoint.txid && output.tx_pos == outpoint.vout as usize
+		});
 		is_spent.then_some(())
 	})
 	.await;

tests/integration_tests_rust.rs

@@ -807,7 +807,7 @@ async fn reorged_onchain_payment_returns_to_unconfirmed() {
 			.call("generateblock", &[json!(replacement_address.to_string()), json!([])])
 			.expect("failed to generate empty block");
 	}
-	wait_for_block(&electrsd.client, original_height as usize + 1).await;
+	wait_for_block(&bitcoind.client, &electrsd.client, original_height as usize + 1).await;
 
 	node_a.sync_wallets().unwrap();
 	node_b.sync_wallets().unwrap();
@@ -2056,7 +2056,7 @@ async fn splice_payment_reorged_to_unconfirmed() {
 			.call("generateblock", &[json!(replacement_address.to_string()), json!([])])
 			.expect("failed to generate empty block");
 	}
-	wait_for_block(&electrsd.client, original_height as usize + 1).await;
+	wait_for_block(&bitcoind.client, &electrsd.client, original_height as usize + 1).await;
 	node_b.sync_wallets().unwrap();
 
 	// The funding payment returns to `Unconfirmed` and stays `Pending`, exercising the

tests/reorg_test.proptest-regressions

@@ -0,0 +1,2 @@
+cc 06354c9b049db51c31557bf46d86a68bdd4049577cbd9190fb81c7824b18f0e6 # shrinks to reorg_depth = 6, force_close = false
+cc ffba5725835411b0948e834640dd37fc9a35696a301d7f2d8f2054f158bd2cae # shrinks to reorg_depth = 1, force_close = true

tests/reorg_test.rs

@@ -8,11 +8,27 @@ use proptest::prelude::prop;
 use proptest::proptest;
 
 use crate::common::{
-	expect_event, generate_blocks_and_wait, invalidate_blocks, open_channel,
-	premine_and_distribute_funds, random_chain_source, random_config, setup_bitcoind_and_electrsd,
-	setup_node, wait_for_outpoint_spend,
+	expect_event, exponential_backoff_poll, generate_blocks_and_wait, invalidate_blocks,
+	open_channel, premine_and_distribute_funds, random_chain_source, random_config,
+	setup_bitcoind_and_electrsd, setup_node, wait_for_outpoint_spend, wait_for_tx,
 };
 
+async fn wait_for_pending_sweep_balance<F>(
+	node: &ldk_node::Node, mut matches_balance: F,
+) -> PendingSweepBalance
+where
+	F: FnMut(&PendingSweepBalance) -> bool,
+{
+	exponential_backoff_poll(|| {
+		node.sync_wallets().unwrap();
+		node.list_balances()
+			.pending_balances_from_channel_closures
+			.into_iter()
+			.find(|balance| matches_balance(balance))
+	})
+	.await
+}
+
 proptest! {
 	#![proptest_config(proptest::test_runner::Config::with_cases(5))]
 	#[test]
@@ -76,7 +92,9 @@ proptest! {
 				nodes_funding_tx.insert(node.node_id(), funding_txo);
 			}
 
-			generate_blocks_and_wait(bitcoind, electrs, 6).await;
+			// Keep funding confirmed across the deepest reorg. rust-lightning PR #4231 exempts
+			// only trusted zero-conf channels; regular channels still force-close at zero confirmations.
+			generate_blocks_and_wait(bitcoind, electrs, 7).await;
 			sync_wallets!();
 
 			reorg!(reorg_depth);
@@ -144,40 +162,60 @@ proptest! {
 			sync_wallets!();
 
 			if force_close {
-				for node in &nodes {
-					node.sync_wallets().unwrap();
-					// If there is no more balance, there is nothing to process here.
-					if node.list_balances().lightning_balances.len() < 1 {
-						return;
-					}
-					match node.list_balances().lightning_balances[0] {
-						LightningBalance::ClaimableAwaitingConfirmations {
-							confirmation_height,
-							..
-						} => {
-							let cur_height = node.status().current_best_block.height;
-							let blocks_to_go = confirmation_height - cur_height;
-							generate_blocks_and_wait(bitcoind, electrs, blocks_to_go as usize).await;
-							node.sync_wallets().unwrap();
-						},
-						_ => panic!("Unexpected balance state for node_hub!"),
-					}
+				let claimable_nodes = nodes
+					.iter()
+					.filter_map(|node| {
+						node.list_balances().lightning_balances.iter().find_map(|balance| {
+							match balance {
+								LightningBalance::ClaimableAwaitingConfirmations {
+									confirmation_height,
+									..
+								} => Some((node, *confirmation_height)),
+								_ => None,
+							}
+						})
+					})
+					.collect::<Vec<_>>();
+				let confirmation_height = claimable_nodes
+					.iter()
+					.map(|(_, confirmation_height)| *confirmation_height)
+					.max()
+					.expect("Missing claimable force-close balance");
+				let cur_height = nodes[0].status().current_best_block.height;
+				let blocks_to_go = confirmation_height.saturating_sub(cur_height);
+				if blocks_to_go > 0 {
+					generate_blocks_and_wait(bitcoind, electrs, blocks_to_go as usize).await;
+					sync_wallets!();
+				}
 
-					assert!(node.list_balances().lightning_balances.len() < 2);
-					assert!(node.list_balances().pending_balances_from_channel_closures.len() > 0);
-					match node.list_balances().pending_balances_from_channel_closures[0] {
-						PendingSweepBalance::BroadcastAwaitingConfirmation { .. } => {},
-						_ => panic!("Unexpected balance state!"),
+				// Mining for one node advances the shared chain for every node. Mature all
+				// claimable outputs together, wait for every sweep to reach the mempool, then
+				// confirm them and wait for `AwaitingThresholdConfirmations`.
+				for (node, _) in &claimable_nodes {
+					let pending_balance = wait_for_pending_sweep_balance(node, |balance| {
+						matches!(
+							balance,
+							PendingSweepBalance::BroadcastAwaitingConfirmation { .. }
+								| PendingSweepBalance::AwaitingThresholdConfirmations { .. }
+						)
+					})
+					.await;
+					if let PendingSweepBalance::BroadcastAwaitingConfirmation {
+						latest_spending_txid,
+						..
+					} = pending_balance
+					{
+						wait_for_tx(electrs, latest_spending_txid).await;
 					}
+				}
 
-					generate_blocks_and_wait(&bitcoind, electrs, 1).await;
-					node.sync_wallets().unwrap();
-					assert!(node.list_balances().lightning_balances.len() < 2);
-					assert!(node.list_balances().pending_balances_from_channel_closures.len() > 0);
-					match node.list_balances().pending_balances_from_channel_closures[0] {
-						PendingSweepBalance::AwaitingThresholdConfirmations { .. } => {},
-						_ => panic!("Unexpected balance state!"),
-					}
+				generate_blocks_and_wait(bitcoind, electrs, 1).await;
+				sync_wallets!();
+				for (node, _) in &claimable_nodes {
+					wait_for_pending_sweep_balance(node, |balance| {
+						matches!(balance, PendingSweepBalance::AwaitingThresholdConfirmations { .. })
+					})
+					.await;
 				}
 			}