Penalty for reward as alternative to slashing #254
Conversation
vzotova
changed the title
vzotova
requested review from
arjunhassard,
cygnusv,
theref,
derekpierre and
manumonti
| * @notice Resets future reward back to 100% | ||
| * @param _stakingProvider Staking provider address | ||
| */ | ||
| function resetReward(address _stakingProvider) external { |
There was a problem hiding this comment.
Can you clarify what happens in resetReward is not called for a penalized staking provider whose penalty ended?
There was a problem hiding this comment.
resetReward also included in updareReward, so if it none such methods will be called then reward calculation will be looking like staker still has penalty. I know it's not great but with our model - we need active input to update values
There was a problem hiding this comment.
In that case, this needs to be stated in the @notice.
| return effectiveAuthorized(_authorized, _info.penaltyPercent); | ||
| } | ||
|
|
||
| /// @dev This view should be called after updateReward modifier |
There was a problem hiding this comment.
What do you mean with these @dev comments?
There was a problem hiding this comment.
updateReward modifier has resetReward inside, so this view will give proper value after that check. Otherwise it will show penalty in cases when they may be reseted
There was a problem hiding this comment.
Right, this is related to my previous comment. What about something like:
/// @dev In case that a penalty period already ended, this view method may produce
/// outdated results if the penalty hasn't been reset, either by calling
/// `resetReward` explicitly or any function with the `updateReward` modifier.
| if (_info.endPenalty == 0) { | ||
| return _from - _to; | ||
| } | ||
| uint96 effectiveFrom = effectiveAuthorized(_from, _info.penaltyPercent); | ||
| uint96 effectiveTo = effectiveAuthorized(_to, _info.penaltyPercent); | ||
| return effectiveFrom - effectiveTo; |
There was a problem hiding this comment.
Can you just say return effectiveAuthorized(_from, _to)? When endPenalty == 0, L492 will make sure the result is _from - _to.
There was a problem hiding this comment.
not sure that I follow you, second value in effectiveAuthorized is penalty percent, can you elaborate?
in general I made this thing to decrease read operations (still costly)
There was a problem hiding this comment.
Sorry, this is what I meant:
| if (_info.endPenalty == 0) { | |
| return _from - _to; | |
| } | |
| uint96 effectiveFrom = effectiveAuthorized(_from, _info.penaltyPercent); | |
| uint96 effectiveTo = effectiveAuthorized(_to, _info.penaltyPercent); | |
| return effectiveFrom - effectiveTo; | |
| return effectiveAuthorized(_from - _to); |
There was a problem hiding this comment.
now I see your question, but I didn't do that on purpose, reason is it could produce different result because of rounding errors, for all other places it's division first and then addition/subtraction.
Co-authored-by: Manuel Montenegro <[email protected]> Co-authored-by: Derek Pierre <[email protected]> Co-authored-by: David Núñez <[email protected]>
Type of PR:
Required reviews:
What this does:
Penalize reward as an option when slashing is too much. Penalty decreases reward by
penaltyDefaultforpenaltyDuration. Eachpenalizerestarts time, penalty itself is the same value until end of duration. Initially this event starts on Polygon in child app, wherepenalizecan be called by special role (adjudicatorfor now)Issues fixed/closed:
Why it's needed:
Notes for reviewers: