Improved ComparisonPrice function (#32)

* separated comparison price function into a new file, accounted for backup orders

* removed scratch code

* Adjusted documentation, object naming

* Refactored comparisonPrice function to use adjusted rate from optimizer, used native order fee schedule to adjust for order fees

* Small fixes to function, added unit tests

* Adjusted fee calculation for comparisonPrice function

* use available OptimalPathRate object

* fix lint error in test, separate out fee calculation

* Fixed market operation utils test, added additional checks for fee schedule

* removed unused dep, prettier

packages/asset-swapper/src/utils/market_operation_utils/comparison_price.ts

@@ -0,0 +1,79 @@
+import { Web3Wrapper } from '@0x/dev-utils';
+import { BigNumber, logUtils } from '@0x/utils';
+import * as _ from 'lodash';
+
+import { MarketOperation } from '../../types';
+
+import { COMPARISON_PRICE_DECIMALS } from './constants';
+import { ComparisonPrice, ERC20BridgeSource, FeeEstimate, FeeSchedule, MarketSideLiquidity } from './types';
+
+/**
+ * Takes in an optimizer response and returns a price for RFQT MMs to beat
+ * returns the price of the taker asset in terms of the maker asset
+ * So the RFQT MM should aim for a higher price
+ * @param adjustedRate the adjusted rate (accounting for fees) from the optimizer, maker/taker
+ * @param amount the amount specified by the client
+ * @param marketSideLiquidity the results from querying liquidity sources
+ * @param feeSchedule the fee schedule passed to the Optimizer
+ * @return ComparisonPrice object with the prices for RFQ MMs to beat
+ */
+export function getComparisonPrices(
+    adjustedRate: BigNumber,
+    amount: BigNumber,
+    marketSideLiquidity: MarketSideLiquidity,
+    feeSchedule: FeeSchedule,
+): ComparisonPrice {
+    let wholeOrder: BigNumber | undefined;
+    let feeInEth: BigNumber | number;
+
+    // HACK: get the fee penalty of a single 0x native order
+    // The FeeSchedule function takes in a `FillData` object and returns a fee estimate in ETH
+    // We don't have fill data here, we just want the cost of a single native order, so we pass in undefined
+    // This works because the feeSchedule returns a constant for Native orders, this will need
+    // to be tweaked if the feeSchedule for native orders uses the fillData passed in
+    // 2 potential issues: there is no native fee schedule or the fee schedule depends on fill data
+    if (feeSchedule[ERC20BridgeSource.Native] === undefined) {
+        logUtils.warn('ComparisonPrice function did not find native order fee schedule');
+
+        return { wholeOrder };
+    } else {
+        try {
+            feeInEth = new BigNumber((feeSchedule[ERC20BridgeSource.Native] as FeeEstimate)(undefined));
+        } catch {
+            logUtils.warn('Native order fee schedule requires fill data');
+
+            return { wholeOrder };
+        }
+    }
+
+    // Calc native order fee penalty in output unit (maker units for sells, taker unit for buys)
+    const feePenalty = !marketSideLiquidity.ethToOutputRate.isZero()
+        ? marketSideLiquidity.ethToOutputRate.times(feeInEth)
+        : // if it's a sell, the input token is the taker token
+          marketSideLiquidity.ethToInputRate
+              .times(feeInEth)
+              .times(marketSideLiquidity.side === MarketOperation.Sell ? adjustedRate : adjustedRate.pow(-1));
+
+    // the adjusted rate is defined as maker/taker
+    // input is the taker token for sells, input is the maker token for buys
+    const orderMakerAmount =
+        marketSideLiquidity.side === MarketOperation.Sell ? adjustedRate.times(amount).plus(feePenalty) : amount;
+    const orderTakerAmount =
+        marketSideLiquidity.side === MarketOperation.Sell ? amount : amount.dividedBy(adjustedRate).minus(feePenalty);
+
+    if (orderTakerAmount.gt(0) && orderMakerAmount.gt(0)) {
+        const optimalMakerUnitAmount = Web3Wrapper.toUnitAmount(
+            // round up maker amount -- err to giving more competitive price
+            orderMakerAmount.integerValue(BigNumber.ROUND_UP),
+            marketSideLiquidity.makerTokenDecimals,
+        );
+        const optimalTakerUnitAmount = Web3Wrapper.toUnitAmount(
+            // round down taker amount -- err to giving more competitive price
+            orderTakerAmount.integerValue(BigNumber.ROUND_DOWN),
+            marketSideLiquidity.takerTokenDecimals,
+        );
+        wholeOrder = optimalMakerUnitAmount.div(optimalTakerUnitAmount).decimalPlaces(COMPARISON_PRICE_DECIMALS);
+    }
+
+    return { wholeOrder };
+}

packages/asset-swapper/src/utils/market_operation_utils/constants.ts

@@ -375,7 +375,7 @@ export const ONE_HOUR_IN_SECONDS = 60 * 60;
 export const ONE_SECOND_MS = 1000;
 export const NULL_BYTES = '0x';
 export const NULL_ADDRESS = '0x0000000000000000000000000000000000000000';
-export const COMPARISON_PRICE_DECIMALS = 5;
+export const COMPARISON_PRICE_DECIMALS = 10;
 
 const EMPTY_BRIDGE_ADDRESSES: BridgeContractAddresses = {
     uniswapBridge: NULL_ADDRESS,

packages/asset-swapper/src/utils/market_operation_utils/index.ts

@@ -1,7 +1,6 @@
 import { RFQTIndicativeQuote } from '@0x/quote-server';
 import { SignedOrder } from '@0x/types';
 import { BigNumber, NULL_ADDRESS } from '@0x/utils';
-import { Web3Wrapper } from '@0x/web3-wrapper';
 import * as _ from 'lodash';
 
 import { AssetSwapperContractAddresses, MarketOperation } from '../../types';
@@ -9,9 +8,9 @@ import { QuoteRequestor } from '../quote_requestor';
 import { getPriceAwareRFQRolloutFlags } from '../utils';
 
 import { generateQuoteReport, QuoteReport } from './../quote_report_generator';
+import { getComparisonPrices } from './comparison_price';
 import {
     BUY_SOURCE_FILTER,
-    COMPARISON_PRICE_DECIMALS,
     DEFAULT_GET_MARKET_ORDERS_OPTS,
     FEE_QUOTE_SOURCES,
     ONE_ETHER,
@@ -559,6 +558,7 @@ export class MarketOperationUtils {
                 liquidityDelivered: bestTwoHopQuote,
                 sourceFlags: SOURCE_FLAGS[ERC20BridgeSource.MultiHop],
                 marketSideLiquidity,
+                adjustedRate: bestTwoHopRate,
             };
         }
 
@@ -584,11 +584,13 @@ export class MarketOperationUtils {
             }
         }
         const collapsedPath = optimalPath.collapse(orderOpts);
+
         return {
             optimizedOrders: collapsedPath.orders,
             liquidityDelivered: collapsedPath.collapsedFills as CollapsedFill[],
             sourceFlags: collapsedPath.sourceFlags,
             marketSideLiquidity,
+            adjustedRate: optimalPathRate,
         };
     }
 
