hifi-finance / hifi

Monorepo implementing the Hifi fixed-rate, fixed-term lending protocol
https://app.hifi.finance
Other
106 stars 15 forks source link

[flash-swap] Handle division by zero in "getRepayAmount" function #68

Closed PaulRBerg closed 2 years ago

PaulRBerg commented 3 years ago

While working on #67, I realized that there is an edge case which is currently not handled by the Uniswap v2 flash swap contract:

AssertionError: Expected transaction to be reverted with FlashUniswapV2__TurnoutNotSatisfied, but other exception was thrown: Error: VM Exception while processing transaction: reverted with panic code 0x12 (Division or modulo division by zero)

That error originates in the getRepayAmount function, when the underlyingReserves happens to match underlyingAmount. The Uniswap v2 pair contract will check for the latter to not be greater than the former, but it will allow it to pass when they are equal. That is because it is possible to have a big amount of one token and a zero amount of the other token in a Uniswap v2 pool.

I can't think off a solution off the top of my head.

scorpion9979 commented 2 years ago

The Uniswap v2 pair contract will check for the latter to not be greater than the former, but it will allow it to pass when they are equal.

I don't think the above premise is valid. I think this edge case is never reachable due to line 162 in the Uniswap V2 pair contract:

require(amount0Out < _reserve0 && amount1Out < _reserve1, 'UniswapV2: INSUFFICIENT_LIQUIDITY');

When underlyingReserves happens to match underlyingAmount, that means either amount0Out matches _reserve0 or amount1Out matches _reserve1. Which leads to the swap function call being reverted with UniswapV2: INSUFFICIENT_LIQUIDITY even before uniswapV2Call(...) is entered.

PaulRBerg commented 2 years ago

Your analysis may be correct, though I am not sure now since it's been a while since I opened this issue. I will let you decide if it should be closed.

scorpion9979 commented 2 years ago

Yeah, I think it should be closed.