Skip to content

Commit

Permalink
LF: check contracts are consumed when found them in the cache (#12527)
Browse files Browse the repository at this point in the history
fixes #11874

CHANGELOG_BEGIN
CHANGELOG_END
  • Loading branch information
remyhaemmerle-da authored Jan 25, 2022
1 parent ce06eb0 commit ebf7908
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 122 deletions.
29 changes: 5 additions & 24 deletions compiler/damlc/tests/daml-test-files/SemanticsEvalOrder.daml
Original file line number Diff line number Diff line change
Expand Up @@ -536,44 +536,25 @@ evUpdExercNonConsumErr = scenario do
abort "EvUpdExercNonConsumErr failed"


-- | Verify that the exercising party is evaluated
-- when the exercise is performed, not after.
-- @ERROR EvUpdExercWithoutActorsErr_1 OK
template T_EvUpdExercWithoutActorsErr_1
with
p : Party
where
signatory p
choice C_EvUpdExercWithoutActorsErr_1 : () with
controller (error @Party "EvUpdExercWithoutActorsErr_1 OK")
do abort "EvUpdExercWithoutActorsErr_1 failed (1)"

evUpdExercWithoutActorsErr_1 = scenario do
p <- getParty "Alice"
submit p do
c <- create (T_EvUpdExercWithoutActorsErr_1 p)
exercise c C_EvUpdExercWithoutActorsErr_1
abort "EvUpdExercWithoutActorsErr_1 failed (2)"

-- | Show that the exercising party is evaluated before
-- | Show that the exercising party is evaluated after
-- we check that the contract is still active.
-- @ERROR EvUpdExercWithoutActorsErr_2 OK
-- @ERROR Attempt to exercise a contract that was consumed in same transaction.
template T_EvUpdExercWithoutActorsErr_2
with
p : Party
where
signatory p
choice C_EvUpdExercWithoutActorsErr_2 : () with
controller (error @Party "EvUpdExercWithoutActorsErr_2 OK")
do abort "EvUpdExercWithoutActorsErr_2 failed (1)"
controller (error @Party "EvUpdExercWithoutActorsErr_2 failed (1)")
do abort "EvUpdExercWithoutActorsErr_2 failed (2)"

evUpdExercWithoutActorsErr_2 = scenario do
p <- getParty "Alice"
submit p do
c <- create (T_EvUpdExercWithoutActorsErr_2 p)
archive c
exercise c C_EvUpdExercWithoutActorsErr_2
abort "EvUpdExercWithoutActorsErr_2 failed (2)"
abort "EvUpdExercWithoutActorsErr_2 failed (3)"

-- | Checks that an authorization / bad actor check occurs
-- at some point during submission.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1032,13 +1032,12 @@ private[lf] object SBuiltin {
val coid = getSContractId(args, 0)
onLedger.cachedContracts.get(coid) match {
case Some(cached) =>
if (cached.templateId != templateId) {
machine.ctrl = SEDamlException(
IE.WronglyTypedContract(coid, templateId, cached.templateId)
)
} else {
machine.returnValue = cached.value
}
if (cached.templateId != templateId)
throw SErrorDamlException(IE.WronglyTypedContract(coid, templateId, cached.templateId))
onLedger.ptx.consumedBy
.get(coid)
.foreach(nid => throw SErrorDamlException(IE.ContractNotActive(coid, templateId, nid)))
machine.returnValue = cached.value
case None =>
throw SpeedyHungry(
SResultNeedContract(
Expand Down Expand Up @@ -1875,8 +1874,6 @@ private[lf] object SBuiltin {
ptx.aborted match {
case Some(Tx.AuthFailureDuringExecution(nid, fa)) =>
throw SErrorDamlException(IE.FailedAuthorization(nid, fa))
case Some(Tx.ContractNotActive(coid, tid, consumedBy)) =>
throw SErrorDamlException(IE.ContractNotActive(coid, tid, consumedBy))
case Some(Tx.DuplicateContractKey(key)) =>
throw SErrorDamlException(IE.DuplicateContractKey(key))
case None =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,8 @@ private[speedy] case class PartialTransaction(
version,
)
mustBeActive(
NameOf.qualifiedNameOfCurrentFunc,
coid,
templateId,
insertLeafNode(node, version, optLocation),
).noteAuthFails(nid, CheckAuthorization.authorizeFetch(optLocation, node), auth)
}
Expand Down Expand Up @@ -582,8 +582,8 @@ private[speedy] case class PartialTransaction(
)

mustBeActive(
NameOf.qualifiedNameOfCurrentFunc,
targetId,
templateId,
copy(
actionNodeLocations = actionNodeLocations :+ optLocation,
nextNodeIdx = nextNodeIdx + 1,
Expand Down Expand Up @@ -762,18 +762,18 @@ private[speedy] case class PartialTransaction(
/** `True` iff the given `ContractId` has been consumed already */
def isConsumed(coid: Value.ContractId): Boolean = consumedBy.contains(coid)

/** Guard the execution of a step with the unconsumedness of a
* `ContractId`
/** Double check the execution of a step with the unconsumedness of a
* `ContractId`.
*/
private[this] def mustBeActive(
loc: => String,
coid: Value.ContractId,
templateId: TypeConName,
f: => PartialTransaction,
): PartialTransaction =
consumedBy.get(coid) match {
case None => f
case Some(nid) => noteAbort(Tx.ContractNotActive(coid, templateId, nid))
}
if (consumedBy.isDefinedAt(coid))
InternalError.runtimeException(loc, "try to build a node using an inactive contract.")
else
f

/** Insert the given `LeafNode` under a fresh node-id, and return it */
def insertLeafNode(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -613,9 +613,7 @@ class EvaluationOrderTest extends AnyFreeSpec with Matchers with Inside {
getContract = getContract,
)
inside(res) { case Success(Left(SErrorDamlException(IE.ContractNotActive(_, T, _)))) =>
// TODO https://github.com/digital-asset/daml/issues/11874
// choice controlles and observer should not be evaluated
msgs shouldBe Seq("starts test", "choice controllers", "choice observers")
msgs shouldBe Seq("starts test")
}
}

Expand Down Expand Up @@ -703,13 +701,7 @@ class EvaluationOrderTest extends AnyFreeSpec with Matchers with Inside {
Set(alice),
)
inside(res) { case Success(Left(SErrorDamlException(IE.ContractNotActive(_, T, _)))) =>
// TODO https://github.com/digital-asset/daml/issues/11874
// choice controlles and observer should not be evaluated
msgs shouldBe Seq(
"starts test",
"choice controllers",
"choice observers",
)
msgs shouldBe Seq("starts test")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -839,15 +839,6 @@ object Transaction {
/** Errors that can happen during building transactions. */
sealed abstract class TransactionError extends Product with Serializable

/** Signals that the contract-id `coid` was expected to be active, but
* is not.
*/
final case class ContractNotActive(
coid: Value.ContractId,
templateId: TypeConName,
consumedBy: NodeId,
) extends TransactionError

/** Signals that within the transaction we got to a point where
* two contracts with the same key were active.
*
Expand Down
Loading

0 comments on commit ebf7908

Please sign in to comment.