feat: add “Incomplete Asset Displayed” metric & fix: should only set …

…default decimals if ERC20 (#27494) ## **Description** ### Overall - Adds "Incomplete Asset Displayed" metric when token detail decimals are not found for Permit Simulations of ERC20 tokens - Fixes issue where decimals may default to 18 when the token is not identified as an ERC20 token ### Details - Refactors fetchErc20Decimals - Defines types in token.ts - Create new useGetTokenStandardAndDetails - Includes new `decimalsNumber` value in return to be used instead of `decimals` (string type) ## **Related issues** Fixes: #27333 ## **Manual testing steps** NA ## **Screenshots/Recordings** NA ## **Pre-merge author checklist** - [X] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [X] I've completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [X] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Jyoti Puri <jyotipuri@gmail.com>
MetaMask · Oct 18, 2024 · 9b370bd · 9b370bd
1 parent 36bde61
commit 9b370bd
Show file tree

Hide file tree

Showing 16 changed files with 336 additions and 108 deletions.
diff --git a/test/e2e/tests/confirmations/signatures/permit.spec.ts b/test/e2e/tests/confirmations/signatures/permit.spec.ts
@@ -126,7 +126,7 @@ async function assertInfoValues(driver: Driver) {
     css: '.name__value',
     text: '0x5B38D...eddC4',
   });
-  const value = driver.findElement({ text: '<0.000001' });
+  const value = driver.findElement({ text: '3,000' });
   const nonce = driver.findElement({ text: '0' });
   const deadline = driver.findElement({ text: '09 June 3554, 16:53' });
 

diff --git a/test/integration/confirmations/signatures/permit.test.tsx b/test/integration/confirmations/signatures/permit.test.tsx
@@ -73,7 +73,7 @@ describe('Permit Confirmation', () => {
     jest.resetAllMocks();
     mockedBackgroundConnection.submitRequestToBackground.mockImplementation(
       createMockImplementation({
-        getTokenStandardAndDetails: { decimals: '2' },
+        getTokenStandardAndDetails: { decimals: '2', standard: 'ERC20' },
       }),
     );
   });

diff --git a/...irmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.test.tsx b/...irmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.test.tsx
@@ -8,15 +8,25 @@ import {
   permitNFTSignatureMsg,
   permitSignatureMsg,
 } from '../../../../../../../../test/data/confirmations/typed_sign';
+import { memoizedGetTokenStandardAndDetails } from '../../../../../utils/token';
 import PermitSimulation from './permit-simulation';
 
 jest.mock('../../../../../../../store/actions', () => {
   return {
-    getTokenStandardAndDetails: jest.fn().mockResolvedValue({ decimals: 2 }),
+    getTokenStandardAndDetails: jest
+      .fn()
+      .mockResolvedValue({ decimals: 2, standard: 'ERC20' }),
   };
 });
 
 describe('PermitSimulation', () => {
+  afterEach(() => {
+    jest.clearAllMocks();
+
+    /** Reset memoized function using getTokenStandardAndDetails for each test */
+    memoizedGetTokenStandardAndDetails?.cache?.clear?.();
+  });
+
   it('renders component correctly', async () => {
     const state = getMockTypedSignConfirmStateForRequest(permitSignatureMsg);
     const mockStore = configureMockStore([])(state);

diff --git a/...info/typed-sign/permit-simulation/value-display/__snapshots__/value-display.test.tsx.snap b/...info/typed-sign/permit-simulation/value-display/__snapshots__/value-display.test.tsx.snap
@@ -56,49 +56,3 @@ exports[`PermitSimulationValueDisplay renders component correctly 1`] = `
   </div>
 </div>
 `;
-
-exports[`PermitSimulationValueDisplay renders component correctly for NFT token 1`] = `
-<div>
-  <div
-    class="mm-box"
-  >
-    <div
-      class="mm-box mm-box--display-flex mm-box--justify-content-flex-end"
-    >
-      <div
-        class="mm-box mm-box--margin-inline-end-1 mm-box--display-inline mm-box--min-width-0"
-      >
-        <div>
-          <p
-            class="mm-box mm-text mm-text--body-md mm-text--text-align-center mm-box--padding-inline-2 mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-xl"
-            data-testid="simulation-token-value"
-            style="padding-top: 1px; padding-bottom: 1px;"
-          >
-            #4321
-          </p>
-        </div>
-      </div>
-      <div
-        class="mm-box mm-box--display-flex"
-      >
-        <div
-          class="name name__missing"
-        >
-          <span
-            class="mm-box name__icon mm-icon mm-icon--size-md mm-box--display-inline-block mm-box--color-inherit"
-            style="mask-image: url('./images/icons/question.svg');"
-          />
-          <p
-            class="mm-box mm-text name__value mm-text--body-md mm-box--color-text-default"
-          >
-            0xA0b86...6eB48
-          </p>
-        </div>
-      </div>
-    </div>
-    <div
-      class="mm-box"
-    />
-  </div>
-</div>
-`;
diff --git a/...components/confirm/info/typed-sign/permit-simulation/value-display/value-display.test.tsx b/...components/confirm/info/typed-sign/permit-simulation/value-display/value-display.test.tsx
@@ -4,14 +4,24 @@ import configureMockStore from 'redux-mock-store';
 
 import mockState from '../../../../../../../../../test/data/mock-state.json';
 import { renderWithProvider } from '../../../../../../../../../test/lib/render-helpers';
+import useTrackERC20WithoutDecimalInformation from '../../../../../../hooks/useTrackERC20WithoutDecimalInformation';
 import PermitSimulationValueDisplay from './value-display';
 
 jest.mock('../../../../../../../../store/actions', () => {
   return {
-    getTokenStandardAndDetails: jest.fn().mockResolvedValue({ decimals: 4 }),
+    getTokenStandardAndDetails: jest
+      .fn()
+      .mockResolvedValue({ decimals: 4, standard: 'ERC20' }),
   };
 });
 
+jest.mock(
+  '../../../../../../hooks/useTrackERC20WithoutDecimalInformation',
+  () => {
+    return jest.fn();
+  },
+);
+
 describe('PermitSimulationValueDisplay', () => {
   it('renders component correctly', async () => {
     const mockStore = configureMockStore([])(mockState);
@@ -30,20 +40,19 @@ describe('PermitSimulationValueDisplay', () => {
     });
   });
 
-  it('renders component correctly for NFT token', async () => {
+  it('should invoke method to track missing decimal information for ERC20 tokens', async () => {
     const mockStore = configureMockStore([])(mockState);
 
     await act(async () => {
-      const { container, findByText } = renderWithProvider(
+      renderWithProvider(
         <PermitSimulationValueDisplay
           tokenContract="0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48"
-          tokenId="4321"
+          value="4321"
         />,
         mockStore,
       );
 
-      expect(await findByText('#4321')).toBeInTheDocument();
-      expect(container).toMatchSnapshot();
+      expect(useTrackERC20WithoutDecimalInformation).toHaveBeenCalled();
     });
   });
 });
diff --git a/...ions/components/confirm/info/typed-sign/permit-simulation/value-display/value-display.tsx b/...ions/components/confirm/info/typed-sign/permit-simulation/value-display/value-display.tsx
@@ -2,16 +2,18 @@ import React, { useMemo } from 'react';
 import { NameType } from '@metamask/name-controller';
 import { Hex } from '@metamask/utils';
 import { captureException } from '@sentry/browser';
-import { shortenString } from '../../../../../../../../helpers/utils/util';
 
+import { MetaMetricsEventLocation } from '../../../../../../../../../shared/constants/metametrics';
+import { shortenString } from '../../../../../../../../helpers/utils/util';
 import { calcTokenAmount } from '../../../../../../../../../shared/lib/transactions-controller-utils';
 import useTokenExchangeRate from '../../../../../../../../components/app/currency-input/hooks/useTokenExchangeRate';
 import { IndividualFiatDisplay } from '../../../../../simulation-details/fiat-display';
 import {
   formatAmount,
   formatAmountMaxPrecision,
 } from '../../../../../simulation-details/formatAmount';
-import { useAsyncResult } from '../../../../../../../../hooks/useAsyncResult';
+import { useGetTokenStandardAndDetails } from '../../../../../../hooks/useGetTokenStandardAndDetails';
+import useTrackERC20WithoutDecimalInformation from '../../../../../../hooks/useTrackERC20WithoutDecimalInformation';
 
 import {
   Box,
@@ -27,7 +29,7 @@ import {
   TextAlign,
 } from '../../../../../../../../helpers/constants/design-system';
 import Name from '../../../../../../../../components/app/name/name';
-import { fetchErc20Decimals } from '../../../../../../utils/token';
+import { TokenDetailsERC20 } from '../../../../../../utils/token';
 
 type PermitSimulationValueDisplayParams = {
   /** The primaryType of the typed sign message */
@@ -52,12 +54,13 @@ const PermitSimulationValueDisplay: React.FC<
 > = ({ primaryType, tokenContract, value, tokenId }) => {
   const exchangeRate = useTokenExchangeRate(tokenContract);
 
-  const { value: tokenDecimals } = useAsyncResult(async () => {
-    if (tokenId) {
-      return undefined;
-    }
-    return await fetchErc20Decimals(tokenContract);
-  }, [tokenContract]);
+  const tokenDetails = useGetTokenStandardAndDetails(tokenContract);
+  useTrackERC20WithoutDecimalInformation(
+    tokenContract,
+    tokenDetails as TokenDetailsERC20,
+    MetaMetricsEventLocation.SignatureConfirmation,
+  );
+  const { decimalsNumber: tokenDecimals } = tokenDetails;
 
   const fiatValue = useMemo(() => {
     if (exchangeRate && value && !tokenId) {

diff --git a/ui/pages/confirmations/components/confirm/row/dataTree.tsx b/ui/pages/confirmations/components/confirm/row/dataTree.tsx
@@ -11,7 +11,6 @@ import { isValidHexAddress } from '../../../../../../shared/modules/hexstring-ut
 import { sanitizeString } from '../../../../../helpers/utils/util';
 import { Box } from '../../../../../components/component-library';
 import { BlockSize } from '../../../../../helpers/constants/design-system';
-import { useAsyncResult } from '../../../../../hooks/useAsyncResult';
 import { useI18nContext } from '../../../../../hooks/useI18nContext';
 import {
   ConfirmInfoRow,
@@ -20,7 +19,7 @@ import {
   ConfirmInfoRowText,
   ConfirmInfoRowTextTokenUnits,
 } from '../../../../../components/app/confirm/info/row';
-import { fetchErc20Decimals } from '../../../utils/token';
+import { useGetTokenStandardAndDetails } from '../../../hooks/useGetTokenStandardAndDetails';
 
 type ValueType = string | Record<string, TreeData> | TreeData[];
 
@@ -78,9 +77,9 @@ const NONE_DATE_VALUE = -1;
  *
  * @param dataTreeData
  */
-const getTokenDecimalsOfDataTree = async (
+const getTokenContractInDataTree = (
   dataTreeData: Record<string, TreeData> | TreeData[],
-): Promise<void | number> => {
+): Hex | undefined => {
   if (Array.isArray(dataTreeData)) {
     return undefined;
   }
@@ -91,7 +90,7 @@ const getTokenDecimalsOfDataTree = async (
     return undefined;
   }
 
-  return await fetchErc20Decimals(tokenContract);
+  return tokenContract;
 };
 
 export const DataTree = ({
@@ -103,13 +102,10 @@ export const DataTree = ({
   primaryType?: PrimaryType;
   tokenDecimals?: number;
 }) => {
-  const { value: decimalsResponse } = useAsyncResult(
-    async () => await getTokenDecimalsOfDataTree(data),
-    [data],
-  );
-
+  const tokenContract = getTokenContractInDataTree(data);
+  const { decimalsNumber } = useGetTokenStandardAndDetails(tokenContract);
   const tokenDecimals =
-    typeof decimalsResponse === 'number' ? decimalsResponse : tokenDecimalsProp;
+    typeof decimalsNumber === 'number' ? decimalsNumber : tokenDecimalsProp;
 
   return (
     <Box width={BlockSize.Full}>

diff --git a/ui/pages/confirmations/components/confirm/row/typed-sign-data-v1/typedSignDataV1.test.tsx b/ui/pages/confirmations/components/confirm/row/typed-sign-data-v1/typedSignDataV1.test.tsx
@@ -1,22 +1,28 @@
 import React from 'react';
-import { render } from '@testing-library/react';
+import configureMockStore from 'redux-mock-store';
+import mockState from '../../../../../../../test/data/mock-state.json';
+import { renderWithProvider } from '../../../../../../../test/lib/render-helpers';
 import { unapprovedTypedSignMsgV1 } from '../../../../../../../test/data/confirmations/typed_sign';
 import { TypedSignDataV1Type } from '../../../../types/confirm';
 import { ConfirmInfoRowTypedSignDataV1 } from './typedSignDataV1';
 
+const mockStore = configureMockStore([])(mockState);
+
 describe('ConfirmInfoRowTypedSignData', () => {
   it('should match snapshot', () => {
-    const { container } = render(
+    const { container } = renderWithProvider(
       <ConfirmInfoRowTypedSignDataV1
         data={unapprovedTypedSignMsgV1.msgParams?.data as TypedSignDataV1Type}
       />,
+      mockStore,
     );
     expect(container).toMatchSnapshot();
   });
 
   it('should return null if data is not defined', () => {
-    const { container } = render(
+    const { container } = renderWithProvider(
       <ConfirmInfoRowTypedSignDataV1 data={undefined} />,
+      mockStore,
     );
     expect(container).toBeEmptyDOMElement();
   });

diff --git a/ui/pages/confirmations/components/simulation-details/useBalanceChanges.test.ts b/ui/pages/confirmations/components/simulation-details/useBalanceChanges.test.ts
@@ -9,7 +9,7 @@ import { TokenStandard } from '../../../../../shared/constants/transaction';
 import { getConversionRate } from '../../../../ducks/metamask/metamask';
 import { getTokenStandardAndDetails } from '../../../../store/actions';
 import { fetchTokenExchangeRates } from '../../../../helpers/utils/util';
-import { fetchErc20Decimals } from '../../utils/token';
+import { memoizedGetTokenStandardAndDetails } from '../../utils/token';
 import { useBalanceChanges } from './useBalanceChanges';
 import { FIAT_UNAVAILABLE } from './types';
 
@@ -92,7 +92,7 @@ describe('useBalanceChanges', () => {
 
   afterEach(() => {
     /** Reset memoized function for each test */
-    fetchErc20Decimals?.cache?.clear?.();
+    memoizedGetTokenStandardAndDetails?.cache?.clear?.();
   });
 
   describe('pending states', () => {

diff --git a/ui/pages/confirmations/confirm/confirm.test.tsx b/ui/pages/confirmations/confirm/confirm.test.tsx
@@ -17,7 +17,7 @@ import mockState from '../../../../test/data/mock-state.json';
 import { renderWithConfirmContextProvider } from '../../../../test/lib/confirmations/render-helpers';
 import * as actions from '../../../store/actions';
 import { SignatureRequestType } from '../types/confirm';
-import { fetchErc20Decimals } from '../utils/token';
+import { memoizedGetTokenStandardAndDetails } from '../utils/token';
 import Confirm from './confirm';
 
 jest.mock('react-router-dom', () => ({
@@ -34,7 +34,7 @@ describe('Confirm', () => {
     jest.resetAllMocks();
 
     /** Reset memoized function using getTokenStandardAndDetails for each test */
-    fetchErc20Decimals?.cache?.clear?.();
+    memoizedGetTokenStandardAndDetails?.cache?.clear?.();
   });
 
   it('should render', () => {
@@ -59,7 +59,7 @@ describe('Confirm', () => {
 
     jest.spyOn(actions, 'getTokenStandardAndDetails').mockResolvedValue({
       decimals: '2',
-      standard: 'erc20',
+      standard: 'ERC20',
     });
 
     const mockStore = configureMockStore(middleware)(mockStateTypedSign);
@@ -103,7 +103,7 @@ describe('Confirm', () => {
 
     jest.spyOn(actions, 'getTokenStandardAndDetails').mockResolvedValue({
       decimals: '2',
-      standard: 'erc20',
+      standard: 'ERC20',
     });
 
     const mockStore = configureMockStore(middleware)(mockStateTypedSign);
@@ -146,7 +146,7 @@ describe('Confirm', () => {
 
     jest.spyOn(actions, 'getTokenStandardAndDetails').mockResolvedValue({
       decimals: '2',
-      standard: 'erc20',
+      standard: 'ERC20',
     });
 
     await act(async () => {
@@ -170,7 +170,7 @@ describe('Confirm', () => {
 
     jest.spyOn(actions, 'getTokenStandardAndDetails').mockResolvedValue({
       decimals: '2',
-      standard: 'erc20',
+      standard: 'ERC20',
     });
 
     await act(async () => {