Skip to content

fix(bridge-core): signing out over WalletConnect no longer offers a dead session to resume - #102

Merged
Yoni-Starkware merged 2 commits into
mainfrom
fix/walletconnect-logout-resume
Oct 7, 2026
Merged

Yoni-Starkware merged 2 commits into
mainfrom
fix/walletconnect-logout-resume

Conversation

@Yoni-Starkware

@Yoni-Starkware Yoni-Starkware commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Bug (reported from offmarket on a phone): sign out over WalletConnect, then sign straight back in without a reload. The app offers "Resume session", the identity personal_sign goes to the deleted WC session, no wallet popup appears, and the UI is stuck until the 120s sign timeout. Desktop extensions are unaffected because their provider stays alive. Only a WC connection actually dies on sign-out.

Root cause

  1. disconnect() fired resetWalletConnectProvider() without awaiting it. The reset only dropped the WC registry entry after the session delete crossed the relay.
  2. In that window, clearing the selection re-ran the silent eth_accounts read, which reached the dying provider and got the signed-out address back, so canResume became true.
  3. WC emits no disconnect for a self-initiated delete, so onDisconnect never cleared it.
  4. Resume → personal_sign into the deleted session. A reload was the only way back, because the WC entry was only registered at mount.

Fix

  • resetWalletConnectProvider drops the singleton and the registry entry synchronously, before awaiting the delete.
  • A module-level teardown promise makes the next getWalletConnectProvider() wait for it, so a new provider can't rehydrate the session being deleted. A failed init only clears the singleton if it is still the current one.
  • disconnect() refreshes the picker as the reset starts. Once the teardown settles it re-registers a fresh WC entry, so WC sign-in works without a reload. The refresh logic is now one shared refreshProviders callback.
  • No new latch: same-address logout→login and A→B need no extra state, and both are tested.
  • Network switch is unaffected: apps/bridge calls setActiveNetwork before disconnect(), so the fresh provider is built from the new network's RPC map.

Tests

  • New WalletProvider.wc-logout-relogin.test.tsx (5 tests) runs the real WalletProvider, registry and WC singleton against the e2e WC provider with the relay delete held open. All 5 were red before the fix:
    • canResume during teardown;
    • a personal_sign sent to the dead session;
    • no WC entry after teardown;
    • same-address relogin;
    • A→B.
  • getWalletConnectProvider.test.ts pins the drop-before-await ordering (red before).
  • npx vitest run src/react/wallet: 10 files, 60 tests pass. Package tsc --noEmit and eslint on the wallet dir pass.

Not verified / residual risk

  • No live WC relay or phone wallet was used; everything ran on the e2e synthetic provider.
  • If the real session delete fails (the error is swallowed), the fresh provider could rehydrate that session from storage and canResume could return. That is narrower than today's bug; a disconnect-scoped latch could close it if wanted.
  • The branch is based on 0117a47 (4 unrelated fee-quote commits behind main); it merges cleanly.

Release: offmarket needs 0.1.24 (the usual chore(release): bump bridge-core PR) to pick this up.

🤖 Generated with Claude Code


This change is Reviewable

…ead session to resume

disconnect() fired resetWalletConnectProvider() without awaiting it, and the
reset only dropped the WC registry entry after the session delete crossed the
relay. In that window the silent eth_accounts read reached the dying provider,
got the signed-out address back and set canResume; WC emits no disconnect for a
self-initiated delete, so it never cleared. Resume then sent personal_sign to the
deleted session: no wallet popup, stuck until the 120s sign timeout. A reload was
the only way back, because the WC entry was only registered at mount.

- resetWalletConnectProvider drops the singleton and the registry entry
  synchronously, before awaiting the delete; a module-level teardown promise
  makes the next getWalletConnectProvider() wait for it, so a new provider can't
  rehydrate the session being deleted. A failed init only clears the singleton
  if it is still the current one.
- disconnect() refreshes the picker as the reset starts, then re-registers a
  fresh WC entry once the teardown settles, so WC sign-in works without a reload.
- No new latch: same-address logout->login and A->B need no extra state (tested).

Tests: WalletProvider.wc-logout-relogin.test.tsx (5, red before the fix) runs the
real WalletProvider against the e2e WC provider with the relay delete held open;
getWalletConnectProvider.test.ts pins the drop-before-await ordering.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cursor

cursor Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes wallet session teardown and provider singleton timing on sign-out; well-covered by tests but affects identity signing over WalletConnect.

Overview
Fixes WalletConnect sign-out where a slow relay delete left Resume session enabled and personal_sign could target a dying session until reload.

resetWalletConnectProvider now clears the singleton and unregisters WalletConnect from EIP-6963 before awaiting session delete. A module-level teardown promise makes the next getWalletConnectProvider() wait so a new instance cannot rehydrate the session being removed.

disconnect() in WalletProvider immediately refreshProviders() after starting reset (shared refreshProviders callback), then registerWalletConnect + another refresh when teardown finishes so WalletConnect reappears in the picker without a page reload.

New WalletProvider.wc-logout-relogin.test.tsx and an extended getWalletConnectProvider.test.ts case cover canResume during teardown, no signs to dead sessions, relogin cycles, and account switches.

Reviewed by Cursor Bugbot for commit a663adc. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Yoni-Starkware
Yoni-Starkware merged commit eb3d863 into main Oct 7, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant