diff --git a/packages/snaps-controllers/CHANGELOG.md b/packages/snaps-controllers/CHANGELOG.md index 2e20a981f9..fd31792a1e 100644 --- a/packages/snaps-controllers/CHANGELOG.md +++ b/packages/snaps-controllers/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add two new controller state metadata properties: `includeInStateLogs` and `usedInUi` ([#3632](https://github.com/MetaMask/snaps/pull/3632)) + ## [14.2.2] ### Fixed diff --git a/packages/snaps-controllers/package.json b/packages/snaps-controllers/package.json index ce8c05b5b4..0c54d51fb3 100644 --- a/packages/snaps-controllers/package.json +++ b/packages/snaps-controllers/package.json @@ -121,6 +121,7 @@ "@types/concat-stream": "^2.0.0", "@types/gunzip-maybe": "^1.4.0", "@types/jest": "^27.5.1", + "@types/lodash": "^4", "@types/luxon": "^3", "@types/node": "18.14.2", "@types/readable-stream": "^4.0.15", @@ -133,6 +134,7 @@ "jest": "^29.0.2", "jest-fetch-mock": "^3.0.3", "jest-silent-reporter": "^0.6.0", + "lodash": "^4.17.21", "prettier": "^3.3.3", "rimraf": "^4.1.2", "ts-node": "^10.9.1", diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.test.ts b/packages/snaps-controllers/src/cronjob/CronjobController.test.ts index 4d5efa5e7d..2e911ca8db 100644 --- a/packages/snaps-controllers/src/cronjob/CronjobController.test.ts +++ b/packages/snaps-controllers/src/cronjob/CronjobController.test.ts @@ -1,3 +1,4 @@ +import { deriveStateFromMetadata } from '@metamask/base-controller'; import { SnapEndowments } from '@metamask/snaps-rpc-methods'; import type { TruncatedSnap } from '@metamask/snaps-utils'; import { HandlerType } from '@metamask/snaps-utils'; @@ -1126,4 +1127,66 @@ describe('CronjobController', () => { cronjobController.destroy(); }); }); + + describe('metadata', () => { + it('includes expected state in debug snapshots', () => { + const controller = new CronjobController({ + messenger: getRestrictedCronjobControllerMessenger(), + stateManager: getMockStateManager(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'anonymous', + ), + ).toMatchInlineSnapshot(`{}`); + }); + + it('includes expected state in state logs', () => { + const controller = new CronjobController({ + messenger: getRestrictedCronjobControllerMessenger(), + stateManager: getMockStateManager(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInStateLogs', + ), + ).toMatchInlineSnapshot(`{}`); + }); + + it('persists expected state', () => { + const controller = new CronjobController({ + messenger: getRestrictedCronjobControllerMessenger(), + stateManager: getMockStateManager(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'persist', + ), + ).toMatchInlineSnapshot(`{}`); + }); + + it('exposes expected state to UI', () => { + const controller = new CronjobController({ + messenger: getRestrictedCronjobControllerMessenger(), + stateManager: getMockStateManager(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'usedInUi', + ), + ).toMatchInlineSnapshot(`{}`); + }); + }); }); diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.ts b/packages/snaps-controllers/src/cronjob/CronjobController.ts index c98416064a..ba7c2497b2 100644 --- a/packages/snaps-controllers/src/cronjob/CronjobController.ts +++ b/packages/snaps-controllers/src/cronjob/CronjobController.ts @@ -178,7 +178,12 @@ export class CronjobController extends BaseController< super({ messenger, metadata: { - events: { persist: false, anonymous: false }, + events: { + includeInStateLogs: false, + persist: false, + anonymous: false, + usedInUi: false, + }, }, name: controllerName, state: { diff --git a/packages/snaps-controllers/src/insights/SnapInsightsController.test.ts b/packages/snaps-controllers/src/insights/SnapInsightsController.test.ts index 17f6830ebc..6ba96b60bb 100644 --- a/packages/snaps-controllers/src/insights/SnapInsightsController.test.ts +++ b/packages/snaps-controllers/src/insights/SnapInsightsController.test.ts @@ -1,3 +1,4 @@ +import { deriveStateFromMetadata } from '@metamask/base-controller'; import { InternalError } from '@metamask/snaps-sdk'; import { HandlerType } from '@metamask/snaps-utils'; import { @@ -649,4 +650,70 @@ describe('SnapInsightsController', () => { }, ); }); + + describe('metadata', () => { + it('includes expected state in debug snapshots', () => { + const controller = new SnapInsightsController({ + messenger: getRestrictedSnapInsightsControllerMessenger(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'anonymous', + ), + ).toMatchInlineSnapshot(`{}`); + }); + + it('includes expected state in state logs', () => { + const controller = new SnapInsightsController({ + messenger: getRestrictedSnapInsightsControllerMessenger(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInStateLogs', + ), + ).toMatchInlineSnapshot(` + { + "insights": {}, + } + `); + }); + + it('persists expected state', () => { + const controller = new SnapInsightsController({ + messenger: getRestrictedSnapInsightsControllerMessenger(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'persist', + ), + ).toMatchInlineSnapshot(`{}`); + }); + + it('exposes expected state to UI', () => { + const controller = new SnapInsightsController({ + messenger: getRestrictedSnapInsightsControllerMessenger(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'usedInUi', + ), + ).toMatchInlineSnapshot(` + { + "insights": {}, + } + `); + }); + }); }); diff --git a/packages/snaps-controllers/src/insights/SnapInsightsController.ts b/packages/snaps-controllers/src/insights/SnapInsightsController.ts index f1a22a2acd..f4074cb7ed 100644 --- a/packages/snaps-controllers/src/insights/SnapInsightsController.ts +++ b/packages/snaps-controllers/src/insights/SnapInsightsController.ts @@ -100,7 +100,12 @@ export class SnapInsightsController extends BaseController< super({ messenger, metadata: { - insights: { persist: false, anonymous: false }, + insights: { + includeInStateLogs: true, + persist: false, + anonymous: false, + usedInUi: true, + }, }, name: controllerName, state: { insights: {}, ...state }, diff --git a/packages/snaps-controllers/src/interface/SnapInterfaceController.test.tsx b/packages/snaps-controllers/src/interface/SnapInterfaceController.test.tsx index f257a77320..90831604ed 100644 --- a/packages/snaps-controllers/src/interface/SnapInterfaceController.test.tsx +++ b/packages/snaps-controllers/src/interface/SnapInterfaceController.test.tsx @@ -1,4 +1,4 @@ -import { getPersistentState } from '@metamask/base-controller'; +import { deriveStateFromMetadata } from '@metamask/base-controller'; import type { SnapId } from '@metamask/snaps-sdk'; import { form, @@ -82,40 +82,6 @@ describe('SnapInterfaceController', () => { }); }); - describe('constructor', () => { - it('persists notification interfaces', () => { - const rootMessenger = getRootSnapInterfaceControllerMessenger(); - const controllerMessenger = - getRestrictedSnapInterfaceControllerMessenger(rootMessenger); - - const controller = new SnapInterfaceController({ - messenger: controllerMessenger, - state: { - interfaces: { - // @ts-expect-error missing properties - '1': { - contentType: ContentType.Notification, - }, - // @ts-expect-error missing properties - '2': { - contentType: ContentType.Dialog, - }, - }, - }, - }); - - expect( - getPersistentState(controller.state, controller.metadata), - ).toStrictEqual({ - interfaces: { - '1': { - contentType: ContentType.Notification, - }, - }, - }); - }); - }); - describe('createInterface', () => { it('can create a new interface', async () => { const rootMessenger = getRootSnapInterfaceControllerMessenger(); @@ -1745,4 +1711,112 @@ describe('SnapInterfaceController', () => { ).rejects.toThrow(`Approval request with id '${id}' not found.`); }); }); + + describe('metadata', () => { + it('includes expected state in debug snapshots', () => { + const controller = new SnapInterfaceController({ + messenger: getRestrictedSnapInterfaceControllerMessenger(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'anonymous', + ), + ).toMatchInlineSnapshot(`{}`); + }); + + it('includes expected state in state logs', () => { + const controller = new SnapInterfaceController({ + messenger: getRestrictedSnapInterfaceControllerMessenger(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInStateLogs', + ), + ).toMatchInlineSnapshot(` + { + "interfaces": {}, + } + `); + }); + + describe('persist', () => { + it('persists expected state', () => { + const controller = new SnapInterfaceController({ + messenger: getRestrictedSnapInterfaceControllerMessenger(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'persist', + ), + ).toMatchInlineSnapshot(` + { + "interfaces": {}, + } + `); + }); + + it('persists notification interfaces', () => { + const rootMessenger = getRootSnapInterfaceControllerMessenger(); + const controllerMessenger = + getRestrictedSnapInterfaceControllerMessenger(rootMessenger); + + const controller = new SnapInterfaceController({ + messenger: controllerMessenger, + state: { + interfaces: { + // @ts-expect-error missing properties + '1': { + contentType: ContentType.Notification, + }, + // @ts-expect-error missing properties + '2': { + contentType: ContentType.Dialog, + }, + }, + }, + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'persist', + ), + ).toStrictEqual({ + interfaces: { + '1': { + contentType: ContentType.Notification, + }, + }, + }); + }); + }); + + it('exposes expected state to UI', () => { + const controller = new SnapInterfaceController({ + messenger: getRestrictedSnapInterfaceControllerMessenger(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'usedInUi', + ), + ).toMatchInlineSnapshot(` + { + "interfaces": {}, + } + `); + }); + }); }); diff --git a/packages/snaps-controllers/src/interface/SnapInterfaceController.ts b/packages/snaps-controllers/src/interface/SnapInterfaceController.ts index 30f5ae61c0..2a29d8788e 100644 --- a/packages/snaps-controllers/src/interface/SnapInterfaceController.ts +++ b/packages/snaps-controllers/src/interface/SnapInterfaceController.ts @@ -202,6 +202,7 @@ export class SnapInterfaceController extends BaseController< messenger, metadata: { interfaces: { + includeInStateLogs: true, persist: (interfaces: Record) => { return Object.entries(interfaces).reduce< Record @@ -216,6 +217,7 @@ export class SnapInterfaceController extends BaseController< }, {}); }, anonymous: false, + usedInUi: true, }, }, name: controllerName, diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index 69b1eafe91..8e9990b8b0 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -1,4 +1,7 @@ -import { getPersistentState } from '@metamask/base-controller'; +import { + deriveStateFromMetadata, + getPersistentState, +} from '@metamask/base-controller'; import { encrypt } from '@metamask/browser-passworder'; import { createAsyncMiddleware, @@ -72,6 +75,7 @@ import { sha512 } from '@noble/hashes/sha512'; import { File } from 'buffer'; import { createReadStream } from 'fs'; import fetchMock from 'jest-fetch-mock'; +import { pick } from 'lodash'; import path from 'path'; import { pipeline } from 'readable-stream'; import type { Duplex } from 'readable-stream'; @@ -305,80 +309,6 @@ describe('SnapController', () => { snapController.destroy(); }); - it('can rehydrate state', async () => { - const id = 'npm:foo' as SnapId; - const firstSnapController = getSnapController( - getSnapControllerOptions({ - state: { - snaps: getPersistedSnapsState( - getPersistedSnapObject({ - version: '0.0.1', - sourceCode: DEFAULT_SNAP_BUNDLE, - id, - status: SnapStatus.Stopped, - }), - ), - }, - }), - ); - - // persist the state somewhere - const persistedState = getPersistentState( - firstSnapController.state, - firstSnapController.metadata, - ); - - // create a new controller - const secondSnapController = getSnapController( - getSnapControllerOptions({ - state: persistedState, - }), - ); - - expect(secondSnapController.isRunning(id)).toBe(false); - await secondSnapController.startSnap(id); - - expect(secondSnapController.state.snaps[id]).toBeDefined(); - expect(secondSnapController.isRunning(id)).toBe(true); - firstSnapController.destroy(); - secondSnapController.destroy(); - }); - - it('does not persist snaps in the installing state', async () => { - const firstSnapController = getSnapController( - getSnapControllerOptions({ - state: { - snaps: getPersistedSnapsState( - getPersistedSnapObject({ - version: '0.0.1', - sourceCode: DEFAULT_SNAP_BUNDLE, - status: SnapStatus.Installing, - }), - ), - }, - }), - ); - - expect(firstSnapController.state.snaps[MOCK_SNAP_ID]).toBeDefined(); - - // persist the state somewhere - const persistedState = getPersistentState( - firstSnapController.state, - firstSnapController.metadata, - ); - - // create a new controller - const secondSnapController = getSnapController( - getSnapControllerOptions({ - state: persistedState, - }), - ); - - expect(secondSnapController.state.snaps[MOCK_SNAP_ID]).toBeUndefined(); - firstSnapController.destroy(); - secondSnapController.destroy(); - }); - it('handles an error event on the controller messenger', async () => { const options = getSnapControllerWithEESOptions({ state: { @@ -12735,4 +12665,212 @@ describe('SnapController', () => { snapController.destroy(); }); }); + + describe('metadata', () => { + it('includes expected state in debug snapshots', () => { + const controller = getSnapController(); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'anonymous', + ), + ).toMatchInlineSnapshot(`{}`); + }); + + describe('includeInStateLogs', () => { + it('includes expected state in state logs', () => { + const controller = getSnapController(); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInStateLogs', + ), + ).toMatchInlineSnapshot(` + { + "snaps": {}, + } + `); + }); + + it('strips out large state properties', () => { + const id = 'npm:foo' as SnapId; + const auxiliaryFile = new VirtualFile({ + path: 'src/foo.json', + value: stringToBytes('{ "foo" : "bar" }'), + }); + const controller = getSnapController( + getSnapControllerOptions({ + state: { + snaps: getPersistedSnapsState( + getPersistedSnapObject({ + version: '0.0.1', + sourceCode: DEFAULT_SNAP_BUNDLE, + id, + status: SnapStatus.Stopped, + auxiliaryFiles: [ + { + path: auxiliaryFile.path, + value: auxiliaryFile.toString('base64'), + }, + ], + }), + ), + }, + }), + ); + const largeProperties = ['sourceCode', 'auxiliaryFiles']; + const originalSnapState = controller.state.snaps[id]; + const derivedControllerState = deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInStateLogs', + ); + const derivedSnapState = + // Lets assume snaps is an object here. If it's not, the snapshot will tell us. + (derivedControllerState.snaps as Record)[id]; + const originalSnapLargeProperties = pick( + originalSnapState, + largeProperties, + ); + const derivedSnapLargeProperties = pick( + derivedSnapState, + largeProperties, + ); + + expect(originalSnapLargeProperties).toMatchInlineSnapshot(` + { + "auxiliaryFiles": [ + { + "path": "src/foo.json", + "value": "eyAiZm9vIiA6ICJiYXIiIH0=", + }, + ], + "sourceCode": " + module.exports.onRpcRequest = ({ request }) => { + console.log("Hello, world!"); + + const { method, id } = request; + return method + id; + }; + ", + } + `); + expect(derivedSnapLargeProperties).toMatchInlineSnapshot(`{}`); + }); + }); + + describe('persist', () => { + it('persists expected state', () => { + const controller = getSnapController(); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'persist', + ), + ).toMatchInlineSnapshot(` + { + "snapStates": {}, + "snaps": {}, + "unencryptedSnapStates": {}, + } + `); + }); + + it('can rehydrate state', async () => { + const id = 'npm:foo' as SnapId; + const firstSnapController = getSnapController( + getSnapControllerOptions({ + state: { + snaps: getPersistedSnapsState( + getPersistedSnapObject({ + version: '0.0.1', + sourceCode: DEFAULT_SNAP_BUNDLE, + id, + status: SnapStatus.Stopped, + }), + ), + }, + }), + ); + + // persist the state somewhere + const persistedState = getPersistentState( + firstSnapController.state, + firstSnapController.metadata, + ); + + // create a new controller + const secondSnapController = getSnapController( + getSnapControllerOptions({ + state: persistedState, + }), + ); + + expect(secondSnapController.isRunning(id)).toBe(false); + await secondSnapController.startSnap(id); + + expect(secondSnapController.state.snaps[id]).toBeDefined(); + expect(secondSnapController.isRunning(id)).toBe(true); + firstSnapController.destroy(); + secondSnapController.destroy(); + }); + + it('does not persist snaps in the installing state', async () => { + const firstSnapController = getSnapController( + getSnapControllerOptions({ + state: { + snaps: getPersistedSnapsState( + getPersistedSnapObject({ + version: '0.0.1', + sourceCode: DEFAULT_SNAP_BUNDLE, + status: SnapStatus.Installing, + }), + ), + }, + }), + ); + + expect(firstSnapController.state.snaps[MOCK_SNAP_ID]).toBeDefined(); + + // persist the state somewhere + const persistedState = getPersistentState( + firstSnapController.state, + firstSnapController.metadata, + ); + + // create a new controller + const secondSnapController = getSnapController( + getSnapControllerOptions({ + state: persistedState, + }), + ); + + expect(secondSnapController.state.snaps[MOCK_SNAP_ID]).toBeUndefined(); + firstSnapController.destroy(); + secondSnapController.destroy(); + }); + }); + + it('exposes expected state to UI', () => { + const controller = getSnapController(); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'usedInUi', + ), + ).toMatchInlineSnapshot(` + { + "snaps": {}, + } + `); + }); + }); }); diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 69228d3f8a..c4359371dc 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -976,14 +976,31 @@ export class SnapController extends BaseController< messenger, metadata: { snapStates: { + includeInStateLogs: false, persist: true, anonymous: false, + usedInUi: false, }, unencryptedSnapStates: { + includeInStateLogs: false, persist: true, anonymous: false, + usedInUi: false, }, snaps: { + includeInStateLogs: (snaps) => { + // Delete larger snap properties + return Object.values(snaps).reduce>>( + (acc, snap) => { + const snapCopy: Partial = { ...snap }; + delete snapCopy.sourceCode; + delete snapCopy.auxiliaryFiles; + acc[snap.id] = snapCopy; + return acc; + }, + {}, + ); + }, persist: (snaps) => { return ( Object.values(snaps) @@ -1004,6 +1021,9 @@ export class SnapController extends BaseController< ); }, anonymous: false, + // TODO: Ensure larger snap properties are not sent to the UI + // Currently these are stripped out manually in the extension + usedInUi: true, }, }, name: controllerName, diff --git a/packages/snaps-controllers/src/snaps/registry/json.test.ts b/packages/snaps-controllers/src/snaps/registry/json.test.ts index ce245c4b72..3dd5448eaa 100644 --- a/packages/snaps-controllers/src/snaps/registry/json.test.ts +++ b/packages/snaps-controllers/src/snaps/registry/json.test.ts @@ -1,3 +1,4 @@ +import { deriveStateFromMetadata } from '@metamask/base-controller'; import type { SnapsRegistryDatabase } from '@metamask/snaps-registry'; import { DEFAULT_SNAP_SHASUM, @@ -529,4 +530,58 @@ describe('JsonSnapsRegistry', () => { expect(fetchMock).toHaveBeenCalledTimes(2); }); }); + + describe('metadata', () => { + it('includes expected state in debug snapshots', () => { + const { registry } = getRegistry(); + + expect( + deriveStateFromMetadata(registry.state, registry.metadata, 'anonymous'), + ).toMatchInlineSnapshot(`{}`); + }); + + it('includes expected state in state logs', () => { + const { registry } = getRegistry(); + + expect( + deriveStateFromMetadata( + registry.state, + registry.metadata, + 'includeInStateLogs', + ), + ).toMatchInlineSnapshot(` + { + "database": null, + "databaseUnavailable": false, + "lastUpdated": null, + } + `); + }); + + it('persists expected state', () => { + const { registry } = getRegistry(); + + expect( + deriveStateFromMetadata(registry.state, registry.metadata, 'persist'), + ).toMatchInlineSnapshot(` + { + "database": null, + "databaseUnavailable": false, + "lastUpdated": null, + } + `); + }); + + it('exposes expected state to UI', () => { + const { registry } = getRegistry(); + + expect( + deriveStateFromMetadata(registry.state, registry.metadata, 'usedInUi'), + ).toMatchInlineSnapshot(` + { + "database": null, + } + `); + }); + }); }); diff --git a/packages/snaps-controllers/src/snaps/registry/json.ts b/packages/snaps-controllers/src/snaps/registry/json.ts index b8b3b44385..e7acb76e62 100644 --- a/packages/snaps-controllers/src/snaps/registry/json.ts +++ b/packages/snaps-controllers/src/snaps/registry/json.ts @@ -142,9 +142,24 @@ export class JsonSnapsRegistry extends BaseController< super({ messenger, metadata: { - database: { persist: true, anonymous: false }, - lastUpdated: { persist: true, anonymous: false }, - databaseUnavailable: { persist: true, anonymous: false }, + database: { + includeInStateLogs: true, + persist: true, + anonymous: false, + usedInUi: true, + }, + lastUpdated: { + includeInStateLogs: true, + persist: true, + anonymous: false, + usedInUi: false, + }, + databaseUnavailable: { + includeInStateLogs: true, + persist: true, + anonymous: false, + usedInUi: false, + }, }, name: controllerName, state: { diff --git a/yarn.lock b/yarn.lock index e23df3ced5..ca331597e5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4245,6 +4245,7 @@ __metadata: "@types/concat-stream": "npm:^2.0.0" "@types/gunzip-maybe": "npm:^1.4.0" "@types/jest": "npm:^27.5.1" + "@types/lodash": "npm:^4" "@types/luxon": "npm:^3" "@types/node": "npm:18.14.2" "@types/readable-stream": "npm:^4.0.15" @@ -4264,6 +4265,7 @@ __metadata: jest: "npm:^29.0.2" jest-fetch-mock: "npm:^3.0.3" jest-silent-reporter: "npm:^0.6.0" + lodash: "npm:^4.17.21" luxon: "npm:^3.5.0" nanoid: "npm:^3.3.10" prettier: "npm:^3.3.3"