[ED] Clean-up [DPP-1210] (#15127)

* [ED Clean-up] Extract validateTimestamp to FieldValidations and improve error messages changelog_begin changelog_end * Permanently enable ExplicitDisclosureIT:EDMetadata * Cleanup ExplicitDisclosureIT
2024-11-10 10:46:11 +03:00 · 2022-09-30 14:34:59 +02:00 · 2022-09-30 14:34:59 +02:00 · 0f59026a67
commit 0f59026a67
parent 6364deb68e
4 changed files with 62 additions and 69 deletions
--- a/ledger/ledger-api-common/src/main/scala/com/digitalasset/ledger/api/validation/ValidateDisclosedContracts.scala
+++ b/ledger/ledger-api-common/src/main/scala/com/digitalasset/ledger/api/validation/ValidateDisclosedContracts.scala
@ -3,33 +3,24 @@

 package com.daml.ledger.api.validation

-import com.daml.api.util.TimestampConversion
 import com.daml.error.ContextualizedErrorLogger
 import com.daml.error.definitions.LedgerApiErrors
-import com.daml.lf.command.{ContractMetadata, DisclosedContract}
-import com.daml.lf.data.ImmArray
 import com.daml.ledger.api.v1.commands.{
  Commands => ProtoCommands,
  DisclosedContract => ProtoDisclosedContract,
 }
-import com.daml.ledger.api.validation.ValidationErrors.invalidArgument
 import com.daml.ledger.api.validation.ValueValidator.{
  validateOptionalIdentifier,
  validateRecordFields,
 }
+import com.daml.lf.command.{ContractMetadata, DisclosedContract}
+import com.daml.lf.data.ImmArray
 import com.daml.lf.value.Value.ValueRecord
-import com.daml.platform.server.api.validation.FieldValidations.{
-  requireContractId,
-  requirePresence,
-  validateHash,
-  validateIdentifier,
-}
-import com.google.protobuf.timestamp.Timestamp
-import io.grpc.StatusRuntimeException
-import com.daml.lf.data.Time
 import com.daml.lf.value.ValueOuterClass.VersionedValue
 import com.daml.lf.value.{Value, ValueCoder}
+import com.daml.platform.server.api.validation.FieldValidations._
 import com.google.protobuf.any.Any.toJavaProto
+import io.grpc.StatusRuntimeException

 import scala.collection.mutable
 import scala.util.Try
@ -73,8 +64,8 @@ class ValidateDisclosedContracts(explicitDisclosureFeatureEnabled: Boolean) {
      .map(_.result())
  }

-  def validateDisclosedContractArguments(arguments: ProtoDisclosedContract.Arguments)(implicit
-      contextualizedErrorLogger: ContextualizedErrorLogger
+  private def validateDisclosedContractArguments(arguments: ProtoDisclosedContract.Arguments)(
+      implicit contextualizedErrorLogger: ContextualizedErrorLogger
  ): Either[StatusRuntimeException, Value] =
    arguments match {
      case ProtoDisclosedContract.Arguments.CreateArguments(value) =>
@ -88,7 +79,7 @@ class ValidateDisclosedContracts(explicitDisclosureFeatureEnabled: Boolean) {
          versionedValue <- validateVersionedValue(protoAny)
        } yield versionedValue.unversioned
      case ProtoDisclosedContract.Arguments.Empty =>
-        Left(ValidationErrors.missingField("arguments"))
+        Left(ValidationErrors.missingField("DisclosedContract.arguments"))
    }

  private def validateProtoAny(value: com.google.protobuf.any.Any)(implicit
@ -115,14 +106,26 @@ class ValidateDisclosedContracts(explicitDisclosureFeatureEnabled: Boolean) {
      contextualizedErrorLogger: ContextualizedErrorLogger
  ): Either[StatusRuntimeException, DisclosedContract] =
    for {
-      templateId <- requirePresence(disclosedContract.templateId, "template_id")
+      templateId <- requirePresence(disclosedContract.templateId, "DisclosedContract.template_id")
      validatedTemplateId <- validateIdentifier(templateId)
-      contractId <- requireContractId(disclosedContract.contractId, "contract_id")
+      contractId <- requireContractId(
+        disclosedContract.contractId,
+        "DisclosedContract.contract_id",
+      )
      argument <- validateDisclosedContractArguments(disclosedContract.arguments)
-      metadata <- requirePresence(disclosedContract.metadata, "metadata")
-      createdAt <- requirePresence(metadata.createdAt, "created_at")
-      validatedCreatedAt <- validateCreatedAt(createdAt)
-      keyHash <- validateHash(metadata.contractKeyHash, "contract_key_hash")
+      metadata <- requirePresence(disclosedContract.metadata, "DisclosedContract.metadata")
+      createdAt <- requirePresence(
+        metadata.createdAt,
+        "DisclosedContract.metadata.created_at",
+      )
+      validatedCreatedAt <- validateTimestamp(
+        createdAt,
+        "DisclosedContract.metadata.created_at",
+      )
+      keyHash <- validateHash(
+        metadata.contractKeyHash,
+        "DisclosedContract.metadata.contract_key_hash",
+      )
    } yield DisclosedContract(
      contractId = contractId,
      templateId = validatedTemplateId,
@ -133,21 +136,4 @@ class ValidateDisclosedContracts(explicitDisclosureFeatureEnabled: Boolean) {
        driverMetadata = ImmArray.from(metadata.driverMetadata.toByteArray),
      ),
    )
-
-  // TODO ED: Extract in utility library
-  private def validateCreatedAt(createdAt: Timestamp)(implicit
-      contextualizedErrorLogger: ContextualizedErrorLogger
-  ): Either[StatusRuntimeException, Time.Timestamp] =
-    Try(
-      TimestampConversion
-        .toLf(
-          protoTimestamp = createdAt,
-          mode = TimestampConversion.ConversionMode.Exact,
-        )
-    ).toEither.left
-      .map(errMsg =>
-        invalidArgument(
-          s"Can not represent disclosed contract createdAt ($createdAt) as a Daml timestamp: ${errMsg.getMessage}"
-        )
-      )
 }
--- a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/FieldValidations.scala
+++ b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/FieldValidations.scala
@ -3,6 +3,7 @@

 package com.daml.platform.server.api.validation

+import com.daml.api.util.TimestampConversion
 import com.daml.error.ContextualizedErrorLogger
 import com.daml.error.definitions.LedgerApiErrors
 import com.daml.ledger.api.domain
@ -16,10 +17,11 @@ import com.daml.ledger.participant.state.index.ResourceAnnotationValidation.{
  InvalidAnnotationsKeyError,
 }
 import com.daml.lf.crypto.Hash
-import com.daml.lf.data.{Bytes, Ref}
+import com.daml.lf.data.{Bytes, Ref, Time}
 import com.daml.lf.data.Ref.Party
 import com.daml.lf.value.Value.ContractId
 import com.google.protobuf.ByteString
+import com.google.protobuf.timestamp.Timestamp
 import io.grpc.StatusRuntimeException

 import scala.util.{Failure, Success, Try}
@ -241,4 +243,20 @@ object FieldValidations {
      case Right(_) => Right(annotations)
    }
  }
+
+  def validateTimestamp(timestamp: Timestamp, fieldName: String)(implicit
+      errorLogger: ContextualizedErrorLogger
+  ): Either[StatusRuntimeException, Time.Timestamp] =
+    Try(
+      TimestampConversion
+        .toLf(
+          protoTimestamp = timestamp,
+          mode = TimestampConversion.ConversionMode.Exact,
+        )
+    ).toEither.left
+      .map(errMsg =>
+        invalidArgument(
+          s"Can not represent $fieldName ($timestamp) as a Daml timestamp: ${errMsg.getMessage}"
+        )
+      )
 }
--- a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/ledger/api/validation/ValidateDisclosedContractsTest.scala
+++ b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/ledger/api/validation/ValidateDisclosedContractsTest.scala
@ -68,7 +68,7 @@ class ValidateDisclosedContractsTest extends AnyFlatSpec with Matchers with Vali
      request = validateDisclosedContracts(withMissingTemplateId),
      code = Status.Code.INVALID_ARGUMENT,
      description =
-        "MISSING_FIELD(8,0): The submitted command is missing a mandatory field: template_id",
+        "MISSING_FIELD(8,0): The submitted command is missing a mandatory field: DisclosedContract.template_id",
      metadata = Map.empty,
    )
  }
@ -100,7 +100,7 @@ class ValidateDisclosedContractsTest extends AnyFlatSpec with Matchers with Vali
      request = validateDisclosedContracts(withMissingContractId),
      code = Status.Code.INVALID_ARGUMENT,
      description =
-        "MISSING_FIELD(8,0): The submitted command is missing a mandatory field: contract_id",
+        "MISSING_FIELD(8,0): The submitted command is missing a mandatory field: DisclosedContract.contract_id",
      metadata = Map.empty,
    )
  }
@ -115,7 +115,7 @@ class ValidateDisclosedContractsTest extends AnyFlatSpec with Matchers with Vali
      request = validateDisclosedContracts(withInvalidContractId),
      code = Status.Code.INVALID_ARGUMENT,
      description =
