Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pause timer on fetch #1756

Merged
merged 9 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/snaps-execution-environments/coverage.json
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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({
Expand Down Expand Up @@ -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();
Expand All @@ -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({
Expand Down Expand Up @@ -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();
Expand All @@ -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({
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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({
Expand All @@ -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({
FrederikBolding marked this conversation as resolved.
Show resolved Hide resolved
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';
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { NotifyFunction } from '../BaseSnapExecutor';
import { rootRealmGlobal } from '../globalObject';
import consoleEndowment from './console';
import crypto from './crypto';
Expand All @@ -11,6 +12,7 @@ import timeout from './timeout';

export type EndowmentFactoryOptions = {
snapId?: string;
notify?: NotifyFunction;
};

export type EndowmentFactory = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
Loading