From 6f6f9287277ec9287aa6b1ec9db9afd2404325f7 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 7 Nov 2025 14:03:13 +0100 Subject: [PATCH 1/6] feat: Wait for onboarding before allowing usage of SnapController --- .../src/snaps/SnapController.ts | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 14717e84af..d740dfc2f6 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -815,6 +815,13 @@ type SnapControllerArgs = { * MetaMetrics event tracking hook. */ trackEvent: TrackEventHook; + + /** + * A hook that returns a promise that resolves when the onboarding has completed. + * + * @returns A promise that resolves when onboarding is complete. + */ + waitForOnboarding: () => Promise; }; type AddSnapArgs = { @@ -932,6 +939,8 @@ export class SnapController extends BaseController< readonly #trackSnapExport: ReturnType; + readonly #waitForOnboarding: () => Promise; + constructor({ closeAllConnections, messenger, @@ -951,6 +960,7 @@ export class SnapController extends BaseController< getFeatureFlags = () => ({}), clientCryptography, trackEvent, + waitForOnboarding, }: SnapControllerArgs) { super({ messenger, @@ -1034,6 +1044,7 @@ export class SnapController extends BaseController< this.#rollbackSnapshots = new Map(); this.#snapsRuntimeData = new Map(); this.#trackEvent = trackEvent; + this.#waitForOnboarding = waitForOnboarding; this.#pollForLastRequestStatus(); @@ -1467,7 +1478,7 @@ export class SnapController extends BaseController< * Also updates any preinstalled Snaps to the latest allowlisted version. */ async updateRegistry(): Promise { - this.#assertCanUsePlatform(); + await this.#assertCanUsePlatform(); await this.messenger.call('SnapsRegistry:update'); const blockedSnaps = await this.messenger.call( @@ -1645,9 +1656,12 @@ export class SnapController extends BaseController< } /** - * Asserts whether the Snaps platform is allowed to run. + * Waits for onboarding and then asserts whether the Snaps platform is allowed to run. */ - #assertCanUsePlatform() { + async #assertCanUsePlatform() { + // Ensure the user has onboarded before allowing access to Snaps. + await this.#waitForOnboarding(); + const flags = this.#getFeatureFlags(); assert( flags.disableSnaps !== true, @@ -1730,7 +1744,7 @@ export class SnapController extends BaseController< * @param snapId - The id of the Snap to start. */ async startSnap(snapId: SnapId): Promise { - this.#assertCanUsePlatform(); + await this.#assertCanUsePlatform(); const snap = this.state.snaps[snapId]; if (!snap.enabled) { @@ -2630,7 +2644,7 @@ export class SnapController extends BaseController< origin: string, requestedSnaps: RequestSnapsParams, ): Promise { - this.#assertCanUsePlatform(); + await this.#assertCanUsePlatform(); const result: RequestSnapsResult = {}; @@ -2916,7 +2930,7 @@ export class SnapController extends BaseController< if (!automaticUpdate) { this.#assertCanInstallSnaps(); } - this.#assertCanUsePlatform(); + await this.#assertCanUsePlatform(); const snap = this.getExpect(snapId); @@ -3551,7 +3565,7 @@ export class SnapController extends BaseController< handler: handlerType, request: rawRequest, }: SnapRpcHookArgs & { snapId: SnapId }): Promise { - this.#assertCanUsePlatform(); + await this.#assertCanUsePlatform(); const snap = this.get(snapId); From a526ba52f57fedf761d9f8555b15642e70a4902f Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 7 Nov 2025 14:15:01 +0100 Subject: [PATCH 2/6] Add mock --- packages/snaps-controllers/src/test-utils/controller.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/snaps-controllers/src/test-utils/controller.tsx b/packages/snaps-controllers/src/test-utils/controller.tsx index d259c5f164..374404bb7d 100644 --- a/packages/snaps-controllers/src/test-utils/controller.tsx +++ b/packages/snaps-controllers/src/test-utils/controller.tsx @@ -479,7 +479,6 @@ export const getSnapControllerMessenger = ( 'ExecutionService:executeSnap', 'ExecutionService:terminateSnap', 'ExecutionService:handleRpcRequest', - 'NetworkController:getNetworkClientById', 'PermissionController:getEndowments', 'PermissionController:hasPermission', 'PermissionController:hasPermissions', @@ -490,7 +489,6 @@ export const getSnapControllerMessenger = ( 'PermissionController:revokePermissionForAllSubjects', 'PermissionController:updateCaveat', 'PermissionController:getSubjectNames', - 'SelectedNetworkController:getNetworkClientIdForDomain', 'SubjectMetadataController:getSubjectMetadata', 'SubjectMetadataController:addSubjectMetadata', 'SnapsRegistry:get', @@ -574,6 +572,7 @@ export const getSnapControllerOptions = ( clientCryptography: {}, encryptor: getSnapControllerEncryptor(), trackEvent: jest.fn(), + waitForOnboarding: jest.fn().mockResolvedValue(undefined), ...opts, } as SnapControllerConstructorParams; @@ -608,6 +607,7 @@ export const getSnapControllerWithEESOptions = ({ encryptor: getSnapControllerEncryptor(), fetchFunction: jest.fn(), trackEvent: jest.fn(), + waitForOnboarding: jest.fn().mockResolvedValue(undefined), ...options, } as SnapControllerConstructorParams & { rootMessenger: ReturnType; From 1adaf8d121f7a66041e048f92d3fc8ef76cae048 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 12 Nov 2025 13:44:56 +0100 Subject: [PATCH 3/6] Rename --- packages/snaps-controllers/src/snaps/SnapController.ts | 10 +++++----- .../snaps-controllers/src/test-utils/controller.tsx | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index d740dfc2f6..54f15eaaa4 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -821,7 +821,7 @@ type SnapControllerArgs = { * * @returns A promise that resolves when onboarding is complete. */ - waitForOnboarding: () => Promise; + ensureOnboardingComplete: () => Promise; }; type AddSnapArgs = { @@ -939,7 +939,7 @@ export class SnapController extends BaseController< readonly #trackSnapExport: ReturnType; - readonly #waitForOnboarding: () => Promise; + readonly #ensureOnboardingComplete: () => Promise; constructor({ closeAllConnections, @@ -960,7 +960,7 @@ export class SnapController extends BaseController< getFeatureFlags = () => ({}), clientCryptography, trackEvent, - waitForOnboarding, + ensureOnboardingComplete, }: SnapControllerArgs) { super({ messenger, @@ -1044,7 +1044,7 @@ export class SnapController extends BaseController< this.#rollbackSnapshots = new Map(); this.#snapsRuntimeData = new Map(); this.#trackEvent = trackEvent; - this.#waitForOnboarding = waitForOnboarding; + this.#ensureOnboardingComplete = ensureOnboardingComplete; this.#pollForLastRequestStatus(); @@ -1660,7 +1660,7 @@ export class SnapController extends BaseController< */ async #assertCanUsePlatform() { // Ensure the user has onboarded before allowing access to Snaps. - await this.#waitForOnboarding(); + await this.#ensureOnboardingComplete(); const flags = this.#getFeatureFlags(); assert( diff --git a/packages/snaps-controllers/src/test-utils/controller.tsx b/packages/snaps-controllers/src/test-utils/controller.tsx index 374404bb7d..508fcabe48 100644 --- a/packages/snaps-controllers/src/test-utils/controller.tsx +++ b/packages/snaps-controllers/src/test-utils/controller.tsx @@ -572,7 +572,7 @@ export const getSnapControllerOptions = ( clientCryptography: {}, encryptor: getSnapControllerEncryptor(), trackEvent: jest.fn(), - waitForOnboarding: jest.fn().mockResolvedValue(undefined), + ensureOnboardingComplete: jest.fn().mockResolvedValue(undefined), ...opts, } as SnapControllerConstructorParams; @@ -607,7 +607,7 @@ export const getSnapControllerWithEESOptions = ({ encryptor: getSnapControllerEncryptor(), fetchFunction: jest.fn(), trackEvent: jest.fn(), - waitForOnboarding: jest.fn().mockResolvedValue(undefined), + ensureOnboardingComplete: jest.fn().mockResolvedValue(undefined), ...options, } as SnapControllerConstructorParams & { rootMessenger: ReturnType; From 3de1f8077b94307689d5916c20edfc672949eec5 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 12 Nov 2025 14:03:18 +0100 Subject: [PATCH 4/6] Add test --- .../src/snaps/SnapController.test.tsx | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index 1a5a062bfd..4a7b09ecd3 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -2681,6 +2681,55 @@ describe('SnapController', () => { }, ); + it('ensures onboarding has completed before processing requests', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + + const callActionSpy = jest.spyOn(messenger, 'call'); + + const { promise, resolve } = createDeferredPromise(); + const ensureOnboardingComplete = jest.fn().mockReturnValue(promise); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), + }, + ensureOnboardingComplete, + }), + ); + + const snap = snapController.getExpect(MOCK_SNAP_ID); + + const requestPromise = snapController.handleRequest({ + snapId: snap.id, + origin: METAMASK_ORIGIN, + handler: HandlerType.OnRpcRequest, + request: { + jsonrpc: '2.0', + method: 'test', + params: {}, + }, + }); + + await sleep(100); + + expect(callActionSpy).not.toHaveBeenCalledWith( + 'ExecutionService:executeSnap', + expect.objectContaining({ snapId: MOCK_SNAP_ID }), + ); + + resolve(); + expect(await requestPromise).toBeUndefined(); + + expect(callActionSpy).toHaveBeenCalledWith( + 'ExecutionService:executeSnap', + expect.objectContaining({ snapId: MOCK_SNAP_ID }), + ); + + snapController.destroy(); + }); + it('throws if the snap does not have permission to handle JSON-RPC requests from dapps', async () => { const rootMessenger = getControllerMessenger(); const messenger = getSnapControllerMessenger(rootMessenger); From 4196a556ae55c7640cd73ed3fdb95ae024109f4a Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 12 Nov 2025 14:07:45 +0100 Subject: [PATCH 5/6] Tweak assertions --- .../src/snaps/SnapController.test.tsx | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index 4a7b09ecd3..0884fc8c1d 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -795,7 +795,7 @@ describe('SnapController', () => { [MOCK_SNAP_ID]: expectedSnapObject, }); - expect(messenger.call).toHaveBeenCalledTimes(10); + expect(messenger.call).toHaveBeenCalledTimes(9); expect(messenger.call).toHaveBeenNthCalledWith( 1, @@ -5497,7 +5497,7 @@ describe('SnapController', () => { expect(result).toStrictEqual({ [MOCK_LOCAL_SNAP_ID]: truncatedSnap }); - expect(messenger.call).toHaveBeenCalledTimes(12); + expect(messenger.call).toHaveBeenCalledTimes(11); expect(messenger.call).toHaveBeenNthCalledWith( 1, @@ -5647,7 +5647,7 @@ describe('SnapController', () => { [MOCK_LOCAL_SNAP_ID]: truncatedSnap, }); - expect(messenger.call).toHaveBeenCalledTimes(23); + expect(messenger.call).toHaveBeenCalledTimes(22); expect(messenger.call).toHaveBeenNthCalledWith( 1, @@ -6894,7 +6894,7 @@ describe('SnapController', () => { expect(result).toStrictEqual({ [MOCK_SNAP_ID]: truncatedSnap, }); - expect(messenger.call).toHaveBeenCalledTimes(10); + expect(messenger.call).toHaveBeenCalledTimes(9); expect(messenger.call).toHaveBeenNthCalledWith( 1, @@ -7587,7 +7587,7 @@ describe('SnapController', () => { [MOCK_SNAP_ID]: { version: newVersionRange }, }); - expect(messenger.call).toHaveBeenCalledTimes(21); + expect(messenger.call).toHaveBeenCalledTimes(20); expect(messenger.call).toHaveBeenNthCalledWith( 3, @@ -8463,7 +8463,7 @@ describe('SnapController', () => { date: expect.any(Number), }, ]); - expect(callActionSpy).toHaveBeenCalledTimes(21); + expect(callActionSpy).toHaveBeenCalledTimes(20); expect(callActionSpy).toHaveBeenNthCalledWith( 12, @@ -8628,7 +8628,7 @@ describe('SnapController', () => { [MOCK_SNAP_ID]: { version: '1.1.0' }, }); - expect(callActionSpy).toHaveBeenCalledTimes(21); + expect(callActionSpy).toHaveBeenCalledTimes(20); expect(callActionSpy).toHaveBeenNthCalledWith( 12, 'ApprovalController:addRequest', @@ -8744,7 +8744,7 @@ describe('SnapController', () => { [MOCK_SNAP_ID]: { version: '1.1.0' }, }); - expect(callActionSpy).toHaveBeenCalledTimes(22); + expect(callActionSpy).toHaveBeenCalledTimes(21); expect(callActionSpy).toHaveBeenNthCalledWith( 12, 'ApprovalController:addRequest', @@ -8885,7 +8885,7 @@ describe('SnapController', () => { const isRunning = controller.isRunning(MOCK_SNAP_ID); - expect(callActionSpy).toHaveBeenCalledTimes(12); + expect(callActionSpy).toHaveBeenCalledTimes(11); expect(callActionSpy).toHaveBeenNthCalledWith( 1, @@ -9240,7 +9240,7 @@ describe('SnapController', () => { [MOCK_SNAP_ID]: { version: '1.1.0' }, }); - expect(callActionSpy).toHaveBeenCalledTimes(23); + expect(callActionSpy).toHaveBeenCalledTimes(22); expect(callActionSpy).toHaveBeenNthCalledWith( 12, From dd191419ac5a065936b78922fbe86345456cf44a Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 12 Nov 2025 15:26:00 +0100 Subject: [PATCH 6/6] Attempt to fix test --- .../src/snaps/SnapController.test.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index 0884fc8c1d..4365b17110 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -1869,10 +1869,12 @@ describe('SnapController', () => { }, }); - const results = await Promise.allSettled([ - snapController.removeSnap(snap.id), - promise, - ]); + const removeSnap = async () => { + await sleep(1); + return snapController.removeSnap(snap.id); + }; + + const results = await Promise.allSettled([removeSnap(), promise]); expect(results[0].status).toBe('fulfilled'); expect(results[1].status).toBe('rejected');