Skip to content

Commit

Permalink
feat: Emit snapInstalled and snapUpdated for preinstalled Snaps (#…
Browse files Browse the repository at this point in the history
…2900)

Fixes an oversight where preinstalled Snaps would not emit
`snapInstalled` and `snapUpdated`. This would cause lifecycle hooks
among other things to not work. This PR also adds a `preinstalled` flag
to the events, this will be necessary to not collect metrics for
installation/update of preinstalled Snaps.

Additionally, this PR fixes an oversight where preinstalled Snaps could
have the manual update flow triggered. For now, we disallow this
behaviour.

Progresses #2899
  • Loading branch information
FrederikBolding authored Nov 21, 2024
1 parent 406f7a2 commit 126990b
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 6 deletions.
4 changes: 2 additions & 2 deletions packages/snaps-controllers/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 92.85,
"branches": 92.89,
"functions": 96.71,
"lines": 98,
"statements": 97.7
"statements": 97.71
}
86 changes: 86 additions & 0 deletions packages/snaps-controllers/src/snaps/SnapController.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,7 @@ describe('SnapController', () => {
'SnapController:snapInstalled',
getTruncatedSnap(),
MOCK_ORIGIN,
false,
);

snapController.destroy();
Expand Down Expand Up @@ -4665,6 +4666,7 @@ describe('SnapController', () => {
it('supports preinstalled snaps', async () => {
const rootMessenger = getControllerMessenger();
jest.spyOn(rootMessenger, 'call');
jest.spyOn(rootMessenger, 'publish');

// The snap should not have permission initially
rootMessenger.registerActionHandler(
Expand Down Expand Up @@ -4711,6 +4713,13 @@ describe('SnapController', () => {
},
);

expect(rootMessenger.publish).toHaveBeenCalledWith(
'SnapController:snapInstalled',
getTruncatedSnap(),
'metamask',
true,
);

// After install the snap should have permissions
rootMessenger.registerActionHandler(
'PermissionController:getPermissions',
Expand Down Expand Up @@ -4880,6 +4889,7 @@ describe('SnapController', () => {
it('supports updating preinstalled snaps', async () => {
const rootMessenger = getControllerMessenger();
jest.spyOn(rootMessenger, 'call');
jest.spyOn(rootMessenger, 'publish');

const preinstalledSnaps = [
{
Expand Down Expand Up @@ -4934,6 +4944,21 @@ describe('SnapController', () => {
},
);

expect(rootMessenger.publish).toHaveBeenCalledWith(
'SnapController:snapUpdated',
getTruncatedSnap({
version: '1.2.3',
initialPermissions: {
'endowment:rpc': { dapps: false, snaps: true },
// eslint-disable-next-line @typescript-eslint/naming-convention
snap_getEntropy: {},
},
}),
'1.0.0',
'metamask',
true,
);

// After install the snap should have permissions
rootMessenger.registerActionHandler(
'PermissionController:getPermissions',
Expand Down Expand Up @@ -5092,6 +5117,66 @@ describe('SnapController', () => {
snapController.destroy();
});

it('disallows manual updates of preinstalled snaps', async () => {
const rootMessenger = getControllerMessenger();
jest.spyOn(rootMessenger, 'call');

// The snap should not have permissions initially
rootMessenger.registerActionHandler(
'PermissionController:getPermissions',
() => ({}),
);

const preinstalledSnaps = [
{
snapId: MOCK_SNAP_ID,
manifest: getSnapManifest(),
files: [
{
path: DEFAULT_SOURCE_PATH,
value: stringToBytes(DEFAULT_SNAP_BUNDLE),
},
{
path: DEFAULT_ICON_PATH,
value: stringToBytes(DEFAULT_SNAP_ICON),
},
],
},
];

const snapControllerOptions = getSnapControllerWithEESOptions({
preinstalledSnaps,
rootMessenger,
});
const [snapController] = getSnapControllerWithEES(snapControllerOptions);

// After install the snap should have permissions
rootMessenger.registerActionHandler(
'PermissionController:getPermissions',
() => MOCK_SNAP_PERMISSIONS,
);

const { manifest } = await getMockSnapFilesWithUpdatedChecksum({
manifest: getSnapManifest({
version: '1.1.0' as SemVerVersion,
}),
});

const detectSnapLocation = loopbackDetect({
manifest: manifest.result,
});

await expect(
snapController.updateSnap(
MOCK_ORIGIN,
MOCK_SNAP_ID,
detectSnapLocation(),
),
).rejects.toThrow('Preinstalled Snaps cannot be manually updated.');

snapController.destroy();
});

it('supports preinstalled Snaps specifying the hidden flag', async () => {
const rootMessenger = getControllerMessenger();
jest.spyOn(rootMessenger, 'call');
Expand Down Expand Up @@ -6772,6 +6857,7 @@ describe('SnapController', () => {
newSnapTruncated,
'1.0.0',
MOCK_ORIGIN,
false,
);

controller.destroy();
Expand Down
39 changes: 35 additions & 4 deletions packages/snaps-controllers/src/snaps/SnapController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ export type SnapInstallFailed = {
*/
export type SnapInstalled = {
type: `${typeof controllerName}:snapInstalled`;
payload: [snap: TruncatedSnap, origin: string];
payload: [snap: TruncatedSnap, origin: string, preinstalled: boolean];
};

/**
Expand All @@ -500,7 +500,12 @@ export type SnapUnblocked = {
*/
export type SnapUpdated = {
type: `${typeof controllerName}:snapUpdated`;
payload: [snap: TruncatedSnap, oldVersion: string, origin: string];
payload: [
snap: TruncatedSnap,
oldVersion: string,
origin: string,
preinstalled: boolean,
];
};

/**
Expand Down Expand Up @@ -1229,6 +1234,24 @@ export class SnapController extends BaseController<
this.update((state) => {
state.snaps[snapId].status = SnapStatus.Stopped;
});

// Emit events
if (isUpdate) {
this.messagingSystem.publish(
'SnapController:snapUpdated',
this.getTruncatedExpect(snapId),
existingSnap.version,
'metamask',
true,
);
} else {
this.messagingSystem.publish(
'SnapController:snapInstalled',
this.getTruncatedExpect(snapId),
'metamask',
true,
);
}
}
}

Expand Down Expand Up @@ -2323,6 +2346,7 @@ export class SnapController extends BaseController<
`SnapController:snapInstalled`,
this.getTruncatedExpect(snapId),
origin,
false,
),
);

Expand All @@ -2332,6 +2356,7 @@ export class SnapController extends BaseController<
this.getTruncatedExpect(snapId),
oldVersion,
origin,
false,
),
);

Expand Down Expand Up @@ -2537,6 +2562,13 @@ export class SnapController extends BaseController<
): Promise<TruncatedSnap> {
this.#assertCanInstallSnaps();
this.#assertCanUsePlatform();

const snap = this.getExpect(snapId);

if (snap.preinstalled) {
throw new Error('Preinstalled Snaps cannot be manually updated.');
}

if (!isValidSemVerRange(newVersionRange)) {
throw new Error(
`Received invalid snap version range: "${newVersionRange}".`,
Expand All @@ -2557,8 +2589,6 @@ export class SnapController extends BaseController<
true,
);

const snap = this.getExpect(snapId);

const oldManifest = snap.manifest;

const newSnap = await fetchSnap(snapId, location);
Expand Down Expand Up @@ -2679,6 +2709,7 @@ export class SnapController extends BaseController<
truncatedSnap,
snap.version,
origin,
false,
);
}

Expand Down

0 comments on commit 126990b

Please sign in to comment.