search

harvest-finance / harvest

Bread for the people!
218 stars 94 forks source link

sanity check around input parameter of notifyRewardAmount(uint256 reward) #7

Closed rayeaster closed 4 years ago

rayeaster commented 4 years ago

in the https://github.com/harvest-finance/harvest/blob/master/contracts/RewardPool.sol#L753 a sanity value check around the resulting rewardRate needed?

say if by some mistake, the rewardRate is set to very very big value, then following call on modifier updateReward (specifically rewardPerToken) might get revert on SafeMath.add due to overflow?

rayeaster commented 4 years ago

hear from discrod Brandon Radar: this is already discussed and resolved. @Byron-McKeeby @apecap would you please confirm it is fixed and close this issue, Thanks

I believe the DelayMinter that's been implemented since does the check described in the issue.
https://etherscan.io/address/0x284d7200a0dabb05ee6de698da10d00df164f61d#code

I misspoke, according to my notes there's a contract called NotifyHelper somewhere that does this.
https://github.com/harvest-finance/harvest/blob/master/contracts/NotifyHelper.sol

looks like we did not use the notifyhelper based on the notification that occurred for this recently deployed stakepool. 
https://etherscan.io/tx/0x3f71ae957f200473a893e63aa6b4b2c4c1e538a92b48831b302d64785d82b458

checking to see if the stakepool contract has been updated and/or the rewarddistribution is zeroed out. 
https://etherscan.io/address/0x8bcbf139b8d7b26f37d89f2c8aa9de810b5a3814#code
breadforthepeople commented 4 years ago

Confirming that this is resolved.