[Governance 4/4] Runtime wiring#2808
Conversation
🛡️ AI Review — Skeptic (security review)VERDICT: VULNERABLE BASELINE scrutiny: established contributor with write permission and substantive prior merged PRs; no Gittensor allowlist match found; branch governance-wiring -> governance-referenda. Reviewed the prefetched diff as authoritative. No Findings
ConclusionThe PR is vulnerable because the new root-registration bookkeeping can grant governance eligibility without an actual root registration in a replacement edge case. 🔍 AI Review — Auditor (domain review)VERDICT: 👎 l0r1s is an established repository contributor with write access; no curated/on-chain Gittensor match found in the trusted allowlists. Description discrepancy: the PR body still describes this as a placeholder with no runtime or pallet changes and no testing. The current diff now wires the governance runtime, adds root-registration EMA/indexing, introduces a migration, updates weights/benchmarks, and adds Rust plus TypeScript governance coverage. Duplicate-work check: the overlap data includes the prerequisite governance-stack PRs (#2805-#2807) and common-file overlaps, but none appears to be a better duplicate of this runtime-wiring PR. Findings
Other findings
Prior-comment reconciliation
ConclusionBlocking because the runtime currently treats partially populated governance collectives as live voter sets, which can reduce the documented Triumvirate/review quorums before root dispatch. Rerun after the minimum-size invariant is enforced and the stale PR body is updated. 📜 Previous run (superseded)
|
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
l0r1s
force-pushed
the
governance-referenda
branch
from
6b00274 to
f11a3bb
Compare
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
eco-tests changed — indexer review requiredThis PR modifies files under Changed files
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| id: CollectiveId::Triumvirate, | ||
| info: CollectiveInfo { | ||
| name: pad_name(b"triumvirate"), | ||
| min_members: 3, |
There was a problem hiding this comment.
[HIGH] Partially populated collectives are treated as live governance quorums
These min_members values do not protect the referendum path. pallet_multi_collective::add_member permits building a collective below min_members, while pallet_referenda only checks that the voter set is non-empty. That means a single Triumvirate member can satisfy a 2/3 approval threshold as 1-of-1, and an undersized Economic/Building review set can fast-track root dispatch with far fewer than the documented seats. Enforce the configured minimum size before a collective is considered usable for MemberSet/referenda, or require atomic set_members initialization and reject partially initialized voter sets. Add tests for one-member Triumvirate and one-sided/undersized Economic/Building review snapshots.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
| let replaced_owner = Owner::<T>::get(&replaced_hotkey); | ||
| Self::decrement_root_registered_hotkey_count(&replaced_owner); | ||
| Self::increment_root_registered_hotkey_count(&coldkey); |
There was a problem hiding this comment.
[HIGH] Root eligibility updates even when replacement is skipped
replace_neuron can return without replacing when the selected UID is the protected subnet-owner hotkey, but these new lines still decrement the previous owner and increment the caller. In that case root_register succeeds without adding the caller's hotkey to Keys[ROOT], while RootRegisteredHotkeyCount and EconomicEligible are updated as if it had. A caller with higher stake than the protected lowest root UID can therefore enter the Economic eligibility pool without actually being root-registered, and the real root owner can be removed from eligibility. Make replace_neuron report whether it actually replaced, or re-read Keys[ROOT, lowest_uid] after the call and only update the counters when the stored hotkey changed; otherwise return an error.
|
🔄 AI review updated — Skeptic: VULNERABLE |
1 participant
Summary
Final part of #2804.
Wires the generic governance pallets into the Subtensor runtime and defines Subtensor's concrete on-chain governance model.
Root calls now go through a two-stage process: Proposers submit to the Triumvirate track, and approved calls move into a review track where the Economic and Building collectives can fast-track, delay, or cancel enactment.
What Changed
pallet-multi-collective,pallet-signed-voting, andpallet-referendainto the runtime.runtime/src/governancewith track definitions, collective definitions, voter-set handling, term rotation, EMA selection, benchmarks, and weights.Proposers,Triumvirate,Economic,Building, andEconomicEligible.0,triumvirate: 7-day approve/reject decision by the Triumvirate.1,review: Economic + Building review with fast-track, delay, and cancel behavior.Detailed Reference
See
docs/governance/README.mdfor the end-to-end governance model andruntime/src/governance/README.mdfor the concrete runtime configuration. Together they cover the two-track flow, collective roles, Economic and Building selection, review delay formula, referendum lifecycle, storage/audit trail, runtime constants, and operational notes.Reviewers may want to focus on the runtime invariants: track
1is not directly submittable, review voters are the deduplicated union ofEconomicandBuilding, active polls use snapshot-based eligibility, and rotating collectives leave the previous member set in place if a full 16-member selection cannot be produced.Behavioral Impact
This is the PR that activates the governance stack in the runtime.
The review track is not directly submittable. It can only be reached after Triumvirate approval, which prevents bypassing the first governance gate. Voting uses snapshot-based voter sets, so active referenda are not affected by later collective rotations.
Migration / Spec Version
Includes a migration to initialize root-registered hotkey counts from existing state. This PR adds runtime storage and should be merged with the appropriate runtime release/spec-version handling for the target base branch.
Testing
Coverage includes Rust unit/integration tests for the new Subtensor root-registration logic and TypeScript governance E2E suites under: