-
Notifications
You must be signed in to change notification settings - Fork 475
fix: release voter epoch rewards CELO to LockedGold #11699
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.
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
base: release/core-contracts/17
Are you sure you want to change the base?
Changes from all commits
ec380e9
2be06ce
f3bc3b2
3f99d13
f5a5cd1
5832af3
a9b7271
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,6 +71,14 @@ contract EpochManager is | |
|
|
||
| uint256 public toProcessGroups = 0; | ||
|
|
||
| // Sum of voter rewards actually distributed to elected groups during the | ||
| // current epoch. May be less than `epochProcessing.totalRewardsVoter` due to | ||
| // per-group score, slashing multipliers, and integer rounding in | ||
| // `Election.getGroupEpochRewardsBasedOnScore`. | ||
| // NOTE: declared after all pre-existing state variables to preserve the | ||
| // proxy storage layout on in-place upgrades. | ||
| uint256 public totalDistributedVoterRewards; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Detail but could be helpful: |
||
|
|
||
| /** | ||
| * @notice Event emitted when epochProcessing has begun. | ||
| * @param epochNumber The epoch number that is being processed. | ||
|
|
@@ -316,6 +324,7 @@ contract EpochManager is | |
|
|
||
| if (epochRewards != type(uint256).max) { | ||
| election.distributeEpochRewards(group, epochRewards, lesser, greater); | ||
| totalDistributedVoterRewards += epochRewards; | ||
| } | ||
|
|
||
| delete processedGroups[group]; | ||
|
|
@@ -377,6 +386,7 @@ contract EpochManager is | |
| require(epochRewards > 0, "group not from current elected set"); | ||
| if (epochRewards != type(uint256).max) { | ||
| election.distributeEpochRewards(groups[i], epochRewards, lessers[i], greaters[i]); | ||
| totalDistributedVoterRewards += epochRewards; | ||
| } | ||
|
|
||
| delete processedGroups[groups[i]]; | ||
|
|
@@ -761,6 +771,13 @@ contract EpochManager is | |
| _setElectedSigners(_newlyElected); | ||
|
|
||
| ICeloUnreleasedTreasury celoUnreleasedTreasury = getCeloUnreleasedTreasury(); | ||
| // Release only the voter rewards that were actually distributed to groups | ||
| // (post score, slashing multiplier, and rounding) to avoid stranding excess | ||
| // CELO in LockedGold without matching vote units. | ||
| celoUnreleasedTreasury.release( | ||
| registry.getAddressForOrDie(LOCKED_GOLD_REGISTRY_ID), | ||
| totalDistributedVoterRewards | ||
| ); | ||
| celoUnreleasedTreasury.release( | ||
| registry.getAddressForOrDie(GOVERNANCE_REGISTRY_ID), | ||
| _epochProcessing.totalRewardsCommunity | ||
|
|
@@ -775,6 +792,7 @@ contract EpochManager is | |
| _epochProcessing.totalRewardsVoter = 0; | ||
| _epochProcessing.totalRewardsCommunity = 0; | ||
| _epochProcessing.totalRewardsCarbonFund = 0; | ||
| totalDistributedVoterRewards = 0; | ||
|
|
||
| emit EpochProcessingEnded(currentEpochNumber - 1); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ contract EpochManagerTest is TestWithUtils08 { | |
| address carbonOffsettingPartner; | ||
| address communityRewardFund; | ||
| address reserveAddress; | ||
| address lockedGoldAddress; | ||
| address scoreManagerAddress; | ||
|
|
||
| uint256 firstEpochNumber = 3; | ||
|
|
@@ -90,6 +91,7 @@ contract EpochManagerTest is TestWithUtils08 { | |
| scoreManagerAddress = actor("scoreManagerAddress"); | ||
|
|
||
| reserveAddress = actor("reserve"); | ||
| lockedGoldAddress = actor("lockedGold"); | ||
|
|
||
| carbonOffsettingPartner = actor("carbonOffsettingPartner"); | ||
| communityRewardFund = actor("communityRewardFund"); | ||
|
|
@@ -108,6 +110,7 @@ contract EpochManagerTest is TestWithUtils08 { | |
| registry.setAddressFor(ScoreManagerContract, address(scoreManager)); | ||
| registry.setAddressFor(StableTokenContract, address(stableToken)); | ||
| registry.setAddressFor(ReserveContract, reserveAddress); | ||
| registry.setAddressFor(LockedGoldContract, lockedGoldAddress); | ||
| registry.setAddressFor(ElectionContract, address(election)); | ||
|
|
||
| celoUnreleasedTreasury.setRegistry(REGISTRY_ADDRESS); | ||
|
|
@@ -576,6 +579,56 @@ contract EpochManagerTest_finishNextEpochProcess is EpochManagerTest { | |
|
|
||
| assertEq(celoToken.balanceOf(communityRewardFund), epochRewards.totalRewardsCommunity()); | ||
| assertEq(celoToken.balanceOf(carbonOffsettingPartner), epochRewards.totalRewardsCarbonFund()); | ||
| // LockedGold receives the sum of voter rewards actually distributed to groups, | ||
| // which for this fixture is `groupEpochRewards` (a single elected group). | ||
| assertEq( | ||
| celoToken.balanceOf(lockedGoldAddress), | ||
| groupEpochRewards, | ||
| "LockedGold should receive distributed voter rewards" | ||
| ); | ||
| } | ||
|
|
||
| function test_ReleasesOnlyDistributedVoterRewards_WhenSlashed() public { | ||
| // Simulate a slashed/score-reduced group where Election distributes less than | ||
| // the target voter bucket. Release must match the distributed amount (not the | ||
| // target), otherwise excess CELO would be stranded in LockedGold. | ||
| uint256 reducedRewards = groupEpochRewards / 4; | ||
| election.setGroupEpochRewardsBasedOnScore(group, reducedRewards); | ||
|
|
||
| ( | ||
| address[] memory groups, | ||
| address[] memory lessers, | ||
| address[] memory greaters | ||
| ) = getGroupsWithLessersAndGreaters(); | ||
|
|
||
| epochManagerContract.startNextEpochProcess(); | ||
| epochManagerContract.finishNextEpochProcess(groups, lessers, greaters); | ||
|
|
||
| assertEq( | ||
| celoToken.balanceOf(lockedGoldAddress), | ||
| reducedRewards, | ||
| "LockedGold should receive only the distributed (reduced) voter rewards" | ||
| ); | ||
| } | ||
|
|
||
| function test_ReleasesNothingToLockedGold_WhenAllGroupsIneligible() public { | ||
| // Group is ineligible / fully slashed -> distributed amount is 0. | ||
| election.setGroupEpochRewardsBasedOnScore(group, 0); | ||
|
|
||
| ( | ||
| address[] memory groups, | ||
| address[] memory lessers, | ||
| address[] memory greaters | ||
| ) = getGroupsWithLessersAndGreaters(); | ||
|
|
||
| epochManagerContract.startNextEpochProcess(); | ||
| epochManagerContract.finishNextEpochProcess(groups, lessers, greaters); | ||
|
|
||
| assertEq( | ||
| celoToken.balanceOf(lockedGoldAddress), | ||
| 0, | ||
| "LockedGold should receive nothing when no voter rewards were distributed" | ||
| ); | ||
| } | ||
|
|
||
| function test_TransfersToValidatorGroup() public { | ||
|
|
@@ -741,6 +794,29 @@ contract EpochManagerTest_processGroup is EpochManagerTest { | |
|
|
||
| assertEq(celoToken.balanceOf(communityRewardFund), epochRewards.totalRewardsCommunity()); | ||
| assertEq(celoToken.balanceOf(carbonOffsettingPartner), epochRewards.totalRewardsCarbonFund()); | ||
| assertEq( | ||
| celoToken.balanceOf(lockedGoldAddress), | ||
| groupEpochRewards, | ||
| "LockedGold should receive distributed voter rewards via processGroup path" | ||
| ); | ||
| } | ||
|
|
||
| function test_ReleasesOnlyDistributedVoterRewards_WhenSlashed() public { | ||
| // Regression: per-group score / slashing multiplier reduces the distributed | ||
| // amount below the target voter bucket. Release must match what was actually | ||
| // distributed via the processGroup path. | ||
|
Comment on lines
+805
to
+807
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if comment is necessary. Do we need an in-code history of which unit tests were for regressions? |
||
| uint256 reducedRewards = groupEpochRewards / 4; | ||
| election.setGroupEpochRewardsBasedOnScore(group, reducedRewards); | ||
|
|
||
| epochManagerContract.startNextEpochProcess(); | ||
| epochManagerContract.setToProcessGroups(); | ||
| epochManagerContract.processGroup(group, address(0), address(0)); | ||
|
|
||
| assertEq( | ||
| celoToken.balanceOf(lockedGoldAddress), | ||
| reducedRewards, | ||
| "LockedGold should receive only the distributed (reduced) voter rewards" | ||
| ); | ||
| } | ||
|
|
||
|
pahor167 marked this conversation as resolved.
|
||
| function test_TransfersToValidatorGroup() public { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part of comment unnecessary.