-
Notifications
You must be signed in to change notification settings - Fork 56
Cache and restore wallets #703
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
8b8cd1a to
53a3312
Compare
| currencyConfigs: EdgePluginMap<EdgeCurrencyConfig> | ||
| currencyWallets: { [walletId: string]: EdgeCurrencyWallet } | ||
| activeWalletIds: string[] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused currencyConfigs in WalletCacheSetup return value
Low Severity
The currencyConfigs property is built and returned in WalletCacheSetup, but no consumer ever accesses cacheSetup.currencyConfigs. The configs are used internally when creating cached wallets, but the exported property is dead code. Either remove it from the interface and return value, or document why it's exposed for future use.
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
Or push these changes by commenting: Preview (48c5071755)diff --git a/src/core/cache/cache-wallet-loader.ts b/src/core/cache/cache-wallet-loader.ts
--- a/src/core/cache/cache-wallet-loader.ts
+++ b/src/core/cache/cache-wallet-loader.ts
@@ -23,7 +23,6 @@
const WALLET_ID_DISPLAY_LENGTH = 8
export interface WalletCacheSetup {
- currencyConfigs: EdgePluginMap<EdgeCurrencyConfig>
currencyWallets: { [walletId: string]: EdgeCurrencyWallet }
activeWalletIds: string[]
}
@@ -48,7 +47,7 @@
* @param jsonData The raw JSON string containing wallet cache data
* @param currencyInfos Map of pluginId to EdgeCurrencyInfo for available plugins
* @param options Optional configuration for cached wallet creation
- * @returns Setup data for cached wallets including configs, wallets, and active IDs
+ * @returns Setup data for cached wallets including wallets and active IDs
*/
export function loadWalletCache(
jsonData: string,
@@ -144,7 +143,6 @@
)
return {
- currencyConfigs,
currencyWallets,
activeWalletIds
} |
53a3312 to
520c5ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is ON. A Cloud Agent has been kicked off to fix the reported issues.
| for (const pluginId of Object.keys(state.plugins.currency)) { | ||
| currencyInfos[pluginId] = | ||
| state.plugins.currency[pluginId].currencyInfo | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale state snapshot used after await in cache loader
Medium Severity
The state variable is destructured from input.props at line 85, before await waitForPlugins(ai) at line 172. Inside tryLoadCache, state.plugins.currency is read at line 120 to build currencyInfos. If plugins finish loading during the await, the captured state is stale and state.plugins.currency could be empty, producing zero cached wallets and silently defeating the caching feature. The rest of the codebase (e.g., makeAccountApi at line 103) correctly uses ai.props.state.plugins.currency for live state. The tryLoadCache function needs to do the same.
Additional Locations (1)
| result[walletId] = wallet | ||
| } | ||
| } | ||
| return result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getter always allocates new object reducing bridge efficiency
Low Severity
When cacheSetup is non-null (for the lifetime of the account after a cache-first login), the currencyWallets getter always allocates a new result object on every access. The watcher pixie calls update(accountApi) on every Redux state change, causing yaob to re-read this getter, see a new object reference, and push the full wallet map over the bridge—even when the underlying wallet set hasn't changed. Previously the getter returned a stable reference from the pixie output.
|
Bugbot Autofix prepared fixes for 2 of the 2 bugs found in the latest run.
Or push these changes by commenting: Preview (d9c45c40da)diff --git a/src/core/account/account-api.ts b/src/core/account/account-api.ts
--- a/src/core/account/account-api.ts
+++ b/src/core/account/account-api.ts
@@ -632,13 +632,21 @@
const pixieWallets =
ai.props.output.accounts[accountId]?.currencyWallets ?? {}
- // If no cache, just return pixie wallets
+ // If no cache, just return pixie wallets (stable reference)
if (cacheSetup == null) {
return pixieWallets
}
+ // Check if all active wallets are now in pixieWallets.
+ // If so, return the stable pixieWallets reference to avoid
+ // allocating a new object on every getter access.
+ const activeIds = this.activeWalletIds
+ const allWalletsLoaded = activeIds.every(id => pixieWallets[id] != null)
+ if (allWalletsLoaded) {
+ return pixieWallets
+ }
+
// Merge: real wallets take priority, cached fill gaps
- const activeIds = this.activeWalletIds
const result: { [walletId: string]: EdgeCurrencyWallet } = {}
for (const walletId of activeIds) {
// Prefer real wallet, fall back to cached
diff --git a/src/core/account/account-pixie.ts b/src/core/account/account-pixie.ts
--- a/src/core/account/account-pixie.ts
+++ b/src/core/account/account-pixie.ts
@@ -82,7 +82,7 @@
async update() {
const ai = toApiInput(input)
- const { accountId, accountState, log, state } = input.props
+ const { accountId, accountState, log } = input.props
const { accountWalletInfos } = accountState
async function loadAllFiles(): Promise<void> {
@@ -114,12 +114,14 @@
const cacheJson = await ai.props.io.disklet.getText(cachePath)
// Build currency info map from loaded plugins:
+ // Use ai.props.state for live state (not the captured `state` snapshot)
+ // since plugins may have finished loading during earlier awaits.
+ const currencyPlugins = ai.props.state.plugins.currency
const currencyInfos: {
[pluginId: string]: import('../../types/types').EdgeCurrencyInfo
} = {}
- for (const pluginId of Object.keys(state.plugins.currency)) {
- currencyInfos[pluginId] =
- state.plugins.currency[pluginId].currencyInfo
+ for (const pluginId of Object.keys(currencyPlugins)) {
+ currencyInfos[pluginId] = currencyPlugins[pluginId].currencyInfo
}
// Create cached wallets with real wallet lookup for delegation: |
Add missing return in fake plugin getBalance for token balance. Improve @ts-expect-error comment and log swap quote close errors. Co-authored-by: Cursor <[email protected]>
Improve login performance by caching wallet state on a per account level in unencrypted JSON file and rehydrate before wallets load.
fixup! Cache and restore wallets Co-authored-by: Cursor <[email protected]>
520c5ad to
2bfc435
Compare



Improve login performance by caching wallet state on a per account level in unencrypted JSON file and rehydrate before wallets load.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
noneNote
Medium Risk
Touches login/account initialization and wallet exposure paths, adding async cache load/save and delegation logic that could affect wallet lifecycle behavior if edge cases slip through; mitigated by extensive new end-to-end tests.
Overview
Adds cache-first login by saving wallet state to
accountCache/<storageWalletId>/walletCache.jsonand rehydrating lightweightEdgeCurrencyWallet/EdgeCurrencyConfigobjects before engines load, then delegating to real objects once available.Updates
account-pixieto attempt cache load after plugins are ready (falling back to normal login on missing/invalid cache) and introduces a throttledcacheSaverpixie to persist wallet/token state reactively. Updatesaccount-apito acceptcacheSetup, return cachedactiveWalletIds/currencyWalletsuntil keys load, and merge cached + real wallets.Introduces new cache modules for schema validation, shared real-object polling, delegating
otherMethodsand disklets, and adds extensive end-to-end tests (plus fake plugin controls) to verify instant availability, delegation behavior, yaob update propagation for setters, and disklet/otherMethods waiting semantics. Also improves swap quote cleanup logging and fixes a missingreturnin the fake engine token balance path.Written by Cursor Bugbot for commit 520c5ad. This will update automatically on new commits. Configure here.