Skip to content

cherry-pick: Fix migration 120.3 error caused by invalid state (#26485)#26517

Merged
Gudahtt merged 1 commit intoVersion-v12.0.6from
cherry-pick/fix-migration-120.3
Aug 21, 2024
Merged

cherry-pick: Fix migration 120.3 error caused by invalid state (#26485)#26517
Gudahtt merged 1 commit intoVersion-v12.0.6from
cherry-pick/fix-migration-120.3

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Aug 19, 2024

This cherry-picks #26485 into v12.0.6. Original description:

Description

For a small number of users, migration 120.3 is failing due to invalid TransactionController.transactions state. We aren't sure yet how this is happening, but we can resolve the problem by updating the migration to delete this invalid state if we detect it. No other state relies on this state, and if it's invalid it won't work properly anyway.

The migration has been renamed from 120.3 to 120.6 so that it will be re-run for any users who encountered this migration on v12.0.1 already.

Open in GitHub Codespaces

Related issues

Fixes #26423

Manual testing steps

  • Create a dev build from v12.0.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.TransactionController.transactions = {}; chrome.storage.local.set(state, () => chrome.runtime.reload()); } );
  • Disable the extension
  • Switch to v12.0.1 and create a dev build
  • Enable and reload the extension
    • You should see in the console that migration 120.3 has failed
  • 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 120.6 has run without error
  • You can run chrome.storage.local.get(console.log) to check that the transactions state has been removed.

Screenshots/Recordings

N/A

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.

For a small number of users, migration 120.3 is failing due to invalid
`TransactionController.transactions` state. We aren't sure yet how this
is happening, but we can resolve the problem by updating the migration
to delete this invalid state if we detect it. No other state relies on
this state, and if it's invalid it won't work properly anyway.

The migration has been renamed from 120.3 to 120.6 so that it will be
re-run for any users who encountered this migration on v12.0.1 already.

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

Fixes #26423

* Create a dev build from v12.0.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.TransactionController.transactions = {};
      chrome.storage.local.set(state, () => chrome.runtime.reload());
    }
  );
  ```
* Disable the extension
* Switch to v12.0.1 and create a dev build
* Enable and reload the extension
  * You should see in the console that migration 120.3 has failed
* 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 120.6 has run without
error
* You can run `chrome.storage.local.get(console.log)` to check that the
transactions state has been removed.

N/A

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

- [ ] 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.
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.75%. Comparing base (6188c5c) to head (3350494).
Report is 2 commits behind head on Version-v12.0.6.

Additional details and impacted files
@@               Coverage Diff                @@
##           Version-v12.0.6   #26517   +/-   ##
================================================
  Coverage            65.75%   65.75%           
================================================
  Files                 1372     1372           
  Lines                54766    54767    +1     
  Branches             14260    14260           
================================================
+ Hits                 36006    36007    +1     
  Misses               18760    18760           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Gudahtt Gudahtt marked this pull request as ready for review August 19, 2024 17:45
@Gudahtt Gudahtt requested a review from a team as a code owner August 19, 2024 17:45
@Gudahtt Gudahtt added team-extension-platform Extension Platform team needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Aug 19, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [3350494]
Page Load Metrics (67 ± 9 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint761601032612
domContentLoaded10351684
load4510667189
domInteractive10351684

Copy link
Copy Markdown
Contributor

@desi desi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Gudahtt Gudahtt merged commit 4e45b22 into Version-v12.0.6 Aug 21, 2024
@Gudahtt Gudahtt deleted the cherry-pick/fix-migration-120.3 branch August 21, 2024 18:29
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Aug 21, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

team-extension-platform Extension Platform team

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants