-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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: display conversion rate #21185
fix: display conversion rate #21185
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. |
// Display ETH | ||
currency = EtherDenomination.ETH; | ||
// Display Native currency | ||
currency = nativeCurrency; |
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.
Here we notice this piece of code if(showFiat){....} else{ fallback to ETH}
. When you set Fiat in the settings=>General, the problem does not occur.
When you choose crypto, it falls back automatically to 'display ETH' and currency is currency = EtherDenomination.ETH;
regardless of the network you are on.
On the core repo; inside TokenRatesController.ts
there is this fct fetchAndMapExchangeRates
responsible for fetching the conversion rates. That function first checks if the nativeCurrencySupported
by coingecko which in the case of BNB returns true
and in the case of "MATIC" returns false
.
If it is supported it will return the prices from coingecko, if it is not; it will
1- fetch from coingecko the exchange rate of that token exp (USDC) against the fallback currency (eth)
2- fetch from cryptocompare the conversion of native currency exp (ETH) vs(MATIC)
And then eventually it will multiply them tokenToVsCurrencyConversionRate * vsCurrencyToNativeCurrencyConversionRate
returning the conversion of USDC in the native currency (MATIC).
6c95976
to
888715c
Compare
Builds ready [888715c]
Page Load Metrics (535 ± 321 ms)
Bundle size diffs
|
888715c
to
fbf94bb
Compare
Builds ready [fbf94bb]
Page Load Metrics (359 ± 259 ms)
Bundle size diffs
|
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #21185 +/- ##
========================================
Coverage 68.64% 68.64%
========================================
Files 1017 1017
Lines 40797 40797
Branches 10893 10893
========================================
Hits 28004 28004
Misses 12793 12793
☔ View full report in Codecov by Sentry. |
fbf94bb
to
52c4561
Compare
Builds ready [52c4561]
Page Load Metrics (784 ± 374 ms)
Bundle size diffs
|
Description
This PR solves one bug shown in this github issue: #20106
When you try to send a token, on Polygon, it does not show the conversion of the amount, it just shows "ETH".
If you chose "FIAT" in the settings=>general, this issue does not occur. This only happens when you choose crypto.
PS: I have also noticed this issue on BNB chain. On Ethereum network, all works fine. If you try to send a token on other chains, you won't be able to see the conversion correctly and it will always just show "ETH".
Manual testing steps
_1. Chose polygon network
_2. Go to settings => General and choose "MATIC" as the primary currency.
_3. Go back to the home screen and choose a token to send (exp USDC)
_4. Click on the "send" button and choose an account
_5. Notice that if you type "5" in the amount input, you will see "ETH" as the conversion of that amount.
(The same steps are applicable on BNB chain)
Screenshots/Recordings
If applicable, add screenshots and/or recordings to visualize the before and after of your change.
Before
On Polygon
On BNB
After
On Polygon:
On BNB:
Related issues
(_Fixes #MMASSETS-26)[https://consensyssoftware.atlassian.net/browse/MMASSETS-26]
Pre-merge author checklist
Pre-merge reviewer checklist