feat(frontend): allow multiple simultaneous WalletConnect connections#13152
feat(frontend): allow multiple simultaneous WalletConnect connections#13152sbpublic wants to merge 10 commits into
Conversation
Add disconnectSession(topic) to the WalletConnectListener interface and WalletConnectClient, scoped to a single topic, alongside the existing all-sessions disconnect(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The persistent listener wraps a singleton WalletKit, so its reference no longer changes when a session is added or removed. Add walletConnectSessionsStore as the reactive source of truth for the connected-apps UI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…WalletConnect session Reuse the existing listener when adding a connection instead of tearing it down (cold-start init now uses cleanSlate: false). Add syncSessions(), a per-session disconnectSession(topic) service that falls through to a full teardown when the last app is removed, and resetListenerIfNoSessions() which detaches handlers and resets only when no sessions remain. Apply the guard to session_delete and to the proposal reject/cancel and pair-failure paths so they no longer drop sibling connections. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sessions modal The connected-apps list now reads the reactive sessions store. Each row's close button disconnects only that session by topic; a new "Disconnect all" toolbar button (i18n key wallet_connect.text.disconnect_all) tears every connection down at once. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Update PRODUCT.md to describe multiple independently-managed dApp connections and add the spec to the spec-driven-development specs folder. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…orted languages Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the frontend WalletConnect integration to support multiple simultaneous dApp sessions, aligning the app-layer behavior with Reown WalletKit’s native multi-session capability. It introduces per-topic session teardown, makes the “Connected Apps” UI reactive via a dedicated sessions store, and adjusts lifecycle handling so that adding/removing one session doesn’t drop siblings.
Changes:
- Add
disconnectSession(topic)to the listener/provider and introduce service helpers (syncSessions,resetListenerIfNoSessions) to keep a stable listener while sessions come and go. - Add
walletConnectSessionsStoreas the reactive source of truth for the connected-apps UI; update modal/session/review flows to keep it in sync. - Add “Disconnect all” UI + i18n key (
wallet_connect.text.disconnect_all) and extend unit tests for the new multi-session behavior.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/frontend/src/lib/services/wallet-connect.services.ts | Reuse existing listener on add; add session sync/reset helpers and per-topic disconnect service. |
| src/frontend/src/lib/providers/wallet-connect.providers.ts | Implement per-topic disconnect via WalletKit disconnectSession. |
| src/frontend/src/lib/stores/wallet-connect.store.ts | Add walletConnectSessionsStore to drive reactive connected-apps UI. |
| src/frontend/src/lib/types/wallet-connect.ts | Extend listener contract with disconnectSession(topic). |
| src/frontend/src/lib/components/wallet-connect/WalletConnectSessionsModal.svelte | Render sessions from sessions store; add per-row disconnect + “Disconnect all” button. |
| src/frontend/src/lib/components/wallet-connect/WalletConnectSession.svelte | Seed sessions store after reconnect; keep listener alive unless last session ends. |
| src/frontend/src/lib/components/wallet-connect/WalletConnectReview.svelte | Guard reset paths to avoid dropping sibling sessions; sync sessions after approve/remove. |
| src/frontend/src/lib/types/i18n.d.ts | Add wallet_connect.text.disconnect_all typing. |
| src/frontend/src/lib/i18n/en.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/ar.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/cs.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/de.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/es.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/fr.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/hi.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/it.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/ja.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/ko-KR.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/pl.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/pt.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/ru.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/vi.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/lib/i18n/zh-CN.json | Add wallet_connect.text.disconnect_all string. |
| src/frontend/src/tests/lib/services/wallet-connect.services.spec.ts | Add coverage for syncSessions, guarded reset, and per-topic disconnect teardown behavior. |
| src/frontend/src/tests/lib/stores/wallet-connect.store.spec.ts | Add coverage for sessions store set/reset behavior. |
| src/frontend/src/tests/lib/providers/wallet-connect.providers.spec.ts | Add coverage for provider per-topic disconnect. |
| src/frontend/src/tests/lib/components/wallet-connect/WalletConnectSessionsModal.spec.ts | Add coverage for per-row disconnect by topic, “Disconnect all”, and reactive list updates. |
| src/frontend/src/tests/btc/services/wallet-connect.services.spec.ts | Update listener mock to include disconnectSession. |
| src/frontend/src/tests/sol/services/wallet-connect.services.spec.ts | Update listener mock to include disconnectSession. |
| docs/ai/spec-driven-development/specs/2026-06-20-impr-multiple-walletconnect-connections.md | Add spec documenting the multi-session design + acceptance criteria. |
| docs/ai/PRODUCT.md | Document multi-connection WalletConnect behavior in product notes. |
disconnectSession swallows its own errors, so disconnectOne showed the "disconnected" success toast even when the disconnect failed. Return ResultSuccess from the service and toast only when success is true. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…cessible name
The icon-only per-row disconnect button had no accessible name. Add a
localized ariaLabel via a new i18n key wallet_connect.text.disconnect_app
("Disconnect $name"), resolved per row with the dApp name, translated
across supported languages.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Independent spec review (Step 6 — Cowork) I authored the spec but not the code, so this is the independent review pass. I read the full diff at commit 7fa3b05 (I did not run the app or CI — the format/lint/check/test gates remain CI's backstop). Overall this looks ready to merge: it faithfully implements all six in-scope acceptance criteria and the three resolved decisions, and gets the subtle correctness points right. Verified against the spec. The three choke points are fixed as designed — The two spec open questions resolved cleanly: the add path never re-runs Finding — test gap (main actionable item). Acceptance criterion 1, the headline behavior (connecting a 2nd dApp must not drop the 1st), has no direct automated test. Minor / low (pre-existing). Confirm in UI QA. In a running build (and in dark theme): closing one row updates the list in place without reopening the modal, and a dApp-side disconnect updates the open list. The store wiring looks correct; this is the "correct in the diff still needs a human in the running app" check. |
|
Good catch — adding direct Test gap: agreed — the headline "adding a 2nd dApp keeps the 1st connected" path had no direct test, only the helpers it composes. Adding a
UI QA: over to you for the running-build / dark-theme check (in-place row removal on per-row close, and a dApp-side disconnect updating the open list). 🤖 Claude Opus 4.8 |
… listener Add a connectListener test for acceptance criterion 1 (connecting a new dApp must not drop already-connected dApps): pre-seed the listener store and assert the existing listener's disconnect is not called, WalletConnectClient.init is not re-invoked, and pair(uri) + attachHandlers run on the existing listener. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
✅ No security or compliance issues detected. Reviewed everything up to 31a1518. Security Overview
Detected Code Changes
|
Motivation
Today a user can only have one WalletConnect connection open at a time — connecting a second dApp silently drops the first, and the "Connected Apps" list can only ever show one entry. This is an app-layer restriction; Reown WalletKit natively supports many concurrent sessions. This change lets users connect several dApps (e.g. Uniswap and Liquidium) at once and manage each independently.
Implements
docs/ai/spec-driven-development/specs/2026-06-20-impr-multiple-walletconnect-connections.md.Changes
disconnectSession(topic)to the WalletConnect listener (single-session teardown) alongside the existing all-sessionsdisconnect().walletConnectSessionsStoreas the source of truth for the connected-apps UI (the persistent listener reference no longer changes on add/remove).cleanSlate: false); addsyncSessions(), a per-session disconnect service that falls through to full teardown when the last app is removed, and a guardedresetListenerIfNoSessions().session_deleteand on proposal reject/cancel/pair-failure.wallet_connect.text.disconnect_all).disconnect_alllabel and translate it for all 14 supported non-English languages.PRODUCT.md; add the spec.Divergence from the spec
session_deleteguard lives in the call-site callbacks via a sharedresetListenerIfNoSessions()rather than insideonSessionDelete(same effect, less duplication).resetListener()unconditionally, which would drop a live first connection when a second proposal is rejected or fails to pair.resetListenerIfNoSessions()also detaches handlers before reset to avoid leaking duplicate handlers on the singleton WalletKit emitter.testIdinstead of a new aria-label string, keeping to the single new i18n key the spec specified.Tests
disconnectSession, sessions store, services (syncSessions, per-session disconnect, guarded reset, last-app teardown), and modal (per-row disconnect by topic, "Disconnect all", reactive updates).npm run format,npm run lint --max-warnings 0,npm run checkall clean; the 108 wallet-connect/scanner tests pass. (Full localnpm run testhas unrelated failures from a local Node/jsdomlocalStorageclash in untouched files — not reproducible in CI.)Review feedback
disconnectSessionswallows its own errors, so the per-row "disconnected" toast fired even on a failed disconnect. The service now returnsResultSuccessand the modal toasts only on success.ariaLabelvia a new i18n keywallet_connect.text.disconnect_app("Disconnect $name"), translated across the supported languages.connectListenertest for acceptance criterion 1 (connecting a 2nd dApp must not drop the 1st) — asserts the existing listener is reused (disconnectnot called,WalletConnectClient.initnot re-invoked,pair+attachHandlersrun on it).🤖 Generated with Claude Code — Opus 4.8 (claude-opus-4-8)