Add test for monitor update after funding spend
What changed, and why it matters
This commit adds a regression test that reproduces a crash (panic) in the Lightning Dev Kit's channel monitoring logic. The crash happens when a blockchain event and a peer message arrive in an unlucky order: after the counterparty's commitment transaction confirms on-chain, the local node still processes a new commitment_signed message from that peer. The code marks the channel monitor as frozen, but a later legitimate payment preimage claim returns a success status, conflicting with the earlier frozen status and triggering an internal assertion that crashes the node.
Treat this as a known bug reproducer. A follow-up patch should reconcile ChannelMonitorUpdateStatus transitions when update_monitor returns Err after funding_spend_seen, ensuring that a later Completed update does not trigger the InProgress->Completed assertion. Reviewers should verify the eventual fix in ChainMonitor::update_channel or ChannelMonitor::update_monitor and add a non-panic version of this test.
Security signals we found
panic/assertion failure in channel monitor state machine
race condition between on-chain confirmation and peer message processing
InProgress/Completed status mismatch after error override
regression test for denial-of-service-like node crash
no code fix present in this commit
Evidence from the diff
The new test, test_monitor_update_fail_after_funding_spend, demonstrates a state-machine inconsistency in ChainMonitor/ChannelMonitor update handling. When the counterparty commitment transaction confirms, ChannelMonitor sets funding_spend_seen, so no_further_updates_allowed() returns true. A later commitment_signed from the counterparty causes update_monitor to return Err; ChainMonitor overrides that Err to ChannelMonitorUpdateStatus::InProgress. Later, a preimage claim produces a Completed update. The per-channel logic asserts that Completed must not follow InProgress, causing a panic. The commit only adds the failing test (annotated with #[should_panic]); it does not include a fix.
Changed components
lightning/src/ln/chanmon_update_fail_tests.rsChainMonitorChannelMonitorchannel monitor update status handlingInspect captured patch +70 / −1
diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs
index 11fc8ac..87856c9 100644
--- a/lightning/src/ln/chanmon_update_fail_tests.rs
+++ b/lightning/src/ln/chanmon_update_fail_tests.rs
@@ -16,7 +16,7 @@ use crate::chain::chaininterface::LowerBoundedFeeEstimator;
use crate::chain::chainmonitor::ChainMonitor;
use crate::chain::channelmonitor::{ChannelMonitor, MonitorEvent, ANTI_REORG_DELAY};
use crate::chain::transaction::OutPoint;
-use crate::chain::{ChannelMonitorUpdateStatus, Listen, Watch};
+use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
use crate::events::{ClosureReason, Event, HTLCHandlingFailureType, PaymentPurpose};
use crate::ln::channel::AnnouncementSigsState;
use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder};
@@ -5421,3 +5421,72 @@ fn test_late_counterparty_commitment_update_after_holder_commitment_spend() {
fn test_late_counterparty_commitment_update_after_holder_commitment_spend_dust() {
do_test_late_counterparty_commitment_update_after_holder_commitment_spend(true);
}
+
+#[test]
+#[should_panic(
+ expected = "Watch::update_channel returned Completed while prior updates are still InProgress"
+)]
+fn test_monitor_update_fail_after_funding_spend() {
+ // When a counterparty commitment transaction confirms (funding spend), the
+ // ChannelMonitor sets funding_spend_seen. If a commitment_signed from the
+ // counterparty is then processed (a race between chain events and message
+ // processing), update_monitor returns Err because no_further_updates_allowed()
+ // is true. ChainMonitor overrides the result to InProgress, permanently
+ // freezing the channel. A subsequent preimage claim returning Completed then
+ // triggers the per-channel assertion.
+ let chanmon_cfgs = create_chanmon_cfgs(2);
+ let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+ let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+ let node_a_id = nodes[0].node.get_our_node_id();
+
+ let (_, _, chan_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1);
+
+ // Route payment 1 fully so B can claim it later.
+ let (payment_preimage_1, _payment_hash_1, ..) =
+ route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
+
+ // Get A's commitment tx (this is the "counterparty" commitment from B's perspective).
+ let as_commitment_tx = get_local_commitment_txn!(nodes[0], chan_id);
+ assert_eq!(as_commitment_tx.len(), 1);
+
+ // Confirm A's commitment tx on B's chain_monitor ONLY (not on B's ChannelManager).
+ // This sets funding_spend_seen in the monitor, making no_further_updates_allowed() true.
+ let (block_hash, height) = nodes[1].best_block_info();
+ let block = create_dummy_block(block_hash, height + 1, vec![as_commitment_tx[0].clone()]);
+ let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
+ nodes[1].chain_monitor.chain_monitor.transactions_confirmed(&block.header, &txdata, height + 1);
+
+ // Send payment 2 from A to B.
+ let (route, payment_hash_2, _, payment_secret_2) =
+ get_route_and_payment_hash!(&nodes[0], nodes[1], 1_000_000);
+ nodes[0]
+ .node
+ .send_payment_with_route(
+ route,
+ payment_hash_2,
+ RecipientOnionFields::secret_only(payment_secret_2, 1_000_000),
+ PaymentId(payment_hash_2.0),
+ )
+ .unwrap();
+ check_added_monitors(&nodes[0], 1);
+
+ let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+ assert_eq!(events.len(), 1);
+ let payment_event = SendEvent::from_event(events.remove(0));
+
+ nodes[1].node.handle_update_add_htlc(node_a_id, &payment_event.msgs[0]);
+
+ // B processes commitment_signed. The monitor's update_monitor succeeds on the
+ // update steps, but returns Err at the end because no_further_updates_allowed()
+ // is true (funding_spend_seen). ChainMonitor overrides the result to InProgress.
+ nodes[1].node.handle_commitment_signed(node_a_id, &payment_event.commitment_msg[0]);
+ check_added_monitors(&nodes[1], 1);
+ assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+ // B claims payment 1. The PaymentPreimage monitor update returns Completed
+ // (update_monitor succeeds for preimage, and persister returns Completed),
+ // but the prior InProgress from the commitment_signed is still pending.
+ nodes[1].node.claim_funds(payment_preimage_1);
+}
Why this scored 56/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.