Skip to content

Update reporter delegated amount during slashing #11572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pahor167 wants to merge 1 commit into
base: release/core-contracts/16
Choose a base branch
from
+94 −0
Open

Update reporter delegated amount during slashing #11572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/protocol/contracts/governance/LockedGold.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ contract LockedGold is
}

_updateDelegatedAmount(account);
_updateDelegatedAmount(reporter);
Copy link
Copy Markdown
Contributor

@Mc01 Mc01 Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When slashing through GovernanceSlasher the reporter is 0x0 so I would add this exclusion:

Suggested change
_updateDelegatedAmount(reporter);
if (reporter != address(0)) {
_updateDelegatedAmount(reporter);
}

and appropriate test case for it in LockedGold.t.sol

Copy link
Copy Markdown
Contributor

@Mc01 Mc01 Nov 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
  ...
}

Copy link
Copy Markdown
Contributor

@m-chrzan m-chrzan Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When slashing through GovernanceSlasher the reporter is 0x0 so I would add this exclusion

Yes, this is probably even necessary, otherwise the call to voteSignerToAccount inside _updateDelegatedAmount would revert, making Governance slashings impossible.

Copy link
Copy Markdown
Contributor

@m-chrzan m-chrzan Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


address communityFund = registry.getAddressForOrDie(GOVERNANCE_REGISTRY_ID);
address payable communityFundPayable = address(uint160(communityFund));
Expand Down
93 changes: 93 additions & 0 deletions packages/protocol/test-sol/unit/governance/voting/LockedGold.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1404,6 +1404,99 @@ contract LockedGoldTest_slash is LockedGoldTest {
assertEq(lockedGold.getAccountTotalLockedGold(delegator), 0);
assertEq(lockedGold.getAccountTotalGovernanceVotingPower(delegatee1), 0);
}

function test_ShouldUpdateReporterDelegatedAmount_WhenReporterIsDelegatingAndReceivesReward()
public
{
uint256 penalty = value;
uint256 reward = value / 2;
uint256 reporterDelegationPercent = 50;

// Setup: Give reporter funds and lock CELO, then delegate to delegatee2
vm.deal(reporter, 10 ether);
vm.prank(reporter);
lockedGold.lock.value(value)();
vm.prank(reporter);
lockedGold.delegateGovernanceVotes(
delegatee2,
FixidityLib.newFixedFraction(reporterDelegationPercent, 100).unwrap()
);

// Verify initial state
uint256 initialReporterBalance = lockedGold.getAccountNonvotingLockedGold(reporter);
uint256 initialDelegatedAmount = (value * reporterDelegationPercent) / 100;
assertEq(initialReporterBalance, value);
assertEq(lockedGold.totalDelegatedCelo(delegatee2), initialDelegatedAmount);

// Slash the account, reporter receives reward
helper_WhenAccountIsSlashedForAllOfItsLockedGold(penalty, reward, caller);

// Verify reporter's balance increased
uint256 newReporterBalance = lockedGold.getAccountNonvotingLockedGold(reporter);
assertEq(newReporterBalance, value + reward);

// Verify reporter's delegated amount was updated correctly
uint256 expectedDelegatedAmount = ((value + reward) * reporterDelegationPercent) / 100;
(uint256 fraction, uint256 currentAmount) = lockedGold.getDelegatorDelegateeInfo(
reporter,
delegatee2
);
assertEq(FixidityLib.wrap(fraction * 100).fromFixed(), reporterDelegationPercent);
assertEq(currentAmount, expectedDelegatedAmount);
assertEq(lockedGold.totalDelegatedCelo(delegatee2), expectedDelegatedAmount);

// Verify delegatee's voting power was updated
assertEq(lockedGold.getAccountTotalGovernanceVotingPower(delegatee2), expectedDelegatedAmount);
}

function test_ShouldUpdateBothAccountAndReporterDelegatedAmounts_WhenBothAreDelegating() public {
uint256 penalty = value;
uint256 reward = value / 2;
uint256 accountDelegationPercent = 40;
uint256 reporterDelegationPercent = 60;

// Setup: Account locks CELO and delegates to delegatee1
whenVoteSigner_LockedGoldDelegateGovernanceVotes();
vm.prank(delegator);
lockedGold.delegateGovernanceVotes(
delegatee1,
FixidityLib.newFixedFraction(accountDelegationPercent, 100).unwrap()
);

// Setup: Give reporter funds, lock CELO and delegate to delegatee2
vm.deal(reporter, 10 ether);
vm.prank(reporter);
lockedGold.lock.value(value)();
vm.prank(reporter);
lockedGold.delegateGovernanceVotes(
delegatee2,
FixidityLib.newFixedFraction(reporterDelegationPercent, 100).unwrap()
);

// Record initial delegated amounts
uint256 initialAccountDelegated = (value * accountDelegationPercent) / 100;
uint256 initialReporterDelegated = (value * reporterDelegationPercent) / 100;
assertEq(lockedGold.totalDelegatedCelo(delegatee1), initialAccountDelegated);
assertEq(lockedGold.totalDelegatedCelo(delegatee2), initialReporterDelegated);

// Slash the account
helper_WhenAccountIsSlashedForAllOfItsLockedGold(penalty, reward, delegator);

// Verify account (delegator) was slashed and delegation removed
assertEq(lockedGold.getAccountNonvotingLockedGold(delegator), 0);
assertEq(lockedGold.getAccountTotalLockedGold(delegator), 0);
assertEq(lockedGold.totalDelegatedCelo(delegatee1), 0);
assertEq(lockedGold.getAccountTotalGovernanceVotingPower(delegatee1), 0);

// Verify reporter received reward and delegation was updated
assertEq(lockedGold.getAccountNonvotingLockedGold(reporter), value + reward);
uint256 expectedReporterDelegated = ((value + reward) * reporterDelegationPercent) / 100;
assertEq(lockedGold.totalDelegatedCelo(delegatee2), expectedReporterDelegated);
assertEq(
lockedGold.getAccountTotalGovernanceVotingPower(delegatee2),
expectedReporterDelegated
);
}
}

contract LockedGoldTest_delegateGovernanceVotes is LockedGoldTest {
Expand Down
Loading