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

EVM/VM refactor #1892

Merged
merged 15 commits into from
May 23, 2022
Merged

EVM/VM refactor #1892

merged 15 commits into from
May 23, 2022

Conversation

gabrocheleau
Copy link
Contributor

@gabrocheleau gabrocheleau commented May 17, 2022

Cleanup of #1862

Resolves #528

@codecov
Copy link

codecov bot commented May 17, 2022

Codecov Report

Merging #1892 (08c647b) into develop (44dac5b) will decrease coverage by 0.13%.
The diff coverage is 66.10%.

Impacted file tree graph

Flag Coverage Δ
block 85.28% <ø> (ø)
blockchain 83.13% <ø> (ø)
client 77.75% <100.00%> (ø)
common 94.92% <ø> (ø)
devp2p 82.83% <ø> (ø)
ethash 90.81% <ø> (ø)
statemanager 84.24% <ø> (ø)
trie 80.14% <ø> (ø)
tx 92.09% <ø> (ø)
util 87.29% <ø> (ø)
vm 81.08% <65.95%> (-0.54%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@gabrocheleau gabrocheleau force-pushed the evm-vm-refactor-final branch from b99a766 to afac4b3 Compare May 17, 2022 22:34
@gabrocheleau gabrocheleau mentioned this pull request May 17, 2022
6 tasks
packages/vm/src/evm/evm.ts Outdated Show resolved Hide resolved
packages/vm/src/evm/evm.ts Outdated Show resolved Hide resolved
packages/vm/src/evm/evm.ts Outdated Show resolved Hide resolved
packages/vm/src/evm/evm.ts Outdated Show resolved Hide resolved
packages/vm/src/evm/evm.ts Outdated Show resolved Hide resolved
packages/vm/src/evm/evm.ts Outdated Show resolved Hide resolved
packages/vm/src/evm/evm.ts Outdated Show resolved Hide resolved
@ryanio
Copy link
Contributor

ryanio commented May 18, 2022

overall looks great, really nice refactoring, thanks for sticking through with this massive PR @gabrocheleau!

just a few last nits then all good from me! some more review from holger and jochem would be great

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Wow, this looks hyper-insanely-great 🎉 🚀 😀 ❤️, thanks so much for sticking through with this Gabriel!

From my side this can be approved and merged once the review comments are addressed.

packages/vm/src/evm/evm.ts Outdated Show resolved Hide resolved
/**
* A {@link StateManager} instance to use as the state store
*/
stateManager: StateManager
Copy link
Member

Choose a reason for hiding this comment

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

Just a note: this should rather be an in-between-step, after the EEI extraction (where external things like state access is encapsulated), a state manager dependency in the EVM itself should not be necessary any more.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, good point. In general I think that VM will instantiate EEI, StateManager and EVM. EEI will have a EVM dependency since EVM will introduce the EEI interface. EEI will depend upon StateManager - here StateManager is also used to export the interface. In "default" mode, VM instantiates EEI(StateManager) and EVM(EEI). EEI will function as the bridge between VM/StateManager, so also stuff like loading storage should move to the EEI (which calls into StateManager)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that describes it well.

packages/vm/src/evm/evm.ts Show resolved Hide resolved
@gabrocheleau gabrocheleau force-pushed the evm-vm-refactor-final branch 2 times, most recently from 3a0edf9 to d4af503 Compare May 19, 2022 10:24
packages/vm/src/evm/evm.ts Outdated Show resolved Hide resolved
packages/vm/src/evm/evm.ts Outdated Show resolved Hide resolved
@gabrocheleau gabrocheleau force-pushed the evm-vm-refactor-final branch from d4af503 to bc73342 Compare May 19, 2022 20:49
ryanio
ryanio previously approved these changes May 19, 2022
Copy link
Contributor

@ryanio ryanio left a comment

Choose a reason for hiding this comment

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

lgtm! holger can give final approval and merge :) really nice work!

@gabrocheleau
Copy link
Contributor Author

lgtm! holger can give final approval and merge :) really nice work!

Hey :) Just tagging @holgerd77 for final approval & merge

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

LGTM!

Thanks a lot Gabriel! 🙂

@holgerd77 holgerd77 merged commit 66f8779 into develop May 23, 2022
g11tech pushed a commit that referenced this pull request Jun 2, 2022
* VM -> EVM/VM Refactor: new EVMOpts EVM options dict, move common and supported HFs to EVM

* VM -> EVM/VM Refactor: standalone DEBUG property in EVM and Interpreter

* VM -> EVM/VM Refactor: standalone EVM stateManager option and property

* VM -> EVM/VM Refactor: moved runCall()/runCode() to EVM, deleted dedicated files, top-level EVM instantiation in VM

* VM -> EVM/VM Refactor: made EVM AsyncEventEmitter, moved beforeMessage, afterMessage, newContract, step events to EVM

* VM -> EVM/VM Refactor: moved allowUnlimitedContractSize option to EVM

* VM -> EVM/VM Refactor: moved opcode and gas handler functionality and options to EVM, removed EVM/Precompiles VM dependency

* Client: fix cliqueActiveSigners method assignments

* VM: refactor TxContext to an interface

* VM: improve Message class types and defaults handling

* VM: refactor runCall and executeMessage into unified runCall

* VM: fix gas refund reset to 0 after tx finishes and fix parenthesis in runBlock receipt ternary

