From 4db33ba2b37f4682ecae5da325e631d546a8a24e Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Mon, 22 Oct 2018 23:23:50 -0700 Subject: feat(asset-buyer): update buyQuoteCalculator to match rounding behavior in contracts --- packages/asset-buyer/src/asset_buyer.ts | 8 +- packages/asset-buyer/src/constants.ts | 9 +- .../standard_relayer_api_order_provider.ts | 2 +- .../asset-buyer/src/utils/buy_quote_calculator.ts | 120 +++++++++++++-------- .../src/utils/order_provider_response_processor.ts | 5 +- packages/asset-buyer/src/utils/order_utils.ts | 68 +++++++++--- .../asset-buyer/test/buy_quote_calculator_test.ts | 12 ++- 7 files changed, 155 insertions(+), 69 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]; + }, }; diff --git a/packages/asset-buyer/test/buy_quote_calculator_test.ts b/packages/asset-buyer/test/buy_quote_calculator_test.ts index 0f516a0f7..0ea371982 100644 --- a/packages/asset-buyer/test/buy_quote_calculator_test.ts +++ b/packages/asset-buyer/test/buy_quote_calculator_test.ts @@ -49,9 +49,9 @@ describe('buyQuoteCalculator', () => { remainingFillableMakerAssetAmounts: [smallFeeOrder.makerAssetAmount], }; const largeFeeOrder = orderFactory.createSignedOrderFromPartial({ - makerAssetAmount: new BigNumber(110), + makerAssetAmount: new BigNumber(113), takerAssetAmount: new BigNumber(200), - takerFee: new BigNumber(10), + takerFee: new BigNumber(11), }); allFeeOrdersAndFillableAmounts = { orders: [smallFeeOrder, largeFeeOrder], @@ -70,6 +70,7 @@ describe('buyQuoteCalculator', () => { new BigNumber(500), 0, 0, + false, ), ).to.throw(AssetBuyerError.InsufficientAssetLiquidity); }); @@ -82,6 +83,7 @@ describe('buyQuoteCalculator', () => { new BigNumber(300), 0, 0, + false, ), ).to.throw(AssetBuyerError.InsufficientZrxLiquidity); }); @@ -97,6 +99,7 @@ describe('buyQuoteCalculator', () => { assetBuyAmount, feePercentage, slippagePercentage, + false, ); // test if orders are correct expect(buyQuote.orders).to.deep.equal([ordersAndFillableAmounts.orders[0]]); @@ -134,6 +137,7 @@ describe('buyQuoteCalculator', () => { assetBuyAmount, feePercentage, slippagePercentage, + false, ); // test if orders are correct expect(buyQuote.orders).to.deep.equal(ordersAndFillableAmounts.orders); @@ -149,9 +153,9 @@ describe('buyQuoteCalculator', () => { expect(buyQuote.bestCaseQuoteInfo.feeEthAmount).to.bignumber.equal(expectedFeeEthAmount); expect(buyQuote.bestCaseQuoteInfo.totalEthAmount).to.bignumber.equal(expectedTotalEthAmount); expect(buyQuote.bestCaseQuoteInfo.ethPerAssetPrice).to.bignumber.equal(expectedEthPerAssetPrice); - // 100 eth to fill the first order + 200 eth for fees + // 100 eth to fill the first order + 208 eth for fees const expectedWorstEthAmountForAsset = new BigNumber(100); - const expectedWorstEthAmountForZrxFees = new BigNumber(200); + const expectedWorstEthAmountForZrxFees = new BigNumber(208); const expectedWorstFillEthAmount = expectedWorstEthAmountForAsset.plus(expectedWorstEthAmountForZrxFees); const expectedWorstFeeEthAmount = expectedWorstEthAmountForAsset.mul(feePercentage); const expectedWorstTotalEthAmount = expectedWorstFillEthAmount.plus(expectedWorstFeeEthAmount); -- cgit v1.2.3