Remove the TransactionVersion.minExceptions flag (#18441)

2024-09-20 01:07:18 +03:00 · 2024-02-09 19:25:23 +01:00 · 2024-02-09 19:25:23 +01:00 · 499cb7f618
commit 499cb7f618
parent beb4ad605e
11 changed files with 35 additions and 179 deletions
--- a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Speedy.scala
+++ b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Speedy.scala
@ -400,7 +400,7 @@ private[lf] object Speedy {
        } else {
          popKont() match {
            case handler: KTryCatchHandler =>
-              ptx = ptx.rollbackTry(excep)
+              ptx = ptx.rollbackTry()
              Some(handler)
            case _: KCloseExercise =>
              ptx = ptx.abortExercises
--- a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/transaction/PartialTransaction.scala
+++ b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/transaction/PartialTransaction.scala
@ -618,13 +618,7 @@ private[speedy] case class PartialTransaction(
  /** Close a try context, by catching an exception,
    * i.e. a exception was thrown inside the context, and the catch associated to the try context did handle it.
    */
-  def rollbackTry(ex: SValue.SAny): PartialTransaction = {
-    // we must never create a rollback containing a node with a version pre-dating exceptions
-    if (context.minChildVersion < TxVersion.minExceptions) {
-      throw SError.SErrorDamlException(
-        interpretation.Error.UnhandledException(ex.ty, ex.value.toUnnormalizedValue)
-      )
-    }
+  def rollbackTry(): PartialTransaction = {
    context.info match {
      case info: TryContextInfo =>
        // In the case of there being no children we could drop the entire rollback node.
--- a/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/PartialTransactionSpec.scala
+++ b/daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/PartialTransactionSpec.scala
@ -87,7 +87,7 @@ class PartialTransactionSpec extends AnyWordSpec with Matchers with Inside {
      ptx.endExercises(_ => Value.ValueNone)

    def rollbackTry_ : PartialTransaction =
-      ptx.rollbackTry(null /* dummy exception, never read */ )
+      ptx.rollbackTry()
  }

  private[this] val outputCids =
--- a/daml-lf/transaction-test-lib/src/main/scala/lf/transaction/test/NodeIdTransactionBuilder.scala
+++ b/daml-lf/transaction-test-lib/src/main/scala/lf/transaction/test/NodeIdTransactionBuilder.scala
@ -56,7 +56,7 @@ class NodeIdTransactionBuilder extends TestIdFactory {
    val txVersion = finalNodes.values.foldLeft(TransactionVersion.minVersion)((acc, node) =>
      node.optVersion match {
        case Some(version) => acc max version
-        case None => acc max TransactionVersion.minExceptions
+        case None => acc
      }
    )
    val finalRoots = roots.toImmArray
--- a/daml-lf/transaction-test-lib/src/main/scala/lf/value/test/ValueGenerators.scala
+++ b/daml-lf/transaction-test-lib/src/main/scala/lf/value/test/ValueGenerators.scala
@ -29,7 +29,6 @@ import scalaz.scalacheck.ScalaCheckBinding._

 object ValueGenerators {

-  import TransactionVersion.minExceptions
  import TransactionVersion.minInterfaces

  // generate decimal values
@ -392,8 +391,7 @@ object ValueGenerators {
        .listOf(Arbitrary.arbInt.arbitrary)
        .map(_.map(NodeId(_)))
        .map(_.to(ImmArray))
-      exerciseResult <-
-        if (version < minExceptions) valueGen().map(Some(_)) else Gen.option(valueGen())
+      exerciseResult <- Gen.option(valueGen())
      key <- Gen.option(keyWithMaintainersGen(templateId))
      byKey <- Gen.oneOf(true, false)
    } yield Node.Exercise(
@ -473,15 +471,11 @@ object ValueGenerators {
      )
    )

-  def danglingRefGenNodeWithVersion(version: TransactionVersion): Gen[(NodeId, Node)] = {
-    if (version < minExceptions)
-      danglingRefGenActionNodeWithVersion(version)
-    else
-      Gen.frequency(
-        3 -> danglingRefGenActionNodeWithVersion(version),
-        1 -> refGenNode(danglingRefRollbackNodeGen),
-      )
-  }
+  def danglingRefGenNodeWithVersion(version: TransactionVersion): Gen[(NodeId, Node)] =
+    Gen.frequency(
+      3 -> danglingRefGenActionNodeWithVersion(version),
+      1 -> refGenNode(danglingRefRollbackNodeGen),
+    )

  /** Aside from the invariants failed as listed under `malformedCreateNodeGen`,
    * resulting transactions may be malformed in several other ways:
--- a/daml-lf/transaction/BUILD.bazel
+++ b/daml-lf/transaction/BUILD.bazel
@ -127,6 +127,5 @@ da_scala_test(
    deps = [
        ":transaction",
        "//daml-lf/data",
-        "//daml-lf/language",
    ],
 )
--- a/daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/transaction/TransactionCoder.scala
+++ b/daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/transaction/TransactionCoder.scala
@ -214,9 +214,7 @@ object TransactionCoder {
      enclosingVersion: TransactionVersion,
      nodeId: NodeId,
      node: Node,
-      disableVersionCheck: Boolean =
-        false, // true allows encoding of bad protos (for testing of decode checks)
-  ): Either[EncodeError, TransactionOuterClass.Node] = {
+  ) = {

    val nodeBuilder =
      TransactionOuterClass.Node.newBuilder().setNodeId(encodeNid.asString(nodeId))
@ -225,13 +223,7 @@ object TransactionCoder {
      case Node.Rollback(children) =>
        val builder = TransactionOuterClass.NodeRollback.newBuilder()
        children.foreach(id => discard(builder.addChildren(encodeNid.asString(id))))
-        for {
-          _ <- Either.cond(
-            test = enclosingVersion >= TransactionVersion.minExceptions || disableVersionCheck,
-            right = (),
-            left = EncodeError(enclosingVersion, isTooOldFor = "rollback nodes"),
-          )
-        } yield nodeBuilder.setRollback(builder).build()
+        Right(nodeBuilder.setRollback(builder).build())

      case node: Node.Action =>
        val nodeVersion = node.version
@ -338,11 +330,7 @@ object TransactionCoder {
                      builder.setResultUnversioned,
                    )
                  case None =>
-                    Either.cond(
-                      test = ne.version >= TransactionVersion.minExceptions || disableVersionCheck,
-                      right = (),
-                      left = EncodeError(nodeVersion, isTooOldFor = "NodeExercises without result"),
-                    )
+                    Right(())
                }
                _ <- encodeAndSetContractKey(
                  nodeVersion,
@ -460,13 +448,6 @@ object TransactionCoder {
      case NodeTypeCase.ROLLBACK =>
        val protoRollback = protoNode.getRollback
        for {
-          _ <- Either.cond(
-            test = nodeVersion >= TransactionVersion.minExceptions,
-            right = (),
-            left = DecodeError(
-              s"rollback node (supported since ${TransactionVersion.minExceptions}) unexpected in transaction of version $nodeVersion"
-            ),
-          )
          ni <- nodeId
          children <- decodeChildren(decodeNid, protoRollback.getChildrenList)
        } yield ni -> Node.Rollback(children)
@ -533,13 +514,7 @@ object TransactionCoder {
          templateId <- ValueCoder.decodeIdentifier(protoExe.getTemplateId)
          rvOpt <-
            if (!protoExe.hasResultVersioned && protoExe.getResultUnversioned.isEmpty) {
-              Either.cond(
-                test = nodeVersion >= TransactionVersion.minExceptions,
-                right = None,
-                left = DecodeError(
-                  s"NodeExercises without result (supported since ${TransactionVersion.minExceptions}) unexpected in transaction of version $nodeVersion"
-                ),
-              )
+              Right(None)
            } else {
              decodeValue(
                decodeCid,
--- a/daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/transaction/TransactionVersion.scala
+++ b/daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/transaction/TransactionVersion.scala
@ -50,7 +50,6 @@ object TransactionVersion {

  // TODO(https://github.com/digital-asset/daml/issues/18240) remove these 3 feature flags and kill
  //  the transitively dead code. Make sure canton doesn't mention them anymore.
-  private[lf] val minExceptions = V14
  private[lf] val minByKey = V14
  private[lf] val minInterfaces = V15

@ -71,7 +70,7 @@ object TransactionVersion {
    import scala.Ordering.Implicits.infixOrderingOps
    tx.nodes.valuesIterator.foldLeft(TransactionVersion.minVersion) {
      case (acc, action: Node.Action) => acc max action.version
-      case (acc, _: Node.Rollback) => acc max minExceptions
+      case (acc, _: Node.Rollback) => acc
    }
  }

--- a/daml-lf/transaction/src/test/scala/com/digitalasset/daml/lf/transaction/TransactionCoderSpec.scala
+++ b/daml-lf/transaction/src/test/scala/com/digitalasset/daml/lf/transaction/TransactionCoderSpec.scala
@ -114,25 +114,23 @@ class TransactionCoderSpec
    "do Node.Rollback" in {
      forAll(danglingRefRollbackNodeGen) { node =>
        forEvery(transactionVersions) { txVersion =>
-          if (txVersion >= TransactionVersion.minExceptions) {
-            val normalizedNode = normalizeNode(node)
-            val Right(encodedNode) =
-              TransactionCoder
-                .encodeNode(
-                  TransactionCoder.NidEncoder,
-                  ValueCoder.CidEncoder,
-                  txVersion,
-                  NodeId(0),
-                  normalizedNode,
-                )
+          val normalizedNode = normalizeNode(node)
+          val Right(encodedNode) =
            TransactionCoder
-              .decodeVersionedNode(
-                TransactionCoder.NidDecoder,
-                ValueCoder.CidDecoder,
+              .encodeNode(
+                TransactionCoder.NidEncoder,
+                ValueCoder.CidEncoder,
                txVersion,
-                encodedNode,
-              ) shouldBe Right((NodeId(0), normalizedNode))
-          }
+                NodeId(0),
+                normalizedNode,
+              )
+          TransactionCoder
+            .decodeVersionedNode(
+              TransactionCoder.NidDecoder,
+              ValueCoder.CidDecoder,
+              txVersion,
+              encodedNode,
+            ) shouldBe Right((NodeId(0), normalizedNode))
        }
      }
    }
@ -243,44 +241,6 @@ class TransactionCoderSpec

  "encodeVersionedNode" should {

-    "fail if try encode rollback node in version < minExceptions" in {
-      forAll(danglingRefRollbackNodeGen) { node =>
-        forEvery(transactionVersions) { txVersion =>
-          val normalizedNode = normalizeNode(node)
-          val result = TransactionCoder
-            .encodeNode(
-              TransactionCoder.NidEncoder,
-              ValueCoder.CidEncoder,
-              txVersion,
-              NodeId(0),
-              normalizedNode,
-            )
-
-          result.isLeft shouldBe (txVersion < TransactionVersion.minExceptions)
-        }
-      }
-    }
-
-    "fail if try encode missing exerciseResult in version < minExceptions" in {
-      forAll(danglingRefExerciseNodeGen, versionInIncreasingOrder()) {
-        case (node, (nodeVersion, txVersion)) =>
-          val normalizedNode =
-            normalizeExe(node.updateVersion(nodeVersion)).copy(
-              exerciseResult = None
-            )
-          val result = TransactionCoder
-            .encodeNode(
-              TransactionCoder.NidEncoder,
-              ValueCoder.CidEncoder,
-              txVersion,
-              NodeId(0),
-              normalizedNode,
-            )
-
-          result.isLeft shouldBe (nodeVersion < TransactionVersion.minExceptions)
-      }
-    }
-
    "fail if try to encode a node in a version newer than the transaction" in {

      forAll(danglingRefGenActionNode, versionInStrictIncreasingOrder(), minSuccessful(10)) {
@ -865,62 +825,6 @@ class TransactionCoderSpec
      }
    }

-    "fail if we try to decode a rollback node in a version < minExceptions" in {
-      forAll(danglingRefRollbackNodeGen) { node =>
-        forEvery(transactionVersions) { txVersion =>
-          val normalizedNode = normalizeNode(node)
-          val Right(encodedNode) =
-            TransactionCoder
-              .encodeNode(
-                TransactionCoder.NidEncoder,
-                ValueCoder.CidEncoder,
-                txVersion,
-                NodeId(0),
-                normalizedNode,
-                disableVersionCheck = true, // so the bad proto can be created
-              )
-          val result =
-            TransactionCoder
-              .decodeVersionedNode(
-                TransactionCoder.NidDecoder,
-                ValueCoder.CidDecoder,
-                txVersion,
-                encodedNode,
-              )
-          result.isLeft shouldBe (txVersion < TransactionVersion.minExceptions)
-        }
-      }
-    }
-
-    "fail if we try to decode an exercise node with a missing result in a version < minExceptions" in {
-      forAll(danglingRefExerciseNodeGen, versionInIncreasingOrder()) { case (node, (v1, v2)) =>
-        val normalizedNode =
-          normalizeExe(node.updateVersion(v1))
-            .copy(
-              exerciseResult = None
-            )
-        val Right(encodedNode) =
-          TransactionCoder
-            .encodeNode(
-              TransactionCoder.NidEncoder,
-              ValueCoder.CidEncoder,
-              v2,
-              NodeId(0),
-              normalizedNode,
-              disableVersionCheck = true, // so the bad proto can be created
-            )
-        val result =
-          TransactionCoder
-            .decodeVersionedNode(
-              TransactionCoder.NidDecoder,
-              ValueCoder.CidDecoder,
-              v2,
-              encodedNode,
-            )
-        result.isLeft shouldBe (v1 < TransactionVersion.minExceptions)
-      }
-    }
-
    s"preserve byKey on exercise in version >= ${TransactionVersion.minByKey} and drop before" in {
      forAll(
        Arbitrary.arbInt.arbitrary,
@ -1216,16 +1120,7 @@ class TransactionCoderSpec
          exe.interfaceId
        else None,
      chosenValue = normalize(exe.chosenValue, exe.version),
-      exerciseResult = exe.exerciseResult match {
-        case None =>
-          if (exe.version >= TransactionVersion.minExceptions) {
-            None
-          } else {
-            Some(Value.ValueText("not-missing"))
-          }
-        case Some(v) =>
-          Some(normalize(v, exe.version))
-      },
+      exerciseResult = exe.exerciseResult.map(normalize(_, exe.version)),
      choiceObservers = exe.choiceObservers,
      choiceAuthorizers =
        if (exe.version >= TransactionVersion.minChoiceAuthorizers) exe.choiceAuthorizers else None,
--- a/daml-lf/transaction/src/test/scala/com/digitalasset/daml/lf/transaction/TransactionSpec.scala
+++ b/daml-lf/transaction/src/test/scala/com/digitalasset/daml/lf/transaction/TransactionSpec.scala
@ -191,7 +191,7 @@ class TransactionSpec
  "isReplayedBy" - {
    def genTrans(node: Node) = {
      val nid = NodeId(1)
-      val version = node.optVersion.getOrElse(TransactionVersion.minExceptions)
+      val version = node.optVersion.getOrElse(TransactionVersion.minVersion)
      VersionedTransaction(version, HashMap(nid -> node), ImmArray(nid))
    }

@ -227,7 +227,7 @@ class TransactionSpec
      }

      forAll(genEmptyNode, minSuccessful(10)) { n =>
-        val version = n.optVersion.getOrElse(TransactionVersion.minExceptions)
+        val version = n.optVersion.getOrElse(TransactionVersion.minVersion)
        n match {
          case _: Node.Rollback => ()
          case n: Node.Action =>
--- a/daml-lf/transaction/src/test/scala/com/digitalasset/daml/lf/validation/ValidationSpec.scala
+++ b/daml-lf/transaction/src/test/scala/com/digitalasset/daml/lf/validation/ValidationSpec.scala
@ -165,7 +165,7 @@ class ValidationSpec extends AnyFreeSpec with Matchers with TableDrivenPropertyC
  private def flatVTXs: Seq[VTX] =
    (someCreates ++ someFetches ++ someLookups ++ someExercises).map { node =>
      val nid = NodeId(0)
-      val version = TransactionVersion.minExceptions
+      val version = TransactionVersion.minVersion
      VersionedTransaction(version, HashMap(nid -> node), ImmArray(nid))
    }

@ -177,7 +177,7 @@ class ValidationSpec extends AnyFreeSpec with Matchers with TableDrivenPropertyC
      val nid0 = NodeId(0)
      val nid1 = NodeId(1)
      val parent = exe.copy(children = ImmArray(nid1))
-      val version = TransactionVersion.minExceptions
+      val version = TransactionVersion.minVersion
      VersionedTransaction(
        version,
        HashMap(nid0 -> parent, nid1 -> child),