diff --git a/packages/keyring-controller/package.json b/packages/keyring-controller/package.json index e0af02d2ad..303318a456 100644 --- a/packages/keyring-controller/package.json +++ b/packages/keyring-controller/package.json @@ -36,7 +36,6 @@ "@metamask/eth-keyring-controller": "^17.0.1", "@metamask/keyring-api": "^3.0.0", "@metamask/message-manager": "^7.3.7", - "@metamask/preferences-controller": "^6.0.0", "@metamask/utils": "^8.3.0", "async-mutex": "^0.2.6", "ethereumjs-util": "^7.0.10", @@ -62,9 +61,6 @@ "typescript": "~4.8.4", "uuid": "^8.3.2" }, - "peerDependencies": { - "@metamask/preferences-controller": "^6.0.0" - }, "engines": { "node": ">=16.0.0" }, diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index c1a12d3f25..117ae1b93d 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -24,7 +24,6 @@ import type { PersonalMessageParams, TypedMessageParams, } from '@metamask/message-manager'; -import type { PreferencesController } from '@metamask/preferences-controller'; import type { Eip1024EncryptedData, Hex, Json } from '@metamask/utils'; import { assertIsStrictHexString, hasProperty } from '@metamask/utils'; import { Mutex } from 'async-mutex'; @@ -200,10 +199,10 @@ export type KeyringControllerMessenger = RestrictedControllerMessenger< >; export type KeyringControllerOptions = { - syncIdentities: PreferencesController['syncIdentities']; - updateIdentities: PreferencesController['updateIdentities']; - setSelectedAddress: PreferencesController['setSelectedAddress']; - setAccountLabel?: PreferencesController['setAccountLabel']; + syncIdentities: (addresses: string[]) => string; + updateIdentities: (addresses: string[]) => void; + setSelectedAddress: (selectedAddress: string) => void; + setAccountLabel?: (address: string, label: string) => void; keyringBuilders?: { (): EthKeyring; type: string }[]; messenger: KeyringControllerMessenger; state?: { vault?: string }; @@ -249,9 +248,11 @@ export enum SignTypedDataVersion { V4 = 'V4', } -const defaultState: KeyringControllerState = { - isUnlocked: false, - keyrings: [], +export const getDefaultKeyringState = (): KeyringControllerState => { + return { + isUnlocked: false, + keyrings: [], + }; }; /** @@ -289,13 +290,13 @@ export class KeyringController extends BaseController< > { private readonly mutex = new Mutex(); - private readonly syncIdentities: PreferencesController['syncIdentities']; + private readonly syncIdentities: (addresses: string[]) => string; - private readonly updateIdentities: PreferencesController['updateIdentities']; + private readonly updateIdentities: (addresses: string[]) => void; - private readonly setSelectedAddress: PreferencesController['setSelectedAddress']; + private readonly setSelectedAddress: (selectedAddress: string) => void; - private readonly setAccountLabel?: PreferencesController['setAccountLabel']; + private readonly setAccountLabel?: (address: string, label: string) => void; #keyring: EthKeyringController; @@ -339,7 +340,7 @@ export class KeyringController extends BaseController< }, messenger, state: { - ...defaultState, + ...getDefaultKeyringState(), ...state, }, }); diff --git a/packages/keyring-controller/tsconfig.build.json b/packages/keyring-controller/tsconfig.build.json index 4cd8644e34..38d8a31843 100644 --- a/packages/keyring-controller/tsconfig.build.json +++ b/packages/keyring-controller/tsconfig.build.json @@ -11,9 +11,6 @@ }, { "path": "../message-manager/tsconfig.build.json" - }, - { - "path": "../preferences-controller/tsconfig.build.json" } ], "include": ["../../types", "./src"] diff --git a/packages/keyring-controller/tsconfig.json b/packages/keyring-controller/tsconfig.json index 5fc1bae6c4..831b2ae3b4 100644 --- a/packages/keyring-controller/tsconfig.json +++ b/packages/keyring-controller/tsconfig.json @@ -9,9 +9,6 @@ }, { "path": "../message-manager" - }, - { - "path": "../preferences-controller" } ], "include": ["../../types", "./src", "./tests"] diff --git a/packages/preferences-controller/package.json b/packages/preferences-controller/package.json index 952e88075c..70a1b0b90a 100644 --- a/packages/preferences-controller/package.json +++ b/packages/preferences-controller/package.json @@ -36,14 +36,19 @@ }, "devDependencies": { "@metamask/auto-changelog": "^3.4.4", + "@metamask/keyring-controller": "^12.1.0", "@types/jest": "^27.4.1", "deepmerge": "^4.2.2", "jest": "^27.5.1", + "lodash": "^4.17.21", "ts-jest": "^27.1.4", "typedoc": "^0.24.8", "typedoc-plugin-missing-exports": "^2.0.0", "typescript": "~4.8.4" }, + "peerDependencies": { + "@metamask/keyring-controller": "^12.1.0" + }, "engines": { "node": ">=16.0.0" }, diff --git a/packages/preferences-controller/src/PreferencesController.test.ts b/packages/preferences-controller/src/PreferencesController.test.ts index cce9969e09..4d4947f0ec 100644 --- a/packages/preferences-controller/src/PreferencesController.test.ts +++ b/packages/preferences-controller/src/PreferencesController.test.ts @@ -1,7 +1,14 @@ import { ControllerMessenger } from '@metamask/base-controller'; +import { getDefaultKeyringState } from '@metamask/keyring-controller'; +import { cloneDeep } from 'lodash'; import { ETHERSCAN_SUPPORTED_CHAIN_IDS } from './constants'; -import type { EtherscanSupportedHexChainId } from './PreferencesController'; +import type { + AllowedEvents, + EtherscanSupportedHexChainId, + PreferencesControllerActions, + PreferencesControllerEvents, +} from './PreferencesController'; import { PreferencesController } from './PreferencesController'; describe('PreferencesController', () => { @@ -32,6 +39,199 @@ describe('PreferencesController', () => { }); }); + describe('KeyringController:stateChange', () => { + it('should update identities state to reflect new keyring accounts', () => { + const messenger = getControllerMessenger(); + const controller = setupPreferencesController({ + options: { + state: { + identities: { + '0x00': { address: '0x00', name: 'Account 1' }, + }, + selectedAddress: '0x00', + }, + }, + messenger, + }); + + messenger.publish( + 'KeyringController:stateChange', + { + ...getDefaultKeyringState(), + keyrings: [ + { accounts: ['0x00', '0x01', '0x02'], type: 'CustomKeyring' }, + ], + }, + [], + ); + + expect(controller.state.identities).toMatchObject({ + '0x00': { address: '0x00', name: 'Account 1' }, + '0x01': { + address: '0x01', + importTime: expect.any(Number), + name: 'Account 2', + }, + '0x02': { + address: '0x02', + importTime: expect.any(Number), + name: 'Account 3', + }, + }); + expect(controller.state.selectedAddress).toBe('0x00'); + }); + + it('should update identities state to reflect removed keyring accounts', () => { + const messenger = getControllerMessenger(); + const controller = setupPreferencesController({ + options: { + state: { + identities: { + '0x00': { address: '0x00', name: 'Account 1' }, + '0x01': { address: '0x01', name: 'Account 2' }, + '0x02': { address: '0x02', name: 'Account 3' }, + }, + selectedAddress: '0x00', + }, + }, + messenger, + }); + + messenger.publish( + 'KeyringController:stateChange', + { + ...getDefaultKeyringState(), + keyrings: [{ accounts: ['0x00'], type: 'CustomKeyring' }], + }, + [], + ); + + expect(controller.state.identities).toStrictEqual({ + '0x00': { address: '0x00', name: 'Account 1' }, + }); + }); + + it('should update selected address to first identity if the selected address was removed', () => { + const messenger = getControllerMessenger(); + const controller = setupPreferencesController({ + options: { + state: { + identities: { + '0x00': { address: '0x00', name: 'Account 1' }, + '0x01': { address: '0x01', name: 'Account 2' }, + '0x02': { address: '0x02', name: 'Account 3' }, + }, + selectedAddress: '0x02', + }, + }, + messenger, + }); + + messenger.publish( + 'KeyringController:stateChange', + { + ...getDefaultKeyringState(), + keyrings: [{ accounts: ['0x00'], type: 'CustomKeyring' }], + }, + [], + ); + + expect(controller.state.selectedAddress).toBe('0x00'); + }); + + it('should not update existing identities', () => { + const identitiesState = { + '0x00': { address: '0x00', importTime: 1, name: 'Account 1' }, + '0x01': { address: '0x01', importTime: 2, name: 'Account 2' }, + '0x02': { address: '0x02', importTime: 3, name: 'Account 3' }, + }; + const messenger = getControllerMessenger(); + const controller = setupPreferencesController({ + options: { + state: { + identities: cloneDeep(identitiesState), + selectedAddress: '0x00', + }, + }, + messenger, + }); + + messenger.publish( + 'KeyringController:stateChange', + { + ...getDefaultKeyringState(), + keyrings: [ + { accounts: ['0x00', '0x01', '0x02'], type: 'CustomKeyring' }, + ], + }, + [], + ); + + expect(controller.state.identities).toStrictEqual(identitiesState); + }); + + it('should not duplicate accounts present in multiple keyrings', () => { + const identitiesState = { + '0x00': { address: '0x00', importTime: 1, name: 'Account 1' }, + '0x01': { address: '0x01', importTime: 2, name: 'Account 2' }, + '0x02': { address: '0x02', importTime: 3, name: 'Account 3' }, + }; + const messenger = getControllerMessenger(); + const controller = setupPreferencesController({ + options: { + state: { + identities: cloneDeep(identitiesState), + selectedAddress: '0x00', + }, + }, + messenger, + }); + + messenger.publish( + 'KeyringController:stateChange', + { + ...getDefaultKeyringState(), + keyrings: [ + { accounts: ['0x00', '0x01', '0x02'], type: 'CustomKeyring' }, + { accounts: ['0x00', '0x01', '0x02'], type: 'CustomKeyring' }, + ], + }, + [], + ); + + expect(controller.state.identities).toStrictEqual(identitiesState); + }); + + it('should not update selected address on account removal if it is still among identities', () => { + const identitiesState = { + '0x00': { address: '0x00', importTime: 1, name: 'Account 1' }, + '0x01': { address: '0x01', importTime: 2, name: 'Account 2' }, + '0x02': { address: '0x02', importTime: 3, name: 'Account 3' }, + }; + const messenger = getControllerMessenger(); + const controller = setupPreferencesController({ + options: { + state: { + identities: cloneDeep(identitiesState), + selectedAddress: '0x01', + }, + }, + messenger, + }); + + messenger.publish( + 'KeyringController:stateChange', + { + ...getDefaultKeyringState(), + keyrings: [{ accounts: ['0x00', '0x01'], type: 'CustomKeyring' }], + }, + [], + ); + + expect(controller.state.selectedAddress).toBe('0x01'); + }); + }); + it('should add identities', () => { const controller = setupPreferencesController(); controller.addIdentities(['0x00']); @@ -45,13 +245,15 @@ describe('PreferencesController', () => { it('should add multiple identities, skipping those that are already in state', () => { const controller = setupPreferencesController({ - state: { - identities: { - '0x00': { address: '0x00', name: 'Account 1' }, - '0x01': { address: '0x01', name: 'Account 2' }, - '0x02': { address: '0x02', name: 'Account 3' }, + options: { + state: { + identities: { + '0x00': { address: '0x00', name: 'Account 1' }, + '0x01': { address: '0x01', name: 'Account 2' }, + '0x02': { address: '0x02', name: 'Account 3' }, + }, + selectedAddress: '0x00', }, - selectedAddress: '0x00', }, }); @@ -76,13 +278,15 @@ describe('PreferencesController', () => { it('should remove identity', () => { const controller = setupPreferencesController({ - state: { - identities: { - '0x00': { address: '0x00', name: 'Account 1' }, - '0x01': { address: '0x01', name: 'Account 2' }, - '0x02': { address: '0x02', name: 'Account 3' }, + options: { + state: { + identities: { + '0x00': { address: '0x00', name: 'Account 1' }, + '0x01': { address: '0x01', name: 'Account 2' }, + '0x02': { address: '0x02', name: 'Account 3' }, + }, + selectedAddress: '0x00', }, - selectedAddress: '0x00', }, }); controller.removeIdentity('0x00'); @@ -138,8 +342,10 @@ describe('PreferencesController', () => { it('should not update existing identities', () => { const controller = setupPreferencesController({ - state: { - identities: { '0x01': { address: '0x01', name: 'Custom name' } }, + options: { + state: { + identities: { '0x01': { address: '0x01', name: 'Custom name' } }, + }, }, }); controller.updateIdentities(['0x00', '0x01']); @@ -155,10 +361,12 @@ describe('PreferencesController', () => { it('should remove identities', () => { const controller = setupPreferencesController({ - state: { - identities: { - '0x01': { address: '0x01', name: 'Account 2' }, - '0x00': { address: '0x00', name: 'Account 1' }, + options: { + state: { + identities: { + '0x01': { address: '0x01', name: 'Account 2' }, + '0x00': { address: '0x00', name: 'Account 1' }, + }, }, }, }); @@ -170,12 +378,14 @@ describe('PreferencesController', () => { it('should not update selected address if it is still among identities', () => { const controller = setupPreferencesController({ - state: { - identities: { - '0x01': { address: '0x01', name: 'Account 2' }, - '0x00': { address: '0x00', name: 'Account 1' }, + options: { + state: { + identities: { + '0x01': { address: '0x01', name: 'Account 2' }, + '0x00': { address: '0x00', name: 'Account 1' }, + }, + selectedAddress: '0x01', }, - selectedAddress: '0x01', }, }); controller.updateIdentities(['0x00', '0x01']); @@ -184,13 +394,15 @@ describe('PreferencesController', () => { it('should update selected address to first identity if it was removed from identities', () => { const controller = setupPreferencesController({ - state: { - identities: { - '0x01': { address: '0x01', name: 'Account 2' }, - '0x02': { address: '0x02', name: 'Account 3' }, - '0x00': { address: '0x00', name: 'Account 1' }, + options: { + state: { + identities: { + '0x01': { address: '0x01', name: 'Account 2' }, + '0x02': { address: '0x02', name: 'Account 3' }, + '0x00': { address: '0x00', name: 'Account 1' }, + }, + selectedAddress: '0x02', }, - selectedAddress: '0x02', }, }); controller.updateIdentities(['0x00', '0x01']); @@ -262,22 +474,50 @@ describe('PreferencesController', () => { }); }); +/** + * Construct a controller messenger for use in PreferencesController tests. + * + * This is a utility function that saves us from manually entering the correct + * type parameters for the ControllerMessenger each time we construct it. + * + * @returns A controller messenger + */ +function getControllerMessenger(): ControllerMessenger< + PreferencesControllerActions, + PreferencesControllerEvents | AllowedEvents +> { + return new ControllerMessenger< + PreferencesControllerActions, + PreferencesControllerEvents | AllowedEvents + >(); +} + /** * Setup a PreferencesController instance for testing. * - * @param options - PreferencesController options. + * @param args - Arguments + * @param args.options - PreferencesController options. + * @param args.messenger - A controller messenger. * @returns A PreferencesController instance. */ -function setupPreferencesController( - options: Partial[0]> = {}, -) { - const controllerMessenger = new ControllerMessenger(); +function setupPreferencesController({ + options = {}, + messenger, +}: { + options?: Partial[0]>; + messenger?: ControllerMessenger< + PreferencesControllerActions, + PreferencesControllerEvents | AllowedEvents + >; +} = {}) { + const controllerMessenger = messenger ?? getControllerMessenger(); const preferencesControllerMessenger = controllerMessenger.getRestricted< 'PreferencesController', never, - never + AllowedEvents['type'] >({ name: 'PreferencesController', + allowedEvents: ['KeyringController:stateChange'], }); return new PreferencesController({ messenger: preferencesControllerMessenger, diff --git a/packages/preferences-controller/src/PreferencesController.ts b/packages/preferences-controller/src/PreferencesController.ts index 3f847c738f..060b162c38 100644 --- a/packages/preferences-controller/src/PreferencesController.ts +++ b/packages/preferences-controller/src/PreferencesController.ts @@ -5,6 +5,10 @@ import { type RestrictedControllerMessenger, } from '@metamask/base-controller'; import { toChecksumHexAddress } from '@metamask/controller-utils'; +import type { + KeyringControllerState, + KeyringControllerStateChangeEvent, +} from '@metamask/keyring-controller'; import { ETHERSCAN_SUPPORTED_CHAIN_IDS } from './constants'; @@ -139,12 +143,14 @@ export type PreferencesControllerActions = PreferencesControllerGetStateAction; export type PreferencesControllerEvents = PreferencesControllerStateChangeEvent; +export type AllowedEvents = KeyringControllerStateChangeEvent; + export type PreferencesControllerMessenger = RestrictedControllerMessenger< typeof name, PreferencesControllerActions, - PreferencesControllerEvents, + PreferencesControllerEvents | AllowedEvents, never, - never + AllowedEvents['type'] >; /** @@ -224,6 +230,19 @@ export class PreferencesController extends BaseController< ...state, }, }); + + messenger.subscribe( + 'KeyringController:stateChange', + (keyringState: KeyringControllerState) => { + const accounts = new Set(); + for (const keyring of keyringState.keyrings) { + for (const account of keyring.accounts) { + accounts.add(account); + } + } + this.syncIdentities(Array.from(accounts)); + }, + ); } /** @@ -301,6 +320,7 @@ export class PreferencesController extends BaseController< * * @param addresses - List of addresses corresponding to identities to sync. * @returns Newly-selected address after syncing. + * @deprecated This will be removed in a future release */ syncIdentities(addresses: string[]) { addresses = addresses.map((address: string) => diff --git a/packages/preferences-controller/tsconfig.build.json b/packages/preferences-controller/tsconfig.build.json index bbfe057a20..6a7f4f10ac 100644 --- a/packages/preferences-controller/tsconfig.build.json +++ b/packages/preferences-controller/tsconfig.build.json @@ -7,7 +7,8 @@ }, "references": [ { "path": "../base-controller/tsconfig.build.json" }, - { "path": "../controller-utils/tsconfig.build.json" } + { "path": "../controller-utils/tsconfig.build.json" }, + { "path": "../keyring-controller/tsconfig.build.json" } ], "include": ["../../types", "./src"] } diff --git a/packages/preferences-controller/tsconfig.json b/packages/preferences-controller/tsconfig.json index 7ee9852347..0a81b60369 100644 --- a/packages/preferences-controller/tsconfig.json +++ b/packages/preferences-controller/tsconfig.json @@ -5,7 +5,8 @@ }, "references": [ { "path": "../base-controller" }, - { "path": "../controller-utils" } + { "path": "../controller-utils" }, + { "path": "../keyring-controller" } ], "include": ["../../types", "./src"] } diff --git a/yarn.lock b/yarn.lock index cdfbbdf1cd..515103ff52 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2245,7 +2245,6 @@ __metadata: "@metamask/eth-sig-util": ^7.0.1 "@metamask/keyring-api": ^3.0.0 "@metamask/message-manager": ^7.3.7 - "@metamask/preferences-controller": ^6.0.0 "@metamask/scure-bip39": ^2.1.1 "@metamask/utils": ^8.3.0 "@types/jest": ^27.4.1 @@ -2262,8 +2261,6 @@ __metadata: typedoc-plugin-missing-exports: ^2.0.0 typescript: ~4.8.4 uuid: ^8.3.2 - peerDependencies: - "@metamask/preferences-controller": ^6.0.0 languageName: unknown linkType: soft @@ -2529,13 +2526,17 @@ __metadata: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^4.1.0 "@metamask/controller-utils": ^8.0.1 + "@metamask/keyring-controller": ^12.1.0 "@types/jest": ^27.4.1 deepmerge: ^4.2.2 jest: ^27.5.1 + lodash: ^4.17.21 ts-jest: ^27.1.4 typedoc: ^0.24.8 typedoc-plugin-missing-exports: ^2.0.0 typescript: ~4.8.4 + peerDependencies: + "@metamask/keyring-controller": ^12.1.0 languageName: unknown linkType: soft