Track vault migrations and errors generically

Vault migration is no longer tracked as Argon2 specifically, but
generically for all migrations. Already-migrated vaults are not tracked,
and the migration function return value reflects that no migration was
performed. Additionally, error messages are bubbled out of the migration
function and reported up to the caller.

The main outcome here is that PostHog migration events include the
migrated-to version, and PostHog migration failure events include the
error message. This will leave us open to future migrations, and will
let us know if there are certain failures that are happening broadly
that we may be able to do something about.

Notably, wrong passwords will be tracked as migration errors if a wrong
password is typed with an older vault version in the mix. Mitigating
this may or may not be a good idea.

background/lib/posthog.ts

@@ -12,14 +12,14 @@ export enum AnalyticsEvent {
   NEW_ACCOUNT_TO_TRACK = "Address added to tracking on network",
   CUSTOM_CHAIN_ADDED = "Custom chain added",
   DAPP_CONNECTED = "Dapp Connected",
+  VAULT_MIGRATION = "Migrate to newer vault version",
+  VAULT_MIGRATION_FAILED = "Vault version migration failed",
 }
 
 export enum OneTimeAnalyticsEvent {
   ONBOARDING_STARTED = "Onboarding Started",
   ONBOARDING_FINISHED = "Onboarding Finished",
   CHAIN_ADDED = "Chain Added",
-  ARGON_MIGRATION = "Migrate to Argon2",
-  ARGON_MIGRATION_FAILED = "Migrate to Argon2 failed",
 }
 
 export const isOneTimeAnalyticsEvent = (

background/services/internal-signer/index.ts

@@ -8,7 +8,7 @@ import { normalizeEVMAddress, sameEVMAddress } from "../../lib/utils"
 import { ServiceCreatorFunction, ServiceLifecycleEvents } from "../types"
 import {
   getEncryptedVaults,
-  migrateVaultsToArgon,
+  migrateVaultsToLatestVersion,
   writeLatestEncryptedVault,
 } from "./storage"
 import {
@@ -30,7 +30,7 @@ import logger from "../../lib/logger"
 import PreferenceService from "../preferences"
 import { DEFAULT_AUTOLOCK_INTERVAL } from "../preferences/defaults"
 import AnalyticsService from "../analytics"
-import { OneTimeAnalyticsEvent } from "../../lib/posthog"
+import { AnalyticsEvent } from "../../lib/posthog"
 
 export enum SignerInternalTypes {
   mnemonicBIP39S128 = "mnemonic#bip39:128",
@@ -279,17 +279,18 @@ export default class InternalSignerService extends BaseService<Events> {
 
     const {
       encryptedData: { vaults, version },
-      success,
-    } = await migrateVaultsToArgon(password)
+      ...migrationResults
+    } = await migrateVaultsToLatestVersion(password)
     this.#cachedVaultVersion = version
 
-    if (success) {
-      this.analyticsService.sendOneTimeAnalyticsEvent(
-        OneTimeAnalyticsEvent.ARGON_MIGRATION
-      )
-    } else {
-      this.analyticsService.sendOneTimeAnalyticsEvent(
-        OneTimeAnalyticsEvent.ARGON_MIGRATION_FAILED
+    if (migrationResults.migrated) {
+      this.analyticsService.sendAnalyticsEvent(AnalyticsEvent.VAULT_MIGRATION, {
+        version,
+      })
+    } else if (migrationResults.errorMessage !== undefined) {
+      this.analyticsService.sendAnalyticsEvent(
+        AnalyticsEvent.VAULT_MIGRATION_FAILED,
+        { error: migrationResults.errorMessage }
       )
     }
 

background/services/internal-signer/storage.ts

@@ -96,12 +96,20 @@ export async function writeLatestEncryptedVault(
   }
 }
 
-export async function migrateVaultsToArgon(
-  password: string
-): Promise<{ encryptedData: SerializedEncryptedVaults; success: boolean }> {
+export async function migrateVaultsToLatestVersion(password: string): Promise<
+  | {
+      encryptedData: SerializedEncryptedVaults
+      migrated: true
+    }
+  | {
+      encryptedData: SerializedEncryptedVaults
+      migrated: false
+      errorMessage?: string
+    }
+> {
   const serializedVaults = await getEncryptedVaults()
   if (serializedVaults.version === VaultVersion.Argon2) {
-    return { encryptedData: serializedVaults, success: true }
+    return { encryptedData: serializedVaults, migrated: false }
   }
 
   const { vaults } = serializedVaults
@@ -160,9 +168,13 @@ export async function migrateVaultsToArgon(
       tallyVaults: newSerializedVaults,
     })
 
-    return { encryptedData: newSerializedVaults, success: true }
+    return { encryptedData: newSerializedVaults, migrated: true }
   } catch (error) {
-    logger.error("Failed to migrate vaults to Argon2")
-    return { encryptedData: serializedVaults, success: false }
+    logger.error("Failed to migrate vaults to Argon2", error)
+    return {
+      encryptedData: serializedVaults,
+      migrated: false,
+      errorMessage: String(error),
+    }
   }
 }

background/services/internal-signer/tests/storage.unit.test.ts

@@ -6,7 +6,7 @@ import {
 } from "../encryption"
 import {
   getEncryptedVaults,
-  migrateVaultsToArgon,
+  migrateVaultsToLatestVersion,
   writeLatestEncryptedVault,
 } from "../storage"
 
@@ -74,10 +74,10 @@ describe("Storage utils", () => {
 
     const {
       encryptedData: { vaults, version },
-      success,
-    } = await migrateVaultsToArgon(mockedPassword)
+      migrated,
+    } = await migrateVaultsToLatestVersion(mockedPassword)
 
-    expect(success).toEqual(true)
+    expect(migrated).toEqual(true)
     expect(version).toEqual(VaultVersion.Argon2)
     expect(vaults.length).toEqual(1)
 
@@ -100,11 +100,36 @@ describe("Storage utils", () => {
 
     const {
       encryptedData: { vaults, version },
-      success,
-    } = await migrateVaultsToArgon(mockedPassword)
+      ...migrationData
+    } = await migrateVaultsToLatestVersion(mockedPassword)
 
-    expect(success).toEqual(true)
+    expect(migrationData.migrated).toEqual(false)
+    expect(
+      migrationData.migrated === false && migrationData.errorMessage
+    ).toBeUndefined()
     expect(version).toEqual(VaultVersion.Argon2)
     expect(vaults[0].vault).toEqual(vaultEncryptedWithArgon2)
   })
+
+  it("should report migration errors in the return value", async () => {
+    await browser.storage.local.set({
+      tallyVaults: {
+        version: VaultVersion.PBKDF2,
+        vaults: [],
+      },
+    })
+    await writeLatestEncryptedVault(vaultEncryptedWithPBKDF2)
+
+    const migrationData = await migrateVaultsToLatestVersion("wrong password")
+
+    expect(migrationData.migrated).toEqual(false)
+    expect(
+      migrationData.migrated === false && migrationData.errorMessage
+    ).not.toBeUndefined()
+    expect(
+      migrationData.migrated === false &&
+        migrationData.errorMessage !== undefined &&
+        migrationData.errorMessage.length
+    ).toBeGreaterThan(1)
+  })
 })