Add hosts to an existing external party: the serial-N+1 topology write - #379
Open
schronck wants to merge 3 commits into
Open
Add hosts to an existing external party: the serial-N+1 topology write#379schronck wants to merge 3 commits into
schronck wants to merge 3 commits into
Conversation
Canton runs this same validator when a participant uploads a DAR, so a broken lineage failed at deploy time on a real network and the PR that caused it got no signal. #320 bumped governance-utility-onboarding-v1 to 0.3.0 and a reviewer ran the check by hand. The command in the issue, `dpm upgrade-check --both`, does not run as written: the tool requires DAR arguments. Passing ../releases/v1/*.dar checks every committed version together, so a package with three releases has its whole chain validated rather than only the newest pair. It reads the DARs dpm build --all already produced, so it costs seconds. Verified it fails, not only that it passes: built governance-action-v1 as 0.2.0 with a non-Optional field added to GovernableActionView, and the check exited 1 with NOT_VALID_UPGRADE_PACKAGE naming the field. Reverted after. Closes #357
The step's comment claimed it read what dpm build --all produced. It did not: it read ../releases/v1/*.dar, the committed history. Copilot caught the wording, and checking the code showed the wording was describing the better behaviour. A version bumped in daml.yaml has no committed DAR until someone commits one, so the committed-only check could not see it. That is exactly when a reviewer wants to hear about a broken upgrade. Now passes both sets. A package in both appears once, since the validator keys on package id and the DAR-verification step above already pins those identical. Lineages checked went from 57 to 69. Re-proved the gate still fails with the shipping invocation: built governance-action-v1 as 0.2.0 with a non-Optional field and no committed DAR, and it exited 1 naming the field. That case would have passed under the previous command. Review finding from Copilot on #357.
The tenant API could only ever write serial 1. validate_onboarding_topology rejects any higher serial and rejects the onboarding marker outright, so an external party could never gain a host after it was created — which both plans for decentralizing an existing party need. add_hosts_mapping reads the current PartyToParticipant, carries every current host over untouched, and appends the new ones at Confirmation with Canton's Onboarding marker. The threshold does not move: a new host does not count toward it until its marker clears, so bundling the raise would let the party's active hosts fall below its own threshold mid-flight. The wallet pins the base serial because it compares what every host prepared byte-for-byte, and two hosts reading head state a moment apart would otherwise disagree for a reason that is not an attack. validate_add_hosts_topology is the sibling of the onboarding validator and exists for the same reason: the host co-signs the caller's bytes with its own topology key. The stakes are higher here because the party already holds contracts, so a forged serial N+1 could evict its current hosts or drop its threshold. Every field is checked against this node's own head-state read. No endpoint yet — that is the tenant API v2 work.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds the first building block for “decentralize an existing external party” by introducing a dedicated read/modify/write path to generate and validate a PartyToParticipant topology update at serial N+1 (rather than onboarding’s fixed serial 1). This enables adding hosting participants to an already-existing external party without weakening the onboarding validator’s stricter rules.
Changes:
- Introduces
external_party::add_hostswith:- a head-state reader for current authorized
PartyToParticipant+ serial, - a deterministic mapping builder that adds new hosts at Confirmation with the
Onboardingmarker, - a validator that refuses any topology bundle that isn’t exactly an “add hosts only” update at serial N+1.
- a head-state reader for current authorized
- Exposes the existing
party_queryhelper to sibling modules to support add-hosts topology reads. - Extends CI to automatically verify Daml package upgrade lineage across newly built DARs and the shipped release history.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/decman/src/workflow/external_party/steps.rs |
Makes party_query visible to sibling modules to support the new add-hosts flow. |
crates/decman/src/workflow/external_party/mod.rs |
Exposes the new add_hosts module. |
crates/decman/src/workflow/external_party/add_hosts.rs |
Implements add-hosts topology reader/builder/validator plus a comprehensive unit test suite. |
.github/workflows/ci.yml |
Adds an automated Daml upgrade-lineage validation step in the Daml CI job. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
scolear
approved these changes
Aug 27, 2026
Comment on lines
+17
to
+20
| //! * The threshold does not move here. Raising it is a separate serial bump, | ||
| //! because a new host does not count toward the threshold until its marker | ||
| //! clears — bundling the two would let the party's active hosts fall below its | ||
| //! own threshold mid-flight. |
Member
There was a problem hiding this comment.
Is this really true? We can't do threshold + new hosts in one serial bump safely?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First piece of "decentralize an existing party": the tenant API can now build a topology write that is not serial 1.
Why
validate_onboarding_topologyhard-rejects any serial other than 1 and rejects theOnboardingmarker outright (steps.rs:150, steps.rs:189). That is correct for onboarding — it creates a party and must never rewrite one — but it also means an external party can never gain a host after creation. Both Plan A (existing external party) and Plan B1 (converted local party) need exactly that write.Rather than loosen the onboarding validator, this adds a sibling path with its own rules.
What
crates/decman/src/workflow/external_party/add_hosts.rs:read_party_to_participant— the party's authorized mapping and its serial, from head state.add_hosts_mapping— read-modify-write. Current hosts carried over byte-identical, new hosts appended at Confirmation with theOnboardingmarker, participants sorted canonically.prepare_add_hosts— checks the pinned base serial against head state, thenGenerateTransactionsat serial N+1.validate_add_hosts_topology— the guard for the co-signing path.Two deliberate non-changes, both load-bearing:
The threshold does not move here. A new host does not count toward the threshold until its marker clears. Bundling the raise into this write would leave the party's active hosts below its own threshold mid-flight. Raising it is a separate serial bump.
The wallet pins the base serial. It compares what every host prepared byte-for-byte before it signs. Two hosts reading head state a moment apart would produce different transactions and fail that comparison for a reason that is not an attack — so a host whose view has moved on fails loudly instead.
The validator
Same reason the onboarding one exists: the host co-signs the caller's bytes with its own topology key. The stakes are higher here, because the party already exists and already holds contracts — a forged serial N+1 could evict its current hosts or drop its threshold rather than merely create something unwanted. Every field is checked against this node's own head-state read, never against anything the caller sent.
Refused: a serial that is not exactly one past current, a base serial that disagrees with head state, a dropped or altered current host, a new host without the marker, a new host at Submission or Observation, any threshold change, any signing-key change, a duplicate host, a mapping that does not host this node, a transaction that adds nobody, another party's mapping, and (via a struct-equality check) any field the per-field checks do not know about.
Scope
No HTTP endpoint — that is the tenant API v2 work, next. This is the builder and the guard, with 20 unit tests.
cargo clippy --all-targets --all-features -- -D warningsis clean. I have not runcargo testlocally per the usual rule, so CI is the first run of the new tests.