OpenOrg-gg / yearn-euler

GNU Affero General Public License v3.0
1 stars 0 forks source link

Feedback1.2 #3

Open OpenOrg-gg opened 2 years ago

dudesahn commented 2 years ago

you don't need to set this to address(0), will be that by default https://github.com/OpenOrg-gg/yearn-euler/blob/12a8137fbd93533b54294c23f9f1df30873dc21e/contracts/Strategy.sol#L100

dudesahn commented 2 years ago

for all of the other addresses, you also don't need to re-cast them as address(x) https://github.com/OpenOrg-gg/yearn-euler/blob/12a8137fbd93533b54294c23f9f1df30873dc21e/contracts/Strategy.sol#L101 just do

address public constant strategistMultisig = 0x16388463d60FFE0661Cf7F1f31a7D658aC790ff7;
dudesahn commented 2 years ago

you can remove this: https://github.com/OpenOrg-gg/yearn-euler/blob/12a8137fbd93533b54294c23f9f1df30873dc21e/contracts/Strategy.sol#L96 and this: https://github.com/OpenOrg-gg/yearn-euler/blob/12a8137fbd93533b54294c23f9f1df30873dc21e/contracts/Strategy.sol#L98

since these are already inherited from baseStrategy

dudesahn commented 2 years ago

https://github.com/OpenOrg-gg/yearn-euler/blob/12a8137fbd93533b54294c23f9f1df30873dc21e/contracts/Strategy.sol#L108

This doesn't seem to be used anywhere?

dudesahn commented 2 years ago

What are the two addresses here? Would be good to at least have a comment: https://github.com/OpenOrg-gg/yearn-euler/blob/12a8137fbd93533b54294c23f9f1df30873dc21e/contracts/Strategy.sol#L119 https://github.com/OpenOrg-gg/yearn-euler/blob/12a8137fbd93533b54294c23f9f1df30873dc21e/contracts/Strategy.sol#L122