[SDK] Fix native token fallback when insight query fails #7666
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 8b4cfef The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughA changeset file was added to document a patch update for the "thirdweb" package, specifically noting a fix to the native token fallback mechanism during insight query failures. Additionally, the internal logic of the Changes
Estimated code review effort2 (~15 minutes) Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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). (8)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
joaquim-verges
changed the title
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/thirdweb/src/react/core/hooks/usePaymentMethods.ts (1)
99-142: Consider extracting token conversion logic.The token conversion logic (lines 124-139) is duplicated at lines 150-165. Consider extracting this into a helper function to follow DRY principles.
Extract the conversion logic into a reusable function:
function convertBalancesToTokens(balances: GetWalletBalanceResult[]): Array<{ balance: bigint; originToken: Token; }> { return balances .filter((b) => b.value > 0n) .map((b) => ({ balance: b.value, originToken: { address: b.tokenAddress, chainId: b.chainId, decimals: b.decimals, iconUri: "", name: b.name, prices: { USD: 0, }, symbol: b.symbol, } as Token, })); }Then use it in both places:
- // Convert to our format and filter out zero balances - const tokensWithBalance = batch - .filter((b) => b.value > 0n) - .map((b) => ({ - balance: b.value, - originToken: { - address: b.tokenAddress, - chainId: b.chainId, - decimals: b.decimals, - iconUri: "", - name: b.name, - prices: { - USD: 0, - }, - symbol: b.symbol, - } as Token, - })); + const tokensWithBalance = convertBalancesToTokens(batch);
🧹 Nitpick comments (2)
packages/thirdweb/src/react/core/hooks/usePaymentMethods.ts (2)
124-125: Remove redundant filtering.The code filters for
b.value > 0ntwice - once at line 121 and again at line 125. This is redundant sincebatchalready contains only non-zero balances.- const tokensWithBalance = batch - .filter((b) => b.value > 0n) - .map((b) => ({ + const tokensWithBalance = batch.map((b) => ({
128-138: Enhance token data with proper defaults.The token construction uses hardcoded defaults that might not be optimal. Consider using more descriptive defaults or constants.
originToken: { address: b.tokenAddress, chainId: b.chainId, decimals: b.decimals, - iconUri: "", + iconUri: "", // TODO: Add native token icon URL name: b.name, prices: { - USD: 0, + USD: 0, // Price will be fetched later via routes }, symbol: b.symbol, } as Token,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/eighty-rings-think.md(1 hunks)packages/thirdweb/src/react/core/hooks/usePaymentMethods.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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
Files:
packages/thirdweb/src/react/core/hooks/usePaymentMethods.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/react/core/hooks/usePaymentMethods.ts
🧠 Learnings (3)
📓 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: MananTank
PR: thirdweb-dev/js#7177
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/erc20/_hooks/useTokenPriceData.ts:49-49
Timestamp: 2025-05-27T19:55:25.056Z
Learning: In the ERC20 public pages token price data hook (`useTokenPriceData.ts`), direct array access on `json.data[0]` without optional chaining is intentionally correct and should not be changed to use safety checks.
Learnt from: MananTank
PR: thirdweb-dev/js#7081
File: apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/assets/create/create-token-page-impl.tsx:110-118
Timestamp: 2025-05-20T18:54:15.781Z
Learning: In the thirdweb dashboard's token asset creation flow, the `transferBatch` function from `thirdweb/extensions/erc20` accepts the raw quantity values from the form without requiring explicit conversion to wei using `toUnits()`. The function appears to handle this conversion internally or is designed to work with the values in the format they're already provided.
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/** : Support for in-app wallets (social/email login)
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/** : Smart wallets with account abstraction
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
Learnt from: gregfromstl
PR: thirdweb-dev/js#7450
File: packages/thirdweb/src/bridge/Webhook.ts:57-81
Timestamp: 2025-06-26T19:46:04.024Z
Learning: In the onramp webhook schema (`packages/thirdweb/src/bridge/Webhook.ts`), the `currencyAmount` field is intentionally typed as `z.number()` while other amount fields use `z.string()` because `currencyAmount` represents fiat currency amounts in decimals (like $10.50), whereas other amount fields represent token amounts in wei (very large integers that benefit from bigint representation). The different naming convention (`currencyAmount` vs `amount`) reflects this intentional distinction.
.changeset/eighty-rings-think.md (4)
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: Surface breaking changes prominently in PR descriptions
Learnt from: MananTank
PR: #7081
File: apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/assets/create/create-token-page-impl.tsx:110-118
Timestamp: 2025-05-20T18:54:15.781Z
Learning: In the thirdweb dashboard's token asset creation flow, the transferBatch function from thirdweb/extensions/erc20 accepts the raw quantity values from the form without requiring explicit conversion to wei using toUnits(). The function appears to handle this conversion internally or is designed to work with the values in the format they're already provided.
Learnt from: MananTank
PR: #7332
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/nft/overview/nft-drop-claim.tsx:82-90
Timestamp: 2025-06-13T13:03:41.732Z
Learning: The thirdweb contract object is serializable and can safely be used in contexts (e.g., React-Query keys) that require serializable values.
packages/thirdweb/src/react/core/hooks/usePaymentMethods.ts (10)
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: MananTank
PR: #7177
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/erc20/_hooks/useTokenPriceData.ts:49-49
Timestamp: 2025-05-27T19:55:25.056Z
Learning: In the ERC20 public pages token price data hook (useTokenPriceData.ts), direct array access on json.data[0] without optional chaining is intentionally correct and should not be changed to use safety checks.
Learnt from: MananTank
PR: #7081
File: apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/assets/create/create-token-page-impl.tsx:110-118
Timestamp: 2025-05-20T18:54:15.781Z
Learning: In the thirdweb dashboard's token asset creation flow, the transferBatch function from thirdweb/extensions/erc20 accepts the raw quantity values from the form without requiring explicit conversion to wei using toUnits(). The function appears to handle this conversion internally or is designed to work with the values in the format they're already provided.
Learnt from: gregfromstl
PR: #7450
File: packages/thirdweb/src/bridge/Webhook.ts:57-81
Timestamp: 2025-06-26T19:46:04.024Z
Learning: In the onramp webhook schema (packages/thirdweb/src/bridge/Webhook.ts), the currencyAmount field is intentionally typed as z.number() while other amount fields use z.string() because currencyAmount represents fiat currency amounts in decimals (like $10.50), whereas other amount fields represent token amounts in wei (very large integers that benefit from bigint representation). The different naming convention (currencyAmount vs amount) reflects this intentional distinction.
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).
Learnt from: MananTank
PR: #7307
File: apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx:324-346
Timestamp: 2025-06-09T15:15:02.350Z
Learning: In the move-funds functionality, MananTank prefers having both individual toast.promise notifications for each token transfer AND batch summary toasts, even though this creates multiple notifications. This dual notification approach is acceptable for the move-funds user experience.
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
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
Learnt from: MananTank
PR: #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.
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to src/exports/react.native.ts : React Native specific exports are in src/exports/react.native.ts
⏰ 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: Build Packages
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
.changeset/eighty-rings-think.md (1)
1-6: LGTM! Changeset properly documents the patch fix.The changeset correctly follows the standard format and accurately describes the native token fallback fix with appropriate patch versioning.
packages/thirdweb/src/react/core/hooks/usePaymentMethods.ts (1)
122-142: No action needed: Native token address is always set to NATIVE_TOKEN_ADDRESSThe fallback paths in usePaymentMethods call getWalletBalance without a
tokenAddress, and in getWalletBalance.ts you can see:
tokenAddress: tokenAddress ?? NATIVE_TOKEN_ADDRESSSince
tokenAddressis injected when fetching native balances,b.tokenAddresswill never be undefined or the zero address—it will always be theNATIVE_TOKEN_ADDRESSconstant (0xeeee…eeee).
size-limit report 📦
|
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #7666 +/- ##
==========================================
- Coverage 56.40% 56.36% -0.05%
==========================================
Files 908 908
Lines 58228 58244 +16
Branches 4226 4226
==========================================
- Hits 32844 32829 -15
- Misses 25274 25307 +33
+ Partials 110 108 -2
🚀 New features to boost your workflow:
|
joaquim-verges
force-pushed
the
_React_Fix_native_token_fallback_when_insight_query_fails
branch
from
0509f94 to
2dbc389
Compare
joaquim-verges
force-pushed
the
_React_Fix_native_token_fallback_when_insight_query_fails
branch
from
2dbc389 to
f7cf351
Compare
joaquim-verges
force-pushed
the
_React_Fix_native_token_fallback_when_insight_query_fails
branch
from
f7cf351 to
8b4cfef
Compare
joaquim-verges
deleted the
_React_Fix_native_token_fallback_when_insight_query_fails
branch
PR-Codex overview
This PR focuses on fixing the fallback mechanism for native tokens when an insight query fails in the
thirdwebpackage.Detailed summary
usePaymentMethods.tsto handle cases where the query fails.tokensWithBalancearray to store formatted token information.Summary by CodeRabbit
Bug Fixes
Chores