Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update @metamask/eth-keyring-controller #1441

Merged
merged 19 commits into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/keyring-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,10 @@
"dependencies": {
"@keystonehq/metamask-airgapped-keyring": "^0.13.1",
"@metamask/base-controller": "workspace:^",
"@metamask/controller-utils": "workspace:^",
"@metamask/eth-keyring-controller": "^10.0.1",
"@metamask/eth-sig-util": "^6.0.0",
"@metamask/eth-keyring-controller": "^13.0.0",
"@metamask/message-manager": "workspace:^",
"@metamask/preferences-controller": "workspace:^",
"@metamask/utils": "^6.2.0",
"async-mutex": "^0.2.6",
"ethereumjs-util": "^7.0.10",
"ethereumjs-wallet": "^1.0.1",
Expand All @@ -46,6 +45,7 @@
"@ethereumjs/tx": "^4.2.0",
"@keystonehq/bc-ur-registry-eth": "^0.9.0",
"@metamask/auto-changelog": "^3.1.0",
"@metamask/eth-sig-util": "^6.0.0",
"@metamask/scure-bip39": "^2.1.0",
"@types/jest": "^27.4.1",
"deepmerge": "^4.2.2",
Expand Down
120 changes: 86 additions & 34 deletions packages/keyring-controller/src/KeyringController.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { Common } from '@ethereumjs/common';
import { Chain, Common, Hardfork } from '@ethereumjs/common';
import { TransactionFactory } from '@ethereumjs/tx';
import { CryptoHDKey, ETHSignature } from '@keystonehq/bc-ur-registry-eth';
import { MetaMaskKeyring as QRKeyring } from '@keystonehq/metamask-airgapped-keyring';
import { ControllerMessenger } from '@metamask/base-controller';
import { isValidHexAddress } from '@metamask/controller-utils';
import { keyringBuilderFactory } from '@metamask/eth-keyring-controller';
import {
normalize,
Expand All @@ -12,12 +11,17 @@ import {
SignTypedDataVersion,
} from '@metamask/eth-sig-util';
import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english';
import {
isValidHexAddress,
type Hex,
type Keyring,
type Json,
} from '@metamask/utils';
import { bufferToHex } from 'ethereumjs-util';
import * as sinon from 'sinon';
import * as uuid from 'uuid';

import type {
KeyringObject,
KeyringControllerEvents,
KeyringControllerMessenger,
KeyringControllerState,
Expand Down Expand Up @@ -55,7 +59,7 @@ const privateKey =
'1e4e6a4c0c077f4ae8ddfbf372918e61dd0fb4a4cfa592cb16e7546d505e68fc';
const password = 'password123';

const commonConfig = { chain: 'goerli', hardfork: 'berlin' };
const commonConfig = { chain: Chain.Goerli, hardfork: Hardfork.Berlin };

describe('KeyringController', () => {
afterEach(() => {
Expand Down Expand Up @@ -272,7 +276,7 @@ describe('KeyringController', () => {
expect(initialSeedWord).not.toBe(currentSeedWord);
expect(
isValidHexAddress(
cleanKeyringController.state.keyrings[0].accounts[0],
cleanKeyringController.state.keyrings[0].accounts[0] as Hex,
),
).toBe(true);
expect(controller.state.vault).toBeDefined();
Expand Down Expand Up @@ -347,24 +351,42 @@ describe('KeyringController', () => {
});

describe('exportSeedPhrase', () => {
describe('when correct password is provided', () => {
it('should export seed phrase', async () => {
describe('when mnemonic is not exportable', () => {
it('should throw error', async () => {
await withController(async ({ controller }) => {
const seed = await controller.exportSeedPhrase(password);
expect(seed).not.toBe('');
const primaryKeyring = controller.getKeyringsByType(
KeyringTypes.hd,
)[0] as Keyring<Json> & { mnemonic: string };

primaryKeyring.mnemonic = '';

await expect(controller.exportSeedPhrase(password)).rejects.toThrow(
"Can't get mnemonic bytes from keyring",
);
});
});
});

describe('when wrong password is provided', () => {
it('should export seed phrase', async () => {
await withController(async ({ controller, encryptor }) => {
sinon
.stub(encryptor, 'decrypt')
.throws(new Error('Invalid password'));
await expect(controller.exportSeedPhrase('')).rejects.toThrow(
'Invalid password',
);
describe('when mnemonic is exportable', () => {
describe('when correct password is provided', () => {
it('should export seed phrase', async () => {
await withController(async ({ controller }) => {
const seed = await controller.exportSeedPhrase(password);
expect(seed).not.toBe('');
});
});
});

describe('when wrong password is provided', () => {
it('should export seed phrase', async () => {
await withController(async ({ controller, encryptor }) => {
sinon
.stub(encryptor, 'decrypt')
.throws(new Error('Invalid password'));
await expect(controller.exportSeedPhrase('')).rejects.toThrow(
'Invalid password',
);
});
});
});
});
Expand All @@ -390,7 +412,9 @@ describe('KeyringController', () => {
await withController(async ({ controller }) => {
await expect(
controller.exportAccount(password, ''),
).rejects.toThrow(/^No keyring found for the requested account./u);
).rejects.toThrow(
'KeyringController - No keyring found. Error info: The address passed in is invalid/empty',
);
});
});
});
Expand Down Expand Up @@ -433,10 +457,11 @@ describe('KeyringController', () => {
it('should get correct keyring', async () => {
await withController(async ({ controller }) => {
const normalizedInitialAccounts =
controller.state.keyrings[0].accounts.map(normalize) as string[];
controller.state.keyrings[0].accounts.map(normalize);
const keyring = (await controller.getKeyringForAccount(
normalizedInitialAccounts[0],
)) as KeyringObject;
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
normalizedInitialAccounts[0]!,
)) as Keyring<Json>;
expect(keyring.type).toBe('HD Key Tree');
expect(keyring.getAccounts()).toStrictEqual(
normalizedInitialAccounts,
Expand All @@ -448,8 +473,12 @@ describe('KeyringController', () => {
describe('when non-existing account is provided', () => {
it('should throw error', async () => {
await withController(async ({ controller }) => {
await expect(controller.getKeyringForAccount('0x0')).rejects.toThrow(
'No keyring found for the requested account. Error info: There are keyrings, but none match the address',
await expect(
controller.getKeyringForAccount(
'0x0000000000000000000000000000000000000000',
),
).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
);
});
});
Expand All @@ -462,7 +491,7 @@ describe('KeyringController', () => {
await withController(async ({ controller }) => {
const keyrings = controller.getKeyringsByType(
KeyringTypes.hd,
) as KeyringObject[];
) as Keyring<Json>[];
expect(keyrings).toHaveLength(1);
expect(keyrings[0].type).toBe(KeyringTypes.hd);
expect(keyrings[0].getAccounts()).toStrictEqual(
Expand Down Expand Up @@ -658,7 +687,7 @@ describe('KeyringController', () => {
*/
it('should remove HD Key Tree keyring from state when single account associated with it is deleted', async () => {
await withController(async ({ controller, initialState }) => {
const account = initialState.keyrings[0].accounts[0];
const account = initialState.keyrings[0].accounts[0] as Hex;
await controller.removeAccount(account);
expect(controller.state.keyrings).toHaveLength(0);
});
Expand Down Expand Up @@ -700,12 +729,16 @@ describe('KeyringController', () => {
[privateKey],
);

await expect(controller.removeAccount('')).rejects.toThrow(
/^No keyring found for the requested account/u,
await expect(
controller.removeAccount(
'0x0000000000000000000000000000000000000000',
),
).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
);

await expect(controller.removeAccount('DUMMY_INPUT')).rejects.toThrow(
/^No keyring found for the requested account/u,
await expect(controller.removeAccount('0xDUMMY_INPUT')).rejects.toThrow(
'KeyringController - No keyring found. Error info: The address passed in is invalid/empty',
);
});
});
Expand Down Expand Up @@ -744,7 +777,7 @@ describe('KeyringController', () => {
from: '',
}),
).rejects.toThrow(
'No keyring found for the requested account. Error info: There are keyrings, but none match the address',
'KeyringController - No keyring found. Error info: The address passed in is invalid/empty',
);
});
});
Expand Down Expand Up @@ -788,7 +821,7 @@ describe('KeyringController', () => {
from: '',
}),
).rejects.toThrow(
'No keyring found for the requested account. Error info: There are keyrings, but none match the address',
'KeyringController - No keyring found. Error info: The address passed in is invalid/empty',
);
});
});
Expand All @@ -797,6 +830,7 @@ describe('KeyringController', () => {
describe('signTypedMessage', () => {
it('should throw when given invalid version', async () => {
await withController(
// @ts-expect-error QRKeyring is not yet compatible with Keyring type.
{ keyringBuilders: [keyringBuilderFactory(QRKeyring)] },
async ({ controller, initialState }) => {
const typedMsgParams = [
Expand Down Expand Up @@ -826,6 +860,7 @@ describe('KeyringController', () => {

it('should sign typed message V1', async () => {
await withController(
// @ts-expect-error QRKeyring is not yet compatible with Keyring type.
{ keyringBuilders: [keyringBuilderFactory(QRKeyring)] },
async ({ controller, initialState }) => {
const typedMsgParams = [
Expand Down Expand Up @@ -857,6 +892,7 @@ describe('KeyringController', () => {

it('should sign typed message V3', async () => {
await withController(
// @ts-expect-error QRKeyring is not yet compatible with Keyring type.
{ keyringBuilders: [keyringBuilderFactory(QRKeyring)] },
async ({ controller, initialState }) => {
const msgParams = {
Expand Down Expand Up @@ -913,6 +949,7 @@ describe('KeyringController', () => {

it('should sign typed message V4', async () => {
await withController(
// @ts-expect-error QRKeyring is not yet compatible with Keyring type.
{ keyringBuilders: [keyringBuilderFactory(QRKeyring)] },
async ({ controller, initialState }) => {
const msgParams = {
Expand Down Expand Up @@ -1073,7 +1110,7 @@ describe('KeyringController', () => {
expect(unsignedEthTx.v).toBeUndefined();
await controller.signTransaction(unsignedEthTx, '');
}).rejects.toThrow(
'No keyring found for the requested account. Error info: There are keyrings, but none match the address',
'KeyringController - No keyring found. Error info: The address passed in is invalid/empty',
);
});
});
Expand All @@ -1086,6 +1123,7 @@ describe('KeyringController', () => {
await withController(async ({ controller, initialState }) => {
await expect(async () => {
const account = initialState.keyrings[0].accounts[0];
// @ts-expect-error invalid transaction
await controller.signTransaction({}, account);
}).rejects.toThrow('tx.sign is not a function');
});
Expand Down Expand Up @@ -1158,6 +1196,20 @@ describe('KeyringController', () => {
expect(seedPhrase).toBeInstanceOf(Uint8Array);
});
});

it('should throw if mnemonic is not defined', async () => {
await withController(async ({ controller }) => {
const primaryKeyring = controller.getKeyringsByType(
KeyringTypes.hd,
)[0] as Keyring<Json> & { mnemonic: string };

primaryKeyring.mnemonic = '';

await expect(controller.verifySeedPhrase()).rejects.toThrow(
"Can't get mnemonic bytes from keyring",
);
});
});
});

describe('verifyPassword', () => {
Expand Down Expand Up @@ -1221,6 +1273,7 @@ describe('KeyringController', () => {

beforeEach(async () => {
signProcessKeyringController = await withController(
// @ts-expect-error QRKeyring is not yet compatible with Keyring type.
{ keyringBuilders: [keyringBuilderFactory(QRKeyring)] },
({ controller }) => controller,
);
Expand Down Expand Up @@ -1730,7 +1783,6 @@ async function withController<ReturnValue>(
const messenger = buildKeyringControllerMessenger();
const controller = new KeyringController({
encryptor,
cacheEncryptionKey: false,
messenger,
...preferences,
...rest,
Expand Down
Loading