From b87c1b85262e5faa101fe8d0562523b8fe2c1c00 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Mon, 17 Apr 2023 15:45:01 -0230 Subject: [PATCH] Make `setProviderType` async (#18604) The network controller method `setProviderType` is now async, and the async operation `_setProviderConfig` called at the end of the method is now awaited. Because the only async operation was the last step, this should have no impact upon the flow of execution. The only functional change is that now any callers have the option of waiting until the network switch operation has completed. One such change was made, in the `switch-ethereum-chain` middleware. As a result, an error thrown while the network is switching will now be thrown in this middleware and returned to the dapp as an internal error. Relates to https://github.com/MetaMask/metamask-extension/issues/18587 --- app/scripts/controllers/detect-tokens.test.js | 12 +- .../network/network-controller.test.ts | 197 ++++-------------- .../controllers/network/network-controller.ts | 4 +- .../handlers/switch-ethereum-chain.js | 2 +- 4 files changed, 49 insertions(+), 166 deletions(-) diff --git a/app/scripts/controllers/detect-tokens.test.js b/app/scripts/controllers/detect-tokens.test.js index 10db93359..d5f6eb7bd 100644 --- a/app/scripts/controllers/detect-tokens.test.js +++ b/app/scripts/controllers/detect-tokens.test.js @@ -278,7 +278,7 @@ describe('DetectTokensController', function () { it('should be called on every polling period', async function () { const clock = sandbox.useFakeTimers(); - network.setProviderType(NETWORK_TYPES.MAINNET); + await network.setProviderType(NETWORK_TYPES.MAINNET); const controller = new DetectTokensController({ preferences, network, @@ -304,7 +304,7 @@ describe('DetectTokensController', function () { it('should not check and add tokens while on unsupported networks', async function () { sandbox.useFakeTimers(); - network.setProviderType(NETWORK_TYPES.SEPOLIA); + await network.setProviderType(NETWORK_TYPES.SEPOLIA); const tokenListMessengerSepolia = new ControllerMessenger().getRestricted({ name: 'TokenListController', }); @@ -337,7 +337,7 @@ describe('DetectTokensController', function () { it('should skip adding tokens listed in ignoredTokens array', async function () { sandbox.useFakeTimers(); - network.setProviderType(NETWORK_TYPES.MAINNET); + await network.setProviderType(NETWORK_TYPES.MAINNET); const controller = new DetectTokensController({ preferences, network, @@ -388,7 +388,7 @@ describe('DetectTokensController', function () { it('should check and add tokens while on supported networks', async function () { sandbox.useFakeTimers(); - network.setProviderType(NETWORK_TYPES.MAINNET); + await network.setProviderType(NETWORK_TYPES.MAINNET); const controller = new DetectTokensController({ preferences, network, @@ -483,7 +483,7 @@ describe('DetectTokensController', function () { it('should not trigger detect new tokens when not unlocked', async function () { const clock = sandbox.useFakeTimers(); - network.setProviderType(NETWORK_TYPES.MAINNET); + await network.setProviderType(NETWORK_TYPES.MAINNET); const controller = new DetectTokensController({ preferences, network, @@ -504,7 +504,7 @@ describe('DetectTokensController', function () { it('should not trigger detect new tokens when not open', async function () { const clock = sandbox.useFakeTimers(); - network.setProviderType(NETWORK_TYPES.MAINNET); + await network.setProviderType(NETWORK_TYPES.MAINNET); const controller = new DetectTokensController({ preferences, network, diff --git a/app/scripts/controllers/network/network-controller.test.ts b/app/scripts/controllers/network/network-controller.test.ts index 068a85859..c5d5975e1 100644 --- a/app/scripts/controllers/network/network-controller.test.ts +++ b/app/scripts/controllers/network/network-controller.test.ts @@ -1135,7 +1135,7 @@ describe('NetworkController', () => { }); expect(oldChainIdResult).toBe('0x1337'); - controller.setProviderType(networkType); + await controller.setProviderType(networkType); const promisifiedSendAsync2 = promisify(provider.sendAsync).bind( provider, ); @@ -2652,21 +2652,9 @@ describe('NetworkController', () => { network1.mockEssentialRpcCalls({ eth_getBlockByNumber: { beforeCompleting: async () => { - await waitForPublishedEvents({ - messenger: unrestrictedMessenger, - eventType: NetworkControllerEventType.NetworkDidChange, - operation: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkStatus'], - operation: () => { - controller.setProviderType( - anotherNetwork.networkType, - ); - }, - }); - }, - }); + await controller.setProviderType( + anotherNetwork.networkType, + ); }, }, }); @@ -3513,13 +3501,7 @@ describe('NetworkController', () => { { response: SUCCESSFUL_NET_VERSION_RESPONSE, beforeCompleting: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkStatus'], - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); }, }, ], @@ -3618,13 +3600,7 @@ describe('NetworkController', () => { result: '111', }, beforeCompleting: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkStatus'], - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); }, }, }); @@ -3675,13 +3651,7 @@ describe('NetworkController', () => { net_version: { response: SUCCESSFUL_NET_VERSION_RESPONSE, beforeCompleting: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkDetails'], - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); }, }, }); @@ -3738,13 +3708,7 @@ describe('NetworkController', () => { network1.mockEssentialRpcCalls({ net_version: { beforeCompleting: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkDetails'], - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); }, }, }); @@ -3841,13 +3805,7 @@ describe('NetworkController', () => { }, }, beforeCompleting: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkStatus'], - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); }, }, ], @@ -3904,13 +3862,7 @@ describe('NetworkController', () => { network1.mockEssentialRpcCalls({ eth_getBlockByNumber: { beforeCompleting: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkStatus'], - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); }, }, net_version: { @@ -3965,13 +3917,7 @@ describe('NetworkController', () => { latestBlock: POST_1559_BLOCK, eth_getBlockByNumber: { beforeCompleting: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkDetails'], - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); }, }, }); @@ -4028,13 +3974,7 @@ describe('NetworkController', () => { network1.mockEssentialRpcCalls({ eth_getBlockByNumber: { beforeCompleting: async () => { - await waitForStateChanges({ - controller, - propertyPath: ['networkDetails'], - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); }, }, }); @@ -4629,7 +4569,7 @@ describe('NetworkController', () => { }); network.mockEssentialRpcCalls(); - controller.setProviderType(networkType); + await controller.setProviderType(networkType); expect(controller.store.getState().provider).toStrictEqual({ type: networkType, @@ -4662,6 +4602,8 @@ describe('NetworkController', () => { messenger: unrestrictedMessenger, eventType: NetworkControllerEventType.NetworkWillChange, operation: () => { + // Intentionally not awaited because we're capturing an event + // emitted partway through the operation controller.setProviderType(networkType); }, }); @@ -4714,6 +4656,8 @@ describe('NetworkController', () => { // happens before networkDidChange count: 1, operation: () => { + // Intentionally not awaited because we're checking the state + // partway through the operation controller.setProviderType(networkType); }, }); @@ -4767,6 +4711,8 @@ describe('NetworkController', () => { // happens before networkDidChange count: 1, operation: () => { + // Intentionally not awaited because we're checking the state + // partway through the operation controller.setProviderType(networkType); }, }); @@ -4786,7 +4732,7 @@ describe('NetworkController', () => { }); network.mockEssentialRpcCalls(); - controller.setProviderType(networkType); + await controller.setProviderType(networkType); const { provider } = controller.getProviderAndBlockTracker(); assert(provider, 'Provider is somehow unset'); @@ -4813,7 +4759,7 @@ describe('NetworkController', () => { const { provider: providerBefore } = controller.getProviderAndBlockTracker(); - controller.setProviderType(networkType); + await controller.setProviderType(networkType); const { provider: providerAfter } = controller.getProviderAndBlockTracker(); @@ -4836,8 +4782,8 @@ describe('NetworkController', () => { const networkDidChange = await waitForPublishedEvents({ messenger: unrestrictedMessenger, eventType: NetworkControllerEventType.NetworkDidChange, - operation: () => { - controller.setProviderType(networkType); + operation: async () => { + await controller.setProviderType(networkType); }, }); @@ -4872,7 +4818,7 @@ describe('NetworkController', () => { eventType: NetworkControllerEventType.InfuraIsBlocked, }); - controller.setProviderType(networkType); + await controller.setProviderType(networkType); expect(await promiseForNoInfuraIsUnblockedEvents).toBeTruthy(); expect(await promiseForInfuraIsBlocked).toBeTruthy(); @@ -4891,13 +4837,7 @@ describe('NetworkController', () => { latestBlock: BLOCK, }); - await waitForStateChanges({ - controller, - propertyPath: ['networkStatus'], - operation: () => { - controller.setProviderType(networkType); - }, - }); + await controller.setProviderType(networkType); expect(controller.store.getState().networkStatus).toBe('available'); }); @@ -4921,16 +4861,7 @@ describe('NetworkController', () => { latestBlock: POST_1559_BLOCK, }); - await waitForStateChanges({ - controller, - propertyPath: ['networkDetails'], - // setProviderType clears networkDetails first, and then updates - // it to what we expect it to be - count: 2, - operation: () => { - controller.setProviderType(networkType); - }, - }); + await controller.setProviderType(networkType); expect(controller.store.getState().networkDetails).toStrictEqual({ EIPS: { @@ -4946,7 +4877,7 @@ describe('NetworkController', () => { describe('given a type of "rpc"', () => { it('throws', async () => { await withController(async ({ controller }) => { - expect(() => controller.setProviderType('rpc')).toThrow( + await expect(() => controller.setProviderType('rpc')).rejects.toThrow( new Error( 'NetworkController - cannot call "setProviderType" with type "rpc". Use "setActiveNetwork"', ), @@ -4958,7 +4889,9 @@ describe('NetworkController', () => { describe('given an invalid Infura network name', () => { it('throws', async () => { await withController(async ({ controller }) => { - expect(() => controller.setProviderType('sadlflaksdj')).toThrow( + await expect(() => + controller.setProviderType('sadlflaksdj'), + ).rejects.toThrow( new Error('Unknown Infura provider type "sadlflaksdj".'), ); }); @@ -6393,12 +6326,7 @@ describe('NetworkController', () => { currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); expect(controller.store.getState().provider).toStrictEqual({ type: 'goerli', rpcUrl: '', @@ -6466,12 +6394,7 @@ describe('NetworkController', () => { }); currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); await waitForLookupNetworkToComplete({ controller, @@ -6518,12 +6441,7 @@ describe('NetworkController', () => { currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); expect(controller.store.getState().networkStatus).toBe('available'); await waitForLookupNetworkToComplete({ @@ -6579,12 +6497,7 @@ describe('NetworkController', () => { }); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); expect(controller.store.getState().networkDetails).toStrictEqual({ EIPS: { 1559: true, @@ -6645,12 +6558,7 @@ describe('NetworkController', () => { }); currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); await waitForLookupNetworkToComplete({ controller, @@ -6700,12 +6608,7 @@ describe('NetworkController', () => { }); currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); const { provider: providerBefore } = controller.getProviderAndBlockTracker(); @@ -6754,12 +6657,7 @@ describe('NetworkController', () => { currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); await waitForLookupNetworkToComplete({ controller, @@ -6809,12 +6707,7 @@ describe('NetworkController', () => { currentNetwork.mockEssentialRpcCalls(); previousNetwork.mockEssentialRpcCalls(); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); await waitForLookupNetworkToComplete({ controller, @@ -6869,12 +6762,7 @@ describe('NetworkController', () => { }, }); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); expect(controller.store.getState().networkStatus).toBe('available'); await waitForLookupNetworkToComplete({ @@ -6919,12 +6807,7 @@ describe('NetworkController', () => { latestBlock: POST_1559_BLOCK, }); - await waitForLookupNetworkToComplete({ - controller, - operation: () => { - controller.setProviderType('goerli'); - }, - }); + await controller.setProviderType('goerli'); 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 2cfdadd2e..783e368a1 100644 --- a/app/scripts/controllers/network/network-controller.ts +++ b/app/scripts/controllers/network/network-controller.ts @@ -729,7 +729,7 @@ export class NetworkController extends EventEmitter { * @throws if the `type` is "rpc" or if it is not a known Infura-supported * network. */ - setProviderType(type: string): void { + async setProviderType(type: string) { assert.notStrictEqual( type, NETWORK_TYPES.RPC, @@ -740,7 +740,7 @@ export class NetworkController extends EventEmitter { `Unknown Infura provider type "${type}".`, ); const network = BUILT_IN_INFURA_NETWORKS[type]; - this.#setProviderConfig({ + await this.#setProviderConfig({ type, rpcUrl: '', chainId: network.chainId, diff --git a/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js b/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js index 81c6887e0..faeded072 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js @@ -113,7 +113,7 @@ async function switchEthereumChainHandler( approvedRequestData.type !== NETWORK_TYPES.LOCALHOST && approvedRequestData.type !== NETWORK_TYPES.LINEA_TESTNET ) { - setProviderType(approvedRequestData.type); + await setProviderType(approvedRequestData.type); } else { await setActiveNetwork(approvedRequestData.id); }