Upgrade timelock misc - #12
Conversation
| _withdrawUnderlyingToVault(maxAmount, false); | ||
| updateBalance(); | ||
| _deleverage(type(uint256).max); | ||
| _withdrawUnderlyingToVault(balanceOfPool, false); |
There was a problem hiding this comment.
sending max uint to withdrawUnderlyingToVault overflows when doing the slippage check.
| return harvestLog.length; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
contract size issues.
There was a problem hiding this comment.
I think it would be preferable to keep the log slice function. It could be used by our front-end for an automated solution of our APY display problems. From example you call the function with (5), get [100, 90, 53535, 123, 88, 95] <-- an algorithm can analyze this and throw out the 3rd number, then make a new average.
There was a problem hiding this comment.
There is already a function to get the size of the log, and any element in the log can be queried directly by index. I have some other ideas to reduce contract size in #11 but that leads to a lot of refactoring then.
| */ | ||
| modifier doUpdateBalance { | ||
| _; | ||
| updateBalance(); |
There was a problem hiding this comment.
Modifier adds a lot of contract size doesn't it? Since it just pastes in the whole function. I think it is preferable to call the updateBalance() where needed to save contract size and keep some of these other functions.
There was a problem hiding this comment.
Contract size with modifier is 24.438. Without modifier is 24.436. No difference really.
| require(_newWftmToWantRoute[0] == WFTM, "bad route"); | ||
| require(_newWftmToWantRoute[_newWftmToWantRoute.length - 1] == want, "bad route"); | ||
| delete wftmToWantRoute; | ||
| wftmToWantRoute = _newWftmToWantRoute; |
There was a problem hiding this comment.
Isn't re-assigning the variable also deleting the old one? Why the need to delete?
There was a problem hiding this comment.
Probably. Just being a bit paranoid about it.
|
I wonder if we should add another role like "keeper" and add this to the setTargetLTV function. This would allow us to add a harvester bot script to monitor leveraging profitability and to have automatic leveraging for yield optimization. |
this is a good idea, I can make this change. |
2 participants
No description provided.