From a30a3445ad05d3789f7d55e97897746d95291e4c Mon Sep 17 00:00:00 2001 From: shanejonas Date: Wed, 2 Jun 2021 15:35:27 -0700 Subject: [PATCH 01/11] refactor plugin + worker interface --- packages/controllers/src/index.ts | 2 +- .../src/plugins/PluginController.test.ts | 46 +++++++----- .../src/plugins/PluginController.ts | 70 ++++++++++--------- .../PluginExecutionEnvironmentService.ts | 25 +++++++ .../WorkerExecutionEnvironment.test.ts} | 14 ++-- .../WorkerExecutionEnvironment.ts} | 39 ++++++----- 6 files changed, 120 insertions(+), 76 deletions(-) create mode 100644 packages/controllers/src/services/PluginExecutionEnvironmentService.ts rename packages/controllers/src/{workers/WorkerController.test.ts => services/WorkerExecutionEnvironment.test.ts} (71%) rename packages/controllers/src/{workers/WorkerController.ts => services/WorkerExecutionEnvironment.ts} (88%) diff --git a/packages/controllers/src/index.ts b/packages/controllers/src/index.ts index 5d9be9cce3..555bf6df1e 100644 --- a/packages/controllers/src/index.ts +++ b/packages/controllers/src/index.ts @@ -1,3 +1,3 @@ -export * from './workers/WorkerController'; +export * from './services/WorkerExecutionEnvironment'; export * from './plugins/PluginController'; export * from './resource/ExternalResourceController'; diff --git a/packages/controllers/src/plugins/PluginController.test.ts b/packages/controllers/src/plugins/PluginController.test.ts index ad7a1102a2..7a0f707963 100644 --- a/packages/controllers/src/plugins/PluginController.test.ts +++ b/packages/controllers/src/plugins/PluginController.test.ts @@ -1,6 +1,6 @@ import fs from 'fs'; import { ControllerMessenger } from '@metamask/controllers/dist/ControllerMessenger'; -import { WorkerController } from '../workers/WorkerController'; +import { WorkerExecutionEnvironment } from '../services/WorkerExecutionEnvironment'; import { PluginController } from './PluginController'; const workerCode = fs.readFileSync( @@ -10,20 +10,26 @@ const workerCode = fs.readFileSync( describe('PluginController Controller', () => { it('can create a worker and plugin controller', async () => { - const workerController = new WorkerController({ + const workerExecutionEnvironment = new WorkerExecutionEnvironment({ setupWorkerConnection: jest.fn(), workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), }); const pluginController = new PluginController({ - command: workerController.command.bind(workerController), - createPluginWorker: workerController.createPluginWorker.bind( - workerController, + command: workerExecutionEnvironment.command.bind( + workerExecutionEnvironment, ), - terminateAll: workerController.terminateAll.bind(workerController), - terminateWorkerOf: workerController.terminateWorkerOf.bind( - workerController, + createPluginEnvironment: workerExecutionEnvironment.createPluginEnvironment.bind( + workerExecutionEnvironment, + ), + terminateAll: workerExecutionEnvironment.terminateAllPlugins.bind( + workerExecutionEnvironment, + ), + terminatePlugin: workerExecutionEnvironment.terminatePlugin.bind( + workerExecutionEnvironment, + ), + startPlugin: workerExecutionEnvironment.startPlugin.bind( + workerExecutionEnvironment, ), - startPlugin: workerController.startPlugin.bind(workerController), removeAllPermissionsFor: jest.fn(), getPermissions: jest.fn(), hasPermission: jest.fn(), @@ -42,20 +48,26 @@ describe('PluginController Controller', () => { }); it('can add a plugin and use its JSON-RPC api', async () => { - const workerController = new WorkerController({ + const workerExecutionEnvironment = new WorkerExecutionEnvironment({ setupWorkerConnection: jest.fn(), workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), }); const pluginController = new PluginController({ - command: workerController.command.bind(workerController), - createPluginWorker: workerController.createPluginWorker.bind( - workerController, + command: workerExecutionEnvironment.command.bind( + workerExecutionEnvironment, + ), + createPluginEnvironment: workerExecutionEnvironment.createPluginEnvironment.bind( + workerExecutionEnvironment, + ), + terminateAll: workerExecutionEnvironment.terminateAllPlugins.bind( + workerExecutionEnvironment, + ), + terminatePlugin: workerExecutionEnvironment.terminatePlugin.bind( + workerExecutionEnvironment, ), - terminateAll: workerController.terminateAll.bind(workerController), - terminateWorkerOf: workerController.terminateWorkerOf.bind( - workerController, + startPlugin: workerExecutionEnvironment.startPlugin.bind( + workerExecutionEnvironment, ), - startPlugin: workerController.startPlugin.bind(workerController), removeAllPermissionsFor: jest.fn(), getPermissions: jest.fn(), hasPermission: jest.fn(), diff --git a/packages/controllers/src/plugins/PluginController.ts b/packages/controllers/src/plugins/PluginController.ts index 16ca8f2ecb..cb581d9180 100644 --- a/packages/controllers/src/plugins/PluginController.ts +++ b/packages/controllers/src/plugins/PluginController.ts @@ -5,9 +5,14 @@ import { BaseControllerV2 as BaseController, RestrictedControllerMessenger, } from '@metamask/controllers'; -import { Json, JsonRpcRequest } from 'json-rpc-engine'; -import { PluginData } from '@mm-snap/types'; -import { PluginWorkerMetadata } from '../workers/WorkerController'; +import { Json } from 'json-rpc-engine'; +import { + Command, + CreatePluginEnvironment, + StartPlugin, + TerminateAll, + TerminatePlugin, +} from '../services/PluginExecutionEnvironmentService'; import { INLINE_PLUGINS } from './inlinePlugins'; export const PLUGIN_PREFIX = 'wallet_plugin_'; @@ -59,18 +64,6 @@ type HasPermissionFunction = ( permissionName: string, ) => boolean; type GetPermissionsFunction = (domain: string) => IOcapLdCapability[]; -type TerminateWorkerOf = (pluginName: string) => void; -type Command = ( - workerId: string, - message: JsonRpcRequest, -) => Promise; -type TerminateAll = () => void; -type CreatePluginWorker = (metadate: PluginWorkerMetadata) => Promise; -type StartPlugin = ( - workerId: string, - pluginData: PluginData, -) => Promise; - type PluginId = string; type StoredPlugins = Record; @@ -90,10 +83,10 @@ interface PluginControllerArgs { requestPermissions: RequestPermissionsFunction; getPermissions: GetPermissionsFunction; hasPermission: HasPermissionFunction; - terminateWorkerOf: TerminateWorkerOf; + terminatePlugin: TerminatePlugin; command: Command; terminateAll: TerminateAll; - createPluginWorker: CreatePluginWorker; + createPluginEnvironment: CreatePluginEnvironment; startPlugin: StartPlugin; } @@ -149,13 +142,13 @@ export class PluginController extends BaseController< private _hasPermission: HasPermissionFunction; - private _terminateWorkerOf: TerminateWorkerOf; + private _terminatePlugin: TerminatePlugin; private _command: Command; private _terminateAll: TerminateAll; - private _createPluginWorker: CreatePluginWorker; + private _createPluginEnvironment: CreatePluginEnvironment; private _startPlugin: StartPlugin; @@ -166,10 +159,10 @@ export class PluginController extends BaseController< closeAllConnections, requestPermissions, getPermissions, - terminateWorkerOf, + terminatePlugin, terminateAll, hasPermission, - createPluginWorker, + createPluginEnvironment, startPlugin, command, messenger, @@ -207,9 +200,9 @@ export class PluginController extends BaseController< this._getPermissions = getPermissions; this._hasPermission = hasPermission; - this._terminateWorkerOf = terminateWorkerOf; + this._terminatePlugin = terminatePlugin; this._terminateAll = terminateAll; - this._createPluginWorker = createPluginWorker; + this._createPluginEnvironment = createPluginEnvironment; this._startPlugin = startPlugin; this._command = command; @@ -236,7 +229,7 @@ export class PluginController extends BaseController< console.log(`Starting: ${pluginName}`); try { - await this._startPluginInWorker(pluginName, sourceCode); + await this._startPluginInExecutionEnvironment(pluginName, sourceCode); } catch (err) { console.warn(`Failed to start "${pluginName}", deleting it.`, err); // Clean up failed plugins: @@ -262,7 +255,10 @@ export class PluginController extends BaseController< } try { - await this._startPluginInWorker(pluginName, plugin.sourceCode); + await this._startPluginInExecutionEnvironment( + pluginName, + plugin.sourceCode, + ); } catch (err) { console.error(`Failed to start "${pluginName}".`, err); } @@ -298,7 +294,7 @@ export class PluginController extends BaseController< private _stopPlugin(pluginName: string, setNotRunning = true): void { this._removePluginHooks(pluginName); this._closeAllConnections(pluginName); - this._terminateWorkerOf(pluginName); + this._terminatePlugin(pluginName); if (setNotRunning) { this._setPluginToNotRunning(pluginName); } @@ -525,7 +521,7 @@ export class PluginController extends BaseController< await this.authorize(pluginName); - await this._startPluginInWorker(pluginName, sourceCode); + await this._startPluginInExecutionEnvironment(pluginName, sourceCode); return this.getSerializable(pluginName) as SerializablePlugin; } catch (err) { @@ -691,7 +687,10 @@ export class PluginController extends BaseController< * Test method. */ runInlinePlugin(inlinePluginName: keyof typeof INLINE_PLUGINS = 'IDLE') { - this._startPluginInWorker('inlinePlugin', INLINE_PLUGINS[inlinePluginName]); + this._startPluginInExecutionEnvironment( + 'inlinePlugin', + INLINE_PLUGINS[inlinePluginName], + ); this.update((state: any) => { state.inlinePluginIsRunning = true; }); @@ -707,12 +706,15 @@ export class PluginController extends BaseController< this.removePlugin('inlinePlugin'); } - private async _startPluginInWorker(pluginName: string, sourceCode: string) { - const workerId = await this._createPluginWorker({ + private async _startPluginInExecutionEnvironment( + pluginName: string, + sourceCode: string, + ) { + await this._createPluginEnvironment({ hostname: pluginName, }); - this._createPluginHooks(pluginName, workerId); - await this._startPlugin(workerId, { + this._createPluginHooks(pluginName); + await this._startPlugin({ pluginName, sourceCode, }); @@ -728,12 +730,12 @@ export class PluginController extends BaseController< return this._pluginRpcHooks.get(pluginName); } - private _createPluginHooks(pluginName: string, workerId: string) { + private _createPluginHooks(pluginName: string) { const rpcHook = async ( origin: string, request: Record, ) => { - return await this._command(workerId, { + return await this._command(pluginName, { id: nanoid(), jsonrpc: '2.0', method: 'pluginRpc', diff --git a/packages/controllers/src/services/PluginExecutionEnvironmentService.ts b/packages/controllers/src/services/PluginExecutionEnvironmentService.ts new file mode 100644 index 0000000000..be8f01b664 --- /dev/null +++ b/packages/controllers/src/services/PluginExecutionEnvironmentService.ts @@ -0,0 +1,25 @@ +import { JsonRpcRequest } from 'json-rpc-engine'; +import { PluginData } from '@mm-snap/types'; + +export interface PluginMetadata { + hostname: string; +} + +export type TerminatePlugin = (pluginName: string) => void; +export type Command = ( + pluginName: string, + message: JsonRpcRequest, +) => Promise; +export type TerminateAll = () => void; +export type CreatePluginEnvironment = ( + metadata: PluginMetadata, +) => Promise; +export type StartPlugin = (pluginData: PluginData) => Promise; + +export interface PluginExecutionEnvironmentService { + terminatePlugin: TerminatePlugin; + terminateAllPlugins: TerminateAll; + createPluginEnvironment: CreatePluginEnvironment; + command: Command; + startPlugin: StartPlugin; +} diff --git a/packages/controllers/src/workers/WorkerController.test.ts b/packages/controllers/src/services/WorkerExecutionEnvironment.test.ts similarity index 71% rename from packages/controllers/src/workers/WorkerController.test.ts rename to packages/controllers/src/services/WorkerExecutionEnvironment.test.ts index 3b07ff0744..538840e69b 100644 --- a/packages/controllers/src/workers/WorkerController.test.ts +++ b/packages/controllers/src/services/WorkerExecutionEnvironment.test.ts @@ -1,5 +1,5 @@ import fs from 'fs'; -import { WorkerController } from './WorkerController'; +import { WorkerExecutionEnvironment } from './WorkerExecutionEnvironment'; const workerCode = fs.readFileSync( require.resolve('@mm-snap/workers/dist/PluginWorker.js'), @@ -8,7 +8,7 @@ const workerCode = fs.readFileSync( describe('Worker Controller', () => { it('can boot', async () => { - const workerController = new WorkerController({ + const workerController = new WorkerExecutionEnvironment({ setupWorkerConnection: () => { // do nothing }, @@ -17,30 +17,30 @@ describe('Worker Controller', () => { expect(workerController).toBeDefined(); }); it('can create a plugin worker and get the workerId back', async () => { - const workerController = new WorkerController({ + const workerController = new WorkerExecutionEnvironment({ setupWorkerConnection: () => { // do nothing }, workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), }); expect( - typeof (await workerController.createPluginWorker({ + typeof (await workerController.createPluginEnvironment({ hostname: 'foobarbaz', })), ).toEqual('string'); }); it('can create a plugin worker and start the plugin', async () => { - const workerController = new WorkerController({ + const workerController = new WorkerExecutionEnvironment({ setupWorkerConnection: () => { // do nothing }, workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), }); const pluginName = 'foo.bar.baz'; - const workerId = await workerController.createPluginWorker({ + await workerController.createPluginEnvironment({ hostname: pluginName, }); - const response = await workerController.startPlugin(workerId, { + const response = await workerController.startPlugin({ pluginName, sourceCode: ` console.log('foo'); diff --git a/packages/controllers/src/workers/WorkerController.ts b/packages/controllers/src/services/WorkerExecutionEnvironment.ts similarity index 88% rename from packages/controllers/src/workers/WorkerController.ts rename to packages/controllers/src/services/WorkerExecutionEnvironment.ts index 0adaef0837..59bfad9976 100644 --- a/packages/controllers/src/workers/WorkerController.ts +++ b/packages/controllers/src/services/WorkerExecutionEnvironment.ts @@ -13,6 +13,7 @@ import { JsonRpcRequest, PendingJsonRpcResponse, } from 'json-rpc-engine'; +import { PluginExecutionEnvironmentService } from '../services/PluginExecutionEnvironmentService'; export type SetupWorkerConnection = (metadata: any, stream: Duplex) => void; @@ -37,7 +38,9 @@ interface WorkerWrapper { worker: Worker; } -export class WorkerController extends SafeEventEmitter { +export class WorkerExecutionEnvironment + extends SafeEventEmitter + implements PluginExecutionEnvironmentService { public store: ObservableStore<{ workers: Record }>; private workerUrl: URL; @@ -81,9 +84,14 @@ export class WorkerController extends SafeEventEmitter { } async command( - workerId: string, + pluginName: string, message: JsonRpcRequest, ): Promise { + const workerId = this.pluginToWorkerMap.get(pluginName); + if (workerId === undefined) { + throw new Error(`No worker found. workerId: ${workerId}`); + } + if (typeof message !== 'object') { throw new Error('Must send object.'); } @@ -103,13 +111,13 @@ export class WorkerController extends SafeEventEmitter { return response.result; } - terminateAll(): void { + terminateAllPlugins(): void { for (const workerId of this.workers.keys()) { this.terminate(workerId); } } - terminateWorkerOf(pluginName: string): void { + terminatePlugin(pluginName: string): void { const workerId = this.pluginToWorkerMap.get(pluginName); workerId && this.terminate(workerId); } @@ -134,18 +142,18 @@ export class WorkerController extends SafeEventEmitter { console.log(`worker:${workerId} terminated`); } - async startPlugin( - workerId: string, - pluginData: PluginData, - ): Promise { - const _workerId: string = workerId || this.workers.keys().next()?.value(); + async startPlugin(pluginData: PluginData): Promise { + // find worker by pluginName or use an existing workerId; + const _workerId = + this.pluginToWorkerMap.get(pluginData.pluginName) || + this.workers.keys().next()?.value; if (!_workerId) { throw new Error('No workers available.'); } - this._mapPluginAndWorker(pluginData.pluginName, workerId); + this._mapPluginAndWorker(pluginData.pluginName, _workerId); - return this.command(_workerId, { + return this.command(pluginData.pluginName, { jsonrpc: '2.0', method: 'installPlugin', params: pluginData, @@ -156,7 +164,9 @@ export class WorkerController extends SafeEventEmitter { /** * @returns The ID of the newly created worker. */ - async createPluginWorker(metadata: PluginWorkerMetadata): Promise { + async createPluginEnvironment( + metadata: PluginWorkerMetadata, + ): Promise { return this._initWorker(metadata); } @@ -211,11 +221,6 @@ export class WorkerController extends SafeEventEmitter { rpcEngine, worker, }); - await this.command(workerId, { - jsonrpc: '2.0', - method: 'ping', - id: nanoid(), - }); return workerId; } From 61639159793997f13696e1bedaa0e7e02ec46a4d Mon Sep 17 00:00:00 2001 From: shanejonas Date: Tue, 15 Jun 2021 10:37:27 -0700 Subject: [PATCH 02/11] rename WorkerExecutionEnvironment + move command + _pluginRpcHooks into WebWorkerExecutionEnvironmentService --- .../src/plugins/PluginController.test.ts | 146 ++++++++++++++---- .../src/plugins/PluginController.ts | 107 ++++--------- ...vice.ts => ExecutionEnvironmentService.ts} | 7 +- ...bWorkerExecutionEnvironmentService.test.ts | 39 +++++ ...> WebWorkerExecutionEnvironmentService.ts} | 96 +++++++----- .../WorkerExecutionEnvironment.test.ts | 51 ------ 6 files changed, 250 insertions(+), 196 deletions(-) rename packages/controllers/src/services/{PluginExecutionEnvironmentService.ts => ExecutionEnvironmentService.ts} (76%) create mode 100644 packages/controllers/src/services/WebWorkerExecutionEnvironmentService.test.ts rename packages/controllers/src/services/{WorkerExecutionEnvironment.ts => WebWorkerExecutionEnvironmentService.ts} (78%) delete mode 100644 packages/controllers/src/services/WorkerExecutionEnvironment.test.ts diff --git a/packages/controllers/src/plugins/PluginController.test.ts b/packages/controllers/src/plugins/PluginController.test.ts index 7a0f707963..03a87c62b5 100644 --- a/packages/controllers/src/plugins/PluginController.test.ts +++ b/packages/controllers/src/plugins/PluginController.test.ts @@ -1,6 +1,10 @@ import fs from 'fs'; import { ControllerMessenger } from '@metamask/controllers/dist/ControllerMessenger'; -import { WorkerExecutionEnvironment } from '../services/WorkerExecutionEnvironment'; +import { + PluginRpcHook, + WebWorkerExecutionEnvironmentService, +} from '../services/WebWorkerExecutionEnvironmentService'; +import { PluginExecutionEnvironmentService } from '../services/ExecutionEnvironmentService'; import { PluginController } from './PluginController'; const workerCode = fs.readFileSync( @@ -10,18 +14,14 @@ const workerCode = fs.readFileSync( describe('PluginController Controller', () => { it('can create a worker and plugin controller', async () => { - const workerExecutionEnvironment = new WorkerExecutionEnvironment({ - setupWorkerConnection: jest.fn(), - workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), - }); + const workerExecutionEnvironment = new WebWorkerExecutionEnvironmentService( + { + setupWorkerConnection: jest.fn(), + workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), + }, + ); const pluginController = new PluginController({ - command: workerExecutionEnvironment.command.bind( - workerExecutionEnvironment, - ), - createPluginEnvironment: workerExecutionEnvironment.createPluginEnvironment.bind( - workerExecutionEnvironment, - ), - terminateAll: workerExecutionEnvironment.terminateAllPlugins.bind( + terminateAllPlugins: workerExecutionEnvironment.terminateAllPlugins.bind( workerExecutionEnvironment, ), terminatePlugin: workerExecutionEnvironment.terminatePlugin.bind( @@ -30,6 +30,9 @@ describe('PluginController Controller', () => { startPlugin: workerExecutionEnvironment.startPlugin.bind( workerExecutionEnvironment, ), + getRpcMessageHandler: workerExecutionEnvironment.getRpcMessageHandler.bind( + workerExecutionEnvironment, + ), removeAllPermissionsFor: jest.fn(), getPermissions: jest.fn(), hasPermission: jest.fn(), @@ -47,26 +50,115 @@ describe('PluginController Controller', () => { expect(pluginController).toBeDefined(); }); - it('can add a plugin and use its JSON-RPC api', async () => { - const workerExecutionEnvironment = new WorkerExecutionEnvironment({ - setupWorkerConnection: jest.fn(), - workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), - }); + it('can add a plugin and use its JSON-RPC api with a WebWorkerExecutionEnvironmentService', async () => { + const webWorkerExecutionEnvironment = new WebWorkerExecutionEnvironmentService( + { + setupWorkerConnection: jest.fn(), + workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), + }, + ); const pluginController = new PluginController({ - command: workerExecutionEnvironment.command.bind( - workerExecutionEnvironment, + terminateAllPlugins: webWorkerExecutionEnvironment.terminateAllPlugins.bind( + webWorkerExecutionEnvironment, ), - createPluginEnvironment: workerExecutionEnvironment.createPluginEnvironment.bind( - workerExecutionEnvironment, + terminatePlugin: webWorkerExecutionEnvironment.terminatePlugin.bind( + webWorkerExecutionEnvironment, ), - terminateAll: workerExecutionEnvironment.terminateAllPlugins.bind( - workerExecutionEnvironment, + startPlugin: webWorkerExecutionEnvironment.startPlugin.bind( + webWorkerExecutionEnvironment, ), - terminatePlugin: workerExecutionEnvironment.terminatePlugin.bind( - workerExecutionEnvironment, + getRpcMessageHandler: webWorkerExecutionEnvironment.getRpcMessageHandler.bind( + webWorkerExecutionEnvironment, ), - startPlugin: workerExecutionEnvironment.startPlugin.bind( - workerExecutionEnvironment, + removeAllPermissionsFor: jest.fn(), + getPermissions: jest.fn(), + hasPermission: jest.fn(), + requestPermissions: jest.fn(), + closeAllConnections: jest.fn(), + messenger: new ControllerMessenger().getRestricted({ + name: 'PluginController', + }), + state: { + inlinePluginIsRunning: false, + pluginStates: {}, + plugins: {}, + }, + }); + const plugin = await pluginController.add({ + name: 'TestPlugin', + sourceCode: ` + wallet.registerRpcMessageHandler(async (origin, request) => { + const {method, params, id} = request; + wallet.request({method: 'setState'}) + return method + id; + }); + `, + manifest: { + web3Wallet: { + initialPermissions: {}, + }, + version: '0.0.0-development', + }, + }); + await pluginController.startPlugin(plugin.name); + const handle = pluginController.getRpcMessageHandler(plugin.name); + if (!handle) { + throw Error('rpc handler not found'); + } + const result = await handle('foo.com', { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 1, + }); + expect(result).toEqual('test1'); + }); + it('can add a plugin and use its JSON-RPC api with a stub execution env service', async () => { + class ExecutionEnvironmentStub + implements PluginExecutionEnvironmentService { + private _pluginRpcHooks: Map; + + constructor() { + this._pluginRpcHooks = new Map(); + // + } + + terminateAllPlugins() { + // empty stub + } + + getRpcMessageHandler() { + return (_: any, request: Record) => { + return new Promise((resolve) => { + const results = `${request.method}${request.id}`; + resolve(results); + }); + }; + } + + async startPlugin() { + return 'some-unique-id'; + } + + terminatePlugin() { + // empty stub + } + } + + const executionEnvironmentStub = new ExecutionEnvironmentStub(); + + const pluginController = new PluginController({ + terminateAllPlugins: executionEnvironmentStub.terminateAllPlugins.bind( + executionEnvironmentStub, + ), + terminatePlugin: executionEnvironmentStub.terminatePlugin.bind( + executionEnvironmentStub, + ), + startPlugin: executionEnvironmentStub.startPlugin.bind( + executionEnvironmentStub, + ), + getRpcMessageHandler: executionEnvironmentStub.getRpcMessageHandler.bind( + executionEnvironmentStub, ), removeAllPermissionsFor: jest.fn(), getPermissions: jest.fn(), diff --git a/packages/controllers/src/plugins/PluginController.ts b/packages/controllers/src/plugins/PluginController.ts index cb581d9180..73875e7a7c 100644 --- a/packages/controllers/src/plugins/PluginController.ts +++ b/packages/controllers/src/plugins/PluginController.ts @@ -1,18 +1,17 @@ import { ethErrors, serializeError } from 'eth-rpc-errors'; import { IOcapLdCapability } from 'rpc-cap/dist/src/@types/ocap-ld'; -import { nanoid } from 'nanoid'; import { BaseControllerV2 as BaseController, RestrictedControllerMessenger, } from '@metamask/controllers'; import { Json } from 'json-rpc-engine'; import { - Command, - CreatePluginEnvironment, + GetRpcMessageHandler, StartPlugin, TerminateAll, TerminatePlugin, } from '../services/PluginExecutionEnvironmentService'; +import { PluginRpcHook } from '../services/WorkerExecutionEnvironment'; import { INLINE_PLUGINS } from './inlinePlugins'; export const PLUGIN_PREFIX = 'wallet_plugin_'; @@ -39,12 +38,6 @@ export interface Plugin extends SerializablePlugin { sourceCode: string; } -// The plugin is the callee -export type PluginRpcHook = ( - origin: string, - request: Record, -) => Promise; - export type ProcessPluginReturnType = | SerializablePlugin | { error: ReturnType }; @@ -84,10 +77,9 @@ interface PluginControllerArgs { getPermissions: GetPermissionsFunction; hasPermission: HasPermissionFunction; terminatePlugin: TerminatePlugin; - command: Command; - terminateAll: TerminateAll; - createPluginEnvironment: CreatePluginEnvironment; + terminateAllPlugins: TerminateAll; startPlugin: StartPlugin; + getRpcMessageHandler: GetRpcMessageHandler; } interface AddPluginBase { @@ -132,8 +124,6 @@ export class PluginController extends BaseController< > { private _removeAllPermissionsFor: RemoveAllPermissionsFunction; - private _pluginRpcHooks: Map; - private _closeAllConnections: CloseAllConnectionsFunction; private _requestPermissions: RequestPermissionsFunction; @@ -144,14 +134,12 @@ export class PluginController extends BaseController< private _terminatePlugin: TerminatePlugin; - private _command: Command; - - private _terminateAll: TerminateAll; - - private _createPluginEnvironment: CreatePluginEnvironment; + private _terminateAllPlugins: TerminateAll; private _startPlugin: StartPlugin; + private _getRpcMessageHandler: GetRpcMessageHandler; + private _pluginsBeingAdded: Map>; constructor({ @@ -160,11 +148,10 @@ export class PluginController extends BaseController< requestPermissions, getPermissions, terminatePlugin, - terminateAll, + terminateAllPlugins, hasPermission, - createPluginEnvironment, startPlugin, - command, + getRpcMessageHandler, messenger, state, }: PluginControllerArgs) { @@ -201,12 +188,10 @@ export class PluginController extends BaseController< this._hasPermission = hasPermission; this._terminatePlugin = terminatePlugin; - this._terminateAll = terminateAll; - this._createPluginEnvironment = createPluginEnvironment; + this._terminateAllPlugins = terminateAllPlugins; this._startPlugin = startPlugin; - this._command = command; + this._getRpcMessageHandler = getRpcMessageHandler; - this._pluginRpcHooks = new Map(); this._pluginsBeingAdded = new Map(); } @@ -229,7 +214,10 @@ export class PluginController extends BaseController< console.log(`Starting: ${pluginName}`); try { - await this._startPluginInExecutionEnvironment(pluginName, sourceCode); + await this._startPlugin({ + pluginName, + sourceCode, + }); } catch (err) { console.warn(`Failed to start "${pluginName}", deleting it.`, err); // Clean up failed plugins: @@ -255,13 +243,14 @@ export class PluginController extends BaseController< } try { - await this._startPluginInExecutionEnvironment( + await this._startPlugin({ pluginName, - plugin.sourceCode, - ); + sourceCode: plugin.sourceCode, + }); } catch (err) { console.error(`Failed to start "${pluginName}".`, err); } + this._setPluginToRunning(pluginName); } /** @@ -292,7 +281,6 @@ export class PluginController extends BaseController< * Should only be set to false if the plugin is about to be deleted. */ private _stopPlugin(pluginName: string, setNotRunning = true): void { - this._removePluginHooks(pluginName); this._closeAllConnections(pluginName); this._terminatePlugin(pluginName); if (setNotRunning) { @@ -387,12 +375,11 @@ export class PluginController extends BaseController< * handlers, event listeners, and permissions; tear down all plugin providers. */ clearState() { - this._pluginRpcHooks.clear(); const pluginNames = Object.keys(this.state.plugins); pluginNames.forEach((pluginName) => { this._closeAllConnections(pluginName); }); - this._terminateAll(); + this._terminateAllPlugins(); this._removeAllPermissionsFor(pluginNames); this.update((state: any) => { state.inlinePluginIsRunning = false; @@ -521,7 +508,10 @@ export class PluginController extends BaseController< await this.authorize(pluginName); - await this._startPluginInExecutionEnvironment(pluginName, sourceCode); + await this._startPlugin({ + pluginName, + sourceCode, + }); return this.getSerializable(pluginName) as SerializablePlugin; } catch (err) { @@ -687,10 +677,10 @@ export class PluginController extends BaseController< * Test method. */ runInlinePlugin(inlinePluginName: keyof typeof INLINE_PLUGINS = 'IDLE') { - this._startPluginInExecutionEnvironment( - 'inlinePlugin', - INLINE_PLUGINS[inlinePluginName], - ); + this._startPlugin({ + pluginName: 'inlinePlugin', + sourceCode: INLINE_PLUGINS[inlinePluginName], + }); this.update((state: any) => { state.inlinePluginIsRunning = true; }); @@ -706,52 +696,13 @@ export class PluginController extends BaseController< this.removePlugin('inlinePlugin'); } - private async _startPluginInExecutionEnvironment( - pluginName: string, - sourceCode: string, - ) { - await this._createPluginEnvironment({ - hostname: pluginName, - }); - this._createPluginHooks(pluginName); - await this._startPlugin({ - pluginName, - sourceCode, - }); - this._setPluginToRunning(pluginName); - } - /** * Gets the RPC message handler for the given plugin. * * @param pluginName - The name of the plugin whose message handler to get. */ getRpcMessageHandler(pluginName: string): PluginRpcHook | undefined { - return this._pluginRpcHooks.get(pluginName); - } - - private _createPluginHooks(pluginName: string) { - const rpcHook = async ( - origin: string, - request: Record, - ) => { - return await this._command(pluginName, { - id: nanoid(), - jsonrpc: '2.0', - method: 'pluginRpc', - params: { - origin, - request, - target: pluginName, - }, - }); - }; - - this._pluginRpcHooks.set(pluginName, rpcHook); - } - - private _removePluginHooks(pluginName: string) { - this._pluginRpcHooks.delete(pluginName); + return this._getRpcMessageHandler(pluginName); } private _setPluginToRunning(pluginName: string): void { diff --git a/packages/controllers/src/services/PluginExecutionEnvironmentService.ts b/packages/controllers/src/services/ExecutionEnvironmentService.ts similarity index 76% rename from packages/controllers/src/services/PluginExecutionEnvironmentService.ts rename to packages/controllers/src/services/ExecutionEnvironmentService.ts index be8f01b664..fa67e63955 100644 --- a/packages/controllers/src/services/PluginExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/ExecutionEnvironmentService.ts @@ -1,5 +1,6 @@ import { JsonRpcRequest } from 'json-rpc-engine'; import { PluginData } from '@mm-snap/types'; +import { PluginRpcHook } from './WebWorkerExecutionEnvironmentService'; export interface PluginMetadata { hostname: string; @@ -15,11 +16,13 @@ export type CreatePluginEnvironment = ( metadata: PluginMetadata, ) => Promise; export type StartPlugin = (pluginData: PluginData) => Promise; +export type GetRpcMessageHandler = ( + pluginName: string, +) => PluginRpcHook | undefined; export interface PluginExecutionEnvironmentService { terminatePlugin: TerminatePlugin; terminateAllPlugins: TerminateAll; - createPluginEnvironment: CreatePluginEnvironment; - command: Command; startPlugin: StartPlugin; + getRpcMessageHandler: GetRpcMessageHandler; } diff --git a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.test.ts b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.test.ts new file mode 100644 index 0000000000..6b5eccdbc9 --- /dev/null +++ b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.test.ts @@ -0,0 +1,39 @@ +import fs from 'fs'; +import { WebWorkerExecutionEnvironmentService } from './WebWorkerExecutionEnvironmentService'; + +const workerCode = fs.readFileSync( + require.resolve('@mm-snap/workers/dist/PluginWorker.js'), + 'utf8', +); + +describe('Worker Controller', () => { + it('can boot', async () => { + const webWorkerExecutionEnvironmentService = new WebWorkerExecutionEnvironmentService( + { + setupWorkerConnection: () => { + // do nothing + }, + workerUrl: new URL('https://foo.bar.baz'), + }, + ); + expect(webWorkerExecutionEnvironmentService).toBeDefined(); + }); + it('can create a plugin worker and start the plugin', async () => { + const webWorkerExecutionEnvironmentService = new WebWorkerExecutionEnvironmentService( + { + setupWorkerConnection: () => { + // do nothing + }, + workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), + }, + ); + const pluginName = 'foo.bar.baz'; + const response = await webWorkerExecutionEnvironmentService.startPlugin({ + pluginName, + sourceCode: ` + console.log('foo'); + `, + }); + expect(response).toEqual('OK'); + }); +}); diff --git a/packages/controllers/src/services/WorkerExecutionEnvironment.ts b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts similarity index 78% rename from packages/controllers/src/services/WorkerExecutionEnvironment.ts rename to packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts index 59bfad9976..312687197a 100644 --- a/packages/controllers/src/services/WorkerExecutionEnvironment.ts +++ b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts @@ -13,13 +13,10 @@ import { JsonRpcRequest, PendingJsonRpcResponse, } from 'json-rpc-engine'; -import { PluginExecutionEnvironmentService } from '../services/PluginExecutionEnvironmentService'; +import { PluginExecutionEnvironmentService } from '../services/ExecutionEnvironmentService'; -export type SetupWorkerConnection = (metadata: any, stream: Duplex) => void; +export type SetupWorkerConnection = (stream: Duplex) => void; -export interface PluginWorkerMetadata { - hostname: string; -} interface WorkerControllerArgs { setupWorkerConnection: SetupWorkerConnection; workerUrl: URL; @@ -31,6 +28,12 @@ interface WorkerStreams { _connection: WorkerParentPostMessageStream; } +// The plugin is the callee +export type PluginRpcHook = ( + origin: string, + request: Record, +) => Promise; + interface WorkerWrapper { workerId: string; streams: WorkerStreams; @@ -38,11 +41,13 @@ interface WorkerWrapper { worker: Worker; } -export class WorkerExecutionEnvironment +export class WebWorkerExecutionEnvironmentService extends SafeEventEmitter implements PluginExecutionEnvironmentService { public store: ObservableStore<{ workers: Record }>; + private _pluginRpcHooks: Map; + private workerUrl: URL; private workers: Map; @@ -61,6 +66,7 @@ export class WorkerExecutionEnvironment this.workers = new Map(); this.pluginToWorkerMap = new Map(); this.workerToPluginMap = new Map(); + this._pluginRpcHooks = new Map(); } _setWorker(workerId: string, workerObj: WorkerWrapper): void { @@ -83,15 +89,10 @@ export class WorkerExecutionEnvironment this.store.updateState({ workers: newWorkerState }); } - async command( - pluginName: string, + async _command( + workerId: string, message: JsonRpcRequest, ): Promise { - const workerId = this.pluginToWorkerMap.get(pluginName); - if (workerId === undefined) { - throw new Error(`No worker found. workerId: ${workerId}`); - } - if (typeof message !== 'object') { throw new Error('Must send object.'); } @@ -115,11 +116,13 @@ export class WorkerExecutionEnvironment for (const workerId of this.workers.keys()) { this.terminate(workerId); } + this._pluginRpcHooks.clear(); } terminatePlugin(pluginName: string): void { const workerId = this.pluginToWorkerMap.get(pluginName); workerId && this.terminate(workerId); + this._removePluginHooks(pluginName); } terminate(workerId: string): void { @@ -142,34 +145,55 @@ export class WorkerExecutionEnvironment console.log(`worker:${workerId} terminated`); } + /** + * Gets the RPC message handler for the given plugin. + * + * @param pluginName - The name of the plugin whose message handler to get. + */ + getRpcMessageHandler(pluginName: string): PluginRpcHook | undefined { + return this._pluginRpcHooks.get(pluginName); + } + + private _removePluginHooks(pluginName: string) { + this._pluginRpcHooks.delete(pluginName); + } + + private _createPluginHooks(pluginName: string, workerId: string) { + const rpcHook = async ( + origin: string, + request: Record, + ) => { + return await this._command(workerId, { + id: nanoid(), + jsonrpc: '2.0', + method: 'pluginRpc', + params: { + origin, + request, + target: pluginName, + }, + }); + }; + + this._pluginRpcHooks.set(pluginName, rpcHook); + } + async startPlugin(pluginData: PluginData): Promise { - // find worker by pluginName or use an existing workerId; - const _workerId = - this.pluginToWorkerMap.get(pluginData.pluginName) || - this.workers.keys().next()?.value; - if (!_workerId) { - throw new Error('No workers available.'); - } + const _workerId = await this._initWorker(); this._mapPluginAndWorker(pluginData.pluginName, _workerId); - return this.command(pluginData.pluginName, { + return this._command(_workerId, { jsonrpc: '2.0', method: 'installPlugin', params: pluginData, id: nanoid(), + }).then((result: unknown) => { + this._createPluginHooks(pluginData.pluginName, _workerId); + return result; }); } - /** - * @returns The ID of the newly created worker. - */ - async createPluginEnvironment( - metadata: PluginWorkerMetadata, - ): Promise { - return this._initWorker(metadata); - } - _mapPluginAndWorker(pluginName: string, workerId: string): void { this.pluginToWorkerMap.set(pluginName, workerId); this.workerToPluginMap.set(workerId, pluginName); @@ -199,14 +223,14 @@ export class WorkerExecutionEnvironment this.pluginToWorkerMap.delete(pluginName); } - async _initWorker(metadata: PluginWorkerMetadata): Promise { + async _initWorker(): Promise { console.log('_initWorker'); const workerId = nanoid(); const worker = new Worker(this.workerUrl, { name: workerId, }); - const streams = this._initWorkerStreams(worker, workerId, metadata); + const streams = this._initWorkerStreams(worker, workerId); const rpcEngine = new JsonRpcEngine(); const jsonRpcConnection = createStreamMiddleware(); @@ -224,11 +248,7 @@ export class WorkerExecutionEnvironment return workerId; } - _initWorkerStreams( - worker: Worker, - workerId: string, - metadata: PluginWorkerMetadata, - ): WorkerStreams { + _initWorkerStreams(worker: Worker, workerId: string): WorkerStreams { const workerStream = new WorkerParentPostMessageStream({ worker }); // Typecast justification: stream type mismatch const mux = setupMultiplex( @@ -239,7 +259,7 @@ export class WorkerExecutionEnvironment const commandStream = mux.createStream(PLUGIN_STREAM_NAMES.COMMAND); const rpcStream = mux.createStream(PLUGIN_STREAM_NAMES.JSON_RPC); - this.setupWorkerConnection(metadata, (rpcStream as unknown) as Duplex); + this.setupWorkerConnection((rpcStream as unknown) as Duplex); // Typecast justification: stream type mismatch return { diff --git a/packages/controllers/src/services/WorkerExecutionEnvironment.test.ts b/packages/controllers/src/services/WorkerExecutionEnvironment.test.ts deleted file mode 100644 index 538840e69b..0000000000 --- a/packages/controllers/src/services/WorkerExecutionEnvironment.test.ts +++ /dev/null @@ -1,51 +0,0 @@ -import fs from 'fs'; -import { WorkerExecutionEnvironment } from './WorkerExecutionEnvironment'; - -const workerCode = fs.readFileSync( - require.resolve('@mm-snap/workers/dist/PluginWorker.js'), - 'utf8', -); - -describe('Worker Controller', () => { - it('can boot', async () => { - const workerController = new WorkerExecutionEnvironment({ - setupWorkerConnection: () => { - // do nothing - }, - workerUrl: new URL('https://foo.bar.baz'), - }); - expect(workerController).toBeDefined(); - }); - it('can create a plugin worker and get the workerId back', async () => { - const workerController = new WorkerExecutionEnvironment({ - setupWorkerConnection: () => { - // do nothing - }, - workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), - }); - expect( - typeof (await workerController.createPluginEnvironment({ - hostname: 'foobarbaz', - })), - ).toEqual('string'); - }); - it('can create a plugin worker and start the plugin', async () => { - const workerController = new WorkerExecutionEnvironment({ - setupWorkerConnection: () => { - // do nothing - }, - workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), - }); - const pluginName = 'foo.bar.baz'; - await workerController.createPluginEnvironment({ - hostname: pluginName, - }); - const response = await workerController.startPlugin({ - pluginName, - sourceCode: ` - console.log('foo'); - `, - }); - expect(response).toEqual('OK'); - }); -}); From e8e30e5bc17e29e4224f5037bc5777a36667d177 Mon Sep 17 00:00:00 2001 From: shanejonas Date: Tue, 15 Jun 2021 10:41:00 -0700 Subject: [PATCH 03/11] fix: add ping back --- packages/controllers/src/plugins/PluginController.ts | 4 ++-- .../src/services/WebWorkerExecutionEnvironmentService.ts | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/controllers/src/plugins/PluginController.ts b/packages/controllers/src/plugins/PluginController.ts index 73875e7a7c..1db3d76cb1 100644 --- a/packages/controllers/src/plugins/PluginController.ts +++ b/packages/controllers/src/plugins/PluginController.ts @@ -10,8 +10,8 @@ import { StartPlugin, TerminateAll, TerminatePlugin, -} from '../services/PluginExecutionEnvironmentService'; -import { PluginRpcHook } from '../services/WorkerExecutionEnvironment'; +} from '../services/ExecutionEnvironmentService'; +import { PluginRpcHook } from '../services/WebWorkerExecutionEnvironmentService'; import { INLINE_PLUGINS } from './inlinePlugins'; export const PLUGIN_PREFIX = 'wallet_plugin_'; diff --git a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts index 312687197a..a77fb9f736 100644 --- a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts @@ -245,6 +245,11 @@ export class WebWorkerExecutionEnvironmentService rpcEngine, worker, }); + await this._command(workerId, { + jsonrpc: '2.0', + method: 'ping', + id: nanoid(), + }); return workerId; } From c996ee497e8382d078bb2c29a97158eb0b5da12e Mon Sep 17 00:00:00 2001 From: shanejonas Date: Tue, 15 Jun 2021 10:44:20 -0700 Subject: [PATCH 04/11] remove EventEmitter from WebWorkerExecutionEnvironmentService --- packages/controllers/src/index.ts | 2 +- .../src/plugins/PluginController.test.ts | 18 +++--------------- .../services/ExecutionEnvironmentService.ts | 2 +- .../WebWorkerExecutionEnvironmentService.ts | 7 ++----- 4 files changed, 7 insertions(+), 22 deletions(-) diff --git a/packages/controllers/src/index.ts b/packages/controllers/src/index.ts index 555bf6df1e..4a1f01cd2d 100644 --- a/packages/controllers/src/index.ts +++ b/packages/controllers/src/index.ts @@ -1,3 +1,3 @@ -export * from './services/WorkerExecutionEnvironment'; +export * from './services/WebWorkerExecutionEnvironmentService'; export * from './plugins/PluginController'; export * from './resource/ExternalResourceController'; diff --git a/packages/controllers/src/plugins/PluginController.test.ts b/packages/controllers/src/plugins/PluginController.test.ts index 03a87c62b5..4cd5fdf1c1 100644 --- a/packages/controllers/src/plugins/PluginController.test.ts +++ b/packages/controllers/src/plugins/PluginController.test.ts @@ -1,10 +1,7 @@ import fs from 'fs'; import { ControllerMessenger } from '@metamask/controllers/dist/ControllerMessenger'; -import { - PluginRpcHook, - WebWorkerExecutionEnvironmentService, -} from '../services/WebWorkerExecutionEnvironmentService'; -import { PluginExecutionEnvironmentService } from '../services/ExecutionEnvironmentService'; +import { WebWorkerExecutionEnvironmentService } from '../services/WebWorkerExecutionEnvironmentService'; +import { ExecutionEnvironmentService } from '../services/ExecutionEnvironmentService'; import { PluginController } from './PluginController'; const workerCode = fs.readFileSync( @@ -114,15 +111,7 @@ describe('PluginController Controller', () => { expect(result).toEqual('test1'); }); it('can add a plugin and use its JSON-RPC api with a stub execution env service', async () => { - class ExecutionEnvironmentStub - implements PluginExecutionEnvironmentService { - private _pluginRpcHooks: Map; - - constructor() { - this._pluginRpcHooks = new Map(); - // - } - + class ExecutionEnvironmentStub implements ExecutionEnvironmentService { terminateAllPlugins() { // empty stub } @@ -179,7 +168,6 @@ describe('PluginController Controller', () => { sourceCode: ` wallet.registerRpcMessageHandler(async (origin, request) => { const {method, params, id} = request; - wallet.request({method: 'setState'}) return method + id; }); `, diff --git a/packages/controllers/src/services/ExecutionEnvironmentService.ts b/packages/controllers/src/services/ExecutionEnvironmentService.ts index fa67e63955..2776dc6420 100644 --- a/packages/controllers/src/services/ExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/ExecutionEnvironmentService.ts @@ -20,7 +20,7 @@ export type GetRpcMessageHandler = ( pluginName: string, ) => PluginRpcHook | undefined; -export interface PluginExecutionEnvironmentService { +export interface ExecutionEnvironmentService { terminatePlugin: TerminatePlugin; terminateAllPlugins: TerminateAll; startPlugin: StartPlugin; diff --git a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts index a77fb9f736..dc7ae71510 100644 --- a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts @@ -2,7 +2,6 @@ import { Duplex } from 'stream'; import { nanoid } from 'nanoid'; import pump from 'pump'; import { ObservableStore } from '@metamask/obs-store'; -import SafeEventEmitter from '@metamask/safe-event-emitter'; import ObjectMultiplex from '@metamask/object-multiplex'; import { WorkerParentPostMessageStream } from '@metamask/post-message-stream'; import { PLUGIN_STREAM_NAMES } from '@mm-snap/workers'; @@ -13,7 +12,7 @@ import { JsonRpcRequest, PendingJsonRpcResponse, } from 'json-rpc-engine'; -import { PluginExecutionEnvironmentService } from '../services/ExecutionEnvironmentService'; +import { ExecutionEnvironmentService } from '../services/ExecutionEnvironmentService'; export type SetupWorkerConnection = (stream: Duplex) => void; @@ -42,8 +41,7 @@ interface WorkerWrapper { } export class WebWorkerExecutionEnvironmentService - extends SafeEventEmitter - implements PluginExecutionEnvironmentService { + implements ExecutionEnvironmentService { public store: ObservableStore<{ workers: Record }>; private _pluginRpcHooks: Map; @@ -59,7 +57,6 @@ export class WebWorkerExecutionEnvironmentService private workerToPluginMap: Map; constructor({ setupWorkerConnection, workerUrl }: WorkerControllerArgs) { - super(); this.workerUrl = workerUrl; this.setupWorkerConnection = setupWorkerConnection; this.store = new ObservableStore({ workers: {} }); From b8592701c3cbf97518d53392f27fcd65bcf513eb Mon Sep 17 00:00:00 2001 From: shanejonas Date: Tue, 15 Jun 2021 13:58:41 -0700 Subject: [PATCH 05/11] add _executePlugin method for bookkeeping plugins running state --- .../src/plugins/PluginController.test.ts | 11 ++++++----- .../src/plugins/PluginController.ts | 19 ++++++++++++------- .../services/ExecutionEnvironmentService.ts | 6 +++--- .../WebWorkerExecutionEnvironmentService.ts | 6 +++--- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/packages/controllers/src/plugins/PluginController.test.ts b/packages/controllers/src/plugins/PluginController.test.ts index 4cd5fdf1c1..4938b10538 100644 --- a/packages/controllers/src/plugins/PluginController.test.ts +++ b/packages/controllers/src/plugins/PluginController.test.ts @@ -98,7 +98,7 @@ describe('PluginController Controller', () => { }, }); await pluginController.startPlugin(plugin.name); - const handle = pluginController.getRpcMessageHandler(plugin.name); + const handle = await pluginController.getRpcMessageHandler(plugin.name); if (!handle) { throw Error('rpc handler not found'); } @@ -110,13 +110,14 @@ describe('PluginController Controller', () => { }); expect(result).toEqual('test1'); }); + it('can add a plugin and use its JSON-RPC api with a stub execution env service', async () => { class ExecutionEnvironmentStub implements ExecutionEnvironmentService { - terminateAllPlugins() { + async terminateAllPlugins() { // empty stub } - getRpcMessageHandler() { + async getRpcMessageHandler() { return (_: any, request: Record) => { return new Promise((resolve) => { const results = `${request.method}${request.id}`; @@ -129,7 +130,7 @@ describe('PluginController Controller', () => { return 'some-unique-id'; } - terminatePlugin() { + async terminatePlugin() { // empty stub } } @@ -179,7 +180,7 @@ describe('PluginController Controller', () => { }, }); await pluginController.startPlugin(plugin.name); - const handle = pluginController.getRpcMessageHandler(plugin.name); + const handle = await pluginController.getRpcMessageHandler(plugin.name); if (!handle) { throw Error('rpc handler not found'); } diff --git a/packages/controllers/src/plugins/PluginController.ts b/packages/controllers/src/plugins/PluginController.ts index 1db3d76cb1..bc4bbadab9 100644 --- a/packages/controllers/src/plugins/PluginController.ts +++ b/packages/controllers/src/plugins/PluginController.ts @@ -5,13 +5,13 @@ import { RestrictedControllerMessenger, } from '@metamask/controllers'; import { Json } from 'json-rpc-engine'; +import { PluginData } from '@mm-snap/types'; import { GetRpcMessageHandler, StartPlugin, TerminateAll, TerminatePlugin, } from '../services/ExecutionEnvironmentService'; -import { PluginRpcHook } from '../services/WebWorkerExecutionEnvironmentService'; import { INLINE_PLUGINS } from './inlinePlugins'; export const PLUGIN_PREFIX = 'wallet_plugin_'; @@ -214,7 +214,7 @@ export class PluginController extends BaseController< console.log(`Starting: ${pluginName}`); try { - await this._startPlugin({ + await this._executePlugin({ pluginName, sourceCode, }); @@ -243,14 +243,13 @@ export class PluginController extends BaseController< } try { - await this._startPlugin({ + await this._executePlugin({ pluginName, sourceCode: plugin.sourceCode, }); } catch (err) { console.error(`Failed to start "${pluginName}".`, err); } - this._setPluginToRunning(pluginName); } /** @@ -508,7 +507,7 @@ export class PluginController extends BaseController< await this.authorize(pluginName); - await this._startPlugin({ + await this._executePlugin({ pluginName, sourceCode, }); @@ -551,6 +550,12 @@ export class PluginController extends BaseController< return this._pluginsBeingAdded.get(pluginName) as Promise; } + private async _executePlugin(pluginData: PluginData) { + const result = await this._startPlugin(pluginData); + this._setPluginToRunning(pluginData.pluginName); + return result; + } + /** * Internal method. See the "add" method. * @@ -677,7 +682,7 @@ export class PluginController extends BaseController< * Test method. */ runInlinePlugin(inlinePluginName: keyof typeof INLINE_PLUGINS = 'IDLE') { - this._startPlugin({ + this._executePlugin({ pluginName: 'inlinePlugin', sourceCode: INLINE_PLUGINS[inlinePluginName], }); @@ -701,7 +706,7 @@ export class PluginController extends BaseController< * * @param pluginName - The name of the plugin whose message handler to get. */ - getRpcMessageHandler(pluginName: string): PluginRpcHook | undefined { + async getRpcMessageHandler(pluginName: string) { return this._getRpcMessageHandler(pluginName); } diff --git a/packages/controllers/src/services/ExecutionEnvironmentService.ts b/packages/controllers/src/services/ExecutionEnvironmentService.ts index 2776dc6420..23afe67884 100644 --- a/packages/controllers/src/services/ExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/ExecutionEnvironmentService.ts @@ -6,19 +6,19 @@ export interface PluginMetadata { hostname: string; } -export type TerminatePlugin = (pluginName: string) => void; +export type TerminatePlugin = (pluginName: string) => Promise; export type Command = ( pluginName: string, message: JsonRpcRequest, ) => Promise; -export type TerminateAll = () => void; +export type TerminateAll = () => Promise; export type CreatePluginEnvironment = ( metadata: PluginMetadata, ) => Promise; export type StartPlugin = (pluginData: PluginData) => Promise; export type GetRpcMessageHandler = ( pluginName: string, -) => PluginRpcHook | undefined; +) => Promise; export interface ExecutionEnvironmentService { terminatePlugin: TerminatePlugin; diff --git a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts index dc7ae71510..522da952b7 100644 --- a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts @@ -109,14 +109,14 @@ export class WebWorkerExecutionEnvironmentService return response.result; } - terminateAllPlugins(): void { + async terminateAllPlugins() { for (const workerId of this.workers.keys()) { this.terminate(workerId); } this._pluginRpcHooks.clear(); } - terminatePlugin(pluginName: string): void { + async terminatePlugin(pluginName: string) { const workerId = this.pluginToWorkerMap.get(pluginName); workerId && this.terminate(workerId); this._removePluginHooks(pluginName); @@ -147,7 +147,7 @@ export class WebWorkerExecutionEnvironmentService * * @param pluginName - The name of the plugin whose message handler to get. */ - getRpcMessageHandler(pluginName: string): PluginRpcHook | undefined { + async getRpcMessageHandler(pluginName: string) { return this._pluginRpcHooks.get(pluginName); } From 230ceb2d369c6c7a6386887976980aa08c7973db Mon Sep 17 00:00:00 2001 From: shanejonas Date: Tue, 15 Jun 2021 14:03:58 -0700 Subject: [PATCH 06/11] add await in invokePlugin --- packages/rpc-methods/src/restricted/invokePlugin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rpc-methods/src/restricted/invokePlugin.ts b/packages/rpc-methods/src/restricted/invokePlugin.ts index 1570d59459..872c790976 100644 --- a/packages/rpc-methods/src/restricted/invokePlugin.ts +++ b/packages/rpc-methods/src/restricted/invokePlugin.ts @@ -64,7 +64,7 @@ function getInvokePluginHandlerGetter({ }); } - const handler = getPluginRpcHandler(pluginOriginString); + const handler = await getPluginRpcHandler(pluginOriginString); if (!handler) { return end( ethErrors.rpc.methodNotFound({ From db6e05d699da9ce6ad9b07c723b57a6f2c57f7b8 Mon Sep 17 00:00:00 2001 From: shanejonas Date: Tue, 15 Jun 2021 14:05:02 -0700 Subject: [PATCH 07/11] make _command private --- .../src/services/WebWorkerExecutionEnvironmentService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts index 522da952b7..ab6372e69b 100644 --- a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts @@ -86,7 +86,7 @@ export class WebWorkerExecutionEnvironmentService this.store.updateState({ workers: newWorkerState }); } - async _command( + private async _command( workerId: string, message: JsonRpcRequest, ): Promise { From 73e8527d3a5603e91a6a050c229ddd7fbf8c3a0f Mon Sep 17 00:00:00 2001 From: shanejonas Date: Tue, 15 Jun 2021 14:09:15 -0700 Subject: [PATCH 08/11] refactor startPlugin code to use await internally --- .../src/services/WebWorkerExecutionEnvironmentService.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts index ab6372e69b..fce8774318 100644 --- a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts @@ -180,15 +180,14 @@ export class WebWorkerExecutionEnvironmentService this._mapPluginAndWorker(pluginData.pluginName, _workerId); - return this._command(_workerId, { + const result = await this._command(_workerId, { jsonrpc: '2.0', method: 'installPlugin', params: pluginData, id: nanoid(), - }).then((result: unknown) => { - this._createPluginHooks(pluginData.pluginName, _workerId); - return result; }); + this._createPluginHooks(pluginData.pluginName, _workerId); + return result; } _mapPluginAndWorker(pluginName: string, workerId: string): void { From f62bb3d10e059464a0c76a1eb461ecf0a78426e8 Mon Sep 17 00:00:00 2001 From: shanejonas Date: Tue, 15 Jun 2021 14:48:56 -0700 Subject: [PATCH 09/11] rename startPlugin to executePlugin in ExecutionEnvironmentService --- .../src/plugins/PluginController.test.ts | 8 +++---- .../src/plugins/PluginController.ts | 22 +++++++++---------- .../services/ExecutionEnvironmentService.ts | 4 ++-- ...bWorkerExecutionEnvironmentService.test.ts | 2 +- .../WebWorkerExecutionEnvironmentService.ts | 2 +- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/controllers/src/plugins/PluginController.test.ts b/packages/controllers/src/plugins/PluginController.test.ts index 4938b10538..34c01512ba 100644 --- a/packages/controllers/src/plugins/PluginController.test.ts +++ b/packages/controllers/src/plugins/PluginController.test.ts @@ -24,7 +24,7 @@ describe('PluginController Controller', () => { terminatePlugin: workerExecutionEnvironment.terminatePlugin.bind( workerExecutionEnvironment, ), - startPlugin: workerExecutionEnvironment.startPlugin.bind( + executePlugin: workerExecutionEnvironment.executePlugin.bind( workerExecutionEnvironment, ), getRpcMessageHandler: workerExecutionEnvironment.getRpcMessageHandler.bind( @@ -61,7 +61,7 @@ describe('PluginController Controller', () => { terminatePlugin: webWorkerExecutionEnvironment.terminatePlugin.bind( webWorkerExecutionEnvironment, ), - startPlugin: webWorkerExecutionEnvironment.startPlugin.bind( + executePlugin: webWorkerExecutionEnvironment.executePlugin.bind( webWorkerExecutionEnvironment, ), getRpcMessageHandler: webWorkerExecutionEnvironment.getRpcMessageHandler.bind( @@ -126,7 +126,7 @@ describe('PluginController Controller', () => { }; } - async startPlugin() { + async executePlugin() { return 'some-unique-id'; } @@ -144,7 +144,7 @@ describe('PluginController Controller', () => { terminatePlugin: executionEnvironmentStub.terminatePlugin.bind( executionEnvironmentStub, ), - startPlugin: executionEnvironmentStub.startPlugin.bind( + executePlugin: executionEnvironmentStub.executePlugin.bind( executionEnvironmentStub, ), getRpcMessageHandler: executionEnvironmentStub.getRpcMessageHandler.bind( diff --git a/packages/controllers/src/plugins/PluginController.ts b/packages/controllers/src/plugins/PluginController.ts index bc4bbadab9..c73d7ce4ee 100644 --- a/packages/controllers/src/plugins/PluginController.ts +++ b/packages/controllers/src/plugins/PluginController.ts @@ -8,7 +8,7 @@ import { Json } from 'json-rpc-engine'; import { PluginData } from '@mm-snap/types'; import { GetRpcMessageHandler, - StartPlugin, + ExecutePlugin, TerminateAll, TerminatePlugin, } from '../services/ExecutionEnvironmentService'; @@ -78,7 +78,7 @@ interface PluginControllerArgs { hasPermission: HasPermissionFunction; terminatePlugin: TerminatePlugin; terminateAllPlugins: TerminateAll; - startPlugin: StartPlugin; + executePlugin: ExecutePlugin; getRpcMessageHandler: GetRpcMessageHandler; } @@ -136,7 +136,7 @@ export class PluginController extends BaseController< private _terminateAllPlugins: TerminateAll; - private _startPlugin: StartPlugin; + private _executePlugin: ExecutePlugin; private _getRpcMessageHandler: GetRpcMessageHandler; @@ -150,7 +150,7 @@ export class PluginController extends BaseController< terminatePlugin, terminateAllPlugins, hasPermission, - startPlugin, + executePlugin, getRpcMessageHandler, messenger, state, @@ -189,7 +189,7 @@ export class PluginController extends BaseController< this._terminatePlugin = terminatePlugin; this._terminateAllPlugins = terminateAllPlugins; - this._startPlugin = startPlugin; + this._executePlugin = executePlugin; this._getRpcMessageHandler = getRpcMessageHandler; this._pluginsBeingAdded = new Map(); @@ -214,7 +214,7 @@ export class PluginController extends BaseController< console.log(`Starting: ${pluginName}`); try { - await this._executePlugin({ + await this._startPlugin({ pluginName, sourceCode, }); @@ -243,7 +243,7 @@ export class PluginController extends BaseController< } try { - await this._executePlugin({ + await this._startPlugin({ pluginName, sourceCode: plugin.sourceCode, }); @@ -507,7 +507,7 @@ export class PluginController extends BaseController< await this.authorize(pluginName); - await this._executePlugin({ + await this._startPlugin({ pluginName, sourceCode, }); @@ -550,8 +550,8 @@ export class PluginController extends BaseController< return this._pluginsBeingAdded.get(pluginName) as Promise; } - private async _executePlugin(pluginData: PluginData) { - const result = await this._startPlugin(pluginData); + private async _startPlugin(pluginData: PluginData) { + const result = await this._executePlugin(pluginData); this._setPluginToRunning(pluginData.pluginName); return result; } @@ -682,7 +682,7 @@ export class PluginController extends BaseController< * Test method. */ runInlinePlugin(inlinePluginName: keyof typeof INLINE_PLUGINS = 'IDLE') { - this._executePlugin({ + this._startPlugin({ pluginName: 'inlinePlugin', sourceCode: INLINE_PLUGINS[inlinePluginName], }); diff --git a/packages/controllers/src/services/ExecutionEnvironmentService.ts b/packages/controllers/src/services/ExecutionEnvironmentService.ts index 23afe67884..69444b868d 100644 --- a/packages/controllers/src/services/ExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/ExecutionEnvironmentService.ts @@ -15,7 +15,7 @@ export type TerminateAll = () => Promise; export type CreatePluginEnvironment = ( metadata: PluginMetadata, ) => Promise; -export type StartPlugin = (pluginData: PluginData) => Promise; +export type ExecutePlugin = (pluginData: PluginData) => Promise; export type GetRpcMessageHandler = ( pluginName: string, ) => Promise; @@ -23,6 +23,6 @@ export type GetRpcMessageHandler = ( export interface ExecutionEnvironmentService { terminatePlugin: TerminatePlugin; terminateAllPlugins: TerminateAll; - startPlugin: StartPlugin; + executePlugin: ExecutePlugin; getRpcMessageHandler: GetRpcMessageHandler; } diff --git a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.test.ts b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.test.ts index 6b5eccdbc9..b9d46143fa 100644 --- a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.test.ts +++ b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.test.ts @@ -28,7 +28,7 @@ describe('Worker Controller', () => { }, ); const pluginName = 'foo.bar.baz'; - const response = await webWorkerExecutionEnvironmentService.startPlugin({ + const response = await webWorkerExecutionEnvironmentService.executePlugin({ pluginName, sourceCode: ` console.log('foo'); diff --git a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts index fce8774318..b1aa6ef3c6 100644 --- a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts @@ -175,7 +175,7 @@ export class WebWorkerExecutionEnvironmentService this._pluginRpcHooks.set(pluginName, rpcHook); } - async startPlugin(pluginData: PluginData): Promise { + async executePlugin(pluginData: PluginData): Promise { const _workerId = await this._initWorker(); this._mapPluginAndWorker(pluginData.pluginName, _workerId); From b4fee6fac1f23598e7820eca818bb4f49411acb9 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 16 Jun 2021 09:22:59 -0700 Subject: [PATCH 10/11] Update packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com> --- .../src/services/WebWorkerExecutionEnvironmentService.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts index b1aa6ef3c6..2aac97c165 100644 --- a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts @@ -176,8 +176,11 @@ export class WebWorkerExecutionEnvironmentService } async executePlugin(pluginData: PluginData): Promise { - const _workerId = await this._initWorker(); + if (this.pluginToWorkerMap.has(pluginData.pluginName)) { + throw new Error(`Plugin "${pluginData.pluginName}" is already being executed.`); + } + const _workerId = await this._initWorker(); this._mapPluginAndWorker(pluginData.pluginName, _workerId); const result = await this._command(_workerId, { From b243cf283087ae6a8f71502bffcb84e53439362e Mon Sep 17 00:00:00 2001 From: shanejonas Date: Wed, 16 Jun 2021 09:33:18 -0700 Subject: [PATCH 11/11] fix linting --- .../src/services/WebWorkerExecutionEnvironmentService.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts index 2aac97c165..6a7a40c304 100644 --- a/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts +++ b/packages/controllers/src/services/WebWorkerExecutionEnvironmentService.ts @@ -177,7 +177,9 @@ export class WebWorkerExecutionEnvironmentService async executePlugin(pluginData: PluginData): Promise { if (this.pluginToWorkerMap.has(pluginData.pluginName)) { - throw new Error(`Plugin "${pluginData.pluginName}" is already being executed.`); + throw new Error( + `Plugin "${pluginData.pluginName}" is already being executed.`, + ); } const _workerId = await this._initWorker();