diff --git a/packages/keyring-controller/package.json b/packages/keyring-controller/package.json index 9ed47a1b30..57f59be85b 100644 --- a/packages/keyring-controller/package.json +++ b/packages/keyring-controller/package.json @@ -38,7 +38,8 @@ "@metamask/preferences-controller": "workspace:^", "async-mutex": "^0.2.6", "ethereumjs-util": "^7.0.10", - "ethereumjs-wallet": "^1.0.1" + "ethereumjs-wallet": "^1.0.1", + "immer": "^9.0.6" }, "devDependencies": { "@ethereumjs/common": "^2.6.1", diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index 552642cf42..b0761d5cec 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -14,14 +14,17 @@ import * as uuid from 'uuid'; import { isValidHexAddress, NetworkType } from '@metamask/controller-utils'; import { keyringBuilderFactory } from '@metamask/eth-keyring-controller'; import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english'; +import { ControllerMessenger } from '@metamask/base-controller'; import MockEncryptor, { mockKey } from '../tests/mocks/mockEncryptor'; import { AccountImportStrategy, - KeyringConfig, KeyringController, - KeyringMemState, KeyringObject, + KeyringControllerEvents, + KeyringControllerMessenger, + KeyringControllerState, KeyringTypes, + KeyringControllerOptions, } from './KeyringController'; jest.mock('uuid', () => { @@ -60,24 +63,21 @@ describe('KeyringController', () => { it('should add new account', async () => { await withController( async ({ controller, initialState, preferences }) => { - const { - keyringState: currentKeyringMemState, - addedAccountAddress, - } = await controller.addNewAccount(); + const { addedAccountAddress } = await controller.addNewAccount(); expect(initialState.keyrings).toHaveLength(1); expect(initialState.keyrings[0].accounts).not.toStrictEqual( - currentKeyringMemState.keyrings[0].accounts, + controller.state.keyrings[0].accounts, ); - expect(currentKeyringMemState.keyrings[0].accounts).toHaveLength(2); + expect(controller.state.keyrings[0].accounts).toHaveLength(2); expect(initialState.keyrings[0].accounts).not.toContain( addedAccountAddress, ); expect(addedAccountAddress).toBe( - currentKeyringMemState.keyrings[0].accounts[1], + controller.state.keyrings[0].accounts[1], ); expect( preferences.updateIdentities.calledWith( - currentKeyringMemState.keyrings[0].accounts, + controller.state.keyrings[0].accounts, ), ).toBe(true); expect(preferences.setSelectedAddress.called).toBe(false); @@ -90,26 +90,23 @@ describe('KeyringController', () => { it('should add new account if accountCount is in sequence', async () => { await withController( async ({ controller, initialState, preferences }) => { - const { - keyringState: currentKeyringMemState, - addedAccountAddress, - } = await controller.addNewAccount( + const { addedAccountAddress } = await controller.addNewAccount( initialState.keyrings[0].accounts.length, ); expect(initialState.keyrings).toHaveLength(1); expect(initialState.keyrings[0].accounts).not.toStrictEqual( - currentKeyringMemState.keyrings[0].accounts, + controller.state.keyrings[0].accounts, ); - expect(currentKeyringMemState.keyrings[0].accounts).toHaveLength(2); + expect(controller.state.keyrings[0].accounts).toHaveLength(2); expect(initialState.keyrings[0].accounts).not.toContain( addedAccountAddress, ); expect(addedAccountAddress).toBe( - currentKeyringMemState.keyrings[0].accounts[1], + controller.state.keyrings[0].accounts[1], ); expect( preferences.updateIdentities.calledWith( - currentKeyringMemState.keyrings[0].accounts, + controller.state.keyrings[0].accounts, ), ).toBe(true); expect(preferences.setSelectedAddress.called).toBe(false); @@ -131,10 +128,10 @@ describe('KeyringController', () => { const accountCount = initialState.keyrings[0].accounts.length; const { addedAccountAddress: firstAccountAdded } = await controller.addNewAccount(accountCount); - const { keyringState, addedAccountAddress: secondAccountAdded } = + const { addedAccountAddress: secondAccountAdded } = await controller.addNewAccount(accountCount); expect(firstAccountAdded).toBe(secondAccountAdded); - expect(keyringState.keyrings[0].accounts).toHaveLength( + expect(controller.state.keyrings[0].accounts).toHaveLength( accountCount + 1, ); }); @@ -148,13 +145,12 @@ describe('KeyringController', () => { async ({ controller, initialState, preferences }) => { const initialUpdateIdentitiesCallCount = preferences.updateIdentities.callCount; - const currentKeyringMemState = - await controller.addNewAccountWithoutUpdate(); + await controller.addNewAccountWithoutUpdate(); expect(initialState.keyrings).toHaveLength(1); expect(initialState.keyrings[0].accounts).not.toStrictEqual( - currentKeyringMemState.keyrings[0].accounts, + controller.state.keyrings[0].accounts, ); - expect(currentKeyringMemState.keyrings[0].accounts).toHaveLength(2); + expect(controller.state.keyrings[0].accounts).toHaveLength(2); // we make sure that updateIdentities is not called // during this test expect(preferences.updateIdentities.callCount).toBe( @@ -173,11 +169,11 @@ describe('KeyringController', () => { { cacheEncryptionKey }, async ({ controller, initialState }) => { const initialVault = controller.state.vault; - const currentState = await controller.createNewVaultAndRestore( + await controller.createNewVaultAndRestore( password, uint8ArraySeed, ); - expect(initialState).not.toBe(currentState); + expect(controller.state).not.toBe(initialState); expect(controller.state.vault).toBeDefined(); expect(controller.state.vault).toStrictEqual(initialVault); }, @@ -192,11 +188,11 @@ describe('KeyringController', () => { password, ); - const currentState = await controller.createNewVaultAndRestore( + await controller.createNewVaultAndRestore( password, currentSeedWord, ); - expect(initialState).toStrictEqual(currentState); + expect(initialState).toStrictEqual(controller.state); }, ); }); @@ -232,12 +228,12 @@ describe('KeyringController', () => { cacheEncryptionKey && it('should set encryptionKey and encryptionSalt in state', async () => { withController({ cacheEncryptionKey }, async ({ controller }) => { - const currentState = await controller.createNewVaultAndRestore( + await controller.createNewVaultAndRestore( password, uint8ArraySeed, ); - expect(currentState.encryptionKey).toBeDefined(); - expect(currentState.encryptionSalt).toBeDefined(); + expect(controller.state.encryptionKey).toBeDefined(); + expect(controller.state.encryptionSalt).toBeDefined(); }); }); }), @@ -252,25 +248,28 @@ describe('KeyringController', () => { await withController( { cacheEncryptionKey }, async ({ controller, initialState, preferences, encryptor }) => { - const cleanKeyringController = new KeyringController( - preferences, - { cacheEncryptionKey, encryptor }, - ); + const cleanKeyringController = new KeyringController({ + ...preferences, + messenger: buildKeyringControllerMessenger(), + cacheEncryptionKey, + encryptor, + }); const initialSeedWord = await controller.exportSeedPhrase( password, ); - const currentState = - await cleanKeyringController.createNewVaultAndKeychain( - password, - ); + await cleanKeyringController.createNewVaultAndKeychain( + password, + ); const currentSeedWord = await cleanKeyringController.exportSeedPhrase(password); expect(initialSeedWord).toBeDefined(); - expect(initialState).not.toBe(currentState); + expect(initialState).not.toBe(cleanKeyringController.state); expect(currentSeedWord).toBeDefined(); expect(initialSeedWord).not.toBe(currentSeedWord); expect( - isValidHexAddress(currentState.keyrings[0].accounts[0]), + isValidHexAddress( + cleanKeyringController.state.keyrings[0].accounts[0], + ), ).toBe(true); expect(controller.state.vault).toBeDefined(); }, @@ -282,7 +281,6 @@ describe('KeyringController', () => { expect(controller.state.keyrings).not.toStrictEqual([]); const keyring = controller.state.keyrings[0]; expect(keyring.accounts).not.toStrictEqual([]); - expect(keyring.index).toStrictEqual(0); expect(keyring.type).toStrictEqual('HD Key Tree'); expect(controller.state.vault).toBeDefined(); }); @@ -298,14 +296,12 @@ describe('KeyringController', () => { password, ); const initialVault = controller.state.vault; - const currentState = await controller.createNewVaultAndKeychain( - password, - ); + await controller.createNewVaultAndKeychain(password); const currentSeedWord = await controller.exportSeedPhrase( password, ); expect(initialSeedWord).toBeDefined(); - expect(initialState).toBe(currentState); + expect(initialState).toBe(controller.state); expect(currentSeedWord).toBeDefined(); expect(initialSeedWord).toBe(currentSeedWord); expect(initialVault).toStrictEqual(controller.state.vault); @@ -329,8 +325,19 @@ describe('KeyringController', () => { it('should set locked correctly', async () => { await withController(async ({ controller }) => { expect(controller.isUnlocked()).toBe(true); + expect(controller.state.isUnlocked).toBe(true); controller.setLocked(); expect(controller.isUnlocked()).toBe(false); + expect(controller.state.isUnlocked).toBe(false); + }); + }); + + it('should emit KeyringController:lock event', async () => { + await withController(async ({ controller, messenger }) => { + const listener = sinon.spy(); + messenger.subscribe('KeyringController:lock', listener); + await controller.setLocked(); + expect(listener.called).toBe(true); }); }); }); @@ -481,7 +488,7 @@ describe('KeyringController', () => { accounts: [address], type: 'Simple Key Pair', }; - const { keyringState, importedAccountAddress } = + const { importedAccountAddress } = await controller.importAccountWithStrategy( AccountImportStrategy.privateKey, [privateKey], @@ -490,7 +497,7 @@ describe('KeyringController', () => { ...initialState, keyrings: [initialState.keyrings[0], newKeyring], }; - expect(keyringState).toStrictEqual(modifiedState); + expect(controller.state).toStrictEqual(modifiedState); expect(importedAccountAddress).toBe(address); }); }); @@ -550,7 +557,7 @@ describe('KeyringController', () => { const somePassword = 'holachao123'; const address = '0xb97c80fab7a3793bbe746864db80d236f1345ea7'; - const { keyringState, importedAccountAddress } = + const { importedAccountAddress } = await controller.importAccountWithStrategy( AccountImportStrategy.json, [input, somePassword], @@ -564,7 +571,7 @@ describe('KeyringController', () => { ...initialState, keyrings: [initialState.keyrings[0], newKeyring], }; - expect(keyringState).toStrictEqual(modifiedState); + expect(controller.state).toStrictEqual(modifiedState); expect(importedAccountAddress).toBe(address); }); }); @@ -648,8 +655,8 @@ describe('KeyringController', () => { it('should remove HD Key Tree keyring from state when single account associated with it is deleted', async () => { await withController(async ({ controller, initialState }) => { const account = initialState.keyrings[0].accounts[0]; - const finalState = await controller.removeAccount(account); - expect(finalState.keyrings).toHaveLength(0); + await controller.removeAccount(account); + expect(controller.state.keyrings).toHaveLength(0); }); }); @@ -659,10 +666,10 @@ describe('KeyringController', () => { AccountImportStrategy.privateKey, [privateKey], ); - const finalState = await controller.removeAccount( + await controller.removeAccount( '0x51253087e6f8358b5f10c0a94315d69db3357859', ); - expect(finalState).toStrictEqual(initialState); + expect(controller.state).toStrictEqual(initialState); }); }); @@ -1072,8 +1079,20 @@ describe('KeyringController', () => { await withController( { cacheEncryptionKey }, async ({ controller, initialState }) => { - const recoveredState = await controller.submitPassword(password); - expect(recoveredState).toStrictEqual(initialState); + await controller.submitPassword(password); + expect(controller.state).toStrictEqual(initialState); + }, + ); + }); + + it('should emit KeyringController:unlock event', async () => { + await withController( + { cacheEncryptionKey }, + async ({ controller, messenger }) => { + const listener = sinon.spy(); + messenger.subscribe('KeyringController:unlock', listener); + await controller.submitPassword(password); + expect(listener.called).toBe(true); }, ); }); @@ -1081,9 +1100,9 @@ describe('KeyringController', () => { cacheEncryptionKey && it('should set encryptionKey and encryptionSalt in state', async () => { withController({ cacheEncryptionKey }, async ({ controller }) => { - const recoveredState = await controller.submitPassword(password); - expect(recoveredState.encryptionKey).toBeDefined(); - expect(recoveredState.encryptionSalt).toBeDefined(); + await controller.submitPassword(password); + expect(controller.state.encryptionKey).toBeDefined(); + expect(controller.state.encryptionSalt).toBeDefined(); }); }); }), @@ -1095,57 +1114,16 @@ describe('KeyringController', () => { await withController( { cacheEncryptionKey: true }, async ({ controller, initialState }) => { - const recoveredState = await controller.submitEncryptionKey( + await controller.submitEncryptionKey( mockKey.toString('hex'), initialState.encryptionSalt as string, ); - expect(recoveredState).toStrictEqual(initialState); + expect(controller.state).toStrictEqual(initialState); }, ); }); }); - describe('subscribe and unsubscribe', () => { - it('should subscribe and unsubscribe', async () => { - await withController(async ({ controller }) => { - const listener = sinon.stub(); - controller.subscribe(listener); - await controller.importAccountWithStrategy( - AccountImportStrategy.privateKey, - [privateKey], - ); - expect(listener.called).toBe(true); - controller.unsubscribe(listener); - await controller.removeAccount( - '0x51253087e6f8358b5f10c0a94315d69db3357859', - ); - expect(listener.calledTwice).toBe(false); - }); - }); - }); - - describe('onLock', () => { - it('should receive lock event', async () => { - await withController(async ({ controller }) => { - const listenerLock = sinon.stub(); - controller.onLock(listenerLock); - await controller.setLocked(); - expect(listenerLock.called).toBe(true); - }); - }); - }); - - describe('onUnlock', () => { - it('should receive unlock event', async () => { - await withController(async ({ controller }) => { - const listenerUnlock = sinon.stub(); - controller.onUnlock(listenerUnlock); - await controller.submitPassword(password); - expect(listenerUnlock.called).toBe(true); - }); - }); - }); - describe('verifySeedPhrase', () => { it('should return current seedphrase', async () => { await withController(async ({ controller }) => { @@ -1657,6 +1635,7 @@ type WithControllerCallback = ({ preferences, initialState, encryptor, + messenger, }: { controller: KeyringController; preferences: { @@ -1667,15 +1646,44 @@ type WithControllerCallback = ({ setSelectedAddress: sinon.SinonStub; }; encryptor: MockEncryptor; - initialState: KeyringMemState; + initialState: KeyringControllerState; + messenger: KeyringControllerMessenger; }) => Promise | ReturnValue; -type WithControllerOptions = Partial; +type WithControllerOptions = Partial; type WithControllerArgs = | [WithControllerCallback] | [WithControllerOptions, WithControllerCallback]; +/** + * Build a controller messenger that includes all events used by the keyring + * controller. + * + * @returns The controller messenger. + */ +function buildMessenger() { + return new ControllerMessenger(); +} + +/** + * Build a restricted controller messenger for the keyring controller. + * + * @param messenger - A controller messenger. + * @returns The keyring controller restricted messenger. + */ +function buildKeyringControllerMessenger(messenger = buildMessenger()) { + return messenger.getRestricted({ + name: 'KeyringController', + allowedActions: [], + allowedEvents: [ + 'KeyringController:stateChange', + 'KeyringController:lock', + 'KeyringController:unlock', + ], + }); +} + /** * Builds a controller based on the given options and creates a new vault * and keychain, then calls the given function with that controller. @@ -1698,16 +1706,20 @@ async function withController( updateIdentities: sinon.stub(), setSelectedAddress: sinon.stub(), }; - const controller = new KeyringController(preferences, { + const messenger = buildKeyringControllerMessenger(); + const controller = new KeyringController({ encryptor, cacheEncryptionKey: false, + messenger, + ...preferences, ...rest, }); - const initialState = await controller.createNewVaultAndKeychain(password); + await controller.createNewVaultAndKeychain(password); return await fn({ controller, preferences, encryptor, - initialState, + initialState: controller.state, + messenger, }); } diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 8173b42e4a..59950e861a 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -6,10 +6,7 @@ import { stripHexPrefix, getBinarySize, } from 'ethereumjs-util'; -import { - isValidHexAddress, - toChecksumHexAddress, -} from '@metamask/controller-utils'; +import { isValidHexAddress } from '@metamask/controller-utils'; import { normalize as normalizeAddress, signTypedData, @@ -22,16 +19,17 @@ import { IKeyringState as IQRKeyringState, } from '@keystonehq/metamask-airgapped-keyring'; import { - BaseController, - BaseConfig, - BaseState, - Listener, + BaseControllerV2, + RestrictedControllerMessenger, } from '@metamask/base-controller'; import { PreferencesController } from '@metamask/preferences-controller'; import { PersonalMessageParams, TypedMessageParams, } from '@metamask/message-manager'; +import type { Patch } from 'immer'; + +const name = 'KeyringController'; /** * Available keyring types @@ -57,44 +55,77 @@ export interface KeyringObject { } /** - * @type KeyringState + * @type KeyringControllerState * * Keyring controller state * @property vault - Encrypted string representing keyring data - * @property keyrings - Group of accounts - */ -export interface KeyringState extends BaseState { - vault?: string; - keyrings: Keyring[]; -} - -/** - * @type KeyringMemState - * - * Keyring mem controller state * @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 */ -export interface KeyringMemState extends BaseState { +export type KeyringControllerState = { + vault?: string; isUnlocked: boolean; keyringTypes: string[]; keyrings: Keyring[]; encryptionKey?: string; encryptionSalt?: string; -} - -/** - * @type KeyringConfig - * - * Keyring controller configuration - * @property encryptor - Keyring encryptor - */ -export interface KeyringConfig extends BaseConfig { +}; + +export type KeyringControllerMemState = Omit< + KeyringControllerState, + 'vault' | 'encryptionKey' | 'encryptionSalt' +>; + +export type KeyringControllerGetStateAction = { + type: `${typeof name}:getState`; + handler: () => KeyringControllerState; +}; + +export type KeyringControllerStateChangeEvent = { + type: `${typeof name}:stateChange`; + payload: [KeyringControllerState, Patch[]]; +}; + +export type KeyringControllerLockEvent = { + type: `${typeof name}:lock`; + payload: []; +}; + +export type KeyringControllerUnlockEvent = { + type: `${typeof name}:unlock`; + payload: []; +}; + +export type KeyringControllerActions = KeyringControllerGetStateAction; + +export type KeyringControllerEvents = + | KeyringControllerStateChangeEvent + | KeyringControllerLockEvent + | KeyringControllerUnlockEvent; + +export type KeyringControllerMessenger = RestrictedControllerMessenger< + typeof name, + KeyringControllerActions, + KeyringControllerEvents, + string, + string +>; + +export type KeyringControllerOptions = { + removeIdentity: PreferencesController['removeIdentity']; + syncIdentities: PreferencesController['syncIdentities']; + updateIdentities: PreferencesController['updateIdentities']; + setSelectedAddress: PreferencesController['setSelectedAddress']; + setAccountLabel?: PreferencesController['setAccountLabel']; encryptor?: any; keyringBuilders?: any[]; cacheEncryptionKey?: boolean; -} + messenger: KeyringControllerMessenger; + state?: Partial; +}; /** * @type Keyring @@ -102,13 +133,11 @@ export interface KeyringConfig extends BaseConfig { * Keyring object to return in fullUpdate * @property type - Keyring type * @property accounts - Associated accounts - * @property index - Associated index */ -export interface Keyring { +export type Keyring = { accounts: string[]; type: string; - index?: number; -} +}; /** * A strategy for importing an account @@ -129,6 +158,12 @@ export enum SignTypedDataVersion { V4 = 'V4', } +const defaultState: KeyringControllerState = { + isUnlocked: false, + keyringTypes: [], + keyrings: [], +}; + /** * Controller responsible for establishing and managing user identity. * @@ -138,17 +173,13 @@ export enum SignTypedDataVersion { * with the internal keyring controller and handling certain complex operations that involve the * keyrings. */ -export class KeyringController extends BaseController< - KeyringConfig, - KeyringState +export class KeyringController extends BaseControllerV2< + typeof name, + KeyringControllerState, + KeyringControllerMessenger > { private mutex = new Mutex(); - /** - * Name of this controller used during composition - */ - override name = 'KeyringController'; - private removeIdentity: PreferencesController['removeIdentity']; private syncIdentities: PreferencesController['syncIdentities']; @@ -164,51 +195,63 @@ export class KeyringController extends BaseController< /** * Creates a KeyringController instance. * - * @param options - The controller options. - * @param options.removeIdentity - Remove the identity with the given address. - * @param options.syncIdentities - Sync identities with the given list of addresses. - * @param options.updateIdentities - Generate an identity for each address given that doesn't already have an identity. - * @param options.setSelectedAddress - Set the selected address. - * @param options.setAccountLabel - Set a new name for account. - * @param config - Initial options used to configure this controller. - * @param state - Initial state to set on this controller. + * @param opts - Initial options used to configure this controller + * @param opts.removeIdentity - Remove the identity with the given address. + * @param opts.syncIdentities - Sync identities with the given list of addresses. + * @param opts.updateIdentities - Generate an identity for each address given that doesn't already have an identity. + * @param opts.setSelectedAddress - Set the selected address. + * @param opts.setAccountLabel - Set a new name for account. + * @param opts.encryptor - An optional object for defining encryption schemes. + * @param opts.keyringBuilders - Set a new name for account. + * @param opts.cacheEncryptionKey - Whether to cache or not encryption key. + * @param opts.messenger - A restricted controller messenger. + * @param opts.state - Initial state to set on this controller. */ - constructor( - { - removeIdentity, - syncIdentities, - updateIdentities, - setSelectedAddress, - setAccountLabel, - }: { - removeIdentity: PreferencesController['removeIdentity']; - syncIdentities: PreferencesController['syncIdentities']; - updateIdentities: PreferencesController['updateIdentities']; - setSelectedAddress: PreferencesController['setSelectedAddress']; - setAccountLabel?: PreferencesController['setAccountLabel']; - }, - config?: Partial, - state?: Partial, - ) { - super(config, state); + constructor({ + removeIdentity, + syncIdentities, + updateIdentities, + setSelectedAddress, + setAccountLabel, + encryptor, + keyringBuilders, + cacheEncryptionKey, + messenger, + state, + }: KeyringControllerOptions) { + super({ + name, + metadata: { + vault: { persist: true, anonymous: false }, + isUnlocked: { persist: false, anonymous: true }, + keyringTypes: { persist: false, anonymous: false }, + keyrings: { persist: false, anonymous: false }, + encryptionKey: { persist: false, anonymous: false }, + encryptionSalt: { persist: false, anonymous: false }, + }, + messenger, + state: { + ...defaultState, + ...state, + }, + }); + this.#keyring = new EthKeyringController( - Object.assign({ initState: state }, config), + Object.assign( + { initState: state }, + { encryptor, keyringBuilders, cacheEncryptionKey }, + ), ); - this.#keyring.store.subscribe(() => { - this.update({ vault: this.#keyring.store.getState().vault }); - }); + this.#keyring.memStore.subscribe(this.#fullUpdate.bind(this)); + this.#keyring.store.subscribe(this.#fullUpdate.bind(this)); + this.#keyring.on('lock', this.#handleLock.bind(this)); + this.#keyring.on('unlock', this.#handleUnlock.bind(this)); - this.defaultState = { - ...this.#keyring.store.getState(), - keyrings: [], - }; this.removeIdentity = removeIdentity; this.syncIdentities = syncIdentities; this.updateIdentities = updateIdentities; this.setSelectedAddress = setSelectedAddress; this.setAccountLabel = setAccountLabel; - this.initialize(); - this.fullUpdate(); } /** @@ -220,7 +263,7 @@ export class KeyringController extends BaseController< * address. */ async addNewAccount(accountCount?: number): Promise<{ - keyringState: KeyringMemState; + keyringState: KeyringControllerMemState; addedAccountAddress: string; }> { const primaryKeyring = this.#keyring.getKeyringsByType('HD Key Tree')[0]; @@ -237,7 +280,7 @@ export class KeyringController extends BaseController< // we return the account already existing at index `accountCount` const primaryKeyringAccounts = await primaryKeyring.getAccounts(); return { - keyringState: await this.fullUpdate(), + keyringState: this.#getMemState(), addedAccountAddress: primaryKeyringAccounts[accountCount], }; } @@ -252,7 +295,7 @@ export class KeyringController extends BaseController< (selectedAddress: string) => !oldAccounts.includes(selectedAddress), ); return { - keyringState: await this.fullUpdate(), + keyringState: this.#getMemState(), addedAccountAddress, }; } @@ -262,7 +305,7 @@ export class KeyringController extends BaseController< * * @returns Promise resolving to current state when the account is added. */ - async addNewAccountWithoutUpdate(): Promise { + async addNewAccountWithoutUpdate(): Promise { const primaryKeyring = this.#keyring.getKeyringsByType('HD Key Tree')[0]; /* istanbul ignore if */ if (!primaryKeyring) { @@ -270,7 +313,7 @@ export class KeyringController extends BaseController< } await this.#keyring.addNewAccount(primaryKeyring); await this.verifySeedPhrase(); - return this.fullUpdate(); + return this.#getMemState(); } /** @@ -282,7 +325,10 @@ export class KeyringController extends BaseController< * either as a string or an array of UTF-8 bytes that represent the string. * @returns Promise resolving to the restored keychain object. */ - async createNewVaultAndRestore(password: string, seed: Uint8Array) { + async createNewVaultAndRestore( + password: string, + seed: Uint8Array, + ): Promise { const releaseLock = await this.mutex.acquire(); if (!password || !password.length) { throw new Error('Invalid password'); @@ -290,13 +336,9 @@ export class KeyringController extends BaseController< try { this.updateIdentities([]); - const vault = await this.#keyring.createNewVaultAndRestore( - password, - seed, - ); + await this.#keyring.createNewVaultAndRestore(password, seed); this.updateIdentities(await this.#keyring.getAccounts()); - this.fullUpdate(); - return vault; + return this.#getMemState(); } finally { releaseLock(); } @@ -311,17 +353,12 @@ export class KeyringController extends BaseController< async createNewVaultAndKeychain(password: string) { const releaseLock = await this.mutex.acquire(); try { - let vault; const accounts = await this.getAccounts(); - if (accounts.length > 0) { - vault = await this.fullUpdate(); - } else { - vault = await this.#keyring.createNewVaultAndKeychain(password); + if (!accounts.length) { + await this.#keyring.createNewVaultAndKeychain(password); this.updateIdentities(await this.getAccounts()); - await this.fullUpdate(); } - - return vault; + return this.#getMemState(); } finally { releaseLock(); } @@ -343,7 +380,7 @@ export class KeyringController extends BaseController< * @returns Boolean returning true if the vault is unlocked. */ isUnlocked(): boolean { - return this.#keyring.memStore.getState().isUnlocked; + return this.state.isUnlocked; } /** @@ -418,7 +455,7 @@ export class KeyringController extends BaseController< strategy: AccountImportStrategy, args: any[], ): Promise<{ - keyringState: KeyringMemState; + keyringState: KeyringControllerMemState; importedAccountAddress: string; }> { let privateKey; @@ -468,7 +505,7 @@ export class KeyringController extends BaseController< const allAccounts = await this.#keyring.getAccounts(); this.updateIdentities(allAccounts); return { - keyringState: await this.fullUpdate(), + keyringState: this.#getMemState(), importedAccountAddress: accounts[0], }; } @@ -479,10 +516,10 @@ export class KeyringController extends BaseController< * @param address - Address of the account to remove. * @returns Promise resolving current state when this account removal completes. */ - async removeAccount(address: string): Promise { + async removeAccount(address: string): Promise { this.removeIdentity(address); await this.#keyring.removeAccount(address); - return this.fullUpdate(); + return this.#getMemState(); } /** @@ -490,7 +527,7 @@ export class KeyringController extends BaseController< * * @returns Promise resolving to current state. */ - setLocked(): Promise { + setLocked(): Promise { return this.#keyring.setLocked(); } @@ -606,8 +643,9 @@ export class KeyringController extends BaseController< async submitEncryptionKey( encryptionKey: string, encryptionSalt: string, - ): Promise { - return this.#keyring.submitEncryptionKey(encryptionKey, encryptionSalt); + ): Promise { + await this.#keyring.submitEncryptionKey(encryptionKey, encryptionSalt); + return this.#getMemState(); } /** @@ -617,50 +655,11 @@ export class KeyringController extends BaseController< * @param password - Password to unlock the keychain. * @returns Promise resolving to the current state. */ - async submitPassword(password: string): Promise { + async submitPassword(password: string): Promise { await this.#keyring.submitPassword(password); const accounts = await this.#keyring.getAccounts(); await this.syncIdentities(accounts); - return this.fullUpdate(); - } - - /** - * Adds new listener to be notified of state changes. - * - * @param listener - Callback triggered when state changes. - */ - override subscribe(listener: Listener) { - this.#keyring.store.subscribe(listener); - } - - /** - * Removes existing listener from receiving state changes. - * - * @param listener - Callback to remove. - * @returns True if a listener is found and unsubscribed. - */ - override unsubscribe(listener: Listener) { - return this.#keyring.store.unsubscribe(listener); - } - - /** - * Adds new listener to be notified when the wallet is locked. - * - * @param listener - Callback triggered when wallet is locked. - * @returns EventEmitter if listener added. - */ - onLock(listener: () => void) { - return this.#keyring.on('lock', listener); - } - - /** - * Adds new listener to be notified when the wallet is unlocked. - * - * @param listener - Callback triggered when wallet is unlocked. - * @returns EventEmitter if listener added. - */ - onUnlock(listener: () => void) { - return this.#keyring.on('unlock', listener); + return this.#getMemState(); } /** @@ -706,31 +705,6 @@ export class KeyringController extends BaseController< return seedWords; } - /** - * Update keyrings in state and calls KeyringController fullUpdate method returning current state. - * - * @returns The current state. - */ - async fullUpdate(): Promise { - const keyrings: Keyring[] = await Promise.all( - this.#keyring.keyrings.map( - async (keyring: KeyringObject, index: number): Promise => { - const keyringAccounts = await keyring.getAccounts(); - const accounts = Array.isArray(keyringAccounts) - ? keyringAccounts.map((address) => toChecksumHexAddress(address)) - : /* istanbul ignore next */ []; - return { - accounts, - index, - type: keyring.type, - }; - }, - ), - ); - this.update({ keyrings: [...keyrings] }); - return this.#keyring.fullUpdate(); - } - // QR Hardware related methods /** @@ -739,9 +713,7 @@ export class KeyringController extends BaseController< * @returns The added keyring */ private async addQRKeyring(): Promise { - const keyring = await this.#keyring.addNewKeyring(KeyringTypes.qr); - await this.fullUpdate(); - return keyring; + return this.#keyring.addNewKeyring(KeyringTypes.qr); } /** @@ -756,8 +728,8 @@ export class KeyringController extends BaseController< async restoreQRKeyring(serialized: any): Promise { (await this.getOrAddQRKeyring()).deserialize(serialized); + await this.#keyring.persistAllKeyrings(); this.updateIdentities(await this.#keyring.getAccounts()); - await this.fullUpdate(); } async resetQRKeyringState(): Promise { @@ -841,7 +813,6 @@ export class KeyringController extends BaseController< } }); await this.#keyring.persistAllKeyrings(); - await this.fullUpdate(); } async getAccountKeyringType(account: string): Promise { @@ -856,7 +827,45 @@ export class KeyringController extends BaseController< this.setSelectedAddress(account); }); await this.#keyring.persistAllKeyrings(); - await this.fullUpdate(); + } + + /** + * Sync controller state with current keyring store + * and memStore states. + * + * @fires KeyringController:stateChange + */ + #fullUpdate() { + this.update(() => ({ + ...this.#keyring.store.getState(), + ...this.#keyring.memStore.getState(), + })); + } + + /** + * Handle keyring lock event. + * + * @fires KeyringController:lock + */ + #handleLock() { + this.messagingSystem.publish(`${name}:lock`); + } + + /** + * Handle keyring unlock event. + * + * @fires KeyringController:unlock + */ + #handleUnlock() { + this.messagingSystem.publish(`${name}:unlock`); + } + + #getMemState(): KeyringControllerMemState { + return { + isUnlocked: this.state.isUnlocked, + keyrings: this.state.keyrings, + keyringTypes: this.state.keyringTypes, + }; } } diff --git a/yarn.lock b/yarn.lock index 322b90704b..ce61665a0d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1725,6 +1725,7 @@ __metadata: deepmerge: ^4.2.2 ethereumjs-util: ^7.0.10 ethereumjs-wallet: ^1.0.1 + immer: ^9.0.6 jest: ^27.5.1 sinon: ^9.2.4 ts-jest: ^27.1.4