From fc955bd04821462c9f675ac9cb3a9f861e2c2ecd Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 15 Nov 2024 17:34:44 -0600 Subject: [PATCH] remove queuedRequestPreference --- .../src/QueuedRequestMiddleware.test.ts | 9 +- .../src/QueuedRequestMiddleware.ts | 8 +- .../src/SelectedNetworkController.ts | 62 +- .../tests/SelectedNetworkController.test.ts | 1061 ++++++----------- 4 files changed, 394 insertions(+), 746 deletions(-) diff --git a/packages/queued-request-controller/src/QueuedRequestMiddleware.test.ts b/packages/queued-request-controller/src/QueuedRequestMiddleware.test.ts index 6af151aae0..80738262d3 100644 --- a/packages/queued-request-controller/src/QueuedRequestMiddleware.test.ts +++ b/packages/queued-request-controller/src/QueuedRequestMiddleware.test.ts @@ -86,7 +86,6 @@ describe('createQueuedRequestMiddleware', () => { const mockEnqueueRequest = getMockEnqueueRequest(); const middleware = buildQueuedRequestMiddleware({ enqueueRequest: mockEnqueueRequest, - useRequestQueue: () => true, }); const request = { @@ -105,7 +104,7 @@ describe('createQueuedRequestMiddleware', () => { const mockEnqueueRequest = getMockEnqueueRequest(); const middleware = buildQueuedRequestMiddleware({ enqueueRequest: mockEnqueueRequest, - useRequestQueue: () => true, + shouldEnqueueRequest: ({ method }) => method === 'method_with_confirmation', }); @@ -145,7 +144,6 @@ describe('createQueuedRequestMiddleware', () => { it('calls next after a request is queued and processed', async () => { const middleware = buildQueuedRequestMiddleware({ enqueueRequest: getMockEnqueueRequest(), - useRequestQueue: () => true, }); const request = { ...getRequestDefaults(), @@ -167,7 +165,7 @@ describe('createQueuedRequestMiddleware', () => { enqueueRequest: jest .fn() .mockRejectedValue(new Error('enqueuing error')), - useRequestQueue: () => true, + shouldEnqueueRequest: () => true, }); const request = { @@ -191,7 +189,7 @@ describe('createQueuedRequestMiddleware', () => { enqueueRequest: jest .fn() .mockRejectedValue(new Error('enqueuing error')), - useRequestQueue: () => true, + shouldEnqueueRequest: () => true, }); const request = { @@ -271,7 +269,6 @@ function buildQueuedRequestMiddleware( ) { const options = { enqueueRequest: getMockEnqueueRequest(), - useRequestQueue: () => false, shouldEnqueueRequest: () => false, ...overrideOptions, }; diff --git a/packages/queued-request-controller/src/QueuedRequestMiddleware.ts b/packages/queued-request-controller/src/QueuedRequestMiddleware.ts index 5edecf787e..5b52fb649b 100644 --- a/packages/queued-request-controller/src/QueuedRequestMiddleware.ts +++ b/packages/queued-request-controller/src/QueuedRequestMiddleware.ts @@ -38,17 +38,14 @@ function hasRequiredMetadata( * * @param options - Configuration options. * @param options.enqueueRequest - A method for enqueueing a request. - * @param options.useRequestQueue - A function that determines if the request queue feature is enabled. * @param options.shouldEnqueueRequest - A function that returns if a request should be handled by the QueuedRequestController. * @returns The JSON-RPC middleware that manages queued requests. */ export const createQueuedRequestMiddleware = ({ enqueueRequest, - useRequestQueue, shouldEnqueueRequest, }: { enqueueRequest: QueuedRequestController['enqueueRequest']; - useRequestQueue: () => boolean; shouldEnqueueRequest: ( request: QueuedRequestMiddlewareJsonRpcRequest, ) => boolean; @@ -56,9 +53,8 @@ export const createQueuedRequestMiddleware = ({ return createAsyncMiddleware(async (req: JsonRpcRequest, res, next) => { hasRequiredMetadata(req); - // if the request queue feature is turned off, or this method is not a confirmation method - // bypass the queue completely - if (!useRequestQueue() || !shouldEnqueueRequest(req)) { + // if this method is not a confirmation method bypass the queue completely + if (!shouldEnqueueRequest(req)) { return await next(); } diff --git a/packages/selected-network-controller/src/SelectedNetworkController.ts b/packages/selected-network-controller/src/SelectedNetworkController.ts index 8f73418122..ab87c541ca 100644 --- a/packages/selected-network-controller/src/SelectedNetworkController.ts +++ b/packages/selected-network-controller/src/SelectedNetworkController.ts @@ -102,10 +102,6 @@ export type SelectedNetworkControllerMessenger = RestrictedControllerMessenger< export type SelectedNetworkControllerOptions = { state?: SelectedNetworkControllerState; messenger: SelectedNetworkControllerMessenger; - useRequestQueuePreference: boolean; - onPreferencesStateChange: ( - listener: (preferencesState: { useRequestQueue: boolean }) => void, - ) => void; domainProxyMap: Map; }; @@ -124,23 +120,17 @@ export class SelectedNetworkController extends BaseController< > { #domainProxyMap: Map; - #useRequestQueuePreference: boolean; - /** * Construct a SelectedNetworkController controller. * * @param options - The controller options. * @param options.messenger - The restricted controller messenger for the EncryptionPublicKey controller. * @param options.state - The controllers initial state. - * @param options.useRequestQueuePreference - A boolean indicating whether to use the request queue preference. - * @param options.onPreferencesStateChange - A callback that is called when the preference state changes. * @param options.domainProxyMap - A map for storing domain-specific proxies that are held in memory only during use. */ constructor({ messenger, state = getDefaultState(), - useRequestQueuePreference, - onPreferencesStateChange, domainProxyMap, }: SelectedNetworkControllerOptions) { super({ @@ -149,7 +139,6 @@ export class SelectedNetworkController extends BaseController< messenger, state, }); - this.#useRequestQueuePreference = useRequestQueuePreference; this.#domainProxyMap = domainProxyMap; this.#registerMessageHandlers(); @@ -247,21 +236,6 @@ export class SelectedNetworkController extends BaseController< } }, ); - - onPreferencesStateChange(({ useRequestQueue }) => { - if (this.#useRequestQueuePreference !== useRequestQueue) { - if (!useRequestQueue) { - // Loop through all domains and points each domain's proxy - // to the NetworkController's own proxy of the globally selected networkClient - Object.keys(this.state.domains).forEach((domain) => { - this.#unsetNetworkClientIdForDomain(domain); - }); - } else { - this.#resetAllPermissionedDomains(); - } - this.#useRequestQueuePreference = useRequestQueue; - } - }); } #registerMessageHandlers(): void { @@ -326,31 +300,10 @@ export class SelectedNetworkController extends BaseController< ); } - // Loop through all domains and for those with permissions it points that domain's proxy - // to an unproxied instance of the globally selected network client. - // NOT the NetworkController's proxy of the globally selected networkClient - #resetAllPermissionedDomains() { - this.#domainProxyMap.forEach((_: NetworkProxy, domain: string) => { - const { selectedNetworkClientId } = this.messagingSystem.call( - 'NetworkController:getState', - ); - // can't use public setNetworkClientIdForDomain because it will throw an error - // rather than simply skip if the domain doesn't have permissions which can happen - // in this case since proxies are added for each site the user visits - if (this.#domainHasPermissions(domain)) { - this.#setNetworkClientIdForDomain(domain, selectedNetworkClientId); - } - }); - } - setNetworkClientIdForDomain( domain: Domain, networkClientId: NetworkClientId, ) { - if (!this.#useRequestQueuePreference) { - return; - } - if (domain === METAMASK_DOMAIN) { throw new Error( `NetworkClientId for domain "${METAMASK_DOMAIN}" cannot be set on the SelectedNetworkController`, @@ -373,9 +326,6 @@ export class SelectedNetworkController extends BaseController< getNetworkClientIdForDomain(domain: Domain): NetworkClientId { const { selectedNetworkClientId: metamaskSelectedNetworkClientId } = this.messagingSystem.call('NetworkController:getState'); - if (!this.#useRequestQueuePreference) { - return metamaskSelectedNetworkClientId; - } return this.state.domains[domain] ?? metamaskSelectedNetworkClientId; } @@ -403,10 +353,7 @@ export class SelectedNetworkController extends BaseController< let networkProxy = this.#domainProxyMap.get(domain); if (networkProxy === undefined) { let networkClient; - if ( - this.#useRequestQueuePreference && - this.#domainHasPermissions(domain) - ) { + if (this.#domainHasPermissions(domain)) { const networkClientId = this.getNetworkClientIdForDomain(domain); networkClient = this.messagingSystem.call( 'NetworkController:getNetworkClientById', @@ -416,10 +363,11 @@ export class SelectedNetworkController extends BaseController< networkClient = this.messagingSystem.call( 'NetworkController:getSelectedNetworkClient', ); - if (networkClient === undefined) { - throw new Error('Selected network not initialized'); - } } + if (networkClient === undefined) { + throw new Error('Selected network not initialized'); + } + networkProxy = { provider: createEventEmitterProxy(networkClient.provider), blockTracker: createEventEmitterProxy(networkClient.blockTracker, { diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index fd975016ca..dcc01ae78d 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -121,15 +121,10 @@ jest.mock('@metamask/swappable-obj-proxy'); const setup = ({ getSubjectNames = [], state, - useRequestQueuePreference = false, domainProxyMap = new Map(), }: { state?: SelectedNetworkControllerState; getSubjectNames?: string[]; - useRequestQueuePreference?: boolean; - onPreferencesStateChange?: ( - listener: (preferencesState: { useRequestQueue: boolean }) => void, - ) => void; domainProxyMap?: Map; } = {}) => { const mockProviderProxy = { @@ -173,34 +168,18 @@ const setup = ({ getSubjectNames, }); - const preferencesStateChangeListeners: ((state: { - useRequestQueue: boolean; - }) => void)[] = []; const controller = new SelectedNetworkController({ messenger: restrictedMessenger, state, - useRequestQueuePreference, - onPreferencesStateChange: (listener) => { - preferencesStateChangeListeners.push(listener); - }, domainProxyMap, }); - const triggerPreferencesStateChange = (preferencesState: { - useRequestQueue: boolean; - }) => { - for (const listener of preferencesStateChangeListeners) { - listener(preferencesState); - } - }; - return { controller, messenger, mockProviderProxy, mockBlockTrackerProxy, domainProxyMap, - triggerPreferencesStateChange, createEventEmitterProxyMock, ...mockMessengerActions, }; @@ -226,296 +205,232 @@ describe('SelectedNetworkController', () => { }); }); - describe('when useRequestQueuePreference is true', () => { - it('should set networkClientId for domains not already in state', async () => { - const { controller } = setup({ - state: { - domains: { - 'existingdomain.com': 'initialNetworkId', - }, + it('should set networkClientId for domains not already in state', async () => { + const { controller } = setup({ + state: { + domains: { + 'existingdomain.com': 'initialNetworkId', }, - getSubjectNames: ['newdomain.com'], - useRequestQueuePreference: true, - }); - - expect(controller.state.domains).toStrictEqual({ - 'newdomain.com': 'mainnet', - 'existingdomain.com': 'initialNetworkId', - }); + }, + getSubjectNames: ['newdomain.com'], }); - it('should not modify domains already in state', async () => { - const { controller } = setup({ - state: { - domains: { - 'existingdomain.com': 'initialNetworkId', - }, - }, - getSubjectNames: ['existingdomain.com'], - useRequestQueuePreference: true, - }); - - expect(controller.state.domains).toStrictEqual({ - 'existingdomain.com': 'initialNetworkId', - }); + expect(controller.state.domains).toStrictEqual({ + 'newdomain.com': 'mainnet', + 'existingdomain.com': 'initialNetworkId', }); }); - describe('when useRequestQueuePreference is false', () => { - it('should not set networkClientId for new domains', async () => { - const { controller } = setup({ - state: { - domains: { - 'existingdomain.com': 'initialNetworkId', - }, + it('should not modify domains already in state', async () => { + const { controller } = setup({ + state: { + domains: { + 'existingdomain.com': 'initialNetworkId', }, - getSubjectNames: ['newdomain.com'], - }); - - expect(controller.state.domains).toStrictEqual({ - 'existingdomain.com': 'initialNetworkId', - }); + }, + getSubjectNames: ['existingdomain.com'], }); - it('should not modify domains already in state', async () => { - const { controller } = setup({ - state: { - domains: { - 'existingdomain.com': 'initialNetworkId', - }, - }, - getSubjectNames: ['existingdomain.com'], - }); - - expect(controller.state.domains).toStrictEqual({ - 'existingdomain.com': 'initialNetworkId', - }); + expect(controller.state.domains).toStrictEqual({ + 'existingdomain.com': 'initialNetworkId', }); }); - }); - - describe('NetworkController:stateChange', () => { - describe('when a network is deleted from the network controller', () => { - const initialDomains = { - 'not-deleted-network.com': 'linea-mainnet', - 'deleted-network.com': 'goerli', - }; - - const deleteNetwork = ( - chainId: Hex, - networkControllerState: NetworkState, - messenger: ReturnType, - mockNetworkControllerGetState: jest.Mock, - ) => { - delete networkControllerState.networkConfigurationsByChainId[chainId]; - mockNetworkControllerGetState.mockReturnValueOnce( - networkControllerState, - ); - messenger.publish( - 'NetworkController:stateChange', - networkControllerState, - [ - { - op: 'remove', - path: ['networkConfigurationsByChainId', chainId], - }, - ], - ); - }; - - it('does not update state when useRequestQueuePreference is false', () => { - const { controller, messenger, mockNetworkControllerGetState } = setup({ - state: { domains: initialDomains }, - useRequestQueuePreference: false, - }); - - const networkControllerState = getDefaultNetworkControllerState(); - deleteNetwork( - '0x5', - networkControllerState, - messenger, - mockNetworkControllerGetState, - ); - expect(controller.state.domains).toStrictEqual(initialDomains); - }); - - it('redirects domains to the globally selected network when useRequestQueuePreference is true', () => { - const { controller, messenger, mockNetworkControllerGetState } = setup({ - state: { domains: initialDomains }, - useRequestQueuePreference: true, - }); + describe('NetworkController:stateChange', () => { + describe('when a network is deleted from the network controller', () => { + const initialDomains = { + 'not-deleted-network.com': 'linea-mainnet', + 'deleted-network.com': 'goerli', + }; - const networkControllerState = { - ...getDefaultNetworkControllerState(), - selectedNetworkClientId: 'mainnet', + const deleteNetwork = ( + chainId: Hex, + networkControllerState: NetworkState, + messenger: ReturnType, + mockNetworkControllerGetState: jest.Mock, + ) => { + delete networkControllerState.networkConfigurationsByChainId[chainId]; + mockNetworkControllerGetState.mockReturnValueOnce( + networkControllerState, + ); + messenger.publish( + 'NetworkController:stateChange', + networkControllerState, + [ + { + op: 'remove', + path: ['networkConfigurationsByChainId', chainId], + }, + ], + ); }; - deleteNetwork( - '0x5', - networkControllerState, - messenger, - mockNetworkControllerGetState, - ); + it('redirects domains to the globally selected network', () => { + const { controller, messenger, mockNetworkControllerGetState } = + setup({ + state: { domains: initialDomains }, + }); - expect(controller.state.domains).toStrictEqual({ - ...initialDomains, - 'deleted-network.com': networkControllerState.selectedNetworkClientId, - }); - }); + const networkControllerState = { + ...getDefaultNetworkControllerState(), + selectedNetworkClientId: 'mainnet', + }; - it('redirects domains to the globally selected network when useRequestQueuePreference is true and handles garbage collected proxies', () => { - const domainProxyMap = new Map(); - const { - controller, - messenger, - mockNetworkControllerGetState, - mockGetNetworkClientById, - } = setup({ - state: { domains: initialDomains }, - useRequestQueuePreference: true, - domainProxyMap, + deleteNetwork( + '0x5', + networkControllerState, + messenger, + mockNetworkControllerGetState, + ); + + expect(controller.state.domains).toStrictEqual({ + ...initialDomains, + 'deleted-network.com': + networkControllerState.selectedNetworkClientId, + }); }); - // Simulate proxies being garbage collected - domainProxyMap.clear(); + it('redirects domains to the globally selected network and handles garbage collected proxies', () => { + const domainProxyMap = new Map(); + const { + controller, + messenger, + mockNetworkControllerGetState, + mockGetNetworkClientById, + } = setup({ + state: { domains: initialDomains }, - const networkControllerState = { - ...getDefaultNetworkControllerState(), - selectedNetworkClientId: 'mainnet', - }; + domainProxyMap, + }); - mockGetNetworkClientById.mockImplementation((id) => { - // Simulate the previous domain being deleted in NetworkController - if (id !== 'mainnet') { - throw new Error('Network client does not exist'); - } + // Simulate proxies being garbage collected + domainProxyMap.clear(); - return { - provider: { request: jest.fn() }, - blockTracker: { getLatestBlock: jest.fn() }, + const networkControllerState = { + ...getDefaultNetworkControllerState(), + selectedNetworkClientId: 'mainnet', }; - }); - deleteNetwork( - '0x5', - networkControllerState, - messenger, - mockNetworkControllerGetState, - ); + mockGetNetworkClientById.mockImplementation((id) => { + // Simulate the previous domain being deleted in NetworkController + if (id !== 'mainnet') { + throw new Error('Network client does not exist'); + } - expect(controller.state.domains).toStrictEqual({ - ...initialDomains, - 'deleted-network.com': networkControllerState.selectedNetworkClientId, - }); - }); - }); + return { + provider: { request: jest.fn() }, + blockTracker: { getLatestBlock: jest.fn() }, + }; + }); - describe('when a network is updated', () => { - it('redirects domains when the default rpc endpoint is switched', () => { - const initialDomains = { - 'different-chain.com': 'mainnet', - 'chain-with-new-default.com': 'goerli', - }; + deleteNetwork( + '0x5', + networkControllerState, + messenger, + mockNetworkControllerGetState, + ); - const { controller, messenger, mockNetworkControllerGetState } = setup({ - state: { domains: initialDomains }, - useRequestQueuePreference: true, + expect(controller.state.domains).toStrictEqual({ + ...initialDomains, + 'deleted-network.com': + networkControllerState.selectedNetworkClientId, + }); }); + }); - const networkControllerState = getDefaultNetworkControllerState(); - const goerliNetwork = - networkControllerState.networkConfigurationsByChainId['0x5']; + describe('when a network is updated', () => { + it('redirects domains when the default rpc endpoint is switched', () => { + const initialDomains = { + 'different-chain.com': 'mainnet', + 'chain-with-new-default.com': 'goerli', + }; - goerliNetwork.defaultRpcEndpointIndex = - goerliNetwork.rpcEndpoints.push({ - type: RpcEndpointType.Custom, - url: 'https://new-default.com', - networkClientId: 'new-default-network-client-id', - }) - 1; + const { controller, messenger, mockNetworkControllerGetState } = + setup({ + state: { domains: initialDomains }, + }); - mockNetworkControllerGetState.mockReturnValueOnce( - networkControllerState, - ); + const networkControllerState = getDefaultNetworkControllerState(); + const goerliNetwork = + networkControllerState.networkConfigurationsByChainId['0x5']; - messenger.publish( - 'NetworkController:stateChange', - networkControllerState, - [ - { - op: 'replace', - path: ['networkConfigurationsByChainId', '0x5'], - }, - ], - ); + goerliNetwork.defaultRpcEndpointIndex = + goerliNetwork.rpcEndpoints.push({ + type: RpcEndpointType.Custom, + url: 'https://new-default.com', + networkClientId: 'new-default-network-client-id', + }) - 1; - expect(controller.state.domains).toStrictEqual({ - ...initialDomains, - 'chain-with-new-default.com': 'new-default-network-client-id', - }); - }); + mockNetworkControllerGetState.mockReturnValueOnce( + networkControllerState, + ); - it('redirects domains when the default rpc endpoint is deleted and replaced', () => { - const initialDomains = { - 'different-chain.com': 'mainnet', - 'chain-with-new-default.com': 'goerli', - }; + messenger.publish( + 'NetworkController:stateChange', + networkControllerState, + [ + { + op: 'replace', + path: ['networkConfigurationsByChainId', '0x5'], + }, + ], + ); - const { controller, messenger, mockNetworkControllerGetState } = setup({ - state: { domains: initialDomains }, - useRequestQueuePreference: true, + expect(controller.state.domains).toStrictEqual({ + ...initialDomains, + 'chain-with-new-default.com': 'new-default-network-client-id', + }); }); - const networkControllerState = getDefaultNetworkControllerState(); - const goerliNetwork = - networkControllerState.networkConfigurationsByChainId['0x5']; + it('redirects domains when the default rpc endpoint is deleted and replaced', () => { + const initialDomains = { + 'different-chain.com': 'mainnet', + 'chain-with-new-default.com': 'goerli', + }; - goerliNetwork.rpcEndpoints = [ - { - type: RpcEndpointType.Custom, - url: 'https://new-default.com', - networkClientId: 'new-default-network-client-id', - }, - ]; + const { controller, messenger, mockNetworkControllerGetState } = + setup({ + state: { domains: initialDomains }, + }); - mockNetworkControllerGetState.mockReturnValueOnce( - networkControllerState, - ); + const networkControllerState = getDefaultNetworkControllerState(); + const goerliNetwork = + networkControllerState.networkConfigurationsByChainId['0x5']; - messenger.publish( - 'NetworkController:stateChange', - networkControllerState, - [ + goerliNetwork.rpcEndpoints = [ { - op: 'replace', - path: ['networkConfigurationsByChainId', '0x5'], + type: RpcEndpointType.Custom, + url: 'https://new-default.com', + networkClientId: 'new-default-network-client-id', }, - ], - ); + ]; - expect(controller.state.domains).toStrictEqual({ - ...initialDomains, - 'chain-with-new-default.com': 'new-default-network-client-id', - }); - }); - }); - }); + mockNetworkControllerGetState.mockReturnValueOnce( + networkControllerState, + ); - describe('setNetworkClientIdForDomain', () => { - it('does not update state when the useRequestQueuePreference is false', () => { - const { controller } = setup({ - state: { - domains: {}, - }, - }); + messenger.publish( + 'NetworkController:stateChange', + networkControllerState, + [ + { + op: 'replace', + path: ['networkConfigurationsByChainId', '0x5'], + }, + ], + ); - controller.setNetworkClientIdForDomain('1.com', '1'); - expect(controller.state.domains).toStrictEqual({}); + expect(controller.state.domains).toStrictEqual({ + ...initialDomains, + 'chain-with-new-default.com': 'new-default-network-client-id', + }); + }); + }); }); - describe('when useRequestQueuePreference is true', () => { + describe('setNetworkClientIdForDomain', () => { it('should throw an error when passed "metamask" as domain arg', () => { - const { controller } = setup({ useRequestQueuePreference: true }); + const { controller } = setup(); expect(() => { controller.setNetworkClientIdForDomain('metamask', 'mainnet'); }).toThrow( @@ -528,7 +443,6 @@ describe('SelectedNetworkController', () => { it('skips setting the networkClientId for the passed in domain', () => { const { controller, mockHasPermissions } = setup({ state: { domains: {} }, - useRequestQueuePreference: true, }); mockHasPermissions.mockReturnValue(true); const snapDomainOne = 'npm:@metamask/bip32-example-snap'; @@ -559,7 +473,6 @@ describe('SelectedNetworkController', () => { it('sets the networkClientId for the passed in domain', () => { const { controller, mockHasPermissions } = setup({ state: { domains: {} }, - useRequestQueuePreference: true, }); mockHasPermissions.mockReturnValue(true); const domain = 'example.com'; @@ -571,7 +484,6 @@ describe('SelectedNetworkController', () => { it('updates the provider and block tracker proxy when they already exist for the domain', () => { const { controller, mockProviderProxy, mockHasPermissions } = setup({ state: { domains: {} }, - useRequestQueuePreference: true, }); mockHasPermissions.mockReturnValue(true); const initialNetworkClientId = '123'; @@ -603,7 +515,6 @@ describe('SelectedNetworkController', () => { it('throws an error and does not set the networkClientId for the passed in domain', () => { const { controller, mockHasPermissions } = setup({ state: { domains: {} }, - useRequestQueuePreference: true, }); mockHasPermissions.mockReturnValue(false); @@ -618,17 +529,8 @@ describe('SelectedNetworkController', () => { }); }); }); - }); - - describe('getNetworkClientIdForDomain', () => { - it('returns the selectedNetworkClientId from the NetworkController when useRequestQueuePreference is false', () => { - const { controller } = setup(); - expect(controller.getNetworkClientIdForDomain('example.com')).toBe( - 'mainnet', - ); - }); - describe('when useRequestQueuePreference is true', () => { + describe('getNetworkClientIdForDomain', () => { it('returns the networkClientId from state when a networkClientId has been set for the requested domain', () => { const { controller } = setup({ state: { @@ -636,7 +538,6 @@ describe('SelectedNetworkController', () => { 'example.com': '1', }, }, - useRequestQueuePreference: true, }); const result = controller.getNetworkClientIdForDomain('example.com'); @@ -646,480 +547,286 @@ describe('SelectedNetworkController', () => { it('returns the selectedNetworkClientId from the NetworkController when no networkClientId has been set for the requested domain', () => { const { controller } = setup({ state: { domains: {} }, - useRequestQueuePreference: true, }); expect(controller.getNetworkClientIdForDomain('example.com')).toBe( 'mainnet', ); }); }); - }); - - describe('getProviderAndBlockTracker', () => { - it('returns the cached proxy provider and block tracker when the domain already has a cached networkProxy in the domainProxyMap', () => { - const mockProxyProvider = { - setTarget: jest.fn(), - } as unknown as ProviderProxy; - const mockProxyBlockTracker = { - setTarget: jest.fn(), - } as unknown as BlockTrackerProxy; - - const domainProxyMap = new Map([ - [ - 'example.com', - { - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, - }, - ], - [ - 'test.com', - { - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, - }, - ], - ]); - const { controller } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: true, - domainProxyMap, - }); - - const result = controller.getProviderAndBlockTracker('example.com'); - expect(result).toStrictEqual({ - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, - }); - }); - - describe('when the domain does not have a cached networkProxy in the domainProxyMap and useRequestQueuePreference is true', () => { - describe('when the domain has permissions', () => { - it('calls to NetworkController:getNetworkClientById and creates a new proxy provider and block tracker with the non-proxied globally selected network client', () => { - const { controller, messenger, mockHasPermissions } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: true, - }); - jest.spyOn(messenger, 'call'); - mockHasPermissions.mockReturnValue(true); - const result = controller.getProviderAndBlockTracker('example.com'); - expect(result).toBeDefined(); - // unfortunately checking which networkController method is called is the best - // proxy (no pun intended) for checking that the correct instance of the networkClient is used - expect(messenger.call).toHaveBeenCalledWith( - 'NetworkController:getNetworkClientById', - 'mainnet', - ); - }); - }); + describe('getProviderAndBlockTracker', () => { + it('returns the cached proxy provider and block tracker when the domain already has a cached networkProxy in the domainProxyMap', () => { + const mockProxyProvider = { + setTarget: jest.fn(), + } as unknown as ProviderProxy; + const mockProxyBlockTracker = { + setTarget: jest.fn(), + } as unknown as BlockTrackerProxy; - describe('when the domain does not have permissions', () => { - it('calls to NetworkController:getSelectedNetworkClient and creates a new proxy provider and block tracker with the proxied globally selected network client', () => { - const { controller, messenger, mockHasPermissions } = setup({ - state: { - domains: {}, + const domainProxyMap = new Map([ + [ + 'example.com', + { + provider: mockProxyProvider, + blockTracker: mockProxyBlockTracker, }, - useRequestQueuePreference: true, - }); - jest.spyOn(messenger, 'call'); - mockHasPermissions.mockReturnValue(false); - const result = controller.getProviderAndBlockTracker('example.com'); - expect(result).toBeDefined(); - // unfortunately checking which networkController method is called is the best - // proxy (no pun intended) for checking that the correct instance of the networkClient is used - expect(messenger.call).toHaveBeenCalledWith( - 'NetworkController:getSelectedNetworkClient', - ); - }); - - it('throws an error if the globally selected network client is not initialized', () => { - const { controller, mockGetSelectedNetworkClient } = setup({ - state: { - domains: {}, + ], + [ + 'test.com', + { + provider: mockProxyProvider, + blockTracker: mockProxyBlockTracker, }, - useRequestQueuePreference: false, - }); - mockGetSelectedNetworkClient.mockReturnValue(undefined); - expect(() => - controller.getProviderAndBlockTracker('example.com'), - ).toThrow('Selected network not initialized'); - }); - }); - }); - - describe('when the domain does not have a cached networkProxy in the domainProxyMap and useRequestQueuePreference is false', () => { - it('calls to NetworkController:getSelectedNetworkClient and creates a new proxy provider and block tracker with the proxied globally selected network client', () => { - const { controller, messenger } = setup({ + ], + ]); + const { controller } = setup({ state: { domains: {}, }, - useRequestQueuePreference: false, + + domainProxyMap, }); - jest.spyOn(messenger, 'call'); const result = controller.getProviderAndBlockTracker('example.com'); - expect(result).toBeDefined(); - // unfortunately checking which networkController method is called is the best - // proxy (no pun intended) for checking that the correct instance of the networkClient is used - expect(messenger.call).toHaveBeenCalledWith( - 'NetworkController:getSelectedNetworkClient', - ); - }); - }); - - // TODO - improve these tests by using a full NetworkController and doing more robust behavioral testing - describe('when the domain is a snap (starts with "npm:" or "local:")', () => { - it('returns a proxied globally selected networkClient and does not create a new proxy in the domainProxyMap', () => { - const { controller, domainProxyMap, messenger } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: true, + expect(result).toStrictEqual({ + provider: mockProxyProvider, + blockTracker: mockProxyBlockTracker, }); - jest.spyOn(messenger, 'call'); - const snapDomain = 'npm:@metamask/bip32-example-snap'; - - const result = controller.getProviderAndBlockTracker(snapDomain); - - expect(domainProxyMap.get(snapDomain)).toBeUndefined(); - expect(messenger.call).toHaveBeenCalledWith( - 'NetworkController:getSelectedNetworkClient', - ); - expect(result).toBeDefined(); }); - it('throws an error if the globally selected network client is not initialized', () => { - const { controller, mockGetSelectedNetworkClient } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: false, - }); - const snapDomain = 'npm:@metamask/bip32-example-snap'; + it('throws an error if passed a domain that does not have permissions and the globally selected network client is not initialized', () => { + const { controller, mockGetSelectedNetworkClient, mockHasPermissions } = + setup(); mockGetSelectedNetworkClient.mockReturnValue(undefined); - - expect(() => controller.getProviderAndBlockTracker(snapDomain)).toThrow( + mockHasPermissions.mockReturnValue(false); + expect(() => controller.getProviderAndBlockTracker('test.com')).toThrow( 'Selected network not initialized', ); }); - }); - - describe('when the domain is a "metamask"', () => { - it('returns a proxied globally selected networkClient and does not create a new proxy in the domainProxyMap', () => { - const { controller, domainProxyMap, messenger } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: true, - }); - jest.spyOn(messenger, 'call'); - const result = controller.getProviderAndBlockTracker(METAMASK_DOMAIN); - - expect(result).toBeDefined(); - expect(domainProxyMap.get(METAMASK_DOMAIN)).toBeUndefined(); - expect(messenger.call).toHaveBeenCalledWith( - 'NetworkController:getSelectedNetworkClient', + it('throws and error if passed a domain that has permissions and the globally selected network client is not initialized', () => { + const { controller, mockGetNetworkClientById, mockHasPermissions } = + setup(); + mockGetNetworkClientById.mockReturnValue(undefined); + mockHasPermissions.mockReturnValue(true); + expect(() => controller.getProviderAndBlockTracker('test.com')).toThrow( + 'Selected network not initialized', ); }); - it('throws an error if the globally selected network client is not initialized', () => { - const { controller, mockGetSelectedNetworkClient } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: false, + describe('when the domain does not have a cached networkProxy in the domainProxyMap', () => { + describe('when the domain has permissions', () => { + it('calls to NetworkController:getNetworkClientById and creates a new proxy provider and block tracker with the non-proxied globally selected network client', () => { + const { controller, messenger, mockHasPermissions } = setup({ + state: { + domains: {}, + }, + }); + jest.spyOn(messenger, 'call'); + mockHasPermissions.mockReturnValue(true); + + const result = controller.getProviderAndBlockTracker('example.com'); + expect(result).toBeDefined(); + // unfortunately checking which networkController method is called is the best + // proxy (no pun intended) for checking that the correct instance of the networkClient is used + expect(messenger.call).toHaveBeenCalledWith( + 'NetworkController:getNetworkClientById', + 'mainnet', + ); + }); }); - mockGetSelectedNetworkClient.mockReturnValue(undefined); - - expect(() => - controller.getProviderAndBlockTracker(METAMASK_DOMAIN), - ).toThrow('Selected network not initialized'); - }); - }); - }); - describe('PermissionController:stateChange', () => { - describe('on permission add', () => { - it('should add new domain to domains list when useRequestQueuePreference is true', async () => { - const { controller, messenger } = setup({ - useRequestQueuePreference: true, + describe('when the domain does not have permissions', () => { + it('calls to NetworkController:getSelectedNetworkClient and creates a new proxy provider and block tracker with the proxied globally selected network client', () => { + const { controller, messenger, mockHasPermissions } = setup({ + state: { + domains: {}, + }, + }); + jest.spyOn(messenger, 'call'); + mockHasPermissions.mockReturnValue(false); + const result = controller.getProviderAndBlockTracker('example.com'); + expect(result).toBeDefined(); + // unfortunately checking which networkController method is called is the best + // proxy (no pun intended) for checking that the correct instance of the networkClient is used + expect(messenger.call).toHaveBeenCalledWith( + 'NetworkController:getSelectedNetworkClient', + ); + }); }); - const mockPermission = { - parentCapability: 'eth_accounts', - id: 'example.com', - date: Date.now(), - caveats: [{ type: 'restrictToAccounts', value: ['0x...'] }], - }; - - messenger.publish( - 'PermissionController:stateChange', - { subjects: {} }, - [ - { - op: 'add', - path: ['subjects', 'example.com', 'permissions'], - value: mockPermission, - }, - ], - ); - - const { domains } = controller.state; - expect(domains['example.com']).toBeDefined(); }); - it('should not add new domain to domains list when useRequestQueuePreference is false', async () => { - const { controller, messenger } = setup({}); - const mockPermission = { - parentCapability: 'eth_accounts', - id: 'example.com', - date: Date.now(), - caveats: [{ type: 'restrictToAccounts', value: ['0x...'] }], - }; - - messenger.publish( - 'PermissionController:stateChange', - { subjects: {} }, - [ - { - op: 'add', - path: ['subjects', 'example.com', 'permissions'], - value: mockPermission, + // TODO - improve these tests by using a full NetworkController and doing more robust behavioral testing + describe('when the domain is a snap (starts with "npm:" or "local:")', () => { + it('returns a proxied globally selected networkClient and does not create a new proxy in the domainProxyMap', () => { + const { controller, domainProxyMap, messenger } = setup({ + state: { + domains: {}, }, - ], - ); + }); + jest.spyOn(messenger, 'call'); + const snapDomain = 'npm:@metamask/bip32-example-snap'; - const { domains } = controller.state; - expect(domains['example.com']).toBeUndefined(); - }); - }); + const result = controller.getProviderAndBlockTracker(snapDomain); - describe('on permission removal', () => { - it('should remove domain from domains list', async () => { - const { controller, messenger } = setup({ - state: { domains: { 'example.com': 'foo' } }, + expect(domainProxyMap.get(snapDomain)).toBeUndefined(); + expect(messenger.call).toHaveBeenCalledWith( + 'NetworkController:getSelectedNetworkClient', + ); + expect(result).toBeDefined(); }); - messenger.publish( - 'PermissionController:stateChange', - { subjects: {} }, - [ - { - op: 'remove', - path: ['subjects', 'example.com', 'permissions'], + it('throws an error if the globally selected network client is not initialized', () => { + const { controller, mockGetSelectedNetworkClient } = setup({ + state: { + domains: {}, }, - ], - ); - - const { domains } = controller.state; - expect(domains['example.com']).toBeUndefined(); - }); + }); + const snapDomain = 'npm:@metamask/bip32-example-snap'; + mockGetSelectedNetworkClient.mockReturnValue(undefined); - it('should set the proxy to the globally selected network if the globally selected network client is initialized and a proxy exists for the domain', async () => { - const { controller, messenger, mockProviderProxy } = setup({ - state: { domains: { 'example.com': 'foo' } }, + expect(() => + controller.getProviderAndBlockTracker(snapDomain), + ).toThrow('Selected network not initialized'); }); - controller.getProviderAndBlockTracker('example.com'); + }); - messenger.publish( - 'PermissionController:stateChange', - { subjects: {} }, - [ - { - op: 'remove', - path: ['subjects', 'example.com', 'permissions'], + describe('when the domain is a "metamask"', () => { + it('returns a proxied globally selected networkClient and does not create a new proxy in the domainProxyMap', () => { + const { controller, domainProxyMap, messenger } = setup({ + state: { + domains: {}, }, - ], - ); - - expect(mockProviderProxy.setTarget).toHaveBeenCalledWith( - expect.objectContaining({ request: expect.any(Function) }), - ); - expect(mockProviderProxy.setTarget).toHaveBeenCalledTimes(1); + }); + jest.spyOn(messenger, 'call'); - const { domains } = controller.state; - expect(domains['example.com']).toBeUndefined(); - }); + const result = controller.getProviderAndBlockTracker(METAMASK_DOMAIN); - it('should delete the proxy if the globally selected network client is not initialized but a proxy exists for the domain', async () => { - const { - controller, - messenger, - domainProxyMap, - mockProviderProxy, - mockGetSelectedNetworkClient, - } = setup({ - state: { domains: { 'example.com': 'foo' } }, + expect(result).toBeDefined(); + expect(domainProxyMap.get(METAMASK_DOMAIN)).toBeUndefined(); + expect(messenger.call).toHaveBeenCalledWith( + 'NetworkController:getSelectedNetworkClient', + ); }); - controller.getProviderAndBlockTracker('example.com'); - mockGetSelectedNetworkClient.mockReturnValue(undefined); - expect(domainProxyMap.get('example.com')).toBeDefined(); - messenger.publish( - 'PermissionController:stateChange', - { subjects: {} }, - [ - { - op: 'remove', - path: ['subjects', 'example.com', 'permissions'], + it('throws an error if the globally selected network client is not initialized', () => { + const { controller, mockGetSelectedNetworkClient } = setup({ + state: { + domains: {}, }, - ], - ); + }); + mockGetSelectedNetworkClient.mockReturnValue(undefined); - expect(mockProviderProxy.setTarget).toHaveBeenCalledTimes(0); - expect(domainProxyMap.get('example.com')).toBeUndefined(); + expect(() => + controller.getProviderAndBlockTracker(METAMASK_DOMAIN), + ).toThrow('Selected network not initialized'); + }); }); }); - }); - // because of the opacity of the networkClient and proxy implementations, - // its impossible to make valuable assertions around which networkClient proxies - // should be targeted when the useRequestQueuePreference state is toggled on and off: - // When toggled on, the networkClient for the globally selected networkClientId should be used - **not** the NetworkController's proxy of this networkClient. - // When toggled off, the NetworkControllers proxy of the globally selected networkClient should be used - // TODO - improve these tests by using a full NetworkController and doing more robust behavioral testing - describe('onPreferencesStateChange', () => { - const mockProxyProvider = { - setTarget: jest.fn(), - } as unknown as ProviderProxy; - const mockProxyBlockTracker = { - setTarget: jest.fn(), - } as unknown as BlockTrackerProxy; - - describe('when toggled from off to on', () => { - describe('when domains have permissions', () => { - it('sets the target of the existing proxies to the non-proxied networkClient for the globally selected networkClientId', () => { - const domainProxyMap = new Map([ - [ - 'example.com', - { - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, - }, - ], + describe('PermissionController:stateChange', () => { + describe('on permission add', () => { + it('should add new domain to domains list', async () => { + const { controller, messenger } = setup({}); + const mockPermission = { + parentCapability: 'eth_accounts', + id: 'example.com', + date: Date.now(), + caveats: [{ type: 'restrictToAccounts', value: ['0x...'] }], + }; + + messenger.publish( + 'PermissionController:stateChange', + { subjects: {} }, [ - 'test.com', { - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, + op: 'add', + path: ['subjects', 'example.com', 'permissions'], + value: mockPermission, }, ], - ]); - - const { - mockHasPermissions, - triggerPreferencesStateChange, - messenger, - } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: false, - domainProxyMap, - }); - jest.spyOn(messenger, 'call'); - - mockHasPermissions.mockReturnValue(true); - - triggerPreferencesStateChange({ useRequestQueue: true }); - - // this is a very imperfect way to test this, but networkClients and proxies are opaque - // when the proxy is set with the networkClient fetched via NetworkController:getNetworkClientById - // it **is not** tied to the NetworkController's own proxy of the networkClient - expect(messenger.call).toHaveBeenCalledWith( - 'NetworkController:getNetworkClientById', - 'mainnet', ); - expect(mockProxyProvider.setTarget).toHaveBeenCalledTimes(2); - expect(mockProxyBlockTracker.setTarget).toHaveBeenCalledTimes(2); + + const { domains } = controller.state; + expect(domains['example.com']).toBeDefined(); }); }); - describe('when domains do not have permissions', () => { - it('does not change the target of the existing proxy', () => { - const domainProxyMap = new Map([ + describe('on permission removal', () => { + it('should remove domain from domains list', async () => { + const { controller, messenger } = setup({ + state: { domains: { 'example.com': 'foo' } }, + }); + + messenger.publish( + 'PermissionController:stateChange', + { subjects: {} }, [ - 'example.com', { - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, + op: 'remove', + path: ['subjects', 'example.com', 'permissions'], }, ], + ); + + const { domains } = controller.state; + expect(domains['example.com']).toBeUndefined(); + }); + + it('should set the proxy to the globally selected network if the globally selected network client is initialized and a proxy exists for the domain', async () => { + const { controller, messenger, mockProviderProxy } = setup({ + state: { domains: { 'example.com': 'foo' } }, + }); + controller.getProviderAndBlockTracker('example.com'); + + messenger.publish( + 'PermissionController:stateChange', + { subjects: {} }, [ - 'test.com', { - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, + op: 'remove', + path: ['subjects', 'example.com', 'permissions'], }, ], - ]); - const { mockHasPermissions, triggerPreferencesStateChange } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: false, - domainProxyMap, - }); - - mockHasPermissions.mockReturnValue(false); + ); - triggerPreferencesStateChange({ useRequestQueue: true }); + expect(mockProviderProxy.setTarget).toHaveBeenCalledWith( + expect.objectContaining({ request: expect.any(Function) }), + ); + expect(mockProviderProxy.setTarget).toHaveBeenCalledTimes(1); - expect(mockProxyProvider.setTarget).toHaveBeenCalledTimes(0); - expect(mockProxyBlockTracker.setTarget).toHaveBeenCalledTimes(0); + const { domains } = controller.state; + expect(domains['example.com']).toBeUndefined(); }); - }); - }); - describe('when toggled from on to off', () => { - it('sets the target of the existing proxies to the proxied globally selected networkClient', () => { - const domainProxyMap = new Map([ - [ - 'example.com', - { - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, - }, - ], - [ - 'test.com', - { - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, - }, - ], - ]); - - const { mockHasPermissions, triggerPreferencesStateChange, messenger } = - setup({ - state: { - domains: { - 'example.com': 'foo', - 'test.com': 'bar', - }, - }, - useRequestQueuePreference: true, + it('should delete the proxy if the globally selected network client is not initialized but a proxy exists for the domain', async () => { + const { + controller, + messenger, domainProxyMap, + mockProviderProxy, + mockGetSelectedNetworkClient, + } = setup({ + state: { domains: { 'example.com': 'foo' } }, }); - jest.spyOn(messenger, 'call'); - - mockHasPermissions.mockReturnValue(true); + controller.getProviderAndBlockTracker('example.com'); - triggerPreferencesStateChange({ useRequestQueue: false }); + mockGetSelectedNetworkClient.mockReturnValue(undefined); + expect(domainProxyMap.get('example.com')).toBeDefined(); + messenger.publish( + 'PermissionController:stateChange', + { subjects: {} }, + [ + { + op: 'remove', + path: ['subjects', 'example.com', 'permissions'], + }, + ], + ); - // this is a very imperfect way to test this, but networkClients and proxies are opaque - // when the proxy is set with the networkClient fetched via NetworkController:getSelectedNetworkClient - // it **is** tied to the NetworkController's own proxy of the networkClient - expect(messenger.call).toHaveBeenCalledWith( - 'NetworkController:getSelectedNetworkClient', - ); - expect(mockProxyProvider.setTarget).toHaveBeenCalledTimes(2); - expect(mockProxyBlockTracker.setTarget).toHaveBeenCalledTimes(2); + expect(mockProviderProxy.setTarget).toHaveBeenCalledTimes(0); + expect(domainProxyMap.get('example.com')).toBeUndefined(); + }); }); }); });