EVM refactor (old) #1912
EVM refactor (old) #1912
Conversation
vm: move eei/vmstate into new EEI dir
681b281 to
ac33941
Compare
|
ac33941 is temporary, need to approach this in small steps otherwise I exhaust myself with too many tasks to do at the same time (and then fixing consensus bugs is gonna be very hard) (Also writing this down bit for myself) Next steps: (keeping old steps for journaling)
Next steps
|
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
EEI and EVM both blend some revert/commit logic. Not all revert/commit logic should move to EVM but things like refund counters can easily be handled in EVM. Commit notes |
vm: add comment
97fc68a to
5999d28
Compare
32a5d51 to
d403193
Compare
277fd25 to
2732e54
Compare
vm: fix gas refunds on CREATE opcdoes
vm/evm: fix gasRefund
vm/evm: fixup TransientStorage/EEI creation
1acc189 to
339c581
Compare
| beforeMessage: (data: Message, resolve?: (result: any) => void) => void | ||
| afterTx: (data: EVMResult, resolve?: (result: any) => void) => void | ||
| step: (data: InterpreterStep, resolve?: (result: any) => void) => void | ||
| } |
There was a problem hiding this comment.
Ok, besides all the fear I have that too much stuff is getting into this PR I have to say:
This is a dramatic improvement on the current status quo!!! 🎉 🙏 💯
There was a problem hiding this comment.
(is it correct to have the afterTx event here and another one in VM?)
There was a problem hiding this comment.
Good catch, I copied these function sigs around and forgot to remove afterTx
There was a problem hiding this comment.
And yes, the types are absolutely fantastic 😍
| runCall(opts: RunCallOpts): Promise<EVMResult> | ||
| runCode?(opts: RunCodeOpts): Promise<ExecResult> | ||
| precompiles: Map<string, any> // Note: the `any` type is used because VM only needs to have the addresses of the precompiles (not their functions) | ||
| } |
|
Ok, I very much think we can close here since work is continued in #1950 and will actually do, feel free to reopen if this was mistaken. |
|
Yep that's right, but will use this original post to keep track of the remaining points |
Closes #1903
NOTE: change target branch to
develop!!Commit after branching off from
develop; ae950d2TODOs related to refactor: (probably incomplete)
EEI._state->state, or probably a better name, (journal?))VmStatedependency from EVMRemove-> Will not do this, will be part of standard EVM, but will not be part of EVM interface (since this is not used internally in any of our packages)runCodefrom EVM and fit this logic insiderunCallPossibly add a submodule to-> This can be refactored in a later PR and is non-breaking.Interpreterto cover all the methods moved fromEEIintoEVMInterpretercan be optimized or removed (directly read from_env, for instancegetCallValue()(maybe make_env->env)VMandEVMare extensions ofAsyncEventEmitter, but the latter does not have a type, thereforeVMandEVMare interpreted asanytype with certain fields which are sure there, but we want to ensure they are cast as right type.stepevent (an enum to ensure one does not subscribe to e.g.VM.step-> should subscribe toEVM.step)RemoveStateManagerfromVMas dependency (All chain through EEI)EVM, their names are sometimes inconsistentEVM, also sometimes inconsistent (twogasUsed, one for interpreter and one for tx (which also covers data cost))protectedand/orreadonly_eeiinstead ofeei) and if we should keep this (and be consistent)authseems to be something which should go intorunState). Possibly also consider the frame types introduced intypes.ts(FrameEnvironmenttypes)activePrecompileslogic into EVM (from VM)Note: warm address logic should be kept into EEI because we need to set this up before calling into EVM (add warm addresses in the access list + own account + target account). TODO this is not true, can set this up as input into the EVM.
Cleanups:
EXPfromfunctions.tsintogas.ts(this also ensures thestepevent produces the right dynamic gas once it is invoked)