Merged
Conversation
* Added new API routes for executing user operations, generating transfer user operations, and checking Nexus account status. * Introduced support for EIP-7702 sponsored transactions and delegation contract handling. * Updated bundler server URL configuration to use a default path, simplifying environment variable requirements. * Refactored existing components and hooks to utilize the new bundler API structure, enhancing transaction handling and user experience. * Removed deprecated delegation contract address handling, streamlining the codebase for future development.
* Cleaned up the .env.example file by removing extra blank lines for improved readability and consistency.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a server-side bundler subsystem (ERC‑4337 / EIP‑7702): new bundler libraries and constants, six new Next.js API routes under /api/bundler, client changes routing bundler calls to internal Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Client as Client Component
participant APIRoute as /api/bundler (server)
participant Chain as chains.ts (client creation)
participant UserOp as userOp module
participant RPC as Blockchain/RPC
User->>Client: Request transfer/upgrade
Client->>APIRoute: POST /generate-userop
APIRoute->>Chain: parseChainId / getClients
Chain->>RPC: create publicClient
APIRoute->>UserOp: generateUserOp(publicClient,...)
UserOp->>RPC: estimate gas / fetch fees / call paymaster
UserOp-->>APIRoute: unsigned userOp + hash
APIRoute-->>Client: return userOp
Client->>APIRoute: POST /execute (signed userOp)
APIRoute->>UserOp: ensurePrefundIfNeeded / executeUserOp
UserOp->>RPC: check balance / send prefund / submit to EntryPoint
UserOp-->>APIRoute: transactionHash
APIRoute-->>Client: execution result
sequenceDiagram
actor User
participant Client as Client Component
participant APIRoute as /api/bundler/execute-sponsored
participant Chain as chains.ts (getClients with sponsor)
participant Sponsored as executeSponsored module
participant RPC as Blockchain/RPC
User->>Client: Initiate sponsored tx
Client->>APIRoute: POST /execute-sponsored (rpcUrl, account, callData, eip7702Authorization?)
APIRoute->>Chain: getClients(chainId, rpcUrl) (walletClient uses sponsor key)
APIRoute->>Sponsored: executeSponsored(...)
alt eip7702Authorization present
Sponsored->>Sponsored: parseAuth, build delegation tx
Sponsored->>RPC: send delegation tx (type: eip7702)
RPC-->>Sponsored: receipt
Sponsored-->>APIRoute: delegationTransactionHash
else no delegation
Sponsored->>RPC: get nonce, estimate fees
Sponsored->>RPC: send execution tx with sponsor wallet
RPC-->>Sponsored: tx hash / receipt
Sponsored-->>APIRoute: transactionHash
end
APIRoute-->>Client: return tx hash(es)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 5❌ Failed checks (5 warnings)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
chibie
requested changes
Mar 13, 2026
…utes * Eliminated unnecessary console log statements in the bundler execution routes and the TransactionPreview component to clean up the code and improve performance.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/hooks/useSmartWalletTransfer.ts (1)
236-246:⚠️ Potential issue | 🟡 MinorUpdate stale error message referencing removed configuration.
The error message at line 244 still references
NEXT_PUBLIC_BUNDLER_SERVER_URL, but the hook now uses a hardcoded internal path. This would confuse users debugging connection issues.🔧 Suggested fix
throw new Error( isNetworkErr - ? `Cannot reach the transaction server. Check that NEXT_PUBLIC_BUNDLER_SERVER_URL is set and the server is running. For ${chainName}, ensure the server has been restarted with support for this network.` + ? `Cannot reach the bundler API. For ${chainName}, ensure the server is running and supports this network.` : (fetchErr as Error)?.message ?? "Network request failed", );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/hooks/useSmartWalletTransfer.ts` around lines 236 - 246, The catch block in useSmartWalletTransfer (catching fetchErr) still mentions NEXT_PUBLIC_BUNDLER_SERVER_URL even though the code uses a hardcoded internal path; update the thrown Error message in that catch to remove the stale env var reference and instead mention the actual internal endpoint or service (e.g., the bundler/transaction server or the hardcoded path used), so the error reads something like “Cannot reach the transaction server at <internal path> — ensure the server is running/restarted with support for this network” and keep the existing branch for non-network errors returning fetchErr.message; modify the message in the catch in useSmartWalletTransfer accordingly.
🧹 Nitpick comments (4)
app/api/bundler/is-nexus/route.ts (1)
1-45: Re-export the canonical handler instead of duplicating it.This alias is byte-for-byte identical to
app/api/bundler/nexus/status/route.ts; keeping one implementation avoids drift the next time validation or response fields change.Suggested fix
-import { NextRequest, NextResponse } from "next/server"; -import { getClients, parseChainId, parseRpcUrl } from "@/app/lib/bundler/chains"; -import { getNexusStatus } from "@/app/lib/bundler/userOp"; - -/** - * GET /api/bundler/is-nexus?smartAccountAddress=0x...&chainId=56&rpcUrl=... - * Alias for nexus/status for compatibility with upgrade-server path. - */ -export async function GET(request: NextRequest) { - try { - const { searchParams } = request.nextUrl; - const smartAccountAddress = searchParams.get("smartAccountAddress"); - const chainIdRaw = searchParams.get("chainId"); - const rpcUrl = parseRpcUrl(searchParams.get("rpcUrl")); - - if (!rpcUrl) { - return NextResponse.json({ error: "rpcUrl is required" }, { status: 400 }); - } - if (!smartAccountAddress || !/^0x[0-9a-fA-F]{40}$/.test(smartAccountAddress)) { - return NextResponse.json( - { error: "Query param smartAccountAddress (0x + 40 hex) is required" }, - { status: 400 } - ); - } - - const chainId = parseChainId(chainIdRaw); - const { publicClient } = getClients(chainId, rpcUrl); - const status = await getNexusStatus(publicClient, smartAccountAddress as `0x${string}`); - - return NextResponse.json({ - smartAccountAddress, - chainId, - rpcUrl, - ...status, - }); - } catch (error) { - return NextResponse.json( - { - error: - error instanceof Error ? error.message : "Failed to check nexus status", - }, - { status: 400 } - ); - } -} +export { GET } from "../nexus/status/route";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bundler/is-nexus/route.ts` around lines 1 - 45, This file duplicates the nexus status handler; replace the local GET implementation by re-exporting the canonical GET handler from the existing nexus status route module (the file that currently implements the nexus/status GET export). Remove the duplicated logic (validation, parseChainId/parseRpcUrl/getClients/getNexusStatus usage) and instead add a single re-export like: export { GET } from '<the nexus status route module>'; ensure the exported symbol is the same GET handler so this alias stays byte-for-byte identical going forward.app/api/bundler/generate-userop/route.ts (2)
61-65: Redundant null coalescing for validated field.
rpcUrlis already validated as required at line 24, so the?? nullfallback on line 64 is unreachable.🔧 Minor cleanup
return NextResponse.json({ ...result, chainId, - rpcUrl: rpcUrl ?? null, + rpcUrl, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bundler/generate-userop/route.ts` around lines 61 - 65, The rpcUrl field is validated as required earlier, so remove the redundant `?? null` fallback when returning the response; update the return in the route handler that calls NextResponse.json to simply include rpcUrl (e.g., use rpcUrl directly instead of `rpcUrl ?? null`) so the response mirrors the validated value and avoids unreachable code.
8-11: Consider warning when PAYMASTER_URL is set but PAYMASTER_API_KEY is missing.When
PAYMASTER_URLis configured butPAYMASTER_API_KEYis absent, the paymaster request proceeds without the API key header. Depending on the paymaster service, this may result in unclear 401/403 errors rather than a helpful diagnostic message.Consider logging a warning during startup or request handling when this configuration mismatch is detected.
Also applies to: 54-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bundler/generate-userop/route.ts` around lines 8 - 11, Add a startup/request-time warning when PAYMASTER_URL is set but PAYMASTER_API_KEY is missing: detect the mismatch by checking the constants PAYMASTER_URL and PAYMASTER_API_KEY and emit a clear warning (via the app logger or console.warn) either during module initialization or at the start of the request handler (e.g., the route/generate handler) so callers see a diagnostic instead of opaque 401/403 responses; apply the same check and warning wherever the PAYMASTER_* values are used (including the other occurrence referenced at lines 54-56).app/api/bundler/execute/route.ts (1)
66-78: Clarify why paymasterAndData is hardcoded.Line 76 sets
paymasterAndData: "0x"regardless of what's inraw.paymasterAndData. This appears intentional for self-funded execution, but a brief comment would clarify this is deliberate and prevent future maintainers from "fixing" it.📝 Add clarifying comment
const signedUserOp: SerializedUserOperation = { sender: getAddress(sender) as `0x${string}`, nonce: String(raw.nonce ?? ""), initCode: (raw.initCode ?? "0x") as `0x${string}`, callData: (raw.callData ?? "0x") as `0x${string}`, callGasLimit: String(raw.callGasLimit ?? "0"), verificationGasLimit: String(raw.verificationGasLimit ?? "0"), preVerificationGas: String(raw.preVerificationGas ?? "0"), maxFeePerGas: String(raw.maxFeePerGas ?? "0"), maxPriorityFeePerGas: String(raw.maxPriorityFeePerGas ?? "0"), + // This route is for self-funded execution; paymaster data is intentionally ignored paymasterAndData: "0x", signature: (raw.signature ?? "0x") as `0x${string}`, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bundler/execute/route.ts` around lines 66 - 78, The object signedUserOp currently sets paymasterAndData: "0x" unconditionally which is intentional for self-funded execution; update the signedUserOp construction (in route.ts where signedUserOp is created) to include a short clarifying comment above or inline explaining that paymasterAndData is deliberately hardcoded to "0x" to enforce self-funded ops and that raw.paymasterAndData is intentionally ignored, so future maintainers won't change it; optionally mention any security/behavior rationale (e.g., preventing external paymaster usage) and reference raw.paymasterAndData in the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 140-143: The comment in .env.example references the wrong env var
name: it mentions SPONSOR_PRIVATE_KEY but the actual variable used is
SPONSOR_EVM_WALLET_PRIVATE_KEY (as read in app/lib/bundler/chains.ts). Update
the comment text to refer to SPONSOR_EVM_WALLET_PRIVATE_KEY (or add both names
if you want to clarify legacy vs current) so the fallback guidance matches the
variable declaration and the code that consumes it.
In `@app/api/bundler/execute-sponsored/route.ts`:
- Line 30: Remove the two leftover debug console.log calls that print
delegationContractAddress and the other temporary debug at the same spot;
replace them with structured logging using the app's logger at an appropriate
level (e.g., logger.debug or processLogger.debug) or delete entirely if not
needed. Locate the console.log("delegationContractAddress",
delegationContractAddress) and the other console.log on the same function/route
in execute-sponsored/route.ts and either remove them or swap to a structured
logger call that includes a clear message and the variable (e.g., logger.debug({
msg: "delegationContractAddress", delegationContractAddress })). Ensure no raw
console.log remains in that route handler.
In `@app/api/bundler/generate-transfer-userop/route.ts`:
- Around line 59-68: The catch currently returns 500 for all failures; changes
should detect client-side validation errors from parseChainId, parseRpcUrl, the
calls[] validation and BigInt parsing and return a 400 Bad Request instead.
Implement a small ValidationError (or throw Error with a distinct name) from the
input validation paths (where parseChainId, parseRpcUrl, calls[] checks and
BigInt(...) are invoked) and in the catch block test for that error type/name
(or known parsing error names) to return NextResponse.json({ error:
error.message }, { status: 400 }) for validation failures, falling back to the
existing 500 response for other errors. Ensure you update the code locations
that call parseChainId/parseRpcUrl/BigInt and any calls[] validation to throw
the new ValidationError so the catch can distinguish them.
In `@app/components/WalletTransferApprovalModal.tsx`:
- Around line 275-288: The bundlerServerUrl validation in
WalletTransferApprovalModal incorrectly uses new URL(bundlerServerUrl) on a
relative path (bundlerServerUrl = "/api/bundler"), causing the catch to throw a
misleading "Invalid NEXT_PUBLIC_BUNDLER_SERVER_URL" error; remove the entire URL
parsing/validation try/catch block that references new URL(...) and the protocol
checks so the hardcoded internal path is accepted, leaving the meeApiKey
existence check intact.
In `@app/lib/bundler/chains.ts`:
- Around line 31-35: The parseRpcUrl function currently allows arbitrary URLs
which lets callers proxy requests to any host; change parseRpcUrl to resolve
incoming values against the server-side chain configuration (or a strict host
allowlist) instead of accepting arbitrary strings: lookup the requested chain
identifier in the server-side chains config and return the configured RPC URL,
or if you must accept a URL validate the hostname against an explicit allowlist
and reject loopback (localhost, 127.0.0.0/8, ::1), link-local and private ranges
(10/8, 172.16/12, 192.168/16, 169.254/16, etc.) and non-HTTP schemes, throwing
an error from parseRpcUrl when the URL is not an approved/resolved endpoint so
callers of parseRpcUrl/http(rpcUrl) cannot target arbitrary hosts.
- Around line 58-69: getClients currently always calls getSponsorAccount and
constructs a WalletClient, causing handlers that only need a PublicClient to
fail when no sponsor key exists; change getClients to make wallet creation
opt-in by adding an optional parameter (e.g., includeWallet: boolean = true) or
accept an optional sponsorAccount argument, then only call getSponsorAccount and
createWalletClient when includeWallet is true (or sponsorAccount is provided);
ensure the function signature and return type reflect that walletClient can be
undefined (or return a separate object shape) and update callers of getClients
(e.g., endpoints /api/bundler/nexus/status, /api/bundler/is-nexus,
/api/bundler/generate-transfer-userop) to call it with includeWallet=false when
they only need the PublicClient; keep SUPPORTED_CHAINS, createPublicClient, and
transport: http(rpcUrl) usage unchanged.
- Around line 19-20: The parseChainId function currently defaults missing/empty
input to bsc.id (56); change it to treat undefined/null/'' as invalid and throw
a clear error instead. In parseChainId(value) check for value === undefined ||
value === null || value === '' and throw a descriptive Error (e.g. "Missing or
empty chainId"); then parse numeric strings (Number(value) or parseInt) and
validate the result is a finite positive integer before returning it. Keep
references to parseChainId and bsc.id so reviewers can find the location.
In `@app/lib/bundler/executeSponsored.ts`:
- Around line 61-75: In the executeSponsored branch that checks
params.eip7702Authorization, the code sets delegationTransactionHash via
walletClient.sendTransaction and waits for the receipt but returns without
exposing that delegation hash; update the early return so it includes the
delegationTransactionHash (e.g., return an object containing both
transactionHash and delegationTransactionHash or a dedicated
delegationTransactionHash field) so callers can detect the delegation path after
parseAuth/walletClient.sendTransaction and
publicClient.waitForTransactionReceipt.
In `@app/lib/bundler/userOp.ts`:
- Around line 787-812: The current code hard-caps callGasLimit and
verificationGasLimit using minBigInt against GAS_CONFIG.callGasLimit and
GAS_CONFIG.verificationGasLimit even when estimateGas succeeds, which forces
large legitimate estimates down and causes AA23/OOG; change the logic so the
static GAS_CONFIG values act only as fallbacks when estimation fails: keep the
estimate path from publicClient.estimateGas (the try block and call to
multiplyByBps) and remove the minBigInt clamping against GAS_CONFIG.callGasLimit
there, only apply GAS_CONFIG.callGasLimit as a ceiling if estimateGas threw
(i.e., when you set the fallback callGasLimit in the catch), and similarly
ensure verificationGasLimit (computed from callGasLimit and constants) is not
forcibly minBigInt-capped by GAS_CONFIG.verificationGasLimit when an actual
estimate was available — instead use the config only as a fallback/maximum when
no estimate exists.
- Around line 813-827: The multipliers are currently shrinking suggested fees
(5_000n = 50%) and the fallback can produce maxPriorityFeePerGas > maxFeePerGas;
change multiplyByBps(...) calls (used where fees are set after
publicClient.estimateFeesPerGas() and publicClient.getGasPrice()) to use a
multiplier > 10_000n (e.g., 15_000n) so we add headroom instead of reducing
fees, and ensure maxPriorityFeePerGas is capped to not exceed maxFeePerGas by
computing it as min(maxFeePerGas, maxBigInt(maxFeePerGas / 10n, 1_000_000n))
(use functions multiplyByBps and maxBigInt and the variables
maxFeePerGas/maxPriorityFeePerGas to locate the fixes).
In `@app/pages/TransactionPreview.tsx`:
- Line 430: Remove the debug console.log statements left in
TransactionPreview.tsx: delete the console.log("res", res) statement and the
console.log that prints the transaction hash (e.g., the one logging txHash)
inside the transaction handling/submit flow (look for the
submit/handleSend/handleConfirm function or where "res" and "txHash" variables
are used). Ensure no console.* debug logging remains in that component before
merging.
---
Outside diff comments:
In `@app/hooks/useSmartWalletTransfer.ts`:
- Around line 236-246: The catch block in useSmartWalletTransfer (catching
fetchErr) still mentions NEXT_PUBLIC_BUNDLER_SERVER_URL even though the code
uses a hardcoded internal path; update the thrown Error message in that catch to
remove the stale env var reference and instead mention the actual internal
endpoint or service (e.g., the bundler/transaction server or the hardcoded path
used), so the error reads something like “Cannot reach the transaction server at
<internal path> — ensure the server is running/restarted with support for this
network” and keep the existing branch for non-network errors returning
fetchErr.message; modify the message in the catch in useSmartWalletTransfer
accordingly.
---
Nitpick comments:
In `@app/api/bundler/execute/route.ts`:
- Around line 66-78: The object signedUserOp currently sets paymasterAndData:
"0x" unconditionally which is intentional for self-funded execution; update the
signedUserOp construction (in route.ts where signedUserOp is created) to include
a short clarifying comment above or inline explaining that paymasterAndData is
deliberately hardcoded to "0x" to enforce self-funded ops and that
raw.paymasterAndData is intentionally ignored, so future maintainers won't
change it; optionally mention any security/behavior rationale (e.g., preventing
external paymaster usage) and reference raw.paymasterAndData in the comment.
In `@app/api/bundler/generate-userop/route.ts`:
- Around line 61-65: The rpcUrl field is validated as required earlier, so
remove the redundant `?? null` fallback when returning the response; update the
return in the route handler that calls NextResponse.json to simply include
rpcUrl (e.g., use rpcUrl directly instead of `rpcUrl ?? null`) so the response
mirrors the validated value and avoids unreachable code.
- Around line 8-11: Add a startup/request-time warning when PAYMASTER_URL is set
but PAYMASTER_API_KEY is missing: detect the mismatch by checking the constants
PAYMASTER_URL and PAYMASTER_API_KEY and emit a clear warning (via the app logger
or console.warn) either during module initialization or at the start of the
request handler (e.g., the route/generate handler) so callers see a diagnostic
instead of opaque 401/403 responses; apply the same check and warning wherever
the PAYMASTER_* values are used (including the other occurrence referenced at
lines 54-56).
In `@app/api/bundler/is-nexus/route.ts`:
- Around line 1-45: This file duplicates the nexus status handler; replace the
local GET implementation by re-exporting the canonical GET handler from the
existing nexus status route module (the file that currently implements the
nexus/status GET export). Remove the duplicated logic (validation,
parseChainId/parseRpcUrl/getClients/getNexusStatus usage) and instead add a
single re-export like: export { GET } from '<the nexus status route module>';
ensure the exported symbol is the same GET handler so this alias stays
byte-for-byte identical going forward.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 81f631d6-53dc-48c9-9bca-8f8b25d8171b
📒 Files selected for processing (16)
.env.exampleapp/api/bundler/execute-sponsored/route.tsapp/api/bundler/execute/route.tsapp/api/bundler/generate-transfer-userop/route.tsapp/api/bundler/generate-userop/route.tsapp/api/bundler/is-nexus/route.tsapp/api/bundler/nexus/status/route.tsapp/components/WalletTransferApprovalModal.tsxapp/hooks/useSmartWalletTransfer.tsapp/lib/bundler/chains.tsapp/lib/bundler/config.tsapp/lib/bundler/executeSponsored.tsapp/lib/bundler/userOp.tsapp/lib/config.tsapp/pages/TransactionPreview.tsxapp/types.ts
💤 Files with no reviewable changes (2)
- app/types.ts
- app/lib/config.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/bundler/execute/route.ts`:
- Around line 100-129: The code currently calls ensurePrefundIfNeeded(...) which
performs an irreversible depositTo before executeUserOp(...), risking lost
sponsor funds if execution later reverts; modify the flow to perform a preflight
simulation (e.g., simulateUserOp or callStatic style check) using the same
inputs derived from getRequiredPrefundWei(...) and signedUserOp, and only if the
simulation succeeds proceed to call ensurePrefundIfNeeded(...); alternatively
split the endpoint so prefund is an explicit separate action the client must
call (move ensurePrefundIfNeeded out of the executeUserOp handler) and surface
simulation results to the caller so the deposit is not made automatically prior
to a confirmed successful execution.
- Around line 42-55: The code is overwriting the client-signed UserOperation
fields (notably raw.sender and paymasterAndData) for deployed accounts; instead
preserve the original signed payload. Change the logic around variables sender,
hasInitCode, smartAccountAddress and paymasterAndData so you do not replace
raw.sender with smartAccountAddress when hasInitCode is false — always use
raw.sender as the canonical sender from the client (but validate it and
throw/return an error if it's malformed), and remove any code that silently
forces paymasterAndData = "0x"; apply the same fix to the similar block around
lines 66-77. Ensure validation uses the existing raw.sender checks (the regex)
and surface an explicit error on invalid inputs rather than substituting values.
- Around line 24-31: The handler currently calls getClients(chainId, rpcUrl)
before verifying that parsed chainId is valid; move or add validation after
parseChainId (using parseChainId and the chainId variable) so that if chainId is
unsupported or invalid you return a client error via NextResponse.json({ error:
"chainId is required or unsupported" }, { status: 400 }) instead of letting
getClients throw and produce a 500; ensure the same validation is applied at the
later check around the logic that currently lives at lines 92-97 to keep
behavior consistent.
- Around line 68-90: The current builder for signedUserOp zero-fills gas/fee
fields (callGasLimit, verificationGasLimit, preVerificationGas, maxFeePerGas,
maxPriorityFeePerGas) which lets incomplete payloads pass; update the
construction/validation so these fields are required: remove the "?? '0'"
fallbacks and instead check raw.callGasLimit, raw.verificationGasLimit,
raw.preVerificationGas, raw.maxFeePerGas, and raw.maxPriorityFeePerGas for
null/undefined and return a 400 JSON error (similar to the existing
nonce/callData/signature checks) if any are missing or empty before creating
signedUserOp or submitting handleOps. Ensure you reference the signedUserOp
creation and the same validation pattern used for nonce/callData/signature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c7286a7-36f4-46c7-978c-53d2f1215b5c
📒 Files selected for processing (3)
app/api/bundler/execute-sponsored/route.tsapp/api/bundler/execute/route.tsapp/pages/TransactionPreview.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- app/api/bundler/execute-sponsored/route.ts
- app/pages/TransactionPreview.tsx
* Added new API routes for bundler execution and improved error responses for missing parameters. * Updated RPC URL handling to prevent SSRF vulnerabilities by using server-side configuration. * Refactored components and hooks to include authorization headers for API requests, enhancing security. * Improved chain ID validation to ensure it is a finite positive integer. * Cleaned up the .env.example file for better readability and consistency.
chibie
changed the title
* Adjusted the gas fee multipliers from 5,000 to 11,000 for both maxFeePerGas and maxPriorityFeePerGas to improve transaction cost estimation. * Ensured maxPriorityFeePerGas is calculated using minBigInt to prevent exceeding maxFeePerGas, enhancing fee accuracy and reliability.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
app/lib/bundler/userOp.ts (1)
823-832:⚠️ Potential issue | 🟠 MajorClamp the fallback priority fee to
maxFeePerGas.In the
gasPricebranches, low-fee networks can still producemaxPriorityFeePerGas > maxFeePerGas, which makes the returned EIP-1559 fee tuple invalid.Suggested change
- maxPriorityFeePerGas = maxBigInt(maxFeePerGas / 10n, 1_000_000n); + maxPriorityFeePerGas = minBigInt( + maxBigInt(maxFeePerGas / 10n, 1_000_000n), + maxFeePerGas + ); @@ - maxPriorityFeePerGas = maxBigInt(maxFeePerGas / 10n, 1_000_000n); + maxPriorityFeePerGas = minBigInt( + maxBigInt(maxFeePerGas / 10n, 1_000_000n), + maxFeePerGas + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/bundler/userOp.ts` around lines 823 - 832, In the gas-price fallback branches (handling fees.gasPrice bigint and the publicClient.getGasPrice() catch), clamp maxPriorityFeePerGas so it never exceeds maxFeePerGas: after computing maxFeePerGas and maxPriorityFeePerGas (using multiplyByBps and maxBigInt), set maxPriorityFeePerGas = min(maxPriorityFeePerGas, maxFeePerGas) (or use an equivalent helper) to ensure the returned EIP-1559 tuple is valid; update both the fees.gasPrice bigint branch and the publicClient.getGasPrice() fallback in userOp.ts accordingly.app/api/bundler/execute/route.ts (1)
76-109:⚠️ Potential issue | 🟠 MajorReject malformed
userOpfields with 400s.These checks only verify presence. Strings like
"abc"in gas/fee fields or non-hexcallData/signaturewill survive this block and then blow up later inBigInt()or ABI encoding, which currently turns a bad request into a 500.Suggested change
+ const numericFields = [ + "nonce", + "callGasLimit", + "verificationGasLimit", + "preVerificationGas", + "maxFeePerGas", + "maxPriorityFeePerGas", + ] as const; + for (const field of numericFields) { + try { + if (raw[field] == null || raw[field] === "") throw new Error(); + BigInt(String(raw[field])); + } catch { + return NextResponse.json( + { error: `userOp.${field} must be a bigint-compatible value` }, + { status: 400 } + ); + } + } + if (typeof raw.callData !== "string" || !raw.callData.startsWith("0x")) { + return NextResponse.json( + { error: "userOp.callData must be a 0x-prefixed hex string" }, + { status: 400 } + ); + } + if (typeof raw.signature !== "string" || !raw.signature.startsWith("0x")) { + return NextResponse.json( + { error: "userOp.signature must be a 0x-prefixed hex string" }, + { status: 400 } + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bundler/execute/route.ts` around lines 76 - 109, The current presence checks (missingGasOrFee) allow invalid strings like "abc" or non-hex data to reach BigInt/ABI and cause 500s; add explicit validation before building signedUserOp: verify numeric gas/fee/nonce fields (raw.callGasLimit, raw.verificationGasLimit, raw.preVerificationGas, raw.maxFeePerGas, raw.maxPriorityFeePerGas, raw.nonce) are parseable as non-negative integers (BigInt-parsable or /^\d+$/) and return NextResponse.json({error: ...}, {status: 400}) on failure; validate hex fields initCode, callData, paymasterAndData (if present), and signature are valid 0x-prefixed hex strings of allowed length (use /^0x[0-9a-fA-F]*$/ and signature length check) before creating the SerializedUserOperation object so malformed inputs produce 400s instead of 500s.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/bundler/nexus/status/route.ts`:
- Around line 9-40: The GET handler currently returns 400 for all errors; keep
the existing 400 responses for parameter validation (the smartAccountAddress
regex check) but change the outer catch to return status 500 for
runtime/RPC/provider failures (errors thrown by getClients or getNexusStatus).
Specifically, update the catch block in the exported async function GET to
detect non-validation errors and return NextResponse.json({ error: error
instanceof Error ? error.message : "Failed to check nexus status" }, { status:
500 }), while leaving the initial validation branch and its
NextResponse.json(..., { status: 400 }) unchanged so only bad input yields 400
and service/runtime failures yield 500.
In `@app/components/WalletTransferApprovalModal.tsx`:
- Around line 283-286: Replace the direct call to getAccessToken() with
getAccessTokenWithRetry() in WalletTransferApprovalModal so bundler requests get
a retried token; specifically, update the code that builds authHeaders
(currently using getAccessToken and falling back to {}) to call
getAccessTokenWithRetry(), await it, and use the returned token in the
Authorization header to avoid sending an empty header object for
middleware-protected /api/bundler/* calls.
In `@app/hooks/useSmartWalletTransfer.ts`:
- Line 132: The catch message is stale because the code hardcodes bundlerUrl =
"/api/bundler" in useSmartWalletTransfer.ts but still instructs users to set
NEXT_PUBLIC_BUNDLER_SERVER_URL; update the recovery text to match the
implementation: either (A) switch bundlerUrl to read from
process.env.NEXT_PUBLIC_BUNDLER_SERVER_URL (falling back to "/api/bundler") and
keep the existing env var troubleshooting message, or (B) keep the hardcoded
const bundlerUrl = "/api/bundler" and change the catch/log text to tell users
that the request targets the internal endpoint "/api/bundler" and to check
server routing/next.js API route availability (or contact the app owner) instead
of advising setting NEXT_PUBLIC_BUNDLER_SERVER_URL; apply the same change for
the messages around the other block (lines referenced near 239-249) so both
catch handlers are consistent.
- Around line 227-234: The code calls fetch(`${bundlerUrl}/execute-sponsored`)
after calling getAccessToken() which can return null; because the bundler routes
are middleware-protected you must require a non-null token before making the
request. Update the logic in useSmartWalletTransfer (the block around
getAccessToken, headers, and the fetch call) to check the returned value from
getAccessToken(), and if it is null either throw/return an explicit error or
surface a user-facing flow (e.g., request re-authentication) instead of calling
fetch without Authorization; ensure the headers.Authorization is only used when
a token exists and abort the network call when token === null so the protected
endpoint is not invoked with no credentials.
In `@app/lib/bundler/userOp.ts`:
- Around line 803-809: The code currently clamps preVerificationGas using
minBigInt(..., GAS_CONFIG.preVerificationGas) which forces large calldata
estimates back down; remove that upper clamp so the estimate can scale with
calldata. Specifically, replace the minBigInt call around
estimatedPreVerification/maxBigInt with a simple assignment that uses
preVerificationGas = maxBigInt(estimatedPreVerification, 45_000n) (keep
calldataBytes and estimatedPreVerification calculations intact) and stop
applying GAS_CONFIG.preVerificationGas as an upper bound in this calculation.
---
Duplicate comments:
In `@app/api/bundler/execute/route.ts`:
- Around line 76-109: The current presence checks (missingGasOrFee) allow
invalid strings like "abc" or non-hex data to reach BigInt/ABI and cause 500s;
add explicit validation before building signedUserOp: verify numeric
gas/fee/nonce fields (raw.callGasLimit, raw.verificationGasLimit,
raw.preVerificationGas, raw.maxFeePerGas, raw.maxPriorityFeePerGas, raw.nonce)
are parseable as non-negative integers (BigInt-parsable or /^\d+$/) and return
NextResponse.json({error: ...}, {status: 400}) on failure; validate hex fields
initCode, callData, paymasterAndData (if present), and signature are valid
0x-prefixed hex strings of allowed length (use /^0x[0-9a-fA-F]*$/ and signature
length check) before creating the SerializedUserOperation object so malformed
inputs produce 400s instead of 500s.
In `@app/lib/bundler/userOp.ts`:
- Around line 823-832: In the gas-price fallback branches (handling
fees.gasPrice bigint and the publicClient.getGasPrice() catch), clamp
maxPriorityFeePerGas so it never exceeds maxFeePerGas: after computing
maxFeePerGas and maxPriorityFeePerGas (using multiplyByBps and maxBigInt), set
maxPriorityFeePerGas = min(maxPriorityFeePerGas, maxFeePerGas) (or use an
equivalent helper) to ensure the returned EIP-1559 tuple is valid; update both
the fees.gasPrice bigint branch and the publicClient.getGasPrice() fallback in
userOp.ts accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 245c5dde-6bec-4d1c-afc5-43b3e561601e
📒 Files selected for processing (13)
.env.exampleapp/api/bundler/execute-sponsored/route.tsapp/api/bundler/execute/route.tsapp/api/bundler/generate-transfer-userop/route.tsapp/api/bundler/generate-userop/route.tsapp/api/bundler/is-nexus/route.tsapp/api/bundler/nexus/status/route.tsapp/components/WalletTransferApprovalModal.tsxapp/hooks/useSmartWalletTransfer.tsapp/lib/bundler/chains.tsapp/lib/bundler/userOp.tsapp/pages/TransactionPreview.tsxmiddleware.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- app/api/bundler/is-nexus/route.ts
- .env.example
- app/pages/TransactionPreview.tsx
- app/lib/bundler/chains.ts
- app/api/bundler/execute-sponsored/route.ts
- app/api/bundler/generate-transfer-userop/route.ts
- app/api/bundler/generate-userop/route.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/lib/bundler/userOp.ts (1)
806-809:⚠️ Potential issue | 🟠 MajorRemove the
preVerificationGashard ceiling.Line [806] caps dynamic estimation with
GAS_CONFIG.preVerificationGas, which can underprice large calldata userOps and cause pre-verification failure.Suggested fix
- preVerificationGas = minBigInt( - maxBigInt(estimatedPreVerification, 45_000n), - GAS_CONFIG.preVerificationGas - ); + preVerificationGas = maxBigInt( + estimatedPreVerification, + GAS_CONFIG.preVerificationGas + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/bundler/userOp.ts` around lines 806 - 809, The code currently forces preVerificationGas to be minBigInt(maxBigInt(estimatedPreVerification, 45_000n), GAS_CONFIG.preVerificationGas), which applies a hard ceiling (GAS_CONFIG.preVerificationGas) that can underprice large calldata userOps; remove that ceiling so preVerificationGas is computed without capping by GAS_CONFIG.preVerificationGas (e.g. compute preVerificationGas as maxBigInt(estimatedPreVerification, 45_000n) or equivalent), updating the expression that sets preVerificationGas and any logic that relies on it (references: preVerificationGas, estimatedPreVerification, maxBigInt, minBigInt, GAS_CONFIG.preVerificationGas).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/lib/bundler/userOp.ts`:
- Around line 618-628: generateUserOp is ignoring paymaster options because the
sponsorship block calling fetchPaymasterAndData was commented out; restore that
logic so when options.paymasterUrl (and optional options.paymasterApiKey) are
provided you call fetchPaymasterAndData(userOp, options.paymasterUrl,
options.paymasterApiKey), assign userOp.paymasterAndData =
sponsor.paymasterAndData and set numeric fields (preVerificationGas,
verificationGasLimit, callGasLimit) to BigInt if present; keep the default '0x'
fallback when no sponsor is returned and ensure any exceptions from
fetchPaymasterAndData are caught/logged so generateUserOp still returns a valid
userOp.
- Around line 202-212: The current parsing in the hex handling branch is too
permissive and may return bogus addresses; tighten it by explicitly validating
the revert selector and exact payload layout before slicing out the address:
check raw.slice(0,8) equals the known 4-byte selector (0x6ca7b806) and that
raw.length is at least the expected 4+32 bytes (i.e., 8+64 hex chars) before
extracting addrHex from the last 40 chars, and in the fallback branch only
accept fixed-length (or well-formed) payloads (validate raw.length and expected
padding/offset) before using raw.slice(...) and calling getAddress; use the
existing identifiers (hex, raw, addrHex, getAddress) and return a
failure/undefined or throw if validations fail instead of returning a
potentially bogus address.
---
Duplicate comments:
In `@app/lib/bundler/userOp.ts`:
- Around line 806-809: The code currently forces preVerificationGas to be
minBigInt(maxBigInt(estimatedPreVerification, 45_000n),
GAS_CONFIG.preVerificationGas), which applies a hard ceiling
(GAS_CONFIG.preVerificationGas) that can underprice large calldata userOps;
remove that ceiling so preVerificationGas is computed without capping by
GAS_CONFIG.preVerificationGas (e.g. compute preVerificationGas as
maxBigInt(estimatedPreVerification, 45_000n) or equivalent), updating the
expression that sets preVerificationGas and any logic that relies on it
(references: preVerificationGas, estimatedPreVerification, maxBigInt, minBigInt,
GAS_CONFIG.preVerificationGas).
* Changed error response status from 400 to 500 in the nexus status route to reflect server errors. * Refactored WalletTransferApprovalModal to use getAccessTokenWithRetry for improved token retrieval. * Enhanced error handling in useSmartWalletTransfer to throw an error if authentication is required, ensuring better user feedback.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/api/bundler/nexus/status/route.ts (1)
10-23:⚠️ Potential issue | 🟠 MajorReturn 400 for invalid
chainIdand reserve 500 for runtime failures.Line 22 validation errors currently fall into the outer catch (Line 33), so bad input is returned as 500. Split validation from runtime handling so client errors stay 400.
Suggested fix
export async function GET(request: NextRequest) { - try { - const { searchParams } = request.nextUrl; - const smartAccountAddress = searchParams.get("smartAccountAddress"); - const chainIdRaw = searchParams.get("chainId"); + const { searchParams } = request.nextUrl; + const smartAccountAddress = searchParams.get("smartAccountAddress"); + const chainIdRaw = searchParams.get("chainId"); - if (!smartAccountAddress || !/^0x[0-9a-fA-F]{40}$/.test(smartAccountAddress)) { - return NextResponse.json( - { error: "Query param smartAccountAddress (0x + 40 hex) is required" }, - { status: 400 } - ); - } + if (!smartAccountAddress || !/^0x[0-9a-fA-F]{40}$/.test(smartAccountAddress)) { + return NextResponse.json( + { error: "Query param smartAccountAddress (0x + 40 hex) is required" }, + { status: 400 } + ); + } - const chainId = parseChainId(chainIdRaw); + let chainId: number; + try { + chainId = parseChainId(chainIdRaw); + } catch (error) { + return NextResponse.json( + { error: error instanceof Error ? error.message : "Invalid chainId" }, + { status: 400 } + ); + } + + try { const rpcUrl = parseRpcUrl(chainId); const { publicClient } = getClients(chainId, rpcUrl, false); const status = await getNexusStatus(publicClient, smartAccountAddress as `0x${string}`); @@ - } catch (error) { + } catch (error) { return NextResponse.json( { error: error instanceof Error ? error.message : "Failed to check nexus status", }, { status: 500 } ); } }Also applies to: 33-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bundler/nexus/status/route.ts` around lines 10 - 23, The handler currently lets invalid chainIdRaw errors bubble to the outer catch and return 500; before calling parseRpcUrl or other runtime code, validate chainIdRaw and handle parseChainId failures as client errors: check that chainIdRaw exists and is a valid chain id (e.g., numeric or matches expected pattern) or call parseChainId inside a small try/catch and if it throws return NextResponse.json({ error: "Invalid chainId" }, { status: 400 }); then continue to call parseRpcUrl(chainId) and the rest; keep the outer try/catch for true runtime failures so validation returns 400 while unexpected errors still return 500.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/bundler/nexus/status/route.ts`:
- Around line 6-7: The route docstring incorrectly lists rpcUrl as a query
parameter; update the comment above the GET handler (the docstring for the Nexus
status route in route.ts) to remove `rpcUrl` from the example query string and
description so it only documents `smartAccountAddress` and `chainId` (the
handler computes rpcUrl via parseRpcUrl(chainId)). Ensure the GET
/api/bundler/nexus/status line and any accompanying explanatory text reflect
that rpcUrl is not accepted as a query param.
---
Duplicate comments:
In `@app/api/bundler/nexus/status/route.ts`:
- Around line 10-23: The handler currently lets invalid chainIdRaw errors bubble
to the outer catch and return 500; before calling parseRpcUrl or other runtime
code, validate chainIdRaw and handle parseChainId failures as client errors:
check that chainIdRaw exists and is a valid chain id (e.g., numeric or matches
expected pattern) or call parseChainId inside a small try/catch and if it throws
return NextResponse.json({ error: "Invalid chainId" }, { status: 400 }); then
continue to call parseRpcUrl(chainId) and the rest; keep the outer try/catch for
true runtime failures so validation returns 400 while unexpected errors still
return 500.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b99ac7b5-f764-495c-b7ff-f00b2f4fd624
📒 Files selected for processing (3)
app/api/bundler/nexus/status/route.tsapp/components/WalletTransferApprovalModal.tsxapp/hooks/useSmartWalletTransfer.ts
* Revised API documentation to clarify query parameters for the Nexus status endpoint, removing the RPC URL requirement. * Enhanced error handling in getSenderAddressFromInitCode to streamline address extraction from revert data, improving reliability and clarity in error responses.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
References
Testing
Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
New Features
Chores
Other