From 0e8f5004d6a53929a4cc1de9231c43899c8b6eeb Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Fri, 6 Apr 2018 18:54:38 +1000 Subject: Add Mnemonic wallet subprovider --- packages/subproviders/CHANGELOG.json | 4 + packages/subproviders/package.json | 2 + packages/subproviders/src/globals.d.ts | 2 + packages/subproviders/src/index.ts | 1 + packages/subproviders/src/subproviders/ledger.ts | 1 - .../subproviders/mnemonic_wallet_subprovider.ts | 123 ++++++++++++++ .../subproviders/private_key_wallet_subprovider.ts | 4 +- packages/subproviders/src/types.ts | 3 + .../test/unit/mnemonic_wallet_subprovider_test.ts | 182 +++++++++++++++++++++ packages/subproviders/test/utils/fixture_data.ts | 2 + 10 files changed, 320 insertions(+), 4 deletions(-) create mode 100644 packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts create mode 100644 packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts diff --git a/packages/subproviders/CHANGELOG.json b/packages/subproviders/CHANGELOG.json index f0702cd5d..ccf807695 100644 --- a/packages/subproviders/CHANGELOG.json +++ b/packages/subproviders/CHANGELOG.json @@ -5,6 +5,10 @@ { "note": "Add private key subprovider and refactor shared functionality into a base wallet subprovider", "pr": 506 + }, + { + "note": "Add mnemonic wallet subprovider, deprecating our truffle-hdwallet-provider fork", + "pr": 507 } ] }, diff --git a/packages/subproviders/package.json b/packages/subproviders/package.json index cadbac0e8..d557eef29 100644 --- a/packages/subproviders/package.json +++ b/packages/subproviders/package.json @@ -56,9 +56,11 @@ "@0xproject/monorepo-scripts": "^0.1.16", "@0xproject/tslint-config": "^0.4.14", "@0xproject/utils": "^0.5.0", + "@types/bip39": "^2.4.0", "@types/lodash": "4.14.104", "@types/mocha": "^2.2.42", "@types/node": "^8.0.53", + "bip39": "^2.5.0", "chai": "^4.0.1", "chai-as-promised": "^7.1.0", "copyfiles": "^1.2.0", diff --git a/packages/subproviders/src/globals.d.ts b/packages/subproviders/src/globals.d.ts index c5ad26876..754d164e8 100644 --- a/packages/subproviders/src/globals.d.ts +++ b/packages/subproviders/src/globals.d.ts @@ -54,7 +54,9 @@ declare module '@ledgerhq/hw-transport-node-hid' { // hdkey declarations declare module 'hdkey' { class HDNode { + public static fromMasterSeed(seed: Buffer): HDNode; public publicKey: Buffer; + public privateKey: Buffer; public chainCode: Buffer; public constructor(); public derive(path: string): HDNode; diff --git a/packages/subproviders/src/index.ts b/packages/subproviders/src/index.ts index dd553fde4..9c30c6ba1 100644 --- a/packages/subproviders/src/index.ts +++ b/packages/subproviders/src/index.ts @@ -13,6 +13,7 @@ export { GanacheSubprovider } from './subproviders/ganache'; export { Subprovider } from './subproviders/subprovider'; export { NonceTrackerSubprovider } from './subproviders/nonce_tracker'; export { PrivateKeyWalletSubprovider } from './subproviders/private_key_wallet_subprovider'; +export { MnemonicWalletSubprovider } from './subproviders/mnemonic_wallet_subprovider'; export { Callback, ErrorCallback, diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index aa86bf6c0..23ed3c93e 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -1,5 +1,4 @@ import { assert } from '@0xproject/assert'; -import { JSONRPCRequestPayload } from '@0xproject/types'; import { addressUtils } from '@0xproject/utils'; import EthereumTx = require('ethereumjs-tx'); import ethUtil = require('ethereumjs-util'); diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts new file mode 100644 index 000000000..fdb497776 --- /dev/null +++ b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts @@ -0,0 +1,123 @@ +import { assert } from '@0xproject/assert'; +import * as bip39 from 'bip39'; +import ethUtil = require('ethereumjs-util'); +import HDNode = require('hdkey'); +import * as _ from 'lodash'; + +import { MnemonicSubproviderErrors, PartialTxParams } from '../types'; + +import { BaseWalletSubprovider } from './base_wallet_subprovider'; +import { PrivateKeyWalletSubprovider } from './private_key_wallet_subprovider'; + +const DEFAULT_DERIVATION_PATH = `44'/60'/0'`; +const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; +const DEFAULT_ADDRESS_SEARCH_LIMIT = 100; + +/** + * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. + * This subprovider intercepts all account related RPC requests (e.g message/transaction signing, etc...) and handles + * all requests with accounts derived from the supplied mnemonic. + */ +export class MnemonicWalletSubprovider extends BaseWalletSubprovider { + private _derivationPath: string; + private _hdKey: HDNode; + private _derivationPathIndex: number; + constructor(mnemonic: string, derivationPath: string = DEFAULT_DERIVATION_PATH) { + assert.isString('mnemonic', mnemonic); + super(); + this._hdKey = HDNode.fromMasterSeed(bip39.mnemonicToSeed(mnemonic)); + this._derivationPathIndex = 0; + this._derivationPath = derivationPath; + } + /** + * Retrieve the set derivation path + * @returns derivation path + */ + public getPath(): string { + return this._derivationPath; + } + /** + * Set a desired derivation path when computing the available user addresses + * @param derivationPath The desired derivation path (e.g `44'/60'/0'`) + */ + public setPath(derivationPath: string) { + this._derivationPath = derivationPath; + } + /** + * Set the final derivation path index. If a user wishes to sign a message with the + * 6th address in a derivation path, before calling `signPersonalMessageAsync`, you must + * call this method with pathIndex `6`. + * @param pathIndex Desired derivation path index + */ + public setPathIndex(pathIndex: number) { + this._derivationPathIndex = pathIndex; + } + /** + * Retrieve the account associated with the supplied private key. + * This method is implicitly called when issuing a `eth_accounts` JSON RPC request + * via your providerEngine instance. + * @return An array of accounts + */ + public async getAccountsAsync(numberOfAccounts: number = DEFAULT_NUM_ADDRESSES_TO_FETCH): Promise { + const accounts: string[] = []; + for (let i = 0; i < numberOfAccounts; i++) { + const derivedHDNode = this._hdKey.derive(`m/${this._derivationPath}/${i + this._derivationPathIndex}`); + const derivedPublicKey = derivedHDNode.publicKey; + const shouldSanitizePublicKey = true; + const ethereumAddressUnprefixed = ethUtil + .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) + .toString('hex'); + const ethereumAddressPrefixed = ethUtil.addHexPrefix(ethereumAddressUnprefixed); + accounts.push(ethereumAddressPrefixed.toLowerCase()); + } + return accounts; + } + + /** + * Signs a transaction with the from account (if specificed in txParams) or the first account. + * If you've added this Subprovider to your app's provider, you can simply send + * an `eth_sendTransaction` JSON RPC request, and * this method will be called auto-magically. + * If you are not using this via a ProviderEngine instance, you can call it directly. + * @param txParams Parameters of the transaction to sign + * @return Signed transaction hex string + */ + public async signTransactionAsync(txParams: PartialTxParams): Promise { + const accounts = await this.getAccountsAsync(); + const hdKey = this._findHDKeyByPublicAddress(txParams.from || accounts[0]); + const privateKeyWallet = new PrivateKeyWalletSubprovider(hdKey.privateKey.toString('hex')); + const signedTx = privateKeyWallet.signTransactionAsync(txParams); + return signedTx; + } + /** + * Sign a personal Ethereum signed message. The signing address will be + * derived from the set path. + * If you've added the PKWalletSubprovider to your app's provider, you can simply send an `eth_sign` + * or `personal_sign` JSON RPC request, and this method will be called auto-magically. + * If you are not using this via a ProviderEngine instance, you can call it directly. + * @param data Message to sign + * @return Signature hex string (order: rsv) + */ + public async signPersonalMessageAsync(data: string): Promise { + const accounts = await this.getAccountsAsync(); + const hdKey = this._findHDKeyByPublicAddress(accounts[0]); + const privateKeyWallet = new PrivateKeyWalletSubprovider(hdKey.privateKey.toString('hex')); + const sig = await privateKeyWallet.signPersonalMessageAsync(data); + return sig; + } + + private _findHDKeyByPublicAddress(address: string, searchLimit: number = DEFAULT_ADDRESS_SEARCH_LIMIT): HDNode { + for (let i = 0; i < searchLimit; i++) { + const derivedHDNode = this._hdKey.derive(`m/${this._derivationPath}/${i + this._derivationPathIndex}`); + const derivedPublicKey = derivedHDNode.publicKey; + const shouldSanitizePublicKey = true; + const ethereumAddressUnprefixed = ethUtil + .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) + .toString('hex'); + const ethereumAddressPrefixed = ethUtil.addHexPrefix(ethereumAddressUnprefixed); + if (ethereumAddressPrefixed === address) { + return derivedHDNode; + } + } + throw new Error(MnemonicSubproviderErrors.AddressSearchExhausted); + } +} diff --git a/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts b/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts index c3a53773a..0aa2fb590 100644 --- a/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts @@ -1,13 +1,11 @@ import { assert } from '@0xproject/assert'; -import { JSONRPCRequestPayload } from '@0xproject/types'; import EthereumTx = require('ethereumjs-tx'); import * as ethUtil from 'ethereumjs-util'; import * as _ from 'lodash'; -import { Callback, ErrorCallback, PartialTxParams, ResponseWithTxParams, WalletSubproviderErrors } from '../types'; +import { PartialTxParams, WalletSubproviderErrors } from '../types'; import { BaseWalletSubprovider } from './base_wallet_subprovider'; -import { Subprovider } from './subprovider'; /** * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts index bacb7091b..de04499ce 100644 --- a/packages/subproviders/src/types.ts +++ b/packages/subproviders/src/types.ts @@ -95,6 +95,9 @@ export interface ResponseWithTxParams { tx: PartialTxParams; } +export enum MnemonicSubproviderErrors { + AddressSearchExhausted = 'ADDRESS_SEARCH_EXHAUSTED', +} export enum WalletSubproviderErrors { DataMissingForSignPersonalMessage = 'DATA_MISSING_FOR_SIGN_PERSONAL_MESSAGE', SenderInvalidOrNotSupplied = 'SENDER_INVALID_OR_NOT_SUPPLIED', diff --git a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts new file mode 100644 index 000000000..e58461005 --- /dev/null +++ b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts @@ -0,0 +1,182 @@ +import { JSONRPCResponsePayload } from '@0xproject/types'; +import * as chai from 'chai'; +import * as ethUtils from 'ethereumjs-util'; +import * as _ from 'lodash'; +import Web3ProviderEngine = require('web3-provider-engine'); + +import { GanacheSubprovider, MnemonicWalletSubprovider } from '../../src/'; +import { + DoneCallback, + LedgerCommunicationClient, + LedgerSubproviderErrors, + MnemonicSubproviderErrors, + WalletSubproviderErrors, +} from '../../src/types'; +import { chaiSetup } from '../chai_setup'; +import { fixtureData } from '../utils/fixture_data'; +import { reportCallbackErrors } from '../utils/report_callback_errors'; + +chaiSetup.configure(); +const expect = chai.expect; + +describe('MnemonicWalletSubprovider', () => { + let subprovider: MnemonicWalletSubprovider; + before(async () => { + subprovider = new MnemonicWalletSubprovider( + fixtureData.TEST_RPC_MNEMONIC, + fixtureData.TEST_RPC_MNEMONIC_DERIVATION_PATH, + ); + }); + describe('direct method calls', () => { + describe('success cases', () => { + it('returns the account', async () => { + const accounts = await subprovider.getAccountsAsync(); + expect(accounts[0]).to.be.equal(fixtureData.TEST_RPC_ACCOUNT_0); + expect(accounts.length).to.be.equal(10); + }); + it('signs a personal message', async () => { + const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); + const ecSignatureHex = await subprovider.signPersonalMessageAsync(data); + expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT); + }); + it('signs a transaction', async () => { + const txHex = await subprovider.signTransactionAsync(fixtureData.TX_DATA); + expect(txHex).to.be.equal(fixtureData.TX_DATA_SIGNED_RESULT); + }); + }); + describe('failure cases', () => { + it('throws an error if account cannot be found', async () => { + const txData = { ...fixtureData.TX_DATA, from: '0x0' }; + return expect(subprovider.signTransactionAsync(txData)).to.be.rejectedWith( + MnemonicSubproviderErrors.AddressSearchExhausted, + ); + }); + }); + }); + describe('calls through a provider', () => { + let provider: Web3ProviderEngine; + before(() => { + provider = new Web3ProviderEngine(); + provider.addProvider(subprovider); + const ganacheSubprovider = new GanacheSubprovider({}); + provider.addProvider(ganacheSubprovider); + provider.start(); + }); + describe('success cases', () => { + it('returns a list of accounts', (done: DoneCallback) => { + const payload = { + jsonrpc: '2.0', + method: 'eth_accounts', + params: [], + id: 1, + }; + const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { + expect(err).to.be.a('null'); + expect(response.result[0]).to.be.equal(fixtureData.TEST_RPC_ACCOUNT_0); + expect(response.result.length).to.be.equal(10); + done(); + }); + provider.sendAsync(payload, callback); + }); + it('signs a personal message with eth_sign', (done: DoneCallback) => { + const messageHex = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); + const payload = { + jsonrpc: '2.0', + method: 'eth_sign', + params: ['0x0000000000000000000000000000000000000000', messageHex], + id: 1, + }; + const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { + expect(err).to.be.a('null'); + expect(response.result).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT); + done(); + }); + provider.sendAsync(payload, callback); + }); + it('signs a personal message with personal_sign', (done: DoneCallback) => { + const messageHex = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); + const payload = { + jsonrpc: '2.0', + method: 'personal_sign', + params: [messageHex, '0x0000000000000000000000000000000000000000'], + id: 1, + }; + const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { + expect(err).to.be.a('null'); + expect(response.result).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT); + done(); + }); + provider.sendAsync(payload, callback); + }); + }); + describe('failure cases', () => { + it('should throw if `data` param not hex when calling eth_sign', (done: DoneCallback) => { + const nonHexMessage = 'hello world'; + const payload = { + jsonrpc: '2.0', + method: 'eth_sign', + params: ['0x0000000000000000000000000000000000000000', nonHexMessage], + id: 1, + }; + const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { + expect(err).to.not.be.a('null'); + expect(err.message).to.be.equal('Expected data to be of type HexString, encountered: hello world'); + done(); + }); + provider.sendAsync(payload, callback); + }); + it('should throw if `data` param not hex when calling personal_sign', (done: DoneCallback) => { + const nonHexMessage = 'hello world'; + const payload = { + jsonrpc: '2.0', + method: 'personal_sign', + params: [nonHexMessage, '0x0000000000000000000000000000000000000000'], + id: 1, + }; + const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { + expect(err).to.not.be.a('null'); + expect(err.message).to.be.equal('Expected data to be of type HexString, encountered: hello world'); + done(); + }); + provider.sendAsync(payload, callback); + }); + it('should throw if `from` param missing when calling eth_sendTransaction', (done: DoneCallback) => { + const tx = { + to: '0xafa3f8684e54059998bc3a7b0d2b0da075154d66', + value: '0xde0b6b3a7640000', + }; + const payload = { + jsonrpc: '2.0', + method: 'eth_sendTransaction', + params: [tx], + id: 1, + }; + const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { + expect(err).to.not.be.a('null'); + expect(err.message).to.be.equal(WalletSubproviderErrors.SenderInvalidOrNotSupplied); + done(); + }); + provider.sendAsync(payload, callback); + }); + it('should throw if `from` param invalid address when calling eth_sendTransaction', (done: DoneCallback) => { + const tx = { + to: '0xafa3f8684e54059998bc3a7b0d2b0da075154d66', + from: '0xIncorrectEthereumAddress', + value: '0xde0b6b3a7640000', + }; + const payload = { + jsonrpc: '2.0', + method: 'eth_sendTransaction', + params: [tx], + id: 1, + }; + const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { + expect(err).to.not.be.a('null'); + expect(err.message).to.be.equal(WalletSubproviderErrors.SenderInvalidOrNotSupplied); + done(); + }); + provider.sendAsync(payload, callback); + }); + }); + }); +}); diff --git a/packages/subproviders/test/utils/fixture_data.ts b/packages/subproviders/test/utils/fixture_data.ts index 890573d0d..5ce3ff08f 100644 --- a/packages/subproviders/test/utils/fixture_data.ts +++ b/packages/subproviders/test/utils/fixture_data.ts @@ -3,6 +3,8 @@ const networkId = 42; export const fixtureData = { TEST_RPC_ACCOUNT_0, TEST_RPC_ACCOUNT_0_ACCOUNT_PRIVATE_KEY: 'F2F48EE19680706196E2E339E5DA3491186E0C4C5030670656B0E0164837257D', + TEST_RPC_MNEMONIC: 'concert load couple harbor equip island argue ramp clarify fence smart topic', + TEST_RPC_MNEMONIC_DERIVATION_PATH: `44'/60'/0'/0`, PERSONAL_MESSAGE_STRING: 'hello world', PERSONAL_MESSAGE_SIGNED_RESULT: '0x1b0ec5e2908e993d0c8ab6b46da46be2688fdf03c7ea6686075de37392e50a7d7fcc531446699132fbda915bd989882e0064d417018773a315fb8d43ed063c9b00', -- cgit From 5b69cd4a22ce1720f4441aaa74c86f895015c0fd Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Tue, 10 Apr 2018 11:58:12 +1000 Subject: Added walletUtils and address in signMessage --- .../src/subproviders/base_wallet_subprovider.ts | 5 +- .../subproviders/mnemonic_wallet_subprovider.ts | 85 ++++++++++------------ .../subproviders/private_key_wallet_subprovider.ts | 6 +- packages/subproviders/src/types.ts | 10 ++- packages/subproviders/src/walletUtils.ts | 58 +++++++++++++++ .../test/unit/mnemonic_wallet_subprovider_test.ts | 26 +++++-- .../unit/private_key_wallet_subprovider_test.ts | 23 +++++- 7 files changed, 149 insertions(+), 64 deletions(-) create mode 100644 packages/subproviders/src/walletUtils.ts diff --git a/packages/subproviders/src/subproviders/base_wallet_subprovider.ts b/packages/subproviders/src/subproviders/base_wallet_subprovider.ts index 034f83e7f..47b45a126 100644 --- a/packages/subproviders/src/subproviders/base_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/base_wallet_subprovider.ts @@ -21,7 +21,7 @@ export abstract class BaseWalletSubprovider extends Subprovider { public abstract async getAccountsAsync(): Promise; public abstract async signTransactionAsync(txParams: PartialTxParams): Promise; - public abstract async signPersonalMessageAsync(data: string): Promise; + public abstract async signPersonalMessageAsync(data: string, address?: string): Promise; /** * This method conforms to the web3-provider-engine interface. @@ -85,8 +85,9 @@ export abstract class BaseWalletSubprovider extends Subprovider { case 'eth_sign': case 'personal_sign': const data = payload.method === 'eth_sign' ? payload.params[1] : payload.params[0]; + const address = payload.method === 'eth_sign' ? payload.params[0] : payload.params[1]; try { - const ecSignatureHex = await this.signPersonalMessageAsync(data); + const ecSignatureHex = await this.signPersonalMessageAsync(data, address); end(null, ecSignatureHex); } catch (err) { end(err); diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts index fdb497776..456bde05c 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts @@ -4,14 +4,15 @@ import ethUtil = require('ethereumjs-util'); import HDNode = require('hdkey'); import * as _ from 'lodash'; -import { MnemonicSubproviderErrors, PartialTxParams } from '../types'; +import { DerivedHDKey, PartialTxParams, WalletSubproviderErrors } from '../types'; +import { walletUtils } from '../walletUtils'; import { BaseWalletSubprovider } from './base_wallet_subprovider'; import { PrivateKeyWalletSubprovider } from './private_key_wallet_subprovider'; const DEFAULT_DERIVATION_PATH = `44'/60'/0'`; const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; -const DEFAULT_ADDRESS_SEARCH_LIMIT = 100; +const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; /** * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. @@ -19,15 +20,22 @@ const DEFAULT_ADDRESS_SEARCH_LIMIT = 100; * all requests with accounts derived from the supplied mnemonic. */ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { + private _addressSearchLimit: number; private _derivationPath: string; private _hdKey: HDNode; - private _derivationPathIndex: number; - constructor(mnemonic: string, derivationPath: string = DEFAULT_DERIVATION_PATH) { + + constructor( + mnemonic: string, + derivationPath: string = DEFAULT_DERIVATION_PATH, + addressSearchLimit: number = DEFAULT_ADDRESS_SEARCH_LIMIT, + ) { assert.isString('mnemonic', mnemonic); + assert.isString('derivationPath', derivationPath); + assert.isNumber('addressSearchLimit', addressSearchLimit); super(); this._hdKey = HDNode.fromMasterSeed(bip39.mnemonicToSeed(mnemonic)); - this._derivationPathIndex = 0; this._derivationPath = derivationPath; + this._addressSearchLimit = addressSearchLimit; } /** * Retrieve the set derivation path @@ -44,32 +52,14 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { this._derivationPath = derivationPath; } /** - * Set the final derivation path index. If a user wishes to sign a message with the - * 6th address in a derivation path, before calling `signPersonalMessageAsync`, you must - * call this method with pathIndex `6`. - * @param pathIndex Desired derivation path index - */ - public setPathIndex(pathIndex: number) { - this._derivationPathIndex = pathIndex; - } - /** - * Retrieve the account associated with the supplied private key. + * Retrieve the accounts associated with the mnemonic. * This method is implicitly called when issuing a `eth_accounts` JSON RPC request * via your providerEngine instance. * @return An array of accounts */ public async getAccountsAsync(numberOfAccounts: number = DEFAULT_NUM_ADDRESSES_TO_FETCH): Promise { - const accounts: string[] = []; - for (let i = 0; i < numberOfAccounts; i++) { - const derivedHDNode = this._hdKey.derive(`m/${this._derivationPath}/${i + this._derivationPathIndex}`); - const derivedPublicKey = derivedHDNode.publicKey; - const shouldSanitizePublicKey = true; - const ethereumAddressUnprefixed = ethUtil - .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) - .toString('hex'); - const ethereumAddressPrefixed = ethUtil.addHexPrefix(ethereumAddressUnprefixed); - accounts.push(ethereumAddressPrefixed.toLowerCase()); - } + const derivedKeys = walletUtils._calculateDerivedHDKeys(this._hdKey, this._derivationPath, numberOfAccounts); + const accounts = _.map(derivedKeys, 'address'); return accounts; } @@ -82,9 +72,10 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @return Signed transaction hex string */ public async signTransactionAsync(txParams: PartialTxParams): Promise { - const accounts = await this.getAccountsAsync(); - const hdKey = this._findHDKeyByPublicAddress(txParams.from || accounts[0]); - const privateKeyWallet = new PrivateKeyWalletSubprovider(hdKey.privateKey.toString('hex')); + const derivedKey = _.isUndefined(txParams.from) + ? walletUtils._firstDerivedKey(this._hdKey, this._derivationPath) + : this._findDerivedKeyByPublicAddress(txParams.from); + const privateKeyWallet = new PrivateKeyWalletSubprovider(derivedKey.hdKey.privateKey.toString('hex')); const signedTx = privateKeyWallet.signTransactionAsync(txParams); return signedTx; } @@ -95,29 +86,27 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * or `personal_sign` JSON RPC request, and this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. * @param data Message to sign + * @param address Address to sign with * @return Signature hex string (order: rsv) */ - public async signPersonalMessageAsync(data: string): Promise { - const accounts = await this.getAccountsAsync(); - const hdKey = this._findHDKeyByPublicAddress(accounts[0]); - const privateKeyWallet = new PrivateKeyWalletSubprovider(hdKey.privateKey.toString('hex')); - const sig = await privateKeyWallet.signPersonalMessageAsync(data); + public async signPersonalMessageAsync(data: string, address?: string): Promise { + const derivedKey = _.isUndefined(address) + ? walletUtils._firstDerivedKey(this._hdKey, this._derivationPath) + : this._findDerivedKeyByPublicAddress(address); + const privateKeyWallet = new PrivateKeyWalletSubprovider(derivedKey.hdKey.privateKey.toString('hex')); + const sig = await privateKeyWallet.signPersonalMessageAsync(data, derivedKey.address); return sig; } - - private _findHDKeyByPublicAddress(address: string, searchLimit: number = DEFAULT_ADDRESS_SEARCH_LIMIT): HDNode { - for (let i = 0; i < searchLimit; i++) { - const derivedHDNode = this._hdKey.derive(`m/${this._derivationPath}/${i + this._derivationPathIndex}`); - const derivedPublicKey = derivedHDNode.publicKey; - const shouldSanitizePublicKey = true; - const ethereumAddressUnprefixed = ethUtil - .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) - .toString('hex'); - const ethereumAddressPrefixed = ethUtil.addHexPrefix(ethereumAddressUnprefixed); - if (ethereumAddressPrefixed === address) { - return derivedHDNode; - } + private _findDerivedKeyByPublicAddress(address: string): DerivedHDKey { + const matchedDerivedKey = walletUtils._findDerivedKeyByAddress( + address, + this._hdKey, + this._derivationPath, + this._addressSearchLimit, + ); + if (_.isUndefined(matchedDerivedKey)) { + throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`); } - throw new Error(MnemonicSubproviderErrors.AddressSearchExhausted); + return matchedDerivedKey; } } diff --git a/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts b/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts index 0aa2fb590..f6906bab6 100644 --- a/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts @@ -52,12 +52,16 @@ export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { * or `personal_sign` JSON RPC request, and this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. * @param data Message to sign + * @param address Address to sign with * @return Signature hex string (order: rsv) */ - public async signPersonalMessageAsync(dataIfExists: string): Promise { + public async signPersonalMessageAsync(dataIfExists: string, address?: string): Promise { if (_.isUndefined(dataIfExists)) { throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage); } + if (!_.isUndefined(address) && address !== this._address) { + throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`); + } assert.isHexString('data', dataIfExists); const dataBuff = ethUtil.toBuffer(dataIfExists); const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff); diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts index de04499ce..105ffa7cc 100644 --- a/packages/subproviders/src/types.ts +++ b/packages/subproviders/src/types.ts @@ -1,4 +1,5 @@ import { ECSignature, JSONRPCRequestPayload } from '@0xproject/types'; +import HDNode = require('hdkey'); import * as _ from 'lodash'; export interface LedgerCommunicationClient { @@ -95,10 +96,8 @@ export interface ResponseWithTxParams { tx: PartialTxParams; } -export enum MnemonicSubproviderErrors { - AddressSearchExhausted = 'ADDRESS_SEARCH_EXHAUSTED', -} export enum WalletSubproviderErrors { + AddressNotFound = 'ADDRESS_NOT_FOUND', DataMissingForSignPersonalMessage = 'DATA_MISSING_FOR_SIGN_PERSONAL_MESSAGE', SenderInvalidOrNotSupplied = 'SENDER_INVALID_OR_NOT_SUPPLIED', } @@ -112,6 +111,11 @@ export enum NonceSubproviderErrors { EmptyParametersFound = 'EMPTY_PARAMETERS_FOUND', CannotDetermineAddressFromPayload = 'CANNOT_DETERMINE_ADDRESS_FROM_PAYLOAD', } +export interface DerivedHDKey { + address: string; + derivationPath: string; + hdKey: HDNode; +} export type ErrorCallback = (err: Error | null, data?: any) => void; export type Callback = () => void; diff --git a/packages/subproviders/src/walletUtils.ts b/packages/subproviders/src/walletUtils.ts new file mode 100644 index 000000000..631636a71 --- /dev/null +++ b/packages/subproviders/src/walletUtils.ts @@ -0,0 +1,58 @@ +import ethUtil = require('ethereumjs-util'); +import HDNode = require('hdkey'); +import * as _ from 'lodash'; + +import { DerivedHDKey, WalletSubproviderErrors } from './types'; + +const DEFAULT_ADDRESS_SEARCH_OFFSET = 0; +const BATCH_SIZE = 10; +export const walletUtils = { + _calculateDerivedHDKeys( + initialHDKey: HDNode, + derivationPath: string, + searchLimit: number, + offset: number = DEFAULT_ADDRESS_SEARCH_OFFSET, + ): DerivedHDKey[] { + const derivedKeys: DerivedHDKey[] = []; + _.times(searchLimit, i => { + const path = `m/${derivationPath}/${i + offset}`; + const hdKey = initialHDKey.derive(path); + const derivedPublicKey = hdKey.publicKey; + const shouldSanitizePublicKey = true; + const ethereumAddressUnprefixed = ethUtil + .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) + .toString('hex'); + const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed); + const derivedKey: DerivedHDKey = { + derivationPath: path, + hdKey, + address, + }; + derivedKeys.push(derivedKey); + }); + return derivedKeys; + }, + + _findDerivedKeyByAddress( + address: string, + initialHDKey: HDNode, + derivationPath: string, + searchLimit: number, + ): DerivedHDKey | undefined { + let matchedKey: DerivedHDKey | undefined; + for (let index = 0; index < searchLimit; index = index + BATCH_SIZE) { + const derivedKeys = walletUtils._calculateDerivedHDKeys(initialHDKey, derivationPath, BATCH_SIZE, index); + matchedKey = _.find(derivedKeys, derivedKey => derivedKey.address === address); + if (matchedKey) { + break; + } + } + return matchedKey; + }, + + _firstDerivedKey(initialHDKey: HDNode, derivationPath: string): DerivedHDKey { + const derivedKeys = walletUtils._calculateDerivedHDKeys(initialHDKey, derivationPath, 1); + const firstDerivedKey = derivedKeys[0]; + return firstDerivedKey; + }, +}; diff --git a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts index e58461005..7aaef4944 100644 --- a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts @@ -9,7 +9,6 @@ import { DoneCallback, LedgerCommunicationClient, LedgerSubproviderErrors, - MnemonicSubproviderErrors, WalletSubproviderErrors, } from '../../src/types'; import { chaiSetup } from '../chai_setup'; @@ -48,7 +47,7 @@ describe('MnemonicWalletSubprovider', () => { it('throws an error if account cannot be found', async () => { const txData = { ...fixtureData.TX_DATA, from: '0x0' }; return expect(subprovider.signTransactionAsync(txData)).to.be.rejectedWith( - MnemonicSubproviderErrors.AddressSearchExhausted, + WalletSubproviderErrors.AddressNotFound, ); }); }); @@ -83,7 +82,7 @@ describe('MnemonicWalletSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'eth_sign', - params: ['0x0000000000000000000000000000000000000000', messageHex], + params: [fixtureData.TEST_RPC_ACCOUNT_0, messageHex], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { @@ -98,7 +97,7 @@ describe('MnemonicWalletSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'personal_sign', - params: [messageHex, '0x0000000000000000000000000000000000000000'], + params: [messageHex, fixtureData.TEST_RPC_ACCOUNT_0], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { @@ -115,7 +114,7 @@ describe('MnemonicWalletSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'eth_sign', - params: ['0x0000000000000000000000000000000000000000', nonHexMessage], + params: [fixtureData.TEST_RPC_ACCOUNT_0, nonHexMessage], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { @@ -130,7 +129,7 @@ describe('MnemonicWalletSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'personal_sign', - params: [nonHexMessage, '0x0000000000000000000000000000000000000000'], + params: [nonHexMessage, fixtureData.TEST_RPC_ACCOUNT_0], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { @@ -140,6 +139,21 @@ describe('MnemonicWalletSubprovider', () => { }); provider.sendAsync(payload, callback); }); + it('should throw if `address` param not found when calling personal_sign', (done: DoneCallback) => { + const nonHexMessage = 'hello world'; + const payload = { + jsonrpc: '2.0', + method: 'personal_sign', + params: [nonHexMessage, '0x0'], + id: 1, + }; + const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { + expect(err).to.not.be.a('null'); + expect(err.message).to.be.equal(`${WalletSubproviderErrors.AddressNotFound}: 0x0`); + done(); + }); + provider.sendAsync(payload, callback); + }); it('should throw if `from` param missing when calling eth_sendTransaction', (done: DoneCallback) => { const tx = { to: '0xafa3f8684e54059998bc3a7b0d2b0da075154d66', diff --git a/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts b/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts index ca0665871..8aaddeaf4 100644 --- a/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts @@ -71,7 +71,7 @@ describe('PrivateKeyWalletSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'eth_sign', - params: ['0x0000000000000000000000000000000000000000', messageHex], + params: [fixtureData.TEST_RPC_ACCOUNT_0, messageHex], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { @@ -86,7 +86,7 @@ describe('PrivateKeyWalletSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'personal_sign', - params: [messageHex, '0x0000000000000000000000000000000000000000'], + params: [messageHex, fixtureData.TEST_RPC_ACCOUNT_0], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { @@ -103,7 +103,7 @@ describe('PrivateKeyWalletSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'eth_sign', - params: ['0x0000000000000000000000000000000000000000', nonHexMessage], + params: [fixtureData.TEST_RPC_ACCOUNT_0, nonHexMessage], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { @@ -118,7 +118,7 @@ describe('PrivateKeyWalletSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'personal_sign', - params: [nonHexMessage, '0x0000000000000000000000000000000000000000'], + params: [nonHexMessage, fixtureData.TEST_RPC_ACCOUNT_0], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { @@ -165,6 +165,21 @@ describe('PrivateKeyWalletSubprovider', () => { }); provider.sendAsync(payload, callback); }); + it('should throw if `address` param not found when calling personal_sign', (done: DoneCallback) => { + const nonHexMessage = 'hello world'; + const payload = { + jsonrpc: '2.0', + method: 'personal_sign', + params: [nonHexMessage, '0x0'], + id: 1, + }; + const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { + expect(err).to.not.be.a('null'); + expect(err.message).to.be.equal(`${WalletSubproviderErrors.AddressNotFound}: 0x0`); + done(); + }); + provider.sendAsync(payload, callback); + }); }); }); }); -- cgit From 84a4b7d1c16d008ffbf07ba5b22c61b025b5165f Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Tue, 10 Apr 2018 14:39:43 +1000 Subject: Refactor ledger to support multiple accounts with from address --- packages/subproviders/src/subproviders/ledger.ts | 111 ++++++++++++--------- .../subproviders/mnemonic_wallet_subprovider.ts | 5 +- packages/subproviders/src/types.ts | 1 + packages/subproviders/src/utils/wallet_utils.ts | 76 ++++++++++++++ packages/subproviders/src/walletUtils.ts | 58 ----------- .../test/integration/ledger_subprovider_test.ts | 3 +- .../test/unit/ledger_subprovider_test.ts | 8 +- .../test/unit/mnemonic_wallet_subprovider_test.ts | 5 +- packages/subproviders/test/utils/fixture_data.ts | 2 + 9 files changed, 155 insertions(+), 114 deletions(-) create mode 100644 packages/subproviders/src/utils/wallet_utils.ts delete mode 100644 packages/subproviders/src/walletUtils.ts diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 23ed3c93e..a7b79c128 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -8,6 +8,7 @@ import { Lock } from 'semaphore-async-await'; import { Callback, + DerivedHDKey, LedgerEthereumClient, LedgerEthereumClientFactoryAsync, LedgerSubproviderConfigs, @@ -16,6 +17,7 @@ import { ResponseWithTxParams, WalletSubproviderErrors, } from '../types'; +import { walletUtils } from '../utils/wallet_utils'; import { BaseWalletSubprovider } from './base_wallet_subprovider'; @@ -34,10 +36,11 @@ export class LedgerSubprovider extends BaseWalletSubprovider { private _connectionLock = new Lock(); private _networkId: number; private _derivationPath: string; - private _derivationPathIndex: number; private _ledgerEthereumClientFactoryAsync: LedgerEthereumClientFactoryAsync; private _ledgerClientIfExists?: LedgerEthereumClient; private _shouldAlwaysAskForConfirmation: boolean; + private _addressSearchLimit: number; + private _hardenedKey: boolean = true; /** * Instantiates a LedgerSubprovider. Defaults to derivationPath set to `44'/60'/0'`. * TestRPC/Ganache defaults to `m/44'/60'/0'/0`, so set this in the configs if desired. @@ -54,7 +57,11 @@ export class LedgerSubprovider extends BaseWalletSubprovider { !_.isUndefined(config.accountFetchingConfigs.shouldAskForOnDeviceConfirmation) ? config.accountFetchingConfigs.shouldAskForOnDeviceConfirmation : ASK_FOR_ON_DEVICE_CONFIRMATION; - this._derivationPathIndex = 0; + this._addressSearchLimit = + !_.isUndefined(config.accountFetchingConfigs) && + !_.isUndefined(config.accountFetchingConfigs.numAddressesToReturn) + ? config.accountFetchingConfigs.numAddressesToReturn + : DEFAULT_NUM_ADDRESSES_TO_FETCH; } /** * Retrieve the set derivation path @@ -70,15 +77,6 @@ export class LedgerSubprovider extends BaseWalletSubprovider { public setPath(derivationPath: string) { this._derivationPath = derivationPath; } - /** - * Set the final derivation path index. If a user wishes to sign a message with the - * 6th address in a derivation path, before calling `signPersonalMessageAsync`, you must - * call this method with pathIndex `6`. - * @param pathIndex Desired derivation path index - */ - public setPathIndex(pathIndex: number) { - this._derivationPathIndex = pathIndex; - } /** * Retrieve a users Ledger accounts. The accounts are derived from the derivationPath, * master public key and chainCode. Because of this, you can request as many accounts @@ -89,34 +87,15 @@ export class LedgerSubprovider extends BaseWalletSubprovider { * @return An array of accounts */ public async getAccountsAsync(numberOfAccounts: number = DEFAULT_NUM_ADDRESSES_TO_FETCH): Promise { - this._ledgerClientIfExists = await this._createLedgerClientAsync(); - - let ledgerResponse; - try { - ledgerResponse = await this._ledgerClientIfExists.getAddress( - this._derivationPath, - this._shouldAlwaysAskForConfirmation, - SHOULD_GET_CHAIN_CODE, - ); - } finally { - await this._destroyLedgerClientAsync(); - } - - const hdKey = new HDNode(); - hdKey.publicKey = new Buffer(ledgerResponse.publicKey, 'hex'); - hdKey.chainCode = new Buffer(ledgerResponse.chainCode, 'hex'); - - const accounts: string[] = []; - for (let i = 0; i < numberOfAccounts; i++) { - const derivedHDNode = hdKey.derive(`m/${i + this._derivationPathIndex}`); - const derivedPublicKey = derivedHDNode.publicKey; - const shouldSanitizePublicKey = true; - const ethereumAddressUnprefixed = ethUtil - .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) - .toString('hex'); - const ethereumAddressPrefixed = ethUtil.addHexPrefix(ethereumAddressUnprefixed); - accounts.push(ethereumAddressPrefixed.toLowerCase()); - } + const initialHDKey = await this._initialHDKeyAsync(); + const derivedKeys = walletUtils._calculateDerivedHDKeys( + initialHDKey, + this._derivationPath, + numberOfAccounts, + 0, + true, + ); + const accounts = _.map(derivedKeys, 'address'); return accounts; } /** @@ -129,6 +108,11 @@ export class LedgerSubprovider extends BaseWalletSubprovider { */ public async signTransactionAsync(txParams: PartialTxParams): Promise { LedgerSubprovider._validateTxParams(txParams); + const initialHDKey = await this._initialHDKeyAsync(); + const derivedKey = _.isUndefined(txParams.from) + ? walletUtils._firstDerivedKey(initialHDKey, this._derivationPath, this._hardenedKey) + : this._findDerivedKeyByPublicAddress(initialHDKey, txParams.from); + this._ledgerClientIfExists = await this._createLedgerClientAsync(); const tx = new EthereumTx(txParams); @@ -140,7 +124,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { const txHex = tx.serialize().toString('hex'); try { - const derivationPath = this._getDerivationPath(); + const derivationPath = `${derivedKey.derivationPath}/${derivedKey.derivationIndex}`; const result = await this._ledgerClientIfExists.signTransaction(derivationPath, txHex); // Store signature in transaction tx.r = Buffer.from(result.r, 'hex'); @@ -165,22 +149,28 @@ export class LedgerSubprovider extends BaseWalletSubprovider { } /** * Sign a personal Ethereum signed message. The signing address will be the one - * retrieved given a derivationPath and pathIndex set on the subprovider. + * either the provided address or the first address on the ledger device. * The Ledger adds the Ethereum signed message prefix on-device. If you've added * the LedgerSubprovider to your app's provider, you can simply send an `eth_sign` * or `personal_sign` JSON RPC request, and this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. * @param data Message to sign + * @param address Address to sign with * @return Signature hex string (order: rsv) */ - public async signPersonalMessageAsync(data: string): Promise { + public async signPersonalMessageAsync(data: string, address?: string): Promise { if (_.isUndefined(data)) { throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage); } assert.isHexString('data', data); + const initialHDKey = await this._initialHDKeyAsync(); + const derivedKey = _.isUndefined(address) + ? walletUtils._firstDerivedKey(initialHDKey, this._derivationPath, this._hardenedKey) + : this._findDerivedKeyByPublicAddress(initialHDKey, address); + this._ledgerClientIfExists = await this._createLedgerClientAsync(); try { - const derivationPath = this._getDerivationPath(); + const derivationPath = `${derivedKey.derivationPath}/${derivedKey.derivationIndex}`; const result = await this._ledgerClientIfExists.signPersonalMessage( derivationPath, ethUtil.stripHexPrefix(data), @@ -198,10 +188,6 @@ export class LedgerSubprovider extends BaseWalletSubprovider { throw err; } } - private _getDerivationPath() { - const derivationPath = `${this.getPath()}/${this._derivationPathIndex}`; - return derivationPath; - } private async _createLedgerClientAsync(): Promise { await this._connectionLock.acquire(); if (!_.isUndefined(this._ledgerClientIfExists)) { @@ -222,4 +208,35 @@ export class LedgerSubprovider extends BaseWalletSubprovider { this._ledgerClientIfExists = undefined; this._connectionLock.release(); } + private async _initialHDKeyAsync(): Promise { + this._ledgerClientIfExists = await this._createLedgerClientAsync(); + + let ledgerResponse; + try { + ledgerResponse = await this._ledgerClientIfExists.getAddress( + this._derivationPath, + this._shouldAlwaysAskForConfirmation, + SHOULD_GET_CHAIN_CODE, + ); + } finally { + await this._destroyLedgerClientAsync(); + } + const hdKey = new HDNode(); + hdKey.publicKey = new Buffer(ledgerResponse.publicKey, 'hex'); + hdKey.chainCode = new Buffer(ledgerResponse.chainCode, 'hex'); + return hdKey; + } + private _findDerivedKeyByPublicAddress(initalHDKey: HDNode, address: string): DerivedHDKey { + const matchedDerivedKey = walletUtils._findDerivedKeyByAddress( + address, + initalHDKey, + this._derivationPath, + this._addressSearchLimit, + this._hardenedKey, + ); + if (_.isUndefined(matchedDerivedKey)) { + throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`); + } + return matchedDerivedKey; + } } diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts index 456bde05c..3ff437659 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts @@ -5,12 +5,12 @@ import HDNode = require('hdkey'); import * as _ from 'lodash'; import { DerivedHDKey, PartialTxParams, WalletSubproviderErrors } from '../types'; -import { walletUtils } from '../walletUtils'; +import { walletUtils } from '../utils/wallet_utils'; import { BaseWalletSubprovider } from './base_wallet_subprovider'; import { PrivateKeyWalletSubprovider } from './private_key_wallet_subprovider'; -const DEFAULT_DERIVATION_PATH = `44'/60'/0'`; +const DEFAULT_DERIVATION_PATH = `44'/60'/0'/0`; const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; @@ -23,7 +23,6 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { private _addressSearchLimit: number; private _derivationPath: string; private _hdKey: HDNode; - constructor( mnemonic: string, derivationPath: string = DEFAULT_DERIVATION_PATH, diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts index 105ffa7cc..fe7ae921e 100644 --- a/packages/subproviders/src/types.ts +++ b/packages/subproviders/src/types.ts @@ -114,6 +114,7 @@ export enum NonceSubproviderErrors { export interface DerivedHDKey { address: string; derivationPath: string; + derivationIndex: number; hdKey: HDNode; } diff --git a/packages/subproviders/src/utils/wallet_utils.ts b/packages/subproviders/src/utils/wallet_utils.ts new file mode 100644 index 000000000..6c698a006 --- /dev/null +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -0,0 +1,76 @@ +import ethUtil = require('ethereumjs-util'); +import HDNode = require('hdkey'); +import * as _ from 'lodash'; + +import { DerivedHDKey, WalletSubproviderErrors } from '../types'; + +const DEFAULT_ADDRESS_SEARCH_OFFSET = 0; +const BATCH_SIZE = 10; + +// Derivation Paths +// BIP44 m / purpose' / coin_type' / account' / change / address_index +// m/44'/60'/0'/0 +// m/44'/60'/0'/0/0 +// m/44'/60'/0'/0/{account_index} - testrpc +// m/44'/60'/0' - ledger + +export const walletUtils = { + _calculateDerivedHDKeys( + initialHDKey: HDNode, + derivationPath: string, + searchLimit: number, + offset: number = DEFAULT_ADDRESS_SEARCH_OFFSET, + hardened: boolean = false, + ): DerivedHDKey[] { + const derivedKeys: DerivedHDKey[] = []; + _.times(searchLimit, i => { + const derivationIndex = offset + i; + const path = hardened ? `m/${derivationIndex}` : `m/${derivationPath}/${derivationIndex}`; + const hdKey = initialHDKey.derive(path); + const derivedPublicKey = hdKey.publicKey; + const shouldSanitizePublicKey = true; + const ethereumAddressUnprefixed = ethUtil + .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) + .toString('hex'); + const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed); + const derivedKey: DerivedHDKey = { + derivationPath, + hdKey, + address, + derivationIndex, + }; + derivedKeys.push(derivedKey); + }); + return derivedKeys; + }, + + _findDerivedKeyByAddress( + address: string, + initialHDKey: HDNode, + derivationPath: string, + searchLimit: number, + hardened: boolean = false, + ): DerivedHDKey | undefined { + let matchedKey: DerivedHDKey | undefined; + for (let index = 0; index < searchLimit; index = index + BATCH_SIZE) { + const derivedKeys = walletUtils._calculateDerivedHDKeys( + initialHDKey, + derivationPath, + BATCH_SIZE, + index, + hardened, + ); + matchedKey = _.find(derivedKeys, derivedKey => derivedKey.address === address); + if (matchedKey) { + break; + } + } + return matchedKey; + }, + + _firstDerivedKey(initialHDKey: HDNode, derivationPath: string, hardened: boolean = false): DerivedHDKey { + const derivedKeys = walletUtils._calculateDerivedHDKeys(initialHDKey, derivationPath, 1, 0, hardened); + const firstDerivedKey = derivedKeys[0]; + return firstDerivedKey; + }, +}; diff --git a/packages/subproviders/src/walletUtils.ts b/packages/subproviders/src/walletUtils.ts deleted file mode 100644 index 631636a71..000000000 --- a/packages/subproviders/src/walletUtils.ts +++ /dev/null @@ -1,58 +0,0 @@ -import ethUtil = require('ethereumjs-util'); -import HDNode = require('hdkey'); -import * as _ from 'lodash'; - -import { DerivedHDKey, WalletSubproviderErrors } from './types'; - -const DEFAULT_ADDRESS_SEARCH_OFFSET = 0; -const BATCH_SIZE = 10; -export const walletUtils = { - _calculateDerivedHDKeys( - initialHDKey: HDNode, - derivationPath: string, - searchLimit: number, - offset: number = DEFAULT_ADDRESS_SEARCH_OFFSET, - ): DerivedHDKey[] { - const derivedKeys: DerivedHDKey[] = []; - _.times(searchLimit, i => { - const path = `m/${derivationPath}/${i + offset}`; - const hdKey = initialHDKey.derive(path); - const derivedPublicKey = hdKey.publicKey; - const shouldSanitizePublicKey = true; - const ethereumAddressUnprefixed = ethUtil - .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) - .toString('hex'); - const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed); - const derivedKey: DerivedHDKey = { - derivationPath: path, - hdKey, - address, - }; - derivedKeys.push(derivedKey); - }); - return derivedKeys; - }, - - _findDerivedKeyByAddress( - address: string, - initialHDKey: HDNode, - derivationPath: string, - searchLimit: number, - ): DerivedHDKey | undefined { - let matchedKey: DerivedHDKey | undefined; - for (let index = 0; index < searchLimit; index = index + BATCH_SIZE) { - const derivedKeys = walletUtils._calculateDerivedHDKeys(initialHDKey, derivationPath, BATCH_SIZE, index); - matchedKey = _.find(derivedKeys, derivedKey => derivedKey.address === address); - if (matchedKey) { - break; - } - } - return matchedKey; - }, - - _firstDerivedKey(initialHDKey: HDNode, derivationPath: string): DerivedHDKey { - const derivedKeys = walletUtils._calculateDerivedHDKeys(initialHDKey, derivationPath, 1); - const firstDerivedKey = derivedKeys[0]; - return firstDerivedKey; - }, -}; diff --git a/packages/subproviders/test/integration/ledger_subprovider_test.ts b/packages/subproviders/test/integration/ledger_subprovider_test.ts index 503618089..88a3c6db1 100644 --- a/packages/subproviders/test/integration/ledger_subprovider_test.ts +++ b/packages/subproviders/test/integration/ledger_subprovider_test.ts @@ -42,9 +42,10 @@ describe('LedgerSubprovider', () => { expect(accounts[0]).to.not.be.an('undefined'); expect(accounts.length).to.be.equal(10); }); - it('returns the expected first account from a ledger set up with the test mnemonic', async () => { + it('returns the expected accounts from a ledger set up with the test mnemonic', async () => { const accounts = await ledgerSubprovider.getAccountsAsync(); expect(accounts[0]).to.be.equal(fixtureData.TEST_RPC_ACCOUNT_0); + expect(accounts[1]).to.be.equal(fixtureData.TEST_RPC_ACCOUNT_1); }); it('returns requested number of accounts', async () => { const numberOfAccounts = 20; diff --git a/packages/subproviders/test/unit/ledger_subprovider_test.ts b/packages/subproviders/test/unit/ledger_subprovider_test.ts index c18506681..d3a3fbe26 100644 --- a/packages/subproviders/test/unit/ledger_subprovider_test.ts +++ b/packages/subproviders/test/unit/ledger_subprovider_test.ts @@ -132,7 +132,7 @@ describe('LedgerSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'eth_sign', - params: ['0x0000000000000000000000000000000000000000', messageHex], + params: [FAKE_ADDRESS, messageHex], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { @@ -149,7 +149,7 @@ describe('LedgerSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'personal_sign', - params: [messageHex, '0x0000000000000000000000000000000000000000'], + params: [messageHex, FAKE_ADDRESS], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { @@ -190,7 +190,7 @@ describe('LedgerSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'eth_sign', - params: ['0x0000000000000000000000000000000000000000', nonHexMessage], + params: [FAKE_ADDRESS, nonHexMessage], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { @@ -205,7 +205,7 @@ describe('LedgerSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'personal_sign', - params: [nonHexMessage, '0x0000000000000000000000000000000000000000'], + params: [nonHexMessage, FAKE_ADDRESS], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { diff --git a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts index 7aaef4944..16981cf86 100644 --- a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts @@ -28,9 +28,12 @@ describe('MnemonicWalletSubprovider', () => { }); describe('direct method calls', () => { describe('success cases', () => { - it('returns the account', async () => { + it('returns the accounts', async () => { const accounts = await subprovider.getAccountsAsync(); + // tslint:disable-next-line:no-console + console.log(accounts); expect(accounts[0]).to.be.equal(fixtureData.TEST_RPC_ACCOUNT_0); + expect(accounts[1]).to.be.equal(fixtureData.TEST_RPC_ACCOUNT_1); expect(accounts.length).to.be.equal(10); }); it('signs a personal message', async () => { diff --git a/packages/subproviders/test/utils/fixture_data.ts b/packages/subproviders/test/utils/fixture_data.ts index 5ce3ff08f..62b76e132 100644 --- a/packages/subproviders/test/utils/fixture_data.ts +++ b/packages/subproviders/test/utils/fixture_data.ts @@ -1,8 +1,10 @@ const TEST_RPC_ACCOUNT_0 = '0x5409ed021d9299bf6814279a6a1411a7e866a631'; +const TEST_RPC_ACCOUNT_1 = '0x6ecbe1db9ef729cbe972c83fb886247691fb6beb'; const networkId = 42; export const fixtureData = { TEST_RPC_ACCOUNT_0, TEST_RPC_ACCOUNT_0_ACCOUNT_PRIVATE_KEY: 'F2F48EE19680706196E2E339E5DA3491186E0C4C5030670656B0E0164837257D', + TEST_RPC_ACCOUNT_1, TEST_RPC_MNEMONIC: 'concert load couple harbor equip island argue ramp clarify fence smart topic', TEST_RPC_MNEMONIC_DERIVATION_PATH: `44'/60'/0'/0`, PERSONAL_MESSAGE_STRING: 'hello world', -- cgit From a34c9095c3a5d217d50318471660a5508922bcc2 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Tue, 10 Apr 2018 15:50:08 +1000 Subject: Rename to IS_CHILD_KEY --- packages/subproviders/src/subproviders/ledger.ts | 8 ++++---- packages/subproviders/src/utils/wallet_utils.ts | 22 +++++++++------------- .../test/unit/mnemonic_wallet_subprovider_test.ts | 2 -- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index a7b79c128..fabff88cd 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -25,6 +25,7 @@ const DEFAULT_DERIVATION_PATH = `44'/60'/0'`; const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; const ASK_FOR_ON_DEVICE_CONFIRMATION = false; const SHOULD_GET_CHAIN_CODE = true; +const IS_CHILD_KEY = true; /** * Subprovider for interfacing with a user's [Ledger Nano S](https://www.ledgerwallet.com/products/ledger-nano-s). @@ -40,7 +41,6 @@ export class LedgerSubprovider extends BaseWalletSubprovider { private _ledgerClientIfExists?: LedgerEthereumClient; private _shouldAlwaysAskForConfirmation: boolean; private _addressSearchLimit: number; - private _hardenedKey: boolean = true; /** * Instantiates a LedgerSubprovider. Defaults to derivationPath set to `44'/60'/0'`. * TestRPC/Ganache defaults to `m/44'/60'/0'/0`, so set this in the configs if desired. @@ -110,7 +110,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { LedgerSubprovider._validateTxParams(txParams); const initialHDKey = await this._initialHDKeyAsync(); const derivedKey = _.isUndefined(txParams.from) - ? walletUtils._firstDerivedKey(initialHDKey, this._derivationPath, this._hardenedKey) + ? walletUtils._firstDerivedKey(initialHDKey, this._derivationPath, IS_CHILD_KEY) : this._findDerivedKeyByPublicAddress(initialHDKey, txParams.from); this._ledgerClientIfExists = await this._createLedgerClientAsync(); @@ -165,7 +165,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { assert.isHexString('data', data); const initialHDKey = await this._initialHDKeyAsync(); const derivedKey = _.isUndefined(address) - ? walletUtils._firstDerivedKey(initialHDKey, this._derivationPath, this._hardenedKey) + ? walletUtils._firstDerivedKey(initialHDKey, this._derivationPath, IS_CHILD_KEY) : this._findDerivedKeyByPublicAddress(initialHDKey, address); this._ledgerClientIfExists = await this._createLedgerClientAsync(); @@ -232,7 +232,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { initalHDKey, this._derivationPath, this._addressSearchLimit, - this._hardenedKey, + IS_CHILD_KEY, ); if (_.isUndefined(matchedDerivedKey)) { throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`); diff --git a/packages/subproviders/src/utils/wallet_utils.ts b/packages/subproviders/src/utils/wallet_utils.ts index 6c698a006..48d475559 100644 --- a/packages/subproviders/src/utils/wallet_utils.ts +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -7,25 +7,21 @@ import { DerivedHDKey, WalletSubproviderErrors } from '../types'; const DEFAULT_ADDRESS_SEARCH_OFFSET = 0; const BATCH_SIZE = 10; -// Derivation Paths -// BIP44 m / purpose' / coin_type' / account' / change / address_index -// m/44'/60'/0'/0 -// m/44'/60'/0'/0/0 -// m/44'/60'/0'/0/{account_index} - testrpc -// m/44'/60'/0' - ledger - export const walletUtils = { _calculateDerivedHDKeys( initialHDKey: HDNode, derivationPath: string, searchLimit: number, offset: number = DEFAULT_ADDRESS_SEARCH_OFFSET, - hardened: boolean = false, + isChildKey: boolean = false, ): DerivedHDKey[] { const derivedKeys: DerivedHDKey[] = []; _.times(searchLimit, i => { const derivationIndex = offset + i; - const path = hardened ? `m/${derivationIndex}` : `m/${derivationPath}/${derivationIndex}`; + // Normally we need to set the full derivation path to walk the tree from the root + // as the initial key is at the root. + // But with ledger the initial key is a child so we walk the tree relative to that child + const path = isChildKey ? `m/${derivationIndex}` : `m/${derivationPath}/${derivationIndex}`; const hdKey = initialHDKey.derive(path); const derivedPublicKey = hdKey.publicKey; const shouldSanitizePublicKey = true; @@ -49,7 +45,7 @@ export const walletUtils = { initialHDKey: HDNode, derivationPath: string, searchLimit: number, - hardened: boolean = false, + isChild: boolean = false, ): DerivedHDKey | undefined { let matchedKey: DerivedHDKey | undefined; for (let index = 0; index < searchLimit; index = index + BATCH_SIZE) { @@ -58,7 +54,7 @@ export const walletUtils = { derivationPath, BATCH_SIZE, index, - hardened, + isChild, ); matchedKey = _.find(derivedKeys, derivedKey => derivedKey.address === address); if (matchedKey) { @@ -68,8 +64,8 @@ export const walletUtils = { return matchedKey; }, - _firstDerivedKey(initialHDKey: HDNode, derivationPath: string, hardened: boolean = false): DerivedHDKey { - const derivedKeys = walletUtils._calculateDerivedHDKeys(initialHDKey, derivationPath, 1, 0, hardened); + _firstDerivedKey(initialHDKey: HDNode, derivationPath: string, isChild: boolean = false): DerivedHDKey { + const derivedKeys = walletUtils._calculateDerivedHDKeys(initialHDKey, derivationPath, 1, 0, isChild); const firstDerivedKey = derivedKeys[0]; return firstDerivedKey; }, diff --git a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts index 16981cf86..77e5d35ae 100644 --- a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts @@ -30,8 +30,6 @@ describe('MnemonicWalletSubprovider', () => { describe('success cases', () => { it('returns the accounts', async () => { const accounts = await subprovider.getAccountsAsync(); - // tslint:disable-next-line:no-console - console.log(accounts); expect(accounts[0]).to.be.equal(fixtureData.TEST_RPC_ACCOUNT_0); expect(accounts[1]).to.be.equal(fixtureData.TEST_RPC_ACCOUNT_1); expect(accounts.length).to.be.equal(10); -- cgit From 4e4842a62f6b2d728e2d9986acf5116032f763f2 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Tue, 10 Apr 2018 16:01:34 +1000 Subject: Improve Documentation for functions and constructors --- .../src/subproviders/mnemonic_wallet_subprovider.ts | 18 ++++++++++++++---- .../src/subproviders/private_key_wallet_subprovider.ts | 12 ++++++++++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts index 3ff437659..02d06e2cd 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts @@ -23,6 +23,14 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { private _addressSearchLimit: number; private _derivationPath: string; private _hdKey: HDNode; + /** + * Instantiates a MnemonicWalletSubprovider. Defaults to derivationPath set to `44'/60'/0'/0`. + * This is the default in TestRPC/Ganache, this can be overridden if desired. + * @param mnemonic The mnemonic seed + * @param derivationPath The derivation path, defaults to `44'/60'/0'/0` + * @param addressSearchLimit The limit on address search attempts before raising `WalletSubproviderErrors.AddressNotFound` + * @return MnemonicWalletSubprovider instance + */ constructor( mnemonic: string, derivationPath: string = DEFAULT_DERIVATION_PATH, @@ -32,7 +40,8 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { assert.isString('derivationPath', derivationPath); assert.isNumber('addressSearchLimit', addressSearchLimit); super(); - this._hdKey = HDNode.fromMasterSeed(bip39.mnemonicToSeed(mnemonic)); + const seed = bip39.mnemonicToSeed(mnemonic); + this._hdKey = HDNode.fromMasterSeed(seed); this._derivationPath = derivationPath; this._addressSearchLimit = addressSearchLimit; } @@ -54,6 +63,7 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * Retrieve the accounts associated with the mnemonic. * This method is implicitly called when issuing a `eth_accounts` JSON RPC request * via your providerEngine instance. + * @param numberOfAccounts Number of accounts to retrieve (default: 10) * @return An array of accounts */ public async getAccountsAsync(numberOfAccounts: number = DEFAULT_NUM_ADDRESSES_TO_FETCH): Promise { @@ -79,9 +89,9 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { return signedTx; } /** - * Sign a personal Ethereum signed message. The signing address will be - * derived from the set path. - * If you've added the PKWalletSubprovider to your app's provider, you can simply send an `eth_sign` + * Sign a personal Ethereum signed message. The signing address used will be + * address provided or the first address derived from the set path. + * If you've added the MnemonicWalletSubprovider to your app's provider, you can simply send an `eth_sign` * or `personal_sign` JSON RPC request, and this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. * @param data Message to sign diff --git a/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts b/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts index f6906bab6..005d36f93 100644 --- a/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts @@ -15,6 +15,11 @@ import { BaseWalletSubprovider } from './base_wallet_subprovider'; export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { private _address: string; private _privateKeyBuffer: Buffer; + /** + * Instantiates a PrivateKeyWalletSubprovider. + * @param privateKey The private key of the ethereum address + * @return PrivateKeyWalletSubprovider instance + */ constructor(privateKey: string) { assert.isString('privateKey', privateKey); super(); @@ -40,6 +45,9 @@ export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { */ public async signTransactionAsync(txParams: PartialTxParams): Promise { PrivateKeyWalletSubprovider._validateTxParams(txParams); + if (!_.isUndefined(txParams.from) && txParams.from !== this._address) { + throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${txParams.from}`); + } const tx = new EthereumTx(txParams); tx.sign(this._privateKeyBuffer); const rawTx = `0x${tx.serialize().toString('hex')}`; @@ -47,8 +55,8 @@ export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { } /** * Sign a personal Ethereum signed message. The signing address will be - * calculated from the private key. - * If you've added the PKWalletSubprovider to your app's provider, you can simply send an `eth_sign` + * calculated from the private key, if an address is provided it must match the address calculated from the private key. + * If you've added this Subprovider to your app's provider, you can simply send an `eth_sign` * or `personal_sign` JSON RPC request, and this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. * @param data Message to sign -- cgit From 9169913a2cbe9d421629db2e8169c0806044e3fc Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 12:22:41 +1000 Subject: Add isChildKey to derived key --- packages/subproviders/src/subproviders/ledger.ts | 54 +++++++++----------- .../subproviders/mnemonic_wallet_subprovider.ts | 30 ++++++----- packages/subproviders/src/types.ts | 2 + packages/subproviders/src/utils/wallet_utils.ts | 59 ++++++++++------------ 4 files changed, 73 insertions(+), 72 deletions(-) diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index fabff88cd..6f66e3018 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -22,7 +22,6 @@ import { walletUtils } from '../utils/wallet_utils'; import { BaseWalletSubprovider } from './base_wallet_subprovider'; const DEFAULT_DERIVATION_PATH = `44'/60'/0'`; -const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; const ASK_FOR_ON_DEVICE_CONFIRMATION = false; const SHOULD_GET_CHAIN_CODE = true; const IS_CHILD_KEY = true; @@ -59,9 +58,9 @@ export class LedgerSubprovider extends BaseWalletSubprovider { : ASK_FOR_ON_DEVICE_CONFIRMATION; this._addressSearchLimit = !_.isUndefined(config.accountFetchingConfigs) && - !_.isUndefined(config.accountFetchingConfigs.numAddressesToReturn) - ? config.accountFetchingConfigs.numAddressesToReturn - : DEFAULT_NUM_ADDRESSES_TO_FETCH; + !_.isUndefined(config.accountFetchingConfigs.addressSearchLimit) + ? config.accountFetchingConfigs.addressSearchLimit + : walletUtils.DEFAULT_ADDRESS_SEARCH_LIMIT; } /** * Retrieve the set derivation path @@ -86,15 +85,12 @@ export class LedgerSubprovider extends BaseWalletSubprovider { * @param numberOfAccounts Number of accounts to retrieve (default: 10) * @return An array of accounts */ - public async getAccountsAsync(numberOfAccounts: number = DEFAULT_NUM_ADDRESSES_TO_FETCH): Promise { - const initialHDKey = await this._initialHDKeyAsync(); - const derivedKeys = walletUtils._calculateDerivedHDKeys( - initialHDKey, - this._derivationPath, - numberOfAccounts, - 0, - true, - ); + public async getAccountsAsync( + numberOfAccounts: number = walletUtils.DEFAULT_NUM_ADDRESSES_TO_FETCH, + ): Promise { + const offset = 0; + const initialHDerivedKey = await this._initialDerivedKeyAsync(); + const derivedKeys = walletUtils.calculateDerivedHDKeys(initialHDerivedKey, numberOfAccounts, offset); const accounts = _.map(derivedKeys, 'address'); return accounts; } @@ -108,10 +104,10 @@ export class LedgerSubprovider extends BaseWalletSubprovider { */ public async signTransactionAsync(txParams: PartialTxParams): Promise { LedgerSubprovider._validateTxParams(txParams); - const initialHDKey = await this._initialHDKeyAsync(); + const initialDerivedKey = await this._initialDerivedKeyAsync(); const derivedKey = _.isUndefined(txParams.from) - ? walletUtils._firstDerivedKey(initialHDKey, this._derivationPath, IS_CHILD_KEY) - : this._findDerivedKeyByPublicAddress(initialHDKey, txParams.from); + ? walletUtils._firstDerivedKey(initialDerivedKey) + : this._findDerivedKeyByPublicAddress(initialDerivedKey, txParams.from); this._ledgerClientIfExists = await this._createLedgerClientAsync(); @@ -163,10 +159,10 @@ export class LedgerSubprovider extends BaseWalletSubprovider { throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage); } assert.isHexString('data', data); - const initialHDKey = await this._initialHDKeyAsync(); + const initialDerivedKey = await this._initialDerivedKeyAsync(); const derivedKey = _.isUndefined(address) - ? walletUtils._firstDerivedKey(initialHDKey, this._derivationPath, IS_CHILD_KEY) - : this._findDerivedKeyByPublicAddress(initialHDKey, address); + ? walletUtils._firstDerivedKey(initialDerivedKey) + : this._findDerivedKeyByPublicAddress(initialDerivedKey, address); this._ledgerClientIfExists = await this._createLedgerClientAsync(); try { @@ -208,7 +204,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { this._ledgerClientIfExists = undefined; this._connectionLock.release(); } - private async _initialHDKeyAsync(): Promise { + private async _initialDerivedKeyAsync(): Promise { this._ledgerClientIfExists = await this._createLedgerClientAsync(); let ledgerResponse; @@ -224,16 +220,16 @@ export class LedgerSubprovider extends BaseWalletSubprovider { const hdKey = new HDNode(); hdKey.publicKey = new Buffer(ledgerResponse.publicKey, 'hex'); hdKey.chainCode = new Buffer(ledgerResponse.chainCode, 'hex'); - return hdKey; + return { + hdKey, + address: ledgerResponse.address, + isChildKey: true, + derivationPath: this._derivationPath, + derivationIndex: 0, + }; } - private _findDerivedKeyByPublicAddress(initalHDKey: HDNode, address: string): DerivedHDKey { - const matchedDerivedKey = walletUtils._findDerivedKeyByAddress( - address, - initalHDKey, - this._derivationPath, - this._addressSearchLimit, - IS_CHILD_KEY, - ); + private _findDerivedKeyByPublicAddress(initalHDKey: DerivedHDKey, address: string): DerivedHDKey { + const matchedDerivedKey = walletUtils.findDerivedKeyByAddress(address, initalHDKey, this._addressSearchLimit); if (_.isUndefined(matchedDerivedKey)) { throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`); } diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts index 02d06e2cd..53013c44c 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts @@ -11,8 +11,6 @@ import { BaseWalletSubprovider } from './base_wallet_subprovider'; import { PrivateKeyWalletSubprovider } from './private_key_wallet_subprovider'; const DEFAULT_DERIVATION_PATH = `44'/60'/0'/0`; -const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; -const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; /** * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. @@ -22,7 +20,7 @@ const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; export class MnemonicWalletSubprovider extends BaseWalletSubprovider { private _addressSearchLimit: number; private _derivationPath: string; - private _hdKey: HDNode; + private _derivedKey: DerivedHDKey; /** * Instantiates a MnemonicWalletSubprovider. Defaults to derivationPath set to `44'/60'/0'/0`. * This is the default in TestRPC/Ganache, this can be overridden if desired. @@ -34,15 +32,22 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { constructor( mnemonic: string, derivationPath: string = DEFAULT_DERIVATION_PATH, - addressSearchLimit: number = DEFAULT_ADDRESS_SEARCH_LIMIT, + addressSearchLimit: number = walletUtils.DEFAULT_ADDRESS_SEARCH_LIMIT, ) { assert.isString('mnemonic', mnemonic); assert.isString('derivationPath', derivationPath); assert.isNumber('addressSearchLimit', addressSearchLimit); super(); const seed = bip39.mnemonicToSeed(mnemonic); - this._hdKey = HDNode.fromMasterSeed(seed); + const hdKey = HDNode.fromMasterSeed(seed); this._derivationPath = derivationPath; + this._derivedKey = { + address: walletUtils.addressOfHDKey(hdKey), + derivationPath: this._derivationPath, + derivationIndex: 0, + hdKey, + isChildKey: false, + }; this._addressSearchLimit = addressSearchLimit; } /** @@ -66,8 +71,10 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @param numberOfAccounts Number of accounts to retrieve (default: 10) * @return An array of accounts */ - public async getAccountsAsync(numberOfAccounts: number = DEFAULT_NUM_ADDRESSES_TO_FETCH): Promise { - const derivedKeys = walletUtils._calculateDerivedHDKeys(this._hdKey, this._derivationPath, numberOfAccounts); + public async getAccountsAsync( + numberOfAccounts: number = walletUtils.DEFAULT_NUM_ADDRESSES_TO_FETCH, + ): Promise { + const derivedKeys = walletUtils.calculateDerivedHDKeys(this._derivedKey, numberOfAccounts); const accounts = _.map(derivedKeys, 'address'); return accounts; } @@ -82,7 +89,7 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { */ public async signTransactionAsync(txParams: PartialTxParams): Promise { const derivedKey = _.isUndefined(txParams.from) - ? walletUtils._firstDerivedKey(this._hdKey, this._derivationPath) + ? walletUtils._firstDerivedKey(this._derivedKey) : this._findDerivedKeyByPublicAddress(txParams.from); const privateKeyWallet = new PrivateKeyWalletSubprovider(derivedKey.hdKey.privateKey.toString('hex')); const signedTx = privateKeyWallet.signTransactionAsync(txParams); @@ -100,17 +107,16 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { */ public async signPersonalMessageAsync(data: string, address?: string): Promise { const derivedKey = _.isUndefined(address) - ? walletUtils._firstDerivedKey(this._hdKey, this._derivationPath) + ? walletUtils._firstDerivedKey(this._derivedKey) : this._findDerivedKeyByPublicAddress(address); const privateKeyWallet = new PrivateKeyWalletSubprovider(derivedKey.hdKey.privateKey.toString('hex')); const sig = await privateKeyWallet.signPersonalMessageAsync(data, derivedKey.address); return sig; } private _findDerivedKeyByPublicAddress(address: string): DerivedHDKey { - const matchedDerivedKey = walletUtils._findDerivedKeyByAddress( + const matchedDerivedKey = walletUtils.findDerivedKeyByAddress( address, - this._hdKey, - this._derivationPath, + this._derivedKey, this._addressSearchLimit, ); if (_.isUndefined(matchedDerivedKey)) { diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts index fe7ae921e..9f557731f 100644 --- a/packages/subproviders/src/types.ts +++ b/packages/subproviders/src/types.ts @@ -51,6 +51,7 @@ export interface LedgerSubproviderConfigs { * before fetching their addresses */ export interface AccountFetchingConfigs { + addressSearchLimit?: number; numAddressesToReturn?: number; shouldAskForOnDeviceConfirmation?: boolean; } @@ -116,6 +117,7 @@ export interface DerivedHDKey { derivationPath: string; derivationIndex: number; hdKey: HDNode; + isChildKey: boolean; } export type ErrorCallback = (err: Error | null, data?: any) => void; diff --git a/packages/subproviders/src/utils/wallet_utils.ts b/packages/subproviders/src/utils/wallet_utils.ts index 48d475559..0c3e895b6 100644 --- a/packages/subproviders/src/utils/wallet_utils.ts +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -8,54 +8,51 @@ const DEFAULT_ADDRESS_SEARCH_OFFSET = 0; const BATCH_SIZE = 10; export const walletUtils = { - _calculateDerivedHDKeys( - initialHDKey: HDNode, - derivationPath: string, + DEFAULT_NUM_ADDRESSES_TO_FETCH: 10, + DEFAULT_ADDRESS_SEARCH_LIMIT: 1000, + calculateDerivedHDKeys( + initialDerivedKey: DerivedHDKey, searchLimit: number, offset: number = DEFAULT_ADDRESS_SEARCH_OFFSET, - isChildKey: boolean = false, ): DerivedHDKey[] { const derivedKeys: DerivedHDKey[] = []; _.times(searchLimit, i => { + const derivationPath = initialDerivedKey.derivationPath; const derivationIndex = offset + i; - // Normally we need to set the full derivation path to walk the tree from the root - // as the initial key is at the root. - // But with ledger the initial key is a child so we walk the tree relative to that child - const path = isChildKey ? `m/${derivationIndex}` : `m/${derivationPath}/${derivationIndex}`; - const hdKey = initialHDKey.derive(path); - const derivedPublicKey = hdKey.publicKey; - const shouldSanitizePublicKey = true; - const ethereumAddressUnprefixed = ethUtil - .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) - .toString('hex'); - const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed); + // If the DerivedHDKey is a child then we walk relative, if not we walk the full derivation path + const path = initialDerivedKey.isChildKey + ? `m/${derivationIndex}` + : `m/${derivationPath}/${derivationIndex}`; + const hdKey = initialDerivedKey.hdKey.derive(path); + const address = walletUtils.addressOfHDKey(hdKey); const derivedKey: DerivedHDKey = { - derivationPath, - hdKey, address, + hdKey, + derivationPath, derivationIndex, + isChildKey: initialDerivedKey.isChildKey, }; derivedKeys.push(derivedKey); }); return derivedKeys; }, - - _findDerivedKeyByAddress( + addressOfHDKey(hdKey: HDNode): string { + const shouldSanitizePublicKey = true; + const derivedPublicKey = hdKey.publicKey; + const ethereumAddressUnprefixed = ethUtil + .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) + .toString('hex'); + const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed); + return address; + }, + findDerivedKeyByAddress( address: string, - initialHDKey: HDNode, - derivationPath: string, + initialDerivedKey: DerivedHDKey, searchLimit: number, - isChild: boolean = false, ): DerivedHDKey | undefined { let matchedKey: DerivedHDKey | undefined; for (let index = 0; index < searchLimit; index = index + BATCH_SIZE) { - const derivedKeys = walletUtils._calculateDerivedHDKeys( - initialHDKey, - derivationPath, - BATCH_SIZE, - index, - isChild, - ); + const derivedKeys = walletUtils.calculateDerivedHDKeys(initialDerivedKey, BATCH_SIZE, index); matchedKey = _.find(derivedKeys, derivedKey => derivedKey.address === address); if (matchedKey) { break; @@ -64,8 +61,8 @@ export const walletUtils = { return matchedKey; }, - _firstDerivedKey(initialHDKey: HDNode, derivationPath: string, isChild: boolean = false): DerivedHDKey { - const derivedKeys = walletUtils._calculateDerivedHDKeys(initialHDKey, derivationPath, 1, 0, isChild); + _firstDerivedKey(initialDerivedKey: DerivedHDKey): DerivedHDKey { + const derivedKeys = walletUtils.calculateDerivedHDKeys(initialDerivedKey, 1, 0); const firstDerivedKey = derivedKeys[0]; return firstDerivedKey; }, -- cgit From 20a1deb187d8fb38a42ae87fc8f046dff4b4ba61 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 12:47:17 +1000 Subject: Throw errors when the address is not specified or invalid --- .../src/subproviders/base_wallet_subprovider.ts | 2 +- packages/subproviders/src/subproviders/ledger.ts | 31 ++++++++++------------ .../subproviders/mnemonic_wallet_subprovider.ts | 27 ++++++++++++------- .../subproviders/private_key_wallet_subprovider.ts | 12 ++++----- packages/subproviders/src/types.ts | 4 +-- .../test/integration/ledger_subprovider_test.ts | 5 +++- .../test/unit/ledger_subprovider_test.ts | 5 ++-- .../test/unit/mnemonic_wallet_subprovider_test.ts | 2 +- .../unit/private_key_wallet_subprovider_test.ts | 21 +++++++++++++-- 9 files changed, 67 insertions(+), 42 deletions(-) diff --git a/packages/subproviders/src/subproviders/base_wallet_subprovider.ts b/packages/subproviders/src/subproviders/base_wallet_subprovider.ts index 47b45a126..0a9b99ae4 100644 --- a/packages/subproviders/src/subproviders/base_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/base_wallet_subprovider.ts @@ -21,7 +21,7 @@ export abstract class BaseWalletSubprovider extends Subprovider { public abstract async getAccountsAsync(): Promise; public abstract async signTransactionAsync(txParams: PartialTxParams): Promise; - public abstract async signPersonalMessageAsync(data: string, address?: string): Promise; + public abstract async signPersonalMessageAsync(data: string, address: string): Promise; /** * This method conforms to the web3-provider-engine interface. diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 6f66e3018..3c0442e4c 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -105,11 +105,9 @@ export class LedgerSubprovider extends BaseWalletSubprovider { public async signTransactionAsync(txParams: PartialTxParams): Promise { LedgerSubprovider._validateTxParams(txParams); const initialDerivedKey = await this._initialDerivedKeyAsync(); - const derivedKey = _.isUndefined(txParams.from) - ? walletUtils._firstDerivedKey(initialDerivedKey) - : this._findDerivedKeyByPublicAddress(initialDerivedKey, txParams.from); + const derivedKey = this._findDerivedKeyByPublicAddress(initialDerivedKey, txParams.from); - this._ledgerClientIfExists = await this._createLedgerClientAsync(); + const ledgerClient = await this._createLedgerClientAsync(); const tx = new EthereumTx(txParams); @@ -121,7 +119,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { const txHex = tx.serialize().toString('hex'); try { const derivationPath = `${derivedKey.derivationPath}/${derivedKey.derivationIndex}`; - const result = await this._ledgerClientIfExists.signTransaction(derivationPath, txHex); + const result = await ledgerClient.signTransaction(derivationPath, txHex); // Store signature in transaction tx.r = Buffer.from(result.r, 'hex'); tx.s = Buffer.from(result.s, 'hex'); @@ -145,7 +143,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { } /** * Sign a personal Ethereum signed message. The signing address will be the one - * either the provided address or the first address on the ledger device. + * the provided address. * The Ledger adds the Ethereum signed message prefix on-device. If you've added * the LedgerSubprovider to your app's provider, you can simply send an `eth_sign` * or `personal_sign` JSON RPC request, and this method will be called auto-magically. @@ -154,23 +152,18 @@ export class LedgerSubprovider extends BaseWalletSubprovider { * @param address Address to sign with * @return Signature hex string (order: rsv) */ - public async signPersonalMessageAsync(data: string, address?: string): Promise { + public async signPersonalMessageAsync(data: string, address: string): Promise { if (_.isUndefined(data)) { throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage); } assert.isHexString('data', data); const initialDerivedKey = await this._initialDerivedKeyAsync(); - const derivedKey = _.isUndefined(address) - ? walletUtils._firstDerivedKey(initialDerivedKey) - : this._findDerivedKeyByPublicAddress(initialDerivedKey, address); + const derivedKey = this._findDerivedKeyByPublicAddress(initialDerivedKey, address); - this._ledgerClientIfExists = await this._createLedgerClientAsync(); + const ledgerClient = await this._createLedgerClientAsync(); try { const derivationPath = `${derivedKey.derivationPath}/${derivedKey.derivationIndex}`; - const result = await this._ledgerClientIfExists.signPersonalMessage( - derivationPath, - ethUtil.stripHexPrefix(data), - ); + const result = await ledgerClient.signPersonalMessage(derivationPath, ethUtil.stripHexPrefix(data)); const v = result.v - 27; let vHex = v.toString(16); if (vHex.length < 2) { @@ -192,6 +185,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { } const ledgerEthereumClient = await this._ledgerEthereumClientFactoryAsync(); this._connectionLock.release(); + this._ledgerClientIfExists = ledgerEthereumClient; return ledgerEthereumClient; } private async _destroyLedgerClientAsync() { @@ -205,11 +199,11 @@ export class LedgerSubprovider extends BaseWalletSubprovider { this._connectionLock.release(); } private async _initialDerivedKeyAsync(): Promise { - this._ledgerClientIfExists = await this._createLedgerClientAsync(); + const ledgerClient = await this._createLedgerClientAsync(); let ledgerResponse; try { - ledgerResponse = await this._ledgerClientIfExists.getAddress( + ledgerResponse = await ledgerClient.getAddress( this._derivationPath, this._shouldAlwaysAskForConfirmation, SHOULD_GET_CHAIN_CODE, @@ -229,6 +223,9 @@ export class LedgerSubprovider extends BaseWalletSubprovider { }; } private _findDerivedKeyByPublicAddress(initalHDKey: DerivedHDKey, address: string): DerivedHDKey { + if (_.isUndefined(address)) { + throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); + } const matchedDerivedKey = walletUtils.findDerivedKeyByAddress(address, initalHDKey, this._addressSearchLimit); if (_.isUndefined(matchedDerivedKey)) { throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`); diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts index 53013c44c..730453bc6 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts @@ -63,6 +63,10 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { */ public setPath(derivationPath: string) { this._derivationPath = derivationPath; + this._derivedKey = { + ...this._derivedKey, + derivationPath: this._derivationPath, + }; } /** * Retrieve the accounts associated with the mnemonic. @@ -88,10 +92,7 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @return Signed transaction hex string */ public async signTransactionAsync(txParams: PartialTxParams): Promise { - const derivedKey = _.isUndefined(txParams.from) - ? walletUtils._firstDerivedKey(this._derivedKey) - : this._findDerivedKeyByPublicAddress(txParams.from); - const privateKeyWallet = new PrivateKeyWalletSubprovider(derivedKey.hdKey.privateKey.toString('hex')); + const privateKeyWallet = this._privateKeyWalletFromAddress(txParams.from); const signedTx = privateKeyWallet.signTransactionAsync(txParams); return signedTx; } @@ -105,15 +106,21 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @param address Address to sign with * @return Signature hex string (order: rsv) */ - public async signPersonalMessageAsync(data: string, address?: string): Promise { - const derivedKey = _.isUndefined(address) - ? walletUtils._firstDerivedKey(this._derivedKey) - : this._findDerivedKeyByPublicAddress(address); - const privateKeyWallet = new PrivateKeyWalletSubprovider(derivedKey.hdKey.privateKey.toString('hex')); - const sig = await privateKeyWallet.signPersonalMessageAsync(data, derivedKey.address); + public async signPersonalMessageAsync(data: string, address: string): Promise { + const privateKeyWallet = this._privateKeyWalletFromAddress(address); + const sig = await privateKeyWallet.signPersonalMessageAsync(data, address); return sig; } + private _privateKeyWalletFromAddress(address: string): PrivateKeyWalletSubprovider { + const derivedKey = this._findDerivedKeyByPublicAddress(address); + const privateKeyHex = derivedKey.hdKey.privateKey.toString('hex'); + const privateKeyWallet = new PrivateKeyWalletSubprovider(privateKeyHex); + return privateKeyWallet; + } private _findDerivedKeyByPublicAddress(address: string): DerivedHDKey { + if (_.isUndefined(address)) { + throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); + } const matchedDerivedKey = walletUtils.findDerivedKeyByAddress( address, this._derivedKey, diff --git a/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts b/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts index 005d36f93..f8afab722 100644 --- a/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts @@ -63,15 +63,15 @@ export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { * @param address Address to sign with * @return Signature hex string (order: rsv) */ - public async signPersonalMessageAsync(dataIfExists: string, address?: string): Promise { - if (_.isUndefined(dataIfExists)) { + public async signPersonalMessageAsync(data: string, address: string): Promise { + if (_.isUndefined(data)) { throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage); } - if (!_.isUndefined(address) && address !== this._address) { - throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`); + if (_.isUndefined(address) || address !== this._address) { + throw new Error(`${WalletSubproviderErrors.FromAddressMissingOrInvalid}: ${address}`); } - assert.isHexString('data', dataIfExists); - const dataBuff = ethUtil.toBuffer(dataIfExists); + assert.isHexString('data', data); + const dataBuff = ethUtil.toBuffer(data); const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff); const sig = ethUtil.ecsign(msgHashBuff, this._privateKeyBuffer); const rpcSig = ethUtil.toRpcSig(sig.v, sig.r, sig.s); diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts index 9f557731f..140c7c2df 100644 --- a/packages/subproviders/src/types.ts +++ b/packages/subproviders/src/types.ts @@ -80,7 +80,7 @@ export interface PartialTxParams { gasPrice?: string; gas: string; to: string; - from?: string; + from: string; value?: string; data?: string; chainId: number; // EIP 155 chainId - mainnet: 1, ropsten: 3 @@ -101,10 +101,10 @@ export enum WalletSubproviderErrors { AddressNotFound = 'ADDRESS_NOT_FOUND', DataMissingForSignPersonalMessage = 'DATA_MISSING_FOR_SIGN_PERSONAL_MESSAGE', SenderInvalidOrNotSupplied = 'SENDER_INVALID_OR_NOT_SUPPLIED', + FromAddressMissingOrInvalid = 'FROM_ADDRESS_MISSING_OR_INVALID', } export enum LedgerSubproviderErrors { TooOldLedgerFirmware = 'TOO_OLD_LEDGER_FIRMWARE', - FromAddressMissingOrInvalid = 'FROM_ADDRESS_MISSING_OR_INVALID', MultipleOpenConnectionsDisallowed = 'MULTIPLE_OPEN_CONNECTIONS_DISALLOWED', } diff --git a/packages/subproviders/test/integration/ledger_subprovider_test.ts b/packages/subproviders/test/integration/ledger_subprovider_test.ts index 88a3c6db1..4a831af67 100644 --- a/packages/subproviders/test/integration/ledger_subprovider_test.ts +++ b/packages/subproviders/test/integration/ledger_subprovider_test.ts @@ -55,7 +55,10 @@ describe('LedgerSubprovider', () => { }); it('signs a personal message', async () => { const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); - const ecSignatureHex = await ledgerSubprovider.signPersonalMessageAsync(data); + const ecSignatureHex = await ledgerSubprovider.signPersonalMessageAsync( + data, + fixtureData.TEST_RPC_ACCOUNT_0, + ); expect(ecSignatureHex.length).to.be.equal(132); expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT); }); diff --git a/packages/subproviders/test/unit/ledger_subprovider_test.ts b/packages/subproviders/test/unit/ledger_subprovider_test.ts index d3a3fbe26..ad1154831 100644 --- a/packages/subproviders/test/unit/ledger_subprovider_test.ts +++ b/packages/subproviders/test/unit/ledger_subprovider_test.ts @@ -82,7 +82,7 @@ describe('LedgerSubprovider', () => { }); it('signs a personal message', async () => { const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); - const ecSignatureHex = await ledgerSubprovider.signPersonalMessageAsync(data); + const ecSignatureHex = await ledgerSubprovider.signPersonalMessageAsync(data, FAKE_ADDRESS); expect(ecSignatureHex).to.be.equal( '0xa6cc284bff14b42bdf5e9286730c152be91719d478605ec46b3bebcd0ae491480652a1a7b742ceb0213d1e744316e285f41f878d8af0b8e632cbca4c279132d001', ); @@ -94,7 +94,7 @@ describe('LedgerSubprovider', () => { return expect( Promise.all([ ledgerSubprovider.getAccountsAsync(), - ledgerSubprovider.signPersonalMessageAsync(data), + ledgerSubprovider.signPersonalMessageAsync(data, FAKE_ADDRESS), ]), ).to.be.rejectedWith(LedgerSubproviderErrors.MultipleOpenConnectionsDisallowed); }); @@ -168,6 +168,7 @@ describe('LedgerSubprovider', () => { gasPrice: '0x00', nonce: '0x00', gas: '0x00', + from: FAKE_ADDRESS, }; const payload = { jsonrpc: '2.0', diff --git a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts index 77e5d35ae..a1a5a2296 100644 --- a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts @@ -36,7 +36,7 @@ describe('MnemonicWalletSubprovider', () => { }); it('signs a personal message', async () => { const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); - const ecSignatureHex = await subprovider.signPersonalMessageAsync(data); + const ecSignatureHex = await subprovider.signPersonalMessageAsync(data, fixtureData.TEST_RPC_ACCOUNT_0); expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT); }); it('signs a transaction', async () => { diff --git a/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts b/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts index 8aaddeaf4..b84aebb18 100644 --- a/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts @@ -32,7 +32,7 @@ describe('PrivateKeyWalletSubprovider', () => { }); it('signs a personal message', async () => { const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); - const ecSignatureHex = await subprovider.signPersonalMessageAsync(data); + const ecSignatureHex = await subprovider.signPersonalMessageAsync(data, fixtureData.TEST_RPC_ACCOUNT_0); expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT); }); it('signs a transaction', async () => { @@ -128,6 +128,23 @@ describe('PrivateKeyWalletSubprovider', () => { }); provider.sendAsync(payload, callback); }); + it('should throw if `address` param is not an address from private key when calling personal_sign', (done: DoneCallback) => { + const nonHexMessage = 'hello world'; + const payload = { + jsonrpc: '2.0', + method: 'personal_sign', + params: [nonHexMessage, fixtureData.TEST_RPC_ACCOUNT_1], + id: 1, + }; + const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { + expect(err).to.not.be.a('null'); + expect(err.message).to.be.equal( + `${WalletSubproviderErrors.FromAddressMissingOrInvalid}: ${fixtureData.TEST_RPC_ACCOUNT_1}`, + ); + done(); + }); + provider.sendAsync(payload, callback); + }); it('should throw if `from` param missing when calling eth_sendTransaction', (done: DoneCallback) => { const tx = { to: '0xafa3f8684e54059998bc3a7b0d2b0da075154d66', @@ -175,7 +192,7 @@ describe('PrivateKeyWalletSubprovider', () => { }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { expect(err).to.not.be.a('null'); - expect(err.message).to.be.equal(`${WalletSubproviderErrors.AddressNotFound}: 0x0`); + expect(err.message).to.be.equal(`${WalletSubproviderErrors.FromAddressMissingOrInvalid}: 0x0`); done(); }); provider.sendAsync(payload, callback); -- cgit From eee190826a44dfe5444f15165577ea73b4f78c2f Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 12:51:57 +1000 Subject: Test signed messages with the second account --- .../subproviders/test/integration/ledger_subprovider_test.ts | 9 ++++++++- .../subproviders/test/unit/mnemonic_wallet_subprovider_test.ts | 5 +++++ packages/subproviders/test/utils/fixture_data.ts | 2 ++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/subproviders/test/integration/ledger_subprovider_test.ts b/packages/subproviders/test/integration/ledger_subprovider_test.ts index 4a831af67..ea5010696 100644 --- a/packages/subproviders/test/integration/ledger_subprovider_test.ts +++ b/packages/subproviders/test/integration/ledger_subprovider_test.ts @@ -59,9 +59,16 @@ describe('LedgerSubprovider', () => { data, fixtureData.TEST_RPC_ACCOUNT_0, ); - expect(ecSignatureHex.length).to.be.equal(132); expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT); }); + it('signs a personal message with second address', async () => { + const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); + const ecSignatureHex = await ledgerSubprovider.signPersonalMessageAsync( + data, + fixtureData.TEST_RPC_ACCOUNT_1, + ); + expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_ACCOUNT_1_SIGNED_RESULT); + }); it('signs a transaction', async () => { const txHex = await ledgerSubprovider.signTransactionAsync(fixtureData.TX_DATA); expect(txHex).to.be.equal(fixtureData.TX_DATA_SIGNED_RESULT); diff --git a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts index a1a5a2296..aacaef9f4 100644 --- a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts @@ -39,6 +39,11 @@ describe('MnemonicWalletSubprovider', () => { const ecSignatureHex = await subprovider.signPersonalMessageAsync(data, fixtureData.TEST_RPC_ACCOUNT_0); expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT); }); + it('signs a personal message with second address', async () => { + const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); + const ecSignatureHex = await subprovider.signPersonalMessageAsync(data, fixtureData.TEST_RPC_ACCOUNT_1); + expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_ACCOUNT_1_SIGNED_RESULT); + }); it('signs a transaction', async () => { const txHex = await subprovider.signTransactionAsync(fixtureData.TX_DATA); expect(txHex).to.be.equal(fixtureData.TX_DATA_SIGNED_RESULT); diff --git a/packages/subproviders/test/utils/fixture_data.ts b/packages/subproviders/test/utils/fixture_data.ts index 62b76e132..f47fbd428 100644 --- a/packages/subproviders/test/utils/fixture_data.ts +++ b/packages/subproviders/test/utils/fixture_data.ts @@ -10,6 +10,8 @@ export const fixtureData = { PERSONAL_MESSAGE_STRING: 'hello world', PERSONAL_MESSAGE_SIGNED_RESULT: '0x1b0ec5e2908e993d0c8ab6b46da46be2688fdf03c7ea6686075de37392e50a7d7fcc531446699132fbda915bd989882e0064d417018773a315fb8d43ed063c9b00', + PERSONAL_MESSAGE_ACCOUNT_1_SIGNED_RESULT: + '0xe7ae0c21d02eb38f2c2a20d9d7876a98cc7ef035b7a4559d49375e2ec735e06f0d0ab0ff92ee56c5ffc28d516e6ed0692d0270feae8796408dbef060c6c7100f01', TESTRPC_DERIVATION_PATH: `m/44'/60'/0'/0`, NETWORK_ID: networkId, TX_DATA: { -- cgit From 65b2c936abdf3a67d471cd17eeed5069825ac3d4 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 12:56:52 +1000 Subject: Tests for signed transactions and multiple accounts --- packages/subproviders/test/integration/ledger_subprovider_test.ts | 5 +++++ packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts | 5 +++++ packages/subproviders/test/utils/fixture_data.ts | 2 ++ 3 files changed, 12 insertions(+) diff --git a/packages/subproviders/test/integration/ledger_subprovider_test.ts b/packages/subproviders/test/integration/ledger_subprovider_test.ts index ea5010696..2db4befb3 100644 --- a/packages/subproviders/test/integration/ledger_subprovider_test.ts +++ b/packages/subproviders/test/integration/ledger_subprovider_test.ts @@ -73,6 +73,11 @@ describe('LedgerSubprovider', () => { const txHex = await ledgerSubprovider.signTransactionAsync(fixtureData.TX_DATA); expect(txHex).to.be.equal(fixtureData.TX_DATA_SIGNED_RESULT); }); + it('signs a transaction with the second address', async () => { + const txData = { ...fixtureData.TX_DATA, from: fixtureData.TEST_RPC_ACCOUNT_1 }; + const txHex = await ledgerSubprovider.signTransactionAsync(txData); + expect(txHex).to.be.equal(fixtureData.TX_DATA_ACCOUNT_1_SIGNED_RESULT); + }); }); describe('calls through a provider', () => { let defaultProvider: Web3ProviderEngine; diff --git a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts index aacaef9f4..2bc84abc1 100644 --- a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts @@ -48,6 +48,11 @@ describe('MnemonicWalletSubprovider', () => { const txHex = await subprovider.signTransactionAsync(fixtureData.TX_DATA); expect(txHex).to.be.equal(fixtureData.TX_DATA_SIGNED_RESULT); }); + it('signs a transaction with the second address', async () => { + const txData = { ...fixtureData.TX_DATA, from: fixtureData.TEST_RPC_ACCOUNT_1 }; + const txHex = await subprovider.signTransactionAsync(txData); + expect(txHex).to.be.equal(fixtureData.TX_DATA_ACCOUNT_1_SIGNED_RESULT); + }); }); describe('failure cases', () => { it('throws an error if account cannot be found', async () => { diff --git a/packages/subproviders/test/utils/fixture_data.ts b/packages/subproviders/test/utils/fixture_data.ts index f47fbd428..57b69b2f8 100644 --- a/packages/subproviders/test/utils/fixture_data.ts +++ b/packages/subproviders/test/utils/fixture_data.ts @@ -26,4 +26,6 @@ export const fixtureData = { // This is the signed result of the abouve Transaction Data TX_DATA_SIGNED_RESULT: '0xf85f8080822710940000000000000000000000000000000000000000808078a0712854c73c69445cc1b22a7c3d7312ff9a97fe4ffba35fd636e8236b211b6e7ca0647cee031615e52d916c7c707025bc64ad525d8f1b9876c3435a863b42743178', + TX_DATA_ACCOUNT_1_SIGNED_RESULT: + '0xf85f8080822710940000000000000000000000000000000000000000808078a04b02af7ff3f18ce114b601542cc8ebdc50921354f75dd510d31793453a0710e6a0540082a01e475465801b8186a2edc79ec1a2dcf169b9781c25a58a417023c9ca', }; -- cgit From 4017c172a217b4ba225560cb358f04d2227b6d69 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 13:43:43 +1000 Subject: Iterator pattern for walking derived keys --- packages/subproviders/src/subproviders/ledger.ts | 3 +- packages/subproviders/src/utils/wallet_utils.ts | 101 +++++++++++++---------- packages/subproviders/tsconfig.json | 3 +- 3 files changed, 62 insertions(+), 45 deletions(-) diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 3c0442e4c..6c685c84d 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -88,9 +88,8 @@ export class LedgerSubprovider extends BaseWalletSubprovider { public async getAccountsAsync( numberOfAccounts: number = walletUtils.DEFAULT_NUM_ADDRESSES_TO_FETCH, ): Promise { - const offset = 0; const initialHDerivedKey = await this._initialDerivedKeyAsync(); - const derivedKeys = walletUtils.calculateDerivedHDKeys(initialHDerivedKey, numberOfAccounts, offset); + const derivedKeys = walletUtils.calculateDerivedHDKeys(initialHDerivedKey, numberOfAccounts); const accounts = _.map(derivedKeys, 'address'); return accounts; } diff --git a/packages/subproviders/src/utils/wallet_utils.ts b/packages/subproviders/src/utils/wallet_utils.ts index 0c3e895b6..bd1851d9a 100644 --- a/packages/subproviders/src/utils/wallet_utils.ts +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -6,64 +6,81 @@ import { DerivedHDKey, WalletSubproviderErrors } from '../types'; const DEFAULT_ADDRESS_SEARCH_OFFSET = 0; const BATCH_SIZE = 10; +const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; + +class DerivedHDKeyIterator implements IterableIterator { + private _initialDerivedKey: DerivedHDKey; + private _searchLimit: number; + private _index: number; + + constructor(initialDerivedKey: DerivedHDKey, searchLimit: number = DEFAULT_ADDRESS_SEARCH_OFFSET) { + this._searchLimit = searchLimit; + this._initialDerivedKey = initialDerivedKey; + this._index = 0; + } + + public next(): IteratorResult { + const derivationPath = this._initialDerivedKey.derivationPath; + const derivationIndex = this._index; + // If the DerivedHDKey is a child then we walk relative, if not we walk the full derivation path + const path = this._initialDerivedKey.isChildKey + ? `m/${derivationIndex}` + : `m/${derivationPath}/${derivationIndex}`; + const hdKey = this._initialDerivedKey.hdKey.derive(path); + const address = walletUtils.addressOfHDKey(hdKey); + const derivedKey: DerivedHDKey = { + address, + hdKey, + derivationPath, + derivationIndex, + isChildKey: this._initialDerivedKey.isChildKey, + }; + const done = this._index === this._searchLimit; + this._index++; + return { + done, + value: derivedKey, + }; + } + + public [Symbol.iterator](): IterableIterator { + return this; + } +} export const walletUtils = { + DEFAULT_ADDRESS_SEARCH_LIMIT, DEFAULT_NUM_ADDRESSES_TO_FETCH: 10, - DEFAULT_ADDRESS_SEARCH_LIMIT: 1000, - calculateDerivedHDKeys( - initialDerivedKey: DerivedHDKey, - searchLimit: number, - offset: number = DEFAULT_ADDRESS_SEARCH_OFFSET, - ): DerivedHDKey[] { + calculateDerivedHDKeys(initialDerivedKey: DerivedHDKey, searchLimit: number): DerivedHDKey[] { const derivedKeys: DerivedHDKey[] = []; - _.times(searchLimit, i => { - const derivationPath = initialDerivedKey.derivationPath; - const derivationIndex = offset + i; - // If the DerivedHDKey is a child then we walk relative, if not we walk the full derivation path - const path = initialDerivedKey.isChildKey - ? `m/${derivationIndex}` - : `m/${derivationPath}/${derivationIndex}`; - const hdKey = initialDerivedKey.hdKey.derive(path); - const address = walletUtils.addressOfHDKey(hdKey); - const derivedKey: DerivedHDKey = { - address, - hdKey, - derivationPath, - derivationIndex, - isChildKey: initialDerivedKey.isChildKey, - }; - derivedKeys.push(derivedKey); - }); + const derivedKeyIterator = new DerivedHDKeyIterator(initialDerivedKey, searchLimit); + for (const key of derivedKeyIterator) { + derivedKeys.push(key); + } return derivedKeys; }, - addressOfHDKey(hdKey: HDNode): string { - const shouldSanitizePublicKey = true; - const derivedPublicKey = hdKey.publicKey; - const ethereumAddressUnprefixed = ethUtil - .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) - .toString('hex'); - const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed); - return address; - }, findDerivedKeyByAddress( address: string, initialDerivedKey: DerivedHDKey, searchLimit: number, ): DerivedHDKey | undefined { let matchedKey: DerivedHDKey | undefined; - for (let index = 0; index < searchLimit; index = index + BATCH_SIZE) { - const derivedKeys = walletUtils.calculateDerivedHDKeys(initialDerivedKey, BATCH_SIZE, index); - matchedKey = _.find(derivedKeys, derivedKey => derivedKey.address === address); - if (matchedKey) { + const derivedKeyIterator = new DerivedHDKeyIterator(initialDerivedKey, searchLimit); + for (const key of derivedKeyIterator) { + if (key.address === address) { + matchedKey = key; break; } } return matchedKey; }, - - _firstDerivedKey(initialDerivedKey: DerivedHDKey): DerivedHDKey { - const derivedKeys = walletUtils.calculateDerivedHDKeys(initialDerivedKey, 1, 0); - const firstDerivedKey = derivedKeys[0]; - return firstDerivedKey; + addressOfHDKey(hdKey: HDNode): string { + const shouldSanitizePublicKey = true; + const derivedPublicKey = hdKey.publicKey; + const ethereumAddressUnprefixed = ethUtil + .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) + .toString('hex'); + const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed); + return address; }, }; diff --git a/packages/subproviders/tsconfig.json b/packages/subproviders/tsconfig.json index e35816553..72dfee80b 100644 --- a/packages/subproviders/tsconfig.json +++ b/packages/subproviders/tsconfig.json @@ -1,7 +1,8 @@ { "extends": "../../tsconfig", "compilerOptions": { - "outDir": "lib" + "outDir": "lib", + "downlevelIteration": true }, "include": ["./src/**/*", "./test/**/*"] } -- cgit From a824957de7e692d9932a591bb5730ba798bcd8ed Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 13:46:01 +1000 Subject: Follow file naming pattern for mnemonic and private key subprovider --- packages/subproviders/src/index.ts | 4 +- .../src/subproviders/mnemonic_wallet.ts | 134 +++++++++++++++++++++ .../subproviders/mnemonic_wallet_subprovider.ts | 134 --------------------- .../src/subproviders/private_key_wallet.ts | 80 ++++++++++++ .../subproviders/private_key_wallet_subprovider.ts | 80 ------------ 5 files changed, 216 insertions(+), 216 deletions(-) create mode 100644 packages/subproviders/src/subproviders/mnemonic_wallet.ts delete mode 100644 packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts create mode 100644 packages/subproviders/src/subproviders/private_key_wallet.ts delete mode 100644 packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts diff --git a/packages/subproviders/src/index.ts b/packages/subproviders/src/index.ts index 9c30c6ba1..01aec956a 100644 --- a/packages/subproviders/src/index.ts +++ b/packages/subproviders/src/index.ts @@ -12,8 +12,8 @@ export { LedgerSubprovider } from './subproviders/ledger'; export { GanacheSubprovider } from './subproviders/ganache'; export { Subprovider } from './subproviders/subprovider'; export { NonceTrackerSubprovider } from './subproviders/nonce_tracker'; -export { PrivateKeyWalletSubprovider } from './subproviders/private_key_wallet_subprovider'; -export { MnemonicWalletSubprovider } from './subproviders/mnemonic_wallet_subprovider'; +export { PrivateKeyWalletSubprovider } from './subproviders/private_key_wallet'; +export { MnemonicWalletSubprovider } from './subproviders/mnemonic_wallet'; export { Callback, ErrorCallback, diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet.ts b/packages/subproviders/src/subproviders/mnemonic_wallet.ts new file mode 100644 index 000000000..40f584250 --- /dev/null +++ b/packages/subproviders/src/subproviders/mnemonic_wallet.ts @@ -0,0 +1,134 @@ +import { assert } from '@0xproject/assert'; +import * as bip39 from 'bip39'; +import ethUtil = require('ethereumjs-util'); +import HDNode = require('hdkey'); +import * as _ from 'lodash'; + +import { DerivedHDKey, PartialTxParams, WalletSubproviderErrors } from '../types'; +import { walletUtils } from '../utils/wallet_utils'; + +import { BaseWalletSubprovider } from './base_wallet_subprovider'; +import { PrivateKeyWalletSubprovider } from './private_key_wallet'; + +const DEFAULT_DERIVATION_PATH = `44'/60'/0'/0`; + +/** + * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. + * This subprovider intercepts all account related RPC requests (e.g message/transaction signing, etc...) and handles + * all requests with accounts derived from the supplied mnemonic. + */ +export class MnemonicWalletSubprovider extends BaseWalletSubprovider { + private _addressSearchLimit: number; + private _derivationPath: string; + private _derivedKey: DerivedHDKey; + /** + * Instantiates a MnemonicWalletSubprovider. Defaults to derivationPath set to `44'/60'/0'/0`. + * This is the default in TestRPC/Ganache, this can be overridden if desired. + * @param mnemonic The mnemonic seed + * @param derivationPath The derivation path, defaults to `44'/60'/0'/0` + * @param addressSearchLimit The limit on address search attempts before raising `WalletSubproviderErrors.AddressNotFound` + * @return MnemonicWalletSubprovider instance + */ + constructor( + mnemonic: string, + derivationPath: string = DEFAULT_DERIVATION_PATH, + addressSearchLimit: number = walletUtils.DEFAULT_ADDRESS_SEARCH_LIMIT, + ) { + assert.isString('mnemonic', mnemonic); + assert.isString('derivationPath', derivationPath); + assert.isNumber('addressSearchLimit', addressSearchLimit); + super(); + const seed = bip39.mnemonicToSeed(mnemonic); + const hdKey = HDNode.fromMasterSeed(seed); + this._derivationPath = derivationPath; + this._derivedKey = { + address: walletUtils.addressOfHDKey(hdKey), + derivationPath: this._derivationPath, + derivationIndex: 0, + hdKey, + isChildKey: false, + }; + this._addressSearchLimit = addressSearchLimit; + } + /** + * Retrieve the set derivation path + * @returns derivation path + */ + public getPath(): string { + return this._derivationPath; + } + /** + * Set a desired derivation path when computing the available user addresses + * @param derivationPath The desired derivation path (e.g `44'/60'/0'`) + */ + public setPath(derivationPath: string) { + this._derivationPath = derivationPath; + this._derivedKey = { + ...this._derivedKey, + derivationPath: this._derivationPath, + }; + } + /** + * Retrieve the accounts associated with the mnemonic. + * This method is implicitly called when issuing a `eth_accounts` JSON RPC request + * via your providerEngine instance. + * @param numberOfAccounts Number of accounts to retrieve (default: 10) + * @return An array of accounts + */ + public async getAccountsAsync( + numberOfAccounts: number = walletUtils.DEFAULT_NUM_ADDRESSES_TO_FETCH, + ): Promise { + const derivedKeys = walletUtils.calculateDerivedHDKeys(this._derivedKey, numberOfAccounts); + const accounts = _.map(derivedKeys, 'address'); + return accounts; + } + + /** + * Signs a transaction with the from account (if specificed in txParams) or the first account. + * If you've added this Subprovider to your app's provider, you can simply send + * an `eth_sendTransaction` JSON RPC request, and * this method will be called auto-magically. + * If you are not using this via a ProviderEngine instance, you can call it directly. + * @param txParams Parameters of the transaction to sign + * @return Signed transaction hex string + */ + public async signTransactionAsync(txParams: PartialTxParams): Promise { + const privateKeyWallet = this._privateKeyWalletFromAddress(txParams.from); + const signedTx = privateKeyWallet.signTransactionAsync(txParams); + return signedTx; + } + /** + * Sign a personal Ethereum signed message. The signing address used will be + * address provided or the first address derived from the set path. + * If you've added the MnemonicWalletSubprovider to your app's provider, you can simply send an `eth_sign` + * or `personal_sign` JSON RPC request, and this method will be called auto-magically. + * If you are not using this via a ProviderEngine instance, you can call it directly. + * @param data Message to sign + * @param address Address to sign with + * @return Signature hex string (order: rsv) + */ + public async signPersonalMessageAsync(data: string, address: string): Promise { + const privateKeyWallet = this._privateKeyWalletFromAddress(address); + const sig = await privateKeyWallet.signPersonalMessageAsync(data, address); + return sig; + } + private _privateKeyWalletFromAddress(address: string): PrivateKeyWalletSubprovider { + const derivedKey = this._findDerivedKeyByPublicAddress(address); + const privateKeyHex = derivedKey.hdKey.privateKey.toString('hex'); + const privateKeyWallet = new PrivateKeyWalletSubprovider(privateKeyHex); + return privateKeyWallet; + } + private _findDerivedKeyByPublicAddress(address: string): DerivedHDKey { + if (_.isUndefined(address)) { + throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); + } + const matchedDerivedKey = walletUtils.findDerivedKeyByAddress( + address, + this._derivedKey, + this._addressSearchLimit, + ); + if (_.isUndefined(matchedDerivedKey)) { + throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`); + } + return matchedDerivedKey; + } +} diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts b/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts deleted file mode 100644 index 730453bc6..000000000 --- a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts +++ /dev/null @@ -1,134 +0,0 @@ -import { assert } from '@0xproject/assert'; -import * as bip39 from 'bip39'; -import ethUtil = require('ethereumjs-util'); -import HDNode = require('hdkey'); -import * as _ from 'lodash'; - -import { DerivedHDKey, PartialTxParams, WalletSubproviderErrors } from '../types'; -import { walletUtils } from '../utils/wallet_utils'; - -import { BaseWalletSubprovider } from './base_wallet_subprovider'; -import { PrivateKeyWalletSubprovider } from './private_key_wallet_subprovider'; - -const DEFAULT_DERIVATION_PATH = `44'/60'/0'/0`; - -/** - * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. - * This subprovider intercepts all account related RPC requests (e.g message/transaction signing, etc...) and handles - * all requests with accounts derived from the supplied mnemonic. - */ -export class MnemonicWalletSubprovider extends BaseWalletSubprovider { - private _addressSearchLimit: number; - private _derivationPath: string; - private _derivedKey: DerivedHDKey; - /** - * Instantiates a MnemonicWalletSubprovider. Defaults to derivationPath set to `44'/60'/0'/0`. - * This is the default in TestRPC/Ganache, this can be overridden if desired. - * @param mnemonic The mnemonic seed - * @param derivationPath The derivation path, defaults to `44'/60'/0'/0` - * @param addressSearchLimit The limit on address search attempts before raising `WalletSubproviderErrors.AddressNotFound` - * @return MnemonicWalletSubprovider instance - */ - constructor( - mnemonic: string, - derivationPath: string = DEFAULT_DERIVATION_PATH, - addressSearchLimit: number = walletUtils.DEFAULT_ADDRESS_SEARCH_LIMIT, - ) { - assert.isString('mnemonic', mnemonic); - assert.isString('derivationPath', derivationPath); - assert.isNumber('addressSearchLimit', addressSearchLimit); - super(); - const seed = bip39.mnemonicToSeed(mnemonic); - const hdKey = HDNode.fromMasterSeed(seed); - this._derivationPath = derivationPath; - this._derivedKey = { - address: walletUtils.addressOfHDKey(hdKey), - derivationPath: this._derivationPath, - derivationIndex: 0, - hdKey, - isChildKey: false, - }; - this._addressSearchLimit = addressSearchLimit; - } - /** - * Retrieve the set derivation path - * @returns derivation path - */ - public getPath(): string { - return this._derivationPath; - } - /** - * Set a desired derivation path when computing the available user addresses - * @param derivationPath The desired derivation path (e.g `44'/60'/0'`) - */ - public setPath(derivationPath: string) { - this._derivationPath = derivationPath; - this._derivedKey = { - ...this._derivedKey, - derivationPath: this._derivationPath, - }; - } - /** - * Retrieve the accounts associated with the mnemonic. - * This method is implicitly called when issuing a `eth_accounts` JSON RPC request - * via your providerEngine instance. - * @param numberOfAccounts Number of accounts to retrieve (default: 10) - * @return An array of accounts - */ - public async getAccountsAsync( - numberOfAccounts: number = walletUtils.DEFAULT_NUM_ADDRESSES_TO_FETCH, - ): Promise { - const derivedKeys = walletUtils.calculateDerivedHDKeys(this._derivedKey, numberOfAccounts); - const accounts = _.map(derivedKeys, 'address'); - return accounts; - } - - /** - * Signs a transaction with the from account (if specificed in txParams) or the first account. - * If you've added this Subprovider to your app's provider, you can simply send - * an `eth_sendTransaction` JSON RPC request, and * this method will be called auto-magically. - * If you are not using this via a ProviderEngine instance, you can call it directly. - * @param txParams Parameters of the transaction to sign - * @return Signed transaction hex string - */ - public async signTransactionAsync(txParams: PartialTxParams): Promise { - const privateKeyWallet = this._privateKeyWalletFromAddress(txParams.from); - const signedTx = privateKeyWallet.signTransactionAsync(txParams); - return signedTx; - } - /** - * Sign a personal Ethereum signed message. The signing address used will be - * address provided or the first address derived from the set path. - * If you've added the MnemonicWalletSubprovider to your app's provider, you can simply send an `eth_sign` - * or `personal_sign` JSON RPC request, and this method will be called auto-magically. - * If you are not using this via a ProviderEngine instance, you can call it directly. - * @param data Message to sign - * @param address Address to sign with - * @return Signature hex string (order: rsv) - */ - public async signPersonalMessageAsync(data: string, address: string): Promise { - const privateKeyWallet = this._privateKeyWalletFromAddress(address); - const sig = await privateKeyWallet.signPersonalMessageAsync(data, address); - return sig; - } - private _privateKeyWalletFromAddress(address: string): PrivateKeyWalletSubprovider { - const derivedKey = this._findDerivedKeyByPublicAddress(address); - const privateKeyHex = derivedKey.hdKey.privateKey.toString('hex'); - const privateKeyWallet = new PrivateKeyWalletSubprovider(privateKeyHex); - return privateKeyWallet; - } - private _findDerivedKeyByPublicAddress(address: string): DerivedHDKey { - if (_.isUndefined(address)) { - throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); - } - const matchedDerivedKey = walletUtils.findDerivedKeyByAddress( - address, - this._derivedKey, - this._addressSearchLimit, - ); - if (_.isUndefined(matchedDerivedKey)) { - throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`); - } - return matchedDerivedKey; - } -} diff --git a/packages/subproviders/src/subproviders/private_key_wallet.ts b/packages/subproviders/src/subproviders/private_key_wallet.ts new file mode 100644 index 000000000..f8afab722 --- /dev/null +++ b/packages/subproviders/src/subproviders/private_key_wallet.ts @@ -0,0 +1,80 @@ +import { assert } from '@0xproject/assert'; +import EthereumTx = require('ethereumjs-tx'); +import * as ethUtil from 'ethereumjs-util'; +import * as _ from 'lodash'; + +import { PartialTxParams, WalletSubproviderErrors } from '../types'; + +import { BaseWalletSubprovider } from './base_wallet_subprovider'; + +/** + * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. + * This subprovider intercepts all account related RPC requests (e.g message/transaction signing, etc...) and handles + * all requests with the supplied Ethereum private key. + */ +export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { + private _address: string; + private _privateKeyBuffer: Buffer; + /** + * Instantiates a PrivateKeyWalletSubprovider. + * @param privateKey The private key of the ethereum address + * @return PrivateKeyWalletSubprovider instance + */ + constructor(privateKey: string) { + assert.isString('privateKey', privateKey); + super(); + this._privateKeyBuffer = new Buffer(privateKey, 'hex'); + this._address = `0x${ethUtil.privateToAddress(this._privateKeyBuffer).toString('hex')}`; + } + /** + * Retrieve the account associated with the supplied private key. + * This method is implicitly called when issuing a `eth_accounts` JSON RPC request + * via your providerEngine instance. + * @return An array of accounts + */ + public async getAccountsAsync(): Promise { + return [this._address]; + } + /** + * Sign a transaction with the private key. If you've added this Subprovider to your + * app's provider, you can simply send an `eth_sendTransaction` JSON RPC request, and + * this method will be called auto-magically. If you are not using this via a ProviderEngine + * instance, you can call it directly. + * @param txParams Parameters of the transaction to sign + * @return Signed transaction hex string + */ + public async signTransactionAsync(txParams: PartialTxParams): Promise { + PrivateKeyWalletSubprovider._validateTxParams(txParams); + if (!_.isUndefined(txParams.from) && txParams.from !== this._address) { + throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${txParams.from}`); + } + const tx = new EthereumTx(txParams); + tx.sign(this._privateKeyBuffer); + const rawTx = `0x${tx.serialize().toString('hex')}`; + return rawTx; + } + /** + * Sign a personal Ethereum signed message. The signing address will be + * calculated from the private key, if an address is provided it must match the address calculated from the private key. + * If you've added this Subprovider to your app's provider, you can simply send an `eth_sign` + * or `personal_sign` JSON RPC request, and this method will be called auto-magically. + * If you are not using this via a ProviderEngine instance, you can call it directly. + * @param data Message to sign + * @param address Address to sign with + * @return Signature hex string (order: rsv) + */ + public async signPersonalMessageAsync(data: string, address: string): Promise { + if (_.isUndefined(data)) { + throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage); + } + if (_.isUndefined(address) || address !== this._address) { + throw new Error(`${WalletSubproviderErrors.FromAddressMissingOrInvalid}: ${address}`); + } + assert.isHexString('data', data); + const dataBuff = ethUtil.toBuffer(data); + const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff); + const sig = ethUtil.ecsign(msgHashBuff, this._privateKeyBuffer); + const rpcSig = ethUtil.toRpcSig(sig.v, sig.r, sig.s); + return rpcSig; + } +} diff --git a/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts b/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts deleted file mode 100644 index f8afab722..000000000 --- a/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { assert } from '@0xproject/assert'; -import EthereumTx = require('ethereumjs-tx'); -import * as ethUtil from 'ethereumjs-util'; -import * as _ from 'lodash'; - -import { PartialTxParams, WalletSubproviderErrors } from '../types'; - -import { BaseWalletSubprovider } from './base_wallet_subprovider'; - -/** - * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. - * This subprovider intercepts all account related RPC requests (e.g message/transaction signing, etc...) and handles - * all requests with the supplied Ethereum private key. - */ -export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { - private _address: string; - private _privateKeyBuffer: Buffer; - /** - * Instantiates a PrivateKeyWalletSubprovider. - * @param privateKey The private key of the ethereum address - * @return PrivateKeyWalletSubprovider instance - */ - constructor(privateKey: string) { - assert.isString('privateKey', privateKey); - super(); - this._privateKeyBuffer = new Buffer(privateKey, 'hex'); - this._address = `0x${ethUtil.privateToAddress(this._privateKeyBuffer).toString('hex')}`; - } - /** - * Retrieve the account associated with the supplied private key. - * This method is implicitly called when issuing a `eth_accounts` JSON RPC request - * via your providerEngine instance. - * @return An array of accounts - */ - public async getAccountsAsync(): Promise { - return [this._address]; - } - /** - * Sign a transaction with the private key. If you've added this Subprovider to your - * app's provider, you can simply send an `eth_sendTransaction` JSON RPC request, and - * this method will be called auto-magically. If you are not using this via a ProviderEngine - * instance, you can call it directly. - * @param txParams Parameters of the transaction to sign - * @return Signed transaction hex string - */ - public async signTransactionAsync(txParams: PartialTxParams): Promise { - PrivateKeyWalletSubprovider._validateTxParams(txParams); - if (!_.isUndefined(txParams.from) && txParams.from !== this._address) { - throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${txParams.from}`); - } - const tx = new EthereumTx(txParams); - tx.sign(this._privateKeyBuffer); - const rawTx = `0x${tx.serialize().toString('hex')}`; - return rawTx; - } - /** - * Sign a personal Ethereum signed message. The signing address will be - * calculated from the private key, if an address is provided it must match the address calculated from the private key. - * If you've added this Subprovider to your app's provider, you can simply send an `eth_sign` - * or `personal_sign` JSON RPC request, and this method will be called auto-magically. - * If you are not using this via a ProviderEngine instance, you can call it directly. - * @param data Message to sign - * @param address Address to sign with - * @return Signature hex string (order: rsv) - */ - public async signPersonalMessageAsync(data: string, address: string): Promise { - if (_.isUndefined(data)) { - throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage); - } - if (_.isUndefined(address) || address !== this._address) { - throw new Error(`${WalletSubproviderErrors.FromAddressMissingOrInvalid}: ${address}`); - } - assert.isHexString('data', data); - const dataBuff = ethUtil.toBuffer(data); - const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff); - const sig = ethUtil.ecsign(msgHashBuff, this._privateKeyBuffer); - const rpcSig = ethUtil.toRpcSig(sig.v, sig.r, sig.s); - return rpcSig; - } -} -- cgit From 260ab2d4134e24a3a2f3fab845fa72c1e1766a3e Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 14:04:27 +1000 Subject: Update changelog and add derivationBasePath --- packages/subproviders/CHANGELOG.json | 3 ++- packages/subproviders/src/subproviders/ledger.ts | 26 +++++++++++----------- .../src/subproviders/mnemonic_wallet.ts | 19 ++++++++-------- packages/subproviders/src/types.ts | 3 ++- packages/subproviders/src/utils/wallet_utils.ts | 11 ++++----- 5 files changed, 33 insertions(+), 29 deletions(-) diff --git a/packages/subproviders/CHANGELOG.json b/packages/subproviders/CHANGELOG.json index ccf807695..554fbd0cf 100644 --- a/packages/subproviders/CHANGELOG.json +++ b/packages/subproviders/CHANGELOG.json @@ -7,7 +7,8 @@ "pr": 506 }, { - "note": "Add mnemonic wallet subprovider, deprecating our truffle-hdwallet-provider fork", + "note": + "Add mnemonic wallet subprovider, deprecating our truffle-hdwallet-provider fork. Support multiple addresses in ledger and mnemonic wallets", "pr": 507 } ] diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 6c685c84d..975893f8a 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -24,7 +24,6 @@ import { BaseWalletSubprovider } from './base_wallet_subprovider'; const DEFAULT_DERIVATION_PATH = `44'/60'/0'`; const ASK_FOR_ON_DEVICE_CONFIRMATION = false; const SHOULD_GET_CHAIN_CODE = true; -const IS_CHILD_KEY = true; /** * Subprovider for interfacing with a user's [Ledger Nano S](https://www.ledgerwallet.com/products/ledger-nano-s). @@ -35,7 +34,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { private _nonceLock = new Lock(); private _connectionLock = new Lock(); private _networkId: number; - private _derivationPath: string; + private _derivationBasePath: string; private _ledgerEthereumClientFactoryAsync: LedgerEthereumClientFactoryAsync; private _ledgerClientIfExists?: LedgerEthereumClient; private _shouldAlwaysAskForConfirmation: boolean; @@ -50,7 +49,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { super(); this._networkId = config.networkId; this._ledgerEthereumClientFactoryAsync = config.ledgerEthereumClientFactoryAsync; - this._derivationPath = config.derivationPath || DEFAULT_DERIVATION_PATH; + this._derivationBasePath = config.derivationPath || DEFAULT_DERIVATION_PATH; this._shouldAlwaysAskForConfirmation = !_.isUndefined(config.accountFetchingConfigs) && !_.isUndefined(config.accountFetchingConfigs.shouldAskForOnDeviceConfirmation) @@ -67,14 +66,14 @@ export class LedgerSubprovider extends BaseWalletSubprovider { * @returns derivation path */ public getPath(): string { - return this._derivationPath; + return this._derivationBasePath; } /** * Set a desired derivation path when computing the available user addresses * @param derivationPath The desired derivation path (e.g `44'/60'/0'`) */ public setPath(derivationPath: string) { - this._derivationPath = derivationPath; + this._derivationBasePath = derivationPath; } /** * Retrieve a users Ledger accounts. The accounts are derived from the derivationPath, @@ -88,8 +87,8 @@ export class LedgerSubprovider extends BaseWalletSubprovider { public async getAccountsAsync( numberOfAccounts: number = walletUtils.DEFAULT_NUM_ADDRESSES_TO_FETCH, ): Promise { - const initialHDerivedKey = await this._initialDerivedKeyAsync(); - const derivedKeys = walletUtils.calculateDerivedHDKeys(initialHDerivedKey, numberOfAccounts); + const initialDerivedKey = await this._initialDerivedKeyAsync(); + const derivedKeys = walletUtils.calculateDerivedHDKeys(initialDerivedKey, numberOfAccounts); const accounts = _.map(derivedKeys, 'address'); return accounts; } @@ -117,8 +116,8 @@ export class LedgerSubprovider extends BaseWalletSubprovider { const txHex = tx.serialize().toString('hex'); try { - const derivationPath = `${derivedKey.derivationPath}/${derivedKey.derivationIndex}`; - const result = await ledgerClient.signTransaction(derivationPath, txHex); + const fullDerivationPath = derivedKey.derivationPath; + const result = await ledgerClient.signTransaction(fullDerivationPath, txHex); // Store signature in transaction tx.r = Buffer.from(result.r, 'hex'); tx.s = Buffer.from(result.s, 'hex'); @@ -161,8 +160,8 @@ export class LedgerSubprovider extends BaseWalletSubprovider { const ledgerClient = await this._createLedgerClientAsync(); try { - const derivationPath = `${derivedKey.derivationPath}/${derivedKey.derivationIndex}`; - const result = await ledgerClient.signPersonalMessage(derivationPath, ethUtil.stripHexPrefix(data)); + const fullDerivationPath = derivedKey.derivationPath; + const result = await ledgerClient.signPersonalMessage(fullDerivationPath, ethUtil.stripHexPrefix(data)); const v = result.v - 27; let vHex = v.toString(16); if (vHex.length < 2) { @@ -203,7 +202,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { let ledgerResponse; try { ledgerResponse = await ledgerClient.getAddress( - this._derivationPath, + this._derivationBasePath, this._shouldAlwaysAskForConfirmation, SHOULD_GET_CHAIN_CODE, ); @@ -217,7 +216,8 @@ export class LedgerSubprovider extends BaseWalletSubprovider { hdKey, address: ledgerResponse.address, isChildKey: true, - derivationPath: this._derivationPath, + derivationBasePath: this._derivationBasePath, + derivationPath: `${this._derivationBasePath}/${0}`, derivationIndex: 0, }; } diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet.ts b/packages/subproviders/src/subproviders/mnemonic_wallet.ts index 40f584250..972f7e65e 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet.ts @@ -19,31 +19,32 @@ const DEFAULT_DERIVATION_PATH = `44'/60'/0'/0`; */ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { private _addressSearchLimit: number; - private _derivationPath: string; + private _derivationBasePath: string; private _derivedKey: DerivedHDKey; /** * Instantiates a MnemonicWalletSubprovider. Defaults to derivationPath set to `44'/60'/0'/0`. * This is the default in TestRPC/Ganache, this can be overridden if desired. * @param mnemonic The mnemonic seed - * @param derivationPath The derivation path, defaults to `44'/60'/0'/0` + * @param derivationBasePath The derivation path, defaults to `44'/60'/0'/0` * @param addressSearchLimit The limit on address search attempts before raising `WalletSubproviderErrors.AddressNotFound` * @return MnemonicWalletSubprovider instance */ constructor( mnemonic: string, - derivationPath: string = DEFAULT_DERIVATION_PATH, + derivationBasePath: string = DEFAULT_DERIVATION_PATH, addressSearchLimit: number = walletUtils.DEFAULT_ADDRESS_SEARCH_LIMIT, ) { assert.isString('mnemonic', mnemonic); - assert.isString('derivationPath', derivationPath); + assert.isString('derivationPath', derivationBasePath); assert.isNumber('addressSearchLimit', addressSearchLimit); super(); const seed = bip39.mnemonicToSeed(mnemonic); const hdKey = HDNode.fromMasterSeed(seed); - this._derivationPath = derivationPath; + this._derivationBasePath = derivationBasePath; this._derivedKey = { address: walletUtils.addressOfHDKey(hdKey), - derivationPath: this._derivationPath, + derivationBasePath: this._derivationBasePath, + derivationPath: `${this._derivationBasePath}/${0}`, derivationIndex: 0, hdKey, isChildKey: false, @@ -55,17 +56,17 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @returns derivation path */ public getPath(): string { - return this._derivationPath; + return this._derivationBasePath; } /** * Set a desired derivation path when computing the available user addresses * @param derivationPath The desired derivation path (e.g `44'/60'/0'`) */ public setPath(derivationPath: string) { - this._derivationPath = derivationPath; + this._derivationBasePath = derivationPath; this._derivedKey = { ...this._derivedKey, - derivationPath: this._derivationPath, + derivationBasePath: this._derivationBasePath, }; } /** diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts index 140c7c2df..c76be1bf8 100644 --- a/packages/subproviders/src/types.ts +++ b/packages/subproviders/src/types.ts @@ -114,8 +114,9 @@ export enum NonceSubproviderErrors { } export interface DerivedHDKey { address: string; - derivationPath: string; derivationIndex: number; + derivationBasePath: string; + derivationPath: string; hdKey: HDNode; isChildKey: boolean; } diff --git a/packages/subproviders/src/utils/wallet_utils.ts b/packages/subproviders/src/utils/wallet_utils.ts index bd1851d9a..2d9d14e44 100644 --- a/packages/subproviders/src/utils/wallet_utils.ts +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -20,18 +20,19 @@ class DerivedHDKeyIterator implements IterableIterator { } public next(): IteratorResult { - const derivationPath = this._initialDerivedKey.derivationPath; + const derivationBasePath = this._initialDerivedKey.derivationBasePath; const derivationIndex = this._index; // If the DerivedHDKey is a child then we walk relative, if not we walk the full derivation path - const path = this._initialDerivedKey.isChildKey - ? `m/${derivationIndex}` - : `m/${derivationPath}/${derivationIndex}`; + const fullDerivationPath = `m/${derivationBasePath}/${derivationIndex}`; + const relativeDerivationPath = `m/${derivationIndex}`; + const path = this._initialDerivedKey.isChildKey ? relativeDerivationPath : fullDerivationPath; const hdKey = this._initialDerivedKey.hdKey.derive(path); const address = walletUtils.addressOfHDKey(hdKey); const derivedKey: DerivedHDKey = { address, hdKey, - derivationPath, + derivationPath: fullDerivationPath, + derivationBasePath: this._initialDerivedKey.derivationBasePath, derivationIndex, isChildKey: this._initialDerivedKey.isChildKey, }; -- cgit From 4aa67e292504fea307a4e5f15a124349fc769da6 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 14:11:16 +1000 Subject: Update JSDoc for methods in ledger and mnemonic wallet --- packages/subproviders/src/subproviders/ledger.ts | 6 +++--- packages/subproviders/src/subproviders/mnemonic_wallet.ts | 12 ++++++------ packages/subproviders/src/utils/wallet_utils.ts | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 975893f8a..0c037b488 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -140,8 +140,8 @@ export class LedgerSubprovider extends BaseWalletSubprovider { } } /** - * Sign a personal Ethereum signed message. The signing address will be the one - * the provided address. + * Sign a personal Ethereum signed message. The signing account will be the account + * associated with the provided address. * The Ledger adds the Ethereum signed message prefix on-device. If you've added * the LedgerSubprovider to your app's provider, you can simply send an `eth_sign` * or `personal_sign` JSON RPC request, and this method will be called auto-magically. @@ -217,7 +217,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { address: ledgerResponse.address, isChildKey: true, derivationBasePath: this._derivationBasePath, - derivationPath: `${this._derivationBasePath}/${0}`, + derivationPath: `${this._derivationBasePath}/0`, derivationIndex: 0, }; } diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet.ts b/packages/subproviders/src/subproviders/mnemonic_wallet.ts index 972f7e65e..855db21ad 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet.ts @@ -85,7 +85,7 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { } /** - * Signs a transaction with the from account (if specificed in txParams) or the first account. + * Signs a transaction with the from account specificed in txParams. * If you've added this Subprovider to your app's provider, you can simply send * an `eth_sendTransaction` JSON RPC request, and * this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. @@ -93,13 +93,13 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @return Signed transaction hex string */ public async signTransactionAsync(txParams: PartialTxParams): Promise { - const privateKeyWallet = this._privateKeyWalletFromAddress(txParams.from); + const privateKeyWallet = this._privateKeyWalletForAddress(txParams.from); const signedTx = privateKeyWallet.signTransactionAsync(txParams); return signedTx; } /** - * Sign a personal Ethereum signed message. The signing address used will be - * address provided or the first address derived from the set path. + * Sign a personal Ethereum signed message. The signing account will be the account + * associated with the provided address. * If you've added the MnemonicWalletSubprovider to your app's provider, you can simply send an `eth_sign` * or `personal_sign` JSON RPC request, and this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. @@ -108,11 +108,11 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @return Signature hex string (order: rsv) */ public async signPersonalMessageAsync(data: string, address: string): Promise { - const privateKeyWallet = this._privateKeyWalletFromAddress(address); + const privateKeyWallet = this._privateKeyWalletForAddress(address); const sig = await privateKeyWallet.signPersonalMessageAsync(data, address); return sig; } - private _privateKeyWalletFromAddress(address: string): PrivateKeyWalletSubprovider { + private _privateKeyWalletForAddress(address: string): PrivateKeyWalletSubprovider { const derivedKey = this._findDerivedKeyByPublicAddress(address); const privateKeyHex = derivedKey.hdKey.privateKey.toString('hex'); const privateKeyWallet = new PrivateKeyWalletSubprovider(privateKeyHex); diff --git a/packages/subproviders/src/utils/wallet_utils.ts b/packages/subproviders/src/utils/wallet_utils.ts index 2d9d14e44..097d2b82f 100644 --- a/packages/subproviders/src/utils/wallet_utils.ts +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -52,9 +52,9 @@ class DerivedHDKeyIterator implements IterableIterator { export const walletUtils = { DEFAULT_ADDRESS_SEARCH_LIMIT, DEFAULT_NUM_ADDRESSES_TO_FETCH: 10, - calculateDerivedHDKeys(initialDerivedKey: DerivedHDKey, searchLimit: number): DerivedHDKey[] { + calculateDerivedHDKeys(initialDerivedKey: DerivedHDKey, numberOfKeys: number): DerivedHDKey[] { const derivedKeys: DerivedHDKey[] = []; - const derivedKeyIterator = new DerivedHDKeyIterator(initialDerivedKey, searchLimit); + const derivedKeyIterator = new DerivedHDKeyIterator(initialDerivedKey, numberOfKeys); for (const key of derivedKeyIterator) { derivedKeys.push(key); } -- cgit From 3ad693d33409dfd9d61beff3f43c4abaa369c6b1 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 14:22:02 +1000 Subject: Test valid address format but not found --- packages/subproviders/src/subproviders/ledger.ts | 2 +- .../subproviders/src/subproviders/mnemonic_wallet.ts | 7 +++++-- packages/subproviders/src/utils/wallet_utils.ts | 9 +++++---- .../test/unit/mnemonic_wallet_subprovider_test.ts | 16 ++++++++++++---- packages/subproviders/test/utils/fixture_data.ts | 4 +++- 5 files changed, 26 insertions(+), 12 deletions(-) diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 0c037b488..9b2d9d7d0 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -222,7 +222,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { }; } private _findDerivedKeyByPublicAddress(initalHDKey: DerivedHDKey, address: string): DerivedHDKey { - if (_.isUndefined(address)) { + if (_.isUndefined(address) || !addressUtils.isAddress(address)) { throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); } const matchedDerivedKey = walletUtils.findDerivedKeyByAddress(address, initalHDKey, this._addressSearchLimit); diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet.ts b/packages/subproviders/src/subproviders/mnemonic_wallet.ts index 855db21ad..de34a9d11 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet.ts @@ -1,4 +1,5 @@ import { assert } from '@0xproject/assert'; +import { addressUtils } from '@0xproject/utils'; import * as bip39 from 'bip39'; import ethUtil = require('ethereumjs-util'); import HDNode = require('hdkey'); @@ -43,8 +44,10 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { this._derivationBasePath = derivationBasePath; this._derivedKey = { address: walletUtils.addressOfHDKey(hdKey), + // HACK this isn't the base path for this root key, but is is the base path + // we want all of the derived children to spawn from derivationBasePath: this._derivationBasePath, - derivationPath: `${this._derivationBasePath}/${0}`, + derivationPath: 'm/0', derivationIndex: 0, hdKey, isChildKey: false, @@ -119,7 +122,7 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { return privateKeyWallet; } private _findDerivedKeyByPublicAddress(address: string): DerivedHDKey { - if (_.isUndefined(address)) { + if (_.isUndefined(address) || !addressUtils.isAddress(address)) { throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); } const matchedDerivedKey = walletUtils.findDerivedKeyByAddress( diff --git a/packages/subproviders/src/utils/wallet_utils.ts b/packages/subproviders/src/utils/wallet_utils.ts index 097d2b82f..d5ebf5ce6 100644 --- a/packages/subproviders/src/utils/wallet_utils.ts +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -22,19 +22,20 @@ class DerivedHDKeyIterator implements IterableIterator { public next(): IteratorResult { const derivationBasePath = this._initialDerivedKey.derivationBasePath; const derivationIndex = this._index; + const isChildKey = this._initialDerivedKey.isChildKey; // If the DerivedHDKey is a child then we walk relative, if not we walk the full derivation path const fullDerivationPath = `m/${derivationBasePath}/${derivationIndex}`; const relativeDerivationPath = `m/${derivationIndex}`; - const path = this._initialDerivedKey.isChildKey ? relativeDerivationPath : fullDerivationPath; + const path = isChildKey ? relativeDerivationPath : fullDerivationPath; const hdKey = this._initialDerivedKey.hdKey.derive(path); const address = walletUtils.addressOfHDKey(hdKey); const derivedKey: DerivedHDKey = { address, hdKey, - derivationPath: fullDerivationPath, - derivationBasePath: this._initialDerivedKey.derivationBasePath, + derivationBasePath, derivationIndex, - isChildKey: this._initialDerivedKey.isChildKey, + derivationPath: fullDerivationPath, + isChildKey, }; const done = this._index === this._searchLimit; this._index++; diff --git a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts index 2bc84abc1..9131a8b6a 100644 --- a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts @@ -55,10 +55,16 @@ describe('MnemonicWalletSubprovider', () => { }); }); describe('failure cases', () => { - it('throws an error if account cannot be found', async () => { + it('throws an error if address is invalid ', async () => { const txData = { ...fixtureData.TX_DATA, from: '0x0' }; return expect(subprovider.signTransactionAsync(txData)).to.be.rejectedWith( - WalletSubproviderErrors.AddressNotFound, + WalletSubproviderErrors.FromAddressMissingOrInvalid, + ); + }); + it('throws an error if address is valid format but not found', async () => { + const txData = { ...fixtureData.TX_DATA, from: fixtureData.NULL_ADDRESS }; + return expect(subprovider.signTransactionAsync(txData)).to.be.rejectedWith( + `${WalletSubproviderErrors.AddressNotFound}: ${fixtureData.NULL_ADDRESS}`, ); }); }); @@ -155,12 +161,14 @@ describe('MnemonicWalletSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'personal_sign', - params: [nonHexMessage, '0x0'], + params: [nonHexMessage, fixtureData.NULL_ADDRESS], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { expect(err).to.not.be.a('null'); - expect(err.message).to.be.equal(`${WalletSubproviderErrors.AddressNotFound}: 0x0`); + expect(err.message).to.be.equal( + `${WalletSubproviderErrors.AddressNotFound}: ${fixtureData.NULL_ADDRESS}`, + ); done(); }); provider.sendAsync(payload, callback); diff --git a/packages/subproviders/test/utils/fixture_data.ts b/packages/subproviders/test/utils/fixture_data.ts index 57b69b2f8..a973961ce 100644 --- a/packages/subproviders/test/utils/fixture_data.ts +++ b/packages/subproviders/test/utils/fixture_data.ts @@ -1,7 +1,9 @@ const TEST_RPC_ACCOUNT_0 = '0x5409ed021d9299bf6814279a6a1411a7e866a631'; const TEST_RPC_ACCOUNT_1 = '0x6ecbe1db9ef729cbe972c83fb886247691fb6beb'; +const NULL_ADDRESS = '0x0000000000000000000000000000000000000000'; const networkId = 42; export const fixtureData = { + NULL_ADDRESS, TEST_RPC_ACCOUNT_0, TEST_RPC_ACCOUNT_0_ACCOUNT_PRIVATE_KEY: 'F2F48EE19680706196E2E339E5DA3491186E0C4C5030670656B0E0164837257D', TEST_RPC_ACCOUNT_1, @@ -18,7 +20,7 @@ export const fixtureData = { nonce: '0x00', gasPrice: '0x0', gas: '0x2710', - to: '0x0000000000000000000000000000000000000000', + to: NULL_ADDRESS, value: '0x00', chainId: networkId, from: TEST_RPC_ACCOUNT_0, -- cgit From b08c616713e153ca767d2f01976da5f1ef903251 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 14:53:58 +1000 Subject: Move type declaration for hdkey to typescript-typings --- packages/subproviders/src/globals.d.ts | 13 ------------- packages/typescript-typings/CHANGELOG.json | 9 +++++++++ packages/typescript-typings/types/hdkey/index.d.ts | 11 +++++++++++ 3 files changed, 20 insertions(+), 13 deletions(-) create mode 100644 packages/typescript-typings/types/hdkey/index.d.ts diff --git a/packages/subproviders/src/globals.d.ts b/packages/subproviders/src/globals.d.ts index 754d164e8..4b3ecdf3c 100644 --- a/packages/subproviders/src/globals.d.ts +++ b/packages/subproviders/src/globals.d.ts @@ -51,19 +51,6 @@ declare module '@ledgerhq/hw-transport-node-hid' { } } -// hdkey declarations -declare module 'hdkey' { - class HDNode { - public static fromMasterSeed(seed: Buffer): HDNode; - public publicKey: Buffer; - public privateKey: Buffer; - public chainCode: Buffer; - public constructor(); - public derive(path: string): HDNode; - } - export = HDNode; -} - declare module '*.json' { const json: any; /* tslint:disable */ diff --git a/packages/typescript-typings/CHANGELOG.json b/packages/typescript-typings/CHANGELOG.json index 294d9ee9f..467cae29b 100644 --- a/packages/typescript-typings/CHANGELOG.json +++ b/packages/typescript-typings/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "0.1.1", + "changes": [ + { + "note": "Add types for HDKey", + "pr": 507 + } + ] + }, { "version": "0.1.0", "changes": [ diff --git a/packages/typescript-typings/types/hdkey/index.d.ts b/packages/typescript-typings/types/hdkey/index.d.ts new file mode 100644 index 000000000..84b751bd7 --- /dev/null +++ b/packages/typescript-typings/types/hdkey/index.d.ts @@ -0,0 +1,11 @@ +declare module 'hdkey' { + class HDNode { + public static fromMasterSeed(seed: Buffer): HDNode; + public publicKey: Buffer; + public privateKey: Buffer; + public chainCode: Buffer; + public constructor(); + public derive(path: string): HDNode; + } + export = HDNode; +} -- cgit From f44ef7ce59cd5c811a92662d3fb095f21d80f665 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 15:12:02 +1000 Subject: Update website to support latest ledger --- packages/subproviders/package-lock.json | 25 ++++++++++++++++++++++ packages/subproviders/src/index.ts | 1 - packages/subproviders/src/types.ts | 6 ------ packages/website/ts/blockchain.ts | 9 +------- .../ts/components/dialogs/ledger_config_dialog.tsx | 1 - 5 files changed, 26 insertions(+), 16 deletions(-) create mode 100644 packages/subproviders/package-lock.json diff --git a/packages/subproviders/package-lock.json b/packages/subproviders/package-lock.json new file mode 100644 index 000000000..61675f9ea --- /dev/null +++ b/packages/subproviders/package-lock.json @@ -0,0 +1,25 @@ +{ + "name": "@0xproject/subproviders", + "version": "0.8.4", + "lockfileVersion": 1, + "requires": true, + "dependencies": { + "@types/bip39": { + "version": "2.4.0", + "resolved": "https://registry.npmjs.org/@types/bip39/-/bip39-2.4.0.tgz", + "integrity": "sha512-qxJBGh55SPbSGv+91D6H3WOR8vKdA/p8Oc58oK/DTbORgjO6Ebuo8MRzdy2OWi+dw/lxtX4VWJkkCUTSQvhAnw==", + "dev": true, + "requires": { + "@types/node": "9.6.2" + }, + "dependencies": { + "@types/node": { + "version": "9.6.2", + "resolved": "https://registry.npmjs.org/@types/node/-/node-9.6.2.tgz", + "integrity": "sha512-UWkRY9X7RQHp5OhhRIIka58/gVVycL1zHZu0OTsT5LI86ABaMOSbUjAl+b0FeDhQcxclrkyft3kW5QWdMRs8wQ==", + "dev": true + } + } + } + } +} diff --git a/packages/subproviders/src/index.ts b/packages/subproviders/src/index.ts index 01aec956a..dc45ea9f1 100644 --- a/packages/subproviders/src/index.ts +++ b/packages/subproviders/src/index.ts @@ -18,7 +18,6 @@ export { Callback, ErrorCallback, NextCallback, - LedgerWalletSubprovider, LedgerCommunicationClient, NonceSubproviderErrors, LedgerSubproviderConfigs, diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts index c76be1bf8..121992278 100644 --- a/packages/subproviders/src/types.ts +++ b/packages/subproviders/src/types.ts @@ -69,12 +69,6 @@ export interface LedgerGetAddressResult { chainCode: string; } -export interface LedgerWalletSubprovider { - getPath: () => string; - setPath: (path: string) => void; - setPathIndex: (pathIndex: number) => void; -} - export interface PartialTxParams { nonce: string; gasPrice?: string; diff --git a/packages/website/ts/blockchain.ts b/packages/website/ts/blockchain.ts index fd34ab82d..bb1f47dd0 100644 --- a/packages/website/ts/blockchain.ts +++ b/packages/website/ts/blockchain.ts @@ -20,7 +20,6 @@ import { InjectedWeb3Subprovider, ledgerEthereumBrowserClientFactoryAsync, LedgerSubprovider, - LedgerWalletSubprovider, RedundantRPCSubprovider, } from '@0xproject/subproviders'; import { Provider } from '@0xproject/types'; @@ -76,7 +75,7 @@ export class Blockchain { private _userAddressIfExists: string; private _cachedProvider: Provider; private _cachedProviderNetworkId: number; - private _ledgerSubprovider: LedgerWalletSubprovider; + private _ledgerSubprovider: LedgerSubprovider; private _defaultGasPrice: BigNumber; private static _getNameGivenProvider(provider: Provider): string { const providerType = utils.getProviderType(provider); @@ -168,12 +167,6 @@ export class Blockchain { } this._ledgerSubprovider.setPath(path); } - public updateLedgerDerivationIndex(pathIndex: number) { - if (_.isUndefined(this._ledgerSubprovider)) { - return; // noop - } - this._ledgerSubprovider.setPathIndex(pathIndex); - } public async updateProviderToLedgerAsync(networkId: number) { utils.assert(!_.isUndefined(this._zeroEx), 'ZeroEx must be instantiated.'); diff --git a/packages/website/ts/components/dialogs/ledger_config_dialog.tsx b/packages/website/ts/components/dialogs/ledger_config_dialog.tsx index d7190c0bb..a72d33183 100644 --- a/packages/website/ts/components/dialogs/ledger_config_dialog.tsx +++ b/packages/website/ts/components/dialogs/ledger_config_dialog.tsx @@ -199,7 +199,6 @@ export class LedgerConfigDialog extends React.Component Date: Wed, 11 Apr 2018 18:48:46 +1000 Subject: Renamed DerivedHDKey to DerivedHDKeyInfo Added assertions on addresses for public methods Throw a helpful error message when signer address is not instantiated address in PrivateKeyWalletSubprovider Update changelog and rename derivationBasePath to baseDerivationPath When returning undefined use pattern of IfExists Added configuration object for MnemonicWallet Put constants back into each individual wallet rather than in walletUtils Delete accidental package-lock.json --- packages/subproviders/CHANGELOG.json | 10 ++- packages/subproviders/package-lock.json | 25 ------ packages/subproviders/package.json | 9 +-- packages/subproviders/src/subproviders/ledger.ts | 73 +++++++++-------- .../src/subproviders/mnemonic_wallet.ts | 91 ++++++++++++---------- .../src/subproviders/private_key_wallet.ts | 21 +++-- packages/subproviders/src/types.ts | 18 ++++- packages/subproviders/src/utils/wallet_utils.ts | 40 +++++----- .../test/integration/ledger_subprovider_test.ts | 2 +- .../test/unit/mnemonic_wallet_subprovider_test.ts | 12 +-- .../unit/private_key_wallet_subprovider_test.ts | 16 ++-- packages/subproviders/test/utils/fixture_data.ts | 4 +- 12 files changed, 164 insertions(+), 157 deletions(-) delete mode 100644 packages/subproviders/package-lock.json diff --git a/packages/subproviders/CHANGELOG.json b/packages/subproviders/CHANGELOG.json index 554fbd0cf..5287ee987 100644 --- a/packages/subproviders/CHANGELOG.json +++ b/packages/subproviders/CHANGELOG.json @@ -1,14 +1,18 @@ [ { - "version": "0.8.5", + "version": "0.9.0", "changes": [ { - "note": "Add private key subprovider and refactor shared functionality into a base wallet subprovider", + "note": "Add private key subprovider and refactor shared functionality into a base wallet subprovider.", "pr": 506 }, + { + "note": "Add mnemonic wallet subprovider, deprecating our truffle-hdwallet-provider fork.", + "pr": 507 + }, { "note": - "Add mnemonic wallet subprovider, deprecating our truffle-hdwallet-provider fork. Support multiple addresses in ledger and mnemonic wallets", + "Support multiple addresses in ledger and mnemonic wallets. Renamed derivationPath to baseDerivationPath.", "pr": 507 } ] diff --git a/packages/subproviders/package-lock.json b/packages/subproviders/package-lock.json deleted file mode 100644 index 61675f9ea..000000000 --- a/packages/subproviders/package-lock.json +++ /dev/null @@ -1,25 +0,0 @@ -{ - "name": "@0xproject/subproviders", - "version": "0.8.4", - "lockfileVersion": 1, - "requires": true, - "dependencies": { - "@types/bip39": { - "version": "2.4.0", - "resolved": "https://registry.npmjs.org/@types/bip39/-/bip39-2.4.0.tgz", - "integrity": "sha512-qxJBGh55SPbSGv+91D6H3WOR8vKdA/p8Oc58oK/DTbORgjO6Ebuo8MRzdy2OWi+dw/lxtX4VWJkkCUTSQvhAnw==", - "dev": true, - "requires": { - "@types/node": "9.6.2" - }, - "dependencies": { - "@types/node": { - "version": "9.6.2", - "resolved": "https://registry.npmjs.org/@types/node/-/node-9.6.2.tgz", - "integrity": "sha512-UWkRY9X7RQHp5OhhRIIka58/gVVycL1zHZu0OTsT5LI86ABaMOSbUjAl+b0FeDhQcxclrkyft3kW5QWdMRs8wQ==", - "dev": true - } - } - } - } -} diff --git a/packages/subproviders/package.json b/packages/subproviders/package.json index d557eef29..afb6ef5d6 100644 --- a/packages/subproviders/package.json +++ b/packages/subproviders/package.json @@ -21,15 +21,14 @@ "manual:postpublish": "yarn build; node ./scripts/postpublish.js", "docs:stage": "yarn build && node ./scripts/stage_docs.js", "docs:json": "typedoc --excludePrivate --excludeExternals --target ES5 --json $JSON_FILE_PATH $PROJECT_FILES", - "upload_docs_json": "aws s3 cp generated_docs/index.json $S3_URL --profile 0xproject --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers --content-type application/json" + "upload_docs_json": + "aws s3 cp generated_docs/index.json $S3_URL --profile 0xproject --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers --content-type application/json" }, "config": { "postpublish": { "assets": [], "docPublishConfigs": { - "extraFileIncludes": [ - "../types/src/index.ts" - ], + "extraFileIncludes": ["../types/src/index.ts"], "s3BucketPath": "s3://doc-jsons/subproviders/", "s3StagingBucketPath": "s3://staging-doc-jsons/subproviders/" } @@ -46,6 +45,7 @@ "ethereumjs-tx": "^1.3.3", "ethereumjs-util": "^5.1.1", "ganache-core": "0xProject/ganache-core", + "bip39": "^2.5.0", "hdkey": "^0.7.1", "lodash": "^4.17.4", "semaphore-async-await": "^1.5.1", @@ -60,7 +60,6 @@ "@types/lodash": "4.14.104", "@types/mocha": "^2.2.42", "@types/node": "^8.0.53", - "bip39": "^2.5.0", "chai": "^4.0.1", "chai-as-promised": "^7.1.0", "copyfiles": "^1.2.0", diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 9b2d9d7d0..33aa5c1bf 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -8,7 +8,7 @@ import { Lock } from 'semaphore-async-await'; import { Callback, - DerivedHDKey, + DerivedHDKeyInfo, LedgerEthereumClient, LedgerEthereumClientFactoryAsync, LedgerSubproviderConfigs, @@ -21,9 +21,11 @@ import { walletUtils } from '../utils/wallet_utils'; import { BaseWalletSubprovider } from './base_wallet_subprovider'; -const DEFAULT_DERIVATION_PATH = `44'/60'/0'`; +const DEFAULT_BASE_DERIVATION_PATH = `44'/60'/0'`; const ASK_FOR_ON_DEVICE_CONFIRMATION = false; const SHOULD_GET_CHAIN_CODE = true; +const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; +const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; /** * Subprovider for interfacing with a user's [Ledger Nano S](https://www.ledgerwallet.com/products/ledger-nano-s). @@ -34,7 +36,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { private _nonceLock = new Lock(); private _connectionLock = new Lock(); private _networkId: number; - private _derivationBasePath: string; + private _baseDerivationPath: string; private _ledgerEthereumClientFactoryAsync: LedgerEthereumClientFactoryAsync; private _ledgerClientIfExists?: LedgerEthereumClient; private _shouldAlwaysAskForConfirmation: boolean; @@ -49,7 +51,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { super(); this._networkId = config.networkId; this._ledgerEthereumClientFactoryAsync = config.ledgerEthereumClientFactoryAsync; - this._derivationBasePath = config.derivationPath || DEFAULT_DERIVATION_PATH; + this._baseDerivationPath = config.baseDerivationPath || DEFAULT_BASE_DERIVATION_PATH; this._shouldAlwaysAskForConfirmation = !_.isUndefined(config.accountFetchingConfigs) && !_.isUndefined(config.accountFetchingConfigs.shouldAskForOnDeviceConfirmation) @@ -59,21 +61,21 @@ export class LedgerSubprovider extends BaseWalletSubprovider { !_.isUndefined(config.accountFetchingConfigs) && !_.isUndefined(config.accountFetchingConfigs.addressSearchLimit) ? config.accountFetchingConfigs.addressSearchLimit - : walletUtils.DEFAULT_ADDRESS_SEARCH_LIMIT; + : DEFAULT_ADDRESS_SEARCH_LIMIT; } /** * Retrieve the set derivation path * @returns derivation path */ public getPath(): string { - return this._derivationBasePath; + return this._baseDerivationPath; } /** * Set a desired derivation path when computing the available user addresses - * @param derivationPath The desired derivation path (e.g `44'/60'/0'`) + * @param basDerivationPath The desired derivation path (e.g `44'/60'/0'`) */ - public setPath(derivationPath: string) { - this._derivationBasePath = derivationPath; + public setPath(basDerivationPath: string) { + this._baseDerivationPath = basDerivationPath; } /** * Retrieve a users Ledger accounts. The accounts are derived from the derivationPath, @@ -84,12 +86,10 @@ export class LedgerSubprovider extends BaseWalletSubprovider { * @param numberOfAccounts Number of accounts to retrieve (default: 10) * @return An array of accounts */ - public async getAccountsAsync( - numberOfAccounts: number = walletUtils.DEFAULT_NUM_ADDRESSES_TO_FETCH, - ): Promise { - const initialDerivedKey = await this._initialDerivedKeyAsync(); - const derivedKeys = walletUtils.calculateDerivedHDKeys(initialDerivedKey, numberOfAccounts); - const accounts = _.map(derivedKeys, 'address'); + public async getAccountsAsync(numberOfAccounts: number = DEFAULT_NUM_ADDRESSES_TO_FETCH): Promise { + const initialDerivedKeyInfo = await this._initialDerivedKeyInfoAsync(); + const derivedKeyInfos = walletUtils.calculateDerivedHDKeyInfos(initialDerivedKeyInfo, numberOfAccounts); + const accounts = _.map(derivedKeyInfos, k => k.address); return accounts; } /** @@ -102,8 +102,11 @@ export class LedgerSubprovider extends BaseWalletSubprovider { */ public async signTransactionAsync(txParams: PartialTxParams): Promise { LedgerSubprovider._validateTxParams(txParams); - const initialDerivedKey = await this._initialDerivedKeyAsync(); - const derivedKey = this._findDerivedKeyByPublicAddress(initialDerivedKey, txParams.from); + if (_.isUndefined(txParams.from) || !addressUtils.isAddress(txParams.from)) { + throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); + } + const initialDerivedKeyInfo = await this._initialDerivedKeyInfoAsync(); + const derivedKeyInfo = this._findDerivedKeyInfoForAddress(initialDerivedKeyInfo, txParams.from); const ledgerClient = await this._createLedgerClientAsync(); @@ -116,7 +119,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { const txHex = tx.serialize().toString('hex'); try { - const fullDerivationPath = derivedKey.derivationPath; + const fullDerivationPath = derivedKeyInfo.derivationPath; const result = await ledgerClient.signTransaction(fullDerivationPath, txHex); // Store signature in transaction tx.r = Buffer.from(result.r, 'hex'); @@ -155,12 +158,13 @@ export class LedgerSubprovider extends BaseWalletSubprovider { throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage); } assert.isHexString('data', data); - const initialDerivedKey = await this._initialDerivedKeyAsync(); - const derivedKey = this._findDerivedKeyByPublicAddress(initialDerivedKey, address); + assert.isETHAddressHex('address', address); + const initialDerivedKeyInfo = await this._initialDerivedKeyInfoAsync(); + const derivedKeyInfo = this._findDerivedKeyInfoForAddress(initialDerivedKeyInfo, address); const ledgerClient = await this._createLedgerClientAsync(); try { - const fullDerivationPath = derivedKey.derivationPath; + const fullDerivationPath = derivedKeyInfo.derivationPath; const result = await ledgerClient.signPersonalMessage(fullDerivationPath, ethUtil.stripHexPrefix(data)); const v = result.v - 27; let vHex = v.toString(16); @@ -196,13 +200,13 @@ export class LedgerSubprovider extends BaseWalletSubprovider { this._ledgerClientIfExists = undefined; this._connectionLock.release(); } - private async _initialDerivedKeyAsync(): Promise { + private async _initialDerivedKeyInfoAsync(): Promise { const ledgerClient = await this._createLedgerClientAsync(); let ledgerResponse; try { ledgerResponse = await ledgerClient.getAddress( - this._derivationBasePath, + this._baseDerivationPath, this._shouldAlwaysAskForConfirmation, SHOULD_GET_CHAIN_CODE, ); @@ -212,23 +216,26 @@ export class LedgerSubprovider extends BaseWalletSubprovider { const hdKey = new HDNode(); hdKey.publicKey = new Buffer(ledgerResponse.publicKey, 'hex'); hdKey.chainCode = new Buffer(ledgerResponse.chainCode, 'hex'); + const derivationPath = `${this._baseDerivationPath}/0`; + const derivationIndex = 0; return { hdKey, address: ledgerResponse.address, isChildKey: true, - derivationBasePath: this._derivationBasePath, - derivationPath: `${this._derivationBasePath}/0`, - derivationIndex: 0, + baseDerivationPath: this._baseDerivationPath, + derivationPath, + derivationIndex, }; } - private _findDerivedKeyByPublicAddress(initalHDKey: DerivedHDKey, address: string): DerivedHDKey { - if (_.isUndefined(address) || !addressUtils.isAddress(address)) { - throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); - } - const matchedDerivedKey = walletUtils.findDerivedKeyByAddress(address, initalHDKey, this._addressSearchLimit); - if (_.isUndefined(matchedDerivedKey)) { + private _findDerivedKeyInfoForAddress(initalHDKey: DerivedHDKeyInfo, address: string): DerivedHDKeyInfo { + const matchedDerivedKeyInfo = walletUtils.findDerivedKeyInfoForAddressIfExists( + address, + initalHDKey, + this._addressSearchLimit, + ); + if (_.isUndefined(matchedDerivedKeyInfo)) { throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`); } - return matchedDerivedKey; + return matchedDerivedKeyInfo; } } diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet.ts b/packages/subproviders/src/subproviders/mnemonic_wallet.ts index de34a9d11..9a627b1ec 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet.ts @@ -5,13 +5,17 @@ import ethUtil = require('ethereumjs-util'); import HDNode = require('hdkey'); import * as _ from 'lodash'; -import { DerivedHDKey, PartialTxParams, WalletSubproviderErrors } from '../types'; +import { DerivedHDKeyInfo, MnemonicWalletSubproviderConfigs, PartialTxParams, WalletSubproviderErrors } from '../types'; import { walletUtils } from '../utils/wallet_utils'; import { BaseWalletSubprovider } from './base_wallet_subprovider'; import { PrivateKeyWalletSubprovider } from './private_key_wallet'; -const DEFAULT_DERIVATION_PATH = `44'/60'/0'/0`; +const DEFAULT_BASE_DERIVATION_PATH = `44'/60'/0'/0`; +const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; +const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; +const ROOT_KEY_DERIVATION_PATH = 'm/'; +const ROOT_KEY_DERIVATION_INDEX = 0; /** * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. @@ -20,35 +24,33 @@ const DEFAULT_DERIVATION_PATH = `44'/60'/0'/0`; */ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { private _addressSearchLimit: number; - private _derivationBasePath: string; - private _derivedKey: DerivedHDKey; + private _baseDerivationPath: string; + private _derivedKeyInfo: DerivedHDKeyInfo; /** - * Instantiates a MnemonicWalletSubprovider. Defaults to derivationPath set to `44'/60'/0'/0`. - * This is the default in TestRPC/Ganache, this can be overridden if desired. - * @param mnemonic The mnemonic seed - * @param derivationBasePath The derivation path, defaults to `44'/60'/0'/0` - * @param addressSearchLimit The limit on address search attempts before raising `WalletSubproviderErrors.AddressNotFound` + * Instantiates a MnemonicWalletSubprovider. Defaults to baseDerivationPath set to `44'/60'/0'/0`. + * This is the default in TestRPC/Ganache, it can be overridden if desired. + * @param config Several available configurations * @return MnemonicWalletSubprovider instance */ - constructor( - mnemonic: string, - derivationBasePath: string = DEFAULT_DERIVATION_PATH, - addressSearchLimit: number = walletUtils.DEFAULT_ADDRESS_SEARCH_LIMIT, - ) { + constructor(config: MnemonicWalletSubproviderConfigs) { + const mnemonic = config.mnemonic; assert.isString('mnemonic', mnemonic); - assert.isString('derivationPath', derivationBasePath); + const baseDerivationPath = config.baseDerivationPath || DEFAULT_BASE_DERIVATION_PATH; + assert.isString('baseDerivationPath', baseDerivationPath); + const addressSearchLimit = config.addressSearchLimit || DEFAULT_ADDRESS_SEARCH_LIMIT; assert.isNumber('addressSearchLimit', addressSearchLimit); super(); + const seed = bip39.mnemonicToSeed(mnemonic); const hdKey = HDNode.fromMasterSeed(seed); - this._derivationBasePath = derivationBasePath; - this._derivedKey = { + this._baseDerivationPath = baseDerivationPath; + this._derivedKeyInfo = { address: walletUtils.addressOfHDKey(hdKey), // HACK this isn't the base path for this root key, but is is the base path - // we want all of the derived children to spawn from - derivationBasePath: this._derivationBasePath, - derivationPath: 'm/0', - derivationIndex: 0, + // we want all of the derived children to branched from + baseDerivationPath: this._baseDerivationPath, + derivationPath: ROOT_KEY_DERIVATION_PATH, + derivationIndex: ROOT_KEY_DERIVATION_INDEX, hdKey, isChildKey: false, }; @@ -59,17 +61,17 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @returns derivation path */ public getPath(): string { - return this._derivationBasePath; + return this._baseDerivationPath; } /** * Set a desired derivation path when computing the available user addresses - * @param derivationPath The desired derivation path (e.g `44'/60'/0'`) + * @param baseDerivationPath The desired derivation path (e.g `44'/60'/0'`) */ - public setPath(derivationPath: string) { - this._derivationBasePath = derivationPath; - this._derivedKey = { - ...this._derivedKey, - derivationBasePath: this._derivationBasePath, + public setPath(baseDerivationPath: string) { + this._baseDerivationPath = baseDerivationPath; + this._derivedKeyInfo = { + ...this._derivedKeyInfo, + baseDerivationPath, }; } /** @@ -79,11 +81,9 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @param numberOfAccounts Number of accounts to retrieve (default: 10) * @return An array of accounts */ - public async getAccountsAsync( - numberOfAccounts: number = walletUtils.DEFAULT_NUM_ADDRESSES_TO_FETCH, - ): Promise { - const derivedKeys = walletUtils.calculateDerivedHDKeys(this._derivedKey, numberOfAccounts); - const accounts = _.map(derivedKeys, 'address'); + public async getAccountsAsync(numberOfAccounts: number = DEFAULT_NUM_ADDRESSES_TO_FETCH): Promise { + const derivedKeys = walletUtils.calculateDerivedHDKeyInfos(this._derivedKeyInfo, numberOfAccounts); + const accounts = _.map(derivedKeys, k => k.address); return accounts; } @@ -96,6 +96,9 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @return Signed transaction hex string */ public async signTransactionAsync(txParams: PartialTxParams): Promise { + if (_.isUndefined(txParams.from) || !addressUtils.isAddress(txParams.from)) { + throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); + } const privateKeyWallet = this._privateKeyWalletForAddress(txParams.from); const signedTx = privateKeyWallet.signTransactionAsync(txParams); return signedTx; @@ -111,28 +114,30 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @return Signature hex string (order: rsv) */ public async signPersonalMessageAsync(data: string, address: string): Promise { + if (_.isUndefined(data)) { + throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage); + } + assert.isHexString('data', data); + assert.isETHAddressHex('address', address); const privateKeyWallet = this._privateKeyWalletForAddress(address); const sig = await privateKeyWallet.signPersonalMessageAsync(data, address); return sig; } private _privateKeyWalletForAddress(address: string): PrivateKeyWalletSubprovider { - const derivedKey = this._findDerivedKeyByPublicAddress(address); - const privateKeyHex = derivedKey.hdKey.privateKey.toString('hex'); + const derivedKeyInfo = this._findDerivedKeyInfoForAddress(address); + const privateKeyHex = derivedKeyInfo.hdKey.privateKey.toString('hex'); const privateKeyWallet = new PrivateKeyWalletSubprovider(privateKeyHex); return privateKeyWallet; } - private _findDerivedKeyByPublicAddress(address: string): DerivedHDKey { - if (_.isUndefined(address) || !addressUtils.isAddress(address)) { - throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); - } - const matchedDerivedKey = walletUtils.findDerivedKeyByAddress( + private _findDerivedKeyInfoForAddress(address: string): DerivedHDKeyInfo { + const matchedDerivedKeyInfo = walletUtils.findDerivedKeyInfoForAddressIfExists( address, - this._derivedKey, + this._derivedKeyInfo, this._addressSearchLimit, ); - if (_.isUndefined(matchedDerivedKey)) { + if (_.isUndefined(matchedDerivedKeyInfo)) { throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`); } - return matchedDerivedKey; + return matchedDerivedKeyInfo; } } diff --git a/packages/subproviders/src/subproviders/private_key_wallet.ts b/packages/subproviders/src/subproviders/private_key_wallet.ts index f8afab722..f3727039c 100644 --- a/packages/subproviders/src/subproviders/private_key_wallet.ts +++ b/packages/subproviders/src/subproviders/private_key_wallet.ts @@ -17,7 +17,7 @@ export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { private _privateKeyBuffer: Buffer; /** * Instantiates a PrivateKeyWalletSubprovider. - * @param privateKey The private key of the ethereum address + * @param privateKey The corresponding private key to an Ethereum address * @return PrivateKeyWalletSubprovider instance */ constructor(privateKey: string) { @@ -46,7 +46,11 @@ export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { public async signTransactionAsync(txParams: PartialTxParams): Promise { PrivateKeyWalletSubprovider._validateTxParams(txParams); if (!_.isUndefined(txParams.from) && txParams.from !== this._address) { - throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${txParams.from}`); + throw new Error( + `Requested to sign transaction with address: ${txParams.from}, instantiated with address: ${ + this._address + }`, + ); } const tx = new EthereumTx(txParams); tx.sign(this._privateKeyBuffer); @@ -54,8 +58,8 @@ export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { return rawTx; } /** - * Sign a personal Ethereum signed message. The signing address will be - * calculated from the private key, if an address is provided it must match the address calculated from the private key. + * Sign a personal Ethereum signed message. The signing address will be calculated from the private key. + * if an address is provided it must match the address calculated from the private key. * If you've added this Subprovider to your app's provider, you can simply send an `eth_sign` * or `personal_sign` JSON RPC request, and this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. @@ -67,10 +71,13 @@ export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { if (_.isUndefined(data)) { throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage); } - if (_.isUndefined(address) || address !== this._address) { - throw new Error(`${WalletSubproviderErrors.FromAddressMissingOrInvalid}: ${address}`); - } assert.isHexString('data', data); + assert.isETHAddressHex('address', address); + if (address !== this._address) { + throw new Error( + `Requested to sign message with address: ${address}, instantiated with address: ${this._address}`, + ); + } const dataBuff = ethUtil.toBuffer(data); const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff); const sig = ethUtil.ecsign(msgHashBuff, this._privateKeyBuffer); diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts index 121992278..b9d9d08ee 100644 --- a/packages/subproviders/src/types.ts +++ b/packages/subproviders/src/types.ts @@ -41,11 +41,12 @@ export type LedgerEthereumClientFactoryAsync = () => Promise { - private _initialDerivedKey: DerivedHDKey; +class DerivedHDKeyInfoIterator implements IterableIterator { + private _initialDerivedKey: DerivedHDKeyInfo; private _searchLimit: number; private _index: number; - constructor(initialDerivedKey: DerivedHDKey, searchLimit: number = DEFAULT_ADDRESS_SEARCH_OFFSET) { + constructor(initialDerivedKey: DerivedHDKeyInfo, searchLimit: number = DEFAULT_ADDRESS_SEARCH_LIMIT) { this._searchLimit = searchLimit; this._initialDerivedKey = initialDerivedKey; this._index = 0; } - public next(): IteratorResult { - const derivationBasePath = this._initialDerivedKey.derivationBasePath; + public next(): IteratorResult { + const baseDerivationPath = this._initialDerivedKey.baseDerivationPath; const derivationIndex = this._index; const isChildKey = this._initialDerivedKey.isChildKey; // If the DerivedHDKey is a child then we walk relative, if not we walk the full derivation path - const fullDerivationPath = `m/${derivationBasePath}/${derivationIndex}`; + const fullDerivationPath = `m/${baseDerivationPath}/${derivationIndex}`; const relativeDerivationPath = `m/${derivationIndex}`; const path = isChildKey ? relativeDerivationPath : fullDerivationPath; const hdKey = this._initialDerivedKey.hdKey.derive(path); const address = walletUtils.addressOfHDKey(hdKey); - const derivedKey: DerivedHDKey = { + const derivedKey: DerivedHDKeyInfo = { address, hdKey, - derivationBasePath, + baseDerivationPath, derivationIndex, derivationPath: fullDerivationPath, isChildKey, @@ -45,29 +43,27 @@ class DerivedHDKeyIterator implements IterableIterator { }; } - public [Symbol.iterator](): IterableIterator { + public [Symbol.iterator](): IterableIterator { return this; } } export const walletUtils = { - DEFAULT_ADDRESS_SEARCH_LIMIT, - DEFAULT_NUM_ADDRESSES_TO_FETCH: 10, - calculateDerivedHDKeys(initialDerivedKey: DerivedHDKey, numberOfKeys: number): DerivedHDKey[] { - const derivedKeys: DerivedHDKey[] = []; - const derivedKeyIterator = new DerivedHDKeyIterator(initialDerivedKey, numberOfKeys); + calculateDerivedHDKeyInfos(initialDerivedKey: DerivedHDKeyInfo, numberOfKeys: number): DerivedHDKeyInfo[] { + const derivedKeys: DerivedHDKeyInfo[] = []; + const derivedKeyIterator = new DerivedHDKeyInfoIterator(initialDerivedKey, numberOfKeys); for (const key of derivedKeyIterator) { derivedKeys.push(key); } return derivedKeys; }, - findDerivedKeyByAddress( + findDerivedKeyInfoForAddressIfExists( address: string, - initialDerivedKey: DerivedHDKey, + initialDerivedKey: DerivedHDKeyInfo, searchLimit: number, - ): DerivedHDKey | undefined { - let matchedKey: DerivedHDKey | undefined; - const derivedKeyIterator = new DerivedHDKeyIterator(initialDerivedKey, searchLimit); + ): DerivedHDKeyInfo | undefined { + let matchedKey: DerivedHDKeyInfo | undefined; + const derivedKeyIterator = new DerivedHDKeyInfoIterator(initialDerivedKey, searchLimit); for (const key of derivedKeyIterator) { if (key.address === address) { matchedKey = key; diff --git a/packages/subproviders/test/integration/ledger_subprovider_test.ts b/packages/subproviders/test/integration/ledger_subprovider_test.ts index 2db4befb3..11b5f6410 100644 --- a/packages/subproviders/test/integration/ledger_subprovider_test.ts +++ b/packages/subproviders/test/integration/ledger_subprovider_test.ts @@ -33,7 +33,7 @@ describe('LedgerSubprovider', () => { ledgerSubprovider = new LedgerSubprovider({ networkId, ledgerEthereumClientFactoryAsync: ledgerEthereumNodeJsClientFactoryAsync, - derivationPath: fixtureData.TESTRPC_DERIVATION_PATH, + baseDerivationPath: fixtureData.TESTRPC_BASE_DERIVATION_PATH, }); }); describe('direct method calls', () => { diff --git a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts index 9131a8b6a..93300f47d 100644 --- a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts @@ -21,10 +21,10 @@ const expect = chai.expect; describe('MnemonicWalletSubprovider', () => { let subprovider: MnemonicWalletSubprovider; before(async () => { - subprovider = new MnemonicWalletSubprovider( - fixtureData.TEST_RPC_MNEMONIC, - fixtureData.TEST_RPC_MNEMONIC_DERIVATION_PATH, - ); + subprovider = new MnemonicWalletSubprovider({ + mnemonic: fixtureData.TEST_RPC_MNEMONIC, + baseDerivationPath: fixtureData.TEST_RPC_MNEMONIC_BASE_DERIVATION_PATH, + }); }); describe('direct method calls', () => { describe('success cases', () => { @@ -157,11 +157,11 @@ describe('MnemonicWalletSubprovider', () => { provider.sendAsync(payload, callback); }); it('should throw if `address` param not found when calling personal_sign', (done: DoneCallback) => { - const nonHexMessage = 'hello world'; + const messageHex = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); const payload = { jsonrpc: '2.0', method: 'personal_sign', - params: [nonHexMessage, fixtureData.NULL_ADDRESS], + params: [messageHex, fixtureData.NULL_ADDRESS], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { diff --git a/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts b/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts index b84aebb18..5c1b5cd25 100644 --- a/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts @@ -128,18 +128,20 @@ describe('PrivateKeyWalletSubprovider', () => { }); provider.sendAsync(payload, callback); }); - it('should throw if `address` param is not an address from private key when calling personal_sign', (done: DoneCallback) => { - const nonHexMessage = 'hello world'; + it('should throw if `address` param is not the address from private key when calling personal_sign', (done: DoneCallback) => { + const messageHex = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); const payload = { jsonrpc: '2.0', method: 'personal_sign', - params: [nonHexMessage, fixtureData.TEST_RPC_ACCOUNT_1], + params: [messageHex, fixtureData.TEST_RPC_ACCOUNT_1], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { expect(err).to.not.be.a('null'); expect(err.message).to.be.equal( - `${WalletSubproviderErrors.FromAddressMissingOrInvalid}: ${fixtureData.TEST_RPC_ACCOUNT_1}`, + `Requested to sign message with address: ${ + fixtureData.TEST_RPC_ACCOUNT_1 + }, instantiated with address: ${fixtureData.TEST_RPC_ACCOUNT_0}`, ); done(); }); @@ -183,16 +185,16 @@ describe('PrivateKeyWalletSubprovider', () => { provider.sendAsync(payload, callback); }); it('should throw if `address` param not found when calling personal_sign', (done: DoneCallback) => { - const nonHexMessage = 'hello world'; + const messageHex = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING)); const payload = { jsonrpc: '2.0', method: 'personal_sign', - params: [nonHexMessage, '0x0'], + params: [messageHex, '0x0'], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { expect(err).to.not.be.a('null'); - expect(err.message).to.be.equal(`${WalletSubproviderErrors.FromAddressMissingOrInvalid}: 0x0`); + expect(err.message).to.be.equal(`Expected address to be of type ETHAddressHex, encountered: 0x0`); done(); }); provider.sendAsync(payload, callback); diff --git a/packages/subproviders/test/utils/fixture_data.ts b/packages/subproviders/test/utils/fixture_data.ts index a973961ce..3137e08b0 100644 --- a/packages/subproviders/test/utils/fixture_data.ts +++ b/packages/subproviders/test/utils/fixture_data.ts @@ -8,13 +8,13 @@ export const fixtureData = { TEST_RPC_ACCOUNT_0_ACCOUNT_PRIVATE_KEY: 'F2F48EE19680706196E2E339E5DA3491186E0C4C5030670656B0E0164837257D', TEST_RPC_ACCOUNT_1, TEST_RPC_MNEMONIC: 'concert load couple harbor equip island argue ramp clarify fence smart topic', - TEST_RPC_MNEMONIC_DERIVATION_PATH: `44'/60'/0'/0`, + TEST_RPC_MNEMONIC_BASE_DERIVATION_PATH: `44'/60'/0'/0`, PERSONAL_MESSAGE_STRING: 'hello world', PERSONAL_MESSAGE_SIGNED_RESULT: '0x1b0ec5e2908e993d0c8ab6b46da46be2688fdf03c7ea6686075de37392e50a7d7fcc531446699132fbda915bd989882e0064d417018773a315fb8d43ed063c9b00', PERSONAL_MESSAGE_ACCOUNT_1_SIGNED_RESULT: '0xe7ae0c21d02eb38f2c2a20d9d7876a98cc7ef035b7a4559d49375e2ec735e06f0d0ab0ff92ee56c5ffc28d516e6ed0692d0270feae8796408dbef060c6c7100f01', - TESTRPC_DERIVATION_PATH: `m/44'/60'/0'/0`, + TESTRPC_BASE_DERIVATION_PATH: `m/44'/60'/0'/0`, NETWORK_ID: networkId, TX_DATA: { nonce: '0x00', -- cgit From 63b941fbaf08167234cf7871e874c1a96e4347fa Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 20:51:31 +1000 Subject: Update documentation and add to website docs --- packages/subproviders/src/subproviders/ledger.ts | 12 ++++++------ packages/subproviders/src/subproviders/mnemonic_wallet.ts | 10 +++++----- .../subproviders/src/subproviders/private_key_wallet.ts | 6 +++--- packages/typescript-typings/CHANGELOG.json | 13 ++++--------- .../website/ts/containers/subproviders_documentation.ts | 9 +++++++++ 5 files changed, 27 insertions(+), 23 deletions(-) diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 33aa5c1bf..06c115105 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -79,7 +79,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { } /** * Retrieve a users Ledger accounts. The accounts are derived from the derivationPath, - * master public key and chainCode. Because of this, you can request as many accounts + * master public key and chain code. Because of this, you can request as many accounts * as you wish and it only requires a single request to the Ledger device. This method * is automatically called when issuing a `eth_accounts` JSON RPC request via your providerEngine * instance. @@ -93,9 +93,9 @@ export class LedgerSubprovider extends BaseWalletSubprovider { return accounts; } /** - * Sign a transaction with the Ledger. If you've added the LedgerSubprovider to your - * app's provider, you can simply send an `eth_sendTransaction` JSON RPC request, and - * this method will be called auto-magically. If you are not using this via a ProviderEngine + * Signs a transaction on the Ledger with the account specificed by the `from` field in txParams. + * If you've added the LedgerSubprovider to your app's provider, you can simply send an `eth_sendTransaction` + * JSON RPC request, and this method will be called auto-magically. If you are not using this via a ProviderEngine * instance, you can call it directly. * @param txParams Parameters of the transaction to sign * @return Signed transaction hex string @@ -149,8 +149,8 @@ export class LedgerSubprovider extends BaseWalletSubprovider { * the LedgerSubprovider to your app's provider, you can simply send an `eth_sign` * or `personal_sign` JSON RPC request, and this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. - * @param data Message to sign - * @param address Address to sign with + * @param data Hex string message to sign + * @param address Address of the account to sign with * @return Signature hex string (order: rsv) */ public async signPersonalMessageAsync(data: string, address: string): Promise { diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet.ts b/packages/subproviders/src/subproviders/mnemonic_wallet.ts index 9a627b1ec..5dc1b3be7 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet.ts @@ -29,7 +29,7 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { /** * Instantiates a MnemonicWalletSubprovider. Defaults to baseDerivationPath set to `44'/60'/0'/0`. * This is the default in TestRPC/Ganache, it can be overridden if desired. - * @param config Several available configurations + * @param config Configuration for the mnemonic wallet, must contain the mnemonic * @return MnemonicWalletSubprovider instance */ constructor(config: MnemonicWalletSubproviderConfigs) { @@ -88,9 +88,9 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { } /** - * Signs a transaction with the from account specificed in txParams. + * Signs a transaction with the account specificed by the `from` field in txParams. * If you've added this Subprovider to your app's provider, you can simply send - * an `eth_sendTransaction` JSON RPC request, and * this method will be called auto-magically. + * an `eth_sendTransaction` JSON RPC request, and this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. * @param txParams Parameters of the transaction to sign * @return Signed transaction hex string @@ -109,8 +109,8 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * If you've added the MnemonicWalletSubprovider to your app's provider, you can simply send an `eth_sign` * or `personal_sign` JSON RPC request, and this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. - * @param data Message to sign - * @param address Address to sign with + * @param data Hex string message to sign + * @param address Address of the account to sign with * @return Signature hex string (order: rsv) */ public async signPersonalMessageAsync(data: string, address: string): Promise { diff --git a/packages/subproviders/src/subproviders/private_key_wallet.ts b/packages/subproviders/src/subproviders/private_key_wallet.ts index f3727039c..b3f48fd90 100644 --- a/packages/subproviders/src/subproviders/private_key_wallet.ts +++ b/packages/subproviders/src/subproviders/private_key_wallet.ts @@ -59,12 +59,12 @@ export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider { } /** * Sign a personal Ethereum signed message. The signing address will be calculated from the private key. - * if an address is provided it must match the address calculated from the private key. + * The address must be provided it must match the address calculated from the private key. * If you've added this Subprovider to your app's provider, you can simply send an `eth_sign` * or `personal_sign` JSON RPC request, and this method will be called auto-magically. * If you are not using this via a ProviderEngine instance, you can call it directly. - * @param data Message to sign - * @param address Address to sign with + * @param data Hex string message to sign + * @param address Address of the account to sign with * @return Signature hex string (order: rsv) */ public async signPersonalMessageAsync(data: string, address: string): Promise { diff --git a/packages/typescript-typings/CHANGELOG.json b/packages/typescript-typings/CHANGELOG.json index 467cae29b..baafd800b 100644 --- a/packages/typescript-typings/CHANGELOG.json +++ b/packages/typescript-typings/CHANGELOG.json @@ -1,19 +1,14 @@ [ - { - "version": "0.1.1", - "changes": [ - { - "note": "Add types for HDKey", - "pr": 507 - } - ] - }, { "version": "0.1.0", "changes": [ { "note": "Add types for more packages", "pr": 501 + }, + { + "note": "Add types for HDKey", + "pr": 507 } ] }, diff --git a/packages/website/ts/containers/subproviders_documentation.ts b/packages/website/ts/containers/subproviders_documentation.ts index 7aa05f9a5..2462c4157 100644 --- a/packages/website/ts/containers/subproviders_documentation.ts +++ b/packages/website/ts/containers/subproviders_documentation.ts @@ -30,6 +30,8 @@ const docSections = { redundantRPCSubprovider: 'redundantRPCSubprovider', ganacheSubprovider: 'ganacheSubprovider', nonceTrackerSubprovider: 'nonceTrackerSubprovider', + privateKeyWalletSubprovider: 'privateKeyWalletSubprovider', + mnemonicWalletSubprovider: 'mnemonicWalletSubprovider', types: docConstants.TYPES_SECTION_NAME, }; @@ -44,6 +46,8 @@ const docsInfoConfig: DocsInfoConfig = { subprovider: [docSections.subprovider], ['ledger-subprovider']: [docSections.ledgerSubprovider], ['ledger-node-hid-issue']: [docSections.ledgerNodeHid], + ['private-key-wallet-subprovider']: [docSections.privateKeyWalletSubprovider], + ['mnemonic-wallet-subprovider']: [docSections.mnemonicWalletSubprovider], ['factory-methods']: [docSections.factoryMethods], ['emptyWallet-subprovider']: [docSections.emptyWalletSubprovider], ['fakeGasEstimate-subprovider']: [docSections.fakeGasEstimateSubprovider], @@ -61,6 +65,8 @@ const docsInfoConfig: DocsInfoConfig = { sectionNameToModulePath: { [docSections.subprovider]: ['"subproviders/src/subproviders/subprovider"'], [docSections.ledgerSubprovider]: ['"subproviders/src/subproviders/ledger"'], + [docSections.privateKeyWalletSubprovider]: ['"subproviders/src/subproviders/private_key_wallet"'], + [docSections.mnemonicWalletSubprovider]: ['"subproviders/src/subproviders/mnemonic_wallet"'], [docSections.factoryMethods]: ['"subproviders/src/index"'], [docSections.emptyWalletSubprovider]: ['"subproviders/src/subproviders/empty_wallet_subprovider"'], [docSections.fakeGasEstimateSubprovider]: ['"subproviders/src/subproviders/fake_gas_estimate_subprovider"'], @@ -75,6 +81,8 @@ const docsInfoConfig: DocsInfoConfig = { visibleConstructors: [ docSections.subprovider, docSections.ledgerSubprovider, + docSections.privateKeyWalletSubprovider, + docSections.mnemonicWalletSubprovider, docSections.emptyWalletSubprovider, docSections.fakeGasEstimateSubprovider, docSections.injectedWeb3Subprovider, @@ -97,6 +105,7 @@ const docsInfoConfig: DocsInfoConfig = { 'PartialTxParams', 'LedgerEthereumClient', 'LedgerSubproviderConfigs', + 'MnemonicWalletSubproviderConfigs', ], typeNameToExternalLink: { Web3: 'https://github.com/ethereum/wiki/wiki/JavaScript-API', -- cgit From b669508c34e541416b157babe3fc57d74216ee50 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Thu, 12 Apr 2018 16:49:42 +1000 Subject: Pluck key off of base path branch for Mnemonic This reduces the tree walk complexity in wallet utils as it is assumed that we always walk relative. It also removes a HACK :) --- packages/subproviders/src/subproviders/ledger.ts | 16 ++++----- .../src/subproviders/mnemonic_wallet.ts | 40 ++++++++++++---------- packages/subproviders/src/types.ts | 2 -- packages/subproviders/src/utils/wallet_utils.ts | 9 ++--- 4 files changed, 32 insertions(+), 35 deletions(-) diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 06c115105..d956a4f9d 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -26,6 +26,7 @@ const ASK_FOR_ON_DEVICE_CONFIRMATION = false; const SHOULD_GET_CHAIN_CODE = true; const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; +const INITIAL_KEY_DERIVATION_INDEX = 0; /** * Subprovider for interfacing with a user's [Ledger Nano S](https://www.ledgerwallet.com/products/ledger-nano-s). @@ -203,10 +204,11 @@ export class LedgerSubprovider extends BaseWalletSubprovider { private async _initialDerivedKeyInfoAsync(): Promise { const ledgerClient = await this._createLedgerClientAsync(); + const parentKeyDerivationPath = `m/${this._baseDerivationPath}`; let ledgerResponse; try { ledgerResponse = await ledgerClient.getAddress( - this._baseDerivationPath, + parentKeyDerivationPath, this._shouldAlwaysAskForConfirmation, SHOULD_GET_CHAIN_CODE, ); @@ -216,16 +218,14 @@ export class LedgerSubprovider extends BaseWalletSubprovider { const hdKey = new HDNode(); hdKey.publicKey = new Buffer(ledgerResponse.publicKey, 'hex'); hdKey.chainCode = new Buffer(ledgerResponse.chainCode, 'hex'); - const derivationPath = `${this._baseDerivationPath}/0`; - const derivationIndex = 0; - return { + const address = walletUtils.addressOfHDKey(hdKey); + const initialDerivedKeyInfo = { hdKey, - address: ledgerResponse.address, - isChildKey: true, + address, + derivationPath: parentKeyDerivationPath, baseDerivationPath: this._baseDerivationPath, - derivationPath, - derivationIndex, }; + return initialDerivedKeyInfo; } private _findDerivedKeyInfoForAddress(initalHDKey: DerivedHDKeyInfo, address: string): DerivedHDKeyInfo { const matchedDerivedKeyInfo = walletUtils.findDerivedKeyInfoForAddressIfExists( diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet.ts b/packages/subproviders/src/subproviders/mnemonic_wallet.ts index 5dc1b3be7..287dcfd7d 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet.ts @@ -14,8 +14,7 @@ import { PrivateKeyWalletSubprovider } from './private_key_wallet'; const DEFAULT_BASE_DERIVATION_PATH = `44'/60'/0'/0`; const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; -const ROOT_KEY_DERIVATION_PATH = 'm/'; -const ROOT_KEY_DERIVATION_INDEX = 0; +const INITIAL_KEY_DERIVATION_INDEX = 0; /** * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. @@ -26,6 +25,8 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { private _addressSearchLimit: number; private _baseDerivationPath: string; private _derivedKeyInfo: DerivedHDKeyInfo; + private _mnemonic: string; + /** * Instantiates a MnemonicWalletSubprovider. Defaults to baseDerivationPath set to `44'/60'/0'/0`. * This is the default in TestRPC/Ganache, it can be overridden if desired. @@ -41,20 +42,10 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { assert.isNumber('addressSearchLimit', addressSearchLimit); super(); - const seed = bip39.mnemonicToSeed(mnemonic); - const hdKey = HDNode.fromMasterSeed(seed); + this._mnemonic = mnemonic; this._baseDerivationPath = baseDerivationPath; - this._derivedKeyInfo = { - address: walletUtils.addressOfHDKey(hdKey), - // HACK this isn't the base path for this root key, but is is the base path - // we want all of the derived children to branched from - baseDerivationPath: this._baseDerivationPath, - derivationPath: ROOT_KEY_DERIVATION_PATH, - derivationIndex: ROOT_KEY_DERIVATION_INDEX, - hdKey, - isChildKey: false, - }; this._addressSearchLimit = addressSearchLimit; + this._derivedKeyInfo = this._initialDerivedKeyInfo(this._baseDerivationPath); } /** * Retrieve the set derivation path @@ -69,10 +60,7 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { */ public setPath(baseDerivationPath: string) { this._baseDerivationPath = baseDerivationPath; - this._derivedKeyInfo = { - ...this._derivedKeyInfo, - baseDerivationPath, - }; + this._derivedKeyInfo = this._initialDerivedKeyInfo(this._baseDerivationPath); } /** * Retrieve the accounts associated with the mnemonic. @@ -140,4 +128,20 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { } return matchedDerivedKeyInfo; } + private _initialDerivedKeyInfo(baseDerivationPath: string): DerivedHDKeyInfo { + const seed = bip39.mnemonicToSeed(this._mnemonic); + const hdKey = HDNode.fromMasterSeed(seed); + // Walk down to base derivation level (i.e m/44'/60'/0') and create an initial key at that level + // all children will then be walked relative (i.e m/0) + const parentKeyDerivationPath = `m/${baseDerivationPath}`; + const parentHDKeyAtDerivationPath = hdKey.derive(parentKeyDerivationPath); + const address = walletUtils.addressOfHDKey(parentHDKeyAtDerivationPath); + const derivedKeyInfo = { + address, + baseDerivationPath, + derivationPath: parentKeyDerivationPath, + hdKey: parentHDKeyAtDerivationPath, + }; + return derivedKeyInfo; + } } diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts index b9d9d08ee..74ecec23b 100644 --- a/packages/subproviders/src/types.ts +++ b/packages/subproviders/src/types.ts @@ -120,11 +120,9 @@ export enum NonceSubproviderErrors { } export interface DerivedHDKeyInfo { address: string; - derivationIndex: number; baseDerivationPath: string; derivationPath: string; hdKey: HDNode; - isChildKey: boolean; } export type ErrorCallback = (err: Error | null, data?: any) => void; diff --git a/packages/subproviders/src/utils/wallet_utils.ts b/packages/subproviders/src/utils/wallet_utils.ts index a37597dff..4db876748 100644 --- a/packages/subproviders/src/utils/wallet_utils.ts +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -20,20 +20,15 @@ class DerivedHDKeyInfoIterator implements IterableIterator { public next(): IteratorResult { const baseDerivationPath = this._initialDerivedKey.baseDerivationPath; const derivationIndex = this._index; - const isChildKey = this._initialDerivedKey.isChildKey; - // If the DerivedHDKey is a child then we walk relative, if not we walk the full derivation path const fullDerivationPath = `m/${baseDerivationPath}/${derivationIndex}`; - const relativeDerivationPath = `m/${derivationIndex}`; - const path = isChildKey ? relativeDerivationPath : fullDerivationPath; + const path = `m/${derivationIndex}`; const hdKey = this._initialDerivedKey.hdKey.derive(path); const address = walletUtils.addressOfHDKey(hdKey); const derivedKey: DerivedHDKeyInfo = { address, hdKey, baseDerivationPath, - derivationIndex, derivationPath: fullDerivationPath, - isChildKey, }; const done = this._index === this._searchLimit; this._index++; @@ -78,7 +73,7 @@ export const walletUtils = { const ethereumAddressUnprefixed = ethUtil .publicToAddress(derivedPublicKey, shouldSanitizePublicKey) .toString('hex'); - const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed); + const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed).toLowerCase(); return address; }, }; -- cgit From ce3f25d48f3eb7a71953515e30a3f0c49881eac4 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Thu, 12 Apr 2018 17:10:17 +1000 Subject: Rename to parentDerivedKeyInfo to be explicity about how we walk the tree --- packages/subproviders/src/utils/wallet_utils.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/subproviders/src/utils/wallet_utils.ts b/packages/subproviders/src/utils/wallet_utils.ts index 4db876748..cd5cd9ba0 100644 --- a/packages/subproviders/src/utils/wallet_utils.ts +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -7,22 +7,22 @@ import { DerivedHDKeyInfo, WalletSubproviderErrors } from '../types'; const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; class DerivedHDKeyInfoIterator implements IterableIterator { - private _initialDerivedKey: DerivedHDKeyInfo; + private _parentDerivedKeyInfo: DerivedHDKeyInfo; private _searchLimit: number; private _index: number; constructor(initialDerivedKey: DerivedHDKeyInfo, searchLimit: number = DEFAULT_ADDRESS_SEARCH_LIMIT) { this._searchLimit = searchLimit; - this._initialDerivedKey = initialDerivedKey; + this._parentDerivedKeyInfo = initialDerivedKey; this._index = 0; } public next(): IteratorResult { - const baseDerivationPath = this._initialDerivedKey.baseDerivationPath; + const baseDerivationPath = this._parentDerivedKeyInfo.baseDerivationPath; const derivationIndex = this._index; const fullDerivationPath = `m/${baseDerivationPath}/${derivationIndex}`; const path = `m/${derivationIndex}`; - const hdKey = this._initialDerivedKey.hdKey.derive(path); + const hdKey = this._parentDerivedKeyInfo.hdKey.derive(path); const address = walletUtils.addressOfHDKey(hdKey); const derivedKey: DerivedHDKeyInfo = { address, @@ -44,9 +44,9 @@ class DerivedHDKeyInfoIterator implements IterableIterator { } export const walletUtils = { - calculateDerivedHDKeyInfos(initialDerivedKey: DerivedHDKeyInfo, numberOfKeys: number): DerivedHDKeyInfo[] { + calculateDerivedHDKeyInfos(parentDerivedKeyInfo: DerivedHDKeyInfo, numberOfKeys: number): DerivedHDKeyInfo[] { const derivedKeys: DerivedHDKeyInfo[] = []; - const derivedKeyIterator = new DerivedHDKeyInfoIterator(initialDerivedKey, numberOfKeys); + const derivedKeyIterator = new DerivedHDKeyInfoIterator(parentDerivedKeyInfo, numberOfKeys); for (const key of derivedKeyIterator) { derivedKeys.push(key); } @@ -54,11 +54,11 @@ export const walletUtils = { }, findDerivedKeyInfoForAddressIfExists( address: string, - initialDerivedKey: DerivedHDKeyInfo, + parentDerivedKeyInfo: DerivedHDKeyInfo, searchLimit: number, ): DerivedHDKeyInfo | undefined { let matchedKey: DerivedHDKeyInfo | undefined; - const derivedKeyIterator = new DerivedHDKeyInfoIterator(initialDerivedKey, searchLimit); + const derivedKeyIterator = new DerivedHDKeyInfoIterator(parentDerivedKeyInfo, searchLimit); for (const key of derivedKeyIterator) { if (key.address === address) { matchedKey = key; -- cgit From 9a91e39b3f9419287ba0c508b86d519db4e72dd7 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Thu, 12 Apr 2018 17:26:17 +1000 Subject: Revert Ledger back to assigning ledgerClient to instance variable --- packages/subproviders/CHANGELOG.json | 14 +++++++++++--- packages/subproviders/src/subproviders/ledger.ts | 17 +++++++++-------- .../subproviders/src/subproviders/mnemonic_wallet.ts | 6 ++---- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/packages/subproviders/CHANGELOG.json b/packages/subproviders/CHANGELOG.json index 9166c93d5..d3ba7a928 100644 --- a/packages/subproviders/CHANGELOG.json +++ b/packages/subproviders/CHANGELOG.json @@ -8,16 +8,24 @@ "pr": 500 }, { - "note": "Add private key subprovider and refactor shared functionality into a base wallet subprovider", + "note": "Add PrivateKeySubprovider and refactor shared functionality into a base wallet subprovider", "pr": 506 }, { - "note": "Add mnemonic wallet subprovider, deprecating our truffle-hdwallet-provider fork.", + "note": "Add MnemonicWalletsubprovider, deprecating our truffle-hdwallet-provider fork", + "pr": 507 + }, + { + "note": "Support multiple addresses in ledger and mnemonic wallets", "pr": 507 }, { "note": - "Support multiple addresses in ledger and mnemonic wallets. Renamed derivationPath to baseDerivationPath.", + "Refactors LedgerSubprovider such that explicitly setting the `pathIndex` is no longer required. Simply set the request `from` address as desired", + "pr": 507 + }, + { + "note": "Renamed derivationPath to baseDerivationPath.", "pr": 507 } ], diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index d956a4f9d..563e5a56a 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -26,7 +26,6 @@ const ASK_FOR_ON_DEVICE_CONFIRMATION = false; const SHOULD_GET_CHAIN_CODE = true; const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; -const INITIAL_KEY_DERIVATION_INDEX = 0; /** * Subprovider for interfacing with a user's [Ledger Nano S](https://www.ledgerwallet.com/products/ledger-nano-s). @@ -109,7 +108,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { const initialDerivedKeyInfo = await this._initialDerivedKeyInfoAsync(); const derivedKeyInfo = this._findDerivedKeyInfoForAddress(initialDerivedKeyInfo, txParams.from); - const ledgerClient = await this._createLedgerClientAsync(); + this._ledgerClientIfExists = await this._createLedgerClientAsync(); const tx = new EthereumTx(txParams); @@ -121,7 +120,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { const txHex = tx.serialize().toString('hex'); try { const fullDerivationPath = derivedKeyInfo.derivationPath; - const result = await ledgerClient.signTransaction(fullDerivationPath, txHex); + const result = await this._ledgerClientIfExists.signTransaction(fullDerivationPath, txHex); // Store signature in transaction tx.r = Buffer.from(result.r, 'hex'); tx.s = Buffer.from(result.s, 'hex'); @@ -163,10 +162,13 @@ export class LedgerSubprovider extends BaseWalletSubprovider { const initialDerivedKeyInfo = await this._initialDerivedKeyInfoAsync(); const derivedKeyInfo = this._findDerivedKeyInfoForAddress(initialDerivedKeyInfo, address); - const ledgerClient = await this._createLedgerClientAsync(); + this._ledgerClientIfExists = await this._createLedgerClientAsync(); try { const fullDerivationPath = derivedKeyInfo.derivationPath; - const result = await ledgerClient.signPersonalMessage(fullDerivationPath, ethUtil.stripHexPrefix(data)); + const result = await this._ledgerClientIfExists.signPersonalMessage( + fullDerivationPath, + ethUtil.stripHexPrefix(data), + ); const v = result.v - 27; let vHex = v.toString(16); if (vHex.length < 2) { @@ -188,7 +190,6 @@ export class LedgerSubprovider extends BaseWalletSubprovider { } const ledgerEthereumClient = await this._ledgerEthereumClientFactoryAsync(); this._connectionLock.release(); - this._ledgerClientIfExists = ledgerEthereumClient; return ledgerEthereumClient; } private async _destroyLedgerClientAsync() { @@ -202,12 +203,12 @@ export class LedgerSubprovider extends BaseWalletSubprovider { this._connectionLock.release(); } private async _initialDerivedKeyInfoAsync(): Promise { - const ledgerClient = await this._createLedgerClientAsync(); + this._ledgerClientIfExists = await this._createLedgerClientAsync(); const parentKeyDerivationPath = `m/${this._baseDerivationPath}`; let ledgerResponse; try { - ledgerResponse = await ledgerClient.getAddress( + ledgerResponse = await this._ledgerClientIfExists.getAddress( parentKeyDerivationPath, this._shouldAlwaysAskForConfirmation, SHOULD_GET_CHAIN_CODE, diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet.ts b/packages/subproviders/src/subproviders/mnemonic_wallet.ts index 287dcfd7d..080bfeb4c 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet.ts @@ -14,7 +14,6 @@ import { PrivateKeyWalletSubprovider } from './private_key_wallet'; const DEFAULT_BASE_DERIVATION_PATH = `44'/60'/0'/0`; const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10; const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000; -const INITIAL_KEY_DERIVATION_INDEX = 0; /** * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. @@ -34,15 +33,14 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { * @return MnemonicWalletSubprovider instance */ constructor(config: MnemonicWalletSubproviderConfigs) { - const mnemonic = config.mnemonic; - assert.isString('mnemonic', mnemonic); + assert.isString('mnemonic', config.mnemonic); const baseDerivationPath = config.baseDerivationPath || DEFAULT_BASE_DERIVATION_PATH; assert.isString('baseDerivationPath', baseDerivationPath); const addressSearchLimit = config.addressSearchLimit || DEFAULT_ADDRESS_SEARCH_LIMIT; assert.isNumber('addressSearchLimit', addressSearchLimit); super(); - this._mnemonic = mnemonic; + this._mnemonic = config.mnemonic; this._baseDerivationPath = baseDerivationPath; this._addressSearchLimit = addressSearchLimit; this._derivedKeyInfo = this._initialDerivedKeyInfo(this._baseDerivationPath); -- cgit