[SDK] Include native tokens in Insight.getOwnedTokens #7243
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 5856cf0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
## Walkthrough
The changes update the `getOwnedTokens` function in the "thirdweb" component to use a new API endpoint that includes native tokens in its results. Corresponding adjustments are made to the function's parameters and its usage in the wallet balance-fetching logic to ensure native tokens are handled consistently.
## Changes
| File(s) | Change Summary |
|---------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------|
| .changeset/shaky-seals-cheer.md | Documents the patch update for including native tokens in `Insight.getOwnedTokens`. |
| packages/thirdweb/src/insight/get-tokens.ts | Refactors `getOwnedTokens` to use the new `/v1/tokens` endpoint, updates types, parameters, and query options accordingly. |
| packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/fetchBalancesForWallet.tsx | Implements pagination in `getOwnedTokens` calls, filters tokens with zero balance, and adjusts fallback logic to skip insight-enabled chains entirely. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant UI
participant fetchBalancesForWallet
participant getOwnedTokens
participant InsightAPI
UI->>fetchBalancesForWallet: Request wallet balances
fetchBalancesForWallet->>getOwnedTokens: Call with { ownerAddress, chains, queryOptions }
getOwnedTokens->>InsightAPI: GET /v1/tokens (includes native tokens)
InsightAPI-->>getOwnedTokens: Return token list (including native tokens)
getOwnedTokens-->>fetchBalancesForWallet: Return tokens
fetchBalancesForWallet-->>UI: Return balances (native + ERC-20)Possibly related PRs
Suggested reviewers
|
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: 1
🧹 Nitpick comments (1)
packages/thirdweb/src/insight/get-tokens.ts (1)
12-12: Update function documentation to reflect native token inclusion.The comment still states "Get ERC20 tokens owned by an address" but the function now also returns native tokens. Please update the documentation to be more accurate.
-/** - * Get ERC20 tokens owned by an address +/** + * Get tokens (ERC20 and native) owned by an address
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/shaky-seals-cheer.md(1 hunks)packages/thirdweb/src/insight/get-tokens.ts(3 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/fetchBalancesForWallet.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/thirdweb/src/insight/get-tokens.ts (2)
packages/insight/src/client/types.gen.ts (1)
GetV1TokensData(2199-2238)packages/insight/src/client/sdk.gen.ts (1)
getV1Tokens(553-570)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
packages/thirdweb/src/insight/get-tokens.ts (4)
29-32: Well-structured type definition for query options.The type definition correctly omits the conflicting parameters that are now handled internally, maintaining a clean API surface while allowing additional query customization.
35-35: API endpoint change aligns with objectives.The switch from
getV1TokensErc20ByOwnerAddresstogetV1Tokenscorrectly implements the change to include native tokens as described in the PR objectives.
52-58: Excellent implementation of native token inclusion.The default query options correctly include
include_native: "true"to enable native token fetching by default, which is the core objective of this PR. The parameter structure is well-organized and maintains backward compatibility.
60-67: API call structure correctly updated.The change from path-based to query-based parameters aligns with the new endpoint structure. All parameters are now properly passed through the query object.
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/fetchBalancesForWallet.tsx (2)
148-150: Appropriate limit addition for token fetching.Adding a limit of 100 tokens is a reasonable performance optimization that prevents potential issues with wallets containing many tokens while still covering most practical use cases.
200-200: Logic correctly updated for native token inclusion.The removal of
&& !isNativefrom the condition is correct since native tokens are now included in the insight API results. This prevents duplicate balance fetching for native tokens that were already retrieved via the insight endpoint..changeset/shaky-seals-cheer.md (1)
1-6: Clear and appropriate changeset documentation.The changeset correctly identifies this as a patch-level change and provides a concise description of the functionality enhancement.
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 #7243 +/- ##
==========================================
- Coverage 55.62% 55.55% -0.07%
==========================================
Files 908 908
Lines 58586 58571 -15
Branches 4138 4131 -7
==========================================
- Hits 32589 32540 -49
- Misses 25891 25927 +36
+ Partials 106 104 -2
🚀 New features to boost your workflow:
|
joaquim-verges
deleted the
_SDK_Include_native_tokens_in_Insight.getOwnedTokens
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 including native tokens in the
Insight.getOwnedTokensfunctionality, updating type definitions and query options accordingly.Detailed summary
GetV1TokensErc20ByOwnerAddresstoGetV1Tokens.OwnedTokentype to useGetV1TokensResponse.owner_address,chain_id, andinclude_native.Summary by CodeRabbit
New Features
Bug Fixes