From 3632d8efc91947a0a76deb2cf8327df6c948a16f Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 9 Apr 2026 15:04:01 +0200 Subject: [PATCH 1/5] fix: Always consume latest registry in updateRegistry --- .../src/snaps/SnapController.ts | 12 ++++++++-- .../snaps/registry/SnapRegistryController.ts | 23 ++++++++++++------- .../src/test-utils/registry.ts | 1 + 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 72c5702f6f..b97d1abb2e 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -1448,11 +1448,19 @@ export class SnapController extends BaseController< /** * Trigger an update of the registry. * - * As a side-effect of this, preinstalled Snaps may be updated and Snaps may be blocked/unblocked. + * This will _always_ check if preinstalled Snaps can be updated and whether any Snaps need to beblocked/unblocked. */ async updateRegistry(): Promise { await this.#ensureCanUsePlatform(); - await this.messenger.call('SnapRegistryController:requestUpdate'); + const updated = await this.messenger.call( + 'SnapRegistryController:requestUpdate', + ); + + // When returning false, the `:stateChange` event is not emitted, so we force update. + // We do this to enable mainly to provide a path to retry OTA updates. + if (!updated) { + await this.#handleRegistryUpdate(); + } } /** diff --git a/packages/snaps-controllers/src/snaps/registry/SnapRegistryController.ts b/packages/snaps-controllers/src/snaps/registry/SnapRegistryController.ts index 4944f3789b..c85c2025a4 100644 --- a/packages/snaps-controllers/src/snaps/registry/SnapRegistryController.ts +++ b/packages/snaps-controllers/src/snaps/registry/SnapRegistryController.ts @@ -122,7 +122,7 @@ export class SnapRegistryController extends BaseController< readonly #refetchOnAllowlistMiss: boolean; - #currentUpdate: Promise | null; + #currentUpdate: Promise | null; constructor({ messenger, @@ -196,30 +196,34 @@ export class SnapRegistryController extends BaseController< * Triggers an update of the registry database. * * If an existing update is in progress this function will await that update. + * + * @returns True if an update was performed, otherwise false. */ - async requestUpdate() { + async requestUpdate(): Promise { // If an update is ongoing, wait for that. if (this.#currentUpdate) { - await this.#currentUpdate; - return; + return await this.#currentUpdate; } // If no update exists, create promise and store globally. if (this.#currentUpdate === null) { this.#currentUpdate = this.#update(); } - await this.#currentUpdate; + const result = await this.#currentUpdate; this.#currentUpdate = null; + return result; } /** * Updates the registry database if the registry hasn't been updated recently. * * NOTE: SHOULD NOT be called directly, instead `triggerUpdate` should be used. + * + * @returns True if an update was performed, otherwise false. */ - async #update() { + async #update(): Promise { // No-op if we recently fetched the registry. if (this.#wasRecentlyFetched()) { - return; + return false; } try { @@ -236,7 +240,7 @@ export class SnapRegistryController extends BaseController< state.lastUpdated = Date.now(); state.databaseUnavailable = false; }); - return; + return false; } await this.#verifySignature(database, signatureJson); @@ -247,11 +251,14 @@ export class SnapRegistryController extends BaseController< state.databaseUnavailable = false; state.signature = signatureJson.signature; }); + + return true; } catch { // Ignore this.update((state) => { state.databaseUnavailable = true; }); + return false; } } diff --git a/packages/snaps-controllers/src/test-utils/registry.ts b/packages/snaps-controllers/src/test-utils/registry.ts index 34bacae94a..c49d322d62 100644 --- a/packages/snaps-controllers/src/test-utils/registry.ts +++ b/packages/snaps-controllers/src/test-utils/registry.ts @@ -57,5 +57,6 @@ export class MockSnapRegistryController { }, [], ); + return true; }); } From 3154da53ca0d9c43f7a34820222fd4ac7c3ecd98 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 9 Apr 2026 15:14:39 +0200 Subject: [PATCH 2/5] Update action types --- .../src/snaps/SnapController-method-action-types.ts | 2 +- .../registry/SnapRegistryController-method-action-types.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController-method-action-types.ts b/packages/snaps-controllers/src/snaps/SnapController-method-action-types.ts index aade71d1ff..087188a4f3 100644 --- a/packages/snaps-controllers/src/snaps/SnapController-method-action-types.ts +++ b/packages/snaps-controllers/src/snaps/SnapController-method-action-types.ts @@ -21,7 +21,7 @@ export type SnapControllerInitAction = { /** * Trigger an update of the registry. * - * As a side-effect of this, preinstalled Snaps may be updated and Snaps may be blocked/unblocked. + * This will _always_ check if preinstalled Snaps can be updated and whether any Snaps need to beblocked/unblocked. */ export type SnapControllerUpdateRegistryAction = { type: `SnapController:updateRegistry`; diff --git a/packages/snaps-controllers/src/snaps/registry/SnapRegistryController-method-action-types.ts b/packages/snaps-controllers/src/snaps/registry/SnapRegistryController-method-action-types.ts index 526ff3d0e6..7bddc896a8 100644 --- a/packages/snaps-controllers/src/snaps/registry/SnapRegistryController-method-action-types.ts +++ b/packages/snaps-controllers/src/snaps/registry/SnapRegistryController-method-action-types.ts @@ -9,6 +9,8 @@ import type { SnapRegistryController } from './SnapRegistryController'; * Triggers an update of the registry database. * * If an existing update is in progress this function will await that update. + * + * @returns True if an update was performed, otherwise false. */ export type SnapRegistryControllerRequestUpdateAction = { type: `SnapRegistryController:requestUpdate`; From b723cb475bf31b63f89661a1b3bee005ade723db Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 9 Apr 2026 15:20:27 +0200 Subject: [PATCH 3/5] Add test --- .../src/snaps/SnapController.test.tsx | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index 2e13d6de0b..d379dabfd9 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -10826,6 +10826,103 @@ describe('SnapController', () => { snapController.destroy(); }); + it('retries updating preinstalled Snaps', async () => { + const rootMessenger = getRootMessenger(); + const registry = new MockSnapRegistryController(rootMessenger); + + // Simulate previous permissions, some of which will be removed + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => { + return { + [SnapEndowments.Rpc]: MOCK_RPC_ORIGINS_PERMISSION, + [SnapEndowments.LifecycleHooks]: MOCK_LIFECYCLE_HOOKS_PERMISSION, + }; + }, + ); + + const snapId = 'npm:@metamask/jsx-example-snap' as SnapId; + + const mockSnap = getPersistedSnapObject({ + id: snapId, + preinstalled: true, + }); + + // Indicate no registry update performed + registry.requestUpdate.mockResolvedValue(false); + + const updateVersion = '1.2.1'; + + registry.resolveVersion.mockResolvedValue(updateVersion); + const fetchFunction = jest.fn().mockResolvedValueOnce({ + // eslint-disable-next-line no-restricted-globals + headers: new Headers({ 'content-length': '5477' }), + ok: true, + body: Readable.toWeb( + createReadStream( + path.resolve( + __dirname, + `../../test/fixtures/metamask-jsx-example-snap-${updateVersion}.tgz`, + ), + ), + ), + }); + + const options = getSnapControllerOptions({ + rootMessenger, + state: { + snaps: getPersistedSnapsState(mockSnap), + }, + fetchFunction, + featureFlags: { + autoUpdatePreinstalledSnaps: true, + }, + }); + + const snapController = await getSnapController(options); + + await snapController.updateRegistry(); + + const updatedSnap = snapController.getSnap(snapId); + assert(updatedSnap); + + expect(updatedSnap.version).toStrictEqual(updateVersion); + expect(updatedSnap.preinstalled).toBe(true); + + expect(options.messenger.call).toHaveBeenNthCalledWith( + 7, + 'StorageService:setItem', + controllerName, + snapId, + { sourceCode: expect.any(String) }, + ); + + expect(options.messenger.call).toHaveBeenNthCalledWith( + 9, + 'PermissionController:revokePermissions', + { [snapId]: [SnapEndowments.Rpc, SnapEndowments.LifecycleHooks] }, + ); + + expect(options.messenger.call).toHaveBeenNthCalledWith( + 10, + 'PermissionController:grantPermissions', + { + approvedPermissions: { + 'endowment:rpc': { + caveats: [{ type: 'rpcOrigin', value: { dapps: true } }], + }, + // eslint-disable-next-line @typescript-eslint/naming-convention + snap_dialog: {}, + // eslint-disable-next-line @typescript-eslint/naming-convention + snap_manageState: {}, + }, + subject: { origin: snapId }, + }, + ); + + snapController.destroy(); + }); + it('does not update preinstalled Snaps when the feature flag is off', async () => { const rootMessenger = getRootMessenger(); const registry = new MockSnapRegistryController(rootMessenger); From fe8ded1e27a958615be5c406bcd688c96642694c Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 9 Apr 2026 16:35:22 +0200 Subject: [PATCH 4/5] Use an event instead --- .../src/snaps/SnapController.test.tsx | 8 +++-- .../src/snaps/SnapController.ts | 36 +++++++------------ ...pRegistryController-method-action-types.ts | 2 -- .../snaps/registry/SnapRegistryController.ts | 34 ++++++++++-------- .../src/snaps/registry/index.ts | 1 + .../src/test-utils/controller.tsx | 2 +- .../src/test-utils/registry.ts | 2 +- 7 files changed, 40 insertions(+), 45 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index d379dabfd9..15957d0d7d 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -10848,8 +10848,10 @@ describe('SnapController', () => { preinstalled: true, }); - // Indicate no registry update performed - registry.requestUpdate.mockResolvedValue(false); + // Indicate no database update performed + registry.requestUpdate.mockImplementation(() => { + rootMessenger.publish('SnapRegistryController:registryUpdated', false); + }); const updateVersion = '1.2.1'; @@ -10882,6 +10884,8 @@ describe('SnapController', () => { const snapController = await getSnapController(options); await snapController.updateRegistry(); + await waitForStateChange(options.messenger); + await sleep(100); const updatedSnap = snapController.getSnap(snapId); assert(updatedSnap); diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index b97d1abb2e..ed28f5347b 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -167,7 +167,7 @@ import type { SnapRegistryControllerRequestUpdateAction, SnapRegistryInfo, SnapRegistryRequest, - SnapRegistryControllerStateChangeEvent, + SnapRegistryControllerRegistryUpdatedEvent, } from './registry'; import { SnapRegistryStatus } from './registry'; import { getRunnableSnaps } from './selectors'; @@ -547,7 +547,7 @@ type AllowedEvents = | SnapControllerSnapInstalledEvent | SnapControllerSnapUpdatedEvent | KeyringControllerLockEvent - | SnapRegistryControllerStateChangeEvent; + | SnapRegistryControllerRegistryUpdatedEvent; export type SnapControllerMessenger = Messenger< typeof controllerName, @@ -974,19 +974,15 @@ export class SnapController extends BaseController< this.#handleLock.bind(this), ); - this.messenger.subscribe( - 'SnapRegistryController:stateChange', - () => { - this.#handleRegistryUpdate().catch((error) => { - logError( - `Error when processing Snaps registry update: ${getErrorMessage( - error, - )}`, - ); - }); - }, - ({ database }) => database, - ); + this.messenger.subscribe('SnapRegistryController:registryUpdated', () => { + this.#handleRegistryUpdate().catch((error) => { + logError( + `Error when processing Snaps registry update: ${getErrorMessage( + error, + )}`, + ); + }); + }); this.#initializeStateMachine(); this.#registerMessageHandlers(); @@ -1452,15 +1448,7 @@ export class SnapController extends BaseController< */ async updateRegistry(): Promise { await this.#ensureCanUsePlatform(); - const updated = await this.messenger.call( - 'SnapRegistryController:requestUpdate', - ); - - // When returning false, the `:stateChange` event is not emitted, so we force update. - // We do this to enable mainly to provide a path to retry OTA updates. - if (!updated) { - await this.#handleRegistryUpdate(); - } + await this.messenger.call('SnapRegistryController:requestUpdate'); } /** diff --git a/packages/snaps-controllers/src/snaps/registry/SnapRegistryController-method-action-types.ts b/packages/snaps-controllers/src/snaps/registry/SnapRegistryController-method-action-types.ts index 7bddc896a8..526ff3d0e6 100644 --- a/packages/snaps-controllers/src/snaps/registry/SnapRegistryController-method-action-types.ts +++ b/packages/snaps-controllers/src/snaps/registry/SnapRegistryController-method-action-types.ts @@ -9,8 +9,6 @@ import type { SnapRegistryController } from './SnapRegistryController'; * Triggers an update of the registry database. * * If an existing update is in progress this function will await that update. - * - * @returns True if an update was performed, otherwise false. */ export type SnapRegistryControllerRequestUpdateAction = { type: `SnapRegistryController:requestUpdate`; diff --git a/packages/snaps-controllers/src/snaps/registry/SnapRegistryController.ts b/packages/snaps-controllers/src/snaps/registry/SnapRegistryController.ts index c85c2025a4..47effba7c3 100644 --- a/packages/snaps-controllers/src/snaps/registry/SnapRegistryController.ts +++ b/packages/snaps-controllers/src/snaps/registry/SnapRegistryController.ts @@ -73,8 +73,14 @@ export type SnapRegistryControllerStateChangeEvent = ControllerStateChangeEvent< SnapRegistryControllerState >; +export type SnapRegistryControllerRegistryUpdatedEvent = { + type: `${typeof controllerName}:registryUpdated`; + payload: [databaseUpdated: boolean]; +}; + export type SnapRegistryControllerEvents = - SnapRegistryControllerStateChangeEvent; + | SnapRegistryControllerStateChangeEvent + | SnapRegistryControllerRegistryUpdatedEvent; export type SnapRegistryControllerMessenger = Messenger< typeof controllerName, @@ -122,7 +128,7 @@ export class SnapRegistryController extends BaseController< readonly #refetchOnAllowlistMiss: boolean; - #currentUpdate: Promise | null; + #currentUpdate: Promise | null; constructor({ messenger, @@ -196,34 +202,31 @@ export class SnapRegistryController extends BaseController< * Triggers an update of the registry database. * * If an existing update is in progress this function will await that update. - * - * @returns True if an update was performed, otherwise false. */ - async requestUpdate(): Promise { + async requestUpdate() { // If an update is ongoing, wait for that. if (this.#currentUpdate) { - return await this.#currentUpdate; + await this.#currentUpdate; + return; } // If no update exists, create promise and store globally. if (this.#currentUpdate === null) { this.#currentUpdate = this.#update(); } - const result = await this.#currentUpdate; + await this.#currentUpdate; this.#currentUpdate = null; - return result; } /** * Updates the registry database if the registry hasn't been updated recently. * * NOTE: SHOULD NOT be called directly, instead `triggerUpdate` should be used. - * - * @returns True if an update was performed, otherwise false. */ - async #update(): Promise { + async #update() { // No-op if we recently fetched the registry. if (this.#wasRecentlyFetched()) { - return false; + this.messenger.publish('SnapRegistryController:registryUpdated', false); + return; } try { @@ -240,7 +243,8 @@ export class SnapRegistryController extends BaseController< state.lastUpdated = Date.now(); state.databaseUnavailable = false; }); - return false; + this.messenger.publish('SnapRegistryController:registryUpdated', false); + return; } await this.#verifySignature(database, signatureJson); @@ -252,13 +256,13 @@ export class SnapRegistryController extends BaseController< state.signature = signatureJson.signature; }); - return true; + this.messenger.publish('SnapRegistryController:registryUpdated', true); } catch { // Ignore this.update((state) => { state.databaseUnavailable = true; }); - return false; + this.messenger.publish('SnapRegistryController:registryUpdated', false); } } diff --git a/packages/snaps-controllers/src/snaps/registry/index.ts b/packages/snaps-controllers/src/snaps/registry/index.ts index a4c0875fa5..4fb3ca5cc6 100644 --- a/packages/snaps-controllers/src/snaps/registry/index.ts +++ b/packages/snaps-controllers/src/snaps/registry/index.ts @@ -6,6 +6,7 @@ export type { SnapRegistryControllerMessenger, SnapRegistryControllerState, SnapRegistryControllerStateChangeEvent, + SnapRegistryControllerRegistryUpdatedEvent, } from './SnapRegistryController'; export { SnapRegistryController } from './SnapRegistryController'; export type { diff --git a/packages/snaps-controllers/src/test-utils/controller.tsx b/packages/snaps-controllers/src/test-utils/controller.tsx index 0fd143a10f..33b7942d63 100644 --- a/packages/snaps-controllers/src/test-utils/controller.tsx +++ b/packages/snaps-controllers/src/test-utils/controller.tsx @@ -509,7 +509,7 @@ export const getSnapControllerMessenger = ( 'ExecutionService:outboundRequest', 'ExecutionService:outboundResponse', 'KeyringController:lock', - 'SnapRegistryController:stateChange', + 'SnapRegistryController:registryUpdated', ], messenger: snapControllerMessenger, }); diff --git a/packages/snaps-controllers/src/test-utils/registry.ts b/packages/snaps-controllers/src/test-utils/registry.ts index c49d322d62..0e62625bd9 100644 --- a/packages/snaps-controllers/src/test-utils/registry.ts +++ b/packages/snaps-controllers/src/test-utils/registry.ts @@ -57,6 +57,6 @@ export class MockSnapRegistryController { }, [], ); - return true; + this.#messenger.publish('SnapRegistryController:registryUpdated', true); }); } From 621a01af918b98f0753a236c438948e8916bd925 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 9 Apr 2026 17:25:33 +0200 Subject: [PATCH 5/5] Update JSDoc --- .../src/snaps/SnapController-method-action-types.ts | 2 +- packages/snaps-controllers/src/snaps/SnapController.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController-method-action-types.ts b/packages/snaps-controllers/src/snaps/SnapController-method-action-types.ts index 087188a4f3..ca5abccd5e 100644 --- a/packages/snaps-controllers/src/snaps/SnapController-method-action-types.ts +++ b/packages/snaps-controllers/src/snaps/SnapController-method-action-types.ts @@ -21,7 +21,7 @@ export type SnapControllerInitAction = { /** * Trigger an update of the registry. * - * This will _always_ check if preinstalled Snaps can be updated and whether any Snaps need to beblocked/unblocked. + * As a side-effect, this will _always_ check if preinstalled Snaps can be updated and whether any Snaps need to be blocked/unblocked. */ export type SnapControllerUpdateRegistryAction = { type: `SnapController:updateRegistry`; diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index ed28f5347b..1d636e7379 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -1444,7 +1444,7 @@ export class SnapController extends BaseController< /** * Trigger an update of the registry. * - * This will _always_ check if preinstalled Snaps can be updated and whether any Snaps need to beblocked/unblocked. + * As a side-effect, this will _always_ check if preinstalled Snaps can be updated and whether any Snaps need to be blocked/unblocked. */ async updateRegistry(): Promise { await this.#ensureCanUsePlatform();