Skip to content

Commit b7f37c0

Browse files
authored
feat: track snap account metrics (#14686)
## **Description** This PR adds metric tracking for basic snap account actions. This brings the metric tracking more inline with the events we already track on the extension. The main events that we are tracking are the following... 1. Snap account creation - [Equivalent extension event](https://github.com/MetaMask/metamask-extension/blob/5d953bf41b950eeb8358a735729c22d1e45386f6/app/scripts/lib/snap-keyring/snap-keyring.ts#L362) - [Extension event definition](https://github.com/MetaMask/metamask-extension/blob/478b45f3ffa7619378e43f9367e62edff6b5b810/shared/constants/metametrics.ts#L645) 2. Snap account removal - [Equivalent extension event ](https://github.com/MetaMask/metamask-extension/blob/5d953bf41b950eeb8358a735729c22d1e45386f6/app/scripts/lib/snap-keyring/snap-keyring.ts#L547) - [Extension event definition](https://github.com/MetaMask/metamask-extension/blob/478b45f3ffa7619378e43f9367e62edff6b5b810/shared/constants/metametrics.ts#L826) 3. Failed attempts at snap account removal - [Equivalent extension event](https://github.com/MetaMask/metamask-extension/blob/5d953bf41b950eeb8358a735729c22d1e45386f6/app/scripts/lib/snap-keyring/snap-keyring.ts#L532) - [Extension event definition](https://github.com/MetaMask/metamask-extension/blob/478b45f3ffa7619378e43f9367e62edff6b5b810/shared/constants/metametrics.ts#L827) On mobile we do not have snap account confirmations so we skipped all of those events. ## **Related issues** Fixes: MetaMask/accounts-planning#695 ## **Manual testing steps** 1. ensure you are building metamask flask by editing the `METAMASK_BUILD_TYPE` to `flask` in the `.js.env` 2. `yarn setup && yarn start:ios` 3. import/create an account 4. click on the selected account at the top of the screen 5. click `add new account or hardware wallet` 6. click on `solana` 7. the solana account should be created 8. repeat the above steps and then click on `Bitcoin` instead 9. the Bitcoin account should be created. ## **Screenshots/Recordings** ### **Before** <!-- [screenshots/recordings] --> ### **After** I added a log to the trackSnapAccountEvent to verify that it is called when we create a new solana account https://github.com/user-attachments/assets/328b95c3-0b8b-4a62-b712-4303bf1fde27 ## **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.
1 parent 3a22704 commit b7f37c0

File tree

7 files changed

+437
-5
lines changed

7 files changed

+437
-5
lines changed

app/core/Analytics/MetaMetrics.events.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,9 @@ enum EVENT_NAME {
381381

382382
// Remove an account
383383
ACCOUNT_REMOVED = 'Account removed',
384+
ACCOUNT_REMOVE_FAILED = 'Account remove failed',
385+
// Account added
386+
ACCOUNT_ADDED = 'Account Added',
384387

385388
//Notifications
386389
ALL_NOTIFICATIONS = 'All Notifications',
@@ -493,11 +496,11 @@ const events = {
493496
EVENT_NAME.CONNECT_REQUEST_OTPFAILURE,
494497
),
495498
CONNECT_REQUEST_CANCELLED: generateOpt(EVENT_NAME.CONNECT_REQUEST_CANCELLED),
496-
499+
497500
// Phishing events
498501
PHISHING_PAGE_DISPLAYED: generateOpt(EVENT_NAME.PHISHING_PAGE_DISPLAYED),
499502
PROCEED_ANYWAY_CLICKED: generateOpt(EVENT_NAME.PROCEED_ANYWAY_CLICKED),
500-
503+
501504
WALLET_OPENED: generateOpt(EVENT_NAME.WALLET_OPENED),
502505
TOKEN_ADDED: generateOpt(EVENT_NAME.TOKEN_ADDED),
503506
COLLECTIBLE_ADDED: generateOpt(EVENT_NAME.COLLECTIBLE_ADDED),
@@ -863,6 +866,8 @@ const events = {
863866

864867
// Remove an account
865868
ACCOUNT_REMOVED: generateOpt(EVENT_NAME.ACCOUNT_REMOVED),
869+
ACCOUNT_REMOVE_FAILED: generateOpt(EVENT_NAME.ACCOUNT_REMOVE_FAILED),
870+
ACCOUNT_ADDED: generateOpt(EVENT_NAME.ACCOUNT_ADDED),
866871

867872
// Smart transactions
868873
SMART_TRANSACTION_OPT_IN: generateOpt(EVENT_NAME.SMART_TRANSACTION_OPT_IN),
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { IMetaMetricsEvent, MetaMetrics } from '../..';
2+
import { trackSnapAccountEvent } from './trackSnapAccountEvent';
3+
import { MetricsEventBuilder } from '../../MetricsEventBuilder';
4+
import { IMetaMetrics } from '../../MetaMetrics.types';
5+
6+
const mockTrackEvent = jest.fn();
7+
const mockAddProperties = jest.fn().mockReturnThis();
8+
const mockBuild = jest.fn().mockReturnValue({ mockBuiltEvent: true });
9+
const mockCreateEventBuilder = jest.fn().mockReturnValue({
10+
addProperties: mockAddProperties,
11+
build: mockBuild,
12+
});
13+
14+
jest
15+
.spyOn(MetricsEventBuilder, 'createEventBuilder')
16+
.mockImplementation(mockCreateEventBuilder);
17+
18+
jest.spyOn(MetaMetrics, 'getInstance').mockReturnValue({
19+
trackEvent: mockTrackEvent,
20+
isEnabled: jest.fn(),
21+
enable: jest.fn(),
22+
addTraitsToUser: jest.fn(),
23+
group: jest.fn(),
24+
reset: jest.fn(),
25+
flush: jest.fn(),
26+
createDataDeletionTask: jest.fn(),
27+
checkDataDeleteStatus: jest.fn(),
28+
getDeleteRegulationCreationDate: jest.fn(),
29+
getDeleteRegulationId: jest.fn(),
30+
isDataRecorded: jest.fn(),
31+
configure: jest.fn(),
32+
getMetaMetricsId: jest.fn(),
33+
} as IMetaMetrics);
34+
35+
describe('trackSnapAccountEvent', () => {
36+
const mockMetricEvent: IMetaMetricsEvent = {
37+
category: 'testCategory',
38+
properties: { name: 'testEvent' },
39+
};
40+
const mockSnapId = 'npm:@metamask/test-snap';
41+
const mockSnapName = 'Test Snap';
42+
43+
beforeEach(() => {
44+
jest.clearAllMocks();
45+
});
46+
47+
it('should create and track an event with the correct properties', () => {
48+
trackSnapAccountEvent(mockMetricEvent, mockSnapId, mockSnapName);
49+
expect(mockCreateEventBuilder).toHaveBeenCalledWith(mockMetricEvent);
50+
expect(mockAddProperties).toHaveBeenCalledWith({
51+
account_type: 'Snap',
52+
snap_id: mockSnapId,
53+
snap_name: mockSnapName,
54+
});
55+
56+
expect(mockBuild).toHaveBeenCalled();
57+
// Verify MetaMetrics trackEvent was called with the built event
58+
expect(mockTrackEvent).toHaveBeenCalledWith({ mockBuiltEvent: true });
59+
});
60+
});
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { IMetaMetricsEvent, MetaMetrics } from '../..';
2+
import Logger from '../../../../util/Logger';
3+
import { MetricsEventBuilder } from '../../MetricsEventBuilder';
4+
5+
/**
6+
* Track a Snap account-related event.
7+
*
8+
* @param metricEvent - The name of the event to track.
9+
* @param snapId - The ID of the Snap.
10+
* @param snapName - The name of the Snap.
11+
*/
12+
export function trackSnapAccountEvent(
13+
metricEvent: IMetaMetricsEvent,
14+
snapId: string,
15+
snapName: string,
16+
): void {
17+
try {
18+
const event = MetricsEventBuilder.createEventBuilder(metricEvent)
19+
.addProperties({
20+
account_type: 'Snap',
21+
snap_id: snapId,
22+
snap_name: snapName,
23+
})
24+
.build();
25+
26+
MetaMetrics.getInstance().trackEvent(event);
27+
} catch (error) {
28+
Logger.error(
29+
error as Error,
30+
`Error tracking snap account event: ${JSON.stringify(metricEvent)}`,
31+
);
32+
}
33+
}

app/core/SnapKeyring/SnapKeyring.test.ts

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES } from '../RPCMethods/RPCMethod
1616
import { showAccountNameSuggestionDialog } from './utils/showDialog';
1717
import Logger from '../../util/Logger';
1818
import { isSnapPreinstalled } from './utils/snaps';
19+
import { trackSnapAccountEvent } from '../Analytics/helpers/SnapKeyring/trackSnapAccountEvent';
1920

2021
const mockAddRequest = jest.fn();
2122
const mockStartFlow = jest.fn();
@@ -144,6 +145,12 @@ const createSnapKeyringBuilder = () =>
144145
// Mock the isSnapPreinstalled function
145146
jest.mock('./utils/snaps', () => ({
146147
isSnapPreinstalled: jest.fn(),
148+
getSnapName: jest.fn().mockReturnValue('Mock Snap Name'),
149+
}));
150+
151+
// Mock the trackSnapAccountEvent function
152+
jest.mock('../Analytics/helpers/SnapKeyring/trackSnapAccountEvent', () => ({
153+
trackSnapAccountEvent: jest.fn(),
147154
}));
148155

149156
describe('Snap Keyring Methods', () => {
@@ -212,6 +219,12 @@ describe('Snap Keyring Methods', () => {
212219
expect(mockGetAccounts).toHaveBeenCalledTimes(1);
213220
expect(mockSetAccountName).not.toHaveBeenCalled();
214221
expect(mockEndFlow).toHaveBeenCalledWith([{ id: mockFlowId }]);
222+
223+
// Wait for any pending promises (including the account finalization which tracks the event)
224+
await waitForAllPromises();
225+
226+
// Verify trackSnapAccountEvent was called for successful account creation
227+
expect(trackSnapAccountEvent).toHaveBeenCalled();
215228
});
216229

217230
it('handles account creation with user defined name', async () => {
@@ -250,6 +263,12 @@ describe('Snap Keyring Methods', () => {
250263
]);
251264
expect(mockEndFlow).toHaveBeenCalledTimes(2);
252265
expect(mockEndFlow).toHaveBeenCalledWith([{ id: mockFlowId }]);
266+
267+
// Wait for any pending promises (including the account finalization which tracks the event)
268+
await waitForAllPromises();
269+
270+
// Verify trackSnapAccountEvent was called
271+
expect(trackSnapAccountEvent).toHaveBeenCalled();
253272
});
254273

255274
it('throws an error when user denies account creation', async () => {
@@ -342,6 +361,7 @@ describe('Snap Keyring Methods', () => {
342361
expect(mockEndFlow).toHaveBeenCalledTimes(2);
343362
expect(mockEndFlow).toHaveBeenNthCalledWith(1, [{ id: mockFlowId }]);
344363
expect(mockEndFlow).toHaveBeenNthCalledWith(2, [{ id: mockFlowId }]);
364+
expect(trackSnapAccountEvent).not.toHaveBeenCalled();
345365
});
346366
it('skips account name suggestion dialog for preinstalled snaps when displayAccountNameSuggestion is false', async () => {
347367
// Mock isSnapPreinstalled to return true for this test
@@ -467,4 +487,95 @@ describe('Snap Keyring Methods', () => {
467487
]);
468488
});
469489
});
490+
describe('removeAccount', () => {
491+
beforeEach(() => {
492+
mockAddRequest.mockReturnValue(true).mockReturnValue({ success: true });
493+
(isSnapPreinstalled as jest.Mock).mockReset();
494+
});
495+
afterEach(() => {
496+
jest.resetAllMocks();
497+
});
498+
499+
it('calls removeAccountHelper and persistKeyringHelper when account is deleted', async () => {
500+
const builder = createSnapKeyringBuilder();
501+
const snapKeyring = builder();
502+
503+
// First add an account to the keyring so that it can be removed
504+
// NOTE: This callback will not be triggered if there are no accounts in the keyring
505+
await snapKeyring.handleKeyringSnapMessage(mockSnapId, {
506+
method: KeyringEvent.AccountCreated,
507+
params: {
508+
account: mockAccount,
509+
displayConfirmation: false,
510+
},
511+
});
512+
513+
// Reset mocks after account creation
514+
mockRemoveAccountHelper.mockReset();
515+
mockPersisKeyringHelper.mockReset();
516+
517+
// Now delete the account
518+
await snapKeyring.handleKeyringSnapMessage(mockSnapId, {
519+
method: KeyringEvent.AccountDeleted,
520+
params: {
521+
id: mockAccount.id,
522+
},
523+
});
524+
525+
expect(mockRemoveAccountHelper).toHaveBeenCalledTimes(1);
526+
expect(mockRemoveAccountHelper).toHaveBeenCalledWith(
527+
mockAccount.address.toLowerCase(),
528+
);
529+
expect(mockPersisKeyringHelper).toHaveBeenCalledTimes(2);
530+
531+
// Verify trackSnapAccountEvent was called
532+
expect(trackSnapAccountEvent).toHaveBeenCalled();
533+
});
534+
535+
it('handles errors when removing an account', async () => {
536+
const loggerSpy = jest.spyOn(Logger, 'error').mockImplementation();
537+
538+
// Set up mock to throw an error
539+
const errorMessage = 'Failed to remove account';
540+
mockRemoveAccountHelper.mockRejectedValue(new Error(errorMessage));
541+
542+
const builder = createSnapKeyringBuilder();
543+
const snapKeyring = builder();
544+
545+
// First add an account to the keyring so that it can be removed
546+
await snapKeyring.handleKeyringSnapMessage(mockSnapId, {
547+
method: KeyringEvent.AccountCreated,
548+
params: {
549+
account: mockAccount,
550+
displayConfirmation: false,
551+
},
552+
});
553+
554+
// Reset mocks after account creation
555+
mockRemoveAccountHelper.mockReset();
556+
mockPersisKeyringHelper.mockReset();
557+
mockRemoveAccountHelper.mockRejectedValue(new Error(errorMessage));
558+
559+
// Expect the error to be thrown
560+
await expect(
561+
snapKeyring.handleKeyringSnapMessage(mockSnapId, {
562+
method: KeyringEvent.AccountDeleted,
563+
params: {
564+
id: mockAccount.id,
565+
},
566+
}),
567+
).rejects.toThrow(errorMessage);
568+
569+
// Verify error was logged
570+
expect(loggerSpy).toHaveBeenCalledWith(
571+
expect.objectContaining({ message: errorMessage }),
572+
expect.stringContaining(
573+
`Error removing snap account: ${mockAccount.address.toLowerCase()}`,
574+
),
575+
);
576+
577+
// Verify trackSnapAccountEvent was called for error case
578+
expect(trackSnapAccountEvent).toHaveBeenCalled();
579+
});
580+
});
470581
});

app/core/SnapKeyring/SnapKeyring.ts

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ import { SnapKeyringBuilderMessenger } from './types';
1010
import { SnapId } from '@metamask/snaps-sdk';
1111
import { assertIsValidSnapId } from '@metamask/snaps-utils';
1212
import { getUniqueAccountName } from './utils/getUniqueAccountName';
13-
import { isSnapPreinstalled } from './utils/snaps';
13+
import { getSnapName, isSnapPreinstalled } from './utils/snaps';
1414
import { endTrace, trace, TraceName, TraceOperation } from '../../util/trace';
1515
import { getTraceTags } from '../../util/sentry/tags';
1616
import { store } from '../../store';
17+
import { MetaMetricsEvents } from '../../core/Analytics/MetaMetrics.events';
18+
import { trackSnapAccountEvent } from '../Analytics/helpers/SnapKeyring/trackSnapAccountEvent';
1719

1820
/**
1921
* Builder type for the Snap keyring.
@@ -140,6 +142,8 @@ class SnapKeyringImpl implements SnapKeyringCallbacks {
140142
}
141143

142144
private async addAccountFinalize({
145+
address: _address,
146+
snapId,
143147
accountName,
144148
onceSaved,
145149
}: {
@@ -176,6 +180,15 @@ class SnapKeyringImpl implements SnapKeyringCallbacks {
176180
accountName,
177181
);
178182
}
183+
184+
// Track successful account addition
185+
const snapName = getSnapName(snapId as SnapId, this.#messenger);
186+
trackSnapAccountEvent(
187+
MetaMetricsEvents.ACCOUNT_ADDED,
188+
snapId,
189+
snapName,
190+
);
191+
179192
endTrace({
180193
name: TraceName.AddSnapAccount,
181194
});
@@ -231,17 +244,39 @@ class SnapKeyringImpl implements SnapKeyringCallbacks {
231244
handleUserInput: (accepted: boolean) => Promise<void>,
232245
) {
233246
assertIsValidSnapId(snapId);
247+
234248
// TODO: Implement proper snap account confirmations. Currently, we are approving everything for testing purposes.
235249
Logger.log(
236250
`SnapKeyring: removeAccount called with \n
237251
- address: ${address} \n
238252
- handleUserInput: ${handleUserInput} \n
239253
- snapId: ${snapId} \n`,
240254
);
255+
241256
// Approve everything for now because we have not implemented snap account confirmations yet
242257
await handleUserInput(true);
243-
await this.#removeAccountHelper(address);
244-
await this.#persistKeyringHelper();
258+
259+
try {
260+
await this.#removeAccountHelper(address);
261+
await this.#persistKeyringHelper();
262+
263+
// Track successful account removal
264+
const snapName = getSnapName(snapId as SnapId, this.#messenger);
265+
trackSnapAccountEvent(
266+
MetaMetricsEvents.ACCOUNT_REMOVED,
267+
snapId,
268+
snapName,
269+
);
270+
} catch (error) {
271+
Logger.error(error as Error, `Error removing snap account: ${address}`);
272+
const snapName = getSnapName(snapId as SnapId, this.#messenger);
273+
trackSnapAccountEvent(
274+
MetaMetricsEvents.ACCOUNT_REMOVED,
275+
snapId,
276+
snapName,
277+
);
278+
throw error;
279+
}
245280
}
246281

247282
async redirectUser(snapId: string, url: string, message: string) {

0 commit comments

Comments
 (0)