From 7a21c6854bec32f9a36e8ca3de14a815e9c9fa7d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 23 Nov 2017 11:14:21 -0600 Subject: Add option config for gasPrice and gasLimit for every transaction sending method --- packages/0x.js/src/contract.ts | 22 +++- .../src/contract_wrappers/ether_token_wrapper.ts | 14 ++- .../src/contract_wrappers/exchange_wrapper.ts | 139 +++++---------------- .../0x.js/src/contract_wrappers/token_wrapper.ts | 41 +++--- packages/0x.js/src/types.ts | 17 ++- 5 files changed, 95 insertions(+), 138 deletions(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/src/contract.ts b/packages/0x.js/src/contract.ts index e9c49c9f1..347f47aa0 100644 --- a/packages/0x.js/src/contract.ts +++ b/packages/0x.js/src/contract.ts @@ -34,9 +34,10 @@ export class Contract implements Web3.ContractInstance { } else { const cbStyleFunction = this.contract[functionAbi.name]; const cbStyleEstimateGasFunction = this.contract[functionAbi.name].estimateGas; + const estimateGasAsync = promisify(cbStyleEstimateGasFunction, this.contract); this[functionAbi.name] = { - estimateGasAsync: promisify(cbStyleEstimateGasFunction, this.contract), - sendTransactionAsync: this.promisifyWithDefaultParams(cbStyleFunction), + estimateGasAsync, + sendTransactionAsync: this.promisifyWithDefaultParams(cbStyleFunction, estimateGasAsync), }; } }); @@ -47,18 +48,29 @@ export class Contract implements Web3.ContractInstance { this[eventAbi.name] = this.contract[eventAbi.name]; }); } - private promisifyWithDefaultParams(fn: (...args: any[]) => void): (...args: any[]) => Promise { + private promisifyWithDefaultParams( + web3CbStyleFunction: (...args: any[]) => void, + estimateGasAsync: (...args: any[]) => Promise, + ): (...args: any[]) => Promise { const promisifiedWithDefaultParams = async (...args: any[]) => { - const promise = new Promise((resolve, reject) => { + const promise = new Promise(async (resolve, reject) => { const lastArg = args[args.length - 1]; let txData: Partial = {}; if (this.isTxData(lastArg)) { txData = args.pop(); } + // Gas amounts priorities: + // 1 - method level + // 2 - Library defaults + // 3 - estimate txData = { ...this.defaults, ...txData, }; + if (_.isUndefined(txData.gas)) { + const estimatedGas = await estimateGasAsync.apply(this.contract, [...args, txData]); + txData.gas = estimatedGas; + } const callback = (err: Error, data: any) => { if (_.isNull(err)) { resolve(data); @@ -68,7 +80,7 @@ export class Contract implements Web3.ContractInstance { }; args.push(txData); args.push(callback); - fn.apply(this.contract, args); + web3CbStyleFunction.apply(this.contract, args); }); return promise; }; 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 7a3f2bc52..65f1cda24 100644 --- a/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts +++ b/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts @@ -2,7 +2,7 @@ import BigNumber from 'bignumber.js'; import * as _ from 'lodash'; import {artifacts} from '../artifacts'; -import {EtherTokenContract, ZeroExError} from '../types'; +import {EtherTokenContract, TransactionOpts, ZeroExError} from '../types'; import {assert} from '../utils/assert'; import {Web3Wrapper} from '../web3_wrapper'; @@ -27,10 +27,11 @@ export class EtherTokenWrapper extends ContractWrapper { * 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 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): Promise { + public async depositAsync(amountInWei: BigNumber, depositor: string, txOpts: TransactionOpts): Promise { assert.isValidBaseUnitAmount('amountInWei', amountInWei); await assert.isSenderAddressAsync('depositor', depositor, this._web3Wrapper); @@ -41,6 +42,8 @@ export class EtherTokenWrapper extends ContractWrapper { const txHash = await wethContract.deposit.sendTransactionAsync({ from: depositor, value: amountInWei, + gas: txOpts.gasLimit, + gasPrice: txOpts.gasPrice, }); return txHash; } @@ -49,9 +52,10 @@ export class EtherTokenWrapper extends ContractWrapper { * equivalent number of wrapped ETH tokens. * @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): Promise { + public async withdrawAsync(amountInWei: BigNumber, withdrawer: string, txOpts: TransactionOpts): Promise { assert.isValidBaseUnitAmount('amountInWei', amountInWei); await assert.isSenderAddressAsync('withdrawer', withdrawer, this._web3Wrapper); @@ -62,6 +66,8 @@ export class EtherTokenWrapper extends ContractWrapper { const wethContract = await this._getEtherTokenContractAsync(); const txHash = await wethContract.withdraw.sendTransactionAsync(amountInWei, { from: withdrawer, + gas: txOpts.gasLimit, + gasPrice: txOpts.gasPrice, }); return txHash; } diff --git a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts index 91b41c4a4..76a537b45 100644 --- a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts +++ b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts @@ -169,16 +169,14 @@ export class ExchangeWrapper extends ContractWrapper { public async fillOrderAsync(signedOrder: SignedOrder, fillTakerTokenAmount: BigNumber, shouldThrowOnInsufficientBalanceOrAllowance: boolean, takerAddress: string, - orderTransactionOpts?: OrderTransactionOpts): Promise { + orderTransactionOpts: OrderTransactionOpts = {}): Promise { assert.doesConformToSchema('signedOrder', signedOrder, schemas.signedOrderSchema); assert.isValidBaseUnitAmount('fillTakerTokenAmount', fillTakerTokenAmount); assert.isBoolean('shouldThrowOnInsufficientBalanceOrAllowance', shouldThrowOnInsufficientBalanceOrAllowance); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? - SHOULD_VALIDATE_BY_DEFAULT : - orderTransactionOpts.shouldValidate; + const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; if (shouldValidate) { const zrxTokenAddress = this.getZRXTokenAddress(); const exchangeTradeEmulator = new ExchangeTransferSimulator(this._tokenWrapper, BlockParamLiteral.Latest); @@ -188,18 +186,6 @@ export class ExchangeWrapper extends ContractWrapper { const [orderAddresses, orderValues] = ExchangeWrapper._getOrderAddressesAndValues(signedOrder); - const gas = await exchangeInstance.fillOrder.estimateGasAsync( - orderAddresses, - orderValues, - fillTakerTokenAmount, - shouldThrowOnInsufficientBalanceOrAllowance, - signedOrder.ecSignature.v, - signedOrder.ecSignature.r, - signedOrder.ecSignature.s, - { - from: takerAddress, - }, - ); const txHash: string = await exchangeInstance.fillOrder.sendTransactionAsync( orderAddresses, orderValues, @@ -210,7 +196,8 @@ export class ExchangeWrapper extends ContractWrapper { signedOrder.ecSignature.s, { from: takerAddress, - gas, + gas: orderTransactionOpts.gasLimit, + gasPrice: orderTransactionOpts.gasPrice, }, ); return txHash; @@ -236,7 +223,7 @@ export class ExchangeWrapper extends ContractWrapper { public async fillOrdersUpToAsync(signedOrders: SignedOrder[], fillTakerTokenAmount: BigNumber, shouldThrowOnInsufficientBalanceOrAllowance: boolean, takerAddress: string, - orderTransactionOpts?: OrderTransactionOpts): Promise { + orderTransactionOpts: OrderTransactionOpts = {}): Promise { assert.doesConformToSchema('signedOrders', signedOrders, schemas.signedOrdersSchema); const takerTokenAddresses = _.map(signedOrders, signedOrder => signedOrder.takerTokenAddress); assert.hasAtMostOneUniqueValue(takerTokenAddresses, @@ -248,9 +235,7 @@ export class ExchangeWrapper extends ContractWrapper { assert.isBoolean('shouldThrowOnInsufficientBalanceOrAllowance', shouldThrowOnInsufficientBalanceOrAllowance); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? - SHOULD_VALIDATE_BY_DEFAULT : - orderTransactionOpts.shouldValidate; + const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; if (shouldValidate) { const zrxTokenAddress = this.getZRXTokenAddress(); const exchangeTradeEmulator = new ExchangeTransferSimulator(this._tokenWrapper, BlockParamLiteral.Latest); @@ -278,18 +263,6 @@ export class ExchangeWrapper extends ContractWrapper { ); const exchangeInstance = await this._getExchangeContractAsync(); - const gas = await exchangeInstance.fillOrdersUpTo.estimateGasAsync( - orderAddressesArray, - orderValuesArray, - fillTakerTokenAmount, - shouldThrowOnInsufficientBalanceOrAllowance, - vArray, - rArray, - sArray, - { - from: takerAddress, - }, - ); const txHash = await exchangeInstance.fillOrdersUpTo.sendTransactionAsync( orderAddressesArray, orderValuesArray, @@ -300,7 +273,8 @@ export class ExchangeWrapper extends ContractWrapper { sArray, { from: takerAddress, - gas, + gas: orderTransactionOpts.gasLimit, + gasPrice: orderTransactionOpts.gasPrice, }, ); return txHash; @@ -328,7 +302,7 @@ export class ExchangeWrapper extends ContractWrapper { public async batchFillOrdersAsync(orderFillRequests: OrderFillRequest[], shouldThrowOnInsufficientBalanceOrAllowance: boolean, takerAddress: string, - orderTransactionOpts?: OrderTransactionOpts): Promise { + orderTransactionOpts: OrderTransactionOpts = {}): Promise { assert.doesConformToSchema('orderFillRequests', orderFillRequests, schemas.orderFillRequestsSchema); const exchangeContractAddresses = _.map( orderFillRequests, @@ -338,9 +312,7 @@ export class ExchangeWrapper extends ContractWrapper { ExchangeContractErrs.BatchOrdersMustHaveSameExchangeAddress); assert.isBoolean('shouldThrowOnInsufficientBalanceOrAllowance', shouldThrowOnInsufficientBalanceOrAllowance); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? - SHOULD_VALIDATE_BY_DEFAULT : - orderTransactionOpts.shouldValidate; + const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; if (shouldValidate) { const zrxTokenAddress = this.getZRXTokenAddress(); const exchangeTradeEmulator = new ExchangeTransferSimulator(this._tokenWrapper, BlockParamLiteral.Latest); @@ -370,18 +342,6 @@ export class ExchangeWrapper extends ContractWrapper { ); const exchangeInstance = await this._getExchangeContractAsync(); - const gas = await exchangeInstance.batchFillOrders.estimateGasAsync( - orderAddressesArray, - orderValuesArray, - fillTakerTokenAmounts, - shouldThrowOnInsufficientBalanceOrAllowance, - vArray, - rArray, - sArray, - { - from: takerAddress, - }, - ); const txHash = await exchangeInstance.batchFillOrders.sendTransactionAsync( orderAddressesArray, orderValuesArray, @@ -392,7 +352,8 @@ export class ExchangeWrapper extends ContractWrapper { sArray, { from: takerAddress, - gas, + gas: orderTransactionOpts.gasLimit, + gasPrice: orderTransactionOpts.gasPrice, }, ); return txHash; @@ -411,16 +372,14 @@ export class ExchangeWrapper extends ContractWrapper { @decorators.contractCallErrorHandler public async fillOrKillOrderAsync(signedOrder: SignedOrder, fillTakerTokenAmount: BigNumber, takerAddress: string, - orderTransactionOpts?: OrderTransactionOpts): Promise { + orderTransactionOpts: OrderTransactionOpts = {}): Promise { assert.doesConformToSchema('signedOrder', signedOrder, schemas.signedOrderSchema); assert.isValidBaseUnitAmount('fillTakerTokenAmount', fillTakerTokenAmount); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? - SHOULD_VALIDATE_BY_DEFAULT : - orderTransactionOpts.shouldValidate; + const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; if (shouldValidate) { const zrxTokenAddress = this.getZRXTokenAddress(); const exchangeTradeEmulator = new ExchangeTransferSimulator(this._tokenWrapper, BlockParamLiteral.Latest); @@ -429,18 +388,6 @@ export class ExchangeWrapper extends ContractWrapper { } const [orderAddresses, orderValues] = ExchangeWrapper._getOrderAddressesAndValues(signedOrder); - - const gas = await exchangeInstance.fillOrKillOrder.estimateGasAsync( - orderAddresses, - orderValues, - fillTakerTokenAmount, - signedOrder.ecSignature.v, - signedOrder.ecSignature.r, - signedOrder.ecSignature.s, - { - from: takerAddress, - }, - ); const txHash = await exchangeInstance.fillOrKillOrder.sendTransactionAsync( orderAddresses, orderValues, @@ -450,7 +397,8 @@ export class ExchangeWrapper extends ContractWrapper { signedOrder.ecSignature.s, { from: takerAddress, - gas, + gas: orderTransactionOpts.gasLimit, + gasPrice: orderTransactionOpts.gasPrice, }, ); return txHash; @@ -467,7 +415,7 @@ export class ExchangeWrapper extends ContractWrapper { @decorators.contractCallErrorHandler public async batchFillOrKillAsync(orderFillRequests: OrderFillRequest[], takerAddress: string, - orderTransactionOpts?: OrderTransactionOpts): Promise { + orderTransactionOpts: OrderTransactionOpts = {}): Promise { assert.doesConformToSchema('orderFillRequests', orderFillRequests, schemas.orderFillRequestsSchema); const exchangeContractAddresses = _.map( @@ -482,9 +430,7 @@ export class ExchangeWrapper extends ContractWrapper { } const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? - SHOULD_VALIDATE_BY_DEFAULT : - orderTransactionOpts.shouldValidate; + const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; if (shouldValidate) { const zrxTokenAddress = this.getZRXTokenAddress(); const exchangeTradeEmulator = new ExchangeTransferSimulator(this._tokenWrapper, BlockParamLiteral.Latest); @@ -509,18 +455,6 @@ export class ExchangeWrapper extends ContractWrapper { // We use _.unzip because _.unzip doesn't type check if values have different types :'( const [orderAddresses, orderValues, fillTakerTokenAmounts, vParams, rParams, sParams] = _.unzip(orderAddressesValuesAndTakerTokenFillAmounts); - - const gas = await exchangeInstance.batchFillOrKillOrders.estimateGasAsync( - orderAddresses, - orderValues, - fillTakerTokenAmounts, - vParams, - rParams, - sParams, - { - from: takerAddress, - }, - ); const txHash = await exchangeInstance.batchFillOrKillOrders.sendTransactionAsync( orderAddresses, orderValues, @@ -530,7 +464,8 @@ export class ExchangeWrapper extends ContractWrapper { sParams, { from: takerAddress, - gas, + gas: orderTransactionOpts.gasLimit, + gasPrice: orderTransactionOpts.gasPrice, }, ); return txHash; @@ -546,16 +481,14 @@ export class ExchangeWrapper extends ContractWrapper { @decorators.contractCallErrorHandler public async cancelOrderAsync(order: Order|SignedOrder, cancelTakerTokenAmount: BigNumber, - orderTransactionOpts?: OrderTransactionOpts): Promise { + orderTransactionOpts: OrderTransactionOpts = {}): Promise { assert.doesConformToSchema('order', order, schemas.orderSchema); assert.isValidBaseUnitAmount('takerTokenCancelAmount', cancelTakerTokenAmount); await assert.isSenderAddressAsync('order.maker', order.maker, this._web3Wrapper); const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? - SHOULD_VALIDATE_BY_DEFAULT : - orderTransactionOpts.shouldValidate; + const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; if (shouldValidate) { const orderHash = utils.getOrderHashHex(order); const unavailableTakerTokenAmount = await this.getUnavailableTakerAmountAsync(orderHash); @@ -564,21 +497,14 @@ export class ExchangeWrapper extends ContractWrapper { } const [orderAddresses, orderValues] = ExchangeWrapper._getOrderAddressesAndValues(order); - const gas = await exchangeInstance.cancelOrder.estimateGasAsync( - orderAddresses, - orderValues, - cancelTakerTokenAmount, - { - from: order.maker, - }, - ); const txHash = await exchangeInstance.cancelOrder.sendTransactionAsync( orderAddresses, orderValues, cancelTakerTokenAmount, { from: order.maker, - gas, + gas: orderTransactionOpts.gasLimit, + gasPrice: orderTransactionOpts.gasPrice, }, ); return txHash; @@ -593,7 +519,7 @@ export class ExchangeWrapper extends ContractWrapper { */ @decorators.contractCallErrorHandler public async batchCancelOrdersAsync(orderCancellationRequests: OrderCancellationRequest[], - orderTransactionOpts?: OrderTransactionOpts): Promise { + orderTransactionOpts: OrderTransactionOpts = {}): Promise { assert.doesConformToSchema('orderCancellationRequests', orderCancellationRequests, schemas.orderCancellationRequestsSchema); const exchangeContractAddresses = _.map( @@ -606,9 +532,7 @@ export class ExchangeWrapper extends ContractWrapper { assert.hasAtMostOneUniqueValue(makers, ExchangeContractErrs.MultipleMakersInSingleCancelBatchDisallowed); const maker = makers[0]; await assert.isSenderAddressAsync('maker', maker, this._web3Wrapper); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? - SHOULD_VALIDATE_BY_DEFAULT : - orderTransactionOpts.shouldValidate; + const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; if (shouldValidate) { for (const orderCancellationRequest of orderCancellationRequests) { const orderHash = utils.getOrderHashHex(orderCancellationRequest.order); @@ -633,21 +557,14 @@ export class ExchangeWrapper extends ContractWrapper { // We use _.unzip because _.unzip doesn't type check if values have different types :'( const [orderAddresses, orderValues, cancelTakerTokenAmounts] = _.unzip(orderAddressesValuesAndTakerTokenCancelAmounts); - const gas = await exchangeInstance.batchCancelOrders.estimateGasAsync( - orderAddresses, - orderValues, - cancelTakerTokenAmounts, - { - from: maker, - }, - ); const txHash = await exchangeInstance.batchCancelOrders.sendTransactionAsync( orderAddresses, orderValues, cancelTakerTokenAmounts, { from: maker, - gas, + gas: orderTransactionOpts.gasLimit, + gasPrice: orderTransactionOpts.gasPrice, }, ); return txHash; diff --git a/packages/0x.js/src/contract_wrappers/token_wrapper.ts b/packages/0x.js/src/contract_wrappers/token_wrapper.ts index 5c6cfeaed..4a1dfcf8d 100644 --- a/packages/0x.js/src/contract_wrappers/token_wrapper.ts +++ b/packages/0x.js/src/contract_wrappers/token_wrapper.ts @@ -12,6 +12,7 @@ import { TokenContract, TokenContractEventArgs, TokenEvents, + TransactionOpts, ZeroExError, } from '../types'; import {AbiDecoder} from '../utils/abi_decoder'; @@ -66,24 +67,21 @@ export class TokenWrapper extends ContractWrapper { * for spenderAddress. * @param spenderAddress The hex encoded user Ethereum address who will be able to spend the set allowance. * @param amountInBaseUnits The allowance amount you would like to set. + * @param txOpts Transaction parameters. * @return Transaction hash. */ public async setAllowanceAsync(tokenAddress: string, ownerAddress: string, spenderAddress: string, - amountInBaseUnits: BigNumber): Promise { + amountInBaseUnits: BigNumber, txOpts: TransactionOpts = {}): Promise { await assert.isSenderAddressAsync('ownerAddress', ownerAddress, this._web3Wrapper); assert.isETHAddressHex('spenderAddress', spenderAddress); assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isValidBaseUnitAmount('amountInBaseUnits', amountInBaseUnits); const tokenContract = await this._getTokenContractAsync(tokenAddress); - // Hack: for some reason default estimated gas amount causes `base fee exceeds gas limit` exception - // on testrpc. Probably related to https://github.com/ethereumjs/testrpc/issues/294 - // TODO: Debug issue in testrpc and submit a PR, then remove this hack - const networkId = this._web3Wrapper.getNetworkId(); - const gas = networkId === constants.TESTRPC_NETWORK_ID ? ALLOWANCE_TO_ZERO_GAS_AMOUNT : undefined; const txHash = await tokenContract.approve.sendTransactionAsync(spenderAddress, amountInBaseUnits, { from: ownerAddress, - gas, + gas: txOpts.gasLimit, + gasPrice: txOpts.gasPrice, }); return txHash; } @@ -96,12 +94,13 @@ export class TokenWrapper extends ContractWrapper { * @param ownerAddress The hex encoded user Ethereum address who would like to set an allowance * for spenderAddress. * @param spenderAddress The hex encoded user Ethereum address who will be able to spend the set allowance. + * @param txOpts Transaction parameters. * @return Transaction hash. */ public async setUnlimitedAllowanceAsync(tokenAddress: string, ownerAddress: string, - spenderAddress: string): Promise { + spenderAddress: string, txOpts: TransactionOpts = {}): Promise { const txHash = await this.setAllowanceAsync( - tokenAddress, ownerAddress, spenderAddress, this.UNLIMITED_ALLOWANCE_IN_BASE_UNITS, + tokenAddress, ownerAddress, spenderAddress, this.UNLIMITED_ALLOWANCE_IN_BASE_UNITS, txOpts, ); return txHash; } @@ -147,16 +146,19 @@ export class TokenWrapper extends ContractWrapper { * @param ownerAddress The hex encoded user Ethereum address who is setting an allowance * for the Proxy contract. * @param amountInBaseUnits The allowance amount specified in baseUnits. + * @param txOpts Transaction parameters. * @return Transaction hash. */ public async setProxyAllowanceAsync(tokenAddress: string, ownerAddress: string, - amountInBaseUnits: BigNumber): Promise { + amountInBaseUnits: BigNumber, txOpts: TransactionOpts = {}): Promise { assert.isETHAddressHex('ownerAddress', ownerAddress); assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isValidBaseUnitAmount('amountInBaseUnits', amountInBaseUnits); const proxyAddress = this._tokenTransferProxyWrapper.getContractAddress(); - const txHash = await this.setAllowanceAsync(tokenAddress, ownerAddress, proxyAddress, amountInBaseUnits); + const txHash = await this.setAllowanceAsync( + tokenAddress, ownerAddress, proxyAddress, amountInBaseUnits, txOpts, + ); return txHash; } /** @@ -167,11 +169,14 @@ export class TokenWrapper extends ContractWrapper { * @param tokenAddress The hex encoded contract Ethereum address where the ERC20 token is deployed. * @param ownerAddress The hex encoded user Ethereum address who is setting an allowance * for the Proxy contract. + * @param txOpts Transaction parameters. * @return Transaction hash. */ - public async setUnlimitedProxyAllowanceAsync(tokenAddress: string, ownerAddress: string): Promise { + public async setUnlimitedProxyAllowanceAsync( + tokenAddress: string, ownerAddress: string, txOpts: TransactionOpts = {}, + ): Promise { const txHash = await this.setProxyAllowanceAsync( - tokenAddress, ownerAddress, this.UNLIMITED_ALLOWANCE_IN_BASE_UNITS, + tokenAddress, ownerAddress, this.UNLIMITED_ALLOWANCE_IN_BASE_UNITS, txOpts, ); return txHash; } @@ -181,10 +186,11 @@ export class TokenWrapper extends ContractWrapper { * @param fromAddress The hex encoded user Ethereum address that will send the funds. * @param toAddress The hex encoded user Ethereum address that will receive the funds. * @param amountInBaseUnits The amount (specified in baseUnits) of the token to transfer. + * @param txOpts Transaction parameters. * @return Transaction hash. */ public async transferAsync(tokenAddress: string, fromAddress: string, toAddress: string, - amountInBaseUnits: BigNumber): Promise { + amountInBaseUnits: BigNumber, txOpts: TransactionOpts = {}): Promise { assert.isETHAddressHex('tokenAddress', tokenAddress); await assert.isSenderAddressAsync('fromAddress', fromAddress, this._web3Wrapper); assert.isETHAddressHex('toAddress', toAddress); @@ -199,6 +205,8 @@ export class TokenWrapper extends ContractWrapper { const txHash = await tokenContract.transfer.sendTransactionAsync(toAddress, amountInBaseUnits, { from: fromAddress, + gas: txOpts.gasLimit, + gasPrice: txOpts.gasPrice, }); return txHash; } @@ -213,10 +221,11 @@ export class TokenWrapper extends ContractWrapper { * `fromAddress` must have set an allowance to the `senderAddress` * before this call. * @param amountInBaseUnits The amount (specified in baseUnits) of the token to transfer. + * @param txOpts Transaction parameters. * @return Transaction hash. */ public async transferFromAsync(tokenAddress: string, fromAddress: string, toAddress: string, - senderAddress: string, amountInBaseUnits: BigNumber): + senderAddress: string, amountInBaseUnits: BigNumber, txOpts: TransactionOpts = {}): Promise { assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isETHAddressHex('fromAddress', fromAddress); @@ -240,6 +249,8 @@ export class TokenWrapper extends ContractWrapper { fromAddress, toAddress, amountInBaseUnits, { from: senderAddress, + gas: txOpts.gasLimit, + gasPrice: txOpts.gasPrice, }, ); return txHash; diff --git a/packages/0x.js/src/types.ts b/packages/0x.js/src/types.ts index af4f054f0..d187af01a 100644 --- a/packages/0x.js/src/types.ts +++ b/packages/0x.js/src/types.ts @@ -332,6 +332,7 @@ export interface TxOpts { from: string; gas?: number; value?: BigNumber; + gasPrice?: BigNumber; } export interface TokenAddressBySymbol { @@ -476,17 +477,27 @@ export interface ValidateOrderFillableOpts { * let's the user query the blockchain's state at an arbitrary point in time. In order for this to work, the * backing Ethereum node must keep the entire historical state of the chain (e.g setting `--pruning=archive` * flag when running Parity). + * gasPrice: Gas price to use for a transaction in */ export interface MethodOpts { defaultBlock?: Web3.BlockParam; } +/* + * gasPrice: Gas price in Wei to use for a transaction + * gasLimit: The amount of gas to send with a transaction + */ +export interface TransactionOpts { + gasPrice?: BigNumber; + gasLimit?: number; +} + /* * shouldValidate: Flag indicating whether the library should make attempts to validate a transaction before - * broadcasting it. For example, order has a valid signature, maker has sufficient funds, etc. + * broadcasting it. For example, order has a valid signature, maker has sufficient funds, etc. Default: true */ -export interface OrderTransactionOpts { - shouldValidate: boolean; +export interface OrderTransactionOpts extends TransactionOpts { + shouldValidate?: boolean; } export type FilterObject = Web3.FilterObject; -- cgit v1.2.3 From 0e44a630f0479651522179df5743d80e6f596ace Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 23 Nov 2017 11:17:48 -0600 Subject: Add CHANGELOG entry --- packages/0x.js/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/0x.js') diff --git a/packages/0x.js/CHANGELOG.md b/packages/0x.js/CHANGELOG.md index 67b2b89b6..7fce9167a 100644 --- a/packages/0x.js/CHANGELOG.md +++ b/packages/0x.js/CHANGELOG.md @@ -8,6 +8,7 @@ vx.x.x * Remove `ZeroExError.ContractNotFound` and replace it with contract-specific errors (#233) * Make `DecodedLogEvent` contain `LogWithDecodedArgs` under log key instead of merging it in like web3 does (#234) * Rename `removed` to `isRemoved` in `DecodedLogEvent` (#234) + * Add config allowing to specify gasPrice and gasLimit for every transaction sending method (#235) * Modify order validation methods to validate against the `latest` block, not against the `pending` block (#236) v0.26.0 -- cgit v1.2.3 From d1065cd266b204f7b7f7468f5fbe0d663acf2a7b Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 23 Nov 2017 11:19:06 -0600 Subject: Add an initializer for txOpts in etherToken --- packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'packages/0x.js') 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 65f1cda24..ede0460bd 100644 --- a/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts +++ b/packages/0x.js/src/contract_wrappers/ether_token_wrapper.ts @@ -31,7 +31,9 @@ export class EtherTokenWrapper extends ContractWrapper { * @param txOpts Transaction parameters. * @return Transaction hash. */ - public async depositAsync(amountInWei: BigNumber, depositor: string, txOpts: TransactionOpts): Promise { + public async depositAsync( + amountInWei: BigNumber, depositor: string, txOpts: TransactionOpts = {}, + ): Promise { assert.isValidBaseUnitAmount('amountInWei', amountInWei); await assert.isSenderAddressAsync('depositor', depositor, this._web3Wrapper); @@ -55,7 +57,9 @@ export class EtherTokenWrapper extends ContractWrapper { * @param txOpts Transaction parameters. * @return Transaction hash. */ - public async withdrawAsync(amountInWei: BigNumber, withdrawer: string, txOpts: TransactionOpts): Promise { + public async withdrawAsync( + amountInWei: BigNumber, withdrawer: string, txOpts: TransactionOpts = {}, + ): Promise { assert.isValidBaseUnitAmount('amountInWei', amountInWei); await assert.isSenderAddressAsync('withdrawer', withdrawer, this._web3Wrapper); -- cgit v1.2.3 From 33a8c7a9fb707f6a5f1bc345988ba1e420af67c5 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 27 Nov 2017 11:55:51 -0600 Subject: Update testrpc --- packages/0x.js/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/package.json b/packages/0x.js/package.json index 024cb0fe9..c542fa2df 100644 --- a/packages/0x.js/package.json +++ b/packages/0x.js/package.json @@ -61,7 +61,7 @@ "copyfiles": "^1.2.0", "coveralls": "^3.0.0", "dirty-chai": "^2.0.1", - "ethereumjs-testrpc": "4.0.1", + "ethereumjs-testrpc": "^6.0.3", "json-loader": "^0.5.4", "mocha": "^4.0.0", "npm-run-all": "^4.0.2", -- cgit v1.2.3 From ee93f091ea5fc78d039f4f91cb234a94f428ec6a Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 27 Nov 2017 13:39:32 -0600 Subject: Add gas margin and remove undefined values --- packages/0x.js/src/contract.ts | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/src/contract.ts b/packages/0x.js/src/contract.ts index 347f47aa0..6dc0dab66 100644 --- a/packages/0x.js/src/contract.ts +++ b/packages/0x.js/src/contract.ts @@ -5,6 +5,8 @@ import * as Web3 from 'web3'; import {AbiType} from './types'; +const GAS_MARGIN = 30000; + export class Contract implements Web3.ContractInstance { public address: string; public abi: Web3.ContractAbi; @@ -63,21 +65,23 @@ export class Contract implements Web3.ContractInstance { // 1 - method level // 2 - Library defaults // 3 - estimate + const removeUndefinedProperties = _.pickBy; txData = { - ...this.defaults, - ...txData, + ...removeUndefinedProperties(this.defaults), + ...removeUndefinedProperties(txData), }; if (_.isUndefined(txData.gas)) { - const estimatedGas = await estimateGasAsync.apply(this.contract, [...args, txData]); - txData.gas = estimatedGas; - } - const callback = (err: Error, data: any) => { - if (_.isNull(err)) { - resolve(data); - } else { + try { + const estimatedGas = await estimateGasAsync.apply(this.contract, [...args, txData]); + const gas = estimatedGas + GAS_MARGIN; + txData.gas = gas; + console.log('withGas', txData); + } catch (err) { reject(err); + return; } - }; + } + const callback = (err: Error, data: any) => _.isNull(err) ? resolve(data) : reject(err); args.push(txData); args.push(callback); web3CbStyleFunction.apply(this.contract, args); -- cgit v1.2.3 From 65697f589697b206cb576025e169e8d36ef2c740 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 27 Nov 2017 13:40:03 -0600 Subject: Update MAX_REASONABLE_GAS_COST_IN_WEI --- packages/0x.js/test/ether_token_wrapper_test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/test/ether_token_wrapper_test.ts b/packages/0x.js/test/ether_token_wrapper_test.ts index 5b5e4c656..d3e4439ee 100644 --- a/packages/0x.js/test/ether_token_wrapper_test.ts +++ b/packages/0x.js/test/ether_token_wrapper_test.ts @@ -18,7 +18,7 @@ const blockchainLifecycle = new BlockchainLifecycle(); // a small amount of ETH will be used to pay this gas cost. We therefore check that the difference between // the expected balance and actual balance (given the amount of ETH deposited), only deviates by the amount // required to pay gas costs. -const MAX_REASONABLE_GAS_COST_IN_WEI = 62237; +const MAX_REASONABLE_GAS_COST_IN_WEI = 62517; describe('EtherTokenWrapper', () => { let web3: Web3; -- cgit v1.2.3 From 128fccb763bb780aa96063031116e45a98d163f9 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 27 Nov 2017 15:29:51 -0600 Subject: Fix defaults for shouldValidate --- .../src/contract_wrappers/exchange_wrapper.ts | 28 ++++++++++++++++------ 1 file changed, 21 insertions(+), 7 deletions(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts index 76a537b45..273b348ff 100644 --- a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts +++ b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts @@ -176,7 +176,9 @@ export class ExchangeWrapper extends ContractWrapper { await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; + const shouldValidate = _.isUndefined(orderTransactionOpts.shouldValidate) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { const zrxTokenAddress = this.getZRXTokenAddress(); const exchangeTradeEmulator = new ExchangeTransferSimulator(this._tokenWrapper, BlockParamLiteral.Latest); @@ -235,7 +237,9 @@ export class ExchangeWrapper extends ContractWrapper { assert.isBoolean('shouldThrowOnInsufficientBalanceOrAllowance', shouldThrowOnInsufficientBalanceOrAllowance); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); - const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; + const shouldValidate = _.isUndefined(orderTransactionOpts.shouldValidate) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { const zrxTokenAddress = this.getZRXTokenAddress(); const exchangeTradeEmulator = new ExchangeTransferSimulator(this._tokenWrapper, BlockParamLiteral.Latest); @@ -312,7 +316,9 @@ export class ExchangeWrapper extends ContractWrapper { ExchangeContractErrs.BatchOrdersMustHaveSameExchangeAddress); assert.isBoolean('shouldThrowOnInsufficientBalanceOrAllowance', shouldThrowOnInsufficientBalanceOrAllowance); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); - const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; + const shouldValidate = _.isUndefined(orderTransactionOpts.shouldValidate) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { const zrxTokenAddress = this.getZRXTokenAddress(); const exchangeTradeEmulator = new ExchangeTransferSimulator(this._tokenWrapper, BlockParamLiteral.Latest); @@ -379,7 +385,9 @@ export class ExchangeWrapper extends ContractWrapper { const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; + const shouldValidate = _.isUndefined(orderTransactionOpts.shouldValidate) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { const zrxTokenAddress = this.getZRXTokenAddress(); const exchangeTradeEmulator = new ExchangeTransferSimulator(this._tokenWrapper, BlockParamLiteral.Latest); @@ -430,7 +438,9 @@ export class ExchangeWrapper extends ContractWrapper { } const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; + const shouldValidate = _.isUndefined(orderTransactionOpts.shouldValidate) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { const zrxTokenAddress = this.getZRXTokenAddress(); const exchangeTradeEmulator = new ExchangeTransferSimulator(this._tokenWrapper, BlockParamLiteral.Latest); @@ -488,7 +498,9 @@ export class ExchangeWrapper extends ContractWrapper { const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; + const shouldValidate = _.isUndefined(orderTransactionOpts.shouldValidate) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { const orderHash = utils.getOrderHashHex(order); const unavailableTakerTokenAmount = await this.getUnavailableTakerAmountAsync(orderHash); @@ -532,7 +544,9 @@ export class ExchangeWrapper extends ContractWrapper { assert.hasAtMostOneUniqueValue(makers, ExchangeContractErrs.MultipleMakersInSingleCancelBatchDisallowed); const maker = makers[0]; await assert.isSenderAddressAsync('maker', maker, this._web3Wrapper); - const shouldValidate = orderTransactionOpts.shouldValidate || SHOULD_VALIDATE_BY_DEFAULT; + const shouldValidate = _.isUndefined(orderTransactionOpts.shouldValidate) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { for (const orderCancellationRequest of orderCancellationRequests) { const orderHash = utils.getOrderHashHex(orderCancellationRequest.order); -- cgit v1.2.3 From 5977fa881e6d68c6779d4708d55306cd05a61fb3 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 27 Nov 2017 15:30:23 -0600 Subject: Increase gas margin --- packages/0x.js/src/contract.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/src/contract.ts b/packages/0x.js/src/contract.ts index 6dc0dab66..7e16c580d 100644 --- a/packages/0x.js/src/contract.ts +++ b/packages/0x.js/src/contract.ts @@ -5,7 +5,7 @@ import * as Web3 from 'web3'; import {AbiType} from './types'; -const GAS_MARGIN = 30000; +const GAS_MARGIN = 300000; export class Contract implements Web3.ContractInstance { public address: string; @@ -75,7 +75,6 @@ export class Contract implements Web3.ContractInstance { const estimatedGas = await estimateGasAsync.apply(this.contract, [...args, txData]); const gas = estimatedGas + GAS_MARGIN; txData.gas = gas; - console.log('withGas', txData); } catch (err) { reject(err); return; -- cgit v1.2.3 From 082c5c375e07fc8781ca74671326e5184422d298 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 27 Nov 2017 15:31:06 -0600 Subject: Fix tests --- packages/0x.js/test/exchange_wrapper_test.ts | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/0x.js') diff --git a/packages/0x.js/test/exchange_wrapper_test.ts b/packages/0x.js/test/exchange_wrapper_test.ts index f6c823cc4..3e2160330 100644 --- a/packages/0x.js/test/exchange_wrapper_test.ts +++ b/packages/0x.js/test/exchange_wrapper_test.ts @@ -124,6 +124,7 @@ describe('ExchangeWrapper', () => { it('should not validate when orderTransactionOptions specify not to validate', async () => { return expect(zeroEx.exchange.batchFillOrKillAsync(orderFillRequests, takerAddress, { shouldValidate: false, + gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); -- cgit v1.2.3 From e6887dc9d4b129eee062c9c2f80a970548ee7650 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 27 Nov 2017 15:33:20 -0600 Subject: Fix a comment --- packages/0x.js/src/contract.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/src/contract.ts b/packages/0x.js/src/contract.ts index 7e16c580d..25a0609aa 100644 --- a/packages/0x.js/src/contract.ts +++ b/packages/0x.js/src/contract.ts @@ -61,10 +61,10 @@ export class Contract implements Web3.ContractInstance { if (this.isTxData(lastArg)) { txData = args.pop(); } - // Gas amounts priorities: - // 1 - method level - // 2 - Library defaults - // 3 - estimate + // Gas amount sourced with the following priorities: + // 1. Optional param passed it to public method call + // 2. Global config passed in at library instantiation + // 3. Gas estimate calculation + safety margin const removeUndefinedProperties = _.pickBy; txData = { ...removeUndefinedProperties(this.defaults), -- cgit v1.2.3 From 0500602ac3e8d4ea3c54d1cf2d4dcc2b0bcf80ba Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 27 Nov 2017 17:07:37 -0600 Subject: Fix tests --- packages/0x.js/test/exchange_wrapper_test.ts | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'packages/0x.js') diff --git a/packages/0x.js/test/exchange_wrapper_test.ts b/packages/0x.js/test/exchange_wrapper_test.ts index 3e2160330..d862c347d 100644 --- a/packages/0x.js/test/exchange_wrapper_test.ts +++ b/packages/0x.js/test/exchange_wrapper_test.ts @@ -184,6 +184,7 @@ describe('ExchangeWrapper', () => { it('should not validate when orderTransactionOptions specify not to validate', async () => { return expect(zeroEx.exchange.fillOrKillOrderAsync(signedOrder, emptyFillableAmount, takerAddress, { shouldValidate: false, + gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); @@ -363,6 +364,7 @@ describe('ExchangeWrapper', () => { return expect(zeroEx.exchange.batchFillOrdersAsync( orderFillBatch, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, { shouldValidate: false, + gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); @@ -420,6 +422,7 @@ describe('ExchangeWrapper', () => { return expect(zeroEx.exchange.fillOrdersUpToAsync( signedOrders, emptyFillUpToAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, { shouldValidate: false, + gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); @@ -468,6 +471,7 @@ describe('ExchangeWrapper', () => { it('should not validate when orderTransactionOptions specify not to validate', async () => { return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, emptyCancelTakerTokenAmount, { shouldValidate: false, + gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); }); }); @@ -543,6 +547,7 @@ describe('ExchangeWrapper', () => { it('should not validate when orderTransactionOptions specify not to validate', async () => { return expect(zeroEx.exchange.batchCancelOrdersAsync(cancelBatch, { shouldValidate: false, + gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); }); }); -- cgit v1.2.3 From 36b21e6e7b8033117c9a1313c1294682184462f8 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 28 Nov 2017 11:44:10 -0600 Subject: Add fake gas estimate suprovider for tests --- packages/0x.js/src/contract.ts | 2 +- .../src/subproviders/empty_wallet_subprovider.ts | 27 ------------------- packages/0x.js/test/utils/constants.ts | 1 + .../utils/subproviders/empty_wallet_subprovider.ts | 27 +++++++++++++++++++ .../subproviders/fake_gas_estimate_subprovider.ts | 31 ++++++++++++++++++++++ packages/0x.js/test/utils/web3_factory.ts | 4 ++- 6 files changed, 63 insertions(+), 29 deletions(-) delete mode 100644 packages/0x.js/src/subproviders/empty_wallet_subprovider.ts create mode 100644 packages/0x.js/test/utils/subproviders/empty_wallet_subprovider.ts create mode 100644 packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts (limited to 'packages/0x.js') diff --git a/packages/0x.js/src/contract.ts b/packages/0x.js/src/contract.ts index 25a0609aa..0cd3a315f 100644 --- a/packages/0x.js/src/contract.ts +++ b/packages/0x.js/src/contract.ts @@ -58,7 +58,7 @@ export class Contract implements Web3.ContractInstance { const promise = new Promise(async (resolve, reject) => { const lastArg = args[args.length - 1]; let txData: Partial = {}; - if (this.isTxData(lastArg)) { + if (!_.isUndefined(lastArg) && this.isTxData(lastArg)) { txData = args.pop(); } // Gas amount sourced with the following priorities: diff --git a/packages/0x.js/src/subproviders/empty_wallet_subprovider.ts b/packages/0x.js/src/subproviders/empty_wallet_subprovider.ts deleted file mode 100644 index 2993bc801..000000000 --- a/packages/0x.js/src/subproviders/empty_wallet_subprovider.ts +++ /dev/null @@ -1,27 +0,0 @@ -import {JSONRPCPayload} from '../types'; - -/* - * This class implements the web3-provider-engine subprovider interface and returns - * that the provider has no addresses when queried. - * Source: https://github.com/MetaMask/provider-engine/blob/master/subproviders/subprovider.js - */ -export class EmptyWalletSubProvider { - // This method needs to be here to satisfy the interface but linter wants it to be static. - // tslint:disable-next-line:prefer-function-over-method - public handleRequest(payload: JSONRPCPayload, next: () => void, end: (err: Error|null, result: any) => void) { - switch (payload.method) { - case 'eth_accounts': - end(null, []); - return; - - default: - next(); - return; - } - } - // Required to implement this method despite not needing it for this subprovider - // tslint:disable-next-line:prefer-function-over-method - public setEngine(engine: any) { - // noop - } -} diff --git a/packages/0x.js/test/utils/constants.ts b/packages/0x.js/test/utils/constants.ts index 212abf4d6..75fdf49c9 100644 --- a/packages/0x.js/test/utils/constants.ts +++ b/packages/0x.js/test/utils/constants.ts @@ -8,4 +8,5 @@ export const constants = { KOVAN_RPC_URL: 'https://kovan.infura.io', ROPSTEN_RPC_URL: 'https://ropsten.infura.io', ZRX_DECIMALS: 18, + GAS_ESTIMATE: 500000, }; diff --git a/packages/0x.js/test/utils/subproviders/empty_wallet_subprovider.ts b/packages/0x.js/test/utils/subproviders/empty_wallet_subprovider.ts new file mode 100644 index 000000000..bc11e56d8 --- /dev/null +++ b/packages/0x.js/test/utils/subproviders/empty_wallet_subprovider.ts @@ -0,0 +1,27 @@ +import {JSONRPCPayload} from '../../../src/types'; + +/* + * This class implements the web3-provider-engine subprovider interface and returns + * that the provider has no addresses when queried. + * Source: https://github.com/MetaMask/provider-engine/blob/master/subproviders/subprovider.js + */ +export class EmptyWalletSubProvider { + // This method needs to be here to satisfy the interface but linter wants it to be static. + // tslint:disable-next-line:prefer-function-over-method + public handleRequest(payload: JSONRPCPayload, next: () => void, end: (err: Error|null, result: any) => void) { + switch (payload.method) { + case 'eth_accounts': + end(null, []); + return; + + default: + next(); + return; + } + } + // Required to implement this method despite not needing it for this subprovider + // tslint:disable-next-line:prefer-function-over-method + public setEngine(engine: any) { + // noop + } +} diff --git a/packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts b/packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts new file mode 100644 index 000000000..5eccb3d24 --- /dev/null +++ b/packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts @@ -0,0 +1,31 @@ +import {JSONRPCPayload} from '../../../src/types'; + +/* + * This class implements the web3-provider-engine subprovider interface and returns + * the constant gas estimate when queried. + * Source: https://github.com/MetaMask/provider-engine/blob/master/subproviders/subprovider.js + */ +export class FakeGasEstimateProvider { + private constantGasAmount: number; + constructor(constantGasAmount: number) { + this.constantGasAmount = constantGasAmount; + } + // This method needs to be here to satisfy the interface but linter wants it to be static. + // tslint:disable-next-line:prefer-function-over-method + public handleRequest(payload: JSONRPCPayload, next: () => void, end: (err: Error|null, result: any) => void) { + switch (payload.method) { + case 'eth_estimateGas': + end(null, this.constantGasAmount); + return; + + default: + next(); + return; + } + } + // Required to implement this method despite not needing it for this subprovider + // tslint:disable-next-line:prefer-function-over-method + public setEngine(engine: any) { + // noop + } +} diff --git a/packages/0x.js/test/utils/web3_factory.ts b/packages/0x.js/test/utils/web3_factory.ts index b4bf1acd3..854c6091d 100644 --- a/packages/0x.js/test/utils/web3_factory.ts +++ b/packages/0x.js/test/utils/web3_factory.ts @@ -7,7 +7,8 @@ import * as Web3 from 'web3'; import ProviderEngine = require('web3-provider-engine'); import RpcSubprovider = require('web3-provider-engine/subproviders/rpc'); -import {EmptyWalletSubProvider} from '../../src/subproviders/empty_wallet_subprovider'; +import {EmptyWalletSubProvider} from './subproviders/empty_wallet_subprovider'; +import {FakeGasEstimateProvider} from './subproviders/fake_gas_estimate_subprovider'; import {constants} from './constants'; @@ -24,6 +25,7 @@ export const web3Factory = { if (!hasAddresses) { provider.addProvider(new EmptyWalletSubProvider()); } + provider.addProvider(new FakeGasEstimateProvider(constants.GAS_ESTIMATE)); provider.addProvider(new RpcSubprovider({ rpcUrl, })); -- cgit v1.2.3 From d6a9e7520da63b9163894bb5aa191a5dfc83ce40 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 28 Nov 2017 11:51:10 -0600 Subject: Remove gas params from tests --- packages/0x.js/test/exchange_wrapper_test.ts | 6 ------ 1 file changed, 6 deletions(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/test/exchange_wrapper_test.ts b/packages/0x.js/test/exchange_wrapper_test.ts index d862c347d..f6c823cc4 100644 --- a/packages/0x.js/test/exchange_wrapper_test.ts +++ b/packages/0x.js/test/exchange_wrapper_test.ts @@ -124,7 +124,6 @@ describe('ExchangeWrapper', () => { it('should not validate when orderTransactionOptions specify not to validate', async () => { return expect(zeroEx.exchange.batchFillOrKillAsync(orderFillRequests, takerAddress, { shouldValidate: false, - gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); @@ -184,7 +183,6 @@ describe('ExchangeWrapper', () => { it('should not validate when orderTransactionOptions specify not to validate', async () => { return expect(zeroEx.exchange.fillOrKillOrderAsync(signedOrder, emptyFillableAmount, takerAddress, { shouldValidate: false, - gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); @@ -364,7 +362,6 @@ describe('ExchangeWrapper', () => { return expect(zeroEx.exchange.batchFillOrdersAsync( orderFillBatch, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, { shouldValidate: false, - gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); @@ -422,7 +419,6 @@ describe('ExchangeWrapper', () => { return expect(zeroEx.exchange.fillOrdersUpToAsync( signedOrders, emptyFillUpToAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, { shouldValidate: false, - gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); @@ -471,7 +467,6 @@ describe('ExchangeWrapper', () => { it('should not validate when orderTransactionOptions specify not to validate', async () => { return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, emptyCancelTakerTokenAmount, { shouldValidate: false, - gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); }); }); @@ -547,7 +542,6 @@ describe('ExchangeWrapper', () => { it('should not validate when orderTransactionOptions specify not to validate', async () => { return expect(zeroEx.exchange.batchCancelOrdersAsync(cancelBatch, { shouldValidate: false, - gasLimit: 200000, // If we don't pass this gas estimation will fail })).to.not.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); }); }); -- cgit v1.2.3 From d5e58ecdd88d4e85bd2e98b091e500b37ca66bfe Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 28 Nov 2017 12:05:32 -0600 Subject: Pin testrpc version --- packages/0x.js/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/package.json b/packages/0x.js/package.json index c542fa2df..a8e99bacd 100644 --- a/packages/0x.js/package.json +++ b/packages/0x.js/package.json @@ -61,7 +61,7 @@ "copyfiles": "^1.2.0", "coveralls": "^3.0.0", "dirty-chai": "^2.0.1", - "ethereumjs-testrpc": "^6.0.3", + "ethereumjs-testrpc": "6.0.3", "json-loader": "^0.5.4", "mocha": "^4.0.0", "npm-run-all": "^4.0.2", -- cgit v1.2.3 From 0c5a9fbc2c615002cc0ce3d8f9c4b79a6fd3b5a2 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 28 Nov 2017 12:06:11 -0600 Subject: Fix a typo --- packages/0x.js/src/contract.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/src/contract.ts b/packages/0x.js/src/contract.ts index 0cd3a315f..872f18b2a 100644 --- a/packages/0x.js/src/contract.ts +++ b/packages/0x.js/src/contract.ts @@ -62,7 +62,7 @@ export class Contract implements Web3.ContractInstance { txData = args.pop(); } // Gas amount sourced with the following priorities: - // 1. Optional param passed it to public method call + // 1. Optional param passed in to public method call // 2. Global config passed in at library instantiation // 3. Gas estimate calculation + safety margin const removeUndefinedProperties = _.pickBy; -- cgit v1.2.3 From 1d584b1329427e0cf756f9312c54a1ed7a7d4b34 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 28 Nov 2017 12:10:06 -0600 Subject: Remove a comment --- packages/0x.js/src/types.ts | 1 - 1 file changed, 1 deletion(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/src/types.ts b/packages/0x.js/src/types.ts index d187af01a..3949c17c5 100644 --- a/packages/0x.js/src/types.ts +++ b/packages/0x.js/src/types.ts @@ -477,7 +477,6 @@ export interface ValidateOrderFillableOpts { * let's the user query the blockchain's state at an arbitrary point in time. In order for this to work, the * backing Ethereum node must keep the entire historical state of the chain (e.g setting `--pruning=archive` * flag when running Parity). - * gasPrice: Gas price to use for a transaction in */ export interface MethodOpts { defaultBlock?: Web3.BlockParam; -- cgit v1.2.3 From 50bf6a991d75fab76fd5d5e2078d805528a32b45 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 28 Nov 2017 12:12:25 -0600 Subject: Normalise subprovider names --- .../0x.js/test/utils/subproviders/empty_wallet_subprovider.ts | 2 +- .../test/utils/subproviders/fake_gas_estimate_subprovider.ts | 2 +- packages/0x.js/test/utils/web3_factory.ts | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/test/utils/subproviders/empty_wallet_subprovider.ts b/packages/0x.js/test/utils/subproviders/empty_wallet_subprovider.ts index bc11e56d8..e5e279873 100644 --- a/packages/0x.js/test/utils/subproviders/empty_wallet_subprovider.ts +++ b/packages/0x.js/test/utils/subproviders/empty_wallet_subprovider.ts @@ -5,7 +5,7 @@ import {JSONRPCPayload} from '../../../src/types'; * that the provider has no addresses when queried. * Source: https://github.com/MetaMask/provider-engine/blob/master/subproviders/subprovider.js */ -export class EmptyWalletSubProvider { +export class EmptyWalletSubprovider { // This method needs to be here to satisfy the interface but linter wants it to be static. // tslint:disable-next-line:prefer-function-over-method public handleRequest(payload: JSONRPCPayload, next: () => void, end: (err: Error|null, result: any) => void) { diff --git a/packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts b/packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts index 5eccb3d24..14a6b33cf 100644 --- a/packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts +++ b/packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts @@ -5,7 +5,7 @@ import {JSONRPCPayload} from '../../../src/types'; * the constant gas estimate when queried. * Source: https://github.com/MetaMask/provider-engine/blob/master/subproviders/subprovider.js */ -export class FakeGasEstimateProvider { +export class FakeGasEstimateSubprovider { private constantGasAmount: number; constructor(constantGasAmount: number) { this.constantGasAmount = constantGasAmount; diff --git a/packages/0x.js/test/utils/web3_factory.ts b/packages/0x.js/test/utils/web3_factory.ts index 854c6091d..da4828943 100644 --- a/packages/0x.js/test/utils/web3_factory.ts +++ b/packages/0x.js/test/utils/web3_factory.ts @@ -7,8 +7,8 @@ import * as Web3 from 'web3'; import ProviderEngine = require('web3-provider-engine'); import RpcSubprovider = require('web3-provider-engine/subproviders/rpc'); -import {EmptyWalletSubProvider} from './subproviders/empty_wallet_subprovider'; -import {FakeGasEstimateProvider} from './subproviders/fake_gas_estimate_subprovider'; +import {EmptyWalletSubprovider} from './subproviders/empty_wallet_subprovider'; +import {FakeGasEstimateSubprovider} from './subproviders/fake_gas_estimate_subprovider'; import {constants} from './constants'; @@ -23,9 +23,9 @@ export const web3Factory = { const provider = new ProviderEngine(); const rpcUrl = `http://${constants.RPC_HOST}:${constants.RPC_PORT}`; if (!hasAddresses) { - provider.addProvider(new EmptyWalletSubProvider()); + provider.addProvider(new EmptyWalletSubprovider()); } - provider.addProvider(new FakeGasEstimateProvider(constants.GAS_ESTIMATE)); + provider.addProvider(new FakeGasEstimateSubprovider(constants.GAS_ESTIMATE)); provider.addProvider(new RpcSubprovider({ rpcUrl, })); -- cgit v1.2.3 From 9f5e724aec9c7fcc3ab2de4fff6d4ef8a1782080 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 28 Nov 2017 12:13:50 -0600 Subject: Add a HACK comment --- .../0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts | 3 +++ 1 file changed, 3 insertions(+) (limited to 'packages/0x.js') diff --git a/packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts b/packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts index 14a6b33cf..059163f2e 100644 --- a/packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts +++ b/packages/0x.js/test/utils/subproviders/fake_gas_estimate_subprovider.ts @@ -3,6 +3,9 @@ import {JSONRPCPayload} from '../../../src/types'; /* * This class implements the web3-provider-engine subprovider interface and returns * the constant gas estimate when queried. + * HACK: We need this so that our tests don't use testrpc gas estimation which sometimes kills the node. + * Source: https://github.com/trufflesuite/ganache-cli/issues/417 + * Source: https://github.com/trufflesuite/ganache-cli/issues/437 * Source: https://github.com/MetaMask/provider-engine/blob/master/subproviders/subprovider.js */ export class FakeGasEstimateSubprovider { -- cgit v1.2.3 From bead76a5b7eeb7d9bdaa0a677c366d2618feb425 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 28 Nov 2017 12:16:02 -0600 Subject: Add CHANGELOG comment --- packages/0x.js/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/0x.js') diff --git a/packages/0x.js/CHANGELOG.md b/packages/0x.js/CHANGELOG.md index 7fce9167a..8f9444da6 100644 --- a/packages/0x.js/CHANGELOG.md +++ b/packages/0x.js/CHANGELOG.md @@ -9,6 +9,7 @@ vx.x.x * Make `DecodedLogEvent` contain `LogWithDecodedArgs` under log key instead of merging it in like web3 does (#234) * Rename `removed` to `isRemoved` in `DecodedLogEvent` (#234) * Add config allowing to specify gasPrice and gasLimit for every transaction sending method (#235) + * All transaction sending methods now call `estimateGas` if no gas amount was supplied (#235) * Modify order validation methods to validate against the `latest` block, not against the `pending` block (#236) v0.26.0 -- cgit v1.2.3 From 353abffba9afd73d70fce47fee2f2d6067c6c8ae Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 28 Nov 2017 12:23:41 -0600 Subject: Improve the comment --- packages/0x.js/src/contract.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'packages/0x.js') diff --git a/packages/0x.js/src/contract.ts b/packages/0x.js/src/contract.ts index 872f18b2a..a4ee03910 100644 --- a/packages/0x.js/src/contract.ts +++ b/packages/0x.js/src/contract.ts @@ -5,7 +5,9 @@ import * as Web3 from 'web3'; import {AbiType} from './types'; -const GAS_MARGIN = 300000; +// HACK: Gas estimates on testrpc don't take into account gas refunds. +// Our calls can trigger max 8 gas refunds for SSTORE per transaction for 15k gas each which gives 120k. +const GAS_MARGIN = 120000; export class Contract implements Web3.ContractInstance { public address: string; -- cgit v1.2.3