Skip to content

Fix governance proposal kind ambiguity#448

Draft
carrion256 wants to merge 5 commits into
audit/governance-role-topology-a044-a047-a063-a084from
audit/governance-proposal-identity-and-timelock-semantics
Draft

Fix governance proposal kind ambiguity#448
carrion256 wants to merge 5 commits into
audit/governance-role-topology-a044-a047-a063-a084from
audit/governance-proposal-identity-and-timelock-semantics

Conversation

@carrion256

@carrion256 carrion256 commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes governance proposal broad-kind ambiguity and documents the in-flight timelock policy with one finding-scoped commit per fix:

  • #FIND-065accept_kind ambiguity when multiple pending proposals share the same broad action kind.
  • #FIND-088revoke_kind previously cancelled all proposals of a broad kind while accept_kind accepted only one.
  • #FIND-095 — in-flight proposals intentionally retain the submit-time timelock even if that timelock is raised later.

Commits

  • fix governance accept kind ambiguity (#FIND-065)
  • fix governance revoke kind semantics (#FIND-088)
  • test governance in-flight timelock policy (#FIND-095)

Changes

  • Make accept_kind fail with GovernanceError::DuplicatePending when more than one pending proposal matches the requested broad GovernanceActionKind.
  • Make revoke_kind use the same exactly-one broad-kind policy instead of revoking every matching proposal.
  • Keep proposal-id and action-key revocation paths unchanged for precise proposal management.
  • Add regressions for ambiguous CapGroup acceptance, ambiguous TimelockConfig revocation, and submit-time timelock semantics.

Triage notes

  • #FIND-091: current branch already carries focused no-change/classifier coverage around duplicate submissions, cap/group classifiers, and membership clear semantics; no additional code change was needed in this PR slice.
  • #FIND-059: current branch already mirrors cap-group membership state and tests membership clear/no-change classification.
  • #FIND-061: not changed here; broad abdication remains kind-level by design unless product wants a new precise-action abdication API.

Verification

  • cargo fmt -p templar-soroban-governance
  • cargo test -p templar-soroban-governance accept_kind_rejects_ambiguous_broad_kind -- --nocapture
  • cargo test -p templar-soroban-governance revoke_kind_rejects_ambiguous_broad_kind -- --nocapture
  • cargo test -p templar-soroban-governance in_flight_proposal_keeps_submit_time_timelock_after_timelock_raise -- --nocapture
  • cargo test -p templar-soroban-governance -- --nocapture
  • post-commit hook: Soroban size-budget-check passed (93961 bytes)

This change is Reviewable

@coderabbitai

coderabbitai Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 437ac9a5-d610-4a3d-9c17-7bcb1f230e38

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch audit/governance-proposal-identity-and-timelock-semantics

Comment @coderabbitai help to get the list of available commands and usage tips.

@carrion256 carrion256 force-pushed the audit/governance-proposal-identity-and-timelock-semantics branch 2 times, most recently from dd4aa5f to 7234c47 Compare May 19, 2026 08:28
Reject broad accept_kind when multiple pending proposals share the requested GovernanceActionKind, so callers cannot accidentally accept an arbitrary matching proposal. Add a CapGroup regression proving ambiguous broad-kind acceptance leaves all proposals pending.
Make revoke_kind match accept_kind exactly-one semantics: a broad GovernanceActionKind revoke now fails with DuplicatePending when multiple pending proposals share that kind instead of cancelling all of them. Add a TimelockConfig regression proving ambiguous broad-kind revocation preserves both proposals.
Make the policy explicit: pending proposals keep their submit-time ready_at_ns even if the corresponding governance timelock is raised before acceptance. The regression accepts the curator proposal at the original maturity after raising the curator timelock.
@carrion256 carrion256 force-pushed the audit/governance-proposal-identity-and-timelock-semantics branch from 7234c47 to 14fa1ec Compare May 20, 2026 09:44
@carrion256 carrion256 changed the base branch from spr/refactor/vault-ergonomics/4f330057 to audit/governance-role-topology-a044-a047-a063-a084 May 20, 2026 09:44
Stop granting the governance contract full Curator authority at runtime bootstrap. Governance-executed policy commands now use the execute_governance identity check plus internal preauthorized policy mutation paths, while direct policy admin entrypoints remain Curator-gated.
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.

1 participant