diff --git a/lightning-tests/src/upgrade_downgrade_tests.rs b/lightning-tests/src/upgrade_downgrade_tests.rs index 7f607bba848..7cc59227af4 100644 --- a/lightning-tests/src/upgrade_downgrade_tests.rs +++ b/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); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e07ee7fceab..63d96fc1682 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -368,8 +368,7 @@ enum InboundUpdateAdd { blinded_failure: Option, 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 Writeable for FundedChannel { } } let mut removed_htlc_attribution_data: Vec<&Option> = 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 Writeable for FundedChannel { 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)?; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a7a0942f0c8..77ce7ab7593 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/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); }