Move EEI into EVM

[jochem-brouwer]

The purpose of this PR is to move the EEI into EVM and perform cleanups.

Note: this targets the statemanager-cache-rewrite-new, which currently points to master, but this PR is super breaking and it should eventually land in dev-v7.

[codecov]

Codecov Report

Merging #2604 (6416dd2) into statemanager-cache-rewrite-new (b6f57fc) will decrease coverage by 0.05%.
The diff coverage is 89.36%.

Additional details and impacted files

Flag	Coverage Δ
block	90.16% <ø> (ø)
blockchain	90.40% <ø> (ø)
client	86.85% <100.00%> (ø)
common	95.74% <ø> (ø)
devp2p	91.86% <ø> (+0.08%)	⬆️
ethash	∅ <ø> (∅)
evm	79.51% <82.14%> (+0.31%)	⬆️
rlp	?
statemanager	83.78% <100.00%> (?)
trie	90.66% <ø> (?)
tx	94.33% <ø> (ø)
util	84.50% <ø> (ø)
vm	80.89% <100.00%> (-3.51%)	⬇️

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

[holgerd77]

I think we should be really sparse with taking over interfaces to Common and not "throwing things all over here".

So these should really only be the main interfaces, like StateManagerInterface, eventually BlockchainInterface, and not much else.

So these should be very clear, very well-managed interfaces, where there is a lot of attention onto. Otherwise I fear that we will likely overload and produce too much complexity which won't make things easier to oversee respectively more stable.

I would particularly suggest that we do not expose the cache so prominently, I guess I added this (right?), but anyhow, it would likely be better if we just hack this a bit on the very few direct cache accesses we have internally than build up such heavy structures like this.

I would suggest the following:

• Remove cache from SM Interface, delete all related interfaces
• (CacheClearingOpts is outdated anyhow)
• Remove the dumpStorage and StorageDump from the interface in general, this is only used in one very obscure client location, we can hack this in there
• Throw the StateAccess and StateManagerInterface interfaces together (never gotten a big fan of this)
• Definitely remove this AccountFields partial
• Eventually keep the Proof types, but really only if stable enough

Two other things:

1. This could also very well be an own PR, totally independent from other stuff
2. I think we should rather do a dedicated file interfaces.ts for this

[holgerd77]

I don't think we need this option.

I somewhat understand the intention to "warn" users. I would argue that this is somewhat intuitive though (if an EVM has no connection to whatever blockchain, no information from a/the blockchain can be read). So I think if we have this prominently in the docs that really should be enough.

At the same time the option causes a lot of bloat - like in the tests.

[holgerd77]

Hi Jochem,
could you please reason a bit what you want to achieve with the PR respectively what the design goals are? We actually did a lot of work to extract the EEI from the EVM and move this over to VM, now we are moving back here, at least to some extend? This might be justified if we have a clearer view here now and this will come with an adopted structure, I would like to have some more clear reasoning though so that we can get a bit more assured that we are on a good and sustainable path with this.

I also have the impression that there are several subtasks involved here which might be able to be separated, like:

1. Extracting the interfaces (and discuss them)
2. Go directly "through" to StateManager and not use the EEI
3. Not sure if there are others, e.g. related to this "Fake/Mock/Default Blockchain" idea

Not sure if it might be worth to think a bit again about the order of tasks and eventually submit separately? Whatever is best and easier to realize. Just feels a bit several things are mixed here together.

[holgerd77]

Another thing: I would really like to go through the VMState functionality one by one and see what information we can directly get out of the cache here. Guess we can just do one-by-one - would be great if you can help a bit - and remove these originalStorage "cache" structures, the warmed address and storage stuff, maybe at the end we can just completely delete this file.

Would be good if we think this a bit along here, do you think this work has any bigger implications on this? 🤔 Just had a look, you didn't touch VMState that much, right?

We might also not need to finish this work before breaking, just make sure we have the structures in place (so do not expose VMState in any way or encourage usage or customization (not sure if we are doing this now?), the StateManager interface might also need some additional adjustments (so the getContractStorage() call might just return the warm/cold information additionally if requested e.g., stuff like that).

So that we have all the breaking things in place. 🙂

[holgerd77]

Maybe we should just do some call on this and discuss this directly 😋, maybe on Thursday morning?

[jochem-brouwer]

Follow-up PR #2649