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

@@ -2037,7 +2037,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,
@@ -2550,6 +2549,7 @@ impl AddSigned for u64 {
 /// Info about a pending splice
 struct PendingSplice {
 	funding_negotiation: Option<FundingNegotiation>,
+	pending_funding: Vec<FundingScope>,
 
 	/// The funding txid used in the `splice_locked` sent to the counterparty.
 	sent_funding_txid: Option<Txid>,
@@ -2576,11 +2576,14 @@ impl FundingNegotiation {
 
 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[confirmed_funding_index];
 		if !context.check_funding_meets_minimum_depth(funding, height) {
 			return None;
 		}
@@ -6632,7 +6635,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.
 	///
@@ -6654,19 +6656,16 @@ where
 }
 
 macro_rules! promote_splice_funding {
-	($self: expr, $funding: expr) => {{
+	($self: expr, $pending_splice: expr, $funding: expr) => {{
 		let prev_funding_txid = $self.funding.get_funding_txid();
 		if let Some(scid) = $self.funding.short_channel_id {
 			$self.context.historical_scids.push(scid);
 		}
+
 		core::mem::swap(&mut $self.funding, $funding);
-		$self.interactive_tx_signing_session = None;
-		$self.pending_splice = None;
-		$self.context.announcement_sigs = None;
-		$self.context.announcement_sigs_state = AnnouncementSigsState::NotSent;
 
 		// The swap above places the previous `FundingScope` into `pending_funding`.
-		let discarded_funding = $self
+		let discarded_funding = $pending_splice
 			.pending_funding
 			.drain(..)
 			.filter(|funding| funding.get_funding_txid() != prev_funding_txid)
@@ -6682,6 +6681,12 @@ macro_rules! promote_splice_funding {
 					})
 			})
 			.collect::<Vec<_>>();
+
+		$self.interactive_tx_signing_session = None;
+		$self.pending_splice = None;
+		$self.context.announcement_sigs = None;
+		$self.context.announcement_sigs_state = AnnouncementSigsState::NotSent;
+
 		discarded_funding
 	}};
 }
@@ -6796,6 +6801,24 @@ where
 			})
 	}
 
+	fn pending_funding(&self) -> &[FundingScope] {
+		if let Some(pending_splice) = &self.pending_splice {
+			pending_splice.pending_funding.as_slice()
+		} else {
+			&[]
+		}
+	}
+
+	fn funding_and_pending_funding_iter_mut(&mut self) -> impl Iterator<Item = &mut FundingScope> {
+		core::iter::once(&mut self.funding).chain(
+			self.pending_splice
+				.as_mut()
+				.map(|pending_splice| pending_splice.pending_funding.as_mut_slice())
+				.unwrap_or(&mut [])
+				.iter_mut(),
+		)
+	}
+
 	#[rustfmt::skip]
 	fn check_remote_fee<F: Deref, L: Deref>(
 		channel_type: &ChannelTypeFeatures, fee_estimator: &LowerBoundedFeeEstimator<F>,
@@ -7425,7 +7448,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:
@@ -7680,7 +7703,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(),
 			));
@@ -7747,9 +7770,9 @@ where
 
 		// Any commitment_signed not associated with a FundingScope is ignored below if a
 		// pending splice transaction has confirmed since receiving the batch.
-		let mut commitment_txs = Vec::with_capacity(self.pending_funding.len() + 1);
+		let mut commitment_txs = Vec::with_capacity(self.pending_funding().len() + 1);
 		let mut htlc_data = None;
-		for funding in core::iter::once(&self.funding).chain(self.pending_funding.iter()) {
+		for funding in core::iter::once(&self.funding).chain(self.pending_funding().iter()) {
 			let funding_txid =
 				funding.get_funding_txid().expect("Funding txid must be known for pending scope");
 			let msg = messages.get(&funding_txid).ok_or_else(|| {
@@ -8419,7 +8442,7 @@ where
 			}
 		}
 
-		for funding in core::iter::once(&mut self.funding).chain(self.pending_funding.iter_mut()) {
+		for funding in self.funding_and_pending_funding_iter_mut() {
 			funding.value_to_self_msat =
 				(funding.value_to_self_msat as i64 + value_to_self_msat_diff) as u64;
 		}
@@ -8749,7 +8772,7 @@ where
 		debug_assert!(!self.funding.get_channel_type().supports_anchor_zero_fee_commitments());
 
 		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, feerate_per_kw, fee_estimator, logger));
 		if !can_send_update_fee {
 			return None;
@@ -9052,14 +9075,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))
 	}
 
