From 205bfafc9c3197462438c5531f28e3ef3c1eb2da Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 27 Jan 2025 11:45:45 -0500 Subject: [PATCH] fix: Type fixes and optimizations for `EngineService`, Redux store (#12509) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** - ~Define types: `ReduxStore`, `ReduxState`~ (superseded by https://github.com/MetaMask/metamask-mobile/pull/12538) - Fix `any` types in `EngineService`. - Optimize `EngineService` event subscriptions: - Fix `update_bg_state_cb` callback being re-defined on every iteration. - Consolidate `stateChange` events collections into `BACKGROUND_STATE_CHANGE_EVENT_NAMES` constant for maintainability. ## **Related issues** - Blocked by https://github.com/MetaMask/metamask-mobile/pull/10441 - Blocked by https://github.com/MetaMask/core/pull/4968 and corresponding release. ## **Manual testing steps** ## **Screenshots/Recordings** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- app/core/EngineService/EngineService.ts | 163 ++++-------------------- app/store/index.ts | 20 ++- 2 files changed, 33 insertions(+), 150 deletions(-) diff --git a/app/core/EngineService/EngineService.ts b/app/core/EngineService/EngineService.ts index 6285908493f6..cae00ab9c54b 100644 --- a/app/core/EngineService/EngineService.ts +++ b/app/core/EngineService/EngineService.ts @@ -1,5 +1,5 @@ -import Engine from '../Engine'; -import AppConstants from '../AppConstants'; +import UntypedEngine from '../Engine'; +import { Engine as TypedEngine } from '../Engine/Engine'; import { getVaultFromBackup } from '../BackupVault'; import Logger from '../../util/Logger'; import { @@ -9,6 +9,7 @@ import { import { getTraceTags } from '../../util/sentry/tags'; import { trace, endTrace, TraceName, TraceOperation } from '../../util/trace'; import getUIStartupSpan from '../Performance/UIStartup'; +import { BACKGROUND_STATE_CHANGE_EVENT_NAMES } from '../Engine/constants'; import ReduxService from '../redux'; import NavigationService from '../NavigationService'; import Routes from '../../constants/navigation/Routes'; @@ -52,7 +53,8 @@ export class EngineService { parentContext: getUIStartupSpan(), tags: getTraceTags(reduxState), }); - const state = reduxState?.engine?.backgroundState || {}; + const state = reduxState?.engine?.backgroundState ?? {}; + const Engine = UntypedEngine; try { Logger.log(`${LOG_TAG}: Initializing Engine:`, { hasState: Object.keys(state).length > 0, @@ -60,7 +62,8 @@ export class EngineService { const metaMetricsId = await MetaMetrics.getInstance().getMetaMetricsId(); Engine.init(state, null, metaMetricsId); - this.updateControllers(Engine); + // `Engine.init()` call mutates `typeof UntypedEngine` to `TypedEngine` + this.updateControllers(Engine as unknown as TypedEngine); } catch (error) { Logger.error( error as Error, @@ -74,9 +77,7 @@ export class EngineService { endTrace({ name: TraceName.EngineInitialization }); }; - // TODO: Replace "any" with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - private updateControllers = (engine: any) => { + private updateControllers = (engine: TypedEngine) => { if (!engine.context) { Logger.error( new Error( @@ -86,125 +87,6 @@ export class EngineService { return; } - const controllers = [ - { - name: 'AddressBookController', - key: `${engine.context.AddressBookController.name}:stateChange`, - }, - { name: 'NftController', key: 'NftController:stateChange' }, - { - name: 'TokensController', - key: `${engine.context.TokensController.name}:stateChange`, - }, - { - name: 'KeyringController', - key: `${engine.context.KeyringController.name}:stateChange`, - }, - { - name: 'AccountTrackerController', - key: 'AccountTrackerController:stateChange', - }, - { - name: 'NetworkController', - key: AppConstants.NETWORK_STATE_CHANGE_EVENT, - }, - { - name: 'PhishingController', - key: `${engine.context.PhishingController.name}:stateChange`, - }, - { - name: 'PreferencesController', - key: `${engine.context.PreferencesController.name}:stateChange`, - }, - { - name: 'RemoteFeatureFlagController', - key: `${engine.context.RemoteFeatureFlagController.name}:stateChange`, - }, - { - name: 'SelectedNetworkController', - key: `${engine.context.SelectedNetworkController.name}:stateChange`, - }, - { - name: 'TokenBalancesController', - key: `${engine.context.TokenBalancesController.name}:stateChange`, - }, - { name: 'TokenRatesController', key: 'TokenRatesController:stateChange' }, - { - name: 'TransactionController', - key: `${engine.context.TransactionController.name}:stateChange`, - }, - { - name: 'SmartTransactionsController', - key: `${engine.context.SmartTransactionsController.name}:stateChange`, - }, - { - name: 'SwapsController', - key: `${engine.context.SwapsController.name}:stateChange`, - }, - { - name: 'TokenListController', - key: `${engine.context.TokenListController.name}:stateChange`, - }, - { - name: 'CurrencyRateController', - key: `${engine.context.CurrencyRateController.name}:stateChange`, - }, - { - name: 'GasFeeController', - key: `${engine.context.GasFeeController.name}:stateChange`, - }, - { - name: 'ApprovalController', - key: `${engine.context.ApprovalController.name}:stateChange`, - }, - ///: BEGIN:ONLY_INCLUDE_IF(preinstalled-snaps,external-snaps) - { - name: 'SnapController', - key: `${engine.context.SnapController.name}:stateChange`, - }, - { - name: 'SubjectMetadataController', - key: `${engine.context.SubjectMetadataController.name}:stateChange`, - }, - { - name: 'AuthenticationController', - key: 'AuthenticationController:stateChange', - }, - { - name: 'UserStorageController', - key: 'UserStorageController:stateChange', - }, - { - name: 'NotificationServicesController', - key: 'NotificationServicesController:stateChange', - }, - { - name: 'NotificationServicesPushController', - key: 'NotificationServicesPushController:stateChange', - }, - ///: END:ONLY_INCLUDE_IF - { - name: 'PermissionController', - key: `${engine.context.PermissionController.name}:stateChange`, - }, - { - name: 'LoggingController', - key: `${engine.context.LoggingController.name}:stateChange`, - }, - { - name: 'AccountsController', - key: `${engine.context.AccountsController.name}:stateChange`, - }, - { - name: 'PPOMController', - key: `${engine.context.PPOMController.name}:stateChange`, - }, - { - name: 'SignatureController', - key: `${engine.context.SignatureController.name}:stateChange`, - }, - ]; - engine.controllerMessenger.subscribeOnceIf( 'ComposableController:stateChange', () => { @@ -217,18 +99,20 @@ export class EngineService { () => !this.engineInitialized, ); - controllers.forEach((controller) => { - const { name, key } = controller; - const update_bg_state_cb = () => { - if (!engine.context.KeyringController.metadata.vault) { - Logger.log('keyringController vault missing for UPDATE_BG_STATE_KEY'); - } - ReduxService.store.dispatch({ - type: UPDATE_BG_STATE_KEY, - payload: { key: name }, - }); - }; - engine.controllerMessenger.subscribe(key, update_bg_state_cb); + const update_bg_state_cb = (controllerName: string) => { + if (!engine.context.KeyringController.metadata.vault) { + Logger.log('keyringController vault missing for UPDATE_BG_STATE_KEY'); + } + ReduxService.store.dispatch({ + type: UPDATE_BG_STATE_KEY, + payload: { key: controllerName }, + }); + }; + + BACKGROUND_STATE_CHANGE_EVENT_NAMES.forEach((eventName) => { + engine.controllerMessenger.subscribe(eventName, () => + update_bg_state_cb(eventName.split(':')[0]), + ); }); }; @@ -244,7 +128,8 @@ export class EngineService { async initializeVaultFromBackup(): Promise { const keyringState = await getVaultFromBackup(); const reduxState = ReduxService.store.getState(); - const state = reduxState?.engine?.backgroundState || {}; + const state = reduxState?.engine?.backgroundState ?? {}; + const Engine = UntypedEngine; // This ensures we create an entirely new engine await Engine.destroyEngine(); this.engineInitialized = false; diff --git a/app/store/index.ts b/app/store/index.ts index 7dca2b0e1c86..1b36e26bd524 100644 --- a/app/store/index.ts +++ b/app/store/index.ts @@ -1,4 +1,4 @@ -import { Store } from 'redux'; +import { AnyAction } from 'redux'; import { configureStore } from '@reduxjs/toolkit'; import { persistStore, persistReducer } from 'redux-persist'; import createSagaMiddleware from 'redux-saga'; @@ -12,20 +12,18 @@ import thunk from 'redux-thunk'; import persistConfig from './persistConfig'; import getUIStartupSpan from '../core/Performance/UIStartup'; -import ReduxService from '../core/redux'; +import ReduxService, { ReduxStore } from '../core/redux'; import { onPersistedDataLoaded } from '../actions/user'; import { validatePostMigrationState } from './validateMigration/validateMigration'; -// TODO: Improve type safety by using real Action types instead of `any` -// TODO: Replace "any" with type -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const pReducer = persistReducer(persistConfig, rootReducer); +// TODO: Improve type safety by using real Action types instead of `AnyAction` +const pReducer = persistReducer( + persistConfig, + rootReducer, +); -// TODO: Fix the Action type. It's set to `any` now because some of the -// TypeScript reducers have invalid actions -// TODO: Replace "any" with type -// eslint-disable-next-line @typescript-eslint/no-explicit-any, import/no-mutable-exports -let store: Store, persistor; +// eslint-disable-next-line import/no-mutable-exports +let store: ReduxStore, persistor; const createStoreAndPersistor = async () => { trace({ name: TraceName.StoreInit,