Skip to content

pallet-referenda extracted from governance umbrella PR#2714

Open
l0r1s wants to merge 4 commits into
devnet-readyfrom
governance-referenda
Open

pallet-referenda extracted from governance umbrella PR#2714
l0r1s wants to merge 4 commits into
devnet-readyfrom
governance-referenda

Conversation

@l0r1s

@l0r1s l0r1s commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Part of #2198

pallet-referenda owns the referendum state machine and coordinates with a
voting backend such as pallet-signed-voting through the shared poll traits.

What Changed

  • Added pallet-referenda with submit, kill, advance_referendum, and
    enact extrinsics.
  • Added track-based referendum configuration through TracksInfo, including
    proposer sets, voter sets, voting schemes, proposal authorization, and
    per-track decision strategies.
  • Added PassOrFail referenda for approve/reject-by-deadline decisions, with
    approval either executing the call or delegating it into a child review
    referendum.
  • Added Adjustable referenda for scheduled-call timing decisions, where votes
    can fast-track, cancel, or shift the scheduled dispatch time.
  • Added scheduler wrapper handling so governed dispatch and the Enacted
    status transition happen atomically.
  • Added global and per-proposer active referendum limits.
  • Added strategy snapshotting at submission time so runtime track changes only
    affect new referenda.
  • Added runtime integrity checks for duplicate tracks, invalid review targets,
    empty/zero decision parameters, and overlapping approve/reject or
    fast-track/cancel thresholds.
  • Added benchmarks, generated weights, mock runtime support, README
    documentation, and unit tests covering submission, lifecycle transitions,
    delegation, scheduler/preimage cleanup, voting-backend hooks, track integrity,
    and try-state checks.
  • Added the shared pad_name helper used by fixed-width track names.

Behavioral Impact

This PR adds the referenda pallet crate and supporting common helper, but does
not wire the pallet into the runtime by itself.

Once connected by a follow-up runtime integration PR, referenda will act as the
poll producer for voting backends. Submitting a referendum will create a poll,
schedule the appropriate alarm or enactment task, and notify the voting backend.
Tally updates from the backend will re-arm the referendum for deferred state
machine evaluation, keeping voting updates separate from governance execution.

The pallet supports two governance flows:

  • PassOrFail: a proposal is approved, rejected, or expired before a deadline.
  • Adjustable: a proposal is scheduled up front, then votes can accelerate,
    cancel, or delay dispatch.

Migration / Spec Version

The new pallet defines storage, but this PR does not add it to the runtime.
Because the pallet is not wired into the runtime in this PR, there is no live
storage migration or spec-version bump required here.

A later runtime integration PR should handle runtime configuration, track
definitions, spec-version changes, and any migration implications when this
pallet is wired in.

Testing

The PR includes pallet unit tests and benchmark code for the new referenda
surface. Suggested local verification:

cargo test -p pallet-referenda

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/referenda/src/lib.rs
Comment thread pallets/referenda/src/lib.rs Outdated
Comment thread pallets/referenda/src/lib.rs
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: VULNERABLE

Baseline scrutiny: l0r1s has write permission, an established 2018 account, substantial prior subtensor history, and author/committer match; branch is governance-referenda -> devnet-ready.

The PR does not modify .github/ai-review/* or .github/copilot-instructions.md. Dependency changes are limited to a new in-repo pallet-referenda crate using existing workspace/path dependencies; I did not find a new external supply-chain dependency. Two prior findings are addressed by the latest commits, but the submit-time accounting leak remains present.

Findings

Sev File Finding
HIGH pallets/referenda/src/lib.rs:526 Submit can leak active referendum slots on scheduler/preimage failure inline

Prior-comment reconciliation

  • 559b42bf: not addressed — The active counters are still incremented before set_alarm, Preimages::bound, and schedule_enactment can fail in submit.
  • 6be9f109: addressedset_alarm now uses reschedule_named when an alarm already exists instead of cancelling first and then scheduling a replacement.
  • 9679e946: addressed — Track integrity now rejects PassOrFail threshold pairs whose sum is at or below 100%, preventing both approval and rejection thresholds from being valid for the same signed-voting tally split.

Conclusion

This looks like a legitimate governance-pallet extraction, but submit can still permanently consume governance queue/proposer slots if a fallible scheduler or preimage operation fails after the counters are incremented.


📜 Previous run (superseded)
Sev File Finding Status
HIGH pallets/referenda/src/lib.rs:510 Submit can leak active referendum slots on scheduler/preimage failure ➡️ Carried forward to current findings
The active counters are still incremented before set_alarm, Preimages::bound, and schedule_enactment can fail in submit.
MEDIUM pallets/referenda/src/lib.rs:1033 Replacing an alarm can delete the only wake-up if scheduling fails ✅ Addressed
set_alarm now uses reschedule_named when an alarm already exists instead of cancelling first and then scheduling a replacement.
MEDIUM pallets/referenda/src/lib.rs:754 Pass-or-fail tracks approve when both thresholds are crossed ✅ Addressed
Track integrity now rejects PassOrFail threshold pairs whose sum is at or below 100%, preventing both approval and rejection thresholds from being valid for the same signed-voting tally split.

# 🔍 AI Review — Auditor (domain review) has not yet run on this PR.

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@l0r1s l0r1s mentioned this pull request Jun 3, 2026
@l0r1s l0r1s marked this pull request as ready for review June 3, 2026 19:49

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/referenda/src/lib.rs
Comment thread pallets/referenda/src/lib.rs Outdated
Comment thread pallets/referenda/src/lib.rs
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@l0r1s l0r1s changed the base branch from governance-signed-voting to devnet-ready June 22, 2026 18:13
@l0r1s l0r1s added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Jun 22, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/referenda/src/lib.rs
Comment thread pallets/referenda/src/lib.rs Outdated
/// do not need to track whether one is currently pending.
fn set_alarm(index: ReferendumIndex, when: BlockNumberFor<T>) -> Result<(), DispatchError> {
let call = T::Preimages::bound(CallOf::<T>::from(Call::advance_referendum { index }))?;
let _ = T::Scheduler::cancel_named(alarm_name(index));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Replacing an alarm can delete the only wake-up if scheduling fails

set_alarm cancels the existing alarm before attempting schedule_named. Callers such as on_tally_updated and expire_or_rearm_deadline rely on this alarm for automatic referendum progress; if schedule_named fails after the cancel, the referendum can be left Ongoing with no future wake-up except manual root intervention. Preserve the existing alarm until the replacement is known to be installed, or use a reschedule path that does not drop the old alarm on failure.

Comment thread pallets/referenda/src/lib.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/referenda/src/lib.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-cargo-audit This PR fails cargo audit but needs to be merged anyway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant