From 5c939d904b71558459e93c1d1a1f67096ac2da1d Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 28 Nov 2024 23:36:02 +0100 Subject: [PATCH 1/8] feat: migrate AppStateController to inherit from BaseController V2 --- app/scripts/background.js | 4 +- .../controllers/app-state-controller.test.ts | 1010 ++++++++--------- .../controllers/app-state-controller.ts | 506 ++++++--- .../controllers/metametrics-controller.ts | 1 - .../controllers/mmi-controller.test.ts | 2 +- .../createRPCMethodTrackingMiddleware.test.js | 18 +- app/scripts/metamask-controller.js | 9 +- 7 files changed, 845 insertions(+), 705 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 2b2f5c4693df..1dca86ad3796 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -1024,8 +1024,8 @@ export function setupController( METAMASK_CONTROLLER_EVENTS.UPDATE_BADGE, updateBadge, ); - controller.appStateController.on( - METAMASK_CONTROLLER_EVENTS.UPDATE_BADGE, + controller.controllerMessenger.subscribe( + METAMASK_CONTROLLER_EVENTS.APP_STATE_UNLOCK_CHANGE, updateBadge, ); diff --git a/app/scripts/controllers/app-state-controller.test.ts b/app/scripts/controllers/app-state-controller.test.ts index 4bc1cb63e390..aa6edd6cb26d 100644 --- a/app/scripts/controllers/app-state-controller.test.ts +++ b/app/scripts/controllers/app-state-controller.test.ts @@ -1,4 +1,9 @@ import { ControllerMessenger } from '@metamask/base-controller'; +import type { + AcceptRequest, + AddApprovalRequest, +} from '@metamask/approval-controller'; +import { KeyringControllerQRKeyringStateChangeEvent } from '@metamask/keyring-controller'; import { Browser } from 'webextension-polyfill'; import { ENVIRONMENT_TYPE_POPUP, @@ -6,15 +11,19 @@ import { POLLING_TOKEN_ENVIRONMENT_TYPES, } from '../../../shared/constants/app'; import { AccountOverviewTabKey } from '../../../shared/constants/app-state'; +import { MINUTE } from '../../../shared/constants/time'; import { AppStateController } from './app-state-controller'; import type { - AllowedActions, - AllowedEvents, AppStateControllerActions, AppStateControllerEvents, + AppStateControllerOptions, AppStateControllerState, } from './app-state-controller'; -import { PreferencesControllerState } from './preferences-controller'; +import type { + PreferencesControllerState, + PreferencesControllerGetStateAction, + PreferencesControllerStateChangeEvent, +} from './preferences-controller'; jest.mock('webextension-polyfill'); @@ -25,12 +34,6 @@ jest.mock('../../../shared/modules/mv3.utils', () => ({ }, })); -let appStateController: AppStateController; -let controllerMessenger: ControllerMessenger< - AppStateControllerActions | AllowedActions, - AppStateControllerEvents | AllowedEvents ->; - const extensionMock = { alarms: { getAll: jest.fn(() => Promise.resolve([])), @@ -43,691 +46,660 @@ const extensionMock = { } as unknown as jest.Mocked; describe('AppStateController', () => { - const createAppStateController = ( - initState: Partial = {}, - ): { - appStateController: AppStateController; - controllerMessenger: typeof controllerMessenger; - } => { - controllerMessenger = new ControllerMessenger(); - jest.spyOn(ControllerMessenger.prototype, 'call'); - const appStateMessenger = controllerMessenger.getRestricted({ - name: 'AppStateController', - allowedActions: [ - `ApprovalController:addRequest`, - `ApprovalController:acceptRequest`, - `PreferencesController:getState`, - ], - allowedEvents: [ - `PreferencesController:stateChange`, - `KeyringController:qrKeyringStateChange`, - ], - }); - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ - preferences: { - autoLockTimeLimit: 0, - }, - }), - ); - controllerMessenger.registerActionHandler( - 'ApprovalController:addRequest', - jest.fn().mockReturnValue({ - catch: jest.fn(), - }), - ); - appStateController = new AppStateController({ - addUnlockListener: jest.fn(), - isUnlocked: jest.fn(() => true), - initState, - onInactiveTimeout: jest.fn(), - messenger: appStateMessenger, - extension: extensionMock, - }); - - return { appStateController, controllerMessenger }; - }; - - const createIsUnlockedMock = (isUnlocked: boolean) => { - return jest - .spyOn( - appStateController as unknown as { isUnlocked: () => boolean }, - 'isUnlocked', - ) - .mockReturnValue(isUnlocked); - }; - - beforeEach(() => { - ({ appStateController } = createAppStateController()); - }); - describe('setOutdatedBrowserWarningLastShown', () => { - it('sets the last shown time', () => { - ({ appStateController } = createAppStateController()); - const timestamp: number = Date.now(); + it('sets the last shown time', async () => { + await withController(({ controller }) => { + const timestamp: number = Date.now(); - appStateController.setOutdatedBrowserWarningLastShown(timestamp); + controller.setOutdatedBrowserWarningLastShown(timestamp); - expect( - appStateController.store.getState().outdatedBrowserWarningLastShown, - ).toStrictEqual(timestamp); + expect(controller.state.outdatedBrowserWarningLastShown).toStrictEqual( + timestamp, + ); + }); }); - it('sets outdated browser warning last shown timestamp', () => { - const lastShownTimestamp: number = Date.now(); - ({ appStateController } = createAppStateController()); - const updateStateSpy = jest.spyOn( - appStateController.store, - 'updateState', - ); + it('sets outdated browser warning last shown timestamp', async () => { + await withController(({ controller }) => { + const lastShownTimestamp: number = Date.now(); - appStateController.setOutdatedBrowserWarningLastShown(lastShownTimestamp); + controller.setOutdatedBrowserWarningLastShown(lastShownTimestamp); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - expect(updateStateSpy).toHaveBeenCalledWith({ - outdatedBrowserWarningLastShown: lastShownTimestamp, + expect(controller.state.outdatedBrowserWarningLastShown).toStrictEqual( + lastShownTimestamp, + ); }); - - updateStateSpy.mockRestore(); }); }); describe('getUnlockPromise', () => { it('waits for unlock if the extension is locked', async () => { - ({ appStateController } = createAppStateController()); - const isUnlockedMock = createIsUnlockedMock(false); - const waitForUnlockSpy = jest.spyOn(appStateController, 'waitForUnlock'); - - appStateController.getUnlockPromise(true); - expect(isUnlockedMock).toHaveBeenCalled(); - expect(waitForUnlockSpy).toHaveBeenCalledWith(expect.any(Function), true); + await withController(({ controller }) => { + const isUnlockedMock = jest + .spyOn(controller, 'isUnlocked') + .mockReturnValue(false); + expect(controller.waitingForUnlock).toHaveLength(0); + + controller.getUnlockPromise(true); + expect(isUnlockedMock).toHaveBeenCalled(); + expect(controller.waitingForUnlock).toHaveLength(1); + }); }); it('resolves immediately if the extension is already unlocked', async () => { - ({ appStateController } = createAppStateController()); - const isUnlockedMock = createIsUnlockedMock(true); + await withController(async ({ controller }) => { + const isUnlockedMock = jest + .spyOn(controller, 'isUnlocked') + .mockReturnValue(true); - await expect( - appStateController.getUnlockPromise(false), - ).resolves.toBeUndefined(); + await expect( + controller.getUnlockPromise(false), + ).resolves.toBeUndefined(); - expect(isUnlockedMock).toHaveBeenCalled(); + expect(isUnlockedMock).toHaveBeenCalled(); + }); }); - }); - describe('waitForUnlock', () => { - it('resolves immediately if already unlocked', async () => { - const emitSpy = jest.spyOn(appStateController, 'emit'); - const resolveFn: () => void = jest.fn(); - appStateController.waitForUnlock(resolveFn, false); - expect(emitSpy).toHaveBeenCalledWith('updateBadge'); - expect(controllerMessenger.call).toHaveBeenCalledTimes(1); + it("doesn't resolves immediately if unlocked is false", async () => { + await withController(async ({ controller, controllerMessenger }) => { + jest.spyOn(controller, 'isUnlocked').mockReturnValue(false); + + const unlockPromise = controller.getUnlockPromise(false); + + const timeoutPromise = new Promise((resolve) => + setTimeout(() => resolve('timeout'), 100), + ); + + const result = await Promise.race([unlockPromise, timeoutPromise]); + + expect(result).toBe('timeout'); + + expect(controllerMessenger.publish).toHaveBeenCalledWith( + 'AppStateController:unlockChange', + ); + expect(controllerMessenger.call).toHaveBeenCalledTimes(1); + }); }); it('creates approval request when waitForUnlock is called with shouldShowUnlockRequest as true', async () => { - createIsUnlockedMock(false); - - const resolveFn: () => void = jest.fn(); - appStateController.waitForUnlock(resolveFn, true); - - expect(controllerMessenger.call).toHaveBeenCalledTimes(2); - expect(controllerMessenger.call).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - expect.objectContaining({ - id: expect.any(String), - origin: ORIGIN_METAMASK, - type: 'unlock', - }), - true, - ); + await withController(async ({ controller, controllerMessenger }) => { + jest.spyOn(controller, 'isUnlocked').mockReturnValue(false); + + controller.getUnlockPromise(true); + + expect(controllerMessenger.call).toHaveBeenCalledTimes(2); + expect(controllerMessenger.call).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + expect.objectContaining({ + id: expect.any(String), + origin: ORIGIN_METAMASK, + type: 'unlock', + }), + true, + ); + }); }); - }); - describe('handleUnlock', () => { - beforeEach(() => { - createIsUnlockedMock(false); - }); - afterEach(() => { - jest.clearAllMocks(); - }); it('accepts approval request revolving all the related promises', async () => { - const emitSpy = jest.spyOn(appStateController, 'emit'); - const resolveFn: () => void = jest.fn(); - appStateController.waitForUnlock(resolveFn, true); - - appStateController.handleUnlock(); - - expect(emitSpy).toHaveBeenCalled(); - expect(emitSpy).toHaveBeenCalledWith('updateBadge'); - expect(controllerMessenger.call).toHaveBeenCalled(); - expect(controllerMessenger.call).toHaveBeenCalledWith( - 'ApprovalController:acceptRequest', - expect.any(String), + let unlockListener: () => void; + await withController( + { + addUnlockListener: (listener) => { + unlockListener = listener; + }, + }, + ({ controller, controllerMessenger }) => { + jest.spyOn(controller, 'isUnlocked').mockReturnValue(false); + controller.getUnlockPromise(true); + + unlockListener(); + + expect(controllerMessenger.publish).toHaveBeenCalled(); + expect(controllerMessenger.publish).toHaveBeenCalledWith( + 'AppStateController:unlockChange', + ); + expect(controllerMessenger.call).toHaveBeenCalled(); + expect(controllerMessenger.call).toHaveBeenCalledWith( + 'ApprovalController:acceptRequest', + expect.any(String), + ); + }, ); }); }); describe('setDefaultHomeActiveTabName', () => { - it('sets the default home tab name', () => { - appStateController.setDefaultHomeActiveTabName( - AccountOverviewTabKey.Activity, - ); - expect(appStateController.store.getState().defaultHomeActiveTabName).toBe( - AccountOverviewTabKey.Activity, - ); + it('sets the default home tab name', async () => { + await withController(({ controller }) => { + controller.setDefaultHomeActiveTabName(AccountOverviewTabKey.Activity); + + expect(controller.state.defaultHomeActiveTabName).toBe( + AccountOverviewTabKey.Activity, + ); + }); }); }); describe('setConnectedStatusPopoverHasBeenShown', () => { - it('sets connected status popover as shown', () => { - appStateController.setConnectedStatusPopoverHasBeenShown(); - expect( - appStateController.store.getState().connectedStatusPopoverHasBeenShown, - ).toBe(true); + it('sets connected status popover as shown', async () => { + await withController(({ controller }) => { + controller.setConnectedStatusPopoverHasBeenShown(); + + expect(controller.state.connectedStatusPopoverHasBeenShown).toBe(true); + }); }); }); describe('setRecoveryPhraseReminderHasBeenShown', () => { - it('sets recovery phrase reminder as shown', () => { - appStateController.setRecoveryPhraseReminderHasBeenShown(); - expect( - appStateController.store.getState().recoveryPhraseReminderHasBeenShown, - ).toBe(true); + it('sets recovery phrase reminder as shown', async () => { + await withController(({ controller }) => { + controller.setRecoveryPhraseReminderHasBeenShown(); + + expect(controller.state.recoveryPhraseReminderHasBeenShown).toBe(true); + }); }); }); describe('setRecoveryPhraseReminderLastShown', () => { - it('sets the last shown time of recovery phrase reminder', () => { - const timestamp: number = Date.now(); - appStateController.setRecoveryPhraseReminderLastShown(timestamp); - - expect( - appStateController.store.getState().recoveryPhraseReminderLastShown, - ).toBe(timestamp); + it('sets the last shown time of recovery phrase reminder', async () => { + await withController(({ controller }) => { + const timestamp = Date.now(); + controller.setRecoveryPhraseReminderLastShown(timestamp); + + expect(controller.state.recoveryPhraseReminderLastShown).toBe( + timestamp, + ); + }); }); }); describe('setLastActiveTime', () => { - it('sets the last active time to the current time', () => { - const spy = jest.spyOn( - appStateController as unknown as { _resetTimer: () => void }, - '_resetTimer', - ); - appStateController.setLastActiveTime(); - - expect(spy).toHaveBeenCalled(); + it('sets the timer if timeoutMinutes is set', async () => { + await withController(({ controller, controllerMessenger }) => { + const timeout = Date.now(); + controllerMessenger.publish( + 'PreferencesController:stateChange', + { + preferences: { autoLockTimeLimit: timeout }, + } as unknown as PreferencesControllerState, + [], + ); + jest.spyOn(global, 'setTimeout'); + + controller.setLastActiveTime(); + + expect(setTimeout).toHaveBeenCalledWith( + expect.any(Function), + timeout * MINUTE, + ); + }); }); - it('sets the timer if timeoutMinutes is set', () => { - const timeout = Date.now(); - controllerMessenger.publish( - 'PreferencesController:stateChange', - { - preferences: { autoLockTimeLimit: timeout }, - } as unknown as PreferencesControllerState, - [], - ); - const spy = jest.spyOn( - appStateController as unknown as { _resetTimer: () => void }, - '_resetTimer', - ); - appStateController.setLastActiveTime(); + it("doesn't set the timer if timeoutMinutes is not set", async () => { + await withController(({ controller }) => { + jest.spyOn(global, 'setTimeout'); + + controller.setLastActiveTime(); - expect(spy).toHaveBeenCalled(); + expect(setTimeout).toHaveBeenCalledTimes(0); + }); }); }); describe('setBrowserEnvironment', () => { - it('sets the current browser and OS environment', () => { - appStateController.setBrowserEnvironment('Windows', 'Chrome'); - expect( - appStateController.store.getState().browserEnvironment, - ).toStrictEqual({ - os: 'Windows', - browser: 'Chrome', + it('sets the current browser and OS environment', async () => { + await withController(({ controller }) => { + controller.setBrowserEnvironment('Windows', 'Chrome'); + + expect(controller.state.browserEnvironment).toStrictEqual({ + os: 'Windows', + browser: 'Chrome', + }); }); }); }); describe('addPollingToken', () => { - it('adds a pollingToken for a given environmentType', () => { - const pollingTokenType = - POLLING_TOKEN_ENVIRONMENT_TYPES[ENVIRONMENT_TYPE_POPUP]; - appStateController.addPollingToken('token1', pollingTokenType); - expect(appStateController.store.getState()[pollingTokenType]).toContain( - 'token1', - ); + it('adds a pollingToken for a given environmentType', async () => { + await withController(({ controller }) => { + const pollingTokenType = + POLLING_TOKEN_ENVIRONMENT_TYPES[ENVIRONMENT_TYPE_POPUP]; + controller.addPollingToken('token1', pollingTokenType); + + expect(controller.state[pollingTokenType]).toContain('token1'); + }); }); }); describe('removePollingToken', () => { - it('removes a pollingToken for a given environmentType', () => { - const pollingTokenType = - POLLING_TOKEN_ENVIRONMENT_TYPES[ENVIRONMENT_TYPE_POPUP]; - appStateController.addPollingToken('token1', pollingTokenType); - appStateController.removePollingToken('token1', pollingTokenType); - expect( - appStateController.store.getState()[pollingTokenType], - ).not.toContain('token1'); + it('removes a pollingToken for a given environmentType', async () => { + await withController(({ controller }) => { + const pollingTokenType = + POLLING_TOKEN_ENVIRONMENT_TYPES[ENVIRONMENT_TYPE_POPUP]; + + controller.addPollingToken('token1', pollingTokenType); + controller.removePollingToken('token1', pollingTokenType); + + expect(controller.state[pollingTokenType]).not.toContain('token1'); + }); }); }); describe('clearPollingTokens', () => { - it('clears all pollingTokens', () => { - appStateController.addPollingToken('token1', 'popupGasPollTokens'); - appStateController.addPollingToken('token2', 'notificationGasPollTokens'); - appStateController.addPollingToken('token3', 'fullScreenGasPollTokens'); - appStateController.clearPollingTokens(); - - expect( - appStateController.store.getState().popupGasPollTokens, - ).toStrictEqual([]); - expect( - appStateController.store.getState().notificationGasPollTokens, - ).toStrictEqual([]); - expect( - appStateController.store.getState().fullScreenGasPollTokens, - ).toStrictEqual([]); + it('clears all pollingTokens', async () => { + await withController(({ controller }) => { + controller.addPollingToken('token1', 'popupGasPollTokens'); + controller.addPollingToken('token2', 'notificationGasPollTokens'); + controller.addPollingToken('token3', 'fullScreenGasPollTokens'); + controller.clearPollingTokens(); + + expect(controller.state.popupGasPollTokens).toStrictEqual([]); + expect(controller.state.notificationGasPollTokens).toStrictEqual([]); + expect(controller.state.fullScreenGasPollTokens).toStrictEqual([]); + }); }); }); describe('setShowTestnetMessageInDropdown', () => { - it('sets whether the testnet dismissal link should be shown in the network dropdown', () => { - appStateController.setShowTestnetMessageInDropdown(true); - expect( - appStateController.store.getState().showTestnetMessageInDropdown, - ).toBe(true); + it('sets whether the testnet dismissal link should be shown in the network dropdown', async () => { + await withController(({ controller }) => { + controller.setShowTestnetMessageInDropdown(true); + + expect(controller.state.showTestnetMessageInDropdown).toBe(true); - appStateController.setShowTestnetMessageInDropdown(false); - expect( - appStateController.store.getState().showTestnetMessageInDropdown, - ).toBe(false); + controller.setShowTestnetMessageInDropdown(false); + + expect(controller.state.showTestnetMessageInDropdown).toBe(false); + }); }); }); describe('setShowBetaHeader', () => { - it('sets whether the beta notification heading on the home page', () => { - appStateController.setShowBetaHeader(true); - expect(appStateController.store.getState().showBetaHeader).toBe(true); + it('sets whether the beta notification heading on the home page', async () => { + await withController(({ controller }) => { + controller.setShowBetaHeader(true); - appStateController.setShowBetaHeader(false); - expect(appStateController.store.getState().showBetaHeader).toBe(false); + expect(controller.state.showBetaHeader).toBe(true); + + controller.setShowBetaHeader(false); + + expect(controller.state.showBetaHeader).toBe(false); + }); }); }); describe('setCurrentPopupId', () => { - it('sets the currentPopupId in the appState', () => { - const popupId = 12345; + it('sets the currentPopupId in the appState', async () => { + await withController(({ controller }) => { + const popupId = 12345; - appStateController.setCurrentPopupId(popupId); - expect(appStateController.store.getState().currentPopupId).toBe(popupId); + controller.setCurrentPopupId(popupId); + + expect(controller.state.currentPopupId).toBe(popupId); + }); }); }); describe('getCurrentPopupId', () => { - it('retrieves the currentPopupId saved in the appState', () => { - const popupId = 54321; + it('retrieves the currentPopupId saved in the appState', async () => { + await withController(({ controller }) => { + const popupId = 54321; - appStateController.setCurrentPopupId(popupId); - expect(appStateController.getCurrentPopupId()).toBe(popupId); + controller.setCurrentPopupId(popupId); + + expect(controller.getCurrentPopupId()).toBe(popupId); + }); }); }); describe('setFirstTimeUsedNetwork', () => { - it('updates the array of the first time used networks', () => { - const chainId = '0x1'; + it('updates the array of the first time used networks', async () => { + await withController(({ controller }) => { + const chainId = '0x1'; - appStateController.setFirstTimeUsedNetwork(chainId); - expect(appStateController.store.getState().usedNetworks[chainId]).toBe( - true, - ); + controller.setFirstTimeUsedNetwork(chainId); + + expect(controller.state.usedNetworks[chainId]).toBe(true); + }); }); }); describe('setLastInteractedConfirmationInfo', () => { - it('sets information about last confirmation user has interacted with', () => { - const lastInteractedConfirmationInfo = { - id: '123', - chainId: '0x1', - timestamp: new Date().getTime(), - }; - appStateController.setLastInteractedConfirmationInfo( - lastInteractedConfirmationInfo, - ); - expect(appStateController.getLastInteractedConfirmationInfo()).toBe( - lastInteractedConfirmationInfo, - ); + it('sets information about last confirmation user has interacted with', async () => { + await withController(({ controller }) => { + const lastInteractedConfirmationInfo = { + id: '123', + chainId: '0x1', + timestamp: new Date().getTime(), + }; - appStateController.setLastInteractedConfirmationInfo(undefined); - expect(appStateController.getLastInteractedConfirmationInfo()).toBe( - undefined, - ); + controller.setLastInteractedConfirmationInfo( + lastInteractedConfirmationInfo, + ); + + expect(controller.getLastInteractedConfirmationInfo()).toBe( + lastInteractedConfirmationInfo, + ); + + controller.setLastInteractedConfirmationInfo(undefined); + + expect(controller.getLastInteractedConfirmationInfo()).toBe(undefined); + }); }); }); describe('setSnapsInstallPrivacyWarningShownStatus', () => { - it('updates the status of snaps install privacy warning', () => { - ({ appStateController } = createAppStateController()); - const updateStateSpy = jest.spyOn( - appStateController.store, - 'updateState', - ); - - appStateController.setSnapsInstallPrivacyWarningShownStatus(true); + it('updates the status of snaps install privacy warning', async () => { + await withController(({ controller }) => { + controller.setSnapsInstallPrivacyWarningShownStatus(true); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - expect(updateStateSpy).toHaveBeenCalledWith({ - snapsInstallPrivacyWarningShown: true, + expect(controller.state.snapsInstallPrivacyWarningShown).toStrictEqual( + true, + ); }); - - updateStateSpy.mockRestore(); }); }); describe('institutional', () => { - it('set the interactive replacement token with a url and the old refresh token', () => { - ({ appStateController } = createAppStateController()); - const updateStateSpy = jest.spyOn( - appStateController.store, - 'updateState', - ); - - const mockParams = { - url: 'https://example.com', - oldRefreshToken: 'old', - }; - - appStateController.showInteractiveReplacementTokenBanner(mockParams); - - expect(updateStateSpy).toHaveBeenCalledTimes(1); - expect(updateStateSpy).toHaveBeenCalledWith({ - interactiveReplacementToken: mockParams, + it('set the interactive replacement token with a url and the old refresh token', async () => { + await withController(({ controller }) => { + const mockParams = { + url: 'https://example.com', + oldRefreshToken: 'old', + }; + + controller.showInteractiveReplacementTokenBanner(mockParams); + + expect(controller.state.interactiveReplacementToken).toStrictEqual( + mockParams, + ); }); - - updateStateSpy.mockRestore(); }); - it('set the setCustodianDeepLink with the fromAddress and custodyId', () => { - ({ appStateController } = createAppStateController()); - const updateStateSpy = jest.spyOn( - appStateController.store, - 'updateState', - ); - - const mockParams = { - fromAddress: '0x', - custodyId: 'custodyId', - }; + it('set the setCustodianDeepLink with the fromAddress and custodyId', async () => { + await withController(({ controller }) => { + const mockParams = { + fromAddress: '0x', + custodyId: 'custodyId', + }; - appStateController.setCustodianDeepLink(mockParams); + controller.setCustodianDeepLink(mockParams); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - expect(updateStateSpy).toHaveBeenCalledWith({ - custodianDeepLink: mockParams, + expect(controller.state.custodianDeepLink).toStrictEqual(mockParams); }); - - updateStateSpy.mockRestore(); }); - it('set the setNoteToTraderMessage with a message', () => { - ({ appStateController } = createAppStateController()); - const updateStateSpy = jest.spyOn( - appStateController.store, - 'updateState', - ); - - const mockParams = 'some message'; + it('set the setNoteToTraderMessage with a message', async () => { + await withController(({ controller }) => { + const mockParams = 'some message'; - appStateController.setNoteToTraderMessage(mockParams); + controller.setNoteToTraderMessage(mockParams); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - expect(updateStateSpy).toHaveBeenCalledWith({ - noteToTraderMessage: mockParams, + expect(controller.state.noteToTraderMessage).toStrictEqual(mockParams); }); - - updateStateSpy.mockRestore(); }); }); describe('setSurveyLinkLastClickedOrClosed', () => { - it('set the surveyLinkLastClickedOrClosed time', () => { - ({ appStateController } = createAppStateController()); - const updateStateSpy = jest.spyOn( - appStateController.store, - 'updateState', - ); - - const mockParams = Date.now(); + it('set the surveyLinkLastClickedOrClosed time', async () => { + await withController(({ controller }) => { + const mockParams = Date.now(); - appStateController.setSurveyLinkLastClickedOrClosed(mockParams); + controller.setSurveyLinkLastClickedOrClosed(mockParams); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - expect(updateStateSpy).toHaveBeenCalledWith({ - surveyLinkLastClickedOrClosed: mockParams, + expect(controller.state.surveyLinkLastClickedOrClosed).toStrictEqual( + mockParams, + ); }); - - updateStateSpy.mockRestore(); }); }); describe('setOnboardingDate', () => { - it('set the onboardingDate', () => { - ({ appStateController } = createAppStateController()); - const updateStateSpy = jest.spyOn( - appStateController.store, - 'updateState', - ); + it('set the onboardingDate', async () => { + await withController(({ controller }) => { + const mockDateNow = 1620000000000; + jest.spyOn(Date, 'now').mockReturnValue(mockDateNow); - appStateController.setOnboardingDate(); + controller.setOnboardingDate(); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - - updateStateSpy.mockRestore(); + expect(controller.state.onboardingDate).toStrictEqual(mockDateNow); + }); }); }); describe('setLastViewedUserSurvey', () => { - it('set the lastViewedUserSurvey with id 1', () => { - ({ appStateController } = createAppStateController()); - const updateStateSpy = jest.spyOn( - appStateController.store, - 'updateState', - ); - - const mockParams = 1; + it('set the lastViewedUserSurvey with id 1', async () => { + await withController(({ controller }) => { + const mockParams = 1; - appStateController.setLastViewedUserSurvey(mockParams); + controller.setLastViewedUserSurvey(mockParams); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - expect(updateStateSpy).toHaveBeenCalledWith({ - lastViewedUserSurvey: mockParams, + expect(controller.state.lastViewedUserSurvey).toStrictEqual(mockParams); }); - - updateStateSpy.mockRestore(); }); }); describe('setNewPrivacyPolicyToastClickedOrClosed', () => { - it('set the newPrivacyPolicyToastClickedOrClosed to true', () => { - ({ appStateController } = createAppStateController()); - const updateStateSpy = jest.spyOn( - appStateController.store, - 'updateState', - ); - - appStateController.setNewPrivacyPolicyToastClickedOrClosed(); + it('set the newPrivacyPolicyToastClickedOrClosed to true', async () => { + await withController(({ controller }) => { + controller.setNewPrivacyPolicyToastClickedOrClosed(); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - expect( - appStateController.store.getState() - .newPrivacyPolicyToastClickedOrClosed, - ).toStrictEqual(true); - - updateStateSpy.mockRestore(); + expect( + controller.state.newPrivacyPolicyToastClickedOrClosed, + ).toStrictEqual(true); + }); }); }); describe('setNewPrivacyPolicyToastShownDate', () => { - it('set the newPrivacyPolicyToastShownDate', () => { - ({ appStateController } = createAppStateController()); - const updateStateSpy = jest.spyOn( - appStateController.store, - 'updateState', - ); - - const mockParams = Date.now(); + it('set the newPrivacyPolicyToastShownDate', async () => { + await withController(({ controller }) => { + const mockParams = Date.now(); - appStateController.setNewPrivacyPolicyToastShownDate(mockParams); + controller.setNewPrivacyPolicyToastShownDate(mockParams); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - expect(updateStateSpy).toHaveBeenCalledWith({ - newPrivacyPolicyToastShownDate: mockParams, + expect(controller.state.newPrivacyPolicyToastShownDate).toStrictEqual( + mockParams, + ); }); - expect( - appStateController.store.getState().newPrivacyPolicyToastShownDate, - ).toStrictEqual(mockParams); - - updateStateSpy.mockRestore(); }); }); describe('setTermsOfUseLastAgreed', () => { - it('set the termsOfUseLastAgreed timestamp', () => { - ({ appStateController } = createAppStateController()); - const updateStateSpy = jest.spyOn( - appStateController.store, - 'updateState', - ); - - const mockParams = Date.now(); + it('set the termsOfUseLastAgreed timestamp', async () => { + await withController(({ controller }) => { + const mockParams = Date.now(); - appStateController.setTermsOfUseLastAgreed(mockParams); + controller.setTermsOfUseLastAgreed(mockParams); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - expect(updateStateSpy).toHaveBeenCalledWith({ - termsOfUseLastAgreed: mockParams, + expect(controller.state.termsOfUseLastAgreed).toStrictEqual(mockParams); }); - expect( - appStateController.store.getState().termsOfUseLastAgreed, - ).toStrictEqual(mockParams); - - updateStateSpy.mockRestore(); }); }); describe('onPreferencesStateChange', () => { - it('should update the timeoutMinutes with the autoLockTimeLimit', () => { - ({ appStateController, controllerMessenger } = - createAppStateController()); - const timeout = Date.now(); - - controllerMessenger.publish( - 'PreferencesController:stateChange', - { - preferences: { autoLockTimeLimit: timeout }, - } as unknown as PreferencesControllerState, - [], - ); - - expect(appStateController.store.getState().timeoutMinutes).toStrictEqual( - timeout, - ); + it('should update the timeoutMinutes with the autoLockTimeLimit', async () => { + await withController(({ controller, controllerMessenger }) => { + const timeout = Date.now(); + + controllerMessenger.publish( + 'PreferencesController:stateChange', + { + preferences: { autoLockTimeLimit: timeout }, + } as unknown as PreferencesControllerState, + [], + ); + + expect(controller.state.timeoutMinutes).toStrictEqual(timeout); + }); }); }); describe('isManifestV3', () => { - it('creates alarm when isManifestV3 is true', () => { + it('creates alarm when isManifestV3 is true', async () => { mockIsManifestV3.mockReturnValue(true); - ({ appStateController } = createAppStateController()); - - const timeout = Date.now(); - controllerMessenger.publish( - 'PreferencesController:stateChange', - { - preferences: { autoLockTimeLimit: timeout }, - } as unknown as PreferencesControllerState, - [], - ); - const spy = jest.spyOn( - appStateController as unknown as { _resetTimer: () => void }, - '_resetTimer', - ); - appStateController.setLastActiveTime(); - - expect(spy).toHaveBeenCalled(); - expect(extensionMock.alarms.clear).toHaveBeenCalled(); - expect(extensionMock.alarms.onAlarm.addListener).toHaveBeenCalled(); + await withController(({ controller, controllerMessenger }) => { + const timeout = Date.now(); + controllerMessenger.publish( + 'PreferencesController:stateChange', + { + preferences: { autoLockTimeLimit: timeout }, + } as unknown as PreferencesControllerState, + [], + ); + controller.setLastActiveTime(); + + expect(extensionMock.alarms.clear).toHaveBeenCalled(); + expect(extensionMock.alarms.onAlarm.addListener).toHaveBeenCalled(); + }); }); }); describe('AppStateController:getState', () => { - it('should return the current state of the property', () => { - expect( - appStateController.store.getState().recoveryPhraseReminderHasBeenShown, - ).toStrictEqual(false); - expect( - controllerMessenger.call('AppStateController:getState') - .recoveryPhraseReminderHasBeenShown, - ).toStrictEqual(false); + it('should return the current state of the property', async () => { + await withController(({ controller, controllerMessenger }) => { + expect( + controller.state.recoveryPhraseReminderHasBeenShown, + ).toStrictEqual(false); + + expect( + controllerMessenger.call('AppStateController:getState') + .recoveryPhraseReminderHasBeenShown, + ).toStrictEqual(false); + }); }); }); describe('AppStateController:stateChange', () => { - it('subscribers will recieve the state when published', () => { - expect( - appStateController.store.getState().surveyLinkLastClickedOrClosed, - ).toStrictEqual(null); - const timeNow = Date.now(); - controllerMessenger.subscribe( - 'AppStateController:stateChange', - (state: Partial) => { - if (typeof state.surveyLinkLastClickedOrClosed === 'number') { - appStateController.setSurveyLinkLastClickedOrClosed( - state.surveyLinkLastClickedOrClosed, - ); - } - }, - ); - - controllerMessenger.publish( - 'AppStateController:stateChange', - { - surveyLinkLastClickedOrClosed: timeNow, - } as unknown as AppStateControllerState, - [], - ); - - expect( - appStateController.store.getState().surveyLinkLastClickedOrClosed, - ).toStrictEqual(timeNow); - expect( - controllerMessenger.call('AppStateController:getState') - .surveyLinkLastClickedOrClosed, - ).toStrictEqual(timeNow); - }); - - it('state will be published when there is state change', () => { - expect( - appStateController.store.getState().surveyLinkLastClickedOrClosed, - ).toStrictEqual(null); - const timeNow = Date.now(); - controllerMessenger.subscribe( - 'AppStateController:stateChange', - (state: Partial) => { - expect(state.surveyLinkLastClickedOrClosed).toStrictEqual(timeNow); - }, - ); - - appStateController.setSurveyLinkLastClickedOrClosed(timeNow); + it('subscribers will recieve the state when published', async () => { + await withController(({ controller, controllerMessenger }) => { + expect(controller.state.surveyLinkLastClickedOrClosed).toStrictEqual( + null, + ); + const timeNow = Date.now(); + controllerMessenger.subscribe( + 'AppStateController:stateChange', + (state: Partial) => { + if (typeof state.surveyLinkLastClickedOrClosed === 'number') { + controller.setSurveyLinkLastClickedOrClosed( + state.surveyLinkLastClickedOrClosed, + ); + } + }, + ); + + controllerMessenger.publish( + 'AppStateController:stateChange', + { + surveyLinkLastClickedOrClosed: timeNow, + } as unknown as AppStateControllerState, + [], + ); + + expect(controller.state.surveyLinkLastClickedOrClosed).toStrictEqual( + timeNow, + ); + expect( + controllerMessenger.call('AppStateController:getState') + .surveyLinkLastClickedOrClosed, + ).toStrictEqual(timeNow); + }); + }); - expect( - appStateController.store.getState().surveyLinkLastClickedOrClosed, - ).toStrictEqual(timeNow); - expect( - controllerMessenger.call('AppStateController:getState') - .surveyLinkLastClickedOrClosed, - ).toStrictEqual(timeNow); + it('state will be published when there is state change', async () => { + await withController(({ controller, controllerMessenger }) => { + expect(controller.state.surveyLinkLastClickedOrClosed).toStrictEqual( + null, + ); + const timeNow = Date.now(); + controllerMessenger.subscribe( + 'AppStateController:stateChange', + (state: Partial) => { + expect(state.surveyLinkLastClickedOrClosed).toStrictEqual(timeNow); + }, + ); + + controller.setSurveyLinkLastClickedOrClosed(timeNow); + + expect(controller.state.surveyLinkLastClickedOrClosed).toStrictEqual( + timeNow, + ); + expect( + controllerMessenger.call('AppStateController:getState') + .surveyLinkLastClickedOrClosed, + ).toStrictEqual(timeNow); + }); }); }); }); + +type WithControllerOptions = Partial; + +type WithControllerCallback = ({ + controller, + controllerMessenger, +}: { + controller: AppStateController; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + controllerMessenger: any; +}) => ReturnValue; + +type WithControllerArgs = + | [WithControllerCallback] + | [WithControllerOptions, WithControllerCallback]; + +async function withController( + ...args: WithControllerArgs +): Promise { + const [options = {}, fn] = args.length === 2 ? args : [{}, args[0]]; + + const controllerMessenger = new ControllerMessenger< + | AppStateControllerActions + | AddApprovalRequest + | AcceptRequest + | PreferencesControllerGetStateAction, + | AppStateControllerEvents + | PreferencesControllerStateChangeEvent + | KeyringControllerQRKeyringStateChangeEvent + >(); + jest.spyOn(ControllerMessenger.prototype, 'call'); + jest.spyOn(ControllerMessenger.prototype, 'publish'); + const appStateMessenger = controllerMessenger.getRestricted({ + name: 'AppStateController', + allowedActions: [ + `ApprovalController:addRequest`, + `ApprovalController:acceptRequest`, + `PreferencesController:getState`, + ], + allowedEvents: [ + `PreferencesController:stateChange`, + `KeyringController:qrKeyringStateChange`, + ], + }); + controllerMessenger.registerActionHandler( + 'PreferencesController:getState', + jest.fn().mockReturnValue({ + preferences: { + autoLockTimeLimit: 0, + }, + }), + ); + controllerMessenger.registerActionHandler( + 'ApprovalController:addRequest', + jest.fn().mockReturnValue({ + catch: jest.fn(), + }), + ); + + return fn({ + controller: new AppStateController({ + addUnlockListener: jest.fn(), + isUnlocked: jest.fn(() => true), + onInactiveTimeout: jest.fn(), + messenger: appStateMessenger, + extension: extensionMock, + ...options, + }), + controllerMessenger, + }); +} diff --git a/app/scripts/controllers/app-state-controller.ts b/app/scripts/controllers/app-state-controller.ts index 605f307ec0e4..bef544a50da0 100644 --- a/app/scripts/controllers/app-state-controller.ts +++ b/app/scripts/controllers/app-state-controller.ts @@ -1,17 +1,19 @@ -import EventEmitter from 'events'; -import { ObservableStore } from '@metamask/obs-store'; import { v4 as uuid } from 'uuid'; import log from 'loglevel'; import { ApprovalType } from '@metamask/controller-utils'; import { KeyringControllerQRKeyringStateChangeEvent } from '@metamask/keyring-controller'; -import { RestrictedControllerMessenger } from '@metamask/base-controller'; +import { + BaseController, + ControllerGetStateAction, + ControllerStateChangeEvent, + RestrictedControllerMessenger, +} from '@metamask/base-controller'; import { AcceptRequest, AddApprovalRequest, } from '@metamask/approval-controller'; import { Json } from '@metamask/utils'; import { Browser } from 'webextension-polyfill'; -import { METAMASK_CONTROLLER_EVENTS } from '../metamask-controller'; import { MINUTE } from '../../../shared/constants/time'; import { AUTO_LOCK_TIMEOUT_ALARM } from '../../../shared/constants/alarms'; import { isManifestV3 } from '../../../shared/modules/mv3.utils'; @@ -82,10 +84,10 @@ const controllerName = 'AppStateController'; /** * Returns the state of the {@link AppStateController}. */ -export type AppStateControllerGetStateAction = { - type: 'AppStateController:getState'; - handler: () => AppStateControllerState; -}; +export type AppStateControllerGetStateAction = ControllerGetStateAction< + typeof controllerName, + AppStateControllerState +>; /** * Actions exposed by the {@link AppStateController}. @@ -95,7 +97,7 @@ export type AppStateControllerActions = AppStateControllerGetStateAction; /** * Actions that this controller is allowed to call. */ -export type AllowedActions = +type AllowedActions = | AddApprovalRequest | AcceptRequest | PreferencesControllerGetStateAction; @@ -103,20 +105,27 @@ export type AllowedActions = /** * Event emitted when the state of the {@link AppStateController} changes. */ -export type AppStateControllerStateChangeEvent = { - type: 'AppStateController:stateChange'; - payload: [AppStateControllerState, []]; +export type AppStateControllerStateChangeEvent = ControllerStateChangeEvent< + typeof controllerName, + AppStateControllerState +>; + +export type AppStateControllerUnlockChange = { + type: 'AppStateController:unlockChange'; + payload: []; }; /** * Events emitted by {@link AppStateController}. */ -export type AppStateControllerEvents = AppStateControllerStateChangeEvent; +export type AppStateControllerEvents = + | AppStateControllerStateChangeEvent + | AppStateControllerUnlockChange; /** * Events that this controller is allowed to subscribe. */ -export type AllowedEvents = +type AllowedEvents = | PreferencesControllerStateChangeEvent | KeyringControllerQRKeyringStateChangeEvent; @@ -147,18 +156,16 @@ type AppStateControllerInitState = Partial< > >; -type AppStateControllerOptions = { +export type AppStateControllerOptions = { addUnlockListener: (callback: () => void) => void; isUnlocked: () => boolean; - initState?: AppStateControllerInitState; + state?: AppStateControllerInitState; onInactiveTimeout?: () => void; messenger: AppStateControllerMessenger; extension: Browser; }; -const getDefaultAppStateControllerState = ( - initState?: AppStateControllerInitState, -): AppStateControllerState => ({ +const getDefaultAppStateControllerState = (): AppStateControllerState => ({ timeoutMinutes: DEFAULT_AUTO_LOCK_TIME_LIMIT, connectedStatusPopoverHasBeenShown: true, defaultHomeActiveTabName: null, @@ -181,7 +188,6 @@ const getDefaultAppStateControllerState = ( newPrivacyPolicyToastClickedOrClosed: null, newPrivacyPolicyToastShownDate: null, hadAdvancedGasFeesSetPriorToMigration92_3: false, - ...initState, qrHardware: {}, nftsDropdownState: {}, usedNetworks: { @@ -196,54 +202,221 @@ const getDefaultAppStateControllerState = ( currentExtensionPopupId: 0, }); -export class AppStateController extends EventEmitter { - private readonly extension: AppStateControllerOptions['extension']; +const controllerMetadata = { + timeoutMinutes: { + persist: true, + anonymous: true, + }, + connectedStatusPopoverHasBeenShown: { + persist: true, + anonymous: true, + }, + defaultHomeActiveTabName: { + persist: true, + anonymous: true, + }, + browserEnvironment: { + persist: true, + anonymous: true, + }, + popupGasPollTokens: { + persist: true, + anonymous: true, + }, + notificationGasPollTokens: { + persist: true, + anonymous: true, + }, + fullScreenGasPollTokens: { + persist: true, + anonymous: true, + }, + recoveryPhraseReminderHasBeenShown: { + persist: true, + anonymous: true, + }, + recoveryPhraseReminderLastShown: { + persist: true, + anonymous: true, + }, + outdatedBrowserWarningLastShown: { + persist: true, + anonymous: true, + }, + nftsDetectionNoticeDismissed: { + persist: true, + anonymous: true, + }, + showTestnetMessageInDropdown: { + persist: true, + anonymous: true, + }, + showBetaHeader: { + persist: true, + anonymous: true, + }, + showPermissionsTour: { + persist: true, + anonymous: true, + }, + showNetworkBanner: { + persist: true, + anonymous: true, + }, + showAccountBanner: { + persist: true, + anonymous: true, + }, + trezorModel: { + persist: true, + anonymous: true, + }, + currentPopupId: { + persist: false, + anonymous: true, + }, + onboardingDate: { + persist: false, + anonymous: true, + }, + lastViewedUserSurvey: { + persist: true, + anonymous: true, + }, + newPrivacyPolicyToastClickedOrClosed: { + persist: true, + anonymous: true, + }, + newPrivacyPolicyToastShownDate: { + persist: true, + anonymous: true, + }, + hadAdvancedGasFeesSetPriorToMigration92_3: { + persist: true, + anonymous: true, + }, + qrHardware: { + persist: true, + anonymous: true, + }, + nftsDropdownState: { + persist: true, + anonymous: true, + }, + usedNetworks: { + persist: true, + anonymous: true, + }, + surveyLinkLastClickedOrClosed: { + persist: true, + anonymous: true, + }, + signatureSecurityAlertResponses: { + persist: true, + anonymous: true, + }, + switchedNetworkDetails: { + persist: false, + anonymous: true, + }, + switchedNetworkNeverShowMessage: { + persist: false, + anonymous: true, + }, + currentExtensionPopupId: { + persist: false, + anonymous: true, + }, + lastInteractedConfirmationInfo: { + persist: true, + anonymous: true, + }, + termsOfUseLastAgreed: { + persist: true, + anonymous: true, + }, + snapsInstallPrivacyWarningShown: { + persist: true, + anonymous: true, + }, + interactiveReplacementToken: { + persist: true, + anonymous: true, + }, + noteToTraderMessage: { + persist: true, + anonymous: true, + }, + custodianDeepLink: { + persist: true, + anonymous: true, + }, +}; - private readonly onInactiveTimeout: () => void; +export class AppStateController extends BaseController< + typeof controllerName, + AppStateControllerState, + AppStateControllerMessenger +> { + readonly #extension: AppStateControllerOptions['extension']; - store: ObservableStore; + readonly #onInactiveTimeout: () => void; - private timer: NodeJS.Timeout | null; + #timer: NodeJS.Timeout | null; isUnlocked: () => boolean; - private readonly waitingForUnlock: { resolve: () => void }[]; - - private readonly messagingSystem: AppStateControllerMessenger; + readonly waitingForUnlock: { resolve: () => void }[]; #approvalRequestId: string | null; - constructor(opts: AppStateControllerOptions) { - const { - addUnlockListener, - isUnlocked, - initState, - onInactiveTimeout, + constructor({ + state = {}, + messenger, + addUnlockListener, + isUnlocked, + onInactiveTimeout, + extension, + }: AppStateControllerOptions) { + super({ + name: controllerName, + metadata: controllerMetadata, + state: { + ...getDefaultAppStateControllerState(), + ...state, + qrHardware: {}, + nftsDropdownState: {}, + usedNetworks: { + '0x1': true, + '0x5': true, + '0x539': true, + }, + surveyLinkLastClickedOrClosed: null, + signatureSecurityAlertResponses: {}, + switchedNetworkDetails: null, + switchedNetworkNeverShowMessage: false, + currentExtensionPopupId: 0, + }, messenger, - extension, - } = opts; - super(); - - this.extension = extension; - this.onInactiveTimeout = onInactiveTimeout || (() => undefined); - this.store = new ObservableStore( - getDefaultAppStateControllerState(initState), - ); - this.timer = null; + }); + + this.#extension = extension; + this.#onInactiveTimeout = onInactiveTimeout || (() => undefined); + this.#timer = null; this.isUnlocked = isUnlocked; this.waitingForUnlock = []; - addUnlockListener(this.handleUnlock.bind(this)); + addUnlockListener(this.#handleUnlock.bind(this)); messenger.subscribe( 'PreferencesController:stateChange', ({ preferences }: { preferences: Partial }) => { - const currentState = this.store.getState(); + const currentState = this.state; if ( typeof preferences?.autoLockTimeLimit === 'number' && currentState.timeoutMinutes !== preferences.autoLockTimeLimit ) { - this._setInactiveTimeout(preferences.autoLockTimeLimit); + this.#setInactiveTimeout(preferences.autoLockTimeLimit); } }, ); @@ -251,24 +424,17 @@ export class AppStateController extends EventEmitter { messenger.subscribe( 'KeyringController:qrKeyringStateChange', (qrHardware: Json) => - this.store.updateState({ - qrHardware, + this.update((currentState) => { + // @ts-expect-error this is caused by a bug in Immer, not being able to handle recursive types like Json + currentState.qrHardware = qrHardware; }), ); const { preferences } = messenger.call('PreferencesController:getState'); if (typeof preferences.autoLockTimeLimit === 'number') { - this._setInactiveTimeout(preferences.autoLockTimeLimit); + this.#setInactiveTimeout(preferences.autoLockTimeLimit); } - this.messagingSystem = messenger; - this.messagingSystem.registerActionHandler( - 'AppStateController:getState', - () => this.store.getState(), - ); - this.store.subscribe((state: AppStateControllerState) => { - this.messagingSystem.publish('AppStateController:stateChange', state, []); - }); this.#approvalRequestId = null; } @@ -286,7 +452,7 @@ export class AppStateController extends EventEmitter { if (this.isUnlocked()) { resolve(); } else { - this.waitForUnlock(resolve, shouldShowUnlockRequest); + this.#waitForUnlock(resolve, shouldShowUnlockRequest); } }); } @@ -300,26 +466,26 @@ export class AppStateController extends EventEmitter { * @param shouldShowUnlockRequest - Whether the extension notification * popup should be opened. */ - waitForUnlock(resolve: () => void, shouldShowUnlockRequest: boolean): void { + #waitForUnlock(resolve: () => void, shouldShowUnlockRequest: boolean): void { this.waitingForUnlock.push({ resolve }); - this.emit(METAMASK_CONTROLLER_EVENTS.UPDATE_BADGE); + this.messagingSystem.publish('AppStateController:unlockChange'); if (shouldShowUnlockRequest) { - this._requestApproval(); + this.#requestApproval(); } } /** * Drains the waitingForUnlock queue, resolving all the related Promises. */ - handleUnlock(): void { + #handleUnlock(): void { if (this.waitingForUnlock.length > 0) { while (this.waitingForUnlock.length > 0) { this.waitingForUnlock.shift()?.resolve(); } - this.emit(METAMASK_CONTROLLER_EVENTS.UPDATE_BADGE); + this.messagingSystem.publish('AppStateController:unlockChange'); } - this._acceptApproval(); + this.#acceptApproval(); } /** @@ -330,8 +496,8 @@ export class AppStateController extends EventEmitter { setDefaultHomeActiveTabName( defaultHomeActiveTabName: AccountOverviewTabKey | null, ): void { - this.store.updateState({ - defaultHomeActiveTabName, + this.update((state) => { + state.defaultHomeActiveTabName = defaultHomeActiveTabName; }); } @@ -339,8 +505,8 @@ export class AppStateController extends EventEmitter { * Record that the user has seen the connected status info popover */ setConnectedStatusPopoverHasBeenShown(): void { - this.store.updateState({ - connectedStatusPopoverHasBeenShown: true, + this.update((state) => { + state.connectedStatusPopoverHasBeenShown = true; }); } @@ -348,38 +514,38 @@ export class AppStateController extends EventEmitter { * Record that the user has been shown the recovery phrase reminder. */ setRecoveryPhraseReminderHasBeenShown(): void { - this.store.updateState({ - recoveryPhraseReminderHasBeenShown: true, + this.update((state) => { + state.recoveryPhraseReminderHasBeenShown = true; }); } setSurveyLinkLastClickedOrClosed(time: number): void { - this.store.updateState({ - surveyLinkLastClickedOrClosed: time, + this.update((state) => { + state.surveyLinkLastClickedOrClosed = time; }); } setOnboardingDate(): void { - this.store.updateState({ - onboardingDate: Date.now(), + this.update((state) => { + state.onboardingDate = Date.now(); }); } setLastViewedUserSurvey(id: number) { - this.store.updateState({ - lastViewedUserSurvey: id, + this.update((state) => { + state.lastViewedUserSurvey = id; }); } setNewPrivacyPolicyToastClickedOrClosed(): void { - this.store.updateState({ - newPrivacyPolicyToastClickedOrClosed: true, + this.update((state) => { + state.newPrivacyPolicyToastClickedOrClosed = true; }); } setNewPrivacyPolicyToastShownDate(time: number): void { - this.store.updateState({ - newPrivacyPolicyToastShownDate: time, + this.update((state) => { + state.newPrivacyPolicyToastShownDate = time; }); } @@ -389,8 +555,8 @@ export class AppStateController extends EventEmitter { * @param lastShown - timestamp when user was last shown the reminder. */ setRecoveryPhraseReminderLastShown(lastShown: number): void { - this.store.updateState({ - recoveryPhraseReminderLastShown: lastShown, + this.update((state) => { + state.recoveryPhraseReminderLastShown = lastShown; }); } @@ -400,8 +566,8 @@ export class AppStateController extends EventEmitter { * @param lastAgreed - timestamp when user last accepted the terms of use */ setTermsOfUseLastAgreed(lastAgreed: number): void { - this.store.updateState({ - termsOfUseLastAgreed: lastAgreed, + this.update((state) => { + state.termsOfUseLastAgreed = lastAgreed; }); } @@ -412,8 +578,8 @@ export class AppStateController extends EventEmitter { * @param shown - shown status */ setSnapsInstallPrivacyWarningShownStatus(shown: boolean): void { - this.store.updateState({ - snapsInstallPrivacyWarningShown: shown, + this.update((state) => { + state.snapsInstallPrivacyWarningShown = shown; }); } @@ -423,8 +589,8 @@ export class AppStateController extends EventEmitter { * @param lastShown - Timestamp (in milliseconds) of when the user was last shown the warning. */ setOutdatedBrowserWarningLastShown(lastShown: number): void { - this.store.updateState({ - outdatedBrowserWarningLastShown: lastShown, + this.update((state) => { + state.outdatedBrowserWarningLastShown = lastShown; }); } @@ -432,7 +598,7 @@ export class AppStateController extends EventEmitter { * Sets the last active time to the current time. */ setLastActiveTime(): void { - this._resetTimer(); + this.#resetTimer(); } /** @@ -440,12 +606,12 @@ export class AppStateController extends EventEmitter { * * @param timeoutMinutes - The inactive timeout in minutes. */ - private _setInactiveTimeout(timeoutMinutes: number): void { - this.store.updateState({ - timeoutMinutes, + #setInactiveTimeout(timeoutMinutes: number): void { + this.update((state) => { + state.timeoutMinutes = timeoutMinutes; }); - this._resetTimer(); + this.#resetTimer(); } /** @@ -455,13 +621,13 @@ export class AppStateController extends EventEmitter { * timer will not be created. * */ - private _resetTimer(): void { - const { timeoutMinutes } = this.store.getState(); + #resetTimer(): void { + const { timeoutMinutes } = this.state; - if (this.timer) { - clearTimeout(this.timer); + if (this.#timer) { + clearTimeout(this.#timer); } else if (isManifestV3) { - this.extension.alarms.clear(AUTO_LOCK_TIMEOUT_ALARM); + this.#extension.alarms.clear(AUTO_LOCK_TIMEOUT_ALARM); } if (!timeoutMinutes) { @@ -478,21 +644,21 @@ export class AppStateController extends EventEmitter { const timeoutToSet = Number(timeoutMinutes); if (isManifestV3) { - this.extension.alarms.create(AUTO_LOCK_TIMEOUT_ALARM, { + this.#extension.alarms.create(AUTO_LOCK_TIMEOUT_ALARM, { delayInMinutes: timeoutToSet, periodInMinutes: timeoutToSet, }); - this.extension.alarms.onAlarm.addListener( + this.#extension.alarms.onAlarm.addListener( (alarmInfo: { name: string }) => { if (alarmInfo.name === AUTO_LOCK_TIMEOUT_ALARM) { - this.onInactiveTimeout(); - this.extension.alarms.clear(AUTO_LOCK_TIMEOUT_ALARM); + this.#onInactiveTimeout(); + this.#extension.alarms.clear(AUTO_LOCK_TIMEOUT_ALARM); } }, ); } else { - this.timer = setTimeout( - () => this.onInactiveTimeout(), + this.#timer = setTimeout( + () => this.#onInactiveTimeout(), timeoutToSet * MINUTE, ); } @@ -505,7 +671,9 @@ export class AppStateController extends EventEmitter { * @param browser */ setBrowserEnvironment(os: string, browser: string): void { - this.store.updateState({ browserEnvironment: { os, browser } }); + this.update((state) => { + state.browserEnvironment = { os, browser }; + }); } /** @@ -538,9 +706,8 @@ export class AppStateController extends EventEmitter { pollingToken: string, pollingTokenType: PollingTokenType, ) { - const currentTokens: string[] = this.store.getState()[pollingTokenType]; - this.store.updateState({ - [pollingTokenType]: [...currentTokens, pollingToken], + this.update((state) => { + state[pollingTokenType].push(pollingToken); }); } @@ -558,12 +725,12 @@ export class AppStateController extends EventEmitter { pollingTokenType.toString() !== POLLING_TOKEN_ENVIRONMENT_TYPES[ENVIRONMENT_TYPE_BACKGROUND] ) { - const currentTokens: string[] = this.store.getState()[pollingTokenType]; + const currentTokens: string[] = this.state[pollingTokenType]; if (this.#isValidPollingTokenType(pollingTokenType)) { - this.store.updateState({ - [pollingTokenType]: currentTokens.filter( + this.update((state) => { + state[pollingTokenType] = currentTokens.filter( (token: string) => token !== pollingToken, - ), + ); }); } } @@ -589,10 +756,10 @@ export class AppStateController extends EventEmitter { * clears all pollingTokens */ clearPollingTokens(): void { - this.store.updateState({ - popupGasPollTokens: [], - notificationGasPollTokens: [], - fullScreenGasPollTokens: [], + this.update((state) => { + state.popupGasPollTokens = []; + state.notificationGasPollTokens = []; + state.fullScreenGasPollTokens = []; }); } @@ -602,7 +769,9 @@ export class AppStateController extends EventEmitter { * @param showTestnetMessageInDropdown */ setShowTestnetMessageInDropdown(showTestnetMessageInDropdown: boolean): void { - this.store.updateState({ showTestnetMessageInDropdown }); + this.update((state) => { + state.showTestnetMessageInDropdown = showTestnetMessageInDropdown; + }); } /** @@ -611,7 +780,9 @@ export class AppStateController extends EventEmitter { * @param showBetaHeader */ setShowBetaHeader(showBetaHeader: boolean): void { - this.store.updateState({ showBetaHeader }); + this.update((state) => { + state.showBetaHeader = showBetaHeader; + }); } /** @@ -620,7 +791,9 @@ export class AppStateController extends EventEmitter { * @param showPermissionsTour */ setShowPermissionsTour(showPermissionsTour: boolean): void { - this.store.updateState({ showPermissionsTour }); + this.update((state) => { + state.showPermissionsTour = showPermissionsTour; + }); } /** @@ -629,7 +802,9 @@ export class AppStateController extends EventEmitter { * @param showNetworkBanner */ setShowNetworkBanner(showNetworkBanner: boolean): void { - this.store.updateState({ showNetworkBanner }); + this.update((state) => { + state.showNetworkBanner = showNetworkBanner; + }); } /** @@ -638,7 +813,9 @@ export class AppStateController extends EventEmitter { * @param showAccountBanner */ setShowAccountBanner(showAccountBanner: boolean): void { - this.store.updateState({ showAccountBanner }); + this.update((state) => { + state.showAccountBanner = showAccountBanner; + }); } /** @@ -647,7 +824,9 @@ export class AppStateController extends EventEmitter { * @param currentExtensionPopupId */ setCurrentExtensionPopupId(currentExtensionPopupId: number): void { - this.store.updateState({ currentExtensionPopupId }); + this.update((state) => { + state.currentExtensionPopupId = currentExtensionPopupId; + }); } /** @@ -659,14 +838,18 @@ export class AppStateController extends EventEmitter { setSwitchedNetworkDetails( switchedNetworkDetails: { origin: string; networkClientId: string } | null, ): void { - this.store.updateState({ switchedNetworkDetails }); + this.update((state) => { + state.switchedNetworkDetails = switchedNetworkDetails; + }); } /** * Clears the switched network details in state */ clearSwitchedNetworkDetails(): void { - this.store.updateState({ switchedNetworkDetails: null }); + this.update((state) => { + state.switchedNetworkDetails = null; + }); } /** @@ -678,9 +861,9 @@ export class AppStateController extends EventEmitter { setSwitchedNetworkNeverShowMessage( switchedNetworkNeverShowMessage: boolean, ): void { - this.store.updateState({ - switchedNetworkDetails: null, - switchedNetworkNeverShowMessage, + this.update((state) => { + state.switchedNetworkDetails = null; + state.switchedNetworkNeverShowMessage = switchedNetworkNeverShowMessage; }); } @@ -690,7 +873,9 @@ export class AppStateController extends EventEmitter { * @param trezorModel - The Trezor model. */ setTrezorModel(trezorModel: string | null): void { - this.store.updateState({ trezorModel }); + this.update((state) => { + state.trezorModel = trezorModel; + }); } /** @@ -699,8 +884,8 @@ export class AppStateController extends EventEmitter { * @param nftsDropdownState */ updateNftDropDownState(nftsDropdownState: Json): void { - this.store.updateState({ - nftsDropdownState, + this.update((state) => { + state.nftsDropdownState = nftsDropdownState; }); } @@ -710,11 +895,9 @@ export class AppStateController extends EventEmitter { * @param chainId */ setFirstTimeUsedNetwork(chainId: string): void { - const currentState = this.store.getState(); - const { usedNetworks } = currentState; - usedNetworks[chainId] = true; - - this.store.updateState({ usedNetworks }); + this.update((state) => { + state.usedNetworks[chainId] = true; + }); } ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) @@ -732,11 +915,11 @@ export class AppStateController extends EventEmitter { url: string; oldRefreshToken: string; }): void { - this.store.updateState({ - interactiveReplacementToken: { + this.update((state) => { + state.interactiveReplacementToken = { url, oldRefreshToken, - }, + }; }); } @@ -754,14 +937,14 @@ export class AppStateController extends EventEmitter { fromAddress: string; custodyId: string; }): void { - this.store.updateState({ - custodianDeepLink: { fromAddress, custodyId }, + this.update((state) => { + state.custodianDeepLink = { fromAddress, custodyId }; }); } setNoteToTraderMessage(message: string): void { - this.store.updateState({ - noteToTraderMessage: message, + this.update((state) => { + state.noteToTraderMessage = message; }); } @@ -770,23 +953,17 @@ export class AppStateController extends EventEmitter { getSignatureSecurityAlertResponse( securityAlertId: string, ): SecurityAlertResponse { - return this.store.getState().signatureSecurityAlertResponses[ - securityAlertId - ]; + return this.state.signatureSecurityAlertResponses[securityAlertId]; } addSignatureSecurityAlertResponse( securityAlertResponse: SecurityAlertResponse, ): void { - const currentState = this.store.getState(); - const { signatureSecurityAlertResponses } = currentState; if (securityAlertResponse.securityAlertId) { - this.store.updateState({ - signatureSecurityAlertResponses: { - ...signatureSecurityAlertResponses, - [String(securityAlertResponse.securityAlertId)]: - securityAlertResponse, - }, + this.update((state) => { + state.signatureSecurityAlertResponses[ + String(securityAlertResponse.securityAlertId) + ] = securityAlertResponse; }); } } @@ -797,8 +974,8 @@ export class AppStateController extends EventEmitter { * @param currentPopupId */ setCurrentPopupId(currentPopupId: number): void { - this.store.updateState({ - currentPopupId, + this.update((state) => { + state.currentPopupId = currentPopupId; }); } @@ -808,7 +985,7 @@ export class AppStateController extends EventEmitter { getLastInteractedConfirmationInfo(): | LastInteractedConfirmationInfo | undefined { - return this.store.getState().lastInteractedConfirmationInfo; + return this.state.lastInteractedConfirmationInfo; } /** @@ -819,8 +996,8 @@ export class AppStateController extends EventEmitter { setLastInteractedConfirmationInfo( lastInteractedConfirmationInfo: LastInteractedConfirmationInfo | undefined, ): void { - this.store.updateState({ - lastInteractedConfirmationInfo, + this.update((state) => { + state.lastInteractedConfirmationInfo = lastInteractedConfirmationInfo; }); } @@ -828,10 +1005,10 @@ export class AppStateController extends EventEmitter { * A getter to retrieve currentPopupId saved in the appState */ getCurrentPopupId(): number | undefined { - return this.store.getState().currentPopupId; + return this.state.currentPopupId; } - private _requestApproval(): void { + #requestApproval(): void { // If we already have a pending request this is a no-op if (this.#approvalRequestId) { return; @@ -854,12 +1031,7 @@ export class AppStateController extends EventEmitter { }); } - // Override emit method to provide strong typing for events - emit(event: string) { - return super.emit(event); - } - - private _acceptApproval(): void { + #acceptApproval(): void { if (!this.#approvalRequestId) { return; } diff --git a/app/scripts/controllers/metametrics-controller.ts b/app/scripts/controllers/metametrics-controller.ts index b2b78a4e6406..e286a3a47d02 100644 --- a/app/scripts/controllers/metametrics-controller.ts +++ b/app/scripts/controllers/metametrics-controller.ts @@ -584,7 +584,6 @@ export default class MetaMetricsController extends BaseController< : {}; this.update((state) => { - // @ts-expect-error this is caused by a bug in Immer, not being able to handle recursive types like Json state.fragments[id] = merge(additionalFragmentProps, fragment); }); diff --git a/app/scripts/controllers/mmi-controller.test.ts b/app/scripts/controllers/mmi-controller.test.ts index 71c8329b3dcd..8e52655ee11e 100644 --- a/app/scripts/controllers/mmi-controller.test.ts +++ b/app/scripts/controllers/mmi-controller.test.ts @@ -273,7 +273,7 @@ describe('MMIController', function () { appStateController: new AppStateController({ addUnlockListener: jest.fn(), isUnlocked: jest.fn(() => true), - initState: {}, + state: {}, onInactiveTimeout: jest.fn(), showUnlockRequest: jest.fn(), messenger: { diff --git a/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js b/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js index 1949ca7f876e..98bb790cfa32 100644 --- a/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js +++ b/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js @@ -36,20 +36,16 @@ const expectedMetametricsEventUndefinedProps = { }; const appStateController = { - store: { - getState: () => ({ - signatureSecurityAlertResponses: { - 1: { - result_type: BlockaidResultType.Malicious, - reason: BlockaidReason.maliciousDomain, - }, + state: { + signatureSecurityAlertResponses: { + 1: { + result_type: BlockaidResultType.Malicious, + reason: BlockaidReason.maliciousDomain, }, - }), + }, }, getSignatureSecurityAlertResponse: (id) => { - return appStateController.store.getState().signatureSecurityAlertResponses[ - id - ]; + return appStateController.state.signatureSecurityAlertResponses[id]; }, }; diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 992302983baf..5b8ca370e66f 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -383,6 +383,7 @@ export const METAMASK_CONTROLLER_EVENTS = { UPDATE_BADGE: 'updateBadge', // TODO: Add this and similar enums to the `controllers` repo and export them APPROVAL_STATE_CHANGE: 'ApprovalController:stateChange', + APP_STATE_UNLOCK_CHANGE: 'AppStateController:unlockChange', QUEUED_REQUEST_STATE_CHANGE: 'QueuedRequestController:stateChange', METAMASK_NOTIFICATIONS_LIST_UPDATED: 'NotificationServicesController:notificationsListUpdated', @@ -861,7 +862,7 @@ export default class MetamaskController extends EventEmitter { this.appStateController = new AppStateController({ addUnlockListener: this.on.bind(this, 'unlock'), isUnlocked: this.isUnlocked.bind(this), - initState: initState.AppStateController, + state: initState.AppStateController, onInactiveTimeout: () => this.setLocked(), messenger: this.controllerMessenger.getRestricted({ name: 'AppStateController', @@ -2469,7 +2470,7 @@ export default class MetamaskController extends EventEmitter { this.store.updateStructure({ AccountsController: this.accountsController, - AppStateController: this.appStateController.store, + AppStateController: this.appStateController, AppMetadataController: this.appMetadataController, MultichainBalancesController: this.multichainBalancesController, TransactionController: this.txController, @@ -2524,7 +2525,7 @@ export default class MetamaskController extends EventEmitter { this.memStore = new ComposableObservableStore({ config: { AccountsController: this.accountsController, - AppStateController: this.appStateController.store, + AppStateController: this.appStateController, AppMetadataController: this.appMetadataController, MultichainBalancesController: this.multichainBalancesController, NetworkController: this.networkController, @@ -6847,7 +6848,7 @@ export default class MetamaskController extends EventEmitter { const appStatePollingTokenType = POLLING_TOKEN_ENVIRONMENT_TYPES[environmentType]; const pollingTokensToDisconnect = - this.appStateController.store.getState()[appStatePollingTokenType]; + this.appStateController.state[appStatePollingTokenType]; pollingTokensToDisconnect.forEach((pollingToken) => { this.gasFeeController.stopPollingByPollingToken(pollingToken); this.currencyRateController.stopPollingByPollingToken(pollingToken); From 6edc9bc8e3b3714b38de06c275e696c8257a202f Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Fri, 29 Nov 2024 01:52:48 +0100 Subject: [PATCH 2/8] fix: update state in optional to be partial --- app/scripts/controllers/app-state-controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/controllers/app-state-controller.ts b/app/scripts/controllers/app-state-controller.ts index bef544a50da0..586d13b7e28b 100644 --- a/app/scripts/controllers/app-state-controller.ts +++ b/app/scripts/controllers/app-state-controller.ts @@ -159,7 +159,7 @@ type AppStateControllerInitState = Partial< export type AppStateControllerOptions = { addUnlockListener: (callback: () => void) => void; isUnlocked: () => boolean; - state?: AppStateControllerInitState; + state?: Partial; onInactiveTimeout?: () => void; messenger: AppStateControllerMessenger; extension: Browser; From 410999b53d2258ee6eff62c867020e36773914aa Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Fri, 29 Nov 2024 01:57:55 +0100 Subject: [PATCH 3/8] fix: do not persist overwritten state properties --- app/scripts/controllers/app-state-controller.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/scripts/controllers/app-state-controller.ts b/app/scripts/controllers/app-state-controller.ts index 586d13b7e28b..a78cfd9308c5 100644 --- a/app/scripts/controllers/app-state-controller.ts +++ b/app/scripts/controllers/app-state-controller.ts @@ -296,23 +296,23 @@ const controllerMetadata = { anonymous: true, }, qrHardware: { - persist: true, + persist: false, anonymous: true, }, nftsDropdownState: { - persist: true, + persist: false, anonymous: true, }, usedNetworks: { - persist: true, + persist: false, anonymous: true, }, surveyLinkLastClickedOrClosed: { - persist: true, + persist: false, anonymous: true, }, signatureSecurityAlertResponses: { - persist: true, + persist: false, anonymous: true, }, switchedNetworkDetails: { From 6f82d5850151f70ee20e524f574b03c61cbf19f0 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Fri, 29 Nov 2024 02:26:28 +0100 Subject: [PATCH 4/8] fix: rollback state option as already partial --- app/scripts/controllers/app-state-controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/controllers/app-state-controller.ts b/app/scripts/controllers/app-state-controller.ts index a78cfd9308c5..37b6b2fce314 100644 --- a/app/scripts/controllers/app-state-controller.ts +++ b/app/scripts/controllers/app-state-controller.ts @@ -159,7 +159,7 @@ type AppStateControllerInitState = Partial< export type AppStateControllerOptions = { addUnlockListener: (callback: () => void) => void; isUnlocked: () => boolean; - state?: Partial; + state?: AppStateControllerInitState; onInactiveTimeout?: () => void; messenger: AppStateControllerMessenger; extension: Browser; From a298ef3bd8e46f34605892a0b9342704eee1cc50 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Fri, 29 Nov 2024 02:35:02 +0100 Subject: [PATCH 5/8] fix: update controller metadata --- .../controllers/app-state-controller.ts | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/app/scripts/controllers/app-state-controller.ts b/app/scripts/controllers/app-state-controller.ts index 37b6b2fce314..d7283c74f685 100644 --- a/app/scripts/controllers/app-state-controller.ts +++ b/app/scripts/controllers/app-state-controller.ts @@ -220,15 +220,15 @@ const controllerMetadata = { anonymous: true, }, popupGasPollTokens: { - persist: true, + persist: false, anonymous: true, }, notificationGasPollTokens: { - persist: true, + persist: false, anonymous: true, }, fullScreenGasPollTokens: { - persist: true, + persist: false, anonymous: true, }, recoveryPhraseReminderHasBeenShown: { @@ -276,7 +276,7 @@ const controllerMetadata = { anonymous: true, }, onboardingDate: { - persist: false, + persist: true, anonymous: true, }, lastViewedUserSurvey: { @@ -304,11 +304,11 @@ const controllerMetadata = { anonymous: true, }, usedNetworks: { - persist: false, + persist: true, anonymous: true, }, surveyLinkLastClickedOrClosed: { - persist: false, + persist: true, anonymous: true, }, signatureSecurityAlertResponses: { @@ -320,7 +320,7 @@ const controllerMetadata = { anonymous: true, }, switchedNetworkNeverShowMessage: { - persist: false, + persist: true, anonymous: true, }, currentExtensionPopupId: { @@ -386,15 +386,8 @@ export class AppStateController extends BaseController< ...state, qrHardware: {}, nftsDropdownState: {}, - usedNetworks: { - '0x1': true, - '0x5': true, - '0x539': true, - }, - surveyLinkLastClickedOrClosed: null, signatureSecurityAlertResponses: {}, switchedNetworkDetails: null, - switchedNetworkNeverShowMessage: false, currentExtensionPopupId: 0, }, messenger, From 888832625dee6ad7b589146612f2807e9ddef925 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Fri, 29 Nov 2024 03:42:10 +0100 Subject: [PATCH 6/8] feat: add utility for overridden state properties --- .../controllers/app-state-controller.ts | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/app/scripts/controllers/app-state-controller.ts b/app/scripts/controllers/app-state-controller.ts index d7283c74f685..93276c39bc8c 100644 --- a/app/scripts/controllers/app-state-controller.ts +++ b/app/scripts/controllers/app-state-controller.ts @@ -188,20 +188,26 @@ const getDefaultAppStateControllerState = (): AppStateControllerState => ({ newPrivacyPolicyToastClickedOrClosed: null, newPrivacyPolicyToastShownDate: null, hadAdvancedGasFeesSetPriorToMigration92_3: false, - qrHardware: {}, - nftsDropdownState: {}, usedNetworks: { '0x1': true, '0x5': true, '0x539': true, }, surveyLinkLastClickedOrClosed: null, - signatureSecurityAlertResponses: {}, - switchedNetworkDetails: null, switchedNetworkNeverShowMessage: false, - currentExtensionPopupId: 0, + ...getDefaultOverrides(), }); +function getDefaultOverrides() { + return { + qrHardware: {}, + nftsDropdownState: {}, + signatureSecurityAlertResponses: {}, + switchedNetworkDetails: null, + currentExtensionPopupId: 0, + }; +} + const controllerMetadata = { timeoutMinutes: { persist: true, @@ -384,11 +390,7 @@ export class AppStateController extends BaseController< state: { ...getDefaultAppStateControllerState(), ...state, - qrHardware: {}, - nftsDropdownState: {}, - signatureSecurityAlertResponses: {}, - switchedNetworkDetails: null, - currentExtensionPopupId: 0, + ...getDefaultOverrides(), }, messenger, }); From c5df2de54d0ace8aa8abd125686fb57fe0342152 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Tue, 3 Dec 2024 01:49:31 +0100 Subject: [PATCH 7/8] fix: address PR review --- .../controllers/app-state-controller.test.ts | 93 ++----------------- .../controllers/app-state-controller.ts | 4 +- 2 files changed, 12 insertions(+), 85 deletions(-) diff --git a/app/scripts/controllers/app-state-controller.test.ts b/app/scripts/controllers/app-state-controller.test.ts index 87e67b648d58..de531fe6a74b 100644 --- a/app/scripts/controllers/app-state-controller.test.ts +++ b/app/scripts/controllers/app-state-controller.test.ts @@ -17,7 +17,6 @@ import type { AppStateControllerActions, AppStateControllerEvents, AppStateControllerOptions, - AppStateControllerState, } from './app-state-controller'; import type { PreferencesControllerState, @@ -542,83 +541,6 @@ describe('AppStateController', () => { }); }); }); - - describe('AppStateController:getState', () => { - it('should return the current state of the property', async () => { - await withController(({ controller, controllerMessenger }) => { - expect( - controller.state.recoveryPhraseReminderHasBeenShown, - ).toStrictEqual(false); - - expect( - controllerMessenger.call('AppStateController:getState') - .recoveryPhraseReminderHasBeenShown, - ).toStrictEqual(false); - }); - }); - }); - - describe('AppStateController:stateChange', () => { - it('subscribers will recieve the state when published', async () => { - await withController(({ controller, controllerMessenger }) => { - expect(controller.state.surveyLinkLastClickedOrClosed).toStrictEqual( - null, - ); - const timeNow = Date.now(); - controllerMessenger.subscribe( - 'AppStateController:stateChange', - (state: Partial) => { - if (typeof state.surveyLinkLastClickedOrClosed === 'number') { - controller.setSurveyLinkLastClickedOrClosed( - state.surveyLinkLastClickedOrClosed, - ); - } - }, - ); - - controllerMessenger.publish( - 'AppStateController:stateChange', - { - surveyLinkLastClickedOrClosed: timeNow, - } as unknown as AppStateControllerState, - [], - ); - - expect(controller.state.surveyLinkLastClickedOrClosed).toStrictEqual( - timeNow, - ); - expect( - controllerMessenger.call('AppStateController:getState') - .surveyLinkLastClickedOrClosed, - ).toStrictEqual(timeNow); - }); - }); - - it('state will be published when there is state change', async () => { - await withController(({ controller, controllerMessenger }) => { - expect(controller.state.surveyLinkLastClickedOrClosed).toStrictEqual( - null, - ); - const timeNow = Date.now(); - controllerMessenger.subscribe( - 'AppStateController:stateChange', - (state: Partial) => { - expect(state.surveyLinkLastClickedOrClosed).toStrictEqual(timeNow); - }, - ); - - controller.setSurveyLinkLastClickedOrClosed(timeNow); - - expect(controller.state.surveyLinkLastClickedOrClosed).toStrictEqual( - timeNow, - ); - expect( - controllerMessenger.call('AppStateController:getState') - .surveyLinkLastClickedOrClosed, - ).toStrictEqual(timeNow); - }); - }); - }); }); type WithControllerOptions = Partial; @@ -628,8 +550,15 @@ type WithControllerCallback = ({ controllerMessenger, }: { controller: AppStateController; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - controllerMessenger: any; + controllerMessenger: ControllerMessenger< + | AppStateControllerActions + | AddApprovalRequest + | AcceptRequest + | PreferencesControllerGetStateAction, + | AppStateControllerEvents + | PreferencesControllerStateChangeEvent + | KeyringControllerQRKeyringStateChangeEvent + >; }) => ReturnValue; type WithControllerArgs = @@ -674,9 +603,7 @@ async function withController( ); controllerMessenger.registerActionHandler( 'ApprovalController:addRequest', - jest.fn().mockReturnValue({ - catch: jest.fn(), - }), + jest.fn().mockResolvedValue(undefined), ); return fn({ diff --git a/app/scripts/controllers/app-state-controller.ts b/app/scripts/controllers/app-state-controller.ts index ae7edcdc89e0..17261045ad55 100644 --- a/app/scripts/controllers/app-state-controller.ts +++ b/app/scripts/controllers/app-state-controller.ts @@ -109,7 +109,7 @@ export type AppStateControllerStateChangeEvent = ControllerStateChangeEvent< AppStateControllerState >; -export type AppStateControllerUnlockChange = { +export type AppStateControllerUnlockChangeEvent = { type: 'AppStateController:unlockChange'; payload: []; }; @@ -119,7 +119,7 @@ export type AppStateControllerUnlockChange = { */ export type AppStateControllerEvents = | AppStateControllerStateChangeEvent - | AppStateControllerUnlockChange; + | AppStateControllerUnlockChangeEvent; /** * Events that this controller is allowed to subscribe. From 3a345be828a25c4e5d4289396c72ae067bb6087b Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 4 Dec 2024 00:05:30 +0100 Subject: [PATCH 8/8] fix: remove ControllerMessenger mock --- .../controllers/app-state-controller.test.ts | 68 +++++++++++-------- .../controllers/app-state-controller.ts | 6 +- 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/app/scripts/controllers/app-state-controller.test.ts b/app/scripts/controllers/app-state-controller.test.ts index de531fe6a74b..727df9b853e1 100644 --- a/app/scripts/controllers/app-state-controller.test.ts +++ b/app/scripts/controllers/app-state-controller.test.ts @@ -99,10 +99,14 @@ describe('AppStateController', () => { }); }); - it("doesn't resolves immediately if unlocked is false", async () => { + it('publishes an unlock change event when isUnlocked is set to false', async () => { await withController(async ({ controller, controllerMessenger }) => { jest.spyOn(controller, 'isUnlocked').mockReturnValue(false); - + const unlockChangeSpy = jest.fn(); + controllerMessenger.subscribe( + 'AppStateController:unlockChange', + unlockChangeSpy, + ); const unlockPromise = controller.getUnlockPromise(false); const timeoutPromise = new Promise((resolve) => @@ -113,27 +117,24 @@ describe('AppStateController', () => { expect(result).toBe('timeout'); - expect(controllerMessenger.publish).toHaveBeenCalledWith( - 'AppStateController:unlockChange', - ); - expect(controllerMessenger.call).toHaveBeenCalledTimes(1); + expect(unlockChangeSpy).toHaveBeenCalled(); }); }); it('creates approval request when waitForUnlock is called with shouldShowUnlockRequest as true', async () => { - await withController(async ({ controller, controllerMessenger }) => { + const addRequestMock = jest.fn().mockResolvedValue(undefined); + await withController({ addRequestMock }, async ({ controller }) => { jest.spyOn(controller, 'isUnlocked').mockReturnValue(false); controller.getUnlockPromise(true); - expect(controllerMessenger.call).toHaveBeenCalledTimes(2); - expect(controllerMessenger.call).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - expect.objectContaining({ + expect(addRequestMock).toHaveBeenCalled(); + expect(addRequestMock).toHaveBeenCalledWith( + { id: expect.any(String), origin: ORIGIN_METAMASK, type: 'unlock', - }), + }, true, ); }); @@ -141,26 +142,37 @@ describe('AppStateController', () => { it('accepts approval request revolving all the related promises', async () => { let unlockListener: () => void; + const addRequestMock = jest.fn().mockResolvedValue(undefined); await withController( { - addUnlockListener: (listener) => { - unlockListener = listener; + addRequestMock, + options: { + addUnlockListener: (listener) => { + unlockListener = listener; + }, }, }, ({ controller, controllerMessenger }) => { jest.spyOn(controller, 'isUnlocked').mockReturnValue(false); + const unlockChangeSpy = jest.fn(); + controllerMessenger.subscribe( + 'AppStateController:unlockChange', + unlockChangeSpy, + ); + controller.getUnlockPromise(true); unlockListener(); - expect(controllerMessenger.publish).toHaveBeenCalled(); - expect(controllerMessenger.publish).toHaveBeenCalledWith( - 'AppStateController:unlockChange', - ); - expect(controllerMessenger.call).toHaveBeenCalled(); - expect(controllerMessenger.call).toHaveBeenCalledWith( - 'ApprovalController:acceptRequest', - expect.any(String), + expect(unlockChangeSpy).toHaveBeenCalled(); + expect(addRequestMock).toHaveBeenCalled(); + expect(addRequestMock).toHaveBeenCalledWith( + { + id: expect.any(String), + origin: ORIGIN_METAMASK, + type: 'unlock', + }, + true, ); }, ); @@ -543,7 +555,10 @@ describe('AppStateController', () => { }); }); -type WithControllerOptions = Partial; +type WithControllerOptions = { + options?: Partial; + addRequestMock?: jest.Mock; +}; type WithControllerCallback = ({ controller, @@ -568,7 +583,8 @@ type WithControllerArgs = async function withController( ...args: WithControllerArgs ): Promise { - const [options = {}, fn] = args.length === 2 ? args : [{}, args[0]]; + const [{ ...rest }, fn] = args.length === 2 ? args : [{}, args[0]]; + const { addRequestMock, options = {} } = rest; const controllerMessenger = new ControllerMessenger< | AppStateControllerActions @@ -579,8 +595,6 @@ async function withController( | PreferencesControllerStateChangeEvent | KeyringControllerQRKeyringStateChangeEvent >(); - jest.spyOn(ControllerMessenger.prototype, 'call'); - jest.spyOn(ControllerMessenger.prototype, 'publish'); const appStateMessenger = controllerMessenger.getRestricted({ name: 'AppStateController', allowedActions: [ @@ -603,7 +617,7 @@ async function withController( ); controllerMessenger.registerActionHandler( 'ApprovalController:addRequest', - jest.fn().mockResolvedValue(undefined), + addRequestMock || jest.fn().mockResolvedValue(undefined), ); return fn({ diff --git a/app/scripts/controllers/app-state-controller.ts b/app/scripts/controllers/app-state-controller.ts index 17261045ad55..612f328eb82d 100644 --- a/app/scripts/controllers/app-state-controller.ts +++ b/app/scripts/controllers/app-state-controller.ts @@ -186,10 +186,10 @@ const getDefaultAppStateControllerState = (): AppStateControllerState => ({ hadAdvancedGasFeesSetPriorToMigration92_3: false, surveyLinkLastClickedOrClosed: null, switchedNetworkNeverShowMessage: false, - ...getDefaultAppStateOverrides(), + ...getInitialStateOverrides(), }); -function getDefaultAppStateOverrides() { +function getInitialStateOverrides() { return { qrHardware: {}, nftsDropdownState: {}, @@ -377,7 +377,7 @@ export class AppStateController extends BaseController< state: { ...getDefaultAppStateControllerState(), ...state, - ...getDefaultAppStateOverrides(), + ...getInitialStateOverrides(), }, messenger, });