Add eth-query types

Type definitions have been added for the `eth-query` package. These
were copied from the type definitions used in the extension.

The addition of these types revealed various invalid assumptions we've
made throughout the codebase, requiring the addition of runtime checks.
Mostly this consists of throwing an error whenever someone tries to use
the `ethQuery` instance before it has been set. In most cases, this
would only introduce an error in code paths that would already throw an
error.

The one exception to that is the gas fee controller, which used to get
`ethQuery` upon construction. It has been updated to wait until the
`providerConfigChange` event before attempting to fetch that, since
it's never defined upon construction in practice anyway.

There are two places in the network controller where these types
revealed that we're not validating the network response to the
`net_version` and `eth_getBlockByNumber` calls made during network
lookup. A comment has been left about validating this in a future PR.

Closes #1204

packages/assets-controllers/src/AccountTrackerController.ts

@@ -1,4 +1,5 @@
 import EthQuery from 'eth-query';
+import type { Provider } from 'eth-query';
 import { Mutex } from 'async-mutex';
 import {
   BaseConfig,
@@ -30,7 +31,7 @@ export interface AccountInformation {
  */
 export interface AccountTrackerConfig extends BaseConfig {
   interval: number;
-  provider?: any;
+  provider?: Provider;
 }
 
 /**
@@ -50,7 +51,7 @@ export class AccountTrackerController extends BaseController<
   AccountTrackerConfig,
   AccountTrackerState
 > {
-  private ethQuery: any;
+  private ethQuery?: EthQuery;
 
   private mutex = new Mutex();
 
@@ -125,7 +126,7 @@ export class AccountTrackerController extends BaseController<
    *
    * @param provider - Provider used to create a new underlying EthQuery instance.
    */
-  set provider(provider: any) {
+  set provider(provider: Provider) {
     this.ethQuery = new EthQuery(provider);
   }
 
@@ -157,6 +158,9 @@ export class AccountTrackerController extends BaseController<
     const accounts = { ...this.state.accounts };
     for (const address in accounts) {
       await safelyExecuteWithTimeout(async () => {
+        if (!this.ethQuery) {
+          throw new Error('Provider not set');
+        }
         const balance = await query(this.ethQuery, 'getBalance', [address]);
         accounts[address] = { balance: BNToHex(balance) };
       });
@@ -176,6 +180,9 @@ export class AccountTrackerController extends BaseController<
     return await Promise.all(
       addresses.map((address): Promise<[string, string] | undefined> => {
         return safelyExecuteWithTimeout(async () => {
+          if (!this.ethQuery) {
+            throw new Error('Provider not set');
+          }
           const balance = await query(this.ethQuery, 'getBalance', [address]);
           return [address, balance];
         });

packages/controller-utils/package.json

@@ -31,7 +31,9 @@
   "dependencies": {
     "@metamask/utils": "^5.0.2",
     "@spruceid/siwe-parser": "1.1.3",
+    "babel-runtime": "^6.26.0",
     "eth-ens-namehash": "^2.0.8",
+    "eth-query": "^2.1.2",
     "eth-rpc-errors": "^4.0.2",
     "ethereumjs-util": "^7.0.10",
     "ethjs-unit": "^0.1.6",

packages/controller-utils/src/util.test.ts

@@ -441,6 +441,8 @@ describe('util', () => {
         const ethQuery = {
           getBlockByHash: (blockId: any, cb: any) => cb(null, { id: blockId }),
         };
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         const result = await util.query(ethQuery, 'getBlockByHash', ['0x1234']);
         expect(result).toStrictEqual({ id: '0x1234' });
       });
@@ -451,6 +453,8 @@ describe('util', () => {
             cb(new Error('uh oh'), null),
         };
         await expect(
+          // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+          // @ts-ignore
           util.query(ethQuery, 'getBlockByHash', ['0x1234']),
         ).rejects.toThrow('uh oh');
       });

packages/controller-utils/src/util.ts

@@ -6,11 +6,12 @@ import {
   toChecksumAddress,
   stripHexPrefix,
 } from 'ethereumjs-util';
+import type EthQuery from 'eth-query';
 import { fromWei, toWei } from 'ethjs-unit';
 import ensNamehash from 'eth-ens-namehash';
 import deepEqual from 'fast-deep-equal';
 import type { Hex } from '@metamask/utils';
-import { isStrictHexString } from '@metamask/utils';
+import { hasProperty, isStrictHexString } from '@metamask/utils';
 import type { Json } from './types';
 import { MAX_SAFE_CHAIN_ID } from './constants';
 
@@ -426,20 +427,26 @@ export function normalizeEnsName(ensName: string): string | null {
  * @returns Promise resolving the request.
  */
 export function query(
-  ethQuery: any,
+  ethQuery: EthQuery,
   method: string,
   args: any[] = [],
 ): Promise<any> {
   return new Promise((resolve, reject) => {
-    const cb = (error: Error, result: any) => {
+    const cb = (error: unknown, result: any) => {
       if (error) {
         reject(error);
         return;
       }
       resolve(result);
     };
 
-    if (typeof ethQuery[method] === 'function') {
+    if (
+      hasProperty(ethQuery, method) &&
+      typeof ethQuery[method] === 'function'
+    ) {
+      // All of the generated method types have this signature, but our types don't support these yet
+      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+      // @ts-ignore
       ethQuery[method](...args, cb);
     } else {
       ethQuery.sendAsync({ method, params: args }, cb);

packages/gas-fee-controller/src/GasFeeController.ts

@@ -255,7 +255,7 @@ export class GasFeeController extends BaseControllerV2<
 
   private currentChainId;
 
-  private ethQuery: any;
+  private ethQuery?: EthQuery;
 
   private clientId?: string;
 
@@ -346,9 +346,6 @@ export class GasFeeController extends BaseControllerV2<
         'NetworkController:getProviderConfig',
       );
       this.currentChainId = providerConfig.chainId;
-      this.ethQuery = this.messagingSystem.call(
-        'NetworkController:getEthQuery',
-      );
 
       this.messagingSystem.subscribe(
         'NetworkController:providerConfigChange',

packages/gas-fee-controller/src/fetchBlockFeeHistory.test.ts

@@ -40,12 +40,16 @@ describe('fetchBlockFeeHistory', () => {
 
     beforeEach(() => {
       when(mockedQuery)
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         .calledWith(ethQuery, 'blockNumber')
         .mockResolvedValue(new BN(latestBlockNumber));
     });
 
     it('should return a representation of fee history from the Ethereum network, organized by block rather than type of data', async () => {
       when(mockedQuery)
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         .calledWith(ethQuery, 'eth_feeHistory', [
           toHex(numberOfRequestedBlocks),
           toHex(latestBlockNumber),
@@ -95,6 +99,8 @@ describe('fetchBlockFeeHistory', () => {
 
     it('should be able to handle an "empty" response from eth_feeHistory', async () => {
       when(mockedQuery)
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         .calledWith(ethQuery, 'eth_feeHistory', [
           toHex(numberOfRequestedBlocks),
           toHex(latestBlockNumber),
@@ -116,6 +122,8 @@ describe('fetchBlockFeeHistory', () => {
 
     it('should be able to handle an response with undefined baseFeePerGas from eth_feeHistory', async () => {
       when(mockedQuery)
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         .calledWith(ethQuery, 'eth_feeHistory', [
           toHex(numberOfRequestedBlocks),
           toHex(latestBlockNumber),
@@ -153,6 +161,8 @@ describe('fetchBlockFeeHistory', () => {
       });
 
       when(mockedQuery)
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         .calledWith(ethQuery, 'blockNumber')
         .mockResolvedValue(new BN(latestBlockNumber));
 
@@ -165,6 +175,8 @@ describe('fetchBlockFeeHistory', () => {
           .map((block) => block.gasUsedRatio);
 
         when(mockedQuery)
+          // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+          // @ts-ignore
           .calledWith(ethQuery, 'eth_feeHistory', [
             toHex(endBlockNumber - startBlockNumber + 1),
             toHex(endBlockNumber),
@@ -201,6 +213,8 @@ describe('fetchBlockFeeHistory', () => {
       const numberOfRequestedBlocks = 3;
       const endBlock = new BN(latestBlockNumber);
       when(mockedQuery)
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         .calledWith(ethQuery, 'eth_feeHistory', [
           toHex(numberOfRequestedBlocks),
           toHex(endBlock),
@@ -228,12 +242,16 @@ describe('fetchBlockFeeHistory', () => {
 
     beforeEach(() => {
       when(mockedQuery)
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         .calledWith(ethQuery, 'blockNumber')
         .mockResolvedValue(new BN(latestBlockNumber));
     });
 
     it('should match each item in the "reward" key from the response to its percentile', async () => {
       when(mockedQuery)
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         .calledWith(ethQuery, 'eth_feeHistory', [
           toHex(numberOfRequestedBlocks),
           toHex(latestBlockNumber),
@@ -309,6 +327,8 @@ describe('fetchBlockFeeHistory', () => {
 
     it('should be able to handle an "empty" response from eth_feeHistory including an empty "reward" array', async () => {
       when(mockedQuery)
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         .calledWith(ethQuery, 'eth_feeHistory', [
           toHex(numberOfRequestedBlocks),
           toHex(latestBlockNumber),
@@ -337,6 +357,8 @@ describe('fetchBlockFeeHistory', () => {
 
     it('includes an extra block with an estimated baseFeePerGas', async () => {
       when(mockedQuery)
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         .calledWith(ethQuery, 'eth_feeHistory', [
           toHex(numberOfRequestedBlocks),
           toHex(latestBlockNumber),
@@ -403,6 +425,8 @@ describe('fetchBlockFeeHistory', () => {
       const endBlock = new BN(latestBlockNumber);
 
       when(mockedQuery)
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         .calledWith(ethQuery, 'eth_feeHistory', [
           toHex(latestBlockNumber),
           toHex(latestBlockNumber),

packages/gas-fee-controller/src/fetchGasEstimatesViaEthFeeHistory.test.ts

@@ -83,6 +83,8 @@ describe('fetchGasEstimatesViaEthFeeHistory', () => {
       .calledWith(blocks)
       .mockReturnValue(levelSpecificEstimates);
 
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
     const gasFeeEstimates = await fetchGasEstimatesViaEthFeeHistory(ethQuery);
 
     expect(gasFeeEstimates).toStrictEqual({

packages/gas-fee-controller/src/fetchGasEstimatesViaEthFeeHistory.ts

@@ -1,7 +1,7 @@
 import { fromWei } from 'ethjs-unit';
+import type EthQuery from 'eth-query';
 import { GWEI } from '@metamask/controller-utils';
 import { GasFeeEstimates } from './GasFeeController';
-import { EthQuery } from './fetchGasEstimatesViaEthFeeHistory/types';
 import fetchBlockFeeHistory from './fetchBlockFeeHistory';
 import fetchLatestBlock from './fetchGasEstimatesViaEthFeeHistory/fetchLatestBlock';
 import calculateGasFeeEstimatesForPriorityLevels from './fetchGasEstimatesViaEthFeeHistory/calculateGasFeeEstimatesForPriorityLevels';

packages/gas-fee-controller/src/fetchGasEstimatesViaEthFeeHistory/fetchLatestBlock.ts

@@ -1,5 +1,6 @@
 import { query, fromHex } from '@metamask/controller-utils';
-import { EthBlock, EthQuery } from './types';
+import type EthQuery from 'eth-query';
+import { EthBlock } from './types';
 
 /**
  * Returns information about the latest completed block.

packages/gas-fee-controller/src/fetchGasEstimatesViaEthFeeHistory/types.ts

@@ -5,10 +5,4 @@ export type EthBlock = {
   baseFeePerGas: BN;
 };
 
-export type EthQuery = {
-  getBlockByNumber: (
-    blockNumber: BN | 'latest' | 'earliest' | 'pending',
-  ) => Promise<EthBlock>;
-};
-
 export type FeeRange = [string, string];