[Governance 3/4] Introduce pallet-referenda#2807
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
🛡️ AI Review — Skeptic (security review)VERDICT: VULNERABLE BASELINE scrutiny: l0r1s is an established contributor with write permission; no Gittensor allowlist hit found. Branch is governance-referenda -> governance-signed-voting. No Findings
Prior-comment reconciliation
ConclusionThe prior queue-slot DoS in 📜 Previous run (superseded)
# 🔍 AI Review — Auditor (domain review) has not yet run on this PR. |
|
🔄 AI review updated — Skeptic: VULNERABLE |
l0r1s
force-pushed
the
governance-signed-voting
branch
from
51a033f to
0ebd213
Compare
l0r1s
force-pushed
the
governance-referenda
branch
from
6b00274 to
f11a3bb
Compare
| let index = ReferendumCount::<T>::get(); | ||
| ReferendumCount::<T>::put(index.saturating_add(1)); | ||
| ActiveCount::<T>::put(active.saturating_add(1)); | ||
| ActivePerProposer::<T>::insert(&proposer, active_per_proposer.saturating_add(1)); | ||
|
|
||
| let proposal = match &track_info.decision_strategy { | ||
| DecisionStrategy::PassOrFail { | ||
| decision_period, .. | ||
| } => { | ||
| let when = now.saturating_add(*decision_period); | ||
| Self::set_alarm(index, when)?; | ||
| let bounded_call = T::Preimages::bound(*call)?; | ||
| Proposal::Action(bounded_call) | ||
| } | ||
| DecisionStrategy::Adjustable { initial_delay, .. } => { | ||
| let when = now.saturating_add(*initial_delay); | ||
| Self::schedule_enactment(index, DispatchTime::At(when), call)?; | ||
| Proposal::Review | ||
| } | ||
| }; |
There was a problem hiding this comment.
[HIGH] submit leaks queue slots when setup fails
submit increments ReferendumCount, ActiveCount, and ActivePerProposer before the fallible setup work. If set_alarm, T::Preimages::bound, or schedule_enactment returns an error, the extrinsic aborts without inserting ReferendumStatusFor, so there is no referendum that kill/conclude can use to release the claimed capacity. A proposer can turn scheduler/preimage setup failures into a permanent governance queue DoS. Build the proposal and scheduler/preimage state first, then claim the counters only after no fallible setup remains; also drop the already-bound proposal if alarm setup fails.
| let index = ReferendumCount::<T>::get(); | |
| ReferendumCount::<T>::put(index.saturating_add(1)); | |
| ActiveCount::<T>::put(active.saturating_add(1)); | |
| ActivePerProposer::<T>::insert(&proposer, active_per_proposer.saturating_add(1)); | |
| let proposal = match &track_info.decision_strategy { | |
| DecisionStrategy::PassOrFail { | |
| decision_period, .. | |
| } => { | |
| let when = now.saturating_add(*decision_period); | |
| Self::set_alarm(index, when)?; | |
| let bounded_call = T::Preimages::bound(*call)?; | |
| Proposal::Action(bounded_call) | |
| } | |
| DecisionStrategy::Adjustable { initial_delay, .. } => { | |
| let when = now.saturating_add(*initial_delay); | |
| Self::schedule_enactment(index, DispatchTime::At(when), call)?; | |
| Proposal::Review | |
| } | |
| }; | |
| let index = ReferendumCount::<T>::get(); | |
| let proposal = match &track_info.decision_strategy { | |
| DecisionStrategy::PassOrFail { | |
| decision_period, .. | |
| } => { | |
| let bounded_call = T::Preimages::bound(*call)?; | |
| let when = now.saturating_add(*decision_period); | |
| if let Err(err) = Self::set_alarm(index, when) { | |
| T::Preimages::drop(&bounded_call); | |
| return Err(err); | |
| } | |
| Proposal::Action(bounded_call) | |
| } | |
| DecisionStrategy::Adjustable { initial_delay, .. } => { | |
| let when = now.saturating_add(*initial_delay); | |
| Self::schedule_enactment(index, DispatchTime::At(when), call)?; | |
| Proposal::Review | |
| } | |
| }; | |
| ReferendumCount::<T>::put(index.saturating_add(1)); | |
| ActiveCount::<T>::put(active.saturating_add(1)); | |
| ActivePerProposer::<T>::insert(&proposer, active_per_proposer.saturating_add(1)); |
|
🔄 AI review updated — Skeptic: VULNERABLE |
| ReferendumCount::<T>::put(index.saturating_add(1)); | ||
| ActiveCount::<T>::put(active.saturating_add(1)); | ||
| ActivePerProposer::<T>::insert(&proposer, active_per_proposer.saturating_add(1)); | ||
|
|
||
| let proposal = match &track_info.decision_strategy { | ||
| DecisionStrategy::PassOrFail { | ||
| decision_period, .. | ||
| } => { | ||
| let when = now.saturating_add(*decision_period); | ||
| Self::set_alarm(index, when)?; | ||
| let bounded_call = T::Preimages::bound(*call)?; | ||
| Proposal::Action(bounded_call) | ||
| } | ||
| DecisionStrategy::Adjustable { initial_delay, .. } => { | ||
| let when = now.saturating_add(*initial_delay); | ||
| Self::schedule_enactment(index, DispatchTime::At(when), call)?; |
There was a problem hiding this comment.
[HIGH] submit leaks queue slots when setup fails
submit increments ReferendumCount, ActiveCount, and ActivePerProposer before the fallible setup calls at set_alarm, Preimages::bound, and schedule_enactment. Without making the extrinsic transactional or deferring these writes until after setup succeeds, an attacker can repeatedly submit calls that fail in scheduler/preimage setup and still consume global and per-proposer active capacity. No ReferendumStatusFor entry is inserted on these error paths, so there is no referendum that kill, expiry, or completion can later use to release the leaked slots. Move the capacity mutations after all fallible setup succeeds, add explicit rollback/cleanup, or wrap the dispatchable in a transactional boundary.
|
🔄 AI review updated — Skeptic: VULNERABLE |
1 participant
Summary
Part of #2804.
Adds
pallet-referenda, a generic track-based referendum state machine. It owns proposal lifecycle, scheduling, status transitions, and governed dispatch, while delegating voting to a backend such aspallet-signed-voting.What Changed
pallet-referenda.submit,kill,advance_referendum, andenactextrinsics.PassOrFailreferenda for approve/reject-by-deadline decisions.Adjustablereferenda for scheduled calls that can be fast-tracked, delayed, or cancelled.Enactedstatus update happen atomically.Detailed Reference
See
pallets/referenda/README.mdfor the full design reference. It covers the track model, decision strategies, state machines, scheduler wrapper, tally-update deferral, track-config snapshotting, per-proposer quotas, adjustment curve, integrity checks, and runtime configuration.Reviewers may want to focus on the dispatch safety path: governed calls are scheduled through
referenda.enact(index, call), so dispatch and theEnactedstatus transition happen in the same root call, and stale scheduled wrappers cannot execute terminal-no-dispatch referenda.Behavioral Impact
No live runtime behavior changes in this PR. The referenda pallet is added to the codebase but is not wired into the Subtensor runtime until the governance wiring PR.
Once wired, this pallet becomes the governance state machine that opens polls, reacts to voting tallies, schedules execution, and records final referendum status.
Migration / Spec Version
The pallet defines storage, but this PR does not add it to the runtime. No live storage migration or spec-version bump is required here.
Testing
Suggested local verification:
cargo test -p pallet-referenda