[POC] Introducing property based testing to Core

Christewart commented at 1:48 am on August 6, 2016: member

This pull request is a proof of concept for introducting property based testing into Bitcoin Core

In QuickCheck the programmer writes assertions about logical properties that a function should fulfill. Then QuickCheck attempts to generate a test case that falsifies these assertions. Once such a test case is found, QuickCheck tries to reduce it to a minimal failing subset by removing or simplifying input data that are not needed to make the test fail.

This has been very useful for a Bitcoin library I’ve been working on and thought it would be worthwhile to develop a POC for Bitcoin Core. The property based library I am using for C++ is called rapidcheck. Here are the docs.

This pull request currently contains two properties, one testing CKey generation and the other testing serialization symmetry for CKey and CBitcoinSecret. These are rather trivial properties, but useful for illustrating the power of property based testing if there was a bug inside of Core.

I want to solicit some feedback from developers if this is something that would actually be merged into Core. Eventually we could have a large library of generators that would allow us to quickly prototype, test, and reason about the behavior of new code added to Core. Here is an example of a library of generators (in Scala) that could give you a little more of an idea of what I am talking about.

Thoughts?

in depends/packages/rapidcheck.mk: in 6f2af6be0e outdated

0@@ -0,0 +1,21 @@
1+package=rapidcheck
2+
3+$(package)_version:1.0
4+
5+$(package)_download_path:https://github.com/Christewart/rapidcheck/releases/download/1.0
6+
7+$(package)_file_name:rapidcheck-1.0.tar.gz

fanquake commented at 2:33 am on August 8, 2016:

You should be able to use something like $(package)-$($(package)_version) here.

in depends/packages/rapidcheck.mk: in 6f2af6be0e outdated

0@@ -0,0 +1,21 @@
1+package=rapidcheck
2+

fanquake commented at 2:33 am on August 8, 2016:

No need for the empty lines.

in depends/packages/rapidcheck.mk: in 6f2af6be0e outdated

0@@ -0,0 +1,21 @@
1+package=rapidcheck
2+
3+$(package)_version:1.0

fanquake commented at 2:34 am on August 8, 2016:

Use = instead of : here, as well as the lines below.

in configure.ac: in 6f2af6be0e outdated

507@@ -508,7 +508,7 @@ if test x$TARGET_OS = xdarwin; then
508   AX_CHECK_LINK_FLAG([[-Wl,-dead_strip]], [LDFLAGS="$LDFLAGS -Wl,-dead_strip"])
509 fi
510 
511-AC_CHECK_HEADERS([endian.h sys/endian.h byteswap.h stdio.h stdlib.h unistd.h strings.h sys/types.h sys/stat.h sys/select.h sys/prctl.h])
512+AC_CHECK_HEADERS([endian.h sys/endian.h byteswap.h stdio.h stdlib.h unistd.h strings.h sys/types.h sys/stat.h sys/select.h sys/prctl.h ])

fanquake commented at 2:34 am on August 8, 2016:

nit: space introduced here.

fanquake commented at 2:35 am on August 8, 2016: member

This needs a rebase/conflicts fixed.

in src/test/key_properties.cpp: in 6f2af6be0e outdated

0@@ -0,0 +1,48 @@
1+// Copyright (c) 2012-2015 The Bitcoin Core developers

fanquake commented at 2:36 am on August 8, 2016:

nit: Just 2016

in depends/packages/rapidcheck.mk: in 6f2af6be0e outdated

 5+$(package)_download_path:https://github.com/Christewart/rapidcheck/releases/download/1.0
 6+
 7+$(package)_file_name:rapidcheck-1.0.tar.gz
 8+
 9+$(package)_sha256_hash:c228dc21ec24618bfb6afa31d622d1f4ea71168f04ee499e1ffcfc63cd5833f4
10+

fanquake commented at 2:37 am on August 8, 2016:

Does this need $(package)_build_subdir=build ?

