From 6cf035146294119c75bab056715410e53369cced Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 29 Sep 2023 09:16:48 -0700 Subject: [PATCH 1/2] Note required levels of descendant transactions in get_spendable_outputs Three levels of descendant transactions starting from the channel's funding transaction should cover all potential spendable outputs. The first level covers the commitment transaction. The second level covers the to_self claims, to_remote claims, second-stage HTLC claims and justice transactions. The third levels covers the justice transactions on second-stage HTLCs, and to_self claims on second-stage HTLCs. --- lightning/src/chain/channelmonitor.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 7088d32d160..61669b784f8 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1680,8 +1680,7 @@ impl ChannelMonitor { /// missed/unhandled descriptors. For the purpose of gathering historical records, if the /// channel close has fully resolved (i.e., [`ChannelMonitor::get_claimable_balances`] returns /// an empty set), you can retrieve all spendable outputs by providing all descendant spending - /// transactions starting from the channel's funding or closing transaction that have at least - /// [`ANTI_REORG_DELAY`] confirmations. + /// transactions starting from the channel's funding transaction and going down three levels. /// /// `tx` is a transaction we'll scan the outputs of. Any transaction can be provided. If any /// outputs which can be spent by us are found, at least one descriptor is returned. From f267a30cc7fe564d8f767c9c973e211f5bf64a82 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 29 Sep 2023 09:40:34 -0700 Subject: [PATCH 2/2] Only yield DelayedPaymentOutput descriptors once their delay expires Otherwise, we could give users a descriptor ahead of time that will result in an invalid transaction spend/broadcast. --- lightning/src/chain/channelmonitor.rs | 17 ++++++++++++----- lightning/src/ln/monitor_tests.rs | 13 ++++++++++--- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 61669b784f8..cfd29759bb5 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1672,6 +1672,8 @@ impl ChannelMonitor { /// Returns the descriptors for relevant outputs (i.e., those that we can spend) within the /// transaction if they exist and the transaction has at least [`ANTI_REORG_DELAY`] + /// confirmations. For [`SpendableOutputDescriptor::DelayedPaymentOutput`] descriptors to be + /// returned, the transaction must have at least `max(ANTI_REORG_DELAY, to_self_delay)` /// confirmations. /// /// Descriptors returned by this method are primarily exposed via [`Event::SpendableOutputs`] @@ -1689,11 +1691,16 @@ impl ChannelMonitor { pub fn get_spendable_outputs(&self, tx: &Transaction, confirmation_height: u32) -> Vec { let inner = self.inner.lock().unwrap(); let current_height = inner.best_block.height; - if current_height.saturating_sub(ANTI_REORG_DELAY) + 1 >= confirmation_height { - inner.get_spendable_outputs(tx) - } else { - Vec::new() - } + let mut spendable_outputs = inner.get_spendable_outputs(tx); + spendable_outputs.retain(|descriptor| { + let mut conf_threshold = current_height.saturating_sub(ANTI_REORG_DELAY) + 1; + if let SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) = descriptor { + conf_threshold = cmp::min(conf_threshold, + current_height.saturating_sub(descriptor.to_self_delay as u32) + 1); + } + conf_threshold >= confirmation_height + }); + spendable_outputs } } diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index f0cc872db0a..c950d13bb01 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -637,7 +637,7 @@ fn test_balances_on_local_commitment_htlcs() { // First confirm the commitment transaction on nodes[0], which should leave us with three // claimable balances. let node_a_commitment_claimable = nodes[0].best_block_info().1 + BREAKDOWN_TIMEOUT as u32; - mine_transaction(&nodes[0], &as_txn[0]); + let commitment_tx_conf_height_a = block_from_scid(&mine_transaction(&nodes[0], &as_txn[0])); check_added_monitors!(nodes[0], 1); check_closed_broadcast!(nodes[0], true); check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed, [nodes[1].node.get_our_node_id()], 1000000); @@ -729,13 +729,20 @@ fn test_balances_on_local_commitment_htlcs() { // Connect blocks until the commitment transaction's CSV expires, providing us the relevant // `SpendableOutputs` event and removing the claimable balance entry. - connect_blocks(&nodes[0], node_a_commitment_claimable - nodes[0].best_block_info().1); + connect_blocks(&nodes[0], node_a_commitment_claimable - nodes[0].best_block_info().1 - 1); + assert!(get_monitor!(nodes[0], chan_id) + .get_spendable_outputs(&as_txn[0], commitment_tx_conf_height_a).is_empty()); + connect_blocks(&nodes[0], 1); assert_eq!(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, confirmation_height: node_a_htlc_claimable, }], nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); - test_spendable_output(&nodes[0], &as_txn[0]); + let to_self_spendable_output = test_spendable_output(&nodes[0], &as_txn[0]); + assert_eq!( + get_monitor!(nodes[0], chan_id).get_spendable_outputs(&as_txn[0], commitment_tx_conf_height_a), + to_self_spendable_output + ); // Connect blocks until the HTLC-Timeout's CSV expires, providing us the relevant // `SpendableOutputs` event and removing the claimable balance entry.