Add a reason text field to RejectReason.Inconsistent (#5180) (#5815)

* Add a reason text field to RejectReason.Inconsistent (#5180) CHANGELOG_BEGIN - Add a reason text field to RejectReason.Inconsistent. See `#5810 <https://github.com/digital-asset/daml/issues/5810>`__. CHANGELOG_END * Change wording in contributing instructions to reflect best practice (#5820) * Also make add reason text to other reject reasons that don't have it (#5820) * Update with review comments (#5820) * Update with review comments (#5820) * Update with review comments (#5820)
2024-09-19 16:57:40 +03:00 · 2020-05-04 12:54:17 +01:00 · 2020-05-04 12:54:17 +01:00 · a8d97c9ec0
commit a8d97c9ec0
parent 9252515f4a
7 changed files with 48 additions and 36 deletions
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@ -21,7 +21,7 @@ For Git commit messages, our principle is that `git log --pretty=oneline` should
  * *Improve explanation of …*
  * *Remove module X because it is not used.*
 * Commits should have a description that concisely explains the rationale and context for the change if that is not obvious.
-* Commit descriptions should include a `Fixes #XX` line indicating what GitHub issue number the commit fixes.
+* The first line of the description should include a reference to the fixed issue `(#XXX)`.
 * The git logs are not intended for user-facing change logs, but should be a useful reference when writing them.

 ## Pull request checklist
@ -35,9 +35,9 @@ For Git commit messages, our principle is that `git log --pretty=oneline` should

 The following is an example of a well-formed commit, including the description (first line) and a body that includes changelog additions:

-      Fixes #1311
+      Introduced a new API for package management (#1311)

-      Also fixes a typo in the Scala bindings documentation.
+      Also fixes a typo in the Scala bindings documentation

      CHANGELOG_BEGIN

--- a/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/KeyValueConsumption.scala
+++ b/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/KeyValueConsumption.scala
@ -191,13 +191,13 @@ object KeyValueConsumption {
      case DamlTransactionRejectionEntry.ReasonCase.DISPUTED =>
        wrap(RejectionReason.Disputed(rejEntry.getDisputed.getDetails))
      case DamlTransactionRejectionEntry.ReasonCase.INCONSISTENT =>
-        wrap(RejectionReason.Inconsistent)
+        wrap(RejectionReason.Inconsistent(rejEntry.getInconsistent.getDetails))
      case DamlTransactionRejectionEntry.ReasonCase.RESOURCES_EXHAUSTED =>
-        wrap(RejectionReason.ResourcesExhausted)
+        wrap(RejectionReason.ResourcesExhausted(rejEntry.getResourcesExhausted.getDetails))
      case DamlTransactionRejectionEntry.ReasonCase.DUPLICATE_COMMAND =>
        List()
      case DamlTransactionRejectionEntry.ReasonCase.PARTY_NOT_KNOWN_ON_LEDGER =>
-        wrap(RejectionReason.PartyNotKnownOnLedger)
+        wrap(RejectionReason.PartyNotKnownOnLedger(rejEntry.getPartyNotKnownOnLedger.getDetails))
      case DamlTransactionRejectionEntry.ReasonCase.SUBMITTER_CANNOT_ACT_VIA_PARTICIPANT =>
        wrap(
          RejectionReason.SubmitterCannotActViaParticipant(
--- a/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/committing/ProcessTransactionSubmission.scala
+++ b/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/committing/ProcessTransactionSubmission.scala
@ -158,7 +158,11 @@ private[kvutils] class ProcessTransactionSubmission(
      case None =>
        reject(
          recordTime,
-          buildRejectionLogEntry(transactionEntry, RejectionReason.PartyNotKnownOnLedger))
+          buildRejectionLogEntry(
+            transactionEntry,
+            RejectionReason.PartyNotKnownOnLedger(
+              s"Submitting party '${transactionEntry.submitter}' not known"))
+        )
    }

  /** Validate ledger effective time and the command's time-to-live. */
@ -315,7 +319,11 @@ private[kvutils] class ProcessTransactionSubmission(
    if (causalKeyMonotonicity)
      pass
    else
-      reject(recordTime, buildRejectionLogEntry(transactionEntry, RejectionReason.Inconsistent))
+      reject(
+        recordTime,
+        buildRejectionLogEntry(
+          transactionEntry,
+          RejectionReason.Inconsistent("Causal monotonicity violated")))
  }

  /** Check that all informee parties mentioned of a transaction are allocated. */
@ -341,7 +349,9 @@ private[kvutils] class ProcessTransactionSubmission(
      else
        reject(
          recordTime,
-          buildRejectionLogEntry(transactionEntry, RejectionReason.PartyNotKnownOnLedger)
+          buildRejectionLogEntry(
+            transactionEntry,
+            RejectionReason.PartyNotKnownOnLedger("Not all parties known"))
        )
    } yield result
  }
@ -571,14 +581,14 @@ private[kvutils] class ProcessTransactionSubmission(
      .setSubmitterInfo(transactionEntry.submitterInfo)

    reason match {
-      case RejectionReason.Inconsistent =>
-        builder.setInconsistent(Inconsistent.newBuilder.setDetails(""))
-      case RejectionReason.Disputed(disputeReason) =>
-        builder.setDisputed(Disputed.newBuilder.setDetails(disputeReason))
-      case RejectionReason.ResourcesExhausted =>
-        builder.setResourcesExhausted(ResourcesExhausted.newBuilder.setDetails(""))
-      case RejectionReason.PartyNotKnownOnLedger =>
-        builder.setPartyNotKnownOnLedger(PartyNotKnownOnLedger.newBuilder.setDetails(""))
+      case RejectionReason.Inconsistent(reason) =>
+        builder.setInconsistent(Inconsistent.newBuilder.setDetails(reason))
+      case RejectionReason.Disputed(reason) =>
+        builder.setDisputed(Disputed.newBuilder.setDetails(reason))
+      case RejectionReason.ResourcesExhausted(reason) =>
+        builder.setResourcesExhausted(ResourcesExhausted.newBuilder.setDetails(reason))
+      case RejectionReason.PartyNotKnownOnLedger(reason) =>
+        builder.setPartyNotKnownOnLedger(PartyNotKnownOnLedger.newBuilder.setDetails(reason))
      case RejectionReason.SubmitterCannotActViaParticipant(details) =>
        builder.setSubmitterCannotActViaParticipant(
          SubmitterCannotActViaParticipant.newBuilder
--- a/ledger/participant-state/kvutils/src/test/lib/scala/com/daml/ledger/participant/state/kvutils/ParticipantStateIntegrationSpecBase.scala
+++ b/ledger/participant-state/kvutils/src/test/lib/scala/com/daml/ledger/participant/state/kvutils/ParticipantStateIntegrationSpecBase.scala
@ -467,7 +467,7 @@ abstract class ParticipantStateIntegrationSpecBase(implementationName: String)(
          offset2 should be(toOffset(2))
          inside(update2) {
            case CommandRejected(_, _, reason) =>
-              reason should be(RejectionReason.PartyNotKnownOnLedger)
+              reason should be(a[RejectionReason.PartyNotKnownOnLedger])
          }

          offset3 should be(toOffset(3))
--- a/ledger/participant-state/src/main/scala/com/daml/ledger/participant/state/v1/RejectionReason.scala
+++ b/ledger/participant-state/src/main/scala/com/daml/ledger/participant/state/v1/RejectionReason.scala
@ -19,8 +19,8 @@ object RejectionReason {
    * See https://docs.daml.com/concepts/ledger-model/ledger-integrity.html
    * for the definition of ledger consistency.
    */
-  final case object Inconsistent extends RejectionReason {
-    override def description: String = "Inconsistent"
+  final case class Inconsistent(reason: String) extends RejectionReason {
+    override def description: String = s"Inconsistent: $reason"
  }

  /** The transaction has been disputed.
@ -30,14 +30,14 @@ object RejectionReason {
    * in the submission or validation logic, or due to malicious behaviour.
    */
  final case class Disputed(reason: String) extends RejectionReason {
-    override def description: String = "Disputed: " + reason
+    override def description: String = s"Disputed: $reason"
  }

  /** The Participant node did not have sufficient resources with the
    * ledger to submit the transaction.
    */
-  final case object ResourcesExhausted extends RejectionReason {
-    override def description: String = "Resources exhausted"
+  final case class ResourcesExhausted(reason: String) extends RejectionReason {
+    override def description: String = s"Resources exhausted: $reason"
  }

  /** A party mentioned as a stakeholder or actor has not been on-boarded on
@ -49,19 +49,19 @@ object RejectionReason {
    * identity-manager.
    *
    */
-  final case object PartyNotKnownOnLedger extends RejectionReason {
-    override def description: String = "Party not known on ledger"
+  final case class PartyNotKnownOnLedger(reason: String) extends RejectionReason {
+    override def description: String = s"Party not known on ledger: $reason"
  }

  /** The submitter cannot act via this participant.
    *
-    * @param details: details on why the submitter cannot act; e.g., because
+    * @param reason: details on why the submitter cannot act; e.g., because
    *   it is not hosted on the participant or because its write access to
    *   the ledger has been deactivated.
    *
    */
-  final case class SubmitterCannotActViaParticipant(details: String) extends RejectionReason {
-    override def description: String = "Submitter cannot act via participant: " + details
+  final case class SubmitterCannotActViaParticipant(reason: String) extends RejectionReason {
+    override def description: String = s"Submitter cannot act via participant: $reason"
  }

  /** The ledger time of the transaction submission violated one of the
@ -73,6 +73,6 @@ object RejectionReason {
    *    to the ledger time of any contract used by the transaction.
    */
  final case class InvalidLedgerTime(reason: String) extends RejectionReason {
-    override def description: String = "Invalid ledger time: " + reason
+    override def description: String = s"Invalid ledger time: $reason"
  }
 }
--- a/ledger/sandbox/src/main/scala/com/digitalasset/platform/indexer/JdbcIndexer.scala
+++ b/ledger/sandbox/src/main/scala/com/digitalasset/platform/indexer/JdbcIndexer.scala
@ -323,11 +323,13 @@ class JdbcIndexer private[indexer] (
  private def toDomainRejection(
      submitterInfo: SubmitterInfo,
      state: RejectionReason): domain.RejectionReason = state match {
-    case RejectionReason.Inconsistent =>
-      domain.RejectionReason.Inconsistent(RejectionReason.Inconsistent.description)
-    case RejectionReason.Disputed(_) => domain.RejectionReason.Disputed(state.description)
-    case RejectionReason.ResourcesExhausted => domain.RejectionReason.OutOfQuota(state.description)
-    case RejectionReason.PartyNotKnownOnLedger =>
+    case RejectionReason.Inconsistent(_) =>
+      domain.RejectionReason.Inconsistent(state.description)
+    case RejectionReason.Disputed(_) =>
+      domain.RejectionReason.Disputed(state.description)
+    case RejectionReason.ResourcesExhausted(_) =>
+      domain.RejectionReason.OutOfQuota(state.description)
+    case RejectionReason.PartyNotKnownOnLedger(_) =>
      domain.RejectionReason.PartyNotKnownOnLedger(state.description)
    case RejectionReason.SubmitterCannotActViaParticipant(_) =>
      domain.RejectionReason.SubmitterCannotActViaParticipant(state.description)
--- a/ledger/sandbox/src/main/scala/com/digitalasset/platform/store/dao/events/PostCommitValidation.scala
+++ b/ledger/sandbox/src/main/scala/com/digitalasset/platform/store/dao/events/PostCommitValidation.scala
@ -227,7 +227,7 @@ object PostCommitValidation {
    )

  private[events] val UnknownContract: RejectionReason =
-    RejectionReason.Inconsistent
+    RejectionReason.Inconsistent("Unknown contract")

  private[events] def CausalMonotonicityViolation(
      contractLedgerEffectiveTime: Instant,