Shouldn't fees be excluded when estimating the price impact?

[Voyz]

In https://github.com/uniswap-python/uniswap-python/blob/master/uniswap/uniswap.py#L1879:L1920 estimate_price_impact function the price impact is estimated by operating on the current raw price of the pool and the output of get_price_input call.

• Do I understand correctly that the get_price_input includes the change resulting from both the price impact and the fee?
• If so, why isn't the fee excluded from the price impact to provide the accurate price impact?

In Uniswap Interface, they refer to this 'accurate price impact' as 'realised', and indeed it gets calculated by taking the price impact, and subtracting the fees from it:

export function computeRealizedPriceImpact(trade: Trade<Currency, Currency, TradeType>): Percent {
  const realizedLpFeePercent = computeRealizedLPFeePercent(trade)
  return trade.priceImpact.subtract(realizedLpFeePercent)
}

Where trade.priceImpact is the calculated price impact, and the computeRealizedLPFeePercent is a function that gets the total fees of the trade expressed as a percentage.

Note, that Uniswap Interface uses the Auto Router, which could carry out a single trade through different swaps, meaning that there would be multiple fees that need to be taken into account, hence the computeRealizedLPFeePercent() function that calculates these. However in case of uniswap-python library we don't use the Auto Router (at the time of writing this), hence we could simply subtract the fixed percentage fee which we know as it is passed as a parameter of estimate_price_impact function. Ie.:

fee_percentage = fee / 1000000
return float((price_small - price_amount) / price_small - fee_percentage ) 

Hence:

• Would this, in a sense, be a more accurate estimation of the actual price impact?
• Naturally, the resulting swap output the user receives from a trade would include the impact of both the fees and the price impact.
• Still, wouldn't it be more clear to explicitly separate the estimation of the price impact from estimating fees, and have the user check for these separately if they wanted to? It makes sense to me, seeing that this is what the Uniswap Interface does.

[ErikBjare]

This is a very good observation, thank you.

I'd be happy to accept PRs addressing this.

[Voyz]

@ErikBjare added PR addressing this: #325