From bdbd01f965d76f5f3a5524d5d688eac14427a875 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 17 Jul 2017 14:40:38 -0700 Subject: Add a test: should throw when maker has balance to cover fees or transfer but not both --- test/exchange_wrapper_test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 0dc67bb01..813afdddd 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -303,6 +303,21 @@ describe('ExchangeWrapper', () => { )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerFeeAllowance); }); }); + it('should throw when maker has balance to cover fees or transfer but not both', async () => { + const makerFee = new BigNumber(1); + const takerFee = new BigNumber(1); + const signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + zrxTokenAddress, takerTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + const balanceToSubtractFromMaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, + ); + return expect( + zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress), + ).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerBalance); + }); }); describe('successful fills', () => { it('should fill a valid order', async () => { -- cgit From c7d89d98f3b6a95f41bcdf4f09aa7aa516a0fa64 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 17 Jul 2017 14:43:17 -0700 Subject: Move _validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync to orderValidationUtils --- src/contract_wrappers/exchange_wrapper.ts | 67 ++----------------------------- src/utils/order_validation_utils.ts | 66 ++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 63 deletions(-) create mode 100644 src/utils/order_validation_utils.ts diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 9cc29e286..8895c5000 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -35,6 +35,7 @@ import { import {assert} from '../utils/assert'; import {utils} from '../utils/utils'; import {eventUtils} from '../utils/event_utils'; +import {orderValidationUtils} from '../utils/order_validation_utils'; import {ContractWrapper} from './contract_wrapper'; import {ProxyWrapper} from './proxy_wrapper'; import {ecSignatureSchema} from '../schemas/ec_signature_schema'; @@ -667,8 +668,9 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(ExchangeContractErrs.OrderFillExpired); } const zrxTokenAddress = await this._getZRXTokenAddressAsync(signedOrder.exchangeContractAddress); - await this._validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, - senderAddress, zrxTokenAddress); + await orderValidationUtils.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync( + signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, + ); const wouldRoundingErrorOccur = await this._isRoundingErrorAsync( signedOrder.takerTokenAmount, fillTakerAmount, signedOrder.makerTokenAmount, @@ -702,67 +704,6 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(ExchangeContractErrs.InsufficientRemainingFillAmount); } } - /** - * This method does not currently validate the edge-case where the makerToken or takerToken is also the token used - * to pay fees (ZRX). It is possible for them to have enough for fees and the transfer but not both. - * Handling the edge-cases that arise when this happens would require making sure that the user has sufficient - * funds to pay both the fees and the transfer amount. We decided to punt on this for now as the contracts - * will throw for these edge-cases. - * TODO: Throw errors before calling the smart contract for these edge-cases in order to minimize - * the callers gas costs. - */ - private async _validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder: SignedOrder, - fillTakerAmount: BigNumber.BigNumber, - senderAddress: string, - zrxTokenAddress: string, - ): Promise { - - const makerBalance = await this._tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, - signedOrder.maker); - const takerBalance = await this._tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); - const makerAllowance = await this._tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress, - signedOrder.maker); - const takerAllowance = await this._tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, - senderAddress); - - // exchangeRate is the price of one maker token denominated in taker tokens - const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); - const fillMakerAmountInBaseUnits = fillTakerAmount.div(exchangeRate); - - if (fillTakerAmount.greaterThan(takerBalance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerBalance); - } - if (fillTakerAmount.greaterThan(takerAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); - } - if (fillMakerAmountInBaseUnits.greaterThan(makerBalance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerBalance); - } - if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); - } - - const makerFeeBalance = await this._tokenWrapper.getBalanceAsync(zrxTokenAddress, - signedOrder.maker); - const takerFeeBalance = await this._tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); - const makerFeeAllowance = await this._tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - signedOrder.maker); - const takerFeeAllowance = await this._tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - senderAddress); - - if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerFeeBalance); - } - if (signedOrder.takerFee.greaterThan(takerFeeAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerFeeAllowance); - } - if (signedOrder.makerFee.greaterThan(makerFeeBalance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerFeeBalance); - } - if (signedOrder.makerFee.greaterThan(makerFeeAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerFeeAllowance); - } - } private _throwErrorLogsAsErrors(logs: ContractEvent[]): void { const errEvent = _.find(logs, {event: 'LogError'}); if (!_.isUndefined(errEvent)) { diff --git a/src/utils/order_validation_utils.ts b/src/utils/order_validation_utils.ts new file mode 100644 index 000000000..05c284ac3 --- /dev/null +++ b/src/utils/order_validation_utils.ts @@ -0,0 +1,66 @@ +import {ExchangeContractErrs, SignedOrder} from '../types'; + +export const orderValidationUtils = { + /** + * This method does not currently validate the edge-case where the makerToken or takerToken is also the token used + * to pay fees (ZRX). It is possible for them to have enough for fees and the transfer but not both. + * Handling the edge-cases that arise when this happens would require making sure that the user has sufficient + * funds to pay both the fees and the transfer amount. We decided to punt on this for now as the contracts + * will throw for these edge-cases. + * TODO: Throw errors before calling the smart contract for these edge-cases in order to minimize + * the callers gas costs. + */ + async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync( + signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string, + ): Promise { + + const makerBalance = await this._tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, + signedOrder.maker); + const takerBalance = await this._tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); + const makerAllowance = await this._tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress, + signedOrder.maker); + const takerAllowance = await this._tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, + senderAddress); + + // exchangeRate is the price of one maker token denominated in taker tokens + const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); + const fillMakerAmountInBaseUnits = fillTakerAmount.div(exchangeRate); + + const isMakerTokenZRX = signedOrder.makerTokenAddress === zrxTokenAddress; + const isTakerTokenZRX = signedOrder.takerTokenAddress === zrxTokenAddress; + + if (fillTakerAmount.greaterThan(takerBalance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerBalance); + } + if (fillTakerAmount.greaterThan(takerAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); + } + if (fillMakerAmountInBaseUnits.greaterThan(makerBalance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerBalance); + } + if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); + } + + const makerFeeBalance = await this._tokenWrapper.getBalanceAsync(zrxTokenAddress, + signedOrder.maker); + const takerFeeBalance = await this._tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); + const makerFeeAllowance = await this._tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, + signedOrder.maker); + const takerFeeAllowance = await this._tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, + senderAddress); + + if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerFeeBalance); + } + if (signedOrder.takerFee.greaterThan(takerFeeAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerFeeAllowance); + } + if (signedOrder.makerFee.greaterThan(makerFeeBalance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerFeeBalance); + } + if (signedOrder.makerFee.greaterThan(makerFeeAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerFeeAllowance); + } + }, +}; -- cgit From d9a8b961542dfbde0a736e1caea2de41d0e350cd Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 17 Jul 2017 14:52:34 -0700 Subject: Pass tokenWrapper to validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync --- src/contract_wrappers/exchange_wrapper.ts | 2 +- src/utils/order_validation_utils.ts | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 8895c5000..232531a83 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -669,7 +669,7 @@ export class ExchangeWrapper extends ContractWrapper { } const zrxTokenAddress = await this._getZRXTokenAddressAsync(signedOrder.exchangeContractAddress); await orderValidationUtils.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync( - signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, + this._tokenWrapper, signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, ); const wouldRoundingErrorOccur = await this._isRoundingErrorAsync( diff --git a/src/utils/order_validation_utils.ts b/src/utils/order_validation_utils.ts index 05c284ac3..924e9aebd 100644 --- a/src/utils/order_validation_utils.ts +++ b/src/utils/order_validation_utils.ts @@ -1,4 +1,5 @@ import {ExchangeContractErrs, SignedOrder} from '../types'; +import {TokenWrapper} from '../contract_wrappers/token_wrapper'; export const orderValidationUtils = { /** @@ -11,15 +12,14 @@ export const orderValidationUtils = { * the callers gas costs. */ async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync( - signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string, - ): Promise { - - const makerBalance = await this._tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, + tokenWrapper: TokenWrapper, signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, + senderAddress: string, zrxTokenAddress: string): Promise { + const makerBalance = await tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); - const takerBalance = await this._tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); - const makerAllowance = await this._tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress, + const takerBalance = await tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); + const makerAllowance = await tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); - const takerAllowance = await this._tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, + const takerAllowance = await tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, senderAddress); // exchangeRate is the price of one maker token denominated in taker tokens @@ -42,12 +42,12 @@ export const orderValidationUtils = { throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); } - const makerFeeBalance = await this._tokenWrapper.getBalanceAsync(zrxTokenAddress, + const makerFeeBalance = await tokenWrapper.getBalanceAsync(zrxTokenAddress, signedOrder.maker); - const takerFeeBalance = await this._tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); - const makerFeeAllowance = await this._tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, + const takerFeeBalance = await tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); + const makerFeeAllowance = await tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, signedOrder.maker); - const takerFeeAllowance = await this._tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, + const takerFeeAllowance = await tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, senderAddress); if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { -- cgit From 58d2b799d6adb8d8af7565af4b63e3a3c5748c10 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 17 Jul 2017 16:43:52 -0700 Subject: Refactor OrderValidationUtils to check for the case when ZRX is one of the tokens traded --- src/contract_wrappers/exchange_wrapper.ts | 4 +- src/utils/order_validation_utils.ts | 132 ++++++++++++++++++------------ 2 files changed, 81 insertions(+), 55 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 232531a83..f75f8d9bb 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -35,7 +35,7 @@ import { import {assert} from '../utils/assert'; import {utils} from '../utils/utils'; import {eventUtils} from '../utils/event_utils'; -import {orderValidationUtils} from '../utils/order_validation_utils'; +import {OrderValidationUtils} from '../utils/order_validation_utils'; import {ContractWrapper} from './contract_wrapper'; import {ProxyWrapper} from './proxy_wrapper'; import {ecSignatureSchema} from '../schemas/ec_signature_schema'; @@ -668,7 +668,7 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(ExchangeContractErrs.OrderFillExpired); } const zrxTokenAddress = await this._getZRXTokenAddressAsync(signedOrder.exchangeContractAddress); - await orderValidationUtils.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync( + await OrderValidationUtils.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync( this._tokenWrapper, signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, ); diff --git a/src/utils/order_validation_utils.ts b/src/utils/order_validation_utils.ts index 924e9aebd..2d7acd905 100644 --- a/src/utils/order_validation_utils.ts +++ b/src/utils/order_validation_utils.ts @@ -1,66 +1,92 @@ import {ExchangeContractErrs, SignedOrder} from '../types'; import {TokenWrapper} from '../contract_wrappers/token_wrapper'; -export const orderValidationUtils = { - /** - * This method does not currently validate the edge-case where the makerToken or takerToken is also the token used - * to pay fees (ZRX). It is possible for them to have enough for fees and the transfer but not both. - * Handling the edge-cases that arise when this happens would require making sure that the user has sufficient - * funds to pay both the fees and the transfer amount. We decided to punt on this for now as the contracts - * will throw for these edge-cases. - * TODO: Throw errors before calling the smart contract for these edge-cases in order to minimize - * the callers gas costs. - */ - async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync( +export class OrderValidationUtils { + public static async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync( tokenWrapper: TokenWrapper, signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string): Promise { - const makerBalance = await tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, - signedOrder.maker); - const takerBalance = await tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); - const makerAllowance = await tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress, - signedOrder.maker); - const takerAllowance = await tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, - senderAddress); + await OrderValidationUtils.validateFillOrderMakerBalancesAndAllowancesAndThrowIfInvalidAsync( + tokenWrapper, signedOrder, fillTakerAmount, zrxTokenAddress, + ); + await OrderValidationUtils.validateFillOrderTakerBalancesAndAllowancesAndThrowIfInvalidAsync( + tokenWrapper, signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, + ); + } + private static async validateFillOrderMakerBalancesAndAllowancesAndThrowIfInvalidAsync( + tokenWrapper: TokenWrapper, signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, + zrxTokenAddress: string, + ): Promise { + const makerBalance = await tokenWrapper.getBalanceAsync( + signedOrder.makerTokenAddress, signedOrder.maker); + const makerAllowance = await tokenWrapper.getProxyAllowanceAsync( + signedOrder.makerTokenAddress, signedOrder.maker); + const makerZRXBalance = await tokenWrapper.getBalanceAsync( + zrxTokenAddress, signedOrder.maker); + const makerZRXAllowance = await tokenWrapper.getProxyAllowanceAsync( + zrxTokenAddress, signedOrder.maker); + const isMakerTokenZRX = signedOrder.makerTokenAddress === zrxTokenAddress; // exchangeRate is the price of one maker token denominated in taker tokens const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); - const fillMakerAmountInBaseUnits = fillTakerAmount.div(exchangeRate); - - const isMakerTokenZRX = signedOrder.makerTokenAddress === zrxTokenAddress; - const isTakerTokenZRX = signedOrder.takerTokenAddress === zrxTokenAddress; + const fillMakerAmount = fillTakerAmount.div(exchangeRate); - if (fillTakerAmount.greaterThan(takerBalance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerBalance); - } - if (fillTakerAmount.greaterThan(takerAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); - } - if (fillMakerAmountInBaseUnits.greaterThan(makerBalance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerBalance); - } - if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); + if (isMakerTokenZRX) { + const requiredMakerAmount = fillMakerAmount.plus(signedOrder.makerFee); + if (requiredMakerAmount.greaterThan(makerBalance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerBalance); + } + if (requiredMakerAmount.greaterThan(makerAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); + } + } else { + if (fillMakerAmount.greaterThan(makerBalance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerBalance); + } + if (fillMakerAmount.greaterThan(makerAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); + } + if (signedOrder.makerFee.greaterThan(makerZRXBalance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerFeeBalance); + } + if (signedOrder.makerFee.greaterThan(makerZRXAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerFeeAllowance); + } } + } + private static async validateFillOrderTakerBalancesAndAllowancesAndThrowIfInvalidAsync( + tokenWrapper: TokenWrapper, signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, + senderAddress: string, zrxTokenAddress: string, + ): Promise { + const takerBalance = await tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); + const takerAllowance = await tokenWrapper.getProxyAllowanceAsync( + signedOrder.takerTokenAddress, senderAddress); + const takerZRXBalance = await tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); + const takerZRXAllowance = await tokenWrapper.getProxyAllowanceAsync( + zrxTokenAddress, senderAddress); - const makerFeeBalance = await tokenWrapper.getBalanceAsync(zrxTokenAddress, - signedOrder.maker); - const takerFeeBalance = await tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); - const makerFeeAllowance = await tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - signedOrder.maker); - const takerFeeAllowance = await tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - senderAddress); + const isTakerTokenZRX = signedOrder.takerTokenAddress === zrxTokenAddress; - if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerFeeBalance); - } - if (signedOrder.takerFee.greaterThan(takerFeeAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerFeeAllowance); - } - if (signedOrder.makerFee.greaterThan(makerFeeBalance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerFeeBalance); - } - if (signedOrder.makerFee.greaterThan(makerFeeAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerFeeAllowance); + if (isTakerTokenZRX) { + const requiredTakerAmount = fillTakerAmount.plus(signedOrder.takerFee); + if (requiredTakerAmount.greaterThan(takerBalance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerBalance); + } + if (requiredTakerAmount.greaterThan(takerAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); + } + } else { + if (fillTakerAmount.greaterThan(takerBalance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerBalance); + } + if (fillTakerAmount.greaterThan(takerAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); + } + if (signedOrder.takerFee.greaterThan(takerZRXBalance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerFeeBalance); + } + if (signedOrder.takerFee.greaterThan(takerZRXAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerFeeAllowance); + } } - }, -}; + } +} -- cgit From defd09459dd774791b5bb88e2ec3a5216e8b6b4a Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 17 Jul 2017 17:40:19 -0700 Subject: Cover all possible branches of order validation errors with tests --- src/utils/order_validation_utils.ts | 19 ++++------ test/exchange_wrapper_test.ts | 73 ++++++++++++++++++++++++++++++------- 2 files changed, 67 insertions(+), 25 deletions(-) diff --git a/src/utils/order_validation_utils.ts b/src/utils/order_validation_utils.ts index 2d7acd905..4452a7aef 100644 --- a/src/utils/order_validation_utils.ts +++ b/src/utils/order_validation_utils.ts @@ -16,14 +16,9 @@ export class OrderValidationUtils { tokenWrapper: TokenWrapper, signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, zrxTokenAddress: string, ): Promise { - const makerBalance = await tokenWrapper.getBalanceAsync( - signedOrder.makerTokenAddress, signedOrder.maker); + const makerBalance = await tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); const makerAllowance = await tokenWrapper.getProxyAllowanceAsync( signedOrder.makerTokenAddress, signedOrder.maker); - const makerZRXBalance = await tokenWrapper.getBalanceAsync( - zrxTokenAddress, signedOrder.maker); - const makerZRXAllowance = await tokenWrapper.getProxyAllowanceAsync( - zrxTokenAddress, signedOrder.maker); const isMakerTokenZRX = signedOrder.makerTokenAddress === zrxTokenAddress; // exchangeRate is the price of one maker token denominated in taker tokens @@ -39,6 +34,9 @@ export class OrderValidationUtils { throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); } } else { + const makerZRXBalance = await tokenWrapper.getBalanceAsync(zrxTokenAddress, signedOrder.maker); + const makerZRXAllowance = await tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, signedOrder.maker); + if (fillMakerAmount.greaterThan(makerBalance)) { throw new Error(ExchangeContractErrs.InsufficientMakerBalance); } @@ -58,11 +56,7 @@ export class OrderValidationUtils { senderAddress: string, zrxTokenAddress: string, ): Promise { const takerBalance = await tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); - const takerAllowance = await tokenWrapper.getProxyAllowanceAsync( - signedOrder.takerTokenAddress, senderAddress); - const takerZRXBalance = await tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); - const takerZRXAllowance = await tokenWrapper.getProxyAllowanceAsync( - zrxTokenAddress, senderAddress); + const takerAllowance = await tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, senderAddress); const isTakerTokenZRX = signedOrder.takerTokenAddress === zrxTokenAddress; @@ -75,6 +69,9 @@ export class OrderValidationUtils { throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); } } else { + const takerZRXBalance = await tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); + const takerZRXAllowance = await tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, senderAddress); + if (fillTakerAmount.greaterThan(takerBalance)) { throw new Error(ExchangeContractErrs.InsufficientTakerBalance); } diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 813afdddd..2488e6428 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -303,20 +303,65 @@ describe('ExchangeWrapper', () => { )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerFeeAllowance); }); }); - it('should throw when maker has balance to cover fees or transfer but not both', async () => { - const makerFee = new BigNumber(1); - const takerFee = new BigNumber(1); - const signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( - zrxTokenAddress, takerTokenAddress, makerFee, takerFee, - makerAddress, takerAddress, fillableAmount, feeRecipient, - ); - const balanceToSubtractFromMaker = new BigNumber(1); - await zeroEx.token.transferAsync( - zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, - ); - return expect( - zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress), - ).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerBalance); + describe('should throw on insufficient balance or allowance when makerToken is ZRX', + () => { + const makerFee = new BigNumber(2); + const takerFee = new BigNumber(2); + let signedOrder: SignedOrder; + beforeEach(async () => { + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + zrxTokenAddress, takerTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + }); + it('should throw on insufficient balance when makerToken is ZRX', async () => { + const balanceToSubtractFromMaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerBalance); + }); + it('should throw on insufficient allowance when makerToken is ZRX', async () => { + const oldAllowance = await zeroEx.token.getProxyAllowanceAsync(zrxTokenAddress, makerAddress); + const newAllowanceWhichIsInsufficient = oldAllowance.minus(1); + await zeroEx.token.setProxyAllowanceAsync( + zrxTokenAddress, makerAddress, newAllowanceWhichIsInsufficient); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerAllowance); + }); + }); + describe('should throw on insufficient balance or allowance when takerToken is ZRX', + () => { + const makerFee = new BigNumber(2); + const takerFee = new BigNumber(2); + let signedOrder: SignedOrder; + before(async () => { + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerTokenAddress, zrxTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + }); + it('should throw on insufficient balance when takerToken is ZRX', async () => { + const balanceToSubtractFromTaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerBalance); + }); + it('should throw on insufficient allowance when takerToken is ZRX', async () => { + const oldAllowance = await zeroEx.token.getProxyAllowanceAsync(zrxTokenAddress, takerAddress); + const newAllowanceWhichIsInsufficient = oldAllowance.minus(1); + await zeroEx.token.setProxyAllowanceAsync( + zrxTokenAddress, takerAddress, newAllowanceWhichIsInsufficient); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerAllowance); + }); }); }); describe('successful fills', () => { -- cgit From 4dda6c094949d5fb130a43c49d6ba32c6fd9924a Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 17 Jul 2017 18:11:33 -0700 Subject: Factor out order validation tests --- test/exchange_wrapper_test.ts | 147 ------------------------------ test/order_validation_test.ts | 203 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+), 147 deletions(-) create mode 100644 test/order_validation_test.ts diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 2488e6428..4b4cfeccb 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -204,48 +204,6 @@ describe('ExchangeWrapper', () => { signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.OrderFillExpired); }); - describe('should throw when not enough balance or allowance to fulfill the order', () => { - const balanceToSubtractFromMaker = new BigNumber(3); - const lackingAllowance = new BigNumber(3); - let signedOrder: SignedOrder; - beforeEach('create fillable signed order', async () => { - signedOrder = await fillScenarios.createFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, - ); - }); - it('should throw when taker balance is less than fill amount', async () => { - await zeroEx.token.transferAsync( - takerTokenAddress, takerAddress, coinbase, balanceToSubtractFromMaker, - ); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerBalance); - }); - it('should throw when taker allowance is less than fill amount', async () => { - const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); - await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, - newAllowanceWhichIsLessThanFillAmount); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerAllowance); - }); - it('should throw when maker balance is less than maker fill amount', async () => { - await zeroEx.token.transferAsync( - makerTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, - ); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerBalance); - }); - it('should throw when maker allowance is less than maker fill amount', async () => { - const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); - await zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, - newAllowanceWhichIsLessThanFillAmount); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerAllowance); - }); - }); it('should throw when there a rounding error would have occurred', async () => { const makerAmount = new BigNumber(3); const takerAmount = new BigNumber(5); @@ -258,111 +216,6 @@ describe('ExchangeWrapper', () => { signedOrder, fillTakerAmountThatCausesRoundingError, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.OrderFillRoundingError); }); - describe('should throw when not enough balance or allowance to pay fees', () => { - const makerFee = new BigNumber(2); - const takerFee = new BigNumber(2); - let signedOrder: SignedOrder; - beforeEach('setup', async () => { - signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( - makerTokenAddress, takerTokenAddress, makerFee, takerFee, - makerAddress, takerAddress, fillableAmount, feeRecipient, - ); - }); - it('should throw when maker doesn\'t have enough balance to pay fees', async () => { - const balanceToSubtractFromMaker = new BigNumber(1); - await zeroEx.token.transferAsync( - zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, - ); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerFeeBalance); - }); - it('should throw when maker doesn\'t have enough allowance to pay fees', async () => { - const newAllowanceWhichIsLessThanFees = makerFee.minus(1); - await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, makerAddress, - newAllowanceWhichIsLessThanFees); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerFeeAllowance); - }); - it('should throw when taker doesn\'t have enough balance to pay fees', async () => { - const balanceToSubtractFromTaker = new BigNumber(1); - await zeroEx.token.transferAsync( - zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, - ); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerFeeBalance); - }); - it('should throw when taker doesn\'t have enough allowance to pay fees', async () => { - const newAllowanceWhichIsLessThanFees = makerFee.minus(1); - await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, takerAddress, - newAllowanceWhichIsLessThanFees); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerFeeAllowance); - }); - }); - describe('should throw on insufficient balance or allowance when makerToken is ZRX', - () => { - const makerFee = new BigNumber(2); - const takerFee = new BigNumber(2); - let signedOrder: SignedOrder; - beforeEach(async () => { - signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( - zrxTokenAddress, takerTokenAddress, makerFee, takerFee, - makerAddress, takerAddress, fillableAmount, feeRecipient, - ); - }); - it('should throw on insufficient balance when makerToken is ZRX', async () => { - const balanceToSubtractFromMaker = new BigNumber(1); - await zeroEx.token.transferAsync( - zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, - ); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerBalance); - }); - it('should throw on insufficient allowance when makerToken is ZRX', async () => { - const oldAllowance = await zeroEx.token.getProxyAllowanceAsync(zrxTokenAddress, makerAddress); - const newAllowanceWhichIsInsufficient = oldAllowance.minus(1); - await zeroEx.token.setProxyAllowanceAsync( - zrxTokenAddress, makerAddress, newAllowanceWhichIsInsufficient); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerAllowance); - }); - }); - describe('should throw on insufficient balance or allowance when takerToken is ZRX', - () => { - const makerFee = new BigNumber(2); - const takerFee = new BigNumber(2); - let signedOrder: SignedOrder; - before(async () => { - signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( - makerTokenAddress, zrxTokenAddress, makerFee, takerFee, - makerAddress, takerAddress, fillableAmount, feeRecipient, - ); - }); - it('should throw on insufficient balance when takerToken is ZRX', async () => { - const balanceToSubtractFromTaker = new BigNumber(1); - await zeroEx.token.transferAsync( - zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, - ); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerBalance); - }); - it('should throw on insufficient allowance when takerToken is ZRX', async () => { - const oldAllowance = await zeroEx.token.getProxyAllowanceAsync(zrxTokenAddress, takerAddress); - const newAllowanceWhichIsInsufficient = oldAllowance.minus(1); - await zeroEx.token.setProxyAllowanceAsync( - zrxTokenAddress, takerAddress, newAllowanceWhichIsInsufficient); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerAllowance); - }); - }); }); describe('successful fills', () => { it('should fill a valid order', async () => { diff --git a/test/order_validation_test.ts b/test/order_validation_test.ts new file mode 100644 index 000000000..36c3d8193 --- /dev/null +++ b/test/order_validation_test.ts @@ -0,0 +1,203 @@ +import * as chai from 'chai'; +import {chaiSetup} from './utils/chai_setup'; +import * as Web3 from 'web3'; +import * as BigNumber from 'bignumber.js'; +import promisify = require('es6-promisify'); +import {web3Factory} from './utils/web3_factory'; +import {ZeroEx, SignedOrder, Token, ExchangeContractErrs} from '../src'; +import {TokenUtils} from './utils/token_utils'; +import {BlockchainLifecycle} from './utils/blockchain_lifecycle'; +import {FillScenarios} from './utils/fill_scenarios'; + +chaiSetup.configure(); +const expect = chai.expect; +const blockchainLifecycle = new BlockchainLifecycle(); + +describe('OrderValidationUtils', () => { + let web3: Web3; + let zeroEx: ZeroEx; + let userAddresses: string[]; + let tokens: Token[]; + let tokenUtils: TokenUtils; + let exchangeContractAddress: string; + let zrxTokenAddress: string; + let fillScenarios: FillScenarios; + let makerTokenAddress: string; + let takerTokenAddress: string; + let coinbase: string; + let makerAddress: string; + let takerAddress: string; + let feeRecipient: string; + const fillableAmount = new BigNumber(5); + const fillTakerAmount = new BigNumber(5); + const shouldCheckTransfer = true; + before(async () => { + web3 = web3Factory.create(); + zeroEx = new ZeroEx(web3.currentProvider); + exchangeContractAddress = await zeroEx.exchange.getContractAddressAsync(); + userAddresses = await promisify(web3.eth.getAccounts)(); + [coinbase, makerAddress, takerAddress, feeRecipient] = userAddresses; + tokens = await zeroEx.tokenRegistry.getTokensAsync(); + tokenUtils = new TokenUtils(tokens); + zrxTokenAddress = tokenUtils.getProtocolTokenOrThrow().address; + fillScenarios = new FillScenarios(zeroEx, userAddresses, tokens, zrxTokenAddress, exchangeContractAddress); + const [makerToken, takerToken] = tokenUtils.getNonProtocolTokens(); + makerTokenAddress = makerToken.address; + takerTokenAddress = takerToken.address; + }); + beforeEach(async () => { + await blockchainLifecycle.startAsync(); + }); + afterEach(async () => { + await blockchainLifecycle.revertAsync(); + }); + describe('#validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync', () => { + describe('should throw when not enough balance or allowance to fulfill the order', () => { + const balanceToSubtractFromMaker = new BigNumber(3); + const lackingAllowance = new BigNumber(3); + let signedOrder: SignedOrder; + beforeEach('create fillable signed order', async () => { + signedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + ); + }); + it('should throw when taker balance is less than fill amount', async () => { + await zeroEx.token.transferAsync( + takerTokenAddress, takerAddress, coinbase, balanceToSubtractFromMaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerBalance); + }); + it('should throw when taker allowance is less than fill amount', async () => { + const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); + await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, + newAllowanceWhichIsLessThanFillAmount); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerAllowance); + }); + it('should throw when maker balance is less than maker fill amount', async () => { + await zeroEx.token.transferAsync( + makerTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerBalance); + }); + it('should throw when maker allowance is less than maker fill amount', async () => { + const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); + await zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, + newAllowanceWhichIsLessThanFillAmount); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerAllowance); + }); + }); + describe('should throw when not enough balance or allowance to pay fees', () => { + const makerFee = new BigNumber(2); + const takerFee = new BigNumber(2); + let signedOrder: SignedOrder; + beforeEach('setup', async () => { + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerTokenAddress, takerTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + }); + it('should throw when maker doesn\'t have enough balance to pay fees', async () => { + const balanceToSubtractFromMaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerFeeBalance); + }); + it('should throw when maker doesn\'t have enough allowance to pay fees', async () => { + const newAllowanceWhichIsLessThanFees = makerFee.minus(1); + await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, makerAddress, + newAllowanceWhichIsLessThanFees); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerFeeAllowance); + }); + it('should throw when taker doesn\'t have enough balance to pay fees', async () => { + const balanceToSubtractFromTaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerFeeBalance); + }); + it('should throw when taker doesn\'t have enough allowance to pay fees', async () => { + const newAllowanceWhichIsLessThanFees = makerFee.minus(1); + await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, takerAddress, + newAllowanceWhichIsLessThanFees); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerFeeAllowance); + }); + }); + describe('should throw on insufficient balance or allowance when makerToken is ZRX', + () => { + const makerFee = new BigNumber(2); + const takerFee = new BigNumber(2); + let signedOrder: SignedOrder; + beforeEach(async () => { + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + zrxTokenAddress, takerTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + }); + it('should throw on insufficient balance when makerToken is ZRX', async () => { + const balanceToSubtractFromMaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerBalance); + }); + it('should throw on insufficient allowance when makerToken is ZRX', async () => { + const oldAllowance = await zeroEx.token.getProxyAllowanceAsync(zrxTokenAddress, makerAddress); + const newAllowanceWhichIsInsufficient = oldAllowance.minus(1); + await zeroEx.token.setProxyAllowanceAsync( + zrxTokenAddress, makerAddress, newAllowanceWhichIsInsufficient); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerAllowance); + }); + }); + describe('should throw on insufficient balance or allowance when takerToken is ZRX', + () => { + const makerFee = new BigNumber(2); + const takerFee = new BigNumber(2); + let signedOrder: SignedOrder; + before(async () => { + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerTokenAddress, zrxTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + }); + it('should throw on insufficient balance when takerToken is ZRX', async () => { + const balanceToSubtractFromTaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerBalance); + }); + it('should throw on insufficient allowance when takerToken is ZRX', async () => { + const oldAllowance = await zeroEx.token.getProxyAllowanceAsync(zrxTokenAddress, takerAddress); + const newAllowanceWhichIsInsufficient = oldAllowance.minus(1); + await zeroEx.token.setProxyAllowanceAsync( + zrxTokenAddress, takerAddress, newAllowanceWhichIsInsufficient); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.InsufficientTakerAllowance); + }); + }); + }); +}); -- cgit From c11ba988c78e2d6b7c8e1a57f937c6069337c702 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 17 Jul 2017 22:10:37 -0700 Subject: Fix tests --- test/order_validation_test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/order_validation_test.ts b/test/order_validation_test.ts index 36c3d8193..48938fb6f 100644 --- a/test/order_validation_test.ts +++ b/test/order_validation_test.ts @@ -174,7 +174,7 @@ describe('OrderValidationUtils', () => { const makerFee = new BigNumber(2); const takerFee = new BigNumber(2); let signedOrder: SignedOrder; - before(async () => { + beforeEach(async () => { signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( makerTokenAddress, zrxTokenAddress, makerFee, takerFee, makerAddress, takerAddress, fillableAmount, feeRecipient, -- cgit From 73e8f890b520ceaf16b5ad38f926b67dd3ea148b Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 17 Jul 2017 22:45:34 -0700 Subject: Remove unused imports --- src/0x.ts | 1 - src/contract_wrappers/exchange_wrapper.ts | 6 ------ src/contract_wrappers/proxy_wrapper.ts | 1 - src/contract_wrappers/token_registry_wrapper.ts | 1 - src/types.ts | 1 - src/utils/assert.ts | 1 - test/0x.js_test.ts | 1 - test/artifacts_test.ts | 1 - test/ether_token_wrapper_test.ts | 2 +- test/exchange_wrapper_test.ts | 1 - 10 files changed, 1 insertion(+), 15 deletions(-) diff --git a/src/0x.ts b/src/0x.ts index eaaa67d59..b581d242d 100644 --- a/src/0x.ts +++ b/src/0x.ts @@ -3,7 +3,6 @@ import * as BigNumber from 'bignumber.js'; import {bigNumberConfigs} from './bignumber_config'; import * as ethUtil from 'ethereumjs-util'; import contract = require('truffle-contract'); -import * as Web3 from 'web3'; import findVersions = require('find-versions'); import compareVersions = require('compare-versions'); import {Web3Wrapper} from './web3_wrapper'; diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index f75f8d9bb..65bb37760 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -1,7 +1,6 @@ import * as _ from 'lodash'; import * as BigNumber from 'bignumber.js'; import promisify = require('es6-promisify'); -import * as Web3 from 'web3'; import {Web3Wrapper} from '../web3_wrapper'; import { ECSignature, @@ -27,17 +26,12 @@ import { LogErrorContractEventArgs, LogFillContractEventArgs, LogCancelContractEventArgs, - EventCallback, - ContractEventArg, - ExchangeContractByAddress, - ContractArtifact, } from '../types'; import {assert} from '../utils/assert'; import {utils} from '../utils/utils'; import {eventUtils} from '../utils/event_utils'; import {OrderValidationUtils} from '../utils/order_validation_utils'; import {ContractWrapper} from './contract_wrapper'; -import {ProxyWrapper} from './proxy_wrapper'; import {ecSignatureSchema} from '../schemas/ec_signature_schema'; import {signedOrdersSchema} from '../schemas/signed_orders_schema'; import {subscriptionOptsSchema} from '../schemas/subscription_opts_schema'; diff --git a/src/contract_wrappers/proxy_wrapper.ts b/src/contract_wrappers/proxy_wrapper.ts index c247754f8..9deed5231 100644 --- a/src/contract_wrappers/proxy_wrapper.ts +++ b/src/contract_wrappers/proxy_wrapper.ts @@ -1,5 +1,4 @@ import * as _ from 'lodash'; -import {Web3Wrapper} from '../web3_wrapper'; import {ContractWrapper} from './contract_wrapper'; import * as ProxyArtifacts from '../artifacts/Proxy.json'; import {ProxyContract} from '../types'; diff --git a/src/contract_wrappers/token_registry_wrapper.ts b/src/contract_wrappers/token_registry_wrapper.ts index c9f21e46f..d15e5c4f7 100644 --- a/src/contract_wrappers/token_registry_wrapper.ts +++ b/src/contract_wrappers/token_registry_wrapper.ts @@ -1,7 +1,6 @@ import * as _ from 'lodash'; import {Web3Wrapper} from '../web3_wrapper'; import {Token, TokenRegistryContract, TokenMetadata} from '../types'; -import {assert} from '../utils/assert'; import {ContractWrapper} from './contract_wrapper'; import * as TokenRegistryArtifacts from '../artifacts/TokenRegistry.json'; diff --git a/src/types.ts b/src/types.ts index 974057fed..ee45acf11 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,4 +1,3 @@ -import * as _ from 'lodash'; import * as Web3 from 'web3'; export enum ZeroExError { diff --git a/src/utils/assert.ts b/src/utils/assert.ts index dab796c4e..969209208 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -3,7 +3,6 @@ import * as BigNumber from 'bignumber.js'; import * as Web3 from 'web3'; import {Web3Wrapper} from '../web3_wrapper'; import {SchemaValidator} from './schema_validator'; -import {utils} from './utils'; const HEX_REGEX = /^0x[0-9A-F]*$/i; diff --git a/test/0x.js_test.ts b/test/0x.js_test.ts index 3d9aa105b..03bf21e96 100644 --- a/test/0x.js_test.ts +++ b/test/0x.js_test.ts @@ -6,7 +6,6 @@ import * as BigNumber from 'bignumber.js'; import * as Sinon from 'sinon'; import {ZeroEx, Order} from '../src'; import {constants} from './utils/constants'; -import {assert} from '../src/utils/assert'; import {web3Factory} from './utils/web3_factory'; chaiSetup.configure(); diff --git a/test/artifacts_test.ts b/test/artifacts_test.ts index 04b9ecb2d..65d75f16e 100644 --- a/test/artifacts_test.ts +++ b/test/artifacts_test.ts @@ -3,7 +3,6 @@ import * as chai from 'chai'; import {chaiSetup} from './utils/chai_setup'; import HDWalletProvider = require('truffle-hdwallet-provider'); import {ZeroEx} from '../src'; -import {web3Factory} from './utils/web3_factory'; import {constants} from './utils/constants'; chaiSetup.configure(); diff --git a/test/ether_token_wrapper_test.ts b/test/ether_token_wrapper_test.ts index 5ed800fc7..a8186499c 100644 --- a/test/ether_token_wrapper_test.ts +++ b/test/ether_token_wrapper_test.ts @@ -5,7 +5,7 @@ import * as Web3 from 'web3'; import * as BigNumber from 'bignumber.js'; import promisify = require('es6-promisify'); import {web3Factory} from './utils/web3_factory'; -import {ZeroEx, ZeroExError, Token} from '../src'; +import {ZeroEx, ZeroExError} from '../src'; import {BlockchainLifecycle} from './utils/blockchain_lifecycle'; chaiSetup.configure(); diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 4b4cfeccb..9c3da81b2 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -1,5 +1,4 @@ import 'mocha'; -import * as _ from 'lodash'; import * as chai from 'chai'; import * as Web3 from 'web3'; import * as BigNumber from 'bignumber.js'; -- cgit From 31d068b83ff8d8e9117e410693c3afbc6f8a82ab Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 19 Jul 2017 10:14:31 -0700 Subject: Remove and from names --- src/contract_wrappers/exchange_wrapper.ts | 2 +- src/utils/order_validation_utils.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 65bb37760..88dca82b2 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -662,7 +662,7 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(ExchangeContractErrs.OrderFillExpired); } const zrxTokenAddress = await this._getZRXTokenAddressAsync(signedOrder.exchangeContractAddress); - await OrderValidationUtils.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync( + await OrderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync( this._tokenWrapper, signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, ); diff --git a/src/utils/order_validation_utils.ts b/src/utils/order_validation_utils.ts index 4452a7aef..f85b8dd23 100644 --- a/src/utils/order_validation_utils.ts +++ b/src/utils/order_validation_utils.ts @@ -2,17 +2,17 @@ import {ExchangeContractErrs, SignedOrder} from '../types'; import {TokenWrapper} from '../contract_wrappers/token_wrapper'; export class OrderValidationUtils { - public static async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync( + public static async validateFillOrderBalancesAllowancesThrowIfInvalidAsync( tokenWrapper: TokenWrapper, signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string): Promise { - await OrderValidationUtils.validateFillOrderMakerBalancesAndAllowancesAndThrowIfInvalidAsync( + await OrderValidationUtils.validateFillOrderMakerBalancesAllowancesThrowIfInvalidAsync( tokenWrapper, signedOrder, fillTakerAmount, zrxTokenAddress, ); - await OrderValidationUtils.validateFillOrderTakerBalancesAndAllowancesAndThrowIfInvalidAsync( + await OrderValidationUtils.validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( tokenWrapper, signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, ); } - private static async validateFillOrderMakerBalancesAndAllowancesAndThrowIfInvalidAsync( + private static async validateFillOrderMakerBalancesAllowancesThrowIfInvalidAsync( tokenWrapper: TokenWrapper, signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, zrxTokenAddress: string, ): Promise { @@ -51,7 +51,7 @@ export class OrderValidationUtils { } } } - private static async validateFillOrderTakerBalancesAndAllowancesAndThrowIfInvalidAsync( + private static async validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( tokenWrapper: TokenWrapper, signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string, ): Promise { -- cgit From 4d27b89fe38e0bb617aa05c86735025de9799fe9 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 19 Jul 2017 10:22:09 -0700 Subject: Store tokenWrapper inside of OrdervalidationUtils --- src/contract_wrappers/exchange_wrapper.ts | 6 ++-- src/utils/order_validation_utils.ts | 46 +++++++++++++++++-------------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 88dca82b2..2ddd63422 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -61,6 +61,7 @@ export class ExchangeWrapper extends ContractWrapper { }; private _exchangeContractIfExists?: ExchangeContract; private _exchangeLogEventEmitters: ContractEventEmitter[]; + private _orderValidationUtils: OrderValidationUtils; private _tokenWrapper: TokenWrapper; private static _getOrderAddressesAndValues(order: Order): [OrderAddresses, OrderValues] { const orderAddresses: OrderAddresses = [ @@ -83,6 +84,7 @@ export class ExchangeWrapper extends ContractWrapper { constructor(web3Wrapper: Web3Wrapper, tokenWrapper: TokenWrapper) { super(web3Wrapper); this._tokenWrapper = tokenWrapper; + this._orderValidationUtils = new OrderValidationUtils(tokenWrapper); this._exchangeLogEventEmitters = []; } /** @@ -662,8 +664,8 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(ExchangeContractErrs.OrderFillExpired); } const zrxTokenAddress = await this._getZRXTokenAddressAsync(signedOrder.exchangeContractAddress); - await OrderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync( - this._tokenWrapper, signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, + await this._orderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, ); const wouldRoundingErrorOccur = await this._isRoundingErrorAsync( diff --git a/src/utils/order_validation_utils.ts b/src/utils/order_validation_utils.ts index f85b8dd23..12b7f27fd 100644 --- a/src/utils/order_validation_utils.ts +++ b/src/utils/order_validation_utils.ts @@ -2,22 +2,25 @@ import {ExchangeContractErrs, SignedOrder} from '../types'; import {TokenWrapper} from '../contract_wrappers/token_wrapper'; export class OrderValidationUtils { - public static async validateFillOrderBalancesAllowancesThrowIfInvalidAsync( - tokenWrapper: TokenWrapper, signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - senderAddress: string, zrxTokenAddress: string): Promise { - await OrderValidationUtils.validateFillOrderMakerBalancesAllowancesThrowIfInvalidAsync( - tokenWrapper, signedOrder, fillTakerAmount, zrxTokenAddress, + private tokenWrapper: TokenWrapper; + constructor(tokenWrapper: TokenWrapper) { + this.tokenWrapper = tokenWrapper; + } + public async validateFillOrderBalancesAllowancesThrowIfInvalidAsync( + signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string, + ): Promise { + await this.validateFillOrderMakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, zrxTokenAddress, ); - await OrderValidationUtils.validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( - tokenWrapper, signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, + await this.validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, ); } - private static async validateFillOrderMakerBalancesAllowancesThrowIfInvalidAsync( - tokenWrapper: TokenWrapper, signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - zrxTokenAddress: string, + private async validateFillOrderMakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, zrxTokenAddress: string, ): Promise { - const makerBalance = await tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); - const makerAllowance = await tokenWrapper.getProxyAllowanceAsync( + const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); + const makerAllowance = await this.tokenWrapper.getProxyAllowanceAsync( signedOrder.makerTokenAddress, signedOrder.maker); const isMakerTokenZRX = signedOrder.makerTokenAddress === zrxTokenAddress; @@ -34,8 +37,9 @@ export class OrderValidationUtils { throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); } } else { - const makerZRXBalance = await tokenWrapper.getBalanceAsync(zrxTokenAddress, signedOrder.maker); - const makerZRXAllowance = await tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, signedOrder.maker); + const makerZRXBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, signedOrder.maker); + const makerZRXAllowance = await this.tokenWrapper.getProxyAllowanceAsync( + zrxTokenAddress, signedOrder.maker); if (fillMakerAmount.greaterThan(makerBalance)) { throw new Error(ExchangeContractErrs.InsufficientMakerBalance); @@ -51,12 +55,12 @@ export class OrderValidationUtils { } } } - private static async validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( - tokenWrapper: TokenWrapper, signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - senderAddress: string, zrxTokenAddress: string, + private async validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string, ): Promise { - const takerBalance = await tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); - const takerAllowance = await tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, senderAddress); + const takerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); + const takerAllowance = await this.tokenWrapper.getProxyAllowanceAsync( + signedOrder.takerTokenAddress, senderAddress); const isTakerTokenZRX = signedOrder.takerTokenAddress === zrxTokenAddress; @@ -69,8 +73,8 @@ export class OrderValidationUtils { throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); } } else { - const takerZRXBalance = await tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); - const takerZRXAllowance = await tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, senderAddress); + const takerZRXBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); + const takerZRXAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, senderAddress); if (fillTakerAmount.greaterThan(takerBalance)) { throw new Error(ExchangeContractErrs.InsufficientTakerBalance); -- cgit From 1690aae1cfcd19be23db9f77bd0da0c86141e679 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 19 Jul 2017 10:25:40 -0700 Subject: Simplify order checks --- src/utils/order_validation_utils.ts | 48 ++++++++++++++----------------------- 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/src/utils/order_validation_utils.ts b/src/utils/order_validation_utils.ts index 12b7f27fd..8a737040c 100644 --- a/src/utils/order_validation_utils.ts +++ b/src/utils/order_validation_utils.ts @@ -28,25 +28,19 @@ export class OrderValidationUtils { const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); const fillMakerAmount = fillTakerAmount.div(exchangeRate); - if (isMakerTokenZRX) { - const requiredMakerAmount = fillMakerAmount.plus(signedOrder.makerFee); - if (requiredMakerAmount.greaterThan(makerBalance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerBalance); - } - if (requiredMakerAmount.greaterThan(makerAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); - } - } else { + const requiredMakerAmount = isMakerTokenZRX ? fillMakerAmount.plus(signedOrder.makerFee) : fillMakerAmount; + if (requiredMakerAmount.greaterThan(makerBalance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerBalance); + } + if (requiredMakerAmount.greaterThan(makerAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); + } + + if (!isMakerTokenZRX) { const makerZRXBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, signedOrder.maker); const makerZRXAllowance = await this.tokenWrapper.getProxyAllowanceAsync( zrxTokenAddress, signedOrder.maker); - if (fillMakerAmount.greaterThan(makerBalance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerBalance); - } - if (fillMakerAmount.greaterThan(makerAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); - } if (signedOrder.makerFee.greaterThan(makerZRXBalance)) { throw new Error(ExchangeContractErrs.InsufficientMakerFeeBalance); } @@ -64,24 +58,18 @@ export class OrderValidationUtils { const isTakerTokenZRX = signedOrder.takerTokenAddress === zrxTokenAddress; - if (isTakerTokenZRX) { - const requiredTakerAmount = fillTakerAmount.plus(signedOrder.takerFee); - if (requiredTakerAmount.greaterThan(takerBalance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerBalance); - } - if (requiredTakerAmount.greaterThan(takerAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); - } - } else { + const requiredTakerAmount = isTakerTokenZRX ? fillTakerAmount.plus(signedOrder.takerFee) : fillTakerAmount; + if (requiredTakerAmount.greaterThan(takerBalance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerBalance); + } + if (requiredTakerAmount.greaterThan(takerAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); + } + + if (!isTakerTokenZRX) { const takerZRXBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); const takerZRXAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, senderAddress); - if (fillTakerAmount.greaterThan(takerBalance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerBalance); - } - if (fillTakerAmount.greaterThan(takerAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); - } if (signedOrder.takerFee.greaterThan(takerZRXBalance)) { throw new Error(ExchangeContractErrs.InsufficientTakerFeeBalance); } -- cgit From c6e0acdb041abcb09e3646b7e5df9c7dd5524d6a Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 19 Jul 2017 10:26:10 -0700 Subject: Rearrange imports --- test/order_validation_test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/order_validation_test.ts b/test/order_validation_test.ts index 48938fb6f..09df40ad4 100644 --- a/test/order_validation_test.ts +++ b/test/order_validation_test.ts @@ -1,8 +1,8 @@ import * as chai from 'chai'; -import {chaiSetup} from './utils/chai_setup'; import * as Web3 from 'web3'; import * as BigNumber from 'bignumber.js'; import promisify = require('es6-promisify'); +import {chaiSetup} from './utils/chai_setup'; import {web3Factory} from './utils/web3_factory'; import {ZeroEx, SignedOrder, Token, ExchangeContractErrs} from '../src'; import {TokenUtils} from './utils/token_utils'; -- cgit From 660aa224ca990af27867cdd1523553f50ba50b77 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 19 Jul 2017 10:35:43 -0700 Subject: Indent callbacks --- test/order_validation_test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/order_validation_test.ts b/test/order_validation_test.ts index 09df40ad4..773a23aa7 100644 --- a/test/order_validation_test.ts +++ b/test/order_validation_test.ts @@ -140,7 +140,7 @@ describe('OrderValidationUtils', () => { }); }); describe('should throw on insufficient balance or allowance when makerToken is ZRX', - () => { + () => { const makerFee = new BigNumber(2); const takerFee = new BigNumber(2); let signedOrder: SignedOrder; @@ -170,7 +170,7 @@ describe('OrderValidationUtils', () => { }); }); describe('should throw on insufficient balance or allowance when takerToken is ZRX', - () => { + () => { const makerFee = new BigNumber(2); const takerFee = new BigNumber(2); let signedOrder: SignedOrder; -- cgit