♻️ app: prefer sendCalls, fix chain handling#879
♻️ app: prefer sendCalls, fix chain handling#879dieguezguille wants to merge 11 commits intotransportfrom
sendCalls, fix chain handling#879Conversation
🦋 Changeset detectedLatest commit: 80a1e20 The changes in this PR will be included in the next version bump. 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 |
|
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:
WalkthroughAdds chain-scoped context to many on-chain hooks, converts direct contract writes to mutation-driven sendCalls + waitForCallsStatus (paymaster integration), refactors Bridge to use Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Mobile Client
participant Mutate as useMutation (swap/send)
participant SendCalls as useSendCalls
participant Paymaster as Alchemy Paymaster
participant Chain as Blockchain Network
participant Status as waitForCallsStatus
Client->>Mutate: trigger swap/proposal (encoded calls + chainId)
Mutate->>SendCalls: submit calls (paymaster policy, chainId)
SendCalls->>Paymaster: request sponsored submission
Paymaster->>Chain: submit transaction
Chain->>Chain: execute calls
Mutate->>Status: poll call status with chainId
Status->>Chain: query receipt/status
Chain->>Status: return status
Status->>Mutate: resolve success/failure
Mutate->>Client: return result
sequenceDiagram
participant Component as UI Component
participant ChainObj as chain (generated)
participant Hook as Data Hook
participant Query as On-chain Query Layer
Component->>ChainObj: import chain
Component->>Hook: call hook with { ..., chainId: chain.id }
Hook->>Query: execute query scoped by chainId
Query->>Hook: return chain-specific data
Hook->>Component: provide scoped results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
cruzdanilo
changed the title
sendCalls, fix chain handling
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors core transaction logic to leverage Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## transport #879 +/- ##
=============================================
- Coverage 71.67% 71.44% -0.23%
=============================================
Files 228 228
Lines 8283 8315 +32
Branches 2662 2671 +9
=============================================
+ Hits 5937 5941 +4
- Misses 2116 2145 +29
+ Partials 230 229 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const { calls, chainId, from, id } = params[0] as { | ||
| calls: readonly Call[]; | ||
| chainId?: Hex; | ||
| from?: Address; | ||
| id?: string; | ||
| }; | ||
| if (from && from !== accountAddress) throw new Error("bad account"); | ||
| if (queryClient.getQueryData<AuthMethod>(["method"]) === "webauthn") { | ||
| const { hash } = await client.sendUserOperation({ |
There was a problem hiding this comment.
🚩 webauthn path in accountClient ignores chainId parameter — safe for current callers
The wallet_sendCalls handler at src/utils/accountClient.ts:165 now extracts chainId from params but the webauthn path (lines 172-176) ignores it entirely, always sending user operations on the default chain and encoding chain.id in the response. This is currently safe because all callers that go through the smart account config (exa) don't pass explicit chainId — the bridge uses ownerConfig directly. However, if a future caller passes a non-default chainId through the exa config with webauthn auth, the transaction would silently execute on the wrong chain.
(Refers to lines 165-176)
Was this helpful? React with 👍 or 👎 to provide feedback.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
calls
branch
2 times, most recently
from
991a102 to
80f63cc
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/pay-mode/Pay.tsx (1)
473-501:⚠️ Potential issue | 🟠 MajorAdd
chainId: chain.idto external assetmutateSendCallsinvocation.Same issue as the other mutation: the external repay path should also specify the chain to prevent cross-chain mismatches.
🔧 Proposed fix
const { id } = await mutateSendCalls({ + chainId: chain.id, calls: [ { to: selectedAsset.address, abi: erc20Abi, functionName: "approve", args: [swapperAddress, route.fromAmount], }, // ... rest of calls ], capabilities: { paymasterService: { url: `${chain.rpcUrls.alchemy.http[0]}/${alchemyAPIKey}`, context: { policyId: alchemyGasPolicyId }, }, }, });
♻️ Duplicate comments (6)
.changeset/open-beds-stand.md (1)
1-5:⚠️ Potential issue | 🟠 MajorRewrite or drop this stale changeset entry.
This note still says “add bottom padding to swaps screen on web”, but this PR is about
sendCallsand chain handling. If the change is internal, this should be an empty changeset; otherwise, rewrite it to the actual user-visible fix.Based on learnings, empty changesets are required for non-user-facing changes, and changeset summaries in
.changeset/*.mdmust start with a gitmoji and follow the format<emoji> <message>(no scope).src/components/shared/PluginUpgrade.tsx (1)
33-46:⚠️ Potential issue | 🔴 CriticalPin the upgrade write path to
chain.idas well.These reads are now scoped to
chain.id, but the batch submission and status polling below still default to the active wallet chain. If the wallet is on another network, the upgrade can be submitted or polled against a different chain than the one you just simulated.🔧 Minimal fix
const { id } = await mutateSendCalls({ + chainId: chain.id, calls: [ { ...uninstallPluginSimulation.request, to: address }, { to: address, @@ capabilities: { paymasterService: { url: `${chain.rpcUrls.alchemy.http[0]}/${alchemyAPIKey}`, context: { policyId: alchemyGasPolicyId }, }, }, }); - const { status } = await waitForCallsStatus(exa, { id }); + const { status } = await waitForCallsStatus(exa, { id, chainId: chain.id });In wagmi, do useSendCalls/sendCalls mutation variables and waitForCallsStatus accept a chainId parameter, and should it be passed when reads/simulations are already pinned to a specific chain?Also applies to: 59-85
src/components/swaps/Swaps.tsx (1)
339-380:⚠️ Potential issue | 🔴 CriticalPin the swap submission and status polling to
chain.id.The simulations above are scoped to
chain.id, but this batch write path still uses the wallet’s active chain by default. That can send the swap on a different network than the one you just simulated, or poll the wrong chain for status.🔧 Minimal fix
const { id } = await mutateSendCalls({ + chainId: chain.id, calls: [call], capabilities: { paymasterService: { url: `${chain.rpcUrls.alchemy.http[0]}/${alchemyAPIKey}`, context: { policyId: alchemyGasPolicyId }, }, }, }); - const { status } = await waitForCallsStatus(exaConfig, { id }); + const { status } = await waitForCallsStatus(exaConfig, { id, chainId: chain.id });In wagmi, do useSendCalls/sendCalls mutation variables and waitForCallsStatus accept a chainId parameter, and should it be passed when reads/simulations are already pinned to a specific chain?src/components/roll-debt/RollDebt.tsx (1)
290-300:⚠️ Potential issue | 🟠 MajorAdd
chainId: chain.idto themutateSendCallsinvocation.The mutation uses chain-scoped simulation and polling, but
mutateSendCallsis called without an explicitchainId. This can cause the transaction to be submitted on the wallet's currently connected chain rather than the intended chain. The past review comment flagged this issue as addressed, but the fix appears to be missing.🔧 Proposed fix
const { id } = await mutateSendCalls({ + chainId: chain.id, calls: [{ to, data: encodeFunctionData({ abi, functionName, args }) }], capabilities: { paymasterService: { url: `${chain.rpcUrls.alchemy.http[0]}/${alchemyAPIKey}`, context: { policyId: alchemyGasPolicyId }, }, }, });src/components/send-funds/Amount.tsx (1)
165-178:⚠️ Potential issue | 🟠 MajorAdd
chainId: chain.idto themutateSendCallsinvocation.All simulations in this component are chain-scoped, but
mutateSendCallsis called without an explicitchainId. This can result in the transaction being submitted on the wallet's currently connected chain instead of the intended chain. The subsequentwaitForCallsStatuscall uses the single-chainexaconfig, which may fail to poll status correctly if chains mismatch.🔧 Proposed fix
const sendCalls = async (calls: readonly { data?: `0x${string}`; to: `0x${string}`; value?: bigint }[]) => { const { id } = await mutateSendCalls({ + chainId: chain.id, calls, capabilities: { paymasterService: { url: `${chain.rpcUrls.alchemy.http[0]}/${alchemyAPIKey}`, context: { policyId: alchemyGasPolicyId }, }, }, });src/components/pay-mode/Pay.tsx (1)
427-435:⚠️ Potential issue | 🟠 MajorAdd
chainId: chain.idtomutateSendCallsinvocation.All reads and simulations in this component explicitly pin to
chain.id, but this mutation executes calls without specifying the chain. A wallet connected to a different network could submit the transaction on the wrong chain whilewaitForCallsStatuspolls onexa's configured chain.🔧 Proposed fix
const { id } = await mutateSendCalls({ + chainId: chain.id, calls: [call], capabilities: { paymasterService: { url: `${chain.rpcUrls.alchemy.http[0]}/${alchemyAPIKey}`, context: { policyId: alchemyGasPolicyId }, }, }, });
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f49faff4-bf2f-4d46-a9e9-1b0dacece46f
📒 Files selected for processing (48)
.changeset/bright-foxes-swim.md.changeset/cool-icons-grow.md.changeset/curly-pumas-jam.md.changeset/cyan-flies-camp.md.changeset/funny-aliens-mix.md.changeset/gold-cow-eat.md.changeset/great-dryers-kick.md.changeset/jolly-teeth-flow.md.changeset/loose-papers-take.md.changeset/open-beds-stand.md.changeset/wide-cats-hug.mdsrc/components/add-funds/Bridge.tsxsrc/components/card/Card.tsxsrc/components/card/exa-card/CardContents.tsxsrc/components/defi/DeFi.tsxsrc/components/getting-started/GettingStarted.tsxsrc/components/home/AssetList.tsxsrc/components/home/CardLimits.tsxsrc/components/home/Home.tsxsrc/components/home/HomeActions.tsxsrc/components/home/Portfolio.tsxsrc/components/home/card-upgrade/UpgradeAccount.tsxsrc/components/loans/Amount.tsxsrc/components/loans/Asset.tsxsrc/components/loans/CreditLine.tsxsrc/components/loans/LoanSummary.tsxsrc/components/loans/Loans.tsxsrc/components/loans/Review.tsxsrc/components/pay-mode/OverduePayments.tsxsrc/components/pay-mode/Pay.tsxsrc/components/pay-mode/PayMode.tsxsrc/components/pay-mode/PaySelector.tsxsrc/components/pay-mode/RepayAmountSelector.tsxsrc/components/pay-mode/UpcomingPayments.tsxsrc/components/roll-debt/RollDebt.tsxsrc/components/send-funds/Amount.tsxsrc/components/shared/InstallmentSelector.tsxsrc/components/shared/PluginUpgrade.tsxsrc/components/swaps/Failure.tsxsrc/components/swaps/Pending.tsxsrc/components/swaps/Success.tsxsrc/components/swaps/Swaps.tsxsrc/utils/accountClient.tssrc/utils/useAsset.tssrc/utils/useAuth.tssrc/utils/usePendingOperations.tssrc/utils/usePortfolio.tssrc/utils/useSimulateProposal.ts
| --- | ||
| "@exactly/mobile": patch | ||
| --- | ||
|
|
||
| ♻️ migrate remaining flows to send calls |
There was a problem hiding this comment.
Make this changeset empty unless the refactor is user-visible.
migrate remaining flows to send calls reads like an internal implementation detail, not a release note. If this is only transaction-plumbing work, this should be an empty changeset; otherwise, rewrite it in user-facing terms. Based on learnings, empty changesets are required in this repo for non-user-facing changes that do not warrant release notes.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20f2f6d3e7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const { calls, chainId, from, id } = params[0] as { | ||
| calls: readonly Call[]; | ||
| chainId?: Hex; |
There was a problem hiding this comment.
wallet_sendCalls now accepts a chainId, but this code path does not apply it for webauthn: it always sends via the default chain client and returns an id encoded with chain.id. If a caller requests a different network, webauthn users will submit on the wrong chain (or poll status against the wrong chain metadata), which breaks chain-aware send flows. Please either reject non-default chainId for webauthn explicitly or execute against a client configured for the requested chain.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@coderabbitai are we passing the chain id in the webauthn flow in the latest force-pushed code?
There was a problem hiding this comment.
@dieguezguille — looking at the current file content, the chain ID is partially handled in the webauthn flow:
requestedChainIdis now correctly computed from the incomingchainIdparam (or falls back tochain.id).- It is encoded into the returned ID:
This means
return { id: concat([hash, numberToHex(requestedChainId, { size: 32 }), UO_MAGIC_ID]) };
wallet_getCallsStatuswill read back the correct chain from the ID — addressing the status-polling concern.
However, the actual execution — client.sendUserOperation(...) — still uses the client instance that was constructed at startup with the default chain. There is no chain-switching or alternate bundler client being used if a different chainId is requested.
So to summarize:
- ✅ The returned ID now encodes
requestedChainId(status polling is correct). - ❌ The user operation is still submitted via the bundler client bound to the default
chain, regardless of whatchainIdwas requested.
If in your app the webauthn flow is only ever called with the default chain, this is fine in practice. But if cross-chain calls are possible for webauthn users, the execution still won't target the requested chain.
🧠 Learnings used
Learnt from: cruzdanilo
Repo: exactly/exa PR: 633
File: src/utils/accountClient.ts:188-203
Timestamp: 2026-01-12T20:25:48.408Z
Learning: In src/utils/accountClient.ts, the wallet_getCallsStatus handler explicitly checks for UO_MAGIC_ID to handle user operation receipts, while TX_MAGIC_ID falls through to wagmi's getCallsStatus, which automatically handles the custom ID format. This is intentional—viem/wagmi natively supports TX_MAGIC_ID, and the UO_MAGIC_ID scheme was inspired by this pattern. When reviewing changes or extending support for additional magic IDs, ensure new IDs are compatible with wagmi/viem expectations and do not inadvertently bypass intended handling. This guidance is specific to this file (and related ID handling patterns) and should not be generalized beyond this context.
Learnt from: cruzdanilo
Repo: exactly/exa PR: 669
File: src/utils/queryClient.ts:55-63
Timestamp: 2026-01-22T21:34:44.852Z
Learning: When a function returns Promisable<T> (defined as T | PromiseLike<T>), wrap the returned value with Promise.resolve() to ensure the function always returns a proper Promise<T>. This provides a consistent asynchronous API and avoids mixed sync/async return types. Apply this pattern to TypeScript code handling Promisable<T>, including files like src/utils/queryClient.ts and other TS files in the repository.
Learnt from: itofarina
Repo: exactly/exa PR: 807
File: contracts/script/Redeployer.s.sol:101-101
Timestamp: 2026-02-19T16:02:22.395Z
Learning: Trailing commas should be used in TypeScript and JavaScript files (extensions .ts, .tsx, .js, .jsx) to improve diffs and reduce merge conflicts. Do NOT use trailing commas in Solidity (*.sol) or standard JSON (*.json) files, as they are syntax errors in those languages. Apply this guidance across the repository for files matching the listed extensions; ensure tooling (linters/formatters) are configured accordingly to enforce the rule.
Learnt from: mainqueg
Repo: exactly/exa PR: 896
File: server/hooks/bridge.ts:115-115
Timestamp: 2026-03-20T13:41:53.508Z
Learning: In this TypeScript codebase (with `noUncheckedIndexedAccess` enabled), array index access like `arr[0]` is typed as `T | undefined`. A preceding length guard such as `arr.length === 1` does not narrow the specific index access to non-undefined. Therefore, do not recommend removing optional chaining (e.g., `arr[0]?.prop`) based solely on a preceding `.length` check; keep `?.` when the index access type includes `undefined`.
Learnt from: cruzdanilo
Repo: exactly/exa PR: 914
File: src/utils/useSimulateProposal.ts:82-94
Timestamp: 2026-03-25T23:18:25.761Z
Learning: In exactly/exa, `viem`’s exported `multicall3Abi` is only the subset of the Multicall3 contract ABI covering aggregation functions (e.g., `aggregate3`), and it does not include utility/view functions such as `getBlockNumber` (and likely not `getCurrentBlockTimestamp`). When using `useReadContracts` to call Multicall3 utility functions (e.g., `getBlockNumber`, `getCurrentBlockTimestamp`), provide an inline ABI for those specific function(s) instead of reusing `multicall3Abi`. Do not suggest replacing inline ABIs with `multicall3Abi` for Multicall3 utility/view calls.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dieguezguille
force-pushed
the
transport
branch
from
ae0aad2 to
89535f1
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1625103ae8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }, | ||
| onSuccess() { | ||
| queryClient.invalidateQueries({ queryKey: ["lifi", "tokenBalances"] }).catch(reportError); | ||
| queryClient.invalidateQueries({ queryKey: marketsQueryKey }).catch(reportError); |
There was a problem hiding this comment.
marketsQueryKey is read from useMarkets(), but that hook does not define a dedicated markets key in its return value, so this value can be undefined; passing undefined to invalidateQueries broadens the filter and can invalidate/refetch the entire cache after each successful swap. That adds avoidable network churn and UI refreshes in unrelated screens whenever a swap succeeds. Use an explicit markets query key (or expose one from useMarkets) before calling invalidateQueries here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b296cd1f1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }, | ||
| }, | ||
| }); | ||
| const { status } = await waitForCallsStatus(exaConfig, { id }); |
There was a problem hiding this comment.
This path now hard-requires waitForCallsStatus, but the account client can still fall back from wallet_sendCalls to a plain sendTransaction when the owner wallet lacks 5792 support. In that fallback, the returned id is not reliably resolvable by wallet_getCallsStatus, so this await can throw even after the transaction was actually sent/mined, turning successful swaps into failures for those wallets. Please add a fallback receipt path (or ensure fallback ids are fully supported by status polling) before treating this as a hard failure.
Useful? React with 👍 / 👎.
| const amount = withUSDC ? repayAssets : route?.fromAmount; | ||
| if (!amount) throw new Error("no route"); |
There was a problem hiding this comment.
The new guard enforces route?.fromAmount for every non-USDC repayment mode, but legacyCrossRepay submits legacyCrossRepaySimulation.request and does not consume route payload in its call construction. If LI.FI quoting is unavailable, users on the legacy plugin can no longer repay even when on-chain simulation is ready. This introduces an unnecessary external dependency in a flow that previously could proceed.
Useful? React with 👍 / 👎.
| function walkCause(error: unknown): unknown { | ||
| if (typeof error === "object" && error !== null && "cause" in error && error.cause !== undefined) | ||
| return walkCause(error.cause); | ||
| return error; |
There was a problem hiding this comment.
🚩 walkCause traverses to deepest error cause, changing which properties are examined
The new walkCause function at src/utils/reportError.ts:166-169 recursively walks the .cause chain to the deepest error. This changes parseError to extract code, status, name, and message from the innermost cause rather than the top-level error. For standard wallet rejection flows (MetaMask, injected wallets), the deepest cause is typically the raw RPC error object { code: 4001, message: "..." }, so walletRejected (status === "4001") is correctly detected. However, for some wallet connectors where the deepest cause is a plain Error("User rejected...") without a code property, walletRejected would be false. This matters critically in src/utils/accountClient.ts:194 where classifyError(error).authKnown gates whether a rejection is re-thrown vs. falling through to the sendTransaction fallback. The owner config at src/utils/wagmi/owner.ts:15 uses injected() and farcasterMiniApp() connectors (no WalletConnect), so the risk is limited to those specific connector error structures. Note also that revert and reason at lines 101-102 still use the original error, not root, creating an intentional asymmetry.
Was this helpful? React with 👍 or 👎 to provide feedback.
dieguezguille
force-pushed
the
transport
branch
from
ca87c0e to
6acd9ae
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80a1e20ca0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const result = await waitForCallsStatus(exa, { id }); | ||
| if (result.status === "failure") throw new Error("failed to send"); |
There was a problem hiding this comment.
This new send path hard-requires waitForCallsStatus, but the SIWE account client can still fall back from wallet_sendCalls to sendTransaction when 5792 is unavailable. In that fallback case the returned id is tx-based (not reliably resolvable by wallet_getCallsStatus), so this await can throw even after the transfer is actually mined, incorrectly surfacing a failed send. Please add a receipt/hash fallback path (or status handling for tx-style ids) before treating this as a hard failure.
Useful? React with 👍 / 👎.
| tokenSelectionType === "to" | ||
| ? selected.address === fromToken.token.address | ||
| ? parseUnits(formatUnits(old.fromAmount, fromToken.token.decimals), toToken.token.decimals) | ||
| : old.fromAmount |
There was a problem hiding this comment.
This branch converts old.fromAmount, but after the refactor that stopped syncing route results back into swap state, old.fromAmount can be stale whenever activeInput === "to" (the displayed source amount is now route.fromAmount). If the user swaps sides by selecting the current source token in the destination picker, this can overwrite the live quoted amount with an outdated value. Use the effective current source amount instead of persisted old.fromAmount in this path.
Useful? React with 👍 / 👎.
| dropError: (error) => { | ||
| if (error instanceof ValiError) return true; | ||
| if (error instanceof UserRejectedRequestError) return true; | ||
| const { passkeyCancelled, passkeyNotAllowed } = classifyError(error); | ||
| return passkeyCancelled || passkeyNotAllowed; | ||
| }, |
There was a problem hiding this comment.
🟡 Auth query dropError no longer drops wallet rejection errors for SIWE flow
The dropError handler for the ["auth"] query in server.ts:65-69 removed the error instanceof UserRejectedRequestError check but only checks passkeyCancelled || passkeyNotAllowed, which are passkey-specific classifications. In the SIWE auth flow, when a user rejects a wallet signature, viem throws a UserRejectedRequestError. The new classifyError correctly identifies this as walletRejected (via status === "4001") and authKnown, but dropError doesn't check those properties. As a result, the wallet rejection error is NOT dropped — it flows through the QueryCache.onError at src/utils/queryClient.ts:44-56 and is reported to Sentry (as a warning-level event since knownWarning includes walletRejected). Previously this was silently dropped. The dropError should also return true for authKnown errors (or at least walletRejected).
| dropError: (error) => { | |
| if (error instanceof ValiError) return true; | |
| if (error instanceof UserRejectedRequestError) return true; | |
| const { passkeyCancelled, passkeyNotAllowed } = classifyError(error); | |
| return passkeyCancelled || passkeyNotAllowed; | |
| }, | |
| dropError: (error) => { | |
| if (error instanceof ValiError) return true; | |
| const { passkeyCancelled, passkeyNotAllowed, authKnown } = classifyError(error); | |
| return passkeyCancelled || passkeyNotAllowed || authKnown; | |
| }, |
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary by CodeRabbit
Bug Fixes
Improvements
Style