Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bug]: "MetaMask migration error #88: Invalid Character" prevents metamask from loading #25938

Closed
danjm opened this issue Jul 18, 2024 · 1 comment · Fixed by #26397
Closed

[Bug]: "MetaMask migration error #88: Invalid Character" prevents metamask from loading #25938

danjm opened this issue Jul 18, 2024 · 1 comment · Fixed by #26397
Assignees
@Gudahtt
Labels
release-12.3.0 Issue or pull request that will be included in release 12.3.0 Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-extension-platform type-bug

Comments

@danjm
Copy link
Contributor

danjm commented Jul 18, 2024

Describe the bug

Users report MetaMask being stuck on infinite load. Two users reports can be found in a comment on another issue: #22533 (comment) and #22533 (comment)

The console log includes this error: "MetaMask migration error #88: Invalid Character"

The stack trace associated with the console logs show that the error comes from the parseBase method in the instance of bn.js imported from ethereumjs-util within the migration 88 file, in particular with the toHex function within that file.

Expected behavior

Migrations should complete successfully and not throw an error

Screenshots/Recordings

One user shared this:

chrome on win 10

image

image

Steps to reproduce

Currently unknown

Error messages or log output

No response

Detection stage

In production (default)

Version

11.16.14

Build type

None

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

Severity

No response
@danjm danjm added the type-bug label Jul 18, 2024
@danjm danjm mentioned this issue Jul 18, 2024
Open
@danjm danjm added the Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. label Jul 18, 2024
@metamaskbot metamaskbot added the regression-prod-11.16.14 Regression bug that was found in production in release 11.16.14 label Jul 18, 2024
@danjm danjm added team-extension-platform and removed regression-prod-11.16.14 Regression bug that was found in production in release 11.16.14 labels Jul 18, 2024
@NiranjanaBinoy NiranjanaBinoy self-assigned this Aug 1, 2024
@sentry-io Sentry.io
Copy link

sentry-io bot commented Aug 13, 2024

Sentry Issue: METAMASK-X69A

Gudahtt added a commit that referenced this issue Aug 13, 2024
@Gudahtt
fix: Fix invalid state causing migration 88 to fail
47a2415 
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.

Fixes #25938
@Gudahtt Gudahtt mentioned this issue Aug 13, 2024
Merged
7 tasks
Gudahtt added a commit that referenced this issue Aug 13, 2024
@Gudahtt
fix: Fix invalid state causing migration 88 to fail (#26397)
8901b68 
<!--
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**

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.
Gudahtt added a commit that referenced this issue Aug 13, 2024
@Gudahtt
fix: Fix invalid state causing migration 88 to fail (#26397)
434bd9c 
<!--
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**

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.
@Gudahtt Gudahtt mentioned this issue Aug 13, 2024
Merged
7 tasks
@metamaskbot metamaskbot added the release-12.4.0 Issue or pull request that will be included in release 12.4.0 label Aug 13, 2024
danjm pushed a commit that referenced this issue Aug 13, 2024
@Gudahtt
fix: Fix invalid state causing migration 88 to fail (#26397) (#26401)
ecfd64d 
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.
@NiranjanaBinoy NiranjanaBinoy removed their assignment Aug 14, 2024
@gauthierpetetin gauthierpetetin added release-12.3.0 Issue or pull request that will be included in release 12.3.0 and removed release-12.4.0 Issue or pull request that will be included in release 12.4.0 labels Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Gudahtt Gudahtt

Labels
release-12.3.0 Issue or pull request that will be included in release 12.3.0 Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-extension-platform type-bug
Projects
Bugs by severity
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Fix invalid state causing migration 88 to fail MetaMask/metamask-extension
5 participants
@Gudahtt @gauthierpetetin @danjm @metamaskbot @NiranjanaBinoy