-        "INVALID_FIELD(8,0): The submitted command has a field with invalid value: Invalid field contract_id: cannot parse ContractId \"badContractId\"",
+        "INVALID_FIELD(8,0): The submitted command has a field with invalid value: Invalid field DisclosedContract.contract_id: cannot parse ContractId \"badContractId\"",
      metadata = Map.empty,
    )
  }
@ -128,13 +128,13 @@ class ValidateDisclosedContractsTest extends AnyFlatSpec with Matchers with Vali
      request = validateDisclosedContracts(withMissingCreateArguments),
      code = Status.Code.INVALID_ARGUMENT,
      description =
-        "MISSING_FIELD(8,0): The submitted command is missing a mandatory field: arguments",
+        "MISSING_FIELD(8,0): The submitted command is missing a mandatory field: DisclosedContract.arguments",
      metadata = Map.empty,
    )
  }

  it should "fail validation on invalid create arguments record field" in {
-    def invalidateArguments(arguments: ProtoArguments): ProtoArguments =
+    def invalidArguments(arguments: ProtoArguments): ProtoArguments =
      ProtoArguments.CreateArguments(
        arguments.createArguments.get.update(
          _.fields.set(scala.Seq(ProtoRecordField("something", None)))
@ -144,7 +144,7 @@ class ValidateDisclosedContractsTest extends AnyFlatSpec with Matchers with Vali
      ProtoCommands(disclosedContracts =
        scala.Seq(
          api.protoDisclosedContract.update(
-            _.arguments.modify(invalidateArguments)
+            _.arguments.modify(invalidArguments)
          )
        )
      )
@ -165,7 +165,7 @@ class ValidateDisclosedContractsTest extends AnyFlatSpec with Matchers with Vali
      request = validateDisclosedContracts(withMissingMetadata),
      code = Status.Code.INVALID_ARGUMENT,
      description =
-        "MISSING_FIELD(8,0): The submitted command is missing a mandatory field: metadata",
+        "MISSING_FIELD(8,0): The submitted command is missing a mandatory field: DisclosedContract.metadata",
      metadata = Map.empty,
    )
  }
@ -180,7 +180,7 @@ class ValidateDisclosedContractsTest extends AnyFlatSpec with Matchers with Vali
      request = validateDisclosedContracts(withMissingCreatedAt),
      code = Status.Code.INVALID_ARGUMENT,
      description =
-        "MISSING_FIELD(8,0): The submitted command is missing a mandatory field: created_at",
+        "MISSING_FIELD(8,0): The submitted command is missing a mandatory field: DisclosedContract.metadata.created_at",
      metadata = Map.empty,
    )
  }
@ -195,7 +195,7 @@ class ValidateDisclosedContractsTest extends AnyFlatSpec with Matchers with Vali
      request = validateDisclosedContracts(withInvalidCreatedAt),
      code = Status.Code.INVALID_ARGUMENT,
      description =
