Skip to content

Version v12.0.2 RC#26357

Merged
danjm merged 7 commits intomasterfrom
Version-v12.0.2
Aug 13, 2024
Merged

Version v12.0.2 RC#26357
danjm merged 7 commits intomasterfrom
Version-v12.0.2

Conversation

@metamaskbot
Copy link
Copy Markdown
Collaborator

📦 🚀

@metamaskbot
Copy link
Copy Markdown
Collaborator Author

Builds ready [27f5cbc]
Page Load Metrics (61 ± 10 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7215789189
domContentLoaded9211231
load44135612010
domInteractive9211231

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.69%. Comparing base (b9e113b) to head (27f5cbc).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #26357   +/-   ##
=======================================
  Coverage   65.69%   65.69%           
=======================================
  Files        1370     1370           
  Lines       54705    54705           
  Branches    14235    14235           
=======================================
  Hits        35935    35935           
  Misses      18770    18770           

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

Cherry pick #26381 to
v12.0.2

Co-authored-by: Prithpal Sooriya <prithpal.sooriya@gmail.com>
@github-actions
Copy link
Copy Markdown
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.

@socket-security
Copy link
Copy Markdown

socket-security bot commented Aug 13, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/qs@6.11.0 None 0 229 kB ljharb

🚮 Removed packages: npm/qs@6.11.2

View full report↗︎

danjm and others added 2 commits August 13, 2024 18:21
…#26383)

We are seeing the following sorts of errors in production, as reported
by sentry, in v12.0.2:
`No metadata found for 'conversionDate'`
`No metadata found for 'usdConversionRate'`
`No metadata found for 'nativeCurrency'`
`No metadata found for 'conversionRate'`

Example issue:
https://metamask.sentry.io/issues/5682684113/events/b8006eebb65749f883e907242e52215b/?project=273505&query=is%3Aunresolved+issue.priority%3A%5Bhigh%2C+medium%5D+No+metadata+release%3A12.0.1&referrer=previous-event&statsPeriod=14d&stream_index=1

The `CurrencyRateController` stopped using six such properties with
MetaMask/core#1805, which was brought into the
extension with #21549, however, there was not a state migration to
delete those properties at the time

This PR adds migrations to delete those obsolete properties

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

Fixes: #26356

To observe the error using steps that mimic users in production
1. Install v11.6.0 and onboard
2. Create a local dev build from the `master` branch
3. Update the v11.6.0 install to the local dev build
4. See the errors in the service worker console

If you repeat those steps, but in step two build from this branch
instead of the `master` branch, the errors will not occur

For a faster manual test of this PR, create a local development build of
this branch and then run this script in the service worker console:
```
window.chrome.storage.local.get(({ data, meta }) => chrome.storage.local.set({ data: { ...data, CurrencyController: { ...data.CurrencyController, conversionDate: 'Jan 1', conversionRate: '2', nativeCurrency: 'test' } }, meta: {...meta, version: 120 } }, () => { chrome.runtime.reload() }))
```
There should be no errors like `No metadata found for 'conversionRate'`
(but if you do the same on develop or master, those errors should be
present)

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

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

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

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

---------

Co-authored-by: Mark Stacey <markjstacey@gmail.com>
…ntrol… (#26396)

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

## **Description**

Expands on #26383 to
delete more obsolete state from the Network and Phishing controllers, to
eliminate other sentry errors.

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

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

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

### **Before**

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

### **After**

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

## **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
- [ ] 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.

---------

Co-authored-by: Mark Stacey <markjstacey@gmail.com>
@metamaskbot
Copy link
Copy Markdown
Collaborator Author

Builds ready [84b0007]
Page Load Metrics (73 ± 26 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint673901076933
domContentLoaded9231342
load42295735526
domInteractive9231342

Gudahtt and others added 3 commits August 13, 2024 18:57
This is a cherry-pick of #26397. Original description:

## **Description**

We have found evidence that migration 88 is failing for some users due
to a `null` key in the `TokensController.allTokens` state. The migration
has been updated to delete any invalid `null`-keys prior to migrating
it, preventing the error.

[![Open in GitHub

Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26397?quickstart=1)

## **Related issues**

Fixes #25938

## **Manual testing steps**

The unit tests demonstrate the affected scenario fairly well. We
addressed the "null key" case for a variety of different parts of state,
but specifically the one we are seeing in prod is the `allTokens` state
having a `null` key.

## **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.
fix(cherry-pick): Add migration 120.4 to delete obsolete currency, phishing and network controller state
Updates the changelog for v12.0.2

---------

Co-authored-by: Mark Stacey <markjstacey@gmail.com>
@danjm danjm marked this pull request as ready for review August 13, 2024 22:59
@danjm danjm requested a review from a team as a code owner August 13, 2024 22:59
@danjm danjm merged commit cd841c8 into master Aug 13, 2024
@danjm danjm deleted the Version-v12.0.2 branch August 13, 2024 23:00
@github-actions github-actions bot locked and limited conversation to collaborators Aug 13, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator Author

Builds ready [4542105]
Page Load Metrics (140 ± 180 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint701078894
domContentLoaded9241131
load431778140376180
domInteractive9241131

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.

3 participants