Fix Transaction.Metadata used_packages field computation for interface (#13441)

fixes #13437 CHANGELOG_BEGIN CHANGELOG_END
2024-09-20 09:17:43 +03:00 · 2022-03-29 16:20:04 +02:00 · 2022-03-29 16:20:04 +02:00 · dfee9dfaf8
commit dfee9dfaf8
parent af288a6bce
4 changed files with 204 additions and 40 deletions
--- a/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Engine.scala
+++ b/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Engine.scala
@ -111,21 +111,7 @@ class Engine(val config: EngineConfig = Engine.StableConfig) {
          ledgerTime = cmds.ledgerEffectiveTime,
          submissionTime = submissionTime,
          seeding = Engine.initialSeeding(submissionSeed, participantId, submissionTime),
-        ) map { case (tx, meta) =>
-          // Annotate the transaction with the package dependencies. Since
-          // all commands are actions on a contract template, with a fully typed
-          // argument, we only need to consider the templates mentioned in the command
-          // to compute the full dependencies.
-          val deps = processedCmds.foldLeft(Set.empty[PackageId]) { (pkgIds, cmd) =>
-            val pkgId = cmd.templateId.packageId
-            val transitiveDeps =
-              compiledPackages
-                .getPackageDependencies(pkgId)
-                .getOrElse(sys.error(s"INTERNAL ERROR: Missing dependencies of package $pkgId"))
-            (pkgIds + pkgId) union transitiveDeps
-          }
-          tx -> meta.copy(submissionSeed = Some(submissionSeed), usedPackages = deps)
-        }
+        ) map { case (tx, meta) => tx -> meta.copy(submissionSeed = Some(submissionSeed)) }
      }
  }

@ -320,6 +306,26 @@ class Engine(val config: EngineConfig = Engine.StableConfig) {
    interpretLoop(machine, ledgerTime)
  }

+  @SuppressWarnings(Array("org.wartremover.warts.Return"))
+  private[engine] def deps(tx: VersionedTransaction): Result[Set[PackageId]] = {
+    val nodePkgIds =
+      tx.nodes.values.collect { case node: Node.Action => node.templateId.packageId }.toSet
+    val deps = nodePkgIds.foldLeft(nodePkgIds)((acc, pkgId) =>
+      acc | compiledPackages
+        .getPackageDependencies(pkgId)
+        .getOrElse(
+          return ResultError(
+            Error.Interpretation.Internal(
+              NameOf.qualifiedNameOfCurrentFunc,
+              s"INTERNAL ERROR: Missing dependencies of package $pkgId",
+              None,
+            )
+          )
+        )
+    )
+    ResultDone(deps)
+  }
+
  // TODO SC remove 'return', notwithstanding a love of unhandled exceptions
  @SuppressWarnings(Array("org.wartremover.warts.Return"))
  private[engine] def interpretLoop(
@ -388,20 +394,22 @@ class Engine(val config: EngineConfig = Engine.StableConfig) {

    onLedger.finish match {
      case PartialTransaction.CompleteTransaction(tx, _, nodeSeeds) =>
-        val meta = Tx.Metadata(
-          submissionSeed = None,
-          submissionTime = onLedger.ptxInternal.submissionTime,
-          usedPackages = Set.empty,
-          dependsOnTime = onLedger.dependsOnTime,
-          nodeSeeds = nodeSeeds,
-        )
-        config.profileDir.foreach { dir =>
-          val desc = Engine.profileDesc(tx)
-          machine.profile.name = s"${meta.submissionTime}-$desc"
-          val profileFile = dir.resolve(s"${meta.submissionTime}-$desc.json")
-          machine.profile.writeSpeedscopeJson(profileFile)
+        deps(tx).flatMap { deps =>
+          val meta = Tx.Metadata(
+            submissionSeed = None,
+            submissionTime = onLedger.ptxInternal.submissionTime,
+            usedPackages = deps,
+            dependsOnTime = onLedger.dependsOnTime,
+            nodeSeeds = nodeSeeds,
+          )
+          config.profileDir.foreach { dir =>
+            val desc = Engine.profileDesc(tx)
+            machine.profile.name = s"${meta.submissionTime}-$desc"
+            val profileFile = dir.resolve(s"${meta.submissionTime}-$desc.json")
+            machine.profile.writeSpeedscopeJson(profileFile)
+          }
+          ResultDone((tx, meta))
        }
-        ResultDone((tx, meta))
      case PartialTransaction.IncompleteTransaction(ptx) =>
        ResultError(
          Error.Interpretation.Internal(
--- a/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/MetaDataTest.scala
+++ b/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/MetaDataTest.scala
@ -0,0 +1,164 @@
+// Copyright (c) 2022 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package com.daml.lf
+package engine
+
+import com.daml.lf.data.Ref
+import com.daml.lf.transaction.Node
+import com.daml.lf.transaction.test.TransactionBuilder
+import com.daml.lf.transaction.test.TransactionBuilder.Implicits._
+import com.daml.lf.value.Value.{ValueParty, ValueUnit}
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.prop.TableDrivenPropertyChecks
+import org.scalatest.wordspec.AnyWordSpec
+
+class MetaDataTest extends AnyWordSpec with Matchers with TableDrivenPropertyChecks {
+
+  import MetaDataTest._
+
+  "Engine#desp" should {
+
+    val createWithoutInterface = newBuilder.create(
+      id = TransactionBuilder.newCid,
+      templateId = Ref.Identifier("pkgT", "M:T"),
+      argument = ValueUnit,
+      signatories = parties,
+      observers = noOne,
+      key = Some(ValueParty("alice")),
+      maintainers = parties,
+      byInterface = None,
+    )
+    val nodeWithoutInterface = Table[TransactionBuilder => Node](
+      "transaction",
+      _ => createWithoutInterface,
+      _.exercise(
+        contract = createWithoutInterface,
+        choice = "ChT",
+        consuming = false,
+        actingParties = parties,
+        argument = ValueUnit,
+        byInterface = createWithoutInterface.byInterface,
+      ),
+      _.exerciseByKey(
+        contract = createWithoutInterface,
+        choice = "ChT",
+        consuming = false,
+        actingParties = parties,
+        argument = ValueUnit,
+      ),
+      _.fetch(contract = createWithoutInterface, byInterface = createWithoutInterface.byInterface),
+      _.fetchByKey(contract = createWithoutInterface),
+      _.lookupByKey(contract = createWithoutInterface),
+    )
+
+    val createWithInterface = newBuilder.create(
+      id = TransactionBuilder.newCid,
+      templateId = Ref.Identifier("pkgImpl", "M:Impl"),
+      argument = ValueUnit,
+      signatories = parties,
+      observers = noOne,
+      key = Some(ValueParty("alice")),
+      maintainers = parties,
+      byInterface = Some(Ref.Identifier("pkgInt", "M:Int")),
+    )
+    val nodeWithInterface = Table[TransactionBuilder => Node](
+      "transaction",
+      _ => createWithInterface,
+      _.exercise(
+        contract = createWithInterface,
+        choice = "ChI",
+        consuming = false,
+        actingParties = parties,
+        argument = ValueUnit,
+        byInterface = createWithInterface.byInterface,
+      ),
+      _.fetch(contract = createWithInterface, byInterface = createWithInterface.byInterface),
+    )
+
+    "works as expected on root actions node by template" in {
+      val expected = ResultDone(Set("pkgT", "pkgTLib"))
+      forEvery(nodeWithoutInterface) { mkNode =>
+        val builder = newBuilder
+        builder.add(mkNode(builder))
+        engine.deps(builder.build()) shouldBe expected
+      }
+    }
+
+    "works as expected on root action nodes by interface" in {
+      val expected = ResultDone(Set("pkgInt", "pkgIntLib", "pkgImpl", "pkgImplLib"))
+      forEvery(nodeWithInterface) { mkNode =>
+        val builder = newBuilder
+        builder.add(mkNode(builder))
+        engine.deps(builder.build()) shouldBe expected
+      }
+    }
+
+    "works as expected on non-root action nodes" in {
+      val expected = ResultDone(
+        Set(
+          "pkgBase",
+          "pkgBaseLib",
+          "pkgT",
+          "pkgTLib",
+          "pkgInt",
+          "pkgIntLib",
+          "pkgImpl",
+          "pkgImplLib",
+        )
+      )
+      val contract = newBuilder.create(
+        id = TransactionBuilder.newCid,
+        templateId = Ref.Identifier("pkgBase", "M:T"),
+        argument = ValueUnit,
+        signatories = parties,
+        observers = noOne,
+        byInterface = None,
+      )
+      forEvery(nodeWithoutInterface) { mkNodeWithout =>
+        forEvery(nodeWithInterface) { mkNodeWith =>
+          val builder = newBuilder
+          val exeId = builder.add(builder.exercise(contract, "Ch0", true, parties, ValueUnit))
+          builder.add(mkNodeWithout(builder), exeId)
+          builder.add(mkNodeWith(builder), exeId)
+          engine.deps(builder.build()) shouldBe expected
+        }
+      }
+    }
+  }
+
+}
+
+object MetaDataTest {
+
+  private[this] val langVersion =
+    // TODO https://github.com/digital-asset/daml/issues/12051:
+    //  replace by LanguageVersion.default once LF 1.15 is make stable
+    language.LanguageVersion.v1_dev
+
+  private def newBuilder = new TransactionBuilder(_ =>
+    transaction.TransactionVersion.assignNodeVersion(langVersion)
+  )
+
+  private val engine = Engine.DevEngine()
+
+  private[this] val emptyPkg = language.Ast.Package(Map.empty, Set.empty, langVersion, None)
+
+  // For the sake of simplicity we load the engine with empty packages where only the directDeps is set.
+  List(
+    "pkgTLib" -> emptyPkg,
+    "pkgT" -> emptyPkg.copy(directDeps = Set("pkgTLib")),
+    "pkgIntLib" -> emptyPkg,
+    "pkgInt" -> emptyPkg.copy(directDeps = Set("pkgIntLib")),
+    "pkgBaseLib" -> emptyPkg,
+    "pkgBase" -> emptyPkg.copy(directDeps = Set("pkgBaseLib", "pkgT", "pkgInt")),
+    "pkgImplLib" -> emptyPkg,
+    "pkgImpl" -> emptyPkg.copy(directDeps = Set("pkgImplLib", "pkgInt")),
+  ).foreach { case (pkgId, pkg) =>
+    require(engine.preloadPackage(pkgId, pkg).isInstanceOf[ResultDone[_]])
+  }
+
+  private val parties = Set[Ref.Party]("alice")
+  private val noOne = Set.empty
+
+}
--- a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Command.scala
+++ b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Command.scala
@ -9,11 +9,9 @@ import com.daml.lf.speedy.SValue._
 // ---------------------
 // Preprocessed commands
 // ---------------------
-sealed abstract class Command extends Product with Serializable {
-  val templateId: Identifier
-}
+private[lf] sealed abstract class Command extends Product with Serializable

-object Command {
+private[lf] object Command {

  /** Create a template, not by interface */
  final case class Create(
@ -53,13 +51,7 @@ object Command {
      contractId: SContractId,
      choiceId: ChoiceName,
      argument: SValue,
-  ) extends Command {
-    // TODO https://github.com/digital-asset/daml/issues/12051
-    // TODO https://github.com/digital-asset/daml/issues/11342
-    //   The actual template id isn't known until run time.
-    //   The interface id is the best we've got.
-    val templateId = interfaceId
-  }
+  ) extends Command

  final case class ExerciseByKey(
      templateId: Identifier,
--- a/daml-lf/transaction-test-lib/src/main/scala/lf/transaction/test/TransactionBuilder.scala
+++ b/daml-lf/transaction-test-lib/src/main/scala/lf/transaction/test/TransactionBuilder.scala
@ -180,7 +180,7 @@ final class TransactionBuilder(pkgTxVersion: Ref.PackageId => TransactionVersion
  def fetchByKey(contract: Node.Create): Node.Fetch =
    fetch(contract, byKey = true)

-  def lookupByKey(contract: Node.Create, found: Boolean): Node.LookupByKey =
+  def lookupByKey(contract: Node.Create, found: Boolean = true): Node.LookupByKey =
    Node.LookupByKey(
      templateId = contract.templateId,
      key = contract.key.get,