diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 852ceabf88d..c17cd87d492 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2571,6 +2571,16 @@ impl ChannelMonitor { 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 ChannelMonitor { transaction_fee_satoshis, } }) - .collect(); + .collect::>(); let confirmed_balance_candidate_index = core::iter::once(&us.funding) .chain(us.pending_funding.iter()) .enumerate() @@ -3057,14 +3067,25 @@ impl ChannelMonitor { }) .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 ChannelMonitorImpl { } 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 4d5355a8d78..bc089d495bb 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")]