Skip to content

Commit

Permalink
refactor: use withKeyring method (#25435)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mikesposito
mikesposito committed Sep 10, 2024
1 parent 92c8a06 commit e323e79
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 142 deletions.
262 changes: 157 additions & 105 deletions app/scripts/metamask-controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ import {
import { Interface } from '@ethersproject/abi';
import { abiERC1155, abiERC721 } from '@metamask/metamask-eth-abis';
import { isEvmAccountType } from '@metamask/keyring-api';
import { normalize } from '@metamask/eth-sig-util';
import {
AuthenticationController,
UserStorageController,
Expand Down Expand Up @@ -4145,7 +4146,10 @@ export default class MetamaskController extends EventEmitter {
// keyring's iframe and have the setting initialized properly
// Optimistically called to not block MetaMask login due to
// Ledger Keyring GitHub downtime
this.setLedgerTransportPreference();
this.withKeyringForDevice(
{ name: HardwareDeviceNames.ledger },
async (keyring) => this.setLedgerTransportPreference(keyring),
);
}
} finally {
releaseLock();
Expand Down Expand Up @@ -4277,7 +4281,10 @@ export default class MetamaskController extends EventEmitter {
// Optimistically called to not block MetaMask login due to
// Ledger Keyring GitHub downtime
if (completedOnboarding) {
this.setLedgerTransportPreference();
this.withKeyringForDevice(
{ name: HardwareDeviceNames.ledger },
async (keyring) => this.setLedgerTransportPreference(keyring),
);
}
}

Expand Down Expand Up @@ -4381,52 +4388,78 @@ export default class MetamaskController extends EventEmitter {
// Hardware
//

async getKeyringForDevice(deviceName, hdPath = null) {
/**
* Select a hardware wallet device and execute a
* callback with the keyring for that device.
*
* Note that KeyringController state is not updated before
* the end of the callback execution, and calls to KeyringController
* methods within the callback can lead to deadlocks.
*
* @param {object} options - The options for the device
* @param {string} options.name - The device name to select
* @param {string} options.hdPath - An optional hd path to be set on the device
* keyring
* @param {*} callback - The callback to execute with the keyring
* @returns {*} The result of the callback
*/
async withKeyringForDevice(options, callback) {
const keyringOverrides = this.opts.overrides?.keyrings;
let keyringName = null;
switch (deviceName) {
let keyringType = null;
switch (options.name) {
case HardwareDeviceNames.trezor:
keyringName = keyringOverrides?.trezor?.type || TrezorKeyring.type;
keyringType = keyringOverrides?.trezor?.type || TrezorKeyring.type;
break;
case HardwareDeviceNames.ledger:
keyringName = keyringOverrides?.ledger?.type || LedgerKeyring.type;
keyringType = keyringOverrides?.ledger?.type || LedgerKeyring.type;
break;
case HardwareDeviceNames.qr:
keyringName = QRHardwareKeyring.type;
keyringType = QRHardwareKeyring.type;
break;
case HardwareDeviceNames.lattice:
keyringName = keyringOverrides?.lattice?.type || LatticeKeyring.type;
keyringType = keyringOverrides?.lattice?.type || LatticeKeyring.type;
break;
default:
throw new Error(
'MetamaskController:getKeyringForDevice - Unknown device',
'MetamaskController:withKeyringForDevice - Unknown device',
);
}
let [keyring] = await this.keyringController.getKeyringsByType(keyringName);
if (!keyring) {
keyring = await this.keyringController.addNewKeyring(keyringName);
}
if (hdPath && keyring.setHdPath) {
keyring.setHdPath(hdPath);
}
if (deviceName === HardwareDeviceNames.lattice) {
keyring.appName = 'MetaMask';
}
if (deviceName === HardwareDeviceNames.trezor) {
const model = keyring.getModel();
this.appStateController.setTrezorModel(model);
}

keyring.network = getProviderConfig({
metamask: this.networkController.state,
}).type;
return this.keyringController.withKeyring(
{ type: keyringType },
async (keyring) => {
if (options.hdPath && keyring.setHdPath) {
keyring.setHdPath(options.hdPath);
}

keyring.network = getProviderConfig({
metamask: this.networkController.state,
}).type;

if (options.name === HardwareDeviceNames.lattice) {
keyring.appName = 'MetaMask';
}

if (options.name === HardwareDeviceNames.trezor) {
const model = keyring.getModel();
this.appStateController.setTrezorModel(model);
}

return keyring;
keyring.network = this.networkController.state.providerConfig.type;

return await callback(keyring);
},
{
createIfMissing: true,
},
);
}

async attemptLedgerTransportCreation() {
const keyring = await this.getKeyringForDevice(HardwareDeviceNames.ledger);
return await keyring.attemptMakeApp();
return await this.withKeyringForDevice(
HardwareDeviceNames.ledger,
async (keyring) => keyring.attemptMakeApp(),
);
}

/**
Expand All @@ -4438,35 +4471,38 @@ export default class MetamaskController extends EventEmitter {
* @returns [] accounts
*/
async connectHardware(deviceName, page, hdPath) {
const keyring = await this.getKeyringForDevice(deviceName, hdPath);

if (deviceName === HardwareDeviceNames.ledger) {
await this.setLedgerTransportPreference(keyring);
}
return this.withKeyringForDevice(
{ name: deviceName, hdPath },
async (keyring) => {
if (deviceName === HardwareDeviceNames.ledger) {
await this.setLedgerTransportPreference(keyring);
}

let accounts = [];
switch (page) {
case -1:
accounts = await keyring.getPreviousPage();
break;
case 1:
accounts = await keyring.getNextPage();
break;
default:
accounts = await keyring.getFirstPage();
}
let accounts = [];
switch (page) {
case -1:
accounts = await keyring.getPreviousPage();
break;
case 1:
accounts = await keyring.getNextPage();
break;
default:
accounts = await keyring.getFirstPage();
}

// Merge with existing accounts
// and make sure addresses are not repeated
const oldAccounts = await this.keyringController.getAccounts();
// Merge with existing accounts
// and make sure addresses are not repeated
const oldAccounts = await this.keyringController.getAccounts();

const accountsToTrack = [
...new Set(
oldAccounts.concat(accounts.map((a) => a.address.toLowerCase())),
),
];
this.accountTracker.syncWithAddresses(accountsToTrack);
return accounts;
const accountsToTrack = [
...new Set(
oldAccounts.concat(accounts.map((a) => a.address.toLowerCase())),
),
];
this.accountTracker.syncWithAddresses(accountsToTrack);
return accounts;
},
);
}

/**
Expand All @@ -4477,8 +4513,12 @@ export default class MetamaskController extends EventEmitter {
* @returns {Promise<boolean>}
*/
async checkHardwareStatus(deviceName, hdPath) {
const keyring = await this.getKeyringForDevice(deviceName, hdPath);
return keyring.isUnlocked();
return this.withKeyringForDevice(
{ name: deviceName, hdPath },
async (keyring) => {
return keyring.isUnlocked();
},
);
}

/**
Expand All @@ -4488,14 +4528,15 @@ export default class MetamaskController extends EventEmitter {
* @returns {Promise<boolean>}
*/
async forgetDevice(deviceName) {
const keyring = await this.getKeyringForDevice(deviceName);
return this.withKeyringForDevice({ name: deviceName }, async (keyring) => {
for (const address of keyring.accounts) {
await this._onAccountRemoved(address);
}

for (const address of keyring.accounts) {
await this.removeAccount(address);
}
keyring.forgetDevice();

keyring.forgetDevice();
return true;
return true;
});
}

/**
Expand Down Expand Up @@ -4533,21 +4574,22 @@ export default class MetamaskController extends EventEmitter {
* @returns {'ledger' | 'lattice' | string | undefined}
*/
async getDeviceModel(address) {
const keyring = await this.keyringController.getKeyringForAccount(address);
switch (keyring.type) {
case KeyringType.trezor:
return keyring.getModel();
case KeyringType.qr:
return keyring.getName();
case KeyringType.ledger:
// TODO: get model after ledger keyring exposes method
return HardwareDeviceNames.ledger;
case KeyringType.lattice:
// TODO: get model after lattice keyring exposes method
return HardwareDeviceNames.lattice;
default:
return undefined;
}
return this.keyringController.withKeyring({ address }, async (keyring) => {
switch (keyring.type) {
case KeyringType.trezor:
return keyring.getModel();
case KeyringType.qr:
return keyring.getName();
case KeyringType.ledger:
// TODO: get model after ledger keyring exposes method
return HardwareDeviceNames.ledger;
case KeyringType.lattice:
// TODO: get model after lattice keyring exposes method
return HardwareDeviceNames.lattice;
default:
return undefined;
}
});
}

/**
Expand Down Expand Up @@ -4577,16 +4619,24 @@ export default class MetamaskController extends EventEmitter {
hdPath,
hdPathDescription,
) {
const keyring = await this.getKeyringForDevice(deviceName, hdPath);

keyring.setAccountToUnlock(index);
const unlockedAccount =
await this.keyringController.addNewAccountForKeyring(keyring);
const label = this.getAccountLabel(
deviceName === HardwareDeviceNames.qr ? keyring.getName() : deviceName,
index,
hdPathDescription,
const { address: unlockedAccount, label } = await this.withKeyringForDevice(
{ name: deviceName, hdPath },
async (keyring) => {
keyring.setAccountToUnlock(index);
const [address] = await keyring.addAccounts(1);
return {
address: normalize(address),
label: this.getAccountLabel(
deviceName === HardwareDeviceNames.qr
? keyring.getName()
: deviceName,
index,
hdPathDescription,
),
};
},
);

// Set the account label to Trezor 1 / Ledger 1 / QR Hardware 1, etc
this.preferencesController.setAccountLabel(unlockedAccount, label);
// Select the account
Expand Down Expand Up @@ -4763,20 +4813,8 @@ export default class MetamaskController extends EventEmitter {
* @param {string[]} address - A hex address
*/
async removeAccount(address) {
// Remove all associated permissions
this.removeAllAccountPermissions(address);

///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
this.custodyController.removeAccount(address);
///: END:ONLY_INCLUDE_IF(build-mmi)

const keyring = await this.keyringController.getKeyringForAccount(address);
// Remove account from the keyring
await this._onAccountRemoved(address);
await this.keyringController.removeAccount(address);
const updatedKeyringAccounts = keyring ? await keyring.getAccounts() : {};
if (updatedKeyringAccounts?.length === 0) {
keyring.destroy?.();
}

return address;
}
Expand Down Expand Up @@ -6025,6 +6063,21 @@ export default class MetamaskController extends EventEmitter {
this._notifyChainChange();
}

/**
* Execute side effects of a removed account.
*
* @param {string} address - The address of the account to remove.
* @returns {Promise<void>}
*/
async _onAccountRemoved(address) {
// Remove all associated permissions
this.removeAllAccountPermissions(address);

///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
this.custodyController.removeAccount(address);
///: END:ONLY_INCLUDE_IF(build-mmi)
}

// misc

/**
Expand Down Expand Up @@ -6306,16 +6359,15 @@ export default class MetamaskController extends EventEmitter {
/**
* Sets the Ledger Live preference to use for Ledger hardware wallet support
*
* @param _keyring
* @param keyring
* @deprecated This method is deprecated and will be removed in the future.
* Only webhid connections are supported in chrome and u2f in firefox.
*/
async setLedgerTransportPreference(_keyring) {
async setLedgerTransportPreference(keyring) {
const transportType = window.navigator.hid
? LedgerTransportTypes.webhid
: LedgerTransportTypes.u2f;
const keyring =
_keyring || (await this.getKeyringForDevice(HardwareDeviceNames.ledger));

if (keyring?.updateTransportMethod) {
return keyring.updateTransportMethod(transportType).catch((e) => {
throw e;
Expand Down
Loading

0 comments on commit e323e79

Please sign in to comment.