Use tokenList to get token details, when available, in getTokenStanda… (#17891)

* Use tokenList to get token details, when available, in getTokenStandardAndDetails

Previously, every call to getTokenStandardAndDetails would fetch data via the provider.
This would result in at least 3 network requests whenever that method is called for an
ERC20 token, contributing to unneccesary loading and lagging in multiple places.
This commit takes advantage of stored data we already have available to avoid the unnecessary
loading.

* Lint fix

* Fix build-quote test

* bump coverage targets

* Pass provider to token-util, for use in ethers Contract module

* Check all possible sources of ERC20 token data before async call to assetsContractController

* Add and update tests

* Update app/scripts/metamask-controller.js

Co-authored-by: Alex Donesky <adonesky@gmail.com>

* Update app/scripts/metamask-controller.js

Co-authored-by: Alex Donesky <adonesky@gmail.com>

* Remove unnecessary this.ethQuery changes

* Use metamask-eth-abis instead of human-standard-token-abi in token-util.ts

* Add explanatory comments to getTokenStandardAndDetails

* lint fix

* Cleanup

* fix test

* Update app/scripts/metamask-controller.js

Co-authored-by: Alex Donesky <adonesky@gmail.com>

* update error message

---------

Co-authored-by: Alex Donesky <adonesky@gmail.com>

app/scripts/metamask-controller.js

@@ -68,6 +68,7 @@ import {
   AssetType,
   TransactionStatus,
   TransactionType,
+  TokenStandard,
 } from '../../shared/constants/transaction';
 import {
   GAS_API_BASE_URL,
@@ -104,7 +105,10 @@ import {
 } from '../../shared/constants/app';
 import { EVENT, EVENT_NAMES } from '../../shared/constants/metametrics';
 
-import { getTokenIdParam } from '../../shared/lib/token-util';
+import {
+  getTokenIdParam,
+  fetchTokenBalance,
+} from '../../shared/lib/token-util.ts';
 import { isEqualCaseInsensitive } from '../../shared/modules/string-utils';
 import { parseStandardTokenTransactionData } from '../../shared/modules/transaction.utils';
 import { STATIC_MAINNET_TOKEN_LIST } from '../../shared/constants/tokens';
@@ -944,10 +948,7 @@ export default class MetamaskController extends EventEmitter {
         this.getExternalPendingTransactions.bind(this),
       getAccountType: this.getAccountType.bind(this),
       getDeviceModel: this.getDeviceModel.bind(this),
-      getTokenStandardAndDetails:
+      getTokenStandardAndDetails: this.getTokenStandardAndDetails.bind(this),
-        this.assetsContractController.getTokenStandardAndDetails.bind(
-          this.assetsContractController,
-        ),
       securityProviderRequest: this.securityProviderRequest.bind(this),
     });
     this.txController.on('newUnapprovedTx', () => opts.showUserConfirmation());
@@ -2189,12 +2190,72 @@ export default class MetamaskController extends EventEmitter {
   }
 
   async getTokenStandardAndDetails(address, userAddress, tokenId) {
-    const details =
+    const { tokenList } = this.tokenListController.state;
-      await this.assetsContractController.getTokenStandardAndDetails(
+    const { tokens } = this.tokensController.state;
+
+    const staticTokenListDetails =
+      STATIC_MAINNET_TOKEN_LIST[address.toLowerCase()] || {};
+    const tokenListDetails = tokenList[address.toLowerCase()] || {};
+    const userDefinedTokenDetails =
+      tokens.find(({ address: _address }) =>
+        isEqualCaseInsensitive(_address, address),
+      ) || {};
+
+    const tokenDetails = {
+      ...staticTokenListDetails,
+      ...tokenListDetails,
+      ...userDefinedTokenDetails,
+    };
+    const tokenDetailsStandardIsERC20 =
+      isEqualCaseInsensitive(tokenDetails.standard, TokenStandard.ERC20) ||
+      tokenDetails.erc20 === true;
+    const noEvidenceThatTokenIsAnNFT =
+      !tokenId &&
+      !isEqualCaseInsensitive(tokenDetails.standard, TokenStandard.ERC1155) &&
+      !isEqualCaseInsensitive(tokenDetails.standard, TokenStandard.ERC721) &&
+      !tokenDetails.erc721;
+
+    const otherDetailsAreERC20Like =
+      tokenDetails.decimals !== undefined && tokenDetails.symbol;
+
+    const tokenCanBeTreatedAsAnERC20 =
+      tokenDetailsStandardIsERC20 ||
+      (noEvidenceThatTokenIsAnNFT && otherDetailsAreERC20Like);
+
+    let details;
+    if (tokenCanBeTreatedAsAnERC20) {
+      try {
+        const balance = await fetchTokenBalance(
+          address,
+          userAddress,
+          this.provider,
+        );
+
+        details = {
+          address,
+          balance,
+          standard: TokenStandard.ERC20,
+          decimals: tokenDetails.decimals,
+          symbol: tokenDetails.symbol,
+        };
+      } catch (e) {
+        // If the `fetchTokenBalance` call failed, `details` remains undefined, and we
+        // fall back to the below `assetsContractController.getTokenStandardAndDetails` call
+        log.warning(`Failed to get token balance. Error: ${e}`);
+      }
+    }
+
+    // `details`` will be undefined if `tokenCanBeTreatedAsAnERC20`` is false,
+    // or if it is true but the `fetchTokenBalance`` call failed. In either case, we should
+    // attempt to retrieve details from `assetsContractController.getTokenStandardAndDetails`
+    if (details === undefined) {
+      details = await this.assetsContractController.getTokenStandardAndDetails(
         address,
         userAddress,
         tokenId,
       );
+    }
+
     return {
       ...details,
       decimals: details?.decimals?.toString(10),

app/scripts/metamask-controller.test.js

@@ -10,6 +10,7 @@ import { wordlist as englishWordlist } from '@metamask/scure-bip39/dist/wordlist
 import { TransactionStatus } from '../../shared/constants/transaction';
 import createTxMeta from '../../test/lib/createTxMeta';
 import { NETWORK_TYPES } from '../../shared/constants/network';
+import { createTestProviderTools } from '../../test/stub/provider';
 import {
   HardwareDeviceNames,
   HardwareKeyringTypes,
@@ -85,8 +86,28 @@ const createLoggerMiddlewareMock = () => (req, res, next) => {
   next();
 };
 
+const MOCK_TOKEN_BALANCE = '888';
+
+function MockEthContract() {
+  return () => {
+    return {
+      at: () => {
+        return {
+          balanceOf: () => MOCK_TOKEN_BALANCE,
+        };
+      },
+    };
+  };
+}
+
+// TODO, Feb 24, 2023:
+// ethjs-contract is being added to proxyquire, but we might want to discontinue proxyquire
+// this is for expediency as we resolve a bug for v10.26.0. The proper solution here would have
+// us set up the test infrastructure for a mocked provider. Github ticket for that is:
+// https://github.com/MetaMask/metamask-extension/issues/17890
 const MetaMaskController = proxyquire('./metamask-controller', {
   './lib/createLoggerMiddleware': { default: createLoggerMiddlewareMock },
+  'ethjs-contract': MockEthContract,
 }).default;
 
 const currentNetworkId = '5';
@@ -1273,4 +1294,321 @@ describe('MetaMaskController', function () {
       );
     });
   });
+
+  describe('getTokenStandardAndDetails', function () {
+    it('gets token data from the token list if available, and with a balance retrieved by fetchTokenBalance', async function () {
+      const providerResultStub = {
+        eth_getCode: '0x123',
+        eth_call:
+          '0x00000000000000000000000000000000000000000000000029a2241af62c0000',
+      };
+      const { provider } = createTestProviderTools({
+        scaffold: providerResultStub,
+        networkId: '5',
+        chainId: '5',
+      });
+
+      const tokenData = {
+        decimals: 18,
+        symbol: 'DAI',
+      };
+
+      metamaskController.tokenListController.update(() => {
+        return {
+          tokenList: {
+            '0x6b175474e89094c44da98b954eedeac495271d0f': tokenData,
+          },
+        };
+      });
+
+      metamaskController.provider = provider;
+      const tokenDetails = await metamaskController.getTokenStandardAndDetails(
+        '0x6B175474E89094C44Da98b954EedeAC495271d0F',
+        '0xf0d172594caedee459b89ad44c94098e474571b6',
+      );
+
+      assert.ok(
+        tokenDetails.standard === 'ERC20',
+        'tokenDetails should include token standard in upper case',
+      );
+      assert.ok(
+        tokenDetails.decimals === String(tokenData.decimals),
+        'tokenDetails should include token decimals as a string',
+      );
+      assert.ok(
+        tokenDetails.symbol === tokenData.symbol,
+        'tokenDetails should include token symbol',
+      );
+      assert.ok(
+        tokenDetails.balance === '3000000000000000000',
+        'tokenDetails should include a balance',
+      );
+    });
+
+    it('gets token data from tokens if available, and with a balance retrieved by fetchTokenBalance', async function () {
+      const providerResultStub = {
+        eth_getCode: '0x123',
+        eth_call:
+          '0x00000000000000000000000000000000000000000000000029a2241af62c0000',
+      };
+      const { provider } = createTestProviderTools({
+        scaffold: providerResultStub,
+        networkId: '5',
+        chainId: '5',
+      });
+
+      const tokenData = {
+        decimals: 18,
+        symbol: 'DAI',
+      };
+
+      metamaskController.tokensController.update({
+        tokens: [
+          {
+            address: '0x6b175474e89094c44da98b954eedeac495271d0f',
+            ...tokenData,
+          },
+        ],
+      });
+
+      metamaskController.provider = provider;
+      const tokenDetails = await metamaskController.getTokenStandardAndDetails(
+        '0x6B175474E89094C44Da98b954EedeAC495271d0F',
+        '0xf0d172594caedee459b89ad44c94098e474571b6',
+      );
+
+      assert.ok(
+        tokenDetails.standard === 'ERC20',
+        'tokenDetails should include token standard in upper case',
+      );
+      assert.ok(
+        tokenDetails.decimals === String(tokenData.decimals),
+        'tokenDetails should include token decimals as a string',
+      );
+      assert.ok(
+        tokenDetails.symbol === tokenData.symbol,
+        'tokenDetails should include token symbol',
+      );
+      assert.ok(
+        tokenDetails.balance === '3000000000000000000',
+        'tokenDetails should include a balance',
+      );
+    });
+
+    it('gets token data from contract-metadata if available, and with a balance retrieved by fetchTokenBalance', async function () {
+      const providerResultStub = {
+        eth_getCode: '0x123',
+        eth_call:
+          '0x00000000000000000000000000000000000000000000000029a2241af62c0000',
+      };
+      const { provider } = createTestProviderTools({
+        scaffold: providerResultStub,
+        networkId: '5',
+        chainId: '5',
+      });
+
+      metamaskController.provider = provider;
+      const tokenDetails = await metamaskController.getTokenStandardAndDetails(
+        '0x6B175474E89094C44Da98b954EedeAC495271d0F',
+        '0xf0d172594caedee459b89ad44c94098e474571b6',
+      );
+
+      assert.ok(
+        tokenDetails.standard === 'ERC20',
+        'tokenDetails should include token standard in upper case',
+      );
+      assert.ok(
+        tokenDetails.decimals === '18',
+        'tokenDetails should include token decimals as a string',
+      );
+      assert.ok(
+        tokenDetails.symbol === 'DAI',
+        'tokenDetails should include token symbol',
+      );
+      assert.ok(
+        tokenDetails.balance === '3000000000000000000',
+        'tokenDetails should include a balance',
+      );
+    });
+
+    it('gets token data from the blockchain, via the assetsContractController, if not available through other sources', async function () {
+      const providerResultStub = {
+        eth_getCode: '0x123',
+        eth_call:
+          '0x00000000000000000000000000000000000000000000000029a2241af62c0000',
+      };
+      const { provider } = createTestProviderTools({
+        scaffold: providerResultStub,
+        networkId: '5',
+        chainId: '5',
+      });
+
+      const tokenData = {
+        standard: 'ERC20',
+        decimals: 18,
+        symbol: 'DAI',
+        balance: '333',
+      };
+
+      metamaskController.tokenListController.update(() => {
+        return {
+          tokenList: {
+            '0x6b175474e89094c44da98b954eedeac495271d0f': {},
+          },
+        };
+      });
+
+      metamaskController.provider = provider;
+
+      sandbox
+        .stub(
+          metamaskController.assetsContractController,
+          'getTokenStandardAndDetails',
+        )
+        .callsFake(() => {
+          return tokenData;
+        });
+
+      const tokenDetails = await metamaskController.getTokenStandardAndDetails(
+        '0xNotInTokenList',
+        '0xf0d172594caedee459b89ad44c94098e474571b6',
+      );
+      assert.ok(
+        tokenDetails.standard === tokenData.standard.toUpperCase(),
+        'tokenDetails should include token standard in upper case',
+      );
+      assert.ok(
+        tokenDetails.decimals === String(tokenData.decimals),
+        'tokenDetails should include token decimals as a string',
+      );
+      assert.ok(
+        tokenDetails.symbol === tokenData.symbol,
+        'tokenDetails should include token symbol',
+      );
+      assert.ok(
+        tokenDetails.balance === tokenData.balance,
+        'tokenDetails should include a balance',
+      );
+    });
+
+    it('gets token data from the blockchain, via the assetsContractController, if it is in the token list but is an ERC721', async function () {
+      const providerResultStub = {
+        eth_getCode: '0x123',
+        eth_call:
+          '0x00000000000000000000000000000000000000000000000029a2241af62c0000',
+      };
+      const { provider } = createTestProviderTools({
+        scaffold: providerResultStub,
+        networkId: '5',
+        chainId: '5',
+      });
+
+      const tokenData = {
+        standard: 'ERC721',
+        decimals: 18,
+        symbol: 'DAI',
+        balance: '333',
+      };
+
+      metamaskController.tokenListController.update(() => {
+        return {
+          tokenList: {
+            '0xaaa75474e89094c44da98b954eedeac495271d0f': tokenData,
+          },
+        };
+      });
+
+      metamaskController.provider = provider;
+
+      sandbox
+        .stub(
+          metamaskController.assetsContractController,
+          'getTokenStandardAndDetails',
+        )
+        .callsFake(() => {
+          return tokenData;
+        });
+
+      const tokenDetails = await metamaskController.getTokenStandardAndDetails(
+        '0xAAA75474e89094c44da98b954eedeac495271d0f',
+        '0xf0d172594caedee459b89ad44c94098e474571b6',
+      );
+      assert.ok(
+        tokenDetails.standard === tokenData.standard.toUpperCase(),
+        'tokenDetails should include token standard in upper case',
+      );
+      assert.ok(
+        tokenDetails.decimals === String(tokenData.decimals),
+        'tokenDetails should include token decimals as a string',
+      );
+      assert.ok(
+        tokenDetails.symbol === tokenData.symbol,
+        'tokenDetails should include token symbol',
+      );
+      assert.ok(
+        tokenDetails.balance === tokenData.balance,
+        'tokenDetails should include a balance',
+      );
+    });
+
+    it('gets token data from the blockchain, via the assetsContractController, if it is in the token list but is an ERC1155', async function () {
+      const providerResultStub = {
+        eth_getCode: '0x123',
+        eth_call:
+          '0x00000000000000000000000000000000000000000000000029a2241af62c0000',
+      };
+      const { provider } = createTestProviderTools({
+        scaffold: providerResultStub,
+        networkId: '5',
+        chainId: '5',
+      });
+
+      const tokenData = {
+        standard: 'ERC1155',
+        decimals: 18,
+        symbol: 'DAI',
+        balance: '333',
+      };
+
+      metamaskController.tokenListController.update(() => {
+        return {
+          tokenList: {
+            '0xaaa75474e89094c44da98b954eedeac495271d0f': tokenData,
+          },
+        };
+      });
+
+      metamaskController.provider = provider;
+
+      sandbox
+        .stub(
+          metamaskController.assetsContractController,
+          'getTokenStandardAndDetails',
+        )
+        .callsFake(() => {
+          return tokenData;
+        });
+
+      const tokenDetails = await metamaskController.getTokenStandardAndDetails(
+        '0xAAA75474e89094c44da98b954eedeac495271d0f',
+        '0xf0d172594caedee459b89ad44c94098e474571b6',
+      );
+      assert.ok(
+        tokenDetails.standard === tokenData.standard.toUpperCase(),
+        'tokenDetails should include token standard in upper case',
+      );
+      assert.ok(
+        tokenDetails.decimals === String(tokenData.decimals),
+        'tokenDetails should include token decimals as a string',
+      );
+      assert.ok(
+        tokenDetails.symbol === tokenData.symbol,
+        'tokenDetails should include token symbol',
+      );
+      assert.ok(
+        tokenDetails.balance === tokenData.balance,
+        'tokenDetails should include a balance',
+      );
+    });
+  });
 });

