Repository navigation
Electrum client per wallet refactor - #1510
Draft
julian-CStack wants to merge 25 commits into
Draft
julian-CStack wants to merge 25 commits into
julian-CStack wants to merge 25 commits into
Conversation
added 22 commits
November 11, 2026 10:10
Allow users to review and approve untrusted Electrum certificates per endpoint. Persist approvals, propagate separate primary/failover certificates, and retire connections when approvals change. Guard stale tests and node-form replacement. Integrate electrum_adapter 5.0.0 with bounded setup, subscription lifecycle handling, setup failover, and caller-specific request deadlines. History and broadcast requests default to five minutes; Firo set requests use two minutes. Broadcasts retain existing send/retry behavior with a longer per-attempt deadline. Validation: 246 tests across 36 suites passed. Analysis of 45 changed Dart files found no errors; existing warnings/lint findings remain. All changed Dart files pass formatting checks. Production dependency pins resolved without overrides.
A failed header subscription detached and closed the coin's shared client at once, failing other wallets' requests that were still waiting on it. Retire it like a failover replacement instead, so those requests finish before the connection closes.
A damaged saved certificate on a failover was only detected after the coin's working connection had been retired, so every primary failure cost other wallets a reconnect for an attempt that could not succeed. Validate the approval first.
Connections still finishing requests after a failover could keep using a changed or revoked approval when no wallet of the coin was loaded to close them. The node change now closes them itself.
Replace the per-coin ClientManager registry with a connection owned by each ElectrumXClient instance, so each loaded wallet has its own socket. - getElectrumAdapter() returns the instance's own client. - Failover retires the previous connection with DrainingElectrumClient, tracked per instance, so this client's in-flight requests finish. A connection on an outdated Tor route is closed instead of drained. - closeAdapter() and Tor preference changes close the current, pending and draining connections at once. - A corrupt saved PEM is still checked before the working connection is retired, resets the failover index and is rethrown. - fetchChainHeight() requests the tip on the wallet's own connection instead of reading the shared header subscription, and keeps its Tor/clearnet-only node check. - openConnection() is a protected hook so tests can supply a transport. - node_change_notifier only updates loaded wallets; there is no shared connection left to close.
The watched-transaction check builds its own ElectrumXClient every interval. It no longer shares a per-coin connection, so close it once the check finishes. The node-change listener stays: it still closes a setup that is pending when an approval or node changes.
A wallet's Electrum connection is now its own, and no later client of the coin replaces it. Close it when the wallet is deleted.
The in-memory mock used to publish its client into ClientManager. It now overrides ElectrumXClient.openConnection, so setup, server.version and reuse follow the production path. The mock server answers server.version, and no header subscription request is expected.
- Delete client_manager_lifecycle_test: it covered the ClientManager registry and its header subscription, which no longer exist. - Failover: keep the same-owner drain cases; turn the cross-owner reuse and publication cases into isolation checks; drop the sharing test. - TLS: a client with the same approval opens its own connection; a revoked client never reuses a trusted one; updateNode() closes the wallet connection a revocation replaces. Drop the "revoke" and "close all" draining variants that relied on the registry. - Request policy: each client keeps its own deadline on its own connection. - Remaining files only lose ClientManager teardown and wording.
Two clients of the same coin and node get distinct connections, and one client's failover, request timeout or closeAdapter() never fails another client's in-flight request. Wallet chain height is read on each wallet's own connection without a cached value, and a clearnet-only node still fails it under Tor.
- The node change test now checks that loaded wallets of the coin are updated and refreshed, which is what closes their old connections. - The notification test no longer reads the registry, and a new case checks that a finished check closes its own connection.
- Register blockchain.headers, so the header notifications that getBlockHeadTip subscribes each connection to are accepted instead of printed as unknown methods on every block. - Notification checks use the wallet's own client instead of opening a new connection for every watched transaction. - An exited wallet releases its connection once its refresh ends. ElectrumXClient.disconnect() lets in-flight requests finish and keeps the client usable. - A cancelled or failed restore closes the client of the wallet it never added. - updateElectrumX replaces the client before closing the old one, so overlapping node updates close every client they create. - A node change refreshes a wallet again when its running refresh was using the replaced client.
Cancelling a restore deleted the wallet but left its client connected until recover() finished. Close it on cancel, so the restore stops its network work and fails before the wallet can be added.
Neither delete flow notified wallets, so loaded clients kept the deleted node's endpoint and its approved certificate. Run the node-change notifier after deletion, as every other node action does.
Connect did nothing, without feedback, when the node changed while it was being tested, and dropped any certificate approved in that test. It now shows a warning asking to try again. Desktop Connect also accepted further taps while a test was running, so a double-click opened two certificate dialogs. It now runs one test at a time.
The options sheet's Connect accepted further taps while a test was running. A double tap stacked certificate dialogs, and once the first run saved the node, the second showed the new "node changed" warning although Connect had worked. It now runs one test at a time, like the desktop card.
The full Spark used-tag set is one response of about 32 MiB, and the 2-minute deadline failed it on any link slower than about 2.2 Mbit/s. Each failure restarted the whole download on the next node, so Spark never synced on slow connections. Staging set no deadline for these requests.
A view-only restore that failed or was cancelled left its wallet's own Electrum connection open, because nothing else owns an unadded wallet. Cancel now closes it, and so does the end of any restore that did not add the wallet. A cancelled restore also no longer adds the deleted wallet when recovery finishes after the cancel.
julian-CStack
force-pushed
the
electrum-client-per-wallet-refactor
branch
from
November 12, 2026 15:03
4716c65 to
5be828b
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## staging #1510 +/- ##
===========================================
+ Coverage 14.08% 16.08% +2.00%
===========================================
Files 1139 1145 +6
Lines 114742 114390 -352
===========================================
+ Hits 16156 18401 +2245
+ Misses 98586 95989 -2597 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.