Implemented reference functions and added tests for _calculateCompleteRightFill

contracts/exchange/src/reference_functions.ts

@@ -1,8 +1,11 @@
 import { ReferenceFunctions as ExchangeLibsReferenceFunctions } from '@0x/contracts-exchange-libs';
-import { FillResults, OrderWithoutDomain } from '@0x/types';
+import { constants } from '@0x/contracts-test-utils';
+import { ReferenceFunctions as UtilsReferenceFunctions } from '@0x/contracts-utils';
+import { FillResults, MatchedFillResults, OrderWithoutDomain } from '@0x/types';
 import { BigNumber } from '@0x/utils';
 
-const { safeGetPartialAmountFloor } = ExchangeLibsReferenceFunctions;
+const { safeGetPartialAmountCeil, safeGetPartialAmountFloor } = ExchangeLibsReferenceFunctions;
+const { safeSub } = UtilsReferenceFunctions;
 
 /**
  * Calculates amounts filled and fees paid by maker and taker.
@@ -22,3 +25,199 @@ export function calculateFillResults(order: OrderWithoutDomain, takerAssetFilled
         takerFeePaid,
     };
 }
+
+/**
+ * Calculates amounts filled and fees paid by maker and taker.
+ */
+export function calculateMatchedFillResults(
+    leftOrder: OrderWithoutDomain,
+    rightOrder: OrderWithoutDomain,
+    leftOrderTakerAssetFilledAmount: BigNumber,
+    rightOrderTakerAssetFilledAmount: BigNumber,
+): MatchedFillResults {
+    // Get an empty matched fill results object to fill with this data.
+    const matchedFillResults = emptyMatchedFillResults();
+
+    // Calculate the remaining amounts of maker and taker assets for both orders.
+    const [
+        leftMakerAssetAmountRemaining,
+        leftTakerAssetAmountRemaining,
+        rightMakerAssetAmountRemaining,
+        rightTakerAssetAmountRemaining,
+    ] = getRemainingFillAmounts(
+        leftOrder,
+        rightOrder,
+        leftOrderTakerAssetFilledAmount,
+        rightOrderTakerAssetFilledAmount,
+    );
+
+    // Calculate the profit from the matching
+    matchedFillResults.profitInLeftMakerAsset = safeSub(
+        matchedFillResults.left.makerAssetFilledAmount,
+        matchedFillResults.right.takerAssetFilledAmount,
+    );
+
+    if (leftTakerAssetAmountRemaining.isGreaterThan(rightMakerAssetAmountRemaining)) {
+        // Case 1: Right order is fully filled
+        calculateCompleteRightFill(
+            leftOrder,
+            rightMakerAssetAmountRemaining,
+            rightTakerAssetAmountRemaining,
+            matchedFillResults,
+        );
+    } else if (leftTakerAssetAmountRemaining.isLessThan(rightMakerAssetAmountRemaining)) {
+        // Case 2: Left order is fully filled
+        matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining;
+        matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining;
+        matchedFillResults.right.makerAssetFilledAmount = leftTakerAssetAmountRemaining;
+        matchedFillResults.right.takerAssetFilledAmount = safeGetPartialAmountCeil(
+            rightOrder.takerAssetAmount,
+            rightOrder.makerAssetAmount,
+            leftTakerAssetAmountRemaining,
+        );
+    } else {
+        // Case 3: Both orders are fully filled. Technically, this could be captured by the above cases, but
+        //         this calculation will be more precise since it does not include rounding.
+        calculateCompleteFillBoth(
+            leftMakerAssetAmountRemaining,
+            leftTakerAssetAmountRemaining,
+            rightMakerAssetAmountRemaining,
+            rightTakerAssetAmountRemaining,
+            matchedFillResults,
+        );
+    }
+
+    // Compute the fees from the order matching
+    calculateFees(
+        leftOrder,
+        rightOrder,
+        matchedFillResults,
+    );
+
+    return matchedFillResults;
+}
+
+/**
+ * Calculates the complete fill of both orders and updates the provided MatchedFillResults object.
+ */
+export function calculateCompleteFillBoth(
+    leftMakerAssetAmountRemaining: BigNumber,
+    leftTakerAssetAmountRemaining: BigNumber,
+    rightMakerAssetAmountRemaining: BigNumber,
+    rightTakerAssetAmountRemaining: BigNumber,
+    matchedFillResults?: MatchedFillResults,
+): MatchedFillResults {
+    if (matchedFillResults === undefined) {
+        matchedFillResults = emptyMatchedFillResults(); // tslint:disable-line:no-parameter-reassignment
+    }
+    matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining;
+    matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining;
+    matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining;
+    matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining;
+    return matchedFillResults;
+}
+
+/**
+ * Calculates the complete fill of the right order and updates the provided MatchedFillResults object.
+ */
+export function calculateCompleteRightFill(
+    leftOrder: OrderWithoutDomain,
+    rightMakerAssetAmountRemaining: BigNumber,
+    rightTakerAssetAmountRemaining: BigNumber,
+    matchedFillResults?: MatchedFillResults,
+): MatchedFillResults {
+    if (matchedFillResults === undefined) {
+        matchedFillResults = emptyMatchedFillResults(); // tslint:disable-line:no-parameter-reassignment
+    }
+    matchedFillResults.left.makerAssetFilledAmount = safeGetPartialAmountFloor(
+        leftOrder.makerAssetAmount,
+        leftOrder.takerAssetAmount,
+        rightMakerAssetAmountRemaining,
+    );
+    matchedFillResults.left.takerAssetFilledAmount = rightMakerAssetAmountRemaining;
+    matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining;
+    matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining;
+    return matchedFillResults;
+}
+
+/**
+ * Compute the fees for the left and right orders.
+ */
+function calculateFees(
+    leftOrder: OrderWithoutDomain,
+    rightOrder: OrderWithoutDomain,
+    matchedFillResults: MatchedFillResults,
+): void {
+    // Compute fees for left order
+    matchedFillResults.left.makerFeePaid = safeGetPartialAmountFloor(
+        matchedFillResults.left.makerAssetFilledAmount,
+        leftOrder.makerAssetAmount,
+        leftOrder.makerFee,
+    );
+    matchedFillResults.left.takerFeePaid = safeGetPartialAmountFloor(
+        matchedFillResults.left.takerAssetFilledAmount,
+        leftOrder.takerAssetAmount,
+        leftOrder.takerFee,
+    );
+
+    // Compute fees for right order
+    matchedFillResults.right.makerFeePaid = safeGetPartialAmountFloor(
+        matchedFillResults.right.makerAssetFilledAmount,
+        rightOrder.makerAssetAmount,
+        rightOrder.makerFee,
+    );
+    matchedFillResults.right.takerFeePaid = safeGetPartialAmountFloor(
+        matchedFillResults.right.takerAssetFilledAmount,
+        rightOrder.takerAssetAmount,
+        rightOrder.takerFee,
+    );
+}
+
+/**
+ * Returns an empty fill results object.
+ */
+function emptyFillResults(): FillResults {
+    return {
+        makerAssetFilledAmount: constants.ZERO_AMOUNT,
+        takerAssetFilledAmount: constants.ZERO_AMOUNT,
+        makerFeePaid: constants.ZERO_AMOUNT,
+        takerFeePaid: constants.ZERO_AMOUNT,
+    };
+}
+
+/**
+ * Returns an empty matched fill results object.
+ */
+function emptyMatchedFillResults(): MatchedFillResults {
+    return {
+        left: emptyFillResults(),
+        right: emptyFillResults(),
+        profitInLeftMakerAsset: constants.ZERO_AMOUNT,
+        profitInRightMakerAsset: constants.ZERO_AMOUNT,
+    };
+}
+
+/**
+ * Returns the token amounts that are remaining to be filled.
+ */
+function getRemainingFillAmounts(
+    leftOrder: OrderWithoutDomain,
+    rightOrder: OrderWithoutDomain,
+    leftOrderTakerAssetFilledAmount: BigNumber,
+    rightOrderTakerAssetFilledAmount: BigNumber,
+): [ BigNumber, BigNumber, BigNumber, BigNumber ] {
+    return [
+        safeGetPartialAmountFloor(
+            leftOrder.makerAssetAmount,
+            leftOrder.takerAssetAmount,
+            leftOrderTakerAssetFilledAmount
+        ),
+        safeSub(leftOrder.takerAssetAmount, leftOrderTakerAssetFilledAmount),
+        safeGetPartialAmountFloor(
+            rightOrder.makerAssetAmount,
+            rightOrder.takerAssetAmount,
+            rightOrderTakerAssetFilledAmount
+        ),
+        safeSub(rightOrder.takerAssetAmount, rightOrderTakerAssetFilledAmount),
+    ];
+}

contracts/exchange/test/match_orders_unit_tests.ts

@@ -25,7 +25,7 @@ import {
 } from '@0x/contracts-test-utils';
 import { BlockchainLifecycle } from '@0x/dev-utils';
 import { assetDataUtils, orderHashUtils } from '@0x/order-utils';
-import { Order, OrderStatus, SignedOrder } from '@0x/types';
+import { OrderWithoutDomain, OrderStatus, SignedOrder } from '@0x/types';
 import { BigNumber, providerUtils } from '@0x/utils';
 import { Web3Wrapper } from '@0x/web3-wrapper';
 import * as chai from 'chai';
@@ -40,6 +40,7 @@ import {
     ReferenceFunctions,
     TestMatchOrdersContract,
 } from '../src';
+import { calculateCompleteFillBoth, calculateCompleteRightFill } from '../src/reference_functions';
 
 import { MatchOrderTester, TokenBalances } from './utils/match_order_tester';
 
@@ -48,30 +49,6 @@ import { MatchOrderTester, TokenBalances } from './utils/match_order_tester';
 constants.NUM_DUMMY_ERC721_TO_DEPLOY = 1;
 constants.NUM_DUMMY_ERC1155_CONTRACTS_TO_DEPLOY = 1;
 
-/**
- * Converts a SignedOrder object into an Order object by removing the signature field.
- *
- */
-//function toOrder(order: SignedOrder): Order {
-//    return {
-//        domain: order.domain,
-//        makerAddress: order.makerAddress,
-//        takerAddress: order.takerAddress,
-//        feeRecipientAddress: order.feeRecipientAddress,
-//        senderAddress: order.senderAddress,
-//        makerAssetAmount: order.makerAssetAmount,
-//        takerAssetAmount: order.takerAssetAmount,
-//        makerFee: order.makerFee,
-//        takerFee: order.takerFee,
-//        expirationTimeSeconds: order.expirationTimeSeconds,
-//        salt: order.salt,
-//        makerAssetData: order.makerAssetData,
-//        takerAssetData: order.takerAssetData,
-//        makerFeeAssetData: order.makerFeeAssetData,
-//        takerFeeAssetData: order.takerFeeAssetData,
-//    }
-//}
-
 /**
  * Tests the _calculateCompleteFillBoth function with the provided inputs by making a call
  * to the provided matchOrders contract's externalCalculateCompleteFillBoth function with the
@@ -90,19 +67,55 @@ async function testCalculateCompleteFillBothAsync(
     // Ensure that the correct number of arguments were provided.
     expect(args.length).to.be.eq(4);
 
-    // Get the resultant matched fill results from the call to _calculateCompleteFillBoth.
+    // Get the expected matched fill results from calling the reference function.
-    const matchedFillResults = await matchOrders.externalCalculateCompleteFillBoth.callAsync(
+    const expectedMatchedFillResults = calculateCompleteFillBoth(
         args[0],
         args[1],
         args[2],
         args[3],
     );
 
-    // Ensure that the matched fill results are correct.
+    // Get the resultant matched fill results from the call to _calculateCompleteFillBoth.
-    expect(matchedFillResults.left.makerAssetFilledAmount).bignumber.to.be.eq(args[0]);
+    const actualMatchedFillResults = await matchOrders.externalCalculateCompleteFillBoth.callAsync(
-    expect(matchedFillResults.left.takerAssetFilledAmount).bignumber.to.be.eq(args[1]);
+        args[0],
-    expect(matchedFillResults.right.makerAssetFilledAmount).bignumber.to.be.eq(args[2]);
+        args[1],
-    expect(matchedFillResults.right.takerAssetFilledAmount).bignumber.to.be.eq(args[3]);
+        args[2],
+        args[3],
+    );
+
+    expect(actualMatchedFillResults).to.be.deep.eq(expectedMatchedFillResults);
+}
+
+/**
+ * Tests the _calculateCompleteRightFill function with the provided inputs by making a call
+ * to the provided matchOrders contract's externalCalculateCompleteRightFill function with the
+ * provided inputs and asserting that the resultant struct is correct.
+ * @param matchOrders The TestMatchOrders contract object that should be used to make the call to
+ *                    the smart contract.
+ */
+async function testCalculateCompleteRightFillAsync(
+    matchOrders: TestMatchOrdersContract,
+    leftOrder: OrderWithoutDomain,
+    args: BigNumber[],
+): Promise<void> {
+    // Ensure that the correct number of arguments were provided.
+    expect(args.length).to.be.eq(2);
+
+    // Get the expected matched fill results from calling the reference function.
+    const expectedMatchedFillResults = calculateCompleteRightFill(
+        leftOrder,
+        args[0],
+        args[1],
+    );
+
+    // Get the resultant matched fill results from the call to _calculateCompleteRightFill.
+    const actualMatchedFillResults = await matchOrders.publicCalculateCompleteRightFill.callAsync(
+        leftOrder,
+        args[0],
+        args[1],
+    );
+
+    expect(actualMatchedFillResults).to.be.deep.eq(expectedMatchedFillResults);
 }
 
 /**
@@ -138,6 +151,9 @@ async function testCalculateCompleteFillBothAsync(
 //    expect(matchedFillResults.right.takerAssetFilledAmount).bignumber.to.be.eq(args[3]);
 //}
 
+chaiSetup.configure();
+const expect = chai.expect;
+
 blockchainTests.resets.only('MatchOrders Tests', ({ web3Wrapper, txDefaults }) => {
     let chainId: number;
     let makerAddressLeft: string;
@@ -327,14 +343,6 @@ blockchainTests.resets.only('MatchOrders Tests', ({ web3Wrapper, txDefaults }) =
         );
     });
 
-    beforeEach(async () => {
-        await blockchainLifecycle.startAsync();
-    });
-
-    afterEach(async () => {
-        await blockchainLifecycle.revertAsync();
-    });
-
     describe('_assertValidMatch', () => {
 
     });
@@ -400,9 +408,63 @@ blockchainTests.resets.only('MatchOrders Tests', ({ web3Wrapper, txDefaults }) =
     });
 
     describe('_calculateCompleteRightFill', () => {
-        // FIXME - Test a few different situations.
+        /**
-        // FIXME - Verify that rounding fails when it should. Add a comment that says that it
+         * NOTE(jalextowle): These test cases actually cover all code branches of _calculateCompleteRightFill (in
-        //         can possibly be removed after more rigorous unit testing
+         * fact any one of these test cases provide 100% coverage), but they do not verify that _safeGetPartialAmountCeil
+         * and _safeGetPartialAmountFloor revert appropriately. Keeping in the spirit of unit testing, these functions
+         * are unit tested to ensure that they exhibit the correct behavior in a more isolated setting.
+         */
+
+        it('should correctly calculate the complete right fill', async () => {
+            const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
+                makerAddress: makerAddressLeft,
+                makerAssetAmount: Web3Wrapper.toBaseUnitAmount(17, 0),
+                takerAssetAmount: Web3Wrapper.toBaseUnitAmount(98, 0),
+                feeRecipientAddress: feeRecipientAddressLeft,
+            });
+            await testCalculateCompleteRightFillAsync(
+                matchOrders,
+                signedOrderLeft,
+                [
+                    Web3Wrapper.toBaseUnitAmount(75, 0),
+                    Web3Wrapper.toBaseUnitAmount(13, 0),
+                ],
+            );
+        });
+
+        it('should correctly calculate the complete right fill', async () => {
+            const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
+                makerAddress: makerAddressLeft,
+                makerAssetAmount: Web3Wrapper.toBaseUnitAmount(12, 0),
+                takerAssetAmount: Web3Wrapper.toBaseUnitAmount(97, 0),
+                feeRecipientAddress: feeRecipientAddressLeft,
+            });
+            await testCalculateCompleteRightFillAsync(
+                matchOrders,
+                signedOrderLeft,
+                [
+                    Web3Wrapper.toBaseUnitAmount(89, 0),
+                    Web3Wrapper.toBaseUnitAmount(1, 0),
+                ],
+            );
+        });
+
+        it('should correctly calculate the complete right fill', async () => {
+            const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
+                makerAddress: makerAddressLeft,
+                makerAssetAmount: Web3Wrapper.toBaseUnitAmount(50, 18),
+                takerAssetAmount: Web3Wrapper.toBaseUnitAmount(100, 18),
+                feeRecipientAddress: feeRecipientAddressLeft,
+            });
+            await testCalculateCompleteRightFillAsync(
+                matchOrders,
+                signedOrderLeft,
+                [
+                    Web3Wrapper.toBaseUnitAmount(10, 18),
+                    Web3Wrapper.toBaseUnitAmount(2, 18),
+                ],
+            );
+        });
     });
 
     describe('_settleMatchedOrders', () => {