Don't persist inbound committed onions in prod

A few PRs ago, we started persisting inbound committed HTLC onions in Channels.
These onions were persisted to lay groundwork for reconstructing the
ChannelManager's pending HTLC maps from them in a future version. However,
we've since discovered a different direction where we can instead reconstruct
these same maps using persistent monitor events, which may mean that we don't
need to persist these onions.

Since persisting a bunch of onions on every manager write is a big commitment
that we're not fully confident in yet, switch it off for now until we either
confirm the monitor events direction and can delete all this onion persisting
code OR realize that we definitely do need it.

Author: valentinewallace

lightning/src/ln/channel.rs

@@ -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, {