Skip to content

Commit 981e95f

Browse files
authored
Merge pull request #4081 from TheBlueMatt/2025-09-drop-inb-unfunded-monitors
Immediately archive `ChannelMonitor`s for inbound unfuned channels
2 parents bc6ec42 + 744664e commit 981e95f

File tree

2 files changed

+66
-10
lines changed

2 files changed

+66
-10
lines changed

lightning/src/chain/channelmonitor.rs

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2599,6 +2599,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
25992599
let current_height = self.current_best_block().height;
26002600
let mut inner = self.inner.lock().unwrap();
26012601

2602+
if inner.is_closed_without_updates()
2603+
&& is_all_funds_claimed
2604+
&& !inner.funding_spend_seen
2605+
{
2606+
// We closed the channel without ever advancing it and didn't have any funds in it.
2607+
// We should immediately archive this monitor as there's nothing for us to ever do with
2608+
// it.
2609+
return (true, false);
2610+
}
2611+
26022612
if is_all_funds_claimed && !inner.funding_spend_seen {
26032613
debug_assert!(false, "We should see funding spend by the time a monitor clears out");
26042614
is_all_funds_claimed = false;
@@ -3073,7 +3083,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
30733083
transaction_fee_satoshis,
30743084
}
30753085
})
3076-
.collect();
3086+
.collect::<Vec<_>>();
30773087
let confirmed_balance_candidate_index = core::iter::once(&us.funding)
30783088
.chain(us.pending_funding.iter())
30793089
.enumerate()
@@ -3086,14 +3096,25 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
30863096
})
30873097
.map(|(idx, _)| idx)
30883098
.expect("We must have one FundingScope that is confirmed");
3089-
res.push(Balance::ClaimableOnChannelClose {
3090-
balance_candidates,
3091-
confirmed_balance_candidate_index,
3092-
outbound_payment_htlc_rounded_msat,
3093-
outbound_forwarded_htlc_rounded_msat,
3094-
inbound_claiming_htlc_rounded_msat,
3095-
inbound_htlc_rounded_msat,
3096-
});
3099+
3100+
// Only push a primary balance if either the channel isn't closed or we've advanced the
3101+
// channel state machine at least once (implying there are multiple previous commitment
3102+
// transactions) or we actually have a balance.
3103+
// Avoiding including a `Balance` if none of these are true allows us to prune monitors
3104+
// for chanels that were opened inbound to us but where the funding transaction never
3105+
// confirmed at all.
3106+
if !us.is_closed_without_updates()
3107+
|| balance_candidates.iter().any(|bal| bal.amount_satoshis != 0)
3108+
{
3109+
res.push(Balance::ClaimableOnChannelClose {
3110+
balance_candidates,
3111+
confirmed_balance_candidate_index,
3112+
outbound_payment_htlc_rounded_msat,
3113+
outbound_forwarded_htlc_rounded_msat,
3114+
inbound_claiming_htlc_rounded_msat,
3115+
inbound_htlc_rounded_msat,
3116+
});
3117+
}
30973118
}
30983119

30993120
res
@@ -4310,6 +4331,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
43104331
} else { ret }
43114332
}
43124333

4334+
/// Returns true if the channel has been closed (i.e. no further updates are allowed) and no
4335+
/// commitment state updates ever happened.
4336+
fn is_closed_without_updates(&self) -> bool {
4337+
let mut commitment_not_advanced =
4338+
self.current_counterparty_commitment_number == INITIAL_COMMITMENT_NUMBER;
4339+
commitment_not_advanced &=
4340+
self.current_holder_commitment_number == INITIAL_COMMITMENT_NUMBER;
4341+
(self.holder_tx_signed || self.lockdown_from_offchain) && commitment_not_advanced
4342+
}
4343+
43134344
fn no_further_updates_allowed(&self) -> bool {
43144345
self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
43154346
}

lightning/src/ln/functional_tests.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use crate::chain::channelmonitor::{
1818
Balance, ANTI_REORG_DELAY, CLTV_CLAIM_BUFFER, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE,
1919
LATENCY_GRACE_PERIOD_BLOCKS,
2020
};
21+
use crate::chain::transaction::OutPoint;
2122
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
2223
use crate::events::{
2324
ClosureReason, Event, HTLCHandlingFailureType, PathFailure, PaymentFailureReason,
@@ -6649,9 +6650,13 @@ pub fn test_channel_conf_timeout() {
66496650

66506651
let node_a_id = nodes[0].node.get_our_node_id();
66516652

6652-
let _funding_tx =
6653+
let funding_tx =
66536654
create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 1_000_000, 100_000);
66546655

6656+
// Inbound channels which haven't advanced state at all and never were funded will generate
6657+
// claimable `Balance`s until they're closed.
6658+
assert!(!nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());
6659+
66556660
// The outbound node should wait forever for confirmation:
66566661
// This matches `channel::FUNDING_CONF_DEADLINE_BLOCKS` and BOLT 2's suggested timeout, thus is
66576662
// copied here instead of directly referencing the constant.
@@ -6663,6 +6668,10 @@ pub fn test_channel_conf_timeout() {
66636668
check_added_monitors(&nodes[1], 0);
66646669
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
66656670

6671+
nodes[1].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
6672+
assert_eq!(nodes[1].chain_monitor.chain_monitor.list_monitors().len(), 1);
6673+
assert!(!nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());
6674+
66666675
connect_blocks(&nodes[1], 1);
66676676
check_added_monitors(&nodes[1], 1);
66686677
check_closed_event!(nodes[1], 1, ClosureReason::FundingTimedOut, [node_a_id], 1000000);
@@ -6681,6 +6690,22 @@ pub fn test_channel_conf_timeout() {
66816690
},
66826691
_ => panic!("Unexpected event"),
66836692
}
6693+
6694+
// Once an inbound never-confirmed channel is closed, it will no longer generate any claimable
6695+
// `Balance`s.
6696+
assert!(nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());
6697+
6698+
// Once the funding times out the monitor should be immediately archived.
6699+
nodes[1].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
6700+
assert_eq!(nodes[1].chain_monitor.chain_monitor.list_monitors().len(), 0);
6701+
assert!(nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());
6702+
6703+
// Remove the corresponding outputs and transactions the chain source is
6704+
// watching. This is to make sure the `Drop` function assertions pass.
6705+
nodes[1].chain_source.remove_watched_txn_and_outputs(
6706+
OutPoint { txid: funding_tx.compute_txid(), index: 0 },
6707+
funding_tx.output[0].script_pubkey.clone(),
6708+
);
66846709
}
66856710

66866711
#[xtest(feature = "_externalize_tests")]

0 commit comments

Comments
 (0)