Christewart commented at 10:00 pm on August 21, 2016:

Not sure what you mean by this, do you mean we don’t need lines 11 - 21?

MarcoFalke commented at 8:07 am on August 8, 2016: member

I think this approach of testing is interesting and may render useful for bitcoin. The user guide https://github.com/emil-e/rapidcheck/blob/master/doc/user_guide.md mentions that the API is not finalized, so it may be better to integrate it via a subtree (maybe in /test)?

jonasschnelli commented at 8:09 am on August 8, 2016: contributor

I think this is useful. I’m not very familiar with property based testing, but this seems to be something that would make sense for consensus and security critical projects.

Some thoughts:

the rapidcheck dependency should probably only be required if one passes --enable-tests.
I think you can remove the changes in src/test/script_tests.cpp and its header.

If others also agree to take this into master, we should probably have a first logical slice of property based tests (maybe serialization).

Needs rebase.

jonasschnelli added the label Feature on Aug 8, 2016

jonasschnelli added the label Tests on Aug 8, 2016

jonasschnelli removed the label Feature on Aug 8, 2016

MarcoFalke commented at 4:50 pm on August 18, 2016: member

@theuni Can you help here to decide if using depends is fine or a subtree would be cleaner? (I prefer a subtree per my above comment, but I am not familiar with the build system. Input is appreciated)

Christewart commented at 5:09 pm on August 18, 2016: member

I would probably need help with the build code to get it production ready. I plan to keep on adding properties to this pull request though when time permits. I’m going to try and get around implementing fanquake’s comments and figure out how to do the --enable-test as per jonasschnelli’s comment this weekend

in depends/packages/rapidcheck.mk: in 29d90f6617 outdated

0@@ -0,0 +1,21 @@
1+package=rapidcheck
2+
3+$(package)_version:1.0
4+
5+$(package)_download_path:https://github.com/Christewart/rapidcheck/releases/download/1.0

MarcoFalke commented at 10:52 am on August 19, 2016:

Again, I think a subtree is the better choice per my previous comment. Also, I don’t think we can have someone verify this binary each time it is bumped, even if it was done deterministically.

theuni commented at 6:37 pm on August 19, 2016:

Not sure what you mean about the deterministic bump. This is just a source tarball. Verifying would be no different than any of the other depends.

The general rule of thumb, though, is whether builders will be able to use it without any extra work. Since rapidcheck isn’t present in any distros (as far as I can see), the answer to that is “no”.

So, unless we’re willing to require a depends build to run the new tests, a subtree would be easiest. However.

Tests are a bit of a special case. It may be reasonable to keep this in depends for now and let the c-i run it until there’s sufficient coverage.

There’s also the fact that it uses CMake, which we really don’t want to introduce into our build (nothing against CMake, we just don’t want to require every build tool under the sun).

MarcoFalke commented at 7:31 pm on August 19, 2016:

Ok, fine with me. Thanks for letting us know.

Christewart force-pushed on Aug 21, 2016

in depends/packages/rapidcheck.mk: in 0a658bbe30 outdated

0@@ -0,0 +1,21 @@
1+package=rapidcheck
2+
3+$(package)_version=1.0
4+
5+$(package)_download_path=https://github.com/Christewart/rapidcheck/releases/download/1.0
6+
7+$(package)_file_name=$(package)-$($(package)_version).tar.gz
8+
9+$(package)_sha256_hash=c228dc21ec24618bfb6afa31d622d1f4ea71168f04ee499e1ffcfc63cd5833f4

MarcoFalke commented at 10:13 am on August 24, 2016:

Can you try to adjust the folder name within the targz to have the version appened?

This may fix the travis issue.

Christewart commented at 3:08 pm on August 24, 2016:

I should note that I did NOT get any of this deterministic build working on my machine, I was trying to copy what I have seen on other other files to no avail – I just ended up adding rapidcheck to my includes directory on my local machine. I’m fairly new to C++ & it’s build systems so I was trying to just hack the pieces together. So I wouldn’t be surprised if there are deeper issues than a naming issue although I will try and look at that later today. What exact name are you talking about? Are you talking about line 7? Line 5?

