test(multi-srp): add remove last account test

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: 94.04,
+      branches: 94.61,
       functions: 100,
-      lines: 98.87,
-      statements: 98.88,
+      lines: 99.02,
+      statements: 99.03,
     },
   },
 

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

@@ -261,12 +261,11 @@ describe('KeyringController', () => {
     });
   });
 
-  describe('behavior when keyring metadata length mismatch', () => {
+  describe('when the keyringMetadata length is different from the number of keyrings', () => {
     it('should throw an error if the keyring metadata length mismatch', async () => {
-      let vaultWithOneKeyring;
-      await withController(async ({ controller }) => {
-        vaultWithOneKeyring = controller.state.vault;
-      });
+      const vaultWithOneKeyring = await withController(
+        async ({ controller }) => controller.state.vault,
+      );
 
       await withController(
         {
@@ -468,18 +467,29 @@ describe('KeyringController', () => {
           );
         });
 
-        it('should restore same vault if old seedWord is used', async () => {
+        it('should call encryptor.encrypt with the same keyrings if old seedWord is used', async () => {
           await withController(
             { cacheEncryptionKey },
-            async ({ controller, initialState }) => {
+            async ({ controller, encryptor }) => {
+              const encryptSpy = jest.spyOn(encryptor, 'encrypt');
+              const serializedKeyring = await controller.withKeyring(
+                { type: 'HD Key Tree' },
+                async (keyring) => keyring.serialize(),
+              );
               const currentSeedWord =
                 await controller.exportSeedPhrase(password);
 
               await controller.createNewVaultAndRestore(
                 password,
                 currentSeedWord,
               );
-              expect(initialState.vault).toStrictEqual(controller.state.vault);
+
+              expect(encryptSpy).toHaveBeenCalledWith(password, [
+                {
+                  data: serializedKeyring,
+                  type: 'HD Key Tree',
+                },
+              ]);
             },
           );
         });
@@ -1450,6 +1460,19 @@ describe('KeyringController', () => {
           );
         });
       });
+
+      it('should remove the keyring if last account is removed and its not primary keyring', async () => {
+        await withController(async ({ controller }) => {
+          await controller.addNewKeyring(KeyringTypes.hd);
+          expect(controller.state.keyrings).toHaveLength(2);
+          expect(controller.state.keyringsMetadata).toHaveLength(2);
+          await controller.removeAccount(
+            controller.state.keyrings[1].accounts[0],
+          );
+          expect(controller.state.keyrings).toHaveLength(1);
+          expect(controller.state.keyringsMetadata).toHaveLength(1);
+        });
+      });
     });
 
     describe('when the keyring for the given address does not support removeAccount', () => {
@@ -2682,7 +2705,7 @@ describe('KeyringController', () => {
         await controller.submitPassword('123');
 
         await expect(controller.verifySeedPhrase()).rejects.toThrow(
-          'KeyringController - No HD Keyring found',
+          KeyringControllerError.KeyringNotFound,
         );
       });
     });
@@ -4050,6 +4073,8 @@ describe('KeyringController', () => {
         await withController(
           { keyringBuilders: [keyringBuilderFactory(MockKeyring)] },
           async ({ controller, initialState }) => {
+            // We're mocking BaseController .update() to throw an error, as it's the last operation
+            // that is called before the function is rolled back.
             jest.spyOn(controller, 'update' as never).mockImplementation(() => {
               throw new Error('You will never be able to change me!');
             });

packages/keyring-controller/src/KeyringController.ts

@@ -686,13 +686,13 @@ export class KeyringController extends BaseController<
     this.#assertIsUnlocked();
 
     return this.#persistOrRollback(async () => {
-      const selectedKeyring = this.getKeyringsByType(
-        'HD Key Tree',
-      )[0] as EthKeyring<Json>;
-      if (!selectedKeyring) {
+      const primaryKeyring = this.getKeyringsByType('HD Key Tree')[0] as
+        | EthKeyring<Json>
+        | undefined;
+      if (!primaryKeyring) {
         throw new Error('No HD keyring found');
       }
-      const oldAccounts = await selectedKeyring.getAccounts();
+      const oldAccounts = await primaryKeyring.getAccounts();
 
       if (accountCount && oldAccounts.length !== accountCount) {
         if (accountCount > oldAccounts.length) {
@@ -708,7 +708,7 @@ export class KeyringController extends BaseController<
         return existingAccount;
       }
 
-      const [addedAccountAddress] = await selectedKeyring.addAccounts(1);
+      const [addedAccountAddress] = await primaryKeyring.addAccounts(1);
       await this.#verifySeedPhrase();
 
       return addedAccountAddress;
@@ -1412,16 +1412,6 @@ export class KeyringController extends BaseController<
    */
   async verifySeedPhrase(keyringId?: string): Promise<Uint8Array> {
     this.#assertIsUnlocked();
-    const keyring = this.#getKeyringByIdOrDefault(keyringId);
-
-    if (!keyring) {
-      throw new Error(KeyringControllerError.NoHdKeyring);
-    }
-
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
-    if (keyring.type !== KeyringTypes.hd) {
-      throw new Error(KeyringControllerError.UnsupportedVerifySeedPhrase);
-    }
 
     return this.#withControllerLock(async () =>
       this.#verifySeedPhrase(keyringId),
@@ -1894,7 +1884,7 @@ export class KeyringController extends BaseController<
     this.#assertControllerMutexIsLocked();
 
     // QRKeyring is not yet compatible with Keyring type from @metamask/utils
-return (await this.#newKeyring(KeyringTypes.qr)) as unknown as QRKeyring;
+    return (await this.#newKeyring(KeyringTypes.qr)) as unknown as QRKeyring;
   }
 
   /**
@@ -1974,9 +1964,13 @@ return (await this.#newKeyring(KeyringTypes.qr)) as unknown as QRKeyring;
 
     const keyring = this.#getKeyringByIdOrDefault(keyringId);
 
-    // This will never going to be undefined because we are checking for it in all of the callers
     if (!keyring) {
-      throw new Error('No HD keyring found.');
+      throw new Error(KeyringControllerError.KeyringNotFound);
+    }
+
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
+    if (keyring.type !== KeyringTypes.hd) {
+      throw new Error(KeyringControllerError.UnsupportedVerifySeedPhrase);
     }
 
     assertHasUint8ArrayMnemonic(keyring);