From a72d16aa1eed7824ecaf7ec04d60fec0273b030d Mon Sep 17 00:00:00 2001 From: Remy Haemmerle Date: Thu, 12 Dec 2024 14:39:49 +0100 Subject: [PATCH 01/13] wip --- .../daml/lf/engine/ValueEnricher.scala | 59 +++++++++++++------ .../preprocessing/CommandPreprocessor.scala | 36 ++++++----- .../engine/preprocessing/Preprocessor.scala | 4 +- .../preprocessing/ValueTranslator.scala | 10 +++- .../daml/lf/engine/PreprocessorSpec.scala | 2 + .../daml/lf/engine/ValueEnricherSpec.scala | 2 +- .../digitalasset/daml/lf/ScenarioRunner.scala | 4 +- 7 files changed, 77 insertions(+), 40 deletions(-) diff --git a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ValueEnricher.scala b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ValueEnricher.scala index 4c2d34fdedda..d80b5bed56a9 100644 --- a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ValueEnricher.scala +++ b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ValueEnricher.scala @@ -28,7 +28,7 @@ import com.daml.lf.speedy.SValue final class ValueEnricher( compiledPackages: CompiledPackages, - translateValue: (Ast.Type, Value) => Result[SValue], + translateValue: (Ast.Type, Boolean, Value) => Result[SValue], loadPackage: (PackageId, language.Reference) => Result[Unit], ) { @@ -39,15 +39,16 @@ final class ValueEnricher( engine.loadPackage, ) - def enrichValue(typ: Ast.Type, value: Value): Result[Value] = - translateValue(typ, value).map(_.toUnnormalizedValue) + def enrichValue(typ: Ast.Type, upgradable: Boolean, value: Value): Result[Value] = + translateValue(typ, upgradable, value).map(_.toUnnormalizedValue) def enrichVersionedValue( typ: Ast.Type, + upgradable: Boolean, versionedValue: VersionedValue, ): Result[VersionedValue] = for { - value <- enrichValue(typ, versionedValue.unversioned) + value <- enrichValue(typ, upgradable, versionedValue.unversioned) } yield versionedValue.map(_ => value) def enrichContract( @@ -61,17 +62,27 @@ final class ValueEnricher( contract: Value.VersionedContractInstance ): Result[Value.VersionedContractInstance] = for { - arg <- enrichValue(Ast.TTyCon(contract.unversioned.template), contract.unversioned.arg) + pkg <- handleLookup( + compiledPackages.pkgInterface.lookupPackage(contract.unversioned.template.packageId) + ) + arg <- enrichValue( + Ast.TTyCon(contract.unversioned.template), + pkg.upgradable, + contract.unversioned.arg, + ) } yield contract.map(_.copy(arg = arg)) def enrichView( interfaceId: Identifier, viewValue: Value, ): Result[Value] = for { + pkg <- handleLookup( + compiledPackages.pkgInterface.lookupPackage(interfaceId.packageId) + ) iface <- handleLookup( compiledPackages.pkgInterface.lookupInterface(interfaceId) ) - r <- enrichValue(iface.view, viewValue) + r <- enrichValue(iface.view, pkg.upgradable, viewValue) } yield r def enrichVersionedView( @@ -82,7 +93,12 @@ final class ValueEnricher( } yield viewValue.copy(unversioned = view) def enrichContract(tyCon: Identifier, value: Value): Result[Value] = - enrichValue(Ast.TTyCon(tyCon), value) + for { + pkg <- handleLookup( + compiledPackages.pkgInterface.lookupPackage(tyCon.packageId) + ) + enrichedValue <- enrichValue(Ast.TTyCon(tyCon), pkg.upgradable, value) + } yield enrichedValue private[this] def pkgInterface = compiledPackages.pkgInterface @@ -106,15 +122,19 @@ final class ValueEnricher( interfaceId: Option[Identifier], choiceName: Name, value: Value, - ): Result[Value] = - handleLookup( + ): Result[Value] = for { + choice <- handleLookup( pkgInterface.lookupChoice( Identifier(choicePackageId, qualifiedTemplateName), interfaceId, choiceName, ) ) - .flatMap(choice => enrichValue(choice.argBinder._2, value)) + pkg <- handleLookup( + compiledPackages.pkgInterface.lookupPackage(choicePackageId) + ) + enrichedValue <- enrichValue(choice.argBinder._2, pkg.upgradable, value) + } yield enrichedValue // Deprecated def enrichChoiceArgument( @@ -137,16 +157,17 @@ final class ValueEnricher( interfaceId: Option[Identifier], choiceName: Name, value: Value, - ): Result[Value] = { - handleLookup( + ): Result[Value] = for { + choice <- handleLookup( pkgInterface.lookupChoice( Identifier(choicePackageId, qualifiedTemplateName), interfaceId, choiceName, ) ) - .flatMap(choice => enrichValue(choice.returnType, value)) - } + pkg <- handleLookup(pkgInterface.lookupPackage(choicePackageId)) + enrichedValue <- enrichValue(choice.returnType, pkg.upgradable, value) + } yield enrichedValue // Deprecated def enrichChoiceResult( @@ -163,8 +184,11 @@ final class ValueEnricher( ) def enrichContractKey(tyCon: Identifier, value: Value): Result[Value] = - handleLookup(pkgInterface.lookupTemplateKey(tyCon)) - .flatMap(key => enrichValue(key.typ, value)) + for { + key <- handleLookup(pkgInterface.lookupTemplateKey(tyCon)) + pkg <- handleLookup(pkgInterface.lookupPackage(tyCon.packageId)) + enrichedValue <- enrichValue(key.typ, pkg.upgradable, value) + } yield enrichedValue private val ResultNone = ResultDone(None) @@ -196,7 +220,8 @@ final class ValueEnricher( ResultDone(rb) case create: Node.Create => for { - arg <- enrichValue(Ast.TTyCon(create.templateId), create.arg) + pkg <- handleLookup(pkgInterface.lookupPackage(create.templateId.packageId)) + arg <- enrichValue(Ast.TTyCon(create.templateId), pkg.upgradable, create.arg) key <- enrichContractKey(create.keyOpt, create.version, create.packageName) } yield create.copy(arg = arg, keyOpt = key) case fetch: Node.Fetch => diff --git a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/CommandPreprocessor.scala b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/CommandPreprocessor.scala index 8b9587cdbea1..fa5b09244650 100644 --- a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/CommandPreprocessor.scala +++ b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/CommandPreprocessor.scala @@ -30,13 +30,14 @@ private[lf] final class CommandPreprocessor( private[this] def translateArg( typ: Ast.Type, + upgradable: Boolean, value: Value, ) = - valueTranslator.unsafeTranslateValue(ty = typ, value = value) + valueTranslator.unsafeTranslateValue(ty = typ, upgradable = upgradable, value = value) // This is used by value enricher - def unsafeTranslateValue(typ: Ast.Type, value: Value) = - valueTranslator.unsafeTranslateValue(typ, value) + def unsafeTranslateValue(typ: Ast.Type, upgradable: Boolean, value: Value) = + valueTranslator.unsafeTranslateValue(typ, upgradable = upgradable, value) @throws[Error.Preprocessing.Error] def unsafePreprocessDisclosedContract( @@ -59,7 +60,7 @@ private[lf] final class CommandPreprocessor( } // TODO: https://github.com/digital-asset/daml/issues/17082 // for now we need the package of the disclosed contract - val arg = translateArg(Ast.TTyCon(disc.templateId), disc.argument) + val arg = translateArg(Ast.TTyCon(disc.templateId), upgradable(disc.templateId), disc.argument) validateCid(disc.contractId) speedy.DisclosedContract( templateId = disc.templateId, @@ -72,7 +73,7 @@ private[lf] final class CommandPreprocessor( @throws[Error.Preprocessing.Error] def unsafePreprocessCreate(templateId: Ref.Identifier, argument: Value): speedy.Command.Create = { discard(handleLookup(pkgInterface.lookupTemplate(templateId))) - val arg = translateArg(Ast.TTyCon(templateId), argument) + val arg = translateArg(Ast.TTyCon(templateId), upgradable(templateId), argument) speedy.Command.Create(templateId, arg) } @@ -100,7 +101,7 @@ private[lf] final class CommandPreprocessor( templateId = templateId, contractId = valueTranslator.unsafeTranslateCid(contractId), choiceId = choiceId, - argument = translateArg(choice.argBinder._2, argument), + argument = translateArg(choice.argBinder._2, upgradable(templateId), argument), ) } @@ -115,7 +116,7 @@ private[lf] final class CommandPreprocessor( interfaceId = ifaceId, contractId = valueTranslator.unsafeTranslateCid(contractId), choiceId = choiceId, - argument = translateArg(choice.argBinder._2, argument), + argument = translateArg(choice.argBinder._2, upgradable(ifaceId), argument), ) } @@ -129,9 +130,10 @@ private[lf] final class CommandPreprocessor( val choiceArgType = handleLookup( pkgInterface.lookupTemplateChoice(templateId, choiceId) ).argBinder._2 + val upgradable = this.upgradable(templateId) val ckTtype = handleLookup(pkgInterface.lookupTemplateKey(templateId)).typ - val arg = translateArg(choiceArgType, argument) - val key = translateArg(ckTtype, contractKey) + val arg = translateArg(choiceArgType, upgradable, argument) + val key = translateArg(ckTtype, upgradable, contractKey) speedy.Command.ExerciseByKey(templateId, key, choiceId, arg) } @@ -142,11 +144,12 @@ private[lf] final class CommandPreprocessor( choiceId: Ref.ChoiceName, choiceArgument: Value, ): speedy.Command.CreateAndExercise = { - val createArg = translateArg(Ast.TTyCon(templateId), createArgument) + val upgradable = this.upgradable(templateId) + val createArg = translateArg(Ast.TTyCon(templateId), upgradable, createArgument) val choiceArgType = handleLookup( pkgInterface.lookupTemplateChoice(templateId, choiceId) ).argBinder._2 - val choiceArg = translateArg(choiceArgType, choiceArgument) + val choiceArg = translateArg(choiceArgType, upgradable, choiceArgument) speedy.Command.CreateAndExercise(templateId, createArg, choiceId, choiceArg) } @@ -156,7 +159,7 @@ private[lf] final class CommandPreprocessor( contractKey: Value, ): speedy.Command.LookupByKey = { val ckTtype = handleLookup(pkgInterface.lookupTemplateKey(templateId)).typ - val key = translateArg(ckTtype, contractKey) + val key = translateArg(ckTtype, upgradable(templateId), contractKey) speedy.Command.LookupByKey(templateId, key) } @@ -277,11 +280,11 @@ private[lf] final class CommandPreprocessor( } case command.ReplayCommand.FetchByKey(templateId, key) => val ckTtype = handleLookup(pkgInterface.lookupTemplateKey(templateId)).typ - val sKey = translateArg(ckTtype, key) + val sKey = translateArg(ckTtype, upgradable(templateId), key) speedy.Command.FetchByKey(templateId, sKey) case command.ReplayCommand.LookupByKey(templateId, key) => val ckTtype = handleLookup(pkgInterface.lookupTemplateKey(templateId)).typ - val sKey = translateArg(ckTtype, key) + val sKey = translateArg(ckTtype, upgradable(templateId), key) speedy.Command.LookupByKey(templateId, sKey) } @@ -323,7 +326,7 @@ private[lf] final class CommandPreprocessor( discard(handleLookup(pkgInterface.lookupInterface(interfaceId))) discard(handleLookup(pkgInterface.lookupInterfaceInstance(interfaceId, templateId))) - val arg = translateArg(Ast.TTyCon(templateId), argument) + val arg = translateArg(Ast.TTyCon(templateId), upgradable(templateId), argument) speedy.InterfaceView(templateId, arg, interfaceId) } @@ -362,4 +365,7 @@ private[lf] final class CommandPreprocessor( ) } + def upgradable(tyCon: Ref.TypeConName): Boolean = + handleLookup(pkgInterface.lookupPackage(tyCon.packageId)).upgradable + } diff --git a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/Preprocessor.scala b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/Preprocessor.scala index a4cd5ec36154..7eed413b1783 100644 --- a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/Preprocessor.scala +++ b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/Preprocessor.scala @@ -158,10 +158,10 @@ private[engine] final class Preprocessor( * Fails if the nesting is too deep or if v0 does not match the type `ty0`. * Assumes ty0 is a well-formed serializable typ. */ - def translateValue(ty0: Ast.Type, v0: Value): Result[SValue] = + def translateValue(ty0: Ast.Type, upgradable: Boolean, v0: Value): Result[SValue] = safelyRun(pullTypePackages(ty0)) { // this is used only by the value enricher - commandPreprocessor.unsafeTranslateValue(ty0, v0) + commandPreprocessor.unsafeTranslateValue(ty0, upgradable, v0) } private[engine] def preprocessApiCommand( diff --git a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala index 8c6969a57640..c2474d70a066 100644 --- a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala +++ b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala @@ -62,6 +62,7 @@ private[lf] final class ValueTranslator( @throws[Error.Preprocessing.Error] private[preprocessing] def unsafeTranslateValue( ty: Type, + upgradable: Boolean, value: Value, ): SValue = { // TODO: https://github.com/digital-asset/daml/issues/17082 @@ -178,7 +179,6 @@ private[lf] final class ValueTranslator( typeError() } case TTyCon(tyCon) => - val upgradable = handleLookup(pkgInterface.lookupPackage(tyCon.packageId)).upgradable value0 match { // variant case ValueVariant(mbId, constructorName, val0) => @@ -361,9 +361,13 @@ private[lf] final class ValueTranslator( go(ty, value) } - def translateValue(ty: Type, value: Value): Either[Error.Preprocessing.Error, SValue] = + def translateValue( + ty: Type, + upgradable: Boolean, + value: Value, + ): Either[Error.Preprocessing.Error, SValue] = safelyRun( - unsafeTranslateValue(ty, value) + unsafeTranslateValue(ty, upgradable, value) ) } diff --git a/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/PreprocessorSpec.scala b/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/PreprocessorSpec.scala index 5b0963318ece..0dbe3c7c21b8 100644 --- a/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/PreprocessorSpec.scala +++ b/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/PreprocessorSpec.scala @@ -51,6 +51,7 @@ class PreprocessorSpec(majorLanguageVersion: LanguageMajorVersion) val intermediaryResult = preprocessor .translateValue( Ast.TTyCon("Mod:WithoutKey"), + upgradable = true, ValueRecord("", ImmArray("owners" -> parties, "data" -> ValueInt64(42))), ) intermediaryResult shouldBe a[ResultNeedPackage[_]] @@ -63,6 +64,7 @@ class PreprocessorSpec(majorLanguageVersion: LanguageMajorVersion) val intermediaryResult = preprocessor .translateValue( Ast.TTyCon("Mod:WithoutKey"), + upgradable = true, ValueRecord( "", ImmArray("owners" -> parties, "wrong_field" -> ValueInt64(42)), diff --git a/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/ValueEnricherSpec.scala b/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/ValueEnricherSpec.scala index 008369d2a0de..a14e22b72c65 100644 --- a/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/ValueEnricherSpec.scala +++ b/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/ValueEnricherSpec.scala @@ -154,7 +154,7 @@ class ValueEnricherSpec(majorLanguageVersion: LanguageMajorVersion) "enrich values as expected" in { forEvery(testCases) { (typ, input, output) => - enricher.enrichValue(typ, input) shouldBe ResultDone(output) + enricher.enrichValue(typ, upgradable = true, input) shouldBe ResultDone(output) } } } diff --git a/sdk/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/ScenarioRunner.scala b/sdk/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/ScenarioRunner.scala index a7c716290791..ee94f7afb453 100644 --- a/sdk/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/ScenarioRunner.scala +++ b/sdk/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/ScenarioRunner.scala @@ -403,8 +403,8 @@ private[lf] object ScenarioRunner { pkgInterface = compiledPackages.pkgInterface, checkV1ContractIdSuffixes = config.requireSuffixedGlobalContractId, ) - def translateValue(typ: Ast.Type, value: Value): Result[SValue] = - valueTranslator.translateValue(typ, value) match { + def translateValue(typ: Ast.Type, upgradable: Boolean, value: Value): Result[SValue] = + valueTranslator.translateValue(typ, upgradable, value) match { case Left(err) => ResultError(err) case Right(sv) => ResultDone(sv) } From d3e818b00306b455a2444b2d069760ee7aa7080a Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Thu, 12 Dec 2024 17:05:12 +0000 Subject: [PATCH 02/13] Much value translator plumbing --- .../preprocessing/CommandPreprocessor.scala | 2 +- .../preprocessing/ValueTranslator.scala | 2 +- .../daml/lf/engine/script/Converter.scala | 22 +++++++++++++++---- .../daml/lf/engine/script/v1/Converter.scala | 3 ++- .../daml/lf/engine/script/v1/ScriptF.scala | 10 ++++----- .../ledgerinteraction/IdeLedgerClient.scala | 4 +++- .../daml/lf/engine/script/v2/Converter.scala | 3 ++- .../daml/lf/engine/script/v2/ScriptF.scala | 10 +++++---- .../ledgerinteraction/IdeLedgerClient.scala | 4 +++- .../v2/ledgerinteraction/SubmitError.scala | 4 ++-- .../daml/lf/engine/trigger/Converter.scala | 2 +- 11 files changed, 43 insertions(+), 23 deletions(-) diff --git a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/CommandPreprocessor.scala b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/CommandPreprocessor.scala index fa5b09244650..d241162af180 100644 --- a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/CommandPreprocessor.scala +++ b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/CommandPreprocessor.scala @@ -356,7 +356,7 @@ private[lf] final class CommandPreprocessor( key.contractKey, ) val ckTtype = handleLookup(pkgInterface.lookupTemplateKey(templateId)).typ - val preprocessedKey = translateArg(ckTtype, key.contractKey) + val preprocessedKey = translateArg(ckTtype, upgradable(templateId), key.contractKey) speedy.Speedy.Machine .globalKey(pkgInterface, templateId, preprocessedKey) diff --git a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala index c2474d70a066..f67fb2a54e1b 100644 --- a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala +++ b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala @@ -15,7 +15,7 @@ import com.daml.lf.value.Value._ import scala.annotation.tailrec private[lf] final class ValueTranslator( - pkgInterface: language.PackageInterface, + val pkgInterface: language.PackageInterface, checkV1ContractIdSuffixes: Boolean, ) { diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala index 66b96fa06e3f..d28e18e3d2f4 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala @@ -102,6 +102,18 @@ class Converter(majorLanguageVersion: LanguageMajorVersion) { ) } + private[lf] def upgradable(translator: preprocessing.ValueTranslator, tyCon: Ref.TypeConName): Boolean = + translator.pkgInterface.lookupPackage(tyCon.packageId).fold(_ => false, _.upgradable) + + // Interface takes precedence. If interface doesn't support it, template can't + // fall back to template upgradability + private[lf] def upgradableWithInterface( + translator: preprocessing.ValueTranslator, + tplTyCon: Ref.TypeConName, + ifaceTyCon: Option[Ref.TypeConName], + ): Boolean = + ifaceTyCon.fold(upgradable(translator, tplTyCon))(upgradable(translator, _)) + private[lf] def fromTemplateTypeRep(templateId: SValue): SValue = record(stablePackages.TemplateTypeRep, ("getTemplateTypeRep", templateId)) @@ -136,7 +148,7 @@ class Converter(majorLanguageVersion: LanguageMajorVersion) { ): Either[String, SValue] = for { translated <- translator - .translateValue(TTyCon(templateId), argument) + .translateValue(TTyCon(templateId), upgradable(translator, templateId), argument) .left .map(err => s"Failed to translate create argument: $err") } yield record( @@ -170,8 +182,9 @@ class Converter(majorLanguageVersion: LanguageMajorVersion) { ): Either[String, SValue] = { for { choice <- lookupChoice(templateId, interfaceId, choiceName) + isUpgradable = upgradableWithInterface(translator, templateId, interfaceId) translated <- translator - .translateValue(choice.argBinder._2, argument) + .translateValue(choice.argBinder._2, isUpgradable, argument) .left .map(err => s"Failed to translate exercise argument: $err") } yield record( @@ -369,12 +382,13 @@ class Converter(majorLanguageVersion: LanguageMajorVersion) { private[lf] def fromInterfaceView( translator: preprocessing.ValueTranslator, + interfaceId: Identifier, viewType: Type, value: Value, ): Either[String, SValue] = { for { translated <- translator - .translateValue(viewType, value) + .translateValue(viewType, upgradable(translator, interfaceId), value) .left .map(err => s"Failed to translate value of interface view: $err") } yield translated @@ -513,7 +527,7 @@ class Converter(majorLanguageVersion: LanguageMajorVersion) { checkV1ContractIdSuffixes = false, ) sValue <- valueTranslator - .translateValue(ty, lfValue) + .translateValue(ty, false, lfValue) .left .map(_.message) } yield sValue diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/Converter.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/Converter.scala index 191024e579f8..83c3e939ef3b 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/Converter.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/Converter.scala @@ -41,8 +41,9 @@ object Converter extends script.Converter(LanguageMajorVersion.V1) { for { choice <- Name.fromString(result.choice) c <- lookupChoice(result.templateId, result.interfaceId, choice) + isUpgradable = upgradableWithInterface(translator, result.templateId, result.interfaceId) translated <- translator - .translateValue(c.returnType, result.result) + .translateValue(c.returnType, isUpgradable, result.result) .left .map(err => s"Failed to translate exercise result: $err") } yield translated diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ScriptF.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ScriptF.scala index cb09aa6dbd8e..525b75102e10 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ScriptF.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ScriptF.scala @@ -81,10 +81,6 @@ object ScriptF { case Right(key) => Right(key.view) case Left(err) => Left(err.pretty) } - - def translateValue(ty: Ast.Type, value: Value): Either[String, SValue] = - valueTranslator.translateValue(ty, value).left.map(_.toString) - } final case class Submit(data: SubmitData) extends Cmd { override def stackTrace = data.stackTrace @@ -294,6 +290,7 @@ object ScriptF { for { view <- Converter.fromInterfaceView( env.valueTranslator, + interfaceId, viewType, view, ) @@ -326,7 +323,7 @@ object ScriptF { client <- Converter.toFuture(env.clients.getPartiesParticipant(parties)) optR <- client.queryInterfaceContractId(parties, interfaceId, viewType, cid) optR <- Converter.toFuture( - optR.traverse(Converter.fromInterfaceView(env.valueTranslator, viewType, _)) + optR.traverse(Converter.fromInterfaceView(env.valueTranslator, interfaceId, viewType, _)) ) } yield SEAppAtomic(SEValue(continue), Array(SEValue(SOptional(optR)))) } @@ -346,7 +343,8 @@ object ScriptF { )(id: Identifier, v: Value): Either[String, SValue] = for { keyTy <- env.lookupKeyTy(id) - translated <- env.valueTranslator.translateValue(keyTy, v).left.map(_.message) + isUpgradable = Converter.upgradable(env.valueTranslator, tplId) + translated <- env.valueTranslator.translateValue(keyTy, isUpgradable, v).left.map(_.message) } yield translated override def execute(env: Env)(implicit diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ledgerinteraction/IdeLedgerClient.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ledgerinteraction/IdeLedgerClient.scala index 1c0f4d857421..2a6b4c377b85 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ledgerinteraction/IdeLedgerClient.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ledgerinteraction/IdeLedgerClient.scala @@ -149,7 +149,9 @@ class IdeLedgerClient( checkV1ContractIdSuffixes = false, ) - valueTranslator.translateValue(TTyCon(templateId), arg) match { + val isUpgradable = compiledPackages.pkgInterface.lookupPackage(interfaceId.packageId).fold(_ => false, _.upgradable) + + valueTranslator.translateValue(TTyCon(templateId), isUpgradable, arg) match { case Left(_) => sys.error("computeView: translateValue failed") diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/Converter.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/Converter.scala index 55fd2328c6c5..f92888d28f37 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/Converter.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/Converter.scala @@ -45,8 +45,9 @@ final class Converter(majorLanguageVersion: LanguageMajorVersion) for { choice <- Name.fromString(result.choice) c <- lookupChoice(result.templateId, result.interfaceId, choice) + isUpgradable = upgradableWithInterface(translator, result.templateId, result.interfaceId) translated <- translator - .translateValue(c.returnType, result.result) + .translateValue(c.returnType, isUpgradable, result.result) .left .map(err => s"Failed to translate exercise result: $err") } yield translated diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ScriptF.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ScriptF.scala index f61b4cc43329..ff0e1b113ccf 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ScriptF.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ScriptF.scala @@ -79,8 +79,8 @@ object ScriptF { case Left(err) => Left(err.pretty) } - def translateValue(ty: Ast.Type, value: Value): Either[String, SValue] = - valueTranslator.translateValue(ty, value).left.map(_.toString) + def translateValue(ty: Ast.Type, isUpgradable: Boolean, value: Value): Either[String, SValue] = + valueTranslator.translateValue(ty, isUpgradable, value).left.map(_.toString) def lookupLanguageVersion(packageId: PackageId): Either[String, LanguageVersion] = { compiledPackages.pkgInterface.lookupPackageLanguageVersion(packageId) match { @@ -353,6 +353,7 @@ class ScriptF(majorLanguageVersion: LanguageMajorVersion) { for { view <- converter.fromInterfaceView( env.valueTranslator, + interfaceId, viewType, view, ) @@ -381,7 +382,7 @@ class ScriptF(majorLanguageVersion: LanguageMajorVersion) { client <- converter.toFuture(env.clients.getPartiesParticipant(parties)) optR <- client.queryInterfaceContractId(parties, interfaceId, viewType, cid) optR <- converter.toFuture( - optR.traverse(converter.fromInterfaceView(env.valueTranslator, viewType, _)) + optR.traverse(converter.fromInterfaceView(env.valueTranslator, interfaceId, viewType, _)) ) } yield SEValue(SOptional(optR)) } @@ -395,8 +396,9 @@ class ScriptF(majorLanguageVersion: LanguageMajorVersion) { private def translateKey(env: Env)(id: Identifier, v: Value): Either[String, SValue] = for { keyTy <- env.lookupKeyTy(id) + isUpgradable = converter.upgradable(env.valueTranslator, tplId) translated <- env.valueTranslator - .translateValue(keyTy, v) + .translateValue(keyTy, isUpgradable, v) .left .map(_.message) } yield translated diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/IdeLedgerClient.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/IdeLedgerClient.scala index 55997445ebbf..48c3c181cdc0 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/IdeLedgerClient.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/IdeLedgerClient.scala @@ -196,7 +196,9 @@ class IdeLedgerClient( checkV1ContractIdSuffixes = false, ) - valueTranslator.translateValue(TTyCon(templateId), arg) match { + val isUpgradable = compiledPackages.pkgInterface.lookupPackage(interfaceId.packageId).fold(_ => false, _.upgradable) + + valueTranslator.translateValue(TTyCon(templateId), isUpgradable, arg) match { case Left(e) => sys.error(s"computeView: translateValue failed: $e") diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/SubmitError.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/SubmitError.scala index 776d55eae894..dd6308d5d414 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/SubmitError.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/SubmitError.scala @@ -64,7 +64,7 @@ class SubmitErrors(majorLanguageVersion: LanguageMajorVersion) { def globalKeyToAnyContractKey(env: Env, templateId: Identifier, key: Value): SValue = { val ty = env.lookupKeyTy(templateId).toOption.get - val sValue = env.translateValue(ty, key).toOption.get + val sValue = env.translateValue(ty, upgradable(env.valueTranslator, templateId), key).toOption.get fromAnyContractKey(AnyContractKey(templateId, ty, sValue)) } @@ -239,7 +239,7 @@ class SubmitErrors(majorLanguageVersion: LanguageMajorVersion) { sealed case class UnhandledException(exc: Option[(Identifier, Value)]) extends SubmitError { override def toDamlSubmitError(env: Env): SValue = { val sValue = exc.map { case (ty, value) => - SAny(Ast.TTyCon(ty), env.translateValue(Ast.TTyCon(ty), value).toOption.get) + SAny(Ast.TTyCon(ty), env.translateValue(Ast.TTyCon(ty), false, value).toOption.get) } SubmitErrorConverters(env).damlScriptError( "UnhandledException", diff --git a/sdk/triggers/runner/src/main/scala/com/digitalasset/daml/lf/engine/trigger/Converter.scala b/sdk/triggers/runner/src/main/scala/com/digitalasset/daml/lf/engine/trigger/Converter.scala index 27f12c1a0066..bc40027986e8 100644 --- a/sdk/triggers/runner/src/main/scala/com/digitalasset/daml/lf/engine/trigger/Converter.scala +++ b/sdk/triggers/runner/src/main/scala/com/digitalasset/daml/lf/engine/trigger/Converter.scala @@ -61,7 +61,7 @@ final class Converter( private[this] def translateValue(ty: Type, value: Value): Either[String, SValue] = valueTranslator - .translateValue(ty, value) + .translateValue(ty, false, value) .left .map(res => s"Failure to translate value: $res") From 14285ba0a1eef1e1a8ee4486332005285d1b6d80 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 13 Dec 2024 09:43:50 +0000 Subject: [PATCH 03/13] Change SBuiltinBeginExercise node version --- .../main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala index 9f86701de745..f26e6d315afd 100644 --- a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala +++ b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala @@ -1036,7 +1036,7 @@ private[lf] object SBuiltin { } ) val interfaceVersion = interfaceId.map(machine.tmplId2TxVersion) - val exerciseVersion = interfaceVersion.fold(templateVersion)(_.max(templateVersion)) + val exerciseVersion = interfaceVersion.fold(templateVersion)(_.max(templateVersion min TransactionVersion.V15)) val chosenValue = args.get(0).toNormalizedValue(exerciseVersion) val controllers = extractParties(NameOf.qualifiedNameOfCurrentFunc, args.get(2)) machine.enforceChoiceControllersLimit( From 79dcde5d3f38cc51e4e463bb1f448d78f160e0dc Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 13 Dec 2024 09:44:22 +0000 Subject: [PATCH 04/13] Format --- .../com/digitalasset/daml/lf/speedy/SBuiltin.scala | 3 ++- .../daml/lf/engine/script/Converter.scala | 11 +++++++---- .../script/v1/ledgerinteraction/IdeLedgerClient.scala | 4 +++- .../script/v2/ledgerinteraction/IdeLedgerClient.scala | 4 +++- .../script/v2/ledgerinteraction/SubmitError.scala | 3 ++- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala index f26e6d315afd..f0cca8802dc8 100644 --- a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala +++ b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala @@ -1036,7 +1036,8 @@ private[lf] object SBuiltin { } ) val interfaceVersion = interfaceId.map(machine.tmplId2TxVersion) - val exerciseVersion = interfaceVersion.fold(templateVersion)(_.max(templateVersion min TransactionVersion.V15)) + val exerciseVersion = + interfaceVersion.fold(templateVersion)(_.max(templateVersion min TransactionVersion.V15)) val chosenValue = args.get(0).toNormalizedValue(exerciseVersion) val controllers = extractParties(NameOf.qualifiedNameOfCurrentFunc, args.get(2)) machine.enforceChoiceControllersLimit( diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala index d28e18e3d2f4..560006c77857 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala @@ -102,15 +102,18 @@ class Converter(majorLanguageVersion: LanguageMajorVersion) { ) } - private[lf] def upgradable(translator: preprocessing.ValueTranslator, tyCon: Ref.TypeConName): Boolean = + private[lf] def upgradable( + translator: preprocessing.ValueTranslator, + tyCon: Ref.TypeConName, + ): Boolean = translator.pkgInterface.lookupPackage(tyCon.packageId).fold(_ => false, _.upgradable) // Interface takes precedence. If interface doesn't support it, template can't // fall back to template upgradability private[lf] def upgradableWithInterface( - translator: preprocessing.ValueTranslator, - tplTyCon: Ref.TypeConName, - ifaceTyCon: Option[Ref.TypeConName], + translator: preprocessing.ValueTranslator, + tplTyCon: Ref.TypeConName, + ifaceTyCon: Option[Ref.TypeConName], ): Boolean = ifaceTyCon.fold(upgradable(translator, tplTyCon))(upgradable(translator, _)) diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ledgerinteraction/IdeLedgerClient.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ledgerinteraction/IdeLedgerClient.scala index 2a6b4c377b85..0180ed1f837f 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ledgerinteraction/IdeLedgerClient.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ledgerinteraction/IdeLedgerClient.scala @@ -149,7 +149,9 @@ class IdeLedgerClient( checkV1ContractIdSuffixes = false, ) - val isUpgradable = compiledPackages.pkgInterface.lookupPackage(interfaceId.packageId).fold(_ => false, _.upgradable) + val isUpgradable = compiledPackages.pkgInterface + .lookupPackage(interfaceId.packageId) + .fold(_ => false, _.upgradable) valueTranslator.translateValue(TTyCon(templateId), isUpgradable, arg) match { case Left(_) => diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/IdeLedgerClient.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/IdeLedgerClient.scala index 48c3c181cdc0..7085e2dda603 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/IdeLedgerClient.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/IdeLedgerClient.scala @@ -196,7 +196,9 @@ class IdeLedgerClient( checkV1ContractIdSuffixes = false, ) - val isUpgradable = compiledPackages.pkgInterface.lookupPackage(interfaceId.packageId).fold(_ => false, _.upgradable) + val isUpgradable = compiledPackages.pkgInterface + .lookupPackage(interfaceId.packageId) + .fold(_ => false, _.upgradable) valueTranslator.translateValue(TTyCon(templateId), isUpgradable, arg) match { case Left(e) => diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/SubmitError.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/SubmitError.scala index dd6308d5d414..c2ea07be3101 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/SubmitError.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/ledgerinteraction/SubmitError.scala @@ -64,7 +64,8 @@ class SubmitErrors(majorLanguageVersion: LanguageMajorVersion) { def globalKeyToAnyContractKey(env: Env, templateId: Identifier, key: Value): SValue = { val ty = env.lookupKeyTy(templateId).toOption.get - val sValue = env.translateValue(ty, upgradable(env.valueTranslator, templateId), key).toOption.get + val sValue = + env.translateValue(ty, upgradable(env.valueTranslator, templateId), key).toOption.get fromAnyContractKey(AnyContractKey(templateId, ty, sValue)) } From 3dc448c7cb1f4119e82828f4db7f6eb30d9a2b6d Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 13 Dec 2024 09:53:16 +0000 Subject: [PATCH 05/13] Update version test to new logic and ignore coimplements in 1.17 --- .../lf/speedy/TransactionVersionTest.scala | 62 +++++++++++++++---- 1 file changed, 50 insertions(+), 12 deletions(-) diff --git a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala index b9e9059a148e..f6281992d58c 100644 --- a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala +++ b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala @@ -46,18 +46,16 @@ class TransactionVersionTest(majorLanguageVersion: LanguageMajorVersion) ) shouldBe Set(commonVersion) } - "template version > interface version" in { - val oldPkg1 = templatePkg.copy(languageVersion = oldVersion) - val oldPkg2 = interfacesPkg.copy(languageVersion = oldVersion) - val newPkg1 = implementsPkg.copy(languageVersion = newVersion) - val newPkg2 = coImplementsPkg.copy(languageVersion = newVersion) + // Before upgrades, node version should be max of interface and template version + // we also still support coimplements + "template version > interface version pre Upgrades" in { val pkgs = SpeedyTestLib.typeAndCompile( majorLanguageVersion, Map( - templatePkgId -> oldPkg1, - interfacesPkgId -> oldPkg2, - implementsPkgId -> newPkg1, - coImplementsPkgId -> newPkg2, + templatePkgId -> templatePkg.copy(languageVersion = olderVersion), + interfacesPkgId -> interfacesPkg.copy(languageVersion = olderVersion), + implementsPkgId -> implementsPkg.copy(languageVersion = oldVersion), + coImplementsPkgId -> coImplementsPkg.copy(languageVersion = oldVersion), ), ) @@ -73,11 +71,44 @@ class TransactionVersionTest(majorLanguageVersion: LanguageMajorVersion) ) inside(result) { case Right(transaction) => - transaction.version shouldBe TransactionVersion.assignNodeVersion(newVersion) + transaction.version shouldBe TransactionVersion.assignNodeVersion(oldVersion) } } } + // Post upgrades, we only consider a node 1.17 if the interface is 1.17 + // We do not support coimplements + "template version > interface version post Upgrades" in { + val pkgs = SpeedyTestLib.typeAndCompile( + majorLanguageVersion, + Map( + templatePkgId -> templatePkg, // Unused + interfacesPkgId -> interfacesPkg.copy(languageVersion = oldVersion), + implementsPkgId -> implementsPkg.copy(languageVersion = newVersion), + coImplementsPkgId -> coImplementsPkg, // Unused + ), + ) + + // Only use the implements case, dont support coimplements here + val (templateId, interfaceId, contract) = + (implementsTemplateId, implementsInterfaceId, implementsContract) + + val result = evaluateBeginExercise( + pkgs, + templateId, + Some(interfaceId), + contractId, + committers = Set(contractParty), + controllers = Set(contractParty), + getContract = Map(contractId -> contract), + ) + + inside(result) { case Right(transaction) => + // Here its still the old version, as the interface is isn't 1.17 + transaction.version shouldBe TransactionVersion.assignNodeVersion(oldVersion) + } + } + "template version == interface version" in { val pkgs = SpeedyTestLib.typeAndCompile( majorLanguageVersion, @@ -110,13 +141,20 @@ class TransactionVersionTest(majorLanguageVersion: LanguageMajorVersion) private[lf] class TransactionVersionTestHelpers(majorLanguageVersion: LanguageMajorVersion) { - val (commonVersion, oldVersion, newVersion) = majorLanguageVersion match { - case V1 => (LanguageVersion.default, LanguageVersion.v1_15, LanguageVersion.v1_dev) + val (commonVersion, olderVersion, oldVersion, newVersion) = majorLanguageVersion match { + case V1 => + ( + LanguageVersion.default, + LanguageVersion.v1_14, + LanguageVersion.v1_15, + LanguageVersion.v1_dev, + ) case V2 => ( // TODO(#17366): Use something like languageVersion.default(V2) once available LanguageVersion.v2_1, LanguageVersion.v2_1, + LanguageVersion.v2_1, LanguageVersion.v2_dev, ) } From 5839ad8263b1407cad790bbb3da7d8aa7cb8d5e5 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 13 Dec 2024 11:16:31 +0000 Subject: [PATCH 06/13] Fix missing translateValue call --- .../com/daml/lf/engine/script/test/AbstractScriptTest.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/daml-script/test/src/test-utils/com/daml/lf/engine/script/test/AbstractScriptTest.scala b/sdk/daml-script/test/src/test-utils/com/daml/lf/engine/script/test/AbstractScriptTest.scala index d7f18a4dd369..4d805f88dd5b 100644 --- a/sdk/daml-script/test/src/test-utils/com/daml/lf/engine/script/test/AbstractScriptTest.scala +++ b/sdk/daml-script/test/src/test-utils/com/daml/lf/engine/script/test/AbstractScriptTest.scala @@ -56,7 +56,7 @@ trait AbstractScriptTest extends CantonFixture with PekkoBeforeAndAfterAll { dar.compiledPackages.pkgInterface, checkV1ContractIdSuffixes = true, ) - .translateValue(typ, input) + .translateValue(typ, false, input) .left .map(_.message) Runner From d393f9691d4bbae2bf94b2e0f0f4e4042d0e84ec Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 13 Dec 2024 12:55:15 +0000 Subject: [PATCH 07/13] Fix enrichment and substituted in translator before optional check --- .../com/digitalasset/daml/lf/engine/ValueEnricher.scala | 8 +++++--- .../daml/lf/engine/preprocessing/ValueTranslator.scala | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ValueEnricher.scala b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ValueEnricher.scala index d80b5bed56a9..0cc9c82408e0 100644 --- a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ValueEnricher.scala +++ b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ValueEnricher.scala @@ -152,7 +152,7 @@ final class ValueEnricher( ) def enrichChoiceResult( - choicePackageId: PackageId, + templatePackageId: PackageId, qualifiedTemplateName: QualifiedName, interfaceId: Option[Identifier], choiceName: Name, @@ -160,12 +160,14 @@ final class ValueEnricher( ): Result[Value] = for { choice <- handleLookup( pkgInterface.lookupChoice( - Identifier(choicePackageId, qualifiedTemplateName), + Identifier(templatePackageId, qualifiedTemplateName), interfaceId, choiceName, ) ) - pkg <- handleLookup(pkgInterface.lookupPackage(choicePackageId)) + pkg <- handleLookup( + pkgInterface.lookupPackage(interfaceId.fold(templatePackageId)(_.packageId)) + ) enrichedValue <- enrichValue(choice.returnType, pkg.upgradable, value) } yield enrichedValue diff --git a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala index f67fb2a54e1b..442a699a5791 100644 --- a/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala +++ b/sdk/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala @@ -197,8 +197,11 @@ private[lf] final class ValueTranslator( case ValueRecord(mbId, sourceElements) => checkUserTypeId(upgradable, tyCon, mbId) val lookupResult = handleLookup(pkgInterface.lookupDataRecord(tyCon)) - val targetFieldsAndTypes = lookupResult.dataRecord.fields val subst = lookupResult.subst(tyArgs) + val targetFieldsAndTypes = + lookupResult.dataRecord.fields.map { case (lbl, typ) => + lbl -> AstUtil.substitute(typ, subst) + } def addMissingField(lbl: Ref.Name, ty: Type): (Option[Ref.Name], Value) = ty match { @@ -279,8 +282,7 @@ private[lf] final class ValueTranslator( // Recursive substitution val translatedCorrectFields = correctFields.map { case (lbl, v, typ) => - val replacedTyp = AstUtil.substitute(typ, subst) - lbl -> go(replacedTyp, v, newNesting) + lbl -> go(typ, v, newNesting) } extraFields.foreach { From e5e67803279eebaec1e560bae4ecb2dcb9a91e89 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 13 Dec 2024 13:23:04 +0000 Subject: [PATCH 08/13] Fix various tests --- .../daml/lf/engine/EngineTest.scala | 13 +- .../daml/lf/engine/ValueTranslatorSpec.scala | 342 +++++++++--------- 2 files changed, 188 insertions(+), 167 deletions(-) diff --git a/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala b/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala index 32d0ef7e738e..09b8e7343a19 100644 --- a/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala +++ b/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala @@ -883,7 +883,7 @@ class EngineTest(langVersion: LanguageVersion) "translate empty list" in { val list = ValueNil val res = preprocessor - .translateValue(TList(TBuiltin(BTInt64)), list) + .translateValue(TList(TBuiltin(BTInt64)), false, list) .consume(lookupContract, lookupPackage, lookupKey) res shouldEqual Right(SList(FrontStack.empty)) @@ -892,7 +892,7 @@ class EngineTest(langVersion: LanguageVersion) "translate singleton" in { val list = ValueList(FrontStack(ValueInt64(1))) val res = preprocessor - .translateValue(TList(TBuiltin(BTInt64)), list) + .translateValue(TList(TBuiltin(BTInt64)), false, list) .consume(lookupContract, lookupPackage, lookupKey) res shouldEqual Right(SList(FrontStack(SInt64(1)))) @@ -903,7 +903,7 @@ class EngineTest(langVersion: LanguageVersion) FrontStack(ValueInt64(1), ValueInt64(2), ValueInt64(3), ValueInt64(4), ValueInt64(5)) ) val res = preprocessor - .translateValue(TList(TBuiltin(BTInt64)), list) + .translateValue(TList(TBuiltin(BTInt64)), false, list) .consume(lookupContract, lookupPackage, lookupKey) res shouldEqual Right( @@ -918,6 +918,7 @@ class EngineTest(langVersion: LanguageVersion) preprocessor .translateValue( TTyConApp(TypeConName(basicTestsPkgId, "BasicTests:Nesting0"), ImmArray.Empty), + false, nested, ) .consume(lookupContract, lookupPackage, lookupKey) @@ -951,6 +952,7 @@ class EngineTest(langVersion: LanguageVersion) val res = preprocessor .translateValue( TTyConApp(Identifier(basicTestsPkgId, "BasicTests:MyNestedRec"), ImmArray.Empty), + false, rec, ) .consume(lookupContract, lookupPackage, lookupKey) @@ -973,6 +975,7 @@ class EngineTest(langVersion: LanguageVersion) val res = preprocessor .translateValue( TTyConApp(Identifier(basicTestsPkgId, "BasicTests:TypeWithParameters"), ImmArray.Empty), + false, rec, ) .consume(lookupContract, lookupPackage, lookupKey) @@ -996,6 +999,7 @@ class EngineTest(langVersion: LanguageVersion) val res = preprocessor .translateValue( TTyConApp(Identifier(basicTestsPkgId, "BasicTests:TypeWithParameters"), ImmArray.Empty), + false, rec, ) .consume(lookupContract, lookupPackage, lookupKey) @@ -1016,6 +1020,7 @@ class EngineTest(langVersion: LanguageVersion) val res = preprocessor .translateValue( TTyConApp(Identifier(basicTestsPkgId, "BasicTests:TypeWithParameters"), ImmArray.Empty), + false, rec, ) .consume(lookupContract, lookupPackage, lookupKey) @@ -1038,6 +1043,7 @@ class EngineTest(langVersion: LanguageVersion) val res = preprocessor .translateValue( TTyConApp(Identifier(basicTestsPkgId, "BasicTests:TypeWithParameters"), ImmArray.Empty), + false, rec, ) .consume(lookupContract, lookupPackage, lookupKey) @@ -1058,6 +1064,7 @@ class EngineTest(langVersion: LanguageVersion) val res = preprocessor .translateValue( TTyConApp(Identifier(basicTestsPkgId, "BasicTests:TypeWithParameters"), ImmArray.Empty), + false, rec, ) .consume(lookupContract, lookupPackage, lookupKey) diff --git a/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/ValueTranslatorSpec.scala b/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/ValueTranslatorSpec.scala index 083f9987d29a..291a5019a7d1 100644 --- a/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/ValueTranslatorSpec.scala +++ b/sdk/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/ValueTranslatorSpec.scala @@ -382,182 +382,196 @@ class ValueTranslatorSpec nonUpgradableUserTestCases ++ upgradableUserTestCases ++ emptyBuiltinTestCase ++ nonEmptyBuiltinTestCases ) { (typ, normal, nonNormal, svalue) => (normal +: nonNormal).takeRight(1).foreach { v => - Try(unsafeTranslateValue(typ, v)) shouldBe Success(svalue) + Try(unsafeTranslateValue(typ, true, v)) shouldBe Success(svalue) } } } "return proper mismatch error" in { - val testCases = Table[Ast.Type, Value, PartialFunction[Error.Preprocessing.Error, _]]( - ("type", "value", "error"), - ( - TRecordNonUpgradable, - ValueRecord( - "", - ImmArray( - "fieldA" -> aInt + val testCases = + Table[Ast.Type, Value, Boolean, PartialFunction[Error.Preprocessing.Error, _]]( + ("type", "value", "upgradable", "error"), + ( + TRecordNonUpgradable, + ValueRecord( + "", + ImmArray( + "fieldA" -> aInt + ), ), + false, + { case Error.Preprocessing.TypeMismatch(typ, _, msg) => + typ shouldBe TRecordNonUpgradable + msg should include regex "Expecting 2 field for record .*, but got 1" + }, ), - { case Error.Preprocessing.TypeMismatch(typ, _, msg) => - typ shouldBe TRecordNonUpgradable - msg should include regex "Expecting 2 field for record .*, but got 1" - }, - ), - ( - TRecordNonUpgradable, - ValueRecord( - "", - ImmArray( - "fieldA" -> aInt, - "fieldB" -> someText, - "fieldC" -> none, + ( + TRecordNonUpgradable, + ValueRecord( + "", + ImmArray( + "fieldA" -> aInt, + "fieldB" -> someText, + "fieldC" -> none, + ), ), + false, + { case Error.Preprocessing.TypeMismatch(typ, _, msg) => + typ shouldBe TRecordNonUpgradable + msg should include regex "Expecting 2 field for record .*, but got 3" + }, ), - { case Error.Preprocessing.TypeMismatch(typ, _, msg) => - typ shouldBe TRecordNonUpgradable - msg should include regex "Expecting 2 field for record .*, but got 3" - }, - ), - ( - TRecordNonUpgradable, - ValueRecord( - "", - ImmArray( - "fieldA" -> aInt, - "fieldB" -> someText, - "fieldC" -> someParty, - "fieldD" -> aInt, // extra non-optional field + ( + TRecordNonUpgradable, + ValueRecord( + "", + ImmArray( + "fieldA" -> aInt, + "fieldB" -> someText, + "fieldC" -> someParty, + "fieldD" -> aInt, // extra non-optional field + ), ), + false, + { case Error.Preprocessing.TypeMismatch(typ, _, _) => + typ shouldBe TRecordNonUpgradable + }, ), - { case Error.Preprocessing.TypeMismatch(typ, _, _) => - typ shouldBe TRecordNonUpgradable - }, - ), - ( - TRecordUpgradable, - ValueRecord( - "", - ImmArray( - "fieldA" -> aInt, - "fieldB" -> someParty, // Here the field has type Party instead of Text - "fieldC" -> none, + ( + TRecordUpgradable, + ValueRecord( + "", + ImmArray( + "fieldA" -> aInt, + "fieldB" -> someParty, // Here the field has type Party instead of Text + "fieldC" -> none, + ), ), + true, + { case Error.Preprocessing.TypeMismatch(typ, value, _) => + typ shouldBe t"Text" + value shouldBe aParty + }, ), - { case Error.Preprocessing.TypeMismatch(typ, value, _) => - typ shouldBe t"Text" - value shouldBe aParty - }, - ), - ( - TVariantNonUpgradable, - ValueVariant("", "ConsB", aInt), // Here the variant has type Text instead of Int64 - { case Error.Preprocessing.TypeMismatch(typ, value, _) => - typ shouldBe t"Text" - value shouldBe aInt - }, - ), - ( - TVariantNonUpgradable, - ValueVariant("", "ConsC", aInt), // ConsC is not a constructor of Mod:Variant - { - case Error.Preprocessing.Lookup( - LookupError.NotFound( - Reference.DataVariantConstructor(_, consName), - Reference.DataVariantConstructor(_, _), - ) - ) => - consName shouldBe "ConsC" - }, - ), - ( - TEnumNonUpgradable, - ValueEnum("", "Cons3"), // Cons3 is not a constructor of Mod:Enum - { - case Error.Preprocessing.Lookup( - LookupError.NotFound( - Reference.DataEnumConstructor(_, consName), - _, - ) - ) => - consName shouldBe "Cons3" - }, - ), - ( - TRecordUpgradable, - ValueRecord( - "", - ImmArray( // fields non-order and non fully labelled. - "fieldA" -> aInt, - "" -> none, - "fieldB" -> someText, + ( + TVariantNonUpgradable, + ValueVariant("", "ConsB", aInt), // Here the variant has type Text instead of Int64 + false, + { case Error.Preprocessing.TypeMismatch(typ, value, _) => + typ shouldBe t"Text" + value shouldBe aInt + }, + ), + ( + TVariantNonUpgradable, + ValueVariant("", "ConsC", aInt), // ConsC is not a constructor of Mod:Variant + false, + { + case Error.Preprocessing.Lookup( + LookupError.NotFound( + Reference.DataVariantConstructor(_, consName), + Reference.DataVariantConstructor(_, _), + ) + ) => + consName shouldBe "ConsC" + }, + ), + ( + TEnumNonUpgradable, + ValueEnum("", "Cons3"), // Cons3 is not a constructor of Mod:Enum + false, + { + case Error.Preprocessing.Lookup( + LookupError.NotFound( + Reference.DataEnumConstructor(_, consName), + _, + ) + ) => + consName shouldBe "Cons3" + }, + ), + ( + TRecordUpgradable, + ValueRecord( + "", + ImmArray( // fields non-order and non fully labelled. + "fieldA" -> aInt, + "" -> none, + "fieldB" -> someText, + ), ), + true, + { case Error.Preprocessing.TypeMismatch(typ, _, _) => + typ shouldBe TRecordUpgradable + }, ), - { case Error.Preprocessing.TypeMismatch(typ, _, _) => - typ shouldBe TRecordUpgradable - }, - ), - ( - TRecordUpgradable, - ValueRecord( - "", - ImmArray(), // missing a non-optional field + ( + TRecordUpgradable, + ValueRecord( + "", + ImmArray(), // missing a non-optional field + ), + true, + { case Error.Preprocessing.TypeMismatch(typ, _, _) => + typ shouldBe TRecordUpgradable + }, ), - { case Error.Preprocessing.TypeMismatch(typ, _, _) => - typ shouldBe TRecordUpgradable - }, - ), - ( - TRecordUpgradable, - ValueRecord( - "", - ImmArray( - "fieldA" -> aInt, - "fieldB" -> someText, - "fieldC" -> someParty, - "fieldD" -> aInt, // extra non-optional field + ( + TRecordUpgradable, + ValueRecord( + "", + ImmArray( + "fieldA" -> aInt, + "fieldB" -> someText, + "fieldC" -> someParty, + "fieldD" -> aInt, // extra non-optional field + ), ), + true, + { case Error.Preprocessing.TypeMismatch(typ, _, _) => + typ shouldBe TRecordUpgradable + }, ), - { case Error.Preprocessing.TypeMismatch(typ, _, _) => - typ shouldBe TRecordUpgradable - }, - ), - ( - TVariantUpgradable, - ValueVariant("", "ConsB", aInt), // Here the variant has type Text instead of Int64 - { case Error.Preprocessing.TypeMismatch(typ, value, _) => - typ shouldBe t"Text" - value shouldBe aInt - }, - ), - ( - TVariantUpgradable, - ValueVariant("", "ConsC", aInt), // ConsC is not a constructor of Mod:Variant - { - case Error.Preprocessing.Lookup( - LookupError.NotFound( - Reference.DataVariantConstructor(_, consName), - Reference.DataVariantConstructor(_, _), - ) - ) => - consName shouldBe "ConsC" - }, - ), - ( - TEnumUpgradable, - ValueEnum("", "Cons3"), // Cons3 is not a constructor of Mod:Enum - { - case Error.Preprocessing.Lookup( - LookupError.NotFound( - Reference.DataEnumConstructor(_, consName), - Reference.DataEnumConstructor(_, _), - ) - ) => - consName shouldBe "Cons3" - }, - ), - ) - forEvery(testCases)((typ, value, checkError) => - inside(Try(unsafeTranslateValue(typ, value))) { + ( + TVariantUpgradable, + ValueVariant("", "ConsB", aInt), // Here the variant has type Text instead of Int64 + true, + { case Error.Preprocessing.TypeMismatch(typ, value, _) => + typ shouldBe t"Text" + value shouldBe aInt + }, + ), + ( + TVariantUpgradable, + ValueVariant("", "ConsC", aInt), // ConsC is not a constructor of Mod:Variant + true, + { + case Error.Preprocessing.Lookup( + LookupError.NotFound( + Reference.DataVariantConstructor(_, consName), + Reference.DataVariantConstructor(_, _), + ) + ) => + consName shouldBe "ConsC" + }, + ), + ( + TEnumUpgradable, + ValueEnum("", "Cons3"), // Cons3 is not a constructor of Mod:Enum + true, + { + case Error.Preprocessing.Lookup( + LookupError.NotFound( + Reference.DataEnumConstructor(_, consName), + Reference.DataEnumConstructor(_, _), + ) + ) => + consName shouldBe "Cons3" + }, + ), + ) + forEvery(testCases)((typ, value, upgradable, checkError) => + inside(Try(unsafeTranslateValue(typ, upgradable, value))) { case Failure(error: Error.Preprocessing.Error) => checkError(error) } @@ -569,7 +583,7 @@ class ValueTranslatorSpec forAll(nonEmptyBuiltinTestCases) { (_, value2, _, _) => if (value1 != value2) { a[Error.Preprocessing.Error] shouldBe thrownBy( - unsafeTranslateValue(typ1, value2) + unsafeTranslateValue(typ1, true, value2) ) } } @@ -590,8 +604,8 @@ class ValueTranslatorSpec val tooBig = mkMyList(50) val failure = Failure(Error.Preprocessing.ValueNesting(tooBig)) - Try(unsafeTranslateValue(t"Mod:MyList", notTooBig)) shouldBe a[Success[_]] - Try(unsafeTranslateValue(t"Mod:MyList", tooBig)) shouldBe failure + Try(unsafeTranslateValue(t"Mod:MyList", true, notTooBig)) shouldBe a[Success[_]] + Try(unsafeTranslateValue(t"Mod:MyList", true, tooBig)) shouldBe failure } def testCasesForCid(culprit: ContractId) = { @@ -630,7 +644,7 @@ class ValueTranslatorSpec cids.foreach(cid => forEvery(testCasesForCid(cid))((typ, value) => - Try(valueTranslator.unsafeTranslateValue(typ, value)) shouldBe a[Success[ + Try(valueTranslator.unsafeTranslateValue(typ, true, value)) shouldBe a[Success[ _ ]] ) @@ -653,10 +667,10 @@ class ValueTranslatorSpec val failure = Failure(Error.Preprocessing.IllegalContractId.NonSuffixV1ContractId(illegalCid)) forEvery(testCasesForCid(legalCid))((typ, value) => - Try(valueTranslator.unsafeTranslateValue(typ, value)) shouldBe a[Success[_]] + Try(valueTranslator.unsafeTranslateValue(typ, true, value)) shouldBe a[Success[_]] ) forEvery(testCasesForCid(illegalCid))((typ, value) => - Try(valueTranslator.unsafeTranslateValue(typ, value)) shouldBe failure + Try(valueTranslator.unsafeTranslateValue(typ, true, value)) shouldBe failure ) } From 167fc7fda71fd969f979f4e32f7ea9142d58726b Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 13 Dec 2024 13:47:40 +0000 Subject: [PATCH 09/13] Update security --- sdk/security-evidence.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/security-evidence.md b/sdk/security-evidence.md index 7b1c65d0fdf8..f8a1dd454fbc 100644 --- a/sdk/security-evidence.md +++ b/sdk/security-evidence.md @@ -149,7 +149,7 @@ - Evaluation order of successful lookup_by_key of a local contract: [EvaluationOrderTest.scala](daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/EvaluationOrderTest.scala#L3120) - Evaluation order of successful lookup_by_key of a non-cached global contract: [EvaluationOrderTest.scala](daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/EvaluationOrderTest.scala#L2956) - Exceptions, throw/catch.: [ExceptionTest.scala](daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/ExceptionTest.scala#L35) -- Rollback creates cannot be exercise: [EngineTest.scala](daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala#L2125) +- Rollback creates cannot be exercise: [EngineTest.scala](daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala#L2132) - Smart Contract Upgrade: Can catch different errors thrown by different choice version within Update, using AnyException: [Exceptions.daml](daml-script/test/daml/upgrades/Exceptions.daml#L273) - Smart Contract Upgrade: Can catch different errors thrown by different choice version, where one is new to V2 within Update, using AnyException: [Exceptions.daml](daml-script/test/daml/upgrades/Exceptions.daml#L277) - Smart Contract Upgrade: Can catch same errors thrown by different choice versions within Update: [Exceptions.daml](daml-script/test/daml/upgrades/Exceptions.daml#L269) From 9b7ee5b0376b93db3301af89aac8029c894de4e9 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 13 Dec 2024 17:35:38 +0000 Subject: [PATCH 10/13] Apply suggestions from code review --- .../scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala | 7 +++++-- .../com/digitalasset/daml/lf/engine/script/Converter.scala | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala index f0cca8802dc8..4bc1e8cd410a 100644 --- a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala +++ b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala @@ -1036,8 +1036,11 @@ private[lf] object SBuiltin { } ) val interfaceVersion = interfaceId.map(machine.tmplId2TxVersion) - val exerciseVersion = - interfaceVersion.fold(templateVersion)(_.max(templateVersion min TransactionVersion.V15)) + // Interfaces were added in LF1.15, therefore the interface version is 1.15+ + // Interfaces in LF1.17 cannot have template version < interface version, as retroactive instances aren't supported in 17 + // Therefore, if the interface support upgrades, we pick the interface version + // if it doesn't, then its 1.15, which is correct + val exerciseVersion = interfaceVersion.getOrElse(templateVersion) val chosenValue = args.get(0).toNormalizedValue(exerciseVersion) val controllers = extractParties(NameOf.qualifiedNameOfCurrentFunc, args.get(2)) machine.enforceChoiceControllersLimit( diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala index 560006c77857..c3704605b52f 100644 --- a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala +++ b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala @@ -115,7 +115,7 @@ class Converter(majorLanguageVersion: LanguageMajorVersion) { tplTyCon: Ref.TypeConName, ifaceTyCon: Option[Ref.TypeConName], ): Boolean = - ifaceTyCon.fold(upgradable(translator, tplTyCon))(upgradable(translator, _)) + upgradable(translator, ifaceTyCon.getOrElse(tplTyCon)) private[lf] def fromTemplateTypeRep(templateId: SValue): SValue = record(stablePackages.TemplateTypeRep, ("getTemplateTypeRep", templateId)) From a72209c1f20f66a1daa9f209d5061ad353a00949 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Mon, 16 Dec 2024 10:21:18 +0000 Subject: [PATCH 11/13] Remove unused import --- .../main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala index 4bc1e8cd410a..f3b524d0e217 100644 --- a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala +++ b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala @@ -36,7 +36,6 @@ import java.util import scala.annotation.nowarn import scala.collection.immutable.TreeSet import scala.jdk.CollectionConverters._ -import scala.math.Ordering.Implicits.infixOrderingOps /** Speedy builtins represent LF functional forms. As such, they *always* have a non-zero arity. * From c993358db16a7a28055d6e10365fb1e7add38f2f Mon Sep 17 00:00:00 2001 From: Remy Haemmerle Date: Mon, 16 Dec 2024 13:22:23 +0100 Subject: [PATCH 12/13] fix test --- .../lf/speedy/TransactionVersionTest.scala | 60 ++++--------------- 1 file changed, 11 insertions(+), 49 deletions(-) diff --git a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala index f6281992d58c..c69775f34f5d 100644 --- a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala +++ b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala @@ -46,16 +46,18 @@ class TransactionVersionTest(majorLanguageVersion: LanguageMajorVersion) ) shouldBe Set(commonVersion) } - // Before upgrades, node version should be max of interface and template version - // we also still support coimplements - "template version > interface version pre Upgrades" in { + "template version > interface version" in { + val oldPkg1 = templatePkg.copy(languageVersion = oldVersion) + val oldPkg2 = interfacesPkg.copy(languageVersion = oldVersion) + val newPkg1 = implementsPkg.copy(languageVersion = newVersion) + val newPkg2 = coImplementsPkg.copy(languageVersion = newVersion) val pkgs = SpeedyTestLib.typeAndCompile( majorLanguageVersion, Map( - templatePkgId -> templatePkg.copy(languageVersion = olderVersion), - interfacesPkgId -> interfacesPkg.copy(languageVersion = olderVersion), - implementsPkgId -> implementsPkg.copy(languageVersion = oldVersion), - coImplementsPkgId -> coImplementsPkg.copy(languageVersion = oldVersion), + templatePkgId -> oldPkg1, + interfacesPkgId -> oldPkg2, + implementsPkgId -> newPkg1, + coImplementsPkgId -> newPkg2, ), ) @@ -76,39 +78,6 @@ class TransactionVersionTest(majorLanguageVersion: LanguageMajorVersion) } } - // Post upgrades, we only consider a node 1.17 if the interface is 1.17 - // We do not support coimplements - "template version > interface version post Upgrades" in { - val pkgs = SpeedyTestLib.typeAndCompile( - majorLanguageVersion, - Map( - templatePkgId -> templatePkg, // Unused - interfacesPkgId -> interfacesPkg.copy(languageVersion = oldVersion), - implementsPkgId -> implementsPkg.copy(languageVersion = newVersion), - coImplementsPkgId -> coImplementsPkg, // Unused - ), - ) - - // Only use the implements case, dont support coimplements here - val (templateId, interfaceId, contract) = - (implementsTemplateId, implementsInterfaceId, implementsContract) - - val result = evaluateBeginExercise( - pkgs, - templateId, - Some(interfaceId), - contractId, - committers = Set(contractParty), - controllers = Set(contractParty), - getContract = Map(contractId -> contract), - ) - - inside(result) { case Right(transaction) => - // Here its still the old version, as the interface is isn't 1.17 - transaction.version shouldBe TransactionVersion.assignNodeVersion(oldVersion) - } - } - "template version == interface version" in { val pkgs = SpeedyTestLib.typeAndCompile( majorLanguageVersion, @@ -141,20 +110,13 @@ class TransactionVersionTest(majorLanguageVersion: LanguageMajorVersion) private[lf] class TransactionVersionTestHelpers(majorLanguageVersion: LanguageMajorVersion) { - val (commonVersion, olderVersion, oldVersion, newVersion) = majorLanguageVersion match { - case V1 => - ( - LanguageVersion.default, - LanguageVersion.v1_14, - LanguageVersion.v1_15, - LanguageVersion.v1_dev, - ) + val (commonVersion, oldVersion, newVersion) = majorLanguageVersion match { + case V1 => (LanguageVersion.default, LanguageVersion.v1_15, LanguageVersion.v1_17) case V2 => ( // TODO(#17366): Use something like languageVersion.default(V2) once available LanguageVersion.v2_1, LanguageVersion.v2_1, - LanguageVersion.v2_1, LanguageVersion.v2_dev, ) } From 302187ed60f78b91af4d0bd8ae93b2683a567f37 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Mon, 16 Dec 2024 12:53:07 +0000 Subject: [PATCH 13/13] Fix test attempt 2 --- .../lf/speedy/TransactionVersionTest.scala | 52 +++++++++++-------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala index c69775f34f5d..3975da54f1ec 100644 --- a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala +++ b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TransactionVersionTest.scala @@ -46,35 +46,36 @@ class TransactionVersionTest(majorLanguageVersion: LanguageMajorVersion) ) shouldBe Set(commonVersion) } - "template version > interface version" in { - val oldPkg1 = templatePkg.copy(languageVersion = oldVersion) - val oldPkg2 = interfacesPkg.copy(languageVersion = oldVersion) - val newPkg1 = implementsPkg.copy(languageVersion = newVersion) - val newPkg2 = coImplementsPkg.copy(languageVersion = newVersion) + // Post upgrades, we only consider a node 1.17 if the interface is 1.17 + // We do not support coimplements + "template version > interface" in { val pkgs = SpeedyTestLib.typeAndCompile( majorLanguageVersion, Map( - templatePkgId -> oldPkg1, - interfacesPkgId -> oldPkg2, - implementsPkgId -> newPkg1, - coImplementsPkgId -> newPkg2, + templatePkgId -> templatePkg, // Unused + interfacesPkgId -> interfacesPkg.copy(languageVersion = oldVersion), + implementsPkgId -> implementsPkg.copy(languageVersion = newVersion), + coImplementsPkgId -> coImplementsPkg, // Unused ), ) - for ((templateId, interfaceId, contract) <- testData) { - val result = evaluateBeginExercise( - pkgs, - templateId, - Some(interfaceId), - contractId, - committers = Set(contractParty), - controllers = Set(contractParty), - getContract = Map(contractId -> contract), - ) + // Only use the implements case, dont support coimplements here + val (templateId, interfaceId, contract) = + (implementsTemplateId, implementsInterfaceId, implementsContract) - inside(result) { case Right(transaction) => - transaction.version shouldBe TransactionVersion.assignNodeVersion(oldVersion) - } + val result = evaluateBeginExercise( + pkgs, + templateId, + Some(interfaceId), + contractId, + committers = Set(contractParty), + controllers = Set(contractParty), + getContract = Map(contractId -> contract), + ) + + inside(result) { case Right(transaction) => + // Here its still the old version, as the interface is isn't 1.17 + transaction.version shouldBe TransactionVersion.assignNodeVersion(oldVersion) } } @@ -111,7 +112,12 @@ class TransactionVersionTest(majorLanguageVersion: LanguageMajorVersion) private[lf] class TransactionVersionTestHelpers(majorLanguageVersion: LanguageMajorVersion) { val (commonVersion, oldVersion, newVersion) = majorLanguageVersion match { - case V1 => (LanguageVersion.default, LanguageVersion.v1_15, LanguageVersion.v1_17) + case V1 => + ( + LanguageVersion.default, + LanguageVersion.v1_15, + LanguageVersion.v1_dev, + ) case V2 => ( // TODO(#17366): Use something like languageVersion.default(V2) once available