fix: release voter epoch rewards CELO to LockedGold#11699
fix: release voter epoch rewards CELO to LockedGold#11699pahor167 wants to merge 7 commits intorelease/core-contracts/16from
Conversation
Voter epoch rewards were distributed as virtual vote weight increases in the Election contract but no actual CELO was released from CeloUnreleasedTreasury to LockedGold to back them. This creates an unbacked liability (~1.84M CELO to date) that will eventually cause LockedGold.withdraw() to revert with "Inconsistent balance". Add celoUnreleasedTreasury.release() to LockedGold for totalRewardsVoter in _finishEpochHelper(), matching the existing releases to Governance and Carbon Fund. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2be06ced3e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The deployed version is 1.1.0.2, so the version check expects patch +1. A minor bump was incorrect for a bug fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3bc3b2814
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Track the actual per-group voter rewards distributed via Election.distributeEpochRewards in a new EpochProcessState accumulator (totalDistributedVoterRewards) and release that exact total in _finishEpochHelper instead of the target totalRewardsVoter bucket. The previous release amount could exceed what voters actually received (due to per-group score, slashing multiplier, and integer rounding in Election.getGroupEpochRewardsBasedOnScore), stranding excess CELO in LockedGold without matching vote units. Tests: - Extend test_TransfersToCommunityAndCarbonOffsetting in both the finishNextEpochProcess and processGroup suites to assert LockedGold receives the distributed amount (groupEpochRewards), not the target. - Add test_ReleasesOnlyDistributedVoterRewards_WhenSlashed regression for score/slashing-reduced groups in both suites. - Add test_ReleasesNothingToLockedGold_WhenAllGroupsIneligible. - Drop the duplicate test_TransfersVoterRewardsToLockedGold and test_TransfersAllEpochRewards.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f99d13ca5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…age layout Adding a field to EpochProcessState would have shifted the storage slots of every state variable declared after epochProcessing (epochs, validatorPendingPayments, electedSigners, toProcessGroups), corrupting state on an in-place proxy upgrade. Move the accumulator to a new top-level state variable declared after all pre-existing storage. This is append-only, so the slot layout of existing variables is unchanged. Bump patch version to 1.1.0.4.
| // `Election.getGroupEpochRewardsBasedOnScore`. | ||
| // NOTE: declared after all pre-existing state variables to preserve the | ||
| // proxy storage layout on in-place upgrades. | ||
| uint256 public totalDistributedVoterRewards; |
There was a problem hiding this comment.
Detail but could be helpful: totalDistributedVoterRewards could be added as a view to IEpochManager.sol
| // NOTE: declared after all pre-existing state variables to preserve the | ||
| // proxy storage layout on in-place upgrades. |
There was a problem hiding this comment.
This part of comment unnecessary.
| // 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. |
There was a problem hiding this comment.
Not sure if comment is necessary. Do we need an in-code history of which unit tests were for regressions?
3 participants
No description provided.