-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Continuing port of java comparison tool #8141
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
Conversation
|
Nice! |
|
Why is it necessary to accept/ignore overly long encoded varints? |
|
@sipa I think the right test would be to make sure that receiving a block with an overly long encoded varint doesn't cause the block to be marked permanently bad. The test included here is overly prescriptive by requiring that the overly long encoding be rejected; we should just loosen that and still check that the normally encoded block be accepted. |
|
Awesome! Concept ACK. |
|
Anything holding back merging this? |
|
It'd be helpful to know what of the Java test remains to be ported. |
|
Just the "expensive test," which consists of: "Test massive reorgs (in terms of tx count)" |
|
ACK I've looked through this test and the java test, and I believe this faithfully implements all the consensus tests from the java comparison tool that we run in travis. I think before we turn off running the java tool in travis, we should first do more careful experiments to make sure that the java tool wasn't catching things that this python test would miss, say by introducing intentional bugs to bitcoind and verifying that this test fails. I haven't done this at all yet. |
|
@sdaftuar agreed. @mrbandrews To my knowledge, the "expensive" test has been broken for ages and has not been used in any automated testing. |
|
Concept ACK |
|
Agree with @sdaftuar. Some tests with intentional bugs (including historical actual bugs) would be nice. |
But not a requirement for merging this, as this doesn't disable the java tester. Improving the python tests is always good, independent from anything else. Although the deadline for that is approaching too. |
|
utACK ff2dcf2 |
ff2dcf2 Tests: Edit bloated varint test and add option for 'barely expensive' tests (mrbandrews) 12c5a16 Catch exceptions from non-canonical encoding and print only to log (mrbandrews) 291f8aa Continuing port of java comptool (mrbandrews) 8c9e681 Tests: Rework blockstore to avoid re-serialization. (mrbandrews)
ff2dcf2 Tests: Edit bloated varint test and add option for 'barely expensive' tests (mrbandrews) 12c5a16 Catch exceptions from non-canonical encoding and print only to log (mrbandrews) 291f8aa Continuing port of java comptool (mrbandrews) 8c9e681 Tests: Rework blockstore to avoid re-serialization. (mrbandrews)
ff2dcf2 Tests: Edit bloated varint test and add option for 'barely expensive' tests (mrbandrews) 12c5a16 Catch exceptions from non-canonical encoding and print only to log (mrbandrews) 291f8aa Continuing port of java comptool (mrbandrews) 8c9e681 Tests: Rework blockstore to avoid re-serialization. (mrbandrews)
This continues the port of the java comparison tool to python.
Changes to the blockstore and a small change in main.cpp are in separate commits.
This includes and runs both the "inexpensive" and "barely expensive" tests. The "barely expensive" test (which consists of a few re-orgs) take a minute or two on my machine.
Java test is here:
https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java