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/vm: Hide read only flag from Interpreter interface #17461

Merged
merged 1 commit into from
Sep 7, 2018

Conversation

chfast
Copy link
Member

@chfast chfast commented Aug 21, 2018

Not really a functional change, but makes Interface interface a bit more stateless and abstract.

@@ -125,7 +121,7 @@ func (in *EVMInterpreter) enforceRestrictions(op OpCode, operation operation, st
// It's important to note that any errors returned by the interpreter should be
// considered a revert-and-consume-all-gas operation except for
// errExecutionReverted which means revert-and-keep-gas-left.
func (in *EVMInterpreter) Run(contract *Contract, input []byte) (ret []byte, err error) {
func (in *EVMInterpreter) Run(contract *Contract, input []byte, static bool) (ret []byte, err error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I forgot to rename this to readOnly.

Copy link
Contributor

@fjl fjl left a comment

Choose a reason for hiding this comment

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

I like this change, but please fix the variable name.

@chfast
Copy link
Member Author

chfast commented Sep 3, 2018

Done.

core/vm/interpreter.go Outdated Show resolved Hide resolved
@holiman
Copy link
Contributor

holiman commented Sep 4, 2018

I can live with this, but it forces the global readonly:ness to be part of the internal state of whatever evm is used anyway, so I can't imagine it changes a lot for the evm implementation.

@axic axic mentioned this pull request Sep 4, 2018
@chfast
Copy link
Member Author

chfast commented Sep 5, 2018

I can live with this, but it forces the global readonly:ness to be part of the internal state of whatever evm is used anyway, so I can't imagine it changes a lot for the evm implementation.

Obviously this change is dictated by EVMC design. The EVMC tries to keep the responsibility for EVM features totally inside the VMs, if feasible. This makes VM "stateless" because VM does not need to pass any information between executions, all information is included in parameters of the execute function.

So in the EVMC PR for geth this flag is never used by the EVMC VM and is also not accurate, so querying it might give wrong results.

I'm not sure this provided enough arguments for it to make you feel comfortable with the change.

@gballet gballet merged commit ae992a5 into ethereum:master Sep 7, 2018
5chdn added a commit to goerli/go-ethereum-aura that referenced this pull request Sep 8, 2018
…) (#6)

Makes Interface interface a bit more stateless and abstract.

Obviously this change is dictated by EVMC design. The EVMC tries to keep the responsibility for EVM features totally inside the VMs, if feasible. This makes VM "stateless" because VM does not need to pass any information between executions, all information is included in parameters of the execute function.
dutterbutter pushed a commit to goerli/go-ethereum-aura that referenced this pull request Sep 8, 2018
* go modules added; preparing to write aura consensus

* adding goerli flag

* goerli flag added

* adding configs

* core/vm: Hide read only flag from Interpreter interface (ethereum#17461) (#6)

Makes Interface interface a bit more stateless and abstract.

Obviously this change is dictated by EVMC design. The EVMC tries to keep the responsibility for EVM features totally inside the VMs, if feasible. This makes VM "stateless" because VM does not need to pass any information between executions, all information is included in parameters of the execute function.

* configuring genesis

* removing duplicate imports

* adding configs for aura api

* configuring goerli cli flag

* flag function fix

* typo fix
ChainSafeSystems pushed a commit to goerli/go-ethereum-aura that referenced this pull request Sep 8, 2018
* go modules added; preparing to write aura consensus

* adding goerli flag

* goerli flag added

* adding configs

* core/vm: Hide read only flag from Interpreter interface (ethereum#17461) (#6)

Makes Interface interface a bit more stateless and abstract.

Obviously this change is dictated by EVMC design. The EVMC tries to keep the responsibility for EVM features totally inside the VMs, if feasible. This makes VM "stateless" because VM does not need to pass any information between executions, all information is included in parameters of the execute function.

* configuring genesis

* removing duplicate imports

* adding configs for aura api

* configuring goerli cli flag

* flag function fix

* typo fix

* goerli flag finalized

* config mod
ChainSafeSystems pushed a commit to goerli/go-ethereum-aura that referenced this pull request Sep 8, 2018
* go modules added; preparing to write aura consensus

* adding goerli flag

* goerli flag added

* adding configs

* core/vm: Hide read only flag from Interpreter interface (ethereum#17461) (#6)

Makes Interface interface a bit more stateless and abstract.

Obviously this change is dictated by EVMC design. The EVMC tries to keep the responsibility for EVM features totally inside the VMs, if feasible. This makes VM "stateless" because VM does not need to pass any information between executions, all information is included in parameters of the execute function.

* configuring genesis

* removing duplicate imports

* adding configs for aura api

* configuring goerli cli flag

* flag function fix

* modifying configs
@axic axic deleted the vm-static-flag branch April 24, 2019 09:36
thanhnguyennguyen pushed a commit to thanhnguyennguyen/tomochain-v1 that referenced this pull request Sep 29, 2019
Makes Interface interface a bit more stateless and abstract.

Obviously this change is dictated by EVMC design. The EVMC tries to keep the responsibility for EVM features totally inside the VMs, if feasible. This makes VM "stateless" because VM does not need to pass any information between executions, all information is included in parameters of the execute function.
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.

4 participants