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

[Bug]: Token Conversion - API request to cryptocompare is made with the token ticker name resulting in a non-real balance conversion #28262

Open
seaona opened this issue Nov 4, 2024 · 1 comment
Labels
regression-RC-12.7.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-assets type-bug

Comments

@seaona
Copy link
Contributor

seaona commented Nov 4, 2024

Describe the bug

Whenever I change the default ticker to a different one, I see how the API request to cryptocompare is made with the new ticker name as a payload. This results in getting a non real conversion of my current assets in that network.
Ie.

  • I'm in Sepolia and I have some Sepolia ETH which is $135,706.33 converted in fiat.
  • I have show native token as main balance disabled
  • I have show conversion in test nets enabled

Now I change the token name to BTC and I can see how a request to cryptocompare is made with the BTC token payload.
The result is that now I see in my wallet a total of $3,780,853.33

Expected behavior

I'm not sure we should base the cryptocompare request on the name of the token.
Maybe we shouldn't display the conversion for such cases, as the network doesn't have the expected token name? This is what's currently happening in prod: we don't display the conversion but rather the token, with whichever name we set.

Screenshots/Recordings

currency-change-ticker.mp4

Steps to reproduce

  1. In settings, select Show conversion in testnets
  2. Disable the show native token as main balance
  3. Select Sepolia
  4. See your balance in USD
  5. Edit Sepolia network and change the token name to BTC or ETH
  6. See your balance in USD is now different

Error messages or log output

No response

Detection stage

During release testing

Version

12.7.0

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

No response

Severity

No response

@seaona seaona added type-bug Sev2-normal Normal severity; minor loss of service or inconvenience. team-assets labels Nov 4, 2024
@bergeron
Copy link
Contributor

bergeron commented Nov 4, 2024

The native currency prices have always been ticker based. But on chains where it doesn't match what's in github.com/ethereum-lists/chains, we're meant to not show fiat and issue a warning like:

Screenshot 2024-11-04 at 8 15 12 AM

It might be that that the new aggregated portfolio balance is not doing similar symbol verification. Could also be testnets behaving differently for some reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-RC-12.7.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-assets type-bug
Projects
Status: To be fixed
Status: To be fixed
Development

No branches or pull requests

3 participants