refactor: remove wallet capabilities fallback#7847
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Deploying explorer-dev with
|
| Latest commit: |
0b71643
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c74df0e4.explorer-dev-dxz.pages.dev |
| Branch Preview URL: | https://refactor-wallet-capabilities-13y7.explorer-dev-dxz.pages.dev |
Deploying swap-dev with
|
| Latest commit: |
0b71643
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8ec708ad.swap-dev-5u6.pages.dev |
| Branch Preview URL: | https://refactor-wallet-capabilities-uosj.swap-dev-5u6.pages.dev |
87dd4b6 to
25f5b15
Compare
31b2c35 to
9a60a15
Compare
fairlighteth
left a comment
There was a problem hiding this comment.
⚠️ AI Review (Codex GPT-5, worked 25m): normalize the mobile empty-capabilities result
Finding: [NON-BLOCKING] Keep walletCapabilitiesAtom on its declared object-or-null contract
- Location:
libs/wallet/src/api/state/walletCapabilitiesAtom.ts:30-43 - The connector routing is correct, but the chain-specific viem call can resolve
undefined. EIP-5792 permits an unsupported requested chain to be omitted, whilemobileInjectedProviderGuard.ts:10-12deliberately converts a hanging MetaMask iOS capability request into{}. - With pinned viem 2.48.8, that raw
{}becomescapabilities[chainId]—undefined. The atom then violates itsPromise<WalletCapabilities | null>contract, and line 75 dereferenceswalletCapabilities.atomic. - The loadable wrapper fails closed to
false, so current UI consumers recover, but the exported raw atom can return an undocumented value. The{}unit mock currently bypasses the wagmi/viem normalization that creates it.
Suggested fix
- Return
capabilities ?? null. - Add a regression test where the action resolves
undefined—or a fake provider returns raw{}—and assert that the raw atom returnsnulland batch support resolvesfalsewithout rejecting.
Review scope and related context
- No existing review thread covers this; CodeRabbit skipped the non-default base branch.
- The PR screenshot confirms Safe via WalletConnect on Avalanche receives
atomic.status: supported. - Base PR #7844 already contains the broader wallet QA matrix. Optionally note which EIP-7702 and unsupported-wallet cases were rerun after this routing change.
- The four Cypress
#do-trade-buttonfailures are identical on the base PR, so they are not attributed to this change.
🤖 Prompt for AI agents
Verify this finding against PR head 4ec1bfbe.
In walletCapabilitiesAtom:
- Preserve connector-based wagmi routing and the existing timeout.
- Normalize an undefined getCapabilities result to null.
- Add coverage for the valid empty/missing-chain response path.
- Confirm walletCapabilitiesAtom never exposes undefined and isAtomicBatchSupportedAsyncAtom resolves false rather than rejecting.
- Do not reintroduce the public-RPC or legacy provider fallback.
Generated using the pr-review skill from the CoW Protocol skills repo.
🧪 Scoped browser QA passed: connected-wallet capability routing and mobile timeout fallback verifiedOutcome
Run details
Artifacts Supported response routed through the connected provider; Swap settled normally:
Hanging mobile capability request settled fail-closed:
How to retest
Generated using the |
4cba8dc to
0eb7628
Compare
4ec1bfb to
0b71643
Compare


Summary
WalletInfo.Background
Capability requests were incorrectly sent to the chain’s public RPC, which does not expose wallet capabilities. Using the connected wallet fixes detection and makes the fallback/provider plumbing unnecessary (seems to work in Safe via WC too).