[SDK] Optimize payment token fetching in payment widgets #7536
+151
−154
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 23e55b5 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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce caching and refactor how insight-enabled chains and payment tokens are determined. Functions for fetching chain services are replaced by a unified approach using a new endpoint for insight-enabled chain IDs. The payment methods hook is reworked to fetch and filter tokens and routes more efficiently, and related types and utility functions are updated or removed accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant usePaymentMethods
participant ChainsAPI
participant InsightAPI
participant TokenAPI
participant RoutesAPI
UI->>usePaymentMethods: Request payment methods
usePaymentMethods->>ChainsAPI: Fetch all supported chains
usePaymentMethods->>InsightAPI: Fetch insight-enabled chain IDs
usePaymentMethods->>TokenAPI: Fetch owned tokens (across insight-enabled chains)
loop For each chain with owned tokens
usePaymentMethods->>RoutesAPI: Fetch routes for chain to destination token
end
usePaymentMethods-->>UI: Return filtered and sorted payment tokens
sequenceDiagram
participant Consumer
participant getInsightEnabledChainIds
participant InsightAPI
Consumer->>getInsightEnabledChainIds: Request enabled chain IDs
getInsightEnabledChainIds->>InsightAPI: Fetch /chains/services?service=insight
InsightAPI-->>getInsightEnabledChainIds: Return enabled chain ID map
getInsightEnabledChainIds-->>Consumer: Return enabled chain ID list
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ 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. |
size-limit report 📦
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7536 +/- ##
==========================================
- Coverage 51.89% 51.87% -0.03%
==========================================
Files 954 954
Lines 64342 64358 +16
Branches 4241 4238 -3
==========================================
- Hits 33393 33386 -7
- Misses 30841 30864 +23
Partials 108 108
🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (2)
packages/thirdweb/src/react/core/hooks/usePaymentMethods.ts (2)
132-158: Consider the performance implications of concurrent route fetching.While the per-chain route fetching with error handling is well-implemented, the TODO comment suggests this approach may still be inefficient. The
Promise.allwith multiple concurrent API calls could impact performance if there are many chains with owned tokens.Consider these optimizations:
- Batch route fetching using the
chainIdsparameter more effectively- Implement request throttling or batching to avoid overwhelming the API
- Add metrics to measure performance impact
// Instead of fetching routes per chain individually, batch them: - await Promise.all( - chainsWithOwnedTokens.map(async (chainId) => { - const routesForChain = await routes({ - chainIds: chainsWithOwnedTokens, - // ... other params - originChainId: chainId, - }); + const allRoutesForChains = await routes({ + chainIds: chainsWithOwnedTokens, + client, + destinationChainId: destinationToken.chainId, + destinationTokenAddress: destinationToken.address, + includePrices: true, + limit: 500, // Higher limit for batched request + maxSteps: 3, + sortBy: "popularity", + });
163-174: Complex token filtering logic - consider extracting to helper function.The token filtering logic is correct but complex. Consider extracting this to a separate helper function for better readability and testability.
+ function filterValidOwnedTokens( + allOwnedTokens: Array<{ balance: bigint; originToken: Token }>, + allValidOriginTokens: Map<string, Token> + ): OwnedTokenWithQuote[] { + const validOwnedTokens: OwnedTokenWithQuote[] = []; + + for (const ownedToken of allOwnedTokens) { + const tokenKey = `${ownedToken.originToken.chainId}-${ownedToken.originToken.address.toLowerCase()}`; + const validOriginToken = allValidOriginTokens.get(tokenKey); + + if (validOriginToken) { + validOwnedTokens.push({ + balance: ownedToken.balance, + originAmount: 0n, + originToken: validOriginToken, + }); + } + } + + return validOwnedTokens; + } // 5. Filter owned tokens to only include valid origin tokens - const validOwnedTokens: OwnedTokenWithQuote[] = []; - - for (const ownedToken of allOwnedTokens) { - const tokenKey = `${ownedToken.originToken.chainId}-${ownedToken.originToken.address.toLowerCase()}`; - const validOriginToken = allValidOriginTokens.get(tokenKey); - - if (validOriginToken) { - validOwnedTokens.push({ - balance: ownedToken.balance, - originAmount: 0n, - originToken: validOriginToken, // Use the token with pricing info from routes - }); - } - } + const validOwnedTokens = filterValidOwnedTokens(allOwnedTokens, allValidOriginTokens);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/thirdweb/src/bridge/Routes.ts(3 hunks)packages/thirdweb/src/chains/utils.ts(1 hunks)packages/thirdweb/src/insight/common.ts(1 hunks)packages/thirdweb/src/react/core/hooks/usePaymentMethods.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: Write idiomatic TypeScript with explicit function declarations ...
**/*.{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
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
packages/thirdweb/src/bridge/Routes.tspackages/thirdweb/src/chains/utils.tspackages/thirdweb/src/insight/common.tspackages/thirdweb/src/react/core/hooks/usePaymentMethods.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: MananTank
PR: thirdweb-dev/js#7298
File: apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx:424-424
Timestamp: 2025-06-06T23:46:08.795Z
Learning: The thirdweb project has an ESLint rule that restricts direct usage of `defineChain`. When it's necessary to use `defineChain` directly, it's acceptable to disable the rule with `// eslint-disable-next-line no-restricted-syntax`.
packages/thirdweb/src/bridge/Routes.ts (1)
Learnt from: MananTank
PR: thirdweb-dev/js#7152
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/nfts/components/table.tsx:304-313
Timestamp: 2025-05-26T16:27:26.443Z
Learning: The `useChainSlug` hook returns `string | number`, not `string | undefined` as previously assumed. It does not return undefined values.
packages/thirdweb/src/chains/utils.ts (2)
Learnt from: MananTank
PR: thirdweb-dev/js#7298
File: apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx:424-424
Timestamp: 2025-06-06T23:46:08.795Z
Learning: The thirdweb project has an ESLint rule that restricts direct usage of `defineChain`. When it's necessary to use `defineChain` directly, it's acceptable to disable the rule with `// eslint-disable-next-line no-restricted-syntax`.
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.488Z
Learning: Applies to **/*.test.{ts,tsx} : Use `FORKED_ETHEREUM_CHAIN` for mainnet interactions and `ANVIL_CHAIN` for isolated tests
packages/thirdweb/src/insight/common.ts (2)
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.488Z
Learning: Applies to **/*.test.{ts,tsx} : Use `FORKED_ETHEREUM_CHAIN` for mainnet interactions and `ANVIL_CHAIN` for isolated tests
Learnt from: MananTank
PR: thirdweb-dev/js#7298
File: apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx:424-424
Timestamp: 2025-06-06T23:46:08.795Z
Learning: The thirdweb project has an ESLint rule that restricts direct usage of `defineChain`. When it's necessary to use `defineChain` directly, it's acceptable to disable the rule with `// eslint-disable-next-line no-restricted-syntax`.
packages/thirdweb/src/react/core/hooks/usePaymentMethods.ts (18)
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to apps/{dashboard,playground-web}/**/*.{tsx,ts} : Client Components: Handle interactive UI with React hooks (`useState`, `useEffect`, React Query, wallet hooks)
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to src/wallets/**/*.{ts,tsx} : Support EIP-1193, EIP-5792, EIP-7702 standards in wallet architecture
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to src/wallets/**/*.{ts,tsx} : Unified `Wallet` and `Account` interfaces in wallet architecture
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to src/wallets/**/*.{ts,tsx} : Support for in-app wallets (social/email login) in wallet architecture
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-06-30T10:26:04.389Z
Learning: Applies to dashboard/**/components/*.client.tsx : Interactive UI that relies on hooks (`useState`, `useEffect`, React Query, wallet hooks).
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to src/wallets/**/*.{ts,tsx} : Smart wallets with account abstraction in wallet architecture
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-06-30T10:26:04.389Z
Learning: Applies to dashboard/**/hooks/**/*.{ts,tsx} : Prefer API routes or server actions to keep tokens secret; the browser only sees relative paths.
Learnt from: MananTank
PR: thirdweb-dev/js#7152
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/nfts/components/table.tsx:304-313
Timestamp: 2025-05-26T16:27:26.443Z
Learning: The `useChainSlug` hook returns `string | number`, not `string | undefined` as previously assumed. It does not return undefined values.
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: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-06-30T10:26:04.389Z
Learning: Applies to dashboard/**/components/*.client.tsx : Anything that consumes hooks from `@tanstack/react-query` or thirdweb SDKs.
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to apps/{dashboard,playground-web}/**/*.{tsx,ts} : Client Side Data Fetching: Wrap calls in React Query (`@tanstack/react-query`)
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-06-30T10:26:04.389Z
Learning: Applies to dashboard/**/hooks/**/*.{ts,tsx} : Use React Query (`@tanstack/react-query`) for all client data fetching.
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to apps/{dashboard,playground-web}/**/*.{tsx,ts} : Client Side Data Fetching: Use descriptive, stable `queryKeys` for cache hits
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to apps/{dashboard,playground-web}/**/*.{tsx,ts} : Client Side Data Fetching: Keep tokens secret via internal API routes or server actions
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to apps/{dashboard,playground-web}/**/*.{tsx,ts} : Server Components: Perform heavy data fetching
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to apps/{dashboard,playground-web}/**/*.{tsx,ts} : Client Components: Support fast transitions with prefetched data
Learnt from: MananTank
PR: thirdweb-dev/js#7298
File: apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx:424-424
Timestamp: 2025-06-06T23:46:08.795Z
Learning: The thirdweb project has an ESLint rule that restricts direct usage of `defineChain`. When it's necessary to use `defineChain` directly, it's acceptable to disable the rule with `// eslint-disable-next-line no-restricted-syntax`.
Learnt from: joaquim-verges
PR: thirdweb-dev/js#7268
File: packages/thirdweb/src/wallets/in-app/core/wallet/in-app-core.ts:210-216
Timestamp: 2025-06-03T23:44:40.243Z
Learning: EIP7702 wallets do not need special handling for switching chains, unlike EIP4337 wallets which require reconnection when switching chains. In the switchChain method condition, EIP7702 should be intentionally excluded from the reconnection logic.
🧬 Code Graph Analysis (1)
packages/thirdweb/src/insight/common.ts (1)
packages/thirdweb/src/chains/utils.ts (1)
getInsightEnabledChainIds(387-406)
⏰ 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, esbuild)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Build Packages
- GitHub Check: Size
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
packages/thirdweb/src/bridge/Routes.ts (1)
135-135: LGTM! Clean implementation of chainIds filtering.The addition of the
chainIdsparameter follows the established patterns in the codebase and correctly implements array-to-comma-separated string conversion for the query parameter. This enhancement supports the PR's optimization goal by allowing more targeted route fetching.Also applies to: 167-169, 199-199
packages/thirdweb/src/chains/utils.ts (1)
387-406: LGTM! Well-implemented caching strategy for insight-enabled chains.The function correctly implements the optimization mentioned in the PR objectives. The 1-day cache time is appropriate for this relatively stable data, and the error handling is robust. This centralized approach will reduce redundant API calls compared to checking services per chain.
packages/thirdweb/src/insight/common.ts (1)
2-2: Excellent refactor! More efficient insight-enabled checking.The refactoring from per-chain service checks to a single cached list lookup significantly improves performance while maintaining the same functionality. The use of
every()andincludes()methods is appropriate, and the error message construction correctly identifies non-enabled chains.Also applies to: 5-6, 10-12, 19-20
packages/thirdweb/src/react/core/hooks/usePaymentMethods.ts (3)
2-2: Good imports for the refactored approach.The new imports support the optimized multi-step approach described in the PR objectives, leveraging caching mechanisms for better performance.
Also applies to: 5-8
63-72: Efficient parallel fetching and filtering strategy.Using
Promise.allto fetch chains and insight-enabled chain IDs in parallel, then filtering to insight-enabled chains is a good optimization that reduces unnecessary token fetching.
80-80: Good optimization: Increased token limit from 100 to 500.This reduces pagination overhead and aligns with the PR's goal of optimizing payment token fetching.
joaquim-verges
force-pushed
the
Optimize_payment_token_fetching_in_payment_widgets
branch
from
cd01e2c to
6928c5a
Compare
joaquim-verges
force-pushed
the
Optimize_payment_token_fetching_in_payment_widgets
branch
from
6928c5a to
23e55b5
Compare
joaquim-verges
deleted the
Optimize_payment_token_fetching_in_payment_widgets
branch
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR-Codex overview
This PR focuses on optimizing the fetching of payment tokens and enhancing the insight functionality within the
thirdwebpackage. It replaces the previous method of checking insight availability with a more efficient approach and introduces caching for improved performance.Detailed summary
assertInsightEnabledandisInsightEnabled.getChainServiceswithgetInsightEnabledChainIdsfor fetching enabled chains.chainsfunction for API responses.ChainServicetype and related code fromutils.ts.usePaymentMethodsfor better performance with owned tokens.Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores