Change orderStates param name to remaingFillableMakerAssetAmounts

3 files changed, 69 insertions, 91 deletions

diff --git a/packages/order-utils/src/market_utils.ts b/packages/order-utils/src/market_utils.ts
index 710eddf8a..d66448a0b 100644
--- a/packages/order-utils/src/market_utils.ts
+++ b/packages/order-utils/src/market_utils.ts
@@ -1,5 +1,5 @@
 import { schemas } from '@0xproject/json-schemas';
-import { OrderRelevantState, SignedOrder } from '@0xproject/types';
+import { SignedOrder } from '@0xproject/types';
 import { BigNumber } from '@0xproject/utils';
 import * as _ from 'lodash';
 
@@ -11,24 +11,33 @@ export const marketUtils = {
      * Takes an array of orders and returns a subset of those orders that has enough makerAssetAmount (taking into account on-chain balances,
      * allowances, and partial fills) in order to fill the input makerAssetFillAmount plus slippageBufferAmount. Iterates from first order to last.
      * Sort the input by ascending rate in order to get the subset of orders that will cost the least ETH.
-     * @param   signedOrders         An array of objects that conform to the SignedOrder interface. All orders should specify the same makerAsset.
-     *                               All orders should specify WETH as the takerAsset.
-     * @param   orderStates          An array of objects corresponding to the signedOrders parameter that each contain on-chain state
-     *                               relevant to that order.
-     * @param   makerAssetFillAmount The amount of makerAsset desired to be filled.
-     * @param   slippageBufferAmount An additional amount makerAsset to be covered by the result in case of trade collisions or partial fills.
+     * @param   signedOrders                        An array of objects that conform to the SignedOrder interface. All orders should specify the same makerAsset.
+     *                                              All orders should specify WETH as the takerAsset.
+     * @param   remainingFillableMakerAssetAmounts  An array of BigNumbers corresponding to the signedOrders parameter.
+     *                                              You can use OrderStateUtils @0xproject/order-utils to perform blockchain lookups
+     *                                              for these values.
+     * @param   makerAssetFillAmount                The amount of makerAsset desired to be filled.
+     * @param   slippageBufferAmount                An additional amount makerAsset to be covered by the result in case of trade collisions or partial fills.
      * @return  Resulting orders and remaining fill amount that could not be covered by the input.
      */
     findOrdersThatCoverMakerAssetFillAmount(
         signedOrders: SignedOrder[],
-        orderStates: OrderRelevantState[],
+        remainingFillableMakerAssetAmounts: BigNumber[],
         makerAssetFillAmount: BigNumber,
         slippageBufferAmount: BigNumber = constants.ZERO_AMOUNT,
     ): { resultOrders: SignedOrder[]; remainingFillAmount: BigNumber } {
         // type assertions
         assert.doesConformToSchema('signedOrders', signedOrders, schemas.signedOrdersSchema);
-        assert.isBigNumber('makerAssetFillAmount', makerAssetFillAmount);
-        assert.isBigNumber('slippageBufferAmount', slippageBufferAmount);
+        _.forEach(remainingFillableMakerAssetAmounts, (amount, index) =>
+            assert.isValidBaseUnitAmount(`remainingFillableMakerAssetAmount[${index}]`, amount),
+        );
+        assert.isValidBaseUnitAmount('makerAssetFillAmount', makerAssetFillAmount);
+        assert.isValidBaseUnitAmount('slippageBufferAmount', slippageBufferAmount);
+        // other assertions
+        assert.assert(
+            signedOrders.length === remainingFillableMakerAssetAmounts.length,
+            'Expected signedOrders.length to equal remainingFillableMakerAssetAmounts.length',
+        );
         // calculate total amount of makerAsset needed to be filled
         const totalFillAmount = makerAssetFillAmount.plus(slippageBufferAmount);
         // iterate through the signedOrders input from left to right until we have enough makerAsset to fill totalFillAmount
@@ -38,8 +47,7 @@ export const marketUtils = {
                 if (remainingFillAmount.lessThanOrEqualTo(constants.ZERO_AMOUNT)) {
                     return { resultOrders, remainingFillAmount: constants.ZERO_AMOUNT };
                 } else {
-                    const orderState = orderStates[index];
-                    const makerAssetAmountAvailable = orderState.remainingFillableMakerAssetAmount;
+                    const makerAssetAmountAvailable = remainingFillableMakerAssetAmounts[index];
                     // if there is no makerAssetAmountAvailable do not append order to resultOrders
                     // if we have exceeded the total amount we want to fill set remainingFillAmount to 0
                     return {
@@ -62,47 +70,64 @@ export const marketUtils = {
      * on-chain balances, allowances, and partial fills) in order to fill the takerFees required by signedOrders plus a
      * slippageBufferAmount. Iterates from first feeOrder to last. Sort the feeOrders by ascending rate in order to get the subset of
      * feeOrders that will cost the least ETH.
-     * @param   signedOrders         An array of objects that conform to the SignedOrder interface. All orders should specify ZRX as
-     *                               the makerAsset and WETH as the takerAsset.
-     * @param   orderStates          An array of objects corresponding to the signedOrders parameter that each contain on-chain state
-     *                               relevant to that order.
-     * @param   signedFeeOrders      An array of objects that conform to the SignedOrder interface. All orders should specify ZRX as
-     *                               the makerAsset and WETH as the takerAsset.
-     * @param   feeOrderStates       An array of objects corresponding to the signedOrders parameter that each contain on-chain state
-     *                               relevant to that order.
-     * @param   makerAssetFillAmount The amount of makerAsset desired to be filled.
-     * @param   slippageBufferAmount An additional amount makerAsset to be covered by the result in case of trade collisions or partial fills.
+     * @param   signedOrders                        An array of objects that conform to the SignedOrder interface. All orders should specify ZRX as
+     *                                              the makerAsset and WETH as the takerAsset.
+     * @param   remainingFillableMakerAssetAmounts  An array of BigNumbers corresponding to the signedOrders parameter.
+     *                                              You can use OrderStateUtils @0xproject/order-utils to perform blockchain lookups
+     *                                              for these values.
+     * @param   signedFeeOrders                     An array of objects that conform to the SignedOrder interface. All orders should specify ZRX as
+     *                                              the makerAsset and WETH as the takerAsset.
+     * @param   remainingFillableFeeAmounts         An array of BigNumbers corresponding to the signedFeeOrders parameter.
+     *                                              You can use OrderStateUtils @0xproject/order-utils to perform blockchain lookups
+     *                                              for these values.
+     * @param   slippageBufferAmount                An additional amount makerAsset to be covered by the result in case of trade collisions or partial fills.
      * @return  Resulting orders and remaining fill amount that could not be covered by the input.
      */
     findFeeOrdersThatCoverFeesForTargetOrders(
         signedOrders: SignedOrder[],
-        orderStates: OrderRelevantState[],
+        remainingFillableMakerAssetAmounts: BigNumber[],
         signedFeeOrders: SignedOrder[],
-        feeOrderStates: OrderRelevantState[],
+        remainingFillableFeeAmounts: BigNumber[],
         slippageBufferAmount: BigNumber = constants.ZERO_AMOUNT,
     ): { resultOrders: SignedOrder[]; remainingFillAmount: BigNumber } {
         // type assertions
         assert.doesConformToSchema('signedOrders', signedOrders, schemas.signedOrdersSchema);
+        _.forEach(remainingFillableMakerAssetAmounts, (amount, index) =>
+            assert.isValidBaseUnitAmount(`remainingFillableMakerAssetAmount[${index}]`, amount),
+        );
         assert.doesConformToSchema('signedFeeOrders', signedFeeOrders, schemas.signedOrdersSchema);
-        assert.isBigNumber('slippageBufferAmount', slippageBufferAmount);
+        _.forEach(remainingFillableFeeAmounts, (amount, index) =>
+            assert.isValidBaseUnitAmount(`remainingFillableFeeAmounts[${index}]`, amount),
+        );
+        assert.isValidBaseUnitAmount('slippageBufferAmount', slippageBufferAmount);
+        // other assertions
+        assert.assert(
+            signedOrders.length === remainingFillableMakerAssetAmounts.length,
+            'Expected signedOrders.length to equal remainingFillableMakerAssetAmounts.length',
+        );
+        assert.assert(
+            signedOrders.length === remainingFillableMakerAssetAmounts.length,
+            'Expected signedFeeOrders.length to equal remainingFillableFeeAmounts.length',
+        );
         // calculate total amount of ZRX needed to fill signedOrders
         const totalFeeAmount = _.reduce(
             signedOrders,
             (accFees, order, index) => {
-                const orderState = orderStates[index];
-                const makerAssetAmountAvailable = orderState.remainingFillableMakerAssetAmount;
+                const makerAssetAmountAvailable = remainingFillableMakerAssetAmounts[index];
                 const feeToFillMakerAssetAmountAvailable = makerAssetAmountAvailable
-                    .div(order.makerAssetAmount)
-                    .mul(order.takerFee);
+                    .mul(order.takerFee)
+                    .div(order.makerAssetAmount);
                 return accFees.plus(feeToFillMakerAssetAmountAvailable);
             },
             constants.ZERO_AMOUNT,
         );
         return marketUtils.findOrdersThatCoverMakerAssetFillAmount(
             signedFeeOrders,
-            feeOrderStates,
+            remainingFillableFeeAmounts,
             totalFeeAmount,
             slippageBufferAmount,
         );
+        // TODO: add more orders here to cover rounding
+        // https://github.com/0xProject/0x-protocol-specification/blob/master/v2/forwarding-contract-specification.md#over-buying-zrx
     },
 };
diff --git a/packages/order-utils/test/market_utils_test.ts b/packages/order-utils/test/market_utils_test.ts
index 93779d035..ac3fb9b93 100644
--- a/packages/order-utils/test/market_utils_test.ts
+++ b/packages/order-utils/test/market_utils_test.ts
@@ -1,10 +1,8 @@
-import { OrderRelevantState, SignedOrder } from '@0xproject/types';
 import { BigNumber } from '@0xproject/utils';
 import * as chai from 'chai';
-import * as _ from 'lodash';
 import 'mocha';
 
-import { constants, marketUtils, orderFactory } from '../src';
+import { constants, marketUtils } from '../src';
 
 import { chaiSetup } from './utils/chai_setup';
 import { testOrderFactory } from './utils/test_order_factory';
@@ -37,19 +35,14 @@ describe('marketUtils', () => {
                 },
                 testOrderCount,
             );
-            // generate order states that cover the required fill amount
-            const inputOrderStates = testOrderFactory.generateTestOrderRelevantStates(
-                {
-                    remainingFillableMakerAssetAmount: makerAssetAmount,
-                },
-                testOrderCount,
-            );
+            // generate remainingFillableMakerAssetAmounts that equal the makerAssetAmount
+            const remainingFillableMakerAssetAmounts = [makerAssetAmount, makerAssetAmount, makerAssetAmount];
             it('returns input orders and zero remainingFillAmount when input exactly matches requested fill amount', async () => {
                 // try to fill 30 units of makerAsset
                 const fillAmount = new BigNumber(30);
                 const { resultOrders, remainingFillAmount } = marketUtils.findOrdersThatCoverMakerAssetFillAmount(
                     inputOrders,
-                    inputOrderStates,
+                    remainingFillableMakerAssetAmounts,
                     fillAmount,
                 );
                 expect(resultOrders).to.be.deep.equal(inputOrders);
@@ -60,7 +53,7 @@ describe('marketUtils', () => {
                 const fillAmount = new BigNumber(25);
                 const { resultOrders, remainingFillAmount } = marketUtils.findOrdersThatCoverMakerAssetFillAmount(
                     inputOrders,
-                    inputOrderStates,
+                    remainingFillableMakerAssetAmounts,
                     fillAmount,
                 );
                 expect(resultOrders).to.be.deep.equal(inputOrders);
@@ -71,7 +64,7 @@ describe('marketUtils', () => {
                 const fillAmount = new BigNumber(35);
                 const { resultOrders, remainingFillAmount } = marketUtils.findOrdersThatCoverMakerAssetFillAmount(
                     inputOrders,
-                    inputOrderStates,
+                    remainingFillableMakerAssetAmounts,
                     fillAmount,
                 );
                 expect(resultOrders).to.be.deep.equal(inputOrders);
@@ -82,7 +75,7 @@ describe('marketUtils', () => {
                 const fillAmount = new BigNumber(10);
                 const { resultOrders, remainingFillAmount } = marketUtils.findOrdersThatCoverMakerAssetFillAmount(
                     inputOrders,
-                    inputOrderStates,
+                    remainingFillableMakerAssetAmounts,
                     fillAmount,
                 );
                 expect(resultOrders).to.be.deep.equal([inputOrders[0]]);
@@ -93,7 +86,7 @@ describe('marketUtils', () => {
                 const fillAmount = new BigNumber(15);
                 const { resultOrders, remainingFillAmount } = marketUtils.findOrdersThatCoverMakerAssetFillAmount(
                     inputOrders,
-                    inputOrderStates,
+                    remainingFillableMakerAssetAmounts,
                     fillAmount,
                 );
                 expect(resultOrders).to.be.deep.equal([inputOrders[0], inputOrders[1]]);
@@ -110,31 +103,17 @@ describe('marketUtils', () => {
                 },
                 testOrderCount,
             );
-            // generate order states that cover different scenarios
+            // generate remainingFillableMakerAssetAmounts that cover different partial fill scenarios
             // 1. order is completely filled already
             // 2. order is partially fillable
             // 3. order is completely fillable
-            const partialOrderStates: Array<Partial<OrderRelevantState>> = [
-                {
-                    remainingFillableMakerAssetAmount: constants.ZERO_AMOUNT,
-                },
-                {
-                    remainingFillableMakerAssetAmount: new BigNumber(5),
-                },
-                {
-                    remainingFillableMakerAssetAmount: makerAssetAmount,
-                },
-            ];
-            const inputOrderStates: OrderRelevantState[] = _.map(
-                partialOrderStates,
-                testOrderFactory.generateTestOrderRelevantState,
-            );
+            const remainingFillableMakerAssetAmounts = [constants.ZERO_AMOUNT, new BigNumber(5), makerAssetAmount];
             it('returns last 2 orders and non-zero remainingFillAmount when trying to fill original makerAssetAmounts', async () => {
                 // try to fill 30 units of makerAsset
                 const fillAmount = new BigNumber(30);
                 const { resultOrders, remainingFillAmount } = marketUtils.findOrdersThatCoverMakerAssetFillAmount(
                     inputOrders,
-                    inputOrderStates,
+                    remainingFillableMakerAssetAmounts,
                     fillAmount,
                 );
                 expect(resultOrders).to.be.deep.equal([inputOrders[1], inputOrders[2]]);
diff --git a/packages/order-utils/test/utils/test_order_factory.ts b/packages/order-utils/test/utils/test_order_factory.ts
index 2c5d8cf61..611e777ea 100644
--- a/packages/order-utils/test/utils/test_order_factory.ts
+++ b/packages/order-utils/test/utils/test_order_factory.ts
@@ -1,5 +1,4 @@
-import { Order, OrderRelevantState, SignedOrder } from '@0xproject/types';
-import { BigNumber } from '@0xproject/utils';
+import { Order, SignedOrder } from '@0xproject/types';
 import * as _ from 'lodash';
 
 import { constants, orderFactory } from '../../src';
@@ -21,43 +20,18 @@ const BASE_TEST_SIGNED_ORDER: SignedOrder = {
     ...BASE_TEST_ORDER,
     signature: constants.NULL_BYTES,
 };
-const BASE_TEST_ORDER_RELEVANT_STATE: OrderRelevantState = {
-    makerBalance: constants.ZERO_AMOUNT,
-    makerProxyAllowance: constants.ZERO_AMOUNT,
-    makerFeeBalance: constants.ZERO_AMOUNT,
-    makerFeeProxyAllowance: constants.ZERO_AMOUNT,
-    filledTakerAssetAmount: constants.ZERO_AMOUNT,
-    remainingFillableMakerAssetAmount: constants.ZERO_AMOUNT,
-    remainingFillableTakerAssetAmount: constants.ZERO_AMOUNT,
-};
 
 export const testOrderFactory = {
     generateTestSignedOrder(partialOrder: Partial<SignedOrder>): SignedOrder {
         return transformObject(BASE_TEST_SIGNED_ORDER, partialOrder);
     },
     generateTestSignedOrders(partialOrder: Partial<SignedOrder>, numOrders: number): SignedOrder[] {
-        const baseTestOrders = generateArrayOfInput(BASE_TEST_SIGNED_ORDER, numOrders);
-        return transformObjects(baseTestOrders, partialOrder);
-    },
-    generateTestOrderRelevantState(partialOrderRelevantState: Partial<OrderRelevantState>): OrderRelevantState {
-        return transformObject(BASE_TEST_ORDER_RELEVANT_STATE, partialOrderRelevantState);
-    },
-    generateTestOrderRelevantStates(
-        partialOrderRelevantState: Partial<OrderRelevantState>,
-        numOrderStates: number,
-    ): OrderRelevantState[] {
-        const baseTestOrderStates = generateArrayOfInput(BASE_TEST_ORDER_RELEVANT_STATE, numOrderStates);
-        return transformObjects(baseTestOrderStates, partialOrderRelevantState);
+        const baseTestOrders = _.map(_.range(numOrders), () => BASE_TEST_SIGNED_ORDER);
+        return _.map(baseTestOrders, order => transformObject(order, partialOrder));
     },
 };
 
-function generateArrayOfInput<T>(input: T, rangeLength: number): T[] {
-    return _.map(_.range(rangeLength), () => input);
-}
 function transformObject<T>(input: T, transformation: Partial<T>): T {
     const copy = _.cloneDeep(input);
     return _.assign(copy, transformation);
 }
-function transformObjects<T>(inputs: T[], transformation: Partial<T>): T[] {
-    return _.map(inputs, input => transformObject(input, transformation));
-}