Migrate to new CurrencyRateController (#11005)

The CurrencyRateController has been migrated to the BaseControllerV2
API, which includes various API changes. These changes include:
* The constructor now expects to be passed a
`RestrictedControllerMessenger`.
* State changes are subscribed to via the `ControllerMessenger` now,
rather than via a `subscribe` function.
* The state and configration are passed in as one "options" object,
rather than as two separate parameters
* The polling needs to be started explicitly by calling `start`. It
can be stopped and started on-demand now as well.
* Changing the current currency or native currency will now throw an
error if we fail to update the conversion rate.

The `ComposableObservableStore` has been updated to accomodate these
new types of controllers. The constructor has been updated to use an
options bag pattern as well, to make the addition of the new required
`controllerMessenger` parameter a bit less unweildly.

app/scripts/lib/ComposableObservableStore.js

@@ -1,18 +1,36 @@
 import { ObservableStore } from '@metamask/obs-store';
 
+/**
+ * @typedef {import('@metamask/controllers').ControllerMessenger} ControllerMessenger
+ */
+
 /**
  * An ObservableStore that can composes a flat
  * structure of child stores based on configuration
  */
 export default class ComposableObservableStore extends ObservableStore {
+  /**
+   * Describes which stores are being composed. The key is the name of the
+   * store, and the value is either an ObserableStore, or a controller that
+   * extends one of the two base controllers in the `@metamask/controllers`
+   * package.
+   * @type {Record<string, Object>}
+   */
+  config = {};
+
   /**
    * Create a new store
    *
-   * @param {Object} [initState] - The initial store state
+   * @param {Object} options
-   * @param {Object} [config] - Map of internal state keys to child stores
+   * @param {Object} [options.config] - Map of internal state keys to child stores
+   * @param {ControllerMessenger} options.controllerMessenger - The controller
+   *   messenger, used for subscribing to events from BaseControllerV2-based
+   *   controllers.
+   * @param {Object} [options.state] - The initial store state
    */
-  constructor(initState, config) {
+  constructor({ config, controllerMessenger, state }) {
-    super(initState);
+    super(state);
+    this.controllerMessenger = controllerMessenger;
     if (config) {
       this.updateStructure(config);
     }
@@ -21,15 +39,31 @@ export default class ComposableObservableStore extends ObservableStore {
   /**
    * Composes a new internal store subscription structure
    *
-   * @param {Object} [config] - Map of internal state keys to child stores
+   * @param {Record<string, Object>} config - Describes which stores are being
+   *   composed. The key is the name of the store, and the value is either an
+   *   ObserableStore, or a controller that extends one of the two base
+   *   controllers in the `@metamask/controllers` package.
    */
   updateStructure(config) {
     this.config = config;
     this.removeAllListeners();
-    for (const key of Object.keys(this.config)) {
+    for (const key of Object.keys(config)) {
-      config[key].subscribe((state) => {
+      if (!config[key]) {
-        this.updateState({ [key]: state });
+        throw new Error(`Undefined '${key}'`);
-      });
+      }
+      const store = config[key];
+      if (store.subscribe) {
+        config[key].subscribe((state) => {
+          this.updateState({ [key]: state });
+        });
+      } else {
+        this.controllerMessenger.subscribe(
+          `${store.name}:stateChange`,
+          (state) => {
+            this.updateState({ [key]: state });
+          },
+        );
+      }
     }
   }
 

app/scripts/lib/ComposableObservableStore.test.js

@@ -1,40 +1,194 @@
 import { strict as assert } from 'assert';
 import { ObservableStore } from '@metamask/obs-store';
+import {
+  BaseController,
+  BaseControllerV2,
+  ControllerMessenger,
+} from '@metamask/controllers';
 import ComposableObservableStore from './ComposableObservableStore';
 
+class OldExampleController extends BaseController {
+  name = 'OldExampleController';
+
+  defaultState = {
+    baz: 'baz',
+  };
+
+  constructor() {
+    super();
+    this.initialize();
+  }
+
+  updateBaz(contents) {
+    this.update({ baz: contents });
+  }
+}
+class ExampleController extends BaseControllerV2 {
+  static defaultState = {
+    bar: 'bar',
+  };
+
+  static metadata = {
+    bar: { persist: true, anonymous: true },
+  };
+
+  constructor({ messenger }) {
+    super({
+      messenger,
+      name: 'ExampleController',
+      metadata: ExampleController.metadata,
+      state: ExampleController.defaultState,
+    });
+  }
+
+  updateBar(contents) {
+    this.update(() => {
+      return { bar: contents };
+    });
+  }
+}
+
 describe('ComposableObservableStore', function () {
   it('should register initial state', function () {
-    const store = new ComposableObservableStore('state');
+    const controllerMessenger = new ControllerMessenger();
+    const store = new ComposableObservableStore({
+      controllerMessenger,
+      state: 'state',
+    });
     assert.strictEqual(store.getState(), 'state');
   });
 
   it('should register initial structure', function () {
+    const controllerMessenger = new ControllerMessenger();
     const testStore = new ObservableStore();
-    const store = new ComposableObservableStore(null, { TestStore: testStore });
+    const store = new ComposableObservableStore({
+      config: { TestStore: testStore },
+      controllerMessenger,
+    });
     testStore.putState('state');
     assert.deepEqual(store.getState(), { TestStore: 'state' });
   });
 
-  it('should update structure', function () {
+  it('should update structure with observable store', function () {
+    const controllerMessenger = new ControllerMessenger();
     const testStore = new ObservableStore();
-    const store = new ComposableObservableStore();
+    const store = new ComposableObservableStore({ controllerMessenger });
     store.updateStructure({ TestStore: testStore });
     testStore.putState('state');
     assert.deepEqual(store.getState(), { TestStore: 'state' });
   });
 
+  it('should update structure with BaseController-based controller', function () {
+    const controllerMessenger = new ControllerMessenger();
+    const oldExampleController = new OldExampleController();
+    const store = new ComposableObservableStore({ controllerMessenger });
+    store.updateStructure({ OldExample: oldExampleController });
+    oldExampleController.updateBaz('state');
+    assert.deepEqual(store.getState(), { OldExample: { baz: 'state' } });
+  });
+
+  it('should update structure with BaseControllerV2-based controller', function () {
+    const controllerMessenger = new ControllerMessenger();
+    const exampleController = new ExampleController({
+      messenger: controllerMessenger,
+    });
+    const store = new ComposableObservableStore({ controllerMessenger });
+    store.updateStructure({ Example: exampleController });
+    exampleController.updateBar('state');
+    console.log(exampleController.state);
+    assert.deepEqual(store.getState(), { Example: { bar: 'state' } });
+  });
+
+  it('should update structure with all three types of stores', function () {
+    const controllerMessenger = new ControllerMessenger();
+    const exampleStore = new ObservableStore();
+    const exampleController = new ExampleController({
+      messenger: controllerMessenger,
+    });
+    const oldExampleController = new OldExampleController();
+    const store = new ComposableObservableStore({ controllerMessenger });
+    store.updateStructure({
+      Example: exampleController,
+      OldExample: oldExampleController,
+      Store: exampleStore,
+    });
+    exampleStore.putState('state');
+    exampleController.updateBar('state');
+    oldExampleController.updateBaz('state');
+    assert.deepEqual(store.getState(), {
+      Example: { bar: 'state' },
+      OldExample: { baz: 'state' },
+      Store: 'state',
+    });
+  });
+
   it('should return flattened state', function () {
+    const controllerMessenger = new ControllerMessenger();
     const fooStore = new ObservableStore({ foo: 'foo' });
-    const barStore = new ObservableStore({ bar: 'bar' });
+    const barController = new ExampleController({
-    const store = new ComposableObservableStore(null, {
+      messenger: controllerMessenger,
-      FooStore: fooStore,
+    });
-      BarStore: barStore,
+    const bazController = new OldExampleController();
+    const store = new ComposableObservableStore({
+      config: {
+        FooStore: fooStore,
+        BarStore: barController,
+        BazStore: bazController,
+      },
+      controllerMessenger,
+      state: {
+        FooStore: fooStore.getState(),
+        BarStore: barController.state,
+        BazStore: bazController.state,
+      },
+    });
+    assert.deepEqual(store.getFlatState(), {
+      foo: 'foo',
+      bar: 'bar',
+      baz: 'baz',
     });
-    assert.deepEqual(store.getFlatState(), { foo: 'foo', bar: 'bar' });
   });
 
   it('should return empty flattened state when not configured', function () {
-    const store = new ComposableObservableStore();
+    const controllerMessenger = new ControllerMessenger();
+    const store = new ComposableObservableStore({ controllerMessenger });
     assert.deepEqual(store.getFlatState(), {});
   });
+
+  it('should throw if the controller messenger is omitted and the config includes a BaseControllerV2 controller', function () {
+    const controllerMessenger = new ControllerMessenger();
+    const exampleController = new ExampleController({
+      messenger: controllerMessenger,
+    });
+    assert.throws(
+      () =>
+        new ComposableObservableStore({
+          config: {
+            Example: exampleController,
+          },
+        }),
+    );
+  });
+
+  it('should throw if the controller messenger is omitted and updateStructure called with a BaseControllerV2 controller', function () {
+    const controllerMessenger = new ControllerMessenger();
+    const exampleController = new ExampleController({
+      messenger: controllerMessenger,
+    });
+    const store = new ComposableObservableStore({});
+    assert.throws(() => store.updateStructure({ Example: exampleController }));
+  });
+
+  it('should throw if initialized with undefined config entry', function () {
+    const controllerMessenger = new ControllerMessenger();
+    assert.throws(
+      () =>
+        new ComposableObservableStore({
+          config: {
+            Example: undefined,
+          },
+          controllerMessenger,
+        }),
+    );
+  });
 });

app/scripts/metamask-controller.js

@@ -20,6 +20,7 @@ import contractMap from '@metamask/contract-metadata';
 import {
   AddressBookController,
   ApprovalController,
+  ControllerMessenger,
   CurrencyRateController,
   PhishingController,
   NotificationController,
@@ -96,8 +97,13 @@ export default class MetamaskController extends EventEmitter {
     this.getRequestAccountTabIds = opts.getRequestAccountTabIds;
     this.getOpenMetamaskTabsIds = opts.getOpenMetamaskTabsIds;
 
+    const controllerMessenger = new ControllerMessenger();
+
     // observable state store
-    this.store = new ComposableObservableStore(initState);
+    this.store = new ComposableObservableStore({
+      state: initState,
+      controllerMessenger,
+    });
 
     // external connections by origin
     // Do not modify directly. Use the associated methods.
@@ -157,10 +163,14 @@ export default class MetamaskController extends EventEmitter {
       preferencesStore: this.preferencesController.store,
     });
 
-    this.currencyRateController = new CurrencyRateController(
+    const currencyRateMessenger = controllerMessenger.getRestricted({
-      { includeUSDRate: true },
+      name: 'CurrencyRateController',
-      initState.CurrencyController,
+    });
-    );
+    this.currencyRateController = new CurrencyRateController({
+      includeUSDRate: true,
+      messenger: currencyRateMessenger,
+      state: initState.CurrencyController,
+    });
 
     this.phishingController = new PhishingController();
 
@@ -222,10 +232,12 @@ export default class MetamaskController extends EventEmitter {
         this.accountTracker.start();
         this.incomingTransactionsController.start();
         this.tokenRatesController.start();
+        this.currencyRateController.start();
       } else {
         this.accountTracker.stop();
         this.incomingTransactionsController.stop();
         this.tokenRatesController.stop();
+        this.currencyRateController.stop();
       }
     });
 
@@ -363,18 +375,15 @@ export default class MetamaskController extends EventEmitter {
       }
     });
 
-    this.networkController.on(NETWORK_EVENTS.NETWORK_DID_CHANGE, () => {
+    this.networkController.on(NETWORK_EVENTS.NETWORK_DID_CHANGE, async () => {
-      this.setCurrentCurrency(
+      const { ticker } = this.networkController.getProviderConfig();
-        this.currencyRateController.state.currentCurrency,
+      try {
-        (error) => {
+        await this.currencyRateController.setNativeCurrency(ticker);
-          if (error) {
+      } catch (error) {
-            throw error;
+        // TODO: Handle failure to get conversion rate more gracefully
-          }
+        console.error(error);
-        },
+      }
-      );
     });
-    const { ticker } = this.networkController.getProviderConfig();
-    this.currencyRateController.configure({ nativeCurrency: ticker ?? 'ETH' });
     this.networkController.lookupNetwork();
     this.messageManager = new MessageManager();
     this.personalMessageManager = new PersonalMessageManager();
@@ -438,33 +447,37 @@ export default class MetamaskController extends EventEmitter {
       NotificationController: this.notificationController,
     });
 
-    this.memStore = new ComposableObservableStore(null, {
+    this.memStore = new ComposableObservableStore({
-      AppStateController: this.appStateController.store,
+      config: {
-      NetworkController: this.networkController.store,
+        AppStateController: this.appStateController.store,
-      AccountTracker: this.accountTracker.store,
+        NetworkController: this.networkController.store,
-      TxController: this.txController.memStore,
+        AccountTracker: this.accountTracker.store,
-      CachedBalancesController: this.cachedBalancesController.store,
+        TxController: this.txController.memStore,
-      TokenRatesController: this.tokenRatesController.store,
+        CachedBalancesController: this.cachedBalancesController.store,
-      MessageManager: this.messageManager.memStore,
+        TokenRatesController: this.tokenRatesController.store,
-      PersonalMessageManager: this.personalMessageManager.memStore,
+        MessageManager: this.messageManager.memStore,
-      DecryptMessageManager: this.decryptMessageManager.memStore,
+        PersonalMessageManager: this.personalMessageManager.memStore,
-      EncryptionPublicKeyManager: this.encryptionPublicKeyManager.memStore,
+        DecryptMessageManager: this.decryptMessageManager.memStore,
-      TypesMessageManager: this.typedMessageManager.memStore,
+        EncryptionPublicKeyManager: this.encryptionPublicKeyManager.memStore,
-      KeyringController: this.keyringController.memStore,
+        TypesMessageManager: this.typedMessageManager.memStore,
-      PreferencesController: this.preferencesController.store,
+        KeyringController: this.keyringController.memStore,
-      MetaMetricsController: this.metaMetricsController.store,
+        PreferencesController: this.preferencesController.store,
-      AddressBookController: this.addressBookController,
+        MetaMetricsController: this.metaMetricsController.store,
-      CurrencyController: this.currencyRateController,
+        AddressBookController: this.addressBookController,
-      AlertController: this.alertController.store,
+        CurrencyController: this.currencyRateController,
-      OnboardingController: this.onboardingController.store,
+        AlertController: this.alertController.store,
-      IncomingTransactionsController: this.incomingTransactionsController.store,
+        OnboardingController: this.onboardingController.store,
-      PermissionsController: this.permissionsController.permissions,
+        IncomingTransactionsController: this.incomingTransactionsController
-      PermissionsMetadata: this.permissionsController.store,
+          .store,
-      ThreeBoxController: this.threeBoxController.store,
+        PermissionsController: this.permissionsController.permissions,
-      SwapsController: this.swapsController.store,
+        PermissionsMetadata: this.permissionsController.store,
-      EnsController: this.ensController.store,
+        ThreeBoxController: this.threeBoxController.store,
-      ApprovalController: this.approvalController,
+        SwapsController: this.swapsController.store,
-      NotificationController: this.notificationController,
+        EnsController: this.ensController.store,
+        ApprovalController: this.approvalController,
+        NotificationController: this.notificationController,
+      },
+      controllerMessenger,
     });
     this.memStore.subscribe(this.sendUpdate.bind(this));
 
@@ -648,7 +661,11 @@ export default class MetamaskController extends EventEmitter {
     return {
       // etc
       getState: (cb) => cb(null, this.getState()),
-      setCurrentCurrency: this.setCurrentCurrency.bind(this),
+      setCurrentCurrency: nodeify(
+        this.currencyRateController.setCurrentCurrency.bind(
+          this.currencyRateController,
+        ),
+      ),
       setUseBlockie: this.setUseBlockie.bind(this),
       setUseNonceField: this.setUseNonceField.bind(this),
       setUsePhishDetect: this.setUsePhishDetect.bind(this),
@@ -2507,29 +2524,6 @@ export default class MetamaskController extends EventEmitter {
 
   // Log blocks
 
-  /**
-   * A method for setting the user's preferred display currency.
-   * @param {string} currencyCode - The code of the preferred currency.
-   * @param {Function} cb - A callback function returning currency info.
-   */
-  setCurrentCurrency(currencyCode, cb) {
-    const { ticker } = this.networkController.getProviderConfig();
-    try {
-      const currencyState = {
-        nativeCurrency: ticker,
-        currentCurrency: currencyCode,
-      };
-      this.currencyRateController.update(currencyState);
-      this.currencyRateController.configure(currencyState);
-      cb(null);
-      return;
-    } catch (err) {
-      cb(err);
-      // eslint-disable-next-line no-useless-return
-      return;
-    }
-  }
-
   /**
    * A method for selecting a custom URL for an ethereum RPC provider and updating it
    * @param {string} rpcUrl - A URL for a valid Ethereum RPC API.

app/scripts/metamask-controller.test.js

@@ -654,46 +654,24 @@ describe('MetaMaskController', function () {
   });
 
   describe('#setCustomRpc', function () {
-    let rpcUrl;
+    it('returns custom RPC that when called', async function () {
-
+      const rpcUrl = await metamaskController.setCustomRpc(
-    beforeEach(function () {
-      rpcUrl = metamaskController.setCustomRpc(
         CUSTOM_RPC_URL,
         CUSTOM_RPC_CHAIN_ID,
       );
+      assert.equal(rpcUrl, CUSTOM_RPC_URL);
     });
 
-    it('returns custom RPC that when called', async function () {
+    it('changes the network controller rpc', async function () {
-      assert.equal(await rpcUrl, CUSTOM_RPC_URL);
+      await metamaskController.setCustomRpc(
-    });
+        CUSTOM_RPC_URL,
-
+        CUSTOM_RPC_CHAIN_ID,
-    it('changes the network controller rpc', function () {
+      );
       const networkControllerState = metamaskController.networkController.store.getState();
       assert.equal(networkControllerState.provider.rpcUrl, CUSTOM_RPC_URL);
     });
   });
 
-  describe('#setCurrentCurrency', function () {
-    let defaultMetaMaskCurrency;
-
-    beforeEach(function () {
-      defaultMetaMaskCurrency =
-        metamaskController.currencyRateController.state.currentCurrency;
-    });
-
-    it('defaults to usd', function () {
-      assert.equal(defaultMetaMaskCurrency, 'usd');
-    });
-
-    it('sets currency to JPY', function () {
-      metamaskController.setCurrentCurrency('JPY', noop);
-      assert.equal(
-        metamaskController.currencyRateController.state.currentCurrency,
-        'JPY',
-      );
-    });
-  });
-
   describe('#addNewAccount', function () {
     it('errors when an primary keyring is does not exist', async function () {
       const addNewAccount = metamaskController.addNewAccount();

package.json

@@ -95,7 +95,7 @@
     "@lavamoat/preinstall-always-fail": "^1.0.0",
     "@material-ui/core": "^4.11.0",
     "@metamask/contract-metadata": "^1.22.0",
-    "@metamask/controllers": "^8.0.0",
+    "@metamask/controllers": "^9.0.0",
     "@metamask/eth-ledger-bridge-keyring": "^0.5.0",
     "@metamask/eth-token-tracker": "^3.0.1",
     "@metamask/etherscan-link": "^2.1.0",

ui/store/actions.test.js

@@ -618,7 +618,7 @@ describe('Actions', () => {
 
     it('calls setCurrentCurrency', async () => {
       const store = mockStore();
-      background.setCurrentCurrency.callsFake((_, cb) => cb());
+      background.setCurrentCurrency = sinon.stub().callsFake((_, cb) => cb());
       actions._setBackgroundConnection(background);
 
       await store.dispatch(actions.setCurrentCurrency('jpy'));
@@ -627,9 +627,9 @@ describe('Actions', () => {
 
     it('throws if setCurrentCurrency throws', async () => {
       const store = mockStore();
-      background.setCurrentCurrency.callsFake((_, cb) =>
+      background.setCurrentCurrency = sinon
-        cb(new Error('error')),
+        .stub()
-      );
+        .callsFake((_, cb) => cb(new Error('error')));
       actions._setBackgroundConnection(background);
 
       const expectedActions = [

yarn.lock

@@ -2630,7 +2630,7 @@
     semver "^7.3.5"
     yargs "^17.0.1"
 
-"@metamask/contract-metadata@^1.19.0", "@metamask/contract-metadata@^1.22.0", "@metamask/contract-metadata@^1.24.0":
+"@metamask/contract-metadata@^1.19.0", "@metamask/contract-metadata@^1.22.0", "@metamask/contract-metadata@^1.25.0":
   version "1.25.0"
   resolved "https://registry.yarnpkg.com/@metamask/contract-metadata/-/contract-metadata-1.25.0.tgz#442ace91fb40165310764b68d8096d0017bb0492"
   integrity sha512-yhmYB9CQPv0dckNcPoWDcgtrdUp0OgK0uvkRE5QIBv4b3qENI1/03BztvK2ijbTuMlORUpjPq7/1MQDUPoRPVw==
@@ -2663,18 +2663,18 @@
     web3 "^0.20.7"
     web3-provider-engine "^16.0.1"
 
-"@metamask/controllers@^8.0.0":
+"@metamask/controllers@^9.0.0":
-  version "8.0.0"
+  version "9.0.0"
-  resolved "https://registry.yarnpkg.com/@metamask/controllers/-/controllers-8.0.0.tgz#42ac5aaef67a03d3fe599a67a36597e01902ca8d"
+  resolved "https://registry.yarnpkg.com/@metamask/controllers/-/controllers-9.0.0.tgz#b8661f6fad246b20b92f684a63ab268b0e5248a7"
-  integrity sha512-TrteMifsCxV1g3WHcSD1X98fF4hKep3sXZNGfrvkPqa8mrF03hJke21WBSTRtvJ3vkNLRWgi+5I6lVXFTzbYuQ==
+  integrity sha512-qj6sKw5j7a542i+EnN/XJW2JsZZzYbj5j8NjYBiRE70kNsyTQ7iXaaEdyXO6re+OAI5M1ULuWgTULoFlWiuHcA==
   dependencies:
-    "@metamask/contract-metadata" "^1.24.0"
+    "@metamask/contract-metadata" "^1.25.0"
     "@types/uuid" "^8.3.0"
     async-mutex "^0.2.6"
     babel-runtime "^6.26.0"
     eth-ens-namehash "^2.0.8"
     eth-json-rpc-infura "^5.1.0"
-    eth-keyring-controller "^6.1.0"
+    eth-keyring-controller "^6.2.1"
     eth-method-registry "1.1.0"
     eth-phishing-detect "^1.1.13"
     eth-query "^2.1.2"
@@ -10639,6 +10639,21 @@ eth-keyring-controller@^6.2.0:
     loglevel "^1.5.0"
     obs-store "^4.0.3"
 
+eth-keyring-controller@^6.2.1:
+  version "6.2.1"
+  resolved "https://registry.yarnpkg.com/eth-keyring-controller/-/eth-keyring-controller-6.2.1.tgz#61901071fc74059ed37cb5ae93870fdcae6e3781"
+  integrity sha512-x2gTM1iHp2Kbvdtd9Eslysw0qzVZiqOzpVB3AU/ni2Xiit+rlcv2H80zYKjrEwlfWFDj4YILD3bOqlnEMmRJOA==
+  dependencies:
+    bip39 "^2.4.0"
+    bluebird "^3.5.0"
+    browser-passworder "^2.0.3"
+    eth-hd-keyring "^3.6.0"
+    eth-sig-util "^3.0.1"
+    eth-simple-keyring "^4.2.0"
+    ethereumjs-util "^7.0.9"
+    loglevel "^1.5.0"
+    obs-store "^4.0.3"
+
 eth-lib@0.2.8:
   version "0.2.8"
   resolved "https://registry.yarnpkg.com/eth-lib/-/eth-lib-0.2.8.tgz#b194058bef4b220ad12ea497431d6cb6aa0623c8"