perf: Move request inspection outside of the executor

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
 }

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();
+  });
 });

packages/snaps-controllers/src/services/AbstractExecutionService.ts

@@ -209,16 +209,18 @@ export abstract class AbstractExecutionService<WorkerType>
   /**
    * 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<Job<WorkerType>> {
-    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<WorkerType>
   /**
    * 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<WorkerType>
         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<WorkerType>
     };
 
     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<WorkerType>
     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);
 

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
 }

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',