Prorated rewards v2 #631
Prorated rewards v2 #631
Conversation
cam-schultz
requested review from
iansuvak,
geoff-vball,
bernard-avalabs and
michaelkaplan13
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| function claimValidationRewards( | ||
| bytes32 validationID, | ||
| uint32 messageIndex | ||
| ) external nonReentrant { | ||
| PoSValidatorManagerStorage storage $ = _getPoSValidatorManagerStorage(); | ||
|
|
||
| Validator memory validator = getValidator(validationID); | ||
| if (validator.status != ValidatorStatus.Active) { | ||
| revert InvalidValidatorStatus(validator.status); | ||
| } | ||
|
|
||
| // Non-PoS validators are required to boostrap the network, but are not eligible for rewards. | ||
| if (!_isPoSValidator(validationID)) { | ||
| revert ValidatorNotPoS(validationID); | ||
| } | ||
|
|
||
| // Rewards can only be claimed by the validator owner. | ||
| if ($._posValidatorInfo[validationID].owner != _msgSender()) { | ||
| revert UnauthorizedOwner(_msgSender()); | ||
| } | ||
|
|
||
| // Check that minimum stake duration has passed. | ||
| uint64 claimTime = uint64(block.timestamp); | ||
| if (claimTime < validator.startedAt + $._posValidatorInfo[validationID].minStakeDuration) { | ||
| revert MinStakeDurationNotPassed(claimTime); | ||
| } | ||
|
|
||
| uint64 uptime = _updateUptime(validationID, messageIndex); | ||
| if (!_validateSufficientUptime(uptime, validator.startedAt, claimTime)) { | ||
| revert ValidatorIneligibleForRewards(validationID); | ||
| } | ||
|
|
||
| // Calculate the reward for the entire staking period | ||
| uint256 totalReward = $._rewardCalculator.calculateReward({ | ||
| stakeAmount: weightToValue(validator.startingWeight), | ||
| validatorStartTime: validator.startedAt, | ||
| stakingStartTime: validator.startedAt, | ||
| stakingEndTime: claimTime, | ||
| uptimeSeconds: uptime | ||
| }); | ||
|
|
||
| // Subtract the rewards that have already been claimed. | ||
| uint256 reward = totalReward - $._posValidatorInfo[validationID].claimedRewards; | ||
| $._posValidatorInfo[validationID].claimedRewards = totalReward; | ||
|
|
||
| _reward($._posValidatorInfo[validationID].owner, reward); | ||
|
|
||
| emit ValidationRewardsClaimed(validationID, reward); | ||
| } |
Check notice
Code scanning / Slither
Block timestamp Low
| function _validateSufficientUptime( | ||
| uint64 uptimeSeconds, | ||
| uint64 periodStartTime, | ||
| uint64 periodEndTime | ||
| ) internal pure returns (bool) { | ||
| // Equivalent to uptimeSeconds/(periodEndTime - periodStartTime) < UPTIME_REWARDS_THRESHOLD_PERCENTAGE/100 | ||
| // Rearranged to prevent integer division truncation. | ||
| if ( | ||
| uptimeSeconds * 100 | ||
| < (periodEndTime - periodStartTime) * UPTIME_REWARDS_THRESHOLD_PERCENTAGE | ||
| ) { | ||
| return false; | ||
| } | ||
| return true; | ||
| } |
Check notice
Code scanning / Slither
Block timestamp Low
|
Without a maximum staking period, we may run into an undesirable situation where a validator can accumulate a large amount of uptime, allowing the operator to turn off the validator for an extended period of time while still earning rewards. This is similar to allowing unused vacation days to be rolled over to the next year, which may be undesirable (to the company) if it can be done indefinitely. Updating the effective validation start time after each reward claim such that it is at most X number of days in the past would cap the amount of saved uptime. Update: Geoff explained during standup how getting uptime from an "effective" start time may not be currently possible. Perhaps an alternative solution is to introduce an uptime tax on reward redemption, where the tax would cap the maximum "rollover" uptime. For example, if we want to cap the rollover to approximately 10 days, then a validator that redeems their reward after validating for 100 days would be assigned a 10 day uptime tax (since 20% uptime is 20 days and a 10 "rollover" target would require a 10 day tax). Assuming that they had perfect uptime previously and turn off their validator immediately after redemption, they can only be offline for 12.5 days (instead of 25 days) before they are below the 80% uptime requirement. The amount taxed is independent of the actual uptime during the staking period. In the previous example, a 10 day uptime tax would be applied even if the actual staking uptime was only 80 days during the first 100 days. This is just something to consider and may not be worthwhile if it introduces too much complexity. |
|
@bernard-avalabs I do think the problem of "accumulating too many vacation days" is a real issue, but I don't think tying any kind of penalty to reward redemption is the solution. In your example, if the validator has 100% uptime for 100 days, but doesn't claim any rewards early, they can still go offline for the full 25 days. I think the only solution we can currently implement at the contract level would be to set a maximum validation time. There's a fundamental issue with how validators track uptime for other validators (which may be worth the tradeoff in terms of efficiency etc) that stops us from restricting the amount of "vacation time" a validator can accrue, assuming that the rewards are based on an 80% uptime cliff. |
| // Equivalent to uptimeSeconds/(validator.endedAt - validator.startedAt) < UPTIME_REWARDS_THRESHOLD_PERCENTAGE/100 | ||
| // Rearranged to prevent integer division truncation. | ||
| if ( | ||
| uptimeSeconds * 100 | ||
| < (stakingEndTime - validatorStartTime) * UPTIME_REWARDS_THRESHOLD_PERCENTAGE | ||
| ) { | ||
| return 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
I think with the current implementation, it would make more sense to keep all the uptime checks in the calculator. If the calculator returns 0, we don't release any rewards which is totally fine.
| * @notice Claim pro-rated validation rewards. Rewards are calculated from the last time rewards were claimed, | ||
| * or the beginning of the validation period, whichever is later. Reward eligibility is determined by the | ||
| * submitted uptime proof. | ||
| * |
| if ( | ||
| uptimeSeconds * 100 | ||
| < (periodEndTime - periodStartTime) * UPTIME_REWARDS_THRESHOLD_PERCENTAGE | ||
| ) { | ||
| return false; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
| if ( | |
| uptimeSeconds * 100 | |
| < (periodEndTime - periodStartTime) * UPTIME_REWARDS_THRESHOLD_PERCENTAGE | |
| ) { | |
| return false; | |
| } | |
| return true; | |
| return uptimeSeconds * 100 >= (periodEndTime - periodStartTime) * UPTIME_REWARDS_THRESHOLD_PERCENTAGE; |
| * Rewards are calculated from the last time this function was called, or the beginning of the | ||
| * validation, whichever is later. |
There was a problem hiding this comment.
I don't think this part is true for this implementation.
That's a good point. We can get around that by performing automatic redemption after a max period. So in my previous example, if the automatic redemption period is 100 days, then in the event that the operator turns off their validator immediately after day 100 (assuming perfect uptime) and tries to redeem at day 125, then they would receive rewards for the first 100 days since an automatic redemption was performed at day 100. However, after the uptime tax from the automatic redemption, they did not meet the uptime requirement for a second redemption. Alternatively, if they tried to redeem at 112.5 days, they would get the reward for the entire duration. I understand this gets a bit complicated even to explain. But a max staking period would not give us the option to provide continuous staking to validator operators. |
Why this should be merged
Alternative to #603 that implements the approach suggested by @geoff-vball .
Rather than normalizing submitted uptimes to determine reward eligibility for a given claim period, we instead check that the submitted uptime is sufficient for the entire validation period to date. We then calculate the total reward for the validation period to date, and subtract any previously claimed rewards.
How this works
This works because the uptime threshold calculation is linear: if period A (beginning from the start of the validation) has sufficient uptime relative to the start of the valdiation, and period B has sufficient uptime relative to the start of the validation, then period B also has sufficient uptime relative to the end of period A.
How this was tested
CI
How is this documented
TODO