ledger-configuration: Return a structured error from checkTime. [KVL-…

…1002] (#10373)

* ledger-configuration: More rigorous tests for LedgerTimeModel.

Let's actually check the return values, not just the constructor.

* ledger-configuration: Create a type that represents time out of range.

Only used in testing for now.

* ledger-configuration: Return a structured error from `checkTime`.

CHANGELOG_BEGIN
CHANGELOG_END

* ledger-configuration: We like braces.

Co-authored-by: Miklos <[email protected]>

Co-authored-by: Miklos <[email protected]>

ledger/ledger-configuration/src/main/scala/com/daml/ledger/configuration/LedgerTimeModel.scala

@@ -5,18 +5,20 @@ package com.daml.ledger.configuration
 
 import java.time.{Duration, Instant}
 
+import com.daml.ledger.configuration.LedgerTimeModel._
+
 import scala.util.Try
 
 /** The ledger time model and associated validations. Some values are given by constructor args;
   * others are derived.
   *
-  * @param avgTransactionLatency The expected average latency of a transaction, i.e., the average
-  *                              time from submitting the transaction to a write service and the
-  *                              transaction being assigned a record time.
-  * @param minSkew               The minimimum skew between ledger time and record time:
-  *                              lt_TX >= rt_TX - minSkew
-  * @param maxSkew               The maximum skew between ledger time and record time:
-  *                              lt_TX <= rt_TX + maxSkew
+  * @param avgTransactionLatency  The expected average latency of a transaction, i.e., the average
+  *                               time from submitting the transaction to a write service and the
+  *                               transaction being assigned a record time.
+  * @param minSkew                The minimimum skew between ledger time and record time:
+  *                               lt_TX >= rt_TX - minSkew
+  * @param maxSkew                The maximum skew between ledger time and record time:
+  *                               lt_TX <= rt_TX + maxSkew
   * @throws IllegalArgumentException if the parameters aren't valid
   */
 case class LedgerTimeModel private (
@@ -31,13 +33,14 @@ case class LedgerTimeModel private (
   def checkTime(
       ledgerTime: Instant,
       recordTime: Instant,
-  ): Either[String, Unit] = {
+  ): Either[OutOfRange, Unit] = {
     val lowerBound = minLedgerTime(recordTime)
     val upperBound = maxLedgerTime(recordTime)
-    if (ledgerTime.isBefore(lowerBound) || ledgerTime.isAfter(upperBound))
-      Left(s"Ledger time $ledgerTime outside of range [$lowerBound, $upperBound]")
-    else
+    if (ledgerTime.isBefore(lowerBound) || ledgerTime.isAfter(upperBound)) {
+      Left(OutOfRange(ledgerTime, lowerBound, upperBound))
+    } else {
       Right(())
+    }
   }
 
   private[ledger] def minLedgerTime(recordTime: Instant): Instant =
@@ -76,4 +79,10 @@ object LedgerTimeModel {
       require(!maxSkew.isNegative, "Negative max skew")
       new LedgerTimeModel(avgTransactionLatency, minSkew, maxSkew)
     }
+
+  final case class OutOfRange(ledgerTime: Instant, lowerBound: Instant, upperBound: Instant) {
+    lazy val message: String =
+      s"Ledger time $ledgerTime outside of range [$lowerBound, $upperBound]"
+  }
+
 }

ledger/ledger-configuration/src/test/suite/scala/com/daml/ledger/configuration/LedgerTimeModelSpec.scala

@@ -5,90 +5,114 @@ package com.daml.ledger.configuration
 
 import java.time._
 
+import com.daml.ledger.configuration.LedgerTimeModel.OutOfRange
 import org.scalatest.matchers.should.Matchers
 import org.scalatest.wordspec.AnyWordSpec
 
 class LedgerTimeModelSpec extends AnyWordSpec with Matchers {
 
   private val referenceTime = Instant.EPOCH
   private val epsilon = Duration.ofMillis(10L)
+  private val defaultSkew = Duration.ofSeconds(30L)
   private val timeModel =
     LedgerTimeModel(
       avgTransactionLatency = Duration.ZERO,
-      minSkew = Duration.ofSeconds(30L),
-      maxSkew = Duration.ofSeconds(30L),
+      minSkew = defaultSkew,
+      maxSkew = defaultSkew,
     ).get
   private val smallSkew = Duration.ofSeconds(1L)
   private val largeSkew = Duration.ofHours(1L)
 
   "Ledger time model" when {
     "checking ledger time" should {
       "succeed if the ledger time equals the record time" in {
-        timeModel.checkTime(referenceTime, referenceTime).isRight shouldEqual true
+        val result = timeModel.checkTime(referenceTime, referenceTime)
+
+        result should be(Right(()))
       }
 
       "succeed if the ledger time is higher than the record time and is within tolerance limit" in {
-        timeModel.checkTime(referenceTime.plus(epsilon), referenceTime).isRight shouldEqual true
+        val result = timeModel.checkTime(referenceTime.plus(epsilon), referenceTime)
+
+        result should be(Right(()))
       }
 
       "succeed if the ledger time is equal to the high boundary" in {
-        timeModel
-          .checkTime(referenceTime.plus(timeModel.maxSkew), referenceTime)
-          .isRight shouldEqual true
+        val result = timeModel.checkTime(referenceTime.plus(timeModel.maxSkew), referenceTime)
+
+        result should be(Right(()))
       }
 
       "fail if the ledger time is higher than the high boundary" in {
-        timeModel
-          .checkTime(referenceTime.plus(timeModel.maxSkew).plus(epsilon), referenceTime)
-          .isRight shouldEqual false
+        val ledgerTime = referenceTime.plus(timeModel.maxSkew).plus(epsilon)
+        val minRecordTime = referenceTime.minus(defaultSkew)
+        val maxRecordTime = referenceTime.plus(defaultSkew)
+
+        val result = timeModel.checkTime(ledgerTime, referenceTime)
+
+        result should be(Left(OutOfRange(ledgerTime, minRecordTime, maxRecordTime)))
       }
 
       "succeed if the ledger time is lower than the record time and is within tolerance limit" in {
-        timeModel.checkTime(referenceTime.minus(epsilon), referenceTime).isRight shouldEqual true
+        val result = timeModel.checkTime(referenceTime.minus(epsilon), referenceTime)
+
+        result should be(Right(()))
       }
 
       "succeed if the ledger time is equal to the low boundary" in {
-        timeModel
-          .checkTime(referenceTime.minus(timeModel.minSkew), referenceTime)
-          .isRight shouldEqual true
+        val result = timeModel.checkTime(referenceTime.minus(timeModel.minSkew), referenceTime)
+
+        result should be(Right(()))
       }
 
       "fail if the ledger time is lower than the low boundary" in {
-        timeModel
-          .checkTime(referenceTime.minus(timeModel.minSkew).minus(epsilon), referenceTime)
-          .isRight shouldEqual false
+        val ledgerTime = referenceTime.minus(timeModel.minSkew).minus(epsilon)
+        val minRecordTime = referenceTime.minus(defaultSkew)
+        val maxRecordTime = referenceTime.plus(defaultSkew)
+
+        val result = timeModel.checkTime(ledgerTime, referenceTime)
+
+        result should be(Left(OutOfRange(ledgerTime, minRecordTime, maxRecordTime)))
       }
 
       "succeed if the ledger time is equal to the high boundary (asymmetric case)" in {
-        val instance = createAsymmetricTimeModel(largeSkew, smallSkew)
+        val instance = createAsymmetricTimeModel(minSkew = largeSkew, maxSkew = smallSkew)
 
-        instance
-          .checkTime(referenceTime.plus(instance.maxSkew), referenceTime)
-          .isRight shouldEqual true
+        val result = instance.checkTime(referenceTime.plus(instance.maxSkew), referenceTime)
+
+        result should be(Right(()))
       }
 
       "succeed if the ledger time is equal to the low boundary (asymmetric case)" in {
-        val instance = createAsymmetricTimeModel(smallSkew, largeSkew)
+        val instance = createAsymmetricTimeModel(minSkew = smallSkew, maxSkew = largeSkew)
+
+        val result = instance.checkTime(referenceTime.minus(instance.minSkew), referenceTime)
 
-        instance
-          .checkTime(referenceTime.minus(instance.minSkew), referenceTime)
-          .isRight shouldEqual true
+        result should be(Right(()))
       }
 
       "fail if the ledger time is higher than the high boundary (asymmetric case)" in {
-        val instance = createAsymmetricTimeModel(largeSkew, smallSkew)
+        val instance = createAsymmetricTimeModel(minSkew = largeSkew, maxSkew = smallSkew)
+
+        val ledgerTime = referenceTime.plus(instance.maxSkew).plus(epsilon)
+        val minRecordTime = referenceTime.minus(largeSkew)
+        val maxRecordTime = referenceTime.plus(smallSkew)
 
-        instance
-          .checkTime(referenceTime.plus(instance.maxSkew).plus(epsilon), referenceTime)
-          .isLeft shouldEqual true
+        val result = instance.checkTime(ledgerTime, referenceTime)
+
+        result should be(Left(OutOfRange(ledgerTime, minRecordTime, maxRecordTime)))
       }
 
       "fail if the ledger time is lower than the low boundary (asymmetric case)" in {
-        val instance = createAsymmetricTimeModel(smallSkew, largeSkew)
+        val instance = createAsymmetricTimeModel(minSkew = smallSkew, maxSkew = largeSkew)
+
+        val ledgerTime = referenceTime.minus(instance.minSkew).minus(epsilon)
+        val minRecordTime = referenceTime.minus(smallSkew)
+        val maxRecordTime = referenceTime.plus(largeSkew)
 
-        instance
-          .checkTime(referenceTime.minus(instance.minSkew).minus(epsilon), referenceTime)
-          .isLeft shouldEqual true
+        val result = instance.checkTime(ledgerTime, referenceTime)
+
+        result should be(Left(OutOfRange(ledgerTime, minRecordTime, maxRecordTime)))
       }
 
       "produce a valid error message" in {
@@ -97,16 +121,15 @@ class LedgerTimeModelSpec extends AnyWordSpec with Matchers {
           minSkew = Duration.ofSeconds(10L),
           maxSkew = Duration.ofSeconds(20L),
         ).get
-        val ledgerTime = "2000-01-01T12:00:00Z"
-        val recordTime = "2000-01-01T12:30:00Z"
-        val lowerBound = "2000-01-01T12:29:50Z"
-        val upperBound = "2000-01-01T12:30:20Z"
-        val expectedMessage = s"Ledger time $ledgerTime outside of range [$lowerBound, $upperBound]"
-
-        timeModel
-          .checkTime(Instant.parse(ledgerTime), Instant.parse(recordTime)) shouldEqual Left(
-          expectedMessage
-        )
+
+        val ledgerTime = Instant.parse("2000-01-01T12:00:00Z")
+        val recordTime = Instant.parse("2000-01-01T12:30:00Z")
+        val minRecordTime = Instant.parse("2000-01-01T12:29:50Z")
+        val maxRecordTime = Instant.parse("2000-01-01T12:30:20Z")
+
+        val result = timeModel.checkTime(ledgerTime, recordTime)
+
+        result should be(Left(OutOfRange(ledgerTime, minRecordTime, maxRecordTime)))
       }
     }
   }

ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/validation/LedgerTimeValidator.scala

@@ -39,10 +39,10 @@ private[transaction] class LedgerTimeValidator(defaultConfig: Configuration)
             timeModel
               .checkTime(ledgerTime = givenLedgerTime, recordTime = recordTime.toInstant)
               .fold(
-                reason =>
+                outOfRange =>
                   rejections.buildRejectionStep(
                     transactionEntry,
-                    RejectionReasonV0.InvalidLedgerTime(reason),
+                    RejectionReasonV0.InvalidLedgerTime(outOfRange.message),
                     commitContext.recordTime,
                   ),
                 _ => StepContinue(transactionEntry),

ledger/sandbox-classic/src/main/scala/platform/sandbox/stores/ledger/TimeModelError.scala

@@ -0,0 +1,23 @@
+// Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package com.daml.platform.sandbox.stores.ledger
+
+import com.daml.ledger.configuration.LedgerTimeModel
+
+private sealed trait TimeModelError {
+  def message: String
+}
+
+private object TimeModelError {
+  object NoLedgerConfiguration extends TimeModelError {
+    val message: String =
+      "No ledger configuration available, cannot validate ledger time"
+  }
+
+  final case class InvalidLedgerTime(outOfRange: LedgerTimeModel.OutOfRange)
+      extends TimeModelError {
+    lazy val message: String =
+      outOfRange.message
+  }
+}

ledger/sandbox-classic/src/main/scala/platform/sandbox/stores/ledger/inmemory/InMemoryLedger.scala

@@ -51,9 +51,9 @@ import com.daml.platform.index.TransactionConversion
 import com.daml.platform.packages.InMemoryPackageStore
 import com.daml.platform.participant.util.LfEngineToApi
 import com.daml.platform.sandbox.stores.InMemoryActiveLedgerState
-import com.daml.platform.sandbox.stores.ledger.Ledger
 import com.daml.platform.sandbox.stores.ledger.ScenarioLoader.LedgerEntryOrBump
 import com.daml.platform.sandbox.stores.ledger.inmemory.InMemoryLedger._
+import com.daml.platform.sandbox.stores.ledger.{Ledger, TimeModelError}
 import com.daml.platform.store.CompletionFromTransaction
 import com.daml.platform.store.Contract.ActiveContract
 import com.daml.platform.store.entries.{
@@ -286,12 +286,18 @@ private[sandbox] final class InMemoryLedger(
     )
 
   // Validates the given ledger time according to the ledger time model
-  private def checkTimeModel(ledgerTime: Instant, recordTime: Instant): Either[String, Unit] = {
+  private def checkTimeModel(
+      ledgerTime: Instant,
+      recordTime: Instant,
+  ): Either[TimeModelError, Unit] =
     ledgerConfiguration
-      .fold[Either[String, Unit]](
-        Left("No ledger configuration available, cannot validate ledger time")
-      )(config => config.timeModel.checkTime(ledgerTime, recordTime))
-  }
+      .toRight(TimeModelError.NoLedgerConfiguration)
+      .flatMap(config =>
+        config.timeModel
+          .checkTime(ledgerTime, recordTime)
+          .left
+          .map(TimeModelError.InvalidLedgerTime)
+      )
 
   private def handleSuccessfulTx(
       transactionId: Ref.LedgerString,
@@ -303,7 +309,7 @@ private[sandbox] final class InMemoryLedger(
     val recordTime = timeProvider.getCurrentTime
     checkTimeModel(ledgerTime, recordTime)
       .fold(
-        reason => handleError(submitterInfo, RejectionReason.InvalidLedgerTime(reason)),
+        error => handleError(submitterInfo, RejectionReason.InvalidLedgerTime(error.message)),
         _ => {
           val (committedTransaction, disclosureForIndex, divulgence) =
             Ledger

ledger/sandbox-classic/src/main/scala/platform/sandbox/stores/ledger/sql/SqlLedger.scala

@@ -42,7 +42,7 @@ import com.daml.platform.sandbox.LedgerIdGenerator
 import com.daml.platform.sandbox.config.LedgerName
 import com.daml.platform.sandbox.stores.ledger.ScenarioLoader.LedgerEntryOrBump
 import com.daml.platform.sandbox.stores.ledger.sql.SqlLedger._
-import com.daml.platform.sandbox.stores.ledger.{Ledger, SandboxOffset}
+import com.daml.platform.sandbox.stores.ledger.{Ledger, SandboxOffset, TimeModelError}
 import com.daml.platform.store.appendonlydao.events.CompressionStrategy
 import com.daml.platform.store.cache.TranslationCacheBackedContractStore
 import com.daml.platform.store.dao.{LedgerDao, LedgerWriteDao}
@@ -398,14 +398,12 @@ private final class SqlLedger(
   ): Either[RejectionReasonV0, Unit] = {
     currentConfiguration
       .get()
-      .fold[Either[RejectionReasonV0, Unit]](
-        Left(
-          RejectionReasonV0
-            .InvalidLedgerTime("No ledger configuration available, cannot validate ledger time")
-        )
-      )(
-        _.timeModel.checkTime(ledgerTime, recordTime).left.map(RejectionReasonV0.InvalidLedgerTime)
+      .toRight(TimeModelError.NoLedgerConfiguration)
+      .flatMap(
+        _.timeModel.checkTime(ledgerTime, recordTime).left.map(TimeModelError.InvalidLedgerTime)
       )
+      .left
+      .map(error => RejectionReasonV0.InvalidLedgerTime(error.message))
   }
 
   override def publishTransaction(