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]: MM crashes during confirmations with some currencies #24558

Closed
bschorchit opened this issue May 16, 2024 · 0 comments · Fixed by #24563
Closed

[Bug]: MM crashes during confirmations with some currencies #24558

bschorchit opened this issue May 16, 2024 · 0 comments · Fixed by #24563
Assignees
@OGPoyraz
Labels
regression-prod-11.15.2 Regression bug that was found in production in release 11.15.2 release-11.18.0 Issue or pull request that will be included in release 11.18.0 Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-confirmations Push issues to confirmations team type-bug

Comments

@bschorchit
Copy link

bschorchit commented May 16, 2024 •
edited

Describe the bug

MM crashes if user selects a currency that is not supported by the token api and starts a transaction that has simulation.

Expected behavior

Not crashing and displaying "Not available" if we're not able to get a conversion.

Screenshots/Recordings

image

Steps to reproduce

  1. Go to Settings > General
  2. Change your currency to Storj
  3. Start a send transaction on Ethereum with any value
  4. Notice MM crash

The only way to recover from the crash is to disable and re-enable MM and changing the network before unlocking it otherwise you'll consistently unlock it and arrive at the crashed screen

Error messages or log output

No response

Version

11.15.2

Build type

None

Browser

Brave

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

Severity

No response
@bschorchit bschorchit added type-bug Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-confirmations Push issues to confirmations team labels May 16, 2024
@metamaskbot metamaskbot added the regression-prod-11.15.2 Regression bug that was found in production in release 11.15.2 label May 16, 2024
@OGPoyraz OGPoyraz mentioned this issue May 16, 2024
Merged
7 tasks
OGPoyraz added a commit that referenced this issue May 17, 2024
@OGPoyraz
fix: add fallback method for currency formatter in useFiatFormatter…
c897b49 
… hook (#24563)

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

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR goal is to add a fallback for unknown currency formats to render
it `${amount} ${currency}` format.

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

## **Related issues**

Fixes: #24558

## **Manual testing steps**

1. Go to Settings > General
2. Change your currency to Storj
3. Start a send transaction on Ethereum with any value
4. MM doesn't crash and shows `amount storj`

## **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 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.
@metamaskbot metamaskbot added the release-11.18.0 Issue or pull request that will be included in release 11.18.0 label May 17, 2024
@OGPoyraz OGPoyraz self-assigned this May 17, 2024
OGPoyraz added a commit that referenced this issue May 17, 2024
@OGPoyraz
fix: add fallback method for currency formatter in useFiatFormatter…
6398c02 
… hook (#24563)

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

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR goal is to add a fallback for unknown currency formats to render
it `${amount} ${currency}` format.

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

## **Related issues**

Fixes: #24558

## **Manual testing steps**

1. Go to Settings > General
2. Change your currency to Storj
3. Start a send transaction on Ethereum with any value
4. MM doesn't crash and shows `amount storj`

## **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 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.
@benjisclowder benjisclowder mentioned this issue May 27, 2024
Open
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@OGPoyraz OGPoyraz

Labels
regression-prod-11.15.2 Regression bug that was found in production in release 11.15.2 release-11.18.0 Issue or pull request that will be included in release 11.18.0 Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-confirmations Push issues to confirmations team type-bug
Projects
Bugs by severity
Archived in project
Bugs by team
Status: To be fixed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: add fallback method for currency formatter in useFiatFormatter hook MetaMask/metamask-extension
3 participants
@OGPoyraz @bschorchit @metamaskbot