-        "INVALID_ARGUMENT(8,0): The submitted command has invalid arguments: Can not represent disclosed contract createdAt (Timestamp(1337,133,UnknownFieldSet(Map()))) as a Daml timestamp: Conversion of 1970-01-01T00:22:17.000000133Z to microsecond granularity would result in loss of precision.",
+        "INVALID_ARGUMENT(8,0): The submitted command has invalid arguments: Can not represent DisclosedContract.metadata.created_at (Timestamp(1337,133,UnknownFieldSet(Map()))) as a Daml timestamp: Conversion of 1970-01-01T00:22:17.000000133Z to microsecond granularity would result in loss of precision.",
      metadata = Map.empty,
    )
  }
@ -214,7 +214,7 @@ class ValidateDisclosedContractsTest extends AnyFlatSpec with Matchers with Vali
      request = validateDisclosedContracts(withInvalidKeyHashInMetadata),
      code = Status.Code.INVALID_ARGUMENT,
      description =
-        "INVALID_FIELD(8,0): The submitted command has a field with invalid value: Invalid field contract_key_hash: hash should have 32 bytes, got 10",
+        "INVALID_FIELD(8,0): The submitted command has a field with invalid value: Invalid field DisclosedContract.metadata.contract_key_hash: hash should have 32 bytes, got 10",
      metadata = Map.empty,
    )
  }
@ -225,7 +225,7 @@ class ValidateDisclosedContractsTest extends AnyFlatSpec with Matchers with Vali
        scala.Seq(
          api.protoDisclosedContract.update(
            _.arguments := ProtoArguments.CreateArgumentsBlob(
-              com.google.protobuf.any.Any("crap", ByteString.EMPTY)
+              com.google.protobuf.any.Any("foo ", ByteString.EMPTY)
            )
          )
        )
--- a/ledger/ledger-api-tests/suites/src/main/scala/com/daml/ledger/api/testtool/suites/v1_dev/ExplicitDisclosureIT.scala
+++ b/ledger/ledger-api-tests/suites/src/main/scala/com/daml/ledger/api/testtool/suites/v1_dev/ExplicitDisclosureIT.scala
@ -39,6 +39,7 @@ import com.daml.ledger.api.v1.transaction_filter.{
 import com.daml.ledger.test.modelext.TestExtension.IDelegated

 import java.time.temporal.ChronoUnit
+import java.util.regex.Pattern
 import scala.concurrent.{ExecutionContext, Future}
 import scala.util.{Success, Try}

@ -179,14 +180,10 @@ final class ExplicitDisclosureIT extends LedgerTestSuite {
    }
  }

-  // TODO ED: When the conformance tests are enabled, check this test for flakiness
  test(
    "EDMetadata",
    "All create events have correctly-defined metadata",
    allocate(Parties(2)),
-    // TODO ED: Remove this conditional toggle once Canton consumes this commit so
-    //          it can pass in the `ledger-api-test-tool-on-canton`
-    enabled = _.explicitDisclosure,
  )(implicit ec => { case Participants(Participant(ledger, owner, delegate)) =>
    val contractKey = ledger.nextKeyId()
    for {
@ -448,23 +445,15 @@ final class ExplicitDisclosureIT extends LedgerTestSuite {
          testContext.disclosedContract.update(_.metadata.modify(_.clearCreatedAt))
        )
        .mustFail("using a disclosed contract with missing createdAt in contract metadata")
-
-      //      // TODO ED: Assert missing contract key hash when ledger side metadata validation is implemented
-      //      // Exercise a choice using an invalid disclosed contract (missing key hash in contract metadata for a contract that has a contract key associated)
-      //      errorMissingKeyHash <- testContext
-      //        .exerciseFetchDelegated(
-      //          testContext.disclosedContract.update(_.metadata.contractKeyHash.set(ByteString.EMPTY))
-      //        )
-      //        .mustFail(
-      //          "using a disclosed contract with missing key hash in contract metadata for a contract that has a contract key associated"
-      //        )
    } yield {
-      assertGrpcError(
+      assertGrpcErrorRegex(
        errorMalformedPayload,
-        // TODO ED: Verify that this error code is good enough for the user
-        //          and that it includes the contract id
        LedgerApiErrors.CommandExecution.Preprocessing.PreprocessingFailed,
-        None,
+        Some(
+          Pattern.compile(
+            "Expecting 2 field for record .*\\:Test\\:Delegated, but got 1"
+          )
+        ),
        checkDefiniteAnswerMetadata = true,
      )
      assertGrpcError(