From 85dc7c64aad276bf621aad406613f51bbfeead93 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Tue, 11 Jun 2024 10:52:22 -0700 Subject: [PATCH 1/8] update QueuedRequestController comment --- .../queued-request-controller/src/QueuedRequestController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/queued-request-controller/src/QueuedRequestController.ts b/packages/queued-request-controller/src/QueuedRequestController.ts index fc506e22ece..62727cc5064 100644 --- a/packages/queued-request-controller/src/QueuedRequestController.ts +++ b/packages/queued-request-controller/src/QueuedRequestController.ts @@ -193,7 +193,7 @@ export class QueuedRequestController extends BaseController< ) { const origin = path[1]; this.#flushQueueForOrigin(origin); - // When a domain is removed from SelectedNetworkController, its because of revoke permissions. + // When a domain is removed from SelectedNetworkController, its because of revoke permissions or the useRequestQueue flag was toggled off. // Rather than subscribe to the permissions controller event in addition to the selectedNetworkController ones, we simplify it and just handle remove on this event alone. if (op === 'remove' && origin === this.#originOfCurrentBatch) { this.#clearPendingConfirmations(); From dd2e9198d32da880fa6a1147a7a72a3c189f2c81 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Tue, 11 Jun 2024 15:35:59 -0700 Subject: [PATCH 2/8] cleanup specs --- .../tests/SelectedNetworkController.test.ts | 399 +++++++++--------- 1 file changed, 194 insertions(+), 205 deletions(-) diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index 26d08127ce2..8714c7a1128 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -206,6 +206,7 @@ describe('SelectedNetworkController', () => { afterEach(() => { jest.clearAllMocks(); }); + describe('constructor', () => { it('can be instantiated with default values', () => { const { controller } = setup(); @@ -213,6 +214,7 @@ describe('SelectedNetworkController', () => { domains: {}, }); }); + it('can be instantiated with a state', () => { const { controller } = setup({ state: { @@ -223,20 +225,82 @@ describe('SelectedNetworkController', () => { domains: { networkClientId: 'goerli' }, }); }); + + describe('when useRequestQueuePreference is true', () => { + 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', + }); + }); + + 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', + }); + }); + }); + + describe('when useRequestQueuePreference is false', () => { + it('should not set networkClientId for new domains', async () => { + const { controller } = setup({ + state: { + domains: { + 'existingdomain.com': 'initialNetworkId', + }, + }, + getSubjectNames: ['newdomain.com'], + }); + + expect(controller.state.domains).toStrictEqual({ + 'existingdomain.com': 'initialNetworkId', + }); + }); + + 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', + }); + }); + }); }); describe('networkController:stateChange', () => { describe('when useRequestQueuePreference is false', () => { describe('when a networkClient is deleted from the network controller state', () => { - it('does not update the networkClientId for domains which were previously set to the deleted networkClientId', () => { + it('does not update state', () => { const { controller, messenger } = setup({ state: { - // normally there would not be any domains in state if useRequestQueuePreference is false - domains: { - metamask: 'goerli', - 'example.com': 'test-network-client-id', - 'test.com': 'test-network-client-id', - }, + domains: {}, }, }); @@ -255,11 +319,7 @@ describe('SelectedNetworkController', () => { }, ], ); - expect(controller.state.domains).toStrictEqual({ - metamask: 'goerli', - 'example.com': 'test-network-client-id', - 'test.com': 'test-network-client-id', - }); + expect(controller.state.domains).toStrictEqual({}); }); }); }); @@ -301,36 +361,20 @@ describe('SelectedNetworkController', () => { }); describe('setNetworkClientIdForDomain', () => { - afterEach(() => { - jest.clearAllMocks(); - }); - - describe('when the useRequestQueue is false', () => { - it('skips setting the networkClientId for the passed in domain', () => { + describe('when the useRequestQueuePreference is false', () => { + it('does not update state', () => { const { controller } = setup({ state: { - domains: { - '1.com': 'mainnet', - '2.com': 'mainnet', - '3.com': 'mainnet', - }, + domains: {}, }, }); - const domains = ['1.com', '2.com', '3.com']; - const networkClientIds = ['1', '2', '3']; - - domains.forEach((domain, i) => - controller.setNetworkClientIdForDomain(domain, networkClientIds[i]), - ); - expect(controller.state.domains).toStrictEqual({ - '1.com': 'mainnet', - '2.com': 'mainnet', - '3.com': 'mainnet', - }); + controller.setNetworkClientIdForDomain('1.com', '1'); + expect(controller.state.domains).toStrictEqual({}); }); }); - describe('when the useRequestQueue is true', () => { + + describe('when the useRequestQueuePreference is true', () => { it('should throw an error when passed "metamask" as domain arg', () => { const { controller } = setup({ useRequestQueuePreference: true }); expect(() => { @@ -340,6 +384,7 @@ describe('SelectedNetworkController', () => { ); expect(controller.state.domains.metamask).toBeUndefined(); }); + describe('when the requesting domain is a snap (starts with "npm:" or "local:"', () => { it('skips setting the networkClientId for the passed in domain', () => { const { controller, mockHasPermissions } = setup({ @@ -370,6 +415,7 @@ describe('SelectedNetworkController', () => { }); }); }); + describe('when the requesting domain has existing permissions', () => { it('sets the networkClientId for the passed in domain', () => { const { controller, mockHasPermissions } = setup({ @@ -415,7 +461,7 @@ describe('SelectedNetworkController', () => { }); describe('when the requesting domain does not have permissions', () => { - it('throw an error and does not set the networkClientId for the passed in domain', () => { + it('throws an error and does not set the networkClientId for the passed in domain', () => { const { controller, mockHasPermissions } = setup({ state: { domains: {} }, useRequestQueuePreference: true, @@ -436,46 +482,31 @@ describe('SelectedNetworkController', () => { }); describe('getNetworkClientIdForDomain', () => { - describe('when the useRequestQueue is false', () => { - it('returns the selectedNetworkClientId from the NetworkController if not no networkClientId is set for requested domain', () => { - const { controller } = setup(); - expect(controller.getNetworkClientIdForDomain('example.com')).toBe( - 'mainnet', - ); - }); - it('returns the selectedNetworkClientId from the NetworkController if a networkClientId is set for the requested domain', () => { + describe('when the useRequestQueuePreference is false', () => { + it('returns the selectedNetworkClientId from the NetworkController', () => { const { controller } = setup(); - const networkClientId = 'network3'; - controller.setNetworkClientIdForDomain('example.com', networkClientId); expect(controller.getNetworkClientIdForDomain('example.com')).toBe( 'mainnet', ); }); - it('returns the networkClientId for the metamask domain when passed "metamask"', () => { - const { controller } = setup(); - const result = controller.getNetworkClientIdForDomain('metamask'); - expect(result).toBe('mainnet'); - }); }); - describe('when the useRequestQueue is true', () => { - it('returns the networkClientId for the passed in domain, when a networkClientId has been set for the requested domain', () => { + describe('when the useRequestQueuePreference is true', () => { + it('returns the networkClientId from state when a networkClientId has been set for the requested domain', () => { const { controller } = setup({ - state: { domains: {} }, + state: { + domains: { + 'example.com': '1', + }, + }, useRequestQueuePreference: true, }); - const networkClientId1 = 'network5'; - const networkClientId2 = 'network6'; - controller.setNetworkClientIdForDomain('example.com', networkClientId1); - controller.setNetworkClientIdForDomain('test.com', networkClientId2); - const result1 = controller.getNetworkClientIdForDomain('example.com'); - const result2 = controller.getNetworkClientIdForDomain('test.com'); - expect(result1).toBe(networkClientId1); - expect(result2).toBe(networkClientId2); + const result = controller.getNetworkClientIdForDomain('example.com'); + expect(result).toBe('1'); }); - it('returns the selectedNetworkClientId from the NetworkController when no networkClientId has been set for the domain requested', () => { + it('returns the selectedNetworkClientId from the NetworkController when no networkClientId has been set for the requested domain', () => { const { controller } = setup({ state: { domains: {} }, useRequestQueuePreference: true, @@ -528,17 +559,19 @@ describe('SelectedNetworkController', () => { }); }); }); + describe('when the domain does not have a cached networkProxy in the domainProxyMap', () => { - describe('when the useRequestQueue preference is true', () => { + describe('when the useRequestQueuePreference preference 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 } = setup({ + 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(); @@ -549,19 +582,8 @@ describe('SelectedNetworkController', () => { 'mainnet', ); }); - it('throws an error if the globally selected network client is not initialized', () => { - const { controller, mockGetSelectedNetworkClient } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: false, - }); - mockGetSelectedNetworkClient.mockReturnValue(undefined); - expect(() => - controller.getProviderAndBlockTracker('example.com'), - ).toThrow('Selected network not initialized'); - }); }); + 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({ @@ -576,14 +598,27 @@ describe('SelectedNetworkController', () => { 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).not.toHaveBeenCalledWith( - 'NetworkController:getNetworkClientById', - 'mainnet', + 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: {}, + }, + useRequestQueuePreference: false, + }); + mockGetSelectedNetworkClient.mockReturnValue(undefined); + expect(() => + controller.getProviderAndBlockTracker('example.com'), + ).toThrow('Selected network not initialized'); + }); }); }); - describe('when the useRequestQueue preference is false', () => { + + describe('when the useRequestQueuePreference preference 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({ state: { @@ -603,6 +638,7 @@ describe('SelectedNetworkController', () => { }); }); }); + // 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', () => { @@ -623,7 +659,23 @@ describe('SelectedNetworkController', () => { ); 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'; + mockGetSelectedNetworkClient.mockReturnValue(undefined); + + expect(() => controller.getProviderAndBlockTracker(snapDomain)).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({ @@ -642,6 +694,7 @@ describe('SelectedNetworkController', () => { 'NetworkController:getSelectedNetworkClient', ); }); + it('throws an error if the globally selected network client is not initialized', () => { const { controller, mockGetSelectedNetworkClient } = setup({ state: { @@ -658,51 +711,63 @@ describe('SelectedNetworkController', () => { }); }); - describe('When a permission is added or removed', () => { - it('should add new domain to domains list on permission add if #useRequestQueuePreference is true', async () => { - const { controller, messenger } = setup({ - useRequestQueuePreference: true, - }); - 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, - }, - ]); + describe('PermissionController:stateChange', () => { + describe('on permission add', () => { + describe('when useRequestQueuePreference is true', () => { + it('should add new domain to domains list', async () => { + const { controller, messenger } = setup({ + useRequestQueuePreference: true, + }); + const mockPermission = { + parentCapability: 'eth_accounts', + id: 'example.com', + date: Date.now(), + caveats: [{ type: 'restrictToAccounts', value: ['0x...'] }], + }; - const { domains } = controller.state; - expect(domains['example.com']).toBeDefined(); - }); + messenger.publish( + 'PermissionController:stateChange', + { subjects: {} }, + [ + { + op: 'add', + path: ['subjects', 'example.com', 'permissions'], + value: mockPermission, + }, + ], + ); - it('should not add new domain to domains list on permission add if #useRequestQueuePreference is false', async () => { - const { controller, messenger } = setup({ - useRequestQueuePreference: false, - }); - 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(); + }); + }); + + describe('when useRequestQueuePreference is false', () => { + it('should not 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: {} }, + [ + { + op: 'add', + path: ['subjects', 'example.com', 'permissions'], + value: mockPermission, + }, + ], + ); - const { domains } = controller.state; - expect(domains['example.com']).toBeUndefined(); + const { domains } = controller.state; + expect(domains['example.com']).toBeUndefined(); + }); + }); }); describe('on permission removal', () => { @@ -783,82 +848,13 @@ describe('SelectedNetworkController', () => { }); }); - describe('Constructor checks for domains in permissions', () => { - describe('when useRequestQueuePreference is true', () => { - 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', - }); - }); - - 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', - }); - }); - }); - - describe('when useRequestQueuePreference is false', () => { - it('should not set networkClientId for new domains', async () => { - const { controller } = setup({ - state: { - domains: { - 'existingdomain.com': 'initialNetworkId', - }, - }, - getSubjectNames: ['newdomain.com'], - }); - - expect(controller.state.domains).toStrictEqual({ - 'existingdomain.com': 'initialNetworkId', - }); - }); - - 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', - }); - }); - }); - }); - // 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 useRequestQueue state is toggled on and off: + // 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('On useRequestQueue toggle state change', () => { + describe('onPreferencesStateChange', () => { const mockProxyProvider = { setTarget: jest.fn(), } as unknown as ProviderProxy; @@ -866,9 +862,6 @@ describe('SelectedNetworkController', () => { setTarget: jest.fn(), } as unknown as BlockTrackerProxy; - afterEach(() => { - jest.clearAllMocks(); - }); 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', () => { @@ -895,10 +888,7 @@ describe('SelectedNetworkController', () => { messenger, } = setup({ state: { - domains: { - 'example.com': 'foo', - 'test.com': 'bar', - }, + domains: {}, }, useRequestQueuePreference: false, domainProxyMap, @@ -920,6 +910,7 @@ describe('SelectedNetworkController', () => { expect(mockProxyBlockTracker.setTarget).toHaveBeenCalledTimes(2); }); }); + describe('when domains do not have permissions', () => { it('does not change the target of the existing proxy', () => { const domainProxyMap = new Map([ @@ -940,10 +931,7 @@ describe('SelectedNetworkController', () => { ]); const { mockHasPermissions, triggerPreferencesStateChange } = setup({ state: { - domains: { - 'example.com': 'foo', - 'test.com': 'bar', - }, + domains: {}, }, useRequestQueuePreference: false, domainProxyMap, @@ -958,6 +946,7 @@ describe('SelectedNetworkController', () => { }); }); }); + 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([ From 644d783802bc5ad56ac7f8267ac4dc7806bbc81b Mon Sep 17 00:00:00 2001 From: jiexi Date: Tue, 18 Jun 2024 10:29:35 -0700 Subject: [PATCH 3/8] Update packages/selected-network-controller/tests/SelectedNetworkController.test.ts Co-authored-by: Alex Donesky --- .../tests/SelectedNetworkController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index d7f6eaa726b..3f45c3342ac 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -372,7 +372,7 @@ describe('SelectedNetworkController', () => { }); }); - describe('when the useRequestQueuePreference is true', () => { + describe('when useRequestQueuePreference is true', () => { it('should throw an error when passed "metamask" as domain arg', () => { const { controller } = setup({ useRequestQueuePreference: true }); expect(() => { From d2e159142bf3a51dd489a98435bc196ab980084c Mon Sep 17 00:00:00 2001 From: jiexi Date: Tue, 18 Jun 2024 10:29:40 -0700 Subject: [PATCH 4/8] Update packages/selected-network-controller/tests/SelectedNetworkController.test.ts Co-authored-by: Alex Donesky --- .../tests/SelectedNetworkController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index 3f45c3342ac..342b296acea 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -480,7 +480,7 @@ describe('SelectedNetworkController', () => { }); describe('getNetworkClientIdForDomain', () => { - describe('when the useRequestQueuePreference is false', () => { + describe('when useRequestQueuePreference is false', () => { it('returns the selectedNetworkClientId from the NetworkController', () => { const { controller } = setup(); expect(controller.getNetworkClientIdForDomain('example.com')).toBe( From a431cbb44cc252c43c29f6bb6bb15813234fba83 Mon Sep 17 00:00:00 2001 From: jiexi Date: Tue, 18 Jun 2024 10:29:49 -0700 Subject: [PATCH 5/8] Update packages/selected-network-controller/tests/SelectedNetworkController.test.ts Co-authored-by: Alex Donesky --- .../tests/SelectedNetworkController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index 342b296acea..2240a046d4a 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -559,7 +559,7 @@ describe('SelectedNetworkController', () => { }); describe('when the domain does not have a cached networkProxy in the domainProxyMap', () => { - describe('when the useRequestQueuePreference preference is true', () => { + describe('when 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({ From e71f4e134ae7c1829ef90b7c7ed9cc72fae1f44c Mon Sep 17 00:00:00 2001 From: jiexi Date: Tue, 18 Jun 2024 10:29:53 -0700 Subject: [PATCH 6/8] Update packages/selected-network-controller/tests/SelectedNetworkController.test.ts Co-authored-by: Alex Donesky --- .../tests/SelectedNetworkController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index 2240a046d4a..5dec4cf0f4a 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -616,7 +616,7 @@ describe('SelectedNetworkController', () => { }); }); - describe('when the useRequestQueuePreference preference is false', () => { + describe('when 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({ state: { From cd071bb9e8b1766664482f743f65f0261fdaea64 Mon Sep 17 00:00:00 2001 From: jiexi Date: Tue, 18 Jun 2024 13:20:57 -0700 Subject: [PATCH 7/8] Update packages/selected-network-controller/tests/SelectedNetworkController.test.ts Co-authored-by: Alex Donesky --- .../tests/SelectedNetworkController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index 5dec4cf0f4a..cf0aeca636c 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -489,7 +489,7 @@ describe('SelectedNetworkController', () => { }); }); - describe('when the useRequestQueuePreference is true', () => { + describe('when useRequestQueuePreference is true', () => { it('returns the networkClientId from state when a networkClientId has been set for the requested domain', () => { const { controller } = setup({ state: { From b51a7ef7f98756eaa265d19c337cfd6b6307b6d8 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Tue, 18 Jun 2024 13:32:56 -0700 Subject: [PATCH 8/8] Flatten specs --- .../tests/SelectedNetworkController.test.ts | 400 +++++++++--------- 1 file changed, 191 insertions(+), 209 deletions(-) diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index cf0aeca636c..cbe603590f7 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -295,81 +295,73 @@ describe('SelectedNetworkController', () => { }); describe('networkController:stateChange', () => { - describe('when useRequestQueuePreference is false', () => { - describe('when a networkClient is deleted from the network controller state', () => { - it('does not update state', () => { - const { controller, messenger } = setup({ - state: { - domains: {}, - }, - }); + describe('when a networkClient is deleted from the network controller state', () => { + it('does not update state when useRequestQueuePreference is false', () => { + const { controller, messenger } = setup({ + state: { + domains: {}, + }, + }); - messenger.publish( - 'NetworkController:stateChange', + messenger.publish( + 'NetworkController:stateChange', + { + selectedNetworkClientId: 'goerli', + networkConfigurations: {}, + networksMetadata: {}, + }, + [ { - selectedNetworkClientId: 'goerli', - networkConfigurations: {}, - networksMetadata: {}, + op: 'remove', + path: ['networkConfigurations', 'test-network-client-id'], }, - [ - { - op: 'remove', - path: ['networkConfigurations', 'test-network-client-id'], - }, - ], - ); - expect(controller.state.domains).toStrictEqual({}); - }); + ], + ); + expect(controller.state.domains).toStrictEqual({}); }); - }); - describe('when useRequestQueuePreference is true', () => { - describe('when a networkClient is deleted from the network controller state', () => { - it('updates the networkClientId for domains which were previously set to the deleted networkClientId', () => { - const { controller, messenger } = setup({ - state: { - domains: { - metamask: 'goerli', - 'example.com': 'test-network-client-id', - 'test.com': 'test-network-client-id', - }, + it('updates the networkClientId for domains which were previously set to the deleted networkClientId when useRequestQueuePreference is true', () => { + const { controller, messenger } = setup({ + state: { + domains: { + metamask: 'goerli', + 'example.com': 'test-network-client-id', + 'test.com': 'test-network-client-id', }, - useRequestQueuePreference: true, - }); + }, + useRequestQueuePreference: true, + }); - messenger.publish( - 'NetworkController:stateChange', + messenger.publish( + 'NetworkController:stateChange', + { + selectedNetworkClientId: 'goerli', + networkConfigurations: {}, + networksMetadata: {}, + }, + [ { - selectedNetworkClientId: 'goerli', - networkConfigurations: {}, - networksMetadata: {}, + op: 'remove', + path: ['networkConfigurations', 'test-network-client-id'], }, - [ - { - op: 'remove', - path: ['networkConfigurations', 'test-network-client-id'], - }, - ], - ); - expect(controller.state.domains['example.com']).toBe('goerli'); - expect(controller.state.domains['test.com']).toBe('goerli'); - }); + ], + ); + expect(controller.state.domains['example.com']).toBe('goerli'); + expect(controller.state.domains['test.com']).toBe('goerli'); }); }); }); describe('setNetworkClientIdForDomain', () => { - describe('when the useRequestQueuePreference is false', () => { - it('does not update state', () => { - const { controller } = setup({ - state: { - domains: {}, - }, - }); - - controller.setNetworkClientIdForDomain('1.com', '1'); - expect(controller.state.domains).toStrictEqual({}); + it('does not update state when the useRequestQueuePreference is false', () => { + const { controller } = setup({ + state: { + domains: {}, + }, }); + + controller.setNetworkClientIdForDomain('1.com', '1'); + expect(controller.state.domains).toStrictEqual({}); }); describe('when useRequestQueuePreference is true', () => { @@ -480,13 +472,11 @@ describe('SelectedNetworkController', () => { }); describe('getNetworkClientIdForDomain', () => { - describe('when useRequestQueuePreference is false', () => { - it('returns the selectedNetworkClientId from the NetworkController', () => { - const { controller } = setup(); - expect(controller.getNetworkClientIdForDomain('example.com')).toBe( - 'mainnet', - ); - }); + 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', () => { @@ -517,115 +507,78 @@ describe('SelectedNetworkController', () => { }); describe('getProviderAndBlockTracker', () => { - describe('when the domain already has a cached networkProxy in the domainProxyMap', () => { - it('returns the cached proxy provider and block tracker', () => { - 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: {}, + 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, }, - useRequestQueuePreference: true, - domainProxyMap, - }); + ], + [ + '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, - }); + 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', () => { - describe('when 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('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: {}, - }, - 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', - ); + 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); - it('throws an error if the globally selected network client is not initialized', () => { - const { controller, mockGetSelectedNetworkClient } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: false, - }); - mockGetSelectedNetworkClient.mockReturnValue(undefined); - expect(() => - controller.getProviderAndBlockTracker('example.com'), - ).toThrow('Selected network not initialized'); - }); + 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('when useRequestQueuePreference is false', () => { + 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 } = setup({ + const { controller, messenger, mockHasPermissions } = setup({ state: { domains: {}, }, - useRequestQueuePreference: false, + 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 @@ -634,6 +587,39 @@ describe('SelectedNetworkController', () => { 'NetworkController:getSelectedNetworkClient', ); }); + + it('throws an error if the globally selected network client is not initialized', () => { + const { controller, mockGetSelectedNetworkClient } = setup({ + state: { + domains: {}, + }, + 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({ + state: { + domains: {}, + }, + useRequestQueuePreference: false, + }); + 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', + ); }); }); @@ -711,60 +697,56 @@ describe('SelectedNetworkController', () => { describe('PermissionController:stateChange', () => { describe('on permission add', () => { - describe('when useRequestQueuePreference is true', () => { - it('should add new domain to domains list', async () => { - const { controller, messenger } = setup({ - useRequestQueuePreference: true, - }); - 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 add new domain to domains list when useRequestQueuePreference is true', async () => { + const { controller, messenger } = setup({ + useRequestQueuePreference: true, }); + 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(); }); - describe('when useRequestQueuePreference is false', () => { - it('should not 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...'] }], - }; + 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, - }, - ], - ); + messenger.publish( + 'PermissionController:stateChange', + { subjects: {} }, + [ + { + op: 'add', + path: ['subjects', 'example.com', 'permissions'], + value: mockPermission, + }, + ], + ); - const { domains } = controller.state; - expect(domains['example.com']).toBeUndefined(); - }); + const { domains } = controller.state; + expect(domains['example.com']).toBeUndefined(); }); });