Merge keychains (#1435)

Co-authored-by: Daniel Sinclair <[email protected]>

src/core/keychain/KeychainManager.ts

@@ -147,7 +147,7 @@ class KeychainManager {
           default:
             throw new Error('Keychain type not recognized.');
         }
-        await this.overrideReadOnlyKeychains(keychain);
+        await this.mergeKeychains(keychain);
         await this.checkForDuplicateInKeychain(keychain);
         this.state.keychains.push(keychain as Keychain);
         return keychain;
@@ -270,20 +270,27 @@ class KeychainManager {
     return false;
   }
 
-  async overrideReadOnlyKeychains(incomingKeychain: Keychain) {
+  async mergeKeychains(incomingKeychain: Keychain) {
     if (incomingKeychain.type === KeychainType.ReadOnlyKeychain) return;
+
     const currentAccounts = await this.getAccounts();
     const incomingAccounts = await incomingKeychain.getAccounts();
     const conflictingAccounts = incomingAccounts.filter((acc) =>
       currentAccounts.includes(acc),
     );
-    await Promise.all(
-      conflictingAccounts.map(async (acc) => {
-        const wallet = await this.getWallet(acc);
-        const isReadOnly = wallet.type === KeychainType.ReadOnlyKeychain;
-        if (isReadOnly) this.removeAccount(acc);
-      }),
-    );
+
+    for (const account of conflictingAccounts) {
+      const wallet = await this.getWallet(account);
+      // the incoming is not readOnly, so if the conflicting is, remove it to leave the one with higher privilages
+      // if the incoming is a hd wallet that derives an account in which the pk is already in the vault, remove this pk to leave the hd as the main
+      if (
+        wallet.type === KeychainType.ReadOnlyKeychain ||
+        (incomingKeychain.type === KeychainType.HdKeychain &&
+          wallet.type === KeychainType.KeyPairKeychain)
+      ) {
+        this.removeAccount(account);
+      }
+    }
   }
 
   async checkForDuplicateInKeychain(keychain: Keychain) {
@@ -326,7 +333,31 @@ class KeychainManager {
     return keychain;
   }
 
+  async removeKeychain(keychain: Keychain) {
+    this.state.keychains = this.state.keychains.filter((k) => k !== keychain);
+  }
+
+  async isMnemonicInVault(mnemonic: string) {
+    for (const k of this.state.keychains) {
+      if (k.type != KeychainType.HdKeychain) continue;
+      if ((await k.exportKeychain()) == mnemonic) return true;
+    }
+    return false;
+  }
+
   async importKeychain(opts: SerializedKeychain): Promise<Keychain> {
+    if (opts.type === KeychainType.KeyPairKeychain) {
+      const newAccount = (await this.deriveAccounts(opts))[0];
+      const existingAccounts = await this.getAccounts();
+      if (existingAccounts.includes(newAccount)) {
+        const existingKeychain = await this.getKeychain(newAccount);
+        // if the account is already in the vault (like in a hd keychain), we don't want to import it again
+        // UNLESS it's a readOnlyKeychain, which we DO WANT to override it, importing the pk
+        if (existingKeychain.type != KeychainType.ReadOnlyKeychain)
+          return existingKeychain;
+      }
+    }
+
     const result = await privates.get(this).restoreKeychain({
       ...opts,
       imported: true,
@@ -512,13 +543,6 @@ class KeychainManager {
     throw new Error('No keychain found for account');
   }
 
-  isAccountInReadOnlyKeychain(address: Address): Keychain | undefined {
-    for (const keychain of this.state.keychains) {
-      if (keychain.type !== KeychainType.ReadOnlyKeychain) continue;
-      if ((keychain as ReadOnlyKeychain).address === address) return keychain;
-    }
-  }
-
   async getSigner(address: Address) {
     const keychain = await this.getKeychain(address);
     return keychain.getSigner(address);

src/core/keychain/index.ts

@@ -31,6 +31,7 @@ import {
 import { addHexPrefix } from '../utils/hex';
 
 import { keychainManager } from './KeychainManager';
+import { SerializedKeypairKeychain } from './keychainTypes/keyPairKeychain';
 
 interface TypedDataTypes {
   EIP712Domain: MessageTypeProperty[];
@@ -99,6 +100,10 @@ export const createWallet = async (): Promise<Address> => {
   return accounts[0];
 };
 
+export const isMnemonicInVault = async (mnemonic: EthereumWalletSeed) => {
+  return keychainManager.isMnemonicInVault(mnemonic);
+};
+
 export const deriveAccountsFromSecret = async (
   secret: EthereumWalletSeed,
 ): Promise<Address[]> => {
@@ -168,12 +173,16 @@ export const importWallet = async (
       return address;
     }
     case EthereumWalletType.privateKey: {
-      const keychain = await keychainManager.importKeychain({
+      const opts: SerializedKeypairKeychain = {
         type: KeychainType.KeyPairKeychain,
         privateKey: secret,
-      });
-      const address = (await keychain.getAccounts())[0];
-      return address;
+      };
+      const newAccount = (await keychainManager.deriveAccounts(opts))[0];
+
+      await keychainManager.importKeychain(opts);
+      // returning the derived address instead of the first from the keychain,
+      // because this pk could have been elevated to hd while importing
+      return newAccount;
     }
     case EthereumWalletType.readOnly: {
       const keychain = await keychainManager.importKeychain({

src/core/keychain/keychainTypes/hdKeychain.ts

@@ -86,10 +86,19 @@ export class HdKeychain implements IKeychain {
         const _privates = privates.get(this)!;
         const derivedWallet = _privates.deriveWallet(index);
 
-        // if account already exists in a readonly keychain, remove it
-        keychainManager
-          .isAccountInReadOnlyKeychain(derivedWallet.address)
-          ?.removeAccount(derivedWallet.address);
+        // if account already exists in a another keychain, remove it
+        keychainManager.state.keychains.forEach(async (keychain) => {
+          const keychainAccounts = await keychain.getAccounts();
+          if (keychainAccounts.includes(derivedWallet.address)) {
+            keychain.removeAccount(derivedWallet.address);
+            if (
+              keychain.type == KeychainType.ReadOnlyKeychain ||
+              keychain.type == KeychainType.KeyPairKeychain
+            ) {
+              keychainManager.removeKeychain(keychain);
+            }
+          }
+        });
 
         const wallet = new Wallet(
           derivedWallet.privateKey as BytesLike,

src/core/types/walletActions.ts

@@ -7,6 +7,7 @@ export enum walletActions {
   unlock = 'unlock',
   verify_password = 'verify_password',
   derive_accounts_from_secret = 'derive_accounts_from_secret',
+  is_mnemonic_in_vault = 'is_mnemonic_in_vault',
   create = 'create',
   import = 'import',
   add = 'add',

src/entries/background/handlers/handleWallets.ts

@@ -23,6 +23,7 @@ import {
   importHardwareWallet,
   importWallet,
   isInitialized,
+  isMnemonicInVault,
   isPasswordSet,
   isVaultUnlocked,
   lockVault,
@@ -152,6 +153,9 @@ export const handleWallets = () =>
               payload as EthereumWalletSeed,
             );
             break;
+          case 'is_mnemonic_in_vault':
+            response = await isMnemonicInVault(payload as EthereumWalletSeed);
+            break;
           case 'get_accounts':
             response = await getAccounts();
             break;

src/entries/popup/components/ImportWallet/ImportWalletSelection.tsx

@@ -130,7 +130,10 @@ export const ImportWalletSelection = ({ onboarding = false }) => {
   const onImport = () =>
     importSecrets({ secrets }).then(() => {
       setCurrentAddress(accountsToImport[0]);
-      if (onboarding) navigate(ROUTES.CREATE_PASSWORD);
+      if (onboarding)
+        navigate(ROUTES.CREATE_PASSWORD, {
+          state: { backTo: ROUTES.IMPORT__SEED },
+        });
       else navigate(ROUTES.HOME);
     });
 

src/entries/popup/components/ImportWallet/ImportWalletSelectionEdit.tsx

@@ -89,8 +89,11 @@ export function ImportWalletSelectionEdit({ onboarding = false }) {
         (a) => !accountsIgnored.includes(a),
       );
       setCurrentAddress(importedAccounts[0]);
-      if (onboarding) navigate(ROUTES.CREATE_PASSWORD);
-      else navigate(ROUTES.HOME);
+      if (onboarding) {
+        navigate(ROUTES.CREATE_PASSWORD, {
+          state: { backTo: ROUTES.IMPORT__SEED },
+        });
+      } else navigate(ROUTES.HOME);
     });
 
   return (

src/entries/popup/components/ImportWallet/ImportWalletViaSeed.tsx

@@ -25,6 +25,7 @@ import {
   Text,
   textStyles,
 } from '~/design-system';
+import { triggerAlert } from '~/design-system/components/Alert/Alert';
 import { accentSelectionStyle } from '~/design-system/components/Input/Input.css';
 import {
   transformScales,
@@ -36,6 +37,7 @@ import {
   removeImportWalletSecrets,
   setImportWalletSecrets,
 } from '../../handlers/importWalletSecrets';
+import { isMnemonicInVault } from '../../handlers/wallet';
 import { useRainbowNavigate } from '../../hooks/useRainbowNavigate';
 import { ROUTES } from '../../urls';
 
@@ -166,6 +168,9 @@ const secretsReducer = (
   return newSecrets;
 };
 
+const emptySecrets12 = Array.from({ length: 12 }).map(() => '');
+const emptySecrets24 = Array.from({ length: 24 }).map(() => '');
+
 const ImportWalletViaSeed = () => {
   const navigate = useRainbowNavigate();
   const location = useLocation();
@@ -174,16 +179,13 @@ const ImportWalletViaSeed = () => {
   const [globalError, setGlobalError] = useState(false);
   const [invalidWords, setInvalidWords] = useState<number[]>([]);
   const [visibleInput, setVisibleInput] = useState<number | null>(null);
-  const [secrets, setSecrets] = useReducer(
-    secretsReducer,
-    Array.from({ length: 12 }).map(() => ''),
-  );
+  const [secrets, setSecrets] = useReducer(secretsReducer, emptySecrets12);
 
   const toggleWordLength = useCallback(() => {
     if (secrets.length === 12) {
-      setSecrets(Array.from({ length: 24 }).map(() => ''));
+      setSecrets(emptySecrets12);
     } else {
-      setSecrets(Array.from({ length: 12 }).map(() => ''));
+      setSecrets(emptySecrets24);
     }
     setInvalidWords([]);
     setGlobalError(false);
@@ -228,10 +230,18 @@ const ImportWalletViaSeed = () => {
   );
 
   const handleImportWallet = useCallback(async () => {
+    if (await isMnemonicInVault(secrets.join(' '))) {
+      triggerAlert({
+        text: i18n.t('import_wallet_via_seed.duplicate_seed'),
+      });
+      setSecrets(emptySecrets12);
+      return;
+    }
+
     return navigate(
       onboarding ? ROUTES.IMPORT__SELECT : ROUTES.NEW_IMPORT_WALLET_SELECTION,
     );
-  }, [navigate, onboarding]);
+  }, [navigate, onboarding, secrets]);
 
   const handleKeyDown = useCallback(
     (e: KeyboardEvent) => {

src/entries/popup/handlers/wallet.ts

@@ -264,6 +264,9 @@ export const updatePassword = async (password: string, newPassword: string) => {
 export const deriveAccountsFromSecret = async (secret: string) =>
   walletAction<Address[]>('derive_accounts_from_secret', secret);
 
+export const isMnemonicInVault = async (secret: string) =>
+  walletAction<boolean>('is_mnemonic_in_vault', secret);
+
 export const verifyPassword = async (password: string) =>
   walletAction<boolean>('verify_password', password);
 

static/json/languages/en_US.json

@@ -950,7 +950,8 @@
     "import_wallet_group": "Import wallet group",
     "n_words_might_be_wrong": "%{n} words may be misspelled or incorrect.",
     "1_word_might_be_wrong": "1 word may be misspelled or incorrect.",
-    "couldnt_paste": "We couldn’t paste your Secret Recovery Phrase"
+    "couldnt_paste": "We couldn’t paste your Secret Recovery Phrase",
+    "duplicate_seed": "This seed is imported already"
   },
   "import_wallet_via_private_key": {
     "title": "Import Your Wallet",