From 1fe28ee15a4032eef9137fe8262542f0d730256b Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 18 Apr 2023 13:25:05 -0230 Subject: [PATCH] Make `setActiveNetwork` async (#18605) The network controller method `setActiveNetwork` is now async, and the asynchronous `_setProviderConfig` step is now awaited. The function will not resolve until the network has finished switching. This change affects the `eth_switchEthereumChain` and `eth_addEthereumChain` middleware, and it affects any network switching performed in our UI. Relates to https://github.com/MetaMask/metamask-extension/issues/18587 --- .../network/network-controller.test.ts | 152 +++++------------- .../controllers/network/network-controller.ts | 4 +- 2 files changed, 39 insertions(+), 117 deletions(-) diff --git a/app/scripts/controllers/network/network-controller.test.ts b/app/scripts/controllers/network/network-controller.test.ts index 503f23a72..508d57fe7 100644 --- a/app/scripts/controllers/network/network-controller.test.ts +++ b/app/scripts/controllers/network/network-controller.test.ts @@ -1192,7 +1192,7 @@ describe('NetworkController', () => { }); expect(oldChainIdResult).toBe('0x5'); - controller.setActiveNetwork('testNetworkConfigurationId'); + await controller.setActiveNetwork('testNetworkConfigurationId'); const promisifiedSendAsync2 = promisify(provider.sendAsync).bind( provider, ); @@ -2431,15 +2431,9 @@ describe('NetworkController', () => { }, }, beforeCompleting: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkStatus'], - operation: () => { - controller.setActiveNetwork( - 'testNetworkConfigurationId', - ); - }, - }); + await controller.setActiveNetwork( + 'testNetworkConfigurationId', + ); }, }, ], @@ -2506,15 +2500,9 @@ describe('NetworkController', () => { network1.mockEssentialRpcCalls({ eth_getBlockByNumber: { beforeCompleting: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkStatus'], - operation: () => { - controller.setActiveNetwork( - 'testNetworkConfigurationId', - ); - }, - }); + await controller.setActiveNetwork( + 'testNetworkConfigurationId', + ); }, }, net_version: { @@ -2578,15 +2566,9 @@ describe('NetworkController', () => { latestBlock: POST_1559_BLOCK, eth_getBlockByNumber: { beforeCompleting: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkStatus'], - operation: () => { - controller.setActiveNetwork( - 'testNetworkConfigurationId', - ); - }, - }); + await controller.setActiveNetwork( + 'testNetworkConfigurationId', + ); }, }, }); @@ -4032,9 +4014,9 @@ describe('NetworkController', () => { async ({ controller, network }) => { network.mockEssentialRpcCalls(); - expect(() => + await expect(() => controller.setActiveNetwork('invalid-network-configuration-id'), - ).toThrow( + ).rejects.toThrow( new Error( 'networkConfigurationId invalid-network-configuration-id does not match a configured networkConfiguration', ), @@ -4075,7 +4057,7 @@ describe('NetworkController', () => { }); network.mockEssentialRpcCalls(); - controller.setActiveNetwork('testNetworkConfigurationId1'); + await controller.setActiveNetwork('testNetworkConfigurationId1'); expect(controller.store.getState().provider).toStrictEqual({ type: 'rpc', @@ -4144,6 +4126,8 @@ describe('NetworkController', () => { messenger: unrestrictedMessenger, eventType: NetworkControllerEventType.NetworkWillChange, operation: () => { + // Intentionally not awaited because we're checking state + // partway through the operation controller.setActiveNetwork('testNetworkConfigurationId2'); }, beforeResolving: () => { @@ -4208,6 +4192,8 @@ describe('NetworkController', () => { // before networkDidChange count: 1, operation: () => { + // Intentionally not awaited because we're checking state + // partway through the operation. controller.setActiveNetwork('testNetworkConfigurationId1'); }, }); @@ -4267,6 +4253,8 @@ describe('NetworkController', () => { // before networkDidChange count: 1, operation: () => { + // Intentionally not awaited because we're checking state + // partway through the operation controller.setActiveNetwork('testNetworkConfigurationId2'); }, }); @@ -4308,7 +4296,7 @@ describe('NetworkController', () => { }, }); - controller.setActiveNetwork('testNetworkConfigurationId'); + await controller.setActiveNetwork('testNetworkConfigurationId'); const { provider } = controller.getProviderAndBlockTracker(); assert(provider, 'Provider is somehow unset'); @@ -4356,7 +4344,7 @@ describe('NetworkController', () => { const { provider: providerBefore } = controller.getProviderAndBlockTracker(); - controller.setActiveNetwork('testNetworkConfigurationId'); + await controller.setActiveNetwork('testNetworkConfigurationId'); const { provider: providerAfter } = controller.getProviderAndBlockTracker(); @@ -4392,8 +4380,8 @@ describe('NetworkController', () => { const networkDidChange = await waitForPublishedEvents({ messenger: unrestrictedMessenger, eventType: NetworkControllerEventType.NetworkDidChange, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); + operation: async () => { + await controller.setActiveNetwork('testNetworkConfigurationId'); }, }); @@ -4429,8 +4417,8 @@ describe('NetworkController', () => { const infuraIsUnblocked = await waitForPublishedEvents({ messenger: unrestrictedMessenger, eventType: NetworkControllerEventType.InfuraIsUnblocked, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); + operation: async () => { + await controller.setActiveNetwork('testNetworkConfigurationId'); }, }); @@ -4462,13 +4450,7 @@ describe('NetworkController', () => { response: SUCCESSFUL_NET_VERSION_RESPONSE, }, }); - await waitForStateChanges({ - controller, - propertyPath: ['networkStatus'], - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); - }, - }); + await controller.setActiveNetwork('testNetworkConfigurationId'); expect(controller.store.getState().networkStatus).toBe('available'); }, @@ -4501,16 +4483,7 @@ describe('NetworkController', () => { latestBlock: POST_1559_BLOCK, }); - await waitForStateChanges({ - controller, - propertyPath: ['networkDetails'], - // setActiveNetwork clears networkDetails first, and then updates it - // to what we expect it to be - count: 2, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); - }, - }); + await controller.setActiveNetwork('testNetworkConfigurationId'); expect(controller.store.getState().networkDetails).toStrictEqual({ EIPS: { @@ -5657,12 +5630,7 @@ describe('NetworkController', () => { currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId2'); - }, - }); + await controller.setActiveNetwork('testNetworkConfigurationId2'); expect(controller.store.getState().provider).toStrictEqual({ type: 'rpc', rpcUrl: 'https://mock-rpc-url-2', @@ -5730,12 +5698,7 @@ describe('NetworkController', () => { }); currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); - }, - }); + await controller.setActiveNetwork('testNetworkConfigurationId'); await waitForLookupNetworkToComplete({ controller, @@ -5787,12 +5750,7 @@ describe('NetworkController', () => { currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); - }, - }); + await controller.setActiveNetwork('testNetworkConfigurationId'); expect(controller.store.getState().networkStatus).toBe( 'available', ); @@ -5855,12 +5813,7 @@ describe('NetworkController', () => { }); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); - }, - }); + await controller.setActiveNetwork('testNetworkConfigurationId'); expect(controller.store.getState().networkDetails).toStrictEqual({ EIPS: { 1559: true, @@ -5926,12 +5879,7 @@ describe('NetworkController', () => { }); currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); - }, - }); + await controller.setActiveNetwork('testNetworkConfigurationId'); await waitForLookupNetworkToComplete({ controller, @@ -5986,12 +5934,7 @@ describe('NetworkController', () => { }); currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); - }, - }); + await controller.setActiveNetwork('testNetworkConfigurationId'); const { provider: providerBefore } = controller.getProviderAndBlockTracker(); @@ -6045,12 +5988,7 @@ describe('NetworkController', () => { currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); - }, - }); + await controller.setActiveNetwork('testNetworkConfigurationId'); await waitForLookupNetworkToComplete({ controller, @@ -6103,12 +6041,7 @@ describe('NetworkController', () => { response: BLOCKED_INFURA_RESPONSE, }, }); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); - }, - }); + await controller.setActiveNetwork('testNetworkConfigurationId'); const promiseForNoInfuraIsUnblockedEvents = waitForPublishedEvents({ messenger: unrestrictedMessenger, @@ -6173,13 +6106,7 @@ describe('NetworkController', () => { }, }); - await waitForStateChanges({ - controller, - propertyPath: ['networkStatus'], - operation: () => { - controller.setActiveNetwork('currentNetworkConfiguration'); - }, - }); + await controller.setActiveNetwork('currentNetworkConfiguration'); expect(controller.store.getState().networkStatus).toBe( 'unavailable', ); @@ -6228,12 +6155,7 @@ describe('NetworkController', () => { latestBlock: POST_1559_BLOCK, }); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setActiveNetwork('testNetworkConfigurationId'); - }, - }); + await controller.setActiveNetwork('testNetworkConfigurationId'); expect(controller.store.getState().networkDetails).toStrictEqual({ EIPS: { 1559: false, diff --git a/app/scripts/controllers/network/network-controller.ts b/app/scripts/controllers/network/network-controller.ts index 51653ab4a..62d89b69f 100644 --- a/app/scripts/controllers/network/network-controller.ts +++ b/app/scripts/controllers/network/network-controller.ts @@ -704,7 +704,7 @@ export class NetworkController extends EventEmitter { * @returns The URL of the RPC endpoint representing the newly switched * network. */ - setActiveNetwork(networkConfigurationId: NetworkConfigurationId): string { + async setActiveNetwork(networkConfigurationId: NetworkConfigurationId) { const targetNetwork = this.store.getState().networkConfigurations[networkConfigurationId]; @@ -714,7 +714,7 @@ export class NetworkController extends EventEmitter { ); } - this.#setProviderConfig({ + await this.#setProviderConfig({ type: NETWORK_TYPES.RPC, ...targetNetwork, });