MarcoFalke commented at 3:17 pm on August 24, 2016:

No worries, I don’t understand cpp or the build system either. I can take a look later today… I think the issue is within the package you uploaded.

MarcoFalke commented at 5:20 pm on August 25, 2016:

@Christewart Please try to fetch https://github.com/MarcoFalke/bitcoin/commit/befb827ccd12267083921e866b97179ba9899902 (Mf1608-rapidcheckRework) and push it here. This should fix your compile issues for now.

Please note that travis will likely reject this due to a bug in gcc4.8.x: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55914

in depends/packages/rapidcheck.mk: in befb827ccd outdated

11-$(package)_sha256_hash=c228dc21ec24618bfb6afa31d622d1f4ea71168f04ee499e1ffcfc63cd5833f4
12-
13-define $(package)_preprocess_cmds
14-  mkdir build
15-endef
16+$(package)_sha256_hash=78cdb8d0185b602e32e66f4e5d1a6ceec1f801dd9641b8a9456c386b1eaaf0e5

Christewart commented at 12:38 pm on August 26, 2016:

Why is this hash different than the one I provided?

MarcoFalke commented at 5:44 pm on August 26, 2016:

The file size is different, so ~~must~~ should the hash.

The targz you provided also included the 1.8 MB .git subfolder, which is not required.

The hash I provided should be the hash of the tarball created by GitHub: https://github.com/emil-e/rapidcheck/archive/f5d3afa4f387ecf147faf98d96710a6edfa420f1.tar.gz

Christewart commented at 10:33 pm on August 26, 2016:

Ok that makes sense. Didn’t realize you removed files. Do you have any idea why this is still failing?

MarcoFalke commented at 7:55 am on August 29, 2016: member

@Christewart Do you need help to address the feedback from @jonasschnelli?

Christewart commented at 12:42 pm on August 29, 2016: member

@MarcoFalke Yeah I’m not really sure how to get that done

in src/test/transaction_properties.cpp: in 130d45fd22 outdated

