Skip to content

refactor: remove wallet capabilities fallback#7847

Merged
kernelwhisperer merged 1 commit into
developfrom
refactor/wallet-capabilities-3
Jul 24, 2026
Merged

refactor: remove wallet capabilities fallback#7847
kernelwhisperer merged 1 commit into
developfrom
refactor/wallet-capabilities-3

Conversation

@kernelwhisperer

@kernelwhisperer kernelwhisperer commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix wallet capability checks by routing them through the connected wallet.
  • Remove the now-unnecessary direct RPC fallback.
  • Remove the unused provider and connection-restoring state from 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).

image

Summary by CodeRabbit

  • Refactor
    • Wallet capability detection now uses the connected wallet connector, with a 30-second timeout.
    • Removed legacy provider fallback handling for capability checks.
    • Simplified wallet connection state by removing provider and connection-restoring details.
    • Capability requests now safely return no capabilities when required wallet information is unavailable, rejected, or times out.

@kernelwhisperer
kernelwhisperer requested a review from Danziger July 13, 2026 09:39
@kernelwhisperer kernelwhisperer self-assigned this Jul 13, 2026
@vercel

vercel Bot commented Jul 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cowfi Ready Ready Preview Jul 24, 2026 9:39am
explorer-dev Ready Ready Preview Jul 24, 2026 9:39am
storybook Ready Ready Preview Jul 24, 2026 9:39am
swap-dev Ready Ready Preview Jul 24, 2026 9:39am
widget-configurator Ready Ready Preview Jul 24, 2026 9:39am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
cosmos Ignored Ignored Jul 24, 2026 9:39am
sdk-tools Ignored Ignored Preview Jul 24, 2026 9:39am

Request Review

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cde1933c-7128-4c48-85d8-decc31b342bd

📥 Commits

Reviewing files that changed from the base of the PR and between 39861d3 and b9e2038.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • libs/wallet/package.json
  • libs/wallet/src/api/state/walletCapabilitiesAtom.test.ts
  • libs/wallet/src/api/state/walletCapabilitiesAtom.ts
  • libs/wallet/src/api/types.ts
  • libs/wallet/src/wagmi/updater.ts
💤 Files with no reviewable changes (2)
  • libs/wallet/package.json
  • libs/wallet/src/api/types.ts

Walkthrough

Wallet capability detection now uses wagmi’s getCapabilities with connector state and a 30-second timeout. Legacy provider data, RPC fallback logic, and reconnect-restoring state were removed from wallet state and updater wiring, with tests updated accordingly.

Changes

Wallet capability refactor

Layer / File(s) Summary
Simplify wallet state contract and updater
libs/wallet/src/api/types.ts, libs/wallet/src/wagmi/updater.ts
WalletInfo and updater state no longer include provider or reconnect-restoring fields.
Replace legacy capability resolution
libs/wallet/src/api/state/walletCapabilitiesAtom.ts, libs/wallet/package.json
walletCapabilitiesAtom calls wagmi getCapabilities with account, chain, and connector state, applies a 30-second timeout, and removes legacy fallback handling.
Update capability and atomic-batch tests
libs/wallet/src/api/state/walletCapabilitiesAtom.test.ts
Tests cover wagmi capability calls, missing state, failures, timeouts, and atomic-batch behavior without legacy provider setup.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WalletUpdater
  participant walletInfoAtom
  participant walletCapabilitiesAtom
  participant WagmiGetCapabilities
  WalletUpdater->>walletInfoAtom: set account, chainId, connector, active
  walletCapabilitiesAtom->>walletInfoAtom: read wallet state
  walletCapabilitiesAtom->>WagmiGetCapabilities: getCapabilities with connector state
  WagmiGetCapabilities-->>walletCapabilitiesAtom: capabilities or error
Loading

Possibly related PRs

Suggested labels: Bug

Suggested reviewers: elena-zh, danziger, limitofzero, alfetopito

Poem

A rabbit hops where providers fade,
Wagmi fetches capabilities, neatly arrayed.
Thirty seconds tick, then errors rest,
Atomic checks still face the test.
“No fallback!” the bunny sings,
As simpler wallet state takes wing.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: removing the wallet capabilities fallback.
Description check ✅ Passed The description covers Summary and Background and matches the PR changes, but it omits the template's To Test section.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/wallet-capabilities-3

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 13, 2026

Copy link
Copy Markdown

Deploying explorer-dev with  Cloudflare Pages  Cloudflare Pages

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

View logs

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 13, 2026

Copy link
Copy Markdown

Deploying swap-dev with  Cloudflare Pages  Cloudflare Pages

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

View logs

