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: Redesigned setApprovalForAll confirmations #27061

Merged
merged 14 commits into from
Sep 24, 2024
Merged
12 changes: 12 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.

3 changes: 0 additions & 3 deletions app/_locales/en_GB/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions sonar-project.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ sonar.projectKey=metamask-extension
sonar.organization=consensys

# Source
sonar.sources=app,development,offscreen,shared,test,types,ui
sonar.exclusions=**/*.test.**,**/*.spec.**,app/images,test/**
sonar.sources=app,development,offscreen,shared,types,ui
sonar.exclusions=**/*.test.**,**/*.spec.**,app/images,test/e2e/page-objects,test/data


# Tests
sonar.tests=app,development,offscreen,shared,test,types,ui
Expand Down
20 changes: 20 additions & 0 deletions test/data/confirmations/contract-interaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,23 @@ export const genUnapprovedApproveConfirmation = ({
},
type: TransactionType.tokenMethodApprove,
});

export const genUnapprovedSetApprovalForAllConfirmation = ({
address = CONTRACT_INTERACTION_SENDER_ADDRESS,
chainId = CHAIN_ID,
}: {
address?: Hex;
chainId?: string;
} = {}) => ({
...genUnapprovedContractInteractionConfirmation({ chainId }),
txParams: {
from: address,
data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001',
gas: '0x16a92',
to: '0x076146c765189d51be3160a2140cf80bfc73ad68',
value: '0x0',
maxFeePerGas: '0x5b06b0c0d',
maxPriorityFeePerGas: '0x59682f00',
},
type: TransactionType.tokenMethodSetApprovalForAll,
});
7 changes: 7 additions & 0 deletions test/data/confirmations/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { CHAIN_IDS } from '../../../shared/constants/network';
import {
genUnapprovedApproveConfirmation,
genUnapprovedContractInteractionConfirmation,
genUnapprovedSetApprovalForAllConfirmation,
} from './contract-interaction';
import { unapprovedPersonalSignMsg } from './personal_sign';
import { unapprovedTypedSignMsgV4 } from './typed_sign';
Expand Down Expand Up @@ -176,3 +177,9 @@ export const getMockApproveConfirmState = () => {
genUnapprovedApproveConfirmation({ chainId: '0x5' }),
);
};

export const getMockSetApprovalForAllConfirmState = () => {
return getMockConfirmStateForTransaction(
genUnapprovedSetApprovalForAllConfirmation({ chainId: '0x5' }),
);
};
1 change: 1 addition & 0 deletions test/e2e/page-objects/common.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export type RawLocator = string | { css: string; text: string };
27 changes: 27 additions & 0 deletions test/e2e/page-objects/pages/confirmation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { Driver } from '../../webdriver/driver';
import { RawLocator } from '../common';

class Confirmation {
protected driver: Driver;

private scrollToBottomButton: RawLocator;

private footerConfirmButton: RawLocator;

constructor(driver: Driver) {
this.driver = driver;

this.scrollToBottomButton = '.confirm-scroll-to-bottom__button';
this.footerConfirmButton = '[data-testid="confirm-footer-button"]';
}

async clickScrollToBottomButton() {
await this.driver.clickElementSafe(this.scrollToBottomButton);
}

async clickFooterConfirmButton() {
await this.driver.clickElement(this.footerConfirmButton);
}
}

export default Confirmation;
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { tEn } from '../../../lib/i18n-helpers';
import { Driver } from '../../webdriver/driver';
import { RawLocator } from '../common';
import TransactionConfirmation from './transaction-confirmation';

class SetApprovalForAllTransactionConfirmation extends TransactionConfirmation {
private setApprovalForAllTitleElement: RawLocator;

private setApprovalForAllSubHeadingElement: RawLocator;

constructor(driver: Driver) {
super(driver);

this.driver = driver;

this.setApprovalForAllTitleElement = {
css: 'h2',
text: tEn('setApprovalForAllRedesignedTitle') as string,
};
this.setApprovalForAllSubHeadingElement = {
css: 'p',
text: tEn('confirmTitleDescApproveTransaction') as string,
};
}

async check_setApprovalForAllTitle() {
await this.driver.waitForSelector(this.setApprovalForAllTitleElement);
}

async check_setApprovalForAllSubHeading() {
await this.driver.waitForSelector(this.setApprovalForAllSubHeadingElement);
}
}

export default SetApprovalForAllTransactionConfirmation;
16 changes: 16 additions & 0 deletions test/e2e/page-objects/pages/test-dapp.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
import { Driver } from '../../webdriver/driver';
import { RawLocator } from '../common';

const DAPP_HOST_ADDRESS = '127.0.0.1:8080';
const DAPP_URL = `http://${DAPP_HOST_ADDRESS}`;

class TestDapp {
private driver: Driver;

private erc721SetApprovalForAllButton: RawLocator;

private erc1155SetApprovalForAllButton: RawLocator;

constructor(driver: Driver) {
this.driver = driver;

this.erc721SetApprovalForAllButton = '#setApprovalForAllButton';
this.erc1155SetApprovalForAllButton = '#setApprovalForAllERC1155Button';
}

async open({
Expand All @@ -32,6 +40,14 @@ class TestDapp {
)}`,
});
}

async clickERC721SetApprovalForAllButton() {
await this.driver.clickElement(this.erc721SetApprovalForAllButton);
}

async clickERC1155SetApprovalForAllButton() {
await this.driver.clickElement(this.erc1155SetApprovalForAllButton);
}
}

export default TestDapp;
5 changes: 5 additions & 0 deletions test/e2e/page-objects/pages/transaction-confirmation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import Confirmation from './confirmation';

class TransactionConfirmation extends Confirmation {}

export default TransactionConfirmation;
25 changes: 18 additions & 7 deletions test/e2e/tests/confirmations/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { TransactionEnvelopeType } from '@metamask/transaction-controller';
import FixtureBuilder from '../../fixture-builder';
import { defaultGanacheOptions, withFixtures } from '../../helpers';
import {
defaultGanacheOptions,
defaultGanacheOptionsForType2Transactions,
withFixtures,
} from '../../helpers';
import { MockedEndpoint, Mockttp } from '../../mock-e2e';
import { SMART_CONTRACTS } from '../../seeder/smart-contracts';
import { Driver } from '../../webdriver/driver';

export async function scrollAndConfirmAndAssertConfirm(driver: Driver) {
Expand All @@ -14,15 +20,15 @@ export function withRedesignConfirmationFixtures(
// title. It's optional because it's sometimes unset.
// eslint-disable-next-line @typescript-eslint/default-param-last
title: string = '',
transactionEnvelopeType: TransactionEnvelopeType,
testFunction: Parameters<typeof withFixtures>[1],
mockSegment?: (mockServer: Mockttp) => Promise<MockedEndpoint[]>, // Add mockSegment as an optional parameter
mocks?: (mockServer: Mockttp) => Promise<MockedEndpoint[]>, // Add mocks as an optional parameter
smartContract?: typeof SMART_CONTRACTS,
) {
return withFixtures(
{
dapp: true,
driverOptions: {
timeOut: 20000,
},
driverOptions: { timeOut: 20000 },
fixtures: new FixtureBuilder()
.withPermissionControllerConnectedToTestDapp()
.withMetaMetricsController({
Expand All @@ -32,12 +38,17 @@ export function withRedesignConfirmationFixtures(
.withPreferencesController({
preferences: {
redesignedConfirmationsEnabled: true,
isRedesignedConfirmationsDeveloperEnabled: true,
},
})
.build(),
ganacheOptions: defaultGanacheOptions,
ganacheOptions:
transactionEnvelopeType === TransactionEnvelopeType.legacy
? defaultGanacheOptions
: defaultGanacheOptionsForType2Transactions,
smartContract,
testSpecificMock: mocks,
title,
testSpecificMock: mockSegment,
},
testFunction,
);
Expand Down
6 changes: 5 additions & 1 deletion test/e2e/tests/confirmations/navigation.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { strict as assert } from 'assert';
import { TransactionEnvelopeType } from '@metamask/transaction-controller';
import { Suite } from 'mocha';
import {
DAPP_HOST_ADDRESS,
WINDOW_TITLES,
openDapp,
unlockWallet,
regularDelayMs,
unlockWallet,
} from '../../helpers';
import { Driver } from '../../webdriver/driver';
import { withRedesignConfirmationFixtures } from './helpers';
Expand All @@ -14,6 +15,7 @@ describe('Navigation Signature - Different signature types', function (this: Sui
it('initiates and queues multiple signatures and confirms', async function () {
await withRedesignConfirmationFixtures(
this.test?.fullTitle(),
TransactionEnvelopeType.legacy,
async ({ driver }: { driver: Driver }) => {
await unlockWallet(driver);
await openDapp(driver);
Expand Down Expand Up @@ -53,6 +55,7 @@ describe('Navigation Signature - Different signature types', function (this: Sui
it('initiates and queues a mix of signatures and transactions and navigates', async function () {
await withRedesignConfirmationFixtures(
this.test?.fullTitle(),
TransactionEnvelopeType.legacy,
async ({ driver }: { driver: Driver }) => {
await unlockWallet(driver);
await openDapp(driver);
Expand Down Expand Up @@ -90,6 +93,7 @@ describe('Navigation Signature - Different signature types', function (this: Sui
it('initiates multiple signatures and rejects all', async function () {
await withRedesignConfirmationFixtures(
this.test?.fullTitle(),
TransactionEnvelopeType.legacy,
async ({ driver }: { driver: Driver }) => {
await unlockWallet(driver);
await openDapp(driver);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { strict as assert } from 'assert';
import { TransactionEnvelopeType } from '@metamask/transaction-controller';
import { Suite } from 'mocha';
import { MockedEndpoint } from 'mockttp';
import { WINDOW_TITLES } from '../../../helpers';
Expand All @@ -19,6 +20,7 @@ describe('Malicious Confirmation Signature - Bad Domain @no-mmi', function (this
it('displays alert for domain binding and confirms', async function () {
await withRedesignConfirmationFixtures(
this.test?.fullTitle(),
TransactionEnvelopeType.legacy,
async ({ driver }: TestSuiteArguments) => {
await openDappAndTriggerSignature(driver, SignatureType.SIWE_BadDomain);

Expand All @@ -41,6 +43,7 @@ describe('Malicious Confirmation Signature - Bad Domain @no-mmi', function (this
it('initiates and rejects from confirmation screen', async function () {
await withRedesignConfirmationFixtures(
this.test?.fullTitle(),
TransactionEnvelopeType.legacy,
async ({
driver,
mockedEndpoint: mockedEndpoints,
Expand Down Expand Up @@ -87,6 +90,7 @@ describe('Malicious Confirmation Signature - Bad Domain @no-mmi', function (this
it('initiates and rejects from alert friction modal', async function () {
await withRedesignConfirmationFixtures(
this.test?.fullTitle(),
TransactionEnvelopeType.legacy,
async ({
driver,
mockedEndpoint: mockedEndpoints,
Expand Down
5 changes: 4 additions & 1 deletion test/e2e/tests/confirmations/signatures/permit.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { strict as assert } from 'assert';
import { TransactionEnvelopeType } from '@metamask/transaction-controller';
import { Suite } from 'mocha';
import { MockedEndpoint } from 'mockttp';
import {
DAPP_HOST_ADDRESS,
WINDOW_TITLES,
openDapp,
unlockWallet,
WINDOW_TITLES,
} from '../../../helpers';
import { Ganache } from '../../../seeder/ganache';
import { Driver } from '../../../webdriver/driver';
Expand All @@ -32,6 +33,7 @@ describe('Confirmation Signature - Permit @no-mmi', function (this: Suite) {
it('initiates and confirms and emits the correct events', async function () {
await withRedesignConfirmationFixtures(
this.test?.fullTitle(),
TransactionEnvelopeType.legacy,
async ({
driver,
ganacheServer,
Expand Down Expand Up @@ -75,6 +77,7 @@ describe('Confirmation Signature - Permit @no-mmi', function (this: Suite) {
it('initiates and rejects and emits the correct events', async function () {
await withRedesignConfirmationFixtures(
this.test?.fullTitle(),
TransactionEnvelopeType.legacy,
async ({
driver,
mockedEndpoint: mockedEndpoints,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { strict as assert } from 'assert';
import { TransactionEnvelopeType } from '@metamask/transaction-controller';
import { Suite } from 'mocha';
import { MockedEndpoint } from 'mockttp';
import { DAPP_HOST_ADDRESS, WINDOW_TITLES } from '../../../helpers';
Expand Down Expand Up @@ -26,6 +27,7 @@ describe('Confirmation Signature - Personal Sign @no-mmi', function (this: Suite
it('initiates and confirms', async function () {
await withRedesignConfirmationFixtures(
this.test?.fullTitle(),
TransactionEnvelopeType.legacy,
async ({
driver,
ganacheServer,
Expand Down Expand Up @@ -65,6 +67,7 @@ describe('Confirmation Signature - Personal Sign @no-mmi', function (this: Suite
it('initiates and rejects', async function () {
await withRedesignConfirmationFixtures(
this.test?.fullTitle(),
TransactionEnvelopeType.legacy,
async ({
driver,
mockedEndpoint: mockedEndpoints,
Expand Down
Loading