Unit test edge case for ZRX and ZRX partial fill

3 files changed, 59 insertions, 44 deletions

diff --git a/packages/0x.js/src/order_watcher/remaining_fillable_calculator.ts b/packages/0x.js/src/order_watcher/remaining_fillable_calculator.ts
index fe373eae4..45f60ada3 100644
--- a/packages/0x.js/src/order_watcher/remaining_fillable_calculator.ts
+++ b/packages/0x.js/src/order_watcher/remaining_fillable_calculator.ts
@@ -10,12 +10,12 @@ export class RemainingFillableCalculator {
     private _remainingMakerTokenAmount: BigNumber;
     private _remainingMakerFeeAmount: BigNumber;
     constructor(signedOrder: SignedOrder,
-                zrxAddress: string,
+                isMakerTokenZRX: boolean,
                 transferrableMakerTokenAmount: BigNumber,
                 transferrableMakerFeeTokenAmount: BigNumber,
                 remainingMakerTokenAmount: BigNumber) {
         this._signedOrder = signedOrder;
-        this._isMakerTokenZRX = signedOrder.makerTokenAddress === zrxAddress;
+        this._isMakerTokenZRX = isMakerTokenZRX;
         this._transferrableMakerTokenAmount = transferrableMakerTokenAmount;
         this._transferrableMakerFeeTokenAmount = transferrableMakerFeeTokenAmount;
         this._remainingMakerTokenAmount = remainingMakerTokenAmount;
@@ -51,18 +51,21 @@ export class RemainingFillableCalculator {
 
     private calculatePartiallyFillableMakerTokenAmount(): BigNumber {
         const orderToFeeRatio = this._signedOrder.makerTokenAmount.dividedToIntegerBy(this._signedOrder.makerFee);
-        const fillableTimesInFeeToken = BigNumber.min(this._transferrableMakerFeeTokenAmount,
-                                                      this._remainingMakerFeeAmount);
-        let fillableTimesInMakerToken = this._transferrableMakerTokenAmount.dividedToIntegerBy(orderToFeeRatio);
+        // Maximum number of times the Maker can fill the order, given the fees
+        const fillableTimesInFeeTokenUnits = BigNumber.min(this._transferrableMakerFeeTokenAmount,
+                                                           this._remainingMakerFeeAmount);
+        // Maximum number of times the Maker can fill the order, given the Maker Token Balance
+        let fillableTimesInMakerTokenUnits = this._transferrableMakerTokenAmount.dividedToIntegerBy(orderToFeeRatio);
         if (this._isMakerTokenZRX) {
-            // when zrx == maker token transferrable maker == transfer
             const totalZRXTokenPooled = this._transferrableMakerTokenAmount;
-            fillableTimesInMakerToken = totalZRXTokenPooled.dividedToIntegerBy(
-                                                             orderToFeeRatio.plus(new BigNumber(1)));
+            // The purchasing power here is less as the tokens are taken from the same Pool
+            // For every one number of fills, we have to take an extra ZRX out of the pool
+            fillableTimesInMakerTokenUnits = totalZRXTokenPooled.dividedToIntegerBy(
+                                                                     orderToFeeRatio.plus(new BigNumber(1)));
 
         }
-        const partiallyFillableMakerTokenAmount = fillableTimesInMakerToken.times(orderToFeeRatio);
-        const partiallyFillableFeeTokenAmount = fillableTimesInFeeToken.times(orderToFeeRatio);
+        const partiallyFillableMakerTokenAmount = fillableTimesInMakerTokenUnits.times(orderToFeeRatio);
+        const partiallyFillableFeeTokenAmount = fillableTimesInFeeTokenUnits.times(orderToFeeRatio);
         return BigNumber.min(partiallyFillableMakerTokenAmount, partiallyFillableFeeTokenAmount);
     }
 }
diff --git a/packages/0x.js/src/utils/order_state_utils.ts b/packages/0x.js/src/utils/order_state_utils.ts
index 9ff26a7f1..1d8f02a18 100644
--- a/packages/0x.js/src/utils/order_state_utils.ts
+++ b/packages/0x.js/src/utils/order_state_utils.ts
@@ -82,8 +82,9 @@ export class OrderStateUtils {
         const transferrableMakerTokenAmount = BigNumber.min([makerProxyAllowance, makerBalance]);
         const transferrableFeeTokenAmount = BigNumber.min([makerFeeProxyAllowance, makerFeeBalance]);
 
+        const isMakerTokenZRX = signedOrder.makerTokenAddress === zrxTokenAddress;
         const remainingFillableCalculator = new RemainingFillableCalculator(signedOrder,
-                                                zrxTokenAddress,
+                                                isMakerTokenZRX,
                                                 transferrableMakerTokenAmount,
                                                 transferrableFeeTokenAmount,
                                                 remainingMakerTokenAmount);
diff --git a/packages/0x.js/test/remaining_fillable_calculator_test.ts b/packages/0x.js/test/remaining_fillable_calculator_test.ts
index 5d7cd9277..5b7fd63f1 100644
--- a/packages/0x.js/test/remaining_fillable_calculator_test.ts
+++ b/packages/0x.js/test/remaining_fillable_calculator_test.ts
@@ -13,26 +13,26 @@ const expect = chai.expect;
 describe.only('RemainingFillableCalculator', () => {
     let calculator: RemainingFillableCalculator;
     let signedOrder: SignedOrder;
-    let makerToken: string;
-    let takerToken: string;
-    let zrxToken: string;
     let transferrableMakerTokenAmount: BigNumber;
     let transferrableMakerFeeTokenAmount: BigNumber;
     let remainingMakerTokenAmount: BigNumber;
     let makerAmount: BigNumber;
     let takerAmount: BigNumber;
     let makerFee: BigNumber;
+    let isMakerTokenZRX: boolean;
+    const makerToken: string = '0x1';
+    const takerToken: string = '0x2';
+    const decimals: number = 4;
     const zero: BigNumber = new BigNumber(0);
     const zeroAddress = '0x0';
     const signature: ECSignature = { v: 27, r: '', s: ''};
     beforeEach(async () => {
-        [makerToken, takerToken, zrxToken] = ['0x1', '0x2', '0x3'];
-        [makerAmount, takerAmount, makerFee] = [ZeroEx.toBaseUnitAmount(new BigNumber(50), 18),
-                                                ZeroEx.toBaseUnitAmount(new BigNumber(5), 18),
-                                                ZeroEx.toBaseUnitAmount(new BigNumber(1), 18)];
+        [makerAmount, takerAmount, makerFee] = [ZeroEx.toBaseUnitAmount(new BigNumber(50), decimals),
+                                                ZeroEx.toBaseUnitAmount(new BigNumber(5), decimals),
+                                                ZeroEx.toBaseUnitAmount(new BigNumber(1), decimals)];
         [transferrableMakerTokenAmount, transferrableMakerFeeTokenAmount] = [
-                                                ZeroEx.toBaseUnitAmount(new BigNumber(50), 18),
-                                                ZeroEx.toBaseUnitAmount(new BigNumber(5), 18)];
+                                                ZeroEx.toBaseUnitAmount(new BigNumber(50), decimals),
+                                                ZeroEx.toBaseUnitAmount(new BigNumber(5), decimals)];
     });
     function buildSignedOrder(): SignedOrder {
         return { ecSignature: signature,
@@ -50,26 +50,20 @@ describe.only('RemainingFillableCalculator', () => {
                  expirationUnixTimestampSec: zero };
     }
     describe('Maker token is NOT ZRX', () => {
-        it('calculates the correct amount when balance is less than remaining fillable', () => {
-            signedOrder = buildSignedOrder();
-            const partiallyFilledAmount = ZeroEx.toBaseUnitAmount(new BigNumber(2), 18);
-            remainingMakerTokenAmount = signedOrder.makerTokenAmount.minus(partiallyFilledAmount);
-            transferrableMakerTokenAmount = remainingMakerTokenAmount.minus(partiallyFilledAmount);
-            calculator = new RemainingFillableCalculator(signedOrder, zrxToken,
-                           transferrableMakerTokenAmount, transferrableMakerFeeTokenAmount, remainingMakerTokenAmount);
-            expect(calculator.computeRemainingMakerFillable()).to.be.bignumber.equal(transferrableMakerTokenAmount);
+        before(async () => {
+            isMakerTokenZRX = false;
         });
         it('calculates the correct amount when unfilled and funds available', () => {
             signedOrder = buildSignedOrder();
             remainingMakerTokenAmount = signedOrder.makerTokenAmount;
-            calculator = new RemainingFillableCalculator(signedOrder, zrxToken,
+            calculator = new RemainingFillableCalculator(signedOrder, isMakerTokenZRX,
                            transferrableMakerTokenAmount, transferrableMakerFeeTokenAmount, remainingMakerTokenAmount);
             expect(calculator.computeRemainingMakerFillable()).to.be.bignumber.equal(remainingMakerTokenAmount);
         });
         it('calculates the correct amount when partially filled and funds available', () => {
             signedOrder = buildSignedOrder();
-            remainingMakerTokenAmount = ZeroEx.toBaseUnitAmount(new BigNumber(1), 18);
-            calculator = new RemainingFillableCalculator(signedOrder, zrxToken,
+            remainingMakerTokenAmount = ZeroEx.toBaseUnitAmount(new BigNumber(1), decimals);
+            calculator = new RemainingFillableCalculator(signedOrder, isMakerTokenZRX,
                            transferrableMakerTokenAmount, transferrableMakerFeeTokenAmount, remainingMakerTokenAmount);
             expect(calculator.computeRemainingMakerFillable()).to.be.bignumber.equal(remainingMakerTokenAmount);
         });
@@ -77,38 +71,37 @@ describe.only('RemainingFillableCalculator', () => {
             signedOrder = buildSignedOrder();
             transferrableMakerFeeTokenAmount = zero;
             remainingMakerTokenAmount = signedOrder.makerTokenAmount;
-            calculator = new RemainingFillableCalculator(signedOrder, zrxToken,
+            calculator = new RemainingFillableCalculator(signedOrder, isMakerTokenZRX,
                            transferrableMakerTokenAmount, transferrableMakerFeeTokenAmount, remainingMakerTokenAmount);
             expect(calculator.computeRemainingMakerFillable()).to.be.bignumber.equal(zero);
         });
-    });
-    describe('Maker Token is ZRX', () => {
-        beforeEach(async () => {
-            makerToken = zrxToken;
-        });
         it('calculates the correct amount when balance is less than remaining fillable', () => {
             signedOrder = buildSignedOrder();
-            const partiallyFilledAmount = ZeroEx.toBaseUnitAmount(new BigNumber(2), 18);
+            const partiallyFilledAmount = ZeroEx.toBaseUnitAmount(new BigNumber(2), decimals);
             remainingMakerTokenAmount = signedOrder.makerTokenAmount.minus(partiallyFilledAmount);
             transferrableMakerTokenAmount = remainingMakerTokenAmount.minus(partiallyFilledAmount);
-            transferrableMakerFeeTokenAmount = transferrableMakerTokenAmount;
-            calculator = new RemainingFillableCalculator(signedOrder, zrxToken,
+            calculator = new RemainingFillableCalculator(signedOrder, isMakerTokenZRX,
                            transferrableMakerTokenAmount, transferrableMakerFeeTokenAmount, remainingMakerTokenAmount);
             expect(calculator.computeRemainingMakerFillable()).to.be.bignumber.equal(transferrableMakerTokenAmount);
         });
+    });
+    describe('Maker Token is ZRX', () => {
+        before(async () => {
+            isMakerTokenZRX = true;
+        });
         it('calculates the correct amount when unfilled and funds available', () => {
             signedOrder = buildSignedOrder();
             transferrableMakerTokenAmount = makerAmount.plus(makerFee);
             transferrableMakerFeeTokenAmount = transferrableMakerTokenAmount;
             remainingMakerTokenAmount = signedOrder.makerTokenAmount;
-            calculator = new RemainingFillableCalculator(signedOrder, zrxToken,
+            calculator = new RemainingFillableCalculator(signedOrder, isMakerTokenZRX,
                            transferrableMakerTokenAmount, transferrableMakerFeeTokenAmount, remainingMakerTokenAmount);
             expect(calculator.computeRemainingMakerFillable()).to.be.bignumber.equal(remainingMakerTokenAmount);
         });
         it('calculates the correct amount when partially filled and funds available', () => {
             signedOrder = buildSignedOrder();
-            remainingMakerTokenAmount = ZeroEx.toBaseUnitAmount(new BigNumber(1), 18);
-            calculator = new RemainingFillableCalculator(signedOrder, zrxToken,
+            remainingMakerTokenAmount = ZeroEx.toBaseUnitAmount(new BigNumber(1), decimals);
+            calculator = new RemainingFillableCalculator(signedOrder, isMakerTokenZRX,
                            transferrableMakerTokenAmount, transferrableMakerFeeTokenAmount, remainingMakerTokenAmount);
             expect(calculator.computeRemainingMakerFillable()).to.be.bignumber.equal(remainingMakerTokenAmount);
         });
@@ -117,9 +110,27 @@ describe.only('RemainingFillableCalculator', () => {
             transferrableMakerTokenAmount = zero;
             transferrableMakerFeeTokenAmount = zero;
             remainingMakerTokenAmount = signedOrder.makerTokenAmount;
-            calculator = new RemainingFillableCalculator(signedOrder, zrxToken,
+            calculator = new RemainingFillableCalculator(signedOrder, isMakerTokenZRX,
                            transferrableMakerTokenAmount, transferrableMakerFeeTokenAmount, remainingMakerTokenAmount);
             expect(calculator.computeRemainingMakerFillable()).to.be.bignumber.equal(zero);
         });
+        it('calculates the correct amount when balance is less than remaining fillable', () => {
+            signedOrder = buildSignedOrder();
+            const partiallyFilledAmount = ZeroEx.toBaseUnitAmount(new BigNumber(2), decimals);
+            remainingMakerTokenAmount = signedOrder.makerTokenAmount.minus(partiallyFilledAmount);
+            transferrableMakerTokenAmount = remainingMakerTokenAmount.minus(partiallyFilledAmount);
+            transferrableMakerFeeTokenAmount = transferrableMakerTokenAmount;
+
+            const orderToFeeRatio = signedOrder.makerTokenAmount.dividedToIntegerBy(signedOrder.makerFee);
+            const expectedRemainingAmount = new BigNumber(450950);
+            const numberOfFillsInRatio = expectedRemainingAmount.dividedToIntegerBy(orderToFeeRatio);
+            calculator = new RemainingFillableCalculator(signedOrder, isMakerTokenZRX,
+                           transferrableMakerTokenAmount, transferrableMakerFeeTokenAmount, remainingMakerTokenAmount);
+            const calculatedRemainingAmount = calculator.computeRemainingMakerFillable();
+            const calculatedRemainingAmountPlusFees = calculatedRemainingAmount.plus(numberOfFillsInRatio);
+            expect(calculatedRemainingAmount).to.be.bignumber.equal(expectedRemainingAmount);
+            expect(calculatedRemainingAmountPlusFees).to.be.bignumber.lessThan(transferrableMakerTokenAmount);
+            expect(calculatedRemainingAmountPlusFees).to.be.bignumber.lessThan(remainingMakerTokenAmount);
+        });
     });
 });