diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json index 3e58e6d076..e198f1a813 100644 --- a/packages/snaps-controllers/coverage.json +++ b/packages/snaps-controllers/coverage.json @@ -1,6 +1,6 @@ { - "branches": 93.54, + "branches": 93.28, "functions": 97.38, - "lines": 98.34, - "statements": 98.08 + "lines": 98.25, + "statements": 98 } diff --git a/packages/snaps-controllers/src/services/AbstractExecutionService.test.ts b/packages/snaps-controllers/src/services/AbstractExecutionService.test.ts index 2f4c2638df..31cabac869 100644 --- a/packages/snaps-controllers/src/services/AbstractExecutionService.test.ts +++ b/packages/snaps-controllers/src/services/AbstractExecutionService.test.ts @@ -164,4 +164,39 @@ describe('AbstractExecutionService', () => { }), ).rejects.toThrow(`${MOCK_SNAP_ID} failed to start.`); }); + + it('skips metamask_chainChanged JSON-RPC notifications', async () => { + const { service } = createService(MockExecutionService); + + await service.executeSnap({ + snapId: 'TestSnap', + sourceCode: ` + module.exports.onRpcRequest = () => null; + `, + endowments: [], + }); + + const { streams, worker } = service.getJobs().values().next().value; + const postMessageSpy = jest.spyOn(worker, 'postMessage'); + + streams.rpc.write({ + name: 'metamask-provider', + data: { method: 'metamask_chainChanged' }, + }); + + streams.rpc.write({ + name: 'metamask-provider', + data: { id: 'foo', result: '1' }, + }); + + expect(postMessageSpy).toHaveBeenCalledTimes(1); + expect(postMessageSpy).toHaveBeenCalledWith({ + data: { + name: 'jsonRpc', + data: { data: { id: 'foo', result: '1' }, name: 'metamask-provider' }, + }, + }); + + await service.terminateAllSnaps(); + }); }); diff --git a/packages/snaps-controllers/src/services/AbstractExecutionService.ts b/packages/snaps-controllers/src/services/AbstractExecutionService.ts index da487dca73..48eeabee86 100644 --- a/packages/snaps-controllers/src/services/AbstractExecutionService.ts +++ b/packages/snaps-controllers/src/services/AbstractExecutionService.ts @@ -209,16 +209,18 @@ export abstract class AbstractExecutionService /** * Initiates a job for a snap. * + * @param snapId - The Snap ID. * @param jobId - The ID of the job to initiate. * @param timer - The timer to use for timeouts. * @returns Information regarding the created job. * @throws If the execution service returns an error or execution times out. */ - protected async initJob( + async #initJob( + snapId: string, jobId: string, timer: Timer, ): Promise> { - const { streams, worker } = await this.initStreams(jobId, timer); + const { streams, worker } = await this.#initStreams(snapId, jobId, timer); const rpcEngine = new JsonRpcEngine(); const jsonRpcConnection = createStreamMiddleware(); @@ -250,12 +252,14 @@ export abstract class AbstractExecutionService /** * Sets up the streams for an initiated job. * + * @param snapId - The Snap ID. * @param jobId - The id of the job. * @param timer - The timer to use for timeouts. * @returns The streams to communicate with the worker and the worker itself. * @throws If the execution service returns an error or execution times out. */ - protected async initStreams( + async #initStreams( + snapId: string, jobId: string, timer: Timer, ): Promise<{ streams: JobStreams; worker: WorkerType }> { @@ -282,8 +286,6 @@ export abstract class AbstractExecutionService return; } - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const snapId = this.#jobToSnapMap.get(jobId)!; if (message.method === 'OutboundRequest') { this.#messenger.publish('ExecutionService:outboundRequest', snapId); } else if (message.method === 'OutboundResponse') { @@ -313,8 +315,31 @@ export abstract class AbstractExecutionService }; commandStream.on('data', notificationHandler); + const rpcStream = mux.createStream(SNAP_STREAM_NAMES.JSON_RPC); + rpcStream.on('data', (chunk) => { + if (chunk?.data && hasProperty(chunk?.data, 'id')) { + this.#messenger.publish('ExecutionService:outboundRequest', snapId); + } + }); + + const originalWrite = rpcStream.write.bind(rpcStream); + + // @ts-expect-error Hack to inspect the messages being written to the stream. + rpcStream.write = (chunk, encoding, callback) => { + // Ignore chain switching notifications as it doesn't matter for the SnapProvider. + if (chunk?.data?.method === 'metamask_chainChanged') { + return; + } + + if (chunk?.data && hasProperty(chunk?.data, 'id')) { + this.#messenger.publish('ExecutionService:outboundResponse', snapId); + } + + originalWrite(chunk, encoding, callback); + }; + // Typecast: stream type mismatch return { streams: { @@ -394,7 +419,7 @@ export abstract class AbstractExecutionService const timer = new Timer(this.#initTimeout); // This may resolve even if the environment has failed to start up fully - const job = await this.initJob(jobId, timer); + const job = await this.#initJob(snapId, jobId, timer); this.#mapSnapAndJob(snapId, job.id); diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index accc65dbcd..4652603339 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,6 +1,6 @@ { - "branches": 90.78, + "branches": 90.7, "functions": 94.96, - "lines": 90.84, - "statements": 90.26 + "lines": 90.62, + "statements": 90.03 } diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts index a1f673c48e..6470c0c216 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts @@ -189,12 +189,6 @@ describe('BaseSnapExecutor', () => { ], }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundRequest', - params: { source: 'ethereum.request' }, - }); - const blockNumRequest = await executor.readRpc(); expect(blockNumRequest).toStrictEqual({ name: 'metamask-provider', @@ -216,12 +210,6 @@ describe('BaseSnapExecutor', () => { }, }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundResponse', - params: { source: 'ethereum.request' }, - }); - expect(await executor.readCommand()).toStrictEqual({ id: 2, jsonrpc: '2.0', @@ -255,12 +243,6 @@ describe('BaseSnapExecutor', () => { ], }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundRequest', - params: { source: 'snap.request' }, - }); - const walletRequest = await executor.readRpc(); expect(walletRequest).toStrictEqual({ name: 'metamask-provider', @@ -282,12 +264,6 @@ describe('BaseSnapExecutor', () => { }, }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundResponse', - params: { source: 'snap.request' }, - }); - expect(await executor.readCommand()).toStrictEqual({ id: 2, jsonrpc: '2.0', @@ -369,12 +345,6 @@ describe('BaseSnapExecutor', () => { ], }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundRequest', - params: { source: 'ethereum.request' }, - }); - const blockNumRequest = await executor.readRpc(); expect(blockNumRequest).toStrictEqual({ name: 'metamask-provider', @@ -396,12 +366,6 @@ describe('BaseSnapExecutor', () => { }, }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundResponse', - params: { source: 'ethereum.request' }, - }); - expect(await executor.readCommand()).toStrictEqual({ id: 2, jsonrpc: '2.0', @@ -437,12 +401,6 @@ describe('BaseSnapExecutor', () => { ], }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundRequest', - params: { source: 'snap.request' }, - }); - const getSnapsRequest = await executor.readRpc(); expect(getSnapsRequest).toStrictEqual({ name: 'metamask-provider', @@ -471,12 +429,6 @@ describe('BaseSnapExecutor', () => { }, }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundResponse', - params: { source: 'snap.request' }, - }); - expect(await executor.readCommand()).toStrictEqual({ id: 2, jsonrpc: '2.0', @@ -808,12 +760,6 @@ describe('BaseSnapExecutor', () => { ], }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundRequest', - params: { source: 'snap.request' }, - }); - const request = await executor.readRpc(); expect(request).toStrictEqual({ name: 'metamask-provider', @@ -841,12 +787,6 @@ describe('BaseSnapExecutor', () => { }, }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundResponse', - params: { source: 'snap.request' }, - }); - expect(await executor.readCommand()).toStrictEqual({ id: 2, jsonrpc: '2.0', @@ -885,12 +825,6 @@ describe('BaseSnapExecutor', () => { ], }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundRequest', - params: { source: 'ethereum.request' }, - }); - const request = await executor.readRpc(); expect(request).toStrictEqual({ name: 'metamask-provider', @@ -920,12 +854,6 @@ describe('BaseSnapExecutor', () => { }, }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundResponse', - params: { source: 'ethereum.request' }, - }); - expect(await executor.readCommand()).toStrictEqual({ id: 2, jsonrpc: '2.0', @@ -1996,12 +1924,6 @@ describe('BaseSnapExecutor', () => { ], }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundRequest', - params: { source: 'ethereum.request' }, - }); - const blockNumRequest = await executor.readRpc(); expect(blockNumRequest).toStrictEqual({ name: 'metamask-provider', @@ -2041,12 +1963,6 @@ describe('BaseSnapExecutor', () => { }, }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundResponse', - params: { source: 'ethereum.request' }, - }); - expect(await executor.readCommand()).toStrictEqual({ id: 3, jsonrpc: '2.0', @@ -2105,12 +2021,6 @@ describe('BaseSnapExecutor', () => { ], }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundRequest', - params: { source: 'ethereum.request' }, - }); - const blockNumRequest = await executor.readRpc(); expect(blockNumRequest).toStrictEqual({ name: 'metamask-provider', @@ -2153,12 +2063,6 @@ describe('BaseSnapExecutor', () => { }, }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundResponse', - params: { source: 'ethereum.request' }, - }); - expect(await executor.readCommand()).toStrictEqual({ id: 3, jsonrpc: '2.0', @@ -2199,12 +2103,6 @@ describe('BaseSnapExecutor', () => { ], }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundRequest', - params: { source: 'ethereum.request' }, - }); - const blockNumRequest = await executor.readRpc(); expect(blockNumRequest).toStrictEqual({ name: 'metamask-provider', @@ -2229,12 +2127,6 @@ describe('BaseSnapExecutor', () => { }, }); - expect(await executor.readCommand()).toStrictEqual({ - jsonrpc: '2.0', - method: 'OutboundResponse', - params: { source: 'ethereum.request' }, - }); - expect(await executor.readCommand()).toStrictEqual({ id: 2, jsonrpc: '2.0', diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts index 9f96315e4a..a32216b239 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts @@ -529,23 +529,7 @@ export class BaseSnapExecutor { // As part of the sanitization, we validate that the args are valid JSON. const sanitizedArgs = sanitizeRequestArguments(args); assertSnapOutboundRequest(sanitizedArgs); - return await withTeardown( - (async () => { - await this.#notify({ - method: 'OutboundRequest', - params: { source: 'snap.request' }, - }); - try { - return await originalRequest(sanitizedArgs); - } finally { - await this.#notify({ - method: 'OutboundResponse', - params: { source: 'snap.request' }, - }); - } - })(), - this as any, - ); + return await withTeardown(originalRequest(sanitizedArgs), this as any); }; const snapsProvider = { request } as SnapsProvider; @@ -571,23 +555,7 @@ export class BaseSnapExecutor { // As part of the sanitization, we validate that the args are valid JSON. const sanitizedArgs = sanitizeRequestArguments(args); assertEthereumOutboundRequest(sanitizedArgs); - return await withTeardown( - (async () => { - await this.#notify({ - method: 'OutboundRequest', - params: { source: 'ethereum.request' }, - }); - try { - return await originalRequest(sanitizedArgs); - } finally { - await this.#notify({ - method: 'OutboundResponse', - params: { source: 'ethereum.request' }, - }); - } - })(), - this as any, - ); + return await withTeardown(originalRequest(sanitizedArgs), this as any); }; const ethereumProvider = { request }; diff --git a/packages/snaps-execution-environments/src/common/endowments/network.ts b/packages/snaps-execution-environments/src/common/endowments/network.ts index b631bd18f8..d80e283d96 100644 --- a/packages/snaps-execution-environments/src/common/endowments/network.ts +++ b/packages/snaps-execution-environments/src/common/endowments/network.ts @@ -226,14 +226,14 @@ const createNetwork = ({ notify }: EndowmentFactoryOptions = {}) => { return await withTeardown( (async () => { try { - await notify({ - method: 'OutboundRequest', - params: { source: 'fetch' }, - }); const fetchPromise = fetch(input, { ...init, signal: abortController.signal, }); + await notify({ + method: 'OutboundRequest', + params: { source: 'fetch' }, + }); openFetchConnection = { cancel: async () => { @@ -256,11 +256,12 @@ const createNetwork = ({ notify }: EndowmentFactoryOptions = {}) => { } finally { if (openFetchConnection !== undefined) { openConnections.delete(openFetchConnection); + + await notify({ + method: 'OutboundResponse', + params: { source: 'fetch' }, + }); } - await notify({ - method: 'OutboundResponse', - params: { source: 'fetch' }, - }); } if (res.body !== null) {