From 0e8f5004d6a53929a4cc1de9231c43899c8b6eeb Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Fri, 6 Apr 2018 18:54:38 +1000 Subject: [PATCH 01/21] 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 + .../subproviders/src/subproviders/ledger.ts | 1 - .../mnemonic_wallet_subprovider.ts | 123 ++++++++++++ .../private_key_wallet_subprovider.ts | 4 +- packages/subproviders/src/types.ts | 3 + .../unit/mnemonic_wallet_subprovider_test.ts | 182 ++++++++++++++++++ .../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 f0702cd5dd..ccf807695e 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 cadbac0e87..d557eef29b 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 c5ad268767..754d164e84 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 dd553fde45..9c30c6ba12 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 aa86bf6c0a..23ed3c93ef 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 0000000000..fdb497776f --- /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 c3a53773ab..0aa2fb5902 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 bacb7091b4..de04499ce3 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 0000000000..e58461005d --- /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 890573d0d1..5ce3ff08f0 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', From 5b69cd4a22ce1720f4441aaa74c86f895015c0fd Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Tue, 10 Apr 2018 11:58:12 +1000 Subject: [PATCH 02/21] Added walletUtils and address in signMessage --- .../subproviders/base_wallet_subprovider.ts | 5 +- .../mnemonic_wallet_subprovider.ts | 85 ++++++++----------- .../private_key_wallet_subprovider.ts | 6 +- packages/subproviders/src/types.ts | 10 ++- packages/subproviders/src/walletUtils.ts | 58 +++++++++++++ .../unit/mnemonic_wallet_subprovider_test.ts | 26 ++++-- .../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 034f83e7fb..47b45a126f 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 fdb497776f..456bde05c7 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 0aa2fb5902..f6906bab6b 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 de04499ce3..105ffa7cc9 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 0000000000..631636a710 --- /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 e58461005d..7aaef49449 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 ca06658719..8aaddeaf4b 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); + }); }); }); }); From 84a4b7d1c16d008ffbf07ba5b22c61b025b5165f Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Tue, 10 Apr 2018 14:39:43 +1000 Subject: [PATCH 03/21] Refactor ledger to support multiple accounts with from address --- .../subproviders/src/subproviders/ledger.ts | 111 ++++++++++-------- .../mnemonic_wallet_subprovider.ts | 5 +- packages/subproviders/src/types.ts | 1 + .../{walletUtils.ts => utils/wallet_utils.ts} | 30 ++++- .../integration/ledger_subprovider_test.ts | 3 +- .../test/unit/ledger_subprovider_test.ts | 8 +- .../unit/mnemonic_wallet_subprovider_test.ts | 5 +- .../subproviders/test/utils/fixture_data.ts | 2 + 8 files changed, 103 insertions(+), 62 deletions(-) rename packages/subproviders/src/{walletUtils.ts => utils/wallet_utils.ts} (70%) diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 23ed3c93ef..a7b79c1280 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 456bde05c7..3ff4376599 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 105ffa7cc9..fe7ae921ec 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/walletUtils.ts b/packages/subproviders/src/utils/wallet_utils.ts similarity index 70% rename from packages/subproviders/src/walletUtils.ts rename to packages/subproviders/src/utils/wallet_utils.ts index 631636a710..6c698a006d 100644 --- a/packages/subproviders/src/walletUtils.ts +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -2,20 +2,30 @@ import ethUtil = require('ethereumjs-util'); import HDNode = require('hdkey'); import * as _ from 'lodash'; -import { DerivedHDKey, WalletSubproviderErrors } from './types'; +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 path = `m/${derivationPath}/${i + offset}`; + const derivationIndex = offset + i; + const path = hardened ? `m/${derivationIndex}` : `m/${derivationPath}/${derivationIndex}`; const hdKey = initialHDKey.derive(path); const derivedPublicKey = hdKey.publicKey; const shouldSanitizePublicKey = true; @@ -24,9 +34,10 @@ export const walletUtils = { .toString('hex'); const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed); const derivedKey: DerivedHDKey = { - derivationPath: path, + derivationPath, hdKey, address, + derivationIndex, }; derivedKeys.push(derivedKey); }); @@ -38,10 +49,17 @@ export const walletUtils = { 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); + const derivedKeys = walletUtils._calculateDerivedHDKeys( + initialHDKey, + derivationPath, + BATCH_SIZE, + index, + hardened, + ); matchedKey = _.find(derivedKeys, derivedKey => derivedKey.address === address); if (matchedKey) { break; @@ -50,8 +68,8 @@ export const walletUtils = { return matchedKey; }, - _firstDerivedKey(initialHDKey: HDNode, derivationPath: string): DerivedHDKey { - const derivedKeys = walletUtils._calculateDerivedHDKeys(initialHDKey, derivationPath, 1); + _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/test/integration/ledger_subprovider_test.ts b/packages/subproviders/test/integration/ledger_subprovider_test.ts index 503618089d..88a3c6db10 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 c18506681d..d3a3fbe26c 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 7aaef49449..16981cf86d 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 5ce3ff08f0..62b76e1328 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', From a34c9095c3a5d217d50318471660a5508922bcc2 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Tue, 10 Apr 2018 15:50:08 +1000 Subject: [PATCH 04/21] Rename to IS_CHILD_KEY --- .../subproviders/src/subproviders/ledger.ts | 8 +++---- .../subproviders/src/utils/wallet_utils.ts | 22 ++++++++----------- .../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 a7b79c1280..fabff88cd4 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 6c698a006d..48d4755591 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 16981cf86d..77e5d35aeb 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); From 4e4842a62f6b2d728e2d9986acf5116032f763f2 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Tue, 10 Apr 2018 16:01:34 +1000 Subject: [PATCH 05/21] Improve Documentation for functions and constructors --- .../mnemonic_wallet_subprovider.ts | 18 ++++++++++++++---- .../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 3ff4376599..02d06e2cd6 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 f6906bab6b..005d36f936 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 From 9169913a2cbe9d421629db2e8169c0806044e3fc Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 12:22:41 +1000 Subject: [PATCH 06/21] Add isChildKey to derived key --- .../subproviders/src/subproviders/ledger.ts | 54 ++++++++--------- .../mnemonic_wallet_subprovider.ts | 30 ++++++---- packages/subproviders/src/types.ts | 2 + .../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 fabff88cd4..6f66e3018d 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 02d06e2cd6..53013c44c6 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 fe7ae921ec..9f557731f4 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 48d4755591..0c3e895b69 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; }, From 20a1deb187d8fb38a42ae87fc8f046dff4b4ba61 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 12:47:17 +1000 Subject: [PATCH 07/21] Throw errors when the address is not specified or invalid --- .../subproviders/base_wallet_subprovider.ts | 2 +- .../subproviders/src/subproviders/ledger.ts | 31 +++++++++---------- .../mnemonic_wallet_subprovider.ts | 27 ++++++++++------ .../private_key_wallet_subprovider.ts | 12 +++---- packages/subproviders/src/types.ts | 4 +-- .../integration/ledger_subprovider_test.ts | 5 ++- .../test/unit/ledger_subprovider_test.ts | 5 +-- .../unit/mnemonic_wallet_subprovider_test.ts | 2 +- .../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 47b45a126f..0a9b99ae49 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 6f66e3018d..3c0442e4c3 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 53013c44c6..730453bc6b 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 005d36f936..f8afab722c 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 9f557731f4..140c7c2dfe 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 88a3c6db10..4a831af67b 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 d3a3fbe26c..ad1154831f 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 77e5d35aeb..a1a5a22963 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 8aaddeaf4b..b84aebb18f 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); From eee190826a44dfe5444f15165577ea73b4f78c2f Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 12:51:57 +1000 Subject: [PATCH 08/21] Test signed messages with the second account --- .../test/integration/ledger_subprovider_test.ts | 9 ++++++++- .../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 4a831af67b..ea5010696c 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 a1a5a22963..aacaef9f4f 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 62b76e1328..f47fbd4281 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: { From 65b2c936abdf3a67d471cd17eeed5069825ac3d4 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 12:56:52 +1000 Subject: [PATCH 09/21] Tests for signed transactions and multiple accounts --- .../subproviders/test/integration/ledger_subprovider_test.ts | 5 +++++ .../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 ea5010696c..2db4befb3e 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 aacaef9f4f..2bc84abc10 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 f47fbd4281..57b69b2f8d 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', }; From 4017c172a217b4ba225560cb358f04d2227b6d69 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 13:43:43 +1000 Subject: [PATCH 10/21] Iterator pattern for walking derived keys --- .../subproviders/src/subproviders/ledger.ts | 3 +- .../subproviders/src/utils/wallet_utils.ts | 107 ++++++++++-------- packages/subproviders/tsconfig.json | 3 +- 3 files changed, 65 insertions(+), 48 deletions(-) diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 3c0442e4c3..6c685c84de 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 0c3e895b69..bd1851d9a5 100644 --- a/packages/subproviders/src/utils/wallet_utils.ts +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -6,35 +6,73 @@ 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( + calculateDerivedHDKeys(initialDerivedKey: DerivedHDKey, searchLimit: number): DerivedHDKey[] { + const derivedKeys: DerivedHDKey[] = []; + const derivedKeyIterator = new DerivedHDKeyIterator(initialDerivedKey, searchLimit); + for (const key of derivedKeyIterator) { + derivedKeys.push(key); + } + return derivedKeys; + }, + findDerivedKeyByAddress( + address: string, initialDerivedKey: DerivedHDKey, searchLimit: number, - offset: number = DEFAULT_ADDRESS_SEARCH_OFFSET, - ): 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); - }); - return derivedKeys; + ): DerivedHDKey | undefined { + let matchedKey: DerivedHDKey | undefined; + const derivedKeyIterator = new DerivedHDKeyIterator(initialDerivedKey, searchLimit); + for (const key of derivedKeyIterator) { + if (key.address === address) { + matchedKey = key; + break; + } + } + return matchedKey; }, addressOfHDKey(hdKey: HDNode): string { const shouldSanitizePublicKey = true; @@ -45,25 +83,4 @@ export const walletUtils = { 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) { - break; - } - } - return matchedKey; - }, - - _firstDerivedKey(initialDerivedKey: DerivedHDKey): DerivedHDKey { - const derivedKeys = walletUtils.calculateDerivedHDKeys(initialDerivedKey, 1, 0); - const firstDerivedKey = derivedKeys[0]; - return firstDerivedKey; - }, }; diff --git a/packages/subproviders/tsconfig.json b/packages/subproviders/tsconfig.json index e358165538..72dfee80b8 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/**/*"] } From a824957de7e692d9932a591bb5730ba798bcd8ed Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 13:46:01 +1000 Subject: [PATCH 11/21] Follow file naming pattern for mnemonic and private key subprovider --- packages/subproviders/src/index.ts | 4 ++-- .../{mnemonic_wallet_subprovider.ts => mnemonic_wallet.ts} | 2 +- ...rivate_key_wallet_subprovider.ts => private_key_wallet.ts} | 0 3 files changed, 3 insertions(+), 3 deletions(-) rename packages/subproviders/src/subproviders/{mnemonic_wallet_subprovider.ts => mnemonic_wallet.ts} (99%) rename packages/subproviders/src/subproviders/{private_key_wallet_subprovider.ts => private_key_wallet.ts} (100%) diff --git a/packages/subproviders/src/index.ts b/packages/subproviders/src/index.ts index 9c30c6ba12..01aec956ae 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_subprovider.ts b/packages/subproviders/src/subproviders/mnemonic_wallet.ts similarity index 99% rename from packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts rename to packages/subproviders/src/subproviders/mnemonic_wallet.ts index 730453bc6b..40f5842508 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet_subprovider.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet.ts @@ -8,7 +8,7 @@ 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'; +import { PrivateKeyWalletSubprovider } from './private_key_wallet'; const DEFAULT_DERIVATION_PATH = `44'/60'/0'/0`; diff --git a/packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts b/packages/subproviders/src/subproviders/private_key_wallet.ts similarity index 100% rename from packages/subproviders/src/subproviders/private_key_wallet_subprovider.ts rename to packages/subproviders/src/subproviders/private_key_wallet.ts From 260ab2d4134e24a3a2f3fab845fa72c1e1766a3e Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 14:04:27 +1000 Subject: [PATCH 12/21] Update changelog and add derivationBasePath --- packages/subproviders/CHANGELOG.json | 3 ++- .../subproviders/src/subproviders/ledger.ts | 26 +++++++++---------- .../src/subproviders/mnemonic_wallet.ts | 19 +++++++------- packages/subproviders/src/types.ts | 3 ++- .../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 ccf807695e..554fbd0cfd 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 6c685c84de..975893f8a4 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 40f5842508..972f7e65e2 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 140c7c2dfe..c76be1bf83 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 bd1851d9a5..2d9d14e449 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, }; From 4aa67e292504fea307a4e5f15a124349fc769da6 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 14:11:16 +1000 Subject: [PATCH 13/21] Update JSDoc for methods in ledger and mnemonic wallet --- packages/subproviders/src/subproviders/ledger.ts | 6 +++--- .../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 975893f8a4..0c037b488e 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 972f7e65e2..855db21ad0 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 2d9d14e449..097d2b82f3 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); } From 3ad693d33409dfd9d61beff3f43c4abaa369c6b1 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 14:22:02 +1000 Subject: [PATCH 14/21] Test valid address format but not found --- packages/subproviders/src/subproviders/ledger.ts | 2 +- .../src/subproviders/mnemonic_wallet.ts | 7 +++++-- packages/subproviders/src/utils/wallet_utils.ts | 9 +++++---- .../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 0c037b488e..9b2d9d7d07 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 855db21ad0..de34a9d119 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 097d2b82f3..d5ebf5ce64 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 2bc84abc10..9131a8b6aa 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 57b69b2f8d..a973961cea 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, From b08c616713e153ca767d2f01976da5f1ef903251 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 14:53:58 +1000 Subject: [PATCH 15/21] 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 754d164e84..4b3ecdf3c1 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 294d9ee9f1..467cae29bf 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 0000000000..84b751bd7f --- /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; +} From f44ef7ce59cd5c811a92662d3fb095f21d80f665 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 15:12:02 +1000 Subject: [PATCH 16/21] 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 +------ .../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 0000000000..61675f9eae --- /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 01aec956ae..dc45ea9f18 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 c76be1bf83..1219922780 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 fd34ab82d9..bb1f47dd04 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 d7190c0bbc..a72d33183a 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: [PATCH 17/21] 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 +- .../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 +++- .../subproviders/src/utils/wallet_utils.ts | 40 ++++---- .../integration/ledger_subprovider_test.ts | 2 +- .../unit/mnemonic_wallet_subprovider_test.ts | 12 +-- .../private_key_wallet_subprovider_test.ts | 16 ++-- .../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 554fbd0cfd..5287ee9870 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 61675f9eae..0000000000 --- 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 d557eef29b..afb6ef5d6c 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 9b2d9d7d07..33aa5c1bf9 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 de34a9d119..9a627b1ec6 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 f8afab722c..f3727039cb 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 1219922780..b9d9d08ee8 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 2db4befb3e..11b5f6410c 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 9131a8b6aa..93300f47d3 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 b84aebb18f..5c1b5cd25b 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 a973961cea..3137e08b0a 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', From 63b941fbaf08167234cf7871e874c1a96e4347fa Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 20:51:31 +1000 Subject: [PATCH 18/21] Update documentation and add to website docs --- packages/subproviders/src/subproviders/ledger.ts | 12 ++++++------ .../src/subproviders/mnemonic_wallet.ts | 10 +++++----- .../src/subproviders/private_key_wallet.ts | 6 +++--- packages/typescript-typings/CHANGELOG.json | 13 ++++--------- .../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 33aa5c1bf9..06c1151053 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 9a627b1ec6..5dc1b3be75 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 f3727039cb..b3f48fd904 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 467cae29bf..baafd800bf 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 7aa05f9a5a..2462c41576 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', From b669508c34e541416b157babe3fc57d74216ee50 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Thu, 12 Apr 2018 16:49:42 +1000 Subject: [PATCH 19/21] 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 :) --- .../subproviders/src/subproviders/ledger.ts | 16 ++++---- .../src/subproviders/mnemonic_wallet.ts | 40 ++++++++++--------- packages/subproviders/src/types.ts | 2 - .../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 06c1151053..d956a4f9d7 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 5dc1b3be75..287dcfd7d1 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 b9d9d08ee8..74ecec23b5 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 a37597dff4..4db8767482 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; }, }; From ce3f25d48f3eb7a71953515e30a3f0c49881eac4 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Thu, 12 Apr 2018 17:10:17 +1000 Subject: [PATCH 20/21] 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 4db8767482..cd5cd9ba0d 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; From 9a91e39b3f9419287ba0c508b86d519db4e72dd7 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Thu, 12 Apr 2018 17:26:17 +1000 Subject: [PATCH 21/21] Revert Ledger back to assigning ledgerClient to instance variable --- packages/subproviders/CHANGELOG.json | 14 +++++++++++--- .../subproviders/src/subproviders/ledger.ts | 17 +++++++++-------- .../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 9166c93d53..d3ba7a9286 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 d956a4f9d7..563e5a56ad 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 287dcfd7d1..080bfeb4c2 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);