RAI-752 use REST token details in website - #211
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds ST0x token-details REST endpoints, typed client helpers, vault mapping, new vault fields, updated UI consumers, a dashboard fallback change, and expanded proxy/integration tests. ChangesST0x REST API token details migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(main)/dashboard/+page.svelte (1)
408-421:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not replace real holdings with synthetic zero balances on multicall failure.
The catch path currently returns valid-looking holdings with
walletBalance: 0n, which can collapse the portfolio into a false “no holdings” state during transient RPC outages. Surface the failure instead of committing incorrect balances.Suggested fix
} catch (error) { console.error('Multicall failed for wallet holdings:', error); - return $sfts.map((sft) => { - const tokenConfig = findApiTokenByAnyAddress(ALL_TOKENS, sft.address); - return { - id: sft.id, - address: tokenConfig?.address ?? sft.address, - name: tokenConfig?.name ?? sft.name, - symbol: tokenConfig?.symbol ?? sft.symbol, - walletBalance: 0n, - decimals: 18 - }; - }); + throw error instanceof Error + ? error + : new Error('Multicall failed for wallet holdings'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/`(main)/dashboard/+page.svelte around lines 408 - 421, The catch block for the multicall failure in the wallet holdings section is returning synthetic SFT data with walletBalance: 0n, which masks the actual error and causes the portfolio to incorrectly display zero holdings. Instead of returning this fake data, rethrow the error or allow it to propagate so the caller can handle the failure appropriately and display the correct error state to the user rather than committing incorrect zero balances to the portfolio.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/queries/vaults.ts`:
- Line 61: The `tokenDetailsSummaryToVault` function is hard-setting
deployTimestamp to an empty string when detail is not available, which causes
list-backed views to lose created-at data. In the function where deployTimestamp
is mapped (around line 61 in the current diff, and also in lines 83-85), replace
the fallback of empty string with logic that preserves the deployTimestamp from
the summary data when detail is unavailable. This ensures that the created-at
information is retained even when the detail object is missing, allowing
list-backed views consuming sft.deployTimestamp to access the proper timestamp
data.
In `@src/routes/`(main)/+page.svelte:
- Around line 150-152: The totalSupply field calculation directly calls BigInt()
on potentially malformed string values from sft.bridgedSupply or sft.totalShares
without validation, which will throw an error if the API returns non-numeric,
decimal, or whitespace-only strings. Replace the BigInt() call in the
formatUnits(BigInt(sft.bridgedSupply ?? sft.totalShares), 18) expression with
toBigInt() imported from src/lib/utils/tokenMath.ts, which safely handles
invalid inputs and returns null on error, preventing runtime failures during
render.
---
Outside diff comments:
In `@src/routes/`(main)/dashboard/+page.svelte:
- Around line 408-421: The catch block for the multicall failure in the wallet
holdings section is returning synthetic SFT data with walletBalance: 0n, which
masks the actual error and causes the portfolio to incorrectly display zero
holdings. Instead of returning this fake data, rethrow the error or allow it to
propagate so the caller can handle the failure appropriately and display the
correct error state to the user rather than committing incorrect zero balances
to the portfolio.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c75d3583-dd1c-4be3-a0c9-5c4b578a614f
📒 Files selected for processing (11)
src/lib/api/st0xApi.tssrc/lib/components/Sidebar.sveltesrc/lib/queries/vaults.tssrc/lib/types/OffchainAssetReceiptVault.tssrc/routes/(main)/+page.sveltesrc/routes/(main)/dashboard/+page.sveltesrc/routes/(main)/trade/[id]/+page.sveltesrc/routes/(main)/trade/[id]/proofs/+layout.sveltesrc/routes/api/st0x/[...path]/+server.tstests/integration/ui/wrapRatio.spec.tstests/lib/api/st0x-proxy.test.ts
|
Two small nits before merge:
|
Dependent PRs
Motivation
RAI-752 moves ST0x token detail data behind the REST API. The website was still reading SFT product data directly through
getSfts/getSftById, including holder lists, transfer arrays, and deposit/withdraw rows.Solution
/v1/tokens/detailsand/v1/tokens/{address}/details./api/st0xproxy with shared caching.sftsquery/store path to use REST token details while preserving the current vault-shaped UI contract.Checks
bun run test -- run tests/lib/api/st0x-proxy.test.tsbunx eslint src/lib/api/st0xApi.ts src/lib/queries/vaults.ts src/lib/types/OffchainAssetReceiptVault.ts 'src/routes/api/st0x/[...path]/+server.ts' 'src/routes/(main)/+page.svelte' 'src/routes/(main)/dashboard/+page.svelte' 'src/routes/(main)/trade/[id]/+page.svelte' 'src/routes/(main)/trade/[id]/proofs/+layout.svelte' src/lib/components/Sidebar.svelte tests/lib/api/st0x-proxy.test.ts tests/integration/ui/wrapRatio.spec.tsbunx svelte-check --no-tsconfig --ignore "tests,node_modules,.svelte-kit,build" --threshold errorgit diff --check origin/main...HEADbun run checkis still blocked locally by the existing missing@playwright/testtype-resolution errors undertests/integration/ui.Summary by CodeRabbit