Skip to content

Prototype SV governance voter flow#6

Draft
ericmann wants to merge 6 commits into
mainfrom
eric-avro/gv-consolidated-prototype
Draft

Prototype SV governance voter flow#6
ericmann wants to merge 6 commits into
mainfrom
eric-avro/gv-consolidated-prototype

Conversation

@ericmann
Copy link
Copy Markdown
Collaborator

@ericmann ericmann commented May 13, 2026

Summary

This PR is a prototype for maintainer and CIP review, not a final upstream design. It makes the proposed Phase 1 SV governance-voter model concrete for discussion under Canton Development Fund PR canton-network#223, especially Milestone 1: Governance-Voting Identity and CIP.

Phase 1 preserves the existing one-vote-per-SV semantics. A governance voter is modeled as an alternate signer for a represented SV's vote, not as a new voting unit or source of additional vote weight.

Changes included:

  • Add a hardcoded governance-voter action taxonomy via isGovernanceVoterAction, with new ActionRequiringConfirmation constructors rejected by default until explicitly reviewed.
  • Extend Vote with castBy and castByRole so vote records distinguish the represented SV from the party/authority path that signed the vote.
  • Add SvGovernanceVoter, an SV-declared binding template without a contract key; downstream paths fetch it by contract ID and validate the one-active-binding assumption at the workflow level.
  • Add DsoRules_CastGovernanceVote, which validates the binding, represented SV, governance-voter signer, signer role, and action allowlist before writing the vote into the represented SV's existing vote slot.
  • Add Daml tests for the action taxonomy, vote-slot/tally semantics, binding lifecycle and authorization, and governance-voter casting/overwrite scenarios.
  • Add an SV-operator documentation note describing the prototype scope, proposed allowlist, attribution semantics, binding invariant, and explicit contract-ID submission shape.

Design IDs covered: GV-001, GV-002, GV-003, GV-004, GV-005, GV-006.

Notes For Reviewers

  • SRARC_OffboardSv is included in the proposed Phase 1 allowlist so reviewers can evaluate it explicitly; this should be validated through maintainer/CIP review because it is a high-impact membership action.
  • governanceVoter == sv is allowed for bootstrap/self-voting, while governanceVoter == dso is rejected.
  • The binding is unilateral/SV-declared in this prototype. If maintainers prefer bilateral consent, this can move toward a Propose-Accept shape in a follow-up revision.
  • The binding intentionally has no contract key in this prototype. The one-active-binding-per-SV rule is treated as an invariant to validate rather than a template-key constraint.

Test Plan

  • direnv exec . sbt "splice-dso-governance-test-daml/damlTest"
  • direnv exec . sbt damlDarsLockFileUpdate

@ericmann
Copy link
Copy Markdown
Collaborator Author

Daml Review Feedback

I ran a fresh Daml review pass over the consolidated governance-voter prototype. The code builds and the Daml tests pass, but there are several points worth making explicit for reviewers.

Key Findings

  • Shared vote slot semantics need explicit confirmation. DsoRules_CastVote and DsoRules_CastGovernanceVote both write to the represented SV's existing vote slot. That preserves one-vote-per-SV behavior, but it also means an operator vote and governance-voter vote can overwrite each other after cooldown. If this is intended, the PR should call it out clearly. If operator votes should take precedence, DsoRules_CastGovernanceVote should reject overwriting a prior VCR_Operator vote.

  • The binding intentionally has no contract key, but uniqueness is external. SvGovernanceVoter does not enforce one active binding per (sv, dso) at the ledger key level. That matches the prototype direction, but reviewers should confirm whether workflow-level validation is sufficient or whether the final design should introduce a registry/key pattern.

  • Unilateral SV-declared binding is clear but should be reviewed as an authority decision. The represented SV can create, rotate, and clear the binding without governance-voter acceptance. This is a simple Phase 1 model, but maintainers may still prefer a Propose-Accept flow if bilateral consent is required.

  • governanceVoter == sv remains allowed. The template rejects sv == dso and governanceVoter == dso, but deliberately allows self-designation for bootstrap/self-voting. Reviewers should confirm that this remains desirable.

  • Cooldown is shared across operator and governance-voter channels. Because both paths use the represented SV's vote slot, a governance voter can consume the cooldown window for the represented SV, and vice versa. This is consistent with one vote per SV, but should be understood as part of the user/operator experience.

  • Vote map keys remain existing SV names. The prototype preserves the current VoteRequest.votes map shape, keyed by SV name. A more robust future design might key by SV party to avoid name-change/name-collision edge cases, but changing that is broader than this prototype.

  • Allowlist validation is transactionally safe but could be reordered for readability. DsoRules_CastGovernanceVote archives the old VoteRequest before checking isGovernanceVoterAction; Daml rollback makes this safe, but fetching/validating before archiving would make the control flow easier to review.

