Use networkVersion for network state; chainId for signing transactions (#9487)

app/scripts/controllers/network/network.js

@@ -124,9 +124,8 @@ export default class NetworkController extends EventEmitter {
       return
     }
 
-    const { type, chainId: configChainId } = this.getProviderConfig()
-    const chainId = NETWORK_TYPE_TO_ID_MAP[type]?.chainId || configChainId
-
+    const { type } = this.getProviderConfig()
+    const chainId = this.getCurrentChainId()
     if (!chainId) {
       log.warn('NetworkController - lookupNetwork aborted due to missing chainId')
       this.setNetworkState('loading')
@@ -136,7 +135,7 @@ export default class NetworkController extends EventEmitter {
     // Ping the RPC endpoint so we can confirm that it works
     const ethQuery = new EthQuery(this._provider)
     const initialNetwork = this.getNetworkState()
-    ethQuery.sendAsync({ method: 'net_version' }, (err, _networkVersion) => {
+    ethQuery.sendAsync({ method: 'net_version' }, (err, networkVersion) => {
       const currentNetwork = this.getNetworkState()
       if (initialNetwork === currentNetwork) {
         if (err) {
@@ -144,12 +143,20 @@ export default class NetworkController extends EventEmitter {
           return
         }
 
-        // Now we set the network state to the chainId computed earlier
-        this.setNetworkState(chainId)
+        this.setNetworkState((
+          type === 'rpc'
+            ? chainId
+            : networkVersion
+        ))
       }
     })
   }
 
+  getCurrentChainId () {
+    const { type, chainId: configChainId } = this.getProviderConfig()
+    return NETWORK_TYPE_TO_ID_MAP[type]?.chainId || configChainId
+  }
+
   setRpcTarget (rpcUrl, chainId, ticker = 'ETH', nickname = '', rpcPrefs) {
     this.setProviderConfig({
       type: 'rpc',

app/scripts/controllers/transactions/index.js

@@ -69,6 +69,7 @@ export default class TransactionController extends EventEmitter {
   constructor (opts) {
     super()
     this.networkStore = opts.networkStore || new ObservableStore({})
+    this._getCurrentChainId = opts.getCurrentChainId
     this.preferencesStore = opts.preferencesStore || new ObservableStore({})
     this.provider = opts.provider
     this.getPermittedAccounts = opts.getPermittedAccounts
@@ -132,9 +133,9 @@ export default class TransactionController extends EventEmitter {
    */
   getChainId () {
     const networkState = this.networkStore.getState()
-    // eslint-disable-next-line radix
-    const integerChainId = parseInt(networkState)
-    if (Number.isNaN(integerChainId)) {
+    const chainId = this._getCurrentChainId()
+    const integerChainId = parseInt(chainId, 16)
+    if (networkState === 'loading' || Number.isNaN(integerChainId)) {
       return 0
     }
     return integerChainId

app/scripts/metamask-controller.js

@@ -231,6 +231,7 @@ export default class MetamaskController extends EventEmitter {
       initState: initState.TransactionController || initState.TransactionManager,
       getPermittedAccounts: this.permissionsController.getAccounts.bind(this.permissionsController),
       networkStore: this.networkController.networkStore,
+      getCurrentChainId: this.networkController.getCurrentChainId.bind(this.networkController),
       preferencesStore: this.preferencesController.store,
       txHistoryLimit: 40,
       getNetwork: this.networkController.getNetworkState.bind(this),
@@ -382,6 +383,7 @@ export default class MetamaskController extends EventEmitter {
   createPublicConfigStore () {
     // subset of state for metamask inpage provider
     const publicConfigStore = new ObservableStore()
+    const { networkController } = this
 
     // setup memStore subscription hooks
     this.on('update', updatePublicConfigStore)
@@ -392,16 +394,17 @@ export default class MetamaskController extends EventEmitter {
     }
 
     function updatePublicConfigStore (memState) {
+      const chainId = networkController.getCurrentChainId()
       if (memState.network !== 'loading') {
-        publicConfigStore.putState(selectPublicState(memState))
+        publicConfigStore.putState(selectPublicState(chainId, memState))
       }
     }
 
-    function selectPublicState ({ isUnlocked, network }) {
+    function selectPublicState (chainId, { isUnlocked, network }) {
       return {
         isUnlocked,
-        chainId: network,
-        networkVersion: Number.parseInt(network, 16).toString(),
+        chainId,
+        networkVersion: network,
       }
     }
     return publicConfigStore

test/unit/app/controllers/transactions/tx-controller-test.js

@@ -19,6 +19,7 @@ import { createTestProviderTools, getTestAccounts } from '../../../../stub/provi
 
 const noop = () => true
 const currentNetworkId = '42'
+const currentChainId = '0x2a'
 
 describe('Transaction Controller', function () {
   let txController, provider, providerResultStub, fromAccount
@@ -48,6 +49,7 @@ describe('Transaction Controller', function () {
         resolve()
       }),
       getPermittedAccounts: () => undefined,
+      getCurrentChainId: () => currentChainId,
     })
     txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop })
   })
@@ -334,7 +336,7 @@ describe('Transaction Controller', function () {
 
   describe('#getChainId', function () {
     it('returns 0 when the chainId is NaN', function () {
-      txController.networkStore = new ObservableStore(NaN)
+      txController.networkStore = new ObservableStore('loading')
       assert.equal(txController.getChainId(), 0)
     })
   })