ethereumjs-monorepo icon indicating copy to clipboard operation
ethereumjs-monorepo copied to clipboard

Review all skipped tests in VM `tester/config.ts` for validity

Open acolytec3 opened this issue 3 years ago • 2 comments

We need to review all of the skipped tests here in the VM test suite to see if any can be re-added or else have been removed from the ethereum/tests repo.

acolytec3 avatar Nov 17 '21 19:11 acolytec3

The final 3 tests have a comment above them, I remember researching it. There appears to be some kind of obscure rule that when two chains have the same total difficulty, it is then the chain with the lowest (or highest) blockHash which should be the tip (have to check it). I also remember researching the other 3.

undefinedOpcodeFirstByte was researched here https://github.com/ethereumjs/ethereumjs-monorepo/issues/1271 and is fixed by https://github.com/ethereumjs/ethereumjs-monorepo/pull/1438, but please note, that last PR is merged in develop as it is a breaking change. (It invalidates the current client state trie database if we would merge that to master). I also remember researching the other two, but I don't recall what was going on there.

jochem-brouwer avatar Nov 30 '21 09:11 jochem-brouwer

Just checked;

These seem to pass: ForkStressTest, ChainAtoChainB, HomesteadOverrideFrontier_FrontierToHomesteadAt5. As mentioned undefinedOpcodeFirstByte is fixed in #1438 but this is not yet merged to master since it is a breaking change.

The other two, blockChainFrontierWithLargerTDvsHomesteadBlockchain and blockChainFrontierWithLargerTDvsHomesteadBlockchain2 are strange; there are two chains, they have the same total difficulty, but one of the chains has faulty blocks. Since this invalid chain has the same total difficulty (TD) and we first knew about the legit chain, we don't really have a reason to re-execute the faulty chain since the TD is the same. But I do recall something about that the "lowest blockhash" should be counted as the "longest chain" in case of equal TDs.

jochem-brouwer avatar Dec 13 '21 19:12 jochem-brouwer

Replaced by #2929

holgerd77 avatar Aug 01 '23 10:08 holgerd77