chore: integrate storageService with tokenListController #7413
+1,576
−95
Conversation
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
sahar-fehri
commented
Dec 9, 2025
| tokensChainsCache: { | ||
| includeInStateLogs: false, | ||
| persist: true, | ||
| persist: false, // Persisted separately via StorageService |
Contributor
Author
There was a problem hiding this comment.
Making this false to block disk writes
7 tasks
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…oading and saving
Prithpal-Sooriya
previously approved these changes
Dec 16, 2025
salimtb
previously approved these changes
Dec 16, 2025
…tchTokenList calls
Contributor
Author
|
@metamaskbot publish-preview |
| // Only remove loaded chains, not chains from initial state that need first persist | ||
| for (const chainId of Object.keys(loadedCache) as Hex[]) { | ||
| this.#changedChainsToPersist.delete(chainId); | ||
| } |
There was a problem hiding this comment.
Initialization removes chains from persistence queue incorrectly
Medium Severity
The #loadCacheFromStorage method removes ALL chains in loadedCache from #changedChainsToPersist, but loadedCache contains all chains from storage, not just chains that were actually added to state. When a chain exists in both initial state and storage, the update() call preserves the initial state's data (correctly), but the chain is still removed from the persistence queue (incorrectly). This means initial state data that should be persisted to storage is silently dropped if the same chain also exists in storage with older data.
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
7 tasks
bergarces
reviewed
Jan 15, 2026
| this.#persistDebounceTimer = undefined; | ||
| } | ||
| this.#changedChainsToPersist.clear(); | ||
| this.#chainsLoadedFromStorage.clear(); |
There was a problem hiding this comment.
Pending state changes lost when controller is destroyed
Medium Severity
The destroy() method cancels the debounce timer and clears #changedChainsToPersist without flushing pending changes to storage. If the controller is destroyed within 500ms of a token fetch or state update, the updated token cache data will be discarded and never persisted. On next app launch, stale data would be loaded from storage instead of the fresher data that was fetched but not persisted.
Prithpal-Sooriya
previously approved these changes
Jan 20, 2026
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Optimizes TokenListController storage to reduce write amplification by persisting tokensChainsCache via StorageService using per-chain files instead of a single monolithic state property.
Mobile: MetaMask/metamask-mobile#24019
Extension: MetaMask/metamask-extension#39250
Related: https://github.com/MetaMask/metamask-mobile/pull/22943/files
Related: https://github.com/MetaMask/decisions/pull/110
Related: #7192
Explanation
The tokensChainsCache (~5MB total, containing token lists for all chains) was persisted as part of the controller state. Every time a single chain's token list was updated (~100-500KB), the entire ~5MB cache was rewritten to disk, causing:
Solution
Per-Chain File Storage:
Each chain's cache is now stored in a separate file (e.g., tokensChainsCache:0x1, tokensChainsCache:0x89)
Only the updated chain (~100-500KB) is written on each token fetch, reducing write operations by ~90-95%
All chains are loaded in parallel at startup to maintain compatibility with TokenDetectionController
Key Changes:
References
Checklist
Note
BREAKING
TokenListControllernow persiststokensChainsCachevia StorageService using per‑chain keys (e.g.,tokensChainsCache:0x1) and requires clients to callawait initialize()after constructiontokensChainsCachepersist: false; state changes are auto‑persisted via debounced subscriptionBehavior/impl changes
clearingTokenListData()now async and removes all per‑chain files; deprecatedstart/restart/stopwrappers maintainedProject updates
@metamask/storage-servicedependency and TS references; extensive unit tests added/updated; changelog updated with breaking noteWritten by Cursor Bugbot for commit d6cac27. This will update automatically on new commits. Configure here.