Simplify legacy TLV resolution in ChannelManagerData::read
What the developer wrote
Authored by Joost Jager
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 <noreply@anthropic.com>
What changed, and why it matters
This commit is a follow-up cleanup that rewrites three sections of deserialization logic in rust-lightning's ChannelManager to use more idiomatic Rust iterator methods. It does not change the underlying behavior, data formats, or security checks; it only makes the code shorter and easier to read.
No security action required. Treat as a normal code-quality refactor; standard review and CI testing are sufficient.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff refactors legacy TLV field resolution in ChannelManagerData::read. pending_outbound_payments is now built with Option::or_else + into_iter().map().collect() instead of nested if-let blocks. in_flight_monitor_updates uses a match with into_iter().collect() to convert legacy OutPoint-keyed entries to ChannelId-keyed entries, preserving the same validation (empty legacy set is invalid; new TLV present and empty is invalid; new TLV present and non-empty wins). events_override resolution is simplified to unwrap_or. No semantic changes are visible in the diff.
Changed components
lightning/src/ln/channelmanager.rs deserialization (ChannelManagerData::read)Inspect captured patch +31 / −26
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 516b902..52fcf69 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -17441,19 +17441,18 @@ impl<'a, ES: EntropySource, SP: SignerProvider, L: Logger>
// 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).
@@ -17462,24 +17461,30 @@ impl<'a, ES: EntropySource, SP: SignerProvider, L: Logger>
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,
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.