Repository navigation
Gate balance effects and FIO refresh on engine readiness (wallet cache v2) - #6080
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
00faa89 to
7f1149f
Compare
7f1149f to
0a089ca
Compare
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
0de615b to
5d1a879
Compare
5d1a879 to
99b80e3
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
99b80e3 to
595cb91
Compare
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
paullinator
left a comment
There was a problem hiding this comment.
Reviewed with edge-core-js#733. The three patches here are right, and I audited the remaining waitForAllWallets / waitForCurrencyWallet / otherMethods call sites (loan dashboard, ramp selection hook, action-queue display/push/evaluate, loan-manager, CreateWalletCompletionScene, resolveName, FioAddressUtils, Cardano/Thorchain adaptors, WalletConnect, migrate scenes, FioActions) — all config-only or engine-gated internally, so nothing else needs patching.
One change needed, in a file this PR does not touch yet:
A cached wallet whose engine fails still renders as a healthy row. With #733, CURRENCY_ENGINE_FAILED sets engineFailure and account.currencyWalletErrors[walletId], but the wallet object was already emitted from the cache, so it stays in account.currencyWallets. WalletListSwipeable.tsx:157 picks WalletListSwipeableCurrencyRow whenever wallet != null, so WalletListSwipeableLoadingRow — the only list component that shows currencyWalletErrors — is now unreachable for any cached wallet. The user sees cached balances with a sync ring that never completes, and taps reject with the engine error. On develop the wallet object never existed in this case, so the error row showed.
The row should stay (wallets appearing before their engines is the point of the cache), but an engine failure is rare and should not happen, so the user needs to be told. Suggest WalletListSwipeableCurrencyRow reads useWatch(account, 'currencyWalletErrors') and, when currencyWalletErrors[wallet.id] != null, shows the error in place of the sync ring, the way the loading row does today. BalanceCard.tsx:75, useAccountSyncRatio.tsx:60, and DeepLinkingManager.tsx:47 already consult the error map independently of the wallet object, so they are fine.
FYI for anyone extending the otherMethods probes: across the yaob bridge Object.keys(wallet.otherMethods) is [] because methods are non-enumerable (true on develop too). Existence checks must probe the property, as waitForWalletOtherMethods does.
|
Fixed the error-row finding. |
16c0b1b to
12fcb2a
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 12fcb2a. Configure here.
88cbb94 to
a0299b6
Compare
paullinator
left a comment
There was a problem hiding this comment.
Re-reviewed at a0299b6. The failed-engine row is fixed: WalletListCurrencyRow watches currencyWalletErrors and shows the error in the card's overlay slot ahead of paused/disabled, so the cached row and balances stay while the failure is visible. Same pattern BalanceCard and useAccountSyncRatio already use.
One cosmetic note, not blocking: the overlay prints the raw engineError.message, which can be long or technical for a card label. A localized "Engine failed" prefix would read better.
a0299b6 to
66d6106
Compare
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
|
Cosmetic note addressed in 66d6106. The overlay now renders a localized prefix instead of the bare message: new key The full jest suite is green on the branch (99 suites, 722 tests), and verify-repo passes on both repos. |
With the core's wallet cache (wallet cache v2 phase 1), wallet objects
exist before their engines load, and waitForAllWallets resolves in that
window. Three login-path surfaces consumed engine state immediately:
- The action queue's address-balance effect read balanceMap right after
awaiting the wallet, which could evaluate a balance effect against
cached, possibly stale balances. It now reports not-yet-effective
until the engine has fully synced, matching the conservatism the loan
flow already applies.
- The FIO address refresh called otherMethods on pre-engine wallets,
which is {} in that window. Services now waits for each FIO wallet's
engine-backed otherMethods (bounded by a generous safety-valve
timeout) before refreshing.
- FioService's periodic expired-domain check called
otherMethods.getFioAddresses the same way (caught live on the sim).
It now skips pre-engine wallets and lets the next 30s cycle retry,
which also avoids wedging its one-shot expiredChecking latch.
The core's new post-login queue staggers cached wallets' engine startup. withWallet covers every wallet-scoped scene, so opening one calls waitForCurrencyWallet, which moves that wallet's engine to the front of the queue.
66d6106 to
4aa4a8e
Compare

