From bd11796db79c99baa3a157c2a208194e75616363 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 28 Feb 2025 17:42:58 +0000 Subject: [PATCH 1/3] Simplify `Channel` deserialization in a few places e23d32dadd839ed4b095798c192691104a5aad99 removed support for reading ancient `Channel`s but left a bit of cleanup for later. Here we clean up deserialization a bit by removing old v1 channel read logic. --- lightning/src/ln/channel.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f43ad342b54..6f019de27f7 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10305,22 +10305,17 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch 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::InvalidValue); + } + // `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 // the low bytes now and the high bytes later. 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)?; - } + // Read the 8 bytes of backwards-compatibility ChannelConfig data. + let mut _val: u64 = Readable::read(reader)?; let channel_id: ChannelId = Readable::read(reader)?; let channel_state = ChannelState::from_u32(Readable::read(reader)?).map_err(|_| DecodeError::InvalidValue)?; @@ -10578,6 +10573,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch let mut next_holder_commitment_point_opt: Option = None; let mut is_manual_broadcast = None; + let mut config = Some(LegacyChannelConfig::default()); + read_tlv_fields!(reader, { (0, announcement_sigs, option), (1, minimum_depth, option), From 0af8ddb2d15b6e89ba7b50ef38b28e02e6e329af Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 28 Feb 2025 17:51:41 +0000 Subject: [PATCH 2/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 6f019de27f7..9c87662bf1e 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 { @@ -10537,20 +10537,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; @@ -10573,7 +10573,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch let mut next_holder_commitment_point_opt: Option = None; let mut is_manual_broadcast = None; - let mut config = Some(LegacyChannelConfig::default()); + let mut config = LegacyChannelConfig::default(); read_tlv_fields!(reader, { (0, announcement_sigs, option), @@ -10581,21 +10581,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), @@ -10612,12 +10612,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(); @@ -10761,7 +10756,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, @@ -10772,7 +10767,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, @@ -10822,7 +10817,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, @@ -10855,7 +10850,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), @@ -11564,7 +11559,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(); From 0b274573d7bab9a2ab574ec99d5cb3574e7ab1aa Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 28 Feb 2025 17:53:51 +0000 Subject: [PATCH 3/3] Remove spurious set introduced in 21ed4778413af700c590b8aaa5e3a1485 21ed477841 got rewritten a few times before it landed and ended up with a spurious set call which is redaundant against the parameter passed in to `ReadableArgs`. This removes that set. --- 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 9c87662bf1e..67ce96a3600 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10643,8 +10643,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel // 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());