From e1ab90d8066e79956ec66b63020bbf386c1fda8e Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Tue, 18 Feb 2025 02:02:26 +0000 Subject: [PATCH 01/19] Add static rows Temporarily add mock simulation data. Add useBatchApproveTokenSimulation hook. Add BatchSimulationDetails component. --- .../hooks/use-approve-token-simulation.ts | 59 +++++++++++--- .../base-transaction-info.tsx | 8 +- .../batch-simulation-details.tsx | 77 ++++++++++++++++++ .../hooks/useBatchApproveTokenSimulation.ts | 78 +++++++++++++++++++ .../simulation-details/simulation-details.tsx | 16 ++++ 5 files changed, 220 insertions(+), 18 deletions(-) create mode 100644 ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx create mode 100644 ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveTokenSimulation.ts diff --git a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts index 7548919daeaf..1d2ec37b2ed6 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts +++ b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts @@ -2,14 +2,48 @@ import { TransactionMeta } from '@metamask/transaction-controller'; import { BigNumber } from 'bignumber.js'; import { useMemo } from 'react'; import { useSelector } from 'react-redux'; +import { Hex } from '@metamask/utils'; import { calcTokenAmount } from '../../../../../../../../shared/lib/transactions-controller-utils'; import { getIntlLocale } from '../../../../../../../ducks/locale/locale'; import { formatAmount } from '../../../../simulation-details/formatAmount'; import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../../shared/constants'; -import { useTokenTransactionData } from '../../hooks/useTokenTransactionData'; +import { parseStandardTokenTransactionData } from '../../../../../../../../shared/modules/transaction.utils'; import { useIsNFT } from './use-is-nft'; -function isSpendingCapUnlimited(decodedSpendingCap: number) { +export function decodeApproveTokenData({ + data, + decimals, + to, +}: { + data?: Hex; + decimals?: number; + locale: string; + to?: Hex; +}) { + if (!data || !to) { + return undefined; + } + + const transactionDescription = parseStandardTokenTransactionData(data); + + if (!transactionDescription) { + return undefined; + } + + const parsedArgs = transactionDescription.args; + + const value = + parsedArgs?._value ?? // ERC-20 - approve + parsedArgs?.increment; // Fiat Token V2 - increaseAllowance + + if (!value) { + return undefined; + } + + return calcTokenAmount(value, decimals ?? 0); +} + +export function isSpendingCapUnlimited(decodedSpendingCap: number) { return decodedSpendingCap >= TOKEN_VALUE_UNLIMITED_THRESHOLD; } @@ -19,18 +53,19 @@ export const useApproveTokenSimulation = ( ) => { const locale = useSelector(getIntlLocale); const { isNFT, pending: isNFTPending } = useIsNFT(transactionMeta); - const { args: parsedArgs } = useTokenTransactionData() ?? {}; + const data = transactionMeta.txParams.data as Hex | undefined; + const to = transactionMeta.txParams.to as Hex | undefined; + const decimalsNumber = decimals ? Number(decimals) : 0; - const parsedValue = - parsedArgs?._value ?? // ERC-20 - approve - parsedArgs?.increment; // Fiat Token V2 - increaseAllowance - - const value = parsedValue ?? new BigNumber(0); + const value = + decodeApproveTokenData({ + data, + decimals: decimalsNumber, + locale, + to, + }) ?? new BigNumber(0); - const decodedSpendingCap = calcTokenAmount( - value, - Number(decimals ?? '0'), - ).toFixed(); + const decodedSpendingCap = value.toFixed(); const tokenPrefix = isNFT ? '#' : ''; diff --git a/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx b/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx index 2b986aa42f3f..449579ba215f 100644 --- a/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx +++ b/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx @@ -1,10 +1,10 @@ import { TransactionMeta } from '@metamask/transaction-controller'; import React from 'react'; import { useConfirmContext } from '../../../../context/confirm'; -import { SimulationDetails } from '../../../simulation-details'; import { AdvancedDetails } from '../shared/advanced-details/advanced-details'; import { GasFeesSection } from '../shared/gas-fees-section/gas-fees-section'; import { TransactionDetails } from '../shared/transaction-details/transaction-details'; +import { BatchSimulationDetails } from '../batch/batch-simulation-details/batch-simulation-details'; const BaseTransactionInfo = () => { const { currentConfirmation: transactionMeta } = @@ -16,11 +16,7 @@ const BaseTransactionInfo = () => { return ( <> - + diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx new file mode 100644 index 000000000000..d6a5699ae1c9 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx @@ -0,0 +1,77 @@ +import React from 'react'; +import { + SimulationTokenStandard, + TransactionMeta, +} from '@metamask/transaction-controller'; +import { + SimulationDetails, + StaticRow, +} from '../../../../simulation-details/simulation-details'; +import { useBatchApproveBalanceChanges } from '../../hooks/useBatchApproveTokenSimulation'; +import { useConfirmContext } from '../../../../../context/confirm'; + +export function BatchSimulationDetails() { + const { currentConfirmation: transactionMeta } = + useConfirmContext(); + + const { value: approveBalanceChanges } = + useBatchApproveBalanceChanges() ?? {}; + + const mockTransactionMeta: TransactionMeta = { + ...transactionMeta, + simulationData: { + nativeBalanceChange: { + previousBalance: '0x123456789', + newBalance: '0x123456789abcdef', + difference: '0x123456789123456', + isDecrease: false, + }, + tokenBalanceChanges: [ + { + address: '0x1234567890123456789012345678901234567890', + previousBalance: '0x123456789', + newBalance: '0x123456789abcdef', + difference: '0x123456789123456789123', + isDecrease: false, + standard: SimulationTokenStandard.erc20, + }, + { + address: '0x2234567890123456789012345678901234567890', + previousBalance: '0x1', + newBalance: '0x0', + difference: '0x1', + isDecrease: true, + standard: SimulationTokenStandard.erc721, + id: '0x123', + }, + ], + }, + }; + + const approvePositiveBalanceChanges = (approveBalanceChanges ?? []).filter( + (balanceChange) => balanceChange.amount.gt(0), + ); + + const approveNegativeBalanceChanges = (approveBalanceChanges ?? []).filter( + (balanceChange) => balanceChange.amount.eq(0), + ); + + const approveRow: StaticRow = { + label: 'You approve', + balanceChanges: approvePositiveBalanceChanges, + }; + + const revokeRow: StaticRow = { + label: 'You revoke', + balanceChanges: approveNegativeBalanceChanges, + }; + + return ( + + ); +} diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveTokenSimulation.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveTokenSimulation.ts new file mode 100644 index 000000000000..fdfa776f3e4b --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveTokenSimulation.ts @@ -0,0 +1,78 @@ +import { + BatchTransactionParams, + TransactionMeta, +} from '@metamask/transaction-controller'; +import { useConfirmContext } from '../../../../context/confirm'; +import { decodeApproveTokenData } from '../approve/hooks/use-approve-token-simulation'; +import { getIntlLocale } from '../../../../../../ducks/locale/locale'; +import { useSelector } from 'react-redux'; +import { BalanceChange } from '../../../simulation-details/types'; +import { useAsyncResult } from '../../../../../../hooks/useAsyncResult'; +import { getTokenStandardAndDetails } from '../../../../../../store/actions'; +import { Hex } from '@metamask/utils'; +import { fetchErc20Decimals } from '../../../../utils/token'; + +export function useBatchApproveBalanceChanges() { + const locale = useSelector(getIntlLocale); + const { currentConfirmation } = useConfirmContext(); + const { chainId, nestedTransactions } = currentConfirmation ?? {}; + + if (!nestedTransactions?.length) { + return undefined; + } + + const { pending, value } = useAsyncResult( + async () => + await buildBalanceChanges({ + chainId: chainId as Hex, + locale, + nestedTransactions, + }), + [chainId, locale, nestedTransactions], + ); + + return { pending, value }; +} + +async function buildBalanceChanges({ + chainId, + locale, + nestedTransactions, +}: { + chainId: Hex; + locale: string; + nestedTransactions: BatchTransactionParams[]; +}): Promise { + const balanceChanges: BalanceChange[] = []; + + for (const transaction of nestedTransactions) { + const { data, to } = transaction; + + if (!data || !to) { + continue; + } + + const decimals = await fetchErc20Decimals(to); + const approveValue = await decodeApproveTokenData({ data, decimals, locale, to }); + + if (approveValue == undefined) { + continue; + } + + const { standard: tokenStandard } = await getTokenStandardAndDetails(to); + + const balanceChange: BalanceChange = { + asset: { + address: transaction.to, + chainId, + standard: tokenStandard as any, + }, + amount: approveValue, + fiatAmount: null, + }; + + balanceChanges.push(balanceChange); + } + + return balanceChanges; +} diff --git a/ui/pages/confirmations/components/simulation-details/simulation-details.tsx b/ui/pages/confirmations/components/simulation-details/simulation-details.tsx index 9fbc8309251b..f7e513b7dc2c 100644 --- a/ui/pages/confirmations/components/simulation-details/simulation-details.tsx +++ b/ui/pages/confirmations/components/simulation-details/simulation-details.tsx @@ -31,11 +31,18 @@ import { useI18nContext } from '../../../../hooks/useI18nContext'; import { BalanceChangeList } from './balance-change-list'; import { useBalanceChanges } from './useBalanceChanges'; import { useSimulationMetrics } from './useSimulationMetrics'; +import { BalanceChange } from './types'; + +export type StaticRow = { + label: string; + balanceChanges: BalanceChange[]; +}; export type SimulationDetailsProps = { enableMetrics?: boolean; isTransactionsRedesign?: boolean; metricsOnly?: boolean; + staticRows?: StaticRow[]; transaction: TransactionMeta; }; @@ -252,12 +259,14 @@ const SimulationDetailsLayout: React.FC<{ * @param props.isTransactionsRedesign - Whether or not the component is being * used inside the transaction redesign flow. * @param props.metricsOnly - Whether to only track metrics and not render the UI. + * @param props.staticRows - Optional static rows to display. */ export const SimulationDetails: React.FC = ({ transaction, enableMetrics = false, isTransactionsRedesign = false, metricsOnly = false, + staticRows = [], }: SimulationDetailsProps) => { const t = useI18nContext(); const { chainId, id: transactionId, simulationData } = transaction; @@ -329,6 +338,7 @@ export const SimulationDetails: React.FC = ({ const outgoing = balanceChanges.filter((bc) => bc.amount.isNegative()); const incoming = balanceChanges.filter((bc) => !bc.amount.isNegative()); + return ( = ({ balanceChanges={incoming} testId="simulation-rows-incoming" /> + {staticRows.map((staticRow) => ( + + ))} ); From 072107248cd2dc5b6ec28ce47019c8850f93f505 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 19 Feb 2025 23:42:13 +0000 Subject: [PATCH 02/19] Add batch simulation storybook --- .../confirmations/contract-interaction.ts | 4 + ui/__mocks__/actions.js | 6 + .../batch-simulation-details.stories.tsx | 130 ++++++++++++++++++ .../batch-simulation-details.tsx | 50 +------ .../batch/batch-simulation-details/index.ts | 1 + .../hooks/useBatchApproveTokenSimulation.ts | 40 ++++-- .../simulation-details/amount-pill.tsx | 51 +++++-- .../simulation-details/balance-change-row.tsx | 4 +- .../components/simulation-details/types.ts | 3 + 9 files changed, 222 insertions(+), 67 deletions(-) create mode 100644 ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.stories.tsx create mode 100644 ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/index.ts diff --git a/test/data/confirmations/contract-interaction.ts b/test/data/confirmations/contract-interaction.ts index b6263ca49da2..25e056cd6a5b 100644 --- a/test/data/confirmations/contract-interaction.ts +++ b/test/data/confirmations/contract-interaction.ts @@ -1,4 +1,5 @@ import { + BatchTransactionParams, CHAIN_IDS, SimulationData, TransactionMeta, @@ -25,11 +26,13 @@ export const genUnapprovedContractInteractionConfirmation = ({ address = CONTRACT_INTERACTION_SENDER_ADDRESS, txData = DEPOSIT_METHOD_DATA, chainId = CHAIN_ID, + nestedTransactions, simulationData, }: { address?: Hex; txData?: Hex; chainId?: string; + nestedTransactions?: BatchTransactionParams[]; simulationData?: SimulationData; } = {}): Confirmation => { const confirmation: Confirmation = { @@ -135,6 +138,7 @@ export const genUnapprovedContractInteractionConfirmation = ({ ], id: '1d7c08c0-fe54-11ee-9243-91b1e533746a', origin: 'https://metamask.github.io', + nestedTransactions, securityAlertResponse: { features: [], reason: '', diff --git a/ui/__mocks__/actions.js b/ui/__mocks__/actions.js index c3c20ba70ad1..56bf28992388 100644 --- a/ui/__mocks__/actions.js +++ b/ui/__mocks__/actions.js @@ -10,6 +10,7 @@ const { const ERC20_TOKEN_1_MOCK = '0x2260fac5e5542a773aa44fbcfedf7c193bc2c599'; // WBTC const ERC20_TOKEN_2_MOCK = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; // USDC +const ERC721_TOKEN_MOCK = '0x06012c8cf97bead5deae237070f9587f8e7a266d'; // CryptoKitties const TOKEN_DETAILS_MOCK = { [ERC20_TOKEN_1_MOCK]: { @@ -24,6 +25,11 @@ const TOKEN_DETAILS_MOCK = { decimals: 6, name: 'USD Coin', }, + [ERC721_TOKEN_MOCK]: { + address: ERC721_TOKEN_MOCK, + standard: 'ERC721', + name: 'CryptoKitties', + }, }; module.exports = { diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.stories.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.stories.tsx new file mode 100644 index 000000000000..6926f035eb73 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.stories.tsx @@ -0,0 +1,130 @@ +import React from 'react'; +import { Provider } from 'react-redux'; + +import configureStore from '../../../../../../../store/store'; +import { BatchSimulationDetails } from './batch-simulation-details'; +import { ConfirmContextProvider } from '../../../../../context/confirm'; +import { + CHAIN_ID, + genUnapprovedContractInteractionConfirmation, +} from '../../../../../../../../test/data/confirmations/contract-interaction'; +import { getMockConfirmStateForTransaction } from '../../../../../../../../test/data/confirmations/helper'; +import { + CHAIN_IDS, + SimulationTokenStandard, +} from '@metamask/transaction-controller'; +import { NameType } from '@metamask/name-controller'; +import { Hex } from '@metamask/utils'; + +const ERC20_TOKEN_1_MOCK = '0x2260fac5e5542a773aa44fbcfedf7c193bc2c599'; // WBTC +const ERC20_TOKEN_2_MOCK = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; // USDC +const ERC721_TOKEN_MOCK = '0x06012c8cf97bead5deae237070f9587f8e7a266d'; // CryptoKitties +const ERC1155_TOKEN_MOCK = '0x60e4d786628fea6478f785a6d7e704777c86a7c6'; // MAYC + +const DUMMY_BALANCE_CHANGE = { + previousBalance: '0xIGNORED' as Hex, + newBalance: '0xIGNORED' as Hex, +}; + +const TRANSACTION_MOCK = genUnapprovedContractInteractionConfirmation({ + nestedTransactions: [ + { + data: '0x095ea7b30000000000000000000000009bc5baf874d2da8d216ae9f137804184ee5afef40000000000000000000000000000000000000000000000000000000123456901', + to: ERC20_TOKEN_2_MOCK, + }, + { + data: '0x095ea7b30000000000000000000000009bc5baf874d2da8d216ae9f137804184ee5afef40000000000000000000000000000000000000000000000000000000000000721', + to: ERC721_TOKEN_MOCK, + } + ], + simulationData: { + nativeBalanceChange: { + ...DUMMY_BALANCE_CHANGE, + difference: '0x12345678912345678', + isDecrease: true, + }, + tokenBalanceChanges: [ + { + ...DUMMY_BALANCE_CHANGE, + address: ERC20_TOKEN_1_MOCK, + difference: '0x123456', + isDecrease: false, + standard: SimulationTokenStandard.erc20, + }, + { + ...DUMMY_BALANCE_CHANGE, + address: ERC1155_TOKEN_MOCK, + difference: '0x13', + isDecrease: false, + id: '0x1155', + standard: SimulationTokenStandard.erc1155, + }, + ], + }, +}); + +const STATE_MOCK = getMockConfirmStateForTransaction(TRANSACTION_MOCK, { + metamask: { + names: { + [NameType.ETHEREUM_ADDRESS]: { + ['0x1234567890123456789012345678901234567890']: { + [CHAIN_ID]: { + name: 'USDC', + }, + }, + }, + }, + tokensChainsCache: { + [CHAIN_ID]: { + data: { + [ERC20_TOKEN_1_MOCK]: { + address: ERC20_TOKEN_1_MOCK, + symbol: 'WBTC', + name: 'Wrapped Bitcoin', + iconUrl: `https://static.cx.metamask.io/api/v1/tokenIcons/1/${ERC20_TOKEN_1_MOCK}.png`, + }, + [ERC20_TOKEN_2_MOCK]: { + address: ERC20_TOKEN_2_MOCK, + symbol: 'USDC', + name: 'USD Coin', + iconUrl: `https://static.cx.metamask.io/api/v1/tokenIcons/1/${ERC20_TOKEN_2_MOCK}.png`, + }, + [ERC721_TOKEN_MOCK]: { + address: ERC721_TOKEN_MOCK, + symbol: 'CK', + name: 'CryptoKitties', + iconUrl: `https://static.cx.metamask.io/api/v1/tokenIcons/1/${ERC721_TOKEN_MOCK}.png`, + }, + [ERC1155_TOKEN_MOCK]: { + address: ERC1155_TOKEN_MOCK, + symbol: 'MAYC', + name: 'Mutant Ape Yacht Club', + iconUrl: `https://static.cx.metamask.io/api/v1/tokenIcons/1/${ERC1155_TOKEN_MOCK}.png `, + }, + }, + }, + }, + }, +}); + +const store = configureStore(STATE_MOCK); + +const Story = { + title: 'Confirmations/Components/Confirm/BatchSimulationDetails', + component: BatchSimulationDetails, + decorators: [ + (story) => { + return ( + + {story()} + + ); + }, + ], +}; + +export default Story; + +export const DefaultStory = () => ; + +DefaultStory.storyName = 'Default'; diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx index d6a5699ae1c9..c47bd03df27e 100644 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx @@ -17,59 +17,15 @@ export function BatchSimulationDetails() { const { value: approveBalanceChanges } = useBatchApproveBalanceChanges() ?? {}; - const mockTransactionMeta: TransactionMeta = { - ...transactionMeta, - simulationData: { - nativeBalanceChange: { - previousBalance: '0x123456789', - newBalance: '0x123456789abcdef', - difference: '0x123456789123456', - isDecrease: false, - }, - tokenBalanceChanges: [ - { - address: '0x1234567890123456789012345678901234567890', - previousBalance: '0x123456789', - newBalance: '0x123456789abcdef', - difference: '0x123456789123456789123', - isDecrease: false, - standard: SimulationTokenStandard.erc20, - }, - { - address: '0x2234567890123456789012345678901234567890', - previousBalance: '0x1', - newBalance: '0x0', - difference: '0x1', - isDecrease: true, - standard: SimulationTokenStandard.erc721, - id: '0x123', - }, - ], - }, - }; - - const approvePositiveBalanceChanges = (approveBalanceChanges ?? []).filter( - (balanceChange) => balanceChange.amount.gt(0), - ); - - const approveNegativeBalanceChanges = (approveBalanceChanges ?? []).filter( - (balanceChange) => balanceChange.amount.eq(0), - ); - const approveRow: StaticRow = { label: 'You approve', - balanceChanges: approvePositiveBalanceChanges, - }; - - const revokeRow: StaticRow = { - label: 'You revoke', - balanceChanges: approveNegativeBalanceChanges, + balanceChanges: approveBalanceChanges ?? [], }; return ( diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/index.ts b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/index.ts new file mode 100644 index 000000000000..5b116c56dce9 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/index.ts @@ -0,0 +1 @@ +export { BatchSimulationDetails } from './batch-simulation-details'; diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveTokenSimulation.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveTokenSimulation.ts index fdfa776f3e4b..3911fac7cec7 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveTokenSimulation.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveTokenSimulation.ts @@ -6,11 +6,16 @@ import { useConfirmContext } from '../../../../context/confirm'; import { decodeApproveTokenData } from '../approve/hooks/use-approve-token-simulation'; import { getIntlLocale } from '../../../../../../ducks/locale/locale'; import { useSelector } from 'react-redux'; -import { BalanceChange } from '../../../simulation-details/types'; +import { + BalanceChange, + TokenAssetIdentifier, +} from '../../../simulation-details/types'; import { useAsyncResult } from '../../../../../../hooks/useAsyncResult'; import { getTokenStandardAndDetails } from '../../../../../../store/actions'; -import { Hex } from '@metamask/utils'; +import { Hex, add0x } from '@metamask/utils'; import { fetchErc20Decimals } from '../../../../utils/token'; +import { TokenStandard } from '../../../../../../../shared/constants/transaction'; +import { toHex } from '@metamask/controller-utils'; export function useBatchApproveBalanceChanges() { const locale = useSelector(getIntlLocale); @@ -52,23 +57,40 @@ async function buildBalanceChanges({ continue; } - const decimals = await fetchErc20Decimals(to); - const approveValue = await decodeApproveTokenData({ data, decimals, locale, to }); + const tokenData = await getTokenStandardAndDetails(to); + const tokenStandard = tokenData?.standard as TokenStandard; + let decimals = undefined; + + if (tokenStandard === TokenStandard.ERC20) { + decimals = await fetchErc20Decimals(to); + } + + const approveAmountOrId = await decodeApproveTokenData({ + data, + decimals, + locale, + to, + }); - if (approveValue == undefined) { + if (approveAmountOrId == undefined) { continue; } - const { standard: tokenStandard } = await getTokenStandardAndDetails(to); + const tokenId = + tokenStandard === TokenStandard.ERC721 + ? add0x(approveAmountOrId.toString(16)) + : undefined; const balanceChange: BalanceChange = { asset: { - address: transaction.to, + address: to, chainId, - standard: tokenStandard as any, + standard: tokenStandard as TokenAssetIdentifier['standard'], + tokenId, }, - amount: approveValue, + amount: approveAmountOrId, fiatAmount: null, + isApproval: true, }; balanceChanges.push(balanceChange); diff --git a/ui/pages/confirmations/components/simulation-details/amount-pill.tsx b/ui/pages/confirmations/components/simulation-details/amount-pill.tsx index a8ad71cfadc6..dbac49ba6407 100644 --- a/ui/pages/confirmations/components/simulation-details/amount-pill.tsx +++ b/ui/pages/confirmations/components/simulation-details/amount-pill.tsx @@ -26,24 +26,25 @@ import { formatAmount, formatAmountMaxPrecision } from './formatAmount'; * @param props * @param props.asset * @param props.amount + * @param props.isApproval */ export const AmountPill: React.FC<{ asset: AssetIdentifier; amount: BigNumber; -}> = ({ asset, amount }) => { + isApproval?: boolean; +}> = ({ asset, amount, isApproval }) => { const locale = useSelector(getIntlLocale); - const backgroundColor = amount.isNegative() - ? BackgroundColor.errorMuted - : BackgroundColor.successMuted; - - const color = amount.isNegative() - ? TextColor.errorAlternative - : TextColor.successDefault; + const backgroundColor = getBackgroundColour({ amount, isApproval }); + const color = getColor({ amount, isApproval }); - const amountParts: string[] = [amount.isNegative() ? '-' : '+']; + const amountParts: string[] = []; const tooltipParts: string[] = []; + if (!isApproval) { + amountParts.push(amount.isNegative() ? '-' : '+'); + } + // ERC721 amounts are always 1 and are not displayed. if (asset.standard !== TokenStandard.ERC721) { const formattedAmount = formatAmount(locale, amount.abs()); @@ -98,3 +99,35 @@ export const AmountPill: React.FC<{ ); }; + +function getBackgroundColour({ + amount, + isApproval, +}: { + amount: BigNumber; + isApproval?: boolean; +}) { + if (isApproval) { + return BackgroundColor.backgroundMuted; + } + + return amount.isNegative() + ? BackgroundColor.errorMuted + : BackgroundColor.successMuted; +} + +function getColor({ + amount, + isApproval, +}: { + amount: BigNumber; + isApproval?: boolean; +}) { + if (isApproval) { + return TextColor.textDefault; + } + + return amount.isNegative() + ? TextColor.errorAlternative + : TextColor.successDefault; +} diff --git a/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx b/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx index 3a26d27f7ae7..a6cd2d372bba 100644 --- a/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx +++ b/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx @@ -25,7 +25,7 @@ export const BalanceChangeRow: React.FC<{ showFiat?: boolean; balanceChange: BalanceChange; }> = ({ label, showFiat, balanceChange }) => { - const { asset, amount, fiatAmount } = balanceChange; + const { asset, amount, fiatAmount, isApproval } = balanceChange; return ( - + {showFiat && } diff --git a/ui/pages/confirmations/components/simulation-details/types.ts b/ui/pages/confirmations/components/simulation-details/types.ts index c1a3290f6838..577c161ef013 100644 --- a/ui/pages/confirmations/components/simulation-details/types.ts +++ b/ui/pages/confirmations/components/simulation-details/types.ts @@ -58,4 +58,7 @@ export type BalanceChange = Readonly<{ * The amount of fiat currency that corresponds to the asset amount. */ fiatAmount: FiatAmount; + + /** Whether the balance change is a token approval. */ + isApproval?: boolean; }>; From 304d5aab207ab27effdf0f766c5ffd8015aacc78 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 20 Feb 2025 00:46:42 +0000 Subject: [PATCH 03/19] Show spending cap cards --- .../approve/spending-cap/spending-cap.tsx | 29 +++++++++++--- .../base-transaction-info.tsx | 2 + .../batch-simulation-details.tsx | 5 +-- .../batch-spending-cap/batch-spending-cap.tsx | 40 +++++++++++++++++++ 4 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 ui/pages/confirmations/components/confirm/info/batch/batch-spending-cap/batch-spending-cap.tsx diff --git a/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx b/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx index 3a8c6a5a26ec..29277ae31fb6 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx +++ b/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx @@ -1,5 +1,6 @@ import { TransactionMeta } from '@metamask/transaction-controller'; import React from 'react'; +import { Hex } from '@metamask/utils'; import { calcTokenAmount } from '../../../../../../../../shared/lib/transactions-controller-utils'; import { ConfirmInfoRow, @@ -18,16 +19,15 @@ const SpendingCapGroup = ({ tokenSymbol, decimals, setIsOpenEditSpendingCapModal, + transactionMeta, }: { tokenSymbol: string; decimals: string; setIsOpenEditSpendingCapModal: (newValue: boolean) => void; + transactionMeta: TransactionMeta; }) => { const t = useI18nContext(); - const { currentConfirmation: transactionMeta } = - useConfirmContext(); - const { spendingCap, isUnlimitedSpendingCap, formattedSpendingCap, value } = useApproveTokenSimulation(transactionMeta, decimals); @@ -69,19 +69,26 @@ const SpendingCapGroup = ({ }; export const SpendingCap = ({ + data, setIsOpenEditSpendingCapModal, + to, }: { + data?: Hex; setIsOpenEditSpendingCapModal: (newValue: boolean) => void; + to?: Hex; }) => { const t = useI18nContext(); const { currentConfirmation: transactionMeta } = useConfirmContext(); + const transactionTo = to ?? transactionMeta.txParams.to; + const transactionData = data ?? transactionMeta.txParams.data; + const { userBalance, tokenSymbol, decimals } = useAssetDetails( - transactionMeta.txParams.to, + transactionTo, transactionMeta.txParams.from, - transactionMeta.txParams.data, + transactionData, transactionMeta.chainId, ); @@ -90,7 +97,16 @@ export const SpendingCap = ({ Number(decimals ?? '0'), ).toFixed(); - const { pending } = useApproveTokenSimulation(transactionMeta, decimals); + const finalTransactionMeta = { + ...transactionMeta, + txParams: { + ...transactionMeta.txParams, + to: transactionTo, + data: transactionData, + }, + }; + + const { pending } = useApproveTokenSimulation(finalTransactionMeta, decimals); if (pending) { return ; @@ -106,6 +122,7 @@ export const SpendingCap = ({ tokenSymbol={tokenSymbol || ''} decimals={decimals || '0'} setIsOpenEditSpendingCapModal={setIsOpenEditSpendingCapModal} + transactionMeta={finalTransactionMeta} /> ); diff --git a/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx b/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx index 449579ba215f..51066fbff5e8 100644 --- a/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx +++ b/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx @@ -5,6 +5,7 @@ import { AdvancedDetails } from '../shared/advanced-details/advanced-details'; import { GasFeesSection } from '../shared/gas-fees-section/gas-fees-section'; import { TransactionDetails } from '../shared/transaction-details/transaction-details'; import { BatchSimulationDetails } from '../batch/batch-simulation-details/batch-simulation-details'; +import { BatchSpendingCap } from '../batch/batch-spending-cap/batch-spending-cap'; const BaseTransactionInfo = () => { const { currentConfirmation: transactionMeta } = @@ -17,6 +18,7 @@ const BaseTransactionInfo = () => { return ( <> + diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx index c47bd03df27e..de905aeba4fa 100644 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx @@ -1,8 +1,5 @@ import React from 'react'; -import { - SimulationTokenStandard, - TransactionMeta, -} from '@metamask/transaction-controller'; +import { TransactionMeta } from '@metamask/transaction-controller'; import { SimulationDetails, StaticRow, diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-spending-cap/batch-spending-cap.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-spending-cap/batch-spending-cap.tsx new file mode 100644 index 000000000000..27bb1eec9f0c --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-spending-cap/batch-spending-cap.tsx @@ -0,0 +1,40 @@ +import React from 'react'; +import { + BatchTransactionParams, + TransactionMeta, +} from '@metamask/transaction-controller'; +import { useConfirmContext } from '../../../../../context/confirm'; +import { Box } from '../../../../../../../components/component-library'; +import { SpendingCap } from '../../approve/spending-cap/spending-cap'; + +export function BatchSpendingCap() { + const { currentConfirmation } = useConfirmContext(); + const { nestedTransactions } = currentConfirmation ?? {}; + + if (!nestedTransactions?.length) { + return null; + } + + return ( + + {nestedTransactions.map((nestedTransaction, index) => ( + + ))} + + ); +} + +function NestedTransactionSpendingCap({ + nestedTransaction, +}: { + nestedTransaction: BatchTransactionParams; +}) { + const { data, to } = nestedTransaction; + + return ( + {}} /> + ); +} From 004db186ec63c292db85ef4754effa9bb162637d Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 20 Feb 2025 07:56:05 +0000 Subject: [PATCH 04/19] Add unit test --- .../batch-simulation-details.test.tsx | 88 +++++++++++++++++++ .../batch-simulation-details.tsx | 2 +- ...on.ts => useBatchApproveBalanceChanges.ts} | 1 - .../simulation-details/simulation-details.tsx | 2 +- 4 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx rename ui/pages/confirmations/components/confirm/info/hooks/{useBatchApproveTokenSimulation.ts => useBatchApproveBalanceChanges.ts} (98%) diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx new file mode 100644 index 000000000000..e4ed9aa137c5 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx @@ -0,0 +1,88 @@ +import React from 'react'; +import { BatchTransactionParams } from '@metamask/transaction-controller'; +import { renderWithConfirmContextProvider } from '../../../../../../../../test/lib/confirmations/render-helpers'; +import configureStore from '../../../../../../../store/store'; +import { getMockConfirmStateForTransaction } from '../../../../../../../../test/data/confirmations/helper'; +import { genUnapprovedContractInteractionConfirmation } from '../../../../../../../../test/data/confirmations/contract-interaction'; +import { BatchSimulationDetails } from './batch-simulation-details'; +import { useBatchApproveBalanceChanges } from '../../hooks/useBatchApproveBalanceChanges'; +import { AlertMetricsProvider } from '../../../../../../../components/app/alert-system/contexts/alertMetricsContext'; +import { useBalanceChanges } from '../../../../simulation-details/useBalanceChanges'; +import BigNumber from 'bignumber.js'; +import { BalanceChange } from '../../../../simulation-details/types'; +import { TokenStandard } from '../../../../../../../../shared/constants/transaction'; + +jest.mock('../../../../simulation-details/useBalanceChanges', () => ({ + useBalanceChanges: jest.fn(), +})); + +jest.mock('../../hooks/useBatchApproveBalanceChanges', () => ({ + useBatchApproveBalanceChanges: jest.fn(), +})); + +const BALANCE_CHANGE_MOCK: BalanceChange = { + asset: { + address: '0x1234567891234567891234567891234567891234', + chainId: '0x123', + standard: TokenStandard.ERC20, + }, + amount: new BigNumber('100'), + fiatAmount: null, + isApproval: true, +}; + +function render() { + const store = configureStore( + getMockConfirmStateForTransaction( + genUnapprovedContractInteractionConfirmation({ + simulationData: { + tokenBalanceChanges: [], + }, + }), + ), + ); + + return renderWithConfirmContextProvider( + + + , + store, + ); +} + +describe('BatchSimulationDetails', () => { + const useBalanceChangesMock = jest.mocked(useBalanceChanges); + + const useBatchApproveBalanceChangesMock = jest.mocked( + useBatchApproveBalanceChanges, + ); + + beforeEach(() => { + jest.resetAllMocks(); + + useBalanceChangesMock.mockReturnValue({ + pending: false, + value: [], + }); + + useBatchApproveBalanceChangesMock.mockReturnValue({ + pending: false, + value: [], + }); + }); + + it('renders approve row if approve balance changes', () => { + useBatchApproveBalanceChangesMock.mockReturnValue({ + pending: false, + value: [BALANCE_CHANGE_MOCK], + }); + + const { getByText } = render(); + expect(getByText('You approve')).toBeInTheDocument(); + }); + + it('does not render approve row if no approve balance changes', () => { + const { queryByText } = render(); + expect(queryByText('You approve')).toBeNull(); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx index de905aeba4fa..6a87e867a04c 100644 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx @@ -4,7 +4,7 @@ import { SimulationDetails, StaticRow, } from '../../../../simulation-details/simulation-details'; -import { useBatchApproveBalanceChanges } from '../../hooks/useBatchApproveTokenSimulation'; +import { useBatchApproveBalanceChanges } from '../../hooks/useBatchApproveBalanceChanges'; import { useConfirmContext } from '../../../../../context/confirm'; export function BatchSimulationDetails() { diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveTokenSimulation.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts similarity index 98% rename from ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveTokenSimulation.ts rename to ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts index 3911fac7cec7..37207026600f 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveTokenSimulation.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts @@ -15,7 +15,6 @@ import { getTokenStandardAndDetails } from '../../../../../../store/actions'; import { Hex, add0x } from '@metamask/utils'; import { fetchErc20Decimals } from '../../../../utils/token'; import { TokenStandard } from '../../../../../../../shared/constants/transaction'; -import { toHex } from '@metamask/controller-utils'; export function useBatchApproveBalanceChanges() { const locale = useSelector(getIntlLocale); diff --git a/ui/pages/confirmations/components/simulation-details/simulation-details.tsx b/ui/pages/confirmations/components/simulation-details/simulation-details.tsx index f7e513b7dc2c..04e92fe2423a 100644 --- a/ui/pages/confirmations/components/simulation-details/simulation-details.tsx +++ b/ui/pages/confirmations/components/simulation-details/simulation-details.tsx @@ -325,7 +325,7 @@ export const SimulationDetails: React.FC = ({ } const balanceChanges = balanceChangesResult.value; - const empty = balanceChanges.length === 0; + const empty = balanceChanges.length === 0 && staticRows.length === 0; if (empty) { return ( Date: Thu, 20 Feb 2025 16:20:12 +0000 Subject: [PATCH 05/19] Add batch simulation unit tests --- .../batch-simulation-details.test.tsx | 64 +++++++++++++++++-- 1 file changed, 58 insertions(+), 6 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx index e4ed9aa137c5..dc97416ab7f1 100644 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx @@ -1,5 +1,4 @@ import React from 'react'; -import { BatchTransactionParams } from '@metamask/transaction-controller'; import { renderWithConfirmContextProvider } from '../../../../../../../../test/lib/confirmations/render-helpers'; import configureStore from '../../../../../../../store/store'; import { getMockConfirmStateForTransaction } from '../../../../../../../../test/data/confirmations/helper'; @@ -20,13 +19,40 @@ jest.mock('../../hooks/useBatchApproveBalanceChanges', () => ({ useBatchApproveBalanceChanges: jest.fn(), })); -const BALANCE_CHANGE_MOCK: BalanceChange = { +const ADDRESS_MOCK = '0x1234567891234567891234567891234567891234'; +const ADDRESS_SHORT_MOCK = '0x12345...91234'; + +const BALANCE_CHANGE_ERC20_MOCK: BalanceChange = { asset: { - address: '0x1234567891234567891234567891234567891234', + address: ADDRESS_MOCK, chainId: '0x123', standard: TokenStandard.ERC20, }, - amount: new BigNumber('100'), + amount: new BigNumber(123.56), + fiatAmount: null, + isApproval: true, +}; + +const BALANCE_CHANGE_ERC721_MOCK: BalanceChange = { + asset: { + address: ADDRESS_MOCK, + chainId: '0x123', + standard: TokenStandard.ERC721, + tokenId: '0x141', + }, + amount: new BigNumber(1), + fiatAmount: null, + isApproval: true, +}; + +const BALANCE_CHANGE_ERC1155_MOCK: BalanceChange = { + asset: { + address: ADDRESS_MOCK, + chainId: '0x123', + standard: TokenStandard.ERC1155, + tokenId: '0x141', + }, + amount: new BigNumber(123), fiatAmount: null, isApproval: true, }; @@ -71,14 +97,40 @@ describe('BatchSimulationDetails', () => { }); }); - it('renders approve row if approve balance changes', () => { + it('renders ERC-20 approve row', () => { + useBatchApproveBalanceChangesMock.mockReturnValue({ + pending: false, + value: [BALANCE_CHANGE_ERC20_MOCK], + }); + + const { getByText } = render(); + expect(getByText('You approve')).toBeInTheDocument(); + expect(getByText('123.6')).toBeInTheDocument(); + expect(getByText(ADDRESS_SHORT_MOCK)).toBeInTheDocument(); + }); + + it('renders ERC-721 approve row', () => { + useBatchApproveBalanceChangesMock.mockReturnValue({ + pending: false, + value: [BALANCE_CHANGE_ERC721_MOCK], + }); + + const { getByText } = render(); + expect(getByText('You approve')).toBeInTheDocument(); + expect(getByText('#321')).toBeInTheDocument(); + expect(getByText(ADDRESS_SHORT_MOCK)).toBeInTheDocument(); + }); + + it('renders ERC-1155 approve row', () => { useBatchApproveBalanceChangesMock.mockReturnValue({ pending: false, - value: [BALANCE_CHANGE_MOCK], + value: [BALANCE_CHANGE_ERC1155_MOCK], }); const { getByText } = render(); expect(getByText('You approve')).toBeInTheDocument(); + expect(getByText('123 #321')).toBeInTheDocument(); + expect(getByText(ADDRESS_SHORT_MOCK)).toBeInTheDocument(); }); it('does not render approve row if no approve balance changes', () => { From 25c4ba2cd2636ea839228583e0531dce4cc30ae9 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 20 Feb 2025 20:27:52 +0000 Subject: [PATCH 06/19] Add batch simulation unit test --- .../batch-simulation-details.test.tsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx index dc97416ab7f1..abccc427a21a 100644 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx @@ -133,6 +133,18 @@ describe('BatchSimulationDetails', () => { expect(getByText(ADDRESS_SHORT_MOCK)).toBeInTheDocument(); }); + it('renders multiple approve rows', () => { + useBatchApproveBalanceChangesMock.mockReturnValue({ + pending: false, + value: [BALANCE_CHANGE_ERC20_MOCK, BALANCE_CHANGE_ERC721_MOCK], + }); + + const { getByText } = render(); + expect(getByText('You approve')).toBeInTheDocument(); + expect(getByText('123.6')).toBeInTheDocument(); + expect(getByText('#321')).toBeInTheDocument(); + }); + it('does not render approve row if no approve balance changes', () => { const { queryByText } = render(); expect(queryByText('You approve')).toBeNull(); From 1a2a160919f1f847efaf993dd302cb7cb5be205b Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 15:11:50 +0000 Subject: [PATCH 07/19] Use existing balance changes hook Remove batch spending cap. --- shared/modules/transaction.utils.ts | 31 +++++ test/data/confirmations/token-approve.ts | 10 ++ .../base-transaction-info.tsx | 2 - .../batch-spending-cap/batch-spending-cap.tsx | 40 ------ .../hooks/useBatchApproveBalanceChanges.ts | 126 ++++++++++-------- .../simulation-details/simulation-details.tsx | 14 +- 6 files changed, 120 insertions(+), 103 deletions(-) delete mode 100644 ui/pages/confirmations/components/confirm/info/batch/batch-spending-cap/batch-spending-cap.tsx diff --git a/shared/modules/transaction.utils.ts b/shared/modules/transaction.utils.ts index 728edbca87c2..9b68125f18fa 100644 --- a/shared/modules/transaction.utils.ts +++ b/shared/modules/transaction.utils.ts @@ -15,6 +15,7 @@ import type { TransactionParams } from '@metamask/transaction-controller'; import type { Provider } from '@metamask/network-controller'; import { Hex } from '@metamask/utils'; +import { BigNumber } from 'ethers'; import { AssetType, TokenStandard } from '../constants/transaction'; import { readAddressAsContract } from './contract-utils'; import { isEqualCaseInsensitive } from './string-utils'; @@ -326,3 +327,33 @@ export function hasTransactionData(transactionData?: Hex): boolean { transactionData?.length && transactionData?.toLowerCase?.() !== '0x', ); } + +export function parseApprovalTransactionData(data: Hex): + | { + amountOrTokenId?: BigNumber; + isApproveAll?: boolean; + isRevokeAll?: boolean; + } + | undefined { + const transactionDescription = parseStandardTokenTransactionData(data); + const { args, name } = transactionDescription ?? {}; + + if ( + !['approve', 'increaseAllowance', 'setApprovalForAll'].includes(name ?? '') + ) { + return undefined; + } + + const amountOrTokenId = + args?._value ?? // ERC-20 - approve + args?.increment; // Fiat Token V2 - increaseAllowance + + const isApproveAll = name === 'setApprovalForAll' && args?._approved === true; + const isRevokeAll = name === 'setApprovalForAll' && args?._approved === false; + + return { + amountOrTokenId, + isApproveAll, + isRevokeAll, + }; +} diff --git a/test/data/confirmations/token-approve.ts b/test/data/confirmations/token-approve.ts index 11fd52a7f945..a52459eaee01 100644 --- a/test/data/confirmations/token-approve.ts +++ b/test/data/confirmations/token-approve.ts @@ -1,11 +1,21 @@ import { TransactionType } from '@metamask/transaction-controller'; import { Hex } from '@metamask/utils'; +import { Interface } from '@ethersproject/abi'; import { CHAIN_ID, CONTRACT_INTERACTION_SENDER_ADDRESS, genUnapprovedContractInteractionConfirmation, } from './contract-interaction'; +export function buildApproveTransactionData( + address: string, + amountOrTokenId: number, +): Hex { + return new Interface([ + 'function approve(address spender, uint256 amountOrTokenId)', + ]).encodeFunctionData('approve', [address, amountOrTokenId]) as Hex; +} + export const genUnapprovedApproveConfirmation = ({ address = CONTRACT_INTERACTION_SENDER_ADDRESS, chainId = CHAIN_ID, diff --git a/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx b/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx index 51066fbff5e8..449579ba215f 100644 --- a/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx +++ b/ui/pages/confirmations/components/confirm/info/base-transaction-info/base-transaction-info.tsx @@ -5,7 +5,6 @@ import { AdvancedDetails } from '../shared/advanced-details/advanced-details'; import { GasFeesSection } from '../shared/gas-fees-section/gas-fees-section'; import { TransactionDetails } from '../shared/transaction-details/transaction-details'; import { BatchSimulationDetails } from '../batch/batch-simulation-details/batch-simulation-details'; -import { BatchSpendingCap } from '../batch/batch-spending-cap/batch-spending-cap'; const BaseTransactionInfo = () => { const { currentConfirmation: transactionMeta } = @@ -18,7 +17,6 @@ const BaseTransactionInfo = () => { return ( <> - diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-spending-cap/batch-spending-cap.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-spending-cap/batch-spending-cap.tsx deleted file mode 100644 index 27bb1eec9f0c..000000000000 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-spending-cap/batch-spending-cap.tsx +++ /dev/null @@ -1,40 +0,0 @@ -import React from 'react'; -import { - BatchTransactionParams, - TransactionMeta, -} from '@metamask/transaction-controller'; -import { useConfirmContext } from '../../../../../context/confirm'; -import { Box } from '../../../../../../../components/component-library'; -import { SpendingCap } from '../../approve/spending-cap/spending-cap'; - -export function BatchSpendingCap() { - const { currentConfirmation } = useConfirmContext(); - const { nestedTransactions } = currentConfirmation ?? {}; - - if (!nestedTransactions?.length) { - return null; - } - - return ( - - {nestedTransactions.map((nestedTransaction, index) => ( - - ))} - - ); -} - -function NestedTransactionSpendingCap({ - nestedTransaction, -}: { - nestedTransaction: BatchTransactionParams; -}) { - const { data, to } = nestedTransaction; - - return ( - {}} /> - ); -} diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts index 37207026600f..63a1c901dbfc 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts @@ -1,53 +1,67 @@ import { BatchTransactionParams, + SimulationTokenBalanceChange, + SimulationTokenStandard, TransactionMeta, } from '@metamask/transaction-controller'; import { useConfirmContext } from '../../../../context/confirm'; -import { decodeApproveTokenData } from '../approve/hooks/use-approve-token-simulation'; -import { getIntlLocale } from '../../../../../../ducks/locale/locale'; -import { useSelector } from 'react-redux'; -import { - BalanceChange, - TokenAssetIdentifier, -} from '../../../simulation-details/types'; import { useAsyncResult } from '../../../../../../hooks/useAsyncResult'; import { getTokenStandardAndDetails } from '../../../../../../store/actions'; -import { Hex, add0x } from '@metamask/utils'; -import { fetchErc20Decimals } from '../../../../utils/token'; -import { TokenStandard } from '../../../../../../../shared/constants/transaction'; +import { Hex } from '@metamask/utils'; +import { parseApprovalTransactionData } from '../../../../../../../shared/modules/transaction.utils'; +import { useBalanceChanges } from '../../../simulation-details/useBalanceChanges'; +import { BalanceChange } from '../../../simulation-details/types'; export function useBatchApproveBalanceChanges() { - const locale = useSelector(getIntlLocale); const { currentConfirmation } = useConfirmContext(); const { chainId, nestedTransactions } = currentConfirmation ?? {}; - if (!nestedTransactions?.length) { - return undefined; - } + const { value: simulationBalanceChanges, pending: pendingSimulationChanges } = + useBatchApproveSimulationBalanceChanges({ + nestedTransactions, + }); + + const { value: balanceChanges, pending: pendingBalanceChanges } = + useBalanceChanges({ + chainId, + simulationData: { + tokenBalanceChanges: simulationBalanceChanges ?? [], + }, + }); - const { pending, value } = useAsyncResult( - async () => - await buildBalanceChanges({ - chainId: chainId as Hex, - locale, - nestedTransactions, - }), - [chainId, locale, nestedTransactions], + const finalBalanceChanges = (balanceChanges ?? []).map( + (change) => ({ + ...change, + isApproval: true, + }), ); - return { pending, value }; + const pending = pendingSimulationChanges || pendingBalanceChanges; + + return { pending, value: finalBalanceChanges }; } -async function buildBalanceChanges({ - chainId, - locale, +function useBatchApproveSimulationBalanceChanges({ nestedTransactions, }: { - chainId: Hex; - locale: string; - nestedTransactions: BatchTransactionParams[]; -}): Promise { - const balanceChanges: BalanceChange[] = []; + nestedTransactions?: BatchTransactionParams[]; +}) { + return useAsyncResult( + async () => buildSimulationTokenBalanceChanges({ nestedTransactions }), + [nestedTransactions], + ); +} + +async function buildSimulationTokenBalanceChanges({ + nestedTransactions, +}: { + nestedTransactions?: BatchTransactionParams[]; +}): Promise { + const balanceChanges: SimulationTokenBalanceChange[] = []; + + if (!nestedTransactions) { + return balanceChanges; + } for (const transaction of nestedTransactions) { const { data, to } = transaction; @@ -57,39 +71,37 @@ async function buildBalanceChanges({ } const tokenData = await getTokenStandardAndDetails(to); - const tokenStandard = tokenData?.standard as TokenStandard; - let decimals = undefined; - if (tokenStandard === TokenStandard.ERC20) { - decimals = await fetchErc20Decimals(to); + if (!tokenData?.standard) { + continue; } - const approveAmountOrId = await decodeApproveTokenData({ - data, - decimals, - locale, - to, - }); + const standard = + tokenData?.standard?.toLowerCase() as SimulationTokenStandard; + const isNFT = standard !== SimulationTokenStandard.erc20; + + const parseResult = parseApprovalTransactionData(data); - if (approveAmountOrId == undefined) { + if (!parseResult) { continue; } - const tokenId = - tokenStandard === TokenStandard.ERC721 - ? add0x(approveAmountOrId.toString(16)) - : undefined; - - const balanceChange: BalanceChange = { - asset: { - address: to, - chainId, - standard: tokenStandard as TokenAssetIdentifier['standard'], - tokenId, - }, - amount: approveAmountOrId, - fiatAmount: null, - isApproval: true, + const { amountOrTokenId, isApproveAll, isRevokeAll } = parseResult; + const amountOrTokenIdHex = amountOrTokenId?.toHexString() as Hex; + + const difference = + isNFT || amountOrTokenId === undefined ? '0x1' : amountOrTokenIdHex; + + const tokenId = isNFT && amountOrTokenId ? amountOrTokenIdHex : undefined; + + const balanceChange: SimulationTokenBalanceChange = { + address: to, + difference, + id: tokenId, + isDecrease: true, + newBalance: '0x0', + previousBalance: '0x0', + standard, }; balanceChanges.push(balanceChange); diff --git a/ui/pages/confirmations/components/simulation-details/simulation-details.tsx b/ui/pages/confirmations/components/simulation-details/simulation-details.tsx index 04e92fe2423a..b29b27cc034d 100644 --- a/ui/pages/confirmations/components/simulation-details/simulation-details.tsx +++ b/ui/pages/confirmations/components/simulation-details/simulation-details.tsx @@ -273,6 +273,10 @@ export const SimulationDetails: React.FC = ({ const balanceChangesResult = useBalanceChanges({ chainId, simulationData }); const loading = !simulationData || balanceChangesResult.pending; + const hasStaticData = + staticRows?.length > 0 && + staticRows.some((row) => row.balanceChanges?.length > 0); + useSimulationMetrics({ enableMetrics, balanceChanges: balanceChangesResult.value, @@ -301,12 +305,13 @@ export const SimulationDetails: React.FC = ({ [ SimulationErrorCode.ChainNotSupported, SimulationErrorCode.Disabled, - ].includes(error?.code as SimulationErrorCode) + ].includes(error?.code as SimulationErrorCode) && + !hasStaticData ) { return null; } - if (error) { + if (error && !hasStaticData) { const inHeaderProp = error.code !== SimulationErrorCode.Reverted && { inHeader: , }; @@ -325,7 +330,7 @@ export const SimulationDetails: React.FC = ({ } const balanceChanges = balanceChangesResult.value; - const empty = balanceChanges.length === 0 && staticRows.length === 0; + const empty = balanceChanges.length === 0 && !hasStaticData; if (empty) { return ( = ({ balanceChanges={incoming} testId="simulation-rows-incoming" /> - {staticRows.map((staticRow) => ( + {staticRows.map((staticRow, index) => ( From 186ceb066e62780fa4c4079cdce05d59631ef7c1 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 17:00:47 +0000 Subject: [PATCH 08/19] Add unit tests for parseApprovalTransactionData function --- shared/modules/transaction.utils.test.js | 65 +++++++++++++++++++ shared/modules/transaction.utils.ts | 8 ++- .../confirmations/set-approval-for-all.ts | 10 +++ test/data/confirmations/token-approve.ts | 9 +++ .../hooks/useBatchApproveBalanceChanges.ts | 2 +- 5 files changed, 91 insertions(+), 3 deletions(-) diff --git a/shared/modules/transaction.utils.test.js b/shared/modules/transaction.utils.test.js index c501491fce6f..74100dccc107 100644 --- a/shared/modules/transaction.utils.test.js +++ b/shared/modules/transaction.utils.test.js @@ -1,15 +1,26 @@ import { TransactionType } from '@metamask/transaction-controller'; +import BigNumber from 'bignumber.js'; import { createTestProviderTools } from '../../test/stub/provider'; +import { + buildApproveTransactionData, + buildIncreaseAllowanceTransactionData, +} from '../../test/data/confirmations/token-approve'; +import { buildSetApproveForAllTransactionData } from '../../test/data/confirmations/set-approval-for-all'; import { determineTransactionType, hasTransactionData, isEIP1559Transaction, isLegacyTransaction, + parseApprovalTransactionData, parseStandardTokenTransactionData, parseTypedDataMessage, } from './transaction.utils'; +const DATA_MOCK = '0x12345678'; +const ADDRESS_MOCK = '0x1234567890123456789012345678901234567890'; +const AMOUNT_MOCK = 123; + describe('Transaction.utils', function () { describe('parseStandardTokenTransactionData', () => { it('should return token data', () => { @@ -432,4 +443,58 @@ describe('Transaction.utils', function () { }, ); }); + + describe('parseApprovalTransactionData', () => { + it('returns undefined if function does not match', () => { + expect(parseApprovalTransactionData(DATA_MOCK)).toBeUndefined(); + }); + + it('returns parsed data if approve', () => { + expect( + parseApprovalTransactionData( + buildApproveTransactionData(ADDRESS_MOCK, AMOUNT_MOCK), + ), + ).toStrictEqual({ + amountOrTokenId: new BigNumber(AMOUNT_MOCK), + isApproveAll: false, + isRevokeAll: false, + }); + }); + + it('returns parsed data if increaseAllowance', () => { + expect( + parseApprovalTransactionData( + buildIncreaseAllowanceTransactionData(ADDRESS_MOCK, AMOUNT_MOCK), + ), + ).toStrictEqual({ + amountOrTokenId: new BigNumber(AMOUNT_MOCK), + isApproveAll: false, + isRevokeAll: false, + }); + }); + + it('returns parsed data if approved setApproveForAll', () => { + expect( + parseApprovalTransactionData( + buildSetApproveForAllTransactionData(ADDRESS_MOCK, true), + ), + ).toStrictEqual({ + amountOrTokenId: undefined, + isApproveAll: true, + isRevokeAll: false, + }); + }); + + it('returns parsed data if revoked setApproveForAll', () => { + expect( + parseApprovalTransactionData( + buildSetApproveForAllTransactionData(ADDRESS_MOCK, false), + ), + ).toStrictEqual({ + amountOrTokenId: undefined, + isApproveAll: false, + isRevokeAll: true, + }); + }); + }); }); diff --git a/shared/modules/transaction.utils.ts b/shared/modules/transaction.utils.ts index 9b68125f18fa..f3f418ebc814 100644 --- a/shared/modules/transaction.utils.ts +++ b/shared/modules/transaction.utils.ts @@ -15,7 +15,7 @@ import type { TransactionParams } from '@metamask/transaction-controller'; import type { Provider } from '@metamask/network-controller'; import { Hex } from '@metamask/utils'; -import { BigNumber } from 'ethers'; +import { BigNumber } from 'bignumber.js'; import { AssetType, TokenStandard } from '../constants/transaction'; import { readAddressAsContract } from './contract-utils'; import { isEqualCaseInsensitive } from './string-utils'; @@ -344,10 +344,14 @@ export function parseApprovalTransactionData(data: Hex): return undefined; } - const amountOrTokenId = + const rawAmountOrTokenId = args?._value ?? // ERC-20 - approve args?.increment; // Fiat Token V2 - increaseAllowance + const amountOrTokenId = rawAmountOrTokenId + ? new BigNumber(rawAmountOrTokenId?.toString()) + : undefined; + const isApproveAll = name === 'setApprovalForAll' && args?._approved === true; const isRevokeAll = name === 'setApprovalForAll' && args?._approved === false; diff --git a/test/data/confirmations/set-approval-for-all.ts b/test/data/confirmations/set-approval-for-all.ts index ff39988fe238..ff57ff851508 100644 --- a/test/data/confirmations/set-approval-for-all.ts +++ b/test/data/confirmations/set-approval-for-all.ts @@ -1,5 +1,6 @@ import { TransactionType } from '@metamask/transaction-controller'; import { Hex } from '@metamask/utils'; +import { Interface } from '@ethersproject/abi'; import { CHAIN_ID, CONTRACT_INTERACTION_SENDER_ADDRESS, @@ -9,6 +10,15 @@ import { export const INCREASE_ALLOWANCE_TRANSACTION_DATA = '0x395093510000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000123'; +export function buildSetApproveForAllTransactionData( + address: string, + approved: boolean, +): Hex { + return new Interface([ + 'function setApprovalForAll(address operator, bool approved)', + ]).encodeFunctionData('setApprovalForAll', [address, approved]) as Hex; +} + export const genUnapprovedSetApprovalForAllConfirmation = ({ address = CONTRACT_INTERACTION_SENDER_ADDRESS, chainId = CHAIN_ID, diff --git a/test/data/confirmations/token-approve.ts b/test/data/confirmations/token-approve.ts index a52459eaee01..9129478a3d87 100644 --- a/test/data/confirmations/token-approve.ts +++ b/test/data/confirmations/token-approve.ts @@ -16,6 +16,15 @@ export function buildApproveTransactionData( ]).encodeFunctionData('approve', [address, amountOrTokenId]) as Hex; } +export function buildIncreaseAllowanceTransactionData( + address: string, + amount: number, +): Hex { + return new Interface([ + 'function increaseAllowance(address spender, uint256 addedValue)', + ]).encodeFunctionData('increaseAllowance', [address, amount]) as Hex; +} + export const genUnapprovedApproveConfirmation = ({ address = CONTRACT_INTERACTION_SENDER_ADDRESS, chainId = CHAIN_ID, diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts index 63a1c901dbfc..88596ad4d41c 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts @@ -87,7 +87,7 @@ async function buildSimulationTokenBalanceChanges({ } const { amountOrTokenId, isApproveAll, isRevokeAll } = parseResult; - const amountOrTokenIdHex = amountOrTokenId?.toHexString() as Hex; + const amountOrTokenIdHex = amountOrTokenId?.toString(16) as Hex; const difference = isNFT || amountOrTokenId === undefined ? '0x1' : amountOrTokenIdHex; From 193884957e2abc7b6ba3b49f86fd3ecbd3c646e9 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 17:46:41 +0000 Subject: [PATCH 09/19] Add unit tests for useBatchApproveBalanceChanges --- .../useBatchApproveBalanceChanges.test.ts | 244 ++++++++++++++++++ .../hooks/useBatchApproveBalanceChanges.ts | 5 +- 2 files changed, 247 insertions(+), 2 deletions(-) create mode 100644 ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts new file mode 100644 index 000000000000..d9edfa780fe8 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts @@ -0,0 +1,244 @@ +import { act } from '@testing-library/react'; +import { renderHookWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; +import { useBatchApproveBalanceChanges } from './useBatchApproveBalanceChanges'; +import { + BatchTransactionParams, + SimulationTokenStandard, +} from '@metamask/transaction-controller'; +import { getMockConfirmStateForTransaction } from '../../../../../../../test/data/confirmations/helper'; +import { + CHAIN_ID, + genUnapprovedContractInteractionConfirmation, +} from '../../../../../../../test/data/confirmations/contract-interaction'; +import { useBalanceChanges } from '../../../simulation-details/useBalanceChanges'; +import { getTokenStandardAndDetails } from '../../../../../../store/actions'; +import { TokenStandard } from '../../../../../../../shared/constants/transaction'; +import BigNumber from 'bignumber.js'; +import { BalanceChange } from '../../../simulation-details/types'; +import { buildApproveTransactionData } from '../../../../../../../test/data/confirmations/token-approve'; + +jest.mock('../../../simulation-details/useBalanceChanges', () => ({ + useBalanceChanges: jest.fn(), +})); + +jest.mock('../../../../../../store/actions', () => ({ + ...jest.requireActual('../../../../../../store/actions'), + getTokenStandardAndDetails: jest.fn(), +})); + +const TO_MOCK = '0x1234567891234567891234567891234567891234'; +const AMOUNT_MOCK = 123; +const AMOUNT_HEX_MOCK = '0x7b'; +const DATA_MOCK = buildApproveTransactionData(TO_MOCK, AMOUNT_MOCK); + +const BALANCE_CHANGE_MOCK: BalanceChange = { + asset: { + address: '0x1234567891234567891234567891234567891234', + chainId: '0x123', + standard: TokenStandard.ERC20, + }, + amount: new BigNumber('123.56'), + fiatAmount: 12.34, + isApproval: false, +}; + +async function runHook({ + nestedTransactions, +}: { + nestedTransactions?: BatchTransactionParams[]; +}) { + const response = renderHookWithConfirmContextProvider( + useBatchApproveBalanceChanges, + getMockConfirmStateForTransaction( + genUnapprovedContractInteractionConfirmation({ + nestedTransactions, + }), + ), + ); + + await act(async () => { + // Ignore + }); + + return response.result.current; +} + +describe('useBatchApproveBalanceChanges', () => { + const useBalanceChangesMock = jest.mocked(useBalanceChanges); + const getTokenStandardAndDetailsMock = jest.mocked( + getTokenStandardAndDetails, + ); + + beforeEach(() => { + jest.resetAllMocks(); + + useBalanceChangesMock.mockReturnValue({ value: [], pending: false }); + getTokenStandardAndDetailsMock.mockResolvedValue({ + standard: TokenStandard.ERC20, + }); + }); + + it('returns balance changes with isApproval set to true', async () => { + useBalanceChangesMock.mockReturnValue({ + value: [BALANCE_CHANGE_MOCK], + pending: false, + }); + + const result = await runHook({}); + + expect(result).toStrictEqual({ + pending: false, + value: [{ ...BALANCE_CHANGE_MOCK, isApproval: true }], + }); + }); + + it('generates ERC-20 token balance changes', async () => { + await runHook({ + nestedTransactions: [ + { + data: DATA_MOCK, + to: TO_MOCK, + }, + ], + }); + + expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ + chainId: CHAIN_ID, + simulationData: { + tokenBalanceChanges: [ + { + address: TO_MOCK, + difference: AMOUNT_HEX_MOCK, + id: undefined, + isDecrease: true, + newBalance: '0x0', + previousBalance: '0x0', + standard: SimulationTokenStandard.erc20, + }, + ], + }, + }); + }); + + it('generates ERC-721 token balance changes', async () => { + getTokenStandardAndDetailsMock.mockResolvedValue({ + standard: TokenStandard.ERC721, + }); + + await runHook({ + nestedTransactions: [ + { + data: DATA_MOCK, + to: TO_MOCK, + }, + ], + }); + + expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ + chainId: CHAIN_ID, + simulationData: { + tokenBalanceChanges: [ + { + address: TO_MOCK, + difference: '0x1', + id: AMOUNT_HEX_MOCK, + isDecrease: true, + newBalance: '0x0', + previousBalance: '0x0', + standard: SimulationTokenStandard.erc721, + }, + ], + }, + }); + }); + + it('generates no token balance changes if no nested transactions', async () => { + await runHook({}); + + expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ + chainId: CHAIN_ID, + simulationData: { + tokenBalanceChanges: [], + }, + }); + }); + + it('generates no token balance changes if no standard', async () => { + getTokenStandardAndDetailsMock.mockResolvedValue({ + standard: undefined as never, + }); + + await runHook({ + nestedTransactions: [ + { + data: DATA_MOCK, + to: TO_MOCK, + }, + ], + }); + + expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ + chainId: CHAIN_ID, + simulationData: { + tokenBalanceChanges: [], + }, + }); + }); + + it('generates no token balance changes if cannot be parsed', async () => { + await runHook({ + nestedTransactions: [ + { + data: '0x12345678', + to: TO_MOCK, + }, + ], + }); + + expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ + chainId: CHAIN_ID, + simulationData: { + tokenBalanceChanges: [], + }, + }); + }); + + it('generates no token balance changes if no to in nested transaction', async () => { + await runHook({ + nestedTransactions: [ + { + data: DATA_MOCK, + }, + ], + }); + + expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ + chainId: CHAIN_ID, + simulationData: { + tokenBalanceChanges: [], + }, + }); + }); + + it('generates no token balance changes if no data in nested transaction', async () => { + await runHook({ + nestedTransactions: [ + { + to: TO_MOCK, + }, + ], + }); + + expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ + chainId: CHAIN_ID, + simulationData: { + tokenBalanceChanges: [], + }, + }); + }); + + it('returns undefined if no balance changes transactions', async () => { + const result = await runHook({}); + expect(result).toStrictEqual({ pending: false, value: [] }); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts index 88596ad4d41c..b342e660b27b 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts @@ -7,7 +7,7 @@ import { import { useConfirmContext } from '../../../../context/confirm'; import { useAsyncResult } from '../../../../../../hooks/useAsyncResult'; import { getTokenStandardAndDetails } from '../../../../../../store/actions'; -import { Hex } from '@metamask/utils'; +import { Hex, add0x } from '@metamask/utils'; import { parseApprovalTransactionData } from '../../../../../../../shared/modules/transaction.utils'; import { useBalanceChanges } from '../../../simulation-details/useBalanceChanges'; import { BalanceChange } from '../../../simulation-details/types'; @@ -78,6 +78,7 @@ async function buildSimulationTokenBalanceChanges({ const standard = tokenData?.standard?.toLowerCase() as SimulationTokenStandard; + const isNFT = standard !== SimulationTokenStandard.erc20; const parseResult = parseApprovalTransactionData(data); @@ -87,7 +88,7 @@ async function buildSimulationTokenBalanceChanges({ } const { amountOrTokenId, isApproveAll, isRevokeAll } = parseResult; - const amountOrTokenIdHex = amountOrTokenId?.toString(16) as Hex; + const amountOrTokenIdHex = add0x(amountOrTokenId?.toString(16) ?? '0x0'); const difference = isNFT || amountOrTokenId === undefined ? '0x1' : amountOrTokenIdHex; From 3543180ad9ffc814e7c49a671d5ae85658d84b97 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 20:33:36 +0000 Subject: [PATCH 10/19] Support unlimited and approve all --- .../batch-simulation-details.stories.tsx | 11 ++++++ .../hooks/useBatchApproveBalanceChanges.ts | 35 ++++++++++++++----- .../simulation-details/amount-pill.tsx | 16 +++++++-- .../simulation-details/balance-change-row.tsx | 18 ++++++++-- .../components/simulation-details/types.ts | 6 ++++ 5 files changed, 73 insertions(+), 13 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.stories.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.stories.tsx index 6926f035eb73..dd15ab13a5cf 100644 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.stories.tsx +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.stories.tsx @@ -15,6 +15,9 @@ import { } from '@metamask/transaction-controller'; import { NameType } from '@metamask/name-controller'; import { Hex } from '@metamask/utils'; +import { buildApproveTransactionData } from '../../../../../../../../test/data/confirmations/token-approve'; +import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../../shared/constants'; +import { buildSetApproveForAllTransactionData } from '../../../../../../../../test/data/confirmations/set-approval-for-all'; const ERC20_TOKEN_1_MOCK = '0x2260fac5e5542a773aa44fbcfedf7c193bc2c599'; // WBTC const ERC20_TOKEN_2_MOCK = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; // USDC @@ -35,6 +38,14 @@ const TRANSACTION_MOCK = genUnapprovedContractInteractionConfirmation({ { data: '0x095ea7b30000000000000000000000009bc5baf874d2da8d216ae9f137804184ee5afef40000000000000000000000000000000000000000000000000000000000000721', to: ERC721_TOKEN_MOCK, + }, + { + data: buildApproveTransactionData(ERC20_TOKEN_2_MOCK, TOKEN_VALUE_UNLIMITED_THRESHOLD), + to: ERC20_TOKEN_2_MOCK, + }, + { + data: buildSetApproveForAllTransactionData(ERC721_TOKEN_MOCK, true), + to: ERC721_TOKEN_MOCK, } ], simulationData: { diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts index b342e660b27b..1f385f5a4611 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts @@ -7,10 +7,16 @@ import { import { useConfirmContext } from '../../../../context/confirm'; import { useAsyncResult } from '../../../../../../hooks/useAsyncResult'; import { getTokenStandardAndDetails } from '../../../../../../store/actions'; -import { Hex, add0x } from '@metamask/utils'; +import { add0x } from '@metamask/utils'; import { parseApprovalTransactionData } from '../../../../../../../shared/modules/transaction.utils'; import { useBalanceChanges } from '../../../simulation-details/useBalanceChanges'; import { BalanceChange } from '../../../simulation-details/types'; +import { isSpendingCapUnlimited } from '../approve/hooks/use-approve-token-simulation'; + +type ApprovalBalanceChange = SimulationTokenBalanceChange & { + isAll: boolean; + isUnlimited: boolean; +}; export function useBatchApproveBalanceChanges() { const { currentConfirmation } = useConfirmContext(); @@ -30,10 +36,16 @@ export function useBatchApproveBalanceChanges() { }); const finalBalanceChanges = (balanceChanges ?? []).map( - (change) => ({ - ...change, - isApproval: true, - }), + (change, index) => { + const simulation = simulationBalanceChanges?.[index]; + + return { + ...change, + isApproval: true, + isAllApproval: simulation?.isAll ?? false, + isUnlimitedApproval: simulation?.isUnlimited ?? false, + }; + }, ); const pending = pendingSimulationChanges || pendingBalanceChanges; @@ -56,8 +68,8 @@ async function buildSimulationTokenBalanceChanges({ nestedTransactions, }: { nestedTransactions?: BatchTransactionParams[]; -}): Promise { - const balanceChanges: SimulationTokenBalanceChange[] = []; +}): Promise { + const balanceChanges: ApprovalBalanceChange[] = []; if (!nestedTransactions) { return balanceChanges; @@ -87,7 +99,7 @@ async function buildSimulationTokenBalanceChanges({ continue; } - const { amountOrTokenId, isApproveAll, isRevokeAll } = parseResult; + const { amountOrTokenId, isApproveAll: isAll } = parseResult; const amountOrTokenIdHex = add0x(amountOrTokenId?.toString(16) ?? '0x0'); const difference = @@ -95,11 +107,16 @@ async function buildSimulationTokenBalanceChanges({ const tokenId = isNFT && amountOrTokenId ? amountOrTokenIdHex : undefined; - const balanceChange: SimulationTokenBalanceChange = { + const isUnlimited = + !isNFT && isSpendingCapUnlimited(amountOrTokenId?.toNumber() ?? 0); + + const balanceChange: ApprovalBalanceChange = { address: to, difference, id: tokenId, + isAll: isAll ?? false, isDecrease: true, + isUnlimited, newBalance: '0x0', previousBalance: '0x0', standard, diff --git a/ui/pages/confirmations/components/simulation-details/amount-pill.tsx b/ui/pages/confirmations/components/simulation-details/amount-pill.tsx index dbac49ba6407..d1c6925aa85e 100644 --- a/ui/pages/confirmations/components/simulation-details/amount-pill.tsx +++ b/ui/pages/confirmations/components/simulation-details/amount-pill.tsx @@ -16,6 +16,7 @@ import { TokenStandard } from '../../../../../shared/constants/transaction'; import Tooltip from '../../../../components/ui/tooltip'; import { getIntlLocale } from '../../../../ducks/locale/locale'; import { shortenString as shortenAssetId } from '../../../../helpers/utils/util'; +import { useI18nContext } from '../../../../hooks/useI18nContext'; import { AssetIdentifier } from './types'; import { formatAmount, formatAmountMaxPrecision } from './formatAmount'; @@ -27,12 +28,17 @@ import { formatAmount, formatAmountMaxPrecision } from './formatAmount'; * @param props.asset * @param props.amount * @param props.isApproval + * @param props.isAllApproval + * @param props.isUnlimitedApproval */ export const AmountPill: React.FC<{ asset: AssetIdentifier; amount: BigNumber; isApproval?: boolean; -}> = ({ asset, amount, isApproval }) => { + isAllApproval?: boolean; + isUnlimitedApproval?: boolean; +}> = ({ asset, amount, isApproval, isAllApproval, isUnlimitedApproval }) => { + const t = useI18nContext(); const locale = useSelector(getIntlLocale); const backgroundColor = getBackgroundColour({ amount, isApproval }); @@ -47,7 +53,10 @@ export const AmountPill: React.FC<{ // ERC721 amounts are always 1 and are not displayed. if (asset.standard !== TokenStandard.ERC721) { - const formattedAmount = formatAmount(locale, amount.abs()); + const formattedAmount = isUnlimitedApproval + ? t('unlimited') + : formatAmount(locale, amount.abs()); + const fullPrecisionAmount = formatAmountMaxPrecision(locale, amount.abs()); amountParts.push(formattedAmount); @@ -68,6 +77,9 @@ export const AmountPill: React.FC<{ amountParts.push(shortenedTokenIdPart); tooltipParts.push(tooltipIdPart); + } else if (isAllApproval) { + amountParts.push(t('all')); + tooltipParts.push(t('all')); } return ( diff --git a/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx b/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx index a6cd2d372bba..19f6256c8372 100644 --- a/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx +++ b/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx @@ -25,7 +25,15 @@ export const BalanceChangeRow: React.FC<{ showFiat?: boolean; balanceChange: BalanceChange; }> = ({ label, showFiat, balanceChange }) => { - const { asset, amount, fiatAmount, isApproval } = balanceChange; + const { + asset, + amount, + fiatAmount, + isApproval, + isAllApproval, + isUnlimitedApproval, + } = balanceChange; + return ( - + {showFiat && } diff --git a/ui/pages/confirmations/components/simulation-details/types.ts b/ui/pages/confirmations/components/simulation-details/types.ts index 577c161ef013..b7c76e8f0f4b 100644 --- a/ui/pages/confirmations/components/simulation-details/types.ts +++ b/ui/pages/confirmations/components/simulation-details/types.ts @@ -61,4 +61,10 @@ export type BalanceChange = Readonly<{ /** Whether the balance change is a token approval. */ isApproval?: boolean; + + /** Whether the balance change is an approval for all tokens. */ + isAllApproval?: boolean; + + /** Whether the balance change is an unlimited token approval. */ + isUnlimitedApproval?: boolean; }>; From febbc38cd1e488fb8f7ea43f64ed68b24c67ef01 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 21:14:19 +0000 Subject: [PATCH 11/19] Remove unnecessary changes --- .../use-approve-token-simulation.test.ts | 15 +---- .../hooks/use-approve-token-simulation.ts | 55 ++++--------------- .../approve/spending-cap/spending-cap.tsx | 29 ++-------- 3 files changed, 19 insertions(+), 80 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.test.ts b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.test.ts index 26ca44587ce4..a4b867f82018 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.test.ts +++ b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.test.ts @@ -36,10 +36,7 @@ describe('useApproveTokenSimulation', () => { "isUnlimitedSpendingCap": false, "pending": undefined, "spendingCap": "#7", - "value": { - "hex": "0x011170", - "type": "BigNumber", - }, + "value": "70000", } `); }); @@ -65,10 +62,7 @@ describe('useApproveTokenSimulation', () => { "isUnlimitedSpendingCap": true, "pending": undefined, "spendingCap": "1000000000000000", - "value": { - "hex": "0x038d7ea4c68000", - "type": "BigNumber", - }, + "value": "1000000000000000", } `); }); @@ -92,10 +86,7 @@ describe('useApproveTokenSimulation', () => { "isUnlimitedSpendingCap": false, "pending": undefined, "spendingCap": "0.000000000000000001", - "value": { - "hex": "0x01", - "type": "BigNumber", - }, + "value": "1", } `); }); diff --git a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts index 1d2ec37b2ed6..b9744e8a339b 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts +++ b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts @@ -7,42 +7,9 @@ import { calcTokenAmount } from '../../../../../../../../shared/lib/transactions import { getIntlLocale } from '../../../../../../../ducks/locale/locale'; import { formatAmount } from '../../../../simulation-details/formatAmount'; import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../../shared/constants'; -import { parseStandardTokenTransactionData } from '../../../../../../../../shared/modules/transaction.utils'; +import { parseApprovalTransactionData } from '../../../../../../../../shared/modules/transaction.utils'; import { useIsNFT } from './use-is-nft'; -export function decodeApproveTokenData({ - data, - decimals, - to, -}: { - data?: Hex; - decimals?: number; - locale: string; - to?: Hex; -}) { - if (!data || !to) { - return undefined; - } - - const transactionDescription = parseStandardTokenTransactionData(data); - - if (!transactionDescription) { - return undefined; - } - - const parsedArgs = transactionDescription.args; - - const value = - parsedArgs?._value ?? // ERC-20 - approve - parsedArgs?.increment; // Fiat Token V2 - increaseAllowance - - if (!value) { - return undefined; - } - - return calcTokenAmount(value, decimals ?? 0); -} - export function isSpendingCapUnlimited(decodedSpendingCap: number) { return decodedSpendingCap >= TOKEN_VALUE_UNLIMITED_THRESHOLD; } @@ -53,19 +20,17 @@ export const useApproveTokenSimulation = ( ) => { const locale = useSelector(getIntlLocale); const { isNFT, pending: isNFTPending } = useIsNFT(transactionMeta); - const data = transactionMeta.txParams.data as Hex | undefined; - const to = transactionMeta.txParams.to as Hex | undefined; - const decimalsNumber = decimals ? Number(decimals) : 0; + const transactionData = transactionMeta?.txParams?.data as Hex | undefined; - const value = - decodeApproveTokenData({ - data, - decimals: decimalsNumber, - locale, - to, - }) ?? new BigNumber(0); + const { amountOrTokenId: parsedValue } = + parseApprovalTransactionData(transactionData ?? '0x') ?? {}; - const decodedSpendingCap = value.toFixed(); + const value = parsedValue ?? new BigNumber(0); + + const decodedSpendingCap = calcTokenAmount( + value, + Number(decimals ?? '0'), + ).toFixed(); const tokenPrefix = isNFT ? '#' : ''; diff --git a/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx b/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx index 29277ae31fb6..3a8c6a5a26ec 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx +++ b/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx @@ -1,6 +1,5 @@ import { TransactionMeta } from '@metamask/transaction-controller'; import React from 'react'; -import { Hex } from '@metamask/utils'; import { calcTokenAmount } from '../../../../../../../../shared/lib/transactions-controller-utils'; import { ConfirmInfoRow, @@ -19,15 +18,16 @@ const SpendingCapGroup = ({ tokenSymbol, decimals, setIsOpenEditSpendingCapModal, - transactionMeta, }: { tokenSymbol: string; decimals: string; setIsOpenEditSpendingCapModal: (newValue: boolean) => void; - transactionMeta: TransactionMeta; }) => { const t = useI18nContext(); + const { currentConfirmation: transactionMeta } = + useConfirmContext(); + const { spendingCap, isUnlimitedSpendingCap, formattedSpendingCap, value } = useApproveTokenSimulation(transactionMeta, decimals); @@ -69,26 +69,19 @@ const SpendingCapGroup = ({ }; export const SpendingCap = ({ - data, setIsOpenEditSpendingCapModal, - to, }: { - data?: Hex; setIsOpenEditSpendingCapModal: (newValue: boolean) => void; - to?: Hex; }) => { const t = useI18nContext(); const { currentConfirmation: transactionMeta } = useConfirmContext(); - const transactionTo = to ?? transactionMeta.txParams.to; - const transactionData = data ?? transactionMeta.txParams.data; - const { userBalance, tokenSymbol, decimals } = useAssetDetails( - transactionTo, + transactionMeta.txParams.to, transactionMeta.txParams.from, - transactionData, + transactionMeta.txParams.data, transactionMeta.chainId, ); @@ -97,16 +90,7 @@ export const SpendingCap = ({ Number(decimals ?? '0'), ).toFixed(); - const finalTransactionMeta = { - ...transactionMeta, - txParams: { - ...transactionMeta.txParams, - to: transactionTo, - data: transactionData, - }, - }; - - const { pending } = useApproveTokenSimulation(finalTransactionMeta, decimals); + const { pending } = useApproveTokenSimulation(transactionMeta, decimals); if (pending) { return ; @@ -122,7 +106,6 @@ export const SpendingCap = ({ tokenSymbol={tokenSymbol || ''} decimals={decimals || '0'} setIsOpenEditSpendingCapModal={setIsOpenEditSpendingCapModal} - transactionMeta={finalTransactionMeta} /> ); From 63048705812d59a099919010bd9e15048826735c Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 21:38:07 +0000 Subject: [PATCH 12/19] Add unit tests for AmountPill --- .../batch-simulation-details.test.tsx | 4 +- .../useBatchApproveBalanceChanges.test.ts | 6 +- .../hooks/useBatchApproveBalanceChanges.ts | 2 +- .../simulation-details/amount-pill.test.tsx | 83 ++++++++++++++++++- .../simulation-details/amount-pill.tsx | 6 +- 5 files changed, 92 insertions(+), 9 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx index abccc427a21a..a8defc5a4e88 100644 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx @@ -1,15 +1,15 @@ import React from 'react'; +import { BigNumber } from 'bignumber.js'; import { renderWithConfirmContextProvider } from '../../../../../../../../test/lib/confirmations/render-helpers'; import configureStore from '../../../../../../../store/store'; import { getMockConfirmStateForTransaction } from '../../../../../../../../test/data/confirmations/helper'; import { genUnapprovedContractInteractionConfirmation } from '../../../../../../../../test/data/confirmations/contract-interaction'; -import { BatchSimulationDetails } from './batch-simulation-details'; import { useBatchApproveBalanceChanges } from '../../hooks/useBatchApproveBalanceChanges'; import { AlertMetricsProvider } from '../../../../../../../components/app/alert-system/contexts/alertMetricsContext'; import { useBalanceChanges } from '../../../../simulation-details/useBalanceChanges'; -import BigNumber from 'bignumber.js'; import { BalanceChange } from '../../../../simulation-details/types'; import { TokenStandard } from '../../../../../../../../shared/constants/transaction'; +import { BatchSimulationDetails } from './batch-simulation-details'; jest.mock('../../../../simulation-details/useBalanceChanges', () => ({ useBalanceChanges: jest.fn(), diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts index d9edfa780fe8..6e99e7c72008 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts @@ -1,10 +1,9 @@ import { act } from '@testing-library/react'; -import { renderHookWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; -import { useBatchApproveBalanceChanges } from './useBatchApproveBalanceChanges'; import { BatchTransactionParams, SimulationTokenStandard, } from '@metamask/transaction-controller'; +import { BigNumber } from 'bignumber.js'; import { getMockConfirmStateForTransaction } from '../../../../../../../test/data/confirmations/helper'; import { CHAIN_ID, @@ -13,9 +12,10 @@ import { import { useBalanceChanges } from '../../../simulation-details/useBalanceChanges'; import { getTokenStandardAndDetails } from '../../../../../../store/actions'; import { TokenStandard } from '../../../../../../../shared/constants/transaction'; -import BigNumber from 'bignumber.js'; +import { renderHookWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; import { BalanceChange } from '../../../simulation-details/types'; import { buildApproveTransactionData } from '../../../../../../../test/data/confirmations/token-approve'; +import { useBatchApproveBalanceChanges } from './useBatchApproveBalanceChanges'; jest.mock('../../../simulation-details/useBalanceChanges', () => ({ useBalanceChanges: jest.fn(), diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts index 1f385f5a4611..db591dff4cc4 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts @@ -4,10 +4,10 @@ import { SimulationTokenStandard, TransactionMeta, } from '@metamask/transaction-controller'; +import { add0x } from '@metamask/utils'; import { useConfirmContext } from '../../../../context/confirm'; import { useAsyncResult } from '../../../../../../hooks/useAsyncResult'; import { getTokenStandardAndDetails } from '../../../../../../store/actions'; -import { add0x } from '@metamask/utils'; import { parseApprovalTransactionData } from '../../../../../../../shared/modules/transaction.utils'; import { useBalanceChanges } from '../../../simulation-details/useBalanceChanges'; import { BalanceChange } from '../../../simulation-details/types'; diff --git a/ui/pages/confirmations/components/simulation-details/amount-pill.test.tsx b/ui/pages/confirmations/components/simulation-details/amount-pill.test.tsx index d08f9d7d1c52..3da92fbd6727 100644 --- a/ui/pages/confirmations/components/simulation-details/amount-pill.test.tsx +++ b/ui/pages/confirmations/components/simulation-details/amount-pill.test.tsx @@ -3,6 +3,7 @@ import { render } from '@testing-library/react'; import { BigNumber } from 'bignumber.js'; import { TokenStandard } from '../../../../../shared/constants/transaction'; import Tooltip from '../../../../components/ui/tooltip'; +import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../confirm/info/shared/constants'; import { AmountPill } from './amount-pill'; import { AssetIdentifier, @@ -55,8 +56,26 @@ const renderAndExpect = ( asset: AssetIdentifier, amount: BigNumber, expected: { text: string; tooltip: string }, + { + isApproval, + isAllApproval, + isUnlimitedApproval, + }: { + isApproval?: boolean; + isAllApproval?: boolean; + isUnlimitedApproval?: boolean; + } = {}, ): void => { - const { getByText } = render(); + const { getByText } = render( + , + ); + expect(getByText(expected.text)).toBeInTheDocument(); expect(Tooltip).toHaveBeenCalledWith( expect.objectContaining({ title: expected.tooltip }), @@ -234,4 +253,66 @@ describe('AmountPill', () => { }, ); }); + + describe('Approval', () => { + it('renders ERC-20 approval', () => { + renderAndExpect( + ERC20_ASSET_MOCK, + new BigNumber(123.45), + { + text: '123.5', + tooltip: '123.45', + }, + { isApproval: true }, + ); + }); + + it('renders ERC-721 approval', () => { + renderAndExpect( + ERC721_ASSET_MOCK, + new BigNumber(1), + { + text: '#2748', + tooltip: '#2748', + }, + { isApproval: true }, + ); + }); + + it('renders unlimited ERC-20 approval', () => { + renderAndExpect( + ERC20_ASSET_MOCK, + new BigNumber(TOKEN_VALUE_UNLIMITED_THRESHOLD), + { + text: '[unlimited]', + tooltip: '1,000,000,000,000,000', + }, + { isApproval: true, isUnlimitedApproval: true }, + ); + }); + + it('renders all ERC-721 approval', () => { + renderAndExpect( + { ...ERC721_ASSET_MOCK, tokenId: undefined }, + new BigNumber(1), + { + text: '[all]', + tooltip: '[all]', + }, + { isApproval: true, isAllApproval: true }, + ); + }); + + it('renders all ERC-1155 approval', () => { + renderAndExpect( + { ...ERC1155_ASSET_MOCK, tokenId: undefined }, + new BigNumber(1), + { + text: '[all]', + tooltip: '[all]', + }, + { isApproval: true, isAllApproval: true }, + ); + }); + }); }); diff --git a/ui/pages/confirmations/components/simulation-details/amount-pill.tsx b/ui/pages/confirmations/components/simulation-details/amount-pill.tsx index d1c6925aa85e..33a6b2a67816 100644 --- a/ui/pages/confirmations/components/simulation-details/amount-pill.tsx +++ b/ui/pages/confirmations/components/simulation-details/amount-pill.tsx @@ -52,7 +52,7 @@ export const AmountPill: React.FC<{ } // ERC721 amounts are always 1 and are not displayed. - if (asset.standard !== TokenStandard.ERC721) { + if (asset.standard !== TokenStandard.ERC721 && !isAllApproval) { const formattedAmount = isUnlimitedApproval ? t('unlimited') : formatAmount(locale, amount.abs()); @@ -77,7 +77,9 @@ export const AmountPill: React.FC<{ amountParts.push(shortenedTokenIdPart); tooltipParts.push(tooltipIdPart); - } else if (isAllApproval) { + } + + if (isAllApproval) { amountParts.push(t('all')); tooltipParts.push(t('all')); } From 2e5b861f5a32eea6a7438c24714a1007d2cd2d45 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 21:44:40 +0000 Subject: [PATCH 13/19] Add unlimited and all unit tests --- .../useBatchApproveBalanceChanges.test.ts | 117 +++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-) diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts index 6e99e7c72008..26a1f7f5821f 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.test.ts @@ -4,6 +4,7 @@ import { SimulationTokenStandard, } from '@metamask/transaction-controller'; import { BigNumber } from 'bignumber.js'; +import { toHex } from '@metamask/controller-utils'; import { getMockConfirmStateForTransaction } from '../../../../../../../test/data/confirmations/helper'; import { CHAIN_ID, @@ -15,7 +16,9 @@ import { TokenStandard } from '../../../../../../../shared/constants/transaction import { renderHookWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; import { BalanceChange } from '../../../simulation-details/types'; import { buildApproveTransactionData } from '../../../../../../../test/data/confirmations/token-approve'; +import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../shared/constants'; import { useBatchApproveBalanceChanges } from './useBatchApproveBalanceChanges'; +import { buildSetApproveForAllTransactionData } from '../../../../../../../test/data/confirmations/set-approval-for-all'; jest.mock('../../../simulation-details/useBalanceChanges', () => ({ useBalanceChanges: jest.fn(), @@ -88,7 +91,14 @@ describe('useBatchApproveBalanceChanges', () => { expect(result).toStrictEqual({ pending: false, - value: [{ ...BALANCE_CHANGE_MOCK, isApproval: true }], + value: [ + { + ...BALANCE_CHANGE_MOCK, + isApproval: true, + isAllApproval: false, + isUnlimitedApproval: false, + }, + ], }); }); @@ -110,6 +120,41 @@ describe('useBatchApproveBalanceChanges', () => { address: TO_MOCK, difference: AMOUNT_HEX_MOCK, id: undefined, + isAll: false, + isDecrease: true, + isUnlimited: false, + newBalance: '0x0', + previousBalance: '0x0', + standard: SimulationTokenStandard.erc20, + }, + ], + }, + }); + }); + + it('generates unlimited ERC-20 token balance changes', async () => { + await runHook({ + nestedTransactions: [ + { + data: buildApproveTransactionData( + TO_MOCK, + TOKEN_VALUE_UNLIMITED_THRESHOLD, + ), + to: TO_MOCK, + }, + ], + }); + + expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ + chainId: CHAIN_ID, + simulationData: { + tokenBalanceChanges: [ + { + address: TO_MOCK, + difference: toHex(TOKEN_VALUE_UNLIMITED_THRESHOLD), + id: undefined, + isAll: false, + isUnlimited: true, isDecrease: true, newBalance: '0x0', previousBalance: '0x0', @@ -142,7 +187,43 @@ describe('useBatchApproveBalanceChanges', () => { address: TO_MOCK, difference: '0x1', id: AMOUNT_HEX_MOCK, + isAll: false, + isDecrease: true, + isUnlimited: false, + newBalance: '0x0', + previousBalance: '0x0', + standard: SimulationTokenStandard.erc721, + }, + ], + }, + }); + }); + + it('generates all ERC-721 token balance changes', async () => { + getTokenStandardAndDetailsMock.mockResolvedValue({ + standard: TokenStandard.ERC721, + }); + + await runHook({ + nestedTransactions: [ + { + data: buildSetApproveForAllTransactionData(TO_MOCK, true), + to: TO_MOCK, + }, + ], + }); + + expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ + chainId: CHAIN_ID, + simulationData: { + tokenBalanceChanges: [ + { + address: TO_MOCK, + difference: '0x1', + id: undefined, + isAll: true, isDecrease: true, + isUnlimited: false, newBalance: '0x0', previousBalance: '0x0', standard: SimulationTokenStandard.erc721, @@ -152,6 +233,40 @@ describe('useBatchApproveBalanceChanges', () => { }); }); + it('generates all ERC-1155 token balance changes', async () => { + getTokenStandardAndDetailsMock.mockResolvedValue({ + standard: TokenStandard.ERC1155, + }); + + await runHook({ + nestedTransactions: [ + { + data: buildSetApproveForAllTransactionData(TO_MOCK, true), + to: TO_MOCK, + }, + ], + }); + + expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ + chainId: CHAIN_ID, + simulationData: { + tokenBalanceChanges: [ + { + address: TO_MOCK, + difference: '0x1', + id: undefined, + isAll: true, + isDecrease: true, + isUnlimited: false, + newBalance: '0x0', + previousBalance: '0x0', + standard: SimulationTokenStandard.erc1155, + }, + ], + }, + }); + }); + it('generates no token balance changes if no nested transactions', async () => { await runHook({}); From 78980a93eaa32a59536054ad79c046f6208c236e Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 22:33:22 +0000 Subject: [PATCH 14/19] Support edit of ERC-20 approvals --- .../edit-spending-cap-modal.tsx | 35 ++++++++++-- .../batch-simulation-details.tsx | 55 +++++++++++++++---- .../hooks/useBatchApproveBalanceChanges.ts | 17 ++++-- .../simulation-details/balance-change-row.tsx | 24 +++++++- .../components/simulation-details/types.ts | 3 + 5 files changed, 114 insertions(+), 20 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.tsx b/ui/pages/confirmations/components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.tsx index f53c7bae42b2..7febfae50e3a 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.tsx +++ b/ui/pages/confirmations/components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.tsx @@ -1,6 +1,7 @@ import { TransactionMeta } from '@metamask/transaction-controller'; import React, { useCallback, useEffect, useState } from 'react'; import { useDispatch } from 'react-redux'; +import { Hex } from '@metamask/utils'; import { calcTokenAmount } from '../../../../../../../../shared/lib/transactions-controller-utils'; import { hexToDecimal } from '../../../../../../../../shared/modules/conversion.utils'; import { @@ -37,11 +38,15 @@ export function countDecimalDigits(numberString: string) { } export const EditSpendingCapModal = ({ + data, isOpenEditSpendingCapModal, setIsOpenEditSpendingCapModal, + to, }: { + data?: Hex; isOpenEditSpendingCapModal: boolean; setIsOpenEditSpendingCapModal: (newValue: boolean) => void; + to?: Hex; }) => { const t = useI18nContext(); @@ -50,10 +55,17 @@ export const EditSpendingCapModal = ({ const { currentConfirmation: transactionMeta } = useConfirmContext(); + const currentTo = transactionMeta.txParams.to; + const currentFrom = transactionMeta.txParams.from; + const currentData = transactionMeta.txParams.data; + + const transactionTo = to ?? currentTo; + const transactionData = data ?? currentData; + const { userBalance, tokenSymbol, decimals } = useAssetDetails( - transactionMeta.txParams.to, - transactionMeta.txParams.from, - transactionMeta.txParams.data, + transactionTo, + currentFrom, + transactionData, transactionMeta.chainId, ); @@ -62,8 +74,18 @@ export const EditSpendingCapModal = ({ Number(decimals ?? '0'), ).toFixed(); + const finalTransactionMeta = { + ...transactionMeta, + txParams: { + ...transactionMeta.txParams, + to: transactionTo, + from: currentFrom, + data: transactionData, + }, + }; + const { formattedSpendingCap, spendingCap } = useApproveTokenSimulation( - transactionMeta, + finalTransactionMeta, decimals, ); @@ -88,6 +110,11 @@ export const EditSpendingCapModal = ({ const [isModalSaving, setIsModalSaving] = useState(false); const handleSubmit = useCallback(async () => { + // Pending future ticket to update data of specific nested transaction within batch transaction. + if (data) { + return; + } + setIsModalSaving(true); const customTxParamsData = getCustomTxParamsData( diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx index 6a87e867a04c..85d1cf79b639 100644 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx @@ -1,11 +1,19 @@ -import React from 'react'; -import { TransactionMeta } from '@metamask/transaction-controller'; +import React, { useCallback, useState } from 'react'; +import { + BatchTransactionParams, + TransactionMeta, +} from '@metamask/transaction-controller'; import { SimulationDetails, StaticRow, } from '../../../../simulation-details/simulation-details'; -import { useBatchApproveBalanceChanges } from '../../hooks/useBatchApproveBalanceChanges'; +import { + ApprovalBalanceChange, + useBatchApproveBalanceChanges, +} from '../../hooks/useBatchApproveBalanceChanges'; import { useConfirmContext } from '../../../../../context/confirm'; +import { EditSpendingCapModal } from '../../approve/edit-spending-cap-modal/edit-spending-cap-modal'; +import { TokenStandard } from '../../../../../../../../shared/constants/transaction'; export function BatchSimulationDetails() { const { currentConfirmation: transactionMeta } = @@ -14,17 +22,44 @@ export function BatchSimulationDetails() { const { value: approveBalanceChanges } = useBatchApproveBalanceChanges() ?? {}; + const [isEditApproveModalOpen, setIsEditApproveModalOpen] = useState(false); + + const [nestedTransactionToEdit, setNestedTransactionToEdit] = useState< + BatchTransactionParams | undefined + >(); + + const handleEdit = useCallback((balanceChange: ApprovalBalanceChange) => { + setNestedTransactionToEdit(balanceChange.nestedTransaction); + setIsEditApproveModalOpen(true); + }, []); + + const finalBalanceChanges = approveBalanceChanges?.map((change) => ({ + ...change, + onEdit: + change.asset.standard === TokenStandard.ERC20 + ? () => handleEdit(change) + : undefined, + })); + const approveRow: StaticRow = { label: 'You approve', - balanceChanges: approveBalanceChanges ?? [], + balanceChanges: finalBalanceChanges ?? [], }; return ( - + <> + + + ); } diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts index db591dff4cc4..d0ada77aa1af 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useBatchApproveBalanceChanges.ts @@ -13,9 +13,14 @@ import { useBalanceChanges } from '../../../simulation-details/useBalanceChanges import { BalanceChange } from '../../../simulation-details/types'; import { isSpendingCapUnlimited } from '../approve/hooks/use-approve-token-simulation'; -type ApprovalBalanceChange = SimulationTokenBalanceChange & { +type ApprovalSimulationBalanceChange = SimulationTokenBalanceChange & { isAll: boolean; isUnlimited: boolean; + nestedTransaction: BatchTransactionParams; +}; + +export type ApprovalBalanceChange = BalanceChange & { + nestedTransaction: BatchTransactionParams; }; export function useBatchApproveBalanceChanges() { @@ -35,7 +40,7 @@ export function useBatchApproveBalanceChanges() { }, }); - const finalBalanceChanges = (balanceChanges ?? []).map( + const finalBalanceChanges = (balanceChanges ?? []).map( (change, index) => { const simulation = simulationBalanceChanges?.[index]; @@ -44,6 +49,7 @@ export function useBatchApproveBalanceChanges() { isApproval: true, isAllApproval: simulation?.isAll ?? false, isUnlimitedApproval: simulation?.isUnlimited ?? false, + nestedTransaction: simulation?.nestedTransaction ?? {}, }; }, ); @@ -68,8 +74,8 @@ async function buildSimulationTokenBalanceChanges({ nestedTransactions, }: { nestedTransactions?: BatchTransactionParams[]; -}): Promise { - const balanceChanges: ApprovalBalanceChange[] = []; +}): Promise { + const balanceChanges: ApprovalSimulationBalanceChange[] = []; if (!nestedTransactions) { return balanceChanges; @@ -110,7 +116,7 @@ async function buildSimulationTokenBalanceChanges({ const isUnlimited = !isNFT && isSpendingCapUnlimited(amountOrTokenId?.toNumber() ?? 0); - const balanceChange: ApprovalBalanceChange = { + const balanceChange: ApprovalSimulationBalanceChange = { address: to, difference, id: tokenId, @@ -118,6 +124,7 @@ async function buildSimulationTokenBalanceChanges({ isDecrease: true, isUnlimited, newBalance: '0x0', + nestedTransaction: transaction, previousBalance: '0x0', standard, }; diff --git a/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx b/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx index 19f6256c8372..9818ea4f1a67 100644 --- a/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx +++ b/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx @@ -1,12 +1,20 @@ import React from 'react'; +import { IconName } from '@metamask/snaps-sdk/jsx'; import { AlignItems, Display, FlexDirection, FlexWrap, + IconColor, TextVariant, } from '../../../../helpers/constants/design-system'; -import { Box, Text } from '../../../../components/component-library'; +import { + Box, + ButtonIcon, + ButtonIconSize, + Text, +} from '../../../../components/component-library'; +import { useI18nContext } from '../../../../hooks/useI18nContext'; import { AssetPill } from './asset-pill'; import { AmountPill } from './amount-pill'; import { BalanceChange } from './types'; @@ -25,6 +33,8 @@ export const BalanceChangeRow: React.FC<{ showFiat?: boolean; balanceChange: BalanceChange; }> = ({ label, showFiat, balanceChange }) => { + const t = useI18nContext(); + const { asset, amount, @@ -32,6 +42,7 @@ export const BalanceChangeRow: React.FC<{ isApproval, isAllApproval, isUnlimitedApproval, + onEdit, } = balanceChange; return ( @@ -56,6 +67,17 @@ export const BalanceChangeRow: React.FC<{ style={{ minWidth: 0 }} > + {onEdit && ( + + )} void; }>; From 53e4625d43f7b12706406697d2a8f159fe49b8df Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 22:56:25 +0000 Subject: [PATCH 15/19] Add edit unit tests --- .../batch-simulation-details.test.tsx | 95 ++++++++++++++++++- .../batch-simulation-details.tsx | 14 +-- .../useBatchApproveBalanceChanges.test.ts | 56 ++++++----- .../balance-change-row.test.tsx | 13 +++ .../simulation-details/balance-change-row.tsx | 1 + 5 files changed, 146 insertions(+), 33 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx index a8defc5a4e88..cf8347a0f6d6 100644 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.test.tsx @@ -1,14 +1,18 @@ import React from 'react'; import { BigNumber } from 'bignumber.js'; +import { BatchTransactionParams } from '@metamask/transaction-controller'; import { renderWithConfirmContextProvider } from '../../../../../../../../test/lib/confirmations/render-helpers'; import configureStore from '../../../../../../../store/store'; import { getMockConfirmStateForTransaction } from '../../../../../../../../test/data/confirmations/helper'; import { genUnapprovedContractInteractionConfirmation } from '../../../../../../../../test/data/confirmations/contract-interaction'; -import { useBatchApproveBalanceChanges } from '../../hooks/useBatchApproveBalanceChanges'; +import { + ApprovalBalanceChange, + useBatchApproveBalanceChanges, +} from '../../hooks/useBatchApproveBalanceChanges'; import { AlertMetricsProvider } from '../../../../../../../components/app/alert-system/contexts/alertMetricsContext'; import { useBalanceChanges } from '../../../../simulation-details/useBalanceChanges'; -import { BalanceChange } from '../../../../simulation-details/types'; import { TokenStandard } from '../../../../../../../../shared/constants/transaction'; +import { buildApproveTransactionData } from '../../../../../../../../test/data/confirmations/token-approve'; import { BatchSimulationDetails } from './batch-simulation-details'; jest.mock('../../../../simulation-details/useBalanceChanges', () => ({ @@ -22,7 +26,12 @@ jest.mock('../../hooks/useBatchApproveBalanceChanges', () => ({ const ADDRESS_MOCK = '0x1234567891234567891234567891234567891234'; const ADDRESS_SHORT_MOCK = '0x12345...91234'; -const BALANCE_CHANGE_ERC20_MOCK: BalanceChange = { +const NESTED_TRANSACTION_MOCK: BatchTransactionParams = { + data: buildApproveTransactionData(ADDRESS_MOCK, 123), + to: ADDRESS_MOCK, +}; + +const BALANCE_CHANGE_ERC20_MOCK: ApprovalBalanceChange = { asset: { address: ADDRESS_MOCK, chainId: '0x123', @@ -31,9 +40,12 @@ const BALANCE_CHANGE_ERC20_MOCK: BalanceChange = { amount: new BigNumber(123.56), fiatAmount: null, isApproval: true, + isAllApproval: false, + isUnlimitedApproval: false, + nestedTransaction: NESTED_TRANSACTION_MOCK, }; -const BALANCE_CHANGE_ERC721_MOCK: BalanceChange = { +const BALANCE_CHANGE_ERC721_MOCK: ApprovalBalanceChange = { asset: { address: ADDRESS_MOCK, chainId: '0x123', @@ -43,9 +55,12 @@ const BALANCE_CHANGE_ERC721_MOCK: BalanceChange = { amount: new BigNumber(1), fiatAmount: null, isApproval: true, + isAllApproval: false, + isUnlimitedApproval: false, + nestedTransaction: NESTED_TRANSACTION_MOCK, }; -const BALANCE_CHANGE_ERC1155_MOCK: BalanceChange = { +const BALANCE_CHANGE_ERC1155_MOCK: ApprovalBalanceChange = { asset: { address: ADDRESS_MOCK, chainId: '0x123', @@ -55,6 +70,9 @@ const BALANCE_CHANGE_ERC1155_MOCK: BalanceChange = { amount: new BigNumber(123), fiatAmount: null, isApproval: true, + isAllApproval: false, + isUnlimitedApproval: false, + nestedTransaction: NESTED_TRANSACTION_MOCK, }; function render() { @@ -109,6 +127,18 @@ describe('BatchSimulationDetails', () => { expect(getByText(ADDRESS_SHORT_MOCK)).toBeInTheDocument(); }); + it('renders unlimited ERC-20 approve row', () => { + useBatchApproveBalanceChangesMock.mockReturnValue({ + pending: false, + value: [{ ...BALANCE_CHANGE_ERC20_MOCK, isUnlimitedApproval: true }], + }); + + const { getByText } = render(); + expect(getByText('You approve')).toBeInTheDocument(); + expect(getByText('Unlimited')).toBeInTheDocument(); + expect(getByText(ADDRESS_SHORT_MOCK)).toBeInTheDocument(); + }); + it('renders ERC-721 approve row', () => { useBatchApproveBalanceChangesMock.mockReturnValue({ pending: false, @@ -121,6 +151,27 @@ describe('BatchSimulationDetails', () => { expect(getByText(ADDRESS_SHORT_MOCK)).toBeInTheDocument(); }); + it('renders all ERC-721 approve row', () => { + useBatchApproveBalanceChangesMock.mockReturnValue({ + pending: false, + value: [ + { + ...BALANCE_CHANGE_ERC721_MOCK, + asset: { + ...BALANCE_CHANGE_ERC721_MOCK.asset, + tokenId: undefined, + }, + isAllApproval: true, + }, + ], + }); + + const { getByText } = render(); + expect(getByText('You approve')).toBeInTheDocument(); + expect(getByText('All')).toBeInTheDocument(); + expect(getByText(ADDRESS_SHORT_MOCK)).toBeInTheDocument(); + }); + it('renders ERC-1155 approve row', () => { useBatchApproveBalanceChangesMock.mockReturnValue({ pending: false, @@ -133,6 +184,27 @@ describe('BatchSimulationDetails', () => { expect(getByText(ADDRESS_SHORT_MOCK)).toBeInTheDocument(); }); + it('renders all ERC-1155 approve row', () => { + useBatchApproveBalanceChangesMock.mockReturnValue({ + pending: false, + value: [ + { + ...BALANCE_CHANGE_ERC1155_MOCK, + asset: { + ...BALANCE_CHANGE_ERC1155_MOCK.asset, + tokenId: undefined, + }, + isAllApproval: true, + }, + ], + }); + + const { getByText } = render(); + expect(getByText('You approve')).toBeInTheDocument(); + expect(getByText('All')).toBeInTheDocument(); + expect(getByText(ADDRESS_SHORT_MOCK)).toBeInTheDocument(); + }); + it('renders multiple approve rows', () => { useBatchApproveBalanceChangesMock.mockReturnValue({ pending: false, @@ -149,4 +221,17 @@ describe('BatchSimulationDetails', () => { const { queryByText } = render(); expect(queryByText('You approve')).toBeNull(); }); + + it('shows edit modal on edit click', () => { + useBatchApproveBalanceChangesMock.mockReturnValue({ + pending: false, + value: [BALANCE_CHANGE_ERC20_MOCK], + }); + + const { getByTestId, getByText } = render(); + + getByTestId('balance-change-edit').click(); + + expect(getByText('Edit spending cap')).toBeInTheDocument(); + }); }); diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx index 85d1cf79b639..06d629d66ff0 100644 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx @@ -48,12 +48,14 @@ export function BatchSimulationDetails() { return ( <> - + {isEditApproveModalOpen && ( + + )} ({ useBalanceChanges: jest.fn(), @@ -97,6 +97,7 @@ describe('useBatchApproveBalanceChanges', () => { isApproval: true, isAllApproval: false, isUnlimitedApproval: false, + nestedTransaction: {}, }, ], }); @@ -123,6 +124,10 @@ describe('useBatchApproveBalanceChanges', () => { isAll: false, isDecrease: true, isUnlimited: false, + nestedTransaction: { + data: DATA_MOCK, + to: TO_MOCK, + }, newBalance: '0x0', previousBalance: '0x0', standard: SimulationTokenStandard.erc20, @@ -133,16 +138,16 @@ describe('useBatchApproveBalanceChanges', () => { }); it('generates unlimited ERC-20 token balance changes', async () => { + const nestedTransaction: BatchTransactionParams = { + data: buildApproveTransactionData( + TO_MOCK, + TOKEN_VALUE_UNLIMITED_THRESHOLD, + ), + to: TO_MOCK, + }; + await runHook({ - nestedTransactions: [ - { - data: buildApproveTransactionData( - TO_MOCK, - TOKEN_VALUE_UNLIMITED_THRESHOLD, - ), - to: TO_MOCK, - }, - ], + nestedTransactions: [nestedTransaction], }); expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ @@ -156,6 +161,7 @@ describe('useBatchApproveBalanceChanges', () => { isAll: false, isUnlimited: true, isDecrease: true, + nestedTransaction, newBalance: '0x0', previousBalance: '0x0', standard: SimulationTokenStandard.erc20, @@ -190,6 +196,10 @@ describe('useBatchApproveBalanceChanges', () => { isAll: false, isDecrease: true, isUnlimited: false, + nestedTransaction: { + data: DATA_MOCK, + to: TO_MOCK, + }, newBalance: '0x0', previousBalance: '0x0', standard: SimulationTokenStandard.erc721, @@ -204,13 +214,13 @@ describe('useBatchApproveBalanceChanges', () => { standard: TokenStandard.ERC721, }); + const nestedTransaction: BatchTransactionParams = { + data: buildSetApproveForAllTransactionData(TO_MOCK, true), + to: TO_MOCK, + }; + await runHook({ - nestedTransactions: [ - { - data: buildSetApproveForAllTransactionData(TO_MOCK, true), - to: TO_MOCK, - }, - ], + nestedTransactions: [nestedTransaction], }); expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ @@ -224,6 +234,7 @@ describe('useBatchApproveBalanceChanges', () => { isAll: true, isDecrease: true, isUnlimited: false, + nestedTransaction, newBalance: '0x0', previousBalance: '0x0', standard: SimulationTokenStandard.erc721, @@ -238,13 +249,13 @@ describe('useBatchApproveBalanceChanges', () => { standard: TokenStandard.ERC1155, }); + const nestedTransaction: BatchTransactionParams = { + data: buildSetApproveForAllTransactionData(TO_MOCK, true), + to: TO_MOCK, + }; + await runHook({ - nestedTransactions: [ - { - data: buildSetApproveForAllTransactionData(TO_MOCK, true), - to: TO_MOCK, - }, - ], + nestedTransactions: [nestedTransaction], }); expect(useBalanceChangesMock).toHaveBeenLastCalledWith({ @@ -258,6 +269,7 @@ describe('useBatchApproveBalanceChanges', () => { isAll: true, isDecrease: true, isUnlimited: false, + nestedTransaction, newBalance: '0x0', previousBalance: '0x0', standard: SimulationTokenStandard.erc1155, diff --git a/ui/pages/confirmations/components/simulation-details/balance-change-row.test.tsx b/ui/pages/confirmations/components/simulation-details/balance-change-row.test.tsx index 944ce3b130b9..35f98b0408a5 100644 --- a/ui/pages/confirmations/components/simulation-details/balance-change-row.test.tsx +++ b/ui/pages/confirmations/components/simulation-details/balance-change-row.test.tsx @@ -74,4 +74,17 @@ describe('BalanceChangeRow', () => { render(); expect(IndividualFiatDisplay).not.toHaveBeenCalled(); }); + + it('renders edit icon if onEdit is provided', () => { + const onEdit = jest.fn(); + + const { getByTestId } = render( + , + ); + + expect(getByTestId('balance-change-edit')).toBeInTheDocument(); + }); }); diff --git a/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx b/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx index 9818ea4f1a67..36324a434c67 100644 --- a/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx +++ b/ui/pages/confirmations/components/simulation-details/balance-change-row.tsx @@ -69,6 +69,7 @@ export const BalanceChangeRow: React.FC<{ {onEdit && ( Date: Fri, 21 Feb 2025 22:58:08 +0000 Subject: [PATCH 16/19] Add translation --- app/_locales/en/messages.json | 3 +++ app/_locales/en_GB/messages.json | 3 +++ .../batch-simulation-details/batch-simulation-details.tsx | 5 ++++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 0eb802250fbb..1a895c83d2b7 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -998,6 +998,9 @@ "confirmRecoveryPhrase": { "message": "Confirm Secret Recovery Phrase" }, + "confirmSimulationApprove": { + "message": "You approve" + }, "confirmTitleApproveTransactionNFT": { "message": "Withdrawal request" }, diff --git a/app/_locales/en_GB/messages.json b/app/_locales/en_GB/messages.json index 0eb802250fbb..1a895c83d2b7 100644 --- a/app/_locales/en_GB/messages.json +++ b/app/_locales/en_GB/messages.json @@ -998,6 +998,9 @@ "confirmRecoveryPhrase": { "message": "Confirm Secret Recovery Phrase" }, + "confirmSimulationApprove": { + "message": "You approve" + }, "confirmTitleApproveTransactionNFT": { "message": "Withdrawal request" }, diff --git a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx index 06d629d66ff0..0eb57e45cccd 100644 --- a/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx +++ b/ui/pages/confirmations/components/confirm/info/batch/batch-simulation-details/batch-simulation-details.tsx @@ -14,8 +14,11 @@ import { import { useConfirmContext } from '../../../../../context/confirm'; import { EditSpendingCapModal } from '../../approve/edit-spending-cap-modal/edit-spending-cap-modal'; import { TokenStandard } from '../../../../../../../../shared/constants/transaction'; +import { useI18nContext } from '../../../../../../../hooks/useI18nContext'; export function BatchSimulationDetails() { + const t = useI18nContext(); + const { currentConfirmation: transactionMeta } = useConfirmContext(); @@ -42,7 +45,7 @@ export function BatchSimulationDetails() { })); const approveRow: StaticRow = { - label: 'You approve', + label: t('confirmSimulationApprove'), balanceChanges: finalBalanceChanges ?? [], }; From 30058aafc7029d43b557e231c2606a70050f2df9 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 23:10:52 +0000 Subject: [PATCH 17/19] Add SimulationDetails unit test --- .../simulation-details.test.tsx | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/ui/pages/confirmations/components/simulation-details/simulation-details.test.tsx b/ui/pages/confirmations/components/simulation-details/simulation-details.test.tsx index e312de542772..806524f369ad 100644 --- a/ui/pages/confirmations/components/simulation-details/simulation-details.test.tsx +++ b/ui/pages/confirmations/components/simulation-details/simulation-details.test.tsx @@ -9,7 +9,8 @@ import { import { BigNumber } from 'bignumber.js'; import { renderWithProvider } from '../../../../../test/lib/render-helpers'; import mockState from '../../../../../test/data/mock-state.json'; -import { SimulationDetails } from './simulation-details'; +import { TokenStandard } from '../../../../../shared/constants/transaction'; +import { SimulationDetails, StaticRow } from './simulation-details'; import { useBalanceChanges } from './useBalanceChanges'; import { BalanceChangeList } from './balance-change-list'; import { BalanceChange } from './types'; @@ -44,6 +45,7 @@ jest.mock('../../context/confirm', () => ({ const renderSimulationDetails = ( simulationData?: Partial, metricsOnly?: boolean, + staticRows?: StaticRow[], ) => renderWithProvider( , store, ); @@ -150,4 +153,38 @@ describe('SimulationDetails', () => { const { container } = renderSimulationDetails({}, true); expect(container).toBeEmptyDOMElement(); }); + + it('renders static rows if provided', () => { + (useBalanceChanges as jest.Mock).mockReturnValue({ + pending: false, + value: [], + }); + + const staticRows: StaticRow[] = [ + { + label: 'Test Label', + balanceChanges: [ + { + asset: { + address: '0x123', + chainId: '0x321', + standard: TokenStandard.ERC20, + }, + amount: new BigNumber(123), + fiatAmount: 456, + }, + ], + }, + ]; + + renderSimulationDetails({}, false, staticRows); + + expect(BalanceChangeList).toHaveBeenLastCalledWith( + expect.objectContaining({ + heading: 'Test Label', + balanceChanges: staticRows[0].balanceChanges, + }), + {}, + ); + }); }); From 84bad487e6f90dbe6705899e9592f5c463758c3e Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 23:19:15 +0000 Subject: [PATCH 18/19] Add unit tests for isSpendingCapUnlimited --- .../use-approve-token-simulation.test.ts | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.test.ts b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.test.ts index a4b867f82018..31ff95170b0e 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.test.ts +++ b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.test.ts @@ -2,7 +2,11 @@ import { TransactionMeta } from '@metamask/transaction-controller'; import { renderHookWithConfirmContextProvider } from '../../../../../../../../test/lib/confirmations/render-helpers'; import { genUnapprovedApproveConfirmation } from '../../../../../../../../test/data/confirmations/token-approve'; import { getMockConfirmStateForTransaction } from '../../../../../../../../test/data/confirmations/helper'; -import { useApproveTokenSimulation } from './use-approve-token-simulation'; +import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../../shared/constants'; +import { + isSpendingCapUnlimited, + useApproveTokenSimulation, +} from './use-approve-token-simulation'; import { useIsNFT } from './use-is-nft'; jest.mock('./use-is-nft', () => ({ @@ -10,6 +14,24 @@ jest.mock('./use-is-nft', () => ({ useIsNFT: jest.fn(), })); +describe('isSpendingCapUnlimited', () => { + it('returns true if spending cap is equal to threshold', () => { + expect(isSpendingCapUnlimited(TOKEN_VALUE_UNLIMITED_THRESHOLD)).toBe(true); + }); + + it('returns true if spending cap is greater than threshold', () => { + expect(isSpendingCapUnlimited(TOKEN_VALUE_UNLIMITED_THRESHOLD + 1)).toBe( + true, + ); + }); + + it('returns false if spending cap is less than threshold', () => { + expect(isSpendingCapUnlimited(TOKEN_VALUE_UNLIMITED_THRESHOLD - 1)).toBe( + false, + ); + }); +}); + describe('useApproveTokenSimulation', () => { beforeEach(() => { jest.resetAllMocks(); From 919d4c87b9d125665e063ccef5513d6f2552e6fc Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 21 Feb 2025 23:40:11 +0000 Subject: [PATCH 19/19] Fix unit test --- .../confirm/title/hooks/useCurrentSpendingCap.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts b/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts index 9bea069d0935..6734fc705f53 100644 --- a/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts +++ b/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts @@ -16,6 +16,6 @@ describe('useCurrentSpendingCap', () => { mockState, ); - expect(result.current.customSpendingCap).toMatchInlineSnapshot(`"#0"`); + expect(result.current.customSpendingCap).toMatchInlineSnapshot(`"#1"`); }); });