Use string network and chain IDs (#8628)

* convert network id enums to string

* stringify appropriate chain/network ids in tests

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

app/scripts/controllers/incoming-transactions.js

@@ -240,7 +240,7 @@ export default class IncomingTransactionsController {
     return {
       blockNumber: txMeta.blockNumber,
       id: createId(),
-      metamaskNetworkId: currentNetworkID.toString(),
+      metamaskNetworkId: currentNetworkID,
       status,
       time,
       txParams: {

app/scripts/controllers/network/createInfuraClient.js

@@ -33,23 +33,23 @@ function createNetworkAndChainIdMiddleware ({ network }) {
 
   switch (network) {
     case 'mainnet':
-      netId = networkEnums.MAINNET_NETWORK_ID.toString()
+      netId = networkEnums.MAINNET_NETWORK_ID
       chainId = '0x01'
       break
     case 'ropsten':
-      netId = networkEnums.ROPSTEN_NETWORK_ID.toString()
+      netId = networkEnums.ROPSTEN_NETWORK_ID
       chainId = '0x03'
       break
     case 'rinkeby':
-      netId = networkEnums.RINKEBY_NETWORK_ID.toString()
+      netId = networkEnums.RINKEBY_NETWORK_ID
       chainId = '0x04'
       break
     case 'kovan':
-      netId = networkEnums.KOVAN_NETWORK_ID.toString()
+      netId = networkEnums.KOVAN_NETWORK_ID
       chainId = networkEnums.KOVAN_CHAIN_ID
       break
     case 'goerli':
-      netId = networkEnums.GOERLI_NETWORK_ID.toString()
+      netId = networkEnums.GOERLI_NETWORK_ID
       chainId = '0x05'
       break
     default:

app/scripts/controllers/network/enums.js

@@ -5,11 +5,11 @@ export const MAINNET = 'mainnet'
 export const GOERLI = 'goerli'
 export const LOCALHOST = 'localhost'
 
-export const MAINNET_NETWORK_ID = 1
+export const MAINNET_NETWORK_ID = '1'
-export const ROPSTEN_NETWORK_ID = 3
+export const ROPSTEN_NETWORK_ID = '3'
-export const RINKEBY_NETWORK_ID = 4
+export const RINKEBY_NETWORK_ID = '4'
-export const GOERLI_NETWORK_ID = 5
+export const GOERLI_NETWORK_ID = '5'
-export const KOVAN_NETWORK_ID = 42
+export const KOVAN_NETWORK_ID = '42'
 
 export const MAINNET_CHAIN_ID = '0x1'
 export const ROPSTEN_CHAIN_ID = '0x3'

app/scripts/controllers/network/network.js

@@ -100,7 +100,7 @@ export default class NetworkController extends EventEmitter {
     if (!type) {
       return
     }
-    network = networks.networkList[type] && networks.networkList[type].chainId ? networks.networkList[type].chainId : network
+    network = networks.networkList[type]?.chainId || network
     return this.networkStore.putState(network)
   }
 
@@ -211,7 +211,7 @@ export default class NetworkController extends EventEmitter {
     const networkClient = createJsonRpcClient({ rpcUrl })
     // hack to add a 'rpc' network with chainId
     networks.networkList['rpc'] = {
-      chainId: chainId,
+      chainId,
       rpcUrl,
       ticker: ticker || 'ETH',
       nickname,

app/scripts/controllers/preferences.js

@@ -482,7 +482,7 @@ export default class PreferencesController {
    * updates custom RPC details
    *
    * @param {string} url - The RPC url to add to frequentRpcList.
-   * @param {number} chainId - Optional chainId of the selected network.
+   * @param {string} chainId - Optional chainId of the selected network.
    * @param {string} ticker   - Optional ticker symbol of the selected network.
    * @param {string} nickname - Optional nickname of the selected network.
    * @returns {Promise<array>} - Promise resolving to updated frequentRpcList.
@@ -510,7 +510,7 @@ export default class PreferencesController {
    * Adds custom RPC url to state.
    *
    * @param {string} url - The RPC url to add to frequentRpcList.
-   * @param {number} chainId - Optional chainId of the selected network.
+   * @param {string} chainId - Optional chainId of the selected network.
    * @param {string} ticker   - Optional ticker symbol of the selected network.
    * @param {string} nickname - Optional nickname of the selected network.
    * @returns {Promise<array>} - Promise resolving to updated frequentRpcList.

app/scripts/controllers/transactions/index.js

@@ -125,14 +125,19 @@ export default class TransactionController extends EventEmitter {
     this._updatePendingTxsAfterFirstBlock()
   }
 
-  /** @returns {number} - the chainId*/
+  /**
+   * Gets the current chainId in the network store as a number, returning 0 if
+   * the chainId parses to NaN.
+   *
+   * @returns {number} The numerical chainId.
+   */
   getChainId () {
     const networkState = this.networkStore.getState()
-    const getChainId = parseInt(networkState)
+    const integerChainId = parseInt(networkState)
-    if (Number.isNaN(getChainId)) {
+    if (Number.isNaN(integerChainId)) {
       return 0
     } else {
-      return getChainId
+      return integerChainId
     }
   }
 

app/scripts/controllers/transactions/lib/recipient-blocklist-checker.js

@@ -1,4 +1,5 @@
 import blocklist from './recipient-blocklist'
+import { MAINNET_NETWORK_ID } from '../../network/enums'
 
 /**
  * Checks if a specified account on a specified network is blocked
@@ -7,8 +8,7 @@ import blocklist from './recipient-blocklist'
  * @throws {Error} if the account is blocked on mainnet
  */
 export function throwIfAccountIsBlocked (networkId, account) {
-  const mainnetId = 1
+  if (networkId !== MAINNET_NETWORK_ID) {
-  if (networkId !== mainnetId) {
     return
   }
 

app/scripts/lib/ens-ipfs/resolver.js

@@ -62,11 +62,8 @@ function hexValueIsEmpty (value) {
 function getRegistryForChainId (chainId) {
   switch (chainId) {
     case 1:
-      // falls through
     case 3:
-      // falls through
     case 4:
-      // falls through
     case 5:
       // Mainnet, Ropsten, Rinkeby, and Goerli, respectively, use the same address
       return '0x00000000000C2E074eC69A0dFb2997BA6C7d2e1e'

app/scripts/metamask-controller.js

@@ -1854,7 +1854,7 @@ export default class MetamaskController extends EventEmitter {
   /**
    * 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.
-   * @param {number} chainId - The chainId of the selected network.
+   * @param {string} chainId - The chainId of the selected network.
    * @param {string} ticker - The ticker symbol of the selected network.
    * @param {string} nickname - Optional nickname of the selected network.
    * @returns {Promise<String>} - The RPC Target URL confirmed.
@@ -1870,7 +1870,7 @@ export default class MetamaskController extends EventEmitter {
   /**
    * A method for selecting a custom URL for an ethereum RPC provider.
    * @param {string} rpcTarget - A URL for a valid Ethereum RPC API.
-   * @param {number} chainId - The chainId of the selected network.
+   * @param {string} chainId - The chainId of the selected network.
    * @param {string} ticker - The ticker symbol of the selected network.
    * @param {string} nickname - Optional nickname of the selected network.
    * @returns {Promise<String>} - The RPC Target URL confirmed.

test/unit/app/controllers/incoming-transactions-test.js

@@ -393,7 +393,7 @@ describe('IncomingTransactionsController', function () {
       assert.deepEqual(result, {
         someKey: 'someValue',
         address: '0xfakeaddress',
-        currentNetworkID: 3,
+        currentNetworkID: '3',
       })
     })
   })

test/unit/app/controllers/metamask-controller-test.js

@@ -64,7 +64,7 @@ const MetaMaskController = proxyquire('../../../../app/scripts/metamask-controll
   './lib/createLoggerMiddleware': { default: createLoggerMiddlewareMock },
 }).default
 
-const currentNetworkId = 42
+const currentNetworkId = '42'
 const DEFAULT_LABEL = 'Account 1'
 const DEFAULT_LABEL_2 = 'Account 2'
 const TEST_SEED = 'debris dizzy just program just float decrease vacant alarm reduce speak stadium'
@@ -605,7 +605,7 @@ describe('MetaMaskController', function () {
       metamaskController.txController.txStateManager._saveTxList([
         createTxMeta({ id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: { from: '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc' } }),
         createTxMeta({ id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: { from: '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc' } }),
-        createTxMeta({ id: 2, status: 'rejected', metamaskNetworkId: 32 }),
+        createTxMeta({ id: 2, status: 'rejected', metamaskNetworkId: '32' }),
         createTxMeta({ id: 3, status: 'submitted', metamaskNetworkId: currentNetworkId, txParams: { from: '0xB09d8505E1F4EF1CeA089D47094f5DD3464083d4' } }),
       ])
 

test/unit/app/controllers/preferences-controller-test.js

@@ -539,27 +539,27 @@ describe('preferences controller', function () {
   describe('#updateRpc', function () {
     it('should update the rpcDetails properly', function () {
       preferencesController.store.updateState({ frequentRpcListDetail: [{}, { rpcUrl: 'test' }, {}] })
-      preferencesController.updateRpc({ rpcUrl: 'test', chainId: 1 })
+      preferencesController.updateRpc({ rpcUrl: 'test', chainId: '1' })
-      preferencesController.updateRpc({ rpcUrl: 'test/1', chainId: 1 })
+      preferencesController.updateRpc({ rpcUrl: 'test/1', chainId: '1' })
-      preferencesController.updateRpc({ rpcUrl: 'test/2', chainId: 1 })
+      preferencesController.updateRpc({ rpcUrl: 'test/2', chainId: '1' })
-      preferencesController.updateRpc({ rpcUrl: 'test/3', chainId: 1 })
+      preferencesController.updateRpc({ rpcUrl: 'test/3', chainId: '1' })
       const list = preferencesController.getFrequentRpcListDetail()
-      assert.deepEqual(list[1], { rpcUrl: 'test', chainId: 1 })
+      assert.deepEqual(list[1], { rpcUrl: 'test', chainId: '1' })
     })
   })
 
   describe('on updateFrequentRpcList', function () {
     it('should add custom RPC url to state', function () {
-      preferencesController.addToFrequentRpcList('rpc_url', 1)
+      preferencesController.addToFrequentRpcList('rpc_url', '1')
-      preferencesController.addToFrequentRpcList('http://localhost:8545', 1)
+      preferencesController.addToFrequentRpcList('http://localhost:8545', '1')
-      assert.deepEqual(preferencesController.store.getState().frequentRpcListDetail, [{ rpcUrl: 'rpc_url', chainId: 1, ticker: 'ETH', nickname: '', rpcPrefs: {} }])
+      assert.deepEqual(preferencesController.store.getState().frequentRpcListDetail, [{ rpcUrl: 'rpc_url', chainId: '1', ticker: 'ETH', nickname: '', rpcPrefs: {} }])
-      preferencesController.addToFrequentRpcList('rpc_url', 1)
+      preferencesController.addToFrequentRpcList('rpc_url', '1')
-      assert.deepEqual(preferencesController.store.getState().frequentRpcListDetail, [{ rpcUrl: 'rpc_url', chainId: 1, ticker: 'ETH', nickname: '', rpcPrefs: {} }])
+      assert.deepEqual(preferencesController.store.getState().frequentRpcListDetail, [{ rpcUrl: 'rpc_url', chainId: '1', ticker: 'ETH', nickname: '', rpcPrefs: {} }])
     })
 
     it('should remove custom RPC url from state', function () {
-      preferencesController.addToFrequentRpcList('rpc_url', 1)
+      preferencesController.addToFrequentRpcList('rpc_url', '1')
-      assert.deepEqual(preferencesController.store.getState().frequentRpcListDetail, [{ rpcUrl: 'rpc_url', chainId: 1, ticker: 'ETH', nickname: '', rpcPrefs: {} }])
+      assert.deepEqual(preferencesController.store.getState().frequentRpcListDetail, [{ rpcUrl: 'rpc_url', chainId: '1', ticker: 'ETH', nickname: '', rpcPrefs: {} }])
       preferencesController.removeFromFrequentRpcList('other_rpc_url')
       preferencesController.removeFromFrequentRpcList('http://localhost:8545')
       preferencesController.removeFromFrequentRpcList('rpc_url')

test/unit/app/controllers/transactions/recipient-blocklist-checker-test.js

@@ -30,7 +30,7 @@ describe('Recipient Blocklist Checker', function () {
     it('fails on mainnet', function () {
       for (const account of publicAccounts) {
         assert.throws(
-          () => throwIfAccountIsBlocked(1, account),
+          () => throwIfAccountIsBlocked('1', account),
           { message: 'Recipient is a public account' },
         )
       }
@@ -38,14 +38,14 @@ describe('Recipient Blocklist Checker', function () {
 
     it('fails for public account - uppercase', function () {
       assert.throws(
-        () => throwIfAccountIsBlocked(1, '0X0D1D4E623D10F9FBA5DB95830F7D3839406C6AF2'),
+        () => throwIfAccountIsBlocked('1', '0X0D1D4E623D10F9FBA5DB95830F7D3839406C6AF2'),
         { message: 'Recipient is a public account' },
       )
     })
 
     it('fails for public account - lowercase', function () {
       assert.throws(
-        () => throwIfAccountIsBlocked(1, '0x0d1d4e623d10f9fba5db95830f7d3839406c6af2'),
+        () => throwIfAccountIsBlocked('1', '0x0d1d4e623d10f9fba5db95830f7d3839406c6af2'),
         { message: 'Recipient is a public account' },
       )
     })

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

@@ -18,7 +18,7 @@ import {
 import { createTestProviderTools, getTestAccounts } from '../../../../stub/provider'
 
 const noop = () => true
-const currentNetworkId = 42
+const currentNetworkId = '42'
 
 describe('Transaction Controller', function () {
   let txController, provider, providerResultStub, fromAccount
@@ -199,7 +199,7 @@ describe('Transaction Controller', function () {
     })
 
     it('should fail if recipient is public', async function () {
-      txController.networkStore = new ObservableStore(1)
+      txController.networkStore = new ObservableStore('1')
       await assert.rejects(
         () => txController.addUnapprovedTransaction({ from: selectedAddress, to: '0x0d1d4e623D10F9FBA5Db95830F7d3839406C6AF2' }),
         { message: 'Recipient is a public account' },
@@ -324,7 +324,7 @@ describe('Transaction Controller', function () {
       txController.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop)
       const rawTx = await txController.signTransaction('1')
       const ethTx = new EthTx(ethUtil.toBuffer(rawTx))
-      assert.equal(ethTx.getChainId(), currentNetworkId)
+      assert.equal(ethTx.getChainId(), parseInt(currentNetworkId))
     })
   })
 

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

@@ -3,7 +3,7 @@ import txHelper from '../../../../../ui/lib/tx-helper'
 
 describe('txHelper', function () {
   it('always shows the oldest tx first', function () {
-    const metamaskNetworkId = 1
+    const metamaskNetworkId = '1'
     const txs = {
       a: { metamaskNetworkId, time: 3 },
       b: { metamaskNetworkId, time: 1 },

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

@@ -7,8 +7,8 @@ const noop = () => true
 
 describe('TransactionStateManager', function () {
   let txStateManager
-  const currentNetworkId = 42
+  const currentNetworkId = '42'
-  const otherNetworkId = 2
+  const otherNetworkId = '2'
 
   beforeEach(function () {
     txStateManager = new TxStateManager({

test/unit/ui/app/actions.spec.js

@@ -21,7 +21,7 @@ describe('Actions', function () {
 
   const noop = () => {}
 
-  const currentNetworkId = 42
+  const currentNetworkId = '42'
 
   let background, metamaskController
 

test/unit/ui/etherscan-prefix-for-network.spec.js

@@ -1,30 +1,30 @@
 import assert from 'assert'
-import etherscanNetworkPrefix from '../../../ui/lib/etherscan-prefix-for-network'
+import { getEtherscanNetworkPrefix } from '../../../ui/lib/etherscan-prefix-for-network'
 
 describe('Etherscan Network Prefix', function () {
 
-  it('returns empy string as default value', function () {
+  it('returns empty string as default value', function () {
-    assert.equal(etherscanNetworkPrefix(), '')
+    assert.equal(getEtherscanNetworkPrefix(), '')
   })
 
   it('returns empty string as a prefix for networkId of 1', function () {
-    assert.equal(etherscanNetworkPrefix(1), '')
+    assert.equal(getEtherscanNetworkPrefix('1'), '')
   })
 
   it('returns ropsten as prefix for networkId of 3', function () {
-    assert.equal(etherscanNetworkPrefix(3), 'ropsten.')
+    assert.equal(getEtherscanNetworkPrefix('3'), 'ropsten.')
   })
 
   it('returns rinkeby as prefix for networkId of 4', function () {
-    assert.equal(etherscanNetworkPrefix(4), 'rinkeby.')
+    assert.equal(getEtherscanNetworkPrefix('4'), 'rinkeby.')
   })
 
   it('returs kovan as prefix for networkId of 42', function () {
-    assert.equal(etherscanNetworkPrefix(42), 'kovan.')
+    assert.equal(getEtherscanNetworkPrefix('42'), 'kovan.')
   })
 
   it('returs goerli as prefix for networkId of 5', function () {
-    assert.equal(etherscanNetworkPrefix(5), 'goerli.')
+    assert.equal(getEtherscanNetworkPrefix('5'), 'goerli.')
   })
 
 })

ui/app/components/app/transaction-activity-log/transaction-activity-log.component.js

@@ -5,7 +5,7 @@ import { getEthConversionFromWeiHex, getValueFromWeiHex } from '../../../helpers
 import { formatDate } from '../../../helpers/utils/util'
 import TransactionActivityLogIcon from './transaction-activity-log-icon'
 import { CONFIRMED_STATUS } from './transaction-activity-log.constants'
-import prefixForNetwork from '../../../../lib/etherscan-prefix-for-network'
+import { getEtherscanNetworkPrefix } from '../../../../lib/etherscan-prefix-for-network'
 
 export default class TransactionActivityLog extends PureComponent {
   static contextTypes = {
@@ -30,7 +30,7 @@ export default class TransactionActivityLog extends PureComponent {
     const { primaryTransaction } = this.props
     const { metamaskNetworkId } = primaryTransaction
 
-    const prefix = prefixForNetwork(metamaskNetworkId)
+    const prefix = getEtherscanNetworkPrefix(metamaskNetworkId)
     const etherscanUrl = `https://${prefix}etherscan.io/tx/${hash}`
 
     global.platform.openTab({ url: etherscanUrl })

ui/app/helpers/utils/transactions.util.js

@@ -7,7 +7,7 @@ import {
   TRANSACTION_STATUS_CONFIRMED,
 } from '../../../../app/scripts/controllers/transactions/enums'
 import { MESSAGE_TYPE } from '../../../../app/scripts/lib/enums'
-import prefixForNetwork from '../../../lib/etherscan-prefix-for-network'
+import { getEtherscanNetworkPrefix } from '../../../lib/etherscan-prefix-for-network'
 import fetchWithCache from './fetch-with-cache'
 
 import {
@@ -239,6 +239,6 @@ export function getBlockExplorerUrlForTx (networkId, hash, rpcPrefs = {}) {
   if (rpcPrefs.blockExplorerUrl) {
     return `${rpcPrefs.blockExplorerUrl.replace(/\/+$/, '')}/tx/${hash}`
   }
-  const prefix = prefixForNetwork(networkId)
+  const prefix = getEtherscanNetworkPrefix(networkId)
   return `https://${prefix}etherscan.io/tx/${hash}`
 }

ui/app/helpers/utils/transactions.util.test.js

@@ -60,19 +60,19 @@ describe('Transactions utils', function () {
       const tests = [
         {
           expected: 'https://etherscan.io/tx/0xabcd',
-          networkId: 1,
+          networkId: '1',
           hash: '0xabcd',
         },
         {
           expected: 'https://ropsten.etherscan.io/tx/0xdef0',
-          networkId: 3,
+          networkId: '3',
           hash: '0xdef0',
           rpcPrefs: {},
         },
         {
           // test handling of `blockExplorerUrl` for a custom RPC
           expected: 'https://block.explorer/tx/0xabcd',
-          networkId: 31,
+          networkId: '31',
           hash: '0xabcd',
           rpcPrefs: {
             blockExplorerUrl: 'https://block.explorer',
@@ -81,7 +81,7 @@ describe('Transactions utils', function () {
         {
           // test handling of trailing `/` in `blockExplorerUrl` for a custom RPC
           expected: 'https://another.block.explorer/tx/0xdef0',
-          networkId: 33,
+          networkId: '33',
           hash: '0xdef0',
           rpcPrefs: {
             blockExplorerUrl: 'https://another.block.explorer/',

ui/lib/etherscan-prefix-for-network.js

@@ -1,24 +1,22 @@
-export default function etherscanNetworkPrefix (network) {
+import * as networkEnums from '../../app/scripts/controllers/network/enums'
-  const net = parseInt(network)
+
-  let prefix
+/**
-  switch (net) {
+ * Gets the etherscan.io URL prefix for a given network ID.
-    case 1: // main net
+ *
-      prefix = ''
+ * @param {string} networkId - The network ID to get the prefix for.
-      break
+ * @returns {string} The etherscan.io URL prefix for the given network ID.
-    case 3: // ropsten test net
+ */
-      prefix = 'ropsten.'
+export function getEtherscanNetworkPrefix (networkId) {
-      break
+  switch (networkId) {
-    case 4: // rinkeby test net
+    case networkEnums.ROPSTEN_NETWORK_ID:
-      prefix = 'rinkeby.'
+      return 'ropsten.'
-      break
+    case networkEnums.RINKEBY_NETWORK_ID:
-    case 42: // kovan test net
+      return 'rinkeby.'
-      prefix = 'kovan.'
+    case networkEnums.KOVAN_NETWORK_ID:
-      break
+      return 'kovan.'
-    case 5: // goerli test net
+    case networkEnums.GOERLI_NETWORK_ID:
-      prefix = 'goerli.'
+      return 'goerli.'
-      break
+    default: // also covers mainnet
-    default:
+      return ''
-      prefix = ''
   }
-  return prefix
 }