From 4147b2dd24d0516e7b21d580ae685669db73a27d Mon Sep 17 00:00:00 2001 From: legobeat <109787230+legobeat@users.noreply.github.com> Date: Tue, 18 Jun 2024 17:39:15 +0900 Subject: [PATCH 1/8] fix: upgrade dependency ws (#25372) --- package.json | 2 ++ yarn.lock | 52 +++++++++++----------------------------------------- 2 files changed, 13 insertions(+), 41 deletions(-) diff --git a/package.json b/package.json index 08ae5a31307a..596e3acaf885 100644 --- a/package.json +++ b/package.json @@ -191,6 +191,8 @@ "regenerator-runtime@^0.13.4": "patch:regenerator-runtime@npm%3A0.13.7#./.yarn/patches/regenerator-runtime-npm-0.13.7-41bcbe64ea.patch", "regenerator-runtime@^0.13.7": "patch:regenerator-runtime@npm%3A0.13.7#./.yarn/patches/regenerator-runtime-npm-0.13.7-41bcbe64ea.patch", "regenerator-runtime@^0.11.0": "patch:regenerator-runtime@npm%3A0.13.7#./.yarn/patches/regenerator-runtime-npm-0.13.7-41bcbe64ea.patch", + "ws@8.13.0": "^8.17.1", + "ws@7.4.6": "^7.5.10", "jsdom@^16.7.0": "patch:jsdom@npm%3A16.7.0#./.yarn/patches/jsdom-npm-16.7.0-216c5c4bf9.patch", "trim": "^0.0.3", "@eslint/eslintrc@npm:^2.1.4": "patch:@eslint/eslintrc@npm%3A2.1.4#~/.yarn/patches/@eslint-eslintrc-npm-2.1.4-1ff4b5f908.patch", diff --git a/yarn.lock b/yarn.lock index 8b513f4b5d76..82e56c6428e5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -35711,9 +35711,9 @@ __metadata: languageName: node linkType: hard -"ws@npm:*, ws@npm:>=8.14.2, ws@npm:^8.11.0, ws@npm:^8.16.0, ws@npm:^8.2.3, ws@npm:^8.5.0, ws@npm:^8.8.0": - version: 8.16.0 - resolution: "ws@npm:8.16.0" +"ws@npm:*, ws@npm:>=8.14.2, ws@npm:^8.11.0, ws@npm:^8.16.0, ws@npm:^8.17.1, ws@npm:^8.2.3, ws@npm:^8.5.0, ws@npm:^8.8.0": + version: 8.17.1 + resolution: "ws@npm:8.17.1" peerDependencies: bufferutil: ^4.0.1 utf-8-validate: ">=5.0.2" @@ -35722,52 +35722,22 @@ __metadata: optional: true utf-8-validate: optional: true - checksum: 10/7c511c59e979bd37b63c3aea4a8e4d4163204f00bd5633c053b05ed67835481995f61a523b0ad2b603566f9a89b34cb4965cb9fab9649fbfebd8f740cea57f17 - languageName: node - linkType: hard - -"ws@npm:7.4.6": - version: 7.4.6 - resolution: "ws@npm:7.4.6" - peerDependencies: - bufferutil: ^4.0.1 - utf-8-validate: ^5.0.2 - peerDependenciesMeta: - bufferutil: - optional: true - utf-8-validate: - optional: true - checksum: 10/150e3f917b7cde568d833a5ea6ccc4132e59c38d04218afcf2b6c7b845752bd011a9e0dc1303c8694d3c402a0bdec5893661a390b71ff88f0fc81a4e4e66b09c - languageName: node - linkType: hard - -"ws@npm:8.13.0": - version: 8.13.0 - resolution: "ws@npm:8.13.0" - peerDependencies: - bufferutil: ^4.0.1 - utf-8-validate: ">=5.0.2" - peerDependenciesMeta: - bufferutil: - optional: true - utf-8-validate: - optional: true - checksum: 10/1769532b6fdab9ff659f0b17810e7501831d34ecca23fd179ee64091dd93a51f42c59f6c7bb4c7a384b6c229aca8076fb312aa35626257c18081511ef62a161d + checksum: 10/4264ae92c0b3e59c7e309001e93079b26937aab181835fb7af79f906b22cd33b6196d96556dafb4e985742dd401e99139572242e9847661fdbc96556b9e6902d languageName: node linkType: hard "ws@npm:^6.1.0": - version: 6.2.2 - resolution: "ws@npm:6.2.2" + version: 6.2.3 + resolution: "ws@npm:6.2.3" dependencies: async-limiter: "npm:~1.0.0" - checksum: 10/bb791ac02ad7e59fd4208cc6dd3a5bf7a67dff4611a128ed33365996f9fc24fa0d699043559f1798b4bc8045639fd21a1fd3ceca81de560124444abd8e321afc + checksum: 10/19f8d1608317f4c98f63da6eebaa85260a6fe1ba459cbfedd83ebe436368177fb1e2944761e2392c6b7321cbb7a375c8a81f9e1be35d555b6b4647eb61eadd46 languageName: node linkType: hard -"ws@npm:^7, ws@npm:^7.2.0, ws@npm:^7.4.5, ws@npm:^7.4.6, ws@npm:^7.5.0": - version: 7.5.9 - resolution: "ws@npm:7.5.9" +"ws@npm:^7, ws@npm:^7.2.0, ws@npm:^7.4.5, ws@npm:^7.4.6, ws@npm:^7.5.0, ws@npm:^7.5.10": + version: 7.5.10 + resolution: "ws@npm:7.5.10" peerDependencies: bufferutil: ^4.0.1 utf-8-validate: ^5.0.2 @@ -35776,7 +35746,7 @@ __metadata: optional: true utf-8-validate: optional: true - checksum: 10/171e35012934bd8788150a7f46f963e50bac43a4dc524ee714c20f258693ac4d3ba2abadb00838fdac42a47af9e958c7ae7e6f4bc56db047ba897b8a2268cf7c + checksum: 10/9c796b84ba80ffc2c2adcdfc9c8e9a219ba99caa435c9a8d45f9ac593bba325563b3f83edc5eb067cc6d21b9a6bf2c930adf76dd40af5f58a5ca6859e81858f0 languageName: node linkType: hard From 3820f018f2c7bec4646530b5e50364b35bcc92f1 Mon Sep 17 00:00:00 2001 From: Corey Janssen <107953793+coreyjanssen@users.noreply.github.com> Date: Tue, 18 Jun 2024 02:02:28 -0700 Subject: [PATCH 2/8] fix: typo in proposed nickname settings (#23562) Deleting an extraneous "the" in the settings description for proposed nicknames. Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com> Co-authored-by: Howard Braham --- app/_locales/en/messages.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 4d251becd20a..2363bc51dc2d 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1775,7 +1775,7 @@ "message": "Proposed nicknames" }, "externalNameSourcesSettingDescription": { - "message": "We’ll fetch proposed nicknames for addresses you interact with from third-party sources like Etherscan, Infura, and Lens Protocol. These sources will be able to see the those addresses and your IP address. Your account address won’t be exposed to third parties." + "message": "We’ll fetch proposed nicknames for addresses you interact with from third-party sources like Etherscan, Infura, and Lens Protocol. These sources will be able to see those addresses and your IP address. Your account address won’t be exposed to third parties." }, "failed": { "message": "Failed" From ed0617634417df97905849f990300bcf4a6ed75f Mon Sep 17 00:00:00 2001 From: Matteo Scurati Date: Tue, 18 Jun 2024 11:38:03 +0200 Subject: [PATCH 3/8] fix: display the correct symbol of a native currency (#25364) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR fixes a small bug in the display of certain types of notifications. Specifically, in the case of receiving or sending notifications of native currencies, the symbol of the native currency is now correctly displayed. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25364?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** ## **Screenshots/Recordings** ### **Before** Screenshot 2024-06-17 at 11 16 01 ### **After** Screenshot 2024-06-17 at 11 43 31 ## **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** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ x] 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. --- .../eth-sent-received/eth-sent-received.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/pages/notifications/notification-components/eth-sent-received/eth-sent-received.tsx b/ui/pages/notifications/notification-components/eth-sent-received/eth-sent-received.tsx index 5b44e42f7112..ec857b45da94 100644 --- a/ui/pages/notifications/notification-components/eth-sent-received/eth-sent-received.tsx +++ b/ui/pages/notifications/notification-components/eth-sent-received/eth-sent-received.tsx @@ -63,8 +63,8 @@ const getTitle = (n: ETHNotification) => { }; const getDescription = (n: ETHNotification) => { - const { nativeCurrencyName } = getNativeCurrency(n); - const items = createTextItems([nativeCurrencyName], TextVariant.bodyMd); + const { nativeCurrencySymbol } = getNativeCurrency(n); + const items = createTextItems([nativeCurrencySymbol], TextVariant.bodyMd); return items; }; From 68d35f0dc2d9edc2a67849f4f932fb2a189af33a Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Tue, 18 Jun 2024 11:31:15 +0100 Subject: [PATCH 4/8] fix: failing push notifications (#25340) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This fixes push notifications so that they are not silent push notifications. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25340?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Create a wallet notification 2. See push notification received ## **Screenshots/Recordings** ### **Before** ![image](https://github.com/MetaMask/metamask-extension/assets/17534261/672702a9-b8d1-40ba-970e-adca7fbfedfc) ### **After** ![Screenshot 2024-06-14 at 21 04 55](https://github.com/MetaMask/metamask-extension/assets/17534261/3fcf8792-04c2-4825-89dc-389006f0e4f9) ## **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. --- .../services/services.ts | 15 +++++++++------ .../utils/get-notification-image.ts | 6 ++++++ .../utils/get-notification-message.ts | 5 ++++- 3 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 app/scripts/controllers/push-platform-notifications/utils/get-notification-image.ts diff --git a/app/scripts/controllers/push-platform-notifications/services/services.ts b/app/scripts/controllers/push-platform-notifications/services/services.ts index da15be11c8d8..392cd22ad051 100644 --- a/app/scripts/controllers/push-platform-notifications/services/services.ts +++ b/app/scripts/controllers/push-platform-notifications/services/services.ts @@ -195,18 +195,21 @@ export async function listenToPushNotifications( const unsubscribePushNotifications = onBackgroundMessage( messaging, async (payload: MessagePayload): Promise => { - const typedPayload = payload; - - // if the payload does not contain data, do nothing try { - const notificationData: NotificationUnion = typedPayload?.data?.data - ? JSON.parse(typedPayload?.data?.data) + const data = payload?.data?.data + ? JSON.parse(payload?.data?.data) : undefined; - if (!notificationData) { + // if the payload does not contain data, do nothing + if (!data) { return; } + const notificationData = { + ...data, + type: data?.type ?? data?.data?.kind, + } as NotificationUnion; + const notification = processNotification(notificationData); onNewNotification(notification); diff --git a/app/scripts/controllers/push-platform-notifications/utils/get-notification-image.ts b/app/scripts/controllers/push-platform-notifications/utils/get-notification-image.ts new file mode 100644 index 000000000000..3dbaf951b3d0 --- /dev/null +++ b/app/scripts/controllers/push-platform-notifications/utils/get-notification-image.ts @@ -0,0 +1,6 @@ +import browser from 'webextension-polyfill'; + +export async function getNotificationImage() { + const iconUrl = await browser.runtime.getURL('../../images/icon-64.png'); + return iconUrl; +} diff --git a/app/scripts/controllers/push-platform-notifications/utils/get-notification-message.ts b/app/scripts/controllers/push-platform-notifications/utils/get-notification-message.ts index e979e9cc5b53..514c5b40fadc 100644 --- a/app/scripts/controllers/push-platform-notifications/utils/get-notification-message.ts +++ b/app/scripts/controllers/push-platform-notifications/utils/get-notification-message.ts @@ -9,6 +9,7 @@ import { t } from '../../../translate'; import type { Notification } from '../../metamask-notifications/types/types'; import ExtensionPlatform from '../../../platforms/extension'; import { getAmount, formatAmount } from './get-notification-data'; +import { getNotificationImage } from './get-notification-image'; type PushNotificationMessage = { title: string; @@ -47,9 +48,11 @@ export async function onPushNotification( return; } + const iconUrl = await getNotificationImage(); + await registration.showNotification(notificationMessage.title, { body: notificationMessage.description, - icon: './images/icon-64.png', + icon: iconUrl, tag: notification?.id, data: notification, }); From e70e44fdfebf011f88ce30915a8190f2752ff825 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 18 Jun 2024 13:43:17 +0200 Subject: [PATCH 5/8] chore: Refactor offscreen creation logic (#25302) ## **Description** Refactor initialization logic to defer creation of the offscreen document until the `MetaMaskController` is initialized. This adds a `offscreenPromise` to the controller that can be awaited for functionality that requires the offscreen document to be created. Additionally this PR adds a message that the offscreen document will send once initial execution of the offscreen page has finished. This is awaited in the `offscreenPromise`. We await `offscreenPromise` before unlocking the keyrings as some keyrings rely on the offscreen document to process requests, e.g. hardware wallets. There may be room for more improvements here though, that I have not tackled in this PR. As the hardware wallet logic doesn't seem to wait for iframes to fully load, so there is a chance of some missed messages. I have tested that hardware wallet support, at least for Ledger, is still working following the changes in this PR. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25302?quickstart=1) --- app/scripts/app-init.js | 24 ----------- app/scripts/background.js | 7 ++++ app/scripts/metamask-controller.js | 6 +++ app/scripts/offscreen.js | 44 +++++++++++++++++++++ offscreen/scripts/offscreen.ts | 6 +++ shared/constants/offscreen-communication.ts | 1 + 6 files changed, 64 insertions(+), 24 deletions(-) create mode 100644 app/scripts/offscreen.js diff --git a/app/scripts/app-init.js b/app/scripts/app-init.js index 3290dc59d84b..766ba15e2d0d 100644 --- a/app/scripts/app-init.js +++ b/app/scripts/app-init.js @@ -185,27 +185,3 @@ const registerInPageContentScript = async () => { }; registerInPageContentScript(); - -/** - * Creates an offscreen document that can be used to load additional scripts - * and iframes that can communicate with the extension through the chrome - * runtime API. Only one offscreen document may exist, so any iframes required - * by extension can be embedded in the offscreen.html file. See the offscreen - * folder for more details. - */ -async function createOffscreen() { - if (!chrome.offscreen || (await chrome.offscreen.hasDocument())) { - return; - } - - await chrome.offscreen.createDocument({ - url: './offscreen.html', - reasons: ['IFRAME_SCRIPTING'], - justification: - 'Used for Hardware Wallet and Snaps scripts to communicate with the extension.', - }); - - console.debug('Offscreen iframe loaded'); -} - -createOffscreen(); diff --git a/app/scripts/background.js b/app/scripts/background.js index cca8a57ab43a..20d4fe885a6e 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -67,6 +67,7 @@ import { shouldEmitDappViewedEvent, } from './lib/util'; import { generateSkipOnboardingState } from './skip-onboarding'; +import { createOffscreen } from './offscreen'; /* eslint-enable import/first */ @@ -261,6 +262,8 @@ function saveTimestamp() { */ async function initialize() { try { + const offscreenPromise = isManifestV3 ? createOffscreen() : null; + const initData = await loadStateFromPersistence(); const initState = initData.data; @@ -293,6 +296,7 @@ async function initialize() { {}, isFirstMetaMaskControllerSetup, initData.meta, + offscreenPromise, ); if (!isManifestV3) { await loadPhishingWarningPage(); @@ -507,6 +511,7 @@ function emitDappViewedMetricEvent( * @param {object} overrides - object with callbacks that are allowed to override the setup controller logic * @param isFirstMetaMaskControllerSetup * @param {object} stateMetadata - Metadata about the initial state and migrations, including the most recent migration version + * @param {Promise} offscreenPromise - A promise that resolves when the offscreen document has finished initialization. */ export function setupController( initState, @@ -514,6 +519,7 @@ export function setupController( overrides, isFirstMetaMaskControllerSetup, stateMetadata, + offscreenPromise, ) { // // MetaMask Controller @@ -542,6 +548,7 @@ export function setupController( isFirstMetaMaskControllerSetup, currentMigrationVersion: stateMetadata.version, featureFlags: {}, + offscreenPromise, }); setupEnsIpfsResolver({ diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 87cf408cb0e1..19c3fc67cfb4 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -374,6 +374,8 @@ export default class MetamaskController extends EventEmitter { // the only thing that uses controller connections are open metamask UI instances this.activeControllerConnections = 0; + this.offscreenPromise = opts.offscreenPromise ?? Promise.resolve(); + this.getRequestAccountTabIds = opts.getRequestAccountTabIds; this.getOpenMetamaskTabsIds = opts.getOpenMetamaskTabsIds; @@ -4069,6 +4071,10 @@ export default class MetamaskController extends EventEmitter { */ async submitPassword(password) { const { completedOnboarding } = this.onboardingController.store.getState(); + + // Before attempting to unlock the keyrings, we need the offscreen to have loaded. + await this.offscreenPromise; + await this.keyringController.submitPassword(password); ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) diff --git a/app/scripts/offscreen.js b/app/scripts/offscreen.js new file mode 100644 index 000000000000..ba796874f2fc --- /dev/null +++ b/app/scripts/offscreen.js @@ -0,0 +1,44 @@ +import { OffscreenCommunicationTarget } from '../../shared/constants/offscreen-communication'; + +/** + * Creates an offscreen document that can be used to load additional scripts + * and iframes that can communicate with the extension through the chrome + * runtime API. Only one offscreen document may exist, so any iframes required + * by extension can be embedded in the offscreen.html file. See the offscreen + * folder for more details. + */ +export async function createOffscreen() { + const { chrome } = globalThis; + if (!chrome.offscreen || (await chrome.offscreen.hasDocument())) { + return; + } + + const loadPromise = new Promise((resolve) => { + const messageListener = (msg) => { + if ( + msg.target === OffscreenCommunicationTarget.extensionMain && + msg.isBooted + ) { + chrome.runtime.onMessage.removeListener(messageListener); + resolve(); + } + }; + chrome.runtime.onMessage.addListener(messageListener); + }); + + await chrome.offscreen.createDocument({ + url: './offscreen.html', + reasons: ['IFRAME_SCRIPTING'], + justification: + 'Used for Hardware Wallet and Snaps scripts to communicate with the extension.', + }); + + // In case we are in a bad state where the offscreen document is not loading, timeout and let execution continue. + const timeoutPromise = new Promise((resolve) => { + setTimeout(resolve, 5000); + }); + + await Promise.race([loadPromise, timeoutPromise]); + + console.debug('Offscreen iframe loaded'); +} diff --git a/offscreen/scripts/offscreen.ts b/offscreen/scripts/offscreen.ts index 7eddac9cb840..654c99151c28 100644 --- a/offscreen/scripts/offscreen.ts +++ b/offscreen/scripts/offscreen.ts @@ -1,5 +1,6 @@ import { BrowserRuntimePostMessageStream } from '@metamask/post-message-stream'; import { ProxySnapExecutor } from '@metamask/snaps-execution-environments'; +import { OffscreenCommunicationTarget } from '../../shared/constants/offscreen-communication'; import initLedger from './ledger'; import initTrezor from './trezor'; import initLattice from './lattice'; @@ -20,3 +21,8 @@ const parentStream = new BrowserRuntimePostMessageStream({ }); ProxySnapExecutor.initialize(parentStream, './snaps/index.html'); + +chrome.runtime.sendMessage({ + target: OffscreenCommunicationTarget.extensionMain, + isBooted: true, +}); diff --git a/shared/constants/offscreen-communication.ts b/shared/constants/offscreen-communication.ts index cab4232c09dc..618239609956 100644 --- a/shared/constants/offscreen-communication.ts +++ b/shared/constants/offscreen-communication.ts @@ -7,6 +7,7 @@ export enum OffscreenCommunicationTarget { ledgerOffscreen = 'ledger-offscreen', latticeOffscreen = 'lattice-offscreen', extension = 'extension-offscreen', + extensionMain = 'extension', } /** From 7e03b4d61fcfe756a9b8fb4b3b58a759bdad586c Mon Sep 17 00:00:00 2001 From: Matteo Scurati Date: Tue, 18 Jun 2024 14:06:25 +0200 Subject: [PATCH 6/8] fix: logic for using notifications tabs (#25350) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR introduces two small fixes: - If there are no snaps with notifications, the tabs for filtering results are not displayed on the notifications page. - Feature announcements are included in the onChain tab and not in the Web3 tab. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25350?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **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/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** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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/notifications/notifications.test.tsx | 2 + ui/pages/notifications/notifications.tsx | 95 ++++++++++--------- 2 files changed, 51 insertions(+), 46 deletions(-) diff --git a/ui/pages/notifications/notifications.test.tsx b/ui/pages/notifications/notifications.test.tsx index 61f793775f34..fc7675a85dea 100644 --- a/ui/pages/notifications/notifications.test.tsx +++ b/ui/pages/notifications/notifications.test.tsx @@ -4,6 +4,7 @@ import { Provider } from 'react-redux'; import { MemoryRouter } from 'react-router-dom'; import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; +import mockState from '../../../test/data/mock-state.json'; import Notifications from './notifications'; const mockDispatch = jest.fn(); @@ -32,6 +33,7 @@ jest.mock('../../store/actions', () => ({ const initialState = { metamask: { + ...mockState.metamask, theme: 'light', isMetamaskNotificationsEnabled: true, isFeatureAnnouncementsEnabled: true, diff --git a/ui/pages/notifications/notifications.tsx b/ui/pages/notifications/notifications.tsx index 138011fe5124..5b5d89e6645a 100644 --- a/ui/pages/notifications/notifications.tsx +++ b/ui/pages/notifications/notifications.tsx @@ -16,7 +16,7 @@ import { import { NotificationsPage } from '../../components/multichain'; import { Content, Header } from '../../components/multichain/pages/page'; import { useMetamaskNotificationsContext } from '../../contexts/metamask-notifications/metamask-notifications'; -import { getNotifications } from '../../selectors'; +import { getNotifications, getNotifySnaps } from '../../selectors'; import { selectIsFeatureAnnouncementsEnabled, selectIsMetamaskNotificationsEnabled, @@ -142,17 +142,15 @@ const filterNotifications = ( } if (activeTab === TAB_KEYS.WALLET) { - return notifications.filter((notification) => - TRIGGER_TYPES_WALLET_SET.has(notification.type), + return notifications.filter( + (notification) => + TRIGGER_TYPES_WALLET_SET.has(notification.type) || + notification.type === TRIGGER_TYPES.FEATURES_ANNOUNCEMENT, ); } if (activeTab === TAB_KEYS.WEB3) { - return notifications.filter( - (notification) => - notification.type === TRIGGER_TYPES.FEATURES_ANNOUNCEMENT || - notification.type === 'SNAP', - ); + return notifications.filter((notification) => notification.type === 'SNAP'); } return notifications; @@ -172,6 +170,9 @@ export default function Notifications() { [activeTab, combinedNotifications], ); + let hasNotifySnaps = false; + hasNotifySnaps = useSelector(getNotifySnaps).length > 0; + return ( {/* Back and Settings Buttons */} @@ -202,44 +203,46 @@ export default function Notifications() { > {t('notifications')} - - setActiveTab(tab)} - tabsClassName="notifications__tabs" - > - - - {t('wallet')} - - - } - tabKey={TAB_KEYS.WALLET} - > - - + + {hasNotifySnaps && ( + setActiveTab(tab)} + tabsClassName="notifications__tabs" + > + + + {t('wallet')} + + + } + tabKey={TAB_KEYS.WALLET} + > + + + )} Date: Tue, 18 Jun 2024 14:14:24 +0200 Subject: [PATCH 7/8] feat: Update the counter over the fox to handle notifications (#25093) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR introduces a new type of counter over the Fox icon to display the number of unread notifications within the extension. This new counter interacts with the existing counter for unapproved transactions in the following way: - If there are unapproved transactions, a blue badge with a label containing three dots is displayed - If there are unread notifications, a red badge with a label containing the number of unread notifications is displayed. If there are more than 10 notifications, the label shows "9+" The two badges are not shown together, and the badge for unapproved transactions takes priority over the badge for unread notifications. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25093?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** In the video, I demonstrate how to test this new feature. ## **Screenshots/Recordings** https://www.loom.com/share/9a32ec2bd02447e8a29adc4ac1cd3522?sid=d1f74163-54f0-4513-948c-1faa158bd601 ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask 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** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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/scripts/background.js | 120 +++++++++++++++--- .../metamask-notifications.ts | 31 ++++- app/scripts/metamask-controller.js | 5 + 3 files changed, 136 insertions(+), 20 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 20d4fe885a6e..263fc5c86c63 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -71,6 +71,15 @@ import { createOffscreen } from './offscreen'; /* eslint-enable import/first */ +import { TRIGGER_TYPES } from './controllers/metamask-notifications/constants/notification-schema'; + +// eslint-disable-next-line @metamask/design-tokens/color-no-hex +const BADGE_COLOR_APPROVAL = '#0376C9'; +// eslint-disable-next-line @metamask/design-tokens/color-no-hex +const BADGE_COLOR_NOTIFICATION = '#D73847'; +const BADGE_LABEL_APPROVAL = '\u22EF'; // unicode ellipsis +const BADGE_MAX_NOTIFICATION_COUNT = 9; + // Setup global hook for improved Sentry state snapshots during initialization const inTest = process.env.IN_TEST; const localStore = inTest ? new ReadOnlyNetworkStore() : new LocalStore(); @@ -781,6 +790,21 @@ export function setupController( updateBadge, ); + controller.controllerMessenger.subscribe( + METAMASK_CONTROLLER_EVENTS.METAMASK_NOTIFICATIONS_LIST_UPDATED, + updateBadge, + ); + + controller.controllerMessenger.subscribe( + METAMASK_CONTROLLER_EVENTS.METAMASK_NOTIFICATIONS_MARK_AS_READ, + updateBadge, + ); + + controller.controllerMessenger.subscribe( + METAMASK_CONTROLLER_EVENTS.NOTIFICATIONS_STATE_CHANGE, + updateBadge, + ); + controller.txController.initApprovals(); /** @@ -788,30 +812,90 @@ export function setupController( * The number reflects the current number of pending transactions or message signatures needing user approval. */ function updateBadge() { + const pendingApprovalCount = getPendingApprovalCount(); + const unreadNotificationsCount = getUnreadNotificationsCount(); + let label = ''; - const count = getUnapprovedTransactionCount(); - if (count) { - label = String(count); + let badgeColor = BADGE_COLOR_APPROVAL; + + if (pendingApprovalCount) { + label = BADGE_LABEL_APPROVAL; + } else if (unreadNotificationsCount > 0) { + label = + unreadNotificationsCount > BADGE_MAX_NOTIFICATION_COUNT + ? `${BADGE_MAX_NOTIFICATION_COUNT}+` + : String(unreadNotificationsCount); + badgeColor = BADGE_COLOR_NOTIFICATION; } - // browserAction has been replaced by action in MV3 - if (isManifestV3) { - browser.action.setBadgeText({ text: label }); - browser.action.setBadgeBackgroundColor({ color: '#037DD6' }); - } else { - browser.browserAction.setBadgeText({ text: label }); - browser.browserAction.setBadgeBackgroundColor({ color: '#037DD6' }); + + try { + const badgeText = { text: label }; + const badgeBackgroundColor = { color: badgeColor }; + + if (isManifestV3) { + browser.action.setBadgeText(badgeText); + browser.action.setBadgeBackgroundColor(badgeBackgroundColor); + } else { + browser.browserAction.setBadgeText(badgeText); + browser.browserAction.setBadgeBackgroundColor(badgeBackgroundColor); + } + } catch (error) { + console.error('Error updating browser badge:', error); } } - function getUnapprovedTransactionCount() { - let count = - controller.appStateController.waitingForUnlock.length + - controller.approvalController.getTotalApprovalCount(); + function getPendingApprovalCount() { + try { + let pendingApprovalCount = + controller.appStateController.waitingForUnlock.length + + controller.approvalController.getTotalApprovalCount(); + + if (controller.preferencesController.getUseRequestQueue()) { + pendingApprovalCount += + controller.queuedRequestController.state.queuedRequestCount; + } + return pendingApprovalCount; + } catch (error) { + console.error('Failed to get pending approval count:', error); + return 0; + } + } - if (controller.preferencesController.getUseRequestQueue()) { - count += controller.queuedRequestController.state.queuedRequestCount; + function getUnreadNotificationsCount() { + try { + const { isMetamaskNotificationsEnabled, isFeatureAnnouncementsEnabled } = + controller.metamaskNotificationsController.state; + + const snapNotificationCount = Object.values( + controller.notificationController.state.notifications, + ).filter((notification) => notification.readDate === null).length; + + const featureAnnouncementCount = isFeatureAnnouncementsEnabled + ? controller.metamaskNotificationsController.state.metamaskNotificationsList.filter( + (notification) => + !notification.isRead && + notification.type === TRIGGER_TYPES.FEATURES_ANNOUNCEMENT, + ).length + : 0; + + const walletNotificationCount = isMetamaskNotificationsEnabled + ? controller.metamaskNotificationsController.state.metamaskNotificationsList.filter( + (notification) => + !notification.isRead && + notification.type !== TRIGGER_TYPES.FEATURES_ANNOUNCEMENT, + ).length + : 0; + + const unreadNotificationsCount = + snapNotificationCount + + featureAnnouncementCount + + walletNotificationCount; + + return unreadNotificationsCount; + } catch (error) { + console.error('Failed to get unread notifications count:', error); + return 0; } - return count; } notificationManager.on( @@ -819,7 +903,7 @@ export function setupController( ({ automaticallyClosed }) => { if (!automaticallyClosed) { rejectUnapprovedNotifications(); - } else if (getUnapprovedTransactionCount() > 0) { + } else if (getPendingApprovalCount() > 0) { triggerUi(); } diff --git a/app/scripts/controllers/metamask-notifications/metamask-notifications.ts b/app/scripts/controllers/metamask-notifications/metamask-notifications.ts index f9929a049b54..b9ad4572742b 100644 --- a/app/scripts/controllers/metamask-notifications/metamask-notifications.ts +++ b/app/scripts/controllers/metamask-notifications/metamask-notifications.ts @@ -175,6 +175,16 @@ export declare type MetamaskNotificationsControllerSelectIsMetamaskNotifications handler: MetamaskNotificationsController['selectIsMetamaskNotificationsEnabled']; }; +export type MetamaskNotificationsControllerNotificationsListUpdatedEvent = { + type: `${typeof controllerName}:notificationsListUpdated`; + payload: [Notification[]]; +}; + +export type MetamaskNotificationsControllerMarkNotificationsAsRead = { + type: `${typeof controllerName}:markNotificationsAsRead`; + payload: [Notification[]]; +}; + // Messenger Actions export type Actions = | MetamaskNotificationsControllerUpdateMetamaskNotificationsList @@ -209,7 +219,9 @@ export type MetamaskNotificationsControllerMessengerEvents = // Allowed Events export type AllowedEvents = | KeyringControllerStateChangeEvent - | PushPlatformNotificationsControllerOnNewNotificationEvent; + | PushPlatformNotificationsControllerOnNewNotificationEvent + | MetamaskNotificationsControllerNotificationsListUpdatedEvent + | MetamaskNotificationsControllerMarkNotificationsAsRead; // Type for the messenger of MetamaskNotificationsController export type MetamaskNotificationsControllerMessenger = @@ -1004,6 +1016,11 @@ export class MetamaskNotificationsController extends BaseController< state.metamaskNotificationsList = metamaskNotifications; }); + this.messagingSystem.publish( + `${controllerName}:notificationsListUpdated`, + this.state.metamaskNotificationsList, + ); + this.#setIsFetchingMetamaskNotifications(false); return metamaskNotifications; } catch (err) { @@ -1068,7 +1085,7 @@ export class MetamaskNotificationsController extends BaseController< log.warn('Something failed when marking notifications as read', err); } - // Update the state (state is also used on counter & badge) + // Update the state this.update((state) => { const currentReadList = state.metamaskNotificationsReadList; const newReadIds = [...featureAnnouncementNotificationIds]; @@ -1088,6 +1105,12 @@ export class MetamaskNotificationsController extends BaseController< }, ); }); + + // Publish the event + this.messagingSystem.publish( + `${controllerName}:markNotificationsAsRead`, + this.state.metamaskNotificationsList, + ); } /** @@ -1120,6 +1143,10 @@ export class MetamaskNotificationsController extends BaseController< notification, ...state.metamaskNotificationsList, ]; + this.messagingSystem.publish( + `${controllerName}:notificationsListUpdated`, + state.metamaskNotificationsList, + ); } }); } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 19c3fc67cfb4..0b902939e962 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -341,6 +341,11 @@ export const METAMASK_CONTROLLER_EVENTS = { // TODO: Add this and similar enums to the `controllers` repo and export them APPROVAL_STATE_CHANGE: 'ApprovalController:stateChange', QUEUED_REQUEST_STATE_CHANGE: 'QueuedRequestController:stateChange', + METAMASK_NOTIFICATIONS_LIST_UPDATED: + 'MetamaskNotificationsController:notificationsListUpdated', + METAMASK_NOTIFICATIONS_MARK_AS_READ: + 'MetamaskNotificationsController:markNotificationsAsRead', + NOTIFICATIONS_STATE_CHANGE: 'NotificationController:stateChange', }; // stream channels From f6890a711ae9c834cbb360bad2ea5959a9f90389 Mon Sep 17 00:00:00 2001 From: Danica Shen Date: Tue, 18 Jun 2024 14:14:06 +0100 Subject: [PATCH 8/8] fix(25335): fix flaky test "Unlock wallet should send first three Page metric events upon fullscreen page load" (#25351) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The flakyness comes from unordered events from time to time. Based on the log of events when unlock wallet: 截屏2024-06-14 22 33 12 The `events` array is not sorted, and there is nothing wrong with metrics itself and the order of sending, which can be validated by `timestamp`. Therefore the fix is to order events by `timestamp` and assert the value. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25351?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/25335 ## **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/develop/.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/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. --- test/e2e/tests/metrics/unlock-wallet.spec.js | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/test/e2e/tests/metrics/unlock-wallet.spec.js b/test/e2e/tests/metrics/unlock-wallet.spec.js index 3961bb3a94ce..2e4b9f5ed3eb 100644 --- a/test/e2e/tests/metrics/unlock-wallet.spec.js +++ b/test/e2e/tests/metrics/unlock-wallet.spec.js @@ -40,15 +40,26 @@ describe('Unlock wallet', function () { await unlockWallet(driver); await waitForAccountRendered(driver); const events = await getEventPayloads(driver, mockedEndpoint); - assert.equal(events.length, 3); - assertBatchValue(events[0], 'Home', '/'); - assertBatchValue(events[1], 'Unlock Page', '/unlock'); - assertBatchValue(events[2], 'Home', '/'); + const sortedEvents = sortEventsByTime(events); + await assert.equal(sortedEvents.length, 3); + assertBatchValue(sortedEvents[0], 'Home', '/'); + assertBatchValue(sortedEvents[1], 'Unlock Page', '/unlock'); + assertBatchValue(sortedEvents[2], 'Home', '/'); }, ); }); }); +function sortEventsByTime(events) { + events.sort((event1, event2) => { + const timestamp1 = new Date(event1.timestamp); + const timestamp2 = new Date(event2.timestamp); + // Compare timestamps, return -1 for earlier, 1 for later, 0 for equal + return timestamp1 - timestamp2; + }); + return events; +} + function assertBatchValue(event, assertedTitle, assertedPath) { const { title, path } = event.context.page; assert.equal(title, assertedTitle);