fix: back up logic to clear vault before reapplying

[sethkfman]

Description

Root Cause of the Issue

backupVault unconditionally calls setInternetCredentials, failing when VAULT_BACKUP_KEY already exists in the keychain due to repeated KEYRING_STATE_CHANGE_EVENT triggers.

The Engine class is initialized, setting up the vault backup mechanism.

The Engine class constructor calls this.handleVaultBackup() to subscribe to keyring state changes, initiating the backup process.
(See app/core/Engine/Engine.ts)

The Engine subscribes to the KEYRING_STATE_CHANGE_EVENT.

This subscription means that whenever the keyring's state changes, the associated callback function will be executed, potentially triggering a vault backup.
(See app/core/Engine/Engine.ts)

The keyring state changes, and the new state contains a vault.

This event signifies that the keyring's vault data has been loaded or modified, prompting a backup. This could happen on unlock, account creation, or import.
(See app/core/Engine/Engine.ts)

The handleVaultBackup function is triggered, calling backupVault.

The subscription callback in handleVaultBackup is executed, leading to the invocation of the backupVault function with the current keyring state.
(See app/core/Engine/Engine.ts)

The backupVault function attempts to store the vault in the keychain without checking for existing credentials.

The backupVault function directly calls setInternetCredentials with VAULT_BACKUP_KEY, VAULT_BACKUP_KEY, and the vault data, without verifying if an entry with that key already exists. This is the most critical step leading to the error.
(See app/core/BackupVault/backupVault.ts)

The setInternetCredentials function throws an error because the keychain item already exists.

Since a vault backup already exists in the keychain (from a previous keyring state change), setInternetCredentials throws an error, halting the backup process.
(See node_modules/react-native-keychain/index.js)

Changes

Back up temporary vault before re-applying primary vault

Since the primary vault needs to be reset before a new one is applied, we added a fail safe (in backupVault function) by first backing up a temporary version of the vault. In the event of failure when attempting to reset and/or re-apply the vault, the temporary one would still be available as the fallback. If the vault is successfully re-applied, then the temporary vault is reset immediately afterwards.

Clear any existing vault backup before creating a new one to avoid keychain errors.

await _resetVaultBackup();

This line calls the _resetVaultBackup function to delete any existing keychain entry associated with VAULT_BACKUP_KEY before attempting to create a new one. This prevents the "Keychain item already exists" error.
(See app/core/BackupVault/backupVault.ts)

Implement error handling to prevent app crashes during vault backup.

try {
  // ... backup logic ...
} catch (error) {
  Logger.error(error, 'Vault backup failed');
  return {
    success: false,
    error: error instanceof Error ? error.message : VAULT_BACKUP_FAILED
  };
}

This code adds a try...catch block around the vault backup logic. If any error occurs during the backup process, it will be caught, logged, and a failure response will be returned, preventing the app from crashing.
(See app/core/BackupVault/backupVault.ts)

Debounce backupVault calls in KeyringController state change subscription

Since this event is triggered may be triggered multiple times in quick succession, we added a debounce mechanism to prevent multiple asynchronous calls for updating the vault

Related issues

Fixes: #9419

Manual testing steps

1. Start up the app
2. Run the app and see the lack of a vault back up error in logs.
3. Remove the state object to trigger vault corruption
4. Confirm app can recover.

Screenshots/Recordings

Before

After

iOS

vault.backup.mov

Android

Pre-merge author checklist

vault.backup.android.mov

• I’ve followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
• I've completed the PR template to the best of my ability
• I’ve included tests if applicable
• I’ve documented my code using JSDoc format if applicable
• I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

• I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
• I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

[Cal-L]

Nice LGTM

[gantunesr]

looks good! I'd just update/add the proper unit tests

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[github-actions]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: e96b50b
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/3b41ba89-049e-485e-b3d8-5f04069c4c83

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
95.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud