Fix random exercise node's childEventId order [DPP-1018] (#13740)

Fixes Index DB insertion order for childEventId.
Adds backwards compatibility treatment: recovering childEventId
order from the order of events at API stream rendering.
Extends TransactionServiceVisibilityIT with the ordering criterion.
Fixes compatibility tests with exclusion.
Adds documentation to event.proto.

CHANGELOG_BEGIN
Fixing Ledger API Bug: Exercise nodes in transaction trees
have child_event_ids out of order.
CHANGELOG_END

compatibility/bazel_tools/testing.bzl

@@ -819,6 +819,18 @@ excluded_test_tool_tests = [
             },
         ],
     },
+    {
+        # Fixing childEventId ordering: this test is now checking conformance to ordering, so it needs to be excluded for conformance tests which come after, and for versions which are older
+        "start": "2.2.0-snapshot.20220425.9780.1",
+        "platform_ranges": [
+            {
+                "end": "2.2.0-snapshot.20220425.9780.0.f4d60375",
+                "exclusions": [
+                    "TransactionServiceVisibilityIT:TXTreeChildOrder",
+                ],
+            },
+        ],
+    },
 ]
 
 def in_range(version, range):

ledger-api/grpc-definitions/com/daml/ledger/api/v1/event.proto

@@ -167,6 +167,7 @@ message ExercisedEvent {
 
   // References to further events in the same transaction that appeared as a result of this ``ExercisedEvent``.
   // It contains only the immediate children of this event, not all members of the subtree rooted at this node.
+  // The order of the children is the same as the event order in the transaction.
   // Each element must be a valid LedgerString (as described in ``value.proto``).
   // Optional
   repeated string child_event_ids = 11;

ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/api/v1/event/EventOps.scala

@@ -93,6 +93,14 @@ object EventOps {
           TreeEvent(TreeExercised(exercise.copy(childEventIds = exercise.childEventIds.filter(f)))),
         create => TreeEvent(TreeCreated(create)),
       )
+    def sortChildEventIdsBy(order: Map[String, Int]): TreeEvent =
+      event.kind.fold(
+        exercise =>
+          TreeEvent(
+            TreeExercised(exercise.copy(childEventIds = exercise.childEventIds.sortBy(order)))
+          ),
+        create => TreeEvent(TreeCreated(create)),
+      )
     def witnessParties: Seq[String] = event.kind.fold(_.witnessParties, _.witnessParties)
     def modifyWitnessParties(f: Seq[String] => Seq[String]): TreeEvent =
       event.kind.fold(

ledger/ledger-api-test-tool-on-canton/BUILD.bazel

@@ -104,6 +104,7 @@ conformance_test(
             "TLSAtLeastOnePointTwoIT",
             "CommandDeduplicationPeriodValidationIT:OffsetPruned",  # requires pruning not available in canton community
             "DeeplyNestedValueIT",  # FIXME: Too deeply nested values flake with a time out (half of the time)
+            "TransactionServiceVisibilityIT:TXTreeChildOrder",  # FIXME: Temporarily disabled: test enhanced testing childEventId order
         ]),
     ],
 ) if not is_windows else None

ledger/ledger-api-tests/suites/deps.bzl

