From 8283ad4608faaaad44b468d6c609715cbd76301b Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 17 Mar 2026 14:44:45 -0700 Subject: [PATCH 1/2] Change FundingInfo::Contribution to expose contributed output scripts Exposing the amounts for each output isn't very helpful because it's possible that they vary across over multiple splice candidates due to RBF. This commit changes `FundingInfo::Contribution` and several of the helpers used to derive it to be based on output scripts instead. --- lightning/src/events/mod.rs | 6 ++-- lightning/src/ln/channel.rs | 47 +++++++++++++++------------- lightning/src/ln/funding.rs | 42 +++++++++++++++++-------- lightning/src/ln/interactivetxs.rs | 49 +++++++++++++++++++----------- lightning/src/ln/splicing_tests.rs | 43 +++++++++++++++----------- 5 files changed, 117 insertions(+), 70 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 011b7f595bc..5cd7d6b9c8b 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -51,7 +51,7 @@ use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::Hash; use bitcoin::script::ScriptBuf; use bitcoin::secp256k1::PublicKey; -use bitcoin::{OutPoint, Transaction, TxOut}; +use bitcoin::{OutPoint, Transaction}; use core::ops::Deref; #[allow(unused_imports)] @@ -81,8 +81,8 @@ pub enum FundingInfo { Contribution { /// UTXOs spent as inputs contributed to the funding transaction. inputs: Vec, - /// Outputs contributed to the funding transaction. - outputs: Vec, + /// Output scripts contributed to the funding transaction. + outputs: Vec, }, } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4241decfba9..dc93538a909 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3083,7 +3083,7 @@ impl PendingFunding { self.contributions.iter().flat_map(|c| c.contributed_inputs()) } - fn contributed_outputs(&self) -> impl Iterator + '_ { + fn contributed_outputs(&self) -> impl Iterator + '_ { self.contributions.iter().flat_map(|c| c.contributed_outputs()) } @@ -3092,7 +3092,7 @@ impl PendingFunding { self.contributions[..len.saturating_sub(1)].iter().flat_map(|c| c.contributed_inputs()) } - fn prior_contributed_outputs(&self) -> impl Iterator + '_ { + fn prior_contributed_outputs(&self) -> impl Iterator + '_ { let len = self.contributions.len(); self.contributions[..len.saturating_sub(1)].iter().flat_map(|c| c.contributed_outputs()) } @@ -3141,7 +3141,7 @@ pub(crate) enum QuiescentAction { pub(super) enum QuiescentError { DoNothing, - DiscardFunding { inputs: Vec, outputs: Vec }, + DiscardFunding { inputs: Vec, outputs: Vec }, FailSplice(SpliceFundingFailed), } @@ -6516,10 +6516,11 @@ impl FundingNegotiationContext { } } - fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { + fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { let contributed_inputs = self.our_funding_inputs.into_iter().map(|input| input.utxo.outpoint).collect(); - let contributed_outputs = self.our_funding_outputs; + let contributed_outputs = + self.our_funding_outputs.into_iter().map(|output| output.script_pubkey).collect(); (contributed_inputs, contributed_outputs) } @@ -6527,12 +6528,15 @@ impl FundingNegotiationContext { self.our_funding_inputs.iter().map(|input| input.utxo.outpoint) } - fn contributed_outputs(&self) -> impl Iterator + '_ { - self.our_funding_outputs.iter() + fn contributed_outputs(&self) -> impl Iterator + '_ { + self.our_funding_outputs.iter().map(|output| output.script_pubkey.as_script()) } - fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) + fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { + ( + self.contributed_inputs().collect(), + self.contributed_outputs().map(|script| script.into()).collect(), + ) } } @@ -6692,8 +6696,8 @@ pub struct SpliceFundingFailed { /// UTXOs spent as inputs contributed to the splice transaction. pub contributed_inputs: Vec, - /// Outputs contributed to the splice transaction. - pub contributed_outputs: Vec, + /// Output scripts contributed to the splice transaction. + pub contributed_outputs: Vec, } macro_rules! maybe_create_splice_funding_failed { @@ -6733,7 +6737,7 @@ macro_rules! maybe_create_splice_funding_failed { contributed_inputs.retain(|i| *i != input); } for output in pending_splice.prior_contributed_outputs() { - contributed_outputs.retain(|o| o.script_pubkey != output.script_pubkey); + contributed_outputs.retain(|i| i != output); } } @@ -6778,15 +6782,16 @@ where fn quiescent_action_into_error(&self, action: QuiescentAction) -> QuiescentError { match action { QuiescentAction::Splice { contribution, .. } => { - let (mut inputs, mut outputs) = contribution.into_contributed_inputs_and_outputs(); - if let Some(ref pending_splice) = self.pending_splice { - for input in pending_splice.contributed_inputs() { - inputs.retain(|i| *i != input); - } - for output in pending_splice.contributed_outputs() { - outputs.retain(|o| o.script_pubkey != output.script_pubkey); - } - } + let (inputs, outputs) = if let Some(ref pending_splice) = self.pending_splice { + contribution + .into_unique_contributions( + pending_splice.contributed_inputs(), + pending_splice.contributed_outputs(), + ) + .unwrap_or((Vec::new(), Vec::new())) + } else { + contribution.into_contributed_inputs_and_outputs() + }; QuiescentError::FailSplice(SpliceFundingFailed { funding_txo: None, channel_type: None, diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index c81024ca080..35fac58c16b 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -422,8 +422,11 @@ impl FundingContribution { self.inputs.iter().map(|input| input.utxo.outpoint) } - pub(super) fn contributed_outputs(&self) -> impl Iterator + '_ { - self.outputs.iter().chain(self.change_output.iter()) + pub(super) fn contributed_outputs(&self) -> impl Iterator + '_ { + self.outputs + .iter() + .chain(self.change_output.iter()) + .map(|output| output.script_pubkey.as_script()) } /// Returns the change output included in this contribution, if any. @@ -444,26 +447,41 @@ impl FundingContribution { (inputs, outputs) } - pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - let (inputs, outputs) = self.into_tx_parts(); - - (inputs.into_iter().map(|input| input.utxo.outpoint).collect(), outputs) + pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { + let FundingContribution { inputs, outputs, change_output, .. } = self; + let contributed_inputs = inputs.into_iter().map(|input| input.utxo.outpoint).collect(); + let contributed_outputs = outputs.into_iter().chain(change_output.into_iter()); + (contributed_inputs, contributed_outputs.map(|output| output.script_pubkey).collect()) } pub(super) fn into_unique_contributions<'a>( self, existing_inputs: impl Iterator, - existing_outputs: impl Iterator, - ) -> Option<(Vec, Vec)> { - let (mut inputs, mut outputs) = self.into_contributed_inputs_and_outputs(); + existing_outputs: impl Iterator, + ) -> Option<(Vec, Vec)> { + let FundingContribution { mut inputs, mut outputs, mut change_output, .. } = self; for existing in existing_inputs { - inputs.retain(|input| *input != existing); + inputs.retain(|input| input.outpoint() != existing); } for existing in existing_outputs { - outputs.retain(|output| *output != *existing); + outputs.retain(|output| output.script_pubkey.as_script() != existing); + // TODO: Replace with `take_if` once our MSRV is >= 1.80. + if change_output + .as_ref() + .filter(|output| output.script_pubkey.as_script() == existing) + .is_some() + { + change_output.take(); + } } - if inputs.is_empty() && outputs.is_empty() { + if inputs.is_empty() && outputs.is_empty() && change_output.as_ref().is_none() { None } else { + let inputs = inputs.into_iter().map(|input| input.outpoint()).collect(); + let outputs = outputs + .into_iter() + .chain(change_output.into_iter()) + .map(|output| output.script_pubkey) + .collect(); Some((inputs, outputs)) } } diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 36367611abb..75863a3df1d 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -94,7 +94,7 @@ impl SerialIdExt for SerialId { pub(crate) struct NegotiationError { pub reason: AbortReason, pub contributed_inputs: Vec, - pub contributed_outputs: Vec, + pub contributed_outputs: Vec, } #[derive(Debug, Clone, Copy, PartialEq)] @@ -390,7 +390,7 @@ impl ConstructedTransaction { .map(|(_, (txin, _))| txin.previous_output) } - fn contributed_outputs(&self) -> impl Iterator + '_ { + fn contributed_outputs(&self) -> impl Iterator + '_ { self.tx .output .iter() @@ -398,14 +398,17 @@ impl ConstructedTransaction { .enumerate() .filter(|(_, (_, output))| output.is_local(self.holder_is_initiator)) .filter(|(index, _)| *index != self.shared_output_index as usize) - .map(|(_, (txout, _))| txout) + .map(|(_, (txout, _))| txout.script_pubkey.as_script()) } - fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) + fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { + ( + self.contributed_inputs().collect(), + self.contributed_outputs().map(|script| script.into()).collect(), + ) } - fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { + fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { let contributed_inputs = self .tx .input @@ -429,7 +432,7 @@ impl ConstructedTransaction { .enumerate() .filter(|(_, (_, output))| output.is_local(self.holder_is_initiator)) .filter(|(index, _)| *index != self.shared_output_index as usize) - .map(|(_, (txout, _))| txout) + .map(|(_, (txout, _))| txout.script_pubkey) .collect(); (contributed_inputs, contributed_outputs) @@ -917,15 +920,19 @@ impl InteractiveTxSigningSession { self.unsigned_tx.contributed_inputs() } - pub(super) fn contributed_outputs(&self) -> impl Iterator + '_ { + pub(super) fn contributed_outputs(&self) -> impl Iterator + '_ { self.unsigned_tx.contributed_outputs() } - pub(super) fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) + pub(super) fn to_contributed_inputs_and_outputs( + &self, + ) -> (Vec, Vec) { + self.unsigned_tx.to_contributed_inputs_and_outputs() } - pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { + pub(super) fn into_contributed_inputs_and_outputs( + self, + ) -> (Vec, Vec) { self.unsigned_tx.into_contributed_inputs_and_outputs() } } @@ -2165,7 +2172,9 @@ impl InteractiveTxConstructor { NegotiationError { reason, contributed_inputs, contributed_outputs } } - pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { + pub(super) fn into_contributed_inputs_and_outputs( + self, + ) -> (Vec, Vec) { let contributed_inputs = self .inputs_to_contribute .into_iter() @@ -2176,8 +2185,9 @@ impl InteractiveTxConstructor { .outputs_to_contribute .into_iter() .filter(|(_, output)| !output.is_shared()) - .map(|(_, output)| output.into_tx_out()) + .map(|(_, output)| output.into_tx_out().script_pubkey) .collect(); + (contributed_inputs, contributed_outputs) } @@ -2188,15 +2198,20 @@ impl InteractiveTxConstructor { .map(|(_, input)| input.tx_in().previous_output) } - pub(super) fn contributed_outputs(&self) -> impl Iterator + '_ { + pub(super) fn contributed_outputs(&self) -> impl Iterator + '_ { self.outputs_to_contribute .iter() .filter(|(_, output)| !output.is_shared()) - .map(|(_, output)| output.tx_out()) + .map(|(_, output)| output.tx_out().script_pubkey.as_script()) } - pub(super) fn to_contributed_inputs_and_outputs(&self) -> (Vec, Vec) { - (self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect()) + pub(super) fn to_contributed_inputs_and_outputs( + &self, + ) -> (Vec, Vec) { + ( + self.contributed_inputs().collect(), + self.contributed_outputs().map(|script| script.into()).collect(), + ) } pub fn is_initiator(&self) -> bool { diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index bdfe14635e0..e2b0fa6df5e 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -3063,7 +3063,8 @@ fn do_abandon_splice_quiescent_action_on_shutdown(local_shutdown: bool, pending_ assert!(inputs.is_empty(), "Expected empty inputs (filtered), got {:?}", inputs); // The change output was filtered (same script_pubkey as the prior splice's // change output), but the splice-out output survives (different script_pubkey). - let expected_outputs: Vec<_> = splice_out_output.into_iter().collect(); + let expected_outputs: Vec<_> = + splice_out_output.into_iter().map(|output| output.script_pubkey).collect(); assert_eq!(*outputs, expected_outputs); }, other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), @@ -3622,19 +3623,15 @@ fn test_funding_contributed_splice_already_pending() { .splice_in_and_out_sync(splice_in_amount, vec![first_splice_out.clone()], &wallet) .unwrap(); - // Initiate a second splice with a DIFFERENT output to test that different outputs + // Initiate a second splice with a DIFFERENT output script to test that different output scripts // are included in DiscardFunding (not filtered out) let second_splice_out = TxOut { - value: Amount::from_sat(6_000), // Different amount - script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())), + value: Amount::from_sat(5_000), + script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), }; // Clear UTXOs and add a LARGER one for the second contribution to ensure // the change output will be different from the first contribution's change - // - // FIXME: Should we actually not consider the change value given DiscardFunding is meant to - // reclaim the change script pubkey? But that means for other cases we'd need to track which - // output is for change later in the pipeline. nodes[0].wallet_source.clear_utxos(); provide_utxo_reserves(&nodes, 1, splice_in_amount * 3); @@ -3645,6 +3642,13 @@ fn test_funding_contributed_splice_already_pending() { .splice_in_and_out_sync(splice_in_amount, vec![second_splice_out.clone()], &wallet) .unwrap(); + // The change script should remain the same. + assert_eq!( + first_contribution.change_output().map(|output| &output.script_pubkey), + second_contribution.change_output().map(|output| &output.script_pubkey), + ); + let change_script = first_contribution.change_output().unwrap().script_pubkey.clone(); + // First funding_contributed - this sets up the quiescent action nodes[0].node.funding_contributed(&channel_id, &node_id_1, first_contribution, None).unwrap(); @@ -3654,8 +3658,9 @@ fn test_funding_contributed_splice_already_pending() { // Second funding_contributed with a different contribution - this should trigger // DiscardFunding because there's already a pending quiescent action (splice contribution). // Only inputs/outputs NOT in the existing contribution should be discarded. - let (expected_inputs, expected_outputs) = + let (expected_inputs, mut expected_outputs) = second_contribution.clone().into_contributed_inputs_and_outputs(); + expected_outputs.retain(|output| *output != change_script); // Returns Err(APIMisuseError) and emits DiscardFunding for the non-duplicate parts of the second contribution assert_eq!( @@ -3665,8 +3670,6 @@ fn test_funding_contributed_splice_already_pending() { }) ); - // The second contribution has different outputs (second_splice_out differs from first_splice_out), - // so those outputs should NOT be filtered out - they should appear in DiscardFunding. let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match &events[0] { @@ -3675,10 +3678,8 @@ fn test_funding_contributed_splice_already_pending() { if let FundingInfo::Contribution { inputs, outputs } = funding_info { // The input is different, so it should be in the discard event assert_eq!(*inputs, expected_inputs); - // The splice-out output is different (6000 vs 5000), so it should be in discard event - assert!(expected_outputs.contains(&second_splice_out)); - assert!(!expected_outputs.contains(&first_splice_out)); - // The different outputs should NOT be filtered out + // The different output should NOT be filtered out, but the change script should as + // it is the same in both contributions. assert_eq!(*outputs, expected_outputs); } else { panic!("Expected FundingInfo::Contribution"); @@ -3781,6 +3782,13 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let second_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); + // The change script should remain the same. + assert_eq!( + first_contribution.change_output().map(|output| &output.script_pubkey), + second_contribution.change_output().map(|output| &output.script_pubkey), + ); + let change_script = first_contribution.change_output().unwrap().script_pubkey.clone(); + // First funding_contributed - sets up the quiescent action and queues STFU nodes[0] .node @@ -3826,8 +3834,9 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { // Call funding_contributed with a different contribution (non-overlapping inputs/outputs). // This hits the funding_negotiation path and returns DiscardFunding. - let (expected_inputs, expected_outputs) = + let (expected_inputs, mut expected_outputs) = second_contribution.clone().into_contributed_inputs_and_outputs(); + expected_outputs.retain(|output| *output != change_script); assert_eq!( nodes[0].node.funding_contributed(&channel_id, &node_id_1, second_contribution, None), Err(APIError::APIMisuseError { @@ -5612,7 +5621,7 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { assert!(inputs.is_empty(), "Expected empty inputs (filtered), got {:?}", inputs); // The change output was filtered (same script_pubkey as round 0's change output), // but the splice-out output survives (different script_pubkey). - assert_eq!(*outputs, vec![splice_out_output.clone()]); + assert_eq!(*outputs, vec![splice_out_output.script_pubkey.clone()]); }, other => panic!("Expected DiscardFunding with Contribution, got {:?}", other), } From ca819d905fcb9234f6b315ad04c889705fbe07c2 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 3 Mar 2026 11:43:56 -0800 Subject: [PATCH 2/2] Produce FundingInfo::Contribution variants in ChannelMonitor Similar to the `ChannelManager`, we expose the contributed inputs and outputs of a splice via `FundingInfo::Contribution` at the `ChannelMonitor` level such that we don't lose the context when the channel closes while a splice is still pending. --- lightning/src/chain/channelmonitor.rs | 117 +++++++++++++++++++++----- lightning/src/ln/channel.rs | 11 ++- lightning/src/ln/splicing_tests.rs | 25 ++++-- lightning/src/util/ser.rs | 2 + 4 files changed, 126 insertions(+), 29 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 8e7b6035523..f2488475ce0 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -44,7 +44,7 @@ use crate::chain::package::{ use crate::chain::transaction::{OutPoint, TransactionData}; use crate::chain::{BestBlock, WatchedOutput}; use crate::events::bump_transaction::{AnchorDescriptor, BumpTransactionEvent}; -use crate::events::{ClosureReason, Event, EventHandler, ReplayEvent}; +use crate::events::{ClosureReason, Event, EventHandler, FundingInfo, ReplayEvent}; use crate::ln::chan_utils::{ self, ChannelTransactionParameters, CommitmentTransaction, CounterpartyCommitmentSecrets, HTLCClaim, HTLCOutputInCommitment, HolderCommitmentTransaction, @@ -688,6 +688,8 @@ pub(crate) enum ChannelMonitorUpdateStep { channel_parameters: ChannelTransactionParameters, holder_commitment_tx: HolderCommitmentTransaction, counterparty_commitment_tx: CommitmentTransaction, + contributed_inputs: Vec, + contributed_outputs: Vec, }, RenegotiatedFundingLocked { funding_txid: Txid, @@ -773,6 +775,8 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep, (1, channel_parameters, (required: ReadableArgs, None)), (3, holder_commitment_tx, required), (5, counterparty_commitment_tx, required), + (7, contributed_inputs, optional_vec), + (9, contributed_outputs, optional_vec), }, (12, RenegotiatedFundingLocked) => { (1, funding_txid, required), @@ -1166,6 +1170,10 @@ struct FundingScope { // transaction for which we have deleted claim information on some watchtowers. current_holder_commitment_tx: HolderCommitmentTransaction, prev_holder_commitment_tx: Option, + + /// Inputs and outputs we contributed when negotiating the corresponding funding transaction. + contributed_inputs: Option>, + contributed_outputs: Option>, } impl FundingScope { @@ -1194,6 +1202,8 @@ impl_writeable_tlv_based!(FundingScope, { (7, current_holder_commitment_tx, required), (9, prev_holder_commitment_tx, option), (11, counterparty_claimable_outpoints, required), + (13, contributed_inputs, option), + (15, contributed_outputs, option), }); #[derive(Clone, PartialEq)] @@ -1755,6 +1765,8 @@ pub(crate) fn write_chanmon_internal( (34, channel_monitor.alternative_funding_confirmed, option), (35, channel_monitor.is_manual_broadcast, required), (37, channel_monitor.funding_seen_onchain, required), + (39, channel_monitor.funding.contributed_inputs, option), + (41, channel_monitor.funding.contributed_outputs, option), }); Ok(()) @@ -1904,6 +1916,9 @@ impl ChannelMonitor { current_holder_commitment_tx: initial_holder_commitment_tx, prev_holder_commitment_tx: None, + + contributed_inputs: None, + contributed_outputs: None, }, pending_funding: vec![], @@ -3958,6 +3973,7 @@ impl ChannelMonitorImpl { &mut self, logger: &WithContext, channel_parameters: &ChannelTransactionParameters, alternative_holder_commitment_tx: &HolderCommitmentTransaction, alternative_counterparty_commitment_tx: &CommitmentTransaction, + contributed_inputs: &[BitcoinOutPoint], contributed_outputs: &[ScriptBuf], ) -> Result<(), ()> { let alternative_counterparty_commitment_txid = alternative_counterparty_commitment_tx.trust().txid(); @@ -4024,6 +4040,8 @@ impl ChannelMonitorImpl { counterparty_claimable_outpoints, current_holder_commitment_tx: alternative_holder_commitment_tx.clone(), prev_holder_commitment_tx: None, + contributed_inputs: Some(contributed_inputs.to_vec()), + contributed_outputs: Some(contributed_outputs.to_vec()), }; let alternative_funding_outpoint = alternative_funding.funding_outpoint(); @@ -4080,6 +4098,58 @@ impl ChannelMonitorImpl { Ok(()) } + fn queue_discard_funding_event( + &mut self, discarded_funding: impl Iterator, + ) { + let (mut discarded_inputs, mut discarded_outputs) = (new_hash_set(), new_hash_set()); + for funding in discarded_funding { + if funding.contributed_inputs.is_none() && funding.contributed_outputs.is_none() { + self.pending_events.push(Event::DiscardFunding { + channel_id: self.channel_id, + funding_info: FundingInfo::OutPoint { outpoint: funding.funding_outpoint() }, + }); + } else { + // Filter out any inputs/outputs that were already included in the locked funding + // transaction. + if let Some(mut maybe_discarded_inputs) = funding.contributed_inputs { + maybe_discarded_inputs.retain(|input| { + let is_input_in_locked_funding = self + .funding + .contributed_inputs + .as_ref() + .map(|inputs| inputs.contains(input)) + // The recently locked funding did not contain any contributed inputs. + .unwrap_or(false); + !is_input_in_locked_funding + }); + discarded_inputs.extend(maybe_discarded_inputs); + } + if let Some(mut maybe_discarded_outputs) = funding.contributed_outputs { + maybe_discarded_outputs.retain(|output| { + let is_output_in_locked_funding = self + .funding + .contributed_outputs + .as_ref() + .map(|outputs| outputs.contains(output)) + // The recently locked funding did not contain any contributed outputs. + .unwrap_or(false); + !is_output_in_locked_funding + }); + discarded_outputs.extend(maybe_discarded_outputs); + } + } + } + if !discarded_inputs.is_empty() || !discarded_outputs.is_empty() { + self.pending_events.push(Event::DiscardFunding { + channel_id: self.channel_id, + funding_info: FundingInfo::Contribution { + inputs: discarded_inputs.into_iter().collect(), + outputs: discarded_outputs.into_iter().collect(), + }, + }); + } + } + fn promote_funding(&mut self, new_funding_txid: Txid) -> Result<(), ()> { let prev_funding_txid = self.funding.funding_txid(); @@ -4110,18 +4180,20 @@ impl ChannelMonitorImpl { let no_further_updates_allowed = self.no_further_updates_allowed(); // The swap above places the previous `FundingScope` into `pending_funding`. - for funding in self.pending_funding.drain(..) { - let funding_txid = funding.funding_txid(); - self.outputs_to_watch.remove(&funding_txid); - if no_further_updates_allowed && funding_txid != prev_funding_txid { - self.pending_events.push(Event::DiscardFunding { - channel_id: self.channel_id, - funding_info: crate::events::FundingInfo::OutPoint { - outpoint: funding.funding_outpoint(), - }, - }); - } + for funding in &self.pending_funding { + self.outputs_to_watch.remove(&funding.funding_txid()); } + let mut discarded_funding = Vec::new(); + mem::swap(&mut self.pending_funding, &mut discarded_funding); + let discarded_funding = discarded_funding + .into_iter() + // The previous funding is filtered out since it was already locked, so nothing needs to + // be discarded. + .filter(|funding| { + no_further_updates_allowed && funding.funding_txid() != prev_funding_txid + }); + self.queue_discard_funding_event(discarded_funding); + if let Some((alternative_funding_txid, _)) = self.alternative_funding_confirmed.take() { // In exceedingly rare cases, it's possible there was a reorg that caused a potential funding to // be locked in that this `ChannelMonitor` has not yet seen. Thus, we avoid a runtime assertion @@ -4238,11 +4310,13 @@ impl ChannelMonitorImpl { }, ChannelMonitorUpdateStep::RenegotiatedFunding { channel_parameters, holder_commitment_tx, counterparty_commitment_tx, + contributed_inputs, contributed_outputs, } => { log_trace!(logger, "Updating ChannelMonitor with alternative holder and counterparty commitment transactions for funding txid {}", channel_parameters.funding_outpoint.unwrap().txid); if let Err(_) = self.renegotiated_funding( logger, channel_parameters, holder_commitment_tx, counterparty_commitment_tx, + contributed_inputs, contributed_outputs, ) { ret = Err(()); } @@ -5809,15 +5883,14 @@ impl ChannelMonitorImpl { self.funding_spend_confirmed = Some(entry.txid); self.confirmed_commitment_tx_counterparty_output = commitment_tx_to_counterparty_output; if self.alternative_funding_confirmed.is_none() { - for funding in self.pending_funding.drain(..) { + // We saw a confirmed commitment for our currently locked funding, so + // discard all pending ones. + for funding in &self.pending_funding { self.outputs_to_watch.remove(&funding.funding_txid()); - self.pending_events.push(Event::DiscardFunding { - channel_id: self.channel_id, - funding_info: crate::events::FundingInfo::OutPoint { - outpoint: funding.funding_outpoint(), - }, - }); } + let mut discarded_funding = Vec::new(); + mem::swap(&mut self.pending_funding, &mut discarded_funding); + self.queue_discard_funding_event(discarded_funding.into_iter()); } }, OnchainEvent::AlternativeFundingConfirmation {} => { @@ -6694,6 +6767,8 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP let mut alternative_funding_confirmed = None; let mut is_manual_broadcast = RequiredWrapper(None); let mut funding_seen_onchain = RequiredWrapper(None); + let mut current_funding_contributed_inputs = None; + let mut current_funding_contributed_outputs = None; read_tlv_fields!(reader, { (1, funding_spend_confirmed, option), (3, htlcs_resolved_on_chain, optional_vec), @@ -6716,6 +6791,8 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP (34, alternative_funding_confirmed, option), (35, is_manual_broadcast, (default_value, false)), (37, funding_seen_onchain, (default_value, true)), + (39, current_funding_contributed_inputs, option), + (41, current_funding_contributed_outputs, option), }); // Note that `payment_preimages_with_info` was added (and is always written) in LDK 0.1, so // we can use it to determine if this monitor was last written by LDK 0.1 or later. @@ -6830,6 +6907,8 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP current_holder_commitment_tx, prev_holder_commitment_tx, + contributed_inputs: current_funding_contributed_inputs, + contributed_outputs: current_funding_contributed_outputs, }, pending_funding: pending_funding.unwrap_or(vec![]), is_manual_broadcast: is_manual_broadcast.0.unwrap(), diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index dc93538a909..750e4354d30 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7925,12 +7925,12 @@ where &mut self, msg: &msgs::CommitmentSigned, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, ) -> Result, ChannelError> { - debug_assert!(self + let signing_session = self .context .interactive_tx_signing_session .as_ref() - .map(|signing_session| !signing_session.has_received_tx_signatures()) - .unwrap_or(false)); + .expect("Signing session must exist for negotiated pending splice"); + debug_assert!(!signing_session.has_received_tx_signatures()); let pending_splice_funding = self .pending_splice @@ -7982,6 +7982,9 @@ where ); } + let (contributed_inputs, contributed_outputs) = + signing_session.to_contributed_inputs_and_outputs(); + log_info!( logger, "Received splice initial commitment_signed from peer with funding txid {}", @@ -7995,6 +7998,8 @@ where channel_parameters: pending_splice_funding.channel_transaction_parameters.clone(), holder_commitment_tx, counterparty_commitment_tx, + contributed_inputs, + contributed_outputs, }], channel_id: Some(self.context.channel_id()), }; diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index e2b0fa6df5e..6fd61a80da7 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -1897,6 +1897,8 @@ fn do_test_splice_commitment_broadcast(splice_status: SpliceStatus, claim_htlcs: let splice_in_amount = initial_channel_capacity / 2; let initiator_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, Amount::from_sat(splice_in_amount)); + let (expected_discarded_inputs, expected_discarded_outputs) = + initiator_contribution.clone().into_contributed_inputs_and_outputs(); let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution); let (preimage2, payment_hash2, ..) = route_payment(&nodes[0], &[&nodes[1]], payment_amount); let htlc_expiry = nodes[0].best_block_info().1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS; @@ -2039,20 +2041,29 @@ fn do_test_splice_commitment_broadcast(splice_status: SpliceStatus, claim_htlcs: .chain_source .remove_watched_txn_and_outputs(funding_outpoint, txout.script_pubkey.clone()); - // `SpendableOutputs` events are also included here, but we don't care for them. let events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events(); assert_eq!(events.len(), if claim_htlcs { 2 } else { 4 }, "{events:?}"); if let Event::DiscardFunding { funding_info, .. } = &events[0] { - assert_eq!(*funding_info, FundingInfo::OutPoint { outpoint: funding_outpoint }); + assert_eq!( + *funding_info, + FundingInfo::Contribution { + inputs: expected_discarded_inputs, + outputs: expected_discarded_outputs, + } + ); } else { panic!(); } + assert!(matches!(&events[1], Event::SpendableOutputs { .. })); + if !claim_htlcs { + assert!(matches!(&events[2], Event::SpendableOutputs { .. })); + assert!(matches!(&events[3], Event::SpendableOutputs { .. })); + } + let events = nodes[1].chain_monitor.chain_monitor.get_and_clear_pending_events(); - assert_eq!(events.len(), if claim_htlcs { 2 } else { 1 }, "{events:?}"); - if let Event::DiscardFunding { funding_info, .. } = &events[0] { - assert_eq!(*funding_info, FundingInfo::OutPoint { outpoint: funding_outpoint }); - } else { - panic!(); + assert_eq!(events.len(), if claim_htlcs { 1 } else { 0 }, "{events:?}"); + if claim_htlcs { + assert!(matches!(&events[0], Event::SpendableOutputs { .. })); } } } diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 5d4b6b2df23..6e4e3fb723a 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -1102,6 +1102,8 @@ impl_for_vec!(crate::ln::channelmanager::MonitorUpdateCompletionAction); impl_for_vec!(crate::ln::channelmanager::PaymentClaimDetails); impl_for_vec!(crate::ln::msgs::SocketAddress); impl_for_vec!((A, B), A, B); +impl_for_vec!(OutPoint); +impl_for_vec!(ScriptBuf); impl_for_vec!(SerialId); impl_for_vec!(TxInMetadata); impl_for_vec!(TxOutMetadata);