Repository navigation
Wallet cache v2 phase 1: emit currency wallets before their engines exist - #733
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
fd70e56 to
0fc4524
Compare
0fc4524 to
ed985f4
Compare
f615690 to
4783de3
Compare
|
Phase 2 test evidence: live in-app verification (iOS sim, edge-funds, 194 wallets, warm login) Captured the core's log stream during a warm login with verbose logging on. Three behaviors verified live:
Phase 1 re-verified on the same run: the wallet list rendered names and balances from |
|
=== Phase 3 in-app evidence: edge-funds (194 wallets), iOS sim, phase-3 core bundle === --- 1. Cold boot (fresh install, no accountCache.json): master-identical sequence --- --- 2. Fresh-process warm relaunch (accountCache.json present): account emits from cache before the deferred loads --- --- 3. Warm PIN login with verbose logging: bulk-seeded wallets enter the startup queue before the loads land --- --- 4. Queue drain: 174 cached wallets at concurrency 8 (cold monero/zano-family wallets bypass) --- --- 5. accountCache.json shape on the sim (no plugin settings; privacy fix) --- |
|
=== Phase 4 in-app evidence (edge-funds, 194 wallets, iOS sim, core webview bundle @ phase-4 HEAD) === --- Cold boot (fresh app data restored from pool image; master-identical ordering, no cache emit) --- --- Warm relaunch (account emits from cache before repo sync/file loads) --- --- Enabled-token toggle round trip through the new set-diff path (L3USD on My Fantom) --- --- Final warm relaunch (token persisted through a cache-seeded boot) --- |
|
=== Phase 5 in-app evidence (edge-funds, 194 wallets, iOS sim, core webview bundle @ phase-5 HEAD) === --- Warm boot on phase-5 core (account emits from cache; wallet list renders pre-engine) --- --- Receive/QR scene (My Fantom / L3USD): address renders, and the engine's answer lands in the cache --- --- otherMethods stub retirement: the FioActions warm-boot TypeError is gone --- --- Final warm relaunch (cache emit, clean boot) --- |
c532ebe to
733c172
Compare
|
Approval noted, no code change for this round. Your correction on the sticky dirty flags is the accurate one: that thread reply predates the write-generation scheme, which landed in a later fixup and is what the branch ships ( |
28c09f5 to
a7068d5
Compare
Answer the Cursor Bugbot finding on PR #733: a logout destroys the pixie under the pending engine wait, so the background address reconcile rejected and reported a shutdown or wallet-missing error to the app. Swallow both, since the correction is moot once the wallet is gone. Fixup-for: auto
a7068d5 to
a1650f3
Compare
Answer the Cursor Bugbot finding on PR #733: a logout destroys the pixie under the pending engine wait, so the background address reconcile rejected and reported a shutdown or wallet-missing error to the app. Swallow both, since the correction is moot once the wallet is gone. Fixup-for: auto
a1650f3 to
4da4bc4
Compare
Answer the Cursor Bugbot finding on PR #733: a logout destroys the pixie under the pending engine wait, so the background address reconcile rejected and reported a shutdown or wallet-missing error to the app. Swallow both, since the correction is moot once the wallet is gone. Route the engine callbacks and the staking query's rejection through the destroy-guarded input, so a report that lands between a logout and the end of killEngine cannot write into the next login's wallet or raise an error for a wallet the user has left. Fixup-for: auto
7b68565 to
5451bec
Compare
5451bec to
0b281e5
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0b281e5. Configure here.
0b281e5 to
af1b807
Compare
The design spans two repos, seven phases, and a file-layout reversal, and the reasoning behind each is not recoverable from the diff. The doc ships with the code so it is reviewed alongside it and stays current instead of drifting in a gist.
A warm login needs the account's boot state and every wallet's UI state on disk in a form that survives a schema bump and a kill part-way through a write. The file carries wallet states, custom tokens, and one entry per wallet holding its name, fiat code, enabled tokens, balances, addresses, public keys, and otherMethods names. Reads accept older versions by upgrading in place. The disklet exposes no rename on either platform and Android truncates the target, so generations alternate between two slots and the reader takes the newest that still parses. The per-wallet walletCache.json reader stays for devices on the old layout.
Once the wallet list renders from cache, every wallet's engine work (repo sync, key derivation, makeCurrencyEngine) still races in the seconds after login, which is where the post-login jank lives. The scheduler admits 8 wallets at a time and lets a caller move one to the front, so a wallet the user actually opened does not wait behind the whole queue. A wedged wallet cannot starve the pool: a watchdog force-releases its slot and temporarily over-admits, which degrades to the unbounded behavior this replaces. Priority bumps carry a TTL, since post-startup engine calls bump constantly and would otherwise mark every wallet as asked-for by the next login.
The seeded state is a guess that the file loads overwrite moments later, and a user change made inside that window would otherwise be reverted by a load that read the file before it. Two seeding actions, one for the account and one carrying every wallet, populate the same reducers the file loads populate, so there is no shadow copy of wallet state. The loads then run deferred, marked fromFile so the reducers can tell an authoritative value from a user change, and each racing field merges rather than replacing: custom tokens per token id, enabled tokens per toggled id, plugin settings per plugin id, wallet states per wallet id. A terminal load failure rejects the waiters instead of leaving them pending forever, and balanceMap keeps its identity when an engine re-reports an unchanged balance, so yaob sees no phantom update.
Reading each wallet's cache from its own pixie costs two dispatches per wallet, so a 194-wallet account paid ~400 store transits before the list could render, and every pixie and watcher re-evaluated against each one. The loader reads the consolidated file once and, for a device still on the per-wallet layout, falls back to reading those files concurrently. Either way it produces one seed per wallet for a single batched dispatch. This is batching, not notification suppression: every consumer is still notified once, with the final state.
The account now emits before its repo is created, so every surface that assumed a live repo could throw during the window the cache exists to make usable. Disklets resolve lazily or fall back to disklets built straight from the keys, which are the same files under the same encryption. Repo- backed calls pend rather than throw: changeWalletStates waits for the repo, sync waits for the storage wallet, plugin-settings writes wait for their load, and changeEnabledTokenIds waits for the plugin's builtin definitions. The account-level engine methods wait through the shared selector and read the wallet list after the wait, so wallets that arrived during it are included. Repo syncs serialize per storage wallet, so two concurrent syncs cannot interleave.
The wallet API refused to emit until its engine existed, so the GUI could not render a name, a fiat code, or a balance until every wallet's engine had loaded, which is the whole of the login delay. The gate now opens on the cached state, and the engine becomes a dependency the API awaits internally: engine-backed methods wait and reject if the engine fails or the wallet is deleted, while repo- writing methods gate on the storage wallet instead, which lands much earlier. otherMethods is a permanent object of delegating stubs built from the cached names, so its identity survives the engine landing. A rotating chain serves its cached receive address immediately and re-asks the engine in the background, emitting addressChanged when the two differ. Without a cache the gate opens on the same conditions as before, so first login is unchanged.
The account itself was the other half of the delay: nothing emitted until plugins loaded, the account repos synced, and every key file was read, and wallet pixies could not even start until that chain finished. The boot now seeds from the cache right after the plugins load and emits, running the repo sync and file loads deferred behind it. Writes go the other way: one serialized throttled saver owns the whole account, so a sync window where 194 engines all report balances costs one write instead of 194, and generations alternate between the file's two slots so an interrupted write costs one generation of staleness. Each write logs its generation, wallet count, and duration, since the write is this design's whole cost and nothing else reports it.
The fake server hands out hash-suffixed store URLs but only routed the bare form, so a repo's second sync inside makeFakeEdgeWorld 404'd. Two-device tests need that second sync to diverge the repo from the cache at all.
Every claim here is a timing claim, so the suites drive the ordering directly rather than sleeping: the fake plugin gets a test-controlled engine gate, the savers get a 50 ms throttle, and the fake world is relaunched from disk to prove a warm boot reads only what is on it. Coverage runs from cold-start equivalence and cached emission through engine-gated calls completing, failing, and being deleted mid-wait, the concurrency queue draining with front-of-queue bumps, the four two-device races the write-path audit found, the address and otherMethods caches, and a torn slot degrading to the older generation. A cache-coverage test asserts every EdgeCurrencyWallet property is classified, so a new property forces a caching decision.
af1b807 to
1c2e1c2
Compare
3 participants
Technical Design Document
edge-wallet-cache-design.md
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
none
Description
Login is slow because the GUI cannot render a wallet list until currency engines exist, and nothing about a wallet survives a logout. This branch caches what the list renders and moves everything else behind it: a warm login reads one file, seeds Redux, and emits the account and every wallet before the account repo syncs or any engine starts.
The design doc ships on this branch at
src/docs/edge-wallet-cache-design.mdand is the place to start: it carries the decisions, the rejected alternatives, the boot-outcome matrix, and a retrospective on where the design was wrong. Supersedes #703, which proved the ~5x login win but was rejected on architecture (a parallelEdgeCurrencyWalletimplementation with a delegation and polling layer). One wallet implementation here, no mirror objects.The ten commits are meant to be read in order. What each one carries:
accountCache.jsonand its cleaners. One file on the account's local disklet holds the account boot state plus, per wallet, name, fiat code, enabled tokens, last-known balances, receive addresses,otherMethodsnames, and the public keys that used to sit in each wallet'spublicKey.json. Reads upgrade older versions in place. The disklet exposes no rename on either platform and Android truncates the target, so generations alternate between two slots and the reader takes the newest that still parses.fromFile, and each field that can race an in-window user change merges rather than replaces: custom tokens per token id, enabled tokens per toggled id, plugin settings per plugin id, wallet states per wallet id.balanceMapkeeps its identity when an engine re-reports an unchanged balance.walletApigate drops itsengine != nullcondition and the engine becomes a dependency the API awaits internally; repo-writing methods gate on the storage wallet instead, which lands much earlier.otherMethodsis an object of delegating stubs built from cached names, keeping its identity while the known name set is unchanged. A rotating chain serves its cached receive address immediately, re-asks the engine in the background, and emitsaddressChangedwhen the two differ.makeFakeEdgeWorld404s and the two-device tests cannot diverge the repo from the cache at all.Semantic shift worth flagging in review:
waitForCurrencyWalletandwaitForAllWalletsnow resolve when the wallet object exists, which can be before its engine loads. Internal core callers want the object and are unaffected; the GUI call sites that consumed engine state at resolve time are patched in the companion PR, EdgeApp/edge-react-gui#6080.Cold start is unchanged. With no cache the gate opens on exactly the conditions master uses, guarded by a regression test from the first commit.
Measured on hardware (Galaxy S9, release builds,
edge-funds, 146 to 156 currency wallets; method and caveats in TDD section 8.4):Payload size is not what a write pays for: writes carrying no wallets took up to 11.4 s during the busy first-login window while a full 89.6 KiB write took 76 ms once it quieted, so the write-amplification tradeoff this design accepted is real in bytes and close to irrelevant in time. Booting with the newest slot truncated still emits from cache off the older slot with no crash, and the next write repairs the damaged slot.
Functional behavior was verified in-app on the iOS simulator across the branch's development, with evidence on this PR (1, 2, 3) and screenshots on #6080: the wallet list renders from cache before any engine exists, wallets drain through the queue with tap-to-front prioritization taking effect mid-drain, an enabled-token round trip persists through a cache-seeded relaunch, and the FioActions warm-boot TypeError (17 per session) dropped to zero.
Post-review followups
The three follow-ups the post-review runs recorded, plus the reviewer-bot findings on the pushed heads (design doc section 6.7):
loadGen; the reducer applies a loaded value only when the generations match. A load that began before the change is dropped, a load that began after it applies, so a rename pulled from another device lands without waiting for the sync server to echo the local write. Test:a rename from another device applies after a local rename.info, withwarnonly for a write slower than 5 s (accountCacheSaverConfig.slowWriteMs). The throttle bounds the line per window for the whole session, so the default level cannot carry every write.saveTxActionandsaveTxMetadatastay engine-gated on purpose: the write needs the transaction in Redux, and every reducer case that fills that map is an engine action, so a file-scan gate would turn a pending call into a "missing tx" error.destroyreleases its engine-startup slot, so a logout mid-startup no longer makes the next login queue behind the old session's awaits; the startup catch returns when the pixie is destroyed, so a failure that lands after logout is not surfaced as a wallet error; and the per-session boot-file memo remembers a miss, so a cold login goes straight to the per-wallet files instead of re-reading both empty slots per wallet. Tests:logout releases in-flight startup slots for the next login,logout during engine startup swallows a late startup failure.a fallback read that outlives its logout does not seed the next session,a wallet created after boot seeds from the cache when unarchived.Asana: Login Perf - Wallet Cache v2
Note
High Risk
Large changes to login ordering, wallet/engine lifecycle, and API timing (including waitForCurrencyWallet semantics) with intentionally stale cached UI data until authoritative loads complete.
Overview
Warm logins now read a consolidated
accountCache.json(dual-slot generations for safe writes), seed Redux, and emit the account and wallet APIs before the synced repo finishes or any currency engine starts; repo sync and authoritative file loads run deferred with retries.Cached wallets start engines through a limited-concurrency queue (with user-opened wallets bumped to the front). Wallet surfaces can show cached balances, receive addresses, and
otherMethodsstubs while engines load;getAddressesmay serve cache first and emitaddressChangedif the engine disagrees.waitForCurrencyWallet/waitForAllWalletsresolve when the wallet object exists, not necessarily when its engine is ready—engine-backed account and wallet methodswaitForCurrencyEngineinstead of failing early.Boot-window races are handled with per-field merge rules (custom tokens, enabled tokens, plugin/swap settings, wallet states) and generation counters for name/fiat/settings loads; plugin settings writes merge into on-disk files under a per-account queue, and one throttled account cache saver persists boot state. Supporting fixes include serialized storage syncs, lazy repo disklets for cache-seeded APIs, and a fake-server store route that accepts hash-suffixed paths.
Reviewed by Cursor Bugbot for commit 1c2e1c2. Bugbot is set up for automated code reviews on this repo. Configure here.
Test evidence
49e5a1fDocument the wallet cache v2 design
agent proof 1216673467164267 p3 03 warm fixed
agent proof 1216673467164267 p3 04 warm pin list
agent proof 1216673467164267 p3 05 final warm
agent proof 1216673467164267 p4 01 cold boot
agent proof 1216673467164267 p4 02 warm wallets
agent proof 1216673467164267 p4 03 warm final
agent proof 1216673467164267 p5 01 warm list
agent proof 1216673467164267 p5 02 receive
agent proof 1216673467164267 p5 03 warm final
warm login wallet list
receive address no affordance
warm login wallet list
warm login after bot fixes
warm login before create
probe wallet created
probe wallet archived
probe wallet restored from cache
probe wallet rearchived
27f9d94fixup! Emit currency wallets before their engines exist
🪓 Uncommitted 240 s snooze before loadFiatFile in the wallet pixie startup block, so a logout could land inside the window; reverted
🪓 warm login list
🪓 logout during startup
🪓 relogin list
🪓 relogin list after stale loads
2450895Accept hash-suffixed routes in the fake server
🪓 removed withWallet's waitForCurrencyWallet priority bump locally so the opened ETC wallet stayed in the engine queue while DAI was enabled; reverted.
logged out mid startup
relogin after mid startup logout
🪓 dai enabled while etc wallet still queued
🪓 dai row from cache after relaunch