Skip to content

Commit 8571a4d

Browse files
FrederikBoldingFrederikBolding
authored
perf: Move request inspection outside of the executor (#3356)
In the current implementation, every RPC request requires a least 3 messages to be sent to manage outbound request/response tracking. This PR proposes to move that tracking to the client side where we can simply monitor the RPC stream for requests and responses. This should alleviate some pressure on the communication layer for Snaps. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Shift outbound request/response tracking to AbstractExecutionService by inspecting the JSON-RPC stream, removing executor-side notifications and updating tests accordingly. > > - **Controllers (`packages/snaps-controllers`)**: > - Update `AbstractExecutionService` to detect outbound activity by inspecting `rpc` stream: > - Publish `ExecutionService:outboundRequest` on inbound RPC chunks with an `id`. > - Intercept `rpcStream.write` to publish `ExecutionService:outboundResponse` when writing responses; ignore `metamask_chainChanged`. > - **Execution Environment (`packages/snaps-execution-environments`)**: > - Remove `OutboundRequest`/`OutboundResponse` notifications from `BaseSnapExecutor` provider wrappers (`snap.request`, `ethereum.request`) and simplify teardown handling. > - Adjust `BaseSnapExecutor.test.browser.ts` to drop expectations for command-stream outbound notifications where applicable, keeping RPC flow assertions intact. > - **Coverage**: Minor metric fluctuations in coverage JSON files. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8b9e40f. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 222928d commit 8571a4d

File tree

5 files changed

+33
-156
lines changed

5 files changed

+33
-156
lines changed

‎packages/snaps-controllers/coverage.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"branches": 95.63,
3-
"functions": 98.99,
4-
"lines": 98.88,
5-
"statements": 98.71
2+
"branches": 95.23,
3+
"functions": 99,
4+
"lines": 98.74,
5+
"statements": 98.58
66
}

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,31 @@ export abstract class AbstractExecutionService<WorkerType>
307307
};
308308

309309
commandStream.on('data', notificationHandler);
310+
310311
const rpcStream = mux.createStream(SNAP_STREAM_NAMES.JSON_RPC);
311312

313+
rpcStream.on('data', (chunk) => {
314+
if (chunk?.data && hasProperty(chunk?.data, 'id')) {
315+
this.#messenger.publish('ExecutionService:outboundRequest', snapId);
316+
}
317+
});
318+
319+
const originalWrite = rpcStream.write.bind(rpcStream);
320+
321+
// @ts-expect-error Hack to inspect the messages being written to the stream.
322+
rpcStream.write = (chunk, encoding, callback) => {
323+
// Ignore chain switching notifications as it doesn't matter for the SnapProvider.
324+
if (chunk?.data?.method === 'metamask_chainChanged') {
325+
return true;
326+
}
327+
328+
if (chunk?.data && hasProperty(chunk?.data, 'id')) {
329+
this.#messenger.publish('ExecutionService:outboundResponse', snapId);
330+
}
331+
332+
return originalWrite(chunk, encoding, callback);
333+
};
334+
312335
// Typecast: stream type mismatch
313336
return {
314337
streams: {

‎packages/snaps-execution-environments/coverage.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"branches": 90.04,
3-
"functions": 94.69,
4-
"lines": 90.42,
5-
"statements": 89.62
2+
"branches": 90.09,
3+
"functions": 94.73,
4+
"lines": 90.24,
5+
"statements": 89.37
66
}

‎packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts‎

Lines changed: 0 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -189,12 +189,6 @@ describe('BaseSnapExecutor', () => {
189189
],
190190
});
191191

