Don't persist inbound committed onions in prod

lightning-tests/src/upgrade_downgrade_tests.rs

@@ -540,11 +540,10 @@ fn upgrade_mid_htlc_intercept_forward() {
 }
 
 fn do_upgrade_mid_htlc_forward(test: MidHtlcForwardCase) {
-	// In 0.3, we started reconstructing the `ChannelManager`'s HTLC forwards maps from the HTLCs
-	// contained in `Channel`s, as part of removing the requirement to regularly persist the
-	// `ChannelManager`. However, HTLC forwards can only be reconstructed this way if they were
-	// received on 0.3 or higher. Test that HTLC forwards that were serialized on <=0.2 will still
-	// succeed when read on 0.3+.
+	// In an upcoming version, we plan to start reconstructing the `ChannelManager`'s HTLC forwards
+	// maps from the HTLCs contained in `Channel`s, as part of removing the requirement to regularly
+	// persist the `ChannelManager`. Preemptively test that HTLC forwards that were serialized on
+	// <=0.2 will still succeed when read on this upcoming version.
 	let (node_a_ser, node_b_ser, node_c_ser, mon_a_1_ser, mon_b_1_ser, mon_b_2_ser, mon_c_1_ser);
 	let (node_a_id, node_b_id, node_c_id);
 	let (payment_secret_bytes, payment_hash_bytes, payment_preimage_bytes);

lightning/src/ln/channel.rs

@@ -368,8 +368,7 @@ enum InboundUpdateAdd {
 		blinded_failure: Option<BlindedFailure>,
 		outbound_hop: OutboundHop,
 	},
-	/// This HTLC was received pre-LDK 0.3, before we started persisting the onion for inbound
-	/// committed HTLCs.
+	/// This HTLC was received before we started persisting the onion for inbound committed HTLCs.
 	Legacy,
 }
 
@@ -7982,8 +7981,9 @@ where
 		Ok(())
 	}
 
-	/// 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.
+	/// Returns true if any committed inbound HTLCs were received before we started serializing
+	/// inbound committed payment onions in `Channel` 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!(
@@ -15570,6 +15570,7 @@ impl<SP: SignerProvider> Writeable for FundedChannel<SP> {
 			}
 		}
 		let mut removed_htlc_attribution_data: Vec<&Option<AttributionData>> = Vec::new();
+		#[cfg_attr(not(test), allow(unused_mut))]
 		let mut inbound_committed_update_adds: Vec<&InboundUpdateAdd> = Vec::new();
 		(self.context.pending_inbound_htlcs.len() as u64 - dropped_inbound_htlcs).write(writer)?;
 		for htlc in self.context.pending_inbound_htlcs.iter() {
@@ -15590,9 +15591,10 @@ impl<SP: SignerProvider> Writeable for FundedChannel<SP> {
 					2u8.write(writer)?;
 					htlc_resolution.write(writer)?;
 				},
-				&InboundHTLCState::Committed { ref update_add_htlc } => {
+				&InboundHTLCState::Committed { update_add_htlc: ref _update_add } => {
 					3u8.write(writer)?;
-					inbound_committed_update_adds.push(update_add_htlc);
+					#[cfg(test)]
+					inbound_committed_update_adds.push(_update_add);
 				},
 				&InboundHTLCState::LocalRemoved(ref removal_reason) => {
 					4u8.write(writer)?;

lightning/src/ln/channelmanager.rs

@@ -17600,15 +17600,13 @@ pub fn provided_init_features(config: &UserConfig) -> InitFeatures {
 const SERIALIZATION_VERSION: u8 = 1;
 const MIN_SERIALIZATION_VERSION: u8 = 1;
 
-// We plan to start writing this version in 0.5.
+// We plan to start writing this version a few versions after we start writing inbound committed
+// payment onions in `Channel`, which is already done in tests but not yet switched on in prod.
 //
-// LDK 0.5+ will reconstruct the set of pending HTLCs from `Channel{Monitor}` data that started
-// being written in 0.3, ignoring legacy `ChannelManager` HTLC maps on read and not writing them.
-// LDK 0.5+ will automatically fail to read if the pending HTLC set cannot be reconstructed, i.e.
-// if we were last written with pending HTLCs on 0.2- or if the new 0.3+ fields are missing.
-//
-// If 0.3 or 0.4 reads this manager version, it knows that the legacy maps were not written and
-// acts accordingly.
+// If `version == 2` on read, we will use said onions when reconstructing the set of pending HTLCs,
+// ignoring legacy `ChannelManager` HTLC maps on read and not writing them. We'll also
+// automatically fail to read if the pending HTLC set cannot be reconstructed, i.e. if the new
+// payment onion field is missing.
 const RECONSTRUCT_HTLCS_FROM_CHANS_VERSION: u8 = 2;
 
 impl_writeable_tlv_based!(PhantomRouteHints, {
@@ -19636,7 +19634,6 @@ 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);
 								}