From 39dff02a0404072d1266fad64748c575377e8fa7 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Mon, 17 Apr 2023 13:24:13 -0230 Subject: [PATCH] Consolidate network stores (#18595) * Consolidate network stores The network controller used to have multiple different state stores, which were composed together to form the main controller state store. They have been consolidated into a single store. This required few changes because most state access was already being done through the composed store. Fixes https://github.com/MetaMask/metamask-extension/issues/18303 * Add JSDoc comment --- .../controllers/network/network-controller.ts | 157 ++++++++---------- app/scripts/metamask-controller.js | 14 +- 2 files changed, 83 insertions(+), 88 deletions(-) diff --git a/app/scripts/controllers/network/network-controller.ts b/app/scripts/controllers/network/network-controller.ts index 2ab9241de..2cfdadd2e 100644 --- a/app/scripts/controllers/network/network-controller.ts +++ b/app/scripts/controllers/network/network-controller.ts @@ -1,6 +1,6 @@ import { strict as assert } from 'assert'; import EventEmitter from 'events'; -import { ComposedStore, ObservableStore } from '@metamask/obs-store'; +import { ObservableStore } from '@metamask/obs-store'; import log from 'loglevel'; import { createSwappableProxy, @@ -379,6 +379,21 @@ function buildDefaultNetworkConfigurationsState(): NetworkConfigurations { return {}; } +/** + * Builds the default state for the network controller. + * + * @returns The default network controller state. + */ +function buildDefaultState() { + return { + provider: buildDefaultProviderConfigState(), + networkId: buildDefaultNetworkIdState(), + networkStatus: buildDefaultNetworkStatusState(), + networkDetails: buildDefaultNetworkDetailsState(), + networkConfigurations: buildDefaultNetworkConfigurationsState(), + }; +} + /** * Returns whether the given argument is a type that our Infura middleware * recognizes. We can't calculate this inline because the usual type of `type`, @@ -414,43 +429,17 @@ export class NetworkController extends EventEmitter { */ #messenger: NetworkControllerMessenger; - /** - * Observable store containing the provider configuration. - */ - providerStore: ObservableStore; - /** * Observable store containing the provider configuration for the previously * configured network. */ #previousProviderConfig: ProviderConfiguration; - /** - * Observable store containing the network ID for the current network or null - * if there is no current network. - */ - networkIdStore: ObservableStore; - - /** - * Observable store for the network status. - */ - networkStatusStore: ObservableStore; - - /** - * Observable store for details about the network. - */ - networkDetails: ObservableStore; - - /** - * Observable store for network configurations. - */ - networkConfigurationsStore: ObservableStore; - /** * Observable store containing a combination of data from all of the * individual stores. */ - store: ComposedStore; + store: ObservableStore; #provider: SafeEventEmitterProvider | null; @@ -484,35 +473,11 @@ export class NetworkController extends EventEmitter { this.#messenger = messenger; - // create stores - this.providerStore = new ObservableStore( - state.provider || buildDefaultProviderConfigState(), - ); - this.#previousProviderConfig = this.providerStore.getState(); - this.networkIdStore = new ObservableStore(buildDefaultNetworkIdState()); - this.networkStatusStore = new ObservableStore( - buildDefaultNetworkStatusState(), - ); - // We need to keep track of a few details about the current network. - // Ideally we'd merge this.networkStatusStore with this new store, but doing - // so will require a decent sized refactor of how we're accessing network - // state. Currently this is only used for detecting EIP-1559 support but can - // be extended to track other network details. - this.networkDetails = new ObservableStore( - state.networkDetails || buildDefaultNetworkDetailsState(), - ); - - this.networkConfigurationsStore = new ObservableStore( - state.networkConfigurations || buildDefaultNetworkConfigurationsState(), - ); - - this.store = new ComposedStore({ - provider: this.providerStore, - networkId: this.networkIdStore, - networkStatus: this.networkStatusStore, - networkDetails: this.networkDetails, - networkConfigurations: this.networkConfigurationsStore, + this.store = new ObservableStore({ + ...buildDefaultState(), + ...state, }); + this.#previousProviderConfig = this.store.getState().provider; // provider and block tracker this.#provider = null; @@ -543,7 +508,7 @@ export class NetworkController extends EventEmitter { * using the provider to gather details about the network. */ async initializeProvider(): Promise { - const { type, rpcUrl, chainId } = this.providerStore.getState(); + const { type, rpcUrl, chainId } = this.store.getState().provider; this.#configureProvider({ type, rpcUrl, chainId }); await this.lookupNetwork(); } @@ -569,7 +534,7 @@ export class NetworkController extends EventEmitter { * and false otherwise. */ async getEIP1559Compatibility(): Promise { - const { EIPS } = this.networkDetails.getState(); + const { EIPS } = this.store.getState().networkDetails; // NOTE: This isn't necessary anymore because the block cache middleware // already prevents duplicate requests from taking place if (EIPS[1559] !== undefined) { @@ -585,10 +550,14 @@ export class NetworkController extends EventEmitter { } const supportsEIP1559 = await this.#determineEIP1559Compatibility(provider); - this.networkDetails.updateState({ - EIPS: { - ...this.networkDetails.getState().EIPS, - 1559: supportsEIP1559, + const { networkDetails } = this.store.getState(); + this.store.updateState({ + networkDetails: { + ...networkDetails, + EIPS: { + ...networkDetails.EIPS, + 1559: supportsEIP1559, + }, }, }); return supportsEIP1559; @@ -606,7 +575,7 @@ export class NetworkController extends EventEmitter { * blocking requests, or if the network is not Infura-supported. */ async lookupNetwork(): Promise { - const { chainId, type } = this.providerStore.getState(); + const { chainId, type } = this.store.getState().provider; const { provider } = this.getProviderAndBlockTracker(); let networkChanged = false; let networkId: NetworkIdState = null; @@ -692,14 +661,20 @@ export class NetworkController extends EventEmitter { listener, ); - this.networkStatusStore.putState(networkStatus); + this.store.updateState({ + networkStatus, + }); if (networkStatus === NetworkStatus.Available) { - this.networkIdStore.putState(networkId); - this.networkDetails.updateState({ - EIPS: { - ...this.networkDetails.getState().EIPS, - 1559: supportsEIP1559, + const { networkDetails } = this.store.getState(); + this.store.updateState({ + networkId, + networkDetails: { + ...networkDetails, + EIPS: { + ...networkDetails.EIPS, + 1559: supportsEIP1559, + }, }, }); } else { @@ -731,7 +706,7 @@ export class NetworkController extends EventEmitter { */ setActiveNetwork(networkConfigurationId: NetworkConfigurationId): string { const targetNetwork = - this.networkConfigurationsStore.getState()[networkConfigurationId]; + this.store.getState().networkConfigurations[networkConfigurationId]; if (!targetNetwork) { throw new Error( @@ -779,7 +754,7 @@ export class NetworkController extends EventEmitter { * Re-initializes the provider and block tracker for the current network. */ resetConnection(): void { - this.#setProviderConfig(this.providerStore.getState()); + this.#setProviderConfig(this.store.getState().provider); } /** @@ -789,7 +764,9 @@ export class NetworkController extends EventEmitter { */ async rollbackToPreviousProvider() { const config = this.#previousProviderConfig; - this.providerStore.putState(config); + this.store.updateState({ + provider: config, + }); await this.#switchNetwork(config); } @@ -843,21 +820,27 @@ export class NetworkController extends EventEmitter { * Clears the stored network ID. */ #resetNetworkId(): void { - this.networkIdStore.putState(buildDefaultNetworkIdState()); + this.store.updateState({ + networkId: buildDefaultNetworkIdState(), + }); } /** * Resets network status to the default ("unknown"). */ #resetNetworkStatus(): void { - this.networkStatusStore.putState(buildDefaultNetworkStatusState()); + this.store.updateState({ + networkStatus: buildDefaultNetworkStatusState(), + }); } /** * Clears details previously stored for the network. */ #resetNetworkDetails(): void { - this.networkDetails.putState(buildDefaultNetworkDetailsState()); + this.store.updateState({ + networkDetails: buildDefaultNetworkDetailsState(), + }); } /** @@ -867,8 +850,8 @@ export class NetworkController extends EventEmitter { * @param providerConfig - The provider configuration. */ async #setProviderConfig(providerConfig: ProviderConfiguration) { - this.#previousProviderConfig = this.providerStore.getState(); - this.providerStore.putState(providerConfig); + this.#previousProviderConfig = this.store.getState().provider; + this.store.updateState({ provider: providerConfig }); await this.#switchNetwork(providerConfig); } @@ -1105,7 +1088,7 @@ export class NetworkController extends EventEmitter { ); } - const networkConfigurations = this.networkConfigurationsStore.getState(); + const { networkConfigurations } = this.store.getState(); const newNetworkConfiguration = { rpcUrl, chainId, @@ -1120,11 +1103,13 @@ export class NetworkController extends EventEmitter { )?.id; const newNetworkConfigurationId = oldNetworkConfigurationId || uuid(); - this.networkConfigurationsStore.putState({ - ...networkConfigurations, - [newNetworkConfigurationId]: { - ...newNetworkConfiguration, - id: newNetworkConfigurationId, + this.store.updateState({ + networkConfigurations: { + ...networkConfigurations, + [newNetworkConfigurationId]: { + ...newNetworkConfiguration, + id: newNetworkConfigurationId, + }, }, }); @@ -1160,9 +1145,11 @@ export class NetworkController extends EventEmitter { networkConfigurationId: NetworkConfigurationId, ): void { const networkConfigurations = { - ...this.networkConfigurationsStore.getState(), + ...this.store.getState().networkConfigurations, }; delete networkConfigurations[networkConfigurationId]; - this.networkConfigurationsStore.putState(networkConfigurations); + this.store.updateState({ + networkConfigurations, + }); } } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 13d986815..e7c7fcc7e 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -544,7 +544,7 @@ export default class MetamaskController extends EventEmitter { messenger: currencyRateMessenger, state: { ...initState.CurrencyController, - nativeCurrency: this.networkController.providerStore.getState().ticker, + nativeCurrency: this.networkController.store.getState().provider.ticker, }, }); @@ -981,8 +981,16 @@ export default class MetamaskController extends EventEmitter { getNetworkId: () => this.networkController.store.getState().networkId, getNetworkStatus: () => this.networkController.store.getState().networkStatus, - onNetworkStateChange: (listener) => - this.networkController.networkIdStore.subscribe(listener), + onNetworkStateChange: (listener) => { + let previousNetworkId = + this.networkController.store.getState().networkId; + this.networkController.store.subscribe((state) => { + if (previousNetworkId !== state.networkId) { + listener(); + previousNetworkId = state.networkId; + } + }); + }, getCurrentChainId: () => this.networkController.store.getState().provider.chainId, preferencesStore: this.preferencesController.store,