Skip to content

Conversation

@andrepimenta
Copy link
Member

@andrepimenta andrepimenta commented Nov 19, 2025

Integrate StorageService for Large Controller Data

Description

Integrates @metamask/storage-service to enable controllers to offload large, infrequently-accessed data from Redux state to persistent storage.

CHANGELOG entry: null

Problem

State bloat impacting mobile performance:

  • 10.79 MB total Engine state
  • 92% (9.94 MB) in just 2 controllers storing rarely-accessed data
  • Slow app startup parsing 10.79 MB on every launch
  • High memory usage (all data loaded even if not needed)
  • Slow persist operations (up to 6.26 MB per controller change)

Root cause: Controllers store large data (snap source code, token caches) in Redux state instead of on disk.

Solution

Add StorageService integration - Controllers can now store large data via messenger:

// Store data (out of state, on disk)
await messenger.call(
  'StorageService:setItem',
  'ControllerA',
  'dataKey',
  largeData,
);

// Load on demand (lazy loading)
const data = await messenger.call(
  'StorageService:getItem',
  'ControllerA',
  'dataKey',
);

What's Included

StorageService Integration:

  • storage-service-init.ts - FilesystemStorage adapter for mobile
  • storage-service-messenger.ts - Messenger factory
  • ✅ Engine registration - Service available to all controllers
  • ✅ Type definitions - StorageService in Engine types
  • ✅ Messenger delegation - Actions/events configured
  • ✅ Tests - Integration verified (5 tests passing)

FilesystemStorage Adapter (mobile-specific):

  • Implements StorageAdapter interface
  • Handles namespace filtering (getAllKeys, clear)
  • Uses STORAGE_KEY_PREFIX constant (storageService:)
  • iOS/Android platform handling
  • Error logging

Impact

Immediate (infrastructure ready):

  • ✅ Service available for controllers to use
  • ✅ Zero performance impact (no controllers using yet)

When controllers migrate (separate PRs):

  • 92% state reduction (10.79 MB → 0.85 MB)
  • SnapController: 5.95 MB sourceCode → disk
  • TokenListController: 3.99 MB cache → disk
  • Faster app startup (92% less data to parse)
  • Memory freed: 9.94 MB

Architecture

Platform-agnostic service:

Controller → messenger.call('StorageService:setItem', ...)
  ↓
StorageService (core) → Publishes events
  ↓
FilesystemStorage (mobile) → Persists to disk

Key format: storageService:{namespace}:{key}
Example: storageService:SnapController:snap-id:sourceCode

Testing

# Run integration tests
yarn jest app/core/Engine/controllers/storage-service-init.test.ts
yarn jest app/core/Engine/messengers/storage-service-messenger.test.ts

# Expected: 5 tests pass

Test coverage:

  • ✅ Service initialization
  • ✅ FilesystemStorage adapter integration
  • ✅ iOS/Android handling
  • ✅ Messenger creation
  • ✅ Error handling

Follow-up Work

Controller migrations (separate PRs):

  1. SnapController - Remove 5.95 MB sourceCode from state
  2. TokenListController - Remove 3.99 MB cache from state

Each migration requires:

  • Controller changes (call StorageService)
  • Store migration (move existing data)
  • Testing and benchmarking

Related

Checklist

  • Tests pass (5/5)
  • No linter errors (except spurious messenger type warning)
  • TypeScript compiles
  • Follows existing service integration patterns (ErrorReportingService)
  • Platform-specific code in adapter only
  • No breaking changes
  • Performance benchmarking (after controller migrations)

Notes

This PR adds infrastructure only - No immediate user-facing changes.

Performance improvements come when controllers are migrated to use StorageService (separate PRs for each controller).

Why FilesystemStorage? Optimized for large files (multi-MB). MMKV is better for small, frequently-accessed data (< 1 MB).


Note

Integrates @metamask/storage-service into the Engine with a FilesystemStorage-based mobile adapter, messenger wiring, type plumbing, and tests.

  • Engine integration
    • Register StorageService via storageServiceInit in Engine.ts and expose in context.
    • Add StorageService to STATELESS_NON_CONTROLLER_NAMES, controller messenger map, and ControllersToInitialize.
    • Extend global actions/events/types to include StorageService in types.ts.
  • Storage adapter
    • New controllers/storage-service-init.ts: mobile adapter implementing getItem, setItem, removeItem, getAllKeys, clear using redux-persist-filesystem-storage, Device.isIos, and Logger; wires StorageService with adapter.
  • Messenger
    • New messengers/storage-service-messenger.ts and registration in messengers/index.ts.
  • Tests
    • Add storage-service-init.test.ts covering adapter behavior and initialization.
    • Add storage-service-messenger.test.ts.
  • Dependencies
    • Add @metamask/storage-service to package.json and lockfile.

Written by Cursor Bugbot for commit 6cdc688. This will update automatically on new commits. Configure here.

Add StorageService integration to enable controllers to offload large data
from Redux state to persistent storage.

**Problem**:
- 10.79 MB Engine state with 92% in 2 controllers
- Slow app startup parsing large state
- High memory usage from rarely-accessed data

**Solution**:
StorageService integration with FilesystemStorage adapter:
- Platform-agnostic service via messenger actions
- Event system for reactive patterns
- FilesystemStorage adapter for mobile persistence
- Namespace isolation (storageService:{namespace}:{key})

**Implementation**:
- StorageService registered in Engine
- FilesystemStorage adapter with namespace filtering
- Messenger delegation configured
- Service available for all controllers
- Uses STORAGE_KEY_PREFIX constant

**Impact** (when controllers migrate):
- 92% state reduction potential (10.79 MB → 0.85 MB)
- SnapController: 6.09 MB sourceCode candidate
- TokenListController: 4.09 MB cache candidate

**Files**:
- storage-service-init.ts: FilesystemStorage adapter
- storage-service-messenger.ts: Messenger factory
- Engine types/messengers: Registration
- Tests: Integration verified
Adapter methods now receive namespace and key separately:
- getItem(namespace, key) - Builds storageService:namespace:key
- setItem(namespace, key, value) - Builds full key internally
- removeItem(namespace, key) - Builds full key internally

Mobile adapter builds keys as: storageService:namespace:key
Using STORAGE_KEY_PREFIX constant from core package.

Matches new StorageAdapter interface from @metamask/storage-service.
…serialization

- Define StoredDataWrapper locally (no longer exported from core)
- Adapter now fully responsible for:
  - Building full storage keys (storageService:namespace:key)
  - Wrapping data with metadata (timestamp)
  - Serialization/deserialization
- Aligns with updated StorageAdapter interface from core
@andrepimenta andrepimenta marked this pull request as ready for review November 26, 2025 15:42
@andrepimenta andrepimenta requested a review from a team as a code owner November 26, 2025 15:42
@socket-security
Copy link

socket-security bot commented Nov 26, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addednpm/​@​metamask/​storage-service@​0.0.1751009989100

View full report

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Nov 26, 2025
Required for messenger parent type resolution - the Messenger constructor
validates that both Actions and Events are in the parent's types.
…NAMES

StorageService is a stateless service, not a controller with state.
Excluding it from ControllerPersistedState fixes the TS2536 error.
- Add jest.clearAllMocks() in beforeEach to fix mock accumulation bug
- Add comprehensive tests for mobileStorageAdapter methods:
  - getItem: happy path, null, parse error, storage error
  - setItem: happy path, Android device, storage error
  - removeItem: happy path, storage error
  - getAllKeys: happy path, empty, null keys, storage error
  - clear: happy path, null keys, empty namespace, storage error
- Update existing tests to verify getAllKeys and clear methods
@github-actions github-actions bot added size-L and removed size-M labels Nov 26, 2025
…rage mock

FilesystemStorage.getItem returns string | undefined, not string | null
Change 'handles empty namespace gracefully' to
'removes zero keys and logs count when namespace has no matching entries'
@andrepimenta andrepimenta marked this pull request as draft November 27, 2025 15:40
- getItem now throws on error instead of returning null
- getAllKeys now throws on error instead of returning []
- Consistent with setItem, removeItem, and clear which already throw
- Allows callers to distinguish 'not found' from 'error'
- Use Json type from @metamask/utils for type safety
- Remove StoredDataWrapper - just stringify/parse values directly
- Update tests to reflect simplified storage format
github-merge-queue bot pushed a commit to MetaMask/core that referenced this pull request Nov 28, 2025
# 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:
- **SnapController**: 5.95 MB of snap source code (55% of total state)
- **TokenListController**: 3.99 MB of token metadata cache (37% of
state)

This data is **rarely accessed** after initial load but stays in Redux
state, causing:
- Slow app startup (parsing 10.79 MB on every launch)
- High memory usage (all data loaded even if not needed)
- Slow persist operations (up to 6.26 MB written per controller change)

**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-service`

A platform-agnostic service that allows controllers to offload large
data from state to persistent storage via messenger actions.

**How it works**:
1. Controllers call `StorageService:setItem` via messenger to store
large data
2. StorageService saves to platform-specific storage (FilesystemStorage
on mobile, IndexedDB on extension)
3. StorageService publishes events
(`StorageService:itemSet:{namespace}`) so other controllers can react
4. Controllers call `StorageService:getItem` to load data lazily (only
when needed)

**Storage adapter pattern**:
```typescript
// Service accepts platform-specific adapter (like ErrorReportingService)
const service = new StorageService({
  messenger,
  storage: filesystemStorageAdapter, // Mobile provides this
});
```

**Example controller usage**:
```typescript
// Store data (out of state)
await this.messenger.call(
  'StorageService:setItem',
  'MyController',
  'dataKey',
  largeData,
);

// Load on demand (lazy loading)
const data = await this.messenger.call(
  'StorageService:getItem',
  'MyController',
  'dataKey',
);

// Subscribe to changes (optional)
this.messenger.subscribe(
  'StorageService:itemSet:MyController',
  (key, value) => {
    // React to storage changes
  },
);
```

### Why this architecture?

**Platform-agnostic**: Service defines `StorageAdapter` interface;
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**:
- State: 10.79 MB → 0.85 MB (**92% reduction**)
- App startup: 92% faster state parsing
- Memory: 9.94 MB freed
- Disk I/O: Up to 9.94 MB less per persist

**This PR adds infrastructure** - Controllers can now use
StorageService. Separate PRs will integrate with SnapController and
TokenListController.

## References

- **ADR**:
[0017-storage-service-large-data.md](https://github.com/MetaMask/decisions/blob/core/0017-storage-service-large-data/decisions/core/0017-storage-service-large-data.md)
- **Mobile PR**: MetaMask/metamask-mobile#22943

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
  - 100% test coverage (44 tests)
- Tests for all storage operations, namespace isolation, events, error
handling
  - Real-world usage test (simulates 6 MB snap source code)
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
  - Complete README with examples
  - JSDoc for all public APIs
  - Architecture documentation in ADR
- [x] I've communicated my changes to consumers by updating changelogs
for packages I've changed, highlighting breaking changes as necessary
  - CHANGELOG.md created for initial release
  - No breaking changes (new package)
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
  - N/A - No breaking changes
  - Consumer PRs will be created after this is merged and released



<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Introduces `@metamask/storage-service` with messenger-based APIs and
an in-memory adapter to offload large controller data, plus repo wiring
and ownership updates.
> 
> - **New Package**: `packages/storage-service`
> - `StorageService`: Messenger-exposed methods `setItem`, `getItem`,
`removeItem`, `getAllKeys`, `clear`; publishes
`StorageService:itemSet:{namespace}` events.
> - `InMemoryStorageAdapter`: Default non-persistent adapter
implementing `StorageAdapter` with key prefix `storageService:`.
> - Types/exports: `StorageAdapter`, `StorageGetResult`, messenger
types, action types file, constants `SERVICE_NAME`,
`STORAGE_KEY_PREFIX`.
> - **Tests & Docs**:
> - Comprehensive unit tests for service and adapter (namespace
isolation, events, errors); Jest config with 100% coverage thresholds.
> - `README.md` with usage/examples; `CHANGELOG.md` initialized;
dual-license files.
> - **Repo Wiring**:
> - Add package to `tsconfig.build.json`, `yarn.lock`, exports/index,
and TypeDoc config.
> - Update `.github/CODEOWNERS` and `teams.json` to include
`storage-service` ownership.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
6bf384a. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Mark Stacey <[email protected]>
…GetResult

- Switch from @metamask-previews/storage-service to @metamask/storage-service
- Update mobileStorageAdapter.getItem to return StorageGetResult:
  - { result } when data found
  - {} when key doesn't exist
  - { error } on failure
- Update tests for new response format
- Remove @metamask-previews/storage-service dependency
@andrepimenta andrepimenta marked this pull request as ready for review December 12, 2025 12:28
@github-actions
Copy link
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeCore, SmokeWalletPlatform, SmokeAccounts
  • Risk Level: medium
  • AI Confidence: 80%
click to see 🤖 AI reasoning details

This PR introduces a new StorageService to the MetaMask Mobile Engine. The changes include:

  1. New Package: @metamask/storage-service@^0.0.1 added to package.json
  2. Engine Integration: StorageService added to the modular controller initialization in Engine.ts
  3. Mobile Storage Adapter: A mobile-specific storage adapter using FilesystemStorage for persistent storage
  4. Messenger Setup: New messenger for StorageService communication
  5. Type Definitions: StorageService types integrated into Engine's type system

Risk Assessment (Medium):

  • This modifies the core Engine (critical infrastructure file)
  • However, the StorageService is purely additive - no existing functionality is modified
  • The service is not yet consumed by any other controllers (grep shows all references are only in the new files)
  • Comprehensive unit tests are included for the new service
  • The service is registered as a "stateless non-controller" in constants.ts

Tag Selection Rationale:

  • SmokeCore: Essential because this modifies Engine.ts, the core of the wallet. Need to verify the Engine still initializes correctly with the new service.
  • SmokeWalletPlatform: Core wallet functionality tests to ensure basic wallet operations work after Engine changes.
  • SmokeAccounts: Account management tests since the Engine changes could affect account-related controllers.

The change is foundational infrastructure that doesn't yet have consumers, so the main risk is ensuring the Engine boots correctly and existing functionality remains intact. Not selecting all tags because the change is additive and well-isolated with no modifications to existing controller logic.

View GitHub Actions results

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

INVALID-PR-TEMPLATE PR's body doesn't match template size-L team-mobile-platform Mobile Platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants