Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

Commit

Permalink
feat!: create new vault with any keyring (#329)
Browse files Browse the repository at this point in the history 
This change updates the logic inside the eth-keyring-controller to allow the creation of a vault with any keyring. This removes the hardcoded HD Keyring from the code and allow the client to pass the parameters to instantiate any keyring that is accepted by the controller.

BREAKING: Unify createNewVaultAndKeychain and createNewVaultAndRestore into new method createNewVaultWithKeyring. createNewVaultWithKeyring accepts a password and a keyring object provided by the client and returns the KeyringControllerState
  • Loading branch information
@gantunesr
gantunesr authored Jan 15, 2024
1 parent cac52da commit aa68888
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 117 deletions.
8 changes: 4 additions & 4 deletions jest.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ module.exports = {
// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
branches: 80.35,
functions: 93.65,
lines: 91.9,
statements: 92.07,
branches: 80.18,
functions: 93.54,
lines: 91.69,
statements: 91.87,
},
},
preset: 'ts-jest',
Expand Down
199 changes: 136 additions & 63 deletions src/KeyringController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,17 @@ async function initializeKeyringController({
if (seedPhrase && !password) {
throw new Error('Password required to restore vault');
} else if (seedPhrase && password) {
await keyringController.createNewVaultAndRestore(
PASSWORD,
walletOneSeedWords,
);
await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
opts: {
mnemonic: walletOneSeedWords,
numberOfAccounts: 1,
},
});
} else if (password) {
await keyringController.createNewVaultAndKeychain(PASSWORD);
await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
});
}

return keyringController;
Expand Down Expand Up @@ -162,7 +167,9 @@ describe('KeyringController', () => {
const keyringController = await initializeKeyringController({
password: PASSWORD,
});
await keyringController.createNewVaultAndKeychain(PASSWORD);
await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
});
await keyringController.persistAllKeyrings();
expect(keyringController.keyrings).toHaveLength(1);

Expand Down Expand Up @@ -257,7 +264,9 @@ describe('KeyringController', () => {
cacheEncryptionKey: true,
},
});
await keyringController.createNewVaultAndKeychain(PASSWORD);
await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
});
deleteEncryptionKeyAndSalt(keyringController);

const response = await keyringController.persistAllKeyrings();
Expand Down Expand Up @@ -293,32 +302,81 @@ describe('KeyringController', () => {
});
});
});

it('should add an `encryptionSalt` to the `memStore` when a vault is restored', async () => {
const keyringController = await initializeKeyringController({
password: PASSWORD,
constructorOptions: {
cacheEncryptionKey: true,
},
});

await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
opts: {
mnemonic: walletOneSeedWords,
numberOfAccounts: 1,
},
});

const finalMemStore = keyringController.memStore.getState();
expect(finalMemStore.encryptionKey).toBe(MOCK_HARDCODED_KEY);
expect(finalMemStore.encryptionSalt).toBe(MOCK_ENCRYPTION_SALT);
});
});

describe('createNewVaultAndKeychain', () => {
it('should create a new vault', async () => {
describe('createNewVaultWithKeyring', () => {
it('should create a new vault with a HD keyring', async () => {
const keyringController = await initializeKeyringController({
password: PASSWORD,
});
keyringController.store.putState({});
assert(!keyringController.store.getState().vault, 'no previous vault');

const newVault = await keyringController.createNewVaultAndKeychain(
const newVault = await keyringController.createNewVaultWithKeyring(
PASSWORD,
{
type: KeyringType.HD,
},
);
const { vault } = keyringController.store.getState();
expect(vault).toStrictEqual(expect.stringMatching('.+'));
expect(typeof newVault).toBe('object');
});

it('should create a new vault with a simple keyring', async () => {
const keyringController = await initializeKeyringController({
password: PASSWORD,
});
keyringController.store.putState({});
assert(!keyringController.store.getState().vault, 'no previous vault');

const newVault = await keyringController.createNewVaultWithKeyring(
PASSWORD,
{
type: KeyringType.Simple,
opts: walletOnePrivateKey,
},
);
const { vault } = keyringController.store.getState();
expect(vault).toStrictEqual(expect.stringMatching('.+'));
expect(typeof newVault).toBe('object');

const accounts = await keyringController.getAccounts();
expect(accounts).toHaveLength(1);
expect(accounts[0]).toBe(walletOneAddresses[0]);
});

it('should unlock the vault', async () => {
const keyringController = await initializeKeyringController({
password: PASSWORD,
});
keyringController.store.putState({});
assert(!keyringController.store.getState().vault, 'no previous vault');

await keyringController.createNewVaultAndKeychain(PASSWORD);
await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
});
const { isUnlocked } = keyringController.memStore.getState();
expect(isUnlocked).toBe(true);
});
Expand All @@ -335,7 +393,9 @@ describe('KeyringController', () => {
keyringController.store.putState({});
assert(!keyringController.store.getState().vault, 'no previous vault');

await keyringController.createNewVaultAndKeychain(PASSWORD);
await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
});
const { vault } = keyringController.store.getState();
// eslint-disable-next-line jest/no-restricted-matchers
expect(vault).toBeTruthy();
Expand All @@ -353,8 +413,10 @@ describe('KeyringController', () => {
.mockImplementation(async () => Promise.resolve([]));

await expect(async () =>
keyringController.createNewVaultAndKeychain(PASSWORD),
).rejects.toThrow(KeyringControllerError.NoAccountOnKeychain);
keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
}),
).rejects.toThrow(KeyringControllerError.NoFirstAccount);
});

describe('when `cacheEncryptionKey` is enabled', () => {
Expand All @@ -366,16 +428,16 @@ describe('KeyringController', () => {
},
});

await keyringController.createNewVaultAndKeychain(PASSWORD);
await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
});

const finalMemStore = keyringController.memStore.getState();
expect(finalMemStore.encryptionKey).toBe(MOCK_HARDCODED_KEY);
expect(finalMemStore.encryptionSalt).toBe(MOCK_ENCRYPTION_SALT);
});
});
});

describe('createNewVaultAndRestore', () => {
it('clears old keyrings and creates a one', async () => {
const keyringController = await initializeKeyringController({
password: PASSWORD,
Expand All @@ -387,10 +449,13 @@ describe('KeyringController', () => {
const allAccounts = await keyringController.getAccounts();
expect(allAccounts).toHaveLength(2);

await keyringController.createNewVaultAndRestore(
PASSWORD,
walletOneSeedWords,
);
await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
opts: {
mnemonic: walletOneSeedWords,
numberOfAccounts: 1,
},
});

const allAccountsAfter = await keyringController.getAccounts();
expect(allAccountsAfter).toHaveLength(1);
Expand All @@ -403,7 +468,13 @@ describe('KeyringController', () => {
});
await expect(async () =>
// @ts-expect-error Missing other required permission types.
keyringController.createNewVaultAndRestore(12, walletTwoSeedWords),
keyringController.createNewVaultWithKeyring(12, {
type: KeyringType.HD,
opts: {
mnemonic: walletTwoSeedWords,
numberOfAccounts: 1,
},
}),
).rejects.toThrow('KeyringController - Password must be of type string.');
});

Expand All @@ -412,16 +483,26 @@ describe('KeyringController', () => {
password: PASSWORD,
});
await expect(async () =>
keyringController.createNewVaultAndRestore(
PASSWORD,
'test test test palace city barely security section midnight wealth south deer',
),
keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
opts: {
mnemonic:
'test test test palace city barely security section midnight wealth south deer',
numberOfAccounts: 1,
},
}),
).rejects.toThrow(
'Eth-Hd-Keyring: Invalid secret recovery phrase provided',
);

await expect(async () =>
keyringController.createNewVaultAndRestore(PASSWORD, '1234'),
keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
opts: {
mnemonic: '1234',
numberOfAccounts: 1,
},
}),
).rejects.toThrow(
'Eth-Hd-Keyring: Invalid secret recovery phrase provided',
);
Expand All @@ -437,10 +518,13 @@ describe('KeyringController', () => {
Buffer.from(walletTwoSeedWords).values(),
);

await keyringController.createNewVaultAndRestore(
PASSWORD,
mnemonicAsArrayOfNumbers,
);
await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
opts: {
mnemonic: mnemonicAsArrayOfNumbers,
numberOfAccounts: 1,
},
});

const allAccountsAfter = await keyringController.getAccounts();
expect(allAccountsAfter).toHaveLength(1);
Expand All @@ -456,31 +540,14 @@ describe('KeyringController', () => {
.mockImplementation(async () => Promise.resolve([]));

await expect(async () =>
keyringController.createNewVaultAndRestore(
PASSWORD,
walletTwoSeedWords,
),
).rejects.toThrow('KeyringController - First Account not found.');
});

describe('when `cacheEncryptionKey` is enabled', () => {
it('should add an `encryptionSalt` to the `memStore` when a vault is restored', async () => {
const keyringController = await initializeKeyringController({
password: PASSWORD,
constructorOptions: {
cacheEncryptionKey: true,
keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
opts: {
mnemonic: walletTwoSeedWords,
numberOfAccounts: 1,
},
});

await keyringController.createNewVaultAndRestore(
PASSWORD,
walletOneSeedWords,
);

const finalMemStore = keyringController.memStore.getState();
expect(finalMemStore.encryptionKey).toBe(MOCK_HARDCODED_KEY);
expect(finalMemStore.encryptionSalt).toBe(MOCK_ENCRYPTION_SALT);
});
}),
).rejects.toThrow('KeyringController - First Account not found.');
});
});

Expand Down Expand Up @@ -943,10 +1010,13 @@ describe('KeyringController', () => {
it('does not throw if a vault exists in state', async () => {
const keyringController = await initializeKeyringController();

await keyringController.createNewVaultAndRestore(
PASSWORD,
walletOneSeedWords,
);
await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
opts: {
mnemonic: walletOneSeedWords,
numberOfAccounts: 1,
},
});

expect(async () =>
keyringController.verifyPassword(PASSWORD),
Expand Down Expand Up @@ -1273,10 +1343,13 @@ describe('KeyringController', () => {
const keyringController = await initializeKeyringController({
password: PASSWORD,
});
await keyringController.createNewVaultAndRestore(
PASSWORD,
walletOneSeedWords,
);
await keyringController.createNewVaultWithKeyring(PASSWORD, {
type: KeyringType.HD,
opts: {
mnemonic: walletOneSeedWords,
numberOfAccounts: 1,
},
});
const privateKey = await keyringController.exportAccount(
// @ts-expect-error this value should never be undefined in this specific context.
walletOneAddresses[0],
Expand Down
Loading

0 comments on commit aa68888

Please sign in to comment.