fix: cp-12.18.0 discard duplicate accounts on unlock

.storybook/test-data.js

@@ -1259,20 +1259,18 @@ const state = {
           '0x64a845a5b02460acf8a3d84503b0d68d028b4bb4',
           '0xb19ac54efa18cc3a14a5b821bfec73d284bf0c5e',
         ],
+        metadata: {
+          id: '01JN08SYECPZHFHB3K0J1NHJ4H',
+          name: '',
+        },
       },
       {
         type: KeyringType.ledger,
         accounts: ['0x9d0ba4ddac06032527b140912ec808ab9451b788'],
-      },
-    ],
-    keyringsMetadata: [
-      {
-        id: '01JN08SYECPZHFHB3K0J1NHJ4H',
-        name: '',
-      },
-      {
-        id: '01JN08T38HEXPYQX2HKP1FCRMZ',
-        name: '',
+        metadata: {
+          id: '01JN08T38HEXPYQX2HKP1FCRMZ',
+          name: '',
+        },
       },
     ],
     ...mockNetworkState(

app/scripts/controllers/mmi-controller.test.ts

@@ -177,7 +177,6 @@ describe('MMIController', function () {
       state: {
         keyrings: [],
         isUnlocked: true,
-        keyringsMetadata: [],
       },
       encryptor: {
         encrypt(_, object) {

app/scripts/controllers/preferences-controller.test.ts

@@ -152,12 +152,10 @@ describe('preferences controller', () => {
             {
               type: 'HD Key Tree',
               accounts: [firstAddress, secondAddress],
-            },
-          ],
-          keyringsMetadata: [
-            {
-              id: '01JKDGGBRE3DGZA7N1PZJSQK4W',
-              name: '',
+              metadata: {
+                id: '01JKDGGBRE3DGZA7N1PZJSQK4W',
+                name: '',
+              },
             },
           ],
         },
@@ -203,12 +201,10 @@ describe('preferences controller', () => {
             {
               type: 'HD Key Tree',
               accounts: [firstAddress, secondAddress],
-            },
-          ],
-          keyringsMetadata: [
-            {
-              id: '01JKDGGBRE3DGZA7N1PZJSQK4W',
-              name: '',
+              metadata: {
+                id: '01JKDGGBRE3DGZA7N1PZJSQK4W',
+                name: '',
+              },
             },
           ],
         },
@@ -260,12 +256,10 @@ describe('preferences controller', () => {
             {
               type: 'HD Key Tree',
               accounts: [firstAddress, secondAddress],
-            },
-          ],
-          keyringsMetadata: [
-            {
-              id: '01JKDGGBRE3DGZA7N1PZJSQK4W',
-              name: '',
+              metadata: {
+                id: '01JKDGGBRE3DGZA7N1PZJSQK4W',
+                name: '',
+              },
             },
           ],
         },
@@ -301,12 +295,10 @@ describe('preferences controller', () => {
             {
               type: 'HD Key Tree',
               accounts: [firstAddress, secondAddress],
-            },
-          ],
-          keyringsMetadata: [
-            {
-              id: '01JKDGGBRE3DGZA7N1PZJSQK4W',
-              name: '',
+              metadata: {
+                id: '01JKDGGBRE3DGZA7N1PZJSQK4W',
+                name: '',
+              },
             },
           ],
         },
@@ -511,12 +503,10 @@ describe('preferences controller', () => {
             {
               type: 'HD Key Tree',
               accounts: [firstAddress, secondAddress],
-            },
-          ],
-          keyringsMetadata: [
-            {
-              id: '01JKDGGBRE3DGZA7N1PZJSQK4W',
-              name: '',
+              metadata: {
+                id: '01JKDGGBRE3DGZA7N1PZJSQK4W',
+                name: '',
+              },
             },
           ],
         },

app/scripts/metamask-controller.actions.test.js

@@ -211,18 +211,19 @@ describe('MetaMaskController', function () {
       const result2 = metamaskController.keyringController.state;
 
       // On restore, a new keyring metadata is generated.
-      expect(result1.keyringsMetadata[0].id).toBe(mockULIDs[0]);
-      expect(result2).toStrictEqual(
-        expect.objectContaining({
-          ...result1,
-          keyringsMetadata: [
-            {
+      expect(result1.keyrings[0].metadata.id).toBe(mockULIDs[0]);
+      expect(result2).toStrictEqual({
+        ...result1,
+        keyrings: [
+          {
+            ...result1.keyrings[0],
+            metadata: {
+              ...result1.keyrings[0].metadata,
               id: mockULIDs[1],
-              name: '',
             },
-          ],
-        }),
-      );
+          },
+        ],
+      });
     });
   });
 

app/scripts/metamask-controller.js

@@ -7129,8 +7129,8 @@ export default class MetamaskController extends EventEmitter {
             .map((keyring, index) => {
               if (keyring.type === KeyringTypes.hd) {
                 return {
-                  id: state.keyringsMetadata[index].id,
-                  name: state.keyringsMetadata[index].name,
+                  id: keyring.metadata.id,
+                  name: keyring.metadata.name,
                   type: 'mnemonic',
                   primary: index === 0,
                 };

app/scripts/metamask-controller.test.js

@@ -3992,10 +3992,9 @@ describe('MetaMaskController', () => {
           metamaskController.keyringController.state.keyrings;
 
         const newlyAddedKeyringId =
-          metamaskController.keyringController.state.keyringsMetadata[
-            metamaskController.keyringController.state.keyringsMetadata.length -
-              2 // -1 for the snap keyring, -1 for the newly added keyring
-          ].id;
+          metamaskController.keyringController.state.keyrings[
+            metamaskController.keyringController.state.keyrings.length - 2 // -1 for the snap keyring, -1 for the newly added keyring
+          ].metadata.id;
 
         const newSRP = Buffer.from(
           await metamaskController.getSeedPhrase(password, newlyAddedKeyringId),

app/scripts/migrations/156.1.test.ts

@@ -0,0 +1,101 @@
+import { migrate } from './156.1';
+
+const expectedVersion = 156.1;
+const previousVersion = 156;
+
+describe(`migration #${expectedVersion}`, () => {
+  it('does nothing if state has no KeyringController property', async () => {
+    const oldVersionedData = {
+      meta: { version: previousVersion },
+      data: {},
+    };
+    const expectedVersionedData = {
+      meta: { version: expectedVersion },
+      data: oldVersionedData.data,
+    };
+
+    const newVersionedData = await migrate(oldVersionedData);
+
+    expect(newVersionedData).toStrictEqual(expectedVersionedData);
+  });
+
+  it('does nothing if state.KeyringController is not an object', async () => {
+    const oldVersionedData = {
+      meta: { version: previousVersion },
+      data: {
+        KeyringController: 'not-an-object',
+      },
+    };
+    const expectedVersionedData = {
+      meta: { version: expectedVersion },
+      data: oldVersionedData.data,
+    };
+
+    const newVersionedData = await migrate(oldVersionedData);
+
+    expect(newVersionedData).toStrictEqual(expectedVersionedData);
+  });
+
+  it('does nothing if state.KeyringController has no keyringsMetadata property', async () => {
+    const oldVersionedData = {
+      meta: { version: previousVersion },
+      data: {
+        KeyringController: {
+          vault: 'vault',
+          keyrings: [
+            {
+              type: 'HD Key Tree',
+              accounts: ['0x1234567890abcdef'],
+            },
+          ],
+        },
+      },
+    };
+    const expectedVersionedData = {
+      meta: { version: expectedVersion },
+      data: oldVersionedData.data,
+    };
+
+    const newVersionedData = await migrate(oldVersionedData);
+
+    expect(newVersionedData).toStrictEqual(expectedVersionedData);
+  });
+
+  it('removes keyringsMetadata from KeyringController state', async () => {
+    const oldVersionedData = {
+      meta: { version: previousVersion },
+      data: {
+        KeyringController: {
+          vault: 'vault',
+          keyrings: [
+            {
+              type: 'HD Key Tree',
+              accounts: ['0x1234567890abcdef'],
+            },
+          ],
+          keyringsMetadata: {
+            someKey: 'someValue',
+          },
+        },
+      },
+    };
+    const expectedVersionedData = {
+      meta: { version: expectedVersion },
+      data: {
+        KeyringController: {
+          vault: 'vault',
+          keyrings: [
+            {
+              type: 'HD Key Tree',
+              accounts: ['0x1234567890abcdef'],
+            },
+          ],
+        },
+      },
+    };
+
+    const newVersionedData = await migrate(oldVersionedData);
+
+    expect(newVersionedData).toStrictEqual(expectedVersionedData);
+  });
+});

app/scripts/migrations/156.1.ts

@@ -0,0 +1,37 @@
+import { hasProperty } from '@metamask/utils';
+import { cloneDeep, isObject } from 'lodash';
+
+export const version = 156.1;
+
+/**
+ * Remove `keyringsMetadata` from `KeyringController` state.
+ *
+ * @param originalVersionedData - Versioned MetaMask extension state, exactly what we persist to dist.
+ * @param originalVersionedData.meta - State metadata.
+ * @param originalVersionedData.meta.version - The current state version.
+ * @param originalVersionedData.data - The persisted MetaMask state, keyed by controller.
+ * @returns Updated versioned MetaMask extension state.
+ */
+export async function migrate(originalVersionedData: {
+  meta: { version: number };
+  data: Record<string, unknown>;
+}) {
+  const versionedData = cloneDeep(originalVersionedData);
+  versionedData.meta.version = version;
+  versionedData.data = transformState(versionedData.data);
+  return versionedData;
+}
+
+function transformState(state: Record<string, unknown>) {
+  const newState = state;
+
+  if (
+    hasProperty(newState, 'KeyringController') &&
+    isObject(newState.KeyringController) &&
+    hasProperty(newState.KeyringController, 'keyringsMetadata')
+  ) {
+    delete newState.KeyringController.keyringsMetadata;
+  }
+
+  return newState;
+}

app/scripts/migrations/index.js

@@ -185,6 +185,7 @@ const migrations = [
   require('./154'),
   require('./155'),
   require('./156'),
+  require('./156.1'),
   require('./157'),
   require('./158'),
 ];

lavamoat/browserify/beta/policy.json

@@ -1678,8 +1678,7 @@
     },
     "@metamask/keyring-controller": {
       "globals": {
-        "console.error": true,
-        "console.log": true
+        "console.error": true
       },
       "packages": {
         "@ethereumjs/tx>@ethereumjs/util": true,
@@ -1691,6 +1690,7 @@
         "@metamask/utils": true,
         "@metamask/name-controller>async-mutex": true,
         "@metamask/keyring-controller>ethereumjs-wallet": true,
+        "lodash": true,
         "@metamask/keyring-controller>ulid": true
       }
     },

lavamoat/browserify/flask/policy.json

@@ -1678,8 +1678,7 @@
     },
     "@metamask/keyring-controller": {
       "globals": {
-        "console.error": true,
-        "console.log": true
+        "console.error": true
       },
       "packages": {
         "@ethereumjs/tx>@ethereumjs/util": true,
@@ -1691,6 +1690,7 @@
         "@metamask/utils": true,
         "@metamask/name-controller>async-mutex": true,
         "@metamask/keyring-controller>ethereumjs-wallet": true,
+        "lodash": true,
         "@metamask/keyring-controller>ulid": true
       }
     },

lavamoat/browserify/main/policy.json

@@ -1678,8 +1678,7 @@
     },
     "@metamask/keyring-controller": {
       "globals": {
-        "console.error": true,
-        "console.log": true
+        "console.error": true
       },
       "packages": {
         "@ethereumjs/tx>@ethereumjs/util": true,
@@ -1691,6 +1690,7 @@
         "@metamask/utils": true,
         "@metamask/name-controller>async-mutex": true,
         "@metamask/keyring-controller>ethereumjs-wallet": true,
+        "lodash": true,
         "@metamask/keyring-controller>ulid": true
       }
     },

lavamoat/browserify/mmi/policy.json

@@ -1809,8 +1809,7 @@
     },
     "@metamask/keyring-controller": {
       "globals": {
-        "console.error": true,
-        "console.log": true
+        "console.error": true
       },
       "packages": {
         "@ethereumjs/tx>@ethereumjs/util": true,
@@ -1822,6 +1821,7 @@
         "@metamask/utils": true,
         "@metamask/name-controller>async-mutex": true,
         "@metamask/keyring-controller>ethereumjs-wallet": true,
+        "lodash": true,
         "@metamask/keyring-controller>ulid": true
       }
     },