From 841716ad8f9fefdb9dd09a9066f9b555b79d72b3 Mon Sep 17 00:00:00 2001 From: Paul Brauner <141240651+paulbrauner-da@users.noreply.github.com> Date: Fri, 30 Aug 2024 12:24:23 +0200 Subject: [PATCH 1/4] cherry-pick the test parts of #19857 --------- Co-authored-by: Remy --- .../digitalasset/daml/lf/speedy/Pretty.scala | 23 ++ .../lf/speedy/SBuiltinInterfaceTest.scala | 364 +++++++++++++++--- .../daml/lf/interpretation/Error.scala | 8 + .../test/daml/upgrades/InterfaceViews.daml | 148 +++++++ .../test/daml/upgrades/Interfaces.daml | 40 +- 5 files changed, 508 insertions(+), 75 deletions(-) create mode 100644 sdk/daml-script/test/daml/upgrades/InterfaceViews.daml diff --git a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala index eea7df2a7f27..78a4d837957d 100644 --- a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala +++ b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala @@ -237,6 +237,29 @@ private[lf] object Pretty { "An optional contract field with a value of Some may not be dropped during downgrading" ) + case Dev.Upgrade.ViewMismatch( + coid, + iterfaceId, + srcTemplateId, + dstTemplateId, + srcViewValue, + dstViewValue, + ) => { + text("View mismatch when trying to upgrade the contract") & prettyContractId( + coid + ) & text("from") & prettyTypeConName(srcTemplateId) & text( + "to" + ) & prettyTypeConName( + dstTemplateId + ) & text("during a fetch or exercise by interface") / + text("Verify that the views of the contract have not changed") / + text("computed view for") & prettyTypeConName(iterfaceId) & text( + "in the source contract is" + ) & prettyValue(false)(srcViewValue) / + text("computed view for") & prettyTypeConName(iterfaceId) & text( + "in the destination contract is" + ) & prettyValue(false)(dstViewValue) + } } } } diff --git a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/SBuiltinInterfaceTest.scala b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/SBuiltinInterfaceTest.scala index bd142d692e37..830bd7a1c918 100644 --- a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/SBuiltinInterfaceTest.scala +++ b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/SBuiltinInterfaceTest.scala @@ -5,13 +5,12 @@ package com.digitalasset.daml.lf package speedy import com.digitalasset.daml.lf.data._ -import com.digitalasset.daml.lf.language.{Ast, LanguageMajorVersion} +import com.digitalasset.daml.lf.language.{Ast, LanguageMajorVersion, LanguageVersion} import com.digitalasset.daml.lf.language.Ast._ -import com.digitalasset.daml.lf.speedy.SError.SError +import com.digitalasset.daml.lf.speedy.SError.{SError, SErrorDamlException} import com.digitalasset.daml.lf.speedy.SExpr._ import com.digitalasset.daml.lf.speedy.SValue.{SValue => _, _} import com.digitalasset.daml.lf.testing.parser.Implicits.SyntaxHelper -import com.digitalasset.daml.lf.testing.parser import com.digitalasset.daml.lf.testing.parser.ParserParameters import com.digitalasset.daml.lf.transaction.{ GlobalKeyWithMaintainers, @@ -24,28 +23,160 @@ import org.scalatest.Inside import org.scalatest.freespec.AnyFreeSpec import org.scalatest.matchers.should.Matchers import org.scalatest.prop.TableDrivenPropertyChecks +import com.digitalasset.daml.lf.interpretation.{Error => IE} -import util.{Failure, Success, Try} +import scala.util.{Failure, Success, Try} -class SBuiltinInterfaceTestV2 extends SBuiltinInterfaceTest(LanguageMajorVersion.V2) +class SBuiltinInterfaceTestDefaultLf + extends SBuiltinInterfaceTest( + LanguageVersion.default, + Compiler.Config.Default(LanguageMajorVersion.V2), + ) +class SBuiltinInterfaceTestDevLf + extends SBuiltinInterfaceTest( + LanguageVersion.v2_dev, + Compiler.Config.Dev(LanguageMajorVersion.V2), + ) + +class SBuiltinInterfaceUpgradeTest extends AnyFreeSpec with Matchers with Inside { + + import EvalHelpers._ + + val alice = Ref.Party.assertFromString("Alice") -class SBuiltinInterfaceTest(majorLanguageVersion: LanguageMajorVersion) + // The following code defines a package -iface-pkg- that defines a single interface Iface. + val ifacePkgName = Ref.PackageName.assertFromString("-iface-pkg-") + val ifacePkgId = Ref.PackageId.assertFromString("-iface-package-id-") + val ifaceParserParams = ParserParameters( + defaultPackageId = ifacePkgId, + // TODO: revert to the default version once it supports upgrades + languageVersion = LanguageVersion.Features.packageUpgrades, + ) + val ifacePkg = + p"""metadata ( '$ifacePkgName' : '1.0.0' ) + module Mod { + val mkParty : Text -> Party = \(t:Text) -> + case TEXT_TO_PARTY t of None -> ERROR @Party "none" | Some x -> x; + + record @serializable MyViewType = { n : Int64 }; + interface (this : Iface) = { + viewtype Mod:MyViewType; + choice @nonConsuming MyChoice (self) (u: Unit): Unit + , controllers (Cons @Party [Mod:mkParty "Alice"] Nil @Party) + , observers (Nil @Party) + to upure @Unit (); + }; + } + """ (ifaceParserParams) + + // The following code defines a family of packages -implem-pkg- versions 1.0.0, 2.0.0, ... that define a + // template T that implements Iface. The view function for version 1 of the package returns 1, the view function + // of version 2 of the package returns 2, etc. + val implemPkgName = Ref.PackageName.assertFromString("-implem-pkg-") + def implemPkgVersion(pkgVersion: Int) = + Ref.PackageVersion.assertFromString(s"${pkgVersion}.0.0") + def implemPkgId(pkgVersion: Int) = + Ref.PackageId.assertFromString(s"-implem-pkg-id-$pkgVersion-") + def implemParserParams(pkgVersion: Int) = ParserParameters( + defaultPackageId = implemPkgId(pkgVersion), + // TODO: revert to the default version once it supports upgrades + languageVersion = LanguageVersion.Features.packageUpgrades, + ) + def implemPkg(pkgVersion: Int) = + p"""metadata ( '$implemPkgName' : '${implemPkgVersion(pkgVersion)}' ) + module Mod { + record @serializable T = { p: Party }; + template (this: T) = { + precondition True; + signatories Cons @Party [Mod:T {p} this] (Nil @Party); + observers Nil @Party; + implements '$ifacePkgId':Mod:Iface { view = '$ifacePkgId':Mod:MyViewType { n = $pkgVersion }; }; + }; + } + """ (implemParserParams(pkgVersion)) + + // All three of -iface-package-id-, -implem-pkg-id-1-, and -implem-pkg-id-2- are made available to the interpreter. + val compiledPackages = PureCompiledPackages.assertBuild( + Map( + ifacePkgId -> ifacePkg, + implemPkgId(1) -> implemPkg(1), + implemPkgId(2) -> implemPkg(2), + ), + // TODO: revert to the default compiler config once it supports upgrades + Compiler.Config.Dev(LanguageMajorVersion.V2), + ) + + // But we prefer version 2 of -implem-pkg-, which will force an upgrade of any version 1 contract when + // fetched/exercised by interface and trigger a view consistency check, which is expected to fail. + val packagePreferences = Map( + ifacePkgName -> ifacePkgId, + implemPkgName -> implemPkgId(2), + ) + + // We assume one contract of type -implem-pkg-id-1-:Mod:T on the ledger, with ID cid. + val cid = Value.ContractId.V1(crypto.Hash.hashPrivateKey("test")) + val Ast.TTyCon(tplV1Id) = t"Mod:T" (implemParserParams(1)) + val tplV1Payload = Value.ValueRecord(None, ImmArray(None -> Value.ValueParty(alice))) + val contracts = Map[Value.ContractId, Value.VersionedContractInstance]( + cid -> Versioned( + TransactionVersion.StableVersions.max, + ContractInstance(implemPkgName, Some(implemPkgVersion(1)), tplV1Id, tplV1Payload), + ) + ) + + "fetch_interface" - { + "should reject inconsistent view upgrades" in { + inside( + evalApp( + e"\(cid: ContractId Mod:Iface) -> fetch_interface @Mod:Iface cid" (ifaceParserParams), + Array(SContractId(cid), SToken), + packageResolution = packagePreferences, + getContract = contracts, + getPkg = PartialFunction.empty, + compiledPackages = compiledPackages, + committers = Set(alice), + ) + ) { case Success(Left(SErrorDamlException(IE.Dev(_, IE.Dev.Upgrade(upgradeError))))) => + upgradeError shouldBe a[IE.Dev.Upgrade.ViewMismatch] + } + } + } + + "exercise_interface" - { + "should reject inconsistent view upgrades" in { + inside( + evalApp( + e"\(cid: ContractId Mod:Iface) -> exercise_interface @Mod:Iface MyChoice cid ()" ( + ifaceParserParams + ), + Array(SContractId(cid), SToken), + packageResolution = packagePreferences, + getContract = contracts, + getPkg = PartialFunction.empty, + compiledPackages = compiledPackages, + committers = Set(alice), + ) + ) { case Success(Left(SErrorDamlException(IE.Dev(_, IE.Dev.Upgrade(upgradeError))))) => + upgradeError shouldBe a[IE.Dev.Upgrade.ViewMismatch] + } + } + } +} + +class SBuiltinInterfaceTest(languageVersion: LanguageVersion, compilerConfig: Compiler.Config) extends AnyFreeSpec with Matchers with TableDrivenPropertyChecks with Inside { - val helpers = new SBuiltinInterfaceTestHelpers(majorLanguageVersion) - import helpers.{parserParameters => _, _} - - implicit val parserParameters: ParserParameters[this.type] = - ParserParameters.defaultFor[this.type](majorLanguageVersion) - val defaultPackageId = parserParameters.defaultPackageId + import EvalHelpers._ + val helpers = new SBuiltinInterfaceTestHelpers(languageVersion, compilerConfig) + import helpers._ "Interface operations" - { - val iouTypeRep = Ref.TypeConName.assertFromString("-pkgId-:Mod:Iou") - val alice = Ref.Party.assertFromString("alice") - val bob = Ref.Party.assertFromString("bob") + val iouTypeRep = + Ref.TypeConName.assertFromString(s"${basePkgId}:Mod:Iou") + implicit val parserParameters: ParserParameters[helpers.type] = basePkgParserParams val testCases = Table[String, SValue]( "expression" -> "string-result", @@ -58,11 +189,67 @@ class SBuiltinInterfaceTest(majorLanguageVersion: LanguageMajorVersion) forEvery(testCases) { (exp, res) => s"""eval[$exp] --> "$res"""" in { - eval(e"$exp") shouldBe Success(Right(res)) + eval(e"$exp", compiledBasePkgs, Set(alice)) shouldBe Success(Right(res)) + } + } + + "exercise_interface" - { + "should prevent view errors from being caught" in { + val cid = Value.ContractId.V1(crypto.Hash.hashPrivateKey("test")) + val Ast.TTyCon(tplId) = t"ViewErrorTest:T" + val tplPayload = Value.ValueRecord(None, ImmArray(None -> Value.ValueParty(alice))) + + inside( + evalApp( + e"\(cid: ContractId I0:I0) -> ViewErrorTest:exercise_interface_and_catch_error cid", + Array(SContractId(cid), SToken), + packageResolution = pkgNameMap, + getContract = Map( + cid -> Versioned( + TransactionVersion.StableVersions.max, + ContractInstance(basePkg.pkgName, basePkg.pkgVersion, tplId, tplPayload), + ) + ), + getPkg = PartialFunction.empty, + compiledPackages = compiledBasePkgs, + committers = Set(alice), + ) + ) { case Success(Left(error)) => + // We expect the error throw by the view to not have been caught by + // fetch_interface_and_catch_error. + error shouldBe a[SErrorDamlException] + } } } "fetch_interface" - { + "should prevent view errors from being caught" in { + val cid = Value.ContractId.V1(crypto.Hash.hashPrivateKey("test")) + val Ast.TTyCon(tplId) = t"ViewErrorTest:T" + val tplPayload = Value.ValueRecord(None, ImmArray(None -> Value.ValueParty(alice))) + + inside( + evalApp( + e"\(cid: ContractId I0:I0) -> ViewErrorTest:fetch_interface_and_catch_error cid", + Array(SContractId(cid), SToken), + packageResolution = pkgNameMap, + getContract = Map( + cid -> Versioned( + TransactionVersion.StableVersions.max, + ContractInstance(basePkg.pkgName, basePkg.pkgVersion, tplId, tplPayload), + ) + ), + getPkg = PartialFunction.empty, + compiledPackages = compiledBasePkgs, + committers = Set(alice), + ) + ) { case Success(Left(error)) => + // We expect the error throw by the view to not have been caught by + // fetch_interface_and_catch_error. + error shouldBe a[SErrorDamlException] + } + } + "should request unknown package before everything else" in { val cid = Value.ContractId.V1(crypto.Hash.hashPrivateKey("test")) @@ -71,7 +258,7 @@ class SBuiltinInterfaceTest(majorLanguageVersion: LanguageMajorVersion) evalApp( e"\(cid: ContractId Mod:Iface) -> fetch_interface @Mod:Iface cid", Array(SContractId(cid), SToken), - packageResolution = basePkgNameMap, + packageResolution = pkgNameMap, getContract = Map( cid -> Versioned( TransactionVersion.StableVersions.max, @@ -84,6 +271,8 @@ class SBuiltinInterfaceTest(majorLanguageVersion: LanguageMajorVersion) ) ), getPkg = PartialFunction.empty, + compiledPackages = compiledBasePkgs, + committers = Set(alice), ) ) { case Success(result) => result shouldBe a[Right[_, _]] @@ -93,14 +282,16 @@ class SBuiltinInterfaceTest(majorLanguageVersion: LanguageMajorVersion) evalApp( e"\(cid: ContractId Mod:Iface) -> fetch_interface @Mod:Iface cid", Array(SContractId(cid), SToken), - packageResolution = basePkgNameMap, + packageResolution = pkgNameMap, getContract = Map( cid -> Versioned( TransactionVersion.StableVersions.max, - ContractInstance(extraPkgName, extraPkgVersion, extraIouId, iouPayload), + ContractInstance(extraPkg.pkgName, extraPkg.pkgVersion, extraIouId, iouPayload), ) ), getPkg = PartialFunction.empty, + compiledPackages = compiledBasePkgs, + committers = Set(alice), ) ) { case Failure(err) => err shouldBe a[SpeedyTestLib.UnknownPackage] @@ -110,16 +301,18 @@ class SBuiltinInterfaceTest(majorLanguageVersion: LanguageMajorVersion) evalApp( e"\(cid: ContractId Mod:Iface) -> fetch_interface @Mod:Iface cid", Array(SContractId(cid), SToken), - packageResolution = basePkgNameMap, + packageResolution = pkgNameMap, getContract = Map( cid -> Versioned( TransactionVersion.StableVersions.max, - ContractInstance(extraPkgName, extraPkgVersion, extraIouId, iouPayload), + ContractInstance(extraPkg.pkgName, extraPkg.pkgVersion, extraIouId, iouPayload), ) ), getPkg = { case `extraPkgId` => compiledExtendedPkgs }, + compiledPackages = compiledBasePkgs, + committers = Set(alice), ) ) { case Success(result) => result shouldBe a[Right[_, _]] @@ -130,20 +323,67 @@ class SBuiltinInterfaceTest(majorLanguageVersion: LanguageMajorVersion) } } -final class SBuiltinInterfaceTestHelpers(majorLanguageVersion: LanguageMajorVersion) { - - import SpeedyTestLib.loggingContext +final class SBuiltinInterfaceTestHelpers( + val languageVersion: LanguageVersion, + val compilerConfig: Compiler.Config, +) { val alice = Ref.Party.assertFromString("Alice") val bob = Ref.Party.assertFromString("Bob") - implicit val parserParameters: ParserParameters[this.type] = - ParserParameters.defaultFor[this.type](majorLanguageVersion) - val basePkgId = parserParameters.defaultPackageId - val compilerConfig = Compiler.Config.Default(majorLanguageVersion) + val basePkgId = Ref.PackageId.assertFromString("-base-package-id-") + val basePkgParserParams: ParserParameters[this.type] = + ParserParameters(defaultPackageId = basePkgId, languageVersion = languageVersion) lazy val basePkg = - p""" metadata ( 'basic-package' : '1.0.0' ) + p""" metadata ( '-base-package-' : '1.0.0' ) + module I0 { + interface (this: I0) = { + viewtype Mod:MyUnit; + choice @nonConsuming MyChoice (self) (u: Unit): Unit + , controllers (Nil @Party) + , observers (Nil @Party) + to upure @Unit (); + coimplements T_Co0_No1:T_Co0_No1 { view = Mod:MyUnit {}; }; + coimplements T_Co0_Co1:T_Co0_Co1 { view = Mod:MyUnit {}; }; + }; + } + + module ViewErrorTest { + val mkParty : Text -> Party = \(t:Text) -> case TEXT_TO_PARTY t of None -> ERROR @Party "none" | Some x -> x; + val alice : Party = ViewErrorTest:mkParty "Alice"; + record @serializable T = { party: Party }; + + record @serializable Ex = { message: Text } ; + exception Ex = { + message \(e: ViewErrorTest:Ex) -> ViewErrorTest:Ex {message} e + }; + + template (this: T) = { + precondition True; + signatories Cons @Party [ViewErrorTest:T {party} this] (Nil @Party); + observers (Nil @Party); + implements I0:I0 { view = throw @Mod:MyUnit @ViewErrorTest:Ex (ViewErrorTest:Ex { message = "user error" }); }; + }; + + val fetch_interface_and_catch_error : ContractId I0:I0 -> Update Unit = + \(cid: ContractId I0:I0) -> + try @Unit + ubind _:I0:I0 <- fetch_interface @I0:I0 cid + in upure @Unit () + catch + e -> Some @(Update Unit) (upure @Unit ()) + ; + + val exercise_interface_and_catch_error : ContractId I0:I0 -> Update Unit = + \(cid: ContractId I0:I0) -> + try @Unit + exercise_interface @I0:I0 MyChoice cid () + catch + e -> Some @(Update Unit) (upure @Unit ()) + ; + } + module Mod { record @serializable MyUnit = {}; @@ -161,34 +401,25 @@ final class SBuiltinInterfaceTestHelpers(majorLanguageVersion: LanguageMajorVers }; val mkParty : Text -> Party = \(t:Text) -> case TEXT_TO_PARTY t of None -> ERROR @Party "none" | Some x -> x; - val alice : Party = Mod:mkParty "alice"; - val bob : Party = Mod:mkParty "bob"; + val alice : Party = Mod:mkParty "Alice"; + val bob : Party = Mod:mkParty "Bob"; val aliceOwesBob : Mod:Iou = Mod:Iou { i = Mod:alice, u = Mod:bob, name = "alice owes bob" }; val aliceOwesBobIface : Mod:Iface = to_interface @Mod:Iface @Mod:Iou Mod:aliceOwesBob; } - """ + """ (basePkgParserParams) val basePkgs = Map(basePkgId -> basePkg) lazy val compiledBasePkgs = PureCompiledPackages.assertBuild(basePkgs, compilerConfig) - val Ast.TTyCon(iouId) = t"'$basePkgId':Mod:Iou" + val Ast.TTyCon(iouId) = t"'$basePkgId':Mod:Iou" (basePkgParserParams) - val extraPkgName = Ref.PackageName.assertFromString("-extra-package-name-") - val extraPkgVersion = Some(Ref.PackageVersion.assertFromString("2.0.0")) val extraPkgId = Ref.PackageId.assertFromString("-extra-package-id-") + val extraPkgParserParams = basePkgParserParams.copy(defaultPackageId = extraPkgId) require(extraPkgId != basePkgId) - val basePkgNameMap = - Map(basePkg.pkgName -> basePkgId, extraPkgName -> extraPkgId) - - lazy val extendedPkgs = { - - val modifiedParserParameters: parser.ParserParameters[this.type] = - parserParameters.copy(defaultPackageId = extraPkgId) - - val pkg = p""" metadata ( 'extended-pkg' : '1.0.0' ) + val extraPkg = p""" metadata ( '-extra-package-name-' : '1.0.0' ) module Mod { record @serializable Iou = { i: Party, u: Party, name: Text }; @@ -201,16 +432,21 @@ final class SBuiltinInterfaceTestHelpers(majorLanguageVersion: LanguageMajorVers }; val mkParty : Text -> Party = \(t:Text) -> case TEXT_TO_PARTY t of None -> ERROR @Party "none" | Some x -> x; - val alice : Party = Mod:mkParty "alice"; - val bob : Party = Mod:mkParty "bob"; + val alice : Party = Mod:mkParty "Alice"; + val bob : Party = Mod:mkParty "Bob"; } - """ (modifiedParserParameters) - basePkgs + (modifiedParserParameters.defaultPackageId -> pkg) - } - lazy val compiledExtendedPkgs = PureCompiledPackages.assertBuild(extendedPkgs, compilerConfig) + """ (extraPkgParserParams) + + lazy val compiledExtendedPkgs = + PureCompiledPackages.assertBuild(basePkgs + (extraPkgId -> extraPkg), compilerConfig) - val Ast.TTyCon(extraIouId) = t"'$extraPkgId':Mod:Iou" + val pkgNameMap = Map( + basePkg.pkgName -> basePkgId, + extraPkg.pkgName -> extraPkgId, + ) + + val Ast.TTyCon(extraIouId) = t"Mod:Iou" (extraPkgParserParams) val iouPayload = Value.ValueRecord( @@ -221,14 +457,24 @@ final class SBuiltinInterfaceTestHelpers(majorLanguageVersion: LanguageMajorVers None -> Value.ValueText("name"), ), ) +} - def eval(e: Expr): Try[Either[SError, SValue]] = +object EvalHelpers { + import SpeedyTestLib.loggingContext + + def eval( + e: Expr, + compiledPackages: PureCompiledPackages, + committers: Set[Ref.Party], + ): Try[Either[SError, SValue]] = evalSExpr( - compiledBasePkgs.compiler.unsafeCompile(e), + compiledPackages.compiler.unsafeCompile(e), Map.empty, PartialFunction.empty, PartialFunction.empty, PartialFunction.empty, + compiledPackages, + committers, ) def evalApp( @@ -239,13 +485,17 @@ final class SBuiltinInterfaceTestHelpers(majorLanguageVersion: LanguageMajorVers getContract: PartialFunction[Value.ContractId, Value.VersionedContractInstance] = PartialFunction.empty, getKey: PartialFunction[GlobalKeyWithMaintainers, Value.ContractId] = PartialFunction.empty, + compiledPackages: PureCompiledPackages, + committers: Set[Ref.Party], ): Try[Either[SError, SValue]] = evalSExpr( - SEApp(compiledBasePkgs.compiler.unsafeCompile(e), args), + SEApp(compiledPackages.compiler.unsafeCompile(e), args), packageResolution, getPkg, getContract, getKey, + compiledPackages, + committers, ) def evalSExpr( @@ -254,14 +504,16 @@ final class SBuiltinInterfaceTestHelpers(majorLanguageVersion: LanguageMajorVers getPkg: PartialFunction[Ref.PackageId, CompiledPackages] = PartialFunction.empty, getContract: PartialFunction[Value.ContractId, Value.VersionedContractInstance], getKey: PartialFunction[GlobalKeyWithMaintainers, Value.ContractId], + compiledPackages: PureCompiledPackages, + committers: Set[Ref.Party], ): Try[Either[SError, SValue]] = { val machine = Speedy.Machine.fromUpdateSExpr( - compiledBasePkgs, + compiledPackages, packageResolution = packageResolution, transactionSeed = crypto.Hash.hashPrivateKey("SBuiltinTest"), updateSE = SELet1(e, SEMakeClo(Array(SELocS(1)), 1, SELocF(0))), - committers = Set(alice), + committers = committers, ) Try(SpeedyTestLib.run(machine, getPkg, getContract, getKey)) } diff --git a/sdk/daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/interpretation/Error.scala b/sdk/daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/interpretation/Error.scala index 3b27e69f58ac..129decaef384 100644 --- a/sdk/daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/interpretation/Error.scala +++ b/sdk/daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/interpretation/Error.scala @@ -177,6 +177,14 @@ object Error { final case class DowngradeDropDefinedField(expectedType: Ast.Type, actualValue: Value) extends Error + final case class ViewMismatch( + coid: ContractId, + iterfaceId: TypeConName, + srcTemplateId: TypeConName, + dstTemplateId: TypeConName, + srcView: Value, + dstView: Value, + ) extends Error } /** A choice guard returned false, invalidating some expectation. */ diff --git a/sdk/daml-script/test/daml/upgrades/InterfaceViews.daml b/sdk/daml-script/test/daml/upgrades/InterfaceViews.daml new file mode 100644 index 000000000000..96818429b2cc --- /dev/null +++ b/sdk/daml-script/test/daml/upgrades/InterfaceViews.daml @@ -0,0 +1,148 @@ +-- Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +-- SPDX-License-Identifier: Apache-2.0 + +{-# LANGUAGE ApplicativeDo #-} + +module InterfaceViews (main) where + +import UpgradeTestLib +import FixedInterface +import FixedInterfaceCaller +import qualified V1.Interfaces as V1 +import qualified V2.Interfaces as V2 +import PackageIds +import DA.Foldable +import DA.Optional +import DA.Text + +-- Fixed package containing only the interface +{- PACKAGE +name: fixed-interface-view +versions: 1 +-} + +{- MODULE +package: fixed-interface-view +contents: | + module FixedInterface where + + data IV = IV with + owner : Party + payload : Int + + interface I where + viewtype IV + + getVersion : Text + nonconsuming choice GetVersion : Text + controller (view this).owner + do + pure $ getVersion this +-} + +-- Another fixed package containing a helper template for exercising the interface in a choice +{- PACKAGE +name: fixed-interface-view-caller +versions: 1 +depends: fixed-interface-view-1.0.0 +-} + +{- MODULE +package: fixed-interface-view-caller +contents: | + module FixedInterfaceCaller where + + import FixedInterface + + template Caller with + party : Party + where + signatory party + + choice CallInterface : Text with + icid : ContractId I + controller party + do + exercise icid GetVersion +-} + +-- The versioned/upgraded package +{- PACKAGE +name: interface-views +versions: 2 +depends: fixed-interface-view-1.0.0 +-} + +{- MODULE +package: interface-views +contents: | + module Interfaces where + + import FixedInterface + import DA.Optional + + template FITemplate with + party : Party + where + signatory party + + interface instance I for FITemplate where + view = IV party 0 -- @V 1 + view = IV party 1 -- @V 2 + getVersion = "V1" -- @V 1 + getVersion = "V2" -- @V 2 + + -- Following two choices exist here for convenience. They could be in a separate package which depends on this, + -- but additional unnecessary packages simply wastes time. + choice FetchFromInterface : Int with + icid : ContractId I + controller party + do + (_, res) <- fromSomeNote "Failed to fetch contract" <$> fetchFromInterface @FITemplate icid + let v = view $ toInterface @I res + pure v.payload + + choice FetchInterface : () with + icid : ContractId I + controller party + do + fetch icid + pure () +-} + +main : TestTree +main = tests + [ ("Calling an interface choice at command level fails as intended when the view is modified", exerciseCommandShouldFail) + , ("Calling an interface choice in choice body fails as intended when the view is modified", exerciseInChoiceBodyShouldFail) + , ("fetchFromInterface fails as intended when the view is modified", fetchFromInterfaceShouldFail) + ] + +setupAliceAndInterface : Script (Party, ContractId I) +setupAliceAndInterface = do + alice <- allocateParty "alice" + cid <- alice `submit` createExactCmd V1.FITemplate with party = alice + pure (alice, toInterfaceContractId @I cid) + +exerciseCommandShouldFail : Test +exerciseCommandShouldFail = test $ do + (alice, icid) <- setupAliceAndInterface + res <- alice `trySubmit` exerciseCmd icid GetVersion + case res of + Left (DevError Upgrade msg) | "View mismatch" `isInfixOf` msg -> pure () + _ -> assertFail ("Expected fetchFromInterface to fail, got " <> show res) + +exerciseInChoiceBodyShouldFail : Test +exerciseInChoiceBodyShouldFail = test $ do + (alice, icid) <- setupAliceAndInterface + res <- alice `trySubmit` createAndExerciseCmd (Caller with party = alice) (CallInterface with icid = icid) + case res of + Left (DevError Upgrade msg) | "View mismatch" `isInfixOf` msg -> pure () + _ -> assertFail ("Expected fetchFromInterface to fail, got " <> show res) + +fetchFromInterfaceShouldFail : Test +fetchFromInterfaceShouldFail = test $ do + (alice, icid) <- setupAliceAndInterface + res <- alice `trySubmit` createAndExerciseExactCmd (V2.FITemplate with party = alice) (V2.FetchFromInterface with icid = icid) + case res of + Left (DevError Upgrade msg) | "View mismatch" `isInfixOf` msg -> pure () + _ -> assertFail ("Expected fetchFromInterface to fail, got " <> show res) diff --git a/sdk/daml-script/test/daml/upgrades/Interfaces.daml b/sdk/daml-script/test/daml/upgrades/Interfaces.daml index 338b3f5c798f..2c7e81606b87 100644 --- a/sdk/daml-script/test/daml/upgrades/Interfaces.daml +++ b/sdk/daml-script/test/daml/upgrades/Interfaces.daml @@ -30,7 +30,7 @@ contents: | data IV = IV with owner : Party - version : Text + payload : Optional Int interface I where viewtype IV @@ -85,24 +85,25 @@ contents: | template FITemplate with party : Party + payload : Optional Int -- @V 2 where signatory party interface instance I for FITemplate where - view = IV party "V1" -- @V 1 - view = IV party "V2" -- @V 2 - getVersion = "V1" -- @V 1 - getVersion = "V2" -- @V 2 + view = IV party None -- @V 1 + view = IV party payload -- @V 2 + getVersion = "V1" -- @V 1 + getVersion = "V2" -- @V 2 -- Following two choices exist here for convenience. They could be in a separate package which depends on this, -- but additional unnecessary packages simply wastes time. - choice FetchFromInterface : Text with + choice FetchFromInterface : Optional Int with icid : ContractId I controller party do (_, res) <- fromSomeNote "Failed to fetch contract" <$> fetchFromInterface @FITemplate icid let v = view $ toInterface @I res - pure v.version + pure v.payload choice FetchInterface : () with icid : ContractId I @@ -128,14 +129,15 @@ contents: | template FITemplateAlt with party : Party + payload : Optional Int -- @V 2 where signatory party interface instance I for FITemplateAlt where - view = IV party "V1" -- @V 1 - view = IV party "V2" -- @V 2 - getVersion = "V1" -- @V 1 - getVersion = "V2" -- @V 2 + view = IV party None -- @V 1 + view = IV party payload -- @V 2 + getVersion = "V1" -- @V 1 + getVersion = "V2" -- @V 2 -} interfacesV1 : PackageId @@ -159,8 +161,8 @@ main = tests , ("Package map preference is used for interface implementation selection at choice body level", interfaceChoicePackagePreferenceUpdate) , ("Multiple interface exercises use their respective package preference entries at command level", multipleInterfaceChoicePackagePreferenceCommand) , ("Multiple interface exercises use their respective package preference entries at choice body level", multipleInterfaceChoicePackagePreferenceUpdate) - , broken ("queryInterfaceContractId uses highest version of view", queryInterfaceUpgrades) - , brokenOnIDELedger ("fetchFromInterface upgrades payload to match request", fetchFromInterfaceUpgrades) + , ("queryInterfaceContractId works as intended in the presence of upgraded packages", queryInterfaceUpgrades) + , ("fetchFromInterface works as intended in the presence of upgraded packages", fetchFromInterfaceUpgrades) -- IDE Ledger doesn't support unvetting -- Can't unvet old version of package on 3x, even though v2 exists. , broken ("Can fetch interface from V1 contract when V1 unvetted", fetchWithoutV1) @@ -210,7 +212,7 @@ multipleInterfaceChoicePackagePreference doExercise = test $ do let icid = toInterfaceContractId @I cid icidAlt = toInterfaceContractId @I cidAlt -- Tests all 4 combinations of 2 versions for 2 packages - cases = + cases = [ (interfacesVersion, interfacesVersionStr, interfacesAltVersion, interfacesAltVersionStr) | (interfacesVersion, interfacesVersionStr) <- [(interfacesV1, "V1"), (interfacesV2, "V2")] , (interfacesAltVersion, interfacesAltVersionStr) <- [(interfacesAltV1, "V1"), (interfacesAltV2, "V2")] @@ -239,15 +241,15 @@ queryInterfaceUpgrades : Test queryInterfaceUpgrades = test $ do (alice, icid) <- setupAliceAndInterface res <- fromSomeNote "Failed to find contract" <$> alice `queryInterfaceContractId` icid - res.version === "V2" + res.payload === None fetchFromInterfaceUpgrades : Test fetchFromInterfaceUpgrades = test $ do (alice, icid) <- setupAliceAndInterface - resV1 <- alice `submit` createAndExerciseExactCmd (V1.FITemplate with party = alice) (V1.FetchFromInterface with icid = icid) - resV1 === "V1" - resV2 <- alice `submit` createAndExerciseExactCmd (V2.FITemplate with party = alice) (V2.FetchFromInterface with icid = icid) - resV2 === "V2" + resV1 <- alice `submit` createAndExerciseExactCmd (V1.FITemplate with party = alice) (V1.FetchFromInterface with cid = icid) + resV1 === None + resV2 <- alice `submit` createAndExerciseExactCmd (V2.FITemplate with party = alice, payload = None) (V2.FetchFromInterface with cid = icid) + resV2 === None fetchWithoutV1 : Test fetchWithoutV1 = test $ do From 610121092a511278b6101574ea425f05f59aa60b Mon Sep 17 00:00:00 2001 From: Paul Brauner Date: Wed, 18 Sep 2024 09:02:07 +0200 Subject: [PATCH 2/4] manually cherry-pick the rest --- .../daml/lf/speedy/Compiler.scala | 127 +++----- .../digitalasset/daml/lf/speedy/Pretty.scala | 1 - .../daml/lf/speedy/SBuiltinFun.scala | 297 +++++++++++++----- .../test/daml/upgrades/Interfaces.daml | 6 +- 4 files changed, 261 insertions(+), 170 deletions(-) diff --git a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Compiler.scala b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Compiler.scala index 6d78e7613a34..36b2a4494559 100644 --- a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Compiler.scala +++ b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Compiler.scala @@ -555,66 +555,51 @@ private[lf] final class Compiler( cidPos: Position, choiceArgPos: Position, tokenPos: Position, - ): s.SExpr = - let( - env, - SBSoftFetchInterface( - env.toSEVar(cidPos), - s.SEValue.None, - ), - ) { (payloadPos, env) => - let( - env, - SBCastAnyInterface(ifaceId)( - env.toSEVar(cidPos), + ): s.SExpr = { + let(env, SBFetchInterface(ifaceId)(env.toSEVar(cidPos))) { (payloadPos, _env) => + val env = + _env.bindExprVar(param, payloadPos).bindExprVar(choice.argBinder._1, choiceArgPos) + let(env, SBExtractSAnyValue(env.toSEVar(payloadPos))) { (castPos, env) => + // We use a chain of let bindings to make the evaluation order of SBResolveSBUBeginExercise's arguments + // is independent from the evaluation strategy imposed by the ANF transformation. + val applyChoiceGuardExpr = SBApplyChoiceGuard(choice.name, Some(ifaceId))( + env.toSEVar(guardPos), env.toSEVar(payloadPos), - ), - ) { (castPos, env) => - let( - env, - s.SEPreventCatch(SBViewInterface(ifaceId)(env.toSEVar(payloadPos))), - ) { (_, _env) => - val env = - _env.bindExprVar(param, payloadPos).bindExprVar(choice.argBinder._1, choiceArgPos) - // We use a chain of let bindings to make the evaluation order of SBResolveSBUBeginExercise's arguments - // is independent from the evaluation strategy imposed by the ANF transformation. - val applyChoiceGuardExpr = SBApplyChoiceGuard(choice.name, Some(ifaceId))( - env.toSEVar(guardPos), - env.toSEVar(payloadPos), - env.toSEVar(cidPos), - ) - let(env, applyChoiceGuardExpr) { (_, env) => - val controllersExpr = s.SEPreventCatch(translateExp(env, choice.controllers)) - let(env, controllersExpr) { (controllersPos, env) => - val observersExpr = choice.choiceObservers match { - case Some(observers) => s.SEPreventCatch(translateExp(env, observers)) + env.toSEVar(cidPos), + ) + let(env, applyChoiceGuardExpr) { (_, env) => + val controllersExpr = s.SEPreventCatch(translateExp(env, choice.controllers)) + let(env, controllersExpr) { (controllersPos, env) => + val observersExpr = choice.choiceObservers match { + case Some(observers) => s.SEPreventCatch(translateExp(env, observers)) + case None => s.SEValue.EmptyList + } + let(env, observersExpr) { (observersPos, env) => + val authorizersExpr = choice.choiceAuthorizers match { + case Some(authorizers) => s.SEPreventCatch(translateExp(env, authorizers)) case None => s.SEValue.EmptyList } - let(env, observersExpr) { (observersPos, env) => - val authorizersExpr = choice.choiceAuthorizers match { - case Some(authorizers) => s.SEPreventCatch(translateExp(env, authorizers)) - case None => s.SEValue.EmptyList - } - let(env, authorizersExpr) { (authorizersPos, env) => - val exerciseExpr = SBResolveSBUBeginExercise( - interfaceId = ifaceId, - choiceName = choice.name, - consuming = choice.consuming, - byKey = false, - explicitChoiceAuthority = choice.choiceAuthorizers.isDefined, - )( - env.toSEVar(payloadPos), - env.toSEVar(choiceArgPos), - env.toSEVar(cidPos), - env.toSEVar(controllersPos), - env.toSEVar(observersPos), - env.toSEVar(authorizersPos), - env.toSEVar(castPos), + let(env, authorizersExpr) { (authorizersPos, env) => + val exerciseExpr = SBResolveSBUBeginExercise( + interfaceId = ifaceId, + choiceName = choice.name, + consuming = choice.consuming, + byKey = false, + explicitChoiceAuthority = choice.choiceAuthorizers.isDefined, + )( + env.toSEVar(payloadPos), + env.toSEVar(choiceArgPos), + env.toSEVar(cidPos), + env.toSEVar(controllersPos), + env.toSEVar(observersPos), + env.toSEVar(authorizersPos), + env.toSEVar(castPos), + ) + let(env, exerciseExpr) { (_, _env) => + val env = _env.bindExprVar(choice.selfBinder, cidPos) + s.SEScopeExercise( + app(translateExp(env, choice.update), env.toSEVar(tokenPos)) ) - let(env, exerciseExpr) { (_, _env) => - val env = _env.bindExprVar(choice.selfBinder, cidPos) - s.SEScopeExercise(app(translateExp(env, choice.update), env.toSEVar(tokenPos))) - } } } } @@ -622,6 +607,7 @@ private[lf] final class Compiler( } } } + } private[this] def compileInterfaceChoice( ifaceId: TypeConName, @@ -756,35 +742,16 @@ private[lf] final class Compiler( ifaceId: Identifier ): (t.SDefinitionRef, SDefinition) = topLevelFunction2(t.FetchInterfaceDefRef(ifaceId)) { (cidPos, _, env) => - let( - env, - SBSoftFetchInterface( - env.toSEVar(cidPos), - s.SEValue.None, - ), - ) { (payloadPos, env) => + let(env, SBFetchInterface(ifaceId)(env.toSEVar(cidPos))) { (payloadPos, env) => let( env, - SBCastAnyInterface(ifaceId)( - env.toSEVar(cidPos), + SBResolveSBUInsertFetchNode(ifaceId)( env.toSEVar(payloadPos), + env.toSEVar(cidPos), + s.SEValue.None, ), ) { (_, env) => - let( - env, - s.SEPreventCatch(SBViewInterface(ifaceId)(env.toSEVar(payloadPos))), - ) { (_, env) => - let( - env, - SBResolveSBUInsertFetchNode(ifaceId)( - env.toSEVar(payloadPos), - env.toSEVar(cidPos), - s.SEValue.None, - ), - ) { (_, env) => - env.toSEVar(payloadPos) - } - } + env.toSEVar(payloadPos) } } } diff --git a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala index 78a4d837957d..5ca1c699a135 100644 --- a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala +++ b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala @@ -628,7 +628,6 @@ private[lf] object Pretty { ) case SBUCreate(id) => text(s"$$create($id)") case SBFetchTemplate(templateId) => text(s"$$fetchAny($templateId)") - case SBSoftFetchInterface => text(s"$$softFetchInterface") case SBUGetTime | SBSGetTime => text("$getTime") case _ => str(x) } diff --git a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltinFun.scala b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltinFun.scala index 54928774b377..a983419da34e 100644 --- a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltinFun.scala +++ b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltinFun.scala @@ -4,19 +4,19 @@ package com.digitalasset.daml.lf package speedy -import java.util +import com.daml.nameof.NameOf +import com.daml.scalautil.Statement.discard +import com.digitalasset.daml.lf.data.Numeric.Scale import com.digitalasset.daml.lf.data.Ref._ import com.digitalasset.daml.lf.data._ -import com.digitalasset.daml.lf.data.Numeric.Scale import com.digitalasset.daml.lf.interpretation.{Error => IE} import com.digitalasset.daml.lf.language.Ast import com.digitalasset.daml.lf.speedy.ArrayList.Implicits._ import com.digitalasset.daml.lf.speedy.SError._ import com.digitalasset.daml.lf.speedy.SExpr._ -import com.digitalasset.daml.lf.speedy.Speedy._ -import com.digitalasset.daml.lf.speedy.{SExpr0 => compileTime} -import com.digitalasset.daml.lf.speedy.{SExpr => runTime} import com.digitalasset.daml.lf.speedy.SValue.{SValue => SV, _} +import com.digitalasset.daml.lf.speedy.Speedy._ +import com.digitalasset.daml.lf.speedy.{SExpr => runTime, SExpr0 => compileTime} import com.digitalasset.daml.lf.transaction.TransactionErrors.{ AuthFailureDuringExecution, DuplicateContractId, @@ -31,12 +31,11 @@ import com.digitalasset.daml.lf.transaction.{ TransactionErrors => TxErr, } import com.digitalasset.daml.lf.value.{Value => V} -import com.daml.nameof.NameOf -import com.daml.scalautil.Statement.discard +import java.util import scala.annotation.nowarn -import scala.jdk.CollectionConverters._ 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. @@ -1098,46 +1097,150 @@ private[lf] object SBuiltinFun { ): Boolean = getInterfaceInstance(machine, interfaceId, templateId).nonEmpty - final case class SBCastAnyInterface(ifaceId: TypeConName) extends SBuiltinFun(2) { - override private[speedy] def execute[Q]( + // Precondition: the package of tplId is loaded in the machine + private[this] def ensureTemplateImplementsInterface[Q]( + machine: Machine[_], + ifaceId: TypeConName, + coid: V.ContractId, + tplId: TypeConName, + )(k: => Control[Q]): Control[Q] = { + if (!interfaceInstanceExists(machine, ifaceId, tplId)) { + Control.Error(IE.ContractDoesNotImplementInterface(ifaceId, coid, tplId)) + } else { + k + } + } + + final case object SBExtractSAnyValue extends UpdateBuiltin(1) { + override protected def executeUpdate( args: util.ArrayList[SValue], - machine: Machine[Q], - ): Control[Nothing] = { - def coid = getSContractId(args, 0) - val (actualTmplId, record) = getSAnyContract(args, 1) - if (!interfaceInstanceExists(machine, ifaceId, actualTmplId)) { - Control.Error(IE.ContractDoesNotImplementInterface(ifaceId, coid, actualTmplId)) - } else { - Control.Value(record) - } + machine: UpdateMachine, + ): Control[Question.Update] = { + val (_, record) = getSAnyContract(args, 0) + Control.Value(record) } } - /** $fetchAny[T] - * :: ContractId a - * -> Optional {key: key, maintainers: List Party} (template key, if present) - * -> a + /** Fetches the requested contract ID, casts its to the requested interface, computes its view and returns it as an + * SAny. In addition, if [[soft]] is true, then upgrades the contract to the preferred template version for the same + * package name, and compares its computed view to that of the old contract. If the two views agree then the upgraded + * contract is cached and returned. */ - - final case class SBFetchTemplate(templateId: TypeConName) extends UpdateBuiltin(2) { + final case class SBFetchInterface(interfaceId: TypeConName) extends UpdateBuiltin(1) { override protected def executeUpdate( args: util.ArrayList[SValue], machine: UpdateMachine, ): Control[Question.Update] = { val coid = getSContractId(args, 0) - val keyOpt = args.get(1) - fetchTemplate(machine, templateId, coid, keyOpt)(Control.Value) + fetchInterface(machine, coid, interfaceId)(Control.Value) + } + } + + /** Fetches the requested contract ID, upgrades it to the preferred template version for the same package name, + * and compares the computed views according to the old and the new versions. If the two views agree then caches + * the upgraded contract and returns it (via the continuation) as an SAny. + */ + private[this] def fetchInterface( + machine: UpdateMachine, + coid: V.ContractId, + interfaceId: TypeConName, + )(k: SAny => Control[Question.Update]): Control[Question.Update] = { + // Continuation called by two different branches of the expression below. Factorized out to avoid duplication. + def cacheContractAndReturnAny( + machine: UpdateMachine, + coid: V.ContractId, + dstTplId: Ref.ValueRef, + dstArg: SValue, + )(k: SAny => Control[Question.Update]): Control[Question.Update] = { + // ensure the contract and its metadata are cached + getContractInfo( + machine, + coid, + dstTplId, + dstArg, + SValue.SValue.None, + ) { _ => + k(SAny(Ast.TTyCon(dstTplId), dstArg)) + } + } + + hardFetchTemplate(machine, coid, SValue.SValue.None) { (pkgName, srcTplId, srcArg) => + ensureTemplateImplementsInterface(machine, interfaceId, coid, srcTplId) { + viewInterface(machine, interfaceId, srcTplId, srcArg) { srcView => + resolvePackageName(machine, pkgName) { pkgId => + val dstTplId = srcTplId.copy(packageId = pkgId) + machine.ensurePackageIsLoaded( + dstTplId.packageId, + language.Reference.Template(dstTplId), + ) { () => + ensureTemplateImplementsInterface(machine, interfaceId, coid, dstTplId) { + fromInterface(machine, srcTplId, srcArg, dstTplId) { + case None => + Control.Error(IE.WronglyTypedContract(coid, dstTplId, srcTplId)) + case Some(dstArg) => + viewInterface(machine, interfaceId, dstTplId, dstArg) { dstView => + executeExpression(machine, SEPreventCatch(srcView)) { srcViewValue => + // If the destination and src templates are the same, we skip the computation + // of the destination template's view. + if (dstTplId == srcTplId) + cacheContractAndReturnAny(machine, coid, dstTplId, dstArg)(k) + else + executeExpression(machine, SEPreventCatch(dstView)) { dstViewValue => + if (srcViewValue != dstViewValue) { + Control.Error( + IE.Dev( + NameOf.qualifiedNameOfCurrentFunc, + IE.Dev.Upgrade( + IE.Dev.Upgrade.ViewMismatch( + coid, + interfaceId, + srcTplId, + dstTplId, + srcView = srcViewValue.toUnnormalizedValue, + dstView = dstViewValue.toUnnormalizedValue, + ) + ), + ) + ) + } else + cacheContractAndReturnAny(machine, coid, dstTplId, dstArg)(k) + } + } + } + } + } + } + } + } + } } } - final case object SBSoftFetchInterface extends UpdateBuiltin(2) { + private[this] def resolvePackageName[Q](machine: UpdateMachine, pkgName: Ref.PackageName)( + k: PackageId => Control[Q] + ): Control[Q] = { + machine.packageResolution.get(pkgName) match { + // TODO https://github.com/digital-asset/daml/issues/17995 + // We need a proper interpretation error here + case None => crash(s"cannot resolve package $pkgName") + case Some(pkgId) => k(pkgId) + } + } + + /** $fetchTemplate[T] + * :: ContractId a + * -> Optional {key: key, maintainers: List Party} (template key, if present) + * -> a + */ + + final case class SBFetchTemplate(templateId: TypeConName) extends UpdateBuiltin(2) { override protected def executeUpdate( args: util.ArrayList[SValue], machine: UpdateMachine, ): Control[Question.Update] = { val coid = getSContractId(args, 0) val keyOpt = args.get(1) - softFetchInterface(machine, coid, keyOpt)(Control.Value) + fetchTemplate(machine, templateId, coid, keyOpt)(Control.Value) } } @@ -1268,32 +1371,42 @@ private[lf] object SBuiltinFun { // by an SAny wrapping the underlying template, we need to check that the SAny type constructor // matches the template type, and then return the SAny internal value. final case class SBFromInterface( - tplId: TypeConName + dstTplId: TypeConName ) extends SBuiltinFun(1) { override private[speedy] def execute[Q]( args: util.ArrayList[SValue], machine: Machine[Q], ): Control[Q] = { - val (tyCon, record) = getSAnyContract(args, 0) + val (srcTplId, srcArg) = getSAnyContract(args, 0) + fromInterface(machine, srcTplId, srcArg, dstTplId) { dstArg => + Control.Value(SOptional(dstArg)) + } + } + } - if (tplId == tyCon) { - Control.Value(SOptional(Some(record))) - } else if (tplId.qualifiedName == tyCon.qualifiedName) { - val (tplIdPkgName, _) = machine.tmplId2PackageNameVersion(tplId) - val (tyConPkgName, _) = machine.tmplId2PackageNameVersion(tyCon) - if (tplIdPkgName == tyConPkgName) { + private[this] def fromInterface[Q]( + machine: Machine[Q], + srcTplId: TypeConName, + srcArg: SRecord, + dstTplId: TypeConName, + )(k: Option[SValue] => Control[Q]): Control[Q] = { + if (dstTplId == srcTplId) { + k(Some(srcArg)) + } else if (dstTplId.qualifiedName == srcTplId.qualifiedName) { + val (srcPkgName, _) = machine.tmplId2PackageNameVersion(dstTplId) + val (dstPkgName, _) = machine.tmplId2PackageNameVersion(srcTplId) + if (srcPkgName == dstPkgName) { // This isn't ideal as its a large uncached computation in a non Update primative. // Ideally this would run in Update, and not iterate the value twice // i.e. using an upgrade transformation function directly on SValues - importValue(machine, tplId, record.toUnnormalizedValue) { templateArg => - Control.Value(SOptional(Some(templateArg))) - } - } else { - Control.Value(SOptional(None)) + importValue(machine, dstTplId, srcArg.toUnnormalizedValue) { templateArg => + k(Some(templateArg)) } } else { - Control.Value(SOptional(None)) + k(None) } + } else { + k(None) } } @@ -1392,19 +1505,27 @@ private[lf] object SBuiltinFun { override private[speedy] def execute[Q]( args: util.ArrayList[SValue], machine: Machine[Q], - ): Control.Expression = { + ): Control[Nothing] = { val (templateId, record) = getSAnyContract(args, 0) - val ref = getInterfaceInstance(machine, ifaceId, templateId).fold( - crash( - s"Attempted to call view for interface ${ifaceId} on a wrapped " + - s"template of type ${ifaceId}, but there's no matching interface instance." - ) - )(iiRef => InterfaceInstanceViewDefRef(iiRef)) - val e = SEApp(SEVal(ref), Array(record)) - Control.Expression(e) + viewInterface(machine, ifaceId, templateId, record)(Control.Expression) } } + private[this] def viewInterface[Q]( + machine: Machine[_], + ifaceId: TypeConName, + templateId: TypeConName, + record: SValue, + )(k: SExpr => Control[Q]): Control[Q] = { + val ref = getInterfaceInstance(machine, ifaceId, templateId).fold( + crash( + s"Attempted to call view for interface ${ifaceId} on a wrapped " + + s"template of type ${ifaceId}, but there's no matching interface instance." + ) + )(iiRef => InterfaceInstanceViewDefRef(iiRef)) + k(SEApp(SEVal(ref), Array(record))) + } + /** $insertFetch[tid] * :: ContractId a * -> Optional {key: key, maintainers: List Party} (template key, if present) @@ -2194,52 +2315,56 @@ private[lf] object SBuiltinFun { } } - // TODO https://github.com/digital-asset/daml/issues/17995 - // redesing contract fetching to improve factotrizstion - private def softFetchInterface( + /** A version of [[fetchTemplate]] without a destination template type. The template type of the contract on ledger + * is used for importing its value, and is returned alongside the value. + */ + private def hardFetchTemplate( machine: UpdateMachine, coid: V.ContractId, keyOpt: SValue, - )(f: SValue => Control[Question.Update]): Control[Question.Update] = { + )( + k: (Ref.PackageName, Ref.TypeConName, SRecord) => Control[Question.Update] + ): Control[Question.Update] = { machine.getIfLocalContract(coid) match { case Some((templateId, templateArg)) => ensureContractActive(machine, coid, templateId) { - f(SValue.SAnyContract(templateId, templateArg)) + getContractInfo(machine, coid, templateId, templateArg, keyOpt) { contract => + k(contract.packageName, templateId, templateArg.asInstanceOf[SRecord]) + } } case None => - machine.lookupContract(coid) { - case V.ContractInstance(packageName, _, srcTmplId, coinstArg) => - machine.packageResolution.get(packageName) match { - case Some(pkgId) => - val dstTmplId = srcTmplId.copy(packageId = pkgId) - machine.ensurePackageIsLoaded( - dstTmplId.packageId, - language.Reference.Template(dstTmplId), - ) { () => - importValue(machine, dstTmplId, coinstArg) { templateArg => - getContractInfo(machine, coid, dstTmplId, templateArg, keyOpt) { contract => - ensureContractActive(machine, coid, contract.templateId) { - - machine.checkContractVisibility(coid, contract) - machine.enforceLimitAddInputContract() - machine.enforceLimitSignatoriesAndObservers(coid, contract) - - // In Validation mode, we always call validateContractInfo - // In Submission mode, we only call validateContractInfo when src != dest - if ((machine.validating) || (srcTmplId.packageId != dstTmplId.packageId)) { - validateContractInfo(machine, coid, srcTmplId, contract) { () => - f(contract.any) - } - } else { - f(contract.any) - } - } + machine.lookupContract(coid) { case V.ContractInstance(_, _, srcTmplId, coinstArg) => + machine.ensurePackageIsLoaded( + srcTmplId.packageId, + language.Reference.Template(srcTmplId), + ) { () => + importValue(machine, srcTmplId, coinstArg) { templateArg => + getContractInfo(machine, coid, srcTmplId, templateArg, keyOpt) { contract => + ensureContractActive(machine, coid, contract.templateId) { + + machine.checkContractVisibility(coid, contract) + machine.enforceLimitAddInputContract() + machine.enforceLimitSignatoriesAndObservers(coid, contract) + + if (machine.validating) { + validateContractInfo(machine, coid, srcTmplId, contract) { () => + k( + contract.packageName, + contract.templateId, + contract.value.asInstanceOf[SRecord], + ) } + } else { + k( + contract.packageName, + contract.templateId, + contract.value.asInstanceOf[SRecord], + ) } } - case None => - crash(s"Could not resolve packageName to packageId: $packageName") + } } + } } } } diff --git a/sdk/daml-script/test/daml/upgrades/Interfaces.daml b/sdk/daml-script/test/daml/upgrades/Interfaces.daml index 2c7e81606b87..f9f4368f1629 100644 --- a/sdk/daml-script/test/daml/upgrades/Interfaces.daml +++ b/sdk/daml-script/test/daml/upgrades/Interfaces.daml @@ -246,14 +246,14 @@ queryInterfaceUpgrades = test $ do fetchFromInterfaceUpgrades : Test fetchFromInterfaceUpgrades = test $ do (alice, icid) <- setupAliceAndInterface - resV1 <- alice `submit` createAndExerciseExactCmd (V1.FITemplate with party = alice) (V1.FetchFromInterface with cid = icid) + resV1 <- alice `submit` createAndExerciseExactCmd (V1.FITemplate with party = alice) (V1.FetchFromInterface with icid = icid) resV1 === None - resV2 <- alice `submit` createAndExerciseExactCmd (V2.FITemplate with party = alice, payload = None) (V2.FetchFromInterface with cid = icid) + resV2 <- alice `submit` createAndExerciseExactCmd (V2.FITemplate with party = alice, payload = None) (V2.FetchFromInterface with icid = icid) resV2 === None fetchWithoutV1 : Test fetchWithoutV1 = test $ do (alice, icid) <- setupAliceAndInterface withUnvettedDarOnParticipant "interfaces-1.0.0" participant0 $ do - alice `submit` createAndExerciseCmd (V2.FITemplate alice) (V2.FetchInterface icid) + alice `submit` createAndExerciseCmd (V2.FITemplate alice None) (V2.FetchInterface icid) pure () From 94c395bf5ff4f152ddf561981e1b0d2357e26ca1 Mon Sep 17 00:00:00 2001 From: Paul Brauner Date: Wed, 18 Sep 2024 11:21:09 +0200 Subject: [PATCH 3/4] fix ExceptionTest.scala --- .../daml/lf/speedy/SBuiltinFun.scala | 6 +++--- .../daml/lf/speedy/ExceptionTest.scala | 17 ++++++++++++----- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltinFun.scala b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltinFun.scala index a983419da34e..7834d73e9822 100644 --- a/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltinFun.scala +++ b/sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltinFun.scala @@ -1396,9 +1396,9 @@ private[lf] object SBuiltinFun { val (srcPkgName, _) = machine.tmplId2PackageNameVersion(dstTplId) val (dstPkgName, _) = machine.tmplId2PackageNameVersion(srcTplId) if (srcPkgName == dstPkgName) { - // This isn't ideal as its a large uncached computation in a non Update primative. - // Ideally this would run in Update, and not iterate the value twice - // i.e. using an upgrade transformation function directly on SValues + // This isn't ideal as its a large uncached computation in a non Update primative. + // Ideally this would run in Update, and not iterate the value twice + // i.e. using an upgrade transformation function directly on SValues importValue(machine, dstTplId, srcArg.toUnnormalizedValue) { templateArg => k(Some(templateArg)) } diff --git a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/ExceptionTest.scala b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/ExceptionTest.scala index ce65040be444..162084c4d9e1 100644 --- a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/ExceptionTest.scala +++ b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/ExceptionTest.scala @@ -8,9 +8,9 @@ import com.digitalasset.daml.lf.data.Ref.Party import com.digitalasset.daml.lf.interpretation.{Error => IE} import com.digitalasset.daml.lf.language.Ast._ import com.digitalasset.daml.lf.language.{LanguageMajorVersion, LanguageVersion} -import com.digitalasset.daml.lf.speedy.SResult.{SResultError, SResultFinal} import com.digitalasset.daml.lf.speedy.SError.{SError, SErrorDamlException} import com.digitalasset.daml.lf.speedy.SExpr._ +import com.digitalasset.daml.lf.speedy.SResult.{SResultError, SResultFinal} import com.digitalasset.daml.lf.speedy.SValue.{SParty, SUnit} import com.digitalasset.daml.lf.speedy.SpeedyTestLib.typeAndCompile import com.digitalasset.daml.lf.stablepackages.StablePackages @@ -20,8 +20,8 @@ import com.digitalasset.daml.lf.testing.parser.ParserParameters import com.digitalasset.daml.lf.value.Value.{ValueRecord, ValueText} import org.scalatest.Inside import org.scalatest.freespec.AnyFreeSpec -import org.scalatest.prop.TableDrivenPropertyChecks import org.scalatest.matchers.should.Matchers +import org.scalatest.prop.TableDrivenPropertyChecks class ExceptionTestV2 extends ExceptionTest(LanguageMajorVersion.V2) @@ -515,7 +515,7 @@ class ExceptionTest(majorLanguageVersion: LanguageMajorVersion) "uncatchable exceptions" - { "not be caught" in { - val pkgs: PureCompiledPackages = typeAndCompile(p""" + val pkg: Package = p""" metadata ( 'pkg' : '1.0.0' ) module M { @@ -572,7 +572,8 @@ class ExceptionTest(majorLanguageVersion: LanguageMajorVersion) }; }; } - """) + """ + val pkgs: PureCompiledPackages = typeAndCompile(pkg) val transactionSeed: crypto.Hash = crypto.Hash.hashPrivateKey("transactionSeed") @@ -637,7 +638,13 @@ class ExceptionTest(majorLanguageVersion: LanguageMajorVersion) """ val res = Speedy.Machine - .fromUpdateSExpr(pkgs, transactionSeed, applyToParty(pkgs, expr, party), Set(party)) + .fromUpdateSExpr( + pkgs, + transactionSeed, + applyToParty(pkgs, expr, party), + Set(party), + packageResolution = Map(pkg.pkgName -> defaultParserParameters.defaultPackageId), + ) .run() if (description.contains("can be caught")) inside(res) { case SResultFinal(SUnit) => From 9ffb19485762e76e9f3a691d96d7800fa1521ff7 Mon Sep 17 00:00:00 2001 From: Paul Brauner Date: Tue, 1 Oct 2024 17:16:42 +0200 Subject: [PATCH 4/4] address review comments --- .../daml/lf/speedy/SBuiltinInterfaceTest.scala | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/SBuiltinInterfaceTest.scala b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/SBuiltinInterfaceTest.scala index 830bd7a1c918..8df9151dfab9 100644 --- a/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/SBuiltinInterfaceTest.scala +++ b/sdk/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/SBuiltinInterfaceTest.scala @@ -42,15 +42,18 @@ class SBuiltinInterfaceUpgradeTest extends AnyFreeSpec with Matchers with Inside import EvalHelpers._ + // TODO: revert to the default version and compiler config once they support upgrades + val languageVersion = LanguageVersion.Features.packageUpgrades + val compilerConfig = Compiler.Config.Dev(LanguageMajorVersion.V2) + val alice = Ref.Party.assertFromString("Alice") // The following code defines a package -iface-pkg- that defines a single interface Iface. val ifacePkgName = Ref.PackageName.assertFromString("-iface-pkg-") - val ifacePkgId = Ref.PackageId.assertFromString("-iface-package-id-") + val ifacePkgId = Ref.PackageId.assertFromString("-iface-pkg-id-") val ifaceParserParams = ParserParameters( defaultPackageId = ifacePkgId, - // TODO: revert to the default version once it supports upgrades - languageVersion = LanguageVersion.Features.packageUpgrades, + languageVersion = languageVersion, ) val ifacePkg = p"""metadata ( '$ifacePkgName' : '1.0.0' ) @@ -79,8 +82,7 @@ class SBuiltinInterfaceUpgradeTest extends AnyFreeSpec with Matchers with Inside Ref.PackageId.assertFromString(s"-implem-pkg-id-$pkgVersion-") def implemParserParams(pkgVersion: Int) = ParserParameters( defaultPackageId = implemPkgId(pkgVersion), - // TODO: revert to the default version once it supports upgrades - languageVersion = LanguageVersion.Features.packageUpgrades, + languageVersion = languageVersion, ) def implemPkg(pkgVersion: Int) = p"""metadata ( '$implemPkgName' : '${implemPkgVersion(pkgVersion)}' ) @@ -102,8 +104,7 @@ class SBuiltinInterfaceUpgradeTest extends AnyFreeSpec with Matchers with Inside implemPkgId(1) -> implemPkg(1), implemPkgId(2) -> implemPkg(2), ), - // TODO: revert to the default compiler config once it supports upgrades - Compiler.Config.Dev(LanguageMajorVersion.V2), + compilerConfig, ) // But we prefer version 2 of -implem-pkg-, which will force an upgrade of any version 1 contract when