fix: cherry pick accounts controller migration improvements #26778
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR renames migration 126 to 121.1. The reason for this renaming is that commit 759b92e is being cherry-picked into version 12.1.1. Between version 12.1.1 and the develop branch, there are missing migrations. To avoid skipping these migrations in subsequent releases, we need to rename migration 126 to 121.1.
|
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. |
owencraston
changed the title
## **Description** Migration 121.1 has been updated to include more state validation, so that it does not throw an error in the event that state is corrupted in some unexpected way. Unexpected corrupted state is now reported to Sentry as well. [](https://codespaces.new/MetaMask/metamask-extension/pull/26773?quickstart=1) ## **Related issues** Related: #26377 ## **Manual testing steps** The unit tests outline the scenarios that the added validation are meant to cover. Probably not worth anyone's time to manually test those. To test the migration in general though, here are the steps: * Create a dev build from v12.1.0 * Install the dev build from the `dist/chrome` directory and proceed through onboarding * Run this command in the background console: ``` chrome.storage.local.get( null, (state) => { state.data.AccountsController.internalAccounts.selectedAccount = 'unknown id'; chrome.storage.local.set(state, () => chrome.runtime.reload()); } ); ``` * The extension should now be in a broken state * Note that you can use this same script to corrupt state in other ways to test other scenarios * Disable the extension * Switch to this branch and create a dev build * Enable and reload the extension * You should see in the console that migration 121.1 has run without error * You can run `chrome.storage.local.get(console.log)` to check that the AccountsController state is now valid * The extension should no longer be broken ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.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 - [x] 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-extension/blob/develop/.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.
owencraston
changed the title
Gudahtt
reviewed
Aug 29, 2024
Gudahtt
requested changes
Aug 29, 2024
Gudahtt
approved these changes
Aug 29, 2024
chloeYue
approved these changes
Aug 30, 2024
](https://codespaces.new/MetaMask/metamask-extension/pull/26773?quickstart=1){kind=link}
Builds ready [11b2115]
Page Load Metrics (146 ± 152 ms)
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR cherry picks two improvements to the latest accounts controller migrations into the latest release.
Related issues
Fixes: #26377
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist