Update migration 048 (#9560)

* Delete CachedBalancesController.cachedBalances

* Migrate provider to Rinkeby instead of deleting it

* Convert hex transaction metamaskNetworkId values to decimal

* Don't migrate provider state in e2e tests

* Don't kick custom RPC users to Rinkeby unnecessarily

app/scripts/migrations/048.js

@@ -4,10 +4,12 @@ const version = 48
 
 /**
  * 1.  Delete NetworkController.settings
- * 2a. Delete NetworkController.provider if set to type 'rpc' or 'localhost'.
- *     It will be re-set to Mainnet on background initialization.
+ * 2a. Migrate NetworkController.provider to Rinkeby if set to type 'rpc' or
+ *     'localhost'.
  * 2b. Re-key provider.rpcTarget to provider.rpcUrl
  * 3.  Add localhost network to frequentRpcListDetail.
+ * 4.  Delete CachedBalancesController.cachedBalances
+ * 5.  Convert transactions metamaskNetworkId to decimal if they are hex
  */
 export default {
   version,
@@ -20,14 +22,30 @@ export default {
   },
 }
 
+const hexRegEx = (/^0x[0-9a-f]+$/ui)
+const chainIdRegEx = (/^0x[1-9a-f]+[0-9a-f]*$/ui)
+
 function transformState (state = {}) {
   // 1. Delete NetworkController.settings
   delete state.NetworkController?.settings
 
-  // 2. Delete NetworkController.provider or rename rpcTarget key
-  const providerType = state.NetworkController?.provider?.type
-  if (providerType === 'rpc' || providerType === 'localhost') {
-    delete state.NetworkController.provider
+  // 2. Migrate NetworkController.provider to Rinkeby or rename rpcTarget key
+  const provider = state.NetworkController?.provider || {}
+  const isCustomRpcWithInvalidChainId = (
+    provider.type === 'rpc' && (
+      typeof provider.chainId !== 'string' ||
+      !chainIdRegEx.test(provider.chainId)
+    )
+  )
+  if (isCustomRpcWithInvalidChainId || provider.type === 'localhost') {
+    state.NetworkController.provider = {
+      type: 'rinkeby',
+      rpcUrl: '',
+      chainId: '0x4',
+      nickname: '',
+      rpcPrefs: {},
+      ticker: 'ETH',
+    }
   } else if (state.NetworkController?.provider) {
     if ('rpcTarget' in state.NetworkController.provider) {
       const rpcUrl = state.NetworkController.provider.rpcTarget
@@ -51,5 +69,23 @@ function transformState (state = {}) {
     rpcPrefs: {},
   })
 
+  // 4.  Delete CachedBalancesController.cachedBalances
+  delete state.CachedBalancesController?.cachedBalances
+
+  // 5.  Convert transactions metamaskNetworkId to decimal if they are hex
+  const transactions = state.TransactionController?.transactions
+  if (Array.isArray(transactions)) {
+    transactions.forEach((transaction) => {
+      const metamaskNetworkId = transaction?.metamaskNetworkId
+      if (
+        typeof metamaskNetworkId === 'string' &&
+        hexRegEx.test(metamaskNetworkId)
+      ) {
+        transaction.metamaskNetworkId = parseInt(metamaskNetworkId, 16)
+          .toString(10)
+      }
+    })
+  }
+
   return state
 }

test/unit/migrations/048-test.js

@@ -59,13 +59,14 @@ describe('migration #48', function () {
     })
   })
 
-  it('should delete NetworkController.provider if the type is "rpc"', async function () {
+  it('should migrate NetworkController.provider to Rinkeby if the type is "rpc" and the chainId is invalid (1)', async function () {
     const oldStorage = {
       meta: {},
       data: {
         NetworkController: {
           provider: {
             type: 'rpc',
+            chainId: 'foo',
             fizz: 'buzz',
           },
           foo: 'bar',
@@ -78,13 +79,86 @@ describe('migration #48', function () {
     assert.deepEqual(newStorage.data, {
       ...expectedPreferencesState,
       NetworkController: {
+        provider: {
+          type: 'rinkeby',
+          rpcUrl: '',
+          chainId: '0x4',
+          nickname: '',
+          rpcPrefs: {},
+          ticker: 'ETH',
+        },
+        foo: 'bar',
+      },
+      foo: 'bar',
+    })
+  })
+
+  it('should migrate NetworkController.provider to Rinkeby if the type is "rpc" and the chainId is invalid (2)', async function () {
+    const oldStorage = {
+      meta: {},
+      data: {
+        NetworkController: {
+          provider: {
+            type: 'rpc',
+            chainId: '0x01',
+            fizz: 'buzz',
+          },
+          foo: 'bar',
+        },
+        foo: 'bar',
+      },
+    }
+
+    const newStorage = await migration48.migrate(oldStorage)
+    assert.deepEqual(newStorage.data, {
+      ...expectedPreferencesState,
+      NetworkController: {
+        provider: {
+          type: 'rinkeby',
+          rpcUrl: '',
+          chainId: '0x4',
+          nickname: '',
+          rpcPrefs: {},
+          ticker: 'ETH',
+        },
+        foo: 'bar',
+      },
+      foo: 'bar',
+    })
+  })
+
+  it('should not migrate NetworkController.provider to Rinkeby if the type is "rpc" and the chainId is valid', async function () {
+    const oldStorage = {
+      meta: {},
+      data: {
+        NetworkController: {
+          provider: {
+            type: 'rpc',
+            chainId: '0x1',
+            fizz: 'buzz',
+          },
+          foo: 'bar',
+        },
+        foo: 'bar',
+      },
+    }
+
+    const newStorage = await migration48.migrate(oldStorage)
+    assert.deepEqual(newStorage.data, {
+      ...expectedPreferencesState,
+      NetworkController: {
+        provider: {
+          type: 'rpc',
+          chainId: '0x1',
+          fizz: 'buzz',
+        },
         foo: 'bar',
       },
       foo: 'bar',
     })
   })
 
-  it('should delete NetworkController.provider if the type is "localhost"', async function () {
+  it('should migrate NetworkController.provider to Rinkeby if the type is "localhost"', async function () {
     const oldStorage = {
       meta: {},
       data: {
@@ -103,6 +177,14 @@ describe('migration #48', function () {
     assert.deepEqual(newStorage.data, {
       ...expectedPreferencesState,
       NetworkController: {
+        provider: {
+          type: 'rinkeby',
+          rpcUrl: '',
+          chainId: '0x4',
+          nickname: '',
+          rpcPrefs: {},
+          ticker: 'ETH',
+        },
         foo: 'bar',
       },
       foo: 'bar',
@@ -189,4 +271,81 @@ describe('migration #48', function () {
       foo: 'bar',
     })
   })
+
+  it('should delete CachedBalancesController.cachedBalances', async function () {
+    const oldStorage = {
+      meta: {},
+      data: {
+        CachedBalancesController: {
+          cachedBalances: {
+            fizz: 'buzz',
+          },
+          bar: {
+            baz: 'buzz',
+          },
+        },
+        foo: 'bar',
+      },
+    }
+
+    const newStorage = await migration48.migrate(oldStorage)
+    assert.deepEqual(newStorage.data, {
+      ...expectedPreferencesState,
+      CachedBalancesController: {
+        bar: {
+          baz: 'buzz',
+        },
+      },
+      foo: 'bar',
+    })
+  })
+
+  it('should convert hex transaction metamaskNetworkId values to decimal', async function () {
+    const oldStorage = {
+      meta: {},
+      data: {
+        TransactionController: {
+          transactions: [
+            { fizz: 'buzz' },
+            null,
+            undefined,
+            0,
+            '',
+            { foo: 'bar', metamaskNetworkId: '1' },
+            { foo: 'bar', metamaskNetworkId: '0x1' },
+            { foo: 'bar', metamaskNetworkId: 'kaplar' },
+            { foo: 'bar', metamaskNetworkId: '0X2a' },
+            { foo: 'bar', metamaskNetworkId: '3' },
+          ],
+          bar: {
+            baz: 'buzz',
+          },
+        },
+        foo: 'bar',
+      },
+    }
+
+    const newStorage = await migration48.migrate(oldStorage)
+    assert.deepEqual(newStorage.data, {
+      ...expectedPreferencesState,
+      TransactionController: {
+        transactions: [
+          { fizz: 'buzz' },
+          null,
+          undefined,
+          0,
+          '',
+          { foo: 'bar', metamaskNetworkId: '1' },
+          { foo: 'bar', metamaskNetworkId: '1' },
+          { foo: 'bar', metamaskNetworkId: 'kaplar' },
+          { foo: 'bar', metamaskNetworkId: '42' },
+          { foo: 'bar', metamaskNetworkId: '3' },
+        ],
+        bar: {
+          baz: 'buzz',
+        },
+      },
+      foo: 'bar',
+    })
+  })
 })