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

Core: ADT-ize all errors #139

Merged
merged 1 commit into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion crypto/Pact/Core/Crypto/Pairing.hs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,6 @@ data CurvePoint a
| CurveInf
deriving (Eq, Show)

-- todo: sort of scuffed, num instance?
double :: (Field a, Num a, Eq a) => CurvePoint a -> CurvePoint a
double CurveInf = CurveInf
double (Point x y)
Expand Down
2 changes: 1 addition & 1 deletion gasmodel/Pact/Core/GasModel/InterpreterGas.hs
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ gasMtWithHandlerError pdb =
let es = defaultGasEvalState
ps = _eeDefPactStep ee
frame = Mt
value = VError "foo" ()
value = VError [] (UserEnforceError "foo") ()
env = CEKEnv { _cePactDb=pdb
, _ceLocal=mempty
, _ceInCap=False
Expand Down
2 changes: 1 addition & 1 deletion pact-lsp/Pact/Core/LanguageServer.hs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ sendDiagnostics nuri mv content = liftIO runPact >>= \case
PEParseError{} -> "Parse"
PEDesugarError{} -> "Desugar"
PEExecutionError{} -> "Execution"
PERecoverableError{} -> "Execution"
PEUserRecoverableError{} -> "Execution"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be the same as Execution?

Copy link
Member Author

Choose a reason for hiding this comment

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

Great question, @rsoeldner does it matter? I assumed so, it's essentially an error that happens during execution.


spanInfoToRange :: SpanInfo -> Range
spanInfoToRange (SpanInfo sl sc el ec) = mkRange
Expand Down
2 changes: 1 addition & 1 deletion pact-tests/Pact/Core/Test/GasGolden.hs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ tests = do
cases <- gasTestFiles
pure $ testGroup "Gas Goldens"
[ testCase "Capture all builtins" $ captureBuiltins (fst <$> cases)
, goldenVsStringDiff "Gas Goldens: CEK" runDiff (gasTestDir </> "builtinGas.golden") (gasGoldenTests cases interpretReplProgram)
, goldenVsStringDiff "Gas Goldens: CEK" runDiff (gasTestDir </> "builtinGas.golden") (gasGoldenTests cases interpretReplProgramBigStep)
, goldenVsStringDiff "Gas Goldens: CEK smallstep" runDiff (gasTestDir </> "builtinGas.golden") (gasGoldenTests cases interpretReplProgramSmallStep)
, goldenVsStringDiff "Gas Goldens: Direct" runDiff (gasTestDir </> "builtinGas.golden") (gasGoldenTests cases interpretReplProgramDirect)
]
Expand Down
2 changes: 1 addition & 1 deletion pact-tests/Pact/Core/Test/LegacySerialiseTests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ legacyTests = do
Nothing -> error "Reading existing modules failed"
Just ms -> do
modTests <- fmap concat $ forM repl $ \r -> do
sequence [runTest r interpretReplProgram "CEK", runTest r interpretReplProgramDirect "Direct"]
sequence [runTest r interpretReplProgramBigStep "CEK", runTest r interpretReplProgramDirect "Direct"]
pure (testGroup p modTests)
where
runTest r interpreter interpName = do
Expand Down
2 changes: 1 addition & 1 deletion pact-tests/Pact/Core/Test/PersistenceTests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ sqliteRegression =
evalLog <- newIORef Nothing
ee <- defaultEvalEnv pdb replCoreBuiltinMap
ref <- newIORef (ReplState mempty pdb def ee evalLog (SourceCode "" "") mempty mempty Nothing False)
Right _ <- runReplT ref (interpretReplProgram (SourceCode "test" src) (const (pure ())))
Right _ <- runReplT ref (interpretReplProgramBigStep (SourceCode "test" src) (const (pure ())))
Just md <- readModule pdb (ModuleName "test" Nothing)
pure md

4 changes: 2 additions & 2 deletions pact-tests/Pact/Core/Test/ReplTests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ tests :: IO TestTree
tests = do
files <- replTestFiles
pure $ testGroup "ReplTests"
[ testGroup "in-memory db:bigstep" (runFileReplTest interpretReplProgram <$> files)
, testGroup "sqlite db:bigstep" (runFileReplTestSqlite interpretReplProgram <$> files)
[ testGroup "in-memory db:bigstep" (runFileReplTest interpretReplProgramBigStep <$> files)
, testGroup "sqlite db:bigstep" (runFileReplTestSqlite interpretReplProgramBigStep <$> files)
, testGroup "in-memory db:smallstep" (runFileReplTest interpretReplProgramSmallStep <$> files)
, testGroup "sqlite db:smallstep" (runFileReplTestSqlite interpretReplProgramSmallStep <$> files)
, testGroup "in-memory db:direct" (runFileReplTest interpretReplProgramDirect <$> files)
Expand Down
39 changes: 23 additions & 16 deletions pact-tests/Pact/Core/Test/StaticErrorTests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,19 @@ import Pact.Core.Serialise (serialisePact_repl_spaninfo)
import Pact.Core.Test.TestPrisms

isParseError :: Prism' ParseError a -> PactErrorI -> Bool
isParseError p s = isJust $ preview (_PEParseError . _1 . p) s
isParseError p s = has (_PEParseError . _1 . p) s

isDesugarError :: Prism' DesugarError a -> PactErrorI -> Bool
isDesugarError p s = isJust $ preview (_PEDesugarError . _1 . p) s
isDesugarError p s = has (_PEDesugarError . _1 . p) s

isExecutionError :: Prism' EvalError a -> PactErrorI -> Bool
isExecutionError p s = isJust $ preview (_PEExecutionError . _1 . p) s
isExecutionError p s = has (_PEExecutionError . _1 . p) s

runStaticTest :: String -> Text -> (PactErrorI -> Bool) -> Assertion
runStaticTest label src predicate = do
isUserRecoverableError :: Prism' UserRecoverableError a -> PactErrorI -> Bool
isUserRecoverableError p s = has (_PEUserRecoverableError . _1 . p) s

runStaticTest :: String -> Text -> ReplInterpreter -> (PactErrorI -> Bool) -> Assertion
runStaticTest label src interp predicate = do
gasLog <- newIORef Nothing
pdb <- mockPactDb serialisePact_repl_spaninfo
ee <- defaultEvalEnv pdb replCoreBuiltinMap
Expand All @@ -51,7 +54,7 @@ runStaticTest label src predicate = do
, _replNativesEnabled = True
}
stateRef <- newIORef rstate
v <- runReplT stateRef (interpretReplProgram source (const (pure ())))
v <- runReplT stateRef (interpretReplProgram interp source (const (pure ())))
case v of
Left err ->
assertBool ("Expected Error to match predicate, but got " <> show err <> " instead") (predicate err)
Expand Down Expand Up @@ -573,7 +576,7 @@ executionTests =
(defun foo:string ())
)
|])
, ("enforce_ns_define_namespace", isExecutionError _DefineNamespaceError, [text|
, ("enforce_ns_define_namespace", isExecutionError _NativeExecutionError, [text|
0xd34df00d marked this conversation as resolved.
Show resolved Hide resolved
(module m g (defcap g () true)
(defun manage (ns guard) false)
)
Expand Down Expand Up @@ -627,7 +630,7 @@ executionTests =
, ("get_module_unknown", isExecutionError _ModuleDoesNotExist, [text|
(describe-module 'nonexistent)
|])
, ("reexposed_module_missing_name", isExecutionError _NameNotInScope, [text|
, ("reexposed_module_missing_name", isExecutionError _ModuleMemberDoesNotExist, [text|
jmcardon marked this conversation as resolved.
Show resolved Hide resolved
(module m g
(defcap g () true)

Expand All @@ -650,15 +653,15 @@ executionTests =
, ("module_gov_keyset_nonexistent", isExecutionError _NoSuchKeySet, [text|
(module m 'nonexistent (defun f () true))
|])
, ("module_gov_keyset_different", isExecutionError _EvalError, [text|
, ("module_gov_keyset_different", isUserRecoverableError _KeysetPredicateFailure, [text|
(env-data {"ks":["jose"]})
(define-keyset 'somekeyset (read-keyset 'ks))
(module m 'somekeyset (defun f () 1))
|])
, ("module_gov_keyset_empty", isExecutionError _ModuleGovernanceFailure, [text|
, ("module_gov_keyset_empty", isExecutionError _InvalidKeysetNameFormat, [text|
jmcardon marked this conversation as resolved.
Show resolved Hide resolved
(module m "" (defun f () true))
|])
, ("module_gov_keyset_not_in_sigs", isExecutionError _EvalError, [text|
, ("module_gov_keyset_not_in_sigs", isUserRecoverableError _KeysetPredicateFailure, [text|
(env-data { "kall": ["a" "b" "c"], "kadmin": ["admin"] })
(define-keyset 'kall)
(define-keyset 'kadmin)
Expand All @@ -678,7 +681,7 @@ executionTests =
|])

-- CEK errors
, ("modref_no_ns", isExecutionError _ModRefNotRefined, [text|
, ("modref_no_ns", isExecutionError _ModRefImplementsNoInterfaces, [text|
(module m g (defcap g () true))
m
|])
Expand Down Expand Up @@ -1080,8 +1083,8 @@ builtinTests =
, ("at_oob_bound", isExecutionError _ArrayOutOfBoundsException, "(at 3 [1 2 3])")
, ("at_oob_smaller", isExecutionError _ArrayOutOfBoundsException, "(at -1 [1 2 3])")
, ("at_oob_empty", isExecutionError _ArrayOutOfBoundsException, "(at 0 [])")
, ("at_key_missing", isExecutionError _EvalError, "(at 'bar { 'foo: 1 })")
, ("yield_outside", isExecutionError _YieldOutsiteDefPact, "(yield {})")
, ("at_key_missing", isExecutionError _ObjectIsMissingField, "(at 'bar { 'foo: 1 })")
, ("yield_outside", isExecutionError _YieldOutsideDefPact, "(yield {})")
, ("resume_no_defpact", isExecutionError _NoActiveDefPactExec, "(resume { 'field := binder } binder)")
, ("resume_no_yield", isExecutionError _NoYieldInDefPactStep, [text|
(module m g (defcap g () true)
Expand Down Expand Up @@ -1203,6 +1206,10 @@ builtinTests =

tests :: TestTree
tests =
testGroup "CoreStaticTests" (go <$> parseTests <> desugarTests <> executionTests <> builtinTests)
testGroup "CoreStaticTests"
[ testGroup "CoreStaticTests:CEK" (go interpretEvalBigStep <$> allTests)
, testGroup "CoreStaticTests:Direct" (go interpretEvalDirect <$> allTests)
Comment on lines +1210 to +1211
Copy link
Contributor

Choose a reason for hiding this comment

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

Yay that code reuse!

]
where
go (label, p, srcText) = testCase label $ runStaticTest label srcText p
allTests = parseTests <> desugarTests <> executionTests <> builtinTests
go interp (label, p, srcText) = testCase label $ runStaticTest label srcText interp p
1 change: 1 addition & 0 deletions pact-tests/Pact/Core/Test/TestPrisms.hs
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ makePrisms ''LexerError
makePrisms ''ParseError
makePrisms ''DesugarError
makePrisms ''EvalError
makePrisms ''UserRecoverableError
makePrisms ''PactError
4 changes: 2 additions & 2 deletions pact-tests/legacy-serial-tests/coin-v5/coin-v5.repl
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@
(transfer 'emily 'doug 1.0))

(expect-failure "No account for will"
"no such read object"
"No value found in table coin:coin-table for key: will"
(get-balance 'will))

(test-capability (TRANSFER 'doug 'will 1.0))
Expand Down Expand Up @@ -485,7 +485,7 @@
(env-data { "miner2": ["miner2"] })

(expect-failure "no account for miner2"
"no such read object"
"No value found in table coin:coin-table for key: miner2"
(get-balance 'miner2))

(test-capability (COINBASE))
Expand Down
6 changes: 3 additions & 3 deletions pact-tests/pact-tests/caps.repl
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@

(expect-failure
"cap guard fails on wrong cap"
"Capability guard enforce failure cap not in scope"
"Capability not acquired"
(test-cap-guard "A" "B"))

(env-hash (hash 1))
Expand All @@ -831,7 +831,7 @@
(cg-pact 'k2 'k2 "D" "E")
(expect-failure
"cap pact guard fails on wrong cap"
"Capability guard enforce failure cap not in scope"
"Capability not acquired"
(continue-pact 1))

(pact-state true)
Expand Down Expand Up @@ -914,7 +914,7 @@

(expect-failure
"out-of-band call fails"
"Capability guard enforce failure cap not in scope"
"Capability not acquired"
(call-op1 "hello" 2))
(expect
"normal case succeeds for both callees post-fork"
Expand Down
4 changes: 2 additions & 2 deletions pact-tests/pact-tests/coin-v1.repl
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@
(transfer 'emily 'doug 1.0))

(expect-failure "No account for will"
"no such read object"
"No value found in table coin:coin-table for key: will"
(get-balance 'will))

(test-capability (TRANSFER 'doug 'will 1.0))
Expand Down Expand Up @@ -484,7 +484,7 @@
(env-data { "miner2": ["miner2"] })

(expect-failure "no account for miner2"
"no such read object"
"No value found in table coin:coin-table for key: miner2"
(get-balance 'miner2))

(coinbase 'miner2 (read-keyset 'miner2) 1.0)
Expand Down
4 changes: 2 additions & 2 deletions pact-tests/pact-tests/coin-v5.repl
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@
(transfer 'emily 'doug 1.0))

(expect-failure "No account for will"
"no such read object"
"No value found in table coin:coin-table for key: will"
(get-balance 'will))

(test-capability (TRANSFER 'doug 'will 1.0))
Expand Down Expand Up @@ -499,7 +499,7 @@
(env-data { "miner2": ["miner2"] })

(expect-failure "no account for miner2"
"no such read object"
"No value found in table coin:coin-table for key: miner2"
(get-balance 'miner2))

(test-capability (COINBASE))
Expand Down
17 changes: 9 additions & 8 deletions pact/Pact/Core/Builtin.hs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module Pact.Core.Builtin
, IsBuiltin(..)
, ReplCoreBuiltin
, BuiltinForm(..)
, ReplBuiltins(..)
, ReplOnlyBuiltin(..)
)where

import Data.Text(Text)
Expand Down Expand Up @@ -697,9 +697,8 @@ coreBuiltinMap = M.fromList
| b <- [minBound .. maxBound]
, b `notElem` coreBuiltinOverloads]

-- Todo: rename
-- | Our repl builtins.
data ReplBuiltins
data ReplOnlyBuiltin
= RExpect
| RExpectFailure
| RExpectFailureMatch
Expand Down Expand Up @@ -747,7 +746,7 @@ data ReplBuiltins
deriving (Show, Enum, Bounded, Eq, Generic)


instance IsBuiltin ReplBuiltins where
instance IsBuiltin ReplOnlyBuiltin where
builtinName = NativeName . replBuiltinsToText
builtinArity = \case
RExpect -> 3
Expand Down Expand Up @@ -798,7 +797,7 @@ instance IsBuiltin ReplBuiltins where
-- to be implemented later
data ReplBuiltin b
= RBuiltinWrap b
| RBuiltinRepl ReplBuiltins
| RBuiltinRepl ReplOnlyBuiltin
deriving (Eq, Show, Generic)

-- NOTE: Maybe `ReplBuiltin` is not a great abstraction, given
Expand Down Expand Up @@ -830,7 +829,7 @@ instance (Enum b, Bounded b) => Enum (ReplBuiltin b) where
RBuiltinRepl rb -> maxContained + fromEnum rb
{-# SPECIALIZE fromEnum :: ReplBuiltin CoreBuiltin -> Int #-}

replBuiltinsToText :: ReplBuiltins -> Text
replBuiltinsToText :: ReplOnlyBuiltin -> Text
replBuiltinsToText = \case
RExpect -> "expect"
RExpectFailure -> "expect-failure"
Expand Down Expand Up @@ -904,8 +903,10 @@ replCoreBuiltinMap =
, let !txtRepr = replCoreBuiltinToText b]


-- Todo: is not a great abstraction.
-- In particular: the arity could be gathered from the type.
-- | A typeclass for general information about pact builtins, mostly
-- useful for runtime native information and for error messages. Note that
-- builtinArity only has 1 value per builtin, meaning overload resolution must
-- happen before using `builtinArity` to check for app saturation.
class Show b => IsBuiltin b where
builtinArity :: b -> Int
builtinName :: b -> NativeName
Expand Down
9 changes: 7 additions & 2 deletions pact/Pact/Core/Capabilities.hs
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,10 @@ data CapState name v
instance (Ord name, Ord v) => Default (CapState name v) where
def = CapState mempty mempty mempty mempty mempty

-- Todo: Is there a reason why module + name is
-- an unqualified
-- | Our pact event type.
-- Note: the name + module are isomorphic to a
-- QualifiedName, but it is kept in this format currently for
-- ease of legacy integration
data PactEvent v
= PactEvent
{ _peName :: Text
Expand Down Expand Up @@ -163,6 +165,9 @@ data Signer name v = Signer
-- ^ clist for designating signature to specific caps
} deriving (Eq, Ord, Show, Generic)

instance (Pretty name, Pretty v) => Pretty (CapToken name v) where
pretty (CapToken qn args) =
pretty $ PrettyLispApp qn args

instance (NFData name, NFData v) => NFData (Signer name v)
instance (NFData name, NFData e) => NFData (CapForm name e)
Expand Down
1 change: 0 additions & 1 deletion pact/Pact/Core/Compile.hs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ enforceNamespaceInstall info interpreter =
useEvalState (esLoaded . loNamespace) >>= \case
Just ns ->
void $ interpretGuard interpreter info (_nsUser ns)
-- Eval.interpretGuard info bEnv (_nsUser ns)
Nothing ->
enforceRootNamespacePolicy
where
Expand Down
9 changes: 9 additions & 0 deletions pact/Pact/Core/DefPacts/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import Pact.Core.PactValue
import Pact.Core.Names
import Pact.Core.Hash
import Pact.Core.ChainData
import Pact.Core.Pretty

data DefPactContinuation name v
= DefPactContinuation
Expand All @@ -42,6 +43,10 @@ data Provenance = Provenance
-- ^ a hash of current containing module
} deriving (Eq, Show, Generic)

instance Pretty Provenance where
pretty (Provenance (ChainId cid) mh) =
parens $
"Provenance" <+> pretty cid <+> pretty mh

-- | `Yield` representing an object
data Yield
Expand Down Expand Up @@ -79,3 +84,7 @@ instance NFData Yield
instance NFData DefPactStep
instance (NFData name, NFData v) => NFData (DefPactContinuation name v)
instance NFData DefPactExec

instance (Pretty name, Pretty v) => Pretty (DefPactContinuation name v) where
pretty (DefPactContinuation n v) =
pretty $ PrettyLispApp n v
Loading
Loading