Resolve legacy TLV fields during ChannelManagerData deserialization

Move the resolution of legacy/compatibility TLV fields from
from_channel_manager_data (stage 2) into ChannelManagerData::read
(stage 1). This keeps ChannelManagerData minimal by consolidating
mutually exclusive fields into their final form during deserialization:

- pending_outbound_payments: Merge TLV 3, TLV 1 (no_retry), and
  non-TLV compat fields into a single HashMap
- in_flight_monitor_updates: Convert legacy TLV 10 (keyed by OutPoint)
  to TLV 17 format (keyed by ChannelId)
- pending_events: Apply events_override (TLV 8) if present

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

Author: joostjager

lightning/src/ln/channelmanager.rs

@@ -17330,19 +17330,14 @@ where
 	peer_init_features: Vec<(PublicKey, InitFeatures)>,
 	pending_events: VecDeque<(events::Event, Option<EventCompletionAction>)>,
 	highest_seen_timestamp: u32,
-	pending_outbound_payments_compat: HashMap<PaymentId, PendingOutboundPayment>,
-	pending_outbound_payments_no_retry: Option<HashMap<PaymentId, HashSet<[u8; 32]>>>,
 	pending_intercepted_htlcs: Option<HashMap<InterceptId, PendingAddHTLCInfo>>,
-	pending_outbound_payments: Option<HashMap<PaymentId, PendingOutboundPayment>>,
+	pending_outbound_payments: HashMap<PaymentId, PendingOutboundPayment>,
 	pending_claiming_payments: HashMap<PaymentHash, ClaimingPayment>,
 	received_network_pubkey: Option<PublicKey>,
 	monitor_update_blocked_actions_per_peer:
 		Vec<(PublicKey, BTreeMap<ChannelId, Vec<MonitorUpdateCompletionAction>>)>,
 	fake_scid_rand_bytes: Option<[u8; 32]>,
-	events_override: Option<VecDeque<(events::Event, Option<EventCompletionAction>)>>,
 	claimable_htlc_purposes: Option<Vec<events::PaymentPurpose>>,
-	legacy_in_flight_monitor_updates:
-		Option<HashMap<(PublicKey, OutPoint), Vec<ChannelMonitorUpdate>>>,
 	probing_cookie_secret: Option<[u8; 32]>,
 	claimable_htlc_onion_fields: Option<Vec<Option<RecipientOnionFields>>>,
 	decode_update_add_htlcs: Option<HashMap<u64, Vec<msgs::UpdateAddHTLC>>>,
@@ -17539,6 +17534,48 @@ where
 			(21, async_receive_offer_cache, (default_value, async_receive_offer_cache)),
 		});
 
