Split method to reconstruct pending HTLCs into two

In the next commit, we want to dedup fields between the
InboundUpdateAdd::Forwarded's HTLCPreviousHopData and the outer
InboundHTLCOutput/Channel structs, since many fields are duplicated in both
places at the moment. As part of doing this cleanly, we first refactor the
method that retrieves these InboundUpdateAdds for reconstructing the set of
pending HTLCs during ChannelManager deconstruction.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: valentinewallace

lightning/src/ln/channel.rs

@@ -315,7 +315,7 @@ impl InboundHTLCState {
 ///
 /// Useful for reconstructing the pending HTLC set on startup.
 #[derive(Debug, Clone)]
-pub(super) enum InboundUpdateAdd {
+enum InboundUpdateAdd {
 	/// The inbound committed HTLC's update_add_htlc message.
 	WithOnion { update_add_htlc: msgs::UpdateAddHTLC },
 	/// This inbound HTLC is a forward that was irrevocably committed to the outbound edge, allowing
@@ -7885,10 +7885,35 @@ where
 		Ok(())
 	}
 
-	/// Useful for reconstructing the set of pending HTLCs when deserializing the `ChannelManager`.
-	pub(super) fn inbound_committed_unresolved_htlcs(
+	/// Returns true if any committed inbound HTLCs were received pre-LDK 0.3 and cannot be used
+	/// during `ChannelManager` deserialization to reconstruct the set of pending HTLCs.
+	pub(super) fn has_legacy_inbound_htlcs(&self) -> bool {
+		self.context.pending_inbound_htlcs.iter().any(|htlc| {
+			matches!(
+				&htlc.state,
+				InboundHTLCState::Committed { update_add_htlc: InboundUpdateAdd::Legacy }
+			)
+		})
+	}
+
+	/// Returns committed inbound HTLCs whose onion has not yet been decoded and processed. Useful
+	/// for reconstructing the set of pending HTLCs when deserializing the `ChannelManager`.
+	pub(super) fn inbound_htlcs_pending_decode(
+		&self,
+	) -> impl Iterator<Item = msgs::UpdateAddHTLC> + '_ {
+		self.context.pending_inbound_htlcs.iter().filter_map(|htlc| match &htlc.state {
+			InboundHTLCState::Committed {
+				update_add_htlc: InboundUpdateAdd::WithOnion { update_add_htlc },
+			} => Some(update_add_htlc.clone()),
+			_ => None,
+		})
+	}
+
+	/// Returns committed inbound HTLCs that have been forwarded but not yet fully resolved. Useful
+	/// when reconstructing the set of pending HTLCs when deserializing the `ChannelManager`.
+	pub(super) fn inbound_forwarded_htlcs(
 		&self,
-	) -> Vec<(PaymentHash, InboundUpdateAdd)> {
+	) -> impl Iterator<Item = (PaymentHash, HTLCPreviousHopData, u64)> + '_ {
 		// We don't want to return an HTLC as needing processing if it already has a resolution that's
 		// pending in the holding cell.
 		let htlc_resolution_in_holding_cell = |id: u64| -> bool {
@@ -7902,19 +7927,17 @@ where
 			})
 		};
 
-		self.context
-			.pending_inbound_htlcs
-			.iter()
-			.filter_map(|htlc| match &htlc.state {
-				InboundHTLCState::Committed { update_add_htlc } => {
-					if htlc_resolution_in_holding_cell(htlc.htlc_id) {
-						return None;
-					}
-					Some((htlc.payment_hash, update_add_htlc.clone()))
-				},
-				_ => None,
-			})
-			.collect()
+		self.context.pending_inbound_htlcs.iter().filter_map(move |htlc| match &htlc.state {
+			InboundHTLCState::Committed {
+				update_add_htlc: InboundUpdateAdd::Forwarded { hop_data, outbound_amt_msat },
+			} => {
+				if htlc_resolution_in_holding_cell(htlc.htlc_id) {
+					return None;
+				}
+				Some((htlc.payment_hash, hop_data.clone(), *outbound_amt_msat))
+			},
+			_ => None,
+		})
 	}
 
 	/// Useful when reconstructing the set of pending HTLC forwards when deserializing the

lightning/src/ln/channelmanager.rs

@@ -59,9 +59,9 @@ use crate::ln::chan_utils::selected_commitment_sat_per_1000_weight;
 use crate::ln::channel::QuiescentAction;
 use crate::ln::channel::{
 	self, hold_time_since, Channel, ChannelError, ChannelUpdateStatus, DisconnectResult,
-	FundedChannel, FundingTxSigned, InboundUpdateAdd, InboundV1Channel, OutboundV1Channel,
-	PendingV2Channel, ReconnectionMsg, ShutdownResult, SpliceFundingFailed, StfuResponse,
-	UpdateFulfillCommitFetch, WithChannelContext,
+	FundedChannel, FundingTxSigned, InboundV1Channel, OutboundV1Channel, PendingV2Channel,
+	ReconnectionMsg, ShutdownResult, SpliceFundingFailed, StfuResponse, UpdateFulfillCommitFetch,
+	WithChannelContext,
 };
 use crate::ln::channel_state::ChannelDetails;
 use crate::ln::funding::SpliceContribution;
@@ -10185,10 +10185,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 		let per_peer_state = self.per_peer_state.read().unwrap();
 		let peer_state = per_peer_state.get(&cp_id).map(|state| state.lock().unwrap()).unwrap();
 		let chan = peer_state.channel_by_id.get(&chan_id).and_then(|c| c.as_funded()).unwrap();
-		chan.inbound_committed_unresolved_htlcs()
-			.iter()
-			.filter(|(_, htlc)| matches!(htlc, InboundUpdateAdd::WithOnion { .. }))
-			.count()
+		chan.inbound_htlcs_pending_decode().count()
 	}
 
 	#[cfg(test)]
@@ -18626,33 +18623,27 @@ impl<
 					if reconstruct_manager_from_monitors {
 						if let Some(chan) = peer_state.channel_by_id.get(channel_id) {
 							if let Some(funded_chan) = chan.as_funded() {
+								// Legacy HTLCs are from pre-LDK 0.3 and cannot be reconstructed.
+								if funded_chan.has_legacy_inbound_htlcs() {
+									return Err(DecodeError::InvalidValue);
+								}
+								// Reconstruct `ChannelManager::decode_update_add_htlcs` from the serialized
+								// `Channel` as part of removing the requirement to regularly persist the
+								// `ChannelManager`.
 								let scid_alias = funded_chan.context.outbound_scid_alias();
-								let inbound_committed_update_adds =
-									funded_chan.inbound_committed_unresolved_htlcs();
-								for (payment_hash, htlc) in inbound_committed_update_adds {
-									match htlc {
-										InboundUpdateAdd::WithOnion { update_add_htlc } => {
-											// Reconstruct `ChannelManager::decode_update_add_htlcs` from the serialized
-											// `Channel` as part of removing the requirement to regularly persist the
-											// `ChannelManager`.
-											decode_update_add_htlcs
-												.entry(scid_alias)
-												.or_insert_with(Vec::new)
-												.push(update_add_htlc);
-										},
-										InboundUpdateAdd::Forwarded {
-											hop_data,
-											outbound_amt_msat,
-										} => {
-											already_forwarded_htlcs
-												.entry((hop_data.channel_id, payment_hash))
-												.or_insert_with(Vec::new)
-												.push((hop_data, outbound_amt_msat));
-										},
-										InboundUpdateAdd::Legacy => {
-											return Err(DecodeError::InvalidValue)
-										},
-									}
+								for update_add_htlc in funded_chan.inbound_htlcs_pending_decode() {
+									decode_update_add_htlcs
+										.entry(scid_alias)
+										.or_insert_with(Vec::new)
+										.push(update_add_htlc);
+								}
+								for (payment_hash, hop_data, outbound_amt_msat) in
+									funded_chan.inbound_forwarded_htlcs()
+								{
+									already_forwarded_htlcs
+										.entry((hop_data.channel_id, payment_hash))
+										.or_insert_with(Vec::new)
+										.push((hop_data, outbound_amt_msat));
 								}
 							}
 						}