Disallow empty command submission (#3270)

* Disallow empty command submission Fixes #592
2024-09-20 01:07:18 +03:00 · 2019-10-31 03:37:58 -04:00 · 2019-10-31 03:37:58 -04:00 · d9ae487fec
commit d9ae487fec
parent 819f8a8af5
10 changed files with 160 additions and 40 deletions
--- a/ledger/ledger-api-common/src/main/scala/com/digitalasset/ledger/api/validation/CommandsValidator.scala
+++ b/ledger/ledger-api-common/src/main/scala/com/digitalasset/ledger/api/validation/CommandsValidator.scala
@ -41,7 +41,8 @@ final class CommandsValidator(ledgerId: LedgerId) {
      let <- requirePresence(commands.ledgerEffectiveTime, "ledger_effective_time")
      ledgerEffectiveTime = TimestampConversion.toInstant(let)
      mrt <- requirePresence(commands.maximumRecordTime, "maximum_record_time")
-      validatedCommands <- validateInnerCommands(commands.commands, submitter)
+      commandz <- requireNonEmpty(commands.commands, "commands")
+      validatedCommands <- validateInnerCommands(commandz, submitter)
      ledgerEffectiveTimestamp <- Time.Timestamp
        .fromInstant(ledgerEffectiveTime)
        .left
--- a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/ledger/api/validation/SubmitRequestValidatorTest.scala
+++ b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/ledger/api/validation/SubmitRequestValidatorTest.scala
@ -6,14 +6,14 @@ package com.digitalasset.ledger.api.validation
 import java.time.Instant

 import com.digitalasset.api.util.TimestampConversion
-import com.digitalasset.daml.lf.command.{Commands => LfCommands}
+import com.digitalasset.daml.lf.command.{Commands => LfCommands, CreateCommand => LfCreateCommand}
 import com.digitalasset.daml.lf.data._
 import com.digitalasset.daml.lf.value.Value.ValueRecord
 import com.digitalasset.daml.lf.value.{Value => Lf}
 import com.digitalasset.ledger.api.DomainMocks
 import com.digitalasset.ledger.api.DomainMocks.{applicationId, commandId, workflowId}
 import com.digitalasset.ledger.api.domain.{LedgerId, Commands => ApiCommands}
-import com.digitalasset.ledger.api.v1.commands.Commands
+import com.digitalasset.ledger.api.v1.commands.{Command, Commands, CreateCommand}
 import com.digitalasset.ledger.api.v1.value.Value.Sum
 import com.digitalasset.ledger.api.v1.value.{
  List => ApiList,
@ -45,6 +45,14 @@ class SubmitRequestValidatorTest
    val submitter = "party"
    val let = TimestampConversion.fromInstant(Instant.now)
    val mrt = TimestampConversion.fromInstant(Instant.now)
+    val command =
+      Command(
+        Command.Command.Create(CreateCommand(
+          Some(Identifier("package", moduleName = "module", entityName = "entity")),
+          Some(Record(
+            Some(Identifier("package", moduleName = "module", entityName = "entity")),
+            Seq(RecordField("something", Some(Value(Value.Sum.Bool(true)))))))
+        )))

    val commands = Commands(
      ledgerId.unwrap,
@ -54,7 +62,7 @@ class SubmitRequestValidatorTest
      submitter,
      Some(let),
      Some(mrt),
-      Seq.empty
+      Seq(command)
    )
  }

@ -73,7 +81,23 @@ class SubmitRequestValidatorTest
      mrt,
      LfCommands(
        DomainMocks.party,
-        ImmArray.empty,
+        ImmArray(
+          LfCreateCommand(
+            Ref.Identifier(
+              Ref.PackageId.assertFromString("package"),
+              Ref.QualifiedName(
+                Ref.ModuleName.assertFromString("module"),
+                Ref.DottedName.assertFromString("entity"))),
+            Lf.ValueRecord(
+              Option(
+                Ref.Identifier(
+                  Ref.PackageId.assertFromString("package"),
+                  Ref.QualifiedName(
+                    Ref.ModuleName.assertFromString("module"),
+                    Ref.DottedName.assertFromString("entity")))),
+              ImmArray((Option(Ref.Name.assertFromString("something")), Lf.ValueTrue))
+            )
+          )),
        Time.Timestamp.assertFromInstant(let),
        workflowId.unwrap
      )
@ -89,8 +113,11 @@ class SubmitRequestValidatorTest

  "CommandSubmissionRequestValidator" when {
    "validating command submission requests" should {
-      "convert valid requests with empty commands" in {
-        commandsValidator.validateCommands(api.commands) shouldEqual Right(internal.emptyCommands)
+      "reject requests with empty commands" in {
+        requestMustFailWith(
+          commandsValidator.validateCommands(api.commands.withCommands(Seq.empty)),
+          INVALID_ARGUMENT,
+          "Missing field: commands")
      }

      "not allow missing ledgerId" in {
--- a/ledger/ledger-api-integration-tests/src/test/itsuite/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/CommandClientIT.scala
+++ b/ledger/ledger-api-integration-tests/src/test/itsuite/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/CommandClientIT.scala
@ -18,10 +18,10 @@ import com.digitalasset.ledger.api.v1.ledger_offset.LedgerOffset
 import com.digitalasset.ledger.api.v1.ledger_offset.LedgerOffset.LedgerBoundary.LEDGER_BEGIN
 import com.digitalasset.ledger.api.v1.ledger_offset.LedgerOffset.Value.Boundary
 import com.digitalasset.ledger.api.v1.transaction_filter.{Filters, TransactionFilter}
-import com.digitalasset.ledger.api.v1.value.{Record, RecordField}
+import com.digitalasset.ledger.api.v1.value.{Record, RecordField, Value}
 import com.digitalasset.ledger.client.services.commands.{CommandClient, CompletionStreamElement}
 import com.digitalasset.platform.apitesting.LedgerContextExtensions._
-import com.digitalasset.platform.apitesting.TestTemplateIds
+import com.digitalasset.platform.apitesting.LedgerContext
 import com.digitalasset.platform.esf.TestExecutionSequencerFactory
 import com.digitalasset.platform.participant.util.ValueConversions._
 import com.digitalasset.platform.tests.integration.ledger.api.ParameterShowcaseTesting
@ -31,8 +31,8 @@ import io.grpc.{Status, StatusRuntimeException}
 import org.scalatest.time.Span
 import org.scalatest.time.SpanSugar._
 import org.scalatest.{AsyncWordSpec, Matchers, Succeeded, TryValues}
-
 import scalaz.syntax.tag._
+
 import scala.concurrent.Future
 import scala.concurrent.duration.FiniteDuration
 import scala.util.Success
@ -48,15 +48,22 @@ class CommandClientIT
    with TryValues
    with MultiLedgerCommandUtils {

-  protected val testTemplateIds = new TestTemplateIds(config)
-  protected val templateIds = testTemplateIds.templateIds
-
  private val submittingParty: String = submitRequest.getCommands.party
  private val submittingPartyList = List(submittingParty)
  private val LedgerBegin = LedgerOffset(Boundary(LEDGER_BEGIN))

-  private def submitRequestWithId(id: String): SubmitRequest =
-    submitRequest.copy(commands = submitRequest.commands.map(_.copy(commandId = id)))
+  private def submitRequestWithId(id: String, ctx: LedgerContext): SubmitRequest =
+    ctx.command(
+      id,
+      submittingParty,
+      List(
+        CreateCommand(
+          Some(templateIds.dummy),
+          Some(
+            Record(
+              Some(templateIds.dummy),
+              Seq(RecordField("operator", Option(Value(Value.Sum.Party(submittingParty)))))))).wrap)
+    )

  // Commands and completions can be read out of order. Since we use GRPC monocalls to send,
  // they can even be sent out of order.
@ -119,7 +126,7 @@ class CommandClientIT

        for {
          client <- ctx.commandClient()
-          result <- Source(contexts.map(i => Ctx(i, submitRequestWithId(i.toString))))
+          result <- Source(contexts.map(i => Ctx(i, submitRequestWithId(i.toString, ctx))))
            .via(client.submissionFlow)
            .map(_.map(_.isSuccess))
            .runWith(Sink.seq)
@ -130,9 +137,10 @@ class CommandClientIT

      "fail with the expected status on a ledger Id mismatch" in allFixtures { ctx =>
        Source
-          .single(Ctx(
-            1,
-            submitRequestWithId(1.toString).update(_.commands.ledgerId := testNotLedgerId.unwrap)))
+          .single(
+            Ctx(
+              1,
+              submitRequestWithId("1", ctx).update(_.commands.ledgerId := testNotLedgerId.unwrap)))
          .via(ctx.commandClientWithoutTime(testNotLedgerId).submissionFlow)
          .runWith(Sink.head)
          .map(err => IsStatusException(Status.NOT_FOUND)(err.value.failure.exception))
@ -141,7 +149,7 @@ class CommandClientIT
      "fail with INVALID REQUEST for empty application ids" in allFixtures { ctx =>
        val resF = for {
          client <- ctx.commandClient(applicationId = "")
-          request = submitRequestWithId(7000.toString).update(_.commands.applicationId := "")
+          request = submitRequestWithId("7000", ctx).update(_.commands.applicationId := "")
          res <- client.submitSingleCommand(request)
        } yield (res)

@ -192,7 +200,7 @@ class CommandClientIT
            client <- ctx.commandClient()
            checkpoint <- client.getCompletionEnd
            submissionResults <- Source(
-              commandIds.map(i => Ctx(i, submitRequestWithId(i.toString))))
+              commandIds.map(i => Ctx(i, submitRequestWithId(i.toString, ctx))))
              .flatMapMerge(10, randomDelay)
              .via(client.submissionFlow)
              .map(_.value)
@ -225,7 +233,7 @@ class CommandClientIT
          client <- ctx.commandClient()
          checkpoint <- client.getCompletionEnd
          result = readExpectedCommandIds(client, checkpoint.getOffset, commandIdStrings)
-          _ <- Source(commandIds.map(i => Ctx(i, submitRequestWithId(i.toString))))
+          _ <- Source(commandIds.map(i => Ctx(i, submitRequestWithId(i.toString, ctx))))
            .flatMapMerge(10, randomDelay)
            .via(client.submissionFlow)
            .map(_.context)
@ -252,7 +260,7 @@ class CommandClientIT
        for {
          client <- ctx.commandClient()
          tracker <- client.trackCommands[Int](submittingPartyList)
-          result <- Source(contexts.map(i => Ctx(i, submitRequestWithId(i.toString))))
+          result <- Source(contexts.map(i => Ctx(i, submitRequestWithId(i.toString, ctx))))
            .via(tracker)
            .map(_.context)
            .runWith(Sink.seq)
@ -265,7 +273,7 @@ class CommandClientIT
        for {
          client <- ctx.commandClient()
          tracker <- client.trackCommands[Int](submittingPartyList)
-          result <- Source.empty[Ctx[Int, SubmitRequest]].via(tracker).runWith(Sink.ignore)
+          _ <- Source.empty[Ctx[Int, SubmitRequest]].via(tracker).runWith(Sink.ignore)
        } yield {
          succeed
        }
--- a/ledger/ledger-api-integration-tests/src/test/itsuite/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/CommandCompletionServiceIT.scala
+++ b/ledger/ledger-api-integration-tests/src/test/itsuite/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/CommandCompletionServiceIT.scala
@ -46,7 +46,9 @@ import scalaz.syntax.tag._
 import scala.concurrent.Future
 import scala.concurrent.duration._
 import com.digitalasset.platform.apitesting.TestParties._
+import com.digitalasset.platform.participant.util.ValueConversions._
 import com.digitalasset.platform.testing.SingleItemObserver
+
@SuppressWarnings(Array("org.wartremover.warts.Any"))
 class CommandCompletionServiceIT
    extends AsyncWordSpec
@ -75,7 +77,21 @@ class CommandCompletionServiceIT
        for {
          commandClient <- ctx.commandClient(ctx.ledgerId)
          tracker <- commandClient.trackCommands[String](configuredParties)
-          commands = configuredParties.map(p => Ctx(p, ctx.command(p, p, Nil)))
+          commands = configuredParties.map(
+            p =>
+              Ctx(
+                p,
+                ctx.command(
+                  p,
+                  p,
+                  List(
+                    ProtoCreateCommand(
+                      Some(templateIds.dummy),
+                      Some(Record(
+                        Some(templateIds.dummy),
+                        Seq(RecordField("operator", Option(Value(Value.Sum.Party(p)))))))).wrap)
+                )
+            ))
          result <- Source(commands).via(tracker).runWith(Sink.seq)
        } yield {
          val expected = configuredParties.map(p => (p, 0))
@ -132,7 +148,17 @@ class CommandCompletionServiceIT
            startingOffset = startingOffsetResponse.getOffset
            commandClient <- ctx.commandClient(ctx.ledgerId)
            commands = configuredParties
-              .map(p => ctx.command(p, p, Nil))
+              .map(p =>
+                ctx.command(
+                  p,
+                  p,
+                  List(
+                    ProtoCreateCommand(
+                      Some(templateIds.dummy),
+                      Some(Record(
+                        Some(templateIds.dummy),
+                        Seq(RecordField("operator", Option(Value(Value.Sum.Party(p)))))))).wrap)
+              ))
              .zipWithIndex
              .map { case (req, i) => req.update(_.commands.commandId := s"command-id-$i") }
            _ <- Future.sequence(
--- a/ledger/ledger-api-integration-tests/src/test/itsuite/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/CommandServiceBackPressureIT.scala
+++ b/ledger/ledger-api-integration-tests/src/test/itsuite/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/CommandServiceBackPressureIT.scala
@ -12,7 +12,10 @@ import com.digitalasset.ledger.api.testing.utils.{
  MockMessages,
  SuiteResourceManagementAroundAll
 }
-import com.digitalasset.platform.apitesting.{LedgerContext, MultiLedgerFixture}
+import com.digitalasset.ledger.api.v1.commands.CreateCommand
+import com.digitalasset.ledger.api.v1.value.{Record, RecordField, Value}
+import com.digitalasset.platform.apitesting.{LedgerContext, MultiLedgerFixture, TestTemplateIds}
+import com.digitalasset.platform.participant.util.ValueConversions._
 import com.digitalasset.platform.sandbox.config.SandboxConfig
 import com.google.protobuf.empty.Empty
 import io.grpc.Status
@ -32,19 +35,46 @@ class CommandServiceBackPressureIT
    with MultiLedgerFixture
    with SuiteResourceManagementAroundAll {

+  private[this] val testTemplateIds = new TestTemplateIds(config)
+  private[this] val templateIds = testTemplateIds.templateIds
+
  private def submitAndWaitRequest(ctx: LedgerContext, id: String = UUID.randomUUID().toString) =
    MockMessages.submitAndWaitRequest
      .update(
+        _.commands.commands := List(
+          CreateCommand(
+            Some(templateIds.dummy),
+            Some(
+              Record(
+                Some(templateIds.dummy),
+                Seq(
+                  RecordField(
+                    "operator",
+                    Option(Value(
+                      Value.Sum.Party(MockMessages.submitAndWaitRequest.commands.get.party)))))))
+          ).wrap),
        _.commands.ledgerId := ctx.ledgerId.unwrap,
        _.commands.commandId := id,
-        _.optionalTraceContext := None)
+        _.optionalTraceContext := None
+      )

  private def submitRequest(ctx: LedgerContext, id: String = UUID.randomUUID().toString) =
    MockMessages.submitRequest
      .update(
+        _.commands.commands := List(
+          CreateCommand(
+            Some(templateIds.dummy),
+            Some(
+              Record(
+                Some(templateIds.dummy),
+                Seq(RecordField(
+                  "operator",
+                  Option(Value(Value.Sum.Party(MockMessages.submitRequest.commands.get.party)))))))
+          ).wrap),
        _.commands.ledgerId := ctx.ledgerId.unwrap,
        _.commands.commandId := id,
-        _.optionalTraceContext := None)
+        _.optionalTraceContext := None
+      )

  "Commands Submission Service" when {
    "overloaded with commands" should {
--- a/ledger/ledger-api-integration-tests/src/test/itsuite/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/CommandStaticTimeIT.scala
+++ b/ledger/ledger-api-integration-tests/src/test/itsuite/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/CommandStaticTimeIT.scala
@ -26,7 +26,7 @@ import com.digitalasset.ledger.api.v1.value.Value.Sum
 import com.digitalasset.ledger.api.v1.value.Value.Sum.Party
 import com.digitalasset.ledger.api.v1.value.{Identifier, Record, RecordField, Value}
 import com.digitalasset.ledger.client.services.commands.CommandClient
-import com.digitalasset.platform.apitesting.{LedgerContext, TestTemplateIds}
+import com.digitalasset.platform.apitesting.LedgerContext
 import io.grpc.Status
 import org.scalatest.concurrent.ScalaFutures
 import org.scalatest.time.Span
@ -48,9 +48,6 @@ class CommandStaticTimeIT
    with SuiteResourceManagementAroundAll
    with OptionValues {

-  protected val testTemplateIds = new TestTemplateIds(config)
-  protected val templateIds = testTemplateIds.templateIds
-
  override def timeLimit: Span = scaled(15.seconds)

  private val tenDays: time.Duration = java.time.Duration.ofDays(10L)
--- a/ledger/ledger-api-integration-tests/src/test/itsuite/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/FailingCommandsIT.scala
+++ b/ledger/ledger-api-integration-tests/src/test/itsuite/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/FailingCommandsIT.scala
@ -45,8 +45,6 @@ class FailingCommandsIT

      "fail with the expected status on a ledger Id mismatch via sync service (multiple reqs)" in allFixtures {
        ctx =>
-          val contexts = 1 to 10
-
          val cmd1 = SubmitAndWaitRequest(
            failingRequest.commands.map(
              _.update(_.ledgerId := testNotLedgerId.unwrap, _.commandId := "sync ledgerId 1")))
--- a/ledger/ledger-api-integration-tests/src/test/lib/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/MultiLedgerCommandUtils.scala
+++ b/ledger/ledger-api-integration-tests/src/test/lib/scala/com/digitalasset/platform/tests/integration/ledger/api/commands/MultiLedgerCommandUtils.scala
@ -8,13 +8,14 @@ import com.digitalasset.ledger.api.testing.utils.MockMessages
 import com.digitalasset.ledger.api.testing.utils.MockMessages.{applicationId, workflowId}
 import com.digitalasset.ledger.api.v1.command_service.SubmitAndWaitRequest
 import com.digitalasset.ledger.api.v1.command_submission_service.SubmitRequest
-import com.digitalasset.ledger.api.v1.commands.Commands
+import com.digitalasset.ledger.api.v1.commands.{Commands, CreateCommand}
+import com.digitalasset.ledger.api.v1.value.{Record, RecordField, Value}
 import com.digitalasset.ledger.client.services.commands.SynchronousCommandClient
-import com.digitalasset.platform.apitesting.{LedgerContext, MultiLedgerFixture}
+import com.digitalasset.platform.apitesting.{LedgerContext, MultiLedgerFixture, TestTemplateIds}
 import com.digitalasset.platform.common.LedgerIdMode
+import com.digitalasset.platform.participant.util.ValueConversions._
 import com.digitalasset.platform.tests.integration.ledger.api.TransactionServiceHelpers
 import org.scalatest.AsyncTestSuite
-
 import scalaz.syntax.tag._

@SuppressWarnings(
@ -24,6 +25,9 @@ import scalaz.syntax.tag._
 trait MultiLedgerCommandUtils extends MultiLedgerFixture {
  self: AsyncTestSuite =>

+  protected val testTemplateIds = new TestTemplateIds(config)
+  protected val templateIds = testTemplateIds.templateIds
+
  protected final def newSynchronousCommandClient(ctx: LedgerContext): SynchronousCommandClient =
    new SynchronousCommandClient(ctx.commandService)

@ -32,7 +36,20 @@ trait MultiLedgerCommandUtils extends MultiLedgerFixture {
  protected val testLedgerId = domain.LedgerId("ledgerId")
  protected val testNotLedgerId = domain.LedgerId("hotdog")
  protected val submitRequest: SubmitRequest =
-    MockMessages.submitRequest.update(_.commands.ledgerId := testLedgerId.unwrap)
+    MockMessages.submitRequest.update(
+      _.commands.ledgerId := testLedgerId.unwrap,
+      _.commands.commands := List(
+        CreateCommand(
+          Some(templateIds.dummy),
+          Some(
+            Record(
+              Some(templateIds.dummy),
+              Seq(RecordField(
+                "operator",
+                Option(
+                  Value(Value.Sum.Party(MockMessages.submitAndWaitRequest.commands.get.party)))))))
+        ).wrap)
+    )

  protected val failingRequest: SubmitRequest =
    submitRequest.copy(
--- a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/tests/CommandService.scala
+++ b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/tests/CommandService.scala
@ -248,6 +248,20 @@ final class CommandService(session: LedgerSession) extends LedgerTestSuite(sessi
      assertGrpcError(failure, Status.Code.NOT_FOUND, s"Ledger ID '$invalidLedgerId' not found.")
  }

+  private[this] val disallowEmptyCommandSubmission = LedgerTest(
+    "CSDisallowEmptyTransactionsSubmission",
+    "The submission of an empty command should be rejected with INVALID_ARGUMENT"
+  ) { context =>
+    for {
+      ledger <- context.participant()
+      party <- ledger.allocateParty()
+      emptyRequest <- ledger.submitRequest(party)
+      failure <- ledger.submit(emptyRequest).failed
+    } yield {
+      assertGrpcError(failure, Status.Code.INVALID_ARGUMENT, "Missing field: commands")
+    }
+  }
+
  override val tests: Vector[LedgerTest] = Vector(
    submitAndWaitTest,
    submitAndWaitForTransactionTest,
@ -260,6 +274,7 @@ final class CommandService(session: LedgerSession) extends LedgerTestSuite(sessi
    submitAndWaitWithInvalidLedgerIdTest,
    submitAndWaitForTransactionIdWithInvalidLedgerIdTest,
    submitAndWaitForTransactionWithInvalidLedgerIdTest,
-    submitAndWaitForTransactionTreeWithInvalidLedgerIdTest
+    submitAndWaitForTransactionTreeWithInvalidLedgerIdTest,
+    disallowEmptyCommandSubmission
  )
 }
--- a/unreleased.rst
+++ b/unreleased.rst
@ -14,3 +14,4 @@ HEAD — ongoing
  commands if they are not already in flight.
 - [Sandbox] Fixed a bug a database migration script for Sandbox on Postgres introduced in SDK 0.13.32. See `issue #3284 <https://github.com/digital-asset/daml/issues/3284>`__.
 - [DAML Integration Kit] Re-add :doc:`integration kit documentation </daml-integration-kit/index>` that got accidentally deleted.
+- [Ledger API] Disallow empty commands. See `issue #592 <https://github.com/digital-asset/daml/issues/592>`__.