stake() needs to be pausable for completed incentives and two bug fixes.
Pausablecontract and add
stake()to prevent staking into deprecated pools
- Fix a potential overflow bug in the reward notification function reported by samcsun
- Fix to
rewardsDurationto be updated after the initial setting
StakingRewards campaign has completed the contract needs to prevent anyone from staking into it. The staker will not accrue any rewards and can cause blocking issues with inverse Synths that need to be purged so that they can be relanced.
Pausable.sol and modifier
stake() will allow the admin to set
true preventing anyone from staking.
SelfDestructible has not been implemented and given the amount of value in these contracts probably best not to implement.
There is a multiplication overflow that can occur inside the rewardPerToken function, on line 66:
An overflow occurs whenever
rewardRate >= 2^256 / (10^18 * (lastTimeRewardApplicable() - lastUpdateTime)).
This can happen when the updateReward modifier is invoked, which will cause the following functions to revert:
The reward rate is set inside
notifyRewardAmount, if a value that is too large is provided to the function.
Of particular note is that
notifyRewardAmount is itself affected by this problem, which means that if the provided
reward is incorrect, then the problem is unrecoverable.
notifyRewardAmount transaction should be reverted if a value greater than
2^256 / 10^18 is provided.
As an additional safety mechanism, this value will be required to be no greater than the remaining
balance of the rewards token in the contract. This will both prevent the overflow, and also provide an additional check
that the reward rate is being set to a value in the appropriate range (for example, no extra/missing zeroes).
rewardRate = floor(reward / rewardsDuration) = (reward - k) / rewardsDuration
0 <= k < rewardsDuration.
For the bug to occur, we need:
(reward - k) / rewardsDuration >= 2^256 / (10^18 * (lastTimeRewardApplicable - lastUpdateTime)) reward >= rewardsDuration * 2^256 / (10^18 * (lastTimeRewardApplicable - lastUpdateTime)) + k
Hence, we can ensure the bug does not occur if we force:
reward < rewardsDuration * 2^256 / (10^18 * (lastTimeRewardApplicable - lastUpdateTime))
So we should constrain
reward to be less than the minimum value of the RHS.
The smallest possible value of
lastUpdateTime is the block timestamp when
notifyRewardAmount was last called.
The largest possible value of
periodFinish = notificationBlock.timestamp + rewardsDuration (line 121).
Putting these together we have:
(lastTimeRewardApplicable - lastUpdateTime) <= rewardsDuration
Ergo, we need:
reward < rewardsDuration * 2^256 / (10^18 * rewardsDuration) = 2^256 / 10^18
So the problem will not emerge whenever we require
reward < uint(-1) / UNIT
setRewardsDuration was intended to allow the
rewardsDuration to be set after the duration had completed. However a flaw in the require meant it could be changed after the initial setting.
require(periodFinish == 0 || block.timestamp > periodFinish);
require(block.timestamp > periodFinish);
- Inherit the
Pausable.solcontract and add modifier
- Revert the
notifyRewardAmounttransaction if the computer reward rate would pay out more than the balance of the contract over the reward period.
- Change the
setRewardsDurationto only check the period has finished
- should revert when stake is called when paused is true
- should allow a call to stake to succeed when paused is false
- Overflow bugfix
- should revert
notifyRewardAmountif reward is greater than the contract balance
- should revert
notifyRewardAmountif reward plus any leftover from the previous period is greater than the contract balance
- should not revert
notifyRewardAmountif reward is equal to the contract balance
- should revert
- setRewardsDuration bug fix
- should revert when setting setRewardsDuration before the period has finished
- should update rewardsDuration when calling setRewardsDuration after the rewards period has finished
Please list all values configurable via SCCP under this implementation.
Copyright and related rights waived via CC0.