Add check for ROUNDING_ERROR and test for it

4 files changed, 61 insertions, 18 deletions

diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts
index 3fb187de2..fe5fc3d78 100644
--- a/src/contract_wrappers/exchange_wrapper.ts
+++ b/src/contract_wrappers/exchange_wrapper.ts
@@ -154,6 +154,10 @@ 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);
+        }
     }
     private throwErrorLogsAsErrors(logs: ContractEvent[]): void {
         const errEvent = _.find(logs, {event: 'LogError'});
@@ -163,6 +167,18 @@ export class ExchangeWrapper extends ContractWrapper {
             throw new Error(errMessage);
         }
     }
+    private async isRoundingErrorAsync(takerTokenAmount: BigNumber.BigNumber,
+                                       fillTakerAmountInBaseUnits: BigNumber.BigNumber,
+                                       makerTokenAmount: BigNumber.BigNumber): Promise<boolean> {
+        const exchangeInstance = await this.getExchangeContractAsync();
+        const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync();
+        const isRoundingError = await exchangeInstance.isRoundingError.call(
+            takerTokenAmount, fillTakerAmountInBaseUnits, makerTokenAmount, {
+                from: senderAddress,
+            },
+        );
+        return isRoundingError;
+    }
     private async getExchangeContractAsync(): Promise<ExchangeContract> {
         if (!_.isUndefined(this.exchangeContractIfExists)) {
             return this.exchangeContractIfExists;
diff --git a/src/types.ts b/src/types.ts
index f80f98dc4..73c448b85 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -34,6 +34,10 @@ export type OrderValues = [BigNumber.BigNumber, BigNumber.BigNumber, BigNumber.B
 
 export interface ExchangeContract {
     isValidSignature: any;
+    isRoundingError: {
+        call: (takerTokenAmount: BigNumber.BigNumber, fillTakerAmountInBaseUnits: BigNumber.BigNumber,
+               makerTokenAmount: BigNumber.BigNumber, txOpts: TxOpts) => Promise<boolean>;
+    };
     fill: {
         (orderAddresses: OrderAddresses, orderValues: OrderValues, fillAmount: BigNumber.BigNumber,
          shouldCheckTransfer: boolean, v: number, r: string, s: string, txOpts: TxOpts): ContractResponse;
@@ -93,6 +97,7 @@ export const FillOrderValidationErrs = strEnum([
     'NOT_ENOUGH_TAKER_ALLOWANCE',
     'NOT_ENOUGH_MAKER_BALANCE',
     'NOT_ENOUGH_MAKER_ALLOWANCE',
+    'ROUNDING_ERROR',
 ]);
 export type FillOrderValidationErrs = keyof typeof FillOrderValidationErrs;
 
diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts
index 66ba167cc..7f8bad377 100644
--- a/test/exchange_wrapper_test.ts
+++ b/test/exchange_wrapper_test.ts
@@ -127,7 +127,7 @@ describe('ExchangeWrapper', () => {
         describe('failed fills', () => {
             it('should throw when the fill amount is zero', async () => {
                 const fillableAmount = new BigNumber(5);
-                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
                     makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
                 );
                 const zeroFillAmount = new BigNumber(0);
@@ -138,7 +138,7 @@ describe('ExchangeWrapper', () => {
             });
             it('should throw when sender is not a taker', async () => {
                 const fillableAmount = new BigNumber(5);
-                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
                     makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
                 );
                 return expect(zeroEx.exchange.fillOrderAsync(
@@ -148,7 +148,7 @@ describe('ExchangeWrapper', () => {
             it('should throw when order is expired', async () => {
                 const expirationInPast = new BigNumber(42);
                 const fillableAmount = new BigNumber(5);
-                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
                     makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast,
                 );
                 zeroEx.setTransactionSenderAccount(takerAddress);
@@ -158,7 +158,7 @@ describe('ExchangeWrapper', () => {
             });
             it('should throw when taker balance is less than fill amount', async () => {
                 const fillableAmount = new BigNumber(5);
-                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
                     makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
                 );
                 zeroEx.setTransactionSenderAccount(takerAddress);
@@ -169,7 +169,7 @@ describe('ExchangeWrapper', () => {
             });
             it('should throw when taker allowance is less than fill amount', async () => {
                 const fillableAmount = new BigNumber(5);
-                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
                     makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
                 );
                 const newAllowanceWhichIsLessThanFillAmount = fillTakerAmountInBaseUnits.minus(1);
@@ -182,7 +182,7 @@ describe('ExchangeWrapper', () => {
             });
             it('should throw when maker balance is less than maker fill amount', async () => {
                 const fillableAmount = new BigNumber(5);
-                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
                     makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
                 );
                 const lackingMakerBalance = new BigNumber(3);
@@ -194,7 +194,7 @@ describe('ExchangeWrapper', () => {
             });
             it('should throw when maker allowance is less than maker fill amount', async () => {
                 const fillableAmount = new BigNumber(5);
-                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
                     makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
                 );
                 const newAllowanceWhichIsLessThanFillAmount = fillTakerAmountInBaseUnits.minus(1);
@@ -205,11 +205,24 @@ describe('ExchangeWrapper', () => {
                     signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer,
                 )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE);
             });
+            it('should throw when there would be a rounding error', async () => {
+                const makerFillableAmount = new BigNumber(3);
+                const takerFillableAmount = new BigNumber(5);
+                const signedOrder = await fillScenarios.createAsymetricFillableSignedOrderAsync(
+                    makerTokenAddress, takerTokenAddress, makerAddress, takerAddress,
+                    makerFillableAmount, takerFillableAmount,
+                );
+                const fillTakerAmountInBaseUnitsThatCausesRoundingError = new BigNumber(3);
+                zeroEx.setTransactionSenderAccount(takerAddress);
+                return expect(zeroEx.exchange.fillOrderAsync(
+                    signedOrder, fillTakerAmountInBaseUnitsThatCausesRoundingError, shouldCheckTransfer,
+                )).to.be.rejectedWith(FillOrderValidationErrs.ROUNDING_ERROR);
+            });
         });
         describe('successful fills', () => {
             it('should fill the valid order', async () => {
                 const fillableAmount = new BigNumber(5);
-                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
                     makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
                 );
 
@@ -234,7 +247,7 @@ describe('ExchangeWrapper', () => {
             });
             it('should partially fill the valid order', async () => {
                 const fillableAmount = new BigNumber(5);
-                const signedOrder = await fillScenarios.createAFillableSignedOrderAsync(
+                const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
                     makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
                 );
                 const partialFillAmount = new BigNumber(3);
diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts
index 3b66937e6..568b11ef8 100644
--- a/test/utils/fill_scenarios.ts
+++ b/test/utils/fill_scenarios.ts
@@ -14,20 +14,29 @@ export class FillScenarios {
         this.tokens = tokens;
         this.coinBase = userAddresses[0];
     }
-    public async createAFillableSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string,
-                                                 makerAddress: string, takerAddress: string,
-                                                 fillableAmount: BigNumber.BigNumber,
-                                                 expirationUnixTimestampSec?: BigNumber.BigNumber):
+    public async createFillableSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string,
+                                                makerAddress: string, takerAddress: string,
+                                                fillableAmount: BigNumber.BigNumber,
+                                                expirationUnixTimestampSec?: BigNumber.BigNumber):
                                            Promise<SignedOrder> {
-        await this.zeroEx.token.transferAsync(makerTokenAddress, this.coinBase, makerAddress, fillableAmount);
-        await this.zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, fillableAmount);
-        await this.zeroEx.token.transferAsync(takerTokenAddress, this.coinBase, takerAddress, fillableAmount);
-        await this.zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, fillableAmount);
+        return this.createAsymetricFillableSignedOrderAsync(
+            makerTokenAddress, takerTokenAddress, makerAddress, takerAddress,
+            fillableAmount, fillableAmount, expirationUnixTimestampSec,
+        );
+    }
+    public async createAsymetricFillableSignedOrderAsync(
+        makerTokenAddress: string, takerTokenAddress: string, makerAddress: string, takerAddress: string,
+        makerFillableAmount: BigNumber.BigNumber, takerFillableAmount: BigNumber.BigNumber,
+        expirationUnixTimestampSec?: BigNumber.BigNumber): Promise<SignedOrder> {
+        await this.zeroEx.token.transferAsync(makerTokenAddress, this.coinBase, makerAddress, makerFillableAmount);
+        await this.zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, makerFillableAmount);
+        await this.zeroEx.token.transferAsync(takerTokenAddress, this.coinBase, takerAddress, takerFillableAmount);
+        await this.zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, takerFillableAmount);
 
         const transactionSenderAccount = await this.zeroEx.getTransactionSenderAccountIfExistsAsync();
         this.zeroEx.setTransactionSenderAccount(makerAddress);
         const signedOrder = await orderFactory.createSignedOrderAsync(this.zeroEx, makerAddress,
-            takerAddress, fillableAmount, makerTokenAddress, fillableAmount, takerTokenAddress,
+            takerAddress, makerFillableAmount, makerTokenAddress, takerFillableAmount, takerTokenAddress,
             expirationUnixTimestampSec);
         this.zeroEx.setTransactionSenderAccount(transactionSenderAccount as string);
         return signedOrder;