refactor: enhance wallet balance fetching to include raw balances#392
Conversation
* Updated fetchWalletBalance function to return rawBalances as bigint for improved precision. * Modified WalletTransferApprovalModal to handle and display raw balances alongside regular balances. * Ensured compatibility with existing balance fetching logic while enhancing data structure for better usability.
📝 WalkthroughWalkthroughAdded chain-level raw bigint balance tracking ( Changes
Sequence Diagram(s)(Skipped — changes are focused on data plumbing and encoding within two components, not a new multi-actor control flow requiring visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/components/WalletTransferApprovalModal.tsx (1)
79-104: Consider concurrent chain balance fetches to reduce modal load latency.This loop is currently serial; moving to a concurrent best-effort fetch pattern will make balance hydration much faster while still tolerating per-chain failures.
♻️ Suggested refactor
- // Fetch balances for each supported network - for (const network of networks) { - try { - const publicClient = createPublicClient({ - chain: network.chain, - transport: getTransportForChain(network.chain), - }); - - const result = await fetchWalletBalance( - publicClient, - oldAddress - ); - - // Only include chains with non-zero balances - const hasBalance = Object.values(result.balances).some(b => b > 0); - if (hasBalance) { - balancesByChain[network.chain.name] = result.balances; - rawByChain[network.chain.name] = result.rawBalances; - } - } catch (error) { - console.error(`Error fetching balances for ${network.chain.name}:`, error); - } - } + const settled = await Promise.allSettled( + networks.map(async (network) => { + const publicClient = createPublicClient({ + chain: network.chain, + transport: getTransportForChain(network.chain), + }); + const result = await fetchWalletBalance(publicClient, oldAddress); + return { chainName: network.chain.name, result }; + }) + ); + + for (const item of settled) { + if (item.status !== "fulfilled") continue; + const { chainName, result } = item.value; + const hasBalance = Object.values(result.balances).some((b) => b > 0); + if (!hasBalance) continue; + balancesByChain[chainName] = result.balances; + rawByChain[chainName] = result.rawBalances; + }Based on learnings: In TSX files that perform parallel network fetches, use a graceful-degradation approach that keeps fulfilled results and avoids failing the whole flow on individual rejections.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/WalletTransferApprovalModal.tsx` around lines 79 - 104, The serial for-loop in WalletTransferApprovalModal that calls createPublicClient and fetchWalletBalance for each network should be made concurrent: map networks to promises that perform the per-chain fetch (using createPublicClient/getTransportForChain and fetchWalletBalance), then await Promise.allSettled on that array, iterate the settled results and for each fulfilled entry add to balancesByChain and rawByChain (using network.chain.name as the key), and ignore/log rejected entries; finally call setAllChainBalances, setAllChainRawBalances, and setIsFetchingBalances(false). This keeps best-effort behavior per-chain while significantly reducing latency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/components/WalletTransferApprovalModal.tsx`:
- Around line 79-104: The serial for-loop in WalletTransferApprovalModal that
calls createPublicClient and fetchWalletBalance for each network should be made
concurrent: map networks to promises that perform the per-chain fetch (using
createPublicClient/getTransportForChain and fetchWalletBalance), then await
Promise.allSettled on that array, iterate the settled results and for each
fulfilled entry add to balancesByChain and rawByChain (using network.chain.name
as the key), and ignore/log rejected entries; finally call setAllChainBalances,
setAllChainRawBalances, and setIsFetchingBalances(false). This keeps best-effort
behavior per-chain while significantly reducing latency.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Free Tier Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
…ved precision * Modified fetchWalletBalance function to replace rawBalances with balancesInWei, enhancing the accuracy of balance data. * Updated WalletTransferApprovalModal to utilize balancesInWei for consistency in balance handling. * Ensured compatibility with existing logic while improving data structure for better usability.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/utils.ts (1)
591-608: Return type offetchBalanceForNetworkis now out of sync withfetchWalletBalance.This wrapper function's return type still declares
Promise<{ total: number; balances: Record<string, number> }>, but it returns the result offetchWalletBalancewhich now includesbalancesInWei. While this won't break existing callers, the type inconsistency may cause confusion.♻️ Suggestion: Update return type for consistency
export async function fetchBalanceForNetwork( network: { chain: any }, walletAddress: string, -): Promise<{ total: number; balances: Record<string, number> }> { +): Promise<{ total: number; balances: Record<string, number>; balancesInWei: Record<string, bigint> }> { const { createPublicClient, http } = await import("viem");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/utils.ts` around lines 591 - 608, The declared return type of fetchBalanceForNetwork is outdated: it currently promises { total, balances } but actually returns fetchWalletBalance(...) which now includes balancesInWei; update fetchBalanceForNetwork's signature to match fetchWalletBalance by adding the balancesInWei field (and exact type used by fetchWalletBalance) so the function signature for fetchBalanceForNetwork aligns with fetchWalletBalance's returned shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/utils.ts`:
- Around line 591-608: The declared return type of fetchBalanceForNetwork is
outdated: it currently promises { total, balances } but actually returns
fetchWalletBalance(...) which now includes balancesInWei; update
fetchBalanceForNetwork's signature to match fetchWalletBalance by adding the
balancesInWei field (and exact type used by fetchWalletBalance) so the function
signature for fetchBalanceForNetwork aligns with fetchWalletBalance's returned
shape.
Description
This pull request enhances the wallet transfer approval flow by improving how token balances are handled and transferred. The main improvement is the consistent use of raw token amounts (as
bigint) throughout the balance fetching and transfer process, ensuring accuracy for all token values, including very small amounts. This eliminates the need for parsing and conversion at the transfer step, reducing potential errors and simplifying the code.Wallet Balance Handling Improvements:
fetchWalletBalanceutility now returns both human-readable balances and raw token balances (asbigint) in a newrawBalancesfield, ensuring precise value handling for all tokens. [1] [2] [3] [4]WalletTransferApprovalModal.tsx) stores and utilizes these raw balances in a new state variableallChainRawBalances, keeping them in sync with the existing human-readable balances. [1] [2] [3] [4]Token Transfer Logic Updates:
token.rawAmount) when encoding transfer transactions, removing the need forparseUnitsand preventing rounding or precision issues.Code Clean-up:
parseUnitsimport fromvieminWalletTransferApprovalModal.tsxas it is no longer necessary.React Dependency Updates:
Updated React
useMemodependencies to includeallChainRawBalances, ensuring the UI stays in sync with raw balance changes.Updated fetchWalletBalance function to return rawBalances as bigint for improved precision.
Modified WalletTransferApprovalModal to handle and display raw balances alongside regular balances.
Ensured compatibility with existing balance fetching logic while enhancing data structure for better usability.
References
Testing
Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit