Refactor how network state is passed to transaction controller (#16922)

The network state is now passed to the TransactionController via a
getter function and a subscription function, instead of passing one of
the network controller stores directly.

This way of passing the state makes further refactoring easier, as we
don't have to change the input when the store is changed or replaced.
It's also more aligned with our conventions today.

This change was made as part of a larger refactor of the network
controller, as part of the effort to merge the mobile and extension
network controllers.

app/scripts/controllers/transactions/index.js

@@ -113,7 +113,8 @@ const METRICS_STATUS_FAILED = 'failed on-chain';
  *
  * @param {object} opts
  * @param {object} opts.initState - initial transaction list default is an empty array
- * @param {object} opts.networkStore - an observable store for network number
+ * @param {Function} opts.getNetworkState - Get the current network state.
+ * @param {Function} opts.onNetworkStateChange - Subscribe to network state change events.
  * @param {object} opts.blockTracker - An instance of eth-blocktracker
  * @param {object} opts.provider - A network provider.
  * @param {Function} opts.signTransaction - function the signs an @ethereumjs/tx
@@ -126,7 +127,7 @@ const METRICS_STATUS_FAILED = 'failed on-chain';
 export default class TransactionController extends EventEmitter {
   constructor(opts) {
     super();
-    this.networkStore = opts.networkStore || new ObservableStore({});
+    this.getNetworkState = opts.getNetworkState;
     this._getCurrentChainId = opts.getCurrentChainId;
     this.getProviderConfig = opts.getProviderConfig;
     this._getCurrentNetworkEIP1559Compatibility =
@@ -163,7 +164,7 @@ export default class TransactionController extends EventEmitter {
     this.txStateManager = new TransactionStateManager({
       initState: opts.initState,
       txHistoryLimit: opts.txHistoryLimit,
-      getNetwork: this.getNetwork.bind(this),
+      getNetworkState: this.getNetworkState,
       getCurrentChainId: opts.getCurrentChainId,
     });
 
@@ -205,7 +206,7 @@ export default class TransactionController extends EventEmitter {
     // memstore is computed from a few different stores
     this._updateMemstore();
     this.txStateManager.store.subscribe(() => this._updateMemstore());
-    this.networkStore.subscribe(() => {
+    opts.onNetworkStateChange(() => {
       this._onBootCleanUp();
       this._updateMemstore();
     });
@@ -222,7 +223,7 @@ export default class TransactionController extends EventEmitter {
    * @returns {number} The numerical chainId.
    */
   getChainId() {
-    const networkState = this.networkStore.getState();
+    const networkState = this.getNetworkState();
     const chainId = this._getCurrentChainId();
     const integerChainId = parseInt(chainId, 16);
     if (networkState === 'loading' || Number.isNaN(integerChainId)) {
@@ -273,7 +274,7 @@ export default class TransactionController extends EventEmitter {
     // name, chainId and networkId properties. This is done using the
     // `forCustomChain` static method on the Common class.
     const chainId = parseInt(this._getCurrentChainId(), 16);
-    const networkId = this.networkStore.getState();
+    const networkId = this.getNetworkState();
 
     const customChainParams = {
       name,
@@ -1764,9 +1765,6 @@ export default class TransactionController extends EventEmitter {
     /** @returns {object} the state in transaction controller */
     this.getState = () => this.memStore.getState();
 
-    /** @returns {string|number} the network number stored in networkStore */
-    this.getNetwork = () => this.networkStore.getState();
-
     /** @returns {string} the user selected address */
     this.getSelectedAddress = () =>
       this.preferencesStore.getState().selectedAddress;

app/scripts/controllers/transactions/index.test.js

@@ -41,7 +41,12 @@ const VALID_ADDRESS = '0x0000000000000000000000000000000000000000';
 const VALID_ADDRESS_TWO = '0x0000000000000000000000000000000000000001';
 
 describe('Transaction Controller', function () {
-  let txController, provider, providerResultStub, fromAccount, fragmentExists;
+  let txController,
+    provider,
+    providerResultStub,
+    fromAccount,
+    fragmentExists,
+    networkStore;
 
   beforeEach(function () {
     fragmentExists = false;
@@ -57,6 +62,8 @@ describe('Transaction Controller', function () {
       chainId: currentNetworkId,
     }).provider;
 
+    networkStore = new ObservableStore(currentNetworkId);
+
     fromAccount = getTestAccounts()[0];
     const blockTrackerStub = new EventEmitter();
     blockTrackerStub.getCurrentBlock = noop;
@@ -66,7 +73,8 @@ describe('Transaction Controller', function () {
       getGasPrice() {
         return '0xee6b2800';
       },
-      networkStore: new ObservableStore(currentNetworkId),
+      getNetworkState: () => networkStore.getState(),
+      onNetworkStateChange: (listener) => networkStore.subscribe(listener),
       getCurrentNetworkEIP1559Compatibility: () => Promise.resolve(false),
       getCurrentAccountEIP1559Compatibility: () => false,
       txHistoryLimit: 10,
@@ -455,7 +463,7 @@ describe('Transaction Controller', function () {
     });
 
     it('should fail if netId is loading', async function () {
-      txController.networkStore = new ObservableStore('loading');
+      networkStore.putState('loading');
       await assert.rejects(
         () =>
           txController.addUnapprovedTransaction({
@@ -1067,7 +1075,7 @@ describe('Transaction Controller', function () {
 
   describe('#getChainId', function () {
     it('returns 0 when the chainId is NaN', function () {
-      txController.networkStore = new ObservableStore('loading');
+      networkStore.putState('loading');
       assert.equal(txController.getChainId(), 0);
     });
   });

app/scripts/controllers/transactions/tx-state-manager.js

@@ -54,10 +54,15 @@ export const ERROR_SUBMITTING =
  *  transactions list keyed by id
  * @param {number} [opts.txHistoryLimit] - limit for how many finished
  *  transactions can hang around in state
- * @param {Function} opts.getNetwork - return network number
+ * @param {Function} opts.getNetworkState - Get the current network state.
  */
 export default class TransactionStateManager extends EventEmitter {
-  constructor({ initState, txHistoryLimit, getNetwork, getCurrentChainId }) {
+  constructor({
+    initState,
+    txHistoryLimit,
+    getNetworkState,
+    getCurrentChainId,
+  }) {
     super();
 
     this.store = new ObservableStore({
@@ -65,7 +70,7 @@ export default class TransactionStateManager extends EventEmitter {
       ...initState,
     });
     this.txHistoryLimit = txHistoryLimit;
-    this.getNetwork = getNetwork;
+    this.getNetworkState = getNetworkState;
     this.getCurrentChainId = getCurrentChainId;
   }
 
@@ -81,7 +86,7 @@ export default class TransactionStateManager extends EventEmitter {
    * @returns {TransactionMeta} the default txMeta object
    */
   generateTxMeta(opts = {}) {
-    const netId = this.getNetwork();
+    const netId = this.getNetworkState();
     const chainId = this.getCurrentChainId();
     if (netId === 'loading') {
       throw new Error('MetaMask is having trouble connecting to the network');
@@ -144,7 +149,7 @@ export default class TransactionStateManager extends EventEmitter {
    */
   getUnapprovedTxList() {
     const chainId = this.getCurrentChainId();
-    const network = this.getNetwork();
+    const network = this.getNetworkState();
     return pickBy(
       this.store.getState().transactions,
       (transaction) =>
@@ -408,7 +413,7 @@ export default class TransactionStateManager extends EventEmitter {
     limit,
   } = {}) {
     const chainId = this.getCurrentChainId();
-    const network = this.getNetwork();
+    const network = this.getNetworkState();
     // searchCriteria is an object that might have values that aren't predicate
     // methods. When providing any other value type (string, number, etc), we
     // consider this shorthand for "check the value at key for strict equality
@@ -600,7 +605,7 @@ export default class TransactionStateManager extends EventEmitter {
   wipeTransactions(address) {
     // network only tx
     const { transactions } = this.store.getState();
-    const network = this.getNetwork();
+    const network = this.getNetworkState();
     const chainId = this.getCurrentChainId();
 
     // Update state

app/scripts/controllers/transactions/tx-state-manager.test.js

@@ -54,7 +54,7 @@ describe('TransactionStateManager', function () {
         transactions: {},
       },
       txHistoryLimit: 10,
-      getNetwork: () => currentNetworkId,
+      getNetworkState: () => currentNetworkId,
       getCurrentChainId: () => currentChainId,
     });
   });
@@ -181,7 +181,7 @@ describe('TransactionStateManager', function () {
             [confirmedTx.id]: confirmedTx,
           },
         },
-        getNetwork: () => currentNetworkId,
+        getNetworkState: () => currentNetworkId,
         getCurrentChainId: () => currentChainId,
       });
 
@@ -246,7 +246,7 @@ describe('TransactionStateManager', function () {
             [confirmedTx3.id]: confirmedTx3,
           },
         },
-        getNetwork: () => currentNetworkId,
+        getNetworkState: () => currentNetworkId,
         getCurrentChainId: () => currentChainId,
       });
 
@@ -355,7 +355,7 @@ describe('TransactionStateManager', function () {
             [failedTx3Dupe.id]: failedTx3Dupe,
           },
         },
-        getNetwork: () => currentNetworkId,
+        getNetworkState: () => currentNetworkId,
         getCurrentChainId: () => currentChainId,
       });
 

app/scripts/metamask-controller.js

@@ -838,7 +838,9 @@ export default class MetamaskController extends EventEmitter {
         ),
       getCurrentAccountEIP1559Compatibility:
         this.getCurrentAccountEIP1559Compatibility.bind(this),
-      networkStore: this.networkController.networkStore,
+      getNetworkState: () => this.networkController.networkStore.getState(),
+      onNetworkStateChange: (listener) =>
+        this.networkController.networkStore.subscribe(listener),
       getCurrentChainId: this.networkController.getCurrentChainId.bind(
         this.networkController,
       ),

app/scripts/metamask-controller.test.js

@@ -709,7 +709,7 @@ describe('MetaMaskController', function () {
       );
       const getNetworkstub = sinon.stub(
         metamaskController.txController.txStateManager,
-        'getNetwork',
+        'getNetworkState',
       );
 
       selectedAddressStub.returns('0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc');