Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to ethereumjs-testing v1.3.1 (pre-Berlin HF release) / Fix test runs #743

Merged
merged 5 commits into from
Jun 1, 2020

Conversation

holgerd77
Copy link
Member

@holgerd77 holgerd77 commented May 11, 2020

This is a test run of the updated v1.3.1 consensus tests referencing a state of ethereum/tests in a pre-Berlin HF state. Check is mainly for a first impression on how the tests behave.

See v7.0.0 pre-Berlin HF release PR ethereum/tests#687 on the ethereum/testsrepo for hints on what has changed there and what tests have been added.

[ DO NOT MERGE ]

@codecov
Copy link

codecov bot commented May 11, 2020

Codecov Report

Merging #743 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #743   +/-   ##
=======================================
  Coverage   92.02%   92.03%           
=======================================
  Files          46       47    +1     
  Lines        2998     3001    +3     
  Branches      469      469           
=======================================
+ Hits         2759     2762    +3     
  Misses        144      144           
  Partials       95       95           
Flag Coverage Δ
#account 94.11% <ø> (ø)
#block 88.09% <ø> (ø)
#blockchain 88.74% <ø> (ø)
#common 94.37% <ø> (ø)
#tx 94.16% <ø> (+0.13%) ⬆️
#vm 93.09% <ø> (ø)
Impacted Files Coverage Δ
packages/tx/src/index.ts 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56d02e7...08e41aa. Read the comment docs.

@holgerd77
Copy link
Member Author

Ok, StateTests are passing, that's already impressing! 😄

@holgerd77
Copy link
Member Author

54 BlockchainTests fail, this needs some deeper look.

Some example failures:

ok 42921 last block hash
ok 42922 correct header block
# file: bcInvalidRLPTest_BLOCK_ test: BLOCK_HeaderLargerThanRLP_1_Istanbul
ok 42923 correct pre stateRoot
ok 42924 correct genesis RLP
not ok 42925 Error: invalid rlp: total length is larger than the data
  ---
    operator: fail
    at: replenish (/home/runner/work/ethereumjs-vm/ethereumjs-vm/packages/vm/node_modules/async/dist/async.js:440:21)
  ...

ok 42981 last block hash
ok 42982 correct header block
# file: bcInvalidRLPTest_BLOCK_ test: BLOCK_WrongCharAtRLP_0_Istanbul
ok 42983 correct pre stateRoot
ok 42984 correct genesis RLP
not ok 42985 Error: invalid rlp: total length is larger than the data
  ---
    operator: fail
    at: replenish (/home/runner/work/ethereumjs-vm/ethereumjs-vm/packages/vm/node_modules/async/dist/async.js:440:21)
  ...

ok 43031 last block hash
ok 43032 correct header block
# file: bcInvalidRLPTest_BLOCK_ test: BLOCK_ZeroByteAtRLP_0_Istanbul
ok 43033 correct pre stateRoot
ok 43034 correct genesis RLP
not ok 43035 Error: invalid remainder
  ---
    operator: fail
    at: replenish (/home/runner/work/ethereumjs-vm/ethereumjs-vm/packages/vm/node_modules/async/dist/async.js:440:21)
  ...

Note: debugging instructions can be found in the VM developer documentation.

@ryanio
Copy link
Contributor

ryanio commented May 11, 2020

@holgerd77 are these Istanbul tests new?

@holgerd77
Copy link
Member Author

Ok, have written together some release notes for a current-state release over on the ethereum/tests repo ethereum/tests#687, this should provide some ground for working on the fixes here better be able to draw causal relations, for everyone who wants to start on working on an update here.

@ryanio Generally Istanbul tests have been there before. But have a look at "Tests added" section from release notes above. There are also some changes in the test format (see "Test Format Changes" section) which might be the reason for failures here.

Generally there are actually A LOT of changes done since the last tests release. We might want to have a closer look on various updates separately here (just one significant but not too exposed example: VMtests have been integrated into the general BlockchainTests suite, would assume we don't run these yet due to the folder structure, not confirmed though).

We might also want to have a closer look regarding the updates from the release notes if all CURRENT tests are still executed in full extend on the updated test suite, sometimes it happens that CI runs still pass but there are actually significantly less (or - on the extreme - no) tests run in total (spontaneous thought: eventually we can simply add some hardcoded threshold number here to check if "> NUM_TEST_CASES" are actually run).

So: enough work here. 😄

@holgerd77
Copy link
Member Author

Last test file from BlockchainTests where test run errors and is terminated can be run with:

ts-node ./tests/tester --blockchain --file='bcInvalidRLPTest_BLOCK_'

@holgerd77
Copy link
Member Author

Rebased this to see how this behaves on top of the refactored test runner changes from #752.

@holgerd77
Copy link
Member Author

holgerd77 commented May 26, 2020

State tests still pass, 77 blockchain tests fail (see raw log and do a fulltext search for "not ok").

The expectException tests seem to fail again on first look (not confirmed though), my suspicion here is that there might have been a change in the test data - eventually location of the field has moved or something.

Simplest check here is to add a console.log(testdata) in the blockchain test runner - e.g. here if this is the assumed execution flow.

@holgerd77 holgerd77 changed the title ethereumjs-testing v1.3.1 (pre-Berlin HF release) test run Update to ethereumjs-testing v1.3.1 (pre-Berlin HF release) / Fix test runs May 27, 2020
@holgerd77
Copy link
Member Author

We can actually change the semantics from this PR and directly work here to fix the test runs, so everyone with a fix can do PRs towards this branch.

@holgerd77
Copy link
Member Author

(have updated the PR title accordingly)

@holgerd77
Copy link
Member Author

There seem to be two naming conventions present to indicate an exception present on all HFs in the v1.3.1 state of the tests repository (so both exceptException and exceptExceptionALL).

Have pushed a fix and re-triggered the test run.

@cgewecke
Copy link
Contributor

cgewecke commented May 28, 2020

Failing:

sha3_bigOffset test: sha3_bigOffset_Istanbul
sha3_bigOffset2 test: sha3_bigOffset2_Istanbul
sha3_memSizeNoQuadraticCost31 test: sha3_memSizeNoQuadraticCost31_Istanbul
sha3_memSizeQuadraticCost32 test: sha3_memSizeQuadraticCost32_Istanbul
sha3_memSizeQuadraticCost32_zeroSize test: sha3_memSizeQuadraticCost32_zeroSize_Istanbul
sha3_memSizeQuadraticCost33 test: sha3_memSizeQuadraticCost33_Istanbul
sha3_memSizeQuadraticCost63 test: sha3_memSizeQuadraticCost63_Istanbul
sha3_memSizeQuadraticCost64 test: sha3_memSizeQuadraticCost64_Istanbul
sha3_memSizeQuadraticCost64_2 test: sha3_memSizeQuadraticCost64_2_Istanbul
sha3_memSizeQuadraticCost65 test: sha3_memSizeQuadraticCost65_Istanbul

All of these cases have the starting balance below and list transactions that include value amounts.

  "balance" : "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"

ethereum/test 690 asks about why these balance values are expected to overflow when sent to in the quadratic cost tests and it's possible these are failing because of #720.

There's a PR to reduce the sha3 test balances in ethereum/test 688

@holgerd77
Copy link
Member Author

@cgewecke Ok, thanks! Have added the SHA3 tests to the skip list so that we can merge here for now. Then we can independently observe the further development on this.

@evertonfraga evertonfraga merged commit 256d656 into master Jun 1, 2020
@evertonfraga evertonfraga deleted the run-tests-v131 branch June 1, 2020 15:21
@evertonfraga evertonfraga added this to the VM v5 milestone Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants