Add voter reward commission for validator groups#11694
Add voter reward commission for validator groups#11694pahor167 wants to merge 12 commits intorelease/core-contracts/16from
Conversation
Allows validator groups to take a percentage of voter CELO epoch rewards. Adds voterRewardCommission fields to ValidatorGroup struct with time-delayed update pattern, commission deduction in EpochManager before distributing voter rewards, and comprehensive unit tests covering all edge cases. Changes: - Validators.sol: new struct fields, setNextVoterRewardCommissionUpdate(), updateVoterRewardCommission(), getVoterRewardCommission(), setMaxVoterRewardCommission(), version bump to (1,5,0,0) - EpochManager.sol: _deductVoterRewardCommission() helper, commission deduction in processGroup() and finishNextEpochProcess(), version bump to (1,1,1,0) - IValidators.sol: 5 new interface signatures - MockValidators.sol: mock support for voter reward commission - Validators.t.sol: 33 new tests across 4 test contracts - EpochManager.t.sol: 7 new tests for commission distribution
|
@codex review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Free Tier Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Bugbot Autofix is kicking off a free cloud agent to fix these issues. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.
![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: 608edba92c
ℹ️ 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".
Validators: (1,4,1,0) not (1,5,0,0) — additive changes = minor bump EpochManager: (1,1,0,3) not (1,1,1,0) — internal logic change = patch bump
review'd by Claude🟡 Suggestion (3)
🔵 Nitpick (1)
🟢 Good (2)
Bottom line: 1. Bump EpochManager version — currently unchanged from base, should be at least (1,1,0,4). 2. Consider re-checking maxVoterRewardCommission at activation — the commission has direct treasury cost unlike the existing commission, so bypassing the cap is more consequential. 3. Document the economic model — the treasury releases additional CELO beyond the EpochRewards target calculations; this should be explicitly documented for future maintainers. Automated review by reviewd. Findings are AI-generated — use your judgment. |
- Remove unnecessary _sendValidatorGroupPaymentsIfNecessary call from updateVoterRewardCommission (voterRewardCommission is independent from cUSD validator payment commission) - Re-check maxVoterRewardCommission at activation time to prevent cap bypass when governance lowers the cap between queue and activation - Bump EpochManager version to (1,1,0,4) for new behavior - Add NatSpec documenting treasury economic trade-off on _deductVoterRewardCommission - Add tests for max cap re-check at activation
|
Addressed in d2563ca:
|
…ission Add fuzz tests verifying conservation invariant (commission + voterRewards == totalRewards) across random commission rates and reward amounts, plus tests proving that commission changes during epoch processing use the rate active at processGroup() time rather than epoch start. Also adds Validators fuzz tests for queue/activate lifecycle, max cap enforcement, and cap-lowered-after-queue scenarios.
…tribution time Previously, groups that activated a commission before governance lowered maxVoterRewardCommission would keep collecting at the old (higher) rate. Now _deductVoterRewardCommission clamps the effective rate to the cap, ensuring governance changes take effect immediately for all groups.
…nomics The previous comment incorrectly stated commission CELO was 'NOT accounted for' in the reward budget. Commission is carved from the already-budgeted totalRewardsVoter pool — it redirects part of the voter reward from deferred LockedGold claims to immediate treasury releases, not additional emission.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98720090ff
ℹ️ 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".
Remove misleading dev note suggesting epoch-processing blocking is needed (the 3-day commissionUpdateDelay makes this moot), and remove false claim that voter reward commission is 'more consequential' than regular commission (both ultimately cost the protocol).
…ardCommission Governance should decide the initial cap, not the contract NatSpec.
Fuzz tests used 1e30 as upper bound for reward amounts, but the mock treasury only holds ~3.07e26 CELO (L2_INITIAL_STASH_BALANCE). Large fuzzed rewards caused commission releases to revert with 'Insufficient balance'.
pahor167
changed the title
…for validator allocation startNextEpochProcess() calls allocateValidatorsRewards() which releases CELO from treasury before processGroup(). Using full L2_INITIAL_STASH_BALANCE as the reward bound means 100% commission can exceed what remains.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f75e95d1ee
ℹ️ 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".
| uint256 epochRewards | ||
| ) internal returns (uint256 commissionAmount) { | ||
| IValidators validators = getValidators(); | ||
| (uint256 voterRewardCommissionUnwrapped, , ) = validators.getVoterRewardCommission(group); |
There was a problem hiding this comment.
processGroup/finishNextEpochProcess now always call validators.getVoterRewardCommission(group), but Validators.getVoterRewardCommission reverts unless isValidatorGroup(account) is true. A group can still appear in processedGroups from last-epoch membership and then be deregistered before reward processing (for configs where the empty-group duration allows it), which makes this lookup revert and blocks epoch finalization. Before this change, reward processing for such a group would not hard-fail on validator-group existence, so this introduces a liveness risk tied to deregistration timing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72373833b8
ℹ️ 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".
| uint256 epochRewards | ||
| ) internal returns (uint256 commissionAmount) { | ||
| IValidators validators = getValidators(); | ||
| (uint256 voterRewardCommissionUnwrapped, , ) = validators.getVoterRewardCommission(group); |
There was a problem hiding this comment.
_deductVoterRewardCommission unconditionally calls validators.getVoterRewardCommission(group), but that getter reverts unless the address is currently a validator group (Validators.sol enforces isValidatorGroup). Here group is taken from last-epoch membership, so if a group was elected last epoch and deregisters before rewards are processed (e.g., when empty-group duration is configured low enough), processGroup/finishNextEpochProcess will revert and epoch finalization can be blocked.
Useful? React with 👍 / 👎.
1 participant
Summary
Introduces a voter reward commission mechanism that allows validator groups to take a configurable percentage of voter CELO epoch rewards. This is separate from the existing commission field which governs the group's cut of validator cUSD payments.
Example: If a group sets 10% voter reward commission and voters for that group earn 1,000 CELO in epoch rewards, the group receives 100 CELO (released from CeloUnreleasedTreasury) and voters receive the remaining 900 CELO as vote credit inflation.
Changes
EpochManager.sol
_deductVoterRewardCommission()— deducts commission from voter rewards and releases CELO from treasury to the groupmaxVoterRewardCommissionat distribution time, so governance cap changes take effect immediately for all groups (even those with previously-activated higher rates)processGroup()andfinishNextEpochProcess()pathsVoterRewardCommissionDistributedeventValidators.sol
voterRewardCommission,nextVoterRewardCommission,nextVoterRewardCommissionBlockfields onValidatorGroupstructsetNextVoterRewardCommissionUpdate()/updateVoterRewardCommission()— time-delayed queue/activate pattern matching existingcommissionflow, reusingcommissionUpdateDelaysetMaxVoterRewardCommission()— governance-controlled cap (0 = no cap)getVoterRewardCommission()— returns current, pending, and activation blockEconomics
totalRewardsVoterpool — it redirects part of the voter reward from deferred LockedGold claims (vote credit inflation) to immediate treasury releasesTests
commission + voterRewards == totalRewards) across random commission rates and reward amounts, math verification against FixidityLib calculationprocessGroup()time, conservation holds when commission changes mid-processingStorage Safety
ValidatorGroupstruct fields appended at end (safe for mapping-based storage)maxVoterRewardCommissionstate variable appended afterdeprecated_downtimeGracePeriod(safe for proxy upgrade)Deployment Note
After upgrade,
maxVoterRewardCommissiondefaults to 0 (no cap). The governance proposal should atomically callsetMaxVoterRewardCommission()to set the desired cap.