diff options
60 files changed, 1292 insertions, 208 deletions
diff --git a/.appveyor.yml b/.appveyor.yml index 777eebd2c3..18dc78fe5f 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -81,7 +81,7 @@ after_build: - ps: clcache -z #- 7z a bitcoin-%APPVEYOR_BUILD_VERSION%.zip %APPVEYOR_BUILD_FOLDER%\build_msvc\%platform%\%configuration%\*.exe test_script: -- cmd: src\test_bitcoin.exe -k stdout -e stdout 2> NUL +- cmd: src\test_bitcoin.exe -l test_suite - cmd: src\bench_bitcoin.exe -evals=1 -scaling=0 > NUL - ps: python test\util\bitcoin-util-test.py - cmd: python test\util\rpcauth-test.py diff --git a/.cirrus.yml b/.cirrus.yml index f4a3878ed8..f9c3d844be 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -37,7 +37,7 @@ task: timeout_in: 60m env: MAKEJOBS: "-j9" - RUN_CI_ON_HOST: "1" + DANGER_RUN_CI_ON_HOST: "1" TEST_RUNNER_PORT_MIN: "14000" # Must be larger than 12321, which is used for the http cache. See https://cirrus-ci.org/guide/writing-tasks/#http-cache CCACHE_SIZE: "200M" CCACHE_DIR: "/tmp/ccache_dir" diff --git a/.travis.yml b/.travis.yml index 878f5ce518..d66a66ef0c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -38,9 +38,6 @@ cache: - $TRAVIS_BUILD_DIR/depends/sdk-sources - $TRAVIS_BUILD_DIR/ci/scratch/.ccache - $TRAVIS_BUILD_DIR/releases/$HOST - # macOS - - $HOME/Library/Caches/Homebrew - - /usr/local/Homebrew before_cache: - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then brew cleanup; fi stages: @@ -91,6 +88,26 @@ jobs: FILE_ENV="./ci/test/00_setup_env_arm.sh" QEMU_USER_CMD="" + - stage: test + name: 's390x native BE [GOAL: install] [bionic] [no depends, no GUI]' + arch: s390x + dist: bionic + addons: + apt: + packages: + - bsdmainutils + - libboost-filesystem-dev + - libboost-system-dev + - libboost-test-dev + - libboost-thread-dev + - libdb++-dev + - libdb-dev + - libevent-dev + env: >- + DANGER_RUN_CI_ON_HOST=true + CI_USE_APT_INSTALL=no + FILE_ENV="./ci/test/00_setup_env_s390x_host.sh" + # s390 build was disabled temporarily because of disk space issues on the Travis VM # # - stage: test @@ -158,6 +175,12 @@ jobs: # Xcode 11.3.1, macOS 10.14, SDK 10.15 # https://docs.travis-ci.com/user/reference/osx/#macos-version osx_image: xcode11.3 + cache: + directories: + - $TRAVIS_BUILD_DIR/ci/scratch/.ccache + - $TRAVIS_BUILD_DIR/releases/$HOST + - $HOME/Library/Caches/Homebrew + - /usr/local/Homebrew addons: homebrew: packages: @@ -171,4 +194,6 @@ jobs: - ccache - zeromq env: >- + DANGER_RUN_CI_ON_HOST=true + CI_USE_APT_INSTALL=no FILE_ENV="./ci/test/00_setup_env_mac_host.sh" diff --git a/build-aux/m4/ax_boost_base.m4 b/build-aux/m4/ax_boost_base.m4 index 16fa69b41f..2ae33f7140 100644 --- a/build-aux/m4/ax_boost_base.m4 +++ b/build-aux/m4/ax_boost_base.m4 @@ -33,7 +33,7 @@ # and this notice are preserved. This file is offered as-is, without any # warranty. -#serial 47 +#serial 48 # example boost program (need to pass version) m4_define([_AX_BOOST_BASE_PROGRAM], @@ -123,6 +123,7 @@ AC_DEFUN([_AX_BOOST_BASE_RUNDETECT],[ dnl are almost assuredly the ones desired. AS_CASE([${host_cpu}], [i?86],[multiarch_libsubdir="lib/i386-${host_os}"], + [armv7l],[multiarch_libsubdir="lib/arm-${host_os}"], [multiarch_libsubdir="lib/${host_cpu}-${host_os}"] ) diff --git a/ci/test/00_setup_env_mac_host.sh b/ci/test/00_setup_env_mac_host.sh index 5753c3af31..a90d83734e 100644 --- a/ci/test/00_setup_env_mac_host.sh +++ b/ci/test/00_setup_env_mac_host.sh @@ -8,7 +8,6 @@ export LC_ALL=C.UTF-8 export HOST=x86_64-apple-darwin16 export PIP_PACKAGES="zmq" -export RUN_CI_ON_HOST=true export RUN_UNIT_TESTS=true export RUN_FUNCTIONAL_TESTS=false export GOAL="install" diff --git a/ci/test/00_setup_env_native_qt5.sh b/ci/test/00_setup_env_native_qt5.sh index 21c15236d2..a5025339b6 100644 --- a/ci/test/00_setup_env_native_qt5.sh +++ b/ci/test/00_setup_env_native_qt5.sh @@ -10,6 +10,8 @@ export CONTAINER_NAME=ci_native_qt5 export PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libdbus-1-dev libharfbuzz-dev" export DEP_OPTS="NO_QT=1 NO_UPNP=1 DEBUG=1 ALLOW_HOST_PACKAGES=1" export TEST_RUNNER_EXTRA="--coverage --extended --exclude feature_dbcrash" # Run extended tests so that coverage does not fail, but exclude the very slow dbcrash +export RUN_UNIT_TESTS_SEQUENTIAL="true" +export RUN_UNIT_TESTS="false" export GOAL="install" export TEST_PREVIOUS_RELEASES=true export BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-glibc-back-compat --enable-reduce-exports --enable-debug CFLAGS=\"-g0 -O2 -funsigned-char\" CXXFLAGS=\"-g0 -O2 -funsigned-char\"" diff --git a/ci/test/00_setup_env_s390x_host.sh b/ci/test/00_setup_env_s390x_host.sh new file mode 100644 index 0000000000..8f3063604e --- /dev/null +++ b/ci/test/00_setup_env_s390x_host.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# +# Copyright (c) 2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +export LC_ALL=C.UTF-8 + +export HOST=s390x-linux-gnu +export NO_DEPENDS=1 +export BITCOIN_CONFIG="--with-incompatible-bdb --enable-reduce-exports" +export RUN_UNIT_TESTS=true +export RUN_FUNCTIONAL_TESTS=true +export GOAL="install" diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index acf7eeb920..3cfaf77fde 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -35,7 +35,7 @@ fi export P_CI_DIR="$PWD" -if [ -z "$RUN_CI_ON_HOST" ]; then +if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then echo "Creating $DOCKER_NAME_TAG container to run in" ${CI_RETRY_EXE} docker pull "$DOCKER_NAME_TAG" @@ -66,7 +66,7 @@ fi if [[ $DOCKER_NAME_TAG == centos* ]]; then ${CI_RETRY_EXE} DOCKER_EXEC yum -y install epel-release ${CI_RETRY_EXE} DOCKER_EXEC yum -y install $DOCKER_PACKAGES $PACKAGES -elif [ "$TRAVIS_OS_NAME" != "osx" ]; then +elif [ "$CI_USE_APT_INSTALL" != "no" ]; then ${CI_RETRY_EXE} DOCKER_EXEC apt-get update ${CI_RETRY_EXE} DOCKER_EXEC apt-get install --no-install-recommends --no-upgrade -y $PACKAGES $DOCKER_PACKAGES fi @@ -77,6 +77,7 @@ if [ "$TRAVIS_OS_NAME" == "osx" ]; then else DOCKER_EXEC free -m -h DOCKER_EXEC echo "Number of CPUs \(nproc\):" \$\(nproc\) + DOCKER_EXEC echo $(lscpu | grep Endian) DOCKER_EXEC echo "Free disk space:" DOCKER_EXEC df -h fi @@ -90,7 +91,7 @@ export DIR_FUZZ_IN=${DIR_QA_ASSETS}/fuzz_seed_corpus/ DOCKER_EXEC mkdir -p "${BASE_SCRATCH_DIR}/sanitizer-output/" -if [ -z "$RUN_CI_ON_HOST" ]; then +if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then echo "Create $BASE_ROOT_DIR" DOCKER_EXEC rsync -a /ro_base/ $BASE_ROOT_DIR fi diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index 3b32513353..2a98f3fa5e 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -21,13 +21,20 @@ if [ -n "$USE_VALGRIND" ]; then END_FOLD fi +bash -c "${CI_WAIT}" & # Print dots in case the tests take a long time to run + if [ "$RUN_UNIT_TESTS" = "true" ]; then BEGIN_FOLD unit-tests - bash -c "${CI_WAIT}" & # Print dots in case the unit tests take a long time to run DOCKER_EXEC LD_LIBRARY_PATH=$DEPENDS_DIR/$HOST/lib make $MAKEJOBS check VERBOSE=1 END_FOLD fi +if [ "$RUN_UNIT_TESTS_SEQUENTIAL" = "true" ]; then + BEGIN_FOLD unit-tests-seq + DOCKER_EXEC LD_LIBRARY_PATH=$DEPENDS_DIR/$HOST/lib "${BASE_ROOT_DIR}/build/bitcoin-*/src/test/test_bitcoin" --catch_system_errors=no -l test_suite + END_FOLD +fi + if [ "$RUN_FUNCTIONAL_TESTS" = "true" ]; then BEGIN_FOLD functional-tests DOCKER_EXEC test/functional/test_runner.py --ci $MAKEJOBS --tmpdirprefix "${BASE_SCRATCH_DIR}/test_runner/" --ansi --combinedlogslen=4000 ${TEST_RUNNER_EXTRA} --quiet --failfast diff --git a/contrib/devtools/security-check.py b/contrib/devtools/security-check.py index c05c38d513..b924698e56 100755 --- a/contrib/devtools/security-check.py +++ b/contrib/devtools/security-check.py @@ -206,6 +206,23 @@ def check_MACHO_NX(executable) -> bool: return False return True +def check_MACHO_LAZY_BINDINGS(executable) -> bool: + ''' + Check for no lazy bindings. + We don't use or check for MH_BINDATLOAD. See #18295. + ''' + p = subprocess.Popen([OTOOL_CMD, '-l', executable], stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE, universal_newlines=True) + (stdout, stderr) = p.communicate() + if p.returncode: + raise IOError('Error opening file') + + for line in stdout.splitlines(): + tokens = line.split() + if 'lazy_bind_off' in tokens or 'lazy_bind_size' in tokens: + if tokens[1] != '0': + return False + return True + CHECKS = { 'ELF': [ ('PIE', check_ELF_PIE), @@ -221,7 +238,8 @@ CHECKS = { 'MACHO': [ ('PIE', check_MACHO_PIE), ('NOUNDEFS', check_MACHO_NOUNDEFS), - ('NX', check_MACHO_NX) + ('NX', check_MACHO_NX), + ('LAZY_BINDINGS', check_MACHO_LAZY_BINDINGS) ] } diff --git a/doc/developer-notes.md b/doc/developer-notes.md index da07080724..4960ec567a 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -965,7 +965,7 @@ Some good examples of scripted-diff: - [scripted-diff: Rename InitInterfaces to NodeContext](https://github.com/bitcoin/bitcoin/commit/301bd41a2e6765b185bd55f4c541f9e27aeea29d) uses an elegant script to replace occurrences of multiple terms in all source files. -- [scripted-diff: Remove g_connman, g_banman globals](https://github.com/bitcoin/bitcoin/commit/301bd41a2e6765b185bd55f4c541f9e27aeea29d) +- [scripted-diff: Remove g_connman, g_banman globals](https://github.com/bitcoin/bitcoin/commit/8922d7f6b751a3e6b3b9f6fb7961c442877fb65a) replaces specific terms in a list of specific source files. - [scripted-diff: Replace fprintf with tfm::format](https://github.com/bitcoin/bitcoin/commit/fac03ec43a15ad547161e37e53ea82482cc508f9) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index c3021743f4..d53477793c 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -3,6 +3,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. FUZZ_TARGETS = \ + test/fuzz/addition_overflow \ test/fuzz/addr_info_deserialize \ test/fuzz/addrdb \ test/fuzz/address_deserialize \ @@ -27,17 +28,21 @@ FUZZ_TARGETS = \ test/fuzz/bloom_filter \ test/fuzz/bloomfilter_deserialize \ test/fuzz/chain \ + test/fuzz/checkqueue \ test/fuzz/coins_deserialize \ + test/fuzz/cuckoocache \ test/fuzz/decode_tx \ test/fuzz/descriptor_parse \ test/fuzz/diskblockindex_deserialize \ test/fuzz/eval_script \ test/fuzz/fee_rate \ test/fuzz/fee_rate_deserialize \ + test/fuzz/fees \ test/fuzz/flat_file_pos_deserialize \ test/fuzz/flatfile \ test/fuzz/float \ test/fuzz/hex \ + test/fuzz/http_request \ test/fuzz/integer \ test/fuzz/inv_deserialize \ test/fuzz/key \ @@ -58,10 +63,12 @@ FUZZ_TARGETS = \ test/fuzz/parse_numbers \ test/fuzz/parse_script \ test/fuzz/parse_univalue \ + test/fuzz/prevector \ test/fuzz/partial_merkle_tree_deserialize \ test/fuzz/partially_signed_transaction_deserialize \ test/fuzz/pow \ test/fuzz/prefilled_transaction_deserialize \ + test/fuzz/process_messages \ test/fuzz/process_message \ test/fuzz/process_message_addr \ test/fuzz/process_message_block \ @@ -239,6 +246,7 @@ BITCOIN_TESTS =\ test/util_tests.cpp \ test/validation_block_tests.cpp \ test/validation_flush_tests.cpp \ + test/validationinterface_tests.cpp \ test/versionbits_tests.cpp if ENABLE_WALLET @@ -279,6 +287,12 @@ endif if ENABLE_FUZZ +test_fuzz_addition_overflow_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_addition_overflow_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_addition_overflow_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_addition_overflow_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_addition_overflow_SOURCES = test/fuzz/addition_overflow.cpp + test_fuzz_addr_info_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DADDR_INFO_DESERIALIZE=1 test_fuzz_addr_info_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_addr_info_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -423,12 +437,24 @@ test_fuzz_chain_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_chain_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_chain_SOURCES = test/fuzz/chain.cpp +test_fuzz_checkqueue_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_checkqueue_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_checkqueue_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_checkqueue_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_checkqueue_SOURCES = test/fuzz/checkqueue.cpp + test_fuzz_coins_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DCOINS_DESERIALIZE=1 test_fuzz_coins_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_coins_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_coins_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_coins_deserialize_SOURCES = test/fuzz/deserialize.cpp +test_fuzz_cuckoocache_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_cuckoocache_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_cuckoocache_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_cuckoocache_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_cuckoocache_SOURCES = test/fuzz/cuckoocache.cpp + test_fuzz_decode_tx_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_decode_tx_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_decode_tx_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -465,6 +491,12 @@ test_fuzz_fee_rate_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_fee_rate_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_fee_rate_deserialize_SOURCES = test/fuzz/deserialize.cpp +test_fuzz_fees_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_fees_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_fees_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_fees_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_fees_SOURCES = test/fuzz/fees.cpp + test_fuzz_flat_file_pos_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DFLAT_FILE_POS_DESERIALIZE=1 test_fuzz_flat_file_pos_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_flat_file_pos_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -489,6 +521,12 @@ test_fuzz_hex_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_hex_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_hex_SOURCES = test/fuzz/hex.cpp +test_fuzz_http_request_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_http_request_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_http_request_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_http_request_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_http_request_SOURCES = test/fuzz/http_request.cpp + test_fuzz_integer_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_integer_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_integer_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -609,6 +647,12 @@ test_fuzz_parse_univalue_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_parse_univalue_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_parse_univalue_SOURCES = test/fuzz/parse_univalue.cpp +test_fuzz_prevector_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_prevector_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_prevector_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_prevector_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_prevector_SOURCES = test/fuzz/prevector.cpp + test_fuzz_partial_merkle_tree_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DPARTIAL_MERKLE_TREE_DESERIALIZE=1 test_fuzz_partial_merkle_tree_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_partial_merkle_tree_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -633,6 +677,12 @@ test_fuzz_prefilled_transaction_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_prefilled_transaction_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_prefilled_transaction_deserialize_SOURCES = test/fuzz/deserialize.cpp +test_fuzz_process_messages_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_process_messages_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_process_messages_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_process_messages_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_process_messages_SOURCES = test/fuzz/process_messages.cpp + test_fuzz_process_message_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_process_message_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_process_message_LDADD = $(FUZZ_SUITE_LD_COMMON) diff --git a/src/Makefile.test_util.include b/src/Makefile.test_util.include index 505d630b7d..d7bc73defb 100644 --- a/src/Makefile.test_util.include +++ b/src/Makefile.test_util.include @@ -11,6 +11,7 @@ TEST_UTIL_H = \ test/util/blockfilter.h \ test/util/logging.h \ test/util/mining.h \ + test/util/net.h \ test/util/setup_common.h \ test/util/str.h \ test/util/transaction_utils.h \ @@ -22,6 +23,7 @@ libtest_util_a_SOURCES = \ test/util/blockfilter.cpp \ test/util/logging.cpp \ test/util/mining.cpp \ + test/util/net.cpp \ test/util/setup_common.cpp \ test/util/str.cpp \ test/util/transaction_utils.cpp \ diff --git a/src/cuckoocache.h b/src/cuckoocache.h index 4ad5818cdc..2daf676c4a 100644 --- a/src/cuckoocache.h +++ b/src/cuckoocache.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_CUCKOOCACHE_H #define BITCOIN_CUCKOOCACHE_H +#include <algorithm> // std::find #include <array> #include <atomic> #include <cmath> diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 11d73b7c9a..7282b517f4 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -189,7 +189,7 @@ static bool InitHTTPAllowList() } /** HTTP request method as string - use for logging only */ -static std::string RequestMethodString(HTTPRequest::RequestMethod m) +std::string RequestMethodString(HTTPRequest::RequestMethod m) { switch (m) { case HTTPRequest::GET: @@ -510,10 +510,10 @@ void HTTPEvent::trigger(struct timeval* tv) else evtimer_add(ev, tv); // trigger after timeval passed } -HTTPRequest::HTTPRequest(struct evhttp_request* _req) : req(_req), - replySent(false) +HTTPRequest::HTTPRequest(struct evhttp_request* _req, bool _replySent) : req(_req), replySent(_replySent) { } + HTTPRequest::~HTTPRequest() { if (!replySent) { diff --git a/src/httpserver.h b/src/httpserver.h index 46820e6aee..c25ea7a8fb 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -60,7 +60,7 @@ private: bool replySent; public: - explicit HTTPRequest(struct evhttp_request* req); + explicit HTTPRequest(struct evhttp_request* req, bool replySent = false); ~HTTPRequest(); enum RequestMethod { diff --git a/src/init.cpp b/src/init.cpp index 88caed9ded..92faa77059 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -197,7 +197,20 @@ void Shutdown(NodeContext& node) // Because these depend on each-other, we make sure that neither can be // using the other before destroying them. if (node.peer_logic) UnregisterValidationInterface(node.peer_logic.get()); - if (node.connman) node.connman->Stop(); + // Follow the lock order requirements: + // * CheckForStaleTipAndEvictPeers locks cs_main before indirectly calling GetExtraOutboundCount + // which locks cs_vNodes. + // * ProcessMessage locks cs_main and g_cs_orphans before indirectly calling ForEachNode which + // locks cs_vNodes. + // * CConnman::Stop calls DeleteNode, which calls FinalizeNode, which locks cs_main and calls + // EraseOrphansFor, which locks g_cs_orphans. + // + // Thus the implicit locking order requirement is: (1) cs_main, (2) g_cs_orphans, (3) cs_vNodes. + if (node.connman) { + node.connman->StopThreads(); + LOCK2(::cs_main, ::g_cs_orphans); + node.connman->StopNodes(); + } StopTorControl(); @@ -278,13 +291,6 @@ void Shutdown(NodeContext& node) } #endif - try { - if (!fs::remove(GetPidFile())) { - LogPrintf("%s: Unable to remove PID file: File does not exist\n", __func__); - } - } catch (const fs::filesystem_error& e) { - LogPrintf("%s: Unable to remove PID file: %s\n", __func__, fsbridge::get_filesystem_error_message(e)); - } node.chain_clients.clear(); UnregisterAllValidationInterfaces(); GetMainSignals().UnregisterBackgroundSignalScheduler(); @@ -292,6 +298,15 @@ void Shutdown(NodeContext& node) ECC_Stop(); if (node.mempool) node.mempool = nullptr; node.scheduler.reset(); + + try { + if (!fs::remove(GetPidFile())) { + LogPrintf("%s: Unable to remove PID file: File does not exist\n", __func__); + } + } catch (const fs::filesystem_error& e) { + LogPrintf("%s: Unable to remove PID file: %s\n", __func__, fsbridge::get_filesystem_error_message(e)); + } + LogPrintf("%s: done\n", __func__); } diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 645a8709d2..abce09ca4a 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -151,12 +151,12 @@ public: std::string* purpose) override { LOCK(m_wallet->cs_wallet); - auto it = m_wallet->mapAddressBook.find(dest); - if (it == m_wallet->mapAddressBook.end()) { + auto it = m_wallet->m_address_book.find(dest); + if (it == m_wallet->m_address_book.end() || it->second.IsChange()) { return false; } if (name) { - *name = it->second.name; + *name = it->second.GetLabel(); } if (is_mine) { *is_mine = m_wallet->IsMine(dest); @@ -170,8 +170,9 @@ public: { LOCK(m_wallet->cs_wallet); std::vector<WalletAddress> result; - for (const auto& item : m_wallet->mapAddressBook) { - result.emplace_back(item.first, m_wallet->IsMine(item.first), item.second.name, item.second.purpose); + for (const auto& item : m_wallet->m_address_book) { + if (item.second.IsChange()) continue; + result.emplace_back(item.first, m_wallet->IsMine(item.first), item.second.GetLabel(), item.second.purpose); } return result; } diff --git a/src/net.cpp b/src/net.cpp index 8352c40b98..dcc613ba88 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2387,7 +2387,7 @@ void CConnman::Interrupt() } } -void CConnman::Stop() +void CConnman::StopThreads() { if (threadMessageHandler.joinable()) threadMessageHandler.join(); @@ -2399,14 +2399,17 @@ void CConnman::Stop() threadDNSAddressSeed.join(); if (threadSocketHandler.joinable()) threadSocketHandler.join(); +} - if (fAddressesInitialized) - { +void CConnman::StopNodes() +{ + if (fAddressesInitialized) { DumpAddresses(); fAddressesInitialized = false; } // Close sockets + LOCK(cs_vNodes); for (CNode* pnode : vNodes) pnode->CloseSocketDisconnect(); for (ListenSocket& hListenSocket : vhListenSocket) @@ -2415,10 +2418,10 @@ void CConnman::Stop() LogPrintf("CloseSocket(hListenSocket) failed with error %s\n", NetworkErrorString(WSAGetLastError())); // clean up some globals (to help leak detection) - for (CNode *pnode : vNodes) { + for (CNode* pnode : vNodes) { DeleteNode(pnode); } - for (CNode *pnode : vNodesDisconnected) { + for (CNode* pnode : vNodesDisconnected) { DeleteNode(pnode); } vNodes.clear(); @@ -2433,7 +2436,7 @@ void CConnman::DeleteNode(CNode* pnode) assert(pnode); bool fUpdateConnectionTime = false; m_msgproc->FinalizeNode(pnode->GetId(), fUpdateConnectionTime); - if(fUpdateConnectionTime) { + if (fUpdateConnectionTime) { addrman.Connected(pnode->addr); } delete pnode; @@ -188,16 +188,13 @@ public: ~CConnman(); bool Start(CScheduler& scheduler, const Options& options); - // TODO: Remove NO_THREAD_SAFETY_ANALYSIS. Lock cs_vNodes before reading the variable vNodes. - // - // When removing NO_THREAD_SAFETY_ANALYSIS be aware of the following lock order requirements: - // * CheckForStaleTipAndEvictPeers locks cs_main before indirectly calling GetExtraOutboundCount - // which locks cs_vNodes. - // * ProcessMessage locks cs_main and g_cs_orphans before indirectly calling ForEachNode which - // locks cs_vNodes. - // - // Thus the implicit locking order requirement is: (1) cs_main, (2) g_cs_orphans, (3) cs_vNodes. - void Stop() NO_THREAD_SAFETY_ANALYSIS; + void StopThreads(); + void StopNodes(); + void Stop() + { + StopThreads(); + StopNodes(); + }; void Interrupt(); bool GetNetworkActive() const { return fNetworkActive; }; @@ -482,6 +479,7 @@ private: std::atomic<int64_t> m_next_send_inv_to_incoming{0}; friend struct CConnmanTest; + friend struct ConnmanTestMsg; }; void Discover(); void StartMapPort(); @@ -721,6 +719,8 @@ public: class CNode { friend class CConnman; + friend struct ConnmanTestMsg; + public: std::unique_ptr<TransportDeserializer> m_deserializer; std::unique_ptr<TransportSerializer> m_serializer; @@ -797,8 +797,8 @@ public: std::vector<CAddress> vAddrToSend; const std::unique_ptr<CRollingBloomFilter> m_addr_known; bool fGetAddr{false}; - int64_t nNextAddrSend GUARDED_BY(cs_sendProcessing){0}; - int64_t nNextLocalAddrSend GUARDED_BY(cs_sendProcessing){0}; + std::chrono::microseconds m_next_addr_send GUARDED_BY(cs_sendProcessing){0}; + std::chrono::microseconds m_next_local_addr_send GUARDED_BY(cs_sendProcessing){0}; bool IsAddrRelayPeer() const { return m_addr_known != nullptr; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ab430cbe19..d3089c4176 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -97,10 +97,10 @@ void EraseOrphansFor(NodeId peer); /** Increase a node's misbehavior score. */ void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="") EXCLUSIVE_LOCKS_REQUIRED(cs_main); -/** Average delay between local address broadcasts in seconds. */ -static constexpr unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 60 * 60; -/** Average delay between peer address broadcasts in seconds. */ -static const unsigned int AVG_ADDRESS_BROADCAST_INTERVAL = 30; +/** Average delay between local address broadcasts */ +static constexpr std::chrono::hours AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL{24}; +/** Average delay between peer address broadcasts */ +static constexpr std::chrono::seconds AVG_ADDRESS_BROADCAST_INTERVAL{30}; /** Average delay between trickled inventory transmissions in seconds. * Blocks and whitelisted receivers bypass this, outbound peers get half this delay. */ static const unsigned int INVENTORY_BROADCAST_INTERVAL = 5; @@ -1365,7 +1365,7 @@ void RelayTransaction(const uint256& txid, const CConnman& connman) }); } -static void RelayAddress(const CAddress& addr, bool fReachable, CConnman* connman) +static void RelayAddress(const CAddress& addr, bool fReachable, const CConnman& connman) { unsigned int nRelayNodes = fReachable ? 2 : 1; // limited relaying of addresses outside our network(s) @@ -1373,7 +1373,7 @@ static void RelayAddress(const CAddress& addr, bool fReachable, CConnman* connma // Use deterministic randomness to send to the same nodes for 24 hours // at a time so the m_addr_knowns of the chosen nodes prevent repeats uint64_t hashAddr = addr.GetHash(); - const CSipHasher hasher = connman->GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr << 32).Write((GetTime() + hashAddr) / (24*60*60)); + const CSipHasher hasher = connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr << 32).Write((GetTime() + hashAddr) / (24 * 60 * 60)); FastRandomContext insecure_rand; std::array<std::pair<uint64_t, CNode*>,2> best{{{0, nullptr}, {0, nullptr}}}; @@ -1398,7 +1398,7 @@ static void RelayAddress(const CAddress& addr, bool fReachable, CConnman* connma } }; - connman->ForEachNodeThen(std::move(sortfunc), std::move(pushfunc)); + connman.ForEachNodeThen(std::move(sortfunc), std::move(pushfunc)); } void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, const CInv& inv, CConnman* connman) @@ -1911,9 +1911,9 @@ void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::set<uin } } -bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CTxMemPool& mempool, CConnman* connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc) +bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CTxMemPool& mempool, CConnman* connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc) { - LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(strCommand), vRecv.size(), pfrom->GetId()); + LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(msg_type), vRecv.size(), pfrom->GetId()); if (gArgs.IsArgSet("-dropmessagestest") && GetRand(gArgs.GetArg("-dropmessagestest", 0)) == 0) { LogPrintf("dropmessagestest DROPPING RECV MESSAGE\n"); @@ -1922,8 +1922,8 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR if (!(pfrom->GetLocalServices() & NODE_BLOOM) && - (strCommand == NetMsgType::FILTERLOAD || - strCommand == NetMsgType::FILTERADD)) + (msg_type == NetMsgType::FILTERLOAD || + msg_type == NetMsgType::FILTERADD)) { if (pfrom->nVersion >= NO_BLOOM_VERSION) { LOCK(cs_main); @@ -1935,7 +1935,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR } } - if (strCommand == NetMsgType::VERSION) { + if (msg_type == NetMsgType::VERSION) { // Each connection can only send one version message if (pfrom->nVersion != 0) { @@ -2107,7 +2107,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR // At this point, the outgoing message serialization version can't change. const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); - if (strCommand == NetMsgType::VERACK) + if (msg_type == NetMsgType::VERACK) { pfrom->SetRecvVersion(std::min(pfrom->nVersion.load(), PROTOCOL_VERSION)); @@ -2152,7 +2152,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return false; } - if (strCommand == NetMsgType::ADDR) { + if (msg_type == NetMsgType::ADDR) { std::vector<CAddress> vAddr; vRecv >> vAddr; @@ -2192,7 +2192,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR if (addr.nTime > nSince && !pfrom->fGetAddr && vAddr.size() <= 10 && addr.IsRoutable()) { // Relay to a limited number of other nodes - RelayAddress(addr, fReachable, connman); + RelayAddress(addr, fReachable, *connman); } // Do not store addresses outside our network if (fReachable) @@ -2206,13 +2206,13 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::SENDHEADERS) { + if (msg_type == NetMsgType::SENDHEADERS) { LOCK(cs_main); State(pfrom->GetId())->fPreferHeaders = true; return true; } - if (strCommand == NetMsgType::SENDCMPCT) { + if (msg_type == NetMsgType::SENDCMPCT) { bool fAnnounceUsingCMPCTBLOCK = false; uint64_t nCMPCTBLOCKVersion = 0; vRecv >> fAnnounceUsingCMPCTBLOCK >> nCMPCTBLOCKVersion; @@ -2235,7 +2235,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::INV) { + if (msg_type == NetMsgType::INV) { std::vector<CInv> vInv; vRecv >> vInv; if (vInv.size() > MAX_INV_SZ) @@ -2297,7 +2297,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::GETDATA) { + if (msg_type == NetMsgType::GETDATA) { std::vector<CInv> vInv; vRecv >> vInv; if (vInv.size() > MAX_INV_SZ) @@ -2318,7 +2318,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::GETBLOCKS) { + if (msg_type == NetMsgType::GETBLOCKS) { CBlockLocator locator; uint256 hashStop; vRecv >> locator >> hashStop; @@ -2386,7 +2386,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::GETBLOCKTXN) { + if (msg_type == NetMsgType::GETBLOCKTXN) { BlockTransactionsRequest req; vRecv >> req; @@ -2435,7 +2435,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::GETHEADERS) { + if (msg_type == NetMsgType::GETHEADERS) { CBlockLocator locator; uint256 hashStop; vRecv >> locator >> hashStop; @@ -2502,7 +2502,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::TX) { + if (msg_type == NetMsgType::TX) { // Stop processing the transaction early if // We are in blocks only mode and peer is either not whitelisted or whitelistrelay is off // or if this peer is supposed to be a block-relay-only peer @@ -2644,7 +2644,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::CMPCTBLOCK) + if (msg_type == NetMsgType::CMPCTBLOCK) { // Ignore cmpctblock received while importing if (fImporting || fReindex) { @@ -2865,7 +2865,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::BLOCKTXN) + if (msg_type == NetMsgType::BLOCKTXN) { // Ignore blocktxn received while importing if (fImporting || fReindex) { @@ -2947,7 +2947,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::HEADERS) + if (msg_type == NetMsgType::HEADERS) { // Ignore headers received while importing if (fImporting || fReindex) { @@ -2973,7 +2973,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return ProcessHeadersMessage(pfrom, connman, mempool, headers, chainparams, /*via_compact_block=*/false); } - if (strCommand == NetMsgType::BLOCK) + if (msg_type == NetMsgType::BLOCK) { // Ignore block received while importing if (fImporting || fReindex) { @@ -3009,7 +3009,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::GETADDR) { + if (msg_type == NetMsgType::GETADDR) { // This asymmetric behavior for inbound and outbound connections was introduced // to prevent a fingerprinting attack: an attacker can send specific fake addresses // to users' AddrMan and later request them by sending getaddr messages. @@ -3043,7 +3043,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::MEMPOOL) { + if (msg_type == NetMsgType::MEMPOOL) { if (!(pfrom->GetLocalServices() & NODE_BLOOM) && !pfrom->HasPermission(PF_MEMPOOL)) { if (!pfrom->HasPermission(PF_NOBAN)) @@ -3071,7 +3071,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::PING) { + if (msg_type == NetMsgType::PING) { if (pfrom->nVersion > BIP0031_VERSION) { uint64_t nonce = 0; @@ -3092,7 +3092,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::PONG) { + if (msg_type == NetMsgType::PONG) { int64_t pingUsecEnd = nTimeReceived; uint64_t nonce = 0; size_t nAvail = vRecv.in_avail(); @@ -3148,7 +3148,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::FILTERLOAD) { + if (msg_type == NetMsgType::FILTERLOAD) { CBloomFilter filter; vRecv >> filter; @@ -3168,7 +3168,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::FILTERADD) { + if (msg_type == NetMsgType::FILTERADD) { std::vector<unsigned char> vData; vRecv >> vData; @@ -3192,7 +3192,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::FILTERCLEAR) { + if (msg_type == NetMsgType::FILTERCLEAR) { if (pfrom->m_tx_relay == nullptr) { return true; } @@ -3204,7 +3204,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::FEEFILTER) { + if (msg_type == NetMsgType::FEEFILTER) { CAmount newFeeFilter = 0; vRecv >> newFeeFilter; if (MoneyRange(newFeeFilter)) { @@ -3217,7 +3217,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR return true; } - if (strCommand == NetMsgType::NOTFOUND) { + if (msg_type == NetMsgType::NOTFOUND) { // Remove the NOTFOUND transactions from the peer LOCK(cs_main); CNodeState *state = State(pfrom->GetId()); @@ -3243,7 +3243,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vR } // Ignore unknown commands for extensibility - LogPrint(BCLog::NET, "Unknown command \"%s\" from peer=%d\n", SanitizeString(strCommand), pfrom->GetId()); + LogPrint(BCLog::NET, "Unknown command \"%s\" from peer=%d\n", SanitizeString(msg_type), pfrom->GetId()); return true; } @@ -3338,7 +3338,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter LogPrint(BCLog::NET, "PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(msg.m_command), pfrom->GetId()); return fMoreWork; } - const std::string& strCommand = msg.m_command; + const std::string& msg_type = msg.m_command; // Message size unsigned int nMessageSize = msg.m_message_size; @@ -3348,7 +3348,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter if (!msg.m_valid_checksum) { LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR peer=%d\n", __func__, - SanitizeString(strCommand), nMessageSize, pfrom->GetId()); + SanitizeString(msg_type), nMessageSize, pfrom->GetId()); return fMoreWork; } @@ -3356,19 +3356,19 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter bool fRet = false; try { - fRet = ProcessMessage(pfrom, strCommand, vRecv, msg.m_time, chainparams, m_mempool, connman, m_banman, interruptMsgProc); + fRet = ProcessMessage(pfrom, msg_type, vRecv, msg.m_time, chainparams, m_mempool, connman, m_banman, interruptMsgProc); if (interruptMsgProc) return false; if (!pfrom->vRecvGetData.empty()) fMoreWork = true; } catch (const std::exception& e) { - LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' (%s) caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what(), typeid(e).name()); + LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' (%s) caught\n", __func__, SanitizeString(msg_type), nMessageSize, e.what(), typeid(e).name()); } catch (...) { - LogPrint(BCLog::NET, "%s(%s, %u bytes): Unknown exception caught\n", __func__, SanitizeString(strCommand), nMessageSize); + LogPrint(BCLog::NET, "%s(%s, %u bytes): Unknown exception caught\n", __func__, SanitizeString(msg_type), nMessageSize); } if (!fRet) { - LogPrint(BCLog::NET, "%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->GetId()); + LogPrint(BCLog::NET, "%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(msg_type), nMessageSize, pfrom->GetId()); } LOCK(cs_main); @@ -3583,16 +3583,16 @@ bool PeerLogicValidation::SendMessages(CNode* pto) int64_t nNow = GetTimeMicros(); auto current_time = GetTime<std::chrono::microseconds>(); - if (pto->IsAddrRelayPeer() && !::ChainstateActive().IsInitialBlockDownload() && pto->nNextLocalAddrSend < nNow) { + if (pto->IsAddrRelayPeer() && !::ChainstateActive().IsInitialBlockDownload() && pto->m_next_local_addr_send < current_time) { AdvertiseLocal(pto); - pto->nNextLocalAddrSend = PoissonNextSend(nNow, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); + pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); } // // Message: addr // - if (pto->IsAddrRelayPeer() && pto->nNextAddrSend < nNow) { - pto->nNextAddrSend = PoissonNextSend(nNow, AVG_ADDRESS_BROADCAST_INTERVAL); + if (pto->IsAddrRelayPeer() && pto->m_next_addr_send < current_time) { + pto->m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); std::vector<CAddress> vAddr; vAddr.reserve(pto->vAddrToSend.size()); assert(pto->m_addr_known); diff --git a/src/net_processing.h b/src/net_processing.h index 65e3963c41..3d9bc65a3a 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -14,6 +14,7 @@ class CTxMemPool; extern RecursiveMutex cs_main; +extern RecursiveMutex g_cs_orphans; /** Default for -maxorphantx, maximum number of orphan transactions kept in memory */ static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100; diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index 3ac98a5970..3869318fea 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -55,7 +55,7 @@ struct AddressTableEntryLessThan static AddressTableEntry::Type translateTransactionType(const QString &strPurpose, bool isMine) { AddressTableEntry::Type addressType = AddressTableEntry::Hidden; - // "refund" addresses aren't shown, and change addresses aren't in mapAddressBook at all. + // "refund" addresses aren't shown, and change addresses aren't returned by getAddresses at all. if (strPurpose == "send") addressType = AddressTableEntry::Sending; else if (strPurpose == "receive") diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 2918676c22..75f3e9bf45 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -456,7 +456,7 @@ void BitcoinGUI::createMenuBar() QAction* minimize_action = window_menu->addAction(tr("Minimize")); minimize_action->setShortcut(QKeySequence(Qt::CTRL + Qt::Key_M)); connect(minimize_action, &QAction::triggered, [] { - qApp->focusWindow()->showMinimized(); + QApplication::activeWindow()->showMinimized(); }); connect(qApp, &QApplication::focusWindowChanged, [minimize_action] (QWindow* window) { minimize_action->setEnabled(window != nullptr && (window->flags() & Qt::Dialog) != Qt::Dialog && window->windowState() != Qt::WindowMinimized); diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index 0f082802cc..042387286a 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -97,7 +97,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node) auto check_addbook_size = [&wallet](int expected_size) { LOCK(wallet->cs_wallet); - QCOMPARE(static_cast<int>(wallet->mapAddressBook.size()), expected_size); + QCOMPARE(static_cast<int>(wallet->m_address_book.size()), expected_size); }; // We should start with the two addresses we added earlier and nothing else. diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index e84b751c9a..142d4c7e63 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2342,6 +2342,8 @@ UniValue dumptxoutset(const JSONRPCRequest& request) return result; } +void RegisterBlockchainRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -2382,8 +2384,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterBlockchainRPCCommands(CRPCTable &t) -{ for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) t.appendCommand(commands[vcidx].name, &commands[vcidx]); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index bde19d8e79..da9d583fa7 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1022,6 +1022,8 @@ static UniValue estimaterawfee(const JSONRPCRequest& request) return result; } +void RegisterMiningRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -1043,8 +1045,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterMiningRPCCommands(CRPCTable &t) -{ for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) t.appendCommand(commands[vcidx].name, &commands[vcidx]); } diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index c87c1a5418..51a9581349 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -589,6 +589,8 @@ static UniValue echo(const JSONRPCRequest& request) return request.params; } +void RegisterMiscRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -610,8 +612,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterMiscRPCCommands(CRPCTable &t) -{ for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) t.appendCommand(commands[vcidx].name, &commands[vcidx]); } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index caa62ca958..10562126db 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -760,6 +760,8 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request) return ret; } +void RegisterNetRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -780,8 +782,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterNetRPCCommands(CRPCTable &t) -{ for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) t.appendCommand(commands[vcidx].name, &commands[vcidx]); } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index ae3f15cec2..3c0b060439 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1813,6 +1813,8 @@ UniValue analyzepsbt(const JSONRPCRequest& request) return result; } +void RegisterRawTransactionRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -1839,8 +1841,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterRawTransactionRPCCommands(CRPCTable &t) -{ for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) t.appendCommand(commands[vcidx].name, &commands[vcidx]); } diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 54baec6c6f..7b701a2bbe 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -216,7 +216,7 @@ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keyst keystore->AddCScript(script); // Automatically also add the P2WSH wrapped version of the script (to deal with P2SH-P2WSH). // This is done for redeemScript only for compatibility, it is encouraged to use the explicit witnessScript field instead. - CScript witness_output_script{GetScriptForWitness(script)}; + CScript witness_output_script{GetScriptForDestination(WitnessV0ScriptHash(script))}; keystore->AddCScript(witness_output_script); if (!ws.isNull() && !rs.isNull()) { diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 7310498eb6..6314c1a42f 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -52,7 +52,6 @@ struct COrphanTx { NodeId fromPeer; int64_t nTimeExpire; }; -extern RecursiveMutex g_cs_orphans; extern std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans); static CService ip(uint32_t i) diff --git a/src/test/fuzz/addition_overflow.cpp b/src/test/fuzz/addition_overflow.cpp new file mode 100644 index 0000000000..a455992b13 --- /dev/null +++ b/src/test/fuzz/addition_overflow.cpp @@ -0,0 +1,55 @@ +// Copyright (c) 2020 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/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <string> +#include <vector> + +#if defined(__has_builtin) +#if __has_builtin(__builtin_add_overflow) +#define HAVE_BUILTIN_ADD_OVERFLOW +#endif +#elif defined(__GNUC__) && (__GNUC__ >= 5) +#define HAVE_BUILTIN_ADD_OVERFLOW +#endif + +namespace { +template <typename T> +void TestAdditionOverflow(FuzzedDataProvider& fuzzed_data_provider) +{ + const T i = fuzzed_data_provider.ConsumeIntegral<T>(); + const T j = fuzzed_data_provider.ConsumeIntegral<T>(); + const bool is_addition_overflow_custom = AdditionOverflow(i, j); +#if defined(HAVE_BUILTIN_ADD_OVERFLOW) + T result_builtin; + const bool is_addition_overflow_builtin = __builtin_add_overflow(i, j, &result_builtin); + assert(is_addition_overflow_custom == is_addition_overflow_builtin); + if (!is_addition_overflow_custom) { + assert(i + j == result_builtin); + } +#else + if (!is_addition_overflow_custom) { + (void)(i + j); + } +#endif +} +} // namespace + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + TestAdditionOverflow<int64_t>(fuzzed_data_provider); + TestAdditionOverflow<uint64_t>(fuzzed_data_provider); + TestAdditionOverflow<int32_t>(fuzzed_data_provider); + TestAdditionOverflow<uint32_t>(fuzzed_data_provider); + TestAdditionOverflow<int16_t>(fuzzed_data_provider); + TestAdditionOverflow<uint16_t>(fuzzed_data_provider); + TestAdditionOverflow<char>(fuzzed_data_provider); + TestAdditionOverflow<unsigned char>(fuzzed_data_provider); + TestAdditionOverflow<signed char>(fuzzed_data_provider); +} diff --git a/src/test/fuzz/checkqueue.cpp b/src/test/fuzz/checkqueue.cpp new file mode 100644 index 0000000000..2ed097b827 --- /dev/null +++ b/src/test/fuzz/checkqueue.cpp @@ -0,0 +1,65 @@ +// Copyright (c) 2020 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 <checkqueue.h> +#include <optional.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <string> +#include <vector> + +namespace { +struct DumbCheck { + const bool result = false; + + DumbCheck() = default; + + explicit DumbCheck(const bool _result) : result(_result) + { + } + + bool operator()() const + { + return result; + } + + void swap(DumbCheck& x) + { + } +}; +} // namespace + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + + const unsigned int batch_size = fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 1024); + CCheckQueue<DumbCheck> check_queue_1{batch_size}; + CCheckQueue<DumbCheck> check_queue_2{batch_size}; + std::vector<DumbCheck> checks_1; + std::vector<DumbCheck> checks_2; + const int size = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 1024); + for (int i = 0; i < size; ++i) { + const bool result = fuzzed_data_provider.ConsumeBool(); + checks_1.emplace_back(result); + checks_2.emplace_back(result); + } + if (fuzzed_data_provider.ConsumeBool()) { + check_queue_1.Add(checks_1); + } + if (fuzzed_data_provider.ConsumeBool()) { + (void)check_queue_1.Wait(); + } + + CCheckQueueControl<DumbCheck> check_queue_control{&check_queue_2}; + if (fuzzed_data_provider.ConsumeBool()) { + check_queue_control.Add(checks_2); + } + if (fuzzed_data_provider.ConsumeBool()) { + (void)check_queue_control.Wait(); + } +} diff --git a/src/test/fuzz/cuckoocache.cpp b/src/test/fuzz/cuckoocache.cpp new file mode 100644 index 0000000000..f674efe1b1 --- /dev/null +++ b/src/test/fuzz/cuckoocache.cpp @@ -0,0 +1,49 @@ +// Copyright (c) 2020 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 <cuckoocache.h> +#include <optional.h> +#include <script/sigcache.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <test/util/setup_common.h> + +#include <cstdint> +#include <string> +#include <vector> + +namespace { +FuzzedDataProvider* fuzzed_data_provider_ptr = nullptr; + +struct RandomHasher { + template <uint8_t> + uint32_t operator()(const bool& /* unused */) const + { + assert(fuzzed_data_provider_ptr != nullptr); + return fuzzed_data_provider_ptr->ConsumeIntegral<uint32_t>(); + } +}; +} // namespace + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + fuzzed_data_provider_ptr = &fuzzed_data_provider; + CuckooCache::cache<bool, RandomHasher> cuckoo_cache{}; + if (fuzzed_data_provider.ConsumeBool()) { + const size_t megabytes = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 16); + cuckoo_cache.setup_bytes(megabytes << 20); + } else { + cuckoo_cache.setup(fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(0, 4096)); + } + while (fuzzed_data_provider.ConsumeBool()) { + if (fuzzed_data_provider.ConsumeBool()) { + cuckoo_cache.insert(fuzzed_data_provider.ConsumeBool()); + } else { + cuckoo_cache.contains(fuzzed_data_provider.ConsumeBool(), fuzzed_data_provider.ConsumeBool()); + } + } + fuzzed_data_provider_ptr = nullptr; +} diff --git a/src/test/fuzz/fees.cpp b/src/test/fuzz/fees.cpp new file mode 100644 index 0000000000..090994263e --- /dev/null +++ b/src/test/fuzz/fees.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2020 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 <amount.h> +#include <optional.h> +#include <policy/fees.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <string> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + const CFeeRate minimal_incremental_fee{ConsumeMoney(fuzzed_data_provider)}; + FeeFilterRounder fee_filter_rounder{minimal_incremental_fee}; + while (fuzzed_data_provider.ConsumeBool()) { + const CAmount current_minimum_fee = ConsumeMoney(fuzzed_data_provider); + const CAmount rounded_fee = fee_filter_rounder.round(current_minimum_fee); + assert(MoneyRange(rounded_fee)); + } +} diff --git a/src/test/fuzz/http_request.cpp b/src/test/fuzz/http_request.cpp new file mode 100644 index 0000000000..4104c5574d --- /dev/null +++ b/src/test/fuzz/http_request.cpp @@ -0,0 +1,56 @@ +// Copyright (c) 2020 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 <httpserver.h> +#include <netaddress.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <event2/buffer.h> +#include <event2/http.h> +#include <event2/http_struct.h> + +#include <cassert> +#include <cstdint> +#include <string> +#include <vector> + +extern "C" int evhttp_parse_firstline_(struct evhttp_request*, struct evbuffer*); +extern "C" int evhttp_parse_headers_(struct evhttp_request*, struct evbuffer*); +std::string RequestMethodString(HTTPRequest::RequestMethod m); + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + evhttp_request* evreq = evhttp_request_new(nullptr, nullptr); + assert(evreq != nullptr); + evreq->kind = EVHTTP_REQUEST; + evbuffer* evbuf = evbuffer_new(); + assert(evbuf != nullptr); + const std::vector<uint8_t> http_buffer = ConsumeRandomLengthByteVector(fuzzed_data_provider, 4096); + evbuffer_add(evbuf, http_buffer.data(), http_buffer.size()); + if (evhttp_parse_firstline_(evreq, evbuf) != 1 || evhttp_parse_headers_(evreq, evbuf) != 1) { + evbuffer_free(evbuf); + evhttp_request_free(evreq); + return; + } + + HTTPRequest http_request{evreq, true}; + const HTTPRequest::RequestMethod request_method = http_request.GetRequestMethod(); + (void)RequestMethodString(request_method); + (void)http_request.GetURI(); + (void)http_request.GetHeader("Host"); + const std::string header = fuzzed_data_provider.ConsumeRandomLengthString(16); + (void)http_request.GetHeader(header); + (void)http_request.WriteHeader(header, fuzzed_data_provider.ConsumeRandomLengthString(16)); + (void)http_request.GetHeader(header); + const std::string body = http_request.ReadBody(); + assert(body.empty()); + const CService service = http_request.GetPeer(); + assert(service.ToString() == "[::]:0"); + + evbuffer_free(evbuf); + evhttp_request_free(evreq); +} diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index fff2fabd17..7c2537aaf5 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -23,6 +23,7 @@ #include <streams.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> #include <time.h> #include <uint256.h> #include <util/moneystr.h> @@ -35,6 +36,7 @@ #include <cassert> #include <chrono> #include <limits> +#include <set> #include <vector> void initialize() @@ -90,8 +92,12 @@ void test_one_input(const std::vector<uint8_t>& buffer) } (void)GetSizeOfCompactSize(u64); (void)GetSpecialScriptSize(u32); - // (void)GetVirtualTransactionSize(i64, i64); // function defined only for a subset of int64_t inputs - // (void)GetVirtualTransactionSize(i64, i64, u32); // function defined only for a subset of int64_t/uint32_t inputs + if (!MultiplicationOverflow(i64, static_cast<int64_t>(::nBytesPerSigOp)) && !AdditionOverflow(i64 * ::nBytesPerSigOp, static_cast<int64_t>(4))) { + (void)GetVirtualTransactionSize(i64, i64); + } + if (!MultiplicationOverflow(i64, static_cast<int64_t>(u32)) && !AdditionOverflow(i64, static_cast<int64_t>(4)) && !AdditionOverflow(i64 * u32, static_cast<int64_t>(4))) { + (void)GetVirtualTransactionSize(i64, i64, u32); + } (void)HexDigit(ch); (void)MoneyRange(i64); (void)ToString(i64); @@ -109,6 +115,12 @@ void test_one_input(const std::vector<uint8_t>& buffer) (void)memusage::DynamicUsage(u8); const unsigned char uch = static_cast<unsigned char>(u8); (void)memusage::DynamicUsage(uch); + { + const std::set<int64_t> i64s{i64, static_cast<int64_t>(u64)}; + const size_t dynamic_usage = memusage::DynamicUsage(i64s); + const size_t incremental_dynamic_usage = memusage::IncrementalDynamicUsage(i64s); + assert(dynamic_usage == incremental_dynamic_usage * i64s.size()); + } (void)MillisToTimeval(i64); const double d = ser_uint64_to_double(u64); assert(ser_double_to_uint64(d) == u64); diff --git a/src/test/fuzz/prevector.cpp b/src/test/fuzz/prevector.cpp new file mode 100644 index 0000000000..0e51ee3c95 --- /dev/null +++ b/src/test/fuzz/prevector.cpp @@ -0,0 +1,263 @@ +// Copyright (c) 2015-2020 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/fuzz/fuzz.h> +#include <test/fuzz/FuzzedDataProvider.h> + +#include <vector> +#include <prevector.h> + +#include <reverse_iterator.h> +#include <serialize.h> +#include <streams.h> + +namespace { + +template<unsigned int N, typename T> +class prevector_tester { + typedef std::vector<T> realtype; + realtype real_vector; + realtype real_vector_alt; + + typedef prevector<N, T> pretype; + pretype pre_vector; + pretype pre_vector_alt; + + typedef typename pretype::size_type Size; + +public: + void test() const { + const pretype& const_pre_vector = pre_vector; + assert(real_vector.size() == pre_vector.size()); + assert(real_vector.empty() == pre_vector.empty()); + for (Size s = 0; s < real_vector.size(); s++) { + assert(real_vector[s] == pre_vector[s]); + assert(&(pre_vector[s]) == &(pre_vector.begin()[s])); + assert(&(pre_vector[s]) == &*(pre_vector.begin() + s)); + assert(&(pre_vector[s]) == &*((pre_vector.end() + s) - real_vector.size())); + } + // assert(realtype(pre_vector) == real_vector); + assert(pretype(real_vector.begin(), real_vector.end()) == pre_vector); + assert(pretype(pre_vector.begin(), pre_vector.end()) == pre_vector); + size_t pos = 0; + for (const T& v : pre_vector) { + assert(v == real_vector[pos]); + ++pos; + } + for (const T& v : reverse_iterate(pre_vector)) { + --pos; + assert(v == real_vector[pos]); + } + for (const T& v : const_pre_vector) { + assert(v == real_vector[pos]); + ++pos; + } + for (const T& v : reverse_iterate(const_pre_vector)) { + --pos; + assert(v == real_vector[pos]); + } + CDataStream ss1(SER_DISK, 0); + CDataStream ss2(SER_DISK, 0); + ss1 << real_vector; + ss2 << pre_vector; + assert(ss1.size() == ss2.size()); + for (Size s = 0; s < ss1.size(); s++) { + assert(ss1[s] == ss2[s]); + } + } + + void resize(Size s) { + real_vector.resize(s); + assert(real_vector.size() == s); + pre_vector.resize(s); + assert(pre_vector.size() == s); + } + + void reserve(Size s) { + real_vector.reserve(s); + assert(real_vector.capacity() >= s); + pre_vector.reserve(s); + assert(pre_vector.capacity() >= s); + } + + void insert(Size position, const T& value) { + real_vector.insert(real_vector.begin() + position, value); + pre_vector.insert(pre_vector.begin() + position, value); + } + + void insert(Size position, Size count, const T& value) { + real_vector.insert(real_vector.begin() + position, count, value); + pre_vector.insert(pre_vector.begin() + position, count, value); + } + + template<typename I> + void insert_range(Size position, I first, I last) { + real_vector.insert(real_vector.begin() + position, first, last); + pre_vector.insert(pre_vector.begin() + position, first, last); + } + + void erase(Size position) { + real_vector.erase(real_vector.begin() + position); + pre_vector.erase(pre_vector.begin() + position); + } + + void erase(Size first, Size last) { + real_vector.erase(real_vector.begin() + first, real_vector.begin() + last); + pre_vector.erase(pre_vector.begin() + first, pre_vector.begin() + last); + } + + void update(Size pos, const T& value) { + real_vector[pos] = value; + pre_vector[pos] = value; + } + + void push_back(const T& value) { + real_vector.push_back(value); + pre_vector.push_back(value); + } + + void pop_back() { + real_vector.pop_back(); + pre_vector.pop_back(); + } + + void clear() { + real_vector.clear(); + pre_vector.clear(); + } + + void assign(Size n, const T& value) { + real_vector.assign(n, value); + pre_vector.assign(n, value); + } + + Size size() const { + return real_vector.size(); + } + + Size capacity() const { + return pre_vector.capacity(); + } + + void shrink_to_fit() { + pre_vector.shrink_to_fit(); + } + + void swap() { + real_vector.swap(real_vector_alt); + pre_vector.swap(pre_vector_alt); + } + + void move() { + real_vector = std::move(real_vector_alt); + real_vector_alt.clear(); + pre_vector = std::move(pre_vector_alt); + pre_vector_alt.clear(); + } + + void copy() { + real_vector = real_vector_alt; + pre_vector = pre_vector_alt; + } + + void resize_uninitialized(realtype values) { + size_t r = values.size(); + size_t s = real_vector.size() / 2; + if (real_vector.capacity() < s + r) { + real_vector.reserve(s + r); + } + real_vector.resize(s); + pre_vector.resize_uninitialized(s); + for (auto v : values) { + real_vector.push_back(v); + } + auto p = pre_vector.size(); + pre_vector.resize_uninitialized(p + r); + for (auto v : values) { + pre_vector[p] = v; + ++p; + } + } +}; + +} + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider prov(buffer.data(), buffer.size()); + prevector_tester<8, int> test; + + while (prov.remaining_bytes()) { + switch (prov.ConsumeIntegralInRange<int>(0, 13 + 3 * (test.size() > 0))) { + case 0: + test.insert(prov.ConsumeIntegralInRange<size_t>(0, test.size()), prov.ConsumeIntegral<int>()); + break; + case 1: + test.resize(std::max(0, std::min(30, (int)test.size() + prov.ConsumeIntegralInRange<int>(0, 4) - 2))); + break; + case 2: + test.insert(prov.ConsumeIntegralInRange<size_t>(0, test.size()), 1 + prov.ConsumeBool(), prov.ConsumeIntegral<int>()); + break; + case 3: { + int del = prov.ConsumeIntegralInRange<int>(0, test.size()); + int beg = prov.ConsumeIntegralInRange<int>(0, test.size() - del); + test.erase(beg, beg + del); + break; + } + case 4: + test.push_back(prov.ConsumeIntegral<int>()); + break; + case 5: { + int values[4]; + int num = 1 + prov.ConsumeIntegralInRange<int>(0, 3); + for (int k = 0; k < num; ++k) { + values[k] = prov.ConsumeIntegral<int>(); + } + test.insert_range(prov.ConsumeIntegralInRange<size_t>(0, test.size()), values, values + num); + break; + } + case 6: { + int num = 1 + prov.ConsumeIntegralInRange<int>(0, 15); + std::vector<int> values(num); + for (auto& v : values) { + v = prov.ConsumeIntegral<int>(); + } + test.resize_uninitialized(values); + break; + } + case 7: + test.reserve(prov.ConsumeIntegralInRange<size_t>(0, 32767)); + break; + case 8: + test.shrink_to_fit(); + break; + case 9: + test.clear(); + break; + case 10: + test.assign(prov.ConsumeIntegralInRange<size_t>(0, 32767), prov.ConsumeIntegral<int>()); + break; + case 11: + test.swap(); + break; + case 12: + test.copy(); + break; + case 13: + test.move(); + break; + case 14: + test.update(prov.ConsumeIntegralInRange<size_t>(0, test.size() - 1), prov.ConsumeIntegral<int>()); + break; + case 15: + test.erase(prov.ConsumeIntegralInRange<size_t>(0, test.size() - 1)); + break; + case 16: + test.pop_back(); + break; + } + } + + test.test(); +} diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index dc49dd499a..9e3586d162 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -32,7 +32,7 @@ #include <string> #include <vector> -bool ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CTxMemPool& mempool, CConnman* connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc); +bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CTxMemPool& mempool, CConnman* connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc); namespace { diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp new file mode 100644 index 0000000000..12a5dbb607 --- /dev/null +++ b/src/test/fuzz/process_messages.cpp @@ -0,0 +1,75 @@ +// Copyright (c) 2020 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 <consensus/consensus.h> +#include <net.h> +#include <net_processing.h> +#include <protocol.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <test/util/mining.h> +#include <test/util/net.h> +#include <test/util/setup_common.h> +#include <util/memory.h> +#include <validation.h> +#include <validationinterface.h> + +const RegTestingSetup* g_setup; + +void initialize() +{ + static RegTestingSetup setup{}; + g_setup = &setup; + + for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { + MineBlock(g_setup->m_node, CScript() << OP_TRUE); + } + SyncWithValidationInterfaceQueue(); +} + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + + ConnmanTestMsg& connman = *(ConnmanTestMsg*)g_setup->m_node.connman.get(); + std::vector<CNode*> peers; + + const auto num_peers_to_add = fuzzed_data_provider.ConsumeIntegralInRange(1, 3); + for (int i = 0; i < num_peers_to_add; ++i) { + const ServiceFlags service_flags = ServiceFlags(fuzzed_data_provider.ConsumeIntegral<uint64_t>()); + const bool inbound{fuzzed_data_provider.ConsumeBool()}; + const bool block_relay_only{fuzzed_data_provider.ConsumeBool()}; + peers.push_back(MakeUnique<CNode>(i, service_flags, 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, inbound, block_relay_only).release()); + CNode& p2p_node = *peers.back(); + + p2p_node.fSuccessfullyConnected = true; + p2p_node.fPauseSend = false; + p2p_node.nVersion = PROTOCOL_VERSION; + p2p_node.SetSendVersion(PROTOCOL_VERSION); + g_setup->m_node.peer_logic->InitializeNode(&p2p_node); + + connman.AddTestNode(p2p_node); + } + + while (fuzzed_data_provider.ConsumeBool()) { + const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::COMMAND_SIZE).c_str()}; + + CSerializedNetMsg net_msg; + net_msg.command = random_message_type; + net_msg.data = ConsumeRandomLengthByteVector(fuzzed_data_provider); + + CNode& random_node = *peers.at(fuzzed_data_provider.ConsumeIntegralInRange<int>(0, peers.size() - 1)); + + (void)connman.ReceiveMsgFrom(random_node, net_msg); + random_node.fPauseSend = false; + + try { + connman.ProcessMessagesOnce(random_node); + } catch (const std::ios_base::failure&) { + } + } + connman.ClearTestNodes(); + SyncWithValidationInterfaceQueue(); +} diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index b70ea6d90e..ba4b012f95 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -120,4 +120,15 @@ NODISCARD bool MultiplicationOverflow(const T i, const T j) noexcept } } +template <class T> +NODISCARD bool AdditionOverflow(const T i, const T j) noexcept +{ + static_assert(std::is_integral<T>::value, "Integral required."); + if (std::numeric_limits<T>::is_signed) { + return (i > 0 && j > std::numeric_limits<T>::max() - i) || + (i < 0 && j < std::numeric_limits<T>::min() - i); + } + return std::numeric_limits<T>::max() - i < j; +} + #endif // BITCOIN_TEST_FUZZ_UTIL_H diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp new file mode 100644 index 0000000000..09f2f1807f --- /dev/null +++ b/src/test/util/net.cpp @@ -0,0 +1,39 @@ +// Copyright (c) 2020 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/util/net.h> + +#include <chainparams.h> +#include <net.h> + +void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, const char* pch, unsigned int nBytes, bool& complete) const +{ + assert(node.ReceiveMsgBytes(pch, nBytes, complete)); + if (complete) { + size_t nSizeAdded = 0; + auto it(node.vRecvMsg.begin()); + for (; it != node.vRecvMsg.end(); ++it) { + // vRecvMsg contains only completed CNetMessage + // the single possible partially deserialized message are held by TransportDeserializer + nSizeAdded += it->m_raw_message_size; + } + { + LOCK(node.cs_vProcessMsg); + node.vProcessMsg.splice(node.vProcessMsg.end(), node.vRecvMsg, node.vRecvMsg.begin(), it); + node.nProcessQueueSize += nSizeAdded; + node.fPauseRecv = node.nProcessQueueSize > nReceiveFloodSize; + } + } +} + +bool ConnmanTestMsg::ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) const +{ + std::vector<unsigned char> ser_msg_header; + node.m_serializer->prepareForTransport(ser_msg, ser_msg_header); + + bool complete; + NodeReceiveMsgBytes(node, (const char*)ser_msg_header.data(), ser_msg_header.size(), complete); + NodeReceiveMsgBytes(node, (const char*)ser_msg.data.data(), ser_msg.data.size(), complete); + return complete; +} diff --git a/src/test/util/net.h b/src/test/util/net.h new file mode 100644 index 0000000000..ca8cb7fad5 --- /dev/null +++ b/src/test/util/net.h @@ -0,0 +1,33 @@ +// Copyright (c) 2020 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_UTIL_NET_H +#define BITCOIN_TEST_UTIL_NET_H + +#include <net.h> + +struct ConnmanTestMsg : public CConnman { + using CConnman::CConnman; + void AddTestNode(CNode& node) + { + LOCK(cs_vNodes); + vNodes.push_back(&node); + } + void ClearTestNodes() + { + LOCK(cs_vNodes); + for (CNode* node : vNodes) { + delete node; + } + vNodes.clear(); + } + + void ProcessMessagesOnce(CNode& node) { m_msgproc->ProcessMessages(&node, flagInterruptMsgProc); } + + void NodeReceiveMsgBytes(CNode& node, const char* pch, unsigned int nBytes, bool& complete) const; + + bool ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) const; +}; + +#endif // BITCOIN_TEST_UTIL_NET_H diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index d684b97787..a4d0126925 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -139,6 +139,11 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha m_node.banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); m_node.connman = MakeUnique<CConnman>(0x1337, 0x1337); // Deterministic randomness for tests. m_node.peer_logic = MakeUnique<PeerLogicValidation>(m_node.connman.get(), m_node.banman.get(), *m_node.scheduler, *m_node.mempool); + { + CConnman::Options options; + options.m_msgproc = m_node.peer_logic.get(); + m_node.connman->Init(options); + } } TestingSetup::~TestingSetup() diff --git a/src/test/validationinterface_tests.cpp b/src/test/validationinterface_tests.cpp new file mode 100644 index 0000000000..208be92852 --- /dev/null +++ b/src/test/validationinterface_tests.cpp @@ -0,0 +1,60 @@ +// Copyright (c) 2020 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 <boost/test/unit_test.hpp> +#include <consensus/validation.h> +#include <primitives/block.h> +#include <scheduler.h> +#include <test/util/setup_common.h> +#include <util/check.h> +#include <validationinterface.h> + +BOOST_FIXTURE_TEST_SUITE(validationinterface_tests, TestingSetup) + +class TestInterface : public CValidationInterface +{ +public: + TestInterface(std::function<void()> on_call = nullptr, std::function<void()> on_destroy = nullptr) + : m_on_call(std::move(on_call)), m_on_destroy(std::move(on_destroy)) + { + } + virtual ~TestInterface() + { + if (m_on_destroy) m_on_destroy(); + } + void BlockChecked(const CBlock& block, const BlockValidationState& state) override + { + if (m_on_call) m_on_call(); + } + static void Call() + { + CBlock block; + BlockValidationState state; + GetMainSignals().BlockChecked(block, state); + } + std::function<void()> m_on_call; + std::function<void()> m_on_destroy; +}; + +// Regression test to ensure UnregisterAllValidationInterfaces calls don't +// destroy a validation interface while it is being called. Bug: +// https://github.com/bitcoin/bitcoin/pull/18551 +BOOST_AUTO_TEST_CASE(unregister_all_during_call) +{ + bool destroyed = false; + RegisterSharedValidationInterface(std::make_shared<TestInterface>( + [&] { + // First call should decrements reference count 2 -> 1 + UnregisterAllValidationInterfaces(); + BOOST_CHECK(!destroyed); + // Second call should not decrement reference count 1 -> 0 + UnregisterAllValidationInterfaces(); + BOOST_CHECK(!destroyed); + }, + [&] { destroyed = true; })); + TestInterface::Call(); + BOOST_CHECK(destroyed); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index c06647cb0d..11000774c0 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -67,8 +67,8 @@ public: void Clear() { LOCK(m_mutex); - for (auto it = m_list.begin(); it != m_list.end();) { - it = --it->count ? std::next(it) : m_list.erase(it); + for (const auto& entry : m_map) { + if (!--entry.second->count) m_list.erase(entry.second); } m_map.clear(); } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index e4d0a3fa6d..ea54027c48 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -60,12 +60,13 @@ static bool GetWalletAddressesForKey(LegacyScriptPubKeyMan* spk_man, const CWall CKey key; spk_man->GetKey(keyid, key); for (const auto& dest : GetAllDestinationsForKey(key.GetPubKey())) { - if (pwallet->mapAddressBook.count(dest)) { + const auto* address_book_entry = pwallet->FindAddressBookEntry(dest); + if (address_book_entry) { if (!strAddr.empty()) { strAddr += ","; } strAddr += EncodeDestination(dest); - strLabel = EncodeDumpString(pwallet->mapAddressBook.at(dest).name); + strLabel = EncodeDumpString(address_book_entry->GetLabel()); fLabelFound = true; } } @@ -168,7 +169,7 @@ UniValue importprivkey(const JSONRPCRequest& request) // label all new addresses, and label existing addresses if a // label was passed. for (const auto& dest : GetAllDestinationsForKey(pubkey)) { - if (!request.params[1].isNull() || pwallet->mapAddressBook.count(dest) == 0) { + if (!request.params[1].isNull() || !pwallet->FindAddressBookEntry(dest)) { pwallet->SetAddressBook(dest, strLabel, "receive"); } } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d8b3b57b61..61ad2f1198 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -499,8 +499,9 @@ static UniValue listaddressgroupings(const JSONRPCRequest& request) addressInfo.push_back(EncodeDestination(address)); addressInfo.push_back(ValueFromAmount(balances[address])); { - if (pwallet->mapAddressBook.find(address) != pwallet->mapAddressBook.end()) { - addressInfo.push_back(pwallet->mapAddressBook.find(address)->second.name); + const auto* address_book_entry = pwallet->FindAddressBookEntry(address); + if (address_book_entry) { + addressInfo.push_back(address_book_entry->GetLabel()); } } jsonGrouping.push_back(addressInfo); @@ -1092,13 +1093,13 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, const CWalle UniValue ret(UniValue::VARR); std::map<std::string, tallyitem> label_tally; - // Create mapAddressBook iterator + // Create m_address_book iterator // If we aren't filtering, go from begin() to end() - auto start = pwallet->mapAddressBook.begin(); - auto end = pwallet->mapAddressBook.end(); + auto start = pwallet->m_address_book.begin(); + auto end = pwallet->m_address_book.end(); // If we are filtering, find() the applicable entry if (has_filtered_address) { - start = pwallet->mapAddressBook.find(filtered_address); + start = pwallet->m_address_book.find(filtered_address); if (start != end) { end = std::next(start); } @@ -1106,8 +1107,9 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, const CWalle for (auto item_it = start; item_it != end; ++item_it) { + if (item_it->second.IsChange()) continue; const CTxDestination& address = item_it->first; - const std::string& label = item_it->second.name; + const std::string& label = item_it->second.GetLabel(); auto it = mapTally.find(address); if (it == mapTally.end() && !fIncludeEmpty) continue; @@ -1307,8 +1309,9 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, const CWalle MaybePushAddress(entry, s.destination); entry.pushKV("category", "send"); entry.pushKV("amount", ValueFromAmount(-s.amount)); - if (pwallet->mapAddressBook.count(s.destination)) { - entry.pushKV("label", pwallet->mapAddressBook.at(s.destination).name); + const auto* address_book_entry = pwallet->FindAddressBookEntry(s.destination); + if (address_book_entry) { + entry.pushKV("label", address_book_entry->GetLabel()); } entry.pushKV("vout", s.vout); entry.pushKV("fee", ValueFromAmount(-nFee)); @@ -1324,8 +1327,9 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, const CWalle for (const COutputEntry& r : listReceived) { std::string label; - if (pwallet->mapAddressBook.count(r.destination)) { - label = pwallet->mapAddressBook.at(r.destination).name; + const auto* address_book_entry = pwallet->FindAddressBookEntry(r.destination); + if (address_book_entry) { + label = address_book_entry->GetLabel(); } if (filter_label && label != *filter_label) { continue; @@ -1349,7 +1353,7 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, const CWalle entry.pushKV("category", "receive"); } entry.pushKV("amount", ValueFromAmount(r.amount)); - if (pwallet->mapAddressBook.count(r.destination)) { + if (address_book_entry) { entry.pushKV("label", label); } entry.pushKV("vout", r.vout); @@ -1912,44 +1916,52 @@ static UniValue walletpassphrase(const JSONRPCRequest& request) }, }.Check(request); - auto locked_chain = pwallet->chain().lock(); - LOCK(pwallet->cs_wallet); - - if (!pwallet->IsCrypted()) { - throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called."); - } + int64_t nSleepTime; + { + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); - // Note that the walletpassphrase is stored in request.params[0] which is not mlock()ed - SecureString strWalletPass; - strWalletPass.reserve(100); - // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) - // Alternately, find a way to make request.params[0] mlock()'d to begin with. - strWalletPass = request.params[0].get_str().c_str(); + if (!pwallet->IsCrypted()) { + throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called."); + } - // Get the timeout - int64_t nSleepTime = request.params[1].get_int64(); - // Timeout cannot be negative, otherwise it will relock immediately - if (nSleepTime < 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative."); - } - // Clamp timeout - constexpr int64_t MAX_SLEEP_TIME = 100000000; // larger values trigger a macos/libevent bug? - if (nSleepTime > MAX_SLEEP_TIME) { - nSleepTime = MAX_SLEEP_TIME; - } + // Note that the walletpassphrase is stored in request.params[0] which is not mlock()ed + SecureString strWalletPass; + strWalletPass.reserve(100); + // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) + // Alternately, find a way to make request.params[0] mlock()'d to begin with. + strWalletPass = request.params[0].get_str().c_str(); + + // Get the timeout + nSleepTime = request.params[1].get_int64(); + // Timeout cannot be negative, otherwise it will relock immediately + if (nSleepTime < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative."); + } + // Clamp timeout + constexpr int64_t MAX_SLEEP_TIME = 100000000; // larger values trigger a macos/libevent bug? + if (nSleepTime > MAX_SLEEP_TIME) { + nSleepTime = MAX_SLEEP_TIME; + } - if (strWalletPass.empty()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase can not be empty"); - } + if (strWalletPass.empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase can not be empty"); + } - if (!pwallet->Unlock(strWalletPass)) { - throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); - } + if (!pwallet->Unlock(strWalletPass)) { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); + } - pwallet->TopUpKeyPool(); + pwallet->TopUpKeyPool(); - pwallet->nRelockTime = GetTime() + nSleepTime; + pwallet->nRelockTime = GetTime() + nSleepTime; + } + // rpcRunLater must be called without cs_wallet held otherwise a deadlock + // can occur. The deadlock would happen when RPCRunLater removes the + // previous timer (and waits for the callback to finish if already running) + // and the callback locks cs_wallet. + AssertLockNotHeld(wallet->cs_wallet); // Keep a weak pointer to the wallet so that it is possible to unload the // wallet before the following callback is called. If a valid shared pointer // is acquired in the callback then the wallet is still loaded. @@ -2949,9 +2961,9 @@ static UniValue listunspent(const JSONRPCRequest& request) if (fValidAddress) { entry.pushKV("address", EncodeDestination(address)); - auto i = pwallet->mapAddressBook.find(address); - if (i != pwallet->mapAddressBook.end()) { - entry.pushKV("label", i->second.name); + const auto* address_book_entry = pwallet->FindAddressBookEntry(address); + if (address_book_entry) { + entry.pushKV("label", address_book_entry->GetLabel()); } std::unique_ptr<SigningProvider> provider = pwallet->GetSolvingProvider(scriptPubKey); @@ -3698,7 +3710,7 @@ static UniValue AddressBookDataToJSON(const CAddressBookData& data, const bool v { UniValue ret(UniValue::VOBJ); if (verbose) { - ret.pushKV("name", data.name); + ret.pushKV("name", data.GetLabel()); } ret.pushKV("purpose", data.purpose); return ret; @@ -3808,8 +3820,9 @@ UniValue getaddressinfo(const JSONRPCRequest& request) // DEPRECATED: Return label field if existing. Currently only one label can // be associated with an address, so the label should be equivalent to the // value of the name key/value pair in the labels array below. - if ((pwallet->chain().rpcEnableDeprecated("label")) && (pwallet->mapAddressBook.count(dest))) { - ret.pushKV("label", pwallet->mapAddressBook.at(dest).name); + const auto* address_book_entry = pwallet->FindAddressBookEntry(dest); + if (pwallet->chain().rpcEnableDeprecated("label") && address_book_entry) { + ret.pushKV("label", address_book_entry->GetLabel()); } ret.pushKV("ischange", pwallet->IsChange(scriptPubKey)); @@ -3832,14 +3845,13 @@ UniValue getaddressinfo(const JSONRPCRequest& request) // stable if we allow multiple labels to be associated with an address in // the future. UniValue labels(UniValue::VARR); - std::map<CTxDestination, CAddressBookData>::const_iterator mi = pwallet->mapAddressBook.find(dest); - if (mi != pwallet->mapAddressBook.end()) { + if (address_book_entry) { // DEPRECATED: The previous behavior of returning an array containing a // JSON object of `name` and `purpose` key/value pairs is deprecated. if (pwallet->chain().rpcEnableDeprecated("labelspurpose")) { - labels.push_back(AddressBookDataToJSON(mi->second, true)); + labels.push_back(AddressBookDataToJSON(*address_book_entry, true)); } else { - labels.push_back(mi->second.name); + labels.push_back(address_book_entry->GetLabel()); } } ret.pushKV("labels", std::move(labels)); @@ -3883,10 +3895,11 @@ static UniValue getaddressesbylabel(const JSONRPCRequest& request) // Find all addresses that have the given label UniValue ret(UniValue::VOBJ); std::set<std::string> addresses; - for (const std::pair<const CTxDestination, CAddressBookData>& item : pwallet->mapAddressBook) { - if (item.second.name == label) { + for (const std::pair<const CTxDestination, CAddressBookData>& item : pwallet->m_address_book) { + if (item.second.IsChange()) continue; + if (item.second.GetLabel() == label) { std::string address = EncodeDestination(item.first); - // CWallet::mapAddressBook is not expected to contain duplicate + // CWallet::m_address_book is not expected to contain duplicate // address strings, but build a separate set as a precaution just in // case it does. bool unique = addresses.emplace(address).second; @@ -3947,9 +3960,10 @@ static UniValue listlabels(const JSONRPCRequest& request) // Add to a set to sort by label name, then insert into Univalue array std::set<std::string> label_set; - for (const std::pair<const CTxDestination, CAddressBookData>& entry : pwallet->mapAddressBook) { + for (const std::pair<const CTxDestination, CAddressBookData>& entry : pwallet->m_address_book) { + if (entry.second.IsChange()) continue; if (purpose.empty() || entry.second.purpose == purpose) { - label_set.insert(entry.second.name); + label_set.insert(entry.second.GetLabel()); } } @@ -4242,6 +4256,8 @@ UniValue importprunedfunds(const JSONRPCRequest& request); UniValue removeprunedfunds(const JSONRPCRequest& request); UniValue importmulti(const JSONRPCRequest& request); +void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector<std::unique_ptr<interfaces::Handler>>& handlers) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -4305,8 +4321,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector<std::unique_ptr<interfaces::Handler>>& handlers) -{ for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) handlers.emplace_back(chain.handleRpc(commands[vcidx])); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bc9f84a11d..45f5542cad 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1242,8 +1242,9 @@ bool CWallet::IsChange(const CScript& script) const return true; LOCK(cs_wallet); - if (!mapAddressBook.count(address)) + if (!FindAddressBookEntry(address)) { return true; + } } return false; } @@ -3196,11 +3197,11 @@ bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& add bool fUpdated = false; { LOCK(cs_wallet); - std::map<CTxDestination, CAddressBookData>::iterator mi = mapAddressBook.find(address); - fUpdated = mi != mapAddressBook.end(); - mapAddressBook[address].name = strName; + std::map<CTxDestination, CAddressBookData>::iterator mi = m_address_book.find(address); + fUpdated = (mi != m_address_book.end() && !mi->second.IsChange()); + m_address_book[address].SetLabel(strName); if (!strPurpose.empty()) /* update purpose only if requested */ - mapAddressBook[address].purpose = strPurpose; + m_address_book[address].purpose = strPurpose; } NotifyAddressBookChanged(this, address, strName, IsMine(address) != ISMINE_NO, strPurpose, (fUpdated ? CT_UPDATED : CT_NEW) ); @@ -3217,16 +3218,24 @@ bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& s bool CWallet::DelAddressBook(const CTxDestination& address) { + // If we want to delete receiving addresses, we need to take care that DestData "used" (and possibly newer DestData) gets preserved (and the "deleted" address transformed into a change entry instead of actually being deleted) + // NOTE: This isn't a problem for sending addresses because they never have any DestData yet! + // When adding new DestData, it should be considered here whether to retain or delete it (or move it?). + if (IsMine(address)) { + WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT); + return false; + } + { LOCK(cs_wallet); // Delete destdata tuples associated with address std::string strAddress = EncodeDestination(address); - for (const std::pair<const std::string, std::string> &item : mapAddressBook[address].destdata) + for (const std::pair<const std::string, std::string> &item : m_address_book[address].destdata) { WalletBatch(*database).EraseDestData(strAddress, item.first); } - mapAddressBook.erase(address); + m_address_book.erase(address); } NotifyAddressBookChanged(this, address, "", IsMine(address) != ISMINE_NO, "", CT_DELETED); @@ -3462,10 +3471,11 @@ std::set<CTxDestination> CWallet::GetLabelAddresses(const std::string& label) co { LOCK(cs_wallet); std::set<CTxDestination> result; - for (const std::pair<const CTxDestination, CAddressBookData>& item : mapAddressBook) + for (const std::pair<const CTxDestination, CAddressBookData>& item : m_address_book) { + if (item.second.IsChange()) continue; const CTxDestination& address = item.first; - const std::string& strName = item.second.name; + const std::string& strName = item.second.GetLabel(); if (strName == label) result.insert(address); } @@ -3666,26 +3676,26 @@ bool CWallet::AddDestData(WalletBatch& batch, const CTxDestination &dest, const if (boost::get<CNoDestination>(&dest)) return false; - mapAddressBook[dest].destdata.insert(std::make_pair(key, value)); + m_address_book[dest].destdata.insert(std::make_pair(key, value)); return batch.WriteDestData(EncodeDestination(dest), key, value); } bool CWallet::EraseDestData(WalletBatch& batch, const CTxDestination &dest, const std::string &key) { - if (!mapAddressBook[dest].destdata.erase(key)) + if (!m_address_book[dest].destdata.erase(key)) return false; return batch.EraseDestData(EncodeDestination(dest), key); } void CWallet::LoadDestData(const CTxDestination &dest, const std::string &key, const std::string &value) { - mapAddressBook[dest].destdata.insert(std::make_pair(key, value)); + m_address_book[dest].destdata.insert(std::make_pair(key, value)); } bool CWallet::GetDestData(const CTxDestination &dest, const std::string &key, std::string *value) const { - std::map<CTxDestination, CAddressBookData>::const_iterator i = mapAddressBook.find(dest); - if(i != mapAddressBook.end()) + std::map<CTxDestination, CAddressBookData>::const_iterator i = m_address_book.find(dest); + if(i != m_address_book.end()) { CAddressBookData::StringMap::const_iterator j = i->second.destdata.find(key); if(j != i->second.destdata.end()) @@ -3701,7 +3711,7 @@ bool CWallet::GetDestData(const CTxDestination &dest, const std::string &key, st std::vector<std::string> CWallet::GetDestValues(const std::string& prefix) const { std::vector<std::string> values; - for (const auto& address : mapAddressBook) { + for (const auto& address : m_address_book) { for (const auto& data : address.second.destdata) { if (!data.first.compare(0, prefix.size(), prefix)) { values.emplace_back(data.second); @@ -4103,12 +4113,22 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, { walletInstance->WalletLogPrintf("setKeyPool.size() = %u\n", walletInstance->GetKeyPoolSize()); walletInstance->WalletLogPrintf("mapWallet.size() = %u\n", walletInstance->mapWallet.size()); - walletInstance->WalletLogPrintf("mapAddressBook.size() = %u\n", walletInstance->mapAddressBook.size()); + walletInstance->WalletLogPrintf("m_address_book.size() = %u\n", walletInstance->m_address_book.size()); } return walletInstance; } +const CAddressBookData* CWallet::FindAddressBookEntry(const CTxDestination& dest, bool allow_change) const +{ + const auto& address_book_it = m_address_book.find(dest); + if (address_book_it == m_address_book.end()) return nullptr; + if ((!allow_change) && address_book_it->second.IsChange()) { + return nullptr; + } + return &address_book_it->second; +} + void CWallet::postInitProcess() { auto locked_chain = chain().lock(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index fe59773488..6c54c72e76 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -181,14 +181,23 @@ public: /** Address book data */ class CAddressBookData { +private: + bool m_change{true}; + std::string m_label; public: - std::string name; std::string purpose; CAddressBookData() : purpose("unknown") {} typedef std::map<std::string, std::string> StringMap; StringMap destdata; + + bool IsChange() const { return m_change; } + const std::string& GetLabel() const { return m_label; } + void SetLabel(const std::string& label) { + m_change = false; + m_label = label; + } }; struct CRecipient @@ -775,7 +784,8 @@ public: int64_t nOrderPosNext GUARDED_BY(cs_wallet) = 0; uint64_t nAccountingEntryNumber = 0; - std::map<CTxDestination, CAddressBookData> mapAddressBook GUARDED_BY(cs_wallet); + std::map<CTxDestination, CAddressBookData> m_address_book GUARDED_BY(cs_wallet); + const CAddressBookData* FindAddressBookEntry(const CTxDestination&, bool allow_change = false) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); std::set<COutPoint> setLockedCoins GUARDED_BY(cs_wallet); @@ -842,7 +852,10 @@ public: bool LoadMinVersion(int nVersion) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); nWalletVersion = nVersion; nWalletMaxVersion = std::max(nWalletMaxVersion, nVersion); return true; } - //! Adds a destination data tuple to the store, and saves it to disk + /** + * Adds a destination data tuple to the store, and saves it to disk + * When adding new fields, take care to consider how DelAddressBook should handle it! + */ bool AddDestData(WalletBatch& batch, const CTxDestination& dest, const std::string& key, const std::string& value) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Erases a destination data tuple in the store and on disk bool EraseDestData(WalletBatch& batch, const CTxDestination& dest, const std::string& key) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index a1928f45c4..568b21ed00 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -206,11 +206,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, if (strType == DBKeys::NAME) { std::string strAddress; ssKey >> strAddress; - ssValue >> pwallet->mapAddressBook[DecodeDestination(strAddress)].name; + std::string label; + ssValue >> label; + pwallet->m_address_book[DecodeDestination(strAddress)].SetLabel(label); } else if (strType == DBKeys::PURPOSE) { std::string strAddress; ssKey >> strAddress; - ssValue >> pwallet->mapAddressBook[DecodeDestination(strAddress)].purpose; + ssValue >> pwallet->m_address_book[DecodeDestination(strAddress)].purpose; } else if (strType == DBKeys::TX) { uint256 hash; ssKey >> hash; diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index fbfdf9dd6b..8c918f4eb0 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -99,7 +99,7 @@ static void WalletShowInfo(CWallet* wallet_instance) tfm::format(std::cout, "HD (hd seed available): %s\n", wallet_instance->IsHDEnabled() ? "yes" : "no"); tfm::format(std::cout, "Keypool Size: %u\n", wallet_instance->GetKeyPoolSize()); tfm::format(std::cout, "Transactions: %zu\n", wallet_instance->mapWallet.size()); - tfm::format(std::cout, "Address Book: %zu\n", wallet_instance->mapAddressBook.size()); + tfm::format(std::cout, "Address Book: %zu\n", wallet_instance->m_address_book.size()); } bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py new file mode 100755 index 0000000000..6046237101 --- /dev/null +++ b/test/functional/p2p_addr_relay.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +""" +Test addr relay +""" + +from test_framework.messages import ( + CAddress, + NODE_NETWORK, + NODE_WITNESS, + msg_addr, +) +from test_framework.mininode import ( + P2PInterface, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, +) +import time + +ADDRS = [] +for i in range(10): + addr = CAddress() + addr.time = int(time.time()) + i + addr.nServices = NODE_NETWORK | NODE_WITNESS + addr.ip = "123.123.123.{}".format(i % 256) + addr.port = 8333 + i + ADDRS.append(addr) + + +class AddrReceiver(P2PInterface): + def on_addr(self, message): + for addr in message.addrs: + assert_equal(addr.nServices, 9) + assert addr.ip.startswith('123.123.123.') + assert (8333 <= addr.port < 8343) + + +class AddrTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = False + self.num_nodes = 1 + + def run_test(self): + self.log.info('Create connection that sends addr messages') + addr_source = self.nodes[0].add_p2p_connection(P2PInterface()) + msg = msg_addr() + + self.log.info('Send too large addr message') + msg.addrs = ADDRS * 101 + with self.nodes[0].assert_debug_log(['message addr size() = 1010']): + addr_source.send_and_ping(msg) + + self.log.info('Check that addr message content is relayed and added to addrman') + addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) + msg.addrs = ADDRS + with self.nodes[0].assert_debug_log([ + 'Added 10 addresses from 127.0.0.1: 0 tried', + 'received: addr (301 bytes) peer=0', + 'sending addr (301 bytes) peer=1', + ]): + addr_source.send_and_ping(msg) + self.nodes[0].setmocktime(int(time.time()) + 30 * 60) + addr_receiver.sync_with_ping() + + +if __name__ == '__main__': + AddrTest().main() diff --git a/test/functional/rpc_signrawtransaction.py b/test/functional/rpc_signrawtransaction.py index 780758e219..17686f3a78 100755 --- a/test/functional/rpc_signrawtransaction.py +++ b/test/functional/rpc_signrawtransaction.py @@ -4,10 +4,11 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test transaction signing using the signrawtransaction* RPCs.""" +from test_framework.address import check_script, script_to_p2sh from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error, hex_str_to_bytes +from test_framework.util import assert_equal, assert_raises_rpc_error, find_vout_for_address, hex_str_to_bytes from test_framework.messages import sha256 -from test_framework.script import CScript, OP_0 +from test_framework.script import CScript, OP_0, OP_CHECKSIG from decimal import Decimal @@ -168,6 +169,44 @@ class SignRawTransactionsTest(BitcoinTestFramework): assert 'complete' in spending_tx_signed assert_equal(spending_tx_signed['complete'], True) + self.log.info('Try with a P2PKH script as the witnessScript') + embedded_addr_info = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress('', 'legacy')) + embedded_privkey = self.nodes[1].dumpprivkey(embedded_addr_info['address']) + witness_script = embedded_addr_info['scriptPubKey'] + redeem_script = CScript([OP_0, sha256(check_script(witness_script))]).hex() + addr = script_to_p2sh(redeem_script) + script_pub_key = self.nodes[1].validateaddress(addr)['scriptPubKey'] + # Fund that address + txid = self.nodes[0].sendtoaddress(addr, 10) + vout = find_vout_for_address(self.nodes[0], txid, addr) + self.nodes[0].generate(1) + # Now create and sign a transaction spending that output on node[0], which doesn't know the scripts or keys + spending_tx = self.nodes[0].createrawtransaction([{'txid': txid, 'vout': vout}], {self.nodes[1].getnewaddress(): Decimal("9.999")}) + spending_tx_signed = self.nodes[0].signrawtransactionwithkey(spending_tx, [embedded_privkey], [{'txid': txid, 'vout': vout, 'scriptPubKey': script_pub_key, 'redeemScript': redeem_script, 'witnessScript': witness_script, 'amount': 10}]) + # Check the signing completed successfully + assert 'complete' in spending_tx_signed + assert_equal(spending_tx_signed['complete'], True) + self.nodes[0].sendrawtransaction(spending_tx_signed['hex']) + + self.log.info('Try with a P2PK script as the witnessScript') + embedded_addr_info = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress('', 'legacy')) + embedded_privkey = self.nodes[1].dumpprivkey(embedded_addr_info['address']) + witness_script = CScript([hex_str_to_bytes(embedded_addr_info['pubkey']), OP_CHECKSIG]).hex() + redeem_script = CScript([OP_0, sha256(check_script(witness_script))]).hex() + addr = script_to_p2sh(redeem_script) + script_pub_key = self.nodes[1].validateaddress(addr)['scriptPubKey'] + # Fund that address + txid = self.nodes[0].sendtoaddress(addr, 10) + vout = find_vout_for_address(self.nodes[0], txid, addr) + self.nodes[0].generate(1) + # Now create and sign a transaction spending that output on node[0], which doesn't know the scripts or keys + spending_tx = self.nodes[0].createrawtransaction([{'txid': txid, 'vout': vout}], {self.nodes[1].getnewaddress(): Decimal("9.999")}) + spending_tx_signed = self.nodes[0].signrawtransactionwithkey(spending_tx, [embedded_privkey], [{'txid': txid, 'vout': vout, 'scriptPubKey': script_pub_key, 'redeemScript': redeem_script, 'witnessScript': witness_script, 'amount': 10}]) + # Check the signing completed successfully + assert 'complete' in spending_tx_signed + assert_equal(spending_tx_signed['complete'], True) + self.nodes[0].sendrawtransaction(spending_tx_signed['hex']) + def run_test(self): self.successful_signing_test() self.script_verification_error_test() diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 8260c917fe..53bc5ca9e7 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -233,6 +233,10 @@ class TestNode(): # -342 Service unavailable, RPC server started but is shutting down due to error if e.error['code'] != -28 and e.error['code'] != -342: raise # unknown JSON RPC exception + except ConnectionResetError: + # This might happen when the RPC server is in warmup, but shut down before the call to getblockcount + # succeeds. Try again to properly raise the FailedToStartError + pass except ValueError as e: # cookie file not found and no rpcuser or rpcassword. bitcoind still starting if "No RPC credentials" not in str(e): raise diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2f307750a9..faa2dee4ed 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -144,6 +144,7 @@ BASE_SCRIPTS = [ 'rpc_blockchain.py', 'rpc_deprecated.py', 'wallet_disable.py', + 'p2p_addr_relay.py', 'rpc_net.py', 'wallet_keypool.py', 'p2p_mempool.py', diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index 8e2dc03ac2..2ce8d459c6 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -83,6 +83,7 @@ class AvoidReuseTest(BitcoinTestFramework): self.nodes[0].generate(110) self.sync_all() + self.test_change_remains_change(self.nodes[1]) reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) self.test_fund_send_fund_senddirty() reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) @@ -137,6 +138,30 @@ class AvoidReuseTest(BitcoinTestFramework): # Unload temp wallet self.nodes[1].unloadwallet(tempwallet) + def test_change_remains_change(self, node): + self.log.info("Test that change doesn't turn into non-change when spent") + + reset_balance(node, node.getnewaddress()) + addr = node.getnewaddress() + txid = node.sendtoaddress(addr, 1) + out = node.listunspent(minconf=0, query_options={'minimumAmount': 2}) + assert_equal(len(out), 1) + assert_equal(out[0]['txid'], txid) + changeaddr = out[0]['address'] + + # Make sure it's starting out as change as expected + assert node.getaddressinfo(changeaddr)['ischange'] + for logical_tx in node.listtransactions(): + assert logical_tx.get('address') != changeaddr + + # Spend it + reset_balance(node, node.getnewaddress()) + + # It should still be change + assert node.getaddressinfo(changeaddr)['ischange'] + for logical_tx in node.listtransactions(): + assert logical_tx.get('address') != changeaddr + def test_fund_send_fund_senddirty(self): ''' Test the same as test_fund_send_fund_send, except send the 10 BTC with diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 38c9807757..0b3dea94d5 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -253,12 +253,17 @@ def test_dust_to_fee(self, rbf_node, dest_address): self.log.info('Test that bumped output that is dust is dropped to fee') rbfid = spend_one_input(rbf_node, dest_address) fulltx = rbf_node.getrawtransaction(rbfid, 1) - # size of transaction (p2wpkh, 1 input, 2 outputs): 141 vbytes - assert_equal(fulltx["vsize"], 141) - # bump with fee_rate of 0.00350000 BTC per 1000 vbytes - # expected bump fee of 141 vbytes * fee_rate 0.00350000 BTC / 1000 vbytes = 0.00049350 BTC - # but dust is dropped, so actual bump fee is 0.00050000 - bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate": 0.0035}) + # The DER formatting used by Bitcoin to serialize ECDSA signatures means that signatures can have a + # variable size of 70-72 bytes (or possibly even less), with most being 71 or 72 bytes. The signature + # in the witness is divided by 4 for the vsize, so this variance can take the weight across a 4-byte + # boundary. Thus expected transaction size (p2wpkh, 1 input, 2 outputs) is 140-141 vbytes, usually 141. + if not 140 <= fulltx["vsize"] <= 141: + raise AssertionError("Invalid tx vsize of {} (140-141 expected), full tx: {}".format(fulltx["vsize"], fulltx)) + # Bump with fee_rate of 0.00350250 BTC per 1000 vbytes to create dust. + # Expected fee is 141 vbytes * fee_rate 0.00350250 BTC / 1000 vbytes = 0.00049385 BTC. + # or occasionally 140 vbytes * fee_rate 0.00350250 BTC / 1000 vbytes = 0.00049035 BTC. + # Dust should be dropped to the fee, so actual bump fee is 0.00050000 BTC. + bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate": 0.00350250}) full_bumped_tx = rbf_node.getrawtransaction(bumped_tx["txid"], 1) assert_equal(bumped_tx["fee"], Decimal("0.00050000")) assert_equal(len(fulltx["vout"]), 2) diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index bb93060739..87f4255a6f 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -147,6 +147,7 @@ def merge_inputs(*, corpus, test_list, build_dir, merge_dir): args = [ os.path.join(build_dir, 'src', 'test', 'fuzz', t), '-merge=1', + '-use_value_profile=1', # Also done by oss-fuzz https://github.com/google/oss-fuzz/issues/1406#issuecomment-387790487 os.path.join(corpus, t), os.path.join(merge_dir, t), ] |