Skip to content

Commit 860c03f

Browse files
feat(): Adds confirmation screen for 'increaseAllowance' (#23560)
~**NOTE: This PR is blocked by [changes to core repo](MetaMask/core#4069) as well as to the [test dApp](MetaMask/test-dapp#304) first.**~ ## **Description** Reuses the token approve confirmation screen for 'increaseAllowance'. It also includes an e2e test for the complete flow. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23560?quickstart=1) ## **Related issues** Fixes: [#2224](MetaMask/MetaMask-planning#2224) ## **Manual testing steps** 1. Go to the test dApp 2. Create a token 3. Approve token for a small amount 4. Try to transfer a higher amount of tokens from another account. (it should fail) 5. Go back to the first account and increase the allowance. 6. Try to transfer again from the second account. (it should succeed) ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <img width="472" alt="Screenshot 2024-03-18 at 17 54 12" src="https://github.com/MetaMask/metamask-extension/assets/13814744/243a529b-4710-419b-8764-f2c06be5a903"> ### **After** <img width="472" alt="Screenshot 2024-03-18 at 17 54 38" src="https://github.com/MetaMask/metamask-extension/assets/13814744/03843cf3-477d-4dbb-8c24-ae3b808a766f"> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [x] I've included screenshots/recordings if applicable - [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/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] I’ve properly set the pull request status: - [x] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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 b2d843f commit 860c03f

File tree

19 files changed

+380
-37
lines changed

19 files changed

+380
-37
lines changed

app/_locales/en/messages.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/scripts/lib/transaction/metrics.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,7 @@ async function buildEventFragmentProperties({
864864
[
865865
TransactionType.contractInteraction,
866866
TransactionType.tokenMethodApprove,
867+
TransactionType.tokenMethodIncreaseAllowance,
867868
TransactionType.tokenMethodSafeTransferFrom,
868869
TransactionType.tokenMethodSetApprovalForAll,
869870
TransactionType.tokenMethodTransfer,

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@
297297
"@metamask/logging-controller": "^2.0.2",
298298
"@metamask/logo": "^3.1.2",
299299
"@metamask/message-manager": "^7.3.0",
300-
"@metamask/metamask-eth-abis": "^3.0.0",
300+
"@metamask/metamask-eth-abis": "^3.1.1",
301301
"@metamask/name-controller": "^4.2.0",
302302
"@metamask/network-controller": "patch:@metamask/network-controller@npm%3A18.0.1#~/.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch",
303303
"@metamask/notification-controller": "^3.0.0",
@@ -431,7 +431,7 @@
431431
"@metamask/forwarder": "^1.1.0",
432432
"@metamask/phishing-warning": "^3.0.3",
433433
"@metamask/test-bundler": "^1.0.0",
434-
"@metamask/test-dapp": "^8.3.0",
434+
"@metamask/test-dapp": "^8.4.0",
435435
"@playwright/test": "^1.39.0",
436436
"@sentry/cli": "^2.19.4",
437437
"@storybook/addon-a11y": "^7.4.6",

shared/modules/transaction.utils.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { isHexString } from 'ethereumjs-util';
22
import { Interface } from '@ethersproject/abi';
3-
import { abiERC721, abiERC20, abiERC1155 } from '@metamask/metamask-eth-abis';
3+
import {
4+
abiERC721,
5+
abiERC20,
6+
abiERC1155,
7+
abiFiatTokenV2,
8+
} from '@metamask/metamask-eth-abis';
49
import type EthQuery from '@metamask/eth-query';
510
import log from 'loglevel';
611
import {
@@ -18,6 +23,7 @@ const INFERRABLE_TRANSACTION_TYPES: TransactionType[] = [
1823
TransactionType.tokenMethodSetApprovalForAll,
1924
TransactionType.tokenMethodTransfer,
2025
TransactionType.tokenMethodTransferFrom,
26+
TransactionType.tokenMethodIncreaseAllowance,
2127
TransactionType.contractInteraction,
2228
TransactionType.simpleSend,
2329
];
@@ -32,6 +38,7 @@ type InferTransactionTypeResult = {
3238
const erc20Interface = new Interface(abiERC20);
3339
const erc721Interface = new Interface(abiERC721);
3440
const erc1155Interface = new Interface(abiERC1155);
41+
const USDCInterface = new Interface(abiFiatTokenV2);
3542

3643
/**
3744
* Determines if the maxFeePerGas and maxPriorityFeePerGas fields are supplied
@@ -116,6 +123,12 @@ export function parseStandardTokenTransactionData(data: string) {
116123
// ignore and return undefined
117124
}
118125

126+
try {
127+
return USDCInterface.parseTransaction({ data });
128+
} catch {
129+
// ignore and return undefined
130+
}
131+
119132
return undefined;
120133
}
121134

@@ -169,6 +182,7 @@ export async function determineTransactionType(
169182
TransactionType.tokenMethodSetApprovalForAll,
170183
TransactionType.tokenMethodTransfer,
171184
TransactionType.tokenMethodTransferFrom,
185+
TransactionType.tokenMethodIncreaseAllowance,
172186
TransactionType.tokenMethodSafeTransferFrom,
173187
].find((methodName) => isEqualCaseInsensitive(methodName, name));
174188
return {
@@ -227,6 +241,7 @@ export async function determineTransactionAssetType(
227241
TransactionType.tokenMethodSetApprovalForAll,
228242
TransactionType.tokenMethodTransfer,
229243
TransactionType.tokenMethodTransferFrom,
244+
TransactionType.tokenMethodIncreaseAllowance,
230245
].find((methodName) => methodName === inferrableType);
231246

232247
if (

test/e2e/helpers.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,9 @@ const PRIVATE_KEY =
695695
const PRIVATE_KEY_TWO =
696696
'0xa444f52ea41e3a39586d7069cb8e8233e9f6b9dea9cbb700cce69ae860661cc8';
697697

698+
const ACCOUNT_1 = '0x5cfe73b6021e818b776b421b1c4db2474086a7e1';
699+
const ACCOUNT_2 = '0x09781764c08de8ca82e156bbf156a3ca217c7950';
700+
698701
const defaultGanacheOptions = {
699702
accounts: [{ secretKey: PRIVATE_KEY, balance: convertETHToHexGwei(25) }],
700703
};
@@ -1090,6 +1093,8 @@ module.exports = {
10901093
TEST_SEED_PHRASE_TWO,
10911094
PRIVATE_KEY,
10921095
PRIVATE_KEY_TWO,
1096+
ACCOUNT_1,
1097+
ACCOUNT_2,
10931098
getWindowHandles,
10941099
convertToHexValue,
10951100
tinyDelayMs,

test/e2e/tests/tokens/custom-token-add-approve.spec.js

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ describe('Create token, approve token and approve token without gas', function (
238238
text: 'View details',
239239
css: '.token-allowance-container__view-details',
240240
});
241+
241242
await driver.clickElement({
242243
text: 'Next',
243244
tag: 'button',
@@ -253,13 +254,6 @@ describe('Create token, approve token and approve token without gas', function (
253254
'5 TST',
254255
'Default value is not correctly set',
255256
);
256-
await driver.waitForSelector(
257-
{
258-
css: '.box--flex-direction-row > h6',
259-
text: '0.000895 ETH',
260-
},
261-
{ timeout: 15000 },
262-
);
263257

264258
// editing gas fee
265259
const editBtn = await driver.findElement({
@@ -269,17 +263,9 @@ describe('Create token, approve token and approve token without gas', function (
269263

270264
editBtn.click();
271265

272-
await driver.clickElement({
273-
text: 'Edit suggested gas fee',
274-
tag: 'button',
275-
});
276-
277266
await driver.waitForSelector({
278267
text: 'Edit priority',
279-
});
280-
await driver.waitForSelector({
281-
text: '0.00089526 ETH',
282-
tag: 'h1',
268+
tag: 'header',
283269
});
284270

285271
await editGasFeeForm(driver, '60001', '10');

test/e2e/tests/tokens/custom-token-send-transfer.spec.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,6 @@ describe('Transfer custom tokens @no-mmi', function () {
128128

129129
// edit gas fee
130130
await driver.clickElement({ text: 'Edit', tag: 'button' });
131-
await driver.clickElement(
132-
{ text: 'Edit suggested gas fee', tag: 'button' },
133-
10000,
134-
);
135131
await editGasFeeForm(driver, '60000', '10');
136132
await driver.clickElement({ text: 'Confirm', tag: 'button' });
137133

0 commit comments

Comments
 (0)