9 - emergencyWithdraw override function to withdraw funds after strategy shutdown #44
Conversation
…strategy shutdown
mcclurejt
changed the title
mcclurejt
changed the title
| /// likelihood of reverts. | ||
| function _emergencyWithdraw(uint256) internal override { | ||
| IEverlongStrategy.EverlongPosition memory position; | ||
| while (!_portfolio.isEmpty()) { |
There was a problem hiding this comment.
there might be a lot of open positions and closing them is not cheap. worst case, you can't fit them in a single block. consider just closing positions until at least amount parameter is reached (can still fully close the individual positions, something like while (!empty && closed < amount) { ... closed += bondAmount }. And then change the natspec that amount is actually a bond amount).
There was a problem hiding this comment.
Added the change as described with one difference...
I found it more straightforward to have the parameter be a hard limit, meaning that the amount of bonds transferred is guaranteed not to exceed it.
There was a problem hiding this comment.
Quick question: according to the current implementation, the limit is put on the bond amount, and the current position to work on is position = _portfolio.head();. Then, is there a possibility that once a very big position holding more than the maxBondAmount stands in the way, you won't be able to get around it even though there are still other smaller positions could have been able to be closed?
If the concern is on the gas cost & block size limit, maybe imposing the limit on the # of positions closed works better than using the bond amount?
There was a problem hiding this comment.
Then, is there a possibility that once a very big position holding more than the maxBondAmount stands in the way, you won't be able to get around it even though there are still other smaller positions could have been able to be closed?
so this queue is ordered and you can inspect it from off-chain and then see how much needs to be closed. I think in practice it doesn't make much difference if you use maxBondAmount for the bond amount or number of positions to close.
I just felt like bond amount was more flexible.
Maybe you're suggesting passing in an array of maturities to close so you can skip the head, but unfortunately this uint256 parameter is given by yearn's API and you can't change it.
Use the input `uint256` in `_emergencyWithdraw` as a limit on the maximum number of bonds to be transferred. Add tests to ensure it works.
This pr implements the
_emergencyWithdrawstrategy hook so that funds can be withdrawn when the strategy is shut down.The hook transfers all hyperdrive long tokens to the trusted
Emergency Adminaddress so that they can be closed and the proceeds transferred back to the strategy.issue