diff options
25 files changed, 122 insertions, 95 deletions
diff --git a/packages/0x.js/CHANGELOG.md b/packages/0x.js/CHANGELOG.md index 963b3d56c..9874b0ca8 100644 --- a/packages/0x.js/CHANGELOG.md +++ b/packages/0x.js/CHANGELOG.md @@ -1,5 +1,11 @@ # CHANGELOG +v0.28.0 - _TBD_ +------------------------ +* Add `etherTokenAddress` arg to `depositAsync` and `withdrawAsync` methods on `zeroEx.etherToken` (#267) +* Removed accidentally included `unsubscribeAll` method from `zeroEx.proxy`, `zeroEx.etherToken` and `zeroEx.tokenRegistry` (#267) +* Removed `etherTokenContractAddress` from `ZeroEx` constructor arg `ZeroExConfig` (#267) + v0.27.1 - _November 28, 2017_ ------------------------ * Export `TransactionOpts` type diff --git a/packages/0x.js/src/0x.ts b/packages/0x.js/src/0x.ts index 5a2d6cb05..41fefb993 100644 --- a/packages/0x.js/src/0x.ts +++ b/packages/0x.js/src/0x.ts @@ -199,7 +199,7 @@ export class ZeroEx { this._web3Wrapper, config.networkId, config.tokenRegistryContractAddress, ); this.etherToken = new EtherTokenWrapper( - this._web3Wrapper, config.networkId, this.token, config.etherTokenContractAddress, + this._web3Wrapper, config.networkId, this.token, ); this.orderStateWatcher = new OrderStateWatcher( this._web3Wrapper, this._abiDecoder, this.token, this.exchange, config.orderWatcherConfig, diff --git a/packages/0x.js/src/contract_wrappers/contract_wrapper.ts b/packages/0x.js/src/contract_wrappers/contract_wrapper.ts index a796dc1ec..46916ebf4 100644 --- a/packages/0x.js/src/contract_wrappers/contract_wrapper.ts +++ b/packages/0x.js/src/contract_wrappers/contract_wrapper.ts @@ -50,10 +50,7 @@ export class ContractWrapper { this._onLogAddedSubscriptionToken = undefined; this._onLogRemovedSubscriptionToken = undefined; } - /** - * Cancels all existing subscriptions - */ - public unsubscribeAll(): void { + protected unsubscribeAll(): void { const filterTokens = _.keys(this._filterCallbacks); _.each(filterTokens, filterToken => { this._unsubscribe(filterToken); diff --git a/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts b/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts index 896cfde3d..a6acbe45d 100644 --- a/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts +++ b/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts @@ -17,24 +17,22 @@ import {TokenWrapper} from './token_wrapper'; export class EtherTokenWrapper extends ContractWrapper { private _etherTokenContractIfExists?: EtherTokenContract; private _tokenWrapper: TokenWrapper; - private _contractAddressIfExists?: string; - constructor(web3Wrapper: Web3Wrapper, networkId: number, tokenWrapper: TokenWrapper, - contractAddressIfExists?: string) { + constructor(web3Wrapper: Web3Wrapper, networkId: number, tokenWrapper: TokenWrapper) { super(web3Wrapper, networkId); this._tokenWrapper = tokenWrapper; - this._contractAddressIfExists = contractAddressIfExists; } /** * Deposit ETH into the Wrapped ETH smart contract and issues the equivalent number of wrapped ETH tokens * to the depositor address. These wrapped ETH tokens can be used in 0x trades and are redeemable for 1-to-1 * for ETH. - * @param amountInWei Amount of ETH in Wei the caller wishes to deposit. - * @param depositor The hex encoded user Ethereum address that would like to make the deposit. - * @param txOpts Transaction parameters. + * @param etherTokenAddress EtherToken address you wish to deposit into. + * @param amountInWei Amount of ETH in Wei the caller wishes to deposit. + * @param depositor The hex encoded user Ethereum address that would like to make the deposit. + * @param txOpts Transaction parameters. * @return Transaction hash. */ public async depositAsync( - amountInWei: BigNumber, depositor: string, txOpts: TransactionOpts = {}, + etherTokenAddress: string, amountInWei: BigNumber, depositor: string, txOpts: TransactionOpts = {}, ): Promise<string> { assert.isValidBaseUnitAmount('amountInWei', amountInWei); await assert.isSenderAddressAsync('depositor', depositor, this._web3Wrapper); @@ -42,7 +40,7 @@ export class EtherTokenWrapper extends ContractWrapper { const ethBalanceInWei = await this._web3Wrapper.getBalanceInWeiAsync(depositor); assert.assert(ethBalanceInWei.gte(amountInWei), ZeroExError.InsufficientEthBalanceForDeposit); - const wethContract = await this._getEtherTokenContractAsync(); + const wethContract = await this._getEtherTokenContractAsync(etherTokenAddress); const txHash = await wethContract.deposit.sendTransactionAsync({ from: depositor, value: amountInWei, @@ -54,22 +52,22 @@ export class EtherTokenWrapper extends ContractWrapper { /** * Withdraw ETH to the withdrawer's address from the wrapped ETH smart contract in exchange for the * equivalent number of wrapped ETH tokens. + * @param etherTokenAddress EtherToken address you wish to withdraw from. * @param amountInWei Amount of ETH in Wei the caller wishes to withdraw. * @param withdrawer The hex encoded user Ethereum address that would like to make the withdrawl. * @param txOpts Transaction parameters. * @return Transaction hash. */ public async withdrawAsync( - amountInWei: BigNumber, withdrawer: string, txOpts: TransactionOpts = {}, + etherTokenAddress: string, amountInWei: BigNumber, withdrawer: string, txOpts: TransactionOpts = {}, ): Promise<string> { assert.isValidBaseUnitAmount('amountInWei', amountInWei); await assert.isSenderAddressAsync('withdrawer', withdrawer, this._web3Wrapper); - const wethContractAddress = this.getContractAddress(); - const WETHBalanceInBaseUnits = await this._tokenWrapper.getBalanceAsync(wethContractAddress, withdrawer); + const WETHBalanceInBaseUnits = await this._tokenWrapper.getBalanceAsync(etherTokenAddress, withdrawer); assert.assert(WETHBalanceInBaseUnits.gte(amountInWei), ZeroExError.InsufficientWEthBalanceForWithdrawal); - const wethContract = await this._getEtherTokenContractAsync(); + const wethContract = await this._getEtherTokenContractAsync(etherTokenAddress); const txHash = await wethContract.withdraw.sendTransactionAsync(amountInWei, { from: withdrawer, gas: txOpts.gasLimit, @@ -77,25 +75,12 @@ export class EtherTokenWrapper extends ContractWrapper { }); return txHash; } - /** - * Retrieves the Wrapped Ether token contract address - * @return The Wrapped Ether token contract address - */ - public getContractAddress(): string { - const contractAddress = this._getContractAddress( - artifacts.EtherTokenArtifact, this._contractAddressIfExists, - ); - return contractAddress; - } private _invalidateContractInstance(): void { delete this._etherTokenContractIfExists; } - private async _getEtherTokenContractAsync(): Promise<EtherTokenContract> { - if (!_.isUndefined(this._etherTokenContractIfExists)) { - return this._etherTokenContractIfExists; - } + private async _getEtherTokenContractAsync(etherTokenAddress: string): Promise<EtherTokenContract> { const web3ContractInstance = await this._instantiateContractIfExistsAsync( - artifacts.EtherTokenArtifact, this._contractAddressIfExists, + artifacts.EtherTokenArtifact, etherTokenAddress, ); const contractInstance = new EtherTokenContract(web3ContractInstance, this._web3Wrapper.getContractDefaults()); this._etherTokenContractIfExists = contractInstance; diff --git a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts index 9bed40079..3ca5695c4 100644 --- a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts +++ b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts @@ -608,6 +608,12 @@ export class ExchangeWrapper extends ContractWrapper { this._unsubscribe(subscriptionToken); } /** + * Cancels all existing subscriptions + */ + public unsubscribeAll(): void { + super.unsubscribeAll(); + } + /** * Gets historical logs without creating a subscription * @param eventName The exchange contract event you would like to subscribe to. * @param subscriptionOpts Subscriptions options that let you configure the subscription. diff --git a/packages/0x.js/src/contract_wrappers/token_wrapper.ts b/packages/0x.js/src/contract_wrappers/token_wrapper.ts index d1553fa7b..eccb74871 100644 --- a/packages/0x.js/src/contract_wrappers/token_wrapper.ts +++ b/packages/0x.js/src/contract_wrappers/token_wrapper.ts @@ -282,6 +282,12 @@ export class TokenWrapper extends ContractWrapper { this._unsubscribe(subscriptionToken); } /** + * Cancels all existing subscriptions + */ + public unsubscribeAll(): void { + super.unsubscribeAll(); + } + /** * Gets historical logs without creating a subscription * @param tokenAddress An address of the token that emmited the logs. * @param eventName The token contract event you would like to subscribe to. diff --git a/packages/0x.js/src/schemas/zero_ex_config_schema.ts b/packages/0x.js/src/schemas/zero_ex_config_schema.ts index 121d81257..e0d985749 100644 --- a/packages/0x.js/src/schemas/zero_ex_config_schema.ts +++ b/packages/0x.js/src/schemas/zero_ex_config_schema.ts @@ -8,7 +8,6 @@ export const zeroExConfigSchema = { gasPrice: {$ref: '/Number'}, exchangeContractAddress: {$ref: '/Address'}, tokenRegistryContractAddress: {$ref: '/Address'}, - etherTokenContractAddress: {$ref: '/Address'}, orderWatcherConfig: { type: 'object', properties: { diff --git a/packages/0x.js/src/types.ts b/packages/0x.js/src/types.ts index f33e05bb8..fb3cfa6df 100644 --- a/packages/0x.js/src/types.ts +++ b/packages/0x.js/src/types.ts @@ -268,7 +268,6 @@ export interface OrderStateWatcherConfig { * gasPrice: Gas price to use with every transaction * exchangeContractAddress: The address of an exchange contract to use * tokenRegistryContractAddress: The address of a token registry contract to use - * etherTokenContractAddress: The address of an ether token contract to use * tokenTransferProxyContractAddress: The address of the token transfer proxy contract to use * orderWatcherConfig: All the configs related to the orderWatcher */ @@ -277,7 +276,6 @@ export interface ZeroExConfig { gasPrice?: BigNumber; exchangeContractAddress?: string; tokenRegistryContractAddress?: string; - etherTokenContractAddress?: string; tokenTransferProxyContractAddress?: string; orderWatcherConfig?: OrderStateWatcherConfig; } diff --git a/packages/0x.js/test/0x.js_test.ts b/packages/0x.js/test/0x.js_test.ts index e1ceb12c8..819ac12f9 100644 --- a/packages/0x.js/test/0x.js_test.ts +++ b/packages/0x.js/test/0x.js_test.ts @@ -82,9 +82,9 @@ describe('ZeroEx library', () => { it('should return true if the signature does pertain to the dataHex & address', async () => { const isValidSignatureLocal = ZeroEx.isValidSignature(dataHex, signature, address); expect(isValidSignatureLocal).to.be.true(); - const isValidSignatureOnContract = await (zeroEx.exchange as any) - ._isValidSignatureUsingContractCallAsync(dataHex, signature, address); - return expect(isValidSignatureOnContract).to.be.true(); + return expect( + (zeroEx.exchange as any)._isValidSignatureUsingContractCallAsync(dataHex, signature, address), + ).to.become(true); }); }); describe('#generateSalt', () => { @@ -244,14 +244,6 @@ describe('ZeroEx library', () => { const zeroExWithWrongExchangeAddress = new ZeroEx(web3.currentProvider, zeroExConfig); expect(zeroExWithWrongExchangeAddress.exchange.getContractAddress()).to.be.equal(ZeroEx.NULL_ADDRESS); }); - it('allows to specify ether token contract address', async () => { - const zeroExConfig = { - etherTokenContractAddress: ZeroEx.NULL_ADDRESS, - networkId: constants.TESTRPC_NETWORK_ID, - }; - const zeroExWithWrongEtherTokenAddress = new ZeroEx(web3.currentProvider, zeroExConfig); - expect(zeroExWithWrongEtherTokenAddress.etherToken.getContractAddress()).to.be.equal(ZeroEx.NULL_ADDRESS); - }); it('allows to specify token registry token contract address', async () => { const zeroExConfig = { tokenRegistryContractAddress: ZeroEx.NULL_ADDRESS, diff --git a/packages/0x.js/test/ether_token_wrapper_test.ts b/packages/0x.js/test/ether_token_wrapper_test.ts index e0d738f84..b5ed19308 100644 --- a/packages/0x.js/test/ether_token_wrapper_test.ts +++ b/packages/0x.js/test/ether_token_wrapper_test.ts @@ -5,6 +5,7 @@ import 'mocha'; import * as Web3 from 'web3'; import {ZeroEx, ZeroExError} from '../src'; +import {artifacts} from '../src/artifacts'; import {chaiSetup} from './utils/chai_setup'; import {constants} from './utils/constants'; @@ -38,7 +39,7 @@ describe('EtherTokenWrapper', () => { zeroEx = new ZeroEx(web3.currentProvider, zeroExConfig); userAddresses = await zeroEx.getAvailableAddressesAsync(); addressWithETH = userAddresses[0]; - wethContractAddress = zeroEx.etherToken.getContractAddress(); + wethContractAddress = (zeroEx.etherToken as any)._getContractAddress(artifacts.EtherTokenArtifact); depositWeiAmount = (zeroEx as any)._web3Wrapper.toWei(new BigNumber(5)); decimalPlaces = 7; }); @@ -55,7 +56,7 @@ describe('EtherTokenWrapper', () => { expect(preETHBalance).to.be.bignumber.gt(0); expect(preWETHBalance).to.be.bignumber.equal(0); - const txHash = await zeroEx.etherToken.depositAsync(depositWeiAmount, addressWithETH); + const txHash = await zeroEx.etherToken.depositAsync(wethContractAddress, depositWeiAmount, addressWithETH); await zeroEx.awaitTransactionMinedAsync(txHash); const postETHBalanceInWei = await (zeroEx as any)._web3Wrapper.getBalanceInWeiAsync(addressWithETH); @@ -73,7 +74,7 @@ describe('EtherTokenWrapper', () => { const overETHBalanceinWei = preETHBalance.add(extraETHBalance); return expect( - zeroEx.etherToken.depositAsync(overETHBalanceinWei, addressWithETH), + zeroEx.etherToken.depositAsync(wethContractAddress, overETHBalanceinWei, addressWithETH), ).to.be.rejectedWith(ZeroExError.InsufficientEthBalanceForDeposit); }); }); @@ -81,7 +82,7 @@ describe('EtherTokenWrapper', () => { it('should successfully withdraw ETH in return for Wrapped ETH tokens', async () => { const ETHBalanceInWei = await (zeroEx as any)._web3Wrapper.getBalanceInWeiAsync(addressWithETH); - await zeroEx.etherToken.depositAsync(depositWeiAmount, addressWithETH); + await zeroEx.etherToken.depositAsync(wethContractAddress, depositWeiAmount, addressWithETH); const expectedPreETHBalance = ETHBalanceInWei.minus(depositWeiAmount); const preETHBalance = await (zeroEx as any)._web3Wrapper.getBalanceInWeiAsync(addressWithETH); @@ -90,7 +91,7 @@ describe('EtherTokenWrapper', () => { expect(gasCost).to.be.bignumber.lte(MAX_REASONABLE_GAS_COST_IN_WEI); expect(preWETHBalance).to.be.bignumber.equal(depositWeiAmount); - const txHash = await zeroEx.etherToken.withdrawAsync(depositWeiAmount, addressWithETH); + const txHash = await zeroEx.etherToken.withdrawAsync(wethContractAddress, depositWeiAmount, addressWithETH); await zeroEx.awaitTransactionMinedAsync(txHash); const postETHBalance = await (zeroEx as any)._web3Wrapper.getBalanceInWeiAsync(addressWithETH); @@ -108,7 +109,7 @@ describe('EtherTokenWrapper', () => { const overWETHBalance = preWETHBalance.add(999999999); return expect( - zeroEx.etherToken.withdrawAsync(overWETHBalance, addressWithETH), + zeroEx.etherToken.withdrawAsync(wethContractAddress, overWETHBalance, addressWithETH), ).to.be.rejectedWith(ZeroExError.InsufficientWEthBalanceForWithdrawal); }); }); diff --git a/packages/0x.js/test/utils/web3_factory.ts b/packages/0x.js/test/utils/web3_factory.ts index 6f72cec58..7e65a4381 100644 --- a/packages/0x.js/test/utils/web3_factory.ts +++ b/packages/0x.js/test/utils/web3_factory.ts @@ -3,7 +3,6 @@ // we are not running in a browser env. // Filed issue: https://github.com/ethereum/web3.js/issues/844 (global as any).XMLHttpRequest = undefined; -import * as Web3 from 'web3'; import ProviderEngine = require('web3-provider-engine'); import RpcSubprovider = require('web3-provider-engine/subproviders/rpc'); @@ -12,6 +11,13 @@ import {FakeGasEstimateSubprovider} from './subproviders/fake_gas_estimate_subpr import {constants} from './constants'; +// HACK: web3 leaks XMLHttpRequest into the global scope and causes requests to hang +// because they are using the wrong XHR package. +// importing web3 after subproviders fixes this issue +// Filed issue: https://github.com/ethereum/web3.js/issues/844 +// tslint:disable-next-line:ordered-imports +import * as Web3 from 'web3'; + export const web3Factory = { create(hasAddresses: boolean = true): Web3 { const provider = this.getRpcProvider(hasAddresses); diff --git a/packages/contracts/test/ts/ether_token.ts b/packages/contracts/test/ts/ether_token.ts index 2f9df59a4..99630583f 100644 --- a/packages/contracts/test/ts/ether_token.ts +++ b/packages/contracts/test/ts/ether_token.ts @@ -28,7 +28,6 @@ contract('EtherToken', (accounts: string[]) => { etherTokenAddress = EtherToken.address; zeroEx = new ZeroEx(web3.currentProvider, { gasPrice, - etherTokenContractAddress: etherTokenAddress, networkId: constants.TESTRPC_NETWORK_ID, }); }); @@ -45,7 +44,7 @@ contract('EtherToken', (accounts: string[]) => { const initEthBalance = await getEthBalanceAsync(account); const ethToDeposit = initEthBalance.plus(1); - return expect(zeroEx.etherToken.depositAsync(ethToDeposit, account)) + return expect(zeroEx.etherToken.depositAsync(etherTokenAddress, ethToDeposit, account)) .to.be.rejectedWith(ZeroExError.InsufficientEthBalanceForDeposit); }); @@ -55,7 +54,7 @@ contract('EtherToken', (accounts: string[]) => { const ethToDeposit = new BigNumber(web3.toWei(1, 'ether')); - const txHash = await zeroEx.etherToken.depositAsync(ethToDeposit, account); + const txHash = await zeroEx.etherToken.depositAsync(etherTokenAddress, ethToDeposit, account); const receipt = await zeroEx.awaitTransactionMinedAsync(txHash); const ethSpentOnGas = gasPrice.times(receipt.gasUsed); @@ -72,7 +71,7 @@ contract('EtherToken', (accounts: string[]) => { const initEthTokenBalance = await zeroEx.token.getBalanceAsync(etherTokenAddress, account); const ethTokensToWithdraw = initEthTokenBalance.plus(1); - return expect(zeroEx.etherToken.withdrawAsync(ethTokensToWithdraw, account)) + return expect(zeroEx.etherToken.withdrawAsync(etherTokenAddress, ethTokensToWithdraw, account)) .to.be.rejectedWith(ZeroExError.InsufficientWEthBalanceForWithdrawal); }); @@ -81,7 +80,7 @@ contract('EtherToken', (accounts: string[]) => { const initEthBalance = await getEthBalanceAsync(account); const ethTokensToWithdraw = initEthTokenBalance; expect(ethTokensToWithdraw).to.not.be.bignumber.equal(0); - const txHash = await zeroEx.etherToken.withdrawAsync(ethTokensToWithdraw, account, { + const txHash = await zeroEx.etherToken.withdrawAsync(etherTokenAddress, ethTokensToWithdraw, account, { gasLimit: constants.MAX_ETHERTOKEN_WITHDRAW_GAS, }); const receipt = await zeroEx.awaitTransactionMinedAsync(txHash); diff --git a/packages/contracts/test/ts/ether_token_v2.ts b/packages/contracts/test/ts/ether_token_v2.ts index 61ac4cb57..480c4035b 100644 --- a/packages/contracts/test/ts/ether_token_v2.ts +++ b/packages/contracts/test/ts/ether_token_v2.ts @@ -28,7 +28,6 @@ contract('EtherTokenV2', (accounts: string[]) => { etherTokenAddress = etherToken.address; zeroEx = new ZeroEx(web3.currentProvider, { gasPrice, - etherTokenContractAddress: etherTokenAddress, networkId: constants.TESTRPC_NETWORK_ID, }); }); @@ -45,7 +44,7 @@ contract('EtherTokenV2', (accounts: string[]) => { const initEthBalance = await getEthBalanceAsync(account); const ethToDeposit = initEthBalance.plus(1); - return expect(zeroEx.etherToken.depositAsync(ethToDeposit, account)) + return expect(zeroEx.etherToken.depositAsync(etherTokenAddress, ethToDeposit, account)) .to.be.rejectedWith(ZeroExError.InsufficientEthBalanceForDeposit); }); @@ -55,7 +54,7 @@ contract('EtherTokenV2', (accounts: string[]) => { const ethToDeposit = new BigNumber(web3.toWei(1, 'ether')); - const txHash = await zeroEx.etherToken.depositAsync(ethToDeposit, account); + const txHash = await zeroEx.etherToken.depositAsync(etherTokenAddress, ethToDeposit, account); const receipt = await zeroEx.awaitTransactionMinedAsync(txHash); const ethSpentOnGas = gasPrice.times(receipt.gasUsed); @@ -69,7 +68,7 @@ contract('EtherTokenV2', (accounts: string[]) => { it('should log 1 event with correct arguments', async () => { const ethToDeposit = new BigNumber(web3.toWei(1, 'ether')); - const txHash = await zeroEx.etherToken.depositAsync(ethToDeposit, account); + const txHash = await zeroEx.etherToken.depositAsync(etherTokenAddress, ethToDeposit, account); const receipt = await zeroEx.awaitTransactionMinedAsync(txHash); const logs = receipt.logs; @@ -88,14 +87,14 @@ contract('EtherTokenV2', (accounts: string[]) => { describe('withdraw', () => { beforeEach(async () => { const ethToDeposit = new BigNumber(web3.toWei(1, 'ether')); - await zeroEx.etherToken.depositAsync(ethToDeposit, account); + await zeroEx.etherToken.depositAsync(etherTokenAddress, ethToDeposit, account); }); it('should throw if caller attempts to withdraw greater than caller balance', async () => { const initEthTokenBalance = await zeroEx.token.getBalanceAsync(etherTokenAddress, account); const ethTokensToWithdraw = initEthTokenBalance.plus(1); - return expect(zeroEx.etherToken.withdrawAsync(ethTokensToWithdraw, account)) + return expect(zeroEx.etherToken.withdrawAsync(etherTokenAddress, ethTokensToWithdraw, account)) .to.be.rejectedWith(ZeroExError.InsufficientWEthBalanceForWithdrawal); }); @@ -104,7 +103,7 @@ contract('EtherTokenV2', (accounts: string[]) => { const initEthBalance = await getEthBalanceAsync(account); const ethTokensToWithdraw = initEthTokenBalance; expect(ethTokensToWithdraw).to.not.be.bignumber.equal(0); - const txHash = await zeroEx.etherToken.withdrawAsync(ethTokensToWithdraw, account, { + const txHash = await zeroEx.etherToken.withdrawAsync(etherTokenAddress, ethTokensToWithdraw, account, { gasLimit: constants.MAX_ETHERTOKEN_WITHDRAW_GAS, }); const receipt = await zeroEx.awaitTransactionMinedAsync(txHash); @@ -122,7 +121,7 @@ contract('EtherTokenV2', (accounts: string[]) => { const initEthTokenBalance = await zeroEx.token.getBalanceAsync(etherTokenAddress, account); const ethTokensToWithdraw = initEthTokenBalance; expect(ethTokensToWithdraw).to.not.be.bignumber.equal(0); - const txHash = await zeroEx.etherToken.withdrawAsync(ethTokensToWithdraw, account, { + const txHash = await zeroEx.etherToken.withdrawAsync(etherTokenAddress, ethTokensToWithdraw, account, { gasLimit: constants.MAX_ETHERTOKEN_WITHDRAW_GAS, }); const receipt = await zeroEx.awaitTransactionMinedAsync(txHash); diff --git a/packages/kovan-faucets/src/ts/handler.ts b/packages/kovan-faucets/src/ts/handler.ts index 1c6866a1c..d31d1478d 100644 --- a/packages/kovan-faucets/src/ts/handler.ts +++ b/packages/kovan-faucets/src/ts/handler.ts @@ -13,7 +13,7 @@ import {ZRXRequestQueue} from './zrx_request_queue'; // HACK: web3 leaks XMLHttpRequest into the global scope and causes requests to hang // because they are using the wrong XHR package. -// Issue: https://github.com/trufflesuite/truffle-contract/issues/14 +// Filed issue: https://github.com/ethereum/web3.js/issues/844 // tslint:disable-next-line:ordered-imports import * as Web3 from 'web3'; diff --git a/packages/kovan-faucets/src/ts/request_queue.ts b/packages/kovan-faucets/src/ts/request_queue.ts index 0b5e66ae9..1ad7e68be 100644 --- a/packages/kovan-faucets/src/ts/request_queue.ts +++ b/packages/kovan-faucets/src/ts/request_queue.ts @@ -3,7 +3,7 @@ import * as timers from 'timers'; // HACK: web3 leaks XMLHttpRequest into the global scope and causes requests to hang // because they are using the wrong XHR package. -// Issue: https://github.com/trufflesuite/truffle-contract/issues/14 +// Filed issue: https://github.com/ethereum/web3.js/issues/844 // tslint:disable-next-line:ordered-imports import * as Web3 from 'web3'; diff --git a/packages/kovan-faucets/src/ts/zrx_request_queue.ts b/packages/kovan-faucets/src/ts/zrx_request_queue.ts index 3b505690f..717adc3c4 100644 --- a/packages/kovan-faucets/src/ts/zrx_request_queue.ts +++ b/packages/kovan-faucets/src/ts/zrx_request_queue.ts @@ -9,7 +9,7 @@ import {utils} from './utils'; // HACK: web3 leaks XMLHttpRequest into the global scope and causes requests to hang // because they are using the wrong XHR package. -// Issue: https://github.com/trufflesuite/truffle-contract/issues/14 +// Filed issue: https://github.com/ethereum/web3.js/issues/844 // tslint:disable-next-line:ordered-imports import * as Web3 from 'web3'; diff --git a/packages/subproviders/CHANGELOG.md b/packages/subproviders/CHANGELOG.md index b4cce6be0..358fcfa15 100644 --- a/packages/subproviders/CHANGELOG.md +++ b/packages/subproviders/CHANGELOG.md @@ -1,4 +1,5 @@ # CHANGELOG -vx.x.x +v0.x.x - _TBD, 2017_ ------------------------ + * Improve the performance of address fetching (#271) diff --git a/packages/subproviders/package.json b/packages/subproviders/package.json index 8a222457d..56fc9bf8b 100644 --- a/packages/subproviders/package.json +++ b/packages/subproviders/package.json @@ -23,6 +23,7 @@ "es6-promisify": "^5.0.0", "ethereumjs-tx": "^1.3.3", "ethereumjs-util": "^5.1.1", + "hdkey": "^0.7.1", "ledgerco": "0xProject/ledger-node-js-api", "lodash": "^4.17.4", "semaphore-async-await": "^1.5.1", diff --git a/packages/subproviders/src/globals.d.ts b/packages/subproviders/src/globals.d.ts index adef23806..3800c970f 100644 --- a/packages/subproviders/src/globals.d.ts +++ b/packages/subproviders/src/globals.d.ts @@ -48,7 +48,7 @@ declare module 'ledgerco' { public comm: comm; constructor(comm: comm); public getAddress_async(path: string, display?: boolean, chaincode?: boolean): - Promise<{publicKey: string; address: string}>; + Promise<{publicKey: string; address: string; chainCode: string}>; public signTransaction_async(path: string, rawTxHex: string): Promise<ECSignatureString>; public getAppConfiguration_async(): Promise<{ arbitraryDataEnabled: number; version: string }>; public signPersonalMessage_async(path: string, messageHex: string): Promise<ECSignature>; @@ -91,3 +91,14 @@ declare module 'web3-provider-engine' { } export = Web3ProviderEngine; } + +// hdkey declarations +declare module 'hdkey' { + class HDNode { + public publicKey: Buffer; + public chainCode: Buffer; + public constructor(); + public derive(path: string): HDNode; + } + export = HDNode; +} diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 9ffc105a4..f9922fdda 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -2,6 +2,7 @@ import {assert} from '@0xproject/assert'; import {addressUtils} from '@0xproject/utils'; import EthereumTx = require('ethereumjs-tx'); import ethUtil = require('ethereumjs-util'); +import HDNode = require('hdkey'); import * as _ from 'lodash'; import Semaphore from 'semaphore-async-await'; import Web3 = require('web3'); @@ -20,7 +21,7 @@ import {Subprovider} from './subprovider'; const DEFAULT_DERIVATION_PATH = `44'/60'/0'`; const NUM_ADDRESSES_TO_FETCH = 10; const ASK_FOR_ON_DEVICE_CONFIRMATION = false; -const SHOULD_GET_CHAIN_CODE = false; +const SHOULD_GET_CHAIN_CODE = true; export class LedgerSubprovider extends Subprovider { private _nonceLock: Semaphore; @@ -127,21 +128,30 @@ export class LedgerSubprovider extends Subprovider { public async getAccountsAsync(): Promise<string[]> { this._ledgerClientIfExists = await this.createLedgerClientAsync(); - // TODO: replace with generating addresses without hitting Ledger + let ledgerResponse; + try { + ledgerResponse = await this._ledgerClientIfExists.getAddress_async( + this._derivationPath, this._shouldAlwaysAskForConfirmation, SHOULD_GET_CHAIN_CODE, + ); + } finally { + await this.destoryLedgerClientAsync(); + } + + const hdKey = new HDNode(); + hdKey.publicKey = new Buffer(ledgerResponse.publicKey, 'hex'); + hdKey.chainCode = new Buffer(ledgerResponse.chainCode, 'hex'); + const accounts = []; for (let i = 0; i < NUM_ADDRESSES_TO_FETCH; i++) { - try { - const derivationPath = `${this._derivationPath}/${i + this._derivationPathIndex}`; - const result = await this._ledgerClientIfExists.getAddress_async( - derivationPath, this._shouldAlwaysAskForConfirmation, SHOULD_GET_CHAIN_CODE, - ); - accounts.push(result.address.toLowerCase()); - } catch (err) { - await this.destoryLedgerClientAsync(); - throw err; - } + 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()); } - await this.destoryLedgerClientAsync(); return accounts; } public async signTransactionAsync(txParams: PartialTxParams): Promise<string> { diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts index 1e7d3eab0..c5ccf1fda 100644 --- a/packages/subproviders/src/types.ts +++ b/packages/subproviders/src/types.ts @@ -10,8 +10,10 @@ export interface LedgerCommunicationClient { * NodeJs and Browser communication are supported. */ export interface LedgerEthereumClient { + // shouldGetChainCode is defined as `true` instead of `boolean` because other types rely on the assumption + // that we get back the chain code and we don't have dependent types to express it properly getAddress_async: (derivationPath: string, askForDeviceConfirmation: boolean, - shouldGetChainCode: boolean) => Promise<LedgerGetAddressResult>; + shouldGetChainCode: true) => Promise<LedgerGetAddressResult>; signPersonalMessage_async: (derivationPath: string, messageHex: string) => Promise<ECSignature>; signTransaction_async: (derivationPath: string, txHex: string) => Promise<ECSignatureString>; comm: LedgerCommunicationClient; @@ -63,6 +65,8 @@ export interface SignatureData { export interface LedgerGetAddressResult { address: string; + publicKey: string; + chainCode: string; } export interface LedgerWalletSubprovider { diff --git a/packages/subproviders/test/unit/ledger_subprovider_test.ts b/packages/subproviders/test/unit/ledger_subprovider_test.ts index bd4d93325..237090051 100644 --- a/packages/subproviders/test/unit/ledger_subprovider_test.ts +++ b/packages/subproviders/test/unit/ledger_subprovider_test.ts @@ -18,7 +18,7 @@ import {reportCallbackErrors} from '../utils/report_callback_errors'; chaiSetup.configure(); const expect = chai.expect; -const FAKE_ADDRESS = '0x9901c66f2d4b95f7074b553da78084d708beca70'; +const FAKE_ADDRESS = '0xb088a3bc93f71b4de97b9de773e9647645983688'; describe('LedgerSubprovider', () => { const networkId: number = 42; @@ -28,8 +28,14 @@ describe('LedgerSubprovider', () => { // tslint:disable:no-object-literal-type-assertion const ledgerEthClient = { getAddress_async: async () => { + // tslint:disable-next-line:max-line-length + const publicKey = '04f428290f4c5ed6a198f71b8205f488141dbb3f0840c923bbfa798ecbee6370986c03b5575d94d506772fb48a6a44e345e4ebd4f028a6f609c44b655d6d3e71a1'; + const chainCode = 'ac055a5537c0c7e9e02d14a197cad6b857836da2a12043b46912a37d959b5ae8'; + const address = '0xBa388BA5e5EEF2c6cE42d831c2B3A28D3c99bdB1'; return { - address: FAKE_ADDRESS, + publicKey, + address, + chainCode, }; }, signPersonalMessage_async: async () => { diff --git a/packages/website/ts/blockchain.ts b/packages/website/ts/blockchain.ts index 6877a301a..a42b19cff 100644 --- a/packages/website/ts/blockchain.ts +++ b/packages/website/ts/blockchain.ts @@ -388,18 +388,18 @@ export class Blockchain { const balance = await this.web3Wrapper.getBalanceInEthAsync(owner); return balance; } - public async convertEthToWrappedEthTokensAsync(amount: BigNumber): Promise<void> { + public async convertEthToWrappedEthTokensAsync(etherTokenAddress: string, amount: BigNumber): Promise<void> { utils.assert(!_.isUndefined(this.zeroEx), 'ZeroEx must be instantiated.'); utils.assert(this.doesUserAddressExist(), BlockchainCallErrs.USER_HAS_NO_ASSOCIATED_ADDRESSES); - const txHash = await this.zeroEx.etherToken.depositAsync(amount, this.userAddress); + const txHash = await this.zeroEx.etherToken.depositAsync(etherTokenAddress, amount, this.userAddress); await this.showEtherScanLinkAndAwaitTransactionMinedAsync(txHash); } - public async convertWrappedEthTokensToEthAsync(amount: BigNumber): Promise<void> { + public async convertWrappedEthTokensToEthAsync(etherTokenAddress: string, amount: BigNumber): Promise<void> { utils.assert(!_.isUndefined(this.zeroEx), 'ZeroEx must be instantiated.'); utils.assert(this.doesUserAddressExist(), BlockchainCallErrs.USER_HAS_NO_ASSOCIATED_ADDRESSES); - const txHash = await this.zeroEx.etherToken.withdrawAsync(amount, this.userAddress); + const txHash = await this.zeroEx.etherToken.withdrawAsync(etherTokenAddress, amount, this.userAddress); await this.showEtherScanLinkAndAwaitTransactionMinedAsync(txHash); } public async doesContractExistAtAddressAsync(address: string) { diff --git a/packages/website/ts/components/eth_weth_conversion_button.tsx b/packages/website/ts/components/eth_weth_conversion_button.tsx index b017de27b..c8a279de9 100644 --- a/packages/website/ts/components/eth_weth_conversion_button.tsx +++ b/packages/website/ts/components/eth_weth_conversion_button.tsx @@ -88,12 +88,12 @@ export class EthWethConversionButton extends let balance = tokenState.balance; try { if (direction === Side.deposit) { - await this.props.blockchain.convertEthToWrappedEthTokensAsync(value); + await this.props.blockchain.convertEthToWrappedEthTokensAsync(token.address, value); const ethAmount = ZeroEx.toUnitAmount(value, constants.ETH_DECIMAL_PLACES); this.props.dispatcher.showFlashMessage(`Successfully wrapped ${ethAmount.toString()} ETH to WETH`); balance = balance.plus(value); } else { - await this.props.blockchain.convertWrappedEthTokensToEthAsync(value); + await this.props.blockchain.convertWrappedEthTokensToEthAsync(token.address, value); const tokenAmount = ZeroEx.toUnitAmount(value, token.decimals); this.props.dispatcher.showFlashMessage(`Successfully unwrapped ${tokenAmount.toString()} WETH to ETH`); balance = balance.minus(value); @@ -4199,7 +4199,7 @@ hawk@~6.0.2: hoek "4.x.x" sntp "2.x.x" -hdkey@^0.7.0: +hdkey@^0.7.0, hdkey@^0.7.1: version "0.7.1" resolved "https://registry.yarnpkg.com/hdkey/-/hdkey-0.7.1.tgz#caee4be81aa77921e909b8d228dd0f29acaee632" dependencies: |