* VM: reimplement transient storage clear method

* VM: use VM async create method instead of constructor

* VM: unify interpreter and evm DEBUG property

Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com>
g11tech pushed a commit that referenced this pull request Jun 2, 2022
* VM -> EVM/VM Refactor: new EVMOpts EVM options dict, move common and supported HFs to EVM

* VM -> EVM/VM Refactor: standalone DEBUG property in EVM and Interpreter

* VM -> EVM/VM Refactor: standalone EVM stateManager option and property

* VM -> EVM/VM Refactor: moved runCall()/runCode() to EVM, deleted dedicated files, top-level EVM instantiation in VM

* VM -> EVM/VM Refactor: made EVM AsyncEventEmitter, moved beforeMessage, afterMessage, newContract, step events to EVM

* VM -> EVM/VM Refactor: moved allowUnlimitedContractSize option to EVM

* VM -> EVM/VM Refactor: moved opcode and gas handler functionality and options to EVM, removed EVM/Precompiles VM dependency

* Client: fix cliqueActiveSigners method assignments

* VM: refactor TxContext to an interface

* VM: improve Message class types and defaults handling

* VM: refactor runCall and executeMessage into unified runCall

* VM: fix gas refund reset to 0 after tx finishes and fix parenthesis in runBlock receipt ternary

* VM: reimplement transient storage clear method

* VM: use VM async create method instead of constructor

* VM: unify interpreter and evm DEBUG property

Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com>
g11tech pushed a commit that referenced this pull request Jun 2, 2022
* VM -> EVM/VM Refactor: new EVMOpts EVM options dict, move common and supported HFs to EVM

* VM -> EVM/VM Refactor: standalone DEBUG property in EVM and Interpreter

* VM -> EVM/VM Refactor: standalone EVM stateManager option and property

* VM -> EVM/VM Refactor: moved runCall()/runCode() to EVM, deleted dedicated files, top-level EVM instantiation in VM

* VM -> EVM/VM Refactor: made EVM AsyncEventEmitter, moved beforeMessage, afterMessage, newContract, step events to EVM

* VM -> EVM/VM Refactor: moved allowUnlimitedContractSize option to EVM

* VM -> EVM/VM Refactor: moved opcode and gas handler functionality and options to EVM, removed EVM/Precompiles VM dependency

* Client: fix cliqueActiveSigners method assignments

* VM: refactor TxContext to an interface

* VM: improve Message class types and defaults handling

* VM: refactor runCall and executeMessage into unified runCall

* VM: fix gas refund reset to 0 after tx finishes and fix parenthesis in runBlock receipt ternary

* VM: reimplement transient storage clear method

* VM: use VM async create method instead of constructor

* VM: unify interpreter and evm DEBUG property

Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com>
g11tech pushed a commit that referenced this pull request Jun 3, 2022
* VM -> EVM/VM Refactor: new EVMOpts EVM options dict, move common and supported HFs to EVM

* VM -> EVM/VM Refactor: standalone DEBUG property in EVM and Interpreter

* VM -> EVM/VM Refactor: standalone EVM stateManager option and property

* VM -> EVM/VM Refactor: moved runCall()/runCode() to EVM, deleted dedicated files, top-level EVM instantiation in VM

* VM -> EVM/VM Refactor: made EVM AsyncEventEmitter, moved beforeMessage, afterMessage, newContract, step events to EVM

* VM -> EVM/VM Refactor: moved allowUnlimitedContractSize option to EVM

* VM -> EVM/VM Refactor: moved opcode and gas handler functionality and options to EVM, removed EVM/Precompiles VM dependency

* Client: fix cliqueActiveSigners method assignments

* VM: refactor TxContext to an interface

* VM: improve Message class types and defaults handling

* VM: refactor runCall and executeMessage into unified runCall

* VM: fix gas refund reset to 0 after tx finishes and fix parenthesis in runBlock receipt ternary

* VM: reimplement transient storage clear method

* VM: use VM async create method instead of constructor

* VM: unify interpreter and evm DEBUG property

Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com>
holgerd77 added a commit that referenced this pull request Jun 8, 2022
* VM -> EVM/VM Refactor: new EVMOpts EVM options dict, move common and supported HFs to EVM

* VM -> EVM/VM Refactor: standalone DEBUG property in EVM and Interpreter

* VM -> EVM/VM Refactor: standalone EVM stateManager option and property

* VM -> EVM/VM Refactor: moved runCall()/runCode() to EVM, deleted dedicated files, top-level EVM instantiation in VM

* VM -> EVM/VM Refactor: made EVM AsyncEventEmitter, moved beforeMessage, afterMessage, newContract, step events to EVM

* VM -> EVM/VM Refactor: moved allowUnlimitedContractSize option to EVM

* VM -> EVM/VM Refactor: moved opcode and gas handler functionality and options to EVM, removed EVM/Precompiles VM dependency

* Client: fix cliqueActiveSigners method assignments

* VM: refactor TxContext to an interface

* VM: improve Message class types and defaults handling

* VM: refactor runCall and executeMessage into unified runCall

* VM: fix gas refund reset to 0 after tx finishes and fix parenthesis in runBlock receipt ternary

* VM: reimplement transient storage clear method

* VM: use VM async create method instead of constructor

* VM: unify interpreter and evm DEBUG property

Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com>
@holgerd77 holgerd77 deleted the evm-vm-refactor-final branch March 2, 2023 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants