Skip to content

Conversation

@DDDDDanica
Copy link
Contributor

@DDDDDanica DDDDDanica commented Apr 25, 2025

Description

Cherry-pick #32287

Bumping @metamask/keyring-controller to ^21.0.4

## [21.0.4]

### Fixed

- Ensure no duplicate accounts are persisted ([#5710](https://github.com/MetaMask/core/pull/5710))

Open in GitHub Codespaces

Related issues

Fixes: #32097

Manual testing steps

In order to test the fix, you first need a wallet bricked because of a duplicate account:

  • Checkout on the v12.16.0 release branch and build locally (a version with multi-srp support)
  • Import or create an SRP (I used child guilt hollow arrive average popular nasty soon summer like scheme diary pill country rapid)
  • Import an account that is part of the mnemonic (e.g. I used 0x80842b7e3cfb1118e86a427cdec418e3b4179ef5bbbfd71c02a76349831c8a8b which is the account at index 2 of the above SRP)
  • Add a new account on the main HD - note that you'll be able to add the account, but (1) the account is the same as the imported one (expectable) and (2) the newly added account is wrongly labeled as "imported"
  • Lock the wallet
  • Unlock it with the right password - you will see the reported error:
Error: KeyringController - The account you are trying to import is a duplicate
await in withLock		
submitPassword	@	metamask-controller.js:4343
await in submitPassword		
(anonymous)	@	createMetaRPCHandler.js:23

Testing the fix

  • Without removing the extension from the browser, switch to this PR branch and build again
  • Visit chrome://extensions/ and reload the extension
  • Try to login again, you should be able to access the wallet again

Screenshots/Recordings

Before

After

Pre-merge author checklist

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

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

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.

@socket-security
Copy link

socket-security bot commented Apr 25, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updated@​metamask/​keyring-controller@​21.0.3 ⏵ 21.0.4941007698 +1100

View full report

… `^21.0.4` (#32287)

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

<!--
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?
-->
Bumping `@metamask/keyring-controller` to `^21.0.4`
```markdown

- Ensure no duplicate accounts are persisted ([#5710](MetaMask/core#5710))
```

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

Fixes: #32097

a duplicate account:
- Checkout on the v12.16.0 release branch and build locally (a version
with multi-srp support)
- Import or create an SRP (I used `child guilt hollow arrive average
popular nasty soon summer like scheme diary pill country rapid`)
- Import an account that is part of the mnemonic (e.g. I used
`0x80842b7e3cfb1118e86a427cdec418e3b4179ef5bbbfd71c02a76349831c8a8b`
which is the account at index 2 of the above SRP)
- Add a new account on the main HD - note that you'll be able to add the
account, but (1) the account is the same as the imported one
(expectable) and (2) the newly added account is wrongly labeled as
"imported"
- Lock the wallet
- Unlock it with the right password - you will see the reported error:
```
Error: KeyringController - The account you are trying to import is a duplicate
await in withLock
submitPassword	@	metamask-controller.js:4343
await in submitPassword
(anonymous)	@	createMetaRPCHandler.js:23
```

- Without removing the extension from the browser, switch to this PR
branch and build again
- Visit [chrome://extensions/](chrome://extensions/) and reload the
extension
- Try to login again, you should be able to access the wallet again

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

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

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

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/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-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

- [ ] 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.
@DDDDDanica DDDDDanica force-pushed the cherry-pick-e887cc7 branch from ba01b1e to 133ca1d Compare April 25, 2025 13:43
mikesposito
mikesposito previously approved these changes Apr 25, 2025
Co-authored-by: Michele Esposito <34438276+mikesposito@users.noreply.github.com>
@DDDDDanica DDDDDanica merged commit b8f35aa into Version-v12.17.0 Apr 25, 2025
11 of 13 checks passed
@github-actions
Copy link
Contributor

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.

@DDDDDanica DDDDDanica deleted the cherry-pick-e887cc7 branch April 25, 2025 14:06
@github-actions github-actions bot locked and limited conversation to collaborators Apr 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants