feat: allow EIP7702 wallets to batch transactions#7844
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
WalkthroughTWAP now derives wallet support from Safe metadata and wallet capabilities. Unsupported wallets receive a dedicated form state, disabled action button, warning, and analytics event. Capability resolution no longer falls back to arbitrary chains, and atomic-batch support uses capability status fields. ChangesTWAP wallet support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant WalletMetadata
participant useTwapFormState
participant getTwapFormState
participant TwapFormWarnings
participant PrimaryActionButton
WalletMetadata->>useTwapFormState: provide Safe wallet metadata
useTwapFormState->>getTwapFormState: pass wallet and bundling support
getTwapFormState-->>TwapFormWarnings: return WALLET_NOT_SUPPORTED
getTwapFormState-->>PrimaryActionButton: return WALLET_NOT_SUPPORTED
TwapFormWarnings-->>PrimaryActionButton: render unsupported wallet warning and disabled action
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Deploying explorer-dev with
|
| Latest commit: |
0eb7628
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://14cdcc4c.explorer-dev-dxz.pages.dev |
| Branch Preview URL: | https://feat-7702-batch-support.explorer-dev-dxz.pages.dev |
| * WalletConnect in mobile browsers initiates a request with confirmation to the wallet | ||
| * to get the capabilities. It breaks the flow with perpetual requests. | ||
| */ | ||
| export async function getShouldSkipCapabilitiesCheck( |
There was a problem hiding this comment.
This function was removed in #7742 and replaced with a 5s timeout. It was reintroduced during conflict resolution.
There was a problem hiding this comment.
Assuming all the test scenarios are ok, I'm fine with it. For me, when resolving some of the conflicts I found around this, it was not working as expected in some scenarios. But if we are going to check for batching support for EOAs then this is probably not relevant anymore anyway.
Deploying swap-dev with
|
| Latest commit: |
0eb7628
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cb4b7b83.swap-dev-5u6.pages.dev |
| Branch Preview URL: | https://feat-7702-batch-support.swap-dev-5u6.pages.dev |
|
|
||
| logWallet.debug('Fetching wallet capabilities', { account, chainId }) | ||
|
|
||
| // TODO remove this completely: it doesn't resolve capabilities for no wallet: MM, Safe, Ambire. |
There was a problem hiding this comment.
From my testing wagmi is useless, it never resolves capabilities...
There was a problem hiding this comment.
Fixed in #7847, and removed the fallback
kernelwhisperer
changed the title
elena-zh
left a comment
elena-zh
left a comment
There was a problem hiding this comment.
Hey @kernelwhisperer , thank you.
Base smart account is working perfectly fine now.
What is not working/not working as expected:
- Safe+WC: bundling stopped working
- MM smart account: now it does not use gasless transactions for placing orders with permittable tokens
- MM+Smart account: impossible to place a ETH-flow order without wrapping
4 MM+regular account (not smart one): impossible to sign a permit
- MM+ regular account on Avalanche, Plasma, Ink (chains that do not support smart accounts): impossible to place an order with a gasless permit inside
Could you please review this issues and fix them?
fairlighteth
left a comment
fairlighteth
left a comment
There was a problem hiding this comment.
⚠️ AI Review (Codex GPT-5, worked 25m): atomic batching can partially execute or use wrong-chain capabilities
Finding: [BLOCKING] Require atomic execution for ready wallets
- Location:
libs/wallet/src/api/state/walletCapabilitiesAtom.ts:177andlibs/wallet/src/api/hooks/useSendBatchTransactions.ts:36 - This one is important: the PR accepts
atomic.status === 'ready', but the sender omitsforceAtomic. - viem defaults
forceAtomictofalse, producingatomicRequired: false. EIP-5792 then permits sequential execution and does not require areadywallet to upgrade. - Approve/presign and wrap/approve/presign flows can partially execute, leaving allowances or wrapped funds behind, or activating a posted order after token preparation failed.
- Security classification:
verified-vulnerability,P2 (Medium). A targeted viem reproduction capturedatomicRequiredas[false, true]without and withforceAtomic.
Suggested fix
- Pass
forceAtomic: truefromuseSendBatchTransactions. - Add a boundary test asserting the resulting
wallet_sendCalls.atomicRequired, including upgrade rejection and unsupported atomicity.
Finding: [BLOCKING] Do not reuse another chain's batching capability
- Location:
libs/wallet/src/api/state/walletCapabilitiesAtom.ts:35 resolveCapabilitiesForChainreturns the first response entry when the active chain is absent. EIP-5792 states that atomic support is chain-specific and an absent chain means unsupported.- With
readynow accepted, a Mainnet entry can incorrectly enable batching on Avalanche, Plasma, or Ink. - Commit
171de9b93previously restricted this fallback to confirmed Safe-via-WalletConnect connections specifically to prevent non-Safe cross-chain false positives.
Suggested fix
- Require the exact numeric/hex active-chain key for non-Safe wallets. Preserve a Safe-WC fallback only if it remains necessary.
- Replace the current mismatched-chain test with assertions that non-Safe batching stays disabled.
Finding: [NON-BLOCKING] Keep batching-incompatible Safes out of compatible analytics
- Location:
apps/cowswap-frontend/src/modules/twap/containers/TwapFormWidget/index.tsx:132 - The warning handles both unsupported states, but analytics now checks only
WALLET_NOT_SUPPORTED. - A Safe-WC connection with failed capability detection remains
TX_BUNDLING_NOT_SUPPORTED, then falls through tocompatibleorsafe-that-could-be-convertedeven though TWAP is disabled.
Suggested fix
- Handle both unsupported states before the fallback-handler branches, or add a distinct batching-unavailable event.
- Add effect-level coverage for a Safe with batching disabled.
Review scope and related context
- DeepSec PR mode ran against the exact PR file list: run
20260710151419-a335c60ae97dff7f, exit1, artifact/tmp/deepsec-comment-pr-7844-review-2055455.md. - DeepSec confirmed the wrong-chain fallback. Its other two findings were pre-existing behavior outside this PR's changes.
- The existing changes-requested review already covers Safe-WC, permit, gasless, ETH-flow, and unsupported-chain symptoms; those are not repeated here.
- Targeted verification passed: 29 wallet tests, 4 TWAP tests, and changed-file ESLint.
- The failing Cypress cases reproduce identically on the base branch and
develop, so they are not attributable to this PR.
🤖 Prompt for AI agents
Verify these findings against PR 7844's current head.
1. Require atomic execution when `ready` enables batching and test the final wallet_sendCalls payload.
2. Reject non-Safe capability responses that omit the active chain.
3. Keep TX_BUNDLING_NOT_SUPPORTED out of compatible TWAP analytics.
Keep changes focused and rerun the wallet capability and TWAP tests.
Generated using the pr-review skill from the CoW Protocol skills repo.
Generated using the security-review skill from the CoW Protocol skills repo.
75a8c27 to
8e1e43a
Compare
86d6ba4 to
5a1c3f9
Compare
87dd4b6 to
25f5b15
Compare
This one is supposed to be intermittent, that's why we have the "Use Safe web app" message.
I understand, but this is the flow for smart contracts, like Safe. I wonder, why this special behavior? Would it also make sense for Safes to support this behavior? Would like to clarify the use cases before implementing.
There was a bug, can you try again? @elena-zh |
|
@kernelwhisperer tests are red |
| * Safe WC returns EIP-5792 capabilities keyed by hex chain id (e.g. "0xaa36a7") | ||
| * while walletInfoAtom.chainId is numeric (e.g. 11155111). Numeric lookup alone misses them. | ||
| */ | ||
| export function resolveCapabilitiesForChain( |
There was a problem hiding this comment.
Note: This whole function is gone in #7847 (review) anyway.
| // or error. So, if we cannot check, then we must be false, | ||
| // not null (as some components/functions like `validateTradeForm` treat `null` as loading): | ||
| if (!walletCapabilities) return false | ||
| if (walletCapabilities === null) return false |
There was a problem hiding this comment.
Then maybe walletCapabilities could return false or a hardcoded { atomic: { status: false }} or similar? I think it would be better to be able to distinguish loading from not supported (either because it's actually not supported or because we got some kind of empty response / error). Otherwise, this will probably trigger those annoying UI flickers we have all over the app, as it will always briefly show the UI branches for unsupported batching, and then switch to the supported ones once/if wallet capabilities resolve positively.
There was a problem hiding this comment.
I also dislike that null is used for both error state and loading state. I will not address it in this PR though.
5a1c3f9 to
bd984ee
Compare
|
Hey @kernelwhisperer , thanks for the reply.
Not exactly: we show "Use Safe web app" message in case the app is connected to Safe+Rabby (it does not support bundling), and when connected to Safe's deprecated mobile app in iOS (also does not support bundling). In all the rest cases this message should not appear. I've retested the PR, I still see 'Unsupported wallet' when I'm connected to Safe app (web app)using WC and this is what I see on Develop for the same conditions
Not sure if I get you correctly here. But I will try to answer.
Permits stopped working at all when connected to MM now. :( Another issue that I've noticed: |
# Summary 1. Improve logging 2. Delete PromiseWithTimeout 3. Refactor withTimeout which is used by the explorer # To Test This PR doesn't really change any logic, just a refactor. 1. Connect with a safe wallet - [ ] Batching works in swap - [ ] TWAP support is detected # Background In the next PR (#7844) I will implement https://linear.app/cowswap/issue/FE-265/batch-support-for-eoas-and-eip7702-delegated-eoas <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Improved wallet capability detection with clearer timeout handling and legacy fallback. - **Bug Fixes** - Wallet capability fetch now returns `null` when providers fail or time out. - Improved timeout handling to avoid lingering timers. - **Refactor** - Standardized timeout handling to use a typed, options-based `withTimeout` API and `TimeoutError`. - Expanded environment-specific timeout messages for requests. - **Chores** - Removed the old `PromiseWithTimeout` utility export and updated related tests. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
4cba8dc to
0eb7628
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/modules/twap/pure/PrimaryActionButton/getTwapFormState.tsx (1)
51-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEnsure hierarchical resolution of loading states to avoid UI flicker.
Currently, if
isTxBundlingSupportedresolves tofalsewhileisWalletSupportedis still loading (null), the function will prematurely returnTX_BUNDLING_NOT_SUPPORTED. OnceisWalletSupportedresolves tofalse, the state will flip toWALLET_NOT_SUPPORTED, causing a flicker in the UI.To prevent this, handle the loading (
null) check for a higher-priority condition before evaluating the lower-priority failure state.♻️ Proposed refactor
if (isWalletSupported === false) return TwapFormState.WALLET_NOT_SUPPORTED + if (isWalletSupported === null) return TwapFormState.LOADING_SAFE_INFO + if (isTxBundlingSupported === false) return TwapFormState.TX_BUNDLING_NOT_SUPPORTED - if (verification === null || isTxBundlingSupported === null || isWalletSupported === null) { + if (verification === null || isTxBundlingSupported === null) { return TwapFormState.LOADING_SAFE_INFO }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cowswap-frontend/src/modules/twap/pure/PrimaryActionButton/getTwapFormState.tsx` around lines 51 - 56, Update the state-resolution ordering in getTwapFormState so any null loading value for the higher-priority wallet support check is handled before returning TX_BUNDLING_NOT_SUPPORTED. Preserve the final priority where WALLET_NOT_SUPPORTED takes precedence once isWalletSupported resolves false, while retaining LOADING_SAFE_INFO for unresolved verification or bundling support.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/wallet/src/api/state/walletCapabilitiesAtom.ts`:
- Around line 155-180: Update isAtomicBatchSupportedAsyncAtom so a null
walletCapabilities result returns null rather than false, preserving the
distinction between unresolved capabilities and definitively unsupported
wallets. Keep the existing capability-status checks unchanged for non-null
results.
---
Nitpick comments:
In
`@apps/cowswap-frontend/src/modules/twap/pure/PrimaryActionButton/getTwapFormState.tsx`:
- Around line 51-56: Update the state-resolution ordering in getTwapFormState so
any null loading value for the higher-priority wallet support check is handled
before returning TX_BUNDLING_NOT_SUPPORTED. Preserve the final priority where
WALLET_NOT_SUPPORTED takes precedence once isWalletSupported resolves false,
while retaining LOADING_SAFE_INFO for unresolved verification or bundling
support.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cec4035a-2799-4818-97df-d3df53d9a48c
📒 Files selected for processing (11)
apps/cowswap-frontend/src/locales/en-US.poapps/cowswap-frontend/src/modules/twap/containers/TwapFormWarnings/index.tsxapps/cowswap-frontend/src/modules/twap/containers/TwapFormWarnings/warnings/UnsupportedWalletWarning.tsxapps/cowswap-frontend/src/modules/twap/containers/TwapFormWidget/index.tsxapps/cowswap-frontend/src/modules/twap/hooks/useTwapFormState.tsapps/cowswap-frontend/src/modules/twap/index.tsapps/cowswap-frontend/src/modules/twap/pure/PrimaryActionButton/getTwapFormState.test.tsapps/cowswap-frontend/src/modules/twap/pure/PrimaryActionButton/getTwapFormState.tsxapps/cowswap-frontend/src/modules/twap/pure/PrimaryActionButton/index.tsxlibs/wallet/src/api/state/walletCapabilitiesAtom.test.tslibs/wallet/src/api/state/walletCapabilitiesAtom.ts
| export const isAtomicBatchSupportedAsyncAtom = atom(async (get): Promise<boolean | null> => { | ||
| const isSafeApp = get(isSafeAppAtom) | ||
|
|
||
| if (isSafeApp === null) return null | ||
|
|
||
| if (isSafeApp) return true | ||
|
|
||
| const accountType = get(accountTypeAtom) | ||
| const isSmartContractWallet = get(isSmartContractWalletAtom) | ||
| const isSafeWallet = get(isSafeWalletAtom) | ||
| const isSafeViaWc = get(isSafeViaWcAtom) | ||
|
|
||
| if (accountType === null || isSmartContractWallet === null || isSafeViaWc === null) return null | ||
|
|
||
| // Smart accounts (ERC-4337, Coinbase Smart Wallet, EIP-7702, etc.) that are not a Safe lack the | ||
| // fallback handler mechanism TWAP requires, so we treat them as unsupported. | ||
| // Note: useIsSmartContractWallet() only detects AccountType.SMART_CONTRACT, not EIP-7702 accounts | ||
| // (which keep the same EOA address but have delegation bytecode). We check both explicitly. | ||
| if ((isSmartContractWallet || accountType === AccountType.EIP7702EOA) && !isSafeWallet && !isSafeViaWc) return false | ||
| /** | ||
| * A SafeWallet connected through SafeApp is assumed to have support. | ||
| * A SafeWallet connected through WC or any other provider needs to pass the capabilities check. | ||
| */ | ||
| if (isSafeApp) return true | ||
| if (isSafeApp === null || isSafeViaWc === null) return null | ||
|
|
||
| const walletCapabilities = await get(walletCapabilitiesAtom) | ||
|
|
||
| // If `walletCapabilitiesAtom` returns `null` it's because `shouldSkipCapabilitiesCheck === true`, | ||
| // or because some kind of API empty response or error. So, if we cannot check, then we must be false, | ||
| // If `walletCapabilitiesAtom` returns `null` it's because some kind of API empty response | ||
| // or error. So, if we cannot check, then we must be false, | ||
| // not null (as some components/functions like `validateTradeForm` treat `null` as loading): | ||
| if (!walletCapabilities) return false | ||
| if (walletCapabilities === null) return false | ||
|
|
||
| const status = walletCapabilities.atomic?.status || '' | ||
| const supported = walletCapabilities?.atomicBatch?.supported | ||
| const status = walletCapabilities.atomic?.status | ||
| const isLegacyAtomicBatchSupported = Boolean(walletCapabilities?.atomicBatch?.supported) | ||
|
|
||
| // See https://www.eip5792.xyz/getting-started: | ||
| // - supported: The wallet will execute all calls atomically and contiguously | ||
| // - ready: The wallet is able to upgrade to supported pending user approval (e.g. via EIP-7702) | ||
| return status === 'supported' || !!supported | ||
| // return status === 'supported' || status === 'ready' | ||
| return status === 'supported' || status === 'ready' || isLegacyAtomicBatchSupported | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Collapsing "capabilities unresolved" into false likely explains the reported Safe-via-WC "Unsupported wallet" regression.
if (walletCapabilities === null) return false treats an unresolved/errored capabilities check identically to a definitively-unsupported wallet. This is the same design point Danziger raised previously: it prevents callers from distinguishing "loading/unknown" from "not supported," which both risks UI flicker and — more importantly per the PR conversation — matches the exact symptom Elena reported: Safe web app connections via WalletConnect still show "Unsupported wallet" (rather than resolving to true/null) when the capabilities call for that chain doesn't resolve cleanly for that connection type.
Given resolveCapabilitiesForChain was just changed to return null more often (no more fallback), this path is now reached more frequently, which could make the regression worse rather than better for wallets whose capability responses don't clearly key by chain.
Based on learnings from the past review comment on this exact code path: "Then maybe walletCapabilities could return false or a hardcoded { atomic: { status: false }}... I think it would be better to be able to distinguish loading from not supported... Otherwise, this will probably trigger those annoying UI flickers."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/wallet/src/api/state/walletCapabilitiesAtom.ts` around lines 155 - 180,
Update isAtomicBatchSupportedAsyncAtom so a null walletCapabilities result
returns null rather than false, preserving the distinction between unresolved
capabilities and definitively unsupported wallets. Keep the existing
capability-status checks unchanged for non-null results.
5 participants
Summary
Implements https://linear.app/cowswap/issue/FE-265/batch-support-for-eoas-and-eip7702-delegated-eoas
Fixes #7661
Adds EIP-7702 batching capability detection while keeping TWAP restricted to Safe wallets.
atomic.statusvaluessupportedandreadyatomicBatch.supporteddetectionTo Test
Screencast.From.2026-07-10.10-58-16.webm
Screencast.From.2026-07-10.10-59-24.webm
Background
Capability detection is independent from TWAP wallet eligibility: this change recognizes EIP-7702 batching, but TWAP remains restricted to Safe wallets.
Summary by CodeRabbit
Bug Fixes
Tests