Was able to reproduce it on main, but not v7.22.0 build 1326.
Went back and checked a QA build from #93189508 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
Steps to reproduce
Create a wallet
Add QR hardware address
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.
This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 7 days. Thank you for your contributions.
Describe the bug
Observed this today when feature QA testing chore: Update @metamask/keyring-controller to v16
9570.
93189508 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 labelAccount y
where y-1 is how many other addresses are present in the wallet.Devices observed:
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 likeQR
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
Steps to reproduce
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
93189508 being included in the next RC.Severity
This could lead to user confusion and make it challenging to keep track of QR hardware addresses.