Fix disclosure bug in the presence of upgrades in 2.x (#19199)

* Backport pull/19167 * fix integration test
digital-asset · May 15, 2024 · adf414d · adf414d
1 parent a6c51e7
commit adf414d
Show file tree

Hide file tree

Showing 12 changed files with 152 additions and 89 deletions.
diff --git a/sdk/daml-assistant/integration-tests/src/DA/Daml/Assistant/IntegrationTests.hs b/sdk/daml-assistant/integration-tests/src/DA/Daml/Assistant/IntegrationTests.hs
@@ -163,7 +163,7 @@ damlStart tmpDir _disableUpgradeValidation = do
     scriptOutput <- readFileUTF8 (projDir </> scriptOutputFile)
     let alice = (read scriptOutput :: String)
     -- TODO(raphael-speyer-da): Use a package name once supported by canton out of the box.
-    let packageRef = "6310b9aa3d506211b592dd657afb167d63637453f31eae986fad5aa1b6d61401"
+    packageRef <- extractPackageRefFromDar projDir
     pure $
         DamlStartResource
             { projDir = projDir
@@ -180,6 +180,20 @@ damlStart tmpDir _disableUpgradeValidation = do
             , stdoutChan = outChan
             }
 
+extractPackageRefFromDar :: String -> IO String
+extractPackageRefFromDar projDir = do
+    inspectOutput <-
+        readCreateProcess
+            ( shell $
+                "daml damlc inspect "
+                    <> projDir
+                    <> "/.daml/dist/assistant-integration-tests-1.0.dar"
+            )
+            ""
+    case words $ head $ lines inspectOutput of
+        [_, packageRef] -> pure packageRef
+        _ -> error "Failed to extract package ref from DAR"
+
 tests :: SdkVersioned => FilePath -> TestTree
 tests tmpDir =
     withSdkResource $ \_ ->
@@ -482,7 +496,7 @@ damlStartTestsWithoutValidation getDamlStart =
         untilM_ (pure ()) $ do
             line <- atomically $ readTChan stdoutReadChan
             pure ("Rebuild complete" `isInfixOf` line)
-        let newPackageRef = "8caa3f3b63b66d0338e0bb8b931c5c57644a4362b3bd82c482bd37d7438abcc8"
+        newPackageRef <- extractPackageRefFromDar projDir
         initialRequest <-
             parseRequest $ "http://localhost:" <> show jsonApiPort <> "/v1/query"
         manager <- newManager defaultManagerSettings

diff --git a/sdk/daml-script/daml/Daml/Script.daml b/sdk/daml-script/daml/Daml/Script.daml
@@ -302,7 +302,7 @@ data QueryDisclosurePayload a = QueryDisclosurePayload
   { parties : [Party]
   , tplId : TemplateTypeRep
   , cid : ContractId ()
-  , continue : Optional Text -> a
+  , continue : Optional (TemplateTypeRep, Text) -> a
   , locations : [(Text, SrcLoc)]
   } deriving Functor
 
@@ -317,7 +317,7 @@ queryDisclosure p c = lift $ Free $ QueryDisclosure QueryDisclosurePayload with
     parties = toParties p
     tplId = templateTypeRep @t
     cid = cid
-    continue = pure . fmap \blob -> Disclosure tplId cid blob
+    continue = pure . fmap \(actualTplId, blob) -> Disclosure actualTplId cid blob
     locations = getCallStack callStack
   where
     tplId = templateTypeRep @t

diff --git a/sdk/daml-script/daml3/Daml/Script/Questions/Query.daml b/sdk/daml-script/daml3/Daml/Script/Questions/Query.daml
@@ -36,20 +36,23 @@ data QueryContractId = QueryContractId with
   parties : [Party]
   tplId : TemplateTypeRep
   cid : ContractId ()
-instance IsQuestion QueryContractId (Optional (AnyTemplate, Text)) where command = "QueryContractId"
+instance IsQuestion QueryContractId (Optional (AnyTemplate, TemplateTypeRep, Text)) where command = "QueryContractId"
 
 -- | Query for the contract with the given contract id.
 --
 -- Returns `None` if there is no active contract the party is a stakeholder on.
+-- Otherwise returns a triplet (anyTemplate, templateId, blob) where anyTemplate
+-- is the contract upgraded or downgraded to `t`, templateId is the ID of the
+-- template as stored in the ledger (may be different from `t`), and blob is the
+-- disclosure of the template as stored in the ledger (of type templateId).
 --
 -- WARNING: Over the gRPC and with the JSON API
 -- in-memory backend this performs a linear search so only use this if the number of
 -- active contracts is small.
 --
 -- This is semantically equivalent to calling `query`
 -- and filtering on the client side.
-queryContractId_ : forall t p. (Template t, IsParties p) => HasCallStack => p -> ContractId t -> Script (Optional (AnyTemplate, Text))
--- The 'HasAgreement t' constraint prevents this function from being used on interface types.
+queryContractId_ : forall t p. (Template t, IsParties p) => HasCallStack => p -> ContractId t -> Script (Optional (AnyTemplate, TemplateTypeRep, Text))
 queryContractId_ p c = lift $ QueryContractId with
     parties = toParties p
     tplId = templateTypeRep @t
@@ -59,14 +62,13 @@ queryContractId_ p c = lift $ QueryContractId with
 --    convert = fmap $ fromSome . fromAnyTemplate
 
 queryContractId: forall t p. (Template t, HasAgreement t, IsParties p) => HasCallStack => p -> ContractId t -> Script (Optional t)
-queryContractId p c = fmap (fmap $ fromSome . fromAnyTemplate . fst) $ queryContractId_ p c
+queryContractId p c = fmap (fmap $ \(anyTpl, _, _) -> fromSome (fromAnyTemplate anyTpl)) $ queryContractId_ p c
 
 -- TODO https://github.com/digital-asset/daml/issues/17755
 --  clean the API for different query function
 queryDisclosure: forall t p. (Template t, IsParties p) => HasCallStack => p -> ContractId t -> Script (Optional Disclosure)
-queryDisclosure p c = fmap (fmap  $ \(_, blob) -> Disclosure tplId cid blob) $ queryContractId_ p c
+queryDisclosure p c = fmap (fmap  $ \(_, tplId, blob) -> Disclosure tplId cid blob) $ queryContractId_ p c
  where
-    tplId = templateTypeRep @t
     cid = coerceContractId c
 
 data QueryInterface = QueryInterface with

diff --git a/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala b/sdk/daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Converter.scala
@@ -103,8 +103,14 @@ abstract class ConverterMethods(stablePackages: StablePackages) {
     )
   }
 
+  private[lf] def fromTemplateTypeRep(templateId: SValue): SValue =
+    record(stablePackages.TemplateTypeRep, ("getTemplateTypeRep", templateId))
+
   private[lf] def fromTemplateTypeRep(templateId: value.Identifier): SValue =
-    record(stablePackages.TemplateTypeRep, ("getTemplateTypeRep", fromIdentifier(templateId)))
+    fromTemplateTypeRep(fromIdentifier(templateId))
+
+  private[lf] def fromTemplateTypeRep(templateId: Identifier): SValue =
+    fromTemplateTypeRep(STypeRep(TTyCon(templateId)))
 
   private[lf] def fromAnyContractId(
       scriptIds: ScriptIds,
@@ -245,10 +251,7 @@ abstract class ConverterMethods(stablePackages: StablePackages) {
       ("getAnyContractKey", SAny(key.ty, key.key)),
       (
         "getAnyContractKeyTemplateTypeRep",
-        record(
-          stablePackages.TemplateTypeRep,
-          ("getTemplateTypeRep", STypeRep(TTyCon(key.templateId))),
-        ),
+        fromTemplateTypeRep(key.templateId),
       ),
     )
 

diff --git a/...ml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/Converter.scala b/...ml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/Converter.scala
@@ -402,4 +402,9 @@ object Converter extends script.ConverterMethods(StablePackagesV1) {
     }
     iter(freeAp, List())
   }
+
+  def makePair(v1: SValue, v2: SValue): SValue = {
+    import com.daml.script.converter.Converter.record
+    record(StablePackagesV1.Tuple2, ("_1", v1), ("_2", v2))
+  }
 }
diff --git a/...daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ScriptF.scala b/...daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v1/ScriptF.scala
@@ -6,35 +6,35 @@ package engine
 package script
 package v1
 
-import java.time.Clock
-import org.apache.pekko.stream.Materializer
 import com.daml.grpc.adapter.ExecutionSequencerFactory
 import com.daml.ledger.api.domain.{User, UserRight}
 import com.daml.lf.data.FrontStack
-import com.daml.lf.{CompiledPackages, command}
-import com.daml.lf.engine.preprocessing.ValueTranslator
-import com.daml.lf.data.Ref.{Identifier, Name, PackageId, Party, UserId}
+import com.daml.lf.data.Ref._
 import com.daml.lf.data.Time.Timestamp
-import com.daml.lf.language.{Ast, StablePackagesV1}
-import com.daml.lf.speedy.SExpr.{SEAppAtomic, SEValue}
-import com.daml.lf.speedy.{ArrayList, SError, SValue}
-import com.daml.lf.speedy.SExpr.SExpr
+import com.daml.lf.engine.preprocessing.ValueTranslator
+import com.daml.lf.language.Ast
+import com.daml.lf.speedy.SExpr.{SEAppAtomic, SEValue, SExpr}
 import com.daml.lf.speedy.SValue._
 import com.daml.lf.speedy.Speedy.PureMachine
+import com.daml.lf.speedy.{ArrayList, SError, SValue}
 import com.daml.lf.value.Value
 import com.daml.lf.value.Value.ContractId
