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

Do not fetch fiat rates for tokens without definitions #10783

Closed
Tracked by #10782
tomasklim opened this issue Jan 24, 2024 · 7 comments · Fixed by #10034
Closed
Tracked by #10782

Do not fetch fiat rates for tokens without definitions #10783

tomasklim opened this issue Jan 24, 2024 · 7 comments · Fixed by #10034
Assignees
Labels
bug Something isn't working as expected EVM Issues related to networks based on Ethereum Virtual Machine

Comments

@tomasklim
Copy link
Member

Describe the bug
Do not fetch fiat rates for tokens without definitions Screenshot 2024-01-02 at 5 02 55

@tomasklim tomasklim added the bug Something isn't working as expected label Jan 24, 2024
@tomasklim tomasklim added the EVM Issues related to networks based on Ethereum Virtual Machine label Jan 24, 2024
@tomasklim tomasklim mentioned this issue Jan 24, 2024
@tomasklim tomasklim self-assigned this Jan 24, 2024
@tomasklim tomasklim moved this to 🔎 Needs review in Suite Desktop Feb 5, 2024
@github-project-automation github-project-automation bot moved this from 🔎 Needs review to 🤝 Needs QA in Suite Desktop Feb 6, 2024
@bosomt
Copy link
Contributor

bosomt commented Feb 9, 2024

QA OK

Info:

  • Suite version: web 24.3.0 (b05fc18)
  • Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:122.0) Gecko/20100101 Firefox/122.0
  • OS: MacIntel
  • Screen: 1663x1080
  • Device: Trezor T2B1 2.6.4 regular (revision 42e9ed0e09033d474dee1a560fe5870646fa440e)
  • Transport: BridgeTransport 2.0.33

@bosomt bosomt moved this from 🤝 Needs QA to ✅ Approved in Suite Desktop Feb 9, 2024
@bosomt
Copy link
Contributor

bosomt commented Feb 9, 2024

Maybe we can consider not fetching tokens that are not available on Coingecko over and over again ?
We have one token thats not available and Suite tries to fetch it over and over again

https://etherscan.io/address/0xa74476443119A942dE498590Fe1f2454d7D4aC0d

image image

@Hannsek
Copy link
Contributor

Hannsek commented Feb 9, 2024

Strange, if it is not in the definitions it should be in the Unrecognized tokens tab.

@sime
Copy link
Contributor

sime commented Feb 11, 2024

It looks like a lovely edge case. GNT token has been replaced by GLM. From CoinGecko:

GNT is migrating to GLM. Refer to https://blog.golemproject.net/gnt-to-glm-migration/. Contract address has changed from 0xa74476443119a942de498590fe1f2454d7d4ac0d to 0x7DD9c5Cba05E151C895FDe1CF355C9A1D5DA6429.

Both contracts are in our definitions. I guess CoinGecko knows it's no longer a valid token.

@Hannsek
Copy link
Contributor

Hannsek commented Feb 11, 2024

Ok, it will likely be solved in March when we release updated definitions.

@sime
Copy link
Contributor

sime commented Feb 12, 2024

GNT>GLM migration was in 2020. I believe the contract will exist in our definitions for a while.

We may need some custom handling for deprecated but once valid coins.

There is a chance to optimise this handling, as @bosomt mentioned.

  • When token fiat rates returns 40x, don't refetch
  • Can we skip fiat rates for token balances of zero?

@Hannsek
Copy link
Contributor

Hannsek commented Feb 12, 2024

So far coingecko was doing ok job by removing the deprecated coins.

When token fiat rates returns 40x, don't refetch
Can we skip fiat rates for token balances of zero?

@tomasklim does it seem ok? do you have any other ideas which logic we can implement?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected EVM Issues related to networks based on Ethereum Virtual Machine
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants