Skip to content

fix: prevent WalletConnect session proposal race conditions (WAPI-1070)#26121

Merged
chakra-guy merged 1 commit into
mainfrom
fix/wapi-1070-on-qr-fix
Feb 20, 2026
Merged

fix: prevent WalletConnect session proposal race conditions (WAPI-1070)#26121
chakra-guy merged 1 commit into
mainfrom
fix/wapi-1070-on-qr-fix

Conversation

@chakra-guy

@chakra-guy chakra-guy commented Feb 16, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes a set of race conditions in the WalletConnect v2 connection flow that caused a "stuck" or looping connection tray when scanning QR codes. The root cause was multiple layers of concurrent/duplicate processing:

  1. Duplicate OS deeplink delivery:
    The OS can deliver the same deeplink 3-5 times via Linking + Branch. Each delivery called pair() and pushed a loading modal, stacking duplicate modals on screen.
  2. Duplicate relay events:
    The WalletConnect relay can fire the same session_proposal event multiple times when duplicate pair() calls were made, causing duplicate AccountConnect screens.
  3. Concurrent proposal handling:
    Rapid QR scans caused overlapping proposals to fight over shared state (wc2Metadata, navigation, approval queue).
  4. Navigation re-trigger loop:
    Changes to the pendingApprovals Redux state re-triggered PermissionApproval's useEffect, causing repeated navigation to AccountConnect for the same approval.

Each issue is fixed at the layer where it occurs:

Layer Guard Purpose
connect() seenTopics + 5 s TTL Blocks duplicate OS deeplink deliveries; TTL allows manual retries
onSessionProposal() proposalLock Serializes concurrent proposals
_handleSessionProposal() handledProposalIds Blocks duplicate relay events
PermissionApproval lastNavigatedApprovalIdRef Prevents re-navigation for the same approval ID

Additionally fixes the SDKLoading Lottie animation not being visible.

Compatibility with #24040: The PermissionApproval change keeps pendingApprovals in the useEffect deps (so the effect still re-runs when the queue changes, preventing stuck approvals). The added ref guard only skips navigation for the same approval ID; new approvals with a different ID navigate normally.

Changelog

CHANGELOG entry: Fixed WalletConnect connection tray getting stuck or looping when scanning QR codes, and fixed the loading animation not displaying during connection.

Related issues

Fixes: WAPI-1070

Manual testing steps

Feature: WalletConnect QR code connection

  Scenario: In-app QR scanner connection
    Given the user opens a dApp that supports WalletConnect (e.g. Coinbase Commerce)
    When the user scans the WC QR code using the in-app scanner
    Then a loading animation should briefly appear
    And the AccountConnect approval screen should appear once
    And tapping Connect should complete the connection

  Scenario: Camera app deeplink connection
    Given the user opens a dApp that supports WalletConnect
    When the user scans the WC QR code using the device camera app
    Then the loading modal should appear only once (not duplicated)
    And the AccountConnect screen should appear once
    And tapping Connect should complete the connection

  Scenario: Rapid repeated scans
    Given the user has completed one connection via QR scan
    When the user immediately scans another WC QR code
    Then each scan should produce exactly one connection flow
    And no flows should get stuck or loop

  Scenario: Retry after failed relay delivery
    Given the user scans a QR code but the WC relay fails to send a session_proposal
    When the user waits ~5 seconds and scans the same QR code again
    Then a new connection attempt should proceed normally

Screenshots/Recordings

Screen.Recording.2026-02-16.at.14.16.502.mov

Pre-merge author checklist

  • I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
  • I've completed the PR template to the best of my ability
  • I've included tests if applicable
  • I've documented my code using JSDoc format if applicable
  • I've applied the right labels on the PR

Note

Medium Risk
Touches WalletConnect v2 connection/proposal control flow and approval navigation, so regressions could block or duplicate connection attempts. Changes are localized and include added guards and updated tests, reducing but not eliminating behavioral risk.

Overview
Prevents WalletConnect v2 connection flows from looping/stacking by deduplicating connect() calls per pairing topic (5s TTL) and serializing session_proposal handling with a lock plus proposal-id dedupe.

Fixes repeated AccountConnect navigation by guarding PermissionApproval against re-navigating for the same approval metadata.id even when pendingApprovals changes, and updates tests accordingly. Also fixes the SDK loading tray animation sizing by adding an explicit Lottie aspectRatio, and tightens logging/metadata cleanup on proposal rejection to avoid stale UI state.

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

@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added team-wallet-integrations Wallet Integrations team INVALID-PR-TEMPLATE PR's body doesn't match template labels Feb 16, 2026
@chakra-guy chakra-guy marked this pull request as ready for review February 16, 2026 13:21
@chakra-guy chakra-guy requested review from a team as code owners February 16, 2026 13:21
Comment thread app/core/WalletConnect/WalletConnectV2.ts
Base automatically changed from ad/in-app-qr-code-universal-link-ios-bug to main February 18, 2026 21:02
@chakra-guy chakra-guy requested a review from a team as a code owner February 18, 2026 21:02
@chakra-guy chakra-guy force-pushed the fix/wapi-1070-on-qr-fix branch from e71ce4b to 183bbe0 Compare February 19, 2026 09:37
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

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

E2E Test Selection:
The changes focus on three main areas:

  1. WalletConnectV2.ts - Critical changes adding deduplication mechanisms (seenTopics, handledProposalIds) and a proposal lock to serialize session proposal handling. This fixes race conditions where concurrent proposals could fight over shared state. Also clears stale metadata on user rejection. These changes directly affect WalletConnect dApp connection flows.

  2. PermissionApproval.tsx - Added a ref (lastNavigatedApprovalIdRef) to prevent re-navigation for the same approval when pendingApprovals changes. This is a fix for the permission approval flow used during dApp connections.

  3. SDKLoading/index.tsx - Minor UI fix adding aspectRatio to the Lottie animation style.

  4. connectWithWC.ts and handleMetaMaskDeeplink.ts - Minor log message changes with no functional impact.

Selected tags rationale:

  • SmokeNetworkExpansion: Tests multi-chain provider architecture, dApp connect/disconnect flows, and Solana wallet standard compliance. The WalletConnect changes directly affect how dApp connections are handled.
  • SmokeMultiChainAPI: Tests CAIP-25 multi-chain session API (wallet_createSession, wallet_getSession, wallet_revokeSession, wallet_sessionChanged). The permission approval changes affect how these sessions are managed.
  • SmokeNetworkAbstractions: Tests chain permission system for dApps, which is related to the permission approval flow changes.

Not selected:

  • SmokeConfirmations: Changes don't affect transaction/signature confirmation UI
  • SmokeAccounts: No account management changes
  • SmokeTrade/SmokeRamps: No trading or ramp functionality affected
  • FlaskBuildTests: No Snaps-related changes

Performance Test Selection:
The changes are focused on race condition fixes and deduplication logic in WalletConnect session handling and permission approval navigation. These are correctness fixes rather than performance-impacting changes. The SDKLoading aspectRatio change is a minor styling fix. None of these changes affect rendering performance, data loading, account/network list components, or critical user flow performance metrics.

View GitHub Actions results

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Comment thread app/core/WalletConnect/WalletConnectV2.ts
@sonarqubecloud

Copy link
Copy Markdown

@chakra-guy chakra-guy enabled auto-merge February 20, 2026 13:51
@chakra-guy chakra-guy added this pull request to the merge queue Feb 20, 2026
Merged via the queue into main with commit 1d037c1 Feb 20, 2026
94 checks passed
@chakra-guy chakra-guy deleted the fix/wapi-1070-on-qr-fix branch February 20, 2026 15:42
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 20, 2026
@metamaskbot metamaskbot added the release-7.68.0 Issue or pull request that will be included in release 7.68.0 label Feb 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

INVALID-PR-TEMPLATE PR's body doesn't match template release-7.68.0 Issue or pull request that will be included in release 7.68.0 size-S team-wallet-integrations Wallet Integrations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants