feat!: Use `StorageService` to store source code #3777

GuillaumeRx · 2025-12-16T11:34:06Z

This PR adds the use of StorageService to store the Snap source codes. Due to this storage being async, the SnapController initialization is now async and the preinstalled snaps setup now lives in init()

Note

Adds @metamask/storage-service as a new dependency in packages/snaps-controllers/package.json.

Updates dependencies to include @metamask/storage-service@^0.0.1

^{Written by Cursor Bugbot for commit 197f300. This will update automatically on new commits. Configure here.}

socket-security · 2025-12-16T11:34:49Z

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

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@metamask/storage-service@0.0.1

View full report

packages/snaps-controllers/src/snaps/SnapController.ts

codecov · 2026-01-19T13:29:34Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.35%. Comparing base (afc2c54) to head (197f300).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3777   +/-   ##
=======================================
  Coverage   98.35%   98.35%           
=======================================
  Files         430      430           
  Lines       12340    12362   +22     
  Branches     1919     1920    +1     
=======================================
+ Hits        12137    12159   +22     
  Misses        203      203

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

packages/snaps-controllers/src/snaps/SnapController.ts

packages/snaps-controllers/src/test-utils/controller.tsx

packages/snaps-controllers/src/snaps/SnapController.test.tsx

packages/snaps-utils/src/snaps.ts

Mrtenz · 2026-01-21T09:19:46Z

packages/snaps-controllers/package.json

    "@metamask/snaps-rpc-methods": "workspace:^",
    "@metamask/snaps-sdk": "workspace:^",
    "@metamask/snaps-utils": "workspace:^",
+    "@metamask/storage-service": "^0.0.1",


This should really be >=1.0.0 before we use this in production 😵

I know but I was never released as 1.0.0 😅

If Andre considers it stable, we could ask for a bump to v1

packages/snaps-controllers/src/snaps/SnapController.ts

FrederikBolding · 2026-01-21T12:04:59Z

packages/snaps-controllers/src/snaps/SnapController.ts

+   * @param sourceCode - The source code for the Snap.
+   */
+  async #setSourceCode(snapId: SnapId, sourceCode: string): Promise<void> {
+    await this.messenger.call('StorageService:setItem', this.name, snapId, {


Not a problem for right now, if we were storing more than just sourceCode in the storage service, this would overwrite it. Does the StorageService API let us use it more as a KV-store than we are right now?

Unfortunately no, this is as far as it goes as a KV-store. You're right that this will need a rework if we store more than the source code inside but since that's the only thing that we store right now I didn't feel like it was necessary to implement a more complex solution for the moment.

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

packages/snaps-controllers/src/snaps/SnapController.ts

FrederikBolding · 2026-01-23T11:48:29Z

packages/snaps-controllers/src/snaps/SnapController.ts


+    // If the snap has a previous source code, set it back to the previous source code.
+    // If it doesn't, we don't need to set it back to the previous source code because it means we haven't updated the source code.
+    if (previousSourceCode !== '') {


Can we make previousSourceCode optional so we don't need to compare to empty string?

FrederikBolding · 2026-01-23T11:50:16Z

packages/snaps-utils/src/snaps.ts

-  /**
-   * The source code of the Snap.
-   */
-  sourceCode: string;


Is there anywhere in the clients we use the sourceCode from state or returned via get()? If yes, that may break from this 🤔

I've looked at both clients and couldn't find any usage

FrederikBolding · 2026-01-23T11:51:19Z

packages/snaps-utils/src/snaps.ts

+
 export type PersistedSnap = Snap;

+export type StoredSnap = Snap & StorageServiceSnapData;


Suggested change

export type StoredSnap = Snap & StorageServiceSnapData;

export type StoredSnap = PersistedSnap & StorageServiceSnapData;

Nit

lol: #3777 (comment)

fight with @Mrtenz 😂

I meant that we can remove the StoredSnap type 😅

Suggested change

export type StoredSnap = Snap & StorageServiceSnapData;

export type PersistedSnap = Snap & StorageServiceSnapData;

Well, not entirely since that is used for typing out the persisted state of the controller itself

Having both PersistedSnap and StoredSnap seems confusing in my opinion, since they mean very similar things.

Yeah, I don't disagree. But unsure what to propose

FrederikBolding · 2026-01-23T11:52:28Z

packages/snaps-controllers/src/test-utils/controller.tsx

      return getSnapObject({ id: snapId as SnapId });
    });
-
-    messenger.registerActionHandler(


I don't follow why we are changing the default mocks for permissions 🤔

We're changing this to avoid throwing a lot of errors due to the fact that now, because we init the controller in the tests, we call callLifecycleHooks. In this path we call hasPermission and getPermissions, if hasPermission returns true but getPermissions doesn't return the correct handler permissions we throw an error and that was happening for each test.

That makes a lot of sense. Can we document that in the PR description?

FrederikBolding · 2026-01-23T11:52:45Z

packages/snaps-controllers/src/test-utils/controller.tsx

-  messenger.registerActionHandler('PermissionController:hasPermission', () => {
-    return true;
-  });
+  messenger.registerActionHandler(


https://github.com/MetaMask/snaps/pull/3777/files#r2720900517

FrederikBolding · 2026-01-23T11:53:17Z

packages/snaps-controllers/src/test-utils/controller.tsx


 export const approvalControllerMock = new MockApprovalController();

+export const storageAdapter = new InMemoryStorageAdapter();


Should this not be instantiated with each controller instance?

Not very useful as we need to use it when registering the action handlers for the StorageService

I was thinking it makes it easier to have a clean starting point for each test, without needing to clear it out manually

FrederikBolding · 2026-01-23T11:57:21Z

packages/snaps-controllers/src/snaps/SnapController.test.tsx

    fetchMock.mockImplementation(async () => {
      throw new AssertionError({ message: 'Unmocked access to internet.' });
    });
+
+    // Hydrate the storage service with the default snap data.
+    await hydrateStorageService();


Shouldn't this be dependent on the test? Not all tests need the default Snap to be in the storage service

#3777 (comment)

FrederikBolding · 2026-01-23T11:57:59Z

packages/snaps-controllers/src/snaps/SnapController.test.tsx

@@ -10216,7 +10381,12 @@ describe('SnapController', () => {
        origin: 'bar.io',
      });

-      const snapController = getSnapController(
+      await hydrateStorageService({


I don't fully understand when we need to do this and when we don't. Could getSnapController handle this instead? 🤔

we hydrate the defaults in beforeEach so if the test only uses the default values there's no need to call hydrateStorageService. However if the test uses different values in the snaps state we need to set the corresponding source codes in the StorageService

Couldn't each call to getSnapController set it up instead? So we avoid the beforeEach

Mrtenz reviewed Dec 16, 2025

View reviewed changes

packages/snaps-controllers/src/snaps/SnapController.ts Outdated Show resolved Hide resolved

FrederikBolding reviewed Dec 16, 2025

View reviewed changes

packages/snaps-controllers/src/snaps/SnapController.ts Outdated Show resolved Hide resolved

GuillaumeRx force-pushed the gr/storage-service branch from 57095fe to 6a20e47 Compare December 18, 2025 10:37

GuillaumeRx force-pushed the gr/storage-service branch from 202b635 to 801cc12 Compare January 6, 2026 11:10

GuillaumeRx force-pushed the gr/storage-service branch from 655f32e to 85b792f Compare January 19, 2026 13:22

GuillaumeRx force-pushed the gr/storage-service branch from 85b792f to e3515be Compare January 21, 2026 09:05

Mrtenz requested changes Jan 21, 2026

View reviewed changes

GuillaumeRx requested a review from Mrtenz January 21, 2026 10:38

GuillaumeRx marked this pull request as ready for review January 21, 2026 11:16

GuillaumeRx requested a review from a team as a code owner January 21, 2026 11:16

GuillaumeRx force-pushed the gr/storage-service branch from b5235de to 31b6881 Compare January 21, 2026 11:16

This comment was marked as outdated.

Sign in to view

FrederikBolding reviewed Jan 21, 2026

View reviewed changes

packages/snaps-controllers/src/snaps/SnapController.ts Show resolved Hide resolved

FrederikBolding reviewed Jan 21, 2026

View reviewed changes

packages/snaps-controllers/src/snaps/SnapController.ts Show resolved Hide resolved

FrederikBolding reviewed Jan 21, 2026

View reviewed changes

packages/snaps-controllers/src/snaps/SnapController.ts Outdated Show resolved Hide resolved

FrederikBolding reviewed Jan 21, 2026

View reviewed changes

GuillaumeRx requested a review from FrederikBolding January 22, 2026 10:14

This comment was marked as outdated.

Sign in to view

cursor bot reviewed Jan 22, 2026

View reviewed changes

packages/snaps-controllers/src/snaps/SnapController.ts Outdated Show resolved Hide resolved

packages/snaps-controllers/src/snaps/SnapController.ts Show resolved Hide resolved

GuillaumeRx added 9 commits January 22, 2026 14:55

Add StorageService and handle preinstalled snap setup in init

3d47e28

WIP

6872d29

WIP

4040d93

rework is ready

18d0eef

fix SnapController tests

933ade4

fix yarn.lock

cce7e8e

fix simulation test

8791c93

fix interface controller test

1ef0585

address requested changes

71d8a07

GuillaumeRx added 5 commits January 22, 2026 14:55

address more changes

4e22c7b

fix tests

7b60580

make init async

723bd1b

save state patches before any async op that could fail

d1d12a6

fix test

197f300

GuillaumeRx force-pushed the gr/storage-service branch from df10375 to 197f300 Compare January 22, 2026 13:55

FrederikBolding reviewed Jan 23, 2026

View reviewed changes


		export type PersistedSnap = Snap;

		export type StoredSnap = Snap & StorageServiceSnapData;

	export type StoredSnap = Snap & StorageServiceSnapData;
	export type StoredSnap = PersistedSnap & StorageServiceSnapData;

	export type StoredSnap = Snap & StorageServiceSnapData;
	export type PersistedSnap = Snap & StorageServiceSnapData;


		export const approvalControllerMock = new MockApprovalController();

		export const storageAdapter = new InMemoryStorageAdapter();

feat!: Use StorageService to store source code #3777

Are you sure you want to change the base?

feat!: Use StorageService to store source code #3777

Conversation

GuillaumeRx commented Dec 16, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

socket-security bot commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mrtenz Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

feat!: Use `StorageService` to store source code #3777

feat!: Use `StorageService` to store source code #3777

GuillaumeRx commented Dec 16, 2025 •

edited by cursor bot

Loading

socket-security bot commented Dec 16, 2025 •

edited

Loading

codecov bot commented Jan 19, 2026 •

edited

Loading

Mrtenz Jan 23, 2026 •

edited

Loading

FrederikBolding Jan 23, 2026 •

edited

Loading