Skip to content

Governance: tech-collective-only (hardened)#604

Merged
n13 merged 4 commits into
mainfrom
governance/tech-collective-only
Jun 24, 2026
Merged

Governance: tech-collective-only (hardened)#604
n13 merged 4 commits into
mainfrom
governance/tech-collective-only

Conversation

@n13

@n13 n13 commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Alternative to #603. Instead of introducing a new pallet-upgrade-gov, this PR keeps the tech-collective referenda lane as the sole governance mechanism and hardens it. Both PRs share the same base — the permissionless, token-weighted community lane is removed (pallet-conviction-voting deleted, community Referenda instance dropped) — so they compare directly:

Net change vs main: +342 / −5339 — strictly less code than #603.

The commit history contains the upgrade-gov pallet (introduced on the shared base) and its removal; the net diff against main contains no upgrade-gov files.

Removed

Tech-lane audit fixes

Finding Sev Fix
#91247 / #91270 high TechCollectiveTracksInfo::track_for accepts only a Root proposal origin. Previously any Signed(_) mapped to track 0, letting a passed referendum dispatch as an arbitrary account (impersonation) or route Root dispatch through the single low-threshold track. All legitimate submissions already use a Root proposal origin.
#91271 high ready_for_deciding honors the bounded-queue insertion result instead of always setting in_queue = true → no more ghost-queued referenda (skipped by timeout yet absent from TrackQueue).
#91210 high schedule_enactment / set_alarm log a loud error on scheduler failure instead of failing silently in release (was debug_assert!-only).
#91213 low cancel/kill release a deciding slot only when the referendum actually held one → no DecidingCount corruption (matters at max_deciding = 1).

Already fixed on the shared base: #91272, #91267, #91248, #91165.

Parameter hardening

  • prepare_period: 20 blocks (4 min) → 2 hours advance notice before deciding.
  • UndecidingTimeout comment corrected (value is 45 days, not 90).
  • CancelOrigin = Root and max_deciding = 1 retained (serialized upgrade lane); the real-time defense against a malicious referendum is nay votes within the 24h confirm window — see docs/TECH_COLLECTIVE_GOVERNANCE_TUNING.md.

Assessed as not-applicable / non-issue

  • #91141 (abstentions satisfy approval), #91166 (self-vote weight): pallet-conviction-voting issues — that pallet is deleted; the tech lane uses ranked-collective's aye/nay head-count tally (no abstain).
  • #91193 (unbounded member exchange): exchange_member is NeverEnsureOrigin (disabled).
  • #91265 (removed members' votes stay active): upstream ranked-collective behavior; membership is Root-gated and changes are rare in a small trusted collective. Treated as a non-issue.

Test plan

  • cargo test -p pallet-referenda — 39/39
  • cargo build -p quantus-runtime (native + WASM)
  • cargo test -p quantus-runtime --features fast-governance — 23 passed / 1 ignored
  • cargo check -p quantus-runtime --features runtime-benchmarks
  • Regenerate referenda weights if desired (logic-only changes)

n13 added 4 commits June 24, 2026 14:52
… tech collective

Replace the public/token-weighted community governance lane with a minimal,
upgrades-only governance pallet, keep the tech-collective referenda lane as a
transitional fallback, and fix verified audit findings in the retained code.

New:
- pallet-upgrade-gov (index 22): M-of-N member approval + timelock that
  authorizes runtime upgrades via frame_system::authorize_upgrade only. Fixed
  Action enum (no arbitrary dispatch), internal membership/threshold/delay
  management, on_initialize timelock. Seeded at genesis with the same trusted
  set as the tech collective. 11 unit tests.

Removed (community lane):
- pallet-conviction-voting (deleted).
- Community Referenda instance (idx 10) and ConvictionVoting (idx 12); indices
  kept vacant. CommunityTracksInfo removed.
- Dead community-lane integration tests (engine.rs, logic.rs).

Retained as transitional fallback: TechCollective + TechReferenda, to be removed
once upgrades via pallet-upgrade-gov are proven on-chain.

Audit fixes in retained code:
- #91267: tech-collective Add/RemoveOrigin now EnsureRootWithSuccess (Root-only,
  i.e. a passed referendum) instead of any single member.
- #91248: removed dead Root branch in RootOrMemberForTechReferendaOrigin.
- #91272: referenda submission deposit now refundable for Rejected/TimedOut.
- #91165: preimage per-byte deposit corrected (was 1000x too low).

spec_version 132 -> 133.
Alternative to the pallet-upgrade-gov approach (#603): keep the tech-collective
referenda lane as the sole governance mechanism and harden it, rather than
adding a new minimal upgrade pallet. Builds on the shared base (community lane
removed, #91267/#91248/#91272/#91165 fixed).

Removed:
- pallet-upgrade-gov (crate, runtime wiring, config, genesis seeding). The tech
  lane authorizes runtime upgrades and other Root calls directly.

Tech-lane audit fixes (local referenda fork + runtime config):
- #91247/#91270 (high): TechCollectiveTracksInfo::track_for now accepts only a
  Root proposal origin. Previously any Signed(_) mapped to track 0, so a passed
  referendum could dispatch as an arbitrary account (impersonation) or route
  Root-level dispatch through the single low-threshold track. All legitimate
  submissions already use a Root proposal origin.
- #91271 (high): ready_for_deciding honors the bounded-queue insertion result
  instead of unconditionally setting in_queue = true, preventing ghost-queued
  referenda (skipped by timeout yet absent from TrackQueue).
- #91210 (high): schedule_enactment and set_alarm log a loud error on scheduler
  failure instead of failing silently (debug_assert-only in release).
- #91213 (low): cancel/kill only release a deciding slot when the referendum
  actually held one, preventing DecidingCount corruption (relevant at
  max_deciding = 1).

Parameter hardening:
- TechReferenda prepare_period 20 blocks (4 min) -> 2 hours of advance notice.
- UndecidingTimeout comment corrected (value is 45 days, not 90).

Moot via the community-lane removal already on this branch:
- #91141 (abstentions satisfy approval), #91166 (self-vote weight): both were
  pallet-conviction-voting issues; that pallet is deleted.
- #91193 (unbounded member exchange): exchange_member is NeverEnsureOrigin.
- #91265 (removed members' votes): non-issue (upstream ranked-collective
  behavior; membership changes are Root-gated and rare).

Tests: pallet-referenda 39/39; runtime governance 23 passed/1 ignored; native +
WASM build and runtime-benchmarks compile clean.
@n13 n13 changed the title Governance: tech-collective-only (hardened) — alternative to #603 Governance: tech-collective-only (hardened) Jun 24, 2026
@n13

n13 commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

I diffed our forks against upstream master and the latest published crates (pulled the real source for pallet-referenda, pallet-ranked-collective, pallet-conviction-voting). Here's the full mapping.

Version context

Our forks are ~3 majors behind: pallet-referenda 45.0.0 → 48.0.0 latest (verified), with ranked-collective/preimage 45.x and scheduler 40.x similar. So "higher versions exist" is true — but as you'll see, the high-severity bugs we fixed are still unfixed even in master, so bumping versions would not have delivered them.

Finding-by-finding: upstream vs our fix

A. Fixes we made in the kept lane (referenda)

Finding Sev Fixed in a higher upstream version? Our fix Better
#91271 ghost-queued referenda High No. master 48.0.0 still does status.in_queue = true; then ignores the bounded-insert result (referenda/src/lib.rs:1024) We set in_queue to the actual insert_sorted_by_key(...) boolean Ours — upstream still buggy everywhere
#91210 silent scheduler failures High No. master still debug_assert!(...)-only in set_alarm (l.953) and schedule_enactment (l.932) — silent in release log::error! on schedule failure Ours — upstream still silent in release
#91213 cancel/kill corrupt DecidingCount Low Master only, unreleased. master added the identical if status.deciding.is_some() guard (l.603/630); not in 45.0.0 and not in latest release 48.0.0 Same status.deciding.is_some() guard Tie — we independently matched the upstream master fix, ahead of every release
#91272 deposit frozen after reject/timeout Critical No. master take_submission_deposit still returns Err for Rejected/TimedOut/Killed (refunds only Approved/Cancelled) Refund Rejected/TimedOut; Killed → Ok(None) Ours is a deliberate divergence (see flag below)

B. Config-level findings (not a pallet-version thing)

Finding Sev Upstream Our fix Better
#91247 / #91270 impersonation via track_for High The pallet stores proposal_origin verbatim in all versions (by design). Real upstream runtimes (Polkadot/Kusama root track) reject non-Root in track_for track_for accepts Root only Aligned with upstream runtime convention
#91267 single member adds/removes members Low Config; upstream runtimes gate membership via governance origins, never a lone member AddOrigin/RemoveOrigin = Root Aligned

C. Moot for us (community lane deleted)

Finding Sev Upstream higher version Our approach Better
#91141 abstentions satisfy approval High Fixed in a release: pallet-conviction-voting@45.0.3, PR #11567 — adds if total.is_zero() { Perbill::zero() } We deleted conviction-voting; and the tech lane's ranked tally was never vulnerable (1.max(ayes+nays), ranked-collective/src/lib.rs:135) Both resolve it — deletion also removes the whole class of community-lane findings; #11567 is what you'd use if keeping it
#91166 underweighted remove_other_vote Med conviction-voting weights Deleted n/a

D. Audit "false positive" — and upstream agrees with you

Finding Sev Upstream Our decision
#91265 removed members' votes stay active High (audit) Not treated as a bug. Upstream cleans votes post-poll via cleanup_poll; remove_member (even after PR #2591) does not reconcile votes in ongoing polls Won't-fix — matches upstream by-design behavior

E. Not addressed by us / not separately diffed

#91193 (unbounded exchange_member — disabled via NeverEnsureOrigin in our runtime), #91207 (referenda migration), #91219/#91225 (scheduler retry feature), #91273 (preimage), #91173 (preimage try-runtime QA). We didn't modify scheduler/preimage logic, and I didn't diff these against upstream — flag if you want me to.

Verdict

  • The two findings that matter most (#91271, #91210, both High) are unfixed in every upstream version including master. Our patches are genuinely net-new — version-bumping wouldn't help.
  • #91213: upstream master converged on the exact guard we wrote — strong validation — but it's not in any release yet, so again a bump wouldn't deliver it.
  • #91141 is the only one upstream fixed in a release (45.0.3); we went further by removing the pallet.

One thing to double-check

#91272 is our only real divergence from upstream. Upstream deliberately keeps the submission deposit non-refundable on Rejected/TimedOut (master still does). Our change makes it refundable. I believe that's the correct, user-fair policy (only kill should burn it), and tests pass — but since we're now off the upstream path here, it's worth a conscious sign-off that this is intended policy rather than an invariant we're breaking.

Recommended follow-up (separate from these PRs): rebase the forks onto 48.0.0 to pick up unrelated upstream fixes/security patches, then re-apply our 4 patches on top — since the fixes we need aren't upstream-released anyway.

Want me to (a) write this up as a doc next to PR #604, (b) pull the exact upstream diff snippets for #91271/#91210/#91213 into the PR description, or (c) scope the 45→48 rebase?

@n13 n13 merged commit f7581b7 into main Jun 24, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants