Skip to content

Commit 5990071

Browse files
perf: Reduce unnecessary validation on responses (#3350)
Previously we were validating that responses were JSON, but we already do that elsewhere and are guaranteed for the responses to be valid by the time we get to `isValidResponse`. Additionally, we don't need an entirely byte accurate length for the response, if the Snap goes beyond the limit they simply won't receive a response from the Snap.
1 parent d75fa4d commit 5990071

File tree

5 files changed

+12
-16
lines changed

5 files changed

+12
-16
lines changed
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"branches": 90.78,
33
"functions": 94.96,
4-
"lines": 90.85,
5-
"statements": 90.27
4+
"lines": 90.84,
5+
"statements": 90.26
66
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ export class BaseSnapExecutor {
346346
});
347347
}
348348

349-
async #respond(id: JsonRpcId, response: Record<string, unknown>) {
349+
async #respond(id: JsonRpcId, response: Record<string, Json>) {
350350
if (!isValidResponse(response)) {
351351
// Instead of throwing, we directly respond with an error.
352352
// This prevents an issue where we wouldn't respond when errors were non-serializable

packages/snaps-execution-environments/src/common/utils.test.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,6 @@ describe('isValidResponse', () => {
6363
expect(isValidResponse('foo')).toBe(false);
6464
});
6565

66-
it('returns false if the value is not JSON serializable', () => {
67-
expect(isValidResponse({ foo: BigInt(0) })).toBe(false);
68-
});
69-
7066
it('returns false if the value is too large', () => {
7167
expect(isValidResponse({ foo: '1'.repeat(100_000_000) })).toBe(false);
7268
});

packages/snaps-execution-environments/src/common/utils.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import type { RequestArguments } from '@metamask/providers';
22
import { rpcErrors } from '@metamask/rpc-errors';
3-
import { assert, getJsonSize, getSafeJson, isObject } from '@metamask/utils';
3+
import { getJsonSizeUnsafe } from '@metamask/snaps-utils';
4+
import type { Json } from '@metamask/utils';
5+
import { assert, getSafeJson, isObject } from '@metamask/utils';
46

57
import { log } from '../logging';
68

@@ -128,16 +130,13 @@ export function sanitizeRequestArguments(value: unknown): RequestArguments {
128130
* @param response - The response.
129131
* @returns True if the response is valid, otherwise false.
130132
*/
131-
export function isValidResponse(response: Record<string, unknown>) {
133+
export function isValidResponse(response: Record<string, Json>) {
132134
if (!isObject(response)) {
133135
return false;
134136
}
135137

136-
try {
137-
// If the JSON is invalid this will throw and we should return false.
138-
const size = getJsonSize(response);
139-
return size < MAX_RESPONSE_JSON_SIZE;
140-
} catch {
141-
return false;
142-
}
138+
// We know that the response is valid JSON already and we don't
139+
// need the size to be exact.
140+
const size = getJsonSizeUnsafe(response);
141+
return size < MAX_RESPONSE_JSON_SIZE;
143142
}

packages/snaps-utils/src/index.executionenv.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,6 @@ export * from './errors';
33
export * from './handlers/exports';
44
export * from './handlers/types';
55
export * from './iframe';
6+
export * from './json';
67
export * from './logging';
78
export * from './types';

0 commit comments

Comments
 (0)