coverage-targets.js

@@ -7,9 +7,9 @@
 module.exports = {
   global: {
     lines: 64.5,
-    branches: 53,
+    branches: 53.15,
-    statements: 63,
+    statements: 63.57,
-    functions: 56.5,
+    functions: 56.58,
   },
   transforms: {
     branches: 100,

shared/lib/token-util.ts

@@ -0,0 +1,37 @@
+import { abiERC20 } from '@metamask/metamask-eth-abis';
+import { Contract } from '@ethersproject/contracts';
+import { Web3Provider } from '@ethersproject/providers';
+
+/**
+ * Gets the '_value' parameter of the given token transaction data
+ * (i.e function call) per the Human Standard Token ABI, if present.
+ *
+ * @param {object} tokenData - ethers Interface token data.
+ * @returns {string | undefined} A decimal string value.
+ */
+/**
+ * Gets either the '_tokenId' parameter or the 'id' param of the passed token transaction data.,
+ * These are the parsed tokenId values returned by `parseStandardTokenTransactionData` as defined
+ * in the ERC721 and ERC1155 ABIs from metamask-eth-abis (https://github.com/MetaMask/metamask-eth-abis/tree/main/src/abis)
+ *
+ * @param tokenData - ethers Interface token data.
+ * @returns A decimal string value.
+ */
+export function getTokenIdParam(tokenData: any = {}): string | undefined {
+  return (
+    tokenData?.args?._tokenId?.toString() ?? tokenData?.args?.id?.toString()
+  );
+}
+
+export async function fetchTokenBalance(
+  address: string,
+  userAddress: string,
+  provider: any,
+): Promise<any> {
+  const ethersProvider = new Web3Provider(provider);
+  const tokenContract = new Contract(address, abiERC20, ethersProvider);
+  const tokenBalancePromise = tokenContract
+    ? tokenContract.balanceOf(userAddress)
+    : Promise.resolve();
+  return await tokenBalancePromise;
+}

ui/components/app/confirm-page-container/confirm-page-container.component.js

@@ -24,7 +24,7 @@ import Typography from '../../ui/typography';
 import { TypographyVariant } from '../../../helpers/constants/design-system';
 
 import NetworkAccountBalanceHeader from '../network-account-balance-header/network-account-balance-header';
-import { fetchTokenBalance } from '../../../pages/swaps/swaps.util';
+import { fetchTokenBalance } from '../../../../shared/lib/token-util.ts';
 import SetApproveForAllWarning from '../set-approval-for-all-warning';
 import { useI18nContext } from '../../../hooks/useI18nContext';
 ///: BEGIN:ONLY_INCLUDE_IN(flask)
@@ -132,7 +132,11 @@ const ConfirmPageContainer = (props) => {
     NETWORK_TO_NAME_MAP[currentTransaction.chainId] || networkIdentifier;
 
   const fetchCollectionBalance = useCallback(async () => {
-    const tokenBalance = await fetchTokenBalance(tokenAddress, fromAddress);
+    const tokenBalance = await fetchTokenBalance(
+      tokenAddress,
+      fromAddress,
+      global.ethereumProvider,
+    );
     setCollectionBalance(tokenBalance?.balance?.words?.[0] || 0);
   }, [fromAddress, tokenAddress]);
 

ui/pages/swaps/build-quote/build-quote.js

@@ -98,14 +98,11 @@ import {
   setSmartTransactionsOptInStatus,
   clearSmartTransactionFees,
 } from '../../../store/actions';
-import {
+import { countDecimals, fetchTokenPrice } from '../swaps.util';
-  countDecimals,
-  fetchTokenPrice,
-  fetchTokenBalance,
-} from '../swaps.util';
 import SwapsFooter from '../swaps-footer';
 import { isEqualCaseInsensitive } from '../../../../shared/modules/string-utils';
 import { calcTokenAmount } from '../../../../shared/lib/transactions-controller-utils';
+import { fetchTokenBalance } from '../../../../shared/lib/token-util.ts';
 import { shouldEnableDirectWrapping } from '../../../../shared/lib/swaps-utils';
 import {
   getValueFromWeiHex,
@@ -313,24 +310,26 @@ export default function BuildQuote({
         isEqualCaseInsensitive(usersToken.address, token.address),
       )
     ) {
-      fetchTokenBalance(token.address, selectedAccountAddress).then(
+      fetchTokenBalance(
-        (fetchedBalance) => {
+        token.address,
-          if (fetchedBalance?.balance) {
+        selectedAccountAddress,
-            const balanceAsDecString = fetchedBalance.balance.toString(10);
+        global.ethereumProvider,
-            const userTokenBalance = calcTokenAmount(
+      ).then((fetchedBalance) => {
-              balanceAsDecString,
+        if (fetchedBalance?.balance) {
-              token.decimals,
+          const balanceAsDecString = fetchedBalance.balance.toString(10);
-            );
+          const userTokenBalance = calcTokenAmount(
-            dispatch(
+            balanceAsDecString,
-              setSwapsFromToken({
+            token.decimals,
-                ...token,
+          );
-                string: userTokenBalance.toString(10),
+          dispatch(
-                balance: balanceAsDecString,
+            setSwapsFromToken({
-              }),
+              ...token,
-            );
+              string: userTokenBalance.toString(10),
-          }
+              balance: balanceAsDecString,
-        },
+            }),
-      );
+          );
+        }
+      });
     }
     dispatch(setSwapsFromToken(token));
     onInputChange(

ui/pages/swaps/build-quote/build-quote.test.js

@@ -8,6 +8,7 @@ import {
   setBackgroundConnection,
   fireEvent,
 } from '../../../../test/jest';
+import { createTestProviderTools } from '../../../../test/stub/provider';
 import {
   setSwapsFromToken,
   setSwapToToken,
@@ -61,8 +62,27 @@ jest.mock('../swaps.util', () => {
   };
 });
 
+const providerResultStub = {
+  eth_getCode: '0x123',
+  eth_call:
+    '0x00000000000000000000000000000000000000000000000029a2241af62c0000',
+};
+const { provider } = createTestProviderTools({
+  scaffold: providerResultStub,
+  networkId: '5',
+  chainId: '5',
+});
+
 describe('BuildQuote', () => {
+  beforeAll(() => {
+    jest.clearAllMocks();
+  });
+
   beforeEach(() => {
+    global.ethereumProvider = provider;
+  });
+
+  afterEach(() => {
     jest.clearAllMocks();
   });
 

ui/pages/swaps/swaps.util.ts

@@ -1,5 +1,4 @@
 import { BigNumber } from 'bignumber.js';
-import abi from 'human-standard-token-abi';
 import { Json } from '@metamask/controller-utils';
 import { IndividualTxFees } from '@metamask/smart-transactions-controller/dist/types';
 import {
@@ -215,17 +214,6 @@ export async function fetchTokenPrice(address: string): Promise<any> {
   return prices?.[address]?.eth;
 }
 
-export async function fetchTokenBalance(
-  address: string,
-  userAddress: string,
-): Promise<any> {
-  const tokenContract = (global as any).eth.contract(abi).at(address);
-  const tokenBalancePromise = tokenContract
-    ? tokenContract.balanceOf(userAddress)
-    : Promise.resolve();
-  return await tokenBalancePromise;
-}
-
 export async function fetchSwapsGasPrices(chainId: any): Promise<
   | any
   | {