fix: `ComposableController` instantiation and functionality is broken

[MajorLift]

What is this about?

Explanation

ComposableController usage in the mobile Engine is currently broken in two ways.

1. The first argument of the ComposableController's constructor has a type constraint that is incompatible with V1 members of the controllers input array.

Argument of type '(PhishingController | AccountsController | AccountTrackerController | AddressBookController | ... 22 more ... | SwapsController)[]' is not assignable to parameter of type 'ControllerList'.
  Type 'PhishingController | AccountsController | AccountTrackerController | AddressBookController | ... 22 more ... | SwapsController' is not assignable to type 'BaseController<any, any> | { name: string; state: Record<string, unknown>; }'.
    Type 'AccountTrackerController' is not assignable to type 'BaseController<any, any> | { name: string; state: Record<string, unknown>; }'.
      Type 'AccountTrackerController' is not assignable to type 'BaseController<any, any>'.
        Types have separate declarations of a private property 'initialConfig'.ts(2345)

(This error is also encountered with later versions of ComposableController. See https://github.com/MetaMask/core/issues/4448)

Up until now, this error has been suppressed with the following comment, which is incorrect:

// @ts-expect-error The ComposableController needs to be updated to support BaseControllerV2

• ComposableController has supported V2 controllers since before its first release (see https://github.com/MetaMask/core/pull/447).
• The error message makes it clear that V1 controllers, not V2, are causing the issue.

2. The second argument passed into the ComposableController constructor has always been a ControllerMessenger class instance, when it should have been a RestrictedControllerMessenger.

    this.datamodel = new ComposableController(
      controllers,
-     this.controllerMessenger,
+     this.controllerMessenger.getRestricted({
+       name: 'ComposableController',
+       allowedActions: [],
+       allowedEvents: [
...
+       ],
+     )},
    );

This grants the ComposableController wider permissions than intended (we only want to allow stateChange events for all child controllers), and generally goes against our recommended usage pattern for messengers.

(However, it shouldn't prevent the controllerMessenger or ComposableController state from subscribing to state updates from child controllers)

Solution

1. Add tests to Engine.test.js file verifying that the datamodel has subscribed to state updates from all child controllers (both those with a messagingSystem, and V1 controllers with no messagingSystem).

  const engine = Engine.init(initialState);
  const { controllerMessenger } = engine;
  test.each(Object.entries(engine.context))('controller-messenger subscribes to all controllers', ([name, controller]) => {
    if (hasProperty(controller, 'messagingSystem')) {
      const eventType = `${name}:stateChange`;
      expect(controllerMessenger.unsubscribe(eventType)).toThrow(
        `Subscription not found for event: ${eventType}`,
      );
    } else if (hasProperty(controller, 'unsubscribe')) {
      expect(controller.unsubscribe((state) => {
        this.update({ [name]: state });
      })).toHaveReturned(true);
    } else {
      fail('not a valid controller');
    }
  });

2. Bump @metamask/composable-controller to the latest version which offers better type safety and quality-of-life improvements.

@metamask/[email protected] features widespread any usage, which would unnecessarily complicate any debugging efforts.

3. For the issue of defining a type constraint that can accept V1 controllers for the first argument controllers, attempt to resolve by bumping V1 controllers from mobile.

• All of the V1 controllers in Engine's context have a V2 release, except for
• SwapsController, which has a V2 upgrade being implemented by the accounts team, and
• AssetsContractController, which will be upgraded into a non-controller, necessitating changes in ComposableController (https://github.com/MetaMask/core/issues/4444).

If unsuccessful, wait until the issue is fixed from the ComposableController side: https://github.com/MetaMask/core/issues/4448

Scenario

No response

Design

No response

Technical Details

No response

Threat Modeling Framework

No response

Acceptance Criteria

No response

Stakeholder review needed before the work gets merged

• [ ] Engineering (needed in most cases)
• [ ] Design
• [ ] Product
• [ ] QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
• [ ] Security
• [ ] Legal
• [ ] Marketing
• [ ] Management (please specify)
• [ ] Other (please specify)

References

No response