-import scalaz.{Foldable, OneAnd}
-import scalaz.syntax.traverse._
+import com.daml.lf.{CompiledPackages, command}
+import com.daml.script.converter.Converter.{toContractId, toText}
+import org.apache.pekko.stream.Materializer
 import scalaz.std.either._
 import scalaz.std.list._
 import scalaz.std.option._
-import com.daml.script.converter.Converter.{toContractId, toText}
+import scalaz.syntax.traverse._
+import scalaz.{Foldable, OneAnd}
 
+import java.time.Clock
 import scala.concurrent.{ExecutionContext, Future}
 
 sealed trait ScriptF
 
 object ScriptF {
+
   final case class Catch(act: SValue, handle: SValue, continue: SValue) extends ScriptF
   final case class Throw(exc: SAny) extends ScriptF
 
@@ -230,6 +230,7 @@ object ScriptF {
       } yield SEAppAtomic(SEValue(continue), Array(SEValue(SList(res))))
 
   }
+
   final case class QueryContractId(
       parties: OneAnd[Set, Party],
       tplId: Identifier,
@@ -250,7 +251,14 @@ object ScriptF {
         optR <- client.queryContractId(parties, tplId, cid)
         optR <- Converter.toFuture(
           if (asDisclosure)
-            Right(optR.map(c => SValue.SText(c.blob.toHexString)))
+            Right(
+              optR.map(c =>
+                Converter.makePair(
+                  Converter.fromTemplateTypeRep(c.templateId),
+                  SValue.SText(c.blob.toHexString),
+                )
+              )
+            )
           else
             optR.traverse(Converter.fromContract(env.valueTranslator, _))
         )
@@ -271,11 +279,6 @@ object ScriptF {
         esf: ExecutionSequencerFactory,
     ): Future[SExpr] = {
 
-      def makePair(v1: SValue, v2: SValue): SValue = {
-        import com.daml.script.converter.Converter.record
-        record(StablePackagesV1.Tuple2, ("_1", v1), ("_2", v2))
-      }
-
       for {
         viewType <- Converter.toFuture(env.lookupInterfaceViewTy(interfaceId))
         client <- Converter.toFuture(env.clients.getPartiesParticipant(parties))
@@ -286,7 +289,7 @@ object ScriptF {
             .traverse { case (cid, optView) =>
               optView match {
                 case None =>
-                  Right(makePair(SContractId(cid), SOptional(None)))
+                  Right(Converter.makePair(SContractId(cid), SOptional(None)))
                 case Some(view) =>
                   for {
                     view <- Converter.fromInterfaceView(
@@ -295,7 +298,7 @@ object ScriptF {
                       view,
                     )
                   } yield {
-                    makePair(SContractId(cid), SOptional(Some(view)))
+                    Converter.makePair(SContractId(cid), SOptional(Some(view)))
                   }
               }
             }

diff --git a/...ml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/Converter.scala b/...ml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/v2/Converter.scala
@@ -218,26 +218,23 @@ object Converter extends script.ConverterMethods(StablePackagesV2) {
       )
       .map { xs => SList(xs.to(FrontStack)) }
 
-  // Convert an active contract to AnyTemplate
-  def fromContract(
-      translator: preprocessing.ValueTranslator,
-      contract: ScriptLedgerClient.ActiveContract,
-      enableContractUpgrading: Boolean = false,
-  ): Either[String, SValue] =
-    fromAnyTemplate(translator, contract.templateId, contract.argument, enableContractUpgrading)
-
   // Convert a Created event to a pair of (ContractId (), AnyTemplate)
   def fromCreated(
       translator: preprocessing.ValueTranslator,
       contract: ScriptLedgerClient.ActiveContract,
+      targetTemplateId: Identifier,
       enableContractUpgrading: Boolean = false,
   ): Either[String, SValue] = {
     for {
-      anyTpl <- fromContract(translator, contract, enableContractUpgrading)
-    } yield record(
-      StablePackagesV2.Tuple2,
-      ("_1", SContractId(contract.contractId)),
-      ("_2", anyTpl),
+      anyTpl <- fromAnyTemplate(
+        translator,
+        targetTemplateId,
+        contract.argument,
+        enableContractUpgrading,
+      )
+    } yield makeTuple(
+      SContractId(contract.contractId),
+      anyTpl,
     )
   }
 
@@ -417,4 +414,10 @@ object Converter extends script.ConverterMethods(StablePackagesV2) {
       ("version", SText(packageName.version.toString)),
     )
   }
+
+  def makeTuple(v1: SValue, v2: SValue): SValue =
+    record(StablePackagesV2.Tuple2, ("_1", v1), ("_2", v2))
+
+  def makeTuple(v1: SValue, v2: SValue, v3: SValue): SValue =
+    record(StablePackagesV2.Tuple3, ("_1", v1), ("_2", v2), ("_3", v3))
 }