Support NUCK in daml-script by samuel-williams-da · Pull Request #22830 · digital-asset/daml

samuel-williams-da · 2026-03-25T16:29:20Z

Needs canton with changes from this PR to pass
New script primitive awaiting naming discussion

paulbrauner-da

Nice! Shorter than I expected.

paulbrauner-da · 2026-03-25T20:52:00Z

sdk/compiler/script-service/client/src/DA/Daml/LF/ScriptServiceClient/LowLevel.hs

+            -- If we get an error about --ide-ledger-protocol-version, forward it by itself, as we're depending
+            -- on the script service for parsing errors to avoid duplication of protocol versions
+            -- We clean up the error a bit to re-point it to script-service.protocol-version


Wouldn't it be simpler to validate the pv on the client side and thus assume that the script service will always parse it? This would remove this complex parsing logic and we could just printStderr as before. I realize that's some code duplication between the client and the server that could get out of sync, but we could maybe add a test to prevent this.

The issue is its one more place we need to update the latest protocol version as well, but this approach its a "one and done"

sdk/compiler/script-service/client/src/DA/Daml/LF/ScriptServiceClient.hs

paulbrauner-da · 2026-03-25T21:02:57Z

sdk/daml-script/daml/Daml/Script/Internal/Questions/Query.daml

+--
+-- This is semantically equivalent to calling `query`
+-- and filtering on the client side.
+queryNContractKey : forall t k p. HasCallStack => (TemplateKey t k, IsParties p) => p -> k -> Int -> Script [(ContractId t, t)]


As discussed, we need to think about the naming. The name you've chosen doesn't clash with queryNByKey because there's no "by" (also I'm not sure the other one has N in the name). But it's pretty close. On the other hand, this is consistent with the queryContractKey name that we can't change because GS is using it already.

@remyhaemmerle-da, any opinion?

paulbrauner-da · 2026-03-25T21:07:16Z

sdk/daml-script/daml/Daml/Script/Internal/Questions/Query.daml

+  parties : [Party]
+  tplId : TemplateTypeRep
+  contractKey : AnyContractKey
+  limit : Int


Since we're anyway fetching everything and because this is only meant to be used for testing, I wonder if we need the limit. Let's discuss tomorrow.

paulbrauner-da · 2026-03-25T21:09:40Z

sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala

      v: ExtendedValue,
      lookupContractKeyType: Identifier => Either[String, Type],
-      legacyAnyContractKey: Boolean,
+      legacyAnyContractKey: Boolean = false,


What introduces the need to assign it a default value?

The parser for queryNContractKeys never needs legacyAnyContractKey, so I omit the argument.

samuel-williams-da · 2026-03-29T16:31:58Z

sdk/daml-script/test/daml/MultiTestWithKeys.daml

This is the original test file, renamed to WithKeys.
The MultiTest is a copy of this with all key tests removed

samuel-williams-da · 2026-03-29T16:32:22Z

sdk/daml-script/test/daml/ScriptTestWithKeys.daml

This is the original test file, renamed to WithKeys.
The ScriptTest is a copy of this with all key tests removed

samuel-williams-da · 2026-03-29T16:32:56Z

...st/src/main/scala/com/digitalasset/daml/lf/engine/script/test/DamlScriptTestRunnerPV35.scala

Single "Stable" is actually latest PV, which is currently PV34 (no key support)
I've added a test specifically for PV35

samuel-williams-da · 2026-03-29T16:33:44Z

sdk/compiler/damlc/tests/daml-test-files/ExceptionSemanticsWithKeys.daml

+-- TODO:
+-- Exception semantics with keys have changed significantly, this test needs to be re-thought.
+-- Consider cases like fetch/queryby keys in a rollback, as well as non-consuming exercise by key
+


samuel-williams-da · 2026-03-29T16:34:53Z

sdk/compiler/damlc/tests/daml-test-files/LfDevMustFailCommit.daml

This tested mustFail using duplicate keys. I've deleted it for now, perhaps we should add back a version that fails using a different error.

samuel-williams-da · 2026-03-29T16:35:12Z

sdk/compiler/damlc/tests/daml-test-files/LfStableExceptionAuth.daml

 {-# OPTIONS_GHC -Wno-x-exceptions #-}

+-- @IDE-PV V34
+


We might want a version of this file for PV35, that avoids rollbacks

samuel-williams-da · 2026-03-29T16:35:59Z

sdk/compiler/damlc/tests/src/DA/Test/DamlcIntegration.hs

All changes here are to allow different PVs in the IDE ledger per test.
We need a separate script service per PV, as we do for build options (which Jasper added some time back). I've extended his logic to also account for PV

samuel-williams-da · 2026-03-29T16:39:47Z

sdk/daml-script/test/daml/upgrades/stable/Exceptions.daml.disabled

 -- Copyright (c) 2025 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
 -- SPDX-License-Identifier: Apache-2.0

+-- TODO: This test is disabled as it requires PV34, refactoring the upgrade test suite to allow PV34 would be too much effort for now


See here, unsure what we want to do. Do we want the suite to cover PV34?

samuel-williams-da · 2026-03-29T16:40:43Z

...rc/main/scala/com/digitalasset/daml/lf/engine/script/test/DamlScriptTestRunnerPVLatest.scala

Diff is being bad here, I didn't delete these tests, they're just in PV34 now.

samuel-williams-da · 2026-03-29T16:41:28Z

sdk/docs/BUILD.bazel

+# Disabled as template uses keys and rollbacks, which is no longer permitted
+# TODO: Remove/refactor this template and re-enable the test


samuel-williams-da · 2026-03-29T16:41:53Z

sdk/test-common/canton/it-lib/src/main/scala/com/daml/integrationtest/CantonConfig.scala

    jarPath: Path = CantonRunner.cantonPath,
    authSecret: Option[String] = None,
-    devMode: Boolean = false,
+    protocolVersion: String = "latest",


Maybe its worth changing the string to a trait, containing say "dev", "latest" and "explicit(ver: String)"?

samuel-williams-da · 2026-03-29T16:42:20Z

sdk/security-evidence.md

Now every test in our security evidence is disabled 🙃

github-actions bot added the Standard-Change label Mar 25, 2026

samuel-williams-da requested a review from paulbrauner-da March 25, 2026 17:35

paulbrauner-da reviewed Mar 25, 2026

View reviewed changes

samuel-williams-da added 4 commits March 27, 2026 18:07

Support NUCK in daml-script

bc99f3b

Updates from canton changes

708e825

Update to canton with ide ledger NUCK

7f3c830

Various test fixes, waiting for patch in canton

a5249e1

samuel-williams-da force-pushed the sw/nuck-daml-script-support branch from 562cb6d to a5249e1 Compare March 27, 2026 18:27

samuel-williams-da added 5 commits March 27, 2026 19:22

Fix attestation test

28cb42d

Bump canton to fixed IDE

9af2ada

Batch of fixes, has PV34 error

7e8b092

Sync docs

1d6c4fc

More test fixes

906ba8f

samuel-williams-da commented Mar 29, 2026

View reviewed changes

sdk/security-evidence.md

Copy link
Copy Markdown

Contributor Author

samuel-williams-da Mar 29, 2026

Choose a reason for hiding this comment

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

Now every test in our security evidence is disabled 🙃

samuel-williams-da added 4 commits March 29, 2026 17:44

Self review and fixes

efabd09

Update unreleased

34a5ebc

Change remainder of script-test-2.ðev.dar uses

516eed2

Fix locations in test

ec1eef5

		# Disabled as template uses keys and rollbacks, which is no longer permitted
		# TODO: Remove/refactor this template and re-enable the test

Conversation

samuel-williams-da commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paulbrauner-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!

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

samuel-williams-da commented Mar 25, 2026 •

edited

Loading