From b33b9a6fefbe832bf45a6c7717d0537f27597bff Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 25 Oct 2013 03:52:53 -0400 Subject: [PATCH 1/4] Fix comparison tool by asking for blocks more aggressively --- src/main.cpp | 4 ++-- src/main.h | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 6ffbc5a44..56bd7a5cf 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2355,7 +2355,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo uniqueTx.insert(block.GetTxHash(i)); } if (uniqueTx.size() != block.vtx.size()) - return state.DoS(100, error("CheckBlock() : duplicate transaction")); + return state.DoS(100, error("CheckBlock() : duplicate transaction"), true); unsigned int nSigOps = 0; BOOST_FOREACH(const CTransaction& tx, block.vtx) @@ -3783,7 +3783,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) LOCK(cs_main); CValidationState state; - if (ProcessBlock(state, pfrom, &block)) + if (ProcessBlock(state, pfrom, &block) || state.CorruptionPossible()) mapAlreadyAskedFor.erase(inv); int nDoS = 0; if (state.IsInvalid(nDoS)) diff --git a/src/main.h b/src/main.h index 76de47071..2a67747b4 100644 --- a/src/main.h +++ b/src/main.h @@ -946,13 +946,15 @@ private: MODE_ERROR, // run-time error } mode; int nDoS; + bool corruptionPossible; public: CValidationState() : mode(MODE_VALID), nDoS(0) {} - bool DoS(int level, bool ret = false) { + bool DoS(int level, bool ret = false, bool corruptionIn = false) { if (mode == MODE_ERROR) return ret; nDoS += level; mode = MODE_INVALID; + corruptionPossible = corruptionIn; return ret; } bool Invalid(bool ret = false) { @@ -982,6 +984,9 @@ public: } return false; } + bool CorruptionPossible() { + return corruptionPossible; + } }; /** An in-memory indexed chain of blocks. */ From 95fa14da69b6345f087991ceedcc34c73e66bab8 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 27 Oct 2013 01:47:12 -0400 Subject: [PATCH 2/4] Re-enable BitcoindComparisonTool: * Use the latest version, with limited memory usage, and path to on-disk db (try mouting qa/tmp on a tmpfs)\ * enable -debug=net * re-enable BitcoindComparisonTool in pull-tester --- Makefile.am | 6 ++++-- qa/pull-tester/build-tests.sh.in | 5 +---- qa/pull-tester/run-bitcoind-for-test.sh.in | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Makefile.am b/Makefile.am index dc0e6d8f7..e5bc9b99c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -118,7 +118,8 @@ test_bitcoin_filtered.info: test_bitcoin.info $(LCOV) -r $< "/usr/include/*" -o $@ block_test.info: test_bitcoin_filtered.info - -@TIMEOUT=15 qa/pull-tester/run-bitcoind-for-test.sh $(JAVA) -Xmx2G -jar $(JAVA_COMPARISON_TOOL) 1 18444 + $(MKDIR_P) qa/tmp + -@TIMEOUT=15 qa/pull-tester/run-bitcoind-for-test.sh $(JAVA) -jar $(JAVA_COMPARISON_TOOL) qa/tmp/compTool 1 18444 $(LCOV) -c -d $(abs_builddir)/src --t BitcoinJBlockTest -o $@ $(LCOV) -z -d $(abs_builddir)/src $(LCOV) -z -d $(abs_builddir)/src/leveldb @@ -146,7 +147,8 @@ endif if USE_COMPARISON_TOOL check-local: - @qa/pull-tester/run-bitcoind-for-test.sh $(JAVA) -Xmx2G -jar $(JAVA_COMPARISON_TOOL) 1 18444 + $(MKDIR_P) qa/tmp + @qa/pull-tester/run-bitcoind-for-test.sh $(JAVA) -jar $(JAVA_COMPARISON_TOOL) qa/tmp/compTool 1 18444 endif EXTRA_DIST = $(top_srcdir)/share/genbuild.sh qa/pull-tester/pull-tester.sh $(WINDOWS_PACKAGING) $(OSX_PACKAGING) diff --git a/qa/pull-tester/build-tests.sh.in b/qa/pull-tester/build-tests.sh.in index 461e7be04..392ff12dc 100755 --- a/qa/pull-tester/build-tests.sh.in +++ b/qa/pull-tester/build-tests.sh.in @@ -32,10 +32,7 @@ cd @abs_top_srcdir@ make distdir mv $DISTDIR linux-build cd linux-build -# TODO: re-enable blockchain tester tool, as of 11 Oct 2013 is it not working properly -# on the pull-tester machine. -#./configure --disable-silent-rules --disable-ccache --with-comparison-tool="$JAVA_COMPARISON_TOOL" -./configure --disable-silent-rules --disable-ccache +./configure --disable-silent-rules --disable-ccache --with-comparison-tool="$JAVA_COMPARISON_TOOL" make -j$JOBS # link interesting binaries to parent out/ directory, if it exists. Do this before diff --git a/qa/pull-tester/run-bitcoind-for-test.sh.in b/qa/pull-tester/run-bitcoind-for-test.sh.in index e02fef3b5..e8bcb4bf7 100755 --- a/qa/pull-tester/run-bitcoind-for-test.sh.in +++ b/qa/pull-tester/run-bitcoind-for-test.sh.in @@ -5,7 +5,7 @@ mkdir -p "$DATADIR"/regtest touch "$DATADIR/regtest/debug.log" tail -q -n 1 -F "$DATADIR/regtest/debug.log" | grep -m 1 -q "Done loading" & WAITER=$! -"@abs_top_builddir@/src/bitcoind@EXEEXT@" -connect=0.0.0.0 -datadir="$DATADIR" -rpcuser=user -rpcpassword=pass -listen -keypool=3 -debug -logtimestamps -port=18444 -regtest & +"@abs_top_builddir@/src/bitcoind@EXEEXT@" -connect=0.0.0.0 -datadir="$DATADIR" -rpcuser=user -rpcpassword=pass -listen -keypool=3 -debug -debug=net -logtimestamps -port=18444 -regtest & BITCOIND=$! #Install a watchdog. From 47b9374e39a00d6e981aa1f4d0175d9426d91ac8 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 28 Oct 2013 01:16:41 -0400 Subject: [PATCH 3/4] Make large-reorg tests optional in block-tester --- Makefile.am | 4 ++-- configure.ac | 17 ++++++++++++++++- qa/pull-tester/build-tests.sh.in | 11 ++++++++--- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/Makefile.am b/Makefile.am index e5bc9b99c..bb950ee9f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -119,7 +119,7 @@ test_bitcoin_filtered.info: test_bitcoin.info block_test.info: test_bitcoin_filtered.info $(MKDIR_P) qa/tmp - -@TIMEOUT=15 qa/pull-tester/run-bitcoind-for-test.sh $(JAVA) -jar $(JAVA_COMPARISON_TOOL) qa/tmp/compTool 1 18444 + -@TIMEOUT=15 qa/pull-tester/run-bitcoind-for-test.sh $(JAVA) -jar $(JAVA_COMPARISON_TOOL) qa/tmp/compTool 0 18444 $(LCOV) -c -d $(abs_builddir)/src --t BitcoinJBlockTest -o $@ $(LCOV) -z -d $(abs_builddir)/src $(LCOV) -z -d $(abs_builddir)/src/leveldb @@ -148,7 +148,7 @@ endif if USE_COMPARISON_TOOL check-local: $(MKDIR_P) qa/tmp - @qa/pull-tester/run-bitcoind-for-test.sh $(JAVA) -jar $(JAVA_COMPARISON_TOOL) qa/tmp/compTool 1 18444 + @qa/pull-tester/run-bitcoind-for-test.sh $(JAVA) -jar $(JAVA_COMPARISON_TOOL) qa/tmp/compTool $(COMPARISON_TOOL_REORG_TESTS) 18444 endif EXTRA_DIST = $(top_srcdir)/share/genbuild.sh qa/pull-tester/pull-tester.sh $(WINDOWS_PACKAGING) $(OSX_PACKAGING) diff --git a/configure.ac b/configure.ac index 905acd573..f6d870f33 100644 --- a/configure.ac +++ b/configure.ac @@ -69,10 +69,15 @@ AC_ARG_ENABLE(tests, [use_tests=yes]) AC_ARG_WITH([comparison-tool], - AS_HELP_STRING([with-comparison-tool],[path to java comparison tool (requires --enable-tests)]), + AS_HELP_STRING([--with-comparison-tool],[path to java comparison tool (requires --enable-tests)]), [use_comparison_tool=$withval], [use_comparison_tool=no]) +AC_ARG_ENABLE([comparison-tool-reorg-tests], + AS_HELP_STRING([--enable-comparison-tool-reorg-tests],[enable expensive reorg tests in the comparison tool (default no)]), + [use_comparison_tool_reorg_tests=$enableval], + [use_comparison_tool_reorg_tests=no]) + AC_ARG_WITH([qrencode], [AS_HELP_STRING([--with-qrencode], [enable QR code support (default is yes if qt is enabled and libqrencode is found)])], @@ -235,6 +240,15 @@ if test x$use_comparison_tool != xno; then AC_SUBST(JAVA_COMPARISON_TOOL, $use_comparison_tool) fi +if test x$use_comparison_tool_reorg_tests != xno; then + if test x$use_comparison_tool == x; then + AC_MSG_ERROR("comparison tool reorg tests but comparison tool was not specified") + fi + AC_SUBST(COMPARISON_TOOL_REORG_TESTS, 1) +else + AC_SUBST(COMPARISON_TOOL_REORG_TESTS, 0) +fi + if test x$use_lcov == xyes; then if test x$LCOV == x; then AC_MSG_ERROR("lcov testing requested but lcov not found") @@ -678,6 +692,7 @@ AM_CONDITIONAL([TARGET_WINDOWS], [test x$TARGET_OS = xwindows]) AM_CONDITIONAL([USE_QRCODE], [test x$use_qr = xyes]) AM_CONDITIONAL([USE_LCOV],[test x$use_lcov == xyes]) AM_CONDITIONAL([USE_COMPARISON_TOOL],[test x$use_comparison_tool != xno]) +AM_CONDITIONAL([USE_COMPARISON_TOOL_REORG_TESTS],[test x$use_comparison_tool_reorg_test != xno]) AC_DEFINE(CLIENT_VERSION_MAJOR, _CLIENT_VERSION_MAJOR, [Major version]) AC_DEFINE(CLIENT_VERSION_MINOR, _CLIENT_VERSION_MINOR, [Minor version]) diff --git a/qa/pull-tester/build-tests.sh.in b/qa/pull-tester/build-tests.sh.in index 392ff12dc..9640bcaf6 100755 --- a/qa/pull-tester/build-tests.sh.in +++ b/qa/pull-tester/build-tests.sh.in @@ -9,8 +9,9 @@ set -o xtrace MINGWPREFIX=$1 JAVA_COMPARISON_TOOL=$2 -JOBS=${3-1} -OUT_DIR=${4-} +RUN_LARGE_REORGS=$3 +JOBS=${4-1} +OUT_DIR=${5-} if [ $# -lt 2 ]; then echo "Usage: $0 [mingw-prefix] [java-comparison-tool] " @@ -32,7 +33,11 @@ cd @abs_top_srcdir@ make distdir mv $DISTDIR linux-build cd linux-build -./configure --disable-silent-rules --disable-ccache --with-comparison-tool="$JAVA_COMPARISON_TOOL" +if [ $RUN_LARGE_REORGS = 1 ]; then + ./configure --disable-silent-rules --disable-ccache --with-comparison-tool="$JAVA_COMPARISON_TOOL" --enable-comparison-tool-reorg-tests +else + ./configure --disable-silent-rules --disable-ccache --with-comparison-tool="$JAVA_COMPARISON_TOOL" +fi make -j$JOBS # link interesting binaries to parent out/ directory, if it exists. Do this before From a27253dc007faaa37efa090a26c2515522efc141 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 28 Oct 2013 01:20:09 -0400 Subject: [PATCH 4/4] pull-tester.py: Re-enable coverage msg, new args to run test script --- qa/pull-tester/pull-tester.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/qa/pull-tester/pull-tester.py b/qa/pull-tester/pull-tester.py index 34dd74c7e..6a7c45ccd 100755 --- a/qa/pull-tester/pull-tester.py +++ b/qa/pull-tester/pull-tester.py @@ -67,7 +67,12 @@ Contact BlueMatt on freenode if something looks broken.""" auth=(os.environ['GITHUB_USER'], os.environ["GITHUB_AUTH_TOKEN"])) if success == True: - post_data = { "body" : "Automatic sanity-testing: PASSED, see " + linkUrl + " for binaries and test log." + common_message} + if needTests: + message = "Automatic sanity-testing: PLEASE ADD TEST-CASES, though technically passed. See " + linkUrl + " for binaries and test log." + else: + message = "Automatic sanity-testing: PASSED, see " + linkUrl + " for binaries and test log." + + post_data = { "body" : message + common_message} elif inMerge: post_data = { "body" : "Automatic sanity-testing: FAILED MERGE, see " + linkUrl + " for test log." + """ @@ -113,7 +118,7 @@ def testpull(number, comment_url, clone_url, commit): run("chown -R ${BUILD_USER}:${BUILD_GROUP} ${CHROOT_COPY}/${OUT_DIR}", fail_hard=False) script = os.environ["BUILD_PATH"]+"/qa/pull-tester/pull-tester.sh" - script += " ${BUILD_PATH} ${MINGW_DEPS_DIR} ${SCRIPTS_DIR}/BitcoindComparisonTool.jar 6 ${OUT_DIR}" + script += " ${BUILD_PATH} ${MINGW_DEPS_DIR} ${SCRIPTS_DIR}/BitcoindComparisonTool_jar/BitcoindComparisonTool.jar 0 6 ${OUT_DIR}" returncode = run("chroot ${CHROOT_COPY} sudo -u ${BUILD_USER} -H timeout ${TEST_TIMEOUT} "+script, fail_hard=False, stdout=out, stderr=out)