Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Note
|
| Cohort / File(s) | Summary |
|---|---|
New Token Metadata API Integrationhooks/evm/useZealousSwapMetadata.ts |
New hook file providing SWR-based fetching from Zealous Swap API. Exports useZealousSwapTokensMetadata() for raw metadata, useErc20Image(chainId, contractAddress) for logo URLs with fallback support, and useErc20Price(chainId, contractAddress) for token pricing. Includes chain validation (Kasplex mainnet only) and error/loading state propagation. |
Hook Refactoring—Balance Retrievalhooks/evm/useErc20Balance.ts, hooks/evm/useEvmKasBalance.ts |
Shifted from per-token/per-chain single-call patterns to batch-based aggregate hooks. Removed decimals parameter from signatures. Introduced useErc20Balances(), useErc20BalancesByAddress(evmAddress) and useEvmKasBalances(), useEvmKasBalancesByAddress(evmAddress) for multi-chain/multi-token retrieval. Updated SWR keys and fetchers to support address-driven batch operations with per-item error handling. |
Hook Integration—Portfolio Calculationshooks/kasplex/useTotalBalanceByAccount.ts, hooks/usePortfolioPerformance.ts |
Integrated EVM balance and pricing hooks to compute totalErc20Usd and totalEvmKasUsd by aggregating ERC20 and EVM Kaspa balances with current prices. Added evmAddress derivation from account.evmPublicKey. Updated balance summations to include multi-chain EVM Kaspa amounts alongside on-chain Kaspa; introduces TODO for ERC20 last-day price data. |
Component Updates—Asset Displaycomponents/dashboard/Erc20Asset.tsx, components/erc20-asset/Erc20Info.tsx |
Integrated useErc20Image and useErc20Price to fetch dynamic logo URLs and token prices. Replaced hardcoded zero-price and static image fallbacks with real data. Updated balance display to use raw balance (no parsing). Token image now prefers asset.image with logoUrl fallback. |
Component Updates—Asset Selection & Sendingcomponents/screens/asset-selector/Erc20SelectItem.tsx, components/send/evm/erc20-send/ConfirmStep.tsx, components/send/evm/erc20-send/DetailsStep.tsx |
Unified changes: simplified useErc20Balance calls by removing decimals parameter; integrated useErc20Image and useErc20Price for dynamic logo and pricing. Replaced kasIcon static fallback with logoUrl. Updated balance formatting to use raw balance values. Adjusted fiat/currency calculations to use fetched prices instead of hardcoded zeros. |
Sequence Diagrams
sequenceDiagram
participant UI as UI Component
participant SWR as SWR Hook
participant MetaAPI as Zealous Swap API
participant Cache as Local Cache
rect rgb(200, 230, 250)
Note over UI,Cache: Token Metadata & Pricing Flow
UI->>SWR: useErc20Price(chainId, contractAddress)
SWR->>Cache: Check cached metadata
alt Cache hit & fresh
Cache-->>SWR: Return cached data
else Cache miss or stale
SWR->>MetaAPI: Fetch tokens metadata
MetaAPI-->>SWR: TokenMetadata[]
SWR->>Cache: Store (60s refresh interval)
end
SWR->>SWR: Find token by address, extract price
SWR-->>UI: { price, error, isLoading }
end
sequenceDiagram
participant UI as UI Component
participant Hook as Batch Balance Hook
participant SWR as SWR Multi-Chain
participant Chain1 as Chain (Mainnet)
participant Chain2 as Chain (Testnet)
rect rgb(220, 250, 220)
Note over UI,Chain2: Multi-Chain Balance Aggregation
UI->>Hook: useEvmKasBalances() or useErc20Balances()
Hook->>Hook: Resolve current evmAddress
Hook->>SWR: Fetch with key: balances:<networkId>-<address>
SWR->>Chain1: Query balance for address
Chain1-->>SWR: rawBalance, ether-formatted balance
SWR->>Chain2: Query balance for address
Chain2-->>SWR: rawBalance, ether-formatted balance
SWR->>SWR: Aggregate results into map {chainId → balance}
SWR-->>Hook: { data: {[chainId]: balance}, error, isLoading }
Hook-->>UI: Requested balance or aggregated map
end
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~50 minutes
The changes span 10 files with heterogeneous modifications: new SWR-based metadata API integration requiring validation of chain-gating and error handling; substantial refactoring of balance hooks introducing batch operations and multi-chain aggregation with updated call signatures affecting multiple consumers; six UI components with similar but distinct integration patterns (image fallbacks, price calculations, balance formatting). The logic density is moderate-to-high around hook refactoring and SWR key management, and consistent verification of signature changes across all call sites is required.
Poem
🐰 Hops through chains with batches bright,
Metadata flows from Zealous Swap's light,
No more zeros hardcoded in sight,
Images and prices, aggregated right,
Multi-chain balances—a hoppy delight! 🎉
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'feat: include layer2 balances' directly and clearly summarizes the main objective of the changeset, which integrates Layer 2 ERC-20 balances, prices, and images across multiple components and hooks. |
✨ Finishing touches
- 📝 Generate docstrings
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 @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Pull request overview
This PR adds Layer 2 (EVM-based) balance integration to the portfolio performance calculations. It refactors EVM balance hooks to fetch balances from multiple chains concurrently, integrates with the ZealousSwap API for ERC20 token pricing and images, and updates UI components to display Layer 2 assets with proper pricing information.
Key Changes:
- Added Layer 2 (EVM KAS and ERC20) balances to portfolio performance and total balance calculations
- Integrated ZealousSwap API for ERC20 token metadata including prices and logos
- Refactored balance hooks to support batch fetching across multiple chains and tokens
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| hooks/usePortfolioPerformance.ts | Integrates EVM KAS balances from Layer 2 chains into portfolio performance calculations |
| hooks/kasplex/useTotalBalanceByAccount.ts | Adds ERC20 and EVM KAS USD values to total account balance |
| hooks/evm/useZealousSwapMetadata.ts | New hook providing ERC20 token metadata (prices, images) from ZealousSwap API |
| hooks/evm/useEvmKasBalance.ts | Refactored to fetch KAS balances from all supported chains concurrently |
| hooks/evm/useErc20Balance.ts | Refactored to batch-fetch all ERC20 token balances in a single hook call |
| components/send/evm/erc20-send/DetailsStep.tsx | Updated to use new balance hook signature and display token prices/images |
| components/send/evm/erc20-send/ConfirmStep.tsx | Integrated ERC20 pricing for transaction confirmation display |
| components/screens/asset-selector/Erc20SelectItem.tsx | Updated to use refactored balance hook and display token logos |
| components/erc20-asset/Erc20Info.tsx | Added ERC20 price and image display using ZealousSwap metadata |
| components/dashboard/Erc20Asset.tsx | Integrated ERC20 pricing and images for dashboard asset display |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| asset.chainId, | ||
| ); | ||
| const { data } = useErc20Balance(asset.address, asset.chainId); | ||
| const balance = data?.balance ?? "0"; |
There was a problem hiding this comment.
The balance type has changed from string to number (based on the useErc20Balance refactoring), but the fallback value is still a string "0". This type inconsistency could cause issues. The balance should be typed as number and the fallback should be numeric 0.
| } catch (error) { | ||
| console.error(`Error fetching balance for chain ${chainId}:`, error); | ||
| balances[chainId] = { | ||
| rawBalance: 0n, | ||
| balance: "0", | ||
| }; | ||
| } |
There was a problem hiding this comment.
The error handling returns an object with rawBalance set to 0n (bigint) and balance as "0" (string) when an error occurs. However, there's no indication to the caller that an error occurred - the error is only logged to console. Consider returning an error state or throwing to let the caller handle the error appropriately, or at minimum include an error flag in the returned object.
| return useSWR(key, evmAddress ? fetcher : null, { | ||
| refreshInterval: 5_000, |
There was a problem hiding this comment.
The refreshInterval is set to 5 seconds (5_000ms), which means the hook will make network requests to fetch all ERC20 balances every 5 seconds. If there are many tokens in the asset list, this could result in a large number of concurrent RPC calls every 5 seconds, potentially causing performance issues or hitting rate limits. Consider increasing the refresh interval or implementing a more efficient balance update strategy.
| @@ -1,51 +1,120 @@ | |||
| import useSWR from "swr"; | |||
| import { createPublicClient, http, erc20Abi } from "viem"; | |||
| import { createPublicClient, http, erc20Abi, Hex, Address } from "viem"; | |||
There was a problem hiding this comment.
Unused import Hex.
| import { createPublicClient, http, erc20Abi, Hex, Address } from "viem"; | |
| import { createPublicClient, http, erc20Abi, Address } from "viem"; |
| import useKaspaBalance from "@/hooks/wallet/useKaspaBalance"; | ||
| import { useErc20Prices } from "@/hooks/evm/useZealousSwapMetadata"; | ||
| import { | ||
| useEvmKasBalances, |
There was a problem hiding this comment.
Unused import useEvmKasBalances.
| useEvmKasBalances, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
hooks/evm/useZealousSwapMetadata.ts (1)
25-31: Consider adding dedupingInterval to SWR config.The SWR configuration includes
refreshIntervaland revalidation flags but lacksdedupingInterval. Adding it can prevent redundant requests when multiple components mount simultaneously and request the same data.🔎 Suggested improvement
export function useZealousSwapTokensMetadata() { return useSWR<ZealousSwapApiResponse>(ZEALOUS_SWAP_API_URL, fetcher, { refreshInterval: 60_000, // Refresh every minute revalidateOnFocus: false, revalidateOnReconnect: true, + dedupingInterval: 60_000, // Dedupe requests within 1 minute }); }hooks/evm/useEvmKasBalance.ts (1)
64-78: Consider structured error tracking for balance fetch failures.Lines 64-78 catch per-chain balance fetch errors, log them to console, and replace with 0 balance. While this prevents one chain's failure from blocking others, using
console.errormeans errors won't be tracked in production monitoring systems like Sentry.Consider using structured error tracking:
🔎 Suggested improvement
} catch (error) { - console.error(`Error fetching balance for chain ${chainId}:`, error); + captureException(error, { + tags: { chainId, component: 'useEvmKasBalance' }, + extra: { evmAddress }, + }); balances[chainId] = { rawBalance: 0n, balance: "0", }; }Don't forget to import
captureExceptionfrom@sentry/reactat the top of the file.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
components/dashboard/Erc20Asset.tsxcomponents/erc20-asset/Erc20Info.tsxcomponents/screens/asset-selector/Erc20SelectItem.tsxcomponents/send/evm/erc20-send/ConfirmStep.tsxcomponents/send/evm/erc20-send/DetailsStep.tsxhooks/evm/useErc20Balance.tshooks/evm/useEvmKasBalance.tshooks/evm/useZealousSwapMetadata.tshooks/kasplex/useTotalBalanceByAccount.tshooks/usePortfolioPerformance.ts
🧰 Additional context used
🧬 Code graph analysis (7)
hooks/usePortfolioPerformance.ts (4)
hooks/wallet/useWalletManager.ts (1)
useWalletManager(3-5)hooks/useKaspaPrice.ts (1)
useKaspaPrice(3-5)hooks/wallet/useKaspaBalance.ts (1)
useKaspaBalance(3-6)hooks/evm/useEvmKasBalance.ts (1)
useEvmKasBalancesByAddress(41-95)
hooks/kasplex/useTotalBalanceByAccount.ts (3)
hooks/evm/useErc20Balance.ts (1)
useErc20BalancesByAddress(44-115)hooks/evm/useZealousSwapMetadata.ts (1)
useErc20Prices(85-93)hooks/evm/useEvmKasBalance.ts (1)
useEvmKasBalancesByAddress(41-95)
hooks/evm/useEvmKasBalance.ts (3)
hooks/useSettings.ts (1)
useSettings(3-8)lib/layer2.ts (2)
MAINNET_SUPPORTED_EVM_L2_CHAINS(56-56)TESTNET_SUPPORTED_EVM_L2_CHAINS(55-55)hooks/evm/useEvmAddress.ts (1)
useEvmAddress(4-12)
components/send/evm/erc20-send/ConfirmStep.tsx (1)
hooks/evm/useZealousSwapMetadata.ts (1)
useErc20Price(59-83)
components/erc20-asset/Erc20Info.tsx (4)
components/dashboard/Erc20Asset.tsx (1)
Erc20Asset(17-63)contexts/EvmAssets.tsx (1)
Erc20Asset(12-18)hooks/evm/useZealousSwapMetadata.ts (2)
useErc20Price(59-83)useErc20Image(33-57)hooks/useCurrencyValue.ts (1)
useCurrencyValue(14-31)
hooks/evm/useErc20Balance.ts (3)
hooks/evm/useErc20Assets.ts (1)
useErc20Assets(7-55)lib/layer2.ts (1)
ALL_SUPPORTED_EVM_L2_CHAINS(58-61)hooks/evm/useEvmAddress.ts (1)
useEvmAddress(4-12)
components/send/evm/erc20-send/DetailsStep.tsx (5)
hooks/evm/useErc20Balance.ts (1)
useErc20Balance(8-42)hooks/evm/useEvmKasBalance.ts (1)
useEvmKasBalance(17-39)hooks/evm/useZealousSwapMetadata.ts (2)
useErc20Image(33-57)useErc20Price(59-83)hooks/useCurrencyValue.ts (1)
useCurrencyValue(14-31)lib/utils.ts (1)
formatToken(61-69)
🪛 Biome (2.1.2)
components/dashboard/Erc20Asset.tsx
[error] 17-17: Shouldn't redeclare 'Erc20Asset'. Consider to delete it or rename it.
'Erc20Asset' is defined here:
(lint/suspicious/noRedeclare)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (12)
components/send/evm/erc20-send/ConfirmStep.tsx (1)
29-29: LGTM! Dynamic ERC20 pricing now integrated.The replacement of the hardcoded
tokenPrice = 0with the dynamic price fromuseErc20Pricecorrectly enables real-time fiat amount calculations for ERC20 transfers on Kasplex.Also applies to: 73-73, 76-76
components/erc20-asset/Erc20Info.tsx (1)
8-17: LGTM! Dynamic price and image metadata integrated correctly.The component now fetches real-time token prices and logo URLs from the Zealous Swap API, replacing hardcoded placeholders. The price is correctly fed into
useCurrencyValuefor fiat conversion, and the logo fallback mechanism is properly implemented.components/dashboard/Erc20Asset.tsx (1)
20-31: LGTM! Dynamic pricing and image fallback correctly implemented.The balance retrieval, price calculation, and image fallback logic are correctly updated to use the new hooks. The
formatTokencall on line 31 correctly passes a number (data?.balance ?? 0), and the total balance calculation properly multiplies price by balance.Note: The static analysis warning about redeclaring
Erc20Asseton line 17 is a false positive—TypeScript allows the same identifier in different namespaces (type vs value).Also applies to: 39-39
components/send/evm/erc20-send/DetailsStep.tsx (1)
27-30: LGTM! Balance, price, and image handling correctly refactored.The updated balance retrieval (removing the
decimalsparameter), dynamic price fetching, and logo fallback are all correctly implemented. Balance is consistently treated as a number, aligning with theformatTokensignature requirements.Also applies to: 51-56, 81-81, 91-92
hooks/evm/useEvmKasBalance.ts (1)
84-94: LGTM! Multi-chain balance aggregation correctly implemented.The new architecture fetches balances across all supported chains in parallel, gracefully handles per-chain errors, and maintains backward compatibility through the existing
useEvmKasBalance(chainId)interface.Optional: Consider adding
dedupingInterval: 10000to the SWR config to match therefreshIntervaland prevent duplicate requests during the same window.hooks/kasplex/useTotalBalanceByAccount.ts (1)
6-12: LGTM! Layer2 balance aggregation correctly integrated.The hook now properly aggregates ERC20 token balances and EVM KAS balances across supported chains into the total USD balance. The implementation includes proper error handling (skipping items with errors, handling missing prices) and correctly converts string balances to numbers for calculations.
Also applies to: 23-55
hooks/usePortfolioPerformance.ts (3)
9-10: LGTM!The new imports correctly support the Layer2 balance integration.
31-32: USD calculations correctly use aggregated balance.The calculations properly incorporate both on-chain and Layer2 balances. However, ensure the precision issues from lines 25-30 are addressed to maintain accuracy in these USD valuations.
17-23: Publickey to address derivation is correct and the TODO is appropriately marked.The
publicKeyToAddressimport fromviem/utilsis valid in viem 2.37.13, and the EVM address derivation correctly handles the case whereevmPublicKeymay not exist. The TODO for ERC20 token performance calculation is appropriate for tracking future work.hooks/evm/useErc20Balance.ts (3)
1-6: LGTM!The imports correctly support the refactored batch-based balance fetching approach.
117-120: LGTM!Clean wrapper pattern that delegates to
useErc20BalancesByAddresswith the current user's EVM address.
8-42: The review comment's claim about a breaking change is incorrect. TheuseErc20Balancefunction signature is(tokenAddress: string, chainId: string)with only 2 parameters, and all existing call sites in the codebase (3 usages: Erc20Asset.tsx, DetailsStep.tsx, Erc20SelectItem.tsx) already use this signature. There is no decimals parameter in the function's signature—decimals is fetched internally viauseErc20Balances()from the asset data and included in the returned balance objects. The refactored design is correct.The type cast on line 35
(balanceData as { error: string }).errorcould be made more explicit by defining a proper error type for better type safety.Likely an incorrect or invalid review comment.
|
|
||
| export default function Erc20Asset({ asset }: { asset: Erc20Asset }) { | ||
| const navigate = useNavigate(); | ||
| const [settings] = useSettings(); |
There was a problem hiding this comment.
Missing import for useSettings.
Line 19 uses useSettings() but the import statement is missing. This will cause a compilation error.
🔎 Proposed fix
Add the missing import at the top of the file:
import { Erc20Asset } from "@/contexts/EvmAssets";
import useErc20Balance from "@/hooks/evm/useErc20Balance";
+import { useSettings } from "@/hooks/useSettings";
import Layer2AssetImage from "../Layer2AssetImage";Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/dashboard/Erc20Asset.tsx around line 19, the hook useSettings is
used but not imported; add an import for useSettings at the top of the file
(e.g. import { useSettings } from '<module-where-useSettings-is-exported>';),
placing it with the other React/hooks imports so the file compiles.
| const { data } = useErc20Balance(asset.address, asset.chainId); | ||
| const balance = data?.balance ?? "0"; | ||
| const { logoUrl } = useErc20Image(asset.chainId, asset.address); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the balance type returned by useErc20Balance hook and its usage across components
echo "=== Checking useErc20Balance return type definition ==="
fd -e ts -e tsx | xargs rg -l "useErc20Balance" | head -5
echo ""
echo "=== Finding useErc20Balance hook definition ==="
fd -e ts -e tsx -path "*/hooks/*" | xargs rg "useErc20Balance" -l
echo ""
echo "=== Checking the hook implementation ==="
rg -A 15 "export.*function useErc20Balance|export.*const useErc20Balance" --type=ts --type=tsx
echo ""
echo "=== Checking balance usage in Erc20SelectItem.tsx ==="
rg -n -A 3 -B 3 "balance" components/screens/asset-selector/Erc20SelectItem.tsx
echo ""
echo "=== Checking formatToken function signature ==="
rg -B 2 -A 8 "function formatToken|const formatToken|export.*formatToken" --type=ts --type=tsx | head -20
echo ""
echo "=== Checking balance type in other files ==="
rg -n "useErc20Balance" --type=ts --type=tsx -A 2 | head -30Repository: forbole/kastle
Length of output: 1899
🏁 Script executed:
#!/bin/bash
echo "=== Checking useErc20Balance hook implementation ==="
cat hooks/evm/useErc20Balance.ts
echo ""
echo "=== Searching for formatToken function ==="
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "function formatToken\|const formatToken\|export.*formatToken" 2>/dev/null
echo ""
echo "=== Getting formatToken definition ==="
find . -name "*.ts" -o -name "*.tsx" | xargs grep -B 2 -A 10 "function formatToken\|const formatToken.*=" 2>/dev/null | head -30Repository: forbole/kastle
Length of output: 4683
Fix critical type mismatch with balance variable.
The useErc20Balance hook returns data.balance as a number (parsed via parseFloat(formatUnits(...))), but line 17 defaults it to the string "0", creating a number | string union type. This causes a type error on line 55 when passing balance to formatToken(number), which expects only a number. Additionally, line 28 compares balance !== "0" (string comparison), which is inconsistent if balance is a number.
Fix by using a numeric default: const balance = data?.balance ?? 0;
🤖 Prompt for AI Agents
In components/screens/asset-selector/Erc20SelectItem.tsx around lines 16 to 18,
the balance variable is defaulting to a string "0" while useErc20Balance returns
a number, producing a number|string union and causing type errors when passed to
functions expecting a number; change the default to a numeric 0 and update any
comparisons to use numeric checks (e.g., balance !== 0) so balance is
consistently typed as a number throughout the file.
| </div> | ||
| </div> | ||
| {!isLedger && <span>{formatToken(parseFloat(balance))}</span>} | ||
| {!isLedger && <span>{formatToken(balance)}</span>} |
There was a problem hiding this comment.
Fix type mismatch in formatToken call.
The balance variable is typed as string (line 17: data?.balance ?? "0"), but formatToken expects a number as its first parameter. This will cause a TypeScript error or unexpected runtime behavior.
🔎 Proposed fix
- {!isLedger && <span>{formatToken(balance)}</span>}
+ {!isLedger && <span>{formatToken(Number(balance))}</span>}Or better yet, fix line 17 to treat balance as a number:
- const balance = data?.balance ?? "0";
+ const balance = data?.balance ?? 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {!isLedger && <span>{formatToken(balance)}</span>} | |
| {!isLedger && <span>{formatToken(Number(balance))}</span>} |
🤖 Prompt for AI Agents
components/screens/asset-selector/Erc20SelectItem.tsx around line 55:
formatToken is being passed balance which is currently a string (set at line 17
as data?.balance ?? "0"), causing a type mismatch; change the balance
initialization to produce a number (e.g. parseFloat(data?.balance ?? "0") or
Number(data?.balance ?? 0)) so formatToken receives a numeric first argument,
and update any related typings to reflect balance: number.
| export function useErc20BalancesByAddress(evmAddress?: Address) { | ||
| const { assets } = useErc20Assets(); | ||
|
|
||
| // Use all tokens from useErc20Tokens | ||
| const tokensToFetch = | ||
| assets?.map((asset) => { | ||
| return { | ||
| tokenAddress: asset.address, | ||
| decimals: asset.decimals, | ||
| chainId: asset.chainId, | ||
| }; | ||
| }) ?? []; | ||
|
|
||
| export default function useErc20Balance( | ||
| tokenAddress: string, | ||
| decimals: number, | ||
| chainId: string, | ||
| ) { | ||
| const evmAddress = useEvmAddress(); | ||
| const fetcher = async () => { | ||
| if (!evmAddress) { | ||
| return {}; | ||
| if (!evmAddress || tokensToFetch.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| const chain = ALL_SUPPORTED_EVM_L2_CHAINS.find( | ||
| (c) => numberToHex(c.id) === chainId, | ||
| const balancePromises = tokensToFetch.map( | ||
| async ({ tokenAddress, decimals, chainId }) => { | ||
| const chain = ALL_SUPPORTED_EVM_L2_CHAINS.find( | ||
| (c) => numberToHex(c.id) === chainId, | ||
| ); | ||
| if (!chain) { | ||
| return { | ||
| error: `Unsupported chain ID: ${chainId}`, | ||
| }; | ||
| } | ||
|
|
||
| const client = createPublicClient({ | ||
| chain, | ||
| transport: http(), | ||
| }); | ||
|
|
||
| try { | ||
| const rawBalance = await client.readContract({ | ||
| address: tokenAddress as `0x${string}`, | ||
| abi: erc20Abi, | ||
| functionName: "balanceOf", | ||
| args: [evmAddress], | ||
| }); | ||
| return { | ||
| tokenAddress, | ||
| chainId, | ||
| decimals, | ||
| rawBalance, | ||
| balance: parseFloat(formatUnits(rawBalance, decimals)), | ||
| }; | ||
| } catch (error) { | ||
| console.error(`Error fetching balance for ${tokenAddress}:`, error); | ||
| return { | ||
| error: `Failed to fetch balance: ${(error as Error).message}`, | ||
| }; | ||
| } | ||
| }, | ||
| ); | ||
| if (!chain) { | ||
| throw new Error(`Unsupported chain ID: ${chainId}`); | ||
| } | ||
|
|
||
| const client = createPublicClient({ | ||
| chain, | ||
| transport: http(), | ||
| }); | ||
|
|
||
| const rawBalance = await client.readContract({ | ||
| address: tokenAddress as `0x${string}`, | ||
| abi: erc20Abi, | ||
| functionName: "balanceOf", | ||
| args: [evmAddress], | ||
| }); | ||
| return { | ||
| rawBalance, | ||
| balance: formatUnits(rawBalance, decimals), | ||
| }; | ||
| return Promise.all(balancePromises); | ||
| }; | ||
|
|
||
| return useSWR( | ||
| evmAddress ? `erc20Balance:${tokenAddress}:${evmAddress}-${chainId}` : null, | ||
| evmAddress ? fetcher : null, | ||
| { | ||
| refreshInterval: 10000, // Refresh every 10 seconds | ||
| revalidateOnFocus: false, | ||
| revalidateOnReconnect: false, | ||
| }, | ||
| ); | ||
| const key = | ||
| evmAddress && tokensToFetch && tokensToFetch.length > 0 | ||
| ? `erc-20-balances:${evmAddress}` | ||
| : null; | ||
|
|
||
| return useSWR(key, evmAddress ? fetcher : null, { | ||
| refreshInterval: 5_000, | ||
| revalidateOnFocus: false, | ||
| revalidateOnReconnect: false, | ||
| dedupingInterval: 2_000, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve type safety and error handling in batch fetching.
The batch balance fetching implementation has several concerns:
-
Mixed result types: Success and error objects are mixed in the same array (lines 68-70, 94-96) without a discriminated union type. This makes it difficult for consumers to safely handle results.
-
No explicit return type: The function lacks an explicit return type annotation, making the API contract unclear.
-
Type safety issue: The error objects
{ error: string }and success objects have different shapes but no discriminator field.
🔎 Suggested improvements
Define a discriminated union type:
type BalanceResult =
| {
success: true;
tokenAddress: string;
chainId: string;
decimals: number;
rawBalance: bigint;
balance: number;
}
| {
success: false;
tokenAddress?: string;
chainId?: string;
error: string;
};Update the function signature:
-export function useErc20BalancesByAddress(evmAddress?: Address) {
+export function useErc20BalancesByAddress(evmAddress?: Address): SWRResponse<BalanceResult[], Error> {Update error returns to include the discriminator:
return {
+ success: false,
+ tokenAddress,
+ chainId,
error: `Failed to fetch balance: ${(error as Error).message}`,
};And success returns:
return {
+ success: true,
tokenAddress,
chainId,
decimals,
rawBalance,
balance: parseFloat(formatUnits(rawBalance, decimals)),
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
hooks/evm/useErc20Balance.ts lines 44-115: the batch fetch mixes heterogeneous
success and error objects without a discriminator and lacks an explicit return
type, making consumers unsafe; define a discriminated union (e.g. BalanceResult
with success: true {tokenAddress, chainId, decimals, rawBalance, balance} and
success: false {tokenAddress?, chainId?, error}), annotate the hook and fetcher
to return Promise<BalanceResult[]>, update every early/exception return to
produce the failure shape (success: false, error, optional tokenAddress/chainId)
and every successful return to produce the success shape (success: true plus
fields), and ensure Promise.all is typed to BalanceResult[] so callers can
safely narrow on success.
| export function useErc20Image(chainId: Hex, contractAddress: Address) { | ||
| const { data, error, isLoading } = useZealousSwapTokensMetadata(); | ||
|
|
||
| const numericChainId = hexToNumber(chainId); | ||
| const isKasplex = numericChainId === kasplexMainnet.id; | ||
|
|
||
| if (!isKasplex) { | ||
| return { logoUrl: undefined, error: "Unsupported chain", isLoading: false }; | ||
| } | ||
|
|
||
| const normalizedAddress = contractAddress.toLowerCase() as Address; | ||
| const token = data?.tokens.find( | ||
| (t) => t.address.toLowerCase() === normalizedAddress, | ||
| ); | ||
|
|
||
| const logoUrl = token | ||
| ? `${ZEALOUS_SWAP_IMAGE_URL}${token.logoURI}` | ||
| : undefined; | ||
|
|
||
| return { | ||
| logoUrl, | ||
| error, | ||
| isLoading, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Inconsistent error handling for unsupported chains.
useErc20Image returns { error: "Unsupported chain" } for non-Kasplex chains (line 40), while useErc20Price returns { error: undefined } for the same scenario (lines 66-70). This inconsistency may confuse consumers of these hooks.
Consider standardizing the error handling approach—either both hooks should return an error message for unsupported chains, or neither should.
🔎 Suggested fix for consistency
Option 1: Both return undefined error (indicating "not an error, just not supported"):
if (!isKasplex) {
- return { logoUrl: undefined, error: "Unsupported chain", isLoading: false };
+ return { logoUrl: undefined, error: undefined, isLoading: false };
}Option 2: Both return an error message:
if (!isKasplex) {
return {
price: 0,
- error: undefined,
+ error: "Unsupported chain",
isLoading: false,
};
}Also applies to: 59-83
🤖 Prompt for AI Agents
hooks/evm/useZealousSwapMetadata.ts lines 33-57 (and similarly lines 59-83): the
hooks currently handle unsupported chains inconsistently (one returns error:
"Unsupported chain", the other error: undefined) — standardize to return error:
undefined for unsupported chains to match useErc20Price behavior; update the
early-return branches for non-Kasplex chains to return { logoUrl: undefined,
error: undefined, isLoading: false } (and for the other hook return its
corresponding value shape with error: undefined) so consumers see a consistent
"not supported" non-error state across both hooks.
| const kaspaBalance = | ||
| balance + | ||
| Object.values(evmKasBalances ?? {}).reduce( | ||
| (acc, { balance }) => acc + parseFloat(balance), | ||
| 0, | ||
| ); |
There was a problem hiding this comment.
Consider precision loss and add error handling for balance parsing.
The balance aggregation has potential issues:
-
Floating-point precision: Using
parseFloaton line 28 can introduce precision errors when dealing with cryptocurrency amounts. Consider using BigInt or decimal libraries for accurate financial calculations. -
No error handling: If
balanceis not a valid number string,parseFloatwill returnNaN, which will propagate through calculations silently. -
Silent failures: Failed balance fetches in
evmKasBalanceswould be included in the aggregation without indication.
🔎 Suggested improvements
const kaspaBalance =
balance +
Object.values(evmKasBalances ?? {}).reduce(
- (acc, { balance }) => acc + parseFloat(balance),
+ (acc, { balance }) => {
+ const parsed = parseFloat(balance);
+ if (isNaN(parsed)) {
+ console.error(`Invalid balance value: ${balance}`);
+ return acc;
+ }
+ return acc + parsed;
+ },
0,
);Consider using a decimal library like decimal.js or bignumber.js for precise financial calculations.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const kaspaBalance = | |
| balance + | |
| Object.values(evmKasBalances ?? {}).reduce( | |
| (acc, { balance }) => acc + parseFloat(balance), | |
| 0, | |
| ); | |
| const kaspaBalance = | |
| balance + | |
| Object.values(evmKasBalances ?? {}).reduce( | |
| (acc, { balance }) => { | |
| const parsed = parseFloat(balance); | |
| if (isNaN(parsed)) { | |
| console.error(`Invalid balance value: ${balance}`); | |
| return acc; | |
| } | |
| return acc + parsed; | |
| }, | |
| 0, | |
| ); |
🤖 Prompt for AI Agents
In hooks/usePortfolioPerformance.ts around lines 25 to 30, the aggregation uses
parseFloat which can cause floating-point precision errors and silently
propagate NaN for invalid balance strings; replace the aggregation to use a
fixed-precision library (e.g., decimal.js or bignumber.js) to sum values,
validate each evmKasBalances entry before including it (skip or treat invalid
strings as zero), and surface or log malformed entries so failures aren’t
silent; ensure the final kaspaBalance is returned/converted in the expected type
consistently.
1 participant
Pull Request Checklist
Before Submission
Description
Additional Notes
By submitting this pull request, I confirm that:
project's
license terms
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.