Daml: Add weight, kind to ValidatorLicense, and SV grant license support #2903

dfordivam · 2025-10-28T04:06:45Z

The daml version bump, and update of dars/package*.json is not in this PR to keep the changes easier to review. It would make sense to combine this PR with the one implementing governance of weight change, and then do the bump of the versions and update the release-notes.

meiersi-da · 2025-10-30T15:39:51Z

I'll review tomorrow morning my time.

meiersi-da

Nice work @dfordivam !

Functionally we're almost there, but we need a few more tests.

Also: we should talk about how to land this and it's follow-up changes best.

We are just around the 0.5.0 release from main, and I'd like to avoid releasing the Daml changes too early. Given that we don't yet have support for Daml snapshots (#514), I'd suggest we land all of your changes on a cip-73 feature branch and merge them once they are done. If you agree with that, then I'll create the branch; and I'll take care of shepherding it ultimately into main.

What I do care about is that we have a decent commit history. So I'd like this PR to also include the work of the version bumps, so it passes CI and can ultimately be merged into the cip-73 feature branch.

FYI: @isegall-da

daml/splice-amulet/daml/Splice/Issuance.daml

daml/splice-dso-governance-test/daml/Splice/Scripts/DsoTestUtils.daml

daml/splice-dso-governance-test/daml/Splice/Scripts/TestOnboarding.daml

daml/splice-dso-governance/daml/Splice/DsoRules.daml

daml/splice-amulet-test/daml/Splice/Scripts/TestValidatorFaucet.daml

daml/splice-amulet/daml/Splice/AmuletRules.daml

meiersi-da · 2025-10-31T08:51:17Z

daml/splice-dso-governance-test/daml/Splice/Scripts/TestOnboarding.daml

+  -- Test kind merging: Some OperatorLicense if any license has None or Some OperatorLicense
+  v3 <- allocateParty "v3"
+
+  -- Test case 1: mixing None and Some NonOperatorLicense -> should result in Some OperatorLicense


I'd suggest reorganizing the code so you can do this as a unit test:

define a function mergeValidatorLicenses : [ValidatorLicense] -> ValidatorLicense in Splice.ValidatorLicense

create a unit test in Splice.Scripts.UnitTests.ValidatorLicense in splice-amulet-test

Also consider testing a few of the constituent functions on their own to get a bit more succinct testing.

I did a split here 4936052, though there seems to be no need for a unit test in this case. (Unless I move the logic out from the DsoRules_MergeValidatorLicense and test the API separately?)

yeah, as I just commented in my review, I found it challenging to check your tests as there's so much setup noise for what is essentially a three line assertion.

Introducing mergeValidatorLicenseWeights and its friends and testing them on their own would remediate that.

daml/splice-dso-governance-test/daml/Splice/Scripts/TestOnboarding.daml

dfordivam · 2025-11-03T05:12:39Z

Making use of a cip-73 feature branch sounds good. But in order to make each PR pass CI, I will have to include fixes elsewhere in code (not just Daml). These fixes should be trivial to review for the daml changes we plan to do.

meiersi-da · 2025-11-03T08:56:32Z

Making use of a cip-73 feature branch sounds good. But in order to make each PR pass CI, I will have to include fixes elsewhere in code (not just Daml). These fixes should be trivial to review for the daml changes we plan to do.

Note that the branch is named feature-cip-73. You can find it here: https://github.com/hyperledger-labs/splice/tree/feature-cip-73

I do not mind the changes required to make a PR pass CI. Feel free to add them from the get-go.

What I care about is that we split the changes into: daml, backend, and frontend changes, as this limits the amount of change per PR. Each of these kinds of have their own tests, and can thus be validated on their own.

Where we do have separate pieces of functionality, then splitting them into separate sets of daml, backend, and frontend changes is also helpful to get more incremental progress and simplify reviewing (and often also development).

dfordivam · 2025-11-05T08:46:57Z

@meiersi-da I updated the PR and addressed the comments.
I have kept the fixes to the scala code at a minimal 16bb8c4, and they are not correct, but I hope that this should atleast get the CI green for the daml changes.

I have tried to make the bigger changes to the test files in multiple commits for ease of review, so it may be better to go over the changes commit by commit.

meiersi-da

Nice work. Not yet approving as I'd like to see the CI run results first.

daml/splice-amulet/daml/Splice/Issuance.daml

daml/splice-amulet/daml/Splice/ValidatorLicense.daml

daml/splice-dso-governance/daml/Splice/DsoRules.daml

meiersi-da · 2025-11-05T10:00:07Z

daml/splice-amulet-test/daml/Splice/Scripts/TestValidatorFaucet.daml

+-- With weight 0.0, Bob should receive no rewards
+test_ValidatorFaucetWithWeight0: Script ()
+test_ValidatorFaucetWithWeight0 = do
+    -- bob actually loses balance when doing minting with 0 weight. So just confirm bobExpectedMax


Consider adjusting the test infra to setup the app with no fees to simplify these tests.

I've added this kind of infra to the token standard testing:

splice/token-standard/splice-token-standard-test/daml/Splice/Testing/Registries/AmuletRegistry.daml

Lines 125 to 128 in 5689449

let amuletConfig

| config.noTransferFee = defaultAmuletConfig with

transferConfig = noTransferFeeConfig

| otherwise = defaultAmuletConfig

You could copy the same approach over to Splice.Scripts.Util.setupApp to make your tests more precise; and simplify later Daml changes.

I wouldn't recommend to switch all testing at once to using no-fees. Too onerous to do.

I have added such a setup in 2cb34b6 and used it in these tests.

daml/splice-amulet-test/daml/Splice/Scripts/TestValidatorFaucet.daml

daml/splice-dso-governance-test/daml/Splice/Scripts/TestValidatorLicense.daml

meiersi-da · 2025-11-05T11:02:09Z

daml/splice-dso-governance-test/daml/Splice/Scripts/TestValidatorLicense.daml

+    faucetState = Some FaucetState with
+      firstReceivedFor = Round 0
+      lastReceivedFor = Round 2
+      numCouponsMissed = 0


check that they are summed properly by setting them to a different non-zero amount

This field (or rather any field in the FaucetState) is not modified in the existing merge logic. And I don't see any reason to add such a functionality in this PR.

meiersi-da · 2025-11-05T13:41:19Z

daml/splice-dso-governance-test/daml/Splice/Scripts/TestValidatorLicense.daml

+  DsoRules_MergeValidatorLicenseResult mergedKind2 <- submitMulti [sv1] [dso] $ exerciseCmd rulesCid (DsoRules_MergeValidatorLicense [validatorLicenseK3, validatorLicenseK4] (Some sv1))
+  [(_, mergedLicense2)] <- queryFilter @ValidatorLicense dso (\license -> license.validator == v4)
+  -- Should be OperatorLicense to prevent downgrading
+  mergedLicense2.kind === Some OperatorLicense


I still can't shake the feeling that these tests use a lot of code for testing that a particular pure function behaves as it should. Consider refactoring the code so you can use unit tests as proposed in my previous review. Your call though.

Right, so I moved to code to pure function, in a separate file, and added unit-tests which are quite concise now.

meiersi-da

Very nice! Thanks you very much for your cleanup contributions. Much appreciated to have your help on this 🙇

meiersi-da · 2025-11-06T09:05:47Z

daml/dars.lock

+splice-util-featured-app-proxies-test 1.0.3 827f48e333c24b96ca7b900f0ca4b903a2b3d1c71ea99d5bb5a5f1fa1d2ecb53
 splice-util-token-standard-wallet 1.0.0 1da198cb7968fa478cfa12aba9fdf128a63a8af6ab284ea6be238cf92a3733ac
-splice-util-token-standard-wallet-test 1.0.0 ee328d4744bce339f040078d7271a3d73c932d23589cfa1cfbd99aaf6408c3c2
+splice-util-token-standard-wallet-test 1.0.0 90ddc14ec12a0e5b8bdfcf350fa9d2120b581c32c64caa23696cfe343c260c4c


missing version bump

meiersi-da · 2025-11-06T09:05:56Z

daml/dars.lock

 splice-util-featured-app-proxies 1.0.0 48e0c4fe4ea05e3b740404ebe37004ddd741efbdcd665c1c3199a5d6d9d944d7
 splice-util-featured-app-proxies 1.1.0 81dd5a9e5c02d0de03208522a895fb85eeb12fbea4aca7c4ad0ad106f3b0bfce
-splice-util-featured-app-proxies-test 1.0.3 115db68a3362e7e16754c147367c89f3a149bf8db199d4dcde91d416f9f894f4
+splice-util-featured-app-proxies-test 1.0.3 827f48e333c24b96ca7b900f0ca4b903a2b3d1c71ea99d5bb5a5f1fa1d2ecb53


missing version bump

meiersi-da · 2025-11-06T09:07:07Z

daml/splice-amulet-test/daml/Splice/Scripts/TestValidatorFaucet.daml

+        aliceExpectedMin = aliceExpectedMin
+        aliceExpectedMax = aliceExpectedMax
+        bobExpectedMin = aliceExpectedMin * bobWeight
+        bobExpectedMax = aliceExpectedMax * bobWeight


nice. Thanks you for this compactification!

meiersi-da · 2025-11-06T09:07:29Z

daml/splice-amulet-test/daml/Splice/Scripts/Util.daml

  return app

+-- | Setup the DSO party and the contracts defining the Amulet app with no fees.
+setupAppNoFees : Script AmuletApp


thanks a lot!

meiersi-da · 2025-11-06T09:09:25Z

daml/splice-dso-governance/daml/Splice/DsoRules/Utils.daml

+import Splice.ValidatorLicense
+import Splice.Types (Round(..))
+
+-- | Merges a list of `ValidatorLicense` contracts into a single license.


nice. thanks!

meiersi-da · 2025-11-06T13:04:37Z

@dfordivam : there's a bug to with our runner for CI on contributor forks. I've thus created a copy that I pushed here:

IGNORE: clone of Daml changes for validator weights #3093

Looking at the errors I see:

Found problems in the sbt output:
[info] daml/Splice/Scripts/UnitTests/MergeValidatorLicense.daml:test_ValidationError: ok, 0 active contracts, 0 transactions.
Total: 1 lines with problems.

==> rename the test. we have a simple grep for Error that is triggering this

Package (splice-util-featured-app-proxies-test,1.0.4) changed hash from 07a7177d917b3b405284d6433dfb81f6382d476bccf1380297dcd60f0edbf1b9 to ef633c6eff2d786cc571e58f572c5a0c72d31b7ffc1eeeedc420791c50b9a7f1

fix the version; run pre-commit hooks before push

meiersi-da · 2025-11-06T16:14:07Z

I've fixed the simple actions here:

af744e3 -- feel free to just cherry-pick it from my branch
this failure needs more attention -- seems to be a problem in your new test: https://github.com/hyperledger-labs/splice/actions/runs/19137267776/job/54704760978?pr=3093

Signed-off-by: Divam <[email protected]>

Just to make the code compile Signed-off-by: Divam <[email protected]>

Signed-off-by: Divam <[email protected]>

Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Divam <[email protected]>

Signed-off-by: Divam <[email protected]>

…alance change checks This has no transferFee and holdingFee, and makes it easier to confirm the cases where user's account balance change is not expected. Signed-off-by: Divam <[email protected]>

… code reuse. Signed-off-by: Divam <[email protected]>

Signed-off-by: Divam <[email protected]>

[ci] Signed-off-by: Simon Meier <[email protected]>

Signed-off-by: Divam <[email protected]>

dfordivam · 2025-11-07T07:45:39Z

I have rebased the branch over latest feature-cip-73, cherry-picked the af744e3 commit, and did the fix for test failure here 1274d07

PR reviewed in hyperledger-labs#2903 CI run hyperledger-labs#3093 Squash commit created by rebasing 5e8d90e7f over f47f017 Signed-off-by: Divam <[email protected]>

dfordivam · 2025-11-12T07:50:19Z

The commits reviewed in this PR have been squashed and pushed here obsidiansystems@c9296bf

dfordivam had a problem deploying to ci-forks October 28, 2025 04:06 — with GitHub Actions Error

github-actions bot assigned isegall-da, martinflorian-da and ray-roestenburg-da Oct 28, 2025

dfordivam force-pushed the dn/cip-0073-daml-weighted-validator-license branch from 99fa074 to 373e496 Compare October 28, 2025 04:19

dfordivam had a problem deploying to ci-forks October 28, 2025 04:19 — with GitHub Actions Error

dfordivam mentioned this pull request Oct 28, 2025

Support for granting ValidatorLicense via SV UI #2905

Open

6 tasks

dfordivam force-pushed the dn/cip-0073-daml-weighted-validator-license branch from 373e496 to 01384d1 Compare October 30, 2025 04:33

dfordivam had a problem deploying to ci-forks October 30, 2025 04:33 — with GitHub Actions Error

meiersi-da reviewed Oct 31, 2025

View reviewed changes

dfordivam mentioned this pull request Oct 31, 2025

Daml: Enhance DsoRules to allow weight change and withdrawal of ValidatorLicense #3011

Draft

martinflorian-da assigned meiersi-da and unassigned ray-roestenburg-da, isegall-da and martinflorian-da Nov 3, 2025

dfordivam force-pushed the dn/cip-0073-daml-weighted-validator-license branch from 01384d1 to 1cab7be Compare November 5, 2025 08:23

dfordivam deployed to ci-forks November 5, 2025 08:23 — with GitHub Actions Active

dfordivam changed the base branch from main to feature-cip-73 November 5, 2025 08:24

meiersi-da reviewed Nov 5, 2025

View reviewed changes

dfordivam force-pushed the dn/cip-0073-daml-weighted-validator-license branch from 7b810d0 to 020c441 Compare November 6, 2025 08:32

meiersi-da approved these changes Nov 6, 2025

View reviewed changes

meiersi-da mentioned this pull request Nov 6, 2025

IGNORE: clone of Daml changes for validator weights #3093

Draft

6 tasks

dfordivam added 2 commits November 7, 2025 06:17

Add weight, kind to ValidatorLicense

26b627c

Signed-off-by: Divam <[email protected]>

DsoRules_GrantValidatorLicense

f0e076e

Signed-off-by: Divam <[email protected]>

dfordivam and others added 24 commits November 7, 2025 06:17

Add tests for confirming runNextIssuance(D) APIs use license weights

28a1450

Signed-off-by: Divam <[email protected]>

Split test_MergeValidatorLicense into multiple smaller tests

2835562

Signed-off-by: Divam <[email protected]>

Add a note about merging of weights

d1bfb93

Signed-off-by: Divam <[email protected]>

Bump daml.yaml package versions

70fd439

Signed-off-by: Divam <[email protected]>

Add new dars

8099e01

Signed-off-by: Divam <[email protected]>

Add new dars in DarResources.scala

45811e6

Signed-off-by: Divam <[email protected]>

Bump versions in package.json

dfc5590

Signed-off-by: Divam <[email protected]>

Code fixes for daml changes

45ef18f

Just to make the code compile Signed-off-by: Divam <[email protected]>

Update apps/package-lock.json

395c3ff

Signed-off-by: Divam <[email protected]>

Update dars.lock

9e366de

Signed-off-by: Divam <[email protected]>

Doc comment update

53ee5d1

Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Divam <[email protected]>

Doc comment update

872e0ca

Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Divam <[email protected]>

Refactor hasOperatorLicense

c1de1cb

Co-authored-by: Simon Meier <[email protected]> Signed-off-by: Divam <[email protected]>

Remove redundant import

dd600c0

Signed-off-by: Divam <[email protected]>

Move merge ValidatorLicense logic to pure API and use unit tests

88741ca

Signed-off-by: Divam <[email protected]>

Refactor, use Total.minimumOn

64688e5

Signed-off-by: Divam <[email protected]>

Add setupDefaultAppWithUsersNoFees for tests requiring more precise b…

2689d2d

…alance change checks This has no transferFee and holdingFee, and makes it easier to confirm the cases where user's account balance change is not expected. Signed-off-by: Divam <[email protected]>

Refactor ValidatorLicense weight tests to use no-fees setup, and more…

f332f9d

… code reuse. Signed-off-by: Divam <[email protected]>

Update daml/dars

06f587a

Signed-off-by: Divam <[email protected]>

Update daml/dars.lock

dc4985a

Signed-off-by: Divam <[email protected]>

Bump daml version

3c1bc2a

Signed-off-by: Divam <[email protected]>

Update daml/dars.lock

9e053fd

Signed-off-by: Divam <[email protected]>

fix CI errors

ee324c2

[ci] Signed-off-by: Simon Meier <[email protected]>

Upload dars in batches to avoid hitting gRPC limit

1274d07

Signed-off-by: Divam <[email protected]>

dfordivam force-pushed the dn/cip-0073-daml-weighted-validator-license branch from eede310 to af2cc92 Compare November 7, 2025 07:41

Update dars.lock

389dce7

Signed-off-by: Divam <[email protected]>

dfordivam force-pushed the dn/cip-0073-daml-weighted-validator-license branch from af2cc92 to 389dce7 Compare November 7, 2025 07:43

	let amuletConfig
	\| config.noTransferFee = defaultAmuletConfig with
	transferConfig = noTransferFeeConfig
	\| otherwise = defaultAmuletConfig

Daml: Add weight, kind to ValidatorLicense, and SV grant license support #2903

Are you sure you want to change the base?

Daml: Add weight, kind to ValidatorLicense, and SV grant license support #2903

Uh oh!

Conversation

dfordivam commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meiersi-da commented Oct 30, 2025

Uh oh!

meiersi-da left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dfordivam commented Nov 3, 2025

Uh oh!

meiersi-da commented Nov 3, 2025

Uh oh!

dfordivam commented Nov 5, 2025

Uh oh!

meiersi-da left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

meiersi-da left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

meiersi-da commented Nov 6, 2025

Uh oh!

meiersi-da commented Nov 6, 2025

Uh oh!

dfordivam commented Nov 7, 2025

Uh oh!

dfordivam commented Nov 12, 2025

Uh oh!

Reviewers

dfordivam commented Oct 28, 2025 •

edited

Loading