Skip to content

fix: detect SDKConnectV2 connections in useOriginSource via v2Connections store#28290

Merged
adonesky1 merged 3 commits intomainfrom
fix/WAPI-1380-useOriginSource-v2-detection
Apr 2, 2026
Merged

fix: detect SDKConnectV2 connections in useOriginSource via v2Connections store#28290
adonesky1 merged 3 commits intomainfrom
fix/WAPI-1380-useOriginSource-v2-detection

Conversation

@adonesky1
Copy link
Copy Markdown
Contributor

@adonesky1 adonesky1 commented Apr 1, 2026

Summary

  • Fixes a bug where useOriginSource misclassified all SDKConnectV2 (MWP) connections as in-app browser, causing the source property on CONNECT_REQUEST_STARTED, CONNECT_REQUEST_COMPLETED, and CONNECT_REQUEST_CANCELLED analytics events to be incorrect for V2 connections.
  • The hook's existing check for the MMSDKCONNECTV2:: prefix was dead code — the prefix is stripped by RPCMethodMiddleware before it reaches the permission system. V2 connections arrive with a bare UUID as their origin.
  • Adds a lookup against the v2Connections Redux store (already populated by HostApplicationAdapter.syncConnectionList) to correctly identify V2/MWP connections and return SourceType.SDK_CONNECT_V2.

Root cause

The RPCBridgeAdapter constructs middlewareHostname = MMSDKCONNECTV2::<connId>, but:

  1. RPCMethodMiddleware.ts immediately strips the prefix (hostname.replace(SDK_CONNECT_V2_ORIGIN, ''))
  2. For the CAIP path (wallet_createSession), BackgroundBridge.channelIdOrOrigin returns the bare channelId (UUID) when isMMSDK=true
  3. The PermissionController approval request therefore carries a bare UUID as metadata.origin
  4. useOriginSource couldn't match this UUID to V2 — it only checked the V1 SDKConnect store

CHANGELOG entry: null

Test plan

  • Unit tests: 7 passing (2 new V2-specific tests added)
  • Manual: Open an MWP deeplink, verify CONNECT_REQUEST_STARTED event has source: 'sdk_connect_v2' instead of source: 'in-app browser'
  • Verify no regression for V1 SDK connections (source: 'sdk')
  • Verify no regression for WalletConnect connections (source: 'walletconnect')
  • Verify no regression for in-app browser connections (source: 'in-app browser')

Related


Note

Medium Risk
Changes the source-classification logic used for connection-request analytics by introducing a new SDK v2 detection path and reordering precedence, which could shift event attribution if the v2Connections store is incomplete or stale.

Overview
Fixes useOriginSource to correctly detect SDK Connect v2 (MWP) permission origins by treating a bare UUID as v2 when it exists in the Redux sdk.v2Connections map (instead of relying on a removed/stripped v2 prefix).

Refactors the hook to use an explicit priority order (v2 → v1 → WalletConnect → in-app browser) and adds unit coverage for v2 detection (including the case where the same UUID exists in both v1 and v2, with v2 winning). Tests that build mock Redux sdk state were updated to include an empty v2Connections field.

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2026

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 the team-wallet-integrations Wallet Integrations team label Apr 1, 2026
@github-actions github-actions bot added size-S risk-medium Moderate testing recommended · Possible bug introduction risk labels Apr 1, 2026
Copy link
Copy Markdown

@cursor cursor bot left a comment

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.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

@adonesky1 adonesky1 force-pushed the fix/WAPI-1380-useOriginSource-v2-detection branch from db1a3c9 to 0bf5c12 Compare April 1, 2026 20:18
@github-actions github-actions bot added size-M risk-medium Moderate testing recommended · Possible bug introduction risk and removed size-S risk-medium Moderate testing recommended · Possible bug introduction risk labels Apr 1, 2026
…ions store

The useOriginSource hook had a dead-code check for the MMSDKCONNECTV2::
prefix, which is stripped by RPCMethodMiddleware before it ever reaches
the permission system. V2 connections use a bare UUID as their origin,
so the hook was falling through to return 'in-app browser' instead of
'sdk_connect_v2'.

Look up the origin UUID in the v2Connections Redux store (already
populated by HostApplicationAdapter.syncConnectionList) to correctly
identify V2/MWP connections. This fixes the source property on all
CONNECT_REQUEST_* analytics events for V2 connections.

Also refactors the hook for clarity: removes the dead MMSDKCONNECTV2::
prefix check, adds JSDoc explaining the origin format per connection
type and the priority order, and annotates non-obvious checks (e.g.
WalletConnect's global-state heuristic).

Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-1380
@adonesky1 adonesky1 force-pushed the fix/WAPI-1380-useOriginSource-v2-detection branch from 0bf5c12 to 97d225f Compare April 1, 2026 20:22
@github-actions github-actions bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Apr 1, 2026
…Connect tests

v2Connections may be undefined on app upgrade (persisted sdk state
predates the field, no migration). Use optional chaining in the
lookup. Also add v2Connections: {} to all 7 partial sdk mock states
in AccountConnect.test.tsx so the test store matches the real
reducer shape.
@github-actions github-actions bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Apr 1, 2026
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Apr 1, 2026
Comment on lines +61 to +65
// wc2Metadata is a single Redux slot holding the *most recent* WC proposal
// metadata (set on session_proposal, cleared after approval/rejection).
// It is not keyed by origin — we rely on the WC proposal flow being
// serialized (via proposalLock in WalletConnectV2) so that during the
// approval window, a non-empty id implies *this* origin is from WC.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

😭 😭 😭

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Created a ticket to improve this here: https://consensyssoftware.atlassian.net/browse/WAPI-1383

@github-actions github-actions bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Apr 2, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

🔍 Smart E2E Test Selection

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

E2E Test Selection:
The changes modify useOriginSource.ts, a hook that determines the connection source type (SDK v2, SDK v1, WalletConnect, in-app browser) for analytics during dApp permission/connection flows.

Key change: SDK v2 detection now uses v2Connections Redux store lookup (UUID key presence) instead of checking for a SDK_CONNECT_V2_ORIGIN string prefix. This is a behavioral fix that corrects misclassification of SDK v2 connections.

The hook is consumed by:

  1. AccountConnect.tsx - account connection/permission approval screen
  2. MultichainAccountConnect.tsx - multi-chain account connection
  3. PermissionApproval.tsx - fires analytics events on permission requests

The changes are analytics-only (source type classification), not affecting core connection logic. However, since the hook is embedded in the dApp connection and permission approval flows, tests covering those flows should be run:

  • SmokeNetworkAbstractions: Tests chain permission system for dApps (granting/revoking chain access, dApp chain switch requests) — directly exercises AccountConnect and PermissionApproval
  • SmokeNetworkExpansion: Tests Solana/multi-chain dApp connect/disconnect flows — uses MultichainAccountConnect and the same permission approval path
  • SmokeMultiChainAPI: Tests CAIP-25 session API (wallet_createSession, wallet_getSession, wallet_revokeSession) — exercises PermissionApproval with multi-chain scopes

Per tag dependency rules: SmokeMultiChainAPI requires SmokeNetworkAbstractions and SmokeNetworkExpansion (already selected).

The changes are unit-test and test-fixture updates alongside the logic fix — no new UI components, no rendering changes, no data loading changes.

Performance Test Selection:
The changes are purely logic/analytics classification changes in a hook. No UI rendering, no list components, no data loading, no state management changes that would affect performance. No performance tests are warranted.

View GitHub Actions results

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

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

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.86%. Comparing base (bd7e6fd) to head (5a7473e).
⚠️ Report is 33 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #28290      +/-   ##
==========================================
+ Coverage   82.66%   82.86%   +0.19%     
==========================================
  Files        4866     4873       +7     
  Lines      126134   126408     +274     
  Branches    28268    28336      +68     
==========================================
+ Hits       104273   104751     +478     
+ Misses      14660    14454     -206     
- Partials     7201     7203       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 2, 2026

@adonesky1 adonesky1 added this pull request to the merge queue Apr 2, 2026
Merged via the queue into main with commit 27cccb3 Apr 2, 2026
78 of 79 checks passed
@adonesky1 adonesky1 deleted the fix/WAPI-1380-useOriginSource-v2-detection branch April 2, 2026 19:25
@github-actions github-actions bot locked and limited conversation to collaborators Apr 2, 2026
@weitingsun weitingsun added release-7.74.0 Issue or pull request that will be included in release 7.74.0 and removed release-99999.1.0 labels Apr 3, 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.74.0 Issue or pull request that will be included in release 7.74.0 risk-medium Moderate testing recommended · Possible bug introduction risk size-M team-wallet-integrations Wallet Integrations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants