Skip to content

Commit 1c6391c

Browse files
fix: remove reliance on transaction decode in confirmations (#29341)
## **Description** Disable all advanced transaction data decoding using Sourcify, 4Byte and Uniswap, if the `Decode smart contracts` toggle is disabled. Remove all reliance on the advanced decoding excluding the `Data` section. Specifically: - Create `useTokenTransactionData` hook to decode all token transactions locally using the ABIs. - Replace all usages of `useDecodedTransactionData` with the new hook, except for the `TransactionData` component. - Update related unit and integration tests to decode valid test data rather than rely on mocking hooks. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29341?quickstart=1) ## **Related issues** ## **Manual testing steps** Regression of redesigned transaction confirmations. Specifically: - Token Transfer Recipient - Token Transfer Amount - Approval Spender - Approval Spending Cap ## **Screenshots/Recordings** ### **Before** ### **After** <img width="354" alt="New Toggle Description" src="https://github.com/user-attachments/assets/180e23d4-39d8-44b3-b3cf-38bed360ec9e" /> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
1 parent 2eced17 commit 1c6391c

File tree

52 files changed

+1542
-671
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+1542
-671
lines changed

app/_locales/de/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/el/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/en/messages.json

+3-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/es/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/fr/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/hi/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/id/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/ja/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/ko/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/pt/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/ru/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/tl/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/tr/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/vi/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/zh_CN/messages.json

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/data/confirmations/set-approval-for-all.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import {
66
genUnapprovedContractInteractionConfirmation,
77
} from './contract-interaction';
88

9+
export const INCREASE_ALLOWANCE_TRANSACTION_DATA =
10+
'0x395093510000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000123';
11+
912
export const genUnapprovedSetApprovalForAllConfirmation = ({
1013
address = CONTRACT_INTERACTION_SENDER_ADDRESS,
1114
chainId = CHAIN_ID,
@@ -16,7 +19,7 @@ export const genUnapprovedSetApprovalForAllConfirmation = ({
1619
...genUnapprovedContractInteractionConfirmation({ chainId }),
1720
txParams: {
1821
from: address,
19-
data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001',
22+
data: '0xa22cb4650000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001',
2023
gas: '0x16a92',
2124
to: '0x076146c765189d51be3160a2140cf80bfc73ad68',
2225
value: '0x0',

test/data/confirmations/token-approve.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,16 @@ import {
99
export const genUnapprovedApproveConfirmation = ({
1010
address = CONTRACT_INTERACTION_SENDER_ADDRESS,
1111
chainId = CHAIN_ID,
12+
amountHex = '0000000000000000000000000000000000000000000000000000000000000001',
1213
}: {
1314
address?: Hex;
1415
chainId?: string;
16+
amountHex?: string;
1517
} = {}) => ({
1618
...genUnapprovedContractInteractionConfirmation({ chainId }),
1719
txParams: {
1820
from: address,
19-
data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001',
21+
data: `0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b${amountHex}`,
2022
gas: '0x16a92',
2123
to: '0x076146c765189d51be3160a2140cf80bfc73ad68',
2224
value: '0x0',

test/data/confirmations/token-transfer.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,24 @@ import {
66
genUnapprovedContractInteractionConfirmation,
77
} from './contract-interaction';
88

9+
export const TRANSFER_FROM_TRANSACTION_DATA =
10+
'0x23b872dd0000000000000000000000002e0D7E8c45221FcA00d74a3609A0f7097035d09B0000000000000000000000002e0D7E8c45221FcA00d74a3609A0f7097035d09C0000000000000000000000000000000000000000000000000000000000000123';
11+
912
export const genUnapprovedTokenTransferConfirmation = ({
1013
address = CONTRACT_INTERACTION_SENDER_ADDRESS,
1114
chainId = CHAIN_ID,
1215
isWalletInitiatedConfirmation = false,
16+
amountHex = '0000000000000000000000000000000000000000000000000000000000000001',
1317
}: {
1418
address?: Hex;
1519
chainId?: string;
1620
isWalletInitiatedConfirmation?: boolean;
21+
amountHex?: string;
1722
} = {}) => ({
1823
...genUnapprovedContractInteractionConfirmation({ chainId }),
1924
txParams: {
2025
from: address,
21-
data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001',
26+
data: `0xa9059cbb0000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b${amountHex}`,
2227
gas: '0x16a92',
2328
to: '0x076146c765189d51be3160a2140cf80bfc73ad68',
2429
value: '0x0',

test/integration/confirmations/transactions/increase-allowance.test.tsx

+3-3
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ describe('ERC20 increaseAllowance Confirmation', () => {
213213
'simulation-token-value',
214214
);
215215
expect(simulationSection).toContainElement(spendingCapValue);
216-
expect(spendingCapValue).toHaveTextContent('1');
216+
expect(spendingCapValue).toHaveTextContent('3');
217217
expect(simulationSection).toHaveTextContent('0x07614...3ad68');
218218
});
219219

@@ -240,7 +240,7 @@ describe('ERC20 increaseAllowance Confirmation', () => {
240240

241241
expect(approveDetails).toContainElement(approveDetailsSpender);
242242
expect(approveDetailsSpender).toHaveTextContent(tEn('spender') as string);
243-
expect(approveDetailsSpender).toHaveTextContent('0x2e0D7...5d09B');
243+
expect(approveDetailsSpender).toHaveTextContent('0x9bc5b...AfEF4');
244244
const spenderTooltip = await screen.findByTestId(
245245
'confirmation__approve-spender-tooltip',
246246
);
@@ -301,7 +301,7 @@ describe('ERC20 increaseAllowance Confirmation', () => {
301301
);
302302
expect(spendingCapSection).toContainElement(spendingCapGroup);
303303
expect(spendingCapGroup).toHaveTextContent(tEn('spendingCap') as string);
304-
expect(spendingCapGroup).toHaveTextContent('1');
304+
expect(spendingCapGroup).toHaveTextContent('3');
305305

306306
const spendingCapGroupTooltip = await screen.findByTestId(
307307
'confirmation__approve-spending-cap-group-tooltip',

test/integration/confirmations/transactions/set-approval-for-all.test.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ describe('ERC721 setApprovalForAll Confirmation', () => {
246246
expect(approveDetailsSpender).toHaveTextContent(
247247
tEn('permissionFor') as string,
248248
);
249-
expect(approveDetailsSpender).toHaveTextContent('0x2e0D7...5d09B');
249+
expect(approveDetailsSpender).toHaveTextContent('0x9bc5b...AfEF4');
250250
const spenderTooltip = await screen.findByTestId(
251251
'confirmation__approve-spender-tooltip',
252252
);

ui/helpers/constants/settings.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ const SETTINGS_CONSTANTS = [
212212
{
213213
tabMessage: (t) => t('securityAndPrivacy'),
214214
sectionMessage: (t) => t('use4ByteResolution'),
215-
descriptionMessage: (t) => t('use4ByteResolutionDescription'),
215+
descriptionMessage: (t) => t('toggleDecodeDescription'),
216216
route: `${SECURITY_ROUTE}#decode-smart-contracts`,
217217
icon: 'fa fa-lock',
218218
},

0 commit comments

Comments
 (0)