Skip to content

fix: Fix redux store event handler leak in WCv2#27932

Merged
jiexi merged 2 commits into
mainfrom
jl/WAPI-1355/fix-redux-store-leak-wc
Mar 25, 2026
Merged

fix: Fix redux store event handler leak in WCv2#27932
jiexi merged 2 commits into
mainfrom
jl/WAPI-1355/fix-redux-store-leak-wc

Conversation

@jiexi

@jiexi jiexi commented Mar 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Description

Fixes an issue where after a WCv2 connection could still send chainChanged events to the WC relay when the connection was removed. This was happening regardless of if the connection was removed on the wallet side or signaled removed from the WC side.

This PR fixes that by properly unsubscribing from the store.

Changelog

CHANGELOG entry: null

Not user facing.

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-1355

Manual testing steps

  1. Open app in iOS expo build
  2. Open js debugger
  3. add breakpoint to onStoreChange in WalletConnect2Session.ts
  4. Connect a new WC session via demo app
  5. You should see onStoreChange fire
  6. Visit settings, experimental, and disconnect this session (long tap)
  7. You should not see onStoreChange fire anymore
  8. reconnect to demo dapp
  9. You should see onStoreChange fire
  10. disconnect from the demo dapp
  11. you should not see onStoreChange fire anymore

Visit https://react-app.walletconnect.com/ in the native browser and smoke test

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Touches WalletConnect v2 session lifecycle/cleanup, so mistakes could leave sessions partially disconnected or change disconnect timing. Logic is straightforward and covered by updated unit tests, limiting risk.

Overview
Fixes a WalletConnect v2 listener leak by storing the Redux store.subscribe unsubscribe in WalletConnect2Session and invoking it during removeListeners().

Updates WC2Manager.removeAll() to tear down every WalletConnect2Session via removeListeners() before clearing the sessions map, preventing orphaned subscriptions/bridges when session_delete fires after local state is cleared. Tests were extended to assert the unsubscribe is called and removeAll() invokes removeListeners() for each session.

Written by Cursor Bugbot for commit 7633be5. This will update automatically on new commits. Configure here.

@jiexi jiexi requested a review from a team as a code owner March 25, 2026 18:06
@jiexi jiexi added the no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed label Mar 25, 2026
@metamaskbot metamaskbot added the team-wallet-integrations Wallet Integrations team label Mar 25, 2026
@github-actions github-actions Bot added size-S risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 25, 2026
@github-actions

github-actions Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeConfirmations, SmokeMultiChainAPI, SmokeNetworkExpansion, SmokeNetworkAbstractions
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 88%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR fixes two WalletConnect V2 lifecycle bugs:

  1. Listener leak fix (WalletConnect2Session.ts): Stores the Redux store.subscribe() unsubscribe function and calls it in removeListeners(). Previously, every WalletConnect session that was removed would leave a dangling Redux store subscription, causing memory leaks and potential stale chain-change handlers.

  2. Race condition fix (WalletConnectV2.ts): In removeAll(), removeListeners() is now called on all sessions before clearing this.sessions. Previously, session_delete events could fire after the map was cleared, causing removeListeners() to be skipped entirely, leaking both Redux subscriptions and BackgroundBridge connections.

These are targeted bug fixes in the WalletConnect V2 session management layer. The changes affect:

  • dApp connection/disconnection flows via WalletConnect
  • Session cleanup when disconnecting all WalletConnect sessions
  • Chain change detection reliability during WalletConnect sessions

Selected tags:

  • SmokeConfirmations: WalletConnect is the primary transport for dApp-initiated signature and transaction requests. The session cleanup fixes could affect confirmation flows.
  • SmokeMultiChainAPI: WalletConnect V2 implements CAIP-25 multi-chain sessions; the removeAll() fix directly affects session teardown for multi-chain API tests.
  • SmokeNetworkExpansion: Multi-chain provider connections (EVM + Solana) use WalletConnect; session lifecycle changes affect these flows.
  • SmokeNetworkAbstractions: Chain permissions for dApps connected via WalletConnect are affected by the chain-change subscription fix.

Tag dependencies satisfied: SmokeMultiChainAPI → SmokeNetworkAbstractions + SmokeNetworkExpansion; SmokeNetworkExpansion → SmokeConfirmations.

Performance Test Selection:
The changes are bug fixes for WalletConnect session cleanup (listener unsubscription and ordering of teardown). These do not affect UI rendering, data loading, account/network list components, or app startup performance. No performance tests are warranted.

View GitHub Actions results

@github-actions

github-actions Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

✅ E2E Fixture Validation — Schema is up to date
18 value mismatches detected (expected — fixture represents an existing user).
View details

@sonarqubecloud

sonarqubecloud Bot commented Mar 25, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

adonesky1
adonesky1 approved these changes Mar 25, 2026
View reviewed changes

@adonesky1 adonesky1 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.

LGTM!

@adonesky1 adonesky1 mentioned this pull request Mar 25, 2026
Merged
5 tasks
@jiexi jiexi added this pull request to the merge queue Mar 25, 2026
Merged via the queue into main with commit 8077384 Mar 25, 2026
140 checks passed
@jiexi jiexi deleted the jl/WAPI-1355/fix-redux-store-leak-wc branch March 25, 2026 21:39
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 25, 2026
@metamaskbot metamaskbot added the release-7.72.0 Issue or pull request that will be included in release 7.72.0 label Mar 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@adonesky1 adonesky1 adonesky1 approved these changes

Assignees

No one assigned

Labels

no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed release-7.72.0 Issue or pull request that will be included in release 7.72.0 risk-medium Moderate testing recommended · Possible bug introduction risk size-S team-wallet-integrations Wallet Integrations team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jiexi @adonesky1 @metamaskbot