16slim/ve angle #6
Conversation
|
|
src/Strategy.sol
Outdated
| // ----------------- SUPPORT & UTILITY FUNCTIONS ---------- | ||
|
|
||
| function balanceOfWant() public view returns (uint256) { | ||
| return want.balanceOf(address(this)); | ||
| } | ||
|
|
||
| function balanceOfStakedSanToken() public view returns (uint256) { | ||
| return IERC20(address(sanTokenGauge)).balanceOf(address(this)); | ||
| return strategyProxy.balanceOf(address(sanTokenGauge)); |
There was a problem hiding this comment.
Maybe we could add a helper function like you did with balanceOfSanToken?
There was a problem hiding this comment.
Sorry I am not understanding, it's the same case as balanceOfSanToken no? Calling a function in the proxy
There was a problem hiding this comment.
I was saying add a balanceOfStakedSanToken function in the proxy, like this:
angle_protocol/src/AngleStrategyVoterProxy.sol
Lines 167 to 169 in d7571d9
|
Passing: |
|
LGTM! Think it would be great to get @wavey0x review as well if he's able |
|
|
src/AngleStrategyVoterProxy.sol
Outdated
| voters[_voter] = false; | ||
| } | ||
|
|
||
| function lock(uint256 amount, uint256 unlockTime) external { |
There was a problem hiding this comment.
Do you see any harm adjusting this function to have zero arguments? My thought is that we always max lock the full balance. There is no access control on this function, what if someone locks for less than max? Do we care?
There was a problem hiding this comment.
I included a msg.sender check to it too to check for governance, I normally prefer to leave things as flexible as possible just in case. Wdyt?
src/AngleStrategyVoterProxy.sol
Outdated
|
|
||
| function lock(uint256 amount, uint256 unlockTime) external { | ||
| if (amount > 0) { | ||
| IERC20(angleToken).transfer(address(yearnAngleVoter), amount); |
There was a problem hiding this comment.
I'd like to define the expected behavior of this function a bit better. Ideally we have an open .lock() function which allows anyone to max lock any ANGLE in the voter on our behalf.
Seems this design is expecting the proxy to carry an ANGLE token balance. Usually all tokens kept will be in the voter, not proxy. Which means this logic will probably get skipped.
There was a problem hiding this comment.
I had the flow to leave the tokens in voter proxy rather than voter as I wanted to the voter to be a mere interface for veAngle. But as I think it's standard design, I changed it to Angle rewards be kept in the voter instead
src/Strategy.sol
Outdated
|
|
||
| uint256 _stakedBalance = balanceOfStakedSanToken(); | ||
| if (_stakedBalance > 0) { | ||
| strategyProxy.withdraw(address(sanTokenGauge), address(sanToken), _stakedBalance); |
There was a problem hiding this comment.
I forgot about its existence, should be same result but replacing as it's exactly its purpose!
|
All passing: |
|
|
Necessary changes for foundry migration!