From 70b2068c37e38a12e1510908d1d0a04bce18b06a Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Tue, 16 Jul 2024 13:08:06 -0400 Subject: [PATCH 01/35] Added initial WIP --- .../multicainSubscriptionManager.test.ts | 45 +++++++++ .../multichainSubscriptionManager.ts | 93 +++++++++++++++++++ .../lib/multichain-api/provider-authorize.js | 17 +++- app/scripts/metamask-controller.js | 23 +++++ 4 files changed, 174 insertions(+), 4 deletions(-) create mode 100644 app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts create mode 100644 app/scripts/lib/multichain-api/multichainSubscriptionManager.ts diff --git a/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts b/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts new file mode 100644 index 000000000000..3cecd50e9720 --- /dev/null +++ b/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts @@ -0,0 +1,45 @@ +import MultichainSubscriptionManager from "./multichainSubscriptionManager"; + +jest.mock('@metamask/eth-json-rpc-filters/subscriptionManager') + +const newHeadsMock = { + "difficulty": "0x15d9223a23aa", + "extraData": "0xd983010305844765746887676f312e342e328777696e646f7773", + "gasLimit": "0x47e7c4", + "gasUsed": "0x38658", + "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "miner": "0xf8b483dba2c3b7176a3da549ad41a48bb3121069", + "nonce": "0x084149998194cc5f", + "number": "0x1348c9", + "parentHash": "0x7736fab79e05dc611604d22470dadad26f56fe494421b5b333de816ce1f25701", + "receiptRoot": "0x2fab35823ad00c7bb388595cb46652fe7886e00660a01e867824d3dceb1c8d36", + "sha3Uncles": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347", + "stateRoot": "0xb3346685172db67de536d8765c43c31009d0eb3bd9c501c9be3229203f15f378", + "timestamp": "0x56ffeff8", +} + +describe('multichainSubscriptionManager', () => { + + it('should subscribe to a chain', (done) => { + const chain = 'eip155:1'; + const mockFindNetworkClientIdByChainId = jest.fn().mockImplementation(() => newHeadsMock); + const subscriptionManager = new MultichainSubscriptionManager({ + findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, + }); + subscriptionManager.subscribe(chain) + subscriptionManager.on('notification', (notification: any) => { + expect(notification).toMatchObject({ + scope: chain, + request: { + method: 'eth_subscription', + params: { + result: newHeadsMock, + } + }, + subscription: "0xdeadbeef" + }); + done(); + }); + }); + +}); diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts new file mode 100644 index 000000000000..66b209b309c8 --- /dev/null +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -0,0 +1,93 @@ +import { NetworkControllerFindNetworkClientIdByChainIdAction } from "@metamask/network-controller"; +import SafeEventEmitter from "@metamask/safe-event-emitter"; +import { parseCaipChainId } from "@metamask/utils"; +import { Scope } from "./scope"; + +const createSubscriptionManager = require('@metamask/eth-json-rpc-filters/subscriptionManager'); + +type MultichainSubscriptionManagerOptions = { + findNetworkClientIdByChainId: NetworkControllerFindNetworkClientIdByChainIdAction['handler']; +}; + +export default class MultichainSubscriptionManager extends SafeEventEmitter { + private subscriptionsByChain: { [scope: string]: unknown }; + private findNetworkClientIdByChainId: NetworkControllerFindNetworkClientIdByChainIdAction['handler']; + private subscriptionManagerByChain: { [scope: string]: any }; + constructor(options: MultichainSubscriptionManagerOptions) { + super(); + this.findNetworkClientIdByChainId = options.findNetworkClientIdByChainId; + this.subscriptionManagerByChain = {}; + this.subscriptionsByChain = {}; + } + onNotification(scope: Scope, message: any) { + this.emit('notification', { + method: 'wallet_invokeMethod', + params: { + scope, + request: message, + } + }); + } + subscribe(scope: Scope) { + const subscriptionManager = createSubscriptionManager(this.findNetworkClientIdByChainId(parseCaipChainId(scope).reference)); + this.subscriptionManagerByChain[scope] = subscriptionManager; + this.subscriptionsByChain[scope] = (message: any) => + this.emit('notification', { + method: 'wallet_invokeMethod', + params: { + scope, + request: message, + } + }) + subscriptionManager.events.on('notification', this.subscriptionsByChain[scope]); + return subscriptionManager; + } + unsubscribe(scope: Scope) { + const subscriptionManager = this.subscriptionManagerByChain[scope]; + subscriptionManager.events.off('notification', this.subscriptionsByChain[scope]); + subscriptionManager.destroy() + delete this.subscriptionManagerByChain[scope]; + delete this.subscriptionsByChain[scope]; + } +} + + +export const createMultichainMiddlewareManager = () => { + const middlewaresByScope: Record = {}; + const removeMiddleware = (key: string) => { + const middleware = middlewaresByScope[key]; + middleware.destroy(); + delete middlewaresByScope[key]; + } + + const addMiddleware = (scope: Scope, middleware: any) => { + middlewaresByScope[scope] = middleware; + } + + return { + middleware: async (req: any, res: any, next: any) => { + const returnHandlers: any = []; + for (const key in middlewaresByScope) { + middlewaresByScope[key](req, res, (returnHandler: any) => { + if (returnHandler) { + returnHandlers.push(returnHandler); + } + }); + } + for (const handler of returnHandlers) { + await new Promise((resolve, reject) => { + handler((error: any) => (error ? reject(error) : resolve())); + }); + } + return { + destroy: () => { + for (const key in middlewaresByScope) { + removeMiddleware(key); + } + }, + } + }, + addMiddleware, + removeMiddleware, + } +} diff --git a/app/scripts/lib/multichain-api/provider-authorize.js b/app/scripts/lib/multichain-api/provider-authorize.js index 72ecbb789201..a83eaed19af7 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.js +++ b/app/scripts/lib/multichain-api/provider-authorize.js @@ -113,14 +113,23 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { }, }); + const mergedScopes = mergeScopes( + flattenedRequiredScopes, + flattenedOptionalScopes, + ); + + Object.entries(mergedScopes).forEach(([scope]) => { + const subscriptionManager = hooks.subscriptionManager.subscribe(scope); + hooks.multichainMiddlewareManager.addMiddleware( + subscriptionManager.middleware, + ); + }); + // TODO: metrics/tracking after approval res.result = { sessionId, - sessionScopes: mergeScopes( - flattenedRequiredScopes, - flattenedOptionalScopes, - ), + sessionScopes: mergedScopes, sessionProperties, }; return end(); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 2b786de2a88d..44717b8753b2 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -333,6 +333,7 @@ import { } from './lib/multichain-api/caip25permissions'; import { multichainMethodCallValidatorMiddleware } from './lib/multichain-api/multichainMethodCallValidator'; import { decodeTransactionData } from './lib/transaction/decode/util'; +import MultichainSubscriptionManager, { createMultichainMiddlewareManager } from './lib/multichain-api/multichainSubscriptionManager'; export const METAMASK_CONTROLLER_EVENTS = { // Fired after state changes that impact the extension badge (unapproved msg count) @@ -528,6 +529,16 @@ export default class MetamaskController extends EventEmitter { trackMetaMetricsEvent: (...args) => this.metaMetricsController.trackEvent(...args), }); + + this.multichainSubscriptionManager = new MultichainSubscriptionManager({ + findNetworkClientIdByChainId: + this.networkController.findNetworkClientIdByChainId.bind( + this.networkController, + ), + }); + + this.multichainMiddlewareManager = createMultichainMiddlewareManager(); + this.networkController.initializeProvider(); this.provider = this.networkController.getProviderAndBlockTracker().provider; @@ -4543,6 +4554,10 @@ export default class MetamaskController extends EventEmitter { config.type !== networkConfigurationId, ); + const scope = `eip155:${parseInt(chainId, 16)}`; + this.multichainSubscriptionManager.unsubscribe(scope); + this.multichainMiddlewareManager.removeMiddleware(scope); + // if this network configuration is only one for a given chainId // remove all permissions for that chainId if (!hasOtherConfigsForChainId) { @@ -5594,6 +5609,8 @@ export default class MetamaskController extends EventEmitter { createScaffoldMiddleware({ [MESSAGE_TYPE.PROVIDER_AUTHORIZE]: (request, response, next, end) => { return providerAuthorizeHandler(request, response, next, end, { + multichainMiddlewareManager: this.multichainMiddlewareManager, + subscriptionManager: this.multichainSubscriptionManager, grantPermissions: this.permissionController.grantPermissions.bind( this.permissionController, ), @@ -5787,6 +5804,12 @@ export default class MetamaskController extends EventEmitter { engine.push(this.metamaskMiddleware); + this.multichainSubscriptionManager.events.on('notification', (message) => + engine.emit('notification', message), + ); + + engine.push(this.multichainSubscriptionManager.middleware); + engine.push((req, res, _next, end) => { const { provider } = this.networkController.getNetworkClientById( req.networkClientId, From 9e8f0df1a62be52798ccc8f20a03fdc57f64396a Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Tue, 16 Jul 2024 16:13:36 -0400 Subject: [PATCH 02/35] pushing WIP WIP --- .../multichainSubscriptionManager.ts | 44 +++++++++---------- .../lib/multichain-api/provider-authorize.js | 6 +++ app/scripts/metamask-controller.js | 12 ++--- 3 files changed, 33 insertions(+), 29 deletions(-) diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index 66b209b309c8..03e5addd05a2 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -2,6 +2,7 @@ import { NetworkControllerFindNetworkClientIdByChainIdAction } from "@metamask/n import SafeEventEmitter from "@metamask/safe-event-emitter"; import { parseCaipChainId } from "@metamask/utils"; import { Scope } from "./scope"; +import { toHex } from "@metamask/controller-utils"; const createSubscriptionManager = require('@metamask/eth-json-rpc-filters/subscriptionManager'); @@ -29,7 +30,9 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { }); } subscribe(scope: Scope) { - const subscriptionManager = createSubscriptionManager(this.findNetworkClientIdByChainId(parseCaipChainId(scope).reference)); + const subscriptionManager = createSubscriptionManager(this.findNetworkClientIdByChainId( + toHex(parseCaipChainId(scope).reference)) + ); this.subscriptionManagerByChain[scope] = subscriptionManager; this.subscriptionsByChain[scope] = (message: any) => this.emit('notification', { @@ -49,9 +52,15 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { delete this.subscriptionManagerByChain[scope]; delete this.subscriptionsByChain[scope]; } -} + unsubscribeAll() { + Object.keys(this.subscriptionManagerByChain).forEach((scope) => { + this.unsubscribe(scope); + }); + } +} +// per scope middleware to handle legacy middleware export const createMultichainMiddlewareManager = () => { const middlewaresByScope: Record = {}; const removeMiddleware = (key: string) => { @@ -60,34 +69,23 @@ export const createMultichainMiddlewareManager = () => { delete middlewaresByScope[key]; } + const removeAllMiddleware = () => { + console.log('removing all middleware', middlewaresByScope); + Object.keys(middlewaresByScope).forEach((key) => { + removeMiddleware(key); + }); + } + const addMiddleware = (scope: Scope, middleware: any) => { middlewaresByScope[scope] = middleware; } return { - middleware: async (req: any, res: any, next: any) => { - const returnHandlers: any = []; - for (const key in middlewaresByScope) { - middlewaresByScope[key](req, res, (returnHandler: any) => { - if (returnHandler) { - returnHandlers.push(returnHandler); - } - }); - } - for (const handler of returnHandlers) { - await new Promise((resolve, reject) => { - handler((error: any) => (error ? reject(error) : resolve())); - }); - } - return { - destroy: () => { - for (const key in middlewaresByScope) { - removeMiddleware(key); - } - }, - } + middleware: async (req: any, res: any, next: any, end: any) => { + middlewaresByScope[req.scope](req, res, next, end); }, addMiddleware, removeMiddleware, + removeAllMiddleware, } } diff --git a/app/scripts/lib/multichain-api/provider-authorize.js b/app/scripts/lib/multichain-api/provider-authorize.js index a83eaed19af7..073941e460d1 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.js +++ b/app/scripts/lib/multichain-api/provider-authorize.js @@ -118,6 +118,12 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { flattenedOptionalScopes, ); + // clear per scope middleware + hooks.multichainMiddlewareManager.removeAllMiddleware(); + hooks.subscriptionManager.unsubscribeAll(); + + // if you added any notifications, like eth_subscribe + // then we have to get the subscriptionManager going per scope Object.entries(mergedScopes).forEach(([scope]) => { const subscriptionManager = hooks.subscriptionManager.subscribe(scope); hooks.multichainMiddlewareManager.addMiddleware( diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 44717b8753b2..47302705897d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5729,11 +5729,11 @@ export default class MetamaskController extends EventEmitter { }, ), // TODO remove this hook - requestPermissionsForOrigin: - this.permissionController.requestPermissions.bind( - this.permissionController, - { origin }, - ), + // requestPermissionsForOrigin: + // this.permissionController.requestPermissions.bind( + // this.permissionController, + // { origin }, + // ), getCaveat: ({ target, caveatType }) => { try { return this.permissionController.getCaveat( @@ -5804,7 +5804,7 @@ export default class MetamaskController extends EventEmitter { engine.push(this.metamaskMiddleware); - this.multichainSubscriptionManager.events.on('notification', (message) => + this.multichainSubscriptionManager.on('notification', (message) => engine.emit('notification', message), ); From e59638929e0272d3f9a7deab05240d8c91fa4661 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Tue, 16 Jul 2024 17:10:14 -0400 Subject: [PATCH 03/35] fix: get it almost there --- .../multicainSubscriptionManager.test.ts | 3 ++ .../multichainSubscriptionManager.ts | 31 ++++++++++++------- .../lib/multichain-api/provider-authorize.js | 1 + .../lib/multichain-api/provider-request.js | 1 + app/scripts/metamask-controller.js | 9 ++++-- 5 files changed, 31 insertions(+), 14 deletions(-) diff --git a/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts b/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts index 3cecd50e9720..2e61f5d537a7 100644 --- a/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts +++ b/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts @@ -23,10 +23,13 @@ describe('multichainSubscriptionManager', () => { it('should subscribe to a chain', (done) => { const chain = 'eip155:1'; const mockFindNetworkClientIdByChainId = jest.fn().mockImplementation(() => newHeadsMock); + const mockGetNetworkClientById = jest.fn().mockImplementation(() => newHeadsMock); const subscriptionManager = new MultichainSubscriptionManager({ findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, + getNetworkClientById: mockGetNetworkClientById, }); subscriptionManager.subscribe(chain) + // do something in between subscriptionManager.on('notification', (notification: any) => { expect(notification).toMatchObject({ scope: chain, diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index 03e5addd05a2..978d58f47745 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -1,22 +1,27 @@ -import { NetworkControllerFindNetworkClientIdByChainIdAction } from "@metamask/network-controller"; +import { NetworkControllerFindNetworkClientIdByChainIdAction, NetworkControllerGetNetworkClientByIdAction } from "@metamask/network-controller"; import SafeEventEmitter from "@metamask/safe-event-emitter"; import { parseCaipChainId } from "@metamask/utils"; import { Scope } from "./scope"; import { toHex } from "@metamask/controller-utils"; +import { createAsyncMiddleware } from "json-rpc-engine"; +import { SelectedNetworkControllerGetNetworkClientIdForDomainAction } from "@metamask/selected-network-controller"; const createSubscriptionManager = require('@metamask/eth-json-rpc-filters/subscriptionManager'); type MultichainSubscriptionManagerOptions = { findNetworkClientIdByChainId: NetworkControllerFindNetworkClientIdByChainIdAction['handler']; + getNetworkClientById: NetworkControllerGetNetworkClientByIdAction['handler']; }; export default class MultichainSubscriptionManager extends SafeEventEmitter { private subscriptionsByChain: { [scope: string]: unknown }; private findNetworkClientIdByChainId: NetworkControllerFindNetworkClientIdByChainIdAction['handler']; + private getNetworkClientById: NetworkControllerGetNetworkClientByIdAction['handler']; private subscriptionManagerByChain: { [scope: string]: any }; constructor(options: MultichainSubscriptionManagerOptions) { super(); this.findNetworkClientIdByChainId = options.findNetworkClientIdByChainId; + this.getNetworkClientById = options.getNetworkClientById; this.subscriptionManagerByChain = {}; this.subscriptionsByChain = {}; } @@ -30,9 +35,12 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { }); } subscribe(scope: Scope) { - const subscriptionManager = createSubscriptionManager(this.findNetworkClientIdByChainId( - toHex(parseCaipChainId(scope).reference)) - ); + const networkClientId = this.findNetworkClientIdByChainId(toHex(parseCaipChainId(scope).reference)); + const networkClient = this.getNetworkClientById(networkClientId); + const subscriptionManager = createSubscriptionManager({ + blockTracker: networkClient.blockTracker, + provider: networkClient.provider, + }); this.subscriptionManagerByChain[scope] = subscriptionManager; this.subscriptionsByChain[scope] = (message: any) => this.emit('notification', { @@ -63,16 +71,15 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { // per scope middleware to handle legacy middleware export const createMultichainMiddlewareManager = () => { const middlewaresByScope: Record = {}; - const removeMiddleware = (key: string) => { - const middleware = middlewaresByScope[key]; + const removeMiddleware = (scope: Scope) => { + const middleware = middlewaresByScope[scope]; middleware.destroy(); - delete middlewaresByScope[key]; + delete middlewaresByScope[scope]; } const removeAllMiddleware = () => { - console.log('removing all middleware', middlewaresByScope); - Object.keys(middlewaresByScope).forEach((key) => { - removeMiddleware(key); + Object.keys(middlewaresByScope).forEach((scope) => { + removeMiddleware(scope); }); } @@ -81,8 +88,8 @@ export const createMultichainMiddlewareManager = () => { } return { - middleware: async (req: any, res: any, next: any, end: any) => { - middlewaresByScope[req.scope](req, res, next, end); + middleware: (req: any, res: any, next: any, end: any) => { + return middlewaresByScope[req.scope](req, res, next, end); }, addMiddleware, removeMiddleware, diff --git a/app/scripts/lib/multichain-api/provider-authorize.js b/app/scripts/lib/multichain-api/provider-authorize.js index 073941e460d1..21e7530854c3 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.js +++ b/app/scripts/lib/multichain-api/provider-authorize.js @@ -127,6 +127,7 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { Object.entries(mergedScopes).forEach(([scope]) => { const subscriptionManager = hooks.subscriptionManager.subscribe(scope); hooks.multichainMiddlewareManager.addMiddleware( + scope, subscriptionManager.middleware, ); }); diff --git a/app/scripts/lib/multichain-api/provider-request.js b/app/scripts/lib/multichain-api/provider-request.js index 130ab15014ae..b061135b0edd 100644 --- a/app/scripts/lib/multichain-api/provider-request.js +++ b/app/scripts/lib/multichain-api/provider-request.js @@ -75,6 +75,7 @@ export async function providerRequestHandler( } Object.assign(request, { + scope, networkClientId, method: wrappedRequest.method, params: wrappedRequest.params, diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 47302705897d..30d551e7b68a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -333,7 +333,9 @@ import { } from './lib/multichain-api/caip25permissions'; import { multichainMethodCallValidatorMiddleware } from './lib/multichain-api/multichainMethodCallValidator'; import { decodeTransactionData } from './lib/transaction/decode/util'; -import MultichainSubscriptionManager, { createMultichainMiddlewareManager } from './lib/multichain-api/multichainSubscriptionManager'; +import MultichainSubscriptionManager, { + createMultichainMiddlewareManager, +} from './lib/multichain-api/multichainSubscriptionManager'; export const METAMASK_CONTROLLER_EVENTS = { // Fired after state changes that impact the extension badge (unapproved msg count) @@ -531,6 +533,9 @@ export default class MetamaskController extends EventEmitter { }); this.multichainSubscriptionManager = new MultichainSubscriptionManager({ + getNetworkClientById: this.networkController.getNetworkClientById.bind( + this.networkController, + ), findNetworkClientIdByChainId: this.networkController.findNetworkClientIdByChainId.bind( this.networkController, @@ -5808,7 +5813,7 @@ export default class MetamaskController extends EventEmitter { engine.emit('notification', message), ); - engine.push(this.multichainSubscriptionManager.middleware); + engine.push(this.multichainMiddlewareManager.middleware); engine.push((req, res, _next, end) => { const { provider } = this.networkController.getNetworkClientById( From 02ebac6df96ec68f783e8bc537743d1742f9f0e5 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Tue, 16 Jul 2024 14:34:12 -0700 Subject: [PATCH 04/35] lint --- .../multicainSubscriptionManager.test.ts | 53 +++++++++-------- .../multichainSubscriptionManager.ts | 59 +++++++++++-------- 2 files changed, 64 insertions(+), 48 deletions(-) diff --git a/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts b/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts index 2e61f5d537a7..020039b92586 100644 --- a/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts +++ b/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts @@ -1,34 +1,42 @@ -import MultichainSubscriptionManager from "./multichainSubscriptionManager"; +import MultichainSubscriptionManager from './multichainSubscriptionManager'; -jest.mock('@metamask/eth-json-rpc-filters/subscriptionManager') +jest.mock('@metamask/eth-json-rpc-filters/subscriptionManager'); const newHeadsMock = { - "difficulty": "0x15d9223a23aa", - "extraData": "0xd983010305844765746887676f312e342e328777696e646f7773", - "gasLimit": "0x47e7c4", - "gasUsed": "0x38658", - "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", - "miner": "0xf8b483dba2c3b7176a3da549ad41a48bb3121069", - "nonce": "0x084149998194cc5f", - "number": "0x1348c9", - "parentHash": "0x7736fab79e05dc611604d22470dadad26f56fe494421b5b333de816ce1f25701", - "receiptRoot": "0x2fab35823ad00c7bb388595cb46652fe7886e00660a01e867824d3dceb1c8d36", - "sha3Uncles": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347", - "stateRoot": "0xb3346685172db67de536d8765c43c31009d0eb3bd9c501c9be3229203f15f378", - "timestamp": "0x56ffeff8", -} + difficulty: '0x15d9223a23aa', + extraData: '0xd983010305844765746887676f312e342e328777696e646f7773', + gasLimit: '0x47e7c4', + gasUsed: '0x38658', + logsBloom: + '0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000', + miner: '0xf8b483dba2c3b7176a3da549ad41a48bb3121069', + nonce: '0x084149998194cc5f', + number: '0x1348c9', + parentHash: + '0x7736fab79e05dc611604d22470dadad26f56fe494421b5b333de816ce1f25701', + receiptRoot: + '0x2fab35823ad00c7bb388595cb46652fe7886e00660a01e867824d3dceb1c8d36', + sha3Uncles: + '0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347', + stateRoot: + '0xb3346685172db67de536d8765c43c31009d0eb3bd9c501c9be3229203f15f378', + timestamp: '0x56ffeff8', +}; describe('multichainSubscriptionManager', () => { - it('should subscribe to a chain', (done) => { const chain = 'eip155:1'; - const mockFindNetworkClientIdByChainId = jest.fn().mockImplementation(() => newHeadsMock); - const mockGetNetworkClientById = jest.fn().mockImplementation(() => newHeadsMock); + const mockFindNetworkClientIdByChainId = jest + .fn() + .mockImplementation(() => newHeadsMock); + const mockGetNetworkClientById = jest + .fn() + .mockImplementation(() => newHeadsMock); const subscriptionManager = new MultichainSubscriptionManager({ findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, getNetworkClientById: mockGetNetworkClientById, }); - subscriptionManager.subscribe(chain) + subscriptionManager.subscribe(chain); // do something in between subscriptionManager.on('notification', (notification: any) => { expect(notification).toMatchObject({ @@ -37,12 +45,11 @@ describe('multichainSubscriptionManager', () => { method: 'eth_subscription', params: { result: newHeadsMock, - } + }, }, - subscription: "0xdeadbeef" + subscription: '0xdeadbeef', }); done(); }); }); - }); diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index 978d58f47745..4cf8267ec631 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -1,10 +1,11 @@ -import { NetworkControllerFindNetworkClientIdByChainIdAction, NetworkControllerGetNetworkClientByIdAction } from "@metamask/network-controller"; -import SafeEventEmitter from "@metamask/safe-event-emitter"; -import { parseCaipChainId } from "@metamask/utils"; -import { Scope } from "./scope"; -import { toHex } from "@metamask/controller-utils"; -import { createAsyncMiddleware } from "json-rpc-engine"; -import { SelectedNetworkControllerGetNetworkClientIdForDomainAction } from "@metamask/selected-network-controller"; +import { + NetworkControllerFindNetworkClientIdByChainIdAction, + NetworkControllerGetNetworkClientByIdAction, +} from '@metamask/network-controller'; +import SafeEventEmitter from '@metamask/safe-event-emitter'; +import { parseCaipChainId } from '@metamask/utils'; +import { toHex } from '@metamask/controller-utils'; +import { Scope } from './scope'; const createSubscriptionManager = require('@metamask/eth-json-rpc-filters/subscriptionManager'); @@ -15,9 +16,13 @@ type MultichainSubscriptionManagerOptions = { export default class MultichainSubscriptionManager extends SafeEventEmitter { private subscriptionsByChain: { [scope: string]: unknown }; + private findNetworkClientIdByChainId: NetworkControllerFindNetworkClientIdByChainIdAction['handler']; + private getNetworkClientById: NetworkControllerGetNetworkClientByIdAction['handler']; + private subscriptionManagerByChain: { [scope: string]: any }; + constructor(options: MultichainSubscriptionManagerOptions) { super(); this.findNetworkClientIdByChainId = options.findNetworkClientIdByChainId; @@ -25,38 +30,42 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { this.subscriptionManagerByChain = {}; this.subscriptionsByChain = {}; } + onNotification(scope: Scope, message: any) { this.emit('notification', { method: 'wallet_invokeMethod', params: { scope, request: message, - } + }, }); } + subscribe(scope: Scope) { - const networkClientId = this.findNetworkClientIdByChainId(toHex(parseCaipChainId(scope).reference)); + const networkClientId = this.findNetworkClientIdByChainId( + toHex(parseCaipChainId(scope).reference), + ); const networkClient = this.getNetworkClientById(networkClientId); const subscriptionManager = createSubscriptionManager({ blockTracker: networkClient.blockTracker, provider: networkClient.provider, }); this.subscriptionManagerByChain[scope] = subscriptionManager; - this.subscriptionsByChain[scope] = (message: any) => - this.emit('notification', { - method: 'wallet_invokeMethod', - params: { - scope, - request: message, - } - }) - subscriptionManager.events.on('notification', this.subscriptionsByChain[scope]); + this.subscriptionsByChain[scope] = this.onNotification.bind(this, scope); + subscriptionManager.events.on( + 'notification', + this.subscriptionsByChain[scope], + ); return subscriptionManager; } + unsubscribe(scope: Scope) { const subscriptionManager = this.subscriptionManagerByChain[scope]; - subscriptionManager.events.off('notification', this.subscriptionsByChain[scope]); - subscriptionManager.destroy() + subscriptionManager.events.off( + 'notification', + this.subscriptionsByChain[scope], + ); + subscriptionManager.destroy(); delete this.subscriptionManagerByChain[scope]; delete this.subscriptionsByChain[scope]; } @@ -75,17 +84,17 @@ export const createMultichainMiddlewareManager = () => { const middleware = middlewaresByScope[scope]; middleware.destroy(); delete middlewaresByScope[scope]; - } + }; const removeAllMiddleware = () => { Object.keys(middlewaresByScope).forEach((scope) => { removeMiddleware(scope); }); - } + }; const addMiddleware = (scope: Scope, middleware: any) => { middlewaresByScope[scope] = middleware; - } + }; return { middleware: (req: any, res: any, next: any, end: any) => { @@ -94,5 +103,5 @@ export const createMultichainMiddlewareManager = () => { addMiddleware, removeMiddleware, removeAllMiddleware, - } -} + }; +}; From d093eab6c6245220c8ea092d746bb6122792a215 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Wed, 17 Jul 2024 09:58:58 -0400 Subject: [PATCH 05/35] fix: issue with destroy --- .../multichain-api/multichainSubscriptionManager.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index 4cf8267ec631..77314ac40d1f 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -61,11 +61,13 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { unsubscribe(scope: Scope) { const subscriptionManager = this.subscriptionManagerByChain[scope]; - subscriptionManager.events.off( - 'notification', - this.subscriptionsByChain[scope], - ); - subscriptionManager.destroy(); + if (subscriptionManager) { + subscriptionManager.events.off( + 'notification', + this.subscriptionsByChain[scope], + ); + subscriptionManager.destroy(); + } delete this.subscriptionManagerByChain[scope]; delete this.subscriptionsByChain[scope]; } From f5fb9c7648e42c91e4d7466c0a509092dff4cde9 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Thu, 18 Jul 2024 12:37:07 -0400 Subject: [PATCH 06/35] fix: add tests for multichain subscription manage and middleware manager --- .../multicainSubscriptionManager.test.ts | 98 +++++++++++-------- .../lib/multichain-api/provider-authorize.js | 27 +++-- .../multichain-api/provider-authorize.test.js | 11 +++ .../multichain-api/provider-request.test.js | 2 + app/scripts/metamask-controller.js | 2 +- 5 files changed, 90 insertions(+), 50 deletions(-) diff --git a/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts b/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts index 020039b92586..01926837fd6c 100644 --- a/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts +++ b/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts @@ -1,55 +1,75 @@ -import MultichainSubscriptionManager from './multichainSubscriptionManager'; +import { JsonRpcMiddleware } from 'json-rpc-engine'; +import MultichainSubscriptionManager, { createMultichainMiddlewareManager } from './multichainSubscriptionManager'; -jest.mock('@metamask/eth-json-rpc-filters/subscriptionManager'); - -const newHeadsMock = { - difficulty: '0x15d9223a23aa', - extraData: '0xd983010305844765746887676f312e342e328777696e646f7773', - gasLimit: '0x47e7c4', - gasUsed: '0x38658', - logsBloom: - '0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000', - miner: '0xf8b483dba2c3b7176a3da549ad41a48bb3121069', - nonce: '0x084149998194cc5f', - number: '0x1348c9', - parentHash: - '0x7736fab79e05dc611604d22470dadad26f56fe494421b5b333de816ce1f25701', - receiptRoot: - '0x2fab35823ad00c7bb388595cb46652fe7886e00660a01e867824d3dceb1c8d36', - sha3Uncles: - '0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347', - stateRoot: - '0xb3346685172db67de536d8765c43c31009d0eb3bd9c501c9be3229203f15f378', - timestamp: '0x56ffeff8', +const newHeadsNotificationMock = { + method: 'eth_subscription', + params: { + result: { + difficulty: '0x15d9223a23aa', + extraData: '0xd983010305844765746887676f312e342e328777696e646f7773', + gasLimit: '0x47e7c4', + gasUsed: '0x38658', + logsBloom: + '0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000', + miner: '0xf8b483dba2c3b7176a3da549ad41a48bb3121069', + nonce: '0x084149998194cc5f', + number: '0x1348c9', + parentHash: + '0x7736fab79e05dc611604d22470dadad26f56fe494421b5b333de816ce1f25701', + receiptRoot: + '0x2fab35823ad00c7bb388595cb46652fe7886e00660a01e867824d3dceb1c8d36', + sha3Uncles: + '0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347', + stateRoot: + '0xb3346685172db67de536d8765c43c31009d0eb3bd9c501c9be3229203f15f378', + timestamp: '0x56ffeff8', + }, + }, }; describe('multichainSubscriptionManager', () => { it('should subscribe to a chain', (done) => { - const chain = 'eip155:1'; - const mockFindNetworkClientIdByChainId = jest - .fn() - .mockImplementation(() => newHeadsMock); - const mockGetNetworkClientById = jest - .fn() - .mockImplementation(() => newHeadsMock); + const scope = 'eip155:1'; + const mockFindNetworkClientIdByChainId = jest.fn() + const mockGetNetworkClientById = jest.fn().mockImplementation(() => ({ + blockTracker: {}, + provider: {}, + })); const subscriptionManager = new MultichainSubscriptionManager({ findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, getNetworkClientById: mockGetNetworkClientById, }); - subscriptionManager.subscribe(chain); - // do something in between + subscriptionManager.subscribe(scope); subscriptionManager.on('notification', (notification: any) => { expect(notification).toMatchObject({ - scope: chain, - request: { - method: 'eth_subscription', - params: { - result: newHeadsMock, - }, - }, - subscription: '0xdeadbeef', + method: "wallet_invokeMethod", + params: { + scope, + request: newHeadsNotificationMock, + } }); done(); }); + subscriptionManager.onNotification(scope, newHeadsNotificationMock); }); }); +describe('multichainMiddlewareManager', () => { + it('should add middleware and get called for the scope', () => { + const multichainMiddlewareManager = createMultichainMiddlewareManager(); + const middlewareSpy = jest.fn(); + multichainMiddlewareManager.addMiddleware('eip155:1', middlewareSpy); + multichainMiddlewareManager.middleware({scope: 'eip155:1'}, {}, () => { }, () => {}); + expect(middlewareSpy).toHaveBeenCalled(); + }) + it('should remove middleware', () => { + const multichainMiddlewareManager = createMultichainMiddlewareManager(); + const middlewareMock = jest.fn(); + (middlewareMock as unknown as {destroy: () => void}).destroy = jest.fn(); + const scope = 'eip155:1'; + multichainMiddlewareManager.addMiddleware(scope, middlewareMock); + multichainMiddlewareManager.removeMiddleware(scope); + expect(() => { + multichainMiddlewareManager.middleware({scope}, {}, () => { }, () => {}); + }).toThrow(); + }) +}); diff --git a/app/scripts/lib/multichain-api/provider-authorize.js b/app/scripts/lib/multichain-api/provider-authorize.js index 21e7530854c3..0633bad320ba 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.js +++ b/app/scripts/lib/multichain-api/provider-authorize.js @@ -55,6 +55,7 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { ), ); } + console.log(' got to before sessionid'); const sessionId = '0xdeadbeef'; @@ -72,6 +73,7 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { [RestrictedMethods.eth_accounts]: {}, }, ); + console.log(' got to after requestPermissions'); const permittedAccounts = getAccountsFromPermission(subjectPermission); const { flattenedRequiredScopes, flattenedOptionalScopes } = processScopes( requiredScopes, @@ -120,16 +122,21 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { // clear per scope middleware hooks.multichainMiddlewareManager.removeAllMiddleware(); - hooks.subscriptionManager.unsubscribeAll(); - - // if you added any notifications, like eth_subscribe - // then we have to get the subscriptionManager going per scope - Object.entries(mergedScopes).forEach(([scope]) => { - const subscriptionManager = hooks.subscriptionManager.subscribe(scope); - hooks.multichainMiddlewareManager.addMiddleware( - scope, - subscriptionManager.middleware, - ); + hooks.multichainSubscriptionManager.unsubscribeAll(); + + // if the eth_subscription notification is in the scope and eth_subscribe is in the methods + // then get the subscriptionManager going for that scope + Object.entries(mergedScopes).forEach(([scope, scopeObject]) => { + if ( + scopeObject.notifications.includes('eth_subscription') && + scopeObject.methods.includes('eth_subscribe') + ) { + const subscriptionManager = hooks.subscriptionManager.subscribe(scope); + hooks.multichainMiddlewareManager.addMiddleware( + scope, + subscriptionManager.middleware, + ); + } }); // TODO: metrics/tracking after approval diff --git a/app/scripts/lib/multichain-api/provider-authorize.test.js b/app/scripts/lib/multichain-api/provider-authorize.test.js index 01f1ffb49312..a883b28753a4 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.test.js +++ b/app/scripts/lib/multichain-api/provider-authorize.test.js @@ -58,6 +58,16 @@ const createMockedHandler = () => { const response = {}; const handler = (request) => providerAuthorizeHandler(request, response, next, end, { + multichainMiddlewareManager: { + addMiddleware: jest.fn(), + removeMiddleware: jest.fn(), + removeAllMiddleware: jest.fn(), + }, + multichainSubscriptionManager: { + subscribe: jest.fn(), + unsubscribe: jest.fn(), + unsubscribeAll: jest.fn(), + }, findNetworkClientIdByChainId, requestPermissions, grantPermissions, @@ -295,6 +305,7 @@ describe('provider_authorize', () => { }, }); await handler(baseRequest); + console.log('RESPONSE ===', response); expect(response.result).toStrictEqual({ sessionId: '0xdeadbeef', diff --git a/app/scripts/lib/multichain-api/provider-request.test.js b/app/scripts/lib/multichain-api/provider-request.test.js index 20f629cd12ff..f5619e07848b 100644 --- a/app/scripts/lib/multichain-api/provider-request.test.js +++ b/app/scripts/lib/multichain-api/provider-request.test.js @@ -154,6 +154,7 @@ describe('provider_request', () => { await handler(request); expect(request).toStrictEqual({ + scope: 'eip155:1', origin: 'http://test.com', networkClientId: 'mainnet', method: 'eth_call', @@ -223,6 +224,7 @@ describe('provider_request', () => { }; await handler(walletRequest); expect(walletRequest).toStrictEqual({ + scope: 'wallet', origin: 'http://test.com', networkClientId: 'selectedNetworkClientId', method: 'wallet_watchAsset', diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index d8631a339192..5c9ba796cbb1 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5623,7 +5623,7 @@ export default class MetamaskController extends EventEmitter { [MESSAGE_TYPE.PROVIDER_AUTHORIZE]: (request, response, next, end) => { return providerAuthorizeHandler(request, response, next, end, { multichainMiddlewareManager: this.multichainMiddlewareManager, - subscriptionManager: this.multichainSubscriptionManager, + multichainSubscriptionManager: this.multichainSubscriptionManager, grantPermissions: this.permissionController.grantPermissions.bind( this.permissionController, ), From dc671f9943980b139a35da72363d9de20c29d072 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Tue, 23 Jul 2024 09:37:57 -0400 Subject: [PATCH 07/35] fix: wip --- .../multicainSubscriptionManager.test.ts | 19 ++- .../multichainSubscriptionManager.ts | 149 ++++++++++++++---- .../lib/multichain-api/provider-authorize.js | 11 +- .../multichain-api/provider-authorize.test.js | 4 + .../lib/multichain-api/scope/supported.ts | 4 +- app/scripts/metamask-controller.js | 14 +- 6 files changed, 149 insertions(+), 52 deletions(-) diff --git a/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts b/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts index 01926837fd6c..294ce5755a13 100644 --- a/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts +++ b/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts @@ -29,6 +29,7 @@ const newHeadsNotificationMock = { describe('multichainSubscriptionManager', () => { it('should subscribe to a chain', (done) => { + const domain = 'example.com'; const scope = 'eip155:1'; const mockFindNetworkClientIdByChainId = jest.fn() const mockGetNetworkClientById = jest.fn().mockImplementation(() => ({ @@ -39,8 +40,8 @@ describe('multichainSubscriptionManager', () => { findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, getNetworkClientById: mockGetNetworkClientById, }); - subscriptionManager.subscribe(scope); - subscriptionManager.on('notification', (notification: any) => { + subscriptionManager.subscribe(scope, domain); + subscriptionManager.on('notification', (domain: string, notification: any) => { expect(notification).toMatchObject({ method: "wallet_invokeMethod", params: { @@ -50,14 +51,15 @@ describe('multichainSubscriptionManager', () => { }); done(); }); - subscriptionManager.onNotification(scope, newHeadsNotificationMock); + subscriptionManager.onNotification(scope, domain, newHeadsNotificationMock); }); }); describe('multichainMiddlewareManager', () => { it('should add middleware and get called for the scope', () => { const multichainMiddlewareManager = createMultichainMiddlewareManager(); const middlewareSpy = jest.fn(); - multichainMiddlewareManager.addMiddleware('eip155:1', middlewareSpy); + const domain = 'example.com'; + multichainMiddlewareManager.addMiddleware('eip155:1', domain, middlewareSpy); multichainMiddlewareManager.middleware({scope: 'eip155:1'}, {}, () => { }, () => {}); expect(middlewareSpy).toHaveBeenCalled(); }) @@ -66,10 +68,11 @@ describe('multichainMiddlewareManager', () => { const middlewareMock = jest.fn(); (middlewareMock as unknown as {destroy: () => void}).destroy = jest.fn(); const scope = 'eip155:1'; - multichainMiddlewareManager.addMiddleware(scope, middlewareMock); + const domain = 'example.com'; + multichainMiddlewareManager.addMiddleware(scope, domain, middlewareMock); multichainMiddlewareManager.removeMiddleware(scope); - expect(() => { - multichainMiddlewareManager.middleware({scope}, {}, () => { }, () => {}); - }).toThrow(); + const nextSpy = jest.fn(); + multichainMiddlewareManager.middleware({scope}, {}, nextSpy, () => {}); + expect(nextSpy).toHaveBeenCalled(); }) }); diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index 77314ac40d1f..40ac7090c99e 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -6,6 +6,7 @@ import SafeEventEmitter from '@metamask/safe-event-emitter'; import { parseCaipChainId } from '@metamask/utils'; import { toHex } from '@metamask/controller-utils'; import { Scope } from './scope'; +import { JsonRpcMiddleware } from 'json-rpc-engine'; const createSubscriptionManager = require('@metamask/eth-json-rpc-filters/subscriptionManager'); @@ -15,13 +16,18 @@ type MultichainSubscriptionManagerOptions = { }; export default class MultichainSubscriptionManager extends SafeEventEmitter { - private subscriptionsByChain: { [scope: string]: unknown }; + private subscriptionsByChain: { + [scope: string]: { + [domain: string]: unknown; + }; + }; private findNetworkClientIdByChainId: NetworkControllerFindNetworkClientIdByChainIdAction['handler']; private getNetworkClientById: NetworkControllerGetNetworkClientByIdAction['handler']; private subscriptionManagerByChain: { [scope: string]: any }; + private subscriptionsCountByScope: { [scope: string]: number }; constructor(options: MultichainSubscriptionManagerOptions) { super(); @@ -29,10 +35,11 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { this.getNetworkClientById = options.getNetworkClientById; this.subscriptionManagerByChain = {}; this.subscriptionsByChain = {}; + this.subscriptionsCountByScope = {}; } - onNotification(scope: Scope, message: any) { - this.emit('notification', { + onNotification(scope: Scope, domain: string, message: any) { + this.emit('notification', domain, { method: 'wallet_invokeMethod', params: { scope, @@ -41,69 +48,147 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { }); } - subscribe(scope: Scope) { - const networkClientId = this.findNetworkClientIdByChainId( - toHex(parseCaipChainId(scope).reference), + subscribe(scope: Scope, domain: string) { + this.subscriptionsCountByScope[scope] = + this.subscriptionsCountByScope[scope] || 0; + this.subscriptionsCountByScope[scope]++; + let subscriptionManager; + if (this.subscriptionManagerByChain[scope]) { + subscriptionManager = this.subscriptionManagerByChain[scope]; + } else { + const networkClientId = this.findNetworkClientIdByChainId( + toHex(parseCaipChainId(scope).reference), + ); + const networkClient = this.getNetworkClientById(networkClientId); + subscriptionManager = createSubscriptionManager({ + blockTracker: networkClient.blockTracker, + provider: networkClient.provider, + }); + this.subscriptionManagerByChain[scope] = subscriptionManager; + } + this.subscriptionsByChain[scope] = this.subscriptionsByChain[scope] || {}; + this.subscriptionsByChain[scope][domain] = this.onNotification.bind( + this, + scope, + domain, ); - const networkClient = this.getNetworkClientById(networkClientId); - const subscriptionManager = createSubscriptionManager({ - blockTracker: networkClient.blockTracker, - provider: networkClient.provider, - }); - this.subscriptionManagerByChain[scope] = subscriptionManager; - this.subscriptionsByChain[scope] = this.onNotification.bind(this, scope); subscriptionManager.events.on( 'notification', - this.subscriptionsByChain[scope], + this.subscriptionsByChain[scope][domain], ); return subscriptionManager; } - unsubscribe(scope: Scope) { + unsubscribe(scope: Scope, domain: string) { const subscriptionManager = this.subscriptionManagerByChain[scope]; - if (subscriptionManager) { + if (subscriptionManager && this.subscriptionsByChain[scope][domain]) { subscriptionManager.events.off( 'notification', - this.subscriptionsByChain[scope], + this.subscriptionsByChain[scope][domain], ); - subscriptionManager.destroy(); + delete this.subscriptionsByChain[scope][domain]; + } + if (this.subscriptionsCountByScope[scope]) { + this.subscriptionsCountByScope[scope]--; + if (this.subscriptionsCountByScope[scope] === 0) { + subscriptionManager.destroy(); + delete this.subscriptionsCountByScope[scope]; + delete this.subscriptionManagerByChain[scope]; + delete this.subscriptionsByChain[scope]; + } } - delete this.subscriptionManagerByChain[scope]; - delete this.subscriptionsByChain[scope]; } unsubscribeAll() { - Object.keys(this.subscriptionManagerByChain).forEach((scope) => { - this.unsubscribe(scope); - }); + Object.entries(this.subscriptionsByChain).forEach( + ([scope, domainObject]) => { + Object.entries(domainObject).forEach(([domain]) => { + this.unsubscribe(scope, domain); + }); + }, + ); + } + unsubscribeScope(scope: string) { + Object.entries(this.subscriptionsByChain).forEach( + ([_scope, domainObject]) => { + if (scope === _scope) { + Object.entries(domainObject).forEach(([_domain]) => { + this.unsubscribe(_scope, _domain); + }); + } + }, + ); + } + + unsubscribeDomain(domain: string) { + Object.entries(this.subscriptionsByChain).forEach( + ([scope, domainObject]) => { + Object.entries(domainObject).forEach(([_domain]) => { + if (domain === _domain) { + this.unsubscribe(scope, _domain); + } + }); + }, + ); } } // per scope middleware to handle legacy middleware export const createMultichainMiddlewareManager = () => { const middlewaresByScope: Record = {}; - const removeMiddleware = (scope: Scope) => { - const middleware = middlewaresByScope[scope]; - middleware.destroy(); - delete middlewaresByScope[scope]; + const middlewareCountByDomainAndScope: Record> = {}; + const removeMiddleware = (scope: Scope, domain?: string) => { + middlewareCountByDomainAndScope[scope] = middlewareCountByDomainAndScope[scope] || {}; + if (domain) { + middlewareCountByDomainAndScope[scope][domain]--; + } + if (typeof domain === 'undefined' || middlewareCountByDomainAndScope[scope][domain] <= 0) { + const middleware = middlewaresByScope[scope]; + if (domain) { + delete middlewareCountByDomainAndScope[scope][domain]; + } + middleware.destroy(); + delete middlewaresByScope[scope]; + } + }; + + const removeAllMiddlewareForDomain = (domain: string) => { + for (const [scope, domains] of Object.entries(middlewareCountByDomainAndScope)) { + for (const [_domain] of Object.entries(domains)) { + if (_domain === domain) { + removeMiddleware(scope, domain); + } + } + } }; const removeAllMiddleware = () => { - Object.keys(middlewaresByScope).forEach((scope) => { - removeMiddleware(scope); - }); + for (const [scope, domainObject] of Object.entries(middlewareCountByDomainAndScope)) { + for (const domain of Object.keys(domainObject)) { + removeMiddleware(scope, domain); + } + } }; - const addMiddleware = (scope: Scope, middleware: any) => { - middlewaresByScope[scope] = middleware; + const addMiddleware = (scope: Scope, domain: string, middleware: JsonRpcMiddleware) => { + middlewareCountByDomainAndScope[scope] = middlewareCountByDomainAndScope[scope] || {}; + middlewareCountByDomainAndScope[scope][domain] = middlewareCountByDomainAndScope[scope][domain] || 0; + middlewareCountByDomainAndScope[scope][domain]++; + if (!middlewaresByScope[scope]) { + middlewaresByScope[scope] = middleware; + } }; return { middleware: (req: any, res: any, next: any, end: any) => { + if (!middlewaresByScope[req.scope]) { + return next(); + } return middlewaresByScope[req.scope](req, res, next, end); }, addMiddleware, removeMiddleware, removeAllMiddleware, + removeAllMiddlewareForDomain }; }; diff --git a/app/scripts/lib/multichain-api/provider-authorize.js b/app/scripts/lib/multichain-api/provider-authorize.js index 0633bad320ba..3a90820fcde5 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.js +++ b/app/scripts/lib/multichain-api/provider-authorize.js @@ -55,8 +55,6 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { ), ); } - console.log(' got to before sessionid'); - const sessionId = '0xdeadbeef'; if (sessionProperties && Object.keys(sessionProperties).length === 0) { @@ -120,9 +118,8 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { flattenedOptionalScopes, ); - // clear per scope middleware - hooks.multichainMiddlewareManager.removeAllMiddleware(); - hooks.multichainSubscriptionManager.unsubscribeAll(); + hooks.multichainMiddlewareManager.removeAllMiddlewareForDomain(origin); + hooks.multichainSubscriptionManager.unsubscribeDomain(origin); // if the eth_subscription notification is in the scope and eth_subscribe is in the methods // then get the subscriptionManager going for that scope @@ -131,9 +128,11 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { scopeObject.notifications.includes('eth_subscription') && scopeObject.methods.includes('eth_subscribe') ) { - const subscriptionManager = hooks.subscriptionManager.subscribe(scope); + const subscriptionManager = + hooks.multichainSubscriptionManager.subscribe(scope, origin); hooks.multichainMiddlewareManager.addMiddleware( scope, + origin, subscriptionManager.middleware, ); } diff --git a/app/scripts/lib/multichain-api/provider-authorize.test.js b/app/scripts/lib/multichain-api/provider-authorize.test.js index a883b28753a4..a89cbe58faa5 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.test.js +++ b/app/scripts/lib/multichain-api/provider-authorize.test.js @@ -9,6 +9,7 @@ import { Caip25CaveatType, Caip25EndowmentPermissionName, } from './caip25permissions'; +import { unsubscribe } from 'diagnostics_channel'; jest.mock('./scope', () => ({ ...jest.requireActual('./scope'), @@ -62,11 +63,14 @@ const createMockedHandler = () => { addMiddleware: jest.fn(), removeMiddleware: jest.fn(), removeAllMiddleware: jest.fn(), + removeAllMiddlewareForDomain: jest.fn(), }, multichainSubscriptionManager: { subscribe: jest.fn(), unsubscribe: jest.fn(), unsubscribeAll: jest.fn(), + unsubscribeDomain: jest.fn(), + unsubscribeScope: jest.fn(), }, findNetworkClientIdByChainId, requestPermissions, diff --git a/app/scripts/lib/multichain-api/scope/supported.ts b/app/scripts/lib/multichain-api/scope/supported.ts index 71af26ee98f6..a708c518f10e 100644 --- a/app/scripts/lib/multichain-api/scope/supported.ts +++ b/app/scripts/lib/multichain-api/scope/supported.ts @@ -80,5 +80,7 @@ export const isSupportedAccount = ( // TODO: Needs to go into a capabilties/routing controller // TODO: These make no sense in a multichain world. accountsChange becomes authorization/permissionChanged? export const isSupportedNotification = (notification: string): boolean => { - return ['accountsChanged', 'chainChanged'].includes(notification); + return ['accountsChanged', 'chainChanged', 'eth_subscription'].includes( + notification, + ); }; diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 5c9ba796cbb1..9dd1e01e9ff2 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -536,6 +536,7 @@ export default class MetamaskController extends EventEmitter { this.metaMetricsController.trackEvent(...args), }); + this.networkController.initializeProvider(); this.multichainSubscriptionManager = new MultichainSubscriptionManager({ getNetworkClientById: this.networkController.getNetworkClientById.bind( this.networkController, @@ -547,8 +548,6 @@ export default class MetamaskController extends EventEmitter { }); this.multichainMiddlewareManager = createMultichainMiddlewareManager(); - - this.networkController.initializeProvider(); this.provider = this.networkController.getProviderAndBlockTracker().provider; this.blockTracker = @@ -4568,7 +4567,7 @@ export default class MetamaskController extends EventEmitter { ); const scope = `eip155:${parseInt(chainId, 16)}`; - this.multichainSubscriptionManager.unsubscribe(scope); + this.multichainSubscriptionManager.unsubscribeScope(scope); this.multichainMiddlewareManager.removeMiddleware(scope); // if this network configuration is only one for a given chainId @@ -5817,8 +5816,13 @@ export default class MetamaskController extends EventEmitter { engine.push(this.metamaskMiddleware); - this.multichainSubscriptionManager.on('notification', (message) => - engine.emit('notification', message), + this.multichainSubscriptionManager.on( + 'notification', + (_origin, message) => { + if (origin === _origin) { + engine.emit('notification', message); + } + }, ); engine.push(this.multichainMiddlewareManager.middleware); From cd6f10e62211da4434ebbd9b913769d2c446f6bd Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Tue, 23 Jul 2024 14:23:12 -0400 Subject: [PATCH 08/35] fix: wip --- app/scripts/lib/multichain-api/multichainSubscriptionManager.ts | 2 ++ app/scripts/metamask-controller.js | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index 40ac7090c99e..d8d0b1e7fae7 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -138,6 +138,7 @@ export const createMultichainMiddlewareManager = () => { const middlewaresByScope: Record = {}; const middlewareCountByDomainAndScope: Record> = {}; const removeMiddleware = (scope: Scope, domain?: string) => { + console.log('removing middleware for scope', scope, 'domain', domain, 'middlewareCountByDomainAndScope', middlewareCountByDomainAndScope[scope]) middlewareCountByDomainAndScope[scope] = middlewareCountByDomainAndScope[scope] || {}; if (domain) { middlewareCountByDomainAndScope[scope][domain]--; @@ -181,6 +182,7 @@ export const createMultichainMiddlewareManager = () => { return { middleware: (req: any, res: any, next: any, end: any) => { + console.log('middleware', middlewaresByScope, middlewareCountByDomainAndScope); if (!middlewaresByScope[req.scope]) { return next(); } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 9dd1e01e9ff2..857a3135b1f7 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -6086,7 +6086,7 @@ export default class MetamaskController extends EventEmitter { */ _onStateUpdate(newState) { this.isClientOpenAndUnlocked = newState.isUnlocked && this._isClientOpen; - this._notifyChainChange(); + // this._notifyChainChange(); } // misc From c51e95f9ddf3855a327408bcc836599da7fff704 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Wed, 24 Jul 2024 16:13:45 -0400 Subject: [PATCH 09/35] fix: get destroy working and fix linting --- .../MultichainMiddlewareManager.test.ts | 45 +++++++++ .../MultichainMiddlewareManager.ts | 76 +++++++++++++++ ... => MultichainSubscriptionManager.test.ts} | 27 +----- .../multichainSubscriptionManager.ts | 97 ++++++------------- .../lib/multichain-api/provider-authorize.js | 1 + app/scripts/metamask-controller.js | 7 +- 6 files changed, 154 insertions(+), 99 deletions(-) create mode 100644 app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts create mode 100644 app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts rename app/scripts/lib/multichain-api/{multicainSubscriptionManager.test.ts => MultichainSubscriptionManager.test.ts} (63%) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts new file mode 100644 index 000000000000..ee90e02e4ad1 --- /dev/null +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts @@ -0,0 +1,45 @@ +import { JsonRpcRequest } from '@metamask/utils'; +import MultichainMiddlewareManager from './MultichainMiddlewareManager'; + +describe('MultichainMiddlewareManager', () => { + it('should add middleware and get called for the scope', () => { + const multichainMiddlewareManager = new MultichainMiddlewareManager(); + const middlewareSpy = jest.fn(); + const domain = 'example.com'; + multichainMiddlewareManager.addMiddleware( + 'eip155:1', + domain, + middlewareSpy, + ); + multichainMiddlewareManager.middleware( + { scope: 'eip155:1' } as unknown as JsonRpcRequest, + { jsonrpc: '2.0', id: 0 }, + () => { + // + }, + () => { + // + }, + ); + expect(middlewareSpy).toHaveBeenCalled(); + }); + it('should remove middleware', () => { + const multichainMiddlewareManager = new MultichainMiddlewareManager(); + const middlewareMock = jest.fn(); + (middlewareMock as unknown as { destroy: () => void }).destroy = jest.fn(); + const scope = 'eip155:1'; + const domain = 'example.com'; + multichainMiddlewareManager.addMiddleware(scope, domain, middlewareMock); + multichainMiddlewareManager.removeMiddleware(scope); + const nextSpy = jest.fn(); + multichainMiddlewareManager.middleware( + { scope } as unknown as JsonRpcRequest, + { jsonrpc: '2.0', id: 0 }, + nextSpy, + () => { + // + }, + ); + expect(nextSpy).toHaveBeenCalled(); + }); +}); diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts new file mode 100644 index 000000000000..44beecb19b1a --- /dev/null +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts @@ -0,0 +1,76 @@ +import { JsonRpcMiddleware } from 'json-rpc-engine'; +import { Scope } from './scope'; + +// per scope middleware to handle legacy middleware +export default class MultichainMiddlewareManager { + private middlewaresByScope: Record = {}; + + private middlewareCountByDomainAndScope: Record< + Scope, + Record + > = {}; + + removeMiddleware(scope: Scope, domain?: string) { + this.middlewareCountByDomainAndScope[scope] = + this.middlewareCountByDomainAndScope[scope] || {}; + if (domain) { + this.middlewareCountByDomainAndScope[scope][domain] -= 1; + } + if ( + typeof domain === 'undefined' || + this.middlewareCountByDomainAndScope[scope][domain] <= 0 + ) { + const middleware = this.middlewaresByScope[scope]; + if (domain) { + delete this.middlewareCountByDomainAndScope[scope][domain]; + } + middleware.destroy(); + delete this.middlewaresByScope[scope]; + } + } + + removeAllMiddlewareForDomain(domain: string) { + for (const [scope, domains] of Object.entries( + this.middlewareCountByDomainAndScope, + )) { + for (const [_domain] of Object.entries(domains)) { + if (_domain === domain) { + this.removeMiddleware(scope, domain); + } + } + } + } + + removeAllMiddleware() { + for (const [scope, domainObject] of Object.entries( + this.middlewareCountByDomainAndScope, + )) { + for (const domain of Object.keys(domainObject)) { + this.removeMiddleware(scope, domain); + } + } + } + + addMiddleware( + scope: Scope, + domain: string, + middleware: JsonRpcMiddleware, + ) { + this.middlewareCountByDomainAndScope[scope] = + this.middlewareCountByDomainAndScope[scope] || {}; + this.middlewareCountByDomainAndScope[scope][domain] = + this.middlewareCountByDomainAndScope[scope][domain] || 0; + this.middlewareCountByDomainAndScope[scope][domain] += 1; + if (!this.middlewaresByScope[scope]) { + this.middlewaresByScope[scope] = middleware; + } + } + + middleware: JsonRpcMiddleware = (req, res, next, end) => { + const r = req as unknown as { scope: string }; + if (!this.middlewaresByScope[r.scope]) { + return next(); + } + return this.middlewaresByScope[r.scope](req, res, next, end); + }; +} diff --git a/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts b/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts similarity index 63% rename from app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts rename to app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts index 294ce5755a13..6c750e2a37dd 100644 --- a/app/scripts/lib/multichain-api/multicainSubscriptionManager.test.ts +++ b/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts @@ -1,5 +1,4 @@ -import { JsonRpcMiddleware } from 'json-rpc-engine'; -import MultichainSubscriptionManager, { createMultichainMiddlewareManager } from './multichainSubscriptionManager'; +import MultichainSubscriptionManager from './MultichainSubscriptionManager'; const newHeadsNotificationMock = { method: 'eth_subscription', @@ -27,7 +26,7 @@ const newHeadsNotificationMock = { }, }; -describe('multichainSubscriptionManager', () => { +describe('MultichainSubscriptionManager', () => { it('should subscribe to a chain', (done) => { const domain = 'example.com'; const scope = 'eip155:1'; @@ -54,25 +53,3 @@ describe('multichainSubscriptionManager', () => { subscriptionManager.onNotification(scope, domain, newHeadsNotificationMock); }); }); -describe('multichainMiddlewareManager', () => { - it('should add middleware and get called for the scope', () => { - const multichainMiddlewareManager = createMultichainMiddlewareManager(); - const middlewareSpy = jest.fn(); - const domain = 'example.com'; - multichainMiddlewareManager.addMiddleware('eip155:1', domain, middlewareSpy); - multichainMiddlewareManager.middleware({scope: 'eip155:1'}, {}, () => { }, () => {}); - expect(middlewareSpy).toHaveBeenCalled(); - }) - it('should remove middleware', () => { - const multichainMiddlewareManager = createMultichainMiddlewareManager(); - const middlewareMock = jest.fn(); - (middlewareMock as unknown as {destroy: () => void}).destroy = jest.fn(); - const scope = 'eip155:1'; - const domain = 'example.com'; - multichainMiddlewareManager.addMiddleware(scope, domain, middlewareMock); - multichainMiddlewareManager.removeMiddleware(scope); - const nextSpy = jest.fn(); - multichainMiddlewareManager.middleware({scope}, {}, nextSpy, () => {}); - expect(nextSpy).toHaveBeenCalled(); - }) -}); diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index d8d0b1e7fae7..dbe29ae6d34c 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -6,8 +6,22 @@ import SafeEventEmitter from '@metamask/safe-event-emitter'; import { parseCaipChainId } from '@metamask/utils'; import { toHex } from '@metamask/controller-utils'; import { Scope } from './scope'; -import { JsonRpcMiddleware } from 'json-rpc-engine'; +type SubscriptionManager = { + events: { + on: ( + event: string, + listener: MultichainSubscriptionManager['onNotification'], + ) => void; + off: ( + event: string, + listener: MultichainSubscriptionManager['onNotification'], + ) => void; + }; + destroy?: () => void; +}; + +// eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires const createSubscriptionManager = require('@metamask/eth-json-rpc-filters/subscriptionManager'); type MultichainSubscriptionManagerOptions = { @@ -18,7 +32,7 @@ type MultichainSubscriptionManagerOptions = { export default class MultichainSubscriptionManager extends SafeEventEmitter { private subscriptionsByChain: { [scope: string]: { - [domain: string]: unknown; + [domain: string]: MultichainSubscriptionManager['onNotification']; }; }; @@ -26,7 +40,8 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { private getNetworkClientById: NetworkControllerGetNetworkClientByIdAction['handler']; - private subscriptionManagerByChain: { [scope: string]: any }; + private subscriptionManagerByChain: { [scope: string]: SubscriptionManager }; + private subscriptionsCountByScope: { [scope: string]: number }; constructor(options: MultichainSubscriptionManagerOptions) { @@ -38,7 +53,7 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { this.subscriptionsCountByScope = {}; } - onNotification(scope: Scope, domain: string, message: any) { + onNotification(scope: Scope, domain: string, message: unknown) { this.emit('notification', domain, { method: 'wallet_invokeMethod', params: { @@ -51,7 +66,7 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { subscribe(scope: Scope, domain: string) { this.subscriptionsCountByScope[scope] = this.subscriptionsCountByScope[scope] || 0; - this.subscriptionsCountByScope[scope]++; + this.subscriptionsCountByScope[scope] += 1; let subscriptionManager; if (this.subscriptionManagerByChain[scope]) { subscriptionManager = this.subscriptionManagerByChain[scope]; @@ -80,7 +95,7 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { } unsubscribe(scope: Scope, domain: string) { - const subscriptionManager = this.subscriptionManagerByChain[scope]; + const subscriptionManager: SubscriptionManager = this.subscriptionManagerByChain[scope]; if (subscriptionManager && this.subscriptionsByChain[scope][domain]) { subscriptionManager.events.off( 'notification', @@ -89,9 +104,12 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { delete this.subscriptionsByChain[scope][domain]; } if (this.subscriptionsCountByScope[scope]) { - this.subscriptionsCountByScope[scope]--; + this.subscriptionsCountByScope[scope] -= 1; if (this.subscriptionsCountByScope[scope] === 0) { - subscriptionManager.destroy(); + // might be destroyed already + if (subscriptionManager.destroy) { + subscriptionManager.destroy(); + } delete this.subscriptionsCountByScope[scope]; delete this.subscriptionManagerByChain[scope]; delete this.subscriptionsByChain[scope]; @@ -108,6 +126,7 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { }, ); } + unsubscribeScope(scope: string) { Object.entries(this.subscriptionsByChain).forEach( ([_scope, domainObject]) => { @@ -132,65 +151,3 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { ); } } - -// per scope middleware to handle legacy middleware -export const createMultichainMiddlewareManager = () => { - const middlewaresByScope: Record = {}; - const middlewareCountByDomainAndScope: Record> = {}; - const removeMiddleware = (scope: Scope, domain?: string) => { - console.log('removing middleware for scope', scope, 'domain', domain, 'middlewareCountByDomainAndScope', middlewareCountByDomainAndScope[scope]) - middlewareCountByDomainAndScope[scope] = middlewareCountByDomainAndScope[scope] || {}; - if (domain) { - middlewareCountByDomainAndScope[scope][domain]--; - } - if (typeof domain === 'undefined' || middlewareCountByDomainAndScope[scope][domain] <= 0) { - const middleware = middlewaresByScope[scope]; - if (domain) { - delete middlewareCountByDomainAndScope[scope][domain]; - } - middleware.destroy(); - delete middlewaresByScope[scope]; - } - }; - - const removeAllMiddlewareForDomain = (domain: string) => { - for (const [scope, domains] of Object.entries(middlewareCountByDomainAndScope)) { - for (const [_domain] of Object.entries(domains)) { - if (_domain === domain) { - removeMiddleware(scope, domain); - } - } - } - }; - - const removeAllMiddleware = () => { - for (const [scope, domainObject] of Object.entries(middlewareCountByDomainAndScope)) { - for (const domain of Object.keys(domainObject)) { - removeMiddleware(scope, domain); - } - } - }; - - const addMiddleware = (scope: Scope, domain: string, middleware: JsonRpcMiddleware) => { - middlewareCountByDomainAndScope[scope] = middlewareCountByDomainAndScope[scope] || {}; - middlewareCountByDomainAndScope[scope][domain] = middlewareCountByDomainAndScope[scope][domain] || 0; - middlewareCountByDomainAndScope[scope][domain]++; - if (!middlewaresByScope[scope]) { - middlewaresByScope[scope] = middleware; - } - }; - - return { - middleware: (req: any, res: any, next: any, end: any) => { - console.log('middleware', middlewaresByScope, middlewareCountByDomainAndScope); - if (!middlewaresByScope[req.scope]) { - return next(); - } - return middlewaresByScope[req.scope](req, res, next, end); - }, - addMiddleware, - removeMiddleware, - removeAllMiddleware, - removeAllMiddlewareForDomain - }; -}; diff --git a/app/scripts/lib/multichain-api/provider-authorize.js b/app/scripts/lib/multichain-api/provider-authorize.js index 3a90820fcde5..9042acc64956 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.js +++ b/app/scripts/lib/multichain-api/provider-authorize.js @@ -147,6 +147,7 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { }; return end(); } catch (err) { + console.log('ERROR in authorize', err); return end(err); } } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 857a3135b1f7..8c7752a49073 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -337,9 +337,8 @@ import { } from './lib/multichain-api/caip25permissions'; import { multichainMethodCallValidatorMiddleware } from './lib/multichain-api/multichainMethodCallValidator'; import { decodeTransactionData } from './lib/transaction/decode/util'; -import MultichainSubscriptionManager, { - createMultichainMiddlewareManager, -} from './lib/multichain-api/multichainSubscriptionManager'; +import MultichainSubscriptionManager from './lib/multichain-api/MultichainSubscriptionManager'; +import MultichainMiddlewareManager from './lib/multichain-api/MultichainMiddlewareManager'; export const METAMASK_CONTROLLER_EVENTS = { // Fired after state changes that impact the extension badge (unapproved msg count) @@ -547,7 +546,7 @@ export default class MetamaskController extends EventEmitter { ), }); - this.multichainMiddlewareManager = createMultichainMiddlewareManager(); + this.multichainMiddlewareManager = new MultichainMiddlewareManager(); this.provider = this.networkController.getProviderAndBlockTracker().provider; this.blockTracker = From 44e5f814ba4acca22a68dbf96399b0898c9f4921 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Wed, 24 Jul 2024 16:20:41 -0400 Subject: [PATCH 10/35] fix: remove console logs --- app/scripts/lib/multichain-api/provider-authorize.js | 2 -- app/scripts/lib/multichain-api/provider-authorize.test.js | 1 - 2 files changed, 3 deletions(-) diff --git a/app/scripts/lib/multichain-api/provider-authorize.js b/app/scripts/lib/multichain-api/provider-authorize.js index 9042acc64956..d729386b0020 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.js +++ b/app/scripts/lib/multichain-api/provider-authorize.js @@ -71,7 +71,6 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { [RestrictedMethods.eth_accounts]: {}, }, ); - console.log(' got to after requestPermissions'); const permittedAccounts = getAccountsFromPermission(subjectPermission); const { flattenedRequiredScopes, flattenedOptionalScopes } = processScopes( requiredScopes, @@ -147,7 +146,6 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { }; return end(); } catch (err) { - console.log('ERROR in authorize', err); return end(err); } } diff --git a/app/scripts/lib/multichain-api/provider-authorize.test.js b/app/scripts/lib/multichain-api/provider-authorize.test.js index a89cbe58faa5..23393a68f770 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.test.js +++ b/app/scripts/lib/multichain-api/provider-authorize.test.js @@ -309,7 +309,6 @@ describe('provider_authorize', () => { }, }); await handler(baseRequest); - console.log('RESPONSE ===', response); expect(response.result).toStrictEqual({ sessionId: '0xdeadbeef', From 52003d2bdf44f3d13c67848d7a03f8d31fb2d2b3 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Wed, 24 Jul 2024 16:24:57 -0400 Subject: [PATCH 11/35] fix: remove hook --- app/scripts/metamask-controller.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 71af7b925134..3e36c4265b88 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5879,12 +5879,6 @@ export default class MetamaskController extends EventEmitter { }, }, ), - // TODO remove this hook - // requestPermissionsForOrigin: - // this.permissionController.requestPermissions.bind( - // this.permissionController, - // { origin }, - // ), getCaveat: ({ target, caveatType }) => { try { return this.permissionController.getCaveat( From 12c15e2bf6da12b062aff30c5f42ddbc35b6aff3 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Mon, 29 Jul 2024 15:16:45 -0400 Subject: [PATCH 12/35] fix: add nullish coalescing operating instead of || --- .../lib/multichain-api/MultichainMiddlewareManager.ts | 5 ++--- .../lib/multichain-api/multichainSubscriptionManager.ts | 7 +++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts index 44beecb19b1a..782a97a38f9d 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts @@ -3,7 +3,7 @@ import { Scope } from './scope'; // per scope middleware to handle legacy middleware export default class MultichainMiddlewareManager { - private middlewaresByScope: Record = {}; + private middlewaresByScope: Record void }> = {}; private middlewareCountByDomainAndScope: Record< Scope, @@ -11,8 +11,7 @@ export default class MultichainMiddlewareManager { > = {}; removeMiddleware(scope: Scope, domain?: string) { - this.middlewareCountByDomainAndScope[scope] = - this.middlewareCountByDomainAndScope[scope] || {}; + this.middlewareCountByDomainAndScope[scope] ??= {}; if (domain) { this.middlewareCountByDomainAndScope[scope][domain] -= 1; } diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index dbe29ae6d34c..8c6ae92e2948 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -7,7 +7,7 @@ import { parseCaipChainId } from '@metamask/utils'; import { toHex } from '@metamask/controller-utils'; import { Scope } from './scope'; -type SubscriptionManager = { +export type SubscriptionManager = { events: { on: ( event: string, @@ -64,9 +64,6 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { } subscribe(scope: Scope, domain: string) { - this.subscriptionsCountByScope[scope] = - this.subscriptionsCountByScope[scope] || 0; - this.subscriptionsCountByScope[scope] += 1; let subscriptionManager; if (this.subscriptionManagerByChain[scope]) { subscriptionManager = this.subscriptionManagerByChain[scope]; @@ -91,6 +88,8 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { 'notification', this.subscriptionsByChain[scope][domain], ); + this.subscriptionsCountByScope[scope] ??= 0; + this.subscriptionsCountByScope[scope] += 1; return subscriptionManager; } From adf042c5f3d9d5052284950ee71a9efe917e1131 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Mon, 29 Jul 2024 15:19:55 -0400 Subject: [PATCH 13/35] fix: NetworkController types --- .../lib/multichain-api/multichainSubscriptionManager.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index 8c6ae92e2948..b1c54c320b0d 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -1,4 +1,5 @@ import { + NetworkController, NetworkControllerFindNetworkClientIdByChainIdAction, NetworkControllerGetNetworkClientByIdAction, } from '@metamask/network-controller'; @@ -25,8 +26,8 @@ export type SubscriptionManager = { const createSubscriptionManager = require('@metamask/eth-json-rpc-filters/subscriptionManager'); type MultichainSubscriptionManagerOptions = { - findNetworkClientIdByChainId: NetworkControllerFindNetworkClientIdByChainIdAction['handler']; - getNetworkClientById: NetworkControllerGetNetworkClientByIdAction['handler']; + findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId'] + getNetworkClientById: NetworkController['getNetworkClientById']; }; export default class MultichainSubscriptionManager extends SafeEventEmitter { @@ -36,9 +37,9 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { }; }; - private findNetworkClientIdByChainId: NetworkControllerFindNetworkClientIdByChainIdAction['handler']; + private findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId']; - private getNetworkClientById: NetworkControllerGetNetworkClientByIdAction['handler']; + private getNetworkClientById: NetworkController['getNetworkClientById']; private subscriptionManagerByChain: { [scope: string]: SubscriptionManager }; From c098ef4d61b0eadba9860f6409b2b3c411c860cc Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Mon, 29 Jul 2024 15:21:15 -0400 Subject: [PATCH 14/35] fix: remove unused import --- app/scripts/lib/multichain-api/provider-authorize.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/scripts/lib/multichain-api/provider-authorize.test.js b/app/scripts/lib/multichain-api/provider-authorize.test.js index 23393a68f770..407798eda54a 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.test.js +++ b/app/scripts/lib/multichain-api/provider-authorize.test.js @@ -9,7 +9,6 @@ import { Caip25CaveatType, Caip25EndowmentPermissionName, } from './caip25permissions'; -import { unsubscribe } from 'diagnostics_channel'; jest.mock('./scope', () => ({ ...jest.requireActual('./scope'), From 2f39c907c402cb5f38349204132f49895af9ffaa Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Tue, 30 Jul 2024 11:57:24 -0400 Subject: [PATCH 15/35] fix: typings for onNotification + middleware destroy --- .../MultichainMiddlewareManager.ts | 20 +++++++++++-- .../multichainSubscriptionManager.ts | 29 ++++++------------- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts index 782a97a38f9d..d655d5c0d4f9 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts @@ -1,9 +1,19 @@ import { JsonRpcMiddleware } from 'json-rpc-engine'; import { Scope } from './scope'; +// Extend JsonRpcMiddleware to include the destroy method +type ExtendedJsonRpcMiddleware = { + destroy: () => void; +} & JsonRpcMiddleware; + +type MiddlewareByScope = Record< + Scope, + ExtendedJsonRpcMiddleware | { destroy: () => void } +>; + // per scope middleware to handle legacy middleware export default class MultichainMiddlewareManager { - private middlewaresByScope: Record void }> = {}; + private middlewaresByScope: MiddlewareByScope = {}; private middlewareCountByDomainAndScope: Record< Scope, @@ -53,7 +63,7 @@ export default class MultichainMiddlewareManager { addMiddleware( scope: Scope, domain: string, - middleware: JsonRpcMiddleware, + middleware: ExtendedJsonRpcMiddleware, ) { this.middlewareCountByDomainAndScope[scope] = this.middlewareCountByDomainAndScope[scope] || {}; @@ -67,9 +77,13 @@ export default class MultichainMiddlewareManager { middleware: JsonRpcMiddleware = (req, res, next, end) => { const r = req as unknown as { scope: string }; + const middlewareFn = this.middlewaresByScope[r.scope] as JsonRpcMiddleware< + unknown, + unknown + >; if (!this.middlewaresByScope[r.scope]) { return next(); } - return this.middlewaresByScope[r.scope](req, res, next, end); + return middlewareFn(req, res, next, end); }; } diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index b1c54c320b0d..2c856d625451 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -1,8 +1,4 @@ -import { - NetworkController, - NetworkControllerFindNetworkClientIdByChainIdAction, - NetworkControllerGetNetworkClientByIdAction, -} from '@metamask/network-controller'; +import { NetworkController } from '@metamask/network-controller'; import SafeEventEmitter from '@metamask/safe-event-emitter'; import { parseCaipChainId } from '@metamask/utils'; import { toHex } from '@metamask/controller-utils'; @@ -10,14 +6,8 @@ import { Scope } from './scope'; export type SubscriptionManager = { events: { - on: ( - event: string, - listener: MultichainSubscriptionManager['onNotification'], - ) => void; - off: ( - event: string, - listener: MultichainSubscriptionManager['onNotification'], - ) => void; + on: (event: string, listener: (message: unknown) => void) => void; + off: (event: string, listener: (message: unknown) => void) => void; }; destroy?: () => void; }; @@ -33,7 +23,7 @@ type MultichainSubscriptionManagerOptions = { export default class MultichainSubscriptionManager extends SafeEventEmitter { private subscriptionsByChain: { [scope: string]: { - [domain: string]: MultichainSubscriptionManager['onNotification']; + [domain: string]: (message: unknown) => void; }; }; @@ -80,11 +70,9 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { this.subscriptionManagerByChain[scope] = subscriptionManager; } this.subscriptionsByChain[scope] = this.subscriptionsByChain[scope] || {}; - this.subscriptionsByChain[scope][domain] = this.onNotification.bind( - this, - scope, - domain, - ); + this.subscriptionsByChain[scope][domain] = (message) => { + this.onNotification(scope, domain, message); + }; subscriptionManager.events.on( 'notification', this.subscriptionsByChain[scope][domain], @@ -95,7 +83,8 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { } unsubscribe(scope: Scope, domain: string) { - const subscriptionManager: SubscriptionManager = this.subscriptionManagerByChain[scope]; + const subscriptionManager: SubscriptionManager = + this.subscriptionManagerByChain[scope]; if (subscriptionManager && this.subscriptionsByChain[scope][domain]) { subscriptionManager.events.off( 'notification', From c547db3065d2c38a104a4a75e99d7192f92fc9ee Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Tue, 30 Jul 2024 12:46:02 -0400 Subject: [PATCH 16/35] fix: linting --- app/scripts/lib/multichain-api/multichainSubscriptionManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index 2c856d625451..934694bada38 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -16,7 +16,7 @@ export type SubscriptionManager = { const createSubscriptionManager = require('@metamask/eth-json-rpc-filters/subscriptionManager'); type MultichainSubscriptionManagerOptions = { - findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId'] + findNetworkClientIdByChainId: NetworkController['findNetworkClientIdByChainId']; getNetworkClientById: NetworkController['getNetworkClientById']; }; From a5f73ec14b14baff8c49e2f9a5c8ff95e0d7f20d Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Tue, 30 Jul 2024 13:20:50 -0400 Subject: [PATCH 17/35] fix: add notifications in stateChange instead of provider authorize --- .../controllers/permissions/selectors.js | 32 ++++++++++++++++ .../lib/multichain-api/provider-authorize.js | 38 +++++++++---------- app/scripts/metamask-controller.js | 37 ++++++++++++++++++ 3 files changed, 88 insertions(+), 19 deletions(-) diff --git a/app/scripts/controllers/permissions/selectors.js b/app/scripts/controllers/permissions/selectors.js index 86a2d9b61d32..f5a653eb904c 100644 --- a/app/scripts/controllers/permissions/selectors.js +++ b/app/scripts/controllers/permissions/selectors.js @@ -164,3 +164,35 @@ export const getChangedAuthorizations = ( } return changedAuthorizations; }; + +/** + * + * @param {Map} newAuthorizationsMap - The new origin:authorization map. + * @param {Map} [previousAuthorizationsMap] - The previous origin:authorization map. + * @returns {Map} The origin:authorization map of changed authorizations. + */ +export const getRemovedAuthorizations = ( + newAuthorizationsMap, + previousAuthorizationsMap, +) => { + // If there are no previous authorizations, there are no removed authorizations. + if (previousAuthorizationsMap === undefined) { + return new Map(); + } + + const removedAuthorizations = new Map(); + if (newAuthorizationsMap === previousAuthorizationsMap) { + return removedAuthorizations; + } + + const previousOrigins = new Set([...previousAuthorizationsMap.keys()]); + for (const origin of newAuthorizationsMap.keys()) { + previousOrigins.delete(origin); + } + + for (const origin of previousOrigins.keys()) { + removedAuthorizations.set(origin, previousAuthorizationsMap.get(origin)); + } + + return removedAuthorizations; +}; diff --git a/app/scripts/lib/multichain-api/provider-authorize.js b/app/scripts/lib/multichain-api/provider-authorize.js index d729386b0020..0a3ab256cfd6 100644 --- a/app/scripts/lib/multichain-api/provider-authorize.js +++ b/app/scripts/lib/multichain-api/provider-authorize.js @@ -117,25 +117,25 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { flattenedOptionalScopes, ); - hooks.multichainMiddlewareManager.removeAllMiddlewareForDomain(origin); - hooks.multichainSubscriptionManager.unsubscribeDomain(origin); - - // if the eth_subscription notification is in the scope and eth_subscribe is in the methods - // then get the subscriptionManager going for that scope - Object.entries(mergedScopes).forEach(([scope, scopeObject]) => { - if ( - scopeObject.notifications.includes('eth_subscription') && - scopeObject.methods.includes('eth_subscribe') - ) { - const subscriptionManager = - hooks.multichainSubscriptionManager.subscribe(scope, origin); - hooks.multichainMiddlewareManager.addMiddleware( - scope, - origin, - subscriptionManager.middleware, - ); - } - }); + // hooks.multichainMiddlewareManager.removeAllMiddlewareForDomain(origin); + // hooks.multichainSubscriptionManager.unsubscribeDomain(origin); + + // // if the eth_subscription notification is in the scope and eth_subscribe is in the methods + // // then get the subscriptionManager going for that scope + // Object.entries(mergedScopes).forEach(([scope, scopeObject]) => { + // if ( + // scopeObject.notifications.includes('eth_subscription') && + // scopeObject.methods.includes('eth_subscribe') + // ) { + // const subscriptionManager = + // hooks.multichainSubscriptionManager.subscribe(scope, origin); + // hooks.multichainMiddlewareManager.addMiddleware( + // scope, + // origin, + // subscriptionManager.middleware, + // ); + // } + // }); // TODO: metrics/tracking after approval diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 3e36c4265b88..2f525ac39abc 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -305,6 +305,7 @@ import { getPermissionBackgroundApiMethods, getPermissionSpecifications, getPermittedAccountsByOrigin, + getRemovedAuthorizations, NOTIFICATION_NAMES, PermissionNames, unrestrictedMethods, @@ -2720,7 +2721,43 @@ export default class MetamaskController extends EventEmitter { previousValue, ); + const removedAuthorizations = getRemovedAuthorizations( + currentValue, + previousValue, + ); + + // remove any existing notification subscriptions for removed authorizations + for (const [origin] of removedAuthorizations.entries()) { + this.multichainMiddlewareManager.removeAllMiddlewareForDomain(origin); + this.multichainSubscriptionManager.unsubscribeDomain(origin); + } + + // add new notification subscriptions for changed authorizations for (const [origin, authorization] of changedAuthorizations.entries()) { + this.multichainMiddlewareManager.removeAllMiddlewareForDomain(origin); + this.multichainSubscriptionManager.unsubscribeDomain(origin); + const mergedScopes = mergeScopes( + authorization.requiredScopes, + authorization.optionalScopes, + ); + + // if the eth_subscription notification is in the scope and eth_subscribe is in the methods + // then get the subscriptionManager going for that scope + Object.entries(mergedScopes).forEach(([scope, scopeObject]) => { + if ( + scopeObject.notifications.includes('eth_subscription') && + scopeObject.methods.includes('eth_subscribe') + ) { + const subscriptionManager = + this.multichainSubscriptionManager.subscribe(scope, origin); + this.multichainMiddlewareManager.addMiddleware( + scope, + origin, + subscriptionManager.middleware, + ); + } + }); + this._notifyAuthorizationChange(origin, authorization); } }, From 757a205d63f2a092e1e24726f930d4a3972801f6 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Tue, 30 Jul 2024 12:48:59 -0700 Subject: [PATCH 18/35] Fix --- app/scripts/controllers/permissions/specifications.js | 4 +--- app/scripts/metamask-controller.js | 6 ------ 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/app/scripts/controllers/permissions/specifications.js b/app/scripts/controllers/permissions/specifications.js index 45018d23ee2c..2e4e761a7c3a 100644 --- a/app/scripts/controllers/permissions/specifications.js +++ b/app/scripts/controllers/permissions/specifications.js @@ -99,8 +99,6 @@ export const getCaveatSpecifications = ({ }; }; -const caip25Spec = caip25EndowmentBuilder; - /** * Gets the specifications for all permissions that will be recognized by the * PermissionController. @@ -125,7 +123,7 @@ export const getPermissionSpecifications = ({ findNetworkClientIdByChainId, }) => { return { - [caip25Spec.targetName]: caip25Spec.specificationBuilder({ + [caip25EndowmentBuilder.targetName]: caip25EndowmentBuilder.specificationBuilder({ findNetworkClientIdByChainId, getInternalAccounts, }), diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index e55c4b998ac3..72287abc32c1 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5543,8 +5543,6 @@ export default class MetamaskController extends EventEmitter { // They must nevertheless be placed _behind_ the permission middleware. engine.push( createEip1193MethodMiddleware({ - origin, - subjectType, // Miscellaneous @@ -6034,10 +6032,6 @@ export default class MetamaskController extends EventEmitter { this.alertController.setWeb3ShimUsageRecorded.bind( this.alertController, ), - getNetworkConfigurationByNetworkClientId: - this.networkController.getNetworkConfigurationByNetworkClientId.bind( - this.networkController, - ), }), ); From 2e63b0f0154aa58421418c25ca965fac60f52c1b Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Tue, 30 Jul 2024 16:53:20 -0400 Subject: [PATCH 19/35] fix: stateChange logic --- app/scripts/metamask-controller.js | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 72287abc32c1..79959ffdc0d8 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -2753,15 +2753,26 @@ export default class MetamaskController extends EventEmitter { ); // remove any existing notification subscriptions for removed authorizations - for (const [origin] of removedAuthorizations.entries()) { - this.multichainMiddlewareManager.removeAllMiddlewareForDomain(origin); - this.multichainSubscriptionManager.unsubscribeDomain(origin); + for (const [origin, authorization] of removedAuthorizations.entries()) { + const mergedScopes = mergeScopes( + authorization.requiredScopes, + authorization.optionalScopes, + ); + // if the eth_subscription notification is in the scope and eth_subscribe is in the methods + // then remove middleware and unsubscribe + Object.entries(mergedScopes).forEach(([scope, scopeObject]) => { + if ( + scopeObject.notifications.includes('eth_subscription') && + scopeObject.methods.includes('eth_subscribe') + ) { + this.multichainMiddlewareManager.removeMiddleware(scope, origin); + this.multichainSubscriptionManager.unsubscribe(scope, origin); + } + }); } // add new notification subscriptions for changed authorizations for (const [origin, authorization] of changedAuthorizations.entries()) { - this.multichainMiddlewareManager.removeAllMiddlewareForDomain(origin); - this.multichainSubscriptionManager.unsubscribeDomain(origin); const mergedScopes = mergeScopes( authorization.requiredScopes, authorization.optionalScopes, @@ -2774,6 +2785,8 @@ export default class MetamaskController extends EventEmitter { scopeObject.notifications.includes('eth_subscription') && scopeObject.methods.includes('eth_subscribe') ) { + this.multichainMiddlewareManager.removeMiddleware(scope, origin); + this.multichainSubscriptionManager.unsubscribe(scope, origin); const subscriptionManager = this.multichainSubscriptionManager.subscribe(scope, origin); this.multichainMiddlewareManager.addMiddleware( From 820abb9a38dc8712813d0388e44a584b9feaa24a Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Tue, 30 Jul 2024 16:54:04 -0400 Subject: [PATCH 20/35] fix: remove unused code --- .../provider-authorize/handler.js | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/app/scripts/lib/multichain-api/provider-authorize/handler.js b/app/scripts/lib/multichain-api/provider-authorize/handler.js index 29f2f19d5de4..c0d608fd4ca2 100644 --- a/app/scripts/lib/multichain-api/provider-authorize/handler.js +++ b/app/scripts/lib/multichain-api/provider-authorize/handler.js @@ -196,26 +196,6 @@ export async function providerAuthorizeHandler(req, res, _next, end, hooks) { }, }); - // hooks.multichainMiddlewareManager.removeAllMiddlewareForDomain(origin); - // hooks.multichainSubscriptionManager.unsubscribeDomain(origin); - - // // if the eth_subscription notification is in the scope and eth_subscribe is in the methods - // // then get the subscriptionManager going for that scope - // Object.entries(sessionScopes).forEach(([scope, scopeObject]) => { - // if ( - // scopeObject.notifications.includes('eth_subscription') && - // scopeObject.methods.includes('eth_subscribe') - // ) { - // const subscriptionManager = - // hooks.multichainSubscriptionManager.subscribe(scope, origin); - // hooks.multichainMiddlewareManager.addMiddleware( - // scope, - // origin, - // subscriptionManager.middleware, - // ); - // } - // }); - // TODO: metrics/tracking after approval res.result = { From 581fe0c7fd1900bfb8cedce95ef4495bd40d37e4 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Thu, 1 Aug 2024 16:33:06 -0400 Subject: [PATCH 21/35] fix: add more tests for MultichainMiddlewareManager and subscriptionManager --- .../MultichainMiddlewareManager.test.ts | 47 +++++-- .../MultichainMiddlewareManager.ts | 126 ++++++++++-------- .../MultichainSubscriptionManager.test.ts | 107 +++++++++++++-- .../multichainSubscriptionManager.ts | 8 +- app/scripts/metamask-controller.js | 1 + 5 files changed, 203 insertions(+), 86 deletions(-) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts index ee90e02e4ad1..5651d207b873 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts @@ -1,17 +1,21 @@ import { JsonRpcRequest } from '@metamask/utils'; -import MultichainMiddlewareManager from './MultichainMiddlewareManager'; +// eslint-disable-next-line import/no-named-as-default +import createMultichainMiddlewareManager, { + ExtendedJsonRpcMiddleware, +} from './MultichainMiddlewareManager'; describe('MultichainMiddlewareManager', () => { it('should add middleware and get called for the scope', () => { - const multichainMiddlewareManager = new MultichainMiddlewareManager(); - const middlewareSpy = jest.fn(); + const multichainMiddlewareManager = createMultichainMiddlewareManager(); + const middlewareSpy = jest.fn() as unknown as ExtendedJsonRpcMiddleware; + (middlewareSpy as { destroy: () => void }).destroy = jest.fn(); const domain = 'example.com'; multichainMiddlewareManager.addMiddleware( 'eip155:1', domain, middlewareSpy, ); - multichainMiddlewareManager.middleware( + (multichainMiddlewareManager as unknown as any).middleware( { scope: 'eip155:1' } as unknown as JsonRpcRequest, { jsonrpc: '2.0', id: 0 }, () => { @@ -24,22 +28,43 @@ describe('MultichainMiddlewareManager', () => { expect(middlewareSpy).toHaveBeenCalled(); }); it('should remove middleware', () => { - const multichainMiddlewareManager = new MultichainMiddlewareManager(); - const middlewareMock = jest.fn(); - (middlewareMock as unknown as { destroy: () => void }).destroy = jest.fn(); + const multichainMiddlewareManager = createMultichainMiddlewareManager(); + const middlewareMock = jest.fn() as unknown as ExtendedJsonRpcMiddleware; + middlewareMock.destroy = jest.fn(); + const scope = 'eip155:1'; + const domain = 'example.com'; + multichainMiddlewareManager.addMiddleware(scope, domain, middlewareMock); + multichainMiddlewareManager.removeMiddleware(scope, domain); + const endSpy = jest.fn(); + multichainMiddlewareManager.middleware( + { scope } as unknown as JsonRpcRequest, + { jsonrpc: '2.0', id: 0 }, + () => { + // + }, + endSpy, + ); + expect(endSpy).not.toHaveBeenCalled(); + }); + it('should remove all middleware', () => { + const multichainMiddlewareManager = createMultichainMiddlewareManager(); + const middlewareMock = jest.fn() as unknown as ExtendedJsonRpcMiddleware; + middlewareMock.destroy = jest.fn(); const scope = 'eip155:1'; + const scope2 = 'eip155:2'; const domain = 'example.com'; multichainMiddlewareManager.addMiddleware(scope, domain, middlewareMock); - multichainMiddlewareManager.removeMiddleware(scope); - const nextSpy = jest.fn(); + multichainMiddlewareManager.addMiddleware(scope2, domain, middlewareMock); + multichainMiddlewareManager.removeAllMiddleware(); + const endSpy = jest.fn(); multichainMiddlewareManager.middleware( { scope } as unknown as JsonRpcRequest, { jsonrpc: '2.0', id: 0 }, - nextSpy, () => { // }, + endSpy, ); - expect(nextSpy).toHaveBeenCalled(); + expect(endSpy).not.toHaveBeenCalled(); }); }); diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts index d655d5c0d4f9..a955daa6fc6d 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts @@ -2,88 +2,98 @@ import { JsonRpcMiddleware } from 'json-rpc-engine'; import { Scope } from './scope'; // Extend JsonRpcMiddleware to include the destroy method -type ExtendedJsonRpcMiddleware = { - destroy: () => void; -} & JsonRpcMiddleware; +export type ExtendedJsonRpcMiddleware = + | { + destroy: () => void; + } + | JsonRpcMiddleware; type MiddlewareByScope = Record< Scope, ExtendedJsonRpcMiddleware | { destroy: () => void } >; -// per scope middleware to handle legacy middleware -export default class MultichainMiddlewareManager { - private middlewaresByScope: MiddlewareByScope = {}; - - private middlewareCountByDomainAndScope: Record< - Scope, - Record - > = {}; +type MiddlewareManager = { + removeMiddleware: (scope: Scope, domain: string) => void; + removeAllMiddleware: () => void; + addMiddleware: ( + scope: Scope, + domain: string, + middleware: ExtendedJsonRpcMiddleware, + ) => void; + middleware: ExtendedJsonRpcMiddleware | { destroy: () => void }; + destroy: () => void; +}; - removeMiddleware(scope: Scope, domain?: string) { - this.middlewareCountByDomainAndScope[scope] ??= {}; - if (domain) { - this.middlewareCountByDomainAndScope[scope][domain] -= 1; - } - if ( - typeof domain === 'undefined' || - this.middlewareCountByDomainAndScope[scope][domain] <= 0 - ) { - const middleware = this.middlewaresByScope[scope]; - if (domain) { - delete this.middlewareCountByDomainAndScope[scope][domain]; - } - middleware.destroy(); - delete this.middlewaresByScope[scope]; - } - } +export default function createMultichainMiddlewareManager(): MiddlewareManager { + const middlewareCountByDomainAndScope: { + [scope: string]: { [domain: string]: number }; + } = {}; - removeAllMiddlewareForDomain(domain: string) { - for (const [scope, domains] of Object.entries( - this.middlewareCountByDomainAndScope, - )) { - for (const [_domain] of Object.entries(domains)) { - if (_domain === domain) { - this.removeMiddleware(scope, domain); - } - } - } - } + const middlewaresByScope: MiddlewareByScope = {}; - removeAllMiddleware() { + function removeAllMiddleware() { for (const [scope, domainObject] of Object.entries( - this.middlewareCountByDomainAndScope, + middlewareCountByDomainAndScope, )) { for (const domain of Object.keys(domainObject)) { - this.removeMiddleware(scope, domain); + removeMiddleware(scope, domain); } } } - addMiddleware( + function addMiddleware( scope: Scope, domain: string, middleware: ExtendedJsonRpcMiddleware, ) { - this.middlewareCountByDomainAndScope[scope] = - this.middlewareCountByDomainAndScope[scope] || {}; - this.middlewareCountByDomainAndScope[scope][domain] = - this.middlewareCountByDomainAndScope[scope][domain] || 0; - this.middlewareCountByDomainAndScope[scope][domain] += 1; - if (!this.middlewaresByScope[scope]) { - this.middlewaresByScope[scope] = middleware; + middlewareCountByDomainAndScope[scope] = + middlewareCountByDomainAndScope[scope] || {}; + middlewareCountByDomainAndScope[scope][domain] = + middlewareCountByDomainAndScope[scope][domain] || 0; + middlewareCountByDomainAndScope[scope][domain] += 1; + if (!middlewaresByScope[scope]) { + middlewaresByScope[scope] = middleware; + } + } + + function removeMiddleware(scope: Scope, domain: string) { + if (middlewareCountByDomainAndScope[scope]?.[domain]) { + middlewareCountByDomainAndScope[scope][domain] -= 1; + if (middlewareCountByDomainAndScope[scope][domain] === 0) { + delete middlewareCountByDomainAndScope[scope][domain]; + } + if (Object.keys(middlewareCountByDomainAndScope[scope]).length === 0) { + delete middlewareCountByDomainAndScope[scope]; + delete middlewaresByScope[scope]; + } } } - middleware: JsonRpcMiddleware = (req, res, next, end) => { + const middleware: ExtendedJsonRpcMiddleware | { destroy: () => void } = ( + req, + res, + next, + end, + ) => { const r = req as unknown as { scope: string }; - const middlewareFn = this.middlewaresByScope[r.scope] as JsonRpcMiddleware< - unknown, - unknown - >; - if (!this.middlewaresByScope[r.scope]) { - return next(); + const { scope } = r; + if (typeof middlewaresByScope[scope] === 'function') { + middlewaresByScope[scope](req, res, next, end); + } else { + next(); } - return middlewareFn(req, res, next, end); + }; + + function destroy() { + removeAllMiddleware(); + } + + return { + removeAllMiddleware, + addMiddleware, + removeMiddleware, + middleware, + destroy, }; } diff --git a/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts b/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts index eccf2b310dfc..637970d7dd26 100644 --- a/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts +++ b/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts @@ -27,7 +27,7 @@ const newHeadsNotificationMock = { }; describe('MultichainSubscriptionManager', () => { - it('should subscribe to a chain', (done) => { + it('should subscribe to a domain and scope', () => { const domain = 'example.com'; const scope = 'eip155:1'; const mockFindNetworkClientIdByChainId = jest.fn(); @@ -39,20 +39,103 @@ describe('MultichainSubscriptionManager', () => { findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, getNetworkClientById: mockGetNetworkClientById, }); + const spy = jest.fn(); + + subscriptionManager.on('notification', spy); subscriptionManager.subscribe(scope, domain); - subscriptionManager.on( + subscriptionManager.subscriptionManagerByChain[scope].events.emit( 'notification', - (domain: string, notification: any) => { - expect(notification).toMatchObject({ - method: 'wallet_invokeMethod', - params: { - scope, - request: newHeadsNotificationMock, - }, - }); - done(); + newHeadsNotificationMock, + ); + expect(spy).toHaveBeenCalledWith(domain, { + method: 'wallet_invokeMethod', + params: { + scope, + request: newHeadsNotificationMock, }, + }); + }); + it('should unsubscribe from a domain and scope', () => { + const domain = 'example.com'; + const scope = 'eip155:1'; + const mockFindNetworkClientIdByChainId = jest.fn(); + const mockGetNetworkClientById = jest.fn().mockImplementation(() => ({ + blockTracker: {}, + provider: {}, + })); + const subscriptionManager = new MultichainSubscriptionManager({ + findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, + getNetworkClientById: mockGetNetworkClientById, + }); + const spy = jest.fn(); + subscriptionManager.on('notification', spy); + subscriptionManager.subscribe(scope, domain); + const scopeSubscriptionManager = + subscriptionManager.subscriptionManagerByChain[scope]; + subscriptionManager.unsubscribe(scope, domain); + scopeSubscriptionManager.events.emit( + 'notification', + newHeadsNotificationMock, + ); + + expect(spy).not.toHaveBeenCalled(); + }); + it('should unsubscribe from a scope', () => { + const domain = 'example.com'; + const scope = 'eip155:1'; + const mockFindNetworkClientIdByChainId = jest.fn(); + const mockGetNetworkClientById = jest.fn().mockImplementation(() => ({ + blockTracker: {}, + provider: {}, + })); + const subscriptionManager = new MultichainSubscriptionManager({ + findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, + getNetworkClientById: mockGetNetworkClientById, + }); + const spy = jest.fn(); + subscriptionManager.on('notification', spy); + subscriptionManager.subscribe(scope, domain); + const scopeSubscriptionManager = + subscriptionManager.subscriptionManagerByChain[scope]; + subscriptionManager.unsubscribeScope(scope); + scopeSubscriptionManager.events.emit( + 'notification', + newHeadsNotificationMock, + ); + + expect(spy).not.toHaveBeenCalled(); + }); + it('should unsubscribe all', () => { + const domain = 'example.com'; + const scope = 'eip155:1'; + const mockFindNetworkClientIdByChainId = jest.fn(); + const mockGetNetworkClientById = jest.fn().mockImplementation(() => ({ + blockTracker: {}, + provider: {}, + })); + const subscriptionManager = new MultichainSubscriptionManager({ + findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, + getNetworkClientById: mockGetNetworkClientById, + }); + const spy = jest.fn(); + subscriptionManager.on('notification', spy); + subscriptionManager.subscribe(scope, domain); + const scope2 = 'eip155:2'; + subscriptionManager.subscribe(scope2, domain); + const scopeSubscriptionManager = + subscriptionManager.subscriptionManagerByChain[scope]; + const scopeSubscriptionManager2 = + subscriptionManager.subscriptionManagerByChain[scope2]; + subscriptionManager.unsubscribeAll(); + scopeSubscriptionManager.events.emit( + 'notification', + newHeadsNotificationMock, ); - subscriptionManager.onNotification(scope, domain, newHeadsNotificationMock); + scopeSubscriptionManager2.events.emit( + 'notification', + newHeadsNotificationMock, + ); + + expect(spy).not.toHaveBeenCalled(); }); }); diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts index 934694bada38..e9fb72c29fe6 100644 --- a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts +++ b/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts @@ -1,3 +1,4 @@ +import EventEmitter from 'events'; import { NetworkController } from '@metamask/network-controller'; import SafeEventEmitter from '@metamask/safe-event-emitter'; import { parseCaipChainId } from '@metamask/utils'; @@ -5,10 +6,7 @@ import { toHex } from '@metamask/controller-utils'; import { Scope } from './scope'; export type SubscriptionManager = { - events: { - on: (event: string, listener: (message: unknown) => void) => void; - off: (event: string, listener: (message: unknown) => void) => void; - }; + events: EventEmitter; destroy?: () => void; }; @@ -31,7 +29,7 @@ export default class MultichainSubscriptionManager extends SafeEventEmitter { private getNetworkClientById: NetworkController['getNetworkClientById']; - private subscriptionManagerByChain: { [scope: string]: SubscriptionManager }; + public subscriptionManagerByChain: { [scope: string]: SubscriptionManager }; private subscriptionsCountByScope: { [scope: string]: number }; diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 79959ffdc0d8..43fe1cac6829 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -6054,6 +6054,7 @@ export default class MetamaskController extends EventEmitter { 'notification', (_origin, message) => { if (origin === _origin) { + console.log('notification', message, _origin); engine.emit('notification', message); } }, From a51fb95124f8cec7124fa3b335c370f96ef4d7c7 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Thu, 1 Aug 2024 16:35:02 -0400 Subject: [PATCH 22/35] fix: remove console log --- app/scripts/metamask-controller.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 43fe1cac6829..79959ffdc0d8 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -6054,7 +6054,6 @@ export default class MetamaskController extends EventEmitter { 'notification', (_origin, message) => { if (origin === _origin) { - console.log('notification', message, _origin); engine.emit('notification', message); } }, From 87c00172d92273dc1678784cf2adf5f5e8ec2529 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Thu, 1 Aug 2024 16:46:50 -0400 Subject: [PATCH 23/35] fix: destroy --- .../lib/multichain-api/MultichainMiddlewareManager.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts index a955daa6fc6d..0b7da6999ccc 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts @@ -22,7 +22,6 @@ type MiddlewareManager = { middleware: ExtendedJsonRpcMiddleware, ) => void; middleware: ExtendedJsonRpcMiddleware | { destroy: () => void }; - destroy: () => void; }; export default function createMultichainMiddlewareManager(): MiddlewareManager { @@ -85,15 +84,14 @@ export default function createMultichainMiddlewareManager(): MiddlewareManager { } }; - function destroy() { + (middleware as any).destroy = () => { removeAllMiddleware(); - } + }; return { removeAllMiddleware, addMiddleware, removeMiddleware, middleware, - destroy, }; } From b5cd7d9bb93673112d5551712f549673d5d091e5 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 1 Aug 2024 16:26:25 -0500 Subject: [PATCH 24/35] use intersection type instead of union type for adding destroy to middleware JsonRpcMiddleware type --- .../lib/multichain-api/MultichainMiddlewareManager.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts index 0b7da6999ccc..6d2e66d5648e 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts @@ -2,11 +2,7 @@ import { JsonRpcMiddleware } from 'json-rpc-engine'; import { Scope } from './scope'; // Extend JsonRpcMiddleware to include the destroy method -export type ExtendedJsonRpcMiddleware = - | { - destroy: () => void; - } - | JsonRpcMiddleware; +export type ExtendedJsonRpcMiddleware = JsonRpcMiddleware & { destroy?: () => void }; type MiddlewareByScope = Record< Scope, @@ -21,7 +17,7 @@ type MiddlewareManager = { domain: string, middleware: ExtendedJsonRpcMiddleware, ) => void; - middleware: ExtendedJsonRpcMiddleware | { destroy: () => void }; + middleware: ExtendedJsonRpcMiddleware; }; export default function createMultichainMiddlewareManager(): MiddlewareManager { @@ -69,7 +65,7 @@ export default function createMultichainMiddlewareManager(): MiddlewareManager { } } - const middleware: ExtendedJsonRpcMiddleware | { destroy: () => void } = ( + const middleware: ExtendedJsonRpcMiddleware = ( req, res, next, From 1a88f4e7bee0e4bf9885170339f3cedd8bb2975a Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Fri, 2 Aug 2024 09:03:49 -0400 Subject: [PATCH 25/35] fix: type for middleware --- .../multichain-api/MultichainMiddlewareManager.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts index 6d2e66d5648e..503e7a0dfb4c 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts @@ -2,7 +2,9 @@ import { JsonRpcMiddleware } from 'json-rpc-engine'; import { Scope } from './scope'; // Extend JsonRpcMiddleware to include the destroy method -export type ExtendedJsonRpcMiddleware = JsonRpcMiddleware & { destroy?: () => void }; +export type ExtendedJsonRpcMiddleware = JsonRpcMiddleware & { + destroy?: () => void; +}; type MiddlewareByScope = Record< Scope, @@ -65,12 +67,7 @@ export default function createMultichainMiddlewareManager(): MiddlewareManager { } } - const middleware: ExtendedJsonRpcMiddleware = ( - req, - res, - next, - end, - ) => { + const middleware: ExtendedJsonRpcMiddleware = (req, res, next, end) => { const r = req as unknown as { scope: string }; const { scope } = r; if (typeof middlewaresByScope[scope] === 'function') { @@ -80,7 +77,7 @@ export default function createMultichainMiddlewareManager(): MiddlewareManager { } }; - (middleware as any).destroy = () => { + middleware.destroy = () => { removeAllMiddleware(); }; From 5ceb06122cdba148ec07ee8d41b3246f0f9ecbdb Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Fri, 2 Aug 2024 09:05:37 -0400 Subject: [PATCH 26/35] fix: more typings --- .../lib/multichain-api/MultichainMiddlewareManager.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts index 5651d207b873..717daeb0e45b 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts @@ -15,7 +15,7 @@ describe('MultichainMiddlewareManager', () => { domain, middlewareSpy, ); - (multichainMiddlewareManager as unknown as any).middleware( + multichainMiddlewareManager.middleware( { scope: 'eip155:1' } as unknown as JsonRpcRequest, { jsonrpc: '2.0', id: 0 }, () => { From 0aa3d008d8ea9feed94b7036749d4af2f3b527b8 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Fri, 2 Aug 2024 10:22:34 -0400 Subject: [PATCH 27/35] fix: linting --- app/scripts/controllers/permissions/specifications.js | 9 +++++---- .../multichain-api/provider-authorize/handler.test.js | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/scripts/controllers/permissions/specifications.js b/app/scripts/controllers/permissions/specifications.js index 2e4e761a7c3a..22d479a8cb02 100644 --- a/app/scripts/controllers/permissions/specifications.js +++ b/app/scripts/controllers/permissions/specifications.js @@ -123,10 +123,11 @@ export const getPermissionSpecifications = ({ findNetworkClientIdByChainId, }) => { return { - [caip25EndowmentBuilder.targetName]: caip25EndowmentBuilder.specificationBuilder({ - findNetworkClientIdByChainId, - getInternalAccounts, - }), + [caip25EndowmentBuilder.targetName]: + caip25EndowmentBuilder.specificationBuilder({ + findNetworkClientIdByChainId, + getInternalAccounts, + }), [PermissionNames.eth_accounts]: { permissionType: PermissionType.RestrictedMethod, targetName: PermissionNames.eth_accounts, diff --git a/app/scripts/lib/multichain-api/provider-authorize/handler.test.js b/app/scripts/lib/multichain-api/provider-authorize/handler.test.js index 68a7b655a566..db54950408ff 100644 --- a/app/scripts/lib/multichain-api/provider-authorize/handler.test.js +++ b/app/scripts/lib/multichain-api/provider-authorize/handler.test.js @@ -94,7 +94,7 @@ const createMockedHandler = () => { upsertNetworkConfiguration, removeNetworkConfiguration, multichainMiddlewareManager, - multichainSubscriptionManager + multichainSubscriptionManager, }); return { From 3df485d7fa747c96c77ca96d9ffbad1ea7e7fac5 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Fri, 2 Aug 2024 10:43:48 -0400 Subject: [PATCH 28/35] fix: remove unneeded typing --- .../lib/multichain-api/MultichainMiddlewareManager.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts index 503e7a0dfb4c..a3ac77d91f44 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts @@ -6,10 +6,7 @@ export type ExtendedJsonRpcMiddleware = JsonRpcMiddleware & { destroy?: () => void; }; -type MiddlewareByScope = Record< - Scope, - ExtendedJsonRpcMiddleware | { destroy: () => void } ->; +type MiddlewareByScope = Record; type MiddlewareManager = { removeMiddleware: (scope: Scope, domain: string) => void; From 2a5920ce796ceb7b6453e7ff3534fb5504fa8af6 Mon Sep 17 00:00:00 2001 From: Shane Date: Fri, 2 Aug 2024 08:46:08 -0700 Subject: [PATCH 29/35] Rename multichainSubscriptionManager.ts to MultichainSubscriptionManager.ts --- ...ainSubscriptionManager.ts => MultichainSubscriptionManager.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/scripts/lib/multichain-api/{multichainSubscriptionManager.ts => MultichainSubscriptionManager.ts} (100%) diff --git a/app/scripts/lib/multichain-api/multichainSubscriptionManager.ts b/app/scripts/lib/multichain-api/MultichainSubscriptionManager.ts similarity index 100% rename from app/scripts/lib/multichain-api/multichainSubscriptionManager.ts rename to app/scripts/lib/multichain-api/MultichainSubscriptionManager.ts From 11476292513520ac4e6f1b7608d118aa7b1f8e71 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Fri, 2 Aug 2024 16:06:42 -0400 Subject: [PATCH 30/35] fix: convert to class and set middleware destroy in constructor --- .../MultichainMiddlewareManager.test.ts | 9 +-- .../MultichainMiddlewareManager.ts | 76 ++++++++----------- 2 files changed, 34 insertions(+), 51 deletions(-) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts index 717daeb0e45b..4028fb3d9658 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts @@ -1,12 +1,11 @@ import { JsonRpcRequest } from '@metamask/utils'; -// eslint-disable-next-line import/no-named-as-default -import createMultichainMiddlewareManager, { +import MultichainMiddlewareManager, { ExtendedJsonRpcMiddleware, } from './MultichainMiddlewareManager'; describe('MultichainMiddlewareManager', () => { it('should add middleware and get called for the scope', () => { - const multichainMiddlewareManager = createMultichainMiddlewareManager(); + const multichainMiddlewareManager = new MultichainMiddlewareManager(); const middlewareSpy = jest.fn() as unknown as ExtendedJsonRpcMiddleware; (middlewareSpy as { destroy: () => void }).destroy = jest.fn(); const domain = 'example.com'; @@ -28,7 +27,7 @@ describe('MultichainMiddlewareManager', () => { expect(middlewareSpy).toHaveBeenCalled(); }); it('should remove middleware', () => { - const multichainMiddlewareManager = createMultichainMiddlewareManager(); + const multichainMiddlewareManager = new MultichainMiddlewareManager(); const middlewareMock = jest.fn() as unknown as ExtendedJsonRpcMiddleware; middlewareMock.destroy = jest.fn(); const scope = 'eip155:1'; @@ -47,7 +46,7 @@ describe('MultichainMiddlewareManager', () => { expect(endSpy).not.toHaveBeenCalled(); }); it('should remove all middleware', () => { - const multichainMiddlewareManager = createMultichainMiddlewareManager(); + const multichainMiddlewareManager = new MultichainMiddlewareManager(); const middlewareMock = jest.fn() as unknown as ExtendedJsonRpcMiddleware; middlewareMock.destroy = jest.fn(); const scope = 'eip155:1'; diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts index a3ac77d91f44..8983796eca85 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts @@ -8,80 +8,64 @@ export type ExtendedJsonRpcMiddleware = JsonRpcMiddleware & { type MiddlewareByScope = Record; -type MiddlewareManager = { - removeMiddleware: (scope: Scope, domain: string) => void; - removeAllMiddleware: () => void; - addMiddleware: ( - scope: Scope, - domain: string, - middleware: ExtendedJsonRpcMiddleware, - ) => void; - middleware: ExtendedJsonRpcMiddleware; -}; +export default class MultichainMiddlewareManager { + constructor() { + this.middleware.destroy = this.removeAllMiddleware.bind(this); + } -export default function createMultichainMiddlewareManager(): MiddlewareManager { - const middlewareCountByDomainAndScope: { + private middlewareCountByDomainAndScope: { [scope: string]: { [domain: string]: number }; } = {}; - const middlewaresByScope: MiddlewareByScope = {}; + private middlewaresByScope: MiddlewareByScope = {}; - function removeAllMiddleware() { + public removeAllMiddleware() { for (const [scope, domainObject] of Object.entries( - middlewareCountByDomainAndScope, + this.middlewareCountByDomainAndScope, )) { for (const domain of Object.keys(domainObject)) { - removeMiddleware(scope, domain); + this.removeMiddleware(scope, domain); } } } - function addMiddleware( + public addMiddleware( scope: Scope, domain: string, middleware: ExtendedJsonRpcMiddleware, ) { - middlewareCountByDomainAndScope[scope] = - middlewareCountByDomainAndScope[scope] || {}; - middlewareCountByDomainAndScope[scope][domain] = - middlewareCountByDomainAndScope[scope][domain] || 0; - middlewareCountByDomainAndScope[scope][domain] += 1; - if (!middlewaresByScope[scope]) { - middlewaresByScope[scope] = middleware; + this.middlewareCountByDomainAndScope[scope] = + this.middlewareCountByDomainAndScope[scope] || {}; + this.middlewareCountByDomainAndScope[scope][domain] = + this.middlewareCountByDomainAndScope[scope][domain] || 0; + this.middlewareCountByDomainAndScope[scope][domain] += 1; + if (!this.middlewaresByScope[scope]) { + this.middlewaresByScope[scope] = middleware; } } - function removeMiddleware(scope: Scope, domain: string) { - if (middlewareCountByDomainAndScope[scope]?.[domain]) { - middlewareCountByDomainAndScope[scope][domain] -= 1; - if (middlewareCountByDomainAndScope[scope][domain] === 0) { - delete middlewareCountByDomainAndScope[scope][domain]; + public removeMiddleware(scope: Scope, domain: string) { + if (this.middlewareCountByDomainAndScope[scope]?.[domain]) { + this.middlewareCountByDomainAndScope[scope][domain] -= 1; + if (this.middlewareCountByDomainAndScope[scope][domain] === 0) { + delete this.middlewareCountByDomainAndScope[scope][domain]; } - if (Object.keys(middlewareCountByDomainAndScope[scope]).length === 0) { - delete middlewareCountByDomainAndScope[scope]; - delete middlewaresByScope[scope]; + if ( + Object.keys(this.middlewareCountByDomainAndScope[scope]).length === 0 + ) { + delete this.middlewareCountByDomainAndScope[scope]; + delete this.middlewaresByScope[scope]; } } } - const middleware: ExtendedJsonRpcMiddleware = (req, res, next, end) => { + public middleware: ExtendedJsonRpcMiddleware = (req, res, next, end) => { const r = req as unknown as { scope: string }; const { scope } = r; - if (typeof middlewaresByScope[scope] === 'function') { - middlewaresByScope[scope](req, res, next, end); + if (typeof this.middlewaresByScope[scope] === 'function') { + this.middlewaresByScope[scope](req, res, next, end); } else { next(); } }; - - middleware.destroy = () => { - removeAllMiddleware(); - }; - - return { - removeAllMiddleware, - addMiddleware, - removeMiddleware, - middleware, - }; } From f172d1b8999b8b89e3a42521e5c7fe87861ed3ac Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Fri, 2 Aug 2024 16:53:57 -0400 Subject: [PATCH 31/35] fix: remove destroy typings from test --- .../lib/multichain-api/MultichainMiddlewareManager.test.ts | 3 --- .../lib/multichain-api/MultichainSubscriptionManager.test.ts | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts index 4028fb3d9658..cb1060770c42 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.test.ts @@ -7,7 +7,6 @@ describe('MultichainMiddlewareManager', () => { it('should add middleware and get called for the scope', () => { const multichainMiddlewareManager = new MultichainMiddlewareManager(); const middlewareSpy = jest.fn() as unknown as ExtendedJsonRpcMiddleware; - (middlewareSpy as { destroy: () => void }).destroy = jest.fn(); const domain = 'example.com'; multichainMiddlewareManager.addMiddleware( 'eip155:1', @@ -29,7 +28,6 @@ describe('MultichainMiddlewareManager', () => { it('should remove middleware', () => { const multichainMiddlewareManager = new MultichainMiddlewareManager(); const middlewareMock = jest.fn() as unknown as ExtendedJsonRpcMiddleware; - middlewareMock.destroy = jest.fn(); const scope = 'eip155:1'; const domain = 'example.com'; multichainMiddlewareManager.addMiddleware(scope, domain, middlewareMock); @@ -48,7 +46,6 @@ describe('MultichainMiddlewareManager', () => { it('should remove all middleware', () => { const multichainMiddlewareManager = new MultichainMiddlewareManager(); const middlewareMock = jest.fn() as unknown as ExtendedJsonRpcMiddleware; - middlewareMock.destroy = jest.fn(); const scope = 'eip155:1'; const scope2 = 'eip155:2'; const domain = 'example.com'; diff --git a/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts b/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts index 637970d7dd26..587b38e71dd5 100644 --- a/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts +++ b/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts @@ -55,6 +55,7 @@ describe('MultichainSubscriptionManager', () => { }, }); }); + it('should unsubscribe from a domain and scope', () => { const domain = 'example.com'; const scope = 'eip155:1'; @@ -80,6 +81,7 @@ describe('MultichainSubscriptionManager', () => { expect(spy).not.toHaveBeenCalled(); }); + it('should unsubscribe from a scope', () => { const domain = 'example.com'; const scope = 'eip155:1'; @@ -105,6 +107,7 @@ describe('MultichainSubscriptionManager', () => { expect(spy).not.toHaveBeenCalled(); }); + it('should unsubscribe all', () => { const domain = 'example.com'; const scope = 'eip155:1'; From 77760fedad6feda23c6a5302e32d3d118b9e0731 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Mon, 19 Aug 2024 10:05:45 -0400 Subject: [PATCH 32/35] fix: add note about JsonRpcMiddleware type and give more descriptive name for spy in middleware tests --- .../MultichainMiddlewareManager.ts | 1 + .../MultichainSubscriptionManager.test.ts | 24 +++++++++---------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts index 8983796eca85..a3c9c84c5e8e 100644 --- a/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts +++ b/app/scripts/lib/multichain-api/MultichainMiddlewareManager.ts @@ -2,6 +2,7 @@ import { JsonRpcMiddleware } from 'json-rpc-engine'; import { Scope } from './scope'; // Extend JsonRpcMiddleware to include the destroy method +// this was introduced in 7.0.0 of json-rpc-engine: https://github.com/MetaMask/json-rpc-engine/blob/v7.0.0/src/JsonRpcEngine.ts#L29-L40 export type ExtendedJsonRpcMiddleware = JsonRpcMiddleware & { destroy?: () => void; }; diff --git a/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts b/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts index 587b38e71dd5..0160e120406f 100644 --- a/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts +++ b/app/scripts/lib/multichain-api/MultichainSubscriptionManager.test.ts @@ -39,15 +39,15 @@ describe('MultichainSubscriptionManager', () => { findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, getNetworkClientById: mockGetNetworkClientById, }); - const spy = jest.fn(); + const onNotificationSpy = jest.fn(); - subscriptionManager.on('notification', spy); + subscriptionManager.on('notification', onNotificationSpy); subscriptionManager.subscribe(scope, domain); subscriptionManager.subscriptionManagerByChain[scope].events.emit( 'notification', newHeadsNotificationMock, ); - expect(spy).toHaveBeenCalledWith(domain, { + expect(onNotificationSpy).toHaveBeenCalledWith(domain, { method: 'wallet_invokeMethod', params: { scope, @@ -68,8 +68,8 @@ describe('MultichainSubscriptionManager', () => { findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, getNetworkClientById: mockGetNetworkClientById, }); - const spy = jest.fn(); - subscriptionManager.on('notification', spy); + const onNotificationSpy = jest.fn(); + subscriptionManager.on('notification', onNotificationSpy); subscriptionManager.subscribe(scope, domain); const scopeSubscriptionManager = subscriptionManager.subscriptionManagerByChain[scope]; @@ -79,7 +79,7 @@ describe('MultichainSubscriptionManager', () => { newHeadsNotificationMock, ); - expect(spy).not.toHaveBeenCalled(); + expect(onNotificationSpy).not.toHaveBeenCalled(); }); it('should unsubscribe from a scope', () => { @@ -94,8 +94,8 @@ describe('MultichainSubscriptionManager', () => { findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, getNetworkClientById: mockGetNetworkClientById, }); - const spy = jest.fn(); - subscriptionManager.on('notification', spy); + const onNotificationSpy = jest.fn(); + subscriptionManager.on('notification', onNotificationSpy); subscriptionManager.subscribe(scope, domain); const scopeSubscriptionManager = subscriptionManager.subscriptionManagerByChain[scope]; @@ -105,7 +105,7 @@ describe('MultichainSubscriptionManager', () => { newHeadsNotificationMock, ); - expect(spy).not.toHaveBeenCalled(); + expect(onNotificationSpy).not.toHaveBeenCalled(); }); it('should unsubscribe all', () => { @@ -120,8 +120,8 @@ describe('MultichainSubscriptionManager', () => { findNetworkClientIdByChainId: mockFindNetworkClientIdByChainId, getNetworkClientById: mockGetNetworkClientById, }); - const spy = jest.fn(); - subscriptionManager.on('notification', spy); + const onNotificationSpy = jest.fn(); + subscriptionManager.on('notification', onNotificationSpy); subscriptionManager.subscribe(scope, domain); const scope2 = 'eip155:2'; subscriptionManager.subscribe(scope2, domain); @@ -139,6 +139,6 @@ describe('MultichainSubscriptionManager', () => { newHeadsNotificationMock, ); - expect(spy).not.toHaveBeenCalled(); + expect(onNotificationSpy).not.toHaveBeenCalled(); }); }); From 39db51ee6b964dbf202c666f83deee9fe771b813 Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Mon, 19 Aug 2024 10:56:18 -0400 Subject: [PATCH 33/35] fix: add tests and feedback for removed authorizations selector --- .../controllers/permissions/selectors.js | 14 ++++---- .../controllers/permissions/selectors.test.js | 36 ++++++++++++++++++- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/app/scripts/controllers/permissions/selectors.js b/app/scripts/controllers/permissions/selectors.js index f5a653eb904c..af5ca429c5a0 100644 --- a/app/scripts/controllers/permissions/selectors.js +++ b/app/scripts/controllers/permissions/selectors.js @@ -175,13 +175,15 @@ export const getRemovedAuthorizations = ( newAuthorizationsMap, previousAuthorizationsMap, ) => { - // If there are no previous authorizations, there are no removed authorizations. - if (previousAuthorizationsMap === undefined) { - return new Map(); - } - const removedAuthorizations = new Map(); - if (newAuthorizationsMap === previousAuthorizationsMap) { + + // If there are no previous authorizations, there are no removed authorizations. + // OR If the new authorizations map is the same as the previous authorizations map, + // there are no removed authorizations + if ( + previousAuthorizationsMap === undefined || + newAuthorizationsMap === previousAuthorizationsMap + ) { return removedAuthorizations; } diff --git a/app/scripts/controllers/permissions/selectors.test.js b/app/scripts/controllers/permissions/selectors.test.js index a32eabf7738e..cb0705906bd8 100644 --- a/app/scripts/controllers/permissions/selectors.test.js +++ b/app/scripts/controllers/permissions/selectors.test.js @@ -1,5 +1,9 @@ import { cloneDeep } from 'lodash'; -import { getChangedAccounts, getPermittedAccountsByOrigin } from './selectors'; +import { + getChangedAccounts, + getPermittedAccountsByOrigin, + getRemovedAuthorizations, +} from './selectors'; describe('PermissionController selectors', () => { describe('getChangedAccounts', () => { @@ -113,4 +117,34 @@ describe('PermissionController selectors', () => { expect(selected2).toBe(getPermittedAccountsByOrigin(state2)); }); }); + describe('getRemovedAuthorizations', () => { + it('returns an empty map if the new and previous values are the same', () => { + const newAuthorizations = new Map(); + expect( + getRemovedAuthorizations(newAuthorizations, newAuthorizations), + ).toStrictEqual(new Map()); + }); + + it('returns a new map of the removed authorizations if the new and previous values differ', () => { + const mockAuthorization = { + requiredScopes: { + 'eip155:1': { + methods: ['eth_sendTransaction'], + notifications: [], + }, + }, + optionalScopes: {}, + }; + const previousAuthorizations = new Map([ + ['foo.bar', mockAuthorization], + ['bar.baz', mockAuthorization], + ]); + + const newAuthorizations = new Map([['foo.bar', mockAuthorization]]); + + expect( + getRemovedAuthorizations(newAuthorizations, previousAuthorizations), + ).toStrictEqual(new Map([['bar.baz', mockAuthorization]])); + }); + }); }); From 804a55b243a8e5eac44e459f8e6c19455ee4fc6f Mon Sep 17 00:00:00 2001 From: Shane Jonas Date: Mon, 19 Aug 2024 15:22:00 -0400 Subject: [PATCH 34/35] fix: dedupe --- yarn.lock | 153 +++--------------------------------------------------- 1 file changed, 8 insertions(+), 145 deletions(-) diff --git a/yarn.lock b/yarn.lock index f3fa10cc0e85..f80096299e83 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4242,20 +4242,13 @@ __metadata: languageName: node linkType: hard -"@json-schema-tools/traverse@npm:^1.10.4": +"@json-schema-tools/traverse@npm:^1.10.4, @json-schema-tools/traverse@npm:^1.7.5, @json-schema-tools/traverse@npm:^1.7.8": version: 1.10.4 resolution: "@json-schema-tools/traverse@npm:1.10.4" checksum: 10/0027bc90df01c5eeee0833e722b7320b53be8b5ce3f4e0e4a6e45713a38e6f88f21aba31e3dd973093ef75cd21a40c07fe8f112da8f49a7919b1c0e44c904d20 languageName: node linkType: hard -"@json-schema-tools/traverse@npm:^1.7.5, @json-schema-tools/traverse@npm:^1.7.8": - version: 1.10.3 - resolution: "@json-schema-tools/traverse@npm:1.10.3" - checksum: 10/690623740d223ea373d8e561dad5c70bf86461bcedc5fc45da01c87bcdf3284bbdbad3006d4a423f8d82e4b2d4580e45f92c0b272f006024fb597d7f01876215 - languageName: node - linkType: hard - "@juggle/resize-observer@npm:^3.3.1": version: 3.4.0 resolution: "@juggle/resize-observer@npm:3.4.0" @@ -10358,17 +10351,7 @@ __metadata: languageName: node linkType: hard -"@types/eslint@npm:*, @types/eslint@npm:^8.44.7": - version: 8.44.8 - resolution: "@types/eslint@npm:8.44.8" - dependencies: - "@types/estree": "npm:*" - "@types/json-schema": "npm:*" - checksum: 10/d6e0788eb7bff90e5f5435b0babe057e76a7d3eed1e36080bacd7b749098eddae499ddb3c0ce6438addce98cc6020d9653b5012dec54e47ca96faa7b8e25d068 - languageName: node - linkType: hard - -"@types/eslint@npm:^8.4.2": +"@types/eslint@npm:*, @types/eslint@npm:^8.4.2, @types/eslint@npm:^8.44.7": version: 8.56.11 resolution: "@types/eslint@npm:8.56.11" dependencies: @@ -10406,7 +10389,7 @@ __metadata: languageName: node linkType: hard -"@types/express-serve-static-core@npm:*": +"@types/express-serve-static-core@npm:*, @types/express-serve-static-core@npm:^4.17.33": version: 4.17.41 resolution: "@types/express-serve-static-core@npm:4.17.41" dependencies: @@ -10418,19 +10401,7 @@ __metadata: languageName: node linkType: hard -"@types/express-serve-static-core@npm:^4.17.33": - version: 4.17.35 - resolution: "@types/express-serve-static-core@npm:4.17.35" - dependencies: - "@types/node": "npm:*" - "@types/qs": "npm:*" - "@types/range-parser": "npm:*" - "@types/send": "npm:*" - checksum: 10/9f08212ac163e9b2a1005d84cc43ace52d5057dfaa009c575eb3f3a659949b9c9cecec0cbff863622871c56e1c604bd67857a5e1d353256eaf9adacec59f87bf - languageName: node - linkType: hard - -"@types/express@npm:*, @types/express@npm:^4.17.21": +"@types/express@npm:*, @types/express@npm:^4.17.21, @types/express@npm:^4.7.0": version: 4.17.21 resolution: "@types/express@npm:4.17.21" dependencies: @@ -10442,18 +10413,6 @@ __metadata: languageName: node linkType: hard -"@types/express@npm:^4.7.0": - version: 4.17.17 - resolution: "@types/express@npm:4.17.17" - dependencies: - "@types/body-parser": "npm:*" - "@types/express-serve-static-core": "npm:^4.17.33" - "@types/qs": "npm:*" - "@types/serve-static": "npm:*" - checksum: 10/e2959a5fecdc53f8a524891a16e66dfc330ee0519e89c2579893179db686e10cfa6079a68e0fb8fd00eedbcaf3eabfd10916461939f3bc02ef671d848532c37e - languageName: node - linkType: hard - "@types/filesystem@npm:*": version: 0.0.36 resolution: "@types/filesystem@npm:0.0.36" @@ -10935,20 +10894,13 @@ __metadata: languageName: node linkType: hard -"@types/prettier@npm:^2.6.0": +"@types/prettier@npm:^2.6.0, @types/prettier@npm:^2.7.2": version: 2.7.3 resolution: "@types/prettier@npm:2.7.3" checksum: 10/cda84c19acc3bf327545b1ce71114a7d08efbd67b5030b9e8277b347fa57b05178045f70debe1d363ff7efdae62f237260713aafc2d7217e06fc99b048a88497 languageName: node linkType: hard -"@types/prettier@npm:^2.7.2": - version: 2.7.2 - resolution: "@types/prettier@npm:2.7.2" - checksum: 10/8b91984884220a4b14b8b0803b5ed02acfe7b8cbee3f4d814e7c021818fbaf936b0d8a67b9aa1bb6c0126fbdd788432095416ffcf48576de71541e998717b18a - languageName: node - linkType: hard - "@types/pretty-hrtime@npm:^1.0.0": version: 1.0.1 resolution: "@types/pretty-hrtime@npm:1.0.1" @@ -11509,13 +11461,6 @@ __metadata: languageName: node linkType: hard -"@typescript-eslint/types@npm:5.59.6": - version: 5.59.6 - resolution: "@typescript-eslint/types@npm:5.59.6" - checksum: 10/fda210cb8118a1484a7a7536d7e64e44ad749c914d907a5a7ac289b3e320522d9c3a61faef1fa2d12264df68d2f20b63fe6e5d69ba616be539548e4894cc2c61 - languageName: node - linkType: hard - "@typescript-eslint/types@npm:5.62.0": version: 5.62.0 resolution: "@typescript-eslint/types@npm:5.62.0" @@ -11548,7 +11493,7 @@ __metadata: languageName: node linkType: hard -"@typescript-eslint/typescript-estree@npm:5.62.0": +"@typescript-eslint/typescript-estree@npm:5.62.0, @typescript-eslint/typescript-estree@npm:^5.59.5": version: 5.62.0 resolution: "@typescript-eslint/typescript-estree@npm:5.62.0" dependencies: @@ -11585,24 +11530,6 @@ __metadata: languageName: node linkType: hard -"@typescript-eslint/typescript-estree@npm:^5.59.5": - version: 5.59.6 - resolution: "@typescript-eslint/typescript-estree@npm:5.59.6" - dependencies: - "@typescript-eslint/types": "npm:5.59.6" - "@typescript-eslint/visitor-keys": "npm:5.59.6" - debug: "npm:^4.3.4" - globby: "npm:^11.1.0" - is-glob: "npm:^4.0.3" - semver: "npm:^7.3.7" - tsutils: "npm:^3.21.0" - peerDependenciesMeta: - typescript: - optional: true - checksum: 10/3798ef5804b0ac40909ed793b20fb1912a83b01dd3773711cd2b3670477dc59d0af7524849b24c809f54df76c83ec4e1c6ca5685bd101bf519a1118f79adcfe3 - languageName: node - linkType: hard - "@typescript-eslint/utils@npm:7.11.0": version: 7.11.0 resolution: "@typescript-eslint/utils@npm:7.11.0" @@ -11645,16 +11572,6 @@ __metadata: languageName: node linkType: hard -"@typescript-eslint/visitor-keys@npm:5.59.6": - version: 5.59.6 - resolution: "@typescript-eslint/visitor-keys@npm:5.59.6" - dependencies: - "@typescript-eslint/types": "npm:5.59.6" - eslint-visitor-keys: "npm:^3.3.0" - checksum: 10/b52c0b60fab876f817352f90ffee1cdb1813a83b06924bebf4b1b2d784ef8decb1c5318c09b3473350c657af6c788f005e19ea61874c14b8b454f3e2edab2447 - languageName: node - linkType: hard - "@typescript-eslint/visitor-keys@npm:5.62.0": version: 5.62.0 resolution: "@typescript-eslint/visitor-keys@npm:5.62.0" @@ -13248,17 +13165,6 @@ __metadata: languageName: node linkType: hard -"axios@npm:^1.6.7": - version: 1.6.8 - resolution: "axios@npm:1.6.8" - dependencies: - follow-redirects: "npm:^1.15.6" - form-data: "npm:^4.0.0" - proxy-from-env: "npm:^1.1.0" - checksum: 10/3f9a79eaf1d159544fca9576261ff867cbbff64ed30017848e4210e49f3b01e97cf416390150e6fdf6633f336cd43dc1151f890bbd09c3c01ad60bb0891eee63 - languageName: node - linkType: hard - "b4a@npm:^1.6.4": version: 1.6.4 resolution: "b4a@npm:1.6.4" @@ -15550,15 +15456,6 @@ __metadata: languageName: node linkType: hard -"contentful-resolve-response@npm:^1.8.1": - version: 1.8.1 - resolution: "contentful-resolve-response@npm:1.8.1" - dependencies: - fast-copy: "npm:^2.1.7" - checksum: 10/6023824e98843d47c900501d2252336dd1dfebe8d868cb81b650252f5946963063cbe2b466e8e2238d41634a779dbfe4a43ba6c7996ca77124824c2554f66cab - languageName: node - linkType: hard - "contentful-resolve-response@npm:^1.9.0": version: 1.9.0 resolution: "contentful-resolve-response@npm:1.9.0" @@ -15568,19 +15465,6 @@ __metadata: languageName: node linkType: hard -"contentful-sdk-core@npm:^8.1.0": - version: 8.1.2 - resolution: "contentful-sdk-core@npm:8.1.2" - dependencies: - fast-copy: "npm:^2.1.7" - lodash.isplainobject: "npm:^4.0.6" - lodash.isstring: "npm:^4.0.1" - p-throttle: "npm:^4.1.1" - qs: "npm:^6.11.2" - checksum: 10/9e5b6dc1b2a1a91a9c4a01b0f24aff23606d0a6f7b56b7ca5930d57f4c29503886874e2ac653d924b742e9445da31b862c651b2d98e1f74ec35f6f0a3afdb912 - languageName: node - linkType: hard - "contentful-sdk-core@npm:^8.3.1": version: 8.3.1 resolution: "contentful-sdk-core@npm:8.3.1" @@ -15594,7 +15478,7 @@ __metadata: languageName: node linkType: hard -"contentful@npm:^10.3.6": +"contentful@npm:^10.3.6, contentful@npm:^10.8.7": version: 10.14.0 resolution: "contentful@npm:10.14.0" dependencies: @@ -15609,20 +15493,6 @@ __metadata: languageName: node linkType: hard -"contentful@npm:^10.8.7": - version: 10.8.7 - resolution: "contentful@npm:10.8.7" - dependencies: - "@contentful/rich-text-types": "npm:^16.0.2" - axios: "npm:^1.6.7" - contentful-resolve-response: "npm:^1.8.1" - contentful-sdk-core: "npm:^8.1.0" - json-stringify-safe: "npm:^5.0.1" - type-fest: "npm:^4.0.0" - checksum: 10/db0bf6342ed9fdc577c583bc627626ea95ee2af3a0b47600fae48dbb1354f64bc226174c4d98dfd224d4f991391d1bfa367a5a01bad9ea44fbc5f9f424de6798 - languageName: node - linkType: hard - "continuable-cache@npm:^0.3.1": version: 0.3.1 resolution: "continuable-cache@npm:0.3.1" @@ -27767,20 +27637,13 @@ __metadata: languageName: node linkType: hard -"node-forge@npm:^1": +"node-forge@npm:^1, node-forge@npm:^1.2.1": version: 1.3.1 resolution: "node-forge@npm:1.3.1" checksum: 10/05bab6868633bf9ad4c3b1dd50ec501c22ffd69f556cdf169a00998ca1d03e8107a6032ba013852f202035372021b845603aeccd7dfcb58cdb7430013b3daa8d languageName: node linkType: hard -"node-forge@npm:^1.2.1": - version: 1.3.0 - resolution: "node-forge@npm:1.3.0" - checksum: 10/ce829501c839b0ed9b6d752d2166eff136fab60c309d32dbd900e5e2764b2d631d4e4519c2389da97ebb214dce3bc6962c9f288028e13ae070bc947d4110abbc - languageName: node - linkType: hard - "node-gyp-build@npm:4.4.0": version: 4.4.0 resolution: "node-gyp-build@npm:4.4.0" From 9b9b581e6ed23d88f334992cc06a18c707888d43 Mon Sep 17 00:00:00 2001 From: MetaMask Bot Date: Mon, 19 Aug 2024 20:01:45 +0000 Subject: [PATCH 35/35] Update LavaMoat policies --- lavamoat/build-system/policy.json | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lavamoat/build-system/policy.json b/lavamoat/build-system/policy.json index b9781e48445f..0dcb0cc02724 100644 --- a/lavamoat/build-system/policy.json +++ b/lavamoat/build-system/policy.json @@ -2117,8 +2117,7 @@ "chokidar>normalize-path": true, "chokidar>readdirp": true, "del>is-glob": true, - "eslint>glob-parent": true, - "tsx>fsevents": true + "eslint>glob-parent": true } }, "chokidar>anymatch": { @@ -8850,13 +8849,6 @@ "typescript": true } }, - "tsx>fsevents": { - "globals": { - "console.assert": true, - "process.platform": true - }, - "native": true - }, "typescript": { "builtin": { "buffer.Buffer": true,