Add tests and checks for fees balances and allowances

3 files changed, 96 insertions, 10 deletions

diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts
index fe5fc3d78..fa5c16485 100644
--- a/src/contract_wrappers/exchange_wrapper.ts
+++ b/src/contract_wrappers/exchange_wrapper.ts
@@ -74,9 +74,9 @@ export class ExchangeWrapper extends ContractWrapper {
         assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer);
 
         const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync();
-        await this.validateFillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, senderAddress);
-
         const exchangeInstance = await this.getExchangeContractAsync();
+        const zrxTokenAddress = await exchangeInstance.ZRX.call();
+        await this.validateFillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, senderAddress, zrxTokenAddress);
 
         const orderAddresses: OrderAddresses = [
             signedOrder.maker,
@@ -121,7 +121,7 @@ export class ExchangeWrapper extends ContractWrapper {
         this.throwErrorLogsAsErrors(response.logs);
     }
     private async validateFillOrderAsync(signedOrder: SignedOrder, fillTakerAmountInBaseUnits: BigNumber.BigNumber,
-                                         senderAddress: string) {
+                                         senderAddress: string, zrxTokenAddress: string): Promise<void> {
         if (fillTakerAmountInBaseUnits.eq(0)) {
             throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO);
         }
@@ -131,13 +131,32 @@ export class ExchangeWrapper extends ContractWrapper {
         if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) {
             throw new Error(FillOrderValidationErrs.EXPIRED);
         }
+
+        await this.validateFillOrderBalancesAndAllowancesAsync(signedOrder, fillTakerAmountInBaseUnits,
+                                                               senderAddress, zrxTokenAddress);
+
+        if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmountInBaseUnits,
+                                            signedOrder.makerTokenAmount)) {
+            throw new Error(FillOrderValidationErrs.ROUNDING_ERROR);
+        }
+    }
+    private async validateFillOrderBalancesAndAllowancesAsync(signedOrder: SignedOrder,
+                                                              fillTakerAmountInBaseUnits: BigNumber.BigNumber,
+                                                              senderAddress: string,
+                                                              zrxTokenAddress: string): Promise<void> {
+        // TODO: There is a possibility that the user might have enough funds
+        // to fulfill the order or pay fees but not both. This will happen if
+        // makerToken === zrxToken || makerToken === zrxToken
+        // We don't check it for now. The contract checks it and throws.
+
         const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress,
-                                                                        signedOrder.maker);
+            signedOrder.maker);
         const takerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress);
         const makerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress,
-                                                                              signedOrder.maker);
+            signedOrder.maker);
         const takerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress,
-                                                                              senderAddress);
+            senderAddress);
+
         // How many taker tokens would you get for 1 maker token;
         const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount);
         const fillMakerAmountInBaseUnits = fillTakerAmountInBaseUnits.div(exchangeRate);
@@ -154,9 +173,26 @@ export class ExchangeWrapper extends ContractWrapper {
         if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) {
             throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE);
         }
-        if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmountInBaseUnits,
-                                            signedOrder.makerTokenAmount)) {
-            throw new Error(FillOrderValidationErrs.ROUNDING_ERROR);
+
+        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(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_BALANCE);
+        }
+        if (signedOrder.takerFee.greaterThan(takerFeeAllowance)) {
+            throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE);
+        }
+        if (signedOrder.makerFee.greaterThan(makerFeeBalance)) {
+            throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_BALANCE);
+        }
+        if (signedOrder.makerFee.greaterThan(makerFeeAllowance)) {
+            throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE);
         }
     }
     private throwErrorLogsAsErrors(logs: ContractEvent[]): void {
diff --git a/src/types.ts b/src/types.ts
index 73c448b85..7ec3b1cf2 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -44,6 +44,9 @@ export interface ExchangeContract {
         estimateGas: (orderAddresses: OrderAddresses, orderValues: OrderValues, fillAmount: BigNumber.BigNumber,
                       shouldCheckTransfer: boolean, v: number, r: string, s: string, txOpts: TxOpts) => number;
     };
+    ZRX: {
+        call: () => Promise<string>;
+    };
 }
 
 export interface TokenContract {
@@ -97,6 +100,10 @@ export const FillOrderValidationErrs = strEnum([
     'NOT_ENOUGH_TAKER_ALLOWANCE',
     'NOT_ENOUGH_MAKER_BALANCE',
     'NOT_ENOUGH_MAKER_ALLOWANCE',
+    'NOT_ENOUGH_TAKER_FEE_BALANCE',
+    'NOT_ENOUGH_TAKER_FEE_ALLOWANCE',
+    'NOT_ENOUGH_MAKER_FEE_BALANCE',
+    'NOT_ENOUGH_MAKER_FEE_ALLOWANCE',
     'ROUNDING_ERROR',
 ]);
 export type FillOrderValidationErrs = keyof typeof FillOrderValidationErrs;
diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts
index ecb5a408b..6f4105e1e 100644
--- a/test/exchange_wrapper_test.ts
+++ b/test/exchange_wrapper_test.ts
@@ -9,7 +9,7 @@ import promisify = require('es6-promisify');
 import {web3Factory} from './utils/web3_factory';
 import {ZeroEx} from '../src/0x.js';
 import {BlockchainLifecycle} from './utils/blockchain_lifecycle';
-import {FillOrderValidationErrs, Token} from '../src/types';
+import {FillOrderValidationErrs, SignedOrder, Token} from '../src/types';
 import {FillScenarios} from './utils/fill_scenarios';
 import {TokenUtils} from './utils/token_utils';
 
@@ -222,6 +222,49 @@ describe('ExchangeWrapper', () => {
                     signedOrder, fillTakerAmountInBaseUnitsThatCausesRoundingError, shouldCheckTransfer,
                 )).to.be.rejectedWith(FillOrderValidationErrs.ROUNDING_ERROR);
             });
+            describe('should raise when not enough balance or allowance to pay fees', () => {
+                const fillableAmount = new BigNumber(5);
+                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,
+                    );
+                    zeroEx.setTransactionSenderAccount(takerAddress);
+                });
+                it('should throw when maker doesn\'t have enough balance to pay fees', async () => {
+                    const lackingBalance = new BigNumber(1);
+                    await zeroEx.token.transferAsync(zrxTokenAddress, makerAddress, coinBase, lackingBalance);
+                    return expect(zeroEx.exchange.fillOrderAsync(
+                        signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer,
+                    )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_BALANCE);
+                });
+                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, fillTakerAmountInBaseUnits, shouldCheckTransfer,
+                    )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE);
+                });
+                it('should throw when taker doesn\'t have enough balance to pay fees', async () => {
+                    const lackingBalance = new BigNumber(1);
+                    await zeroEx.token.transferAsync(zrxTokenAddress, takerAddress, coinBase, lackingBalance);
+                    return expect(zeroEx.exchange.fillOrderAsync(
+                        signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer,
+                    )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_BALANCE);
+                });
+                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, fillTakerAmountInBaseUnits, shouldCheckTransfer,
+                    )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE);
+                });
+            });
         });
         describe('successful fills', () => {
             it('should fill the valid order', async () => {