Fix authorization in try contexts (#9823)

wildcard pattern matches bite once again. We clearly do not want to use the committers here. I tried to move most of the logic to PartialTransaction but unfortunately moving the submitters to PartialTransaction doesn’t quite work since as usual scenarios make things difficult because submitters change at weird places and we reset the partial transaction in weird places. changelog_begin changelog_end
digital-asset · May 28, 2021 · 269237e · 269237e
1 parent cb06fb3
commit 269237e
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 9 deletions.
diff --git a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Speedy.scala b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Speedy.scala
@@ -12,7 +12,6 @@ import com.daml.lf.language.Ast._
 import com.daml.lf.language.{Util => AstUtil}
 import com.daml.lf.ledger.Authorize
 import com.daml.lf.speedy.Compiler.{CompilationError, PackageNotFound}
-import com.daml.lf.speedy.PartialTransaction.ExercisesContextInfo
 import com.daml.lf.speedy.SError._
 import com.daml.lf.speedy.SExpr._
 import com.daml.lf.speedy.SResult._
@@ -319,12 +318,7 @@ private[lf] object Speedy {
 
     private[lf] def contextActors: Set[Party] =
       withOnLedger("ptx") { onLedger =>
-        onLedger.ptx.context.info match {
-          case ctx: ExercisesContextInfo =>
-            ctx.actingParties union ctx.signatories
-          case _ =>
-            onLedger.committers
-        }
+        onLedger.ptx.context.info.authorizers.getOrElse(onLedger.committers)
       }
 
     private[lf] def auth: Authorize = Authorize(this.contextActors)

diff --git a/.../interpreter/src/main/scala/com/digitalasset/daml/lf/transaction/PartialTransaction.scala b/.../interpreter/src/main/scala/com/digitalasset/daml/lf/transaction/PartialTransaction.scala
@@ -37,9 +37,14 @@ private[lf] object PartialTransaction {
 
   sealed abstract class ContextInfo {
     val actionChildSeed: Int => crypto.Hash
+    // This is None for root actions since
+    // PartialTransaction does not keep track of committers.
+    def authorizers: Option[Set[Party]]
   }
 
-  sealed abstract class RootContextInfo extends ContextInfo
+  sealed abstract class RootContextInfo extends ContextInfo {
+    override val authorizers: Option[Set[Party]] = None
+  }
 
   private[PartialTransaction] final class SeededTransactionRootContext(seed: crypto.Hash)
       extends RootContextInfo {
@@ -133,6 +138,7 @@ private[lf] object PartialTransaction {
   ) extends ContextInfo {
     val actionNodeSeed = parent.nextActionChildSeed
     val actionChildSeed = crypto.Hash.deriveNodeSeed(actionNodeSeed, _)
+    override val authorizers: Option[Set[Party]] = Some(actingParties union signatories)
   }
 
   final case class ActiveLedgerState(
@@ -146,6 +152,9 @@ private[lf] object PartialTransaction {
       // beginState stores the consumed contracts at the beginning of
       // the try so that we can restore them on rollback.
       beginState: ActiveLedgerState,
+      // Set to the authorizers (the union of signatories & actors) of the nearest
+      // parent exercise or None if there is no parent exercise.
+      authorizers: Option[Set[Party]],
   ) extends ContextInfo {
     val actionChildSeed: NodeIdx => crypto.Hash = parent.info.actionChildSeed
   }
@@ -617,7 +626,7 @@ private[lf] case class PartialTransaction(
     */
   def beginTry: PartialTransaction = {
     val nid = NodeId(nextNodeIdx)
-    val info = TryContextInfo(nid, context, activeState)
+    val info = TryContextInfo(nid, context, activeState, authorizers = context.info.authorizers)
     copy(
       nextNodeIdx = nextNodeIdx + 1,
       context = Context(info, BackStack.empty, context.nextActionChildIdx),

diff --git a/daml-lf/tests/scenario/dev/exception-auth/EXPECTED.ledger b/daml-lf/tests/scenario/dev/exception-auth/EXPECTED.ledger
@@ -0,0 +1,38 @@
+transactions:
+TX #0 1970-01-01T00:00:00Z [Test:42] version: dev
+#0:0 version: dev
+│   referenced by #1:0, #2:0
+│   known to (since): a (#0), b (#0)
+└─> create Test:Tester@XXXXXXXX
+    with: { a = 'a', b = 'b' }
+
+TX #1 1970-01-01T00:00:00Z [Test:44] version: dev
+#1:0 version: dev
+│   known to (since): a (#1), b (#1)
+└─> b exercises CreateInTry:Test:Tester@XXXXXXXX on 009877e9c7e647b905ba9185add7af1b24e664508899260672b7e21c59ad9168d1
+    with {  }
+    children:
+    #1:1 no-version
+    │   known to (since): a (#1), b (#1)
+    └─> rollback:
+    #1:2 version: dev
+    │   known to (since): a (#1), b (#1)
+    └─> create Test:T@XXXXXXXX
+        with: { ps = ['a', 'b'] }
+
+TX #2 1970-01-01T00:00:00Z [Test:47] version: dev
+#2:0 version: dev
+│   known to (since): a (#2), b (#2)
+└─> b exercises CreateInNestedTry:Test:Tester@XXXXXXXX on 009877e9c7e647b905ba9185add7af1b24e664508899260672b7e21c59ad9168d1
+    with {  }
+    children:
+    #2:1 no-version
+    │   known to (since): a (#2), b (#2)
+    └─> rollback:
+    #2:2 version: dev
+    │   known to (since): a (#2), b (#2)
+    └─> create Test:T@XXXXXXXX
+        with: { ps = ['a', 'b'] }
+
+active contracts:
+   009877e9c7e647b905ba9185add7af1b24e664508899260672b7e21c59ad9168d1
diff --git a/daml-lf/tests/scenario/dev/exception-auth/Test.daml b/daml-lf/tests/scenario/dev/exception-auth/Test.daml
@@ -0,0 +1,47 @@
+-- Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+-- SPDX-License-Identifier: Apache-2.0
+
+
+module Test where
+
+import DA.Exception
+
+template T
+  with
+    ps : [Party]
+  where
+    signatory ps
+
+template Tester
+  with
+    a : Party
+    b : Party
+  where
+    signatory a
+    observer b
+    nonconsuming choice CreateInTry : ()
+      controller b
+      do try do
+           create (T [a, b])
+           abort ""
+         catch
+           GeneralError _ -> pure ()
+
+    nonconsuming choice CreateInNestedTry : ()
+      controller b
+      do try try do create (T [a, b])
+                    abort ""
+             catch
+               GeneralError _ -> pure ()
+         catch
+           GeneralError _ -> pure ()
+
+run = scenario do
+  a <- getParty "a"
+  b <- getParty "b"
+  cid <- submit a $ create (Tester a b)
+  -- Tests that we have the authorization from the parent exercise.
+  submit b $ exercise cid CreateInTry
+  -- Tests that we have the authorization from the parent exercise
+  -- even if there is another rollback in between.
+  submit b $ exercise cid CreateInNestedTry