-
Notifications
You must be signed in to change notification settings - Fork 544
[SDK] Add Bridge.tokens() function to retrieve supported tokens #7240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SDK] Add Bridge.tokens() function to retrieve supported tokens #7240
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: cac3039 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 |
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant ThirdwebClient
participant BridgeAPI
App->>ThirdwebClient: tokens({filters, pagination, client})
ThirdwebClient->>BridgeAPI: GET /bridge/tokens?filters...
BridgeAPI-->>ThirdwebClient: [token objects]
ThirdwebClient-->>App: [token objects]
sequenceDiagram
participant App
participant ThirdwebClient
participant getTokenPrice
participant BridgeAPI
App->>ThirdwebClient: convertCryptoToFiat/convertFiatToCrypto(...)
ThirdwebClient->>getTokenPrice: getTokenPrice(client, tokenAddress, chainId)
getTokenPrice->>BridgeAPI: GET /bridge/tokens?tokenAddress&chainId
BridgeAPI-->>getTokenPrice: [token object with priceUsd]
getTokenPrice-->>ThirdwebClient: priceUsd
ThirdwebClient-->>App: Conversion result
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ 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 (
|
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.changeset/bridge-tokens-function.md (1)
7-7
: Minor documentation enhancement suggestion.The documentation is clear and accurate. For slightly improved readability, consider adding a determiner:
-New function allows fetching and filtering tokens supported by the Universal Bridge service. +This new function allows fetching and filtering tokens supported by the Universal Bridge service.However, the current text is perfectly acceptable.
packages/thirdweb/src/bridge/Token.test.ts (2)
5-90
: Excellent test coverage! Consider adding edge case tests.The test suite is comprehensive and well-structured. The tests cover the main functionality effectively with good assertions.
Suggestions for additional test coverage:
Edge cases: Consider adding tests for empty results, invalid parameters, or error conditions.
Symbol filtering behavior: The current test uses
toContain("ETH")
which suggests substring matching. If this is the intended behavior, consider adding a test case that explicitly validates this:it("should support substring matching for symbol filter", async () => { const result = await tokens({ client: TEST_CLIENT, symbol: "USD", // Should match USDC, USDT, etc. }); for (const token of result) { expect(token.symbol).toContain("USD"); } });
- Offset parameter: If the tokens function supports offset for pagination, consider adding a test for that as well.
17-35
: Consider simplifying the structure validation.The nested conditional check could be simplified:
// Basic structure validation if (result.length > 0) { const token = result[0]; expect(token).toBeDefined(); expect(token).toHaveProperty("chainId"); expect(token).toHaveProperty("address"); expect(token).toHaveProperty("decimals"); expect(token).toHaveProperty("symbol"); expect(token).toHaveProperty("name"); expect(token).toHaveProperty("priceUsd"); - if (token) { expect(typeof token.chainId).toBe("number"); expect(typeof token.address).toBe("string"); expect(typeof token.decimals).toBe("number"); expect(typeof token.symbol).toBe("string"); expect(typeof token.name).toBe("string"); expect(typeof token.priceUsd).toBe("number"); - } }The inner
if (token)
check is redundant since we already confirmed the token exists and is defined.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/bridge-tokens-function.md
(1 hunks)packages/thirdweb/src/bridge/Token.test.ts
(1 hunks)packages/thirdweb/src/bridge/Token.ts
(1 hunks)packages/thirdweb/src/bridge/index.ts
(1 hunks)packages/thirdweb/src/pay/convert/cryptoToFiat.ts
(3 hunks)packages/thirdweb/src/pay/convert/fiatToCrypto.ts
(3 hunks)packages/thirdweb/src/pay/convert/get-token.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/thirdweb/src/pay/convert/fiatToCrypto.ts (2)
packages/thirdweb/src/pay/convert/get-token.ts (1)
getTokenPrice
(5-24)packages/engine/src/client/client.gen.ts (1)
client
(24-28)
packages/thirdweb/src/pay/convert/cryptoToFiat.ts (2)
packages/thirdweb/src/pay/convert/get-token.ts (1)
getTokenPrice
(5-24)packages/engine/src/client/client.gen.ts (1)
client
(24-28)
🪛 LanguageTool
.changeset/bridge-tokens-function.md
[uncategorized] ~6-~6: A determiner appears to be missing. Consider inserting it.
Context: ...ieve supported Universal Bridge tokens New function allows fetching and filtering ...
(AI_EN_LECTOR_MISSING_DETERMINER)
🪛 Gitleaks (8.26.0)
packages/thirdweb/src/bridge/Token.ts
79-79: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Size
- GitHub Check: Build Packages
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (12)
packages/thirdweb/src/bridge/index.ts (1)
9-9
: LGTM! Export follows established conventions.The export statement correctly follows the same pattern as other bridge exports and is appropriately placed among similar single-function exports.
packages/thirdweb/src/pay/convert/fiatToCrypto.ts (3)
8-8
: LGTM: Clean import of new helper function.The switch to using the dedicated
getTokenPrice
helper improves code organization and reusability.
63-63
: Good: Removed unused parameter from destructuring.Removing the unused
from
parameter cleans up the code since it's not used in the function logic.
93-99
: LGTM: Simplified token price fetching with improved error handling.The refactoring to use
getTokenPrice
simplifies the logic while maintaining the same functionality. The error condition properly handles both null/undefined and zero price scenarios.packages/thirdweb/src/pay/convert/cryptoToFiat.ts (3)
8-8
: LGTM: Consistent use of the getTokenPrice helper.Good to see consistent refactoring approach across both conversion functions.
62-62
: Good: Removed unused parameter.Cleaning up the unused
to
parameter improves code clarity.
95-101
: LGTM: Simplified price fetching and calculation.The refactoring maintains the same functionality while using the centralized
getTokenPrice
helper. The multiplication logic for the final result is correct.packages/thirdweb/src/bridge/Token.ts (5)
1-6
: LGTM: Clean imports and dependencies.All necessary imports are properly organized and relevant to the functionality.
7-117
: Excellent: Comprehensive documentation with practical examples.The JSDoc documentation is exceptionally thorough with multiple usage examples covering:
- Basic usage
- Filtering by various parameters (chainId, symbol, name, tokenAddress)
- Pagination with limit/offset
- Expected response structure
This level of documentation significantly improves developer experience.
🧰 Tools
🪛 Gitleaks (8.26.0)
79-79: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
118-143
: LGTM: Robust parameter handling and URL construction.The function properly handles optional parameters with explicit null/undefined checks, which is important for query parameter construction. The URL building logic correctly adds parameters only when they have valid values.
144-157
: LGTM: Comprehensive error handling and response processing.The error handling is well-structured:
- Proper HTTP status checking
- Structured error parsing with fallbacks
- ApiError with relevant context (code, message, correlationId, statusCode)
- Clean response data extraction
159-184
: LGTM: Well-structured TypeScript namespace declarations.The namespace provides clear type definitions for both input options and result types, improving type safety and developer experience.
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (74.32%) 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 #7240 +/- ##
==========================================
- Coverage 55.61% 55.61% -0.01%
==========================================
Files 908 910 +2
Lines 58583 58597 +14
Branches 4133 4137 +4
==========================================
+ Hits 32583 32589 +6
- Misses 25894 25903 +9
+ Partials 106 105 -1
🚀 New features to boost your workflow:
|
size-limit report 📦
|
PR-Codex overview
This PR introduces the
Bridge.tokens()
function to retrieve and filter supported Universal Bridge tokens, enhancing the ability to fetch token data by various parameters such as chain ID, token address, symbol, and name.Detailed summary
tokens
export inpackages/thirdweb/src/bridge/index.ts
.getTokenPrice
function inpackages/thirdweb/src/pay/convert/get-token.ts
.tokens
inpackages/thirdweb/src/bridge/Token.test.ts
.convertFiatToCrypto
andconvertCryptoToFiat
functions to usegetTokenPrice
.tokens
function inpackages/thirdweb/src/bridge/Token.ts
with filtering capabilities.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Refactor