Submitted by Varun_05, also found by fyamf
Before allowing anyone to stake a service id, the checkpoint function should be called in order to distribute already present rewards in the contract and thus update the availableRewards. If this doesnt happen and a new service id is staked, then after some time the rewards are then shared among the new service id as well; thus, causing the service ids which were staked before the new service id to receive less rewards. Furthermore, as there is no mention who calls checkpoint function and when is it called, the likelihood of this error is high and impact is loss of rewards thus impact is also high.
Following is the stake function:
As we can see clearly, the checkpoint function is not called anywhere in the stake function. From above, we can also see that the staking is not allowed if there are no availableRewards. So in order to get the latest and the correct value of the available rewards, the checkpoint function should be called at the start of stake function and then it should be checked whether availableRewards are zero or not so as to decide whether staking should be allowed or not.
Lets explain this issue with a simple example:
Here, suppose the liveness period has passed and checkpoint can be called but is not called. Suppose available rewards left are 300 and there are 3 current services.
Let liveness period = 10 and let rewards per sec = 10.
As of now, all the 3 services should receive 10 * 10 (I have assumed that the staking time to be exact 10 sec can be any value) = 100 rewards each and thus, the available rewards should become 0.
But as checkpoint is not called before the stake function and new service id is staked. Legitimately, the new service should not be able to stake, as all the available rewards should go to the already present three service ids. However, now as checkpoint is not called before calling the stake function, a new service id is also staked. Now, 5 more seconds have passed and checkpoint is called. Now, the already present three service ids would be eligible for rewards: 10 * 15 = 150.
Also, now suppose the new service id added also passes the liveness ratio; therefore, it also becomes eligible for rewards: 10 * 5 = 50. Thus, the total rewards would now be equal to 150 * 3 + 50 = 500; whereas total available rewards are 300. Therefore, the first three service ids would receive (150 * 300)/500 = 90 rewards, as opposed to the original 100 rewards they should be applicable to. Therefore, there is loss of rewards for the service ids.
In all, the staking protocols all the updation are done before allowing anyone to further stake so as there is no loss of rewards.
Call the checkpoint function at the start of stake function as follows:
Context
0xA5DF (judge) commented:
kupermind (Olas) acknowledged and commented:
Note: For full discussion, see here.
Olas mitigated
Varun_05 (warden) commented:
