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/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..750e4354d30 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, @@ -7920,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 @@ -7977,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 {}", @@ -7990,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/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..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 { .. })); } } } @@ -3063,7 +3074,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 +3634,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 +3653,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 +3669,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 +3681,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 +3689,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 +3793,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 +3845,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 +5632,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 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);