AI-generated analysisPublished automatically and not human-verified. Validated context appears in community notes below.
← Watch feed
Low 42 Bitcoin

Rebuild manager forwarded htlcs maps from Channels

Public commit record

What the developer wrote

Authored by Valentine Wallace

95/100 · Strong
Rebuild manager forwarded htlcs maps from Channels

We have an overarching goal of (mostly) getting rid of ChannelManager
persistence and rebuilding the ChannelManager's state from existing
ChannelMonitors, due to issues when the two structs are out-of-sync on restart.
The main issue that can arise is channel force closure.

Here we start this process by rebuilding
ChannelManager::decode_update_add_htlcs, forward_htlcs, and
pending_intercepted_htlcs from Channel data, which will soon be included in the
ChannelMonitors as part of a different series of PRs.

We also fix the reload_node test util to use the node's pre-reload config after
restart. The previous behavior was a bit surprising and led to one of this
commit's tests failing.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification
The short version

What changed, and why it matters

This commit changes how a Lightning node rebuilds its internal list of in-flight forwarded payments when it restarts. The goal is to eventually stop requiring a separate snapshot of the ChannelManager, instead reconstructing state from the more-reliable ChannelMonitor. The patch adds logic to avoid duplicate HTLC entries when both old and new reconstruction sources are present, and fixes a test helper so reload tests use the node's actual configuration. It is a forward-looking infrastructure change with safety implications if the deduplication logic is wrong, but it does not appear to introduce a remotely exploitable vulnerability.

Recommended action

Review the deduplication conditions carefully during audit: ensure prev_outbound_scid_alias and htlc_id pairs are globally unique, that FailHTLC/FailMalformedHTLC entries are correctly matched, and that pending_events_read stays consistent with pending_intercepted_htlcs. Run the new reload tests and any existing inconsistent-monitor/data-loss tests. Treat this as a reliability/hardening change rather than an urgent security patch.

Security signals we found

01

State-reconstruction logic for in-flight HTLCs during node restart

02

Deduplication between legacy and newly-rebuilt HTLC maps

03

Potential for double-processing or dropped HTLCs if reconciliation is incorrect

04

Test-only change to reload_node! config preservation, which surfaced a real test failure

05

Part of a larger effort to reduce ChannelManager persistence and avoid force-closure from ChannelManager/ChannelMonitor desync

Risk score

Why this scored 42/100

Our methodology →
Potential impact 12/30
Exploitability 5/25
Stealth signal 8/15
Affected reach 7/15
Confidence 7/10
Evidence quality 3/5
Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.