From 0a4f8db8edfeb7facd572092975f53197bb71901 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 19 Dec 2024 12:09:04 +0100 Subject: [PATCH] fix: Skip unnecesary provider initialization (#2967) Since we only expose the `request` function from the provider and don't allow listening to events or using `provider.chainId` etc we can initialize the provider without making the `metamask_getProviderState` request. This saves us a potential network request that may add overhead to booting the Snap. Specifically the clients call `net_version` since this property is required in the `MetaMaskInpageProvider`. Closes https://github.com/MetaMask/snaps/issues/2968 --- .../src/snaps/SnapController.test.tsx | 18 +-------- .../src/test-utils/execution-environment.ts | 10 +---- .../src/test-utils/service.ts | 10 ----- .../coverage.json | 6 +-- .../src/common/BaseSnapExecutor.ts | 8 ++-- .../src/common/SnapProvider.ts | 10 +++++ .../src/common/test-utils/executor.ts | 21 ----------- .../iframe/IFrameSnapExecutor.test.browser.ts | 37 ------------------- .../ChildProcessSnapExecutor.test.ts | 32 +--------------- .../node-thread/ThreadSnapExecutor.test.ts | 32 +--------------- .../WebWorkerSnapExecutor.test.browser.ts | 37 ------------------- 11 files changed, 22 insertions(+), 199 deletions(-) create mode 100644 packages/snaps-execution-environments/src/common/SnapProvider.ts diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index b60903fed1..a99703d581 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -1652,14 +1652,7 @@ describe('SnapController', () => { const stream = mux.createStream('metamask-provider'); const engine = new JsonRpcEngine(); const middleware = createAsyncMiddleware(async (req, res, _next) => { - if (req.method === 'metamask_getProviderState') { - res.result = { - isUnlocked: false, - accounts: [], - chainId: '0x1', - networkVersion: '1', - }; - } else if (req.method === 'eth_blockNumber') { + if (req.method === 'eth_blockNumber') { await sleep(100); res.result = MOCK_BLOCK_NUMBER; } @@ -1737,14 +1730,7 @@ describe('SnapController', () => { const stream = mux.createStream('metamask-provider'); const engine = new JsonRpcEngine(); const middleware = createAsyncMiddleware(async (req, res, _next) => { - if (req.method === 'metamask_getProviderState') { - res.result = { - isUnlocked: false, - accounts: [], - chainId: '0x1', - networkVersion: '1', - }; - } else if (req.method === 'eth_blockNumber') { + if (req.method === 'eth_blockNumber') { await sleep(100); res.result = MOCK_BLOCK_NUMBER; } diff --git a/packages/snaps-controllers/src/test-utils/execution-environment.ts b/packages/snaps-controllers/src/test-utils/execution-environment.ts index dc8e847071..a3778da6c6 100644 --- a/packages/snaps-controllers/src/test-utils/execution-environment.ts +++ b/packages/snaps-controllers/src/test-utils/execution-environment.ts @@ -43,15 +43,7 @@ export const getNodeEES = (messenger: ReturnType) => const stream = mux.createStream('metamask-provider'); const engine = new JsonRpcEngine(); engine.push((req, res, next, end) => { - if (req.method === 'metamask_getProviderState') { - res.result = { - isUnlocked: false, - accounts: [], - chainId: '0x1', - networkVersion: '1', - }; - return end(); - } else if (req.method === 'eth_blockNumber') { + if (req.method === 'eth_blockNumber') { res.result = MOCK_BLOCK_NUMBER; return end(); } diff --git a/packages/snaps-controllers/src/test-utils/service.ts b/packages/snaps-controllers/src/test-utils/service.ts index 90f150b493..b80014cb9b 100644 --- a/packages/snaps-controllers/src/test-utils/service.ts +++ b/packages/snaps-controllers/src/test-utils/service.ts @@ -39,16 +39,6 @@ export const createService = < const stream = mux.createStream('metamask-provider'); const engine = new JsonRpcEngine(); engine.push((req, res, next, end) => { - if (req.method === 'metamask_getProviderState') { - res.result = { - isUnlocked: false, - accounts: [], - chainId: '0x1', - networkVersion: '1', - }; - return end(); - } - if (req.method === 'eth_blockNumber') { res.result = MOCK_BLOCK_NUMBER; return end(); diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index 1a3193285c..926fe3031b 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,6 +1,6 @@ { "branches": 80.68, - "functions": 89.26, - "lines": 90.67, - "statements": 90.06 + "functions": 89.33, + "lines": 90.68, + "statements": 90.08 } diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts index 55ac582c1b..8e7ae6d086 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts @@ -1,8 +1,7 @@ // eslint-disable-next-line @typescript-eslint/triple-slash-reference, spaced-comment /// import { createIdRemapMiddleware } from '@metamask/json-rpc-engine'; -import type { RequestArguments } from '@metamask/providers'; -import { StreamProvider } from '@metamask/providers/stream-provider'; +import type { RequestArguments, StreamProvider } from '@metamask/providers'; import { errorCodes, rpcErrors, serializeError } from '@metamask/rpc-errors'; import type { SnapsEthereumProvider, SnapsProvider } from '@metamask/snaps-sdk'; import { getErrorData } from '@metamask/snaps-sdk'; @@ -40,6 +39,7 @@ import type { CommandMethodsMapping } from './commands'; import { getCommandMethodImplementations } from './commands'; import { createEndowments } from './endowments'; import { addEventListener, removeEventListener } from './globalEvents'; +import { SnapProvider } from './SnapProvider'; import { sortParamKeys } from './sortParams'; import { assertEthereumOutboundRequest, @@ -369,12 +369,12 @@ export class BaseSnapExecutor { }); }; - const provider = new StreamProvider(this.rpcStream, { + const provider = new SnapProvider(this.rpcStream, { jsonRpcStreamName: 'metamask-provider', rpcMiddleware: [createIdRemapMiddleware()], }); - await provider.initialize(); + provider.initializeSync(); const snap = this.createSnapGlobal(provider); const ethereum = this.createEIP1193Provider(provider); diff --git a/packages/snaps-execution-environments/src/common/SnapProvider.ts b/packages/snaps-execution-environments/src/common/SnapProvider.ts new file mode 100644 index 0000000000..b90cc31fea --- /dev/null +++ b/packages/snaps-execution-environments/src/common/SnapProvider.ts @@ -0,0 +1,10 @@ +import { StreamProvider } from '@metamask/providers/stream-provider'; + +export class SnapProvider extends StreamProvider { + // Since only the request function is exposed to the Snap, we can initialize the provider + // without making the metamask_getProviderState request, saving us a + // potential network request before boot. + initializeSync() { + this._initializeState(); + } +} diff --git a/packages/snaps-execution-environments/src/common/test-utils/executor.ts b/packages/snaps-execution-environments/src/common/test-utils/executor.ts index 8ce65462f5..26ffb011af 100644 --- a/packages/snaps-execution-environments/src/common/test-utils/executor.ts +++ b/packages/snaps-execution-environments/src/common/test-utils/executor.ts @@ -111,7 +111,6 @@ export class TestSnapExecutor extends BaseSnapExecutor { code: string, endowments: string[], ) { - const providerRequestPromise = this.readRpc(); await this.writeCommand({ jsonrpc: '2.0', id, @@ -124,26 +123,6 @@ export class TestSnapExecutor extends BaseSnapExecutor { if ('clock' in setTimeout) { jest.advanceTimersByTime(1); } - - const providerRequest = await providerRequestPromise; - await this.writeRpc({ - name: 'metamask-provider', - data: { - jsonrpc: '2.0', - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - id: providerRequest.data.id!, - result: { - isUnlocked: false, - accounts: [], - chainId: '0x1', - networkVersion: '1', - }, - }, - }); - - if ('clock' in setTimeout) { - jest.advanceTimersByTime(1); - } } public async writeCommand(message: JsonRpcRequest): Promise { diff --git a/packages/snaps-execution-environments/src/iframe/IFrameSnapExecutor.test.browser.ts b/packages/snaps-execution-environments/src/iframe/IFrameSnapExecutor.test.browser.ts index ce5ba43de3..12ec2b7c9c 100644 --- a/packages/snaps-execution-environments/src/iframe/IFrameSnapExecutor.test.browser.ts +++ b/packages/snaps-execution-environments/src/iframe/IFrameSnapExecutor.test.browser.ts @@ -6,7 +6,6 @@ import { MOCK_SNAP_ID, MockWindowPostMessageStream, } from '@metamask/snaps-utils/test-utils'; -import type { JsonRpcRequest } from '@metamask/utils'; import { IFrameSnapExecutor } from './IFrameSnapExecutor'; @@ -39,22 +38,6 @@ async function getResponse( }); } -/** - * Wait for a outbound request from the stream. - * - * @param stream - The stream to wait for a response on. - * @returns The raw JSON-RPC response object. - */ -async function getOutboundRequest( - stream: MockWindowPostMessageStream, -): Promise { - return new Promise((resolve) => { - stream.once('outbound', (data) => { - resolve(data.data); - }); - }); -} - describe('IFrameSnapExecutor', () => { before(() => { // @ts-expect-error - `globalThis.process` is not optional. @@ -104,26 +87,6 @@ describe('IFrameSnapExecutor', () => { }, }); - const outboundRequest = await getOutboundRequest(mockStream); - expect(outboundRequest.method).toBe('metamask_getProviderState'); - - writeMessage(mockStream, { - name: 'jsonRpc', - data: { - name: 'metamask-provider', - data: { - jsonrpc: '2.0', - id: outboundRequest.id, - result: { - isUnlocked: false, - accounts: [], - chainId: '0x1', - networkVersion: '1', - }, - }, - }, - }); - expect(await getResponse(mockStream)).toStrictEqual({ jsonrpc: '2.0', id: 2, diff --git a/packages/snaps-execution-environments/src/node-process/ChildProcessSnapExecutor.test.ts b/packages/snaps-execution-environments/src/node-process/ChildProcessSnapExecutor.test.ts index 04aa7e13d5..ea4ab0dd7d 100644 --- a/packages/snaps-execution-environments/src/node-process/ChildProcessSnapExecutor.test.ts +++ b/packages/snaps-execution-environments/src/node-process/ChildProcessSnapExecutor.test.ts @@ -2,7 +2,7 @@ import 'ses'; import { HandlerType, SNAP_STREAM_NAMES } from '@metamask/snaps-utils'; import { MOCK_ORIGIN, MOCK_SNAP_ID } from '@metamask/snaps-utils/test-utils'; -import type { Json, JsonRpcRequest, JsonRpcSuccess } from '@metamask/utils'; +import type { Json, JsonRpcSuccess } from '@metamask/utils'; import { EventEmitter } from 'stream'; import { ChildProcessSnapExecutor } from './ChildProcessSnapExecutor'; @@ -34,18 +34,6 @@ describe('ChildProcessSnapExecutor', () => { // Utility functions const emit = (data: Json) => parentEmitter.emit('message', { data }); const emitChunk = (name: string, data: Json) => emit({ name, data }); - const waitForOutbound = (request: Partial>): any => - new Promise((resolve) => { - childEmitter.on('message', ({ data: { name, data } }) => { - if ( - name === SNAP_STREAM_NAMES.JSON_RPC && - data.name === 'metamask-provider' && - data.data.method === request.method - ) { - resolve(data.data); - } - }); - }); const waitForResponse = async (response: JsonRpcSuccess) => new Promise((resolve) => { @@ -72,24 +60,6 @@ describe('ChildProcessSnapExecutor', () => { params: [MOCK_SNAP_ID, CODE, []], }); - const providerRequest = await waitForOutbound({ - method: 'metamask_getProviderState', - }); - - emitChunk(SNAP_STREAM_NAMES.JSON_RPC, { - name: 'metamask-provider', - data: { - jsonrpc: '2.0', - id: providerRequest.id, - result: { - isUnlocked: false, - accounts: [], - chainId: '0x1', - networkVersion: '1', - }, - }, - }); - expect( await waitForResponse({ result: 'OK', id: 1, jsonrpc: '2.0' }), ).not.toBeNull(); diff --git a/packages/snaps-execution-environments/src/node-thread/ThreadSnapExecutor.test.ts b/packages/snaps-execution-environments/src/node-thread/ThreadSnapExecutor.test.ts index 0cd9fadf71..e6e585ac38 100644 --- a/packages/snaps-execution-environments/src/node-thread/ThreadSnapExecutor.test.ts +++ b/packages/snaps-execution-environments/src/node-thread/ThreadSnapExecutor.test.ts @@ -1,7 +1,7 @@ // eslint-disable-next-line import/no-unassigned-import import 'ses'; import { SNAP_STREAM_NAMES, HandlerType } from '@metamask/snaps-utils'; -import type { Json, JsonRpcRequest, JsonRpcSuccess } from '@metamask/utils'; +import type { Json, JsonRpcSuccess } from '@metamask/utils'; import { EventEmitter } from 'stream'; import { parentPort } from 'worker_threads'; @@ -48,18 +48,6 @@ describe('ThreadSnapExecutor', () => { // Utility functions const emit = (data: Json) => parentEmitter.emit('message', { data }); const emitChunk = (name: string, data: Json) => emit({ name, data }); - const waitForOutbound = (request: Partial>): any => - new Promise((resolve) => { - childEmitter.on('message', ({ data: { name, data } }) => { - if ( - name === SNAP_STREAM_NAMES.JSON_RPC && - data.name === 'metamask-provider' && - data.data.method === request.method - ) { - resolve(data.data); - } - }); - }); const waitForResponse = async (response: JsonRpcSuccess) => new Promise((resolve) => { @@ -86,24 +74,6 @@ describe('ThreadSnapExecutor', () => { params: [FAKE_SNAP_NAME, CODE, []], }); - const providerRequest = await waitForOutbound({ - method: 'metamask_getProviderState', - }); - - emitChunk(SNAP_STREAM_NAMES.JSON_RPC, { - name: 'metamask-provider', - data: { - jsonrpc: '2.0', - id: providerRequest.id, - result: { - isUnlocked: false, - accounts: [], - chainId: '0x1', - networkVersion: '1', - }, - }, - }); - expect( await waitForResponse({ result: 'OK', id: 1, jsonrpc: '2.0' }), ).not.toBeNull(); diff --git a/packages/snaps-execution-environments/src/webworker/executor/WebWorkerSnapExecutor.test.browser.ts b/packages/snaps-execution-environments/src/webworker/executor/WebWorkerSnapExecutor.test.browser.ts index f1bb74d384..9a240a4a80 100644 --- a/packages/snaps-execution-environments/src/webworker/executor/WebWorkerSnapExecutor.test.browser.ts +++ b/packages/snaps-execution-environments/src/webworker/executor/WebWorkerSnapExecutor.test.browser.ts @@ -8,7 +8,6 @@ import { MockWindowPostMessageStream, spy, } from '@metamask/snaps-utils/test-utils'; -import type { JsonRpcRequest } from '@metamask/utils'; import { WebWorkerSnapExecutor } from './WebWorkerSnapExecutor'; @@ -41,22 +40,6 @@ async function getResponse( }); } -/** - * Wait for a outbound request from the stream. - * - * @param stream - The stream to wait for a response on. - * @returns The raw JSON-RPC response object. - */ -async function getOutboundRequest( - stream: MockWindowPostMessageStream, -): Promise { - return new Promise((resolve) => { - stream.once('outbound', (data) => { - resolve(data.data); - }); - }); -} - describe('WebWorkerSnapExecutor', () => { let consoleSpy: SpyFunction; @@ -116,26 +99,6 @@ describe('WebWorkerSnapExecutor', () => { }, }); - const outboundRequest = await getOutboundRequest(mockStream); - expect(outboundRequest.method).toBe('metamask_getProviderState'); - - writeMessage(mockStream, { - name: 'jsonRpc', - data: { - name: 'metamask-provider', - data: { - jsonrpc: '2.0', - id: outboundRequest.id, - result: { - isUnlocked: false, - accounts: [], - chainId: '0x1', - networkVersion: '1', - }, - }, - }, - }); - expect(await getResponse(mockStream)).toStrictEqual({ jsonrpc: '2.0', id: 2,