fix(wallet): purge stale account managers when the device seed changes underneath them#239
Merged
Merged
Conversation
…s underneath them Recurring customer bug: the UI showed one wallet's addresses/balances while the device OLED showed another's (field report: UI 0x4C1922..., device 0x27de622c...). The in-memory managers (btcAccounts/evmAddresses) are kept across disconnect for watch-only and only re-derive when empty; the resets that should empty them are event-INFERRED and every trigger has a blind spot: - needs_passphrase reset: skipped on reconnect with a pre-cached passphrase (device goes straight to ready) - device-switch reset: skipped when deviceId is unchanged - seed-changed: skipped on hidden->standard transitions — hidden wallets never persist seed_eth_<id> (privacy), so the stored identity is the standard wallet's and MATCHES once the device returns to it; no event fires - passphrase toggle: applySettings+clearSession resets the engine's fingerprint/identity but never the managers Fix checks the RESULT instead of the cause. The seed-identity address (ETH m/44'/60'/0'/0/0) is bit-for-bit evmAddressPath(0), so the EVM manager's index-0 address must equal the device-derived identity. One invariant covers every cause, including future ones. Two legs: - getBalances: derive the identity FRESH from the device (new RAM-only engine.deriveSeedIdentity) on every fetch and reconcile before the init guards — every balance fetch is now self-verifying against the device - state-change(ready): reconcile against engine.currentSeedEthAddress once checkSeedIdentity / hidden-scope derive / reconnect probe classify the session (probe's confirmed-hidden path now re-emits state-change so the guard actually runs there) On purge: reset both managers (init guards re-derive), push empty account sets, wipe the deviceId-scoped DB cache (standard sessions only — a stale fetch classified as standard may have already persisted the WRONG wallet's rows; hidden sessions never persist and their rows must not be touched), and notify the frontend via new 'wallet-data-purged' message. Dashboard clears the displayed balances immediately and force-refreshes (in-flight fetches self-heal — the purge runs at their start). Never purges on uncertainty: unknown identity or uninitialized managers no-op, so cold start and watch-only keep their data. evmAddressPath moved to shared/chains.ts (pure, test-importable — src/bun pulls ./db -> electrobun import side effects that break bun test) and re-exported from evm-addresses for existing importers. Tests: __tests__/seed-reconcile.test.ts pins the path equivalence the invariant rests on, the never-purge-on-uncertainty rule, both transition directions, and the exact field-report scenario. Full suite 365 pass / 2 pre-existing environmental failures (live :1646 vault, live Solana RPC) — identical on baseline.
…sumers Review follow-up to the stale-wallet purge. Three gaps where stale account managers could still be read before a full getBalances ran: 1. getBalance (single-chain) and buildTx read btcAccounts/evmAddresses with no reconciliation. getBalance backs AssetPage / tx-push / post-send refreshes; buildTx is the SIGNING path — a stale xpub/evmAddressIndex would build a tx against the wrong wallet's context. Added a shared ensureManagersForSeed() boundary (derive seed identity fresh from device → reconcile → purge if stale) and call it at every consumer: getBalances, getBalance, getBtcAccounts, getEvmAddresses, and buildTx. buildTx THROWS on purge so the half-prepared send is abandoned and the user re-initiates against refreshed balances. 2. The DB-cleanup on purge was gated on !isPassphraseWallet, so during the conservative cached-passphrase reconnect window it was skipped — and when the probe later reclassified to standard the managers already matched, so no second purge ever cleared the stale rows. Clearing only REMOVES rows (never writes hidden data), so it's privacy-safe: run it unconditionally. 3. reconcileSeedManagers only compared the EVM index-0 address, so a BTC-only initialized stale state (getBtcAccounts initializes BTC independently) had no proof to compare and survived. Added a seed-owner STAMP (managersSeedOwner): the seed the managers were derived under, set after every init and checked alongside the EVM index-0 proof. Either mismatch purges. All reset paths now go through resetSeedManagers() so the stamp stays in lock-step. The engine reconnect probe's confirmed-hidden branch already re-emits state-change (prior commit) so the ready-handler leg re-runs there too. Tests: seed-reconcile.test.ts adds the seed-owner-stamp (BTC-only) cases. Project suite 344 pass / 2 pre-existing env failures (live :1646, live Solana RPC). Differential tsc clean; bun + vite build clean.
This was referenced Jun 12, 2026
Merged
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
1 participant
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.
Summary
Recurring customer bug: the UI shows one wallet's addresses/balances while the device OLED shows another's (field report: UI
0x4C1922…, device0x27de622c…; BTC balance missing after removing passphrase + reconnect).The in-memory account managers (
btcAccounts/evmAddresses) are deliberately kept across disconnect for watch-only and only re-derive when empty. The resets that should empty them infer seed changes from events, and every trigger has a blind spot:needs_passphraseresetready)seed-changedseed_eth_<id>(privacy), so the stored identity is the standard wallet's and matches once the device returns to itapplySettings+clearSessionresets the engine's fingerprint/identity but never the managersFix: check the result, not the cause
The seed-identity address (ETH
m/44'/60'/0'/0/0) is bit-for-bitevmAddressPath(0), so the EVM manager's index-0 address must equal the device-derived identity. One invariant covers every cause, including future ones. Two legs:getBalancesderives the identity fresh from the device (new RAM-onlyengine.deriveSeedIdentity()) on every fetch and reconciles before the init guards — every balance fetch is now self-verifying against the device.state-change(ready)reconciles againstengine.currentSeedEthAddressonce the session is classified (the reconnect probe's confirmed-hidden path now re-emitsstate-changeso the guard runs there too).On purge: reset both managers (init guards re-derive), push empty account sets, wipe the deviceId-scoped DB cache (standard sessions only — a stale fetch classified as standard may have already persisted the wrong wallet's rows; hidden-session rows are never touched), and notify the frontend via new
wallet-data-purgedmessage. Dashboard clears displayed balances immediately and force-refreshes — "removing passphrase now triggers a rescan".Never purges on uncertainty: unknown identity or uninitialized managers no-op, so cold start and watch-only keep their data.
evmAddressPathmoved toshared/chains.ts(pure, test-importable —src/bunpulls./db→ electrobun import side effects that breakbun test) and re-exported fromevm-addresses.tsfor existing importers.Test plan
__tests__/seed-reconcile.test.tspins: the path equivalence the invariant rests on, never-purge-on-uncertainty, both transition directions, the exact field-report scenario, and watch-only continuity on same-seed reconnectbun test: 365 pass / 2 fail — both failures pre-existing & environmental (live vault at :1646, live Solana RPC), identical on baselinetsc: no new genuine errors (only the samerpc.sendfalse-positive class every existing call produces outside the real build)bun build(bun side) +vite build(frontend) both clean