Technical Design Document
edge-wallet-cache-design.md
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
EdgeApp/edge-core-js#733
Requirements
If you have made any visual changes to the GUI. Make sure you have:
Description
Technical design doc
GUI-side patches for wallet cache v2 phase 1 (TDD section 7: pinned, live). With EdgeApp/edge-core-js#733, wallet objects exist before their engines load and
waitForAllWalletsresolves in that window, so the login-path surfaces that consumed engine state at resolve-time are gated on engine readiness:checkActionEffect.ts(address-balance): reported the effect againstbalanceMapimmediately after awaiting the wallet, which could now evaluate cached, possibly stale balances. It reports not-yet-effective until the engine has fully synced (same conservatism as the loan flow'swaitForLoanAccountSyncand the existing< 1treatment in spend paths, TDD 7.4), letting the action queue's normal 15s poll re-check.Services.tsx: the post-waitForAllWalletsFIO refreshes (refreshConnectedWallets,refreshAllFioAddresses) callwallet.otherMethods.*, which the core guarantees is{}pre-engine. A newwaitForWalletOtherMethodsutil watchesotherMethodsuntil the engine's methods land (10-minute safety-valve timeout, roughly matching how longwaitForAllWalletscould already take on large accounts before the cache existed).FioService.ts: the periodic expired-domain check callsotherMethods.getFioAddressesthe same way. This one was NOT in the TDD's section-7 audit; it was caught live on the simulator (red dev alertwallet.otherMethods.getFioAddresses is not a functionseconds after a warm cached login). Being a 30s periodic task, it skips pre-engine wallets and lets the next cycle retry, which also avoids wedging its one-shotexpiredCheckinglatch when no wallet is ready yet.Remaining
otherMethodscall sites (FIO scenes, staking, WalletConnect) are user-navigation surfaces audited in the TDD as safe (null-probes, or flows that imply an engine exists) and are unchanged.Tested on the iOS simulator against the linked core build (edge-funds, 194 wallets): cold login wrote all 194
walletCache.jsonfiles; warm relaunch rendered the full wallet list with names and balances from the cache while engines were still loading; no FIO alert through 140s of runtime; drilling into a wallet shows live engine-backed data on the same wallet object. Screenshots attached below.Phase 2: tap-prioritization
The core now staggers cached wallets' engine startup through a limited-concurrency queue (EdgeApp/edge-core-js#733 phase 2).
withWalletwraps every wallet-scoped scene, so opening one callsaccount.waitForCurrencyWallet(walletId), which moves that wallet's engine startup to the front of the queue. The call is a fire-and-forget hint; a deleted or broken wallet is already handled by the existing goBack effect.Phase 6: provisional receive address (TDD section 7.5, decision 9.9)
The receive scene (
RequestScene.tsx) waited on the engine for every address, so a rotating-address chain showed a loading state until the engine started. It now opts into the core's cached address (getAddresses({ allowCached: true }), EdgeApp/edge-core-js#733 phase 6) and renders it immediately.!hasStableAddresses), the cached address is provisional: a static inline affordance sits under the address (a muted circled-i glyph, "Checking for your latest address", and a spinner; informational, not tappable, not a warning color). A 350ms grace timer gates it on, so a warm engine that confirms first never flashes it; the QR is capped to reserve the row's height so toggling never reflows it.withWalletinstance is reused across wallet switches) or after unmount is ignored rather than overwriting the current address. A later refresh /addressChangedrotation takes the plain engine-gated path, so it never shows a stale address.Rotating-chain behavior is provable in-app on a UTXO chain (BTC/LTC) with no plugin change; the stable-chain skip and the core gating are covered by unit tests. Depends on the core
allowCachedoption (EdgeApp/edge-core-js#733) and, for stable chains, the plugin flags (draft EdgeApp/edge-currency-accountbased#1076).TDD (pinned, live): implementation divergences and the decisions are documented inline in the affected sections.
Asana: https://app.asana.com/1/9976422036640/project/1213843652804305/task/1216673467164267
Post-review followup: the FIO refreshes read a populated wallet list
On a warm login the readiness wait in
Services.tsxresolves beforeFioService's watch-driven effect has publishedui.fio.fioWallets, sorefreshConnectedWalletsandrefreshAllFioAddressescould run against an empty list.Services.tsxnow dispatchesUPDATE_FIO_WALLETSwith the list it already computed, right before the refreshes;FioService's own dispatch stays and carries the same content.Phase 7: the provisional receive affordance is reverted
The commit that added the provisional address UI ("Show a provisional receive address on warm login") was dropped from this branch, along with the core
allowCachedopt-in and thehasStableAddressesplugin flags (edge-currency-accountbased#1076, closed). Serving a cached address silently was accepted as an edge case, which removes what the affordance existed to disclose. The commit was dropped rather than reverted on top, so this branch's history never contains it.RequestScene.tsxneeds no replacement code. It callsgetAddresseson mount and already subscribes toaddressChanged, which is exactly the contract the core now provides: the first query is answered from the cache so the QR renders immediately on a warm login, and the wallet emitsaddressChangedif the engine goes on to derive a different address, so the scene re-queries and lands on it. That reconcile is in edge-core-js#733, and it fixes every consumer of the address rather than this one scene.Note
Medium Risk
Changes login-time FIO refresh ordering, action-queue balance triggers, and wallet startup prioritization—behavior that can affect large accounts and automated flows, though failures are mostly deferred or logged rather than silent wrong outcomes.
Overview
Adapts the GUI for wallet cache v2, where wallet objects appear before their engines finish loading.
Engine readiness gates: Post-login FIO work (
refreshConnectedWallets,refreshAllFioAddresses) and the periodic expired-domain check inFioServiceno longer callotherMethodson cold wallets. A newwaitForWalletOtherMethodshelper waits for engine-backed methods (with a long timeout), andFioServiceskips wallets untilgetFioAddressesexists, with afinallyso the expired-check latch cannot stick. Action queueaddress-balanceeffects treat cached balances as not effective untilsyncStatus.totalRatioreachesDONE_THRESHOLD, then re-poll on the existing 15s delay.Warm-login fix:
ServicesdispatchesUPDATE_FIO_WALLETSbefore the FIO refreshes so an empty Redux list does not run refreshes ahead ofFioService's watch effect.Tap prioritization:
withWalletfire-and-forgetsaccount.waitForCurrencyWallet(walletId)so opening a wallet-scoped scene moves that engine to the front of the post-login queue.UX: Wallet list rows show an Engine Failed overlay from
currencyWalletErrorswhen a cache-seeded wallet's engine fails. ESLint no longer exemptsFioServiceafter typing it asReact.FC.Reviewed by Cursor Bugbot for commit 4aa4a8e. Bugbot is set up for automated code reviews on this repo. Configure here.
Test evidence
b96a03bGate balance effects and FIO refresh on engine readiness
🪓 forced engineError to a non-null Error in WalletListCurrencyRow, since an engine failure has no natural trigger on the sim; reverted, tree clean.
agent proof 1216673467164267 01 cold login wallet list
agent proof 1216673467164267 02 warm login cached wallet list
agent proof 1216673467164267 03 wallet detail engine loaded
agent proof 1216673467164267 04 warm login after review fixes
warm login wallet list
🪓 engine error row
88cbb94Prioritize an opened wallet's engine startup
agent proof 1216673467164267 05 warm login list
agent proof 1216673467164267 06 sepolia detail after tap
agent proof 1216673467164267 p6 01 ltc receive cached
agent proof 1216673467164267 p6 02 ltc receive warmlogin
🪓 🩹 HACK-FORCED: provisional affordance
receive address no affordance
warm login wallet list