From 0e10bab6bc28bb83916c14426fa0439f69903418 Mon Sep 17 00:00:00 2001 From: Alejandro Garcia Anglada Date: Tue, 17 Dec 2024 12:39:47 +0100 Subject: [PATCH 1/9] fix: nanoid audit issue (#29268) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Fixes `nanoid` audit [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29268?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.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: MetaMask Bot --- .../controllers/permissions/background-api.js | 2 +- app/scripts/metamask-controller.js | 2 +- lavamoat/browserify/beta/policy.json | 14 ++++++-------- lavamoat/browserify/flask/policy.json | 14 ++++++-------- lavamoat/browserify/main/policy.json | 14 ++++++-------- lavamoat/browserify/mmi/policy.json | 14 ++++++-------- lavamoat/build-system/policy.json | 2 +- package.json | 2 +- yarn.lock | 12 ++++++------ 9 files changed, 34 insertions(+), 42 deletions(-) diff --git a/app/scripts/controllers/permissions/background-api.js b/app/scripts/controllers/permissions/background-api.js index b778ff42385d..8a0942667f17 100644 --- a/app/scripts/controllers/permissions/background-api.js +++ b/app/scripts/controllers/permissions/background-api.js @@ -1,4 +1,4 @@ -import nanoid from 'nanoid'; +import { nanoid } from 'nanoid'; import { CaveatTypes, RestrictedMethods, diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index e3a223da4e02..8ab78f2c7e92 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -47,7 +47,7 @@ import { rawChainData } from 'eth-chainlist'; import { MetaMaskKeyring as QRHardwareKeyring } from '@keystonehq/metamask-airgapped-keyring'; import EthQuery from '@metamask/eth-query'; import EthJSQuery from '@metamask/ethjs-query'; -import nanoid from 'nanoid'; +import { nanoid } from 'nanoid'; import { captureException } from '@sentry/browser'; import { AddressBookController } from '@metamask/address-book-controller'; import { diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index bed94b6bf3c6..42c0b3ff7385 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -637,9 +637,9 @@ "console.info": true }, "packages": { - "@metamask/approval-controller>nanoid": true, "@metamask/base-controller": true, - "@metamask/rpc-errors": true + "@metamask/rpc-errors": true, + "nanoid": true } }, "@metamask/approval-controller>nanoid": { @@ -1959,11 +1959,11 @@ "@metamask/base-controller": true, "@metamask/controller-utils": true, "@metamask/json-rpc-engine": true, - "@metamask/permission-controller>nanoid": true, "@metamask/rpc-errors": true, "@metamask/utils": true, "deep-freeze-strict": true, - "immer": true + "immer": true, + "nanoid": true } }, "@metamask/permission-controller>nanoid": { @@ -2446,7 +2446,6 @@ "@metamask/snaps-controllers>@xstate/fsm": true, "@metamask/snaps-controllers>concat-stream": true, "@metamask/snaps-controllers>get-npm-tarball-url": true, - "@metamask/snaps-controllers>nanoid": true, "@metamask/snaps-controllers>readable-web-to-node-stream": true, "@metamask/snaps-controllers>tar-stream": true, "@metamask/snaps-rpc-methods": true, @@ -2457,6 +2456,7 @@ "browserify>browserify-zlib": true, "eslint>fast-deep-equal": true, "immer": true, + "nanoid": true, "readable-stream": true, "semver": true } @@ -4514,9 +4514,7 @@ }, "nanoid": { "globals": { - "crypto": true, - "msCrypto": true, - "navigator": true + "crypto.getRandomValues": true } }, "nock>debug": { diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index bed94b6bf3c6..42c0b3ff7385 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -637,9 +637,9 @@ "console.info": true }, "packages": { - "@metamask/approval-controller>nanoid": true, "@metamask/base-controller": true, - "@metamask/rpc-errors": true + "@metamask/rpc-errors": true, + "nanoid": true } }, "@metamask/approval-controller>nanoid": { @@ -1959,11 +1959,11 @@ "@metamask/base-controller": true, "@metamask/controller-utils": true, "@metamask/json-rpc-engine": true, - "@metamask/permission-controller>nanoid": true, "@metamask/rpc-errors": true, "@metamask/utils": true, "deep-freeze-strict": true, - "immer": true + "immer": true, + "nanoid": true } }, "@metamask/permission-controller>nanoid": { @@ -2446,7 +2446,6 @@ "@metamask/snaps-controllers>@xstate/fsm": true, "@metamask/snaps-controllers>concat-stream": true, "@metamask/snaps-controllers>get-npm-tarball-url": true, - "@metamask/snaps-controllers>nanoid": true, "@metamask/snaps-controllers>readable-web-to-node-stream": true, "@metamask/snaps-controllers>tar-stream": true, "@metamask/snaps-rpc-methods": true, @@ -2457,6 +2456,7 @@ "browserify>browserify-zlib": true, "eslint>fast-deep-equal": true, "immer": true, + "nanoid": true, "readable-stream": true, "semver": true } @@ -4514,9 +4514,7 @@ }, "nanoid": { "globals": { - "crypto": true, - "msCrypto": true, - "navigator": true + "crypto.getRandomValues": true } }, "nock>debug": { diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index bed94b6bf3c6..42c0b3ff7385 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -637,9 +637,9 @@ "console.info": true }, "packages": { - "@metamask/approval-controller>nanoid": true, "@metamask/base-controller": true, - "@metamask/rpc-errors": true + "@metamask/rpc-errors": true, + "nanoid": true } }, "@metamask/approval-controller>nanoid": { @@ -1959,11 +1959,11 @@ "@metamask/base-controller": true, "@metamask/controller-utils": true, "@metamask/json-rpc-engine": true, - "@metamask/permission-controller>nanoid": true, "@metamask/rpc-errors": true, "@metamask/utils": true, "deep-freeze-strict": true, - "immer": true + "immer": true, + "nanoid": true } }, "@metamask/permission-controller>nanoid": { @@ -2446,7 +2446,6 @@ "@metamask/snaps-controllers>@xstate/fsm": true, "@metamask/snaps-controllers>concat-stream": true, "@metamask/snaps-controllers>get-npm-tarball-url": true, - "@metamask/snaps-controllers>nanoid": true, "@metamask/snaps-controllers>readable-web-to-node-stream": true, "@metamask/snaps-controllers>tar-stream": true, "@metamask/snaps-rpc-methods": true, @@ -2457,6 +2456,7 @@ "browserify>browserify-zlib": true, "eslint>fast-deep-equal": true, "immer": true, + "nanoid": true, "readable-stream": true, "semver": true } @@ -4514,9 +4514,7 @@ }, "nanoid": { "globals": { - "crypto": true, - "msCrypto": true, - "navigator": true + "crypto.getRandomValues": true } }, "nock>debug": { diff --git a/lavamoat/browserify/mmi/policy.json b/lavamoat/browserify/mmi/policy.json index 8d54d36f2abd..cd8c3a1c7396 100644 --- a/lavamoat/browserify/mmi/policy.json +++ b/lavamoat/browserify/mmi/policy.json @@ -729,9 +729,9 @@ "console.info": true }, "packages": { - "@metamask/approval-controller>nanoid": true, "@metamask/base-controller": true, - "@metamask/rpc-errors": true + "@metamask/rpc-errors": true, + "nanoid": true } }, "@metamask/approval-controller>nanoid": { @@ -2051,11 +2051,11 @@ "@metamask/base-controller": true, "@metamask/controller-utils": true, "@metamask/json-rpc-engine": true, - "@metamask/permission-controller>nanoid": true, "@metamask/rpc-errors": true, "@metamask/utils": true, "deep-freeze-strict": true, - "immer": true + "immer": true, + "nanoid": true } }, "@metamask/permission-controller>nanoid": { @@ -2538,7 +2538,6 @@ "@metamask/snaps-controllers>@xstate/fsm": true, "@metamask/snaps-controllers>concat-stream": true, "@metamask/snaps-controllers>get-npm-tarball-url": true, - "@metamask/snaps-controllers>nanoid": true, "@metamask/snaps-controllers>readable-web-to-node-stream": true, "@metamask/snaps-controllers>tar-stream": true, "@metamask/snaps-rpc-methods": true, @@ -2549,6 +2548,7 @@ "browserify>browserify-zlib": true, "eslint>fast-deep-equal": true, "immer": true, + "nanoid": true, "readable-stream": true, "semver": true } @@ -4606,9 +4606,7 @@ }, "nanoid": { "globals": { - "crypto": true, - "msCrypto": true, - "navigator": true + "crypto.getRandomValues": true } }, "nock>debug": { diff --git a/lavamoat/build-system/policy.json b/lavamoat/build-system/policy.json index 9874c5915d8e..59a807e7313f 100644 --- a/lavamoat/build-system/policy.json +++ b/lavamoat/build-system/policy.json @@ -6510,7 +6510,7 @@ "process.env.NODE_ENV": true }, "packages": { - "postcss>nanoid": true, + "nanoid": true, "postcss>picocolors": true, "postcss>source-map-js": true } diff --git a/package.json b/package.json index 1072a40599f4..cca9f509c9b5 100644 --- a/package.json +++ b/package.json @@ -398,7 +398,7 @@ "loglevel": "^1.8.1", "lottie-web": "^5.12.2", "luxon": "^3.2.1", - "nanoid": "^2.1.6", + "nanoid": "^3.3.8", "pify": "^5.0.0", "promise-to-callback": "^1.0.0", "prop-types": "^15.6.1", diff --git a/yarn.lock b/yarn.lock index f670242e47b3..0359c83c1da9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -26801,7 +26801,7 @@ __metadata: mocha: "npm:^10.2.0" mocha-junit-reporter: "npm:^2.2.1" mockttp: "npm:^3.10.1" - nanoid: "npm:^2.1.6" + nanoid: "npm:^3.3.8" nock: "patch:nock@npm%3A13.5.4#~/.yarn/patches/nock-npm-13.5.4-2c4f77b249.patch" node-fetch: "npm:^2.6.1" nyc: "npm:^15.1.0" @@ -27953,19 +27953,19 @@ __metadata: languageName: node linkType: hard -"nanoid@npm:^2.0.0, nanoid@npm:^2.1.6": +"nanoid@npm:^2.0.0": version: 2.1.11 resolution: "nanoid@npm:2.1.11" checksum: 10/cf2a2eedcf9d8893a4687f11743ccf8381f047bc2b3d3887a23721bbef8fe64c5759b9cba6eb945e40efeb4a7e7379b3417e4dc5f6cc03050322d2c24a7ff69b languageName: node linkType: hard -"nanoid@npm:^3.1.31, nanoid@npm:^3.3.7": - version: 3.3.7 - resolution: "nanoid@npm:3.3.7" +"nanoid@npm:^3.1.31, nanoid@npm:^3.3.7, nanoid@npm:^3.3.8": + version: 3.3.8 + resolution: "nanoid@npm:3.3.8" bin: nanoid: bin/nanoid.cjs - checksum: 10/ac1eb60f615b272bccb0e2b9cd933720dad30bf9708424f691b8113826bb91aca7e9d14ef5d9415a6ba15c266b37817256f58d8ce980c82b0ba3185352565679 + checksum: 10/2d1766606cf0d6f47b6f0fdab91761bb81609b2e3d367027aff45e6ee7006f660fb7e7781f4a34799fe6734f1268eeed2e37a5fdee809ade0c2d4eb11b0f9c40 languageName: node linkType: hard From b9c57634048694a8cfba9e6794747a49bbd0a768 Mon Sep 17 00:00:00 2001 From: seaona <54408225+seaona@users.noreply.github.com> Date: Tue, 17 Dec 2024 14:45:38 +0100 Subject: [PATCH 2/9] test: [POM] Privacy Mode spec (#29263) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** - Migrates `test/e2e/tests/privacy-mode/privacy-mode.spec.js` spec to POM and ts - Updates method classes to accommodate new functions needed in this spec - Fixes several spec issues that where found during the migration (see comments) [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29263?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/29262 ## **Manual testing steps** 1. Check ci is green 2. Run test `yarn test:e2e:single test/e2e/tests/privacy-mode/privacy-mode.spec.ts --browser=chrome --leave-running=true` ## **Screenshots/Recordings** ### **Before** ### **After** ## **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/main/.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/main/.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. --- .../page-objects/pages/account-list-page.ts | 13 +++ test/e2e/page-objects/pages/header-navbar.ts | 19 ++-- test/e2e/page-objects/pages/home/homepage.ts | 37 ++++-- .../tests/privacy-mode/privacy-mode.spec.js | 106 ------------------ .../tests/privacy-mode/privacy-mode.spec.ts | 74 ++++++++++++ 5 files changed, 128 insertions(+), 121 deletions(-) delete mode 100644 test/e2e/tests/privacy-mode/privacy-mode.spec.js create mode 100644 test/e2e/tests/privacy-mode/privacy-mode.spec.ts diff --git a/test/e2e/page-objects/pages/account-list-page.ts b/test/e2e/page-objects/pages/account-list-page.ts index bbf25013f607..981955eb19cd 100644 --- a/test/e2e/page-objects/pages/account-list-page.ts +++ b/test/e2e/page-objects/pages/account-list-page.ts @@ -547,6 +547,19 @@ class AccountListPage { }); } + /** + * Verifies that all occurrences of the account balance value and symbol are displayed as private. + * + */ + async check_balanceIsPrivateEverywhere(): Promise { + console.log(`Verify all account balance occurrences are private`); + const balanceSelectors = { + tag: 'span', + text: '••••••', + }; + await this.driver.elementCountBecomesN(balanceSelectors, 6); + } + async check_currentAccountIsImported(): Promise { console.log(`Check that current account is an imported account`); await this.driver.waitForSelector({ diff --git a/test/e2e/page-objects/pages/header-navbar.ts b/test/e2e/page-objects/pages/header-navbar.ts index 5c0808eea93a..65d4cbd48358 100644 --- a/test/e2e/page-objects/pages/header-navbar.ts +++ b/test/e2e/page-objects/pages/header-navbar.ts @@ -80,13 +80,6 @@ class HeaderNavbar { await this.driver.clickElement(this.switchNetworkDropDown); } - async check_currentSelectedNetwork(networkName: string): Promise { - console.log(`Validate the Switch network to ${networkName}`); - await this.driver.waitForSelector( - `button[data-testid="network-display"][aria-label="Network Menu ${networkName}"]`, - ); - } - /** * Verifies that the displayed account label in header matches the expected label. * @@ -101,6 +94,18 @@ class HeaderNavbar { text: expectedLabel, }); } + + /** + * Validates that the currently selected network matches the expected network name. + * + * @param networkName - The expected name of the currently selected network. + */ + async check_currentSelectedNetwork(networkName: string): Promise { + console.log(`Validate the Switch network to ${networkName}`); + await this.driver.waitForSelector( + `button[data-testid="network-display"][aria-label="Network Menu ${networkName}"]`, + ); + } } export default HeaderNavbar; diff --git a/test/e2e/page-objects/pages/home/homepage.ts b/test/e2e/page-objects/pages/home/homepage.ts index 8240e51f736c..6b3f7c9f2a87 100644 --- a/test/e2e/page-objects/pages/home/homepage.ts +++ b/test/e2e/page-objects/pages/home/homepage.ts @@ -8,8 +8,9 @@ class HomePage { public headerNavbar: HeaderNavbar; - private readonly activityTab = - '[data-testid="account-overview__activity-tab"]'; + private readonly activityTab = { + testId: 'account-overview__activity-tab', + }; private readonly balance = '[data-testid="eth-overview__primary-currency"]'; @@ -23,19 +24,35 @@ class HomePage { tag: 'h6', }; - private readonly erc20TokenDropdown = '[data-testid="import-token-button"]'; + private readonly erc20TokenDropdown = { + testId: 'import-token-button', + }; - private readonly nftTab = '[data-testid="account-overview__nfts-tab"]'; + private readonly nftTab = { + testId: 'account-overview__nfts-tab', + }; private readonly popoverBackground = '.popover-bg'; - private readonly popoverCloseButton = '[data-testid="popover-close"]'; + private readonly popoverCloseButton = { + testId: 'popover-close', + }; - private readonly refreshErc20Tokens = '[data-testid="refreshList"]'; + private readonly privacyBalanceToggle = { + testId: 'sensitive-toggle', + }; - private readonly sendButton = '[data-testid="eth-overview-send"]'; + private readonly refreshErc20Tokens = { + testId: 'refreshList', + }; - private readonly tokensTab = '[data-testid="account-overview__asset-tab"]'; + private readonly sendButton = { + testId: 'eth-overview-send', + }; + + private readonly tokensTab = { + testId: 'account-overview__asset-tab', + }; constructor(driver: Driver) { this.driver = driver; @@ -93,6 +110,10 @@ class HomePage { await this.driver.clickElement(this.sendButton); } + async togglePrivacyBalance(): Promise { + await this.driver.clickElement(this.privacyBalanceToggle); + } + /** * Checks if the toaster message for adding a network is displayed on the homepage. * diff --git a/test/e2e/tests/privacy-mode/privacy-mode.spec.js b/test/e2e/tests/privacy-mode/privacy-mode.spec.js deleted file mode 100644 index ede37f900e66..000000000000 --- a/test/e2e/tests/privacy-mode/privacy-mode.spec.js +++ /dev/null @@ -1,106 +0,0 @@ -const { strict: assert } = require('assert'); -const { - withFixtures, - unlockWallet, - defaultGanacheOptions, -} = require('../../helpers'); -const FixtureBuilder = require('../../fixture-builder'); - -describe('Privacy Mode', function () { - it('should activate privacy mode, then deactivate it', async function () { - await withFixtures( - { - dapp: true, - fixtures: new FixtureBuilder().withPreferencesController().build(), - ganacheOptions: defaultGanacheOptions, - title: this.test.fullTitle(), - }, - async ({ driver }) => { - async function checkForHeaderValue(value) { - const balanceElement = await driver.findElement( - '[data-testid="account-value-and-suffix"]', - ); - const surveyText = await balanceElement.getText(); - assert.equal( - surveyText, - value, - `Header balance should be "${value}"`, - ); - } - - async function checkForTokenValue(value) { - const balanceElement = await driver.findElement( - '[data-testid="multichain-token-list-item-value"]', - ); - const surveyText = await balanceElement.getText(); - assert.equal(surveyText, value, `Token balance should be "${value}"`); - } - - async function checkForPrivacy() { - await checkForHeaderValue('••••••'); - await checkForTokenValue('••••••'); - } - - async function checkForNoPrivacy() { - await checkForHeaderValue('25'); - await checkForTokenValue('25 ETH'); - } - - async function togglePrivacy() { - const balanceElement = await driver.findElement( - '[data-testid="account-value-and-suffix"]', - ); - const initialText = await balanceElement.getText(); - - await driver.clickElement('[data-testid="sensitive-toggle"]'); - await driver.wait(async () => { - const currentText = await balanceElement.getText(); - return currentText !== initialText; - }, 2e3); - } - - await unlockWallet(driver); - await checkForNoPrivacy(); - await togglePrivacy(); - await checkForPrivacy(); - await togglePrivacy(); - await checkForNoPrivacy(); - }, - ); - }); - - it('should hide fiat balance and token balance when privacy mode is activated', async function () { - await withFixtures( - { - fixtures: new FixtureBuilder().withPreferencesController().build(), - ganacheOptions: defaultGanacheOptions, - title: this.test.fullTitle(), - }, - async ({ driver }) => { - await unlockWallet(driver); - - async function togglePrivacy() { - const balanceElement = await driver.findElement( - '[data-testid="account-value-and-suffix"]', - ); - const initialText = await balanceElement.getText(); - - await driver.clickElement('[data-testid="sensitive-toggle"]'); - await driver.wait(async () => { - const currentText = await balanceElement.getText(); - return currentText !== initialText; - }, 2e3); - } - - await togglePrivacy(); - await driver.clickElement('[data-testid="account-menu-icon"]'); - const valueText = await driver.findElement( - '[data-testid="account-value-and-suffix"]', - ); - const valueTextContent = await valueText.getText(); - - assert.equal(valueTextContent, '••••••'); - }, - ); - }); -}); diff --git a/test/e2e/tests/privacy-mode/privacy-mode.spec.ts b/test/e2e/tests/privacy-mode/privacy-mode.spec.ts new file mode 100644 index 000000000000..60d9faedc080 --- /dev/null +++ b/test/e2e/tests/privacy-mode/privacy-mode.spec.ts @@ -0,0 +1,74 @@ +import { Driver } from '../../webdriver/driver'; +import { defaultGanacheOptions, withFixtures } from '../../helpers'; +import FixtureBuilder from '../../fixture-builder'; +import AccountListPage from '../../page-objects/pages/account-list-page'; +import HeaderNavbar from '../../page-objects/pages/header-navbar'; +import HomePage from '../../page-objects/pages/home/homepage'; +import { + loginWithBalanceValidation, + loginWithoutBalanceValidation, +} from '../../page-objects/flows/login.flow'; +import { Ganache } from '../../seeder/ganache'; + +describe('Privacy Mode', function () { + it('should hide fiat balance and token balance when privacy mode is activated', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + ganacheOptions: defaultGanacheOptions, + title: this.test?.fullTitle(), + }, + async ({ + driver, + ganacheServer, + }: { + driver: Driver; + ganacheServer: Ganache; + }) => { + await loginWithBalanceValidation(driver, ganacheServer); + const homePage = new HomePage(driver); + await homePage.check_pageIsLoaded(); + await homePage.togglePrivacyBalance(); + await homePage.check_expectedBalanceIsDisplayed('••••••', '••••••'); + + const headerNavbar = new HeaderNavbar(driver); + await headerNavbar.openAccountMenu(); + + const accountList = new AccountListPage(driver); + await accountList.check_pageIsLoaded(); + await accountList.check_balanceIsPrivateEverywhere(); + }, + ); + }); + + it('should show fiat balance and token balance when privacy mode is deactivated', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder() + .withPreferencesController({ + preferences: { + privacyMode: true, + }, + }) + .build(), + ganacheOptions: defaultGanacheOptions, + title: this.test?.fullTitle(), + }, + async ({ driver }: { driver: Driver }) => { + await loginWithoutBalanceValidation(driver); + + const homePage = new HomePage(driver); + await homePage.check_pageIsLoaded(); + await homePage.togglePrivacyBalance(); + await homePage.check_expectedBalanceIsDisplayed('25 ETH'); + + const headerNavbar = new HeaderNavbar(driver); + await headerNavbar.openAccountMenu(); + + const accountList = new AccountListPage(driver); + await accountList.check_pageIsLoaded(); + await accountList.check_accountBalanceDisplayed('25'); + }, + ); + }); +}); From 0ef0d54f5c0e1cafad3dd8c18391f192dc1da61e Mon Sep 17 00:00:00 2001 From: Vinicius Stevam <45455812+vinistevam@users.noreply.github.com> Date: Tue, 17 Dec 2024 14:55:15 +0000 Subject: [PATCH 3/9] fix: increase gas limit validation threshold (#29264) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR aims to increase the gas limit validation threshold to 30M. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29264?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/21927 ## **Manual testing steps** 1. send a transaction 2. edit gaslimit on advance tab ## **Screenshots/Recordings** [Screencast from 17-12-2024 08:51:19.webm](https://github.com/user-attachments/assets/a8ff1e67-681f-4d6f-9dd8-b0ff79d86baa) ### **Before** ### **After** ## **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/main/.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/main/.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. --- .../advanced-gas-fee-gas-limit.test.js | 4 ++-- ui/pages/confirmations/send/send.constants.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/pages/confirmations/components/advanced-gas-fee-popover/advanced-gas-fee-gas-limit/advanced-gas-fee-gas-limit.test.js b/ui/pages/confirmations/components/advanced-gas-fee-popover/advanced-gas-fee-gas-limit/advanced-gas-fee-gas-limit.test.js index 0f91cdfbb781..e8529f01b8b4 100644 --- a/ui/pages/confirmations/components/advanced-gas-fee-popover/advanced-gas-fee-gas-limit/advanced-gas-fee-gas-limit.test.js +++ b/ui/pages/confirmations/components/advanced-gas-fee-popover/advanced-gas-fee-gas-limit/advanced-gas-fee-gas-limit.test.js @@ -101,7 +101,7 @@ describe('AdvancedGasFeeGasLimit', () => { ), ).toBeInTheDocument(); fireEvent.change(document.getElementsByTagName('input')[0], { - target: { value: 8000000 }, + target: { value: 80000000 }, }); expect( screen.queryByText( @@ -154,7 +154,7 @@ describe('AdvancedGasFeeGasLimit', () => { ); expect( screen.queryByText( - `Gas limit must be greater than 29999 and less than 7920050`, + `Gas limit must be greater than 29999 and less than 30000000`, ), ).toBeInTheDocument(); }); diff --git a/ui/pages/confirmations/send/send.constants.js b/ui/pages/confirmations/send/send.constants.js index acf129412135..7fc3aa02484a 100644 --- a/ui/pages/confirmations/send/send.constants.js +++ b/ui/pages/confirmations/send/send.constants.js @@ -5,7 +5,7 @@ import { EtherDenomination } from '../../../../shared/constants/common'; const MIN_GAS_PRICE_DEC = '0'; const MIN_GAS_PRICE_HEX = parseInt(MIN_GAS_PRICE_DEC, 10).toString(16); const MIN_GAS_LIMIT_DEC = new Numeric('21000', 10); -const MAX_GAS_LIMIT_DEC = '7920027'; +const MAX_GAS_LIMIT_DEC = '30000000'; const HIGH_FEE_WARNING_MULTIPLIER = 1.5; const MIN_GAS_PRICE_GWEI = new Numeric( From a31d80803815fd5c7ec22f0ca2a887ee68bd17a3 Mon Sep 17 00:00:00 2001 From: seaona <54408225+seaona@users.noreply.github.com> Date: Tue, 17 Dec 2024 16:18:39 +0100 Subject: [PATCH 4/9] test: [POM] Migrate Send tx Revoke Permissions spec (#29273) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** - Migrates `test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.js` spec to POM and ts - Updates method classes to accommodate new functions needed in this spec - [A new bug](https://github.com/MetaMask/metamask-extension/issues/29272) has been discovered thanks to this migration, and a new TODO has been added, to add a new spec to cover that flow, once the bug is fixed [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29273?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/29276 ## **Manual testing steps** 1. Check ci 2. Run test ` yarn test:e2e:single test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.ts --browser=chrome --leave-running=true` ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.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. --- test/e2e/page-objects/pages/test-dapp.ts | 20 +++++ .../sendTx-revokePermissions.spec.js | 58 -------------- .../sendTx-revokePermissions.spec.ts | 78 +++++++++++++++++++ 3 files changed, 98 insertions(+), 58 deletions(-) delete mode 100644 test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.js create mode 100644 test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.ts diff --git a/test/e2e/page-objects/pages/test-dapp.ts b/test/e2e/page-objects/pages/test-dapp.ts index 5155707663b6..ef5aca0c6bf7 100644 --- a/test/e2e/page-objects/pages/test-dapp.ts +++ b/test/e2e/page-objects/pages/test-dapp.ts @@ -321,6 +321,26 @@ class TestDapp { }); } + /** + * Verifies the accounts connected to the test dapp. + * + * @param connectedAccounts - The expected connected accounts separated by a comma. If no accounts are connected we can omit the param. + */ + async check_connectedAccounts(connectedAccounts: string = '') { + console.log('Verify connected accounts'); + if (connectedAccounts) { + await this.driver.waitForSelector({ + css: this.connectedAccount, + text: connectedAccounts, + }); + } else { + await this.driver.waitForSelector({ + css: this.connectedAccount, + text: ' ', + }); + } + } + /** * Verify the failed personal sign signature. * diff --git a/test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.js b/test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.js deleted file mode 100644 index 363f7c94cc74..000000000000 --- a/test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.js +++ /dev/null @@ -1,58 +0,0 @@ -const FixtureBuilder = require('../../fixture-builder'); -const { - withFixtures, - openDapp, - unlockWallet, - DAPP_URL, - defaultGanacheOptions, -} = require('../../helpers'); -const { PAGES } = require('../../webdriver/driver'); - -describe('Request Queuing', function () { - it('should clear tx confirmation when revokePermission is called from origin dapp', async function () { - await withFixtures( - { - dapp: true, - fixtures: new FixtureBuilder() - .withPermissionControllerConnectedToTestDapp() - .withPreferencesControllerUseRequestQueueEnabled() - .withSelectedNetworkControllerPerDomain() - .build(), - ganacheOptions: { - ...defaultGanacheOptions, - }, - title: this.test.fullTitle(), - }, - async ({ driver }) => { - await unlockWallet(driver); - - // Navigate to extension home screen - await driver.navigate(PAGES.HOME); - - // Open Dapp One - await openDapp(driver, undefined, DAPP_URL); - - // wallet_revokePermissions request - const revokePermissionsRequest = JSON.stringify({ - jsonrpc: '2.0', - method: 'wallet_revokePermissions', - params: [ - { - eth_accounts: {}, - }, - ], - }); - - await driver.executeScript( - `return window.ethereum.request(${revokePermissionsRequest})`, - ); - - // Should have cleared the tx confirmation - await driver.waitUntilXWindowHandles(2); - - // Cleared eth_accounts account label - await driver.findElement({ xpath: '//span[@id="accounts"][.=""]' }); - }, - ); - }); -}); diff --git a/test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.ts b/test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.ts new file mode 100644 index 000000000000..c5ff864df99c --- /dev/null +++ b/test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.ts @@ -0,0 +1,78 @@ +import { loginWithBalanceValidation } from '../../page-objects/flows/login.flow'; +import TestDapp from '../../page-objects/pages/test-dapp'; +import TransactionConfirmation from '../../page-objects/pages/confirmations/redesign/transaction-confirmation'; +import { Ganache } from '../../seeder/ganache'; +import { Driver } from '../../webdriver/driver'; +import { DEFAULT_FIXTURE_ACCOUNT } from '../../constants'; +import FixtureBuilder from '../../fixture-builder'; +import { + withFixtures, + defaultGanacheOptions, + WINDOW_TITLES, +} from '../../helpers'; + +describe('Request Queuing', function () { + // TODO: add a new spec which checks that after revoking and connecting again + // a pending tx is still closed when using revokePermissions. + // To be done once this bug is fixed: #29272 + it('should clear tx confirmation when revokePermission is called from origin dapp', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withPermissionControllerConnectedToTestDapp() + .withPreferencesControllerUseRequestQueueEnabled() + .withSelectedNetworkControllerPerDomain() + .build(), + ganacheOptions: { + ...defaultGanacheOptions, + }, + title: this.test?.fullTitle(), + }, + async ({ + driver, + ganacheServer, + }: { + driver: Driver; + ganacheServer?: Ganache; + }) => { + await loginWithBalanceValidation(driver, ganacheServer); + + // Open test dapp + const testDapp = new TestDapp(driver); + await testDapp.openTestDappPage(); + await testDapp.check_connectedAccounts( + DEFAULT_FIXTURE_ACCOUNT.toLowerCase(), + ); + + // Trigger a tx + await testDapp.clickSimpleSendButton(); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); + + const transactionConfirmation = new TransactionConfirmation(driver); + await transactionConfirmation.check_dappInitiatedHeadingTitle(); + + // wallet_revokePermissions request + const revokePermissionsRequest = JSON.stringify({ + jsonrpc: '2.0', + method: 'wallet_revokePermissions', + params: [ + { + eth_accounts: {}, + }, + ], + }); + await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); + await driver.executeScript( + `return window.ethereum.request(${revokePermissionsRequest})`, + ); + + // Should have cleared the tx confirmation + await driver.waitUntilXWindowHandles(2); + + // Cleared eth_accounts account label + await testDapp.check_connectedAccounts(); + }, + ); + }); +}); From 18bcad9eb83d03f6bb69a312975881503d3aae21 Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Tue, 17 Dec 2024 21:12:35 +0530 Subject: [PATCH 5/9] fix: Fix low gas display label in speed-up modal (#29277) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Fix issue that `undefined` is sometime visible on speedup modal. It was actually visible in edge case when `10% low` is less than low estimate. ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/28840 ## **Manual testing steps** 1. Go to test dapp 2. Submit a transaction with less gas so that it takes longer to complete 3. Open speedup modal and switch to `10% less` gas estimate 4. You should not see undefined string ## **Screenshots/Recordings** Screenshot 2024-12-17 at 7 05 58 PM ## **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/main/.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/main/.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. --- .../gas-timing/gas-timing.component.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/ui/pages/confirmations/components/gas-timing/gas-timing.component.js b/ui/pages/confirmations/components/gas-timing/gas-timing.component.js index 5c8e054647c3..dc495468d11c 100644 --- a/ui/pages/confirmations/components/gas-timing/gas-timing.component.js +++ b/ui/pages/confirmations/components/gas-timing/gas-timing.component.js @@ -178,13 +178,15 @@ export default function GasTiming({ return ( - - {text} - + {text && ( + + {text} + + )} {time && ( From 8ca78e7fcbbf921c24abf8e415089e5a45f7e7fe Mon Sep 17 00:00:00 2001 From: Danica Shen Date: Tue, 17 Dec 2024 16:04:04 +0000 Subject: [PATCH 6/9] fix(27140): fix styles for Slippage Tolerance Button Group in Transaction Settings Modal (#29246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR aims to fix a broken visual display for Slippage Tolerance Button Group in Transaction Settings Modal. After inspecting the styles for `ButtonGroup`, which is the component applies to this part of UI, has overwritten issues of styles between `button-group__button` and `radio-button`. Hence the solution is to specify css based on variant more clearly and avoid the overwrite problem. Screenshot 2024-12-16 at 18 59 19 ## **Description** [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29246?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/27140 ## **Manual testing steps** 1. Click "Swap" from the home screen. 2. In the swap interface, click the cog icon in the top right to open the transaction settings modal. 3. Observe the slippage tolerance button group—it's visually broken. ## **Screenshots/Recordings** ### **Before** 1 ### **After** Screenshot 2024-12-16 at 19 19 04 ## **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/main/.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/main/.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. --- .../button-group-component.test.js.snap | 35 +++- .../button-group-component.test.js | 16 +- .../ui/button-group/button-group.component.js | 7 +- ui/components/ui/button-group/index.scss | 2 +- .../transaction-settings.test.js.snap | 8 +- .../swaps/transaction-settings/index.scss | 4 +- .../transaction-settings.js | 171 +++++++++--------- 7 files changed, 143 insertions(+), 100 deletions(-) diff --git a/ui/components/ui/button-group/__snapshots__/button-group-component.test.js.snap b/ui/components/ui/button-group/__snapshots__/button-group-component.test.js.snap index 7e8116227458..b04aa2fbd4f1 100644 --- a/ui/components/ui/button-group/__snapshots__/button-group-component.test.js.snap +++ b/ui/components/ui/button-group/__snapshots__/button-group-component.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`ButtonGroup Component should match snapshot 1`] = ` +exports[`ButtonGroup Component should match snapshot with default variant 1`] = `
`; + +exports[`ButtonGroup Component should match snapshot with radiogroup variant 1`] = ` +
+
+ +
+
+`; diff --git a/ui/components/ui/button-group/button-group-component.test.js b/ui/components/ui/button-group/button-group-component.test.js index 2aaccad78a0c..584042491867 100644 --- a/ui/components/ui/button-group/button-group-component.test.js +++ b/ui/components/ui/button-group/button-group-component.test.js @@ -16,15 +16,25 @@ describe('ButtonGroup Component', () => { , - , - , + + + - - - - + } + }} + value={customValue || ''} + /> +
+ ) : ( + customValueText + )} + {(customValue || enteringCustomValue) && ( +
+ % +
+ )} + + )} From fe809431e6cbfdd7d18a2d5ba90570d6c5ae1337 Mon Sep 17 00:00:00 2001 From: George Marshall Date: Tue, 17 Dec 2024 08:40:14 -0800 Subject: [PATCH 7/9] fix: add focus state to swaps input for improved accessibility (#29252) ## **Description** The current swaps input implementation hides the focus indicator, making it inaccessible for users with vision impairments and those relying on keyboard navigation. This creates barriers for: - Users with low vision who need clear visual indicators - Users with motor impairments who navigate via keyboard - Users of screen magnification tools who need to track their position This PR improves accessibility by: 1. Removing 'outline: none' from the swaps input to restore focus visibility 2. Adjusting padding (from 0 to 4px) and margins to maintain visual consistency 3. Adding proper focus visibility for keyboard navigation These changes ensure compliance with [WCAG 2.1 Success Criterion 2.4.7 (Focus Visible)](https://www.w3.org/WAI/WCAG21/Understanding/focus-visible.html), which requires that keyboard focus indicators be visible and distinguishable. ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/26662 ## **Manual testing steps** 1. Navigate to the swaps page 2. Use Tab key to move focus to the input field 3. Verify that a visible focus indicator appears around the input 4. Ensure the text alignment and spacing remain consistent 5. Test that the input remains right-aligned with the new padding ## **Screenshots/Recordings** ### **Before** Input field shows no focus indicator when navigating with keyboard https://github.com/user-attachments/assets/961ea815-2014-47e3-b4f2-514197fae9dd ### **After** Input field shows clear focus indicator when navigating with keyboard, with padding-right: 4px https://github.com/user-attachments/assets/27a27dd5-51f3-473a-ad5b-bbde22caa7e2 ## **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/main/.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 ## **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 --- ui/pages/swaps/prepare-swap-page/index.scss | 4 ++-- ui/pages/swaps/prepare-swap-page/prepare-swap-page.js | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ui/pages/swaps/prepare-swap-page/index.scss b/ui/pages/swaps/prepare-swap-page/index.scss index b443006330d3..a6e027c84830 100644 --- a/ui/pages/swaps/prepare-swap-page/index.scss +++ b/ui/pages/swaps/prepare-swap-page/index.scss @@ -133,10 +133,10 @@ &__from-token-amount { border: 0; - outline: none; + margin-right: -4px; input { - padding-right: 0; + padding-right: 4px; text-align: right; font-weight: var(--typography-s-heading-lg-font-weight); font-size: var(--typography-s-heading-lg-font-size); diff --git a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js index a100b490b4d6..2bcc2672e335 100644 --- a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js +++ b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js @@ -893,6 +893,7 @@ export default function PrepareSwapPage({ display={DISPLAY.FLEX} justifyContent={JustifyContent.spaceBetween} alignItems={AlignItems.center} + gap={4} > Date: Tue, 17 Dec 2024 08:40:22 -0800 Subject: [PATCH 8/9] chore: quote timeout treatment for bridging (#29172) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** when the quote is older than 30s - if the user has sufficient balance, hide quote and show a button that restarts polling on click - if the user doesn't have sufficient balance, keep showing the quote quote but display a button that restarts polling on click [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29172?quickstart=1) ## **Related issues** Fixes: N/A ## **Manual testing steps** 1. Request quotes 2. Wait for ~3 minutes after fetching starts 3. Quote should time out 4. Clicking the Fetch button should restart polling ## **Screenshots/Recordings** ### **Before** N/A ### **After** ![Screenshot 2024-12-12 at 1 50 40 PM](https://github.com/user-attachments/assets/1a0e2c1e-1260-40fb-bd10-2f78ac61b1e1) ![Screenshot 2024-12-12 at 1 46 18 PM](https://github.com/user-attachments/assets/5836cc59-dd32-4afe-9213-6cadad3e7fab) ## **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/main/.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/main/.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. --- app/_locales/en/messages.json | 5 +- .../bridge/__snapshots__/index.test.tsx.snap | 12 ++- .../bridge-cta-button.test.tsx.snap | 34 +++++--- .../prepare-bridge-page.test.tsx.snap | 24 ++++-- .../bridge/prepare/bridge-cta-button.test.tsx | 12 +-- ui/pages/bridge/prepare/bridge-cta-button.tsx | 82 ++++++++++++------- .../bridge/prepare/prepare-bridge-page.tsx | 32 ++++++-- ui/pages/bridge/utils/quote.ts | 11 +++ 8 files changed, 145 insertions(+), 67 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 12df50e257d7..1ff278090f61 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -883,6 +883,9 @@ "bridgeExplorerLinkViewOn": { "message": "View on $1" }, + "bridgeFetchNewQuotes": { + "message": "Fetch a new one?" + }, "bridgeFrom": { "message": "Bridge from" }, @@ -897,7 +900,7 @@ "message": "Net cost" }, "bridgeQuoteExpired": { - "message": "Quotes expired - please request again" + "message": "Your quote timed out." }, "bridgeSelectNetwork": { "message": "Select network" diff --git a/ui/pages/bridge/__snapshots__/index.test.tsx.snap b/ui/pages/bridge/__snapshots__/index.test.tsx.snap index 51473d6fefa0..072c7d7eee6b 100644 --- a/ui/pages/bridge/__snapshots__/index.test.tsx.snap +++ b/ui/pages/bridge/__snapshots__/index.test.tsx.snap @@ -222,11 +222,15 @@ exports[`Bridge renders the component with initial props 1`] = `
-

- Select token and amount -

+

+ Select token and amount +

+
diff --git a/ui/pages/bridge/prepare/__snapshots__/bridge-cta-button.test.tsx.snap b/ui/pages/bridge/prepare/__snapshots__/bridge-cta-button.test.tsx.snap index 0b46d2764523..6014dbaddb23 100644 --- a/ui/pages/bridge/prepare/__snapshots__/bridge-cta-button.test.tsx.snap +++ b/ui/pages/bridge/prepare/__snapshots__/bridge-cta-button.test.tsx.snap @@ -2,9 +2,13 @@ exports[`BridgeCTAButton should disable the component when quotes are loading and there are no existing quotes 1`] = `
-

+

+

+

`; @@ -23,20 +27,28 @@ exports[`BridgeCTAButton should enable the component when quotes are loading and exports[`BridgeCTAButton should render the component when amount and dest token is missing 1`] = `
-

- Select token and amount -

+

+ Select token and amount +

+
`; exports[`BridgeCTAButton should render the component's initial state 1`] = `
-

- Select token -

+

+ Select token +

+
`; diff --git a/ui/pages/bridge/prepare/__snapshots__/prepare-bridge-page.test.tsx.snap b/ui/pages/bridge/prepare/__snapshots__/prepare-bridge-page.test.tsx.snap index d5b980d13fd6..c69368d0b88e 100644 --- a/ui/pages/bridge/prepare/__snapshots__/prepare-bridge-page.test.tsx.snap +++ b/ui/pages/bridge/prepare/__snapshots__/prepare-bridge-page.test.tsx.snap @@ -172,11 +172,15 @@ exports[`PrepareBridgePage should render the component, with initial state 1`] =
-

- Select token and amount -

+

+ Select token and amount +

+
@@ -357,11 +361,15 @@ exports[`PrepareBridgePage should render the component, with inputs set 1`] = `
-

- Select token and amount -

+

+ Select token and amount +

+
diff --git a/ui/pages/bridge/prepare/bridge-cta-button.test.tsx b/ui/pages/bridge/prepare/bridge-cta-button.test.tsx index d4e0fd0855c5..5dc368043d3a 100644 --- a/ui/pages/bridge/prepare/bridge-cta-button.test.tsx +++ b/ui/pages/bridge/prepare/bridge-cta-button.test.tsx @@ -23,7 +23,7 @@ describe('BridgeCTAButton', () => { bridgeSliceOverrides: { fromTokenInputValue: 1 }, }); const { container, getByText } = renderWithProvider( - , + , configureStore(mockStore), ); @@ -54,7 +54,7 @@ describe('BridgeCTAButton', () => { }, }); const { getByText } = renderWithProvider( - , + , configureStore(mockStore), ); @@ -83,7 +83,7 @@ describe('BridgeCTAButton', () => { }, }); const { getByText, container } = renderWithProvider( - , + , configureStore(mockStore), ); @@ -118,7 +118,7 @@ describe('BridgeCTAButton', () => { }, }); const { getByText, getByRole } = renderWithProvider( - , + , configureStore(mockStore), ); @@ -159,7 +159,7 @@ describe('BridgeCTAButton', () => { }, }); const { container } = renderWithProvider( - , + , configureStore(mockStore), ); @@ -199,7 +199,7 @@ describe('BridgeCTAButton', () => { }, }); const { getByText, getByRole, container } = renderWithProvider( - , + , configureStore(mockStore), ); diff --git a/ui/pages/bridge/prepare/bridge-cta-button.tsx b/ui/pages/bridge/prepare/bridge-cta-button.tsx index 8812e26eb099..8e45f90cd1db 100644 --- a/ui/pages/bridge/prepare/bridge-cta-button.tsx +++ b/ui/pages/bridge/prepare/bridge-cta-button.tsx @@ -1,6 +1,7 @@ -import React, { useEffect, useMemo, useState } from 'react'; +import React, { useMemo, useState } from 'react'; import { useSelector } from 'react-redux'; import { + ButtonLink, ButtonPrimary, ButtonPrimarySize, Text, @@ -18,7 +19,9 @@ import { import { useI18nContext } from '../../../hooks/useI18nContext'; import useSubmitBridgeTransaction from '../hooks/useSubmitBridgeTransaction'; import { + AlignItems, BlockSize, + JustifyContent, TextAlign, TextColor, TextVariant, @@ -32,8 +35,14 @@ import { useTradeProperties } from '../../../hooks/bridge/events/useTradePropert import { MetaMetricsEventName } from '../../../../shared/constants/metametrics'; import { SWAPS_CHAINID_DEFAULT_TOKEN_MAP } from '../../../../shared/constants/swaps'; import { getNativeCurrency } from '../../../ducks/metamask/metamask'; - -export const BridgeCTAButton = () => { +import { Row } from '../layout'; +import { isQuoteExpired as isQuoteExpiredUtil } from '../utils/quote'; + +export const BridgeCTAButton = ({ + onFetchNewQuotes, +}: { + onFetchNewQuotes: () => void; +}) => { const t = useI18nContext(); const fromToken = useSelector(getFromToken); @@ -43,9 +52,14 @@ export const BridgeCTAButton = () => { const fromAmount = useSelector(getFromAmount); - const { isLoading, activeQuote, isQuoteGoingToRefresh, quotesRefreshCount } = + const { isLoading, activeQuote, isQuoteGoingToRefresh, quotesLastFetchedMs } = useSelector(getBridgeQuotes); - const { maxRefreshCount, refreshRate } = useSelector(getBridgeQuotesConfig); + const { refreshRate } = useSelector(getBridgeQuotesConfig); + const isQuoteExpired = isQuoteExpiredUtil( + isQuoteGoingToRefresh, + refreshRate, + quotesLastFetchedMs, + ); const { submitBridgeTransaction } = useSubmitBridgeTransaction(); const [isSubmitting, setIsSubmitting] = useState(false); @@ -76,7 +90,6 @@ export const BridgeCTAButton = () => { const tradeProperties = useTradeProperties(); const ticker = useSelector(getNativeCurrency); - const [isQuoteExpired, setIsQuoteExpired] = useState(false); const isInsufficientBalance = isInsufficientBalance_(balanceAmount); @@ -85,28 +98,12 @@ export const BridgeCTAButton = () => { const isInsufficientGasForQuote = isInsufficientGasForQuote_(nativeAssetBalance); - useEffect(() => { - let timeout: NodeJS.Timeout; - // Reset the isQuoteExpired if quote fethching restarts - if (quotesRefreshCount === 0) { - setIsQuoteExpired(false); - return () => clearTimeout(timeout); - } - // After the last quote refresh, set a timeout to expire the quote and disable the CTA - if (quotesRefreshCount >= maxRefreshCount && !isQuoteGoingToRefresh) { - timeout = setTimeout(() => { - setIsQuoteExpired(true); - }, refreshRate); - } - return () => clearTimeout(timeout); - }, [isQuoteGoingToRefresh, quotesRefreshCount]); - const label = useMemo(() => { if (wasTxDeclined) { return t('youDeclinedTheTransaction'); } - if (isQuoteExpired && !isNoQuotesAvailable) { + if (isQuoteExpired) { return t('bridgeQuoteExpired'); } @@ -149,7 +146,15 @@ export const BridgeCTAButton = () => { isQuoteExpired, ]); - return activeQuote && !wasTxDeclined ? ( + // Label for the secondary button that re-starts quote fetching + const secondaryButtonLabel = useMemo(() => { + if (wasTxDeclined || isQuoteExpired) { + return t('bridgeFetchNewQuotes'); + } + return undefined; + }, [wasTxDeclined, isQuoteExpired]); + + return activeQuote && !secondaryButtonLabel ? ( { {label} ) : ( - - {label} - + + {label} + + {secondaryButtonLabel && ( + + {secondaryButtonLabel} + + )} + ); }; diff --git a/ui/pages/bridge/prepare/prepare-bridge-page.tsx b/ui/pages/bridge/prepare/prepare-bridge-page.tsx index b4774ec5489a..fc076b73629b 100644 --- a/ui/pages/bridge/prepare/prepare-bridge-page.tsx +++ b/ui/pages/bridge/prepare/prepare-bridge-page.tsx @@ -67,7 +67,11 @@ import { hexToDecimal } from '../../../../shared/modules/conversion.utils'; import { QuoteRequest } from '../types'; import { calcTokenValue } from '../../../../shared/lib/swaps-utils'; import { BridgeQuoteCard } from '../quotes/bridge-quote-card'; -import { formatTokenAmount, isValidQuoteRequest } from '../utils/quote'; +import { + formatTokenAmount, + isQuoteExpired as isQuoteExpiredUtil, + isValidQuoteRequest, +} from '../utils/quote'; import { getProviderConfig } from '../../../../shared/modules/selectors/networks'; import { CrossChainSwapsEventProperties, @@ -121,12 +125,24 @@ const PrepareBridgePage = () => { const slippage = useSelector(getSlippage); const quoteRequest = useSelector(getQuoteRequest); - const { isLoading, activeQuote, isQuoteGoingToRefresh } = - useSelector(getBridgeQuotes); - + const { + isLoading, + activeQuote: activeQuote_, + isQuoteGoingToRefresh, + quotesLastFetchedMs, + } = useSelector(getBridgeQuotes); const { refreshRate } = useSelector(getBridgeQuotesConfig); const wasTxDeclined = useSelector(getWasTxDeclined); + // If latest quote is expired and user has sufficient balance + // set activeQuote to undefined to hide stale quotes but keep inputs filled + const isQuoteExpired = isQuoteExpiredUtil( + isQuoteGoingToRefresh, + refreshRate, + quotesLastFetchedMs, + ); + const activeQuote = + isQuoteExpired && !quoteRequest.insufficientBal ? undefined : activeQuote_; const keyring = useSelector(getCurrentKeyring); // @ts-expect-error keyring type is wrong maybe? @@ -533,9 +549,13 @@ const PrepareBridgePage = () => { backgroundColor={BackgroundColor.primaryMuted} /> )} - {!wasTxDeclined && } + {!wasTxDeclined && activeQuote && }