EVM-State-Tests

[ScottyPoi]

Referencing issue #2171

Run State Tests from ethereum-tests using just the EVM.

This PR adapts the test runner from VM/tests for this use.

• tester/config.ts

• tester/testLoader.ts

• tester/index.ts

• tester/util.ts

• tester/scripts/state-test-run-test.sh

• tester/runners/GeneralStateRunner.ts

  • Use of VM was removed
  • VM.runTX() was replaced by EVM.runCall()*
    • *EVM.runCall() does not, on it's own, change the state.
    • runAltTx.ts is adapted from VM.runTx() to simulate all the state change that happens in the transaction.

[codecov[bot]]

Codecov Report

Merging #2299 (d843fc5) into master (a44518d) will increase coverage by 0.00%. The diff coverage is n/a.

Additional details and impacted files

Flag	Coverage Δ
block	92.86% <ø> (ø)
blockchain	90.21% <ø> (ø)
client	86.97% <ø> (ø)
common	98.45% <ø> (ø)
devp2p	92.45% <ø> (+0.05%)	:arrow_up:
ethash	∅ <ø> (∅)
evm	79.23% <ø> (ø)
rlp	?
statemanager	88.43% <ø> (ø)
trie	90.36% <ø> (+0.04%)	:arrow_up:
tx	97.98% <ø> (ø)
util	88.99% <ø> (ø)
vm	85.31% <ø> (ø)

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

[ScottyPoi]

The CI won't automatically run this test yet. Can be tested manually with (from packages/evm): npm run test:state npm run test:state: allForks npm run test:state:SelectedForks npm run test:state:slow

[holgerd77]

Hi Scotty, thanks for starting with this, great! 😄

The greatest problem I have for now is that we replicate far (far) too much code with this for now. So > 1.000 LOC is by far too much and couldn't justify such a refactor.

So let's take a step back here again: I guess the main thing which we want here is to have the state tests attribute to the test coverage for the EVM. Is this correct (also question to everyone else)? 🤔

Somewhat of a related side question: is it really settled that the state tests should rather "work" for the EVM coverage and the blockchain tests for the VM coverage (also rather a question for everyone)? An eventual alternative would be to have both test types rather work "for" the EVM and have the remaining VM code solely covered by API tests. I am a bit indifferent, I would think the more important step is to have at least one of these tests work for EVM in the first place.

So my current judgement is that these test runners (state/blockchain) are too tighly coupled and we shouldn't tear this apart, seeing here to how much code replication this leads (even taking into account that e.g. the state runner file can likely be deleted later on the VM side).

Since it seems even necessary - as seen in the PR - to replicate close to all of the VM.runTx() logic this really makes no sense. So I would rather pledge that we (as we do now, lol) take the VM.runTx() logic for the test execution and keep the whole test runner "suite" together and the sole thing we do is to take the state test execution commands test:state:* out of the VM package.json and move it over to the EVM and eventually adopt a bit if this is not directly working. Then we have "the best" [TM] of both worlds, getting the state coverage for the EVM, the blockchain coverage for the VM, and have/keep the test runner integrated without the need of code replication.

The one thing we can think about is to extract the whole test runner (everything in tester, not sure @jochem-brouwer, retesteth as well?) into a separate folder evm-vm-test-runner or the like on the packages file level.

We can also think about a generally somewhat adopted file structure regarding (VM/EVM) test related logic integrating the ethereum/tests submodule as well like (again on packages level):

• evm-vm-tests
  • test-runner
  • retesteth
  • ethereum-tests

What do you guys think? 🤔

[ScottyPoi]

The greatest problem I have for now is that we replicate far (far) too much code with this for now. So > 1.000 LOC is by far too much and couldn't justify such a refactor.

I agree, this seems impractical / unhelpful

[ScottyPoi]

The one thing we can think about is to extract the whole test runner (everything in tester, not sure @jochem-brouwer, retesteth as well?) into a separate folder evm-vm-test-runner or the like on the packages file level.

I though about this, and it does solve the issue of replicating the test runner code.

To run the State tests with just EVM without reproducing VM.runTx()... I don't know :thinking:

[holgerd77]

To run the State tests with just EVM without reproducing VM.runTx()... I don't know 🤔

No no, we run with VM.runTx() (or at least that would be the suggestion), this would then also execute the EVM internally which then counts to code coverage when triggered from the EVM package.json which I assume should then be fine.