+		// Merge legacy pending_outbound_payments fields into a single HashMap.
+		// Priority: pending_outbound_payments (TLV 3) > pending_outbound_payments_no_retry (TLV 1)
+		//           > pending_outbound_payments_compat (non-TLV legacy)
+		let pending_outbound_payments = if let Some(payments) = pending_outbound_payments {
+			payments
+		} else if let Some(mut pending_outbound_payments_no_retry) = pending_outbound_payments_no_retry
+		{
+			let mut outbounds = new_hash_map();
+			for (id, session_privs) in pending_outbound_payments_no_retry.drain() {
+				outbounds.insert(id, PendingOutboundPayment::Legacy { session_privs });
+			}
+			outbounds
+		} else {
+			pending_outbound_payments_compat
+		};
+
+		// Merge legacy in-flight monitor updates (keyed by OutPoint) into the new format (keyed by
+		// ChannelId).
+		if let Some(legacy_in_flight_upds) = legacy_in_flight_monitor_updates {
+			// We should never serialize an empty map.
+			if legacy_in_flight_upds.is_empty() {
+				return Err(DecodeError::InvalidValue);
+			}
+			if in_flight_monitor_updates.is_none() {
+				let in_flight_upds = in_flight_monitor_updates.get_or_insert_with(new_hash_map);
+				for ((counterparty_node_id, funding_txo), updates) in legacy_in_flight_upds {
+					// All channels with legacy in flight monitor updates are v1 channels.
+					let channel_id = ChannelId::v1_from_funding_outpoint(funding_txo);
+					in_flight_upds.insert((counterparty_node_id, channel_id), updates);
+				}
+			} else if in_flight_monitor_updates.as_ref().unwrap().is_empty() {
+				// Both TLVs present - the new one takes precedence but must not be empty.
+				return Err(DecodeError::InvalidValue);
+			}
+		}
+
+		// Resolve events_override: if present, it replaces pending_events.
+		let mut pending_events = pending_events;
+		if let Some(events) = events_override {
+			pending_events = events;
+		}
+
 		Ok(ChannelManagerData {
 			chain_hash,
 			best_block_height,
@@ -17549,8 +17586,6 @@ where
 			peer_init_features,
 			pending_events,
 			highest_seen_timestamp,
-			pending_outbound_payments_compat,
-			pending_outbound_payments_no_retry,
 			pending_intercepted_htlcs,
 			pending_outbound_payments,
 			// unwrap safety: pending_claiming_payments is guaranteed to be `Some` after read_tlv_fields
@@ -17560,9 +17595,7 @@ where
 			monitor_update_blocked_actions_per_peer: monitor_update_blocked_actions_per_peer
 				.unwrap(),
 			fake_scid_rand_bytes,
-			events_override,
 			claimable_htlc_purposes,
-			legacy_in_flight_monitor_updates,
 			probing_cookie_secret,
 			claimable_htlc_onion_fields,
 			decode_update_add_htlcs,
@@ -17897,7 +17930,7 @@ where
 		let mut claimable_htlcs_list = data.claimable_htlcs;
 		let mut pending_intercepted_htlcs_legacy =
 			data.pending_intercepted_htlcs.unwrap_or_else(new_hash_map);
-		let mut pending_outbound_payments = data.pending_outbound_payments;
+		let pending_outbound_payments = data.pending_outbound_payments;
 		let mut fake_scid_rand_bytes = data.fake_scid_rand_bytes;
 		let mut probing_cookie_secret = data.probing_cookie_secret;
 		let mut decode_update_add_htlcs_legacy =
@@ -18200,26 +18233,12 @@ where
 			inbound_payment_id_secret = Some(args.entropy_source.get_secure_random_bytes());
 		}
 
-		if let Some(events) = data.events_override {
-			pending_events_read = events;
-		}
-
 		if !channel_closures.is_empty() {
 			pending_events_read.append(&mut channel_closures);
 		}
 
-		if pending_outbound_payments.is_none() && data.pending_outbound_payments_no_retry.is_none()
-		{
-			pending_outbound_payments = Some(data.pending_outbound_payments_compat);
-		} else if pending_outbound_payments.is_none() {
-			let mut outbounds = new_hash_map();
-			for (id, session_privs) in data.pending_outbound_payments_no_retry.unwrap().drain() {
-				outbounds.insert(id, PendingOutboundPayment::Legacy { session_privs });
-			}
-			pending_outbound_payments = Some(outbounds);
-		}
 		let pending_outbounds =
-			OutboundPayments::new(pending_outbound_payments.unwrap(), args.logger.clone());
+			OutboundPayments::new(pending_outbound_payments, args.logger.clone());
 
 		if let Some(peer_storage_dir) = data.peer_storage_dir {
 			for (peer_pubkey, peer_storage) in peer_storage_dir {
@@ -18229,28 +18248,6 @@ where
 			}
 		}
 
-		// Handle transitioning from the legacy TLV to the new one on upgrades.
-		if let Some(legacy_in_flight_upds) = data.legacy_in_flight_monitor_updates {
-			// We should never serialize an empty map.
-			if legacy_in_flight_upds.is_empty() {
-				return Err(DecodeError::InvalidValue);
-			}
-			if in_flight_monitor_updates.is_none() {
-				let in_flight_upds =
-					in_flight_monitor_updates.get_or_insert_with(|| new_hash_map());
-				for ((counterparty_node_id, funding_txo), updates) in legacy_in_flight_upds {
-					// All channels with legacy in flight monitor updates are v1 channels.
-					let channel_id = ChannelId::v1_from_funding_outpoint(funding_txo);
-					in_flight_upds.insert((counterparty_node_id, channel_id), updates);
-				}
-			} else {
-				// We should never serialize an empty map.
-				if in_flight_monitor_updates.as_ref().unwrap().is_empty() {
-					return Err(DecodeError::InvalidValue);
-				}
-			}
-		}
-
 		// We have to replay (or skip, if they were completed after we wrote the `ChannelManager`)
 		// each `ChannelMonitorUpdate` in `in_flight_monitor_updates`. After doing so, we have to
 		// check that each channel we have isn't newer than the latest `ChannelMonitorUpdate`(s) we