Track uptimes for delegation period #520
Conversation
cam-schultz
requested review from
iansuvak,
feuGeneA,
geoff-vball,
minghinmatthewlam,
bernard-avalabs and
michaelkaplan13
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
I think we should also add an external updateUptime function that can be called by anyone to update a validator's uptime.
I think we should also split the initializeEndDelegation and initializeEndValidation into two functions, one that expects the user to be reward eligible and one that doesn't. That way if a used calls the function expecting to be reward eligible, we can revert and not cause irreversible losses of rewards.
| $._validatorUptimes[validationID] = uptime; | ||
| emit ValidationUptimeUpdated(validationID, uptime); |
There was a problem hiding this comment.
We should only update the validator's uptime if the uptime provided is greater than the previously provided uptime. This way delegators may not have to provide an uptime proof to get their rewards, provided the validator has had a sufficiently high uptime.
| uint64 uptime; | ||
| if (includeUptimeProof) { | ||
| uptime = _updateUptime(validationID, messageIndex); |
There was a problem hiding this comment.
We should read from state instead of using a return from _updateUptime. That way if the validator's uptime has been updated recently, anyone ending their delegation won't have to submit another uptime proof, provided that the latest reported uptime up until now hits the 80% threshold.
Discussed both of these points in a call, and agreed that they would be optimizations that would potentially trade gas efficiency for usability. Requiring an uptime proof to be submitted when intializing validation/delegation end in order to be eligible for rewards is simple to reason about, and cheap assuming that fetching and verifying an uptime proof is cheap. On the other hand, decoupling uptime proof delivery from delegation/validation end could save gas in some circumstances, but would require the user to first check if they're eligible for rewards under the latest known proof, and would also require that proof be stored in the contract state. |
| "PoSValidatorManager: invalid origin sender address" | ||
| ); | ||
|
|
||
| (bytes32 uptimeValidationID, uint64 uptime) = |
There was a problem hiding this comment.
Just for my own understanding, does uptime return the absolute amount of time that the validator was up, or a percent of time that it was up during the staking period? I'm just thinking of whether other information is necessary to calculate rewards.
There was a problem hiding this comment.
It returns the absolute number of seconds the validator has been up
Why this should be merged
Modifies
initializeEndDelegationto accept an uptime proof from the P-Chain to be used when calculating rewards incompleteEndDelegation. If no uptime proof is provided, the uptime is assumed to be zero.This PR does not implement delegation rewards calculation or unlocks. That should be done in the same fashion as #489, or added as part of that work.
How this works
See above
How this was tested
CI. Skipped unit tests for now, since we are not doing anything interesting with the uptime proofs yet.
How is this documented
N/A