Synthetix Staking Rewards Issue - Inefficient Reward Distribution
During an audit by Macro's security team, they found a bug in Synthetix Staking Rewards distribution. While the issue is not an exploitable bug, the Ethereum community should be made aware of it.
*Note, this issue was found by the Macro Audits team and is not an exploitable bug. Instead, it is a medium-to-low severity bug that affects the protocol rewards distribution cycle.
Context
Synthetix’s StakingRewards is a battle-tested smart contract that distributes rewards to stakers. Synthetix has been a pioneer in DeFi and has established a well-audited standard for staking rewards that is used by many protocols across the Ethereum ecosystem.
Recently, Macro Audits did an audit of Sommelier’s cellar staking smart contract.
During the audit, one of Macro’s auditors, Abhishek Vispute, found a medium-to-low severity bug that, in certain cases, a portion of rewards can remain unused within the Sommelier's StakingRewards contract.
Macro Audits brought this issue to the attention of both the Sommelier and Synthetix teams.
This brief post summarizes the issue; and while the issue is not an exploitable bug, and therefore not a “must fix” issue, the Ethereum community should be made aware of it, in case your project — or another project you know of — is using the Synthetix StakingRewards contract.
As you may know, many projects either (a) fork the Synthetix StakingRewards contract or (b) implement a similar pattern. Therefore, we hope that sharing this issue will help any relevant projects out there.
Details can be found below. If you’re interested in working with Macro Audits, please contact us by filling out our Audit Request Form.
Summary
If stake() is not called in the same block of notifyRewardAmount(),
depending on delay, a portion of rewards will remain unused inside the contract.
Description:Let’s consider that you have a StakingRewards contract with a reward duration of one month seconds (2592000):
Block N Timestamp = X
You call notifyRewardAmount() with a reward of one month seconds (2592000) only. The intention is for a period of a month, 1 reward token per second should be distributed to stakers.
State :
rewardRate = 1
periodFinish = X +**2592000**
Block M Timestamp = X + Y
Y time has passed, and the first staker stakes some amount:
1.stake()
2. updateReward
rewardPerTokenStored = 0
lastUpdateTime = X + Y
Hence, for this staker, the clock has started from X+Y, and he or she will accumulate rewards from this point.
Please note, though, that the periodFinish is X + rewardsDuration, not X + Y + rewardsDuration. Therefore, the contract will only distribute rewards until X + rewardsDuration, losing Y * rewardRate => Y * 1 inside of the contract, as rewardRate = 1 (if we consider the above example).
Now, if we consider delay(Y) to be 30 minutes, then:
Only 2592000-1800= 2590200 tokens will be distributed, and these 1800 tokens will remain unused in the contract until the next cycle of notifyRewardAmount().
Whenever a protocol starts a reward cycle, it intends to distribute X amount of rewards during that time.
If a certain amount remains unused (like the above example inside the contract), and a new reward cycle is not started, that amount remains dormant inside the contract.
Even if the given protocol decides to start a new reward cycle to cover this unused amount, it is a redundant execution.
As there will likely be a delay between the first stake and the reward cycle initiation, consider resolving this issue.
Recommended Remediation
Consider defining periodFinish in the first stake() that is done after notifyRewardAmount(), when total deposits are zero.
Check out how the Sommelier team tackled this issue with _startProgram():
PeggyJV