From 0e822767d90ce5ba5fecd0e6a6bb835bcc8e5960 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 26 Sep 2024 09:29:18 +0100 Subject: [PATCH] feat: ERC20 Revoke Allowance (#26906) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Includes e2e test. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26906?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3004 ## **Manual testing steps** 1. Go to https://etherscan.io/token/0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599#writeContract 2. Connect your wallet 3. Go to approve 4. Input an address under spender 5. Input 0 under value 6. Click write 7. Notice MM confirmation ## **Screenshots/Recordings** ### **Before** ### **After** Screenshot 2024-09-04 at 17 00 32 ## **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/develop/.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/develop/.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. --------- Co-authored-by: Priya Narayanaswamy --- app/_locales/en/messages.json | 6 + .../erc20-approve-redesign.spec.ts | 5 +- .../increase-token-allowance-redesign.spec.ts | 39 +++---- .../revoke-allowance-redesign.spec.ts | 110 ++++++++++++++++++ .../__snapshots__/approve.test.tsx.snap | 4 +- .../approve-static-simulation.tsx | 4 +- .../confirm/info/approve/approve.test.tsx | 8 ++ .../confirm/info/approve/approve.tsx | 34 +++++- .../edit-spending-cap-modal.test.tsx | 21 ++++ .../hooks/use-approve-token-simulation.ts | 24 +++- .../approve/revoke-details/revoke-details.tsx | 11 ++ .../revoke-static-simulation.tsx | 60 ++++++++++ .../spending-cap/spending-cap.test.tsx | 8 ++ .../title/hooks/useCurrentSpendingCap.test.ts | 23 ++++ .../title/hooks/useCurrentSpendingCap.ts | 49 ++++++++ .../components/confirm/title/title.test.tsx | 16 +++ .../components/confirm/title/title.tsx | 19 ++- ui/pages/confirmations/confirm/confirm.tsx | 4 +- .../confirmations/hooks/useAssetDetails.js | 8 ++ 19 files changed, 414 insertions(+), 39 deletions(-) create mode 100644 test/e2e/tests/confirmations/transactions/revoke-allowance-redesign.spec.ts create mode 100644 ui/pages/confirmations/components/confirm/info/approve/revoke-details/revoke-details.tsx create mode 100644 ui/pages/confirmations/components/confirm/info/approve/revoke-static-simulation/revoke-static-simulation.tsx create mode 100644 ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts create mode 100644 ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.ts diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index e312be4794e5..80f9d2090814 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1068,6 +1068,9 @@ "confirmTitlePermitTokens": { "message": "Spending cap request" }, + "confirmTitleRevokeApproveTransaction": { + "message": "Remove permission" + }, "confirmTitleSIWESignature": { "message": "Sign-in request" }, @@ -4513,6 +4516,9 @@ "revokePermission": { "message": "Revoke permission" }, + "revokeSimulationDetailsDesc": { + "message": "You're removing someone's permission to spend tokens from your account." + }, "revokeSpendingCap": { "message": "Revoke spending cap for your $1", "description": "$1 is a token symbol" diff --git a/test/e2e/tests/confirmations/transactions/erc20-approve-redesign.spec.ts b/test/e2e/tests/confirmations/transactions/erc20-approve-redesign.spec.ts index 60a141144833..baa3638330b6 100644 --- a/test/e2e/tests/confirmations/transactions/erc20-approve-redesign.spec.ts +++ b/test/e2e/tests/confirmations/transactions/erc20-approve-redesign.spec.ts @@ -87,9 +87,10 @@ describe('Confirmation Redesign ERC20 Approve Component', function () { }); }); -async function mocked4Bytes(mockServer: MockttpServer) { +export async function mocked4BytesApprove(mockServer: MockttpServer) { return await mockServer .forGet('https://www.4byte.directory/api/v1/signatures/') + .always() .withQuery({ hex_signature: '0x095ea7b3' }) .thenCallback(() => ({ statusCode: 200, @@ -111,7 +112,7 @@ async function mocked4Bytes(mockServer: MockttpServer) { } async function mocks(server: MockttpServer) { - return [await mocked4Bytes(server)]; + return [await mocked4BytesApprove(server)]; } export async function importTST(driver: Driver) { diff --git a/test/e2e/tests/confirmations/transactions/increase-token-allowance-redesign.spec.ts b/test/e2e/tests/confirmations/transactions/increase-token-allowance-redesign.spec.ts index 2571a69107b3..4eed23b20f44 100644 --- a/test/e2e/tests/confirmations/transactions/increase-token-allowance-redesign.spec.ts +++ b/test/e2e/tests/confirmations/transactions/increase-token-allowance-redesign.spec.ts @@ -108,11 +108,27 @@ function generateFixtureOptionsForEIP1559Tx(mochaContext: Mocha.Context) { }; } +async function createAndAssertIncreaseAllowanceSubmission( + driver: Driver, + newSpendingCap: string, + contractRegistry?: GanacheContractAddressRegistry, +) { + await openDAppWithContract(driver, contractRegistry, SMART_CONTRACTS.HST); + + await createERC20IncreaseAllowanceTransaction(driver); + + await editSpendingCap(driver, newSpendingCap); + + await scrollAndConfirmAndAssertConfirm(driver); + + await assertChangedSpendingCap(driver, newSpendingCap); +} + async function mocks(server: Mockttp) { return [await mocked4BytesIncreaseAllowance(server)]; } -async function mocked4BytesIncreaseAllowance(mockServer: Mockttp) { +export async function mocked4BytesIncreaseAllowance(mockServer: Mockttp) { return await mockServer .forGet('https://www.4byte.directory/api/v1/signatures/') .always() @@ -131,7 +147,6 @@ async function mocked4BytesIncreaseAllowance(mockServer: Mockttp) { text_signature: 'increaseAllowance(address,uint256)', hex_signature: '0x39509351', bytes_signature: '9P“Q', - test: 'Priya', }, ], }, @@ -139,28 +154,12 @@ async function mocked4BytesIncreaseAllowance(mockServer: Mockttp) { }); } -async function createAndAssertIncreaseAllowanceSubmission( - driver: Driver, - newSpendingCap: string, - contractRegistry?: GanacheContractAddressRegistry, -) { - await openDAppWithContract(driver, contractRegistry, SMART_CONTRACTS.HST); - - await createERC20IncreaseAllowanceTransaction(driver); - - await editSpendingCap(driver, newSpendingCap); - - await scrollAndConfirmAndAssertConfirm(driver); - - await assertChangedSpendingCap(driver, newSpendingCap); -} - async function createERC20IncreaseAllowanceTransaction(driver: Driver) { await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); await driver.clickElement('#increaseTokenAllowance'); } -async function editSpendingCap(driver: Driver, newSpendingCap: string) { +export async function editSpendingCap(driver: Driver, newSpendingCap: string) { await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); await driver.clickElement('[data-testid="edit-spending-cap-icon"'); @@ -177,7 +176,7 @@ async function editSpendingCap(driver: Driver, newSpendingCap: string) { await driver.delay(veryLargeDelayMs * 2); } -async function assertChangedSpendingCap( +export async function assertChangedSpendingCap( driver: Driver, newSpendingCap: string, ) { diff --git a/test/e2e/tests/confirmations/transactions/revoke-allowance-redesign.spec.ts b/test/e2e/tests/confirmations/transactions/revoke-allowance-redesign.spec.ts new file mode 100644 index 000000000000..ba97d9cda4cd --- /dev/null +++ b/test/e2e/tests/confirmations/transactions/revoke-allowance-redesign.spec.ts @@ -0,0 +1,110 @@ +/* eslint-disable @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires */ +import { MockttpServer } from 'mockttp'; +import { WINDOW_TITLES } from '../../../helpers'; +import { Driver } from '../../../webdriver/driver'; +import { scrollAndConfirmAndAssertConfirm } from '../helpers'; +import { mocked4BytesApprove } from './erc20-approve-redesign.spec'; +import { + assertChangedSpendingCap, + editSpendingCap, +} from './increase-token-allowance-redesign.spec'; +import { openDAppWithContract, TestSuiteArguments } from './shared'; + +const { + defaultGanacheOptions, + defaultGanacheOptionsForType2Transactions, + withFixtures, +} = require('../../../helpers'); +const FixtureBuilder = require('../../../fixture-builder'); +const { SMART_CONTRACTS } = require('../../../seeder/smart-contracts'); + +describe('Confirmation Redesign ERC20 Revoke Allowance', function () { + const smartContract = SMART_CONTRACTS.HST; + + describe('Submit an revoke transaction @no-mmi', function () { + it('Sends a type 0 transaction (Legacy)', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withPermissionControllerConnectedToTestDapp() + .withPreferencesController({ + preferences: { + redesignedConfirmationsEnabled: true, + isRedesignedConfirmationsDeveloperEnabled: true, + }, + }) + .build(), + ganacheOptions: defaultGanacheOptions, + smartContract, + testSpecificMock: mocks, + title: this.test?.fullTitle(), + }, + async ({ driver, contractRegistry }: TestSuiteArguments) => { + await openDAppWithContract(driver, contractRegistry, smartContract); + + await createERC20ApproveTransaction(driver); + + const NEW_SPENDING_CAP = '0'; + await editSpendingCap(driver, NEW_SPENDING_CAP); + + await driver.waitForSelector({ + css: 'h2', + text: 'Remove permission', + }); + + await scrollAndConfirmAndAssertConfirm(driver); + + await assertChangedSpendingCap(driver, NEW_SPENDING_CAP); + }, + ); + }); + + it('Sends a type 2 transaction (EIP1559)', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withPermissionControllerConnectedToTestDapp() + .withPreferencesController({ + preferences: { + redesignedConfirmationsEnabled: true, + isRedesignedConfirmationsDeveloperEnabled: true, + }, + }) + .build(), + ganacheOptions: defaultGanacheOptionsForType2Transactions, + smartContract, + testSpecificMock: mocks, + title: this.test?.fullTitle(), + }, + async ({ driver, contractRegistry }: TestSuiteArguments) => { + await openDAppWithContract(driver, contractRegistry, smartContract); + + await createERC20ApproveTransaction(driver); + + const NEW_SPENDING_CAP = '0'; + await editSpendingCap(driver, NEW_SPENDING_CAP); + + await driver.waitForSelector({ + css: 'h2', + text: 'Remove permission', + }); + + await scrollAndConfirmAndAssertConfirm(driver); + + await assertChangedSpendingCap(driver, NEW_SPENDING_CAP); + }, + ); + }); + }); +}); + +async function mocks(server: MockttpServer) { + return [await mocked4BytesApprove(server)]; +} + +async function createERC20ApproveTransaction(driver: Driver) { + await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); + await driver.clickElement('#approveTokens'); +} diff --git a/ui/pages/confirmations/components/confirm/info/approve/__snapshots__/approve.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/approve/__snapshots__/approve.test.tsx.snap index 648ecff92c1a..9e7ff1b8db31 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/__snapshots__/approve.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/info/approve/__snapshots__/approve.test.tsx.snap @@ -80,7 +80,7 @@ exports[` renders component for approve request 1`] = ` class="mm-box mm-text mm-text--body-md mm-text--text-align-center mm-box--padding-inline-2 mm-box--align-items-center mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-xl" data-testid="simulation-token-value" > - 0 + 1000

@@ -414,7 +414,7 @@ exports[` renders component for approve request 1`] = ` class="mm-box mm-text mm-text--body-md mm-box--color-inherit" style="white-space: pre-wrap;" > - 0 + 1000