diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index a0634759d2f..31a90f3be16 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/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 { + fn get_latest_holder_commitment_txn(&self, funding_txo: OutPoint) -> Result, ()> { 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 diff --git a/lightning/src/chain/mod.rs b/lightning/src/chain/mod.rs index 3d822b4fb35..e7d97fde249 100644 --- a/lightning/src/chain/mod.rs +++ b/lightning/src/chain/mod.rs @@ -300,7 +300,7 @@ pub trait Watch { 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; + fn get_latest_holder_commitment_txn(&self, funding_txo: OutPoint) -> Result, ()>; /// Returns any monitor events since the last call. Subsequent calls must only return new /// events. diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index a7bd179ec35..bd682f5917a 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/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, - /// 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, /// 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, } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1b433e3e655..969b2e433f9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5120,21 +5120,45 @@ impl Channel { } } - 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); } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 19d37c1ab70..984693ea324 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2020,10 +2020,12 @@ where } } - fn get_latest_holder_commitment_txn_internal(&self, channel_lock: &ChannelLock<::Signer>) -> Vec { + fn get_latest_holder_commitment_txn_internal(&self, channel_lock: &ChannelLock<::Signer>) -> Result, 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) -> Result<(), APIError> { @@ -2156,7 +2158,7 @@ where } /// - pub fn get_latest_holder_commitment_txn(&self, channel_lock: &ChannelLock<::Signer>) -> Vec { + pub fn get_latest_holder_commitment_txn(&self, channel_lock: &ChannelLock<::Signer>) -> Result, APIError> { self.get_latest_holder_commitment_txn_internal(channel_lock) } diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 830ca7676de..301536e1f97 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -257,6 +257,10 @@ impl<'a> chain::Watch 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, ()> { + todo!() + } } pub struct TestPersister {