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

…annelManager

lightning/src/chain/chainmonitor.rs

@@ -669,10 +669,10 @@ where C::Target: chain::Filter,
 		}
 	}
 
-	fn get_latest_holder_commitment_txn(&self, funding_txo: OutPoint) -> Vec<bitcoin::Transaction> {
+	fn get_latest_holder_commitment_txn(&self, funding_txo: OutPoint) -> Result<Vec<bitcoin::Transaction>, ()> {
 		let monitors = self.monitors.read().unwrap();
-		let monitor = monitors.get(&funding_txo).expect("To have a monitor for the requested channel");
-		monitor.monitor.get_latest_holder_commitment_txn_internal(&self.logger)
+		let monitor = monitors.get(&funding_txo).ok_or(())?;
+		Ok(monitor.monitor.get_latest_holder_commitment_txn_internal(&self.logger))
 	}
 
 	/// Note that we persist the given `ChannelMonitor` update while holding the

lightning/src/chain/mod.rs

@@ -300,7 +300,7 @@ pub trait Watch<ChannelSigner: WriteableEcdsaChannelSigner> {
 	fn update_channel_funding_txo(&self, old_funding_txo: OutPoint, new_funding_txo: OutPoint, channel_value_satoshis: u64) -> ChannelMonitorUpdateStatus;
 
 	/// Get the latest commitment transaction to broadcast
-	fn get_latest_holder_commitment_txn(&self, funding_txo: OutPoint) -> Vec<bitcoin::Transaction>;
+	fn get_latest_holder_commitment_txn(&self, funding_txo: OutPoint) -> Result<Vec<bitcoin::Transaction>, ()>;
 
 	/// Returns any monitor events since the last call. Subsequent calls must only return new
 	/// events.

lightning/src/ln/chan_utils.rs

@@ -823,7 +823,14 @@ pub struct ChannelTransactionParameters {
 	/// The late-bound counterparty channel transaction parameters.
 	/// These parameters are populated at the point in the protocol where the counterparty provides them.
 	pub counterparty_parameters: Option<CounterpartyChannelTransactionParameters>,
-	/// The late-bound funding outpoint
+	/// The late-bound funding outpoint.
+	///
+	/// 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
+	/// transaction.
 	pub funding_outpoint: Option<chain::transaction::OutPoint>,
 	/// Are anchors (zero fee HTLC transaction variant) used for this channel. Boolean is
 	/// serialization backwards-compatible.
@@ -832,7 +839,9 @@ pub struct ChannelTransactionParameters {
 	/// It is intended merely for backwards compatibility with signers that need it.
 	/// There is no support for this feature in LDK channel negotiation.
 	pub opt_non_zero_fee_anchors: Option<()>,
-	///
+	/// This value always corresponds to the actual funding outpoint. This is different to
+	/// [`ChannelTransactionParameters::funding_outpoint`], which varies depending on the type
+	/// of Lightning channel we have.
 	pub original_funding_outpoint: Option<chain::transaction::OutPoint>,
 }
 

lightning/src/ln/channel.rs

@@ -5120,21 +5120,45 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 					}
 				}
 
-				let original_funding_outpoint = self.get_original_funding_txo().map(|x| x.into_bitcoin_outpoint());
-				let funding_bitcoin_outpoint = funding_txo.into_bitcoin_outpoint();
-				let funding_match = |prev_outpoint: &bitcoin::OutPoint| -> bool {
-					prev_outpoint == &funding_bitcoin_outpoint
+				// If we have a vanilla LN channel, this checks if the transaction
+				// spends from the actual funding output.
+				//
+				// 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.
+				let is_funding_or_glue_txo = |prev_outpoint: &bitcoin::OutPoint| -> bool {
+					prev_outpoint == &funding_txo.into_bitcoin_outpoint()
 				};
-				let original_funding_match = |prev_outpoint: &bitcoin::OutPoint, outputs: &[bitcoin::TxOut]| -> bool {
-					if let Some(original_funding_outpoint) = original_funding_outpoint {
-						// We want to filter for the split tx which is not closing the LN channel.
-						prev_outpoint == &original_funding_outpoint && !(outputs.len() == 2 && outputs[0].script_pubkey == outputs[1].script_pubkey)
-					} else {
-						false
+
+				// This check only runs if the check above returns `false`.
+				//
+				// 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`.
+				//
+				// 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!
+				//
+				// 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 {
+					match self.get_original_funding_txo().map(|x| x.into_bitcoin_outpoint()) {
+						Some(original_funding_outpoint) => {
+						        // Transaction spends from actual funding output.
+						        prev_outpoint == &original_funding_outpoint &&
+							// Transaction is _not_ a split transaction.
+						        !(outputs.len() == 2 && outputs[0].script_pubkey == outputs[1].script_pubkey)
+					        }
+						None => false,
 					}
 				};
 				for inp in tx.input.iter() {
-					if funding_match(&inp.previous_output) || original_funding_match(&inp.previous_output, &tx.output) {
+					if is_funding_or_glue_txo(&inp.previous_output) || is_revoked_commitment_transaction(&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);
 					}

lightning/src/ln/channelmanager.rs

@@ -2020,10 +2020,12 @@ where
 		}
 	}
 
-	fn get_latest_holder_commitment_txn_internal(&self, channel_lock: &ChannelLock<<SP::Target as SignerProvider>::Signer>) -> Vec<Transaction> {
+	fn get_latest_holder_commitment_txn_internal(&self, channel_lock: &ChannelLock<<SP::Target as SignerProvider>::Signer>) -> Result<Vec<Transaction>, APIError> {
 		let chan = &channel_lock.channel;
 
-		self.chain_monitor.get_latest_holder_commitment_txn(chan.get_original_funding_txo().expect("To have a funding txo"))
+		self.chain_monitor.get_latest_holder_commitment_txn(
+			chan.get_original_funding_txo().ok_or_else(|| APIError::APIMisuseError { err: "Channel does not have a funding txo.".to_string() })?
+		).map_err(|_| APIError::APIMisuseError { err: "No channel monitor for channel".to_string() })
 	}
 
 	fn close_channel_internal(&self, channel_id: &[u8; 32], counterparty_node_id: &PublicKey, target_feerate_sats_per_1000_weight: Option<u32>) -> Result<(), APIError> {
@@ -2156,7 +2158,7 @@ where
 	}
 
 	///
-	pub fn get_latest_holder_commitment_txn(&self, channel_lock: &ChannelLock<<SP::Target as SignerProvider>::Signer>) -> Vec<Transaction> {
+	pub fn get_latest_holder_commitment_txn(&self, channel_lock: &ChannelLock<<SP::Target as SignerProvider>::Signer>) -> Result<Vec<Transaction>, APIError> {
 		self.get_latest_holder_commitment_txn_internal(channel_lock)
 	}
 

lightning/src/util/test_utils.rs

@@ -257,6 +257,10 @@ impl<'a> chain::Watch<EnforcingSigner> for TestChainMonitor<'a> {
 	fn update_channel_funding_txo(&self, _: OutPoint, new_funding_txo: OutPoint, _: u64) -> chain::ChannelMonitorUpdateStatus {
 		todo!()
 	}
+
+	fn get_latest_holder_commitment_txn(&self, _funding_txo: OutPoint) -> Result<Vec<bitcoin::Transaction>, ()> {
+	    todo!()
+	}
 }
 
 pub struct TestPersister {