Conversation
github-actions
bot
added
size-S
risk-low
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| ? dappUrl | ||
| : isOriginWalletConnect | ||
| ? wc2Metadata?.url | ||
| : channelIdOrHostname; |
There was a problem hiding this comment.
V2 SDK connections not handled in referrer computation
Medium Severity
The referrer computation only checks isOriginMMSDKRemoteConn (v1 SDK connections) but this component also supports v2 SDK connections via isOriginMMSDKV2RemoteConn. For v2 SDK connections, isOriginMMSDKRemoteConn is false, so referrer falls through to channelIdOrHostname instead of using the self-reported dapp URL from dappUrl (which already includes sdkV2Connection?.originatorInfo?.url at lines 478–481). This defeats the PR's purpose of reporting the self-reported dapp URL for SDK connections.
jiexi
added
the
no-changelog
jiexi
changed the title
| // Should be the self reported dapp url if SDK or WC connection, null if no self reported dapp url. | ||
| // If not SDK or WC connection, i.e. a regular external connection, it should be the hostname. | ||
| const referrer = isOriginMMSDKRemoteConn | ||
| ? dappUrl | ||
| : isOriginWalletConnect | ||
| ? wc2Metadata?.url | ||
| : channelIdOrHostname; |
There was a problem hiding this comment.
looks like dappUrl will default to '' not null? not sure about wc2Metadata?.url
There was a problem hiding this comment.
yeah good point. The comment is wrong. I really just want the value to be falsey here (i.e. not the channelId) when the connection is SDK or WC and the self reported dapp url is not available.
github-actions
bot
added
risk-low
jiexi
changed the title
…ction-request-completed
|
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. |
github-actions
bot
added
risk-low
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
These changes do NOT affect:
However, since these components are in the critical dApp connection path (both EVM and multichain), it's prudent to run tests that exercise these flows to ensure no regressions were introduced:
Performance tests are not warranted as these are pure analytics property changes with no UI rendering, data loading, or performance-sensitive code paths affected. Performance Test Selection: |
|
✅ E2E Fixture Validation — Schema is up to date |
|
jiexi
deleted the
jl/WAPI-1338/self-reported-dapp-url-in-connection-request-completed
branch
weitingsun
added
release-7.74.0
5 participants
Description
Uses the self reported dapp url when available for the
CONNECT_REQUEST_COMPLETEDevent when the connection is a SDKv1, SDKv2, or WC connection. It uses the trusted origin otherwise.Changelog
CHANGELOG entry: null
Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-1338
Manual testing steps
referrerthat should be the dapp's url, not a UUIDIt may be easier to use an expo build and the js debugger to view analytic network events.
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Low Risk
Low risk analytics-only change that alters the
referrervalue emitted forCONNECT_REQUEST_COMPLETED, potentially affecting downstream dashboards but not user-facing behavior.Overview
Updates analytics for connection approvals so
MetaMetricsEvents.CONNECT_REQUEST_COMPLETEDnow reports a computedreferrer(SDK/WC self-reported dapp URL when available, otherwise the trusted hostname/origin) instead of always usingrequest.metadata.origin.Applies the same
referrerlogic in bothAccountConnectandMultichainAccountConnect, and wires it into thehandleConnecttracking dependencies.Written by Cursor Bugbot for commit ad3bb42. This will update automatically on new commits. Configure here.