Migrate CurrencyRateController to BaseControllerV2

The CurrencyRateController has been migrated to the new base controller. The configuration can now only be set during construction, as it was never changed at runtime in practice with the old controller. Similarly, the `disable` function was removed as it wasn't relied upon in practice. The TokenRatesController has been detached temporarily from the CurrencyRateController, just to get this to compile and to get tests running. This can't be merged as-is, because now the TokenRatesController doesn't get currency rate updates anymore.
MetaMask · Apr 15, 2021 · 14b2f4f · 14b2f4f
1 parent f63c6f1
commit 14b2f4f
Show file tree

Hide file tree

Showing 7 changed files with 257 additions and 208 deletions.
diff --git a/src/BaseControllerV2.ts b/src/BaseControllerV2.ts
@@ -96,7 +96,7 @@ export interface StatePropertyMetadata<T> {
 
 type Json = null | boolean | number | string | Json[] | { [prop: string]: Json };
 
-type StateChangeEvent<N extends string, S, E> = E extends { type: `${N}:stateChange`; payload: [S, Patch[]] }
+export type StateChangeEvent<N extends string, S, E> = E extends { type: `${N}:stateChange`; payload: [S, Patch[]] }
   ? E
   : never;
 
@@ -108,7 +108,7 @@ export class BaseController<N extends string, S extends Record<string, unknown>>
 
   protected messagingSystem: RestrictedControllerMessenger<N, any, StateChangeEvent<N, S, any>, string, string>;
 
-  private name: N;
+  public name: N;
 
   public readonly metadata: StateMetadata<S>;
 

diff --git a/src/ComposableController.test.ts b/src/ComposableController.test.ts
@@ -2,15 +2,22 @@ import { stub } from 'sinon';
 import AddressBookController from './user/AddressBookController';
 import EnsController from './third-party/EnsController';
 import ComposableController from './ComposableController';
+import { ControllerMessenger } from './ControllerMessenger';
 import PreferencesController from './user/PreferencesController';
 import TokenRatesController from './assets/TokenRatesController';
 import { AssetsController } from './assets/AssetsController';
 import { NetworkController, NetworksChainId } from './network/NetworkController';
 import { AssetsContractController } from './assets/AssetsContractController';
-import CurrencyRateController from './assets/CurrencyRateController';
+import { CurrencyRateController, CurrencyRateStateChange } from './assets/CurrencyRateController';
 
 describe('ComposableController', () => {
   it('should compose controller state', () => {
+    const controllerMessenger = new ControllerMessenger<any, CurrencyRateStateChange>();
+    const currencyRateMessenger = controllerMessenger.getRestricted<'CurrencyRateController', never, never>({
+      name: 'CurrencyRateController',
+      allowedActions: [],
+      allowedEvents: [],
+    });
     const preferencesController = new PreferencesController();
     const networkController = new NetworkController();
     const assetContractController = new AssetsContractController();
@@ -21,20 +28,24 @@ describe('ComposableController', () => {
       getAssetSymbol: assetContractController.getAssetSymbol.bind(assetContractController),
       getCollectibleTokenURI: assetContractController.getCollectibleTokenURI.bind(assetContractController),
     });
-    const currencyRateController = new CurrencyRateController();
-    const controller = new ComposableController([
-      new AddressBookController(),
-      assetController,
-      assetContractController,
-      new EnsController(),
-      currencyRateController,
-      networkController,
-      preferencesController,
-      new TokenRatesController({
-        onAssetsStateChange: (listener) => assetController.subscribe(listener),
-        onCurrencyRateStateChange: (listener) => currencyRateController.subscribe(listener),
-      }),
-    ]);
+    const currencyRateController = new CurrencyRateController({ messenger: currencyRateMessenger });
+    const controller = new ComposableController(
+      [
+        new AddressBookController(),
+        assetController,
+        assetContractController,
+        new EnsController(),
+        currencyRateController as any,
+        networkController,
+        preferencesController,
+        new TokenRatesController({
+          onAssetsStateChange: (listener) => assetController.subscribe(listener),
+          onCurrencyRateStateChange: (listener) =>
+            controllerMessenger.subscribe('CurrencyRateController:stateChange', listener),
+        }),
+      ],
+      controllerMessenger,
+    );
     expect(controller.state).toEqual({
       AddressBookController: { addressBook: {} },
       AssetsContractController: {},
@@ -54,7 +65,9 @@ describe('ComposableController', () => {
         conversionRate: 0,
         currentCurrency: 'usd',
         nativeCurrency: 'ETH',
-        usdConversionRate: 0,
+        pendingCurrentCurrency: null,
+        pendingNativeCurrency: null,
+        usdConversionRate: null,
       },
       EnsController: {
         ensEntries: {},
@@ -76,6 +89,12 @@ describe('ComposableController', () => {
   });
 
   it('should compose flat controller state', () => {
+    const controllerMessenger = new ControllerMessenger<any, CurrencyRateStateChange>();
+    const currencyRateMessenger = controllerMessenger.getRestricted<'CurrencyRateController', never, never>({
+      name: 'CurrencyRateController',
+      allowedActions: [],
+      allowedEvents: [],
+    });
     const preferencesController = new PreferencesController();
     const networkController = new NetworkController();
     const assetContractController = new AssetsContractController();
@@ -86,20 +105,24 @@ describe('ComposableController', () => {
       getAssetSymbol: assetContractController.getAssetSymbol.bind(assetContractController),
       getCollectibleTokenURI: assetContractController.getCollectibleTokenURI.bind(assetContractController),
     });
-    const currencyRateController = new CurrencyRateController();
-    const controller = new ComposableController([
-      new AddressBookController(),
-      assetController,
-      assetContractController,
-      new EnsController(),
-      currencyRateController,
-      networkController,
-      preferencesController,
-      new TokenRatesController({
-        onAssetsStateChange: (listener) => assetController.subscribe(listener),
-        onCurrencyRateStateChange: (listener) => currencyRateController.subscribe(listener),
-      }),
-    ]);
+    const currencyRateController = new CurrencyRateController({ messenger: currencyRateMessenger });
+    const controller = new ComposableController(
+      [
+        new AddressBookController(),
+        assetController,
+        assetContractController,
+        new EnsController(),
+        currencyRateController as any,
+        networkController,
+        preferencesController,
+        new TokenRatesController({
+          onAssetsStateChange: (listener) => assetController.subscribe(listener),
+          onCurrencyRateStateChange: (listener) =>
+            controllerMessenger.subscribe('CurrencyRateController:stateChange', listener),
+        }),
+      ],
+      controllerMessenger,
+    );
     expect(controller.flatState).toEqual({
       addressBook: {},
       allCollectibleContracts: {},
@@ -121,15 +144,18 @@ describe('ComposableController', () => {
       lostIdentities: {},
       nativeCurrency: 'ETH',
       network: 'loading',
+      pendingCurrentCurrency: null,
+      pendingNativeCurrency: null,
       provider: { type: 'mainnet', chainId: NetworksChainId.mainnet },
       selectedAddress: '',
       suggestedAssets: [],
       tokens: [],
-      usdConversionRate: 0,
+      usdConversionRate: null,
     });
   });
 
   it('should set initial state', () => {
+    const controllerMessenger = new ControllerMessenger<any, any>();
     const state = {
       addressBook: {
         '0x1': {
@@ -143,13 +169,14 @@ describe('ComposableController', () => {
         },
       },
     };
-    const controller = new ComposableController([new AddressBookController(undefined, state)]);
+    const controller = new ComposableController([new AddressBookController(undefined, state)], controllerMessenger);
     expect(controller.state).toEqual({ AddressBookController: state });
   });
 
   it('should notify listeners of nested state change', () => {
+    const controllerMessenger = new ControllerMessenger<any, any>();
     const addressBookController = new AddressBookController();
-    const controller = new ComposableController([addressBookController]);
+    const controller = new ComposableController([addressBookController], controllerMessenger);
     const listener = stub();
     controller.subscribe(listener);
     addressBookController.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo');

diff --git a/src/ComposableController.ts b/src/ComposableController.ts
@@ -1,14 +1,18 @@
 import BaseController from './BaseController';
+import { ControllerMessenger } from './ControllerMessenger';
+import { BaseController as BaseControllerV2 } from './BaseControllerV2';
 
 /**
  * List of child controller instances
  */
-export type ControllerList = BaseController<any, any>[];
+export type ControllerList = (BaseController<any, any> | BaseControllerV2<any, any>)[];
 
 /**
  * Controller that can be used to compose multiple controllers together
  */
 export class ComposableController extends BaseController<never, any> {
+  private messagingSystem: ControllerMessenger<any, any>;
+
   private controllers: ControllerList = [];
 
   /**
@@ -22,21 +26,27 @@ export class ComposableController extends BaseController<never, any> {
    * @param controllers - Map of names to controller instances
    * @param initialState - Initial state keyed by child controller name
    */
-  constructor(controllers: ControllerList) {
+  constructor(controllers: ControllerList, messenger: ControllerMessenger<any, any>) {
     super(
       undefined,
       controllers.reduce((state, controller) => {
         state[controller.name] = controller.state;
         return state;
       }, {} as any),
     );
+    this.messagingSystem = messenger;
     this.initialize();
     this.controllers = controllers;
     this.controllers.forEach((controller) => {
       const { name } = controller;
-      controller.subscribe((state) => {
+      const eventHandler = (state: any) => {
         this.update({ [name]: state });
-      });
+      };
+      if (controller instanceof BaseController) {
+        controller.subscribe(eventHandler);
+      } else {
+        (this.messagingSystem.subscribe as any)(`${name}:updateState`, eventHandler);
+      }
     });
   }
 

diff --git a/src/ControllerMessenger.ts b/src/ControllerMessenger.ts
@@ -379,7 +379,7 @@ export class ControllerMessenger<Action extends ActionConstraint, Event extends
    * @param options.allowedEvents - The list of events that this restricted controller messenger
    *   should be allowed to subscribe to.
    */
-  getRestricted<N extends string, AllowedAction extends string, AllowedEvent extends string>({
+  getRestricted<N extends string, AllowedAction extends string | never, AllowedEvent extends string | never>({
     name,
     allowedActions,
     allowedEvents,