Overall Assessment

No blocker was found for using this as a review prototype. The main review question is whether cross-channel overwrites are the desired Phase 1 semantics, or whether operator-cast votes should have precedence over governance-voter-cast votes.

@ericmann
Copy link
Copy Markdown
Collaborator Author

Follow-up Professor review after vote-slot fixes

I reran Professor review after commit ef1873501 (Tighten governance voter vote slots). Focus areas were the SvGovernanceVoter binding and the updated DsoRules vote-slot semantics.

What Professor confirmed

  • The governance-voter binding remains sound for the stated prototype constraints: SV-declared binding, sv as sole signatory, dso and governanceVoter as observers, governanceVoter == sv allowed, and governanceVoter == dso rejected.
  • The lack of a contract key is still called out as a production-roadmap concern, but matches the prototype/design-doc intent for this slice.
  • VoteRequest.votes : Map Party Vote now gives one stable vote slot per represented SV party.
  • All write paths now target the represented SV party slot:
    • requester vote: requester
    • operator vote: vote.sv
    • governance-voter vote: binding.sv
  • Governance voters can no longer overwrite a prior VCR_Operator vote.
  • Operators can still overwrite governance-voter votes, which matches the intended precedence model.
  • The governance-voter path now fetches/checks before archiving, which improves readability and intent even though transaction rollback would protect correctness.
  • Shared cooldown is still per represented SV slot, matching the one-vote-per-SV model.

Remaining Professor cautions / follow-up candidates

  • SvGovernanceVoter has no DSO revocation path. This is not part of the Phase 1 prototype but should be considered for production/offboarding hardening.
  • Duplicate bindings remain possible without a key. This remains intentional for the prototype, with the one-active-binding-per-SV invariant left to workflow validation/review.
  • Cast choices do not explicitly check now < request.voteBefore; late votes are accepted until the request is closed. If this is not intended existing behavior, add a deadline guard to both operator and governance-voter cast paths.
  • DsoRules_CloseVoteRequest uses sv : Optional Party as controller. Professor flagged the empty-controller shape as worth hardening if the helper ever accepts None.
  • Governance-voter-over-governance-voter overwrite remains last-writer-wins if duplicate active bindings exist. This follows the prototype assumption that binding uniqueness is enforced outside the template key space.

Local validation

  • direnv exec . sbt "splice-dso-governance-test-daml/damlTest"
  • direnv exec . sbt damlDarsLockFileUpdate

Copy link
Copy Markdown

@agentdamo agentdamo left a comment

Choose a reason for hiding this comment

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

Automated Review

PR: #6 — Prototype SV governance voter flow

Summary

Adds a Phase 1 governance-voter model to Splice DSO governance. An SV can declare a SvGovernanceVoter binding designating a separate party to cast votes on its behalf for an explicit allowlist of governance actions. Vote records are extended with castBy/castByRole for attribution. VoteRequest.votes is migrated from Map Text Vote to Map Party Vote.

Review

1. Correctness

  • DsoRules_CastVote silently normalizes caller-supplied castBy/castByRole: The recorded vote always sets castBy = vote.sv and castByRole = VCR_Operator regardless of what was passed. The controller is vote.sv so attribution is correct, but this normalization is invisible to callers. Consider either a require asserting the values match, or a comment explaining the intentional overwrite.
  • executeAllDefinitiveVotes fix looks correct: Switching from map (.name) $ Map.values to Map.keys aligns properly with the map-key migration. Good catch.
  • Map Text VoteMap Party Vote is a breaking data model change: Any existing in-flight VoteRequest contracts on a live network would need a migration path. For a prototype this is expected, but should be flagged explicitly in the CIP/upstream PR so reviewers understand the upgrade surface.

2. Security

  • Unilateral binding: SvGovernanceVoter has signatory sv only — the governance voter is an observer with no consent step. An SV can secretly designate any party. The docs flag this and propose a Propose-Accept shape as a follow-up, which is the right call. Worth flagging for CIP review: designating a governance voter without their consent could be surprising.
  • No contract key / one-active-binding not enforced at template level: An SV can create multiple SvGovernanceVoter contracts simultaneously. Only the explicitly submitted bindingCid is validated, so this doesn't compromise vote integrity — but it creates confusion about which binding is canonical. A fetchByKey or explicit de-dup check in the UI/workflow layer should be documented as a requirement.
  • Operator vote precedence is correct: The guard pastVote.castByRole == VCR_Operator → fail enforces that governance-voter casts cannot overwrite operator casts. Governance-voter-over-governance-voter overwrites are allowed, which matches the stated intent.
  • Action allowlist is correctly defensive: _ -> False default ensures new constructors are rejected until deliberately reviewed. SRARC_OffboardSv inclusion is appropriately called out for CIP scrutiny.
  • DsoRules_CastGovernanceVote redundant but safe normalization: The recordedVote block reassigns castBy = binding.governanceVoter and castByRole = VCR_GovernanceVoter even though both were already validated by earlier requires. This is good defense-in-depth — stored data always reflects the binding, not caller-supplied fields.

3. Clarity

  • DsoRules_CastVote normalization: As noted above, the silent overwrite of castBy/castByRole in the operator path needs a comment. Otherwise a future maintainer may think the caller's values are honored.
  • VoteCastRole deriving style: deriving(Eq, Show) is missing a space (deriving (Eq, Show)) — minor, but inconsistent with the surrounding Daml style.
  • DsoRules_CastGovernanceVote is nonconsuming (it archives and recreates the request manually). The pattern diverges from DsoRules_CastVote which uses fetchAndArchive. A short comment explaining why fetchChecked + manual archive is used here (validation must occur before archiving) would help.

4. Edge Cases

  • Multiple active bindings: Addressed above. No double-vote risk, but the workflow-layer invariant assumption should be documented more explicitly (current doc says "treated as an invariant to validate" but doesn't specify where/how).
  • Governance voter == another active SV: Nothing prevents sv1's governance voter from being sv2's party. sv2 could then cast governance-voter votes for sv1 while also casting operator votes for itself. No double-counting occurs (different vote slots), but this could be confusing operationally. Worth a note in the docs or a comment.
  • Cooldown on first governance-voter cast: Correctly unenforced when no prior vote exists (the Map.lookup returns None). This is the right behavior.

5. Excellence

  • Tests are comprehensive: binding lifecycle, casting/overwrite semantics, action taxonomy, and vote-slot tally are all covered. The use of submitMustFail throughout is idiomatic and thorough.
  • The doc page (sv_governance_voter.rst) is clear and well-written. The explicit "prototype scope" framing is good.
  • Including the compiled DAR (splice-dso-governance-0.1.25.dar) and updated lock file is appropriate for a prototype review.

DAML Professor

DAML Professor unavailable, skipping.


Automated review by Damo. PR is draft — comment only, no approval.

Copy link
Copy Markdown

@agentdamo agentdamo left a comment

Choose a reason for hiding this comment

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

Approving at author's request (draft used to prevent accidental merge, not to indicate incompleteness). Findings in the prior comment are all suggestions — nothing blocking. LGTM for prototype/CIP review purposes.

ericmann added 6 commits May 13, 2026 16:32
Define the initial Phase 1 allowlist for governance-voter eligible actions and cover the proposed taxonomy plus represented-SV vote-slot semantics in Daml tests.

Signed-off-by: Eric Mann <eric@avrofi.com>
Add explicit vote-cast role metadata so the vote record can identify the represented SV and operator signing path without changing tally semantics.

Signed-off-by: Eric Mann <eric@avrofi.com>
Introduce the SV-declared governance-voter binding lifecycle without a contract key so the Phase 1 authority model can be reviewed independently of submit-path mechanics.

Signed-off-by: Eric Mann <eric@avrofi.com>
Use the SV governance-voter binding to authorize alternate signing while recording the vote in the represented SV's existing vote slot.

Signed-off-by: Eric Mann <eric@avrofi.com>
Keep the consolidated prototype to a single governance package version bump so intermediate stack-only DAR versions do not leak into the final review branch.

Signed-off-by: Eric Mann <eric@avrofi.com>
Use represented SV parties as vote map keys and prevent governance-voter submissions from replacing prior operator votes, making the shared vote-slot semantics explicit for review.

Signed-off-by: Eric Mann <eric@avrofi.com>
@ericmann ericmann force-pushed the eric-avro/gv-consolidated-prototype branch from ef18735 to e764089 Compare May 13, 2026 16:32
Copy link
Copy Markdown

@samsondav samsondav left a comment

Choose a reason for hiding this comment

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

Reviewed against canton-network#5533 (the upstream prototype). The diff confirms this branch is earlier — a few of the larger gaps below have already evolved in the upstream version, so treat this as a prototype-review thread rather than a merge gate for this fork.

Three findings are load-bearing for both this branch and the upstream prototype:

  1. controller vote.castBy on DsoRules_CastGovernanceVote is the choice-parameter-as-controller footgun. It is functionally correct today only because of five sequential require checks; engine-level authorization is degraded to handwritten runtime assertions.
  2. SvGovernanceVoter has no on-ledger single-binding enforcement. RotateGovernanceVoter and ClearGovernanceVoter are consuming on the source cid, but a bare create SvGovernanceVoter by the SV signatory bypasses them. Two governance voters can simultaneously write to the same votes[binding.sv] slot (subject to the operator-precedence carve-out in this branch). No test covers the failure mode. Daml 3.x dropped key/maintainer, so the standard fix is unavailable — needs an off-ledger ACS check encoded via a DsoRules-mediated registration choice, or a governanceVoterBindingCid field in SvInfo that the cast choice gates against.
  3. Schema break in VoteRequest.votes (Map Text → Map Party) and Vote (two new mandatory fields). LF 2.1 upgradability does not handle this in-place. Package-version bump is present but the PR description needs an explicit migration story (close all open requests before upgrade, or a one-shot reshape choice).

The rest of the inline comments are medium / low. Several PR-6-specific shapes (empty ClearGovernanceVoter body, lack of RequestGovernanceVote, operator-precedence carve-out, operator path with no eligibility check) appear already addressed in canton-network#5533 — flagging here mostly as breadcrumbs in case any of them slipped during the cherry-pick back.

Also: SRARC_OffboardSv in the allowlist is the highest-impact policy decision in the PR. Already flagged in PR description for upstream review — keep it loud in the CIP discussion.

Comment thread daml/splice-dso-governance/daml/Splice/DsoRules.daml
Comment thread daml/splice-dso-governance/daml/Splice/DSO/GovernanceVoter.daml
Comment thread daml/splice-dso-governance/daml/Splice/DSO/GovernanceVoter.daml
Comment thread daml/splice-dso-governance/daml/Splice/DSO/GovernanceVoter.daml
Comment thread daml/splice-dso-governance/daml/Splice/DsoRules.daml
Comment thread daml/splice-dso-governance/daml/Splice/DsoRules.daml
Comment thread daml/splice-dso-governance/daml/Splice/DsoRules.daml
Comment thread daml/splice-dso-governance/daml/Splice/DsoRules.daml
Comment thread daml/splice-dso-governance/daml/Splice/DsoRules.daml
Comment thread daml/splice-dso-governance/daml/Splice/DsoRules.daml
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.

3 participants