49+/** Check CTransaction serialization symmetry */
50+RC_BOOST_PROP(ctransaction_serialization_symmetry, (CTransaction tx)) {
51+  CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
52+  ss << tx;
53+  deserialize_type t; 
54+  CTransaction tx2(t,ss);

sipa commented at 1:46 am on January 8, 2017:

The intended idiom is just

0CTransaction tx2(deserialize, ss);

gmaxwell commented at 5:19 am on January 8, 2017: contributor

I like this kind of testing– testing invariants on random test cases–, and we use it extensively in libsecp256k1. It’s one of the answers to my irritation with the way many of the unit tests in Bitcoin core have worked in the past: hard-coding the exact behavior. Meaning that if you change anything, the tests all fail and you’re left wondering if you broke something important or if the test was just mandating the behavior you were fixing. In either case, updating the test is often more work than the change.

Taking a dependency for it seems unfortunate; but I’m not qualified to judge if it does snazzy things that make generating cases easy. I have had bad experience with test frameworks in the past which had very high overhead making test cases that should have taken seconds take minutes, resulting in less testing. I see in this one that it uses a excessively slow PRNG based on skein, which makes me a bit dubious– but if thats the only problem it would be easy to fix if we forked it (by replacing it with xorshift128+ or likewise).

Christewart commented at 4:45 pm on January 8, 2017: member

It should also be noted that they way the test are currently written they tie in the boost testing framework, if we add rapidcheck as a dependency and decide to remove boost, which #8670 suggests, we will have to rewrite these test cases to be independent of boost.

In terms of speed, you would know more than I would about choosing PRNG. Rapidcheck currently defaults to running 100 test cases against every property you have specified. You can configure this property to be higher or lower depending on the situation. I think it would make sense to configure this to lower value for local development, and a higher value for travis builds to try and exhaustively test our invariants. You can read more about configuration of rapidcheck here. @jonasschnelli ’s comment above was interesting I thought, I’m not super familiar with C++ build systems but only including the rapidcheck dependency if the flag --enable-test flag was given seems to makes sense.

I think the long term value add here is having a library of generators (for various protocol data structures) that we can use to test new protocol changes. I’m going to be adding some properties about creating tx types in the coming weeks. To get an idea of what I’m looking to add, you can look at some of the generators I have created in bitcoin-s.

Christewart force-pushed on Apr 7, 2017

Christewart force-pushed on May 1, 2017

Christewart force-pushed on May 29, 2017

TheBlueMatt commented at 6:08 pm on July 11, 2017: member

Hmm, looks like travis is failing to build rapidcheck here? Are you interested in rebasing and potentially fixing it?

Christewart commented at 3:53 pm on July 12, 2017: member

It appears there is a bug with gcc 4.8 that causes a problems with lambdas and parameter packs:

https://stackoverflow.com/questions/45062090/gcc-4-8-4-error-parameter-packs-not-expanded-with?noredirect=1#comment77097698_45062090

https://stackoverflow.com/questions/22568642/parameter-packs-not-expanded-with-another-variadic-template-bug-with-gc

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41933

sipa referenced this in commit fee0d803fb on Jul 17, 2017

MarcoFalke added this to the milestone 0.16.0 on Aug 9, 2017

MarcoFalke commented at 11:07 am on August 9, 2017: member

Assigned 0.16, as we might add gcc to depends by then.

Christewart force-pushed on Oct 8, 2017

Christewart commented at 4:20 pm on October 12, 2017: member

An update about commits 56edb65 and d97b980:

These commits add generators for various scriptPubKey types
Using the script generators, I’ve created generators for various transaction types
Using the transaction generators, I’ve created new test cases specifying this invariant: If we use ProduceSignature to create a transaction, it should always be valid when run through the interpreter. Here is the test cases

I’m hoping this illustrates the power that property based testing can be used for once we you have a small set of primitive generators. You can keep building more and more complex data structures on these primitives and then specify invariants about various functions that use those data structures.

MarcoFalke removed this from the milestone 0.16.0 on Nov 22, 2017

MarcoFalke added this to the milestone 0.17.0 on Nov 22, 2017

fanquake commented at 11:42 am on November 29, 2017: member

This is using a somewhat outdated version of rapidcheck from March last year. Can this PR be updated to point to a newer version?

fanquake assigned theuni on Nov 29, 2017

in configure.ac:746 in d97b9806aa outdated

718@@ -719,6 +719,7 @@ if test x$use_upnp != xno; then
719   )
720 fi
721 
722+

fanquake commented at 11:43 am on November 29, 2017:

nit: empty line

in depends/packages/rapidcheck.mk:3 in d97b9806aa outdated

0@@ -0,0 +1,13 @@
1+package=rapidcheck
2+$(package)_version=f5d3afa
3+$(package)_download_path=https://bitcoin-10596.firebaseapp.com/depends

fanquake commented at 11:44 am on November 29, 2017:

We’ll probably want to put this on https://bitcoincore.org/depends-sources

Christewart commented at 8:36 pm on December 31, 2017:

How do I go about this? Is there a process document some where?

MarcoFalke commented at 12:13 pm on January 2, 2018:

This needs to be done by someone who has push access to that folder. Can be done after (or just before) this pull is merged.

Christewart force-pushed on Dec 20, 2017

Christewart force-pushed on Dec 28, 2017

jb55 commented at 8:44 pm on December 31, 2017: member

Very Strong Concept ACK. I use quickcheck a lot in Haskell land, it’s amazing how many bugs it can catch, and with shrinking it can find minimal breaking test cases. I’ll give this PR a spin soon.

Christewart commented at 10:55 pm on December 31, 2017: member

@jb55 I am Chris_Stewart_5 on freenode. Feel free to ping. If you want to see something a little more interesting, I am starting to use this pull request as the basis for testing Mark Friedenbach’s BIP98/116/117 MAST bips.

https://github.com/maaku/bitcoin/pull/2

ryanofsky commented at 6:07 pm on February 9, 2018: member

RE: #8469 (review)

@Christewart Please try to fetch https://github.com/MarcoFalke/bitcoin/commit/befb827ccd12267083921e866b97179ba9899902 (Mf1608-rapidcheckRework) and push it here. This should fix your compile issues for now.

I couldn’t get the current depends build or Marco’s build to work. RapidCheck install steps seem to be missing so the built rapidcheck tarball is completely empty. Following depends/autoconf changes fix the problem for me and also allow normal builds without RapidCheck to work: ca0dbbd64fba9d31d0ff88984529deb4b87229fc

MarcoFalke commented at 7:57 pm on February 9, 2018: member

@ryanofsky Thanks for the autotools fix! Will take a look soon

MarcoFalke commented at 4:05 pm on February 10, 2018: member

I tested ca0dbbd64fba9d31d0ff88984529deb4b87229fc. Some notes:

The cmake dependency should be mentioned in ./depends/README.md (linux and windows)
The build still fails on travis, due to the gcc bug

 0[100%] Building CXX object CMakeFiles/rapidcheck.dir/src/gen/detail/ScaleInteger.cpp.o
 1In file included from /bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/include/rapidcheck/gen/Container.h:30:0,
 2                 from /bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/include/rapidcheck/gen/Text.hpp:6,
 3                 from /bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/include/rapidcheck/gen/Text.h:23,
 4                 from /bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/src/gen/Text.cpp:1:
 5/bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/include/rapidcheck/gen/Container.hpp: In lambda function:
 6/bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/include/rapidcheck/gen/Container.hpp:491:42: error: parameter packs not expanded with '...':
 7     return helper.generate(random, size, gens...);
 8                                          ^
 9/bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/include/rapidcheck/gen/Container.hpp:491:42: note:         'gens'
10/bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/include/rapidcheck/gen/Container.hpp:491:46: error: expansion pattern 'gens' contains no argument packs
11     return helper.generate(random, size, gens...);
12                                              ^
13/bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/include/rapidcheck/gen/Container.hpp: In lambda function:
14/bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/include/rapidcheck/gen/Container.hpp:501:49: error: parameter packs not expanded with '...':
15     return helper.generate(count, random, size, gens...);
16                                                 ^
17/bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/include/rapidcheck/gen/Container.hpp:501:49: note:         'gens'
18/bitcoin/depends/work/build/x86_64-pc-linux-gnu/rapidcheck/f5d3afa-4cda083952b/include/rapidcheck/gen/Container.hpp:501:53: error: expansion pattern 'gens' contains no argument packs
19     return helper.generate(count, random, size, gens...);
20                                                     ^
21make[3]: *** [CMakeFiles/rapidcheck.dir/src/gen/Text.cpp.o] Error 1

MarcoFalke commented at 4:30 pm on February 10, 2018: member

See https://github.com/MarcoFalke/bitcoin/commit/fa9f426eed2b1705cb34f77448333bd700c75a43 for a version bump of rapidcheck

I think we can work around the gcc bug by building only when rapidcheck is explicitly enabled. I.e. implement something that does:

0cd depends && make  # Doesn't build rapidcheck (RAPIDCHECK=0 by default)
1cd depends && make RAPIDCHECK=1  # Builds rapidcheck

Christewart force-pushed on Feb 13, 2018

MarcoFalke commented at 6:15 pm on February 16, 2018: member

Please see https://github.com/MarcoFalke/bitcoin/commit/1862c49a27b2b7ca9c9e3bd2227911bd2d30ab77 on how to disable rapidcheck by default. This should work around the travis issue (finally)

