[SDK] move over some utility read only endpoints to API #8005
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds API-based implementations for wallet/contract metadata reads, updates related tests to Base Sepolia fixtures, introduces @thirdweb-dev/api dependency, and tweaks repo/tooling config (.gitignore, biome). getContractMetadata becomes API-first with RPC fallback; getWalletBalance/getTokenBalance switch to API-only. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Caller Code
participant GC as getContractMetadata()
participant API as api.thirdweb.com
participant RPC as RPC/On-chain
Dev->>GC: call(options: { chain, address, client })
GC->>API: GET /contract-metadata?chainId&address (configured with clientId/secret)
alt API returns ABI
API-->>GC: { abi, compiler, language, docs... }
par Resolve optional name/symbol
GC->>RPC: name() via ERC20/721 if needed
GC->>RPC: symbol() via ERC20/721 if needed
end
GC-->>Dev: { name, symbol, abi, compiler, language, devdoc, userdoc }
else API error or no ABI
API-->>GC: error/empty
Note over GC,RPC: Fallback to legacy RPC-based metadata
GC->>RPC: contractURI()/fetch metadata JSON
GC->>RPC: name(), symbol()
GC-->>Dev: { name, symbol, ...from RPC }
end
sequenceDiagram
autonumber
actor Dev as Caller Code
participant GWB as getWalletBalance()/getTokenBalance()
participant API as api.thirdweb.com
Dev->>GWB: call({ address, chain, tokenAddress?, client })
GWB->>API: GET /wallet-balance?chainId&address[&tokenAddress]
API-->>GWB: { result: [balanceData] }
alt balanceData present
GWB-->>Dev: { chainId, tokenAddress, name, symbol, decimals, value(BigInt), displayValue }
else missing/empty
GWB-->>Dev: throw Error("No balance data")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🧪 Generate unit tests
Comment |
github-actions
bot
added
Dashboard
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. |
size-limit report 📦
|
![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 (16)
apps/dashboard/package.json (1)
2-130: Format-only churn: consider adding JSON files to your formatter scopeTo avoid future whitespace-only diffs, include package.json in your Biome/Prettier targets (or a root format script).
packages/thirdweb/src/wallets/utils/getTokenBalance.test.ts (4)
6-9: Misleading comment on test addressThe address 0x742d… is a well-known mainnet account, not “Base deployer”. Consider neutral wording or use a local test account from test/src/test-wallets.ts.
Apply:
-// Create a mock account for testing -const testAccount = { - address: "0x742d35Cc6645C0532b6C766684f4b4E99Bf87E8A", // Base deployer address -}; +// Use a deterministic test account (no balance assumptions) +const testAccount = { + address: "0x742d35Cc6645C0532b6C766684f4b4E99Bf87E8A", +};Or import from your shared fixtures:
+import { TEST_ACCOUNT_D as testAccount } from "~test/test-wallets.js";
19-24: Avoid brittle asserts on native token nameHard-coding "Sepolia Ether" may break across chain metadata updates. Assert against chain metadata instead.
- expect(result.decimals).toBe(18); - expect(result.name).toBe("Sepolia Ether"); - expect(result.symbol).toBe("ETH"); + expect(result.decimals).toBe(baseSepolia.nativeCurrency.decimals); + expect(result.name).toBe(baseSepolia.nativeCurrency.name); + expect(result.symbol).toBe(baseSepolia.nativeCurrency.symbol);
27-36: Prefer MSW over live API in unit testsThese tests rely on network + TW_SECRET_KEY, which can flake. Use MSW to stub @thirdweb-dev/api responses and keep tests deterministic.
I can sketch MSW handlers for the wallet balance endpoint if helpful.
38-44: USDC decimals assert is fine; add minimal shape checksSince value/displayValue types are asserted, also consider asserting symbol/name non-empty to guard empty API payloads.
- expect(result.symbol).toBeDefined(); - expect(result.name).toBeDefined(); + expect(result.symbol).toMatch(/\S+/); + expect(result.name).toMatch(/\S+/);packages/thirdweb/src/wallets/utils/getWalletBalance.ts (5)
1-4: Lazy-load @thirdweb-dev/api to keep bundles leanImporting the API client at top-level pulls it into bundles even when unused. Dynamic import inside the function avoids upfront cost.
-import { - getWalletBalance as apiGetWalletBalance, - configure, -} from "@thirdweb-dev/api"; +// Defer loading heavy deps until neededInside the function, before using it:
+ const { getWalletBalance: apiGetWalletBalance, configure } = + await import("@thirdweb-dev/api");
47-55: Support cancellation for better UXConsider accepting an optional AbortSignal in options and passing it to the underlying request (if supported) to allow callers to cancel in-flight balance fetches.
Additional changes outside this hunk:
// Extend options export type GetWalletBalanceOptions = { // ... signal?: AbortSignal; };And when calling:
const response = await apiGetWalletBalance( { path: { address }, query: { chainId: [chain.id], ...(tokenAddress && { tokenAddress }) } }, // second param only if the API supports it; otherwise pass { signal } within the same object if that's the shape { signal: options.signal }, );
57-66: Improve error messages with contextInclude address/chain/token to ease debugging.
- if (!response.data?.result || response.data.result.length === 0) { - throw new Error("No balance data returned from API"); - } + if (!response.data?.result || response.data.result.length === 0) { + throw new Error( + `No balance data returned from API for address ${address} on chain ${chain.id}${tokenAddress ? ` (token ${tokenAddress})` : ""}`, + ); + } - if (!balanceData) { - throw new Error("Balance data not found for the specified chain"); - } + if (!balanceData) { + throw new Error( + `Balance data not found for address ${address} on chain ${chain.id}${tokenAddress ? ` (token ${tokenAddress})` : ""}`, + ); + }
68-77: Guard BigInt conversionDefensive parsing avoids cryptic throws if the API shape changes.
- value: BigInt(balanceData.value), + value: + (() => { + try { + return BigInt(balanceData.value); + } catch { + throw new Error( + `Invalid balance value "${balanceData.value}" for address ${address} on chain ${chain.id}`, + ); + } + })(),
21-35: Add visibility tag to TSDocPublic symbol docs require a custom tag. You already use @walletUtils; if this is experimental, add @beta/@experimental for clarity.
/** * Retrieves the balance of a token or native currency for a given wallet. + * @beta * @param options - The options for retrieving the token balance.packages/thirdweb/src/wallets/utils/getTokenBalance.ts (3)
48-56: Normalize and validate tokenAddress before sending to APIEnsure
tokenAddressis well-formed to avoid API cache misses or 400s. Normalize to lowercase and optionally validate.- query: { - chainId: [chain.id], - ...(tokenAddress && { tokenAddress }), - }, + query: { + chainId: [chain.id], + ...(tokenAddress && { tokenAddress: tokenAddress.toLowerCase() }), + },Optionally, add an address guard:
// at function top (after destructure) import { isAddress } from "viem"; if (tokenAddress && !isAddress(tokenAddress)) { throw new Error(`Invalid tokenAddress: ${tokenAddress}`); }
58-67: Defensive result selection and clearer errors
- If the API ever returns multiple results, assert the selected item matches
chain.id.- Improve error messages with context to aid debugging.
- if (!response.data?.result || response.data.result.length === 0) { - throw new Error("No balance data returned from API"); - } + if (!response.data?.result || response.data.result.length === 0) { + throw new Error(`No balance data returned from API for chainId=${chain.id}`); + } - const balanceData = response.data.result[0]; + const balanceData = response.data.result[0]; - if (!balanceData) { - throw new Error("Balance data not found for the specified chain"); - } + if (!balanceData) { + throw new Error(`Balance data not found for chainId=${chain.id}`); + } + if (typeof balanceData.chainId === "number" && balanceData.chainId !== chain.id) { + throw new Error(`Mismatched chainId: expected ${chain.id}, got ${balanceData.chainId}`); + }
71-76: Guard BigInt parsingEnsure
valueis a stringified integer beforeBigInt, avoiding accidental decimal or non-numeric inputs.- value: BigInt(balanceData.value), + value: BigInt(String(balanceData.value)),packages/thirdweb/src/extensions/common/read/getContractMetadata.test.ts (1)
10-21: Good shift to stable assertions; consider stubbing network for determinismThe partial/object and type checks are robust. To avoid dependence on external API/RPC and
TW_SECRET_KEY, add an MSW stub for the metadata endpoint (and optionally thename/symbolRPC calls) so the test is deterministic.packages/thirdweb/src/extensions/common/read/getContractMetadata.ts (2)
58-69: Type the ABI entries to avoidanyand brittle property accessDefine a minimal ABI item type for functions and narrow checks instead of
(item: any). This improves readability and safety.type AbiItem = { type: string; name?: string; inputs?: unknown[] }; const isZeroArgFn = (i: AbiItem, n: "name" | "symbol") => i.type === "function" && i.name === n && (i.inputs?.length ?? 0) === 0; const nameFunc = (abi as AbiItem[]).find((i) => isZeroArgFn(i, "name")); const symbolFunc = (abi as AbiItem[]).find((i) => isZeroArgFn(i, "symbol"));
93-95: Prefer internal logger overconsole.debugin library code
console.debugin library paths is noisy and hard to control. Route through an internal logger or gate with a verbose flag.
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.gitignore(1 hunks)apps/dashboard/package.json(1 hunks)packages/thirdweb/package.json(1 hunks)packages/thirdweb/src/extensions/common/read/getContractMetadata.test.ts(1 hunks)packages/thirdweb/src/extensions/common/read/getContractMetadata.ts(2 hunks)packages/thirdweb/src/wallets/utils/getTokenBalance.test.ts(1 hunks)packages/thirdweb/src/wallets/utils/getTokenBalance.ts(2 hunks)packages/thirdweb/src/wallets/utils/getWalletBalance.test.ts(1 hunks)packages/thirdweb/src/wallets/utils/getWalletBalance.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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:
packages/thirdweb/src/extensions/common/read/getContractMetadata.test.tspackages/thirdweb/src/wallets/utils/getTokenBalance.test.tspackages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.tspackages/thirdweb/src/extensions/common/read/getContractMetadata.tspackages/thirdweb/src/wallets/utils/getTokenBalance.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co‑locate tests asfoo.test.ts(x)next to the implementation
Use real function invocations with stub data; avoid brittle mocks
Use MSW to intercept HTTP calls for network interactions; mock only hard‑to‑reproduce scenarios
Keep tests deterministic and side‑effect free; use Vitest
Files:
packages/thirdweb/src/extensions/common/read/getContractMetadata.test.tspackages/thirdweb/src/wallets/utils/getTokenBalance.test.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/extensions/common/read/getContractMetadata.test.tspackages/thirdweb/src/wallets/utils/getTokenBalance.test.tspackages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.tspackages/thirdweb/src/extensions/common/read/getContractMetadata.tspackages/thirdweb/src/wallets/utils/getTokenBalance.ts
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}: Every public symbol must have comprehensive TSDoc with at least one compiling@exampleand a custom tag (@beta,@internal,@experimental, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf"))
Files:
packages/thirdweb/src/extensions/common/read/getContractMetadata.test.tspackages/thirdweb/src/wallets/utils/getTokenBalance.test.tspackages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.tspackages/thirdweb/src/extensions/common/read/getContractMetadata.tspackages/thirdweb/src/wallets/utils/getTokenBalance.ts
packages/thirdweb/src/wallets/**
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/wallets/**: UnifiedWalletandAccountinterfaces in wallet architecture
Support for in-app wallets (social/email login)
Smart wallets with account abstraction
EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Files:
packages/thirdweb/src/wallets/utils/getTokenBalance.test.tspackages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.tspackages/thirdweb/src/wallets/utils/getTokenBalance.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Track bundle budgets via
package.json#size-limit
Files:
apps/dashboard/package.jsonpackages/thirdweb/package.json
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
📚 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 test/src/test-wallets.ts : Predefined test accounts are in `test/src/test-wallets.ts`
Applied to files:
packages/thirdweb/src/wallets/utils/getTokenBalance.test.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
📚 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 **/*.test.{ts,tsx} : Use `FORKED_ETHEREUM_CHAIN` for mainnet interactions and `ANVIL_CHAIN` for isolated tests
Applied to files:
packages/thirdweb/src/wallets/utils/getTokenBalance.test.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
📚 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 packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Applied to files:
packages/thirdweb/src/wallets/utils/getTokenBalance.test.tspackages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
📚 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 packages/thirdweb/src/wallets/** : Unified `Wallet` and `Account` interfaces in wallet architecture
Applied to files:
packages/thirdweb/src/wallets/utils/getWalletBalance.ts
🧬 Code graph analysis (2)
packages/thirdweb/src/wallets/utils/getTokenBalance.test.ts (2)
packages/thirdweb/src/wallets/utils/getTokenBalance.ts (1)
getTokenBalance(37-77)packages/thirdweb/src/exports/chains.ts (1)
baseSepolia(18-18)
packages/thirdweb/src/wallets/utils/getWalletBalance.test.ts (2)
packages/thirdweb/src/wallets/utils/getWalletBalance.ts (1)
getWalletBalance(36-78)packages/thirdweb/src/exports/chains.ts (1)
baseSepolia(18-18)
⏰ 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). (8)
- GitHub Check: Build Packages
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
.gitignore (1)
37-37: LGTM: sensible ignore for ephemeral .tmp dirsThis will ignore ".tmp" directories at any depth. No issues.
packages/thirdweb/package.json (1)
26-26: Dependency add looks goodAdding @thirdweb-dev/api aligns with the API-first utilities in this PR.
| import { | ||
| getContractMetadata as apiGetContractMetadata, | ||
| configure, | ||
| } from "@thirdweb-dev/api"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid global configure() races; centralize auth similar to balance util
Same concern as in balance utils: configure() is global and can race across concurrent clients. Use a shared ensureApiConfigured(options.contract.client) helper (see suggestion in getTokenBalance.ts) to dedupe and avoid cross-request credential bleed.
- // Configure the API client
- configure({
- clientId: options.contract.client.clientId,
- secretKey: options.contract.client.secretKey,
- });
+ ensureApiConfigured(options.contract.client);Also applies to: 32-37
🤖 Prompt for AI Agents
In packages/thirdweb/src/extensions/common/read/getContractMetadata.ts around
lines 1-4 (and also affecting lines 32-37), the file currently calls the global
configure() from @thirdweb-dev/api which can race across concurrent clients and
leak credentials; replace direct configure() usage with a centralized helper
(e.g., ensureApiConfigured(options.contract.client)) that initializes API auth
for the given client idempotently and safely, import and call that helper before
any apiGetContractMetadata calls, and remove any other global configure()
invocations so auth setup is deduplicated and scoped to the client to prevent
cross-request credential bleed.
| if (response.data?.result?.output?.abi) { | ||
| // Extract name and symbol from ABI or devdoc/userdoc | ||
| const abi = response.data.result.output.abi; | ||
| const devdoc = response.data.result.output.devdoc; | ||
| const userdoc = response.data.result.output.userdoc; | ||
|
|
||
| // Try to find name and symbol from ABI functions | ||
| let contractName = ""; | ||
| let contractSymbol = ""; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Early return can emit empty name/symbol; gate return or merge with fallback
If name()/symbol() RPC calls fail, contractName/contractSymbol remain empty, yet the function returns early with empty strings. Only return early if at least one is resolved; otherwise, let the RPC/URI fallback path run.
- return {
- name: contractName,
- symbol: contractSymbol,
- abi: abi,
- compiler: response.data.result.compiler,
- language: response.data.result.language,
- devdoc: devdoc,
- userdoc: userdoc,
- };
+ if (contractName || contractSymbol) {
+ return {
+ name: contractName,
+ symbol: contractSymbol,
+ abi,
+ compiler: response.data.result.compiler,
+ language: response.data.result.language,
+ devdoc,
+ userdoc,
+ };
+ }
+ // No name/symbol resolvable via ABI/RPC; fall through to fallback below.Optional improvement: capture abi/devdoc/userdoc/compiler/language into a local apiExtras and merge them into the final return after the fallback so you don’t lose the API payload when taking the fallback path.
Also applies to: 71-79, 82-92
🤖 Prompt for AI Agents
In packages/thirdweb/src/extensions/common/read/getContractMetadata.ts around
lines 47-55 (and similarly at 71-79 and 82-92), the code currently performs an
early return even when contractName/contractSymbol are empty; change the logic
so you only return early if at least one of name or symbol was successfully
resolved (non-empty) — otherwise do not return and allow the RPC/URI fallback
path to run. Additionally, capture abi, devdoc, userdoc, compiler and language
into a local apiExtras object and ensure you merge apiExtras into the final
return value after the fallback path (so the API payload is preserved when
falling back).
| getWalletBalance as apiGetWalletBalance, | ||
| configure, | ||
| } from "@thirdweb-dev/api"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Global configure in a hot path can race across clients; avoid mutable global auth
configure() mutates module-global state. Concurrent calls with different client credentials can interleave and leak headers across requests. Move auth to per-request (preferred), or at least dedupe per client and detect conflicting credentials.
Apply this change locally:
- // Configure the API client with credentials from the thirdweb client
- configure({
- clientId: client.clientId,
- secretKey: client.secretKey,
- });
+ // Ensure API auth is set for this client (no-ops if already set for same creds)
+ ensureApiConfigured(client);Add a shared helper (place once, e.g., src/internal/api/config.ts, then import here):
// internal/api/config.ts
import { configure } from "@thirdweb-dev/api";
import type { ThirdwebClient } from "../../client/client.js";
let activeClientId: string | undefined;
let activeSecret: string | undefined;
export function ensureApiConfigured(client: ThirdwebClient): void {
// Fast path: same creds already applied
if (client.clientId === activeClientId && client.secretKey === activeSecret) return;
// If different creds are already active, prefer explicitness:
// either throw or overwrite with a warning. Choose policy; for now, overwrite.
if (activeClientId && (client.clientId !== activeClientId || client.secretKey !== activeSecret)) {
// Consider routing these through a logger if available
// console.warn("Switching @thirdweb-dev/api credentials at runtime.");
}
configure({ clientId: client.clientId, secretKey: client.secretKey });
activeClientId = client.clientId;
activeSecret = client.secretKey;
}Also applies to: 42-47
| it("should work for native currency", async () => { | ||
| // Use a known address with ETH on Base Sepolia | ||
| // This is a public address that should have some ETH for testing | ||
| const testAddress = "0x742d35Cc6645C0532b6C766684f4b4E99Bf87E8A"; // Base deployer address | ||
|
|
||
| const result = await getWalletBalance({ | ||
| address: TEST_ACCOUNT_A.address, | ||
| chain: ANVIL_CHAIN, | ||
| address: testAddress, | ||
| chain: baseSepolia, | ||
| client: TEST_CLIENT, | ||
| tokenAddress: erc20Address, | ||
| }); | ||
| const expectedDecimal = 18; | ||
|
|
||
| expect(result).toBeDefined(); | ||
| expect(result.decimals).toBe(expectedDecimal); | ||
| expect(result.symbol).toBeDefined(); | ||
| expect(result.name).toBeDefined(); | ||
| expect(result.value).toBe(BigInt(amount) * 10n ** BigInt(expectedDecimal)); | ||
| expect(result.displayValue).toBe(amount.toString()); | ||
| expect(result.chainId).toBe(84532); // Base Sepolia chain ID | ||
| expect(result.decimals).toBe(18); | ||
| expect(result.symbol).toBe("ETH"); | ||
| expect(result.name).toBe("Sepolia Ether"); | ||
| expect(result.tokenAddress).toBeDefined(); | ||
| expect(result.value).toBeTypeOf("bigint"); | ||
| expect(result.displayValue).toBeTypeOf("string"); | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Make the native-currency test deterministic; relax brittle assertions
This test depends on live Base Sepolia state and a hard-coded address (comment also mislabels it as “Base deployer”). It will flake and can fail if the address has 0 balance or API semantics change. Also, native balances may not include tokenAddress.
- Stub the API using MSW and assert on the contract instead of the network.
- Relax assertions for
nameandtokenAddress(native may omittokenAddress).
Minimal assertion tweak:
- expect(result.name).toBe("Sepolia Ether");
- expect(result.tokenAddress).toBeDefined();
+ expect(result.name).toBeTypeOf("string");
+ // For native, API may omit tokenAddress; don't assert presence.Example MSW handler you can co-locate in this test (or shared test setup):
// at top of file
import { http, HttpResponse } from "msw";
import { setupServer } from "msw/node";
const server = setupServer(
http.get(/\/wallets\/.+\/balance$/, () =>
HttpResponse.json({
result: [
{
chainId: 84532,
decimals: 18,
displayValue: "1.2345",
symbol: "ETH",
name: "Sepolia Ether",
tokenAddress: null,
value: "1234500000000000000",
},
],
}),
),
);
beforeAll(() => server.listen());
afterAll(() => server.close());
afterEach(() => server.resetHandlers());Run with TW_SECRET_KEY unset to ensure this path is used.
Stub network calls and relax brittle native-currency assertions
- Avoid hitting live Base Sepolia with a hard-coded address (0x742d35Cc6645C…); flakes if balance changes or API omits native fields.
- Co-locate an MSW handler to stub
GET /wallets/:address/balanceand assert on its JSON payload. - Replace strict
nameandtokenAddresschecks with:
- expect(result.name).toBe("Sepolia Ether");
- expect(result.tokenAddress).toBeDefined();
+ expect(result.name).toBeTypeOf("string");
+ // native may omit tokenAddress📝 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.
| it("should work for native currency", async () => { | |
| // Use a known address with ETH on Base Sepolia | |
| // This is a public address that should have some ETH for testing | |
| const testAddress = "0x742d35Cc6645C0532b6C766684f4b4E99Bf87E8A"; // Base deployer address | |
| const result = await getWalletBalance({ | |
| address: TEST_ACCOUNT_A.address, | |
| chain: ANVIL_CHAIN, | |
| address: testAddress, | |
| chain: baseSepolia, | |
| client: TEST_CLIENT, | |
| tokenAddress: erc20Address, | |
| }); | |
| const expectedDecimal = 18; | |
| expect(result).toBeDefined(); | |
| expect(result.decimals).toBe(expectedDecimal); | |
| expect(result.symbol).toBeDefined(); | |
| expect(result.name).toBeDefined(); | |
| expect(result.value).toBe(BigInt(amount) * 10n ** BigInt(expectedDecimal)); | |
| expect(result.displayValue).toBe(amount.toString()); | |
| expect(result.chainId).toBe(84532); // Base Sepolia chain ID | |
| expect(result.decimals).toBe(18); | |
| expect(result.symbol).toBe("ETH"); | |
| expect(result.name).toBe("Sepolia Ether"); | |
| expect(result.tokenAddress).toBeDefined(); | |
| expect(result.value).toBeTypeOf("bigint"); | |
| expect(result.displayValue).toBeTypeOf("string"); | |
| }); | |
| it("should work for native currency", async () => { | |
| // Use a known address with ETH on Base Sepolia | |
| // This is a public address that should have some ETH for testing | |
| const testAddress = "0x742d35Cc6645C0532b6C766684f4b4E99Bf87E8A"; // Base deployer address | |
| const result = await getWalletBalance({ | |
| address: testAddress, | |
| chain: baseSepolia, | |
| client: TEST_CLIENT, | |
| }); | |
| expect(result).toBeDefined(); | |
| expect(result.chainId).toBe(84532); // Base Sepolia chain ID | |
| expect(result.decimals).toBe(18); | |
| expect(result.symbol).toBe("ETH"); | |
| expect(result.name).toBeTypeOf("string"); | |
| // native may omit tokenAddress | |
| expect(result.value).toBeTypeOf("bigint"); | |
| expect(result.displayValue).toBeTypeOf("string"); | |
| }); |
| it("should work for ERC20 token", async () => { | ||
| // Use a known ERC20 token on Base Sepolia | ||
| // Using the official test token: Base Sepolia USD Coin | ||
| const usdcAddress = "0x036CbD53842c5426634e7929541eC2318f3dCF7e"; // USDC on Base Sepolia | ||
| const testAddress = "0x742d35Cc6645C0532b6C766684f4b4E99Bf87E8A"; // Address with potential balance | ||
|
|
||
| const result = await getWalletBalance({ | ||
| address: TEST_ACCOUNT_A.address, | ||
| chain: ANVIL_CHAIN, | ||
| address: testAddress, | ||
| chain: baseSepolia, | ||
| client: TEST_CLIENT, | ||
| tokenAddress: usdcAddress, | ||
| }); | ||
|
|
||
| expect(result).toBeDefined(); | ||
| expect(result.decimals).toBe(18); | ||
| expect(result.chainId).toBe(84532); | ||
| expect(result.decimals).toBe(6); // USDC has 6 decimals | ||
| expect(result.symbol).toBeDefined(); | ||
| expect(result.value).toBeGreaterThan(0n); | ||
| expect(result.displayValue).toBeDefined(); | ||
| expect(result.name).toBeDefined(); | ||
| expect(result.tokenAddress).toBe(usdcAddress); | ||
| expect(result.value).toBeTypeOf("bigint"); | ||
| expect(result.displayValue).toBeTypeOf("string"); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ERC20 test also relies on live chain and a possibly incorrect USDC address
Using a real address with unknown holdings on Base Sepolia is flaky. Keep decimals check, but stub the API and assert types and equality against the stubbed payload instead of chain state.
Suggested tweaks:
- expect(result.symbol).toBeDefined();
- expect(result.name).toBeDefined();
+ expect(typeof result.symbol).toBe("string");
+ expect(typeof result.name).toBe("string");And in the MSW stub for this case, return a second handler that matches the tokenAddress query and returns a 6-decimal payload for USDC so you can keep:
expect(result.decimals).toBe(6);
expect(result.tokenAddress).toBe(usdcAddress.toLowerCase());🤖 Prompt for AI Agents
In packages/thirdweb/src/wallets/utils/getWalletBalance.test.ts around lines
28-49, the ERC20 test depends on a live chain and an unreliable address; replace
the real-chain call with a stubbed API response: add an MSW handler that matches
the tokenAddress query and returns a deterministic ERC20 payload (symbol, name,
decimals: 6, tokenAddress in lowercase, value/displayValue) and use TEST_CLIENT
configured to hit MSW; keep the decimals assertion but assert tokenAddress
equals the stubbed usdcAddress.toLowerCase() and assert types/values against the
stubbed payload rather than live chain state.
| configure({ | ||
| clientId: client.clientId, | ||
| secretKey: client.secretKey, | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Do not pass secretKey in browser; also avoid global mutable config
Leaking secretKey to client runtimes is a security risk, and global configure() is unsafe with multiple clients in the same process (race/bleed across requests).
- configure({
- clientId: client.clientId,
- secretKey: client.secretKey,
- });
+ // Only include secretKey on server; avoid leaking in browser bundles
+ const config: { clientId?: string; secretKey?: string } = {};
+ if (client.clientId) config.clientId = client.clientId;
+ if (typeof window === "undefined" && client.secretKey) {
+ config.secretKey = client.secretKey;
+ }
+ configure(config);Run this scan to find other global configure() usages that may suffer the same issue:
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts --type=tsx -C2 '\bconfigure\s*\('Length of output: 27
🏁 Script executed:
#!/bin/bash
rg -nP '\bconfigure\s*\(' -g '*.ts' -g '*.tsx' -C2Length of output: 4345
Avoid leaking secretKey in browser; guard configure calls server-side
- In
getWalletBalance.ts,getTokenBalance.ts, andgetContractMetadata.ts, build a localconfigobject that always includesclientIdbut only addssecretKeywhentypeof window === 'undefined', then callconfigure(config)instead of passing both keys unconditionally. - This prevents shipping your
secretKeyin browser bundles and removes reliance on a global mutable config. Tests running in Node (e.g.in-app-gateway.test.ts) will still receivesecretKeyas expected. - Double-check that no other client-side code calls
configure({ secretKey }).
🤖 Prompt for AI Agents
In packages/thirdweb/src/wallets/utils/getWalletBalance.ts around lines 42-45,
the code calls configure with clientId and secretKey unconditionally which can
leak secretKey into browser bundles; change to build a local config object that
always sets clientId and only adds secretKey when typeof window === 'undefined',
then call configure(config) instead of passing both keys; apply the same pattern
to getTokenBalance.ts and getContractMetadata.ts and scan for any other
client-side code calling configure({ secretKey }) to remove reliance on a global
mutable config.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
packages/thirdweb/src/wallets/utils/getWalletBalance.ts (1)
41-45: Do not pass secretKey in browser; avoid global mutable configure across requestsPassing secretKey unconditionally risks leaking it to browser bundles; global configure() can bleed creds across concurrent requests.
- // Configure the API client with credentials from the thirdweb client - configure({ - clientId: client.clientId, - secretKey: client.secretKey, - }); + // Configure API client safely (no secretKey in browser; minimize global state) + const { getWalletBalance: apiGetWalletBalance, configure } = + await import("@thirdweb-dev/api"); + const cfg: { clientId?: string; secretKey?: string } = {}; + if (client.clientId) cfg.clientId = client.clientId; + // Only attach secretKey on server runtimes + if (typeof window === "undefined" && client.secretKey) { + cfg.secretKey = client.secretKey; + } + configure(cfg);Optional: if the API supports per-call config instead of global state, prefer that to eliminate race conditions.
packages/thirdweb/src/wallets/utils/getWalletBalance.test.ts (4)
6-16: Make tests deterministic: stub the API with MSW; don’t gate on secretsThese tests hit live infra and are flaky. Stub
GET /wallets/:address/balancewith MSW and run unconditionally.Add an MSW server alongside this test (minimal sketch):
import { http, HttpResponse } from "msw"; import { setupServer } from "msw/node"; const server = setupServer( // native http.get(/\/wallets\/[^/]+\/balance$/, ({ request }) => { const url = new URL(request.url); const token = url.searchParams.get("tokenAddress"); if (!token) { return HttpResponse.json({ result: [{ chainId: 84532, decimals: 18, symbol: "ETH", name: "Sepolia Ether", tokenAddress: null, value: "1234500000000000000", displayValue: "1.2345", }], }); } // ERC20 return HttpResponse.json({ result: [{ chainId: 84532, decimals: 6, symbol: "USDC", name: "USD Coin", tokenAddress: token.toLowerCase(), value: "42", displayValue: "0.000042", }], }); }), ); beforeAll(() => server.listen()); afterAll(() => server.close()); afterEach(() => server.resetHandlers());Then remove
runIf(process.env.TW_SECRET_KEY)so the unit tests run offline.
19-25: Relax brittle native-asset assertions; tokenAddress may be null/omittedAPI may omit
tokenAddressfor native. Don’t assert presence; assert types instead of exactname.- expect(result.decimals).toBe(18); - expect(result.symbol).toBe("ETH"); - expect(result.name).toBe("Sepolia Ether"); - expect(result.tokenAddress).toBeDefined(); + expect(result.decimals).toBe(18); + expect(typeof result.symbol).toBe("string"); + expect(typeof result.name).toBe("string"); + // native may omit tokenAddress; don't assert presence
28-39: Stub ERC20 path too; don’t depend on live USDC at a possibly empty walletMirror the native-case MSW stub with a handler that returns USDC-like payload (6 decimals) when
tokenAddressis present; assert against the stub.
44-47: Compare tokenAddress case-insensitively; assert types for symbol/nameAPIs often normalize to lowercase; current strict match can fail.
- expect(result.symbol).toBeDefined(); - expect(result.name).toBeDefined(); - expect(result.tokenAddress).toBe(usdcAddress); + expect(typeof result.symbol).toBe("string"); + expect(typeof result.name).toBe("string"); + expect(result.tokenAddress?.toLowerCase()).toBe(usdcAddress.toLowerCase());
🧹 Nitpick comments (7)
biome.json (1)
38-41: Remove redundant formatter override in overridesTop-level formatter already enforces indentStyle. Keeping a duplicate per-file override for package.json adds noise without benefit.
"assist": { "actions": { "source": { "useSortedKeys": "off" } } }, - "formatter": { - "enabled": true, - "indentStyle": "space" - }, "includes": ["package.json"]packages/thirdweb/biome.json (1)
14-17: Drop duplicate formatter block inside overridesIndent settings are already defined globally; no special formatting is needed for package.json here.
{ "assist": { "actions": { "source": { "useSortedKeys": "off" } } }, - "formatter": { - "enabled": true, - "indentStyle": "space" - }, "includes": ["package.json"] }packages/thirdweb/src/wallets/utils/getWalletBalance.ts (2)
2-5: Consider lazy-loading @thirdweb-dev/api to keep bundles leanImporting the API client at top-level can bloat initial bundles; dynamically import inside the async function.
-import { - getWalletBalance as apiGetWalletBalance, - configure, -} from "@thirdweb-dev/api"; +// lazy-load to reduce initial bundle size +// (tree-shaken and code-split in consumer apps)And inside the function (see other comment) dynamically import:
const { getWalletBalance: apiGetWalletBalance, configure } = await import("@thirdweb-dev/api");
68-77: Harden value parsing and normalize native token addressGuard against malformed values and normalize zero-address to undefined for native token to match existing GetBalanceResult expectations.
- return { - chainId: balanceData.chainId, - decimals: balanceData.decimals, - displayValue: balanceData.displayValue, - name: balanceData.name, - symbol: balanceData.symbol, - tokenAddress: balanceData.tokenAddress, - value: BigInt(balanceData.value), - }; + let bigValue: bigint; + try { + bigValue = BigInt(balanceData.value); + } catch { + throw new Error( + `Invalid balance value for address=${address} chainId=${chain.id}: ${balanceData.value}`, + ); + } + const ZERO = "0x0000000000000000000000000000000000000000"; + return { + chainId: Number(balanceData.chainId), + decimals: balanceData.decimals, + displayValue: balanceData.displayValue, + name: balanceData.name, + symbol: balanceData.symbol, + tokenAddress: + balanceData.tokenAddress && + balanceData.tokenAddress.toLowerCase() !== ZERO + ? balanceData.tokenAddress + : undefined, + value: bigValue, + };packages/thirdweb/src/wallets/utils/getWalletBalance.test.ts (3)
3-3: Prefer central chain export to avoid deep import couplingImport
baseSepoliafrom the public/central export to reduce fragility if internal paths change.-import { baseSepolia } from "../../chains/chain-definitions/base-sepolia.js"; +import { baseSepolia } from "../../exports/chains.js";
8-11: Fix misleading comment and avoid relying on a “known” live addressThis isn’t the “Base deployer” and may have zero balance on Base Sepolia. With MSW, the specific address doesn’t matter—use a neutral label.
- // Use a known address with ETH on Base Sepolia - // This is a public address that should have some ETH for testing - const testAddress = "0x742d35Cc6645C0532b6C766684f4b4E99Bf87E8A"; // Base deployer address + const testAddress = "0x1111111111111111111111111111111111111111"; // dummy for stubbed test
41-49: Add a negative-path test for empty API resultCover the error branch (“No balance data returned from API”) by stubbing
{ result: [] }and assertingrejects.toThrow.
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.gitignore(1 hunks)biome.json(1 hunks)packages/thirdweb/biome.json(2 hunks)packages/thirdweb/package.json(1 hunks)packages/thirdweb/src/extensions/common/read/getContractMetadata.test.ts(1 hunks)packages/thirdweb/src/extensions/common/read/getContractMetadata.ts(2 hunks)packages/thirdweb/src/wallets/utils/getTokenBalance.test.ts(1 hunks)packages/thirdweb/src/wallets/utils/getTokenBalance.ts(2 hunks)packages/thirdweb/src/wallets/utils/getWalletBalance.test.ts(1 hunks)packages/thirdweb/src/wallets/utils/getWalletBalance.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- packages/thirdweb/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/thirdweb/src/extensions/common/read/getContractMetadata.test.ts
- packages/thirdweb/src/wallets/utils/getTokenBalance.ts
- packages/thirdweb/src/wallets/utils/getTokenBalance.test.ts
- packages/thirdweb/src/extensions/common/read/getContractMetadata.ts
🧰 Additional context used
📓 Path-based instructions (6)
biome.json
📄 CodeRabbit inference engine (CLAUDE.md)
Biome is the primary linter/formatter; rules are defined in
biome.jsonBiome governs formatting and linting; project rules live in biome.json
Files:
biome.json
**/*.{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:
packages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
packages/thirdweb/src/wallets/**
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/wallets/**: UnifiedWalletandAccountinterfaces in wallet architecture
Support for in-app wallets (social/email login)
Smart wallets with account abstraction
EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Files:
packages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}: Every public symbol must have comprehensive TSDoc with at least one compiling@exampleand a custom tag (@beta,@internal,@experimental, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf"))
Files:
packages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co‑locate tests asfoo.test.ts(x)next to the implementation
Use real function invocations with stub data; avoid brittle mocks
Use MSW to intercept HTTP calls for network interactions; mock only hard‑to‑reproduce scenarios
Keep tests deterministic and side‑effect free; use Vitest
Files:
packages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Unified `Wallet` and `Account` interfaces in wallet architecture
📚 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 biome.json : Biome is the primary linter/formatter; rules are defined in `biome.json`
Applied to files:
biome.jsonpackages/thirdweb/biome.json
📚 Learning: 2025-08-29T15:37:38.513Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T15:37:38.513Z
Learning: Applies to biome.json : Biome governs formatting and linting; project rules live in biome.json
Applied to files:
biome.jsonpackages/thirdweb/biome.json
📚 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 packages/thirdweb/src/wallets/** : Unified `Wallet` and `Account` interfaces in wallet architecture
Applied to files:
packages/thirdweb/src/wallets/utils/getWalletBalance.ts
📚 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 packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Applied to files:
packages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
📚 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 test/src/test-wallets.ts : Predefined test accounts are in `test/src/test-wallets.ts`
Applied to files:
packages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
📚 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 **/*.test.{ts,tsx} : Use `FORKED_ETHEREUM_CHAIN` for mainnet interactions and `ANVIL_CHAIN` for isolated tests
Applied to files:
packages/thirdweb/src/wallets/utils/getWalletBalance.test.ts
🧬 Code graph analysis (1)
packages/thirdweb/src/wallets/utils/getWalletBalance.test.ts (2)
packages/thirdweb/src/wallets/utils/getWalletBalance.ts (1)
getWalletBalance(36-78)packages/thirdweb/src/exports/chains.ts (1)
baseSepolia(18-18)
⏰ 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). (8)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Build Packages
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/thirdweb/biome.json (2)
4-4: Setting"root": falsehere is correct for a workspace packagePrevents this package config from shadowing the repo root rules.
3-5: Ignore invalid extends suggestion The"extends": "//"microsyntax in Biome v2 marks the config as nested (root: false) and extends the root configuration in monorepos (biomejs.dev). No changes required.Likely an incorrect or invalid review comment.
| if (!response.data?.result || response.data.result.length === 0) { | ||
| throw new Error("No balance data returned from API"); | ||
| } | ||
| // native token case | ||
| const rpcRequest = getRpcClient({ chain, client }); | ||
|
|
||
| const [nativeSymbol, nativeDecimals, nativeName, nativeBalance] = | ||
| await Promise.all([ | ||
| getChainSymbol(chain), | ||
| getChainDecimals(chain), | ||
| getChainNativeCurrencyName(chain), | ||
| eth_getBalance(rpcRequest, { address }), | ||
| ]); | ||
| // Get the first result (should match our chain) | ||
| const balanceData = response.data.result[0]; | ||
|
|
||
| if (!balanceData) { | ||
| throw new Error("Balance data not found for the specified chain"); | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Don’t assume result[0]; filter by chainId (and tokenAddress) to avoid mismatches
API may return multiple results; picking the first is brittle. Filter to the requested chainId and tokenAddress.
- if (!response.data?.result || response.data.result.length === 0) {
+ if (!response.data?.result || response.data.result.length === 0) {
throw new Error("No balance data returned from API");
}
-
- // Get the first result (should match our chain)
- const balanceData = response.data.result[0];
-
- if (!balanceData) {
- throw new Error("Balance data not found for the specified chain");
- }
+ const chainIdStr = String(chain.id);
+ const balanceData = response.data.result.find((r) => String(r.chainId) === chainIdStr
+ && (tokenAddress ? r.tokenAddress?.toLowerCase() === tokenAddress.toLowerCase() : true));
+ if (!balanceData) {
+ throw new Error(
+ `Balance not found for address=${address} chainId=${chain.id}` +
+ (tokenAddress ? ` token=${tokenAddress}` : ""),
+ );
+ }Please confirm the API guarantees single-element results when query.chainId contains one item; if guaranteed, we can keep a fast-path but still assert chainId equality before using [0].
🌐 Web query:
Does @thirdweb-dev/api getWalletBalance return a single result when query.chainId has a single id, and is the order guaranteed?
💡 Result:
Short answer: Yes — when you call getWalletBalance for a single chain the SDK returns a single balance object (not an array). For multichain calls the Insight-backed endpoints return a data array with per-item chain_id and a meta.chain_ids list, but you should not rely on the array index for mapping — use the chain_id (or meta.chain_ids) to match results to requested chains. [1][2]
Sources:
- getWalletBalance return type (single object with chainId). [1]
- Insight multichain response format (meta.chain_ids + per-item chain_id; use those fields to identify chain). [2]
Handle single-chain vs multi-chain responses explicitly
The SDK returns a single object (not an array) when query.chainId has one entry; for multi-chain calls it returns an array in arbitrary order. Update the logic so that:
- If
response.data.resultis an object, assert itschainIdmatches before using it. - If it’s an array, filter by
chainId(andtokenAddress) instead of usingresult[0].
🤖 Prompt for AI Agents
In packages/thirdweb/src/wallets/utils/getWalletBalance.ts around lines 57 to
67, the current code blindly uses response.data.result[0] and throws when
missing; change it to handle both object and array shapes: if
response.data.result is an object, verify result.chainId (and tokenAddress when
provided) matches the requested chainId/tokenAddress and use it; if it is an
array, filter the array for entries matching chainId (and tokenAddress if
present) and pick the matching entry (throw if none); keep existing error
messages when no matching balance is found.
| @@ -1,6 +1,7 @@ | |||
| { | |||
| "$schema": "https://biomejs.dev/schemas/2.0.6/schema.json", | |||
| "extends": "//", | |||
| "root": false, | |||
There was a problem hiding this comment.
i think that extends: "//" is wrong no? that's prob why its not inheriting the root rules?
| @@ -25,6 +29,72 @@ export async function getContractMetadata( | |||
| // biome-ignore lint/suspicious/noExplicitAny: TODO: fix any | |||
| [key: string]: any; | |||
| }> { | |||
| // Configure the API client | |||
| configure({ | |||
There was a problem hiding this comment.
instead of calling configure, pass clientFetch as a custom fetch impl to each api call. Check how i do it for insight/engine hey-api packages. Also make sure we respect the thirdweb domains value for dev/prod
| "../../extensions/erc20/read/getBalance.js" | ||
| ); | ||
| return getBalance({ | ||
|
|
There was a problem hiding this comment.
do we use this function in gateway ? gotta make sure there's no infinite loop here
PR-Codex overview
This PR focuses on refactoring the wallet and token balance functionalities to utilize the
@thirdweb-dev/apifor fetching balances and contract metadata, improving the structure and error handling in the code.Detailed summary
formatterconfiguration tobiome.json.pnpm-lock.yamlto link workspace dependencies.getTokenBalanceandgetWalletBalanceto use API methods for fetching balances.Summary by CodeRabbit
New Features
Tests
Chores