diff --git a/packages/keyring-controller/jest.config.js b/packages/keyring-controller/jest.config.js index 3dbee998978..53a583464ab 100644 --- a/packages/keyring-controller/jest.config.js +++ b/packages/keyring-controller/jest.config.js @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 95.51, + branches: 94.26, functions: 100, - lines: 99.07, - statements: 99.08, + lines: 98.96, + statements: 98.98, }, }, diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index bbba030eee3..b9fab0c14a0 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -191,17 +191,30 @@ describe('KeyringController', () => { ); }); }); - }); - it('should throw error with no HD keyring', async () => { - await withController( - { skipVaultCreation: true }, - async ({ controller }) => { + it('should throw an error if there is no primary keyring', async () => { + await withController(async ({ controller, encryptor }) => { + await controller.setLocked(); + jest + .spyOn(encryptor, 'decrypt') + .mockResolvedValueOnce([{ type: 'Unsupported', data: '' }]); + await controller.submitPassword('123'); + await expect(controller.addNewAccount()).rejects.toThrow( 'No HD keyring found', ); - }, - ); + }); + }); + }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.addNewAccount()).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); }); // Testing fix for bug #4157 {@link https://github.com/MetaMask/core/issues/4157} @@ -353,6 +366,17 @@ describe('KeyringController', () => { }); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + const keyring = controller.getKeyringsByType(KeyringTypes.hd)[0]; + await controller.setLocked(); + + await expect( + controller.addNewAccountForKeyring(keyring as EthKeyring), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('addNewKeyring', () => { @@ -376,6 +400,16 @@ describe('KeyringController', () => { }); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.addNewKeyring(KeyringTypes.hd)).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('createNewVaultAndRestore', () => { @@ -630,6 +664,16 @@ describe('KeyringController', () => { expect(listener.called).toBe(true); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.setLocked()).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('exportSeedPhrase', () => { @@ -672,6 +716,16 @@ describe('KeyringController', () => { }); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.exportSeedPhrase(password)).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('exportAccount', () => { @@ -751,6 +805,16 @@ describe('KeyringController', () => { expect(accounts).toStrictEqual(initialAccount); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.getAccounts()).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('getEncryptionPublicKey', () => { @@ -792,6 +856,18 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + await controller.setLocked(); + + await expect( + controller.getEncryptionPublicKey( + initialState.keyrings[0].accounts[0], + ), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('decryptMessage', () => { @@ -868,6 +944,24 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + await controller.setLocked(); + + await expect( + controller.decryptMessage({ + from: initialState.keyrings[0].accounts[0], + data: { + version: '1.0', + nonce: '123456', + ephemPublicKey: '0xabcdef1234567890', + ciphertext: '0xabcdef1234567890', + }, + }), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('getKeyringForAccount', () => { @@ -889,7 +983,7 @@ describe('KeyringController', () => { }); describe('when non-existing account is provided', () => { - it('should throw error', async () => { + it('should throw error if no account matches the address', async () => { await withController(async ({ controller }) => { await expect( controller.getKeyringForAccount( @@ -901,19 +995,44 @@ describe('KeyringController', () => { }); }); - it('should throw an error if there are no keyrings', async () => { - await withController( - { skipVaultCreation: true }, - async ({ controller }) => { - await expect( - controller.getKeyringForAccount( - '0x51253087e6f8358b5f10c0a94315d69db3357859', - ), - ).rejects.toThrow( - 'KeyringController - No keyring found. Error info: There are no keyrings', - ); - }, - ); + it('should throw an error if there is no keyring', async () => { + await withController(async ({ controller, encryptor }) => { + await controller.setLocked(); + jest + .spyOn(encryptor, 'decrypt') + .mockResolvedValueOnce([{ type: 'Unsupported', data: '' }]); + await controller.submitPassword('123'); + + await expect( + controller.getKeyringForAccount( + '0x0000000000000000000000000000000000000000', + ), + ).rejects.toThrow( + 'KeyringController - No keyring found. Error info: There are no keyrings', + ); + }); + }); + + it('should throw an error if the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect( + controller.getKeyringForAccount( + '0x51253087e6f8358b5f10c0a94315d69db3357859', + ), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); + }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + await controller.setLocked(); + + await expect( + controller.getKeyringForAccount(initialState.keyrings[0].accounts[0]), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); }); }); }); @@ -942,6 +1061,16 @@ describe('KeyringController', () => { }); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + expect(() => controller.getKeyringsByType(KeyringTypes.hd)).toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('persistAllKeyrings', () => { @@ -963,7 +1092,7 @@ describe('KeyringController', () => { await controller.setLocked(); await expect(controller.persistAllKeyrings()).rejects.toThrow( - KeyringControllerError.MissingCredentials, + KeyringControllerError.ControllerLocked, ); }); }); @@ -1152,6 +1281,19 @@ describe('KeyringController', () => { }); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect( + controller.importAccountWithStrategy( + AccountImportStrategy.privateKey, + [input, 'password'], + ), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('removeAccount', () => { @@ -1246,6 +1388,16 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + await controller.setLocked(); + + await expect( + controller.removeAccount(initialState.keyrings[0].accounts[0]), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('signMessage', () => { @@ -1309,6 +1461,20 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + await controller.setLocked(); + + await expect( + controller.signMessage({ + from: initialState.keyrings[0].accounts[0], + data: '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0', + origin: 'https://metamask.github.io', + }), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('signPersonalMessage', () => { @@ -1379,6 +1545,20 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + await controller.setLocked(); + + await expect( + controller.signPersonalMessage({ + from: initialState.keyrings[0].accounts[0], + data: '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0', + origin: 'https://metamask.github.io', + }), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('signTypedMessage', () => { @@ -1652,6 +1832,34 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + await controller.setLocked(); + + await expect( + controller.signTypedMessage( + { + from: initialState.keyrings[0].accounts[0], + data: [ + { + type: 'string', + name: 'Message', + value: 'Hi, Alice!', + }, + { + type: 'uint32', + name: 'A number', + value: '1337', + }, + ], + origin: 'https://metamask.github.io', + }, + SignTypedDataVersion.V1, + ), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('signTransaction', () => { @@ -1740,6 +1948,19 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + await controller.setLocked(); + + await expect( + controller.signTransaction( + buildMockTransaction(), + initialState.keyrings[0].accounts[0], + ), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('prepareUserOperation', () => { @@ -1818,6 +2039,20 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + await controller.setLocked(); + + await expect( + controller.prepareUserOperation( + initialState.keyrings[0].accounts[0], + [], + executionContext, + ), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('patchUserOperation', () => { @@ -1905,6 +2140,32 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + await controller.setLocked(); + + await expect( + controller.patchUserOperation( + initialState.keyrings[0].accounts[0], + { + sender: '0x4584d2B4905087A100420AFfCe1b2d73fC69B8E4', + nonce: '0x1', + initCode: '0x', + callData: '0x7064', + callGasLimit: '0x58a83', + verificationGasLimit: '0xe8c4', + preVerificationGas: '0xc57c', + maxFeePerGas: '0x87f0878c0', + maxPriorityFeePerGas: '0x1dcd6500', + paymasterAndData: '0x', + signature: '0x', + }, + executionContext, + ), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('signUserOperation', () => { @@ -1989,6 +2250,32 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + await controller.setLocked(); + + await expect( + controller.signUserOperation( + initialState.keyrings[0].accounts[0], + { + sender: '0x4584d2B4905087A100420AFfCe1b2d73fC69B8E4', + nonce: '0x1', + initCode: '0x', + callData: '0x7064', + callGasLimit: '0x58a83', + verificationGasLimit: '0xe8c4', + preVerificationGas: '0xc57c', + maxFeePerGas: '0x87f0878c0', + maxPriorityFeePerGas: '0x1dcd6500', + paymasterAndData: '0x', + signature: '0x', + }, + executionContext, + ), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('changePassword', () => { @@ -2018,9 +2305,9 @@ describe('KeyringController', () => { async ({ controller }) => { await controller.setLocked(); - await expect(controller.changePassword('')).rejects.toThrow( - KeyringControllerError.MissingCredentials, - ); + await expect(async () => + controller.changePassword(''), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); }, ); }); @@ -2047,6 +2334,16 @@ describe('KeyringController', () => { }, ); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(async () => + controller.changePassword('whatever'), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }), ); }); @@ -2265,16 +2562,40 @@ describe('KeyringController', () => { }); }); - it('should throw error with no HD keyring', async () => { + it('should throw error if the controller is locked', async () => { await withController( { skipVaultCreation: true }, async ({ controller }) => { await expect(controller.verifySeedPhrase()).rejects.toThrow( - 'No HD keyring found', + KeyringControllerError.ControllerLocked, ); }, ); }); + + it('should throw an error if there is no primary keyring', async () => { + await withController(async ({ controller, encryptor }) => { + await controller.setLocked(); + jest + .spyOn(encryptor, 'decrypt') + .mockResolvedValueOnce([{ type: 'Unsupported', data: '' }]); + await controller.submitPassword('123'); + + await expect(controller.verifySeedPhrase()).rejects.toThrow( + 'No HD keyring found', + ); + }); + }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.verifySeedPhrase()).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('verifyPassword', () => { @@ -2457,6 +2778,18 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect( + controller.withKeyring({ type: KeyringTypes.hd }, async (keyring) => + keyring.getAccounts(), + ), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('QR keyring', () => { @@ -2533,6 +2866,16 @@ describe('KeyringController', () => { expect(qrKeyring).toBeUndefined(); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + expect(() => controller.getQRKeyring()).toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('connectQRHardware', () => { @@ -2569,6 +2912,16 @@ describe('KeyringController', () => { ); expect(qrKeyring?.accounts).toHaveLength(3); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.connectQRHardware(0)).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('signMessage', () => { @@ -2795,6 +3148,16 @@ describe('KeyringController', () => { .sign.request, ).toBeUndefined(); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.resetQRKeyringState()).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('forgetQRDevice', () => { @@ -2825,6 +3188,16 @@ describe('KeyringController', () => { expect(remainingAccounts).toHaveLength(0); }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.forgetQRDevice()).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('restoreQRKeyring', () => { @@ -2859,6 +3232,39 @@ describe('KeyringController', () => { signProcessKeyringController.state.keyrings[1].accounts, ).toHaveLength(1); }); + + it('should throw error when the controller is locked', async () => { + const serializedQRKeyring = { + initialized: true, + accounts: ['0xE410157345be56688F43FF0D9e4B2B38Ea8F7828'], + currentAccount: 0, + page: 0, + perPage: 5, + keyringAccount: 'account.standard', + keyringMode: 'hd', + name: 'Keystone', + version: 1, + xfp: '5271c071', + xpub: 'xpub6CNhtuXAHDs84AhZj5ALZB6ii4sP5LnDXaKDSjiy6kcBbiysq89cDrLG29poKvZtX9z4FchZKTjTyiPuDeiFMUd1H4g5zViQxt4tpkronJr', + hdPath: "m/44'/60'/0'", + childrenPath: '0/*', + indexes: { + '0xE410157345be56688F43FF0D9e4B2B38Ea8F7828': 0, + '0xEEACb7a5e53600c144C0b9839A834bb4b39E540c': 1, + '0xA116800A72e56f91cF1677D40C9984f9C9f4B2c7': 2, + '0x4826BadaBC9894B3513e23Be408605611b236C0f': 3, + '0x8a1503beb17Ef02cC4Ff288b0A73583c4ce547c7': 4, + }, + paths: {}, + }; + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect( + controller.restoreQRKeyring(serializedQRKeyring), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('getAccountKeyringType', () => { @@ -2875,6 +3281,16 @@ describe('KeyringController', () => { await signProcessKeyringController.getAccountKeyringType(qrAccount), ).toBe(KeyringTypes.qr); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.getAccountKeyringType('0x0')).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('submitQRCryptoHDKey', () => { @@ -2891,6 +3307,16 @@ describe('KeyringController', () => { await signProcessKeyringController.submitQRCryptoHDKey('anything'); expect(submitCryptoHDKeyStub.calledWith('anything')).toBe(true); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.submitQRCryptoHDKey('0x0')).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('submitQRCryptoAccount', () => { @@ -2907,6 +3333,16 @@ describe('KeyringController', () => { await signProcessKeyringController.submitQRCryptoAccount('anything'); expect(submitCryptoAccountStub.calledWith('anything')).toBe(true); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + + await expect(controller.submitQRCryptoAccount('0x0')).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); }); describe('submitQRSignature', () => { diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 138fb6e6e4f..b04582ee3ca 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -655,6 +655,8 @@ export class KeyringController extends BaseController< * @returns Promise resolving to the added account address. */ async addNewAccount(accountCount?: number): Promise { + this.#assertIsUnlocked(); + return this.#persistOrRollback(async () => { const primaryKeyring = this.getKeyringsByType('HD Key Tree')[0] as | EthKeyring @@ -662,6 +664,7 @@ export class KeyringController extends BaseController< if (!primaryKeyring) { throw new Error('No HD keyring found'); } + const oldAccounts = await primaryKeyring.getAccounts(); if (accountCount && oldAccounts.length !== accountCount) { @@ -700,6 +703,8 @@ export class KeyringController extends BaseController< // We still uses `Hex` here, since we are not using this method when creating // and account using a "Snap Keyring". This function assume the `keyring` is // ethereum compatible, but "Snap Keyring" might not be. + this.#assertIsUnlocked(); + return this.#persistOrRollback(async () => { const oldAccounts = await this.#getAccountsFromKeyrings(); @@ -783,6 +788,8 @@ export class KeyringController extends BaseController< type: KeyringTypes | string, opts?: unknown, ): Promise { + this.#assertIsUnlocked(); + if (type === KeyringTypes.qr) { return this.getOrAddQRKeyring(); } @@ -819,6 +826,7 @@ export class KeyringController extends BaseController< * @returns Promise resolving to the seed phrase. */ async exportSeedPhrase(password: string): Promise { + this.#assertIsUnlocked(); await this.verifyPassword(password); assertHasUint8ArrayMnemonic(this.#keyrings[0]); return this.#keyrings[0].mnemonic; @@ -850,6 +858,7 @@ export class KeyringController extends BaseController< * @returns A promise resolving to an array of addresses. */ async getAccounts(): Promise { + this.#assertIsUnlocked(); return this.state.keyrings.reduce( (accounts, keyring) => accounts.concat(keyring.accounts), [], @@ -868,6 +877,7 @@ export class KeyringController extends BaseController< account: string, opts?: Record, ): Promise { + this.#assertIsUnlocked(); const address = ethNormalize(account) as Hex; const keyring = (await this.getKeyringForAccount( account, @@ -891,6 +901,7 @@ export class KeyringController extends BaseController< from: string; data: Eip1024EncryptedData; }): Promise { + this.#assertIsUnlocked(); const address = ethNormalize(messageParams.from) as Hex; const keyring = (await this.getKeyringForAccount( address, @@ -913,6 +924,7 @@ export class KeyringController extends BaseController< * @returns Promise resolving to keyring of the `account` if one exists. */ async getKeyringForAccount(account: string): Promise { + this.#assertIsUnlocked(); const address = normalize(account); const candidates = await Promise.all( @@ -952,6 +964,7 @@ export class KeyringController extends BaseController< * @returns An array of keyrings of the given type. */ getKeyringsByType(type: KeyringTypes | string): unknown[] { + this.#assertIsUnlocked(); return this.#keyrings.filter((keyring) => keyring.type === type); } @@ -963,6 +976,7 @@ export class KeyringController extends BaseController< * operation completes. */ async persistAllKeyrings(): Promise { + this.#assertIsUnlocked(); return this.#persistOrRollback(async () => true); } @@ -980,6 +994,7 @@ export class KeyringController extends BaseController< // eslint-disable-next-line @typescript-eslint/no-explicit-any args: any[], ): Promise { + this.#assertIsUnlocked(); return this.#persistOrRollback(async () => { let privateKey; switch (strategy) { @@ -1036,6 +1051,8 @@ export class KeyringController extends BaseController< * @returns Promise resolving when the account is removed. */ async removeAccount(address: string): Promise { + this.#assertIsUnlocked(); + await this.#persistOrRollback(async () => { const keyring = (await this.getKeyringForAccount( address, @@ -1069,6 +1086,8 @@ export class KeyringController extends BaseController< * @returns Promise resolving when the operation completes. */ async setLocked(): Promise { + this.#assertIsUnlocked(); + return this.#withRollback(async () => { this.#unsubscribeFromQRKeyringsEvents(); @@ -1093,6 +1112,8 @@ export class KeyringController extends BaseController< * @returns Promise resolving to a signed message string. */ async signMessage(messageParams: PersonalMessageParams): Promise { + this.#assertIsUnlocked(); + if (!messageParams.data) { throw new Error("Can't sign an empty message"); } @@ -1115,6 +1136,7 @@ export class KeyringController extends BaseController< * @returns Promise resolving to a signed message string. */ async signPersonalMessage(messageParams: PersonalMessageParams) { + this.#assertIsUnlocked(); const address = ethNormalize(messageParams.from) as Hex; const keyring = (await this.getKeyringForAccount( address, @@ -1140,6 +1162,8 @@ export class KeyringController extends BaseController< messageParams: TypedMessageParams, version: SignTypedDataVersion, ): Promise { + this.#assertIsUnlocked(); + try { if ( ![ @@ -1189,6 +1213,7 @@ export class KeyringController extends BaseController< from: string, opts?: Record, ): Promise { + this.#assertIsUnlocked(); const address = ethNormalize(from) as Hex; const keyring = (await this.getKeyringForAccount( address, @@ -1213,6 +1238,7 @@ export class KeyringController extends BaseController< transactions: EthBaseTransaction[], executionContext: KeyringExecutionContext, ): Promise { + this.#assertIsUnlocked(); const address = ethNormalize(from) as Hex; const keyring = (await this.getKeyringForAccount( address, @@ -1243,6 +1269,7 @@ export class KeyringController extends BaseController< userOp: EthUserOperation, executionContext: KeyringExecutionContext, ): Promise { + this.#assertIsUnlocked(); const address = ethNormalize(from) as Hex; const keyring = (await this.getKeyringForAccount( address, @@ -1268,6 +1295,7 @@ export class KeyringController extends BaseController< userOp: EthUserOperation, executionContext: KeyringExecutionContext, ): Promise { + this.#assertIsUnlocked(); const address = ethNormalize(from) as Hex; const keyring = (await this.getKeyringForAccount( address, @@ -1287,11 +1315,8 @@ export class KeyringController extends BaseController< * @returns Promise resolving when the operation completes. */ changePassword(password: string): Promise { + this.#assertIsUnlocked(); return this.#persistOrRollback(async () => { - if (!this.state.isUnlocked) { - throw new Error(KeyringControllerError.MissingCredentials); - } - assertIsValidPassword(password); this.#password = password; @@ -1349,6 +1374,7 @@ export class KeyringController extends BaseController< * @returns Promise resolving to the seed phrase as Uint8Array. */ async verifySeedPhrase(): Promise { + this.#assertIsUnlocked(); return this.#withControllerLock(async () => this.#verifySeedPhrase()); } @@ -1418,6 +1444,8 @@ export class KeyringController extends BaseController< createIfMissing: false, }, ): Promise { + this.#assertIsUnlocked(); + return this.#persistOrRollback(async () => { let keyring: SelectedKeyring | undefined; @@ -1465,6 +1493,7 @@ export class KeyringController extends BaseController< * @deprecated Use `withKeyring` instead. */ getQRKeyring(): QRKeyring | undefined { + this.#assertIsUnlocked(); // QRKeyring is not yet compatible with Keyring type from @metamask/utils return this.getKeyringsByType(KeyringTypes.qr)[0] as unknown as QRKeyring; } @@ -1476,6 +1505,8 @@ export class KeyringController extends BaseController< * @deprecated Use `addNewKeyring` and `withKeyring` instead. */ async getOrAddQRKeyring(): Promise { + this.#assertIsUnlocked(); + return ( this.getQRKeyring() || (await this.#persistOrRollback(async () => this.#addQRKeyring())) @@ -1492,6 +1523,8 @@ export class KeyringController extends BaseController< // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any async restoreQRKeyring(serialized: any): Promise { + this.#assertIsUnlocked(); + return this.#persistOrRollback(async () => { const keyring = this.getQRKeyring() || (await this.#addQRKeyring()); keyring.deserialize(serialized); @@ -1505,6 +1538,8 @@ export class KeyringController extends BaseController< * @deprecated Use `withKeyring` instead. */ async resetQRKeyringState(): Promise { + this.#assertIsUnlocked(); + (await this.getOrAddQRKeyring()).resetStore(); } @@ -1516,6 +1551,8 @@ export class KeyringController extends BaseController< * instead. */ async getQRKeyringState(): Promise { + this.#assertIsUnlocked(); + return (await this.getOrAddQRKeyring()).getMemStore(); } @@ -1527,6 +1564,8 @@ export class KeyringController extends BaseController< * @deprecated Use `withKeyring` instead. */ async submitQRCryptoHDKey(cryptoHDKey: string): Promise { + this.#assertIsUnlocked(); + (await this.getOrAddQRKeyring()).submitCryptoHDKey(cryptoHDKey); } @@ -1538,6 +1577,8 @@ export class KeyringController extends BaseController< * @deprecated Use `withKeyring` instead. */ async submitQRCryptoAccount(cryptoAccount: string): Promise { + this.#assertIsUnlocked(); + (await this.getOrAddQRKeyring()).submitCryptoAccount(cryptoAccount); } @@ -1553,6 +1594,8 @@ export class KeyringController extends BaseController< requestId: string, ethSignature: string, ): Promise { + this.#assertIsUnlocked(); + (await this.getOrAddQRKeyring()).submitSignature(requestId, ethSignature); } @@ -1563,6 +1606,8 @@ export class KeyringController extends BaseController< * @deprecated Use `withKeyring` instead. */ async cancelQRSignRequest(): Promise { + this.#assertIsUnlocked(); + (await this.getOrAddQRKeyring()).cancelSignRequest(); } @@ -1573,6 +1618,8 @@ export class KeyringController extends BaseController< * @deprecated Use `withKeyring` instead. */ async cancelQRSynchronization(): Promise { + this.#assertIsUnlocked(); + (await this.getOrAddQRKeyring()).cancelSync(); } @@ -1588,6 +1635,8 @@ export class KeyringController extends BaseController< async connectQRHardware( page: number, ): Promise<{ balance: string; address: string; index: number }[]> { + this.#assertIsUnlocked(); + return this.#persistOrRollback(async () => { try { const keyring = this.getQRKeyring() || (await this.#addQRKeyring()); @@ -1628,6 +1677,8 @@ export class KeyringController extends BaseController< * @deprecated Use `withKeyring` instead. */ async unlockQRHardwareWalletAccount(index: number): Promise { + this.#assertIsUnlocked(); + return this.#persistOrRollback(async () => { const keyring = this.getQRKeyring() || (await this.#addQRKeyring()); @@ -1637,6 +1688,8 @@ export class KeyringController extends BaseController< } async getAccountKeyringType(account: string): Promise { + this.#assertIsUnlocked(); + const keyring = (await this.getKeyringForAccount( account, )) as EthKeyring; @@ -1653,6 +1706,8 @@ export class KeyringController extends BaseController< removedAccounts: string[]; remainingAccounts: string[]; }> { + this.#assertIsUnlocked(); + return this.#persistOrRollback(async () => { const keyring = this.getQRKeyring(); @@ -2338,6 +2393,17 @@ export class KeyringController extends BaseController< this.messagingSystem.publish(`${name}:unlock`); } + /** + * Assert that the controller is unlocked. + * + * @throws If the controller is locked. + */ + #assertIsUnlocked(): void { + if (!this.state.isUnlocked) { + throw new Error(KeyringControllerError.ControllerLocked); + } + } + /** * Execute the given function after acquiring the controller lock * and save the keyrings to state after it, or rollback to their diff --git a/packages/keyring-controller/src/constants.ts b/packages/keyring-controller/src/constants.ts index fe58710cfaa..4da2115a417 100644 --- a/packages/keyring-controller/src/constants.ts +++ b/packages/keyring-controller/src/constants.ts @@ -24,6 +24,7 @@ export enum KeyringControllerError { UnsupportedPatchUserOperation = 'KeyringController - The keyring for the current address does not support the method patchUserOperation.', UnsupportedSignUserOperation = 'KeyringController - The keyring for the current address does not support the method signUserOperation.', NoAccountOnKeychain = "KeyringController - The keychain doesn't have accounts.", + ControllerLocked = 'KeyringController - The operation cannot be completed while the controller is locked.', MissingCredentials = 'KeyringController - Cannot persist vault without password and encryption key', MissingVaultData = 'KeyringController - Cannot persist vault without vault information', ExpiredCredentials = 'KeyringController - Encryption key and salt provided are expired',