-
Notifications
You must be signed in to change notification settings - Fork 619
[BLD-277] Fix Server Wallets balance fetching for custom chains #8026
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
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. WalkthroughReplaces static chain resolution (defineChain) with v5 dashboard chain adapter hooks in wallet table UI and ABI processing files, updating imports and passing hook-resolved chain objects to balance and contract ABI retrieval logic without changing public APIs or signatures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as ServerWalletTableRow
participant Hook as useV5DashboardChain
participant Bal as useWalletBalance
participant RPC as RPC Provider
UI->>Hook: resolve chain(chainId)
Hook-->>UI: chain object
UI->>Bal: request balance(address, chain)
Bal->>RPC: fetch balance via chain RPC
RPC-->>Bal: balance
Bal-->>UI: balance data
Note over UI,Hook: Chain resolution moved to v5 adapter hook
sequenceDiagram
autonumber
participant HookFn as useAbiMultiFetch
participant Get as useGetV5DashboardChain
participant TW as getContract
participant RPC as RPC Provider
HookFn->>Get: resolve chain(Number(chainId))
Get-->>HookFn: chain object
HookFn->>TW: getContract(address, chain, client)
TW->>RPC: fetch ABI/contract data
RPC-->>TW: ABI/contract instance
TW-->>HookFn: contract instance/ABI
Note over HookFn,Get: Chain resolution delegated to v5 adapter
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (2 passed, 3 warnings)❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (2)
✨ Finishing Touches
🧪 Generate unit tests
Comment |
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
| // eslint-disable-next-line no-restricted-syntax | ||
| return defineChain(chainId); | ||
| }, [chainId]); | ||
| const chain = useV5DashboardChain(chainId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential behavior change with useV5DashboardChain
The switch from defineChain(chainId) to useV5DashboardChain(chainId) may introduce different handling for invalid/unsupported chain IDs:
defineChain()would attempt to create a chain object even for unknown chainsuseV5DashboardChain()might returnnullorundefinedfor unsupported chains
This could cause runtime errors in components that expect a valid chain object. Consider:
- Adding validation to handle potential
null/undefinedchain values - Ensuring
useV5DashboardChainhas equivalent fallback behavior todefineChain - Adding tests for edge cases with unsupported chain IDs
This is particularly important for the wallet balance functionality which depends on a valid chain object.
| const chain = useV5DashboardChain(chainId); | |
| const chain = useV5DashboardChain(chainId) ?? defineChain(chainId); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8026 +/- ##
=======================================
Coverage 56.62% 56.62%
=======================================
Files 904 904
Lines 58677 58677
Branches 4161 4161
=======================================
Hits 33225 33225
Misses 25346 25346
Partials 106 106
🚀 New features to boost your workflow:
|
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx (1)
150-153: Invalidate the fulluseWalletBalancekey including the wallet address
The hook’s internal key is["walletBalance", chain?.id, address](see useWalletBalance implementation), so invalidating only["walletBalance", selectedChainId]won’t match. Update to:- await queryClient.invalidateQueries({ - queryKey: ["walletBalance", selectedChainId], - }); + await queryClient.invalidateQueries({ + queryKey: ["walletBalance", selectedChainId, selectedAddress], + });Or, if you need to catch all balance queries regardless of address, use a predicate as originally suggested.
🧹 Nitpick comments (7)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.ts (3)
58-63: Consider guarding missing chain data for clearer errorsIf a custom chain isn’t loaded yet,
defineDashboardChainshould still return a chain, but if the adapter ever yields an unexpected value, the error surfaces only fromgetContract/resolveAbiFromContractApi. A fast-fail improves debuggability.- const chainObj = getChain(Number(chainId)); + const chainObj = getChain(Number(chainId)); + if (!chainObj) { + throw new Error(`Unknown chain: ${chainId}`); + }
76-93: Key by chain+address to avoid cross-chain collisions
abisByAddressandfetchedAbisare keyed only by address. Same address on different chains will overwrite each other.- const map: Record<string, { chainId: string; address: string; data?: AbiData["abi"]; error?: unknown; status: "success" | "error"; }> = {}; + const map: Record<string, { chainId: string; address: string; data?: AbiData["abi"]; error?: unknown; status: "success" | "error"; }> = {}; for (const item of queryResult.data) { - map[item.address] = item; + map[`${item.chainId}:${item.address.toLowerCase()}`] = item; } - const abis: Record<string, AbiData> = {}; + const abis: Record<string, AbiData> = {}; ... - abis[item.address] = { + abis[`${item.chainId}:${item.address.toLowerCase()}`] = { abi: abi, fetchedAt: new Date().toISOString(), status: "success", ...(type === "event" ? { events: items } : { functions: items }), };Follow-up: If downstream callers expect address-only keys, we can expose both maps or return tuples
{ key, value }.Also applies to: 95-121
48-74: Stabilize the queryKey against ordering/whitespace changes
addressesstring andchainIdsorder can produce cache misses. Usepairs(already deduped) and a normalized key.- queryKey: ["abis", chainIds, addresses, type], + queryKey: [ + "abis", + pairs.map((p) => `${p.chainId}:${p.address.toLowerCase()}`), + type, + ],apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx (4)
281-296: Gate smart account address query until chain is readyIf the adapter hasn’t produced a chain yet (initial render or slow chain data), the query may error unnecessarily. Enable only when
chainexists.- const smartAccountAddressQuery = useQuery({ + const smartAccountAddressQuery = useQuery({ queryFn: async () => { const smartAccountAddress = await predictSmartAccountAddress({ adminAddress: wallet.address, - chain: chain, + chain: chain, client: client, factoryAddress: DEFAULT_ACCOUNT_FACTORY_V0_7, }); return smartAccountAddress; }, - enabled: showSmartAccount, + enabled: showSmartAccount && !!chain, queryKey: ["smart-account-address", wallet.address, chainId], });
459-464: Handle undefined chain gracefully in balance cellOn first render, the adapter can return
undefined. Ensure UI doesn’t flash “N/A” due to a transient undefined chain by treating it as loading.- const balance = useWalletBalance({ + const balance = useWalletBalance({ address: props.address, - chain: chain, + chain: chain, client: props.client, }); + if (!chain) return <Skeleton className="h-5 w-16" />;Note: The hook still runs; this only improves the rendered state.
57-75: ExposeclassNameon the root per dashboard guidelinesComponents under apps/dashboard should accept a
classNameand apply it on the root element.export function ServerWalletsTableUI({ wallets, project, teamSlug, managementAccessToken, totalRecords, currentPage, totalPages, client, + className, }: { wallets: Wallet[]; project: Project; teamSlug: string; managementAccessToken: string | undefined; totalRecords: number; currentPage: number; totalPages: number; client: ThirdwebClient; + className?: string; }) { @@ - return ( - <div> + return ( + <div className={cn(className)}>Also applies to: 80-82
214-243: Optional: PreferNavLinkfor internal navigationThe dashboard guidelines recommend
NavLinkfor active-state handling. Consider swapping for pagination and the “Send test transaction” link in a follow-up.Also applies to: 246-259, 413-421
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx(4 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.tsapps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.tsapps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx
apps/{dashboard,playground-web}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/{dashboard,playground-web}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/*(Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in dashboard and playground apps
UseNavLinkfor internal navigation with automatic active states in dashboard and playground apps
Use Tailwind CSS only – no inline styles or CSS modules
Usecn()from@/lib/utilsfor conditional class logic
Use design system tokens (e.g.,bg-card,border-border,text-muted-foreground)
Server Components (Node edge): Start files withimport "server-only";
Client Components (browser): Begin files with'use client';
Always callgetAuthToken()to retrieve JWT from cookies on server side
UseAuthorization: Bearerheader – never embed tokens in URLs
Return typed results (e.g.,Project[],User[]) – avoidany
Wrap client-side data fetching calls in React Query (@tanstack/react-query)
Use descriptive, stablequeryKeysfor React Query cache hits
ConfigurestaleTime/cacheTimein React Query based on freshness (default ≥ 60s)
Keep tokens secret via internal API routes or server actions
Never importposthog-jsin server components
Files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.tsapps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx
apps/{dashboard,playground}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/{dashboard,playground}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/_(e.g., Button, Input, Tabs, Card)
UseNavLinkfor internal navigation to get active state handling
Use Tailwind CSS for styling; no inline styles
Merge class names withcn()from@/lib/utilsfor conditional classes
Stick to design tokens (e.g., bg-card, border-border, text-muted-foreground)
Server Components must start withimport "server-only"; usenext/headers, server‑only env, heavy data fetching, andredirect()where appropriate
Client Components must start with'use client'; handle interactivity with hooks and browser APIs
Server-side data fetching: callgetAuthToken()from cookies, sendAuthorization: Bearer <token>header, and return typed results (avoidany)
Client-side data fetching: wrap calls in React Query with descriptive, stablequeryKeysand set sensiblestaleTime/cacheTime(≥ 60s default); keep tokens secret via internal routes or server actions
Do not importposthog-jsin server components (client-side only)
Files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.tsapps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx
apps/{dashboard,playground}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Expose a
classNameprop on the root element of every component
Files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx
🧠 Learnings (6)
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Anything that consumes hooks from `tanstack/react-query` or thirdweb SDKs.
Applied to files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.tsapps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Use React Query (`tanstack/react-query`) for all client data fetching.
Applied to files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.ts
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Interactive UI that relies on hooks (`useState`, `useEffect`, React Query, wallet hooks).
Applied to files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.tsapps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to apps/{dashboard,playground-web}/**/*.{ts,tsx} : Wrap client-side data fetching calls in React Query (`tanstack/react-query`)
Applied to files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.ts
📚 Learning: 2025-05-27T19:54:55.885Z
Learnt from: MananTank
PR: thirdweb-dev/js#7177
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/erc20/erc20.tsx:15-17
Timestamp: 2025-05-27T19:54:55.885Z
Learning: The `fetchDashboardContractMetadata` function from "3rdweb-sdk/react/hooks/useDashboardContractMetadata" has internal error handlers for all promises and cannot throw errors, so external error handling is not needed when calling this function.
Applied to files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.ts
📚 Learning: 2025-07-07T21:21:47.488Z
Learnt from: saminacodes
PR: thirdweb-dev/js#7543
File: apps/portal/src/app/pay/page.mdx:4-4
Timestamp: 2025-07-07T21:21:47.488Z
Learning: In the thirdweb-dev/js repository, lucide-react icons must be imported with the "Icon" suffix (e.g., ExternalLinkIcon, RocketIcon) as required by the new linting rule, contrary to the typical lucide-react convention of importing without the suffix.
Applied to files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx
🧬 Code graph analysis (2)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.ts (1)
apps/dashboard/src/@/hooks/chains/v5-adapter.ts (1)
useGetV5DashboardChain(30-39)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx (1)
apps/dashboard/src/@/hooks/chains/v5-adapter.ts (1)
useV5DashboardChain(14-28)
⏰ 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). (3)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useAbiProcessing.ts (2)
7-7: Good move: switch to v5 dashboard chain adapterThis aligns ABI resolution with custom-chain support across the app.
32-32: Initialize adapter once per render for stable chain resolutionInstantiating
getChainvia the adapter hook here is correct and keeps the query fn clean.apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/transactions/server-wallets/wallet-table/wallet-table-ui.client.tsx (1)
51-51: Right direction: v5 chain adapter for custom-chain compatibilityUsing
useV5DashboardChainis the correct fix to make balances work on custom chains.
Merge activity
|
<!--
## title your PR with this format: "[SDK/Dashboard/Portal] Feature/Fix: Concise title for the changes"
If you did not copy the branch name from Linear, paste the issue tag here (format is TEAM-0000):
## Notes for the reviewer
Anything important to call out? Be sure to also clarify these in your comments.
## How to test
Unit tests, playground, etc.
-->
<!-- start pr-codex -->
---
## PR-Codex overview
This PR focuses on refactoring the code to utilize a new hook, `useGetV5DashboardChain`, which replaces the previous `defineChain` function in multiple files. This change enhances the code's readability and maintainability by standardizing the method of retrieving chain information.
### Detailed summary
- Replaced `defineChain` with `useGetV5DashboardChain` in `useAbiProcessing.ts`.
- Updated the `chain` variable initialization in `ServerWalletTableRow` to use `useV5DashboardChain`.
- Modified the `chain` parameter in `WalletBalanceCell` to utilize `useV5DashboardChain` instead of `defineChain`.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
<!-- end pr-codex -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Refactor**
* Unified chain handling across wallet tables and webhook ABI processing using a new dashboard adapter, improving consistency and reliability.
* Streamlined balance and smart account address resolution to reduce redundant computations and potential mismatches.
* Internal imports and hooks updated without changing public interfaces or user workflows.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
0ba43d8 to
85d6074
Compare

PR-Codex overview
This PR focuses on refactoring the code in the
useAbiProcessing.tsandwallet-table-ui.client.tsxfiles to replace the usage ofdefineChainfromthirdweb/chainswith a new custom hook,useGetV5DashboardChainanduseV5DashboardChain, improving the chain handling logic.Detailed summary
In
useAbiProcessing.ts:defineChainwithgetChainfromuseGetV5DashboardChain.In
wallet-table-ui.client.tsx:defineChainusage in favor ofuseV5DashboardChain.Summary by CodeRabbit