Skip to content

Retain Electrum Filterregistrations acrossstop/start`` - #1025

Open
Jolah1 wants to merge 2 commits into
lightningdevkit:mainfrom
Jolah1:fix-electrum-filter-registrations
Open

Retain Electrum Filterregistrations acrossstop/start``#1025
Jolah1 wants to merge 2 commits into
lightningdevkit:mainfrom
Jolah1:fix-electrum-filter-registrations

Conversation

@Jolah1

@Jolah1 Jolah1 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #1005.

Problem

ElectrumRuntimeStatus::stop reset itself via *self = Self::new(), which dropped the ElectrumRuntimeClient and the pending-registration vectors. Since the client owns the ElectrumSyncClient, which in turn owns its own copy of the
registered transactions and outputs, all Filter state was gone. start then drain(..)ed the pending vectors, so registrations didn't survive even a single cycle.

This matters because ChannelMonitors only register their watched transactions and outputs while being loaded in Builder::build. A stop/start cycle that doesn't rebuild the node therefore leaves the chain source with no
registrations at all, and the node stops learning about confirmations and spends of, e.g., its funding outputs.

As discussed on the issue, replaying ChannelMonitor::load_outputs_to_watch on start wouldn't be sufficient either: the OutputSweeper also registers outputs it wants to see spent, and exposes no way to reload them.

The Esplora chain source isn't affected, as its tx_sync is long-lived and never torn down — so this also restores parity between the two backends.

Fix

ElectrumRuntimeStatus becomes a struct holding an Option<Arc> alongside a canonical, deduplicated registration inventory (HashMap<Txid, ScriptBuf> and HashSet) that is maintained regardless of whether we're currently started:

  • register_tx/register_output always record the entry, and additionally forward it to the client if one is live.
  • start replays the whole inventory to the fresh client without consuming it.
  • stop merely drops the client.

Note the inventory intentionally retains entries for already-confirmed transactions. That mirrors what already happens on every process restart via monitor load, and matches the existing behaviour of ChainSource::registered_txids, which is likewise never pruned.

client() keeps its existing Option<Arc> signature, so all call sites are untouched.

Testing

Adds electrum_registrations_survive_chain_source_restart: it opens a channel, leaves the funding transaction unconfirmed, then stops and starts the node repeatedly before confirming it. The node can only emit ChannelReady if the chain source replayed its registrations on every start — the repetition specifically covers the non-draining behaviour.

Verified the test fails on current main (node times out after 60s waiting for ChannelReady) and passes with this fix. The other Electrum-backed tests still pass.

This change was developed with the assistance of Claude Code. The design, review, and testing decisions are my own.

Jolah1 and others added 2 commits August 6, 2026 21:48
`ElectrumRuntimeStatus::stop` reset itself to `Stopped` with empty pending
registration vectors, dropping the `ElectrumRuntimeClient` and, with it, the
`ElectrumSyncClient` that owns the registered transactions and outputs.
`start` then drained the pending vectors, so nothing survived even a single
cycle.

As `ChannelMonitor`s only register their watched transactions and outputs
while being loaded in `Builder::build`, a `stop`/`start` cycle that doesn't
rebuild the node left the chain source with no registrations at all, i.e., the
node would no longer learn about confirmations or spends of, e.g., its funding
outputs. Note we can't simply re-register from
`ChannelMonitor::load_outputs_to_watch` on `start` either, as, e.g., the
`OutputSweeper` also registers outputs it wants to see spent.

Here we therefore keep a canonical, deduplicated registration inventory in
`ElectrumRuntimeStatus` that is maintained whether we're started or not:
`register_tx`/`register_output` always record the entry and additionally
forward it to the client if one is live, `start` replays the inventory to the
fresh client without consuming it, and `stop` merely drops the client.

This also restores parity with the Esplora chain source, whose `tx_sync` is
long-lived and hence never loses its registrations.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Open a channel, leave the funding transaction unconfirmed, then repeatedly
stop and start the node before confirming it. The node can only emit
`ChannelReady` if its chain source replayed the `Filter` registrations on
every `start`, so this fails without the preceding fix (the node times out
waiting for the event) and passes with it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ldk-reviews-bot

ldk-reviews-bot commented Aug 6, 2026

Copy link
Copy Markdown

I've assigned @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-reviews-bot
ldk-reviews-bot requested a review from tnull August 6, 2026 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Electrum chain source stop() discards registered Filter entries

2 participants