-
Notifications
You must be signed in to change notification settings - Fork 627
[SDK] Support ERC5792 batch transactions for swaps, add slippage option to Bridge API #8484
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] Support ERC5792 batch transactions for swaps, add slippage option to Bridge API #8484
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 3d2c961 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
WalkthroughThis PR adds ERC-5792 batch (sendCalls) support to the transaction executor, threads an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Executor as StepExecutor
participant Account as AccountCapabilities
participant Blockchain
participant Poller as ReceiptPoller
Client->>Executor: execute step
Executor->>Account: check supportsAtomic(chainId)
alt supportsAtomic == true
Account-->>Executor: atomic available
Executor->>Executor: apply gas buffer (50000n)
Executor->>Blockchain: sendCalls (batch)
Blockchain-->>Executor: batch accepted (tx submitted)
Executor->>Poller: waitForCallsReceipt(batchId)
Poller->>Blockchain: poll receipts
Blockchain-->>Poller: receipts returned
Poller->>Executor: final receipt/status
Executor-->>Client: success (with preparedQuote.type)
else supportsAtomic == false
Account-->>Executor: atomic unavailable
Executor->>Executor: apply gas buffer (50000n)
Executor->>Blockchain: send single tx (executeBatch/normal)
Blockchain-->>Executor: receipt
Executor-->>Client: success
end
alt failure paths
Executor-->>Client: Error: No receipts found / No supported execution mode
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (3)
Comment |
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts (1)
636-647: MissingexecuteSendCallsin dependency array.The
executecallback usesexecuteSendCalls(line 575) but it's not included in the dependency array. This will cause stale closure issues ifexecuteSendCallsis recreated.}, [ executionState, wallet, currentTxIndex, flatTxs, executeSingleTx, executeBatch, + executeSendCalls, onrampStatus, executeOnramp, onComplete, preparedQuote, ]);
🧹 Nitpick comments (5)
packages/thirdweb/src/bridge/Sell.ts (1)
355-355: Consider adding client-side validation for slippageToleranceBps.The
slippageToleranceBpsparameter is correctly threaded through the implementation. However, adding client-side validation would improve user experience by catching invalid values early (e.g., negative numbers or values exceeding 10,000 bps).Consider adding validation before the API call:
export async function prepare( options: prepare.Options, ): Promise<prepare.Result> { const { originChainId, originTokenAddress, destinationChainId, destinationTokenAddress, amount, sender, receiver, client, purchaseData, maxSteps, paymentLinkId, slippageToleranceBps, } = options; + + if (slippageToleranceBps !== undefined && (slippageToleranceBps < 0 || slippageToleranceBps > 10000)) { + throw new Error("slippageToleranceBps must be between 0 and 10000 (0% to 100%)"); + } const clientFetch = getClientFetch(client);Also applies to: 374-374, 453-454
packages/thirdweb/src/bridge/Buy.ts (1)
369-369: Consider adding client-side validation for slippageToleranceBps.The
slippageToleranceBpsparameter is correctly threaded through the implementation. However, adding client-side validation would improve user experience by catching invalid values early (e.g., negative numbers or values exceeding 10,000 bps).Consider adding validation before the API call:
export async function prepare( options: prepare.Options, ): Promise<prepare.Result> { const { originChainId, originTokenAddress, destinationChainId, destinationTokenAddress, sender, receiver, client, amount, purchaseData, maxSteps, paymentLinkId, slippageToleranceBps, } = options; + + if (slippageToleranceBps !== undefined && (slippageToleranceBps < 0 || slippageToleranceBps > 10000)) { + throw new Error("slippageToleranceBps must be between 0 and 10000 (0% to 100%)"); + } const clientFetch = getClientFetch(client);Also applies to: 388-388, 465-466
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts (2)
15-15: Inconsistent import pattern: lazy-loadwaitForCallsReceiptinsideexecuteSendCalls.Other heavy dependencies (
prepareTransaction,sendCalls,status) are dynamically imported inside async functions. This top-level import breaks that pattern. As per coding guidelines, lazy-load heavy dependencies inside async paths to keep the initial bundle lean.Move the import inside
executeSendCalls:-import { waitForCallsReceipt } from "../../../wallets/eip5792/wait-for-calls-receipt.js";Then in
executeSendCalls(around line 385):+ const { waitForCallsReceipt } = await import( + "../../../wallets/eip5792/wait-for-calls-receipt.js" + ); const callsStatus = await waitForCallsReceipt(result);
716-726: Add explicit return type annotation.Per coding guidelines, async functions should have explicit return types for clarity and type safety.
-async function supportsAtomic(account: Account, chainId: number) { +async function supportsAtomic(account: Account, chainId: number): Promise<boolean> { const capabilitiesFn = account.getCapabilities;packages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx (1)
64-65: Success-screen side-effect query and cache invalidation behaviorUsing
useQueryClient()plusqueryClient.invalidateQuerieson success to refreshbridge/v1/wallets,walletBalance, andpayment-methodsis a reasonable way to ensure post-payment state is fresh. One thing to double-check is whether it’s acceptable for thisuseQueryto potentially re-run on refocus or remount (depending on the global React Query config), which would re-trigger the tracking + invalidations multiple times rather than strictly once-per-success.If you want strictly one-shot behavior, consider either:
- Tying
enabled/staleTimeexplicitly to the lifecycle of this screen, or- Converting this into a
useEffectwith an internal guard, and usingqueryClient.invalidateQueriesfrom there.Also applies to: 66-100
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.changeset/thirty-banks-itch.md(1 hunks)packages/thirdweb/src/bridge/Buy.ts(3 hunks)packages/thirdweb/src/bridge/Sell.ts(3 hunks)packages/thirdweb/src/react/core/hooks/useStepExecutor.ts(18 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx(3 hunks)packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript 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 in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.tspackages/thirdweb/src/react/core/hooks/useStepExecutor.tspackages/thirdweb/src/bridge/Sell.tspackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsxpackages/thirdweb/src/bridge/Buy.ts
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.tspackages/thirdweb/src/react/core/hooks/useStepExecutor.tspackages/thirdweb/src/bridge/Sell.tspackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsxpackages/thirdweb/src/bridge/Buy.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.tspackages/thirdweb/src/react/core/hooks/useStepExecutor.tspackages/thirdweb/src/bridge/Sell.tspackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsxpackages/thirdweb/src/bridge/Buy.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.tspackages/thirdweb/src/react/core/hooks/useStepExecutor.tspackages/thirdweb/src/bridge/Sell.tspackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsxpackages/thirdweb/src/bridge/Buy.ts
🧬 Code graph analysis (1)
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts (3)
packages/thirdweb/src/exports/thirdweb.ts (1)
prepareTransaction(181-181)packages/thirdweb/src/bridge/Onramp.ts (1)
status(13-13)packages/thirdweb/src/react/core/hooks/useBridgePrepare.ts (1)
BridgePrepareResult(23-27)
⏰ 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). (9)
- GitHub Check: Vercel Agent Review
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: Build Packages
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
.changeset/thirty-banks-itch.md (1)
1-5: Changeset format and versioning look good.The YAML front matter is correctly formatted for the Changesets tool, and the minor version bump is appropriate for the new features being introduced (ERC-5792 batch transaction support and slippage tolerance parameters to the Bridge API). The description accurately summarizes the key changes in this PR.
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts (3)
332-419: LGTM!The
executeSendCallsimplementation correctly handles ERC-5792 batch calls. UsinglastReceipt.transactionHashfor status polling is appropriate since the batch is atomic, and the chain validation in the batching logic (line 557) ensuresfirstTx.chainIdis valid for all transactions.
551-595: LGTM!The batching logic correctly prioritizes
sendBatchTransactionover ERC-5792sendCalls, and the defensive else branch at lines 582-585 provides a safety net (though logically unreachable given thecanBatch || canSendCallscondition).
204-204: LGTM!Consistent 50k gas buffer applied across all transaction execution paths.
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts (1)
119-119: Aggressive refetch is complementary to invalidation; verify if necessary for UX.Query invalidation is properly implemented in
SuccessScreen.tsx(line 88–90), which targets the["bridge/v1/wallets"]query after transactions. However,refetchOnMount: 'always'serves a different purpose: it ensures fresh token balances when users navigate back to the bridge widget, regardless of how they reached it.This may be intentional for a bridge/swap context where balance accuracy is critical. However, consider whether:
refetchOnMount: true(default) with astaleTimeconfig would be sufficient instead of forcing every mount- Testing confirms the aggressive refetch improves UX without causing excessive API load
- Pagination-driven query instances (via
page/limitin the queryKey) already create separate cache entriespackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx (2)
3-4: React Query hook import looks consistent with existing patternsImporting
useQuery/useQueryClientfrom@tanstack/react-queryis aligned with how the rest of the file uses the query cache; nothing to change here.
78-87: Transfer payment tracking payload appears internally consistentThe new
preparedQuote.type === "transfer"branch sendschainId/toChainIdaspreparedQuote.intent.chainIdandfromToken/toTokenaspreparedQuote.intent.tokenAddress, which makes sense for a same-chain, same-asset transfer. This keeps the analytics shape consistent with buy/sell events.Please confirm that the
BridgePrepareResulttransfer intent shape indeed useschainIdandtokenAddress(and not, for example,originChainId/destinationChainId) so this won’t drift if the intent type evolves.
size-limit report 📦
|
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (4.51%) 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 #8484 +/- ##
==========================================
- Coverage 54.83% 54.73% -0.11%
==========================================
Files 920 920
Lines 60891 61013 +122
Branches 4141 4145 +4
==========================================
+ Hits 33390 33395 +5
- Misses 27399 27516 +117
Partials 102 102
🚀 New features to boost your workflow:
|
315351c to
3d2c961
Compare
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)
packages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx (1)
78-97: Transfer analytics + cache invalidation are reasonable; consider minor cleanup and confirm key semantics
- The new
preparedQuote.type === "transfer"branch mirrors the buy/sell analytics shape and uses the same"ub:ui:success_screen"event, which is consistent. Please double‑check that using the same token and chain for bothfrom*andto*fields matches what your analytics pipeline expects for a transfer (especially ifBridgePrepareResult.intentever distinguishes source vs destination chain/token more explicitly).- The three
invalidateQueriescalls will run once per success-screen mount and should refresh bridge wallets, balances, and payment methods. Make sure thesequeryKeys match the prefixes actually used by the corresponding hooks/selectors (e.g., if keys are tuples like["walletBalance", walletAddress, chainId], this relies on React Query’s partial key matching behavior in your version).- Optional: there’s a bit of duplication between the buy/sell and transfer
trackPayEventpayloads; you could factor out a sharedbaseEvent({ client, event: "ub:ui:success_screen" }) or a small helper function to keep this easier to maintain if more types are added later.packages/thirdweb/src/react/core/hooks/useStepExecutor.ts (2)
204-204: Extract gas buffer to a named constant.The gas buffer value
50000nis hardcoded in three locations (lines 204, 281, 372). Consider extracting this to a named constant at the top of the file for better maintainability.+// Gas buffer for transaction execution +const TX_GAS_BUFFER = 50000n; + import { useCallback, useEffect, useMemo, useRef, useState } from "react";Then use
extraGas: TX_GAS_BUFFERin all three locations.Also applies to: 281-281, 372-372
250-330: Consider refactoring to reduce duplication betweenexecuteBatchandexecuteSendCalls.The two functions share significant structural similarity (~60+ lines of common logic):
- Transaction preparation and mapping
- Error handling patterns
- Status polling logic
While the current implementation works correctly, extracting shared logic into helper functions would improve maintainability and reduce the risk of divergent bug fixes.
Example refactoring approach:
// Shared transaction preparation async function prepareBatchTransactions(txs: FlattenedTx[]) { const { prepareTransaction } = await import( "../../../transaction/prepare-transaction.js" ); return Promise.all( txs.map(async (tx) => { return prepareTransaction({ chain: tx.chain, client: tx.client, data: tx.data, to: tx.to, value: tx.value, extraGas: TX_GAS_BUFFER, }); }), ); } // Shared status polling async function pollForBatchCompletion( firstTx: FlattenedTx, transactionHash: string, preparedQuote: BridgePrepareResult, completedStatusResults: CompletedStatusResult[], poller: (...) => ..., abortSignal: AbortSignal, ) { const { status } = await import("../../../bridge/Status.js"); await poller(async () => { const statusResult = await status({ chainId: firstTx.chainId, client: firstTx.client, transactionHash, }); if (statusResult.status === "COMPLETED") { const typedStatusResult = { type: preparedQuote.type, ...statusResult, }; completedStatusResults.push(typedStatusResult); return { completed: true }; } if (statusResult.status === "FAILED") { throw new Error("Payment failed"); } return { completed: false }; }, abortSignal); }Also applies to: 332-419
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.changeset/thirty-banks-itch.md(1 hunks)packages/thirdweb/src/bridge/Buy.ts(3 hunks)packages/thirdweb/src/bridge/Sell.ts(3 hunks)packages/thirdweb/src/react/core/hooks/useStepExecutor.ts(8 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx(3 hunks)packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts
- packages/thirdweb/src/bridge/Sell.ts
- packages/thirdweb/src/bridge/Buy.ts
- .changeset/thirty-banks-itch.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript 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 in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/react/core/hooks/useStepExecutor.tspackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/react/core/hooks/useStepExecutor.tspackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/react/core/hooks/useStepExecutor.tspackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/react/core/hooks/useStepExecutor.tspackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx
🧬 Code graph analysis (1)
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts (2)
packages/thirdweb/src/exports/thirdweb.ts (1)
prepareTransaction(181-181)packages/thirdweb/src/bridge/Onramp.ts (1)
status(13-13)
⏰ 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). (9)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Vercel Agent Review
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx (1)
3-3: React Query client wiring looks goodImporting
useQueryClientand instantiatingqueryClientat the top of the component is idiomatic and correctly scoped; no changes needed here.Also applies to: 64-64
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts (3)
12-12: LGTM!The import of
waitForCallsReceiptis correctly added to support ERC-5792 batch transaction receipts.
717-727: LGTM!The
supportsAtomichelper correctly checks account capabilities for ERC-5792 atomic execution support. The logic properly handles missing capabilities and checks for both "supported" and "ready" statuses.
543-585: LGTM!The execution selection logic correctly:
- Checks for both
sendBatchTransactionand ERC-5792sendCallssupport- Prefers native batch transactions over sendCalls when both are available
- Includes defensive error handling for the theoretically unreachable else case
The logic properly optimizes batch execution by grouping consecutive same-chain transactions.
| // get tx hash | ||
| const callsStatus = await waitForCallsReceipt(result); | ||
| const lastReceipt = | ||
| callsStatus.receipts?.[callsStatus.receipts.length - 1]; | ||
|
|
||
| if (!lastReceipt) { | ||
| throw new Error("No receipts found"); | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find waitForCallsReceipt implementation
fd -e ts -e tsx | xargs grep -l "waitForCallsReceipt" | head -20Repository: thirdweb-dev/js
Length of output: 498
🏁 Script executed:
#!/bin/bash
# Search for waitForCallsReceipt definition
rg -n "export.*waitForCallsReceipt|function waitForCallsReceipt" --type=tsRepository: thirdweb-dev/js
Length of output: 162
🏁 Script executed:
#!/bin/bash
# Look at useStepExecutor.ts to see context around lines 384-391
cat -n packages/thirdweb/src/react/core/hooks/useStepExecutor.ts | sed -n '370,410p'Repository: thirdweb-dev/js
Length of output: 1514
🏁 Script executed:
#!/bin/bash
# Search for other receipt handling patterns in the codebase
rg -n "receipts\[" --type=ts -C3 | head -50Repository: thirdweb-dev/js
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check if there's a receipt status field used anywhere
rg -n "receipt.*status|status.*receipt" --type=ts -C2 | head -40Repository: thirdweb-dev/js
Length of output: 4254
🏁 Script executed:
#!/bin/bash
# Read the waitForCallsReceipt implementation
cat -n packages/thirdweb/src/wallets/eip5792/wait-for-calls-receipt.ts | head -100Repository: thirdweb-dev/js
Length of output: 3892
🏁 Script executed:
#!/bin/bash
# Look at executeBatch function in useStepExecutor.ts for comparison
cat -n packages/thirdweb/src/react/core/hooks/useStepExecutor.ts | sed -n '250,330p'Repository: thirdweb-dev/js
Length of output: 3209
🏁 Script executed:
#!/bin/bash
# Check if there's validation inside waitForCallsReceipt
rg -n "status.*reverted|reverted.*status" packages/thirdweb/src/wallets/eip5792/wait-for-calls-receipt.ts -C5Repository: thirdweb-dev/js
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check GetCallsStatusResponse type definition
rg -n "type GetCallsStatusResponse|interface GetCallsStatusResponse" packages/thirdweb/src/wallets/eip5792/ -A 20Repository: thirdweb-dev/js
Length of output: 1633
🏁 Script executed:
#!/bin/bash
# Check what status values receipts can have
rg -n "receiptStatuses|receipt.*status" packages/thirdweb/src/wallets/eip5792/get-calls-status.ts -B3 -A3Repository: thirdweb-dev/js
Length of output: 430
🏁 Script executed:
#!/bin/bash
# Check if individual receipt status is ever validated anywhere
rg -n "receipt\.status.*==|receipt\.status.*===|reverted" packages/thirdweb/src/wallets/eip5792/ --type=tsRepository: thirdweb-dev/js
Length of output: 266
🏁 Script executed:
#!/bin/bash
# Check if bundle.status === "success" means all receipts are success
cat -n packages/thirdweb/src/wallets/eip5792/get-calls-status.ts | sed -n '1,110p'Repository: thirdweb-dev/js
Length of output: 4365
🏁 Script executed:
#!/bin/bash
# Check EIP-5792 spec or comments about bundle vs receipt status
rg -n "atomic|bundle|receipt.*reverted" packages/thirdweb/src/wallets/eip5792/ -B2 -A2 | head -60Repository: thirdweb-dev/js
Length of output: 145
Check individual receipt statuses for non-atomic bundles.
The code only uses lastReceipt.transactionHash for polling without verifying receipt success. ERC-5792 supports both atomic and non-atomic call bundles. For atomic bundles, waitForCallsReceipt returning bundle.status === "success" guarantees all receipts succeeded. However, for non-atomic bundles, individual calls can fail independently—their receipt statuses should be validated.
Consider adding validation for non-atomic cases:
// get tx hash
const callsStatus = await waitForCallsReceipt(result);
const lastReceipt =
callsStatus.receipts?.[callsStatus.receipts.length - 1];
if (!lastReceipt) {
throw new Error("No receipts found");
}
+// For non-atomic bundles, verify individual receipt success
+if (!callsStatus.atomic) {
+ for (const receipt of callsStatus.receipts) {
+ if (receipt.status === "reverted") {
+ throw new Error("Transaction in batch failed");
+ }
+ }
+}📝 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.
| // get tx hash | |
| const callsStatus = await waitForCallsReceipt(result); | |
| const lastReceipt = | |
| callsStatus.receipts?.[callsStatus.receipts.length - 1]; | |
| if (!lastReceipt) { | |
| throw new Error("No receipts found"); | |
| } | |
| // get tx hash | |
| const callsStatus = await waitForCallsReceipt(result); | |
| const lastReceipt = | |
| callsStatus.receipts?.[callsStatus.receipts.length - 1]; | |
| if (!lastReceipt) { | |
| throw new Error("No receipts found"); | |
| } | |
| // For non-atomic bundles, verify individual receipt success | |
| if (!callsStatus.atomic) { | |
| for (const receipt of callsStatus.receipts) { | |
| if (receipt.status === "reverted") { | |
| throw new Error("Transaction in batch failed"); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/thirdweb/src/react/core/hooks/useStepExecutor.ts around lines 384 to
391, the code only grabs lastReceipt.transactionHash without validating
individual receipt success for non-atomic bundles; update the logic to detect
non-atomic bundles (e.g., bundle.status !== "success" or appropriate flag from
waitForCallsReceipt result) and iterate over callsStatus.receipts to check each
receipt's status (or reverted/failed fields), and if any receipt indicates
failure, throw or return a descriptive error containing the failing call index
and its transactionHash/revert reason; otherwise continue using the appropriate
successful receipt.transactionHash for polling.
| refetchOnMount: false, | ||
| retry: false, | ||
| refetchOnWindowFocus: false, | ||
| refetchOnMount: "always", |
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.
The refetch behavior change may have unintended side effects. By removing the retry: false and refetchOnWindowFocus: false options, token balance queries will now retry on failure and refetch on window focus, which could increase API calls beyond what was intended.
View Details
📝 Patch Details
diff --git a/packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts b/packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts
index 19521d200..de9c92b31 100644
--- a/packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts
+++ b/packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts
@@ -117,5 +117,7 @@ export function useTokenBalances(options: {
return json.result;
},
refetchOnMount: "always",
+ retry: false,
+ refetchOnWindowFocus: false,
});
}
Analysis
Unintended query behavior change in useTokenBalances() after refactor
What fails: useTokenBalances() in packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts now retries failed requests 3 times and refetches on window focus, contradicting the explicit defensive configuration set in previous commits.
How to reproduce:
- Examine git history:
git log --oneline packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts - Compare commit b8afa98 (which explicitly added
retry: false) vs commit 3d2c961 (which removed it) - Verify React Query defaults using TanStack Query documentation:
retrydefaults to 3 (not false)refetchOnWindowFocusdefaults to true (not false)
Result: Removed three explicit query options without updating the feature:
- Commit b8afa98 (Sep 29) explicitly set
retry: falseto "reduce loading times and improve UI responsiveness" by disabling retries on failed token balance queries - Commit 3d2c961 (Dec 3) removed
retry: false,refetchOnWindowFocus: false, and changedrefetchOnMount: falsetorefetchOnMount: "always" - Now useTokenBalances() will automatically:
- Retry 3 times on API failure (vs previous: never retry)
- Refetch when window regains focus (vs previous: never refetch on focus)
Expected: Query options should maintain the explicit defensive configuration (retry: false, refetchOnWindowFocus: false) while still supporting the new refetchOnMount: "always" behavior. This preserves the intentional choice to minimize unnecessary API calls for token balance queries while ensuring fresh data on component mount.

PR-Codex overview
This PR introduces support for
erc5792batch transactions in thethirdweblibrary, enhancing theBridgeAPI with a slippage option for transactions. It also includes updates to theSuccessScreencomponent and theuseStepExecutorhook for better transaction handling.Detailed summary
erc5792batch transactions in theuseStepExecutorhook.slippageToleranceBpsoption inBuy.tsandSell.tsfor transaction slippage control.SuccessScreento track payment events and invalidate queries upon success.useStepExecutorto handle batch execution and send calls if supported.50000nfor transaction preparation.Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.