Update reporter delegated amount during slashing#11572
Update reporter delegated amount during slashing#11572pahor167 wants to merge 1 commit intorelease/core-contracts/16from
Conversation
… when slashed and receiving rewards. Add unit tests to verify correct behavior for both reporter and account delegation scenarios.
| } | ||
|
|
||
| _updateDelegatedAmount(account); | ||
| _updateDelegatedAmount(reporter); |
There was a problem hiding this comment.
When slashing through GovernanceSlasher the reporter is 0x0 so I would add this exclusion:
| _updateDelegatedAmount(reporter); | |
| if (reporter != address(0)) { | |
| _updateDelegatedAmount(reporter); | |
| } |
and appropriate test case for it in LockedGold.t.sol
There was a problem hiding this comment.
Additionally 👉 how about the found issue within unlock function?
function unlock(uint256 value) external nonReentrant {
...
_decrementNonvotingAccountBalance(msg.sender, value);
_updateDelegatedAmount(msg.sender); // fix for storage
...
}
There was a problem hiding this comment.
When slashing through
GovernanceSlasherthe reporter is0x0so I would add this exclusion
Yes, this is probably even necessary, otherwise the call to voteSignerToAccount inside _updateDelegatedAmount would revert, making Governance slashings impossible.
| } | ||
|
|
||
| _updateDelegatedAmount(account); | ||
| _updateDelegatedAmount(reporter); |
There was a problem hiding this comment.
Just noting: if I remember correctly, the point of having an updateDelegatedAmount function was that we'd allow the stored delegated amount to diverge from the expected amount (in particular, when the delegator accrues epoch rewards). So this change isn't strictly necessary, more of a convenience that adds gas costs to the slash function.
In fact, even the above call with _updateDelegatedAmount(account) isn't really necessary - since the only slashable actors are validators and validator groups, and those are not allowed to delegate.
| } | ||
|
|
||
| _updateDelegatedAmount(account); | ||
| _updateDelegatedAmount(reporter); |
There was a problem hiding this comment.
When slashing through
GovernanceSlasherthe reporter is0x0so I would add this exclusion
Yes, this is probably even necessary, otherwise the call to voteSignerToAccount inside _updateDelegatedAmount would revert, making Governance slashings impossible.
Description
Enhance LockedGold contract to update delegated amounts for reporter when slashed and receiving rewards. Add unit tests to verify correct behavior for both reporter and account delegation scenarios.
Tested
Unit tests
Note
Update
LockedGold.slashto recalculate reporter’s delegated amount when reward is granted and add unit tests covering reporter/account delegation updates.contracts/governance/LockedGold.sol:slash, after balance adjustments, also call_updateDelegatedAmount(reporter)to refresh reporter delegation state.test-sol/unit/governance/voting/LockedGold.t.sol:Written by Cursor Bugbot for commit 62adde1. This will update automatically on new commits. Configure here.