LF: control of language version in the engine (#6847)

This PR allows to control the language version accepted by the engine following #5164. CHANGELOG_BEGIN CHANGELOG_END
digital-asset · Aug 13, 2020 · ec7c53c · ec7c53c
1 parent 2f4aed4
commit ec7c53c
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 42 deletions.
diff --git 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
@@ -241,7 +241,7 @@ class Engine(private[lf] val config: EngineConfig = EngineConfig.Stable) {
       case pkgId :: rest =>
         ResultNeedPackage(pkgId, {
           case Some(pkg) =>
-            compiledPackages.addPackage(pkgId, pkg).flatMap(_ => loadPackages(rest))
+            addPackage(pkgId, pkg).flatMap(_ => loadPackages(rest))
           case None =>
             ResultError(Error(s"package $pkgId not found"))
         })
@@ -321,7 +321,7 @@ class Engine(private[lf] val config: EngineConfig = EngineConfig.Stable) {
           return Result.needPackage(
             pkgId,
             pkg => {
-              compiledPackages.addPackage(pkgId, pkg).flatMap { _ =>
+              addPackage(pkgId, pkg).flatMap { _ =>
                 callback(compiledPackages)
                 interpretLoop(machine, time)
               }
@@ -409,7 +409,7 @@ class Engine(private[lf] val config: EngineConfig = EngineConfig.Stable) {
     * be loaded.
     */
   def preloadPackage(pkgId: PackageId, pkg: Package): Result[Unit] =
-    compiledPackages.addPackage(pkgId, pkg)
+    addPackage(pkgId, pkg)
 
   def setProfileDir(optProfileDir: Option[Path]): Unit = {
     optProfileDir match {
@@ -426,6 +426,18 @@ class Engine(private[lf] val config: EngineConfig = EngineConfig.Stable) {
     compiledPackages.stackTraceMode =
       if (enable) speedy.Compiler.FullStackTrace else speedy.Compiler.NoStackTrace
   }
+
+  private[engine] def addPackage(pkgId: PackageId, pkg: Package): Result[Unit] =
+    if (config.languageVersions.contains(pkg.languageVersion))
+      compiledPackages.addPackage(pkgId, pkg)
+    else
+      ResultError(
+        Error(
+          s"Disallowed language version in package $pkgId: " +
+            s"Expected version between ${config.languageVersions.min} and ${config.languageVersions.max} but got ${pkg.languageVersion}"
+        )
+      )
+
 }
 
 object Engine {

diff --git a/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/EngineConfig.scala b/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/EngineConfig.scala
@@ -3,22 +3,21 @@
 
 package com.daml.lf.engine
 
-import com.daml.lf.data.NoCopy
-import com.daml.lf.{VersionRange, data}
+import com.daml.lf.VersionRange
 import com.daml.lf.language.{LanguageVersion => LV}
-import com.daml.lf.transaction.{TransactionVersion => TV, TransactionVersions}
+import com.daml.lf.transaction.{TransactionVersions, TransactionVersion => TV}
 
 // FIXME: https://github.com/digital-asset/daml/issues/5164
 // Currently only outputTransactionVersions is used.
 // languageVersions and outputTransactionVersions should be plug
-final case class EngineConfig private (
+final case class EngineConfig(
     // constrains the version of language accepted by the engine
     languageVersions: VersionRange[LV],
     // constrains the version of output transactions
     inputTransactionVersions: VersionRange[TV],
     // constrains the version of output transactions
     outputTransactionVersions: VersionRange[TV],
-) extends NoCopy
+)
 
 object EngineConfig {
 
@@ -56,39 +55,8 @@ object EngineConfig {
     )
   )
 
-  def build(
-      languageVersions: VersionRange[LV],
-      inputTransactionVersions: VersionRange[TV],
-      outputTransactionVersions: VersionRange[TV],
-  ): Either[String, EngineConfig] = {
-    val config = new EngineConfig(
-      languageVersions = languageVersions intersect Dev.languageVersions,
-      inputTransactionVersions = inputTransactionVersions intersect Dev.inputTransactionVersions,
-      outputTransactionVersions = outputTransactionVersions intersect Dev.outputTransactionVersions,
-    )
-
-    Either.cond(
-      config.languageVersions.nonEmpty && config.inputTransactionVersions.nonEmpty && config.outputTransactionVersions.nonEmpty,
-      config,
-      "invalid engine configuration"
-    )
-  }
-
-  def assertBuild(
-      languageVersions: VersionRange[LV],
-      inputTransactionVersions: VersionRange[TV],
-      outputTransactionVersions: VersionRange[TV],
-  ): EngineConfig =
-    data.assertRight(
-      build(
-        languageVersions: VersionRange[LV],
-        inputTransactionVersions: VersionRange[TV],
-        outputTransactionVersions: VersionRange[TV],
-      )
-    )
-
   // recommended configuration
-  val Stable: EngineConfig = assertBuild(
+  val Stable: EngineConfig = new EngineConfig(
     languageVersions = VersionRange(
       LV(LV.Major.V1, LV.Minor.Stable("6")),
       LV(LV.Major.V1, LV.Minor.Stable("8")),

diff --git a/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala b/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala
@@ -30,6 +30,7 @@ import com.daml.lf.speedy.SValue._
 import com.daml.lf.command._
 import com.daml.lf.value.ValueVersions.assertAsVersionedValue
 import org.scalactic.Equality
+import org.scalatest.prop.TableDrivenPropertyChecks
 import org.scalatest.{EitherValues, Matchers, WordSpec}
 import scalaz.std.either._
 import scalaz.syntax.apply._
@@ -43,7 +44,12 @@ import scala.language.implicitConversions
     "org.wartremover.warts.Serializable",
     "org.wartremover.warts.Product"
   ))
-class EngineTest extends WordSpec with Matchers with EitherValues with BazelRunfiles {
+class EngineTest
+    extends WordSpec
+    with Matchers
+    with TableDrivenPropertyChecks
+    with EitherValues
+    with BazelRunfiles {
 
   import EngineTest._
 
@@ -1621,6 +1627,54 @@ class EngineTest extends WordSpec with Matchers with EitherValues with BazelRunf
       err.msg should include("precondition violation")
     }
   }
+
+  "Engine.addPackage" should {
+
+    import com.daml.lf.language.{LanguageVersion => LV}
+
+    def engine(min: LV.Minor, max: LV.Minor) =
+      new Engine(
+        EngineConfig.Dev.copy(
+          languageVersions = VersionRange(LV(LV.Major.V1, min), LV(LV.Major.V1, max))
+        )
+      )
+
+    val pkgId = Ref.PackageId.assertFromString("-pkg-")
+
+    def pkg(v: LV.Minor) =
+      language.Ast.Package(
+        Traversable.empty,
+        Traversable.empty,
+        LV(LV.Major.V1, v),
+        None
+      )
+
+    "reject disallow packages" in {
+      val negativeTestCases = Table(
+        ("pkg version", "minVersion", "maxVertion"),
+        (LV.Minor.Stable("6"), LV.Minor.Stable("6"), LV.Minor.Stable("8")),
+        (LV.Minor.Stable("7"), LV.Minor.Stable("6"), LV.Minor.Stable("8")),
+        (LV.Minor.Stable("8"), LV.Minor.Stable("6"), LV.Minor.Stable("8")),
+        (LV.Minor.Dev, LV.Minor.Stable("6"), LV.Minor.Dev),
+      )
+      val positiveTestCases = Table(
+        ("pkg version", "minVersion", "maxVertion"),
+        (LV.Minor.Stable("6"), LV.Minor.Stable("7"), LV.Minor.Dev),
+        (LV.Minor.Stable("7"), LV.Minor.Stable("8"), LV.Minor.Stable("8")),
+        (LV.Minor.Stable("8"), LV.Minor.Stable("6"), LV.Minor.Stable("7")),
+        (LV.Minor.Dev, LV.Minor.Stable("6"), LV.Minor.Stable("8")),
+      )
+
+      forEvery(negativeTestCases)((v, min, max) =>
+        engine(min, max).addPackage(pkgId, pkg(v)) shouldBe a[ResultDone[_]])
+
+      forEvery(positiveTestCases)((v, min, max) =>
+        engine(min, max).addPackage(pkgId, pkg(v)) shouldBe a[ResultError])
+
+    }
+
+  }
+
 }
 
 object EngineTest {

diff --git a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SError.scala b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SError.scala
@@ -30,7 +30,7 @@ object SError {
   /** DAML exceptions that can be caught. These include
     * arithmetic errors, call to error builtin or update
     * errors. */
-  sealed trait SErrorDamlException extends SError
+  sealed abstract class SErrorDamlException extends SError
 
   /** Arithmetic error such as division by zero */
   final case class DamlEArithmeticError(message: String) extends SErrorDamlException {