Thanks for the code pointer, and I agree with you it looks like the deposit function is quite sleek already. The only thing I can think of is the balance view function being calculated for every deposit, which has to do a bunch of nested calls, but my instinct is this doesnāt cost too much gas.
The withdrawal function looks like it could benefit from the methods discussed in this post though:
function withdraw(uint _shares) public {
uint r = (balance().mul(_shares)).div(totalSupply());
_burn(msg.sender, _shares);
// Check balance
uint b = token.balanceOf(address(this));
if (b < r) {
uint _withdraw = r.sub(b);
Controller(controller).withdraw(address(token), _withdraw);
uint _after = token.balanceOf(address(this));
uint _diff = _after.sub(b);
if (_diff < _withdraw) {
r = b.add(_diff);
}
}
token.safeTransfer(msg.sender, r);
}
It looks like the vault already matches a user attempting to withdraw with pending deposits, which is great. But for a user withdrawing an amount greater than the pending deposits, theyāll have to withdraw from the controller, which withdraws from the strategy, which, depending on the strategy, may be expensive.
Furthermore, based on how the system works right now (not harvesting upon each deposit/withdrawal), it seems thereās a subtle (and not severe) āhackā a user could do to withdraw more interest than theyāve earned. Deposits are given shares equal to the current balance of the vault, but this may not necessarily reflect the accrued interest thatās pending being rotated back into the base crop. Specifically, a user could take the following sequence of actions to get slightly more returns than they should (using yWETH vault in my example):
- A user named Eve waits until right before they anticipate
harvest being called. Say harvest on average rotates 10% of the vaultās balance as interest back into the base crop (ie. selling accrued CRV for WETH, or whatever). Say harvest is called on average once per hour.
- Eve deposits a bunch of WETH. The way deposit works right now, Eve would get yWETH vault shares based off the current balances of ETH, but not accounting for pending accrued interest.
-
harvest is called 1 minute after Eveās deposit. Eveās pool shares increase in value based on the accrued interest from the whole past hour, rather than just the past 1 minute.
- Eve withdraws 1 minute after
harvest.
After this sequence of events, Eve only had her money in the vault for ~2 minutes, but she accrued interest for the entire past hour.
A system with a more explicit layer of a ābrokerā contract on top could solve this by having deposits and withdrawals being a part of harvest. End-users interact with the broker contract to stage their deposits/withdrawals, and then harvest processes them immediately after rotating any pending interest back to the desired underlying asset.
Sorry if this is wordy or hard to follow-- let me know if I can clarify or if Iām missing something obvious that makes this work differently than Iām describing.
Contract addresses for reference:
