feat: M014 CosmWasm validator-governance contract with unit tests#73
feat: M014 CosmWasm validator-governance contract with unit tests#73brawlaphant wants to merge 1 commit intoregen-network:mainfrom
Conversation
Implements the Authority Validator Governance mechanism (M014) as a CosmWasm smart contract for Regen Network's PoA transition. Covers PoA validator set management (add/remove/approve/activate), composite performance scoring (uptime 0.4, governance 0.3, ecosystem 0.3), validator lifecycle state machine (Candidate -> Active -> Probation -> Removed/TermExpired), weighted governance proposals with voting, and composition guarantees (min validators per category). All arithmetic uses integer basis points (no floating point). 25 unit tests passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the regen-validator-governance CosmWasm contract, which manages PoA validator sets, performance scoring, and governance for the Regen Network. The implementation includes validator lifecycle management (Candidate to Active/Removed), performance-weighted voting, and automated configuration updates via proposals. Feedback focuses on critical scalability and security issues: several functions perform unbounded iterations over state maps which will eventually lead to Out of Gas (OOG) errors, and pagination is missing from query endpoints. Additionally, the governance execution logic for removing validators fails to enforce the minimum validator security invariant, and there is a lack of input validation for configuration updates (e.g., ensuring min_validators <= max_validators) when triggered via proposals.
| fn count_validators_by_status(deps: Deps, status: Option<ValidatorStatus>) -> StdResult<u32> { | ||
| let count = VALIDATORS | ||
| .range(deps.storage, None, None, Order::Ascending) | ||
| .filter_map(|item| item.ok()) | ||
| .filter(|(_, v)| status.as_ref().map_or(true, |s| v.status == *s)) | ||
| .count() as u32; | ||
| Ok(count) | ||
| } |
There was a problem hiding this comment.
The count_validators_by_status helper iterates over the entire VALIDATORS map. Since this map stores all historical validator records (including Removed and TermExpired), this operation becomes increasingly expensive over time. This can lead to Out of Gas (OOG) errors in critical execution paths like execute_apply_validator and execute_activate_validator. It is recommended to maintain status-based counters in Item storage for O(1) access.
| let cat_count = | ||
| count_active_validators_by_category(deps.as_ref(), &v.category)?; | ||
| if cat_count <= config.min_per_category { | ||
| proposal.status = ProposalStatus::Rejected; | ||
| PROPOSALS.save(deps.storage, proposal_id, &proposal)?; | ||
| return Ok(Response::new() | ||
| .add_attribute("action", "execute_proposal") | ||
| .add_attribute("proposal_id", proposal_id.to_string()) | ||
| .add_attribute("result", "rejected_composition_violation")); | ||
| } | ||
| } |
There was a problem hiding this comment.
The execution logic for RemoveValidator proposals enforces the category composition invariant but fails to check the min_validators security invariant (defined in SPEC section 11). If a proposal to remove a validator is executed when the active set is already at the minimum size, it will drop the network below the required security threshold.
References
- SPEC Section 11: Security Invariants - Graceful Degradation: If active set drops below min_validators, trigger emergency governance escalation. (link)
- Security invariants such as minimum validator count must be enforced on-chain during all state transitions, including those triggered by governance execution.
| .range(deps.storage, None, None, Order::Ascending) | ||
| .filter_map(|item| item.ok()) | ||
| .map(|(_, v)| v) | ||
| .filter(|v| { | ||
| let status_match = status_filter.as_ref().map_or(true, |s| v.status == *s); | ||
| let category_match = category_filter.as_ref().map_or(true, |c| v.category == *c); | ||
| status_match && category_match | ||
| }) | ||
| .collect(); |
There was a problem hiding this comment.
Range queries on VALIDATORS, PROPOSALS, and VOTES (lines 818, 874, 894, 905) lack pagination. As the number of historical records grows, these queries will eventually exceed the block gas limit, leading to a denial of service for both external queries and internal logic. CosmWasm best practices require using limit and start_after parameters for all map iterations.
| let total = count_validators_by_status(deps.as_ref(), None)?; | ||
| if total >= config.max_validators { | ||
| return Err(ContractError::ValidatorSetFull { | ||
| current: total, | ||
| max: config.max_validators, | ||
| }); | ||
| } |
There was a problem hiding this comment.
execute_apply_validator checks the total number of entries in the VALIDATORS map against max_validators. Because Removed and TermExpired validators are never deleted from the map, this logic will eventually prevent any new applications once the historical total reaches the limit, even if the active set is empty. The check should likely only consider Candidate, Approved, Active, and Probation statuses.
| if let Some(v) = voting_period_seconds { | ||
| c.voting_period_seconds = *v; | ||
| } | ||
| CONFIG.save(deps.storage, &c)?; |
There was a problem hiding this comment.
The UpdateConfig proposal execution does not validate that voting_period_seconds is greater than zero, unlike the direct execute_update_config handler (line 776). Setting this to zero via governance would break the voting mechanism for all future proposals. Additionally, neither path validates that min_validators <= max_validators.
1 participant
Summary
contracts/validator-governance/Contract structure
Cargo.tomlsrc/lib.rssrc/error.rssrc/state.rssrc/msg.rssrc/contract.rsSpec coverage
compute_score_and_confidence()— weighted sum with re-normalization for missing factorscompute_flags()— below_performance_threshold, below_uptime_minimum, probation_recommendedTest plan
cargo test)🤖 Generated with Claude Code