Skip to content

Commit

Permalink
feat(): Adds confirmation screen for 'increaseAllowance' (#23560)
Browse files Browse the repository at this point in the history
~**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.**~

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)

Fixes:
[#2224](MetaMask/MetaMask-planning#2224)

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)

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

<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">

<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">

- [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".

- [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.
  • Loading branch information
pedronfigueiredo committed Apr 10, 2024
1 parent 5ac9096 commit 06d20b9
Show file tree
Hide file tree
Showing 19 changed files with 380 additions and 37 deletions.
4 changes: 4 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.

1 change: 1 addition & 0 deletions app/scripts/lib/transaction/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,7 @@ async function buildEventFragmentProperties({
[
TransactionType.contractInteraction,
TransactionType.tokenMethodApprove,
TransactionType.tokenMethodIncreaseAllowance,
TransactionType.tokenMethodSafeTransferFrom,
TransactionType.tokenMethodSetApprovalForAll,
TransactionType.tokenMethodTransfer,
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@
"@metamask/logging-controller": "^2.0.2",
"@metamask/logo": "^3.1.2",
"@metamask/message-manager": "^7.3.0",
"@metamask/metamask-eth-abis": "^3.0.0",
"@metamask/metamask-eth-abis": "^3.1.1",
"@metamask/name-controller": "^4.2.0",
"@metamask/network-controller": "patch:@metamask/network-controller@npm%3A18.0.1#~/.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch",
"@metamask/notification-controller": "^3.0.0",
Expand Down Expand Up @@ -430,7 +430,7 @@
"@metamask/forwarder": "^1.1.0",
"@metamask/phishing-warning": "^3.0.3",
"@metamask/test-bundler": "^1.0.0",
"@metamask/test-dapp": "^8.3.0",
"@metamask/test-dapp": "^8.4.0",
"@playwright/test": "^1.39.0",
"@sentry/cli": "^2.19.4",
"@storybook/addon-a11y": "^7.4.6",
Expand Down
17 changes: 16 additions & 1 deletion shared/modules/transaction.utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { isHexString } from 'ethereumjs-util';
import { Interface } from '@ethersproject/abi';
import { abiERC721, abiERC20, abiERC1155 } from '@metamask/metamask-eth-abis';
import {
abiERC721,
abiERC20,
abiERC1155,
abiFiatTokenV2,
} from '@metamask/metamask-eth-abis';
import type EthQuery from '@metamask/eth-query';
import log from 'loglevel';
import {
Expand All @@ -18,6 +23,7 @@ const INFERRABLE_TRANSACTION_TYPES: TransactionType[] = [
TransactionType.tokenMethodSetApprovalForAll,
TransactionType.tokenMethodTransfer,
TransactionType.tokenMethodTransferFrom,
TransactionType.tokenMethodIncreaseAllowance,
TransactionType.contractInteraction,
TransactionType.simpleSend,
];
Expand All @@ -32,6 +38,7 @@ type InferTransactionTypeResult = {
const erc20Interface = new Interface(abiERC20);
const erc721Interface = new Interface(abiERC721);
const erc1155Interface = new Interface(abiERC1155);
const USDCInterface = new Interface(abiFiatTokenV2);

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

try {
return USDCInterface.parseTransaction({ data });
} catch {
// ignore and return undefined
}

return undefined;
}

Expand Down Expand Up @@ -169,6 +182,7 @@ export async function determineTransactionType(
TransactionType.tokenMethodSetApprovalForAll,
TransactionType.tokenMethodTransfer,
TransactionType.tokenMethodTransferFrom,
TransactionType.tokenMethodIncreaseAllowance,
TransactionType.tokenMethodSafeTransferFrom,
].find((methodName) => isEqualCaseInsensitive(methodName, name));
return {
Expand Down Expand Up @@ -227,6 +241,7 @@ export async function determineTransactionAssetType(
TransactionType.tokenMethodSetApprovalForAll,
TransactionType.tokenMethodTransfer,
TransactionType.tokenMethodTransferFrom,
TransactionType.tokenMethodIncreaseAllowance,
].find((methodName) => methodName === inferrableType);

if (
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,9 @@ const PRIVATE_KEY =
const PRIVATE_KEY_TWO =
'0xa444f52ea41e3a39586d7069cb8e8233e9f6b9dea9cbb700cce69ae860661cc8';

const ACCOUNT_1 = '0x5cfe73b6021e818b776b421b1c4db2474086a7e1';
const ACCOUNT_2 = '0x09781764c08de8ca82e156bbf156a3ca217c7950';

const defaultGanacheOptions = {
accounts: [{ secretKey: PRIVATE_KEY, balance: convertETHToHexGwei(25) }],
};
Expand Down Expand Up @@ -1090,6 +1093,8 @@ module.exports = {
TEST_SEED_PHRASE_TWO,
PRIVATE_KEY,
PRIVATE_KEY_TWO,
ACCOUNT_1,
ACCOUNT_2,
getWindowHandles,
convertToHexValue,
tinyDelayMs,
Expand Down
18 changes: 2 additions & 16 deletions test/e2e/tests/tokens/custom-token-add-approve.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ describe('Create token, approve token and approve token without gas', function (
text: 'View details',
css: '.token-allowance-container__view-details',
});

await driver.clickElement({
text: 'Next',
tag: 'button',
Expand All @@ -253,13 +254,6 @@ describe('Create token, approve token and approve token without gas', function (
'5 TST',
'Default value is not correctly set',
);
await driver.waitForSelector(
{
css: '.box--flex-direction-row > h6',
text: '0.000895 ETH',
},
{ timeout: 15000 },
);

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

editBtn.click();

await driver.clickElement({
text: 'Edit suggested gas fee',
tag: 'button',
});

await driver.waitForSelector({
text: 'Edit priority',
});
await driver.waitForSelector({
text: '0.00089526 ETH',
tag: 'h1',
tag: 'header',
});

await editGasfeeForm(driver, '60001', '10');
Expand Down
6 changes: 1 addition & 5 deletions test/e2e/tests/tokens/custom-token-send-transfer.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,7 @@ describe('Transfer custom tokens @no-mmi', function () {

// edit gas fee
await driver.clickElement({ text: 'Edit', tag: 'button' });
await driver.clickElement(
{ text: 'Edit suggested gas fee', tag: 'button' },
10000,
);
await editGasfeeForm(driver, '60000', '10');
await editGasFeeForm(driver, '60000', '10');
await driver.clickElement({ text: 'Confirm', tag: 'button' });

// in extension, check that transaction has completed correctly and is displayed in the activity list
Expand Down
Loading

0 comments on commit 06d20b9

Please sign in to comment.