diff --git a/app/scripts/lib/ComposableObservableStore.js b/app/scripts/lib/ComposableObservableStore.js index 7e9892bea..789a12d80 100644 --- a/app/scripts/lib/ComposableObservableStore.js +++ b/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} + */ + config = {}; + /** * Create a new store * - * @param {Object} [initState] - The initial store state - * @param {Object} [config] - Map of internal state keys to child stores + * @param {Object} options + * @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) { - super(initState); + constructor({ config, controllerMessenger, state }) { + 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} 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)) { - config[key].subscribe((state) => { - this.updateState({ [key]: state }); - }); + for (const key of Object.keys(config)) { + if (!config[key]) { + 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 }); + }, + ); + } } } diff --git a/app/scripts/lib/ComposableObservableStore.test.js b/app/scripts/lib/ComposableObservableStore.test.js index 620b6df85..063f97cbf 100644 --- a/app/scripts/lib/ComposableObservableStore.test.js +++ b/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 store = new ComposableObservableStore(null, { - FooStore: fooStore, - BarStore: barStore, + const barController = new ExampleController({ + messenger: controllerMessenger, + }); + 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, + }), + ); + }); }); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 970ef9804..16c055f00 100644 --- a/app/scripts/metamask-controller.js +++ b/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( - { includeUSDRate: true }, - initState.CurrencyController, - ); + const currencyRateMessenger = controllerMessenger.getRestricted({ + name: 'CurrencyRateController', + }); + 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.setCurrentCurrency( - this.currencyRateController.state.currentCurrency, - (error) => { - if (error) { - throw error; - } - }, - ); + this.networkController.on(NETWORK_EVENTS.NETWORK_DID_CHANGE, async () => { + const { ticker } = this.networkController.getProviderConfig(); + try { + await this.currencyRateController.setNativeCurrency(ticker); + } catch (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, { - AppStateController: this.appStateController.store, - NetworkController: this.networkController.store, - AccountTracker: this.accountTracker.store, - TxController: this.txController.memStore, - CachedBalancesController: this.cachedBalancesController.store, - TokenRatesController: this.tokenRatesController.store, - MessageManager: this.messageManager.memStore, - PersonalMessageManager: this.personalMessageManager.memStore, - DecryptMessageManager: this.decryptMessageManager.memStore, - EncryptionPublicKeyManager: this.encryptionPublicKeyManager.memStore, - TypesMessageManager: this.typedMessageManager.memStore, - KeyringController: this.keyringController.memStore, - PreferencesController: this.preferencesController.store, - MetaMetricsController: this.metaMetricsController.store, - AddressBookController: this.addressBookController, - CurrencyController: this.currencyRateController, - AlertController: this.alertController.store, - OnboardingController: this.onboardingController.store, - IncomingTransactionsController: this.incomingTransactionsController.store, - PermissionsController: this.permissionsController.permissions, - PermissionsMetadata: this.permissionsController.store, - ThreeBoxController: this.threeBoxController.store, - SwapsController: this.swapsController.store, - EnsController: this.ensController.store, - ApprovalController: this.approvalController, - NotificationController: this.notificationController, + this.memStore = new ComposableObservableStore({ + config: { + AppStateController: this.appStateController.store, + NetworkController: this.networkController.store, + AccountTracker: this.accountTracker.store, + TxController: this.txController.memStore, + CachedBalancesController: this.cachedBalancesController.store, + TokenRatesController: this.tokenRatesController.store, + MessageManager: this.messageManager.memStore, + PersonalMessageManager: this.personalMessageManager.memStore, + DecryptMessageManager: this.decryptMessageManager.memStore, + EncryptionPublicKeyManager: this.encryptionPublicKeyManager.memStore, + TypesMessageManager: this.typedMessageManager.memStore, + KeyringController: this.keyringController.memStore, + PreferencesController: this.preferencesController.store, + MetaMetricsController: this.metaMetricsController.store, + AddressBookController: this.addressBookController, + CurrencyController: this.currencyRateController, + AlertController: this.alertController.store, + OnboardingController: this.onboardingController.store, + IncomingTransactionsController: this.incomingTransactionsController + .store, + PermissionsController: this.permissionsController.permissions, + PermissionsMetadata: this.permissionsController.store, + ThreeBoxController: this.threeBoxController.store, + SwapsController: this.swapsController.store, + 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. diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index 3d8cb57cf..87fed2aef 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -654,46 +654,24 @@ describe('MetaMaskController', function () { }); describe('#setCustomRpc', function () { - let rpcUrl; - - beforeEach(function () { - rpcUrl = metamaskController.setCustomRpc( + it('returns custom RPC that when called', async function () { + const rpcUrl = await metamaskController.setCustomRpc( CUSTOM_RPC_URL, CUSTOM_RPC_CHAIN_ID, ); + assert.equal(rpcUrl, CUSTOM_RPC_URL); }); - it('returns custom RPC that when called', async function () { - assert.equal(await rpcUrl, CUSTOM_RPC_URL); - }); - - it('changes the network controller rpc', function () { + it('changes the network controller rpc', async function () { + await metamaskController.setCustomRpc( + CUSTOM_RPC_URL, + CUSTOM_RPC_CHAIN_ID, + ); 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(); diff --git a/package.json b/package.json index 7842865ad..c46468f07 100644 --- a/package.json +++ b/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", diff --git a/ui/store/actions.test.js b/ui/store/actions.test.js index 409f03853..73975843d 100644 --- a/ui/store/actions.test.js +++ b/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) => - cb(new Error('error')), - ); + background.setCurrentCurrency = sinon + .stub() + .callsFake((_, cb) => cb(new Error('error'))); actions._setBackgroundConnection(background); const expectedActions = [ diff --git a/yarn.lock b/yarn.lock index 5c006c735..f1cfb5184 100644 --- a/yarn.lock +++ b/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": - version "8.0.0" - resolved "https://registry.yarnpkg.com/@metamask/controllers/-/controllers-8.0.0.tgz#42ac5aaef67a03d3fe599a67a36597e01902ca8d" - integrity sha512-TrteMifsCxV1g3WHcSD1X98fF4hKep3sXZNGfrvkPqa8mrF03hJke21WBSTRtvJ3vkNLRWgi+5I6lVXFTzbYuQ== +"@metamask/controllers@^9.0.0": + version "9.0.0" + resolved "https://registry.yarnpkg.com/@metamask/controllers/-/controllers-9.0.0.tgz#b8661f6fad246b20b92f684a63ab268b0e5248a7" + 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"