[Order-utils] Order is valid when maker amount is very small

Previously our min fillable calculation would throw a rounding error
when encountering a valid order (with a small maker amount). This was
inconsistent with the on-chain logic which allowed this order to be
filled.

3 files changed, 143 insertions, 19 deletions

diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json
index 6b49d2ee6..86f0da65a 100644
--- a/packages/order-utils/CHANGELOG.json
+++ b/packages/order-utils/CHANGELOG.json
@@ -3,6 +3,10 @@
         "version": "1.0.1-rc.4",
         "changes": [
             {
+                "note": "Remove rounding error being thrown when maker amount is very small",
+                "pr": 959
+            },
+            {
                 "note": "Added rateUtils and sortingUtils",
                 "pr": 953
             },
diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts
index 189bf4180..7974d5d0b 100644
--- a/packages/order-utils/src/order_state_utils.ts
+++ b/packages/order-utils/src/order_state_utils.ts
@@ -11,6 +11,7 @@ import { BigNumber } from '@0xproject/utils';
 import { AbstractBalanceAndProxyAllowanceFetcher } from './abstract/abstract_balance_and_proxy_allowance_fetcher';
 import { AbstractOrderFilledCancelledFetcher } from './abstract/abstract_order_filled_cancelled_fetcher';
 import { orderHashUtils } from './order_hash';
+import { OrderValidationUtils } from './order_validation_utils';
 import { RemainingFillableCalculator } from './remaining_fillable_calculator';
 import { utils } from './utils';
 
@@ -22,10 +23,9 @@ interface SidedOrderRelevantState {
     traderFeeProxyAllowance: BigNumber;
     filledTakerAssetAmount: BigNumber;
     remainingFillableAssetAmount: BigNumber;
+    isOrderCancelled: boolean;
 }
 
-const ACCEPTABLE_RELATIVE_ROUNDING_ERROR = 0.0001;
-
 export class OrderStateUtils {
     private readonly _balanceAndProxyAllowanceFetcher: AbstractBalanceAndProxyAllowanceFetcher;
     private readonly _orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher;
@@ -34,6 +34,9 @@ export class OrderStateUtils {
         sidedOrderRelevantState: SidedOrderRelevantState,
     ): void {
         const isMakerSide = sidedOrderRelevantState.isMakerSide;
+        if (sidedOrderRelevantState.isOrderCancelled) {
+            throw new Error(ExchangeContractErrs.OrderAlreadyCancelledOrFilled);
+        }
         const availableTakerAssetAmount = signedOrder.takerAssetAmount.minus(
             sidedOrderRelevantState.filledTakerAssetAmount,
         );
@@ -71,23 +74,15 @@ export class OrderStateUtils {
                 );
             }
         }
-
-        let minFillableTakerAssetAmountWithinNoRoundingErrorRange;
-        if (isMakerSide) {
-            minFillableTakerAssetAmountWithinNoRoundingErrorRange = signedOrder.takerAssetAmount
-                .dividedBy(ACCEPTABLE_RELATIVE_ROUNDING_ERROR)
-                .dividedBy(signedOrder.makerAssetAmount);
-        } else {
-            minFillableTakerAssetAmountWithinNoRoundingErrorRange = signedOrder.makerAssetAmount
-                .dividedBy(ACCEPTABLE_RELATIVE_ROUNDING_ERROR)
-                .dividedBy(signedOrder.takerAssetAmount);
-        }
-
-        if (
-            sidedOrderRelevantState.remainingFillableAssetAmount.lessThan(
-                minFillableTakerAssetAmountWithinNoRoundingErrorRange,
-            )
-        ) {
+        const remainingTakerAssetAmount = signedOrder.takerAssetAmount.minus(
+            sidedOrderRelevantState.filledTakerAssetAmount,
+        );
+        const isRoundingError = OrderValidationUtils.isRoundingError(
+            remainingTakerAssetAmount,
+            signedOrder.takerAssetAmount,
+            signedOrder.makerAssetAmount,
+        );
+        if (isRoundingError) {
             throw new Error(ExchangeContractErrs.OrderFillRoundingError);
         }
     }
@@ -101,6 +96,7 @@ export class OrderStateUtils {
     public async getOpenOrderStateAsync(signedOrder: SignedOrder): Promise<OrderState> {
         const orderRelevantState = await this.getOpenOrderRelevantStateAsync(signedOrder);
         const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
+        const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(orderHash);
         const sidedOrderRelevantState = {
             isMakerSide: true,
             traderBalance: orderRelevantState.makerBalance,
@@ -109,6 +105,7 @@ export class OrderStateUtils {
             traderFeeProxyAllowance: orderRelevantState.makerFeeProxyAllowance,
             filledTakerAssetAmount: orderRelevantState.filledTakerAssetAmount,
             remainingFillableAssetAmount: orderRelevantState.remainingFillableMakerAssetAmount,
+            isOrderCancelled,
         };
         try {
             OrderStateUtils._validateIfOrderIsValid(signedOrder, sidedOrderRelevantState);
@@ -278,6 +275,7 @@ export class OrderStateUtils {
             traderFeeProxyAllowance,
             filledTakerAssetAmount,
             remainingFillableAssetAmount,
+            isOrderCancelled,
         };
         return sidedOrderRelevantState;
     }
diff --git a/packages/order-utils/test/order_state_utils_test.ts b/packages/order-utils/test/order_state_utils_test.ts
new file mode 100644
index 000000000..9292c40a4
--- /dev/null
+++ b/packages/order-utils/test/order_state_utils_test.ts
@@ -0,0 +1,122 @@
+import { BigNumber } from '@0xproject/utils';
+import * as chai from 'chai';
+import 'mocha';
+
+import { AbstractBalanceAndProxyAllowanceFetcher, AbstractOrderFilledCancelledFetcher, OrderStateUtils } from '../src';
+
+import { chaiSetup } from './utils/chai_setup';
+import { testOrderFactory } from './utils/test_order_factory';
+
+chaiSetup.configure();
+const expect = chai.expect;
+
+describe('OrderStateUtils', () => {
+    describe('#getOpenOrderStateAsync', () => {
+        const buildMockBalanceFetcher = (takerBalance: BigNumber): AbstractBalanceAndProxyAllowanceFetcher => {
+            const balanceFetcher = {
+                async getBalanceAsync(_assetData: string, _userAddress: string): Promise<BigNumber> {
+                    return takerBalance;
+                },
+                async getProxyAllowanceAsync(_assetData: string, _userAddress: string): Promise<BigNumber> {
+                    return takerBalance;
+                },
+            };
+            return balanceFetcher;
+        };
+        const buildMockOrderFilledFetcher = (
+            filledAmount: BigNumber = new BigNumber(0),
+            cancelled: boolean = false,
+        ): AbstractOrderFilledCancelledFetcher => {
+            const orderFetcher = {
+                async getFilledTakerAmountAsync(_orderHash: string): Promise<BigNumber> {
+                    return filledAmount;
+                },
+                async isOrderCancelledAsync(_orderHash: string): Promise<boolean> {
+                    return cancelled;
+                },
+                getZRXAssetData(): string {
+                    return '';
+                },
+            };
+            return orderFetcher;
+        };
+        it('should have valid order state if order can be fully filled with small maker amount', async () => {
+            const makerAssetAmount = new BigNumber(10);
+            const takerAssetAmount = new BigNumber(10000000000000000);
+            const takerBalance = takerAssetAmount;
+            const orderFilledAmount = new BigNumber(0);
+            const mockBalanceFetcher = buildMockBalanceFetcher(takerBalance);
+            const mockOrderFilledFetcher = buildMockOrderFilledFetcher(orderFilledAmount);
+            const [signedOrder] = testOrderFactory.generateTestSignedOrders(
+                {
+                    makerAssetAmount,
+                    takerAssetAmount,
+                },
+                1,
+            );
+
+            const orderStateUtils = new OrderStateUtils(mockBalanceFetcher, mockOrderFilledFetcher);
+            const orderState = await orderStateUtils.getOpenOrderStateAsync(signedOrder);
+            expect(orderState.isValid).to.eq(true);
+        });
+        it('should be invalid when an order is partially filled where only a rounding error remains', async () => {
+            const makerAssetAmount = new BigNumber(1001);
+            const takerAssetAmount = new BigNumber(3);
+            const takerBalance = takerAssetAmount;
+            const orderFilledAmount = new BigNumber(2);
+            const mockBalanceFetcher = buildMockBalanceFetcher(takerBalance);
+            const mockOrderFilledFetcher = buildMockOrderFilledFetcher(orderFilledAmount);
+            const [signedOrder] = testOrderFactory.generateTestSignedOrders(
+                {
+                    makerAssetAmount,
+                    takerAssetAmount,
+                },
+                1,
+            );
+
+            const orderStateUtils = new OrderStateUtils(mockBalanceFetcher, mockOrderFilledFetcher);
+            const orderState = await orderStateUtils.getOpenOrderStateAsync(signedOrder);
+            expect(orderState.isValid).to.eq(false);
+        });
+        it('should be invalid when an order is cancelled', async () => {
+            const makerAssetAmount = new BigNumber(1000);
+            const takerAssetAmount = new BigNumber(2);
+            const takerBalance = takerAssetAmount;
+            const orderFilledAmount = new BigNumber(0);
+            const isCancelled = true;
+            const mockBalanceFetcher = buildMockBalanceFetcher(takerBalance);
+            const mockOrderFilledFetcher = buildMockOrderFilledFetcher(orderFilledAmount, isCancelled);
+            const [signedOrder] = testOrderFactory.generateTestSignedOrders(
+                {
+                    makerAssetAmount,
+                    takerAssetAmount,
+                },
+                1,
+            );
+
+            const orderStateUtils = new OrderStateUtils(mockBalanceFetcher, mockOrderFilledFetcher);
+            const orderState = await orderStateUtils.getOpenOrderStateAsync(signedOrder);
+            expect(orderState.isValid).to.eq(false);
+        });
+        it('should be invalid when an order is fully filled', async () => {
+            const makerAssetAmount = new BigNumber(1000);
+            const takerAssetAmount = new BigNumber(2);
+            const takerBalance = takerAssetAmount;
+            const orderFilledAmount = takerAssetAmount;
+            const isCancelled = false;
+            const mockBalanceFetcher = buildMockBalanceFetcher(takerBalance);
+            const mockOrderFilledFetcher = buildMockOrderFilledFetcher(orderFilledAmount, isCancelled);
+            const [signedOrder] = testOrderFactory.generateTestSignedOrders(
+                {
+                    makerAssetAmount,
+                    takerAssetAmount,
+                },
+                1,
+            );
+
+            const orderStateUtils = new OrderStateUtils(mockBalanceFetcher, mockOrderFilledFetcher);
+            const orderState = await orderStateUtils.getOpenOrderStateAsync(signedOrder);
+            expect(orderState.isValid).to.eq(false);
+        });
+    });
+});