Skip to content

Commit

Permalink
fix: Type fixes and optimizations for EngineService, Redux store (#…
Browse files Browse the repository at this point in the history
…12509)

## **Description**

- ~Define types: `ReduxStore`, `ReduxState`~ (superseded by
#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 #10441
- Blocked by MetaMask/core#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.
  • Loading branch information
MajorLift authored and nickewansmith committed Jan 30, 2025
1 parent a2d09de commit 205bfaf
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 150 deletions.
163 changes: 24 additions & 139 deletions app/core/EngineService/EngineService.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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';
Expand Down Expand Up @@ -52,15 +53,17 @@ 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,
});

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,
Expand All @@ -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(
Expand All @@ -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',
() => {
Expand All @@ -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]),
);
});
};

Expand All @@ -244,7 +128,8 @@ export class EngineService {
async initializeVaultFromBackup(): Promise<InitializeEngineResult> {
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;
Expand Down
20 changes: 9 additions & 11 deletions app/store/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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<RootState, any>(persistConfig, rootReducer);
// TODO: Improve type safety by using real Action types instead of `AnyAction`
const pReducer = persistReducer<RootState, AnyAction>(
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<RootState, any>, persistor;
// eslint-disable-next-line import/no-mutable-exports
let store: ReduxStore, persistor;
const createStoreAndPersistor = async () => {
trace({
name: TraceName.StoreInit,
Expand Down

0 comments on commit 205bfaf

Please sign in to comment.