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

feat: realtime balance fetcher for recent transactions #1717

Merged
merged 19 commits into from May 7, 2024

Conversation

magiziz
Copy link
Contributor

@magiziz magiziz commented Jan 19, 2024

Changes

  • Added useRefetchBalance hook to TransactionStoreContext to support real-time balance support

Screen recordings / screenshots

realtime_transaction_with_store.mov

What to test

  1. Replace address and value here and try sending the transaction to yourself
  2. After that's done make sure you go to our example dApp here and under "Add recent transactions" make sure you're adding the transaction hash and description
  3. Wait a few seconds and see your balance being refreshed for connect button and account modal

@magiziz magiziz requested a review from a team as a code owner January 19, 2024 21:51
Copy link

vercel bot commented Jan 19, 2024

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

Name Status Preview Comments Updated (UTC)
rainbowkit-example ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 7, 2024 7:00am
rainbowkit-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 7, 2024 7:00am

@magiziz magiziz linked an issue Jan 19, 2024 that may be closed by this pull request
1 task
@magiziz magiziz changed the title feat: realtime balance with useRefetchBalance hook feat: realtime balance updates with useRefetchBalance hook Jan 19, 2024
Copy link
Contributor

@DanielSinclair DanielSinclair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we also need to add docs. Want to create a section below "Advanced" called "Hooks", bump the Modal Hooks doc there as "Modals", and create a new doc for "Balances"?

.changeset/nasty-months-provide.md Outdated Show resolved Hide resolved
@DanielSinclair
Copy link
Contributor

Noticed some merge conflicts too with v2

@DanielSinclair
Copy link
Contributor

DanielSinclair commented Apr 4, 2024

I think we could simplify this:

  • If the dev uses useBalance within their dApp (or calls the refetch fn from the return type), then it should also re-render the balance inside the hook in the RainbowKit usage. We should test to confirm this (if you haven't already). That would reduce the need for us to have a separate function for them to call, as it may already refetch based on their dapp usage (or they could force it that way)
  • We also already have the useAddRecentTransaction hook and the TransactionStoreProvider which allows a dev to tell RainbowKit about a transaction to follow. This API is due for a refactor, but we generally want devs to funnel their transactions into that store so that RainbowKit can react to them. That reactive functionality should suffice for these refetch balance needs. We already poll to follow the status of a transaction there, so we could simply call useBalance again internally upon each tx confirmation (that way RainbowKit always has the latest balance).
  • As part of that eventual transaction store refactor, we could also maybe detect when a transaction is sent from Wagmi and automatically include it in the store and follow it (automatically reacting with balance updates for txs sent by that dapp). There are edge cases though, like the user submitting the tx elsewhere with an i.e. gasless signature.

chore: remove console.log

chore: remove console.log
@magiziz magiziz force-pushed the @mago/realtime-balance-updates branch from 26a1edd to 52d1e3f Compare April 8, 2024 16:34
@magiziz magiziz changed the title feat: realtime balance updates with useRefetchBalance hook feat: realtime balance fetcher with TransactionStoreContext Apr 9, 2024
Comment on lines +59 to +63
useEffect(() => {
if (store && address && chainId) {
return store.onTransactionStatus(onTransactionStatus);
}
}, [store, address, chainId, onTransactionStatus]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could pass this logic in here which would be less work, but that would cause a lot of re-renders + make unnecessary RPC calls since it'll happen pretty often due to how our transaction store provider works.

We'll call refetch whenever is necessary to update our balance (e.g when a tx has successfully passed).

Copy link
Contributor

@DanielSinclair DanielSinclair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything else looks good; just need that 1 balance fetch optimization I think

query: {
enabled: false,
},
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@magiziz Should we move this logic to useProfile so that we're caching it and reduce the number of balance fetches? Currently I believe this hook would be called even if showBalance is disabled, and would cause duplicate balance fetch calls when rendering Profile -> Transaction UI

@DanielSinclair DanielSinclair changed the title feat: realtime balance fetcher with TransactionStoreContext feat: realtime balance fetcher for recent transactions May 7, 2024
@DanielSinclair DanielSinclair merged commit 8841891 into main May 7, 2024
9 checks passed
@DanielSinclair DanielSinclair deleted the @mago/realtime-balance-updates branch May 7, 2024 07:12
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.

[bug] need to handle refresh the balance when any transaction is completed
2 participants