search

scroll-tech / go-ethereum

Scroll's fork of the official Go implementation of the Ethereum protocol
GNU Lesser General Public License v3.0
494 stars 276 forks source link

Diff with the current geth codebase's vm.interpreter.Run function #110

Open hsyodyssey opened 2 years ago

hsyodyssey commented 2 years ago

Hi, just found that our EVM's interpreter's Run function is a bit different than the current codebase of Geth. In order to improve performance, they removed the code in this place that https://github.com/scroll-tech/go-ethereum/blob/zkrollup/core/vm/interpreter.go#L183-L188. (Current Geth: https://github.com/ethereum/go-ethereum/blob/master/core/vm/interpreter.go#L181-L185)

Since this part is the core function of EVM, maybe we should align with them in the next release?

FYI:

lispc commented 2 years ago

I am not sure whether this commit only improves performance, or change the "interface".

PR like this one https://github.com/ethereum/go-ethereum/pull/24867 changes the "interface" (or changes behavior of API) of tracing. We may have to upgrade both zkevm-circuits to handle this change AND rebase the diff into this l2geth repo at the same time later.

But if a commit only improves performance but does not change the API input/output, we don't need to apply the diff urgently.

hsyodyssey commented 2 years ago

It looks like they moved some of the logic to the instruction level. I'm not that familiar with circuit design, so maybe you can see if the opJump and opJumpi instructions have had an impact on the current circuit design.

They have merged this pr to 1.10.14. It looks like we are using 1.10.13. which is only a minor release away😂.