Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bug]: Adding new account through dapp connection- account name inconsistent #25846

Closed
plasmacorral opened this issue Jul 16, 2024 · 1 comment · Fixed by #26542
Closed

[Bug]: Adding new account through dapp connection- account name inconsistent #25846

plasmacorral opened this issue Jul 16, 2024 · 1 comment · Fixed by #26542
Assignees
@montelaidev
Labels
regression-RC-12.0.0 regression-RC-12.1.0 release-12.0.0 Issue or pull request that will be included in release 12.0.0 release-12.2.0 Issue or pull request that will be included in release 12.2.0 release-12.4.0 Issue or pull request that will be included in release 12.4.0 release-blocker This bug is blocking the next release Sev2-normal Normal severity; minor loss of service or inconvenience. team-accounts type-bug

Comments

@plasmacorral
Copy link
Contributor

plasmacorral commented Jul 16, 2024 •
edited
Loading

Describe the bug

When using the create new account action within the dapp connection prompt, the account name is not presented consistently. In the video below, the same account is shown as Account 9 as well as Account 5 within the add new account flow within the dapp connection.

This was observed in RC testing for v12.1.0 and was confirmed to also be the case within v12.0.0 RC

Expected behavior

Account name should be presented consistently throughout the flow, with the name that will appear in the accounts list. Previously in 11.16.3 for example, a newly created HD account would be named Account count(all accounts)+1, but it looks like within this connection flow we are not consistent and show the user an account name of Account count(HD,Imported)+1.

Screenshots/Recordings

Recording

Steps to reproduce

  1. Set up a wallet using an imported Secret Recovery Phrase (SRP).
  2. Import a private key.
  3. Add multiple hardware wallet addresses.
  4. Open the test dapp.
  5. Verify no accounts are connected by tapping Eth_Accounts and noting the empty result.
  6. Tap Connect in the test dapp.
  7. In the Extension, tap New Account.
  8. Observe the account name, which should be the total number of accounts plus one (e.g., Account 9).
  9. Tap Save.
  10. Check the newly created account name in the list of accounts during the connection flow (e.g., Account 5).
  11. Tap Next.
  12. Tap Confirm on the permissions screen.
  13. Open the Extension.
  14. Tap the accounts drop-down menu.
  15. Verify the newly created account name (e.g., Account 9).

Error messages or log output

No response

Version

12.0.0

Build type

None

Browser

Chrome, Firefox

Operating system

MacOS

Hardware wallet

Ledger

Additional context

Observed with Ledger, but could be observed with any hardware wallets present.

Severity

User must have hardware present and use the add new account flow within dapp connection prompt to observe this.
@plasmacorral plasmacorral added type-bug Sev3-low Low severity; minimal to no impact upon users team-accounts release-12.0.0 Issue or pull request that will be included in release 12.0.0 regression-RC-12.0.0 regression-prod-12.0.0 Regression bug that was found in production in release 12.0.0 regression-RC-12.1.0 labels Jul 16, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Jul 16, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Jul 16, 2024
@vpintorico vpintorico removed the regression-prod-12.0.0 Regression bug that was found in production in release 12.0.0 label Jul 23, 2024
@danjm danjm added the release-blocker This bug is blocking the next release label Aug 14, 2024
@montelaidev montelaidev self-assigned this Aug 15, 2024
@montelaidev montelaidev mentioned this issue Aug 20, 2024
Merged
7 tasks
montelaidev added a commit that referenced this issue Aug 20, 2024
@montelaidev
fix: name being out of sync in account list (#26542)
6ed72f0 
## **Description**

This PR fixes the issue where the account name being created in the
connect account flow is out of sync.

<!--
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?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26542?quickstart=1)

## **Related issues**

Fixes:
[25846](#25846)

## **Manual testing steps**

1. Create a snap/hw account 
2. Go to test dapp
3. Click connect 
4. Click on create new account 
5. See that the suggested name is Account 3
6. Create the new account
7. See in the list that there is `Account 3` in the list

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**


https://github.com/user-attachments/assets/9c0d511a-a84a-4da8-80e5-4dbadbb7cee6

<!-- [screenshots/recordings] -->

### **After**


https://github.com/user-attachments/assets/daf22aed-593f-4e81-8535-11a26cd5e4fc

<!-- [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.
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team Aug 20, 2024
@metamaskbot metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Aug 20, 2024
@gantunesr gantunesr mentioned this issue Aug 20, 2024
Merged
@AlexJupiter AlexJupiter added Sev2-normal Normal severity; minor loss of service or inconvenience. and removed Sev3-low Low severity; minimal to no impact upon users labels Aug 22, 2024
@metamaskbot metamaskbot added the release-12.2.0 Issue or pull request that will be included in release 12.2.0 label Aug 28, 2024
@metamaskbot
Copy link
Collaborator

Missing release label release-12.2.0 on issue. Adding release label release-12.2.0 on issue, as issue is linked to PR #26542 which has this release label.

@gauthierpetetin gauthierpetetin added release-12.4.0 Issue or pull request that will be included in release 12.4.0 and removed release-12.5.0 Issue or pull request that will be included in release 12.5.0 labels Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@montelaidev montelaidev

Labels
regression-RC-12.0.0 regression-RC-12.1.0 release-12.0.0 Issue or pull request that will be included in release 12.0.0 release-12.2.0 Issue or pull request that will be included in release 12.2.0 release-12.4.0 Issue or pull request that will be included in release 12.4.0 release-blocker This bug is blocking the next release Sev2-normal Normal severity; minor loss of service or inconvenience. team-accounts type-bug
Projects
Bugs by severity
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: name being out of sync in account list MetaMask/metamask-extension
7 participants
@vpintorico @gauthierpetetin @danjm @AlexJupiter @plasmacorral @metamaskbot @montelaidev