Pause timer on fetch (#1756)

This PR adds notifications from the network access endowment that are sent during fetch. Calling `fetch` will result in `OutboundRequest` upfront and `OutboundResponse` once the promise resolves. Furthermore, once per response object, triggering an async action, e.g. `json()` will also trigger `OutboundRequest` upfront and `OutboundResponse` once the promise resolves. The notifications pause the request processing timer in the same way that requests via `snap.request` and `ethereum.request` pause the timer. This PR also refactors the endowment creation slightly to make it take an options bag and always require an endowment array. Closes #1755
MetaMask · Nov 15, 2023 · 8d3710b · 8d3710b
1 parent 27fe061
commit 8d3710b
Show file tree

Hide file tree

Showing 10 changed files with 404 additions and 169 deletions.
diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json
@@ -1,6 +1,6 @@
 {
-  "branches": 80.71,
-  "functions": 92.02,
-  "lines": 91.66,
-  "statements": 91.36
+  "branches": 80.28,
+  "functions": 90.41,
+  "lines": 91.06,
+  "statements": 90.65
 }
diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts
@@ -191,6 +191,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundRequest',
+      params: { source: 'ethereum.request' },
     });
 
     const blockNumRequest = await executor.readRpc();
@@ -217,6 +218,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundResponse',
+      params: { source: 'ethereum.request' },
     });
 
     expect(await executor.readCommand()).toStrictEqual({
@@ -255,6 +257,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundRequest',
+      params: { source: 'snap.request' },
     });
 
     const walletRequest = await executor.readRpc();
@@ -281,6 +284,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundResponse',
+      params: { source: 'snap.request' },
     });
 
     expect(await executor.readCommand()).toStrictEqual({
@@ -416,6 +420,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundRequest',
+      params: { source: 'ethereum.request' },
     });
 
     const blockNumRequest = await executor.readRpc();
@@ -442,6 +447,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundResponse',
+      params: { source: 'ethereum.request' },
     });
 
     expect(await executor.readCommand()).toStrictEqual({
@@ -482,6 +488,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundRequest',
+      params: { source: 'snap.request' },
     });
 
     const getSnapsRequest = await executor.readRpc();
@@ -516,6 +523,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundResponse',
+      params: { source: 'snap.request' },
     });
 
     expect(await executor.readCommand()).toStrictEqual({
@@ -898,6 +906,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundRequest',
+      params: { source: 'snap.request' },
     });
 
     const request = await executor.readRpc();
@@ -930,6 +939,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundResponse',
+      params: { source: 'snap.request' },
     });
 
     expect(await executor.readCommand()).toStrictEqual({
@@ -973,6 +983,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundRequest',
+      params: { source: 'ethereum.request' },
     });
 
     const request = await executor.readRpc();
@@ -1007,6 +1018,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundResponse',
+      params: { source: 'ethereum.request' },
     });
 
     expect(await executor.readCommand()).toStrictEqual({
@@ -1016,6 +1028,69 @@ describe('BaseSnapExecutor', () => {
     });
   });
 
+  it('reports when outbound requests are made using fetch', async () => {
+    const CODE = `
+      module.exports.onRpcRequest = () => fetch('https://metamask.io').then(res => res.text());
+    `;
+
+    const fetchSpy = spy(globalThis, 'fetch');
+
+    fetchSpy.mockImplementation(async () => {
+      return new Response('foo');
+    });
+
+    const executor = new TestSnapExecutor();
+    await executor.executeSnap(1, MOCK_SNAP_ID, CODE, ['fetch']);
+
+    expect(await executor.readCommand()).toStrictEqual({
+      jsonrpc: '2.0',
+      id: 1,
+      result: 'OK',
+    });
+
+    await executor.writeCommand({
+      jsonrpc: '2.0',
+      id: 2,
+      method: 'snapRpc',
+      params: [
+        MOCK_SNAP_ID,
+        HandlerType.OnRpcRequest,
+        MOCK_ORIGIN,
+        { jsonrpc: '2.0', method: '', params: [] },
+      ],
+    });
+
+    expect(await executor.readCommand()).toStrictEqual({
+      jsonrpc: '2.0',
+      method: 'OutboundRequest',
+      params: { source: 'fetch' },
+    });
+
+    expect(await executor.readCommand()).toStrictEqual({
+      jsonrpc: '2.0',
+      method: 'OutboundResponse',
+      params: { source: 'fetch' },
+    });
+
+    expect(await executor.readCommand()).toStrictEqual({
+      jsonrpc: '2.0',
+      method: 'OutboundRequest',
+      params: { source: 'fetch' },
+    });
+
+    expect(await executor.readCommand()).toStrictEqual({
+      jsonrpc: '2.0',
+      method: 'OutboundResponse',
+      params: { source: 'fetch' },
+    });
+
+    expect(await executor.readCommand()).toStrictEqual({
+      id: 2,
+      jsonrpc: '2.0',
+      result: 'foo',
+    });
+  });
+
   it('notifies execution service of out of band errors via unhandledrejection', async () => {
     const CODE = `
       module.exports.onRpcRequest = async () => 'foo';
@@ -1565,6 +1640,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundRequest',
+      params: { source: 'ethereum.request' },
     });
 
     const blockNumRequest = await executor.readRpc();
@@ -1609,6 +1685,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundResponse',
+      params: { source: 'ethereum.request' },
     });
 
     expect(await executor.readCommand()).toStrictEqual({
@@ -1672,6 +1749,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundRequest',
+      params: { source: 'ethereum.request' },
     });
 
     const blockNumRequest = await executor.readRpc();
@@ -1719,6 +1797,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundResponse',
+      params: { source: 'ethereum.request' },
     });
 
     expect(await executor.readCommand()).toStrictEqual({
@@ -1764,6 +1843,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundRequest',
+      params: { source: 'ethereum.request' },
     });
 
     const blockNumRequest = await executor.readRpc();
@@ -1793,6 +1873,7 @@ describe('BaseSnapExecutor', () => {
     expect(await executor.readCommand()).toStrictEqual({
       jsonrpc: '2.0',
       method: 'OutboundResponse',
+      params: { source: 'ethereum.request' },
     });
 
     expect(await executor.readCommand()).toStrictEqual({

diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts
@@ -110,6 +110,10 @@ const EXECUTION_ENVIRONMENT_METHODS = {
 
 type Methods = typeof EXECUTION_ENVIRONMENT_METHODS;
 
+export type NotifyFunction = (
+  notification: Omit<JsonRpcNotification, 'jsonrpc'>,
+) => Promise<void>;
+
 export class BaseSnapExecutor {
   private readonly snapData: Map<string, SnapData>;
 
@@ -325,7 +329,7 @@ export class BaseSnapExecutor {
   protected async startSnap(
     snapId: string,
     sourceCode: string,
-    _endowments?: string[],
+    _endowments: string[],
   ): Promise<void> {
     log(`Starting snap '${snapId}' in worker.`);
     if (this.snapPromiseErrorHandler) {
@@ -359,12 +363,13 @@ export class BaseSnapExecutor {
     const snapModule: any = { exports: {} };
 
     try {
-      const { endowments, teardown: endowmentTeardown } = createEndowments(
+      const { endowments, teardown: endowmentTeardown } = createEndowments({
         snap,
         ethereum,
         snapId,
-        _endowments,
-      );
+        endowments: _endowments,
+        notify: this.#notify.bind(this),
+      });
 
       // !!! Ensure that this is the only place the data is being set.
       // Other methods access the object value and mutate its properties.
@@ -454,11 +459,17 @@ export class BaseSnapExecutor {
       assertSnapOutboundRequest(sanitizedArgs);
       return await withTeardown(
         (async () => {
-          await this.#notify({ method: 'OutboundRequest' });
+          await this.#notify({
+            method: 'OutboundRequest',
+            params: { source: 'snap.request' },
+          });
           try {
             return await originalRequest(sanitizedArgs);
           } finally {
-            await this.#notify({ method: 'OutboundResponse' });
+            await this.#notify({
+              method: 'OutboundResponse',
+              params: { source: 'snap.request' },
+            });
           }
         })(),
         this as any,
@@ -500,11 +511,17 @@ export class BaseSnapExecutor {
       assertEthereumOutboundRequest(sanitizedArgs);
       return await withTeardown(
         (async () => {
-          await this.#notify({ method: 'OutboundRequest' });
+          await this.#notify({
+            method: 'OutboundRequest',
+            params: { source: 'ethereum.request' },
+          });
           try {
             return await originalRequest(sanitizedArgs);
           } finally {
-            await this.#notify({ method: 'OutboundResponse' });
+            await this.#notify({
+              method: 'OutboundResponse',
+              params: { source: 'ethereum.request' },
+            });
           }
         })(),
         this as any,

diff --git a/packages/snaps-execution-environments/src/common/endowments/commonEndowmentFactory.ts b/packages/snaps-execution-environments/src/common/endowments/commonEndowmentFactory.ts
@@ -1,3 +1,4 @@
+import type { NotifyFunction } from '../BaseSnapExecutor';
 import { rootRealmGlobal } from '../globalObject';
 import consoleEndowment from './console';
 import crypto from './crypto';
@@ -11,6 +12,7 @@ import timeout from './timeout';
 
 export type EndowmentFactoryOptions = {
   snapId?: string;
+  notify?: NotifyFunction;
 };
 
 export type EndowmentFactory = {

diff --git a/packages/snaps-execution-environments/src/common/endowments/endowments.test.browser.ts b/packages/snaps-execution-environments/src/common/endowments/endowments.test.browser.ts
@@ -33,10 +33,16 @@ lockdown({
 globalThis.atob = harden(originalAtob);
 globalThis.btoa = harden(originalBtoa);
 
+const mockNotify = () => {
+  // no-op
+};
+
 describe('endowments', () => {
   describe('hardening', () => {
     const modules = buildCommonEndowments();
-    modules.forEach((endowment) => endowment.factory({ snapId: MOCK_SNAP_ID }));
+    modules.forEach((endowment) =>
+      endowment.factory({ snapId: MOCK_SNAP_ID, notify: mockNotify }),
+    );
 
     // Specially attenuated endowments or endowments that require
     // to be imported in a different way
@@ -56,7 +62,9 @@ describe('endowments', () => {
       Request: RequestHardened,
       Headers: HeadersHardened,
       Response: ResponseHardened,
-    } = network.factory();
+    } = network.factory({
+      notify: mockNotify,
+    });
     const { Date: DateAttenuated } = date.factory();
     const { console: consoleAttenuated } = consoleEndowment.factory({
       snapId: MOCK_SNAP_ID,