192-
expect(await executor.readCommand()).toStrictEqual({
193-
jsonrpc: '2.0',
194-
method: 'OutboundRequest',
195-
params: { source: 'ethereum.request' },
196-
});
197-
198192
const blockNumRequest = await executor.readRpc();
199193
expect(blockNumRequest).toStrictEqual({
200194
name: 'metamask-provider',
@@ -216,12 +210,6 @@ describe('BaseSnapExecutor', () => {
216210
},
217211
});
218212

219-
expect(await executor.readCommand()).toStrictEqual({
220-
jsonrpc: '2.0',
221-
method: 'OutboundResponse',
222-
params: { source: 'ethereum.request' },
223-
});
224-
225213
expect(await executor.readCommand()).toStrictEqual({
226214
id: 2,
227215
jsonrpc: '2.0',
@@ -255,12 +243,6 @@ describe('BaseSnapExecutor', () => {
255243
],
256244
});
257245

258-
expect(await executor.readCommand()).toStrictEqual({
259-
jsonrpc: '2.0',
260-
method: 'OutboundRequest',
261-
params: { source: 'snap.request' },
262-
});
263-
264246
const walletRequest = await executor.readRpc();
265247
expect(walletRequest).toStrictEqual({
266248
name: 'metamask-provider',
@@ -282,12 +264,6 @@ describe('BaseSnapExecutor', () => {
282264
},
283265
});
284266

285-
expect(await executor.readCommand()).toStrictEqual({
286-
jsonrpc: '2.0',
287-
method: 'OutboundResponse',
288-
params: { source: 'snap.request' },
289-
});
290-
291267
expect(await executor.readCommand()).toStrictEqual({
292268
id: 2,
293269
jsonrpc: '2.0',
@@ -369,12 +345,6 @@ describe('BaseSnapExecutor', () => {
369345
],
370346
});
371347

372-
expect(await executor.readCommand()).toStrictEqual({
373-
jsonrpc: '2.0',
374-
method: 'OutboundRequest',
375-
params: { source: 'ethereum.request' },
376-
});
377-
378348
const blockNumRequest = await executor.readRpc();
379349
expect(blockNumRequest).toStrictEqual({
380350
name: 'metamask-provider',
@@ -396,12 +366,6 @@ describe('BaseSnapExecutor', () => {
396366
},
397367
});
398368

399-
expect(await executor.readCommand()).toStrictEqual({
400-
jsonrpc: '2.0',
401-
method: 'OutboundResponse',
402-
params: { source: 'ethereum.request' },
403-
});
404-
405369
expect(await executor.readCommand()).toStrictEqual({
406370
id: 2,
407371
jsonrpc: '2.0',
@@ -437,12 +401,6 @@ describe('BaseSnapExecutor', () => {
437401
],
438402
});
439403

440-
expect(await executor.readCommand()).toStrictEqual({
441-
jsonrpc: '2.0',
442-
method: 'OutboundRequest',
443-
params: { source: 'snap.request' },
444-
});
445-
446404
const getSnapsRequest = await executor.readRpc();
447405
expect(getSnapsRequest).toStrictEqual({
448406
name: 'metamask-provider',
@@ -471,12 +429,6 @@ describe('BaseSnapExecutor', () => {
471429
},
472430
});
473431

474-
expect(await executor.readCommand()).toStrictEqual({
475-
jsonrpc: '2.0',
476-
method: 'OutboundResponse',
477-
params: { source: 'snap.request' },
478-
});
479-
480432
expect(await executor.readCommand()).toStrictEqual({
481433
id: 2,
482434
jsonrpc: '2.0',
@@ -808,12 +760,6 @@ describe('BaseSnapExecutor', () => {
808760
],
809761
});
810762

811-
expect(await executor.readCommand()).toStrictEqual({
812-
jsonrpc: '2.0',
813-
method: 'OutboundRequest',
814-
params: { source: 'snap.request' },
815-
});
816-
817763
const request = await executor.readRpc();
818764
expect(request).toStrictEqual({
819765
name: 'metamask-provider',
@@ -841,12 +787,6 @@ describe('BaseSnapExecutor', () => {
841787
},
842788
});
843789

844-
expect(await executor.readCommand()).toStrictEqual({
845-
jsonrpc: '2.0',
846-
method: 'OutboundResponse',
847-
params: { source: 'snap.request' },
848-
});
849-
850790
expect(await executor.readCommand()).toStrictEqual({
851791
id: 2,
852792
jsonrpc: '2.0',
@@ -885,12 +825,6 @@ describe('BaseSnapExecutor', () => {
885825
],
886826
});
887827

888-
expect(await executor.readCommand()).toStrictEqual({
889-
jsonrpc: '2.0',
890-
method: 'OutboundRequest',
891-
params: { source: 'ethereum.request' },
892-
});
893-
894828
const request = await executor.readRpc();
895829
expect(request).toStrictEqual({
896830
name: 'metamask-provider',
@@ -920,12 +854,6 @@ describe('BaseSnapExecutor', () => {
920854
},
921855
});
922856

923-
expect(await executor.readCommand()).toStrictEqual({
924-
jsonrpc: '2.0',
925-
method: 'OutboundResponse',
926-
params: { source: 'ethereum.request' },
927-
});
928-
929857
expect(await executor.readCommand()).toStrictEqual({
930858
id: 2,
931859
jsonrpc: '2.0',
@@ -2075,12 +2003,6 @@ describe('BaseSnapExecutor', () => {
20752003
],
20762004
});
20772005

2078-
expect(await executor.readCommand()).toStrictEqual({
2079-
jsonrpc: '2.0',
2080-
method: 'OutboundRequest',
2081-
params: { source: 'ethereum.request' },
2082-
});
2083-
20842006
const blockNumRequest = await executor.readRpc();
20852007
expect(blockNumRequest).toStrictEqual({
20862008
name: 'metamask-provider',
@@ -2120,12 +2042,6 @@ describe('BaseSnapExecutor', () => {
21202042
},
21212043
});
21222044

2123-
expect(await executor.readCommand()).toStrictEqual({
2124-
jsonrpc: '2.0',
2125-
method: 'OutboundResponse',
2126-
params: { source: 'ethereum.request' },
2127-
});
2128-
21292045
expect(await executor.readCommand()).toStrictEqual({
21302046
id: 3,
21312047
jsonrpc: '2.0',
@@ -2184,12 +2100,6 @@ describe('BaseSnapExecutor', () => {
21842100
],
21852101
});
21862102

2187-
expect(await executor.readCommand()).toStrictEqual({
2188-
jsonrpc: '2.0',
2189-
method: 'OutboundRequest',
2190-
params: { source: 'ethereum.request' },
2191-
});
2192-
21932103
const blockNumRequest = await executor.readRpc();
21942104
expect(blockNumRequest).toStrictEqual({
21952105
name: 'metamask-provider',
@@ -2232,12 +2142,6 @@ describe('BaseSnapExecutor', () => {
22322142
},
22332143
});
22342144

2235-
expect(await executor.readCommand()).toStrictEqual({
2236-
jsonrpc: '2.0',
2237-
method: 'OutboundResponse',
2238-
params: { source: 'ethereum.request' },
2239-
});
2240-
22412145
expect(await executor.readCommand()).toStrictEqual({
22422146
id: 3,
22432147
jsonrpc: '2.0',
@@ -2278,12 +2182,6 @@ describe('BaseSnapExecutor', () => {
22782182
],
22792183
});
22802184

2281-
expect(await executor.readCommand()).toStrictEqual({
2282-
jsonrpc: '2.0',
2283-
method: 'OutboundRequest',
2284-
params: { source: 'ethereum.request' },
2285-
});
2286-
22872185
const blockNumRequest = await executor.readRpc();
22882186
expect(blockNumRequest).toStrictEqual({
22892187
name: 'metamask-provider',
@@ -2308,12 +2206,6 @@ describe('BaseSnapExecutor', () => {
23082206
},
23092207
});
23102208

2311-
expect(await executor.readCommand()).toStrictEqual({
2312-
jsonrpc: '2.0',
2313-
method: 'OutboundResponse',
2314-
params: { source: 'ethereum.request' },
2315-
});
2316-
23172209
expect(await executor.readCommand()).toStrictEqual({
23182210
id: 2,
23192211
jsonrpc: '2.0',

‎packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts‎

Lines changed: 2 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -529,26 +529,7 @@ export class BaseSnapExecutor {
529529
// As part of the sanitization, we validate that the args are valid JSON.
530530
const sanitizedArgs = sanitizeRequestArguments(args);
531531
assertSnapOutboundRequest(sanitizedArgs);
532-
return await withTeardown(
533-
(async () => {
534-
try {
535-
const promise = originalRequest(sanitizedArgs);
536-
537-
await this.#notify({
538-
method: 'OutboundRequest',
539-
params: { source: 'snap.request' },
540-
});
541-
542-
return await promise;
543-
} finally {
544-
await this.#notify({
545-
method: 'OutboundResponse',
546-
params: { source: 'snap.request' },
547-
});
548-
}
549-
})(),
550-
this as any,
551-
);
532+
return await withTeardown(originalRequest(sanitizedArgs), this as any);
552533
};
553534

554535
const snapsProvider = { request } as SnapsProvider;
@@ -574,26 +555,7 @@ export class BaseSnapExecutor {
574555
// As part of the sanitization, we validate that the args are valid JSON.
575556
const sanitizedArgs = sanitizeRequestArguments(args);
576557
assertEthereumOutboundRequest(sanitizedArgs);
577-
return await withTeardown(
578-
(async () => {
579-
try {
580-
const promise = originalRequest(sanitizedArgs);
581-
582-
await this.#notify({
583-
method: 'OutboundRequest',
584-
params: { source: 'ethereum.request' },
585-
});
586-
587-
return await promise;
588-
} finally {
589-
await this.#notify({
590-
method: 'OutboundResponse',
591-
params: { source: 'ethereum.request' },
592-
});
593-
}
594-
})(),
595-
this as any,
596-
);
558+
return await withTeardown(originalRequest(sanitizedArgs), this as any);
597559
};
598560

599561
const ethereumProvider = { request };

0 commit comments

Comments
 (0)