Skip to content

fix: Delete invalid SelectedNetworkController state (#26428)#26432

Merged
Gudahtt merged 1 commit intoVersion-v12.0.4from
cherry-pick/delete-invalid-ids-from-selected-network-controller-state
Aug 14, 2024
Merged

fix: Delete invalid SelectedNetworkController state (#26428)#26432
Gudahtt merged 1 commit intoVersion-v12.0.4from
cherry-pick/delete-invalid-ids-from-selected-network-controller-state

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Aug 14, 2024

Cherry-pick of #26428 for v12.0.4. Original description:

Description

The SelectedNetworkController state is cleared if any invalid networkConfigurationIds are found in state. We are seeing reports of this happening in production in v12.0.1.

The suspected cause is NetworkController state corruption. We resolved a few cases of this in v12.0.1, but for users that were affected by this, the invalid IDs may have propogated to the
SelectedNetworkController state already. That is what this migration intends to fix.

Open in GitHub Codespaces

Related issues

Fixes #26309

Manual testing steps

We don't have clear reproduction steps for the bug itself. To artificially reproduce the scenario by changing extension state, this should work:

  • Create a dev build from v12.0.2
  • Install the dev build from the dist/chrome directory and proceed through onboarding
  • Visit the test dapp, refresh, and connect to it.
  • Run this command in the background console:

state.data.SelectedNetworkController.domains['https://metamask.github.io'] = '123';
      chrome.storage.local.set(state, () => chrome.runtime.reload());
    }
  );
  • The linked error should now appear in the console of the popup
  • Disable the extension
  • Switch to this branch and create a dev build
  • Enable and reload the extension
    • The error should no longer appear.

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.

## **Description**

The `SelectedNetworkController` state is cleared if any invalid
`networkConfigurationId`s are found in state. We are seeing reports of
this happening in production in v12.0.1.

The suspected cause is `NetworkController` state corruption. We resolved
a few cases of this in v12.0.1, but for users that were affected by
this, the invalid IDs may have propogated to the
`SelectedNetworkController` state already. That is what this migration
intends to fix.

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

## **Related issues**

Fixes #26309

## **Manual testing steps**

We don't have clear reproduction steps for the bug itself. To
artificially reproduce the scenario by changing extension state, this
should work:

* Create a dev build from v12.0.2
* Install the dev build from the `dist/chrome` directory and proceed
through onboarding
* Visit the test dapp, refresh, and connect to it.
* Run this command in the background console:
  ```
  chrome.storage.local.get(
    null,
    (state) => {

state.data.SelectedNetworkController.domains['https://metamask.github.io']
= '123';
      chrome.storage.local.set(state, () => chrome.runtime.reload());
    }
  );
  ```
  * The linked error should now appear in the console of the popup
* Disable the extension
* Switch to this branch and create a dev build
* Enable and reload the extension
  * The error should no longer appear.

## **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.
@Gudahtt Gudahtt force-pushed the cherry-pick/delete-invalid-ids-from-selected-network-controller-state branch from a38cb00 to c211be3 Compare August 14, 2024 22:03
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.75%. Comparing base (95f9248) to head (c211be3).

Additional details and impacted files
@@                 Coverage Diff                 @@
##           Version-v12.0.4   #26432      +/-   ##
===================================================
+ Coverage            65.72%   65.75%   +0.03%     
===================================================
  Files                 1371     1372       +1     
  Lines                54737    54782      +45     
  Branches             14251    14262      +11     
===================================================
+ Hits                 35975    36020      +45     
  Misses               18762    18762              

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

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [c211be3]
Page Load Metrics (46 ± 2 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint66877863
domContentLoaded9121010
load41594652
domInteractive9121010

@Gudahtt Gudahtt marked this pull request as ready for review August 14, 2024 23:49
@Gudahtt Gudahtt requested a review from a team as a code owner August 14, 2024 23:49
@Gudahtt Gudahtt merged commit dcb420e into Version-v12.0.4 Aug 14, 2024
@Gudahtt Gudahtt deleted the cherry-pick/delete-invalid-ids-from-selected-network-controller-state branch August 14, 2024 23:50
@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants