Skip to content

Version v11.10.1#23196

Merged
danjm merged 6 commits intomasterfrom
Version-v11.10.1
Feb 27, 2024
Merged

Version v11.10.1#23196
danjm merged 6 commits intomasterfrom
Version-v11.10.1

Conversation

@danjm
Copy link
Copy Markdown
Contributor

@danjm danjm commented Feb 27, 2024

No description provided.

danjm and others added 3 commits February 27, 2024 08:00
…3194)

## **Description**

We are seeing an error in prod that could only be explained if a
migration fails. This PR adds some error handling to better detect and
monitor one possible source of migration failure.

The prod error is:
> TypeError getSelectedInternalAccount(ui/selectors/selectors.js)
> Cannot read properties of undefined (reading 'selectedAccount')

https://metamask.sentry.io/issues/4999905586/?project=273505&query=is%3Aunresolved+release%3A11.10.0+is%3Anew&referrer=issue-stream&statsPeriod=7d&stream_index=9

## **Manual testing steps**

Unit tests should pass. Also:

1. Build this branch and install metamask
2. During the onboarding flow, make sure to opt _in_ to metametrics
3. Run the following script in the background console
```
chrome.storage.local.get(({ data, meta }) => chrome.storage.local.set({ data: { ...data, PreferencesController: {selectedAddress: undefined} }, meta: {...meta, version: 104} }, () => { window.location.reload() }))
```
You should see a network request to sentry with
`state.PreferencesController?.selectedAddress is undefined` in the
payload

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained what problem this PR is solving and how it
is solved.
- [ ] I've linked related issues
- [ ] I've included manual testing steps
- [ ] I've included screenshots/recordings if applicable
- [ ] 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 properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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**

Fixes an issue where the image files for some networks were not present
in the filesystem, causing metamask to crash when trying to load them.
The affected chains were:

```
0x288 ./images/endurance-smart-chain.png
0x4d2 ./images/setp.svg
0x6a ./images/velas.svg
0x133e40 ./images/zkatana.svg
```

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

## **Related issues**

Fixes: #23084

## **Manual testing steps**

Add any of the 4 affected chains to metamask, and visit the settings ->
networks page. should not crash.

## **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**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained what problem this PR is solving and how it
is solved.
- [ ] I've linked related issues
- [ ] I've included manual testing steps
- [ ] I've included screenshots/recordings if applicable
- [ ] 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 properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
@danjm danjm requested a review from a team as a code owner February 27, 2024 11:33
@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.

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Feb 27, 2024
Disable ppom tests until cdn responses are mocked.
This unblocks ci.
It seems that the description response for the malicious mocked requests
has changed in a recent update.
This PR aims to fix the e2e tests.

This also reveals the strong need for using a stable version of the cdn,
by:
- Either using the cdn test version of ppom (stable), like we used to do
--> that was changed due to some issue Blockaid had on their end
- Or mocking the cdn responses and keep them always the same --> to
investigate if this is doable, since the responses that we get are in a
"weird format" (see example below)
![Screenshot from 2024-02-19
19-23-25](https://github.com/MetaMask/metamask-extension/assets/54408225/54589804-2c9f-40c7-8f67-2ff800f0b0be)

Another scenario to explore for some of these specs, would be to see if
we can inject the files directly in the ppomDB. In the same way we
initialize the wallet with a certain state, we could initialize the
wallet with the ppomDB prefilled in storage.

This PR however, aims to unblock ci in the fastes possible way

Fixes: #23053

1. Check ci or run them locally

![Screenshot from 2024-02-19
19-04-44](https://github.com/MetaMask/metamask-extension/assets/54408225/248ae58c-fb40-4531-8028-a3e218ed44b3)

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

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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.
- [x] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

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

Builds ready [8f5af66]
Page Load Metrics (1875 ± 82 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1171951422010
domContentLoaded11352384
load15982259187517182
domInteractive11352384

@danjm
Copy link
Copy Markdown
Contributor Author

danjm commented Feb 27, 2024

I have manually tested to verify that the change in 77c6808 works as expected

@danjm danjm merged commit e0ce296 into master Feb 27, 2024
@danjm danjm deleted the Version-v11.10.1 branch February 27, 2024 16:34
@github-actions github-actions bot locked and limited conversation to collaborators Feb 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

INVALID-PR-TEMPLATE PR's body doesn't match template release-11.10.1 Issue or pull request that will be included in release 11.10.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants