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

refactor: separate error clearing from inner functionality #28633

Closed
wants to merge 1 commit into from

Conversation

youben11
Copy link

@youben11 youben11 commented Dec 1, 2023

EVMInterpreter.Run was clearing the errStopToken, which is useful in some projects that customize the EVM. One usecase is the fhEVM, which have a special behavior for errStopToken.

The current refactor shouldn't have side effects but only expose a new function in the API that does run without clearing the error

Fix: #28503

`EVMInterpreter.Run` was clearing the `errStopToken`, which is useful in some projects that customize the EVM.
@holiman
Copy link
Contributor

holiman commented Dec 4, 2023

tagged for triage discussion

@karalabe
Copy link
Member

karalabe commented Dec 5, 2023

We've looked at it a bit closer. We kind of think you can already do what you want without an API change.

The Run method returns nil in two possible scenarios:

  1. There is no code to run.
  2. The STOP opcode was hit.

This means that you can actually consider a nil return as hitting a STOP opcode without needing to explicitly check for an error type. Now, depending on how you want the empty code execution to behave, you might want to (or not to) check for the code's existence yourself.

Would that not be enough to solve the issue without an API change?

@youben11
Copy link
Author

youben11 commented Dec 6, 2023

Indeed it would solve it 👍

I would also guess that this behavior of the interpreter isn't meant to change, right?

@holiman
Copy link
Contributor

holiman commented Dec 6, 2023

I would also guess that this behavior of the interpreter isn't meant to change, right?

It's not meant to change, but also it's not set in stone. But generally, the behavour can be described as "run to completion (end of code), unless an error occur (If error occur, return it)". I don't see why we would ever change that.

@holiman holiman closed this Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor Interpreter.Run to separate body from error clearing
4 participants