From 5d2cdb00c202862f70d728278c7e6bd874200c92 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Fri, 4 Feb 2022 10:37:38 +1000 Subject: [PATCH] fix: slippage inconsistency when recalculated in exchange proxy quote consumer (#412) * fix: Pass slippage down rather than recalculate due to accuracy * CHANGELOG --- packages/asset-swapper/CHANGELOG.json | 9 +++++++++ .../exchange_proxy_swap_quote_consumer.ts | 17 ++--------------- packages/asset-swapper/src/swap_quoter.ts | 14 +++++++++----- packages/asset-swapper/src/types.ts | 2 ++ .../exchange_proxy_swap_quote_consumer_test.ts | 2 ++ packages/asset-swapper/test/utils/swap_quote.ts | 1 + 6 files changed, 25 insertions(+), 20 deletions(-) diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index 691d7bce9c..1ffd259a88 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "16.49.3", + "changes": [ + { + "note": "Fix `slippage` inconsistency when recalculated in exchange proxy quote consumer", + "pr": 412 + } + ] + }, { "version": "16.49.2", "changes": [ diff --git a/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts b/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts index 2108db1b50..2cded3115e 100644 --- a/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts +++ b/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts @@ -691,7 +691,7 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase { function slipNonNativeOrders(quote: MarketSellSwapQuote | MarketBuySwapQuote): OptimizedMarketOrder[] { const slippage = getMaxQuoteSlippageRate(quote); - if (!slippage) { + if (slippage === 0) { return quote.orders; } return quote.orders.map(o => { @@ -716,18 +716,5 @@ function slipNonNativeOrders(quote: MarketSellSwapQuote | MarketBuySwapQuote): O } function getMaxQuoteSlippageRate(quote: MarketBuySwapQuote | MarketSellSwapQuote): number { - if (quote.type === MarketOperation.Buy) { - // (worstCaseTaker - bestCaseTaker) / bestCaseTaker - // where worstCaseTaker >= bestCaseTaker - return quote.worstCaseQuoteInfo.takerAmount - .minus(quote.bestCaseQuoteInfo.takerAmount) - .div(quote.bestCaseQuoteInfo.takerAmount) - .toNumber(); - } - // (bestCaseMaker - worstCaseMaker) / bestCaseMaker - // where bestCaseMaker >= worstCaseMaker - return quote.bestCaseQuoteInfo.makerAmount - .minus(quote.worstCaseQuoteInfo.makerAmount) - .div(quote.bestCaseQuoteInfo.makerAmount) - .toNumber(); + return quote.worstCaseQuoteInfo.slippage; } diff --git a/packages/asset-swapper/src/swap_quoter.ts b/packages/asset-swapper/src/swap_quoter.ts index 6ebdc87beb..c40e0153d1 100644 --- a/packages/asset-swapper/src/swap_quoter.ts +++ b/packages/asset-swapper/src/swap_quoter.ts @@ -578,8 +578,8 @@ function calculateQuoteInfo( }); return { - bestCaseQuoteInfo: fillResultsToQuoteInfo(bestCaseFillResult), - worstCaseQuoteInfo: fillResultsToQuoteInfo(worstCaseFillResult), + bestCaseQuoteInfo: fillResultsToQuoteInfo(bestCaseFillResult, 0), + worstCaseQuoteInfo: fillResultsToQuoteInfo(worstCaseFillResult, slippage), sourceBreakdown: getSwapQuoteOrdersBreakdown(bestCaseFillResult.fillAmountBySource), }; } @@ -600,6 +600,7 @@ function calculateTwoHopQuoteInfo( }), ).toNumber(); const isSell = operation === MarketOperation.Sell; + return { bestCaseQuoteInfo: { makerAmount: isSell ? secondHopFill.output : secondHopFill.input, @@ -608,6 +609,7 @@ function calculateTwoHopQuoteInfo( feeTakerTokenAmount: constants.ZERO_AMOUNT, protocolFeeInWeiAmount: constants.ZERO_AMOUNT, gas, + slippage: 0, }, // TODO jacob consolidate this with quote simulation worstCase worstCaseQuoteInfo: { @@ -616,13 +618,14 @@ function calculateTwoHopQuoteInfo( : secondHopOrder.makerAmount, takerAmount: isSell ? firstHopOrder.takerAmount - : firstHopOrder.takerAmount.times(1 + slippage).integerValue(), + : firstHopOrder.takerAmount.times(1 + slippage).integerValue(BigNumber.ROUND_UP), totalTakerAmount: isSell ? firstHopOrder.takerAmount - : firstHopOrder.takerAmount.times(1 + slippage).integerValue(), + : firstHopOrder.takerAmount.times(1 + slippage).integerValue(BigNumber.ROUND_UP), feeTakerTokenAmount: constants.ZERO_AMOUNT, protocolFeeInWeiAmount: constants.ZERO_AMOUNT, gas, + slippage, }, sourceBreakdown: { [ERC20BridgeSource.MultiHop]: { @@ -648,7 +651,7 @@ function getSwapQuoteOrdersBreakdown(fillAmountBySource: { [source: string]: Big return breakdown; } -function fillResultsToQuoteInfo(fr: QuoteFillResult): SwapQuoteInfo { +function fillResultsToQuoteInfo(fr: QuoteFillResult, slippage: number): SwapQuoteInfo { return { makerAmount: fr.totalMakerAssetAmount, takerAmount: fr.takerAssetAmount, @@ -656,6 +659,7 @@ function fillResultsToQuoteInfo(fr: QuoteFillResult): SwapQuoteInfo { feeTakerTokenAmount: fr.takerFeeTakerAssetAmount, protocolFeeInWeiAmount: fr.protocolFeeAmount, gas: fr.gas, + slippage, }; } diff --git a/packages/asset-swapper/src/types.ts b/packages/asset-swapper/src/types.ts index 23c518f512..1e32497b2e 100644 --- a/packages/asset-swapper/src/types.ts +++ b/packages/asset-swapper/src/types.ts @@ -208,6 +208,7 @@ export type SwapQuote = MarketBuySwapQuote | MarketSellSwapQuote; * makerTokenAmount: The amount of makerAsset that will be acquired through the swap. * protocolFeeInWeiAmount: The amount of ETH to pay (in WEI) as protocol fee to perform the swap for desired asset. * gas: Amount of estimated gas needed to fill the quote. + * slippage: Amount of slippage to allow for. */ export interface SwapQuoteInfo { feeTakerTokenAmount: BigNumber; @@ -216,6 +217,7 @@ export interface SwapQuoteInfo { makerAmount: BigNumber; protocolFeeInWeiAmount: BigNumber; gas: number; + slippage: number; } /** diff --git a/packages/asset-swapper/test/exchange_proxy_swap_quote_consumer_test.ts b/packages/asset-swapper/test/exchange_proxy_swap_quote_consumer_test.ts index b381805401..f38f716ae5 100644 --- a/packages/asset-swapper/test/exchange_proxy_swap_quote_consumer_test.ts +++ b/packages/asset-swapper/test/exchange_proxy_swap_quote_consumer_test.ts @@ -125,6 +125,7 @@ describe('ExchangeProxySwapQuoteConsumer', () => { gas: Math.floor(Math.random() * 8e6), protocolFeeInWeiAmount: getRandomAmount(), feeTakerTokenAmount: getRandomAmount(), + slippage: 0, }, worstCaseQuoteInfo: { makerAmount: makerTokenFillAmount, @@ -133,6 +134,7 @@ describe('ExchangeProxySwapQuoteConsumer', () => { gas: Math.floor(Math.random() * 8e6), protocolFeeInWeiAmount: getRandomAmount(), feeTakerTokenAmount: getRandomAmount(), + slippage: 0, }, makerAmountPerEth: getRandomInteger(1, 1e9), takerAmountPerEth: getRandomInteger(1, 1e9), diff --git a/packages/asset-swapper/test/utils/swap_quote.ts b/packages/asset-swapper/test/utils/swap_quote.ts index ac5edc8432..26c4c6e063 100644 --- a/packages/asset-swapper/test/utils/swap_quote.ts +++ b/packages/asset-swapper/test/utils/swap_quote.ts @@ -24,6 +24,7 @@ export async function getFullyFillableSwapQuoteWithNoFeesAsync( totalTakerAmount: takerAmount, protocolFeeInWeiAmount: protocolFeePerOrder.times(orders.length), gas: 200e3, + slippage: 0, }; const breakdown = {