@@ -627,30 +629,17 @@ export class MarketOperationUtils {
         }
 
         // If RFQ liquidity is enabled, make a request to check RFQ liquidity
-        let comparisonPrice: BigNumber | undefined;
+        let wholeOrderPrice: BigNumber | undefined;
         const { rfqt } = _opts;
         if (rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) {
             // Calculate a suggested price. For now, this is simply the overall price of the aggregation.
             if (optimizerResult) {
-                const totalMakerAmount = BigNumber.sum(
+                wholeOrderPrice = getComparisonPrices(
-                    ...optimizerResult.optimizedOrders.map(order => order.makerAssetAmount),
+                    optimizerResult.adjustedRate,
-                );
+                    amount,
-                const totalTakerAmount = BigNumber.sum(
+                    marketSideLiquidity,
-                    ...optimizerResult.optimizedOrders.map(order => order.takerAssetAmount),
+                    _opts.feeSchedule,
-                );
+                ).wholeOrder;
-                if (totalMakerAmount.gt(0)) {
-                    const totalMakerAmountUnitAmount = Web3Wrapper.toUnitAmount(
-                        totalMakerAmount,
-                        marketSideLiquidity.makerTokenDecimals,
-                    );
-                    const totalTakerAmountUnitAmount = Web3Wrapper.toUnitAmount(
-                        totalTakerAmount,
-                        marketSideLiquidity.takerTokenDecimals,
-                    );
-                    comparisonPrice = totalMakerAmountUnitAmount
-                        .div(totalTakerAmountUnitAmount)
-                        .decimalPlaces(COMPARISON_PRICE_DECIMALS);
-                }
             }
 
             const { isFirmPriceAwareEnabled, isIndicativePriceAwareEnabled } = getPriceAwareRFQRolloutFlags(
@@ -664,7 +653,7 @@ export class MarketOperationUtils {
                     nativeOrders[0].takerAssetData,
                     side,
                     amount,
-                    comparisonPrice,
+                    wholeOrderPrice,
                     _opts,
                 );
                 // Re-run optimizer with the new indicative quote
@@ -690,7 +679,7 @@ export class MarketOperationUtils {
                         nativeOrders[0].takerAssetData,
                         amount,
                         side,
-                        comparisonPrice,
+                        wholeOrderPrice,
                         rfqt,
                     );
                     if (firmQuotes.length > 0) {
@@ -731,7 +720,7 @@ export class MarketOperationUtils {
                 _opts.rfqt ? _opts.rfqt.quoteRequestor : undefined,
                 marketSideLiquidity,
                 optimizerResult,
-                comparisonPrice,
+                wholeOrderPrice,
             );
         }
         return { ...optimizerResult, quoteReport };

packages/asset-swapper/src/utils/market_operation_utils/types.ts

@@ -340,6 +340,7 @@ export interface OptimizerResult {
     sourceFlags: number;
     liquidityDelivered: CollapsedFill[] | DexSample<MultiHopFillData>;
     marketSideLiquidity: MarketSideLiquidity;
+    adjustedRate: BigNumber;
 }
 
 export interface OptimizerResultWithReport extends OptimizerResult {
@@ -391,3 +392,7 @@ export interface GenerateOptimizedOrdersOpts {
     allowFallback?: boolean;
     shouldBatchBridgeOrders?: boolean;
 }
+
+export interface ComparisonPrice {
+    wholeOrder: BigNumber | undefined;
+}

packages/asset-swapper/test/comparison_price_test.ts

@@ -0,0 +1,122 @@
+// tslint:disable:custom-no-magic-numbers
+import { BigNumber } from '@0x/utils';
+import * as chai from 'chai';
+import * as _ from 'lodash';
+import 'mocha';
+
+import { MarketOperation } from '../src/types';
+import { getComparisonPrices } from '../src/utils/market_operation_utils/comparison_price';
+import { SourceFilters } from '../src/utils/market_operation_utils/source_filters';
+import { DexSample, ERC20BridgeSource, MarketSideLiquidity } from '../src/utils/market_operation_utils/types';
+
+import { chaiSetup } from './utils/chai_setup';
+
+chaiSetup.configure();
+const expect = chai.expect;
+
+const DAI_TOKEN = '0x6b175474e89094c44da98b954eedeac495271d0f';
+const ETH_TOKEN = '0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee';
+const GAS_PRICE = new BigNumber(50e9); // 50 gwei
+const NATIVE_ORDER_FEE = new BigNumber(220e3); // 220K gas
+
+// DEX samples to fill in MarketSideLiquidity
+const kyberSample1: DexSample = {
+    source: ERC20BridgeSource.Kyber,
+    input: new BigNumber(10000),
+    output: new BigNumber(10001),
+    fillData: {},
+};
+const uniswapSample1: DexSample = {
+    source: ERC20BridgeSource.UniswapV2,
+    input: new BigNumber(10003),
+    output: new BigNumber(10004),
+    fillData: {},
+};
+const dexQuotes: DexSample[] = [kyberSample1, uniswapSample1];
+
+const feeSchedule = {
+    [ERC20BridgeSource.Native]: _.constant(GAS_PRICE.times(NATIVE_ORDER_FEE)),
+};
+
+const buyMarketSideLiquidity: MarketSideLiquidity = {
+    // needed params
+    ethToOutputRate: new BigNumber(500),
+    ethToInputRate: new BigNumber(1),
+    side: MarketOperation.Buy,
+    makerTokenDecimals: 18,
+    takerTokenDecimals: 18,
+    // extra
+    inputAmount: new BigNumber(0),
+    inputToken: ETH_TOKEN,
+    outputToken: DAI_TOKEN,
+    dexQuotes: [dexQuotes],
+    nativeOrders: [],
+    orderFillableAmounts: [],
+    twoHopQuotes: [],
+    rfqtIndicativeQuotes: [],
+    quoteSourceFilters: new SourceFilters(),
+};
+
+const sellMarketSideLiquidity: MarketSideLiquidity = {
+    // needed params
+    ethToOutputRate: new BigNumber(500),
+    ethToInputRate: new BigNumber(1),
+    side: MarketOperation.Sell,
+    makerTokenDecimals: 18,
+    takerTokenDecimals: 18,
+    // extra
+    inputAmount: new BigNumber(0),
+    inputToken: ETH_TOKEN,
+    outputToken: DAI_TOKEN,
+    dexQuotes: [dexQuotes],
+    nativeOrders: [],
+    orderFillableAmounts: [],
+    twoHopQuotes: [],
+    rfqtIndicativeQuotes: [],
+    quoteSourceFilters: new SourceFilters(),
+};
+
+describe('getComparisonPrices', async () => {
+    it('should create a proper comparison price for Sells', () => {
+        // test selling 10 ETH for DAI
+        // here, ETH is the input token
+        // and DAI is the output token
+        const AMOUNT = new BigNumber(10 * 1e18);
+        // raw maker over taker rate, let's say is 500 flat
+        const adjustedRate = new BigNumber(500);
+
+        const comparisonPrices = getComparisonPrices(adjustedRate, AMOUNT, sellMarketSideLiquidity, feeSchedule);
+
+        // expected outcome
+        const EXPECTED_PRICE = new BigNumber('500.55');
+
+        expect(comparisonPrices.wholeOrder).to.deep.eq(EXPECTED_PRICE);
+    });
+    it('should create a proper comparison price for Buys', () => {
+        // test buying 10 ETH with DAI
+        // here, ETH is the input token
+        // and DAI is the output token (now from the maker's perspective)
+        const AMOUNT = new BigNumber(10 * 1e18);
+
+        // raw maker over taker rate, let's say is ETH/DAI rate is 500 flat
+        const adjustedRate = new BigNumber(1).dividedBy(new BigNumber(500));
+
+        const comparisonPrices = getComparisonPrices(adjustedRate, AMOUNT, buyMarketSideLiquidity, feeSchedule);
+
+        // expected outcome
+        const EXPECTED_PRICE = new BigNumber('0.0020022024');
+
+        expect(comparisonPrices.wholeOrder).to.deep.eq(EXPECTED_PRICE);
+    });
+    it('should not return a price if takerAmount is < 0', () => {
+        // test selling 0.00001 ETH for DAI
+        // this will result in a negative comparison price, but here we should return undefined
+        const AMOUNT = new BigNumber(0.00001 * 1e18);
+        // raw maker over taker rate, let's say is 500 flat
+        const adjustedRate = new BigNumber(500);
+
+        const comparisonPrices = getComparisonPrices(adjustedRate, AMOUNT, sellMarketSideLiquidity, feeSchedule);
+
+        expect(comparisonPrices.wholeOrder === undefined);
+    });
+});

packages/asset-swapper/test/market_operation_utils_test.ts

@@ -731,6 +731,11 @@ describe('MarketOperationUtils tests', () => {
                 const mockedQuoteRequestor = TypeMoq.Mock.ofType(QuoteRequestor, TypeMoq.MockBehavior.Loose, false, {});
 
                 let requestedComparisonPrice: BigNumber | undefined;
+
+                // to get a comparisonPrice, you need a feeschedule for a native order
+                const feeSchedule = {
+                    [ERC20BridgeSource.Native]: _.constant(new BigNumber(1)),
+                };
                 mockedQuoteRequestor
                     .setup(mqr =>
                         mqr.requestRfqtFirmQuotesAsync(
@@ -811,6 +816,7 @@ describe('MarketOperationUtils tests', () => {
                     Web3Wrapper.toBaseUnitAmount(1, 18),
                     {
                         ...DEFAULT_OPTS,
+                        feeSchedule,
                         rfqt: {
                             isIndicative: false,
                             apiKey: 'foo',