Conversation
decaf-addict
left a comment
decaf-addict
left a comment
There was a problem hiding this comment.
Over all looks good, will wait for fixes to give the final LG! Thanks for tackling this @newmickymousse !
| if (getPendingBeets() > 0) { | ||
| uint256 rewardBal = balanceOfReward(); | ||
| uint256 prevBeetsBal = balanceOfBeets(); | ||
| masterChef.harvest(masterChefPoolId, address(this)); |
There was a problem hiding this comment.
question: how is TUSD rewards claimed? Will it need to be claimed synchronously too?
There was a problem hiding this comment.
Yes.. the harvest will send both beets and tusd
| // this allows us to also sell bpt externally | ||
| function sellBpt(uint256 _amountBpts) external onlyVaultManagers { | ||
| _sellBpt(_amountBpts); | ||
| } |
There was a problem hiding this comment.
should put this back along with any other external functions deleted and implement the modifier trick. It'll save about ~4% bytecode size. The external functions are pretty important and have saved us in the past
There was a problem hiding this comment.
There was a problem hiding this comment.
Adding this and bringing back the external functions
contracts/Strategy.sol
Outdated
| IERC20[] memory noRewardTokens; | ||
| rewardTokens = noRewardTokens; |
There was a problem hiding this comment.
does delete not work for rewardTokens? Should be able to reset it to default value like swapSteps
| swapSteps = _steps; | ||
| // for partnership rewards like TUSD or airdrops | ||
| function whitelistRewards(address _rewardToken, SwapSteps memory _steps) public isVaultManager { | ||
| IERC20 token = IERC20(_rewardToken); |
There was a problem hiding this comment.
Should check that this is not want, so we dont max approve here
There was a problem hiding this comment.
https://github.com/yearn/yearn-strategies/issues/247#issuecomment-1078471499
gave more reasons here
3 participants
No description provided.