fixup! fixup! Add ability to retrieve latest commitment transactions …

…from ChannelManager

lightning/src/ln/chan_utils.rs

@@ -828,8 +828,8 @@ pub struct ChannelTransactionParameters {
 	/// If it's a vanilla LN channel, this value corresponds to the actual funding outpoint that
 	/// goes on-chain when the channel is created.
 	///
-	/// If instead we're dealing with a split channel, this value corresponds to a glue
-	/// transaction which sits in between the funding transaction and the commitment
+	/// If instead we're dealing with a split channel, this value corresponds to the output of a
+	/// glue transaction which sits in between the funding transaction and the commitment
 	/// transaction.
 	pub funding_outpoint: Option<chain::transaction::OutPoint>,
 	/// Are anchors (zero fee HTLC transaction variant) used for this channel. Boolean is

lightning/src/ln/channel.rs

@@ -5121,32 +5121,40 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 				}
 
 				// If we have a vanilla LN channel, this checks if the transaction
-				// spends from the actual funding output.
+				// spends from the actual funding output. That could be either a
+				// commitment transaction or a mutual close transaction.
 				//
 				// If we have a split channel, this checks if the transaction spends
-				// from the glue output.
-				//
-				// In both cases we are checking if this transaction is the
-				// commitment transaction.
+				// from the glue output. That could only be a commitment
+				// transaction.
 				let is_funding_or_glue_txo = |prev_outpoint: &bitcoin::OutPoint| -> bool {
 					prev_outpoint == &funding_txo.into_bitcoin_outpoint()
 				};
 
-				// This check only runs if the check above returns `false`.
+				// This check only runs if the check above returns `false`. We know
+				// that a vanilla LN channel can only be closed by spending from the
+				// original funding output, so in this check we are only considering
+				// split channels.
 				//
-				// Given that, if we have a vanilla LN channel this check will
-				// return `false` too, because for vanilla LN channels `funding_txo
-				// == original_funding_txo`.
+				// The other ways in which a split channel could be closed are:
 				//
-				// If we have a split channel, this checks if the transaction spends
-				// from the actual funding output BUT it is _not_ the split
-				// transaction. This can happen if the split channel is closed using
-				// a revoked commitment transaction!
+				// - Through a mutual close of the _LN_ channel, which would spend
+				// directly from the original funding output.
+				//
+				// - Through the publication of a revoked commitment transaction
+				// spending from the original funding output!
+				//
+				// And that's exactly what we check here: whether the transaction
+				// spends from the original funding output and, if it does, whether
+				// the transaction is NOT the split transaction (the only other
+				// possible option).
 				//
-				// We purposefully do not announce the closure of the channel with
-				// the confirmation of the split transaction. TODO(lucas): Exactly
-				// why is this, Tibo?
-				let is_revoked_commitment_transaction = |prev_outpoint: &bitcoin::OutPoint, outputs: &[bitcoin::TxOut]| -> bool {
+				// We do not announce the closing of the LN channel with the split
+				// transaction, because that is reserved to either mutual close or
+				// commitment transactions. LDK will only react to this announcement
+				// once, so we should not waste it on the split transaction, as this
+				// can lead to loss of funds.
+				let is_final_tx_spending_from_original_funding_txo = |prev_outpoint: &bitcoin::OutPoint, outputs: &[bitcoin::TxOut]| -> bool {
 					match self.get_original_funding_txo().map(|x| x.into_bitcoin_outpoint()) {
 						Some(original_funding_outpoint) => {
 						        // Transaction spends from actual funding output.
@@ -5158,7 +5166,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 					}
 				};
 				for inp in tx.input.iter() {
-					if is_funding_or_glue_txo(&inp.previous_output) || is_revoked_commitment_transaction(&inp.previous_output, &tx.output) {
+					if is_funding_or_glue_txo(&inp.previous_output) || is_final_tx_spending_from_original_funding_txo(&inp.previous_output, &tx.output) {
 						log_info!(logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.txid(), inp.previous_output.txid, inp.previous_output.vout, log_bytes!(self.channel_id()));
 						return Err(ClosureReason::CommitmentTxConfirmed);
 					}