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

Integration of property based testing into Bitcoin Core #12775

Merged
merged 1 commit into from Sep 8, 2018
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -130,6 +130,12 @@ AC_ARG_ENABLE(gui-tests,
[use_gui_tests=$enableval],
[use_gui_tests=$use_tests])

AC_ARG_WITH([rapidcheck],
[AS_HELP_STRING([--with-rapidcheck],
[enable RapidCheck property based tests (default is yes if librapidcheck is found)])],
[use_rapidcheck=$withval],
[use_rapidcheck=auto])

AC_ARG_ENABLE(bench,
AS_HELP_STRING([--disable-bench],[do not compile benchmarks (default is to compile)]),
[use_bench=$enableval],
@@ -1134,6 +1140,22 @@ AC_CHECK_DECLS([EVP_MD_CTX_new],,,[AC_INCLUDES_DEFAULT
])
CXXFLAGS="${save_CXXFLAGS}"

dnl RapidCheck Property Based Testing

enable_property_tests=no
if test "x$use_rapidcheck" = xauto; then
AC_CHECK_HEADERS([rapidcheck.h], [enable_property_tests=yes])
elif test "x$use_rapidcheck" != xno; then
enable_property_tests=yes
fi

RAPIDCHECK_LIBS=
if test "x$enable_property_tests" = xyes; then
RAPIDCHECK_LIBS=-lrapidcheck
fi
AC_SUBST(RAPIDCHECK_LIBS)
AM_CONDITIONAL([ENABLE_PROPERTY_TESTS], [test x$enable_property_tests = xyes])

This comment has been minimized.

Copy link
@laanwj

laanwj Aug 31, 2018

Member

I think something should be logged to the output depending on whether rapidcheck is used or not, currently there is nothing in configure output that suggests this

oh, just noticed in travis that without specifying explicit --with-rapidcheck there is:

checking rapidcheck.h usability... no
checking rapidcheck.h presence... no
checking for rapidcheck.h... no

dnl univalue check

need_bundled_univalue=yes
@@ -5,6 +5,7 @@ WORK_PATH = $(BASEDIR)/work
BASE_CACHE ?= $(BASEDIR)/built
SDK_PATH ?= $(BASEDIR)/SDKs
NO_QT ?=
RAPIDCHECK ?=
NO_WALLET ?=
NO_UPNP ?=
FALLBACK_DOWNLOAD_PATH ?= https://bitcoincore.org/depends-sources
@@ -93,13 +94,19 @@ qt_packages_$(NO_QT) = $(qt_packages) $(qt_$(host_os)_packages) $(qt_$(host_arch
wallet_packages_$(NO_WALLET) = $(wallet_packages)
upnp_packages_$(NO_UPNP) = $(upnp_packages)

rapidcheck_packages_$(RAPIDCHECK) = $(rapidcheck_packages)

packages += $($(host_arch)_$(host_os)_packages) $($(host_os)_packages) $(qt_packages_) $(wallet_packages_) $(upnp_packages_)
native_packages += $($(host_arch)_$(host_os)_native_packages) $($(host_os)_native_packages)

ifneq ($(qt_packages_),)
native_packages += $(qt_native_packages)
endif

ifeq ($(rapidcheck_packages_),)
packages += $(rapidcheck_packages)
endif

all_packages = $(packages) $(native_packages)

meta_depends = Makefile funcs.mk builders/default.mk hosts/default.mk hosts/$(host_os).mk builders/$(build_os).mk
@@ -63,6 +63,7 @@ The following can be set when running make: make FOO=bar
NO_WALLET: Don't download/build/cache libs needed to enable the wallet
NO_UPNP: Don't download/build/cache packages needed for enabling upnp
DEBUG: disable some optimizations and enable more runtime checking
RAPIDCHECK: build rapidcheck (experimental)
HOST_ID_SALT: Optional salt to use when generating host package ids
BUILD_ID_SALT: Optional salt to use when generating build package ids

@@ -5,6 +5,8 @@ qt_packages = qrencode protobuf zlib

qt_linux_packages:=qt expat dbus libxcb xcb_proto libXau xproto freetype fontconfig libX11 xextproto libXext xtrans

rapidcheck_packages = rapidcheck

qt_darwin_packages=qt
qt_mingw32_packages=qt

@@ -0,0 +1,18 @@
package=rapidcheck
$(package)_version=10fc0cb
$(package)_download_path=https://github.com/MarcoFalke/rapidcheck/archive
$(package)_file_name=$(package)-$($(package)_version).tar.gz
$(package)_sha256_hash=9640926223c00af45bce4c7df8b756b5458a89b2ba74cfe3e404467f13ce26df

define $(package)_config_cmds
cmake -DCMAKE_POSITION_INDEPENDENT_CODE:BOOL=true .

This comment has been minimized.

Copy link
@fanquake

fanquake Apr 19, 2018

Member

Looks like we'll need some documentation about installing/having cmake available

Extracting rapidcheck...
/Users/xxx/GitHub/bitcoin/depends/sources/rapidcheck-10fc0cb.tar.gz: OK
Preprocessing rapidcheck...
Configuring rapidcheck...
/bin/sh: cmake: command not found
endef

define $(package)_build_cmds
$(MAKE) && \
mkdir -p $($(package)_staging_dir)$(host_prefix)/include && \

This comment has been minimized.

Copy link
@laanwj

laanwj Aug 31, 2018

Member

indeed, this is necessary because rapidcheck has no make install

This comment has been minimized.

Copy link
@Christewart

Christewart Aug 31, 2018

Author Contributor

This is an open PR on the repo

emil-e/rapidcheck#152

This comment has been minimized.

Copy link
@Empact

Empact Oct 9, 2018

Member

Now that the PR is merged, should this be switched over?

cp -a include/* $($(package)_staging_dir)$(host_prefix)/include/ && \
cp -a extras/boost_test/include/rapidcheck/* $($(package)_staging_dir)$(host_prefix)/include/rapidcheck/ && \
mkdir -p $($(package)_staging_dir)$(host_prefix)/lib && \
cp -a librapidcheck.a $($(package)_staging_dir)$(host_prefix)/lib/

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Sep 9, 2018

Member

Might want to update to the latest rapidcheck and use the make install target?

endef
@@ -8,6 +8,7 @@ TEST_SRCDIR = test
TEST_BINARY=test/test_bitcoin$(EXEEXT)

JSON_TEST_FILES = \
test/data/script_tests.json \

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Sep 9, 2018

Member

Why is this change required?

test/data/base58_encode_decode.json \
test/data/blockfilters.json \
test/data/key_io_valid.json \
@@ -94,6 +95,15 @@ BITCOIN_TESTS =\
test/validation_block_tests.cpp \
test/versionbits_tests.cpp

if ENABLE_PROPERTY_TESTS
BITCOIN_TESTS += \
test/key_properties.cpp

BITCOIN_TEST_SUITE += \
test/gen/crypto_gen.cpp \
test/gen/crypto_gen.h
endif

if ENABLE_WALLET
BITCOIN_TESTS += \
wallet/test/accounting_tests.cpp \
@@ -118,7 +128,7 @@ test_test_bitcoin_LDADD += $(LIBBITCOIN_SERVER) $(LIBBITCOIN_CLI) $(LIBBITCOIN_C
$(LIBLEVELDB) $(LIBLEVELDB_SSE42) $(LIBMEMENV) $(BOOST_LIBS) $(BOOST_UNIT_TEST_FRAMEWORK_LIB) $(LIBSECP256K1) $(EVENT_LIBS) $(EVENT_PTHREADS_LIBS)
test_test_bitcoin_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)

test_test_bitcoin_LDADD += $(LIBBITCOIN_CONSENSUS) $(BDB_LIBS) $(SSL_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS)
test_test_bitcoin_LDADD += $(LIBBITCOIN_CONSENSUS) $(BDB_LIBS) $(SSL_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS) $(RAPIDCHECK_LIBS)
test_test_bitcoin_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) -static

if ENABLE_ZMQ
@@ -0,0 +1,19 @@
// Copyright (c) 2018 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <test/gen/crypto_gen.h>

#include <key.h>

#include <rapidcheck/gen/Arbitrary.h>
#include <rapidcheck/Gen.h>
#include <rapidcheck/gen/Predicate.h>
#include <rapidcheck/gen/Container.h>

/** Generates 1 to 20 keys for OP_CHECKMULTISIG */
rc::Gen<std::vector<CKey>> MultisigKeys()

This comment has been minimized.

Copy link
@practicalswift

practicalswift Sep 6, 2018

Member

MultisigKeys() seems unused? Is that intentional?

This comment has been minimized.

Copy link
@Christewart

Christewart Sep 6, 2018

Author Contributor

I don't think it is used on this PR, but I believe it is used with the remaining test cases in #8469. Basically this PR splits #8469 in half -- there is still a lot of test cases in that PR. Basically once this PR is merged i'm going to open another one with those test cases

This comment has been minimized.

Copy link
@Empact

Empact Oct 9, 2018

Member

I think we should be more careful about adding unused code like this under the expectation that it will be used in the future. YAGNI + concerns about maintaining an informative record in the code history.

{
return rc::gen::suchThat(rc::gen::arbitrary<std::vector<CKey>>(), [](const std::vector<CKey>& keys) {
return keys.size() >= 1 && keys.size() <= 15;
});
};
@@ -0,0 +1,63 @@
// Copyright (c) 2018 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#ifndef BITCOIN_TEST_GEN_CRYPTO_GEN_H
#define BITCOIN_TEST_GEN_CRYPTO_GEN_H

#include <key.h>
#include <random.h>
#include <uint256.h>
#include <rapidcheck/gen/Arbitrary.h>
#include <rapidcheck/Gen.h>
#include <rapidcheck/gen/Create.h>
#include <rapidcheck/gen/Numeric.h>

/** Generates 1 to 15 keys for OP_CHECKMULTISIG */
rc::Gen<std::vector<CKey>> MultisigKeys();

namespace rc
{
/** Generator for a new CKey */
template <>
struct Arbitrary<CKey> {
static Gen<CKey> arbitrary()
{
return rc::gen::map<int>([](int x) {
CKey key;
key.MakeNewKey(true);
return key;
});
};
};

/** Generator for a CPrivKey */
template <>
struct Arbitrary<CPrivKey> {
static Gen<CPrivKey> arbitrary()
{
return gen::map(gen::arbitrary<CKey>(), [](const CKey& key) {
return key.GetPrivKey();
});
};
};

/** Generator for a new CPubKey */
template <>
struct Arbitrary<CPubKey> {
static Gen<CPubKey> arbitrary()
{
return gen::map(gen::arbitrary<CKey>(), [](const CKey& key) {
return key.GetPubKey();
});
};
};
/** Generates a arbitrary uint256 */
template <>
struct Arbitrary<uint256> {
static Gen<uint256> arbitrary()
{
return rc::gen::just(GetRandHash());
};
};
} //namespace rc
#endif
@@ -0,0 +1,53 @@
// Copyright (c) 2018 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <key.h>

#include <base58.h>
#include <script/script.h>
#include <uint256.h>
#include <util.h>
#include <utilstrencodings.h>
#include <test/test_bitcoin.h>
#include <string>
#include <vector>

#include <boost/test/unit_test.hpp>
#include <rapidcheck/boost_test.h>
#include <rapidcheck/gen/Arbitrary.h>
#include <rapidcheck/Gen.h>

#include <test/gen/crypto_gen.h>

BOOST_FIXTURE_TEST_SUITE(key_properties, BasicTestingSetup)

/** Check CKey uniqueness */
RC_BOOST_PROP(key_uniqueness, (const CKey& key1, const CKey& key2))
{
RC_ASSERT(!(key1 == key2));
}

/** Verify that a private key generates the correct public key */
RC_BOOST_PROP(key_generates_correct_pubkey, (const CKey& key))
{
CPubKey pubKey = key.GetPubKey();
RC_ASSERT(key.VerifyPubKey(pubKey));
}

/** Create a CKey using the 'Set' function must give us the same key */
RC_BOOST_PROP(key_set_symmetry, (const CKey& key))
{
CKey key1;
key1.Set(key.begin(), key.end(), key.IsCompressed());
RC_ASSERT(key1 == key);
}

/** Create a CKey, sign a piece of data, then verify it with the public key */
RC_BOOST_PROP(key_sign_symmetry, (const CKey& key, const uint256& hash))
{
std::vector<unsigned char> vchSig;
key.Sign(hash, vchSig, 0);
const CPubKey& pubKey = key.GetPubKey();
RC_ASSERT(pubKey.Verify(hash, vchSig));
}
BOOST_AUTO_TEST_SUITE_END()
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.