feat: ERC20 Revoke Allowance (#26906)

## **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: MetaMask/MetaMask-planning#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**  <img width="472" alt="Screenshot 2024-09-04 at 17 00 32" src="https://github.com/user-attachments/assets/6520ca14-068b-4fff-a4e3-7783c5fac60b"> ## **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 <[email protected]>
MetaMask · Sep 26, 2024 · 0e82276 · 0e82276
1 parent 406346f
commit 0e82276
Show file tree

Hide file tree

Showing 19 changed files with 414 additions and 39 deletions.
diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json
diff --git 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
@@ -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,36 +147,19 @@ async function mocked4BytesIncreaseAllowance(mockServer: Mockttp) {
               text_signature: 'increaseAllowance(address,uint256)',
               hex_signature: '0x39509351',
               bytes_signature: '9PQ',
-              test: 'Priya',
             },
           ],
         },
       };
     });
 }
 
-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
@@ -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
@@ -80,7 +80,7 @@ exports[`<ApproveInfo /> 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
             </p>
           </div>
           <div>
@@ -414,7 +414,7 @@ exports[`<ApproveInfo /> 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 
         </p>
         <button
           aria-label="Edit"

diff --git a/...s/components/confirm/info/approve/approve-static-simulation/approve-static-simulation.tsx b/...s/components/confirm/info/approve/approve-static-simulation/approve-static-simulation.tsx
@@ -65,7 +65,7 @@ export const ApproveStaticSimulation = () => {
     </Text>
   );
 
-  const simulationElements = (
+  const SpendingCapRow = (
     <ConfirmInfoRow
       label={t(isNFT ? 'simulationApproveHeading' : 'spendingCap')}
     >
@@ -93,6 +93,8 @@ export const ApproveStaticSimulation = () => {
     </ConfirmInfoRow>
   );
 
+  const simulationElements = SpendingCapRow;
+
   return (
     <StaticSimulation
       title={t('simulationDetailsTitle')}

diff --git a/ui/pages/confirmations/components/confirm/info/approve/approve.test.tsx b/ui/pages/confirmations/components/confirm/info/approve/approve.test.tsx
@@ -22,6 +22,14 @@ jest.mock(
   }),
 );
 
+jest.mock('./hooks/use-approve-token-simulation', () => ({
+  useApproveTokenSimulation: jest.fn(() => ({
+    spendingCap: '1000',
+    formattedSpendingCap: '1000',
+    value: '1000',
+  })),
+}));
+
 jest.mock('../../../../hooks/useAssetDetails', () => ({
   useAssetDetails: jest.fn(() => ({
     decimals: 18,

diff --git a/ui/pages/confirmations/components/confirm/info/approve/approve.tsx b/ui/pages/confirmations/components/confirm/info/approve/approve.tsx
@@ -1,14 +1,21 @@
-import { TransactionMeta } from '@metamask/transaction-controller';
+import {
+  TransactionMeta,
+  TransactionType,
+} from '@metamask/transaction-controller';
 import React, { useState } from 'react';
 import { useSelector } from 'react-redux';
 import { useConfirmContext } from '../../../../context/confirm';
+import { useAssetDetails } from '../../../../hooks/useAssetDetails';
 import { selectConfirmationAdvancedDetailsOpen } from '../../../../selectors/preferences';
 import { AdvancedDetails } from '../shared/advanced-details/advanced-details';
 import { GasFeesSection } from '../shared/gas-fees-section/gas-fees-section';
 import { ApproveDetails } from './approve-details/approve-details';
 import { ApproveStaticSimulation } from './approve-static-simulation/approve-static-simulation';
 import { EditSpendingCapModal } from './edit-spending-cap-modal/edit-spending-cap-modal';
+import { useApproveTokenSimulation } from './hooks/use-approve-token-simulation';
 import { useIsNFT } from './hooks/use-is-nft';
+import { RevokeDetails } from './revoke-details/revoke-details';
+import { RevokeStaticSimulation } from './revoke-static-simulation/revoke-static-simulation';
 import { SpendingCap } from './spending-cap/spending-cap';
 
 const ApproveInfo = () => {
@@ -25,15 +32,34 @@ const ApproveInfo = () => {
   const [isOpenEditSpendingCapModal, setIsOpenEditSpendingCapModal] =
     useState(false);
 
+  const { decimals } = useAssetDetails(
+    transactionMeta.txParams.to,
+    transactionMeta.txParams.from,
+    transactionMeta.txParams.data,
+  );
+
+  const { spendingCap } = useApproveTokenSimulation(
+    transactionMeta,
+    decimals || '0',
+  );
+
+  const showRevokeVariant =
+    spendingCap === '0' &&
+    transactionMeta.type === TransactionType.tokenMethodApprove;
+
   if (!transactionMeta?.txParams) {
     return null;
   }
 
   return (
     <>
-      <ApproveStaticSimulation />
-      <ApproveDetails />
-      {!isNFT && (
+      {showRevokeVariant ? (
+        <RevokeStaticSimulation />
+      ) : (
+        <ApproveStaticSimulation />
+      )}
+      {showRevokeVariant ? <RevokeDetails /> : <ApproveDetails />}
+      {!isNFT && !showRevokeVariant && (
         <SpendingCap
           setIsOpenEditSpendingCapModal={setIsOpenEditSpendingCapModal}
         />

diff --git a/.../components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.test.tsx b/.../components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.test.tsx
@@ -26,6 +26,27 @@ jest.mock('../hooks/use-approve-token-simulation', () => ({
   })),
 }));
 
+jest.mock('react-dom', () => ({
+  ...jest.requireActual('react-dom'),
+  createPortal: (node: ReactNode) => node,
+}));
+
+jest.mock('../../../../../../../store/actions', () => ({
+  ...jest.requireActual('../../../../../../../store/actions'),
+  getGasFeeTimeEstimate: jest.fn().mockResolvedValue({
+    lowerTimeBound: 0,
+    upperTimeBound: 60000,
+  }),
+}));
+
+jest.mock('../hooks/use-approve-token-simulation', () => ({
+  useApproveTokenSimulation: jest.fn(() => ({
+    spendingCap: '1000',
+    formattedSpendingCap: '1000',
+    value: '1000',
+  })),
+}));
+
 jest.mock('../../../../../hooks/useAssetDetails', () => ({
   useAssetDetails: jest.fn(() => ({
     decimals: 18,