[Disclosures] Validate hash of disclosed contracts (#16470)

compiler/scenario-service/client/src/DA/Daml/LF/PrettyScenario.hs

@@ -298,12 +298,13 @@ prettyScenarioErrorError (Just err) =  do
                   (prettyContractRef world)
                   scenarioError_ContractNotActiveContractRef
         ]
-    ScenarioErrorErrorDisclosedContractKeyHashingError(ScenarioError_DisclosedContractKeyHashingError contractId templateId reason) ->
+    ScenarioErrorErrorDisclosedContractKeyHashingError(ScenarioError_DisclosedContractKeyHashingError contractId key computedHash declaredHash) ->
       pure $ vcat
-        [ "Failed to cache disclosed contract key"
+        [ "Mismatched disclosed contract key hash for contract"
         , label_ "Contract:" $ prettyMay "<missing contract>" (prettyContractRef world) contractId
-        , label_ "Template:" $ prettyMay "<missing template id>" (prettyDefName world) templateId
-        , label_ "Reason:" $ ltext reason
+        , label_ "key:" $ prettyMay "<missing key>" (prettyValue' False 0 world) key
+        , label_ "computed hash:" $ ltext computedHash
+        , label_ "declared hash:" $ ltext declaredHash    
         ]
     ScenarioErrorErrorCreateEmptyContractKeyMaintainers ScenarioError_CreateEmptyContractKeyMaintainers{..} ->
       pure $ vcat

compiler/scenario-service/protos/scenario_service.proto

@@ -187,8 +187,9 @@ message ScenarioError {
 
   message DisclosedContractKeyHashingError {
       ContractRef contract_ref = 1;
-      Identifier template_id = 2;
-      string reason = 3;
+      Value key = 2;
+      string computed_hash = 3;
+      string declared_hash = 4;
   }
 
   message ContractNotVisible {

compiler/scenario-service/server/src/main/scala/com/digitalasset/daml/lf/scenario/Conversions.scala

@@ -143,12 +143,13 @@ final class Conversions(
                     .setConsumedBy(proto.NodeId.newBuilder.setId(consumedBy.toString).build)
                     .build
                 )
-              case DisclosedContractKeyHashingError(contractId, templateId, reason) =>
+              case DisclosedContractKeyHashingError(contractId, globalKey, hash) =>
                 builder.setDisclosedContractKeyHashingError(
                   proto.ScenarioError.DisclosedContractKeyHashingError.newBuilder
-                    .setContractRef(mkContractRef(contractId, templateId))
-                    .setTemplateId(convertIdentifier(templateId))
-                    .setReason(reason)
+                    .setContractRef(mkContractRef(contractId, globalKey.templateId))
+                    .setKey(convertValue(globalKey.key))
+                    .setComputedHash(globalKey.hash.toHexString)
+                    .setDeclaredHash(hash.toHexString)
                     .build
                 )
               case ContractKeyNotVisible(coid, gk, actAs, readAs, stakeholders) =>

daml-lf/data/src/main/scala/com/digitalasset/daml/lf/InternalError.scala

@@ -16,14 +16,21 @@ private[lf] object InternalError {
   @throws[IllegalArgumentException]
   def illegalArgumentException(location: String, message: String): Nothing = {
     log(location, message)
-    throw new IllegalArgumentException(message)
+    throw new IllegalArgumentException(location + ": " + message)
+  }
+
+  @throws[IllegalStateException]
+  def assertionException(location: String, message: String): Nothing = {
+    log(location, message)
+    throw new AssertionError(location + ": " + message)
   }
 
   @throws[RuntimeException]
   def runtimeException(location: String, message: String): Nothing = {
     log(location, message)
-    throw new RuntimeException(message)
+    throw new RuntimeException(location + ": " + message)
   }
+
 }
 
 trait InternalError {

daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Engine.scala

@@ -370,7 +370,7 @@ class Engine(val config: EngineConfig = Engine.StableConfig) {
         case Right(
               UpdateMachine.Result(tx, _, nodeSeeds, globalKeyMapping, disclosedCreateEvents)
             ) =>
-          val disclosureMap = disclosures.iterator.map(c => c.contractId.value -> c).toMap
+          val disclosureMap = disclosures.iterator.map(c => c.contractId -> c).toMap
           val processedDisclosedContracts = disclosedCreateEvents.map { create =>
             val diclosedContract = disclosureMap(create.coid)
             ProcessedDisclosedContract(

daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/CommandPreprocessor.scala

@@ -43,10 +43,10 @@ private[lf] final class CommandPreprocessor(
       case _ =>
     }
     val arg = valueTranslator.unsafeTranslateValue(Ast.TTyCon(disc.templateId), disc.argument)
-    val coid = valueTranslator.unsafeTranslateCid(disc.contractId)
+    valueTranslator.validateCid(disc.contractId)
     speedy.DisclosedContract(
       templateId = disc.templateId,
-      contractId = coid,
+      contractId = disc.contractId,
       argument = arg,
       metadata = disc.metadata,
     )

daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/ValueTranslator.scala

@@ -41,21 +41,18 @@ private[lf] final class ValueTranslator(
     go(fields.toFrontStack, Map.empty)
   }
 
-  private[this] val unsafeTranslateV1Cid: ContractId.V1 => SValue.SContractId =
-    if (requireV1ContractIdSuffix)
-      cid =>
-        if (cid.suffix.isEmpty)
-          throw Error.Preprocessing.IllegalContractId.NonSuffixV1ContractId(cid)
-        else
-          SValue.SContractId(cid)
-    else
-      SValue.SContractId(_)
+  val validateCid: ContractId => Unit =
+    if (requireV1ContractIdSuffix) { case cid: ContractId.V1 =>
+      if (cid.suffix.isEmpty)
+        throw Error.Preprocessing.IllegalContractId.NonSuffixV1ContractId(cid)
+    }
+    else { _ => () }
 
   @throws[Error.Preprocessing.Error]
-  private[preprocessing] def unsafeTranslateCid(cid: ContractId): SValue.SContractId =
-    cid match {
-      case cid1: ContractId.V1 => unsafeTranslateV1Cid(cid1)
-    }
+  private[preprocessing] def unsafeTranslateCid(cid: ContractId): SValue.SContractId = {
+    validateCid(cid)
+    SValue.SContractId(cid)
+  }
 
   // For efficient reason we do not produce here the monad Result[SValue] but rather throw
   // exception in case of error or package missing.

daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala

@@ -746,7 +746,7 @@ class EngineTest
       )
       val usedDisclosedContract = DisclosedContract(
         templateId,
-        SValue.SContractId(toContractId("BasicTests:WithKey:1")),
+        toContractId("BasicTests:WithKey:1"),
         SValue.SRecord(
           templateId,
           ImmArray(Ref.Name.assertFromString("p"), Ref.Name.assertFromString("k")),
@@ -760,7 +760,7 @@ class EngineTest
       )
       val unusedDisclosedContract = DisclosedContract(
         templateId,
-        SValue.SContractId(toContractId("BasicTests:WithKey:2")),
+        toContractId("BasicTests:WithKey:2"),
         SValue.SRecord(
           templateId,
           ImmArray(Ref.Name.assertFromString("p"), Ref.Name.assertFromString("k")),
@@ -780,7 +780,7 @@ class EngineTest
       val transactionVersion = TxVersions.assignNodeVersion(basicTestsPkg.languageVersion)
       val expectedProcessedDisclosedContract = ProcessedDisclosedContract(
         templateId = usedDisclosedContract.templateId,
-        usedDisclosedContract.contractId.value,
+        usedDisclosedContract.contractId,
         usedDisclosedContract.argument.toNormalizedValue(transactionVersion),
         createdAt = usedDisclosedContract.metadata.createdAt,
         driverMetadata = usedDisclosedContract.metadata.driverMetadata,
@@ -1635,7 +1635,7 @@ class EngineTest
       )
       val usedDisclosedContract = DisclosedContract(
         templateId,
-        SValue.SContractId(toContractId("BasicTests:WithKey:1")),
+        toContractId("BasicTests:WithKey:1"),
         SValue.SRecord(
           templateId,
           ImmArray(Ref.Name.assertFromString("p"), Ref.Name.assertFromString("k")),
@@ -1649,7 +1649,7 @@ class EngineTest
       )
       val unusedDisclosedContract = DisclosedContract(
         templateId,
-        SValue.SContractId(toContractId("BasicTests:WithKey:2")),
+        toContractId("BasicTests:WithKey:2"),
         SValue.SRecord(
           templateId,
           ImmArray(Ref.Name.assertFromString("p"), Ref.Name.assertFromString("k")),
@@ -1669,7 +1669,7 @@ class EngineTest
       val transactionVersion = TxVersions.assignNodeVersion(basicTestsPkg.languageVersion)
       val expectedProcessedDisclosedContract = ProcessedDisclosedContract(
         templateId = usedDisclosedContract.templateId,
-        contractId = usedDisclosedContract.contractId.value,
+        contractId = usedDisclosedContract.contractId,
         argument = usedDisclosedContract.argument.toNormalizedValue(transactionVersion),
         createdAt = usedDisclosedContract.metadata.createdAt,
         driverMetadata = usedDisclosedContract.metadata.driverMetadata,
@@ -1694,7 +1694,7 @@ class EngineTest
     val templateId = Identifier(basicTestsPkgId, "BasicTests:Simple")
     val usedDisclosedContract = DisclosedContract(
       templateId,
-      SValue.SContractId(toContractId("BasicTests:Simple:1")),
+      toContractId("BasicTests:Simple:1"),
       SValue.SRecord(
         templateId,
         ImmArray(Ref.Name.assertFromString("p")),
@@ -1704,7 +1704,7 @@ class EngineTest
     )
     val unusedDisclosedContract = DisclosedContract(
       templateId,
-      SValue.SContractId(toContractId("BasicTests:Simple:2")),
+      toContractId("BasicTests:Simple:2"),
       SValue.SRecord(
         templateId,
         ImmArray(Ref.Name.assertFromString("p")),
@@ -1716,13 +1716,13 @@ class EngineTest
     "unused disclosed contracts not saved to ledger" in {
       val fetchTemplateCommand = speedy.Command.FetchTemplate(
         templateId = templateId,
-        coid = usedDisclosedContract.contractId,
+        coid = SContractId(usedDisclosedContract.contractId),
       )
 
       val transactionVersion = TxVersions.assignNodeVersion(basicTestsPkg.languageVersion)
       val expectedProcessedDisclosedContract = ProcessedDisclosedContract(
         templateId = usedDisclosedContract.templateId,
-        contractId = usedDisclosedContract.contractId.value,
+        contractId = usedDisclosedContract.contractId,
         argument = usedDisclosedContract.argument.toNormalizedValue(transactionVersion),
         createdAt = usedDisclosedContract.metadata.createdAt,
         driverMetadata = usedDisclosedContract.metadata.driverMetadata,
@@ -2832,7 +2832,7 @@ object EngineTest {
         disclosedContracts: DisclosedContract*
     ): Matcher[VersionedTransaction] =
       Matcher { transaction =>
-        val expectedResult = disclosedContracts.map(_.contractId.value).toSet
+        val expectedResult = disclosedContracts.map(_.contractId).toSet
         val actualResult = transaction.inputContracts
         val debugMessage = Seq(
           s"expected but missing contract IDs: ${expectedResult.filter(!actualResult.contains(_))}",

daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Command.scala

@@ -3,9 +3,10 @@
 
 package com.daml.lf.speedy
 
-import com.daml.lf.data.Ref.{ChoiceName, Identifier}
+import com.daml.lf.data.Ref.{Identifier, ChoiceName}
 import com.daml.lf.speedy.SValue._
 import com.daml.lf.command.ContractMetadata
+import com.daml.lf.value.Value.ContractId
 
 // ---------------------
 // Preprocessed commands
@@ -78,7 +79,7 @@ private[lf] object Command {
 
 final case class DisclosedContract(
     templateId: Identifier,
-    contractId: SContractId,
+    contractId: ContractId,
     argument: SValue,
     metadata: ContractMetadata,
 )

daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Compiler.scala

@@ -1056,27 +1056,31 @@ private[lf] final class Compiler(
     var env = env0
 
     s.SELet(
-      disclosures.toList.flatMap { case DisclosedContract(templateId, contractId, argument, _) =>
-        // Let bounded variables occur after the contract disclosure bound variable - hence baseIndex+1
-        // For each disclosed contract, we add 2 members to our let bounded list - hence 2*offset
+      disclosures.toList.flatMap {
+        case DisclosedContract(templateId, contractId, argument, metadata) =>
+          // Let bounded variables occur after the contract disclosure bound variable - hence baseIndex+1
+          // For each disclosed contract, we add 2 members to our let bounded list - hence 2*offset
 
-        val expr1 = checkPreCondition(
-          env,
-          templateId,
-          s.SEValue(argument),
-        )((_) =>
-          s.SEApp(
-            s.SEVal(t.ToCachedContractDefRef(templateId)),
-            List(s.SEValue(argument), s.SEValue.None),
+          val expr1 = checkPreCondition(
+            env,
+            templateId,
+            s.SEValue(argument),
+          )((_) =>
+            s.SEApp(
+              s.SEVal(t.ToCachedContractDefRef(templateId)),
+              List(s.SEValue(argument), s.SEValue.None),
+            )
           )
-        )
-        val contractPos = env.nextPosition
-        env = env.pushVar
-        val expr2 =
-          app(s.SEBuiltin(SBCacheDisclosedContract(contractId.value)), env.toSEVar(contractPos))
-        env = env.pushVar
+          val contractPos = env.nextPosition
+          env = env.pushVar
+          val expr2 =
+            app(
+              s.SEBuiltin(SBCacheDisclosedContract(contractId, metadata.keyHash)),
+              env.toSEVar(contractPos),
+            )
+          env = env.pushVar
 
-        List(expr1, expr2)
+          List(expr1, expr2)
       },
       s.SEValue.Unit,
     )

daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala

@@ -59,9 +59,10 @@ private[lf] object Pretty {
         text("Update failed due to fetch of an inactive contract") & prettyContractId(coid) &
           char('(') + (prettyTypeConName(tid)) + text(").") /
           text(s"The contract had been consumed in sub-transaction #$consumedBy:")
-      case DisclosedContractKeyHashingError(coid, tid, reason) =>
-        text("Failed to cache disclosed contract key for contract") & prettyContractId(coid) &
-          char('(') + (prettyTypeConName(tid)) + text(").") / text(reason)
+      case DisclosedContractKeyHashingError(coid, gkey, declaredHash) =>
+        text("Mismatched disclosed contract key hash for contract") & prettyContractId(coid) &
+          char('(') + prettyTypeConName(gkey.templateId) + text(").") / text("declared hash:") &
+          text(declaredHash.toHexString) & text("found hash:") & text(gkey.hash.toHexString)
       case ContractKeyNotFound(gk) =>
         text(
           "Update failed due to fetch-by-key or exercise-by-key which did not find a contract with key"

daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala

@@ -2130,16 +2130,30 @@ private[lf] object SBuiltin {
   }
 
   /** $cacheDisclosedContract[T] :: ContractId T -> CachedContract T -> Unit */
-  private[speedy] final case class SBCacheDisclosedContract(contractId: V.ContractId)
-      extends UpdateBuiltin(1) {
+  private[speedy] final case class SBCacheDisclosedContract(
+      contractId: V.ContractId,
+      keyHash: Option[crypto.Hash],
+  ) extends UpdateBuiltin(1) {
 
     override protected def executeUpdate(
         args: util.ArrayList[SValue],
         machine: UpdateMachine,
-    ): Control.Value = {
+    ): Control[Question.Update] = {
       val cachedContract = extractCachedContract(machine.tmplId2TxVersion, args.get(0))
-      machine.addDisclosedContracts(contractId, cachedContract)
-      Control.Value(SUnit)
+      (keyHash, cachedContract.keyOpt) match {
+        case (Some(hash), Some(key)) if hash != key.globalKey.hash =>
+          Control.Error(IE.DisclosedContractKeyHashingError(contractId, key.globalKey, hash))
+        case _ =>
+          // Command preprocessing should enforce the following invariant
+          val invariant = (keyHash.isDefined == cachedContract.keyOpt.isDefined)
+          if (!invariant)
+            InternalError.assertionException(
+              NameOf.qualifiedNameOfCurrentFunc,
+              "unexpected mismatching contract key",
+            )
+          machine.addDisclosedContracts(contractId, cachedContract)
+          Control.Value(SUnit)
+      }
     }
   }
 

daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/CompilerTest.scala

@@ -11,7 +11,6 @@ import com.daml.lf.interpretation.Error.TemplatePreconditionViolated
 import com.daml.lf.language.Ast._
 import com.daml.lf.speedy.SError.{SError, SErrorDamlException}
 import com.daml.lf.speedy.SExpr.SExpr
-import com.daml.lf.speedy.SValue.SContractId
 import com.daml.lf.speedy.Speedy.CachedContract
 import com.daml.lf.testing.parser.Implicits._
 import com.daml.lf.transaction.{GlobalKey, GlobalKeyWithMaintainers, TransactionVersion}
@@ -552,7 +551,7 @@ object CompilerTest {
       ),
       ArrayList(
         SValue.SText(keyLabel),
-        SValue.SList(FrontStack(SValue.SParty(maintainer))),
+        SValue.SParty(maintainer),
       ),
     )
     val globalKey =
@@ -569,8 +568,8 @@ object CompilerTest {
     val keyHash = globalKey.map(_.globalKey.hash)
     val disclosedContract = DisclosedContract(
       templateId,
-      SContractId(contractId),
-      contract(keyLabel),
+      contractId,
+      key,
       ContractMetadata(Time.Timestamp.now(), keyHash, Bytes.Empty),
     )
 
@@ -584,7 +583,7 @@ object CompilerTest {
   ): DisclosedContract = {
     DisclosedContract(
       templateId,
-      SContractId(contractId),
+      contractId,
       preCondContract(precondition = precondition),
       ContractMetadata(Time.Timestamp.now(), None, Bytes.Empty),
     )

daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/SBuiltinTest.scala

@@ -1636,7 +1636,7 @@ class SBuiltinTest extends AnyFreeSpec with Matchers with TableDrivenPropertyChe
           evalOnLedger(
             SELet1(
               cachedContractSExpr,
-              SEAppAtomic(SEBuiltin(SBCacheDisclosedContract(contractId)), Array(SELocS(1))),
+              SEAppAtomic(SEBuiltin(SBCacheDisclosedContract(contractId, None)), Array(SELocS(1))),
             ),
             getContract = Map(
               contractId -> Value.VersionedContractInstance(
@@ -1683,7 +1683,10 @@ class SBuiltinTest extends AnyFreeSpec with Matchers with TableDrivenPropertyChe
           evalOnLedger(
             SELet1(
               cachedContractSExpr,
-              SEAppAtomic(SEBuiltin(SBCacheDisclosedContract(contractId)), Array(SELocS(1))),
+              SEAppAtomic(
+                SEBuiltin(SBCacheDisclosedContract(contractId, Some(cachedKey.globalKey.hash))),
+                Array(SELocS(1)),
+              ),
             ),
             getContract = Map(
               contractId -> Value.VersionedContractInstance(
@@ -1906,7 +1909,7 @@ object SBuiltinTest {
       }
     val disclosedContract = DisclosedContract(
       templateId,
-      SContractId(contractId),
+      contractId,
       SValue.SRecord(
         templateId,
         fields.map(Ref.Name.assertFromString),

daml-lf/transaction/src/main/scala/com/daml/lf/Error.scala

@@ -53,8 +53,8 @@ object Error {
   /** When caching a disclosed contract key, hashing the contract key generated an error. */
   final case class DisclosedContractKeyHashingError(
       coid: ContractId,
-      templateId: TypeConName,
-      reason: String,
+      key: GlobalKey,
+      declaredHash: crypto.Hash,
   ) extends Error
 
   final case class ContractKeyNotVisible(

ledger-test-tool/ledger-api-tests/suites/src/main/scala/com/daml/ledger/api/testtool/suites/v1_15/ExplicitDisclosureIT.scala

@@ -505,6 +505,23 @@ final class ExplicitDisclosureIT extends LedgerTestSuite {
           checkDefiniteAnswerMetadata = true,
         )
 
+        // invalid disclosed contract (bad contract key)
+        err <- testContext
+          .dummyCreate(
+            testContext.disclosedContract.update(
+              _.metadata.contractKeyHash := ByteString.copyFromUtf8(
+                "BadKeyBadKeyBadKeyBadKeyBadKey00"
+              )
+            )
+          )
+          .mustFail("using a disclosed contract with missing createdAt in contract metadata")
+        _ = assertGrpcError(
+          err,
+          LedgerApiErrors.CommandExecution.Interpreter.GenericInterpretationError,
+          None,
+          checkDefiniteAnswerMetadata = true,
+        )
+
         //  invalid disclosed contract (missing templateId)
         err <- testContext
           .dummyCreate(
@@ -575,8 +592,8 @@ final class ExplicitDisclosureIT extends LedgerTestSuite {
   })
 
   test(
-    "EDInconsistentSuperfluousDisclosedContracts",
-    "The ledger accepts superfluous disclosed contracts with mismatching meta data",
+    "EDInconsistentCreateTimeSuperfluousDisclosedContracts",
+    "The ledger reject superfluous disclosed contracts with mismatching create time",
     allocate(SingleParty, SingleParty),
     enabled = _.explicitDisclosure,
   )(implicit ec => {
@@ -601,19 +618,6 @@ final class ExplicitDisclosureIT extends LedgerTestSuite {
         // Ensure participants are synchronized
         _ <- synchronize(ownerParticipant, delegateParticipant)
 
-        // Exercise a choice using invalid explicit disclosure (bad contract key)
-        _ <- testContext
-          .exerciseFetchDelegated(
-            testContext.disclosedContract,
-            // Provide a superfluous disclosed contract with mismatching key hash
-            whitKeyDisclosedContract
-              .update(
-                _.metadata.contractKeyHash := ByteString.copyFromUtf8(
-                  "BadKeyBadKeyBadKeyBadKeyBadKey00"
-                )
-              ),
-          )
-
         // Exercise a choice using invalid explicit disclosure (bad ledger time)
         _ <- testContext
           .exerciseFetchDelegated(