-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
[5.3.0][FIX] Prevent crash when funds warning is pressed #4178
[5.3.0][FIX] Prevent crash when funds warning is pressed #4178
Conversation
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some custom networks have the ability to onramp. for example BSC, Polygon, Avalanche.
With that being said if i were to be on any of those custom networks (in my case i was on polygon) & i tap the warning message I am taken the faucet page
To reproduce:
- with an account with no funds add a custom network (for example BSC, Polygon, Avalanche.)
- go to https://metamask.github.io/test-dapp/
- connect your wallet
- tap on deploy contract, or create token
- now tap on the warning message
- notice the error
|
app/components/UI/TransactionReview/TransactionReviewInformation/index.js
Outdated
Show resolved
Hide resolved
@tommasini The app no longer crashes but we have an issue. The custom networks (BSC, Polygon, Avalanche, Celo and Fantom) were hard coded into the networks list: http://recordit.co/NfLmFZAph1 This should not be the case. Only mainnet & testnets should be hard coded in the network lists. To reproduce: Furthermore If I were to select any of the hard coded networks I get see here: http://recordit.co/vCFebnF6qM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🌮 🌮 🌮 🌮
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
The app is crashing when we do not have enough funds to do a transaction on testnet
Proposed Solution
The app will redirect to the MetaMask faucet when it's testnet
The app will redirect to onRamp with the crypto we are trying to transact
Code Impact
Medium, was changed in all transaction confirmations screens/modals:
Test Cases
(For mainnet, avax and rinkeby)
Case 1:
Case 2:
Case 3:
Screenshots/Recordings
Bug:
http://recordit.co/wpPgIUiPcG
Solution:
https://recordit.co/KR5KHCtYOV
Checklist
Issue
Progresses #4177