Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Errors] polish evaluation errors #6043

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ instance (Eq fun, Hashable fun, ToExMemory term) =>
Restricting resb ->
when (exceedsBudget resb newBudget) $
throwingWithCause _EvaluationError
(UserEvaluationError $ CekOutOfExError resb newBudget)
(OperationalEvaluationError $ CekOutOfExError resb newBudget)
Nothing -- No value available for error
```

Expand All @@ -96,7 +96,7 @@ to the current mode:
newBudget <- exBudgetStateBudget <%= (<> budget)
when (exceedsBudget resb newBudget) $
throwingWithCause _EvaluationError
(UserEvaluationError $ CekOutOfExError resb newBudget)
(OperationalEvaluationError $ CekOutOfExError resb newBudget)
Nothing
```

Expand All @@ -114,7 +114,7 @@ of memory very quickly. Changing the code to
Restricting resb ->
when (exceedsBudget resb newBudget) $
throwingWithCause _EvaluationError
(UserEvaluationError $ CekOutOfExError resb newBudget)
(OperationalEvaluationError $ CekOutOfExError resb newBudget)
Nothing
```

Expand Down
2 changes: 1 addition & 1 deletion plutus-benchmark/common/PlutusBenchmark/Common.hs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ haskellValueToTerm = compiledCodeToTerm . Tx.liftCodeDef
{- | Just run a term to obtain an `EvaluationResult` (used for tests etc.) -}
unsafeRunTermCek :: Term -> EvaluationResult Term
unsafeRunTermCek =
unsafeExtractEvaluationResult
unsafeToEvaluationResult
. (\(res, _, _) -> res)
. runCekDeBruijn PLC.defaultCekParameters Cek.restrictingEnormous Cek.noEmitter

Expand Down
2 changes: 1 addition & 1 deletion plutus-benchmark/nofib/exe/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ options = hsubparser
---------------- Evaluation ----------------

evaluateWithCek :: UPLC.Term UPLC.NamedDeBruijn DefaultUni DefaultFun () -> UPLC.EvaluationResult (UPLC.Term UPLC.NamedDeBruijn DefaultUni DefaultFun ())
evaluateWithCek = UPLC.unsafeExtractEvaluationResult . (\(fstT,_,_) -> fstT) . UPLC.runCekDeBruijn PLC.defaultCekParameters UPLC.restrictingEnormous UPLC.noEmitter
evaluateWithCek = UPLC.unsafeToEvaluationResult . (\(fstT,_,_) -> fstT) . UPLC.runCekDeBruijn PLC.defaultCekParameters UPLC.restrictingEnormous UPLC.noEmitter

writeFlatNamed :: UPLC.Program UPLC.NamedDeBruijn DefaultUni DefaultFun () -> IO ()
writeFlatNamed prog = BS.putStr . Flat.flat . UPLC.UnrestrictedProgram $ prog
Expand Down
2 changes: 1 addition & 1 deletion plutus-benchmark/script-contexts/test/Spec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import Test.Tasty
import Test.Tasty.Extras (TestNested, runTestGroupNestedGhc)
import Test.Tasty.HUnit

import PlutusBenchmark.Common (Term, compiledCodeToTerm, runTermCek, unsafeRunTermCek)
import PlutusBenchmark.Common (Term, compiledCodeToTerm, runTermCek)
import PlutusBenchmark.ScriptContexts

import PlutusCore.Evaluation.Result
Expand Down
12 changes: 5 additions & 7 deletions plutus-core/cost-model/budgeting-bench/Common.hs
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,11 @@ benchWith
-> String
-> PlainTerm DefaultUni fun
-> Benchmark
benchWith params name term = bench name $ whnf (unsafeEvaluateCekNoEmit params) term
{- ^ Note that to get sensible results with whnf, we must use an evaluation
function that looks at the result, so eg unsafeEvaluateCek won't work
properly because it returns a pair whose components won't be evaluated by
whnf. We can't use nf because it does too much work: for instance if it gets
back a 'Data' value it'll traverse all of it.
-}
-- Note that to get sensible results with 'whnf', we must use an evaluation function that looks at
-- the result, so e.g. 'evaluateCek' won't work properly because it returns a pair whose components
-- won't be evaluated by 'whnf'. We can't use 'nf' because it does too much work: for instance if it
-- gets back a 'Data' value it'll traverse all of it.
benchWith params name term = bench name $ whnf (evaluateCekNoEmit params) term

benchDefault :: String -> PlainTerm DefaultUni DefaultFun -> Benchmark
benchDefault = benchWith defaultCekParameters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,14 @@ instance (Pretty err, Pretty cause) => Pretty (ErrorWithCause err cause) where

instance (PrettyBy config cause, PrettyBy config err) =>
PrettyBy config (ErrorWithCause err cause) where
prettyBy config (ErrorWithCause err mayCause) =
"An error has occurred: " <+> prettyBy config err <>
case mayCause of
Nothing -> mempty
Just cause -> hardline <> "Caused by:" <+> prettyBy config cause
prettyBy config (ErrorWithCause err mayCause) = fold
[ "An error has occurred:"
, hardline
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just to preserve the old behavior, I'm not really sure why we're using hardline in error messages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will hardline be added twice in case of Just cause ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but there will be pretty-printer err in-between, you can see how it gets rendered at the ~bottom of the diff. Anyways, I'm just preserving the old behavior newlines-wise.

, prettyBy config err
, case mayCause of
Nothing -> mempty
Just cause -> hardline <> "Caused by:" <+> prettyBy config cause
]

instance (PrettyPlc cause, PrettyPlc err) =>
Show (ErrorWithCause err cause) where
Expand Down
23 changes: 3 additions & 20 deletions plutus-core/plutus-core/src/PlutusCore/Evaluation/Machine/Ck.hs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@ module PlutusCore.Evaluation.Machine.Ck
, CkEvaluationException
, CkM
, CkValue
, extractEvaluationResult
, runCk
, extractEvaluationResult
, unsafeToEvaluationResult
, evaluateCk
, evaluateCkNoEmit
, unsafeEvaluateCk
, unsafeEvaluateCkNoEmit
, readKnownCk
) where

Expand Down Expand Up @@ -195,7 +194,7 @@ stack |> Constr _ ty i es = case es of
t : ts -> FrameConstr ty i ts [] : stack |> t
stack |> Case _ _ arg cs = FrameCase cs : stack |> arg
_ |> Error{} =
throwingWithCause _EvaluationError (UserEvaluationError CkEvaluationFailure) Nothing
throwingWithCause _EvaluationError (OperationalEvaluationError CkEvaluationFailure) Nothing
_ |> var@Var{} =
throwingWithCause _MachineError OpenTermEvaluatedMachineError $ Just var

Expand Down Expand Up @@ -312,22 +311,6 @@ evaluateCkNoEmit
-> Either (CkEvaluationException uni fun) (Term TyName Name uni fun ())
evaluateCkNoEmit runtime = fst . runCk runtime False

-- | Evaluate a term using the CK machine with logging enabled. May throw a 'CkEvaluationException'.
unsafeEvaluateCk
:: ThrowableBuiltins uni fun
=> BuiltinsRuntime fun (CkValue uni fun)
-> Term TyName Name uni fun ()
-> (EvaluationResult (Term TyName Name uni fun ()), [Text])
unsafeEvaluateCk runtime = first unsafeExtractEvaluationResult . evaluateCk runtime

-- | Evaluate a term using the CK machine with logging disabled. May throw a 'CkEvaluationException'.
unsafeEvaluateCkNoEmit
:: ThrowableBuiltins uni fun
=> BuiltinsRuntime fun (CkValue uni fun)
-> Term TyName Name uni fun ()
-> EvaluationResult (Term TyName Name uni fun ())
unsafeEvaluateCkNoEmit runtime = unsafeExtractEvaluationResult . evaluateCkNoEmit runtime

-- | Unlift a value using the CK machine.
readKnownCk
:: ReadKnown (Term TyName Name uni fun ()) a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module PlutusCore.Evaluation.Machine.Exception
, throwing_
, throwingWithCause
, extractEvaluationResult
, unsafeExtractEvaluationResult
, unsafeToEvaluationResult
) where

import PlutusPrelude
Expand Down Expand Up @@ -66,13 +66,10 @@ data MachineError fun
deriving stock (Show, Eq, Functor, Generic)
deriving anyclass (NFData)

-- | The type of errors (all of them) which can occur during evaluation
-- (some are used-caused, some are internal).
data EvaluationError user internal
= InternalEvaluationError !internal
-- ^ Indicates bugs.
| UserEvaluationError !user
-- ^ Indicates user errors.
-- | The type of errors (all of them) which can occur during evaluation. TODO: explain
effectfully marked this conversation as resolved.
Show resolved Hide resolved
data EvaluationError operational structural
= OperationalEvaluationError !operational
| StructuralEvaluationError !structural
deriving stock (Show, Eq, Functor, Generic)
deriving anyclass (NFData)

Expand All @@ -81,47 +78,50 @@ mtraverse makeClassyPrisms
, ''EvaluationError
]

instance internal ~ MachineError fun => AsMachineError (EvaluationError user internal) fun where
_MachineError = _InternalEvaluationError
instance AsUnliftingError internal => AsUnliftingError (EvaluationError user internal) where
_UnliftingError = _InternalEvaluationError . _UnliftingError
instance structural ~ MachineError fun =>
AsMachineError (EvaluationError operational structural) fun where
_MachineError = _StructuralEvaluationError
instance AsUnliftingError structural =>
AsUnliftingError (EvaluationError operational structural) where
_UnliftingError = _StructuralEvaluationError . _UnliftingError
instance AsUnliftingError (MachineError fun) where
_UnliftingError = _UnliftingMachineError
instance AsEvaluationFailure user => AsEvaluationFailure (EvaluationError user internal) where
_EvaluationFailure = _UserEvaluationError . _EvaluationFailure
instance AsEvaluationFailure operational =>
AsEvaluationFailure (EvaluationError operational structural) where
_EvaluationFailure = _OperationalEvaluationError . _EvaluationFailure

type EvaluationException user internal =
ErrorWithCause (EvaluationError user internal)
type EvaluationException operational structural =
ErrorWithCause (EvaluationError operational structural)

{- Note [Ignoring context in UserEvaluationError]
The UserEvaluationError error has a term argument, but
{- Note [Ignoring context in OperationalEvaluationError]
The OperationalEvaluationError error has a term argument, but
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get what is happening here, but what I don't understand is why we do all this effort to distinquish between operational vs structural errors at actual script validation, by using
Either (ErrorWithCause structural term) (EvaluationResult a) .

From the user's point of view, validating a script would either EvaluationSuccess (validate) or EvaluationFailure (not validate).

In what way does it matter if the non-validation was caused by operational or structural error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's explained in the Haddocks:

On the chain both of these are just regular failures and we don't distinguish between them there:
if a script fails, it fails, it doesn't matter what the reason was. However in the tests it does
matter why the failure occurred: a structural error may indicate that the test was written
incorrectly while an operational error may be entirely expected.

Basically, it's an extra level of safety for us. unsafeToEvaluationResult is used in quite a lot of places and some tests fail if you remove the distinction between operational and structural errors, in the same way that some tests will start to fail if you get typing wrong, even if operationally nothing changes. Just ensures that any evaluation failures are the expected operational ones, not somebody messing up the test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is an artifact of Tests alone, then perhaps EvaluationResult should belong to a testlib.

Because taking into account other Plutus-based languages (Aiken,Plutarch,etc) there may not be a distinction between structural vs operational errors and instead they have just errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is an artifact of Tests alone, then perhaps EvaluationResult should belong to a testlib.

Maybe. I'm currently removing it from builtins where we have a much more appropriate BuiltinResult. I doubt I'm going to get motivated enough to see it through though.

Because taking into account other Plutus-based languages (Aiken,Plutarch,etc) there may not be a distinction between structural vs operational errors and instead they have just errors?

Perhaps, but I don't really care, I just want us to have that distinction for the same reasons I want us to have a type checker instead of juggling raw Data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt I'm going to get motivated enough to see it through though.

I mean, removing EvaluationResult from all non-test code, not replace it with BuiltinResult in the builtins code -- that ship has sailed.

extractEvaluationResult just discards this and returns
EvaluationFailure. This means that, for example, if we use the `plc`
command to execute a program containing a division by zero, plc exits
silently without reporting that anything has gone wrong (but returning
a non-zero exit code to the shell via `exitFailure`). This is because
UserEvaluationError is used in cases when a PLC program itself goes
OperationalEvaluationError is used in cases when a PLC program itself goes
wrong (for example, a failure due to `(error)`, a failure during
builtin evaluation, or exceeding the gas limit). This is used to
signal unsuccessful in validation and so is not regarded as a real
error; in contrast, machine errors, typechecking failures,
effectfully marked this conversation as resolved.
Show resolved Hide resolved
and so on are genuine errors and we report their context if available.
-}

-- | Turn any 'UserEvaluationError' into an 'EvaluationFailure'.
-- | Turn any 'OperationalEvaluationError' into an 'EvaluationFailure'.
extractEvaluationResult
:: Either (EvaluationException user internal term) a
-> Either (ErrorWithCause internal term) (EvaluationResult a)
:: Either (EvaluationException operational structural term) a
-> Either (ErrorWithCause structural term) (EvaluationResult a)
extractEvaluationResult (Right term) = Right $ EvaluationSuccess term
extractEvaluationResult (Left (ErrorWithCause evalErr cause)) = case evalErr of
InternalEvaluationError err -> Left $ ErrorWithCause err cause
UserEvaluationError _ -> Right $ EvaluationFailure
StructuralEvaluationError err -> Left $ ErrorWithCause err cause
OperationalEvaluationError _ -> Right $ EvaluationFailure

unsafeExtractEvaluationResult
unsafeToEvaluationResult
:: (PrettyPlc internal, PrettyPlc term, Typeable internal, Typeable term)
=> Either (EvaluationException user internal term) a
-> EvaluationResult a
unsafeExtractEvaluationResult = unsafeFromEither . extractEvaluationResult
unsafeToEvaluationResult = unsafeFromEither . extractEvaluationResult

instance (HasPrettyDefaults config ~ 'True, Pretty fun) =>
PrettyBy config (MachineError fun) where
Expand All @@ -148,13 +148,7 @@ instance (HasPrettyDefaults config ~ 'True, Pretty fun) =>

instance
( HasPrettyDefaults config ~ 'True
, PrettyBy config internal, Pretty user
) => PrettyBy config (EvaluationError user internal) where
prettyBy config (InternalEvaluationError err) = fold
[ "error:", hardline
, prettyBy config err
]
prettyBy _ (UserEvaluationError err) = fold
[ "User error:", hardline
, pretty err
]
, Pretty operational, PrettyBy config structural
) => PrettyBy config (EvaluationError operational structural) where
prettyBy _ (OperationalEvaluationError operational) = pretty operational
prettyBy config (StructuralEvaluationError structural) = prettyBy config structural
Comment on lines +171 to +172
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And indeed here you show that it does not matter if the error was operational or structural :P

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It matters when extractEvaluationResult or unsafeToEvaluationResult are called and they do get called. I'm not sure if reporting whether the error was operational or structural would add meaningful context or just confuse the reader who may not be familiar with the distinction. I do want us to be familiar with the distinction.

4 changes: 3 additions & 1 deletion plutus-core/plutus-core/test/Evaluation/Machines.hs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ testMachine
:: (uni ~ DefaultUni, fun ~ DefaultFun, PrettyPlc internal)
=> String
-> (Term TyName Name uni fun () ->
Either (EvaluationException user internal (Term TyName Name uni fun ())) (Term TyName Name uni fun ()))
Either
(EvaluationException operational structural (Term TyName Name uni fun ()))
(Term TyName Name uni fun ()))
-> TestTree
testMachine machine eval =
testGroup machine $ fromInterestingTermGens $ \name ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ import PlutusIR.Compiler qualified as PIR
import PlutusIR.Core qualified as PIR
import PlutusIR.Parser (pTerm)
import UntypedPlutusCore.Core qualified as UPLC
import UntypedPlutusCore.Evaluation.Machine.Cek (CekValue, EvaluationResult (..), logEmitter,
unsafeEvaluateCek)
import UntypedPlutusCore.Evaluation.Machine.Cek (CekValue, EvaluationResult (..), evaluateCek,
logEmitter, unsafeToEvaluationResult)
import UntypedPlutusCore.Evaluation.Machine.Cek.CekMachineCosts (CekMachineCosts)

pirTermFromFile
Expand Down Expand Up @@ -66,7 +66,7 @@ compilePirProgramOrFail pirProgram = do
& runExceptT
>>= \case
Left (er :: PIR.Error DefaultUni DefaultFun (Provenance ())) -> fail $ show er
Right p -> pure (void p)
Right p -> pure (void p)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like stylish-haskell conflicts with fourmolu, I promise I didn't add those spaces manually.


compileTplcProgramOrFail
:: (MonadFail m)
Expand All @@ -83,7 +83,7 @@ evaluateUplcProgramWithTraces
:: UPLC.Program Name DefaultUni DefaultFun ()
-> (EvaluationResult (UPLC.Term Name DefaultUni DefaultFun ()), [Text])
evaluateUplcProgramWithTraces uplcProg =
unsafeEvaluateCek logEmitter machineParameters (uplcProg ^. UPLC.progTerm)
unsafeToEvaluationResult $ evaluateCek logEmitter machineParameters (uplcProg ^. UPLC.progTerm)
where
costModel :: CostModel CekMachineCosts BuiltinCostModel =
CostModel defaultCekMachineCosts defaultBuiltinCostModel
Expand All @@ -102,5 +102,5 @@ defaultCompilationCtx = do
handlePirErrorByFailing
:: (Pretty ann, MonadFail m) => Either (PIR.Error DefaultUni DefaultFun ann) a -> m a
handlePirErrorByFailing = \case
Left e -> fail $ show e
Left e -> fail $ show e
Right x -> pure x
4 changes: 2 additions & 2 deletions plutus-core/testlib/PlutusCore/Generators/Hedgehog/Test.hs
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ sampleProgramValueGolden folder name genTerm = do
propEvaluate
:: ( uni ~ DefaultUni, fun ~ DefaultFun
, KnownTypeAst TyName uni a, MakeKnown (Term TyName Name uni fun ()) a
, PrettyPlc internal
, PrettyPlc structural
)
=> (Term TyName Name uni fun () ->
Either
(EvaluationException user internal (Term TyName Name uni fun ()))
(EvaluationException operational structural (Term TyName Name uni fun ()))
(Term TyName Name uni fun ()))
-- ^ An evaluator.
-> TermGen a -- ^ A term/value generator.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,11 @@ type TypeEvalCheckM uni fun = Either (TypeEvalCheckError uni fun)
typeEvalCheckBy
:: ( uni ~ DefaultUni, fun ~ DefaultFun
, KnownTypeAst TyName uni a, MakeKnown (Term TyName Name uni fun ()) a
, PrettyPlc internal
, PrettyPlc structural
)
=> (Term TyName Name uni fun () ->
Either
(EvaluationException user internal (Term TyName Name uni fun ()))
(EvaluationException operational structural (Term TyName Name uni fun ()))
(Term TyName Name uni fun ()))
-- ^ An evaluator.
-> TermOf (Term TyName Name uni fun ()) a
Expand Down
16 changes: 8 additions & 8 deletions plutus-core/testlib/PlutusCore/Generators/NEAT/Spec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -111,21 +111,21 @@ not exploited.

-- handle a user error and turn it back into an error term
handleError :: Type TyName DefaultUni ()
-> U.ErrorWithCause (U.EvaluationError user internal) term
-> Either (U.ErrorWithCause (U.EvaluationError user internal) term)
-> U.ErrorWithCause (U.EvaluationError operational structural) term
-> Either (U.ErrorWithCause (U.EvaluationError operational structural) term)
(Term TyName Name DefaultUni DefaultFun ())
handleError ty e = case U._ewcError e of
U.UserEvaluationError _ -> return (Error () ty)
U.InternalEvaluationError _ -> throwError e
U.OperationalEvaluationError _ -> return (Error () ty)
U.StructuralEvaluationError _ -> throwError e

-- untyped version of `handleError`
handleUError ::
U.ErrorWithCause (U.EvaluationError user internal) term
-> Either (U.ErrorWithCause (U.EvaluationError user internal) term)
U.ErrorWithCause (U.EvaluationError operational structural) term
-> Either (U.ErrorWithCause (U.EvaluationError operational structural) term)
(U.Term Name DefaultUni DefaultFun ())
handleUError e = case U._ewcError e of
U.UserEvaluationError _ -> return (U.Error ())
U.InternalEvaluationError _ -> throwError e
U.OperationalEvaluationError _ -> return (U.Error ())
U.StructuralEvaluationError _ -> throwError e

-- |Property: check if the type is preserved by evaluation.
--
Expand Down
Loading
Loading