From 30901f6b585bcc7744f4102d415b82c086c18690 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 22 Jan 2026 10:36:15 +0100 Subject: [PATCH 1/6] Split ChannelManager::read into two stages Introduce ChannelManagerData as an intermediate DTO that holds all deserialized data from a ChannelManager before validation. This splits the read implementation into: 1. Stage 1: Pure deserialization into ChannelManagerData 2. Stage 2: Validation and reconstruction using the DTO The existing validation and reconstruction logic remains unchanged; only the deserialization portion was extracted into the DTO's ReadableArgs implementation. Co-Authored-By: Claude Opus 4.5 --- lightning/src/ln/channelmanager.rs | 508 +++++++++++++++++++---------- 1 file changed, 331 insertions(+), 177 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0f9adfcc51a..5719b3455be 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -17381,6 +17381,277 @@ impl Readable for VecDeque<(Event, Option)> { } } +// Raw deserialized data from a ChannelManager, before validation or reconstruction. +// This is an internal DTO used in the two-stage deserialization process. +pub(super) struct ChannelManagerData +where + SP::Target: SignerProvider, +{ + chain_hash: ChainHash, + best_block_height: u32, + best_block_hash: BlockHash, + channels: Vec>, + // Marked `_legacy` because in versions > 0.2 we are taking steps to remove the requirement of + // regularly persisting the `ChannelManager` and instead rebuild the set of HTLC forwards from + // `Channel{Monitor}` data. See [`ChannelManager::read`]. + forward_htlcs_legacy: HashMap>, + claimable_htlcs_list: Vec<(PaymentHash, Vec)>, + peer_init_features: Vec<(PublicKey, InitFeatures)>, + pending_events_read: VecDeque<(events::Event, Option)>, + highest_seen_timestamp: u32, + pending_outbound_payments_compat: HashMap, + pending_outbound_payments_no_retry: Option>>, + // Marked `_legacy` because in versions > 0.2 we are taking steps to remove the requirement of + // regularly persisting the `ChannelManager` and instead rebuild the set of HTLC forwards from + // `Channel{Monitor}` data. See [`ChannelManager::read`]. + pending_intercepted_htlcs_legacy: Option>, + pending_outbound_payments: Option>, + pending_claiming_payments: Option>, + received_network_pubkey: Option, + monitor_update_blocked_actions_per_peer: + Option>)>>, + fake_scid_rand_bytes: Option<[u8; 32]>, + events_override: Option)>>, + claimable_htlc_purposes: Option>, + legacy_in_flight_monitor_updates: + Option>>, + probing_cookie_secret: Option<[u8; 32]>, + claimable_htlc_onion_fields: Option>>, + // Marked `_legacy` because in versions > 0.2 we are taking steps to remove the requirement of + // regularly persisting the `ChannelManager` and instead rebuild the set of HTLC forwards from + // `Channel{Monitor}` data. See [`ChannelManager::read`]. + decode_update_add_htlcs_legacy: Option>>, + inbound_payment_id_secret: Option<[u8; 32]>, + in_flight_monitor_updates: Option>>, + peer_storage_dir: Option)>>, + async_receive_offer_cache: AsyncReceiveOfferCache, +} + +/// Arguments for deserializing [`ChannelManagerData`]. +struct ChannelManagerDataReadArgs<'a, ES: Deref, SP: Deref, L: Deref> +where + ES::Target: EntropySource, + SP::Target: SignerProvider, + L::Target: Logger, +{ + entropy_source: &'a ES, + signer_provider: &'a SP, + config: UserConfig, + logger: &'a L, +} + +impl<'a, ES: Deref, SP: Deref, L: Deref> ReadableArgs> + for ChannelManagerData +where + ES::Target: EntropySource, + SP::Target: SignerProvider, + L::Target: Logger, +{ + fn read( + reader: &mut R, args: ChannelManagerDataReadArgs<'a, ES, SP, L>, + ) -> Result { + let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION); + + let chain_hash: ChainHash = Readable::read(reader)?; + let best_block_height: u32 = Readable::read(reader)?; + let best_block_hash: BlockHash = Readable::read(reader)?; + + const MAX_ALLOC_SIZE: usize = 1024 * 64; + + let channel_count: u64 = Readable::read(reader)?; + let mut channels = Vec::with_capacity(cmp::min(channel_count as usize, 128)); + for _ in 0..channel_count { + let channel: FundedChannel = FundedChannel::read( + reader, + ( + args.entropy_source, + args.signer_provider, + &provided_channel_type_features(&args.config), + ), + )?; + channels.push(channel); + } + + let forward_htlcs_count: u64 = Readable::read(reader)?; + let mut forward_htlcs_legacy: HashMap> = + hash_map_with_capacity(cmp::min(forward_htlcs_count as usize, 128)); + for _ in 0..forward_htlcs_count { + let short_channel_id = Readable::read(reader)?; + let pending_forwards_count: u64 = Readable::read(reader)?; + let mut pending_forwards = Vec::with_capacity(cmp::min( + pending_forwards_count as usize, + MAX_ALLOC_SIZE / mem::size_of::(), + )); + for _ in 0..pending_forwards_count { + pending_forwards.push(Readable::read(reader)?); + } + forward_htlcs_legacy.insert(short_channel_id, pending_forwards); + } + + let claimable_htlcs_count: u64 = Readable::read(reader)?; + let mut claimable_htlcs_list = + Vec::with_capacity(cmp::min(claimable_htlcs_count as usize, 128)); + for _ in 0..claimable_htlcs_count { + let payment_hash = Readable::read(reader)?; + let previous_hops_len: u64 = Readable::read(reader)?; + let mut previous_hops = Vec::with_capacity(cmp::min( + previous_hops_len as usize, + MAX_ALLOC_SIZE / mem::size_of::(), + )); + for _ in 0..previous_hops_len { + previous_hops.push(::read(reader)?); + } + claimable_htlcs_list.push((payment_hash, previous_hops)); + } + + let peer_count: u64 = Readable::read(reader)?; + let mut peer_init_features = Vec::with_capacity(cmp::min(peer_count as usize, 128)); + for _ in 0..peer_count { + let peer_pubkey: PublicKey = Readable::read(reader)?; + let latest_features = Readable::read(reader)?; + peer_init_features.push((peer_pubkey, latest_features)); + } + + let event_count: u64 = Readable::read(reader)?; + let mut pending_events_read: VecDeque<(events::Event, Option)> = + VecDeque::with_capacity(cmp::min( + event_count as usize, + MAX_ALLOC_SIZE / mem::size_of::<(events::Event, Option)>(), + )); + for _ in 0..event_count { + match MaybeReadable::read(reader)? { + Some(event) => pending_events_read.push_back((event, None)), + None => continue, + } + } + + let background_event_count: u64 = Readable::read(reader)?; + for _ in 0..background_event_count { + match ::read(reader)? { + 0 => { + // LDK versions prior to 0.0.116 wrote pending `MonitorUpdateRegeneratedOnStartup`s here, + // however we really don't (and never did) need them - we regenerate all + // on-startup monitor updates. + let _: OutPoint = Readable::read(reader)?; + let _: ChannelMonitorUpdate = Readable::read(reader)?; + }, + _ => return Err(DecodeError::InvalidValue), + } + } + + let _last_node_announcement_serial: u32 = Readable::read(reader)?; // Only used < 0.0.111 + let highest_seen_timestamp: u32 = Readable::read(reader)?; + + // The last version where a pending inbound payment may have been added was 0.0.116. + let pending_inbound_payment_count: u64 = Readable::read(reader)?; + for _ in 0..pending_inbound_payment_count { + let payment_hash: PaymentHash = Readable::read(reader)?; + let logger = WithContext::from(args.logger, None, None, Some(payment_hash)); + let inbound: PendingInboundPayment = Readable::read(reader)?; + log_warn!( + logger, + "Ignoring deprecated pending inbound payment with payment hash {}: {:?}", + payment_hash, + inbound + ); + } + + let pending_outbound_payments_count_compat: u64 = Readable::read(reader)?; + let mut pending_outbound_payments_compat: HashMap = + hash_map_with_capacity(cmp::min( + pending_outbound_payments_count_compat as usize, + MAX_ALLOC_SIZE / 32, + )); + for _ in 0..pending_outbound_payments_count_compat { + let session_priv = Readable::read(reader)?; + let payment = PendingOutboundPayment::Legacy { + session_privs: hash_set_from_iter([session_priv]), + }; + if pending_outbound_payments_compat.insert(PaymentId(session_priv), payment).is_some() { + return Err(DecodeError::InvalidValue); + }; + } + + let mut pending_intercepted_htlcs_legacy: Option> = + None; + let mut decode_update_add_htlcs_legacy: Option>> = + None; + // pending_outbound_payments_no_retry is for compatibility with 0.0.101 clients. + let mut pending_outbound_payments_no_retry: Option>> = + None; + let mut pending_outbound_payments = None; + let mut received_network_pubkey: Option = None; + let mut fake_scid_rand_bytes: Option<[u8; 32]> = None; + let mut probing_cookie_secret: Option<[u8; 32]> = None; + let mut claimable_htlc_purposes = None; + let mut claimable_htlc_onion_fields = None; + let mut pending_claiming_payments = Some(new_hash_map()); + let mut monitor_update_blocked_actions_per_peer: Option>)>> = + Some(Vec::new()); + let mut events_override = None; + let mut legacy_in_flight_monitor_updates: Option< + HashMap<(PublicKey, OutPoint), Vec>, + > = None; + // We use this one over the legacy since they represent the same data, just with a different + // key. We still need to read the legacy one as it's an even TLV. + let mut in_flight_monitor_updates: Option< + HashMap<(PublicKey, ChannelId), Vec>, + > = None; + let mut inbound_payment_id_secret = None; + let mut peer_storage_dir: Option)>> = None; + let mut async_receive_offer_cache: AsyncReceiveOfferCache = AsyncReceiveOfferCache::new(); + read_tlv_fields!(reader, { + (1, pending_outbound_payments_no_retry, option), + (2, pending_intercepted_htlcs_legacy, option), + (3, pending_outbound_payments, option), + (4, pending_claiming_payments, option), + (5, received_network_pubkey, option), + (6, monitor_update_blocked_actions_per_peer, option), + (7, fake_scid_rand_bytes, option), + (8, events_override, option), + (9, claimable_htlc_purposes, optional_vec), + (10, legacy_in_flight_monitor_updates, option), + (11, probing_cookie_secret, option), + (13, claimable_htlc_onion_fields, optional_vec), + (14, decode_update_add_htlcs_legacy, option), + (15, inbound_payment_id_secret, option), + (17, in_flight_monitor_updates, option), + (19, peer_storage_dir, optional_vec), + (21, async_receive_offer_cache, (default_value, async_receive_offer_cache)), + }); + + Ok(ChannelManagerData { + chain_hash, + best_block_height, + best_block_hash, + channels, + forward_htlcs_legacy, + claimable_htlcs_list, + peer_init_features, + pending_events_read, + highest_seen_timestamp, + pending_outbound_payments_compat, + pending_outbound_payments_no_retry, + pending_intercepted_htlcs_legacy, + pending_outbound_payments, + pending_claiming_payments, + received_network_pubkey, + monitor_update_blocked_actions_per_peer, + fake_scid_rand_bytes, + events_override, + claimable_htlc_purposes, + legacy_in_flight_monitor_updates, + probing_cookie_secret, + claimable_htlc_onion_fields, + decode_update_add_htlcs_legacy, + inbound_payment_id_secret, + in_flight_monitor_updates, + peer_storage_dir, + async_receive_offer_cache, + }) + } +} + /// Arguments for the creation of a ChannelManager that are not deserialized. /// /// At a high-level, the process for deserializing a ChannelManager and resuming normal operation @@ -17649,11 +17920,52 @@ where fn read( reader: &mut Reader, mut args: ChannelManagerReadArgs<'a, M, T, ES, NS, SP, F, R, MR, L>, ) -> Result { - let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION); + // Stage 1: Pure deserialization into DTO + let data: ChannelManagerData = ChannelManagerData::read( + reader, + ChannelManagerDataReadArgs { + entropy_source: &args.entropy_source, + signer_provider: &args.signer_provider, + config: args.config.clone(), + logger: &args.logger, + }, + )?; - let chain_hash: ChainHash = Readable::read(reader)?; - let best_block_height: u32 = Readable::read(reader)?; - let best_block_hash: BlockHash = Readable::read(reader)?; + // Stage 2: Validation and reconstruction + let ChannelManagerData { + chain_hash, + best_block_height, + best_block_hash, + channels, + mut forward_htlcs_legacy, + mut claimable_htlcs_list, + peer_init_features, + mut pending_events_read, + highest_seen_timestamp, + pending_outbound_payments_compat, + pending_outbound_payments_no_retry, + pending_intercepted_htlcs_legacy, + mut pending_outbound_payments, + pending_claiming_payments, + received_network_pubkey, + monitor_update_blocked_actions_per_peer, + mut fake_scid_rand_bytes, + events_override, + claimable_htlc_purposes, + legacy_in_flight_monitor_updates, + mut probing_cookie_secret, + claimable_htlc_onion_fields, + decode_update_add_htlcs_legacy, + mut inbound_payment_id_secret, + mut in_flight_monitor_updates, + peer_storage_dir, + async_receive_offer_cache, + } = data; + + let mut pending_intercepted_htlcs_legacy = + pending_intercepted_htlcs_legacy.unwrap_or_else(new_hash_map); + let mut decode_update_add_htlcs_legacy = + decode_update_add_htlcs_legacy.unwrap_or_else(new_hash_map); let empty_peer_state = || PeerState { channel_by_id: new_hash_map(), @@ -17668,25 +17980,18 @@ where is_connected: false, }; + const MAX_ALLOC_SIZE: usize = 1024 * 64; let mut failed_htlcs = Vec::new(); - let channel_count: u64 = Readable::read(reader)?; - let mut channel_id_set = hash_set_with_capacity(cmp::min(channel_count as usize, 128)); + let channel_count = channels.len(); + let mut channel_id_set = hash_set_with_capacity(cmp::min(channel_count, 128)); let mut per_peer_state = hash_map_with_capacity(cmp::min( - channel_count as usize, + channel_count, MAX_ALLOC_SIZE / mem::size_of::<(PublicKey, Mutex>)>(), )); - let mut short_to_chan_info = hash_map_with_capacity(cmp::min(channel_count as usize, 128)); + let mut short_to_chan_info = hash_map_with_capacity(cmp::min(channel_count, 128)); let mut channel_closures = VecDeque::new(); 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, - &provided_channel_type_features(&args.config), - ), - )?; + for mut channel in channels { let logger = WithChannelContext::from(&args.logger, &channel.context, None); let channel_id = channel.context.channel_id(); channel_id_set.insert(channel_id); @@ -17935,168 +18240,15 @@ where } } - const MAX_ALLOC_SIZE: usize = 1024 * 64; - let forward_htlcs_count: u64 = Readable::read(reader)?; - // Marked `_legacy` because in versions > 0.2 we are taking steps to remove the requirement of - // regularly persisting the `ChannelManager` and instead rebuild the set of HTLC forwards from - // `Channel{Monitor}` data. See `reconstruct_manager_from_monitors` usage below. - let mut forward_htlcs_legacy: HashMap> = - hash_map_with_capacity(cmp::min(forward_htlcs_count as usize, 128)); - for _ in 0..forward_htlcs_count { - let short_channel_id = Readable::read(reader)?; - let pending_forwards_count: u64 = Readable::read(reader)?; - let mut pending_forwards = Vec::with_capacity(cmp::min( - pending_forwards_count as usize, - MAX_ALLOC_SIZE / mem::size_of::(), - )); - for _ in 0..pending_forwards_count { - pending_forwards.push(Readable::read(reader)?); - } - forward_htlcs_legacy.insert(short_channel_id, pending_forwards); - } - - let claimable_htlcs_count: u64 = Readable::read(reader)?; - let mut claimable_htlcs_list = - Vec::with_capacity(cmp::min(claimable_htlcs_count as usize, 128)); - for _ in 0..claimable_htlcs_count { - let payment_hash = Readable::read(reader)?; - let previous_hops_len: u64 = Readable::read(reader)?; - let mut previous_hops = Vec::with_capacity(cmp::min( - previous_hops_len as usize, - MAX_ALLOC_SIZE / mem::size_of::(), - )); - for _ in 0..previous_hops_len { - previous_hops.push(::read(reader)?); - } - claimable_htlcs_list.push((payment_hash, previous_hops)); - } - - let peer_count: u64 = Readable::read(reader)?; - for _ in 0..peer_count { - let peer_pubkey: PublicKey = Readable::read(reader)?; - let latest_features = Readable::read(reader)?; + // Apply peer features from deserialized data + for (peer_pubkey, latest_features) in peer_init_features { if let Some(peer_state) = per_peer_state.get_mut(&peer_pubkey) { peer_state.get_mut().unwrap().latest_features = latest_features; } } - let event_count: u64 = Readable::read(reader)?; - let mut pending_events_read: VecDeque<(events::Event, Option)> = - VecDeque::with_capacity(cmp::min( - event_count as usize, - MAX_ALLOC_SIZE / mem::size_of::<(events::Event, Option)>(), - )); - for _ in 0..event_count { - match MaybeReadable::read(reader)? { - Some(event) => pending_events_read.push_back((event, None)), - None => continue, - } - } - - let background_event_count: u64 = Readable::read(reader)?; - for _ in 0..background_event_count { - match ::read(reader)? { - 0 => { - // LDK versions prior to 0.0.116 wrote pending `MonitorUpdateRegeneratedOnStartup`s here, - // however we really don't (and never did) need them - we regenerate all - // on-startup monitor updates. - let _: OutPoint = Readable::read(reader)?; - let _: ChannelMonitorUpdate = Readable::read(reader)?; - }, - _ => return Err(DecodeError::InvalidValue), - } - } - - let _last_node_announcement_serial: u32 = Readable::read(reader)?; // Only used < 0.0.111 - let highest_seen_timestamp: u32 = Readable::read(reader)?; - - // The last version where a pending inbound payment may have been added was 0.0.116. - let pending_inbound_payment_count: u64 = Readable::read(reader)?; - for _ in 0..pending_inbound_payment_count { - let payment_hash: PaymentHash = Readable::read(reader)?; - let logger = WithContext::from(&args.logger, None, None, Some(payment_hash)); - let inbound: PendingInboundPayment = Readable::read(reader)?; - log_warn!( - logger, - "Ignoring deprecated pending inbound payment with payment hash {}: {:?}", - payment_hash, - inbound - ); - } - - let pending_outbound_payments_count_compat: u64 = Readable::read(reader)?; - let mut pending_outbound_payments_compat: HashMap = - hash_map_with_capacity(cmp::min( - pending_outbound_payments_count_compat as usize, - MAX_ALLOC_SIZE / 32, - )); - for _ in 0..pending_outbound_payments_count_compat { - let session_priv = Readable::read(reader)?; - let payment = PendingOutboundPayment::Legacy { - session_privs: hash_set_from_iter([session_priv]), - }; - if pending_outbound_payments_compat.insert(PaymentId(session_priv), payment).is_some() { - return Err(DecodeError::InvalidValue); - }; - } - - // Marked `_legacy` because in versions > 0.2 we are taking steps to remove the requirement of - // regularly persisting the `ChannelManager` and instead rebuild the set of HTLC forwards from - // `Channel{Monitor}` data. See `reconstruct_manager_from_monitors` below. - let mut pending_intercepted_htlcs_legacy: Option> = - None; - let mut decode_update_add_htlcs_legacy: Option>> = - None; - - // pending_outbound_payments_no_retry is for compatibility with 0.0.101 clients. - let mut pending_outbound_payments_no_retry: Option>> = - None; - let mut pending_outbound_payments = None; - let mut received_network_pubkey: Option = None; - let mut fake_scid_rand_bytes: Option<[u8; 32]> = None; - let mut probing_cookie_secret: Option<[u8; 32]> = None; - let mut claimable_htlc_purposes = None; - let mut claimable_htlc_onion_fields = None; - let mut pending_claiming_payments = Some(new_hash_map()); - let mut monitor_update_blocked_actions_per_peer: Option>)>> = - Some(Vec::new()); - let mut events_override = None; - let mut legacy_in_flight_monitor_updates: Option< - HashMap<(PublicKey, OutPoint), Vec>, - > = None; - // We use this one over the legacy since they represent the same data, just with a different - // key. We still need to read the legacy one as it's an even TLV. - let mut in_flight_monitor_updates: Option< - HashMap<(PublicKey, ChannelId), Vec>, - > = None; - let mut inbound_payment_id_secret = None; - let mut peer_storage_dir: Option)>> = None; - let mut async_receive_offer_cache: AsyncReceiveOfferCache = AsyncReceiveOfferCache::new(); - read_tlv_fields!(reader, { - (1, pending_outbound_payments_no_retry, option), - (2, pending_intercepted_htlcs_legacy, option), - (3, pending_outbound_payments, option), - (4, pending_claiming_payments, option), - (5, received_network_pubkey, option), - (6, monitor_update_blocked_actions_per_peer, option), - (7, fake_scid_rand_bytes, option), - (8, events_override, option), - (9, claimable_htlc_purposes, optional_vec), - (10, legacy_in_flight_monitor_updates, option), - (11, probing_cookie_secret, option), - (13, claimable_htlc_onion_fields, optional_vec), - (14, decode_update_add_htlcs_legacy, option), - (15, inbound_payment_id_secret, option), - (17, in_flight_monitor_updates, option), - (19, peer_storage_dir, optional_vec), - (21, async_receive_offer_cache, (default_value, async_receive_offer_cache)), - }); - let mut decode_update_add_htlcs_legacy = - decode_update_add_htlcs_legacy.unwrap_or_else(|| new_hash_map()); - let mut pending_intercepted_htlcs_legacy = - pending_intercepted_htlcs_legacy.unwrap_or_else(|| new_hash_map()); + // Post-deserialization processing let mut decode_update_add_htlcs = new_hash_map(); - let peer_storage_dir: Vec<(PublicKey, Vec)> = peer_storage_dir.unwrap_or_else(Vec::new); if fake_scid_rand_bytes.is_none() { fake_scid_rand_bytes = Some(args.entropy_source.get_secure_random_bytes()); } @@ -18128,9 +18280,11 @@ where } let pending_outbounds = OutboundPayments::new(pending_outbound_payments.unwrap()); - for (peer_pubkey, peer_storage) in peer_storage_dir { - if let Some(peer_state) = per_peer_state.get_mut(&peer_pubkey) { - peer_state.get_mut().unwrap().peer_storage = peer_storage; + if let Some(peer_storage_dir) = peer_storage_dir { + for (peer_pubkey, peer_storage) in peer_storage_dir { + if let Some(peer_state) = per_peer_state.get_mut(&peer_pubkey) { + peer_state.get_mut().unwrap().peer_storage = peer_storage; + } } } @@ -19469,7 +19623,7 @@ where //TODO: Broadcast channel update for closed channels, but only after we've made a //connection or two. - Ok((best_block_hash.clone(), channel_manager)) + Ok((best_block_hash, channel_manager)) } } From 067d0eb1c7ffd9341919f72d7b9d2e3ed0fdf15b Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 28 Jan 2026 10:42:35 +0100 Subject: [PATCH 2/6] Unwrap TLV fields with initialized defaults in ChannelManagerData For TLV fields that are initialized with Some(...) before reading and thus always have a value after deserialization, remove the Option wrapper from ChannelManagerData and unwrap when constructing it. This applies to pending_claiming_payments and monitor_update_blocked_actions_per_peer. Co-Authored-By: Claude Opus 4.5 --- lightning/src/ln/channelmanager.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5719b3455be..e9dc6f67cca 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -17406,10 +17406,10 @@ where // `Channel{Monitor}` data. See [`ChannelManager::read`]. pending_intercepted_htlcs_legacy: Option>, pending_outbound_payments: Option>, - pending_claiming_payments: Option>, + pending_claiming_payments: HashMap, received_network_pubkey: Option, monitor_update_blocked_actions_per_peer: - Option>)>>, + Vec<(PublicKey, BTreeMap>)>, fake_scid_rand_bytes: Option<[u8; 32]>, events_override: Option)>>, claimable_htlc_purposes: Option>, @@ -17634,9 +17634,12 @@ where pending_outbound_payments_no_retry, pending_intercepted_htlcs_legacy, pending_outbound_payments, - pending_claiming_payments, + // unwrap safety: pending_claiming_payments is guaranteed to be `Some` after read_tlv_fields + pending_claiming_payments: pending_claiming_payments.unwrap(), received_network_pubkey, - monitor_update_blocked_actions_per_peer, + // unwrap safety: monitor_update_blocked_actions_per_peer is guaranteed to be `Some` after read_tlv_fields + monitor_update_blocked_actions_per_peer: monitor_update_blocked_actions_per_peer + .unwrap(), fake_scid_rand_bytes, events_override, claimable_htlc_purposes, @@ -19111,9 +19114,7 @@ where let bounded_fee_estimator = LowerBoundedFeeEstimator::new(args.fee_estimator); - for (node_id, monitor_update_blocked_actions) in - monitor_update_blocked_actions_per_peer.unwrap() - { + for (node_id, monitor_update_blocked_actions) in monitor_update_blocked_actions_per_peer { if let Some(peer_state) = per_peer_state.get(&node_id) { for (channel_id, actions) in monitor_update_blocked_actions.iter() { let logger = @@ -19299,7 +19300,7 @@ where decode_update_add_htlcs: Mutex::new(decode_update_add_htlcs), claimable_payments: Mutex::new(ClaimablePayments { claimable_payments, - pending_claiming_payments: pending_claiming_payments.unwrap(), + pending_claiming_payments, }), outbound_scid_aliases: Mutex::new(outbound_scid_aliases), short_to_chan_info: FairRwLock::new(short_to_chan_info), From b72f0c32766522bd46f066f861b6a4716e788058 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 28 Jan 2026 10:50:15 +0100 Subject: [PATCH 3/6] Extract second stage of ChannelManager::read into from_channel_manager_data Move the validation and reconstruction logic (stage 2) from the ReadableArgs::read implementation into a new pub(super) constructor `from_channel_manager_data`. This separates the pure deserialization from the complex reconstruction logic, making the code more modular and easier to test. The read function now: 1. Deserializes into ChannelManagerData (stage 1) 2. Calls from_channel_manager_data for validation/reconstruction (stage 2) Co-Authored-By: Claude Opus 4.5 --- lightning/src/ln/channelmanager.rs | 40 +++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e9dc6f67cca..01b240fd757 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -17921,7 +17921,7 @@ where L::Target: Logger, { fn read( - reader: &mut Reader, mut args: ChannelManagerReadArgs<'a, M, T, ES, NS, SP, F, R, MR, L>, + reader: &mut Reader, args: ChannelManagerReadArgs<'a, M, T, ES, NS, SP, F, R, MR, L>, ) -> Result { // Stage 1: Pure deserialization into DTO let data: ChannelManagerData = ChannelManagerData::read( @@ -17935,6 +17935,44 @@ where )?; // Stage 2: Validation and reconstruction + ChannelManager::from_channel_manager_data(data, args) + } +} + +impl< + M: Deref, + T: Deref, + ES: Deref, + NS: Deref, + SP: Deref, + F: Deref, + R: Deref, + MR: Deref, + L: Deref + Clone, + > ChannelManager +where + M::Target: chain::Watch<::EcdsaSigner>, + T::Target: BroadcasterInterface, + ES::Target: EntropySource, + NS::Target: NodeSigner, + SP::Target: SignerProvider, + F::Target: FeeEstimator, + R::Target: Router, + MR::Target: MessageRouter, + L::Target: Logger, +{ + /// Constructs a `ChannelManager` from deserialized data and runtime dependencies. + /// + /// This is the second stage of deserialization, taking the raw [`ChannelManagerData`] and combining it with the + /// provided [`ChannelManagerReadArgs`] to produce a fully functional `ChannelManager`. + /// + /// This method performs validation, reconciliation with [`ChannelMonitor`]s, and reconstruction of internal state. + /// It may close channels if monitors are ahead of the serialized state, and will replay any pending + /// [`ChannelMonitorUpdate`]s. + pub(super) fn from_channel_manager_data( + data: ChannelManagerData, + mut args: ChannelManagerReadArgs<'_, M, T, ES, NS, SP, F, R, MR, L>, + ) -> Result<(BlockHash, Self), DecodeError> { let ChannelManagerData { chain_hash, best_block_height, From de6f64040af44456b4e1fe7a44e0b1c87befc94c Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 28 Jan 2026 15:35:08 +0100 Subject: [PATCH 4/6] Resolve legacy TLV fields during ChannelManagerData deserialization Move the resolution of legacy/compatibility TLV fields from from_channel_manager_data (stage 2) into ChannelManagerData::read (stage 1). This keeps ChannelManagerData minimal by consolidating mutually exclusive fields into their final form during deserialization: - pending_outbound_payments: Merge TLV 3, TLV 1 (no_retry), and non-TLV compat fields into a single HashMap - in_flight_monitor_updates: Convert legacy TLV 10 (keyed by OutPoint) to TLV 17 format (keyed by ChannelId) - pending_events: Apply events_override (TLV 8) if present Co-Authored-By: Claude Opus 4.5 --- lightning/src/ln/channelmanager.rs | 97 ++++++++++++++---------------- 1 file changed, 46 insertions(+), 51 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 01b240fd757..8734595e783 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -17399,22 +17399,17 @@ where peer_init_features: Vec<(PublicKey, InitFeatures)>, pending_events_read: VecDeque<(events::Event, Option)>, highest_seen_timestamp: u32, - pending_outbound_payments_compat: HashMap, - pending_outbound_payments_no_retry: Option>>, // Marked `_legacy` because in versions > 0.2 we are taking steps to remove the requirement of // regularly persisting the `ChannelManager` and instead rebuild the set of HTLC forwards from // `Channel{Monitor}` data. See [`ChannelManager::read`]. pending_intercepted_htlcs_legacy: Option>, - pending_outbound_payments: Option>, + pending_outbound_payments: HashMap, pending_claiming_payments: HashMap, received_network_pubkey: Option, monitor_update_blocked_actions_per_peer: Vec<(PublicKey, BTreeMap>)>, fake_scid_rand_bytes: Option<[u8; 32]>, - events_override: Option)>>, claimable_htlc_purposes: Option>, - legacy_in_flight_monitor_updates: - Option>>, probing_cookie_secret: Option<[u8; 32]>, claimable_htlc_onion_fields: Option>>, // Marked `_legacy` because in versions > 0.2 we are taking steps to remove the requirement of @@ -17620,6 +17615,49 @@ where (21, async_receive_offer_cache, (default_value, async_receive_offer_cache)), }); + // Merge legacy pending_outbound_payments fields into a single HashMap. + // Priority: pending_outbound_payments (TLV 3) > pending_outbound_payments_no_retry (TLV 1) + // > pending_outbound_payments_compat (non-TLV legacy) + let pending_outbound_payments = if let Some(payments) = pending_outbound_payments { + payments + } else if let Some(mut pending_outbound_payments_no_retry) = + pending_outbound_payments_no_retry + { + let mut outbounds = new_hash_map(); + for (id, session_privs) in pending_outbound_payments_no_retry.drain() { + outbounds.insert(id, PendingOutboundPayment::Legacy { session_privs }); + } + outbounds + } else { + pending_outbound_payments_compat + }; + + // Merge legacy in-flight monitor updates (keyed by OutPoint) into the new format (keyed by + // ChannelId). + if let Some(legacy_in_flight_upds) = legacy_in_flight_monitor_updates { + // We should never serialize an empty map. + if legacy_in_flight_upds.is_empty() { + return Err(DecodeError::InvalidValue); + } + if in_flight_monitor_updates.is_none() { + let in_flight_upds = in_flight_monitor_updates.get_or_insert_with(new_hash_map); + for ((counterparty_node_id, funding_txo), updates) in legacy_in_flight_upds { + // All channels with legacy in flight monitor updates are v1 channels. + let channel_id = ChannelId::v1_from_funding_outpoint(funding_txo); + in_flight_upds.insert((counterparty_node_id, channel_id), updates); + } + } else if in_flight_monitor_updates.as_ref().unwrap().is_empty() { + // Both TLVs present - the new one takes precedence but must not be empty. + return Err(DecodeError::InvalidValue); + } + } + + // Resolve events_override: if present, it replaces pending_events. + let mut pending_events_read = pending_events_read; + if let Some(events) = events_override { + pending_events_read = events; + } + Ok(ChannelManagerData { chain_hash, best_block_height, @@ -17630,8 +17668,6 @@ where peer_init_features, pending_events_read, highest_seen_timestamp, - pending_outbound_payments_compat, - pending_outbound_payments_no_retry, pending_intercepted_htlcs_legacy, pending_outbound_payments, // unwrap safety: pending_claiming_payments is guaranteed to be `Some` after read_tlv_fields @@ -17641,9 +17677,7 @@ where monitor_update_blocked_actions_per_peer: monitor_update_blocked_actions_per_peer .unwrap(), fake_scid_rand_bytes, - events_override, claimable_htlc_purposes, - legacy_in_flight_monitor_updates, probing_cookie_secret, claimable_htlc_onion_fields, decode_update_add_htlcs_legacy, @@ -17983,17 +18017,13 @@ where peer_init_features, mut pending_events_read, highest_seen_timestamp, - pending_outbound_payments_compat, - pending_outbound_payments_no_retry, pending_intercepted_htlcs_legacy, - mut pending_outbound_payments, + pending_outbound_payments, pending_claiming_payments, received_network_pubkey, monitor_update_blocked_actions_per_peer, mut fake_scid_rand_bytes, - events_override, claimable_htlc_purposes, - legacy_in_flight_monitor_updates, mut probing_cookie_secret, claimable_htlc_onion_fields, decode_update_add_htlcs_legacy, @@ -18302,24 +18332,11 @@ where inbound_payment_id_secret = Some(args.entropy_source.get_secure_random_bytes()); } - if let Some(events) = events_override { - pending_events_read = events; - } - if !channel_closures.is_empty() { pending_events_read.append(&mut channel_closures); } - if pending_outbound_payments.is_none() && pending_outbound_payments_no_retry.is_none() { - pending_outbound_payments = Some(pending_outbound_payments_compat); - } else if pending_outbound_payments.is_none() { - let mut outbounds = new_hash_map(); - for (id, session_privs) in pending_outbound_payments_no_retry.unwrap().drain() { - outbounds.insert(id, PendingOutboundPayment::Legacy { session_privs }); - } - pending_outbound_payments = Some(outbounds); - } - let pending_outbounds = OutboundPayments::new(pending_outbound_payments.unwrap()); + let pending_outbounds = OutboundPayments::new(pending_outbound_payments); if let Some(peer_storage_dir) = peer_storage_dir { for (peer_pubkey, peer_storage) in peer_storage_dir { @@ -18329,28 +18346,6 @@ where } } - // Handle transitioning from the legacy TLV to the new one on upgrades. - if let Some(legacy_in_flight_upds) = legacy_in_flight_monitor_updates { - // We should never serialize an empty map. - if legacy_in_flight_upds.is_empty() { - return Err(DecodeError::InvalidValue); - } - if in_flight_monitor_updates.is_none() { - let in_flight_upds = - in_flight_monitor_updates.get_or_insert_with(|| new_hash_map()); - for ((counterparty_node_id, funding_txo), updates) in legacy_in_flight_upds { - // All channels with legacy in flight monitor updates are v1 channels. - let channel_id = ChannelId::v1_from_funding_outpoint(funding_txo); - in_flight_upds.insert((counterparty_node_id, channel_id), updates); - } - } else { - // We should never serialize an empty map. - if in_flight_monitor_updates.as_ref().unwrap().is_empty() { - return Err(DecodeError::InvalidValue); - } - } - } - // We have to replay (or skip, if they were completed after we wrote the `ChannelManager`) // each `ChannelMonitorUpdate` in `in_flight_monitor_updates`. After doing so, we have to // check that each channel we have isn't newer than the latest `ChannelMonitorUpdate`(s) we From 0f3cc73a6b92cbcb811ab40f1bbc1216130fdc18 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 29 Jan 2026 11:18:27 +0100 Subject: [PATCH 5/6] Simplify legacy TLV resolution in ChannelManagerData::read The previous commit intentionally kept the code as close to a move as possible to ease review. This follow-up applies idiomatic simplifications: - Use unwrap_or for events_override resolution - Use unwrap_or_else with iterator chains for pending_outbound_payments - Use match with into_iter().collect() for in_flight_monitor_updates Co-Authored-By: Claude Opus 4.5 --- lightning/src/ln/channelmanager.rs | 57 ++++++++++++++++-------------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8734595e783..7194fadf4df 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -17618,19 +17618,18 @@ where // Merge legacy pending_outbound_payments fields into a single HashMap. // Priority: pending_outbound_payments (TLV 3) > pending_outbound_payments_no_retry (TLV 1) // > pending_outbound_payments_compat (non-TLV legacy) - let pending_outbound_payments = if let Some(payments) = pending_outbound_payments { - payments - } else if let Some(mut pending_outbound_payments_no_retry) = - pending_outbound_payments_no_retry - { - let mut outbounds = new_hash_map(); - for (id, session_privs) in pending_outbound_payments_no_retry.drain() { - outbounds.insert(id, PendingOutboundPayment::Legacy { session_privs }); - } - outbounds - } else { - pending_outbound_payments_compat - }; + let pending_outbound_payments = pending_outbound_payments + .or_else(|| { + pending_outbound_payments_no_retry.map(|no_retry| { + no_retry + .into_iter() + .map(|(id, session_privs)| { + (id, PendingOutboundPayment::Legacy { session_privs }) + }) + .collect() + }) + }) + .unwrap_or(pending_outbound_payments_compat); // Merge legacy in-flight monitor updates (keyed by OutPoint) into the new format (keyed by // ChannelId). @@ -17639,24 +17638,30 @@ where if legacy_in_flight_upds.is_empty() { return Err(DecodeError::InvalidValue); } - if in_flight_monitor_updates.is_none() { - let in_flight_upds = in_flight_monitor_updates.get_or_insert_with(new_hash_map); - for ((counterparty_node_id, funding_txo), updates) in legacy_in_flight_upds { + match &in_flight_monitor_updates { + None => { + // Convert legacy format (OutPoint) to new format (ChannelId). // All channels with legacy in flight monitor updates are v1 channels. - let channel_id = ChannelId::v1_from_funding_outpoint(funding_txo); - in_flight_upds.insert((counterparty_node_id, channel_id), updates); - } - } else if in_flight_monitor_updates.as_ref().unwrap().is_empty() { - // Both TLVs present - the new one takes precedence but must not be empty. - return Err(DecodeError::InvalidValue); + in_flight_monitor_updates = Some( + legacy_in_flight_upds + .into_iter() + .map(|((counterparty_node_id, funding_txo), updates)| { + let channel_id = ChannelId::v1_from_funding_outpoint(funding_txo); + ((counterparty_node_id, channel_id), updates) + }) + .collect(), + ); + }, + Some(upds) if upds.is_empty() => { + // Both TLVs present but new one is empty - invalid. + return Err(DecodeError::InvalidValue); + }, + Some(_) => {}, // New format takes precedence, nothing to do. } } // Resolve events_override: if present, it replaces pending_events. - let mut pending_events_read = pending_events_read; - if let Some(events) = events_override { - pending_events_read = events; - } + let pending_events_read = events_override.unwrap_or(pending_events_read); Ok(ChannelManagerData { chain_hash, From 71f99fb6ba23f38d62b96cb94373be57abd4f3ed Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Fri, 30 Jan 2026 10:08:28 +0100 Subject: [PATCH 6/6] Resolve optional hash map TLV fields during ChannelManagerData deserialization Move the unwrap_or_else(new_hash_map) resolution for pending_intercepted_htlcs and decode_update_add_htlcs from stage 2 (from_channel_manager_data) to stage 1 (ChannelManagerData::read). This changes the struct fields from Option to HashMap, making it explicit that these are always present after deserialization. Co-Authored-By: Claude Opus 4.5 --- lightning/src/ln/channelmanager.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 7194fadf4df..edeed128b93 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -17402,7 +17402,7 @@ where // Marked `_legacy` because in versions > 0.2 we are taking steps to remove the requirement of // regularly persisting the `ChannelManager` and instead rebuild the set of HTLC forwards from // `Channel{Monitor}` data. See [`ChannelManager::read`]. - pending_intercepted_htlcs_legacy: Option>, + pending_intercepted_htlcs_legacy: HashMap, pending_outbound_payments: HashMap, pending_claiming_payments: HashMap, received_network_pubkey: Option, @@ -17415,7 +17415,7 @@ where // Marked `_legacy` because in versions > 0.2 we are taking steps to remove the requirement of // regularly persisting the `ChannelManager` and instead rebuild the set of HTLC forwards from // `Channel{Monitor}` data. See [`ChannelManager::read`]. - decode_update_add_htlcs_legacy: Option>>, + decode_update_add_htlcs_legacy: HashMap>, inbound_payment_id_secret: Option<[u8; 32]>, in_flight_monitor_updates: Option>>, peer_storage_dir: Option)>>, @@ -17673,7 +17673,8 @@ where peer_init_features, pending_events_read, highest_seen_timestamp, - pending_intercepted_htlcs_legacy, + pending_intercepted_htlcs_legacy: pending_intercepted_htlcs_legacy + .unwrap_or_else(new_hash_map), pending_outbound_payments, // unwrap safety: pending_claiming_payments is guaranteed to be `Some` after read_tlv_fields pending_claiming_payments: pending_claiming_payments.unwrap(), @@ -17685,7 +17686,8 @@ where claimable_htlc_purposes, probing_cookie_secret, claimable_htlc_onion_fields, - decode_update_add_htlcs_legacy, + decode_update_add_htlcs_legacy: decode_update_add_htlcs_legacy + .unwrap_or_else(new_hash_map), inbound_payment_id_secret, in_flight_monitor_updates, peer_storage_dir, @@ -18022,7 +18024,7 @@ where peer_init_features, mut pending_events_read, highest_seen_timestamp, - pending_intercepted_htlcs_legacy, + mut pending_intercepted_htlcs_legacy, pending_outbound_payments, pending_claiming_payments, received_network_pubkey, @@ -18031,18 +18033,13 @@ where claimable_htlc_purposes, mut probing_cookie_secret, claimable_htlc_onion_fields, - decode_update_add_htlcs_legacy, + mut decode_update_add_htlcs_legacy, mut inbound_payment_id_secret, mut in_flight_monitor_updates, peer_storage_dir, async_receive_offer_cache, } = data; - let mut pending_intercepted_htlcs_legacy = - pending_intercepted_htlcs_legacy.unwrap_or_else(new_hash_map); - let mut decode_update_add_htlcs_legacy = - decode_update_add_htlcs_legacy.unwrap_or_else(new_hash_map); - let empty_peer_state = || PeerState { channel_by_id: new_hash_map(), inbound_channel_request_by_id: new_hash_map(),