diff --git a/compiler/damlc/tests/daml-test-files/ContractKeyNotEffective.daml b/compiler/damlc/tests/daml-test-files/ContractKeyNotEffective.daml new file mode 100644 index 00000000000..791b10a3868 --- /dev/null +++ b/compiler/damlc/tests/daml-test-files/ContractKeyNotEffective.daml @@ -0,0 +1,18 @@ +-- @ERROR range=14:1-14:19; contract 0055b33c9fd9928bdf0c2c611194865917b9aaf8b4b94a44abc25ddfea6d866cd1 not effective, but we found its key! +module ContractKeyNotEffective where + +import DA.Time + +template T + with + p : Party + where + signatory p + key p : Party + maintainer key + +fetchByKeyMustFail = scenario do + p <- getParty "alice" + submit p $ create (T p) + pass (- seconds 1) + submit p $ fetchByKey @T p diff --git a/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Engine.scala b/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Engine.scala index 9b10032a20f..034d4a29e02 100644 --- a/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Engine.scala +++ b/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Engine.scala @@ -326,11 +326,11 @@ class Engine(val config: EngineConfig = new EngineConfig(LanguageVersion.StableV }, ) - case SResultNeedContract(contractId, _, _, _, cbPresent) => + case SResultNeedContract(contractId, _, _, callback) => return Result.needContract( contractId, { coinst => - cbPresent(coinst) + callback(coinst) interpretLoop(machine, time) }, ) @@ -342,17 +342,16 @@ class Engine(val config: EngineConfig = new EngineConfig(LanguageVersion.StableV case SResultNeedKey(gk, _, cb) => return ResultNeedKey( gk, - result => - if (cb(result)) - interpretLoop(machine, time) - else - ResultError(Error.Interpretation.ContractKeyNotFound(gk.globalKey)), + { result => + cb(result) + interpretLoop(machine, time) + }, ) case SResultNeedLocalKeyVisible(stakeholders, _, cb) => return ResultNeedLocalKeyVisible( stakeholders, - result => { + { result => cb(result.toSVisibleByKey) interpretLoop(machine, time) }, diff --git a/daml-lf/interpreter/BUILD.bazel b/daml-lf/interpreter/BUILD.bazel index 3872806c12d..d3e791b74ca 100644 --- a/daml-lf/interpreter/BUILD.bazel +++ b/daml-lf/interpreter/BUILD.bazel @@ -34,6 +34,7 @@ da_scala_library( "//daml-lf/language", "//daml-lf/transaction", "//daml-lf/validation", + "//libs-scala/scala-utils", "@maven//:com_google_protobuf_protobuf_java", "@maven//:org_slf4j_slf4j_api", ], diff --git a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala index 2f82a4b7735..bc795842fef 100644 --- a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala +++ b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala @@ -20,6 +20,7 @@ import com.daml.lf.speedy.SValue.{SValue => SV} import com.daml.lf.transaction.{Transaction => Tx} import com.daml.lf.value.{Value => V} import com.daml.lf.transaction.{GlobalKey, GlobalKeyWithMaintainers, Node} +import com.daml.scalautil.Statement.discard import scala.jdk.CollectionConverters._ import scala.collection.immutable.TreeSet @@ -1023,11 +1024,7 @@ private[lf] object SBuiltin { coid, templateId, onLedger.committers, - cbMissing = _ => false, - cbPresent = { case V.ContractInst(actualTmplId, V.VersionedValue(_, arg), _) => - // Note that we cannot throw in this continuation -- instead - // set the control appropriately which will crash the machine - // correctly later. + { case V.ContractInst(actualTmplId, V.VersionedValue(_, arg), _) => if (actualTmplId != templateId) { machine.ctrl = SEDamlException(DamlEWronglyTypedContract(coid, templateId, actualTmplId)) @@ -1139,13 +1136,14 @@ private[lf] object SBuiltin { private[this] val typ = AstUtil.TContractId(Ast.TTyCon(templateId)) final protected def importCid(cid: V.ContractId): SEImportValue = + // We have to check that the discriminator of cid does not conflict with a local ones + // however we cannot raise an exception in case of failure here. + // We delegate to CtrlImportValue the task to check cid. SEImportValue(typ, V.ValueContractId(cid)) // Callback from the engine returned NotFound - def handleInputKeyNotFound(machine: Machine): Boolean - // CallBack from the engine returned a new Cid def handleInputKeyFound(machine: Machine, cid: V.ContractId): Unit // We already saw this key, but it was undefined or was archived - def handleInactiveKey(machine: Machine, gkey: GlobalKey): Unit + def handleKeyNotFound(machine: Machine, gkey: GlobalKey): Boolean // We already saw this key and it is still active def handleActiveKey(machine: Machine, cid: V.ContractId): Unit @@ -1156,30 +1154,30 @@ private[lf] object SBuiltin { ): Unit = keyMapping match { case PartialTransaction.KeyActive(cid) => handleActiveKey(machine, cid) - case PartialTransaction.KeyInactive => handleInactiveKey(machine, gkey) + case PartialTransaction.KeyInactive => discard(handleKeyNotFound(machine, gkey)) } } private[this] object KeyOperation { final class Fetch(override val templateId: TypeConName) extends KeyOperation { - override def handleInputKeyNotFound(machine: Machine): Boolean = false override def handleInputKeyFound(machine: Machine, cid: V.ContractId): Unit = machine.ctrl = importCid(cid) - override def handleInactiveKey(machine: Machine, gkey: GlobalKey): Unit = + override def handleKeyNotFound(machine: Machine, gkey: GlobalKey): Boolean = { machine.ctrl = SEDamlException(DamlEContractKeyNotFound(gkey)) + false + } + override def handleActiveKey(machine: Machine, cid: V.ContractId): Unit = machine.returnValue = SContractId(cid) } final class Lookup(override val templateId: TypeConName) extends KeyOperation { - override def handleInputKeyNotFound(machine: Machine): Boolean = { + override def handleInputKeyFound(machine: Machine, cid: V.ContractId): Unit = + machine.ctrl = SBSome(importCid(cid)) + override def handleKeyNotFound(machine: Machine, key: GlobalKey): Boolean = { machine.returnValue = SValue.SValue.None true } - override def handleInputKeyFound(machine: Machine, cid: V.ContractId): Unit = - machine.ctrl = SBSome(importCid(cid)) - override def handleInactiveKey(machine: Machine, key: GlobalKey): Unit = - machine.returnValue = SValue.SValue.None override def handleActiveKey(machine: Machine, cid: V.ContractId): Unit = machine.returnValue = SOptional(Some(SContractId(cid))) } @@ -1200,7 +1198,11 @@ private[lf] object SBuiltin { ) } - final def execute(args: util.ArrayList[SValue], machine: Machine, onLedger: OnLedger): Unit = { + final override def execute( + args: util.ArrayList[SValue], + machine: Machine, + onLedger: OnLedger, + ): Unit = { import PartialTransaction.{KeyActive, KeyInactive} val keyWithMaintainers = extractKeyWithMaintainers(args.get(0)) if (keyWithMaintainers.maintainers.isEmpty) @@ -1244,30 +1246,18 @@ private[lf] object SBuiltin { onLedger.committers, { result => cacheGlobalLookup(onLedger, gkey, result) - // We need to check if the contract was consumed since we only - // modify keys if the archive was for a key - // already brought into scope. - val activeResult = result match { - case Some(cid) if onLedger.ptx.consumedBy.contains(cid) => - None - case _ => - result - } - activeResult match { - case Some(cid) => + result match { + case Some(cid) if !onLedger.ptx.consumedBy.contains(cid) => onLedger.ptx = onLedger.ptx.copy( keys = onLedger.ptx.keys.updated(gkey, KeyActive(cid)) ) - // We have to check that the discriminator of cid does not conflict with a local ones - // however we cannot raise an exception in case of failure here. - // We delegate to CtrlImportValue the task to check cid. operation.handleInputKeyFound(machine, cid) true - case None => + case _ => onLedger.ptx = onLedger.ptx.copy( keys = onLedger.ptx.keys.updated(gkey, KeyInactive) ) - operation.handleInputKeyNotFound(machine) + operation.handleKeyNotFound(machine, gkey) } }, ) diff --git a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SResult.scala b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SResult.scala index be64844987c..8a55912aa5c 100644 --- a/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SResult.scala +++ b/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SResult.scala @@ -33,11 +33,10 @@ object SResult { contractId: ContractId, templateId: TypeConName, committers: Set[Party], - // Callback to signal that the contract was not present - // or visible. Returns true if this was recoverable. - // TODO (MK) Drop now that tryHandleSubmitMustFail is dead. - cbMissing: Unit => Boolean, - cbPresent: ContractInst[Value.VersionedValue[ContractId]] => Unit, + // Callback + // returns the next expression to evaluate. + // In case of failure the call back does not throw but returns a SErrorDamlException + callback: ContractInst[Value.VersionedValue[ContractId]] => Unit, ) extends SResult /** Machine needs a definition that was not present when the machine was @@ -73,14 +72,17 @@ object SResult { key: GlobalKeyWithMaintainers, committers: Set[Party], // Callback. - // returns true if machine can continue with the given result. - cb: Option[ContractId] => Boolean, + // In case of failure, the callback sets machine.ctrl to an SErrorDamlException and return false + callback: Option[ContractId] => Boolean, ) extends SResult final case class SResultNeedLocalKeyVisible( stakeholders: Set[Party], committers: Set[Party], - cb: SVisibleByKey => Unit, + // Callback. + // returns the next expression to evaluate. + // In case of failure the call back does not throw but set machine.ctrl to an SErrorDamlException + callback: SVisibleByKey => Unit, ) extends SResult sealed abstract class SVisibleByKey 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 index 5f51dc16b43..9d4570e6e54 100644 --- 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 @@ -1372,7 +1372,9 @@ private[lf] object Speedy { */ private[speedy] final case class SpeedyHungry(result: SResult) extends RuntimeException - with NoStackTrace + with NoStackTrace { + override def toString = s"SpeedyHungry($result)" + } private[speedy] def deriveTransactionSeed( submissionSeed: crypto.Hash, diff --git a/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/ScenarioRunner.scala b/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/ScenarioRunner.scala index 40f45b4374e..4022b85d941 100644 --- a/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/ScenarioRunner.scala +++ b/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/ScenarioRunner.scala @@ -219,14 +219,15 @@ object ScenarioRunner { // The interface we need from a ledger during submission. We allow abstracting over this so we can play // tricks like caching all responses in some benchmarks. - trait LedgerApi[R] { + abstract class LedgerApi[R] { def lookupContract( coid: ContractId, actAs: Set[Party], readAs: Set[Party], cbPresent: ContractInst[Value.VersionedValue[ContractId]] => Unit, - ): Either[SError, ContractInst[Value.VersionedValue[ContractId]]] + ): Either[SError, Unit] def lookupKey( + machine: Speedy.Machine, gk: GlobalKey, actAs: Set[Party], readAs: Set[Party], @@ -243,20 +244,21 @@ object ScenarioRunner { case class ScenarioLedgerApi(ledger: ScenarioLedger) extends LedgerApi[ScenarioLedger.CommitResult] { + override def lookupContract( acoid: ContractId, actAs: Set[Party], readAs: Set[Party], - cbPresent: ContractInst[Value.VersionedValue[ContractId]] => Unit, - ): Either[SError, ContractInst[Value.VersionedValue[ContractId]]] = - handleUnsafe(lookupContractUnsafe(acoid, actAs, readAs, cbPresent)) + callback: ContractInst[Value.VersionedValue[ContractId]] => Unit, + ): Either[SError, Unit] = + handleUnsafe(lookupContractUnsafe(acoid, actAs, readAs, callback)) private def lookupContractUnsafe( acoid: ContractId, actAs: Set[Party], readAs: Set[Party], - cbPresent: ContractInst[Value.VersionedValue[ContractId]] => Unit, - ): ContractInst[Value.VersionedValue[ContractId]] = { + callback: ContractInst[Value.VersionedValue[ContractId]] => Unit, + ) = { val effectiveAt = ledger.currentTime @@ -268,8 +270,7 @@ object ScenarioRunner { acoid, ) match { case ScenarioLedger.LookupOk(_, coinst, _) => - cbPresent(coinst) - coinst + callback(coinst) case ScenarioLedger.LookupContractNotFound(coid) => // This should never happen, hence we don't have a specific @@ -286,26 +287,33 @@ object ScenarioRunner { missingWith(ScenarioErrorContractNotVisible(coid, tid, actAs, readAs, observers)) } } + override def lookupKey( + machine: Speedy.Machine, gk: GlobalKey, actAs: Set[Party], readAs: Set[Party], - canContinue: Option[ContractId] => Boolean, + callback: Option[ContractId] => Boolean, ): Either[SError, Unit] = - handleUnsafe(lookupKeyUnsafe(gk, actAs, readAs, canContinue)) + handleUnsafe(lookupKeyUnsafe(machine: Speedy.Machine, gk, actAs, readAs, callback)) private def lookupKeyUnsafe( + machine: Speedy.Machine, gk: GlobalKey, actAs: Set[Party], readAs: Set[Party], - canContinue: Option[ContractId] => Boolean, + callback: Option[ContractId] => Boolean, ): Unit = { + val effectiveAt = ledger.currentTime val readers = actAs union readAs def missingWith(err: SError) = - if (!canContinue(None)) + if (!callback(None)) { + machine.returnValue = null + machine.ctrl = null throw SRunnerException(err) + } ledger.ledgerData.activeKeys.get(gk) match { case None => @@ -318,19 +326,21 @@ object ScenarioRunner { ) match { case ScenarioLedger.LookupOk(_, _, stakeholders) => if (!readers.intersect(stakeholders).isEmpty) - // We should always be able to continue with a Some(_). + // We should always be able to continue with a SKeyLookupResult.Found. // Run to get side effects and assert result. - assert(canContinue(Some(acoid))) + assert(callback(Some(acoid))) else throw SRunnerException( ScenarioErrorContractKeyNotVisible(acoid, gk, actAs, readAs, stakeholders) ) case ScenarioLedger.LookupContractNotFound(coid) => - missingWith(SErrorCrash(s"contract $coid not found, but we found its key!")) + missingWith(SErrorCrash(s"contract ${coid.coid} not found, but we found its key!")) case ScenarioLedger.LookupContractNotEffective(_, _, _) => - missingWith(SErrorCrash(s"contract $acoid not effective, but we found its key!")) + missingWith( + SErrorCrash(s"contract ${acoid.coid} not effective, but we found its key!") + ) case ScenarioLedger.LookupContractNotActive(_, _, _) => - missingWith(SErrorCrash(s"contract $acoid not active, but we found its key!")) + missingWith(SErrorCrash(s"contract ${acoid.coid} not active, but we found its key!")) case ScenarioLedger.LookupContractNotVisible( coid, tid @ _, @@ -405,13 +415,19 @@ object ScenarioRunner { } case SResultError(err) => SubmissionError(err, onLedger.ptxInternal) - case SResultNeedContract(coid, tid @ _, committers, _, cbPresent) => - ledger.lookupContract(coid, committers, readAs, cbPresent) match { + case SResultNeedContract(coid, tid @ _, committers, callback) => + ledger.lookupContract(coid, committers, readAs, callback) match { case Left(err) => SubmissionError(err, onLedger.ptxInternal) case Right(_) => go() } - case SResultNeedKey(keyWithMaintainers, committers, cb) => - ledger.lookupKey(keyWithMaintainers.globalKey, committers, readAs, cb) match { + case SResultNeedKey(keyWithMaintainers, committers, callback) => + ledger.lookupKey( + ledgerMachine, + keyWithMaintainers.globalKey, + committers, + readAs, + callback, + ) match { case Left(err) => SubmissionError(err, onLedger.ptxInternal) case Right(_) => go() } diff --git a/daml-lf/scenario-interpreter/src/perf/benches/scala/com/digitalasset/daml/lf/speedy/perf/CollectAuthority.scala b/daml-lf/scenario-interpreter/src/perf/benches/scala/com/digitalasset/daml/lf/speedy/perf/CollectAuthority.scala index 9b750f314da..e5eb0040f7b 100644 --- a/daml-lf/scenario-interpreter/src/perf/benches/scala/com/digitalasset/daml/lf/speedy/perf/CollectAuthority.scala +++ b/daml-lf/scenario-interpreter/src/perf/benches/scala/com/digitalasset/daml/lf/speedy/perf/CollectAuthority.scala @@ -96,7 +96,7 @@ class CollectAuthorityState { callback(value) case ScenarioRunner.SubmissionError(err, _) => crash(s"Submission failed $err") } - case SResultNeedContract(_, _, _, _, _) => + case SResultNeedContract(_, _, _, _) => crash("Off-ledger need contract callback") case SResultFinalValue(v) => finalValue = v case r => crash(s"bench run: unexpected result from speedy: ${r}") @@ -165,13 +165,15 @@ class CachedLedgerApi(initStep: Int, ledger: ScenarioLedger) coid: ContractId, actAs: Set[Party], readAs: Set[Party], - cbPresent: ContractInst[Value.VersionedValue[ContractId]] => Unit, - ): Either[SError.SError, ContractInst[Value.VersionedValue[ContractId]]] = { + callback: ContractInst[Value.VersionedValue[ContractId]] => Unit, + ): Either[SError.SError, Unit] = { step += 1 - super.lookupContract(coid, actAs, readAs, cbPresent).map { result => - cachedContract += step -> result - result - } + super.lookupContract( + coid, + actAs, + readAs, + { coinst => cachedContract += step -> coinst; callback(coinst) }, + ) } } @@ -184,18 +186,18 @@ class CannedLedgerApi( coid: ContractId, actAs: Set[Party], readAs: Set[Party], - cbPresent: ContractInst[Value.VersionedValue[ContractId]] => Unit, - ): Either[SError.SError, ContractInst[Value.VersionedValue[ContractId]]] = { + callback: ContractInst[Value.VersionedValue[ContractId]] => Unit, + ): Either[SError.SError, Unit] = { step += 1 val coinst = cachedContract(step) - cbPresent(coinst) - Right(coinst) + Right(callback(coinst)) } override def lookupKey( + machine: Machine, gk: GlobalKey, actAs: Set[Party], readAs: Set[Party], - canContinue: Option[ContractId] => Boolean, + callback: Option[ContractId] => Boolean, ) = throw new RuntimeException("Keys are not supported in the benchmark") override def currentTime = throw new RuntimeException("getTime is not supported in the benchmark")