[SDK] Chore: Remove onrampTokenAddress set to ETH #7441
Merged
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: e9a7773 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 ↗︎
|
## Walkthrough
This change removes the automatic fallback to `NATIVE_TOKEN_ADDRESS` for the `onrampTokenAddress` parameter in both the backend logic and UI layer. Now, `onrampTokenAddress` is passed as-is, without substituting a default value when undefined or null. Additionally, the StepRunner component's onramp step text was updated to dynamically capitalize the onramp identifier. The onramp transactions are enriched with chain details and client reference, and a delay was added after transaction approval to allow RPC state update. A new UI message was added for when all quote queries fail.
## Changes
| Files | Change Summary |
|-----------------------------------------------------------------------|-------------------------------------------------------------------------------------------------|
| packages/thirdweb/src/pay/buyWithFiat/getQuote.ts | Removed fallback to `NATIVE_TOKEN_ADDRESS` for `onrampTokenAddress` in `prepareOnramp` call. |
| packages/thirdweb/src/react/web/ui/Bridge/QuoteLoader.tsx | Removed import and usage of `NATIVE_TOKEN_ADDRESS` for `onrampTokenAddress` in bridge params. |
| packages/thirdweb/src/react/web/ui/Bridge/StepRunner.tsx | Changed onramp step text from static "TEST" to dynamic capitalized `request.onramp` string. |
| packages/thirdweb/src/bridge/Onramp.ts | Added `chain` and `client` properties to each onramp transaction by using `defineChain`. |
| packages/thirdweb/src/react/core/hooks/useStepExecutor.ts | Added 1-second delay after approval/fee tx receipt to allow RPC node state update. |
| packages/thirdweb/src/react/web/ui/Bridge/payment-selection/FiatProviderSelection.tsx | Added UI message "No quotes available" when all quote queries error out. |
| packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsx | Enhanced step token and chain name display to distinguish bridging vs swapping steps. |
| .changeset/old-bats-love.md | Added patch changeset describing fixes to payment widgets. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant UI as QuoteLoader.tsx
participant Logic as getQuote.ts
participant Onramp as prepareOnramp
participant ChainUtil as defineChain
participant Executor as useStepExecutor
UI->>Logic: getQuote({ onrampTokenAddress })
Logic->>Onramp: prepareOnramp({ onrampTokenAddress })
Onramp->>ChainUtil: defineChain(chainId)
Onramp->>Onramp: enrich tx with chain and client
Executor->>Executor: await approval/fee tx receipt
Executor->>Executor: delay 1 second for RPC updatePossibly 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. |
size-limit report 📦
|
Fixes TOOL-4871
![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 (1)
packages/thirdweb/src/react/web/ui/Bridge/StepRunner.tsx (1)
325-326: Good improvement! Consider adding safety checks for robustness.The dynamic capitalization of the onramp identifier is a nice improvement over static text and aligns well with the PR's goal of making the onramp logic more flexible.
However, consider adding safety checks to handle edge cases:
- {request.onramp.slice(0, 1).toUpperCase() + - request.onramp.slice(1)} + {request.onramp?.slice(0, 1).toUpperCase() + + request.onramp?.slice(1) || 'Onramp'}Alternatively, you could create a utility function for better maintainability:
const capitalizeFirst = (str: string) => str ? str.charAt(0).toUpperCase() + str.slice(1) : 'Onramp'; // Usage: {capitalizeFirst(request.onramp)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/thirdweb/src/react/web/ui/Bridge/StepRunner.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.@(ts|tsx)`: Accept a typed 'props' object and export a named function (e.g...
**/*.@(ts|tsx): Accept a typed 'props' object and export a named function (e.g., export function MyComponent()).
Combine class names via 'cn', expose 'className' prop if useful.
Reuse core UI primitives; avoid re-implementing buttons, cards, modals.
Local state or effects live inside; data fetching happens in hooks.
Merge class names with 'cn' from '@/lib/utils' to keep conditional logic readable.
Stick to design-tokens: background ('bg-card'), borders ('border-border'), muted text ('text-muted-foreground') etc.
Use the 'container' class with a 'max-w-7xl' cap for page width consistency.
Spacing utilities ('px-', 'py-', 'gap-*') are preferred over custom margins.
Responsive helpers follow mobile-first ('max-sm', 'md', 'lg', 'xl').
Never hard-code colors – always go through Tailwind variables.
Tailwind CSS is the styling system – avoid inline styles or CSS modules.
Prefix files with 'import "server-only";' so they never end up in the client bundle (for server-only code).
📄 Source: CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
List of files the instruction was applied to:
packages/thirdweb/src/react/web/ui/Bridge/StepRunner.tsx
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Unit Tests
- GitHub Check: Size
- GitHub Check: Build Packages
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Lint Packages
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Analyze (javascript)
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (5.45%) 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 #7441 +/- ##
==========================================
- Coverage 51.96% 51.92% -0.04%
==========================================
Files 947 947
Lines 63823 63871 +48
Branches 4217 4216 -1
==========================================
+ Hits 33163 33165 +2
- Misses 30554 30600 +46
Partials 106 106
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsx (1)
335-349: Improve readability of Bridge/Swap logic.The conditional logic for displaying Bridge vs Swap operations could be more readable and maintainable.
Consider extracting this logic into a helper function:
const getStepLabel = (step: typeof preparedQuote.steps[0]) => { const isBridge = step.destinationToken.chainId !== step.originToken.chainId; if (isBridge) { const tokensAreSame = step.originToken.symbol === step.destinationToken.symbol; return `Bridge ${tokensAreSame ? step.originToken.symbol : `${step.originToken.symbol} to ${step.destinationToken.symbol}`}`; } return `Swap ${step.originToken.symbol} to ${step.destinationToken.symbol}`; };Then use it in the JSX:
- {step.destinationToken.chainId !== - step.originToken.chainId ? ( - <> - Bridge{" "} - {step.originToken.symbol === - step.destinationToken.symbol - ? step.originToken.symbol - : `${step.originToken.symbol} to ${step.destinationToken.symbol}`} - </> - ) : ( - <> - Swap {step.originToken.symbol} to{" "} - {step.destinationToken.symbol} - </> - )} + {getStepLabel(step)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.@(ts|tsx)`: Accept a typed 'props' object and export a named function (e.g...
**/*.@(ts|tsx): Accept a typed 'props' object and export a named function (e.g., export function MyComponent()).
Combine class names via 'cn', expose 'className' prop if useful.
Reuse core UI primitives; avoid re-implementing buttons, cards, modals.
Local state or effects live inside; data fetching happens in hooks.
Merge class names with 'cn' from '@/lib/utils' to keep conditional logic readable.
Stick to design-tokens: background ('bg-card'), borders ('border-border'), muted text ('text-muted-foreground') etc.
Use the 'container' class with a 'max-w-7xl' cap for page width consistency.
Spacing utilities ('px-', 'py-', 'gap-*') are preferred over custom margins.
Responsive helpers follow mobile-first ('max-sm', 'md', 'lg', 'xl').
Never hard-code colors – always go through Tailwind variables.
Tailwind CSS is the styling system – avoid inline styles or CSS modules.
Prefix files with 'import "server-only";' so they never end up in the client bundle (for server-only code).
📄 Source: CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
List of files the instruction was applied to:
packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsx
🧬 Code Graph Analysis (1)
packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsx (1)
packages/thirdweb/src/react/core/hooks/others/useChainQuery.ts (1)
useChainsQuery(182-196)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Size
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
Comment on lines
+335
to
+376
| {step.destinationToken.chainId !== | ||
| step.originToken.chainId ? ( | ||
| <> | ||
| Bridge{" "} | ||
| {step.originToken.symbol === | ||
| step.destinationToken.symbol | ||
| ? step.originToken.symbol | ||
| : `${step.originToken.symbol} to ${step.destinationToken.symbol}`} | ||
| </> | ||
| ) : ( | ||
| <> | ||
| Swap {step.originToken.symbol} to{" "} | ||
| {step.destinationToken.symbol} | ||
| </> | ||
| )} | ||
| </Text> | ||
| <Text color="secondaryText" size="xs"> | ||
| {step.originToken.name} to{" "} | ||
| {step.destinationToken.name} | ||
| {step.originToken.chainId !== | ||
| step.destinationToken.chainId ? ( | ||
| <> | ||
| { | ||
| chainsMetadata.find( | ||
| (c) => c.chainId === step.originToken.chainId, | ||
| )?.name | ||
| }{" "} | ||
| to{" "} | ||
| { | ||
| chainsMetadata.find( | ||
| (c) => | ||
| c.chainId === step.destinationToken.chainId, | ||
| )?.name | ||
| } | ||
| </> | ||
| ) : ( | ||
| <> | ||
| { | ||
| chainsMetadata.find( | ||
| (c) => c.chainId === step.originToken.chainId, | ||
| )?.name | ||
| } | ||
| </> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add fallback handling for missing chain metadata.
The code assumes chain metadata will always be available when rendering, but during loading or error states, chainsMetadata.find() might return undefined, causing the chain names to not display.
Apply this diff to add fallback handling:
<>
{
- chainsMetadata.find(
- (c) => c.chainId === step.originToken.chainId,
- )?.name
+ chainsMetadata.find(
+ (c) => c.chainId === step.originToken.chainId,
+ )?.name ?? `Chain ${step.originToken.chainId}`
}{" "}
to{" "}
{
- chainsMetadata.find(
- (c) =>
- c.chainId === step.destinationToken.chainId,
- )?.name
+ chainsMetadata.find(
+ (c) =>
+ c.chainId === step.destinationToken.chainId,
+ )?.name ?? `Chain ${step.destinationToken.chainId}`
}
</>
) : (
<>
{
- chainsMetadata.find(
- (c) => c.chainId === step.originToken.chainId,
- )?.name
+ chainsMetadata.find(
+ (c) => c.chainId === step.originToken.chainId,
+ )?.name ?? `Chain ${step.originToken.chainId}`
}
</>
)}This ensures users see meaningful information even when chain metadata is still loading or failed to load.
📝 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.
Suggested change
| {step.destinationToken.chainId !== | |
| step.originToken.chainId ? ( | |
| <> | |
| Bridge{" "} | |
| {step.originToken.symbol === | |
| step.destinationToken.symbol | |
| ? step.originToken.symbol | |
| : `${step.originToken.symbol} to ${step.destinationToken.symbol}`} | |
| </> | |
| ) : ( | |
| <> | |
| Swap {step.originToken.symbol} to{" "} | |
| {step.destinationToken.symbol} | |
| </> | |
| )} | |
| </Text> | |
| <Text color="secondaryText" size="xs"> | |
| {step.originToken.name} to{" "} | |
| {step.destinationToken.name} | |
| {step.originToken.chainId !== | |
| step.destinationToken.chainId ? ( | |
| <> | |
| { | |
| chainsMetadata.find( | |
| (c) => c.chainId === step.originToken.chainId, | |
| )?.name | |
| }{" "} | |
| to{" "} | |
| { | |
| chainsMetadata.find( | |
| (c) => | |
| c.chainId === step.destinationToken.chainId, | |
| )?.name | |
| } | |
| </> | |
| ) : ( | |
| <> | |
| { | |
| chainsMetadata.find( | |
| (c) => c.chainId === step.originToken.chainId, | |
| )?.name | |
| } | |
| </> | |
| )} | |
| <Text color="secondaryText" size="xs"> | |
| {step.originToken.chainId !== | |
| step.destinationToken.chainId ? ( | |
| <> | |
| { | |
| chainsMetadata.find( | |
| (c) => c.chainId === step.originToken.chainId, | |
| )?.name ?? `Chain ${step.originToken.chainId}` | |
| }{" "} | |
| to{" "} | |
| { | |
| chainsMetadata.find( | |
| (c) => | |
| c.chainId === step.destinationToken.chainId, | |
| )?.name ?? `Chain ${step.destinationToken.chainId}` | |
| } | |
| </> | |
| ) : ( | |
| <> | |
| { | |
| chainsMetadata.find( | |
| (c) => c.chainId === step.originToken.chainId, | |
| )?.name ?? `Chain ${step.originToken.chainId}` | |
| } | |
| </> | |
| )} | |
| </Text> |
🤖 Prompt for AI Agents
In packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsx
between lines 335 and 376, the code assumes chainsMetadata.find() always returns
a valid object, but it can be undefined during loading or error states, causing
chain names not to display. To fix this, add fallback values such as a
placeholder string like "Unknown Chain" or the chainId itself when the find
returns undefined, ensuring meaningful information is shown even if metadata is
missing.
Comment on lines
+107
to
+114
| const chainsQuery = useChainsQuery( | ||
| preparedQuote.steps.flatMap((s) => s.transactions.map((t) => t.chain)), | ||
| 10, | ||
| ); | ||
| const chainsMetadata = useMemo( | ||
| () => chainsQuery.map((c) => c.data), | ||
| [chainsQuery], | ||
| ).filter((c) => !!c); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize chain extraction and improve error handling.
The chain extraction logic runs on every render, which could impact performance. Additionally, there's no error handling for failed chain queries, and the memoization dependency could be improved.
Apply this diff to optimize performance and add error handling:
+ const chainList = useMemo(
+ () => preparedQuote.steps.flatMap((s) => s.transactions.map((t) => t.chain)),
+ [preparedQuote.steps],
+ );
+
- const chainsQuery = useChainsQuery(
- preparedQuote.steps.flatMap((s) => s.transactions.map((t) => t.chain)),
- 10,
- );
+ const chainsQuery = useChainsQuery(chainList, 10);
+
const chainsMetadata = useMemo(
- () => chainsQuery.map((c) => c.data),
- [chainsQuery],
+ () => chainsQuery.map((c) => c.data).filter((c) => !!c),
+ [chainsQuery.map(q => q.data)],
- ).filter((c) => !!c);
+ );
+
+ const hasChainErrors = chainsQuery.some((q) => q.error);
+ const isLoadingChains = chainsQuery.some((q) => q.isLoading);Consider adding error handling or fallback behavior when hasChainErrors is true.
📝 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.
Suggested change
| const chainsQuery = useChainsQuery( | |
| preparedQuote.steps.flatMap((s) => s.transactions.map((t) => t.chain)), | |
| 10, | |
| ); | |
| const chainsMetadata = useMemo( | |
| () => chainsQuery.map((c) => c.data), | |
| [chainsQuery], | |
| ).filter((c) => !!c); | |
| const chainList = useMemo( | |
| () => preparedQuote.steps.flatMap((s) => s.transactions.map((t) => t.chain)), | |
| [preparedQuote.steps], | |
| ); | |
| const chainsQuery = useChainsQuery(chainList, 10); | |
| const chainsMetadata = useMemo( | |
| () => chainsQuery.map((c) => c.data).filter((c) => !!c), | |
| [chainsQuery.map((q) => q.data)], | |
| ); | |
| const hasChainErrors = chainsQuery.some((q) => q.error); | |
| const isLoadingChains = chainsQuery.some((q) => q.isLoading); |
🤖 Prompt for AI Agents
In packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsx
around lines 107 to 114, the chain extraction logic runs on every render causing
potential performance issues, and there is no error handling for failed chain
queries. To fix this, memoize the array of chains extracted from
preparedQuote.steps to avoid recalculating on each render, improve the useMemo
dependency to depend on this memoized array, and add error handling logic to
detect if any chain queries have errors (e.g., a hasChainErrors flag) and
provide fallback behavior or error messages accordingly.
gregfromstl
deleted the
greg/tool-4872-remove-eth-onramptokenaddress-from-sdk
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.
We no longer need to force onramping to ETH with the new native fee abstractions. Apps that might need to onramp to a multi-hop route with IAWs should use gas sponsorship.
PR-Codex overview
This PR focuses on enhancing the payment widgets in the
thirdweblibrary by implementing various fixes, improving user experience, and refining the handling of onramp and bridge transactions.Detailed summary
useStepExecutor.ts.StepRunner.tsxto capitalize the onramp request.FiatProviderSelection.tsx.NATIVE_TOKEN_ADDRESSingetQuote.ts.Onramp.ts.QuoteLoader.tsxto remove unusedNATIVE_TOKEN_ADDRESS.chainsQueryandchainsMetadatainPaymentDetails.tsxto enhance data handling.PaymentDetails.tsx.Summary by CodeRabbit