From 02b4cca0942b2c2c111681040c47d1def6bd2173 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 9 Jan 2024 17:51:47 -0330 Subject: [PATCH] feat(preferences-controller): Convert to BaseControllerV2 (#3713) ## Explanation The `PreferencesController` has been migrated to `BaseControllerV2`. As part of this migration, the unused `name` state property has also been removed. ## References Closes #3708 ## Changelog ### `@metamask/preferences-controller` #### Changed - **BREAKING:** Convert to `BaseControllerV2` - The constructor parameters have changed; rather than accepting an empty "config" parameter and a "state" parameter, there is now just a single object for all constructor arguments. This object has a mandatory `messenger` and an optional `state` property. - Additional type exports have been added for the controller messenger and associated types ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate --- .../src/PreferencesController.test.ts | 129 ++++++--- .../src/PreferencesController.ts | 263 +++++++++++------- 2 files changed, 262 insertions(+), 130 deletions(-) diff --git a/packages/preferences-controller/src/PreferencesController.test.ts b/packages/preferences-controller/src/PreferencesController.test.ts index 67a440be8c..cce9969e09 100644 --- a/packages/preferences-controller/src/PreferencesController.test.ts +++ b/packages/preferences-controller/src/PreferencesController.test.ts @@ -1,10 +1,12 @@ +import { ControllerMessenger } from '@metamask/base-controller'; + import { ETHERSCAN_SUPPORTED_CHAIN_IDS } from './constants'; import type { EtherscanSupportedHexChainId } from './PreferencesController'; import { PreferencesController } from './PreferencesController'; describe('PreferencesController', () => { it('should set default state', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); expect(controller.state).toStrictEqual({ featureFlags: {}, identities: {}, @@ -31,7 +33,7 @@ describe('PreferencesController', () => { }); it('should add identities', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.addIdentities(['0x00']); controller.addIdentities(['0x00']); expect(controller.state.identities['0x00'].address).toBe('0x00'); @@ -41,10 +43,48 @@ 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' }, + }, + selectedAddress: '0x00', + }, + }); + + controller.addIdentities(['0x00', '0x01', '0x02', '0x03', '0x04']); + + expect(controller.state.identities).toMatchObject({ + '0x00': { address: '0x00', name: 'Account 1' }, + '0x01': { address: '0x01', name: 'Account 2' }, + '0x02': { address: '0x02', name: 'Account 3' }, + '0x03': { + address: '0x03', + importTime: expect.any(Number), + name: 'Account 4', + }, + '0x04': { + address: '0x04', + importTime: expect.any(Number), + name: 'Account 5', + }, + }); + }); + it('should remove identity', () => { - const controller = new PreferencesController(); - controller.addIdentities(['0x00', '0x01', '0x02']); - controller.update({ selectedAddress: '0x00' }); + const controller = setupPreferencesController({ + state: { + identities: { + '0x00': { address: '0x00', name: 'Account 1' }, + '0x01': { address: '0x01', name: 'Account 2' }, + '0x02': { address: '0x02', name: 'Account 3' }, + }, + selectedAddress: '0x00', + }, + }); controller.removeIdentity('0x00'); controller.removeIdentity('0x02'); controller.removeIdentity('0x00'); @@ -53,7 +93,7 @@ describe('PreferencesController', () => { }); it('should set identity label', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.addIdentities(['0x00']); controller.setAccountLabel('0x00', 'bar'); controller.setAccountLabel('0x01', 'qux'); @@ -62,7 +102,7 @@ describe('PreferencesController', () => { }); it('should sync identities', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.addIdentities(['0x00', '0x01']); controller.syncIdentities(['0x00', '0x01']); expect(controller.state.identities['0x00'].address).toBe('0x00'); @@ -82,7 +122,7 @@ describe('PreferencesController', () => { }); it('should add new identities', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.updateIdentities(['0x00', '0x01']); expect(controller.state.identities['0x00'].address).toBe('0x00'); expect(controller.state.identities['0x00'].name).toBe('Account 1'); @@ -97,10 +137,11 @@ describe('PreferencesController', () => { }); it('should not update existing identities', () => { - const controller = new PreferencesController( - {}, - { identities: { '0x01': { address: '0x01', name: 'Custom name' } } }, - ); + const controller = setupPreferencesController({ + state: { + identities: { '0x01': { address: '0x01', name: 'Custom name' } }, + }, + }); controller.updateIdentities(['0x00', '0x01']); expect(controller.state.identities['0x00'].address).toBe('0x00'); expect(controller.state.identities['0x00'].name).toBe('Account 1'); @@ -113,15 +154,14 @@ describe('PreferencesController', () => { }); it('should remove identities', () => { - const controller = new PreferencesController( - {}, - { + const controller = setupPreferencesController({ + state: { identities: { '0x01': { address: '0x01', name: 'Account 2' }, '0x00': { address: '0x00', name: 'Account 1' }, }, }, - ); + }); controller.updateIdentities(['0x00']); expect(controller.state.identities).toStrictEqual({ '0x00': { address: '0x00', name: 'Account 1' }, @@ -129,24 +169,22 @@ describe('PreferencesController', () => { }); it('should not update selected address if it is still among identities', () => { - const controller = new PreferencesController( - {}, - { + const controller = setupPreferencesController({ + state: { identities: { '0x01': { address: '0x01', name: 'Account 2' }, '0x00': { address: '0x00', name: 'Account 1' }, }, selectedAddress: '0x01', }, - ); + }); controller.updateIdentities(['0x00', '0x01']); expect(controller.state.selectedAddress).toBe('0x01'); }); it('should update selected address to first identity if it was removed from identities', () => { - const controller = new PreferencesController( - {}, - { + const controller = setupPreferencesController({ + state: { identities: { '0x01': { address: '0x01', name: 'Account 2' }, '0x02': { address: '0x02', name: 'Account 3' }, @@ -154,19 +192,19 @@ describe('PreferencesController', () => { }, selectedAddress: '0x02', }, - ); + }); controller.updateIdentities(['0x00', '0x01']); expect(controller.state.selectedAddress).toBe('0x00'); }); it('should set IPFS gateway', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.setIpfsGateway('https://ipfs.infura.io/ipfs/'); expect(controller.state.ipfsGateway).toBe('https://ipfs.infura.io/ipfs/'); }); it('should update selected address as checksummed', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.setSelectedAddress('0x95d2bc047b0ddec1e4a178eeb64d59f5e735cd0a'); expect(controller.state.selectedAddress).toBe( '0x95D2bC047B0dDEc1E4A178EeB64d59F5E735cd0A', @@ -174,52 +212,75 @@ describe('PreferencesController', () => { }); it('should set useTokenDetection', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.setUseTokenDetection(true); expect(controller.state.useTokenDetection).toBe(true); }); it('should set useNftDetection', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.setOpenSeaEnabled(true); controller.setUseNftDetection(true); expect(controller.state.useNftDetection).toBe(true); }); it('should set securityAlertsEnabled', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.setSecurityAlertsEnabled(true); expect(controller.state.securityAlertsEnabled).toBe(true); }); it('should set disabledRpcMethodPreferences', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.setDisabledRpcMethodPreference('eth_sign', true); expect(controller.state.disabledRpcMethodPreferences.eth_sign).toBe(true); }); it('should set isMultiAccountBalancesEnabled', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.setIsMultiAccountBalancesEnabled(true); expect(controller.state.isMultiAccountBalancesEnabled).toBe(true); }); it('should set showTestNetworks', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.setShowTestNetworks(true); expect(controller.state.showTestNetworks).toBe(true); }); it('should set isIpfsGatewayEnabled', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.setIsIpfsGatewayEnabled(true); expect(controller.state.isIpfsGatewayEnabled).toBe(true); }); it('should set showIncomingTransactions to false on ethereum network', () => { - const controller = new PreferencesController(); + const controller = setupPreferencesController(); controller.setEnableNetworkIncomingTransactions('0x1', false); expect(controller.state.showIncomingTransactions['0x1']).toBe(false); }); }); + +/** + * Setup a PreferencesController instance for testing. + * + * @param options - PreferencesController options. + * @returns A PreferencesController instance. + */ +function setupPreferencesController( + options: Partial[0]> = {}, +) { + const controllerMessenger = new ControllerMessenger(); + const preferencesControllerMessenger = controllerMessenger.getRestricted< + 'PreferencesController', + never, + never + >({ + name: 'PreferencesController', + }); + return new PreferencesController({ + messenger: preferencesControllerMessenger, + ...options, + }); +} diff --git a/packages/preferences-controller/src/PreferencesController.ts b/packages/preferences-controller/src/PreferencesController.ts index c7a4acd457..3f847c738f 100644 --- a/packages/preferences-controller/src/PreferencesController.ts +++ b/packages/preferences-controller/src/PreferencesController.ts @@ -1,5 +1,9 @@ -import type { BaseConfig } from '@metamask/base-controller'; -import { BaseControllerV1 } from '@metamask/base-controller'; +import { + BaseController, + type ControllerStateChangeEvent, + type ControllerGetStateAction, + type RestrictedControllerMessenger, +} from '@metamask/base-controller'; import { toChecksumHexAddress } from '@metamask/controller-utils'; import { ETHERSCAN_SUPPORTED_CHAIN_IDS } from './constants'; @@ -70,12 +74,6 @@ export type PreferencesState = { * Map of lost addresses to Identity objects */ lostIdentities: { [address: string]: Identity }; - /** - * The name of the controller - * - * @deprecated This property is never set, and will be removed in a future release - */ - name?: string; /** * Controls whether the OpenSea API is used */ @@ -108,6 +106,47 @@ export type PreferencesState = { useTokenDetection: boolean; }; +const metadata = { + disabledRpcMethodPreferences: { persist: true, anonymous: true }, + featureFlags: { persist: true, anonymous: true }, + identities: { persist: true, anonymous: false }, + ipfsGateway: { persist: true, anonymous: false }, + isIpfsGatewayEnabled: { persist: true, anonymous: true }, + isMultiAccountBalancesEnabled: { persist: true, anonymous: true }, + lostIdentities: { persist: true, anonymous: false }, + openSeaEnabled: { persist: true, anonymous: true }, + securityAlertsEnabled: { persist: true, anonymous: true }, + selectedAddress: { persist: true, anonymous: false }, + showTestNetworks: { persist: true, anonymous: true }, + showIncomingTransactions: { persist: true, anonymous: true }, + useNftDetection: { persist: true, anonymous: true }, + useTokenDetection: { persist: true, anonymous: true }, +}; + +const name = 'PreferencesController'; + +export type PreferencesControllerGetStateAction = ControllerGetStateAction< + typeof name, + PreferencesState +>; + +export type PreferencesControllerStateChangeEvent = ControllerStateChangeEvent< + typeof name, + PreferencesState +>; + +export type PreferencesControllerActions = PreferencesControllerGetStateAction; + +export type PreferencesControllerEvents = PreferencesControllerStateChangeEvent; + +export type PreferencesControllerMessenger = RestrictedControllerMessenger< + typeof name, + PreferencesControllerActions, + PreferencesControllerEvents, + never, + never +>; + /** * Get the default PreferencesController state. * @@ -157,25 +196,34 @@ export function getDefaultPreferencesState() { /** * Controller that stores shared settings and exposes convenience methods */ -export class PreferencesController extends BaseControllerV1< - BaseConfig, - PreferencesState +export class PreferencesController extends BaseController< + typeof name, + PreferencesState, + PreferencesControllerMessenger > { - /** - * Name of this controller used during composition - */ - override name = 'PreferencesController'; - /** * Creates a PreferencesController instance. * - * @param config - Initial options used to configure this controller. - * @param state - Initial state to set on this controller. - */ - constructor(config?: Partial, state?: Partial) { - super(config, state); - this.defaultState = getDefaultPreferencesState(); - this.initialize(); + * @param args - Arguments + * @param args.messenger - The preferences controller messenger. + * @param args.state - Preferences controller state. + */ + constructor({ + messenger, + state, + }: { + messenger: PreferencesControllerMessenger; + state?: Partial; + }) { + super({ + name, + metadata, + messenger, + state: { + ...getDefaultPreferencesState(), + ...state, + }, + }); } /** @@ -184,21 +232,22 @@ export class PreferencesController extends BaseControllerV1< * @param addresses - List of addresses to use to generate new identities. */ addIdentities(addresses: string[]) { - const { identities } = this.state; - addresses.forEach((address) => { - address = toChecksumHexAddress(address); - if (identities[address]) { - return; + const checksummedAddresses = addresses.map(toChecksumHexAddress); + this.update((state) => { + const { identities } = state; + for (const address of checksummedAddresses) { + if (identities[address]) { + continue; + } + const identityCount = Object.keys(identities).length; + + identities[address] = { + name: `Account ${identityCount + 1}`, + address, + importTime: Date.now(), + }; } - const identityCount = Object.keys(identities).length; - - identities[address] = { - name: `Account ${identityCount + 1}`, - address, - importTime: Date.now(), - }; }); - this.update({ identities: { ...identities } }); } /** @@ -212,11 +261,12 @@ export class PreferencesController extends BaseControllerV1< if (!identities[address]) { return; } - delete identities[address]; - this.update({ identities: { ...identities } }); - if (address === this.state.selectedAddress) { - this.update({ selectedAddress: Object.keys(identities)[0] }); - } + this.update((state) => { + delete state.identities[address]; + if (address === state.selectedAddress) { + state.selectedAddress = Object.keys(state.identities)[0]; + } + }); } /** @@ -227,10 +277,11 @@ export class PreferencesController extends BaseControllerV1< */ setAccountLabel(address: string, label: string) { address = toChecksumHexAddress(address); - const { identities } = this.state; - identities[address] = identities[address] || {}; - identities[address].name = label; - this.update({ identities: { ...identities } }); + this.update((state) => { + const identity = state.identities[address] || {}; + identity.name = label; + state.identities[address] = identity; + }); } /** @@ -240,9 +291,9 @@ export class PreferencesController extends BaseControllerV1< * @param activated - Value to assign. */ setFeatureFlag(feature: string, activated: boolean) { - const oldFeatureFlags = this.state.featureFlags; - const featureFlags = { ...oldFeatureFlags, ...{ [feature]: activated } }; - this.update({ featureFlags: { ...featureFlags } }); + this.update((state) => { + state.featureFlags[feature] = activated; + }); } /** @@ -255,28 +306,28 @@ export class PreferencesController extends BaseControllerV1< addresses = addresses.map((address: string) => toChecksumHexAddress(address), ); - const { identities, lostIdentities } = this.state; - const newlyLost: { [address: string]: Identity } = {}; - for (const [address, identity] of Object.entries(identities)) { - if (!addresses.includes(address)) { - newlyLost[address] = identity; - delete identities[address]; - } - } + this.update((state) => { + const { identities } = state; + const newlyLost: { [address: string]: Identity } = {}; - for (const [address, identity] of Object.entries(newlyLost)) { - lostIdentities[address] = identity; - } + for (const [address, identity] of Object.entries(identities)) { + if (!addresses.includes(address)) { + newlyLost[address] = identity; + delete identities[address]; + } + } - this.update({ - identities: { ...identities }, - lostIdentities: { ...lostIdentities }, + for (const [address, identity] of Object.entries(newlyLost)) { + state.lostIdentities[address] = identity; + } }); this.addIdentities(addresses); if (!addresses.includes(this.state.selectedAddress)) { - this.update({ selectedAddress: addresses[0] }); + this.update((state) => { + state.selectedAddress = addresses[0]; + }); } return this.state.selectedAddress; @@ -293,23 +344,23 @@ export class PreferencesController extends BaseControllerV1< addresses = addresses.map((address: string) => toChecksumHexAddress(address), ); - const oldIdentities = this.state.identities; - const identities = addresses.reduce( - (ids: { [address: string]: Identity }, address, index) => { - ids[address] = oldIdentities[address] || { - address, - name: `Account ${index + 1}`, - importTime: Date.now(), - }; - return ids; - }, - {}, - ); - let { selectedAddress } = this.state; - if (!Object.keys(identities).includes(selectedAddress)) { - selectedAddress = Object.keys(identities)[0]; - } - this.update({ identities: { ...identities }, selectedAddress }); + this.update((state) => { + const identities = addresses.reduce( + (ids: { [address: string]: Identity }, address, index) => { + ids[address] = state.identities[address] || { + address, + name: `Account ${index + 1}`, + importTime: Date.now(), + }; + return ids; + }, + {}, + ); + state.identities = identities; + if (!Object.keys(identities).includes(state.selectedAddress)) { + state.selectedAddress = Object.keys(identities)[0]; + } + }); } /** @@ -318,7 +369,9 @@ export class PreferencesController extends BaseControllerV1< * @param selectedAddress - Ethereum address. */ setSelectedAddress(selectedAddress: string) { - this.update({ selectedAddress: toChecksumHexAddress(selectedAddress) }); + this.update((state) => { + state.selectedAddress = toChecksumHexAddress(selectedAddress); + }); } /** @@ -327,7 +380,9 @@ export class PreferencesController extends BaseControllerV1< * @param ipfsGateway - IPFS gateway string. */ setIpfsGateway(ipfsGateway: string) { - this.update({ ipfsGateway }); + this.update((state) => { + state.ipfsGateway = ipfsGateway; + }); } /** @@ -336,7 +391,9 @@ export class PreferencesController extends BaseControllerV1< * @param useTokenDetection - Boolean indicating user preference on token detection. */ setUseTokenDetection(useTokenDetection: boolean) { - this.update({ useTokenDetection }); + this.update((state) => { + state.useTokenDetection = useTokenDetection; + }); } /** @@ -350,7 +407,9 @@ export class PreferencesController extends BaseControllerV1< 'useNftDetection cannot be enabled if openSeaEnabled is false', ); } - this.update({ useNftDetection }); + this.update((state) => { + state.useNftDetection = useNftDetection; + }); } /** @@ -359,10 +418,12 @@ export class PreferencesController extends BaseControllerV1< * @param openSeaEnabled - Boolean indicating user preference on using OpenSea's API. */ setOpenSeaEnabled(openSeaEnabled: boolean) { - this.update({ openSeaEnabled }); - if (!openSeaEnabled) { - this.update({ useNftDetection: false }); - } + this.update((state) => { + state.openSeaEnabled = openSeaEnabled; + if (!openSeaEnabled) { + state.useNftDetection = false; + } + }); } /** @@ -371,7 +432,9 @@ export class PreferencesController extends BaseControllerV1< * @param securityAlertsEnabled - Boolean indicating user preference on using security alerts. */ setSecurityAlertsEnabled(securityAlertsEnabled: boolean) { - this.update({ securityAlertsEnabled }); + this.update((state) => { + state.securityAlertsEnabled = securityAlertsEnabled; + }); } /** @@ -386,7 +449,9 @@ export class PreferencesController extends BaseControllerV1< ...disabledRpcMethodPreferences, [methodName]: isEnabled, }; - this.update({ disabledRpcMethodPreferences: newDisabledRpcMethods }); + this.update((state) => { + state.disabledRpcMethodPreferences = newDisabledRpcMethods; + }); } /** @@ -395,7 +460,9 @@ export class PreferencesController extends BaseControllerV1< * @param isMultiAccountBalancesEnabled - true to enable multiple accounts balance fetch, false to fetch only selectedAddress. */ setIsMultiAccountBalancesEnabled(isMultiAccountBalancesEnabled: boolean) { - this.update({ isMultiAccountBalancesEnabled }); + this.update((state) => { + state.isMultiAccountBalancesEnabled = isMultiAccountBalancesEnabled; + }); } /** @@ -404,7 +471,9 @@ export class PreferencesController extends BaseControllerV1< * @param showTestNetworks - true to show test networks, false to hidden. */ setShowTestNetworks(showTestNetworks: boolean) { - this.update({ showTestNetworks }); + this.update((state) => { + state.showTestNetworks = showTestNetworks; + }); } /** @@ -413,7 +482,9 @@ export class PreferencesController extends BaseControllerV1< * @param isIpfsGatewayEnabled - true to enable ipfs source */ setIsIpfsGatewayEnabled(isIpfsGatewayEnabled: boolean) { - this.update({ isIpfsGatewayEnabled }); + this.update((state) => { + state.isIpfsGatewayEnabled = isIpfsGatewayEnabled; + }); } /** @@ -427,11 +498,11 @@ export class PreferencesController extends BaseControllerV1< isIncomingTransactionNetworkEnable: boolean, ) { if (Object.values(ETHERSCAN_SUPPORTED_CHAIN_IDS).includes(chainId)) { - this.update({ - showIncomingTransactions: { + this.update((state) => { + state.showIncomingTransactions = { ...this.state.showIncomingTransactions, [chainId]: isIncomingTransactionNetworkEnable, - }, + }; }); } }