diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index c84b98844f..b4ab482c4a 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -15,8 +15,10 @@ import { } from './AssetsContractController'; import { SupportedTokenDetectionNetworks } from './assetsUtil'; +const INFURA_PROJECT_ID = '341eacb578dd44a1a049cbc5f6fd4035'; + const MAINNET_PROVIDER = new HttpProvider( - 'https://mainnet.infura.io/v3/341eacb578dd44a1a049cbc5f6fd4035', + `https://mainnet.infura.io/v3/${INFURA_PROJECT_ID}`, ); const ERC20_UNI_ADDRESS = '0x1f9840a85d5af5bf1d1762f925bdaddc4201f984'; @@ -37,7 +39,7 @@ const setupControllers = () => { allowedActions: [], }); const network = new NetworkController({ - infuraProjectId: 'infura-project-id', + infuraProjectId: INFURA_PROJECT_ID, messenger, trackMetaMetricsEvent: jest.fn(), }); diff --git a/packages/gas-fee-controller/src/GasFeeController.test.ts b/packages/gas-fee-controller/src/GasFeeController.test.ts index d2d2133be1..edfe15b8f4 100644 --- a/packages/gas-fee-controller/src/GasFeeController.test.ts +++ b/packages/gas-fee-controller/src/GasFeeController.test.ts @@ -271,10 +271,8 @@ describe('GasFeeController', () => { afterEach(() => { gasFeeController.destroy(); - const { provider } = networkController.getProviderAndBlockTracker(); - if (provider) { - provider.stop(); - } + const { blockTracker } = networkController.getProviderAndBlockTracker(); + blockTracker?.destroy(); sinon.restore(); jest.clearAllMocks(); nock.enableNetConnect(); diff --git a/packages/network-controller/package.json b/packages/network-controller/package.json index 17e6ad6af6..852b427adf 100644 --- a/packages/network-controller/package.json +++ b/packages/network-controller/package.json @@ -31,27 +31,27 @@ "dependencies": { "@metamask/base-controller": "workspace:^", "@metamask/controller-utils": "workspace:^", + "@metamask/eth-json-rpc-infura": "^8.0.0", + "@metamask/eth-json-rpc-middleware": "^11.0.0", + "@metamask/eth-json-rpc-provider": "^1.0.0", "@metamask/swappable-obj-proxy": "^2.1.0", "@metamask/utils": "^5.0.2", "async-mutex": "^0.2.6", "babel-runtime": "^6.26.0", - "eth-json-rpc-infura": "^5.1.0", + "eth-block-tracker": "^7.0.1", "eth-query": "^2.1.2", "eth-rpc-errors": "^4.0.2", "immer": "^9.0.6", - "uuid": "^8.3.2", - "web3-provider-engine": "^16.0.5" + "json-rpc-engine": "^6.1.0", + "uuid": "^8.3.2" }, "devDependencies": { "@json-rpc-specification/meta-schema": "^1.0.6", "@metamask/auto-changelog": "^3.1.0", - "@metamask/eth-json-rpc-provider": "^1.0.0", "@types/jest": "^27.4.1", "@types/lodash": "^4.14.191", "deepmerge": "^4.2.2", - "eth-block-tracker": "^7.0.1", "jest": "^27.5.1", - "json-rpc-engine": "^6.1.0", "lodash": "^4.17.21", "nock": "^13.0.7", "sinon": "^9.2.4", diff --git a/packages/network-controller/src/NetworkController.ts b/packages/network-controller/src/NetworkController.ts index 2ca229ad1f..d07369e095 100644 --- a/packages/network-controller/src/NetworkController.ts +++ b/packages/network-controller/src/NetworkController.ts @@ -17,8 +17,10 @@ import { NetworkType, isSafeChainId, isNetworkType, + toHex, } from '@metamask/controller-utils'; import { + Hex, assertIsStrictHexString, hasProperty, isPlainObject, @@ -255,8 +257,6 @@ export class NetworkController extends BaseControllerV2< #previousNetworkSpecifier: NetworkType | NetworkConfigurationId | null; - #provider: Provider | undefined; - #providerProxy: ProviderProxy | undefined; #blockTrackerProxy: BlockTrackerProxy | undefined; @@ -316,7 +316,11 @@ export class NetworkController extends BaseControllerV2< this.#previousNetworkSpecifier = this.state.providerConfig.type; } - #configureProvider(type: NetworkType, rpcUrl?: string, chainId?: string) { + #configureProvider( + type: NetworkType, + rpcUrl: string | undefined, + chainId: string | undefined, + ) { switch (type) { case NetworkType.mainnet: case NetworkType.goerli: @@ -331,7 +335,7 @@ export class NetworkController extends BaseControllerV2< if (rpcUrl === undefined) { throw new Error('rpcUrl must be provided for custom RPC endpoints'); } - this.#setupStandardProvider(rpcUrl, chainId); + this.#setupStandardProvider(rpcUrl, toHex(chainId)); break; default: throw new Error(`Unrecognized network type: '${type}'`); @@ -363,7 +367,6 @@ export class NetworkController extends BaseControllerV2< const { provider } = this.getProviderAndBlockTracker(); if (provider) { - provider.on('error', this.#verifyNetwork.bind(this)); this.#ethQuery = new EthQuery(provider); } } @@ -378,7 +381,7 @@ export class NetworkController extends BaseControllerV2< this.#updateProvider(provider, blockTracker); } - #setupStandardProvider(rpcUrl: string, chainId: string) { + #setupStandardProvider(rpcUrl: string, chainId: Hex) { const { provider, blockTracker } = createNetworkClient({ chainId, rpcUrl, @@ -388,8 +391,7 @@ export class NetworkController extends BaseControllerV2< this.#updateProvider(provider, blockTracker); } - #updateProvider(provider: Provider, blockTracker: any) { - this.#safelyStopProvider(this.#provider); + #updateProvider(provider: Provider, blockTracker: BlockTracker) { this.#setProviderAndBlockTracker({ provider, blockTracker, @@ -397,18 +399,6 @@ export class NetworkController extends BaseControllerV2< this.#registerProvider(); } - #safelyStopProvider(provider: Provider | undefined) { - setTimeout(() => { - provider?.stop(); - }, 500); - } - - async #verifyNetwork() { - if (this.state.networkStatus !== NetworkStatus.Available) { - await this.lookupNetwork(); - } - } - /** * Method to inilialize the provider, * Creates the provider and block tracker for the configured network, @@ -656,7 +646,6 @@ export class NetworkController extends BaseControllerV2< } else { this.#providerProxy = createEventEmitterProxy(provider); } - this.#provider = provider; if (this.#blockTrackerProxy) { this.#blockTrackerProxy.setTarget(blockTracker); diff --git a/packages/network-controller/src/create-network-client.ts b/packages/network-controller/src/create-network-client.ts index f0c60c6896..c42f02ed58 100644 --- a/packages/network-controller/src/create-network-client.ts +++ b/packages/network-controller/src/create-network-client.ts @@ -1,9 +1,32 @@ -import Subprovider from 'web3-provider-engine/subproviders/provider'; -import createInfuraProvider from 'eth-json-rpc-infura/src/createProvider'; -import createMetamaskProvider from 'web3-provider-engine/zero'; -import { InfuraNetworkType } from '@metamask/controller-utils'; +import { + createAsyncMiddleware, + createScaffoldMiddleware, + JsonRpcEngine, + mergeMiddleware, + JsonRpcMiddleware, +} from 'json-rpc-engine'; +import { + createBlockCacheMiddleware, + createBlockRefMiddleware, + createBlockRefRewriteMiddleware, + createBlockTrackerInspectorMiddleware, + createInflightCacheMiddleware, + createFetchMiddleware, + createRetryOnEmptyMiddleware, +} from '@metamask/eth-json-rpc-middleware'; +import { + providerFromEngine, + providerFromMiddleware, + SafeEventEmitterProvider, +} from '@metamask/eth-json-rpc-provider'; +import { createInfuraMiddleware } from '@metamask/eth-json-rpc-infura'; +import type { Hex } from '@metamask/utils'; +import { PollingBlockTracker } from 'eth-block-tracker'; +import { InfuraNetworkType, NetworksChainId } from '@metamask/controller-utils'; import type { BlockTracker, Provider } from './types'; +const SECOND = 1000; + /** * The type of network client that can be created. */ @@ -17,7 +40,7 @@ export enum NetworkClientType { * custom network. */ type CustomNetworkConfiguration = { - chainId?: string; + chainId: Hex; rpcUrl: string; type: NetworkClientType.Custom; }; @@ -33,7 +56,7 @@ type InfuraNetworkConfiguration = { }; /** - * Create a JSON-RPC network client for a specific network. + * Create a JSON RPC network client for a specific network. * * @param networkConfig - The network configuration. * @returns The network client. @@ -41,52 +64,164 @@ type InfuraNetworkConfiguration = { export function createNetworkClient( networkConfig: CustomNetworkConfiguration | InfuraNetworkConfiguration, ): { provider: Provider; blockTracker: BlockTracker } { - const providerConfig = + const rpcApiMiddleware = networkConfig.type === NetworkClientType.Infura - ? buildInfuraNetworkProviderConfig(networkConfig) - : buildCustomNetworkProviderConfig(networkConfig); + ? createInfuraMiddleware({ + network: networkConfig.network, + projectId: networkConfig.infuraProjectId, + maxAttempts: 5, + source: 'metamask', + }) + : createFetchMiddleware({ + btoa: global.btoa, + fetch: global.fetch, + rpcUrl: networkConfig.rpcUrl, + }); + + const rpcProvider = providerFromMiddleware(rpcApiMiddleware); + + const blockTrackerOpts = + // eslint-disable-next-line node/no-process-env + process.env.IN_TEST && networkConfig.type === 'custom' + ? { pollingInterval: SECOND } + : {}; + const blockTracker = new PollingBlockTracker({ + ...blockTrackerOpts, + provider: rpcProvider, + }); + + const networkMiddleware = + networkConfig.type === NetworkClientType.Infura + ? createInfuraNetworkMiddleware({ + blockTracker, + network: networkConfig.network, + rpcProvider, + rpcApiMiddleware, + }) + : createCustomNetworkMiddleware({ + blockTracker, + chainId: networkConfig.chainId, + rpcApiMiddleware, + }); + + const engine = new JsonRpcEngine(); - const provider = createMetamaskProvider(providerConfig); + engine.push(networkMiddleware); - return { provider, blockTracker: provider._blockTracker }; + const provider = providerFromEngine(engine); + + return { provider, blockTracker }; +} + +/** + * Create middleware for infura. + * + * @param args - The arguments. + * @param args.blockTracker - The block tracker to use. + * @param args.network - The Infura network to use. + * @param args.rpcProvider - The RPC provider to use. + * @param args.rpcApiMiddleware - Additional middleware. + * @returns The collection of middleware that makes up the Infura client. + */ +function createInfuraNetworkMiddleware({ + blockTracker, + network, + rpcProvider, + rpcApiMiddleware, +}: { + blockTracker: PollingBlockTracker; + network: InfuraNetworkType; + rpcProvider: SafeEventEmitterProvider; + rpcApiMiddleware: JsonRpcMiddleware; +}) { + return mergeMiddleware([ + createNetworkAndChainIdMiddleware({ network }), + createBlockCacheMiddleware({ blockTracker }), + createInflightCacheMiddleware(), + createBlockRefMiddleware({ blockTracker, provider: rpcProvider }), + createRetryOnEmptyMiddleware({ blockTracker, provider: rpcProvider }), + createBlockTrackerInspectorMiddleware({ blockTracker }), + rpcApiMiddleware, + ]); } /** - * Construct the configuration object for a provider engine that can be used to - * interface with an Infura network. + * Creates static method middleware. * - * @param networkConfig - Network configuration. - * @returns The complete provider engine configuration object. + * @param args - The Arguments. + * @param args.network - The Infura network to use. + * @returns The middleware that implements eth_chainId & net_version methods. */ -function buildInfuraNetworkProviderConfig( - networkConfig: InfuraNetworkConfiguration, -) { - const infuraProvider = createInfuraProvider({ - network: networkConfig.network, - projectId: networkConfig.infuraProjectId, +function createNetworkAndChainIdMiddleware({ + network, +}: { + network: InfuraNetworkType; +}) { + const chainId = NetworksChainId[network]; + + return createScaffoldMiddleware({ + eth_chainId: `0x${parseInt(chainId, 10).toString(16)}`, + net_version: chainId, }); - const infuraSubprovider = new Subprovider(infuraProvider); - return { - dataSubprovider: infuraSubprovider, - engineParams: { - blockTrackerProvider: infuraProvider, - pollingInterval: 12000, - }, +} + +const createChainIdMiddleware = ( + chainId: string, +): JsonRpcMiddleware => { + return (req, res, next, end) => { + if (req.method === 'eth_chainId') { + res.result = chainId; + return end(); + } + return next(); }; +}; + +/** + * Creates custom middleware. + * + * @param args - The arguments. + * @param args.blockTracker - The block tracker to use. + * @param args.chainId - The chain id to use. + * @param args.rpcApiMiddleware - Additional middleware. + * @returns The collection of middleware that makes up the Infura client. + */ +function createCustomNetworkMiddleware({ + blockTracker, + chainId, + rpcApiMiddleware, +}: { + blockTracker: PollingBlockTracker; + chainId: string; + rpcApiMiddleware: any; +}) { + // eslint-disable-next-line node/no-process-env + const testMiddlewares = process.env.IN_TEST + ? [createEstimateGasDelayTestMiddleware()] + : []; + + return mergeMiddleware([ + ...testMiddlewares, + createChainIdMiddleware(chainId), + createBlockRefRewriteMiddleware({ blockTracker }), + createBlockCacheMiddleware({ blockTracker }), + createInflightCacheMiddleware(), + createBlockTrackerInspectorMiddleware({ blockTracker }), + rpcApiMiddleware, + ]); } /** - * Construct the configuration object for a provider engine that can be used to - * interface with a custom network. + * For use in tests only. + * Adds a delay to `eth_estimateGas` calls. * - * @param networkConfig - Network configuration. - * @returns The complete provider engine configuration object. + * @returns The middleware for delaying gas estimation calls by 2 seconds when in test. */ -function buildCustomNetworkProviderConfig( - networkConfig: CustomNetworkConfiguration, -) { - return { - ...networkConfig, - engineParams: { pollingInterval: 12000 }, - }; +function createEstimateGasDelayTestMiddleware() { + return createAsyncMiddleware(async (req, _, next) => { + if (req.method === 'eth_estimateGas') { + await new Promise((resolve) => setTimeout(resolve, SECOND * 2)); + } + return next(); + }); } diff --git a/packages/network-controller/src/types.ts b/packages/network-controller/src/types.ts index 2dc951611e..1945aaa8d5 100644 --- a/packages/network-controller/src/types.ts +++ b/packages/network-controller/src/types.ts @@ -1,6 +1,6 @@ -import type { ProviderEngine } from 'web3-provider-engine'; +import type { SafeEventEmitterProvider } from '@metamask/eth-json-rpc-provider'; import type { PollingBlockTracker } from 'eth-block-tracker'; -export type Provider = ProviderEngine; +export type Provider = SafeEventEmitterProvider; export type BlockTracker = PollingBlockTracker; diff --git a/packages/network-controller/tests/NetworkController.test.ts b/packages/network-controller/tests/NetworkController.test.ts index f5838512a3..b0d387ea20 100644 --- a/packages/network-controller/tests/NetworkController.test.ts +++ b/packages/network-controller/tests/NetworkController.test.ts @@ -1,12 +1,15 @@ import { inspect, isDeepStrictEqual, promisify } from 'util'; import assert from 'assert'; - import { ControllerMessenger } from '@metamask/base-controller'; import * as ethQueryModule from 'eth-query'; import { Patch } from 'immer'; import { v4 } from 'uuid'; import { ethErrors } from 'eth-rpc-errors'; -import { BUILT_IN_NETWORKS, NetworkType } from '@metamask/controller-utils'; +import { + BUILT_IN_NETWORKS, + NetworkType, + toHex, +} from '@metamask/controller-utils'; import { NetworkController, NetworkControllerActions, @@ -23,7 +26,7 @@ import { NetworkClientType, } from '../src/create-network-client'; import { FakeBlockTracker } from '../../../tests/fake-block-tracker'; -import { FakeProviderEngine, FakeProviderStub } from './fake-provider-engine'; +import { FakeProvider, FakeProviderStub } from './fake-provider'; jest.mock('../src/create-network-client'); @@ -269,169 +272,6 @@ describe('NetworkController', () => { }, ); }); - - it('ensures that the existing provider is stopped while replacing it', async () => { - await withController( - { - state: { - providerConfig: buildProviderConfig({ - type: networkType, - }), - }, - }, - async ({ controller }) => { - const fakeProviders = [ - buildFakeProvider(), - buildFakeProvider(), - ]; - jest.spyOn(fakeProviders[0], 'stop'); - const fakeNetworkClients = [ - buildFakeClient(fakeProviders[0]), - buildFakeClient(fakeProviders[1]), - ]; - createNetworkClientMock - .mockImplementationOnce(() => fakeNetworkClients[0]) - .mockImplementationOnce(() => fakeNetworkClients[1]); - - await controller.initializeProvider(); - await controller.initializeProvider(); - assert(controller.getProviderAndBlockTracker().provider); - jest.runAllTimers(); - - expect(fakeProviders[0].stop).toHaveBeenCalled(); - }, - ); - }); - - describe('when an "error" event occurs on the new provider', () => { - describe('if the network version could not be retrieved while providerConfig was being set', () => { - it('retrieves the network version twice more (due to the "error" event being listened to twice) and, assuming success, persists it to state', async () => { - await withController( - { - state: { - providerConfig: buildProviderConfig({ - type: networkType, - }), - }, - }, - async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider([ - { - request: { - method: 'net_version', - }, - response: { - error: 'oops', - }, - }, - { - request: { - method: 'net_version', - }, - response: { - result: '1', - }, - }, - { - request: { - method: 'net_version', - }, - response: { - result: '2', - }, - }, - ]); - const fakeNetworkClient = buildFakeClient(fakeProvider); - createNetworkClientMock.mockReturnValue(fakeNetworkClient); - - await waitForPublishedEvents( - messenger, - 'NetworkController:providerConfigChange', - { - produceEvents: async () => { - await controller.initializeProvider(); - assert( - controller.getProviderAndBlockTracker().provider, - ); - }, - }, - ); - - await waitForStateChanges(messenger, { - propertyPath: ['networkId'], - count: 2, - produceStateChanges: () => { - controller - .getProviderAndBlockTracker() - .provider?.emit('error', { some: 'error' }); - }, - }); - expect(controller.state.networkId).toBe('2'); - }, - ); - }); - }); - - describe('if the network version could be retrieved while providerConfig was being set', () => { - it('does not retrieve the network version again', async () => { - await withController( - { - state: { - providerConfig: buildProviderConfig({ - type: networkType, - }), - }, - }, - async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider([ - { - request: { - method: 'net_version', - }, - response: { - result: '1', - }, - }, - { - request: { - method: 'net_version', - }, - response: { - result: '2', - }, - }, - ]); - const fakeNetworkClient = buildFakeClient(fakeProvider); - createNetworkClientMock.mockReturnValue(fakeNetworkClient); - - await waitForPublishedEvents( - messenger, - 'NetworkController:providerConfigChange', - { - produceEvents: async () => { - await controller.initializeProvider(); - assert( - controller.getProviderAndBlockTracker().provider, - ); - }, - }, - ); - - await waitForStateChanges(messenger, { - propertyPath: ['networkId'], - count: 0, - produceStateChanges: () => { - controller - .getProviderAndBlockTracker() - .provider?.emit('error', { some: 'error' }); - }, - }); - expect(controller.state.networkId).toBe('1'); - }, - ); - }); - }); - }); }); }, ); @@ -444,7 +284,7 @@ describe('NetworkController', () => { state: { providerConfig: { type: NetworkType.rpc, - chainId: '123', + chainId: '1337', nickname: 'some cool network', rpcUrl: 'http://example.com', ticker: 'ABC', @@ -468,7 +308,7 @@ describe('NetworkController', () => { await controller.initializeProvider(); expect(createNetworkClientMock).toHaveBeenCalledWith({ - chainId: '123', + chainId: toHex(1337), rpcUrl: 'http://example.com', type: NetworkClientType.Custom, }); @@ -487,169 +327,6 @@ describe('NetworkController', () => { }, ); }); - - it('ensures that the existing provider is stopped while replacing it', async () => { - await withController( - { - state: { - providerConfig: buildProviderConfig({ - type: NetworkType.rpc, - rpcUrl: 'http://example.com', - }), - }, - }, - async ({ controller }) => { - const fakeProviders = [buildFakeProvider(), buildFakeProvider()]; - jest.spyOn(fakeProviders[0], 'stop'); - const fakeNetworkClients = [ - buildFakeClient(fakeProviders[0]), - buildFakeClient(fakeProviders[1]), - ]; - createNetworkClientMock - .mockImplementationOnce(() => fakeNetworkClients[0]) - .mockImplementationOnce(() => fakeNetworkClients[1]); - - await controller.initializeProvider(); - await controller.initializeProvider(); - assert(controller.getProviderAndBlockTracker().provider); - jest.runAllTimers(); - - expect(fakeProviders[0].stop).toHaveBeenCalled(); - }, - ); - }); - - describe('when an "error" event occurs on the new provider', () => { - describe('if the network version could not be retrieved while providerConfig was being set', () => { - it('retrieves the network version twice more (due to the "error" event being listened to twice) and, assuming success, persists it to state', async () => { - await withController( - { - state: { - providerConfig: buildProviderConfig({ - type: NetworkType.rpc, - rpcUrl: 'http://example.com', - }), - }, - }, - async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider([ - { - request: { - method: 'net_version', - }, - response: { - error: 'oops', - }, - }, - { - request: { - method: 'net_version', - }, - response: { - result: '1', - }, - }, - { - request: { - method: 'net_version', - }, - response: { - result: '2', - }, - }, - ]); - const fakeNetworkClient = buildFakeClient(fakeProvider); - createNetworkClientMock.mockReturnValue(fakeNetworkClient); - - await waitForPublishedEvents( - messenger, - 'NetworkController:providerConfigChange', - { - produceEvents: async () => { - await controller.initializeProvider(); - assert( - controller.getProviderAndBlockTracker().provider, - ); - }, - }, - ); - - await waitForStateChanges(messenger, { - propertyPath: ['networkId'], - count: 2, - produceStateChanges: () => { - controller - .getProviderAndBlockTracker() - .provider?.emit('error', { some: 'error' }); - }, - }); - expect(controller.state.networkId).toBe('2'); - }, - ); - }); - }); - - describe('if the network version could be retrieved while providerConfig was being set', () => { - it('does not retrieve the network version again', async () => { - await withController( - { - state: { - providerConfig: buildProviderConfig({ - type: NetworkType.rpc, - rpcUrl: 'http://example.com', - }), - }, - }, - async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider([ - { - request: { - method: 'net_version', - }, - response: { - result: '1', - }, - }, - { - request: { - method: 'net_version', - }, - response: { - result: '2', - }, - }, - ]); - const fakeNetworkClient = buildFakeClient(fakeProvider); - createNetworkClientMock.mockReturnValue(fakeNetworkClient); - - await waitForPublishedEvents( - messenger, - 'NetworkController:providerConfigChange', - { - produceEvents: async () => { - await controller.initializeProvider(); - assert( - controller.getProviderAndBlockTracker().provider, - ); - }, - }, - ); - - await waitForStateChanges(messenger, { - propertyPath: ['networkId'], - count: 0, - produceStateChanges: () => { - controller - .getProviderAndBlockTracker() - .provider?.emit('error', { some: 'error' }); - }, - }); - expect(controller.state.networkId).toBe('1'); - }, - ); - }); - }); - }); }); describe('if the provider config does not contain an RPC URL', () => { @@ -3109,27 +2786,6 @@ describe('NetworkController', () => { }); }); - it('ensures that the existing provider is stopped while replacing it', async () => { - await withController(async ({ controller }) => { - const fakeProviders = [buildFakeProvider(), buildFakeProvider()]; - jest.spyOn(fakeProviders[0], 'stop'); - const fakeNetworkClients = [ - buildFakeClient(fakeProviders[0]), - buildFakeClient(fakeProviders[1]), - ]; - createNetworkClientMock - .mockImplementationOnce(() => fakeNetworkClients[0]) - .mockImplementationOnce(() => fakeNetworkClients[1]); - - await controller.setProviderType(networkType); - await controller.setProviderType(networkType); - assert(controller.getProviderAndBlockTracker().provider); - jest.runAllTimers(); - - expect(fakeProviders[0].stop).toHaveBeenCalled(); - }); - }); - it('updates the version of the current network in state (assuming that the request for net_version is made successfully)', async () => { await withController(async ({ controller }) => { const fakeProvider = buildFakeProvider([ @@ -3151,87 +2807,6 @@ describe('NetworkController', () => { expect(controller.state.networkId).toBe('42'); }); }); - - describe('when an "error" event occurs on the new provider', () => { - describe('if the network version could not be retrieved during setProviderType', () => { - it('retrieves the network version again and, assuming success, persists it to state', async () => { - await withController(async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider([ - { - request: { - method: 'net_version', - }, - response: { - error: 'oops', - }, - }, - { - request: { - method: 'net_version', - }, - response: { - result: '42', - }, - }, - ]); - const fakeNetworkClient = buildFakeClient(fakeProvider); - createNetworkClientMock.mockReturnValue(fakeNetworkClient); - - await controller.setProviderType(networkType); - - await waitForStateChanges(messenger, { - propertyPath: ['networkId'], - produceStateChanges: () => { - controller - .getProviderAndBlockTracker() - .provider?.emit('error', { some: 'error' }); - }, - }); - expect(controller.state.networkId).toBe('42'); - }); - }); - }); - - describe('if the network version could be retrieved during setProviderType', () => { - it('does not retrieve the network version again', async () => { - await withController(async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider([ - { - request: { - method: 'net_version', - }, - response: { - result: '1', - }, - }, - { - request: { - method: 'net_version', - }, - response: { - result: '2', - }, - }, - ]); - const fakeNetworkClient = buildFakeClient(fakeProvider); - createNetworkClientMock.mockReturnValue(fakeNetworkClient); - - await controller.setProviderType(networkType); - - await waitForStateChanges(messenger, { - propertyPath: ['networkId'], - count: 0, - produceStateChanges: () => { - controller - .getProviderAndBlockTracker() - .provider?.emit('error', { some: 'error' }); - }, - }); - expect(controller.state.networkId).toBe('1'); - }); - }); - }); - }); }); } @@ -3250,10 +2825,6 @@ describe('NetworkController', () => { }, }, async ({ controller }) => { - const fakeProvider = buildFakeProvider(); - const fakeNetworkClient = buildFakeClient(fakeProvider); - createNetworkClientMock.mockReturnValue(fakeNetworkClient); - await expect(() => controller.setProviderType(NetworkType.rpc), ).rejects.toThrow( @@ -3399,7 +2970,7 @@ describe('NetworkController', () => { await controller.setActiveNetwork('testNetworkConfigurationId'); expect(createNetworkClientMock).toHaveBeenCalledWith({ - chainId: '1337', + chainId: toHex(1337), rpcUrl: 'https://mock-rpc-url', type: NetworkClientType.Custom, }); @@ -3461,43 +3032,6 @@ describe('NetworkController', () => { ); }); - it('ensures that the existing provider is stopped while replacing it', async () => { - await withController( - { - state: { - networkConfigurations: { - testNetworkConfigurationId: { - rpcUrl: 'https://mock-rpc-url', - chainId: '1337', - ticker: 'TEST', - id: 'testNetworkConfigurationId', - nickname: undefined, - rpcPrefs: undefined, - }, - }, - }, - }, - async ({ controller }) => { - const fakeProviders = [buildFakeProvider(), buildFakeProvider()]; - jest.spyOn(fakeProviders[0], 'stop'); - const fakeNetworkClients = [ - buildFakeClient(fakeProviders[0]), - buildFakeClient(fakeProviders[1]), - ]; - createNetworkClientMock - .mockImplementationOnce(() => fakeNetworkClients[0]) - .mockImplementationOnce(() => fakeNetworkClients[1]); - - await controller.setActiveNetwork('testNetworkConfigurationId'); - await controller.setActiveNetwork('testNetworkConfigurationId'); - assert(controller.getProviderAndBlockTracker().provider); - jest.runAllTimers(); - - expect(fakeProviders[0].stop).toHaveBeenCalled(); - }, - ); - }); - it('updates the version of the current network in state (assuming that the request for net_version is made successfully)', async () => { await withController( { @@ -3645,119 +3179,6 @@ describe('NetworkController', () => { ); }); }); - - describe('when an "error" event occurs on the new provider', () => { - describe('if the network version could not be retrieved during the call to setActiveNetwork', () => { - it('retrieves the network version again and, assuming success, persists it to state', async () => { - await withController( - { - state: { - networkConfigurations: { - testNetworkConfigurationId: { - rpcUrl: 'https://mock-rpc-url', - chainId: '1337', - ticker: 'TEST', - id: 'testNetworkConfigurationId', - nickname: undefined, - rpcPrefs: undefined, - }, - }, - }, - }, - async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider([ - { - request: { - method: 'net_version', - }, - response: { - error: 'oops', - }, - }, - { - request: { - method: 'net_version', - }, - response: { - result: '42', - }, - }, - ]); - const fakeNetworkClient = buildFakeClient(fakeProvider); - createNetworkClientMock.mockReturnValue(fakeNetworkClient); - - await controller.setActiveNetwork('testNetworkConfigurationId'); - - await waitForStateChanges(messenger, { - propertyPath: ['networkId'], - produceStateChanges: () => { - controller - .getProviderAndBlockTracker() - .provider?.emit('error', { some: 'error' }); - }, - }); - expect(controller.state.networkId).toBe('42'); - }, - ); - }); - }); - - describe('if the network version could be retrieved during the call to setActiveNetwork', () => { - it('does not retrieve the network version again', async () => { - await withController( - { - state: { - networkConfigurations: { - testNetworkConfigurationId: { - rpcUrl: 'https://mock-rpc-url', - chainId: '1337', - ticker: 'TEST', - id: 'testNetworkConfigurationId', - nickname: undefined, - rpcPrefs: undefined, - }, - }, - }, - }, - async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider([ - { - request: { - method: 'net_version', - }, - response: { - result: '1', - }, - }, - { - request: { - method: 'net_version', - }, - response: { - result: '2', - }, - }, - ]); - const fakeNetworkClient = buildFakeClient(fakeProvider); - createNetworkClientMock.mockReturnValue(fakeNetworkClient); - - await controller.setActiveNetwork('testNetworkConfigurationId'); - - await waitForStateChanges(messenger, { - propertyPath: ['networkId'], - count: 0, - produceStateChanges: () => { - controller - .getProviderAndBlockTracker() - .provider?.emit('error', { some: 'error' }); - }, - }); - expect(controller.state.networkId).toBe('1'); - }, - ); - }); - }); - }); }); describe('getEIP1559Compatibility', () => { @@ -4721,42 +4142,6 @@ describe('NetworkController', () => { ); }); - it('ensures that the existing provider is stopped while replacing it', async () => { - await withController( - { - state: { - providerConfig: { - type: networkType, - // NOTE: This doesn't need to match the logical chain ID of - // the network selected, it just needs to exist - chainId: '9999999', - }, - }, - }, - async ({ controller }) => { - const fakeProviders = [ - buildFakeProvider(), - buildFakeProvider(), - ]; - jest.spyOn(fakeProviders[0], 'stop'); - const fakeNetworkClients = [ - buildFakeClient(fakeProviders[0]), - buildFakeClient(fakeProviders[1]), - ]; - createNetworkClientMock - .mockImplementationOnce(() => fakeNetworkClients[0]) - .mockImplementationOnce(() => fakeNetworkClients[1]); - - await controller.resetConnection(); - await controller.resetConnection(); - assert(controller.getProviderAndBlockTracker().provider); - jest.runAllTimers(); - - expect(fakeProviders[0].stop).toHaveBeenCalled(); - }, - ); - }); - it('checks the status of the network again, updating state appropriately', async () => { await withController( { @@ -4821,51 +4206,6 @@ describe('NetworkController', () => { }, ); }); - - describe('when an "error" event occurs on the new provider', () => { - it('retrieves the network version and, assuming success, persists it to state', async () => { - await withController( - { - state: { - providerConfig: { - type: networkType, - // NOTE: This doesn't need to match the logical chain ID of - // the network selected, it just needs to exist - chainId: '9999999', - }, - }, - }, - async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider([ - { - request: { - method: 'net_version', - }, - response: { - result: '42', - }, - }, - ]); - const fakeNetworkClient = buildFakeClient(fakeProvider); - createNetworkClientMock.mockReturnValue(fakeNetworkClient); - - const resetPromise = controller.resetConnection(); - - await waitForStateChanges(messenger, { - propertyPath: ['networkId'], - produceStateChanges: () => { - controller - .getProviderAndBlockTracker() - .provider?.emit('error', { some: 'error' }); - }, - }); - await resetPromise; - - expect(controller.state.networkId).toBe('42'); - }, - ); - }); - }); }); }, ); @@ -5083,40 +4423,6 @@ describe('NetworkController', () => { ); }); - it('ensures that the existing provider is stopped while replacing it', async () => { - await withController( - { - state: { - providerConfig: { - type: NetworkType.rpc, - rpcUrl: 'https://mock-rpc-url', - chainId: '1337', - ticker: 'TEST', - id: 'testNetworkConfigurationId', - }, - }, - }, - async ({ controller }) => { - const fakeProviders = [buildFakeProvider(), buildFakeProvider()]; - jest.spyOn(fakeProviders[0], 'stop'); - const fakeNetworkClients = [ - buildFakeClient(fakeProviders[0]), - buildFakeClient(fakeProviders[1]), - ]; - createNetworkClientMock - .mockImplementationOnce(() => fakeNetworkClients[0]) - .mockImplementationOnce(() => fakeNetworkClients[1]); - - await controller.resetConnection(); - await controller.resetConnection(); - assert(controller.getProviderAndBlockTracker().provider); - jest.runAllTimers(); - - expect(fakeProviders[0].stop).toHaveBeenCalled(); - }, - ); - }); - it('checks the status of the network again, updating state appropriately', async () => { await withController( { @@ -5239,52 +4545,6 @@ describe('NetworkController', () => { ); }); }); - - describe('when an "error" event occurs on the new provider', () => { - it('retrieves the network version and, assuming success, persists it to state', async () => { - await withController( - { - state: { - providerConfig: { - type: NetworkType.rpc, - rpcUrl: 'https://mock-rpc-url', - chainId: '1337', - ticker: 'TEST', - id: 'testNetworkConfigurationId', - }, - }, - }, - async ({ controller, messenger }) => { - const fakeProvider = buildFakeProvider([ - { - request: { - method: 'net_version', - }, - response: { - result: '42', - }, - }, - ]); - const fakeNetworkClient = buildFakeClient(fakeProvider); - createNetworkClientMock.mockReturnValue(fakeNetworkClient); - - const resetPromise = controller.resetConnection(); - - await waitForStateChanges(messenger, { - propertyPath: ['networkId'], - produceStateChanges: () => { - controller - .getProviderAndBlockTracker() - .provider?.emit('error', { some: 'error' }); - }, - }); - await resetPromise; - - expect(controller.state.networkId).toBe('42'); - }, - ); - }); - }); }); }); @@ -5543,7 +4803,7 @@ describe('NetworkController', () => { networkConfigurations: { testNetworkConfigurationId: { rpcUrl: 'https://mock-rpc-url', - chainId: '0xtest', + chainId: '0x111', ticker: 'TEST', id: 'testNetworkConfigurationId', nickname: undefined, @@ -6225,6 +5485,7 @@ describe('NetworkController', () => { ]); const fakeNetworkClient = buildFakeClient(fakeProvider); createNetworkClientMock.mockReturnValue(fakeNetworkClient); + await controller.setActiveNetwork('testNetworkConfigurationId'); expect(controller.state.networkDetails).toStrictEqual({ isEIP1559Compatible: true, @@ -6340,7 +5601,6 @@ describe('NetworkController', () => { }, async ({ controller }) => { const fakeProvider = buildFakeProvider(); - const fakeNetworkClient = buildFakeClient(fakeProvider); createNetworkClientMock.mockReturnValue(fakeNetworkClient); await controller.setActiveNetwork('testNetworkConfigurationId'); @@ -7035,7 +6295,8 @@ async function withController( try { return await fn({ controller, messenger }); } finally { - controller.getProviderAndBlockTracker().provider?.stop(); + const { blockTracker } = controller.getProviderAndBlockTracker(); + blockTracker?.destroy(); } } @@ -7074,11 +6335,13 @@ function buildFakeClient(provider: Provider) { } /** - * Builds fake provider engine object that `createMetamaskProvider` returns, - * with canned responses optionally provided for certain RPC methods. + * Builds an object that fits the same shape as the object that the + * `@metamask/eth-json-rpc-provider` package builds, with canned responses + * optionally provided for certain RPC methods. * * @param stubs - The list of RPC methods you want to stub along with their - * responses. + * responses. `eth_getBlockByNumber` and `net_version` will be stubbed by + * default. * @returns The object. */ function buildFakeProvider(stubs: FakeProviderStub[] = []): Provider { @@ -7102,7 +6365,7 @@ function buildFakeProvider(stubs: FakeProviderStub[] = []): Provider { discardAfterMatching: false, }); } - return new FakeProviderEngine({ stubs: completeStubs }); + return new FakeProvider({ stubs: completeStubs }); } /** diff --git a/packages/network-controller/tests/fake-provider-engine.ts b/packages/network-controller/tests/fake-provider.ts similarity index 66% rename from packages/network-controller/tests/fake-provider-engine.ts rename to packages/network-controller/tests/fake-provider.ts index 18478957ed..c6c11520bd 100644 --- a/packages/network-controller/tests/fake-provider-engine.ts +++ b/packages/network-controller/tests/fake-provider.ts @@ -1,21 +1,14 @@ import { inspect, isDeepStrictEqual } from 'util'; -import EventEmitter from 'events'; -import type { - ProviderEngine, - RpcPayload, - RpcResponse, -} from 'web3-provider-engine'; -import { serializeError } from 'eth-rpc-errors'; -import { FakeBlockTracker } from '../../../tests/fake-block-tracker'; +import { + JsonRpcEngine, + JsonRpcRequest, + JsonRpcResponse, +} from 'json-rpc-engine'; +import { SafeEventEmitterProvider } from '@metamask/eth-json-rpc-provider/dist/safe-event-emitter-provider'; // Store this in case it gets stubbed later const originalSetTimeout = global.setTimeout; -/** - * The payload that `sendAsync` takes. - */ -type SendAsyncPayload

= RpcPayload

| RpcPayload

[]; - /** * An object that allows specifying the behavior of a specific invocation of * `sendAsync`. The `method` always identifies the stub, but the behavior @@ -49,7 +42,7 @@ type SendAsyncPayload

= RpcPayload

| RpcPayload

[]; export type FakeProviderStub = { request: { method: string; - params?: (string | boolean)[]; + params?: unknown[]; }; delay?: number; discardAfterMatching?: boolean; @@ -59,7 +52,7 @@ export type FakeProviderStub = { response: { result: any } | { error: string }; } | { - error: string | Error; + error: unknown; } | { implementation: () => void; @@ -67,7 +60,7 @@ export type FakeProviderStub = { ); /** - * The set of options that the FakeProviderEngine constructor takes. + * The set of options that the FakeProvider constructor takes. * * @property stubs - A set of objects that allow specifying the behavior * of specific invocations of `sendAsync` matching a `method`. @@ -77,27 +70,23 @@ interface FakeProviderEngineOptions { } /** - * FakeProviderEngine is an implementation of the provider that - * NetworkController exposes, which is actually an instance of - * Web3ProviderEngine (from the `web3-provider-engine` package). Hence it - * supports the same interface as Web3ProviderEngine, except that fake responses - * for any RPC methods that are accessed can be supplied via an API that is more - * succinct than using Jest's mocking API. + * An implementation of the provider that NetworkController exposes, which is + * actually an instance of SafeEventEmitterProvider (from the + * `@metamask/eth-json-rpc-provider` package). Hence it supports the same + * interface as SafeEventEmitterProvider, except that fake responses for any RPC + * methods that are accessed can be supplied via an API that is more succinct + * than using Jest's mocking API. */ -export class FakeProviderEngine extends EventEmitter implements ProviderEngine { +// NOTE: We shouldn't need to extend from the "real" provider here, but +// we'd need a `SafeEventEmitterProvider` _interface_ and that doesn't exist (at +// least not yet). +export class FakeProvider extends SafeEventEmitterProvider { calledStubs: FakeProviderStub[]; #originalStubs: FakeProviderStub[]; #stubs: FakeProviderStub[]; - #isStopped: boolean; - - // NOTE: This is a "private" property defined on ProviderEngine that holds the - // block tracker, so we have to provide that in this fake implementation as - // well - _blockTracker: FakeBlockTracker; - /** * Makes a new instance of the fake provider. * @@ -106,21 +95,29 @@ export class FakeProviderEngine extends EventEmitter implements ProviderEngine { * of specific invocations of `sendAsync` matching a `method`. */ constructor({ stubs = [] }: FakeProviderEngineOptions) { - super(); + super({ engine: new JsonRpcEngine() }); this.#originalStubs = stubs; this.#stubs = this.#originalStubs.slice(); this.calledStubs = []; - this.#isStopped = false; - this._blockTracker = new FakeBlockTracker(); } - stop() { - this.#isStopped = true; - } + send = ( + payload: JsonRpcRequest, + callback: (error: unknown, response?: JsonRpcResponse) => void, + ) => { + return this.#handleSend(payload, callback); + }; + + sendAsync = ( + payload: JsonRpcRequest, + callback: (error: unknown, response?: JsonRpcResponse) => void, + ) => { + return this.#handleSend(payload, callback); + }; - sendAsync( - payload: SendAsyncPayload

, - callback: (error: unknown, response: RpcResponse, V>) => void, + #handleSend( + payload: JsonRpcRequest, + callback: (error: unknown, response?: JsonRpcResponse) => void, ) { if (Array.isArray(payload)) { throw new Error("Arrays aren't supported"); @@ -134,31 +131,7 @@ export class FakeProviderEngine extends EventEmitter implements ProviderEngine { ); }); - if (this.#isStopped) { - console.warn(`The provider has been stopped, yet sendAsync has somehow been called. If this -were not a fake provider, it's likely nothing would happen and the request would -be sent anyway, but this probably means you have a test that ran an asynchronous -operation that was not fulfilled before the test ended.`); - return; - } - - if (index !== -1) { - const stub = this.#stubs[index]; - - if (stub.discardAfterMatching !== false) { - this.#stubs.splice(index, 1); - } - - if (stub.delay) { - originalSetTimeout(() => { - this.#handleRequest(stub, callback); - }, stub.delay); - } else { - this.#handleRequest(stub, callback); - } - - this.calledStubs.push({ ...stub }); - } else { + if (index === -1) { const matchingCalledStubs = this.calledStubs.filter((stub) => { return ( stub.request.method === payload.method && @@ -177,12 +150,28 @@ operation that was not fulfilled before the test ended.`); } throw new Error(message); + } else { + const stub = this.#stubs[index]; + + if (stub.discardAfterMatching !== false) { + this.#stubs.splice(index, 1); + } + + if (stub.delay) { + originalSetTimeout(() => { + this.#handleRequest(stub, callback); + }, stub.delay); + } else { + this.#handleRequest(stub, callback); + } + + this.calledStubs.push({ ...stub }); } } - async #handleRequest( + async #handleRequest( stub: FakeProviderStub, - callback: (error: unknown, response: RpcResponse, V>) => void, + callback: (error: unknown, response?: JsonRpcResponse) => void, ) { if (stub.beforeCompleting) { await stub.beforeCompleting(); @@ -201,7 +190,6 @@ operation that was not fulfilled before the test ended.`); return callback(null, { jsonrpc: '2.0', id: 1, - result: undefined, error: { code: -999, message: stub.response.error, @@ -209,25 +197,9 @@ operation that was not fulfilled before the test ended.`); }); } } else if ('error' in stub) { - let error; - let serializedError; - if (typeof stub.error === 'string') { - error = new Error(stub.error); - serializedError = { - code: -999, - message: stub.error, - }; - } else { - error = stub.error; - serializedError = serializeError(error); - } - return callback(error, { - jsonrpc: '2.0', - id: 1, - result: undefined, - error: serializedError, - }); + return callback(stub.error); } + return undefined; } } diff --git a/packages/network-controller/tests/provider-api-tests/block-hash-in-response.ts b/packages/network-controller/tests/provider-api-tests/block-hash-in-response.ts index 775755c7bb..0583b3e419 100644 --- a/packages/network-controller/tests/provider-api-tests/block-hash-in-response.ts +++ b/packages/network-controller/tests/provider-api-tests/block-hash-in-response.ts @@ -1,6 +1,5 @@ import { ProviderType, - waitForNextBlockTracker, withMockedCommunications, withNetworkClient, } from './helpers'; @@ -25,8 +24,8 @@ type TestsForRpcMethodThatCheckForBlockHashInResponseOptions = { export function testsForRpcMethodsThatCheckForBlockHashInResponse( method: string, { - providerType, numberOfParameters, + providerType, }: TestsForRpcMethodThatCheckForBlockHashInResponseOptions, ) { it('does not hit the RPC endpoint more than once for identical requests and it has a valid blockHash', async () => { @@ -34,6 +33,9 @@ export function testsForRpcMethodsThatCheckForBlockHashInResponse( const mockResult = { blockHash: '0x1' }; await withMockedCommunications({ providerType }, async (comms) => { + // The first time a block-cacheable request is made, the latest block + // number is retrieved through the block tracker first. It doesn't + // matter what this is — it's just used as a cache key. comms.mockNextBlockTrackerRequest(); comms.mockRpcCall({ request: requests[0], @@ -54,6 +56,11 @@ export function testsForRpcMethodsThatCheckForBlockHashInResponse( const mockResults = [{ blockHash: '0x100' }, { blockHash: '0x200' }]; await withMockedCommunications({ providerType }, async (comms) => { + // Note that we have to mock these requests in a specific order. The + // first block tracker request occurs because of the first RPC + // request. The second block tracker request, however, does not occur + // because of the second RPC request, but rather because we call + // `clock.runAll()` below. comms.mockNextBlockTrackerRequest({ blockNumber: '0x1' }); comms.mockRpcCall({ request: requests[0], @@ -69,10 +76,10 @@ export function testsForRpcMethodsThatCheckForBlockHashInResponse( { providerType }, async (client) => { const firstResult = await client.makeRpcCall(requests[0]); - await waitForNextBlockTracker(client.blockTracker, client.clock); - + // Proceed to the next iteration of the block tracker so that a new + // block is fetched and the current block is updated. + client.clock.runAll(); const secondResult = await client.makeRpcCall(requests[1]); - return [firstResult, secondResult]; }, ); @@ -97,7 +104,36 @@ export function testsForRpcMethodsThatCheckForBlockHashInResponse( request: requests[0], response: { result: mockResults[0] }, }); + comms.mockRpcCall({ + request: requests[1], + response: { result: mockResults[1] }, + }); + + const results = await withNetworkClient( + { providerType }, + ({ makeRpcCallsInSeries }) => makeRpcCallsInSeries(requests), + ); + + expect(results).toStrictEqual(mockResults); + }); + }); + it('does not reuse the result of a previous request if result.blockHash was undefined', async () => { + const requests = [{ method }, { method }]; + const mockResults = [ + { extra: 'some value' }, + { blockHash: '0x100', extra: 'some other value' }, + ]; + + await withMockedCommunications({ providerType }, async (comms) => { + // The first time a block-cacheable request is made, the latest block + // number is retrieved through the block tracker first. It doesn't + // matter what this is — it's just used as a cache key. + comms.mockNextBlockTrackerRequest(); + comms.mockRpcCall({ + request: requests[0], + response: { result: mockResults[0] }, + }); comms.mockRpcCall({ request: requests[1], response: { result: mockResults[1] }, @@ -132,7 +168,6 @@ export function testsForRpcMethodsThatCheckForBlockHashInResponse( request: requests[0], response: { result: mockResults[0] }, }); - comms.mockRpcCall({ request: requests[1], response: { result: mockResults[1] }, @@ -224,7 +259,6 @@ export function testsForRpcMethodsThatCheckForBlockHashInResponse( request: requests[0], response: { result: mockResults[0] }, }); - comms.mockRpcCall({ request: requests[1], response: { result: mockResults[1] }, diff --git a/packages/network-controller/tests/provider-api-tests/block-param.ts b/packages/network-controller/tests/provider-api-tests/block-param.ts index fe09de15a6..45ae2b3fc5 100644 --- a/packages/network-controller/tests/provider-api-tests/block-param.ts +++ b/packages/network-controller/tests/provider-api-tests/block-param.ts @@ -2,7 +2,6 @@ import { buildMockParams, buildRequestWithReplacedBlockParam, ProviderType, - waitForNextBlockTracker, waitForPromiseToBeFulfilledAfterRunningAllTimers, withMockedCommunications, withNetworkClient, @@ -10,6 +9,7 @@ import { import { buildFetchFailedErrorMessage, buildInfuraClientRetriesExhaustedErrorMessage, + buildJsonRpcEngineEmptyResponseErrorMessage, } from './shared-tests'; type TestsForRpcMethodSupportingBlockParam = { @@ -57,13 +57,19 @@ export function testsForRpcMethodSupportingBlockParam( await withMockedCommunications({ providerType }, async (comms) => { // The first time a block-cacheable request is made, the block-cache // middleware will request the latest block number through the block - // tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // tracker to determine the cache key. Later, the block-ref + // middleware will request the latest block number again to resolve + // the value of "latest", but the block number is cached once made, + // so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( requests[0], blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: mockResults[0] }, }); @@ -82,7 +88,6 @@ export function testsForRpcMethodSupportingBlockParam( // testing changes in block param is covered under later tests continue; } - it(`does not reuse the result of a previous request if parameter at index "${paramIndex}" differs`, async () => { const firstMockParams = [ ...new Array(numberOfParameters).fill('some value'), @@ -102,22 +107,27 @@ export function testsForRpcMethodSupportingBlockParam( await withMockedCommunications({ providerType }, async (comms) => { // The first time a block-cacheable request is made, the block-cache // middleware will request the latest block number through the block - // tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // tracker to determine the cache key. Later, the block-ref + // middleware will request the latest block number again to resolve + // the value of "latest", but the block number is cached once made, + // so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( requests[0], blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: mockResults[0] }, }); - comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( requests[1], blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: mockResults[1] }, }); @@ -146,11 +156,14 @@ export function testsForRpcMethodSupportingBlockParam( // occur because of the second RPC request, but rather because we // call `clock.runAll()` below. comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( requests[0], blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: mockResults[0] }, }); @@ -159,7 +172,7 @@ export function testsForRpcMethodSupportingBlockParam( request: buildRequestWithReplacedBlockParam( requests[1], blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x200', ), response: { result: mockResults[1] }, }); @@ -170,7 +183,7 @@ export function testsForRpcMethodSupportingBlockParam( const firstResult = await client.makeRpcCall(requests[0]); // Proceed to the next iteration of the block tracker so that a // new block is fetched and the current block is updated. - await waitForNextBlockTracker(client.blockTracker, client.clock); + client.clock.runAll(); const secondResult = await client.makeRpcCall(requests[1]); return [firstResult, secondResult]; }, @@ -191,13 +204,19 @@ export function testsForRpcMethodSupportingBlockParam( await withMockedCommunications({ providerType }, async (comms) => { // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: mockResult }, }); @@ -221,13 +240,19 @@ export function testsForRpcMethodSupportingBlockParam( await withMockedCommunications({ providerType }, async (comms) => { // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( requests[0], blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: mockResults[0] }, }); @@ -235,7 +260,7 @@ export function testsForRpcMethodSupportingBlockParam( request: buildRequestWithReplacedBlockParam( requests[1], blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: mockResults[1] }, }); @@ -252,44 +277,46 @@ export function testsForRpcMethodSupportingBlockParam( it('queues requests while a previous identical call is still pending, then runs the queue when it finishes, reusing the result from the first request', async () => { const requests = [ - { method, params: buildMockParams({ blockParam, blockParamIndex }) }, - { method, params: buildMockParams({ blockParam, blockParamIndex }) }, - { method, params: buildMockParams({ blockParam, blockParamIndex }) }, + { method, params: buildMockParams({ blockParamIndex, blockParam }) }, + { method, params: buildMockParams({ blockParamIndex, blockParam }) }, + { method, params: buildMockParams({ blockParamIndex, blockParam }) }, ]; const mockResults = ['first result', 'second result', 'third result']; await withMockedCommunications({ providerType }, async (comms) => { // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); - // A second block tracker request is made for some reason - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number, and we delay it. comms.mockRpcCall({ delay: 100, request: buildRequestWithReplacedBlockParam( requests[0], blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: mockResults[0] }, }); - // The previous two requests will happen again, in the same order. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( requests[1], blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: mockResults[1] }, }); - comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( requests[2], blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: mockResults[2] }, }); @@ -321,20 +348,23 @@ export function testsForRpcMethodSupportingBlockParam( it('throws an error with a custom message if the request to the RPC endpoint returns a 405 response', async () => { await withMockedCommunications({ providerType }, async (comms) => { - const request = { - method, - params: buildMockParams({ blockParam, blockParamIndex }), - }; + const request = { method }; // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { httpStatus: 405, @@ -367,13 +397,19 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { httpStatus: 418, @@ -399,13 +435,19 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { httpStatus: 429, @@ -431,13 +473,19 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { httpStatus: 418, @@ -463,13 +511,19 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { httpStatus: 429, @@ -489,23 +543,27 @@ export function testsForRpcMethodSupportingBlockParam( it('throws an undescriptive error message if the request to the RPC endpoint returns a response that is not 405, 418, 429, 503, or 504', async () => { await withMockedCommunications({ providerType }, async (comms) => { - const request = { - method, - params: buildMockParams({ blockParam, blockParamIndex }), - }; + const request = { method }; // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { id: 12345, + jsonrpc: '2.0', error: 'some error', httpStatus: 420, }, @@ -533,15 +591,22 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. + // // Here we have the request fail for the first 4 tries, then succeed // on the 5th try. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { error: 'some error', @@ -549,12 +614,11 @@ export function testsForRpcMethodSupportingBlockParam( }, times: 4, }); - comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: 'the result', @@ -575,10 +639,10 @@ export function testsForRpcMethodSupportingBlockParam( }); }); - // Both the Infura middleware and custom RPC middleware detect a 503 or 504 - // response and retry the request to the RPC endpoint automatically but - // differ in what sort of response is returned when the number of retries is - // exhausted. + // Both the Infura middleware and custom RPC middleware detect a 503 or + // 504 response and retry the request to the RPC endpoint automatically + // but differ in what sort of response is returned when the number of + // retries is exhausted. if (providerType === 'infura') { it(`causes a request to fail with a custom error if the request to the RPC endpoint returns a ${httpStatus} response 5 times in a row`, async () => { await withMockedCommunications({ providerType }, async (comms) => { @@ -589,13 +653,19 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { error: 'Some error', @@ -618,7 +688,7 @@ export function testsForRpcMethodSupportingBlockParam( }); }); } else { - it(`produces a response without a result if the request to the RPC endpoint returns a ${httpStatus} response 5 times in a row`, async () => { + it(`produces an empty response if the request to the RPC endpoint returns a ${httpStatus} response 5 times in a row`, async () => { await withMockedCommunications({ providerType }, async (comms) => { const request = { method, @@ -627,13 +697,19 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { error: 'Some error', @@ -641,7 +717,7 @@ export function testsForRpcMethodSupportingBlockParam( }, times: 5, }); - const result = await withNetworkClient( + const promiseForResult = withNetworkClient( { providerType }, async ({ makeRpcCall, clock }) => { return await waitForPromiseToBeFulfilledAfterRunningAllTimers( @@ -650,7 +726,9 @@ export function testsForRpcMethodSupportingBlockParam( ); }, ); - expect(result).toBeUndefined(); + await expect(promiseForResult).rejects.toThrow( + buildJsonRpcEngineEmptyResponseErrorMessage(method), + ); }); }); } @@ -665,25 +743,31 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. + // // Here we have the request fail for the first 4 tries, then // succeed on the 5th try. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: 'ETIMEDOUT: Some message', times: 4, }); - comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: 'the result', @@ -711,22 +795,25 @@ export function testsForRpcMethodSupportingBlockParam( if (providerType === 'infura') { it('causes a request to fail with a custom error if an "ETIMEDOUT" error is thrown while making the request to the RPC endpoint 5 times in a row', async () => { await withMockedCommunications({ providerType }, async (comms) => { - const request = { - method, - params: buildMockParams({ blockParam, blockParamIndex }), - }; + const request = { method }; const errorMessage = 'ETIMEDOUT: Some message'; // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: errorMessage, times: 5, @@ -748,7 +835,7 @@ export function testsForRpcMethodSupportingBlockParam( }); }); } else { - it('produces a response without a result if an "ETIMEDOUT" error is thrown while making the request to the RPC endpoint 5 times in a row', async () => { + it('produces an empty response if an "ETIMEDOUT" error is thrown while making the request to the RPC endpoint 5 times in a row', async () => { await withMockedCommunications({ providerType }, async (comms) => { const request = { method, @@ -758,20 +845,26 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: errorMessage, times: 5, }); - const result = await withNetworkClient( + const promiseForResult = withNetworkClient( { providerType }, async ({ makeRpcCall, clock }) => { return await waitForPromiseToBeFulfilledAfterRunningAllTimers( @@ -781,7 +874,9 @@ export function testsForRpcMethodSupportingBlockParam( }, ); - expect(result).toBeUndefined(); + await expect(promiseForResult).rejects.toThrow( + buildJsonRpcEngineEmptyResponseErrorMessage(method), + ); }); }); } @@ -800,25 +895,31 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. + // // Here we have the request fail for the first 4 tries, then // succeed on the 5th try. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: 'ECONNRESET: Some message', times: 4, }); - comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: 'the result', @@ -850,13 +951,19 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: errorMessage, times: 5, @@ -892,13 +999,19 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: errorMessage, }); @@ -933,25 +1046,31 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. + // // Here we have the request fail for the first 4 tries, then // succeed on the 5th try. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: 'SyntaxError: Some message', times: 4, }); - comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: 'the result', @@ -982,13 +1101,19 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: errorMessage, times: 5, @@ -1020,13 +1145,19 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: errorMessage, }); @@ -1052,18 +1183,23 @@ export function testsForRpcMethodSupportingBlockParam( method, params: buildMockParams({ blockParam, blockParamIndex }), }; - const errorMessage = 'SyntaxError: Some message'; // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: errorMessage, }); @@ -1089,25 +1225,31 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. + // // Here we have the request fail for the first 4 tries, then // succeed on the 5th try. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: 'failed to parse response body: Some message', times: 4, }); - comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: 'the result', @@ -1129,7 +1271,7 @@ export function testsForRpcMethodSupportingBlockParam( }); }); - it('produces a response without a result if a "failed to parse response body" error is thrown while making the request to the RPC endpoint 5 times in a row', async () => { + it('produces an empty response if a "failed to parse response body" error is thrown while making the request to the RPC endpoint 5 times in a row', async () => { await withMockedCommunications({ providerType }, async (comms) => { const request = { method, @@ -1139,18 +1281,24 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: errorMessage, times: 5, }); - const result = await withNetworkClient( + const promiseForResult = withNetworkClient( { providerType }, async ({ makeRpcCall, clock }) => { return await waitForPromiseToBeFulfilledAfterRunningAllTimers( @@ -1160,7 +1308,9 @@ export function testsForRpcMethodSupportingBlockParam( }, ); - expect(result).toBeUndefined(); + await expect(promiseForResult).rejects.toThrow( + buildJsonRpcEngineEmptyResponseErrorMessage(method), + ); }); }); } @@ -1179,13 +1329,19 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: errorMessage, }); @@ -1210,25 +1366,31 @@ export function testsForRpcMethodSupportingBlockParam( // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. + // // Here we have the request fail for the first 4 tries, then // succeed on the 5th try. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: 'Failed to fetch: Some message', times: 4, }); - comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), response: { result: 'the result', @@ -1250,28 +1412,31 @@ export function testsForRpcMethodSupportingBlockParam( }); }); - it('produces a response without a result if a "Failed to fetch" error is thrown while making the request to the RPC endpoint 5 times in a row', async () => { + it('produces an empty response if a "Failed to fetch" error is thrown while making the request to the RPC endpoint 5 times in a row', async () => { await withMockedCommunications({ providerType }, async (comms) => { - const request = { - method, - params: buildMockParams({ blockParam, blockParamIndex }), - }; + const request = { method }; const errorMessage = 'Failed to fetch: some message'; // The first time a block-cacheable request is made, the // block-cache middleware will request the latest block number - // through the block tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // through the block tracker to determine the cache key. Later, + // the block-ref middleware will request the latest block number + // again to resolve the value of "latest", but the block number is + // cached once made, so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: buildRequestWithReplacedBlockParam( request, blockParamIndex, - blockParam === undefined ? null : 'latest', + '0x100', ), error: errorMessage, times: 5, }); - const result = await withNetworkClient( + const promiseForResult = withNetworkClient( { providerType }, async ({ makeRpcCall, clock }) => { return await waitForPromiseToBeFulfilledAfterRunningAllTimers( @@ -1281,7 +1446,9 @@ export function testsForRpcMethodSupportingBlockParam( }, ); - expect(result).toBeUndefined(); + await expect(promiseForResult).rejects.toThrow( + buildJsonRpcEngineEmptyResponseErrorMessage(method), + ); }); }); } @@ -1291,7 +1458,9 @@ export function testsForRpcMethodSupportingBlockParam( ['given a block tag of "earliest"', 'earliest', 'earliest'], ['given a block number', 'block number', '0x100'], ])('%s', (_desc, blockParamType, blockParam) => { - it(`does not hit the RPC endpoint more than once for identical requests when block param is ${blockParam} and param type of ${blockParamType}`, async () => { + // This lint rule gets confused by `describe.each` + // eslint-disable-next-line jest/no-identical-title + it('does not hit the RPC endpoint more than once for identical requests', async () => { const requests = [ { method, @@ -1349,13 +1518,18 @@ export function testsForRpcMethodSupportingBlockParam( await withMockedCommunications({ providerType }, async (comms) => { // The first time a block-cacheable request is made, the block-cache // middleware will request the latest block number through the block - // tracker to determine the cache key. - comms.mockNextBlockTrackerRequest(); + // tracker to determine the cache key. Later, the block-ref + // middleware will request the latest block number again to resolve + // the value of "latest", but the block number is cached once made, + // so we only need to mock the request once. + comms.mockNextBlockTrackerRequest({ blockNumber: '0x100' }); + // The block-ref middleware will make the request as specified + // except that the block param is replaced with the latest block + // number. comms.mockRpcCall({ request: requests[0], response: { result: mockResults[0] }, }); - comms.mockRpcCall({ request: requests[1], response: { result: mockResults[1] }, @@ -1537,7 +1711,6 @@ export function testsForRpcMethodSupportingBlockParam( request: requests[0], response: { result: 'first result' }, }); - comms.mockRpcCall({ request: requests[1], response: { result: 'second result' }, @@ -1552,59 +1725,27 @@ export function testsForRpcMethodSupportingBlockParam( }); }); - describe.each([ + for (const [nestedDesc, currentBlockNumber] of [ ['less than the current block number', '0x200'], - ['equal to the current block number', '0x100'], - ])('%s', (_nestedDesc, currentBlockNumber) => { - it('makes an additional request to the RPC endpoint', async () => { - await withMockedCommunications({ providerType }, async (comms) => { - const request = { - method, - // Note that `blockParam` is `0x100` here - params: buildMockParams({ blockParamIndex, blockParam }), - }; - - // The first time a block-cacheable request is made, the latest - // block number is retrieved through the block tracker first. - comms.mockNextBlockTrackerRequest({ - blockNumber: currentBlockNumber, - }); - comms.mockRpcCall({ - request, - response: { result: 'the result' }, - }); - - const result = await withNetworkClient( - { providerType }, - ({ makeRpcCall }) => makeRpcCall(request), - ); - - expect(result).toStrictEqual('the result'); - }); - }); - - for (const emptyValue of [null, undefined, '\u003cnil\u003e']) { - it(`does not retry an empty response of "${emptyValue}"`, async () => { - const request = { - method, - // Note that `blockParam` is `0x100` here - params: buildMockParams({ blockParamIndex, blockParam }), - }; - const mockResult = emptyValue; - + ['equal to the curent block number', '0x100'], + ]) { + describe(`${nestedDesc}`, () => { + it('makes an additional request to the RPC endpoint', async () => { await withMockedCommunications({ providerType }, async (comms) => { - // The first time a block-cacheable request is made, the latest block - // number is retrieved through the block tracker first. + const request = { + method, + // Note that `blockParam` is `0x100` here + params: buildMockParams({ blockParamIndex, blockParam }), + }; + + // The first time a block-cacheable request is made, the latest + // block number is retrieved through the block tracker first. comms.mockNextBlockTrackerRequest({ blockNumber: currentBlockNumber, }); comms.mockRpcCall({ - request: buildRequestWithReplacedBlockParam( - request, - blockParamIndex, - '0x100', - ), - response: { result: mockResult }, + request, + response: { result: 'the result' }, }); const result = await withNetworkClient( @@ -1612,58 +1753,178 @@ export function testsForRpcMethodSupportingBlockParam( ({ makeRpcCall }) => makeRpcCall(request), ); - expect(result).toStrictEqual(mockResult); + expect(result).toStrictEqual('the result'); }); }); - it(`does not reuse the result of a previous request if it was "${emptyValue}"`, async () => { - const requests = [ - { - method, - // Note that `blockParam` is `0x100` here - params: buildMockParams({ blockParamIndex, blockParam }), - }, - { - method, - // Note that `blockParam` is `0x100` here - params: buildMockParams({ blockParamIndex, blockParam }), - }, - ]; - const mockResults = [emptyValue, { blockHash: '0x100' }]; - - await withMockedCommunications({ providerType }, async (comms) => { - // The first time a block-cacheable request is made, the latest block - // number is retrieved through the block tracker first. - comms.mockNextBlockTrackerRequest({ - blockNumber: currentBlockNumber, + for (const emptyValue of [null, undefined, '\u003cnil\u003e']) { + if (providerType === 'infura') { + it(`retries up to 10 times if a "${emptyValue}" response is returned, returning successful non-empty response if there is one on the 10th try`, async () => { + const request = { + method, + // Note that `blockParam` is `0x100` here + params: buildMockParams({ blockParamIndex, blockParam }), + }; + + await withMockedCommunications( + { providerType }, + async (comms) => { + // The first time a block-cacheable request is made, the latest block + // number is retrieved through the block tracker first. + comms.mockNextBlockTrackerRequest({ + blockNumber: currentBlockNumber, + }); + comms.mockRpcCall({ + request, + response: { result: emptyValue }, + times: 9, + }); + comms.mockRpcCall({ + request, + response: { result: 'some value' }, + }); + + const result = await withNetworkClient( + { providerType }, + ({ makeRpcCall, clock }) => + waitForPromiseToBeFulfilledAfterRunningAllTimers( + makeRpcCall(request), + clock, + ), + ); + + expect(result).toStrictEqual('some value'); + }, + ); }); - comms.mockRpcCall({ - request: buildRequestWithReplacedBlockParam( - requests[0], - blockParamIndex, - '0x100', - ), - response: { result: mockResults[0] }, + + it(`retries up to 10 times if a "${emptyValue}" response is returned, failing after the 10th try`, async () => { + const request = { + method, + // Note that `blockParam` is `0x100` here + params: buildMockParams({ blockParamIndex, blockParam }), + }; + const mockResult = emptyValue; + + await withMockedCommunications( + { providerType }, + async (comms) => { + // The first time a block-cacheable request is made, the latest block + // number is retrieved through the block tracker first. + comms.mockNextBlockTrackerRequest({ + blockNumber: currentBlockNumber, + }); + comms.mockRpcCall({ + request, + response: { result: mockResult }, + times: 10, + }); + + const promiseForResult = withNetworkClient( + { providerType }, + ({ makeRpcCall, clock }) => + waitForPromiseToBeFulfilledAfterRunningAllTimers( + makeRpcCall(request), + clock, + ), + ); + + await expect(promiseForResult).rejects.toThrow( + 'RetryOnEmptyMiddleware - retries exhausted', + ); + }, + ); }); - comms.mockRpcCall({ - request: buildRequestWithReplacedBlockParam( - requests[1], - blockParamIndex, - '0x100', - ), - response: { result: mockResults[1] }, + } else { + it(`does not retry an empty response of "${emptyValue}"`, async () => { + const request = { + method, + // Note that `blockParam` is `0x100` here + params: buildMockParams({ blockParamIndex, blockParam }), + }; + const mockResult = emptyValue; + + await withMockedCommunications( + { providerType }, + async (comms) => { + // The first time a block-cacheable request is made, the latest block + // number is retrieved through the block tracker first. + comms.mockNextBlockTrackerRequest({ + blockNumber: currentBlockNumber, + }); + comms.mockRpcCall({ + request: buildRequestWithReplacedBlockParam( + request, + blockParamIndex, + '0x100', + ), + response: { result: mockResult }, + }); + + const result = await withNetworkClient( + { providerType }, + ({ makeRpcCall }) => makeRpcCall(request), + ); + + expect(result).toStrictEqual(mockResult); + }, + ); }); - const results = await withNetworkClient( - { providerType }, - ({ makeRpcCallsInSeries }) => makeRpcCallsInSeries(requests), - ); - - expect(results).toStrictEqual(mockResults); - }); - }); - } - }); + it(`does not reuse the result of a previous request if it was "${emptyValue}"`, async () => { + const requests = [ + { + method, + // Note that `blockParam` is `0x100` here + params: buildMockParams({ blockParamIndex, blockParam }), + }, + { + method, + // Note that `blockParam` is `0x100` here + params: buildMockParams({ blockParamIndex, blockParam }), + }, + ]; + const mockResults = [emptyValue, { blockHash: '0x100' }]; + + await withMockedCommunications( + { providerType }, + async (comms) => { + // The first time a block-cacheable request is made, the latest block + // number is retrieved through the block tracker first. + comms.mockNextBlockTrackerRequest({ + blockNumber: currentBlockNumber, + }); + comms.mockRpcCall({ + request: buildRequestWithReplacedBlockParam( + requests[0], + blockParamIndex, + '0x100', + ), + response: { result: mockResults[0] }, + }); + comms.mockRpcCall({ + request: buildRequestWithReplacedBlockParam( + requests[1], + blockParamIndex, + '0x100', + ), + response: { result: mockResults[1] }, + }); + + const results = await withNetworkClient( + { providerType }, + ({ makeRpcCallsInSeries }) => + makeRpcCallsInSeries(requests), + ); + + expect(results).toStrictEqual(mockResults); + }, + ); + }); + } + } + }); + } describe('greater than the current block number', () => { it('makes an additional request to the RPC endpoint', async () => { @@ -1771,43 +2032,37 @@ export function testsForRpcMethodSupportingBlockParam( } }); - if (method !== 'eth_getTransactionCount') { - describe('given a block tag of "pending"', () => { - const params = buildMockParams({ - blockParamIndex, - blockParam: 'pending', - }); - - it('hits the RPC endpoint once per request', async () => { - const requests = [ - { method, params }, - { method, params }, - ]; - const mockResults = ['first result', 'second result']; + describe('given a block tag of "pending"', () => { + const params = buildMockParams({ blockParamIndex, blockParam: 'pending' }); - await withMockedCommunications({ providerType }, async (comms) => { - // The first time a block-cacheable request is made, the latest - // block number is retrieved through the block tracker first. It - // doesn't matter what this is — it's just used as a cache key. - comms.mockNextBlockTrackerRequest(); - comms.mockRpcCall({ - request: requests[0], - response: { result: mockResults[0] }, - }); + it('hits the RPC endpoint on all calls and does not cache anything', async () => { + const requests = [ + { method, params }, + { method, params }, + ]; + const mockResults = ['first result', 'second result']; - comms.mockRpcCall({ - request: requests[1], - response: { result: mockResults[1] }, - }); + await withMockedCommunications({ providerType }, async (comms) => { + // The first time a block-cacheable request is made, the latest + // block number is retrieved through the block tracker first. It + // doesn't matter what this is — it's just used as a cache key. + comms.mockNextBlockTrackerRequest(); + comms.mockRpcCall({ + request: requests[0], + response: { result: mockResults[0] }, + }); + comms.mockRpcCall({ + request: requests[1], + response: { result: mockResults[1] }, + }); - const results = await withNetworkClient( - { providerType }, - ({ makeRpcCallsInSeries }) => makeRpcCallsInSeries(requests), - ); + const results = await withNetworkClient( + { providerType }, + ({ makeRpcCallsInSeries }) => makeRpcCallsInSeries(requests), + ); - expect(results).toStrictEqual([mockResults[0], mockResults[1]]); - }); + expect(results).toStrictEqual(mockResults); }); }); - } + }); } diff --git a/packages/network-controller/tests/provider-api-tests/helpers.ts b/packages/network-controller/tests/provider-api-tests/helpers.ts index 0fa16b7b43..60b122f4d5 100644 --- a/packages/network-controller/tests/provider-api-tests/helpers.ts +++ b/packages/network-controller/tests/provider-api-tests/helpers.ts @@ -1,18 +1,13 @@ -/* eslint-disable node/no-process-env */ -import { strict as assert } from 'assert'; import nock, { Scope as NockScope } from 'nock'; import sinon from 'sinon'; -import type EthQuery from 'eth-query'; +import type { JSONRPCResponse } from '@json-rpc-specification/meta-schema'; +import type { InfuraNetworkType } from '@metamask/controller-utils'; +import EthQuery from 'eth-query'; +import { Hex } from '@metamask/utils'; import { - JSONRPCResponse, - JSONRPCResponseResult, -} from '@json-rpc-specification/meta-schema'; -import { ControllerMessenger } from '@metamask/base-controller'; -import { InfuraNetworkType } from '@metamask/controller-utils'; -import { - NetworkController, - NetworkControllerMessenger, -} from '../../src/NetworkController'; + createNetworkClient, + NetworkClientType, +} from '../../src/create-network-client'; /** * A dummy value for the `infuraProjectId` option that `createInfuraClient` @@ -34,12 +29,11 @@ const MOCK_RPC_URL = 'http://foo.com'; */ const DEFAULT_LATEST_BLOCK_NUMBER = '0x42'; -const DEFAULT_BLOCK = { - baseFeePerGas: '0x7e89323d0', - hash: '0x4a32aed26c09820a35756b58b4b68f2613c1ee12a8d7ecb63d7313b99811ab07', - number: DEFAULT_LATEST_BLOCK_NUMBER, - timestamp: '0x63c84fb3', -}; +/** + * A reference to the original `setTimeout` function so that we can use it even + * when using fake timers. + */ +const originalSetTimeout = setTimeout; /** * If you're having trouble writing a test and you're wondering why the test @@ -49,11 +43,8 @@ const DEFAULT_BLOCK = { * @param args - The arguments that `console.log` takes. */ function debug(...args: any) { + /* eslint-disable-next-line node/no-process-env */ if (process.env.DEBUG_PROVIDER_TESTS === '1') { - if (args[0] instanceof Error) { - console.error(args[0]); - return; - } console.log(...args); } } @@ -64,73 +55,17 @@ function debug(...args: any) { * @param rpcUrl - The URL of the RPC endpoint. * @returns The nock scope. */ -function buildScopeForMockingRequests(rpcUrl: string) { +function buildScopeForMockingRequests(rpcUrl: string): NockScope { return nock(rpcUrl).filteringRequestBody((body) => { debug('Nock Received Request: ', body); return body; }); } -type MockBlockTrackerRequestOptions = { - /** - * A nock scope (a set of mocked requests scoped to a certain base url). - */ - nockScope: NockScope; - /** - * The block number that the block tracker should report, as a 0x-prefixed hex string. - */ - blockNumber: string; - - block: any; -}; - -const mockNextBlockTrackerRequest = ({ - nockScope, - blockNumber = DEFAULT_LATEST_BLOCK_NUMBER, - block = DEFAULT_BLOCK, -}: MockBlockTrackerRequestOptions) => { - mockRpcCall({ - nockScope, - request: { method: 'eth_blockNumber', params: [] }, - response: { result: blockNumber }, - }); - - mockRpcCall({ - nockScope, - request: { method: 'eth_getBlockByNumber', params: [blockNumber, false] }, - response: { - result: { ...block, number: blockNumber, hash: `0x${Math.random()}` }, - }, - }); -}; - -const mockAllBlockTrackerRequests = ({ - nockScope, - blockNumber = DEFAULT_LATEST_BLOCK_NUMBER, - block = DEFAULT_BLOCK, -}: MockBlockTrackerRequestOptions) => { - ( - mockRpcCall({ - nockScope, - request: { method: 'eth_blockNumber', params: [] }, - response: { result: blockNumber }, - }) as NockScope - ).persist(); - - ( - mockRpcCall({ - nockScope, - request: { method: 'eth_getBlockByNumber', params: [blockNumber, false] }, - response: { - result: { ...block, number: blockNumber, hash: `0x${Math.random()}` }, - }, - }) as NockScope - ).persist(); -}; - type Request = { method: string; params?: any[] }; type Response = { id?: number | string; + jsonrpc?: '2.0'; error?: any; result?: any; httpStatus?: number; @@ -214,7 +149,8 @@ function mockRpcCall({ } } } - const url = (nockScope as any).basePath.includes('infura.io') + /* @ts-expect-error The types for Nock do not include `basePath` in the interface for Nock.Scope. */ + const url = nockScope.basePath.includes('infura.io') ? `/v3/${MOCK_INFURA_PROJECT_ID}` : '/'; @@ -249,10 +185,10 @@ function mockRpcCall({ } else if (completeResponse !== undefined) { return nockRequest.reply(httpStatus, (_, requestBody: any) => { if (response !== undefined && !('body' in response)) { - if (response.id !== undefined) { - completeResponse.id = response?.id; - } else { + if (response.id === undefined) { completeResponse.id = requestBody.id; + } else { + completeResponse.id = response.id; } } debug('Nock returning Response', completeResponse); @@ -262,14 +198,75 @@ function mockRpcCall({ return nockRequest; } -const makeRpcCall = ( - ethQuery: EthQuery, - request: Request, - clock: any, -): Promise => { +type MockBlockTrackerRequestOptions = { + /** + * A nock scope (a set of mocked requests scoped to a certain base url). + */ + nockScope: NockScope; + /** + * The block number that the block tracker should report, as a 0x-prefixed hex + * string. + */ + blockNumber: string; +}; + +/** + * Mocks the next request for the latest block that the block tracker will make. + * + * @param args - The arguments. + * @param args.nockScope - A nock scope (a set of mocked requests scoped to a + * certain base URL). + * @param args.blockNumber - The block number that the block tracker should + * report, as a 0x-prefixed hex string. + */ +function mockNextBlockTrackerRequest({ + nockScope, + blockNumber = DEFAULT_LATEST_BLOCK_NUMBER, +}: MockBlockTrackerRequestOptions) { + mockRpcCall({ + nockScope, + request: { method: 'eth_blockNumber', params: [] }, + response: { result: blockNumber }, + }); +} + +/** + * Mocks all requests for the latest block that the block tracker will make. + * + * @param args - The arguments. + * @param args.nockScope - A nock scope (a set of mocked requests scoped to a + * certain base URL). + * @param args.blockNumber - The block number that the block tracker should + * report, as a 0x-prefixed hex string. + */ +async function mockAllBlockTrackerRequests({ + nockScope, + blockNumber = DEFAULT_LATEST_BLOCK_NUMBER, +}: MockBlockTrackerRequestOptions) { + const result = await mockRpcCall({ + nockScope, + request: { method: 'eth_blockNumber', params: [] }, + response: { result: blockNumber }, + }); + + if ('persist' in result) { + result.persist(); + } +} + +/** + * Makes a JSON-RPC call through the given eth-query object. + * + * @param ethQuery - The eth-query object. + * @param request - The request data. + * @returns A promise that either resolves with the result from the JSON-RPC + * response if it is successful or rejects with the error from the JSON-RPC + * response otherwise. + */ +function makeRpcCall(ethQuery: EthQuery, request: Request) { return new Promise((resolve, reject) => { - debug('[makeRpcCall] making ethQuery request', request); - ethQuery.sendAsync(request, (error: any, result: JSONRPCResponseResult) => { + debug('[makeRpcCall] making request', request); + ethQuery.sendAsync(request, (error: any, result: any) => { debug('[makeRpcCall > ethQuery handler] error', error, 'result', result); if (error) { reject(error); @@ -277,38 +274,47 @@ const makeRpcCall = ( resolve(result); } }); - clock.next(); // causes stoplight to 'complete' the await - const numTimers = clock.countTimers(); - if (numTimers > 1) { - clock.next(); // causes stoplight to 'complete' the await - } }); -}; +} export type ProviderType = 'infura' | 'custom'; export type MockOptions = { - providerType: ProviderType; infuraNetwork?: InfuraNetworkType; + providerType: ProviderType; customRpcUrl?: string; + customChainId?: Hex; }; export type MockCommunications = { mockNextBlockTrackerRequest: (options?: any) => void; mockAllBlockTrackerRequests: (options?: any) => void; - mockRpcCall: (arg0: CurriedMockRpcCallOptions) => MockRpcCallResult; + mockRpcCall: (options: CurriedMockRpcCallOptions) => MockRpcCallResult; rpcUrl: string; infuraNetwork: InfuraNetworkType; }; -export const withMockedCommunications = async ( +/** + * Sets up request mocks for requests to the provider. + * + * @param options - An options bag. + * @param options.providerType - The type of network client being tested. + * @param options.infuraNetwork - The name of the Infura network being tested, + * assuming that `providerType` is "infura" (default: "mainnet"). + * @param options.customRpcUrl - The URL of the custom RPC endpoint, assuming + * that `providerType` is "custom". + * @param fn - A function which will be called with an object that allows + * interaction with the network client. + * @returns The return value of the given function. + */ +export async function withMockedCommunications( { providerType, - infuraNetwork = InfuraNetworkType.mainnet, + infuraNetwork = 'mainnet', customRpcUrl = MOCK_RPC_URL, }: MockOptions, fn: (comms: MockCommunications) => Promise, -): Promise => { +) { const rpcUrl = providerType === 'infura' ? `https://${infuraNetwork}.infura.io` @@ -335,7 +341,7 @@ export const withMockedCommunications = async ( nock.isDone(); nock.cleanAll(); } -}; +} type MockNetworkClient = { blockTracker: any; @@ -344,36 +350,43 @@ type MockNetworkClient = { makeRpcCallsInSeries: (requests: Request[]) => Promise; }; -export const waitForNextBlockTracker = ( - blockTracker: any, - clock: sinon.SinonFakeTimers, -) => { - const prom = new Promise((resolve) => { - blockTracker.on('latest', () => { - resolve(true); - }); - }); - clock.runAll(); - return prom; -}; - -const originalSetTimeout = setTimeout; -export const waitForPromiseToBeFulfilledAfterRunningAllTimers = async ( +/** + * Some middleware contain logic which retries the request if some condition + * applies. This retrying always happens out of band via `setTimeout`, and + * because we are stubbing time via Jest's fake timers, we have to manually + * advance the clock so that the `setTimeout` handlers get fired. We don't know + * when these timers will get created, however, so we have to keep advancing + * timers until the request has been made an appropriate number of times. + * Unfortunately we don't have a good way to know how many times a request has + * been retried, but the good news is that the middleware won't end, and thus + * the promise which the RPC call returns won't get fulfilled, until all retries + * have been made. + * + * @param promise - The promise which is returned by the RPC call. + * @param clock - A Sinon clock object which can be used to advance to the next + * `setTimeout` handler. + * @returns The given promise. + */ +export async function waitForPromiseToBeFulfilledAfterRunningAllTimers( promise: any, clock: any, -) => { +) { let hasPromiseBeenFulfilled = false; let numTimesClockHasBeenAdvanced = 0; promise - .catch((e: any) => { - debug(e); + .catch((error: any) => { + // This is used to silence Node.js warnings about the rejection + // being handled asynchronously. The error is handled later when + // `promise` is awaited, but we log it here anyway in case it gets + // swallowed. + debug(error); }) .finally(() => { hasPromiseBeenFulfilled = true; }); - // `isPromiseFulfilled` is modified asynchronously. + // `hasPromiseBeenFulfilled` is modified asynchronously. /* eslint-disable-next-line no-unmodified-loop-condition */ while (!hasPromiseBeenFulfilled && numTimesClockHasBeenAdvanced < 15) { clock.runAll(); @@ -382,57 +395,69 @@ export const waitForPromiseToBeFulfilledAfterRunningAllTimers = async ( } return promise; -}; +} -export const withNetworkClient = async ( +/** + * Builds a provider from the middleware (for the provider type) along with a + * block tracker, runs the given function with those two things, and then + * ensures the block tracker is stopped at the end. + * + * @param options - An options bag. + * @param options.providerType - The type of network client being tested. + * @param options.infuraNetwork - The name of the Infura network being tested, + * assuming that `providerType` is "infura" (default: "mainnet"). + * @param options.customRpcUrl - The URL of the custom RPC endpoint, assuming + * that `providerType` is "custom". + * @param options.customChainId - The chain id belonging to the custom RPC + * endpoint, assuming that `providerType` is "custom" (default: "0x1"). + * @param fn - A function which will be called with an object that allows + * interaction with the network client. + * @returns The return value of the given function. + */ +export async function withNetworkClient( { providerType, - infuraNetwork = InfuraNetworkType.mainnet, + infuraNetwork = 'mainnet', customRpcUrl = MOCK_RPC_URL, + customChainId = '0x1', }: MockOptions, fn: (client: MockNetworkClient) => Promise, -): Promise => { - const messenger: NetworkControllerMessenger = - new ControllerMessenger().getRestricted({ - name: 'NetworkController', - allowedEvents: ['NetworkController:providerConfigChange'], - allowedActions: ['NetworkController:getEthQuery'], - }); - +) { + // Faking timers ends up doing two things: + // 1. Halting the block tracker (which depends on `setTimeout` to periodically + // request the latest block) set up in `eth-json-rpc-middleware` + // 2. Halting the retry logic in `@metamask/eth-json-rpc-infura` (which also + // depends on `setTimeout`) const clock = sinon.useFakeTimers(); - const controller = new NetworkController({ - messenger, - infuraProjectId: MOCK_INFURA_PROJECT_ID, - trackMetaMetricsEvent: jest.fn(), - }); - - const lookupNetworkMock = jest - .spyOn(controller, 'lookupNetwork') - .mockImplementation(() => { - return Promise.resolve(); - }); - - if (providerType === 'infura') { - await controller.setProviderType(infuraNetwork); - } else { - await controller.upsertNetworkConfiguration( - { - rpcUrl: customRpcUrl, - chainId: '0x9999', - ticker: 'TEST', - }, - { referrer: 'https://test-dapp.com', source: 'dapp', setActive: true }, - ); - } - const { provider, blockTracker } = controller.getProviderAndBlockTracker(); - assert(provider, 'provider has not been set for some reason'); - assert(blockTracker, 'blockTracker has not been set for some reason'); - const ethQuery = messenger.call('NetworkController:getEthQuery'); - assert(ethQuery, 'EthQuery has not been set for some reason'); - + // The JSON-RPC client wraps `eth_estimateGas` so that it takes 2 seconds longer + // than it usually would to complete. Or at least it should — this doesn't + // appear to be working correctly. Unset `IN_TEST` on `process.env` to prevent + // this behavior. + /* eslint-disable-next-line node/no-process-env */ + const inTest = process.env.IN_TEST; + /* eslint-disable-next-line node/no-process-env */ + delete process.env.IN_TEST; + const clientUnderTest = + providerType === 'infura' + ? createNetworkClient({ + network: infuraNetwork, + infuraProjectId: MOCK_INFURA_PROJECT_ID, + type: NetworkClientType.Infura, + }) + : createNetworkClient({ + chainId: customChainId, + rpcUrl: customRpcUrl, + type: NetworkClientType.Custom, + }); + /* eslint-disable-next-line node/no-process-env */ + process.env.IN_TEST = inTest; + + const { provider, blockTracker } = clientUnderTest; + + const ethQuery = new EthQuery(provider); const curriedMakeRpcCall = (request: Request) => - makeRpcCall(ethQuery, request, clock); + makeRpcCall(ethQuery, request); const makeRpcCallsInSeries = async (requests: Request[]) => { const responses = []; for (const request of requests) { @@ -451,12 +476,11 @@ export const withNetworkClient = async ( try { return await fn(client); } finally { - lookupNetworkMock.mockRestore(); - blockTracker.removeAllListeners(); - provider.stop(); + await blockTracker.destroy(); + clock.restore(); } -}; +} type BuildMockParamsOptions = { // The block parameter value to set. @@ -465,22 +489,48 @@ type BuildMockParamsOptions = { blockParamIndex: number; }; -export const buildMockParams = ({ +/** + * Build mock parameters for a JSON-RPC call. + * + * The string 'some value' is used as the default value for each entry. The + * block parameter index determines the number of parameters to generate. + * + * The block parameter can be set to a custom value. If no value is given, it + * is set as undefined. + * + * @param args - Arguments. + * @param args.blockParamIndex - The index of the block parameter. + * @param args.blockParam - The block parameter value to set. + * @returns The mock params. + */ +export function buildMockParams({ blockParam, blockParamIndex, -}: BuildMockParamsOptions) => { +}: BuildMockParamsOptions) { const params = new Array(blockParamIndex).fill('some value'); params[blockParamIndex] = blockParam; return params; -}; +} -export const buildRequestWithReplacedBlockParam = ( +/** + * Returns a partial JSON-RPC request object, with the "block" param replaced + * with the given value. + * + * @param request - The request object. + * @param request.method - The request method. + * @param request.params - The request params. + * @param blockParamIndex - The index within the `params` array of the block + * param. + * @param blockParam - The desired block param value. + * @returns The updated request object. + */ +export function buildRequestWithReplacedBlockParam( { method, params = [] }: Request, blockParamIndex: number, blockParam: any, -): Request => { +) { const updatedParams = params.slice(); updatedParams[blockParamIndex] = blockParam; return { method, params: updatedParams }; -}; +} diff --git a/packages/network-controller/tests/provider-api-tests/no-block-param.ts b/packages/network-controller/tests/provider-api-tests/no-block-param.ts index f3f1226a1c..974e73d27d 100644 --- a/packages/network-controller/tests/provider-api-tests/no-block-param.ts +++ b/packages/network-controller/tests/provider-api-tests/no-block-param.ts @@ -1,7 +1,5 @@ import { - MockCommunications, ProviderType, - waitForNextBlockTracker, waitForPromiseToBeFulfilledAfterRunningAllTimers, withMockedCommunications, withNetworkClient, @@ -9,36 +7,9 @@ import { import { buildFetchFailedErrorMessage, buildInfuraClientRetriesExhaustedErrorMessage, + buildJsonRpcEngineEmptyResponseErrorMessage, } from './shared-tests'; -const returnsErrorAfterRetries = async ( - providerType: 'infura' | 'custom', - method: string, - errorString: string, - errorMessage: RegExp, - comms: MockCommunications, -) => { - const request = { method, params: [] }; - - comms.mockAllBlockTrackerRequests(); - comms.mockRpcCall({ - request, - error: errorString, - times: 5, - }); - - const promiseForResult = withNetworkClient( - { providerType }, - ({ makeRpcCall, clock }) => - waitForPromiseToBeFulfilledAfterRunningAllTimers( - makeRpcCall(request), - clock, - ), - ); - - await expect(promiseForResult).rejects.toThrow(errorMessage); -}; - type TestsForRpcMethodAssumingNoBlockParamOptions = { providerType: ProviderType; numberOfParameters: number; @@ -111,7 +82,6 @@ export function testsForRpcMethodAssumingNoBlockParam( request: requests[0], response: { result: mockResults[0] }, }); - comms.mockRpcCall({ request: requests[1], response: { result: mockResults[1] }, @@ -154,7 +124,7 @@ export function testsForRpcMethodAssumingNoBlockParam( const firstResult = await client.makeRpcCall(requests[0]); // Proceed to the next iteration of the block tracker so that a new // block is fetched and the current block is updated. - await waitForNextBlockTracker(client.blockTracker, client.clock); + client.clock.runAll(); const secondResult = await client.makeRpcCall(requests[1]); return [firstResult, secondResult]; }, @@ -225,8 +195,6 @@ export function testsForRpcMethodAssumingNoBlockParam( // number is retrieved through the block tracker first. It doesn't // matter what this is — it's just used as a cache key. comms.mockNextBlockTrackerRequest(); - // A second block tracker request is made for some reason - comms.mockNextBlockTrackerRequest(); comms.mockRpcCall({ request: requests[0], response: { result: mockResults[0] }, @@ -412,6 +380,7 @@ export function testsForRpcMethodAssumingNoBlockParam( request, response: { id: 12345, + jsonrpc: '2.0', error: 'some error', httpStatus: 420, }, @@ -448,7 +417,6 @@ export function testsForRpcMethodAssumingNoBlockParam( }, times: 4, }); - comms.mockRpcCall({ request, response: { @@ -471,14 +439,36 @@ export function testsForRpcMethodAssumingNoBlockParam( }); it(`causes a request to fail with a custom error if the request to the RPC endpoint returns a ${httpStatus} response 5 times in a row`, async () => { - await withMockedCommunications({ providerType }, (comms) => { - return returnsErrorAfterRetries( - providerType, - method, - 'Some error', - buildFetchFailedErrorMessage(comms.rpcUrl, 'Some error'), - comms, + await withMockedCommunications({ providerType }, async (comms) => { + const request = { method }; + + // The first time a block-cacheable request is made, the latest block + // number is retrieved through the block tracker first. It doesn't + // matter what this is — it's just used as a cache key. + comms.mockNextBlockTrackerRequest(); + comms.mockRpcCall({ + request, + response: { + error: 'Some error', + httpStatus, + }, + times: 5, + }); + comms.mockNextBlockTrackerRequest(); + const promiseForResult = withNetworkClient( + { providerType }, + async ({ makeRpcCall, clock }) => { + return await waitForPromiseToBeFulfilledAfterRunningAllTimers( + makeRpcCall(request), + clock, + ); + }, ); + const err = + providerType === 'infura' + ? buildInfuraClientRetriesExhaustedErrorMessage('Gateway timeout') + : buildJsonRpcEngineEmptyResponseErrorMessage(method); + await expect(promiseForResult).rejects.toThrow(err); }); }); }); @@ -498,7 +488,6 @@ export function testsForRpcMethodAssumingNoBlockParam( error: 'ETIMEDOUT: Some message', times: 4, }); - comms.mockRpcCall({ request, response: { @@ -527,13 +516,30 @@ export function testsForRpcMethodAssumingNoBlockParam( if (providerType === 'infura') { it('causes a request to fail with a custom error if an "ETIMEDOUT" error is thrown while making the request to the RPC endpoint 5 times in a row', async () => { await withMockedCommunications({ providerType }, async (comms) => { + const request = { method }; const errorMessage = 'ETIMEDOUT: Some message'; - return returnsErrorAfterRetries( - providerType, - method, - errorMessage, + + // The first time a block-cacheable request is made, the latest block + // number is retrieved through the block tracker first. It doesn't + // matter what this is — it's just used as a cache key. + comms.mockNextBlockTrackerRequest(); + comms.mockRpcCall({ + request, + error: errorMessage, + times: 5, + }); + const promiseForResult = withNetworkClient( + { providerType }, + async ({ makeRpcCall, clock }) => { + return await waitForPromiseToBeFulfilledAfterRunningAllTimers( + makeRpcCall(request), + clock, + ); + }, + ); + + await expect(promiseForResult).rejects.toThrow( buildInfuraClientRetriesExhaustedErrorMessage(errorMessage), - comms, ); }); }); @@ -552,7 +558,7 @@ export function testsForRpcMethodAssumingNoBlockParam( error: errorMessage, times: 5, }); - const result = await withNetworkClient( + const promiseForResult = withNetworkClient( { providerType }, async ({ makeRpcCall, clock }) => { return await waitForPromiseToBeFulfilledAfterRunningAllTimers( @@ -562,7 +568,9 @@ export function testsForRpcMethodAssumingNoBlockParam( }, ); - expect(result).toBeUndefined(); + await expect(promiseForResult).rejects.toThrow( + buildJsonRpcEngineEmptyResponseErrorMessage(method), + ); }); }); } @@ -586,7 +594,6 @@ export function testsForRpcMethodAssumingNoBlockParam( error: 'ECONNRESET: Some message', times: 4, }); - comms.mockRpcCall({ request, response: { @@ -611,13 +618,30 @@ export function testsForRpcMethodAssumingNoBlockParam( it('causes a request to fail with a custom error if an "ECONNRESET" error is thrown while making the request to the RPC endpoint 5 times in a row', async () => { await withMockedCommunications({ providerType }, async (comms) => { + const request = { method }; const errorMessage = 'ECONNRESET: Some message'; - await returnsErrorAfterRetries( - providerType, - method, - errorMessage, + + // The first time a block-cacheable request is made, the latest block + // number is retrieved through the block tracker first. It doesn't + // matter what this is — it's just used as a cache key. + comms.mockNextBlockTrackerRequest(); + comms.mockRpcCall({ + request, + error: errorMessage, + times: 5, + }); + const promiseForResult = withNetworkClient( + { providerType }, + async ({ makeRpcCall, clock }) => { + return await waitForPromiseToBeFulfilledAfterRunningAllTimers( + makeRpcCall(request), + clock, + ); + }, + ); + + await expect(promiseForResult).rejects.toThrow( buildInfuraClientRetriesExhaustedErrorMessage(errorMessage), - comms, ); }); }); @@ -675,7 +699,6 @@ export function testsForRpcMethodAssumingNoBlockParam( error: 'SyntaxError: Some message', times: 4, }); - comms.mockRpcCall({ request, response: { @@ -700,13 +723,30 @@ export function testsForRpcMethodAssumingNoBlockParam( it('causes a request to fail with a custom error if an "SyntaxError" error is thrown while making the request to the RPC endpoint 5 times in a row', async () => { await withMockedCommunications({ providerType }, async (comms) => { - const errorMessage = 'ECONNRESET: Some message'; - await returnsErrorAfterRetries( - providerType, - method, - errorMessage, + const request = { method }; + const errorMessage = 'SyntaxError: Some message'; + + // The first time a block-cacheable request is made, the latest block + // number is retrieved through the block tracker first. It doesn't + // matter what this is — it's just used as a cache key. + comms.mockNextBlockTrackerRequest(); + comms.mockRpcCall({ + request, + error: errorMessage, + times: 5, + }); + const promiseForResult = withNetworkClient( + { providerType }, + async ({ makeRpcCall, clock }) => { + return await waitForPromiseToBeFulfilledAfterRunningAllTimers( + makeRpcCall(request), + clock, + ); + }, + ); + + await expect(promiseForResult).rejects.toThrow( buildInfuraClientRetriesExhaustedErrorMessage(errorMessage), - comms, ); }); }); @@ -779,7 +819,6 @@ export function testsForRpcMethodAssumingNoBlockParam( error: 'failed to parse response body: some message', times: 4, }); - comms.mockRpcCall({ request, response: { @@ -816,7 +855,7 @@ export function testsForRpcMethodAssumingNoBlockParam( error: errorMessage, times: 5, }); - const result = await withNetworkClient( + const promiseForResult = withNetworkClient( { providerType }, async ({ makeRpcCall, clock }) => { return await waitForPromiseToBeFulfilledAfterRunningAllTimers( @@ -826,7 +865,9 @@ export function testsForRpcMethodAssumingNoBlockParam( }, ); - expect(result).toBeUndefined(); + await expect(promiseForResult).rejects.toThrow( + buildJsonRpcEngineEmptyResponseErrorMessage(method), + ); }); }); } @@ -874,7 +915,6 @@ export function testsForRpcMethodAssumingNoBlockParam( error: 'Failed to fetch: some message', times: 4, }); - comms.mockRpcCall({ request, response: { @@ -911,7 +951,7 @@ export function testsForRpcMethodAssumingNoBlockParam( error: errorMessage, times: 5, }); - const result = await withNetworkClient( + const promiseForResult = withNetworkClient( { providerType }, async ({ makeRpcCall, clock }) => { return await waitForPromiseToBeFulfilledAfterRunningAllTimers( @@ -921,7 +961,9 @@ export function testsForRpcMethodAssumingNoBlockParam( }, ); - expect(result).toBeUndefined(); + await expect(promiseForResult).rejects.toThrow( + buildJsonRpcEngineEmptyResponseErrorMessage(method), + ); }); }); } diff --git a/packages/network-controller/tests/provider-api-tests/not-handled-by-middleware.ts b/packages/network-controller/tests/provider-api-tests/not-handled-by-middleware.ts index 12fc426145..6371f92e8d 100644 --- a/packages/network-controller/tests/provider-api-tests/not-handled-by-middleware.ts +++ b/packages/network-controller/tests/provider-api-tests/not-handled-by-middleware.ts @@ -36,7 +36,10 @@ export function testsForRpcMethodNotHandledByMiddleware( const expectedResult = 'the result'; await withMockedCommunications({ providerType }, async (comms) => { - comms.mockNextBlockTrackerRequest({ blockNumber: '0x1' }); + // The first time a block-cacheable request is made, the latest block + // number is retrieved through the block tracker first. It doesn't + // matter what this is — it's just used as a cache key. + comms.mockNextBlockTrackerRequest(); comms.mockRpcCall({ request, response: { result: expectedResult }, diff --git a/packages/network-controller/tests/provider-api-tests/shared-tests.ts b/packages/network-controller/tests/provider-api-tests/shared-tests.ts index a0ed68b5f6..a5244b2196 100644 --- a/packages/network-controller/tests/provider-api-tests/shared-tests.ts +++ b/packages/network-controller/tests/provider-api-tests/shared-tests.ts @@ -1,15 +1,12 @@ -import { NetworkType } from '@metamask/controller-utils'; import { testsForRpcMethodsThatCheckForBlockHashInResponse } from './block-hash-in-response'; import { testsForRpcMethodSupportingBlockParam } from './block-param'; import { ProviderType, - waitForNextBlockTracker, withMockedCommunications, withNetworkClient, } from './helpers'; import { testsForRpcMethodAssumingNoBlockParam } from './no-block-param'; import { testsForRpcMethodNotHandledByMiddleware } from './not-handled-by-middleware'; -import { testsForRpcMethodWithStaticResult } from './static-results'; /** * Constructs an error message that the Infura client would produce in the event @@ -25,6 +22,20 @@ export function buildInfuraClientRetriesExhaustedErrorMessage(reason: string) { ); } +/** + * Constructs an error message that JsonRpcEngine would produce in the event + * that the response object is empty as it leaves the middleware. + * + * @param method - The RPC method. + * @returns The error message. + */ +export function buildJsonRpcEngineEmptyResponseErrorMessage(method: string) { + return new RegExp( + `^JsonRpcEngine: Response has no error or result for request:.+"method": "${method}"`, + 'us', + ); +} + /** * Constructs an error message that `fetch` with throw if it cannot make a * request. @@ -54,15 +65,15 @@ export function testsForProviderType(providerType: ProviderType) { describe('methods included in the Ethereum JSON-RPC spec', () => { describe('methods not handled by middleware', () => { const notHandledByMiddleware = [ - // { name: 'eth_newFilter', numberOfParameters: 1 }, - // { name: 'eth_getFilterChanges', numberOfParameters: 1 }, - // { name: 'eth_newBlockFilter', numberOfParameters: 0 }, - // { name: 'eth_newPendingTransactionFilter', numberOfParameters: 0 }, - // { name: 'eth_uninstallFilter', numberOfParameters: 1 }, + { name: 'eth_newFilter', numberOfParameters: 1 }, + { name: 'eth_getFilterChanges', numberOfParameters: 1 }, + { name: 'eth_newBlockFilter', numberOfParameters: 0 }, + { name: 'eth_newPendingTransactionFilter', numberOfParameters: 0 }, + { name: 'eth_uninstallFilter', numberOfParameters: 1 }, - // { name: 'eth_sendRawTransaction', numberOfParameters: 1 }, - // { name: 'eth_sendTransaction', numberOfParameters: 1 }, - // { name: 'eth_sign', numberOfParameters: 2 }, + { name: 'eth_sendRawTransaction', numberOfParameters: 1 }, + { name: 'eth_sendTransaction', numberOfParameters: 1 }, + { name: 'eth_sign', numberOfParameters: 2 }, { name: 'eth_createAccessList', numberOfParameters: 2 }, { name: 'eth_getLogs', numberOfParameters: 1 }, @@ -78,6 +89,13 @@ export function testsForProviderType(providerType: ProviderType) { { name: 'debug_getRawTransaction', numberOfParameters: 1 }, { name: 'debug_getRawReceipts', numberOfParameters: 1 }, { name: 'debug_getBadBlocks', numberOfParameters: 0 }, + + { name: 'eth_accounts', numberOfParameters: 0 }, + { name: 'eth_coinbase', numberOfParameters: 0 }, + { name: 'eth_hashrate', numberOfParameters: 0 }, + { name: 'eth_mining', numberOfParameters: 0 }, + + { name: 'eth_signTransaction', numberOfParameters: 1 }, ]; notHandledByMiddleware.forEach(({ name, numberOfParameters }) => { describe(`method name: ${name}`, () => { @@ -89,46 +107,25 @@ export function testsForProviderType(providerType: ProviderType) { }); }); - describe('methods handled by middleware that returns a static result', () => { - const notHandledByMiddleware = [ - { name: 'eth_accounts', numberOfParameters: 0, result: [] }, - { name: 'eth_coinbase', numberOfParameters: 0, result: null }, - { name: 'eth_hashrate', numberOfParameters: 0, result: '0x00' }, - { name: 'eth_mining', numberOfParameters: 0, result: false }, + describe('methods with block hashes in their result', () => { + const methodsWithBlockHashInResponse = [ + { name: 'eth_getTransactionByHash', numberOfParameters: 1 }, + { name: 'eth_getTransactionReceipt', numberOfParameters: 1 }, ]; - notHandledByMiddleware.forEach(({ name, numberOfParameters, result }) => { + methodsWithBlockHashInResponse.forEach(({ name, numberOfParameters }) => { describe(`method name: ${name}`, () => { - testsForRpcMethodWithStaticResult(name, { - providerType, + testsForRpcMethodsThatCheckForBlockHashInResponse(name, { numberOfParameters, - result, + providerType, }); }); }); }); - describe('methods with block hashes in their result', () => { - const methodsWithBlockHashInResponse = [ - { method: 'eth_getTransactionByHash', numberOfParameters: 1 }, - { method: 'eth_getTransactionReceipt', numberOfParameters: 1 }, - ]; - - methodsWithBlockHashInResponse.forEach( - ({ method, numberOfParameters }) => { - describe(`method name: ${method}`, () => { - testsForRpcMethodsThatCheckForBlockHashInResponse(method, { - providerType, - numberOfParameters, - }); - }); - }, - ); - }); - describe('methods that assume there is no block param', () => { const assumingNoBlockParam = [ - // { name: 'eth_getFilterLogs', numberOfParameters: 1 }, - // { name: 'eth_blockNumber', numberOfParameters: 0 }, + { name: 'eth_getFilterLogs', numberOfParameters: 1 }, + { name: 'eth_blockNumber', numberOfParameters: 0 }, { name: 'eth_estimateGas', numberOfParameters: 2 }, { name: 'eth_gasPrice', numberOfParameters: 0 }, { name: 'eth_getBlockByHash', numberOfParameters: 2 }, @@ -143,9 +140,6 @@ export function testsForProviderType(providerType: ProviderType) { { name: 'eth_getUncleByBlockHashAndIndex', numberOfParameters: 2 }, { name: 'eth_getUncleCountByBlockHash', numberOfParameters: 1 }, ]; - - // NOTE: these methods do take a block param - // but this is not handled by our cache middleware currently const blockParamIgnored = [ { name: 'eth_getUncleCountByBlockNumber', numberOfParameters: 1 }, { name: 'eth_getUncleByBlockNumberAndIndex', numberOfParameters: 2 }, @@ -158,7 +152,6 @@ export function testsForProviderType(providerType: ProviderType) { numberOfParameters: 1, }, ]; - assumingNoBlockParam .concat(blockParamIgnored) .forEach(({ name, numberOfParameters }) => @@ -235,9 +228,8 @@ export function testsForProviderType(providerType: ProviderType) { await withNetworkClient( { providerType }, - async ({ makeRpcCall, clock, blockTracker }) => { + async ({ makeRpcCall, blockTracker }) => { await makeRpcCall(request); - await waitForNextBlockTracker(blockTracker, clock); expect(blockTracker.getCurrentBlock()).toStrictEqual('0x300'); }, ); @@ -266,9 +258,8 @@ export function testsForProviderType(providerType: ProviderType) { await withNetworkClient( { providerType }, - async ({ makeRpcCall, blockTracker, clock }) => { + async ({ makeRpcCall, blockTracker }) => { await makeRpcCall(request); - await waitForNextBlockTracker(blockTracker, clock); expect(blockTracker.getCurrentBlock()).toStrictEqual('0x300'); }, ); @@ -277,86 +268,26 @@ export function testsForProviderType(providerType: ProviderType) { }); describe('eth_chainId', () => { - it('hits the RPC endpoint to get the chain ID', async () => { - await withMockedCommunications({ providerType }, async (comms) => { - const request = { method: 'eth_chainId' }; - comms.mockRpcCall({ - request, - response: { - result: '0x1', - }, - }); - - const networkId = await withNetworkClient( - { providerType }, - ({ makeRpcCall }) => { - return makeRpcCall(request); - }, - ); - - expect(networkId).toStrictEqual('0x1'); - }); - }); - }); - - describe('eth_signTransaction', () => { - it('throws an error', async () => { - const address = '0x0000000000000000000000000000000000000000'; - const dummyTransaction = { - from: address, - to: address, - gasLimit: '21000', - maxFeePerGas: '300', - maxPriorityFeePerGas: '10', - nonce: '0', - value: '10000000000', - }; - const request = { - method: 'eth_signTransaction', - params: [dummyTransaction], - }; - const expectedResult = `Unknown address - unable to sign transaction for this address: "${address}"`; - - await withMockedCommunications({ providerType }, async (comms) => { - comms.mockNextBlockTrackerRequest({ blockNumber: '0x1' }); - - const promiseForResult = withNetworkClient( - { providerType }, - async ({ makeRpcCall }) => makeRpcCall(request), - ); + it('does not hit the RPC endpoint, instead returning the configured chain id', async () => { + const networkId = await withNetworkClient( + { providerType: 'custom', customChainId: '0x1' }, + ({ makeRpcCall }) => { + return makeRpcCall({ method: 'eth_chainId' }); + }, + ); - await expect(promiseForResult).rejects.toThrow(expectedResult); - }); + expect(networkId).toStrictEqual('0x1'); }); }); }); }); describe('methods not included in the Ethereum JSON-RPC spec', () => { - describe('methods handled by middleware that returns a static result', () => { - const notHandledByMiddleware = [ - { name: 'net_listening', numberOfParameters: 0, result: true }, - { - name: 'web3_clientVersion', - numberOfParameters: 0, - result: 'ProviderEngine/v16.0.5/javascript', - }, - ]; - notHandledByMiddleware.forEach(({ name, numberOfParameters, result }) => { - describe(`method name: ${name}`, () => { - testsForRpcMethodWithStaticResult(name, { - providerType, - numberOfParameters, - result, - }); - }); - }); - }); - describe('methods not handled by middleware', () => { const notHandledByMiddleware = [ - // { name: 'eth_subscribe', numberOfParameters: 1 }, - // { name: 'eth_unsubscribe', numberOfParameters: 1 }, + { name: 'net_listening', numberOfParameters: 0 }, + { name: 'eth_subscribe', numberOfParameters: 1 }, + { name: 'eth_unsubscribe', numberOfParameters: 1 }, { name: 'custom_rpc_method', numberOfParameters: 1 }, { name: 'net_peerCount', numberOfParameters: 0 }, { name: 'parity_nextNonce', numberOfParameters: 1 }, @@ -373,6 +304,7 @@ export function testsForProviderType(providerType: ProviderType) { describe('methods that assume there is no block param', () => { const assumingNoBlockParam = [ + { name: 'web3_clientVersion', numberOfParameters: 0 }, { name: 'eth_protocolVersion', numberOfParameters: 0 }, ]; assumingNoBlockParam.forEach(({ name, numberOfParameters }) => @@ -387,26 +319,44 @@ export function testsForProviderType(providerType: ProviderType) { describe('other methods', () => { describe('net_version', () => { - it('does hit RPC endpoint to get net_version', async () => { - await withMockedCommunications( - { providerType, infuraNetwork: NetworkType.goerli }, - async (comms) => { - comms.mockRpcCall({ - request: { method: 'net_version' }, - response: { result: '5' }, - }); - const networkId = await withNetworkClient( - { providerType, infuraNetwork: NetworkType.goerli }, - ({ makeRpcCall }) => { - return makeRpcCall({ - method: 'net_version', - }); - }, - ); - expect(networkId).toStrictEqual('5'); - }, - ); - }); + // The Infura middleware includes `net_version` in its scaffold + // middleware, whereas the custom RPC middleware does not. + if (providerType === 'infura') { + it('does not hit Infura, instead returning the network ID that maps to the Infura network, as a decimal string', async () => { + const networkId = await withNetworkClient( + { providerType: 'infura', infuraNetwork: 'goerli' }, + ({ makeRpcCall }) => { + return makeRpcCall({ + method: 'net_version', + }); + }, + ); + expect(networkId).toStrictEqual('5'); + }); + } else { + it('hits the RPC endpoint', async () => { + await withMockedCommunications( + { providerType: 'custom' }, + async (comms) => { + comms.mockRpcCall({ + request: { method: 'net_version' }, + response: { result: '1' }, + }); + + const networkId = await withNetworkClient( + { providerType: 'custom' }, + ({ makeRpcCall }) => { + return makeRpcCall({ + method: 'net_version', + }); + }, + ); + + expect(networkId).toStrictEqual('1'); + }, + ); + }); + } }); }); }); diff --git a/packages/network-controller/tests/provider-api-tests/static-results.ts b/packages/network-controller/tests/provider-api-tests/static-results.ts deleted file mode 100644 index b56b586fbf..0000000000 --- a/packages/network-controller/tests/provider-api-tests/static-results.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { fill } from 'lodash'; -import { - ProviderType, - withMockedCommunications, - withNetworkClient, -} from './helpers'; - -type TestsForRpcMethodWithStaticResult = { - providerType: ProviderType; - numberOfParameters: number; - result: any; -}; - -export const testsForRpcMethodWithStaticResult = ( - method: string, - { - providerType, - numberOfParameters, - result, - }: TestsForRpcMethodWithStaticResult, -) => { - it('method is handled by middleware and the request is never sent to the network', async () => { - const request = { - method, - params: fill(Array(numberOfParameters), 'some value'), - }; - - await withMockedCommunications({ providerType }, async (comms) => { - comms.mockNextBlockTrackerRequest({ blockNumber: '0x1' }); - const actualResult = await withNetworkClient( - { providerType }, - ({ makeRpcCall }) => makeRpcCall(request), - ); - - expect(actualResult).toStrictEqual(result); - }); - }); -}; diff --git a/yarn.lock b/yarn.lock index 79be6ab117..65fcb93bb5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1573,6 +1573,36 @@ __metadata: languageName: node linkType: hard +"@metamask/eth-json-rpc-infura@npm:^8.0.0": + version: 8.0.0 + resolution: "@metamask/eth-json-rpc-infura@npm:8.0.0" + dependencies: + "@metamask/utils": ^3.0.1 + eth-json-rpc-middleware: ^9.0.0 + eth-rpc-errors: ^4.0.3 + json-rpc-engine: ^6.1.0 + node-fetch: ^2.6.7 + checksum: e8c3a4b75d4f2bb09f68d7d2ac6b992f264df893921b50a05c35968ee684b7bba90180870eebecfc89b7fbf40d11de2a545ab68f1d511f569ce0a6519c64b0aa + languageName: node + linkType: hard + +"@metamask/eth-json-rpc-middleware@npm:^11.0.0": + version: 11.0.0 + resolution: "@metamask/eth-json-rpc-middleware@npm:11.0.0" + dependencies: + "@metamask/eth-json-rpc-provider": ^1.0.0 + "@metamask/eth-sig-util": ^5.0.0 + "@metamask/utils": ^3.0.3 + clone: ^2.1.1 + eth-block-tracker: ^7.0.0 + eth-rpc-errors: ^4.0.3 + json-rpc-engine: ^6.1.0 + pify: ^3.0.0 + safe-stable-stringify: ^2.3.2 + checksum: c866d07a199ab480ceeb7ab8df61c08284640b5ac13aee3dd81dae9e0e5575f4a425d95728070ab5402c0c6cd5f9237fb5f4f22dbcdc99fe0b50bb47df561830 + languageName: node + linkType: hard + "@metamask/eth-json-rpc-provider@npm:^1.0.0": version: 1.0.0 resolution: "@metamask/eth-json-rpc-provider@npm:1.0.0" @@ -1610,7 +1640,7 @@ __metadata: languageName: node linkType: hard -"@metamask/eth-sig-util@npm:^5.0.1, @metamask/eth-sig-util@npm:^5.0.2, @metamask/eth-sig-util@npm:^5.0.3": +"@metamask/eth-sig-util@npm:^5.0.0, @metamask/eth-sig-util@npm:^5.0.1, @metamask/eth-sig-util@npm:^5.0.2, @metamask/eth-sig-util@npm:^5.0.3": version: 5.0.3 resolution: "@metamask/eth-sig-util@npm:5.0.3" dependencies: @@ -1734,6 +1764,8 @@ __metadata: "@metamask/auto-changelog": ^3.1.0 "@metamask/base-controller": "workspace:^" "@metamask/controller-utils": "workspace:^" + "@metamask/eth-json-rpc-infura": ^8.0.0 + "@metamask/eth-json-rpc-middleware": ^11.0.0 "@metamask/eth-json-rpc-provider": ^1.0.0 "@metamask/swappable-obj-proxy": ^2.1.0 "@metamask/utils": ^5.0.2 @@ -1743,7 +1775,6 @@ __metadata: babel-runtime: ^6.26.0 deepmerge: ^4.2.2 eth-block-tracker: ^7.0.1 - eth-json-rpc-infura: ^5.1.0 eth-query: ^2.1.2 eth-rpc-errors: ^4.0.2 immer: ^9.0.6 @@ -1757,7 +1788,6 @@ __metadata: typedoc-plugin-missing-exports: ^0.22.6 typescript: ~4.6.3 uuid: ^8.3.2 - web3-provider-engine: ^16.0.5 languageName: unknown linkType: soft @@ -1946,7 +1976,7 @@ __metadata: languageName: node linkType: hard -"@metamask/utils@npm:^3.0.3, @metamask/utils@npm:^3.4.1": +"@metamask/utils@npm:^3.0.1, @metamask/utils@npm:^3.0.3, @metamask/utils@npm:^3.4.1": version: 3.6.0 resolution: "@metamask/utils@npm:3.6.0" dependencies: @@ -4432,7 +4462,7 @@ __metadata: languageName: node linkType: hard -"eth-block-tracker@npm:^7.0.1": +"eth-block-tracker@npm:^7.0.0, eth-block-tracker@npm:^7.0.1": version: 7.0.1 resolution: "eth-block-tracker@npm:7.0.1" dependencies: @@ -4500,6 +4530,25 @@ __metadata: languageName: node linkType: hard +"eth-json-rpc-middleware@npm:^9.0.0": + version: 9.0.1 + resolution: "eth-json-rpc-middleware@npm:9.0.1" + dependencies: + "@metamask/eth-sig-util": ^5.0.0 + "@metamask/safe-event-emitter": ^2.0.0 + "@metamask/utils": ^3.0.3 + btoa: ^1.2.1 + clone: ^2.1.1 + eth-block-tracker: ^5.0.1 + eth-rpc-errors: ^4.0.3 + json-rpc-engine: ^6.1.0 + json-stable-stringify: ^1.0.1 + node-fetch: ^2.6.7 + pify: ^3.0.0 + checksum: 9512829a6958df6ef739b891a0c0804b51a140407fd2e3ddaaa6b18d975796646cfcf7f7305a18beb7903db09e0c7a91b06dc5434b6bd2d6cdb85d992d9fd3ab + languageName: node + linkType: hard + "eth-method-registry@npm:1.1.0": version: 1.1.0 resolution: "eth-method-registry@npm:1.1.0" @@ -4537,12 +4586,12 @@ __metadata: languageName: node linkType: hard -"eth-rpc-errors@npm:^4.0.2": - version: 4.0.2 - resolution: "eth-rpc-errors@npm:4.0.2" +"eth-rpc-errors@npm:^4.0.2, eth-rpc-errors@npm:^4.0.3": + version: 4.0.3 + resolution: "eth-rpc-errors@npm:4.0.3" dependencies: fast-safe-stringify: ^2.0.6 - checksum: 1dbdee8f416090f1d318e17bdee2251d174d73c8faa4286fa364bc51ae9105672045f2d078ec23ca6a2b4b92af7cfbe7fa1ba17ad49e591fc653a363bf8cbab2 + checksum: 5fa31d1a10fdb340733b9a55e38e7687222c501052ca20743cef4d0c911a9bbcc0cad54aa6bf3e4b428604c071ff519803060e1cbc79ddb7c9257c11d407d32a languageName: node linkType: hard @@ -7461,9 +7510,9 @@ __metadata: languageName: node linkType: hard -"node-fetch@npm:^2.6.0, node-fetch@npm:^2.6.1": - version: 2.6.7 - resolution: "node-fetch@npm:2.6.7" +"node-fetch@npm:^2.6.0, node-fetch@npm:^2.6.1, node-fetch@npm:^2.6.7": + version: 2.6.9 + resolution: "node-fetch@npm:2.6.9" dependencies: whatwg-url: ^5.0.0 peerDependencies: @@ -7471,7 +7520,7 @@ __metadata: peerDependenciesMeta: encoding: optional: true - checksum: 8d816ffd1ee22cab8301c7756ef04f3437f18dace86a1dae22cf81db8ef29c0bf6655f3215cb0cdb22b420b6fe141e64b26905e7f33f9377a7fa59135ea3e10b + checksum: acb04f9ce7224965b2b59e71b33c639794d8991efd73855b0b250921382b38331ffc9d61bce502571f6cc6e11a8905ca9b1b6d4aeb586ab093e2756a1fd190d0 languageName: node linkType: hard @@ -8479,6 +8528,13 @@ __metadata: languageName: node linkType: hard +"safe-stable-stringify@npm:^2.3.2": + version: 2.4.2 + resolution: "safe-stable-stringify@npm:2.4.2" + checksum: 0324ba2e40f78cae63e31a02b1c9bdf1b786621f9e8760845608eb9e81aef401944ac2078e5c9c1533cf516aea34d08fa8052ca853637ced84b791caaf1e394e + languageName: node + linkType: hard + "safer-buffer@npm:>= 2.1.2 < 3, safer-buffer@npm:>= 2.1.2 < 3.0.0, safer-buffer@npm:^2.0.2, safer-buffer@npm:^2.1.0, safer-buffer@npm:~2.1.0": version: 2.1.2 resolution: "safer-buffer@npm:2.1.2"