feat: Add StorageService for offloading large controller data #7192
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
Add @metamask/storage-service package to enable controllers to store large,
infrequently-accessed data outside of Redux state.
**Problem**:
- 10.79 MB Engine state with 92% (10.18 MB) in just 2 controllers
- Slow app startup parsing large state
- High memory usage from rarely-accessed data
**Solution**:
Platform-agnostic StorageService with:
- Messenger integration (setItem, getItem, removeItem actions)
- Event system (itemSet, itemRemoved events)
- Storage adapters (FilesystemStorage, IndexedDB, InMemoryAdapter)
- Namespace isolation (storage:{namespace}:{key})
**Impact**:
- 92% state reduction potential (10.79 MB → 0.85 MB)
- Lazy loading - data loaded only when needed
- Event-driven - controllers react without coupling
**Implementation**:
- 100% test coverage (44 tests)
- Platform-agnostic design
- InMemoryAdapter for tests (zero config)
- Follows ErrorReportingService pattern
**Targets**:
- SnapController: 6.09 MB sourceCode
- TokenListController: 4.09 MB cache
- getAllKeys and clear now take namespace parameter
- Adapters handle filtering and clearing (allows platform-specific optimization)
- Removed internal key registry from core (simpler code)
- Renamed clearNamespace to clear (consistent with other methods)
- Use STORAGE_KEY_PREFIX constant ('storageService:') instead of hardcoded 'storage:'
- InMemoryAdapter implements filtering and clearing logic
- Mobile adapter can now be optimized per-platform
Benefits:
- Simpler core service (no registry maintenance)
- Platform-specific optimizations possible (IndexedDB can use IDBKeyRange)
- Clear adapter responsibilities (filtering, prefix handling)
- Consistent API (all methods take namespace first)
Tests: 100% coverage maintained, all tests passing
7 tasks
Move key building responsibility from core to adapters: - StorageAdapter methods now take (namespace, key) parameters - Adapters build full key format internally - Core no longer has #buildKey() method - Each platform can choose optimal key format Benefits: - Simpler core (no key format knowledge) - Platform flexibility (IndexedDB could use different format) - Adapter owns all key logic - InMemoryAdapter builds: storageService:namespace:key - Mobile adapter builds: storageService:namespace:key Interface change (breaking for adapters): - getItem(key) → getItem(namespace, key) - setItem(key, value) → setItem(namespace, key, value) - removeItem(key) → removeItem(namespace, key) All tests updated and passing (100% coverage).
… adapters - Remove StoredDataWrapper from exports (each adapter defines its own) - Remove STORAGE_KEY_PREFIX from StorageService (adapters handle key building) - Update StorageAdapter interface: getItem returns unknown (adapter handles parsing) - Adapters now fully responsible for: - Building full storage keys (e.g., storageService:namespace:key) - Wrapping data with metadata (timestamp) - Serialization/deserialization - Update tests to match new adapter delegation pattern - 100% test coverage maintained
…rics - Update StorageAdapter interface (adapters handle key building & serialization) - Update adapter examples with full implementation - Rename clearNamespace to clear - Update key prefix from storage: to storageService: - Update metrics to precise values (5.95 MB, 3.99 MB, 9.94 MB, 166 KB, 61 bytes)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Member
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. |
Member
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. |
- Add warnings that service is designed for large values (100KB+) - Add examples of good vs bad usage patterns - Discourage many small key-value pairs in favor of single large objects
Gudahtt
reviewed
Nov 26, 2025
Gudahtt
reviewed
Nov 26, 2025
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
- Remove itemRemoved event publishing from removeItem() and clear() - Remove StorageServiceItemRemovedEvent type - Update StorageServiceEvents to only include itemSet - Simplify API: controllers calling remove/clear already know what they're doing - Update README and ADR documentation
…to storage-service
Add standard MetaMask dual license setup as requested by legal: - LICENSE: Dual license pointer - LICENSE.MIT: MIT License - LICENSE.APACHE2: Apache License 2.0
Since there's no schema validation, using generics gives false type safety. - setItem now accepts value: unknown - getItem now returns Promise<unknown> - Callers must validate/cast the returned data - Updated JSDoc examples to show casting pattern
Gudahtt
reviewed
Nov 27, 2025
- Add MESSENGER_EXPOSED_METHODS constant - Use registerMethodActionHandlers for bulk action registration - Auto-generate action types via generate-method-action-types script - Simplify exports (StorageServiceActions = StorageServiceMethodActions)
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
- Remove marketing copy (Problem/Solution/Real-World Impact) - Remove API section (use inline docs instead) - Move 'When to Use' to top for visibility - Simplify usage examples - Keep StorageAdapter Interface for implementers
…to storage-service
- Verify event publishes with [key, value] payload - Verify event only fires for matching namespace
…to storage-service
- Replace unknown with Json type from @metamask/utils for type safety - Add @metamask/utils as dependency (required for public API types) - Remove StoredDataWrapper - just stringify/parse values directly - Update StorageAdapter interface, StorageService, and InMemoryStorageAdapter
- Add storage-service to .github/CODEOWNERS with extension-platform, mobile-platform, and core-platform as owners - Add storage-service to teams.json with extension and mobile platform teams
Member
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. |
Gudahtt
reviewed
Nov 28, 2025
| const serialized = this.#storage.get(fullKey); | ||
|
|
||
| if (!serialized) { | ||
| return null; |
Member
There was a problem hiding this comment.
Hmm. This could be ambiguous for the caller, there would be no way to differentiate data stored as null versus non-existent data.
Maybe we could return the data wrapped in an object, e.g. { data: ___ }, so that the null return can more clearly mean that it's not present?
Or we could use undefined as the empty return value, but that seems less than idea given that we'll be calling this across process boundaries on extension, and that would end up coerced to null.
Member
There was a problem hiding this comment.
Or I guess we could go Result type with this, e.g. { result } or { error } or {} (empty), to capture the error case as well. If you wanted this to "not crash by default", but in a way that still allows callers to handle the error.
| return JSON.parse(serialized) as Json; | ||
| } catch (error) { | ||
| // istanbul ignore next - defensive error handling for corrupted data | ||
| console.error(`Failed to parse stored data for ${fullKey}:`, error); |
Member
There was a problem hiding this comment.
This seems a bit dangerous. The caller would have no way to differentiate empty data from data that we failed to retrieve. The caller might then proceed when it's unsafe to do so, or overwrite the data, or something like that.
Given that we don't know precisely why the data is being stored/retrieved, or what the consequences of this failure might be, it would be safer to not catch the error and let the caller deal with it. We can highlight that it can throw with a TSDoc @throws directive in the doc comment for getItem (on both the service and the storage adapter) so that it's more obvious to the caller that they should expect failure some of the time.
| try { | ||
| return JSON.parse(serialized) as Json; | ||
| } catch (error) { | ||
| // istanbul ignore next - defensive error handling for corrupted data |
Member
There was a problem hiding this comment.
Nit: This is a legitimate test case, we should be able to test this (even if we do remove the test block). Storage corruption can happen.
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Gudahtt
reviewed
Nov 28, 2025
Distinguish between found, not found, and error conditions:
- { result: Json } - data found and retrieved
- {} (empty object) - key doesn't exist
- { error: Error } - error occurred during retrieval
This allows consumers to distinguish stored null from missing keys.
Also adds test for corrupted data error handling.
…to storage-service
Gudahtt
previously approved these changes
Nov 28, 2025
| return { result }; | ||
| } catch (error) { | ||
| console.error(`Failed to parse stored data for ${fullKey}:`, error); | ||
| return { error: error as Error }; |
Member
There was a problem hiding this comment.
Normally I don't like to cast if we can avoid it, and we can avoid it here by validating the error type.
But... it is pretty easy to tell in this case that the only possible error here is SyntaxError. So.... this seems pretty unobjectionable.
I might propose an improvement here later, but I can do it as a separate PR
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Gudahtt
previously approved these changes
Nov 28, 2025
Gudahtt
approved these changes
Nov 28, 2025
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.
Add StorageService for Large Controller Data
Explanation
What is the current state and why does it need to change?
Current state: MetaMask Mobile Engine state is 10.79 MB, with 92% (9.94 MB) concentrated in just 2 controllers:
This data is rarely accessed after initial load but stays in Redux state, causing:
Why change: Controllers need a way to store large, infrequently-accessed data outside of Redux state while maintaining platform portability and testability.
What is the solution and how does it work?
New package:
@metamask/storage-serviceA platform-agnostic service that allows controllers to offload large data from state to persistent storage via messenger actions.
How it works:
StorageService:setItemvia messenger to store large dataStorageService:itemSet:{namespace}) so other controllers can reactStorageService:getItemto load data lazily (only when needed)Storage adapter pattern:
Example controller usage:
Why this architecture?
Platform-agnostic: Service defines
StorageAdapterinterface; clients provide implementation (mobile: FilesystemStorage, extension: IndexedDB, tests: in-memory)Messenger-integrated: Controllers access storage via messenger actions, no direct dependencies
Event-driven: Controllers can subscribe to storage changes without coupling
Testable: InMemoryAdapter provides zero-config testing (no mocking needed)
Proven pattern: Follows ErrorReportingService design (accepts platform-specific function)
Expected impact?
With both controllers optimized:
This PR adds infrastructure - Controllers can now use StorageService. Separate PRs will integrate with SnapController and TokenListController.
References
Checklist
Note
Introduces
@metamask/storage-servicewith messenger-based APIs and an in-memory adapter to offload large controller data, plus repo wiring and ownership updates.packages/storage-serviceStorageService: Messenger-exposed methodssetItem,getItem,removeItem,getAllKeys,clear; publishesStorageService:itemSet:{namespace}events.InMemoryStorageAdapter: Default non-persistent adapter implementingStorageAdapterwith key prefixstorageService:.StorageAdapter,StorageGetResult, messenger types, action types file, constantsSERVICE_NAME,STORAGE_KEY_PREFIX.README.mdwith usage/examples;CHANGELOG.mdinitialized; dual-license files.tsconfig.build.json,yarn.lock, exports/index, and TypeDoc config..github/CODEOWNERSandteams.jsonto includestorage-serviceownership.Written by Cursor Bugbot for commit 6bf384a. This will update automatically on new commits. Configure here.