feat: upgrade eth-ledger-bridge-keyring to version 4.1.0 #25835
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dawnseeker8
added
needs-dev-review
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@metamask/[email protected], npm/@metamask/[email protected], npm/@metamask/[email protected], npm/[email protected], npm/[email protected] |
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is network access?This module accesses the network. Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
…cies with `yarn lavamoat:auto` command.
@SocketSecurity ignore npm/[email protected] |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #25835 +/- ##
========================================
Coverage 69.67% 69.67%
========================================
Files 1402 1402
Lines 49652 49652
Branches 13720 13720
========================================
Hits 34594 34594
Misses 15058 15058 ☔ View full report in Codecov by Sentry. |
|
# **Description** Bump the BTC Snap to version 0.3.0. - This new release adds the fee estimation feature. [](https://codespaces.new/MetaMask/metamask-extension/pull/25808?quickstart=1) ## **Related issues** N/A ## **Manual testing steps** N/A ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/26115?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> This PR removes BTC accounts from the account picker and `Your Accounts` list in the send flow. [](https://codespaces.new/MetaMask/metamask-extension/pull/26271?quickstart=1) ## **Related issues** Fixes: MetaMask/accounts-planning#546 ## **Manual testing steps** 1. Create btc account 2. Select an evm account 3. Click send 4. See that the BTC account is not available in `Your Accounts` 5. Click the account picker and see that the BTC account is not available. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] -->   ### **After** <!-- [screenshots/recordings] -->   ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Charly Chevalier <[email protected]>
## **Description** Migration 120.2 has been refactored to prepare for the addition of further obsolete state cleanup steps. A few minor functional changes and test changes have been made as well. The changes are: * The migration has been refactored to group together changes for each controller in separate functions. * The `SelectedNetworkController` migration has been updated to delete state rather than set it to default. This is functionally equivalent (for this controller) and it simplified the migration and tests a bit, avoiding the need to verify that the default state was correct. * When the `SnapController` state is invalid, we now capture a Sentry exception * A test has been added for this case as well * JSDoc comments have been added to the migration * Tests have been grouped together by controller * Tests have been added to ensure corrupted controller state does not impact the obsolete state removal steps of other controllers * Effectively each obsolete state removal function is acting as an independent migration here * A test was added for the case where `SnapController` state is not set * The tests have been updated to deep clone the input to guard against the migration failing to do so * The `transformState` function was given an explicit return type, and the unused return value was removed. [](https://codespaces.new/MetaMask/metamask-extension/pull/26298?quickstart=1) ## **Related issues** Relates to #26297 ## **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/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…its (#26227) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/26227?quickstart=1) ## **Related issues** Fixes: #26226 Fixes: MetaMask/MetaMask-planning#2730 (older, original issue) ## **Manual testing steps** 1. Go to cowswap 2. Swap a token with gas-free approve for another token 3. Notice the permit signature screen or 1. run `yarn storybook` 2. go to http://localhost:6006/?path=/story/pages-confirmations-confirm-signatures-signedtypeddatav3orv4--permit-story&args=msgParams.data:3.0001231231212312e+22 3. copy and paste the following into data ``` "{\"types\":{\"EIP712Domain\":[{\"name\":\"name\",\"type\":\"string\"},{\"name\":\"version\",\"type\":\"string\"},{\"name\":\"chainId\",\"type\":\"uint256\"},{\"name\":\"verifyingContract\",\"type\":\"address\"}],\"Permit\":[{\"name\":\"owner\",\"type\":\"address\"},{\"name\":\"spender\",\"type\":\"address\"},{\"name\":\"value\",\"type\":\"uint256\"},{\"name\":\"nonce\",\"type\":\"uint256\"},{\"name\":\"deadline\",\"type\":\"uint256\"}]},\"primaryType\":\"Permit\",\"domain\":{\"name\":\"MyToken\",\"version\":\"1\",\"verifyingContract\":\"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC\",\"chainId\":1},\"message\":{\"owner\":\"0x935e73edb9ff52e23bac7f7e043a1ecd06d05477\",\"spender\":\"0x5B38Da6a701c568545dCfcB03FcB875f56beddC4\",\"value\":30001231231212312138768,\"nonce\":0,\"deadline\":50000000000}}" ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <img width="520" src="https://github.com/user-attachments/assets/1b6e45cb-f54b-45e5-968c-141359de7466"> <img width="320" src="https://github.com/user-attachments/assets/804d4e8a-1c47-470e-88a4-c6195c5cf96e"> ## **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.
## **Description** Migration 120.2 has been updated to remove obsolete NetworkController state as well. [](https://codespaces.new/MetaMask/metamask-extension/pull/26302?quickstart=1) ## **Related issues** Fixes #26297 ## **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/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
## **Description** Sentry reports showed cases of obsolete state in the PhishingController state. This obsolete state has been removed. [](https://codespaces.new/MetaMask/metamask-extension/pull/26308?quickstart=1) ## **Related issues** Fixes #26307 ## **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/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Fix if clause to fetch token decimals for permit and order signatures. [](https://codespaces.new/MetaMask/metamask-extension/pull/26292?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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.
## **Description** Fixes an issue when `Show balance and token price checker` is turned off in settings, certain actions like sending, importing, or swapping a token crash metamask. [](https://codespaces.new/MetaMask/metamask-extension/pull/26264?quickstart=1) ## **Related issues** Fixes: #26243 ## **Manual testing steps** 1. Go into settings > security and privacy 2. Turn off `Show balance and token price checker` 3. Switch chains 4. Click import tokens on the tokens tab. Modal should popup without error 5. Click swap and enter tokens + amount. Swap rate should fetch successfully. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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.
…cking whether the origin was eligible for setting its own network (#26323) ## **Description** This PR fixes a issue where `setNetworkClientIdForDomain` was frequently called with origins that had no account permissions yet (which is the thresshold we currently set for giving site's their own network) resulting in [a large number of silent errors in the background that are clogging up sentry](https://metamask.sentry.io/issues/5659582204/?environment=production&project=273505&qu%5B%E2%80%A6%5Derrer=issue-stream&sort=freq&statsPeriod=90d&stream_index=1). [](https://codespaces.new/MetaMask/metamask-extension/pull/26323?quickstart=1) ## **Related issues** Fixes: https://metamask.sentry.io/issues/5659582204/?environment=production&project=273505&qu%5B%E2%80%A6%5Derrer=issue-stream&sort=freq&statsPeriod=90d&stream_index=1 ## **Manual testing steps** N/A ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** N/A ### **After** N/A ## **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.
## **Description** Remove invalid `providerConfig.id` state. This was one possible explanation for some Sentry errors that we have been encountering in production. The diagnostic state attached to the error was not sufficient to confirm that this was the cause. [](https://codespaces.new/MetaMask/metamask-extension/pull/26310?quickstart=1) ## **Related issues** Relates to #26309 and #26320 ## **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/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** The 'restore backup' feature in Extension advanced settings is currently broken and we suspect it can lead to state corruption issues. As a short term action, we shall disable the feature until we find a way to really fix it. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/26325?quickstart=1) ## **Related issues** Fixes: MetaMask/MetaMask-planning#2946 ## **Manual testing steps** 1. Go to settings in extension 2. Go to advanced 3. Expect only 1 data exporting option left ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <img width="833" alt="截屏2024-08-02 16 46 32" src="https://github.com/user-attachments/assets/f55ea7e3-4d8e-496b-9530-510d37ae1ba0"> <!-- [screenshots/recordings] --> ### **After** <img width="838" alt="截屏2024-08-02 17 08 04" src="https://github.com/user-attachments/assets/cf994c64-6d1e-485b-88ab-c6cd99cb3235"> <!-- [screenshots/recordings] --> ## **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.
…sing (#26327) ## **Description** The `providerConfig` state is not guaranteed to match a built-in or custom network. It should in the vast majority of cases, but there are some edge cases where that would not hold true. For example: * If the wallet crashed partway through removing the current network * If the user has had the same network selected since before we introduced the `networkConfigurations` state The `currentNetwork` selector has been updated to handle this case by returning a configuration object constructed from the `providerConfig` state directly. [](https://codespaces.new/MetaMask/metamask-extension/pull/26327?quickstart=1) ## **Related issues** Fixes #26320 ## **Manual testing steps** * Create a dev build and proceed through onboarding * Run this command in the background console to set `providerConfig` to a custom network that isn't present in the user's tracked network configurations: ``` chrome.storage.local.get( null, (state) => { state.data.NetworkController.providerConfig = { rpcUrl: 'https://mainnet.optimism.io', chainId: '0xa', ticker: 'ETH', type: 'rpc', }; chrome.storage.local.set(state, () => chrome.runtime.reload()); } ); ``` * See that the UI can be opened and does not crash. ## **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/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
#22506) Closes MetaMask/MetaMask-planning#1477 and MetaMask/MetaMask-planning#1903 This commit adds an alternative build process that is much faster than the gulp build we have now, which is quite slow and doesn't make use of modern build system improvements. The speed up is possible by making use of the SWC compiler, and more modern build system. The build system is also a bit simpler and hopefully more maintainable. This build doesn't yet support: * HMR/chromereload (requires we get rid of our circular dependencies) * lavamoat (neither running the build system _in_ lavamoat, or adding lavamoat protections) * production builds (because of not supporting lavamoat) * MV3 (requires writing a webpack plugin) Co-authored-by: Howard Braham <[email protected]>
…y match 'Premature close' (#26336) ## **Description** We have long seen many errors in Sentry that have the message "Premature close". This PR addresses those that occur when the UI is closed, and when a connected dapp is closed. This error is thrown from within the readable-stream module when multiple streams are passed to the same pipeline. We do not fully understand the issue yet, but it is suspected that we are incorrectly triggering or handling the destroy or end of a stream. Some relevant findings: - others have experienced similar problems: mafintosh/pump#25 - there are two separate "premature close" errors. One pre-existed this PR (and v12.0.0) #24533, and one was introduced with that PR - the one that was introduced with that PR goes away if the readable-stream dependency of object-multiplex is revert back to v2.3.3 - "so I think the problem is that we are explicitly calling destroy on some of the stream's in the pipeline, before the pipeline is finished doing whatever it needs to do when when the streams are ended/closed/finished" - if we use `connectionStream.pipe(mux).pipe(connectionStream);` instead of `pipeline(connectionStream, mux, connectionStream`, the error goes away, but that was changed almost 7 years ago in a PR titled "Memory leak fixes - stream and filter life cycles" #2070 - With reference to `setupMultiplex`: - if I change that line to pipeline(connectionStream, mux, (err) => {, the error goes away - but if I change it to pipeline(mux, connectionStream, (err) => {, the error does not go away We understand enough at this point to know that these error are not affecting user experience. For the moment, we are going to explicitly stop logging them, as the are clogging sentry, and [we can revisit the search for root cause in the future ](#26337) Note that there is at least one outstanding case of a "Premature close" error that can occur when the application is loading. This PR does not attempt to remove that error from being logged. [](https://codespaces.new/MetaMask/metamask-extension/pull/26336?quickstart=1) ## **Related issues** Fixes: #26284 ## **Manual testing steps** 1. Open the UI 2. Clear the background console 3. Close the UI 4. There should be no "Premature close" error ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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.
…26268) The functionality is the same as before, but it this change works around a bug in https://github.com/terser/html-minifier-terser which causes the space to be removed, and the tag to become invalid (this only affects the webpack build). Upstream issues: webdiscus/html-bundler-webpack-plugin#106 terser/html-minifier-terser#178
## **Description** A test has been added for migration 120.2 to cover the scenario where all state migrations are applicable. The JSDoc block for the migration has also been updated to reflect all changes, previously it only covered a small subset. [](https://codespaces.new/MetaMask/metamask-extension/pull/26333?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/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> This patches an issue in `add-contact.component.js` where the disabled state of the `Save` button would disappear only after a successful ENS resolution, effectively preventing plain addresses to be entered. I also added some extra unit tests to check for some of the cases that weren't covered before. [](https://codespaces.new/MetaMask/metamask-extension/pull/26155?quickstart=1) ## **Related issues** fixes #25918 fixes #25889 ## **Manual testing steps** 1. Go to Settings > Contacts > Add Contact 2. Enter a name and a valid Ethereum address 3. Observe the Save button getting enabled 4. Edit the address to make it invalid, observe the Save button getting disabled 5. Instead of an address, enter an ENS name and pick the resolved result 6. Observe the Save button getting enabled 7. Observe that the input is still editable (repeat 4.) ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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 - [ ] 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. --------- Signed-off-by: Mircea Nistor <[email protected]>
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Captured in https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/95033/workflows/faea9f93-9600-4207-bb3c-2dc17eec663b/jobs/3536505/tests. This test is flaky from not waiting for token price to be fetched and rendered properly. The simple fix would be - wait until token is imported after modal is closed - use `findVisibleElement` and the built in `timeout` to wait for element to be rendered properly Attached to this PR is a report [here](#26351 (comment)) of running it 20 times after the fix and results in no flakiness. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/26351?quickstart=1) ## **Related issues** Fixes: #26350 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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.
…cies with `yarn lavamoat:auto` command.
|
Rebase error cause a lot of conflict happened in the branch, need to redo the simply change again with new branch( generated from latest |
](https://codespaces.new/MetaMask/metamask-extension/pull/25808?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26115?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26271?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26298?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26227?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26302?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26308?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26292?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26264?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26323?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26310?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26325?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26327?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26336?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26333?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26155?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26351?quickstart=1){kind=link}
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR will upgrade to use latest @metamask/eth-ledger-bridge-keyring library in metamask extensions.
Related issues
Fixes:
Manual testing steps
Area to test:
Screenshots/Recordings
There are no difference in UI after upgrade that library.
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist