Closed scorpion9979 closed 2 years ago
I refactored pretty much everything. For a detailed log, see the commit messages. Here's the high level:
HifiFlashUniswapV2
to CollateralFlashUniswapV2
and HifiFlashUniswapV2Underlying
to UnderlyingFlashUniswapV2
. The former names were too verbose, and they lacked symmetry - why does a contract have a suffix but the other sounds like a generic flash swap implementation? With the updated naming, it's much clearer what is the role played by each contract. This came at the cost of removing the "Hifi" prefix, but this contract is meant to be used internally, so it shouldn't affect our business image on Etherscan.FlashUtils
.UnderlyingFlashUniswapV2
such that there is no reference to the word "collateral" anymore. Even if the underlying is used as collateral, it is confusing to assign the role of the "underlying" to the "collateral" during the function execution. That sort of goes against the principle of Static single assignment form.usdcDepositAmount
to depositUsdcAmount
). This is for monorepo-wide consistency purposes.bot
to subsidizer
. The contract need not be called by a bot. Any accounts with the ability to subsidize the transaction cost would do.You can add your review @scorpion9979 and then I will merge the PR.
After another review, I noticed three other issues:
UnderlyingFlashUniswapV2
are pointless. All the tests whose beforeEach
block manipulate the WBTC price (probably copy-pasted from the other contract's test), but in this case it is not possible to have a liquidity shortfall when the borrow is backed by the underlying itself, and there is no swap made, so the UniswapV2Pair
contract does NOT give any price.UnderlyingFlashUniswapV2
should have accounted for the time-contingent nature of underlying-backed vaults.if (vars.repayUnderlyingAmount > vars.seizedUnderlyingAmount)
Any surplus of underlying should be relayed to the caller, just like we're doing in CollateralFlashUniswapV2
.
Already started working on fixes.
This was a hairy refactor ... I made progress with the latest commit, but the following test cases are still not written:
I'll handle them tomorrow.
Regarding liquidating USDC collateral, the following modifications needed to be applied to the flash-swap contract:
getRepayCollateralAmount(...)
function in flash-swap to enable different calculation for this special case.msg.sender
verification in flash-swap, as it previously required the msg sender address to be equal to the pair address of underlying and collateral computed usingpairFor(...)
. Since it's not possible to have a USDC-USDC pair, we needed to constrain this check to the case where the underlying token is not the same as the collateral to be liquidated.