diff --git a/app/scripts/lib/createRPCMethodTrackingMiddleware.js b/app/scripts/lib/createRPCMethodTrackingMiddleware.js index 3e799cc4828f..40b619f4f847 100644 --- a/app/scripts/lib/createRPCMethodTrackingMiddleware.js +++ b/app/scripts/lib/createRPCMethodTrackingMiddleware.js @@ -18,7 +18,10 @@ import { PRIMARY_TYPES_PERMIT, } from '../../../shared/constants/signatures'; import { SIGNING_METHODS } from '../../../shared/constants/transaction'; -import { getBlockaidMetricsProps } from '../../../ui/helpers/utils/metrics'; +import { + generateSignatureUniqueId, + getBlockaidMetricsProps, +} from '../../../ui/helpers/utils/metrics'; import { REDESIGN_APPROVAL_TYPES } from '../../../ui/pages/confirmations/utils/confirm'; import { getSnapAndHardwareInfoForMetrics } from './snap-keyring/metrics'; @@ -123,14 +126,58 @@ const TRANSFORM_PARAMS_MAP = { const rateLimitTimeoutsByMethod = {}; let globalRateLimitCount = 0; +/** + * Create signature request event fragment with an assigned unique identifier + * + * @param {MetaMetricsController} metaMetricsController + * @param {OriginalRequest} req + * @param {object} properties + */ +function createSignatureFragment(metaMetricsController, req, properties) { + metaMetricsController.createEventFragment({ + category: MetaMetricsEventCategory.InpageProvider, + initialEvent: MetaMetricsEventName.SignatureRequested, + successEvent: MetaMetricsEventName.SignatureApproved, + failureEvent: MetaMetricsEventName.SignatureRejected, + uniqueIdentifier: generateSignatureUniqueId(req.id), + persist: true, + referrer: { + url: req.origin, + }, + properties, + }); +} + +/** + * Updates and finalizes event fragment for signature requests + * + * @param {MetaMetricsController} metaMetricsController + * @param {OriginalRequest} req + * @param {object} options + * @param {boolean} options.abandoned + * @param {object} options.properties + */ +function finalizeSignatureFragment( + metaMetricsController, + req, + { abandoned, properties }, +) { + const signatureUniqueId = generateSignatureUniqueId(req.id); + + metaMetricsController.updateEventFragment(signatureUniqueId, { + properties, + }); + metaMetricsController.finalizeEventFragment(signatureUniqueId, { + abandoned, + }); +} + /** * Returns a middleware that tracks inpage_provider usage using sampling for * each type of event except those that require user interaction, such as * signature requests * * @param {object} opts - options for the rpc method tracking middleware - * @param {Function} opts.trackEvent - trackEvent method from - * MetaMetricsController * @param {Function} opts.getMetricsState - get the state of * MetaMetricsController * @param {number} [opts.rateLimitTimeout] - time, in milliseconds, to wait before @@ -145,11 +192,12 @@ let globalRateLimitCount = 0; * time window that should limit the number of method calls tracked to globalRateLimitMaxAmount. * @param {number} [opts.globalRateLimitMaxAmount] - max number of method calls that should * tracked within the globalRateLimitTimeout time window. + * @param {AppStateController} [opts.appStateController] + * @param {MetaMetricsController} [opts.metaMetricsController] * @returns {Function} */ export default function createRPCMethodTrackingMiddleware({ - trackEvent, getMetricsState, rateLimitTimeout = 60 * 5 * 1000, // 5 minutes rateLimitSamplePercent = 0.001, // 0.1% @@ -160,6 +208,7 @@ export default function createRPCMethodTrackingMiddleware({ isConfirmationRedesignEnabled, snapAndHardwareMessenger, appStateController, + metaMetricsController, }) { return async function rpcMethodTrackingMiddleware( /** @type {any} */ req, @@ -216,6 +265,8 @@ export default function createRPCMethodTrackingMiddleware({ // Don't track if the user isn't participating in metametrics userParticipatingInMetaMetrics === true; + let signatureUniqueId; + if (shouldTrackEvent) { // We track an initial "requested" event as soon as the dapp calls the // provider method. For the events not special cased this is the only @@ -316,14 +367,18 @@ export default function createRPCMethodTrackingMiddleware({ eventProperties.params = transformParams(params); } - trackEvent({ - event, - category: MetaMetricsEventCategory.InpageProvider, - referrer: { - url: origin, - }, - properties: eventProperties, - }); + if (event === MetaMetricsEventName.SignatureRequested) { + createSignatureFragment(metaMetricsController, req, eventProperties); + } else { + metaMetricsController.trackEvent({ + event, + category: MetaMetricsEventCategory.InpageProvider, + referrer: { + url: origin, + }, + properties: eventProperties, + }); + } if (rateLimitType === RATE_LIMIT_TYPES.TIMEOUT) { rateLimitTimeoutsByMethod[method] = setTimeout(() => { @@ -372,22 +427,23 @@ export default function createRPCMethodTrackingMiddleware({ ...eventProperties, ...blockaidMetricProps, location, - // if security_alert_response from blockaidMetricProps is Benign, force set security_alert_reason to empty string - security_alert_reason: - blockaidMetricProps.security_alert_response === - BlockaidResultType.Benign - ? '' - : blockaidMetricProps.security_alert_reason, }; - trackEvent({ - event, - category: MetaMetricsEventCategory.InpageProvider, - referrer: { - url: origin, - }, - properties, - }); + if (signatureUniqueId) { + finalizeSignatureFragment(metaMetricsController, req, { + abandoned: event === eventType.REJECTED, + properties, + }); + } else { + metaMetricsController.trackEvent({ + event, + category: MetaMetricsEventCategory.InpageProvider, + referrer: { + url: origin, + }, + properties, + }); + } return callback(); }); diff --git a/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js b/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js index af3c1ee4768b..569aff50458a 100644 --- a/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js +++ b/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js @@ -1,6 +1,7 @@ import { errorCodes } from 'eth-rpc-errors'; import { detectSIWE } from '@metamask/controller-utils'; +import MetaMetricsController from '../controllers/metametrics'; import { MESSAGE_TYPE } from '../../../shared/constants/app'; import { MetaMetricsEventCategory, @@ -13,12 +14,25 @@ import { BlockaidResultType, } from '../../../shared/constants/security-provider'; import { permitSignatureMsg } from '../../../test/data/confirmations/typed_sign'; +import { createSegmentMock } from './segment'; import createRPCMethodTrackingMiddleware from './createRPCMethodTrackingMiddleware'; -const trackEvent = jest.fn(); +const MOCK_ID = '123'; +const expectedUniqueIdentifier = `signature-${MOCK_ID}`; + const metricsState = { participateInMetaMetrics: null }; const getMetricsState = () => metricsState; +const expectedMetametricsEventUndefinedProps = { + actionId: undefined, + currency: undefined, + environmentType: undefined, + page: undefined, + revenue: undefined, + sensitiveProperties: undefined, + value: undefined, +}; + const appStateController = { store: { getState: () => ({ @@ -37,15 +51,42 @@ const appStateController = { }, }; +const metaMetricsController = new MetaMetricsController({ + segment: createSegmentMock(2, 10000), + getCurrentChainId: () => '0x1338', + onNetworkDidChange: jest.fn(), + preferencesStore: { + subscribe: jest.fn(), + getState: jest.fn(() => ({ + currentLocale: 'en_US', + preferences: {}, + })), + }, + version: '0.0.1', + environment: 'test', + initState: { + participateInMetaMetrics: true, + metaMetricsId: '0xabc', + fragments: {}, + events: {}, + }, + extension: { + runtime: { + id: 'testid', + setUninstallURL: () => undefined, + }, + }, +}); + const createHandler = (opts) => createRPCMethodTrackingMiddleware({ - trackEvent, getMetricsState, rateLimitTimeout: 1000, rateLimitSamplePercent: 0.1, globalRateLimitTimeout: 0, globalRateLimitMaxAmount: 0, appStateController, + metaMetricsController, isConfirmationRedesignEnabled: () => false, ...opts, }); @@ -91,6 +132,13 @@ jest.mock('@metamask/controller-utils', () => { }); describe('createRPCMethodTrackingMiddleware', () => { + let trackEventSpy; + + beforeEach(() => { + trackEventSpy = jest + .spyOn(MetaMetricsController.prototype, 'trackEvent') + .mockImplementation(() => undefined); + }); afterEach(() => { jest.resetAllMocks(); metricsState.participateInMetaMetrics = null; @@ -99,6 +147,7 @@ describe('createRPCMethodTrackingMiddleware', () => { describe('before participateInMetaMetrics is set', () => { it('should not track an event for a signature request', async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.PERSONAL_SIGN, origin: 'some.dapp', }; @@ -110,7 +159,7 @@ describe('createRPCMethodTrackingMiddleware', () => { const handler = createHandler(); handler(req, res, next); await executeMiddlewareStack(); - expect(trackEvent).not.toHaveBeenCalled(); + expect(trackEventSpy).not.toHaveBeenCalled(); }); }); @@ -121,6 +170,7 @@ describe('createRPCMethodTrackingMiddleware', () => { it('should not track an event for a signature request', async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.PERSONAL_SIGN, origin: 'some.dapp', }; @@ -132,7 +182,7 @@ describe('createRPCMethodTrackingMiddleware', () => { const handler = createHandler(); handler(req, res, next); await executeMiddlewareStack(); - expect(trackEvent).not.toHaveBeenCalled(); + expect(trackEventSpy).not.toHaveBeenCalled(); }); }); @@ -143,6 +193,7 @@ describe('createRPCMethodTrackingMiddleware', () => { it(`should immediately track a ${MetaMetricsEventName.SignatureRequested} event`, async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.PERSONAL_SIGN, origin: 'some.dapp', securityAlertResponse: { @@ -158,8 +209,9 @@ describe('createRPCMethodTrackingMiddleware', () => { const { next } = getNext(); const handler = createHandler(); await handler(req, res, next); - expect(trackEvent).toHaveBeenCalledTimes(1); - expect(trackEvent.mock.calls[0][0]).toMatchObject({ + + expect(trackEventSpy).toHaveBeenCalledTimes(1); + expect(trackEventSpy.mock.calls[0][0]).toMatchObject({ category: MetaMetricsEventCategory.InpageProvider, event: MetaMetricsEventName.SignatureRequested, properties: { @@ -173,6 +225,7 @@ describe('createRPCMethodTrackingMiddleware', () => { it(`should track an event with correct blockaid parameters when providerRequestsCount is provided`, async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.ETH_SIGN_TYPED_DATA_V4, origin: 'some.dapp', securityAlertResponse: { @@ -192,15 +245,9 @@ describe('createRPCMethodTrackingMiddleware', () => { const { next } = getNext(); const handler = createHandler(); await handler(req, res, next); - expect(trackEvent).toHaveBeenCalledTimes(1); - /** - * TODO: - * toMatchObject matches even if the matched object does not contain some of the properties of the expected object - * I'm not sure why toMatchObject is used but we should probably check the other tests in this file for correctness in - * another PR. - * - */ - expect(trackEvent.mock.calls[0][0]).toStrictEqual({ + expect(trackEventSpy).toHaveBeenCalledTimes(1); + expect(trackEventSpy.mock.calls[0][0]).toStrictEqual({ + ...expectedMetametricsEventUndefinedProps, category: MetaMetricsEventCategory.InpageProvider, event: MetaMetricsEventName.SignatureRequested, properties: { @@ -211,11 +258,13 @@ describe('createRPCMethodTrackingMiddleware', () => { ppom_eth_getCode_count: 3, }, referrer: { url: 'some.dapp' }, + uniqueIdentifier: expectedUniqueIdentifier, }); }); it(`should track a ${MetaMetricsEventName.SignatureApproved} event if the user approves`, async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.ETH_SIGN_TYPED_DATA_V4, origin: 'some.dapp', }; @@ -227,8 +276,8 @@ describe('createRPCMethodTrackingMiddleware', () => { const handler = createHandler(); await handler(req, res, next); await executeMiddlewareStack(); - expect(trackEvent).toHaveBeenCalledTimes(2); - expect(trackEvent.mock.calls[1][0]).toMatchObject({ + expect(trackEventSpy).toHaveBeenCalledTimes(2); + expect(trackEventSpy.mock.calls[1][0]).toMatchObject({ category: MetaMetricsEventCategory.InpageProvider, event: MetaMetricsEventName.SignatureApproved, properties: { @@ -240,6 +289,7 @@ describe('createRPCMethodTrackingMiddleware', () => { it(`should track a ${MetaMetricsEventName.SignatureRejected} event if the user approves`, async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.PERSONAL_SIGN, origin: 'some.dapp', }; @@ -254,8 +304,8 @@ describe('createRPCMethodTrackingMiddleware', () => { const handler = createHandler(); await handler(req, res, next); await executeMiddlewareStack(); - expect(trackEvent).toHaveBeenCalledTimes(2); - expect(trackEvent.mock.calls[1][0]).toMatchObject({ + expect(trackEventSpy).toHaveBeenCalledTimes(2); + expect(trackEventSpy.mock.calls[1][0]).toMatchObject({ category: MetaMetricsEventCategory.InpageProvider, event: MetaMetricsEventName.SignatureRejected, properties: { @@ -268,6 +318,7 @@ describe('createRPCMethodTrackingMiddleware', () => { it(`should track a ${MetaMetricsEventName.PermissionsApproved} event if the user approves`, async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.ETH_REQUEST_ACCOUNTS, origin: 'some.dapp', }; @@ -277,8 +328,8 @@ describe('createRPCMethodTrackingMiddleware', () => { const handler = createHandler(); await handler(req, res, next); await executeMiddlewareStack(); - expect(trackEvent).toHaveBeenCalledTimes(2); - expect(trackEvent.mock.calls[1][0]).toMatchObject({ + expect(trackEventSpy).toHaveBeenCalledTimes(2); + expect(trackEventSpy.mock.calls[1][0]).toMatchObject({ category: MetaMetricsEventCategory.InpageProvider, event: MetaMetricsEventName.PermissionsApproved, properties: { method: MESSAGE_TYPE.ETH_REQUEST_ACCOUNTS }, @@ -288,6 +339,7 @@ describe('createRPCMethodTrackingMiddleware', () => { it(`should never track blocked methods such as ${MESSAGE_TYPE.GET_PROVIDER_STATE}`, () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.GET_PROVIDER_STATE, origin: 'www.notadapp.com', }; @@ -298,7 +350,7 @@ describe('createRPCMethodTrackingMiddleware', () => { const { next, executeMiddlewareStack } = getNext(); const handler = createHandler(); handler(req, res, next); - expect(trackEvent).not.toHaveBeenCalled(); + expect(trackEventSpy).not.toHaveBeenCalled(); executeMiddlewareStack(); }); @@ -310,6 +362,7 @@ describe('createRPCMethodTrackingMiddleware', () => { `should only track '%s' events while the timeout rate limit is not active`, async (method, eventsTrackedPerRequest) => { const req = { + id: MOCK_ID, method, origin: 'some.dapp', }; @@ -332,8 +385,8 @@ describe('createRPCMethodTrackingMiddleware', () => { } const expectedNumberOfCalls = 2 * eventsTrackedPerRequest; - expect(trackEvent).toHaveBeenCalledTimes(expectedNumberOfCalls); - trackEvent.mock.calls.forEach((call) => { + expect(trackEventSpy).toHaveBeenCalledTimes(expectedNumberOfCalls); + trackEventSpy.mock.calls.forEach((call) => { expect(call[0].properties.method).toBe(method); }); }, @@ -360,6 +413,7 @@ describe('createRPCMethodTrackingMiddleware', () => { `should only track a random percentage of '%s' events`, async (method, eventsTrackedPerRequest) => { const req = { + id: MOCK_ID, method, origin: 'some.dapp', }; @@ -379,8 +433,8 @@ describe('createRPCMethodTrackingMiddleware', () => { } const expectedNumberOfCalls = 2 * eventsTrackedPerRequest; - expect(trackEvent).toHaveBeenCalledTimes(expectedNumberOfCalls); - trackEvent.mock.calls.forEach((call) => { + expect(trackEventSpy).toHaveBeenCalledTimes(expectedNumberOfCalls); + trackEventSpy.mock.calls.forEach((call) => { expect(call[0].properties.method).toBe(method); }); }, @@ -390,6 +444,7 @@ describe('createRPCMethodTrackingMiddleware', () => { describe('events rated globally rate limited', () => { it('should only track events if the global rate limit has not been hit', async () => { const req = { + id: MOCK_ID, method: 'some_method_rate_limited_by_sample', origin: 'some.dapp', }; @@ -415,8 +470,8 @@ describe('createRPCMethodTrackingMiddleware', () => { } } - expect(trackEvent).toHaveBeenCalledTimes(3); - trackEvent.mock.calls.forEach((call) => { + expect(trackEventSpy).toHaveBeenCalledTimes(3); + trackEventSpy.mock.calls.forEach((call) => { expect(call[0].properties.method).toBe( 'some_method_rate_limited_by_sample', ); @@ -426,6 +481,7 @@ describe('createRPCMethodTrackingMiddleware', () => { it('should track Confirmation Redesign through ui_customizations prop if enabled', async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.PERSONAL_SIGN, origin: 'some.dapp', }; @@ -440,9 +496,9 @@ describe('createRPCMethodTrackingMiddleware', () => { await handler(req, res, next); await executeMiddlewareStack(); - expect(trackEvent).toHaveBeenCalledTimes(2); + expect(trackEventSpy).toHaveBeenCalledTimes(2); - expect(trackEvent.mock.calls[1][0]).toMatchObject({ + expect(trackEventSpy.mock.calls[1][0]).toMatchObject({ category: MetaMetricsEventCategory.InpageProvider, event: MetaMetricsEventName.SignatureApproved, properties: { @@ -457,6 +513,7 @@ describe('createRPCMethodTrackingMiddleware', () => { it('should not track Confirmation Redesign through ui_customizations prop if not enabled', async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.PERSONAL_SIGN, origin: 'some.dapp', }; @@ -469,9 +526,9 @@ describe('createRPCMethodTrackingMiddleware', () => { await handler(req, res, next); await executeMiddlewareStack(); - expect(trackEvent).toHaveBeenCalledTimes(2); + expect(trackEventSpy).toHaveBeenCalledTimes(2); - expect(trackEvent.mock.calls[1][0]).toMatchObject({ + expect(trackEventSpy.mock.calls[1][0]).toMatchObject({ category: MetaMetricsEventCategory.InpageProvider, event: MetaMetricsEventName.SignatureApproved, properties: { @@ -483,6 +540,7 @@ describe('createRPCMethodTrackingMiddleware', () => { it('should track Sign-in With Ethereum (SIWE) message if detected', async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.PERSONAL_SIGN, origin: 'some.dapp', }; @@ -499,9 +557,9 @@ describe('createRPCMethodTrackingMiddleware', () => { await handler(req, res, next); await executeMiddlewareStack(); - expect(trackEvent).toHaveBeenCalledTimes(2); + expect(trackEventSpy).toHaveBeenCalledTimes(2); - expect(trackEvent.mock.calls[1][0]).toMatchObject({ + expect(trackEventSpy.mock.calls[1][0]).toMatchObject({ category: MetaMetricsEventCategory.InpageProvider, event: MetaMetricsEventName.SignatureApproved, properties: { @@ -514,6 +572,7 @@ describe('createRPCMethodTrackingMiddleware', () => { it('should track typed-sign permit message if detected', async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.ETH_SIGN_TYPED_DATA_V4, origin: 'some.dapp', params: [undefined, permitSignatureMsg.msgParams.data], @@ -527,9 +586,9 @@ describe('createRPCMethodTrackingMiddleware', () => { await handler(req, res, next); await executeMiddlewareStack(); - expect(trackEvent).toHaveBeenCalledTimes(2); + expect(trackEventSpy).toHaveBeenCalledTimes(2); - expect(trackEvent.mock.calls[1][0]).toMatchObject({ + expect(trackEventSpy.mock.calls[1][0]).toMatchObject({ category: MetaMetricsEventCategory.InpageProvider, event: MetaMetricsEventName.SignatureApproved, properties: { @@ -544,6 +603,7 @@ describe('createRPCMethodTrackingMiddleware', () => { describe('when request is flagged as safe by security provider', () => { it(`should immediately track a ${MetaMetricsEventName.SignatureRequested} event`, async () => { const req = { + id: MOCK_ID, method: MESSAGE_TYPE.ETH_SIGN_TYPED_DATA_V4, origin: 'some.dapp', }; @@ -555,8 +615,8 @@ describe('createRPCMethodTrackingMiddleware', () => { await handler(req, res, next); - expect(trackEvent).toHaveBeenCalledTimes(1); - expect(trackEvent.mock.calls[0][0]).toMatchObject({ + expect(trackEventSpy).toHaveBeenCalledTimes(1); + expect(trackEventSpy.mock.calls[0][0]).toMatchObject({ category: MetaMetricsEventCategory.InpageProvider, event: MetaMetricsEventName.SignatureRequested, properties: { @@ -617,6 +677,7 @@ describe('createRPCMethodTrackingMiddleware', () => { `should include %s in the '%s' tracked events params property`, async (_, method, params, expected) => { const req = { + id: MOCK_ID, method, origin: 'some.dapp', params, @@ -631,8 +692,8 @@ describe('createRPCMethodTrackingMiddleware', () => { await handler(req, res, next); - expect(trackEvent).toHaveBeenCalledTimes(1); - expect(trackEvent.mock.calls[0][0].properties.params).toStrictEqual( + expect(trackEventSpy).toHaveBeenCalledTimes(1); + expect(trackEventSpy.mock.calls[0][0].properties.params).toStrictEqual( expected, ); }, diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 502b816387ae..c48fcace822a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5456,9 +5456,6 @@ export default class MetamaskController extends EventEmitter { engine.push( createRPCMethodTrackingMiddleware({ - trackEvent: this.metaMetricsController.trackEvent.bind( - this.metaMetricsController, - ), getMetricsState: this.metaMetricsController.store.getState.bind( this.metaMetricsController.store, ), @@ -5474,6 +5471,7 @@ export default class MetamaskController extends EventEmitter { ], }), appStateController: this.appStateController, + metaMetricsController: this.metaMetricsController, }), ); diff --git a/shared/constants/metametrics.ts b/shared/constants/metametrics.ts index bea343ea7c41..be5adda68c08 100644 --- a/shared/constants/metametrics.ts +++ b/shared/constants/metametrics.ts @@ -156,7 +156,7 @@ export type MetaMetricsEventFragment = { /** * The event name to fire when the fragment is closed in an affirmative action. */ - successEvent: string; + successEvent?: string; /** * The event name to fire when the fragment is closed with a rejection. */ @@ -169,7 +169,7 @@ export type MetaMetricsEventFragment = { /** * The event category to use for both the success and failure events. */ - category: string; + category?: string; /** * Should this fragment be persisted in state and progressed after the * extension is locked and unlocked. diff --git a/test/e2e/tests/confirmations/signatures/malicious-signatures.spec.ts b/test/e2e/tests/confirmations/signatures/malicious-signatures.spec.ts index 290af3a147ad..7e4568d326f0 100644 --- a/test/e2e/tests/confirmations/signatures/malicious-signatures.spec.ts +++ b/test/e2e/tests/confirmations/signatures/malicious-signatures.spec.ts @@ -68,6 +68,16 @@ describe('Malicious Confirmation Signature - Bad Domain @no-mmi', function (this 'sign_in_with_ethereum', ], location: 'confirmation', + expectedProps: { + alert_action_clicked: [], + alert_key_clicked: [], + alert_resolved: [], + alert_resolved_count: 0, + alert_triggered: ['requestFrom'], + alert_triggered_count: 1, + alert_visualized: [], + alert_visualized_count: 0, + }, }); }, mockSignatureRejected, @@ -105,6 +115,16 @@ describe('Malicious Confirmation Signature - Bad Domain @no-mmi', function (this 'sign_in_with_ethereum', ], location: 'alert_friction_modal', + expectedProps: { + alert_action_clicked: [], + alert_key_clicked: [], + alert_resolved: [], + alert_resolved_count: 0, + alert_triggered: ['requestFrom'], + alert_triggered_count: 1, + alert_visualized: ['requestFrom'], + alert_visualized_count: 1, + }, }); }, mockSignatureRejected, diff --git a/test/e2e/tests/confirmations/signatures/signature-helpers.ts b/test/e2e/tests/confirmations/signatures/signature-helpers.ts index 041d1bfbf23a..6bb8f4ee0b75 100644 --- a/test/e2e/tests/confirmations/signatures/signature-helpers.ts +++ b/test/e2e/tests/confirmations/signatures/signature-helpers.ts @@ -27,6 +27,7 @@ type AssertSignatureMetricsOptions = { primaryType?: string; uiCustomizations?: string[]; location?: string; + expectedProps?: Record; }; type SignatureEventProperty = { @@ -35,6 +36,7 @@ type SignatureEventProperty = { chain_id: '0x539'; environment_type: 'background'; locale: 'en'; + security_alert_reason: string; security_alert_response: 'NotApplicable'; signature_type: string; eip712_primary_type?: string; @@ -42,6 +44,13 @@ type SignatureEventProperty = { location?: string; }; +/** + * Generates expected signature metric properties + * + * @param signatureType + * @param primaryType + * @param uiCustomizations + */ function getSignatureEventProperty( signatureType: string, primaryType: string, @@ -54,6 +63,7 @@ function getSignatureEventProperty( chain_id: '0x539', environment_type: 'background', locale: 'en', + security_alert_reason: 'NotApplicable', security_alert_response: 'NotApplicable', ui_customizations: uiCustomizations, }; @@ -111,6 +121,7 @@ export async function assertSignatureRejectedMetrics({ primaryType = '', uiCustomizations = ['redesigned_confirmation'], location, + expectedProps = {}, }: AssertSignatureMetricsOptions) { const events = await getEventPayloads(driver, mockedEndpoints); const signatureEventProperty = getSignatureEventProperty( @@ -126,6 +137,7 @@ export async function assertSignatureRejectedMetrics({ { ...signatureEventProperty, location, + ...expectedProps, }, 'Signature Rejected event properties do not match', ); diff --git a/test/e2e/tests/metrics/signature-approved.spec.js b/test/e2e/tests/metrics/signature-approved.spec.js index c629955b5db0..c6990820af8b 100644 --- a/test/e2e/tests/metrics/signature-approved.spec.js +++ b/test/e2e/tests/metrics/signature-approved.spec.js @@ -95,6 +95,7 @@ describe('Signature Approved Event @no-mmi', function () { chain_id: '0x539', eip712_primary_type: 'Mail', environment_type: 'background', + security_alert_reason: 'NotApplicable', security_alert_response: 'NotApplicable', }); }, @@ -143,6 +144,7 @@ describe('Signature Approved Event @no-mmi', function () { locale: 'en', chain_id: '0x539', environment_type: 'background', + security_alert_reason: 'NotApplicable', security_alert_response: 'NotApplicable', }); }, @@ -190,6 +192,7 @@ describe('Signature Approved Event @no-mmi', function () { locale: 'en', chain_id: '0x539', environment_type: 'background', + security_alert_reason: 'NotApplicable', security_alert_response: 'NotApplicable', }); }, @@ -237,6 +240,7 @@ describe('Signature Approved Event @no-mmi', function () { locale: 'en', chain_id: '0x539', environment_type: 'background', + security_alert_reason: 'NotApplicable', security_alert_response: 'NotApplicable', }); }, diff --git a/ui/helpers/utils/metrics.js b/ui/helpers/utils/metrics.js index 32d7d3c18c42..ebf3838c4230 100644 --- a/ui/helpers/utils/metrics.js +++ b/ui/helpers/utils/metrics.js @@ -25,6 +25,16 @@ export function formatAccountType(accountType) { return accountType; } +/** + * Generates a unique identifier utilizing the original request id for signature event fragments + * + * @param {number} requestId + * @returns {string} + */ +export function generateSignatureUniqueId(requestId) { + return `signature-${requestId}`; +} + /** * Returns the ui_customization string value based on the result type * diff --git a/ui/pages/confirmations/hooks/useConfirmationAlertMetrics.ts b/ui/pages/confirmations/hooks/useConfirmationAlertMetrics.ts index ef5ba9d63838..9bb3238b0232 100644 --- a/ui/pages/confirmations/hooks/useConfirmationAlertMetrics.ts +++ b/ui/pages/confirmations/hooks/useConfirmationAlertMetrics.ts @@ -1,11 +1,13 @@ -import { TransactionType } from '@metamask/transaction-controller'; import { useCallback, useEffect, useState } from 'react'; import { validate as isUuid } from 'uuid'; import useAlerts from '../../../hooks/useAlerts'; -import { REDESIGN_DEV_TRANSACTION_TYPES } from '../utils'; +import { updateEventFragment } from '../../../store/actions'; +import { SignatureRequestType } from '../types/confirm'; +import { isSignatureTransactionType } from '../utils'; import { Alert } from '../../../ducks/confirm-alerts/confirm-alerts'; import { useConfirmContext } from '../context/confirm'; +import { generateSignatureUniqueId } from '../../../helpers/utils/metrics'; import { AlertsName } from './alerts/constants'; import { useTransactionEventFragment } from './useTransactionEventFragment'; @@ -56,13 +58,8 @@ export function useConfirmationAlertMetrics() { alert_action_clicked: [], }); - // Temporary measure to track metrics only for redesign transaction types - const isValidType = REDESIGN_DEV_TRANSACTION_TYPES.includes( - currentConfirmation?.type as TransactionType, - ); - const properties = - isValidType && alerts.length > 0 + alerts.length > 0 ? { alert_triggered_count: alerts.length, alert_triggered: getAlertNames(alerts), @@ -117,7 +114,17 @@ export function useConfirmationAlertMetrics() { if (!properties) { return; } - updateTransactionEventFragment({ properties }, ownerId); + + if (isSignatureTransactionType(currentConfirmation)) { + const requestId = (currentConfirmation as SignatureRequestType).msgParams + ?.requestId as number; + const fragmentUniqueId = generateSignatureUniqueId(requestId); + updateEventFragment(fragmentUniqueId, { + properties, + }); + } else { + updateTransactionEventFragment({ properties }, ownerId); + } }, [JSON.stringify(properties), updateTransactionEventFragment, ownerId]); useEffect(() => { diff --git a/ui/pages/confirmations/types/confirm.ts b/ui/pages/confirmations/types/confirm.ts index da4981c10216..41c0c125b001 100644 --- a/ui/pages/confirmations/types/confirm.ts +++ b/ui/pages/confirmations/types/confirm.ts @@ -31,6 +31,7 @@ export type SignatureRequestType = { origin: string; data: string | TypedSignDataV1Type; version?: string; + requestId?: number; signatureMethod?: string; siwe?: SIWEMessage; };