From 13b059829473b03035058cc0f7263bcaf02f2fb3 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 28 Feb 2025 11:02:13 -0600 Subject: [PATCH 1/3] Reject channels serialized with version <= 2 e23d32dadd839ed4b095798c192691104a5aad99 removed support for these versions, so serializations with them should be explicitly rejected. --- lightning/src/ln/channel.rs | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f43ad342b54..094976a4648 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10304,6 +10304,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch fn read(reader: &mut R, args: (&'a ES, &'b SP, u32, &'c ChannelTypeFeatures)) -> Result { let (entropy_source, signer_provider, serialized_height, our_supported_features) = args; let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION); + if ver <= 2 { + return Err(DecodeError::UnknownVersion); + } // `user_id` used to be a single u64 value. In order to remain backwards compatible with // versions prior to 0.0.113, the u128 is serialized as two separate u64 values. We read @@ -10311,13 +10314,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch let user_id_low: u64 = Readable::read(reader)?; let mut config = Some(LegacyChannelConfig::default()); - if ver == 1 { - // Read the old serialization of the ChannelConfig from version 0.0.98. - config.as_mut().unwrap().options.forwarding_fee_proportional_millionths = Readable::read(reader)?; - config.as_mut().unwrap().options.cltv_expiry_delta = Readable::read(reader)?; - config.as_mut().unwrap().announce_for_forwarding = Readable::read(reader)?; - config.as_mut().unwrap().commit_upfront_shutdown_pubkey = Readable::read(reader)?; - } else { + { // Read the 8 bytes of backwards-compatibility ChannelConfig data. let mut _val: u64 = Readable::read(reader)?; } @@ -10481,11 +10478,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch let holder_dust_limit_satoshis = Readable::read(reader)?; let counterparty_max_htlc_value_in_flight_msat = Readable::read(reader)?; let mut counterparty_selected_channel_reserve_satoshis = None; - if ver == 1 { - // Read the old serialization from version 0.0.98. - counterparty_selected_channel_reserve_satoshis = Some(Readable::read(reader)?); - } else { - // Read the 8 bytes of backwards-compatibility data. + { + // Read the 8 bytes of backwards-compatibility counterparty_selected_channel_reserve_satoshis data. let _dummy: u64 = Readable::read(reader)?; } let counterparty_htlc_minimum_msat = Readable::read(reader)?; @@ -10493,11 +10487,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch let counterparty_max_accepted_htlcs = Readable::read(reader)?; let mut minimum_depth = None; - if ver == 1 { - // Read the old serialization from version 0.0.98. - minimum_depth = Some(Readable::read(reader)?); - } else { - // Read the 4 bytes of backwards-compatibility data. + { + // Read the 4 bytes of backwards-compatibility minimum_depth data. let _dummy: u32 = Readable::read(reader)?; } From 43a57fa6fac58dc552d727f0846b506c588c4329 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 28 Feb 2025 11:06:23 -0600 Subject: [PATCH 2/3] Remove unnecessary channel_value_satoshis overwrite 21ed4778413af700c590b8aaa5e3a14855f38d08 changed ChannelTransactionParameters deserialization to pass in channel_value_satoshis, so it does not actually need to be overwritten as was done in an earlier draft of the commit. --- lightning/src/ln/channel.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 094976a4648..4f9706e7ac8 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10642,8 +10642,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch // ChannelTransactionParameters may have had an empty features set upon deserialization. // To account for that, we're proactively setting/overriding the field here. channel_parameters.channel_type_features = chan_features.clone(); - // ChannelTransactionParameters::channel_value_satoshis defaults to 0 prior to version 0.2. - channel_parameters.channel_value_satoshis = channel_value_satoshis; let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes()); From 6f7e1c5ed429c5e0536149038752d808759220ba Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 28 Feb 2025 17:51:41 +0000 Subject: [PATCH 3/3] Mark a few TLVs as `required` when reading `Channel`s e23d32dadd839ed4b095798c192691104a5aad99 removed support for reading ancient `Channel`s but left a bit of cleanup for later. Here we mark a few TLVs as `required` which were always written in 0.0.113. --- lightning/src/ln/channel.rs | 43 +++++++++++++----------------- lightning/src/ln/channelmanager.rs | 2 +- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4f9706e7ac8..dd8da25cafc 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10296,13 +10296,13 @@ impl Writeable for FundedChannel where SP::Target: SignerProvider } } -impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c ChannelTypeFeatures)> for FundedChannel +impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c ChannelTypeFeatures)> for FundedChannel where ES::Target: EntropySource, SP::Target: SignerProvider { - fn read(reader: &mut R, args: (&'a ES, &'b SP, u32, &'c ChannelTypeFeatures)) -> Result { - let (entropy_source, signer_provider, serialized_height, our_supported_features) = args; + fn read(reader: &mut R, args: (&'a ES, &'b SP, &'c ChannelTypeFeatures)) -> Result { + let (entropy_source, signer_provider, our_supported_features) = args; let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION); if ver <= 2 { return Err(DecodeError::UnknownVersion); @@ -10313,7 +10313,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch // the low bytes now and the high bytes later. let user_id_low: u64 = Readable::read(reader)?; - let mut config = Some(LegacyChannelConfig::default()); + let mut config = LegacyChannelConfig::default(); { // Read the 8 bytes of backwards-compatibility ChannelConfig data. let mut _val: u64 = Readable::read(reader)?; @@ -10533,20 +10533,20 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch // Prior to supporting channel type negotiation, all of our channels were static_remotekey // only, so we default to that if none was written. let mut channel_type = Some(ChannelTypeFeatures::only_static_remote_key()); - let mut channel_creation_height = Some(serialized_height); + let mut channel_creation_height = 0u32; let mut preimages_opt: Option>> = None; // If we read an old Channel, for simplicity we just treat it as "we never sent an // AnnouncementSignatures" which implies we'll re-send it on reconnect, but that's fine. - let mut announcement_sigs_state = Some(AnnouncementSigsState::NotSent); + let mut announcement_sigs_state = AnnouncementSigsState::NotSent; let mut latest_inbound_scid_alias = None; - let mut outbound_scid_alias = None; + let mut outbound_scid_alias = 0u64; let mut channel_pending_event_emitted = None; let mut channel_ready_event_emitted = None; let mut funding_tx_broadcast_safe_event_emitted = None; let mut user_id_high_opt: Option = None; - let mut channel_keys_id: Option<[u8; 32]> = None; + let mut channel_keys_id = [0u8; 32]; let mut temporary_channel_id: Option = None; let mut holder_max_accepted_htlcs: Option = None; @@ -10575,21 +10575,21 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch (2, channel_type, option), (3, counterparty_selected_channel_reserve_satoshis, option), (4, holder_selected_channel_reserve_satoshis, option), - (5, config, option), // Note that if none is provided we will *not* overwrite the existing one. + (5, config, required), (6, holder_max_htlc_value_in_flight_msat, option), (7, shutdown_scriptpubkey, option), (8, blocked_monitor_updates, optional_vec), (9, target_closing_feerate_sats_per_kw, option), (10, monitor_pending_update_adds, option), // Added in 0.0.122 (11, monitor_pending_finalized_fulfills, optional_vec), - (13, channel_creation_height, option), + (13, channel_creation_height, required), (15, preimages_opt, optional_vec), - (17, announcement_sigs_state, option), + (17, announcement_sigs_state, required), (19, latest_inbound_scid_alias, option), - (21, outbound_scid_alias, option), + (21, outbound_scid_alias, required), (23, channel_ready_event_emitted, option), (25, user_id_high_opt, option), - (27, channel_keys_id, option), + (27, channel_keys_id, required), (28, holder_max_accepted_htlcs, option), (29, temporary_channel_id, option), (31, channel_pending_event_emitted, option), @@ -10606,12 +10606,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch (53, funding_tx_broadcast_safe_event_emitted, option), }); - let (channel_keys_id, holder_signer) = if let Some(channel_keys_id) = channel_keys_id { - let holder_signer = signer_provider.derive_channel_signer(channel_keys_id); - (channel_keys_id, holder_signer) - } else { - return Err(DecodeError::InvalidValue); - }; + let holder_signer = signer_provider.derive_channel_signer(channel_keys_id); if let Some(preimages) = preimages_opt { let mut iter = preimages.into_iter(); @@ -10753,7 +10748,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch context: ChannelContext { user_id, - config: config.unwrap(), + config, prev_config: None, @@ -10764,7 +10759,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch channel_id, temporary_channel_id, channel_state, - announcement_sigs_state: announcement_sigs_state.unwrap(), + announcement_sigs_state, secp_ctx, latest_monitor_update_id, @@ -10814,7 +10809,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch funding_tx_confirmed_in, funding_tx_confirmation_height, short_channel_id, - channel_creation_height: channel_creation_height.unwrap(), + channel_creation_height, counterparty_dust_limit_satoshis, holder_dust_limit_satoshis, @@ -10847,7 +10842,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch latest_inbound_scid_alias, // Later in the ChannelManager deserialization phase we scan for channels and assign scid aliases if its missing - outbound_scid_alias: outbound_scid_alias.unwrap_or(0), + outbound_scid_alias, funding_tx_broadcast_safe_event_emitted: funding_tx_broadcast_safe_event_emitted.unwrap_or(false), channel_pending_event_emitted: channel_pending_event_emitted.unwrap_or(true), @@ -11556,7 +11551,7 @@ mod tests { let mut s = crate::io::Cursor::new(&encoded_chan); let mut reader = crate::util::ser::FixedLengthReader::new(&mut s, encoded_chan.len() as u64); let features = channelmanager::provided_channel_type_features(&config); - let decoded_chan = FundedChannel::read(&mut reader, (&&keys_provider, &&keys_provider, 0, &features)).unwrap(); + let decoded_chan = FundedChannel::read(&mut reader, (&&keys_provider, &&keys_provider, &features)).unwrap(); assert_eq!(decoded_chan.context.pending_outbound_htlcs, pending_outbound_htlcs); assert_eq!(decoded_chan.context.holding_cell_htlc_updates, holding_cell_htlc_updates); } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e4221cdd809..bb2c35d5420 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -13634,7 +13634,7 @@ where let mut close_background_events = Vec::new(); for _ in 0..channel_count { let mut channel: FundedChannel = FundedChannel::read(reader, ( - &args.entropy_source, &args.signer_provider, best_block_height, &provided_channel_type_features(&args.default_config) + &args.entropy_source, &args.signer_provider, &provided_channel_type_features(&args.default_config) ))?; let logger = WithChannelContext::from(&args.logger, &channel.context, None); let channel_id = channel.context.channel_id();