feat(asset-buyer): update buyQuoteCalculator to match rounding behavior in contracts

6 files changed, 147 insertions, 65 deletions

diff --git a/packages/asset-buyer/src/asset_buyer.ts b/packages/asset-buyer/src/asset_buyer.ts
index 2f9a3108e..74f3cb471 100644
--- a/packages/asset-buyer/src/asset_buyer.ts
+++ b/packages/asset-buyer/src/asset_buyer.ts
@@ -135,9 +135,14 @@ export class AssetBuyer {
         assert.isBoolean('shouldForceOrderRefresh', shouldForceOrderRefresh);
         assert.isNumber('slippagePercentage', slippagePercentage);
         const zrxTokenAssetData = this._getZrxTokenAssetDataOrThrow();
+        const isMakerAssetZrxToken = assetData === zrxTokenAssetData;
+        // get the relevant orders for the makerAsset and fees
+        // if the requested assetData is ZRX, don't get the fee info
         const [ordersAndFillableAmounts, feeOrdersAndFillableAmounts] = await Promise.all([
             this._getOrdersAndFillableAmountsAsync(assetData, shouldForceOrderRefresh),
-            this._getOrdersAndFillableAmountsAsync(zrxTokenAssetData, shouldForceOrderRefresh),
+            isMakerAssetZrxToken
+                ? Promise.resolve(constants.EMPTY_ORDERS_AND_FILLABLE_AMOUNTS)
+                : this._getOrdersAndFillableAmountsAsync(zrxTokenAssetData, shouldForceOrderRefresh),
             shouldForceOrderRefresh,
         ]);
         if (ordersAndFillableAmounts.orders.length === 0) {
@@ -149,6 +154,7 @@ export class AssetBuyer {
             assetBuyAmount,
             feePercentage,
             slippagePercentage,
+            isMakerAssetZrxToken,
         );
         return buyQuote;
     }
diff --git a/packages/asset-buyer/src/constants.ts b/packages/asset-buyer/src/constants.ts
index 55effdb23..c912253bd 100644
--- a/packages/asset-buyer/src/constants.ts
+++ b/packages/asset-buyer/src/constants.ts
@@ -1,6 +1,7 @@
+import { SignedOrder } from '@0x/types';
 import { BigNumber } from '@0x/utils';
 
-import { AssetBuyerOpts, BuyQuoteExecutionOpts, BuyQuoteRequestOpts } from './types';
+import { AssetBuyerOpts, BuyQuoteExecutionOpts, BuyQuoteRequestOpts, OrdersAndFillableAmounts } from './types';
 
 const NULL_ADDRESS = '0x0000000000000000000000000000000000000000';
 const MAINNET_NETWORK_ID = 1;
@@ -22,6 +23,11 @@ const DEFAULT_BUY_QUOTE_EXECUTION_OPTS: BuyQuoteExecutionOpts = {
     feeRecipient: NULL_ADDRESS,
 };
 
+const EMPTY_ORDERS_AND_FILLABLE_AMOUNTS: OrdersAndFillableAmounts = {
+    orders: [] as SignedOrder[],
+    remainingFillableMakerAssetAmounts: [] as BigNumber[],
+};
+
 export const constants = {
     ZERO_AMOUNT: new BigNumber(0),
     NULL_ADDRESS,
@@ -31,4 +37,5 @@ export const constants = {
     DEFAULT_BUY_QUOTE_EXECUTION_OPTS,
     DEFAULT_BUY_QUOTE_REQUEST_OPTS,
     MAX_PER_PAGE: 10000,
+    EMPTY_ORDERS_AND_FILLABLE_AMOUNTS,
 };
diff --git a/packages/asset-buyer/src/order_providers/standard_relayer_api_order_provider.ts b/packages/asset-buyer/src/order_providers/standard_relayer_api_order_provider.ts
index 91682b089..41fd1fb30 100644
--- a/packages/asset-buyer/src/order_providers/standard_relayer_api_order_provider.ts
+++ b/packages/asset-buyer/src/order_providers/standard_relayer_api_order_provider.ts
@@ -30,7 +30,7 @@ export class StandardRelayerAPIOrderProvider implements OrderProvider {
                 'remainingTakerAssetAmount',
                 order.takerAssetAmount,
             );
-            const remainingFillableMakerAssetAmount = orderUtils.calculateRemainingMakerAssetAmount(
+            const remainingFillableMakerAssetAmount = orderUtils.getRemainingMakerAmount(
                 order,
                 remainingFillableTakerAssetAmount,
             );
diff --git a/packages/asset-buyer/src/utils/buy_quote_calculator.ts b/packages/asset-buyer/src/utils/buy_quote_calculator.ts
index b0cb4a547..33cbea4a6 100644
--- a/packages/asset-buyer/src/utils/buy_quote_calculator.ts
+++ b/packages/asset-buyer/src/utils/buy_quote_calculator.ts
@@ -1,10 +1,12 @@
-import { marketUtils, rateUtils } from '@0x/order-utils';
+import { marketUtils, rateUtils, SignedOrder } from '@0x/order-utils';
 import { BigNumber } from '@0x/utils';
 import * as _ from 'lodash';
 
 import { constants } from '../constants';
 import { AssetBuyerError, BuyQuote, BuyQuoteInfo, OrdersAndFillableAmounts } from '../types';
 
+import { orderUtils } from './order_utils';
+
 // Calculates a buy quote for orders that have WETH as the takerAsset
 export const buyQuoteCalculator = {
     calculate(
@@ -13,6 +15,7 @@ export const buyQuoteCalculator = {
         assetBuyAmount: BigNumber,
         feePercentage: number,
         slippagePercentage: number,
+        isMakerAssetZrxToken: boolean,
     ): BuyQuote {
         const orders = ordersAndFillableAmounts.orders;
         const remainingFillableMakerAssetAmounts = ordersAndFillableAmounts.remainingFillableMakerAssetAmounts;
@@ -32,22 +35,31 @@ export const buyQuoteCalculator = {
         if (remainingFillAmount.gt(constants.ZERO_AMOUNT)) {
             throw new Error(AssetBuyerError.InsufficientAssetLiquidity);
         }
+        // if we are not buying ZRX:
         // given the orders calculated above, find the fee-orders that cover the desired assetBuyAmount (with slippage)
         // TODO(bmillman): optimization
         // update this logic to find the minimum amount of feeOrders to cover the worst case as opposed to
         // finding order that cover all fees, this will help with estimating ETH and minimizing gas usage
-        const {
-            resultFeeOrders,
-            remainingFeeAmount,
-            feeOrdersRemainingFillableMakerAssetAmounts,
-        } = marketUtils.findFeeOrdersThatCoverFeesForTargetOrders(resultOrders, feeOrders, {
-            remainingFillableMakerAssetAmounts: ordersRemainingFillableMakerAssetAmounts,
-            remainingFillableFeeAmounts,
-        });
-        // if we do not have enough feeOrders to cover the fees, throw
-        if (remainingFeeAmount.gt(constants.ZERO_AMOUNT)) {
-            throw new Error(AssetBuyerError.InsufficientZrxLiquidity);
+        let resultFeeOrders = [] as SignedOrder[];
+        let feeOrdersRemainingFillableMakerAssetAmounts = [] as BigNumber[];
+        if (!isMakerAssetZrxToken) {
+            const feeOrdersAndRemainingFeeAmount = marketUtils.findFeeOrdersThatCoverFeesForTargetOrders(
+                resultOrders,
+                feeOrders,
+                {
+                    remainingFillableMakerAssetAmounts: ordersRemainingFillableMakerAssetAmounts,
+                    remainingFillableFeeAmounts,
+                },
+            );
+            // if we do not have enough feeOrders to cover the fees, throw
+            if (feeOrdersAndRemainingFeeAmount.remainingFeeAmount.gt(constants.ZERO_AMOUNT)) {
+                throw new Error(AssetBuyerError.InsufficientZrxLiquidity);
+            }
+            resultFeeOrders = feeOrdersAndRemainingFeeAmount.resultFeeOrders;
+            feeOrdersRemainingFillableMakerAssetAmounts =
+                feeOrdersAndRemainingFeeAmount.feeOrdersRemainingFillableMakerAssetAmounts;
         }
+
         // assetData information for the result
         const assetData = orders[0].makerAssetData;
         // compile the resulting trimmed set of orders for makerAsset and feeOrders that are needed for assetBuyAmount
@@ -64,6 +76,7 @@ export const buyQuoteCalculator = {
             trimmedFeeOrdersAndFillableAmounts,
             assetBuyAmount,
             feePercentage,
+            isMakerAssetZrxToken,
         );
         // in order to calculate the maxRate, reverse the ordersAndFillableAmounts such that they are sorted from worst rate to best rate
         const worstCaseQuoteInfo = calculateQuoteInfo(
@@ -71,6 +84,7 @@ export const buyQuoteCalculator = {
             reverseOrdersAndFillableAmounts(trimmedFeeOrdersAndFillableAmounts),
             assetBuyAmount,
             feePercentage,
+            isMakerAssetZrxToken,
         );
         return {
             assetData,
@@ -89,22 +103,30 @@ function calculateQuoteInfo(
     feeOrdersAndFillableAmounts: OrdersAndFillableAmounts,
     assetBuyAmount: BigNumber,
     feePercentage: number,
+    isMakerAssetZrxToken: boolean,
 ): BuyQuoteInfo {
     // find the total eth and zrx needed to buy assetAmount from the resultOrders from left to right
-    const [ethAmountToBuyAsset, zrxAmountToBuyAsset] = findEthAndZrxAmountNeededToBuyAsset(
-        ordersAndFillableAmounts,
-        assetBuyAmount,
-    );
-    // find the total eth needed to buy fees
-    const ethAmountToBuyFees = findEthAmountNeededToBuyFees(feeOrdersAndFillableAmounts, zrxAmountToBuyAsset);
-    const affiliateFeeEthAmount = ethAmountToBuyAsset.mul(feePercentage);
-    const totalEthAmountWithoutAffiliateFee = ethAmountToBuyAsset.plus(ethAmountToBuyFees);
-    const totalEthAmount = totalEthAmountWithoutAffiliateFee.plus(affiliateFeeEthAmount);
+    let ethAmountToBuyAsset = constants.ZERO_AMOUNT;
+    let ethAmountToBuyZrx = constants.ZERO_AMOUNT;
+    if (isMakerAssetZrxToken) {
+        ethAmountToBuyAsset = findEthAmountNeededToBuyZrx(ordersAndFillableAmounts, assetBuyAmount);
+    } else {
+        // find eth and zrx amounts needed to buy
+        const ethAndZrxAmountToBuyAsset = findEthAndZrxAmountNeededToBuyAsset(ordersAndFillableAmounts, assetBuyAmount);
+        ethAmountToBuyAsset = ethAndZrxAmountToBuyAsset[0];
+        const zrxAmountToBuyAsset = ethAndZrxAmountToBuyAsset[1];
+        // find eth amount needed to buy zrx
+        ethAmountToBuyZrx = findEthAmountNeededToBuyZrx(feeOrdersAndFillableAmounts, zrxAmountToBuyAsset);
+    }
+    /// find the eth amount needed to buy the affiliate fee
+    const ethAmountToBuyAffiliateFee = ethAmountToBuyAsset.mul(feePercentage);
+    const totalEthAmountWithoutAffiliateFee = ethAmountToBuyAsset.plus(ethAmountToBuyZrx);
+    const ethAmountTotal = totalEthAmountWithoutAffiliateFee.plus(ethAmountToBuyAffiliateFee);
     // divide into the assetBuyAmount in order to find rate of makerAsset / WETH
     const ethPerAssetPrice = totalEthAmountWithoutAffiliateFee.div(assetBuyAmount);
     return {
-        totalEthAmount,
-        feeEthAmount: affiliateFeeEthAmount,
+        totalEthAmount: ethAmountTotal,
+        feeEthAmount: ethAmountToBuyAffiliateFee,
         ethPerAssetPrice,
     };
 }
@@ -119,29 +141,38 @@ function reverseOrdersAndFillableAmounts(ordersAndFillableAmounts: OrdersAndFill
     };
 }
 
-function findEthAmountNeededToBuyFees(
+function findEthAmountNeededToBuyZrx(
     feeOrdersAndFillableAmounts: OrdersAndFillableAmounts,
-    feeAmount: BigNumber,
+    zrxBuyAmount: BigNumber,
 ): BigNumber {
     const { orders, remainingFillableMakerAssetAmounts } = feeOrdersAndFillableAmounts;
     const result = _.reduce(
         orders,
         (acc, order, index) => {
+            const { totalEthAmount, remainingZrxBuyAmount } = acc;
             const remainingFillableMakerAssetAmount = remainingFillableMakerAssetAmounts[index];
-            const amountToFill = BigNumber.min(acc.remainingFeeAmount, remainingFillableMakerAssetAmount);
-            const feeAdjustedRate = rateUtils.getFeeAdjustedRateOfFeeOrder(order);
-            const ethAmountForThisOrder = feeAdjustedRate.mul(amountToFill);
+            const makerFillAmount = BigNumber.min(acc.remainingZrxBuyAmount, remainingFillableMakerAssetAmount);
+            const [takerFillAmount, adjustedMakerFillAmount] = orderUtils.getTakerFillAmountForFeeOrder(
+                order,
+                makerFillAmount,
+            );
+            const extraFeeAmount = remainingFillableMakerAssetAmount.greaterThanOrEqualTo(adjustedMakerFillAmount)
+                ? constants.ZERO_AMOUNT
+                : adjustedMakerFillAmount.sub(makerFillAmount);
             return {
-                ethAmount: acc.ethAmount.plus(ethAmountForThisOrder),
-                remainingFeeAmount: BigNumber.max(constants.ZERO_AMOUNT, acc.remainingFeeAmount.minus(amountToFill)),
+                totalEthAmount: totalEthAmount.plus(takerFillAmount),
+                remainingZrxBuyAmount: BigNumber.max(
+                    constants.ZERO_AMOUNT,
+                    acc.remainingZrxBuyAmount.minus(makerFillAmount).plus(extraFeeAmount),
+                ),
             };
         },
         {
-            ethAmount: constants.ZERO_AMOUNT,
-            remainingFeeAmount: feeAmount,
+            totalEthAmount: constants.ZERO_AMOUNT,
+            remainingZrxBuyAmount: zrxBuyAmount,
         },
     );
-    return result.ethAmount;
+    return result.totalEthAmount;
 }
 
 function findEthAndZrxAmountNeededToBuyAsset(
@@ -152,28 +183,25 @@ function findEthAndZrxAmountNeededToBuyAsset(
     const result = _.reduce(
         orders,
         (acc, order, index) => {
+            const { totalEthAmount, totalZrxAmount, remainingAssetBuyAmount } = acc;
             const remainingFillableMakerAssetAmount = remainingFillableMakerAssetAmounts[index];
-            const amountToFill = BigNumber.min(acc.remainingAssetBuyAmount, remainingFillableMakerAssetAmount);
-            // find the amount of eth required to fill amountToFill (amountToFill / makerAssetAmount) * takerAssetAmount
-            const ethAmountForThisOrder = amountToFill
-                .mul(order.takerAssetAmount)
-                .dividedToIntegerBy(order.makerAssetAmount);
-            // find the amount of zrx required to fill fees for amountToFill (amountToFill / makerAssetAmount) * takerFee
-            const zrxAmountForThisOrder = amountToFill.mul(order.takerFee).dividedToIntegerBy(order.makerAssetAmount);
+            const makerFillAmount = BigNumber.min(acc.remainingAssetBuyAmount, remainingFillableMakerAssetAmount);
+            const takerFillAmount = orderUtils.getTakerFillAmount(order, makerFillAmount);
+            const takerFeeAmount = orderUtils.getTakerFeeAmount(order, takerFillAmount);
             return {
-                ethAmount: acc.ethAmount.plus(ethAmountForThisOrder),
-                zrxAmount: acc.zrxAmount.plus(zrxAmountForThisOrder),
+                totalEthAmount: totalEthAmount.plus(takerFillAmount),
+                totalZrxAmount: totalZrxAmount.plus(takerFeeAmount),
                 remainingAssetBuyAmount: BigNumber.max(
                     constants.ZERO_AMOUNT,
-                    acc.remainingAssetBuyAmount.minus(amountToFill),
+                    remainingAssetBuyAmount.minus(makerFillAmount),
                 ),
             };
         },
         {
-            ethAmount: constants.ZERO_AMOUNT,
-            zrxAmount: constants.ZERO_AMOUNT,
+            totalEthAmount: constants.ZERO_AMOUNT,
+            totalZrxAmount: constants.ZERO_AMOUNT,
             remainingAssetBuyAmount: assetBuyAmount,
         },
     );
-    return [result.ethAmount, result.zrxAmount];
+    return [result.totalEthAmount, result.totalZrxAmount];
 }
diff --git a/packages/asset-buyer/src/utils/order_provider_response_processor.ts b/packages/asset-buyer/src/utils/order_provider_response_processor.ts
index 81464d945..28f684f3c 100644
--- a/packages/asset-buyer/src/utils/order_provider_response_processor.ts
+++ b/packages/asset-buyer/src/utils/order_provider_response_processor.ts
@@ -110,10 +110,7 @@ function getValidOrdersWithRemainingFillableMakerAssetAmountsFromOnChain(
                 traderInfo.makerZrxBalance,
             ]);
             const remainingTakerAssetAmount = order.takerAssetAmount.minus(orderInfo.orderTakerAssetFilledAmount);
-            const remainingMakerAssetAmount = orderUtils.calculateRemainingMakerAssetAmount(
-                order,
-                remainingTakerAssetAmount,
-            );
+            const remainingMakerAssetAmount = orderUtils.getRemainingMakerAmount(order, remainingTakerAssetAmount);
             const remainingFillableCalculator = new RemainingFillableCalculator(
                 order.makerFee,
                 order.makerAssetAmount,
diff --git a/packages/asset-buyer/src/utils/order_utils.ts b/packages/asset-buyer/src/utils/order_utils.ts
index ff47eb7c5..1cc2cf95f 100644
--- a/packages/asset-buyer/src/utils/order_utils.ts
+++ b/packages/asset-buyer/src/utils/order_utils.ts
@@ -12,19 +12,63 @@ export const orderUtils = {
         const currentUnixTimestampSec = new BigNumber(Date.now() / millisecondsInSecond).round();
         return order.expirationTimeSeconds.lessThan(currentUnixTimestampSec.plus(secondsFromNow));
     },
-    calculateRemainingMakerAssetAmount(order: SignedOrder, remainingTakerAssetAmount: BigNumber): BigNumber {
-        if (remainingTakerAssetAmount.eq(0)) {
-            return constants.ZERO_AMOUNT;
-        }
-        return remainingTakerAssetAmount.times(order.makerAssetAmount).dividedToIntegerBy(order.takerAssetAmount);
-    },
-    calculateRemainingTakerAssetAmount(order: SignedOrder, remainingMakerAssetAmount: BigNumber): BigNumber {
-        if (remainingMakerAssetAmount.eq(0)) {
-            return constants.ZERO_AMOUNT;
-        }
-        return remainingMakerAssetAmount.times(order.takerAssetAmount).dividedToIntegerBy(order.makerAssetAmount);
-    },
     isOpenOrder(order: SignedOrder): boolean {
         return order.takerAddress === constants.NULL_ADDRESS;
     },
+    // given a remaining amount of takerAsset, calculate how much makerAsset is available
+    getRemainingMakerAmount(order: SignedOrder, remainingTakerAmount: BigNumber): BigNumber {
+        const remainingMakerAmount = remainingTakerAmount
+            .times(order.makerAssetAmount)
+            .div(order.takerAssetAmount)
+            .floor();
+        return remainingMakerAmount;
+    },
+    // given a desired amount of makerAsset, calculate how much takerAsset is required to fill that amount
+    getTakerFillAmount(order: SignedOrder, makerFillAmount: BigNumber): BigNumber {
+        // Round up because exchange rate favors Maker
+        const takerFillAmount = makerFillAmount
+            .mul(order.takerAssetAmount)
+            .div(order.makerAssetAmount)
+            .ceil();
+        return takerFillAmount;
+    },
+    // given a desired amount of takerAsset to fill, calculate how much fee is required by the taker to fill that amount
+    getTakerFeeAmount(order: SignedOrder, takerFillAmount: BigNumber): BigNumber {
+        // Round down because Taker fee rate favors Taker
+        const takerFeeAmount = takerFillAmount
+            .mul(order.takerFee)
+            .div(order.takerAssetAmount)
+            .floor();
+        return takerFeeAmount;
+    },
+    // given a desired amount of takerAsset to fill, calculate how much makerAsset will be filled
+    getMakerFillAmount(order: SignedOrder, takerFillAmount: BigNumber): BigNumber {
+        // Round down because exchange rate favors Maker
+        const makerFillAmount = takerFillAmount
+            .mul(order.makerAssetAmount)
+            .div(order.takerAssetAmount)
+            .floor();
+        return makerFillAmount;
+    },
+    // given a desired amount of makerAsset, calculate how much fee is required by the maker to fill that amount
+    getMakerFeeAmount(order: SignedOrder, makerFillAmount: BigNumber): BigNumber {
+        // Round down because Maker fee rate favors Maker
+        const makerFeeAmount = makerFillAmount
+            .mul(order.makerFee)
+            .div(order.makerAssetAmount)
+            .floor();
+        return makerFeeAmount;
+    },
+    // given a desired amount of ZRX from a fee order, calculate how much takerAsset is required to fill that amount
+    // also calculate how much ZRX needs to be bought in order fill the desired amount + takerFee
+    getTakerFillAmountForFeeOrder(order: SignedOrder, makerFillAmount: BigNumber): [BigNumber, BigNumber] {
+        // For each unit of TakerAsset we buy (MakerAsset - TakerFee)
+        const adjustedTakerFillAmount = makerFillAmount
+            .mul(order.takerAssetAmount)
+            .div(order.makerAssetAmount.sub(order.takerFee))
+            .ceil();
+        // The amount that we buy will be greater than makerFillAmount, since we buy some amount for fees.
+        const adjustedMakerFillAmount = orderUtils.getMakerFillAmount(order, adjustedTakerFillAmount);
+        return [adjustedTakerFillAmount, adjustedMakerFillAmount];
+    },
 };