Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ERC20 Revoke Allowance #26906

Merged
merged 67 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
fb4449d
feat: Redesign Approve confirmation
pedronfigueiredo Aug 15, 2024
01e792c
feat: Redesign Approve confirmation
pedronfigueiredo Aug 16, 2024
5467758
wip
pedronfigueiredo Aug 19, 2024
a25fcb2
address feedback
pedronfigueiredo Aug 19, 2024
78a1c13
feat: Add approval static simulation
pedronfigueiredo Aug 19, 2024
67818a5
Add redesigned approve and increase allowance confirmations
pedronfigueiredo Aug 20, 2024
824bc38
wip
pedronfigueiredo Aug 22, 2024
c6dbb36
fix linting errors
pedronfigueiredo Aug 22, 2024
93e2db0
fix integration tests
pedronfigueiredo Aug 22, 2024
af3d07e
update snapshot
pedronfigueiredo Aug 22, 2024
90400ae
Fix linting errors in tests
pedronfigueiredo Aug 22, 2024
0517483
Addressing PR comments
pedronfigueiredo Aug 23, 2024
ab4aaf2
Addressing PR comments
pedronfigueiredo Aug 23, 2024
f0fc7bb
fix linting issues
pedronfigueiredo Aug 26, 2024
19eda2a
Update logic for rendering spending cap component
pedronfigueiredo Aug 27, 2024
fa72473
fix linting error
pedronfigueiredo Aug 27, 2024
8e12357
wip
pedronfigueiredo Aug 27, 2024
0eeac81
update modal
pedronfigueiredo Aug 29, 2024
2012dfa
split components into multiple files
pedronfigueiredo Aug 29, 2024
98cfa78
Implement spending cap editing
pedronfigueiredo Aug 30, 2024
296aa18
Fix rerendering bug
pedronfigueiredo Aug 30, 2024
4476bb9
E2e test for increase allowance
pedronfigueiredo Aug 30, 2024
5e07f3e
fix test
pedronfigueiredo Aug 30, 2024
277aa38
Remove unused hook
pedronfigueiredo Sep 2, 2024
80499fd
Recalculate gas fees on large spending cap
pedronfigueiredo Sep 2, 2024
9fffb2a
unit tests
pedronfigueiredo Sep 2, 2024
ad05bc4
Unit tests
pedronfigueiredo Sep 3, 2024
c288289
fix unit tests
pedronfigueiredo Sep 3, 2024
a5ddf00
fix linting errors
pedronfigueiredo Sep 3, 2024
577d46f
Update snapshot
pedronfigueiredo Sep 4, 2024
1295032
Address review feedback
pedronfigueiredo Sep 4, 2024
851a998
Fix lint errors
pedronfigueiredo Sep 4, 2024
34480e9
Fix lint errors
pedronfigueiredo Sep 4, 2024
b811322
Move update logic to edit spending cap modal
pedronfigueiredo Sep 4, 2024
13f1207
wip
pedronfigueiredo Sep 4, 2024
dfbe761
test
pedronfigueiredo Sep 5, 2024
f4ab8ad
test
pedronfigueiredo Sep 5, 2024
c59dbc1
test
pedronfigueiredo Sep 5, 2024
4e0583a
test: add extra screenshots
pnarayanaswamy Sep 5, 2024
1f53228
test: add always to mock server
pnarayanaswamy Sep 6, 2024
7f5d5d8
update tx data in the controller
pedronfigueiredo Sep 6, 2024
9eb6b1b
clean up
pedronfigueiredo Sep 6, 2024
99c2891
Mock react portal
pedronfigueiredo Sep 6, 2024
5dbbb0f
Move dispatch into the callback
pedronfigueiredo Sep 10, 2024
24c0e45
Add loading indicator to the spending cap modal
pedronfigueiredo Sep 10, 2024
e9c139b
wip
pedronfigueiredo Sep 11, 2024
2e34369
feat: ERC20 Revoke Allowance
pedronfigueiredo Sep 4, 2024
56dec9c
wip
pedronfigueiredo Sep 5, 2024
d041600
Address feedback
pedronfigueiredo Sep 5, 2024
5233b45
Update revoke flow logic
pedronfigueiredo Sep 6, 2024
957cb65
fix integration test
pedronfigueiredo Sep 6, 2024
1952658
fix unit tests
pedronfigueiredo Sep 9, 2024
1932ac5
Fix spending cap decoding
pedronfigueiredo Sep 9, 2024
59827eb
fix linting error
pedronfigueiredo Sep 10, 2024
60a149a
Fix linting errors
pedronfigueiredo Sep 16, 2024
7931f3d
Fix hook
pedronfigueiredo Sep 16, 2024
1b8bb2b
tweak heading text logic
pedronfigueiredo Sep 16, 2024
74d7ee3
Extract title.tsx spending cap logic into custom hook
pedronfigueiredo Sep 17, 2024
50b74b9
Fix test
pedronfigueiredo Sep 19, 2024
6386051
Revert unneeded changes
pedronfigueiredo Sep 19, 2024
b85d7bb
generalize value param lookup in hook
pedronfigueiredo Sep 20, 2024
7d08652
Fix lint and test errors after rebase
pedronfigueiredo Sep 24, 2024
73bc748
wip
pedronfigueiredo Sep 24, 2024
dfeede7
wip
pedronfigueiredo Sep 25, 2024
c4be88b
wip
pedronfigueiredo Sep 25, 2024
bf919af
wip
pedronfigueiredo Sep 25, 2024
181af3d
wip
pedronfigueiredo Sep 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 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.

Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -131,36 +147,19 @@ async function mocked4BytesIncreaseAllowance(mockServer: Mockttp) {
text_signature: 'increaseAllowance(address,uint256)',
hex_signature: '0x39509351',
bytes_signature: '9P“Q',
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"');

Expand All @@ -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,
) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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');
}
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export const ApproveStaticSimulation = () => {
</Text>
);

const simulationElements = (
const SpendingCapRow = (
<ConfirmInfoRow
label={t(isNFT ? 'simulationApproveHeading' : 'spendingCap')}
>
Expand Down Expand Up @@ -93,6 +93,8 @@ export const ApproveStaticSimulation = () => {
</ConfirmInfoRow>
);

const simulationElements = SpendingCapRow;

return (
<StaticSimulation
title={t('simulationDetailsTitle')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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 = () => {
Expand All @@ -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}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading