From 3ad693d33409dfd9d61beff3f43c4abaa369c6b1 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 11 Apr 2018 14:22:02 +1000 Subject: Test valid address format but not found --- packages/subproviders/src/subproviders/ledger.ts | 2 +- .../subproviders/src/subproviders/mnemonic_wallet.ts | 7 +++++-- packages/subproviders/src/utils/wallet_utils.ts | 9 +++++---- .../test/unit/mnemonic_wallet_subprovider_test.ts | 16 ++++++++++++---- packages/subproviders/test/utils/fixture_data.ts | 4 +++- 5 files changed, 26 insertions(+), 12 deletions(-) (limited to 'packages/subproviders') diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 0c037b488..9b2d9d7d0 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -222,7 +222,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider { }; } private _findDerivedKeyByPublicAddress(initalHDKey: DerivedHDKey, address: string): DerivedHDKey { - if (_.isUndefined(address)) { + if (_.isUndefined(address) || !addressUtils.isAddress(address)) { throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); } const matchedDerivedKey = walletUtils.findDerivedKeyByAddress(address, initalHDKey, this._addressSearchLimit); diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet.ts b/packages/subproviders/src/subproviders/mnemonic_wallet.ts index 855db21ad..de34a9d11 100644 --- a/packages/subproviders/src/subproviders/mnemonic_wallet.ts +++ b/packages/subproviders/src/subproviders/mnemonic_wallet.ts @@ -1,4 +1,5 @@ import { assert } from '@0xproject/assert'; +import { addressUtils } from '@0xproject/utils'; import * as bip39 from 'bip39'; import ethUtil = require('ethereumjs-util'); import HDNode = require('hdkey'); @@ -43,8 +44,10 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { this._derivationBasePath = derivationBasePath; this._derivedKey = { address: walletUtils.addressOfHDKey(hdKey), + // HACK this isn't the base path for this root key, but is is the base path + // we want all of the derived children to spawn from derivationBasePath: this._derivationBasePath, - derivationPath: `${this._derivationBasePath}/${0}`, + derivationPath: 'm/0', derivationIndex: 0, hdKey, isChildKey: false, @@ -119,7 +122,7 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider { return privateKeyWallet; } private _findDerivedKeyByPublicAddress(address: string): DerivedHDKey { - if (_.isUndefined(address)) { + if (_.isUndefined(address) || !addressUtils.isAddress(address)) { throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid); } const matchedDerivedKey = walletUtils.findDerivedKeyByAddress( diff --git a/packages/subproviders/src/utils/wallet_utils.ts b/packages/subproviders/src/utils/wallet_utils.ts index 097d2b82f..d5ebf5ce6 100644 --- a/packages/subproviders/src/utils/wallet_utils.ts +++ b/packages/subproviders/src/utils/wallet_utils.ts @@ -22,19 +22,20 @@ class DerivedHDKeyIterator implements IterableIterator { public next(): IteratorResult { const derivationBasePath = this._initialDerivedKey.derivationBasePath; const derivationIndex = this._index; + const isChildKey = this._initialDerivedKey.isChildKey; // If the DerivedHDKey is a child then we walk relative, if not we walk the full derivation path const fullDerivationPath = `m/${derivationBasePath}/${derivationIndex}`; const relativeDerivationPath = `m/${derivationIndex}`; - const path = this._initialDerivedKey.isChildKey ? relativeDerivationPath : fullDerivationPath; + const path = isChildKey ? relativeDerivationPath : fullDerivationPath; const hdKey = this._initialDerivedKey.hdKey.derive(path); const address = walletUtils.addressOfHDKey(hdKey); const derivedKey: DerivedHDKey = { address, hdKey, - derivationPath: fullDerivationPath, - derivationBasePath: this._initialDerivedKey.derivationBasePath, + derivationBasePath, derivationIndex, - isChildKey: this._initialDerivedKey.isChildKey, + derivationPath: fullDerivationPath, + isChildKey, }; const done = this._index === this._searchLimit; this._index++; diff --git a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts index 2bc84abc1..9131a8b6a 100644 --- a/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts +++ b/packages/subproviders/test/unit/mnemonic_wallet_subprovider_test.ts @@ -55,10 +55,16 @@ describe('MnemonicWalletSubprovider', () => { }); }); describe('failure cases', () => { - it('throws an error if account cannot be found', async () => { + it('throws an error if address is invalid ', async () => { const txData = { ...fixtureData.TX_DATA, from: '0x0' }; return expect(subprovider.signTransactionAsync(txData)).to.be.rejectedWith( - WalletSubproviderErrors.AddressNotFound, + WalletSubproviderErrors.FromAddressMissingOrInvalid, + ); + }); + it('throws an error if address is valid format but not found', async () => { + const txData = { ...fixtureData.TX_DATA, from: fixtureData.NULL_ADDRESS }; + return expect(subprovider.signTransactionAsync(txData)).to.be.rejectedWith( + `${WalletSubproviderErrors.AddressNotFound}: ${fixtureData.NULL_ADDRESS}`, ); }); }); @@ -155,12 +161,14 @@ describe('MnemonicWalletSubprovider', () => { const payload = { jsonrpc: '2.0', method: 'personal_sign', - params: [nonHexMessage, '0x0'], + params: [nonHexMessage, fixtureData.NULL_ADDRESS], id: 1, }; const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => { expect(err).to.not.be.a('null'); - expect(err.message).to.be.equal(`${WalletSubproviderErrors.AddressNotFound}: 0x0`); + expect(err.message).to.be.equal( + `${WalletSubproviderErrors.AddressNotFound}: ${fixtureData.NULL_ADDRESS}`, + ); done(); }); provider.sendAsync(payload, callback); diff --git a/packages/subproviders/test/utils/fixture_data.ts b/packages/subproviders/test/utils/fixture_data.ts index 57b69b2f8..a973961ce 100644 --- a/packages/subproviders/test/utils/fixture_data.ts +++ b/packages/subproviders/test/utils/fixture_data.ts @@ -1,7 +1,9 @@ const TEST_RPC_ACCOUNT_0 = '0x5409ed021d9299bf6814279a6a1411a7e866a631'; const TEST_RPC_ACCOUNT_1 = '0x6ecbe1db9ef729cbe972c83fb886247691fb6beb'; +const NULL_ADDRESS = '0x0000000000000000000000000000000000000000'; const networkId = 42; export const fixtureData = { + NULL_ADDRESS, TEST_RPC_ACCOUNT_0, TEST_RPC_ACCOUNT_0_ACCOUNT_PRIVATE_KEY: 'F2F48EE19680706196E2E339E5DA3491186E0C4C5030670656B0E0164837257D', TEST_RPC_ACCOUNT_1, @@ -18,7 +20,7 @@ export const fixtureData = { nonce: '0x00', gasPrice: '0x0', gas: '0x2710', - to: '0x0000000000000000000000000000000000000000', + to: NULL_ADDRESS, value: '0x00', chainId: networkId, from: TEST_RPC_ACCOUNT_0, -- cgit v1.2.3