From 78c00b3f4501ddfe598b7bbe7a3796ebfea6a59f Mon Sep 17 00:00:00 2001 From: Salim TOUBAL Date: Wed, 8 Jan 2025 14:13:14 +0100 Subject: [PATCH 01/23] fix: add kaia network logo (#29494) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR is to update the Klaytn details to Kaia because of rebranding changes of the network. Please find more details here. https://www.binance.com/en/support/announcement/binance-will-support-the-kaia-klay-rebranding-to-kaia-kaia-f75f933759ee49d0af1dfbce7e32144c?hl=en https://medium.com/klaytn/say-hello-to-kaia-4182ccafe456 https://kaia.io [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27383?quickstart=1) ## **Related issues** https://github.com/MetaMask/metamask-extension/issues/27382 Fixes: ## **Manual testing steps** 1. Go to this networks section. 2. Add kaia network from 3. https://chainlist.org/?search=kaia 4. Currently it shows the Klaytn logo and also shows for KAIA ticker. ## **Screenshots/Recordings** ### **Before** Screenshot 2024-09-25 at 2 59 30 PM ### **After** Once updated it should show with new Kaia logo. ## **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 - [ ] 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** - [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/images/kaia.svg | 9 ++++++++ app/images/klaytn.svg | 45 ------------------------------------- shared/constants/network.ts | 12 +++++----- 3 files changed, 15 insertions(+), 51 deletions(-) create mode 100644 app/images/kaia.svg delete mode 100644 app/images/klaytn.svg diff --git a/app/images/kaia.svg b/app/images/kaia.svg new file mode 100644 index 000000000000..88a10cd86e68 --- /dev/null +++ b/app/images/kaia.svg @@ -0,0 +1,9 @@ + + image + + + + + + diff --git a/app/images/klaytn.svg b/app/images/klaytn.svg deleted file mode 100644 index 4eb1ff5f107b..000000000000 --- a/app/images/klaytn.svg +++ /dev/null @@ -1,45 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/shared/constants/network.ts b/shared/constants/network.ts index 124fcd5eedd2..38e687e180ce 100644 --- a/shared/constants/network.ts +++ b/shared/constants/network.ts @@ -203,7 +203,7 @@ export const CHAINLIST_CHAIN_IDS_MAP = { HAQQ_NETWORK: '0x2be3', IOTEX_MAINNET: '0x1251', KCC_MAINNET: '0x141', - KLAYTN_MAINNET_CYPRESS: '0x2019', + KAIA_MAINNET: '0x2019', KROMA_MAINNET: '0xff', LIGHTLINK_PHOENIX_MAINNET: '0x762', MANTA_PACIFIC_MAINNET: '0xa9', @@ -375,7 +375,7 @@ const CHAINLIST_CURRENCY_SYMBOLS_MAP = { NEAR_AURORA_MAINNET: 'ETH', KROMA_MAINNET: 'ETH', NEBULA_MAINNET: 'sFUEL', - KLAYTN_MAINNET_CYPRESS: 'KLAY', + KAIA_MAINNET: 'KAIA', ENDURANCE_SMART_CHAIN_MAINNET: 'ACE', CRONOS_MAINNET_BETA: 'CRO', FLARE_MAINNET: 'FLR', @@ -453,7 +453,7 @@ export const IOTEX_MAINNET_IMAGE_URL = './images/iotex.svg'; export const IOTEX_TOKEN_IMAGE_URL = './images/iotex-token.svg'; export const APE_TOKEN_IMAGE_URL = './images/ape-token.svg'; export const KCC_MAINNET_IMAGE_URL = './images/kcc-mainnet.svg'; -export const KLAYTN_MAINNET_IMAGE_URL = './images/klaytn.svg'; +export const KAIA_MAINNET_IMAGE_URL = './images/kaia.svg'; export const KROMA_MAINNET_IMAGE_URL = './images/kroma.svg'; export const LIGHT_LINK_IMAGE_URL = './images/lightlink.svg'; export const MANTA_PACIFIC_MAINNET_IMAGE_URL = './images/manta.svg'; @@ -663,8 +663,8 @@ export const CHAIN_ID_TO_CURRENCY_SYMBOL_MAP = { CHAINLIST_CURRENCY_SYMBOLS_MAP.KROMA_MAINNET, [CHAINLIST_CHAIN_IDS_MAP.NEBULA_MAINNET]: CHAINLIST_CURRENCY_SYMBOLS_MAP.NEBULA_MAINNET, - [CHAINLIST_CHAIN_IDS_MAP.KLAYTN_MAINNET_CYPRESS]: - CHAINLIST_CURRENCY_SYMBOLS_MAP.KLAYTN_MAINNET_CYPRESS, + [CHAINLIST_CHAIN_IDS_MAP.KAIA_MAINNET]: + CHAINLIST_CURRENCY_SYMBOLS_MAP.KAIA_MAINNET, [CHAINLIST_CHAIN_IDS_MAP.MOONRIVER]: CHAINLIST_CURRENCY_SYMBOLS_MAP.MOONRIVER, [CHAINLIST_CHAIN_IDS_MAP.ENDURANCE_SMART_CHAIN_MAINNET]: CHAINLIST_CURRENCY_SYMBOLS_MAP.ENDURANCE_SMART_CHAIN_MAINNET, @@ -798,7 +798,7 @@ export const CHAIN_ID_TO_NETWORK_IMAGE_URL_MAP = { [CHAINLIST_CHAIN_IDS_MAP.IOTEX_MAINNET]: IOTEX_MAINNET_IMAGE_URL, [CHAINLIST_CHAIN_IDS_MAP.HAQQ_NETWORK]: HAQQ_NETWORK_IMAGE_URL, [CHAINLIST_CHAIN_IDS_MAP.KCC_MAINNET]: KCC_MAINNET_IMAGE_URL, - [CHAINLIST_CHAIN_IDS_MAP.KLAYTN_MAINNET_CYPRESS]: KLAYTN_MAINNET_IMAGE_URL, + [CHAINLIST_CHAIN_IDS_MAP.KAIA_MAINNET]: KAIA_MAINNET_IMAGE_URL, [CHAINLIST_CHAIN_IDS_MAP.KROMA_MAINNET]: KROMA_MAINNET_IMAGE_URL, [CHAINLIST_CHAIN_IDS_MAP.LIGHTLINK_PHOENIX_MAINNET]: LIGHT_LINK_IMAGE_URL, [CHAINLIST_CHAIN_IDS_MAP.MANTA_PACIFIC_MAINNET]: From 20b417c84f7d9d29593f89d7eeb367261fb8f669 Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Wed, 8 Jan 2025 14:37:34 +0100 Subject: [PATCH 02/23] fix: Bump smart-transactions-controller to ^16.0.1 (#29478) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Release notes for 16.0.1: https://github.com/MetaMask/smart-transactions-controller/releases/tag/v16.0.1 ## **Related issues** Fixes: ## **Manual testing steps** ## **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. --- package.json | 2 +- yarn.lock | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 8bcb1985e69a..40d8f4845635 100644 --- a/package.json +++ b/package.json @@ -341,7 +341,7 @@ "@metamask/scure-bip39": "^2.0.3", "@metamask/selected-network-controller": "^19.0.0", "@metamask/signature-controller": "^23.1.0", - "@metamask/smart-transactions-controller": "^16.0.0", + "@metamask/smart-transactions-controller": "^16.0.1", "@metamask/snaps-controllers": "^9.16.0", "@metamask/snaps-execution-environments": "^6.11.0", "@metamask/snaps-rpc-methods": "^11.8.0", diff --git a/yarn.lock b/yarn.lock index f45eb5cf233e..7f988937d1bb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6237,9 +6237,9 @@ __metadata: languageName: node linkType: hard -"@metamask/smart-transactions-controller@npm:^16.0.0": - version: 16.0.0 - resolution: "@metamask/smart-transactions-controller@npm:16.0.0" +"@metamask/smart-transactions-controller@npm:^16.0.1": + version: 16.0.1 + resolution: "@metamask/smart-transactions-controller@npm:16.0.1" dependencies: "@babel/runtime": "npm:^7.24.1" "@ethereumjs/tx": "npm:^5.2.1" @@ -6261,7 +6261,7 @@ __metadata: optional: true "@metamask/approval-controller": optional: true - checksum: 10/5bfe2e459ca75b3de31f28213e908f389a6a7e82b45dd43ee7fd4c46a619a561da030f7f90426af03063ff5dcfc90269a155230db484a2c415db11f8ad8caa02 + checksum: 10/9a4dba47e01c1e47f099faa74a9fdf3378c77e8ba6d699317f9b38df1d71893e5278d210f6f9cd0ff6d2641db502991f48f923e00847410be33ae1df0f69377c languageName: node linkType: hard @@ -26674,7 +26674,7 @@ __metadata: "@metamask/scure-bip39": "npm:^2.0.3" "@metamask/selected-network-controller": "npm:^19.0.0" "@metamask/signature-controller": "npm:^23.1.0" - "@metamask/smart-transactions-controller": "npm:^16.0.0" + "@metamask/smart-transactions-controller": "npm:^16.0.1" "@metamask/snaps-controllers": "npm:^9.16.0" "@metamask/snaps-execution-environments": "npm:^6.11.0" "@metamask/snaps-rpc-methods": "npm:^11.8.0" From ae565bc4518fd3d0ee3831088dfddbbb0fe266c1 Mon Sep 17 00:00:00 2001 From: Zbyszek Tenerowicz Date: Wed, 8 Jan 2025 19:45:53 +0100 Subject: [PATCH 03/23] chore: add the new policy.json review process documentation and config (#29383) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Introducing the document outlining policy review process. The current change to CODEOWNERS doesn't remove the all devs group - this makes it a soft-launch of the process where a specific approving review is not yet mandatory. We'll be releasing a training video early January. After merging this, I'm hoping to reference the document in multiple places to aid people in discovering and following the process. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29383?quickstart=1) ## **Related issues** Fixes: lack of attention to policy updates ## **Manual testing steps** 1. Follow the process and experience joy ## **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** - [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. --------- Co-authored-by: Mark Stacey --- .github/CODEOWNERS | 2 +- docs/lavamoat-policy-review-process.md | 29 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 docs/lavamoat-policy-review-process.md diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index ad36b9b96efd..bb1055ada8f9 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -13,7 +13,7 @@ # audit these changes on their own, and leave their analysis in a comment. # These codeowners will review this analysis, and review the policy changes in # further detail if warranted. -lavamoat/ @MetaMask/extension-devs @MetaMask/supply-chain +lavamoat/ @MetaMask/extension-devs @MetaMask/policy-reviewers @MetaMask/supply-chain # The offscreen.ts script file that is included in the offscreen document html # file is responsible, at present, for loading the snaps execution environment diff --git a/docs/lavamoat-policy-review-process.md b/docs/lavamoat-policy-review-process.md new file mode 100644 index 000000000000..097154e6c268 --- /dev/null +++ b/docs/lavamoat-policy-review-process.md @@ -0,0 +1,29 @@ +# LavaMoat Policy Review process in metamask-extension + +When there's a need to change policy (because of new or updated packages that require a different set of capabilities), please follow these steps: + +> In the initial soft-launch of the process the approval from policy reviewers is not mandatory, but it will be in the near future. + +### Engineer on the dev team: + 1. Notice the `Validate lavamoat policy*` PR status check fail because dependency updates or changes need a policy update. + 2. (optional) Generate an updated policy and give it a cursory look in local development whenever you’re testing the change. + - If you're confident your update is complete, you can push it to the PR branch. + 3. To generate a complete set of new policies, call `metamaskbot` for help: + - put `@metamaskbot update-policies` in a comment on the PR. When it produces changes, they need to be reviewed. The following steps assume update-policies produced changes. + - *Note the response from the bot points to instructions on how to review the policy for your convenience. https://lavamoat.github.io/guides/policy-diff/* + 4. Analyze the diff of policy.json and use the understanding of the codebase and change being made to decide whether the capabilities added make sense in that context. Leave a comment listing any doubts you have with brief explanations or if everything is in order \- saying so and explaining why the most powerful capabilities are there (like access to child_process.exec in node or network access in the browser) + *Remember the Security Reviewer comes with more security expertise but less intimate knowledge of the codebase and the change you’ve built, so you are the most qualified to know whether the dependency needs the powers detected by LavaMoat or not.* + - You can use these questions to guide your analysis: + 1. What new powers (globals and builtins) do you see? Why should the package be allowed to use these new powers? Explain if possible + 2. What new packages do you see? Did you intend to introduce them? If you didn’t, which package did? (can you see them in `packages` field in policy of any other package that you updated or introduced?) + - The comment is mandatory even if you don’t understand the policy change, in which case you’re expected to state so (it’s ok to not understand) + - Note: this could be enforced by a job that is only passing if the comment was made + - Note: we’d introduce more tooling to summarize and analyze policy and post that as a comment on the PR + 5. Mention `policy-reviewers` group in your comment. + policy-reviewers group includes security liaisons and their involvement is not limited to (but likely focused more around) their respective teams’ PRs. + +### L1 Security Reviewer: + 1. Look at the policy and the comment from the Developer. Approve the PR if they match and the policy change seems safe. Address questions the Developer had and discuss if the policy change doesn’t seem right. + 2. If changes are hard to explain or seem dangerous, escalate to a review of the dependency and its powers by mentioning `supply-chain` +### (optional) L2 Security Reviewer: + 1. Review the dependency in question and inform the PR reviewers whether it’s deemed malicious or safe. From ed0362c167323f9e7c079a65a784315cc4cb7f8f Mon Sep 17 00:00:00 2001 From: David Drazic Date: Wed, 8 Jan 2025 20:58:14 +0100 Subject: [PATCH 04/23] fix(snaps): Ensure that adjacent form elements take up to 50% width (#29436) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Add some SCSS changes to ensure that adjacent form elements take up to 50% width in Snaps custom UI. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29436?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/snaps/issues/2948 ## **Manual testing steps** 1. Install a Snap with custom UI with adjacent form elements like in the example code provided. 2. Make sure that adjacent form elements take up to 50% of the space available. Snaps JSX used for testing: ```typescript
Option 1 Option 2 Option 3 Option 11 Option 22 Option 33
``` ## **Screenshots/Recordings** ### **Before** ![Screenshot 2025-01-06 at 13 16 47](https://github.com/user-attachments/assets/2c1ade00-a5e3-42e8-8619-e6d59262c1fb) ### **After** ![Screenshot 2025-01-08 at 13 45 12](https://github.com/user-attachments/assets/805e67aa-8b43-43fb-a0b5-a0bcbc038d05) ![Screenshot 2025-01-08 at 13 45 31](https://github.com/user-attachments/assets/a7685bc8-c014-41ee-acaa-6987da4c7bb1) ![Screenshot 2025-01-08 at 13 46 51](https://github.com/user-attachments/assets/ef2670a8-1c68-43e3-ae6b-0fcb6ebe9d39) ![Screenshot 2025-01-08 at 13 47 05](https://github.com/user-attachments/assets/585bba34-eaaf-4ddc-9634-a6226f5b5969) ## **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. --- .../snaps/snap-ui-checkbox/snap-ui-checkbox.tsx | 5 ++++- .../snaps/snap-ui-dropdown/snap-ui-dropdown.tsx | 5 ++++- .../snap-ui-file-input/snap-ui-file-input.tsx | 16 ++++++++++++++-- .../app/snaps/snap-ui-input/snap-ui-input.tsx | 9 +++++++-- .../snap-ui-radio-group/snap-ui-radio-group.tsx | 5 ++++- .../app/snaps/snap-ui-renderer/index.scss | 8 ++++++++ .../snaps/snap-ui-selector/snap-ui-selector.tsx | 9 ++++++++- 7 files changed, 49 insertions(+), 8 deletions(-) diff --git a/ui/components/app/snaps/snap-ui-checkbox/snap-ui-checkbox.tsx b/ui/components/app/snaps/snap-ui-checkbox/snap-ui-checkbox.tsx index 0d85f8ef60dc..9c2606a2c77f 100644 --- a/ui/components/app/snaps/snap-ui-checkbox/snap-ui-checkbox.tsx +++ b/ui/components/app/snaps/snap-ui-checkbox/snap-ui-checkbox.tsx @@ -1,4 +1,5 @@ import React, { FunctionComponent, useEffect, useState } from 'react'; +import classnames from 'classnames'; import { useSnapInterfaceContext } from '../../../../contexts/snaps'; import { BorderColor, @@ -51,7 +52,9 @@ export const SnapUICheckbox: FunctionComponent = ({ return ( diff --git a/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx b/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx index f2cb85cc4ef0..835734b2bf87 100644 --- a/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx +++ b/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx @@ -1,4 +1,5 @@ import React, { FunctionComponent, useEffect, useState } from 'react'; +import classnames from 'classnames'; import { useSnapInterfaceContext } from '../../../../contexts/snaps'; import { Display, @@ -46,7 +47,9 @@ export const SnapUIDropdown: FunctionComponent = ({ return ( diff --git a/ui/components/app/snaps/snap-ui-file-input/snap-ui-file-input.tsx b/ui/components/app/snaps/snap-ui-file-input/snap-ui-file-input.tsx index 1d254ccaa2e3..cfab068ab6db 100644 --- a/ui/components/app/snaps/snap-ui-file-input/snap-ui-file-input.tsx +++ b/ui/components/app/snaps/snap-ui-file-input/snap-ui-file-input.tsx @@ -146,7 +146,13 @@ export const SnapUIFileInput: FunctionComponent = ({ if (compact) { return ( - + {header} = ({ } return ( - + {header} -> = ({ name, form, ...props }) => { +> = ({ name, form, label, ...props }) => { const { handleInputChange, getValue, focusedInput, setCurrentFocusedInput } = useSnapInterfaceContext(); @@ -54,10 +56,13 @@ export const SnapUIInput: FunctionComponent< ref={inputRef} onFocus={handleFocus} onBlur={handleBlur} - className="snap-ui-renderer__input" + className={classnames('snap-ui-renderer__input', { + 'snap-ui-renderer__field': label !== undefined, + })} id={name} value={value} onChange={handleChange} + label={label} {...props} /> ); diff --git a/ui/components/app/snaps/snap-ui-radio-group/snap-ui-radio-group.tsx b/ui/components/app/snaps/snap-ui-radio-group/snap-ui-radio-group.tsx index 4563fbc02fe3..185eed8bddc9 100644 --- a/ui/components/app/snaps/snap-ui-radio-group/snap-ui-radio-group.tsx +++ b/ui/components/app/snaps/snap-ui-radio-group/snap-ui-radio-group.tsx @@ -1,4 +1,5 @@ import React, { FunctionComponent, useEffect, useState } from 'react'; +import classnames from 'classnames'; import { useSnapInterfaceContext } from '../../../../contexts/snaps'; import { AlignItems, @@ -75,7 +76,9 @@ export const SnapUIRadioGroup: FunctionComponent = ({ return ( diff --git a/ui/components/app/snaps/snap-ui-renderer/index.scss b/ui/components/app/snaps/snap-ui-renderer/index.scss index 1b027890b247..cbd4258e1d9d 100644 --- a/ui/components/app/snaps/snap-ui-renderer/index.scss +++ b/ui/components/app/snaps/snap-ui-renderer/index.scss @@ -84,4 +84,12 @@ max-width: $width-screen-lg-min; } } + + &__form { + .snap-ui-renderer__panel { + .snap-ui-renderer__field { + flex: 1 1 50%; // Ensure that adjacent form elements take up to 50% width + } + } + } } diff --git a/ui/components/app/snaps/snap-ui-selector/snap-ui-selector.tsx b/ui/components/app/snaps/snap-ui-selector/snap-ui-selector.tsx index e5722cfd763d..6f844dc29425 100644 --- a/ui/components/app/snaps/snap-ui-selector/snap-ui-selector.tsx +++ b/ui/components/app/snaps/snap-ui-selector/snap-ui-selector.tsx @@ -3,6 +3,7 @@ import React, { useEffect, MouseEvent as ReactMouseEvent, } from 'react'; +import classnames from 'classnames'; import { Box, ButtonBase, @@ -128,7 +129,13 @@ export const SnapUISelector: React.FunctionComponent = ({ return ( <> - + {label && } Date: Thu, 9 Jan 2025 09:11:53 +0100 Subject: [PATCH 05/23] test: fix flaky snap signature test (#29480) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29480?quickstart=1) ## **Related issues** Fixes: [#29380](https://github.com/MetaMask/metamask-extension/issues/29380) ## **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. --- test/e2e/helpers.js | 2 +- test/e2e/snaps/test-snap-siginsights.spec.js | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index 24240d6f609a..40e7258ea4a1 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -722,7 +722,7 @@ async function clickSignOnSignatureConfirmation({ if (snapSigInsights) { // there is no condition we can wait for to know the snap is ready, // so we have to add a small delay as the last alternative to avoid flakiness. - await driver.delay(regularDelayMs); + await driver.delay(largeDelayMs); } await driver.waitForSelector( { text: 'Sign', tag: 'button' }, diff --git a/test/e2e/snaps/test-snap-siginsights.spec.js b/test/e2e/snaps/test-snap-siginsights.spec.js index c463d09d864a..3f90b7d56e28 100644 --- a/test/e2e/snaps/test-snap-siginsights.spec.js +++ b/test/e2e/snaps/test-snap-siginsights.spec.js @@ -175,8 +175,13 @@ describe('Test Snap Signature Insights', function () { // switch back to MetaMask window and switch to tx insights pane await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); + await driver.waitForSelector( + '[data-testid="signature-request-scroll-button"]', + ); // click down arrow - await driver.clickElementSafe('.fa-arrow-down'); + await driver.clickElementSafe( + '[data-testid="signature-request-scroll-button"]', + ); // wait for and click sign await clickSignOnSignatureConfirmation({ @@ -223,7 +228,12 @@ describe('Test Snap Signature Insights', function () { await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); // click down arrow - await driver.clickElementSafe('.fa-arrow-down'); + await driver.waitForSelector( + '[data-testid="signature-request-scroll-button"]', + ); + await driver.clickElementSafe( + '[data-testid="signature-request-scroll-button"]', + ); // wait for and click sign await clickSignOnSignatureConfirmation({ From 00a5db79f52c2e179f70561a72b2ea3463ba7f88 Mon Sep 17 00:00:00 2001 From: Michele Esposito <34438276+mikesposito@users.noreply.github.com> Date: Thu, 9 Jan 2025 12:50:38 +0100 Subject: [PATCH 06/23] chore(deps): bump `@metamask/eth-trezor-keyring` to `^6.0.0` (#27689) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR bumps `@metamask/eth-trezor-keyring` from `^3.1.3` to `^6.0.0`. ``` ## [6.0.0] ### Added - **BREAKING:** Add ESM build ([#40](https://github.com/MetaMask/accounts/pull/40)) - It's no longer possible to import files from `./dist` directly. ## [5.0.0] ### Changed - **BREAKING**: Bump `@metamask/eth-sig-util` dependency from `^7.0.3` to `^8.0.0` ([#79](https://github.com/MetaMask/accounts/pull/79)) - `signTypedData` no longer support `number` for addresses, see [here](https://github.com/MetaMask/eth-sig-util/blob/main/CHANGELOG.md#800). ## [4.0.0] ### Changed - **BREAKING**: `addAccounts` will now only return newly created accounts ([#64](https://github.com/MetaMask/accounts/pull/64)) - This keyring was initially returning every accounts (previous and new ones), which is different from what is expected in the [`Keyring` interface].(https://github.com/MetaMask/utils/blob/v9.2.1/src/keyring.ts#L65) ``` [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27689?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** These changes directly impact Trezor devices: 1. Add one or more Trezor accounts 2. Sign message 3. Sign typed data 4. Sign transaction 5. Remove Trezor accounts ## **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. --- lavamoat/browserify/beta/policy.json | 65 ++++++++++++++++++++------- lavamoat/browserify/flask/policy.json | 65 ++++++++++++++++++++------- lavamoat/browserify/main/policy.json | 65 ++++++++++++++++++++------- lavamoat/browserify/mmi/policy.json | 65 ++++++++++++++++++++------- package.json | 13 +++++- yarn.lock | 28 ++++++------ 6 files changed, 222 insertions(+), 79 deletions(-) diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index e55d57f5ec0f..430b196d5a7f 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -550,7 +550,7 @@ "console": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/notification-services-controller>firebase>@firebase/messaging": { @@ -584,7 +584,7 @@ "@metamask/notification-services-controller>firebase>@firebase/installations": true, "@metamask/notification-services-controller>firebase>@firebase/util": true, "@metamask/notification-services-controller>firebase>@firebase/app>idb": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/notification-services-controller>firebase>@firebase/util": { @@ -634,7 +634,7 @@ "ethereumjs-util>ethereum-cryptography>bs58check": true, "buffer": true, "browserify>buffer": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@keystonehq/metamask-airgapped-keyring": { @@ -815,6 +815,12 @@ "@metamask/eth-snap-keyring>@metamask/eth-sig-util>@metamask/abi-utils>@metamask/utils": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/abi-utils": { + "packages": { + "@metamask/utils>@metamask/superstruct": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util>@metamask/abi-utils": { "packages": { "@metamask/utils>@metamask/superstruct": true, @@ -1121,6 +1127,17 @@ "@metamask/eth-sig-util>tweetnacl": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": { + "packages": { + "@ethereumjs/tx>@ethereumjs/util": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/abi-utils": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": true, + "@metamask/utils>@scure/base": true, + "browserify>buffer": true, + "@ethereumjs/tx>ethereum-cryptography": true, + "@metamask/eth-sig-util>tweetnacl": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util": { "packages": { "@ethereumjs/tx>@ethereumjs/util": true, @@ -1191,6 +1208,7 @@ "packages": { "@ethereumjs/tx": true, "@ethereumjs/tx>@ethereumjs/util": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": true, "@metamask/eth-trezor-keyring>@trezor/connect-plugin-ethereum": true, "@trezor/connect-web": true, "browserify>buffer": true, @@ -2061,6 +2079,21 @@ "semver": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": { + "globals": { + "TextDecoder": true, + "TextEncoder": true + }, + "packages": { + "@metamask/utils>@metamask/superstruct": true, + "@noble/hashes": true, + "@metamask/utils>@scure/base": true, + "browserify>buffer": true, + "nock>debug": true, + "@metamask/utils>pony-cause": true, + "semver": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util>@metamask/utils": { "globals": { "TextDecoder": true, @@ -2684,13 +2717,13 @@ "packages": { "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils": true, "@trezor/connect-web>@trezor/utils": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/eth-trezor-keyring>@trezor/connect-plugin-ethereum": { "packages": { - "@metamask/eth-sig-util": true, - "@swc/helpers>tslib": true + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": true, + "tslib": true } }, "@trezor/connect-web": { @@ -2721,7 +2754,7 @@ "@trezor/connect-web>@trezor/connect": true, "@trezor/connect-web>@trezor/utils": true, "webpack>events": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect": { @@ -2730,7 +2763,7 @@ "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": true, "@trezor/connect-web>@trezor/connect>@trezor/transport": true, "@trezor/connect-web>@trezor/utils": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils": { @@ -2747,7 +2780,7 @@ }, "packages": { "process": true, - "@swc/helpers>tslib": true, + "tslib": true, "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils>ua-parser-js": true } }, @@ -2756,7 +2789,7 @@ "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": true, "browserify>buffer": true, "@trezor/connect-web>@trezor/connect>@trezor/protobuf>protobufjs": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": { @@ -2786,7 +2819,7 @@ "@trezor/connect-web>@trezor/utils>bignumber.js": true, "browserify>buffer": true, "webpack>events": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@welldone-software/why-did-you-render": { @@ -2899,7 +2932,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/transaction-controller>@metamask/nonce-tracker>async-mutex": { @@ -2908,7 +2941,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "string.prototype.matchall>es-abstract>available-typed-arrays": { @@ -3810,7 +3843,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "browserify>util>which-typed-array>for-each": { @@ -5324,7 +5357,7 @@ "document.getSelection": true } }, - "@swc/helpers>tslib": { + "tslib": { "globals": { "SuppressedError": true, "define": true @@ -5412,7 +5445,7 @@ "packages": { "react-focus-lock>use-sidecar>detect-node-es": true, "react": true, - "@swc/helpers>tslib": true + "tslib": true } }, "readable-stream>util-deprecate": { diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index e55d57f5ec0f..430b196d5a7f 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -550,7 +550,7 @@ "console": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/notification-services-controller>firebase>@firebase/messaging": { @@ -584,7 +584,7 @@ "@metamask/notification-services-controller>firebase>@firebase/installations": true, "@metamask/notification-services-controller>firebase>@firebase/util": true, "@metamask/notification-services-controller>firebase>@firebase/app>idb": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/notification-services-controller>firebase>@firebase/util": { @@ -634,7 +634,7 @@ "ethereumjs-util>ethereum-cryptography>bs58check": true, "buffer": true, "browserify>buffer": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@keystonehq/metamask-airgapped-keyring": { @@ -815,6 +815,12 @@ "@metamask/eth-snap-keyring>@metamask/eth-sig-util>@metamask/abi-utils>@metamask/utils": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/abi-utils": { + "packages": { + "@metamask/utils>@metamask/superstruct": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util>@metamask/abi-utils": { "packages": { "@metamask/utils>@metamask/superstruct": true, @@ -1121,6 +1127,17 @@ "@metamask/eth-sig-util>tweetnacl": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": { + "packages": { + "@ethereumjs/tx>@ethereumjs/util": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/abi-utils": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": true, + "@metamask/utils>@scure/base": true, + "browserify>buffer": true, + "@ethereumjs/tx>ethereum-cryptography": true, + "@metamask/eth-sig-util>tweetnacl": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util": { "packages": { "@ethereumjs/tx>@ethereumjs/util": true, @@ -1191,6 +1208,7 @@ "packages": { "@ethereumjs/tx": true, "@ethereumjs/tx>@ethereumjs/util": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": true, "@metamask/eth-trezor-keyring>@trezor/connect-plugin-ethereum": true, "@trezor/connect-web": true, "browserify>buffer": true, @@ -2061,6 +2079,21 @@ "semver": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": { + "globals": { + "TextDecoder": true, + "TextEncoder": true + }, + "packages": { + "@metamask/utils>@metamask/superstruct": true, + "@noble/hashes": true, + "@metamask/utils>@scure/base": true, + "browserify>buffer": true, + "nock>debug": true, + "@metamask/utils>pony-cause": true, + "semver": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util>@metamask/utils": { "globals": { "TextDecoder": true, @@ -2684,13 +2717,13 @@ "packages": { "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils": true, "@trezor/connect-web>@trezor/utils": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/eth-trezor-keyring>@trezor/connect-plugin-ethereum": { "packages": { - "@metamask/eth-sig-util": true, - "@swc/helpers>tslib": true + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": true, + "tslib": true } }, "@trezor/connect-web": { @@ -2721,7 +2754,7 @@ "@trezor/connect-web>@trezor/connect": true, "@trezor/connect-web>@trezor/utils": true, "webpack>events": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect": { @@ -2730,7 +2763,7 @@ "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": true, "@trezor/connect-web>@trezor/connect>@trezor/transport": true, "@trezor/connect-web>@trezor/utils": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils": { @@ -2747,7 +2780,7 @@ }, "packages": { "process": true, - "@swc/helpers>tslib": true, + "tslib": true, "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils>ua-parser-js": true } }, @@ -2756,7 +2789,7 @@ "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": true, "browserify>buffer": true, "@trezor/connect-web>@trezor/connect>@trezor/protobuf>protobufjs": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": { @@ -2786,7 +2819,7 @@ "@trezor/connect-web>@trezor/utils>bignumber.js": true, "browserify>buffer": true, "webpack>events": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@welldone-software/why-did-you-render": { @@ -2899,7 +2932,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/transaction-controller>@metamask/nonce-tracker>async-mutex": { @@ -2908,7 +2941,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "string.prototype.matchall>es-abstract>available-typed-arrays": { @@ -3810,7 +3843,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "browserify>util>which-typed-array>for-each": { @@ -5324,7 +5357,7 @@ "document.getSelection": true } }, - "@swc/helpers>tslib": { + "tslib": { "globals": { "SuppressedError": true, "define": true @@ -5412,7 +5445,7 @@ "packages": { "react-focus-lock>use-sidecar>detect-node-es": true, "react": true, - "@swc/helpers>tslib": true + "tslib": true } }, "readable-stream>util-deprecate": { diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index e55d57f5ec0f..430b196d5a7f 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -550,7 +550,7 @@ "console": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/notification-services-controller>firebase>@firebase/messaging": { @@ -584,7 +584,7 @@ "@metamask/notification-services-controller>firebase>@firebase/installations": true, "@metamask/notification-services-controller>firebase>@firebase/util": true, "@metamask/notification-services-controller>firebase>@firebase/app>idb": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/notification-services-controller>firebase>@firebase/util": { @@ -634,7 +634,7 @@ "ethereumjs-util>ethereum-cryptography>bs58check": true, "buffer": true, "browserify>buffer": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@keystonehq/metamask-airgapped-keyring": { @@ -815,6 +815,12 @@ "@metamask/eth-snap-keyring>@metamask/eth-sig-util>@metamask/abi-utils>@metamask/utils": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/abi-utils": { + "packages": { + "@metamask/utils>@metamask/superstruct": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util>@metamask/abi-utils": { "packages": { "@metamask/utils>@metamask/superstruct": true, @@ -1121,6 +1127,17 @@ "@metamask/eth-sig-util>tweetnacl": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": { + "packages": { + "@ethereumjs/tx>@ethereumjs/util": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/abi-utils": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": true, + "@metamask/utils>@scure/base": true, + "browserify>buffer": true, + "@ethereumjs/tx>ethereum-cryptography": true, + "@metamask/eth-sig-util>tweetnacl": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util": { "packages": { "@ethereumjs/tx>@ethereumjs/util": true, @@ -1191,6 +1208,7 @@ "packages": { "@ethereumjs/tx": true, "@ethereumjs/tx>@ethereumjs/util": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": true, "@metamask/eth-trezor-keyring>@trezor/connect-plugin-ethereum": true, "@trezor/connect-web": true, "browserify>buffer": true, @@ -2061,6 +2079,21 @@ "semver": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": { + "globals": { + "TextDecoder": true, + "TextEncoder": true + }, + "packages": { + "@metamask/utils>@metamask/superstruct": true, + "@noble/hashes": true, + "@metamask/utils>@scure/base": true, + "browserify>buffer": true, + "nock>debug": true, + "@metamask/utils>pony-cause": true, + "semver": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util>@metamask/utils": { "globals": { "TextDecoder": true, @@ -2684,13 +2717,13 @@ "packages": { "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils": true, "@trezor/connect-web>@trezor/utils": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/eth-trezor-keyring>@trezor/connect-plugin-ethereum": { "packages": { - "@metamask/eth-sig-util": true, - "@swc/helpers>tslib": true + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": true, + "tslib": true } }, "@trezor/connect-web": { @@ -2721,7 +2754,7 @@ "@trezor/connect-web>@trezor/connect": true, "@trezor/connect-web>@trezor/utils": true, "webpack>events": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect": { @@ -2730,7 +2763,7 @@ "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": true, "@trezor/connect-web>@trezor/connect>@trezor/transport": true, "@trezor/connect-web>@trezor/utils": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils": { @@ -2747,7 +2780,7 @@ }, "packages": { "process": true, - "@swc/helpers>tslib": true, + "tslib": true, "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils>ua-parser-js": true } }, @@ -2756,7 +2789,7 @@ "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": true, "browserify>buffer": true, "@trezor/connect-web>@trezor/connect>@trezor/protobuf>protobufjs": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": { @@ -2786,7 +2819,7 @@ "@trezor/connect-web>@trezor/utils>bignumber.js": true, "browserify>buffer": true, "webpack>events": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@welldone-software/why-did-you-render": { @@ -2899,7 +2932,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/transaction-controller>@metamask/nonce-tracker>async-mutex": { @@ -2908,7 +2941,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "string.prototype.matchall>es-abstract>available-typed-arrays": { @@ -3810,7 +3843,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "browserify>util>which-typed-array>for-each": { @@ -5324,7 +5357,7 @@ "document.getSelection": true } }, - "@swc/helpers>tslib": { + "tslib": { "globals": { "SuppressedError": true, "define": true @@ -5412,7 +5445,7 @@ "packages": { "react-focus-lock>use-sidecar>detect-node-es": true, "react": true, - "@swc/helpers>tslib": true + "tslib": true } }, "readable-stream>util-deprecate": { diff --git a/lavamoat/browserify/mmi/policy.json b/lavamoat/browserify/mmi/policy.json index 5658498ad3a7..b5f88a04662a 100644 --- a/lavamoat/browserify/mmi/policy.json +++ b/lavamoat/browserify/mmi/policy.json @@ -550,7 +550,7 @@ "console": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/notification-services-controller>firebase>@firebase/messaging": { @@ -584,7 +584,7 @@ "@metamask/notification-services-controller>firebase>@firebase/installations": true, "@metamask/notification-services-controller>firebase>@firebase/util": true, "@metamask/notification-services-controller>firebase>@firebase/app>idb": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/notification-services-controller>firebase>@firebase/util": { @@ -634,7 +634,7 @@ "ethereumjs-util>ethereum-cryptography>bs58check": true, "buffer": true, "browserify>buffer": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@keystonehq/metamask-airgapped-keyring": { @@ -907,6 +907,12 @@ "@metamask/eth-snap-keyring>@metamask/eth-sig-util>@metamask/abi-utils>@metamask/utils": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/abi-utils": { + "packages": { + "@metamask/utils>@metamask/superstruct": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util>@metamask/abi-utils": { "packages": { "@metamask/utils>@metamask/superstruct": true, @@ -1213,6 +1219,17 @@ "@metamask/eth-sig-util>tweetnacl": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": { + "packages": { + "@ethereumjs/tx>@ethereumjs/util": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/abi-utils": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": true, + "@metamask/utils>@scure/base": true, + "browserify>buffer": true, + "@ethereumjs/tx>ethereum-cryptography": true, + "@metamask/eth-sig-util>tweetnacl": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util": { "packages": { "@ethereumjs/tx>@ethereumjs/util": true, @@ -1283,6 +1300,7 @@ "packages": { "@ethereumjs/tx": true, "@ethereumjs/tx>@ethereumjs/util": true, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": true, "@metamask/eth-trezor-keyring>@trezor/connect-plugin-ethereum": true, "@trezor/connect-web": true, "browserify>buffer": true, @@ -2153,6 +2171,21 @@ "semver": true } }, + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util>@metamask/utils": { + "globals": { + "TextDecoder": true, + "TextEncoder": true + }, + "packages": { + "@metamask/utils>@metamask/superstruct": true, + "@noble/hashes": true, + "@metamask/utils>@scure/base": true, + "browserify>buffer": true, + "nock>debug": true, + "@metamask/utils>pony-cause": true, + "semver": true + } + }, "@metamask/keyring-controller>@metamask/eth-sig-util>@metamask/utils": { "globals": { "TextDecoder": true, @@ -2776,13 +2809,13 @@ "packages": { "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils": true, "@trezor/connect-web>@trezor/utils": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/eth-trezor-keyring>@trezor/connect-plugin-ethereum": { "packages": { - "@metamask/eth-sig-util": true, - "@swc/helpers>tslib": true + "@metamask/eth-trezor-keyring>@metamask/eth-sig-util": true, + "tslib": true } }, "@trezor/connect-web": { @@ -2813,7 +2846,7 @@ "@trezor/connect-web>@trezor/connect": true, "@trezor/connect-web>@trezor/utils": true, "webpack>events": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect": { @@ -2822,7 +2855,7 @@ "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": true, "@trezor/connect-web>@trezor/connect>@trezor/transport": true, "@trezor/connect-web>@trezor/utils": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils": { @@ -2839,7 +2872,7 @@ }, "packages": { "process": true, - "@swc/helpers>tslib": true, + "tslib": true, "@trezor/connect-web>@trezor/connect-common>@trezor/env-utils>ua-parser-js": true } }, @@ -2848,7 +2881,7 @@ "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": true, "browserify>buffer": true, "@trezor/connect-web>@trezor/connect>@trezor/protobuf>protobufjs": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@trezor/connect-web>@trezor/connect>@trezor/schema-utils": { @@ -2878,7 +2911,7 @@ "@trezor/connect-web>@trezor/utils>bignumber.js": true, "browserify>buffer": true, "webpack>events": true, - "@swc/helpers>tslib": true + "tslib": true } }, "@welldone-software/why-did-you-render": { @@ -2991,7 +3024,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "@metamask/transaction-controller>@metamask/nonce-tracker>async-mutex": { @@ -3000,7 +3033,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "string.prototype.matchall>es-abstract>available-typed-arrays": { @@ -3902,7 +3935,7 @@ "setTimeout": true }, "packages": { - "@swc/helpers>tslib": true + "tslib": true } }, "browserify>util>which-typed-array>for-each": { @@ -5416,7 +5449,7 @@ "document.getSelection": true } }, - "@swc/helpers>tslib": { + "tslib": { "globals": { "SuppressedError": true, "define": true @@ -5504,7 +5537,7 @@ "packages": { "react-focus-lock>use-sidecar>detect-node-es": true, "react": true, - "@swc/helpers>tslib": true + "tslib": true } }, "readable-stream>util-deprecate": { diff --git a/package.json b/package.json index 40d8f4845635..77b94c6e74a4 100644 --- a/package.json +++ b/package.json @@ -249,7 +249,15 @@ "secp256k1@npm:^4.0.0": "4.0.4", "secp256k1@npm:^4.0.1": "4.0.4", "secp256k1@npm:4.0.2": "4.0.4", - "secp256k1@npm:4.0.3": "4.0.4" + "secp256k1@npm:4.0.3": "4.0.4", + "tslib@npm:^2.0.0": "~2.6.0", + "tslib@npm:^2.0.1": "~2.6.0", + "tslib@npm:^2.0.3": "~2.6.0", + "tslib@npm:^2.1.0": "~2.6.0", + "tslib@npm:^2.3.0": "~2.6.0", + "tslib@npm:^2.3.1": "~2.6.0", + "tslib@npm:^2.4.0": "~2.6.0", + "tslib@npm:^2.6.2": "~2.6.0" }, "dependencies": { "@babel/runtime": "patch:@babel/runtime@npm%3A7.25.9#~/.yarn/patches/@babel-runtime-npm-7.25.9-fe8c62510a.patch", @@ -301,7 +309,7 @@ "@metamask/eth-sig-util": "^7.0.1", "@metamask/eth-snap-keyring": "^7.0.0", "@metamask/eth-token-tracker": "^9.0.0", - "@metamask/eth-trezor-keyring": "^3.1.3", + "@metamask/eth-trezor-keyring": "^6.0.0", "@metamask/etherscan-link": "^3.0.0", "@metamask/ethjs": "^0.6.0", "@metamask/ethjs-contract": "^0.4.1", @@ -432,6 +440,7 @@ "simple-git": "^3.20.0", "single-call-balance-checker-abi": "^1.0.0", "ts-mixer": "patch:ts-mixer@npm%3A6.0.4#~/.yarn/patches/ts-mixer-npm-6.0.4-5d9747bdf5.patch", + "tslib": "~2.6.0", "unicode-confusables": "^0.1.1", "uri-js": "^4.4.1", "uuid": "^8.3.2", diff --git a/yarn.lock b/yarn.lock index 7f988937d1bb..e98bb06943fe 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5459,17 +5459,18 @@ __metadata: languageName: node linkType: hard -"@metamask/eth-trezor-keyring@npm:^3.1.3": - version: 3.1.3 - resolution: "@metamask/eth-trezor-keyring@npm:3.1.3" +"@metamask/eth-trezor-keyring@npm:^6.0.0": + version: 6.0.0 + resolution: "@metamask/eth-trezor-keyring@npm:6.0.0" dependencies: "@ethereumjs/tx": "npm:^4.2.0" "@ethereumjs/util": "npm:^8.1.0" - "@metamask/eth-sig-util": "npm:^7.0.3" + "@metamask/eth-sig-util": "npm:^8.0.0" "@trezor/connect-plugin-ethereum": "npm:^9.0.3" "@trezor/connect-web": "npm:^9.1.11" hdkey: "npm:^2.1.0" - checksum: 10/d32a687bcaab4593e6208a1bb59cbdd2b111eff357fd30e707787454ef571abfb4e6162422504f730f3ab2fe576b555d68114de0406ae5cdad252dab1b635cce + tslib: "npm:^2.6.2" + checksum: 10/d5d799c60eeab963ef3e5533de472044b08b6f72652ecefbf26cec99784829bbcd706df57f6450ddb019c7dff7c41b0e0dad244aad62b7d03b51fc97755e2c4c languageName: node linkType: hard @@ -6231,9 +6232,9 @@ __metadata: linkType: hard "@metamask/slip44@npm:^4.0.0": - version: 4.1.0 - resolution: "@metamask/slip44@npm:4.1.0" - checksum: 10/4265254a1800a24915bd1de15f86f196737132f9af2a084c2efc885decfc5dd87ad8f0687269d90b35e2ec64d3ea4fbff0caa793bcea6e585b1f3a290952b750 + version: 4.0.0 + resolution: "@metamask/slip44@npm:4.0.0" + checksum: 10/3e47e8834b0fbdabe1f126fd78665767847ddc1f9ccc8defb23007dd71fcd2e4899c8ca04857491be3630668a3765bad1e40fdfca9a61ef33236d8d08e51535e languageName: node linkType: hard @@ -26631,7 +26632,7 @@ __metadata: "@metamask/eth-sig-util": "npm:^7.0.1" "@metamask/eth-snap-keyring": "npm:^7.0.0" "@metamask/eth-token-tracker": "npm:^9.0.0" - "@metamask/eth-trezor-keyring": "npm:^3.1.3" + "@metamask/eth-trezor-keyring": "npm:^6.0.0" "@metamask/etherscan-link": "npm:^3.0.0" "@metamask/ethjs": "npm:^0.6.0" "@metamask/ethjs-contract": "npm:^0.4.1" @@ -26959,6 +26960,7 @@ __metadata: through2: "npm:^4.0.2" ts-mixer: "patch:ts-mixer@npm%3A6.0.4#~/.yarn/patches/ts-mixer-npm-6.0.4-5d9747bdf5.patch" ts-node: "npm:^10.9.2" + tslib: "npm:~2.6.0" tsx: "npm:^4.7.1" ttest: "npm:^2.1.1" typescript: "npm:~5.4.5" @@ -35721,10 +35723,10 @@ __metadata: languageName: node linkType: hard -"tslib@npm:^2.0.0, tslib@npm:^2.0.1, tslib@npm:^2.0.3, tslib@npm:^2.1.0, tslib@npm:^2.3.0, tslib@npm:^2.3.1, tslib@npm:^2.4.0": - version: 2.6.2 - resolution: "tslib@npm:2.6.2" - checksum: 10/bd26c22d36736513980091a1e356378e8b662ded04204453d353a7f34a4c21ed0afc59b5f90719d4ba756e581a162ecbf93118dc9c6be5acf70aa309188166ca +"tslib@npm:~2.6.0": + version: 2.6.3 + resolution: "tslib@npm:2.6.3" + checksum: 10/52109bb681f8133a2e58142f11a50e05476de4f075ca906d13b596ae5f7f12d30c482feb0bff167ae01cfc84c5803e575a307d47938999246f5a49d174fc558c languageName: node linkType: hard From 93b1e13410a19bf3659e6dbb12f9b614852472da Mon Sep 17 00:00:00 2001 From: Priya Date: Thu, 9 Jan 2025 14:08:28 +0100 Subject: [PATCH 07/23] chore: fix flaky e2e for nft token send (#29476) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29476?quickstart=1) ## **Related issues** Fixes: [#29382](https://github.com/MetaMask/metamask-extension/issues/29382) ## **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. --- test/e2e/helpers.js | 2 +- .../confirmations/transactions/nft-token-send-redesign.spec.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index 40e7258ea4a1..7d675788f5bd 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -633,9 +633,9 @@ async function unlockWallet( await driver.navigate(); } + await driver.waitForSelector('#password', { state: 'enabled' }); await driver.fill('#password', password); await driver.press('#password', driver.Key.ENTER); - if (waitLoginSuccess) { await driver.assertElementNotPresent('[data-testid="unlock-page"]'); } diff --git a/test/e2e/tests/confirmations/transactions/nft-token-send-redesign.spec.ts b/test/e2e/tests/confirmations/transactions/nft-token-send-redesign.spec.ts index 09bd9d4b32a2..9b712ebb7a65 100644 --- a/test/e2e/tests/confirmations/transactions/nft-token-send-redesign.spec.ts +++ b/test/e2e/tests/confirmations/transactions/nft-token-send-redesign.spec.ts @@ -196,6 +196,7 @@ async function createERC721WalletInitiatedTransactionAndAssertDetails( const testDapp = new TestDapp(driver); await testDapp.openTestDappPage({ contractAddress, url: DAPP_URL }); + await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); await testDapp.clickERC721MintButton(); From c16460e7078a0510bd018768df4ad4f2ba5dfd8e Mon Sep 17 00:00:00 2001 From: chloeYue <105063779+chloeYue@users.noreply.github.com> Date: Thu, 9 Jan 2025 14:22:41 +0100 Subject: [PATCH 08/23] test: [POM] Migrate connections e2e tests to TS and Page Object Model (#29384) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** - Migrate connections e2e tests to TS and Page Object Model ``` test/e2e/tests/connections/edit-account-flow.spec.ts test/e2e/tests/connections/edit-networks-flow.spec.ts ``` - Remove the spec `test/e2e/tests/connections/connect-with-metamask.spec.js` as it's already completely tested in another spec. We don't want to repeat testing. - create more class methods for permission page class [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27155?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/29440 ## **Manual testing steps** Check code readability, make sure tests pass. ## **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. --- .../modules/bridge-utils/bridge.util.test.ts | 4 +- .../pages/permission/site-permission-page.ts | 93 +++++++++++++++++- .../connections/connect-with-metamask.spec.js | 75 -------------- .../connections/edit-account-flow.spec.js | 97 ------------------- .../edit-account-permissions.spec.ts | 74 ++++++++++++++ .../connections/edit-networks-flow.spec.js | 73 -------------- .../edit-networks-permissions.spec.ts | 49 ++++++++++ 7 files changed, 216 insertions(+), 249 deletions(-) delete mode 100644 test/e2e/tests/connections/connect-with-metamask.spec.js delete mode 100644 test/e2e/tests/connections/edit-account-flow.spec.js create mode 100644 test/e2e/tests/connections/edit-account-permissions.spec.ts delete mode 100644 test/e2e/tests/connections/edit-networks-flow.spec.js create mode 100644 test/e2e/tests/connections/edit-networks-permissions.spec.ts diff --git a/shared/modules/bridge-utils/bridge.util.test.ts b/shared/modules/bridge-utils/bridge.util.test.ts index 555de4fc2516..b9fea3db1ea8 100644 --- a/shared/modules/bridge-utils/bridge.util.test.ts +++ b/shared/modules/bridge-utils/bridge.util.test.ts @@ -149,7 +149,7 @@ describe('Bridge utils', () => { (fetchWithCache as jest.Mock).mockRejectedValue(mockError); - await expect(fetchBridgeFeatureFlags()).rejects.toThrowError(mockError); + await expect(fetchBridgeFeatureFlags()).rejects.toThrow(mockError); }); }); @@ -223,7 +223,7 @@ describe('Bridge utils', () => { (fetchWithCache as jest.Mock).mockRejectedValue(mockError); - await expect(fetchBridgeTokens('0xa')).rejects.toThrowError(mockError); + await expect(fetchBridgeTokens('0xa')).rejects.toThrow(mockError); }); }); diff --git a/test/e2e/page-objects/pages/permission/site-permission-page.ts b/test/e2e/page-objects/pages/permission/site-permission-page.ts index bc5eee61c781..9f7ae5d778ff 100644 --- a/test/e2e/page-objects/pages/permission/site-permission-page.ts +++ b/test/e2e/page-objects/pages/permission/site-permission-page.ts @@ -7,7 +7,33 @@ import { Driver } from '../../../webdriver/driver'; class SitePermissionPage { private driver: Driver; - private readonly permissionPage = '[data-testid ="connections-page"]'; + private readonly confirmEditAccountsButton = + '[data-testid="connect-more-accounts-button"]'; + + private readonly confirmEditNetworksButton = + '[data-testid="connect-more-chains-button"]'; + + private readonly connectedAccountsInfo = { + text: 'See your accounts and suggest transactions', + tag: 'p', + }; + + private readonly editAccountsModalTitle = { + text: 'Edit accounts', + tag: 'h4', + }; + + private readonly editButton = '[data-testid="edit"]'; + + private readonly editNetworksModalTitle = { + text: 'Edit networks', + tag: 'h4', + }; + + private readonly enabledNetworksInfo = { + text: 'Use your enabled networks', + tag: 'p', + }; constructor(driver: Driver) { this.driver = driver; @@ -20,7 +46,8 @@ class SitePermissionPage { */ async check_pageIsLoaded(site: string): Promise { try { - await this.driver.waitForSelector(this.permissionPage); + await this.driver.waitForSelector(this.connectedAccountsInfo); + await this.driver.waitForSelector(this.enabledNetworksInfo); await this.driver.waitForSelector({ text: site, tag: 'span' }); } catch (e) { console.log( @@ -31,6 +58,68 @@ class SitePermissionPage { } console.log('Site permission page is loaded'); } + + /** + * Edit permissions for accounts on site permission page + * + * @param accountLabels - Account labels to edit + */ + async editPermissionsForAccount(accountLabels: string[]): Promise { + console.log(`Edit permissions for accounts: ${accountLabels}`); + const editButtons = await this.driver.findElements(this.editButton); + await editButtons[0].click(); + await this.driver.waitForSelector(this.editAccountsModalTitle); + for (const accountLabel of accountLabels) { + await this.driver.clickElement({ text: accountLabel, tag: 'button' }); + } + await this.driver.clickElementAndWaitToDisappear( + this.confirmEditAccountsButton, + ); + } + + /** + * Edit permissions for networks on site permission page + * + * @param networkNames - Network names to edit + */ + async editPermissionsForNetwork(networkNames: string[]): Promise { + console.log(`Edit permissions for networks: ${networkNames}`); + const editButtons = await this.driver.findElements(this.editButton); + await editButtons[1].click(); + await this.driver.waitForSelector(this.editNetworksModalTitle); + for (const networkName of networkNames) { + await this.driver.clickElement({ text: networkName, tag: 'p' }); + } + await this.driver.clickElementAndWaitToDisappear( + this.confirmEditNetworksButton, + ); + } + + /** + * Check if the number of connected accounts is correct + * + * @param number - Expected number of connected accounts + */ + async check_connectedAccountsNumber(number: number): Promise { + console.log(`Check that the number of connected accounts is: ${number}`); + await this.driver.waitForSelector({ + text: `${number} accounts connected`, + tag: 'span', + }); + } + + /** + * Check if the number of connected networks is correct + * + * @param number - Expected number of connected networks + */ + async check_connectedNetworksNumber(number: number): Promise { + console.log(`Check that the number of connected networks is: ${number}`); + await this.driver.waitForSelector({ + text: `${number} networks connected`, + tag: 'span', + }); + } } export default SitePermissionPage; diff --git a/test/e2e/tests/connections/connect-with-metamask.spec.js b/test/e2e/tests/connections/connect-with-metamask.spec.js deleted file mode 100644 index b46fc8730d84..000000000000 --- a/test/e2e/tests/connections/connect-with-metamask.spec.js +++ /dev/null @@ -1,75 +0,0 @@ -const { strict: assert } = require('assert'); -const { - withFixtures, - WINDOW_TITLES, - logInWithBalanceValidation, - defaultGanacheOptions, - openDapp, -} = require('../../helpers'); -const FixtureBuilder = require('../../fixture-builder'); - -describe('Connections page', function () { - it('should render new connections flow', async function () { - await withFixtures( - { - dapp: true, - fixtures: new FixtureBuilder().build(), - title: this.test.fullTitle(), - ganacheOptions: defaultGanacheOptions, - }, - async ({ driver, ganacheServer }) => { - await logInWithBalanceValidation(driver, ganacheServer); - await openDapp(driver); - // Connect to dapp - await driver.clickElement({ - text: 'Connect', - tag: 'button', - }); - - await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); - - // should render new connections page - const newConnectionPage = await driver.waitForSelector({ - tag: 'h2', - text: 'Connect with MetaMask', - }); - assert.ok(newConnectionPage, 'Connection Page is defined'); - await driver.clickElementAndWaitForWindowToClose({ - text: 'Connect', - tag: 'button', - }); - await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); - - // It should render connected status for button if dapp is connected - const getConnectedStatus = await driver.waitForSelector({ - css: '#connectButton', - text: 'Connected', - }); - assert.ok(getConnectedStatus, 'Account is connected to Dapp'); - - // Switch to extension Tab - await driver.switchToWindowWithTitle( - WINDOW_TITLES.ExtensionInFullScreenView, - ); - await driver.clickElement( - '[data-testid ="account-options-menu-button"]', - ); - await driver.clickElement({ text: 'All Permissions', tag: 'div' }); - await driver.clickElement({ - text: '127.0.0.1:8080', - tag: 'p', - }); - const connectionsPageAccountInfo = await driver.isElementPresent({ - text: 'See your accounts and suggest transactions', - tag: 'p', - }); - assert.ok(connectionsPageAccountInfo, 'Connections Page is defined'); - const connectionsPageNetworkInfo = await driver.isElementPresent({ - text: 'Use your enabled networks', - tag: 'p', - }); - assert.ok(connectionsPageNetworkInfo, 'Connections Page is defined'); - }, - ); - }); -}); diff --git a/test/e2e/tests/connections/edit-account-flow.spec.js b/test/e2e/tests/connections/edit-account-flow.spec.js deleted file mode 100644 index 1c4899ed8328..000000000000 --- a/test/e2e/tests/connections/edit-account-flow.spec.js +++ /dev/null @@ -1,97 +0,0 @@ -const { strict: assert } = require('assert'); -const { - withFixtures, - WINDOW_TITLES, - connectToDapp, - logInWithBalanceValidation, - locateAccountBalanceDOM, - defaultGanacheOptions, -} = require('../../helpers'); -const FixtureBuilder = require('../../fixture-builder'); - -const accountLabel2 = '2nd custom name'; -const accountLabel3 = '3rd custom name'; -describe('Edit Accounts Flow', function () { - it('should be able to edit accounts', async function () { - await withFixtures( - { - dapp: true, - fixtures: new FixtureBuilder().build(), - title: this.test.fullTitle(), - ganacheOptions: defaultGanacheOptions, - }, - async ({ driver, ganacheServer }) => { - await logInWithBalanceValidation(driver, ganacheServer); - await connectToDapp(driver); - - // It should render connected status for button if dapp is connected - const getConnectedStatus = await driver.waitForSelector({ - css: '#connectButton', - text: 'Connected', - }); - assert.ok(getConnectedStatus, 'Account is connected to Dapp'); - - // Switch to extension Tab - await driver.switchToWindowWithTitle( - WINDOW_TITLES.ExtensionInFullScreenView, - ); - await driver.clickElement('[data-testid="account-menu-icon"]'); - await driver.clickElement( - '[data-testid="multichain-account-menu-popover-action-button"]', - ); - await driver.clickElement( - '[data-testid="multichain-account-menu-popover-add-account"]', - ); - await driver.fill('[placeholder="Account 2"]', accountLabel2); - await driver.clickElement({ text: 'Add account', tag: 'button' }); - await driver.clickElement('[data-testid="account-menu-icon"]'); - await driver.clickElement( - '[data-testid="multichain-account-menu-popover-action-button"]', - ); - await driver.clickElement( - '[data-testid="multichain-account-menu-popover-add-account"]', - ); - await driver.fill('[placeholder="Account 3"]', accountLabel3); - await driver.clickElement({ text: 'Add account', tag: 'button' }); - await locateAccountBalanceDOM(driver); - await driver.clickElement( - '[data-testid ="account-options-menu-button"]', - ); - await driver.clickElement({ text: 'All Permissions', tag: 'div' }); - await driver.clickElement({ - text: '127.0.0.1:8080', - tag: 'p', - }); - const connectionsPageAccountInfo = await driver.isElementPresent({ - text: 'See your accounts and suggest transactions', - tag: 'p', - }); - assert.ok(connectionsPageAccountInfo, 'Connections Page is defined'); - const editButtons = await driver.findElements('[data-testid="edit"]'); - - // Ensure there are edit buttons - assert.ok(editButtons.length > 0, 'Edit buttons are available'); - - // Click the first (0th) edit button - await editButtons[0].click(); - - await driver.clickElement({ - text: '2nd custom name', - tag: 'button', - }); - await driver.clickElement({ - text: '3rd custom name', - tag: 'button', - }); - await driver.clickElement( - '[data-testid="connect-more-accounts-button"]', - ); - const updatedAccountInfo = await driver.isElementPresent({ - text: '3 accounts connected', - tag: 'span', - }); - assert.ok(updatedAccountInfo, 'Accounts List Updated'); - }, - ); - }); -}); diff --git a/test/e2e/tests/connections/edit-account-permissions.spec.ts b/test/e2e/tests/connections/edit-account-permissions.spec.ts new file mode 100644 index 000000000000..bd1eebdf9ffd --- /dev/null +++ b/test/e2e/tests/connections/edit-account-permissions.spec.ts @@ -0,0 +1,74 @@ +import { withFixtures, WINDOW_TITLES } from '../../helpers'; +import FixtureBuilder from '../../fixture-builder'; +import { + ACCOUNT_TYPE, + DEFAULT_FIXTURE_ACCOUNT, + DAPP_HOST_ADDRESS, +} from '../../constants'; +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 PermissionListPage from '../../page-objects/pages/permission/permission-list-page'; +import SitePermissionPage from '../../page-objects/pages/permission/site-permission-page'; +import TestDapp from '../../page-objects/pages/test-dapp'; +import { loginWithBalanceValidation } from '../../page-objects/flows/login.flow'; + +const accountLabel2 = '2nd custom name'; +const accountLabel3 = '3rd custom name'; +describe('Edit Accounts Permissions', function () { + it('should be able to edit accounts', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder().build(), + title: this.test?.fullTitle(), + }, + async ({ driver }) => { + await loginWithBalanceValidation(driver); + const testDapp = new TestDapp(driver); + await testDapp.openTestDappPage(); + await testDapp.check_pageIsLoaded(); + await testDapp.connectAccount({ + publicAddress: DEFAULT_FIXTURE_ACCOUNT, + }); + await driver.switchToWindowWithTitle( + WINDOW_TITLES.ExtensionInFullScreenView, + ); + await new Homepage(driver).check_pageIsLoaded(); + new HeaderNavbar(driver).openAccountMenu(); + + // create second account with custom label + const accountListPage = new AccountListPage(driver); + await accountListPage.check_pageIsLoaded(); + await accountListPage.addAccount({ + accountType: ACCOUNT_TYPE.Ethereum, + accountName: accountLabel2, + }); + const homepage = new Homepage(driver); + await homepage.check_expectedBalanceIsDisplayed(); + + // create third account with custom label + await homepage.headerNavbar.openAccountMenu(); + await accountListPage.check_pageIsLoaded(); + await accountListPage.addAccount({ + accountType: ACCOUNT_TYPE.Ethereum, + accountName: accountLabel3, + }); + await homepage.check_expectedBalanceIsDisplayed(); + + // go to connections permissions page + await homepage.headerNavbar.openPermissionsPage(); + const permissionListPage = new PermissionListPage(driver); + await permissionListPage.check_pageIsLoaded(); + await permissionListPage.openPermissionPageForSite(DAPP_HOST_ADDRESS); + const sitePermissionPage = new SitePermissionPage(driver); + await sitePermissionPage.check_pageIsLoaded(DAPP_HOST_ADDRESS); + await sitePermissionPage.editPermissionsForAccount([ + accountLabel2, + accountLabel3, + ]); + await sitePermissionPage.check_connectedAccountsNumber(3); + }, + ); + }); +}); diff --git a/test/e2e/tests/connections/edit-networks-flow.spec.js b/test/e2e/tests/connections/edit-networks-flow.spec.js deleted file mode 100644 index 95a091f7e504..000000000000 --- a/test/e2e/tests/connections/edit-networks-flow.spec.js +++ /dev/null @@ -1,73 +0,0 @@ -const { strict: assert } = require('assert'); -const { - withFixtures, - WINDOW_TITLES, - connectToDapp, - logInWithBalanceValidation, - locateAccountBalanceDOM, - defaultGanacheOptions, -} = require('../../helpers'); -const FixtureBuilder = require('../../fixture-builder'); - -describe('Edit Networks Flow', function () { - it('should be able to edit networks', async function () { - await withFixtures( - { - dapp: true, - fixtures: new FixtureBuilder().build(), - title: this.test.fullTitle(), - ganacheOptions: defaultGanacheOptions, - }, - async ({ driver, ganacheServer }) => { - await logInWithBalanceValidation(driver, ganacheServer); - await connectToDapp(driver); - - // It should render connected status for button if dapp is connected - const getConnectedStatus = await driver.waitForSelector({ - css: '#connectButton', - text: 'Connected', - }); - assert.ok(getConnectedStatus, 'Account is connected to Dapp'); - - // Switch to extension Tab - await driver.switchToWindowWithTitle( - WINDOW_TITLES.ExtensionInFullScreenView, - ); - await driver.clickElement('[data-testid="network-display"]'); - await driver.clickElement('.mm-modal-content__dialog .toggle-button'); - await driver.clickElement( - '.mm-modal-content__dialog button[aria-label="Close"]', - ); - await locateAccountBalanceDOM(driver); - await driver.clickElement( - '[data-testid ="account-options-menu-button"]', - ); - await driver.clickElement({ text: 'All Permissions', tag: 'div' }); - await driver.clickElement({ - text: '127.0.0.1:8080', - tag: 'p', - }); - const editButtons = await driver.findElements('[data-testid="edit"]'); - - // Ensure there are edit buttons - assert.ok(editButtons.length > 0, 'Edit buttons are available'); - - // Click the first (0th) edit button - await editButtons[1].click(); - - // Disconnect Mainnet - await driver.clickElement({ - text: 'Ethereum Mainnet', - tag: 'p', - }); - - await driver.clickElement('[data-testid="connect-more-chains-button"]'); - const updatedNetworkInfo = await driver.isElementPresent({ - text: '2 networks connected', - tag: 'span', - }); - assert.ok(updatedNetworkInfo, 'Networks List Updated'); - }, - ); - }); -}); diff --git a/test/e2e/tests/connections/edit-networks-permissions.spec.ts b/test/e2e/tests/connections/edit-networks-permissions.spec.ts new file mode 100644 index 000000000000..55b7cdd2caeb --- /dev/null +++ b/test/e2e/tests/connections/edit-networks-permissions.spec.ts @@ -0,0 +1,49 @@ +import { withFixtures, WINDOW_TITLES } from '../../helpers'; +import { DEFAULT_FIXTURE_ACCOUNT, DAPP_HOST_ADDRESS } from '../../constants'; +import FixtureBuilder from '../../fixture-builder'; +import HeaderNavbar from '../../page-objects/pages/header-navbar'; +import Homepage from '../../page-objects/pages/home/homepage'; +import TestDapp from '../../page-objects/pages/test-dapp'; +import PermissionListPage from '../../page-objects/pages/permission/permission-list-page'; +import SitePermissionPage from '../../page-objects/pages/permission/site-permission-page'; +import { loginWithBalanceValidation } from '../../page-objects/flows/login.flow'; + +describe('Edit Networks Permissions', function () { + it('should be able to edit networks', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder().build(), + title: this.test?.fullTitle(), + }, + async ({ driver }) => { + await loginWithBalanceValidation(driver); + const testDapp = new TestDapp(driver); + await testDapp.openTestDappPage(); + await testDapp.check_pageIsLoaded(); + + await testDapp.connectAccount({ + publicAddress: DEFAULT_FIXTURE_ACCOUNT, + }); + await driver.switchToWindowWithTitle( + WINDOW_TITLES.ExtensionInFullScreenView, + ); + await new Homepage(driver).check_pageIsLoaded(); + + // Open permission page for dapp + new HeaderNavbar(driver).openPermissionsPage(); + const permissionListPage = new PermissionListPage(driver); + await permissionListPage.check_pageIsLoaded(); + await permissionListPage.openPermissionPageForSite(DAPP_HOST_ADDRESS); + const sitePermissionPage = new SitePermissionPage(driver); + await sitePermissionPage.check_pageIsLoaded(DAPP_HOST_ADDRESS); + + // Disconnect Mainnet + await sitePermissionPage.editPermissionsForNetwork([ + 'Ethereum Mainnet', + ]); + await sitePermissionPage.check_connectedNetworksNumber(2); + }, + ); + }); +}); From 3154019858c172231111698f519b19577fc1e2f6 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Thu, 9 Jan 2025 14:25:56 +0100 Subject: [PATCH 09/23] fix(snaps): Scrollbar being partially hidden behind footer (#29435) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This fixes the scrollbar in Snap dialogs being partially hidden behind the footer. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29435?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** ## **Screenshots/Recordings** ### **Before** ![image](https://github.com/user-attachments/assets/66cf18d5-4e0c-49b4-ad44-353384404be8) ![image](https://github.com/user-attachments/assets/81df4231-7b23-4377-b6fe-e18e829d50dd) ### **After** ![image](https://github.com/user-attachments/assets/6390d11c-223e-4e73-8f8e-3f361e0c069f) ![image](https://github.com/user-attachments/assets/269ac240-a7e0-4716-844d-680f2389166f) ## **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. --- .../app/snaps/snap-ui-renderer/components/container.ts | 6 ------ ui/components/app/snaps/snap-ui-renderer/index.scss | 4 ---- .../app/snaps/snap-ui-renderer/snap-ui-renderer.js | 4 ++++ .../__snapshots__/snaps-section.test.tsx.snap | 8 ++++---- 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/ui/components/app/snaps/snap-ui-renderer/components/container.ts b/ui/components/app/snaps/snap-ui-renderer/components/container.ts index cac6788f8c48..b3f717aecbdd 100644 --- a/ui/components/app/snaps/snap-ui-renderer/components/container.ts +++ b/ui/components/app/snaps/snap-ui-renderer/components/container.ts @@ -2,7 +2,6 @@ import { BoxElement, JSXElement } from '@metamask/snaps-sdk/jsx'; import { getJsxChildren } from '@metamask/snaps-utils'; import { mapToTemplate } from '../utils'; import { - BlockSize, Display, FlexDirection, } from '../../../../../helpers/constants/design-system'; @@ -78,12 +77,7 @@ export const container: UIComponentFactory = ({ props: { display: Display.Flex, flexDirection: FlexDirection.Column, - height: BlockSize.Full, className: 'snap-ui-renderer__container', - style: { - overflowY: 'auto', - paddingBottom: useFooter ? '80px' : 'initial', - }, }, }; }; diff --git a/ui/components/app/snaps/snap-ui-renderer/index.scss b/ui/components/app/snaps/snap-ui-renderer/index.scss index cbd4258e1d9d..c840a76b670c 100644 --- a/ui/components/app/snaps/snap-ui-renderer/index.scss +++ b/ui/components/app/snaps/snap-ui-renderer/index.scss @@ -5,10 +5,6 @@ $width-screen-md-min: 80vw; $width-screen-lg-min: 62vw; - &__content { - margin-bottom: 0 !important; - } - &__container { & > *:first-child { gap: 16px; diff --git a/ui/components/app/snaps/snap-ui-renderer/snap-ui-renderer.js b/ui/components/app/snaps/snap-ui-renderer/snap-ui-renderer.js index 332fab99308d..481a51a0a202 100644 --- a/ui/components/app/snaps/snap-ui-renderer/snap-ui-renderer.js +++ b/ui/components/app/snaps/snap-ui-renderer/snap-ui-renderer.js @@ -131,6 +131,10 @@ const SnapUIRendererComponent = ({ className="snap-ui-renderer__content" height={BlockSize.Full} backgroundColor={contentBackgroundColor} + style={{ + overflowY: 'auto', + marginBottom: useFooter ? '80px' : '0', + }} > diff --git a/ui/pages/confirmations/components/confirm/snaps/snaps-section/__snapshots__/snaps-section.test.tsx.snap b/ui/pages/confirmations/components/confirm/snaps/snaps-section/__snapshots__/snaps-section.test.tsx.snap index 27b21ab7ab3c..558011e68629 100644 --- a/ui/pages/confirmations/components/confirm/snaps/snaps-section/__snapshots__/snaps-section.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/snaps/snaps-section/__snapshots__/snaps-section.test.tsx.snap @@ -40,10 +40,10 @@ exports[`SnapsSection renders section for typed sign request 1`] = ` >

Date: Thu, 9 Jan 2025 21:40:23 +0800 Subject: [PATCH 10/23] fix: show localized snap name in snap tag (#29049) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR fixes the issue where the localized snap name is not being used when in the account tag. It also removes tech debt which is the `mergeAccount` function that isn't needed any more. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29049?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/accounts-planning/issues/769 ## **Manual testing steps** 1. Install the BTC snap 2. Create a BTC account 3. Click on the account menu and see that the name is correct. ## **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. --- .../lib/accounts/BalancesController.test.ts | 2 + test/data/mock-send-state.json | 2 +- test/data/mock-state.json | 35 +++++++++- test/jest/mocks.ts | 8 ++- .../account-list-item/account-list-item.js | 22 +++--- .../account-list-item.stories.js | 2 +- .../account-list-item.test.js | 68 +++++++++++++------ .../account-list-menu.test.tsx | 9 ++- .../account-list-menu/account-list-menu.tsx | 33 --------- .../account-list-menu/hidden-account-list.js | 6 +- .../pages/connections/connections.tsx | 23 +++---- .../review-permissions-page.tsx | 13 ++-- .../__snapshots__/your-accounts.test.tsx.snap | 2 +- .../pages/send/components/your-accounts.tsx | 14 ++-- .../multichain/pages/send/send.test.js | 1 + ui/helpers/utils/accounts.js | 12 +++- ui/helpers/utils/accounts.test.js | 13 ++-- .../connect-page/connect-page.tsx | 14 ++-- .../remove-snap-account/snap-account-card.tsx | 11 +-- ui/selectors/selectors.test.js | 3 +- 20 files changed, 161 insertions(+), 132 deletions(-) diff --git a/app/scripts/lib/accounts/BalancesController.test.ts b/app/scripts/lib/accounts/BalancesController.test.ts index e18fbc1e7be8..3dec8c565c73 100644 --- a/app/scripts/lib/accounts/BalancesController.test.ts +++ b/app/scripts/lib/accounts/BalancesController.test.ts @@ -1,6 +1,7 @@ import { ControllerMessenger } from '@metamask/base-controller'; import { Balance, BtcAccountType, CaipAssetType } from '@metamask/keyring-api'; import { InternalAccount } from '@metamask/keyring-internal-api'; +import { KeyringTypes } from '@metamask/keyring-controller'; import { createMockInternalAccount } from '../../../../test/jest/mocks'; import { MultichainNetworks } from '../../../../shared/constants/multichain/networks'; import { @@ -25,6 +26,7 @@ const mockBtcAccount = createMockInternalAccount({ options: { scope: MultichainNetworks.BITCOIN_TESTNET, }, + keyringType: KeyringTypes.snap, }); const mockBalanceResult = { diff --git a/test/data/mock-send-state.json b/test/data/mock-send-state.json index 0b7cc237547f..0270926566d6 100644 --- a/test/data/mock-send-state.json +++ b/test/data/mock-send-state.json @@ -133,7 +133,7 @@ } } }, - "snaps": [{}], + "snaps": {}, "preferences": { "hideZeroBalanceTokens": false, "showFiatInTestnets": false, diff --git a/test/data/mock-state.json b/test/data/mock-state.json index 9ea2e674e7a3..b85b62ae80d7 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -370,6 +370,39 @@ "version": "5.1.2" } ] + }, + "local:snap-id": { + "id": "local:snap-id", + "origin": "local:snap-id", + "version": "5.1.2", + "iconUrl": null, + "initialPermissions": {}, + "manifest": { + "description": "mock snap description", + "proposedName": "mock snap name", + "repository": { + "type": "git", + "url": "https://127.0.0.1" + }, + "source": { + "location": { + "npm": { + "filePath": "dist/bundle.js", + "packageName": "@metamask/test-snap-dialog", + "registry": "https://registry.npmjs.org" + } + }, + "shasum": "L1k+dT9Q+y3KfIqzaH09MpDZVPS9ZowEh9w01ZMTWMU=" + }, + "version": "5.1.2" + }, + "versionHistory": [ + { + "date": 1680686075921, + "origin": "https://metamask.github.io", + "version": "5.1.2" + } + ] } }, "preferences": { @@ -541,7 +574,7 @@ }, "snap": { "enabled": true, - "id": "snap-id", + "id": "local:snap-id", "name": "snap-name" } }, diff --git a/test/jest/mocks.ts b/test/jest/mocks.ts index 9fdc6538d9d6..b165385d8f4b 100644 --- a/test/jest/mocks.ts +++ b/test/jest/mocks.ts @@ -186,7 +186,11 @@ export function createMockInternalAccount({ type = EthAccountType.Eoa, keyringType = KeyringTypes.hd, lastSelected = 0, - snapOptions = undefined, + snapOptions = { + enabled: true, + id: 'npm:snap-id', + name: 'snap-name', + }, options = undefined, }: { name?: string; @@ -236,7 +240,7 @@ export function createMockInternalAccount({ keyring: { type: keyringType, }, - snap: snapOptions, + snap: keyringType === KeyringTypes.snap ? snapOptions : undefined, lastSelected, }, options: options ?? {}, diff --git a/ui/components/multichain/account-list-item/account-list-item.js b/ui/components/multichain/account-list-item/account-list-item.js index 2c080c940d8a..affda29f6881 100644 --- a/ui/components/multichain/account-list-item/account-list-item.js +++ b/ui/components/multichain/account-list-item/account-list-item.js @@ -4,7 +4,7 @@ import classnames from 'classnames'; import { useSelector } from 'react-redux'; import { useI18nContext } from '../../../hooks/useI18nContext'; -import { shortenAddress } from '../../../helpers/utils/util'; +import { getSnapName, shortenAddress } from '../../../helpers/utils/util'; import { AccountListItemMenu } from '../account-list-item-menu'; import { AvatarGroup } from '../avatar-group'; @@ -53,6 +53,7 @@ import { getIsTokenNetworkFilterEqualCurrentNetwork, getShowFiatInTestnets, getChainIdsToPoll, + getSnapsMetadata, } from '../../../selectors'; import { getMultichainIsTestnet, @@ -73,6 +74,7 @@ import { normalizeSafeAddress } from '../../../../app/scripts/lib/multichain/add import { useMultichainSelector } from '../../../hooks/useMultichainSelector'; import { useGetFormattedTokensPerChain } from '../../../hooks/useGetFormattedTokensPerChain'; import { useAccountTotalCrossChainFiatBalance } from '../../../hooks/useAccountTotalCrossChainFiatBalance'; +import { getAccountLabel } from '../../../helpers/utils/accounts'; import { AccountListItemMenuTypes } from './account-list-item.types'; const MAXIMUM_CURRENCY_DECIMALS = 3; @@ -99,6 +101,14 @@ const AccountListItem = ({ const [accountOptionsMenuOpen, setAccountOptionsMenuOpen] = useState(false); const [accountListItemMenuElement, setAccountListItemMenuElement] = useState(); + const snapMetadata = useSelector(getSnapsMetadata); + const accountLabel = getAccountLabel( + account.metadata.keyring.type, + account, + account.metadata.keyring.type === KeyringType.snap + ? getSnapName(snapMetadata)(account.metadata?.snap.id) + : null, + ); const useBlockie = useSelector(getUseBlockie); const { isEvmNetwork } = useMultichainSelector(getMultichainNetwork, account); @@ -402,9 +412,9 @@ const AccountListItem = ({ )} - {account.label ? ( + {accountLabel ? ( setAccountOptionsMenuOpen(false)} isOpen={accountOptionsMenuOpen} - isRemovable={account.keyring.type !== KeyringType.hdKeyTree} + isRemovable={account.metadata.keyring.type !== KeyringType.hdKeyTree} closeMenu={closeMenu} isPinned={isPinned} isHidden={isHidden} @@ -487,10 +497,6 @@ AccountListItem.propTypes = { type: PropTypes.string.isRequired, }).isRequired, }).isRequired, - keyring: PropTypes.shape({ - type: PropTypes.string.isRequired, - }).isRequired, - label: PropTypes.string, }).isRequired, /** * Represents if this account is currently selected diff --git a/ui/components/multichain/account-list-item/account-list-item.stories.js b/ui/components/multichain/account-list-item/account-list-item.stories.js index 913879b5a71a..6de7e61e8ea5 100644 --- a/ui/components/multichain/account-list-item/account-list-item.stories.js +++ b/ui/components/multichain/account-list-item/account-list-item.stories.js @@ -64,7 +64,7 @@ const SNAP_ACCOUNT = { }, snap: { name: 'Test Snap Name', - id: 'snap-id', + id: 'npm:snap-id', enabled: true, }, }, diff --git a/ui/components/multichain/account-list-item/account-list-item.test.js b/ui/components/multichain/account-list-item/account-list-item.test.js index 94619d2ae389..39705dac4e8b 100644 --- a/ui/components/multichain/account-list-item/account-list-item.test.js +++ b/ui/components/multichain/account-list-item/account-list-item.test.js @@ -19,9 +19,6 @@ const mockAccount = { 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3' ], balance: '0x152387ad22c3f0', - keyring: { - type: 'HD Key Tree', - }, }; const mockNonEvmAccount = { @@ -31,6 +28,40 @@ const mockNonEvmAccount = { type: 'bip122:p2wpkh', }; +const mockSnap = { + id: 'local:mock-snap', + origin: 'local:mock-snap', + version: '1.3.7', + iconUrl: null, + initialPermissions: {}, + manifest: { + description: 'mock-description', + proposedName: 'mock-snap-name', + repository: { + type: 'git', + url: 'https://127.0.0.1', + }, + source: { + location: { + npm: { + filePath: 'dist/bundle.js', + packageName: 'local:mock-snap', + }, + }, + shasum: 'L1k+dT9Q+y3KfIqzaH09MpDZVPS9ZowEh9w01ZMTWMU=', + locales: ['en'], + }, + version: '1.3.7', + }, + versionHistory: [ + { + date: 1680686075921, + origin: 'https://metamask.github.io', + version: '1.3.7', + }, + ], +}; + const DEFAULT_PROPS = { account: mockAccount, selected: false, @@ -64,6 +95,10 @@ const render = (props = {}, state = {}) => { conversionRate: '100000', }, }, + snaps: { + ...mockState.metamask.snaps, + [mockSnap.id]: mockSnap, + }, }, activeTab: { id: 113, @@ -172,30 +207,25 @@ describe('AccountListItem', () => { }); ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) - it('renders the snap label for unnamed snap accounts', () => { - const { container } = render({ - account: { - ...mockAccount, - balance: '0x0', - keyring: 'Snap Keyring', - label: 'Snaps (Beta)', - }, - }); - const tag = container.querySelector('.mm-tag'); - expect(tag.textContent).toBe('Snaps (Beta)'); - }); - it('renders the snap name for named snap accounts', () => { const { container } = render({ account: { ...mockAccount, + metadata: { + ...mockAccount.metadata, + snap: { + id: mockSnap.id, + }, + keyring: { + type: 'Snap Keyring', + }, + }, + balance: '0x0', - keyring: 'Snap Keyring', - label: 'Test Snap Name (Beta)', }, }); const tag = container.querySelector('.mm-tag'); - expect(tag.textContent).toBe('Test Snap Name (Beta)'); + expect(tag.textContent).toBe(`${mockSnap.manifest.proposedName} (Beta)`); }); ///: END:ONLY_INCLUDE_IF diff --git a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx index 539899d0eb09..0d7884fcd666 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx @@ -479,6 +479,9 @@ describe('AccountListMenu', () => { keyring: { type: 'Snap Keyring', }, + snap: { + id: 'local:snap-id', + }, }, }, }, @@ -491,7 +494,7 @@ describe('AccountListMenu', () => { '.multichain-account-list-item', ); const tag = listItems[0].querySelector('.mm-tag') as Element; - expect(tag.textContent).toBe('Snaps (Beta)'); + expect(tag.textContent).toBe('mock snap name (Beta)'); }); it('displays the correct label for named snap accounts', () => { @@ -511,7 +514,7 @@ describe('AccountListMenu', () => { }, snap: { name: 'Test Snap Name', - id: 'test-snap-id', + id: 'local:snap-id', }, }, }, @@ -524,7 +527,7 @@ describe('AccountListMenu', () => { '.multichain-account-list-item', ); const tag = listItems[0].querySelector('.mm-tag') as Element; - expect(tag.textContent).toBe('Test Snap Name (Beta)'); + expect(tag.textContent).toBe('mock snap name (Beta)'); }); ///: END:ONLY_INCLUDE_IF diff --git a/ui/components/multichain/account-list-menu/account-list-menu.tsx b/ui/components/multichain/account-list-menu/account-list-menu.tsx index ca9852af1474..ea79ad84f879 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.tsx @@ -23,7 +23,6 @@ import { ///: END:ONLY_INCLUDE_IF } from '@metamask/keyring-api'; ///: BEGIN:ONLY_INCLUDE_IF(build-flask) -import { InternalAccount } from '@metamask/keyring-internal-api'; import { BITCOIN_WALLET_NAME, BITCOIN_WALLET_SNAP_ID, @@ -96,7 +95,6 @@ import { // eslint-disable-next-line import/no-restricted-paths import { getEnvironmentType } from '../../../../app/scripts/lib/util'; import { ENVIRONMENT_TYPE_POPUP } from '../../../../shared/constants/app'; -import { getAccountLabel } from '../../../helpers/utils/accounts'; ///: BEGIN:ONLY_INCLUDE_IF(build-flask) import { ACCOUNT_WATCHER_NAME, @@ -185,36 +183,6 @@ export const getActionTitle = ( } }; -/** - * Merges ordered accounts with balances with each corresponding account data from internal accounts - * - * @param accountsWithBalances - ordered accounts with balances - * @param internalAccounts - internal accounts - * @returns merged accounts list with balances and internal account data - */ -export const mergeAccounts = ( - accountsWithBalances: MergedInternalAccount[], - internalAccounts: InternalAccount[], -) => { - return accountsWithBalances.map((account) => { - const internalAccount = internalAccounts.find( - (intAccount) => intAccount.address === account.address, - ); - if (internalAccount) { - return { - ...account, - ...internalAccount, - keyring: internalAccount.metadata.keyring, - label: getAccountLabel( - internalAccount.metadata.keyring.type, - internalAccount, - ), - }; - } - return account; - }); -}; - type AccountListMenuProps = { onClose: () => void; privacyMode?: boolean; @@ -345,7 +313,6 @@ export const AccountListMenu = ({ fuse.setCollection(filteredAccounts); searchResults = fuse.search(searchQuery); } - searchResults = mergeAccounts(searchResults, filteredAccounts); const title = useMemo( () => getActionTitle(t as (text: string) => string, actionMode), diff --git a/ui/components/multichain/account-list-menu/hidden-account-list.js b/ui/components/multichain/account-list-menu/hidden-account-list.js index 6ae3a32da20b..1fad2cd0a56b 100644 --- a/ui/components/multichain/account-list-menu/hidden-account-list.js +++ b/ui/components/multichain/account-list-menu/hidden-account-list.js @@ -19,7 +19,6 @@ import { useI18nContext } from '../../../hooks/useI18nContext'; import { getConnectedSubjectsForAllAddresses, getHiddenAccountsList, - getInternalAccounts, getMetaMaskAccountsOrdered, getOriginOfCurrentTab, getSelectedAccount, @@ -38,7 +37,6 @@ import { AccountListItem, AccountListItemMenuTypes, } from '../account-list-item'; -import { mergeAccounts } from './account-list-menu'; export const HiddenAccountList = ({ onClose }) => { const t = useI18nContext(); @@ -46,12 +44,10 @@ export const HiddenAccountList = ({ onClose }) => { const dispatch = useDispatch(); const hiddenAddresses = useSelector(getHiddenAccountsList); const accounts = useSelector(getMetaMaskAccountsOrdered); - const internalAccounts = useSelector(getInternalAccounts); - const mergedAccounts = mergeAccounts(accounts, internalAccounts); const selectedAccount = useSelector(getSelectedAccount); const connectedSites = useSelector(getConnectedSubjectsForAllAddresses); const currentTabOrigin = useSelector(getOriginOfCurrentTab); - const filteredHiddenAccounts = mergedAccounts.filter((account) => + const filteredHiddenAccounts = accounts.filter((account) => hiddenAddresses.includes(account.address), ); const [showListItem, setShowListItem] = useState(false); diff --git a/ui/components/multichain/pages/connections/connections.tsx b/ui/components/multichain/pages/connections/connections.tsx index 4a8b188b86b6..0592f5de84ee 100644 --- a/ui/components/multichain/pages/connections/connections.tsx +++ b/ui/components/multichain/pages/connections/connections.tsx @@ -18,7 +18,6 @@ import { getURLHost } from '../../../../helpers/utils/util'; import { useI18nContext } from '../../../../hooks/useI18nContext'; import { getConnectedSitesList, - getInternalAccounts, getOrderedConnectedAccountsForConnectedDapp, getPermissionSubjects, getPermittedAccountsByOrigin, @@ -43,7 +42,6 @@ import { IconSize, Text, } from '../../../component-library'; -import { mergeAccounts } from '../../account-list-menu/account-list-menu'; import { AccountListItem, AccountListItemMenuTypes, @@ -109,11 +107,6 @@ export const Connections = () => { getOrderedConnectedAccountsForConnectedDapp(state, activeTabOrigin), ); const selectedAccount = useSelector(getSelectedAccount); - const internalAccounts = useSelector(getInternalAccounts); - const mergedAccounts = mergeAccounts( - connectedAccounts, - internalAccounts, - ) as AccountType[]; const permittedAccountsByOrigin = useSelector( getPermittedAccountsByOrigin, @@ -168,14 +161,14 @@ export const Connections = () => { } }; - // In the mergeAccounts, we need the lastSelected value to determine which connectedAccount was last selected. - const latestSelected = mergedAccounts.findIndex( + // In the connectedAccounts, we need the lastSelected value to determine which connectedAccount was last selected. + const latestSelected = connectedAccounts.findIndex( // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any (_account: any, index: any) => { return ( index === - mergedAccounts.reduce( + connectedAccounts.reduce( ( indexOfAccountWIthHighestLastSelected: number, currentAccountToCompare: AccountType, @@ -185,7 +178,7 @@ export const Connections = () => { ) => { const currentLastSelected = currentAccountToCompare.metadata.lastSelected ?? 0; - const accountAtIndexLastSelected = mergedAccounts[ + const accountAtIndexLastSelected = connectedAccounts[ indexOfAccountWIthHighestLastSelected ].metadata.lastSelected ? i @@ -251,11 +244,11 @@ export const Connections = () => { - {permittedAccounts.length > 0 && mergeAccounts.length > 0 ? ( + {permittedAccounts.length > 0 && connectedAccounts.length > 0 ? ( {/* TODO: Replace `any` with type */} {/* eslint-disable-next-line @typescript-eslint/no-explicit-any */} - {mergedAccounts.map((account: AccountType, index: any) => { + {connectedAccounts.map((account: AccountType, index: any) => { const connectedSites: ConnectedSites = {}; const connectedSite = connectedSites[account.address]?.find( ({ origin }) => origin === activeTabOrigin, @@ -271,7 +264,7 @@ export const Connections = () => { { /> ) : null} - {permittedAccounts.length > 0 && mergeAccounts.length > 0 ? ( + {permittedAccounts.length > 0 && connectedAccounts.length > 0 ? ( { }; const accounts = useSelector(getUpdatedAndSortedAccounts); - const internalAccounts = useSelector(getInternalAccounts); - const mergedAccounts: MergedInternalAccount[] = useMemo(() => { - return mergeAccounts(accounts, internalAccounts).filter( - (account: InternalAccount) => isEvmAccountType(account.type), + const evmAccounts: MergedInternalAccount[] = useMemo(() => { + return accounts.filter((account: InternalAccount) => + isEvmAccountType(account.type), ); - }, [accounts, internalAccounts]); + }, [accounts]); const connectedAccountAddresses = useSelector((state) => getPermittedAccountsForSelectedTab(state, activeTabOrigin), @@ -196,7 +193,7 @@ export const ReviewPermissions = () => { - snap-name (Beta) + mock snap name (Beta)

diff --git a/ui/components/multichain/pages/send/components/your-accounts.tsx b/ui/components/multichain/pages/send/components/your-accounts.tsx index a0d431919ce6..6123bb1cac78 100644 --- a/ui/components/multichain/pages/send/components/your-accounts.tsx +++ b/ui/components/multichain/pages/send/components/your-accounts.tsx @@ -4,7 +4,6 @@ import { EthAccountType, KeyringAccountType } from '@metamask/keyring-api'; import { InternalAccount } from '@metamask/keyring-internal-api'; import { getUpdatedAndSortedAccounts, - getInternalAccounts, getSelectedInternalAccount, } from '../../../../../selectors'; import { AccountListItem } from '../../..'; @@ -13,13 +12,11 @@ import { updateRecipient, updateRecipientUserInput, } from '../../../../../ducks/send'; -import { mergeAccounts } from '../../../account-list-menu/account-list-menu'; import { MetaMetricsContext } from '../../../../../contexts/metametrics'; import { MetaMetricsEventCategory, MetaMetricsEventName, } from '../../../../../../shared/constants/metametrics'; -import { MergedInternalAccount } from '../../../../../selectors/selectors.types'; import { SendPageRow } from '.'; type SendPageYourAccountsProps = { @@ -36,19 +33,18 @@ export const SendPageYourAccounts = ({ // Your Accounts const accounts = useSelector(getUpdatedAndSortedAccounts); - const internalAccounts = useSelector(getInternalAccounts); - const mergedAccounts: MergedInternalAccount[] = useMemo(() => { - return mergeAccounts(accounts, internalAccounts).filter( - (account: InternalAccount) => allowedAccountTypes.includes(account.type), + const filteredAccounts = useMemo(() => { + return accounts.filter((account: InternalAccount) => + allowedAccountTypes.includes(account.type), ); - }, [accounts, internalAccounts]); + }, [accounts]); const selectedAccount = useSelector(getSelectedInternalAccount); return ( {/* TODO: Replace `any` with type */} {/* eslint-disable-next-line @typescript-eslint/no-explicit-any */} - {mergedAccounts.map((account: any) => ( + {filteredAccounts.map((account: any) => ( { }); describe('Snap Account Label', () => { + const mockSnapName = 'Test Snap Name'; const mockSnapAccountWithName = { ...mockAccount, metadata: { ...mockAccount.metadata, type: KeyringType.snap, - snap: { name: 'Test Snap Name' }, + snap: { name: mockSnapName }, }, }; const mockSnapAccountWithoutName = { @@ -179,9 +180,13 @@ describe('Accounts', () => { }; it('should return snap name with beta tag if snap name is provided', () => { - expect(getAccountLabel(KeyringType.snap, mockSnapAccountWithName)).toBe( - 'Test Snap Name (Beta)', - ); + expect( + getAccountLabel( + KeyringType.snap, + mockSnapAccountWithName, + mockSnapName, + ), + ).toBe('Test Snap Name (Beta)'); }); it('should return generic snap label with beta tag if snap name is not provided', () => { diff --git a/ui/pages/permissions-connect/connect-page/connect-page.tsx b/ui/pages/permissions-connect/connect-page/connect-page.tsx index 741629b3d4d7..0baa9f393b77 100644 --- a/ui/pages/permissions-connect/connect-page/connect-page.tsx +++ b/ui/pages/permissions-connect/connect-page/connect-page.tsx @@ -5,7 +5,6 @@ import { isEvmAccountType } from '@metamask/keyring-api'; import { NetworkConfiguration } from '@metamask/network-controller'; import { useI18nContext } from '../../../hooks/useI18nContext'; import { - getInternalAccounts, getSelectedInternalAccount, getUpdatedAndSortedAccounts, } from '../../../selectors'; @@ -31,8 +30,6 @@ import { FlexDirection, TextVariant, } from '../../../helpers/constants/design-system'; -import { MergedInternalAccount } from '../../../selectors/selectors.types'; -import { mergeAccounts } from '../../../components/multichain/account-list-menu/account-list-menu'; import { TEST_CHAINS } from '../../../../shared/constants/network'; import PermissionsConnectFooter from '../../../components/app/permissions-connect-footer'; import { @@ -117,12 +114,11 @@ export const ConnectPage: React.FC = ({ ); const accounts = useSelector(getUpdatedAndSortedAccounts); - const internalAccounts = useSelector(getInternalAccounts); - const mergedAccounts: MergedInternalAccount[] = useMemo(() => { - return mergeAccounts(accounts, internalAccounts).filter( - (account: InternalAccount) => isEvmAccountType(account.type), + const evmAccounts = useMemo(() => { + return accounts.filter((account: InternalAccount) => + isEvmAccountType(account.type), ); - }, [accounts, internalAccounts]); + }, [accounts]); const currentAccount = useSelector(getSelectedInternalAccount); const currentAccountAddress = isEvmAccountType(currentAccount.type) @@ -157,7 +153,7 @@ export const ConnectPage: React.FC = ({ { const accounts = useSelector(getMetaMaskAccountsOrdered); - const internalAccounts = useSelector(getInternalAccounts); - // We should stop using mergeAccounts and write a new selector instead - const mergedAccounts = mergeAccounts(accounts, internalAccounts); - const account = mergedAccounts.find( + const account = accounts.find( (internalAccount: { address: string }) => internalAccount.address === address, ) as MergedInternalAccount; diff --git a/ui/selectors/selectors.test.js b/ui/selectors/selectors.test.js index aa6ae125bd4b..fce129745561 100644 --- a/ui/selectors/selectors.test.js +++ b/ui/selectors/selectors.test.js @@ -1297,6 +1297,7 @@ describe('Selectors', () => { 'npm:@metamask/test-snap-networkAccess': false, 'npm:@metamask/test-snap-notify': false, 'npm:@metamask/test-snap-wasm': false, + 'local:snap-id': false, }); }); @@ -1520,7 +1521,7 @@ describe('Selectors', () => { name: 'Snap Account 1', snap: { enabled: true, - id: 'snap-id', + id: 'local:snap-id', name: 'snap-name', }, }, From 6e05923cb64e689628697ac05cbee8c86d7b12e2 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 9 Jan 2025 16:04:20 +0000 Subject: [PATCH 11/23] fix: Catch errors from the assets controller (#29439) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR addresses a group of errors reported on Sentry that occur when `getTokenStandardAndDetails` in `assetsContractController` returns "Unable to determine contract standard". [Sentry Issue Link](https://metamask.sentry.io/issues/5660074561/?project=273505&query=is%3Aunresolved%20getTokenStandardAndDetails&referrer=issue-stream&sort=date&statsPeriod=7d&stream_index=0). The error happens fairly often because only contracts that strictly adhere to the standard ABIs for ERC20, ERC721, and ERC1155 are correctly parsed. When a contract does not conform to these standards, the function fails and throws an error. This PR introduces error handling to catch and silence the error when assetsContractController.getTokenStandardAndDetails fails to determine the contract standard. The control flow of the function is adjusted to ensure it continues to execute normally even when the error is caught. ### Changes - Added a try...catch block around the assetsContractController.getTokenStandardAndDetails call to catch and log the error. - Ensured that the function continues to execute and return appropriate details even when the error is caught. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29439?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/25212 ## **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. --- app/scripts/metamask-controller.js | 63 +++++++++++++++++------------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 49517145bcd7..b869cd704633 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -4521,39 +4521,46 @@ export default class MetamaskController extends EventEmitter { // or if it is true but the `fetchTokenBalance`` call failed. In either case, we should // attempt to retrieve details from `assetsContractController.getTokenStandardAndDetails` if (details === undefined) { - details = await this.assetsContractController.getTokenStandardAndDetails( - address, - userAddress, - tokenId, - ); + try { + details = + await this.assetsContractController.getTokenStandardAndDetails( + address, + userAddress, + tokenId, + ); + } catch (e) { + log.warn(`Failed to get token standard and details. Error: ${e}`); + } } - const tokenDetailsStandardIsERC1155 = isEqualCaseInsensitive( - details.standard, - TokenStandard.ERC1155, - ); + if (details) { + const tokenDetailsStandardIsERC1155 = isEqualCaseInsensitive( + details.standard, + TokenStandard.ERC1155, + ); - if (tokenDetailsStandardIsERC1155) { - try { - const balance = await fetchERC1155Balance( - address, - userAddress, - tokenId, - this.provider, - ); + if (tokenDetailsStandardIsERC1155) { + try { + const balance = await fetchERC1155Balance( + address, + userAddress, + tokenId, + this.provider, + ); - const balanceToUse = balance?._hex - ? parseInt(balance._hex, 16).toString() - : null; + const balanceToUse = balance?._hex + ? parseInt(balance._hex, 16).toString() + : null; - details = { - ...details, - balance: balanceToUse, - }; - } catch (e) { - // If the `fetchTokenBalance` call failed, `details` remains undefined, and we - // fall back to the below `assetsContractController.getTokenStandardAndDetails` call - log.warn('Failed to get token balance. Error:', e); + details = { + ...details, + balance: balanceToUse, + }; + } catch (e) { + // If the `fetchTokenBalance` call failed, `details` remains undefined, and we + // fall back to the below `assetsContractController.getTokenStandardAndDetails` call + log.warn('Failed to get token balance. Error:', e); + } } } From 4cbce35280445237862f064dcb4f07d2c12637f9 Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Thu, 9 Jan 2025 10:18:59 -0600 Subject: [PATCH 12/23] chore: Remove toggle to turn on/off Per Dapp Selected Network Feature (#29301) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The "Select networks for each site" preference toggle on the experimental settings page has been live for many releases now since the toggle has been turned on by default. We meant to remove it a while ago. Screenshot 2024-11-19 at 10 58 04 AM This PR removes this toggle ~and integrates new versions of the QueuedRequestController and SelectedNetworkController which remove the backend logic it operated.~ - We have delayed the updates to the controllers side because of some mobile side requirements. See [this PR](https://github.com/MetaMask/core/pull/5065#issue-2736965186) for more context: > We are not yet ready to release per-dapp selected network functionality on the mobile client and with this change there is no clean way to https://github.com/MetaMask/metamask-mobile/issues/12434#issuecomment-2537358920 in the mobile client without having the Domains state starting to populate and possibly become corrupt since its not being consumed by/updated by the frontend in the expected way and may need to be migrated away when its time to actually start using the controller. > Without this revert the @MetaMask/wallet-framework team is blocked from completing their goal to get both clients up to the latest versions of all controllers. Beyond the fact that this removal is overdue, another reason we should remove this now is that having this setting when turned off is [causing a bug](https://github.com/MetaMask/metamask-extension/issues/28441) with `wallet_switchEthereumChain` and the interaction with the new chain permissions feature. ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/2844 ## **Manual testing steps** 1. Go to experimental tab of settings 2. See that there is no longer a toggleable preference called "Selected Networks for each site" 3. See that Per Dapp Selected Network Functionality is still on by default ## **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. --- app/_locales/de/messages.json | 12 - app/_locales/el/messages.json | 12 - app/_locales/en/messages.json | 12 - app/_locales/en_GB/messages.json | 12 - app/_locales/es/messages.json | 12 - app/_locales/fr/messages.json | 12 - app/_locales/hi/messages.json | 12 - app/_locales/id/messages.json | 12 - app/_locales/ja/messages.json | 12 - app/_locales/ko/messages.json | 12 - app/_locales/pt/messages.json | 12 - app/_locales/ru/messages.json | 12 - app/_locales/tl/messages.json | 12 - app/_locales/tr/messages.json | 12 - app/_locales/vi/messages.json | 12 - app/_locales/zh_CN/messages.json | 12 - app/scripts/background.js | 6 +- app/scripts/constants/sentry-state.ts | 1 - .../preferences-controller.test.ts | 13 - .../controllers/preferences-controller.ts | 26 -- app/scripts/fixtures/with-preferences.js | 1 - app/scripts/lib/backup.test.js | 1 - app/scripts/metamask-controller.js | 66 +--- app/scripts/migrations/136.test.ts | 56 +++ app/scripts/migrations/136.ts | 37 ++ app/scripts/migrations/index.js | 1 + test/e2e/default-fixture.js | 1 - test/e2e/fixture-builder.js | 11 +- test/e2e/json-rpc/switchEthereumChain.spec.js | 121 ------ .../pages/settings/experimental-settings.ts | 8 - test/e2e/restore/MetaMaskUserData.json | 1 - .../alerts/queued-confirmations.spec.ts | 8 +- .../review-switch-permission-page.spec.js | 2 +- ...rs-after-init-opt-in-background-state.json | 1 - .../errors-after-init-opt-in-ui-state.json | 1 - ...s-before-init-opt-in-background-state.json | 1 - .../errors-before-init-opt-in-ui-state.json | 1 - .../multichain/all-permissions-page.spec.ts | 1 - .../batch-txs-per-dapp-diff-network.spec.js | 2 - .../batch-txs-per-dapp-extra-tx.spec.js | 2 - .../batch-txs-per-dapp-same-network.spec.js | 2 - .../request-queuing/chainid-check.spec.js | 369 +++++------------- .../dapp1-send-dapp2-signTypedData.spec.js | 2 - .../dapp1-subscribe-network-switch.spec.ts | 1 - ...-switch-dapp2-eth-request-accounts.spec.js | 3 - .../dapp1-switch-dapp2-send.spec.js | 4 - .../request-queuing/enable-queuing.spec.js | 47 --- ...multi-dapp-sendTx-revokePermission.spec.js | 2 - .../multiple-networks-dapps-txs.spec.js | 2 - .../sendTx-revokePermissions.spec.ts | 1 - .../sendTx-switchChain-sendTx.spec.js | 2 +- .../request-queuing/switch-network.spec.js | 2 - .../switchChain-sendTx.spec.js | 2 +- .../switchChain-watchAsset.spec.js | 2 +- test/e2e/tests/request-queuing/ui.spec.js | 25 +- .../watchAsset-switchChain-watchAsset.spec.js | 2 +- .../data/integration-init-state.json | 1 - .../data/onboarding-completion-route.json | 1 - .../nfts/nfts-items/nfts-items.stories.tsx | 1 - .../network-list-menu.test.js | 1 - .../network-list-menu/network-list-menu.tsx | 8 +- ui/index.js | 6 +- ui/pages/routes/routes.component.js | 3 - ui/pages/routes/routes.container.js | 2 - .../experimental-tab.component.tsx | 18 - .../experimental-tab.container.ts | 4 - .../experimental-tab/experimental-tab.test.js | 2 +- ui/selectors/selectors.js | 12 - ui/selectors/selectors.test.js | 1 - ui/store/actions.ts | 8 - 70 files changed, 241 insertions(+), 856 deletions(-) create mode 100644 app/scripts/migrations/136.test.ts create mode 100644 app/scripts/migrations/136.ts delete mode 100644 test/e2e/tests/request-queuing/enable-queuing.spec.js diff --git a/app/_locales/de/messages.json b/app/_locales/de/messages.json index 5a499ce7dc6d..c466b7273b95 100644 --- a/app/_locales/de/messages.json +++ b/app/_locales/de/messages.json @@ -5972,18 +5972,6 @@ "message": "An: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "Dadurch können Sie ein Netzwerk für jede Website auswählen, anstatt ein einziges Netzwerk für alle Websites auszuwählen. Diese Funktion verhindert, dass Sie manuell zwischen den Netzwerken wechseln müssen, was die Benutzerfreundlichkeit auf bestimmten Websites beeinträchtigen könnte." - }, - "toggleRequestQueueField": { - "message": "Wählen Sie Netzwerke für jede Website" - }, - "toggleRequestQueueOff": { - "message": "Aus" - }, - "toggleRequestQueueOn": { - "message": "An" - }, "token": { "message": "Token" }, diff --git a/app/_locales/el/messages.json b/app/_locales/el/messages.json index d930bafaa3d7..c4cce0299949 100644 --- a/app/_locales/el/messages.json +++ b/app/_locales/el/messages.json @@ -5972,18 +5972,6 @@ "message": "Προς: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "Αυτό σας επιτρέπει να επιλέξετε ένα δίκτυο για κάθε ιστότοπο αντί για ένα μόνο επιλεγμένο δίκτυο για όλους τους ιστότοπους. Αυτή η λειτουργία θα σας αποτρέψει από το να αλλάζετε δίκτυα χειροκίνητα, το οποίο μπορεί να διαταράξει την εμπειρία του χρήστη σε ορισμένους ιστότοπους." - }, - "toggleRequestQueueField": { - "message": "Επιλογή δικτύων για κάθε ιστότοπο" - }, - "toggleRequestQueueOff": { - "message": "Απενεργοποίηση" - }, - "toggleRequestQueueOn": { - "message": "Ενεργοποίηση" - }, "token": { "message": "Token" }, diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index a45457e8b4c1..d16ea6af6580 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -6274,18 +6274,6 @@ "toggleDecodeDescription": { "message": "We use 4byte.directory and Sourcify services to decode and display more readable transaction data. This helps you understand the outcome of pending and past transactions, but can result in your IP address being shared." }, - "toggleRequestQueueDescription": { - "message": "This allows you to select a network for each site instead of a single selected network for all sites. This feature will prevent you from switching networks manually, which may break your user experience on certain sites." - }, - "toggleRequestQueueField": { - "message": "Select networks for each site" - }, - "toggleRequestQueueOff": { - "message": "Off" - }, - "toggleRequestQueueOn": { - "message": "On" - }, "token": { "message": "Token" }, diff --git a/app/_locales/en_GB/messages.json b/app/_locales/en_GB/messages.json index f66111064a90..a915059f262f 100644 --- a/app/_locales/en_GB/messages.json +++ b/app/_locales/en_GB/messages.json @@ -5744,18 +5744,6 @@ "toggleEthSignOn": { "message": "ON (Not recommended)" }, - "toggleRequestQueueDescription": { - "message": "This allows you to select a network for each site instead of a single selected network for all sites. This feature will prevent you from switching networks manually, which may break your user experience on certain sites." - }, - "toggleRequestQueueField": { - "message": "Select networks for each site" - }, - "toggleRequestQueueOff": { - "message": "Off" - }, - "toggleRequestQueueOn": { - "message": "On" - }, "token": { "message": "Token" }, diff --git a/app/_locales/es/messages.json b/app/_locales/es/messages.json index bfb53061f73b..7e8253d37d46 100644 --- a/app/_locales/es/messages.json +++ b/app/_locales/es/messages.json @@ -5972,18 +5972,6 @@ "message": "Para: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "Esto le permite seleccionar una red para cada sitio en lugar de una única red seleccionada para todos los sitios. Esta función evitará que cambie de red manualmente, lo que puede afectar su experiencia de usuario en ciertos sitios." - }, - "toggleRequestQueueField": { - "message": "Seleccionar redes para cada sitio" - }, - "toggleRequestQueueOff": { - "message": "Desactivado" - }, - "toggleRequestQueueOn": { - "message": "Activado" - }, "token": { "message": "Token" }, diff --git a/app/_locales/fr/messages.json b/app/_locales/fr/messages.json index a86d465b786d..1d4777e5aaea 100644 --- a/app/_locales/fr/messages.json +++ b/app/_locales/fr/messages.json @@ -5972,18 +5972,6 @@ "message": "Vers : $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "Cette fonction vous permet de sélectionner un réseau pour chaque site au lieu d’un seul réseau pour tous les sites. Vous n’aurez donc pas à changer manuellement de réseau, ce qui pourrait nuire à l’expérience utilisateur sur certains sites." - }, - "toggleRequestQueueField": { - "message": "Sélectionnez les réseaux pour chaque site" - }, - "toggleRequestQueueOff": { - "message": "Désactiver" - }, - "toggleRequestQueueOn": { - "message": "Activer" - }, "token": { "message": "Jeton" }, diff --git a/app/_locales/hi/messages.json b/app/_locales/hi/messages.json index 144db40e41a3..39bd7241e6cc 100644 --- a/app/_locales/hi/messages.json +++ b/app/_locales/hi/messages.json @@ -5972,18 +5972,6 @@ "message": "प्रति: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "ऐसा करके, आप सभी साइटों के लिए कोई सिंगल नेटवर्क चुनने के बजाय हरेक साइट के लिए एक नेटवर्क चुन सकते हैं। यह फीचर आपको मैन्युअल तरीके से नेटवर्क स्विच करने से रोकता है, इस वजह से कुछ साइटों पर आपका यूज़र अनुभव ख़राब हो सकता है।" - }, - "toggleRequestQueueField": { - "message": "हरेक साइट के लिए नेटवर्क चुनें" - }, - "toggleRequestQueueOff": { - "message": "बंद करें" - }, - "toggleRequestQueueOn": { - "message": "चालू करें" - }, "token": { "message": "टोकन" }, diff --git a/app/_locales/id/messages.json b/app/_locales/id/messages.json index 44b2bf804727..4faf653d1419 100644 --- a/app/_locales/id/messages.json +++ b/app/_locales/id/messages.json @@ -5972,18 +5972,6 @@ "message": "Ke: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "Hal ini memungkinkan Anda memilih jaringan untuk setiap situs, daripada satu jaringan yang dipilih untuk semua situs. Fitur ini akan mencegah Anda berpindah jaringan secara manual, yang dapat merusak pengalaman pengguna di situs tertentu." - }, - "toggleRequestQueueField": { - "message": "Pilih jaringan untuk setiap situs" - }, - "toggleRequestQueueOff": { - "message": "Nonaktif" - }, - "toggleRequestQueueOn": { - "message": "Aktif" - }, "token": { "message": "Token" }, diff --git a/app/_locales/ja/messages.json b/app/_locales/ja/messages.json index 70d6da31301e..96d3a404faa4 100644 --- a/app/_locales/ja/messages.json +++ b/app/_locales/ja/messages.json @@ -5972,18 +5972,6 @@ "message": "移動先: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "これにより、選択した単一のネットワークをすべてのサイトで使用するのではなく、サイトごとにネットワークを選択できます。この機能により、特定のサイトでのユーザーエクスペリエンスの妨げとなる、ネットワークの手動切り替えが不要になります。" - }, - "toggleRequestQueueField": { - "message": "サイトごとにネットワークを選択する" - }, - "toggleRequestQueueOff": { - "message": "オフ" - }, - "toggleRequestQueueOn": { - "message": "オン" - }, "token": { "message": "トークン" }, diff --git a/app/_locales/ko/messages.json b/app/_locales/ko/messages.json index 6978164f82f4..ae3942931205 100644 --- a/app/_locales/ko/messages.json +++ b/app/_locales/ko/messages.json @@ -5972,18 +5972,6 @@ "message": "수신: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "이 기능을 이용하면 모든 사이트에 한 가지 네트워크를 선택하여 사용하는 대신, 사이트별로 네트워크를 다르게 선택할 수 있습니다. 이 기능을 사용하면 수동으로 네트워크를 전환하지 않아도 되므로 특정 사이트에서 사용자 경험이 저해되지 않습니다." - }, - "toggleRequestQueueField": { - "message": "각 사이트별 네트워크 선택" - }, - "toggleRequestQueueOff": { - "message": "끄기" - }, - "toggleRequestQueueOn": { - "message": "켜기" - }, "token": { "message": "토큰" }, diff --git a/app/_locales/pt/messages.json b/app/_locales/pt/messages.json index 7819be595e94..7f9f0cb72311 100644 --- a/app/_locales/pt/messages.json +++ b/app/_locales/pt/messages.json @@ -5972,18 +5972,6 @@ "message": "Para: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "Isso permite que você selecione uma rede para cada site em vez de uma única rede selecionada para todos eles. O recurso evitará que você alterne manualmente entre redes, o que pode atrapalhar sua experiência de usuário em determinados sites." - }, - "toggleRequestQueueField": { - "message": "Selecionar redes para cada site" - }, - "toggleRequestQueueOff": { - "message": "Não" - }, - "toggleRequestQueueOn": { - "message": "Sim" - }, "token": { "message": "Token" }, diff --git a/app/_locales/ru/messages.json b/app/_locales/ru/messages.json index 5e281f34ac05..e785dfeb2d67 100644 --- a/app/_locales/ru/messages.json +++ b/app/_locales/ru/messages.json @@ -5972,18 +5972,6 @@ "message": "Адресат: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "Это позволяет вам выбрать сеть для каждого сайта вместо одной выбранной сети для всех сайтов. Эта функция не позволит вам переключать сети вручную, что может снизить удобство использования некоторых сайтов." - }, - "toggleRequestQueueField": { - "message": "Выберите сети для каждого сайта" - }, - "toggleRequestQueueOff": { - "message": "Выкл." - }, - "toggleRequestQueueOn": { - "message": "Вкл." - }, "token": { "message": "Токен" }, diff --git a/app/_locales/tl/messages.json b/app/_locales/tl/messages.json index 5a6bce602970..c854917ae676 100644 --- a/app/_locales/tl/messages.json +++ b/app/_locales/tl/messages.json @@ -5972,18 +5972,6 @@ "message": "Para kay/sa: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "Pinahihintulutan ka nitong pumili ng network para sa bawat site sa halip na iisang piniling network para sa lahat ng site. Pipigilan ka ng feature na ito na magpalit ng network nang mano-mano, na maaaring makasira sa iyong karanasan bilang user sa ilang partikular na site." - }, - "toggleRequestQueueField": { - "message": "Piliin ang mga network para sa bawat site" - }, - "toggleRequestQueueOff": { - "message": "Naka-off" - }, - "toggleRequestQueueOn": { - "message": "Naka-on" - }, "token": { "message": "Token" }, diff --git a/app/_locales/tr/messages.json b/app/_locales/tr/messages.json index 046905392710..dc26bb137e19 100644 --- a/app/_locales/tr/messages.json +++ b/app/_locales/tr/messages.json @@ -5972,18 +5972,6 @@ "message": "Alıcı: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "Bu, tüm siteler için tek bir seçili ağ yerine her bir site için bir ağ seçebilmenize olanak sağlar. Bu özellik, manuel olarak ağ değiştirmenizi önleyebilir ve bu da belirli sitelerde kullanıcı deneyiminizi bozabilir." - }, - "toggleRequestQueueField": { - "message": "Her site için ağ seçin" - }, - "toggleRequestQueueOff": { - "message": "Kapalı" - }, - "toggleRequestQueueOn": { - "message": "Açık" - }, "token": { "message": "Token" }, diff --git a/app/_locales/vi/messages.json b/app/_locales/vi/messages.json index a6baaff586aa..5f372083000f 100644 --- a/app/_locales/vi/messages.json +++ b/app/_locales/vi/messages.json @@ -5972,18 +5972,6 @@ "message": "Đến: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "Tính năng này cho phép bạn chọn mạng cho từng trang web thay vì một mạng duy nhất được chọn cho tất cả các trang web. Tính năng này sẽ ngăn bạn chuyển đổi mạng theo cách thủ công, điều này có thể ảnh hưởng đến trải nghiệm người dùng của bạn trên một số trang web." - }, - "toggleRequestQueueField": { - "message": "Chọn mạng cho từng trang web" - }, - "toggleRequestQueueOff": { - "message": "Tắt" - }, - "toggleRequestQueueOn": { - "message": "Bật" - }, "token": { "message": "Token" }, diff --git a/app/_locales/zh_CN/messages.json b/app/_locales/zh_CN/messages.json index 5472a92a5660..5ea2340440a5 100644 --- a/app/_locales/zh_CN/messages.json +++ b/app/_locales/zh_CN/messages.json @@ -5972,18 +5972,6 @@ "message": "至：$1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, - "toggleRequestQueueDescription": { - "message": "这使您可以为每个网站选择网络，而不是为所有网站选择同一个网络。此功能将阻止您手动切换网络，这可能会破坏您在某些网站上的用户体验。" - }, - "toggleRequestQueueField": { - "message": "为每个网站选择网络" - }, - "toggleRequestQueueOff": { - "message": "关" - }, - "toggleRequestQueueOn": { - "message": "开" - }, "token": { "message": "代币" }, diff --git a/app/scripts/background.js b/app/scripts/background.js index c6a3bccd46aa..029adf026841 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -1153,10 +1153,8 @@ export function setupController( controller.appStateController.waitingForUnlock.length + controller.approvalController.getTotalApprovalCount(); - if (controller.preferencesController.getUseRequestQueue()) { - pendingApprovalCount += - controller.queuedRequestController.state.queuedRequestCount; - } + pendingApprovalCount += + controller.queuedRequestController.state.queuedRequestCount; return pendingApprovalCount; } catch (error) { console.error('Failed to get pending approval count:', error); diff --git a/app/scripts/constants/sentry-state.ts b/app/scripts/constants/sentry-state.ts index 705b33f450ba..23e4c2d45af9 100644 --- a/app/scripts/constants/sentry-state.ts +++ b/app/scripts/constants/sentry-state.ts @@ -275,7 +275,6 @@ export const SENTRY_BACKGROUND_STATE = { useNonceField: true, usePhishDetect: true, useTokenDetection: true, - useRequestQueue: true, useTransactionSimulations: true, enableMV3TimestampSave: true, }, diff --git a/app/scripts/controllers/preferences-controller.test.ts b/app/scripts/controllers/preferences-controller.test.ts index f30e938e635e..eac95430ec3a 100644 --- a/app/scripts/controllers/preferences-controller.test.ts +++ b/app/scripts/controllers/preferences-controller.test.ts @@ -640,19 +640,6 @@ describe('preferences controller', () => { }); }); - describe('useRequestQueue', () => { - it('defaults useRequestQueue to true', () => { - const { controller } = setupController({}); - expect(controller.state.useRequestQueue).toStrictEqual(true); - }); - - it('setUseRequestQueue to false', () => { - const { controller } = setupController({}); - controller.setUseRequestQueue(false); - expect(controller.state.useRequestQueue).toStrictEqual(false); - }); - }); - describe('addSnapAccountEnabled', () => { it('defaults addSnapAccountEnabled to false', () => { const { controller } = setupController({}); diff --git a/app/scripts/controllers/preferences-controller.ts b/app/scripts/controllers/preferences-controller.ts index 217fe43b022d..8a09539ea917 100644 --- a/app/scripts/controllers/preferences-controller.ts +++ b/app/scripts/controllers/preferences-controller.ts @@ -143,7 +143,6 @@ export type PreferencesControllerState = Omit< useMultiAccountBalanceChecker: boolean; use4ByteResolution: boolean; useCurrencyRateCheck: boolean; - useRequestQueue: boolean; ///: BEGIN:ONLY_INCLUDE_IF(build-flask) watchEthereumAccountEnabled: boolean; ///: END:ONLY_INCLUDE_IF @@ -190,7 +189,6 @@ export const getDefaultPreferencesControllerState = useNftDetection: true, use4ByteResolution: true, useCurrencyRateCheck: true, - useRequestQueue: true, openSeaEnabled: true, securityAlertsEnabled: true, watchEthereumAccountEnabled: false, @@ -342,10 +340,6 @@ const controllerMetadata = { persist: true, anonymous: true, }, - useRequestQueue: { - persist: true, - anonymous: true, - }, openSeaEnabled: { persist: true, anonymous: true, @@ -642,17 +636,6 @@ export class PreferencesController extends BaseController< }); } - /** - * Setter for the `useRequestQueue` property - * - * @param val - Whether or not the user wants to have requests queued if network change is required. - */ - setUseRequestQueue(val: boolean): void { - this.update((state) => { - state.useRequestQueue = val; - }); - } - /** * Setter for the `openSeaEnabled` property * @@ -865,15 +848,6 @@ export class PreferencesController extends BaseController< return selectedAccount.address; } - /** - * Getter for the `useRequestQueue` property - * - * @returns whether this option is on or off. - */ - getUseRequestQueue(): boolean { - return this.state.useRequestQueue; - } - /** * Sets a custom label for an account * diff --git a/app/scripts/fixtures/with-preferences.js b/app/scripts/fixtures/with-preferences.js index c3a482ef8f94..8d1845f728bb 100644 --- a/app/scripts/fixtures/with-preferences.js +++ b/app/scripts/fixtures/with-preferences.js @@ -31,7 +31,6 @@ export const FIXTURES_PREFERENCES = { useTokenDetection: true, useCurrencyRateCheck: true, useMultiAccountBalanceChecker: true, - useRequestQueue: true, theme: 'light', useExternalNameSources: true, useTransactionSimulations: true, diff --git a/app/scripts/lib/backup.test.js b/app/scripts/lib/backup.test.js index 826aa04018d9..242b9174add4 100644 --- a/app/scripts/lib/backup.test.js +++ b/app/scripts/lib/backup.test.js @@ -175,7 +175,6 @@ const jsonData = JSON.stringify({ theme: 'light', customNetworkListEnabled: false, textDirection: 'auto', - useRequestQueue: true, }, internalAccounts: { accounts: { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index b869cd704633..38ec6023b264 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1334,13 +1334,13 @@ export default class MetamaskController extends EventEmitter { ], }), state: initState.SelectedNetworkController, - useRequestQueuePreference: - this.preferencesController.state.useRequestQueue, - onPreferencesStateChange: (listener) => { - preferencesMessenger.subscribe( - 'PreferencesController:stateChange', - listener, - ); + useRequestQueuePreference: true, + onPreferencesStateChange: () => { + // noop + // we have removed the ability to toggle the useRequestQueue preference + // both useRequestQueue and onPreferencesStateChange will be removed + // once mobile supports per dapp network selection + // see https://github.com/MetaMask/core/pull/5065#issue-2736965186 }, domainProxyMap: new WeakRefObjectMap(), }); @@ -3360,9 +3360,7 @@ export default class MetamaskController extends EventEmitter { * @returns {Promise<{ isUnlocked: boolean, networkVersion: string, chainId: string, accounts: string[] }>} An object with relevant state properties. */ async getProviderState(origin) { - const providerNetworkState = await this.getProviderNetworkState( - this.preferencesController.getUseRequestQueue() ? origin : undefined, - ); + const providerNetworkState = await this.getProviderNetworkState(origin); return { isUnlocked: this.isUnlocked(), @@ -3517,9 +3515,6 @@ export default class MetamaskController extends EventEmitter { setOpenSeaEnabled: preferencesController.setOpenSeaEnabled.bind( preferencesController, ), - getUseRequestQueue: this.preferencesController.getUseRequestQueue.bind( - this.preferencesController, - ), getProviderConfig: () => getProviderConfig({ metamask: this.networkController.state, @@ -3569,7 +3564,6 @@ export default class MetamaskController extends EventEmitter { preferencesController.setUseTransactionSimulations.bind( preferencesController, ), - setUseRequestQueue: this.setUseRequestQueue.bind(this), setIpfsGateway: preferencesController.setIpfsGateway.bind( preferencesController, ), @@ -5473,14 +5467,6 @@ export default class MetamaskController extends EventEmitter { this.sendUpdate(); } - //============================================================================= - // REQUEST QUEUE - //============================================================================= - - setUseRequestQueue(value) { - this.preferencesController.setUseRequestQueue(value); - } - //============================================================================= // SETUP //============================================================================= @@ -5973,12 +5959,12 @@ export default class MetamaskController extends EventEmitter { enqueueRequest: this.queuedRequestController.enqueueRequest.bind( this.queuedRequestController, ), - useRequestQueue: this.preferencesController.getUseRequestQueue.bind( - this.preferencesController, - ), shouldEnqueueRequest: (request) => { return methodsThatShouldBeEnqueued.includes(request.method); }, + // This will be removed once we can actually remove useRequestQueue state + // i.e. unrevert https://github.com/MetaMask/core/pull/5065 + useRequestQueue: () => true, }); engine.push(requestQueueMiddleware); @@ -7210,31 +7196,17 @@ export default class MetamaskController extends EventEmitter { } async _notifyChainChange() { - if (this.preferencesController.getUseRequestQueue()) { - this.notifyAllConnections(async (origin) => ({ - method: NOTIFICATION_NAMES.chainChanged, - params: await this.getProviderNetworkState(origin), - })); - } else { - this.notifyAllConnections({ - method: NOTIFICATION_NAMES.chainChanged, - params: await this.getProviderNetworkState(), - }); - } + this.notifyAllConnections(async (origin) => ({ + method: NOTIFICATION_NAMES.chainChanged, + params: await this.getProviderNetworkState(origin), + })); } async _notifyChainChangeForConnection(connection, origin) { - if (this.preferencesController.getUseRequestQueue()) { - this.notifyConnection(connection, { - method: NOTIFICATION_NAMES.chainChanged, - params: await this.getProviderNetworkState(origin), - }); - } else { - this.notifyConnection(connection, { - method: NOTIFICATION_NAMES.chainChanged, - params: await this.getProviderNetworkState(), - }); - } + this.notifyConnection(connection, { + method: NOTIFICATION_NAMES.chainChanged, + params: await this.getProviderNetworkState(origin), + }); } async _onFinishedTransaction(transactionMeta) { diff --git a/app/scripts/migrations/136.test.ts b/app/scripts/migrations/136.test.ts new file mode 100644 index 000000000000..01fc9d49d69f --- /dev/null +++ b/app/scripts/migrations/136.test.ts @@ -0,0 +1,56 @@ +import { migrate, version } from './136'; + +const oldVersion = 135; + +describe(`migration #${version}`, () => { + it('updates the version metadata', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: {}, + }; + const newStorage = await migrate(oldStorage); + expect(newStorage.meta).toStrictEqual({ version }); + }); + describe(`migration #${version}`, () => { + it('removes the useRequestQueue preference', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + PreferencesController: { + useRequestQueue: true, + otherPreference: true, + }, + }, + }; + const expectedData = { + PreferencesController: { + otherPreference: true, + }, + }; + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual(expectedData); + }); + + it('does nothing to other PreferencesController state if there is a useRequestQueue preference', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + PreferencesController: { + existingPreference: true, + }, + }, + }; + + const expectedData = { + PreferencesController: { + existingPreference: true, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual(expectedData); + }); + }); +}); diff --git a/app/scripts/migrations/136.ts b/app/scripts/migrations/136.ts new file mode 100644 index 000000000000..3f6bfc661292 --- /dev/null +++ b/app/scripts/migrations/136.ts @@ -0,0 +1,37 @@ +import { hasProperty, isObject } from '@metamask/utils'; +import { cloneDeep } from 'lodash'; + +type VersionedData = { + meta: { version: number }; + data: Record; +}; +export const version = 136; +/** + * This migration removes the useRequestQueue preference from the user's preferences + * + * @param originalVersionedData - Versioned MetaMask extension state, exactly what we persist to dist. + * @param originalVersionedData.meta - State metadata. + * @param originalVersionedData.meta.version - The current state version. + * @param originalVersionedData.data - The persisted MetaMask state, keyed by controller. + * @returns Updated versioned MetaMask extension state. + */ +export async function migrate( + originalVersionedData: VersionedData, +): Promise { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + transformState(versionedData.data); + return versionedData; +} +function transformState( + state: Record, +): Record { + if ( + hasProperty(state, 'PreferencesController') && + isObject(state.PreferencesController) && + hasProperty(state.PreferencesController, 'useRequestQueue') + ) { + delete state.PreferencesController.useRequestQueue; + } + return state; +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index 35b72816b388..797b06d85cb5 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -159,6 +159,7 @@ const migrations = [ require('./133.2'), require('./134'), require('./135'), + require('./136'), ]; export default migrations; diff --git a/test/e2e/default-fixture.js b/test/e2e/default-fixture.js index 5bb2a24d0c5c..51338fd3eea8 100644 --- a/test/e2e/default-fixture.js +++ b/test/e2e/default-fixture.js @@ -244,7 +244,6 @@ function defaultFixture(inputChainId = CHAIN_IDS.LOCALHOST) { useTokenDetection: false, useCurrencyRateCheck: true, useMultiAccountBalanceChecker: true, - useRequestQueue: true, isMultiAccountBalancesEnabled: true, showIncomingTransactions: { [ETHERSCAN_SUPPORTED_CHAIN_IDS.MAINNET]: true, diff --git a/test/e2e/fixture-builder.js b/test/e2e/fixture-builder.js index 020c4db1c64b..6acf95bef637 100644 --- a/test/e2e/fixture-builder.js +++ b/test/e2e/fixture-builder.js @@ -95,7 +95,6 @@ function onboardingFixture() { useTokenDetection: false, useCurrencyRateCheck: true, useMultiAccountBalanceChecker: true, - useRequestQueue: true, isMultiAccountBalancesEnabled: true, showIncomingTransactions: { [ETHERSCAN_SUPPORTED_CHAIN_IDS.MAINNET]: true, @@ -845,15 +844,7 @@ class FixtureBuilder { [DAPP_ONE_URL]: '76e9cd59-d8e2-47e7-b369-9c205ccb602c', }, }), - this.withPreferencesControllerUseRequestQueueEnabled(), - ); - } - - withPreferencesControllerUseRequestQueueEnabled() { - return merge( - this.withPreferencesController({ - useRequestQueue: true, - }), + this, ); } diff --git a/test/e2e/json-rpc/switchEthereumChain.spec.js b/test/e2e/json-rpc/switchEthereumChain.spec.js index e55dfc622865..8395a3ef9603 100644 --- a/test/e2e/json-rpc/switchEthereumChain.spec.js +++ b/test/e2e/json-rpc/switchEthereumChain.spec.js @@ -34,30 +34,6 @@ describe('Switch Ethereum Chain for two dapps', function () { await tempToggleSettingRedesignedTransactionConfirmations(driver); - // Open settings menu button - const accountOptionsMenuSelector = - '[data-testid="account-options-menu-button"]'; - await driver.waitForSelector(accountOptionsMenuSelector); - await driver.clickElement(accountOptionsMenuSelector); - - // Click settings from dropdown menu - const globalMenuSettingsSelector = - '[data-testid="global-menu-settings"]'; - await driver.waitForSelector(globalMenuSettingsSelector); - await driver.clickElement(globalMenuSettingsSelector); - - // Click Experimental tab - const experimentalTabRawLocator = { - text: 'Experimental', - tag: 'div', - }; - await driver.clickElement(experimentalTabRawLocator); - - // Toggle off request queue setting (on by default now) - await driver.clickElement( - '[data-testid="experimental-setting-toggle-request-queue"]', - ); - // open two dapps const dappTwo = await openDapp(driver, undefined, DAPP_ONE_URL); const dappOne = await openDapp(driver, undefined, DAPP_URL); @@ -184,31 +160,6 @@ describe('Switch Ethereum Chain for two dapps', function () { }, async ({ driver }) => { await unlockWallet(driver); - - // Open settings menu button - const accountOptionsMenuSelector = - '[data-testid="account-options-menu-button"]'; - await driver.waitForSelector(accountOptionsMenuSelector); - await driver.clickElement(accountOptionsMenuSelector); - - // Click settings from dropdown menu - const globalMenuSettingsSelector = - '[data-testid="global-menu-settings"]'; - await driver.waitForSelector(globalMenuSettingsSelector); - await driver.clickElement(globalMenuSettingsSelector); - - // Click Experimental tab - const experimentalTabRawLocator = { - text: 'Experimental', - tag: 'div', - }; - await driver.clickElement(experimentalTabRawLocator); - - // Toggle off request queue setting (on by default now) - await driver.clickElement( - '[data-testid="experimental-setting-toggle-request-queue"]', - ); - // open two dapps const dappOne = await openDapp(driver, undefined, DAPP_URL); const dappTwo = await openDapp(driver, undefined, DAPP_ONE_URL); @@ -274,30 +225,6 @@ describe('Switch Ethereum Chain for two dapps', function () { async ({ driver }) => { await unlockWallet(driver); - // Open settings menu button - const accountOptionsMenuSelector = - '[data-testid="account-options-menu-button"]'; - await driver.waitForSelector(accountOptionsMenuSelector); - await driver.clickElement(accountOptionsMenuSelector); - - // Click settings from dropdown menu - const globalMenuSettingsSelector = - '[data-testid="global-menu-settings"]'; - await driver.waitForSelector(globalMenuSettingsSelector); - await driver.clickElement(globalMenuSettingsSelector); - - // Click Experimental tab - const experimentalTabRawLocator = { - text: 'Experimental', - tag: 'div', - }; - await driver.clickElement(experimentalTabRawLocator); - - // Toggle off request queue setting (on by default now) - await driver.clickElement( - '[data-testid="experimental-setting-toggle-request-queue"]', - ); - // open two dapps await openDapp(driver, undefined, DAPP_URL); await openDapp(driver, undefined, DAPP_ONE_URL); @@ -410,30 +337,6 @@ describe('Switch Ethereum Chain for two dapps', function () { async ({ driver }) => { await unlockWallet(driver); - // Open settings menu button - const accountOptionsMenuSelector = - '[data-testid="account-options-menu-button"]'; - await driver.waitForSelector(accountOptionsMenuSelector); - await driver.clickElement(accountOptionsMenuSelector); - - // Click settings from dropdown menu - const globalMenuSettingsSelector = - '[data-testid="global-menu-settings"]'; - await driver.waitForSelector(globalMenuSettingsSelector); - await driver.clickElement(globalMenuSettingsSelector); - - // Click Experimental tab - const experimentalTabRawLocator = { - text: 'Experimental', - tag: 'div', - }; - await driver.clickElement(experimentalTabRawLocator); - - // Toggle off request queue setting (on by default now) - await driver.clickElement( - '[data-testid="experimental-setting-toggle-request-queue"]', - ); - // open two dapps const dappTwo = await openDapp(driver, undefined, DAPP_ONE_URL); const dappOne = await openDapp(driver, undefined, DAPP_URL); @@ -544,30 +447,6 @@ describe('Switch Ethereum Chain for two dapps', function () { async ({ driver }) => { await unlockWallet(driver); - // Open settings menu button - const accountOptionsMenuSelector = - '[data-testid="account-options-menu-button"]'; - await driver.waitForSelector(accountOptionsMenuSelector); - await driver.clickElement(accountOptionsMenuSelector); - - // Click settings from dropdown menu - const globalMenuSettingsSelector = - '[data-testid="global-menu-settings"]'; - await driver.waitForSelector(globalMenuSettingsSelector); - await driver.clickElement(globalMenuSettingsSelector); - - // Click Experimental tab - const experimentalTabRawLocator = { - text: 'Experimental', - tag: 'div', - }; - await driver.clickElement(experimentalTabRawLocator); - - // Toggle off request queue setting (on by default now) - await driver.clickElement( - '[data-testid="experimental-setting-toggle-request-queue"]', - ); - // open two dapps const dappTwo = await openDapp(driver, undefined, DAPP_ONE_URL); const dappOne = await openDapp(driver, undefined, DAPP_URL); diff --git a/test/e2e/page-objects/pages/settings/experimental-settings.ts b/test/e2e/page-objects/pages/settings/experimental-settings.ts index 69df0525093d..f3755db72947 100644 --- a/test/e2e/page-objects/pages/settings/experimental-settings.ts +++ b/test/e2e/page-objects/pages/settings/experimental-settings.ts @@ -19,9 +19,6 @@ class ExperimentalSettings { private readonly redesignedSignatureToggle = '[data-testid="toggle-redesigned-confirmations-container"]'; - private readonly requestQueueToggle = - '[data-testid="experimental-setting-toggle-request-queue"] label'; - private readonly watchAccountToggleState = '[data-testid="watch-account-toggle"]'; @@ -73,11 +70,6 @@ class ExperimentalSettings { await this.driver.clickElement(this.redesignedSignatureToggle); } - async toggleRequestQueue(): Promise { - console.log('Toggle Request Queue on experimental setting page'); - await this.driver.clickElement(this.requestQueueToggle); - } - async toggleWatchAccount(): Promise { console.log('Toggle Watch Account on experimental setting page'); await this.driver.clickElement(this.watchAccountToggle); diff --git a/test/e2e/restore/MetaMaskUserData.json b/test/e2e/restore/MetaMaskUserData.json index 7a687ec254c0..6f08305db357 100644 --- a/test/e2e/restore/MetaMaskUserData.json +++ b/test/e2e/restore/MetaMaskUserData.json @@ -40,7 +40,6 @@ }, "theme": "light", "useBlockie": false, - "useRequestQueue": true, "useNftDetection": false, "useNonceField": false, "usePhishDetect": true, diff --git a/test/e2e/tests/confirmations/alerts/queued-confirmations.spec.ts b/test/e2e/tests/confirmations/alerts/queued-confirmations.spec.ts index 8ecd7e908a30..97b6beb81096 100644 --- a/test/e2e/tests/confirmations/alerts/queued-confirmations.spec.ts +++ b/test/e2e/tests/confirmations/alerts/queued-confirmations.spec.ts @@ -41,7 +41,7 @@ describe('Queued Confirmations', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .withSelectedNetworkControllerPerDomain() .build(), dappOptions: { numberOfDapps: 2 }, @@ -86,7 +86,7 @@ describe('Queued Confirmations', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .withSelectedNetworkControllerPerDomain() .build(), dappOptions: { numberOfDapps: 2 }, @@ -141,7 +141,6 @@ describe('Queued Confirmations', function () { .withPermissionControllerConnectedToTestDapp() .withPreferencesController({ preferences: { redesignedConfirmationsEnabled: true }, - useRequestQueue: true, }) .withSelectedNetworkControllerPerDomain() .build(), @@ -193,7 +192,7 @@ describe('Queued Confirmations', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .withSelectedNetworkControllerPerDomain() .withMetaMetricsController({ metaMetricsId: 'fake-metrics-id', @@ -281,7 +280,6 @@ describe('Queued Confirmations', function () { .withPermissionControllerConnectedToTestDapp() .withPreferencesController({ preferences: { redesignedConfirmationsEnabled: true }, - useRequestQueue: true, }) .withSelectedNetworkControllerPerDomain() .withMetaMetricsController({ diff --git a/test/e2e/tests/connections/review-switch-permission-page.spec.js b/test/e2e/tests/connections/review-switch-permission-page.spec.js index 5fe3d6d19526..121c3b413833 100644 --- a/test/e2e/tests/connections/review-switch-permission-page.spec.js +++ b/test/e2e/tests/connections/review-switch-permission-page.spec.js @@ -21,7 +21,7 @@ describe('Permissions Page when Dapp Switch to an enabled and non permissioned n dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .withSelectedNetworkControllerPerDomain() .build(), ganacheOptions: { diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json index c4d176348946..cab47452ea3f 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json @@ -211,7 +211,6 @@ "useNftDetection": false, "use4ByteResolution": true, "useCurrencyRateCheck": true, - "useRequestQueue": true, "openSeaEnabled": false, "securityAlertsEnabled": "boolean", "watchEthereumAccountEnabled": "boolean", diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json index 76cc18653595..e2b805831ea4 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json @@ -121,7 +121,6 @@ "useTokenDetection": true, "useNftDetection": false, "useCurrencyRateCheck": true, - "useRequestQueue": true, "openSeaEnabled": false, "securityAlertsEnabled": "boolean", "watchEthereumAccountEnabled": "boolean", diff --git a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json index c443b0fe84ec..c85bf92f4334 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json @@ -125,7 +125,6 @@ "useTokenDetection": false, "useCurrencyRateCheck": true, "useMultiAccountBalanceChecker": true, - "useRequestQueue": true, "isMultiAccountBalancesEnabled": "boolean", "showIncomingTransactions": "object" }, diff --git a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json index 012ccabe6f2f..f86a5278e054 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json @@ -125,7 +125,6 @@ "useTokenDetection": false, "useCurrencyRateCheck": true, "useMultiAccountBalanceChecker": true, - "useRequestQueue": true, "isMultiAccountBalancesEnabled": "boolean", "showIncomingTransactions": "object" }, diff --git a/test/e2e/tests/multichain/all-permissions-page.spec.ts b/test/e2e/tests/multichain/all-permissions-page.spec.ts index eca4052fdbf7..4c481520ce81 100644 --- a/test/e2e/tests/multichain/all-permissions-page.spec.ts +++ b/test/e2e/tests/multichain/all-permissions-page.spec.ts @@ -62,7 +62,6 @@ describe('Permissions Page', function () { const experimentalSettings = new ExperimentalSettings(driver); await experimentalSettings.check_pageIsLoaded(); - await experimentalSettings.toggleRequestQueue(); await settingsPage.closeSettingsPage(); // go to homepage and check site permissions diff --git a/test/e2e/tests/request-queuing/batch-txs-per-dapp-diff-network.spec.js b/test/e2e/tests/request-queuing/batch-txs-per-dapp-diff-network.spec.js index 958c1351d8b3..e23db3855609 100644 --- a/test/e2e/tests/request-queuing/batch-txs-per-dapp-diff-network.spec.js +++ b/test/e2e/tests/request-queuing/batch-txs-per-dapp-diff-network.spec.js @@ -22,7 +22,6 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks', fun dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), dappOptions: { numberOfDapps: 2 }, ganacheOptions: { @@ -145,7 +144,6 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks', fun dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), dappOptions: { numberOfDapps: 2 }, ganacheOptions: { diff --git a/test/e2e/tests/request-queuing/batch-txs-per-dapp-extra-tx.spec.js b/test/e2e/tests/request-queuing/batch-txs-per-dapp-extra-tx.spec.js index 066acacab23a..898fe4cf3d90 100644 --- a/test/e2e/tests/request-queuing/batch-txs-per-dapp-extra-tx.spec.js +++ b/test/e2e/tests/request-queuing/batch-txs-per-dapp-extra-tx.spec.js @@ -22,7 +22,6 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks', fun dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), dappOptions: { numberOfDapps: 2 }, ganacheOptions: { @@ -185,7 +184,6 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks', fun dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), dappOptions: { numberOfDapps: 2 }, ganacheOptions: { diff --git a/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js b/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js index d3241c95c9d5..c85631d32800 100644 --- a/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js +++ b/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js @@ -23,7 +23,6 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), dappOptions: { numberOfDapps: 3 }, ganacheOptions: { @@ -180,7 +179,6 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), dappOptions: { numberOfDapps: 3 }, ganacheOptions: { diff --git a/test/e2e/tests/request-queuing/chainid-check.spec.js b/test/e2e/tests/request-queuing/chainid-check.spec.js index 1579a8ae5aa4..95ed3ca6bd8c 100644 --- a/test/e2e/tests/request-queuing/chainid-check.spec.js +++ b/test/e2e/tests/request-queuing/chainid-check.spec.js @@ -13,304 +13,145 @@ const { const { PAGES } = require('../../webdriver/driver'); describe('Request Queueing chainId proxy sync', function () { - describe('request queue is on', function () { - it('should preserve per dapp network selections after connecting and switching without refresh calls @no-mmi', async function () { - const port = 8546; - const chainId = 1338; - await withFixtures( - { - dapp: true, - fixtures: new FixtureBuilder() - .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() - .withSelectedNetworkControllerPerDomain() - .build(), - ganacheOptions: { - ...defaultGanacheOptions, - concurrent: [ - { - port, - chainId, - ganacheOptions2: defaultGanacheOptions, - }, - ], - }, - title: this.test.fullTitle(), + it('should preserve per dapp network selections after connecting and switching without refresh calls @no-mmi', async function () { + const port = 8546; + const chainId = 1338; + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withNetworkControllerDoubleGanache() + + .withSelectedNetworkControllerPerDomain() + .build(), + ganacheOptions: { + ...defaultGanacheOptions, + concurrent: [ + { + port, + chainId, + ganacheOptions2: defaultGanacheOptions, + }, + ], }, - async ({ driver }) => { - await unlockWallet(driver); + title: this.test.fullTitle(), + }, + async ({ driver }) => { + await unlockWallet(driver); - // Navigate to extension home screen - await driver.navigate(PAGES.HOME); + // Navigate to extension home screen + await driver.navigate(PAGES.HOME); - // Open Dapp One - await openDapp(driver, undefined, DAPP_URL); + // Open Dapp One + await openDapp(driver, undefined, DAPP_URL); - await driver.delay(regularDelayMs); + await driver.delay(regularDelayMs); - const chainIdRequest = JSON.stringify({ - method: 'eth_chainId', - }); + const chainIdRequest = JSON.stringify({ + method: 'eth_chainId', + }); - const chainIdBeforeConnect = await driver.executeScript( - `return window.ethereum.request(${chainIdRequest})`, - ); - - assert.equal(chainIdBeforeConnect, '0x539'); // 1337 - - await driver.switchToWindowWithTitle( - WINDOW_TITLES.ExtensionInFullScreenView, - ); - - // Network Selector - await driver.clickElement('[data-testid="network-display"]'); - - // Switch to second network - await driver.clickElement({ - text: 'Ethereum Mainnet', - css: 'p', - }); - - await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); + const chainIdBeforeConnect = await driver.executeScript( + `return window.ethereum.request(${chainIdRequest})`, + ); - const chainIdBeforeConnectAfterManualSwitch = - await driver.executeScript( - `return window.ethereum.request(${chainIdRequest})`, - ); + assert.equal(chainIdBeforeConnect, '0x539'); // 1337 - // before connecting the chainId should change with the wallet - assert.equal(chainIdBeforeConnectAfterManualSwitch, '0x1'); + await driver.switchToWindowWithTitle( + WINDOW_TITLES.ExtensionInFullScreenView, + ); - // Connect to dapp - await driver.findClickableElement({ text: 'Connect', tag: 'button' }); - await driver.clickElement('#connectButton'); - - await driver.delay(regularDelayMs); - - await switchToNotificationWindow(driver); - - await driver.clickElement({ - text: 'Connect', - tag: 'button', - }); - - await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); - - const chainIdAfterConnect = await driver.executeScript( - `return window.ethereum.request(${chainIdRequest})`, - ); + // Network Selector + await driver.clickElement('[data-testid="network-display"]'); - // should still be on the same chainId as the wallet after connecting - assert.equal(chainIdAfterConnect, '0x1'); + // Switch to second network + await driver.clickElement({ + text: 'Ethereum Mainnet', + css: 'p', + }); - const switchEthereumChainRequest = JSON.stringify({ - jsonrpc: '2.0', - method: 'wallet_switchEthereumChain', - params: [{ chainId: '0x539' }], - }); + await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); + const chainIdBeforeConnectAfterManualSwitch = await driver.executeScript( - `window.ethereum.request(${switchEthereumChainRequest})`, - ); - - await switchToNotificationWindow(driver); - await driver.findClickableElements({ - text: 'Confirm', - tag: 'button', - }); - - await driver.clickElement({ text: 'Confirm', tag: 'button' }); - - await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); - - const chainIdAfterDappSwitch = await driver.executeScript( - `return window.ethereum.request(${chainIdRequest})`, - ); - - // should be on the new chainId that was requested - assert.equal(chainIdAfterDappSwitch, '0x539'); // 1337 - - await driver.switchToWindowWithTitle( - WINDOW_TITLES.ExtensionInFullScreenView, - ); - - // Network Selector - await driver.clickElement('[data-testid="network-display"]'); - - // Switch network - await driver.clickElement({ - text: 'Localhost 8546', - css: 'p', - }); - - await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); - - const chainIdAfterManualSwitch = await driver.executeScript( - `return window.ethereum.request(${chainIdRequest})`, - ); - // after connecting the dapp should now have its own selected network - // independent from the globally selected and therefore should not have changed when - // the globally selected network was manually changed via the wallet UI - assert.equal(chainIdAfterManualSwitch, '0x539'); // 1337 - }, - ); - }); - }); - - describe('request queue is off', function () { - it('should always follow the globally selected network after connecting and switching without refresh calls @no-mmi', async function () { - const port = 8546; - const chainId = 1338; - await withFixtures( - { - dapp: true, - fixtures: new FixtureBuilder() - .withNetworkControllerDoubleGanache() - .build(), - ganacheOptions: { - ...defaultGanacheOptions, - concurrent: [ - { - port, - chainId, - ganacheOptions2: defaultGanacheOptions, - }, - ], - }, - title: this.test.fullTitle(), - }, - async ({ driver }) => { - await unlockWallet(driver); - - await driver.clickElement( - '[data-testid="account-options-menu-button"]', - ); - await driver.clickElement({ text: 'Settings', tag: 'div' }); - await driver.clickElement({ text: 'Experimental', tag: 'div' }); - await driver.clickElement( - '[data-testid="experimental-setting-toggle-request-queue"]', - ); - - // Navigate to extension home screen - await driver.navigate(PAGES.HOME); - - // Open Dapp One - await openDapp(driver, undefined, DAPP_URL); - - await driver.delay(regularDelayMs); - - const chainIdRequest = JSON.stringify({ - method: 'eth_chainId', - }); - - const chainIdBeforeConnect = await driver.executeScript( `return window.ethereum.request(${chainIdRequest})`, ); - assert.equal(chainIdBeforeConnect, '0x539'); // 1337 - - await driver.switchToWindowWithTitle( - WINDOW_TITLES.ExtensionInFullScreenView, - ); - - // Network Selector - await driver.clickElement('[data-testid="network-display"]'); + // before connecting the chainId should change with the wallet + assert.equal(chainIdBeforeConnectAfterManualSwitch, '0x1'); - // Switch to second network - await driver.clickElement({ - text: 'Ethereum Mainnet', - css: 'p', - }); + // Connect to dapp + await driver.findClickableElement({ text: 'Connect', tag: 'button' }); + await driver.clickElement('#connectButton'); - await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); + await driver.delay(regularDelayMs); - const chainIdBeforeConnectAfterManualSwitch = - await driver.executeScript( - `return window.ethereum.request(${chainIdRequest})`, - ); + await switchToNotificationWindow(driver); - // before connecting the chainId should change with the wallet - assert.equal(chainIdBeforeConnectAfterManualSwitch, '0x1'); + await driver.clickElement({ + text: 'Connect', + tag: 'button', + }); - // Connect to dapp - await driver.clickElement({ text: 'Connect', tag: 'button' }); + await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); - await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); + const chainIdAfterConnect = await driver.executeScript( + `return window.ethereum.request(${chainIdRequest})`, + ); - await driver.clickElementAndWaitForWindowToClose({ - text: 'Connect', - tag: 'button', - }); + // should still be on the same chainId as the wallet after connecting + assert.equal(chainIdAfterConnect, '0x1'); - await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); + const switchEthereumChainRequest = JSON.stringify({ + jsonrpc: '2.0', + method: 'wallet_switchEthereumChain', + params: [{ chainId: '0x539' }], + }); - const chainIdAfterConnect = await driver.executeScript( - `return window.ethereum.request(${chainIdRequest})`, - ); + await driver.executeScript( + `window.ethereum.request(${switchEthereumChainRequest})`, + ); - // should still be on the same chainId as the wallet after connecting - assert.equal(chainIdAfterConnect, '0x1'); - await driver.waitForSelector({ - css: '[id="chainId"]', - text: '0x1', - }); + await switchToNotificationWindow(driver); + await driver.findClickableElements({ + text: 'Confirm', + tag: 'button', + }); - const switchEthereumChainRequest = JSON.stringify({ - jsonrpc: '2.0', - method: 'wallet_switchEthereumChain', - params: [{ chainId: '0x539' }], - }); + await driver.clickElement({ text: 'Confirm', tag: 'button' }); - await driver.executeScript( - `window.ethereum.request(${switchEthereumChainRequest})`, - ); + await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); - await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); + const chainIdAfterDappSwitch = await driver.executeScript( + `return window.ethereum.request(${chainIdRequest})`, + ); - await driver.clickElementAndWaitForWindowToClose({ - text: 'Confirm', - tag: 'button', - }); + // should be on the new chainId that was requested + assert.equal(chainIdAfterDappSwitch, '0x539'); // 1337 - await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); + await driver.switchToWindowWithTitle( + WINDOW_TITLES.ExtensionInFullScreenView, + ); - const chainIdAfterDappSwitch = await driver.executeScript( - `return window.ethereum.request(${chainIdRequest})`, - ); + // Network Selector + await driver.clickElement('[data-testid="network-display"]'); - // should be on the new chainId that was requested - assert.equal(chainIdAfterDappSwitch, '0x539'); // 1337 + // Switch network + await driver.clickElement({ + text: 'Localhost 8546', + css: 'p', + }); - await driver.waitForSelector({ - css: '[id="chainId"]', - text: '0x539', - }); - await driver.switchToWindowWithTitle( - WINDOW_TITLES.ExtensionInFullScreenView, - ); + await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); - // TODO: check that the wallet network has changed too - - // Network Selector - await driver.clickElement('[data-testid="network-display"]'); - - // Switch network - await driver.clickElement({ - text: 'Ethereum Mainnet', - css: 'p', - }); - - await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); - - const chainIdAfterManualSwitch = await driver.executeScript( - `return window.ethereum.request(${chainIdRequest})`, - ); - // after connecting the dapp should still be following the - // globally selected network and therefore should have changed when - // the globally selected network was manually changed via the wallet UI - assert.equal(chainIdAfterManualSwitch, '0x1'); - }, - ); - }); + const chainIdAfterManualSwitch = await driver.executeScript( + `return window.ethereum.request(${chainIdRequest})`, + ); + // after connecting the dapp should now have its own selected network + // independent from the globally selected and therefore should not have changed when + // the globally selected network was manually changed via the wallet UI + assert.equal(chainIdAfterManualSwitch, '0x539'); // 1337 + }, + ); }); }); diff --git a/test/e2e/tests/request-queuing/dapp1-send-dapp2-signTypedData.spec.js b/test/e2e/tests/request-queuing/dapp1-send-dapp2-signTypedData.spec.js index 28b19efbeb04..9d7fb364f21c 100644 --- a/test/e2e/tests/request-queuing/dapp1-send-dapp2-signTypedData.spec.js +++ b/test/e2e/tests/request-queuing/dapp1-send-dapp2-signTypedData.spec.js @@ -24,7 +24,6 @@ describe('Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .withSelectedNetworkControllerPerDomain() .build(), dappOptions: { numberOfDapps: 2 }, @@ -173,7 +172,6 @@ describe('Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .withSelectedNetworkControllerPerDomain() .build(), dappOptions: { numberOfDapps: 2 }, diff --git a/test/e2e/tests/request-queuing/dapp1-subscribe-network-switch.spec.ts b/test/e2e/tests/request-queuing/dapp1-subscribe-network-switch.spec.ts index ec607fc34936..6b87a7142fbe 100644 --- a/test/e2e/tests/request-queuing/dapp1-subscribe-network-switch.spec.ts +++ b/test/e2e/tests/request-queuing/dapp1-subscribe-network-switch.spec.ts @@ -18,7 +18,6 @@ describe('Request Queueing', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), ganacheOptions: { ...defaultGanacheOptions, diff --git a/test/e2e/tests/request-queuing/dapp1-switch-dapp2-eth-request-accounts.spec.js b/test/e2e/tests/request-queuing/dapp1-switch-dapp2-eth-request-accounts.spec.js index 67efbfe6fee9..bd61fe8d00b9 100644 --- a/test/e2e/tests/request-queuing/dapp1-switch-dapp2-eth-request-accounts.spec.js +++ b/test/e2e/tests/request-queuing/dapp1-switch-dapp2-eth-request-accounts.spec.js @@ -22,7 +22,6 @@ describe('Request Queuing Dapp 1 Send Tx -> Dapp 2 Request Accounts Tx', functio dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .withPermissionControllerConnectedToTestDapp() .build(), dappOptions: { numberOfDapps: 2 }, @@ -116,7 +115,6 @@ describe('Request Queuing Dapp 1 Send Tx -> Dapp 2 Request Accounts Tx', functio dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .withPermissionControllerConnectedToTestDapp() .build(), dappOptions: { numberOfDapps: 2 }, @@ -207,7 +205,6 @@ describe('Request Queuing Dapp 1 Send Tx -> Dapp 2 Request Accounts Tx', functio dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .withPermissionControllerConnectedToTwoTestDapps() .build(), dappOptions: { numberOfDapps: 2 }, diff --git a/test/e2e/tests/request-queuing/dapp1-switch-dapp2-send.spec.js b/test/e2e/tests/request-queuing/dapp1-switch-dapp2-send.spec.js index 24c09ee18d09..e2b5bb0ca9b1 100644 --- a/test/e2e/tests/request-queuing/dapp1-switch-dapp2-send.spec.js +++ b/test/e2e/tests/request-queuing/dapp1-switch-dapp2-send.spec.js @@ -20,7 +20,6 @@ describe('Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .withSelectedNetworkControllerPerDomain() .build(), dappOptions: { numberOfDapps: 2 }, @@ -177,7 +176,6 @@ describe('Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .withSelectedNetworkControllerPerDomain() .build(), dappOptions: { numberOfDapps: 2 }, @@ -328,7 +326,6 @@ describe('Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .withSelectedNetworkControllerPerDomain() .build(), dappOptions: { numberOfDapps: 2 }, @@ -483,7 +480,6 @@ describe('Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .withSelectedNetworkControllerPerDomain() .build(), dappOptions: { numberOfDapps: 2 }, diff --git a/test/e2e/tests/request-queuing/enable-queuing.spec.js b/test/e2e/tests/request-queuing/enable-queuing.spec.js deleted file mode 100644 index ccc5f7cec2c7..000000000000 --- a/test/e2e/tests/request-queuing/enable-queuing.spec.js +++ /dev/null @@ -1,47 +0,0 @@ -const { - withFixtures, - defaultGanacheOptions, - unlockWallet, -} = require('../../helpers'); -const FixtureBuilder = require('../../fixture-builder'); - -describe('Toggle Request Queuing Setting', function () { - it('enables the request queuing experimental setting', async function () { - await withFixtures( - { - dapp: true, - fixtures: new FixtureBuilder().build(), - ganacheOptions: defaultGanacheOptions, - failOnConsoleError: false, - title: this.test.fullTitle(), - }, - async ({ driver }) => { - await unlockWallet(driver); - - // Open account menu button - const accountOptionsMenuSelector = - '[data-testid="account-options-menu-button"]'; - await driver.waitForSelector(accountOptionsMenuSelector); - await driver.clickElement(accountOptionsMenuSelector); - - // Click settings from dropdown menu - const globalMenuSettingsSelector = - '[data-testid="global-menu-settings"]'; - await driver.waitForSelector(globalMenuSettingsSelector); - await driver.clickElement(globalMenuSettingsSelector); - - // Click Experimental tab - const securityAndPrivacyTabRawLocator = { - text: 'Experimental', - tag: 'div', - }; - await driver.clickElement(securityAndPrivacyTabRawLocator); - - // Toggle request queue setting - await driver.clickElement( - '[data-testid="experimental-setting-toggle-request-queue"]', - ); - }, - ); - }); -}); diff --git a/test/e2e/tests/request-queuing/multi-dapp-sendTx-revokePermission.spec.js b/test/e2e/tests/request-queuing/multi-dapp-sendTx-revokePermission.spec.js index 3a413f147e06..66e1b8963731 100644 --- a/test/e2e/tests/request-queuing/multi-dapp-sendTx-revokePermission.spec.js +++ b/test/e2e/tests/request-queuing/multi-dapp-sendTx-revokePermission.spec.js @@ -20,7 +20,6 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks revok dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), dappOptions: { numberOfDapps: 2 }, ganacheOptions: { @@ -141,7 +140,6 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks revok dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), dappOptions: { numberOfDapps: 2 }, ganacheOptions: { diff --git a/test/e2e/tests/request-queuing/multiple-networks-dapps-txs.spec.js b/test/e2e/tests/request-queuing/multiple-networks-dapps-txs.spec.js index f831ff1ff38d..fec528a84c60 100644 --- a/test/e2e/tests/request-queuing/multiple-networks-dapps-txs.spec.js +++ b/test/e2e/tests/request-queuing/multiple-networks-dapps-txs.spec.js @@ -21,7 +21,6 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks.', fu dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .withSelectedNetworkControllerPerDomain() .build(), dappOptions: { numberOfDapps: 2 }, @@ -159,7 +158,6 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks.', fu dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .withSelectedNetworkControllerPerDomain() .build(), dappOptions: { numberOfDapps: 2 }, diff --git a/test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.ts b/test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.ts index 9f5454404806..620735761c69 100644 --- a/test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.ts +++ b/test/e2e/tests/request-queuing/sendTx-revokePermissions.spec.ts @@ -21,7 +21,6 @@ describe('Request Queuing', function () { dapp: true, fixtures: new FixtureBuilder() .withPermissionControllerConnectedToTestDapp() - .withPreferencesControllerUseRequestQueueEnabled() .withSelectedNetworkControllerPerDomain() .build(), ganacheOptions: { diff --git a/test/e2e/tests/request-queuing/sendTx-switchChain-sendTx.spec.js b/test/e2e/tests/request-queuing/sendTx-switchChain-sendTx.spec.js index 5b0c4dca2f07..13d34ecbbb66 100644 --- a/test/e2e/tests/request-queuing/sendTx-switchChain-sendTx.spec.js +++ b/test/e2e/tests/request-queuing/sendTx-switchChain-sendTx.spec.js @@ -21,7 +21,7 @@ describe('Request Queuing Send Tx -> SwitchChain -> SendTx', function () { fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() .withPermissionControllerConnectedToTestDapp() - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, diff --git a/test/e2e/tests/request-queuing/switch-network.spec.js b/test/e2e/tests/request-queuing/switch-network.spec.js index 913bdf459a26..d0a72be430ca 100644 --- a/test/e2e/tests/request-queuing/switch-network.spec.js +++ b/test/e2e/tests/request-queuing/switch-network.spec.js @@ -22,7 +22,6 @@ describe('Request Queuing Switch Network on Dapp Send Tx while on different netw fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() .withPermissionControllerConnectedToTestDapp() - .withPreferencesControllerUseRequestQueueEnabled() .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -109,7 +108,6 @@ describe('Request Queuing Switch Network on Dapp Send Tx while on different netw fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() .withPermissionControllerConnectedToTestDapp() - .withPreferencesControllerUseRequestQueueEnabled() .build(), ganacheOptions: { ...defaultGanacheOptions, diff --git a/test/e2e/tests/request-queuing/switchChain-sendTx.spec.js b/test/e2e/tests/request-queuing/switchChain-sendTx.spec.js index df33600413e1..dd34215fe007 100644 --- a/test/e2e/tests/request-queuing/switchChain-sendTx.spec.js +++ b/test/e2e/tests/request-queuing/switchChain-sendTx.spec.js @@ -16,7 +16,7 @@ describe('Request Queuing SwitchChain -> SendTx', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, diff --git a/test/e2e/tests/request-queuing/switchChain-watchAsset.spec.js b/test/e2e/tests/request-queuing/switchChain-watchAsset.spec.js index 5767bd26def5..080d9ef38903 100644 --- a/test/e2e/tests/request-queuing/switchChain-watchAsset.spec.js +++ b/test/e2e/tests/request-queuing/switchChain-watchAsset.spec.js @@ -20,7 +20,7 @@ describe('Request Queue SwitchChain -> WatchAsset', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, diff --git a/test/e2e/tests/request-queuing/ui.spec.js b/test/e2e/tests/request-queuing/ui.spec.js index 707c252396b7..dd8fa26122bb 100644 --- a/test/e2e/tests/request-queuing/ui.spec.js +++ b/test/e2e/tests/request-queuing/ui.spec.js @@ -239,7 +239,6 @@ describe('Request-queue UI changes', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -303,7 +302,6 @@ describe('Request-queue UI changes', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -429,7 +427,6 @@ describe('Request-queue UI changes', function () { .withPreferencesController({ preferences: { showTestNetworks: true }, }) - .withPreferencesControllerUseRequestQueueEnabled() .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -505,7 +502,6 @@ describe('Request-queue UI changes', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -578,7 +574,7 @@ describe('Request-queue UI changes', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -650,7 +646,7 @@ describe('Request-queue UI changes', function () { driverOptions: { timeOut: 30000 }, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -718,7 +714,6 @@ describe('Request-queue UI changes', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -780,7 +775,7 @@ describe('Request-queue UI changes', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerTripleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -904,7 +899,7 @@ describe('Request-queue UI changes', function () { .withPreferencesController({ preferences: { showTestNetworks: true }, }) - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -973,9 +968,7 @@ describe('Request-queue UI changes', function () { await withFixtures( { dapp: true, - fixtures: new FixtureBuilder() - .withPreferencesControllerUseRequestQueueEnabled() - .build(), + fixtures: new FixtureBuilder().build(), ganacheOptions: defaultGanacheOptions, title: this.test.fullTitle(), driverOptions: { constrainWindowSize: true }, @@ -1019,7 +1012,7 @@ describe('Request-queue UI changes', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -1071,7 +1064,7 @@ describe('Request-queue UI changes', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -1140,7 +1133,7 @@ describe('Request-queue UI changes', function () { dapp: true, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, @@ -1206,7 +1199,7 @@ describe('Request-queue UI changes', function () { driverOptions: { timeOut: 30000 }, fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, diff --git a/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js b/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js index 64ac781a20e0..c60d1d0a9278 100644 --- a/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js +++ b/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js @@ -22,7 +22,7 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () { fixtures: new FixtureBuilder() .withNetworkControllerDoubleGanache() .withPermissionControllerConnectedToTestDapp() - .withPreferencesControllerUseRequestQueueEnabled() + .build(), ganacheOptions: { ...defaultGanacheOptions, diff --git a/test/integration/data/integration-init-state.json b/test/integration/data/integration-init-state.json index 3d3580dc12ca..cbf065ea2aba 100644 --- a/test/integration/data/integration-init-state.json +++ b/test/integration/data/integration-init-state.json @@ -2049,7 +2049,6 @@ "useNftDetection": false, "useNonceField": false, "usePhishDetect": true, - "useRequestQueue": false, "useSafeChainsListValidation": true, "useTokenDetection": false, "useTransactionSimulations": true, diff --git a/test/integration/data/onboarding-completion-route.json b/test/integration/data/onboarding-completion-route.json index 9d3df88dbeae..479c9041b3a2 100644 --- a/test/integration/data/onboarding-completion-route.json +++ b/test/integration/data/onboarding-completion-route.json @@ -462,7 +462,6 @@ "useNftDetection": false, "useNonceField": false, "usePhishDetect": true, - "useRequestQueue": true, "useSafeChainsListValidation": true, "useTokenDetection": true, "useTransactionSimulations": true, diff --git a/ui/components/app/assets/nfts/nfts-items/nfts-items.stories.tsx b/ui/components/app/assets/nfts/nfts-items/nfts-items.stories.tsx index f13af8c13ea4..7a7f4c8ccaa9 100644 --- a/ui/components/app/assets/nfts/nfts-items/nfts-items.stories.tsx +++ b/ui/components/app/assets/nfts/nfts-items/nfts-items.stories.tsx @@ -77,7 +77,6 @@ const createMockState = () => ({ nftContracts: [], nfts: [], ...mockNetworkState({ chainId: CHAIN_IDS.MAINNET }), - useRequestQueue: true, }, appState: { isLoading: false, diff --git a/ui/components/multichain/network-list-menu/network-list-menu.test.js b/ui/components/multichain/network-list-menu/network-list-menu.test.js index ec4539aa55da..90ed2fca6809 100644 --- a/ui/components/multichain/network-list-menu/network-list-menu.test.js +++ b/ui/components/multichain/network-list-menu/network-list-menu.test.js @@ -143,7 +143,6 @@ const render = ({ [CHAIN_IDS.LINEA_MAINNET]: true, }, }, - useRequestQueue: true, domains: { ...(selectedTabOriginInDomainsState ? { [origin]: selectedNetworkClientId } diff --git a/ui/components/multichain/network-list-menu/network-list-menu.tsx b/ui/components/multichain/network-list-menu/network-list-menu.tsx index 481ddfe5142f..491ae289b19c 100644 --- a/ui/components/multichain/network-list-menu/network-list-menu.tsx +++ b/ui/components/multichain/network-list-menu/network-list-menu.tsx @@ -46,7 +46,6 @@ import { getOnboardedInThisUISession, getShowNetworkBanner, getOriginOfCurrentTab, - getUseRequestQueue, getEditedNetwork, getOrderedNetworksList, getIsAddingNewNetwork, @@ -120,7 +119,6 @@ export const NetworkListMenu = ({ onClose }: { onClose: () => void }) => { const showTestNetworks = useSelector(getShowTestNetworks); const currentChainId = useSelector(getCurrentChainId); const selectedTabOrigin = useSelector(getOriginOfCurrentTab); - const useRequestQueue = useSelector(getUseRequestQueue); const isUnlocked = useSelector(getIsUnlocked); const domains = useSelector(getAllDomains); const orderedNetworksList = useSelector(getOrderedNetworksList); @@ -309,11 +307,7 @@ export const NetworkListMenu = ({ onClose }: { onClose: () => void }) => { // If presently on a dapp, communicate a change to // the dapp via silent switchEthereumChain that the // network has changed due to user action - if ( - useRequestQueue && - selectedTabOrigin && - domains[selectedTabOrigin] - ) { + if (selectedTabOrigin && domains[selectedTabOrigin]) { setNetworkClientIdForDomain(selectedTabOrigin, networkClientId); } diff --git a/ui/index.js b/ui/index.js index a63b2acc86fa..54d952eb85f4 100644 --- a/ui/index.js +++ b/ui/index.js @@ -29,7 +29,6 @@ import { getUnapprovedTransactions, getNetworkToAutomaticallySwitchTo, getSwitchedNetworkDetails, - getUseRequestQueue, } from './selectors'; import { ALERT_STATE } from './ducks/alerts'; import { @@ -243,10 +242,7 @@ async function runInitialActions(store) { // Register this window as the current popup // and set in background state - if ( - getUseRequestQueue(state) && - getEnvironmentType() === ENVIRONMENT_TYPE_POPUP - ) { + if (getEnvironmentType() === ENVIRONMENT_TYPE_POPUP) { const thisPopupId = Date.now(); global.metamask.id = thisPopupId; await store.dispatch(actions.setCurrentExtensionPopupId(thisPopupId)); diff --git a/ui/pages/routes/routes.component.js b/ui/pages/routes/routes.component.js index 6212ed4258f3..74f15a5f99c6 100644 --- a/ui/pages/routes/routes.component.js +++ b/ui/pages/routes/routes.component.js @@ -193,7 +193,6 @@ export default class Routes extends Component { automaticallySwitchNetwork: PropTypes.func.isRequired, totalUnapprovedConfirmationCount: PropTypes.number.isRequired, currentExtensionPopupId: PropTypes.number, - useRequestQueue: PropTypes.bool, clearEditedNetwork: PropTypes.func.isRequired, oldestPendingApproval: PropTypes.object.isRequired, pendingApprovals: PropTypes.arrayOf(PropTypes.object).isRequired, @@ -217,7 +216,6 @@ export default class Routes extends Component { activeTabOrigin, totalUnapprovedConfirmationCount, isUnlocked, - useRequestQueue, currentExtensionPopupId, } = this.props; if (theme !== prevProps.theme) { @@ -243,7 +241,6 @@ export default class Routes extends Component { // Terminate the popup when another popup is opened // if the user is using RPC queueing if ( - useRequestQueue && currentExtensionPopupId !== undefined && global.metamask.id !== undefined && currentExtensionPopupId !== global.metamask.id diff --git a/ui/pages/routes/routes.container.js b/ui/pages/routes/routes.container.js index 0342fc2e15d1..feb1be09408e 100644 --- a/ui/pages/routes/routes.container.js +++ b/ui/pages/routes/routes.container.js @@ -21,7 +21,6 @@ import { getSwitchedNetworkDetails, getNetworkToAutomaticallySwitchTo, getNumberOfAllUnapprovedTransactionsAndMessages, - getUseRequestQueue, getCurrentNetwork, getSelectedInternalAccount, oldestPendingConfirmationSelector, @@ -118,7 +117,6 @@ function mapStateToProps(state) { switchedNetworkNeverShowMessage: selectSwitchedNetworkNeverShowMessage(state), currentExtensionPopupId: state.metamask.currentExtensionPopupId, - useRequestQueue: getUseRequestQueue(state), oldestPendingApproval, pendingApprovals, transactionsMetadata, diff --git a/ui/pages/settings/experimental-tab/experimental-tab.component.tsx b/ui/pages/settings/experimental-tab/experimental-tab.component.tsx index 5bea76d80dbe..dd985eb469e7 100644 --- a/ui/pages/settings/experimental-tab/experimental-tab.component.tsx +++ b/ui/pages/settings/experimental-tab/experimental-tab.component.tsx @@ -48,8 +48,6 @@ type ExperimentalTabProps = { addSnapAccountEnabled: boolean; setAddSnapAccountEnabled: (value: boolean) => void; ///: END:ONLY_INCLUDE_IF - useRequestQueue: boolean; - setUseRequestQueue: (value: boolean) => void; petnamesEnabled: boolean; setPetnamesEnabled: (value: boolean) => void; featureNotificationsEnabled: boolean; @@ -237,21 +235,6 @@ export default class ExperimentalTab extends PureComponent } ///: END:ONLY_INCLUDE_IF - renderToggleRequestQueue() { - const { t } = this.context; - const { useRequestQueue, setUseRequestQueue } = this.props; - return this.renderToggleSection({ - title: t('toggleRequestQueueField'), - description: t('toggleRequestQueueDescription'), - toggleValue: useRequestQueue || false, - toggleCallback: (value) => setUseRequestQueue(!value), - toggleContainerDataTestId: 'experimental-setting-toggle-request-queue', - toggleDataTestId: 'experimental-setting-toggle-request-queue', - toggleOffLabel: t('toggleRequestQueueOff'), - toggleOnLabel: t('toggleRequestQueueOn'), - }); - } - renderNotificationsToggle() { const { t } = this.context; const { featureNotificationsEnabled, setFeatureNotificationsEnabled } = @@ -423,7 +406,6 @@ export default class ExperimentalTab extends PureComponent {this.renderToggleRedesignedSignatures()} {this.renderToggleRedesignedTransactions()} {process.env.NOTIFICATIONS ? this.renderNotificationsToggle() : null} - {this.renderToggleRequestQueue()} {/* Section: Account Management Snaps */} { ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) diff --git a/ui/pages/settings/experimental-tab/experimental-tab.container.ts b/ui/pages/settings/experimental-tab/experimental-tab.container.ts index 6de36b9de5c0..e656b035dbbf 100644 --- a/ui/pages/settings/experimental-tab/experimental-tab.container.ts +++ b/ui/pages/settings/experimental-tab/experimental-tab.container.ts @@ -7,7 +7,6 @@ import { ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) setAddSnapAccountEnabled, ///: END:ONLY_INCLUDE_IF - setUseRequestQueue, setPetnamesEnabled, setFeatureNotificationsEnabled, setRedesignedConfirmationsEnabled, @@ -26,7 +25,6 @@ import { ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) getIsAddSnapAccountEnabled, ///: END:ONLY_INCLUDE_IF - getUseRequestQueue, getPetnamesEnabled, getFeatureNotificationsEnabled, getRedesignedConfirmationsEnabled, @@ -52,7 +50,6 @@ const mapStateToProps = (state: MetaMaskReduxState) => { ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) addSnapAccountEnabled: getIsAddSnapAccountEnabled(state), ///: END:ONLY_INCLUDE_IF - useRequestQueue: getUseRequestQueue(state), petnamesEnabled, featureNotificationsEnabled, redesignedConfirmationsEnabled: getRedesignedConfirmationsEnabled(state), @@ -75,7 +72,6 @@ const mapDispatchToProps = (dispatch: MetaMaskReduxDispatch) => { setAddSnapAccountEnabled: (value: boolean) => setAddSnapAccountEnabled(value), ///: END:ONLY_INCLUDE_IF - setUseRequestQueue: (value: boolean) => setUseRequestQueue(value), setPetnamesEnabled: (value: boolean) => { return dispatch(setPetnamesEnabled(value)); }, diff --git a/ui/pages/settings/experimental-tab/experimental-tab.test.js b/ui/pages/settings/experimental-tab/experimental-tab.test.js index 08d02bf94215..cda6b95c0a5d 100644 --- a/ui/pages/settings/experimental-tab/experimental-tab.test.js +++ b/ui/pages/settings/experimental-tab/experimental-tab.test.js @@ -30,7 +30,7 @@ describe('ExperimentalTab', () => { const { getAllByRole } = render(); const toggle = getAllByRole('checkbox'); - expect(toggle).toHaveLength(9); + expect(toggle).toHaveLength(8); }); it('enables add account snap', async () => { diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index ce117af76262..43671bc77a6b 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -2200,11 +2200,9 @@ export function getNetworkToAutomaticallySwitchTo(state) { // This allows the user to be connected on one chain // for one dapp, and automatically change for another const selectedTabOrigin = getOriginOfCurrentTab(state); - const useRequestQueue = getUseRequestQueue(state); if ( getEnvironmentType() === ENVIRONMENT_TYPE_POPUP && getIsUnlocked(state) && - useRequestQueue && selectedTabOrigin && numberOfUnapprovedTx === 0 ) { @@ -2699,16 +2697,6 @@ export function getIstokenDetectionInactiveOnNonMainnetSupportedNetwork(state) { return isDynamicTokenListAvailable && !useTokenDetection && !isMainnet; } -/** - * To get the `useRequestQueue` value which determines whether we use a request queue infront of provider api calls. This will have the effect of implementing per-dapp network switching. - * - * @param {*} state - * @returns Boolean - */ -export function getUseRequestQueue(state) { - return state.metamask.useRequestQueue; -} - /** * To get the `getIsSecurityAlertsEnabled` value which determines whether security check is enabled * diff --git a/ui/selectors/selectors.test.js b/ui/selectors/selectors.test.js index fce129745561..b4afbcadcf38 100644 --- a/ui/selectors/selectors.test.js +++ b/ui/selectors/selectors.test.js @@ -305,7 +305,6 @@ describe('Selectors', () => { }, metamask: { isUnlocked: true, - useRequestQueue: true, selectedTabOrigin: SELECTED_ORIGIN, unapprovedDecryptMsgs: [], unapprovedPersonalMsgs: [], diff --git a/ui/store/actions.ts b/ui/store/actions.ts index 75de201c76b7..b6dc0bc0a7c2 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -5248,14 +5248,6 @@ export async function getSnapAccountsById(snapId: string): Promise { } ///: END:ONLY_INCLUDE_IF -export function setUseRequestQueue(val: boolean): void { - try { - submitRequestToBackground('setUseRequestQueue', [val]); - } catch (error) { - logErrorWithMessage(error); - } -} - export function setUseExternalNameSources(val: boolean): void { try { submitRequestToBackground('setUseExternalNameSources', [val]); From 2d443ffef31e826afa7bed96766fc9f084655365 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang <7315988+dawnseeker8@users.noreply.github.com> Date: Fri, 10 Jan 2025 00:41:13 +0800 Subject: [PATCH 13/23] feat: Enable Ledger clear signing feature (#28909) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR enable the clear signing feature in metamask mobile. Please refer to this feature requests for detail: https://github.com/MetaMask/accounts-planning/issues/544 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28909?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/accounts-planning/issues/544 ## **Manual testing steps** - Test the clear signing using this dapp provided by ledger team: https://clear-signing-tester.vercel.app/ - EIP712 sign message (Polygon mainnet or Ethereum mainnet). - Sign transaction (Linea testnet). - Swap in a layer 2 chain like Linea or Polygon. ## **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** - [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. --------- Co-authored-by: Sébastien Van Eyck Co-authored-by: MetaMask Bot --- .../offscreen-bridge/ledger-offscreen-bridge.ts | 14 ++++++++------ offscreen/scripts/ledger.ts | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/app/scripts/lib/offscreen-bridge/ledger-offscreen-bridge.ts b/app/scripts/lib/offscreen-bridge/ledger-offscreen-bridge.ts index 608d1ee8156b..1ff85111f54a 100644 --- a/app/scripts/lib/offscreen-bridge/ledger-offscreen-bridge.ts +++ b/app/scripts/lib/offscreen-bridge/ledger-offscreen-bridge.ts @@ -1,4 +1,8 @@ -import { LedgerBridge } from '@metamask/eth-ledger-bridge-keyring'; +import { + LedgerBridge, + LedgerSignTypedDataParams, + LedgerSignTypedDataResponse, +} from '@metamask/eth-ledger-bridge-keyring'; import { LedgerAction, OffscreenCommunicationEvents, @@ -161,11 +165,9 @@ export class LedgerOffscreenBridge }); } - deviceSignTypedData(params: { - hdPath: string; - domainSeparatorHex: string; - hashStructMessageHex: string; - }) { + deviceSignTypedData( + params: LedgerSignTypedDataParams, + ): Promise { return new Promise<{ v: number; s: string; diff --git a/offscreen/scripts/ledger.ts b/offscreen/scripts/ledger.ts index c08fdc6ac85a..4cff2f3a6748 100644 --- a/offscreen/scripts/ledger.ts +++ b/offscreen/scripts/ledger.ts @@ -96,7 +96,7 @@ function setupMessageListeners(iframe: HTMLIFrameElement) { export default async function init() { return new Promise((resolve) => { const iframe = document.createElement('iframe'); - iframe.src = 'https://metamask.github.io/eth-ledger-bridge-keyring'; + iframe.src = 'https://metamask.github.io/ledger-iframe-bridge/8.0.0/'; iframe.allow = 'hid'; iframe.onload = () => { setupMessageListeners(iframe); From 87c524e4a34749cff8c94cc43c1cee8d83ff107c Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 9 Jan 2025 14:19:56 -0330 Subject: [PATCH 14/23] ci: Migrate metamaskbot PR comment (#29373) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Migrate the `metamaskbot` PR comment from CircleCI to GitHub Actions. CircleCI is still used for artifact storage for linked artifacts. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29373?quickstart=1) ## **Related issues** Relates to #28572 These changes were extracted from #29256 ## **Manual testing steps** * Test that all links in the `metamaskbot` comment work correctly, except those that are already broken. Links already broken on `main` include: * Some build links (fixed in #29403 ): - Beta builds - Firefox test build, and Firefox flask-test build * MV3 performance stats reports (fixed in #29408 ): * mv3: Background Module Init Stats (link works, page is broken) * mv3: UI Init Stats (link works, page is broken) * mv3: Module Load Stats (link works, page is broken) * Coverage report (fixed by #29410) * mv3 bundle size stats (fixed by #29486) ## **Screenshots/Recordings** N/A ## **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. --- .circleci/config.yml | 8 --- .github/workflows/main.yml | 9 +++ .github/workflows/publish-prerelease.yml | 72 ++++++++++++++++++++ development/highlights/index.js | 2 +- development/metamaskbot-build-announce.js | 81 +++++++++++++---------- 5 files changed, 128 insertions(+), 44 deletions(-) create mode 100644 .github/workflows/publish-prerelease.yml diff --git a/.circleci/config.yml b/.circleci/config.yml index 38ccd80d7059..5243d8ef4043 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1331,14 +1331,6 @@ jobs: - store_artifacts: path: development/ts-migration-dashboard/build/final destination: ts-migration-dashboard - - run: - name: Set branch parent commit env var - command: | - echo "export PARENT_COMMIT=$(git merge-base origin/HEAD HEAD)" >> $BASH_ENV - source $BASH_ENV - - run: - name: build:announce - command: ./development/metamaskbot-build-announce.js job-publish-release: executor: node-browsers-small diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6e5a5121f336..25cb1e016868 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -76,6 +76,15 @@ jobs: name: Wait for CircleCI workflow status uses: ./.github/workflows/wait-for-circleci-workflow-status.yml + publish-prerelease: + name: Publish prerelease + if: ${{ github.event_name == 'pull_request' }} + needs: + - wait-for-circleci-workflow-status + uses: ./.github/workflows/publish-prerelease.yml + secrets: + PR_COMMENT_TOKEN: ${{ secrets.PR_COMMENT_TOKEN }} + all-jobs-completed: name: All jobs completed runs-on: ubuntu-latest diff --git a/.github/workflows/publish-prerelease.yml b/.github/workflows/publish-prerelease.yml new file mode 100644 index 000000000000..9b4e460d546a --- /dev/null +++ b/.github/workflows/publish-prerelease.yml @@ -0,0 +1,72 @@ +name: Publish prerelease + +on: + workflow_call: + secrets: + PR_COMMENT_TOKEN: + required: true + +jobs: + publish-prerelease: + name: Publish prerelease + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 # This is needed to get merge base to calculate bundle size diff + + - name: Setup environment + uses: metamask/github-tools/.github/actions/setup-environment@main + + - name: Get merge base commit hash + id: get-merge-base + env: + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: | + merge_base="$(git merge-base "origin/${BASE_REF}" HEAD)" + echo "Merge base is '${merge_base}'" + echo "MERGE_BASE=${merge_base}" >> "$GITHUB_OUTPUT" + + - name: Get CircleCI job details + id: get-circleci-job-details + env: + REPOSITORY: ${{ github.repository }} + BRANCH: ${{ github.head_ref }} + HEAD_COMMIT_HASH: ${{ github.event.pull_request.head.sha }} + run: | + pipeline_id=$(curl --silent "https://circleci.com/api/v2/project/gh/$OWNER/$REPOSITORY/pipeline?branch=$BRANCH" | jq -r ".items | map(select(.vcs.revision == \"${HEAD_COMMIT_HASH}\" )) | first | .id") + workflow_id=$(curl --silent "https://circleci.com/api/v2/pipeline/$pipeline_id/workflow" | jq -r ".items[0].id") + job_details=$(curl --silent "https://circleci.com/api/v2/workflow/$workflow_id/job" | jq -r '.items[] | select(.name == "job-publish-prerelease")') + build_num=$(echo "$job_details" | jq -r '.job_number') + + echo 'CIRCLE_BUILD_NUM='"$build_num" >> "$GITHUB_OUTPUT" + job_id=$(echo "$job_details" | jq -r '.id') + echo 'CIRCLE_WORKFLOW_JOB_ID='"$job_id" >> "$GITHUB_OUTPUT" + + echo "Getting artifacts from pipeline '${pipeline_id}', workflow '${workflow_id}', build number '${build_num}', job ID '${job_id}'" + + - name: Get CircleCI job artifacts + env: + CIRCLE_WORKFLOW_JOB_ID: ${{ steps.get-circleci-job-details.outputs.CIRCLE_WORKFLOW_JOB_ID }} + run: | + mkdir -p "test-artifacts/chrome/benchmark" + curl --silent --location "https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0/test-artifacts/chrome/benchmark/pageload.json" > "test-artifacts/chrome/benchmark/pageload.json" + + bundle_size=$(curl --silent --location "https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0/test-artifacts/chrome/bundle_size.json") + mkdir -p "test-artifacts/chrome" + echo "${bundle_size}" > "test-artifacts/chrome/bundle_size.json" + + stories=$(curl --silent --location "https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0/storybook/stories.json") + mkdir "storybook-build" + echo "${stories}" > "storybook-build/stories.json" + + - name: Publish prerelease + env: + PR_COMMENT_TOKEN: ${{ secrets.PR_COMMENT_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_COMMIT_HASH: ${{ github.event.pull_request.head.sha }} + MERGE_BASE_COMMIT_HASH: ${{ steps.get-merge-base.outputs.MERGE_BASE }} + CIRCLE_BUILD_NUM: ${{ steps.get-circleci-job-details.outputs.CIRCLE_BUILD_NUM }} + CIRCLE_WORKFLOW_JOB_ID: ${{ steps.get-circleci-job-details.outputs.CIRCLE_WORKFLOW_JOB_ID }} + run: ./development/metamaskbot-build-announce.js diff --git a/development/highlights/index.js b/development/highlights/index.js index 2616d602633e..9efd3073d031 100644 --- a/development/highlights/index.js +++ b/development/highlights/index.js @@ -9,7 +9,7 @@ async function getHighlights({ artifactBase }) { // here we assume the PR base branch ("target") is `main` in lieu of doing // a query against the github api which requires an access token // see https://discuss.circleci.com/t/how-to-retrieve-a-pull-requests-base-branch-name-github/36911 - const changedFiles = await getChangedFiles({ target: 'main' }); + const changedFiles = await getChangedFiles({ target: 'origin/main' }); console.log(`detected changed files vs main:`); for (const filename of changedFiles) { console.log(` ${filename}`); diff --git a/development/metamaskbot-build-announce.js b/development/metamaskbot-build-announce.js index dc8c2ece4be6..e463da31aaec 100755 --- a/development/metamaskbot-build-announce.js +++ b/development/metamaskbot-build-announce.js @@ -5,7 +5,6 @@ const path = require('path'); // Fetch is part of node js in future versions, thus triggering no-shadow // eslint-disable-next-line no-shadow const fetch = require('node-fetch'); -const glob = require('fast-glob'); const VERSION = require('../package.json').version; const { getHighlights } = require('./highlights'); @@ -38,29 +37,41 @@ function getPercentageChange(from, to) { return parseFloat(((to - from) / Math.abs(from)) * 100).toFixed(2); } +/** + * Check whether an artifact exists, + * + * @param {string} url - The URL of the artifact to check. + * @returns True if the artifact exists, false if it doesn't + */ +async function artifactExists(url) { + // Using a regular GET request here rather than HEAD because for some reason CircleCI always + // returns 404 for HEAD requests. + const response = await fetch(url); + return response.ok; +} + async function start() { const { - GITHUB_COMMENT_TOKEN, - CIRCLE_PULL_REQUEST, - CIRCLE_SHA1, + PR_COMMENT_TOKEN, + PR_NUMBER, + HEAD_COMMIT_HASH, + MERGE_BASE_COMMIT_HASH, CIRCLE_BUILD_NUM, CIRCLE_WORKFLOW_JOB_ID, - PARENT_COMMIT, } = process.env; - console.log('CIRCLE_PULL_REQUEST', CIRCLE_PULL_REQUEST); - console.log('CIRCLE_SHA1', CIRCLE_SHA1); + console.log('PR_NUMBER', PR_NUMBER); + console.log('HEAD_COMMIT_HASH', HEAD_COMMIT_HASH); + console.log('MERGE_BASE_COMMIT_HASH', MERGE_BASE_COMMIT_HASH); console.log('CIRCLE_BUILD_NUM', CIRCLE_BUILD_NUM); console.log('CIRCLE_WORKFLOW_JOB_ID', CIRCLE_WORKFLOW_JOB_ID); - console.log('PARENT_COMMIT', PARENT_COMMIT); - if (!CIRCLE_PULL_REQUEST) { - console.warn(`No pull request detected for commit "${CIRCLE_SHA1}"`); + if (!PR_NUMBER) { + console.warn(`No pull request detected for commit "${HEAD_COMMIT_HASH}"`); return; } - const CIRCLE_PR_NUMBER = CIRCLE_PULL_REQUEST.split('/').pop(); - const SHORT_SHA1 = CIRCLE_SHA1.slice(0, 7); + const SHORT_SHA1 = HEAD_COMMIT_HASH.slice(0, 7); const BUILD_LINK_BASE = `https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0`; // build the github comment content @@ -96,30 +107,30 @@ async function start() { // links to bundle browser builds const bundles = {}; - const fileType = '.html'; const sourceMapRoot = '/build-artifacts/source-map-explorer/'; - const bundleFiles = await glob(`.${sourceMapRoot}*${fileType}`); - - bundleFiles.forEach((bundleFile) => { - const fileName = bundleFile.split(sourceMapRoot)[1]; - const bundleName = fileName.split(fileType)[0]; - const url = `${BUILD_LINK_BASE}${sourceMapRoot}${fileName}`; - let fileRoot = bundleName; - let fileIndex = bundleName.match(/-[0-9]{1,}$/u)?.index; - - if (fileIndex) { - fileRoot = bundleName.slice(0, fileIndex); - fileIndex = bundleName.slice(fileIndex + 1, bundleName.length); - } - - const link = `${fileIndex || fileRoot}`; + const fileRoots = [ + 'background', + 'common', + 'ui', + 'content-script', + 'offscreen', + ]; - if (fileRoot in bundles) { + for (const fileRoot of fileRoots) { + bundles[fileRoot] = []; + let fileIndex = 0; + let url = `${BUILD_LINK_BASE}${sourceMapRoot}${fileRoot}-${fileIndex}.html`; + console.log(`Verifying ${url}`); + while (await artifactExists(url)) { + const link = `${fileIndex}`; bundles[fileRoot].push(link); - } else { - bundles[fileRoot] = [link]; + + fileIndex += 1; + url = `${BUILD_LINK_BASE}${sourceMapRoot}${fileRoot}-${fileIndex}.html`; + console.log(`Verifying ${url}`); } - }); + console.log(`Not found: ${url}`); + } const bundleMarkup = `
    ${Object.keys(bundles) .map((key) => `
  • ${key}: ${bundles[key].join(', ')}
    • `) @@ -295,7 +306,7 @@ async function start() { }; const devSizes = Object.keys(prSizes).reduce((sizes, part) => { - sizes[part] = devBundleSizeStats[PARENT_COMMIT][part] || 0; + sizes[part] = devBundleSizeStats[MERGE_BASE_COMMIT_HASH][part] || 0; return sizes; }, {}); @@ -346,7 +357,7 @@ async function start() { } const JSON_PAYLOAD = JSON.stringify({ body: commentBody }); - const POST_COMMENT_URI = `https://api.github.com/repos/metamask/metamask-extension/issues/${CIRCLE_PR_NUMBER}/comments`; + const POST_COMMENT_URI = `https://api.github.com/repos/metamask/metamask-extension/issues/${PR_NUMBER}/comments`; console.log(`Announcement:\n${commentBody}`); console.log(`Posting to: ${POST_COMMENT_URI}`); @@ -355,7 +366,7 @@ async function start() { body: JSON_PAYLOAD, headers: { 'User-Agent': 'metamaskbot', - Authorization: `token ${GITHUB_COMMENT_TOKEN}`, + Authorization: `token ${PR_COMMENT_TOKEN}`, }, }); if (!response.ok) { From edaae773872e404ddedd460c6ff423eaea35378b Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 9 Jan 2025 19:05:04 -0330 Subject: [PATCH 15/23] fix: Fix bundle size tracking (#29486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The bundle size tracking was accidentally broken in #29408. This restores the bundle size tracking. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29486?quickstart=1) ## **Related issues** Fixes #29485 Resolves bug introduced by #29408 ## **Manual testing steps** 1. Check the "mv3: Bundle Size Stats" link in the `metamaskbot` comment 2. Once this is merged, check the bundle size tracker to ensure it's working again: https://github.com/MetaMask/extension_bundlesize_stats * Unfortunately I am not sure how to easily test this on the PR. The tracker is only updated when commits are made to `main`. ## **Screenshots/Recordings** N/A ## **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. --- .circleci/config.yml | 34 ++++++ package.json | 1 + test/e2e/mv3-perf-stats/bundle-size.js | 140 +++++++++++++++++++++++++ 3 files changed, 175 insertions(+) create mode 100755 test/e2e/mv3-perf-stats/bundle-size.js diff --git a/.circleci/config.yml b/.circleci/config.yml index 5243d8ef4043..4a2b1a30a530 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -277,6 +277,9 @@ workflows: - user-actions-benchmark: requires: - prep-build-test + - bundle-size: + requires: + - prep-build-test - job-publish-prerelease: requires: - prep-deps @@ -294,6 +297,7 @@ workflows: - prep-build-ts-migration-dashboard - benchmark - user-actions-benchmark + - bundle-size - all-tests-pass - job-publish-release: filters: @@ -1270,6 +1274,36 @@ jobs: paths: - test-artifacts + bundle-size: + executor: node-browsers-small + steps: + - run: *shallow-git-clone-and-enable-vnc + - run: sudo corepack enable + - attach_workspace: + at: . + - run: + name: Move test build to dist + command: mv ./dist-test ./dist + - run: + name: Move test zips to builds + command: mv ./builds-test ./builds + - run: + name: Measure bundle size + command: yarn bundle-size --out test-artifacts/chrome + - run: + name: Install jq + command: sudo apt install jq -y + - run: + name: Record bundle size at commit + command: ./.circleci/scripts/bundle-stats-commit.sh + - store_artifacts: + path: test-artifacts + destination: test-artifacts + - persist_to_workspace: + root: . + paths: + - test-artifacts + job-publish-prerelease: executor: node-browsers-medium steps: diff --git a/package.json b/package.json index 77b94c6e74a4..5d8ac90663c9 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "start:test:mv2:flask": "ENABLE_MV3=false yarn start:test:flask --apply-lavamoat=false --snow=false", "start:test:mv2": "ENABLE_MV3=false BLOCKAID_FILE_CDN=static.cx.metamask.io/api/v1/confirmations/ppom yarn start:test --apply-lavamoat=false --snow=false", "benchmark:chrome": "SELENIUM_BROWSER=chrome ts-node test/e2e/benchmark.js", + "bundle-size": "SELENIUM_BROWSER=chrome ts-node test/e2e/mv3-perf-stats/bundle-size.js", "user-actions-benchmark:chrome": "SELENIUM_BROWSER=chrome ts-node test/e2e/user-actions-benchmark.js", "benchmark:firefox": "SELENIUM_BROWSER=firefox ts-node test/e2e/benchmark.js", "build:test": "yarn env:e2e build test", diff --git a/test/e2e/mv3-perf-stats/bundle-size.js b/test/e2e/mv3-perf-stats/bundle-size.js new file mode 100755 index 000000000000..b6580d86538c --- /dev/null +++ b/test/e2e/mv3-perf-stats/bundle-size.js @@ -0,0 +1,140 @@ +#!/usr/bin/env node + +/* eslint-disable node/shebang */ +const path = require('path'); +const { promises: fs } = require('fs'); +const yargs = require('yargs/yargs'); +const { hideBin } = require('yargs/helpers'); +const { + isWritable, + getFirstParentDirectoryThatExists, +} = require('../../helpers/file'); + +const { exitWithError } = require('../../../development/lib/exit-with-error'); + +/** + * The e2e test case is used to capture bundle time statistics for extension. + */ + +const backgroundFiles = [ + 'scripts/runtime-lavamoat.js', + 'scripts/lockdown-more.js', + 'scripts/sentry-install.js', + 'scripts/policy-load.js', +]; + +const uiFiles = [ + 'scripts/sentry-install.js', + 'scripts/runtime-lavamoat.js', + 'scripts/lockdown-more.js', + 'scripts/policy-load.js', +]; + +const BackgroundFileRegex = /background-[0-9]*.js/u; +const CommonFileRegex = /common-[0-9]*.js/u; +const UIFileRegex = /ui-[0-9]*.js/u; + +async function main() { + const { argv } = yargs(hideBin(process.argv)).usage( + '$0 [options]', + 'Capture bundle size stats', + (_yargs) => + _yargs.option('out', { + description: + 'Output filename. Output printed to STDOUT of this is omitted.', + type: 'string', + normalize: true, + }), + ); + const { out } = argv; + + const distFolder = 'dist/chrome'; + const backgroundFileList = []; + const uiFileList = []; + const commonFileList = []; + + const files = await fs.readdir(distFolder); + for (let i = 0; i < files.length; i++) { + const file = files[i]; + if (CommonFileRegex.test(file)) { + const stats = await fs.stat(`${distFolder}/${file}`); + commonFileList.push({ name: file, size: stats.size }); + } else if ( + backgroundFiles.includes(file) || + BackgroundFileRegex.test(file) + ) { + const stats = await fs.stat(`${distFolder}/${file}`); + backgroundFileList.push({ name: file, size: stats.size }); + } else if (uiFiles.includes(file) || UIFileRegex.test(file)) { + const stats = await fs.stat(`${distFolder}/${file}`); + uiFileList.push({ name: file, size: stats.size }); + } + } + + const backgroundBundleSize = backgroundFileList.reduce( + (result, file) => result + file.size, + 0, + ); + + const uiBundleSize = uiFileList.reduce( + (result, file) => result + file.size, + 0, + ); + + const commonBundleSize = commonFileList.reduce( + (result, file) => result + file.size, + 0, + ); + + const result = { + background: { + name: 'background', + size: backgroundBundleSize, + fileList: backgroundFileList, + }, + ui: { + name: 'ui', + size: uiBundleSize, + fileList: uiFileList, + }, + common: { + name: 'common', + size: commonBundleSize, + fileList: commonFileList, + }, + }; + + if (out) { + const outPath = `${out}/bundle_size.json`; + const outputDirectory = path.dirname(outPath); + const existingParentDirectory = await getFirstParentDirectoryThatExists( + outputDirectory, + ); + if (!(await isWritable(existingParentDirectory))) { + throw new Error('Specified output file directory is not writable'); + } + if (outputDirectory !== existingParentDirectory) { + await fs.mkdir(outputDirectory, { recursive: true }); + } + await fs.writeFile(outPath, JSON.stringify(result, null, 2)); + await fs.writeFile( + `${out}/bundle_size_stats.json`, + JSON.stringify( + { + background: backgroundBundleSize, + ui: uiBundleSize, + common: commonBundleSize, + timestamp: new Date().getTime(), + }, + null, + 2, + ), + ); + } else { + console.log(JSON.stringify(result, null, 2)); + } +} + +main().catch((error) => { + exitWithError(error); +}); From 46bf1cfc4332d0cdda1091e4d69ccacfc3c2d596 Mon Sep 17 00:00:00 2001 From: jiexi Date: Thu, 9 Jan 2025 15:11:18 -0800 Subject: [PATCH 16/23] test: Remove obsolete permitted chains feature flag tests (#29618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Removes tests that are no longer applicable pertaining to permitted chains. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29618?quickstart=1) ## **Related issues** See: https://github.com/MetaMask/metamask-extension/pull/27847#discussion_r1908944940 ## **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. --- .../handlers/add-ethereum-chain.test.js | 444 ++++-------------- .../handlers/switch-ethereum-chain.test.js | 300 +++--------- 2 files changed, 156 insertions(+), 588 deletions(-) diff --git a/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.test.js b/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.test.js index ee0c9d3f732b..eb35e27a1c2b 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.test.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.test.js @@ -84,92 +84,62 @@ describe('addEthereumChainHandler', () => { jest.clearAllMocks(); }); - describe('with `endowment:permitted-chains` permissioning inactive', () => { - it('creates a new network configuration for the given chainid and switches to it if none exists', async () => { - const mocks = makeMocks(); - await addEthereumChainHandler( - { - origin: 'example.com', - params: [ - { - chainId: CHAIN_IDS.OPTIMISM, - chainName: 'Optimism Mainnet', - rpcUrls: ['https://optimism.llamarpc.com'], - nativeCurrency: { - symbol: 'ETH', - decimals: 18, - }, - blockExplorerUrls: ['https://optimistic.etherscan.io'], - iconUrls: ['https://optimism.icon.com'], - }, - ], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); + it('creates a new network configuration for the given chainid, requests `endowment:permitted-chains` permission and switches to it if no networkConfigurations with the same chainId exist', async () => { + const nonInfuraConfiguration = createMockNonInfuraConfiguration(); - expect(mocks.requestUserApproval).toHaveBeenCalledTimes(1); - expect(mocks.addNetwork).toHaveBeenCalledTimes(1); - expect(mocks.addNetwork).toHaveBeenCalledWith({ - blockExplorerUrls: ['https://optimistic.etherscan.io'], - defaultBlockExplorerUrlIndex: 0, - chainId: '0xa', - defaultRpcEndpointIndex: 0, - name: 'Optimism Mainnet', - nativeCurrency: 'ETH', - rpcEndpoints: [ + const mocks = makeMocks({ + permissionedChainIds: [], + overrides: { + getCurrentChainIdForDomain: jest + .fn() + .mockReturnValue(CHAIN_IDS.MAINNET), + }, + }); + await addEthereumChainHandler( + { + origin: 'example.com', + params: [ { - name: 'Optimism Mainnet', - url: 'https://optimism.llamarpc.com', - type: 'custom', + chainId: nonInfuraConfiguration.chainId, + chainName: nonInfuraConfiguration.name, + rpcUrls: nonInfuraConfiguration.rpcEndpoints.map((rpc) => rpc.url), + nativeCurrency: { + symbol: nonInfuraConfiguration.nativeCurrency, + decimals: 18, + }, + blockExplorerUrls: nonInfuraConfiguration.blockExplorerUrls, }, ], - }); - expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledWith(123); - }); + }, + {}, + jest.fn(), + jest.fn(), + mocks, + ); - it('creates a new networkConfiguration when called without "blockExplorerUrls" property', async () => { - const mocks = makeMocks(); - await addEthereumChainHandler( - { - origin: 'example.com', - params: [ - { - chainId: CHAIN_IDS.OPTIMISM, - chainName: 'Optimism Mainnet', - rpcUrls: ['https://optimism.llamarpc.com'], - nativeCurrency: { - symbol: 'ETH', - decimals: 18, - }, - iconUrls: ['https://optimism.icon.com'], - }, - ], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); - expect(mocks.addNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - }); + expect(mocks.addNetwork).toHaveBeenCalledWith(nonInfuraConfiguration); + expect( + mocks.grantPermittedChainsPermissionIncremental, + ).toHaveBeenCalledTimes(1); + expect( + mocks.grantPermittedChainsPermissionIncremental, + ).toHaveBeenCalledWith([createMockNonInfuraConfiguration().chainId]); + expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); + expect(mocks.setActiveNetwork).toHaveBeenCalledWith(123); + }); - describe('if a networkConfiguration for the given chainId already exists', () => { - it('updates the existing networkConfiguration with the new rpc url if it doesnt already exist', async () => { + describe('if a networkConfiguration for the given chainId already exists', () => { + describe('if the proposed networkConfiguration has a different rpcUrl from the one already in state', () => { + it('create a new networkConfiguration and switches to it without requesting permissions, if the requested chainId has `endowment:permitted-chains` permission granted for requesting origin', async () => { const mocks = makeMocks({ + permissionedChainIds: [CHAIN_IDS.MAINNET], overrides: { - getNetworkConfigurationByChainId: jest + getCurrentChainIdForDomain: jest .fn() - // Start with just infura endpoint - .mockReturnValue(createMockMainnetConfiguration()), + .mockReturnValue(CHAIN_IDS.SEPOLIA), }, }); - // Add a custom endpoint await addEthereumChainHandler( { origin: 'example.com', @@ -192,131 +162,38 @@ describe('addEthereumChainHandler', () => { mocks, ); - expect(mocks.updateNetwork).toHaveBeenCalledTimes(1); - expect(mocks.updateNetwork).toHaveBeenCalledWith( - '0x1', - { - chainId: '0x1', - name: 'Ethereum Mainnet', - // Expect both endpoints - rpcEndpoints: [ - { - networkClientId: 'mainnet', - url: 'https://mainnet.infura.io/v3/', - type: 'infura', - }, - { - name: 'Ethereum Mainnet', - url: 'https://eth.llamarpc.com', - type: 'custom', - }, - ], - // and the new one is the default - defaultRpcEndpointIndex: 1, - nativeCurrency: 'ETH', - blockExplorerUrls: ['https://etherscan.io'], - defaultBlockExplorerUrlIndex: 0, - }, - undefined, - ); + expect(mocks.requestUserApproval).toHaveBeenCalledTimes(1); + expect(mocks.requestPermittedChainsPermission).not.toHaveBeenCalled(); + expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); + expect(mocks.setActiveNetwork).toHaveBeenCalledWith(123); }); - it('makes the rpc url the default if it already exists', async () => { - const existingNetwork = { - chainId: '0x1', - name: 'Ethereum Mainnet', - // Start with infura + custom endpoint - rpcEndpoints: [ - { - networkClientId: 'mainnet', - url: 'https://mainnet.infura.io/v3/', - type: 'infura', - }, - { - name: 'Ethereum Mainnet', - url: 'https://eth.llamarpc.com', - type: 'custom', - }, - ], - // Infura is the default - defaultRpcEndpointIndex: 0, - nativeCurrency: 'ETH', - blockExplorerUrls: ['https://etherscan.io'], - defaultBlockExplorerUrlIndex: 0, - }; - + it('create a new networkConfiguration, requests permissions and switches to it, if the requested chainId does not have permittedChains permission granted for requesting origin', async () => { const mocks = makeMocks({ + permissionedChainIds: [], overrides: { getNetworkConfigurationByChainId: jest .fn() - .mockReturnValue(existingNetwork), - }, - }); - - // Add the same custom endpoint - await addEthereumChainHandler( - { - origin: 'example.com', - params: [ - { - chainId: CHAIN_IDS.MAINNET, - chainName: 'Ethereum Mainnet', - rpcUrls: ['https://eth.llamarpc.com'], - nativeCurrency: { - symbol: 'ETH', - decimals: 18, - }, - blockExplorerUrls: ['https://etherscan.io'], - }, - ], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); - - expect(mocks.updateNetwork).toHaveBeenCalledTimes(1); - expect(mocks.updateNetwork).toHaveBeenCalledWith( - '0x1', - { - ...existingNetwork, - // Verify the custom endpoint becomes the default - defaultRpcEndpointIndex: 1, - }, - undefined, - ); - }); - - it('switches to the network if its not already the currently selected chain id', async () => { - const existingNetwork = createMockMainnetConfiguration(); - - const mocks = makeMocks({ - overrides: { - // Start on sepolia + .mockReturnValue(createMockNonInfuraConfiguration()), getCurrentChainIdForDomain: jest .fn() - .mockReturnValue(CHAIN_IDS.SEPOLIA), - getNetworkConfigurationByChainId: jest - .fn() - .mockReturnValue(existingNetwork), + .mockReturnValue(CHAIN_IDS.MAINNET), }, }); - // Add with rpc + block explorers that already exist await addEthereumChainHandler( { origin: 'example.com', params: [ { - chainId: CHAIN_IDS.MAINNET, - chainName: 'Ethereum Mainnet', - rpcUrls: [existingNetwork.rpcEndpoints[0].url], + chainId: NON_INFURA_CHAIN_ID, + chainName: 'Custom Network', + rpcUrls: ['https://new-custom.network'], nativeCurrency: { - symbol: 'ETH', + symbol: 'CUST', decimals: 18, }, - blockExplorerUrls: ['https://etherscan.io'], + blockExplorerUrls: ['https://custom.blockexplorer'], }, ], }, @@ -326,65 +203,49 @@ describe('addEthereumChainHandler', () => { mocks, ); - // No updates, network already had all the info - expect(mocks.updateNetwork).toHaveBeenCalledTimes(0); - - // User should be prompted to switch chains + expect(mocks.updateNetwork).toHaveBeenCalledTimes(1); + expect( + mocks.grantPermittedChainsPermissionIncremental, + ).toHaveBeenCalledTimes(1); + expect( + mocks.grantPermittedChainsPermissionIncremental, + ).toHaveBeenCalledWith([NON_INFURA_CHAIN_ID]); expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledWith('mainnet'); - }); - - it('should return error for invalid chainId', async () => { - const mocks = makeMocks(); - const mockEnd = jest.fn(); - - await addEthereumChainHandler( - { - origin: 'example.com', - params: [{ chainId: 'invalid_chain_id' }], - }, - {}, - jest.fn(), - mockEnd, - mocks, - ); - - expect(mockEnd).toHaveBeenCalledWith( - rpcErrors.invalidParams({ - message: `Expected 0x-prefixed, unpadded, non-zero hexadecimal string 'chainId'. Received:\ninvalid_chain_id`, - }), - ); }); }); - }); - - describe('with `endowment:permitted-chains` permissioning active', () => { - it('creates a new network configuration for the given chainid, requests `endowment:permitted-chains` permission and switches to it if no networkConfigurations with the same chainId exist', async () => { - const nonInfuraConfiguration = createMockNonInfuraConfiguration(); + it('should switch to the existing networkConfiguration if one already exsits for the given chain id', async () => { const mocks = makeMocks({ - permissionedChainIds: [], + permissionedChainIds: [ + createMockOptimismConfiguration().chainId, + CHAIN_IDS.MAINNET, + ], overrides: { getCurrentChainIdForDomain: jest .fn() .mockReturnValue(CHAIN_IDS.MAINNET), + getNetworkConfigurationByChainId: jest + .fn() + .mockReturnValue(createMockOptimismConfiguration()), }, }); + await addEthereumChainHandler( { origin: 'example.com', params: [ { - chainId: nonInfuraConfiguration.chainId, - chainName: nonInfuraConfiguration.name, - rpcUrls: nonInfuraConfiguration.rpcEndpoints.map( + chainId: createMockOptimismConfiguration().chainId, + chainName: createMockOptimismConfiguration().name, + rpcUrls: createMockOptimismConfiguration().rpcEndpoints.map( (rpc) => rpc.url, ), nativeCurrency: { - symbol: nonInfuraConfiguration.nativeCurrency, + symbol: createMockOptimismConfiguration().nativeCurrency, decimals: 18, }, - blockExplorerUrls: nonInfuraConfiguration.blockExplorerUrls, + blockExplorerUrls: + createMockOptimismConfiguration().blockExplorerUrls, }, ], }, @@ -394,150 +255,11 @@ describe('addEthereumChainHandler', () => { mocks, ); - expect(mocks.addNetwork).toHaveBeenCalledWith(nonInfuraConfiguration); - expect( - mocks.grantPermittedChainsPermissionIncremental, - ).toHaveBeenCalledTimes(1); - expect( - mocks.grantPermittedChainsPermissionIncremental, - ).toHaveBeenCalledWith([createMockNonInfuraConfiguration().chainId]); + expect(mocks.requestPermittedChainsPermission).not.toHaveBeenCalled(); expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledWith(123); - }); - - describe('if a networkConfiguration for the given chainId already exists', () => { - describe('if the proposed networkConfiguration has a different rpcUrl from the one already in state', () => { - it('create a new networkConfiguration and switches to it without requesting permissions, if the requested chainId has `endowment:permitted-chains` permission granted for requesting origin', async () => { - const mocks = makeMocks({ - permissionedChainIds: [CHAIN_IDS.MAINNET], - overrides: { - getCurrentChainIdForDomain: jest - .fn() - .mockReturnValue(CHAIN_IDS.SEPOLIA), - }, - }); - - await addEthereumChainHandler( - { - origin: 'example.com', - params: [ - { - chainId: CHAIN_IDS.MAINNET, - chainName: 'Ethereum Mainnet', - rpcUrls: ['https://eth.llamarpc.com'], - nativeCurrency: { - symbol: 'ETH', - decimals: 18, - }, - blockExplorerUrls: ['https://etherscan.io'], - }, - ], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); - - expect(mocks.requestUserApproval).toHaveBeenCalledTimes(1); - expect(mocks.requestPermittedChainsPermission).not.toHaveBeenCalled(); - expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledWith(123); - }); - - it('create a new networkConfiguration, requests permissions and switches to it, if the requested chainId does not have permittedChains permission granted for requesting origin', async () => { - const mocks = makeMocks({ - permissionedChainIds: [], - overrides: { - getNetworkConfigurationByChainId: jest - .fn() - .mockReturnValue(createMockNonInfuraConfiguration()), - getCurrentChainIdForDomain: jest - .fn() - .mockReturnValue(CHAIN_IDS.MAINNET), - }, - }); - - await addEthereumChainHandler( - { - origin: 'example.com', - params: [ - { - chainId: NON_INFURA_CHAIN_ID, - chainName: 'Custom Network', - rpcUrls: ['https://new-custom.network'], - nativeCurrency: { - symbol: 'CUST', - decimals: 18, - }, - blockExplorerUrls: ['https://custom.blockexplorer'], - }, - ], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); - - expect(mocks.updateNetwork).toHaveBeenCalledTimes(1); - expect( - mocks.grantPermittedChainsPermissionIncremental, - ).toHaveBeenCalledTimes(1); - expect( - mocks.grantPermittedChainsPermissionIncremental, - ).toHaveBeenCalledWith([NON_INFURA_CHAIN_ID]); - expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - }); - }); - - it('should switch to the existing networkConfiguration if one already exsits for the given chain id', async () => { - const mocks = makeMocks({ - permissionedChainIds: [ - createMockOptimismConfiguration().chainId, - CHAIN_IDS.MAINNET, - ], - overrides: { - getCurrentChainIdForDomain: jest - .fn() - .mockReturnValue(CHAIN_IDS.MAINNET), - getNetworkConfigurationByChainId: jest - .fn() - .mockReturnValue(createMockOptimismConfiguration()), - }, - }); - - await addEthereumChainHandler( - { - origin: 'example.com', - params: [ - { - chainId: createMockOptimismConfiguration().chainId, - chainName: createMockOptimismConfiguration().name, - rpcUrls: createMockOptimismConfiguration().rpcEndpoints.map( - (rpc) => rpc.url, - ), - nativeCurrency: { - symbol: createMockOptimismConfiguration().nativeCurrency, - decimals: 18, - }, - blockExplorerUrls: - createMockOptimismConfiguration().blockExplorerUrls, - }, - ], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); - - expect(mocks.requestPermittedChainsPermission).not.toHaveBeenCalled(); - expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledWith( - createMockOptimismConfiguration().rpcEndpoints[0].networkClientId, - ); - }); + expect(mocks.setActiveNetwork).toHaveBeenCalledWith( + createMockOptimismConfiguration().rpcEndpoints[0].networkClientId, + ); }); }); diff --git a/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.test.js b/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.test.js index be612fbc7d8e..abd0c0eb9ff7 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.test.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.test.js @@ -16,16 +16,6 @@ const createMockMainnetConfiguration = () => ({ ], }); -const createMockLineaMainnetConfiguration = () => ({ - chainId: CHAIN_IDS.LINEA_MAINNET, - defaultRpcEndpointIndex: 0, - rpcEndpoints: [ - { - networkClientId: NETWORK_TYPES.LINEA_MAINNET, - }, - ], -}); - describe('switchEthereumChainHandler', () => { const makeMocks = ({ permissionedChainIds = [], @@ -55,228 +45,84 @@ describe('switchEthereumChainHandler', () => { jest.clearAllMocks(); }); - describe('with permittedChains permissioning inactive', () => { - it('should call setActiveNetwork when switching to a built-in infura network', async () => { - const mocks = makeMocks({ - overrides: { - getNetworkConfigurationByChainId: jest - .fn() - .mockReturnValue(createMockMainnetConfiguration()), - }, - }); - const switchEthereumChainHandler = switchEthereumChain.implementation; - await switchEthereumChainHandler( - { - origin: 'example.com', - params: [{ chainId: CHAIN_IDS.MAINNET }], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); - expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledWith( - createMockMainnetConfiguration().rpcEndpoints[0].networkClientId, - ); - }); - - it('should call setActiveNetwork when switching to a built-in infura network, when chainId from request is lower case', async () => { - const mocks = makeMocks({ - overrides: { - getNetworkConfigurationByChainId: jest - .fn() - .mockReturnValue(createMockLineaMainnetConfiguration()), - }, - }); - const switchEthereumChainHandler = switchEthereumChain.implementation; - await switchEthereumChainHandler( - { - origin: 'example.com', - params: [{ chainId: CHAIN_IDS.LINEA_MAINNET.toLowerCase() }], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); - expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledWith( - createMockLineaMainnetConfiguration().rpcEndpoints[0].networkClientId, - ); - }); - - it('should call setActiveNetwork when switching to a built-in infura network, when chainId from request is upper case', async () => { - const mocks = makeMocks({ - overrides: { - getNetworkConfigurationByChainId: jest - .fn() - .mockReturnValue(createMockLineaMainnetConfiguration()), - }, - }); - const switchEthereumChainHandler = switchEthereumChain.implementation; - await switchEthereumChainHandler( - { - origin: 'example.com', - params: [{ chainId: CHAIN_IDS.LINEA_MAINNET.toUpperCase() }], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); - expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledWith( - createMockLineaMainnetConfiguration().rpcEndpoints[0].networkClientId, - ); - }); - - it('should call setActiveNetwork when switching to a custom network', async () => { - const mocks = makeMocks({ - overrides: { - getCurrentChainIdForDomain: jest - .fn() - .mockReturnValue(CHAIN_IDS.MAINNET), - }, - }); - const switchEthereumChainHandler = switchEthereumChain.implementation; - await switchEthereumChainHandler( - { - origin: 'example.com', - params: [{ chainId: NON_INFURA_CHAIN_ID }], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); - expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledWith( - createMockMainnetConfiguration().rpcEndpoints[0].networkClientId, - ); - }); - - it('should handle missing networkConfiguration', async () => { - // Mock a network configuration that has an undefined or missing rpcEndpoints - const mockNetworkConfiguration = undefined; - - const mocks = makeMocks({ - overrides: { - getNetworkConfigurationByChainId: jest - .fn() - .mockReturnValue(mockNetworkConfiguration), - }, - }); - - const switchEthereumChainHandler = switchEthereumChain.implementation; - - const mockEnd = jest.fn(); - await switchEthereumChainHandler( - { - origin: 'example.com', - params: [{ chainId: CHAIN_IDS.MAINNET }], - }, - {}, - jest.fn(), - mockEnd, - mocks, - ); - - // Check that the function handled the missing rpcEndpoints and did not attempt to call setActiveNetwork - expect(mockEnd).toHaveBeenCalledWith( - expect.objectContaining({ - code: 4902, - message: expect.stringContaining('Unrecognized chain ID'), - }), - ); - expect(mocks.setActiveNetwork).not.toHaveBeenCalled(); + it('should call requestPermittedChainsPermission and setActiveNetwork when chainId is not in `endowment:permitted-chains`', async () => { + const mockrequestPermittedChainsPermission = jest.fn().mockResolvedValue(); + const mocks = makeMocks({ + overrides: { + requestPermittedChainsPermission: mockrequestPermittedChainsPermission, + }, }); + const switchEthereumChainHandler = switchEthereumChain.implementation; + await switchEthereumChainHandler( + { + origin: 'example.com', + params: [{ chainId: CHAIN_IDS.MAINNET }], + }, + {}, + jest.fn(), + jest.fn(), + mocks, + ); + + expect(mocks.requestPermittedChainsPermission).toHaveBeenCalledTimes(1); + expect(mocks.requestPermittedChainsPermission).toHaveBeenCalledWith([ + CHAIN_IDS.MAINNET, + ]); + expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); + expect(mocks.setActiveNetwork).toHaveBeenCalledWith( + createMockMainnetConfiguration().rpcEndpoints[0].networkClientId, + ); }); - describe('with permittedChains permissioning active', () => { - it('should call requestPermittedChainsPermission and setActiveNetwork when chainId is not in `endowment:permitted-chains`', async () => { - const mockrequestPermittedChainsPermission = jest - .fn() - .mockResolvedValue(); - const mocks = makeMocks({ - overrides: { - requestPermittedChainsPermission: - mockrequestPermittedChainsPermission, - }, - }); - const switchEthereumChainHandler = switchEthereumChain.implementation; - await switchEthereumChainHandler( - { - origin: 'example.com', - params: [{ chainId: CHAIN_IDS.MAINNET }], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); - - expect(mocks.requestPermittedChainsPermission).toHaveBeenCalledTimes(1); - expect(mocks.requestPermittedChainsPermission).toHaveBeenCalledWith([ - CHAIN_IDS.MAINNET, - ]); - expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledWith( - createMockMainnetConfiguration().rpcEndpoints[0].networkClientId, - ); + it('should call setActiveNetwork without calling requestPermittedChainsPermission when requested chainId is in `endowment:permitted-chains`', async () => { + const mocks = makeMocks({ + permissionedChainIds: [CHAIN_IDS.MAINNET], }); + const switchEthereumChainHandler = switchEthereumChain.implementation; + await switchEthereumChainHandler( + { + origin: 'example.com', + params: [{ chainId: CHAIN_IDS.MAINNET }], + }, + {}, + jest.fn(), + jest.fn(), + mocks, + ); + + expect(mocks.requestPermittedChainsPermission).not.toHaveBeenCalled(); + expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); + expect(mocks.setActiveNetwork).toHaveBeenCalledWith( + createMockMainnetConfiguration().rpcEndpoints[0].networkClientId, + ); + }); - it('should call setActiveNetwork without calling requestPermittedChainsPermission when requested chainId is in `endowment:permitted-chains`', async () => { - const mocks = makeMocks({ - permissionedChainIds: [CHAIN_IDS.MAINNET], - }); - const switchEthereumChainHandler = switchEthereumChain.implementation; - await switchEthereumChainHandler( - { - origin: 'example.com', - params: [{ chainId: CHAIN_IDS.MAINNET }], - }, - {}, - jest.fn(), - jest.fn(), - mocks, - ); - - expect(mocks.requestPermittedChainsPermission).not.toHaveBeenCalled(); - expect(mocks.setActiveNetwork).toHaveBeenCalledTimes(1); - expect(mocks.setActiveNetwork).toHaveBeenCalledWith( - createMockMainnetConfiguration().rpcEndpoints[0].networkClientId, - ); - }); - - it('should handle errors during the switch network permission request', async () => { - const mockError = new Error('Permission request failed'); - const mockrequestPermittedChainsPermission = jest - .fn() - .mockRejectedValue(mockError); - const mocks = makeMocks({ - overrides: { - requestPermittedChainsPermission: - mockrequestPermittedChainsPermission, - }, - }); - const mockEnd = jest.fn(); - const switchEthereumChainHandler = switchEthereumChain.implementation; - - await switchEthereumChainHandler( - { - origin: 'example.com', - params: [{ chainId: CHAIN_IDS.MAINNET }], - }, - {}, - jest.fn(), - mockEnd, - mocks, - ); - - expect(mocks.requestPermittedChainsPermission).toHaveBeenCalledTimes(1); - expect(mockEnd).toHaveBeenCalledWith(mockError); - expect(mocks.setActiveNetwork).not.toHaveBeenCalled(); + it('should handle errors during the switch network permission request', async () => { + const mockError = new Error('Permission request failed'); + const mockrequestPermittedChainsPermission = jest + .fn() + .mockRejectedValue(mockError); + const mocks = makeMocks({ + overrides: { + requestPermittedChainsPermission: mockrequestPermittedChainsPermission, + }, }); + const mockEnd = jest.fn(); + const switchEthereumChainHandler = switchEthereumChain.implementation; + + await switchEthereumChainHandler( + { + origin: 'example.com', + params: [{ chainId: CHAIN_IDS.MAINNET }], + }, + {}, + jest.fn(), + mockEnd, + mocks, + ); + + expect(mocks.requestPermittedChainsPermission).toHaveBeenCalledTimes(1); + expect(mockEnd).toHaveBeenCalledWith(mockError); + expect(mocks.setActiveNetwork).not.toHaveBeenCalled(); }); }); From 64400d8f15db2abb0995ce43dd5a140b58841309 Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Fri, 10 Jan 2025 11:35:48 +0100 Subject: [PATCH 17/23] chore: bump `@metamask/profile-sync-controller` to `v3.2.0` (#29598) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR bumps `@metamask/profile-sync-controller` to `v3.2.0`. This will add better error logging related to all things authentication & profile syncing [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29598?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/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. --- package.json | 2 +- .../errors-after-init-opt-in-background-state.json | 3 ++- .../errors-after-init-opt-in-ui-state.json | 1 + .../notifications&auth/data/notification-state.ts | 1 + ui/selectors/identity/profile-syncing.test.ts | 1 + yarn.lock | 12 ++++++------ 6 files changed, 12 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 5d8ac90663c9..1e084dbda471 100644 --- a/package.json +++ b/package.json @@ -340,7 +340,7 @@ "@metamask/post-message-stream": "^8.0.0", "@metamask/ppom-validator": "0.36.0", "@metamask/preinstalled-example-snap": "^0.3.0", - "@metamask/profile-sync-controller": "^3.1.1", + "@metamask/profile-sync-controller": "^3.2.0", "@metamask/providers": "^18.2.0", "@metamask/queued-request-controller": "^7.0.1", "@metamask/rate-limit-controller": "^6.0.0", diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json index cab47452ea3f..0480d536973d 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json @@ -347,6 +347,7 @@ "isProfileSyncingEnabled": null, "isProfileSyncingUpdateLoading": "boolean", "hasAccountSyncingSyncedAtLeastOnce": "boolean", - "isAccountSyncingReadyToBeDispatched": "boolean" + "isAccountSyncingReadyToBeDispatched": "boolean", + "isAccountSyncingInProgress": "boolean" } } diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json index e2b805831ea4..5a0f45d29c7e 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json @@ -219,6 +219,7 @@ "isProfileSyncingUpdateLoading": "boolean", "hasAccountSyncingSyncedAtLeastOnce": "boolean", "isAccountSyncingReadyToBeDispatched": "boolean", + "isAccountSyncingInProgress": "boolean", "subscriptionAccountsSeen": "object", "isMetamaskNotificationsFeatureSeen": "boolean", "isNotificationServicesEnabled": "boolean", diff --git a/test/integration/notifications&auth/data/notification-state.ts b/test/integration/notifications&auth/data/notification-state.ts index 61d74d161671..d91c1e7f0d2e 100644 --- a/test/integration/notifications&auth/data/notification-state.ts +++ b/test/integration/notifications&auth/data/notification-state.ts @@ -40,6 +40,7 @@ export const getMockedNotificationsState = () => { isProfileSyncingUpdateLoading: false, hasAccountSyncingSyncedAtLeastOnce: false, isAccountSyncingReadyToBeDispatched: false, + isAccountSyncingInProgress: false, isMetamaskNotificationsFeatureSeen: true, isNotificationServicesEnabled: true, isFeatureAnnouncementsEnabled: true, diff --git a/ui/selectors/identity/profile-syncing.test.ts b/ui/selectors/identity/profile-syncing.test.ts index d05512e59523..74d4a31984f9 100644 --- a/ui/selectors/identity/profile-syncing.test.ts +++ b/ui/selectors/identity/profile-syncing.test.ts @@ -7,6 +7,7 @@ describe('Profile Syncing Selectors', () => { isProfileSyncingUpdateLoading: false, isAccountSyncingReadyToBeDispatched: false, hasAccountSyncingSyncedAtLeastOnce: false, + isAccountSyncingInProgress: false, }, }; diff --git a/yarn.lock b/yarn.lock index e98bb06943fe..ccb5a1a0e260 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6073,11 +6073,11 @@ __metadata: languageName: node linkType: hard -"@metamask/profile-sync-controller@npm:^3.1.1": - version: 3.1.1 - resolution: "@metamask/profile-sync-controller@npm:3.1.1" +"@metamask/profile-sync-controller@npm:^3.2.0": + version: 3.2.0 + resolution: "@metamask/profile-sync-controller@npm:3.2.0" dependencies: - "@metamask/base-controller": "npm:^7.0.2" + "@metamask/base-controller": "npm:^7.1.0" "@metamask/keyring-api": "npm:^12.0.0" "@metamask/keyring-controller": "npm:^19.0.2" "@metamask/network-controller": "npm:^22.1.1" @@ -6095,7 +6095,7 @@ __metadata: "@metamask/providers": ^18.1.0 "@metamask/snaps-controllers": ^9.10.0 webextension-polyfill: ^0.10.0 || ^0.11.0 || ^0.12.0 - checksum: 10/42dd1ea0f9595eca6bd1a179681f7d16dfc6cc5090494131c8999a15caa02866f133febe1a95fca7cc74b4f3dafa7ef740f27b08eadbbf3049ebc087a739fb1b + checksum: 10/6fabb31266dfe14e67a3fcfcf8e5826786519bc0086d109b5cf324e4fa87f1fe5916bab2e9ac92489950531dee81e67e298b00b60fdaf9821312d170435005bf languageName: node linkType: hard @@ -26665,7 +26665,7 @@ __metadata: "@metamask/ppom-validator": "npm:0.36.0" "@metamask/preferences-controller": "npm:^15.0.1" "@metamask/preinstalled-example-snap": "npm:^0.3.0" - "@metamask/profile-sync-controller": "npm:^3.1.1" + "@metamask/profile-sync-controller": "npm:^3.2.0" "@metamask/providers": "npm:^18.2.0" "@metamask/queued-request-controller": "npm:^7.0.1" "@metamask/rate-limit-controller": "npm:^6.0.0" From 5778b4a64a2b7aeba40c46e0afb295d214b36355 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Fri, 10 Jan 2025 12:18:33 +0000 Subject: [PATCH 18/23] feat: Display clickable cursor on hover on petname component (#29477) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29477?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2340 ## **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. --- ui/components/app/name/index.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/components/app/name/index.scss b/ui/components/app/name/index.scss index f82638373ecd..8037a25390e4 100644 --- a/ui/components/app/name/index.scss +++ b/ui/components/app/name/index.scss @@ -6,6 +6,7 @@ gap: 5px; font-size: 12px; max-width: 100%; + cursor: pointer; &__missing { background-color: var(--color-background-alternative); From dd26784d605c92cad0bbce04985724c9b1c088c7 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Fri, 10 Jan 2025 14:27:07 +0000 Subject: [PATCH 19/23] feat: Nonce is always editable in advanced details view (#29627) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Previously, the nonce can only be edited if the option to do it is enabled in the corresponding settings toggle. This PR removes the link with that toggle in the redesigned transaction screens. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29627?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/29512 ## **Manual testing steps** 1. Go to the test dApp 2. Disable `Advanced > Customize transaction nonce` in settings 3. Create a send eth transaction 4. Toggle on advanced details 5. The edit nonce icon should be visible in the confirmation screen ## **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. --- .../__snapshots__/approve.test.tsx.snap | 11 ++++++++++ .../advanced-details.test.tsx.snap | 22 +++++++++++++++++++ .../advanced-details/advanced-details.tsx | 6 +---- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/approve/__snapshots__/approve.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/approve/__snapshots__/approve.test.tsx.snap index 8e13a0885858..75fdd3765851 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/__snapshots__/approve.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/info/approve/__snapshots__/approve.test.tsx.snap @@ -634,6 +634,17 @@ exports[` renders component for approve request 1`] = ` class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0" data-testid="advanced-details-displayed-nonce" > +

    renders component when the prop override is passed class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0" data-testid="advanced-details-displayed-nonce" > +

    renders component when the state property is true 1 class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0" data-testid="advanced-details-displayed-nonce" > +

    { } }, [currentConfirmation, dispatch]); - const enableCustomNonce = useSelector(getUseNonceField); const nextNonce = useSelector(getNextSuggestedNonce); const customNonceValue = useSelector(getCustomNonceValue); @@ -65,9 +63,7 @@ const NonceDetails = () => { openEditNonceModal() : undefined - } + onEditClick={() => openEditNonceModal()} editIconClassName="edit-nonce-btn" editIconDataTestId="edit-nonce-icon" /> From 3ff4aba94aacf6a46488735c7656e6a1b7c1cdc2 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Fri, 10 Jan 2025 15:35:34 +0100 Subject: [PATCH 20/23] fix: Remove unwanted empty `div` from signature confirmations (#29622) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** https://github.com/MetaMask/metamask-extension/pull/28854/files introduced an unwanted empty div on top of signature content. This PR aims to relocate that into child component. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29622?quickstart=1) ## **Related issues** ## **Manual testing steps** N/A ## **Screenshots/Recordings** ### **Before** Screenshot 2025-01-10 at 09 59 45 ### **After** Screenshot 2025-01-10 at 09 58 38 ## **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. --- .../smart-transactions-banner-alert.tsx | 45 ++++++++++--------- .../__snapshots__/confirm.test.tsx.snap | 24 ---------- ui/pages/confirmations/confirm/confirm.tsx | 5 +-- 3 files changed, 25 insertions(+), 49 deletions(-) diff --git a/ui/pages/confirmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.tsx b/ui/pages/confirmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.tsx index e9f28a25c318..6a2739c0eeb1 100644 --- a/ui/pages/confirmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.tsx +++ b/ui/pages/confirmations/components/smart-transactions-banner-alert/smart-transactions-banner-alert.tsx @@ -5,6 +5,7 @@ import { useI18nContext } from '../../../../hooks/useI18nContext'; import { BannerAlert, ButtonLink, + Box, Text, BannerAlertSeverity, } from '../../../../components/component-library'; @@ -95,27 +96,29 @@ export const SmartTransactionsBannerAlert: React.FC - - {t('smartTransactionsEnabledTitle')} - - - - {t('smartTransactionsEnabledLink')} - - {t('smartTransactionsEnabledDescription')} - - + + + + {t('smartTransactionsEnabledTitle')} + + + + {t('smartTransactionsEnabledLink')} + + {t('smartTransactionsEnabledDescription')} + + + ); }); diff --git a/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap b/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap index 85c6e24b3d78..f528654a4525 100644 --- a/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap +++ b/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap @@ -116,9 +116,6 @@ exports[`Confirm matches snapshot for signature - personal sign type 1`] = `

-
-
-
-
-
-
-
-
{ return ( @@ -55,9 +54,7 @@ const Confirm = () => (
- - - + { ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) From 1df79a9306a84319121493f1f00605dc9d987713 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 10 Jan 2025 12:14:11 -0330 Subject: [PATCH 21/23] chore: Remove obsolete keys (#29372) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Remove references to obsolete keys. * PUBNUB_*: These were used for the old mobile state sync feature, which was removed a long time ago. * ETHERSCAN_KEY: This was used for incoming transactions, but we've since switched to our own API. * OPENSEA_KEY: We've switched to Reservoir The `ETHERSCAN_KEY ` environment variable is still refrenced in the `gridplus-sdk` package, so it has been left in the `build.yml` file as `null` to indicate that it's intentionally unset. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29372?quickstart=1) ## **Related issues** N/A ## **Manual testing steps** N/A ## **Screenshots/Recordings** N/A ## **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/scripts/metamask-controller.js | 2 -- builds.yml | 11 +++++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 38ec6023b264..c98ba72c0f96 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -761,8 +761,6 @@ export default class MetamaskController extends EventEmitter { }), }); - this.nftController.setApiKey(process.env.OPENSEA_KEY); - const nftDetectionControllerMessenger = this.controllerMessenger.getRestricted({ name: 'NftDetectionController', diff --git a/builds.yml b/builds.yml index 43cf02d3c8e5..ddc4858eb973 100644 --- a/builds.yml +++ b/builds.yml @@ -214,13 +214,9 @@ env: # API keys to 3rd party services ### - - PUBNUB_PUB_KEY: null - - PUBNUB_SUB_KEY: null - SEGMENT_HOST: null - SENTRY_DSN: null - SENTRY_DSN_DEV: null - - OPENSEA_KEY: null - - ETHERSCAN_KEY: null # also INFURA_PROJECT_ID below ### @@ -318,3 +314,10 @@ env: # This should NEVER be enabled in production since it slows down react ### - ENABLE_WHY_DID_YOU_RENDER: false + + ### + # Unused environment variables referenced in dependencies + # Unset environment variables cause a build error. These are set to `null` to tell our build + # system that they are intentionally unset. + ### + - ETHERSCAN_KEY: null # Used by `gridplus-sdk/dist/util.js` From f58258b7ac25137c8e3155e60c6620b206a10f9f Mon Sep 17 00:00:00 2001 From: jiexi Date: Fri, 10 Jan 2025 07:51:34 -0800 Subject: [PATCH 22/23] refactor: remove unused end param in ethereum-chain-util helpers (#29619) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Removes unused `end` param in the ethereum-chain-util helpers * validateChainId * validateAddEthereumChainParams * validateSwitchEthereumChainParams [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29619?quickstart=1) Extending E2E timeout to get past "no timings found" error: ``` flags = { "circleci": { "timeoutMinutes": 30 } } ``` ## **Related issues** See: https://github.com/MetaMask/metamask-extension/pull/27847#discussion_r1907939905 ## **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. --- .../rpc-method-middleware/handlers/add-ethereum-chain.js | 2 +- .../handlers/ethereum-chain-utils.js | 8 ++++---- .../handlers/switch-ethereum-chain.js | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js b/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js index afcc2e167043..66d57dd8786b 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js @@ -50,7 +50,7 @@ async function addEthereumChainHandler( ) { let validParams; try { - validParams = validateAddEthereumChainParams(req.params[0], end); + validParams = validateAddEthereumChainParams(req.params[0]); } catch (error) { return end(error); } diff --git a/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js b/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js index 10973e052715..2f86b30885e5 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js @@ -25,7 +25,7 @@ export function validateChainId(chainId) { return _chainId; } -export function validateSwitchEthereumChainParams(req, end) { +export function validateSwitchEthereumChainParams(req) { if (!req.params?.[0] || typeof req.params[0] !== 'object') { throw rpcErrors.invalidParams({ message: `Expected single, object parameter. Received:\n${JSON.stringify( @@ -43,10 +43,10 @@ export function validateSwitchEthereumChainParams(req, end) { }); } - return validateChainId(chainId, end); + return validateChainId(chainId); } -export function validateAddEthereumChainParams(params, end) { +export function validateAddEthereumChainParams(params) { if (!params || typeof params !== 'object') { throw rpcErrors.invalidParams({ message: `Expected single, object parameter. Received:\n${JSON.stringify( @@ -75,7 +75,7 @@ export function validateAddEthereumChainParams(params, end) { }); } - const _chainId = validateChainId(chainId, end); + const _chainId = validateChainId(chainId); if (!rpcUrls || !Array.isArray(rpcUrls) || rpcUrls.length === 0) { throw rpcErrors.invalidParams({ message: `Expected an array with at least one valid string HTTPS url 'rpcUrls', Received:\n${rpcUrls}`, diff --git a/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js b/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js index 5f907bef4d4b..1fbeedbef3f5 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js @@ -36,7 +36,7 @@ async function switchEthereumChainHandler( ) { let chainId; try { - chainId = validateSwitchEthereumChainParams(req, end); + chainId = validateSwitchEthereumChainParams(req); } catch (error) { return end(error); } From b2c53144a0e67971b625b4f9d430aeba40f11c87 Mon Sep 17 00:00:00 2001 From: Norbert Elter <72046715+itsyoboieltr@users.noreply.github.com> Date: Fri, 10 Jan 2025 21:46:34 +0400 Subject: [PATCH 23/23] fix: metamaskbot comment nits (#29636) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29636?quickstart=1) This PR adds minor fixes and enhancements to things that were not worth blocking the original metamaskbot PR for. Things like, comments, style, reusability should be improved by this PR. Interestingly, I also saw that the `$OWNER` environment variable was missing, but somehow the workflow still worked. I added this environment variable just to make it sane. ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/28572 ## **Manual testing steps** 1. Everything should still work ## **Screenshots/Recordings** Not applicable ## **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. --- .github/workflows/publish-prerelease.yml | 29 +++++++++++++----------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/.github/workflows/publish-prerelease.yml b/.github/workflows/publish-prerelease.yml index 9b4e460d546a..2674b8565eff 100644 --- a/.github/workflows/publish-prerelease.yml +++ b/.github/workflows/publish-prerelease.yml @@ -25,41 +25,44 @@ jobs: BASE_REF: ${{ github.event.pull_request.base.ref }} run: | merge_base="$(git merge-base "origin/${BASE_REF}" HEAD)" - echo "Merge base is '${merge_base}'" echo "MERGE_BASE=${merge_base}" >> "$GITHUB_OUTPUT" + echo "Merge base is '${merge_base}'" - name: Get CircleCI job details id: get-circleci-job-details env: - REPOSITORY: ${{ github.repository }} + OWNER: ${{ github.repository_owner }} + REPOSITORY: ${{ github.event.repository.name }} + # For a `pull_request` event, the branch is `github.head_ref``. BRANCH: ${{ github.head_ref }} + # For a `pull_request` event, the head commit hash is `github.event.pull_request.head.sha`. HEAD_COMMIT_HASH: ${{ github.event.pull_request.head.sha }} + JOB_NAME: job-publish-prerelease run: | - pipeline_id=$(curl --silent "https://circleci.com/api/v2/project/gh/$OWNER/$REPOSITORY/pipeline?branch=$BRANCH" | jq -r ".items | map(select(.vcs.revision == \"${HEAD_COMMIT_HASH}\" )) | first | .id") + pipeline_id=$(curl --silent "https://circleci.com/api/v2/project/gh/$OWNER/$REPOSITORY/pipeline?branch=$BRANCH" | jq --arg head_commit_hash "${HEAD_COMMIT_HASH}" -r '.items | map(select(.vcs.revision == $head_commit_hash)) | first | .id') workflow_id=$(curl --silent "https://circleci.com/api/v2/pipeline/$pipeline_id/workflow" | jq -r ".items[0].id") - job_details=$(curl --silent "https://circleci.com/api/v2/workflow/$workflow_id/job" | jq -r '.items[] | select(.name == "job-publish-prerelease")') - build_num=$(echo "$job_details" | jq -r '.job_number') + job_details=$(curl --silent "https://circleci.com/api/v2/workflow/$workflow_id/job" | jq --arg job_name "${JOB_NAME}" -r '.items[] | select(.name == $job_name)') + build_num=$(echo "$job_details" | jq -r '.job_number') echo 'CIRCLE_BUILD_NUM='"$build_num" >> "$GITHUB_OUTPUT" + job_id=$(echo "$job_details" | jq -r '.id') echo 'CIRCLE_WORKFLOW_JOB_ID='"$job_id" >> "$GITHUB_OUTPUT" - echo "Getting artifacts from pipeline '${pipeline_id}', workflow '${workflow_id}', build number '${build_num}', job ID '${job_id}'" + echo "Getting artifacts from pipeline '${pipeline_id}', workflow '${workflow_id}', build number '${build_num}', job id '${job_id}'" - name: Get CircleCI job artifacts env: CIRCLE_WORKFLOW_JOB_ID: ${{ steps.get-circleci-job-details.outputs.CIRCLE_WORKFLOW_JOB_ID }} run: | - mkdir -p "test-artifacts/chrome/benchmark" + mkdir -p test-artifacts/chrome/benchmark curl --silent --location "https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0/test-artifacts/chrome/benchmark/pageload.json" > "test-artifacts/chrome/benchmark/pageload.json" - bundle_size=$(curl --silent --location "https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0/test-artifacts/chrome/bundle_size.json") - mkdir -p "test-artifacts/chrome" - echo "${bundle_size}" > "test-artifacts/chrome/bundle_size.json" + mkdir -p test-artifacts/chrome + curl --silent --location "https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0/test-artifacts/chrome/bundle_size.json" > "test-artifacts/chrome/bundle_size.json" - stories=$(curl --silent --location "https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0/storybook/stories.json") - mkdir "storybook-build" - echo "${stories}" > "storybook-build/stories.json" + mkdir storybook-build + curl --silent --location "https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0/storybook/stories.json" > "storybook-build/stories.json" - name: Publish prerelease env: