Skip to content

Electrum chain source stop() discards registered Filter entries #1005

Description

@tnull

Discovered in #992 (but pre-existing)

Claude:

The new failure-path chain_source.stop() silently discards Electrum Filter registrations, breaking a failed-start → retry cycle for nodes with existing channels.

During Builder::build, channel monitors are loaded (src/builder.rs:1987) and register their watched txs/outputs with the chain source's Filter impl. For Electrum, these are queued in ElectrumRuntimeStatus::Stopped { pending_registered_txs, pending_registered_outputs } until start() applies
them. But ElectrumRuntimeStatus::stop() does *self = Self::new() (src/chain/electrum.rs:456), wiping both the running client and any pending registrations, and nothing re-registers them afterward — the outer ChainSource::registered_txids set survives but is never re-applied (and doesn't
carry scripts/outputs).

Concrete scenario: a node with existing channels calls start(), the fee-rate update or 0FC validation fails transiently (network blip) → the new error path calls chain_source.stop() → the app retries start(), which now succeeds — but the fresh ElectrumRuntimeClient's tx-sync watches nothing.
Confirmations/spends of existing funding outputs (e.g. a counterparty force-close) go undetected until a full process restart.

To be fair, a normal stop() → start() cycle already has this property on main, so it may be a known limitation — but pre-PR the failed-start path left the registrations intact (at the cost of the leak this PR fixes), and a failed first start with a retry loop is a much more common pattern
than a deliberate stop/restart. I'd suggest having ElectrumRuntimeStatus::stop() preserve the registration set (transition back to Stopped carrying the known registrations) rather than resetting to empty — that would fix both this path and the pre-existing stop/start case.

The long-term fix would still be to get rid of the start/stop mechanism entirely (see bitcoindevkit/rust-electrum-client#166), but as a hotfix we probably should make sure we trigger re-registration after restart (via load_outputs_to_watch

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions