Skip to content
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

Add primaryType field in signature metrics #13132

Merged
merged 2 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,27 +1,15 @@
import { MetaMetricsEvents } from '../../../../core/Analytics';
import {
securityAlertResponse,
typedSignV4ConfirmationState,
typedSignV4SignatureRequest,
} from '../../../../util/test/confirm-data-helpers';
import { renderHookWithProvider } from '../../../../util/test/renderWithProvider';
import { useSignatureMetrics } from './useSignatureMetrics';
import { SignatureRequestType, SignatureRequest } from '@metamask/signature-controller';

const mockSigRequest = {
type: SignatureRequestType.PersonalSign,
messageParams: {
data: '0x4578616d706c652060706572736f6e616c5f7369676e60206d657373616765',
from: '0x935e73edb9ff52e23bac7f7e043a1ecd06d05477',
meta: {
url: 'https://metamask.github.io/test-dapp/',
title: 'E2E Test Dapp',
icon: { uri: 'https://metamask.github.io/metamask-fox.svg' },
analytics: { request_source: 'In-App-Browser' },
},
origin: 'metamask.github.io',
metamaskId: '76b33b40-7b5c-11ef-bc0a-25bce29dbc09',
},
chainId: '0x1' as `0x${string}`,
} as const;

const mockTypedSignV4SignatureRequest = typedSignV4SignatureRequest;
jest.mock('./useSignatureRequest', () => ({
useSignatureRequest: () => mockSigRequest,
useSignatureRequest: () => mockTypedSignV4SignatureRequest,
}));

jest.mock('../../../../util/address', () => ({
Expand All @@ -36,36 +24,53 @@ jest.mock('../../../../core/Analytics', () => ({
},
}));

const mockAddProperties = jest
.fn()
.mockImplementation(() => ({ build: () => ({}) }));
jest.mock('../../../../core/Analytics/MetricsEventBuilder', () => ({
...jest.requireActual('../../../../core/Analytics/MetricsEventBuilder'),
MetricsEventBuilder: {
createEventBuilder: () => ({ addProperties: mockAddProperties }),
},
}));

const SignatureMetrics = {
account_type: '0x935e73edb9ff52e23bac7f7e043a1ecd06d05477',
chain_id: '1',
dapp_host_name: 'metamask.github.io',
eip712_primary_type: 'Permit',
request_source: 'In-App-Browser',
security_alert_reason: 'permit_farming',
security_alert_response: 'Malicious',
security_alert_source: 'api',
signature_type: 'eth_signTypedData',
ui_customizations: ['flagged_as_malicious'],
version: 'V4',
};

describe('useSignatureMetrics', () => {
beforeEach(() => {
jest.clearAllMocks();
});
it('should capture metrics events correctly', async () => {
const { result } = renderHookWithProvider(() => useSignatureMetrics(), {
state: {
engine: {
backgroundState: {
PreferencesController: {
useTransactionSimulations: true,
},
SignatureController: {
signatureRequests: {
[mockSigRequest.messageParams.metamaskId]: mockSigRequest,
} as unknown as Record<string, SignatureRequest>,
},
},
},
...typedSignV4ConfirmationState,
signatureRequest: { securityAlertResponse },
},
});
// first call for 'SIGNATURE_REQUESTED' event
expect(mockTrackEvent).toHaveBeenCalledTimes(1);
expect(mockAddProperties).toHaveBeenCalledWith(SignatureMetrics);
result?.current?.captureSignatureMetrics(
MetaMetricsEvents.SIGNATURE_APPROVED,
);
expect(mockTrackEvent).toHaveBeenCalledTimes(2);
expect(mockAddProperties).toHaveBeenLastCalledWith(SignatureMetrics);
result?.current?.captureSignatureMetrics(
MetaMetricsEvents.SIGNATURE_REJECTED,
);
expect(mockTrackEvent).toHaveBeenCalledTimes(3);
expect(mockAddProperties).toHaveBeenLastCalledWith(SignatureMetrics);
});
});
57 changes: 35 additions & 22 deletions app/components/Views/confirmations/hooks/useSignatureMetrics.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Hex } from '@metamask/utils';
import { DecodingData } from '@metamask/signature-controller';
import { SecurityAlertResponse } from '@metamask/transaction-controller';
import { useCallback, useEffect } from 'react';
import { useCallback, useEffect, useMemo } from 'react';

import getDecimalChainId from '../../../../util/networks/getDecimalChainId';
import { MetricsEventBuilder } from '../../../../core/Analytics/MetricsEventBuilder';
Expand All @@ -14,6 +14,7 @@ import { getSignatureDecodingEventProps } from '../utils/signatureMetrics';
import { useSignatureRequest } from './useSignatureRequest';
import { useSecurityAlertResponse } from './useSecurityAlertResponse';
import { useTypedSignSimulationEnabled } from './useTypedSignSimulationEnabled';
import { getSignatureRequestPrimaryType } from '../utils/signature';

interface MessageParamsType {
meta: Record<string, unknown>;
Expand All @@ -30,6 +31,7 @@ const getAnalyticsParams = (
decodingData: DecodingData | undefined,
decodingLoading: boolean,
isSimulationEnabled: boolean,
primaryType: string,
) => {
const { meta = {}, from, version } = messageParams;

Expand All @@ -40,6 +42,7 @@ const getAnalyticsParams = (
version: version || 'N/A',
chain_id: chainId ? getDecimalChainId(chainId) : '',
ui_customizations: ['redesigned_confirmation'],
...(primaryType ? { eip712_primary_type: primaryType } : {}),
...(meta.analytics as Record<string, string>),
...(securityAlertResponse
? getBlockaidMetricsParams(securityAlertResponse)
Expand All @@ -59,40 +62,50 @@ export const useSignatureMetrics = () => {

const { chainId, decodingData, decodingLoading, messageParams, type } =
signatureRequest ?? {};
const primaryType =
signatureRequest && getSignatureRequestPrimaryType(signatureRequest);

const analyticsParams = useMemo(() => {
if (!type || !isSignatureRequest(type)) {
return;
}

return getAnalyticsParams(
messageParams as unknown as MessageParamsType,
securityAlertResponse as SecurityAlertResponse,
type,
chainId,
decodingData,
!!decodingLoading,
!!isSimulationEnabled,
primaryType,
);
}, [
chainId,
decodingData,
decodingLoading,
isSimulationEnabled,
messageParams,
primaryType,
securityAlertResponse,
type,
]);
jpuri marked this conversation as resolved.
Show resolved Hide resolved

const captureSignatureMetrics = useCallback(
async (
event: (typeof MetaMetricsEvents)[keyof typeof MetaMetricsEvents],
) => {
if (!type || !isSignatureRequest(type)) {
if (!analyticsParams) {
return;
}

MetaMetrics.getInstance().trackEvent(
MetricsEventBuilder.createEventBuilder(event)
.addProperties(
getAnalyticsParams(
messageParams as unknown as MessageParamsType,
securityAlertResponse as SecurityAlertResponse,
type,
chainId,
decodingData,
!!decodingLoading,
!!isSimulationEnabled,
),
)
.addProperties(analyticsParams)
.build(),
);
},
[
chainId,
decodingData,
decodingLoading,
isSimulationEnabled,
messageParams,
securityAlertResponse,
type,
],
[analyticsParams],
);

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export function useTypedSignSimulationEnabled() {
requestType === SignatureRequestType.TypedSign &&
(signatureMethod === SignTypedDataVersion.V3 ||
signatureMethod === SignTypedDataVersion.V4);
const isPermit = isTypedSignV3V4 && isRecognizedPermit(signatureRequest);
const isPermit = isRecognizedPermit(signatureRequest);

const nonPermitSupportedByDecodingAPI: boolean =
isTypedSignV3V4 && isNonPermitSupportedByDecodingAPI(signatureRequest);
Expand Down
84 changes: 69 additions & 15 deletions app/components/Views/confirmations/utils/signature.test.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,34 @@
import { parseTypedDataMessage, isRecognizedPermit } from './signature';
import {
parseTypedDataMessage,
isRecognizedPermit,
isTypedSignV3V4Request,
getSignatureRequestPrimaryType,
} from './signature';
import { PRIMARY_TYPES_PERMIT } from '../constants/signatures';
import { SignatureRequest, SignatureRequestType } from '@metamask/signature-controller';
import {
SignatureRequest,
SignatureRequestType,
} from '@metamask/signature-controller';
import {
personalSignSignatureRequest,
typedSignV1SignatureRequest,
typedSignV3SignatureRequest,
typedSignV4SignatureRequest,
} from '../../../../util/test/confirm-data-helpers';

describe('Signature Utils', () => {
describe('parseTypedDataMessage', () => {
it('should parse typed data message correctly', () => {
const data = JSON.stringify({
message: {
value: '123'
}
value: '123',
},
});
const result = parseTypedDataMessage(data);
expect(result).toEqual({
message: {
value: '123'
}
value: '123',
},
});
});

Expand All @@ -25,13 +39,12 @@ describe('Signature Utils', () => {
expect(result.message.value).toBe('3000123');
});


it('should handle large message values. This prevents native JS number coercion when the value is greater than Number.MAX_SAFE_INTEGER.', () => {
const largeValue = '123456789012345678901234567890';
const data = JSON.stringify({
message: {
value: largeValue
}
value: largeValue,
},
});
const result = parseTypedDataMessage(data);
expect(result.message.value).toBe(largeValue);
Expand All @@ -49,10 +62,11 @@ describe('Signature Utils', () => {
const mockRequest: SignatureRequest = {
messageParams: {
data: JSON.stringify({
primaryType: PRIMARY_TYPES_PERMIT[0]
})
primaryType: PRIMARY_TYPES_PERMIT[0],
}),
version: 'V3',
},
type: SignatureRequestType.TypedSign
type: SignatureRequestType.TypedSign,
} as SignatureRequest;

expect(isRecognizedPermit(mockRequest)).toBe(true);
Expand All @@ -62,13 +76,53 @@ describe('Signature Utils', () => {
const mockRequest: SignatureRequest = {
messageParams: {
data: JSON.stringify({
primaryType: 'UnrecognizedType'
})
primaryType: 'UnrecognizedType',
}),
version: 'V3',
},
type: SignatureRequestType.TypedSign
type: SignatureRequestType.TypedSign,
} as SignatureRequest;

expect(isRecognizedPermit(mockRequest)).toBe(false);
});

it('should return false for typed sign V1 request', () => {
expect(isRecognizedPermit(typedSignV1SignatureRequest)).toBe(false);
expect(isRecognizedPermit(personalSignSignatureRequest)).toBe(false);
});
});

describe('isTypedSignV3V4Request', () => {
it('return true for typed sign V3, V4 messages', () => {
expect(isTypedSignV3V4Request(typedSignV3SignatureRequest)).toBe(true);
expect(isTypedSignV3V4Request(typedSignV4SignatureRequest)).toBe(true);
});
it('return false for typed sign V1 message', () => {
expect(isTypedSignV3V4Request(typedSignV1SignatureRequest)).toBe(false);
});
it('return false for personal sign message', () => {
expect(isTypedSignV3V4Request(personalSignSignatureRequest)).toBe(false);
});
});

describe('getSignatureRequestPrimaryType', () => {
it('return correct primary type', () => {
expect(getSignatureRequestPrimaryType(typedSignV3SignatureRequest)).toBe(
'Mail',
);
expect(getSignatureRequestPrimaryType(typedSignV4SignatureRequest)).toBe(
'Permit',
);
});
it('return undefined for for typed sign V1 message', () => {
expect(getSignatureRequestPrimaryType(typedSignV1SignatureRequest)).toBe(
undefined,
);
});
it('return undefined for personal sign message', () => {
expect(getSignatureRequestPrimaryType(personalSignSignatureRequest)).toBe(
undefined,
);
});
});
});
Loading
Loading