Merge pull request #109 from 0xProject/fees-balance-allowance-validation

Fees balance allowance validation

1 files changed, 6 insertions, 69 deletions

diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts
index 9cc29e286..2ddd63422 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,16 +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';
@@ -66,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 = [
@@ -88,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 = [];
     }
     /**
@@ -667,8 +664,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 this._orderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync(
+            signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress,
+        );
 
         const wouldRoundingErrorOccur = await this._isRoundingErrorAsync(
             signedOrder.takerTokenAmount, fillTakerAmount, signedOrder.makerTokenAmount,
@@ -702,67 +700,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<void> {
-
-        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)) {