From fbcd2a1113b522bab182d881db9cdc1a665f48ba Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 31 May 2023 11:52:04 -0600 Subject: [PATCH] Ensure custom provider configs have chain ID and RPC URL (#19296) Update NetworkController so that when it is creating the network client based on configuration for a custom RPC endpoint, it verifies that the configuration object contains both a chain ID and RPC URL. This check is already present on the core side; bringing it over makes the tests more consistent so we can compare them more easily. --- .../network/network-controller.test.ts | 328 +++++++++--------- .../controllers/network/network-controller.ts | 38 +- 2 files changed, 193 insertions(+), 173 deletions(-) diff --git a/app/scripts/controllers/network/network-controller.test.ts b/app/scripts/controllers/network/network-controller.test.ts index 1cfcc4817..b438896a0 100644 --- a/app/scripts/controllers/network/network-controller.test.ts +++ b/app/scripts/controllers/network/network-controller.test.ts @@ -345,76 +345,190 @@ describe('NetworkController', () => { } describe(`when the type in the provider configuration is "rpc"`, () => { - it('initializes a provider pointed to the given RPC URL whose chain ID matches the configured chain ID', async () => { - await withController( - { - state: { - providerConfig: { - type: 'rpc', - chainId: '0x1337', - rpcUrl: 'https://mock-rpc-url', - ticker: 'TEST', - }, - networkConfigurations: { - testNetworkConfigurationId: { - rpcUrl: 'https://mock-rpc-url', + describe('if chainId and rpcUrl are present in the provider config', () => { + it('initializes a provider pointed to the given RPC URL whose chain ID matches the configured chain ID', async () => { + await withController( + { + state: { + providerConfig: { + type: 'rpc', chainId: '0x1337', + rpcUrl: 'https://mock-rpc-url', ticker: 'TEST', - id: 'testNetworkConfigurationId', + }, + networkConfigurations: { + testNetworkConfigurationId: { + rpcUrl: 'https://mock-rpc-url', + chainId: '0x1337', + ticker: 'TEST', + id: 'testNetworkConfigurationId', + }, }, }, }, - }, - async ({ controller }) => { - const fakeProvider = buildFakeProvider([ - { - request: { - method: 'test', + async ({ controller }) => { + const fakeProvider = buildFakeProvider([ + { + request: { + method: 'test', + }, + response: { + result: 'test response', + }, }, - response: { - result: 'test response', - }, - }, - ]); - const fakeNetworkClient = buildFakeClient(fakeProvider); - mockCreateNetworkClient() - .calledWith({ - chainId: '0x1337', - rpcUrl: 'https://mock-rpc-url', - type: NetworkClientType.Custom, - }) - .mockReturnValue(fakeNetworkClient); - mockCreateNetworkClient().mockReturnValue(fakeNetworkClient); + ]); + const fakeNetworkClient = buildFakeClient(fakeProvider); + mockCreateNetworkClient() + .calledWith({ + chainId: '0x1337', + rpcUrl: 'https://mock-rpc-url', + type: NetworkClientType.Custom, + }) + .mockReturnValue(fakeNetworkClient); + mockCreateNetworkClient().mockReturnValue(fakeNetworkClient); - await controller.initializeProvider(); + await controller.initializeProvider(); - const { provider } = controller.getProviderAndBlockTracker(); - assert(provider, 'Provider is somehow unset'); - const promisifiedSendAsync = promisify(provider.sendAsync).bind( - provider, - ); - const response = await promisifiedSendAsync({ - id: '1', - jsonrpc: '2.0', - method: 'test', - }); - expect(response.result).toBe('test response'); - }, - ); - }); + const { provider } = controller.getProviderAndBlockTracker(); + assert(provider, 'Provider is somehow unset'); + const promisifiedSendAsync = promisify(provider.sendAsync).bind( + provider, + ); + const response = await promisifiedSendAsync({ + id: '1', + jsonrpc: '2.0', + method: 'test', + }); + expect(response.result).toBe('test response'); + }, + ); + }); - lookupNetworkTests({ - expectedProviderConfig: buildProviderConfig({ - type: NETWORK_TYPES.RPC, - }), - initialState: { - providerConfig: buildProviderConfig({ + lookupNetworkTests({ + expectedProviderConfig: buildProviderConfig({ type: NETWORK_TYPES.RPC, }), - }, - operation: async (controller: NetworkController) => { - await controller.initializeProvider(); - }, + initialState: { + providerConfig: buildProviderConfig({ + type: NETWORK_TYPES.RPC, + }), + }, + operation: async (controller: NetworkController) => { + await controller.initializeProvider(); + }, + }); + }); + + describe('if chainId is missing from the provider config', () => { + it('throws', async () => { + await withController( + { + state: { + providerConfig: buildProviderConfig({ + type: NETWORK_TYPES.RPC, + chainId: undefined, + }), + }, + }, + async ({ controller }) => { + const fakeProvider = buildFakeProvider(); + const fakeNetworkClient = buildFakeClient(fakeProvider); + createNetworkClientMock.mockReturnValue(fakeNetworkClient); + + await expect(() => + controller.initializeProvider(), + ).rejects.toThrow( + 'chainId must be provided for custom RPC endpoints', + ); + }, + ); + }); + + it('does not create a network client or capture a provider', async () => { + await withController( + { + state: { + providerConfig: buildProviderConfig({ + type: NETWORK_TYPES.RPC, + chainId: undefined, + }), + }, + }, + async ({ controller }) => { + const fakeProvider = buildFakeProvider(); + const fakeNetworkClient = buildFakeClient(fakeProvider); + createNetworkClientMock.mockReturnValue(fakeNetworkClient); + + try { + await controller.initializeProvider(); + } catch { + // ignore the error + } + + expect(createNetworkClientMock).not.toHaveBeenCalled(); + const { provider, blockTracker } = + controller.getProviderAndBlockTracker(); + expect(provider).toBeNull(); + expect(blockTracker).toBeNull(); + }, + ); + }); + }); + + describe('if rpcUrl is missing from the provider config', () => { + it('throws', async () => { + await withController( + { + state: { + providerConfig: buildProviderConfig({ + type: NETWORK_TYPES.RPC, + rpcUrl: undefined, + }), + }, + }, + async ({ controller }) => { + const fakeProvider = buildFakeProvider(); + const fakeNetworkClient = buildFakeClient(fakeProvider); + createNetworkClientMock.mockReturnValue(fakeNetworkClient); + + await expect(() => + controller.initializeProvider(), + ).rejects.toThrow( + 'rpcUrl must be provided for custom RPC endpoints', + ); + }, + ); + }); + + it('does not create a network client or capture a provider', async () => { + await withController( + { + state: { + providerConfig: buildProviderConfig({ + type: NETWORK_TYPES.RPC, + rpcUrl: undefined, + }), + }, + }, + async ({ controller }) => { + const fakeProvider = buildFakeProvider(); + const fakeNetworkClient = buildFakeClient(fakeProvider); + createNetworkClientMock.mockReturnValue(fakeNetworkClient); + + try { + await controller.initializeProvider(); + } catch { + // ignore the error + } + + expect(createNetworkClientMock).not.toHaveBeenCalled(); + const { provider, blockTracker } = + controller.getProviderAndBlockTracker(); + expect(provider).toBeNull(); + expect(blockTracker).toBeNull(); + }, + ); + }); }); }); }); @@ -900,102 +1014,6 @@ describe('NetworkController', () => { }); }); - describe('if the provider has initialized, but the current network has no chainId', () => { - it('does not update state in any way', async () => { - await withController( - /* @ts-expect-error We are intentionally not including a chainId in the provider config. */ - { - state: { - providerConfig: { - type: 'rpc', - rpcUrl: 'http://example-custom-rpc.metamask.io', - }, - networkDetails: { - EIPS: { - 1559: true, - }, - }, - }, - }, - async ({ controller }) => { - const fakeProvider = buildFakeProvider(); - const fakeNetworkClient = buildFakeClient(fakeProvider); - mockCreateNetworkClient().mockReturnValue(fakeNetworkClient); - await controller.initializeProvider(); - const stateAfterInitialization = controller.store.getState(); - - await controller.lookupNetwork(); - - expect(controller.store.getState()).toStrictEqual( - stateAfterInitialization, - ); - }, - ); - }); - - it('does not emit infuraIsUnblocked', async () => { - await withController( - /* @ts-expect-error We are intentionally not including a chainId in the provider config. */ - { - state: { - providerConfig: { - type: 'rpc', - rpcUrl: 'http://example-custom-rpc.metamask.io', - }, - }, - }, - async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider(); - const fakeNetworkClient = buildFakeClient(fakeProvider); - mockCreateNetworkClient().mockReturnValue(fakeNetworkClient); - await controller.initializeProvider(); - - const promiseForNoInfuraIsUnblockedEvents = waitForPublishedEvents({ - messenger, - eventType: 'NetworkController:infuraIsUnblocked', - count: 0, - operation: async () => { - await controller.lookupNetwork(); - }, - }); - - expect(await promiseForNoInfuraIsUnblockedEvents).toBeTruthy(); - }, - ); - }); - - it('does not emit infuraIsBlocked', async () => { - await withController( - /* @ts-expect-error We are intentionally not including a chainId in the provider config. */ - { - state: { - providerConfig: { - type: 'rpc', - rpcUrl: 'http://example-custom-rpc.metamask.io', - }, - }, - }, - async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider(); - const fakeNetworkClient = buildFakeClient(fakeProvider); - mockCreateNetworkClient().mockReturnValue(fakeNetworkClient); - await controller.initializeProvider(); - - const promiseForNoInfuraIsBlockedEvents = waitForPublishedEvents({ - messenger, - eventType: 'NetworkController:infuraIsBlocked', - count: 0, - operation: async () => { - await controller.lookupNetwork(); - }, - }); - - expect(await promiseForNoInfuraIsBlockedEvents).toBeTruthy(); - }, - ); - }); - }); - INFURA_NETWORKS.forEach(({ networkType }) => { describe(`when the type in the provider configuration is "${networkType}"`, () => { describe('if the network was switched after the eth_getBlockByNumber request started but before it completed', () => { diff --git a/app/scripts/controllers/network/network-controller.ts b/app/scripts/controllers/network/network-controller.ts index ea6afbd66..ac0193d88 100644 --- a/app/scripts/controllers/network/network-controller.ts +++ b/app/scripts/controllers/network/network-controller.ts @@ -570,7 +570,7 @@ export class NetworkController extends EventEmitter { * blocking requests, or if the network is not Infura-supported. */ async lookupNetwork(): Promise { - const { chainId, type } = this.store.getState().providerConfig; + const { type } = this.store.getState().providerConfig; const { provider } = this.getProviderAndBlockTracker(); let networkChanged = false; let networkId: NetworkIdState = null; @@ -584,16 +584,6 @@ export class NetworkController extends EventEmitter { return; } - if (!chainId) { - log.warn( - 'NetworkController - lookupNetwork aborted due to missing chainId', - ); - this.#resetNetworkId(); - this.#resetNetworkStatus(); - this.#resetNetworkDetails(); - return; - } - const isInfura = isInfuraProviderType(type); const listener = () => { @@ -874,11 +864,12 @@ export class NetworkController extends EventEmitter { * the new network. */ async #switchNetwork(providerConfig: ProviderConfiguration) { + const { type, rpcUrl, chainId } = providerConfig; this.#messenger.publish('NetworkController:networkWillChange'); this.#resetNetworkId(); this.#resetNetworkStatus(); this.#resetNetworkDetails(); - this.#configureProvider(providerConfig); + this.#configureProvider({ type, rpcUrl, chainId }); this.#messenger.publish('NetworkController:networkDidChange'); await this.lookupNetwork(); } @@ -888,8 +879,7 @@ export class NetworkController extends EventEmitter { * block tracker) to talk to a network. * * @param args - The arguments. - * @param args.type - The shortname of an Infura-supported network (see - * {@link NETWORK_TYPES}). + * @param args.type - The provider type. * @param args.rpcUrl - The URL of the RPC endpoint that represents the * network. Only used for non-Infura networks. * @param args.chainId - The chain ID of the network (as per EIP-155). Only @@ -897,16 +887,28 @@ export class NetworkController extends EventEmitter { * any Infura-supported network). * @throws if the `type` if not a known Infura-supported network. */ - #configureProvider({ type, rpcUrl, chainId }: ProviderConfiguration): void { + #configureProvider({ + type, + rpcUrl, + chainId, + }: { + type: ProviderType; + rpcUrl: string | undefined; + chainId: Hex | undefined; + }): void { const isInfura = isInfuraProviderType(type); if (isInfura) { - // infura type-based endpoints this.#configureInfuraProvider({ type, infuraProjectId: this.#infuraProjectId, }); - } else if (type === NETWORK_TYPES.RPC && rpcUrl) { - // url-based rpc endpoints + } else if (type === NETWORK_TYPES.RPC) { + if (chainId === undefined) { + throw new Error('chainId must be provided for custom RPC endpoints'); + } + if (rpcUrl === undefined) { + throw new Error('rpcUrl must be provided for custom RPC endpoints'); + } this.#configureStandardProvider(rpcUrl, chainId); } else { throw new Error(