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..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. * - * As a side-effect of this, preinstalled Snaps may be updated and Snaps may be blocked/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.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index 2e13d6de0b..15957d0d7d 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -10826,6 +10826,107 @@ 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 database update performed + registry.requestUpdate.mockImplementation(() => { + rootMessenger.publish('SnapRegistryController:registryUpdated', 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(); + await waitForStateChange(options.messenger); + await sleep(100); + + 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); diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 72c5702f6f..1d636e7379 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(); @@ -1448,7 +1444,7 @@ 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. + * 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(); diff --git a/packages/snaps-controllers/src/snaps/registry/SnapRegistryController.ts b/packages/snaps-controllers/src/snaps/registry/SnapRegistryController.ts index 4944f3789b..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, @@ -219,6 +225,7 @@ export class SnapRegistryController extends BaseController< async #update() { // No-op if we recently fetched the registry. if (this.#wasRecentlyFetched()) { + this.messenger.publish('SnapRegistryController:registryUpdated', false); return; } @@ -236,6 +243,7 @@ export class SnapRegistryController extends BaseController< state.lastUpdated = Date.now(); state.databaseUnavailable = false; }); + this.messenger.publish('SnapRegistryController:registryUpdated', false); return; } @@ -247,11 +255,14 @@ export class SnapRegistryController extends BaseController< state.databaseUnavailable = false; state.signature = signatureJson.signature; }); + + this.messenger.publish('SnapRegistryController:registryUpdated', true); } catch { // Ignore this.update((state) => { state.databaseUnavailable = true; }); + 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 34bacae94a..0e62625bd9 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 { }, [], ); + this.#messenger.publish('SnapRegistryController:registryUpdated', true); }); }