fix: remove metadata for unsupported keyrings

packages/keyring-controller/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Metadata for unsupported keyring is removed on unlock ([#5725](https://github.com/MetaMask/core/pull/5725))
+
 ## [21.0.6]
 
 ### Changed

packages/keyring-controller/jest.config.js

@@ -17,10 +17,10 @@ module.exports = merge(baseConfig, {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 93.64,
+      branches: 93.71,
       functions: 100,
-      lines: 98.92,
-      statements: 98.93,
+      lines: 98.95,
+      statements: 98.95,
     },
   },
 

packages/keyring-controller/src/KeyringController.test.ts

@@ -127,6 +127,92 @@ describe('KeyringController', () => {
         },
       );
     });
+
+    it('allows removing a keyring builder without bricking the wallet when metadata was already generated', async () => {
+      await withController(
+        {
+          skipVaultCreation: true,
+          state: {
+            vault: 'my vault',
+            keyringsMetadata: [
+              { id: 'hd', name: '' },
+              // This keyring is unsupported
+              { id: 'unsupported', name: '' },
+              { id: 'hd2', name: '' },
+            ],
+          },
+        },
+        async ({ controller, encryptor }) => {
+          jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([
+            {
+              type: KeyringTypes.hd,
+              data: '',
+            },
+            {
+              type: 'Unsupported',
+              data: '',
+            },
+            {
+              type: KeyringTypes.hd,
+              data: '',
+            },
+          ]);
+
+          await controller.submitPassword(password);
+
+          expect(controller.state.keyrings).toHaveLength(2);
+          expect(controller.state.keyrings[0].type).toBe(KeyringTypes.hd);
+          expect(controller.state.keyrings[1].type).toBe(KeyringTypes.hd);
+          expect(controller.state.keyringsMetadata).toStrictEqual([
+            { id: 'hd', name: '' },
+            { id: 'hd2', name: '' },
+          ]);
+        },
+      );
+    });
+
+    it('allows removing a keyring builder without bricking the wallet when metadata was not yet generated', async () => {
+      await withController(
+        {
+          skipVaultCreation: true,
+          state: {
+            vault: 'my vault',
+            keyringsMetadata: [
+              { id: 'hd', name: '' },
+              { id: 'hd2', name: '' },
+            ],
+          },
+        },
+        async ({ controller, encryptor }) => {
+          jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([
+            {
+              type: 'HD Key Tree',
+              data: '',
+            },
+            {
+              type: 'HD Key Tree',
+              data: '',
+            },
+            // This keyring was already unsupported
+            // (no metadata, and is at the end of the array)
+            {
+              type: MockKeyring.type,
+              data: 'unsupported',
+            },
+          ]);
+
+          await controller.submitPassword(password);
+
+          expect(controller.state.keyrings).toHaveLength(2);
+          expect(controller.state.keyrings[0].type).toBe(KeyringTypes.hd);
+          expect(controller.state.keyrings[1].type).toBe(KeyringTypes.hd);
+          expect(controller.state.keyringsMetadata).toStrictEqual([
+            { id: 'hd', name: '' },
+            { id: 'hd2', name: '' },
+          ]);
+        },
+      );
+    });
   });
 
   describe('addNewAccount', () => {

packages/keyring-controller/src/KeyringController.ts

@@ -2288,6 +2288,7 @@ export class KeyringController extends BaseController<
 
       this.update((state) => {
         state.keyrings = updatedKeyrings;
+        state.keyringsMetadata = this.#keyringsMetadata.slice();
         if (updatedState.encryptionKey || updatedState.encryptionSalt) {
           state.encryptionKey = updatedState.encryptionKey;
           state.encryptionSalt = updatedState.encryptionSalt;
@@ -2571,6 +2572,11 @@ export class KeyringController extends BaseController<
     } catch (error) {
       console.error(error);
       this.#unsupportedKeyrings.push(serialized);
+      if (this.#keyringsMetadata.length > this.#keyrings.length) {
+        // There was already a metadata entry for the keyring, so
+        // we need to remove it
+        this.#keyringsMetadata.splice(this.#keyrings.length, 1);
+      }
       return undefined;
     }
   }