From fe44764e610d1f174e66a2145d6617abb7c98764 Mon Sep 17 00:00:00 2001 From: Remy Date: Wed, 16 Dec 2020 11:27:54 +0100 Subject: [PATCH] LF: Refactor TransactionVersion (#8299) Simplify transaction version. In particular we drop the dependency to LfVersions abstract class. CHANGELOG_BEGIN CHANGELOG_END --- .../digitalasset/daml/lf/VersionRange.scala | 5 ++ .../daml/lf/repl/testing/Main.scala | 4 +- .../transaction/test/TransactionBuilder.scala | 2 +- .../scala/lf/value/test/ValueGenerators.scala | 7 +-- .../lf/transaction/TransactionCoder.scala | 6 +-- .../lf/transaction/TransactionVersion.scala | 53 +++++++++++-------- .../lf/transaction/TransactionCoderSpec.scala | 43 ++++++++------- .../daml/lf/transaction/TransactionSpec.scala | 2 +- .../transaction/TransactionVersionSpec.scala | 14 ++--- 9 files changed, 69 insertions(+), 67 deletions(-) diff --git a/daml-lf/data/src/main/scala/com/digitalasset/daml/lf/VersionRange.scala b/daml-lf/data/src/main/scala/com/digitalasset/daml/lf/VersionRange.scala index 58026d25e5..e01e63170a 100644 --- a/daml-lf/data/src/main/scala/com/digitalasset/daml/lf/VersionRange.scala +++ b/daml-lf/data/src/main/scala/com/digitalasset/daml/lf/VersionRange.scala @@ -22,12 +22,17 @@ final case class VersionRange[V]( max: V, )(implicit ordering: Ordering[V]) { + require(min <= max) + def intersect(that: VersionRange[V]): VersionRange[V] = VersionRange( min = this.min max that.min, max = this.max min that.max, ) + def map[W](f: V => W)(implicit ordering: Ordering[W]) = + VersionRange(f(min), f(max)) + def contains(v: V): Boolean = min <= v && v <= max diff --git a/daml-lf/repl/src/main/scala/com/digitalasset/daml/lf/repl/testing/Main.scala b/daml-lf/repl/src/main/scala/com/digitalasset/daml/lf/repl/testing/Main.scala index 4fa6c2ae02..6294ff45a6 100644 --- a/daml-lf/repl/src/main/scala/com/digitalasset/daml/lf/repl/testing/Main.scala +++ b/daml-lf/repl/src/main/scala/com/digitalasset/daml/lf/repl/testing/Main.scala @@ -198,12 +198,12 @@ object Repl { if (compilerConfig.allowedLanguageVersions.contains(LV.v1_dev)) ( value.ValueVersion.DevOutputVersions, - transaction.TransactionVersion.DevOutputVersions, + transaction.TransactionVersion.DevVersions, ) else ( value.ValueVersion.StableOutputVersions, - transaction.TransactionVersion.StableOutputVersions, + transaction.TransactionVersion.StableVersions, ) def run(expr: Expr): ( diff --git 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 index 1c33093445..44151e6b50 100644 --- 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 @@ -193,7 +193,7 @@ object TransactionBuilder { private val KeyWithMaintainers = Node.KeyWithMaintainers def apply(): TransactionBuilder = - TransactionBuilder(TransactionVersion.StableOutputVersions.min) + TransactionBuilder(TransactionVersion.StableVersions.min) def apply(txVersion: TransactionVersion): TransactionBuilder = new TransactionBuilder(_ => txVersion) diff --git 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 index 945a27d434..124ec331c4 100644 --- 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 @@ -507,12 +507,7 @@ object ValueGenerators { stringVersionGen.map(ValueVersion(_)).filterNot(ValueVersion.acceptedVersions.contains) def transactionVersionGen: Gen[TransactionVersion] = - Gen.oneOf(TransactionVersion.acceptedVersions) - - def unsupportedTransactionVersionGen: Gen[TransactionVersion] = - stringVersionGen - .map(TransactionVersion(_)) - .filterNot(TransactionVersion.acceptedVersions.contains) + Gen.oneOf(TransactionVersion.Values) object Implicits { implicit val vdateArb: Arbitrary[Time.Date] = Arbitrary(dateGen) diff --git 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 index e0d230af04..0bf1580f5e 100644 --- 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 @@ -464,11 +464,7 @@ object TransactionCoder { } def decodeVersion(vs: String): Either[DecodeError, TransactionVersion] = - TransactionVersion - .isAcceptedVersion(vs) - .fold[Either[DecodeError, TransactionVersion]]( - Left(DecodeError(s"Unsupported transaction version $vs")), - )(v => Right(v)) + TransactionVersion.fromString(vs).left.map(DecodeError) /** * Reads a [[VersionedTransaction]] from protobuf and checks if diff --git 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 index c854806649..e63cd8627b 100644 --- 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 @@ -7,50 +7,51 @@ package transaction import com.daml.lf.data.ImmArray import com.daml.lf.language.LanguageVersion import com.daml.lf.value.{Value, ValueVersion} -import scalaz.NonEmptyList import scala.collection.immutable.HashMap -final case class TransactionVersion(protoValue: String) +sealed abstract class TransactionVersion private (val protoValue: String, private val index: Int) + extends Product + with Serializable /** * Currently supported versions of the DAML-LF transaction specification. */ -object TransactionVersion - extends LfVersions(versionsAscending = - NonEmptyList(new TransactionVersion("10"), new TransactionVersion("dev")))( - _.protoValue, - ) { +object TransactionVersion { - private[lf] implicit val Ordering: Ordering[TransactionVersion] = mkOrdering + case object V10 extends TransactionVersion("10", 10) + case object VDev extends TransactionVersion("dev", Int.MaxValue) - private[lf] val List(v10, vDev) = acceptedVersions + val Values = List(V10, VDev) - val minVersion = v10 - private[transaction] val minChoiceObservers = vDev - private[transaction] val minNodeVersion = vDev + private[lf] implicit val Ordering: scala.Ordering[TransactionVersion] = scala.Ordering.by(_.index) - // Older versions are deprecated https://github.com/digital-asset/daml/issues/5220 - private[lf] val StableOutputVersions: VersionRange[TransactionVersion] = - VersionRange(v10, v10) + private[this] val stringMapping = Values.iterator.map(v => v.protoValue -> v).toMap - private[lf] val DevOutputVersions: VersionRange[TransactionVersion] = - StableOutputVersions.copy(max = acceptedVersions.last) + def fromString(vs: String): Either[String, TransactionVersion] = + stringMapping.get(vs).toRight(s"Unsupported transaction version $vs") + + def assertFromString(vs: String): TransactionVersion = + data.assertRight(fromString(vs)) + + val minVersion = Values.min + private[transaction] val minChoiceObservers = VDev + private[transaction] val minNodeVersion = VDev private[lf] val assignNodeVersion: LanguageVersion => TransactionVersion = { import LanguageVersion._ Map( - v1_6 -> v10, - v1_7 -> v10, - v1_8 -> v10, - v1_dev -> vDev, + v1_6 -> V10, + v1_7 -> V10, + v1_8 -> V10, + v1_dev -> VDev, ) } private[lf] val assignValueVersion: TransactionVersion => ValueVersion = { Map( - v10 -> ValueVersion("6"), - vDev -> ValueVersion("dev"), + V10 -> ValueVersion("6"), + VDev -> ValueVersion("dev"), ) } @@ -66,4 +67,10 @@ object TransactionVersion VersionedTransaction(txVersion, nodes, roots) } + private[lf] val StableVersions: VersionRange[TransactionVersion] = + LanguageVersion.StableVersions.map(assignNodeVersion) + + private[lf] val DevVersions: VersionRange[TransactionVersion] = + LanguageVersion.DevVersions.map(assignNodeVersion) + } diff --git 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 index c5ed09b716..e9b3f79453 100644 --- 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 @@ -32,9 +32,9 @@ class TransactionCoderSpec implicit override val generatorDrivenConfig: PropertyCheckConfiguration = PropertyCheckConfiguration(minSuccessful = 1000, sizeRange = 10) - import TransactionVersion.{v10, vDev} + import TransactionVersion.{V10, VDev} - private[this] val transactionVersions = Table("transaction version", v10, vDev) + private[this] val transactionVersions = Table("transaction version", V10, VDev) "encode-decode" should { @@ -221,31 +221,30 @@ class TransactionCoderSpec "transactions decoding should fail when unsupported transaction version received" in forAll(noDanglingRefGenTransaction, minSuccessful(50)) { tx => - forAll(unsupportedTransactionVersionGen, minSuccessful(20)) { - badTxVer: TransactionVersion => - TransactionVersion.acceptedVersions.contains(badTxVer) shouldEqual false - + forAll(stringVersionGen, minSuccessful(20)) { badTxVer => + whenever(TransactionVersion.fromString(badTxVer).isLeft) { val encodedTxWithBadTxVer: proto.Transaction = assertRight( TransactionCoder .encodeTransactionWithCustomVersion( TransactionCoder.NidEncoder, ValueCoder.CidEncoder, VersionedTransaction( - TransactionVersion.vDev, - tx.nodes.mapValues(_.updateVersion(TransactionVersion.vDev)), + TransactionVersion.VDev, + tx.nodes.mapValues(_.updateVersion(TransactionVersion.VDev)), tx.roots), ), - ).toBuilder.setVersion(badTxVer.protoValue).build() + ).toBuilder.setVersion(badTxVer).build() - encodedTxWithBadTxVer.getVersion shouldEqual badTxVer.protoValue + encodedTxWithBadTxVer.getVersion shouldEqual badTxVer TransactionCoder.decodeTransaction( TransactionCoder.NidDecoder, ValueCoder.CidDecoder, encodedTxWithBadTxVer, ) shouldEqual Left( - DecodeError(s"Unsupported transaction version ${badTxVer.protoValue}"), + DecodeError(s"Unsupported transaction version $badTxVer"), ) + } } } @@ -303,7 +302,7 @@ class TransactionCoderSpec // FIXME: https://github.com/digital-asset/daml/issues/7139 // replace all occurrences of "dev" by "11", once "11" is released "fail iff try to encode choice observers in version < dev" in { - val normalize = minimalistNode(v10) + val normalize = minimalistNode(V10) forAll(danglingRefExerciseNodeGen, minSuccessful(10)) { node => val shouldFail = node.choiceObservers.nonEmpty @@ -321,7 +320,7 @@ class TransactionCoderSpec ValueCoder.CidEncoder, txVersion, NodeId(0), - normalized.updateVersion(v10), + normalized.updateVersion(V10), ) result.isLeft shouldBe shouldFail @@ -355,18 +354,18 @@ class TransactionCoderSpec "decodeVersionedNode" should { """ignore field version if enclosing Transaction message is of version 10""" in { - val normalize = minimalistNode(v10) + val normalize = minimalistNode(V10) forAll(danglingRefGenNode, Gen.asciiStr, minSuccessful(10)) { case ((nodeId, node), str) => - val normalizedNode = normalize(node.updateVersion(v10)) + val normalizedNode = normalize(node.updateVersion(V10)) val Right(encoded) = for { encoded <- TransactionCoder .encodeNode( TransactionCoder.NidEncoder, ValueCoder.CidEncoder, - v10, + V10, nodeId, normalizedNode ) @@ -375,7 +374,7 @@ class TransactionCoderSpec TransactionCoder.decodeVersionedNode( TransactionCoder.NidDecoder, ValueCoder.CidDecoder, - v10, + V10, encoded, ) shouldBe Right(nodeId -> normalizedNode) } @@ -387,8 +386,8 @@ class TransactionCoderSpec forAll( danglingRefGenNode, - transactionVersionGen.filter(_ != v10), - transactionVersionGen.filter(_ != v10), + transactionVersionGen.filter(_ != V10), + transactionVersionGen.filter(_ != V10), minSuccessful(10)) { case ((nodeId, node), version1, version2) => whenever(version1 != version2) { @@ -418,7 +417,7 @@ class TransactionCoderSpec // FIXME: https://github.com/digital-asset/daml/issues/7139 // replace all occurrences of "dev" by "11", once "11" is released "ignore field observers in version < dev" in { - val normalize = minimalistNode(v10) + val normalize = minimalistNode(V10) forAll( Arbitrary.arbInt.arbitrary, @@ -427,7 +426,7 @@ class TransactionCoderSpec minSuccessful(50), ) { (nodeIdx, node, strings) => val nodeId = NodeId(nodeIdx) - val normalizedNode = normalize(node).updateVersion(v10) + val normalizedNode = normalize(node).updateVersion(V10) val Right(encoded) = for { @@ -435,7 +434,7 @@ class TransactionCoderSpec .encodeNode( TransactionCoder.NidEncoder, ValueCoder.CidEncoder, - v10, + V10, nodeId, normalizedNode, ) diff --git 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 index 507e6d98db..1ba77303a3 100644 --- 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 @@ -158,7 +158,7 @@ class TransactionSpec extends AnyFreeSpec with Matchers with ScalaCheckDrivenPro } "fail if version is different" in { - val versions = TransactionVersion.acceptedVersions.toIndexedSeq + val versions = TransactionVersion.Values def diffVersion(v: TransactionVersion) = { val randomVersion = versions(Random.nextInt(versions.length - 1)) if (randomVersion != v) randomVersion else versions.last diff --git a/daml-lf/transaction/src/test/scala/com/digitalasset/daml/lf/transaction/TransactionVersionSpec.scala b/daml-lf/transaction/src/test/scala/com/digitalasset/daml/lf/transaction/TransactionVersionSpec.scala index a7b3ed7187..e0f980447b 100644 --- a/daml-lf/transaction/src/test/scala/com/digitalasset/daml/lf/transaction/TransactionVersionSpec.scala +++ b/daml-lf/transaction/src/test/scala/com/digitalasset/daml/lf/transaction/TransactionVersionSpec.scala @@ -13,16 +13,16 @@ import org.scalatest.wordspec.AnyWordSpec class TransactionVersionSpec extends AnyWordSpec with Matchers with TableDrivenPropertyChecks { import LanguageVersion.{v1_6, v1_7, v1_8, v1_dev} - import TransactionVersion.{v10, vDev} + import TransactionVersion.{V10, VDev} "TransactionVersion.assignNodeVersion" should { val testCases = Table( "language version" -> "transaction version", - v1_6 -> v10, - v1_7 -> v10, - v1_8 -> v10, - v1_dev -> vDev, + v1_6 -> V10, + v1_7 -> V10, + v1_8 -> V10, + v1_dev -> VDev, ) "be stable" in { @@ -38,8 +38,8 @@ class TransactionVersionSpec extends AnyWordSpec with Matchers with TableDrivenP val testCases = Table( "input" -> "output", - v10 -> ValueVersion("6"), - vDev -> ValueVersion("dev") + V10 -> ValueVersion("6"), + VDev -> ValueVersion("dev") ) forEvery(testCases) { (input, expectedOutput) =>