-
Notifications
You must be signed in to change notification settings - Fork 51
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
Limit VMResult cases when concrete #447
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually makes the code cleaner and easier to read, too. I like it a lot, thanks!
@@ -1251,32 +1251,11 @@ getCodeLocation vm = (vm.state.contract, vm.state.pc) | |||
query :: Query t s -> EVM t s () | |||
query = assign #result . Just . HandleEffect . Query | |||
|
|||
choose :: Choose t s -> EVM t s () | |||
choose :: Choose s -> EVM Symbolic s () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notes to self: so choose is only defined for Symbolic now.
@@ -2718,6 +2715,10 @@ instance VMOps Concrete where | |||
|
|||
whenSymbolicElse _ a = a | |||
|
|||
partial _ = internalError "won't happen during concrete exec" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, so partial
cannot happen during concrete exec
@@ -2589,6 +2565,27 @@ instance VMOps Symbolic where | |||
toGas _ = () | |||
whenSymbolicElse a _ = a | |||
|
|||
partial e = assign #result (Just (Unfinished e)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
partial
now only needs to be defined for symbolic exec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the long run, partial
should not be part of VMOps
but it needs a slight refactor to other functions
@@ -46,7 +45,7 @@ data Action t s a where | |||
Wait :: Query t s -> Action t s () | |||
|
|||
-- | Multiple things can happen | |||
Ask :: Choose t s -> Action t s () | |||
Ask :: Choose s -> Action Symbolic s () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is nice actually.
Description
This builds on top of #427. Better types using
VMType
allowed to eliminate dead code and optimize concrete execution with specializedbranch
. The speedup is ~10% on ethereum-tests.Checklist