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: refresh balance on address change #84

Merged
merged 4 commits into from
Mar 7, 2024

Conversation

shahin-hq
Copy link
Contributor

@shahin-hq shahin-hq commented Feb 28, 2024

Closes: https://app.clickup.com/t/86drpzvd8

Problem:
Every time wallet changes useWalletBalance should trigger and recalculate balance in USD, the problem is if wallets get changed multiple times in a fraction of minute we may hit Coingecko rate limits.

Solution:
The root of problem is having a Coingecko instance in Wallet object, this basically causes problem because of how things work:

  • user changes addresses
  • useWalletBalance get called with new wallet which creates a Wallet instance with the new wallet
  • creating a new Wallet instance = creating a new Coingecko which means losing the cached values

Instead of that we could have a Coingecko instance managed by useQuery and data could be passed to the Wallet when it is required to use. I could pass it through the constructor, however, this would be aggressive as there might be cases we might need Wallet instance without using any balance/rate related logic. That is why decided to pass it as a function argument.

new_recording_-_2_28_2024._5_42_18_pm.Original.mp4

Copy link

vercel bot commented Feb 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
arkconnect-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 28, 2024 1:39pm

@shahin-hq shahin-hq force-pushed the fix/refresh-balance-on-address-change branch from 28db195 to 7904c13 Compare February 28, 2024 12:45
@shahin-hq shahin-hq marked this pull request as ready for review February 28, 2024 12:47
…hange' into fix/refresh-balance-on-address-change

# Conflicts:
#	src/app/hooks/useWalletBalance.ts
@ItsANameToo ItsANameToo merged commit 3855df5 into develop Mar 7, 2024
2 checks passed
@ItsANameToo ItsANameToo deleted the fix/refresh-balance-on-address-change branch March 7, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants