chore(deps): Bump @metamask/composable-controller from ^3.0.0 to ^10.0.0 (#10441)

## **Description**

This commit updates `@metamask/composable-controller` to `^10.0.0`. 

This involves fixing bugs outlined in
#10073, and applying
the major changes to the `ComposableController` API that has accumulated
between the intervening versions.

## Blocked by

- MetaMask/core#4968
- `composable-controller` v10 has been written to ensure that the effect
of this optimization is maximized.
- #12348
- #12407
- ~#11162

## Changelog

### Changed

- **BREAKING:** Bump `@metamask/composable-controller` from `^3.0.0` to
`^10.0.0`.
- **BREAKING:** Instantiate `ComposableController` class constructor
option `messenger` with a `RestrictedControllerMessenger` instance
derived from the `controllerMessenger` class field instance of `Engine`,
instead of passing in the unrestricted `controllerMessenger` instance
directly.
- **BREAKING:** Narrow external actions allowlist for `messenger`
instance passed into `ComposableController` constructor from
`GlobalActions['type']` to an empty union.
- **BREAKING:** Narrow external events allowlist for `messenger`
instance passed into `ComposableController` constructor from
`GlobalEvents['type']` to a union of the `stateChange` events of all
controllers included in the `EngineState` type.
- Convert the `EngineState` interface to use type alias syntax to ensure
compatibility with types used in MetaMask controllers.

### Fixed

- **BREAKING:** Narrow `Engine` class `datamodel` field from `any` to
`ComposableController<EngineState, StatefulControllers>`.
- **BREAKING:** The `CurrencyRatesController` constructor now normalizes
`null` into 0 for the `conversionRate` values of all native currencies
keyed in the `currencyRates` object of `CurrencyRatesControllerState`.
- Restore previously suppressed type-level validation for
`ComposableController` constructor option `controllers`.

## **Related issues**

- Closes #10073
- Applies MetaMask/core#4467
  - Blocked by `@metamask/[email protected]` release.
- Supersedes #10011

## **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.

---------

Co-authored-by: Salah-Eddine Saakoun <[email protected]>

Author: MajorLift

app/core/BackgroundBridge/BackgroundBridge.js

@@ -497,7 +497,7 @@ export class BackgroundBridge extends EventEmitter {
    */
   getState() {
     const vault = Engine.context.KeyringController.state.vault;
-    const { selectedAddress } = Engine.datamodel.flatState;
+    const { PreferencesController: { selectedAddress } } = Engine.datamodel.state;
     return {
       isInitialized: !!vault,
       isUnlocked: true,

app/core/BackgroundBridge/WalletConnectPort.ts

@@ -23,7 +23,9 @@ class WalletConnectPort extends EventEmitter {
       if (msg?.data?.method === NOTIFICATION_NAMES.chainChanged) {
         // eslint-disable-next-line @typescript-eslint/ban-ts-comment
         // @ts-ignore
-        const { selectedAddress } = Engine.datamodel.flatState;
+        const {
+          PreferencesController: { selectedAddress },
+        } = Engine.datamodel.state;
         this._wcRequestActions?.updateSession?.({
           chainId: parseInt(msg.data.params.chainId, 16),
           accounts: [selectedAddress],

app/core/Engine/Engine.test.ts

@@ -70,14 +70,7 @@ describe('Engine', () => {
   it('matches initial state fixture', () => {
     const engine = Engine.init({});
     const initialBackgroundState = engine.datamodel.state;
-
-    // AssetsContractController is stateless in v37 resulting in an undefined state
-    const newBackgroundState = {
-      ...backgroundState,
-      AssetsContractController: undefined,
-    };
-
-    expect(initialBackgroundState).toStrictEqual(newBackgroundState);
+    expect(initialBackgroundState).toStrictEqual(backgroundState);
   });
 
   it('setSelectedAccount throws an error if no account exists for the given address', () => {
@@ -89,6 +82,30 @@ describe('Engine', () => {
     );
   });
 
+  it('normalizes CurrencyController state property conversionRate from null to 0', () => {
+    const ticker = 'ETH';
+    const state = {
+      CurrencyRateController: {
+        currentCurrency: 'usd' as const,
+        currencyRates: {
+          [ticker]: {
+            conversionRate: null,
+            conversionDate: 0,
+            usdConversionRate: null,
+          },
+        },
+      },
+    };
+    const engine = Engine.init(state);
+    expect(
+      engine.datamodel.state.CurrencyRateController.currencyRates[ticker],
+    ).toStrictEqual({
+      conversionRate: 0,
+      conversionDate: 0,
+      usdConversionRate: null,
+    });
+  });
+
   describe('getTotalFiatAccountBalance', () => {
     let engine: EngineClass;
     afterEach(() => engine?.destroyEngineInstance());

app/core/Engine/Engine.ts

@@ -200,11 +200,15 @@ import { JsonMap } from '../Analytics/MetaMetrics.types';
 import { isPooledStakingFeatureEnabled } from '../../components/UI/Stake/constants';
 import {
   ControllerMessenger,
-  Controllers,
   EngineState,
   EngineContext,
   TransactionEventPayload,
+  StatefulControllers,
 } from './types';
+import {
+  BACKGROUND_STATE_CHANGE_EVENT_NAMES,
+  STATELESS_NON_CONTROLLER_NAMES,
+} from './constants';
 import {
   getGlobalChainId,
   getGlobalNetworkClientId,
@@ -239,9 +243,7 @@ export class Engine {
   /**
    * ComposableController reference containing all child controllers
    */
-  // TODO: Replace "any" with type
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  datamodel: any;
+  datamodel: ComposableController<EngineState, StatefulControllers>;
 
   /**
    * Object containing the info for the latest incoming tx block
@@ -449,7 +451,25 @@ export class Engine {
         allowedActions: [`${networkController.name}:getNetworkClientById`],
         allowedEvents: [],
       }),
-      state: initialState.CurrencyRateController,
+      // normalize `null` currencyRate to `0`
+      // TODO: handle `null` currencyRate by hiding fiat values instead
+      state: {
+        ...initialState.CurrencyRateController,
+        currencyRates: Object.fromEntries(
+          Object.entries(
+            initialState.CurrencyRateController?.currencyRates ?? {
+              ETH: {
+                conversionRate: 0,
+                conversionDate: 0,
+                usdConversionRate: null,
+              },
+            },
+          ).map(([k, v]) => [
+            k,
+            { ...v, conversionRate: v.conversionRate ?? 0 },
+          ]),
+        ),
+      },
     });
 
     const gasFeeController = new GasFeeController({
@@ -1457,24 +1477,21 @@ export class Engine {
       }),
     };
 
-    // Avoiding `Object.values` and `getKnownPropertyNames` for performance benefits: https://www.measurethat.net/Benchmarks/Show/7173/0/objectvalues-vs-reduce
-    const controllers = (
-      Object.keys(this.context) as (keyof Controllers)[]
-    ).reduce<Controllers[keyof Controllers][]>(
-      (controllers, controllerName) => {
-        const controller = this.context[controllerName];
-        if (controller) {
-          controllers.push(controller);
-        }
-        return controllers;
+    const childControllers = Object.assign({}, this.context);
+    STATELESS_NON_CONTROLLER_NAMES.forEach((name) => {
+      if (name in childControllers && childControllers[name]) {
+        delete childControllers[name];
+      }
+    });
+    this.datamodel = new ComposableController<EngineState, StatefulControllers>(
+      {
+        controllers: childControllers as StatefulControllers,
+        messenger: this.controllerMessenger.getRestricted({
+          name: 'ComposableController',
+          allowedActions: [],
+          allowedEvents: Array.from(BACKGROUND_STATE_CHANGE_EVENT_NAMES),
+        }),
       },
-      [],
-    );
-
-    this.datamodel = new ComposableController(
-      // @ts-expect-error TODO: Filter out non-controller instances
-      controllers,
-      this.controllerMessenger,
     );
 
     const { NftController: nfts } = this.context;
@@ -2074,22 +2091,12 @@ export default {
       AccountsController,
     } = instance.datamodel.state;
 
-    // normalize `null` currencyRate to `0`
-    // TODO: handle `null` currencyRate by hiding fiat values instead
-    const modifiedCurrencyRateControllerState = {
-      ...CurrencyRateController,
-      conversionRate:
-        CurrencyRateController.conversionRate === null
-          ? 0
-          : CurrencyRateController.conversionRate,
-    };
-
     return {
       AccountTrackerController,
       AddressBookController,
       NftController,
       TokenListController,
-      CurrencyRateController: modifiedCurrencyRateControllerState,
+      CurrencyRateController,
       KeyringController,
       NetworkController,
       PhishingController,

app/core/Engine/constants.ts

@@ -0,0 +1,45 @@
+/**
+ * Messageable modules that are part of the Engine's context, but are not defined with state.
+ * TODO: Replace with type guard once consistent inheritance for non-controllers is implemented. See: https://github.com/MetaMask/decisions/pull/41
+ */
+export const STATELESS_NON_CONTROLLER_NAMES = [
+  'AssetsContractController',
+  'NftDetectionController',
+  'TokenDetectionController',
+] as const;
+
+export const BACKGROUND_STATE_CHANGE_EVENT_NAMES = [
+  'AccountsController:stateChange',
+  'AccountTrackerController:stateChange',
+  'AddressBookController:stateChange',
+  'ApprovalController:stateChange',
+  'CurrencyRateController:stateChange',
+  'GasFeeController:stateChange',
+  'KeyringController:stateChange',
+  'LoggingController:stateChange',
+  'NetworkController:stateChange',
+  'NftController:stateChange',
+  'PermissionController:stateChange',
+  'PhishingController:stateChange',
+  'PPOMController:stateChange',
+  'PreferencesController:stateChange',
+  'RemoteFeatureFlagController:stateChange',
+  'SelectedNetworkController:stateChange',
+  'SignatureController:stateChange',
+  'SmartTransactionsController:stateChange',
+  'SwapsController:stateChange',
+  'TokenBalancesController:stateChange',
+  'TokenListController:stateChange',
+  'TokenRatesController:stateChange',
+  'TokensController:stateChange',
+  'TransactionController:stateChange',
+  ///: BEGIN:ONLY_INCLUDE_IF(preinstalled-snaps,external-snaps)
+  'SnapController:stateChange',
+  'SnapsRegistry:stateChange',
+  'SubjectMetadataController:stateChange',
+  'AuthenticationController:stateChange',
+  'UserStorageController:stateChange',
+  'NotificationServicesController:stateChange',
+  'NotificationServicesPushController:stateChange',
+  ///: END:ONLY_INCLUDE_IF
+] as const;

app/core/Engine/types.ts

@@ -159,10 +159,16 @@ import {
   AccountsControllerState,
 } from '@metamask/accounts-controller';
 import { getPermissionSpecifications } from '../Permissions/specifications.js';
+import { ComposableControllerEvents } from '@metamask/composable-controller';
+import { STATELESS_NON_CONTROLLER_NAMES } from './constants';
 import {
   RemoteFeatureFlagController,
-  RemoteFeatureFlagControllerState
+  RemoteFeatureFlagControllerState,
 } from '@metamask/remote-feature-flag-controller';
+import {
+  RemoteFeatureFlagControllerActions,
+  RemoteFeatureFlagControllerEvents,
+} from '@metamask/remote-feature-flag-controller/dist/remote-feature-flag-controller.cjs';
 
 /**
  * Controllers that area always instantiated
@@ -174,6 +180,14 @@ type RequiredControllers = Omit<Controllers, 'PPOMController'>;
  */
 type OptionalControllers = Pick<Controllers, 'PPOMController'>;
 
+/**
+ * Controllers that are defined with state.
+ */
+export type StatefulControllers = Omit<
+  Controllers,
+  (typeof STATELESS_NON_CONTROLLER_NAMES)[number]
+>;
+
 type PermissionsByRpcMethod = ReturnType<typeof getPermissionSpecifications>;
 type Permissions = PermissionsByRpcMethod[keyof PermissionsByRpcMethod];
 
@@ -223,9 +237,11 @@ type GlobalActions =
   | TransactionControllerActions
   | SelectedNetworkControllerActions
   | SmartTransactionsControllerActions
-  | AssetsContractControllerActions;
+  | AssetsContractControllerActions
+  | RemoteFeatureFlagControllerActions;
 
 type GlobalEvents =
+  | ComposableControllerEvents<EngineState>
   | AccountTrackerControllerEvents
   | NftControllerEvents
   | SwapsControllerEvents
@@ -255,7 +271,8 @@ type GlobalEvents =
   | TransactionControllerEvents
   | SelectedNetworkControllerEvents
   | SmartTransactionsControllerEvents
-  | AssetsContractControllerEvents;
+  | AssetsContractControllerEvents
+  | RemoteFeatureFlagControllerEvents;
 
 // TODO: Abstract this into controller utils for TransactionController
 export interface TransactionEventPayload {
@@ -276,7 +293,10 @@ export type ControllerMessenger = ExtendedControllerMessenger<
 /**
  * All mobile controllers, keyed by name
  */
-export interface Controllers {
+// Interfaces are incompatible with our controllers and state types by default.
+// Adding an index signature fixes this, but at the cost of widening the type unnecessarily.
+// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
+export type Controllers = {
   AccountsController: AccountsController;
   AccountTrackerController: AccountTrackerController;
   AddressBookController: AddressBookController;
@@ -316,7 +336,7 @@ export interface Controllers {
   NotificationServicesPushController: NotificationServicesPushController.Controller;
   ///: END:ONLY_INCLUDE_IF
   SwapsController: SwapsController;
-}
+};
 
 /**
  * Combines required and optional controllers for the Engine context type.
@@ -326,7 +346,10 @@ export type EngineContext = RequiredControllers & Partial<OptionalControllers>;
 /**
  * All engine state, keyed by controller name
  */
-export interface EngineState {
+// Interfaces are incompatible with our controllers and state types by default.
+// Adding an index signature fixes this, but at the cost of widening the type unnecessarily.
+// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
+export type EngineState = {
   AccountTrackerController: AccountTrackerControllerState;
   AddressBookController: AddressBookControllerState;
   NftController: NftControllerState;
@@ -359,4 +382,4 @@ export interface EngineState {
   PPOMController: PPOMState;
   AccountsController: AccountsControllerState;
   SelectedNetworkController: SelectedNetworkControllerState;
-}
+};

app/core/EngineService/EngineService.test.ts

@@ -35,7 +35,10 @@ jest.mock('../Engine', () => {
     destroyEngine: jest.fn(),
     init: jest.fn((_, keyringState) => {
       instance = {
-        controllerMessenger: { subscribe: jest.fn() },
+        controllerMessenger: {
+          subscribe: jest.fn(),
+          subscribeOnceIf: jest.fn(),
+        },
         context: {
           AddressBookController: { subscribe: jest.fn() },
           KeyringController: {

app/core/EngineService/EngineService.ts

@@ -173,29 +173,27 @@ class EngineService {
       },
     ];
 
-    engine?.datamodel?.subscribe?.(() => {
-      if (!engine.context.KeyringController.metadata.vault) {
-        Logger.log('keyringController vault missing for INIT_BG_STATE_KEY');
-      }
-      if (!this.engineInitialized) {
+    engine.controllerMessenger.subscribeOnceIf(
+      'ComposableController:stateChange',
+      () => {
+        if (!engine.context.KeyringController.metadata.vault) {
+          Logger.log('keyringController vault missing for INIT_BG_STATE_KEY');
+        }
         store.dispatch({ type: INIT_BG_STATE_KEY });
         this.engineInitialized = true;
-      }
-    });
+      },
+      () => !this.engineInitialized,
+    );
 
     controllers.forEach((controller) => {
-      const { name, key = undefined } = 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');
         }
         store.dispatch({ type: UPDATE_BG_STATE_KEY, payload: { key: name } });
       };
-      if (key) {
-        engine.controllerMessenger.subscribe(key, update_bg_state_cb);
-      } else {
-        engine.context[name].subscribe(update_bg_state_cb);
-      }
+      engine.controllerMessenger.subscribe(key, update_bg_state_cb);
     });
   };
 

app/util/test/initial-background-state.json

@@ -21,7 +21,6 @@
   "AddressBookController": {
     "addressBook": {}
   },
-  "AssetsContractController": {},
   "NftController": {
     "allNftContracts": {},
     "allNfts": {},
@@ -40,8 +39,6 @@
     "tokensChainsCache": {},
     "preventPollingOnNetworkRestart": false
   },
-  "TokenDetectionController": {},
-  "NftDetectionController": {},
   "PPOMController": {
     "storageMetadata": [],
     "versionInfo": []