Skip to content

Commit

Permalink
Disallow empty command submission (#3270)
Browse files Browse the repository at this point in the history 
* Disallow empty command submission

Fixes #592
  • Loading branch information
@stefanobaghino-da @gerolf-da
stefanobaghino-da authored and gerolf-da committed Oct 31, 2019
1 parent 819f8a8 commit d9ae487
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 40 deletions.
3 changes: 2 additions & 1 deletion ...-api-common/src/main/scala/com/digitalasset/ledger/api/validation/CommandsValidator.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 33 additions & 6 deletions .../test/suite/scala/com/digitalasset/ledger/api/validation/SubmitRequestValidatorTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -54,7 +62,7 @@ class SubmitRequestValidatorTest
submitter,
Some(let),
Some(mrt),
Seq.empty
Seq(command)
)
}

Expand All @@ -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
)
Expand All @@ -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 {
Expand Down
42 changes: 25 additions & 17 deletions ...ala/com/digitalasset/platform/tests/integration/ledger/api/commands/CommandClientIT.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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))
Expand All @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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
}
Expand Down
30 changes: 28 additions & 2 deletions ...italasset/platform/tests/integration/ledger/api/commands/CommandCompletionServiceIT.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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(
Expand Down
36 changes: 33 additions & 3 deletions ...alasset/platform/tests/integration/ledger/api/commands/CommandServiceBackPressureIT.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
5 changes: 1 addition & 4 deletions ...com/digitalasset/platform/tests/integration/ledger/api/commands/CommandStaticTimeIT.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions ...a/com/digitalasset/platform/tests/integration/ledger/api/commands/FailingCommandsIT.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")))
Expand Down
25 changes: 21 additions & 4 deletions ...digitalasset/platform/tests/integration/ledger/api/commands/MultiLedgerCommandUtils.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)

Expand All @@ -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(
Expand Down
Loading

0 comments on commit d9ae487

Please sign in to comment.