diff options
65 files changed, 845 insertions, 317 deletions
diff --git a/.github/ISSUE_TEMPLATE/good_first_issue.md b/.github/ISSUE_TEMPLATE/good_first_issue.md index 6782218616..c441f7a307 100644 --- a/.github/ISSUE_TEMPLATE/good_first_issue.md +++ b/.github/ISSUE_TEMPLATE/good_first_issue.md @@ -7,13 +7,14 @@ assignees: '' --- -The purpose of the `good first issue` label is to highlight which issues are suitable for a new contributor without a deep understanding of the codebase. -Useful skills: +#### Useful skills: + +<!-- (For example, “C++11 std::thread”, “Qt5 GUI and async GUI design” or “basic understanding of Bitcoin mining and the Bitcoin Core RPC interface”.) --> -(For example, “C++11 std::thread”, “Qt5 GUI and async GUI design” or “basic understanding of Bitcoin mining and the Bitcoin Core RPC interface”.) +#### Want to work on this issue? -Want to work on this issue? +The purpose of the `good first issue` label is to highlight which issues are suitable for a new contributor without a deep understanding of the codebase. You do not need to request permission to start working on this. You are encouraged to comment on the issue if you are planning to work on it. This will help other contributors monitor which issues are actively being addressed and is also an effective way to request assistance if and when you need it. diff --git a/.travis.yml b/.travis.yml index 4f9dbeded4..38ad79af29 100644 --- a/.travis.yml +++ b/.travis.yml @@ -93,6 +93,7 @@ jobs: arch: s390x env: >- FILE_ENV="./ci/test/00_setup_env_s390x.sh" + QEMU_USER_CMD="" # Can run the tests natively without qemu - stage: test name: 'Win64 [GOAL: deploy] [unit tests, no gui, no functional tests]' @@ -110,7 +111,7 @@ jobs: FILE_ENV="./ci/test/00_setup_env_native_centos.sh" - stage: test - name: 'x86_64 Linux [GOAL: install] [bionic] [uses qt5 dev package instead of depends Qt to speed up build and avoid timeout] [unsigned char]' + name: 'x86_64 Linux [GOAL: install] [bionic] [uses qt5 dev package and some depends packages] [unsigned char]' env: >- FILE_ENV="./ci/test/00_setup_env_native_qt5.sh" @@ -126,6 +127,11 @@ jobs: FILE_ENV="./ci/test/00_setup_env_native_asan.sh" - stage: test + name: 'x86_64 Linux [GOAL: install] [bionic] [no depends, only system libs, valgrind]' + env: >- + FILE_ENV="./ci/test/00_setup_env_native_valgrind.sh" + + - stage: test name: 'x86_64 Linux [GOAL: install] [bionic] [no depends, only system libs, sanitizers: fuzzer,address,undefined]' env: >- FILE_ENV="./ci/test/00_setup_env_native_fuzz.sh" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ecb5704a8f..1e34f65728 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,7 +16,7 @@ release cycle, overall merging, moderation and appointment of maintainers. If you're looking for somewhere to start contributing, check out the [good first issue](https://github.com/bitcoin/bitcoin/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) -list. +list or participate in a weekly [Bitcoin Core PR Review Club](https://bitcoincore.reviews/) meeting. Communication Channels ---------------------- diff --git a/Makefile.am b/Makefile.am index f121c33b24..22b83e80dd 100644 --- a/Makefile.am +++ b/Makefile.am @@ -25,7 +25,7 @@ BITCOIN_QT_BIN=$(top_builddir)/src/qt/$(BITCOIN_GUI_NAME)$(EXEEXT) BITCOIN_CLI_BIN=$(top_builddir)/src/$(BITCOIN_CLI_NAME)$(EXEEXT) BITCOIN_TX_BIN=$(top_builddir)/src/$(BITCOIN_TX_NAME)$(EXEEXT) BITCOIN_WALLET_BIN=$(top_builddir)/src/$(BITCOIN_WALLET_TOOL_NAME)$(EXEEXT) -BITCOIN_WIN_INSTALLER=$(PACKAGE)-$(PACKAGE_VERSION)-win$(WINDOWS_BITS)-setup$(EXEEXT) +BITCOIN_WIN_INSTALLER=$(PACKAGE)-$(PACKAGE_VERSION)-win64-setup$(EXEEXT) empty := space := $(empty) $(empty) diff --git a/ci/test/00_setup_env_native_valgrind.sh b/ci/test/00_setup_env_native_valgrind.sh new file mode 100644 index 0000000000..906ffd7d79 --- /dev/null +++ b/ci/test/00_setup_env_native_valgrind.sh @@ -0,0 +1,15 @@ +#!/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 PACKAGES="valgrind clang llvm python3-zmq libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev" +export USE_VALGRIND=1 +export NO_DEPENDS=1 +export TEST_RUNNER_EXTRA="p2p_segwit.py" # Only run one test for now. TODO enable all and bump timeouts +export RUN_FUNCTIONAL_TESTS=true +export GOAL="install" +export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC=clang CXX=clang++" # TODO enable GUI diff --git a/ci/test/00_setup_env_s390x.sh b/ci/test/00_setup_env_s390x.sh index 89660c7fa4..637d549553 100644 --- a/ci/test/00_setup_env_s390x.sh +++ b/ci/test/00_setup_env_s390x.sh @@ -6,9 +6,11 @@ export LC_ALL=C.UTF-8 -export HOST=s390x-unknown-linux-gnu -export PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" -export NO_DEPENDS=1 +export HOST=s390x-linux-gnu +# The host arch is unknown, so we run the tests through qemu. +# If the host is s390x and wants to run the tests natively, it can set QEMU_USER_CMD to the empty string. +export QEMU_USER_CMD="${QEMU_USER_CMD:"qemu-s390x"}" +export PACKAGES="python3-zmq bsdmainutils qemu-user" 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 c721345250..62f30535cb 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -41,7 +41,7 @@ export ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order= export LSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/lsan" export TSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/tsan:log_path=${BASE_SCRATCH_DIR}/sanitizer-output/tsan" export UBSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" -env | grep -E '^(BITCOIN_CONFIG|CCACHE_|WINEDEBUG|LC_ALL|BOOST_TEST_RANDOM|CONFIG_SHELL|(ASAN|LSAN|TSAN|UBSAN)_OPTIONS)' | tee /tmp/env +env | grep -E '^(BITCOIN_CONFIG|BASE_|CCACHE_|WINEDEBUG|LC_ALL|BOOST_TEST_RANDOM|CONFIG_SHELL|(ASAN|LSAN|TSAN|UBSAN)_OPTIONS)' | tee /tmp/env if [[ $HOST = *-mingw32 ]]; then DOCKER_ADMIN="--cap-add SYS_ADMIN" elif [[ $BITCOIN_CONFIG = *--with-sanitizers=*address* ]]; then # If ran with (ASan + LSan), Docker needs access to ptrace (https://github.com/google/sanitizers/issues/764) diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index a8e0a50f36..e641cef279 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -8,7 +8,6 @@ export LC_ALL=C.UTF-8 if [ -n "$QEMU_USER_CMD" ]; then BEGIN_FOLD wrap-qemu - echo "Prepare to run functional tests for HOST=$HOST" # Generate all binaries, so that they can be wrapped DOCKER_EXEC make $MAKEJOBS -C src/secp256k1 VERBOSE=1 DOCKER_EXEC make $MAKEJOBS -C src/univalue VERBOSE=1 @@ -25,6 +24,12 @@ if [ -n "$QEMU_USER_CMD" ]; then END_FOLD fi +if [ -n "$USE_VALGRIND" ]; then + BEGIN_FOLD wrap-valgrind + DOCKER_EXEC "${BASE_ROOT_DIR}/ci/test/wrap-valgrind.sh" + END_FOLD +fi + 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 diff --git a/ci/test/wrap-valgrind.sh b/ci/test/wrap-valgrind.sh new file mode 100755 index 0000000000..d2192061db --- /dev/null +++ b/ci/test/wrap-valgrind.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# +# Copyright (c) 2018 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +export LC_ALL=C.UTF-8 + +for b_name in "${BASE_OUTDIR}/bin"/*; do + # shellcheck disable=SC2044 + for b in $(find "${BASE_ROOT_DIR}" -executable -type f -name $(basename $b_name)); do + echo "Wrap $b ..." + mv "$b" "${b}_orig" + echo '#!/usr/bin/env bash' > "$b" + echo "valgrind --gen-suppressions=all --quiet --error-exitcode=1 --suppressions=${BASE_ROOT_DIR}/contrib/valgrind.supp \"${b}_orig\" \"\$@\"" >> "$b" + chmod +x "$b" + done +done diff --git a/configure.ac b/configure.ac index e7d14202a7..ad76317820 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,4 @@ -dnl require autoconf 2.60 (AS_ECHO/AS_ECHO_N) -AC_PREREQ([2.60]) +AC_PREREQ([2.69]) define(_CLIENT_VERSION_MAJOR, 0) define(_CLIENT_VERSION_MINOR, 19) define(_CLIENT_VERSION_REVISION, 99) @@ -525,12 +524,6 @@ case $host in if test "x$CXXFLAGS_overridden" = "xno"; then CXXFLAGS="$CXXFLAGS -w" fi - case $host in - i?86-*) WINDOWS_BITS=32 ;; - x86_64-*) WINDOWS_BITS=64 ;; - *) AC_MSG_ERROR("Could not determine win32/win64 for installer") ;; - esac - AC_SUBST(WINDOWS_BITS) dnl libtool insists upon adding -nostdlib and a list of objects/libs to link against. dnl That breaks our ability to build dll's with static libgcc/libstdc++/libssp. Override @@ -784,6 +777,7 @@ dnl this flag screws up non-darwin gcc even when the check fails. special-case i if test x$TARGET_OS = xdarwin; then AX_CHECK_LINK_FLAG([[-Wl,-dead_strip]], [LDFLAGS="$LDFLAGS -Wl,-dead_strip"]) AX_CHECK_LINK_FLAG([[-Wl,-dead_strip_dylibs]], [LDFLAGS="$LDFLAGS -Wl,-dead_strip_dylibs"]) + AX_CHECK_LINK_FLAG([[-Wl,-bind_at_load]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,-bind_at_load"]) fi AC_CHECK_HEADERS([endian.h sys/endian.h byteswap.h stdio.h stdlib.h unistd.h strings.h sys/types.h sys/stat.h sys/select.h sys/prctl.h sys/sysctl.h vm/vm_param.h sys/vmmeter.h sys/resources.h]) diff --git a/contrib/valgrind.supp b/contrib/valgrind.supp index 3c485c1df6..f232bb62c2 100644 --- a/contrib/valgrind.supp +++ b/contrib/valgrind.supp @@ -30,8 +30,6 @@ Memcheck:Cond obj:*/libdb_cxx-*.so fun:__log_put - obj:*/libdb_cxx-*.so - fun:__log_put_record } { Suppress libdb warning @@ -39,9 +37,52 @@ pwrite64(buf) fun:pwrite fun:__os_io +} +{ + Suppress libdb warning + Memcheck:Cond + fun:__log_putr.isra.1 +} +{ + Suppress libdb warning + Memcheck:Param + pwrite64(buf) + fun:pwrite + fun:__os_io + obj:*/libdb_cxx-*.so +} +{ + Suppress uninitialized bytes warning in compat code + Memcheck:Param + ioctl(TCSET{S,SW,SF}) + fun:tcsetattr +} +{ + Suppress libdb warning + Memcheck:Leak + fun:malloc + ... obj:*/libdb_cxx-*.so } { + Suppress leaks on init + Memcheck:Leak + ... + fun:_Z11AppInitMainR11NodeContext +} +{ + Suppress leaks on shutdown + Memcheck:Leak + ... + fun:_Z8ShutdownR11NodeContext +} +{ + Ignore GUI warning + Memcheck:Leak + ... + obj:/usr/lib64/libgdk-3.so.0.2404.7 +} +{ Suppress leveldb warning (leveldb::InitModule()) - https://github.com/google/leveldb/issues/113 Memcheck:Leak match-leak-kinds: reachable @@ -57,16 +98,48 @@ fun:_ZN7leveldbL14InitDefaultEnvEv } { + Suppress leveldb leak + Memcheck:Leak + match-leak-kinds: reachable + fun:_Znwm + ... + fun:_ZN7leveldb6DBImpl14BackgroundCallEv +} +{ + Suppress leveldb leak + Memcheck:Leak + fun:_Znwm + ... + fun:GetCoin +} +{ Suppress wcsnrtombs glibc SSE4 warning (could be related: https://stroika.atlassian.net/browse/STK-626) Memcheck:Addr16 fun:__wcsnlen_sse4_1 fun:wcsnrtombs } { + Suppress wcsnrtombs warning (remove after removing boost::fs) + Memcheck:Cond + ... + fun:_ZN5boost10filesystem6detail11unique_pathERKNS0_4pathEPNS_6system10error_codeE + fun:unique_path +} +{ + Suppress boost warning + Memcheck:Leak + fun:_Znwm + ... + fun:_ZN5boost9unit_test9framework5state17execute_test_treeEmjPKNS2_23random_generator_helperE + fun:_ZN5boost9unit_test9framework3runEmb + fun:_ZN5boost9unit_test14unit_test_mainEPFbvEiPPc + fun:main +} +{ Suppress boost::filesystem warning (fixed in boost 1.70: https://github.com/boostorg/filesystem/commit/bbe9d1771e5d679b3f10c42a58fc81f7e8c024a9) Memcheck:Cond fun:_ZN5boost10filesystem6detail28directory_iterator_incrementERNS0_18directory_iteratorEPNS_6system10error_codeE - fun:_ZN5boost10filesystem6detail28directory_iterator_constructERNS0_18directory_iteratorERKNS0_4pathEPNS_6system10error_codeE + ... obj:*/libboost_filesystem.so.* } { @@ -74,6 +147,7 @@ Memcheck:Leak match-leak-kinds: reachable fun:_Znwm + ... fun:_ZN5boost10filesystem8absoluteERKNS0_4pathES3_ } { diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 2087fec14d..efa76965d5 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -39,12 +39,14 @@ $(package)_config_opts += -no-iconv $(package)_config_opts += -no-kms $(package)_config_opts += -no-linuxfb $(package)_config_opts += -no-libjpeg +$(package)_config_opts += -no-libproxy $(package)_config_opts += -no-libudev $(package)_config_opts += -no-mtdev $(package)_config_opts += -no-openssl $(package)_config_opts += -no-openvg $(package)_config_opts += -no-reduce-relocations $(package)_config_opts += -no-qml-debug +$(package)_config_opts += -no-sctp $(package)_config_opts += -no-securetransport $(package)_config_opts += -no-sql-db2 $(package)_config_opts += -no-sql-ibase @@ -55,12 +57,13 @@ $(package)_config_opts += -no-sql-odbc $(package)_config_opts += -no-sql-psql $(package)_config_opts += -no-sql-sqlite $(package)_config_opts += -no-sql-sqlite2 +$(package)_config_opts += -no-system-proxies $(package)_config_opts += -no-use-gold-linker $(package)_config_opts += -no-xinput2 $(package)_config_opts += -nomake examples $(package)_config_opts += -nomake tests $(package)_config_opts += -opensource -$(package)_config_opts += -optimized-qmake +$(package)_config_opts += -optimized-tools $(package)_config_opts += -pch $(package)_config_opts += -pkg-config $(package)_config_opts += -prefix $(host_prefix) @@ -79,9 +82,12 @@ $(package)_config_opts += -no-feature-dial $(package)_config_opts += -no-feature-filesystemwatcher $(package)_config_opts += -no-feature-fontcombobox $(package)_config_opts += -no-feature-ftp +$(package)_config_opts += -no-feature-http $(package)_config_opts += -no-feature-image_heuristic_mask $(package)_config_opts += -no-feature-keysequenceedit $(package)_config_opts += -no-feature-lcdnumber +$(package)_config_opts += -no-feature-networkdiskcache +$(package)_config_opts += -no-feature-networkproxy $(package)_config_opts += -no-feature-pdf $(package)_config_opts += -no-feature-printdialog $(package)_config_opts += -no-feature-printer @@ -89,6 +95,7 @@ $(package)_config_opts += -no-feature-printpreviewdialog $(package)_config_opts += -no-feature-printpreviewwidget $(package)_config_opts += -no-feature-regularexpression $(package)_config_opts += -no-feature-sessionmanager +$(package)_config_opts += -no-feature-socks5 $(package)_config_opts += -no-feature-sql $(package)_config_opts += -no-feature-statemachine $(package)_config_opts += -no-feature-syntaxhighlighter diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 31d9f4e6d4..b50f552e92 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -820,7 +820,7 @@ Current subtrees include: - **Note**: Follow the instructions in [Upgrading LevelDB](#upgrading-leveldb) when merging upstream changes to the LevelDB subtree. -- src/libsecp256k1 +- src/secp256k1 - Upstream at https://github.com/bitcoin-core/secp256k1/ ; actively maintained by Core contributors. - src/crypto/ctaes diff --git a/doc/release-notes.md b/doc/release-notes.md index cdf83178c6..eec1ef9c46 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -78,6 +78,9 @@ New RPCs New settings ------------ +- RPC Whitelist system. It can give certain RPC users permissions to only some RPC calls. +It can be set with two command line arguments (`rpcwhitelist` and `rpcwhitelistdefault`). (#12763) + Updated settings ---------------- diff --git a/share/setup.nsi.in b/share/setup.nsi.in index 649483c732..be482ae741 100644 --- a/share/setup.nsi.in +++ b/share/setup.nsi.in @@ -1,4 +1,4 @@ -Name "@PACKAGE_NAME@ (@WINDOWS_BITS@-bit)" +Name "@PACKAGE_NAME@ (64-bit)" RequestExecutionLevel highest SetCompressor /SOLID lzma @@ -28,9 +28,7 @@ SetCompressor /SOLID lzma # Included files !include Sections.nsh !include MUI2.nsh -!if "@WINDOWS_BITS@" == "64" !include x64.nsh -!endif # Variables Var StartMenuGroup @@ -48,12 +46,8 @@ Var StartMenuGroup !insertmacro MUI_LANGUAGE English # Installer attributes -OutFile @abs_top_srcdir@/@PACKAGE_TARNAME@-@PACKAGE_VERSION@-win@WINDOWS_BITS@-setup-unsigned.exe -!if "@WINDOWS_BITS@" == "64" +OutFile @abs_top_srcdir@/@PACKAGE_TARNAME@-@PACKAGE_VERSION@-win64-setup-unsigned.exe InstallDir $PROGRAMFILES64\Bitcoin -!else -InstallDir $PROGRAMFILES\Bitcoin -!endif CRCCheck on XPStyle on BrandingText " " @@ -94,7 +88,7 @@ Section -post SEC0001 !insertmacro MUI_STARTMENU_WRITE_BEGIN Application CreateDirectory $SMPROGRAMS\$StartMenuGroup CreateShortcut "$SMPROGRAMS\$StartMenuGroup\$(^Name).lnk" $INSTDIR\@BITCOIN_GUI_NAME@@EXEEXT@ - CreateShortcut "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (testnet, @WINDOWS_BITS@-bit).lnk" "$INSTDIR\@BITCOIN_GUI_NAME@@EXEEXT@" "-testnet" "$INSTDIR\@BITCOIN_GUI_NAME@@EXEEXT@" 1 + CreateShortcut "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (testnet, 64-bit).lnk" "$INSTDIR\@BITCOIN_GUI_NAME@@EXEEXT@" "-testnet" "$INSTDIR\@BITCOIN_GUI_NAME@@EXEEXT@" 1 CreateShortcut "$SMPROGRAMS\$StartMenuGroup\Uninstall $(^Name).lnk" $INSTDIR\uninstall.exe !insertmacro MUI_STARTMENU_WRITE_END WriteRegStr HKCU "SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\$(^Name)" DisplayName "$(^Name)" @@ -138,7 +132,7 @@ Section -un.post UNSEC0001 DeleteRegKey HKCU "SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\$(^Name)" Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\Uninstall $(^Name).lnk" Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\$(^Name).lnk" - Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (testnet, @WINDOWS_BITS@-bit).lnk" + Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (testnet, 64-bit).lnk" Delete /REBOOTOK "$SMSTARTUP\Bitcoin.lnk" Delete /REBOOTOK $INSTDIR\uninstall.exe Delete /REBOOTOK $INSTDIR\debug.log @@ -160,7 +154,6 @@ SectionEnd # Installer functions Function .onInit InitPluginsDir -!if "@WINDOWS_BITS@" == "64" ${If} ${RunningX64} ; disable registry redirection (enable access to 64-bit portion of registry) SetRegView 64 @@ -168,7 +161,6 @@ Function .onInit MessageBox MB_OK|MB_ICONSTOP "Cannot install 64-bit version on a 32-bit system." Abort ${EndIf} -!endif FunctionEnd # Uninstaller functions diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 9e70db116b..1c97e22de8 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -39,9 +39,7 @@ bench_bench_bitcoin_SOURCES = \ bench/bech32.cpp \ bench/lockedpool.cpp \ bench/poly1305.cpp \ - bench/prevector.cpp \ - test/util.h \ - test/util.cpp + bench/prevector.cpp nodist_bench_bench_bitcoin_SOURCES = $(GENERATED_BENCH_FILES) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 0225edf29e..091ef50349 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -8,7 +8,9 @@ FUZZ_TARGETS = \ test/fuzz/address_deserialize \ test/fuzz/addrman_deserialize \ test/fuzz/banentry_deserialize \ + test/fuzz/base_encode_decode \ test/fuzz/bech32 \ + test/fuzz/block \ test/fuzz/block_deserialize \ test/fuzz/block_file_info_deserialize \ test/fuzz/block_filter_deserialize \ @@ -26,6 +28,7 @@ FUZZ_TARGETS = \ test/fuzz/eval_script \ test/fuzz/fee_rate_deserialize \ test/fuzz/flat_file_pos_deserialize \ + test/fuzz/hex \ test/fuzz/integer \ test/fuzz/inv_deserialize \ test/fuzz/key_origin_info_deserialize \ @@ -229,6 +232,12 @@ test_test_bitcoin_LDADD += $(LIBBITCOIN_ZMQ) $(ZMQ_LIBS) endif if ENABLE_FUZZ +test_fuzz_block_SOURCES = $(FUZZ_SUITE) test/fuzz/block.cpp +test_fuzz_block_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_block_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_block_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_block_LDADD = $(FUZZ_SUITE_LD_COMMON) + test_fuzz_block_deserialize_SOURCES = $(FUZZ_SUITE) test/fuzz/deserialize.cpp test_fuzz_block_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DBLOCK_DESERIALIZE=1 test_fuzz_block_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) @@ -271,6 +280,12 @@ test_fuzz_bech32_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_bech32_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_bech32_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_base_encode_decode_SOURCES = $(FUZZ_SUITE) test/fuzz/base_encode_decode.cpp +test_fuzz_base_encode_decode_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_base_encode_decode_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_base_encode_decode_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_base_encode_decode_LDADD = $(FUZZ_SUITE_LD_COMMON) + test_fuzz_txundo_deserialize_SOURCES = $(FUZZ_SUITE) test/fuzz/deserialize.cpp test_fuzz_txundo_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DTXUNDO_DESERIALIZE=1 test_fuzz_txundo_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) @@ -349,6 +364,12 @@ test_fuzz_address_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_address_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_address_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_hex_SOURCES = $(FUZZ_SUITE) test/fuzz/hex.cpp +test_fuzz_hex_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_hex_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_hex_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_hex_LDADD = $(FUZZ_SUITE_LD_COMMON) + test_fuzz_inv_deserialize_SOURCES = $(FUZZ_SUITE) test/fuzz/deserialize.cpp test_fuzz_inv_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DINV_DESERIALIZE=1 test_fuzz_inv_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) diff --git a/src/Makefile.test_util.include b/src/Makefile.test_util.include index cf55d141b0..505d630b7d 100644 --- a/src/Makefile.test_util.include +++ b/src/Makefile.test_util.include @@ -10,22 +10,25 @@ EXTRA_LIBRARIES += \ TEST_UTIL_H = \ test/util/blockfilter.h \ test/util/logging.h \ + test/util/mining.h \ test/util/setup_common.h \ test/util/str.h \ - test/util/transaction_utils.h + test/util/transaction_utils.h \ + test/util/wallet.h libtest_util_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(MINIUPNPC_CPPFLAGS) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS) libtest_util_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libtest_util_a_SOURCES = \ test/util/blockfilter.cpp \ test/util/logging.cpp \ + test/util/mining.cpp \ test/util/setup_common.cpp \ test/util/str.cpp \ test/util/transaction_utils.cpp \ + test/util/wallet.cpp \ $(TEST_UTIL_H) LIBTEST_UTIL += $(LIBBITCOIN_SERVER) LIBTEST_UTIL += $(LIBBITCOIN_COMMON) LIBTEST_UTIL += $(LIBBITCOIN_UTIL) LIBTEST_UTIL += $(LIBBITCOIN_CRYPTO_BASE) - diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 2f47398d99..184367e1e5 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -5,7 +5,8 @@ #include <bench/bench.h> #include <consensus/validation.h> #include <crypto/sha256.h> -#include <test/util.h> +#include <test/util/mining.h> +#include <test/util/wallet.h> #include <txmempool.h> #include <validation.h> diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index 0e660d6bcd..f39dcc0b71 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -6,7 +6,8 @@ #include <interfaces/chain.h> #include <node/context.h> #include <optional.h> -#include <test/util.h> +#include <test/util/mining.h> +#include <test/util/wallet.h> #include <validationinterface.h> #include <wallet/wallet.h> diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 26856a00d3..ac640aa35a 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -263,7 +263,7 @@ public: } return true; } - void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(coins); } + void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); } double guessVerificationProgress(const uint256& block_hash) override { LOCK(cs_main); diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 1877c92178..529fa793dd 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -68,7 +68,7 @@ public: std::string getNetwork() override { return Params().NetworkIDString(); } void initLogging() override { InitLogging(); } void initParameterInteraction() override { InitParameterInteraction(); } - std::string getWarnings(const std::string& type) override { return GetWarnings(type); } + std::string getWarnings() override { return GetWarnings(true); } uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); } bool baseInitialize() override { @@ -167,8 +167,8 @@ public: } int64_t getTotalBytesRecv() override { return m_context.connman ? m_context.connman->GetTotalBytesRecv() : 0; } int64_t getTotalBytesSent() override { return m_context.connman ? m_context.connman->GetTotalBytesSent() : 0; } - size_t getMempoolSize() override { return ::mempool.size(); } - size_t getMempoolDynamicUsage() override { return ::mempool.DynamicMemoryUsage(); } + size_t getMempoolSize() override { return m_context.mempool ? m_context.mempool->size() : 0; } + size_t getMempoolDynamicUsage() override { return m_context.mempool ? m_context.mempool->DynamicMemoryUsage() : 0; } bool getHeaderTip(int& height, int64_t& block_time) override { LOCK(::cs_main); diff --git a/src/interfaces/node.h b/src/interfaces/node.h index adf3de7b07..6bc4668beb 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -78,7 +78,7 @@ public: virtual void initParameterInteraction() = 0; //! Get warnings. - virtual std::string getWarnings(const std::string& type) = 0; + virtual std::string getWarnings() = 0; // Get log flags. virtual uint32_t getLogCategories() = 0; diff --git a/src/netaddress.h b/src/netaddress.h index fbb1553338..32f4476fac 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -54,7 +54,7 @@ class CNetAddr bool IsIPv4() const; // IPv4 mapped address (::FFFF:0:0/96, 0.0.0.0/0) bool IsIPv6() const; // IPv6 address (not mapped IPv4, not Tor) bool IsRFC1918() const; // IPv4 private networks (10.0.0.0/8, 192.168.0.0/16, 172.16.0.0/12) - bool IsRFC2544() const; // IPv4 inter-network communications (192.18.0.0/15) + bool IsRFC2544() const; // IPv4 inter-network communications (198.18.0.0/15) bool IsRFC6598() const; // IPv4 ISP-level NAT (100.64.0.0/10) bool IsRFC5737() const; // IPv4 documentation addresses (192.0.2.0/24, 198.51.100.0/24, 203.0.113.0/24) bool IsRFC3849() const; // IPv6 documentation address (2001:0DB8::/32) diff --git a/src/node/coin.cpp b/src/node/coin.cpp index ad8d1d3af4..f4f86cdbe9 100644 --- a/src/node/coin.cpp +++ b/src/node/coin.cpp @@ -4,14 +4,16 @@ #include <node/coin.h> +#include <node/context.h> #include <txmempool.h> #include <validation.h> -void FindCoins(std::map<COutPoint, Coin>& coins) +void FindCoins(const NodeContext& node, std::map<COutPoint, Coin>& coins) { - LOCK2(cs_main, ::mempool.cs); + assert(node.mempool); + LOCK2(cs_main, node.mempool->cs); CCoinsViewCache& chain_view = ::ChainstateActive().CoinsTip(); - CCoinsViewMemPool mempool_view(&chain_view, ::mempool); + CCoinsViewMemPool mempool_view(&chain_view, *node.mempool); for (auto& coin : coins) { if (!mempool_view.GetCoin(coin.first, coin.second)) { // Either the coin is not in the CCoinsViewCache or is spent. Clear it. diff --git a/src/node/coin.h b/src/node/coin.h index eb95b75cfb..908850e2a5 100644 --- a/src/node/coin.h +++ b/src/node/coin.h @@ -9,14 +9,16 @@ class COutPoint; class Coin; +struct NodeContext; /** * Look up unspent output information. Returns coins in the mempool and in the * current chain UTXO set. Iterates through all the keys in the map and * populates the values. * + * @param[in] node The node context to use for lookup * @param[in,out] coins map to fill */ -void FindCoins(std::map<COutPoint, Coin>& coins); +void FindCoins(const NodeContext& node, std::map<COutPoint, Coin>& coins); #endif // BITCOIN_NODE_COIN_H diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 3c0df2b26e..5e2e502015 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -20,6 +20,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t // node.connman is assigned both before chain clients and before RPC server is accepting calls, // and reset after chain clients and RPC sever are stopped. node.connman should never be null here. assert(node.connman); + assert(node.mempool); std::promise<void> promise; uint256 hashTx = tx->GetHash(); bool callback_set = false; @@ -35,10 +36,10 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t // So if the output does exist, then this transaction exists in the chain. if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_CHAIN; } - if (!mempool.exists(hashTx)) { + if (!node.mempool->exists(hashTx)) { // Transaction is not already in the mempool. Submit it. TxValidationState state; - if (!AcceptToMemoryPool(mempool, state, std::move(tx), + if (!AcceptToMemoryPool(*node.mempool, state, std::move(tx), nullptr /* plTxnReplaced */, false /* bypass_limits */, max_tx_fee)) { err_string = FormatStateMessage(state); if (state.IsInvalid()) { diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 676c15ea43..0021c3dbc7 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -135,7 +135,7 @@ BitcoinCore::BitcoinCore(interfaces::Node& node) : void BitcoinCore::handleRunawayException(const std::exception *e) { PrintExceptionContinue(e, "Runaway exception"); - Q_EMIT runawayException(QString::fromStdString(m_node.getWarnings("gui"))); + Q_EMIT runawayException(QString::fromStdString(m_node.getWarnings())); } void BitcoinCore::initialize() @@ -589,10 +589,10 @@ int GuiMain(int argc, char* argv[]) } } catch (const std::exception& e) { PrintExceptionContinue(&e, "Runaway exception"); - app.handleRunawayException(QString::fromStdString(node->getWarnings("gui"))); + app.handleRunawayException(QString::fromStdString(node->getWarnings())); } catch (...) { PrintExceptionContinue(nullptr, "Runaway exception"); - app.handleRunawayException(QString::fromStdString(node->getWarnings("gui"))); + app.handleRunawayException(QString::fromStdString(node->getWarnings())); } return rv; } diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index 5b216b2705..b84587a99b 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -134,7 +134,7 @@ enum BlockSource ClientModel::getBlockSource() const QString ClientModel::getStatusBarWarnings() const { - return QString::fromStdString(m_node.getWarnings("gui")); + return QString::fromStdString(m_node.getWarnings()); } OptionsModel *ClientModel::getOptionsModel() diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index dfd56511ea..f6d2816ff8 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -134,6 +134,7 @@ void TestGUI(interfaces::Node& node) test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); } node.context()->connman = std::move(test.m_node.connman); + node.context()->mempool = std::move(test.m_node.mempool); std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), WalletDatabase::CreateMock()); bool firstRun; wallet->LoadWallet(firstRun); diff --git a/src/rest.cpp b/src/rest.cpp index 228c122de3..55756ecfdf 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -8,6 +8,7 @@ #include <core_io.h> #include <httpserver.h> #include <index/txindex.h> +#include <node/context.h> #include <primitives/block.h> #include <primitives/transaction.h> #include <rpc/blockchain.h> @@ -16,6 +17,7 @@ #include <streams.h> #include <sync.h> #include <txmempool.h> +#include <util/check.h> #include <util/strencodings.h> #include <validation.h> #include <version.h> @@ -69,6 +71,24 @@ static bool RESTERR(HTTPRequest* req, enum HTTPStatusCode status, std::string me return false; } +/** + * Get the node context mempool. + * + * Set the HTTP error and return nullptr if node context + * mempool is not found. + * + * @param[in] req the HTTP request + * return pointer to the mempool or nullptr if no mempool found + */ +static CTxMemPool* GetMemPool(HTTPRequest* req) +{ + if (!g_rpc_node || !g_rpc_node->mempool) { + RESTERR(req, HTTP_NOT_FOUND, "Mempool disabled or instance not found"); + return nullptr; + } + return g_rpc_node->mempool; +} + static RetFormat ParseDataFormat(std::string& param, const std::string& strReq) { const std::string::size_type pos = strReq.rfind('.'); @@ -295,12 +315,14 @@ static bool rest_mempool_info(HTTPRequest* req, const std::string& strURIPart) { if (!CheckWarmup(req)) return false; + const CTxMemPool* mempool = GetMemPool(req); + if (!mempool) return false; std::string param; const RetFormat rf = ParseDataFormat(param, strURIPart); switch (rf) { case RetFormat::JSON: { - UniValue mempoolInfoObject = MempoolInfoToJSON(::mempool); + UniValue mempoolInfoObject = MempoolInfoToJSON(*mempool); std::string strJSON = mempoolInfoObject.write() + "\n"; req->WriteHeader("Content-Type", "application/json"); @@ -315,14 +337,15 @@ static bool rest_mempool_info(HTTPRequest* req, const std::string& strURIPart) static bool rest_mempool_contents(HTTPRequest* req, const std::string& strURIPart) { - if (!CheckWarmup(req)) - return false; + if (!CheckWarmup(req)) return false; + const CTxMemPool* mempool = GetMemPool(req); + if (!mempool) return false; std::string param; const RetFormat rf = ParseDataFormat(param, strURIPart); switch (rf) { case RetFormat::JSON: { - UniValue mempoolObject = MempoolToJSON(::mempool, true); + UniValue mempoolObject = MempoolToJSON(*mempool, true); std::string strJSON = mempoolObject.write() + "\n"; req->WriteHeader("Content-Type", "application/json"); @@ -500,11 +523,13 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart) }; if (fCheckMemPool) { + const CTxMemPool* mempool = GetMemPool(req); + if (!mempool) return false; // use db+mempool as cache backend in case user likes to query mempool - LOCK2(cs_main, mempool.cs); + LOCK2(cs_main, mempool->cs); CCoinsViewCache& viewChain = ::ChainstateActive().CoinsTip(); - CCoinsViewMemPool viewMempool(&viewChain, mempool); - process_utxos(viewMempool, mempool); + CCoinsViewMemPool viewMempool(&viewChain, *mempool); + process_utxos(viewMempool, *mempool); } else { LOCK(cs_main); // no need to lock mempool! process_utxos(::ChainstateActive().CoinsTip(), CTxMemPool()); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 44f5326b3c..eb5148eebd 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -528,7 +528,7 @@ static UniValue getrawmempool(const JSONRPCRequest& request) if (!request.params[0].isNull()) fVerbose = request.params[0].get_bool(); - return MempoolToJSON(::mempool, fVerbose); + return MempoolToJSON(EnsureMemPool(), fVerbose); } static UniValue getmempoolancestors(const JSONRPCRequest& request) @@ -566,6 +566,7 @@ static UniValue getmempoolancestors(const JSONRPCRequest& request) uint256 hash = ParseHashV(request.params[0], "parameter 1"); + const CTxMemPool& mempool = EnsureMemPool(); LOCK(mempool.cs); CTxMemPool::txiter it = mempool.mapTx.find(hash); @@ -591,7 +592,7 @@ static UniValue getmempoolancestors(const JSONRPCRequest& request) const CTxMemPoolEntry &e = *ancestorIt; const uint256& _hash = e.GetTx().GetHash(); UniValue info(UniValue::VOBJ); - entryToJSON(::mempool, info, e); + entryToJSON(mempool, info, e); o.pushKV(_hash.ToString(), info); } return o; @@ -633,6 +634,7 @@ static UniValue getmempooldescendants(const JSONRPCRequest& request) uint256 hash = ParseHashV(request.params[0], "parameter 1"); + const CTxMemPool& mempool = EnsureMemPool(); LOCK(mempool.cs); CTxMemPool::txiter it = mempool.mapTx.find(hash); @@ -658,7 +660,7 @@ static UniValue getmempooldescendants(const JSONRPCRequest& request) const CTxMemPoolEntry &e = *descendantIt; const uint256& _hash = e.GetTx().GetHash(); UniValue info(UniValue::VOBJ); - entryToJSON(::mempool, info, e); + entryToJSON(mempool, info, e); o.pushKV(_hash.ToString(), info); } return o; @@ -685,6 +687,7 @@ static UniValue getmempoolentry(const JSONRPCRequest& request) uint256 hash = ParseHashV(request.params[0], "parameter 1"); + const CTxMemPool& mempool = EnsureMemPool(); LOCK(mempool.cs); CTxMemPool::txiter it = mempool.mapTx.find(hash); @@ -694,7 +697,7 @@ static UniValue getmempoolentry(const JSONRPCRequest& request) const CTxMemPoolEntry &e = *it; UniValue info(UniValue::VOBJ); - entryToJSON(::mempool, info, e); + entryToJSON(mempool, info, e); return info; } @@ -1070,6 +1073,7 @@ UniValue gettxout(const JSONRPCRequest& request) CCoinsViewCache* coins_view = &::ChainstateActive().CoinsTip(); if (fMempool) { + const CTxMemPool& mempool = EnsureMemPool(); LOCK(mempool.cs); CCoinsViewMemPool view(coins_view, mempool); if (!view.GetCoin(out, coin) || mempool.isSpent(out)) { @@ -1286,7 +1290,7 @@ UniValue getblockchaininfo(const JSONRPCRequest& request) BIP9SoftForkDescPushBack(softforks, "testdummy", consensusParams, Consensus::DEPLOYMENT_TESTDUMMY); obj.pushKV("softforks", softforks); - obj.pushKV("warnings", GetWarnings("statusbar")); + obj.pushKV("warnings", GetWarnings(false)); return obj; } @@ -1448,7 +1452,7 @@ static UniValue getmempoolinfo(const JSONRPCRequest& request) }, }.Check(request); - return MempoolInfoToJSON(::mempool); + return MempoolInfoToJSON(EnsureMemPool()); } static UniValue preciousblock(const JSONRPCRequest& request) @@ -1964,11 +1968,13 @@ static UniValue savemempool(const JSONRPCRequest& request) }, }.Check(request); - if (!::mempool.IsLoaded()) { + const CTxMemPool& mempool = EnsureMemPool(); + + if (!mempool.IsLoaded()) { throw JSONRPCError(RPC_MISC_ERROR, "The mempool was not loaded yet"); } - if (!DumpMempool(::mempool)) { + if (!DumpMempool(mempool)) { throw JSONRPCError(RPC_MISC_ERROR, "Unable to dump mempool to disk"); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index fc16a31423..9f7f7837d3 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -244,6 +244,7 @@ static UniValue getmininginfo(const JSONRPCRequest& request) }.Check(request); LOCK(cs_main); + const CTxMemPool& mempool = EnsureMemPool(); UniValue obj(UniValue::VOBJ); obj.pushKV("blocks", (int)::ChainActive().Height()); @@ -253,7 +254,7 @@ static UniValue getmininginfo(const JSONRPCRequest& request) obj.pushKV("networkhashps", getnetworkhashps(request)); obj.pushKV("pooledtx", (uint64_t)mempool.size()); obj.pushKV("chain", Params().NetworkIDString()); - obj.pushKV("warnings", GetWarnings("statusbar")); + obj.pushKV("warnings", GetWarnings(false)); return obj; } @@ -290,7 +291,7 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "Priority is no longer supported, dummy argument to prioritisetransaction must be 0."); } - mempool.PrioritiseTransaction(hash, nAmount); + EnsureMemPool().PrioritiseTransaction(hash, nAmount); return true; } @@ -476,6 +477,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, PACKAGE_NAME " is in initial sync and waiting for blocks..."); static unsigned int nTransactionsUpdatedLast; + const CTxMemPool& mempool = EnsureMemPool(); if (!lpval.isNull()) { @@ -510,7 +512,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) if (g_best_block_cv.wait_until(lock, checktxtime) == std::cv_status::timeout) { // Timeout: Check transactions for update - // without holding ::mempool.cs to avoid deadlocks + // without holding the mempool lock to avoid deadlocks if (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLastLP) break; checktxtime += std::chrono::seconds(10); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 6ee91f139d..5e53fa5f5d 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -522,7 +522,7 @@ static UniValue getnetworkinfo(const JSONRPCRequest& request) } } obj.pushKV("localaddresses", localAddresses); - obj.pushKV("warnings", GetWarnings("statusbar")); + obj.pushKV("warnings", GetWarnings(false)); return obj; } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index b816a54d2f..5be7acce1c 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -636,6 +636,7 @@ static UniValue combinerawtransaction(const JSONRPCRequest& request) CCoinsView viewDummy; CCoinsViewCache view(&viewDummy); { + const CTxMemPool& mempool = EnsureMemPool(); LOCK(cs_main); LOCK(mempool.cs); CCoinsViewCache &viewChain = ::ChainstateActive().CoinsTip(); @@ -758,7 +759,7 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request) for (const CTxIn& txin : mtx.vin) { coins[txin.prevout]; // Create empty map entry keyed by prevout. } - FindCoins(coins); + FindCoins(*g_rpc_node, coins); // Parse the prevtxs array ParsePrevouts(request.params[2], &keystore, coins); @@ -890,6 +891,7 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) max_raw_tx_fee_rate = CFeeRate(AmountFromValue(request.params[1])); } + CTxMemPool& mempool = EnsureMemPool(); int64_t virtual_size = GetVirtualTransactionSize(*tx); CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); @@ -1508,6 +1510,7 @@ UniValue utxoupdatepsbt(const JSONRPCRequest& request) CCoinsView viewDummy; CCoinsViewCache view(&viewDummy); { + const CTxMemPool& mempool = EnsureMemPool(); LOCK2(cs_main, mempool.cs); CCoinsViewCache &viewChain = ::ChainstateActive().CoinsTip(); CCoinsViewMemPool viewMempool(&viewChain, mempool); diff --git a/src/test/base32_tests.cpp b/src/test/base32_tests.cpp index bd6ece935b..690368b177 100644 --- a/src/test/base32_tests.cpp +++ b/src/test/base32_tests.cpp @@ -20,6 +20,17 @@ BOOST_AUTO_TEST_CASE(base32_testvectors) std::string strDec = DecodeBase32(vstrOut[i]); BOOST_CHECK_EQUAL(strDec, vstrIn[i]); } + + // Decoding strings with embedded NUL characters should fail + bool failure; + (void)DecodeBase32(std::string("invalid", 7), &failure); + BOOST_CHECK_EQUAL(failure, true); + (void)DecodeBase32(std::string("AWSX3VPP", 8), &failure); + BOOST_CHECK_EQUAL(failure, false); + (void)DecodeBase32(std::string("AWSX3VPP\0invalid", 16), &failure); + BOOST_CHECK_EQUAL(failure, true); + (void)DecodeBase32(std::string("AWSX3VPPinvalid", 15), &failure); + BOOST_CHECK_EQUAL(failure, true); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/base64_tests.cpp b/src/test/base64_tests.cpp index a5fed55504..94df4d1955 100644 --- a/src/test/base64_tests.cpp +++ b/src/test/base64_tests.cpp @@ -20,6 +20,17 @@ BOOST_AUTO_TEST_CASE(base64_testvectors) std::string strDec = DecodeBase64(strEnc); BOOST_CHECK_EQUAL(strDec, vstrIn[i]); } + + // Decoding strings with embedded NUL characters should fail + bool failure; + (void)DecodeBase64(std::string("invalid", 7), &failure); + BOOST_CHECK_EQUAL(failure, true); + (void)DecodeBase64(std::string("nQB/pZw=", 8), &failure); + BOOST_CHECK_EQUAL(failure, false); + (void)DecodeBase64(std::string("nQB/pZw=\0invalid", 16), &failure); + BOOST_CHECK_EQUAL(failure, true); + (void)DecodeBase64(std::string("nQB/pZw=invalid", 15), &failure); + BOOST_CHECK_EQUAL(failure, true); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/base_encode_decode.cpp b/src/test/fuzz/base_encode_decode.cpp new file mode 100644 index 0000000000..cb0fbdf76f --- /dev/null +++ b/src/test/fuzz/base_encode_decode.cpp @@ -0,0 +1,47 @@ +// 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. + +#include <test/fuzz/fuzz.h> + +#include <base58.h> +#include <util/string.h> +#include <util/strencodings.h> + +#include <cassert> +#include <cstdint> +#include <string> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + const std::string random_encoded_string(buffer.begin(), buffer.end()); + + std::vector<unsigned char> decoded; + if (DecodeBase58(random_encoded_string, decoded, 100)) { + const std::string encoded_string = EncodeBase58(decoded); + assert(encoded_string == TrimString(encoded_string)); + assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string))); + } + + if (DecodeBase58Check(random_encoded_string, decoded, 100)) { + const std::string encoded_string = EncodeBase58Check(decoded); + assert(encoded_string == TrimString(encoded_string)); + assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string))); + } + + bool pf_invalid; + std::string decoded_string = DecodeBase32(random_encoded_string, &pf_invalid); + if (!pf_invalid) { + const std::string encoded_string = EncodeBase32(decoded_string); + assert(encoded_string == TrimString(encoded_string)); + assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string))); + } + + decoded_string = DecodeBase64(random_encoded_string, &pf_invalid); + if (!pf_invalid) { + const std::string encoded_string = EncodeBase64(decoded_string); + assert(encoded_string == TrimString(encoded_string)); + assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string))); + } +} diff --git a/src/test/fuzz/block.cpp b/src/test/fuzz/block.cpp new file mode 100644 index 0000000000..431248de4a --- /dev/null +++ b/src/test/fuzz/block.cpp @@ -0,0 +1,63 @@ +// 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. + +#include <chainparams.h> +#include <consensus/merkle.h> +#include <consensus/validation.h> +#include <core_io.h> +#include <core_memusage.h> +#include <pubkey.h> +#include <primitives/block.h> +#include <streams.h> +#include <test/fuzz/fuzz.h> +#include <validation.h> +#include <version.h> + +#include <cassert> +#include <string> + +void initialize() +{ + const static auto verify_handle = MakeUnique<ECCVerifyHandle>(); + SelectParams(CBaseChainParams::REGTEST); +} + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + CDataStream ds(buffer, SER_NETWORK, INIT_PROTO_VERSION); + CBlock block; + try { + int nVersion; + ds >> nVersion; + ds.SetVersion(nVersion); + ds >> block; + } catch (const std::ios_base::failure&) { + return; + } + const Consensus::Params& consensus_params = Params().GetConsensus(); + BlockValidationState validation_state_pow_and_merkle; + const bool valid_incl_pow_and_merkle = CheckBlock(block, validation_state_pow_and_merkle, consensus_params, /* fCheckPOW= */ true, /* fCheckMerkleRoot= */ true); + BlockValidationState validation_state_pow; + const bool valid_incl_pow = CheckBlock(block, validation_state_pow, consensus_params, /* fCheckPOW= */ true, /* fCheckMerkleRoot= */ false); + BlockValidationState validation_state_merkle; + const bool valid_incl_merkle = CheckBlock(block, validation_state_merkle, consensus_params, /* fCheckPOW= */ false, /* fCheckMerkleRoot= */ true); + BlockValidationState validation_state_none; + const bool valid_incl_none = CheckBlock(block, validation_state_none, consensus_params, /* fCheckPOW= */ false, /* fCheckMerkleRoot= */ false); + if (valid_incl_pow_and_merkle) { + assert(valid_incl_pow && valid_incl_merkle && valid_incl_none); + } else if (valid_incl_merkle || valid_incl_pow) { + assert(valid_incl_none); + } + (void)block.GetHash(); + (void)block.ToString(); + (void)BlockMerkleRoot(block); + if (!block.vtx.empty()) { + // TODO: Avoid array index out of bounds error in BlockWitnessMerkleRoot + // when block.vtx.empty(). + (void)BlockWitnessMerkleRoot(block); + } + (void)GetBlockWeight(block); + (void)GetWitnessCommitmentIndex(block); + (void)RecursiveDynamicUsage(block); +} diff --git a/src/test/fuzz/hex.cpp b/src/test/fuzz/hex.cpp new file mode 100644 index 0000000000..54693180be --- /dev/null +++ b/src/test/fuzz/hex.cpp @@ -0,0 +1,22 @@ +// 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. + +#include <test/fuzz/fuzz.h> + +#include <util/strencodings.h> + +#include <cassert> +#include <cstdint> +#include <string> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + const std::string random_hex_string(buffer.begin(), buffer.end()); + const std::vector<unsigned char> data = ParseHex(random_hex_string); + const std::string hex_data = HexStr(data); + if (IsHex(random_hex_string)) { + assert(ToLower(random_hex_string) == hex_data); + } +} diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp index 5abd1087ec..4c64d8c833 100644 --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -43,7 +43,7 @@ static void SetupArgs(const std::vector<std::pair<std::string, unsigned int>>& a BOOST_AUTO_TEST_CASE(boolarg) { - const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_BOOL); + const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY); SetupArgs({foo}); ResetArgs("-foo"); BOOST_CHECK(gArgs.GetBoolArg("-foo", false)); @@ -97,8 +97,8 @@ BOOST_AUTO_TEST_CASE(boolarg) BOOST_AUTO_TEST_CASE(stringarg) { - const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_STRING); - const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_STRING); + const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY); + const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY); SetupArgs({foo, bar}); ResetArgs(""); BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), ""); @@ -124,8 +124,8 @@ BOOST_AUTO_TEST_CASE(stringarg) BOOST_AUTO_TEST_CASE(intarg) { - const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_INT); - const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_INT); + const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY); + const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY); SetupArgs({foo, bar}); ResetArgs(""); BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 11), 11); @@ -159,8 +159,8 @@ BOOST_AUTO_TEST_CASE(doubledash) BOOST_AUTO_TEST_CASE(boolargno) { - const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_BOOL); - const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_BOOL); + const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY); + const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY); SetupArgs({foo, bar}); ResetArgs("-nofoo"); BOOST_CHECK(!gArgs.GetBoolArg("-foo", true)); diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 78c11ff202..481dedc356 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -54,6 +54,8 @@ BOOST_AUTO_TEST_CASE(netbase_properties) BOOST_CHECK(ResolveIP("10.0.0.1").IsRFC1918()); BOOST_CHECK(ResolveIP("192.168.1.1").IsRFC1918()); BOOST_CHECK(ResolveIP("172.31.255.255").IsRFC1918()); + BOOST_CHECK(ResolveIP("198.18.0.0").IsRFC2544()); + BOOST_CHECK(ResolveIP("198.19.255.255").IsRFC2544()); BOOST_CHECK(ResolveIP("2001:0DB8::").IsRFC3849()); BOOST_CHECK(ResolveIP("169.254.1.1").IsRFC3927()); BOOST_CHECK(ResolveIP("2002::1").IsRFC3964()); diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 52dd22de7e..84a3980b19 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -112,14 +112,10 @@ BOOST_AUTO_TEST_CASE(rpc_rawsign) std::string notsigned = r.get_str(); std::string privkey1 = "\"KzsXybp9jX64P5ekX1KUxRQ79Jht9uzW7LorgwE65i5rWACL6LQe\""; std::string privkey2 = "\"Kyhdf5LuKTRx4ge69ybABsiUAWjVRK4XGxAKk2FQLp2HjGMy87Z4\""; - NodeContext node; - node.chain = interfaces::MakeChain(node); - g_rpc_node = &node; r = CallRPC(std::string("signrawtransactionwithkey ")+notsigned+" [] "+prevout); BOOST_CHECK(find_value(r.get_obj(), "complete").get_bool() == false); r = CallRPC(std::string("signrawtransactionwithkey ")+notsigned+" ["+privkey1+","+privkey2+"] "+prevout); BOOST_CHECK(find_value(r.get_obj(), "complete").get_bool() == true); - g_rpc_node = nullptr; } BOOST_AUTO_TEST_CASE(rpc_createraw_op_return) diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index b0ee76ea6b..45644834a5 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -4,8 +4,9 @@ #include <util/settings.h> -#include <test/util.h> #include <test/util/setup_common.h> +#include <test/util/str.h> + #include <boost/test/unit_test.hpp> #include <univalue.h> @@ -45,6 +46,19 @@ BOOST_AUTO_TEST_CASE(Simple) CheckValues(settings2, R"("val2")", R"(["val2","val3"])"); } +// Confirm that a high priority setting overrides a lower priority setting even +// if the high priority setting is null. This behavior is useful for a high +// priority setting source to be able to effectively reset any setting back to +// its default value. +BOOST_AUTO_TEST_CASE(NullOverride) +{ + util::Settings settings; + settings.command_line_options["name"].push_back("value"); + BOOST_CHECK_EQUAL(R"("value")", GetSetting(settings, "section", "name", false, false).write().c_str()); + settings.forced_settings["name"] = {}; + BOOST_CHECK_EQUAL(R"(null)", GetSetting(settings, "section", "name", false, false).write().c_str()); +} + // Test different ways settings can be merged, and verify results. This test can // be used to confirm that updates to settings code don't change behavior // unintentionally. diff --git a/src/test/timedata_tests.cpp b/src/test/timedata_tests.cpp index 4721151197..19bd0d142f 100644 --- a/src/test/timedata_tests.cpp +++ b/src/test/timedata_tests.cpp @@ -65,7 +65,7 @@ BOOST_AUTO_TEST_CASE(addtimedata) MultiAddTimeData(1, DEFAULT_MAX_TIME_ADJUSTMENT + 1); //filter size 5 } - BOOST_CHECK(GetWarnings("gui").find("clock is wrong") != std::string::npos); + BOOST_CHECK(GetWarnings(true).find("clock is wrong") != std::string::npos); // nTimeOffset is not changed if the median of offsets exceeds DEFAULT_MAX_TIME_ADJUSTMENT BOOST_CHECK_EQUAL(GetTimeOffset(), 0); diff --git a/src/test/util.h b/src/test/util.h deleted file mode 100644 index f90cb0d623..0000000000 --- a/src/test/util.h +++ /dev/null @@ -1,70 +0,0 @@ -// 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. - -#ifndef BITCOIN_TEST_UTIL_H -#define BITCOIN_TEST_UTIL_H - -#include <memory> -#include <string> - -class CBlock; -class CScript; -class CTxIn; -class CWallet; - -// Constants // - -extern const std::string ADDRESS_BCRT1_UNSPENDABLE; - -// Lower-level utils // - -/** Returns the generated coin */ -CTxIn MineBlock(const CScript& coinbase_scriptPubKey); -/** Prepare a block to be mined */ -std::shared_ptr<CBlock> PrepareBlock(const CScript& coinbase_scriptPubKey); - - -// RPC-like // - -/** Import the address to the wallet */ -void importaddress(CWallet& wallet, const std::string& address); -/** Returns a new address from the wallet */ -std::string getnewaddress(CWallet& w); -/** Returns the generated coin */ -CTxIn generatetoaddress(const std::string& address); - -/** - * Increment a string. Useful to enumerate all fixed length strings with - * characters in [min_char, max_char]. - */ -template <typename CharType, size_t StringLength> -bool NextString(CharType (&string)[StringLength], CharType min_char, CharType max_char) -{ - for (CharType& elem : string) { - bool has_next = elem != max_char; - elem = elem < min_char || elem >= max_char ? min_char : CharType(elem + 1); - if (has_next) return true; - } - return false; -} - -/** - * Iterate over string values and call function for each string without - * successive duplicate characters. - */ -template <typename CharType, size_t StringLength, typename Fn> -void ForEachNoDup(CharType (&string)[StringLength], CharType min_char, CharType max_char, Fn&& fn) { - for (bool has_next = true; has_next; has_next = NextString(string, min_char, max_char)) { - int prev = -1; - bool skip_string = false; - for (CharType c : string) { - if (c == prev) skip_string = true; - if (skip_string || c < min_char || c > max_char) break; - prev = c; - } - if (!skip_string) fn(); - } -} - -#endif // BITCOIN_TEST_UTIL_H diff --git a/src/test/util.cpp b/src/test/util/mining.cpp index ed031270f2..30f0f5d7e6 100644 --- a/src/test/util.cpp +++ b/src/test/util/mining.cpp @@ -2,48 +2,15 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <test/util.h> +#include <test/util/mining.h> #include <chainparams.h> #include <consensus/merkle.h> #include <key_io.h> #include <miner.h> -#include <outputtype.h> #include <pow.h> #include <script/standard.h> #include <validation.h> -#include <validationinterface.h> -#ifdef ENABLE_WALLET -#include <wallet/wallet.h> -#endif - -const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj"; - -#ifdef ENABLE_WALLET -std::string getnewaddress(CWallet& w) -{ - constexpr auto output_type = OutputType::BECH32; - CTxDestination dest; - std::string error; - if (!w.GetNewDestination(output_type, "", dest, error)) assert(false); - - return EncodeDestination(dest); -} - -void importaddress(CWallet& wallet, const std::string& address) -{ - auto spk_man = wallet.GetLegacyScriptPubKeyMan(); - LOCK(wallet.cs_wallet); - AssertLockHeld(spk_man->cs_wallet); - const auto dest = DecodeDestination(address); - assert(IsValidDestination(dest)); - const auto script = GetScriptForDestination(dest); - wallet.MarkDirty(); - assert(!spk_man->HaveWatchOnly(script)); - if (!spk_man->AddWatchOnly(script, 0 /* nCreateTime */)) assert(false); - wallet.SetAddressBook(dest, /* label */ "", "receive"); -} -#endif // ENABLE_WALLET CTxIn generatetoaddress(const std::string& address) { diff --git a/src/test/util/mining.h b/src/test/util/mining.h new file mode 100644 index 0000000000..afe4de684f --- /dev/null +++ b/src/test/util/mining.h @@ -0,0 +1,24 @@ +// 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. + +#ifndef BITCOIN_TEST_UTIL_MINING_H +#define BITCOIN_TEST_UTIL_MINING_H + +#include <memory> +#include <string> + +class CBlock; +class CScript; +class CTxIn; + +/** Returns the generated coin */ +CTxIn MineBlock(const CScript& coinbase_scriptPubKey); + +/** Prepare a block to be mined */ +std::shared_ptr<CBlock> PrepareBlock(const CScript& coinbase_scriptPubKey); + +/** RPC-like helper function, returns the generated coin */ +CTxIn generatetoaddress(const std::string& address); + +#endif // BITCOIN_TEST_UTIL_MINING_H diff --git a/src/test/util/str.h b/src/test/util/str.h index 63629501e8..ef94692df0 100644 --- a/src/test/util/str.h +++ b/src/test/util/str.h @@ -9,4 +9,37 @@ bool CaseInsensitiveEqual(const std::string& s1, const std::string& s2); +/** + * Increment a string. Useful to enumerate all fixed length strings with + * characters in [min_char, max_char]. + */ +template <typename CharType, size_t StringLength> +bool NextString(CharType (&string)[StringLength], CharType min_char, CharType max_char) +{ + for (CharType& elem : string) { + bool has_next = elem != max_char; + elem = elem < min_char || elem >= max_char ? min_char : CharType(elem + 1); + if (has_next) return true; + } + return false; +} + +/** + * Iterate over string values and call function for each string without + * successive duplicate characters. + */ +template <typename CharType, size_t StringLength, typename Fn> +void ForEachNoDup(CharType (&string)[StringLength], CharType min_char, CharType max_char, Fn&& fn) { + for (bool has_next = true; has_next; has_next = NextString(string, min_char, max_char)) { + int prev = -1; + bool skip_string = false; + for (CharType c : string) { + if (c == prev) skip_string = true; + if (skip_string || c < min_char || c > max_char) break; + prev = c; + } + if (!skip_string) fn(); + } +} + #endif // BITCOIN_TEST_UTIL_STR_H diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp new file mode 100644 index 0000000000..226d2df6e4 --- /dev/null +++ b/src/test/util/wallet.cpp @@ -0,0 +1,40 @@ +// 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. + +#include <test/util/wallet.h> + +#include <key_io.h> +#include <outputtype.h> +#include <script/standard.h> +#ifdef ENABLE_WALLET +#include <wallet/wallet.h> +#endif + +const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj"; + +#ifdef ENABLE_WALLET +std::string getnewaddress(CWallet& w) +{ + constexpr auto output_type = OutputType::BECH32; + CTxDestination dest; + std::string error; + if (!w.GetNewDestination(output_type, "", dest, error)) assert(false); + + return EncodeDestination(dest); +} + +void importaddress(CWallet& wallet, const std::string& address) +{ + auto spk_man = wallet.GetLegacyScriptPubKeyMan(); + LOCK(wallet.cs_wallet); + AssertLockHeld(spk_man->cs_wallet); + const auto dest = DecodeDestination(address); + assert(IsValidDestination(dest)); + const auto script = GetScriptForDestination(dest); + wallet.MarkDirty(); + assert(!spk_man->HaveWatchOnly(script)); + if (!spk_man->AddWatchOnly(script, 0 /* nCreateTime */)) assert(false); + wallet.SetAddressBook(dest, /* label */ "", "receive"); +} +#endif // ENABLE_WALLET diff --git a/src/test/util/wallet.h b/src/test/util/wallet.h new file mode 100644 index 0000000000..565ef1756a --- /dev/null +++ b/src/test/util/wallet.h @@ -0,0 +1,24 @@ +// 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. + +#ifndef BITCOIN_TEST_UTIL_WALLET_H +#define BITCOIN_TEST_UTIL_WALLET_H + +#include <string> + +class CWallet; + +// Constants // + +extern const std::string ADDRESS_BCRT1_UNSPENDABLE; + +// RPC-like // + +/** Import the address to the wallet */ +void importaddress(CWallet& wallet, const std::string& address); +/** Returns a new address from the wallet */ +std::string getnewaddress(CWallet& w); + + +#endif // BITCOIN_TEST_UTIL_WALLET_H diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index b9fcd97a8f..86ea56ff36 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -5,9 +5,10 @@ #include <util/system.h> #include <clientversion.h> +#include <optional.h> #include <sync.h> #include <test/util/setup_common.h> -#include <test/util.h> +#include <test/util/str.h> #include <util/moneystr.h> #include <util/strencodings.h> #include <util/string.h> @@ -189,12 +190,119 @@ struct TestArgsManager : public ArgsManager AddArg(arg.first, "", arg.second, OptionsCategory::OPTIONS); } } + using ArgsManager::GetSetting; + using ArgsManager::GetSettingsList; using ArgsManager::ReadConfigStream; using ArgsManager::cs_args; using ArgsManager::m_network; using ArgsManager::m_settings; }; +//! Test GetSetting and GetArg type coercion, negation, and default value handling. +class CheckValueTest : public TestChain100Setup +{ +public: + struct Expect { + util::SettingsValue setting; + bool default_string = false; + bool default_int = false; + bool default_bool = false; + const char* string_value = nullptr; + Optional<int64_t> int_value; + Optional<bool> bool_value; + Optional<std::vector<std::string>> list_value; + const char* error = nullptr; + + Expect(util::SettingsValue s) : setting(std::move(s)) {} + Expect& DefaultString() { default_string = true; return *this; } + Expect& DefaultInt() { default_int = true; return *this; } + Expect& DefaultBool() { default_bool = true; return *this; } + Expect& String(const char* s) { string_value = s; return *this; } + Expect& Int(int64_t i) { int_value = i; return *this; } + Expect& Bool(bool b) { bool_value = b; return *this; } + Expect& List(std::vector<std::string> m) { list_value = std::move(m); return *this; } + Expect& Error(const char* e) { error = e; return *this; } + }; + + void CheckValue(unsigned int flags, const char* arg, const Expect& expect) + { + TestArgsManager test; + test.SetupArgs({{"-value", flags}}); + const char* argv[] = {"ignored", arg}; + std::string error; + bool success = test.ParseParameters(arg ? 2 : 1, (char**)argv, error); + + BOOST_CHECK_EQUAL(test.GetSetting("-value").write(), expect.setting.write()); + auto settings_list = test.GetSettingsList("-value"); + if (expect.setting.isNull() || expect.setting.isFalse()) { + BOOST_CHECK_EQUAL(settings_list.size(), 0); + } else { + BOOST_CHECK_EQUAL(settings_list.size(), 1); + BOOST_CHECK_EQUAL(settings_list[0].write(), expect.setting.write()); + } + + if (expect.error) { + BOOST_CHECK(!success); + BOOST_CHECK_NE(error.find(expect.error), std::string::npos); + } else { + BOOST_CHECK(success); + BOOST_CHECK_EQUAL(error, ""); + } + + if (expect.default_string) { + BOOST_CHECK_EQUAL(test.GetArg("-value", "zzzzz"), "zzzzz"); + } else if (expect.string_value) { + BOOST_CHECK_EQUAL(test.GetArg("-value", "zzzzz"), expect.string_value); + } else { + BOOST_CHECK(!success); + } + + if (expect.default_int) { + BOOST_CHECK_EQUAL(test.GetArg("-value", 99999), 99999); + } else if (expect.int_value) { + BOOST_CHECK_EQUAL(test.GetArg("-value", 99999), *expect.int_value); + } else { + BOOST_CHECK(!success); + } + + if (expect.default_bool) { + BOOST_CHECK_EQUAL(test.GetBoolArg("-value", false), false); + BOOST_CHECK_EQUAL(test.GetBoolArg("-value", true), true); + } else if (expect.bool_value) { + BOOST_CHECK_EQUAL(test.GetBoolArg("-value", false), *expect.bool_value); + BOOST_CHECK_EQUAL(test.GetBoolArg("-value", true), *expect.bool_value); + } else { + BOOST_CHECK(!success); + } + + if (expect.list_value) { + auto l = test.GetArgs("-value"); + BOOST_CHECK_EQUAL_COLLECTIONS(l.begin(), l.end(), expect.list_value->begin(), expect.list_value->end()); + } else { + BOOST_CHECK(!success); + } + } +}; + +BOOST_FIXTURE_TEST_CASE(util_CheckValue, CheckValueTest) +{ + using M = ArgsManager; + + CheckValue(M::ALLOW_ANY, nullptr, Expect{{}}.DefaultString().DefaultInt().DefaultBool().List({})); + CheckValue(M::ALLOW_ANY, "-novalue", Expect{false}.String("0").Int(0).Bool(false).List({})); + CheckValue(M::ALLOW_ANY, "-novalue=", Expect{false}.String("0").Int(0).Bool(false).List({})); + CheckValue(M::ALLOW_ANY, "-novalue=0", Expect{true}.String("1").Int(1).Bool(true).List({"1"})); + CheckValue(M::ALLOW_ANY, "-novalue=1", Expect{false}.String("0").Int(0).Bool(false).List({})); + CheckValue(M::ALLOW_ANY, "-novalue=2", Expect{false}.String("0").Int(0).Bool(false).List({})); + CheckValue(M::ALLOW_ANY, "-novalue=abc", Expect{true}.String("1").Int(1).Bool(true).List({"1"})); + CheckValue(M::ALLOW_ANY, "-value", Expect{""}.String("").Int(0).Bool(true).List({""})); + CheckValue(M::ALLOW_ANY, "-value=", Expect{""}.String("").Int(0).Bool(true).List({""})); + CheckValue(M::ALLOW_ANY, "-value=0", Expect{"0"}.String("0").Int(0).Bool(false).List({"0"})); + CheckValue(M::ALLOW_ANY, "-value=1", Expect{"1"}.String("1").Int(1).Bool(true).List({"1"})); + CheckValue(M::ALLOW_ANY, "-value=2", Expect{"2"}.String("2").Int(2).Bool(true).List({"2"})); + CheckValue(M::ALLOW_ANY, "-value=abc", Expect{"abc"}.String("abc").Int(0).Bool(false).List({"abc"})); +} + BOOST_AUTO_TEST_CASE(util_ParseParameters) { TestArgsManager testArgs; @@ -289,12 +397,12 @@ BOOST_AUTO_TEST_CASE(util_ArgParsing) BOOST_AUTO_TEST_CASE(util_GetBoolArg) { TestArgsManager testArgs; - const auto a = std::make_pair("-a", ArgsManager::ALLOW_BOOL); - const auto b = std::make_pair("-b", ArgsManager::ALLOW_BOOL); - const auto c = std::make_pair("-c", ArgsManager::ALLOW_BOOL); - const auto d = std::make_pair("-d", ArgsManager::ALLOW_BOOL); - const auto e = std::make_pair("-e", ArgsManager::ALLOW_BOOL); - const auto f = std::make_pair("-f", ArgsManager::ALLOW_BOOL); + const auto a = std::make_pair("-a", ArgsManager::ALLOW_ANY); + const auto b = std::make_pair("-b", ArgsManager::ALLOW_ANY); + const auto c = std::make_pair("-c", ArgsManager::ALLOW_ANY); + const auto d = std::make_pair("-d", ArgsManager::ALLOW_ANY); + const auto e = std::make_pair("-e", ArgsManager::ALLOW_ANY); + const auto f = std::make_pair("-f", ArgsManager::ALLOW_ANY); const char *argv_test[] = { "ignored", "-a", "-nob", "-c=0", "-d=1", "-e=false", "-f=true"}; @@ -333,8 +441,8 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases) TestArgsManager testArgs; // Params test - const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_BOOL); - const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_BOOL); + const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY); + const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY); const char *argv_test[] = {"ignored", "-nofoo", "-foo", "-nobar=0"}; testArgs.SetupArgs({foo, bar}); std::string error; @@ -406,16 +514,16 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) TestArgsManager test_args; LOCK(test_args.cs_args); - const auto a = std::make_pair("-a", ArgsManager::ALLOW_BOOL); - const auto b = std::make_pair("-b", ArgsManager::ALLOW_BOOL); - const auto ccc = std::make_pair("-ccc", ArgsManager::ALLOW_STRING); - const auto d = std::make_pair("-d", ArgsManager::ALLOW_STRING); + const auto a = std::make_pair("-a", ArgsManager::ALLOW_ANY); + const auto b = std::make_pair("-b", ArgsManager::ALLOW_ANY); + const auto ccc = std::make_pair("-ccc", ArgsManager::ALLOW_ANY); + const auto d = std::make_pair("-d", ArgsManager::ALLOW_ANY); const auto e = std::make_pair("-e", ArgsManager::ALLOW_ANY); - const auto fff = std::make_pair("-fff", ArgsManager::ALLOW_BOOL); - const auto ggg = std::make_pair("-ggg", ArgsManager::ALLOW_BOOL); - const auto h = std::make_pair("-h", ArgsManager::ALLOW_BOOL); - const auto i = std::make_pair("-i", ArgsManager::ALLOW_BOOL); - const auto iii = std::make_pair("-iii", ArgsManager::ALLOW_INT); + const auto fff = std::make_pair("-fff", ArgsManager::ALLOW_ANY); + const auto ggg = std::make_pair("-ggg", ArgsManager::ALLOW_ANY); + const auto h = std::make_pair("-h", ArgsManager::ALLOW_ANY); + const auto i = std::make_pair("-i", ArgsManager::ALLOW_ANY); + const auto iii = std::make_pair("-iii", ArgsManager::ALLOW_ANY); test_args.SetupArgs({a, b, ccc, d, e, fff, ggg, h, i, iii}); test_args.ReadConfigString(str_config); @@ -618,8 +726,8 @@ BOOST_AUTO_TEST_CASE(util_GetArg) BOOST_AUTO_TEST_CASE(util_GetChainName) { TestArgsManager test_args; - const auto testnet = std::make_pair("-testnet", ArgsManager::ALLOW_BOOL); - const auto regtest = std::make_pair("-regtest", ArgsManager::ALLOW_BOOL); + const auto testnet = std::make_pair("-testnet", ArgsManager::ALLOW_ANY); + const auto regtest = std::make_pair("-regtest", ArgsManager::ALLOW_ANY); test_args.SetupArgs({testnet, regtest}); const char* argv_testnet[] = {"cmd", "-testnet"}; @@ -1069,6 +1177,11 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney) // Parsing negative amounts must fail BOOST_CHECK(!ParseMoney("-1", ret)); + + // Parsing strings with embedded NUL characters should fail + BOOST_CHECK(!ParseMoney(std::string("\0-1", 3), ret)); + BOOST_CHECK(!ParseMoney(std::string("\01", 2), ret)); + BOOST_CHECK(!ParseMoney(std::string("1\0", 2), ret)); } BOOST_AUTO_TEST_CASE(util_IsHex) diff --git a/src/util/moneystr.cpp b/src/util/moneystr.cpp index ba5a12e58c..3e75a2e3e9 100644 --- a/src/util/moneystr.cpp +++ b/src/util/moneystr.cpp @@ -7,6 +7,7 @@ #include <tinyformat.h> #include <util/strencodings.h> +#include <util/string.h> std::string FormatMoney(const CAmount& n) { @@ -32,6 +33,9 @@ std::string FormatMoney(const CAmount& n) bool ParseMoney(const std::string& str, CAmount& nRet) { + if (!ValidAsCString(str)) { + return false; + } return ParseMoney(str.c_str(), nRet); } diff --git a/src/util/settings.cpp b/src/util/settings.cpp index af75fef310..e4979df755 100644 --- a/src/util/settings.cpp +++ b/src/util/settings.cpp @@ -56,6 +56,7 @@ SettingsValue GetSetting(const Settings& settings, bool get_chain_name) { SettingsValue result; + bool done = false; // Done merging any more settings sources. MergeSettings(settings, section, name, [&](SettingsSpan span, Source source) { // Weird behavior preserved for backwards compatibility: Apply negated // setting even if non-negated setting would be ignored. A negated @@ -68,7 +69,9 @@ SettingsValue GetSetting(const Settings& settings, // precedence over early settings, but for backwards compatibility in // the config file the precedence is reversed for all settings except // chain name settings. - const bool reverse_precedence = (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) && !get_chain_name; + const bool reverse_precedence = + (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) && + !get_chain_name; // Weird behavior preserved for backwards compatibility: Negated // -regtest and -testnet arguments which you would expect to override @@ -77,19 +80,23 @@ SettingsValue GetSetting(const Settings& settings, // negated values, or at least warn they are ignored. const bool skip_negated_command_line = get_chain_name; + if (done) return; + // Ignore settings in default config section if requested. - if (ignore_default_section_config && source == Source::CONFIG_FILE_DEFAULT_SECTION && !never_ignore_negated_setting) return; + if (ignore_default_section_config && source == Source::CONFIG_FILE_DEFAULT_SECTION && + !never_ignore_negated_setting) { + return; + } // Skip negated command line settings. if (skip_negated_command_line && span.last_negated()) return; - // Stick with highest priority value, keeping result if already set. - if (!result.isNull()) return; - if (!span.empty()) { result = reverse_precedence ? span.begin()[0] : span.end()[-1]; + done = true; } else if (span.last_negated()) { result = false; + done = true; } }); return result; @@ -101,7 +108,7 @@ std::vector<SettingsValue> GetSettingsList(const Settings& settings, bool ignore_default_section_config) { std::vector<SettingsValue> result; - bool result_complete = false; + bool done = false; // Done merging any more settings sources. bool prev_negated_empty = false; MergeSettings(settings, section, name, [&](SettingsSpan span, Source source) { // Weird behavior preserved for backwards compatibility: Apply config @@ -111,14 +118,16 @@ std::vector<SettingsValue> GetSettingsList(const Settings& settings, // value is followed by non-negated value, in which case config file // settings will be brought back from the dead (but earlier command // line settings will still be ignored). - const bool add_zombie_config_values = (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) && !prev_negated_empty; + const bool add_zombie_config_values = + (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) && + !prev_negated_empty; // Ignore settings in default config section if requested. if (ignore_default_section_config && source == Source::CONFIG_FILE_DEFAULT_SECTION) return; // Add new settings to the result if isn't already complete, or if the // values are zombies. - if (!result_complete || add_zombie_config_values) { + if (!done || add_zombie_config_values) { for (const auto& value : span) { if (value.isArray()) { result.insert(result.end(), value.getValues().begin(), value.getValues().end()); @@ -129,8 +138,8 @@ std::vector<SettingsValue> GetSettingsList(const Settings& settings, } // If a setting was negated, or if a setting was forced, set - // result_complete to true to ignore any later lower priority settings. - result_complete |= span.negated() > 0 || source == Source::FORCED; + // done to true to ignore any later lower priority settings. + done |= span.negated() > 0 || source == Source::FORCED; // Update the negated and empty state used for the zombie values check. prev_negated_empty |= span.last_negated() && result.empty(); diff --git a/src/util/settings.h b/src/util/settings.h index 17832e4d2c..9ca581109d 100644 --- a/src/util/settings.h +++ b/src/util/settings.h @@ -43,11 +43,18 @@ struct Settings { //! [section] keywords) //! @param get_chain_name - enable special backwards compatible behavior //! for GetChainName -SettingsValue GetSetting(const Settings& settings, const std::string& section, const std::string& name, bool ignore_default_section_config, bool get_chain_name); +SettingsValue GetSetting(const Settings& settings, + const std::string& section, + const std::string& name, + bool ignore_default_section_config, + bool get_chain_name); //! Get combined setting value similar to GetSetting(), except if setting was //! specified multiple times, return a list of all the values specified. -std::vector<SettingsValue> GetSettingsList(const Settings& settings, const std::string& section, const std::string& name, bool ignore_default_section_config); +std::vector<SettingsValue> GetSettingsList(const Settings& settings, + const std::string& section, + const std::string& name, + bool ignore_default_section_config); //! Return true if a setting is set in the default config file section, and not //! overridden by a higher priority command-line or network section value. diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 8f2d05f03b..31719cd975 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -191,6 +191,12 @@ std::vector<unsigned char> DecodeBase64(const char* p, bool* pf_invalid) std::string DecodeBase64(const std::string& str, bool* pf_invalid) { + if (!ValidAsCString(str)) { + if (pf_invalid) { + *pf_invalid = true; + } + return {}; + } std::vector<unsigned char> vchRet = DecodeBase64(str.c_str(), pf_invalid); return std::string((const char*)vchRet.data(), vchRet.size()); } @@ -260,6 +266,12 @@ std::vector<unsigned char> DecodeBase32(const char* p, bool* pf_invalid) std::string DecodeBase32(const std::string& str, bool* pf_invalid) { + if (!ValidAsCString(str)) { + if (pf_invalid) { + *pf_invalid = true; + } + return {}; + } std::vector<unsigned char> vchRet = DecodeBase32(str.c_str(), pf_invalid); return std::string((const char*)vchRet.data(), vchRet.size()); } diff --git a/src/util/string.h b/src/util/string.h index c6fa08e5b3..3db8fc8b98 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -11,6 +11,16 @@ #include <string> #include <vector> +NODISCARD inline std::string TrimString(const std::string& str, const std::string& pattern = " \f\n\r\t\v") +{ + std::string::size_type front = str.find_first_not_of(pattern); + if (front == std::string::npos) { + return std::string(); + } + std::string::size_type end = str.find_last_not_of(pattern); + return str.substr(front, end - front + 1); +} + /** * Join a list of items * diff --git a/src/util/system.cpp b/src/util/system.cpp index 563ff6a54b..d99a87a9f2 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -7,6 +7,7 @@ #include <chainparamsbase.h> #include <util/strencodings.h> +#include <util/string.h> #include <util/translation.h> @@ -167,23 +168,6 @@ static std::string SettingName(const std::string& arg) return arg.size() > 0 && arg[0] == '-' ? arg.substr(1) : arg; } -/** Internal helper functions for ArgsManager */ -class ArgsManagerHelper { -public: - /** Determine whether to use config settings in the default section, - * See also comments around ArgsManager::ArgsManager() below. */ - static inline bool UseDefaultSection(const ArgsManager& am, const std::string& arg) EXCLUSIVE_LOCKS_REQUIRED(am.cs_args) - { - return (am.m_network == CBaseChainParams::MAIN || am.m_network_only_args.count(arg) == 0); - } - - static util::SettingsValue Get(const ArgsManager& am, const std::string& arg) - { - LOCK(am.cs_args); - return GetSetting(am.m_settings, am.m_network, SettingName(arg), !UseDefaultSection(am, arg), /* get_chain_name= */ false); - } -}; - /** * Interpret -nofoo as if the user supplied -foo=0. * @@ -326,9 +310,9 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin key.erase(0, 1); std::string section; util::SettingsValue value = InterpretOption(section, key, val); - const unsigned int flags = FlagsOfKnownArg(key); + Optional<unsigned int> flags = GetArgFlags('-' + key); if (flags) { - if (!CheckValid(key, value, flags, error)) { + if (!CheckValid(key, value, *flags, error)) { return false; } // Weird behavior preserved for backwards compatibility: command @@ -344,7 +328,7 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin } } - // we do not allow -includeconf from command line, so we clear it here + // we do not allow -includeconf from command line bool success = true; if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) { for (const auto& include : util::SettingsSpan(*includes)) { @@ -355,25 +339,22 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin return success; } -unsigned int ArgsManager::FlagsOfKnownArg(const std::string& key) const +Optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) const { LOCK(cs_args); for (const auto& arg_map : m_available_args) { - const auto search = arg_map.second.find('-' + key); + const auto search = arg_map.second.find(name); if (search != arg_map.second.end()) { return search->second.m_flags; } } - return ArgsManager::NONE; + return nullopt; } std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const { - LOCK(cs_args); - bool ignore_default_section_config = !ArgsManagerHelper::UseDefaultSection(*this, strArg); std::vector<std::string> result; - for (const util::SettingsValue& value : - util::GetSettingsList(m_settings, m_network, SettingName(strArg), ignore_default_section_config)) { + for (const util::SettingsValue& value : GetSettingsList(strArg)) { result.push_back(value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str()); } return result; @@ -381,29 +362,29 @@ std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const bool ArgsManager::IsArgSet(const std::string& strArg) const { - return !ArgsManagerHelper::Get(*this, strArg).isNull(); + return !GetSetting(strArg).isNull(); } bool ArgsManager::IsArgNegated(const std::string& strArg) const { - return ArgsManagerHelper::Get(*this, strArg).isFalse(); + return GetSetting(strArg).isFalse(); } std::string ArgsManager::GetArg(const std::string& strArg, const std::string& strDefault) const { - const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg); + const util::SettingsValue value = GetSetting(strArg); return value.isNull() ? strDefault : value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str(); } int64_t ArgsManager::GetArg(const std::string& strArg, int64_t nDefault) const { - const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg); + const util::SettingsValue value = GetSetting(strArg); return value.isNull() ? nDefault : value.isFalse() ? 0 : value.isTrue() ? 1 : value.isNum() ? value.get_int64() : atoi64(value.get_str()); } bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const { - const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg); + const util::SettingsValue value = GetSetting(strArg); return value.isNull() ? fDefault : value.isBool() ? value.get_bool() : InterpretBool(value.get_str()); } @@ -679,16 +660,6 @@ fs::path GetConfigFile(const std::string& confPath) return AbsPathForConfigVal(fs::path(confPath), false); } -static std::string TrimString(const std::string& str, const std::string& pattern) -{ - std::string::size_type front = str.find_first_not_of(pattern); - if (front == std::string::npos) { - return std::string(); - } - std::string::size_type end = str.find_last_not_of(pattern); - return str.substr(front, end - front + 1); -} - static bool GetConfigOptions(std::istream& stream, const std::string& filepath, std::string& error, std::vector<std::pair<std::string, std::string>>& options, std::list<SectionInfo>& sections) { std::string str, prefix; @@ -745,9 +716,9 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file std::string section; std::string key = option.first; util::SettingsValue value = InterpretOption(section, key, option.second); - const unsigned int flags = FlagsOfKnownArg(key); + Optional<unsigned int> flags = GetArgFlags('-' + key); if (flags) { - if (!CheckValid(key, value, flags, error)) { + if (!CheckValid(key, value, *flags, error)) { return false; } m_settings.ro_config[section][key].push_back(value); @@ -780,7 +751,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) return false; } // `-includeconf` cannot be included in the command line arguments except - // as `-noincludeconf` (which indicates that no conf file should be used). + // as `-noincludeconf` (which indicates that no included conf file should be used). bool use_conf_file{true}; { LOCK(cs_args); @@ -854,9 +825,9 @@ std::string ArgsManager::GetChainName() const { auto get_net = [&](const std::string& arg) { LOCK(cs_args); - util::SettingsValue value = GetSetting(m_settings, /* section= */ "", SettingName(arg), - /* ignore_default_section_config= */ false, - /* get_chain_name= */ true); + util::SettingsValue value = util::GetSetting(m_settings, /* section= */ "", SettingName(arg), + /* ignore_default_section_config= */ false, + /* get_chain_name= */ true); return value.isNull() ? false : value.isBool() ? value.get_bool() : InterpretBool(value.get_str()); }; @@ -874,6 +845,24 @@ std::string ArgsManager::GetChainName() const return GetArg("-chain", CBaseChainParams::MAIN); } +bool ArgsManager::UseDefaultSection(const std::string& arg) const +{ + return m_network == CBaseChainParams::MAIN || m_network_only_args.count(arg) == 0; +} + +util::SettingsValue ArgsManager::GetSetting(const std::string& arg) const +{ + LOCK(cs_args); + return util::GetSetting( + m_settings, m_network, SettingName(arg), !UseDefaultSection(arg), /* get_chain_name= */ false); +} + +std::vector<util::SettingsValue> ArgsManager::GetSettingsList(const std::string& arg) const +{ + LOCK(cs_args); + return util::GetSettingsList(m_settings, m_network, SettingName(arg), !UseDefaultSection(arg)); +} + bool RenameOver(fs::path src, fs::path dest) { #ifdef WIN32 diff --git a/src/util/system.h b/src/util/system.h index 82903b5187..394adb9555 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -19,6 +19,7 @@ #include <compat/assumptions.h> #include <fs.h> #include <logging.h> +#include <optional.h> #include <sync.h> #include <tinyformat.h> #include <util/memory.h> @@ -132,7 +133,6 @@ class ArgsManager { public: enum Flags { - NONE = 0x00, // Boolean options can accept negation syntax -noOPTION or -noOPTION=1 ALLOW_BOOL = 0x01, ALLOW_INT = 0x02, @@ -148,8 +148,6 @@ public: }; protected: - friend class ArgsManagerHelper; - struct Arg { std::string m_help_param; @@ -166,6 +164,27 @@ protected: NODISCARD bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false); + /** + * Returns true if settings values from the default section should be used, + * depending on the current network and whether the setting is + * network-specific. + */ + bool UseDefaultSection(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(cs_args); + + /** + * Get setting value. + * + * Result will be null if setting was unset, true if "-setting" argument was passed + * false if "-nosetting" argument was passed, and a string if a "-setting=value" + * argument was passed. + */ + util::SettingsValue GetSetting(const std::string& arg) const; + + /** + * Get list of setting values. + */ + std::vector<util::SettingsValue> GetSettingsList(const std::string& arg) const; + public: ArgsManager(); @@ -296,9 +315,9 @@ public: /** * Return Flags for known arg. - * Return ArgsManager::NONE for unknown arg. + * Return nullopt for unknown arg. */ - unsigned int FlagsOfKnownArg(const std::string& key) const; + Optional<unsigned int> GetArgFlags(const std::string& name) const; }; extern ArgsManager gArgs; diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index b4315014cb..be8a71da97 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -14,7 +14,6 @@ bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error) { error.clear(); - TopUp(); // Generate a new key that is added to wallet CPubKey new_key; @@ -1153,8 +1152,6 @@ bool LegacyScriptPubKeyMan::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& key { LOCK(cs_wallet); - TopUp(); - bool fReturningInternal = fRequestedInternal; fReturningInternal &= (IsHDEnabled() && m_storage.CanSupportFeature(FEATURE_HD_SPLIT)) || m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); bool use_split_keypool = set_pre_split_keypool.empty(); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index aa5eac3a85..b78494921c 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -160,6 +160,10 @@ public: virtual void KeepDestination(int64_t index, const OutputType& type) {} virtual void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) {} + /** Fills internal address pool. Use within ScriptPubKeyMan implementations should be used sparingly and only + * when something from the address pool is removed, excluding GetNewDestination and GetReservedDestination. + * External wallet code is primarily responsible for topping up prior to fetching new addresses + */ virtual bool TopUp(unsigned int size = 0) { return false; } //! Mark unused addresses as being used diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 41a816312a..3954f66267 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3105,6 +3105,7 @@ bool CWallet::GetNewDestination(const OutputType type, const std::string label, bool result = false; auto spk_man = m_spk_man.get(); if (spk_man) { + spk_man->TopUp(); result = spk_man->GetNewDestination(type, dest, error); } if (result) { @@ -3118,8 +3119,6 @@ bool CWallet::GetNewChangeDestination(const OutputType type, CTxDestination& des { error.clear(); - m_spk_man->TopUp(); - ReserveDestination reservedest(this, type); if (!reservedest.GetReservedDestination(dest, true)) { error = "Error: Keypool ran out, please call keypoolrefill first"; @@ -3297,6 +3296,8 @@ bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool inter if (nIndex == -1) { + m_spk_man->TopUp(); + CKeyPool keypool; if (!m_spk_man->GetReservedDestination(type, internal, address, nIndex, keypool)) { return false; diff --git a/src/warnings.cpp b/src/warnings.cpp index 35d2033ba8..8df77096f1 100644 --- a/src/warnings.cpp +++ b/src/warnings.cpp @@ -38,41 +38,34 @@ void SetfLargeWorkInvalidChainFound(bool flag) fLargeWorkInvalidChainFound = flag; } -std::string GetWarnings(const std::string& strFor) +std::string GetWarnings(bool verbose) { - std::string strStatusBar; - std::string strGUI; - const std::string uiAlertSeperator = "<hr />"; + std::string warnings_concise; + std::string warnings_verbose; + const std::string warning_separator = "<hr />"; LOCK(cs_warnings); + // Pre-release build warning if (!CLIENT_VERSION_IS_RELEASE) { - strStatusBar = "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"; - strGUI = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications").translated; + warnings_concise = "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"; + warnings_verbose = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications").translated; } // Misc warnings like out of disk space and clock is wrong - if (strMiscWarning != "") - { - strStatusBar = strMiscWarning; - strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + strMiscWarning; + if (strMiscWarning != "") { + warnings_concise = strMiscWarning; + warnings_verbose += (warnings_verbose.empty() ? "" : warning_separator) + strMiscWarning; } - if (fLargeWorkForkFound) - { - strStatusBar = "Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues."; - strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.").translated; - } - else if (fLargeWorkInvalidChainFound) - { - strStatusBar = "Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade."; - strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.").translated; + if (fLargeWorkForkFound) { + warnings_concise = "Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues."; + warnings_verbose += (warnings_verbose.empty() ? "" : warning_separator) + _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.").translated; + } else if (fLargeWorkInvalidChainFound) { + warnings_concise = "Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade."; + warnings_verbose += (warnings_verbose.empty() ? "" : warning_separator) + _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.").translated; } - if (strFor == "gui") - return strGUI; - else if (strFor == "statusbar") - return strStatusBar; - assert(!"GetWarnings(): invalid parameter"); - return "error"; + if (verbose) return warnings_verbose; + else return warnings_concise; } diff --git a/src/warnings.h b/src/warnings.h index e6701ebd9e..58e5e4cd19 100644 --- a/src/warnings.h +++ b/src/warnings.h @@ -13,11 +13,11 @@ void SetfLargeWorkForkFound(bool flag); bool GetfLargeWorkForkFound(); void SetfLargeWorkInvalidChainFound(bool flag); /** Format a string that describes several potential problems detected by the core. - * @param[in] strFor can have the following values: - * - "statusbar": get the most important warning - * - "gui": get all warnings, translated (where possible) for GUI, separated by <hr /> - * @returns the warning string selected by strFor + * @param[in] verbose bool + * - if true, get all warnings, translated (where possible), separated by <hr /> + * - if false, get the most important warning + * @returns the warning string */ -std::string GetWarnings(const std::string& strFor); +std::string GetWarnings(bool verbose); #endif // BITCOIN_WARNINGS_H diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index 2d255c0bb4..bbdd047465 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -15,11 +15,14 @@ import logging # Fuzzers known to lack a seed corpus in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus FUZZERS_MISSING_CORPORA = [ "addr_info_deserialize", + "base_encode_decode", + "block", "block_file_info_deserialize", "block_filter_deserialize", "block_header_and_short_txids_deserialize", "fee_rate_deserialize", "flat_file_pos_deserialize", + "hex", "integer", "key_origin_info_deserialize", "merkle_block_deserialize", |