MetaMask / MetaMask/metamask-mobile

Avoid recreating vault when changing password

Open
#13,429 0 comments 0 reactions 0 assignees View on GitHub
team-mobile-platform team-wallet-framework-deprecated
Dominant language
TypeScript
Stars
3k
Forks
1.7k
Avg merge
1d 14h
Merged PRs (30d)
669

Description

### What is this about?

The "change password" feature currently recreates the entire vault. Recreating the vault correctly is quite complex so this is an error-prone process, and the deletion of the old vault might result in state loss for related state in other controllers.

We should use the `changePassword` method instead, which was added to allow changing password without destroying the vault (https://github.com/MetaMask/core/pull/4279). This would let us eliminate some complex error-prone code, and avoid any risk of state loss from keyrings being deleted.

### Scenario

_No response_

### Design

_No response_

### Technical Details

Here is the relevant code: https://github.com/MetaMask/metamask-mobile/blob/6e9b68821a02c40cf9cbd3bb844d6168b72a03d1/app/core/Vault.js#L56

### Threat Modeling Framework

_No response_

### Acceptance Criteria

_No response_

### Stakeholder review needed before the work gets merged

- [x] Engineering (needed in most cases)
- [ ] Design
- [ ] Product
- [ ] QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
- [ ] Security
- [ ] Legal
- [ ] Marketing
- [ ] Management (please specify)
- [ ] Other (please specify)

### References

_No response_

Contributor guide

Open the contributing guide

Research direction

Begin at app/core/Vault.js around line 56 and compare the current password-change flow with core's changePassword method from PR #4279. Done means changing the password no longer recreates or deletes the vault, avoiding potential keyring state loss; verify the applicable Vault behavior afterward.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, react-native
Domain
mobile, security
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.