VM: Update Ethereum/tests Submodule #1116
Conversation
holgerd77
added
type: tests
prio: P2 very important
package: vm
type: test all hardforks
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Ah, ok, e2d7475 finally triggered something (I was already starting to feel uneasy thinking I might be totally overdoing here. 😜 Generally this went better than I expected though up-till-now). So the Homestead blockchain test run is failing (again: ignore the Berlin tests for now, we can tackle this separately in a new PR), here is an extract: ok 6332 correct pre stateRoot
ok 6333 Expected exception InvalidStateRoot
ok 6334 correct last header block
# file: nonceWrong test: nonceWrong_Homestead
ok 6335 SealEngine setting is not matching chain consensus type, skip test. # SKIP
ok 6336 correct genesis RLP
ok 6337 correct pre stateRoot
Error: invalid POW
at /home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/packages/blockchain/src/index.ts:887:19
at Blockchain.runWithLock (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/packages/blockchain/src/index.ts:407:21)
at Blockchain._putBlockOrHeader (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/packages/blockchain/src/index.ts:851:5)
at Blockchain.putBlock (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/packages/blockchain/src/index.ts:806:5)
at runBlockchainTest (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/packages/vm/tests/BlockchainTestsRunner.ts:127:7)
at /home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/packages/vm/tests/tester.ts:172:19
at fileCallback (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/packages/vm/tests/testLoader.ts:60:11)
not ok 6338 Error: invalid POW |
|
This is likely coming from the "buffer + other tests 761-766" part, other tests are Berlin/Yolo only. |
|
@holgerd77 for cases like this one, where you need to test until which specific commit things were working, there's if you need anything, LMK! |
|
Hmm, a bit strange, I did some isolated test run here with: ts-node ./tests/tester --blockchain --fork=Homestead --file='nonceWrong'(Attention! The This gives the following output and the test then passes: +--------------------------------------------------+
| VM -> BlockchainTests |
| |
| TestGetterArgs |
| skipTests : 51 |
| forkConfig : Homestead |
| file : nonceWrong |
| |
| RunnerArgs |
| forkConfigVM : homestead |
| forkConfigTestSuite : Homestead |
| common : homestead |
+--------------------------------------------------+
TAP version 13
# BlockchainTests
# file: nonceWrong test: nonceWrong_Homestead
ok 1 SealEngine setting is not matching chain consensus type, skip test. # SKIP
ok 2 correct genesis RLP
ok 3 correct pre stateRoot
ok 4 Expected exception InvalidBlockNonce
ok 5 correct last header block
1..5
# tests 5
# pass 5
# okSo might be a hint that there is rather some failure in the test setup and eventually some parameter from a previous test run is not cleaned up correctly or something? 🤔 |
…s/updates, EIP-2929, buffer + other tests)
… Blockchain Homestead expected test number
holgerd77
force-pushed
the
update-ethereum-tests
branch
from
8a0f746
to
b669496
Compare
…e transition, vmArithmetic tests, Yolo v3, removed EIP-2537)
|
Ah ok, all passing, with a small fix. 😄 This is now ready for review. Very much think we can merge, and then we can address fixing the |
| Istanbul: 10759, | ||
| MuirGlacier: 10759, | ||
| Istanbul: 10715, | ||
| MuirGlacier: 10715, |
There was a problem hiding this comment.
Any reason why the amount of tests here have decreased?
There was a problem hiding this comment.
TBH: these number checks are totally out of work anyhow, noticed this after the changes here. Istanbul actually has 11632 checks - see the respective run. But the checks are not enforced and - furthermore - we also have this double structure with redundant test numbers being both in the config.ts and the CI workflow files in CI.
Don't know - I would have a tendency to fix this separately (or at least - to be plain honest - I am not motivated to do this here along this PR atm).
Ok, this is one of the last big tasks needed both for a client and a berlin/Yolo VM release.
This shouldn't be underestimated since the last integration is somewhat long ago (66a55cd from July 13, 2020) and there were countless changes in the
ethereum/testsrepo. We can get some orientation here from the release notes in the v7.0.1 release, this likely not covers all the changes along though since our last internal integration (update: ah, maybe the release does include all the changes since our latest release).With this
v7.0.1release the situation will get better though since there now will be regular releases on the tests repo we can follow along with. 🙂 🎉I think the best chance we have to approach this systematically is to go a few (dozen) commits from
ethereum/testswith every commit here and see if things still work. Otherwise we will just drag in too much changes at once and will get lost. We will have - for sure - to adopt our test structure along the way and eventually also fix some bugs from the tests itself.I'll prepare a corresponding table a start with a first commit just staying on the same initial
ethereum/testsstate mentioned above but trigger all the additional tests with ourtype: test all hardforkslabel (which we haven't used in a while).Phew. Let's start. 😀
Note:
Everyone invited here to continue working on this.