Skip to content

Commit ea93cae

Browse files
gantunesrgantunesr
authored
chore: update accounts metric payload (#38420)
<!-- 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** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/38420?quickstart=1) Updates the send events to use the updated properties for hardware wallets as they're described in [MUL-1257](https://consensyssoftware.atlassian.net/browse/MUL-1257) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MUL-1257 ## **Manual testing steps** 1. Perform a send transaction using a hardware wallet device 2. Check that the properties `hardware_wallet_type` and `account_type` reflect the following values: `Ledger || Trezor || QR Hardware || Lattice || OneKey` for the events Transaction Added, Transaction Approved, Transaction Submitted, and Transaction Finalized ## **Screenshots/Recordings** <img width="750" alt="Screenshot 2025-12-01 at 12 07 02 PM" src="https://github.com/user-attachments/assets/70e349bd-8190-49cb-945f-df4bfebd35c4" /> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/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. [MUL-1257]: https://consensyssoftware.atlassian.net/browse/MUL-1257?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Maps hardware keyring types to human-readable device names for metrics and only includes `account_hardware_type` when present; updates tests accordingly. > > - **Metrics payload (`app/scripts/lib/snap-keyring/metrics.ts`)**: > - Conditionally include `account_hardware_type` only when resolved. > - Keep `account_type`, `device_model`, and snap fields unchanged. > - **Controller (`app/scripts/metamask-controller.js`)**: > - Use `KEYRING_DEVICE_PROPERTY_MAP` to map keyring types to device names in `getHardwareTypeForMetric` and `getAccountType`. > - **Constants (`shared/constants/hardware-wallets.ts`)**: > - Add `KEYRING_DEVICE_PROPERTY_MAP` mapping from `KeyringTypes` to device labels (Ledger, Trezor, OneKey, Lattice, QR Hardware). > - **Tests**: > - Update expectations to use device-name mapping and conditional `account_hardware_type`. > - Add hardware wallet account test; adjust HD and snap account tests. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bf76d61. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 642d0ad commit ea93cae

File tree

5 files changed

+98
-18
lines changed

5 files changed

+98
-18
lines changed

‎app/scripts/lib/snap-keyring/metrics.test.ts‎

Lines changed: 74 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,9 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
3434
});
3535

3636
describe('when the wallet is unlocked', () => {
37-
it('resolves keyring and device info for a HD account', async () => {
37+
it('resolves keyring info for a HD account', async () => {
3838
getAccountType.mockResolvedValue('accountType');
3939
getDeviceModel.mockResolvedValue('deviceModel');
40-
getHardwareTypeForMetric.mockResolvedValue('hardwareType');
4140
messenger.call
4241
.mockReturnValueOnce({
4342
address: '0x123',
@@ -69,7 +68,6 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
6968

7069
expect(getAccountType).toHaveBeenCalledWith('0x123');
7170
expect(getDeviceModel).toHaveBeenCalledWith('0x123');
72-
expect(getHardwareTypeForMetric).toHaveBeenCalledWith('0x123');
7371
expect(messenger.call).toHaveBeenNthCalledWith(
7472
1,
7573
'AccountsController:getSelectedAccount',
@@ -87,20 +85,16 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
8785
device_model: 'deviceModel',
8886
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
8987
// eslint-disable-next-line @typescript-eslint/naming-convention
90-
account_hardware_type: 'hardwareType',
91-
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
92-
// eslint-disable-next-line @typescript-eslint/naming-convention
9388
account_snap_type: undefined,
9489
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
9590
// eslint-disable-next-line @typescript-eslint/naming-convention
9691
account_snap_version: undefined,
9792
});
9893
});
9994

100-
it('resolves keyring, device, and snap info for a snap account', async () => {
95+
it('resolves keyring and snap info for a snap account', async () => {
10196
getAccountType.mockResolvedValue('accountType');
10297
getDeviceModel.mockResolvedValue('deviceModel');
103-
getHardwareTypeForMetric.mockResolvedValue('hardwareType');
10498
messenger.call
10599
.mockReturnValueOnce({
106100
address: '0x123',
@@ -138,7 +132,6 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
138132

139133
expect(getAccountType).toHaveBeenCalledWith('0x123');
140134
expect(getDeviceModel).toHaveBeenCalledWith('0x123');
141-
expect(getHardwareTypeForMetric).toHaveBeenCalledWith('0x123');
142135
expect(messenger.call).toHaveBeenNthCalledWith(
143136
1,
144137
'AccountsController:getSelectedAccount',
@@ -161,13 +154,84 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
161154
device_model: 'deviceModel',
162155
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
163156
// eslint-disable-next-line @typescript-eslint/naming-convention
164-
account_hardware_type: 'hardwareType',
157+
account_snap_type: 'snapId',
158+
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
159+
// eslint-disable-next-line @typescript-eslint/naming-convention
160+
account_snap_version: 'snapVersion',
161+
});
162+
});
163+
164+
it('resolves device and account type info for a hardware wallet account', async () => {
165+
getAccountType.mockResolvedValue('accountType');
166+
getDeviceModel.mockResolvedValue('deviceModel');
167+
getHardwareTypeForMetric.mockResolvedValue('hardwareType');
168+
messenger.call
169+
.mockReturnValueOnce({
170+
address: '0x123',
171+
id: 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3',
172+
metadata: {
173+
name: 'Account 1',
174+
keyring: {
175+
type: 'Snap Keyring',
176+
},
177+
snap: {
178+
id: 'snapId',
179+
name: 'mock-name',
180+
enabled: true,
181+
},
182+
},
183+
options: {},
184+
methods: [
185+
'personal_sign',
186+
'eth_signTransaction',
187+
'eth_signTypedData_v1',
188+
'eth_signTypedData_v3',
189+
'eth_signTypedData_v4',
190+
],
191+
type: 'eip155:eoa',
192+
})
193+
.mockReturnValueOnce({ id: 'snapId', version: 'snapVersion' })
194+
.mockReturnValueOnce({ isUnlocked: true });
195+
196+
const result = await getSnapAndHardwareInfoForMetrics(
197+
getAccountType,
198+
getDeviceModel,
199+
getHardwareTypeForMetric,
200+
messenger,
201+
);
202+
203+
expect(getAccountType).toHaveBeenCalledWith('0x123');
204+
expect(getDeviceModel).toHaveBeenCalledWith('0x123');
205+
expect(getHardwareTypeForMetric).toHaveBeenCalledWith('0x123');
206+
expect(messenger.call).toHaveBeenNthCalledWith(
207+
1,
208+
'AccountsController:getSelectedAccount',
209+
);
210+
expect(messenger.call).toHaveBeenNthCalledWith(
211+
2,
212+
'SnapController:get',
213+
'snapId',
214+
);
215+
expect(messenger.call).toHaveBeenNthCalledWith(
216+
3,
217+
'KeyringController:getState',
218+
);
219+
expect(result).toEqual({
220+
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
221+
// eslint-disable-next-line @typescript-eslint/naming-convention
222+
account_type: 'accountType',
223+
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
224+
// eslint-disable-next-line @typescript-eslint/naming-convention
225+
device_model: 'deviceModel',
165226
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
166227
// eslint-disable-next-line @typescript-eslint/naming-convention
167228
account_snap_type: 'snapId',
168229
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
169230
// eslint-disable-next-line @typescript-eslint/naming-convention
170231
account_snap_version: 'snapVersion',
232+
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
233+
// eslint-disable-next-line @typescript-eslint/naming-convention
234+
account_hardware_type: 'hardwareType',
171235
});
172236
});
173237
});

‎app/scripts/lib/snap-keyring/metrics.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,18 @@ export async function getSnapAndHardwareInfoForMetrics(
5353
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
5454
// eslint-disable-next-line @typescript-eslint/naming-convention
5555
device_model: await getDeviceModel(selectedAddress),
56-
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
57-
// eslint-disable-next-line @typescript-eslint/naming-convention
58-
account_hardware_type: await getHardwareTypeForMetric(selectedAddress),
5956
};
57+
58+
const hardwareType = await getHardwareTypeForMetric(selectedAddress);
59+
60+
if (hardwareType) {
61+
keyringAccountInfo = {
62+
...keyringAccountInfo,
63+
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
64+
// eslint-disable-next-line @typescript-eslint/naming-convention
65+
account_hardware_type: hardwareType,
66+
};
67+
}
6068
}
6169

6270
return {

‎app/scripts/metamask-controller.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ import {
151151

152152
import {
153153
HardwareDeviceNames,
154-
HardwareKeyringType,
155154
LedgerTransportTypes,
155+
KEYRING_DEVICE_PROPERTY_MAP,
156156
} from '../../shared/constants/hardware-wallets';
157157
import { KeyringType } from '../../shared/constants/keyring';
158158
import { RestrictedMethods } from '../../shared/constants/permissions';
@@ -5452,7 +5452,7 @@ export default class MetamaskController extends EventEmitter {
54525452
async getHardwareTypeForMetric(address) {
54535453
return await this.keyringController.withKeyring(
54545454
{ address },
5455-
({ keyring }) => HardwareKeyringType[keyring.type],
5455+
({ keyring }) => KEYRING_DEVICE_PROPERTY_MAP[keyring.type],
54565456
);
54575457
}
54585458

@@ -5490,7 +5490,7 @@ export default class MetamaskController extends EventEmitter {
54905490
case KeyringType.lattice:
54915491
case KeyringType.qr:
54925492
case KeyringType.ledger:
5493-
return 'hardware';
5493+
return KEYRING_DEVICE_PROPERTY_MAP[keyringType];
54945494
case KeyringType.imported:
54955495
return 'imported';
54965496
case KeyringType.snap:

‎app/scripts/metamask-controller.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ import { KeyringTypes } from '@metamask/keyring-controller';
5050
import { createTestProviderTools } from '../../test/stub/provider';
5151
import {
5252
HardwareDeviceNames,
53-
HardwareKeyringType,
53+
KEYRING_DEVICE_PROPERTY_MAP,
5454
} from '../../shared/constants/hardware-wallets';
5555
import { KeyringType } from '../../shared/constants/keyring';
5656
import { LOG_EVENT } from '../../shared/constants/logs';
@@ -1630,7 +1630,7 @@ describe('MetaMaskController', () => {
16301630
const result =
16311631
await metamaskController.getHardwareTypeForMetric('0x123');
16321632

1633-
expect(result).toBe(HardwareKeyringType[type]);
1633+
expect(result).toBe(KEYRING_DEVICE_PROPERTY_MAP[type]);
16341634
},
16351635
);
16361636
});

‎shared/constants/hardware-wallets.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,14 @@ export const DEVICE_KEYRING_MAP = {
147147
[HardwareDeviceNames.qr]: KeyringTypes.qr,
148148
};
149149

150+
export const KEYRING_DEVICE_PROPERTY_MAP = {
151+
[KeyringTypes.ledger]: 'Ledger',
152+
[KeyringTypes.trezor]: 'Trezor',
153+
[KeyringTypes.oneKey]: 'OneKey',
154+
[KeyringTypes.lattice]: 'Lattice',
155+
[KeyringTypes.qr]: 'QR Hardware',
156+
};
157+
150158
export const U2F_ERROR = 'U2F';
151159

152160
export const LEDGER_ERRORS_CODES = {

0 commit comments

Comments
 (0)