-
Notifications
You must be signed in to change notification settings - Fork 5k
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
NewUI Estimating high gas price #4615
Comments
This was referenced Jun 25, 2018
ghost
assigned danjm
Jun 28, 2018
ghost
added
the
in progress
label
Jun 28, 2018
ghost
removed
the
in progress
label
Jul 5, 2018
bergeron
added a commit
that referenced
this issue
Aug 16, 2024
<!-- 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** Patches MetaMask/core#4615 When the price API does not support the native currency (e.g. MATIC), prices are fetched in ETH and an additional conversion hop is performed to the native currency. But this hop was only being performed on the current price, and was missing on the extended market data like marketCap, allTimeHigh, etc. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26460?quickstart=1) ## **Related issues** ## **Manual testing steps** 1. Open the token details page for a polygon token 2. Verify market cap, all time low, all time high are correct ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <img width="357" alt="Screenshot 2024-08-15 at 6 08 43 PM" src="https://github.com/user-attachments/assets/073e2a74-8258-4206-af1d-664eb3d4c1bb"> <img width="353" alt="Screenshot 2024-08-15 at 6 22 15 PM" src="https://github.com/user-attachments/assets/d5603a56-9fea-401a-a7cf-a08654792c17"> ### **After** <img width="351" alt="Screenshot 2024-08-15 at 6 12 46 PM" src="https://github.com/user-attachments/assets/13498481-8fe7-4f6d-852d-8d45508769dd"> <img width="351" alt="Screenshot 2024-08-15 at 6 17 11 PM" src="https://github.com/user-attachments/assets/1f094ce8-0d05-4c7f-bb6d-73221eeb3fef"> <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] 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. ## **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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Using v4.8.0, MetaMask estimated 1gwei on old UI then 21 gwei on New UI.
We should make sure we're using the same estimation logic, since it saves users a lot of money.
This visual shows why:
https://danfinlay.github.io/eth-gas-price-visuals/
The text was updated successfully, but these errors were encountered: