Skip to content

Conversation

@mrbandrews
Copy link
Contributor

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

@jonasschnelli
Copy link
Contributor

Nice!
Concept ACK.

@sipa
Copy link
Member

sipa commented Jun 2, 2016

Why is it necessary to accept/ignore overly long encoded varints?

@sdaftuar
Copy link
Member

sdaftuar commented Jun 3, 2016

@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.

@laanwj
Copy link
Member

laanwj commented Jun 7, 2016

Awesome! Concept ACK.

@laanwj
Copy link
Member

laanwj commented Jun 10, 2016

Anything holding back merging this?

@theuni
Copy link
Member

theuni commented Jun 10, 2016

It'd be helpful to know what of the Java test remains to be ported.

@mrbandrews
Copy link
Contributor Author

Just the "expensive test," which consists of: "Test massive reorgs (in terms of tx count)"

@sdaftuar
Copy link
Member

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.

@theuni
Copy link
Member

theuni commented Jun 10, 2016

@sdaftuar agreed.

@mrbandrews To my knowledge, the "expensive" test has been broken for ages and has not been used in any automated testing.

@sipa
Copy link
Member

sipa commented Jun 11, 2016

Concept ACK

@sipa
Copy link
Member

sipa commented Jun 11, 2016

Agree with @sdaftuar. Some tests with intentional bugs (including historical actual bugs) would be nice.

@laanwj
Copy link
Member

laanwj commented Jun 13, 2016

It'd be helpful to know what of the Java test remains to be ported.

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.

@laanwj
Copy link
Member

laanwj commented Jun 13, 2016

utACK ff2dcf2

@laanwj laanwj merged commit ff2dcf2 into bitcoin:master Jun 13, 2016
laanwj added a commit that referenced this pull request Jun 13, 2016
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)
@mrbandrews mrbandrews deleted the ba-comptool branch June 14, 2016 15:25
codablock pushed a commit to codablock/dash that referenced this pull request Dec 22, 2017
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)
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
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)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants