From 88664bbbe9a3f0d583ac0cb07619fb662a558e09 Mon Sep 17 00:00:00 2001 From: Ariella Vu <20778143+digiwand@users.noreply.github.com> Date: Thu, 19 Sep 2024 15:55:07 +0800 Subject: [PATCH] refactor: memoize fetchErc20Decimals, relocate to utils/token, and apply to other instances (#27088) --- .../value-display/value-display.tsx | 29 +++++++---- .../components/confirm/row/dataTree.tsx | 14 +++--- .../useBalanceChanges.test.ts | 6 +++ .../simulation-details/useBalanceChanges.ts | 24 +-------- .../confirmations/confirm/confirm.test.tsx | 4 ++ ui/pages/confirmations/constants/token.ts | 1 + ui/pages/confirmations/utils/token.test.ts | 50 +++++++++++++++++++ ui/pages/confirmations/utils/token.ts | 32 ++++++++++++ 8 files changed, 119 insertions(+), 41 deletions(-) create mode 100644 ui/pages/confirmations/constants/token.ts create mode 100644 ui/pages/confirmations/utils/token.test.ts create mode 100644 ui/pages/confirmations/utils/token.ts diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/value-display/value-display.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/value-display/value-display.tsx index d0cd9c529d0e..25fad3020103 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/value-display/value-display.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/value-display/value-display.tsx @@ -1,7 +1,7 @@ import React, { useMemo } from 'react'; import { NameType } from '@metamask/name-controller'; +import { Hex } from '@metamask/utils'; import { captureException } from '@sentry/browser'; -import { getTokenStandardAndDetails } from '../../../../../../../../store/actions'; import { shortenString } from '../../../../../../../../helpers/utils/util'; import { calcTokenAmount } from '../../../../../../../../../shared/lib/transactions-controller-utils'; @@ -27,23 +27,30 @@ import { TextAlign, } from '../../../../../../../../helpers/constants/design-system'; import Name from '../../../../../../../../components/app/name/name'; +import { fetchErc20Decimals } from '../../../../../../utils/token'; -const getTokenDecimals = async (tokenContract: string) => { - const tokenDetails = await getTokenStandardAndDetails(tokenContract); - const tokenDecimals = tokenDetails?.decimals; +type PermitSimulationValueDisplayParams = { + /** The primaryType of the typed sign message */ + primaryType?: string; - return parseInt(tokenDecimals ?? '0', 10); -}; + /** + * The ethereum token contract address. It is expected to be in hex format. + * We currently accept strings since we have a patch that accepts a custom string + * {@see .yarn/patches/@metamask-eth-json-rpc-middleware-npm-14.0.1-b6c2ccbe8c.patch} + */ + tokenContract: Hex | string; -const PermitSimulationValueDisplay: React.FC<{ - primaryType?: string; - tokenContract: string; + /** The token amount */ value: number | string; -}> = ({ primaryType, tokenContract, value }) => { +}; + +const PermitSimulationValueDisplay: React.FC< + PermitSimulationValueDisplayParams +> = ({ primaryType, tokenContract, value }) => { const exchangeRate = useTokenExchangeRate(tokenContract); const { value: tokenDecimals } = useAsyncResult( - async () => await getTokenDecimals(tokenContract), + async () => await fetchErc20Decimals(tokenContract), [tokenContract], ); diff --git a/ui/pages/confirmations/components/confirm/row/dataTree.tsx b/ui/pages/confirmations/components/confirm/row/dataTree.tsx index f728b9d5dfa9..aebd1c702aa6 100644 --- a/ui/pages/confirmations/components/confirm/row/dataTree.tsx +++ b/ui/pages/confirmations/components/confirm/row/dataTree.tsx @@ -1,3 +1,4 @@ +import { Hex } from '@metamask/utils'; import React, { memo } from 'react'; import { @@ -6,9 +7,8 @@ import { PRIMARY_TYPES_PERMIT, } from '../../../../../../shared/constants/signatures'; import { isValidHexAddress } from '../../../../../../shared/modules/hexstring-utils'; -import { sanitizeString } from '../../../../../helpers/utils/util'; -import { getTokenStandardAndDetails } from '../../../../../store/actions'; +import { sanitizeString } from '../../../../../helpers/utils/util'; import { Box } from '../../../../../components/component-library'; import { BlockSize } from '../../../../../helpers/constants/design-system'; import { useAsyncResult } from '../../../../../hooks/useAsyncResult'; @@ -19,6 +19,7 @@ import { ConfirmInfoRowText, ConfirmInfoRowTextTokenUnits, } from '../../../../../components/app/confirm/info/row'; +import { fetchErc20Decimals } from '../../../utils/token'; type ValueType = string | Record | TreeData[]; @@ -68,15 +69,12 @@ const getTokenDecimalsOfDataTree = async ( } const tokenContract = (dataTreeData as Record).token - ?.value as string; - if (!tokenContract) { + ?.value as Hex; + if (!tokenContract || !isValidHexAddress(tokenContract)) { return undefined; } - const tokenDetails = await getTokenStandardAndDetails(tokenContract); - const tokenDecimals = tokenDetails?.decimals; - - return parseInt(tokenDecimals ?? '0', 10); + return await fetchErc20Decimals(tokenContract); }; export const DataTree = ({ diff --git a/ui/pages/confirmations/components/simulation-details/useBalanceChanges.test.ts b/ui/pages/confirmations/components/simulation-details/useBalanceChanges.test.ts index 64d9632c75a7..10e4cca518b7 100644 --- a/ui/pages/confirmations/components/simulation-details/useBalanceChanges.test.ts +++ b/ui/pages/confirmations/components/simulation-details/useBalanceChanges.test.ts @@ -9,6 +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 { useBalanceChanges } from './useBalanceChanges'; import { FIAT_UNAVAILABLE } from './types'; @@ -89,6 +90,11 @@ describe('useBalanceChanges', () => { }); }); + afterEach(() => { + /** Reset memoized function for each test */ + fetchErc20Decimals?.cache?.clear?.(); + }); + describe('pending states', () => { it('returns pending=true if no simulation data', async () => { const { result, waitForNextUpdate } = renderHook(() => diff --git a/ui/pages/confirmations/components/simulation-details/useBalanceChanges.ts b/ui/pages/confirmations/components/simulation-details/useBalanceChanges.ts index 6f2a033cc567..2a198d76ea36 100644 --- a/ui/pages/confirmations/components/simulation-details/useBalanceChanges.ts +++ b/ui/pages/confirmations/components/simulation-details/useBalanceChanges.ts @@ -9,11 +9,12 @@ import { import { BigNumber } from 'bignumber.js'; import { ContractExchangeRates } from '@metamask/assets-controllers'; import { useAsyncResultOrThrow } from '../../../../hooks/useAsyncResult'; -import { getTokenStandardAndDetails } from '../../../../store/actions'; import { TokenStandard } from '../../../../../shared/constants/transaction'; import { getConversionRate } from '../../../../ducks/metamask/metamask'; import { getCurrentChainId, getCurrentCurrency } from '../../../../selectors'; import { fetchTokenExchangeRates } from '../../../../helpers/utils/util'; +import { ERC20_DEFAULT_DECIMALS, fetchErc20Decimals } from '../../utils/token'; + import { BalanceChange, FIAT_UNAVAILABLE, @@ -23,8 +24,6 @@ import { const NATIVE_DECIMALS = 18; -const ERC20_DEFAULT_DECIMALS = 18; - // See https://github.com/MikeMcl/bignumber.js/issues/11#issuecomment-23053776 function convertNumberToStringWithPrecisionWarning(value: number): string { return String(value); @@ -57,25 +56,6 @@ function getAssetAmount( ); } -// Fetches the decimals for the given token address. -async function fetchErc20Decimals(address: Hex): Promise { - try { - const { decimals: decStr } = await getTokenStandardAndDetails(address); - if (!decStr) { - return ERC20_DEFAULT_DECIMALS; - } - for (const radix of [10, 16]) { - const parsedDec = parseInt(decStr, radix); - if (isFinite(parsedDec)) { - return parsedDec; - } - } - return ERC20_DEFAULT_DECIMALS; - } catch { - return ERC20_DEFAULT_DECIMALS; - } -} - // Fetches token details for all the token addresses in the SimulationTokenBalanceChanges async function fetchAllErc20Decimals( addresses: Hex[], diff --git a/ui/pages/confirmations/confirm/confirm.test.tsx b/ui/pages/confirmations/confirm/confirm.test.tsx index 0b10976ab48c..dbf4dab7dfaf 100644 --- a/ui/pages/confirmations/confirm/confirm.test.tsx +++ b/ui/pages/confirmations/confirm/confirm.test.tsx @@ -18,6 +18,7 @@ import { 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 Confirm from './confirm'; jest.mock('react-router-dom', () => ({ @@ -32,6 +33,9 @@ const middleware = [thunk]; describe('Confirm', () => { afterEach(() => { jest.resetAllMocks(); + + /** Reset memoized function using getTokenStandardAndDetails for each test */ + fetchErc20Decimals?.cache?.clear?.(); }); it('should render', () => { diff --git a/ui/pages/confirmations/constants/token.ts b/ui/pages/confirmations/constants/token.ts new file mode 100644 index 000000000000..4520a77d3585 --- /dev/null +++ b/ui/pages/confirmations/constants/token.ts @@ -0,0 +1 @@ +export const ERC20_DEFAULT_DECIMALS = 18; diff --git a/ui/pages/confirmations/utils/token.test.ts b/ui/pages/confirmations/utils/token.test.ts new file mode 100644 index 000000000000..e71813713d79 --- /dev/null +++ b/ui/pages/confirmations/utils/token.test.ts @@ -0,0 +1,50 @@ +import { getTokenStandardAndDetails } from '../../../store/actions'; +import { ERC20_DEFAULT_DECIMALS } from '../constants/token'; +import { fetchErc20Decimals } from './token'; + +const MOCK_ADDRESS = '0x514910771af9ca656af840dff83e8264ecf986ca'; +const MOCK_DECIMALS = 36; + +jest.mock('../../../store/actions', () => ({ + getTokenStandardAndDetails: jest.fn(), +})); + +describe('fetchErc20Decimals', () => { + afterEach(() => { + jest.clearAllMocks(); + + /** Reset memoized function using getTokenStandardAndDetails for each test */ + fetchErc20Decimals?.cache?.clear?.(); + }); + + it(`should return the default number, ${ERC20_DEFAULT_DECIMALS}, if no decimals were found from details`, async () => { + (getTokenStandardAndDetails as jest.Mock).mockResolvedValue({}); + const decimals = await fetchErc20Decimals(MOCK_ADDRESS); + + expect(decimals).toBe(ERC20_DEFAULT_DECIMALS); + }); + + it('should return the decimals for a given token address', async () => { + (getTokenStandardAndDetails as jest.Mock).mockResolvedValue({ + decimals: MOCK_DECIMALS, + }); + const decimals = await fetchErc20Decimals(MOCK_ADDRESS); + + expect(decimals).toBe(MOCK_DECIMALS); + }); + + it('should memoize the result for the same token addresses', async () => { + (getTokenStandardAndDetails as jest.Mock).mockResolvedValue({ + decimals: MOCK_DECIMALS, + }); + + const firstCallResult = await fetchErc20Decimals(MOCK_ADDRESS); + const secondCallResult = await fetchErc20Decimals(MOCK_ADDRESS); + + expect(firstCallResult).toBe(secondCallResult); + expect(getTokenStandardAndDetails).toHaveBeenCalledTimes(1); + + await fetchErc20Decimals('0xDifferentAddress'); + expect(getTokenStandardAndDetails).toHaveBeenCalledTimes(2); + }); +}); diff --git a/ui/pages/confirmations/utils/token.ts b/ui/pages/confirmations/utils/token.ts new file mode 100644 index 000000000000..1f94280129a9 --- /dev/null +++ b/ui/pages/confirmations/utils/token.ts @@ -0,0 +1,32 @@ +import { memoize } from 'lodash'; +import { Hex } from '@metamask/utils'; +import { getTokenStandardAndDetails } from '../../../store/actions'; + +export const ERC20_DEFAULT_DECIMALS = 18; + +/** + * Fetches the decimals for the given token address. + * + * @param {Hex | string} address - The ethereum token contract address. It is expected to be in hex format. + * We currently accept strings since we have a patch that accepts a custom string + * {@see .yarn/patches/@metamask-eth-json-rpc-middleware-npm-14.0.1-b6c2ccbe8c.patch} + */ +export const fetchErc20Decimals = memoize( + async (address: Hex | string): Promise => { + try { + const { decimals: decStr } = await getTokenStandardAndDetails(address); + if (!decStr) { + return ERC20_DEFAULT_DECIMALS; + } + for (const radix of [10, 16]) { + const parsedDec = parseInt(decStr, radix); + if (isFinite(parsedDec)) { + return parsedDec; + } + } + return ERC20_DEFAULT_DECIMALS; + } catch { + return ERC20_DEFAULT_DECIMALS; + } + }, +);