Skip to content

Commit

Permalink
feat: Decoding Simulation Metrics (MetaMask#13041)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Add Decoding Simulation details to Signature request metrics

Note:
- Split useTypedSignSimulationEnabled logic into
selectTypedSignSimulationEnabled to support React class components

## **Related issues**

Fixes: MetaMask/MetaMask-planning#3858

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] 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](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.
  • Loading branch information
digiwand authored Jan 20, 2025
1 parent 7139a15 commit e921135
Show file tree
Hide file tree
Showing 7 changed files with 211 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@ jest.mock('react-native-gzip', () => ({
}));

const mockTrackEvent = jest.fn();
const mockCreateEventBuilderAddProperties = jest.fn();

jest.mock('../../../../../hooks/useMetrics', () => ({
useMetrics: () => ({
trackEvent: mockTrackEvent,
createEventBuilder: () => ({
addProperties: () => ({ build: () => ({}) }),
addProperties: mockCreateEventBuilderAddProperties.mockReturnValue({
build: () => ({}),
}),
}),
}),
}));
Expand All @@ -36,7 +40,7 @@ const typedSignV1ConfirmationStateWithBlockaidResponse = {
...typedSignV1ConfirmationState.engine.backgroundState,
ApprovalController: {
pendingApprovals: {
'fb2029e1-b0ab-11ef-9227-05a11087c334': {
'7e62bcb1-a4e9-11ef-9b51-ddf21c91a998': {
...typedSignApproval,
requestData: {
...typedSignApproval.requestData,
Expand Down Expand Up @@ -71,8 +75,15 @@ describe('Confirm', () => {
state: typedSignV1ConfirmationStateWithBlockaidResponse,
},
);

fireEvent.press(getByTestId('accordionheader'));
fireEvent.press(getByText('Report an issue'));

expect(mockTrackEvent).toHaveBeenCalledTimes(1);
expect(mockCreateEventBuilderAddProperties).toHaveBeenCalledWith(
expect.objectContaining({
external_link_clicked: 'security_alert_support_link',
}),
);
});
});
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { MetaMetricsEvents } from '../../../../core/Analytics';
import { renderHookWithProvider } from '../../../../util/test/renderWithProvider';
import { useSignatureMetrics } from './useSignatureMetrics';
import { SignatureRequestType, SignatureRequest } from '@metamask/signature-controller';

const mockSigRequest = {
type: 'personal_sign',
type: SignatureRequestType.PersonalSign,
messageParams: {
data: '0x4578616d706c652060706572736f6e616c5f7369676e60206d657373616765',
from: '0x935e73edb9ff52e23bac7f7e043a1ecd06d05477',
Expand All @@ -16,8 +17,8 @@ const mockSigRequest = {
origin: 'metamask.github.io',
metamaskId: '76b33b40-7b5c-11ef-bc0a-25bce29dbc09',
},
chainId: '0x0',
};
chainId: '0x1' as `0x${string}`,
} as const;

jest.mock('./useSignatureRequest', () => ({
useSignatureRequest: () => mockSigRequest,
Expand All @@ -41,7 +42,20 @@ describe('useSignatureMetrics', () => {
});
it('should capture metrics events correctly', async () => {
const { result } = renderHookWithProvider(() => useSignatureMetrics(), {
state: {},
state: {
engine: {
backgroundState: {
PreferencesController: {
useTransactionSimulations: true,
},
SignatureController: {
signatureRequests: {
[mockSigRequest.messageParams.metamaskId]: mockSigRequest,
} as unknown as Record<string, SignatureRequest>,
},
},
},
},
});
// first call for 'SIGNATURE_REQUESTED' event
expect(mockTrackEvent).toHaveBeenCalledTimes(1);
Expand Down
28 changes: 18 additions & 10 deletions app/components/Views/confirmations/hooks/useSignatureMetrics.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { useCallback, useEffect } from 'react';
import type { Hex } from '@metamask/utils';

import getDecimalChainId from '../../../../util/networks/getDecimalChainId';
import { MetricsEventBuilder } from '../../../../core/Analytics/MetricsEventBuilder';
Expand All @@ -10,7 +9,10 @@ import { getBlockaidMetricsParams } from '../../../../util/blockaid';
import { SecurityAlertResponse } from '../components/BlockaidBanner/BlockaidBanner.types';
import { getHostFromUrl } from '../utils/generic';
import { isSignatureRequest } from '../utils/confirm';
import { getSignatureDecodingEventProps } from '../utils/signatureMetrics';
import { useSignatureRequest } from './useSignatureRequest';
import { useTypedSignSimulationEnabled } from './useTypedSignSimulationEnabled';
import { SignatureRequest } from '@metamask/signature-controller';

interface MessageParamsType {
meta: Record<string, unknown>;
Expand All @@ -20,11 +22,16 @@ interface MessageParamsType {
}

const getAnalyticsParams = (
messageParams: MessageParamsType,
type: string,
chainId?: Hex,
signatureRequest: SignatureRequest,
isSimulationEnabled?: boolean,
) => {
const { meta = {}, from, securityAlertResponse, version } = messageParams;
const { chainId, messageParams, type } = signatureRequest ?? {};
const {
meta = {},
from,
securityAlertResponse,
version
} = (messageParams as unknown as MessageParamsType) || {};

return {
account_type: getAddressAccountType(from as string),
Expand All @@ -37,13 +44,15 @@ const getAnalyticsParams = (
...(securityAlertResponse
? getBlockaidMetricsParams(securityAlertResponse)
: {}),
...getSignatureDecodingEventProps(signatureRequest, isSimulationEnabled),
};
};

export const useSignatureMetrics = () => {
const signatureRequest = useSignatureRequest();
const isSimulationEnabled = useTypedSignSimulationEnabled();

const { chainId, messageParams, type } = signatureRequest ?? {};
const type = signatureRequest?.type;

const captureSignatureMetrics = useCallback(
async (
Expand All @@ -57,15 +66,14 @@ export const useSignatureMetrics = () => {
MetricsEventBuilder.createEventBuilder(event)
.addProperties(
getAnalyticsParams(
messageParams as unknown as MessageParamsType,
type,
chainId,
signatureRequest as SignatureRequest,
isSimulationEnabled,
),
)
.build(),
);
},
[chainId, messageParams, type],
[isSimulationEnabled, type, signatureRequest],
);

useEffect(() => {
Expand Down
8 changes: 5 additions & 3 deletions app/components/Views/confirmations/utils/signature.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { parseTypedDataMessage, isRecognizedPermit } from './signature';
import { PRIMARY_TYPES_PERMIT } from '../constants/signatures';
import { SignatureRequest } from '@metamask/signature-controller';
import { SignatureRequest, SignatureRequestType } from '@metamask/signature-controller';

describe('Signature Utils', () => {
describe('parseTypedDataMessage', () => {
Expand Down Expand Up @@ -51,7 +51,8 @@ describe('Signature Utils', () => {
data: JSON.stringify({
primaryType: PRIMARY_TYPES_PERMIT[0]
})
}
},
type: SignatureRequestType.TypedSign
} as SignatureRequest;

expect(isRecognizedPermit(mockRequest)).toBe(true);
Expand All @@ -63,7 +64,8 @@ describe('Signature Utils', () => {
data: JSON.stringify({
primaryType: 'UnrecognizedType'
})
}
},
type: SignatureRequestType.TypedSign
} as SignatureRequest;

expect(isRecognizedPermit(mockRequest)).toBe(false);
Expand Down
4 changes: 2 additions & 2 deletions app/components/Views/confirmations/utils/signature.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SignatureRequest } from '@metamask/signature-controller';
import { SignatureRequest, SignatureRequestType } from '@metamask/signature-controller';
import { PRIMARY_TYPES_PERMIT } from '../constants/signatures';

/**
Expand Down Expand Up @@ -50,7 +50,7 @@ export const parseTypedDataMessage = (dataToParse: string) => {
* @param request - The signature request to check
*/
export const isRecognizedPermit = (request: SignatureRequest) => {
if (!request) {
if (!request || request.type !== SignatureRequestType.TypedSign) {
return false;
}

Expand Down
121 changes: 121 additions & 0 deletions app/components/Views/confirmations/utils/signatureMetrics.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import { Hex } from '@metamask/utils';
import { getSignatureDecodingEventProps } from './signatureMetrics';
import { DecodingDataChangeType, SignatureRequest, SignatureRequestStatus, SignatureRequestType } from '@metamask/signature-controller';

const mockSignatureRequest = {
id: 'fb2029e1-b0ab-11ef-9227-05a11087c334',
chainId: '0x1' as Hex,
type: SignatureRequestType.TypedSign,
messageParams: {
data: '{"types":{"EIP712Domain":[{"name":"name","type":"string"},{"name":"version","type":"string"},{"name":"chainId","type":"uint256"},{"name":"verifyingContract","type":"address"}],"Permit":[{"name":"owner","type":"address"},{"name":"spender","type":"address"},{"name":"value","type":"uint256"},{"name":"nonce","type":"uint256"},{"name":"deadline","type":"uint256"}]},"primaryType":"Permit","domain":{"name":"MyToken","version":"1","verifyingContract":"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC","chainId":1},"message":{"owner":"0x935e73edb9ff52e23bac7f7e043a1ecd06d05477","spender":"0x5B38Da6a701c568545dCfcB03FcB875f56beddC4","value":3000,"nonce":0,"deadline":50000000000}}',
from: '0x935e73edb9ff52e23bac7f7e043a1ecd06d05477',
version: 'V4',
requestId: 14,
origin: 'https://metamask.github.io',
metamaskId: 'fb2029e0-b0ab-11ef-9227-05a11087c334',
},
networkClientId: '1',
status: SignatureRequestStatus.Unapproved,
time: 1733143817088
} satisfies SignatureRequest;

describe('signatureMetrics', () => {
describe('getSignatureDecodingEventProps', () => {
it('returns empty object when decoding API is disabled', () => {
const mockRequest = {
...mockSignatureRequest,
decodingData: {
stateChanges: [],
error: undefined,
},
} satisfies SignatureRequest;

const result = getSignatureDecodingEventProps(mockRequest, false);
expect(result).toEqual({});
});

it('returns empty object when no decodingData is present', () => {
const mockRequest = {} as SignatureRequest;
const result = getSignatureDecodingEventProps(mockRequest, true);
expect(result).toEqual({});
});

it('returns no change response when stateChanges are empty', () => {
const mockRequest = {
...mockSignatureRequest,
decodingData: {
stateChanges: [],
error: undefined,
},
decodingLoading: false,
} satisfies SignatureRequest;

const result = getSignatureDecodingEventProps(mockRequest, true);
expect(result).toEqual({
decoding_change_types: [],
decoding_description: null,
decoding_response: 'NO_CHANGE',
});
});

it('returns loading response when decodingLoading is true', () => {
const mockRequest = {
...mockSignatureRequest,
decodingData: {
stateChanges: [],
error: undefined,
},
decodingLoading: true,
} satisfies SignatureRequest;

const result = getSignatureDecodingEventProps(mockRequest, true);
expect(result).toEqual({
decoding_change_types: [],
decoding_description: null,
decoding_response: 'decoding_in_progress',
});
});

it('returns error response when error exists', () => {
const mockRequest = {
...mockSignatureRequest,
decodingData: {
stateChanges: [],
error: {
type: 'ERROR_TYPE',
message: 'Error message',
},
},
decodingLoading: false,
} satisfies SignatureRequest;

const result = getSignatureDecodingEventProps(mockRequest, true);
expect(result).toEqual({
decoding_change_types: [],
decoding_description: 'Error message',
decoding_response: 'ERROR_TYPE',
});
});

it('returns change response when stateChanges exist', () => {
const mockRequest = {
...mockSignatureRequest,
decodingData: {
stateChanges: [
{ changeType: DecodingDataChangeType.Approve, assetType: 'ERC20', address: '0x935e73edb9ff52e23bac7f7e043a1ecd06d05477', amount: '12345', contractAddress: '0x6b175474e89094c44da98b954eedeac495271d0f' },
{ changeType: DecodingDataChangeType.Transfer, assetType: 'ERC20', address: '0x935e73edb9ff52e23bac7f7e043a1ecd06d05477', amount: '12345', contractAddress: '0x6b175474e89094c44da98b954eedeac495271d0f' },
],
error: undefined,
},
decodingLoading: false,
} satisfies SignatureRequest;

const result = getSignatureDecodingEventProps(mockRequest, true);
expect(result).toEqual({
decoding_change_types: ['APPROVE', 'TRANSFER'],
decoding_description: null,
decoding_response: 'CHANGE',
});
});
});
});
34 changes: 34 additions & 0 deletions app/components/Views/confirmations/utils/signatureMetrics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { DecodingDataStateChange, SignatureRequest } from '@metamask/signature-controller';

enum DecodingResponseType {
Change = 'CHANGE',
NoChange = 'NO_CHANGE',
Loading = 'decoding_in_progress',
}

export const getSignatureDecodingEventProps = (signatureRequest?: SignatureRequest, isDecodingAPIEnabled: boolean = false) => {
const { decodingData, decodingLoading } = signatureRequest || {};

if (!isDecodingAPIEnabled || !decodingData) {
return {};
}

const { stateChanges, error } = decodingData;

const changeTypes = (stateChanges ?? []).map(
(change: DecodingDataStateChange) => change.changeType,
);

const responseType = error?.type ??
(changeTypes.length
? DecodingResponseType.Change
: DecodingResponseType.NoChange);

return {
decoding_change_types: changeTypes,
decoding_description: decodingData?.error?.message ?? null,
decoding_response: decodingLoading
? DecodingResponseType.Loading
: responseType,
};
};

0 comments on commit e921135

Please sign in to comment.