From a6d4b1e22bfede023fe438dfaee8860ae9d5c201 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 26 Jun 2025 10:06:28 -0700 Subject: [PATCH] Introduce RenegotiatedFundingLocked monitor update variant This is a new `ChannelMonitorUpdateStep` variant intended to be used whenever a new funding transaction that was negotiated and applied via the `RenegotiatedFunding` update reaches its intended confirmation depth and both sides of the channel exchange `channel_ready`/`splice_locked`. This commit primarily focuses on its use for splices, but future work will expand where needed to support RBFs for a dual funded channel. This monitor update ensures that the monitor can safely drop all prior commitment data since it is now considered invalid/unnecessary. Once the update is applied, only state for the new funding transaction is tracked going forward, until the monitor receives another `RenegotiatedFunding` update. --- lightning/src/chain/channelmonitor.rs | 60 +++++++- lightning/src/chain/onchaintx.rs | 9 ++ lightning/src/ln/channel.rs | 202 ++++++++++++++------------ lightning/src/ln/channelmanager.rs | 104 +++++++++---- 4 files changed, 250 insertions(+), 125 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index bfa5acc3bd5..e3db2d69f8f 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -676,6 +676,9 @@ pub(crate) enum ChannelMonitorUpdateStep { holder_commitment_tx: HolderCommitmentTransaction, counterparty_commitment_tx: CommitmentTransaction, }, + RenegotiatedFundingLocked { + funding_txid: Txid, + }, } impl ChannelMonitorUpdateStep { @@ -690,6 +693,7 @@ impl ChannelMonitorUpdateStep { ChannelMonitorUpdateStep::ChannelForceClosed { .. } => "ChannelForceClosed", ChannelMonitorUpdateStep::ShutdownScript { .. } => "ShutdownScript", ChannelMonitorUpdateStep::RenegotiatedFunding { .. } => "RenegotiatedFunding", + ChannelMonitorUpdateStep::RenegotiatedFundingLocked { .. } => "RenegotiatedFundingLocked", } } } @@ -733,6 +737,9 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep, (3, holder_commitment_tx, required), (5, counterparty_commitment_tx, required), }, + (12, RenegotiatedFundingLocked) => { + (1, funding_txid, required), + }, ); /// Indicates whether the balance is derived from a cooperative close, a force-close @@ -1075,6 +1082,10 @@ impl FundingScope { fn funding_txid(&self) -> Txid { self.funding_outpoint().txid } + + fn is_splice(&self) -> bool { + self.channel_parameters.splice_parent_funding_txid.is_some() + } } impl Writeable for FundingScope { @@ -1209,8 +1220,6 @@ pub(crate) struct ChannelMonitorImpl { // interface knows about the TXOs that we want to be notified of spends of. We could probably // be smart and derive them from the above storage fields, but its much simpler and more // Obviously Correct (tm) if we just keep track of them explicitly. - // - // TODO: Remove entries for stale funding transactions on `splice_locked`. outputs_to_watch: HashMap>, #[cfg(any(test, feature = "_test_utils"))] @@ -3670,6 +3679,10 @@ impl ChannelMonitorImpl { ); return Err(()); } + } else if self.funding.is_splice() { + // If we've already spliced at least once, we're no longer able to RBF the original + // funding transaction. + return Err(()); } self.outputs_to_watch.insert( @@ -3681,6 +3694,39 @@ impl ChannelMonitorImpl { Ok(()) } + fn promote_funding(&mut self, new_funding_txid: Txid) -> Result<(), ()> { + let is_pending_splice = self.pending_funding.iter().any(|funding| funding.is_splice()); + + let new_funding = self + .pending_funding + .iter_mut() + .find(|funding| funding.funding_txid() == new_funding_txid); + if new_funding.is_none() { + return Err(()); + } + let mut new_funding = new_funding.unwrap(); + + // `first_confirmed_funding_txo` is set to the first outpoint for the channel upon init. + // If an RBF happens and it confirms, this will no longer be accurate, so update it now + // if we know the RBF doesn't belong to a splice. + if !is_pending_splice && self.first_confirmed_funding_txo == self.funding.funding_outpoint() + { + self.first_confirmed_funding_txo = new_funding.funding_outpoint(); + } + + mem::swap(&mut self.funding, &mut new_funding); + self.onchain_tx_handler.update_after_renegotiated_funding_locked( + self.funding.current_holder_commitment_tx.clone(), + self.funding.prev_holder_commitment_tx.clone(), + ); + + for funding in self.pending_funding.drain(..) { + self.outputs_to_watch.remove(&funding.funding_txid()); + } + + Ok(()) + } + #[rustfmt::skip] fn update_monitor( &mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &WithChannelMonitor @@ -3771,6 +3817,13 @@ impl ChannelMonitorImpl { ret = Err(()); } }, + ChannelMonitorUpdateStep::RenegotiatedFundingLocked { funding_txid } => { + log_trace!(logger, "Updating ChannelMonitor with locked renegotiated funding txid {}", funding_txid); + if let Err(_) = self.promote_funding(*funding_txid) { + log_error!(logger, "Unknown funding with txid {} became locked", funding_txid); + ret = Err(()); + } + }, ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } => { log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast); self.lockdown_from_offchain = true; @@ -3823,7 +3876,8 @@ impl ChannelMonitorImpl { |ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. } |ChannelMonitorUpdateStep::ShutdownScript { .. } |ChannelMonitorUpdateStep::CommitmentSecret { .. } - |ChannelMonitorUpdateStep::RenegotiatedFunding { .. } => + |ChannelMonitorUpdateStep::RenegotiatedFunding { .. } + |ChannelMonitorUpdateStep::RenegotiatedFundingLocked { .. } => is_pre_close_update = true, // After a channel is closed, we don't communicate with our peer about it, so the // only things we will update is getting a new preimage (from a different channel) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index fd0f0d9abf5..89952ebee6b 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -1216,6 +1216,15 @@ impl OnchainTxHandler { self.prev_holder_commitment = Some(replace(&mut self.holder_commitment, tx)); } + /// Replaces the current/prev holder commitment transactions spending the currently confirmed + /// funding outpoint with those spending the new funding outpoint. + pub(crate) fn update_after_renegotiated_funding_locked( + &mut self, current: HolderCommitmentTransaction, prev: Option, + ) { + self.holder_commitment = current; + self.prev_holder_commitment = prev; + } + // Deprecated as of 0.2, only use in cases where it was not previously available. pub(crate) fn channel_parameters(&self) -> &ChannelTransactionParameters { &self.channel_transaction_parameters diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 56273d126ce..52687c31df4 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -6037,6 +6037,12 @@ type BestBlockUpdatedRes = ( Option, ); +pub struct SpliceFundingPromotion { + pub funding_txo: OutPoint, + pub monitor_update: Option, + pub announcement_sigs: Option, +} + impl FundedChannel where SP::Target: SignerProvider, @@ -9523,45 +9529,42 @@ where self.context.check_funding_meets_minimum_depth(funding, height) } + /// Returns `Some` if a splice [`FundingScope`] was promoted. #[cfg(splicing)] - fn maybe_promote_splice_funding( - &mut self, confirmed_funding_index: usize, logger: &L, - ) -> bool + fn maybe_promote_splice_funding( + &mut self, node_signer: &NS, chain_hash: ChainHash, user_config: &UserConfig, + block_height: u32, logger: &L, + ) -> Option where + NS::Target: NodeSigner, L::Target: Logger, { debug_assert!(self.pending_splice.is_some()); - debug_assert!(confirmed_funding_index < self.pending_funding.len()); let pending_splice = self.pending_splice.as_mut().unwrap(); let splice_txid = match pending_splice.sent_funding_txid { Some(sent_funding_txid) => sent_funding_txid, None => { debug_assert!(false); - return false; + return None; }, }; - if pending_splice.sent_funding_txid == pending_splice.received_funding_txid { - log_info!( - logger, - "Promoting splice funding txid {} for channel {}", - splice_txid, - &self.context.channel_id, - ); - - let funding = self.pending_funding.get_mut(confirmed_funding_index).unwrap(); - debug_assert_eq!(Some(splice_txid), funding.get_funding_txid()); - promote_splice_funding!(self, funding); + if let Some(received_funding_txid) = pending_splice.received_funding_txid { + if splice_txid != received_funding_txid { + log_warn!( + logger, + "Mismatched splice_locked txid for channel {}; sent txid {}; received txid {}", + &self.context.channel_id, + splice_txid, + received_funding_txid, + ); + return None; + } - return true; - } else if let Some(received_funding_txid) = pending_splice.received_funding_txid { - log_warn!( - logger, - "Mismatched splice_locked txid for channel {}; sent txid {}; received txid {}", - &self.context.channel_id, - splice_txid, - received_funding_txid, + debug_assert_eq!( + pending_splice.sent_funding_txid, + pending_splice.received_funding_txid ); } else { log_info!( @@ -9570,9 +9573,47 @@ where splice_txid, &self.context.channel_id, ); + return None; + } + + log_info!( + logger, + "Promoting splice funding txid {} for channel {}", + splice_txid, + &self.context.channel_id, + ); + + { + // Scope `funding` since it is swapped within `promote_splice_funding` and we don't want + // to unintentionally use it. + let funding = self + .pending_funding + .iter_mut() + .find(|funding| funding.get_funding_txid() == Some(splice_txid)) + .unwrap(); + promote_splice_funding!(self, funding); } - return false; + let funding_txo = self + .funding + .get_funding_txo() + .expect("Splice FundingScope should always have a funding_txo"); + + self.context.latest_monitor_update_id += 1; + let monitor_update = ChannelMonitorUpdate { + update_id: self.context.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::RenegotiatedFundingLocked { + funding_txid: funding_txo.txid, + }], + channel_id: Some(self.context.channel_id()), + }; + self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new()); + let monitor_update = self.push_ret_blockable_mon_update(monitor_update); + + let announcement_sigs = + self.get_announcement_sigs(node_signer, chain_hash, user_config, block_height, logger); + + Some(SpliceFundingPromotion { funding_txo, monitor_update, announcement_sigs }) } /// When a transaction is confirmed, we check whether it is or spends the funding transaction @@ -9666,18 +9707,16 @@ where &self.context.channel_id, ); - let funding_promoted = - self.maybe_promote_splice_funding(confirmed_funding_index, logger); - let funding_txo = funding_promoted.then(|| { - self.funding - .get_funding_txo() - .expect("Splice FundingScope should always have a funding_txo") - }); - let announcement_sigs = funding_promoted - .then(|| self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger)) - .flatten(); + let (funding_txo, monitor_update, announcement_sigs) = + self.maybe_promote_splice_funding( + node_signer, chain_hash, user_config, height, logger, + ).map(|splice_promotion| ( + Some(splice_promotion.funding_txo), + splice_promotion.monitor_update, + splice_promotion.announcement_sigs, + )).unwrap_or((None, None, None)); - return Ok((Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo)), announcement_sigs)); + return Ok((Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo, monitor_update)), announcement_sigs)); } } @@ -9835,22 +9874,20 @@ where if let Some(splice_locked) = pending_splice.check_get_splice_locked(&self.context, funding, height) { log_info!(logger, "Sending a splice_locked to our peer for channel {}", &self.context.channel_id); - let funding_promoted = - self.maybe_promote_splice_funding(confirmed_funding_index, logger); - let funding_txo = funding_promoted.then(|| { - self.funding - .get_funding_txo() - .expect("Splice FundingScope should always have a funding_txo") - }); - let announcement_sigs = funding_promoted - .then(|| chain_node_signer - .and_then(|(chain_hash, node_signer, user_config)| - self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger) - ) - ) - .flatten(); + let (funding_txo, monitor_update, announcement_sigs) = chain_node_signer + .and_then(|(chain_hash, node_signer, user_config)| { + // We can only promote on blocks connected, which is when we expect + // `chain_node_signer` to be `Some`. + self.maybe_promote_splice_funding(node_signer, chain_hash, user_config, height, logger) + }) + .map(|splice_promotion| ( + Some(splice_promotion.funding_txo), + splice_promotion.monitor_update, + splice_promotion.announcement_sigs, + )) + .unwrap_or((None, None, None)); - return Ok((Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo)), timed_out_htlcs, announcement_sigs)); + return Ok((Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo, monitor_update)), timed_out_htlcs, announcement_sigs)); } } @@ -10344,8 +10381,8 @@ where #[cfg(splicing)] pub fn splice_locked( &mut self, msg: &msgs::SpliceLocked, node_signer: &NS, chain_hash: ChainHash, - user_config: &UserConfig, best_block: &BestBlock, logger: &L, - ) -> Result<(Option, Option), ChannelError> + user_config: &UserConfig, block_height: u32, logger: &L, + ) -> Result, ChannelError> where NS::Target: NodeSigner, L::Target: Logger, @@ -10364,56 +10401,33 @@ where }, }; - if let Some(sent_funding_txid) = pending_splice.sent_funding_txid { - if sent_funding_txid == msg.splice_txid { - if let Some(funding) = self - .pending_funding - .iter_mut() - .find(|funding| funding.get_funding_txid() == Some(sent_funding_txid)) - { - log_info!( - logger, - "Promoting splice funding txid {} for channel {}", - msg.splice_txid, - &self.context.channel_id, - ); - promote_splice_funding!(self, funding); - let funding_txo = self - .funding - .get_funding_txo() - .expect("Splice FundingScope should always have a funding_txo"); - let announcement_sigs = self.get_announcement_sigs( - node_signer, - chain_hash, - user_config, - best_block.height, - logger, - ); - return Ok((Some(funding_txo), announcement_sigs)); - } + if !self + .pending_funding + .iter() + .any(|funding| funding.get_funding_txid() == Some(msg.splice_txid)) + { + let err = "unknown splice funding txid"; + return Err(ChannelError::close(err.to_string())); + } + pending_splice.received_funding_txid = Some(msg.splice_txid); - let err = "unknown splice funding txid"; - return Err(ChannelError::close(err.to_string())); - } else { - log_warn!( - logger, - "Mismatched splice_locked txid for channel {}; sent txid {}; received txid {}", - &self.context.channel_id, - sent_funding_txid, - msg.splice_txid, - ); - } - } else { + if pending_splice.sent_funding_txid.is_none() { log_info!( logger, "Waiting for enough confirmations to send splice_locked txid {} for channel {}", msg.splice_txid, &self.context.channel_id, ); + return Ok(None); } - pending_splice.received_funding_txid = Some(msg.splice_txid); - Ok((None, None)) + Ok(self.maybe_promote_splice_funding( + node_signer, + chain_hash, + user_config, + block_height, + logger, + )) } // Send stuff to our remote peers: diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9e7031bf8cb..2e6824ad775 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7655,7 +7655,8 @@ where ComplFunc: FnOnce( Option, bool, - ) -> (Option, Option), + ) + -> (Option, Option), >( &self, prev_hop: HTLCPreviousHopData, payment_preimage: PaymentPreimage, payment_info: Option, completion_action: ComplFunc, @@ -10219,15 +10220,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self.node_signer, self.chain_hash, &self.default_configuration, - &self.best_block.read().unwrap(), + self.best_block.read().unwrap().height, &&logger, ); - let (funding_txo, announcement_sigs_opt) = - try_channel_entry!(self, peer_state, result, chan_entry); - - if funding_txo.is_some() { - let mut short_to_chan_info = self.short_to_chan_info.write().unwrap(); - insert_short_channel_id!(short_to_chan_info, chan); + let splice_promotion = try_channel_entry!(self, peer_state, result, chan_entry); + if let Some(splice_promotion) = splice_promotion { + { + let mut short_to_chan_info = self.short_to_chan_info.write().unwrap(); + insert_short_channel_id!(short_to_chan_info, chan); + } let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push_back(( @@ -10235,26 +10236,39 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ channel_id: chan.context.channel_id(), user_channel_id: chan.context.get_user_id(), counterparty_node_id: chan.context.get_counterparty_node_id(), - funding_txo: funding_txo - .map(|outpoint| outpoint.into_bitcoin_outpoint()), + funding_txo: Some( + splice_promotion.funding_txo.into_bitcoin_outpoint(), + ), channel_type: chan.funding.get_channel_type().clone(), }, None, )); - } - if let Some(announcement_sigs) = announcement_sigs_opt { - log_trace!( - logger, - "Sending announcement_signatures for channel {}", - chan.context.channel_id() - ); - peer_state.pending_msg_events.push( - MessageSendEvent::SendAnnouncementSignatures { - node_id: counterparty_node_id.clone(), - msg: announcement_sigs, - }, - ); + if let Some(announcement_sigs) = splice_promotion.announcement_sigs { + log_trace!( + logger, + "Sending announcement_signatures for channel {}", + chan.context.channel_id() + ); + peer_state.pending_msg_events.push( + MessageSendEvent::SendAnnouncementSignatures { + node_id: counterparty_node_id.clone(), + msg: announcement_sigs, + }, + ); + } + + if let Some(monitor_update) = splice_promotion.monitor_update { + handle_new_monitor_update!( + self, + splice_promotion.funding_txo, + monitor_update, + peer_state_lock, + peer_state, + per_peer_state, + chan + ); + } } } else { return Err(MsgHandleErrInternal::send_err_msg_no_close( @@ -12280,7 +12294,7 @@ where pub(super) enum FundingConfirmedMessage { Establishment(msgs::ChannelReady), #[cfg(splicing)] - Splice(msgs::SpliceLocked, Option), + Splice(msgs::SpliceLocked, Option, Option), } impl< @@ -12317,6 +12331,8 @@ where let mut failed_channels = Vec::new(); let mut timed_out_htlcs = Vec::new(); + #[cfg(splicing)] + let mut monitor_updates = Vec::new(); { let per_peer_state = self.per_peer_state.read().unwrap(); for (_cp_id, peer_state_mutex) in per_peer_state.iter() { @@ -12354,25 +12370,32 @@ where } }, #[cfg(splicing)] - Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo)) => { + Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo, monitor_update_opt)) => { + let counterparty_node_id = funded_channel.context.get_counterparty_node_id(); + let channel_id = funded_channel.context.channel_id(); + if funding_txo.is_some() { let mut short_to_chan_info = self.short_to_chan_info.write().unwrap(); insert_short_channel_id!(short_to_chan_info, funded_channel); let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push_back((events::Event::ChannelReady { - channel_id: funded_channel.context.channel_id(), + channel_id, user_channel_id: funded_channel.context.get_user_id(), - counterparty_node_id: funded_channel.context.get_counterparty_node_id(), + counterparty_node_id, funding_txo: funding_txo.map(|outpoint| outpoint.into_bitcoin_outpoint()), channel_type: funded_channel.funding.get_channel_type().clone(), }, None)); } pending_msg_events.push(MessageSendEvent::SendSpliceLocked { - node_id: funded_channel.context.get_counterparty_node_id(), + node_id: counterparty_node_id, msg: splice_locked, }); + + if let Some(monitor_update) = monitor_update_opt { + monitor_updates.push((counterparty_node_id, channel_id, monitor_update)); + } }, None => {}, } @@ -12470,6 +12493,31 @@ where } } + #[cfg(splicing)] + for (counterparty_node_id, channel_id, monitor_update) in monitor_updates { + let per_peer_state = self.per_peer_state.read().unwrap(); + if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) { + let mut peer_state_lock = peer_state_mutex.lock().unwrap(); + let peer_state = &mut *peer_state_lock; + if let Some(channel) = peer_state + .channel_by_id + .get_mut(&channel_id) + .and_then(Channel::as_funded_mut) + { + let funding_txo = channel.funding.get_funding_txo().unwrap(); + handle_new_monitor_update!( + self, + funding_txo, + monitor_update, + peer_state_lock, + peer_state, + per_peer_state, + channel + ); + } + } + } + if let Some(height) = height_opt { self.claimable_payments.lock().unwrap().claimable_payments.retain(|payment_hash, payment| { payment.htlcs.retain(|htlc| {