MarcoFalke commented at 6:23 pm on February 16, 2018: member

If you like that commit, you could submit a minimal pull request with only the depends changes and a minimal working rapidcheck example. Merging in baby steps tends to work better.

MarcoFalke commented at 6:08 pm on February 18, 2018: member

You should be able to solve the whitespace issues with clang-format: https://github.com/bitcoin/bitcoin/tree/master/contrib/devtools#clang-format-diffpy

Christewart commented at 6:29 pm on February 18, 2018: member

@MarcoFalke I’ve tried running the script before, and I get this error:

https://pastebin.com/vqS3pt3r

I changed 0 to 22 as we want to run over the last 22 commits i’m guessing? I think it is failing because a bunch of the files in this PR are new in the tree

MarcoFalke commented at 6:34 pm on February 18, 2018: member

What does which clang-format say? Is it properly installed on you system?

in src/test/gen/merkleblock_gen.h:60 in c1f3de1bf4 outdated

55+                return mb;
56+            });
57+    };
58+};
59+
60+/** Generates a CPartialMerkleTree and returns the PartialMerkleTree along

MarcoFalke commented at 7:00 pm on February 18, 2018:

Trailing white space 😛

PierreRochard commented at 6:26 am on March 23, 2018: contributor

Concept ACK, ditto on @gmaxwell’s comment regarding hard-coded behavior

ryanofsky commented at 7:21 pm on March 23, 2018: member

Confirmed this works. Tested with:

0make -C depends RAPIDCHECK=1
1./autogen.sh
2./configure --prefix=$PWD/depends/x86_64-pc-linux-gnu
3make -C src test/test_bitcoin
4src/test/test_bitcoin --log_level=test_suite --run_test="*_properties"

Christewart force-pushed on Mar 24, 2018

Adding rapidcheck dependency, adding CKey properties

Adding rapidcheck dependency, adding CKey properties

Successfully compiling bitcoin with rapidcheck dependency

Adding new property file for CKey

Serialization symmetry for CKey -> CBitcoinSecret -> CKey

Adding generators for CPrivKey, CPubKey, and uint256

[depends] Rework rapidcheck.mk

Adding Bloom filter properties and bloom filter generator

Adding serialization symmetry for CBloomFilter

Adding block header serialization property

Fixing generator bug where I wasn't setting all fields on a BlockHeader

Creating transaction_gen.h, adding generator for COutPoint and serialization symmetry property for COutPoint

Adding script generator and script_propertes, first property is CScript serialization symmetry

Adding CTransaction generator, adding serialization symmetry property for CTransaction

Removing 'oneOrMoreInputs' and 'oneOrMoreOutputs' generators in favor of using rapidcheck's gen::nonEmpty function

Adding CTransactionRef and CBlock generators, adding property for CBlock serialization symmetry

Adding Generators outside of the rc name space, adding generator for LoadedBloomFilter

Adding merkleblock_gen.h, merkleblock serialization symmetry test

Adding merkle_block properties

Committing to try and debug mem leak

Removing comments, rebasing to master

Only adding rapidcheck dependency if tests are enabled

Refactoring inclues in generator files, reordering files in Makefile.test.include

Undoing change in merkleblock.cpp

Adding bloom_gen.cpp

Reworking BetweenZeroAndOne to use rc::gen::map instead of rc::gen::suchThat, removing some unused imports

Adding signedP2PKTx(), signedP2PKHTx(), signedMultisigTx(), and then a generator called signedP2SHTx() that creates a p2sh output and spends it

Adding witness SPKs and witness spending transactions

Making sure helpers are functions and not constant values

fixing nits/ugliness

Adding block_gen.cpp

Modifying code to be closer to the style guide rules, use const/references, fixing more nits

autotools/depends fixes for rapidcheck

Fixes non-depends bitcoin build when rapidcheck is not installed on system,
by adding --with-rapidcheck config option.

Fixes depends build by adding fPIC option and installation steps so the build
is not empty. Allows property-based tests to be run with:

    make -C depends
    ./configure --prefix=$PWD/depends/x86_64-pc-linux-gnu
    make check

depends: Bump rapidcheck to version 10fc0cb

depends: Disable RAPIDCHECK by default

run clang-format

remove trailing white space

remove native ccache in packages.mk

7fd92dade8

Christewart force-pushed on Mar 24, 2018

Christewart commented at 2:41 am on March 25, 2018: member

I believe the travis failure here is unrelated?

laanwj removed this from the milestone 0.17.0 on Jul 19, 2018

DrahtBot commented at 5:30 pm on July 21, 2018: member

DrahtBot closed this on Jul 21, 2018

DrahtBot reopened this on Jul 21, 2018

DrahtBot added the label Needs rebase on Aug 5, 2018

DrahtBot commented at 5:45 pm on August 5, 2018: member

practicalswift commented at 8:18 pm on August 28, 2018: contributor

@Christewart This is excellent work! Do you have time to rebase this and take it to completion, or do you prefer if someone else takes over this work? It is too good to be left unfinished :-)

Christewart commented at 8:23 pm on August 28, 2018: member

Hi @practicalswift it seems like the consensus is moving to #12775 which is a subset of the PR. It includes just the build stuff and a subset of the tests in this PR.

practicalswift commented at 8:38 pm on August 28, 2018: contributor

@Christewart Oh, sorry! That one is rebased and nice! Thanks!

pull[bot] referenced this in commit eea87ef537 on Sep 8, 2018

MarcoFalke removed the label Needs rebase on Sep 9, 2018

DrahtBot commented at 3:51 pm on September 9, 2018: member

DrahtBot added the label Needs rebase on Sep 9, 2018

in src/test/gen/block_gen.h:13 in 7fd92dade8

 8+
 9+#include <rapidcheck/gen/Arbitrary.h>
10+#include <rapidcheck/Gen.h>
11+
12+
13+typedef std::tuple<int32_t, uint256, uint256, uint32_t, uint32_t, uint32_t> BlockHeaderTup;

Empact commented at 11:59 pm on October 8, 2018:

nit: using?

in src/test/gen/bloom_gen.cpp:24 in 7fd92dade8

19+        assert(result >= 0 && result < 1);
20+        return result;
21+    });
22+}
23+
24+rc::Gen<unsigned int> Between1And100()

Empact commented at 0:07 am on October 9, 2018:

nit: looks like this method is defined with the same name here and in merkleblock_gen.h, how about:

applying static more liberally?
renaming one or the other?

Empact commented at 0:30 am on October 9, 2018: member

Moving my review to #14430, which along with #12775 was extracted from this. @Christewart should this be closed?

fanquake commented at 2:39 am on October 10, 2018: member

This was superseded by #12775 and #14430.

fanquake closed this on Oct 10, 2018

fanquake removed the label Needs rebase on Oct 10, 2018

PastaPastaPasta referenced this in commit ddcad06416 on Jul 6, 2019

PastaPastaPasta referenced this in commit 5dd6ebb8b1 on Jul 8, 2019

PastaPastaPasta referenced this in commit bec9fb1324 on Jul 9, 2019

PastaPastaPasta referenced this in commit 479f58c9f4 on Jul 11, 2019

PastaPastaPasta referenced this in commit 0272594216 on Jul 13, 2019

PastaPastaPasta referenced this in commit f1be961886 on Jul 13, 2019

PastaPastaPasta referenced this in commit 220e2506a6 on Jul 17, 2019

PastaPastaPasta referenced this in commit 9ad51edd55 on Jul 23, 2019

PastaPastaPasta referenced this in commit 644274fded on Jul 24, 2019

barrystyle referenced this in commit 88916a17d7 on Jan 22, 2020

MarcoFalke locked this on Sep 8, 2021

[POC] Introducing property based testing to Core #8469