Make choice observers mandatory when available. (#8316)

* Make choice observers mandatory when available.

This is an invariant of the DAML-LF proto.

The haskell AST allows missing choice observers for any version, because the AST represents what was written in the syntax. However, if we are targeting a new DAML-LF version (>= `featureChoiceObservers`) then `Nothing` is converted to `Just (ENil TParty)` by the haskell encoder (`EncodeV1.hs`) to satisfy the proto invariant.

On the scala side, the decoder (`DecodeV1.scala`) now insists that choice observers are present when_ the version is new. (>= `LV.Features.choiceObservers`).

changelog_begin
changelog_end

* LF: LF encoder generate always choice observer if lf >= 1.dev (#8318)

This is a follow up of #8316.
This advances the state of #7709

CHANGELOG_BEGIN
CHANGELOG_END

Co-authored-by: Remy <remy.haemmerle@daml.com>

compiler/daml-lf-proto/src/DA/Daml/LF/Proto3/EncodeV1.hs

@@ -20,6 +20,7 @@ import           Data.Functor.Identity
 import qualified Data.HashMap.Strict as HMS
 import qualified Data.List as L
 import qualified Data.Map.Strict as Map
+import           Data.Maybe (fromMaybe)
 import qualified Data.NameMap as NM
 import qualified Data.Text           as T
 import qualified Data.Text.Lazy      as TL
@@ -878,12 +879,21 @@ encodeTemplateKey TemplateKey{..} = do
     defTemplate_DefKeyMaintainers <- encodeExpr tplKeyMaintainers
     pure P.DefTemplate_DefKey{..}
 
+encodeChoiceObservers :: Maybe Expr -> Encode (Just P.Expr)
+encodeChoiceObservers chcObservers = do
+  EncodeEnv{..} <- get
+  -- The choice-observers field is mandatory when supported. So, when choice-observers are
+  -- not syntactically explicit, we generate an empty-party-list expression here.
+  if version `supports` featureChoiceObservers
+  then encodeExpr (fromMaybe (ENil TParty) chcObservers)
+  else traverse encodeExpr' chcObservers
+
 encodeTemplateChoice :: TemplateChoice -> Encode P.TemplateChoice
 encodeTemplateChoice TemplateChoice{..} = do
     templateChoiceName <- encodeName unChoiceName chcName
     let templateChoiceConsuming = chcConsuming
     templateChoiceControllers <- encodeExpr chcControllers
-    templateChoiceObservers <- traverse encodeExpr' chcObservers
+    templateChoiceObservers <- encodeChoiceObservers chcObservers
     templateChoiceSelfBinder <- encodeName unExprVarName chcSelfBinder
     templateChoiceArgBinder <- Just <$> encodeExprVarWithType chcArgBinder
     templateChoiceRetType <- encodeType chcReturnType

compiler/damlc/tests/daml-test-files/ChoiceObservers.daml

@@ -5,7 +5,7 @@ module ChoiceObservers where
 
 -- @SINCE-LF 1.dev
 
--- @QUERY-LF [ .modules[] | .templates[] | select(lf::get_template_name($pkg) == ["TheTemplate"]) | .choices[] | select(lf::get_name($pkg) == "C1") | .observers ] == [ null ]
+-- @QUERY-LF [ .modules[] | .templates[] | select(lf::get_template_name($pkg) == ["TheTemplate"]) | .choices[] | select(lf::get_name($pkg) == "C1") | .observers | has("nil") ] == [ true ]
 
 -- @QUERY-LF [ .modules[] | .templates[] | select(lf::get_template_name($pkg) == ["TheTemplate"]) | .choices[] | select(lf::get_name($pkg) == "C2") | .observers | has("app") ] == [ true ]
 
@@ -30,7 +30,7 @@ template TheTemplate
       do return ()
 
     choice C3 : () with ys : [Party]
-      observer ([]:[Party]) -- explicit empty choice observers; should be distinguishable from no observer clause
+      observer ([]:[Party]) -- explicit empty choice observers; semantically indistinguishable from no observer clause
       controller ys
       do return ()
 

daml-lf/archive/src/main/protobuf/com/daml/daml_lf_dev/daml_lf_1.proto

@@ -1281,7 +1281,7 @@ message TemplateChoice {
   Expr controllers = 3;
 
   // The additional informees of the choice. They have type `List Party`.
-  Expr observers = 11; // *Available in versions >= 1.dev*
+  Expr observers = 11; // *Mandatory in versions >= 1.dev*
 
   // Name to which the choice argument is bound and its type.
   VarWithType arg_binder = 4;

daml-lf/archive/src/main/scala/com/digitalasset/daml/lf/archive/DecodeV1.scala

@@ -583,6 +583,7 @@ private[archive] class DecodeV1(minor: LV.Minor) extends Decode.OfPackage[PLF.Pa
           assertSince(LV.Features.choiceObservers, "TemplateChoice.observers")
           Some(decodeExpr(lfChoice.getObservers, s"$tpl:$chName:observers"))
         } else {
+          assertUntil(LV.Features.choiceObservers, "missing TemplateChoice.observers")
           None
         },
         selfBinder = selfBinder,

daml-lf/archive/src/test/scala/com/digitalasset/daml/lf/archive/DecodeV1Spec.scala

@@ -946,13 +946,17 @@ class DecodeV1Spec
         val decoder = moduleDecoder(version, stringTable)
 
         decoder.decodeChoice(templateName, protoChoiceWithoutObservers)
-        a[ParseError] should be thrownBy decoder
-          .decodeChoice(templateName, protoChoiceWithObservers)
+        a[ParseError] should be thrownBy (
+          decoder
+            .decodeChoice(
+              templateName,
+              protoChoiceWithObservers)
+          )
 
       }
     }
 
-    "accept choice with or without observers if lv version >= 1.dev" in {
+    "reject choice without observers if lv version >= 1.dev" in {
 
       val unitTyp = DamlLf1.Type.newBuilder().setInterned(0).build()
 
@@ -976,7 +980,11 @@ class DecodeV1Spec
 
         val decoder = moduleDecoder(version, stringTable, ImmArraySeq.empty, typeTable)
 
-        decoder.decodeChoice(templateName, protoChoiceWithoutObservers)
+        a[ParseError] should be thrownBy (
+          decoder.decodeChoice(
+            templateName,
+            protoChoiceWithoutObservers)
+        )
         decoder.decodeChoice(templateName, protoChoiceWithObservers)
       }
     }

daml-lf/encoder/src/main/scala/com/digitalasset/daml/lf/archive/testing/EncodeV1.scala

@@ -7,6 +7,7 @@ package testing
 import com.daml.lf.data.Ref._
 import com.daml.lf.data._
 import com.daml.lf.language.Ast._
+import com.daml.lf.language.{Util => AstUtil}
 import com.daml.lf.language.{LanguageVersion => LV}
 import com.daml.daml_lf_dev.{DamlLf1 => PLF}
 
@@ -663,6 +664,14 @@ private[daml] class EncodeV1(minor: LV.Minor) {
       setString(name, b.setNameStr, b.setNameInternedStr)
       b.setConsuming(choice.consuming)
       b.setControllers(choice.controllers)
+      choice.choiceObservers match {
+        case Some(value) =>
+          assertUntil(LV.Features.choiceObservers, "TemplateChoice.observer")
+          b.setObservers(value)
+        case None if languageVersion >= LV.Features.choiceObservers =>
+          b.setObservers(ENil(AstUtil.TParty))
+        case _ =>
+      }
       b.setArgBinder(choice.argBinder._1 -> choice.argBinder._2)
       b.setRetType(choice.returnType)
       b.setUpdate(choice.update)
@@ -733,6 +742,10 @@ private[daml] class EncodeV1(minor: LV.Minor) {
     if (versionIsOlderThan(minVersion))
       throw EncodeError(s"$description is not supported by DAML-LF 1.$minor")
 
+  private def assertUntil(minVersion: LV, description: String): Unit =
+    if (versionIsOlderThan(minVersion))
+      throw EncodeError(s"non empty $description is not supported by DAML-LF 1.$minor")
+
 }
 
 object EncodeV1 {

daml-lf/encoder/src/test/lf/Template_1.dev_.lf

@@ -0,0 +1,26 @@
+// Copyright (c) 2020 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+
+module TemplateMod {
+
+  record @serializable Person = { person: Party, name: Text } ;
+
+  template (this : Person) =  {
+    precondition True,
+    signatories Cons @Party [TemplateMod:Person {person} this] (Nil @Party),
+    observers (Nil @Party),
+    agreement "Agreement",
+    choices {
+      choice Sleep (self) (u: Unit) : ContractId TemplateMod:Person
+        , controllers Cons @Party [TemplateMod:Person {person} this] (Nil @Party)
+        , observers Cons @Party [TemplateMod:Person {person} this] (Nil @Party)
+        to upure @(ContractId TemplateMod:Person) self,
+      choice @nonConsuming Nap (self) (i : Int64) : Int64
+        , controllers Cons @Party [TemplateMod:Person {person} this] (Nil @Party)
+        , observers Cons @Party [TemplateMod:Person {person} this] (Nil @Party)
+        to upure @Int64 i
+    }
+  } ;
+
+}

daml-lf/encoder/src/test/scala/com/digitalasset/daml/lf/archive/testing/EncodeV1Spec.scala

@@ -7,7 +7,6 @@ import com.daml.lf.archive.Decode
 import com.daml.lf.archive.testing.Encode
 import com.daml.lf.data.Ref._
 import com.daml.lf.language.Ast._
-import com.daml.lf.language.LanguageMajorVersion.V1
 import com.daml.lf.language.{Ast, LanguageVersion}
 import com.daml.lf.testing.parser.Implicits.SyntaxHelper
 import com.daml.lf.testing.parser.{AstRewriter, ParserParameters}
@@ -25,7 +24,7 @@ class EncodeV1Spec extends AnyWordSpec with Matchers with TableDrivenPropertyChe
   val defaultParserParameters: ParserParameters[this.type] =
     ParserParameters(
       pkgId,
-      LanguageVersion(V1, LanguageVersion.Minor("8"))
+      LanguageVersion.StableVersions.max
     )
 
   "Encode and Decode" should {