Conversation
…ctions Certain legacy functionality needs to rely on the tokens being present. They can use the local json if necessary
0225b98 to
c5a4453
Compare
c5a4453 to
7613aff
Compare
src/types/types.ts
Outdated
| canReplaceByFee?: boolean // Defaults to false | ||
| customFeeTemplate?: EdgeObjectTemplate // Indicates custom fee support | ||
| customTokenTemplate?: EdgeObjectTemplate // Indicates custom token support | ||
| customTokenTemplate?: EdgeObjectTemplate // An empty array indicates token support. A filled array indicates custom token support. |
There was a problem hiding this comment.
I don't like this. It's too subtle, and makes the code hard to understand. Let's add an explicit hasTokens?: boolean instead, and leave customTokenTemplate just for custom token support.
src/core/account/custom-tokens.ts
Outdated
| const enabledTokensByPlugin = new Map<string, Set<string>>() | ||
|
|
||
| // Get all wallet IDs for this account | ||
| const walletIds = accountState.currencyWalletIds |
There was a problem hiding this comment.
I am deeply concerned about when this migration happens. The problem is that the list of wallets may be out of date on the device, and then once the data sync takes place, the newly-discovered wallets will not receive the migration. Instead, I would expect this migration to happen as part of the loadTokensFile function, which would use your giant JSON to upgrade currencyCodes. Then, once loadTokensFile returns, the engine pixie can do a diff with the accountState.customTokens[pluginId] and insert any enabled tokens that we find in your big JSON but not in the custom token list.
Moving the migration to the engine-start logic would also eliminate Cursor's worry about uninitialized storage.
This revised plan might be a little slower on first-migrate, but should be faster and more stable long-term.
| const LEGACY_TOKENS_FILE = 'EnabledTokens.json' | ||
| const SEEN_TX_CHECKPOINT_FILE = 'seenTxCheckpoint.json' | ||
| const TOKENS_FILE = 'Tokens.json' | ||
| export const TOKENS_FILE = 'Tokens.json' |
There was a problem hiding this comment.
If we move the migration logic to engine-start, as described above, this can go back to being file-private as it should be. We really do want to keep all disk IO in this one source file so it's easy to audit & understand.
| continue | ||
| } | ||
| currencyCode = allTokens[tokenId].currencyCode | ||
| } |
There was a problem hiding this comment.
The GUI no longer uses the old .balances table, which is currency-code based. Could we just remove it as part of the breaking change?
| engine.disableTokens != null && | ||
| engine.enableTokens != null | ||
| ) { | ||
| const builtinTokens = loadBuiltinTokensJson() |
There was a problem hiding this comment.
The engines no longer implement disableTokens or enableTokens, which are currency-code based. Can we just remove those options as part of the breaking change?
src/core/account/account-reducer.ts
Outdated
|
|
||
| allTokens(state = {}, action, next, prev): EdgePluginMap<EdgeTokenMap> { | ||
| const { builtinTokens, customTokens } = next.self | ||
| const { customTokens } = next.self |
There was a problem hiding this comment.
We can gut this whole reducer, now that allTokens is just an alias for customTokens. It could become simply:
allTokens(state = {}, action, next, prev): EdgePluginMap<EdgeTokenMap> {
return next.self.customTokens
}
src/core/account/plugin-api.ts
Outdated
| const { state } = this._ai.props | ||
| const { _accountId: accountId, _pluginId: pluginId } = this | ||
| return state.accounts[accountId].builtinTokens[pluginId] | ||
| return state.accounts[accountId].allTokens[pluginId] ?? emptyTokens |
There was a problem hiding this comment.
The ?? emptyTokens is not needed, as the updated reducer can never return undefined.
| const accountState = input.props.state.accounts[accountId] | ||
| const allTokens = accountState.allTokens[pluginId] ?? {} | ||
|
|
||
| const allTokens = |
There was a problem hiding this comment.
The ?? {} is not needed, as the updated reducer can never return undefined.
| input.props.state.accounts[accountId].allTokens[pluginId] ?? {} | ||
| const enabledTokenIds = uniqueStrings(tokenIds).filter( | ||
| tokenId => allTokens[tokenId] != null | ||
| tokenId => tokenId in allTokens |
There was a problem hiding this comment.
I prefer the tokenId => allTokens[tokenId] != null test, which is more robust.
| expect(config.allTokens).deep.equals({ | ||
| ...config.customTokens, | ||
| ...config.builtinTokens | ||
| ...config.customTokens |
There was a problem hiding this comment.
We don't need the {...} anymore, but can simply use customTokens directly now.
This reverts commit 46dc32c.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| ), | ||
|
|
||
| allTokens(state = {}, action, next, prev): EdgePluginMap<EdgeTokenMap> { | ||
| const { builtinTokens, customTokens } = next.self |
There was a problem hiding this comment.
allTokens[pluginId] now undefined causing runtime crashes
High Severity
The allTokens reducer now returns next.self.customTokens directly, but customTokens only contains entries for plugins that have explicitly stored tokens. The old reducer iterated over all currency plugins and created entries for each (even empty {}), guaranteeing allTokens[pluginId] was always an object. Now allTokens[pluginId] returns undefined for any plugin without custom tokens. Multiple consumers access allTokens[pluginId] without null guards — e.g., changeEnabledTokenIds does allTokens[tokenId] on the result, and getCurrencyMultiplier/upgradeCurrencyCode call Object.keys(allTokens) — all of which throw TypeError when allTokens is undefined. This affects non-token chains like Bitcoin and any wallet whose plugin has no migrated or user-created tokens yet.


CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
noneNote
Medium Risk
Token plumbing and persistence paths are reworked (builtins now loaded from bundled JSON and merged into
customTokens), which could affect token visibility/enabling and legacy migrations if edge cases are missed.Overview
Removes the separate
builtinTokensstate and login-time builtin loading;allTokensis now served directly fromcustomTokens.Adds bundled
builtinTokens.jsonloading (via Rollup JSON support) and a batchaddCustomTokens/ACCOUNT_CUSTOM_TOKENS_ADDEDpath to merge tokens efficiently, enabling migration of builtin/legacy-enabled tokens intocustomTokens. Also extendsEdgeTokenparsing with an optionalisUserCreatedflag.Written by Cursor Bugbot for commit 8fbb8f0. This will update automatically on new commits. Configure here.