Immediately archive `ChannelMonitor`s for inbound unfuned channels
What changed, and why it matters
This change fixes a cleanup issue in the Lightning Dev Kit: when someone tries to open a channel with you but never actually puts the funding transaction on the blockchain, the software used to keep an unnecessary 'ChannelMonitor' record around for up to about two weeks. After this patch, that leftover record is archived immediately once the channel times out, reducing clutter and avoiding misleading 'claimable balance' reports. It is a reliability/resource issue rather than a direct theft-of-funds bug.
Apply the patch. It is a low-risk cleanup that prevents accumulation of stale monitors and misleading balances from failed inbound channel opens. Monitor for any regressions in balance reporting for legitimately closed channels.
Security signals we found
Resource exhaustion / state bloat: stale ChannelMonitors retained for 2016 blocks
Informational inconsistency: phantom claimable balances reported for non-existent channels
Cleanup of failed inbound channel opens, reducing attack surface for channel-spam DoS
No direct loss of funds path identified in the diff
Evidence from the diff
The commit adds an is_closed_without_updates() helper and uses it in two places: (1) archive_fully_resolved_channel_monitors now immediately archives a monitor when the channel is closed, no commitment updates ever occurred, no funds are claimed, and no funding spend was seen; (2) get_claimable_balances no longer emits a Balance::ClaimableOnChannelClose for such inbound never-funded channels. A functional test is updated to assert that the monitor count drops to zero and claimable balances become empty after the funding timeout closes the channel.
Changed components
lightning/src/chain/channelmonitor.rslightning/src/ln/functional_tests.rsInspect captured patch +66 / −10
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index 852ceab..c17cd87 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -2571,6 +2571,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
let current_height = self.current_best_block().height;
let mut inner = self.inner.lock().unwrap();
+ if inner.is_closed_without_updates()
+ && is_all_funds_claimed
+ && !inner.funding_spend_seen
+ {
+ // We closed the channel without ever advancing it and didn't have any funds in it.
+ // We should immediately archive this monitor as there's nothing for us to ever do with
+ // it.
+ return (true, false);
+ }
+
if is_all_funds_claimed && !inner.funding_spend_seen {
debug_assert!(false, "We should see funding spend by the time a monitor clears out");
is_all_funds_claimed = false;
@@ -3044,7 +3054,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
transaction_fee_satoshis,
}
})
- .collect();
+ .collect::<Vec<_>>();
let confirmed_balance_candidate_index = core::iter::once(&us.funding)
.chain(us.pending_funding.iter())
.enumerate()
@@ -3057,14 +3067,25 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
})
.map(|(idx, _)| idx)
.expect("We must have one FundingScope that is confirmed");
- res.push(Balance::ClaimableOnChannelClose {
- balance_candidates,
- confirmed_balance_candidate_index,
- outbound_payment_htlc_rounded_msat,
- outbound_forwarded_htlc_rounded_msat,
- inbound_claiming_htlc_rounded_msat,
- inbound_htlc_rounded_msat,
- });
+
+ // Only push a primary balance if either the channel isn't closed or we've advanced the
+ // channel state machine at least once (implying there are multiple previous commitment
+ // transactions) or we actually have a balance.
+ // Avoiding including a `Balance` if none of these are true allows us to prune monitors
+ // for chanels that were opened inbound to us but where the funding transaction never
+ // confirmed at all.
+ if !us.is_closed_without_updates()
+ || balance_candidates.iter().any(|bal| bal.amount_satoshis != 0)
+ {
+ res.push(Balance::ClaimableOnChannelClose {
+ balance_candidates,
+ confirmed_balance_candidate_index,
+ outbound_payment_htlc_rounded_msat,
+ outbound_forwarded_htlc_rounded_msat,
+ inbound_claiming_htlc_rounded_msat,
+ inbound_htlc_rounded_msat,
+ });
+ }
}
res
@@ -4352,6 +4373,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
} else { ret }
}
+ /// Returns true if the channel has been closed (i.e. no further updates are allowed) and no
+ /// commitment state updates ever happened.
+ fn is_closed_without_updates(&self) -> bool {
+ let mut commitment_not_advanced =
+ self.current_counterparty_commitment_number == INITIAL_COMMITMENT_NUMBER;
+ commitment_not_advanced &=
+ self.current_holder_commitment_number == INITIAL_COMMITMENT_NUMBER;
+ (self.holder_tx_signed || self.lockdown_from_offchain) && commitment_not_advanced
+ }
+
fn no_further_updates_allowed(&self) -> bool {
self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
}
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 4d5355a..bc089d4 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -18,6 +18,7 @@ use crate::chain::channelmonitor::{
Balance, ANTI_REORG_DELAY, CLTV_CLAIM_BUFFER, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE,
LATENCY_GRACE_PERIOD_BLOCKS,
};
+use crate::chain::transaction::OutPoint;
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
use crate::events::{
ClosureReason, Event, HTLCHandlingFailureType, PathFailure, PaymentFailureReason,
@@ -6631,9 +6632,13 @@ pub fn test_channel_conf_timeout() {
let node_a_id = nodes[0].node.get_our_node_id();
- let _funding_tx =
+ let funding_tx =
create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 1_000_000, 100_000);
+ // Inbound channels which haven't advanced state at all and never were funded will generate
+ // claimable `Balance`s until they're closed.
+ assert!(!nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());
+
// The outbound node should wait forever for confirmation:
// This matches `channel::FUNDING_CONF_DEADLINE_BLOCKS` and BOLT 2's suggested timeout, thus is
// copied here instead of directly referencing the constant.
@@ -6645,6 +6650,10 @@ pub fn test_channel_conf_timeout() {
check_added_monitors(&nodes[1], 0);
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+ nodes[1].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
+ assert_eq!(nodes[1].chain_monitor.chain_monitor.list_monitors().len(), 1);
+ assert!(!nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());
+
connect_blocks(&nodes[1], 1);
check_added_monitors(&nodes[1], 1);
check_closed_event!(nodes[1], 1, ClosureReason::FundingTimedOut, [node_a_id], 1000000);
@@ -6663,6 +6672,22 @@ pub fn test_channel_conf_timeout() {
},
_ => panic!("Unexpected event"),
}
+
+ // Once an inbound never-confirmed channel is closed, it will no longer generate any claimable
+ // `Balance`s.
+ assert!(nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());
+
+ // Once the funding times out the monitor should be immediately archived.
+ nodes[1].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
+ assert_eq!(nodes[1].chain_monitor.chain_monitor.list_monitors().len(), 0);
+ assert!(nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());
+
+ // Remove the corresponding outputs and transactions the chain source is
+ // watching. This is to make sure the `Drop` function assertions pass.
+ nodes[1].chain_source.remove_watched_txn_and_outputs(
+ OutPoint { txid: funding_tx.compute_txid(), index: 0 },
+ funding_tx.output[0].script_pubkey.clone(),
+ );
}
#[xtest(feature = "_externalize_tests")]
Why this scored 35/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.