Major Version Release Planning / Refactor Strategy / v3 and v4 Releases

[holgerd77]

This is a meta issue for planning the next releases and discussing a refactoring strategy, the refactoring strategy not mainly from the point WHAT to do but WHEN to introduce and how to work on in what order, the WHAT parts should be discussed in the respective issues or PRs:

• Refactor evm and prepare for ewasm integration #424
• Basic support for ewasm precompiles #431
• VM: Support for better debugging and state manipulation #410

Some personal pre-note on releases: regarding semantic versioning I for a long time couldn't be so consequent to fully apply this for the VM. This has historical reasons since we were doing HF updates on minor releases and I had so much respect that I didn't dare to do any major version releases. 😄 😄 😄 I would take this current work as an occasion to change this, think we should really just apply semantic versioning here (and especially here) on the VM, everything else just doesn't make sense on a clear and step-back view. Let me know if you have thoughts on this.

Regarding a release strategy I would suggest that we do two follow-up major version releases:

Version v3.0 release: Local refactoring and code modernization

• ES6 modernization, Migrate stateManager and storageReader to es6 class syntax #452, Convert Bloom to es6 class #428 (released), Convert VM to es6 class #478
• Separate module preparation, Move stateManager, storageReader and cache to state module #443, Move bloom to its own directory #429 (released), Mv opcodes, opFns and logTable to lib/vm #425 (released), Rename vm to evm, move precompiles to evm module #481
• Drop hookedVM, Drop createHookedVm #451
• Drop fakeBlockchain, TODO
• Memory class, Refactor evm and prepare for ewasm integration #424, Refactor memory manipulation in evm #442
• Stack class, TODO, Refactor evm and prepare for ewasm integration #424

This list is not complete and doesn't imply that other work can be integrated.

Version v4.0 release: System wide refactoring / TypeScript / eWASM

• TypeScript transition (or move this to v3.0?)
• evm refactoring with Interpreter, opcode and environment (EEI classes), see discussion here, discuss in EVM refactoring #456
• Promises?
• Partly eWASM integration here or maybe also one release later?, Refactor evm and prepare for ewasm integration #424, Basic support for ewasm precompiles #431
• Separate libraries for inner VM, outer processing engine, Bloom, StateManager (or do this later on a v5.0 release)

This is just a draft for some discussion start. We should update this list over time to keep up with the current state of discussion.

Note: feel free to directly edit this list.

[holgerd77]

Ok, ready with some first write-up.

[s1na]

This is amazing, thanks a lot! I had a question: do you think transitioning to a monorepo is something we can consider maybe for v4? I still maintain it'd help a lot with maintenance.

[holgerd77]

Regarding monorepo: what I think might be very well in the scope is that we separate into the modules you prepared (state, bloom, vm, outer VM/Processing).

[holgerd77]

@s1na I think we should put a bit more active thinking in how to introduce eWASM along with the releases. One thing I was wondering: is the EEI implementation so VM (so EVM vs eWASM) agnostic that the same code can be used for both EVM and eWASM environment communication? When looking at the precompile PR #431 I also had the impression that the basic memory implementation is pretty similar to the one from the refactoring already done, don't have the complete picture, is Memory also something which can be reused? Would/could/should 😄 EEI and eventually Memory generic objects moved out of the specific inner VM implementation and instead passed to the VM on initialization?

Would it be a way to start using the EEI on EVM 1.0 together with a refactored runState (probably this keep the local stuff within the specific VM not visible to the outside and stuff like blockchain and the like will be removed and instead be replace by the EEI communication?) and we could introduce this with v4.0, so basically all the agnostic parts from eWASM which apply to both VMs. Feel very free to let me know if this makes no sense at all. 😛 😄

Side thought: I wondered if gas handling could also be refactored similar to Memory and Stack, this seems to me similar in structure, this would then be a candidate to be included in the v3.0 release. Any thoughts about that?

[holgerd77]

I think we should generally try to get this v3.0 release ready within the next 1-2 weeks, since there are also bug fixes included people are waiting for.

@s1na Can you have a look at the missing code modernization parts and generally have one step-back-view on stuff here?

[s1na]

First a general note: While I completely agree with and understand the need for an outlook, the problem is that I'm not so well-versed in the library to be able to come up with a concrete plan/design in advance, without playing around and trying out a bunch of stuff (e.g. #424 which was my first stab at refactoring the VM) until one works! 😅

One reason I'm pursuing both ewasm integration and evm refactoring at the same time is to be able to understand what's common between them and can be abstracted, and answer questions such as the ones you mentioned.

But my rough plan is really similar to all you described, basically try to have all these abstractions (EEI, limited runState, Memory, etc.) work with EVM, and when integrating ewasm, take whatever that's usable in both to the outer vm module, and whatever is specific to their corresponding modules. Most of EEI can be shared between them, but there might be some parts (the async parts) that pose challenges. In cases such as Memory where the implementations are really similar but not exactly the same, we can have an interface which each vm implements.

I agree about gas handling, I'm not sure what the structure would look like however as of yet, do you have any ideas there?

I think we should generally try to get this v3.0 release ready within the next 1-2 weeks, since there are also bug fixes included people are waiting for.

I was thinking about this, and one strategy we might try in future is to have all major refactoring in a different branch and in the meanwhile if there were any bug fixes, master could push releases even if the other branch is not ready to be merged.

But for the upcoming release, what would you like to have in the release that's not there now so I can prioritize them? (btw, from my perspective, we could already do a release (UPDATE: after #466), since the ES6 modernization shouldn't effect the users and can be shipped in a minor release, if that's the only thing left).

[holgerd77]

Makes all very much sense, will give a more detailed answer later the day.

[holgerd77]

Some more in-depth answer: the approach to experiment on various fronts until things settle and one get a better hold of the breadth of the changes needed and a feeling for the approaches which might work best make very much sense, so I would very much encourage you to continue this like you are doing right now. We can then reflect every now and then how if we roll things out and what is settled enough to be planned in for a future release and what might generally be a good strategy which would fit.

Yeah, probably for the next major release v4.0 working on a separate branch will make sense.

Beyond moving the gas related functions from opFns.js to a dedicated file (gas.js, gasMeter.js, ?) I have not looked deep enough into how a refactor could look like. That would already be some start eventually.

For the next release we have already one major breaking change with #451 (hookedVM removal), so if we don't want to revert here we have to do some v3.0 release. I would generally have a tendency to do so, since also the Stack and Memory changes are pretty heavy and while theoretically not breaking this might affect people to a larger extend than one assumes if people have deeper integrated their code with the VM then they should 😛 .

[holgerd77]

One on-top question: is using an ES6 class for index.js easy to integrate and could you do a small PR on this, or did this pose unexpected problems (due to the functionality integration through different files are whatever)?

This would complete the first point in the list above. Do you have any other code modernization wishes we should integrate in this round?

[holgerd77]

(Sorry, mixed up the release version numbers and always had one to low, updated this now)

[holgerd77]

@s1na Unrelated, was wondering:

1. Should the precompiles also be moved to the vm module - since they are clearly associated with the specific VM - and should instantiation also take place further down the line (in runCode or runCall respectively aligned with your rebased structure? I think we should then also rename everything from precompiled to precompiles since this is the (by far) more established name used.
2. Should we rename the vm module to evm to have a clearer distinction between the the inner and outer modules?

[s1na]

@holgerd77

1. Yes! I also had that in mind and was planning that for v4.
2. Also yes, should we do that before the v3 release?

[holgerd77]

1. Maybe we move the folder and do the rename (folder + everywhere in the code base) for v3 but leave the instantiation code as is for this version?
2. Let's already rename, there will be a lot more structural changes coming and if we do more of these local ones now we are better prepared and this is a breaking release anyhow

If you go along with both can you prepare a PR?

[holgerd77]

Informational note: atm I would target a v3 release for next Wednesday or Thursday since we seem to have everything together and then we nevertheless have a couple of days to settle down and prepare a bit.

[s1na]

Sounds good!

By the way, how do you think we can test major releases before publishing to npm? Maybe try upgrading ethereumjs-vm in a dependent project and run the tests to get a rough idea?

[holgerd77]

Hmm, not sure. Do you really think we should take on this and this is our job? At the end this is a breaking release, and things not to work is to some extend expected.

I see a bit the danger that once one starts with this it will draw very much in and take too much time, since we can't do the work on e.g. the Ganache side to do the integration there.

If you have low level/easy-to-apply ideas let me know, not totally against it though.

[s1na]

@holgerd77 So far v4 only changes internal functionality and we're not exposing evm internals (which I think is a good decision). I was wondering now that we're doing a major release, maybe it'd be a good opportunity to ship some other small breaking changes to lay groundwork for later versions. Some options I had in mind:

• Promisifying the API. We're already using Promises internally for most parts and users can if they're not yet ready to migrate to promises use util.callbackify and the likes.
• Using normal arguments, optional arguments, and arguments with default values for VM instantiation and methods. E.g. turn runBlock(opts: RunBlockOpts) to runBlock(block: Block, generate: boolean = false, skipBlockValidation: boolean = false, root?: Buffer). This would lead to cleaner argument parsing and documentation (perhaps @alcuadrado can chime in if he thinks there are downsides to this approach)
• Cleaning results returned from various methods, e.g. InterpreterResult.gasUsed is the same as ExecResult.gasUsed, or Uniform use of VmError for exceptions #521 (there are a few more of these).

These were just ideas at the top of my head. If you think any of them has merit I can create an issue for it for further discussion.

[holgerd77]

@s1na Yes, that definitely makes sense to take the occasion and do preparatory breaking changes.

Some thoughts:

1. Promisifying the API
   -> Would be in favor, we should definitely have another opinion here though
2. Plain arguments instead of opts dictionaries
   -> Not sure, one main pro argument is to keep the flexibility on more easily change the API respectively add more options without having to care for the order or arguments
3. Cleaning results
   -> That definitely makes sense

[holgerd77]

//cc @krzkaczor

Thanks for your assessment on the devp2p TypeScript questions, super helpful! 😄 Do you think the time is there to completely switch to a promise-based API on the VM, or should one still support both promises and callbacks? Discussing this above...

[krzkaczor]

@holgerd77 no problem ;)

1. I would focus only on supporting promises. I never got a user report about introducing callback support 😆
2. Generally, I prefer opts especially with TS where its super simple to check what's really in a given object. This approach scales better as well — it's easy to add (optional) properties later etc and there is nice language-level support for default args (object spread syntax).

[alcuadrado]

1. Promisifying the API
   -> Would be in favor, we should definitely have another opinion here though

I was planning to start a discussion in the organization repo today about this.

Node 12 has already been released, and it has async stack traces if you stick to async/await (i.e. no callbacks, no .then() nor .catch()). It's not easy no understand the reason for this restriction, but it's super easy to adhere to it.

IMO this is the major DX improvement in the JS ecosystem since async/await was introduced, and I believe users will start expecting async stack traces everywhere, forcing library authors to stick to async/await. In fact, this feature was somewhat criticized by some TC39 members because it has the potential to heavily influence the language.

For this to work in the VM almost every EthereumJS library has to be changed, which is a huge task that we shouldn't hurry. For now, we just need to start exposing only promise-based APIs.

For legacy code, we can create something similar but opposite to what @s1na did with the promisified StateManager, or recommend using util.callbackify.

[s1na]

there is nice language-level support for default args (object spread syntax).

@krzkaczor do you mean something like this, or is there a better way? typedoc doesn't seem to detect these default values :(

Ok there seems to be consensus on exposing only a promised-based API. Most of the methods are already calling a promisified private method internally, so it shouldn't be difficult. I think only runBlockchain is pure-callback which should also be made promisified.

[krzkaczor]

@s1na I actually thought about something like:

const default = {
	x = 5
}

const finalArgs = {
	...default,
	...args,
}

but yours solution is also quite nice!

[alcuadrado]

What do you think about releasing new versions of the other libs that have been migrated to TS and updated them here before v4?

Things like this could be typed if we do that:

  tx: any // TODO: Update ethereumjs-tx
  blockchain: any // TODO: update to ethereumjs-blockchain v4.0.0

[holgerd77]

@alcuadrado Yeah, that would make very much sense, let's definitely do this before (beta) release already!

tx: I'm wanting to prepare release notes for this for days but didn't find the time yet. If you want to take on this you are also very much invited, then go along some similar PR like ethereumjs/ethereumjs-blockchain#106 and let me know that you take on the task. Otherwise I will try to do it this late evening (ECT), but can't promise on this from my side.

blockchain: This also needs fixing the browser compatibility issue on the library itself, see
#517

[alcuadrado]

I'm super busy today. I can do it tomorrow.

I created an issue in blockchain to track this: ethereumjs/ethereumjs-blockchain#114

[holgerd77]

👍

[davidmurdoch]

v4.0.0 looks to be requiring that we reference .default when importing via require?

Would something like

export default class VM extends AsyncEventEmitter {
  // ...
}
module.exports = VM;

let us use both import VM from "ethereumjs-vm" and const VM = require("ethereumjs-vm")? I'm not a TSC expert by any means, so maybe there is some optimization or compatibility reason I'm not aware of.

[alcuadrado]

I'm pretty sure that's not possible @davidmurdoch.

One thing that we could do is to stop using default exports. That way you could import this module with import { VM } from "ethereumjs-vm" and const { VM } = require("ethereumjs-vm"), which is more or less the same experience in every module system.

[davidmurdoch]

I did a quick test with one of TS projects and it does seem to work. Though I didn't test very thoroughly. May be worth investigating alternatives to exporting a .default.

[alcuadrado]

I did a quick test with one of TS projects and it does seem to work. Though I didn't test very thoroughly. May be worth investigating alternatives to exporting a .default.

That caught my attention, so I made this test.

tsconfig.json:

{
  "extends": "@ethereumjs/config-tsc",
  "compilerOptions": {
    "outDir": "./dist"
  }
}

index.ts:

const a = 1;

export default a;

// This never gets exported
export const b = 2;

module.exports = a;

Compilation output:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var a = 1;
exports.default = a;
// This never gets exported
exports.b = 2;
module.exports = a;
//# sourceMappingURL=index.js.map

As you can see, this beaks the TS modules as soon as you export more things. Assigning module.exports is not something that TS understands, and it sometimes works just by chance.

I kept reading about this and found another alternative. We can use the export = VM syntax. This has the disadvantage that TS users without esModuleInterop enabled will have to use the weird import VM = require("ethereumjs-vm") syntax.

IMO we should avoid default exports. This is starting to get a pretty common practice in TS and ESM-based js. /cc @krzkaczor

[s1na]

IMO we should avoid default exports. This is starting to get a pretty common practice in TS and ESM-based js.

If that's the case we can start changing the default exports to named ones (export class VM...). But I'd also prefer not to use the export = VM syntax.

[holgerd77]

//cc @dryajov (context for others: Dmitriy worked on the devp2p library TypeScript transition, see ethereumjs/ethereumjs-devp2p#51).

[holgerd77]

This process has been finished with the v4 release #571, will close.