search

makerdao / univ2-lp-oracle

GNU Affero General Public License v3.0
23 stars 13 forks source link

square on the uniswap reserve #21

Closed alexisgayte closed 3 years ago

alexisgayte commented 3 years ago

Just looked at that, There is a known vector attack on uniswap which is the liquidity removal. As there is no fees in v1 and not yet in v2, Someone can add and remove all, then manipulate the reserve. While I don't believe there is a major issue on big pair, it can be on small pair.

I would check the deviation from your oracle and back fall to it if the gap is too big or reserves are diverging from the average. or limiting the gap from your oracle let say +-10%.

https://github.com/makerdao/univ2-lp-oracle/blob/c500d2197f153f70da0e3005e74b87c37ee33a40/src/Univ2LpOracle.sol#L247

alexisgayte commented 3 years ago

Never mind as the k should not change.

alexisgayte commented 3 years ago

I am reopening this one, as I am a bit confused with it to be honest. As I read the comment there is an assumption that the k is constant, but it is not.

require(balance0Adjusted.mul(balance1Adjusted) >= uint(_reserve0).mul(_reserve1).mul(1000**2), 'UniswapV2: K');

the K is >=, it is not constant.

I guess everyone had a look at this one anyway.

On side note, why don't you use the average price facility offered by uniswap? price0CumulativeLast

brianmcmichael commented 3 years ago

Please read the notes. The uniswap provided price is not accurate.