-
Notifications
You must be signed in to change notification settings - Fork 55
[ABA] Parallel BA request + naive formula + warning messages #1953
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,106 +1,74 @@ | ||
import { useEffect, useMemo, useState } from 'react' | ||
import { useEffect, useState } from 'react' | ||
import { Percent } from '@uniswap/sdk-core' | ||
import { OrderKind } from '@gnosis.pm/gp-v2-contracts' | ||
|
||
import { useSwapState } from 'state/swap/hooks' | ||
import { tryParseAmount, useSwapState } from 'state/swap/hooks' | ||
import { Field } from 'state/swap/actions' | ||
import { useGetQuoteAndStatus } from 'state/price/hooks' | ||
import { QuoteInformationObject } from 'state/price/reducer' | ||
import { QuoteError } from 'state/price/actions' | ||
import { DEBOUNCE_TIME } from 'state/price/updater' | ||
|
||
import useExactInSwap, { useCalculateQuote } from './useQuoteAndSwap' | ||
import { FallbackPriceImpactParams } from './commonTypes' | ||
import { calculateFallbackPriceImpact, FeeQuoteParams } from 'utils/price' | ||
import TradeGp from 'state/swap/TradeGp' | ||
import { useActiveWeb3React } from 'hooks/web3' | ||
import { QuoteInformationObject } from 'state/price/reducer' | ||
import { QuoteError } from 'state/price/actions' | ||
|
||
type SwapParams = { trade?: TradeGp; sellToken?: string | null; buyToken?: string | null } | ||
import { useCalculateQuote, useExactInSwap, useExactOutSwap } from 'hooks/usePriceImpact/useQuoteAndSwap' | ||
import { useCurrency } from 'hooks/Tokens' | ||
import useDebounce from 'hooks/useDebounce' | ||
|
||
function _isQuoteValid(quote: QuoteInformationObject | FeeQuoteParams | undefined): quote is QuoteInformationObject { | ||
return Boolean(quote && 'lastCheck' in quote) | ||
} | ||
|
||
function _calculateSwapParams(isExactIn: boolean, { trade, sellToken, buyToken }: SwapParams) { | ||
if (!trade) return undefined | ||
|
||
if (isExactIn) { | ||
return { | ||
outputCurrency: trade.inputAmount.currency, | ||
// Run inverse (B > A) sell trade | ||
sellToken: buyToken, | ||
buyToken: sellToken, | ||
fromDecimals: trade.outputAmount.currency.decimals, | ||
toDecimals: trade.inputAmount.currency.decimals, | ||
} | ||
} else { | ||
// First trade was a buy order | ||
// we need to use the same order but make a sell order | ||
return { | ||
outputCurrency: trade.outputAmount.currency, | ||
// on buy orders we dont inverse it | ||
sellToken, | ||
buyToken, | ||
fromDecimals: trade.inputAmount.currency.decimals, | ||
toDecimals: trade.outputAmount.currency.decimals, | ||
} | ||
} | ||
} | ||
|
||
function _calculateParsedAmount(trade: TradeGp | undefined, isExactIn: boolean, shouldCalculate: boolean) { | ||
if (!shouldCalculate || !trade) return undefined | ||
// First trade was a sell order, we need to make a new sell order using the | ||
// first trade's output amount | ||
const amount = isExactIn ? trade.outputAmount : trade.inputAmount | ||
|
||
return amount | ||
} | ||
|
||
export default function useFallbackPriceImpact({ abTrade, fiatPriceImpact }: FallbackPriceImpactParams) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this method could have some more comments, both in the method and the implementation |
||
const { | ||
typedValue, | ||
typedValue: unbouncedTypedValue, | ||
INPUT: { currencyId: sellToken }, | ||
OUTPUT: { currencyId: buyToken }, | ||
independentField, | ||
} = useSwapState() | ||
const isExactIn = independentField === Field.INPUT | ||
const { chainId } = useActiveWeb3React() | ||
|
||
// Should we even calc this? Check if fiatPriceImpact exists | ||
const typedValue = useDebounce(unbouncedTypedValue, DEBOUNCE_TIME) | ||
|
||
const shouldCalculate = !Boolean(fiatPriceImpact) | ||
|
||
// Calculate the necessary params to get the inverse trade impact | ||
const { parsedAmount, outputCurrency, ...swapQuoteParams } = useMemo( | ||
() => ({ | ||
parsedAmount: _calculateParsedAmount(abTrade, isExactIn, shouldCalculate), | ||
..._calculateSwapParams(isExactIn, { trade: abTrade, sellToken, buyToken }), | ||
}), | ||
[abTrade, buyToken, sellToken, shouldCalculate, isExactIn] | ||
) | ||
const inputCurrency = useCurrency(sellToken) | ||
const outputCurrency = useCurrency(buyToken) | ||
|
||
const isExactIn = independentField === Field.INPUT | ||
const parsedAmount = shouldCalculate | ||
? tryParseAmount(typedValue, (isExactIn ? inputCurrency : outputCurrency) ?? undefined) | ||
: undefined | ||
|
||
const { quote, loading: loading } = useCalculateQuote({ | ||
amountAtoms: parsedAmount?.quotient.toString(), | ||
...swapQuoteParams, | ||
kind: isExactIn ? OrderKind.BUY : OrderKind.SELL, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is asking for comment. I understand from the code:
Anyways, Im not convinced that we should use BUY orders in any case, I will explain in the summary. |
||
sellToken: buyToken, | ||
buyToken: sellToken, | ||
fromDecimals: outputCurrency?.decimals, | ||
toDecimals: inputCurrency?.decimals, | ||
}) | ||
|
||
const baTradeIn = useExactInSwap({ | ||
quote: _isQuoteValid(quote) ? quote : undefined, | ||
parsedAmount: !isExactIn ? parsedAmount : undefined, | ||
outputCurrency: inputCurrency ?? undefined, | ||
}) | ||
|
||
// we calculate the trade going B > A | ||
// using the output values from the original A > B trade | ||
const baTrade = useExactInSwap({ | ||
// if impact, give undefined and dont compute swap | ||
// the amount traded now is the A > B output amount without fees | ||
// TODO: is this the amount with or without fees? | ||
const baTradeOut = useExactOutSwap({ | ||
quote: _isQuoteValid(quote) ? quote : undefined, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can u extract |
||
parsedAmount, | ||
outputCurrency, | ||
parsedAmount: isExactIn ? parsedAmount : undefined, | ||
inputCurrency: outputCurrency ?? undefined, | ||
}) | ||
|
||
const baTrade = baTradeIn || baTradeOut | ||
|
||
const [impact, setImpact] = useState<Percent | undefined>() | ||
const [error, setError] = useState<QuoteError | undefined>() | ||
|
||
// we set price impact to undefined when loading a NEW quote only | ||
const { isGettingNewQuote } = useGetQuoteAndStatus({ token: sellToken, chainId }) | ||
|
||
// primitive values to use as dependencies | ||
const abIn = abTrade?.inputAmount.quotient.toString() | ||
const abOut = abTrade?.outputAmount.quotient.toString() | ||
const baIn = baTrade?.inputAmount.quotient.toString() | ||
const baOut = baTrade?.outputAmount.quotient.toString() | ||
const quoteError = quote?.error | ||
|
||
|
@@ -109,26 +77,21 @@ export default function useFallbackPriceImpact({ abTrade, fiatPriceImpact }: Fal | |
if (quoteError) { | ||
setImpact(undefined) | ||
setError(quoteError) | ||
} else if (!loading && abIn && abOut && baOut) { | ||
const impact = calculateFallbackPriceImpact(isExactIn ? abIn : abOut, baOut) | ||
} else if (!loading && abIn && abOut && baIn && baOut) { | ||
let impact = undefined | ||
if (isExactIn) { | ||
impact = calculateFallbackPriceImpact(abOut, baIn) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't get it, shouldn't this abIn, baOut? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now i get it, kind of, but maybe we can add a few comments to make it easier. |
||
} else { | ||
impact = calculateFallbackPriceImpact(abIn, baOut) | ||
} | ||
setImpact(impact) | ||
setError(undefined) | ||
} else { | ||
// reset all | ||
setImpact(undefined) | ||
setError(undefined) | ||
} | ||
}, [abIn, abOut, baOut, quoteError, isExactIn, loading]) | ||
|
||
// on changes to typedValue, we hide impact | ||
// quote loading so we hide impact | ||
// prevents lingering calculations and jumping impacts | ||
useEffect(() => { | ||
if (typedValue || isGettingNewQuote) { | ||
setImpact(undefined) | ||
setError(undefined) | ||
} | ||
}, [isGettingNewQuote, typedValue]) | ||
}, [abIn, abOut, baIn, baOut, quoteError, isExactIn, loading, unbouncedTypedValue, sellToken, buyToken]) | ||
|
||
return { impact, error, loading } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,53 +1,74 @@ | ||
import { ChainId, WETH } from '@uniswap/sdk' | ||
import { CurrencyAmount, Percent, Token } from '@uniswap/sdk-core' | ||
import BigNumber from 'bignumber.js' | ||
import { Token } from '@uniswap/sdk-core' | ||
import { parseUnits } from 'ethers/lib/utils' | ||
|
||
function _calculateAbaPriceImpact(initialValue: string, finalValue: string) { | ||
const initialValueBn = new BigNumber(initialValue) | ||
const finalValueBn = new BigNumber(finalValue) | ||
// TODO: use correct formula | ||
// ((IV - FV) / IV / 2) * 100 | ||
const [numerator, denominator] = initialValueBn.minus(finalValueBn).div(initialValueBn).div('2').toFraction() | ||
|
||
return new Percent(numerator.toString(), denominator.toString()) | ||
} | ||
import { calculateFallbackPriceImpact } from './price' | ||
|
||
const WETH_MAINNET = new Token(ChainId.MAINNET, WETH[1].address, 18) | ||
const DAI_MAINNET = new Token(ChainId.MAINNET, '0x6b175474e89094c44da98b954eedeac495271d0f', 18) | ||
|
||
describe('A > B > A Price Impact', () => { | ||
const AB_IN = parseUnits('1', WETH_MAINNET.decimals).toString() | ||
const AB_OUT = parseUnits('1000', DAI_MAINNET.decimals).toString() | ||
|
||
const abIn = CurrencyAmount.fromRawAmount(WETH_MAINNET, AB_IN) | ||
const abOut = CurrencyAmount.fromRawAmount(DAI_MAINNET, AB_OUT) | ||
|
||
describe('[SELL] WETH --> DAI', () => { | ||
it('A > B > A SELL return proper price impact', () => { | ||
// GIVEN a 1 WETH >> 1000 DAI AB Trade | ||
// GIVEN a 1000 DAI >> 0.5 WETH BA Trade | ||
const BA_OUT = parseUnits('0.5', WETH_MAINNET.decimals).toString() | ||
const baOut = CurrencyAmount.fromRawAmount(WETH_MAINNET, BA_OUT) | ||
// THEN we expect price impact to be 25 | ||
// (1 - 0.5) / 1 / 2 * 100 | ||
// BUY order = last param TRUE | ||
const abaImpact = _calculateAbaPriceImpact(abIn.quotient.toString(), baOut.quotient.toString()) | ||
expect(abaImpact.toSignificant(2)).toEqual('25') | ||
}) | ||
}) | ||
const ABA_CASES = [ | ||
{ | ||
initialValue: parseUnits('80', DAI_MAINNET.decimals).toString(), | ||
finalValue: parseUnits('56', DAI_MAINNET.decimals).toString(), | ||
expectation: '15', | ||
description: '[SELL] 100 WETH > 80 DAI > 56 WETH', | ||
}, | ||
{ | ||
initialValue: parseUnits('0.48', WETH_MAINNET.decimals).toString(), | ||
finalValue: parseUnits('0.61', WETH_MAINNET.decimals).toString(), | ||
expectation: '-14', | ||
description: '[BUY] 700 DAI > 0.48 WETH > 0.61 WETH', | ||
}, | ||
{ | ||
initialValue: parseUnits('80', DAI_MAINNET.decimals).toString(), | ||
finalValue: parseUnits('56', DAI_MAINNET.decimals).toString(), | ||
expectation: '15', | ||
description: '[SELL] 100 WETH > 80 DAI > 56 WETH', | ||
}, | ||
] | ||
|
||
// describe('A > B > A Price Impact', () => { | ||
// const AB_IN = parseUnits('1', WETH_MAINNET.decimals).toString() | ||
// const AB_OUT = parseUnits('1000', DAI_MAINNET.decimals).toString() | ||
|
||
describe('[BUY] DAI --> WETH', () => { | ||
it('A > B > A BUY returns proper price impact', () => { | ||
// GIVEN a 1000 DAI >> 1 WETH BUY AB Trade | ||
// GIVEN a 1 WETH >> 800 WETH SELL BA Trade | ||
const BA_OUT = parseUnits('800', DAI_MAINNET.decimals).toString() | ||
const baOut = CurrencyAmount.fromRawAmount(DAI_MAINNET, BA_OUT) | ||
// THEN we expect price impact to be 25 | ||
// (1000 - 800) / 1000 / 2 * 100 = 10 | ||
// BUY order = last param FALSE | ||
const abaImpact = _calculateAbaPriceImpact(abOut.quotient.toString(), baOut.quotient.toString()) | ||
expect(abaImpact.toSignificant(2)).toEqual('10') | ||
// const abIn = CurrencyAmount.fromRawAmount(WETH_MAINNET, AB_IN) | ||
// const abOut = CurrencyAmount.fromRawAmount(DAI_MAINNET, AB_OUT) | ||
|
||
// describe('[SELL] WETH --> DAI', () => { | ||
// it('A > B > A SELL return proper price impact', () => { | ||
// // GIVEN a 1 WETH >> 1000 DAI AB Trade | ||
// // GIVEN a 1000 DAI >> 0.5 WETH BA Trade | ||
// const BA_OUT = parseUnits('0.5', WETH_MAINNET.decimals).toString() | ||
// const baOut = CurrencyAmount.fromRawAmount(WETH_MAINNET, BA_OUT) | ||
// // THEN we expect price impact to be 25 | ||
// // (1 - 0.5) / 1 / 2 * 100 | ||
// // BUY order = last param TRUE | ||
// const abaImpact = _calculateAbaPriceImpact(abIn.quotient.toString(), baOut.quotient.toString()) | ||
// expect(abaImpact.toSignificant(2)).toEqual('25') | ||
// }) | ||
// }) | ||
|
||
// describe('[BUY] DAI --> WETH', () => { | ||
// it('A > B > A BUY returns proper price impact', () => { | ||
// // GIVEN a 1000 DAI >> 1 WETH BUY AB Trade | ||
// // GIVEN a 1 WETH >> 800 WETH SELL BA Trade | ||
// const BA_OUT = parseUnits('800', DAI_MAINNET.decimals).toString() | ||
// const baOut = CurrencyAmount.fromRawAmount(DAI_MAINNET, BA_OUT) | ||
// // THEN we expect price impact to be 25 | ||
// // (1000 - 800) / 1000 / 2 * 100 = 10 | ||
// // BUY order = last param FALSE | ||
// const abaImpact = _calculateAbaPriceImpact(abOut.quotient.toString(), baOut.quotient.toString()) | ||
// expect(abaImpact.toSignificant(2)).toEqual('10') | ||
// }) | ||
// }) | ||
// }) | ||
|
||
describe('A > B > A Price Impact', () => { | ||
ABA_CASES.forEach(({ initialValue, finalValue, expectation, description }) => { | ||
it(description, () => { | ||
const abaImpact = calculateFallbackPriceImpact(initialValue.toString(), finalValue.toString()) | ||
expect(abaImpact.toSignificant(2)).toEqual(expectation) | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why defining
FallbackPriceImpactParams
in commonTypes instead of this file. It seems better to me to be close to the function that uses it