Use encryptor isVaultUpdated (#310)

* chore: update browser-passworder

* refactor: remove `updateVault` from `GenericEncryptor`

Author: mikesposito

package.json

@@ -44,7 +44,7 @@
   },
   "dependencies": {
     "@ethereumjs/tx": "^4.2.0",
-    "@metamask/browser-passworder": "^4.2.0",
+    "@metamask/browser-passworder": "^4.3.0",
     "@metamask/eth-hd-keyring": "^7.0.1",
     "@metamask/eth-sig-util": "^7.0.0",
     "@metamask/eth-simple-keyring": "^6.0.1",

src/KeyringController.test.ts

@@ -863,13 +863,11 @@ describe('KeyringController', () => {
           });
           deleteEncryptionKeyAndSalt(keyringController);
           const initialVault = keyringController.store.getState().vault;
-          const updatedVaultMock =
-            '{"vault": "updated_vault_detail", "salt": "salt"}';
           const mockEncryptionResult = {
             data: '0x1234',
             iv: 'an iv',
           };
-          sinon.stub(mockEncryptor, 'updateVault').resolves(updatedVaultMock);
+          sinon.stub(mockEncryptor, 'isVaultUpdated').returns(false);
           sinon
             .stub(mockEncryptor, 'encryptWithKey')
             .resolves(mockEncryptionResult);
@@ -898,21 +896,12 @@ describe('KeyringController', () => {
             },
           });
           const initialVault = keyringController.store.getState().vault;
-          const updatedVaultMock =
-            '{"vault": "updated_vault_detail", "salt": "salt"}';
-          const mockEncryptionResult = {
-            data: '0x1234',
-            iv: 'an iv',
-          };
-          sinon.stub(mockEncryptor, 'updateVault').resolves(updatedVaultMock);
-          sinon
-            .stub(mockEncryptor, 'encryptWithKey')
-            .resolves(mockEncryptionResult);
+          sinon.stub(mockEncryptor, 'isVaultUpdated').returns(false);
 
           await keyringController.unlockKeyrings(PASSWORD);
           const updatedVault = keyringController.store.getState().vault;
 
-          expect(initialVault).not.toBe(updatedVault);
+          expect(initialVault).toBe(updatedVault);
         });
       });
 
@@ -929,7 +918,7 @@ describe('KeyringController', () => {
           const initialVault = keyringController.store.getState().vault;
           const updatedVaultMock =
             '{"vault": "updated_vault_detail", "salt": "salt"}';
-          sinon.stub(mockEncryptor, 'updateVault').resolves(updatedVaultMock);
+          sinon.stub(mockEncryptor, 'isVaultUpdated').returns(false);
           sinon.stub(mockEncryptor, 'encrypt').resolves(updatedVaultMock);
 
           await keyringController.unlockKeyrings(PASSWORD);

src/KeyringController.ts

@@ -931,9 +931,8 @@ class KeyringController extends EventEmitter {
     if (
       this.password &&
       (!this.#cacheEncryptionKey || !encryptionKey) &&
-      this.#encryptor.updateVault &&
-      (await this.#encryptor.updateVault(encryptedVault, this.password)) !==
-        encryptedVault
+      this.#encryptor.isVaultUpdated &&
+      !this.#encryptor.isVaultUpdated(encryptedVault)
     ) {
       // Re-encrypt the vault with safer method if one is available
       await this.persistAllKeyrings();

src/test/encryptor.mock.ts

@@ -81,6 +81,10 @@ export class MockEncryptor implements ExportableKeyEncryptor {
     return _vault;
   }
 
+  isVaultUpdated(_vault: string) {
+    return true;
+  }
+
   generateSalt() {
     return MOCK_ENCRYPTION_SALT;
   }

src/types.ts

@@ -2,6 +2,7 @@ import type {
   DetailedDecryptResult,
   DetailedEncryptionResult,
   EncryptionResult,
+  KeyDerivationOptions,
 } from '@metamask/browser-passworder';
 import type { Json, Keyring } from '@metamask/utils';
 
@@ -63,14 +64,17 @@ export type GenericEncryptor = {
    */
   decrypt: (password: string, encryptedString: string) => Promise<unknown>;
   /**
-   * Optional vault migration helper. Updates the provided vault, re-encrypting
-   * data with a safer algorithm if one is available.
+   * Optional vault migration helper. Checks if the provided vault is up to date
+   * with the desired encryption algorithm.
    *
-   * @param vault - The encrypted string to update.
-   * @param password - The password to decrypt the vault with.
+   * @param vault - The encrypted string to check.
+   * @param targetDerivationParams - The desired target derivation params.
    * @returns The updated encrypted string.
    */
-  updateVault?: (vault: string, password: string) => Promise<string>;
+  isVaultUpdated?: (
+    vault: string,
+    targetDerivationParams?: KeyDerivationOptions,
+  ) => boolean;
 };
 
 /**

yarn.lock

@@ -1127,7 +1127,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@metamask/browser-passworder@npm:^4.2.0":
+"@metamask/browser-passworder@npm:^4.3.0":
   version: 4.3.0
   resolution: "@metamask/browser-passworder@npm:4.3.0"
   dependencies:
@@ -1208,7 +1208,7 @@ __metadata:
     "@lavamoat/allow-scripts": ^2.3.1
     "@lavamoat/preinstall-always-fail": ^1.0.0
     "@metamask/auto-changelog": ^3.0.0
-    "@metamask/browser-passworder": ^4.2.0
+    "@metamask/browser-passworder": ^4.3.0
     "@metamask/eslint-config": ^12.2.0
     "@metamask/eslint-config-jest": ^12.1.0
     "@metamask/eslint-config-nodejs": ^12.1.0