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

Exceptions that are not caused by the VM should terminate program execution #191

Closed
jwasinger opened this issue Sep 6, 2017 · 9 comments
Assignees

Comments

@jwasinger
Copy link
Contributor

jwasinger commented Sep 6, 2017

Right now, all exceptions are caught and interpreted as errors within the EVM environment (i.e. out of gas). This causes a myriad of problems.

@jwasinger jwasinger assigned jwasinger and unassigned jwasinger Sep 6, 2017
@kumavis
Copy link
Member

kumavis commented Oct 22, 2017

yeah this is really annoying
any non-vm error should be considered a catastrophic event and fail immediately

err / vm-err is conflated here https://github.com/ethereumjs/ethereumjs-vm/blob/master/lib/runCode.js#L217
err is lost here https://github.com/ethereumjs/ethereumjs-vm/blob/master/lib/runCall.js#L154-L185

@kumavis
Copy link
Member

kumavis commented Oct 22, 2017

relevant old issue #117

@holgerd77
Copy link
Member

Jared once made the following suggestion when we were chatting on Gitter:

I'm thinking we add special field to mark EVM exceptions.
then in that try/catch
it will look something like this:

try {

} catch (e) {
    if typeof(e) == 'object ' && 'EVMException' in Object.keys(e) {
        OOG Handling...
    }  else {
        throw("real exception, which ends program excecution")
   }
}

@jwasinger: is this still up-to-date or do you have updated ideas on this?

@axic
Copy link
Member

axic commented Oct 24, 2017

#222 is one step towards that (introduces internal_error)

@holgerd77
Copy link
Member

@axic I think it would be better to keep the vm errors logically separate from programmatic errors. Any reasons not to settle this on one level higher and introduce a new runState.internalError variable (with some adoptions to post-handling)?

Then one gets also more flexible to store the whole error with the stack trace.

@axic
Copy link
Member

axic commented Dec 10, 2017

@holgerd77 now that #219 is merged it should be easy to change whatever needs to be changed to return INTERNAL_ERROR instead. #222 will introduce internal errors for stack mismatch (if the opcode returns an inconsistent count).

@axic
Copy link
Member

axic commented Feb 1, 2018

I think this should be possible now since we have INTERNAL_ERROR signaling a caught exception.

Maybe that's not the case. @holgerd77 do you want to review runCode and the possible error codes?

@holgerd77 holgerd77 self-assigned this Feb 2, 2018
@holgerd77
Copy link
Member

@axic Yes, can do.

@axic
Copy link
Member

axic commented Jul 21, 2018

Fixed by #307.

@axic axic closed this as completed Jul 21, 2018
evertonfraga added a commit to evertonfraga/ethereumjs-vm that referenced this issue Feb 4, 2020
holgerd77 added a commit that referenced this issue Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants