Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: token approvals from nested transactions in simulation details #30511

Open
wants to merge 19 commits into
base: feat/atomic-batch-transactions
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions app/_locales/en_GB/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

65 changes: 65 additions & 0 deletions shared/modules/transaction.utils.test.js
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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,
});
});
});
});
35 changes: 35 additions & 0 deletions shared/modules/transaction.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 'bignumber.js';
import { AssetType, TokenStandard } from '../constants/transaction';
import { readAddressAsContract } from './contract-utils';
import { isEqualCaseInsensitive } from './string-utils';
Expand Down Expand Up @@ -326,3 +327,37 @@ 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 ?? '')
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we can compare token method names by leveraging the TransactionTypes like we do in determineTransactionType, instead of hardcoding the strings?

Copy link
Member Author

@matthewwalsh0 matthewwalsh0 Feb 24, 2025

Choose a reason for hiding this comment

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

We technically could, but it's coincidence that the types match the names as some of the transaction types for example don't align with any contract methods.

Though I'm not opposed to using a central enum, assuming one exists or we add one.

) {
return undefined;
}

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;

return {
amountOrTokenId,
isApproveAll,
isRevokeAll,
};
}
4 changes: 4 additions & 0 deletions test/data/confirmations/contract-interaction.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
BatchTransactionParams,
CHAIN_IDS,
SimulationData,
TransactionMeta,
Expand All @@ -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 = {
Expand Down Expand Up @@ -135,6 +138,7 @@ export const genUnapprovedContractInteractionConfirmation = ({
],
id: '1d7c08c0-fe54-11ee-9243-91b1e533746a',
origin: 'https://metamask.github.io',
nestedTransactions,
securityAlertResponse: {
features: [],
reason: '',
Expand Down
10 changes: 10 additions & 0 deletions test/data/confirmations/set-approval-for-all.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
Expand Down
19 changes: 19 additions & 0 deletions test/data/confirmations/token-approve.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,30 @@
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 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,
Expand Down
6 changes: 6 additions & 0 deletions ui/__mocks__/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]: {
Expand All @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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();

Expand All @@ -50,10 +55,17 @@ export const EditSpendingCapModal = ({
const { currentConfirmation: transactionMeta } =
useConfirmContext<TransactionMeta>();

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,
);

Expand All @@ -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,
);

Expand All @@ -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(
Expand Down
Loading
Loading