hifi-finance / hifi

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

Implement Flash Swap for Uniswap V3 #98

Closed scorpion9979 closed 1 year ago

scorpion9979 commented 1 year ago

Since the launch of Uniswap V3, trading liquidity has been shifting from Uniswap V2 to Uniswap V3. As we are currently only considering adding WETH, which has a liquidity of $84M, we may not experience any issues at this time. However, if we were to also add WBTC, which has a liquidity of only $200K, we may encounter difficulties with liquidations. To mitigate this risk, we are migrating liquidations to Uniswap V3.

surbhiaudichya commented 1 year ago

Hey @scorpion9979

Description: While testing the FlashUniswapV3 smart contract code, I discovered a bug. The flashLiquidate function always fails because it attempts to borrow underlying from a pool with the same token pair (token0, token1) and fee tier (fee0), and then in the uniswapV3FlashCallback function, it tries to call swap for underlying on the same pool with the same token pair and fee tier. This results in an error message ("LOK: The reentrancy guard. A transaction cannot re-enter the pool mid-swap.") because a flash call transaction is already in process on the same pool with token0, token1, and fee0, and the current implementation tries to call swap before the flash call is completed in the same transaction. To help identify the root cause of the issue, I recommend reviewing the code around line https://github.com/hifi-finance/hifi/blob/4d8e173d143220b59bcd393f9daee3837c9d4516/packages/flash-swap/contracts/uniswap-v3/FlashUniswapV3.sol#L141 and https://github.com/hifi-finance/hifi/blob/4d8e173d143220b59bcd393f9daee3837c9d4516/packages/flash-swap/contracts/uniswap-v3/FlashUniswapV3.sol#L172

One potential solution to this issue is to call swap on a pool with the same token pair but a different fee tier, i.e. token0, token1, fee1. We can discuss the best approach for fixing this bug, but I am willing to take on the task if needed.

scorpion9979 commented 1 year ago

Hey @scorpion9979

Description: While testing the FlashUniswapV3 smart contract code, I discovered a bug. The flashLiquidate function always fails because it attempts to borrow underlying from a pool with the same token pair (token0, token1) and fee tier (fee0), and then in the uniswapV3FlashCallback function, it tries to call swap for underlying on the same pool with the same token pair and fee tier. This results in an error message ("LOK: The reentrancy guard. A transaction cannot re-enter the pool mid-swap.") because a flash call transaction is already in process on the same pool with token0, token1, and fee0, and the current implementation tries to call swap before the flash call is completed in the same transaction. To help identify the root cause of the issue, I recommend reviewing the code around line

https://github.com/hifi-finance/hifi/blob/4d8e173d143220b59bcd393f9daee3837c9d4516/packages/flash-swap/contracts/uniswap-v3/FlashUniswapV3.sol#L141

and https://github.com/hifi-finance/hifi/blob/4d8e173d143220b59bcd393f9daee3837c9d4516/packages/flash-swap/contracts/uniswap-v3/FlashUniswapV3.sol#L172

One potential solution to this issue is to call swap on a pool with the same token pair but a different fee tier, i.e. token0, token1, fee1. We can discuss the best approach for fixing this bug, but I am willing to take on the task if needed.

Thanks for this, @surbhiaudichya. I've addressed this issue with the latest commits by utilizing two Uniswap V3 pools: one pool to flash the underlying tokens, and another pool of the same token pair but with a different fee tier to sell the liquidated collateral.