Withdrawals from idle can frontrun losses #50
Description
Description
Withdrawals through the vault first withdraw all vault idle. Next, the strategy's idle is withdrawn and if there are not enough funds, longs must closed. This is done in _freeFunds(_amount = withdrawAmount - idle). This function recomputes the current portfolio value. Losses are then computed as currentTotalAssets (= totalIdle + portfolioValue) - previousTotalAssets. The proportional loss attributed to the user is computed as (_amount / previousTotalAssets) * loss. Note that _amount here is the withdraw amount reduced by the total idle. Withdrawers are not penalized with any proportional losses if they withdraw up to the strategy's idle, the _freeFunds function is not even called in this case.
The impact is that the losses are not properly attributed to all withdrawers. The losses should be attributed as (withdrawAmount / previousTotalAssets) * losses (with withdrawAmount = idle + _amount). However, this is technically not possible in yearn-v3's strategies as _freeFunds is only called when the amount to withdraw is above the strategy's idle. The first one to withdraw the idle strategy funds can fully escape the losses. If the loss is later realized (through harvestAndReport()), it is distributed to all other depositors in full.
Recommendation
If the strategy is required to be liquid at all times, keep the strategy's idle low as this leads to withdrawAmount being close to _amount in _freeFunds (call update_debt + tend() often, disable auto-allocate, which moves funds to the strategy's idle instead of keeping them in the vault's idle). Regularly update the strategy's totalAssets() by calling harvestAndReport() to avoid large, natural differences in current and previous total assets.