Fix to infinite loading on approve screen (#14756)

app/_locales/en/messages.json

@@ -2066,6 +2066,9 @@
     "message": "Nonce is higher than suggested nonce of $1",
     "description": "The next nonce according to MetaMask's internal logic"
   },
+  "nft": {
+    "message": "NFT"
+  },
   "nftTokenIdPlaceholder": {
     "message": "Enter the Token ID"
   },

ui/hooks/useAssetDetails.js

@@ -1,7 +1,7 @@
 import { useState, useEffect } from 'react';
 import { useSelector, useDispatch } from 'react-redux';
 import { parseStandardTokenTransactionData } from '../../shared/modules/transaction.utils';
-import { getCollectibles, getTokens } from '../ducks/metamask/metamask';
+import { getCollectibles } from '../ducks/metamask/metamask';
 import { ERC1155, ERC721, ERC20 } from '../helpers/constants/common';
 import {
   calcTokenAmount,
@@ -9,16 +9,13 @@ import {
   getTokenAddressParam,
   getTokenValueParam,
 } from '../helpers/utils/token-util';
-import { getTokenList } from '../selectors';
 import { hideLoadingIndication, showLoadingIndication } from '../store/actions';
 import { usePrevious } from './usePrevious';
 
 export function useAssetDetails(tokenAddress, userAddress, transactionData) {
   const dispatch = useDispatch();
   // state selectors
-  const tokens = useSelector(getTokens);
   const collectibles = useSelector(getCollectibles);
-  const tokenList = useSelector(getTokenList);
 
   // in-hook state
   const [currentAsset, setCurrentAsset] = useState(null);
@@ -36,8 +33,6 @@ export function useAssetDetails(tokenAddress, userAddress, transactionData) {
         userAddress,
         transactionData,
         collectibles,
-        tokens,
-        tokenList,
       );
       setCurrentAsset(assetDetails);
       dispatch(hideLoadingIndication());
@@ -58,8 +53,6 @@ export function useAssetDetails(tokenAddress, userAddress, transactionData) {
     userAddress,
     transactionData,
     collectibles,
-    tokens,
-    tokenList,
   ]);
 
   let assetStandard,
@@ -83,11 +76,13 @@ export function useAssetDetails(tokenAddress, userAddress, transactionData) {
       balance,
       decimals: currentAssetDecimals,
     } = currentAsset;
+
     const tokenData = parseStandardTokenTransactionData(transactionData);
     assetStandard = standard;
     assetAddress = tokenAddress;
-    tokenSymbol = symbol;
+    tokenSymbol = symbol ?? '';
     tokenImage = image;
+
     toAddress = getTokenAddressParam(tokenData);
     if (assetStandard === ERC721 || assetStandard === ERC1155) {
       assetName = name;
@@ -101,6 +96,7 @@ export function useAssetDetails(tokenAddress, userAddress, transactionData) {
         calcTokenAmount(getTokenValueParam(tokenData), decimals).toString(10);
     }
   }
+
   return {
     assetStandard,
     assetName,

ui/hooks/useAssetDetails.test.js

@@ -0,0 +1,197 @@
+import React from 'react';
+import { Provider } from 'react-redux';
+import { renderHook } from '@testing-library/react-hooks';
+
+import configureStore from '../store/store';
+import * as tokenUtils from '../helpers/utils/token-util';
+import { ERC1155, ERC20, ERC721 } from '../helpers/constants/common';
+import { useAssetDetails } from './useAssetDetails';
+
+const renderUseAssetDetails = ({
+  tokenAddress,
+  userAddress,
+  transactionData,
+}) => {
+  const mockState = {
+    metamask: {
+      provider: {
+        type: 'test',
+        chainId: '0x3',
+      },
+      tokenList: {},
+    },
+  };
+
+  const wrapper = ({ children }) => (
+    <Provider store={configureStore(mockState)}>{children}</Provider>
+  );
+
+  return renderHook(
+    () => useAssetDetails(tokenAddress, userAddress, transactionData),
+    { wrapper },
+  );
+};
+
+describe('useAssetDetails', () => {
+  let getAssetDetailsStub;
+  beforeEach(() => {
+    getAssetDetailsStub = jest
+      .spyOn(tokenUtils, 'getAssetDetails')
+      .mockImplementation(() => Promise.resolve({}));
+  });
+  it('should return object with tokenSymbol set to and empty string, when getAssetDetails returns and empty object', async () => {
+    const toAddress = '000000000000000000000000000000000000dead';
+    const tokenAddress = '0x1';
+
+    const transactionData = `0xa9059cbb000000000000000000000000${toAddress}000000000000000000000000000000000000000000000000016345785d8a0000`;
+
+    const { result, waitForNextUpdate } = renderUseAssetDetails({
+      tokenAddress,
+      userAddress: '0x111',
+      transactionData,
+    });
+
+    await waitForNextUpdate();
+
+    expect(result.current).toStrictEqual({
+      assetAddress: tokenAddress,
+      assetName: undefined,
+      assetStandard: undefined,
+      decimals: undefined,
+      toAddress: `0x${toAddress}`,
+      tokenAmount: undefined,
+      tokenId: undefined,
+      tokenImage: undefined,
+      tokenSymbol: '',
+      tokenValue: undefined,
+      userBalance: undefined,
+    });
+  });
+
+  it('should return object with correct tokenValues for an ERC20 token', async () => {
+    const userAddress = '0xf04a5cc80b1e94c69b48f5ee68a08cd2f09a7c3e';
+    const tokenAddress = '0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2';
+    const toAddress = '000000000000000000000000000000000000dead';
+    const transactionData = `0xa9059cbb000000000000000000000000${toAddress}00000000000000000000000000000000000000000000000000000000000001f4`;
+
+    const standard = ERC20;
+    const symbol = 'WETH';
+    const balance = '1';
+    const decimals = 18;
+
+    getAssetDetailsStub.mockImplementation(() =>
+      Promise.resolve({
+        standard,
+        symbol,
+        balance,
+        decimals,
+      }),
+    );
+
+    const { result, waitForNextUpdate } = renderUseAssetDetails({
+      tokenAddress,
+      userAddress,
+      transactionData,
+    });
+
+    await waitForNextUpdate();
+
+    expect(result.current).toStrictEqual({
+      assetAddress: tokenAddress,
+      assetName: undefined,
+      assetStandard: standard,
+      decimals,
+      toAddress: `0x${toAddress}`,
+      tokenAmount: '0.0000000000000005',
+      tokenId: undefined,
+      tokenImage: undefined,
+      tokenSymbol: symbol,
+      tokenValue: undefined,
+      userBalance: balance,
+    });
+  });
+
+  it('should return object with correct tokenValues for an ERC721 token', async () => {
+    const tokenAddress = '0xBC4CA0EdA7647A8aB7C2061c2E118A18a936f13D';
+    const toAddress = '000000000000000000000000000000000000dead';
+    const transactionData = `0x23b872dd000000000000000000000000a544eebe103733f22ef62af556023bc918b73d36000000000000000000000000${toAddress}000000000000000000000000000000000000000000000000000000000000000c`;
+
+    const symbol = 'BAYC';
+    const tokenId = '12';
+    const name = 'BoredApeYachtClub';
+    const image =
+      'https://bafybeihw3gvmthmvrenfmcvagtais5tv7r4nmiezgsv7nyknjubxw4lite.ipfs.dweb.link';
+    const standard = ERC721;
+
+    getAssetDetailsStub.mockImplementation(() =>
+      Promise.resolve({
+        standard,
+        symbol,
+        name,
+        tokenId,
+        image,
+      }),
+    );
+
+    const { result, waitForNextUpdate } = renderUseAssetDetails({
+      tokenAddress,
+      transactionData,
+    });
+
+    await waitForNextUpdate();
+
+    expect(result.current).toStrictEqual({
+      assetAddress: tokenAddress,
+      assetName: name,
+      assetStandard: standard,
+      decimals: undefined,
+      toAddress: `0x${toAddress}`,
+      tokenId,
+      tokenImage: image,
+      tokenSymbol: symbol,
+      tokenValue: undefined,
+      userBalance: undefined,
+      tokenAmount: undefined,
+    });
+  });
+
+  it('should return object with correct tokenValues for an ERC1155 token', async () => {
+    const tokenAddress = '0x76BE3b62873462d2142405439777e971754E8E77';
+    const toAddress = '000000000000000000000000000000000000dead';
+    const transactionData = `0xf242432a000000000000000000000000a544eebe103733f22ef62af556023bc918b73d36000000000000000000000000000000000000000000000000000000000000dead0000000000000000000000000000000000000000000000000000000000000322000000000000000000000000000000000000000000000000000000000000009c00000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000`;
+
+    const tokenId = '121';
+    const image =
+      'https://bafybeihw3gvmthmvrenfmcvagtais5tv7r4nmiezgsv7nyknjubxw4lite.ipfs.dweb.link';
+    const standard = ERC1155;
+
+    getAssetDetailsStub.mockImplementation(() =>
+      Promise.resolve({
+        standard,
+        tokenId,
+        image,
+      }),
+    );
+
+    const { result, waitForNextUpdate } = renderUseAssetDetails({
+      tokenAddress,
+      transactionData,
+    });
+
+    await waitForNextUpdate();
+
+    expect(result.current).toStrictEqual({
+      assetAddress: tokenAddress,
+      assetName: undefined,
+      assetStandard: standard,
+      decimals: undefined,
+      toAddress: `0x${toAddress}`,
+      tokenId: undefined,
+      tokenImage: image,
+      tokenSymbol: '',
+      tokenValue: undefined,
+      userBalance: undefined,
+      tokenAmount: undefined,
+    });
+  });
+});

ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js

@@ -62,6 +62,7 @@ export default class ConfirmApproveContent extends Component {
     txData: PropTypes.object,
     fromAddressIsLedger: PropTypes.bool,
     chainId: PropTypes.string,
+    tokenAddress: PropTypes.string,
     rpcPrefs: PropTypes.object,
     isContract: PropTypes.bool,
     hexTransactionTotal: PropTypes.string,
@@ -183,7 +184,9 @@ export default class ConfirmApproveContent extends Component {
 
   renderERC721OrERC1155PermissionContent() {
     const { t } = this.context;
-    const { origin, toAddress, isContract, assetName, tokenId } = this.props;
+    const { origin, toAddress, isContract } = this.props;
+
+    const titleTokenDescription = this.getTitleTokenDescription();
 
     const displayedAddress = isContract
       ? `${t('contract')} (${addressSummary(toAddress)})`
@@ -198,7 +201,7 @@ export default class ConfirmApproveContent extends Component {
             {t('approvedAsset')}:
           </div>
           <div className="confirm-approve-content__medium-text">
-            {`${assetName} #${tokenId}`}
+            {titleTokenDescription}
           </div>
         </div>
         <div className="flex-row">
@@ -430,6 +433,82 @@ export default class ConfirmApproveContent extends Component {
     );
   }
 
+  getTitleTokenDescription() {
+    const {
+      tokenId,
+      assetName,
+      tokenAddress,
+      rpcPrefs,
+      chainId,
+      assetStandard,
+      tokenSymbol,
+    } = this.props;
+    const { t } = this.context;
+    let titleTokenDescription = t('token');
+    if (rpcPrefs?.blockExplorerUrl || chainId) {
+      const unknownTokenBlockExplorerLink = getTokenTrackerLink(
+        tokenAddress,
+        chainId,
+        null,
+        {
+          blockExplorerUrl: rpcPrefs?.blockExplorerUrl ?? null,
+        },
+      );
+
+      const unknownTokenLink = (
+        <a
+          href={unknownTokenBlockExplorerLink}
+          target="_blank"
+          rel="noopener noreferrer"
+          className="confirm-approve-content__unknown-asset"
+        >
+          {t('token')}
+        </a>
+      );
+      titleTokenDescription = unknownTokenLink;
+    }
+
+    if (assetStandard === ERC20 || (tokenSymbol && !tokenId)) {
+      titleTokenDescription = tokenSymbol;
+    } else if (
+      assetStandard === ERC721 ||
+      assetStandard === ERC1155 ||
+      // if we don't have an asset standard but we do have either both an assetname and a tokenID or both a tokenSymbol and tokenId we assume its an NFT
+      (assetName && tokenId) ||
+      (tokenSymbol && tokenId)
+    ) {
+      const tokenIdWrapped = tokenId ? ` (#${tokenId})` : null;
+      if (assetName || tokenSymbol) {
+        titleTokenDescription = `${assetName ?? tokenSymbol} ${tokenIdWrapped}`;
+      } else {
+        const unknownNFTBlockExplorerLink = getTokenTrackerLink(
+          tokenAddress,
+          chainId,
+          null,
+          {
+            blockExplorerUrl: rpcPrefs?.blockExplorerUrl ?? null,
+          },
+        );
+        const unknownNFTLink = (
+          <>
+            <a
+              href={unknownNFTBlockExplorerLink}
+              target="_blank"
+              rel="noopener noreferrer"
+              className="confirm-approve-content__unknown-asset"
+            >
+              {t('nft')}
+            </a>
+            {tokenIdWrapped && <span>{tokenIdWrapped}</span>}
+          </>
+        );
+        titleTokenDescription = unknownNFTLink;
+      }
+    }
+
+    return titleTokenDescription;
+  }
+
   render() {
     const { t } = this.context;
     const {
@@ -452,11 +531,11 @@ export default class ConfirmApproveContent extends Component {
       rpcPrefs,
       isContract,
       assetStandard,
-      tokenId,
-      assetName,
     } = this.props;
     const { showFullTxDetails } = this.state;
 
+    const titleTokenDescription = this.getTitleTokenDescription();
+
     return (
       <div
         className={classnames('confirm-approve-content', {
@@ -496,11 +575,7 @@ export default class ConfirmApproveContent extends Component {
           </Box>
         </Box>
         <div className="confirm-approve-content__title">
-          {t('allowSpendToken', [
-            assetStandard === ERC20
-              ? tokenSymbol
-              : `${assetName} (#${tokenId})`,
-          ])}
+          {t('allowSpendToken', [titleTokenDescription])}
         </div>
         <div className="confirm-approve-content__description">
           {t('trustSiteApprovePermission', [
@@ -554,7 +629,9 @@ export default class ConfirmApproveContent extends Component {
                   : getAccountLink(
                       toAddress,
                       chainId,
-                      { blockExplorerUrl: rpcPrefs?.blockExplorerUrl ?? null },
+                      {
+                        blockExplorerUrl: rpcPrefs?.blockExplorerUrl ?? null,
+                      },
                       null,
                     );
                 global.platform.openTab({

ui/pages/confirm-approve/confirm-approve-content/index.scss

@@ -9,6 +9,10 @@
     padding: 0 24px 16px 24px;
   }
 
+  &__unknown-asset {
+    color: var(--color-primary-default);
+  }
+
   &__icon-display-content {
     display: flex;
     height: 51px;

ui/pages/confirm-approve/confirm-approve.js

@@ -52,6 +52,7 @@ export default function ConfirmApprove({
   tokenId,
   userAddress,
   toAddress,
+  tokenAddress,
   transaction,
   ethTransactionTotal,
   fiatTransactionTotal,
@@ -173,6 +174,7 @@ export default function ConfirmApprove({
               tokenId={tokenId}
               assetName={assetName}
               assetStandard={assetStandard}
+              tokenAddress={tokenAddress}
               showCustomizeGasModal={approveTransaction}
               showEditApprovalPermissionModal={({
                 /* eslint-disable no-shadow */
@@ -268,6 +270,7 @@ export default function ConfirmApprove({
 ConfirmApprove.propTypes = {
   assetStandard: PropTypes.string,
   assetName: PropTypes.string,
+  tokenAddress: PropTypes.string,
   userBalance: PropTypes.string,
   tokenSymbol: PropTypes.string,
   decimals: PropTypes.string,