diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index c3e20ef5e6f..f7bbfc7c5b7 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::{BlockLocator, 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, @@ -55,6 +55,7 @@ use crate::ln::channel_keys::{ RevocationKey, }; use crate::ln::channelmanager::{HTLCSource, PaymentClaimDetails, SentHTLCId}; +use crate::ln::funding::FundingContribution; use crate::ln::msgs::DecodeError; use crate::ln::types::ChannelId; use crate::sign::{ @@ -688,6 +689,7 @@ pub(crate) enum ChannelMonitorUpdateStep { channel_parameters: ChannelTransactionParameters, holder_commitment_tx: HolderCommitmentTransaction, counterparty_commitment_tx: CommitmentTransaction, + funding_contribution: Option, }, RenegotiatedFundingLocked { funding_txid: Txid, @@ -773,6 +775,7 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep, (1, channel_parameters, (required: ReadableArgs, None)), (3, holder_commitment_tx, required), (5, counterparty_commitment_tx, required), + (7, funding_contribution, option), }, (12, RenegotiatedFundingLocked) => { (1, funding_txid, required), @@ -1166,6 +1169,9 @@ struct FundingScope { // transaction for which we have deleted claim information on some watchtowers. current_holder_commitment_tx: HolderCommitmentTransaction, prev_holder_commitment_tx: Option, + + /// Our funding contribution when negotiating the corresponding funding transaction. + contribution: Option, } impl FundingScope { @@ -1185,6 +1191,14 @@ impl FundingScope { fn channel_type_features(&self) -> &ChannelTypeFeatures { &self.channel_parameters.channel_type_features } + + fn contributed_inputs(&self) -> impl Iterator + '_ { + self.contribution.iter().flat_map(|contribution| contribution.contributed_inputs()) + } + + fn contributed_outputs(&self) -> impl Iterator + '_ { + self.contribution.iter().flat_map(|contribution| contribution.contributed_outputs()) + } } impl_writeable_tlv_based!(FundingScope, { @@ -1194,6 +1208,7 @@ impl_writeable_tlv_based!(FundingScope, { (7, current_holder_commitment_tx, required), (9, prev_holder_commitment_tx, option), (11, counterparty_claimable_outpoints, required), + (13, contribution, option), }); #[derive(Clone, PartialEq)] @@ -1756,6 +1771,7 @@ pub(crate) fn write_chanmon_internal( (35, channel_monitor.is_manual_broadcast, required), (37, channel_monitor.funding_seen_onchain, required), (39, channel_monitor.best_block.previous_blocks, required), + (41, channel_monitor.funding.contribution, option), }); Ok(()) @@ -1905,6 +1921,8 @@ impl ChannelMonitor { current_holder_commitment_tx: initial_holder_commitment_tx, prev_holder_commitment_tx: None, + + contribution: None, }, pending_funding: vec![], @@ -3959,6 +3977,7 @@ impl ChannelMonitorImpl { &mut self, logger: &WithContext, channel_parameters: &ChannelTransactionParameters, alternative_holder_commitment_tx: &HolderCommitmentTransaction, alternative_counterparty_commitment_tx: &CommitmentTransaction, + funding_contribution: &Option, ) -> Result<(), ()> { let alternative_counterparty_commitment_txid = alternative_counterparty_commitment_tx.trust().txid(); @@ -4025,6 +4044,7 @@ impl ChannelMonitorImpl { counterparty_claimable_outpoints, current_holder_commitment_tx: alternative_holder_commitment_tx.clone(), prev_holder_commitment_tx: None, + contribution: funding_contribution.clone(), }; let alternative_funding_outpoint = alternative_funding.funding_outpoint(); @@ -4081,6 +4101,37 @@ 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.contribution.is_none() { + self.pending_events.push(Event::DiscardFunding { + channel_id: self.channel_id, + funding_info: FundingInfo::OutPoint { outpoint: funding.funding_outpoint() }, + }); + } else if let Some(contribution) = funding.contribution { + if let Some((inputs, outputs)) = contribution.into_unique_contributions( + self.funding.contributed_inputs(), + self.funding.contributed_outputs(), + ) { + discarded_inputs.extend(inputs); + discarded_outputs.extend(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(); @@ -4111,18 +4162,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 @@ -4239,11 +4292,13 @@ impl ChannelMonitorImpl { }, ChannelMonitorUpdateStep::RenegotiatedFunding { channel_parameters, holder_commitment_tx, counterparty_commitment_tx, + funding_contribution, } => { 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, + funding_contribution, ) { ret = Err(()); } @@ -5810,15 +5865,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 {} => { @@ -6696,6 +6750,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP let mut is_manual_broadcast = RequiredWrapper(None); let mut funding_seen_onchain = RequiredWrapper(None); let mut best_block_previous_blocks = None; + let mut current_funding_contribution = None; read_tlv_fields!(reader, { (1, funding_spend_confirmed, option), (3, htlcs_resolved_on_chain, optional_vec), @@ -6719,6 +6774,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP (35, is_manual_broadcast, (default_value, false)), (37, funding_seen_onchain, (default_value, true)), (39, best_block_previous_blocks, option), // Added and always set in 0.3 + (41, current_funding_contribution, option), }); if let Some(previous_blocks) = best_block_previous_blocks { best_block.previous_blocks = previous_blocks; @@ -6837,6 +6893,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP current_holder_commitment_tx, prev_holder_commitment_tx, + contribution: current_funding_contribution, }, pending_funding: pending_funding.unwrap_or(vec![]), is_manual_broadcast: is_manual_broadcast.0.unwrap(), diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 73c4a39c76f..405a73901ae 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 fbf2a4caa9f..4a385ce5572 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3115,7 +3115,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()) } @@ -3124,7 +3124,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()) } @@ -3173,7 +3173,7 @@ pub(crate) enum QuiescentAction { pub(super) enum QuiescentError { DoNothing, - DiscardFunding { inputs: Vec, outputs: Vec }, + DiscardFunding { inputs: Vec, outputs: Vec }, FailSplice(SpliceFundingFailed), } @@ -6850,10 +6850,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) } @@ -6861,12 +6862,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(), + ) } } @@ -7027,8 +7031,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 { @@ -7068,7 +7072,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); } } @@ -7119,7 +7123,7 @@ where inputs.retain(|i| *i != input); } for output in pending_splice.contributed_outputs() { - outputs.retain(|o| o.script_pubkey != output.script_pubkey); + outputs.retain(|o| o.as_script() != output); } } SpliceFundingFailed { @@ -8314,12 +8318,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 @@ -8371,6 +8375,12 @@ where ); } + let funding_contribution = self + .pending_splice + .as_ref() + .and_then(|pending_splice| pending_splice.contributions.last()) + .cloned(); + log_info!( logger, "Received splice initial commitment_signed from peer with funding txid {}", @@ -8384,6 +8394,7 @@ where channel_parameters: pending_splice_funding.channel_transaction_parameters.clone(), holder_commitment_tx, counterparty_commitment_tx, + funding_contribution, }], channel_id: Some(self.context.channel_id()), }; diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 20366fe772a..29981bb138c 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -586,12 +586,15 @@ impl FundingContribution { self.is_splice } - pub(super) fn contributed_inputs(&self) -> impl Iterator + '_ { + pub(crate) fn contributed_inputs(&self) -> impl Iterator + '_ { 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(crate) fn contributed_outputs(&self) -> impl Iterator + '_ { + self.outputs + .iter() + .chain(self.change_output.iter()) + .map(|output| output.script_pubkey.as_script()) } /// The value that will be added to the channel after fees. See [`Self::net_value`] for the net @@ -734,26 +737,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>( + pub(crate) 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 ca8c4450012..5aee1d7c191 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) @@ -929,15 +932,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() } } @@ -2177,7 +2184,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() @@ -2188,8 +2197,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) } @@ -2200,15 +2210,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 fa22ccb61c7..3c44aec0809 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -1758,6 +1758,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.clone()); let (preimage2, payment_hash2, ..) = route_payment(&nodes[0], &[&nodes[1]], payment_amount); @@ -1901,14 +1903,25 @@ 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] { @@ -1916,6 +1929,9 @@ fn do_test_splice_commitment_broadcast(splice_status: SpliceStatus, claim_htlcs: } else { panic!(); } + if claim_htlcs { + assert!(matches!(&events[1], Event::SpendableOutputs { .. })); + } } } @@ -2925,7 +2941,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), @@ -3600,19 +3617,15 @@ fn test_funding_contributed_splice_already_pending() { .build() .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); @@ -3626,6 +3639,13 @@ fn test_funding_contributed_splice_already_pending() { .build() .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(); @@ -3635,8 +3655,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!( @@ -3646,8 +3667,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] { @@ -3656,10 +3675,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"); @@ -3762,6 +3779,13 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { let second_contribution = funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &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 @@ -3807,8 +3831,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 { @@ -6141,7 +6166,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), } diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index bd2488bd8d1..88c03638c82 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -1099,6 +1099,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);