-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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]: Duplicate Account Issue in MetaMask 7.23.0 on iPhone 14 #9823
Comments
Hey @fricklik, it would be really helpful if you can post a video reproducing this issue. Edit: A quick question, from which version did you updated from? |
Users state logs (2 reports kept private) are showing the duplicated accounts have a checksummed address for the original one (the properly named one) and the same address but lowercased for the duplicate. Redacted example"accounts": {
"XXXXXXXX-XXXX-XXXX-bXXX-XXXXXXXXXX": {
"address": "0x0...AD",
"id": "XXXXXXXX-XXXX-XXXX-bXXX-XXXXXXXXXX",
"options": {},
"metadata": {
"name": "xxxxxx.eth",
"keyring": { "type": "HD Key Tree" },
"lastSelected": 1234567890123
},
"methods": [
"personal_sign",
"eth_sign",
"eth_signTransaction",
"eth_signTypedData_v1",
"eth_signTypedData_v3",
"eth_signTypedData_v4"
],
"type": "eip155:eoa"
},
"XXXXXXXX-XXXX-XXXX-bXXX-XXXXXXXXXX": {
"id": "XXXXXXXX-XXXX-XXXX-bXXX-XXXXXXXXXX",
"address": "0x0...ad",
"options": {},
"methods": [
"personal_sign",
"eth_sign",
"eth_signTransaction",
"eth_signTypedData_v1",
"eth_signTypedData_v3",
"eth_signTypedData_v4"
],
"type": "eip155:eoa",
"metadata": {
"name": "Account 7",
"keyring": { "type": "HD Key Tree" },
"lastSelected": 1234567890123
}
}, Notice |
I think it's a migration issue. Mobile uses checksummed addresses in the PreferencesController, but the extension uses lowercase addresses. When the AccountsController adds or removes accounts from the KeyringController, the addresses are all in lowercase. The checksummed addresses create different account IDs during the migration, and this also causes the AccountsController to create duplicate accounts since it thinks it's a new account. |
I have this same issue on Galaxy Note20 Ultra 5G |
<!-- 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** This PR addresses the issue of duplicate accounts showing up in the wallet. This bug was introduced when [the accounts controller was integrated](#8759) but was only visible to users in [this PR](#9733) when we started rendering account information in the accounts list. The is that when we ran migration `036.ts`, we simply added the all of the identities from the preferences controller into the AccountsController. However the accounts controller also consumes addresses from the KeyringController and those addresses are in lowercase. The result would be two accounts with the same address, one in checksum format and one in lowercase format. This would result in duplicates when rendering the UI. This PR addresses this issue in two ways... 1. for users who are using <= v7.19.0 - I have patched migration 36 to ensure that the addresses that are added are lowercase. 2. For users who have are using version >= 7.20.0 and are already effected by this bug - I have created a new migration to deduplicate the accounts in the accounts controller and ensure that we are only storing lowercase addresses in state. ## **Related issues** Fixes: #9823 ## QA Builds - [Current branch](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c68d80ea-dbaa-4e81-9018-fe9757cab397) - [7.18.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/011ac0b3-b1d8-4e53-9595-ea0f739d4d10) - [7.24.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a5c54c50-8382-45ad-b203-687b66273c4e) ## **Manual testing steps** ### Path 1: User has duplicate accounts already 1. Install [7.18.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/011ac0b3-b1d8-4e53-9595-ea0f739d4d10) and import a wallet with multiple addresses already 2. Install [7.24.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a5c54c50-8382-45ad-b203-687b66273c4e) and upgrade app. 3. EXPECTED: You should see duplicate accounts when you open the accounts list. If you had 5 accounts previously there will be should now be 10. 4. Install [the current branch QA build](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c68d80ea-dbaa-4e81-9018-fe9757cab397) and upgrade. 5. EXPECT: The duplicate accounts should be removed (back to 5 accounts from 10) 6. EXPECT: The previously selected address should still be selected 7. EXPECT: The account names should be in tact and in chronological order if they are the default account name. ### Path 2: User has not seen duplicate accounts yet this occurs for users who have not upgraded from 7.19.0 yet 1. Install [7.18.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/011ac0b3-b1d8-4e53-9595-ea0f739d4d10) and import a wallet with multiple addresses already 2. Install [the current branch QA build](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c68d80ea-dbaa-4e81-9018-fe9757cab397) and upgrade. 3. EXPECT: All of the account should appear in the accounts list without duplicates 4. EXPECT: The previously selected address should be the same 5. EXPECT: The account names should be the same ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** 7.18.0 <img src="https://github.com/MetaMask/metamask-mobile/assets/22918444/3ee9b1f3-f042-414b-9ce4-82cad7d791cf" width="200" height="400"> After Upgrading to 7.24.0 <img src="https://github.com/MetaMask/metamask-mobile/assets/22918444/47b5ff3d-d2d0-4373-b8a0-ae0007b19b7b" width="200" height="400"> ### **After** Upgrading to this branch <img src="https://github.com/MetaMask/metamask-mobile/assets/22918444/9ed8d80a-ef3c-40a6-9152-e661a1ecdd25" width="200" height="400"> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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 - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.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). - [ ] 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. --> This PR addresses the issue of duplicate accounts showing up in the wallet. This bug was introduced when [the accounts controller was integrated](#8759) but was only visible to users in [this PR](#9733) when we started rendering account information in the accounts list. The is that when we ran migration `036.ts`, we simply added the all of the identities from the preferences controller into the AccountsController. However the accounts controller also consumes addresses from the KeyringController and those addresses are in lowercase. The result would be two accounts with the same address, one in checksum format and one in lowercase format. This would result in duplicates when rendering the UI. This PR addresses this issue in two ways... 1. for users who are using <= v7.19.0 - I have patched migration 36 to ensure that the addresses that are added are lowercase. 2. For users who have are using version >= 7.20.0 and are already effected by this bug - I have created a new migration to deduplicate the accounts in the accounts controller and ensure that we are only storing lowercase addresses in state. Fixes: #9823 - [Current branch](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c68d80ea-dbaa-4e81-9018-fe9757cab397) - [7.18.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/011ac0b3-b1d8-4e53-9595-ea0f739d4d10) - [7.24.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a5c54c50-8382-45ad-b203-687b66273c4e) 1. Install [7.18.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/011ac0b3-b1d8-4e53-9595-ea0f739d4d10) and import a wallet with multiple addresses already 2. Install [7.24.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a5c54c50-8382-45ad-b203-687b66273c4e) and upgrade app. 3. EXPECTED: You should see duplicate accounts when you open the accounts list. If you had 5 accounts previously there will be should now be 10. 4. Install [the current branch QA build](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c68d80ea-dbaa-4e81-9018-fe9757cab397) and upgrade. 5. EXPECT: The duplicate accounts should be removed (back to 5 accounts from 10) 6. EXPECT: The previously selected address should still be selected 7. EXPECT: The account names should be in tact and in chronological order if they are the default account name. this occurs for users who have not upgraded from 7.19.0 yet 1. Install [7.18.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/011ac0b3-b1d8-4e53-9595-ea0f739d4d10) and import a wallet with multiple addresses already 2. Install [the current branch QA build](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c68d80ea-dbaa-4e81-9018-fe9757cab397) and upgrade. 3. EXPECT: All of the account should appear in the accounts list without duplicates 4. EXPECT: The previously selected address should be the same 5. EXPECT: The account names should be the same <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> 7.18.0 <img src="https://github.com/MetaMask/metamask-mobile/assets/22918444/3ee9b1f3-f042-414b-9ce4-82cad7d791cf" width="200" height="400"> After Upgrading to 7.24.0 <img src="https://github.com/MetaMask/metamask-mobile/assets/22918444/47b5ff3d-d2d0-4373-b8a0-ae0007b19b7b" width="200" height="400"> Upgrading to this branch <img src="https://github.com/MetaMask/metamask-mobile/assets/22918444/9ed8d80a-ef3c-40a6-9152-e661a1ecdd25" width="200" height="400"> - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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 - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] 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. --> This PR addresses the issue of duplicate accounts showing up in the wallet. This bug was introduced when [the accounts controller was integrated](#8759) but was only visible to users in [this PR](#9733) when we started rendering account information in the accounts list. The is that when we ran migration `036.ts`, we simply added the all of the identities from the preferences controller into the AccountsController. However the accounts controller also consumes addresses from the KeyringController and those addresses are in lowercase. The result would be two accounts with the same address, one in checksum format and one in lowercase format. This would result in duplicates when rendering the UI. This PR addresses this issue in two ways... 1. for users who are using <= v7.19.0 - I have patched migration 36 to ensure that the addresses that are added are lowercase. 2. For users who have are using version >= 7.20.0 and are already effected by this bug - I have created a new migration to deduplicate the accounts in the accounts controller and ensure that we are only storing lowercase addresses in state. Fixes: #9823 - [Current branch](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c68d80ea-dbaa-4e81-9018-fe9757cab397) - [7.18.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/011ac0b3-b1d8-4e53-9595-ea0f739d4d10) - [7.24.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a5c54c50-8382-45ad-b203-687b66273c4e) 1. Install [7.18.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/011ac0b3-b1d8-4e53-9595-ea0f739d4d10) and import a wallet with multiple addresses already 2. Install [7.24.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a5c54c50-8382-45ad-b203-687b66273c4e) and upgrade app. 3. EXPECTED: You should see duplicate accounts when you open the accounts list. If you had 5 accounts previously there will be should now be 10. 4. Install [the current branch QA build](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c68d80ea-dbaa-4e81-9018-fe9757cab397) and upgrade. 5. EXPECT: The duplicate accounts should be removed (back to 5 accounts from 10) 6. EXPECT: The previously selected address should still be selected 7. EXPECT: The account names should be in tact and in chronological order if they are the default account name. this occurs for users who have not upgraded from 7.19.0 yet 1. Install [7.18.0](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/011ac0b3-b1d8-4e53-9595-ea0f739d4d10) and import a wallet with multiple addresses already 2. Install [the current branch QA build](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c68d80ea-dbaa-4e81-9018-fe9757cab397) and upgrade. 3. EXPECT: All of the account should appear in the accounts list without duplicates 4. EXPECT: The previously selected address should be the same 5. EXPECT: The account names should be the same <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> 7.18.0 <img src="https://github.com/MetaMask/metamask-mobile/assets/22918444/3ee9b1f3-f042-414b-9ce4-82cad7d791cf" width="200" height="400"> After Upgrading to 7.24.0 <img src="https://github.com/MetaMask/metamask-mobile/assets/22918444/47b5ff3d-d2d0-4373-b8a0-ae0007b19b7b" width="200" height="400"> Upgrading to this branch <img src="https://github.com/MetaMask/metamask-mobile/assets/22918444/9ed8d80a-ef3c-40a6-9152-e661a1ecdd25" width="200" height="400"> - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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 - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] 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? --> ## **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 Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.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: sethkfman <10342624+sethkfman@users.noreply.github.com>
Missing release label release-7.24.3 on issue. Adding release label release-7.24.3 on issue, as issue is linked to PR #9943 which has this release label. |
Missing release label release-7.24.4 on issue. Adding release label release-7.24.4 on issue, as issue is linked to PR #9943 which has this release label. |
Describe the bug
After updating MetaMask to version 7.23.0 on my iPhone 14, the same account appears duplicated in the app. When attempting to open the duplicated account, it displays the original account with the same address. A friend with an iPhone 14 Pro has experienced the exact same issue.
Expected behavior
I expected to see only one instance of each account and be able to access each account individually without duplication.
Screenshots/Recordings
Attached is a screenshot showing the duplicated accounts. The original account is highlighted with a red
frame.
Steps to reproduce
Error messages or log output
Version
7.23.0
Build type
None
Device
iPhone 14, iPhone 14 Pro
Operating system
iOS
Additional context
This issue affects multiple users with similar devices and the same version of MetaMask.
Severity
No response
The text was updated successfully, but these errors were encountered: