Move FundedChannel::pending_funding into PendingSplice

FundedChannel::pending_funding and FundedChannel::pending_splice were
developed independently, but the former will only contain values when
the latter is set.

Author: jkczyz

lightning/src/ln/channel.rs

@@ -1804,7 +1804,6 @@ where
 				};
 				let mut funded_channel = FundedChannel {
 					funding: chan.funding,
-					pending_funding: vec![],
 					context: chan.context,
 					interactive_tx_signing_session: chan.interactive_tx_signing_session,
 					holder_commitment_point,
@@ -2176,6 +2175,7 @@ impl FundingScope {
 struct PendingSplice {
 	pub our_funding_contribution: i64,
 	funding: Option<FundingScope>,
+	pending_funding: Vec<FundingScope>,
 
 	/// The funding txid used in the `splice_locked` sent to the counterparty.
 	sent_funding_txid: Option<Txid>,
@@ -2187,11 +2187,14 @@ struct PendingSplice {
 #[cfg(splicing)]
 impl PendingSplice {
 	fn check_get_splice_locked<SP: Deref>(
-		&mut self, context: &ChannelContext<SP>, funding: &FundingScope, height: u32,
+		&mut self, context: &ChannelContext<SP>, confirmed_funding_index: usize, height: u32,
 	) -> Option<msgs::SpliceLocked>
 	where
 		SP::Target: SignerProvider,
 	{
+		debug_assert!(confirmed_funding_index < self.pending_funding.len());
+
+		let funding = self.pending_funding.get(confirmed_funding_index).unwrap();
 		if !context.check_funding_meets_minimum_depth(funding, height) {
 			return None;
 		}
@@ -5931,7 +5934,6 @@ where
 	SP::Target: SignerProvider,
 {
 	pub funding: FundingScope,
-	pending_funding: Vec<FundingScope>,
 	pub context: ChannelContext<SP>,
 	/// The signing session for the current interactive tx construction, if any.
 	///
@@ -5955,7 +5957,6 @@ macro_rules! promote_splice_funding {
 		}
 		core::mem::swap(&mut $self.funding, $funding);
 		$self.pending_splice = None;
-		$self.pending_funding.clear();
 		$self.context.announcement_sigs_state = AnnouncementSigsState::NotSent;
 	};
 }
@@ -6049,6 +6050,14 @@ where
 	SP::Target: SignerProvider,
 	<SP::Target as SignerProvider>::EcdsaSigner: EcdsaChannelSigner,
 {
+	fn pending_funding(&self) -> &[FundingScope] {
+		if let Some(pending_splice) = &self.pending_splice {
+			pending_splice.pending_funding.as_slice()
+		} else {
+			&[]
+		}
+	}
+
 	#[rustfmt::skip]
 	fn check_remote_fee<F: Deref, L: Deref>(
 		channel_type: &ChannelTypeFeatures, fee_estimator: &LowerBoundedFeeEstimator<F>,
@@ -6613,7 +6622,7 @@ where
 		}
 
 		core::iter::once(&self.funding)
-			.chain(self.pending_funding.iter())
+			.chain(self.pending_funding().iter())
 			.try_for_each(|funding| self.context.validate_update_add_htlc(funding, msg, fee_estimator))?;
 
 		// Now update local state:
@@ -6839,7 +6848,7 @@ where
 	{
 		self.commitment_signed_check_state()?;
 
-		if !self.pending_funding.is_empty() {
+		if !self.pending_funding().is_empty() {
 			return Err(ChannelError::close(
 				"Got a single commitment_signed message when expecting a batch".to_owned(),
 			));
@@ -6901,7 +6910,7 @@ where
 		// Any commitment_signed not associated with a FundingScope is ignored below if a
 		// pending splice transaction has confirmed since receiving the batch.
 		let updates = core::iter::once(&self.funding)
-			.chain(self.pending_funding.iter())
+			.chain(self.pending_funding().iter())
 			.map(|funding| {
 				let funding_txid = funding.get_funding_txo().unwrap().txid;
 				let msg = messages.get(&funding_txid).ok_or_else(|| {
@@ -7293,11 +7302,13 @@ where
 
 		#[cfg(any(test, fuzzing))]
 		{
-			for funding in
-				core::iter::once(&mut self.funding).chain(self.pending_funding.iter_mut())
-			{
-				*funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
-				*funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
+			*self.funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
+			*self.funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
+			if let Some(pending_splice) = self.pending_splice.as_mut() {
+				for funding in pending_splice.pending_funding.iter_mut() {
+					*funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
+					*funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
+				}
 			}
 		}
 
@@ -7498,9 +7509,13 @@ where
 			}
 		}
 
-		for funding in core::iter::once(&mut self.funding).chain(self.pending_funding.iter_mut()) {
-			funding.value_to_self_msat =
-				(funding.value_to_self_msat as i64 + value_to_self_msat_diff) as u64;
+		self.funding.value_to_self_msat =
+			(self.funding.value_to_self_msat as i64 + value_to_self_msat_diff) as u64;
+		if let Some(pending_splice) = self.pending_splice.as_mut() {
+			for funding in pending_splice.pending_funding.iter_mut() {
+				funding.value_to_self_msat =
+					(funding.value_to_self_msat as i64 + value_to_self_msat_diff) as u64;
+			}
 		}
 
 		if let Some((feerate, update_state)) = self.context.pending_update_fee {
@@ -7763,7 +7778,7 @@ where
 		}
 
 		let can_send_update_fee = core::iter::once(&self.funding)
-			.chain(self.pending_funding.iter())
+			.chain(self.pending_funding().iter())
 			.all(|funding| self.context.can_send_update_fee(funding, &self.holder_commitment_point, feerate_per_kw, fee_estimator, logger));
 		if !can_send_update_fee {
 			return None;
@@ -8052,14 +8067,14 @@ where
 		}
 
 		core::iter::once(&self.funding)
-			.chain(self.pending_funding.iter())
+			.chain(self.pending_funding().iter())
 			.try_for_each(|funding| FundedChannel::<SP>::check_remote_fee(funding.get_channel_type(), fee_estimator, msg.feerate_per_kw, Some(self.context.feerate_per_kw), logger))?;
 
 		self.context.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced));
 		self.context.update_time_counter += 1;
 
 		core::iter::once(&self.funding)
-			.chain(self.pending_funding.iter())
+			.chain(self.pending_funding().iter())
 			.try_for_each(|funding| self.context.validate_update_fee(funding, fee_estimator, msg))
 	}
 
@@ -9252,7 +9267,7 @@ where
 		let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator);
 
 		core::iter::once(&self.funding)
-			.chain(self.pending_funding.iter())
+			.chain(self.pending_funding().iter())
 			.try_for_each(|funding| self.context.can_accept_incoming_htlc(funding, msg, dust_exposure_limiting_feerate, &logger))
 	}
 
@@ -9535,9 +9550,11 @@ where
 		L::Target: Logger,
 	{
 		debug_assert!(self.pending_splice.is_some());
-		debug_assert!(confirmed_funding_index < self.pending_funding.len());
 
 		let pending_splice = self.pending_splice.as_mut().unwrap();
+
+		debug_assert!(confirmed_funding_index < pending_splice.pending_funding.len());
+
 		let splice_txid = match pending_splice.sent_funding_txid {
 			Some(sent_funding_txid) => sent_funding_txid,
 			None => {
@@ -9554,7 +9571,7 @@ where
 				&self.context.channel_id,
 			);
 
-			let funding = self.pending_funding.get_mut(confirmed_funding_index).unwrap();
+			let funding = pending_splice.pending_funding.get_mut(confirmed_funding_index).unwrap();
 			debug_assert_eq!(Some(splice_txid), funding.get_funding_txid());
 			promote_splice_funding!(self, funding);
 
@@ -9628,18 +9645,20 @@ where
 			#[cfg(splicing)]
 			let mut funding_already_confirmed = false;
 			#[cfg(splicing)]
-			for (index, funding) in self.pending_funding.iter_mut().enumerate() {
-				if self.context.check_for_funding_tx_confirmed(
-					funding, block_hash, height, index_in_block, &mut confirmed_tx, logger,
-				)? {
-					if funding_already_confirmed || confirmed_funding_index.is_some() {
-						let err_reason = "splice tx of another pending funding already confirmed";
-						return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() });
-					}
+			if let Some(pending_splice) = &mut self.pending_splice {
+				for (index, funding) in pending_splice.pending_funding.iter_mut().enumerate() {
+					if self.context.check_for_funding_tx_confirmed(
+						funding, block_hash, height, index_in_block, &mut confirmed_tx, logger,
+					)? {
+						if funding_already_confirmed || confirmed_funding_index.is_some() {
+							let err_reason = "splice tx of another pending funding already confirmed";
+							return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() });
+						}
 
-					confirmed_funding_index = Some(index);
-				} else if funding.funding_tx_confirmation_height != 0 {
-					funding_already_confirmed = true;
+						confirmed_funding_index = Some(index);
+					} else if funding.funding_tx_confirmation_height != 0 {
+						funding_already_confirmed = true;
+					}
 				}
 			}
 
@@ -9654,11 +9673,15 @@ where
 						return Err(ClosureReason::ProcessingError { err });
 					},
 				};
-				let funding = self.pending_funding.get(confirmed_funding_index).unwrap();
 
-				if let Some(splice_locked) = pending_splice.check_get_splice_locked(&self.context, funding, height) {
+				if let Some(splice_locked) = pending_splice.check_get_splice_locked(
+					&self.context,
+					confirmed_funding_index,
+					height,
+				) {
 					for &(idx, tx) in txdata.iter() {
 						if idx > index_in_block {
+							let funding = pending_splice.pending_funding.get(confirmed_funding_index).unwrap();
 							self.context.check_for_funding_tx_spent(funding, tx, logger)?;
 						}
 					}
@@ -9687,10 +9710,11 @@ where
 
 			self.context.check_for_funding_tx_spent(&self.funding, tx, logger)?;
 			#[cfg(splicing)]
-			for funding in self.pending_funding.iter() {
-				self.context.check_for_funding_tx_spent(funding, tx, logger)?;
+			if let Some(pending_splice) = self.pending_splice.as_ref() {
+				for funding in pending_splice.pending_funding.iter() {
+					self.context.check_for_funding_tx_spent(funding, tx, logger)?;
+				}
 			}
-
 		}
 
 		Ok((None, None))
@@ -9794,7 +9818,7 @@ where
 		#[cfg(splicing)]
 		let mut confirmed_funding_index = None;
 		#[cfg(splicing)]
-		for (index, funding) in self.pending_funding.iter().enumerate() {
+		for (index, funding) in self.pending_funding().iter().enumerate() {
 			if funding.funding_tx_confirmation_height != 0 {
 				if confirmed_funding_index.is_some() {
 					let err_reason = "splice tx of another pending funding already confirmed";
@@ -9816,7 +9840,7 @@ where
 					return Err(ClosureReason::ProcessingError { err });
 				},
 			};
-			let funding = self.pending_funding.get_mut(confirmed_funding_index).unwrap();
+			let funding = pending_splice.pending_funding.get_mut(confirmed_funding_index).unwrap();
 
 			// Check if the splice funding transaction was unconfirmed
 			if funding.get_funding_tx_confirmations(height) == 0 {
@@ -9835,8 +9859,11 @@ where
 			}
 
 			let pending_splice = self.pending_splice.as_mut().unwrap();
-			let funding = self.pending_funding.get(confirmed_funding_index).unwrap();
-			if let Some(splice_locked) = pending_splice.check_get_splice_locked(&self.context, funding, height) {
+			if let Some(splice_locked) = pending_splice.check_get_splice_locked(
+				&self.context,
+				confirmed_funding_index,
+				height,
+			) {
 				log_info!(logger, "Sending a splice_locked to our peer for channel {}", &self.context.channel_id);
 
 				let funding_promoted =
@@ -9866,7 +9893,7 @@ where
 
 	pub fn get_relevant_txids(&self) -> impl Iterator<Item = (Txid, u32, Option<BlockHash>)> + '_ {
 		core::iter::once(&self.funding)
-			.chain(self.pending_funding.iter())
+			.chain(self.pending_funding().iter())
 			.map(|funding| {
 				(
 					funding.get_funding_txid(),
@@ -9896,9 +9923,16 @@ where
 	where
 		L::Target: Logger,
 	{
-		let unconfirmed_funding = core::iter::once(&mut self.funding)
-			.chain(self.pending_funding.iter_mut())
-			.find(|funding| funding.get_funding_txid() == Some(*txid));
+		let unconfirmed_funding = (self.funding.get_funding_txid() == Some(*txid))
+			.then(|| &mut self.funding)
+			.or_else(|| {
+				self.pending_splice.as_mut().and_then(|pending_splice| {
+					pending_splice
+						.pending_funding
+						.iter_mut()
+						.find(|funding| funding.get_funding_txid() == Some(*txid))
+				})
+			});
 
 		if let Some(funding) = unconfirmed_funding {
 			if funding.funding_tx_confirmation_height != 0 {
@@ -10254,6 +10288,7 @@ where
 		self.pending_splice = Some(PendingSplice {
 			our_funding_contribution: our_funding_contribution_satoshis,
 			funding: None,
+			pending_funding: vec![],
 			sent_funding_txid: None,
 			received_funding_txid: None,
 		});
@@ -10370,7 +10405,7 @@ where
 
 		if let Some(sent_funding_txid) = pending_splice.sent_funding_txid {
 			if sent_funding_txid == msg.splice_txid {
-				if let Some(funding) = self
+				if let Some(funding) = pending_splice
 					.pending_funding
 					.iter_mut()
 					.find(|funding| funding.get_funding_txid() == Some(sent_funding_txid))
@@ -10569,7 +10604,7 @@ where
 		F::Target: FeeEstimator,
 	{
 		core::iter::once(&self.funding)
-			.chain(self.pending_funding.iter())
+			.chain(self.pending_funding().iter())
 			.map(|funding| self.context.get_available_balances_for_scope(funding, fee_estimator))
 			.reduce(|acc, e| {
 				AvailableBalances {
@@ -10616,14 +10651,14 @@ where
 		}
 		self.context.resend_order = RAACommitmentOrder::RevokeAndACKFirst;
 
-		let mut updates = Vec::with_capacity(self.pending_funding.len() + 1);
-		for funding in core::iter::once(&self.funding).chain(self.pending_funding.iter()) {
+		let mut updates = Vec::with_capacity(self.pending_funding().len() + 1);
+		for funding in core::iter::once(&self.funding).chain(self.pending_funding().iter()) {
 			let (htlcs_ref, counterparty_commitment_tx) =
 				self.build_commitment_no_state_update(funding, logger);
 			let htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)> =
 				htlcs_ref.into_iter().map(|(htlc, htlc_source)| (htlc, htlc_source.map(|source_ref| Box::new(source_ref.clone())))).collect();
 
-			if self.pending_funding.is_empty() {
+			if self.pending_funding().is_empty() {
 				// Soon, we will switch this to `LatestCounterpartyCommitmentTX`,
 				// and provide the full commit tx instead of the information needed to rebuild it.
 				updates.push(ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo {
@@ -10700,7 +10735,7 @@ where
 		L::Target: Logger,
 	{
 		core::iter::once(&self.funding)
-			.chain(self.pending_funding.iter())
+			.chain(self.pending_funding().iter())
 			.map(|funding| self.send_commitment_no_state_update_for_funding(funding, logger))
 			.collect::<Result<Vec<_>, ChannelError>>()
 	}
@@ -11477,7 +11512,6 @@ where
 
 		let mut channel = FundedChannel {
 			funding: self.funding,
-			pending_funding: vec![],
 			context: self.context,
 			interactive_tx_signing_session: None,
 			holder_commitment_point,
@@ -11769,7 +11803,6 @@ where
 		// `ChannelMonitor`.
 		let mut channel = FundedChannel {
 			funding: self.funding,
-			pending_funding: vec![],
 			context: self.context,
 			interactive_tx_signing_session: None,
 			holder_commitment_point,
@@ -12937,7 +12970,6 @@ where
 		let mut next_holder_commitment_point_opt: Option<PublicKey> = None;
 		let mut is_manual_broadcast = None;
 
-		let mut pending_funding = Some(Vec::new());
 		let mut historical_scids = Some(Vec::new());
 
 		let mut interactive_tx_signing_session: Option<InteractiveTxSigningSession> = None;
@@ -13161,7 +13193,6 @@ where
 				short_channel_id,
 				minimum_depth_override,
 			},
-			pending_funding: pending_funding.unwrap(),
 			context: ChannelContext {
 				user_id,