feat: set a per-synchronizer traffic discount by vote - #35
Open
salindne wants to merge 13 commits into
Open
Conversation
RegisteredSynchronizer gains an optional discount factor in (0, 1], bounded by the template ensure and read through getDiscountFactor, which returns 1.0 when it is absent. A RegisteredSynchronizer_SetDiscount choice replaces the value and stores None for the default. Nothing reads the factor yet. The field is Optional because absence has to read as 1.0, and is appended last by convention for a template heading into a release line. No released package contains RegisteredSynchronizer yet, so no upgrade check constrains it today. Daml record fields are strict, so every construction site names the new field. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
SRARC_SetSynchronizerDiscount pins the registration, fetch-checks it against the DSO party and exercises the template choice, the same shape as SRARC_UpdateFeaturedAppRight. The constructor is appended last in the variant because LF encodes constructor ranks, so inserting one breaks upgrade compatibility with the released package. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
Five assertions: absence reads as 1.0, a vote sets the factor, a second vote replaces it rather than adding a registration, a vote of the default stores None, and a factor outside (0, 1] is refused by the ensure. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
The template ensure now calls getDiscountFactor, which gives the accessor its only non-test caller and removes the duplicated fromOptional expression. The registration test reuses onlyRegistration rather than keeping the identical query-and-abort block above it, and drops an assertion that only exercised fromOptional on a Some. The new variant arm gains the PoC prefix its siblings carry. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
The wall-clock shard passed all 16 of its tests; the job failed on the log check over two lines from one event, a 120s gRPC deadline with a name-resolution failure reaching DefaultSequencer-0 and the resulting "failed to acknowledge clean timestamp (usually because sequencer is down)". Neither is in the wall-clock ignore patterns, and 1124 other lines were ignored in the same job. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
The new definitions sat between the block comment and the template it describes, so its closing "This template lives in splice-amulet" pointed at the wrong thing. Moved them above the comment, matching how DedicatedSynchronizerState_ReportActivityResult sits ahead of its own comment and template in this file. Reordering top-level definitions changes the package id, hence the regenerated artefacts. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
The merge commit carried no [ci] token, so the opt-in gate cancelled the whole run. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
| operator : Party | ||
| -- ^ The party operating the dedicated synchronizer. Observes this registration and every | ||
| -- traffic purchase for its synchronizer (via the `operator` field on `MemberTraffic`). | ||
| discountFactor : Optional Decimal |
There was a problem hiding this comment.
I would suggest to slightly refactor this into:
governanceParameters : Optional GovernanceParameters
data GovernanceParameters = GovernanceParameters
with
discountFactor : Decimal
choice RegisteerdSynchronizer_SetGovernanceParameters
The advance of that is that adding more parameters that SVs can vote on becomes trivial as you don't need one choice per parameter.
Collaborator
Author
There was a problem hiding this comment.
okay thank you, have updated
Review feedback: a record of votable parameters behind one setter, so a further parameter costs no new choice, result record, vote action or dispatch arm. It also matches how the rest of governance works, since AmuletRules_SetConfig and DsoRules_SetConfig both take a whole record behind a single action. The setter now always writes the block, so voting the discount back to 1.0 leaves Some (GovernanceParameters with discountFactor = 1.0) rather than clearing the field. Normalising back to None stops being well defined once the block holds more than one parameter, and None now means nothing has been voted yet. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
moritzkiefer-da
approved these changes
Sep 10, 2026
moritzkiefer-da
left a comment
moritzkiefer-da
left a comment
There was a problem hiding this comment.
thanks! ci doesn't look happy
DsoRules never names GovernanceParameters itself, only the choice and result types, so importing it warned and CI fails any Daml job on a DsWarning. That is what broke all 18 jobs on the last run. Local damlTest missed it because the incremental build had not recompiled splice-dso-governance; rebuilt from cleared .daml/dist to confirm the tree is warning-free. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
simtime shard 2 failed its log check on 35 lines of "Consensus not reached." from SvTimeBasedRoundMgmtIntegrationTest, with all 13 tests passing. The ignore file whitelists that message for three other suites but not this one, pending upstream canton-network#6392. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
wall-clock shard 0 failed 16 of 19 tests at fixture construction with "Could not create Prometheus HTTP server", so no test reached the change. Fifth run, fifth distinct runner problem. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
Third time on this shard, third SvTimeBased suite, all 13 tests passing. Raised with DA; the ignore pattern covers three other suites but not this family, pending upstream canton-network#6392. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
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
2 participants
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.
Closes ChainSafe/canton-extending-mainnet#91
Summary:
RegisteredSynchronizercarries an optional traffic discount factor in (0, 1], bounded by the templateensureand read as 1.0 when absent, and a newSRARC_SetSynchronizerDiscountvote action sets or replaces it. Nothing reads the factor yet; applying it at the buy is canton-network#92.Verified: dropping the bound from the
ensurereddens the out-of-bounds script, and dropping theNone-normalisation reddens the vote script.