@@ -9584,7 +9607,7 @@ where
 		//     - MUST process `my_current_funding_locked` as if it was receiving `splice_locked`
 		//       for this `txid`.
 		let inferred_splice_locked = msg.my_current_funding_locked.as_ref().and_then(|funding_locked| {
-			self.pending_funding
+			self.pending_funding()
 				.iter()
 				.find(|funding| funding.get_funding_txid() == Some(funding_locked.txid))
 				.and_then(|_| {
@@ -10358,7 +10381,7 @@ where
 		);
 
 		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, dust_exposure_limiting_feerate, &logger))
 	}
 
@@ -10692,12 +10715,12 @@ where
 		let discarded_funding = {
 			// Scope `funding` since it is swapped within `promote_splice_funding` and we don't want
 			// to unintentionally use it.
-			let funding = self
+			let funding = pending_splice
 				.pending_funding
 				.iter_mut()
 				.find(|funding| funding.get_funding_txid() == Some(splice_txid))
 				.unwrap();
-			promote_splice_funding!(self, funding)
+			promote_splice_funding!(self, pending_splice, funding)
 		};
 
 		let funding_txo = self
@@ -10767,18 +10790,20 @@ where
 
 			let mut confirmed_funding_index = None;
 			let mut funding_already_confirmed = false;
-			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;
+					}
 				}
 			}
 
@@ -10792,9 +10817,8 @@ 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) {
 					log_info!(
 						logger,
 						"Sending splice_locked txid {} to our peer for channel {}",
@@ -10918,7 +10942,7 @@ where
 		}
 
 		let mut confirmed_funding_index = None;
-		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";
@@ -10939,7 +10963,7 @@ where
 					return Err(ClosureReason::ProcessingError { err });
 				},
 			};
-			let funding = self.pending_funding.get_mut(confirmed_funding_index).unwrap();
+			let funding = &mut pending_splice.pending_funding[confirmed_funding_index];
 
 			// Check if the splice funding transaction was unconfirmed
 			if funding.get_funding_tx_confirmations(height) == 0 {
@@ -10958,8 +10982,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);
 				debug_assert!(chain_node_signer.is_some());
 
@@ -10989,7 +11016,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(),
@@ -11019,8 +11046,8 @@ where
 	where
 		L::Target: Logger,
 	{
-		let unconfirmed_funding = core::iter::once(&mut self.funding)
-			.chain(self.pending_funding.iter_mut())
+		let unconfirmed_funding = self
+			.funding_and_pending_funding_iter_mut()
 			.find(|funding| funding.get_funding_txid() == Some(*txid));
 
 		if let Some(funding) = unconfirmed_funding {
@@ -11515,6 +11542,7 @@ where
 
 		self.pending_splice = Some(PendingSplice {
 			funding_negotiation: Some(FundingNegotiation::AwaitingAck(funding_negotiation_context)),
+			pending_funding: vec![],
 			sent_funding_txid: None,
 			received_funding_txid: None,
 		});
@@ -11737,6 +11765,7 @@ where
 				splice_funding,
 				interactive_tx_constructor,
 			)),
+			pending_funding: Vec::new(),
 			received_funding_txid: None,
 			sent_funding_txid: None,
 		});
@@ -11924,7 +11953,7 @@ where
 			},
 		};
 
-		if !self
+		if !pending_splice
 			.pending_funding
 			.iter()
 			.any(|funding| funding.get_funding_txid() == Some(msg.splice_txid))
@@ -12121,7 +12150,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 {
@@ -12168,7 +12197,7 @@ where
 		}
 		self.context.resend_order = RAACommitmentOrder::RevokeAndACKFirst;
 
-		let update = if self.pending_funding.is_empty() {
+		let update = if self.pending_funding().is_empty() {
 			let (htlcs_ref, counterparty_commitment_tx) =
 				self.build_commitment_no_state_update(&self.funding, logger);
 			let htlc_outputs = htlcs_ref.into_iter()
@@ -12191,7 +12220,7 @@ where
 		} else {
 			let mut htlc_data = None;
 			let commitment_txs = core::iter::once(&self.funding)
-				.chain(self.pending_funding.iter())
+				.chain(self.pending_funding().iter())
 				.map(|funding| {
 					let (htlcs_ref, counterparty_commitment_tx) =
 						self.build_commitment_no_state_update(funding, logger);
@@ -12264,7 +12293,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>>()
 	}
@@ -13064,7 +13093,6 @@ where
 
 		let mut channel = FundedChannel {
 			funding: self.funding,
-			pending_funding: vec![],
 			context: self.context,
 			interactive_tx_signing_session: None,
 			holder_commitment_point,
@@ -13350,7 +13378,6 @@ where
 		// `ChannelMonitor`.
 		let mut channel = FundedChannel {
 			funding: self.funding,
-			pending_funding: vec![],
 			context: self.context,
 			interactive_tx_signing_session: None,
 			holder_commitment_point,
@@ -14926,7 +14953,6 @@ where
 				short_channel_id,
 				minimum_depth_override,
 			},
-			pending_funding: Vec::new(),
 			context: ChannelContext {
 				user_id,