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

feat: adding metrics for signature decoding #28719

Merged
merged 16 commits into from
Nov 27, 2024
Merged
20 changes: 20 additions & 0 deletions test/e2e/tests/confirmations/signatures/signature-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ type AssertSignatureMetricsOptions = {
withAnonEvents?: boolean;
securityAlertReason?: string;
securityAlertResponse?: string;
decodingChangeTypes?: string[];
decodingResponse?: string;
};

type SignatureEventProperty = {
Expand All @@ -49,6 +51,8 @@ type SignatureEventProperty = {
security_alert_response: string;
signature_type: string;
eip712_primary_type?: string;
decoding_change_types?: string[];
decoding_response?: string;
ui_customizations?: string[];
location?: string;
};
Expand All @@ -67,13 +71,17 @@ const signatureAnonProperties = {
* @param uiCustomizations
* @param securityAlertReason
* @param securityAlertResponse
* @param decodingChangeTypes
* @param decodingResponse
*/
function getSignatureEventProperty(
signatureType: string,
primaryType: string,
uiCustomizations: string[],
securityAlertReason: string = BlockaidReason.checkingChain,
securityAlertResponse: string = BlockaidResultType.Loading,
decodingChangeTypes?: string[],
decodingResponse?: string,
): SignatureEventProperty {
const signatureEventProperty: SignatureEventProperty = {
account_type: 'MetaMask',
Expand All @@ -91,6 +99,10 @@ function getSignatureEventProperty(
signatureEventProperty.eip712_primary_type = primaryType;
}

if (decodingResponse) {
signatureEventProperty.decoding_change_types = decodingChangeTypes;
signatureEventProperty.decoding_response = decodingResponse;
}
return signatureEventProperty;
}

Expand Down Expand Up @@ -123,6 +135,8 @@ export async function assertSignatureConfirmedMetrics({
withAnonEvents = false,
securityAlertReason,
securityAlertResponse,
decodingChangeTypes,
decodingResponse,
}: AssertSignatureMetricsOptions) {
const events = await getEventPayloads(driver, mockedEndpoints);
const signatureEventProperty = getSignatureEventProperty(
Expand All @@ -131,6 +145,8 @@ export async function assertSignatureConfirmedMetrics({
uiCustomizations,
securityAlertReason,
securityAlertResponse,
decodingChangeTypes,
decodingResponse,
);

assertSignatureRequestedMetrics(
Expand Down Expand Up @@ -164,6 +180,8 @@ export async function assertSignatureRejectedMetrics({
withAnonEvents = false,
securityAlertReason,
securityAlertResponse,
decodingChangeTypes,
decodingResponse,
}: AssertSignatureMetricsOptions) {
const events = await getEventPayloads(driver, mockedEndpoints);
const signatureEventProperty = getSignatureEventProperty(
Expand All @@ -172,6 +190,8 @@ export async function assertSignatureRejectedMetrics({
uiCustomizations,
securityAlertReason,
securityAlertResponse,
decodingChangeTypes,
decodingResponse,
);

assertSignatureRequestedMetrics(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ exports[`PermitSimulation should not render default simulation if decodingLoadin
</p>
<div>
<div
aria-describedby="tippy-tooltip-5"
aria-describedby="tippy-tooltip-7"
class=""
data-original-title="Estimated changes are what might happen if you go through with this transaction. This is just a prediction, not a guarantee."
data-tooltipped=""
Expand Down Expand Up @@ -238,7 +238,7 @@ exports[`PermitSimulation should render default simulation if decoding api retur
</p>
<div>
<div
aria-describedby="tippy-tooltip-3"
aria-describedby="tippy-tooltip-5"
class=""
data-original-title="Estimated changes are what might happen if you go through with this transaction. This is just a prediction, not a guarantee."
data-tooltipped=""
Expand Down Expand Up @@ -299,7 +299,7 @@ exports[`PermitSimulation should render default simulation if decoding api retur
style="min-width: 0;"
>
<div
aria-describedby="tippy-tooltip-4"
aria-describedby="tippy-tooltip-6"
class=""
data-original-title="30"
data-tooltipped=""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import { getMockTypedSignConfirmStateForRequest } from '../../../../../../../../
import { renderWithConfirmContextProvider } from '../../../../../../../../test/lib/confirmations/render-helpers';
import { permitSignatureMsg } from '../../../../../../../../test/data/confirmations/typed_sign';
import { memoizedGetTokenStandardAndDetails } from '../../../../../utils/token';
import * as SignatureMetrics from '../../../../../hooks/useDecodedSignatureMetrics';
import PermitSimulation from './permit-simulation';

jest.mock('../../../../../../../store/actions', () => {
return {
getTokenStandardAndDetails: jest
.fn()
.mockResolvedValue({ decimals: 2, standard: 'ERC20' }),
updateEventFragment: jest.fn(),
};
});

Expand Down Expand Up @@ -44,6 +46,25 @@ describe('PermitSimulation', () => {
});
});

it('should call hook to register signature metrics properties', async () => {
const state = getMockTypedSignConfirmStateForRequest({
...permitSignatureMsg,
decodingLoading: false,
decodingData: undefined,
});
const mockStore = configureMockStore([])(state);

const mockedUseDecodedSignatureMetrics = jest
.spyOn(SignatureMetrics, 'useDecodedSignatureMetrics')
.mockImplementation(() => '');

await act(async () => {
renderWithConfirmContextProvider(<PermitSimulation />, mockStore);

expect(mockedUseDecodedSignatureMetrics).toHaveBeenCalledTimes(1);
});
});

it('should render default simulation if decoding api returns error', async () => {
const state = getMockTypedSignConfirmStateForRequest({
...permitSignatureMsg,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ import React from 'react';

import { SignatureRequestType } from '../../../../../types/confirm';
import { useConfirmContext } from '../../../../../context/confirm';
import { useDecodedSignatureMetrics } from '../../../../../hooks/useDecodedSignatureMetrics';
import { DefaultSimulation } from './default-simulation';
import { DecodedSimulation } from './decoded-simulation';

const PermitSimulation: React.FC<object> = () => {
const { currentConfirmation } = useConfirmContext<SignatureRequestType>();
const { decodingLoading, decodingData } = currentConfirmation;

useDecodedSignatureMetrics();

if (
decodingData?.error ||
(decodingData?.stateChanges === undefined && decodingLoading !== true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ jest.mock(
jest.mock('../../../../../../store/actions', () => {
return {
getTokenStandardAndDetails: jest.fn().mockResolvedValue({ decimals: 2 }),
updateEventFragment: jest.fn(),
};
});

Expand Down
132 changes: 132 additions & 0 deletions ui/pages/confirmations/hooks/useDecodedSignatureMetrics.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
import { DecodingData, DecodingDataChangeType } from '@metamask/signature-controller';

import { getMockTypedSignConfirmStateForRequest } from '../../../../test/data/confirmations/helper';
import { renderHookWithConfirmContextProvider } from '../../../../test/lib/confirmations/render-helpers';
import { permitSignatureMsg } from '../../../../test/data/confirmations/typed_sign';
import * as SignatureEventFragment from './useSignatureEventFragment';
import { useDecodedSignatureMetrics } from './useDecodedSignatureMetrics';

const decodingData: DecodingData = {
stateChanges: [
{
assetType: 'ERC20',
changeType: DecodingDataChangeType.Approve,
address: '0x3fc91a3afd70395cd496c647d5a6cc9d4b2b7fad',
amount: '1461501637330902918203684832716283019655932542975',
contractAddress: '0x6b175474e89094c44da98b954eedeac495271d0f',
},
],
};

describe('useDecodedSignatureMetrics', () => {
process.env.ENABLE_SIGNATURE_DECODING = 'true';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: should we unset the ENV var after the tests are complete or is there another way to add ENV for builds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is being removed in a following PR

it('should not call updateSignatureEventFragment if decodingLoading is true', async () => {
const state = getMockTypedSignConfirmStateForRequest({
...permitSignatureMsg,
decodingLoading: true,
});

const mockUpdateSignatureEventFragment = jest.fn();
jest
.spyOn(SignatureEventFragment, 'useSignatureEventFragment')
.mockImplementation(() => ({
updateSignatureEventFragment: mockUpdateSignatureEventFragment,
}));

renderHookWithConfirmContextProvider(
() => useDecodedSignatureMetrics(),
state,
);

expect(mockUpdateSignatureEventFragment).toHaveBeenCalledTimes(0);
});

it('should call updateSignatureEventFragment with correct parameters if there are no state changes', async () => {
const state = getMockTypedSignConfirmStateForRequest({
...permitSignatureMsg,
decodingLoading: false,
});

const mockUpdateSignatureEventFragment = jest.fn();
jest
.spyOn(SignatureEventFragment, 'useSignatureEventFragment')
.mockImplementation(() => ({
updateSignatureEventFragment: mockUpdateSignatureEventFragment,
}));

renderHookWithConfirmContextProvider(
() => useDecodedSignatureMetrics(),
state,
);

expect(mockUpdateSignatureEventFragment).toHaveBeenCalledTimes(1);
expect(mockUpdateSignatureEventFragment).toHaveBeenLastCalledWith({
properties: {
decoding_change_types: [],
decoding_response: 'NO_CHANGE',
},
});
});

it('should call updateSignatureEventFragment with correct parameters if there are no state changes', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, same name as the other?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

const state = getMockTypedSignConfirmStateForRequest({
...permitSignatureMsg,
decodingLoading: false,
decodingData
});

const mockUpdateSignatureEventFragment = jest.fn();
jest
.spyOn(SignatureEventFragment, 'useSignatureEventFragment')
.mockImplementation(() => ({
updateSignatureEventFragment: mockUpdateSignatureEventFragment,
}));

renderHookWithConfirmContextProvider(
() => useDecodedSignatureMetrics(),
state,
);

expect(mockUpdateSignatureEventFragment).toHaveBeenCalledTimes(1);
expect(mockUpdateSignatureEventFragment).toHaveBeenLastCalledWith({
properties: {
decoding_change_types: ["APPROVE"],
decoding_response: 'CHANGE',
},
});
});

it('should call updateSignatureEventFragment with correct parameters if response has error', async () => {
const state = getMockTypedSignConfirmStateForRequest({
...permitSignatureMsg,
decodingLoading: false,
decodingData: {
stateChanges: [],
error: {
type: 'SOME_ERROR',
message: 'some message'
}
}
});

const mockUpdateSignatureEventFragment = jest.fn();
jest
.spyOn(SignatureEventFragment, 'useSignatureEventFragment')
.mockImplementation(() => ({
updateSignatureEventFragment: mockUpdateSignatureEventFragment,
}));

renderHookWithConfirmContextProvider(
() => useDecodedSignatureMetrics(),
state,
);

expect(mockUpdateSignatureEventFragment).toHaveBeenCalledTimes(1);
expect(mockUpdateSignatureEventFragment).toHaveBeenLastCalledWith({
properties: {
decoding_change_types: [],
decoding_response: 'SOME_ERROR',
},
});
});
});
46 changes: 46 additions & 0 deletions ui/pages/confirmations/hooks/useDecodedSignatureMetrics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { DecodingDataStateChange } from '@metamask/signature-controller';
import { useEffect } from 'react';

import { SignatureRequestType } from '../types/confirm';
import { useConfirmContext } from '../context/confirm';
import { useSignatureEventFragment } from './useSignatureEventFragment';


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

export function useDecodedSignatureMetrics() {
const { updateSignatureEventFragment } = useSignatureEventFragment();
const { currentConfirmation } = useConfirmContext<SignatureRequestType>();
const { decodingLoading, decodingData } = currentConfirmation;

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

const decodingResponse =
decodingData?.error?.type ??
(decodingChangeTypes.length
? DecodingResponseType.Change
: DecodingResponseType.NoChange);

useEffect(() => {
if (decodingLoading || !process.env.ENABLE_SIGNATURE_DECODING) {
return;
}

updateSignatureEventFragment({
properties: {
decoding_response: decodingResponse,
decoding_change_types: decodingChangeTypes,
},
});
}, [
decodingResponse,
decodingLoading,
decodingChangeTypes,
updateSignatureEventFragment,
]);
}
Loading