Use account identities as default petnames #21956
Merged
Conversation
This file contains hidden or 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
|
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. |
dbrans
commented
Nov 27, 2023
dbrans
commented
Dec 4, 2023
dbrans
added
the
team-confirmations-system-deprecated
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #21956 +/- ##
===========================================
+ Coverage 68.05% 68.20% +0.15%
===========================================
Files 1087 1090 +3
Lines 42798 42719 -79
Branches 11372 11374 +2
===========================================
+ Hits 29122 29133 +11
+ Misses 13676 13586 -90 ☔ View full report in Codecov by Sentry. |
Builds ready [a640669]
Page Load Metrics (1166 ± 49 ms)
Bundle size diffs
|
Builds ready [a18c044]
Page Load Metrics (1194 ± 78 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
Changing account name works as expected: rename.movExisting petnames picked up after migration: migration.mov |
Builds ready [165de83]
Page Load Metrics (1315 ± 54 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
## **Description** This PR implements the "You're sending to a contract" acknowledgement in the new send flow. This existed in the old send flow. ## **Related issues** N/A ## **Manual testing steps** 1. Go to the new send page 2. As the recipient, paste: `0x514910771af9ca656af840dff83e8264ecf986ca` ; this is the $LINK token address 3. See the warning, note that you cannot submit the send until acknowledgement ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** N/A ### **After** <img width="415" alt="SCR-20240116-mthm" src="https://github.com/MetaMask/metamask-extension/assets/46655/87955c6d-262b-4c60-82c6-71a295f42e41"> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained what problem this PR is solving and how it is solved. - [ ] I've linked related issues - [ ] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] 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. - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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.
#21038) ## **Description** This pr adds more user friendly error messages instead of just error codes when a user encounters ledger connection issues. Resolves #17606 ## **Manual testing steps** 1. Go to Add hardware wallet 2. On the ledger device, unlock it. 3. On the `Connect a hardware wallet` page, Click on Ledger and then continue 4. Do not click connect when the webhid connection window pops up. 5. On the ledger device, lock the device 6. On the browser, click on the nano and then continue 7. See the new error ## **Screenshots/Recordings** ### **Before**  ### **After**  ## **Related issues** Resolves #17606 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained: - [x] What problem this PR is solving. - [x] How this problem was solved. - [x] How reviewers can test my changes. - [x] I’ve indicated what issue this PR is linked to: Fixes #??? - [x] I’ve included tests if applicable. - [x] I’ve documented any added code. - [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)). - [x] I’ve properly set the pull request status: - [x] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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: Sébastien Van Eyck <[email protected]>
## **Description** The package `@metamask/controller-utils` has been upgraded to `v8.0.1`. The breaking changes did not affect any of the imports used by the extension. See here for the changelog: https://github.com/MetaMask/core/blob/main/packages/controller-utils/CHANGELOG.md#801 ## **Related issues** N/A ## **Manual testing steps** N/A ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [x] I've included screenshots/recordings if applicable - [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. - [x] I’ve properly set the pull request status: - [x] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: MetaMask Bot <[email protected]> Co-authored-by: legobeat <[email protected]>
## **Description** Hello, this is the imToken team. imToken is a mobile crypto wallet founded in 2016. It has been operating safely and steadily for 7 years. Recently, we successfully supported ERC-4527, and imToken can function as a QR code-based signer now. This integration seamlessly aligns with Metamask's bidirectional QR account feature, enhancing the ability to track accounts whose private keys are stored on external devices. Therefore, we propose that Metamask expand its supported QR code-based wallet connection methods and include imToken. This will not only enhance Metamask's functionality and user coverage, but also provide users with a wider, more secure and efficient choice of wallets. We have successfully completed the integration of ERC-4527 and look forward to discussing in depth the possibility of adding imToken into Metamask's QR code-based wallet connection methods. Thank you very much for your attention and feedback on this matter. ## **Related issues** no ## **Manual testing steps** 1. Please check if the added links to the imToken official website and tutorials are correct. 2. The process is consistent with the current QR-Based Keystone and Airgap methods. Please refer to the test video provided below for reference. ### Video [Dropbox link](https://www.dropbox.com/scl/fi/kbfcthouwgqaaueepsslc/imToken_Connect_Metamask_.mp4?rlkey=vz6js9g81oevg2y77vkizfefq&dl=0) ### User Flow  ## **Screenshots/Recordings** ### **Before** <img width="571" alt="image" src="https://github.com/MetaMask/metamask-extension/assets/7024451/e67a1c52-4c1a-46eb-8140-8df9acc8556d"> ### **After** <img width="346" alt="image" src="https://github.com/MetaMask/metamask-extension/assets/7024451/3246af50-7187-49a4-b7f5-a84eaf1874f7"> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [ ] I've linked related issues - [x] I've included manual testing steps - [x] I've included screenshots/recordings if applicable - [ x] I’ve included tests if applicable - [ ] 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. - [x] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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 <[email protected]> Co-authored-by: Sébastien Van Eyck <[email protected]>
## **Description** Currently we send ppom validation requests only for incoming requests, this PR adds ppom validation for send requests from wallets too. ## **Related issues** Fixes: [#1657](MetaMask/MetaMask-planning#1657) ## **Manual testing steps** 1. Build and launch MM 2. Send ETH on mainnet to this address `0x5FbDB2315678afecb367f032d93F642f64180aa3` 3. See that Blockaid banner is not shown 4. Checkout this branch, build and launch MM 5. Send ETH on mainnet to the address `0x5FbDB2315678afecb367f032d93F642f64180aa3` 6. See that Blockaid banner is shown. ## **Screenshots/Recordings** ### **Before** https://github.com/MetaMask/metamask-extension/assets/44811/9c81e157-1d78-4609-a037-7a8897309d12 ### **After** https://github.com/MetaMask/metamask-extension/assets/44811/8b2ec828-9636-4302-a943-b023caaba730 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [x] I've included screenshots/recordings if applicable - [ ] 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. - [x] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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** hardcode a mapping of chainIDs to currency symbol, once the svg are uploaded we mapped the images to the network name and chainIDs **Acceptance criteria** When the network list is displayed within the Select a network modal, or custom network list, or any similar context, the hardcoded network image avatars should be displayed. This will reduce the often appearing "?" that appears when the network avatar is not present. ## **Related issues** Fixes: [#1813](MetaMask/MetaMask-planning#1813) ## **Manual testing steps** 1. Go to the network modal 2. Add a new network displayed on this [list](https://docs.google.com/spreadsheets/d/16G3SRiQvWXK6hp5duob0aopeKyx2HOAQpz152Fy8VNQ/edit#gid=2080069111) 3. check if the image is displayed ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** https://github.com/MetaMask/metamask-extension/assets/26223211/b109668a-9bc2-432b-b603-e90e9c85804f ### **After** https://github.com/MetaMask/metamask-extension/assets/26223211/aae091f7-a89c-4653-a996-223ec288fd31 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [x] I've included screenshots/recordings if applicable - [x] I’ve included tests if applicable - [ ] 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. - [x] I’ve properly set the pull request status: - [x] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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.
…#21553) ## **Description** Currently, MetaMask faces tight coupling between its UI and the keyring-controller. The UI heavily relies on the keyring-controller's state while also amalgamating information from various sources, such as preferences and balances. However, this approach presents some challenges. The keyring-controller must be aware of the UI's limitations, like the need for instant account list provision to avoid lag. Moreover, it takes on the responsibility of adding metadata to accounts, such as the associated keyring type, required for displaying account details. To address these issues, the introduction of the accounts-controller comes into play as a new abstraction layer between the UI and the keyring-controller. This separation of responsibilities allows the keyring-controller to focus on two main tasks: - Routing requests to the appropriate keyring. - Persisting the state of the keyrings. This PR replaces the usage of `getSelectedAddress` with `getSelectedInternalAccount` Depends on: #21554 ## **Related issues** Resolves https://github.com/MetaMask/accounts-planning/issues/135 ## **Manual testing steps** 1. Use a hardware wallet to test all the flows ## **Screenshots/Recordings** No UI/UX changes ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained: - [x] What problem this PR is solving. - [x] How this problem was solved. - [x] How reviewers can test my changes. - [x] I’ve indicated what issue this PR is linked to: Fixes #??? - [x] I’ve included tests if applicable. - [x] I’ve documented any added code. - [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)). - [x] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: MetaMask Bot <[email protected]> Co-authored-by: gantunesr <[email protected]> Co-authored-by: Gustavo Antunes <[email protected]> Co-authored-by: Howard Braham <[email protected]> Co-authored-by: chloeYue <[email protected]> Co-authored-by: Harsh Shukla <[email protected]> Co-authored-by: Olusegun Akintayo <[email protected]> Co-authored-by: Ariella Vu <[email protected]> Co-authored-by: David Walsh <[email protected]> Co-authored-by: Sébastien Van Eyck <[email protected]> Co-authored-by: Mark Stacey <[email protected]> Co-authored-by: legobeat <[email protected]> Co-authored-by: Mako Shan <[email protected]> Co-authored-by: Sébastien Van Eyck <[email protected]>
## **Description** We found some issues at Trezor with the Metamask integration, one of them related to the USB permissions page being linked to a deprecated version of our SDK. Metamask already uses the v9 package, but in this particular instance it's hardcoded to load from v5. We added a workaround on our side for now, by adding a redirect. This PR solves the problem properly by updating the version in the code. # **Related issues** Unknown ## **Manual testing steps** NA ## **Screenshots/Recordings** NA Co-authored-by: Brad Decker <[email protected]> Co-authored-by: legobeat <[email protected]> Co-authored-by: Sébastien Van Eyck <[email protected]>
## **Description** When investigating flaky tests, I found that some tests are flaky because the input field was not cleared before entering a new value. The issue is that Selenium sometimes doesn't clear input fields effectively. Now, we use sendkeys() to clear input fields, i've added another way to use selenium actions to simulate we select all text with keyboard and replace it with the new value. With both ways, we can clear input fields more effectively, which I believe will stabilize multiple flaky tests. Examples: Failing test because input field was not cleared as expected:  Tests failing because although the input field was cleared, the application automatically added a '0'. So when we entered the input value '4000', it was actually entered as '04000'. With my approach, when we find that the input field is not empty, it will select the '0' and replace it with '4000'. This way, we finally get the correct value of '4000'. <img width="363" alt="Screenshot 2024-01-24 at 14 44 04" src="https://github.com/MetaMask/metamask-extension/assets/105063779/98d6d811-46df-4d15-be50-7f18fe13fdf7"> I also fixed another flaky test in this PR. ## **Related issues** Fixes: #22573 ## **Manual testing steps** Run the e2e test.
Builds ready [f38a919]
Page Load Metrics (786 ± 28 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
matthewwalsh0
approved these changes
Jan 26, 2024
Builds ready [30c30b4]
Page Load Metrics (832 ± 30 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
OGPoyraz
approved these changes
Jan 29, 2024
metamaskbot
added
the
release-11.11.0
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
16 participants
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.
Description
Listens for changes to account identities and sets those as petnames.
Related issues
Part of: https://github.com/MetaMask/MetaMask-planning/issues/1624 (https://github.com/orgs/MetaMask/projects/47/views/1?pane=issue&itemId=43756188)
Follow-up: migration to grab any existing account identities.
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist