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.

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

[Bug]: QR accounts labels #9592

Open
plasmacorral opened this issue May 9, 2024 · 0 comments
Open

[Bug]: QR accounts labels #9592

plasmacorral opened this issue May 9, 2024 · 0 comments
Labels
area-hardware regression-RC DEPRECATED: Please use "regresssion-RC-x.y.z" label instead regression-RC-7.23.0 release-7.23.0 Issue or pull request that will be included in release 7.23.0 Sev3-low A possible confusion or deception that is only hypothetical & has no known instances in the wild team-accounts team-hardware-wallets type-bug Something isn't working

Comments

@plasmacorral
Copy link
Contributor

plasmacorral commented May 9, 2024

Describe the bug

Observed this today when feature QA testing chore: Update @metamask/keyring-controller to v16
#9570.

  • Was able to reproduce it on main, but not v7.22.0 build 1326.
  • Went back and checked a QA build from #9318 9508 and was able to observe this label difference, and I suspect that I missed this being introduced within that PR.

Prior to Keyring v13, new QR accounts were added as Keystone x where the x reflected the index position of the Keystone address, regardless of how many accounts were present in MetaMask. Now, QR addresses are being added with the label Account y where y-1 is how many other addresses are present in the wallet.

Devices observed:

  • Samsung a515f running Android 12 with security 1/1/2024
  • Pixel 5a running Android 14 with security Apr 5, 2024
  • iPhone 13 mini running iOS 15.6.1
  • iPhone Xs running iOS 17.4.1

Expected behavior

QR addresses should be added with a reference to their index position rather than the number of other accounts present.

While we are adjusting here, it may be worth getting product feedback on whether we want to continue to name all QR addresses as Keystone or whether we use a more accurate label like QR to reflect the fact that Airgap and any other hardware wallet supporting ERC4527 could be used here.

Screenshots/Recordings

Adresses in the green box were added prior to Keyring Controller v13, and the address in the red box was added after
Screenshot 2024-05-09 at 11 54 49 AM

Steps to reproduce

  1. Create a wallet
  2. Add QR hardware address
  3. Note the account label

Error messages or log output

No response

Version

main

Build type

None

Device

Several

Operating system

iOS, Android

Additional context

Observed on main and QA build from PR 9318/9508, but used the 7.23.0 and RC regression labels as I anticipate 9318 9508 being included in the next RC.

Severity

This could lead to user confusion and make it challenging to keep track of QR hardware addresses.

@plasmacorral plasmacorral added type-bug Something isn't working Sev3-low A possible confusion or deception that is only hypothetical & has no known instances in the wild area-hardware team-accounts regression-RC DEPRECATED: Please use "regresssion-RC-x.y.z" label instead team-hardware-wallets release-7.23.0 Issue or pull request that will be included in release 7.23.0 labels May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-hardware regression-RC DEPRECATED: Please use "regresssion-RC-x.y.z" label instead regression-RC-7.23.0 release-7.23.0 Issue or pull request that will be included in release 7.23.0 Sev3-low A possible confusion or deception that is only hypothetical & has no known instances in the wild team-accounts team-hardware-wallets type-bug Something isn't working
Projects
Status: To be fixed
Status: To be fixed
Development

No branches or pull requests

2 participants