-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: pass anon_id from dapp to wallet in SDKConnectV2 connection metadata for cross-side analytics correlation #28470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 7 commits
a3d6a3b
b794a4a
1ad1f2a
45e194c
70344e4
1e93d48
3771262
1087ce1
ec0f6c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ import { | |
| getUrlObj, | ||
| prefixUrlWithProtocol, | ||
| } from '../../../../util/browser/index.ts'; | ||
| import { getAddressAccountType } from '../../../../util/address/index.ts'; | ||
|
|
||
| // Internal dependencies. | ||
| import { PermissionsRequest } from '@metamask/permission-controller'; | ||
|
|
@@ -258,6 +259,7 @@ const MultichainAccountConnect = (props: AccountConnectProps) => { | |
| const { origin: channelIdOrHostname, isEip1193Request } = hostInfo.metadata; | ||
|
|
||
| const sdkV2Connection = useSDKV2Connection(channelIdOrHostname); | ||
| const anonId = sdkV2Connection?.originatorInfo?.anonId; | ||
| const isOriginMMSDKV2RemoteConn = useMemo( | ||
| () => Boolean(sdkV2Connection?.isV2), | ||
| [sdkV2Connection?.isV2], | ||
|
|
@@ -601,13 +603,15 @@ const MultichainAccountConnect = (props: AccountConnectProps) => { | |
| chain_id_list: chainIds, | ||
| referrer: channelIdOrHostname, | ||
| ...getApiAnalyticsProperties(isMultichainRequest), | ||
| ...(anonId ? { anon_id: anonId } : {}), | ||
| }) | ||
| .build(), | ||
| ); | ||
| }, | ||
| [ | ||
| accountsLength, | ||
| channelIdOrHostname, | ||
| anonId, | ||
| trackEvent, | ||
| createEventBuilder, | ||
| eventSource, | ||
|
|
@@ -694,17 +698,24 @@ const MultichainAccountConnect = (props: AccountConnectProps) => { | |
|
|
||
| triggerDappViewedEvent(connectedAccountLength); | ||
|
|
||
| let accountType: string; | ||
| try { | ||
| accountType = getAddressAccountType(selectedCaipAccountIds[0]); | ||
| } catch { | ||
| accountType = 'unknown'; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated behavior change bundled without test coverageMedium Severity
Additional Locations (1)Reviewed by Cursor Bugbot for commit 3771262. Configure here. |
||
|
|
||
| trackEvent( | ||
| createEventBuilder(MetaMetricsEvents.CONNECT_REQUEST_COMPLETED) | ||
| .addProperties({ | ||
| number_of_accounts: accountsLength, | ||
| number_of_accounts_connected: connectedAccountLength, | ||
| // TODO: Fix this. Not accurate | ||
| account_type: 'multichain', | ||
| account_type: accountType, | ||
| source: eventSource, | ||
| chain_id_list: selectedChainIds, | ||
| referrer, | ||
| ...getApiAnalyticsProperties(isMultichainRequest), | ||
| ...(anonId ? { anon_id: anonId } : {}), | ||
| }) | ||
| .build(), | ||
| ); | ||
|
|
@@ -728,6 +739,7 @@ const MultichainAccountConnect = (props: AccountConnectProps) => { | |
| setIsLoading(false); | ||
| } | ||
| }, [ | ||
| anonId, | ||
| hostInfo, | ||
| channelIdOrHostname, | ||
| requestedRequestWithExistingPermissions, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -148,5 +148,18 @@ export function isConnectionRequest(data: unknown): data is ConnectionRequest { | |
| return false; | ||
| } | ||
|
|
||
| // analytics is purely telemetry — strip it when malformed rather than | ||
| // rejecting the connection, so a dapp-side bug can't break connectivity. | ||
| if (metadata.analytics !== undefined) { | ||
| if ( | ||
| typeof metadata.analytics !== 'object' || | ||
| metadata.analytics === null || | ||
| typeof metadata.analytics.anon_id !== 'string' || | ||
| !isUUID(metadata.analytics.anon_id) | ||
| ) { | ||
| delete (metadata as unknown as Record<string, unknown>).analytics; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type guard mutates its input argument silentlyLow Severity The Reviewed by Cursor Bugbot for commit ec0f6c9. Configure here. |
||
| } | ||
adonesky1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return true; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,4 +11,7 @@ export interface Metadata { | |
| version: string; | ||
| platform: string; | ||
| }; | ||
| analytics?: { | ||
| anon_id: string; | ||
| }; | ||
| } | ||


Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe in all of these cases it should be added as a sensitive property as it is done in all of the SDKv1 instances?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1ad1f2a