specify that deduplication offsets are exclusive (#12488)

CHANGELOG_BEGIN CHANGELOG_END
digital-asset · Jan 21, 2022 · 20cda01 · 20cda01
1 parent e9e1b06
commit 20cda01
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 10 deletions.
diff --git a/docs/source/app-dev/command-deduplication.rst b/docs/source/app-dev/command-deduplication.rst
@@ -219,7 +219,7 @@ Fields in the error metadata are written as ``field`` in lowercase letters.
 
    - * ``FAILED_PRECONDITION`` / :ref:`INVALID_DEDUPLICATION_PERIOD <error_code_INVALID_DEDUPLICATION_PERIOD>`
 
-     * The specified deduplication period is longer than what the Daml ledger supports.
+     * The specified deduplication period is longer than what the Daml ledger supports or the ledger cannot handle the specified deduplication offset.
        ``earliest_offset`` contains the earliest deduplication offset or ``longest_duration`` contains the longest deduplication duration that can be used (at least one of the two must be provided).
 
        Options:
@@ -305,6 +305,11 @@ We recommend the following strategy for using deduplication offsets:
 #. Obtain a recent offset ``OFF0`` on the completion event stream and remember across crashes that you use ``OFF0`` with the chosen command ID. There are several ways to do so:
 
    - Use the :ref:`Command Completion Service <command-completion-service>` by asking for the :ref:`current ledger end <com.daml.ledger.api.v1.CompletionEndRequest>`.
+
+     .. note::
+	Some ledger implementations reject deduplication offsets that do not identify a command completion visible to the submitting parties with the error code id :ref:`INVALID_DEDUPLICATION_PERIOD <error_code_INVALID_DEDUPLICATION_PERIOD>`.
+	In general, the ledger end need not identify a command completion that is visible to the submitting parties.
+	When running on such a ledger, use the Command Service approach described next.
 
    - Use the :ref:`Command Service <command-service>` to obtain a recent offset by repeatedly submitting a dummy command, e.g., a :ref:`Create-And-Exercise command <com.daml.ledger.api.v1.CreateAndExerciseCommand>` of some single-signatory template with the :ref:`Archive <function-da-internal-template-functions-archive-52202>` choice, until you get a successful response.
      The response contains the :ref:`completion offset <com.daml.ledger.api.v1.SubmitAndWaitForTransactionIdResponse.completion_offset>`.

diff --git a/ledger-api/grpc-definitions/com/daml/ledger/api/v1/commands.proto b/ledger-api/grpc-definitions/com/daml/ledger/api/v1/commands.proto
@@ -73,7 +73,7 @@ message Commands {
     // ``ledger_configuration_service.proto``).
     google.protobuf.Duration deduplication_duration = 15;
 
-    // Specifies the start of the deduplication period by a completion stream offset.
+    // Specifies the start of the deduplication period by a completion stream offset (exclusive).
     // Must be a valid LedgerString (as described in ``ledger_offset.proto``).
     string deduplication_offset = 16;
   }

diff --git a/ledger-api/grpc-definitions/com/daml/ledger/api/v1/completion.proto b/ledger-api/grpc-definitions/com/daml/ledger/api/v1/completion.proto
@@ -59,7 +59,7 @@ message Completion {
   //
   // Optional; the deduplication guarantee applies even if the completion omits this field.
   oneof deduplication_period {
-    // Specifies the start of the deduplication period by a completion stream offset.
+    // Specifies the start of the deduplication period by a completion stream offset (exclusive).
     //
     // Must be a valid LedgerString (as described in ``value.proto``).
     string deduplication_offset = 8;

diff --git a/...er/ledger-api-domain/src/main/scala/com/digitalasset/ledger/api/DeduplicationPeriod.scala b/...er/ledger-api-domain/src/main/scala/com/digitalasset/ledger/api/DeduplicationPeriod.scala
@@ -73,7 +73,7 @@ object DeduplicationPeriod {
     )
   }
 
-  /** The `offset` defines the start of the deduplication period. */
+  /** The `offset` defines the start of the deduplication period (exclusive). */
   final case class DeduplicationOffset(offset: Offset) extends DeduplicationPeriod
 
   implicit val `DeduplicationPeriod to LoggingValue`: ToLoggingValue[DeduplicationPeriod] = {

diff --git a/...m/daml/ledger/api/testtool/infrastructure/assertions/CommandDeduplicationAssertions.scala b/...m/daml/ledger/api/testtool/infrastructure/assertions/CommandDeduplicationAssertions.scala
@@ -66,7 +66,7 @@ object CommandDeduplicationAssertions {
               s"No accepted completion with the command ID '${completionResponse.completion.commandId}' has been found",
             )
             assert(
-              acceptedCompletionResponse.offset.getAbsolute >= reportedOffset,
+              acceptedCompletionResponse.offset.getAbsolute > reportedOffset,
               s"No accepted completion with the command ID '${completionResponse.completion.commandId}' after the reported offset $reportedOffset has been found",
             )
             assert(

diff --git a/...test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CommandDeduplicationIT.scala b/...test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CommandDeduplicationIT.scala
@@ -482,7 +482,20 @@ final class CommandDeduplicationIT(
         .submitRequest(party, DummyWithAnnotation(party, "Duplicate command").create.command)
       val acceptedSubmissionId = newSubmissionId()
       runWithTimeModel(configuredParticipants) { delay =>
+        val dummyRequest = ledger.submitRequest(
+          party,
+          DummyWithAnnotation(party, "Dummy command to generate a completion offset").create.command,
+        )
         for {
+          // Send a dummy command to the ledger so that we obtain a recent offset
+          // We should be able to just grab the current ledger end,
+          // but the converter from offsets to durations cannot handle this yet.
+          dummyResponse <- submitRequestAndAssertCompletionAccepted(
+            ledger,
+            dummyRequest,
+            party,
+          )
+          offsetBeforeFirstCompletion = dummyResponse.offset
           response <- submitRequestAndAssertCompletionAccepted(
             ledger,
             updateSubmissionId(request, acceptedSubmissionId),
@@ -497,7 +510,7 @@ final class CommandDeduplicationIT(
             updateWithFreshSubmissionId(
               request.update(
                 _.commands.deduplicationPeriod := DeduplicationPeriod.DeduplicationOffset(
-                  Ref.HexString.assertFromString(response.offset.getAbsolute)
+                  Ref.HexString.assertFromString(offsetBeforeFirstCompletion.getAbsolute)
                 )
               )
             ),

diff --git a/.../com/daml/ledger/participant/state/kvutils/deduplication/DeduplicationPeriodSupport.scala b/.../com/daml/ledger/participant/state/kvutils/deduplication/DeduplicationPeriodSupport.scala
@@ -69,6 +69,13 @@ class DeduplicationPeriodSupport(
               },
               duration => {
                 logger.debug(s"Converted deduplication offset $offset to duration $duration")
+                // We implicitly extend the deduplication period slightly:
+                // If a later offset has the same record time as `offset` (e.g., in static time mode),
+                // command deduplication must consider this later offset.
+                // Yet, a deduplication duration cannot distinguish between offsets with the same record time.
+                // We therefore extend the deduplication period to include all offsets with the same record time
+                // as `offset`, including `offset` itself which would not have to be included in the deduplication period.
+                // This is allowed as the ledger implementation may extend the deduplication period.
                 validation.validate(
                   DeduplicationPeriod.DeduplicationDuration(duration),
                   maxDeduplicationDuration,

diff --git a/...r/participant-state/src/main/scala/com/daml/ledger/participant/state/v2/ReadService.scala b/...r/participant-state/src/main/scala/com/daml/ledger/participant/state/v2/ReadService.scala
@@ -77,10 +77,13 @@ trait ReadService extends ReportsHealth {
     * of the last [[Update.ConfigurationChanged]] before the [[Update.TransactionAccepted]].
     *
     * - *command deduplication*: Let there be a [[Update.TransactionAccepted]] with [[CompletionInfo]]
-    * or a [[Update.CommandRejected]] with [[CompletionInfo]] and [[Update.CommandRejected.definiteAnswer]] at offset `off2`
-    * and let `off1` be the completion offset where the [[CompletionInfo.optDeduplicationPeriod]] starts.
-    * Then there is no other [[Update.TransactionAccepted]] with [[CompletionInfo]] for the same [[CompletionInfo.changeId]]
-    * between the offsets `off1` and `off2` inclusive.
+    *   or a [[Update.CommandRejected]] with [[CompletionInfo]] at offset `off2`.
+    *   If `off2`'s [[CompletionInfo.optDeduplicationPeriod]] is a [[api.DeduplicationPeriod.DeduplicationOffset]],
+    *   let `off1` be the first offset after the deduplication offset.
+    *   If the deduplication period is a [[api.DeduplicationPeriod.DeduplicationDuration]],
+    *   let `off1` be the first offset whose record time is at most the duration before `off2`'s record time (inclusive).
+    *   Then there is no other [[Update.TransactionAccepted]] with [[CompletionInfo]] for the same [[CompletionInfo.changeId]]
+    *   between the offsets `off1` and `off2` inclusive.
     *
     *   So if a command submission has resulted in a [[Update.TransactionAccepted]],
     *   other command submissions with the same [[SubmitterInfo.changeId]] must be deduplicated