@kernelwhisperer kernelwhisperer changed the title refactor: wallet capabilities refactor: wallet capabilities fallback Jul 13, 2026
@kernelwhisperer
kernelwhisperer force-pushed the feat/7702-batch-support branch from 87dd4b6 to 25f5b15 Compare July 13, 2026 09:41
@kernelwhisperer
kernelwhisperer force-pushed the refactor/wallet-capabilities-3 branch from 31b2c35 to 9a60a15 Compare July 13, 2026 09:42

@fairlighteth fairlighteth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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, while mobileInjectedProviderGuard.ts:10-12 deliberately converts a hanging MetaMask iOS capability request into {}.
  • With pinned viem 2.48.8, that raw {} becomes capabilities[chainId]undefined. The atom then violates its Promise<WalletCapabilities | null> contract, and line 75 dereferences walletCapabilities.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 returns null and batch support resolves false without 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-button failures 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.

@fairlighteth

fairlighteth commented Jul 13, 2026

Copy link
Copy Markdown
Contributor
🧪 Scoped browser QA passed: connected-wallet capability routing and mobile timeout fallback verified

Outcome

  • Supported capability routing: a fixture-backed connected provider returned atomic.status: supported. The provider received two wallet_getCapabilities calls, the app logged both successful fetches within 2–4 ms, no HTTP request carried that RPC method, and the Swap form settled without a loading or unsupported-wallet state.
  • Hanging mobile provider: two underlying capability calls remained pending; the guarded app calls settled after 1005 ms and 1003 ms. TWAP rendered Unsupported wallet, with no lingering loading state or page error.
  • Real-wallet smoke retained: MetaMask 13.35.1 previously reached visible wallet-connected-ui on this same head and remained connected after reload.
  • Non-blocking caveat: the hanging path still logs capabilities: undefined. Current UI consumers fail closed correctly, but this run does not invalidate the raw-atom contract review finding.
  • Not checked / follow-up: a real supported Safe via WalletConnect session, wallet_sendCalls, signing/submission, and a real MetaMask iOS reproduction.

Run details

  • Source: PR head 4ec1bfbe8535 via the public Vercel preview; browser console reported commit 4ec1bfb.
  • Environment: Linux; Chromium 140.0.7339.186; en-US.
  • Wallet: fixture-backed wallet-connected-ui for the capability matrix; real MetaMask wallet-connected-ui for the reload smoke; signing-capable behavior was not exercised.
  • AI assistance: Codex orchestrated Playwright execution, inspected the structured and visual artifacts, and drafted this summary.

Artifacts

Supported response routed through the connected provider; Swap settled normally:

Chromium fixture-backed connected provider with supported wallet capabilities and a settled Swap form

Hanging mobile capability request settled fail-closed:

Chromium mobile fixture showing the TWAP unsupported-wallet fallback after wallet_getCapabilities remains pending

How to retest

  • Connect an EIP-6963 provider through the injected-wallet UI on /#/1/swap and return { "0x1": { "atomic": { "status": "supported" } } } from wallet_getCapabilities. Expected: the connected provider receives the method, no public-RPC HTTP request carries it, and the form settles normally.
  • With a mobile user agent, leave the same provider method unresolved on /#/1/advanced/orders. Expected: handling settles after about 1 second, TWAP shows Unsupported wallet, and the UI does not remain loading.

Generated using the pr-qa skill from the CoW Protocol skills repo.

@Danziger Danziger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UACK, nice find! 👌

@kernelwhisperer
kernelwhisperer force-pushed the feat/7702-batch-support branch from 4cba8dc to 0eb7628 Compare July 14, 2026 11:08
@kernelwhisperer kernelwhisperer changed the title refactor: wallet capabilities fallback refactor: remove wallet capabilities fallback Jul 14, 2026
@kernelwhisperer
kernelwhisperer force-pushed the refactor/wallet-capabilities-3 branch from 4ec1bfb to 0b71643 Compare July 14, 2026 11:24
@kernelwhisperer
kernelwhisperer force-pushed the feat/7702-batch-support branch 3 times, most recently from d342b8d to 29f7a04 Compare July 23, 2026 14:02
Base automatically changed from feat/7702-batch-support to develop July 23, 2026 15:08
@kernelwhisperer
kernelwhisperer force-pushed the refactor/wallet-capabilities-3 branch from 0b71643 to b9e2038 Compare July 24, 2026 09:35
@kernelwhisperer
kernelwhisperer enabled auto-merge (squash) July 24, 2026 09:36
@kernelwhisperer
kernelwhisperer merged commit 2ce2a48 into develop Jul 24, 2026
19 checks passed
@kernelwhisperer
kernelwhisperer deleted the refactor/wallet-capabilities-3 branch July 24, 2026 09:42
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 24, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants