diff options
76 files changed, 1059 insertions, 690 deletions
diff --git a/.travis.yml b/.travis.yml index 9a111c03ad..1ac3b9969f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -25,6 +25,8 @@ # [1] https://docs.travis-ci.com/user/caching/#build-phases # [2] https://docs.travis-ci.com/user/customizing-the-build#build-timeouts +version: ~> 1.0 + dist: xenial os: linux language: minimal @@ -84,19 +86,19 @@ jobs: - stage: test name: 'ARM [GOAL: install] [buster] [unit tests, functional tests]' - arch: arm64 + arch: arm64 # Can disable QEMU_USER_CMD and run the tests natively without qemu env: >- FILE_ENV="./ci/test/00_setup_env_arm.sh" - QEMU_USER_CMD="" # Can run the tests natively without qemu + QEMU_USER_CMD="" # s390 build was disabled temporarily because of disk space issues on the Travis VM # # - stage: test # name: 'S390x [GOAL: install] [buster] [unit tests, functional tests]' -# arch: s390x +# arch: s390x # Can disable QEMU_USER_CMD and run the tests natively without qemu # env: >- # FILE_ENV="./ci/test/00_setup_env_s390x.sh" -# QEMU_USER_CMD="" # Can run the tests natively without qemu +# QEMU_USER_CMD="" - stage: test name: 'Win64 [GOAL: deploy] [unit tests, no gui, no functional tests]' @@ -153,8 +155,20 @@ jobs: name: 'macOS 10.14 native [GOAL: install] [GUI] [no depends]' os: osx # Use the most recent version: - # Xcode 11.2.1, macOS 10.14, JDK 13.0.1, SDK 10.15 + # Xcode 11.3.1, macOS 10.14, SDK 10.15 # https://docs.travis-ci.com/user/reference/osx/#macos-version - osx_image: xcode11.2 + osx_image: xcode11.3 + addons: + homebrew: + packages: + - libtool + - berkeley-db4 + - boost + - miniupnpc + - qt + - qrencode + - python3 + - ccache + - zeromq env: >- FILE_ENV="./ci/test/00_setup_env_mac_host.sh" diff --git a/Makefile.am b/Makefile.am index bd41c5ae27..5428ba93ee 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,4 +1,4 @@ -# Copyright (c) 2013-2016 The Bitcoin Core developers +# Copyright (c) 2013-2020 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -43,16 +43,7 @@ OSX_INSTALLER_ICONS=$(top_srcdir)/src/qt/res/icons/bitcoin.icns OSX_PLIST=$(top_builddir)/share/qt/Info.plist #not installed OSX_QT_TRANSLATIONS = da,de,es,hu,ru,uk,zh_CN,zh_TW -DIST_DOCS = \ - README.md \ - $(wildcard doc/*.md) \ - $(wildcard doc/release-notes/*.md) -DIST_CONTRIB = $(top_srcdir)/contrib/bitcoin-cli.bash-completion \ - $(top_srcdir)/contrib/bitcoin-tx.bash-completion \ - $(top_srcdir)/contrib/bitcoind.bash-completion \ - $(top_srcdir)/contrib/debian/copyright \ - $(top_srcdir)/contrib/init \ - $(top_srcdir)/contrib/install_db4.sh \ +DIST_CONTRIB = \ $(top_srcdir)/contrib/linearize/linearize-data.py \ $(top_srcdir)/contrib/linearize/linearize-hashes.py @@ -246,7 +237,7 @@ endif dist_noinst_SCRIPTS = autogen.sh -EXTRA_DIST = $(DIST_SHARE) $(DIST_CONTRIB) $(DIST_DOCS) $(WINDOWS_PACKAGING) $(OSX_PACKAGING) $(BIN_CHECKS) +EXTRA_DIST = $(DIST_SHARE) $(DIST_CONTRIB) $(WINDOWS_PACKAGING) $(OSX_PACKAGING) $(BIN_CHECKS) EXTRA_DIST += \ test/functional \ diff --git a/ci/test/00_setup_env_mac_host.sh b/ci/test/00_setup_env_mac_host.sh index 0b437a723f..5753c3af31 100644 --- a/ci/test/00_setup_env_mac_host.sh +++ b/ci/test/00_setup_env_mac_host.sh @@ -7,7 +7,6 @@ export LC_ALL=C.UTF-8 export HOST=x86_64-apple-darwin16 -export BREW_PACKAGES="automake berkeley-db4 libtool boost miniupnpc pkg-config qt qrencode python3 ccache zeromq" export PIP_PACKAGES="zmq" export RUN_CI_ON_HOST=true export RUN_UNIT_TESTS=true diff --git a/ci/test/00_setup_env_native_valgrind.sh b/ci/test/00_setup_env_native_valgrind.sh index f112aa87f9..2a7b32cefc 100644 --- a/ci/test/00_setup_env_native_valgrind.sh +++ b/ci/test/00_setup_env_native_valgrind.sh @@ -10,7 +10,6 @@ export CONTAINER_NAME=ci_native_valgrind export PACKAGES="valgrind clang llvm python3-zmq libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-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="--exclude feature_abortnode,feature_block,rpc_bind" # Excluded for now -export RUN_FUNCTIONAL_TESTS=true +export TEST_RUNNER_EXTRA="--exclude rpc_bind" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 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/04_install.sh b/ci/test/04_install.sh index c6e8331a71..acf7eeb920 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -14,28 +14,8 @@ if [[ $QEMU_USER_CMD == qemu-s390* ]]; then fi if [ "$TRAVIS_OS_NAME" == "osx" ]; then - set +o errexit - pushd /usr/local/Homebrew || exit 1 - git reset --hard origin/master - popd || exit 1 - set -o errexit - ${CI_RETRY_EXE} brew unlink python@2 - ${CI_RETRY_EXE} brew update - # brew upgrade returns an error if any of the packages is already up to date - # Failure is safe to ignore, unless we really need an update. - brew upgrade $BREW_PACKAGES || true - - # install new packages (brew install returns an error if already installed) - for i in $BREW_PACKAGES; do - if ! brew list | grep -q $i; then - ${CI_RETRY_EXE} brew install $i - fi - done - export PATH="/usr/local/opt/ccache/libexec:$PATH" - ${CI_RETRY_EXE} pip3 install $PIP_PACKAGES - fi mkdir -p "${BASE_SCRATCH_DIR}" @@ -102,7 +82,9 @@ else fi if [ ! -d ${DIR_QA_ASSETS} ]; then + if [ "$RUN_FUZZ_TESTS" = "true" ]; then DOCKER_EXEC git clone https://github.com/bitcoin-core/qa-assets ${DIR_QA_ASSETS} + fi fi export DIR_FUZZ_IN=${DIR_QA_ASSETS}/fuzz_seed_corpus/ diff --git a/configure.ac b/configure.ac index 23a6417a1a..ef87d759f1 100644 --- a/configure.ac +++ b/configure.ac @@ -1255,7 +1255,7 @@ if test x$use_pkgconfig = xyes; then if test x$use_qr != xno; then BITCOIN_QT_CHECK([PKG_CHECK_MODULES([QR], [libqrencode], [have_qrencode=yes], [have_qrencode=no])]) fi - if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests != xnononono; then + if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench != xnonononono; then PKG_CHECK_MODULES([EVENT], [libevent],, [AC_MSG_ERROR(libevent not found.)]) if test x$TARGET_OS != xwindows; then PKG_CHECK_MODULES([EVENT_PTHREADS], [libevent_pthreads],, [AC_MSG_ERROR(libevent_pthreads not found.)]) @@ -1275,7 +1275,7 @@ if test x$use_pkgconfig = xyes; then ) else - if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests != xnononono; then + if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench != xnonononono; then AC_CHECK_HEADER([event2/event.h],, AC_MSG_ERROR(libevent headers missing),) AC_CHECK_LIB([event],[main],EVENT_LIBS=-levent,AC_MSG_ERROR(libevent missing)) if test x$TARGET_OS != xwindows; then diff --git a/contrib/devtools/README.md b/contrib/devtools/README.md index 515a0d8fc6..f5533719c0 100644 --- a/contrib/devtools/README.md +++ b/contrib/devtools/README.md @@ -109,7 +109,7 @@ certain symbols and are only linked against allowed libraries. For Linux this means checking for allowed gcc, glibc and libstdc++ version symbols. This makes sure they are still compatible with the minimum supported distribution versions. -For macOS we check that the executables are only linked against libraries we allow. +For macOS and Windows we check that the executables are only linked against libraries we allow. Example usage after a gitian build: diff --git a/contrib/devtools/symbol-check.py b/contrib/devtools/symbol-check.py index f92d997621..6949cb7ced 100755 --- a/contrib/devtools/symbol-check.py +++ b/contrib/devtools/symbol-check.py @@ -3,9 +3,8 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. ''' -A script to check that the (Linux) executables produced by gitian only contain -allowed gcc and glibc version symbols. This makes sure they are still compatible -with the minimum supported Linux distribution versions. +A script to check that the executables produced by gitian only contain +certain symbols and are only linked against allowed libraries. Example usage: @@ -53,6 +52,7 @@ IGNORE_EXPORTS = { } READELF_CMD = os.getenv('READELF', '/usr/bin/readelf') CPPFILT_CMD = os.getenv('CPPFILT', '/usr/bin/c++filt') +OBJDUMP_CMD = os.getenv('OBJDUMP', '/usr/bin/objdump') OTOOL_CMD = os.getenv('OTOOL', '/usr/bin/otool') # Allowed NEEDED libraries @@ -101,6 +101,26 @@ MACHO_ALLOWED_LIBRARIES = { 'libobjc.A.dylib', # Objective-C runtime library } +PE_ALLOWED_LIBRARIES = { +'ADVAPI32.dll', # security & registry +'IPHLPAPI.DLL', # IP helper API +'KERNEL32.dll', # win32 base APIs +'msvcrt.dll', # C standard library for MSVC +'SHELL32.dll', # shell API +'USER32.dll', # user interface +'WS2_32.dll', # sockets +# bitcoin-qt only +'dwmapi.dll', # desktop window manager +'GDI32.dll', # graphics device interface +'IMM32.dll', # input method editor +'ole32.dll', # component object model +'OLEAUT32.dll', # OLE Automation API +'SHLWAPI.dll', # light weight shell API +'UxTheme.dll', +'VERSION.dll', # version checking +'WINMM.dll', # WinMM audio API +} + class CPPFilt(object): ''' Demangle C++ symbol names. @@ -218,6 +238,26 @@ def check_MACHO_libraries(filename) -> bool: ok = False return ok +def pe_read_libraries(filename) -> List[str]: + p = subprocess.Popen([OBJDUMP_CMD, '-x', filename], stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE, universal_newlines=True) + (stdout, stderr) = p.communicate() + if p.returncode: + raise IOError('Error opening file') + libraries = [] + for line in stdout.splitlines(): + if 'DLL Name:' in line: + tokens = line.split(': ') + libraries.append(tokens[1]) + return libraries + +def check_PE_libraries(filename) -> bool: + ok = True + for dylib in pe_read_libraries(filename): + if dylib not in PE_ALLOWED_LIBRARIES: + print('{} is not in ALLOWED_LIBRARIES!'.format(dylib)) + ok = False + return ok + CHECKS = { 'ELF': [ ('IMPORTED_SYMBOLS', check_imported_symbols), @@ -226,6 +266,9 @@ CHECKS = { ], 'MACHO': [ ('DYNAMIC_LIBRARIES', check_MACHO_libraries) +], +'PE' : [ + ('DYNAMIC_LIBRARIES', check_PE_libraries) ] } diff --git a/contrib/gitian-descriptors/gitian-linux.yml b/contrib/gitian-descriptors/gitian-linux.yml index 4a8b125ae7..a13a42d391 100644 --- a/contrib/gitian-descriptors/gitian-linux.yml +++ b/contrib/gitian-descriptors/gitian-linux.yml @@ -147,13 +147,6 @@ script: | SOURCEDIST=$(echo bitcoin-*.tar.gz) DISTNAME=${SOURCEDIST/%.tar.gz} - # Correct tar file order - mkdir -p temp - pushd temp - tar -xf ../$SOURCEDIST - find bitcoin-* | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST - popd - # Workaround for tarball not building with the bare tag version (prep) make -C src obj/build.h @@ -190,11 +183,12 @@ script: | rm -rf ${DISTNAME}/lib/pkgconfig find ${DISTNAME}/bin -type f -executable -print0 | xargs -0 -n1 -I{} ../contrib/devtools/split-debug.sh {} {} {}.dbg find ${DISTNAME}/lib -type f -print0 | xargs -0 -n1 -I{} ../contrib/devtools/split-debug.sh {} {} {}.dbg - cp ../README.md ${DISTNAME}/ + cp ../../README.md ${DISTNAME}/ find ${DISTNAME} -not -name "*.dbg" | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}.tar.gz find ${DISTNAME} -name "*.dbg" | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}-debug.tar.gz cd ../../ rm -rf distsrc-${i} done - mkdir -p $OUTDIR/src - mv $SOURCEDIST $OUTDIR/src + + mkdir -p ${OUTDIR}/src + git archive --output=${OUTDIR}/src/${DISTNAME}.tar.gz HEAD diff --git a/contrib/gitian-descriptors/gitian-osx.yml b/contrib/gitian-descriptors/gitian-osx.yml index 2b6aa599e0..58531c81b4 100644 --- a/contrib/gitian-descriptors/gitian-osx.yml +++ b/contrib/gitian-descriptors/gitian-osx.yml @@ -110,13 +110,6 @@ script: | SOURCEDIST=$(echo bitcoin-*.tar.gz) DISTNAME=${SOURCEDIST/%.tar.gz} - # Correct tar file order - mkdir -p temp - pushd temp - tar -xf ../$SOURCEDIST - find bitcoin-* | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST - popd - # Workaround for tarball not building with the bare tag version (prep) make -C src obj/build.h @@ -166,6 +159,8 @@ script: | find ${DISTNAME} | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}.tar.gz cd ../../ done - mkdir -p $OUTDIR/src - mv $SOURCEDIST $OUTDIR/src + + mkdir -p ${OUTDIR}/src + git archive --output=${OUTDIR}/src/${DISTNAME}.tar.gz HEAD + mv ${OUTDIR}/${DISTNAME}-x86_64-*.tar.gz ${OUTDIR}/${DISTNAME}-osx64.tar.gz diff --git a/contrib/gitian-descriptors/gitian-win.yml b/contrib/gitian-descriptors/gitian-win.yml index a69439369e..c5eea97c77 100644 --- a/contrib/gitian-descriptors/gitian-win.yml +++ b/contrib/gitian-descriptors/gitian-win.yml @@ -117,13 +117,6 @@ script: | SOURCEDIST=$(echo bitcoin-*.tar.gz) DISTNAME=${SOURCEDIST/%.tar.gz} - # Correct tar file order - mkdir -p temp - pushd temp - tar -xf ../$SOURCEDIST - find bitcoin-* | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST - popd - # Workaround for tarball not building with the bare tag version (prep) make -C src obj/build.h @@ -145,6 +138,7 @@ script: | CONFIG_SITE=${BASEPREFIX}/${i}/share/config.site ./configure --prefix=/ --disable-ccache --disable-maintainer-mode --disable-dependency-tracking ${CONFIGFLAGS} CFLAGS="${HOST_CFLAGS}" CXXFLAGS="${HOST_CXXFLAGS}" make ${MAKEOPTS} make ${MAKEOPTS} -C src check-security + make ${MAKEOPTS} -C src check-symbols make deploy make install DESTDIR=${INSTALLPATH} cp -f --target-directory="${OUTDIR}" ./bitcoin-*-setup-unsigned.exe @@ -161,8 +155,10 @@ script: | cd ../../ rm -rf distsrc-${i} done - mkdir -p $OUTDIR/src - mv $SOURCEDIST $OUTDIR/src + + mkdir -p ${OUTDIR}/src + git archive --output=${OUTDIR}/src/${DISTNAME}.tar.gz HEAD + cp -rf contrib/windeploy $BUILD_DIR cd $BUILD_DIR/windeploy mkdir unsigned diff --git a/contrib/windeploy/win-codesign.cert b/contrib/windeploy/win-codesign.cert index 5bc5dc5809..4023a5b638 100644 --- a/contrib/windeploy/win-codesign.cert +++ b/contrib/windeploy/win-codesign.cert @@ -1,34 +1,34 @@ -----BEGIN CERTIFICATE----- -MIIFcTCCBFmgAwIBAgIRALWcUnSOxv9FQW3xdaMDO6swDQYJKoZIhvcNAQELBQAw +MIIFdDCCBFygAwIBAgIRAL98pqZb/N9LuNaNxKsHNGQwDQYJKoZIhvcNAQELBQAw fDELMAkGA1UEBhMCR0IxGzAZBgNVBAgTEkdyZWF0ZXIgTWFuY2hlc3RlcjEQMA4G A1UEBxMHU2FsZm9yZDEYMBYGA1UEChMPU2VjdGlnbyBMaW1pdGVkMSQwIgYDVQQD -ExtTZWN0aWdvIFJTQSBDb2RlIFNpZ25pbmcgQ0EwHhcNMTkwMzI3MDAwMDAwWhcN -MjAwMzI2MjM1OTU5WjCBtDELMAkGA1UEBhMCQ0gxDTALBgNVBBEMBDgwMDUxCzAJ -BgNVBAgMAlpIMRAwDgYDVQQHDAdaw7xyaWNoMRcwFQYDVQQJDA5NYXR0ZW5nYXNz -ZSAyNzEuMCwGA1UECgwlQml0Y29pbiBDb3JlIENvZGUgU2lnbmluZyBBc3NvY2lh -dGlvbjEuMCwGA1UEAwwlQml0Y29pbiBDb3JlIENvZGUgU2lnbmluZyBBc3NvY2lh -dGlvbjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAK04VDwiY1wxcW3E -WTTGmnbciCwETwC96DG4qcoH2PPNsVy3dfwGh0C02Qj2vL64IfwIGUFSgREvyjZk -CNhEuJO2e0nO0rKNNH5v/JO+P7/VYPZkF5a3uUz9ulmihULXioieHB/q0l6BmiJL -+cYaMVfidL9Y+IJwgiTqjnpRhv1Ik083SPsu6GcfQT9MJfY/+xse2EP0l4GfdFE6 -DRcWjiC8UHpfpGYcImzSFZZpbFbqoAyhueCl28QU4f8QAbS6BqNfaAK9MMACWDcK -eTz3C5JK6CiUxOnGIxilXhljuybFUjR4jGl5eTRpuPWk95NTTYS36q+bx/1nYelx -0n4nnDMCAwEAAaOCAbMwggGvMB8GA1UdIwQYMBaAFA7hOqhTOjHVir7Bu61nGgOF -rTQOMB0GA1UdDgQWBBRbN7ECrPCdVvh58enwy3Dix46h2jAOBgNVHQ8BAf8EBAMC -B4AwDAYDVR0TAQH/BAIwADATBgNVHSUEDDAKBggrBgEFBQcDAzARBglghkgBhvhC -AQEEBAMCBBAwQAYDVR0gBDkwNzA1BgwrBgEEAbIxAQIBAwIwJTAjBggrBgEFBQcC -ARYXaHR0cHM6Ly9zZWN0aWdvLmNvbS9DUFMwQwYDVR0fBDwwOjA4oDagNIYyaHR0 -cDovL2NybC5zZWN0aWdvLmNvbS9TZWN0aWdvUlNBQ29kZVNpZ25pbmdDQS5jcmww -cwYIKwYBBQUHAQEEZzBlMD4GCCsGAQUFBzAChjJodHRwOi8vY3J0LnNlY3RpZ28u -Y29tL1NlY3RpZ29SU0FDb2RlU2lnbmluZ0NBLmNydDAjBggrBgEFBQcwAYYXaHR0 -cDovL29jc3Auc2VjdGlnby5jb20wKwYDVR0RBCQwIoEgam9uYXNAYml0Y29pbmNv -cmVjb2Rlc2lnbmluZy5vcmcwDQYJKoZIhvcNAQELBQADggEBAF/AIXcFBWCC2Red -SHN4Cvko5mdSkDNgzjVFc+OwAJ5RdOgbERde4PnHm3Qmrnx+uMetVnmrC8Fv1Iwb -kkR0bdbWBj6lF6zMsClIN6WJEfY+qfj1qi7wyucu+3OElYRC9bm5Lf0mEHQr8lJ1 -lGvAjPh+/hmxoVNbHFMZ1Ea+BrbjVwiSznt0gzdMh0CispBZKLWCIwRwi+hFjQrw -Z7RLH8HeCJ5Ojl/OTDQqh6AylQ7l9w9KHsUt4Jqy/AnCCyAj2/6xjdwnuo3tCZwb -g/9CydiAacD/83odphEeC2iBa+0wsj9bWmyYKY7S9n0u+wm3wBfZbSVMDDPk/la1 -3qCUDLk= +ExtTZWN0aWdvIFJTQSBDb2RlIFNpZ25pbmcgQ0EwHhcNMjAwMzI0MDAwMDAwWhcN +MjEwMzI0MjM1OTU5WjCBtzELMAkGA1UEBhMCQ0gxDTALBgNVBBEMBDgwMDUxDjAM +BgNVBAgMBVN0YXRlMRAwDgYDVQQHDAdaw7xyaWNoMRcwFQYDVQQJDA5NYXR0ZW5n +YXNzZSAyNzEuMCwGA1UECgwlQml0Y29pbiBDb3JlIENvZGUgU2lnbmluZyBBc3Nv +Y2lhdGlvbjEuMCwGA1UEAwwlQml0Y29pbiBDb3JlIENvZGUgU2lnbmluZyBBc3Nv +Y2lhdGlvbjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMtxC8N4r/jE +OGOdFy/0UtiUvEczPZf9WYZz/7paAkc75XopHIE5/ssmoEX27gG9K00tf3Q62QAx +inZUPWkNTh8X0l+6uSGiIBFIV7dDgztIxnPcxaqw0k7Q2TEqKJvb5qm16zX6WfXJ +R2r6O5utUdQ3AarHnQq9fwdM1j5+ywS5u52te74ENgDMTMKUuB2J3KH1ASg5PAtO +CjPqPL+ZXJ7eT3M0Z+Lbu5ISZSqZB48BcCwOo/fOO0dAiLT9FE1iVtaCpBKHqGmd +glRjPzZdgDv8g28etRmk8wQ5pQmfL2gBjt/LtIgMPTdHHETKLxJO5H3y0CNx1vzL +ql7xNMxELxkCAwEAAaOCAbMwggGvMB8GA1UdIwQYMBaAFA7hOqhTOjHVir7Bu61n +GgOFrTQOMB0GA1UdDgQWBBSHBbl82FUJiUkXyyYJog1awYRsxjAOBgNVHQ8BAf8E +BAMCB4AwDAYDVR0TAQH/BAIwADATBgNVHSUEDDAKBggrBgEFBQcDAzARBglghkgB +hvhCAQEEBAMCBBAwQAYDVR0gBDkwNzA1BgwrBgEEAbIxAQIBAwIwJTAjBggrBgEF +BQcCARYXaHR0cHM6Ly9zZWN0aWdvLmNvbS9DUFMwQwYDVR0fBDwwOjA4oDagNIYy +aHR0cDovL2NybC5zZWN0aWdvLmNvbS9TZWN0aWdvUlNBQ29kZVNpZ25pbmdDQS5j +cmwwcwYIKwYBBQUHAQEEZzBlMD4GCCsGAQUFBzAChjJodHRwOi8vY3J0LnNlY3Rp +Z28uY29tL1NlY3RpZ29SU0FDb2RlU2lnbmluZ0NBLmNydDAjBggrBgEFBQcwAYYX +aHR0cDovL29jc3Auc2VjdGlnby5jb20wKwYDVR0RBCQwIoEgam9uYXNAYml0Y29p +bmNvcmVjb2Rlc2lnbmluZy5vcmcwDQYJKoZIhvcNAQELBQADggEBAAU59qJzQ2ED +aTMIQTsU01zIhZJ/xwQh78i0v2Mnr46RvzYrZOev+btF3SyUYD8veNnbYlY6yEYq +Vb+/PQnE3t1xlqR80qiTZCk/Wmxx/qKvQuWeRL5QQgvsCmWBpycQ7PNfwzOWxbPE +b0Hb2/VFFZfR9iltkfeInRUrzS96CJGYtm7dMf2JtnXYBcwpn1N8BSMH4nXVyN8g +VEE5KyjE7+/awYiSST7+e6Y7FE5AJ4f3FjqnRm+2XetTVqITwMLKZMoV283nSEeH +fA4FNAMGz9QeV38ol65NNqFP2vSSgVoPK79orqH9OOW2LSobt2qun+euddJIQeYV +CMP90b/2WPc= -----END CERTIFICATE----- -----BEGIN CERTIFICATE----- MIIF3jCCA8agAwIBAgIQAf1tMPyjylGoG7xkDjUDLTANBgkqhkiG9w0BAQwFADCB diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 6e8bde47f8..da07080724 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -43,6 +43,7 @@ Developer Notes - [Suggestions and examples](#suggestions-and-examples) - [Release notes](#release-notes) - [RPC interface guidelines](#rpc-interface-guidelines) + - [Internal interface guidelines](#internal-interface-guidelines) <!-- markdown-toc end --> @@ -1100,3 +1101,124 @@ A few guidelines for introducing and reviewing new RPC interfaces: timestamps in the documentation. - *Rationale*: User-facing consistency. + +Internal interface guidelines +----------------------------- + +Internal interfaces between parts of the codebase that are meant to be +independent (node, wallet, GUI), are defined in +[`src/interfaces/`](../src/interfaces/). The main interface classes defined +there are [`interfaces::Chain`](../src/interfaces/chain.h), used by wallet to +access the node's latest chain state, +[`interfaces::Node`](../src/interfaces/node.h), used by the GUI to control the +node, and [`interfaces::Wallet`](../src/interfaces/wallet.h), used by the GUI +to control an individual wallet. There are also more specialized interface +types like [`interfaces::Handler`](../src/interfaces/handler.h) +[`interfaces::ChainClient`](../src/interfaces/chain.h) passed to and from +various interface methods. + +Interface classes are written in a particular style so node, wallet, and GUI +code doesn't need to run in the same process, and so the class declarations +work more easily with tools and libraries supporting interprocess +communication: + +- Interface classes should be abstract and have methods that are [pure + virtual](https://en.cppreference.com/w/cpp/language/abstract_class). This + allows multiple implementations to inherit from the same interface class, + particularly so one implementation can execute functionality in the local + process, and other implementations can forward calls to remote processes. + +- Interface method definitions should wrap existing functionality instead of + implementing new functionality. Any substantial new node or wallet + functionality should be implemented in [`src/node/`](../src/node/) or + [`src/wallet/`](../src/wallet/) and just exposed in + [`src/interfaces/`](../src/interfaces/) instead of being implemented there, + so it can be more modular and accessible to unit tests. + +- Interface method parameter and return types should either be serializable or + be other interface classes. Interface methods shouldn't pass references to + objects that can't be serialized or accessed from another process. + + Examples: + + ```c++ + // Good: takes string argument and returns interface class pointer + virtual unique_ptr<interfaces::Wallet> loadWallet(std::string filename) = 0; + + // Bad: returns CWallet reference that can't be used from another process + virtual CWallet& loadWallet(std::string filename) = 0; + ``` + + ```c++ + // Good: accepts and returns primitive types + virtual bool findBlock(const uint256& hash, int& out_height, int64_t& out_time) = 0; + + // Bad: returns pointer to internal node in a linked list inaccessible to + // other processes + virtual const CBlockIndex* findBlock(const uint256& hash) = 0; + ``` + + ```c++ + // Good: takes plain callback type and returns interface pointer + using TipChangedFn = std::function<void(int block_height, int64_t block_time)>; + virtual std::unique_ptr<interfaces::Handler> handleTipChanged(TipChangedFn fn) = 0; + + // Bad: returns boost connection specific to local process + using TipChangedFn = std::function<void(int block_height, int64_t block_time)>; + virtual boost::signals2::scoped_connection connectTipChanged(TipChangedFn fn) = 0; + ``` + +- For consistency and friendliness to code generation tools, interface method + input and inout parameters should be ordered first and output parameters + should come last. + + Example: + + ```c++ + // Good: error output param is last + virtual bool broadcastTransaction(const CTransactionRef& tx, CAmount max_fee, std::string& error) = 0; + + // Bad: error output param is between input params + virtual bool broadcastTransaction(const CTransactionRef& tx, std::string& error, CAmount max_fee) = 0; + ``` + +- For friendliness to code generation tools, interface methods should not be + overloaded: + + Example: + + ```c++ + // Good: method names are unique + virtual bool disconnectByAddress(const CNetAddr& net_addr) = 0; + virtual bool disconnectById(NodeId id) = 0; + + // Bad: methods are overloaded by type + virtual bool disconnect(const CNetAddr& net_addr) = 0; + virtual bool disconnect(NodeId id) = 0; + ``` + +- For consistency and friendliness to code generation tools, interface method + names should be `lowerCamelCase` and standalone function names should be + `UpperCamelCase`. + + Examples: + + ```c++ + // Good: lowerCamelCase method name + virtual void blockConnected(const CBlock& block, int height) = 0; + + // Bad: uppercase class method + virtual void BlockConnected(const CBlock& block, int height) = 0; + ``` + + ```c++ + // Good: UpperCamelCase standalone function name + std::unique_ptr<Node> MakeNode(LocalInit& init); + + // Bad: lowercase standalone function + std::unique_ptr<Node> makeNode(LocalInit& init); + ``` + + Note: This last convention isn't generally followed outside of + [`src/interfaces/`](../src/interfaces/), though it did come up for discussion + before in [#14635](https://github.com/bitcoin/bitcoin/pull/14635). diff --git a/doc/fuzzing.md b/doc/fuzzing.md index af82371d58..9642337821 100644 --- a/doc/fuzzing.md +++ b/doc/fuzzing.md @@ -1,125 +1,93 @@ -Fuzz-testing Bitcoin Core -========================== - -A special test harness in `src/test/fuzz/` is provided for each fuzz target to -provide an easy entry point for fuzzers and the like. In this document we'll -describe how to use it with AFL and libFuzzer. - -## Preparing fuzzing - -The fuzzer needs some inputs to work on, but the inputs or seeds can be used -interchangeably between libFuzzer and AFL. - -Extract the example seeds (or other starting inputs) into the inputs -directory before starting fuzzing. - -``` -git clone https://github.com/bitcoin-core/qa-assets -export DIR_FUZZ_IN=$PWD/qa-assets/fuzz_seed_corpus -``` - -AFL needs an input directory with examples, and an output directory where it -will place examples that it found. These can be anywhere in the file system, -we'll define environment variables to make it easy to reference them. - -So, only for AFL you need to configure the outputs path: - -``` -mkdir outputs -export AFLOUT=$PWD/outputs -``` - -libFuzzer will use the input directory as output directory. - -## AFL - -### Building AFL - -It is recommended to always use the latest version of afl: -``` -wget http://lcamtuf.coredump.cx/afl/releases/afl-latest.tgz -tar -zxvf afl-latest.tgz -cd afl-<version> -make -export AFLPATH=$PWD -``` - -For macOS you may need to ignore x86 compilation checks when running `make`: -`AFL_NO_X86=1 make`. - -### Instrumentation - -To build Bitcoin Core using AFL instrumentation (this assumes that the -`AFLPATH` was set as above): -``` -./configure --disable-shared --enable-tests --enable-fuzz CC=${AFLPATH}/afl-gcc CXX=${AFLPATH}/afl-g++ -export AFL_HARDEN=1 -make -``` - -If you are using clang you will need to substitute `afl-gcc` with `afl-clang` -and `afl-g++` with `afl-clang++`, so the first line above becomes: -``` -./configure --disable-shared --enable-tests --enable-fuzz CC=${AFLPATH}/afl-clang CXX=${AFLPATH}/afl-clang++ -``` - -We disable ccache because we don't want to pollute the ccache with instrumented -objects, and similarly don't want to use non-instrumented cached objects linked -in. - -The fuzzing can be sped up significantly (~200x) by using `afl-clang-fast` and -`afl-clang-fast++` in place of `afl-gcc` and `afl-g++` when compiling. When -compiling using `afl-clang-fast`/`afl-clang-fast++` the resulting -binary will be instrumented in such a way that the AFL -features "persistent mode" and "deferred forkserver" can be used. See -https://github.com/google/AFL/tree/master/llvm_mode for details. - -### Fuzzing - -To start the actual fuzzing use: - -``` -export FUZZ_TARGET=bech32 # Pick a fuzz_target -mkdir ${AFLOUT}/${FUZZ_TARGET} -$AFLPATH/afl-fuzz -i ${DIR_FUZZ_IN}/${FUZZ_TARGET} -o ${AFLOUT}/${FUZZ_TARGET} -m52 -- src/test/fuzz/${FUZZ_TARGET} -``` - -You may have to change a few kernel parameters to test optimally - `afl-fuzz` -will print an error and suggestion if so. - -On macOS you may need to set `AFL_NO_FORKSRV=1` to get the target to run. -``` -export FUZZ_TARGET=bech32 # Pick a fuzz_target -mkdir ${AFLOUT}/${FUZZ_TARGET} -AFL_NO_FORKSRV=1 $AFLPATH/afl-fuzz -i ${DIR_FUZZ_IN}/${FUZZ_TARGET} -o ${AFLOUT}/${FUZZ_TARGET} -m52 -- src/test/fuzz/${FUZZ_TARGET} -``` - -## libFuzzer - -A recent version of `clang`, the address/undefined sanitizers (ASan/UBSan) and -libFuzzer is needed (all found in the `compiler-rt` runtime libraries package). - -To build all fuzz targets with libFuzzer, run - -``` -./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=clang CXX=clang++ -make -``` - -See https://llvm.org/docs/LibFuzzer.html#running on how to run the libFuzzer -instrumented executable. - -Alternatively, you can run the script through the fuzzing test harness (only -libFuzzer supported so far). You need to pass it the inputs directory and -the specific test target you want to run. - -``` -./test/fuzz/test_runner.py ${DIR_FUZZ_IN} bech32 -``` - -### macOS hints for libFuzzer - -The default clang/llvm version supplied by Apple on macOS does not include +# Fuzzing Bitcoin Core using libFuzzer + +## Quickstart guide + +To quickly get started fuzzing Bitcoin Core using [libFuzzer](https://llvm.org/docs/LibFuzzer.html): + +```sh +$ git clone https://github.com/bitcoin/bitcoin +$ cd bitcoin/ +$ ./autogen.sh +$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined +# macOS users: If you have problem with this step then make sure to read "macOS hints for +# libFuzzer" on https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md#macos-hints-for-libfuzzer +$ make +$ src/test/fuzz/process_message +# abort fuzzing using ctrl-c +``` + +## Fuzzing harnesses, fuzzing output and fuzzing corpora + +[`process_message`](https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/process_message.cpp) is a fuzzing harness for the [`ProcessMessage(...)` function (`net_processing`)](https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp). The available fuzzing harnesses are found in [`src/test/fuzz/`](https://github.com/bitcoin/bitcoin/tree/master/src/test/fuzz). + +The fuzzer will output `NEW` every time it has created a test input that covers new areas of the code under test. For more information on how to interpret the fuzzer output, see the [libFuzzer documentation](https://llvm.org/docs/LibFuzzer.html). + +If you specify a corpus directory then any new coverage increasing inputs will be saved there: + +```sh +$ mkdir -p process_message-seeded-from-thin-air/ +$ src/test/fuzz/process_message process_message-seeded-from-thin-air/ +INFO: Seed: 840522292 +INFO: Loaded 1 modules (424174 inline 8-bit counters): 424174 [0x55e121ef9ab8, 0x55e121f613a6), +INFO: Loaded 1 PC tables (424174 PCs): 424174 [0x55e121f613a8,0x55e1225da288), +INFO: 0 files found in process_message-seeded-from-thin-air/ +INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes +INFO: A corpus is not provided, starting from an empty corpus +#2 INITED cov: 94 ft: 95 corp: 1/1b exec/s: 0 rss: 150Mb +#3 NEW cov: 95 ft: 96 corp: 2/3b lim: 4 exec/s: 0 rss: 150Mb L: 2/2 MS: 1 InsertByte- +#4 NEW cov: 96 ft: 98 corp: 3/7b lim: 4 exec/s: 0 rss: 150Mb L: 4/4 MS: 1 CrossOver- +#21 NEW cov: 96 ft: 100 corp: 4/11b lim: 4 exec/s: 0 rss: 150Mb L: 4/4 MS: 2 ChangeBit-CrossOver- +#324 NEW cov: 101 ft: 105 corp: 5/12b lim: 6 exec/s: 0 rss: 150Mb L: 6/6 MS: 5 CrossOver-ChangeBit-CopyPart-ChangeBit-ChangeBinInt- +#1239 REDUCE cov: 102 ft: 106 corp: 6/24b lim: 14 exec/s: 0 rss: 150Mb L: 13/13 MS: 5 ChangeBit-CrossOver-EraseBytes-ChangeBit-InsertRepeatedBytes- +#1272 REDUCE cov: 102 ft: 106 corp: 6/23b lim: 14 exec/s: 0 rss: 150Mb L: 12/12 MS: 3 ChangeBinInt-ChangeBit-EraseBytes- + NEW_FUNC[1/677]: 0x55e11f456690 in std::_Function_base::~_Function_base() /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/std_function.h:255 + NEW_FUNC[2/677]: 0x55e11f465800 in CDataStream::CDataStream(std::vector<unsigned char, std::allocator<unsigned char> > const&, int, int) src/./streams.h:248 +#2125 REDUCE cov: 4820 ft: 4867 corp: 7/29b lim: 21 exec/s: 0 rss: 155Mb L: 6/12 MS: 2 CopyPart-CMP- DE: "block"- + NEW_FUNC[1/9]: 0x55e11f64d790 in std::_Rb_tree<uint256, std::pair<uint256 const, std::chrono::duration<long, std::ratio<1l, 1000000l> > >, std::_Select1st<std::pair<uint256 const, std::chrono::duration<long, std::ratio<1l, 1000000l> > > >, std::less<uint256>, std::allocator<std::pair<uint256 const, std::chrono::duration<long, std::ratio<1l, 1000000l> > > > >::~_Rb_tree() /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_tree.h:972 + NEW_FUNC[2/9]: 0x55e11f64d870 in std::_Rb_tree<uint256, std::pair<uint256 const, std::chrono::duration<long, std::ratio<1l, 1000000l> > >, std::_Select1st<std::pair<uint256 const, std::chrono::duration<long, std::ratio<1l, 1000000l> > > >, std::less<uint256>, std::allocator<std::pair<uint256 const, std::chrono::duration<long, std::ratio<1l, 1000000l> > > > >::_M_erase(std::_Rb_tree_node<std::pair<uint256 const, std::chrono::duration<long, std::ratio<1l, 1000000l> > > >*) /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_tree.h:1875 +#2228 NEW cov: 4898 ft: 4971 corp: 8/35b lim: 21 exec/s: 0 rss: 156Mb L: 6/12 MS: 3 EraseBytes-CopyPart-PersAutoDict- DE: "block"- + NEW_FUNC[1/5]: 0x55e11f46df70 in std::enable_if<__and_<std::allocator_traits<zero_after_free_allocator<char> >::__construct_helper<char, unsigned char const&>::type>::value, void>::type std::allocator_traits<zero_after_free_allocator<char> >::_S_construct<char, unsigned char const&>(zero_after_free_allocator<char>&, char*, unsigned char const&) /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/alloc_traits.h:243 + NEW_FUNC[2/5]: 0x55e11f477390 in std::vector<unsigned char, std::allocator<unsigned char> >::data() /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/stl_vector.h:1056 +#2456 NEW cov: 4933 ft: 5042 corp: 9/55b lim: 21 exec/s: 0 rss: 160Mb L: 20/20 MS: 3 ChangeByte-InsertRepeatedBytes-PersAutoDict- DE: "block"- +#2467 NEW cov: 4933 ft: 5043 corp: 10/76b lim: 21 exec/s: 0 rss: 161Mb L: 21/21 MS: 1 InsertByte- +#4215 NEW cov: 4941 ft: 5129 corp: 17/205b lim: 29 exec/s: 4215 rss: 350Mb L: 29/29 MS: 5 InsertByte-ChangeBit-CopyPart-InsertRepeatedBytes-CrossOver- +#4567 REDUCE cov: 4941 ft: 5129 corp: 17/204b lim: 29 exec/s: 4567 rss: 404Mb L: 24/29 MS: 2 ChangeByte-EraseBytes- +#6642 NEW cov: 4941 ft: 5138 corp: 18/244b lim: 43 exec/s: 2214 rss: 450Mb L: 43/43 MS: 3 CopyPart-CMP-CrossOver- DE: "verack"- +# abort fuzzing using ctrl-c +$ ls process_message-seeded-from-thin-air/ +349ac589fc66a09abc0b72bb4ae445a7a19e2cd8 4df479f1f421f2ea64b383cd4919a272604087a7 +a640312c98dcc55d6744730c33e41c5168c55f09 b135de16e4709558c0797c15f86046d31c5d86d7 +c000f7b41b05139de8b63f4cbf7d1ad4c6e2aa7f fc52cc00ec1eb1c08470e69f809ae4993fa70082 +$ cat --show-nonprinting process_message-seeded-from-thin-air/349ac589fc66a09abc0b72bb4ae445a7a19e2cd8 +block^@M-^?M-^?M-^?M-^?M-^?nM-^?M-^? +``` + +In this case the fuzzer managed to create a `block` message which when passed to `ProcessMessage(...)` increased coverage. + +The project's collection of seed corpora is found in the [`bitcoin-core/qa-assets`](https://github.com/bitcoin-core/qa-assets) repo. + +To fuzz `process_message` using the [`bitcoin-core/qa-assets`](https://github.com/bitcoin-core/qa-assets) seed corpus: + +```sh +$ git clone https://github.com/bitcoin-core/qa-assets +$ src/test/fuzz/process_message qa-assets/fuzz_seed_corpus/process_message/ +INFO: Seed: 1346407872 +INFO: Loaded 1 modules (424174 inline 8-bit counters): 424174 [0x55d8a9004ab8, 0x55d8a906c3a6), +INFO: Loaded 1 PC tables (424174 PCs): 424174 [0x55d8a906c3a8,0x55d8a96e5288), +INFO: 991 files found in qa-assets/fuzz_seed_corpus/process_message/ +INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes +INFO: seed corpus: files: 991 min: 1b max: 1858b total: 288291b rss: 150Mb +#993 INITED cov: 7063 ft: 8236 corp: 25/3821b exec/s: 0 rss: 181Mb +… +``` + +If you find coverage increasing inputs when fuzzing you are highly encouraged to submit them for inclusion in the [`bitcoin-core/qa-assets`](https://github.com/bitcoin-core/qa-assets) repo. + +Every single pull request submitted against the Bitcoin Core repo is automatically tested against all inputs in the [`bitcoin-core/qa-assets`](https://github.com/bitcoin-core/qa-assets) repo. Contributing new coverage increasing inputs is an easy way to help make Bitcoin Core more robust. + +## macOS hints for libFuzzer + +The default Clang/LLVM version supplied by Apple on macOS does not include fuzzing libraries, so macOS users will need to install a full version, for example using `brew install llvm`. @@ -128,11 +96,40 @@ may need to run `./configure` with `--disable-asm` to avoid errors with certain assembly code from Bitcoin Core's code. See [developer notes on sanitizers](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#sanitizers) for more information. -You may also need to take care of giving the correct path for clang and -clang++, like `CC=/path/to/clang CXX=/path/to/clang++` if the non-systems -clang does not come first in your path. +You may also need to take care of giving the correct path for `clang` and +`clang++`, like `CC=/path/to/clang CXX=/path/to/clang++` if the non-systems +`clang` does not come first in your path. Full configure that was tested on macOS Catalina with `brew` installed `llvm`: -``` + +```sh ./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ --disable-asm ``` + +Read the [libFuzzer documentation](https://llvm.org/docs/LibFuzzer.html) for more information. This [libFuzzer tutorial](https://github.com/google/fuzzing/blob/master/tutorial/libFuzzerTutorial.md) might also be of interest. + +# Fuzzing Bitcoin Core using american fuzzy lop (`afl-fuzz`) + +## Quickstart guide + +To quickly get started fuzzing Bitcoin Core using [`afl-fuzz`](https://github.com/google/afl): + +```sh +$ git clone https://github.com/bitcoin/bitcoin +$ cd bitcoin/ +$ git clone https://github.com/google/afl +$ make -C afl/ +$ make -C afl/llvm_mode/ +$ ./autogen.sh +$ CC=$(pwd)/afl/afl-clang-fast CXX=$(pwd)/afl/afl-clang-fast++ ./configure --enable-fuzz +$ make +# For macOS you may need to ignore x86 compilation checks when running "make". If so, +# try compiling using: AFL_NO_X86=1 make +$ mkdir -p inputs/ outputs/ +$ echo A > inputs/thin-air-input +$ afl/afl-fuzz -i inputs/ -o outputs/ -- src/test/fuzz/bech32 +# You may have to change a few kernel parameters to test optimally - afl-fuzz +# will print an error and suggestion if so. +``` + +Read the [`afl-fuzz` documentation](https://github.com/google/afl) for more information. diff --git a/src/Makefile.am b/src/Makefile.am index 6fc6d5b5a3..8c927f330b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -703,6 +703,11 @@ if TARGET_DARWIN $(AM_V_at) OTOOL=$(OTOOL) $(PYTHON) $(top_srcdir)/contrib/devtools/symbol-check.py $(bin_PROGRAMS) endif +if TARGET_WINDOWS + @echo "Checking Windows dynamic libraries..." + $(AM_V_at) OBJDUMP=$(OBJDUMP) $(PYTHON) $(top_srcdir)/contrib/devtools/symbol-check.py $(bin_PROGRAMS) +endif + if GLIBC_BACK_COMPAT @echo "Checking glibc back compat..." $(AM_V_at) READELF=$(READELF) CPPFILT=$(CPPFILT) $(PYTHON) $(top_srcdir)/contrib/devtools/symbol-check.py $(bin_PROGRAMS) diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 1c97e22de8..eae8b1fcd1 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -43,12 +43,11 @@ bench_bench_bitcoin_SOURCES = \ nodist_bench_bench_bitcoin_SOURCES = $(GENERATED_BENCH_FILES) -bench_bench_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(EVENT_CLFAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/ +bench_bench_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/ bench_bench_bitcoin_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) bench_bench_bitcoin_LDADD = \ $(LIBBITCOIN_SERVER) \ $(LIBBITCOIN_WALLET) \ - $(LIBBITCOIN_SERVER) \ $(LIBBITCOIN_COMMON) \ $(LIBBITCOIN_UTIL) \ $(LIBBITCOIN_CONSENSUS) \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index e92b02a9bc..2938ccdc9f 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -4,6 +4,7 @@ FUZZ_TARGETS = \ test/fuzz/addr_info_deserialize \ + test/fuzz/addrdb \ test/fuzz/address_deserialize \ test/fuzz/addrman_deserialize \ test/fuzz/asmap \ @@ -16,6 +17,7 @@ FUZZ_TARGETS = \ test/fuzz/block_filter_deserialize \ test/fuzz/block_header \ test/fuzz/block_header_and_short_txids_deserialize \ + test/fuzz/blockfilter \ test/fuzz/blockheader_deserialize \ test/fuzz/blocklocator_deserialize \ test/fuzz/blockmerkleroot \ @@ -24,6 +26,7 @@ FUZZ_TARGETS = \ test/fuzz/blockundo_deserialize \ test/fuzz/bloom_filter \ test/fuzz/bloomfilter_deserialize \ + test/fuzz/chain \ test/fuzz/coins_deserialize \ test/fuzz/decode_tx \ test/fuzz/descriptor_parse \ @@ -43,6 +46,7 @@ FUZZ_TARGETS = \ test/fuzz/merkle_block_deserialize \ test/fuzz/messageheader_deserialize \ test/fuzz/multiplication_overflow \ + test/fuzz/net_permissions \ test/fuzz/netaddr_deserialize \ test/fuzz/netaddress \ test/fuzz/out_point_deserialize \ @@ -80,6 +84,7 @@ FUZZ_TARGETS = \ test/fuzz/process_message_tx \ test/fuzz/process_message_verack \ test/fuzz/process_message_version \ + test/fuzz/protocol \ test/fuzz/psbt \ test/fuzz/psbt_input_deserialize \ test/fuzz/psbt_output_deserialize \ @@ -97,6 +102,7 @@ FUZZ_TARGETS = \ test/fuzz/string \ test/fuzz/strprintf \ test/fuzz/sub_net_deserialize \ + test/fuzz/timedata \ test/fuzz/transaction \ test/fuzz/tx_in \ test/fuzz/tx_in_deserialize \ @@ -288,6 +294,12 @@ test_fuzz_addr_info_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_addr_info_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_addr_info_deserialize_SOURCES = $(FUZZ_SUITE) test/fuzz/deserialize.cpp +test_fuzz_addrdb_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_addrdb_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_addrdb_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_addrdb_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_addrdb_SOURCES = $(FUZZ_SUITE) test/fuzz/addrdb.cpp + test_fuzz_address_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DADDRESS_DESERIALIZE=1 test_fuzz_address_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_address_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -360,6 +372,12 @@ test_fuzz_block_header_and_short_txids_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMO test_fuzz_block_header_and_short_txids_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_block_header_and_short_txids_deserialize_SOURCES = $(FUZZ_SUITE) test/fuzz/deserialize.cpp +test_fuzz_blockfilter_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_blockfilter_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_blockfilter_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_blockfilter_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_blockfilter_SOURCES = $(FUZZ_SUITE) test/fuzz/blockfilter.cpp + test_fuzz_blockheader_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DBLOCKHEADER_DESERIALIZE=1 test_fuzz_blockheader_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_blockheader_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -408,6 +426,12 @@ test_fuzz_bloomfilter_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_bloomfilter_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_bloomfilter_deserialize_SOURCES = $(FUZZ_SUITE) test/fuzz/deserialize.cpp +test_fuzz_chain_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_chain_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_chain_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_chain_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_chain_SOURCES = $(FUZZ_SUITE) test/fuzz/chain.cpp + test_fuzz_coins_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DCOINS_DESERIALIZE=1 test_fuzz_coins_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_coins_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -522,6 +546,12 @@ test_fuzz_multiplication_overflow_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_multiplication_overflow_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_multiplication_overflow_SOURCES = $(FUZZ_SUITE) test/fuzz/multiplication_overflow.cpp +test_fuzz_net_permissions_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_net_permissions_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_net_permissions_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_net_permissions_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_net_permissions_SOURCES = $(FUZZ_SUITE) test/fuzz/net_permissions.cpp + test_fuzz_netaddr_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DNETADDR_DESERIALIZE=1 test_fuzz_netaddr_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_netaddr_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -744,6 +774,12 @@ test_fuzz_process_message_version_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_process_message_version_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_process_message_version_SOURCES = $(FUZZ_SUITE) test/fuzz/process_message.cpp +test_fuzz_protocol_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_protocol_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_protocol_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_protocol_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_protocol_SOURCES = $(FUZZ_SUITE) test/fuzz/protocol.cpp + test_fuzz_psbt_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_psbt_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_psbt_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -846,6 +882,12 @@ test_fuzz_sub_net_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_sub_net_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_sub_net_deserialize_SOURCES = $(FUZZ_SUITE) test/fuzz/deserialize.cpp +test_fuzz_timedata_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_timedata_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_timedata_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_timedata_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_timedata_SOURCES = $(FUZZ_SUITE) test/fuzz/timedata.cpp + test_fuzz_transaction_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_transaction_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_transaction_LDADD = $(FUZZ_SUITE_LD_COMMON) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 735f55fba7..b4b2d7ed52 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -20,6 +20,7 @@ #include <util/moneystr.h> #include <util/rbf.h> #include <util/strencodings.h> +#include <util/string.h> #include <util/system.h> #include <util/translation.h> @@ -357,7 +358,7 @@ static void MutateTxAddOutMultiSig(CMutableTransaction& tx, const std::string& s if (required < 1 || required > MAX_PUBKEYS_PER_MULTISIG || numkeys < 1 || numkeys > MAX_PUBKEYS_PER_MULTISIG || numkeys < required) throw std::runtime_error("multisig parameter mismatch. Required " \ - + std::to_string(required) + " of " + std::to_string(numkeys) + "signatures."); + + ToString(required) + " of " + ToString(numkeys) + "signatures."); // extract and validate PUBKEYs std::vector<CPubKey> pubkeys; diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index d16f4e7ccc..9dc0d37cd9 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -166,25 +166,25 @@ public: } void TransactionAddedToMempool(const CTransactionRef& tx) override { - m_notifications->TransactionAddedToMempool(tx); + m_notifications->transactionAddedToMempool(tx); } void TransactionRemovedFromMempool(const CTransactionRef& tx) override { - m_notifications->TransactionRemovedFromMempool(tx); + m_notifications->transactionRemovedFromMempool(tx); } void BlockConnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* index) override { - m_notifications->BlockConnected(*block, index->nHeight); + m_notifications->blockConnected(*block, index->nHeight); } void BlockDisconnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* index) override { - m_notifications->BlockDisconnected(*block, index->nHeight); + m_notifications->blockDisconnected(*block, index->nHeight); } void UpdatedBlockTip(const CBlockIndex* index, const CBlockIndex* fork_index, bool is_ibd) override { - m_notifications->UpdatedBlockTip(); + m_notifications->updatedBlockTip(); } - void ChainStateFlushed(const CBlockLocator& locator) override { m_notifications->ChainStateFlushed(locator); } + void ChainStateFlushed(const CBlockLocator& locator) override { m_notifications->chainStateFlushed(locator); } Chain& m_chain; Chain::Notifications* m_notifications; }; @@ -278,7 +278,10 @@ public: auto it = ::mempool.GetIter(txid); return it && (*it)->GetCountWithDescendants() > 1; } - bool broadcastTransaction(const CTransactionRef& tx, std::string& err_string, const CAmount& max_tx_fee, bool relay) override + bool broadcastTransaction(const CTransactionRef& tx, + const CAmount& max_tx_fee, + bool relay, + std::string& err_string) override { const TransactionError err = BroadcastTransaction(m_node, tx, err_string, max_tx_fee, relay, /*wait_callback*/ false); // Chain clients only care about failures to accept the tx to the mempool. Disregard non-mempool related failures. @@ -366,7 +369,7 @@ public: { LOCK2(::cs_main, ::mempool.cs); for (const CTxMemPoolEntry& entry : ::mempool.mapTx) { - notifications.TransactionAddedToMempool(entry.GetSharedTx()); + notifications.transactionAddedToMempool(entry.GetSharedTx()); } } NodeContext& m_node; diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 03a600d5a3..caefa87e11 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -154,7 +154,10 @@ public: //! Transaction is added to memory pool, if the transaction fee is below the //! amount specified by max_tx_fee, and broadcast to all peers if relay is set to true. //! Return false if the transaction could not be added due to the fee or for another reason. - virtual bool broadcastTransaction(const CTransactionRef& tx, std::string& err_string, const CAmount& max_tx_fee, bool relay) = 0; + virtual bool broadcastTransaction(const CTransactionRef& tx, + const CAmount& max_tx_fee, + bool relay, + std::string& err_string) = 0; //! Calculate mempool ancestor and descendant counts for the given transaction. virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0; @@ -217,12 +220,12 @@ public: { public: virtual ~Notifications() {} - virtual void TransactionAddedToMempool(const CTransactionRef& tx) {} - virtual void TransactionRemovedFromMempool(const CTransactionRef& ptx) {} - virtual void BlockConnected(const CBlock& block, int height) {} - virtual void BlockDisconnected(const CBlock& block, int height) {} - virtual void UpdatedBlockTip() {} - virtual void ChainStateFlushed(const CBlockLocator& locator) {} + virtual void transactionAddedToMempool(const CTransactionRef& tx) {} + virtual void transactionRemovedFromMempool(const CTransactionRef& ptx) {} + virtual void blockConnected(const CBlock& block, int height) {} + virtual void blockDisconnected(const CBlock& block, int height) {} + virtual void updatedBlockTip() {} + virtual void chainStateFlushed(const CBlockLocator& locator) {} }; //! Register handler for notifications. @@ -245,7 +248,7 @@ public: //! Current RPC serialization flags. virtual int rpcSerializationFlags() = 0; - //! Synchronously send TransactionAddedToMempool notifications about all + //! Synchronously send transactionAddedToMempool notifications about all //! current mempool transactions to the specified handler and return after //! the last one is sent. These notifications aren't coordinated with async //! notifications sent by handleNotifications, so out of date async diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index b720a63017..905173d20b 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -152,14 +152,14 @@ public: } return false; } - bool disconnect(const CNetAddr& net_addr) override + bool disconnectByAddress(const CNetAddr& net_addr) override { if (m_context.connman) { return m_context.connman->DisconnectNode(net_addr); } return false; } - bool disconnect(NodeId id) override + bool disconnectById(NodeId id) override { if (m_context.connman) { return m_context.connman->DisconnectNode(id); @@ -262,12 +262,11 @@ public: { return MakeWallet(LoadWallet(*m_context.chain, name, error, warnings)); } - WalletCreationStatus createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector<std::string>& warnings, std::unique_ptr<Wallet>& result) override + std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector<std::string>& warnings, WalletCreationStatus& status) override { std::shared_ptr<CWallet> wallet; - WalletCreationStatus status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, error, warnings, wallet); - result = MakeWallet(wallet); - return status; + status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, error, warnings, wallet); + return MakeWallet(wallet); } std::unique_ptr<Handler> handleInitMessage(InitMessageFn fn) override { diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 38aeb06324..53a20886cd 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -124,10 +124,10 @@ public: virtual bool unban(const CSubNet& ip) = 0; //! Disconnect node by address. - virtual bool disconnect(const CNetAddr& net_addr) = 0; + virtual bool disconnectByAddress(const CNetAddr& net_addr) = 0; //! Disconnect node by id. - virtual bool disconnect(NodeId id) = 0; + virtual bool disconnectById(NodeId id) = 0; //! Get total bytes recv. virtual int64_t getTotalBytesRecv() = 0; @@ -204,7 +204,7 @@ public: virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, std::string& error, std::vector<std::string>& warnings) = 0; //! Create a wallet from file - virtual WalletCreationStatus createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector<std::string>& warnings, std::unique_ptr<Wallet>& result) = 0; + virtual std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector<std::string>& warnings, WalletCreationStatus& status) = 0; //! Register handler for init messages. using InitMessageFn = std::function<void(const std::string& message)>; diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 01ade56b2a..bb6bb4923f 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -253,19 +253,12 @@ public: } bool createBumpTransaction(const uint256& txid, const CCoinControl& coin_control, - CAmount total_fee, std::vector<std::string>& errors, CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx) override { - if (total_fee > 0) { - return feebumper::CreateTotalBumpTransaction(m_wallet.get(), txid, coin_control, total_fee, errors, old_fee, new_fee, mtx) == - feebumper::Result::OK; - } else { - return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) == - feebumper::Result::OK; - } + return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) == feebumper::Result::OK; } bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); } bool commitBumpTransaction(const uint256& txid, @@ -352,11 +345,11 @@ public: } return {}; } - TransactionError fillPSBT(PartiallySignedTransaction& psbtx, - bool& complete, - int sighash_type = 1 /* SIGHASH_ALL */, - bool sign = true, - bool bip32derivs = false) const override + TransactionError fillPSBT(int sighash_type, + bool sign, + bool bip32derivs, + PartiallySignedTransaction& psbtx, + bool& complete) override { return m_wallet->FillPSBT(psbtx, complete, sighash_type, sign, bip32derivs); } @@ -463,8 +456,8 @@ public: } unsigned int getConfirmTarget() override { return m_wallet->m_confirm_target; } bool hdEnabled() override { return m_wallet->IsHDEnabled(); } - bool canGetAddresses() const override { return m_wallet->CanGetAddresses(); } - bool IsWalletFlagSet(uint64_t flag) override { return m_wallet->IsWalletFlagSet(flag); } + bool canGetAddresses() override { return m_wallet->CanGetAddresses(); } + bool privateKeysDisabled() override { return m_wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); } OutputType getDefaultAddressType() override { return m_wallet->m_default_address_type; } OutputType getDefaultChangeType() override { return m_wallet->m_default_change_type; } CAmount getDefaultMaxTxFee() override { return m_wallet->m_default_max_tx_fee; } diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 9476c9f77f..0e551b0a96 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -155,7 +155,6 @@ public: //! Create bump transaction. virtual bool createBumpTransaction(const uint256& txid, const CCoinControl& coin_control, - CAmount total_fee, std::vector<std::string>& errors, CAmount& old_fee, CAmount& new_fee, @@ -193,11 +192,11 @@ public: int& num_blocks) = 0; //! Fill PSBT. - virtual TransactionError fillPSBT(PartiallySignedTransaction& psbtx, - bool& complete, - int sighash_type = 1 /* SIGHASH_ALL */, - bool sign = true, - bool bip32derivs = false) const = 0; + virtual TransactionError fillPSBT(int sighash_type, + bool sign, + bool bip32derivs, + PartiallySignedTransaction& psbtx, + bool& complete) = 0; //! Get balances. virtual WalletBalances getBalances() = 0; @@ -247,10 +246,10 @@ public: virtual bool hdEnabled() = 0; // Return whether the wallet is blank. - virtual bool canGetAddresses() const = 0; + virtual bool canGetAddresses() = 0; - // check if a certain wallet flag is set. - virtual bool IsWalletFlagSet(uint64_t flag) = 0; + // Return whether private keys enabled. + virtual bool privateKeysDisabled() = 0; // Get default address type. virtual OutputType getDefaultAddressType() = 0; diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 5fab267610..2918676c22 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -1258,7 +1258,7 @@ void BitcoinGUI::updateWalletStatus() } WalletModel * const walletModel = walletView->getWalletModel(); setEncryptionStatus(walletModel->getEncryptionStatus()); - setHDStatus(walletModel->privateKeysDisabled(), walletModel->wallet().hdEnabled()); + setHDStatus(walletModel->wallet().privateKeysDisabled(), walletModel->wallet().hdEnabled()); } #endif // ENABLE_WALLET diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui index ebb6bbd4f5..8b70800838 100644 --- a/src/qt/forms/debugwindow.ui +++ b/src/qt/forms/debugwindow.ui @@ -216,17 +216,17 @@ </widget> </item> <item row="7" column="0"> - <widget class="QLabel" name="labelNetwork"> - <property name="font"> - <font> - <weight>75</weight> - <bold>true</bold> - </font> - </property> - <property name="text"> - <string>Network</string> - </property> - </widget> + <widget class="QLabel" name="labelNetwork"> + <property name="font"> + <font> + <weight>75</weight> + <bold>true</bold> + </font> + </property> + <property name="text"> + <string>Network</string> + </property> + </widget> </item> <item row="8" column="0"> <widget class="QLabel" name="label_8"> @@ -503,12 +503,12 @@ <height>24</height> </size> </property> - <property name="text"> - <string/> - </property> <property name="toolTip"> <string>Decrease font size</string> </property> + <property name="text"> + <string/> + </property> <property name="icon"> <iconset resource="../bitcoin.qrc"> <normaloff>:/icons/fontsmaller</normaloff>:/icons/fontsmaller</iconset> @@ -652,12 +652,12 @@ </item> <item> <widget class="QLineEdit" name="lineEdit"> - <property name="placeholderText"> - <string/> - </property> <property name="enabled"> <bool>false</bool> </property> + <property name="placeholderText"> + <string/> + </property> </widget> </item> </layout> @@ -1503,6 +1503,32 @@ </widget> </item> <item row="18" column="0"> + <widget class="QLabel" name="peerMappedASLabel"> + <property name="toolTip"> + <string>The mapped Autonomous System used for diversifying peer selection.</string> + </property> + <property name="text"> + <string>Mapped AS</string> + </property> + </widget> + </item> + <item row="18" column="1"> + <widget class="QLabel" name="peerMappedAS"> + <property name="cursor"> + <cursorShape>IBeamCursor</cursorShape> + </property> + <property name="text"> + <string>N/A</string> + </property> + <property name="textFormat"> + <enum>Qt::PlainText</enum> + </property> + <property name="textInteractionFlags"> + <set>Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse</set> + </property> + </widget> + </item> + <item row="19" column="0"> <spacer name="verticalSpacer_3"> <property name="orientation"> <enum>Qt::Vertical</enum> diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 977076c4c2..58a7591c95 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -17,6 +17,7 @@ #include <net.h> #include <netbase.h> #include <txdb.h> // for -dbcache defaults +#include <util/string.h> #include <QDebug> #include <QSettings> @@ -241,7 +242,7 @@ void OptionsModel::SetPruneEnabled(bool prune, bool force) QSettings settings; settings.setValue("bPrune", prune); const int64_t prune_target_mib = PruneGBtoMiB(settings.value("nPruneSize").toInt()); - std::string prune_val = prune ? std::to_string(prune_target_mib) : "0"; + std::string prune_val = prune ? ToString(prune_target_mib) : "0"; if (force) { m_node.forceSetArg("-prune", prune_val); return; diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 342c7cce31..c376921b72 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -161,7 +161,7 @@ void OverviewPage::setBalance(const interfaces::WalletBalances& balances) { int unit = walletModel->getOptionsModel()->getDisplayUnit(); m_balances = balances; - if (walletModel->privateKeysDisabled()) { + if (walletModel->wallet().privateKeysDisabled()) { ui->labelBalance->setText(BitcoinUnits::formatWithUnit(unit, balances.watch_only_balance, false, BitcoinUnits::separatorAlways)); ui->labelUnconfirmed->setText(BitcoinUnits::formatWithUnit(unit, balances.unconfirmed_watch_only_balance, false, BitcoinUnits::separatorAlways)); ui->labelImmature->setText(BitcoinUnits::formatWithUnit(unit, balances.immature_watch_only_balance, false, BitcoinUnits::separatorAlways)); @@ -184,7 +184,7 @@ void OverviewPage::setBalance(const interfaces::WalletBalances& balances) // for symmetry reasons also show immature label when the watch-only one is shown ui->labelImmature->setVisible(showImmature || showWatchOnlyImmature); ui->labelImmatureText->setVisible(showImmature || showWatchOnlyImmature); - ui->labelWatchImmature->setVisible(!walletModel->privateKeysDisabled() && showWatchOnlyImmature); // show watch-only immature balance + ui->labelWatchImmature->setVisible(!walletModel->wallet().privateKeysDisabled() && showWatchOnlyImmature); // show watch-only immature balance } // show/hide watch-only labels @@ -236,9 +236,9 @@ void OverviewPage::setWalletModel(WalletModel *model) connect(model->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &OverviewPage::updateDisplayUnit); - updateWatchOnlyLabels(wallet.haveWatchOnly() && !model->privateKeysDisabled()); + updateWatchOnlyLabels(wallet.haveWatchOnly() && !model->wallet().privateKeysDisabled()); connect(model, &WalletModel::notifyWatchonlyChanged, [this](bool showWatchOnly) { - updateWatchOnlyLabels(showWatchOnly && !walletModel->privateKeysDisabled()); + updateWatchOnlyLabels(showWatchOnly && !walletModel->wallet().privateKeysDisabled()); }); } diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp index 16597e4758..180550c5ae 100644 --- a/src/qt/receivecoinsdialog.cpp +++ b/src/qt/receivecoinsdialog.cpp @@ -99,11 +99,11 @@ void ReceiveCoinsDialog::setModel(WalletModel *_model) } // Set the button to be enabled or disabled based on whether the wallet can give out new addresses. - ui->receiveButton->setEnabled(model->canGetAddresses()); + ui->receiveButton->setEnabled(model->wallet().canGetAddresses()); // Enable/disable the receive button if the wallet is now able/unable to give out new addresses. connect(model, &WalletModel::canGetAddressesChanged, [this] { - ui->receiveButton->setEnabled(model->canGetAddresses()); + ui->receiveButton->setEnabled(model->wallet().canGetAddresses()); }); } } diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index b82ab9ffe8..0ffdc892c5 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1113,11 +1113,12 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats) ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStats.m_ping_wait_usec)); ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.m_min_ping_usec)); ui->timeoffset->setText(GUIUtil::formatTimeOffset(stats->nodeStats.nTimeOffset)); - ui->peerVersion->setText(QString("%1").arg(QString::number(stats->nodeStats.nVersion))); + ui->peerVersion->setText(QString::number(stats->nodeStats.nVersion)); ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer)); ui->peerDirection->setText(stats->nodeStats.fInbound ? tr("Inbound") : tr("Outbound")); - ui->peerHeight->setText(QString("%1").arg(QString::number(stats->nodeStats.nStartingHeight))); + ui->peerHeight->setText(QString::number(stats->nodeStats.nStartingHeight)); ui->peerWhitelisted->setText(stats->nodeStats.m_legacyWhitelisted ? tr("Yes") : tr("No")); + ui->peerMappedAS->setText(stats->nodeStats.m_mapped_as != 0 ? QString::number(stats->nodeStats.m_mapped_as) : tr("N/A")); // This check fails for example if the lock was busy and // nodeStateStats couldn't be fetched. @@ -1191,7 +1192,7 @@ void RPCConsole::disconnectSelectedNode() // Get currently selected peer address NodeId id = nodes.at(i).data().toLongLong(); // Find the node, disconnect it and clear the selected node - if(m_node.disconnect(id)) + if(m_node.disconnectById(id)) clearSelectedNode(); } } @@ -1216,7 +1217,7 @@ void RPCConsole::banSelectedNode(int bantime) const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow); if (stats) { m_node.ban(stats->nodeStats.addr, BanReasonManuallyAdded, bantime); - m_node.disconnect(stats->nodeStats.addr); + m_node.disconnectByAddress(stats->nodeStats.addr); } } clearSelectedNode(); diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 4ddee513a1..a8c82aaf6c 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -187,7 +187,7 @@ void SendCoinsDialog::setModel(WalletModel *_model) // set default rbf checkbox state ui->optInRBF->setCheckState(Qt::Checked); - if (model->privateKeysDisabled()) { + if (model->wallet().privateKeysDisabled()) { ui->sendButton->setText(tr("Cr&eate Unsigned")); ui->sendButton->setToolTip(tr("Creates a Partially Signed Bitcoin Transaction (PSBT) for use with e.g. an offline %1 wallet, or a PSBT-compatible hardware wallet.").arg(PACKAGE_NAME)); } @@ -312,14 +312,14 @@ void SendCoinsDialog::on_sendButton_clicked() } QString questionString; - if (model->privateKeysDisabled()) { + if (model->wallet().privateKeysDisabled()) { questionString.append(tr("Do you want to draft this transaction?")); } else { questionString.append(tr("Are you sure you want to send?")); } questionString.append("<br /><span style='font-size:10pt;'>"); - if (model->privateKeysDisabled()) { + if (model->wallet().privateKeysDisabled()) { questionString.append(tr("Please, review your transaction proposal. This will produce a Partially Signed Bitcoin Transaction (PSBT) which you can copy and then sign with e.g. an offline %1 wallet, or a PSBT-compatible hardware wallet.").arg(PACKAGE_NAME)); } else { questionString.append(tr("Please, review your transaction.")); @@ -374,8 +374,8 @@ void SendCoinsDialog::on_sendButton_clicked() } else { questionString = questionString.arg("<br /><br />" + formatted.at(0)); } - const QString confirmation = model->privateKeysDisabled() ? tr("Confirm transaction proposal") : tr("Confirm send coins"); - const QString confirmButtonText = model->privateKeysDisabled() ? tr("Copy PSBT to clipboard") : tr("Send"); + const QString confirmation = model->wallet().privateKeysDisabled() ? tr("Confirm transaction proposal") : tr("Confirm send coins"); + const QString confirmButtonText = model->wallet().privateKeysDisabled() ? tr("Copy PSBT to clipboard") : tr("Send"); SendConfirmationDialog confirmationDialog(confirmation, questionString, informative_text, detailed_text, SEND_CONFIRM_DELAY, confirmButtonText, this); confirmationDialog.exec(); QMessageBox::StandardButton retval = static_cast<QMessageBox::StandardButton>(confirmationDialog.result()); @@ -387,11 +387,11 @@ void SendCoinsDialog::on_sendButton_clicked() } bool send_failure = false; - if (model->privateKeysDisabled()) { + if (model->wallet().privateKeysDisabled()) { CMutableTransaction mtx = CMutableTransaction{*(currentTransaction.getWtx())}; PartiallySignedTransaction psbtx(mtx); bool complete = false; - const TransactionError err = model->wallet().fillPSBT(psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */); + const TransactionError err = model->wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, psbtx, complete); assert(!complete); assert(err == TransactionError::OK); // Serialize the PSBT @@ -562,7 +562,7 @@ void SendCoinsDialog::setBalance(const interfaces::WalletBalances& balances) if(model && model->getOptionsModel()) { CAmount balance = balances.balance; - if (model->privateKeysDisabled()) { + if (model->wallet().privateKeysDisabled()) { balance = balances.watch_only_balance; ui->labelBalanceName->setText(tr("Watch-only balance:")); } @@ -652,7 +652,7 @@ void SendCoinsDialog::useAvailableBalance(SendCoinsEntry* entry) } // Include watch-only for wallets without private key - coin_control.fAllowWatchOnly = model->privateKeysDisabled(); + coin_control.fAllowWatchOnly = model->wallet().privateKeysDisabled(); // Calculate available amount to send. CAmount amount = model->wallet().getAvailableBalance(coin_control); @@ -707,7 +707,7 @@ void SendCoinsDialog::updateCoinControlState(CCoinControl& ctrl) ctrl.m_confirm_target = getConfTargetForIndex(ui->confTargetSelector->currentIndex()); ctrl.m_signal_bip125_rbf = ui->optInRBF->isChecked(); // Include watch-only for wallets without private key - ctrl.fAllowWatchOnly = model->privateKeysDisabled(); + ctrl.fAllowWatchOnly = model->wallet().privateKeysDisabled(); } void SendCoinsDialog::updateSmartFeeLabel() diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 7413a1f09e..233c0ab6be 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -218,8 +218,8 @@ void CreateWalletActivity::createWallet() } QTimer::singleShot(500, worker(), [this, name, flags] { - std::unique_ptr<interfaces::Wallet> wallet; - WalletCreationStatus status = node().createWallet(m_passphrase, flags, name, m_error_message, m_warning_message, wallet); + WalletCreationStatus status; + std::unique_ptr<interfaces::Wallet> wallet = node().createWallet(m_passphrase, flags, name, m_error_message, m_warning_message, status); if (status == WalletCreationStatus::SUCCESS) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet)); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 8a84a8c168..41876f7119 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -23,7 +23,7 @@ #include <ui_interface.h> #include <util/system.h> // for GetBoolArg #include <wallet/coincontrol.h> -#include <wallet/wallet.h> +#include <wallet/wallet.h> // for CRecipient #include <stdint.h> @@ -184,7 +184,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact std::string strFailReason; auto& newTx = transaction.getWtx(); - newTx = m_wallet->createTransaction(vecSend, coinControl, !privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, strFailReason); + newTx = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, strFailReason); transaction.setTransactionFee(nFeeRequired); if (fSubtractFeeFromAmount && newTx) transaction.reassignAmounts(nChangePosRet); @@ -482,13 +482,13 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) CAmount old_fee; CAmount new_fee; CMutableTransaction mtx; - if (!m_wallet->createBumpTransaction(hash, coin_control, 0 /* totalFee */, errors, old_fee, new_fee, mtx)) { + if (!m_wallet->createBumpTransaction(hash, coin_control, errors, old_fee, new_fee, mtx)) { QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Increasing transaction fee failed") + "<br />(" + (errors.size() ? QString::fromStdString(errors[0]) : "") +")"); return false; } - const bool create_psbt = privateKeysDisabled(); + const bool create_psbt = m_wallet->privateKeysDisabled(); // allow a user based fee verification QString questionString = create_psbt ? tr("Do you want to draft a transaction with fee increase?") : tr("Do you want to increase the fee?"); @@ -526,7 +526,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) if (create_psbt) { PartiallySignedTransaction psbtx(mtx); bool complete = false; - const TransactionError err = wallet().fillPSBT(psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */); + const TransactionError err = wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, psbtx, complete); if (err != TransactionError::OK || complete) { QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Can't draft transaction.")); return false; @@ -558,16 +558,6 @@ bool WalletModel::isWalletEnabled() return !gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET); } -bool WalletModel::privateKeysDisabled() const -{ - return m_wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); -} - -bool WalletModel::canGetAddresses() const -{ - return m_wallet->canGetAddresses(); -} - QString WalletModel::getWalletName() const { return QString::fromStdString(m_wallet->getWalletName()); diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 8087356f5e..7936014af9 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -140,8 +140,6 @@ public: bool bumpFee(uint256 hash, uint256& new_hash); static bool isWalletEnabled(); - bool privateKeysDisabled() const; - bool canGetAddresses() const; interfaces::Node& node() const { return m_node; } interfaces::Wallet& wallet() const { return *m_wallet; } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 256876a3d1..d6a45dd9e0 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2178,7 +2178,8 @@ static UniValue getblockfilter(const JSONRPCRequest& request) {RPCResult::Type::STR_HEX, "header", "the hex-encoded filter header"}, }}, RPCExamples{ - HelpExampleCli("getblockfilter", "\"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09\" \"basic\"") + HelpExampleCli("getblockfilter", "\"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09\" \"basic\"") + + HelpExampleRpc("getblockfilter", "\"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09\", \"basic\"") } }.Check(request); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index e26ca1b07a..caa62ca958 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -20,6 +20,7 @@ #include <sync.h> #include <timedata.h> #include <util/strencodings.h> +#include <util/string.h> #include <util/system.h> #include <validation.h> #include <version.h> @@ -712,7 +713,7 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request) RPCHelpMan{"getnodeaddresses", "\nReturn known addresses which can potentially be used to find new nodes in the network\n", { - {"count", RPCArg::Type::NUM, /* default */ "1", "How many addresses to return. Limited to the smaller of " + std::to_string(ADDRMAN_GETADDR_MAX) + " or " + std::to_string(ADDRMAN_GETADDR_MAX_PCT) + "% of all known addresses."}, + {"count", RPCArg::Type::NUM, /* default */ "1", "How many addresses to return. Limited to the smaller of " + ToString(ADDRMAN_GETADDR_MAX) + " or " + ToString(ADDRMAN_GETADDR_MAX_PCT) + "% of all known addresses."}, }, RPCResult{ RPCResult::Type::ARR, "", "", diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 37e515ddfe..ae3f15cec2 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -30,6 +30,7 @@ #include <uint256.h> #include <util/moneystr.h> #include <util/strencodings.h> +#include <util/string.h> #include <validation.h> #include <validationinterface.h> @@ -960,7 +961,7 @@ static std::string WriteHDKeypath(std::vector<uint32_t>& keypath) num &= ~0x80000000; } - keypath_str += std::to_string(num); + keypath_str += ToString(num); if (hardened) { keypath_str += "'"; } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 1a79b3d37e..4ba84d2515 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -489,7 +489,7 @@ std::string RPCHelpMan::ToString() const if (i == 0) ret += "\nArguments:\n"; // Push named argument name and description - sections.m_sections.emplace_back(std::to_string(i + 1) + ". " + arg.m_name, arg.ToDescriptionString()); + sections.m_sections.emplace_back(::ToString(i + 1) + ". " + arg.m_name, arg.ToDescriptionString()); sections.m_max_pad = std::max(sections.m_max_pad, sections.m_sections.back().m_left.size()); // Recursively push nested args diff --git a/src/script/script_error.cpp b/src/script/script_error.cpp index 9d7deffc78..ff521d5860 100644 --- a/src/script/script_error.cpp +++ b/src/script/script_error.cpp @@ -58,7 +58,7 @@ const char* ScriptErrorString(const ScriptError serror) case SCRIPT_ERR_MINIMALDATA: return "Data push larger than necessary"; case SCRIPT_ERR_SIG_PUSHONLY: - return "Only non-push operators allowed in signatures"; + return "Only push operators allowed in signatures"; case SCRIPT_ERR_SIG_HIGH_S: return "Non-canonical signature: S value is unnecessarily high"; case SCRIPT_ERR_SIG_NULLDUMMY: diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index 6980b6c0da..f3cc12201c 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -253,6 +253,9 @@ void *PosixLockedPageAllocator::AllocateLocked(size_t len, bool *lockingSuccess) } if (addr) { *lockingSuccess = mlock(addr, len) == 0; +#ifdef MADV_DONTDUMP + madvise(addr, len, MADV_DONTDUMP); +#endif } return addr; } diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 07cebeb35a..dfa8a6df21 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -6,6 +6,7 @@ #include <string> #include <boost/test/unit_test.hpp> #include <util/asmap.h> +#include <util/string.h> #include <test/data/asmap.raw.h> #include <hash.h> @@ -266,7 +267,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions) BOOST_CHECK_EQUAL(addrman.size(), 0U); for (unsigned int i = 1; i < 18; i++) { - CService addr = ResolveService("250.1.1." + std::to_string(i)); + CService addr = ResolveService("250.1.1." + ToString(i)); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); //Test: No collision in new table yet. @@ -292,7 +293,7 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions) BOOST_CHECK_EQUAL(addrman.size(), 0U); for (unsigned int i = 1; i < 80; i++) { - CService addr = ResolveService("250.1.1." + std::to_string(i)); + CService addr = ResolveService("250.1.1." + ToString(i)); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); addrman.Good(CAddress(addr, NODE_NONE)); @@ -425,7 +426,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) for (unsigned int i = 1; i < (8 * 256); i++) { int octet1 = i % 256; int octet2 = i >> 8 % 256; - std::string strAddr = std::to_string(octet1) + "." + std::to_string(octet2) + ".1.23"; + std::string strAddr = ToString(octet1) + "." + ToString(octet2) + ".1.23"; CAddress addr = CAddress(ResolveService(strAddr), NODE_NONE); // Ensure that for all addrs in addrman, isTerrible == false. @@ -477,8 +478,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) std::set<int> buckets; for (int i = 0; i < 255; i++) { CAddrInfo infoi = CAddrInfo( - CAddress(ResolveService("250.1.1." + std::to_string(i)), NODE_NONE), - ResolveIP("250.1.1." + std::to_string(i))); + CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), + ResolveIP("250.1.1." + ToString(i))); int bucket = infoi.GetTriedBucket(nKey1, asmap); buckets.insert(bucket); } @@ -489,8 +490,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) buckets.clear(); for (int j = 0; j < 255; j++) { CAddrInfo infoj = CAddrInfo( - CAddress(ResolveService("250." + std::to_string(j) + ".1.1"), NODE_NONE), - ResolveIP("250." + std::to_string(j) + ".1.1")); + CAddress(ResolveService("250." + ToString(j) + ".1.1"), NODE_NONE), + ResolveIP("250." + ToString(j) + ".1.1")); int bucket = infoj.GetTriedBucket(nKey1, asmap); buckets.insert(bucket); } @@ -531,8 +532,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) std::set<int> buckets; for (int i = 0; i < 255; i++) { CAddrInfo infoi = CAddrInfo( - CAddress(ResolveService("250.1.1." + std::to_string(i)), NODE_NONE), - ResolveIP("250.1.1." + std::to_string(i))); + CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), + ResolveIP("250.1.1." + ToString(i))); int bucket = infoi.GetNewBucket(nKey1, asmap); buckets.insert(bucket); } @@ -544,7 +545,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) for (int j = 0; j < 4 * 255; j++) { CAddrInfo infoj = CAddrInfo(CAddress( ResolveService( - std::to_string(250 + (j / 255)) + "." + std::to_string(j % 256) + ".1.1"), NODE_NONE), + ToString(250 + (j / 255)) + "." + ToString(j % 256) + ".1.1"), NODE_NONE), ResolveIP("251.4.1.1")); int bucket = infoj.GetNewBucket(nKey1, asmap); buckets.insert(bucket); @@ -557,7 +558,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) for (int p = 0; p < 255; p++) { CAddrInfo infoj = CAddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), - ResolveIP("250." + std::to_string(p) + ".1.1")); + ResolveIP("250." + ToString(p) + ".1.1")); int bucket = infoj.GetNewBucket(nKey1, asmap); buckets.insert(bucket); } @@ -610,8 +611,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) std::set<int> buckets; for (int j = 0; j < 255; j++) { CAddrInfo infoj = CAddrInfo( - CAddress(ResolveService("101." + std::to_string(j) + ".1.1"), NODE_NONE), - ResolveIP("101." + std::to_string(j) + ".1.1")); + CAddress(ResolveService("101." + ToString(j) + ".1.1"), NODE_NONE), + ResolveIP("101." + ToString(j) + ".1.1")); int bucket = infoj.GetTriedBucket(nKey1, asmap); buckets.insert(bucket); } @@ -622,8 +623,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) buckets.clear(); for (int j = 0; j < 255; j++) { CAddrInfo infoj = CAddrInfo( - CAddress(ResolveService("250." + std::to_string(j) + ".1.1"), NODE_NONE), - ResolveIP("250." + std::to_string(j) + ".1.1")); + CAddress(ResolveService("250." + ToString(j) + ".1.1"), NODE_NONE), + ResolveIP("250." + ToString(j) + ".1.1")); int bucket = infoj.GetTriedBucket(nKey1, asmap); buckets.insert(bucket); } @@ -664,8 +665,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) std::set<int> buckets; for (int i = 0; i < 255; i++) { CAddrInfo infoi = CAddrInfo( - CAddress(ResolveService("250.1.1." + std::to_string(i)), NODE_NONE), - ResolveIP("250.1.1." + std::to_string(i))); + CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), + ResolveIP("250.1.1." + ToString(i))); int bucket = infoi.GetNewBucket(nKey1, asmap); buckets.insert(bucket); } @@ -677,7 +678,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) for (int j = 0; j < 4 * 255; j++) { CAddrInfo infoj = CAddrInfo(CAddress( ResolveService( - std::to_string(250 + (j / 255)) + "." + std::to_string(j % 256) + ".1.1"), NODE_NONE), + ToString(250 + (j / 255)) + "." + ToString(j % 256) + ".1.1"), NODE_NONE), ResolveIP("251.4.1.1")); int bucket = infoj.GetNewBucket(nKey1, asmap); buckets.insert(bucket); @@ -690,7 +691,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) for (int p = 0; p < 255; p++) { CAddrInfo infoj = CAddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), - ResolveIP("101." + std::to_string(p) + ".1.1")); + ResolveIP("101." + ToString(p) + ".1.1")); int bucket = infoj.GetNewBucket(nKey1, asmap); buckets.insert(bucket); } @@ -702,7 +703,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) for (int p = 0; p < 255; p++) { CAddrInfo infoj = CAddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), - ResolveIP("250." + std::to_string(p) + ".1.1")); + ResolveIP("250." + ToString(p) + ".1.1")); int bucket = infoj.GetNewBucket(nKey1, asmap); buckets.insert(bucket); } @@ -791,7 +792,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) // Add twenty two addresses. CNetAddr source = ResolveIP("252.2.2.2"); for (unsigned int i = 1; i < 23; i++) { - CService addr = ResolveService("250.1.1."+std::to_string(i)); + CService addr = ResolveService("250.1.1."+ToString(i)); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); addrman.Good(addr); @@ -802,7 +803,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) // Ensure Good handles duplicates well. for (unsigned int i = 1; i < 23; i++) { - CService addr = ResolveService("250.1.1."+std::to_string(i)); + CService addr = ResolveService("250.1.1."+ToString(i)); addrman.Good(addr); BOOST_CHECK(addrman.size() == 22); @@ -818,7 +819,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) // Add twenty two addresses. CNetAddr source = ResolveIP("252.2.2.2"); for (unsigned int i = 1; i < 23; i++) { - CService addr = ResolveService("250.1.1."+std::to_string(i)); + CService addr = ResolveService("250.1.1."+ToString(i)); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); addrman.Good(addr); @@ -841,7 +842,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) // Lets create two collisions. for (unsigned int i = 24; i < 33; i++) { - CService addr = ResolveService("250.1.1."+std::to_string(i)); + CService addr = ResolveService("250.1.1."+ToString(i)); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); addrman.Good(addr); @@ -879,7 +880,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) // Add twenty two addresses. CNetAddr source = ResolveIP("252.2.2.2"); for (unsigned int i = 1; i < 23; i++) { - CService addr = ResolveService("250.1.1."+std::to_string(i)); + CService addr = ResolveService("250.1.1."+ToString(i)); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); addrman.Good(addr); diff --git a/src/test/blockchain_tests.cpp b/src/test/blockchain_tests.cpp index 3b4c480f72..aa704642bf 100644 --- a/src/test/blockchain_tests.cpp +++ b/src/test/blockchain_tests.cpp @@ -8,6 +8,7 @@ #include <chain.h> #include <rpc/blockchain.h> +#include <util/string.h> #include <test/util/setup_common.h> /* Equality between doubles is imprecise. Comparison should be done @@ -30,8 +31,8 @@ static CBlockIndex* CreateBlockIndexWithNbits(uint32_t nbits) static void RejectDifficultyMismatch(double difficulty, double expected_difficulty) { BOOST_CHECK_MESSAGE( DoubleEquals(difficulty, expected_difficulty, 0.00001), - "Difficulty was " + std::to_string(difficulty) - + " but was expected to be " + std::to_string(expected_difficulty)); + "Difficulty was " + ToString(difficulty) + + " but was expected to be " + ToString(expected_difficulty)); } /* Given a BlockIndex with the provided nbits, diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 73bce6f789..7310498eb6 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -13,6 +13,7 @@ #include <script/standard.h> #include <serialize.h> #include <util/memory.h> +#include <util/string.h> #include <util/system.h> #include <util/time.h> #include <validation.h> @@ -313,7 +314,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } BOOST_CHECK(banman->IsBanned(addr1)); - gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD)); + gArgs.ForceSetArg("-banscore", ToString(DEFAULT_BANSCORE_THRESHOLD)); bool dummy; peerLogic->FinalizeNode(dummyNode1.GetId(), dummy); diff --git a/src/test/fuzz/addrdb.cpp b/src/test/fuzz/addrdb.cpp new file mode 100644 index 0000000000..f21ff3fac3 --- /dev/null +++ b/src/test/fuzz/addrdb.cpp @@ -0,0 +1,43 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <addrdb.h> +#include <optional.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cassert> +#include <cstdint> +#include <string> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + + const CBanEntry ban_entry = [&] { + switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 3)) { + case 0: + return CBanEntry{fuzzed_data_provider.ConsumeIntegral<int64_t>()}; + break; + case 1: + return CBanEntry{fuzzed_data_provider.ConsumeIntegral<int64_t>(), fuzzed_data_provider.PickValueInArray<BanReason>({ + BanReason::BanReasonUnknown, + BanReason::BanReasonNodeMisbehaving, + BanReason::BanReasonManuallyAdded, + })}; + break; + case 2: { + const Optional<CBanEntry> ban_entry = ConsumeDeserializable<CBanEntry>(fuzzed_data_provider); + if (ban_entry) { + return *ban_entry; + } + break; + } + } + return CBanEntry{}; + }(); + assert(!ban_entry.banReasonToString().empty()); +} diff --git a/src/test/fuzz/blockfilter.cpp b/src/test/fuzz/blockfilter.cpp new file mode 100644 index 0000000000..be9320dcbf --- /dev/null +++ b/src/test/fuzz/blockfilter.cpp @@ -0,0 +1,44 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <blockfilter.h> +#include <optional.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <string> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + const Optional<BlockFilter> block_filter = ConsumeDeserializable<BlockFilter>(fuzzed_data_provider); + if (!block_filter) { + return; + } + { + (void)block_filter->ComputeHeader(ConsumeUInt256(fuzzed_data_provider)); + (void)block_filter->GetBlockHash(); + (void)block_filter->GetEncodedFilter(); + (void)block_filter->GetHash(); + } + { + const BlockFilterType block_filter_type = block_filter->GetFilterType(); + (void)BlockFilterTypeName(block_filter_type); + } + { + const GCSFilter gcs_filter = block_filter->GetFilter(); + (void)gcs_filter.GetN(); + (void)gcs_filter.GetParams(); + (void)gcs_filter.GetEncoded(); + (void)gcs_filter.Match(ConsumeRandomLengthByteVector(fuzzed_data_provider)); + GCSFilter::ElementSet element_set; + while (fuzzed_data_provider.ConsumeBool()) { + element_set.insert(ConsumeRandomLengthByteVector(fuzzed_data_provider)); + gcs_filter.MatchAny(element_set); + } + } +} diff --git a/src/test/fuzz/chain.cpp b/src/test/fuzz/chain.cpp new file mode 100644 index 0000000000..b322516cc7 --- /dev/null +++ b/src/test/fuzz/chain.cpp @@ -0,0 +1,65 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <chain.h> +#include <optional.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + Optional<CDiskBlockIndex> disk_block_index = ConsumeDeserializable<CDiskBlockIndex>(fuzzed_data_provider); + if (!disk_block_index) { + return; + } + + const uint256 zero{}; + disk_block_index->phashBlock = &zero; + (void)disk_block_index->GetBlockHash(); + (void)disk_block_index->GetBlockPos(); + (void)disk_block_index->GetBlockTime(); + (void)disk_block_index->GetBlockTimeMax(); + (void)disk_block_index->GetMedianTimePast(); + (void)disk_block_index->GetUndoPos(); + (void)disk_block_index->HaveTxsDownloaded(); + (void)disk_block_index->IsValid(); + (void)disk_block_index->ToString(); + + const CBlockHeader block_header = disk_block_index->GetBlockHeader(); + (void)CDiskBlockIndex{*disk_block_index}; + (void)disk_block_index->BuildSkip(); + + while (fuzzed_data_provider.ConsumeBool()) { + const BlockStatus block_status = fuzzed_data_provider.PickValueInArray({ + BlockStatus::BLOCK_VALID_UNKNOWN, + BlockStatus::BLOCK_VALID_RESERVED, + BlockStatus::BLOCK_VALID_TREE, + BlockStatus::BLOCK_VALID_TRANSACTIONS, + BlockStatus::BLOCK_VALID_CHAIN, + BlockStatus::BLOCK_VALID_SCRIPTS, + BlockStatus::BLOCK_VALID_MASK, + BlockStatus::BLOCK_HAVE_DATA, + BlockStatus::BLOCK_HAVE_UNDO, + BlockStatus::BLOCK_HAVE_MASK, + BlockStatus::BLOCK_FAILED_VALID, + BlockStatus::BLOCK_FAILED_CHILD, + BlockStatus::BLOCK_FAILED_MASK, + BlockStatus::BLOCK_OPT_WITNESS, + }); + if (block_status & ~BLOCK_VALID_MASK) { + continue; + } + (void)disk_block_index->RaiseValidity(block_status); + } + + CBlockIndex block_index{block_header}; + block_index.phashBlock = &zero; + (void)block_index.GetBlockHash(); + (void)block_index.ToString(); +} diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 24459c21be..63b9296574 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -227,4 +227,44 @@ void test_one_input(const std::vector<uint8_t>& buffer) (void)HasAllDesirableServiceFlags(service_flags); (void)MayHaveUsefulAddressDB(service_flags); } + + { + CDataStream stream(SER_NETWORK, INIT_PROTO_VERSION); + + ser_writedata64(stream, u64); + const uint64_t deserialized_u64 = ser_readdata64(stream); + assert(u64 == deserialized_u64 && stream.empty()); + + ser_writedata32(stream, u32); + const uint32_t deserialized_u32 = ser_readdata32(stream); + assert(u32 == deserialized_u32 && stream.empty()); + + ser_writedata32be(stream, u32); + const uint32_t deserialized_u32be = ser_readdata32be(stream); + assert(u32 == deserialized_u32be && stream.empty()); + + ser_writedata16(stream, u16); + const uint16_t deserialized_u16 = ser_readdata16(stream); + assert(u16 == deserialized_u16 && stream.empty()); + + ser_writedata16be(stream, u16); + const uint16_t deserialized_u16be = ser_readdata16be(stream); + assert(u16 == deserialized_u16be && stream.empty()); + + ser_writedata8(stream, u8); + const uint8_t deserialized_u8 = ser_readdata8(stream); + assert(u8 == deserialized_u8 && stream.empty()); + } + + { + CDataStream stream(SER_NETWORK, INIT_PROTO_VERSION); + + WriteCompactSize(stream, u64); + try { + const uint64_t deserialized_u64 = ReadCompactSize(stream); + assert(u64 == deserialized_u64 && stream.empty()); + } + catch (const std::ios_base::failure&) { + } + } } diff --git a/src/test/fuzz/multiplication_overflow.cpp b/src/test/fuzz/multiplication_overflow.cpp index 923db8058b..a4b158c18b 100644 --- a/src/test/fuzz/multiplication_overflow.cpp +++ b/src/test/fuzz/multiplication_overflow.cpp @@ -10,6 +10,14 @@ #include <string> #include <vector> +#if defined(__has_builtin) +#if __has_builtin(__builtin_mul_overflow) +#define HAVE_BUILTIN_MUL_OVERFLOW +#endif +#elif defined(__GNUC__) && (__GNUC__ >= 5) +#define HAVE_BUILTIN_MUL_OVERFLOW +#endif + namespace { template <typename T> void TestMultiplicationOverflow(FuzzedDataProvider& fuzzed_data_provider) @@ -17,12 +25,18 @@ void TestMultiplicationOverflow(FuzzedDataProvider& fuzzed_data_provider) const T i = fuzzed_data_provider.ConsumeIntegral<T>(); const T j = fuzzed_data_provider.ConsumeIntegral<T>(); const bool is_multiplication_overflow_custom = MultiplicationOverflow(i, j); +#if defined(HAVE_BUILTIN_MUL_OVERFLOW) T result_builtin; const bool is_multiplication_overflow_builtin = __builtin_mul_overflow(i, j, &result_builtin); assert(is_multiplication_overflow_custom == is_multiplication_overflow_builtin); if (!is_multiplication_overflow_custom) { assert(i * j == result_builtin); } +#else + if (!is_multiplication_overflow_custom) { + (void)(i * j); + } +#endif } } // namespace @@ -38,5 +52,4 @@ void test_one_input(const std::vector<uint8_t>& buffer) TestMultiplicationOverflow<char>(fuzzed_data_provider); TestMultiplicationOverflow<unsigned char>(fuzzed_data_provider); TestMultiplicationOverflow<signed char>(fuzzed_data_provider); - TestMultiplicationOverflow<bool>(fuzzed_data_provider); } diff --git a/src/test/fuzz/net_permissions.cpp b/src/test/fuzz/net_permissions.cpp new file mode 100644 index 0000000000..bfc5d21427 --- /dev/null +++ b/src/test/fuzz/net_permissions.cpp @@ -0,0 +1,51 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <net_permissions.h> +#include <optional.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cassert> +#include <cstdint> +#include <string> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + const std::string s = fuzzed_data_provider.ConsumeRandomLengthString(32); + const NetPermissionFlags net_permission_flags = fuzzed_data_provider.ConsumeBool() ? fuzzed_data_provider.PickValueInArray<NetPermissionFlags>({ + NetPermissionFlags::PF_NONE, + NetPermissionFlags::PF_BLOOMFILTER, + NetPermissionFlags::PF_RELAY, + NetPermissionFlags::PF_FORCERELAY, + NetPermissionFlags::PF_NOBAN, + NetPermissionFlags::PF_MEMPOOL, + NetPermissionFlags::PF_ISIMPLICIT, + NetPermissionFlags::PF_ALL, + }) : + static_cast<NetPermissionFlags>(fuzzed_data_provider.ConsumeIntegral<uint32_t>()); + + NetWhitebindPermissions net_whitebind_permissions; + std::string error_net_whitebind_permissions; + if (NetWhitebindPermissions::TryParse(s, net_whitebind_permissions, error_net_whitebind_permissions)) { + (void)NetPermissions::ToStrings(net_whitebind_permissions.m_flags); + (void)NetPermissions::AddFlag(net_whitebind_permissions.m_flags, net_permission_flags); + assert(NetPermissions::HasFlag(net_whitebind_permissions.m_flags, net_permission_flags)); + (void)NetPermissions::ClearFlag(net_whitebind_permissions.m_flags, net_permission_flags); + (void)NetPermissions::ToStrings(net_whitebind_permissions.m_flags); + } + + NetWhitelistPermissions net_whitelist_permissions; + std::string error_net_whitelist_permissions; + if (NetWhitelistPermissions::TryParse(s, net_whitelist_permissions, error_net_whitelist_permissions)) { + (void)NetPermissions::ToStrings(net_whitelist_permissions.m_flags); + (void)NetPermissions::AddFlag(net_whitelist_permissions.m_flags, net_permission_flags); + assert(NetPermissions::HasFlag(net_whitelist_permissions.m_flags, net_permission_flags)); + (void)NetPermissions::ClearFlag(net_whitelist_permissions.m_flags, net_permission_flags); + (void)NetPermissions::ToStrings(net_whitelist_permissions.m_flags); + } +} diff --git a/src/test/fuzz/protocol.cpp b/src/test/fuzz/protocol.cpp new file mode 100644 index 0000000000..954471de6c --- /dev/null +++ b/src/test/fuzz/protocol.cpp @@ -0,0 +1,32 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <optional.h> +#include <protocol.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <stdexcept> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + const Optional<CInv> inv = ConsumeDeserializable<CInv>(fuzzed_data_provider); + if (!inv) { + return; + } + try { + (void)inv->GetCommand(); + } catch (const std::out_of_range&) { + } + (void)inv->ToString(); + const Optional<CInv> another_inv = ConsumeDeserializable<CInv>(fuzzed_data_provider); + if (!another_inv) { + return; + } + (void)(*inv < *another_inv); +} diff --git a/src/test/fuzz/timedata.cpp b/src/test/fuzz/timedata.cpp new file mode 100644 index 0000000000..a0e579a88f --- /dev/null +++ b/src/test/fuzz/timedata.cpp @@ -0,0 +1,29 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <timedata.h> + +#include <cstdint> +#include <string> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + const unsigned int max_size = fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 1000); + // Divide by 2 to avoid signed integer overflow in .median() + const int64_t initial_value = fuzzed_data_provider.ConsumeIntegral<int64_t>() / 2; + CMedianFilter<int64_t> median_filter{max_size, initial_value}; + while (fuzzed_data_provider.remaining_bytes() > 0) { + (void)median_filter.median(); + assert(median_filter.size() > 0); + assert(static_cast<size_t>(median_filter.size()) == median_filter.sorted().size()); + assert(static_cast<unsigned int>(median_filter.size()) <= max_size || max_size == 0); + // Divide by 2 to avoid signed integer overflow in .median() + median_filter.input(fuzzed_data_provider.ConsumeIntegral<int64_t>() / 2); + } +} diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 47f8d3fb27..10be2ebaf7 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -13,6 +13,7 @@ #include <streams.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> +#include <uint256.h> #include <version.h> #include <cstdint> @@ -70,6 +71,15 @@ NODISCARD inline CScriptNum ConsumeScriptNum(FuzzedDataProvider& fuzzed_data_pro return CScriptNum{fuzzed_data_provider.ConsumeIntegral<int64_t>()}; } +NODISCARD inline uint256 ConsumeUInt256(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + const std::vector<unsigned char> v256 = fuzzed_data_provider.ConsumeBytes<unsigned char>(sizeof(uint256)); + if (v256.size() != sizeof(uint256)) { + return {}; + } + return uint256{v256}; +} + template <typename T> bool MultiplicationOverflow(T i, T j) { diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index 85dc961bea..034b7938f9 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -8,6 +8,7 @@ #include <uint256.h> #include <util/system.h> #include <util/strencodings.h> +#include <util/string.h> #include <test/util/setup_common.h> #include <string> @@ -176,7 +177,7 @@ BOOST_AUTO_TEST_CASE(key_signature_tests) bool found_small = false; for (int i = 0; i < 256; ++i) { sig.clear(); - std::string msg = "A message to be signed" + std::to_string(i); + std::string msg = "A message to be signed" + ToString(i); msg_hash = Hash(msg.begin(), msg.end()); BOOST_CHECK(key.Sign(msg_hash, sig)); found = sig[3] == 0x20; diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index cb1ef5dcf3..9b5a86fef2 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -15,6 +15,7 @@ #include <chainparams.h> #include <util/memory.h> #include <util/system.h> +#include <util/string.h> #include <memory> @@ -85,7 +86,7 @@ BOOST_AUTO_TEST_CASE(cnode_listen_port) BOOST_CHECK(port == Params().GetDefaultPort()); // test set port unsigned short altPort = 12345; - BOOST_CHECK(gArgs.SoftSetArg("-port", std::to_string(altPort))); + BOOST_CHECK(gArgs.SoftSetArg("-port", ToString(altPort))); port = GetListenPort(); BOOST_CHECK(port == altPort); } diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index 45644834a5..10b161aa80 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -11,6 +11,7 @@ #include <boost/test/unit_test.hpp> #include <univalue.h> #include <util/strencodings.h> +#include <util/string.h> #include <vector> BOOST_FIXTURE_TEST_SUITE(settings_tests, BasicTestingSetup) @@ -114,7 +115,7 @@ BOOST_FIXTURE_TEST_CASE(Merge, MergeTestingSetup) std::vector<util::SettingsValue>& dest) { if (action == SET || action == SECTION_SET) { for (int i = 0; i < 2; ++i) { - dest.push_back(value_prefix + std::to_string(++value_suffix)); + dest.push_back(value_prefix + ToString(++value_suffix)); desc += " " + name_prefix + name + "=" + dest.back().get_str(); } } else if (action == NEGATE || action == SECTION_NEGATE) { diff --git a/src/test/timedata_tests.cpp b/src/test/timedata_tests.cpp index 19bd0d142f..29b43e9bec 100644 --- a/src/test/timedata_tests.cpp +++ b/src/test/timedata_tests.cpp @@ -5,6 +5,7 @@ #include <netaddress.h> #include <noui.h> +#include <util/string.h> #include <test/util/logging.h> #include <test/util/setup_common.h> #include <timedata.h> @@ -46,7 +47,7 @@ static void MultiAddTimeData(int n, int64_t offset) static int cnt = 0; for (int i = 0; i < n; ++i) { CNetAddr addr; - addr.SetInternal(std::to_string(++cnt)); + addr.SetInternal(ToString(++cnt)); AddTimeData(addr, offset); } } diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 44d4e0890a..96520079d7 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -784,6 +784,40 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) BOOST_CHECK(!IsStandardTx(CTransaction(t), reason)); BOOST_CHECK_EQUAL(reason, "scriptsig-size"); + // Check scriptSig format (non-standard if there are any other ops than just PUSHs) + t.vin[0].scriptSig = CScript() + << OP_TRUE << OP_0 << OP_1NEGATE << OP_16 // OP_n (single byte pushes: n = 1, 0, -1, 16) + << std::vector<unsigned char>(75, 0) // OP_PUSHx [...x bytes...] + << std::vector<unsigned char>(235, 0) // OP_PUSHDATA1 x [...x bytes...] + << std::vector<unsigned char>(1234, 0) // OP_PUSHDATA2 x [...x bytes...] + << OP_9; + BOOST_CHECK(IsStandardTx(CTransaction(t), reason)); + + const std::vector<unsigned char> non_push_ops = { // arbitrary set of non-push operations + OP_NOP, OP_VERIFY, OP_IF, OP_ROT, OP_3DUP, OP_SIZE, OP_EQUAL, OP_ADD, OP_SUB, + OP_HASH256, OP_CODESEPARATOR, OP_CHECKSIG, OP_CHECKLOCKTIMEVERIFY }; + + CScript::const_iterator pc = t.vin[0].scriptSig.begin(); + while (pc < t.vin[0].scriptSig.end()) { + opcodetype opcode; + CScript::const_iterator prev_pc = pc; + t.vin[0].scriptSig.GetOp(pc, opcode); // advance to next op + // for the sake of simplicity, we only replace single-byte push operations + if (opcode >= 1 && opcode <= OP_PUSHDATA4) + continue; + + int index = prev_pc - t.vin[0].scriptSig.begin(); + unsigned char orig_op = *prev_pc; // save op + // replace current push-op with each non-push-op + for (auto op : non_push_ops) { + t.vin[0].scriptSig[index] = op; + BOOST_CHECK(!IsStandardTx(CTransaction(t), reason)); + BOOST_CHECK_EQUAL(reason, "scriptsig-not-pushonly"); + } + t.vin[0].scriptSig[index] = orig_op; // restore op + BOOST_CHECK(IsStandardTx(CTransaction(t), reason)); + } + // Check tx-size (non-standard if transaction weight is > MAX_STANDARD_TX_WEIGHT) t.vin.clear(); t.vin.resize(2438); // size per input (empty scriptSig): 41 bytes diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index e19a96eafc..d684b97787 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -24,6 +24,7 @@ #include <txdb.h> #include <util/memory.h> #include <util/strencodings.h> +#include <util/string.h> #include <util/time.h> #include <util/translation.h> #include <validation.h> diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index 56ad62eb24..0930309c3a 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -13,6 +13,7 @@ #include <random.h> #include <scheduler.h> #include <txmempool.h> +#include <util/string.h> #include <type_traits> diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index eb0d31fbdc..73b37f909f 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -894,7 +894,7 @@ struct ArgsMergeTestingSetup : public BasicTestingSetup { if (action == SECTION_SET || action == SECTION_NEGATE) prefix = section + "."; if (action == SET || action == SECTION_SET) { for (int i = 0; i < 2; ++i) { - values.push_back(prefix + name + "=" + value_prefix + std::to_string(++suffix)); + values.push_back(prefix + name + "=" + value_prefix + ToString(++suffix)); } } if (action == NEGATE || action == SECTION_NEGATE) { @@ -1182,6 +1182,12 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney) BOOST_CHECK_EQUAL(ret, COIN); BOOST_CHECK(ParseMoney("1", ret)); BOOST_CHECK_EQUAL(ret, COIN); + BOOST_CHECK(ParseMoney(" 1", ret)); + BOOST_CHECK_EQUAL(ret, COIN); + BOOST_CHECK(ParseMoney("1 ", ret)); + BOOST_CHECK_EQUAL(ret, COIN); + BOOST_CHECK(ParseMoney(" 1 ", ret)); + BOOST_CHECK_EQUAL(ret, COIN); BOOST_CHECK(ParseMoney("0.1", ret)); BOOST_CHECK_EQUAL(ret, COIN/10); BOOST_CHECK(ParseMoney("0.01", ret)); @@ -1198,12 +1204,26 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney) BOOST_CHECK_EQUAL(ret, COIN/10000000); BOOST_CHECK(ParseMoney("0.00000001", ret)); BOOST_CHECK_EQUAL(ret, COIN/100000000); + BOOST_CHECK(ParseMoney(" 0.00000001 ", ret)); + BOOST_CHECK_EQUAL(ret, COIN/100000000); + BOOST_CHECK(ParseMoney("0.00000001 ", ret)); + BOOST_CHECK_EQUAL(ret, COIN/100000000); + BOOST_CHECK(ParseMoney(" 0.00000001", ret)); + BOOST_CHECK_EQUAL(ret, COIN/100000000); // Parsing amount that can not be represented in ret should fail BOOST_CHECK(!ParseMoney("0.000000001", ret)); // Parsing empty string should fail BOOST_CHECK(!ParseMoney("", ret)); + BOOST_CHECK(!ParseMoney(" ", ret)); + BOOST_CHECK(!ParseMoney(" ", ret)); + + // Parsing two numbers should fail + BOOST_CHECK(!ParseMoney("1 2", ret)); + BOOST_CHECK(!ParseMoney(" 1 2 ", ret)); + BOOST_CHECK(!ParseMoney(" 1.2 3 ", ret)); + BOOST_CHECK(!ParseMoney(" 1 2.3 ", ret)); // Attempted 63 bit overflow should fail BOOST_CHECK(!ParseMoney("92233720368.54775808", ret)); diff --git a/src/test/util_threadnames_tests.cpp b/src/test/util_threadnames_tests.cpp index 78dbf848bb..cee4e0ce3c 100644 --- a/src/test/util_threadnames_tests.cpp +++ b/src/test/util_threadnames_tests.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <util/string.h> #include <util/threadnames.h> #include <test/util/setup_common.h> @@ -32,7 +33,7 @@ std::set<std::string> RenameEnMasse(int num_threads) std::mutex lock; auto RenameThisThread = [&](int i) { - util::ThreadRename(TEST_THREAD_NAME_BASE + std::to_string(i)); + util::ThreadRename(TEST_THREAD_NAME_BASE + ToString(i)); std::lock_guard<std::mutex> guard(lock); names.insert(util::ThreadGetInternalName()); }; @@ -65,7 +66,7 @@ BOOST_AUTO_TEST_CASE(util_threadnames_test_rename_threaded) // Names "test_thread.[n]" should exist for n = [0, 99] for (int i = 0; i < 100; ++i) { - BOOST_CHECK(names.find(TEST_THREAD_NAME_BASE + std::to_string(i)) != names.end()); + BOOST_CHECK(names.find(TEST_THREAD_NAME_BASE + ToString(i)) != names.end()); } } diff --git a/src/util/moneystr.cpp b/src/util/moneystr.cpp index 40d8918dfc..544cfb58f9 100644 --- a/src/util/moneystr.cpp +++ b/src/util/moneystr.cpp @@ -31,12 +31,12 @@ std::string FormatMoney(const CAmount& n) } -bool ParseMoney(const std::string& str, CAmount& nRet) +bool ParseMoney(const std::string& money_string, CAmount& nRet) { - if (!ValidAsCString(str)) { + if (!ValidAsCString(money_string)) { return false; } - + const std::string str = TrimString(money_string); if (str.empty()) { return false; } @@ -44,8 +44,6 @@ bool ParseMoney(const std::string& str, CAmount& nRet) std::string strWhole; int64_t nUnits = 0; const char* p = str.c_str(); - while (IsSpace(*p)) - p++; for (; *p; p++) { if (*p == '.') @@ -60,14 +58,14 @@ bool ParseMoney(const std::string& str, CAmount& nRet) break; } if (IsSpace(*p)) - break; + return false; if (!IsDigit(*p)) return false; strWhole.insert(strWhole.end(), *p); } - for (; *p; p++) - if (!IsSpace(*p)) - return false; + if (*p) { + return false; + } if (strWhole.size() > 10) // guard against 63 bit overflow return false; if (nUnits < 0 || nUnits > COIN) diff --git a/src/util/string.h b/src/util/string.h index 3db8fc8b98..694f0a1ca4 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -8,6 +8,8 @@ #include <attributes.h> #include <cstring> +#include <locale> +#include <sstream> #include <string> #include <vector> @@ -52,4 +54,16 @@ NODISCARD inline bool ValidAsCString(const std::string& str) noexcept return str.size() == strlen(str.c_str()); } +/** + * Locale-independent version of std::to_string + */ +template <typename T> +std::string ToString(const T& t) +{ + std::ostringstream oss; + oss.imbue(std::locale::classic()); + oss << t; + return oss.str(); +} + #endif // BITCOIN_UTIL_STRENCODINGS_H diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 486a6b8bf1..1623ab9074 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -60,7 +60,7 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, std::vector<std::string>& errors) { // check that fee rate is higher than mempool's minimum fee // (no point in bumping fee if we know that the new tx won't be accepted to the mempool) - // This may occur if the user set FeeRate, TotalFee or paytxfee too low, if fallbackfee is too low, or, perhaps, + // This may occur if the user set fee_rate or paytxfee too low, if fallbackfee is too low, or, perhaps, // in a rare situation where the mempool minimum fee increased significantly since the fee estimation just a // moment earlier. In this case, we report an error to the user, who may adjust the fee. CFeeRate minMempoolFeeRate = wallet.chain().mempoolMinFee(); @@ -150,132 +150,6 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid) return res == feebumper::Result::OK; } -Result CreateTotalBumpTransaction(const CWallet* wallet, const uint256& txid, const CCoinControl& coin_control, CAmount total_fee, std::vector<std::string>& errors, - CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx) -{ - new_fee = total_fee; - - auto locked_chain = wallet->chain().lock(); - LOCK(wallet->cs_wallet); - errors.clear(); - auto it = wallet->mapWallet.find(txid); - if (it == wallet->mapWallet.end()) { - errors.push_back("Invalid or non-wallet transaction id"); - return Result::INVALID_ADDRESS_OR_KEY; - } - const CWalletTx& wtx = it->second; - - Result result = PreconditionChecks(*wallet, wtx, errors); - if (result != Result::OK) { - return result; - } - - // figure out which output was change - // if there was no change output or multiple change outputs, fail - int nOutput = -1; - for (size_t i = 0; i < wtx.tx->vout.size(); ++i) { - if (wallet->IsChange(wtx.tx->vout[i])) { - if (nOutput != -1) { - errors.push_back("Transaction has multiple change outputs"); - return Result::WALLET_ERROR; - } - nOutput = i; - } - } - if (nOutput == -1) { - errors.push_back("Transaction does not have a change output"); - return Result::WALLET_ERROR; - } - - // Calculate the expected size of the new transaction. - int64_t txSize = GetVirtualTransactionSize(*(wtx.tx)); - const int64_t maxNewTxSize = CalculateMaximumSignedTxSize(*wtx.tx, wallet); - if (maxNewTxSize < 0) { - errors.push_back("Transaction contains inputs that cannot be signed"); - return Result::INVALID_ADDRESS_OR_KEY; - } - - // calculate the old fee and fee-rate - isminefilter filter = wallet->GetLegacyScriptPubKeyMan() && wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; - old_fee = wtx.GetDebit(filter) - wtx.tx->GetValueOut(); - CFeeRate nOldFeeRate(old_fee, txSize); - // The wallet uses a conservative WALLET_INCREMENTAL_RELAY_FEE value to - // future proof against changes to network wide policy for incremental relay - // fee that our node may not be aware of. - CFeeRate nodeIncrementalRelayFee = wallet->chain().relayIncrementalFee(); - CFeeRate walletIncrementalRelayFee = CFeeRate(WALLET_INCREMENTAL_RELAY_FEE); - if (nodeIncrementalRelayFee > walletIncrementalRelayFee) { - walletIncrementalRelayFee = nodeIncrementalRelayFee; - } - - CAmount minTotalFee = nOldFeeRate.GetFee(maxNewTxSize) + nodeIncrementalRelayFee.GetFee(maxNewTxSize); - if (total_fee < minTotalFee) { - errors.push_back(strprintf("Insufficient totalFee, must be at least %s (oldFee %s + incrementalFee %s)", - FormatMoney(minTotalFee), FormatMoney(nOldFeeRate.GetFee(maxNewTxSize)), FormatMoney(nodeIncrementalRelayFee.GetFee(maxNewTxSize)))); - return Result::INVALID_PARAMETER; - } - CAmount requiredFee = GetRequiredFee(*wallet, maxNewTxSize); - if (total_fee < requiredFee) { - errors.push_back(strprintf("Insufficient totalFee (cannot be less than required fee %s)", - FormatMoney(requiredFee))); - return Result::INVALID_PARAMETER; - } - - // Check that in all cases the new fee doesn't violate maxTxFee - const CAmount max_tx_fee = wallet->m_default_max_tx_fee; - if (new_fee > max_tx_fee) { - errors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than -maxtxfee %s)", - FormatMoney(new_fee), FormatMoney(max_tx_fee))); - return Result::WALLET_ERROR; - } - - // check that fee rate is higher than mempool's minimum fee - // (no point in bumping fee if we know that the new tx won't be accepted to the mempool) - // This may occur if the user set TotalFee or paytxfee too low, if fallbackfee is too low, or, perhaps, - // in a rare situation where the mempool minimum fee increased significantly since the fee estimation just a - // moment earlier. In this case, we report an error to the user, who may use total_fee to make an adjustment. - CFeeRate minMempoolFeeRate = wallet->chain().mempoolMinFee(); - CFeeRate nNewFeeRate = CFeeRate(total_fee, maxNewTxSize); - if (nNewFeeRate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) { - errors.push_back(strprintf( - "New fee rate (%s) is lower than the minimum fee rate (%s) to get into the mempool -- " - "the totalFee value should be at least %s to add transaction", - FormatMoney(nNewFeeRate.GetFeePerK()), - FormatMoney(minMempoolFeeRate.GetFeePerK()), - FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)))); - return Result::WALLET_ERROR; - } - - // Now modify the output to increase the fee. - // If the output is not large enough to pay the fee, fail. - CAmount nDelta = new_fee - old_fee; - assert(nDelta > 0); - mtx = CMutableTransaction{*wtx.tx}; - CTxOut* poutput = &(mtx.vout[nOutput]); - if (poutput->nValue < nDelta) { - errors.push_back("Change output is too small to bump the fee"); - return Result::WALLET_ERROR; - } - - // If the output would become dust, discard it (converting the dust to fee) - poutput->nValue -= nDelta; - if (poutput->nValue <= GetDustThreshold(*poutput, GetDiscardRate(*wallet))) { - wallet->WalletLogPrintf("Bumping fee and discarding dust output\n"); - new_fee += poutput->nValue; - mtx.vout.erase(mtx.vout.begin() + nOutput); - } - - // Mark new tx not replaceable, if requested. - if (!coin_control.m_signal_bip125_rbf.get_value_or(wallet->m_signal_rbf)) { - for (auto& input : mtx.vin) { - if (input.nSequence < 0xfffffffe) input.nSequence = 0xfffffffe; - } - } - - return Result::OK; -} - - Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<std::string>& errors, CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx) { diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h index 9357397606..859f754761 100644 --- a/src/wallet/feebumper.h +++ b/src/wallet/feebumper.h @@ -28,16 +28,6 @@ enum class Result //! Return whether transaction can be bumped. bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid); -//! Create bumpfee transaction based on total amount. -Result CreateTotalBumpTransaction(const CWallet* wallet, - const uint256& txid, - const CCoinControl& coin_control, - CAmount total_fee, - std::vector<std::string>& errors, - CAmount& old_fee, - CAmount& new_fee, - CMutableTransaction& mtx); - //! Create bumpfee transaction based on feerate estimates. Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a913aa3024..0eb7ed2b71 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3342,12 +3342,11 @@ static UniValue bumpfee(const JSONRPCRequest& request) "\nBumps the fee of an opt-in-RBF transaction T, replacing it with a new transaction B.\n" "An opt-in RBF transaction with the given txid must be in the wallet.\n" "The command will pay the additional fee by reducing change outputs or adding inputs when necessary. It may add a new change output if one does not already exist.\n" - "If `totalFee` (DEPRECATED) is given, adding inputs is not supported, so there must be a single change output that is big enough or it will fail.\n" "All inputs in the original transaction will be included in the replacement transaction.\n" "The command will fail if the wallet or mempool contains a transaction that spends one of T's outputs.\n" "By default, the new fee will be calculated automatically using estimatesmartfee.\n" "The user can specify a confirmation target for estimatesmartfee.\n" - "Alternatively, the user can specify totalFee (DEPRECATED), or fee_rate (" + CURRENCY_UNIT + " per kB) for the new transaction .\n" + "Alternatively, the user can specify a fee_rate (" + CURRENCY_UNIT + " per kB) for the new transaction.\n" "At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee\n" "returned by getnetworkinfo) to enter the node's mempool.\n", { @@ -3355,11 +3354,7 @@ static UniValue bumpfee(const JSONRPCRequest& request) {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", { {"confTarget", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"}, - {"totalFee", RPCArg::Type::NUM, /* default */ "fallback to 'confTarget'", "Total fee (NOT feerate) to pay, in satoshis. (DEPRECATED)\n" - " In rare cases, the actual fee paid might be slightly higher than the specified\n" - " totalFee if the tx change output has to be removed because it is too close to\n" - " the dust threshold."}, - {"fee_rate", RPCArg::Type::NUM, /* default */ "fallback to 'confTarget'", "FeeRate (NOT total fee) to pay, in " + CURRENCY_UNIT + " per kB\n" + {"fee_rate", RPCArg::Type::NUM, /* default */ "fall back to 'confTarget'", "fee rate (NOT total fee) to pay, in " + CURRENCY_UNIT + " per kB\n" " Specify a fee rate instead of relying on the built-in fee estimator.\n" "Must be at least 0.0001 " + CURRENCY_UNIT + " per kB higher than the current transaction fee rate.\n"}, {"replaceable", RPCArg::Type::BOOL, /* default */ "true", "Whether the new transaction should still be\n" @@ -3400,7 +3395,6 @@ static UniValue bumpfee(const JSONRPCRequest& request) CCoinControl coin_control; coin_control.fAllowWatchOnly = pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); // optional parameters - CAmount totalFee = 0; coin_control.m_signal_bip125_rbf = true; if (!request.params[1].isNull()) { @@ -3408,26 +3402,15 @@ static UniValue bumpfee(const JSONRPCRequest& request) RPCTypeCheckObj(options, { {"confTarget", UniValueType(UniValue::VNUM)}, - {"totalFee", UniValueType(UniValue::VNUM)}, {"fee_rate", UniValueType(UniValue::VNUM)}, {"replaceable", UniValueType(UniValue::VBOOL)}, {"estimate_mode", UniValueType(UniValue::VSTR)}, }, true, true); - if (options.exists("confTarget") && (options.exists("totalFee") || options.exists("fee_rate"))) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "confTarget can't be set with totalFee or fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate."); - } else if (options.exists("fee_rate") && options.exists("totalFee")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "fee_rate can't be set along with totalFee."); + if (options.exists("confTarget") && options.exists("fee_rate")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "confTarget can't be set with fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate."); } else if (options.exists("confTarget")) { // TODO: alias this to conf_target coin_control.m_confirm_target = ParseConfirmTarget(options["confTarget"], pwallet->chain().estimateMaxBlocks()); - } else if (options.exists("totalFee")) { - if (!pwallet->chain().rpcEnableDeprecated("totalFee")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "totalFee argument has been deprecated and will be removed in 0.20. Please use -deprecatedrpc=totalFee to continue using this argument until removal."); - } - totalFee = options["totalFee"].get_int64(); - if (totalFee <= 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid totalFee %s (must be greater than 0)", FormatMoney(totalFee))); - } } else if (options.exists("fee_rate")) { CFeeRate fee_rate(AmountFromValue(options["fee_rate"])); if (fee_rate <= CFeeRate(0)) { @@ -3460,13 +3443,8 @@ static UniValue bumpfee(const JSONRPCRequest& request) CAmount new_fee; CMutableTransaction mtx; feebumper::Result res; - if (totalFee > 0) { - // Targeting total fee bump. Requires a change output of sufficient size. - res = feebumper::CreateTotalBumpTransaction(pwallet, hash, coin_control, totalFee, errors, old_fee, new_fee, mtx); - } else { - // Targeting feerate bump. - res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx); - } + // Targeting feerate bump. + res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx); if (res != feebumper::Result::OK) { switch(res) { case feebumper::Result::INVALID_ADDRESS_OR_KEY: @@ -4196,7 +4174,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) }, {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Marks this transaction as BIP125 replaceable.\n" " Allows this transaction to be replaced by a transaction with higher fees"}, - {"conf_target", RPCArg::Type::NUM, /* default */ "Fallback to wallet's confirmation target", "Confirmation target (in blocks)"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "fall back to wallet's confirmation target (txconfirmtarget)", "Confirmation target (in blocks)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n" " \"UNSET\"\n" " \"ECONOMICAL\"\n" diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 6fe1d84d64..b96cb0aa1a 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -8,6 +8,7 @@ #include <script/sign.h> #include <util/bip32.h> #include <util/strencodings.h> +#include <util/string.h> #include <util/translation.h> #include <wallet/scriptpubkeyman.h> @@ -989,7 +990,7 @@ void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& // example: 1 | BIP32_HARDENED_KEY_LIMIT == 0x80000001 == 2147483649 if (internal) { chainChildKey.Derive(childKey, hdChain.nInternalChainCounter | BIP32_HARDENED_KEY_LIMIT); - metadata.hdKeypath = "m/0'/1'/" + std::to_string(hdChain.nInternalChainCounter) + "'"; + metadata.hdKeypath = "m/0'/1'/" + ToString(hdChain.nInternalChainCounter) + "'"; metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); metadata.key_origin.path.push_back(1 | BIP32_HARDENED_KEY_LIMIT); metadata.key_origin.path.push_back(hdChain.nInternalChainCounter | BIP32_HARDENED_KEY_LIMIT); @@ -997,7 +998,7 @@ void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& } else { chainChildKey.Derive(childKey, hdChain.nExternalChainCounter | BIP32_HARDENED_KEY_LIMIT); - metadata.hdKeypath = "m/0'/0'/" + std::to_string(hdChain.nExternalChainCounter) + "'"; + metadata.hdKeypath = "m/0'/0'/" + ToString(hdChain.nExternalChainCounter) + "'"; metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); metadata.key_origin.path.push_back(hdChain.nExternalChainCounter | BIP32_HARDENED_KEY_LIMIT); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 01aadcdd6d..9a972febab 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -344,7 +344,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, return false; } -void CWallet::ChainStateFlushed(const CBlockLocator& loc) +void CWallet::chainStateFlushed(const CBlockLocator& loc) { WalletBatch batch(*database); batch.WriteBestBlock(loc); @@ -1089,7 +1089,7 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmatio MarkInputsDirty(ptx); } -void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) { +void CWallet::transactionAddedToMempool(const CTransactionRef& ptx) { auto locked_chain = chain().lock(); LOCK(cs_wallet); CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0); @@ -1101,7 +1101,7 @@ void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) { } } -void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx) { +void CWallet::transactionRemovedFromMempool(const CTransactionRef &ptx) { LOCK(cs_wallet); auto it = mapWallet.find(ptx->GetHash()); if (it != mapWallet.end()) { @@ -1109,7 +1109,7 @@ void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx) { } } -void CWallet::BlockConnected(const CBlock& block, int height) +void CWallet::blockConnected(const CBlock& block, int height) { const uint256& block_hash = block.GetHash(); auto locked_chain = chain().lock(); @@ -1120,11 +1120,11 @@ void CWallet::BlockConnected(const CBlock& block, int height) for (size_t index = 0; index < block.vtx.size(); index++) { CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, block_hash, index); SyncTransaction(block.vtx[index], confirm); - TransactionRemovedFromMempool(block.vtx[index]); + transactionRemovedFromMempool(block.vtx[index]); } } -void CWallet::BlockDisconnected(const CBlock& block, int height) +void CWallet::blockDisconnected(const CBlock& block, int height) { auto locked_chain = chain().lock(); LOCK(cs_wallet); @@ -1141,7 +1141,7 @@ void CWallet::BlockDisconnected(const CBlock& block, int height) } } -void CWallet::UpdatedBlockTip() +void CWallet::updatedBlockTip() { m_best_block_time = GetTime(); } @@ -1785,7 +1785,7 @@ bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay) // Irrespective of the failure reason, un-marking fInMempool // out-of-order is incorrect - it should be unmarked when // TransactionRemovedFromMempool fires. - bool ret = pwallet->chain().broadcastTransaction(tx, err_string, pwallet->m_default_max_tx_fee, relay); + bool ret = pwallet->chain().broadcastTransaction(tx, pwallet->m_default_max_tx_fee, relay, err_string); fInMempool |= ret; return ret; } @@ -3875,7 +3875,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, } auto locked_chain = chain.lock(); - walletInstance->ChainStateFlushed(locked_chain->getTipLocator()); + walletInstance->chainStateFlushed(locked_chain->getTipLocator()); } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) { // Make it impossible to disable private keys after creation error = strprintf(_("Error loading %s: Private keys can only be disabled during creation").translated, walletFile); @@ -4056,7 +4056,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, return nullptr; } } - walletInstance->ChainStateFlushed(locked_chain->getTipLocator()); + walletInstance->chainStateFlushed(locked_chain->getTipLocator()); walletInstance->database->IncrementUpdateCounter(); // Restore wallet transaction metadata after -zapwallettxes=1 diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1fb72c83cd..75fd14a80e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -875,10 +875,10 @@ public: void MarkDirty(); bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true); void LoadToWallet(CWalletTx& wtxIn) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void TransactionAddedToMempool(const CTransactionRef& tx) override; - void BlockConnected(const CBlock& block, int height) override; - void BlockDisconnected(const CBlock& block, int height) override; - void UpdatedBlockTip() override; + void transactionAddedToMempool(const CTransactionRef& tx) override; + void blockConnected(const CBlock& block, int height) override; + void blockDisconnected(const CBlock& block, int height) override; + void updatedBlockTip() override; int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update); struct ScanResult { @@ -897,7 +897,7 @@ public: uint256 last_failed_block; }; ScanResult ScanForWalletTransactions(const uint256& first_block, const uint256& last_block, const WalletRescanReserver& reserver, bool fUpdate); - void TransactionRemovedFromMempool(const CTransactionRef &ptx) override; + void transactionRemovedFromMempool(const CTransactionRef &ptx) override; void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void ResendWalletTransactions(); struct Balance { @@ -1033,7 +1033,7 @@ public: bool IsAllFromMe(const CTransaction& tx, const isminefilter& filter) const; CAmount GetCredit(const CTransaction& tx, const isminefilter& filter) const; CAmount GetChange(const CTransaction& tx) const; - void ChainStateFlushed(const CBlockLocator& loc) override; + void chainStateFlushed(const CBlockLocator& loc) override; DBErrors LoadWallet(bool& fFirstRunRet); DBErrors ZapWalletTx(std::vector<CWalletTx>& vWtx); diff --git a/test/functional/feature_abortnode.py b/test/functional/feature_abortnode.py index 9b878e8bf8..e47e709431 100755 --- a/test/functional/feature_abortnode.py +++ b/test/functional/feature_abortnode.py @@ -14,11 +14,12 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import wait_until, get_datadir_path, connect_nodes import os -class AbortNodeTest(BitcoinTestFramework): +class AbortNodeTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 + self.rpc_timeout = 240 def setup_network(self): self.setup_nodes() @@ -44,5 +45,6 @@ class AbortNodeTest(BitcoinTestFramework): self.log.info("Node crashed - now verifying restart fails") self.nodes[0].assert_start_raises_init_error() + if __name__ == '__main__': AbortNodeTest().main() diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 9e8e11e64f..0c591de869 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -1263,7 +1263,7 @@ class FullBlockTest(BitcoinTestFramework): self.save_spendable_output() spend = self.get_spendable_output() - self.send_blocks(blocks, True, timeout=1920) + self.send_blocks(blocks, True, timeout=2440) chain1_tip = i # now create alt chain of same length @@ -1275,14 +1275,14 @@ class FullBlockTest(BitcoinTestFramework): # extend alt chain to trigger re-org block = self.next_block("alt" + str(chain1_tip + 1), version=4) - self.send_blocks([block], True, timeout=1920) + self.send_blocks([block], True, timeout=2440) # ... and re-org back to the first chain self.move_tip(chain1_tip) block = self.next_block(chain1_tip + 1, version=4) self.send_blocks([block], False, force_send=True) block = self.next_block(chain1_tip + 2, version=4) - self.send_blocks([block], True, timeout=1920) + self.send_blocks([block], True, timeout=2440) self.log.info("Reject a block with an invalid block header version") b_v1 = self.next_block('b_v1', version=1) diff --git a/test/functional/rpc_estimatefee.py b/test/functional/rpc_estimatefee.py index 8bdecfc8cd..8bdecfc8cd 100644..100755 --- a/test/functional/rpc_estimatefee.py +++ b/test/functional/rpc_estimatefee.py diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 3bac8d5d95..ce9b37edfb 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -19,9 +19,8 @@ import datetime import os import time import shutil -import signal -import sys import subprocess +import sys import tempfile import re import logging @@ -183,7 +182,6 @@ BASE_SCRIPTS = [ 'rpc_bind.py --nonloopback', 'mining_basic.py', 'wallet_bumpfee.py', - 'wallet_bumpfee_totalfee_deprecation.py', 'wallet_implicitsegwit.py', 'rpc_named_arguments.py', 'wallet_listsinceblock.py', @@ -366,11 +364,10 @@ def main(): args=passon_args, combined_logs_len=args.combinedlogslen, failfast=args.failfast, - runs_ci=args.ci, use_term_control=args.ansi, ) -def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=False, args=None, combined_logs_len=0, failfast=False, runs_ci, use_term_control): +def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=False, args=None, combined_logs_len=0, failfast=False, use_term_control): args = args or [] # Warn if bitcoind is already running @@ -412,7 +409,6 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage= tmpdir=tmpdir, test_list=test_list, flags=flags, - timeout_duration=40 * 60 if runs_ci else float('inf'), # in seconds use_term_control=use_term_control, ) start_time = time.time() @@ -497,12 +493,11 @@ class TestHandler: Trigger the test scripts passed in via the list. """ - def __init__(self, *, num_tests_parallel, tests_dir, tmpdir, test_list, flags, timeout_duration, use_term_control): + def __init__(self, *, num_tests_parallel, tests_dir, tmpdir, test_list, flags, use_term_control): assert num_tests_parallel >= 1 self.num_jobs = num_tests_parallel self.tests_dir = tests_dir self.tmpdir = tmpdir - self.timeout_duration = timeout_duration self.test_list = test_list self.flags = flags self.num_running = 0 @@ -543,10 +538,6 @@ class TestHandler: time.sleep(.5) for job in self.jobs: (name, start_time, proc, testdir, log_out, log_err) = job - if int(time.time() - start_time) > self.timeout_duration: - # Timeout individual tests if timeout is specified (to stop - # tests hanging and not providing useful output). - proc.send_signal(signal.SIGINT) if proc.poll() is not None: log_out.seek(0), log_err.seek(0) [stdout, stderr] = [log_file.read().decode('utf-8') for log_file in (log_out, log_err)] diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 336e246e33..38c9807757 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -30,6 +30,13 @@ from test_framework.util import ( WALLET_PASSPHRASE = "test" WALLET_PASSPHRASE_TIMEOUT = 3600 +# Fee rates (in BTC per 1000 vbytes) +INSUFFICIENT = 0.00001000 +ECONOMICAL = 0.00050000 +NORMAL = 0.00100000 +HIGH = 0.00500000 +TOO_HIGH = 1.00000000 + class BumpFeeTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 @@ -37,7 +44,6 @@ class BumpFeeTest(BitcoinTestFramework): self.extra_args = [[ "-walletrbf={}".format(i), "-mintxfee=0.00002", - "-deprecatedrpc=totalFee", "-addresstype=bech32", ] for i in range(self.num_nodes)] @@ -75,7 +81,6 @@ class BumpFeeTest(BitcoinTestFramework): test_nonrbf_bumpfee_fails(self, peer_node, dest_address) test_notmine_bumpfee_fails(self, rbf_node, peer_node, dest_address) test_bumpfee_with_descendant_fails(self, rbf_node, rbf_node_address, dest_address) - test_small_output_fails(self, rbf_node, dest_address) test_dust_to_fee(self, rbf_node, dest_address) test_settxfee(self, rbf_node, dest_address) test_watchonly_psbt(self, peer_node, rbf_node, dest_address) @@ -93,13 +98,13 @@ class BumpFeeTest(BitcoinTestFramework): def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address): - self.log.info('Test simple bumpfee') + self.log.info('Test simple bumpfee: {}'.format(mode)) rbfid = spend_one_input(rbf_node, dest_address) rbftx = rbf_node.gettransaction(rbfid) self.sync_mempools((rbf_node, peer_node)) assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool() if mode == "fee_rate": - bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate":0.0015}) + bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate": NORMAL}) else: bumped_tx = rbf_node.bumpfee(rbfid) assert_equal(bumped_tx["errors"], []) @@ -120,21 +125,21 @@ def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address): assert_equal(bumpedwtx["replaces_txid"], rbfid) def test_feerate_args(self, rbf_node, peer_node, dest_address): - self.log.info('Test feerate args') + self.log.info('Test fee_rate args') rbfid = spend_one_input(rbf_node, dest_address) self.sync_mempools((rbf_node, peer_node)) assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool() - assert_raises_rpc_error(-8, "confTarget can't be set with totalFee or fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.", rbf_node.bumpfee, rbfid, {"fee_rate":0.00001, "confTarget":1}) - assert_raises_rpc_error(-8, "confTarget can't be set with totalFee or fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.", rbf_node.bumpfee, rbfid, {"totalFee":0.00001, "confTarget":1}) - assert_raises_rpc_error(-8, "fee_rate can't be set along with totalFee.", rbf_node.bumpfee, rbfid, {"fee_rate":0.00001, "totalFee":0.001}) + assert_raises_rpc_error(-8, "confTarget can't be set with fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.", rbf_node.bumpfee, rbfid, {"fee_rate": NORMAL, "confTarget": 1}) + + assert_raises_rpc_error(-3, "Unexpected key totalFee", rbf_node.bumpfee, rbfid, {"totalFee": NORMAL}) # Bumping to just above minrelay should fail to increase total fee enough, at least - assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, rbfid, {"fee_rate":0.00001000}) + assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, rbfid, {"fee_rate": INSUFFICIENT}) assert_raises_rpc_error(-3, "Amount out of range", rbf_node.bumpfee, rbfid, {"fee_rate":-1}) - assert_raises_rpc_error(-4, "is too high (cannot be higher than", rbf_node.bumpfee, rbfid, {"fee_rate":1}) + assert_raises_rpc_error(-4, "is too high (cannot be higher than", rbf_node.bumpfee, rbfid, {"fee_rate": TOO_HIGH}) def test_segwit_bumpfee_succeeds(self, rbf_node, dest_address): @@ -204,15 +209,6 @@ def test_bumpfee_with_descendant_fails(self, rbf_node, rbf_node_address, dest_ad rbf_node.sendrawtransaction(tx["hex"]) assert_raises_rpc_error(-8, "Transaction has descendants in the wallet", rbf_node.bumpfee, parent_id) -def test_small_output_fails(self, rbf_node, dest_address): - self.log.info('Test totalFee bump with small output fails') - # cannot bump fee with a too-small output - rbfid = spend_one_input(rbf_node, dest_address) - rbf_node.bumpfee(rbfid, {"totalFee": 50000}) - - rbfid = spend_one_input(rbf_node, dest_address) - assert_raises_rpc_error(-4, "Change output is too small", rbf_node.bumpfee, rbfid, {"totalFee": 50001}) - def test_small_output_with_feerate_succeeds(self, rbf_node, dest_address): self.log.info('Testing small output with feerate bump succeeds') @@ -255,15 +251,19 @@ def test_small_output_with_feerate_succeeds(self, rbf_node, dest_address): def test_dust_to_fee(self, rbf_node, dest_address): self.log.info('Test that bumped output that is dust is dropped to fee') - # the bumped tx sets fee=49,900, but it converts to 50,000 rbfid = spend_one_input(rbf_node, dest_address) fulltx = rbf_node.getrawtransaction(rbfid, 1) - # (31-vbyte p2wpkh output size + 67-vbyte p2wpkh spend estimate) * 10k(discard_rate) / 1000 = 980 - bumped_tx = rbf_node.bumpfee(rbfid, {"totalFee": 50000 - 980}) + # size of transaction (p2wpkh, 1 input, 2 outputs): 141 vbytes + assert_equal(fulltx["vsize"], 141) + # bump with fee_rate of 0.00350000 BTC per 1000 vbytes + # expected bump fee of 141 vbytes * fee_rate 0.00350000 BTC / 1000 vbytes = 0.00049350 BTC + # but dust is dropped, so actual bump fee is 0.00050000 + bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate": 0.0035}) full_bumped_tx = rbf_node.getrawtransaction(bumped_tx["txid"], 1) assert_equal(bumped_tx["fee"], Decimal("0.00050000")) assert_equal(len(fulltx["vout"]), 2) assert_equal(len(full_bumped_tx["vout"]), 1) # change output is eliminated + assert_equal(full_bumped_tx["vout"][0]['value'], Decimal("0.00050000")) def test_settxfee(self, rbf_node, dest_address): @@ -285,7 +285,8 @@ def test_settxfee(self, rbf_node, dest_address): def test_maxtxfee_fails(self, rbf_node, dest_address): self.log.info('Test that bumpfee fails when it hits -matxfee') # size of bumped transaction (p2wpkh, 1 input, 2 outputs): 141 vbytes - # expected bumping feerate of 20 sats/vbyte => 141*20 sats = 0.00002820 btc + # expected bump fee of 141 vbytes * 0.00200000 BTC / 1000 vbytes = 0.00002820 BTC + # which exceeds maxtxfee and is expected to raise self.restart_node(1, ['-maxtxfee=0.000025'] + self.extra_args[1]) rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT) rbfid = spend_one_input(rbf_node, dest_address) @@ -356,7 +357,7 @@ def test_watchonly_psbt(self, peer_node, rbf_node, dest_address): assert_equal(len(watcher.decodepsbt(psbt)["tx"]["vin"]), 1) # Bump fee, obnoxiously high to add additional watchonly input - bumped_psbt = watcher.bumpfee(original_txid, {"fee_rate":0.005}) + bumped_psbt = watcher.bumpfee(original_txid, {"fee_rate": HIGH}) assert_greater_than(len(watcher.decodepsbt(bumped_psbt['psbt'])["tx"]["vin"]), 1) assert "txid" not in bumped_psbt assert_equal(bumped_psbt["origfee"], -watcher.gettransaction(original_txid)["fee"]) @@ -378,17 +379,17 @@ def test_watchonly_psbt(self, peer_node, rbf_node, dest_address): def test_rebumping(self, rbf_node, dest_address): self.log.info('Test that re-bumping the original tx fails, but bumping successor works') rbfid = spend_one_input(rbf_node, dest_address) - bumped = rbf_node.bumpfee(rbfid, {"totalFee": 2000}) - assert_raises_rpc_error(-4, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 3000}) - rbf_node.bumpfee(bumped["txid"], {"totalFee": 3000}) + bumped = rbf_node.bumpfee(rbfid, {"fee_rate": ECONOMICAL}) + assert_raises_rpc_error(-4, "already bumped", rbf_node.bumpfee, rbfid, {"fee_rate": NORMAL}) + rbf_node.bumpfee(bumped["txid"], {"fee_rate": NORMAL}) def test_rebumping_not_replaceable(self, rbf_node, dest_address): self.log.info('Test that re-bumping non-replaceable fails') rbfid = spend_one_input(rbf_node, dest_address) - bumped = rbf_node.bumpfee(rbfid, {"totalFee": 10000, "replaceable": False}) + bumped = rbf_node.bumpfee(rbfid, {"fee_rate": ECONOMICAL, "replaceable": False}) assert_raises_rpc_error(-4, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"], - {"totalFee": 20000}) + {"fee_rate": NORMAL}) def test_unconfirmed_not_spendable(self, rbf_node, rbf_node_address): @@ -450,7 +451,7 @@ def test_locked_wallet_fails(self, rbf_node, dest_address): rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT) def test_change_script_match(self, rbf_node, dest_address): - self.log.info('Test that the same change addresses is used for the replacement transaction when possible.') + self.log.info('Test that the same change addresses is used for the replacement transaction when possible') def get_change_address(tx): tx_details = rbf_node.getrawtransaction(tx, 1) @@ -463,7 +464,7 @@ def test_change_script_match(self, rbf_node, dest_address): assert_equal(len(change_addresses), 1) # Now find that address in each subsequent tx, and no other change - bumped_total_tx = rbf_node.bumpfee(rbfid, {"totalFee": 2000}) + bumped_total_tx = rbf_node.bumpfee(rbfid, {"fee_rate": ECONOMICAL}) assert_equal(change_addresses, get_change_address(bumped_total_tx['txid'])) bumped_rate_tx = rbf_node.bumpfee(bumped_total_tx["txid"]) assert_equal(change_addresses, get_change_address(bumped_rate_tx['txid'])) @@ -503,5 +504,6 @@ def test_no_more_inputs_fails(self, rbf_node, dest_address): rbfid = rbf_node.sendtoaddress(rbf_node.getnewaddress(), rbf_node.getbalance(), "", "", True) assert_raises_rpc_error(-4, "Unable to create transaction: Insufficient funds", rbf_node.bumpfee, rbfid) + if __name__ == "__main__": BumpFeeTest().main() diff --git a/test/functional/wallet_bumpfee_totalfee_deprecation.py b/test/functional/wallet_bumpfee_totalfee_deprecation.py deleted file mode 100755 index b8e097c32e..0000000000 --- a/test/functional/wallet_bumpfee_totalfee_deprecation.py +++ /dev/null @@ -1,54 +0,0 @@ -#!/usr/bin/env python3 -# 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. -"""Test deprecation of passing `totalFee` to the bumpfee RPC.""" -from decimal import Decimal - -from test_framework.messages import BIP125_SEQUENCE_NUMBER -from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_raises_rpc_error - -class BumpFeeWithTotalFeeArgumentDeprecationTest(BitcoinTestFramework): - def set_test_params(self): - self.num_nodes = 2 - self.extra_args = [[ - "-walletrbf={}".format(i), - "-mintxfee=0.00002", - ] for i in range(self.num_nodes)] - - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() - - def run_test(self): - peer_node, rbf_node = self.nodes - peer_node.generate(110) - self.sync_all() - peer_node.sendtoaddress(rbf_node.getnewaddress(), 0.001) - self.sync_all() - peer_node.generate(1) - self.sync_all() - rbfid = spend_one_input(rbf_node, peer_node.getnewaddress()) - - self.log.info("Testing bumpfee with totalFee argument raises RPC error with deprecation message") - assert_raises_rpc_error( - -8, - "totalFee argument has been deprecated and will be removed in 0.20. " + - "Please use -deprecatedrpc=totalFee to continue using this argument until removal.", - rbf_node.bumpfee, rbfid, {"totalFee": 2000}) - - self.log.info("Testing bumpfee without totalFee argument does not raise") - rbf_node.bumpfee(rbfid) - -def spend_one_input(node, dest_address, change_size=Decimal("0.00049000")): - tx_input = dict(sequence=BIP125_SEQUENCE_NUMBER, - **next(u for u in node.listunspent() if u["amount"] == Decimal("0.00100000"))) - destinations = {dest_address: Decimal("0.00050000")} - destinations[node.getrawchangeaddress()] = change_size - rawtx = node.createrawtransaction([tx_input], destinations) - signedtx = node.signrawtransactionwithwallet(rawtx) - txid = node.sendrawtransaction(signedtx["hex"]) - return txid - -if __name__ == "__main__": - BumpFeeWithTotalFeeArgumentDeprecationTest().main() diff --git a/test/lint/lint-locale-dependence.sh b/test/lint/lint-locale-dependence.sh index 0cb38b6fdb..70410d7405 100755 --- a/test/lint/lint-locale-dependence.sh +++ b/test/lint/lint-locale-dependence.sh @@ -6,31 +6,16 @@ export LC_ALL=C KNOWN_VIOLATIONS=( "src/bitcoin-tx.cpp.*stoul" - "src/bitcoin-tx.cpp.*std::to_string" "src/bitcoin-tx.cpp.*trim_right" "src/dbwrapper.cpp.*stoul" "src/dbwrapper.cpp:.*vsnprintf" "src/httprpc.cpp.*trim" "src/init.cpp:.*atoi" - "src/qt/optionsmodel.cpp.*std::to_string" "src/qt/rpcconsole.cpp:.*atoi" "src/rest.cpp:.*strtol" - "src/rpc/net.cpp.*std::to_string" - "src/rpc/rawtransaction.cpp.*std::to_string" - "src/rpc/util.cpp.*std::to_string" - "src/test/addrman_tests.cpp.*std::to_string" - "src/test/blockchain_tests.cpp.*std::to_string" "src/test/dbwrapper_tests.cpp:.*snprintf" - "src/test/denialofservice_tests.cpp.*std::to_string" "src/test/fuzz/locale.cpp" "src/test/fuzz/parse_numbers.cpp:.*atoi" - "src/test/key_tests.cpp.*std::to_string" - "src/test/net_tests.cpp.*std::to_string" - "src/test/settings_tests.cpp.*std::to_string" - "src/test/timedata_tests.cpp.*std::to_string" - "src/test/util/setup_common.cpp.*std::to_string" - "src/test/util_tests.cpp.*std::to_string" - "src/test/util_threadnames_tests.cpp.*std::to_string" "src/torcontrol.cpp:.*atoi" "src/torcontrol.cpp:.*strtol" "src/util/strencodings.cpp:.*atoi" @@ -38,7 +23,6 @@ KNOWN_VIOLATIONS=( "src/util/strencodings.cpp:.*strtoul" "src/util/strencodings.h:.*atoi" "src/util/system.cpp:.*atoi" - "src/wallet/scriptpubkeyman.cpp.*std::to_string" ) REGEXP_IGNORE_EXTERNAL_DEPENDENCIES="^src/(crypto/ctaes/|leveldb/|secp256k1/|tinyformat.h|univalue/)" |