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: add OneKey on device selection screen #29610

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
7 changes: 7 additions & 0 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions app/scripts/lib/createRPCMethodTrackingMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ function finalizeSignatureFragment(
* that should be tracked for methods rate limited by random sample.
* @param {Function} opts.getAccountType
* @param {Function} opts.getDeviceModel
* @param {Function} opts.getHardwareTypeForMetric
* @param {Function} opts.isConfirmationRedesignEnabled
* @param {Function} opts.isRedesignedConfirmationsDeveloperEnabled
* @param {RestrictedControllerMessenger} opts.snapAndHardwareMessenger
Expand All @@ -214,6 +215,7 @@ export default function createRPCMethodTrackingMiddleware({
globalRateLimitMaxAmount = 10, // max of events in the globalRateLimitTimeout window. pass 0 for no global rate limit
getAccountType,
getDeviceModel,
getHardwareTypeForMetric,
isConfirmationRedesignEnabled,
isRedesignedConfirmationsDeveloperEnabled,
snapAndHardwareMessenger,
Expand Down Expand Up @@ -335,6 +337,7 @@ export default function createRPCMethodTrackingMiddleware({
const snapAndHardwareInfo = await getSnapAndHardwareInfoForMetrics(
getAccountType,
getDeviceModel,
getHardwareTypeForMetric,
snapAndHardwareMessenger,
);

Expand Down
12 changes: 10 additions & 2 deletions app/scripts/lib/snap-keyring/metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ import { getSnapAndHardwareInfoForMetrics } from './metrics';
describe('getSnapAndHardwareInfoForMetrics', () => {
let getAccountType: jest.Mock;
let getDeviceModel: jest.Mock;
let getHardwareTypeForMetric: jest.Mock;
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let messenger: any;

beforeEach(() => {
getAccountType = jest.fn();
getDeviceModel = jest.fn();
getHardwareTypeForMetric = jest.fn();
messenger = {
call: jest.fn(),
};
Expand All @@ -23,6 +25,7 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
const result = await getSnapAndHardwareInfoForMetrics(
getAccountType,
getDeviceModel,
getHardwareTypeForMetric,
// @ts-expect-error - We're testing the case where messenger is null
null,
);
Expand All @@ -32,6 +35,7 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
it('should call the appropriate functions with the correct arguments', async () => {
getAccountType.mockResolvedValue('accountType');
getDeviceModel.mockResolvedValue('deviceModel');
getHardwareTypeForMetric.mockResolvedValue('hardwareType');
messenger.call
.mockReturnValueOnce({
address: '0x123',
Expand All @@ -57,6 +61,7 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
const result = await getSnapAndHardwareInfoForMetrics(
getAccountType,
getDeviceModel,
getHardwareTypeForMetric,
messenger,
);

Expand All @@ -69,7 +74,7 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
expect(result).toEqual({
account_type: 'accountType',
device_model: 'deviceModel',
account_hardware_type: undefined,
account_hardware_type: 'hardwareType',
account_snap_type: undefined,
account_snap_version: undefined,
});
Expand All @@ -78,6 +83,7 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
it('should call the appropriate functions with the correct arguments the account is a snap account', async () => {
getAccountType.mockResolvedValue('accountType');
getDeviceModel.mockResolvedValue('deviceModel');
getHardwareTypeForMetric.mockResolvedValue('hardwareType');
messenger.call
.mockReturnValueOnce({
address: '0x123',
Expand Down Expand Up @@ -108,11 +114,13 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
const result = await getSnapAndHardwareInfoForMetrics(
getAccountType,
getDeviceModel,
getHardwareTypeForMetric,
messenger,
);

expect(getAccountType).toHaveBeenCalledWith('0x123');
expect(getDeviceModel).toHaveBeenCalledWith('0x123');
expect(getHardwareTypeForMetric).toHaveBeenCalledWith('0x123');
expect(messenger.call).toHaveBeenNthCalledWith(
1,
'AccountsController:getSelectedAccount',
Expand All @@ -125,7 +133,7 @@ describe('getSnapAndHardwareInfoForMetrics', () => {
expect(result).toEqual({
account_type: 'accountType',
device_model: 'deviceModel',
account_hardware_type: undefined,
account_hardware_type: 'hardwareType',
account_snap_type: 'snapId',
account_snap_version: 'snapVersion',
});
Expand Down
13 changes: 3 additions & 10 deletions app/scripts/lib/snap-keyring/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { KeyringControllerGetKeyringForAccountAction } from '@metamask/keyring-c
import { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller';
import { GetSnap } from '@metamask/snaps-controllers';
import { Snap } from '@metamask/snaps-utils';
import { HardwareKeyringType } from '../../../../shared/constants/hardware-wallets';

type AllowedActions =
| GetSnap
Expand All @@ -20,6 +21,7 @@ export type SnapAndHardwareMessenger = RestrictedControllerMessenger<
export async function getSnapAndHardwareInfoForMetrics(
getAccountType: (address: string) => Promise<string>,
getDeviceModel: (address: string) => Promise<string>,
getHardwareTypeForMetric: (address: string) => Promise<HardwareKeyringType>,
messenger: SnapAndHardwareMessenger,
) {
// If it's coming from a unit test, there's no messenger
Expand All @@ -30,7 +32,6 @@ export async function getSnapAndHardwareInfoForMetrics(

const account = messenger.call('AccountsController:getSelectedAccount');
const selectedAddress = account.address;
const { keyring } = account.metadata;

let snap;
if (account.metadata.snap?.id) {
Expand All @@ -40,18 +41,10 @@ export async function getSnapAndHardwareInfoForMetrics(
) as Snap;
}

async function getHardwareWalletType() {
if (keyring?.type?.includes('Hardware')) {
return keyring.type;
}

return undefined;
}

return {
account_type: await getAccountType(selectedAddress),
device_model: await getDeviceModel(selectedAddress),
account_hardware_type: await getHardwareWalletType(),
account_hardware_type: await getHardwareTypeForMetric(selectedAddress),
account_snap_type: snap?.id,
account_snap_version: snap?.version,
};
Expand Down
1 change: 1 addition & 0 deletions app/scripts/lib/transaction/metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const mockTransactionMetricsRequest = {
updateEventFragment: jest.fn(),
getAccountType: jest.fn(),
getDeviceModel: jest.fn(),
getHardwareTypeForMetric: jest.fn(),
getEIP1559GasFeeEstimates: jest.fn(),
getSelectedAddress: jest.fn(),
getParticipateInMetrics: jest.fn(),
Expand Down
3 changes: 3 additions & 0 deletions app/scripts/lib/transaction/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {
type SnapAndHardwareMessenger,
} from '../snap-keyring/metrics';
import { shouldUseRedesignForTransactions } from '../../../../shared/lib/confirmation.utils';
import { HardwareKeyringType } from '../../../../shared/constants/hardware-wallets';

export type TransactionMetricsRequest = {
createEventFragment: (
Expand All @@ -78,6 +79,7 @@ export type TransactionMetricsRequest = {
getDeviceModel: (
address: string,
) => Promise<'ledger' | 'lattice' | 'N/A' | string>;
getHardwareTypeForMetric: (address: string) => Promise<HardwareKeyringType>;
// According to the type GasFeeState returned from getEIP1559GasFeeEstimates
// doesn't include some properties used in buildEventFragmentProperties,
// hence returning any here to avoid type errors.
Expand Down Expand Up @@ -1080,6 +1082,7 @@ async function buildEventFragmentProperties({
const snapAndHardwareInfo = await getSnapAndHardwareInfoForMetrics(
transactionMetricsRequest.getAccountType,
transactionMetricsRequest.getDeviceModel,
transactionMetricsRequest.getHardwareTypeForMetric,
transactionMetricsRequest.snapAndHardwareMessenger,
);
Object.assign(properties, snapAndHardwareInfo);
Expand Down
51 changes: 26 additions & 25 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ import { getAllowedSmartTransactionsChainIds } from '../../shared/constants/smar

import {
HardwareDeviceNames,
HardwareKeyringType,
LedgerTransportTypes,
} from '../../shared/constants/hardware-wallets';
import { KeyringType } from '../../shared/constants/keyring';
Expand Down Expand Up @@ -241,7 +242,6 @@ import {
isHardwareWallet,
getFeatureFlagsByChainId,
getCurrentChainSupportsSmartTransactions,
getHardwareWalletType,
getSmartTransactionsPreferenceEnabled,
} from '../../shared/modules/selectors';
import { createCaipStream } from '../../shared/modules/caip-stream';
Expand Down Expand Up @@ -1934,6 +1934,7 @@ export default class MetamaskController extends EventEmitter {
),
getAccountType: this.getAccountType.bind(this),
getDeviceModel: this.getDeviceModel.bind(this),
getHardwareTypeForMetric: this.getHardwareTypeForMetric.bind(this),
snapAndHardwareMessenger: this.controllerMessenger.getRestricted({
name: 'SnapAndHardwareMessenger',
allowedActions: [
Expand Down Expand Up @@ -2281,8 +2282,8 @@ export default class MetamaskController extends EventEmitter {
getMetaMetricsProps: async () => {
const selectedAddress =
this.accountsController.getSelectedAccount().address;
const accountHardwareType = await getHardwareWalletType(
this._getMetaMaskState(),
const accountHardwareType = await this.getHardwareTypeForMetric(
selectedAddress,
);
const accountType = await this.getAccountType(selectedAddress);
const deviceModel = await this.getDeviceModel(selectedAddress);
Expand Down Expand Up @@ -3605,7 +3606,6 @@ export default class MetamaskController extends EventEmitter {
connectHardware: this.connectHardware.bind(this),
forgetDevice: this.forgetDevice.bind(this),
checkHardwareStatus: this.checkHardwareStatus.bind(this),
getDeviceNameForMetric: this.getDeviceNameForMetric.bind(this),
unlockHardwareWalletAccount: this.unlockHardwareWalletAccount.bind(this),
attemptLedgerTransportCreation:
this.attemptLedgerTransportCreation.bind(this),
Expand Down Expand Up @@ -4941,29 +4941,24 @@ export default class MetamaskController extends EventEmitter {
}

/**
* Get hardware device name for metric logging.
* Get hardware type that will be sent for metrics logging
*
* @param deviceName - HardwareDeviceNames
* @param hdPath - string
* @returns {Promise<string>}
* @param {string} address
* @returns {HardwareKeyringType} Keyring hardware type
*/
async getDeviceNameForMetric(deviceName, hdPath) {
if (deviceName !== HardwareDeviceNames.trezor) {
return deviceName;
}

return await this.#withKeyringForDevice(
{ name: deviceName, hdPath },
(keyring) => {
const { minorVersion } = keyring.bridge;
// Specific case for OneKey devices, see `ONE_KEY_VIA_TREZOR_MINOR_VERSION` for further details.
if (minorVersion && minorVersion === ONE_KEY_VIA_TREZOR_MINOR_VERSION) {
return HardwareDeviceNames.oneKeyViaTrezor;
}

return deviceName;
},
async getHardwareTypeForMetric(address) {
const keyringTypeAndBridge = await this.keyringController.withKeyring(
{ address },
({ type, bridge }) => ({
type,
bridge,
}),
);
const { type: keyringType, bridge: keyringBridge } = keyringTypeAndBridge;
// Specific case for OneKey devices, see `ONE_KEY_VIA_TREZOR_MINOR_VERSION` for further details.
return keyringBridge?.minorVersion === ONE_KEY_VIA_TREZOR_MINOR_VERSION
? HardwareKeyringType.oneKey
: HardwareKeyringType[keyringType];
}

/**
Expand Down Expand Up @@ -6267,6 +6262,7 @@ export default class MetamaskController extends EventEmitter {
createRPCMethodTrackingMiddleware({
getAccountType: this.getAccountType.bind(this),
getDeviceModel: this.getDeviceModel.bind(this),
getHardwareTypeForMetric: this.getHardwareTypeForMetric.bind(this),
isConfirmationRedesignEnabled:
this.isConfirmationRedesignEnabled.bind(this),
isRedesignedConfirmationsDeveloperEnabled:
Expand Down Expand Up @@ -7110,6 +7106,7 @@ export default class MetamaskController extends EventEmitter {
// Other dependencies
getAccountType: this.getAccountType.bind(this),
getDeviceModel: this.getDeviceModel.bind(this),
getHardwareTypeForMetric: this.getHardwareTypeForMetric.bind(this),
getEIP1559GasFeeEstimates:
this.gasFeeController.fetchGasFeeEstimates.bind(this.gasFeeController),
getSelectedAddress: () =>
Expand Down Expand Up @@ -7811,6 +7808,7 @@ export default class MetamaskController extends EventEmitter {
let keyringType = null;
switch (options.name) {
case HardwareDeviceNames.trezor:
case HardwareDeviceNames.oneKey:
keyringType = keyringOverrides?.trezor?.type || TrezorKeyring.type;
break;
case HardwareDeviceNames.ledger:
Expand Down Expand Up @@ -7839,7 +7837,10 @@ export default class MetamaskController extends EventEmitter {
keyring.appName = 'MetaMask';
}

if (options.name === HardwareDeviceNames.trezor) {
if (
options.name === HardwareDeviceNames.trezor ||
options.name === HardwareDeviceNames.oneKey
) {
const model = keyring.getModel();
this.appStateController.setTrezorModel(model);
}
Expand Down
Loading
Loading