fix: Fix reported dapp url for MMC Connection Request Completed event by jiexi · Pull Request #30817 · MetaMask/metamask-mobile

jiexi · 2026-05-29T17:43:04Z

Description

Fixes bug where the channel ID was being used in place of the self reported dapp url for the Connection Request Completed event for the MMC flow

Changelog

CHANGELOG entry: null

Related issues

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

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

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 (see labeling guidelines). Not required for external contributors.

Performance checks (if applicable)

I've tested on Android
- Ideally on a mid-range device; emulator is acceptable
I've tested with a power user scenario
- Use these power-user SRPs to import wallets with many accounts and tokens
I've instrumented key operations with Sentry traces for production performance metrics
- See trace() for usage and addToken for an example

For performance guidelines and tooling, see the Performance Guide.

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

Low Risk
Narrow analytics property fix on the connect flow with added unit tests; no permission or connection logic changes.

Overview
Fixes CONNECT_REQUEST_COMPLETED analytics on the multichain connect screen so referrer uses the dapp’s self-reported URL for SDK v2 (MMConnect) connections, matching SDK v1 and WalletConnect behavior instead of the channel UUID.

The referrer branch now treats isOriginMMSDKV2RemoteConn like legacy SDK remote connections (referrer = dappUrl, or empty when no URL). New tests cover SDK v2, SDK v1, WalletConnect, and plain hostname origins.

^{Reviewed by Cursor Bugbot for commit 43479ad. Bugbot is set up for automated code reviews on this repo. Configure here.}

adonesky1 · 2026-05-29T18:34:52Z

+    () => Boolean(sdkV2Connection?.isV2),
+    [sdkV2Connection?.isV2],


lol. Why does sdkV2Connection ( the object returned from useSDKV2Connection) need an isV2 property?

adonesky1

LGTM other than test rename and audit failure

Co-authored-by: Alex Donesky <adonesky@gmail.com>

…vent-referrer-value

cursor

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

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

^{Reviewed by Cursor Bugbot for commit 4aa0843. Configure here.}

…vent-referrer-value

github-actions · 2026-06-03T22:39:08Z

🔍 Smart E2E Test Selection

Selected E2E tags: SmokeNetworkAbstractions, SmokeMultiChainAPI, SmokeNetworkExpansion
Selected Performance tags: None (no tests recommended)
Risk Level: low
AI Confidence: 82%

click to see 🤖 AI reasoning details

E2E Test Selection:
The changes are confined to two files in the MultichainAccountConnect component:

MultichainAccountConnect.tsx: A single-line bug fix extending the referrer logic for the CONNECT_REQUEST_COMPLETED analytics event. Previously only SDKv1 (isOriginMMSDKRemoteConn) connections used the self-reported dApp URL as the referrer; now SDKv2 (isOriginMMSDKV2RemoteConn) connections also correctly use the dApp URL instead of the raw channel ID (UUID). This is purely an analytics tracking fix — no change to connection behavior, UI, or permissions logic.
MultichainAccountConnect.test.tsx: New unit tests covering the referrer logic for SDKv1, SDKv2, WalletConnect, and regular browser connections.

Why these tags:

SmokeNetworkAbstractions: MultichainAccountConnect is the core account connection modal used when dApps request chain permissions. This tag covers dApp chain permission flows.
SmokeMultiChainAPI: Tests CAIP-25 multi-chain session API and dApp permission management — directly exercises MultichainAccountConnect during connection flows.
SmokeNetworkExpansion: Tests multi-chain provider architecture including Solana dApp connect/disconnect flows, which go through MultichainAccountConnect.

The change is low risk since it only affects analytics tracking (the referrer field), not actual connection behavior. However, since MultichainAccountConnect is a shared component in all dApp connection flows, running the relevant connection-related test suites is prudent to ensure no regressions were introduced.

Performance Test Selection:
The change is a single-line analytics fix in the referrer logic of MultichainAccountConnect. It does not affect UI rendering, data loading, state management, or any performance-sensitive code paths. No performance tests are warranted.

View GitHub Actions results

adonesky1

LGTM!

ccharly

LGTM (not really sure to have the full context, but the code logic looks ok)

Fix reported dapp url for mmc connection_request_completed event

d247d0c

jiexi requested a review from a team as a code owner May 29, 2026 17:43

metamaskbotv2 Bot added the team-wallet-integrations Wallet Integrations team label May 29, 2026

github-actions Bot added the size-M label May 29, 2026

jiexi added the no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed label May 29, 2026

github-actions Bot added the risk:medium AI analysis: medium risk label May 29, 2026

adonesky1 reviewed May 29, 2026

View reviewed changes

Comment thread app/components/Views/AccountConnect/AccountConnect.test.tsx Outdated

adonesky1 previously approved these changes May 29, 2026

View reviewed changes

Update app/components/Views/AccountConnect/AccountConnect.test.tsx

df1a307

Co-authored-by: Alex Donesky <adonesky@gmail.com>

jiexi dismissed adonesky1’s stale review via df1a307 May 29, 2026 18:39

Merge branch 'main' into jl/WAPI-1459/fix-mmc-conn-request-complete-e…

9c7eb9d

…vent-referrer-value

github-actions Bot added risk:high AI analysis: high risk and removed risk:medium AI analysis: medium risk labels May 29, 2026

jiexi added 2 commits May 29, 2026 13:47

Merge branch 'main' into jl/WAPI-1459/fix-mmc-conn-request-complete-e…

ceac85b

…vent-referrer-value

lint:tsc

4aa0843

github-actions Bot added risk:low AI analysis: low risk and removed risk:high AI analysis: high risk labels Jun 1, 2026

cursor Bot reviewed Jun 1, 2026

View reviewed changes

Comment thread app/components/Views/AccountConnect/AccountConnect.test.tsx Outdated

Fix specs

978daa5

github-actions Bot added risk:high AI analysis: high risk and removed risk:low AI analysis: low risk labels Jun 2, 2026

Merge branch 'main' into jl/WAPI-1459/fix-mmc-conn-request-complete-e…

43479ad

…vent-referrer-value

adonesky1 approved these changes Jun 4, 2026

View reviewed changes

hmalik88 approved these changes Jun 4, 2026

View reviewed changes

ccharly approved these changes Jun 4, 2026

View reviewed changes

adonesky1 added this pull request to the merge queue Jun 4, 2026

Merged via the queue into main with commit 65af59b Jun 4, 2026
306 of 308 checks passed

adonesky1 deleted the jl/WAPI-1459/fix-mmc-conn-request-complete-event-referrer-value branch June 4, 2026 17:17

github-actions Bot locked and limited conversation to collaborators Jun 4, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Fix reported dapp url for MMC Connection Request Completed event#30817

fix: Fix reported dapp url for MMC Connection Request Completed event#30817
adonesky1 merged 7 commits into
mainfrom
jl/WAPI-1459/fix-mmc-conn-request-complete-event-referrer-value

jiexi commented May 29, 2026 •

edited by cursor Bot

Loading

Uh oh!

adonesky1 May 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

adonesky1 left a comment

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Jun 3, 2026

Uh oh!

adonesky1 left a comment

Uh oh!

ccharly left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		() => Boolean(sdkV2Connection?.isV2),
		[sdkV2Connection?.isV2],

Uh oh!

Conversation

jiexi commented May 29, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Performance checks (if applicable)

Pre-merge reviewer checklist

Uh oh!

adonesky1 May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adonesky1 left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jun 3, 2026

🔍 Smart E2E Test Selection

Uh oh!

adonesky1 left a comment

Choose a reason for hiding this comment

Uh oh!

ccharly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jiexi commented May 29, 2026 •

edited by cursor Bot

Loading

adonesky1 May 29, 2026 •

edited

Loading