@@ -4,6 +4,7 @@
 def deps(lf_version):
     return [
         "//daml-lf/data",
+        "//daml-lf/transaction",
         "//language-support/scala/bindings",
         "//ledger/error",
         "//ledger/ledger-api-common",

ledger/ledger-api-tests/suites/src/main/scala/com/daml/ledger/api/testtool/suites/v1_8/TransactionServiceVisibilityIT.scala

@@ -21,6 +21,7 @@ import com.daml.ledger.test.model.Iou.IouTransfer._
 import com.daml.ledger.test.model.IouTrade.IouTrade
 import com.daml.ledger.test.model.IouTrade.IouTrade._
 import com.daml.ledger.test.model.Test._
+import com.daml.lf.ledger.EventId
 import com.daml.test.evidence.tag.EvidenceTag
 import com.daml.test.evidence.tag.Security.SecurityTest
 import com.daml.test.evidence.tag.Security.SecurityTest.Property.Privacy
@@ -89,7 +90,6 @@ class TransactionServiceVisibilityIT extends LedgerTestSuite {
         dkkTree <- eventually("transactionTreeById3") {
           delta.transactionTreeById(tree.transactionId, dkk_bank)
         }
-
       } yield {
         def treeIsWellformed(tree: TransactionTree): Unit = {
           val eventsToObserve = mutable.Map.empty[String, TreeEvent] ++= tree.eventsById
@@ -141,6 +141,110 @@ class TransactionServiceVisibilityIT extends LedgerTestSuite {
       }
   })
 
+  test(
+    "TXTreeChildOrder",
+    "Trees formed by childEventIds should be maintaining Transaction event order",
+    allocate(TwoParties, SingleParty, SingleParty),
+    enabled = _.committerEventLog.eventLogType.isCentralized,
+    tags = privacyHappyCase(
+      asset = "Transaction Tree",
+      happyCase = "Trees formed by childEventIds should be maintaining Transaction event order",
+    ),
+  )(implicit ec => {
+    case Participants(
+          Participant(alpha, alice, gbp_bank),
+          Participant(beta, bob),
+          Participant(delta, dkk_bank),
+        ) =>
+      for {
+        gbpIouIssue <- alpha.create(gbp_bank, Iou(gbp_bank, gbp_bank, "GBP", 100, Nil))
+        gbpTransfer <-
+          alpha.exerciseAndGetContract(gbp_bank, gbpIouIssue.exerciseIou_Transfer(_, alice))
+        dkkIouIssue <- delta.create(dkk_bank, Iou(dkk_bank, dkk_bank, "DKK", 110, Nil))
+        dkkTransfer <-
+          delta.exerciseAndGetContract(dkk_bank, dkkIouIssue.exerciseIou_Transfer(_, bob))
+
+        aliceIou1 <- eventually("exerciseIouTransfer_Accept") {
+          alpha.exerciseAndGetContract(alice, gbpTransfer.exerciseIouTransfer_Accept(_))
+        }
+        aliceIou <- eventually("exerciseIou_AddObserver") {
+          alpha.exerciseAndGetContract(alice, aliceIou1.exerciseIou_AddObserver(_, bob))
+        }
+        bobIou <- eventually("exerciseIouTransfer_Accept") {
+          beta.exerciseAndGetContract(bob, dkkTransfer.exerciseIouTransfer_Accept(_))
+        }
+
+        trade <- eventually("create") {
+          alpha.create(
+            alice,
+            IouTrade(alice, bob, aliceIou, gbp_bank, "GBP", 100, dkk_bank, "DKK", 110),
+          )
+        }
+        tree <- eventually("exerciseIouTrade_Accept") {
+          beta.exercise(bob, trade.exerciseIouTrade_Accept(_, bobIou))
+        }
+
+        aliceTree <- eventually("transactionTreeById1") {
+          alpha.transactionTreeById(tree.transactionId, alice)
+        }
+        bobTree <- beta.transactionTreeById(tree.transactionId, bob)
+        gbpTree <- eventually("transactionTreeById2") {
+          alpha.transactionTreeById(tree.transactionId, gbp_bank)
+        }
+        dkkTree <- eventually("transactionTreeById3") {
+          delta.transactionTreeById(tree.transactionId, dkk_bank)
+        }
+        aliceTrees <- alpha.transactionTrees(alice)
+        bobTrees <- alpha.transactionTrees(bob)
+        gbpTrees <- alpha.transactionTrees(gbp_bank)
+        dkkTrees <- alpha.transactionTrees(dkk_bank)
+      } yield {
+        def treeIsWellformed(tree: TransactionTree): Unit = {
+          val eventsToObserve = mutable.Map.empty[String, TreeEvent] ++= tree.eventsById
+
+          def go(eventId: String): Unit = {
+            eventsToObserve.remove(eventId) match {
+              case Some(TreeEvent(Exercised(exercisedEvent))) =>
+                val expected = exercisedEvent.childEventIds.sortBy(stringEventId =>
+                  EventId.assertFromString(stringEventId).nodeId.index
+                )
+                val actual = exercisedEvent.childEventIds
+                assertEquals(
+                  context = s"childEventIds are out of order. Expected: $expected, actual: $actual",
+                  actual = actual,
+                  expected = expected,
+                )
+                exercisedEvent.childEventIds.foreach(go)
+              case Some(TreeEvent(_)) =>
+                ()
+              case None =>
+                throw new AssertionError(
+                  s"Referenced eventId $eventId is not available as node in the transaction."
+                )
+            }
+          }
+
+          tree.rootEventIds.foreach(go)
+          assert(
+            eventsToObserve.isEmpty,
+            s"After traversing the transaction, there are still unvisited nodes: $eventsToObserve",
+          )
+        }
+
+        treeIsWellformed(aliceTree)
+        treeIsWellformed(bobTree)
+        treeIsWellformed(gbpTree)
+        treeIsWellformed(dkkTree)
+
+        Iterator(
+          aliceTrees,
+          bobTrees,
+          gbpTrees,
+          dkkTrees,
+        ).flatten.foreach(treeIsWellformed)
+      }
+  })
+
   test(
     "TXNotDivulge",
     "Data should not be exposed to parties unrelated to a transaction",

ledger/participant-integration-api/src/main/scala/platform/store/backend/DbDto.scala

@@ -71,7 +71,7 @@ object DbDto {
       exercise_argument: Option[Array[Byte]],
       exercise_result: Option[Array[Byte]],
       exercise_actors: Option[Set[String]],
-      exercise_child_event_ids: Option[Set[String]],
+      exercise_child_event_ids: Option[Vector[String]],
       create_key_value_compression: Option[Int],
       exercise_argument_compression: Option[Int],
       exercise_result_compression: Option[Int],

ledger/participant-integration-api/src/main/scala/platform/store/backend/UpdateToDbDto.scala

@@ -217,7 +217,7 @@ object UpdateToDbDto {
                   exercise_child_event_ids = Some(
                     exercise.children.iterator
                       .map(EventId(u.transactionId, _).toLedgerString.toString)
-                      .toSet
+                      .toVector
                   ),
                   create_key_value_compression = compressionStrategy.createKeyValueCompression.id,
                   exercise_argument_compression =

ledger/participant-integration-api/src/main/scala/platform/store/dao/events/EventsTable.scala

@@ -87,14 +87,21 @@ object EventsTable {
       // The identifiers of all visible events in this transactions, preserving
       // the order in which they are retrieved from the index
       val visible = events.map(_.event.eventId)
-      val visibleSet = visible.toSet
+      val visibleOrder = visible.view.zipWithIndex.toMap
 
       // All events in this transaction by their identifier, with their children
       // filtered according to those visible for this request
       val eventsById =
         events.iterator
           .map(_.event)
-          .map(e => e.eventId -> e.filterChildEventIds(visibleSet))
+          .map(e =>
+            e.eventId -> e
+              .filterChildEventIds(visibleOrder.contains)
+              // childEventIds need to be returned in the event order in the original transaction.
+              // Unfortunately, we did not store them ordered in the past so we have to sort it to recover this order.
+              // The order is determined by the order of the events, which follows the event order of the original transaction.
+              .sortChildEventIdsBy(visibleOrder)
+          )
           .toMap
 
       // All event identifiers that appear as a child of another item in this response

ledger/participant-integration-api/src/main/scala/platform/store/dao/events/TransactionLogUpdatesConversions.scala

@@ -211,9 +211,16 @@ private[events] object TransactionLogUpdatesConversions {
           )
           .map { treeEvents =>
             val visible = treeEvents.map(_.eventId)
-            val visibleSet = visible.toSet
+            val visibleOrder = visible.view.zipWithIndex.toMap
             val eventsById = treeEvents.iterator
-              .map(e => e.eventId -> e.filterChildEventIds(visibleSet))
+              .map(e =>
+                e.eventId -> e
+                  .filterChildEventIds(visibleOrder.contains)
+                  // childEventIds need to be returned in the event order in the original transaction.
+                  // Unfortunately, we did not store them ordered in the past so we have to sort it to recover this order.
+                  // The order is determined by the order of the events, which follows the event order of the original transaction.
+                  .sortChildEventIdsBy(visibleOrder)
+              )
               .toMap
 
             // All event identifiers that appear as a child of another item in this response

ledger/participant-integration-api/src/test/lib/scala/platform/store/backend/StorageBackendTestValues.scala

@@ -184,7 +184,7 @@ private[backend] object StorageBackendTestValues {
       exercise_argument = Some(someSerializedDamlLfValue),
       exercise_result = Some(someSerializedDamlLfValue),
       exercise_actors = Some(Set(actor)),
-      exercise_child_event_ids = Some(Set.empty),
+      exercise_child_event_ids = Some(Vector.empty),
       create_key_value_compression = None,
       exercise_argument_compression = None,
       exercise_result_compression = None,

ledger/participant-integration-api/src/test/suite/scala/platform/store/backend/DbDtoToStringsForInterningSpec.scala

@@ -131,7 +131,7 @@ class DbDtoToStringsForInterningSpec extends AnyFlatSpec with Matchers {
       event_sequential_id = 1,
       exercise_choice = Some("60"),
       exercise_result = None,
-      exercise_child_event_ids = Some(Set("61", "62", "63")),
+      exercise_child_event_ids = Some(Vector("61", "62", "63")),
       exercise_result_compression = Some(1),
     ),
     DbDto.CommandCompletion(

ledger/participant-integration-api/src/test/suite/scala/platform/store/backend/UpdateToDbDtoSpec.scala

@@ -473,7 +473,7 @@ class UpdateToDbDtoSpec extends AnyWordSpec with Matchers {
           exercise_argument = Some(emptyArray),
           exercise_result = Some(emptyArray),
           exercise_actors = Some(Set("signatory")),
-          exercise_child_event_ids = Some(Set.empty),
+          exercise_child_event_ids = Some(Vector.empty),
           create_key_value_compression = compressionAlgorithmId,
           exercise_argument_compression = compressionAlgorithmId,
           exercise_result_compression = compressionAlgorithmId,
@@ -558,7 +558,7 @@ class UpdateToDbDtoSpec extends AnyWordSpec with Matchers {
           exercise_argument = Some(emptyArray),
           exercise_result = Some(emptyArray),
           exercise_actors = Some(Set("signatory")),
-          exercise_child_event_ids = Some(Set.empty),
+          exercise_child_event_ids = Some(Vector.empty),
           create_key_value_compression = compressionAlgorithmId,
           exercise_argument_compression = compressionAlgorithmId,
           exercise_result_compression = compressionAlgorithmId,
@@ -670,7 +670,7 @@ class UpdateToDbDtoSpec extends AnyWordSpec with Matchers {
           exercise_result = Some(emptyArray),
           exercise_actors = Some(Set("signatory")),
           exercise_child_event_ids = Some(
-            Set(
+            Vector(
               EventId(update.transactionId, exerciseNodeBId).toLedgerString,
               EventId(update.transactionId, exerciseNodeCId).toLedgerString,
             )
@@ -700,7 +700,7 @@ class UpdateToDbDtoSpec extends AnyWordSpec with Matchers {
           exercise_argument = Some(emptyArray),
           exercise_result = Some(emptyArray),
           exercise_actors = Some(Set("signatory")),
-          exercise_child_event_ids = Some(Set.empty),
+          exercise_child_event_ids = Some(Vector.empty),
           create_key_value_compression = compressionAlgorithmId,
           exercise_argument_compression = compressionAlgorithmId,
           exercise_result_compression = compressionAlgorithmId,
@@ -726,7 +726,7 @@ class UpdateToDbDtoSpec extends AnyWordSpec with Matchers {
           exercise_argument = Some(emptyArray),
           exercise_result = Some(emptyArray),
           exercise_actors = Some(Set("signatory")),
-          exercise_child_event_ids = Some(Set.empty),
+          exercise_child_event_ids = Some(Vector.empty),
           create_key_value_compression = compressionAlgorithmId,
           exercise_argument_compression = compressionAlgorithmId,
           exercise_result_compression = compressionAlgorithmId,
@@ -881,7 +881,7 @@ class UpdateToDbDtoSpec extends AnyWordSpec with Matchers {
           exercise_argument = Some(emptyArray),
           exercise_result = Some(emptyArray),
           exercise_actors = Some(Set("signatory")),
-          exercise_child_event_ids = Some(Set.empty),
+          exercise_child_event_ids = Some(Vector.empty),
           create_key_value_compression = compressionAlgorithmId,
           exercise_argument_compression = compressionAlgorithmId,
           exercise_result_compression = compressionAlgorithmId,
@@ -1010,7 +1010,7 @@ class UpdateToDbDtoSpec extends AnyWordSpec with Matchers {
         exercise_argument = Some(emptyArray),
         exercise_result = Some(emptyArray),
         exercise_actors = Some(Set("signatory")),
-        exercise_child_event_ids = Some(Set.empty),
+        exercise_child_event_ids = Some(Vector.empty),
         create_key_value_compression = compressionAlgorithmId,
         exercise_argument_compression = compressionAlgorithmId,
         exercise_result_compression = compressionAlgorithmId,
@@ -1114,7 +1114,7 @@ class UpdateToDbDtoSpec extends AnyWordSpec with Matchers {
           exercise_argument = Some(emptyArray),
           exercise_result = Some(emptyArray),
           exercise_actors = Some(Set("signatory")),
-          exercise_child_event_ids = Some(Set.empty),
+          exercise_child_event_ids = Some(Vector.empty),
           create_key_value_compression = compressionAlgorithmId,
           exercise_argument_compression = compressionAlgorithmId,
           exercise_result_compression = compressionAlgorithmId,

ledger/sandbox-on-x/BUILD.bazel

@@ -506,6 +506,20 @@ conformance_test(
     name = "conformance-test-in-memory-fan-out",
     ports = [6865],
     server = ":app",
+    server_args = [
+        "--contract-id-seeding=testing-weak",
+        "--participant participant-id=example,port=6865,ledger-api-transactions-buffer-max-size=100000",
+        "--buffered-ledger-api-streams",
+    ],
+    test_tool_args = [
+        "--verbose",
+    ],
+)
+
+conformance_test(
+    name = "conformance-test-in-memory-fan-out-tiny-buffer",
+    ports = [6865],
+    server = ":app",
     server_args = [
         "--contract-id-seeding=testing-weak",
         "--participant participant-id=example,port=6865,ledger-api-transactions-buffer-max-size=10",