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.

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

fix: remove value from price api call #4364

Merged
merged 2 commits into from
Jun 4, 2024

Conversation

salimtb
Copy link
Contributor

@salimtb salimtb commented Jun 4, 2024

Explanation

Delete the value property from the returned payload of the tokenRateController

References

Changelog

@metamask/assets-controller

  • REMOVED: Remove the value property from the returned object

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@salimtb salimtb requested a review from a team as a code owner June 4, 2024 07:43
@salimtb salimtb force-pushed the fix-assets-controller-market-data branch from 7bfd324 to 92ab226 Compare June 4, 2024 07:47
@salimtb salimtb merged commit 5c5012e into main Jun 4, 2024
114 checks passed
@salimtb salimtb deleted the fix-assets-controller-market-data branch June 4, 2024 21:15
danjm pushed a commit to MetaMask/metamask-extension that referenced this pull request Aug 16, 2024
## **Description**

Fixes an issue with erc20 token prices on networks where ETH is not the
native currency.

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.
But the extension was using the raw value without this additional hop.

This PR patches the relevant parts of
MetaMask/core#4364.

Because that change is already in 12.1.0, this patch will no longer be
needed in that version.

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

## **Related issues**


## **Manual testing steps**

1. Load a wallet on polygon with erc20 tokens
2. Verify fiat prices are correct


## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<img width="355" alt="Screenshot 2024-08-15 at 11 19 43 AM"
src="https://github.com/user-attachments/assets/f366027e-789e-44e9-99b8-b7a99f24c250">


### **After**

<img width="355" alt="Screenshot 2024-08-15 at 11 15 00 AM"
src="https://github.com/user-attachments/assets/59113748-3118-4fbb-bd6e-30c12b64b7fe">


## **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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants