From 009c6e14555461642fe88c56f337bddd6ddbc7fa Mon Sep 17 00:00:00 2001 From: David Walsh Date: Wed, 16 Feb 2022 14:54:30 -0600 Subject: [PATCH] Use hardware wallet constants when possible (#13634) --- app/scripts/metamask-controller.js | 6 ++-- app/scripts/metamask-controller.test.js | 31 +++++++++++++------ ui/ducks/app/app.test.js | 3 +- .../connect-hardware/account-list.js | 10 ++++-- ui/store/actions.js | 4 +-- ui/store/actions.test.js | 11 ++++--- 6 files changed, 42 insertions(+), 23 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 93381ed63..b1d783e6b 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -2047,7 +2047,7 @@ export default class MetamaskController extends EventEmitter { if (deviceName === DEVICE_NAMES.LATTICE) { keyring.appName = 'MetaMask'; } - if (deviceName === 'trezor') { + if (deviceName === DEVICE_NAMES.TREZOR) { const model = keyring.getModel(); this.appStateController.setTrezorModel(model); } @@ -2058,7 +2058,7 @@ export default class MetamaskController extends EventEmitter { } async attemptLedgerTransportCreation() { - const keyring = await this.getKeyringForDevice('ledger'); + const keyring = await this.getKeyringForDevice(DEVICE_NAMES.LEDGER); return await keyring.attemptMakeApp(); } @@ -3720,7 +3720,7 @@ export default class MetamaskController extends EventEmitter { transportType, ); - const keyring = await this.getKeyringForDevice('ledger'); + const keyring = await this.getKeyringForDevice(DEVICE_NAMES.LEDGER); if (keyring?.updateTransportMethod) { return keyring.updateTransportMethod(newValue).catch((e) => { // If there was an error updating the transport, we should diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index 5e5414aae..2739c7dcf 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -9,7 +9,10 @@ import proxyquire from 'proxyquire'; import { TRANSACTION_STATUSES } from '../../shared/constants/transaction'; import createTxMeta from '../../test/lib/createTxMeta'; import { NETWORK_TYPE_RPC } from '../../shared/constants/network'; -import { KEYRING_TYPES } from '../../shared/constants/hardware-wallets'; +import { + KEYRING_TYPES, + DEVICE_NAMES, +} from '../../shared/constants/hardware-wallets'; import { addHexPrefix } from './lib/util'; const Ganache = require('../../test/e2e/ganache'); @@ -490,7 +493,9 @@ describe('MetaMaskController', function () { it('should add the Trezor Hardware keyring', async function () { sinon.spy(metamaskController.keyringController, 'addNewKeyring'); - await metamaskController.connectHardware('trezor', 0).catch(() => null); + await metamaskController + .connectHardware(DEVICE_NAMES.TREZOR, 0) + .catch(() => null); const keyrings = await metamaskController.keyringController.getKeyringsByType( KEYRING_TYPES.TREZOR, ); @@ -503,7 +508,9 @@ describe('MetaMaskController', function () { it('should add the Ledger Hardware keyring', async function () { sinon.spy(metamaskController.keyringController, 'addNewKeyring'); - await metamaskController.connectHardware('ledger', 0).catch(() => null); + await metamaskController + .connectHardware(DEVICE_NAMES.LEDGER, 0) + .catch(() => null); const keyrings = await metamaskController.keyringController.getKeyringsByType( KEYRING_TYPES.LEDGER, ); @@ -531,8 +538,12 @@ describe('MetaMaskController', function () { }); it('should be locked by default', async function () { - await metamaskController.connectHardware('trezor', 0).catch(() => null); - const status = await metamaskController.checkHardwareStatus('trezor'); + await metamaskController + .connectHardware(DEVICE_NAMES.TREZOR, 0) + .catch(() => null); + const status = await metamaskController.checkHardwareStatus( + DEVICE_NAMES.TREZOR, + ); assert.equal(status, false); }); }); @@ -550,8 +561,10 @@ describe('MetaMaskController', function () { }); it('should wipe all the keyring info', async function () { - await metamaskController.connectHardware('trezor', 0).catch(() => null); - await metamaskController.forgetDevice('trezor'); + await metamaskController + .connectHardware(DEVICE_NAMES.TREZOR, 0) + .catch(() => null); + await metamaskController.forgetDevice(DEVICE_NAMES.TREZOR); const keyrings = await metamaskController.keyringController.getKeyringsByType( KEYRING_TYPES.TREZOR, ); @@ -592,11 +605,11 @@ describe('MetaMaskController', function () { sinon.spy(metamaskController.preferencesController, 'setSelectedAddress'); sinon.spy(metamaskController.preferencesController, 'setAccountLabel'); await metamaskController - .connectHardware('trezor', 0, `m/44'/1'/0'/0`) + .connectHardware(DEVICE_NAMES.TREZOR, 0, `m/44'/1'/0'/0`) .catch(() => null); await metamaskController.unlockHardwareWalletAccount( accountToUnlock, - 'trezor', + DEVICE_NAMES.TREZOR, `m/44'/1'/0'/0`, ); }); diff --git a/ui/ducks/app/app.test.js b/ui/ducks/app/app.test.js index c71a539e0..f920cb91b 100644 --- a/ui/ducks/app/app.test.js +++ b/ui/ducks/app/app.test.js @@ -1,4 +1,5 @@ import * as actionConstants from '../../store/actionConstants'; +import { DEVICE_NAMES } from '../../../shared/constants/hardware-wallets'; import reduceApp from './app'; const actions = actionConstants; @@ -260,7 +261,7 @@ describe('App State', () => { const state = reduceApp(metamaskState, { type: actions.SET_HARDWARE_WALLET_DEFAULT_HD_PATH, value: { - device: 'ledger', + device: DEVICE_NAMES.LEDGER, path: "m/44'/60'/0'", }, }); diff --git a/ui/pages/create-account/connect-hardware/account-list.js b/ui/pages/create-account/connect-hardware/account-list.js index 3d4c4522a..dcfda41fe 100644 --- a/ui/pages/create-account/connect-hardware/account-list.js +++ b/ui/pages/create-account/connect-hardware/account-list.js @@ -8,6 +8,8 @@ import Dropdown from '../../../components/ui/dropdown'; import { getURLHostName } from '../../../helpers/utils/util'; +import { DEVICE_NAMES } from '../../../../shared/constants/hardware-wallets'; + class AccountList extends Component { state = { pathValue: null, @@ -61,9 +63,11 @@ class AccountList extends Component { renderHeader() { const { device } = this.props; - const shouldShowHDPaths = ['ledger', 'lattice', 'trezor'].includes( - device.toLowerCase(), - ); + const shouldShowHDPaths = [ + DEVICE_NAMES.LEDGER, + DEVICE_NAMES.LATTICE, + DEVICE_NAMES.TREZOR, + ].includes(device.toLowerCase()); return (

diff --git a/ui/store/actions.js b/ui/store/actions.js index 71ba543e3..0717f9ab5 100644 --- a/ui/store/actions.js +++ b/ui/store/actions.js @@ -398,7 +398,7 @@ export function connectHardware(deviceName, page, hdPath, t) { let accounts; try { - if (deviceName === 'ledger') { + if (deviceName === DEVICE_NAMES.LEDGER) { await promisifiedBackground.establishLedgerTransportPreference(); } if ( @@ -424,7 +424,7 @@ export function connectHardware(deviceName, page, hdPath, t) { } catch (error) { log.error(error); if ( - deviceName === 'ledger' && + deviceName === DEVICE_NAMES.LEDGER && ledgerTransportType === LEDGER_TRANSPORT_TYPES.WEBHID && error.message.match('Failed to open the device') ) { diff --git a/ui/store/actions.test.js b/ui/store/actions.test.js index df7ad5425..0866713b4 100644 --- a/ui/store/actions.test.js +++ b/ui/store/actions.test.js @@ -4,6 +4,7 @@ import thunk from 'redux-thunk'; import enLocale from '../../app/_locales/en/messages.json'; import MetaMaskController from '../../app/scripts/metamask-controller'; import { TRANSACTION_STATUSES } from '../../shared/constants/transaction'; +import { DEVICE_NAMES } from '../../shared/constants/hardware-wallets'; import { GAS_LIMITS } from '../../shared/constants/gas'; import * as actions from './actions'; @@ -436,7 +437,7 @@ describe('Actions', () => { actions._setBackgroundConnection(background); await store.dispatch( - actions.checkHardwareStatus('ledger', `m/44'/60'/0'/0`), + actions.checkHardwareStatus(DEVICE_NAMES.LEDGER, `m/44'/60'/0'/0`), ); expect(checkHardwareStatus.callCount).toStrictEqual(1); }); @@ -476,7 +477,7 @@ describe('Actions', () => { actions._setBackgroundConnection(background); - await store.dispatch(actions.forgetDevice('ledger')); + await store.dispatch(actions.forgetDevice(DEVICE_NAMES.LEDGER)); expect(forgetDevice.callCount).toStrictEqual(1); }); @@ -518,7 +519,7 @@ describe('Actions', () => { actions._setBackgroundConnection(background); await store.dispatch( - actions.connectHardware('ledger', 0, `m/44'/60'/0'/0`), + actions.connectHardware(DEVICE_NAMES.LEDGER, 0, `m/44'/60'/0'/0`), ); expect(connectHardware.callCount).toStrictEqual(1); }); @@ -544,7 +545,7 @@ describe('Actions', () => { ]; await expect( - store.dispatch(actions.connectHardware('ledger')), + store.dispatch(actions.connectHardware(DEVICE_NAMES.LEDGER)), ).rejects.toThrow('error'); expect(store.getActions()).toStrictEqual(expectedActions); @@ -567,7 +568,7 @@ describe('Actions', () => { await store.dispatch( actions.unlockHardwareWalletAccounts( [0], - 'ledger', + DEVICE_NAMES.LEDGER, `m/44'/60'/0'/0`, '', ),