diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index 18a0c2319d1..7dfa9fe8bf2 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -25,13 +25,6 @@ import { import * as sinon from 'sinon'; import * as uuid from 'uuid'; -import MockEncryptor, { - MOCK_ENCRYPTION_KEY, -} from '../tests/mocks/mockEncryptor'; -import { MockErc4337Keyring } from '../tests/mocks/mockErc4337Keyring'; -import { MockKeyring } from '../tests/mocks/mockKeyring'; -import MockShallowGetAccountsKeyring from '../tests/mocks/mockShallowGetAccountsKeyring'; -import { buildMockTransaction } from '../tests/mocks/mockTransaction'; import { KeyringControllerError } from './constants'; import type { KeyringControllerEvents, @@ -47,6 +40,13 @@ import { isCustodyKeyring, keyringBuilderFactory, } from './KeyringController'; +import MockEncryptor, { + MOCK_ENCRYPTION_KEY, +} from '../tests/mocks/mockEncryptor'; +import { MockErc4337Keyring } from '../tests/mocks/mockErc4337Keyring'; +import { MockKeyring } from '../tests/mocks/mockKeyring'; +import MockShallowGetAccountsKeyring from '../tests/mocks/mockShallowGetAccountsKeyring'; +import { buildMockTransaction } from '../tests/mocks/mockTransaction'; jest.mock('uuid', () => { return { @@ -181,12 +181,10 @@ describe('KeyringController', () => { it('should not add a new account if called twice with the same accountCount param', async () => { await withController(async ({ controller, initialState }) => { const accountCount = initialState.keyrings[0].accounts.length; - const firstAccountAdded = await controller.addNewAccount( - accountCount, - ); - const secondAccountAdded = await controller.addNewAccount( - accountCount, - ); + const firstAccountAdded = + await controller.addNewAccount(accountCount); + const secondAccountAdded = + await controller.addNewAccount(accountCount); expect(firstAccountAdded).toBe(secondAccountAdded); expect(controller.state.keyrings[0].accounts).toHaveLength( accountCount + 1, @@ -220,13 +218,11 @@ describe('KeyringController', () => { const accountCount = initialState.keyrings[0].accounts.length; // We add a new account for "index 1" (not existing yet) - const firstAccountAdded = await controller.addNewAccount( - accountCount, - ); + const firstAccountAdded = + await controller.addNewAccount(accountCount); // Adding an account for an existing index will return the existing account's address - const secondAccountAdded = await controller.addNewAccount( - accountCount, - ); + const secondAccountAdded = + await controller.addNewAccount(accountCount); expect(firstAccountAdded).toBe(secondAccountAdded); expect(controller.state.keyrings[0].accounts).toHaveLength( accountCount + 1, @@ -258,9 +254,8 @@ describe('KeyringController', () => { const [primaryKeyring] = controller.getKeyringsByType( KeyringTypes.hd, ) as Keyring[]; - const addedAccountAddress = await controller.addNewAccountForKeyring( - primaryKeyring, - ); + const addedAccountAddress = + await controller.addNewAccountForKeyring(primaryKeyring); expect(initialState.keyrings).toHaveLength(1); expect(initialState.keyrings[0].accounts).not.toStrictEqual( controller.state.keyrings[0].accounts, @@ -306,9 +301,8 @@ describe('KeyringController', () => { const [primaryKeyring] = controller.getKeyringsByType( KeyringTypes.hd, ) as Keyring[]; - const addedAccountAddress = await controller.addNewAccountForKeyring( - primaryKeyring, - ); + const addedAccountAddress = + await controller.addNewAccountForKeyring(primaryKeyring); expect(initialState.keyrings).toHaveLength(1); expect(initialState.keyrings[0].accounts).not.toStrictEqual( controller.state.keyrings[0].accounts, @@ -407,9 +401,8 @@ describe('KeyringController', () => { await withController( { cacheEncryptionKey }, async ({ controller, initialState }) => { - const currentSeedWord = await controller.exportSeedPhrase( - password, - ); + const currentSeedWord = + await controller.exportSeedPhrase(password); await controller.createNewVaultAndRestore( password, @@ -475,9 +468,8 @@ describe('KeyringController', () => { async ({ controller }) => { await controller.createNewVaultAndKeychain(password); - const currentSeedPhrase = await controller.exportSeedPhrase( - password, - ); + const currentSeedPhrase = + await controller.exportSeedPhrase(password); expect(currentSeedPhrase.length).toBeGreaterThan(0); expect( @@ -565,17 +557,15 @@ describe('KeyringController', () => { await withController( { cacheEncryptionKey }, async ({ controller, initialState }) => { - const initialSeedWord = await controller.exportSeedPhrase( - password, - ); + const initialSeedWord = + await controller.exportSeedPhrase(password); expect(initialSeedWord).toBeDefined(); const initialVault = controller.state.vault; await controller.createNewVaultAndKeychain(password); - const currentSeedWord = await controller.exportSeedPhrase( - password, - ); + const currentSeedWord = + await controller.exportSeedPhrase(password); expect(initialState).toStrictEqual(controller.state); expect(initialSeedWord).toBe(currentSeedWord); expect(initialVault).toStrictEqual(controller.state.vault); @@ -2556,21 +2546,18 @@ describe('KeyringController', () => { ), ); - const firstPage = await signProcessKeyringController.connectQRHardware( - 0, - ); + const firstPage = + await signProcessKeyringController.connectQRHardware(0); expect(firstPage).toHaveLength(5); expect(firstPage[0].index).toBe(0); - const secondPage = await signProcessKeyringController.connectQRHardware( - 1, - ); + const secondPage = + await signProcessKeyringController.connectQRHardware(1); expect(secondPage).toHaveLength(5); expect(secondPage[0].index).toBe(5); - const goBackPage = await signProcessKeyringController.connectQRHardware( - -1, - ); + const goBackPage = + await signProcessKeyringController.connectQRHardware(-1); expect(goBackPage).toStrictEqual(firstPage); await signProcessKeyringController.unlockQRHardwareWalletAccount(0); diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index bf5b853aeeb..729cbc52464 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -52,32 +52,19 @@ const name = 'KeyringController'; * Available keyring types */ export enum KeyringTypes { - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention simple = 'Simple Key Pair', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention hd = 'HD Key Tree', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention qr = 'QR Hardware Wallet Device', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention trezor = 'Trezor Hardware', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention ledger = 'Ledger Hardware', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention lattice = 'Lattice Hardware', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention snap = 'Snap Keyring', } /** * Custody keyring types are a special case, as they are not a single type * but they all start with the prefix "Custody". + * * @param keyringType - The type of the keyring. * @returns Whether the keyring type is a custody keyring. */ @@ -86,15 +73,9 @@ export const isCustodyKeyring = (keyringType: string): boolean => { }; /** - * @type KeyringControllerState + * KeyringControllerState * - * Keyring controller state - * @property vault - Encrypted string representing keyring data - * @property isUnlocked - Whether vault is unlocked - * @property keyringTypes - Account types - * @property keyrings - Group of accounts - * @property encryptionKey - Keyring encryption key - * @property encryptionSalt - Keyring encryption salt + * The KeyringController state */ export type KeyringControllerState = { vault?: string; @@ -251,11 +232,9 @@ export type KeyringControllerOptions = { ); /** - * @type KeyringObject + * KeyringObject * * Keyring object to return in fullUpdate - * @property type - Keyring type - * @property accounts - Associated accounts */ export type KeyringObject = { accounts: string[]; @@ -266,11 +245,7 @@ export type KeyringObject = { * A strategy for importing an account */ export enum AccountImportStrategy { - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention privateKey = 'privateKey', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention json = 'json', } @@ -404,13 +379,11 @@ export type KeyringSelector = * * @param releaseLock - A function to release the lock. */ -// TODO: Either fix this lint violation or explain why it's necessary to ignore. -// eslint-disable-next-line @typescript-eslint/naming-convention -type MutuallyExclusiveCallback = ({ +type MutuallyExclusiveCallback = ({ releaseLock, }: { releaseLock: MutexInterface.Releaser; -}) => Promise; +}) => Promise; /** * Get builder function for `Keyring` @@ -586,17 +559,17 @@ export class KeyringController extends BaseController< readonly #vaultOperationMutex = new Mutex(); - #keyringBuilders: { (): EthKeyring; type: string }[]; + readonly #keyringBuilders: { (): EthKeyring; type: string }[]; - #keyrings: EthKeyring[]; + readonly #unsupportedKeyrings: SerializedKeyring[]; - #unsupportedKeyrings: SerializedKeyring[]; + readonly #encryptor: GenericEncryptor | ExportableKeyEncryptor; - #password?: string; + readonly #cacheEncryptionKey: boolean; - #encryptor: GenericEncryptor | ExportableKeyEncryptor; + #keyrings: EthKeyring[]; - #cacheEncryptionKey: boolean; + #password?: string; #qrKeyringStateListener?: ( state: ReturnType, @@ -765,6 +738,7 @@ export class KeyringController extends BaseController< * If there is a pre-existing locked vault, it will be replaced. * * @param password - Password to unlock the new vault. + * @returns Promise resolving when the operation ends successfully. */ async createNewVaultAndKeychain(password: string) { return this.#persistOrRollback(async () => { @@ -989,7 +963,7 @@ export class KeyringController extends BaseController< return this.#persistOrRollback(async () => { let privateKey; switch (strategy) { - case 'privateKey': + case AccountImportStrategy.privateKey: const [importedKey] = args; if (!importedKey) { throw new Error('Cannot import an empty key.'); @@ -1013,7 +987,7 @@ export class KeyringController extends BaseController< privateKey = remove0x(prefixed); break; - case 'json': + case AccountImportStrategy.json: let wallet; const [input, password] = args; try { @@ -1024,7 +998,7 @@ export class KeyringController extends BaseController< privateKey = bytesToHex(wallet.getPrivateKey()); break; default: - throw new Error(`Unexpected import strategy: '${strategy}'`); + throw new Error(`Unexpected import strategy: '${String(strategy)}'`); } const newKeyring = (await this.#newKeyring(KeyringTypes.simple, [ privateKey, @@ -1054,7 +1028,6 @@ export class KeyringController extends BaseController< // The `removeAccount` method of snaps keyring is async. We have to update // the interface of the other keyrings to be async as well. - // eslint-disable-next-line @typescript-eslint/await-thenable // FIXME: We do cast to `Hex` to makes the type checker happy here, and // because `Keyring.removeAccount` requires address to be `Hex`. Those // type would need to be updated for a full non-EVM support. @@ -2203,7 +2176,6 @@ export class KeyringController extends BaseController< // cases and allow retro-compatibility too. // FIXME: For some reason, it seems that eslint is complaining about this call being non-thenable // even though it is... For now, we just disable it: - // eslint-disable-next-line @typescript-eslint/await-thenable await keyring.generateRandomMnemonic(); await keyring.addAccounts(1); } @@ -2353,14 +2325,14 @@ export class KeyringController extends BaseController< * and save the keyrings to state after it, or rollback to their * previous state in case of error. * - * @param fn - The function to execute. + * @param callback - The function to execute. * @returns The result of the function. */ - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - async #persistOrRollback(fn: MutuallyExclusiveCallback): Promise { + async #persistOrRollback( + callback: MutuallyExclusiveCallback, + ): Promise { return this.#withRollback(async ({ releaseLock }) => { - const callbackResult = await fn({ releaseLock }); + const callbackResult = await callback({ releaseLock }); // State is committed only if the operation is successful await this.#updateVault(); @@ -2372,18 +2344,18 @@ export class KeyringController extends BaseController< * Execute the given function after acquiring the controller lock * and rollback keyrings and password states in case of error. * - * @param fn - The function to execute atomically. + * @param callback - The function to execute atomically. * @returns The result of the function. */ - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - async #withRollback(fn: MutuallyExclusiveCallback): Promise { + async #withRollback( + callback: MutuallyExclusiveCallback, + ): Promise { return this.#withControllerLock(async ({ releaseLock }) => { const currentSerializedKeyrings = await this.#getSerializedKeyrings(); const currentPassword = this.#password; try { - return await fn({ releaseLock }); + return await callback({ releaseLock }); } catch (e) { // Keyrings and password are restored to their previous state await this.#restoreSerializedKeyrings(currentSerializedKeyrings); @@ -2414,13 +2386,13 @@ export class KeyringController extends BaseController< * controller and that changes its state is executed in a mutually exclusive way, * preventing unsafe concurrent access that could lead to unpredictable behavior. * - * @param fn - The function to execute while the controller mutex is locked. + * @param callback - The function to execute while the controller mutex is locked. * @returns The result of the function. */ - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - async #withControllerLock(fn: MutuallyExclusiveCallback): Promise { - return withLock(this.#controllerOperationMutex, fn); + async #withControllerLock( + callback: MutuallyExclusiveCallback, + ): Promise { + return withLock(this.#controllerOperationMutex, callback); } /** @@ -2431,15 +2403,15 @@ export class KeyringController extends BaseController< * This ensures that each operation that interacts with the vault * is executed in a mutually exclusive way. * - * @param fn - The function to execute while the vault mutex is locked. + * @param callback - The function to execute while the vault mutex is locked. * @returns The result of the function. */ - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - async #withVaultLock(fn: MutuallyExclusiveCallback): Promise { + async #withVaultLock( + callback: MutuallyExclusiveCallback, + ): Promise { this.#assertControllerMutexIsLocked(); - return withLock(this.#vaultOperationMutex, fn); + return withLock(this.#vaultOperationMutex, callback); } } @@ -2449,19 +2421,17 @@ export class KeyringController extends BaseController< * error is thrown. * * @param mutex - The mutex to lock. - * @param fn - The function to execute while the mutex is locked. + * @param callback - The function to execute while the mutex is locked. * @returns The result of the function. */ -// TODO: Either fix this lint violation or explain why it's necessary to ignore. -// eslint-disable-next-line @typescript-eslint/naming-convention -async function withLock( +async function withLock( mutex: Mutex, - fn: MutuallyExclusiveCallback, -): Promise { + callback: MutuallyExclusiveCallback, +): Promise { const releaseLock = await mutex.acquire(); try { - return await fn({ releaseLock }); + return await callback({ releaseLock }); } finally { releaseLock(); }