From 30c9a6019e168d634cbe552a16e72da290936526 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Thu, 4 May 2023 09:52:28 +0200 Subject: [PATCH 1/2] feat: don't create new vault if one already exists --- .../src/KeyringController.test.ts | 56 ++++++++++++++----- .../src/KeyringController.ts | 13 ++++- 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index cab6094f95..1b274dd4e2 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -10,7 +10,7 @@ import { TransactionFactory } from '@ethereumjs/tx'; import { MetaMaskKeyring as QRKeyring } from '@keystonehq/metamask-airgapped-keyring'; import { CryptoHDKey, ETHSignature } from '@keystonehq/bc-ur-registry-eth'; import * as uuid from 'uuid'; -import { NetworkType } from '@metamask/controller-utils'; +import { isValidHexAddress, NetworkType } from '@metamask/controller-utils'; import { keyringBuilderFactory } from '@metamask/eth-keyring-controller'; import MockEncryptor from '../tests/mocks/mockEncryptor'; import { @@ -217,20 +217,46 @@ describe('KeyringController', () => { }); describe('createNewVaultAndKeychain', () => { - it('should create new vault, mnemonic and keychain', async () => { - const initialSeedWord = await keyringController.exportSeedPhrase( - password, - ); - expect(initialSeedWord).toBeDefined(); - const currentState = await keyringController.createNewVaultAndKeychain( - password, - ); - expect(initialState).not.toBe(currentState); - const currentSeedWord = await keyringController.exportSeedPhrase( - password, - ); - expect(currentSeedWord).toBeDefined(); - expect(initialSeedWord).not.toBe(currentSeedWord); + describe('when there is no existing vault', () => { + it('should create new vault, mnemonic and keychain', async () => { + const cleanKeyringController = new KeyringController( + preferences, + baseConfig, + ); + const initialSeedWord = await keyringController.exportSeedPhrase( + password, + ); + const currentState = + await cleanKeyringController.createNewVaultAndKeychain(password); + const currentSeedWord = await cleanKeyringController.exportSeedPhrase( + password, + ); + expect(initialSeedWord).toBeDefined(); + expect(initialState).not.toBe(currentState); + expect(currentSeedWord).toBeDefined(); + expect(initialSeedWord).not.toBe(currentSeedWord); + expect(isValidHexAddress(currentState.keyrings[0].accounts[0])).toBe( + true, + ); + }); + }); + + describe('when there is an existing vault', () => { + it('should return existing vault', async () => { + const initialSeedWord = await keyringController.exportSeedPhrase( + password, + ); + const currentState = await keyringController.createNewVaultAndKeychain( + password, + ); + const currentSeedWord = await keyringController.exportSeedPhrase( + password, + ); + expect(initialSeedWord).toBeDefined(); + expect(initialState).toBe(currentState); + expect(currentSeedWord).toBeDefined(); + expect(initialSeedWord).toBe(currentSeedWord); + }); }); }); diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 70b3b290f9..fcbb8f8cce 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -305,9 +305,16 @@ export class KeyringController extends BaseController< async createNewVaultAndKeychain(password: string) { const releaseLock = await this.mutex.acquire(); try { - const vault = await this.#keyring.createNewVaultAndKeychain(password); - this.updateIdentities(await this.#keyring.getAccounts()); - this.fullUpdate(); + let vault; + const accounts = await this.#keyring.getAccounts(); + if (accounts.length > 0) { + vault = await this.#keyring.fullUpdate(); + } else { + vault = await this.#keyring.createNewVaultAndKeychain(password); + this.updateIdentities(await this.#keyring.getAccounts()); + this.fullUpdate(); + } + return vault; } finally { releaseLock(); From bc9ea62071903b5f41a2e595cdac77c474f5ea02 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Thu, 4 May 2023 14:37:00 +0200 Subject: [PATCH 2/2] fix: use internal fullUpdate --- packages/keyring-controller/src/KeyringController.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index fcbb8f8cce..c20b9c2926 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -306,12 +306,12 @@ export class KeyringController extends BaseController< const releaseLock = await this.mutex.acquire(); try { let vault; - const accounts = await this.#keyring.getAccounts(); + const accounts = await this.getAccounts(); if (accounts.length > 0) { - vault = await this.#keyring.fullUpdate(); + vault = await this.fullUpdate(); } else { vault = await this.#keyring.createNewVaultAndKeychain(password); - this.updateIdentities(await this.#keyring.getAccounts()); + this.updateIdentities(await this.getAccounts()); this.fullUpdate(); }