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

refactor!: migrate HdKeyring to typescript #166

Merged
merged 15 commits into from
Feb 4, 2025
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
refactor: use Hex instead of String
mikesposito committed Jan 29, 2025
commit 4d86d8e3408128b17787712d76071c431f3cbb8e
68 changes: 42 additions & 26 deletions packages/keyring-eth-hd/src/index.ts
Original file line number Diff line number Diff line change
@@ -25,7 +25,13 @@ import {
} from '@metamask/key-tree';
import { generateMnemonic } from '@metamask/scure-bip39';
import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english';
import { assert, assertIsHexString, remove0x } from '@metamask/utils';
import {
add0x,
assert,
assertIsHexString,
type Hex,
remove0x,
} from '@metamask/utils';
import { HDKey } from 'ethereum-cryptography/hdkey';
import { keccak256 } from 'ethereum-cryptography/keccak';
import { bytesToHex } from 'ethereum-cryptography/utils';
@@ -172,7 +178,7 @@ class HdKeyring {
* @param numberOfAccounts - The number of accounts to add.
* @returns The addresses of the new accounts.
*/
async addAccounts(numberOfAccounts = 1): Promise<string[]> {
async addAccounts(numberOfAccounts = 1): Promise<Hex[]> {
if (!this.root) {
throw new Error('Eth-Hd-Keyring: No secret recovery phrase provided');
}
@@ -196,7 +202,7 @@ class HdKeyring {
*
* @returns The addresses of all accounts in the keyring.
*/
getAccounts(): string[] {
getAccounts(): Hex[] {
return this.#wallets.map((wallet) => {
assert(wallet.publicKey, 'Expected public key to be set');
return this.#addressfromPublicKey(wallet.publicKey);
@@ -210,18 +216,17 @@ class HdKeyring {
* @param origin - The origin of the app requesting the account.
* @returns The public address of the account.
*/
async getAppKeyAddress(address: string, origin: string): Promise<string> {
async getAppKeyAddress(address: Hex, origin: string): Promise<Hex> {
if (!origin || typeof origin !== 'string') {
throw new Error(`'origin' must be a non-empty string`);
}
const wallet = this.#getWalletForAccount(address, {
withAppKeyOrigin: origin,
});
assert(wallet.publicKey, 'Expected public key to be set');
const appKeyAddress = normalize(
const appKeyAddress = this.#normalizeAddress(
publicToAddress(wallet.publicKey).toString('hex'),
);
assert(appKeyAddress, 'Expected app key address to be set');
return appKeyAddress;
}

@@ -235,7 +240,7 @@ class HdKeyring {
* @returns The private key of the account.
*/
async exportAccount(
address: string,
address: Hex,
opts?: { withAppKeyOrigin: string },
): Promise<string> {
const wallet = opts
@@ -258,7 +263,7 @@ class HdKeyring {
* @returns The signed transaction.
*/
async signTransaction(
address: string,
address: Hex,
tx: TypedTransaction,
opts = {},
): Promise<TypedTransaction> {
@@ -277,7 +282,7 @@ class HdKeyring {
* @returns The signature of the message.
*/
async signMessage(
address: string,
address: Hex,
data: string,
opts: HDKeyringAccountSelectionOptions = {},
): Promise<string> {
@@ -304,7 +309,7 @@ class HdKeyring {
* @returns The signature of the message.
*/
async signPersonalMessage(
address: string,
address: Hex,
msgHex: string,
opts: HDKeyringAccountSelectionOptions = {},
): Promise<string> {
@@ -323,7 +328,7 @@ class HdKeyring {
* @returns The decrypted message.
*/
async decryptMessage(
withAccount: string,
withAccount: Hex,
encryptedData: EthEncryptedData,
): Promise<string> {
const wallet = this.#getWalletForAccount(withAccount);
@@ -343,7 +348,7 @@ class HdKeyring {
* @returns The signature of the message.
*/
async signTypedData<Types extends MessageTypes>(
withAccount: string,
withAccount: Hex,
typedData: TypedDataV1 | TypedMessage<Types>,
opts: HDKeyringAccountSelectionOptions & {
version: SignTypedDataVersion;
@@ -367,9 +372,8 @@ class HdKeyring {
*
* @param account - The address of the account to remove.
*/
removeAccount(account: string): void {
const address = normalize(account);
assert(address, 'Must specify a valid address.');
removeAccount(account: Hex): void {
const address = this.#normalizeAddress(account);
if (
!this.#wallets
.map(
@@ -394,7 +398,7 @@ class HdKeyring {
* @returns The public key of the account.
*/
async getEncryptionPublicKey(
withAccount: string,
withAccount: Hex,
opts: HDKeyringAccountSelectionOptions = {},
): Promise<string> {
const privKey = this.#getPrivateKeyFor(withAccount, opts);
@@ -478,7 +482,7 @@ class HdKeyring {
* @returns The private key of the account.
*/
#getPrivateKeyFor(
address: string,
address: Hex,
opts?: HDKeyringAccountSelectionOptions,
): Uint8Array | Buffer {
if (!address) {
@@ -495,7 +499,7 @@ class HdKeyring {
* @param account - The address of the account.
* @returns The wallet for the account as HDKey.
*/
#getWalletForAccount(account: string): HDKey;
#getWalletForAccount(account: Hex): HDKey;

/**
* Get the wallet for the specified account and app origin.
@@ -505,7 +509,7 @@ class HdKeyring {
* @returns A key pair representing the wallet.
*/
#getWalletForAccount(
accounts: string,
accounts: Hex,
opts: { withAppKeyOrigin: string },
): { privateKey: Buffer; publicKey: Buffer };

@@ -518,15 +522,15 @@ class HdKeyring {
* @returns A key pair representing the wallet.
*/
#getWalletForAccount(
account: string,
account: Hex,
opts?: HDKeyringAccountSelectionOptions,
): HDKey | { privateKey: Buffer; publicKey: Buffer };

#getWalletForAccount(
account: string,
account: Hex,
{ withAppKeyOrigin }: HDKeyringAccountSelectionOptions = {},
): HDKey | { privateKey: Buffer; publicKey: Buffer } {
const address = normalize(account);
const address = this.#normalizeAddress(account);
const wallet = this.#wallets.find(({ publicKey }) => {
return publicKey && this.#addressfromPublicKey(publicKey) === address;
});
@@ -581,10 +585,22 @@ class HdKeyring {
* @param publicKey - The public key of the account.
* @returns The address of the account.
*/
#addressfromPublicKey(publicKey: Uint8Array): string {
return bufferToHex(
publicToAddress(Buffer.from(publicKey), true),
).toLowerCase();
#addressfromPublicKey(publicKey: Uint8Array): Hex {
return add0x(
bufferToHex(publicToAddress(Buffer.from(publicKey), true)).toLowerCase(),
);
}

/**
* Normalize an address to a lower-cased '0x'-prefixed hex string.
*
* @param address - The address to normalize.
* @returns The normalized address.
*/
#normalizeAddress(address: string): Hex {
const normalized = normalize(address);
assert(normalized, 'Expected address to be set');
return add0x(normalized);
}
}

37 changes: 19 additions & 18 deletions packages/keyring-eth-hd/test/index.test.ts
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@ import {
type MessageTypes,
} from '@metamask/eth-sig-util';
import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english';
import { assert, type Hex } from '@metamask/utils';
import { webcrypto } from 'crypto';
import { keccak256 } from 'ethereum-cryptography/keccak';
// eslint-disable-next-line @typescript-eslint/naming-convention
@@ -39,6 +40,12 @@ const secondAcct = '0x1b00aed43a693f3a957f9feb5cc08afa031e37a0';

const notKeyringAddress = '0xbD20F6F5F1616947a39E11926E78ec94817B3931';

const getAddressAtIndex = (keyring: HdKeyring, index: number): Hex => {
const accounts = keyring.getAccounts();
assert(accounts[index], `Account not found at index ${index}`);
return accounts[index];
};

describe('hd-keyring', () => {
describe('compare old bip39 implementation with new', () => {
it('should derive the same accounts from the same mnemonics', async () => {
@@ -355,8 +362,7 @@ describe('hd-keyring', () => {
await keyring.deserialize();
await keyring.generateRandomMnemonic();
await keyring.addAccounts(1);
const addresses = keyring.getAccounts();
const address = addresses[0] as string;
const address = getAddressAtIndex(keyring, 0);
const signature = await keyring.signTypedData(address, typedData);
const restored = recoverTypedSignature({
data: typedData,
@@ -381,8 +387,7 @@ describe('hd-keyring', () => {
await keyring.deserialize();
await keyring.generateRandomMnemonic();
await keyring.addAccounts(1);
const addresses = keyring.getAccounts();
const address = addresses[0] as string;
const address = getAddressAtIndex(keyring, 0);
const signature = await keyring.signTypedData(address, typedData, {
version: SignTypedDataVersion.V1,
});
@@ -411,8 +416,7 @@ describe('hd-keyring', () => {
mnemonic: sampleMnemonic,
numberOfAccounts: 1,
});
const addresses = keyring.getAccounts();
const address = addresses[0] as string;
const address = getAddressAtIndex(keyring, 0);
const signature = await keyring.signTypedData(address, typedData, {
version: SignTypedDataVersion.V3,
});
@@ -469,8 +473,7 @@ describe('hd-keyring', () => {
await keyring.deserialize();
await keyring.generateRandomMnemonic();
await keyring.addAccounts(1);
const addresses = keyring.getAccounts();
const address = addresses[0] as string;
const address = getAddressAtIndex(keyring, 0);
const signature = await keyring.signTypedData(address, typedData, {
version: SignTypedDataVersion.V3,
});
@@ -674,6 +677,7 @@ describe('hd-keyring', () => {
numberOfAccounts: 1,
});

// @ts-expect-error testing invalid input
await expect(keyring.signMessage('', message)).rejects.toThrow(
'Must specify address.',
);
@@ -707,7 +711,7 @@ describe('hd-keyring', () => {
it('should remove that account', async function () {
const addresses = keyring.getAccounts();
expect(addresses).toHaveLength(1);
keyring.removeAccount(addresses[0] as string);
keyring.removeAccount(getAddressAtIndex(keyring, 0));
const addressesAfterRemoval = keyring.getAccounts();
expect(addressesAfterRemoval).toHaveLength(0);
});
@@ -838,6 +842,7 @@ describe('hd-keyring', () => {
});

it('throw error if address is blank', async function () {
// @ts-expect-error - passing an empty string to test the error
await expect(keyring.getEncryptionPublicKey('')).rejects.toThrow(
'Must specify address.',
);
@@ -916,16 +921,11 @@ describe('hd-keyring', () => {
},
};

const addresses = keyring.getAccounts();
const [address] = addresses;
const address = getAddressAtIndex(keyring, 0);

const signature = await keyring.signTypedData(
address as string,
typedData,
{
version: SignTypedDataVersion.V4,
},
);
const signature = await keyring.signTypedData(address, typedData, {
version: SignTypedDataVersion.V4,
});
expect(signature).toBe(expectedSignature);
const restored = recoverTypedSignature({
data: typedData,
@@ -1018,6 +1018,7 @@ describe('hd-keyring', () => {

it('returns rejected promise if empty address is passed', async function () {
const tx = TransactionFactory.fromTxData(txParams);
// @ts-expect-error testing invalid input
await expect(keyring.signTransaction('', tx)).rejects.toThrow(
'Must specify address.',
);
Loading