Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: memoize fetchErc20Decimals, relocate to utils/token, and apply to other instances #27088

Merged
merged 21 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
b8fec4a
refactor: mv fetchErc20Decimals to utils/token
digiwand Sep 12, 2024
d9913ab
fix: lint needs EOL
digiwand Sep 12, 2024
171c4a1
refactor: use fetchErc20Decimals in permit simulation and dataTree
digiwand Sep 12, 2024
eff0525
refactor: memoize fetchErc20Decimals
digiwand Sep 12, 2024
a83c8af
fix: lint add EOF line
digiwand Sep 12, 2024
557d8a4
test: memoized fetchErc20Decimals util
digiwand Sep 12, 2024
fb228be
test:fix: fetchErc20Decimals test phrasing
digiwand Sep 12, 2024
282a8a6
fix:test: uncomment needed afterEach code
digiwand Sep 12, 2024
6f9b3c3
fix: yarn lint:fix
digiwand Sep 12, 2024
68fa1c5
fix:test: useBalanceChanges memoized fetchErc20Decimals
digiwand Sep 12, 2024
7c4afaf
fix:test: mv useBalanceChanges memoized clear cache
digiwand Sep 12, 2024
d07cc85
Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…
digiwand Sep 12, 2024
595975e
fix: allow string with hex token addresses
digiwand Sep 12, 2024
6ab6f3b
fix:test: memoize fetchErc20Decimals for confirm test
digiwand Sep 12, 2024
09a9fee
fix: lint utils/token comment
digiwand Sep 12, 2024
dba0ba3
Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…
digiwand Sep 12, 2024
5602fd9
Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…
digiwand Sep 13, 2024
913a79a
refactor: rm new utils/token.ts
digiwand Sep 16, 2024
a7c315d
Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…
digiwand Sep 16, 2024
4f8e366
fix: cherry-pick PermitSimulationValueDisplay code
digiwand Sep 18, 2024
99cfb20
Revert "fix: cherry-pick PermitSimulationValueDisplay code"
digiwand Sep 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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],
);

Expand Down
14 changes: 6 additions & 8 deletions ui/pages/confirmations/components/confirm/row/dataTree.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Hex } from '@metamask/utils';
import React, { memo } from 'react';

import {
Expand All @@ -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';
Expand All @@ -19,6 +19,7 @@ import {
ConfirmInfoRowText,
ConfirmInfoRowTextTokenUnits,
} from '../../../../../components/app/confirm/info/row';
import { fetchErc20Decimals } from '../../../utils/token';

type ValueType = string | Record<string, TreeData> | TreeData[];

Expand Down Expand Up @@ -68,15 +69,12 @@ const getTokenDecimalsOfDataTree = async (
}

const tokenContract = (dataTreeData as Record<string, TreeData>).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 = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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(() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -57,25 +56,6 @@ function getAssetAmount(
);
}

// Fetches the decimals for the given token address.
async function fetchErc20Decimals(address: Hex): Promise<number> {
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[],
Expand Down
4 changes: 4 additions & 0 deletions ui/pages/confirmations/confirm/confirm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand All @@ -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', () => {
Expand Down
1 change: 1 addition & 0 deletions ui/pages/confirmations/constants/token.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, given there is very little in index.ts currently, do we warrant this level of modularity?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

questionable, so updated this to share ui/pages/confirmations/utils/token.ts

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const ERC20_DEFAULT_DECIMALS = 18;
50 changes: 50 additions & 0 deletions ui/pages/confirmations/utils/token.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
32 changes: 32 additions & 0 deletions ui/pages/confirmations/utils/token.ts
Original file line number Diff line number Diff line change
@@ -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<number> => {
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;
}
},
);