Merge pull request #2624 from wpaulino/2609-follow-up
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Fri, 29 Sep 2023 20:07:16 +0000 (20:07 +0000)
committerGitHub <noreply@github.com>
Fri, 29 Sep 2023 20:07:16 +0000 (20:07 +0000)
Address 2609 follow-up comments

1  2 
lightning/src/chain/channelmonitor.rs

index 9fa947a82da1d9c9cbe42dbc98e57e33f3161955,cfd29759bb5058c109e99a2b7ebd55b54b490a07..5d3e0ac28bbf45058068761db4cad78f1283558c
@@@ -1672,6 -1672,8 +1672,8 @@@ impl<Signer: WriteableEcdsaChannelSigne
  
        /// 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`]
        /// 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.
        pub fn get_spendable_outputs(&self, tx: &Transaction, confirmation_height: u32) -> Vec<SpendableOutputDescriptor> {
                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
        }
  }
  
@@@ -2503,18 -2509,6 +2509,18 @@@ impl<Signer: WriteableEcdsaChannelSigne
        {
                self.payment_preimages.insert(payment_hash.clone(), payment_preimage.clone());
  
 +              let confirmed_spend_txid = self.funding_spend_confirmed.or_else(|| {
 +                      self.onchain_events_awaiting_threshold_conf.iter().find_map(|event| match event.event {
 +                              OnchainEvent::FundingSpendConfirmation { .. } => Some(event.txid),
 +                              _ => None,
 +                      })
 +              });
 +              let confirmed_spend_txid = if let Some(txid) = confirmed_spend_txid {
 +                      txid
 +              } else {
 +                      return;
 +              };
 +
                // If the channel is force closed, try to claim the output from this preimage.
                // First check if a counterparty commitment transaction has been broadcasted:
                macro_rules! claim_htlcs {
                        }
                }
                if let Some(txid) = self.current_counterparty_commitment_txid {
 -                      if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
 -                              claim_htlcs!(*commitment_number, txid);
 +                      if txid == confirmed_spend_txid {
 +                              if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
 +                                      claim_htlcs!(*commitment_number, txid);
 +                              } else {
 +                                      debug_assert!(false);
 +                                      log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number");
 +                              }
                                return;
                        }
                }
                if let Some(txid) = self.prev_counterparty_commitment_txid {
 -                      if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
 -                              claim_htlcs!(*commitment_number, txid);
 +                      if txid == confirmed_spend_txid {
 +                              if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
 +                                      claim_htlcs!(*commitment_number, txid);
 +                              } else {
 +                                      debug_assert!(false);
 +                                      log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number");
 +                              }
                                return;
                        }
                }
                // *we* sign a holder commitment transaction, not when e.g. a watchtower broadcasts one of our
                // holder commitment transactions.
                if self.broadcasted_holder_revokable_script.is_some() {
 -                      // Assume that the broadcasted commitment transaction confirmed in the current best
 -                      // block. Even if not, its a reasonable metric for the bump criteria on the HTLC
 -                      // transactions.
 -                      let (claim_reqs, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, self.best_block.height());
 -                      self.onchain_tx_handler.update_claims_view_from_requests(claim_reqs, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger);
 -                      if let Some(ref tx) = self.prev_holder_signed_commitment_tx {
 -                              let (claim_reqs, _) = self.get_broadcasted_holder_claims(&tx, self.best_block.height());
 +                      let holder_commitment_tx = if self.current_holder_commitment_tx.txid == confirmed_spend_txid {
 +                              Some(&self.current_holder_commitment_tx)
 +                      } else if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
 +                              if prev_holder_commitment_tx.txid == confirmed_spend_txid {
 +                                      Some(prev_holder_commitment_tx)
 +                              } else {
 +                                      None
 +                              }
 +                      } else {
 +                              None
 +                      };
 +                      if let Some(holder_commitment_tx) = holder_commitment_tx {
 +                              // Assume that the broadcasted commitment transaction confirmed in the current best
 +                              // block. Even if not, its a reasonable metric for the bump criteria on the HTLC
 +                              // transactions.
 +                              let (claim_reqs, _) = self.get_broadcasted_holder_claims(&holder_commitment_tx, self.best_block.height());
                                self.onchain_tx_handler.update_claims_view_from_requests(claim_reqs, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger);
                        }
                }