diff --git a/packages/snaps-controllers/jest.config.js b/packages/snaps-controllers/jest.config.js index 48e2f39559..7ce1be39ea 100644 --- a/packages/snaps-controllers/jest.config.js +++ b/packages/snaps-controllers/jest.config.js @@ -4,10 +4,10 @@ const baseConfig = require('../../jest.config.base'); module.exports = deepmerge(baseConfig, { coverageThreshold: { global: { - branches: 89.03, - functions: 96.44, - lines: 97.06, - statements: 97.06, + branches: 89.08, + functions: 96.42, + lines: 96.79, + statements: 96.79, }, }, projects: [ diff --git a/packages/snaps-controllers/src/services/AbstractExecutionService.ts b/packages/snaps-controllers/src/services/AbstractExecutionService.ts index 2c15cb1c93..e5676c088e 100644 --- a/packages/snaps-controllers/src/services/AbstractExecutionService.ts +++ b/packages/snaps-controllers/src/services/AbstractExecutionService.ts @@ -58,32 +58,34 @@ export type Job = { export abstract class AbstractExecutionService implements ExecutionService { - protected _snapRpcHooks: Map; + #snapRpcHooks: Map; + // Cannot be hash private yet because of tests. protected jobs: Map>; - protected setupSnapProvider: SetupSnapProvider; + // Cannot be hash private yet because of tests. + private setupSnapProvider: SetupSnapProvider; - protected snapToJobMap: Map; + #snapToJobMap: Map; - protected jobToSnapMap: Map; + #jobToSnapMap: Map; - protected _messenger: ExecutionServiceMessenger; + #messenger: ExecutionServiceMessenger; - protected _terminationTimeout: number; + #terminationTimeout: number; constructor({ setupSnapProvider, messenger, terminationTimeout = Duration.Second, }: ExecutionServiceArgs) { - this._snapRpcHooks = new Map(); + this.#snapRpcHooks = new Map(); this.jobs = new Map(); this.setupSnapProvider = setupSnapProvider; - this.snapToJobMap = new Map(); - this.jobToSnapMap = new Map(); - this._messenger = messenger; - this._terminationTimeout = terminationTimeout; + this.#snapToJobMap = new Map(); + this.#jobToSnapMap = new Map(); + this.#messenger = messenger; + this.#terminationTimeout = terminationTimeout; this.registerMessageHandlers(); } @@ -93,23 +95,23 @@ export abstract class AbstractExecutionService * actions. */ private registerMessageHandlers(): void { - this._messenger.registerActionHandler( + this.#messenger.registerActionHandler( `${controllerName}:handleRpcRequest`, (snapId: string, options: SnapRpcHookArgs) => this.handleRpcRequest(snapId, options), ); - this._messenger.registerActionHandler( + this.#messenger.registerActionHandler( `${controllerName}:executeSnap`, (snapData: SnapExecutionData) => this.executeSnap(snapData), ); - this._messenger.registerActionHandler( + this.#messenger.registerActionHandler( `${controllerName}:terminateSnap`, (snapId: string) => this.terminateSnap(snapId), ); - this._messenger.registerActionHandler( + this.#messenger.registerActionHandler( `${controllerName}:terminateAllSnaps`, () => this.terminateAllSnaps(), ); @@ -122,7 +124,7 @@ export abstract class AbstractExecutionService * * @param job - The object corresponding to the job to be terminated. */ - protected abstract _terminate(job: Job): void; + protected abstract terminateJob(job: Job): void; /** * Terminates the job with the specified ID and deletes all its associated @@ -140,13 +142,13 @@ export abstract class AbstractExecutionService // Ping worker and tell it to run teardown, continue with termination if it takes too long const result = await withTimeout( - this._command(jobId, { + this.command(jobId, { jsonrpc: '2.0', method: 'terminate', params: [], id: nanoid(), }), - this._terminationTimeout, + this.#terminationTimeout, ); if (result === hasTimedOut || result !== 'OK') { @@ -167,9 +169,9 @@ export abstract class AbstractExecutionService } }); - this._terminate(jobWrapper); + this.terminateJob(jobWrapper); - this._removeSnapAndJobMapping(jobId); + this.#removeSnapAndJobMapping(jobId); this.jobs.delete(jobId); console.log(`Job "${jobId}" terminated.`); } @@ -181,9 +183,9 @@ export abstract class AbstractExecutionService * * @returns Information regarding the created job. */ - protected async _initJob(): Promise> { + protected async initJob(): Promise> { const jobId = nanoid(); - const { streams, worker } = await this._initStreams(jobId); + const { streams, worker } = await this.initStreams(jobId); const rpcEngine = new JsonRpcEngine(); const jsonRpcConnection = createStreamMiddleware(); @@ -211,10 +213,10 @@ export abstract class AbstractExecutionService * @param jobId - The id of the job. * @returns The streams to communicate with the worker and the worker itself. */ - protected async _initStreams( + protected async initStreams( jobId: string, ): Promise<{ streams: JobStreams; worker: WorkerType }> { - const { worker, stream: envStream } = await this._initEnvStream(jobId); + const { worker, stream: envStream } = await this.initEnvStream(jobId); // Typecast justification: stream type mismatch const mux = setupMultiplex( envStream as unknown as Duplex, @@ -235,14 +237,14 @@ export abstract class AbstractExecutionService } // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const snapId = this.jobToSnapMap.get(jobId)!; + const snapId = this.#jobToSnapMap.get(jobId)!; if (message.method === 'OutboundRequest') { - this._messenger.publish('ExecutionService:outboundRequest', snapId); + this.#messenger.publish('ExecutionService:outboundRequest', snapId); } else if (message.method === 'OutboundResponse') { - this._messenger.publish('ExecutionService:outboundResponse', snapId); + this.#messenger.publish('ExecutionService:outboundResponse', snapId); } else if (message.method === 'UnhandledError') { if (isObject(message.params) && message.params.error) { - this._messenger.publish( + this.#messenger.publish( 'ExecutionService:unhandledError', snapId, message.params.error as SnapErrorJson, @@ -282,7 +284,7 @@ export abstract class AbstractExecutionService * * Depending on the execution environment, this may run forever if the Snap fails to start up properly, therefore any call to this function should be wrapped in a timeout. */ - protected abstract _initEnvStream(jobId: string): Promise<{ + protected abstract initEnvStream(jobId: string): Promise<{ worker: WorkerType; stream: BasePostMessageStream; }>; @@ -295,7 +297,7 @@ export abstract class AbstractExecutionService * @param snapId - The ID of the snap to terminate. */ async terminateSnap(snapId: string) { - const jobId = this.snapToJobMap.get(snapId); + const jobId = this.#snapToJobMap.get(snapId); if (jobId) { await this.terminate(jobId); } @@ -305,7 +307,7 @@ export abstract class AbstractExecutionService await Promise.all( [...this.jobs.keys()].map((jobId) => this.terminate(jobId)), ); - this._snapRpcHooks.clear(); + this.#snapRpcHooks.clear(); } /** @@ -315,7 +317,7 @@ export abstract class AbstractExecutionService * @returns The RPC request handler for the snap. */ private async getRpcRequestHandler(snapId: string) { - return this._snapRpcHooks.get(snapId); + return this.#snapRpcHooks.get(snapId); } /** @@ -328,15 +330,15 @@ export abstract class AbstractExecutionService * @throws If the execution service returns an error. */ async executeSnap(snapData: SnapExecutionData): Promise { - if (this.snapToJobMap.has(snapData.snapId)) { + if (this.#snapToJobMap.has(snapData.snapId)) { throw new Error(`Snap "${snapData.snapId}" is already being executed.`); } - const job = await this._initJob(); - this._mapSnapAndJob(snapData.snapId, job.id); + const job = await this.initJob(); + this.#mapSnapAndJob(snapData.snapId, job.id); // Ping the worker to ensure that it started up - await this._command(job.id, { + await this.command(job.id, { jsonrpc: '2.0', method: 'ping', id: nanoid(), @@ -346,17 +348,18 @@ export abstract class AbstractExecutionService this.setupSnapProvider(snapData.snapId, rpcStream); - const result = await this._command(job.id, { + const result = await this.command(job.id, { jsonrpc: '2.0', method: 'executeSnap', params: snapData, id: nanoid(), }); - this._createSnapHooks(snapData.snapId, job.id); + this.#createSnapHooks(snapData.snapId, job.id); return result as string; } - protected async _command( + // Cannot be hash private yet because of tests. + private async command( jobId: string, message: JsonRpcRequest, ): Promise { @@ -378,13 +381,13 @@ export abstract class AbstractExecutionService return response.result; } - protected _removeSnapHooks(snapId: string) { - this._snapRpcHooks.delete(snapId); + #removeSnapHooks(snapId: string) { + this.#snapRpcHooks.delete(snapId); } - protected _createSnapHooks(snapId: string, workerId: string) { + #createSnapHooks(snapId: string, workerId: string) { const rpcHook = async ({ origin, handler, request }: SnapRpcHookArgs) => { - return await this._command(workerId, { + return await this.command(workerId, { id: nanoid(), jsonrpc: '2.0', method: 'snapRpc', @@ -397,7 +400,7 @@ export abstract class AbstractExecutionService }); }; - this._snapRpcHooks.set(snapId, rpcHook); + this.#snapRpcHooks.set(snapId, rpcHook); } /** @@ -406,8 +409,8 @@ export abstract class AbstractExecutionService * @param snapId - A given snap id. * @returns The ID of the snap's job. */ - protected _getJobForSnap(snapId: string): string | undefined { - return this.snapToJobMap.get(snapId); + #getJobForSnap(snapId: string): string | undefined { + return this.#snapToJobMap.get(snapId); } /** @@ -416,24 +419,24 @@ export abstract class AbstractExecutionService * @param jobId - A given job id. * @returns The ID of the snap that is running the job. */ - _getSnapForJob(jobId: string): string | undefined { - return this.jobToSnapMap.get(jobId); + #getSnapForJob(jobId: string): string | undefined { + return this.#jobToSnapMap.get(jobId); } - protected _mapSnapAndJob(snapId: string, jobId: string): void { - this.snapToJobMap.set(snapId, jobId); - this.jobToSnapMap.set(jobId, snapId); + #mapSnapAndJob(snapId: string, jobId: string): void { + this.#snapToJobMap.set(snapId, jobId); + this.#jobToSnapMap.set(jobId, snapId); } - protected _removeSnapAndJobMapping(jobId: string): void { - const snapId = this.jobToSnapMap.get(jobId); + #removeSnapAndJobMapping(jobId: string): void { + const snapId = this.#jobToSnapMap.get(jobId); if (!snapId) { throw new Error(`job: "${jobId}" has no mapped snap.`); } - this.jobToSnapMap.delete(jobId); - this.snapToJobMap.delete(snapId); - this._removeSnapHooks(snapId); + this.#jobToSnapMap.delete(jobId); + this.#snapToJobMap.delete(snapId); + this.#removeSnapHooks(snapId); } /** diff --git a/packages/snaps-controllers/src/services/iframe/IframeExecutionService.ts b/packages/snaps-controllers/src/services/iframe/IframeExecutionService.ts index 49fe4e5ee8..da2a1667b2 100644 --- a/packages/snaps-controllers/src/services/iframe/IframeExecutionService.ts +++ b/packages/snaps-controllers/src/services/iframe/IframeExecutionService.ts @@ -27,15 +27,15 @@ export class IframeExecutionService extends AbstractExecutionService { this.iframeUrl = iframeUrl; } - protected _terminate(jobWrapper: Job): void { + protected terminateJob(jobWrapper: Job): void { document.getElementById(jobWrapper.id)?.remove(); } - protected async _initEnvStream(jobId: string): Promise<{ + protected async initEnvStream(jobId: string): Promise<{ worker: Window; stream: BasePostMessageStream; }> { - const iframeWindow = await this._createWindow( + const iframeWindow = await this.createWindow( this.iframeUrl.toString(), jobId, ); @@ -58,7 +58,7 @@ export class IframeExecutionService extends AbstractExecutionService { * @param jobId - The job id. * @returns A promise that resolves to the contentWindow of the iframe. */ - private _createWindow(uri: string, jobId: string): Promise { + private createWindow(uri: string, jobId: string): Promise { return new Promise((resolve, reject) => { const iframe = document.createElement('iframe'); // The order of operations appears to matter for everything except this diff --git a/packages/snaps-controllers/src/services/iframe/test/fixJSDOMPostMessageEventSource.ts b/packages/snaps-controllers/src/services/iframe/test/fixJSDOMPostMessageEventSource.ts index d076e94f01..19e42b94b6 100644 --- a/packages/snaps-controllers/src/services/iframe/test/fixJSDOMPostMessageEventSource.ts +++ b/packages/snaps-controllers/src/services/iframe/test/fixJSDOMPostMessageEventSource.ts @@ -4,8 +4,8 @@ import { IframeExecutionService } from '../IframeExecutionService'; const fixJSDOMPostMessageEventSource = ( iframeExecutionService: IframeExecutionService, ) => { - const oldCreateWindow = (iframeExecutionService as any)._createWindow; - (iframeExecutionService as any)._createWindow = async ( + const oldCreateWindow = (iframeExecutionService as any).createWindow; + (iframeExecutionService as any).createWindow = async ( uri: string, envId: string, timeout: number, diff --git a/packages/snaps-controllers/src/services/node/NodeProcessExecutionService.ts b/packages/snaps-controllers/src/services/node/NodeProcessExecutionService.ts index 5ede691e25..b7504fa97c 100644 --- a/packages/snaps-controllers/src/services/node/NodeProcessExecutionService.ts +++ b/packages/snaps-controllers/src/services/node/NodeProcessExecutionService.ts @@ -6,7 +6,7 @@ import { import { AbstractExecutionService, Job } from '..'; export class NodeProcessExecutionService extends AbstractExecutionService { - protected async _initEnvStream(): Promise<{ + protected async initEnvStream(): Promise<{ worker: ChildProcess; stream: BasePostMessageStream; }> { @@ -19,7 +19,7 @@ export class NodeProcessExecutionService extends AbstractExecutionService): void { + protected terminateJob(jobWrapper: Job): void { jobWrapper.worker.kill(); } } diff --git a/packages/snaps-controllers/src/services/node/NodeThreadExecutionService.ts b/packages/snaps-controllers/src/services/node/NodeThreadExecutionService.ts index 4b774efa42..77e7329674 100644 --- a/packages/snaps-controllers/src/services/node/NodeThreadExecutionService.ts +++ b/packages/snaps-controllers/src/services/node/NodeThreadExecutionService.ts @@ -6,7 +6,7 @@ import { import { AbstractExecutionService, Job } from '..'; export class NodeThreadExecutionService extends AbstractExecutionService { - protected async _initEnvStream(): Promise<{ + protected async initEnvStream(): Promise<{ worker: Worker; stream: BasePostMessageStream; }> { @@ -19,7 +19,7 @@ export class NodeThreadExecutionService extends AbstractExecutionService return { worker, stream }; } - protected _terminate(jobWrapper: Job): void { + protected terminateJob(jobWrapper: Job): void { jobWrapper.worker.terminate(); } } diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.ts b/packages/snaps-controllers/src/snaps/SnapController.test.ts index 073bd3692a..e791731a6c 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.test.ts @@ -96,7 +96,7 @@ describe('SnapController', () => { expect(snapState).toStrictEqual(state); expect( // @ts-expect-error Accessing private property - snapController._snapsRuntimeData.get(MOCK_SNAP_ID).state, + snapController.snapsRuntimeData.get(MOCK_SNAP_ID).state, ).toStrictEqual( await passworder.encrypt(`stateEncryption:${MOCK_SNAP_ID}`, state), ); @@ -437,9 +437,9 @@ describe('SnapController', () => { const snap = snapController.getExpect(MOCK_SNAP_ID); await snapController.startSnap(snap.id); - (snapController as any)._maxRequestTime = 50; + (snapController as any).maxRequestTime = 50; - (service as any)._command = () => + (service as any).command = () => new Promise((resolve) => { setTimeout(resolve, 2000); }); @@ -549,7 +549,7 @@ describe('SnapController', () => { jest.spyOn(messenger, 'publish'); jest - .spyOn(snapController as any, '_fetchSnap') + .spyOn(snapController as any, 'fetchSnap') .mockImplementationOnce(async () => { return { manifest: getSnapManifest({ @@ -692,7 +692,7 @@ describe('SnapController', () => { }); jest - .spyOn(controller as any, '_fetchSnap') + .spyOn(controller as any, 'fetchSnap') .mockImplementationOnce(async () => { return { manifest: getSnapManifest(), @@ -729,7 +729,7 @@ describe('SnapController', () => { jest.spyOn(messenger, 'publish'); jest - .spyOn(snapController as any, '_fetchSnap') + .spyOn(snapController as any, 'fetchSnap') .mockImplementationOnce(async () => { return { manifest: getSnapManifest(), @@ -902,7 +902,7 @@ describe('SnapController', () => { expect(snapController.state.snaps[snap.id].status).toStrictEqual('running'); // We set the maxRequestTime to a low enough value for it to time out - (snapController as any)._maxRequestTime = 50; + (snapController as any).maxRequestTime = 50; await expect( snapController.handleRequest({ @@ -973,7 +973,7 @@ describe('SnapController', () => { expect(snapController.state.snaps[snap.id].status).toStrictEqual('running'); // Max request time should be shorter than eth_blockNumber takes to respond - (snapController as any)._maxRequestTime = 300; + (snapController as any).maxRequestTime = 300; expect( await snapController.handleRequest({ @@ -1042,7 +1042,7 @@ describe('SnapController', () => { expect(snapController.state.snaps[snap.id].status).toStrictEqual('running'); // Max request time should be shorter than eth_blockNumber takes to respond - (snapController as any)._maxRequestTime = 300; + (snapController as any).maxRequestTime = 300; expect( await snapController.handleRequest({ @@ -1093,7 +1093,7 @@ describe('SnapController', () => { expect(snapController.state.snaps[snap.id].status).toStrictEqual('running'); // We set the maxRequestTime to a low enough value for it to time out if it werent a long running snap - (snapController as any)._maxRequestTime = 50; + (snapController as any).maxRequestTime = 50; const handlerPromise = snapController.handleRequest({ snapId: snap.id, @@ -1555,9 +1555,7 @@ describe('SnapController', () => { return false; }); - const addMock = jest - .spyOn(snapController as any, '_add') - .mockImplementation(); + const authorizeSpy = jest.spyOn(snapController as any, 'authorize'); const result = await snapController.installSnaps(MOCK_ORIGIN, { [MOCK_SNAP_ID]: {}, }); @@ -1568,7 +1566,7 @@ describe('SnapController', () => { MOCK_ORIGIN, snapObject.permissionName, ); - expect(addMock).not.toHaveBeenCalled(); + expect(authorizeSpy).not.toHaveBeenCalled(); }); it('reinstalls local snaps even if they are already installed (already stopped)', async () => { @@ -1605,7 +1603,7 @@ describe('SnapController', () => { }); const fetchSnapMock = jest - .spyOn(snapController as any, '_fetchSnap') + .spyOn(snapController as any, 'fetchSnap') .mockImplementationOnce(() => { return { ...snapObject, @@ -1711,7 +1709,7 @@ describe('SnapController', () => { }); const fetchSnapMock = jest - .spyOn(snapController as any, '_fetchSnap') + .spyOn(snapController as any, 'fetchSnap') .mockImplementationOnce(() => ({ manifest, sourceCode: DEFAULT_SNAP_BUNDLE, @@ -1881,7 +1879,7 @@ describe('SnapController', () => { }); const fetchSnapMock = jest - .spyOn(snapController as any, '_fetchSnap') + .spyOn(snapController as any, 'fetchSnap') .mockImplementationOnce(() => { return getPersistedSnapObject({ manifest }); }); @@ -1970,7 +1968,7 @@ describe('SnapController', () => { const callActionMock = jest.spyOn(messenger, 'call'); jest - .spyOn(snapController as any, '_fetchSnap') + .spyOn(snapController as any, 'fetchSnap') .mockImplementationOnce(() => { return getPersistedSnapObject({ manifest }); }); @@ -2043,7 +2041,7 @@ describe('SnapController', () => { const callActionMock = jest.spyOn(messenger, 'call'); jest - .spyOn(snapController as any, '_fetchSnap') + .spyOn(snapController as any, 'fetchSnap') .mockImplementationOnce(() => { return getPersistedSnapObject({ manifest }); }); @@ -2136,7 +2134,7 @@ describe('SnapController', () => { }); jest - .spyOn(snapController as any, '_fetchSnap') + .spyOn(snapController as any, 'fetchSnap') .mockImplementationOnce(() => { return getPersistedSnapObject({ manifest }); }); @@ -2224,7 +2222,7 @@ describe('SnapController', () => { ); const fetchSnapMock = jest - .spyOn(controller as any, '_fetchSnap') + .spyOn(controller as any, 'fetchSnap') .mockImplementationOnce(async () => ({ manifest: getSnapManifest(), sourceCode: DEFAULT_SNAP_BUNDLE, @@ -2363,7 +2361,7 @@ describe('SnapController', () => { }); const fetchSnapMock = jest - .spyOn(controller as any, '_fetchSnap') + .spyOn(controller as any, 'fetchSnap') .mockImplementationOnce(async () => ({ manifest: getSnapManifest({ version: newVersion }), sourceCode: DEFAULT_SNAP_BUNDLE, @@ -2410,7 +2408,7 @@ describe('SnapController', () => { }); const fetchSnapMock = jest - .spyOn(controller as any, '_fetchSnap') + .spyOn(controller as any, 'fetchSnap') .mockImplementationOnce(async () => { throw new Error('foo'); }); @@ -2468,7 +2466,7 @@ describe('SnapController', () => { }, }), ); - const fetchSnapSpy = jest.spyOn(controller as any, '_fetchSnap'); + const fetchSnapSpy = jest.spyOn(controller as any, 'fetchSnap'); fetchSnapSpy.mockImplementationOnce(async () => { const manifest: SnapManifest = { @@ -2500,7 +2498,7 @@ describe('SnapController', () => { }, }), ); - const fetchSnapSpy = jest.spyOn(controller as any, '_fetchSnap'); + const fetchSnapSpy = jest.spyOn(controller as any, 'fetchSnap'); const onSnapUpdated = jest.fn(); const onSnapAdded = jest.fn(); @@ -2536,7 +2534,7 @@ describe('SnapController', () => { const controller = getSnapController( getSnapControllerOptions({ messenger }), ); - const fetchSnapSpy = jest.spyOn(controller as any, '_fetchSnap'); + const fetchSnapSpy = jest.spyOn(controller as any, 'fetchSnap'); const callActionSpy = jest.spyOn(messenger, 'call'); const onSnapUpdated = jest.fn(); const onSnapAdded = jest.fn(); @@ -2674,7 +2672,7 @@ describe('SnapController', () => { }, }), ); - const fetchSnapSpy = jest.spyOn(controller as any, '_fetchSnap'); + const fetchSnapSpy = jest.spyOn(controller as any, 'fetchSnap'); const callActionSpy = jest.spyOn(messenger, 'call'); fetchSnapSpy.mockImplementationOnce(async () => { @@ -2701,7 +2699,6 @@ describe('SnapController', () => { await controller.startSnap(MOCK_SNAP_ID); - const startSnapSpy = jest.spyOn(controller as any, '_startSnap'); const stopSnapSpy = jest.spyOn(controller as any, 'stopSnap'); await controller.updateSnap(MOCK_ORIGIN, MOCK_SNAP_ID); @@ -2768,8 +2765,6 @@ describe('SnapController', () => { ); expect(isRunning).toStrictEqual(true); expect(stopSnapSpy).toHaveBeenCalledTimes(1); - expect(startSnapSpy).toHaveBeenCalledTimes(1); - expect(stopSnapSpy).toHaveBeenCalledTimes(1); }); it('returns null on update request denied', async () => { @@ -2782,7 +2777,7 @@ describe('SnapController', () => { }, }), ); - const fetchSnapSpy = jest.spyOn(controller as any, '_fetchSnap'); + const fetchSnapSpy = jest.spyOn(controller as any, 'fetchSnap'); const callActionSpy = jest.spyOn(messenger, 'call'); fetchSnapSpy.mockImplementationOnce(async () => { @@ -2876,7 +2871,7 @@ describe('SnapController', () => { }, }; - const fetchSnapSpy = jest.spyOn(controller as any, '_fetchSnap'); + const fetchSnapSpy = jest.spyOn(controller as any, 'fetchSnap'); const callActionSpy = jest.spyOn(messenger, 'call'); fetchSnapSpy @@ -3033,7 +3028,7 @@ describe('SnapController', () => { }, }; - const fetchSnapSpy = jest.spyOn(snapController as any, '_fetchSnap'); + const fetchSnapSpy = jest.spyOn(snapController as any, 'fetchSnap'); const callActionSpy = jest.spyOn(messenger, 'call'); fetchSnapSpy @@ -3080,7 +3075,7 @@ describe('SnapController', () => { }); }); - describe('_fetchSnap', () => { + describe('fetchSnap', () => { it('can fetch NPM snaps', async () => { const controller = getSnapController(); @@ -3679,7 +3674,7 @@ describe('SnapController', () => { expect(updateSnapStateSpy).toHaveBeenCalledTimes(1); expect( // @ts-expect-error Accessing private property - snapController._snapsRuntimeData.get(MOCK_SNAP_ID).state, + snapController.snapsRuntimeData.get(MOCK_SNAP_ID).state, ).toStrictEqual( await passworder.encrypt(`stateEncryption:${MOCK_SNAP_ID}`, state), ); @@ -3721,11 +3716,11 @@ describe('SnapController', () => { expect(updateSnapStateSpy).toHaveBeenCalledTimes(2); const snapState1 = // @ts-expect-error Accessing private property - snapController._snapsRuntimeData.get(MOCK_SNAP_ID).state; + snapController.snapsRuntimeData.get(MOCK_SNAP_ID).state; const snapState2 = // @ts-expect-error Accessing private property - snapController._snapsRuntimeData.get(MOCK_LOCAL_SNAP_ID).state; + snapController.snapsRuntimeData.get(MOCK_LOCAL_SNAP_ID).state; expect(snapState1).toStrictEqual( await passworder.encrypt(`stateEncryption:${MOCK_SNAP_ID}`, state), @@ -3746,7 +3741,7 @@ describe('SnapController', () => { it('clears the state of a snap', async () => { const messenger = getSnapControllerMessenger(undefined, false); - const snapController = getSnapController( + getSnapController( getSnapControllerOptions({ messenger, state: { @@ -3760,14 +3755,11 @@ describe('SnapController', () => { }), ); - const clearSnapStateSpy = jest.spyOn(snapController, 'clearSnapState'); await messenger.call('SnapController:clearSnapState', MOCK_SNAP_ID); const clearedState = await messenger.call( 'SnapController:getSnapState', MOCK_SNAP_ID, ); - expect(clearSnapStateSpy).toHaveBeenCalledTimes(1); - expect(snapController.state.snapStates).toStrictEqual({}); expect(clearedState).toBeNull(); }); }); diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index b8bd46d36a..e2a221f1f9 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -171,7 +171,7 @@ export type SnapError = { }; /** - * The return type of {@link SnapController._fetchSnap} and its sibling methods. + * The return type of {@link SnapController.fetchSnap} and its sibling methods. */ type FetchSnapResult = { /** @@ -597,31 +597,33 @@ export class SnapController extends BaseController< SnapControllerState, SnapControllerMessenger > { - private _closeAllConnections: CloseAllConnectionsFunction; + #closeAllConnections: CloseAllConnectionsFunction; - private _environmentEndowmentPermissions: string[]; + #environmentEndowmentPermissions: string[]; - private _featureFlags: FeatureFlags; + #featureFlags: FeatureFlags; - private _fetchFunction: typeof fetch; + #fetchFunction: typeof fetch; - private _idleTimeCheckInterval: number; + #idleTimeCheckInterval: number; - private _checkSnapBlockList: CheckSnapBlockList; + #checkSnapBlockList: CheckSnapBlockList; - private _maxIdleTime: number; + #maxIdleTime: number; - private _maxRequestTime: number; + // This property cannot be hash private yet because of tests. + private maxRequestTime: number; - private _npmRegistryUrl?: string; + #npmRegistryUrl?: string; - private _snapsRuntimeData: Map; + // This property cannot be hash private yet because of tests. + private snapsRuntimeData: Map; - private _getAppKey: GetAppKey; + #getAppKey: GetAppKey; - private _timeoutForLastRequestStatus?: number; + #timeoutForLastRequestStatus?: number; - private _statusMachine!: StateMachine.Machine< + #statusMachine!: StateMachine.Machine< StatusContext, StatusEvents, StatusStates @@ -641,20 +643,6 @@ export class SnapController extends BaseController< fetchFunction = globalThis.fetch.bind(globalThis), featureFlags = {}, }: SnapControllerArgs) { - const loadedSourceCode: Record = {}; - const filteredState = { - ...state, - snaps: Object.values(state?.snaps ?? {}).reduce( - (memo: Record, snap) => { - const { sourceCode, ...rest } = snap; - loadedSourceCode[snap.id] = sourceCode; - memo[snap.id] = rest; - return memo; - }, - {}, - ), - snapStates: {}, - }; super({ messenger, metadata: { @@ -667,7 +655,7 @@ export class SnapController extends BaseController< return Object.keys(this.state.snaps).reduce< Record >((acc, cur) => { - acc[cur] = this.getRuntimeExpect(cur).state; + acc[cur] = this.#getRuntimeExpect(cur).state; return acc; }, {}); }, @@ -679,7 +667,7 @@ export class SnapController extends BaseController< .map((snap) => { return { ...snap, - sourceCode: this.getRuntimeExpect(snap.id).sourceCode, + sourceCode: this.#getRuntimeExpect(snap.id).sourceCode, // At the time state is rehydrated, no snap will be running. status: SnapStatus.Stopped, }; @@ -693,26 +681,39 @@ export class SnapController extends BaseController< }, }, name, - state: { ...defaultState, ...filteredState }, + state: { + ...defaultState, + ...{ + ...state, + snaps: Object.values(state?.snaps ?? {}).reduce( + (memo: Record, snap) => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { sourceCode, ...rest } = snap; + memo[snap.id] = rest; + return memo; + }, + {}, + ), + }, + }, }); - this._closeAllConnections = closeAllConnections; - this._environmentEndowmentPermissions = environmentEndowmentPermissions; - this._featureFlags = featureFlags; - this._fetchFunction = fetchFunction; - this._onUnhandledSnapError = this._onUnhandledSnapError.bind(this); - this._getAppKey = getAppKey; - this._idleTimeCheckInterval = idleTimeCheckInterval; - this._checkSnapBlockList = checkBlockList; - this._maxIdleTime = maxIdleTime; - this._maxRequestTime = maxRequestTime; - this._npmRegistryUrl = npmRegistryUrl; + this.#closeAllConnections = closeAllConnections; + this.#environmentEndowmentPermissions = environmentEndowmentPermissions; + this.#featureFlags = featureFlags; + this.#fetchFunction = fetchFunction; + this.#getAppKey = getAppKey; + this.#idleTimeCheckInterval = idleTimeCheckInterval; + this.#checkSnapBlockList = checkBlockList; + this.#maxIdleTime = maxIdleTime; + this.maxRequestTime = maxRequestTime; + this.#npmRegistryUrl = npmRegistryUrl; this._onUnhandledSnapError = this._onUnhandledSnapError.bind(this); this._onOutboundRequest = this._onOutboundRequest.bind(this); this._onOutboundResponse = this._onOutboundResponse.bind(this); - this._snapsRuntimeData = new Map(); + this.snapsRuntimeData = new Map(); - this._pollForLastRequestStatus(); + this.#pollForLastRequestStatus(); this.messagingSystem.subscribe( 'ExecutionService:unhandledError', @@ -729,13 +730,13 @@ export class SnapController extends BaseController< this._onOutboundResponse, ); - this.initializeStateMachine(); - this.registerMessageHandlers(); + this.#initializeStateMachine(); + this.#registerMessageHandlers(); - Object.keys(filteredState.snaps).forEach((id) => - this.setupRuntime(id, { - sourceCode: loadedSourceCode[id], - state: state?.snapStates?.[id] ?? null, + Object.values(state?.snaps ?? {}).forEach((snap) => + this.#setupRuntime(snap.id, { + sourceCode: snap.sourceCode, + state: state?.snapStates?.[snap.id] ?? null, }), ); } @@ -749,7 +750,7 @@ export class SnapController extends BaseController< // We initialize the machine in the instance because the status is currently tightly coupled // with the SnapController - the guard checks for enabled status inside the SnapController state. // In the future, side-effects could be added to the machine during transitions. - private initializeStateMachine() { + #initializeStateMachine() { const disableGuard = ({ snapId }: StatusContext) => { return this.getExpect(snapId).enabled; }; @@ -802,15 +803,15 @@ export class SnapController extends BaseController< }, }, }; - this._statusMachine = createMachine(statusConfig); - validateMachine(this._statusMachine); + this.#statusMachine = createMachine(statusConfig); + validateMachine(this.#statusMachine); } /** * Constructor helper for registering the controller's messaging system * actions. */ - private registerMessageHandlers(): void { + #registerMessageHandlers(): void { this.messagingSystem.registerActionHandler( `${controllerName}:clearSnapState`, (...args) => this.clearSnapState(...args), @@ -892,11 +893,11 @@ export class SnapController extends BaseController< ); } - _pollForLastRequestStatus() { - this._timeoutForLastRequestStatus = setTimeout(async () => { - await this._stopSnapsLastRequestPastMax(); - this._pollForLastRequestStatus(); - }, this._idleTimeCheckInterval) as unknown as number; + #pollForLastRequestStatus() { + this.#timeoutForLastRequestStatus = setTimeout(async () => { + await this.#stopSnapsLastRequestPastMax(); + this.#pollForLastRequestStatus(); + }, this.#idleTimeCheckInterval) as unknown as number; } /** @@ -905,7 +906,7 @@ export class SnapController extends BaseController< * for more information. */ async updateBlockedSnaps(): Promise { - const blockedSnaps = await this._checkSnapBlockList( + const blockedSnaps = await this.#checkSnapBlockList( Object.values(this.state.snaps).reduce( (blockListArg, snap) => { blockListArg[snap.id] = { @@ -922,10 +923,10 @@ export class SnapController extends BaseController< Object.entries(blockedSnaps).map( ([snapId, { blocked, ...blockData }]) => { if (blocked) { - return this._blockSnap(snapId, blockData); + return this.#blockSnap(snapId, blockData); } - return this._unblockSnap(snapId); + return this.#unblockSnap(snapId); }, ), ); @@ -938,7 +939,7 @@ export class SnapController extends BaseController< * @param snapId - The snap to block. * @param blockedSnapInfo - Information detailing why the snap is blocked. */ - private async _blockSnap( + async #blockSnap( snapId: SnapId, blockedSnapInfo: BlockedSnapInfo, ): Promise { @@ -974,7 +975,7 @@ export class SnapController extends BaseController< * * @param snapId - The id of the snap to unblock. */ - private async _unblockSnap(snapId: SnapId): Promise { + async #unblockSnap(snapId: SnapId): Promise { if (!this.has(snapId) || !this.state.snaps[snapId].blocked) { return; } @@ -998,7 +999,7 @@ export class SnapController extends BaseController< snapId: ValidatedSnapId, snapInfo: SnapInfo, ): Promise { - const result = await this._checkSnapBlockList({ + const result = await this.#checkSnapBlockList({ [snapId]: snapInfo, }); return result[snapId].blocked; @@ -1011,10 +1012,7 @@ export class SnapController extends BaseController< * @param snapId - The id of the snap to check. * @param snapInfo - Snap information containing version and shasum. */ - private async _assertIsUnblocked( - snapId: ValidatedSnapId, - snapInfo: SnapInfo, - ) { + async #assertIsUnblocked(snapId: ValidatedSnapId, snapInfo: SnapInfo) { if (await this.isBlocked(snapId, snapInfo)) { throw new Error( `Cannot install version "${snapInfo.version}" of snap "${snapId}": the version is blocked.`, @@ -1022,8 +1020,8 @@ export class SnapController extends BaseController< } } - async _stopSnapsLastRequestPastMax() { - const entries = [...this._snapsRuntimeData.entries()]; + async #stopSnapsLastRequestPastMax() { + const entries = [...this.snapsRuntimeData.entries()]; return Promise.all( entries .filter( @@ -1032,8 +1030,8 @@ export class SnapController extends BaseController< runtime.pendingInboundRequests.length === 0 && // lastRequest should always be set here but TypeScript wants this check runtime.lastRequest && - this._maxIdleTime && - timeSince(runtime.lastRequest) > this._maxIdleTime, + this.#maxIdleTime && + timeSince(runtime.lastRequest) > this.#maxIdleTime, ) .map(([snapId]) => this.stopSnap(snapId, SnapStatusEvents.Stop)), ); @@ -1045,7 +1043,7 @@ export class SnapController extends BaseController< } async _onOutboundRequest(snapId: SnapId) { - const runtime = this.getRuntimeExpect(snapId); + const runtime = this.#getRuntimeExpect(snapId); // Ideally we would only pause the pending request that is making the outbound request // but right now we don't have a way to know which request initiated the outbound request runtime.pendingInboundRequests @@ -1055,7 +1053,7 @@ export class SnapController extends BaseController< } async _onOutboundResponse(snapId: SnapId) { - const runtime = this.getRuntimeExpect(snapId); + const runtime = this.#getRuntimeExpect(snapId); runtime.pendingOutboundRequests -= 1; if (runtime.pendingOutboundRequests === 0) { runtime.pendingInboundRequests @@ -1076,11 +1074,8 @@ export class SnapController extends BaseController< * @param snapId - The id of the snap to transition. * @param event - The event enum to use to transition. */ - private transition( - snapId: SnapId, - event: StatusEvents | StatusEvents['type'], - ) { - const { interpreter } = this.getRuntimeExpect(snapId); + #transition(snapId: SnapId, event: StatusEvents | StatusEvents['type']) { + const { interpreter } = this.#getRuntimeExpect(snapId); interpreter.send(event); this.update((state: any) => { state.snaps[snapId].status = interpreter.state.value; @@ -1094,7 +1089,7 @@ export class SnapController extends BaseController< * @param snapId - The id of the Snap to start. */ async startSnap(snapId: SnapId): Promise { - const runtime = this.getRuntimeExpect(snapId); + const runtime = this.#getRuntimeExpect(snapId); if (this.state.snaps[snapId].enabled === false) { throw new Error(`Snap "${snapId}" is disabled.`); @@ -1102,7 +1097,7 @@ export class SnapController extends BaseController< assert(runtime.sourceCode); - await this._startSnap({ + await this.#startSnap({ snapId, sourceCode: runtime.sourceCode, }); @@ -1162,7 +1157,7 @@ export class SnapController extends BaseController< | SnapStatusEvents.Stop | SnapStatusEvents.Crash = SnapStatusEvents.Stop, ): Promise { - const runtime = this.getRuntime(snapId); + const runtime = this.#getRuntime(snapId); if (!runtime) { throw new Error(`The snap "${snapId}" is not running.`); } @@ -1173,12 +1168,12 @@ export class SnapController extends BaseController< runtime.pendingOutboundRequests = 0; try { if (this.isRunning(snapId)) { - this._closeAllConnections(snapId); - await this.terminateSnap(snapId); + this.#closeAllConnections(snapId); + await this.#terminateSnap(snapId); } } finally { if (this.isRunning(snapId)) { - this.transition(snapId, statusEvent); + this.#transition(snapId, statusEvent); } } } @@ -1188,7 +1183,7 @@ export class SnapController extends BaseController< * * @param snapId - The snap to terminate. */ - private async terminateSnap(snapId: SnapId) { + async #terminateSnap(snapId: SnapId) { await this.messagingSystem.call('ExecutionService:terminateSnap', snapId); this.messagingSystem.publish( 'SnapController:snapTerminated', @@ -1278,8 +1273,8 @@ export class SnapController extends BaseController< * @param newSnapState - The new state of the snap. */ async updateSnapState(snapId: SnapId, newSnapState: Json): Promise { - const encrypted = await this.encryptSnapState(snapId, newSnapState); - const runtime = this.getRuntimeExpect(snapId); + const encrypted = await this.#encryptSnapState(snapId, newSnapState); + const runtime = this.#getRuntimeExpect(snapId); runtime.state = encrypted; } @@ -1290,7 +1285,7 @@ export class SnapController extends BaseController< * @param snapId - The id of the Snap whose state should be cleared. */ async clearSnapState(snapId: SnapId): Promise { - const runtime = this.getRuntimeExpect(snapId); + const runtime = this.#getRuntimeExpect(snapId); runtime.state = null; } @@ -1339,24 +1334,21 @@ export class SnapController extends BaseController< * @throws If the snap state decryption fails. */ async getSnapState(snapId: SnapId): Promise { - const { state } = this.getRuntimeExpect(snapId); - return state ? this.decryptSnapState(snapId, state) : null; + const { state } = this.#getRuntimeExpect(snapId); + return state ? this.#decryptSnapState(snapId, state) : null; } - private async getEncryptionKey(snapId: SnapId): Promise { - return this._getAppKey(snapId, AppKeyType.stateEncryption); + async #getEncryptionKey(snapId: SnapId): Promise { + return this.#getAppKey(snapId, AppKeyType.stateEncryption); } - private async encryptSnapState(snapId: SnapId, state: Json): Promise { - const appKey = await this.getEncryptionKey(snapId); + async #encryptSnapState(snapId: SnapId, state: Json): Promise { + const appKey = await this.#getEncryptionKey(snapId); return passworder.encrypt(appKey, state); } - private async decryptSnapState( - snapId: SnapId, - encrypted: string, - ): Promise { - const appKey = await this.getEncryptionKey(snapId); + async #decryptSnapState(snapId: SnapId, encrypted: string): Promise { + const appKey = await this.#getEncryptionKey(snapId); try { return await passworder.decrypt(appKey, encrypted); } catch (err) { @@ -1373,7 +1365,7 @@ export class SnapController extends BaseController< clearState() { const snapIds = Object.keys(this.state.snaps); snapIds.forEach((snapId) => { - this._closeAllConnections(snapId); + this.#closeAllConnections(snapId); }); this.messagingSystem.call('ExecutionService:terminateAllSnaps'); snapIds.forEach(this.revokeAllSnapPermissions); @@ -1422,7 +1414,7 @@ export class SnapController extends BaseController< permissionName, ); - this._snapsRuntimeData.delete(snapId); + this.snapsRuntimeData.delete(snapId); this.update((state: any) => { delete state.snaps[snapId]; @@ -1459,7 +1451,7 @@ export class SnapController extends BaseController< * @param snapId - The snap id of the snap that was referenced. */ incrementActiveReferences(snapId: SnapId) { - const runtime = this.getRuntimeExpect(snapId); + const runtime = this.#getRuntimeExpect(snapId); runtime.activeReferences += 1; } @@ -1469,7 +1461,7 @@ export class SnapController extends BaseController< * @param snapId - The snap id of the snap that was referenced.. */ decrementActiveReferences(snapId: SnapId) { - const runtime = this.getRuntimeExpect(snapId); + const runtime = this.#getRuntimeExpect(snapId); assert( runtime.activeReferences > 0, 'SnapController reference management is in an invalid state.', @@ -1600,7 +1592,7 @@ export class SnapController extends BaseController< return existingSnap; } - if (this._featureFlags.dappsCanUpdateSnaps === true) { + if (this.#featureFlags.dappsCanUpdateSnaps === true) { try { const updateResult = await this.updateSnap( origin, @@ -1633,7 +1625,7 @@ export class SnapController extends BaseController< } try { - const { sourceCode } = await this._add({ + const { sourceCode } = await this.#add({ origin, id: snapId, versionRange, @@ -1641,7 +1633,7 @@ export class SnapController extends BaseController< await this.authorize(origin, snapId); - await this._startSnap({ + await this.#startSnap({ snapId, sourceCode, }); @@ -1690,7 +1682,7 @@ export class SnapController extends BaseController< ); } - const newSnap = await this._fetchSnap(snapId, newVersionRange); + const newSnap = await this.fetchSnap(snapId, newVersionRange); const newVersion = newSnap.manifest.version; if (!gtVersion(newVersion, snap.version)) { console.warn( @@ -1699,17 +1691,17 @@ export class SnapController extends BaseController< return null; } - await this._assertIsUnblocked(snapId, { + await this.#assertIsUnblocked(snapId, { version: newVersion, shasum: newSnap.manifest.source.shasum, }); - const processedPermissions = this.processSnapPermissions( + const processedPermissions = this.#processSnapPermissions( newSnap.manifest.initialPermissions, ); const { newPermissions, unusedPermissions, approvedPermissions } = - await this.calculatePermissionsChange(snapId, processedPermissions); + await this.#calculatePermissionsChange(snapId, processedPermissions); const id = nanoid(); const { permissions: approvedNewPermissions, ...requestData } = @@ -1737,9 +1729,9 @@ export class SnapController extends BaseController< await this.stopSnap(snapId, SnapStatusEvents.Stop); } - this.transition(snapId, SnapStatusEvents.Update); + this.#transition(snapId, SnapStatusEvents.Update); - this._set({ + this.#set({ origin, id: snapId, manifest: newSnap.manifest, @@ -1765,7 +1757,7 @@ export class SnapController extends BaseController< }); } - await this._startSnap({ snapId, sourceCode: newSnap.sourceCode }); + await this.#startSnap({ snapId, sourceCode: newSnap.sourceCode }); const truncatedSnap = this.getTruncatedExpect(snapId); @@ -1786,7 +1778,7 @@ export class SnapController extends BaseController< * version. * @returns The resulting snap object. */ - private async _add(args: AddSnapArgs): Promise { + async #add(args: AddSnapArgs): Promise { const { id: snapId } = args; validateSnapId(snapId); @@ -1799,8 +1791,8 @@ export class SnapController extends BaseController< ) { throw new Error(`Invalid add snap args for snap "${snapId}".`); } - this.setupRuntime(snapId, { sourceCode: null, state: null }); - const runtime = this.getRuntimeExpect(snapId); + this.#setupRuntime(snapId, { sourceCode: null, state: null }); + const runtime = this.#getRuntimeExpect(snapId); if (!runtime.installPromise) { console.info(`Adding snap: ${snapId}`); @@ -1808,16 +1800,16 @@ export class SnapController extends BaseController< // to null in the authorize() method. runtime.installPromise = (async () => { if ('manifest' in args && 'sourceCode' in args) { - return this._set({ ...args, id: snapId }); + return this.#set({ ...args, id: snapId }); } - const fetchedSnap = await this._fetchSnap(snapId, args.versionRange); - await this._assertIsUnblocked(snapId, { + const fetchedSnap = await this.fetchSnap(snapId, args.versionRange); + await this.#assertIsUnblocked(snapId, { version: fetchedSnap.manifest.version, shasum: fetchedSnap.manifest.source.shasum, }); - return this._set({ + return this.#set({ ...args, ...fetchedSnap, id: snapId, @@ -1835,24 +1827,24 @@ export class SnapController extends BaseController< } } - private async _startSnap(snapData: { snapId: string; sourceCode: string }) { + async #startSnap(snapData: { snapId: string; sourceCode: string }) { const { snapId } = snapData; if (this.isRunning(snapId)) { throw new Error(`Snap "${snapId}" is already started.`); } try { - const result = await this._executeWithTimeout( + const result = await this.#executeWithTimeout( snapId, this.messagingSystem.call('ExecutionService:executeSnap', { ...snapData, - endowments: await this._getEndowments(snapId), + endowments: await this.#getEndowments(snapId), }), ); - this.transition(snapId, SnapStatusEvents.Start); + this.#transition(snapId, SnapStatusEvents.Start); return result; } catch (err) { - await this.terminateSnap(snapId); + await this.#terminateSnap(snapId); throw err; } } @@ -1868,10 +1860,10 @@ export class SnapController extends BaseController< * @param snapId - The id of the snap whose SES endowments to get. * @returns An array of the names of the endowments. */ - private async _getEndowments(snapId: string): Promise { + async #getEndowments(snapId: string): Promise { let allEndowments: string[] = []; - for (const permissionName of this._environmentEndowmentPermissions) { + for (const permissionName of this.#environmentEndowmentPermissions) { if ( await this.messagingSystem.call( 'PermissionController:hasPermission', @@ -1930,7 +1922,7 @@ export class SnapController extends BaseController< * @param args - The add snap args. * @returns The resulting snap object. */ - private _set(args: SetSnapArgs): PersistedSnap { + #set(args: SetSnapArgs): PersistedSnap { const { id: snapId, origin, @@ -1991,7 +1983,7 @@ export class SnapController extends BaseController< id: snapId, initialPermissions, manifest, - status: this._statusMachine.config.initial as StatusStates['value'], + status: this.#statusMachine.config.initial as StatusStates['value'], version, versionHistory, }; @@ -2003,7 +1995,7 @@ export class SnapController extends BaseController< state.snaps[snapId] = snap; }); - const runtime = this.getRuntimeExpect(snapId); + const runtime = this.#getRuntimeExpect(snapId); runtime.sourceCode = sourceCode; this.messagingSystem.publish(`SnapController:snapAdded`, snap, svgIcon); @@ -2013,11 +2005,13 @@ export class SnapController extends BaseController< /** * Fetches the manifest and source code of a snap. * + * This function is not hash private yet because of tests. + * * @param snapId - The id of the Snap. * @param versionRange - The SemVer version of the Snap to fetch. * @returns A tuple of the Snap manifest object and the Snap source code. */ - private async _fetchSnap( + async fetchSnap( snapId: ValidatedSnapId, versionRange: string = DEFAULT_REQUESTED_SNAP_VERSION, ): Promise { @@ -2025,9 +2019,9 @@ export class SnapController extends BaseController< const snapPrefix = getSnapPrefix(snapId); switch (snapPrefix) { case SnapIdPrefixes.local: - return this._fetchLocalSnap(snapId.replace(SnapIdPrefixes.local, '')); + return this.#fetchLocalSnap(snapId.replace(SnapIdPrefixes.local, '')); case SnapIdPrefixes.npm: - return this._fetchNpmSnap( + return this.#fetchNpmSnap( snapId.replace(SnapIdPrefixes.npm, ''), versionRange, ); @@ -2043,7 +2037,7 @@ export class SnapController extends BaseController< } } - private async _fetchNpmSnap( + async #fetchNpmSnap( packageName: string, versionRange: string, ): Promise { @@ -2056,8 +2050,8 @@ export class SnapController extends BaseController< const { manifest, sourceCode, svgIcon } = await fetchNpmSnap( packageName, versionRange, - this._npmRegistryUrl, - this._fetchFunction, + this.#npmRegistryUrl, + this.#fetchFunction, ); return { manifest, sourceCode, svgIcon }; } @@ -2068,9 +2062,7 @@ export class SnapController extends BaseController< * @param localhostUrl - The localhost URL to download from. * @returns The validated manifest and the source code. */ - private async _fetchLocalSnap( - localhostUrl: string, - ): Promise { + async #fetchLocalSnap(localhostUrl: string): Promise { // Local snaps are mostly used for development purposes. Fetches were cached in the browser and were not requested // afterwards which lead to confusing development where old versions of snaps were installed. // Thus we disable caching @@ -2083,7 +2075,7 @@ export class SnapController extends BaseController< } const manifest = await ( - await this._fetchFunction(manifestUrl.toString(), fetchOptions) + await this.#fetchFunction(manifestUrl.toString(), fetchOptions) ).json(); assertIsSnapManifest(manifest); @@ -2097,14 +2089,14 @@ export class SnapController extends BaseController< const [sourceCode, svgIcon] = await Promise.all([ ( - await this._fetchFunction( + await this.#fetchFunction( new URL(filePath, localhostUrl).toString(), fetchOptions, ) ).text(), iconPath ? ( - await this._fetchFunction( + await this.#fetchFunction( new URL(iconPath, localhostUrl).toString(), fetchOptions, ) @@ -2128,7 +2120,7 @@ export class SnapController extends BaseController< * @returns The processed permissions. * @private */ - private processSnapPermissions( + #processSnapPermissions( initialPermissions: SnapPermissions, ): Record< string, @@ -2155,6 +2147,8 @@ export class SnapController extends BaseController< * Initiates a request for the given snap's initial permissions. * Must be called in order. See processRequestedSnap. * + * This function is not hash private yet because of tests. + * * @param origin - The origin of the install request. * @param snapId - The id of the Snap. * @returns The snap's approvedPermissions. @@ -2167,7 +2161,7 @@ export class SnapController extends BaseController< try { const processedPermissions = - this.processSnapPermissions(initialPermissions); + this.#processSnapPermissions(initialPermissions); const id = nanoid(); const { permissions: approvedPermissions, ...requestData } = (await this.messagingSystem.call( @@ -2197,7 +2191,7 @@ export class SnapController extends BaseController< ); } } finally { - const runtime = this.getRuntimeExpect(snapId); + const runtime = this.#getRuntimeExpect(snapId); runtime.installPromise = null; } } @@ -2205,8 +2199,8 @@ export class SnapController extends BaseController< destroy() { super.destroy(); - if (this._timeoutForLastRequestStatus) { - clearTimeout(this._timeoutForLastRequestStatus); + if (this.#timeoutForLastRequestStatus) { + clearTimeout(this.#timeoutForLastRequestStatus); } this.messagingSystem.unsubscribe( @@ -2241,7 +2235,7 @@ export class SnapController extends BaseController< handler: handlerType, request, }: SnapRpcHookArgs & { snapId: SnapId }): Promise { - const handler = await this.getRpcRequestHandler(snapId); + const handler = await this.#getRpcRequestHandler(snapId); if (!handler) { throw new Error( `Snap RPC message handler not found for snap "${snapId}".`, @@ -2256,8 +2250,8 @@ export class SnapController extends BaseController< * @param snapId - The id of the Snap whose message handler to get. * @returns The RPC handler for the given snap. */ - private async getRpcRequestHandler(snapId: SnapId): Promise { - const runtime = this.getRuntimeExpect(snapId); + async #getRpcRequestHandler(snapId: SnapId): Promise { + const runtime = this.#getRuntimeExpect(snapId); const existingHandler = runtime.rpcHandler; if (existingHandler) { return existingHandler; @@ -2316,8 +2310,8 @@ export class SnapController extends BaseController< }); } - const timer = new Timer(this._maxRequestTime); - this._recordSnapRpcRequestStart(snapId, request.id, timer); + const timer = new Timer(this.maxRequestTime); + this.#recordSnapRpcRequestStart(snapId, request.id, timer); const handleRpcRequestPromise = this.messagingSystem.call( 'ExecutionService:handleRpcRequest', @@ -2327,12 +2321,12 @@ export class SnapController extends BaseController< // This will either get the result or reject due to the timeout. try { - const result = await this._executeWithTimeout( + const result = await this.#executeWithTimeout( snapId, handleRpcRequestPromise, timer, ); - this._recordSnapRpcRequestFinish(snapId, request.id); + this.#recordSnapRpcRequestFinish(snapId, request.id); return result; } catch (err) { await this.stopSnap(snapId, SnapStatusEvents.Crash); @@ -2354,7 +2348,7 @@ export class SnapController extends BaseController< * @returns The result of the promise or rejects if the promise times out. * @template PromiseValue - The value of the Promise. */ - private async _executeWithTimeout( + async #executeWithTimeout( snapId: SnapId, promise: Promise, timer?: Timer, @@ -2370,25 +2364,21 @@ export class SnapController extends BaseController< return promise; } - const result = await withTimeout(promise, timer ?? this._maxRequestTime); + const result = await withTimeout(promise, timer ?? this.maxRequestTime); if (result === hasTimedOut) { throw new Error('The request timed out.'); } return result; } - private _recordSnapRpcRequestStart( - snapId: SnapId, - requestId: unknown, - timer: Timer, - ) { - const runtime = this.getRuntimeExpect(snapId); + #recordSnapRpcRequestStart(snapId: SnapId, requestId: unknown, timer: Timer) { + const runtime = this.#getRuntimeExpect(snapId); runtime.pendingInboundRequests.push({ requestId, timer }); runtime.lastRequest = null; } - private _recordSnapRpcRequestFinish(snapId: SnapId, requestId: unknown) { - const runtime = this.getRuntimeExpect(snapId); + #recordSnapRpcRequestFinish(snapId: SnapId, requestId: unknown) { + const runtime = this.#getRuntimeExpect(snapId); runtime.pendingInboundRequests = runtime.pendingInboundRequests.filter( (r) => r.requestId !== requestId, ); @@ -2398,12 +2388,12 @@ export class SnapController extends BaseController< } } - private getRuntime(snapId: SnapId): SnapRuntimeData | undefined { - return this._snapsRuntimeData.get(snapId); + #getRuntime(snapId: SnapId): SnapRuntimeData | undefined { + return this.snapsRuntimeData.get(snapId); } - private getRuntimeExpect(snapId: SnapId): SnapRuntimeData { - const runtime = this.getRuntime(snapId); + #getRuntimeExpect(snapId: SnapId): SnapRuntimeData { + const runtime = this.#getRuntime(snapId); assert( runtime !== undefined, new Error(`Snap "${snapId}" runtime data not found`), @@ -2411,26 +2401,26 @@ export class SnapController extends BaseController< return runtime; } - private setupRuntime( + #setupRuntime( snapId: SnapId, data: { sourceCode: string | null; state: string | null }, ) { - if (this._snapsRuntimeData.has(snapId)) { + if (this.snapsRuntimeData.has(snapId)) { return; } const snap = this.get(snapId); - const interpreter = interpret(this._statusMachine); + const interpreter = interpret(this.#statusMachine); interpreter.start({ context: { snapId }, value: snap?.status ?? - (this._statusMachine.config.initial as StatusStates['value']), + (this.#statusMachine.config.initial as StatusStates['value']), }); forceStrict(interpreter); - this._snapsRuntimeData.set(snapId, { + this.snapsRuntimeData.set(snapId, { lastRequest: null, rpcHandler: null, installPromise: null, @@ -2442,7 +2432,7 @@ export class SnapController extends BaseController< }); } - private async calculatePermissionsChange( + async #calculatePermissionsChange( snapId: SnapId, desiredPermissionsSet: RequestedSnapPermissions, ): Promise<{