Repository navigation
Electrum client per wallet refactor - #1510
Draft
julian-CStack wants to merge 16 commits into
Draft
julian-CStack wants to merge 16 commits into
julian-CStack wants to merge 16 commits into
Conversation
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.
Implement SignVerifyInterface on LibMoneroWallet so Monero wallets get the existing Sign / Verify view. cs_monero 4.0.0 already wraps MONERO_Wallet_signMessage and MONERO_Wallet_verifySignedMessage, so this only forwards them through CsMoneroInterface. Signatures are wallet2 SigV2 spend-key signatures for the chosen primary address or subaddress, compatible with monero-wallet-cli/rpc sign and verify. wallet2 returns an empty string instead of throwing when signing fails, so treat that as an error. View-only wallets cannot sign.
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.