diff options
90 files changed, 1454 insertions, 710 deletions
diff --git a/build-aux/m4/bitcoin_find_bdb48.m4 b/build-aux/m4/bitcoin_find_bdb48.m4 index ea9c795daa..aa0111e5a2 100644 --- a/build-aux/m4/bitcoin_find_bdb48.m4 +++ b/build-aux/m4/bitcoin_find_bdb48.m4 @@ -61,7 +61,7 @@ AC_DEFUN([BITCOIN_FIND_BDB48],[ BDB_CPPFLAGS=${BDB_CFLAGS} fi AC_SUBST(BDB_CPPFLAGS) - + if test "x$BDB_LIBS" = "x"; then # TODO: Ideally this could find the library version and make sure it matches the headers being used for searchlib in db_cxx-4.8 db_cxx db4_cxx; do diff --git a/ci/lint/04_install.sh b/ci/lint/04_install.sh index 01322f61e6..6c3019a1aa 100755 --- a/ci/lint/04_install.sh +++ b/ci/lint/04_install.sh @@ -8,7 +8,7 @@ export LC_ALL=C travis_retry pip3 install codespell==1.15.0 travis_retry pip3 install flake8==3.7.8 -travis_retry pip3 install vulture==0.29 +travis_retry pip3 install vulture==1.0 SHELLCHECK_VERSION=v0.6.0 curl -s "https://storage.googleapis.com/shellcheck/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar --xz -xf - --directory /tmp/ diff --git a/ci/test/00_setup_env_i686.sh b/ci/test/00_setup_env_i686.sh index c5541eaf23..63068dc95d 100644 --- a/ci/test/00_setup_env_i686.sh +++ b/ci/test/00_setup_env_i686.sh @@ -7,6 +7,7 @@ export LC_ALL=C.UTF-8 export HOST=i686-pc-linux-gnu +export DEP_OPTS="PROTOBUF=1" export PACKAGES="g++-multilib python3-zmq" export GOAL="install" export BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-bip70 --enable-glibc-back-compat --enable-reduce-exports LDFLAGS=-static-libstdc++" diff --git a/configure.ac b/configure.ac index e1b7281c30..2445b72683 100644 --- a/configure.ac +++ b/configure.ac @@ -777,6 +777,39 @@ fi AC_CHECK_HEADERS([endian.h sys/endian.h byteswap.h stdio.h stdlib.h unistd.h strings.h sys/types.h sys/stat.h sys/select.h sys/prctl.h]) +# FD_ZERO may be dependent on a declaration of memcpy, e.g. in SmartOS +# check that it fails to build without memcpy, then that it builds with +AC_MSG_CHECKING(FD_ZERO memcpy dependence) +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ + #include <cstddef> + #if HAVE_SYS_SELECT_H + #include <sys/select.h> + #endif + ]],[[ + #if HAVE_SYS_SELECT_H + fd_set fds; + FD_ZERO(&fds); + #endif + ]])], + [ AC_MSG_RESULT(no) ], + [ + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ + #include <cstring> + #if HAVE_SYS_SELECT_H + #include <sys/select.h> + #endif + ]], [[ + #if HAVE_SYS_SELECT_H + fd_set fds; + FD_ZERO(&fds); + #endif + ]])], + [ AC_MSG_RESULT(yes); AC_DEFINE(HAVE_CSTRING_DEPENDENT_FD_ZERO, 1, [Define this symbol if FD_ZERO is dependent of a memcpy declaration being available]) ], + [ AC_MSG_ERROR(failed with cstring include) ] + ) + ] +) + AC_CHECK_DECLS([getifaddrs, freeifaddrs],,, [#include <sys/types.h> #include <ifaddrs.h>] diff --git a/contrib/README.md b/contrib/README.md index e9e72f6686..361975baa4 100644 --- a/contrib/README.md +++ b/contrib/README.md @@ -33,12 +33,12 @@ Files used during the gitian build process. For more information about gitian, s PGP keys used for signing Bitcoin Core [Gitian release](/doc/release-process.md) results. ### [MacDeploy](/contrib/macdeploy) ### -Scripts and notes for Mac builds. +Scripts and notes for Mac builds. ### [Gitian-build](/contrib/gitian-build.py) ### Script for running full Gitian builds. -Test and Verify Tools +Test and Verify Tools --------------------- ### [TestGen](/contrib/testgen) ### diff --git a/contrib/macdeploy/README.md b/contrib/macdeploy/README.md index 6163734e62..e90120ea79 100644 --- a/contrib/macdeploy/README.md +++ b/contrib/macdeploy/README.md @@ -1,7 +1,7 @@ ### MacDeploy ### For Snow Leopard (which uses [Python 2.6](http://www.python.org/download/releases/2.6/)), you will need the param_parser package: - + sudo easy_install argparse This script should not be run manually, instead, after building as usual: diff --git a/contrib/testgen/README.md b/contrib/testgen/README.md index 580ed541cf..573a71a675 100644 --- a/contrib/testgen/README.md +++ b/contrib/testgen/README.md @@ -2,7 +2,7 @@ Utilities to generate test vectors for the data-driven Bitcoin tests. -Usage: +Usage: PYTHONPATH=../../test/functional/test_framework ./gen_key_io_test_vectors.py valid 50 > ../../src/test/data/key_io_keys_valid.json PYTHONPATH=../../test/functional/test_framework ./gen_key_io_test_vectors.py invalid 50 > ../../src/test/data/key_io_keys_invalid.json diff --git a/depends/Makefile b/depends/Makefile index 70af875189..b7e9a9213e 100644 --- a/depends/Makefile +++ b/depends/Makefile @@ -5,6 +5,7 @@ WORK_PATH = $(BASEDIR)/work BASE_CACHE ?= $(BASEDIR)/built SDK_PATH ?= $(BASEDIR)/SDKs NO_QT ?= +PROTOBUF ?= RAPIDCHECK ?= NO_WALLET ?= NO_ZMQ ?= @@ -96,13 +97,15 @@ wallet_packages_$(NO_WALLET) = $(wallet_packages) upnp_packages_$(NO_UPNP) = $(upnp_packages) zmq_packages_$(NO_ZMQ) = $(zmq_packages) +protobuf_packages_$(PROTOBUF) = $(protobuf_packages) rapidcheck_packages_$(RAPIDCHECK) = $(rapidcheck_packages) packages += $($(host_arch)_$(host_os)_packages) $($(host_os)_packages) $(qt_packages_) $(wallet_packages_) $(upnp_packages_) native_packages += $($(host_arch)_$(host_os)_native_packages) $($(host_os)_native_packages) -ifneq ($(qt_packages_),) -native_packages += $(qt_native_packages) +ifeq ($(protobuf_packages_),) +native_packages += $(protobuf_native_packages) +packages += $(protobuf_packages) endif ifneq ($(zmq_packages_),) @@ -150,6 +153,7 @@ $(host_prefix)/share/config.site : config.site.in $(host_prefix)/.stamp_$(final_ -e 's|@allow_host_packages@|$(ALLOW_HOST_PACKAGES)|' \ -e 's|@no_qt@|$(NO_QT)|' \ -e 's|@no_zmq@|$(NO_ZMQ)|' \ + -e 's|@enable_bip70@|$(PROTOBUF)|' \ -e 's|@no_wallet@|$(NO_WALLET)|' \ -e 's|@no_upnp@|$(NO_UPNP)|' \ -e 's|@debug@|$(DEBUG)|' \ diff --git a/depends/README.md b/depends/README.md index ca542be13f..cfb9bbfeb0 100644 --- a/depends/README.md +++ b/depends/README.md @@ -77,6 +77,7 @@ The following can be set when running make: make FOO=bar NO_UPNP: Don't download/build/cache packages needed for enabling upnp DEBUG: disable some optimizations and enable more runtime checking RAPIDCHECK: build rapidcheck (experimental, requires cmake) + PROTOBUF: build protobuf (used for deprecated BIP70 support) HOST_ID_SALT: Optional salt to use when generating host package ids BUILD_ID_SALT: Optional salt to use when generating build package ids diff --git a/depends/config.site.in b/depends/config.site.in index e633752066..d0d36641c4 100644 --- a/depends/config.site.in +++ b/depends/config.site.in @@ -37,6 +37,10 @@ if test -z $enable_zmq && test -n "@no_zmq@"; then enable_zmq=no fi +if test -n $enable_bip70 && test -n "@enable_bip70@"; then + enable_bip70=yes +fi + if test x@host_os@ = xdarwin; then BREW=no PORT=no diff --git a/depends/description.md b/depends/description.md index 9fc7093be4..0a6f2e6442 100644 --- a/depends/description.md +++ b/depends/description.md @@ -1,4 +1,4 @@ -This is a system of building and caching dependencies necessary for building Bitcoin. +This is a system of building and caching dependencies necessary for building Bitcoin. There are several features that make it different from most similar systems: ### It is designed to be builder and host agnostic @@ -26,7 +26,7 @@ Before building, a unique build-id is generated for each package. This id consists of a hash of all files used to build the package (Makefiles, packages, etc), and as well as a hash of the same data for each recursive dependency. If any portion of a package's build recipe changes, it will be rebuilt as well as -any other package that depends on it. If any of the main makefiles (Makefile, +any other package that depends on it. If any of the main makefiles (Makefile, funcs.mk, etc) are changed, all packages will be rebuilt. After building, the results are cached into a tarball that can be re-used and distributed. diff --git a/depends/packages.md b/depends/packages.md index 36c9967a0a..7ed20ea129 100644 --- a/depends/packages.md +++ b/depends/packages.md @@ -32,15 +32,15 @@ These variables are optional: $(package)_build_subdir: cd to this dir before running configure/build/stage commands. - + $(package)_download_file: The file-name of the upstream source if it differs from how it should be stored locally. This can be used to avoid storing file-names with strange characters. - + $(package)_dependencies: Names of any other packages that this one depends on. - + $(package)_patches: Filenames of any patches needed to build the package @@ -134,7 +134,7 @@ the user. Other variables may be defined as needed. Stage the build results. If undefined, does nothing. The following variables are available for each recipe: - + $(1)_staging_dir: package's destination sysroot path $(1)_staging_prefix_dir: prefix path inside of the package's staging dir $(1)_extract_dir: path to the package's extracted sources diff --git a/depends/packages/packages.mk b/depends/packages/packages.mk index 9edcd1eb38..667fde5271 100644 --- a/depends/packages/packages.mk +++ b/depends/packages/packages.mk @@ -1,7 +1,9 @@ packages:=boost openssl libevent -qt_native_packages = native_protobuf -qt_packages = qrencode protobuf zlib +protobuf_native_packages = native_protobuf +protobuf_packages = protobuf + +qt_packages = qrencode zlib qt_linux_packages:=qt expat libxcb xcb_proto libXau xproto freetype fontconfig diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 57208a678a..56045ade50 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -156,9 +156,7 @@ define $(package)_preprocess_cmds sed -i.old "s|updateqm.commands = \$$$$\$$$$LRELEASE|updateqm.commands = $($(package)_extract_dir)/qttools/bin/lrelease|" qttranslations/translations/translations.pro && \ sed -i.old "/updateqm.depends =/d" qttranslations/translations/translations.pro && \ sed -i.old "s/src_plugins.depends = src_sql src_network/src_plugins.depends = src_network/" qtbase/src/src.pro && \ - sed -i.old "s|X11/extensions/XIproto.h|X11/X.h|" qtbase/src/plugins/platforms/xcb/qxcbxsettings.cpp && \ sed -i.old -e 's/if \[ "$$$$XPLATFORM_MAC" = "yes" \]; then xspecvals=$$$$(macSDKify/if \[ "$$$$BUILD_ON_MAC" = "yes" \]; then xspecvals=$$$$(macSDKify/' -e 's|/bin/pwd|pwd|' qtbase/configure && \ - sed -i.old 's/CGEventCreateMouseEvent(0, kCGEventMouseMoved, pos, 0)/CGEventCreateMouseEvent(0, kCGEventMouseMoved, pos, kCGMouseButtonLeft)/' qtbase/src/plugins/platforms/cocoa/qcocoacursor.mm && \ mkdir -p qtbase/mkspecs/macx-clang-linux &&\ cp -f qtbase/mkspecs/macx-clang/Info.plist.lib qtbase/mkspecs/macx-clang-linux/ &&\ cp -f qtbase/mkspecs/macx-clang/Info.plist.app qtbase/mkspecs/macx-clang-linux/ &&\ diff --git a/depends/packages/zlib.mk b/depends/packages/zlib.mk index 1600b11a01..168f85e65e 100644 --- a/depends/packages/zlib.mk +++ b/depends/packages/zlib.mk @@ -5,23 +5,26 @@ $(package)_file_name=$(package)-$($(package)_version).tar.gz $(package)_sha256_hash=c3e5e9fdd5004dcb542feda5ee4f0ff0744628baf8ed2dd5d66f8ca1197cb1a1 define $(package)_set_vars -$(package)_build_opts= CC="$($(package)_cc)" -$(package)_build_opts+=CFLAGS="$($(package)_cflags) $($(package)_cppflags) -fPIC" -$(package)_build_opts+=RANLIB="$($(package)_ranlib)" -$(package)_build_opts+=AR="$($(package)_ar)" -$(package)_build_opts_darwin+=AR="$($(package)_libtool)" -$(package)_build_opts_darwin+=ARFLAGS="-o" +$(package)_config_opts= CC="$($(package)_cc)" +$(package)_config_opts+=CFLAGS="$($(package)_cflags) $($(package)_cppflags) -fPIC" +$(package)_config_opts+=RANLIB="$($(package)_ranlib)" +$(package)_config_opts+=AR="$($(package)_ar)" +$(package)_config_opts_darwin+=AR="$($(package)_libtool)" +$(package)_config_opts_darwin+=ARFLAGS="-o" endef +# zlib has its own custom configure script that takes in options like CC, +# CFLAGS, RANLIB, AR, and ARFLAGS from the environment rather than from +# command-line arguments. define $(package)_config_cmds - ./configure --static --prefix=$(host_prefix) + env $($(package)_config_opts) ./configure --static --prefix=$(host_prefix) endef define $(package)_build_cmds - $(MAKE) $($(package)_build_opts) libz.a + $(MAKE) libz.a endef define $(package)_stage_cmds - $(MAKE) DESTDIR=$($(package)_staging_dir) install $($(package)_build_opts) + $(MAKE) DESTDIR=$($(package)_staging_dir) install endef diff --git a/doc/REST-interface.md b/doc/REST-interface.md index c96871ab5f..3b8b0db162 100644 --- a/doc/REST-interface.md +++ b/doc/REST-interface.md @@ -81,7 +81,7 @@ $ curl localhost:18332/rest/getutxos/checkmempool/b2cdfd7b89def827ff8af7cd9bff76 { "txvers" : 1 "height" : 2147483647, - "value" : 8.8687, + "value" : 8.8687, "scriptPubKey" : { "asm" : "OP_DUP OP_HASH160 1c7cebb529b86a04c683dfa87be49de35bcf589e OP_EQUALVERIFY OP_CHECKSIG", "hex" : "76a9141c7cebb529b86a04c683dfa87be49de35bcf589e88ac", diff --git a/doc/build-unix.md b/doc/build-unix.md index 8f6c702f5c..069c983e6e 100644 --- a/doc/build-unix.md +++ b/doc/build-unix.md @@ -44,7 +44,7 @@ Optional dependencies: miniupnpc | UPnP Support | Firewall-jumping support libdb4.8 | Berkeley DB | Wallet storage (only needed when wallet enabled) qt | GUI | GUI toolkit (only needed when GUI enabled) - protobuf | Payments in GUI | Data interchange format used for payment protocol (only needed when GUI enabled) + protobuf | Payments in GUI | Data interchange format used for payment protocol (only needed when BIP70 enabled) libqrencode | QR codes in GUI | Optional for generating QR codes (only needed when GUI enabled) univalue | Utility | JSON parsing and encoding (bundled version will be used unless --with-system-univalue passed to configure) libzmq3 | ZMQ notification | Optional, allows generating ZMQ notifications (requires ZMQ version >= 4.0.0) diff --git a/doc/dependencies.md b/doc/dependencies.md index 0b23ca0a2d..e5b4084d99 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -35,7 +35,7 @@ Some dependencies are not needed in all configurations. The following are some f #### Options passed to `./configure` * MiniUPnPc is not needed with `--with-miniupnpc=no`. * Berkeley DB is not needed with `--disable-wallet`. -* protobuf is not needed with `--disable-bip70`. +* protobuf is only needed with `--enable-bip70`. * Qt is not needed with `--without-gui`. * If the qrencode dependency is absent, QR support won't be added. To force an error when that happens, pass `--with-qrencode`. * ZeroMQ is needed only with the `--with-zmq` option. diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 561f623cd5..f4a5e2d330 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -193,7 +193,7 @@ Documentation can be generated with `make docs` and cleaned up with `make clean- Before running `make docs`, you will need to install dependencies `doxygen` and `dot`. For example, on macOS via Homebrew: ``` -brew install doxygen --with-graphviz +brew install graphviz doxygen ``` Development tips and tricks diff --git a/doc/init.md b/doc/init.md index 87e939c636..0b327aab58 100644 --- a/doc/init.md +++ b/doc/init.md @@ -53,11 +53,11 @@ Paths All three configurations assume several paths that might need to be adjusted. -Binary: `/usr/bin/bitcoind` -Configuration file: `/etc/bitcoin/bitcoin.conf` -Data directory: `/var/lib/bitcoind` +Binary: `/usr/bin/bitcoind` +Configuration file: `/etc/bitcoin/bitcoin.conf` +Data directory: `/var/lib/bitcoind` PID file: `/var/run/bitcoind/bitcoind.pid` (OpenRC and Upstart) or `/run/bitcoind/bitcoind.pid` (systemd) -Lock file: `/var/lock/subsys/bitcoind` (CentOS) +Lock file: `/var/lock/subsys/bitcoind` (CentOS) The PID directory (if applicable) and data directory should both be owned by the bitcoin user and group. It is advised for security reasons to make the @@ -83,10 +83,10 @@ OpenRC). ### macOS -Binary: `/usr/local/bin/bitcoind` -Configuration file: `~/Library/Application Support/Bitcoin/bitcoin.conf` -Data directory: `~/Library/Application Support/Bitcoin` -Lock file: `~/Library/Application Support/Bitcoin/.lock` +Binary: `/usr/local/bin/bitcoind` +Configuration file: `~/Library/Application Support/Bitcoin/bitcoin.conf` +Data directory: `~/Library/Application Support/Bitcoin` +Lock file: `~/Library/Application Support/Bitcoin/.lock` Installing Service Configuration ----------------------------------- diff --git a/doc/release-notes-16185.md b/doc/release-notes-16185.md index eeeb951e5b..2567ebea40 100644 --- a/doc/release-notes-16185.md +++ b/doc/release-notes-16185.md @@ -1,3 +1,6 @@ RPC changes ----------- -The `gettransaction` RPC now accepts a third (boolean) argument `decode`. If set to `true`, a new `decoded` field will be added to the response containing the decoded transaction. +The `gettransaction` RPC now accepts a third (boolean) argument `verbose`. If +set to `true`, a new `decoded` field will be added to the response containing +the decoded transaction. This field is equivalent to RPC `decoderawtransaction`, +or RPC `getrawtransaction` when `verbose` is passed. diff --git a/doc/release-notes-16512.md b/doc/release-notes-16512.md new file mode 100644 index 0000000000..9aa9cf36f9 --- /dev/null +++ b/doc/release-notes-16512.md @@ -0,0 +1,4 @@ +RPC changes +----------- +The RPC `joinpsbts` will shuffle the order of the inputs and outputs of the resulting joined psbt. +Previously inputs and outputs were added in the order that the PSBTs were provided which makes correlating inputs to outputs extremely easy. diff --git a/share/examples/bitcoin.conf b/share/examples/bitcoin.conf index 709d8b002b..96fb6658a0 100644 --- a/share/examples/bitcoin.conf +++ b/share/examples/bitcoin.conf @@ -1,7 +1,7 @@ ## ## bitcoin.conf configuration file. Lines beginning with # are comments. ## - + # Network-related settings: # Note that if you use testnet or regtest, particularly with the options @@ -97,7 +97,7 @@ # rpcauth=bob:b2dd077cb54591a2f3139e69a897ac$4e71f08d48b4347cf8eff3815c0e25ae2e9a4340474079f55705f40574f4ec99 # How many seconds bitcoin will wait for a complete RPC HTTP request. -# after the HTTP connection is established. +# after the HTTP connection is established. #rpcclienttimeout=30 # By default, only RPC connections from localhost are allowed. @@ -108,7 +108,7 @@ # because the rpcpassword is transmitted over the network unencrypted. # server=1 tells Bitcoin-Qt to accept JSON-RPC commands. -# it is also read by bitcoind to determine if RPC should be enabled +# it is also read by bitcoind to determine if RPC should be enabled #rpcallowip=10.1.1.34/255.255.255.0 #rpcallowip=1.2.3.4/24 #rpcallowip=2001:db8:85a3:0:0:8a2e:370:7334/96 @@ -139,11 +139,11 @@ # both prior transactions and several dozen future transactions. #keypool=100 -# Enable pruning to reduce storage requirements by deleting old blocks. +# Enable pruning to reduce storage requirements by deleting old blocks. # This mode is incompatible with -txindex and -rescan. # 0 = default (no pruning). # 1 = allows manual pruning via RPC. -# >=550 = target to stay under in MiB. +# >=550 = target to stay under in MiB. #prune=550 # User interface options diff --git a/share/qt/Info.plist.in b/share/qt/Info.plist.in index 3cd130ce48..477985bb33 100644 --- a/share/qt/Info.plist.in +++ b/share/qt/Info.plist.in @@ -30,7 +30,7 @@ <key>CFBundleExecutable</key> <string>Bitcoin-Qt</string> - + <key>CFBundleName</key> <string>Bitcoin-Qt</string> @@ -99,7 +99,7 @@ <key>NSRequiresAquaSystemAppearance</key> <string>True</string> - + <key>LSApplicationCategoryType</key> <string>public.app-category.finance</string> </dict> diff --git a/src/Makefile.am b/src/Makefile.am index 8fc7f61d4b..1ef62a656d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -484,6 +484,7 @@ libbitcoin_util_a_SOURCES = \ support/lockedpool.cpp \ chainparamsbase.cpp \ clientversion.cpp \ + compat/glibc_sanity_fdelt.cpp \ compat/glibc_sanity.cpp \ compat/glibcxx_sanity.cpp \ compat/strnlen.cpp \ diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index 361fedf35a..eb7f0098ec 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -27,7 +27,7 @@ static void SetupWalletToolArgs() gArgs.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); gArgs.AddArg("-wallet=<wallet-name>", "Specify wallet name", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS); gArgs.AddArg("-debug=<category>", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); - gArgs.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise.", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); + gArgs.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); gArgs.AddArg("info", "Get wallet info", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); gArgs.AddArg("create", "Create new wallet file", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); diff --git a/src/compat/glibc_sanity.cpp b/src/compat/glibc_sanity.cpp index 1ef66e27b4..cc74f28899 100644 --- a/src/compat/glibc_sanity.cpp +++ b/src/compat/glibc_sanity.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-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. @@ -9,7 +9,7 @@ #include <cstddef> #if defined(HAVE_SYS_SELECT_H) -#include <sys/select.h> +bool sanity_test_fdelt(); #endif extern "C" void* memcpy(void* a, const void* b, size_t c); @@ -41,21 +41,6 @@ bool sanity_test_memcpy() } return true; } - -#if defined(HAVE_SYS_SELECT_H) -// trigger: Call FD_SET to trigger __fdelt_chk. FORTIFY_SOURCE must be defined -// as >0 and optimizations must be set to at least -O2. -// test: Add a file descriptor to an empty fd_set. Verify that it has been -// correctly added. -bool sanity_test_fdelt() -{ - fd_set fds; - FD_ZERO(&fds); - FD_SET(0, &fds); - return FD_ISSET(0, &fds); -} -#endif - } // namespace bool glibc_sanity_test() diff --git a/src/compat/glibc_sanity_fdelt.cpp b/src/compat/glibc_sanity_fdelt.cpp new file mode 100644 index 0000000000..87140d0c71 --- /dev/null +++ b/src/compat/glibc_sanity_fdelt.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2009-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. + +#if defined(HAVE_CONFIG_H) +#include <config/bitcoin-config.h> +#endif + +#if defined(HAVE_SYS_SELECT_H) +#ifdef HAVE_CSTRING_DEPENDENT_FD_ZERO +#include <cstring> +#endif +#include <sys/select.h> + +// trigger: Call FD_SET to trigger __fdelt_chk. FORTIFY_SOURCE must be defined +// as >0 and optimizations must be set to at least -O2. +// test: Add a file descriptor to an empty fd_set. Verify that it has been +// correctly added. +bool sanity_test_fdelt() +{ + fd_set fds; + FD_ZERO(&fds); + FD_SET(0, &fds); + return FD_ISSET(0, &fds); +} +#endif diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index 23ed3ecb53..00ebbbd1ab 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -18,7 +18,7 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-oversize"); - // Check for negative or overflow output values + // Check for negative or overflow output values (see CVE-2010-5139) CAmount nValueOut = 0; for (const auto& txout : tx.vout) { diff --git a/src/init.cpp b/src/init.cpp index bb82130542..7c752d615a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1545,7 +1545,7 @@ bool AppInitMain(InitInterfaces& interfaces) } // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate - if (!ReplayBlocks(chainparams, &::ChainstateActive().CoinsDB())) { + if (!::ChainstateActive().ReplayBlocks(chainparams)) { strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate.").translated; break; } @@ -1557,8 +1557,8 @@ bool AppInitMain(InitInterfaces& interfaces) is_coinsview_empty = fReset || fReindexChainState || ::ChainstateActive().CoinsTip().GetBestBlock().IsNull(); if (!is_coinsview_empty) { - // LoadChainTip sets ::ChainActive() based on CoinsTip()'s best block - if (!LoadChainTip(chainparams)) { + // LoadChainTip initializes the chain based on CoinsTip()'s best block + if (!::ChainstateActive().LoadChainTip(chainparams)) { strLoadError = _("Error initializing block database").translated; break; } @@ -282,6 +282,12 @@ public: bool DisconnectNode(const CNetAddr& addr); bool DisconnectNode(NodeId id); + //! Used to convey which local services we are offering peers during node + //! connection. + //! + //! The data returned by this is used in CNode construction, + //! which is used to advertise which services we are offering + //! that peer during `net_processing.cpp:PushNodeVersion()`. ServiceFlags GetLocalServices() const; //!set the max outbound target in bytes @@ -413,7 +419,18 @@ private: std::atomic<NodeId> nLastNodeId{0}; unsigned int nPrevNodeCount{0}; - /** Services this instance offers */ + /** + * Services this instance offers. + * + * This data is replicated in each CNode instance we create during peer + * connection (in ConnectNode()) under a member also called + * nLocalServices. + * + * This data is not marked const, but after being set it should not + * change. See the note in CNode::nLocalServices documentation. + * + * \sa CNode::nLocalServices + */ ServiceFlags nLocalServices; std::unique_ptr<CSemaphore> semOutbound; @@ -786,8 +803,24 @@ public: private: const NodeId id; const uint64_t nLocalHostNonce; - // Services offered to this peer + + //! Services offered to this peer. + //! + //! This is supplied by the parent CConnman during peer connection + //! (CConnman::ConnectNode()) from its attribute of the same name. + //! + //! This is const because there is no protocol defined for renegotiating + //! services initially offered to a peer. The set of local services we + //! offer should not change after initialization. + //! + //! An interesting example of this is NODE_NETWORK and initial block + //! download: a node which starts up from scratch doesn't have any blocks + //! to serve, but still advertises NODE_NETWORK because it will eventually + //! fulfill this role after IBD completes. P2P code is written in such a + //! way that it can gracefully handle peers who don't make good on their + //! service advertisements. const ServiceFlags nLocalServices; + const int nMyStartingHeight; int nSendVersion{0}; NetPermissionFlags m_permissionFlags{ PF_NONE }; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 7f2fea5584..34d349e8e9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -415,6 +415,9 @@ static void UpdatePreferredDownload(CNode* node, CNodeState* state) EXCLUSIVE_LO static void PushNodeVersion(CNode *pnode, CConnman* connman, int64_t nTime) { + // Note that pnode->GetLocalServices() is a reflection of the local + // services we were offering when the CNode object was created for this + // peer. ServiceFlags nLocalNodeServices = pnode->GetLocalServices(); uint64_t nonce = pnode->GetLocalNonce(); int nNodeStartingHeight = pnode->GetMyStartingHeight(); @@ -2556,7 +2559,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } AddOrphanTx(ptx, pfrom->GetId()); - // DoS prevention: do not allow mapOrphanTransactions to grow unbounded + // DoS prevention: do not allow mapOrphanTransactions to grow unbounded (see CVE-2012-3789) unsigned int nMaxOrphanTx = (unsigned int)std::max((int64_t)0, gArgs.GetArg("-maxorphantx", DEFAULT_MAX_ORPHAN_TRANSACTIONS)); unsigned int nEvicted = LimitOrphanTxSize(nMaxOrphanTx); if (nEvicted > 0) { diff --git a/src/qt/askpassphrasedialog.cpp b/src/qt/askpassphrasedialog.cpp index c9f17d12ec..2ababb5e1e 100644 --- a/src/qt/askpassphrasedialog.cpp +++ b/src/qt/askpassphrasedialog.cpp @@ -44,7 +44,7 @@ AskPassphraseDialog::AskPassphraseDialog(Mode _mode, QWidget *parent, SecureStri switch(mode) { case Encrypt: // Ask passphrase x2 - ui->warningLabel->setText(tr("Enter the new passphrase to the wallet.<br/>Please use a passphrase of <b>ten or more random characters</b>, or <b>eight or more words</b>.")); + ui->warningLabel->setText(tr("Enter the new passphrase for the wallet.<br/>Please use a passphrase of <b>ten or more random characters</b>, or <b>eight or more words</b>.")); ui->passLabel1->hide(); ui->passEdit1->hide(); setWindowTitle(tr("Encrypt wallet")); @@ -67,7 +67,7 @@ AskPassphraseDialog::AskPassphraseDialog(Mode _mode, QWidget *parent, SecureStri break; case ChangePass: // Ask old passphrase + new passphrase x2 setWindowTitle(tr("Change passphrase")); - ui->warningLabel->setText(tr("Enter the old passphrase and new passphrase to the wallet.")); + ui->warningLabel->setText(tr("Enter the old passphrase and new passphrase for the wallet.")); break; } textChanged(); diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index c672171cfb..7671fde705 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -375,7 +375,9 @@ void BitcoinGUI::createActions() for (const std::pair<const std::string, bool>& i : m_wallet_controller->listWalletDir()) { const std::string& path = i.first; QString name = path.empty() ? QString("["+tr("default wallet")+"]") : QString::fromStdString(path); - // Menu items remove single &. Single & are shown when && is in the string, but only the first occurrence. So replace only the first & with && + // Menu items remove single &. Single & are shown when && is in + // the string, but only the first occurrence. So replace only + // the first & with &&. name.replace(name.indexOf(QChar('&')), 1, QString("&&")); QAction* action = m_open_wallet_menu->addAction(name); diff --git a/src/qt/bitcoinstrings.cpp b/src/qt/bitcoinstrings.cpp index 5cde21eec6..3d40ee7823 100644 --- a/src/qt/bitcoinstrings.cpp +++ b/src/qt/bitcoinstrings.cpp @@ -178,6 +178,8 @@ QT_TRANSLATE_NOOP("bitcoin-core", "Unable to generate initial keys"), QT_TRANSLATE_NOOP("bitcoin-core", "Unable to generate keys"), QT_TRANSLATE_NOOP("bitcoin-core", "Unable to start HTTP server. See debug log for details."), QT_TRANSLATE_NOOP("bitcoin-core", "Unknown -blockfilterindex value %s."), +QT_TRANSLATE_NOOP("bitcoin-core", "Unknown address type '%s'"), +QT_TRANSLATE_NOOP("bitcoin-core", "Unknown change type '%s'"), QT_TRANSLATE_NOOP("bitcoin-core", "Unknown network specified in -onlynet: '%s'"), QT_TRANSLATE_NOOP("bitcoin-core", "Unsupported logging category %s=%s."), QT_TRANSLATE_NOOP("bitcoin-core", "Upgrading UTXO database"), diff --git a/src/qt/createwalletdialog.cpp b/src/qt/createwalletdialog.cpp index 10262c37c3..8e6474b0d4 100644 --- a/src/qt/createwalletdialog.cpp +++ b/src/qt/createwalletdialog.cpp @@ -25,7 +25,8 @@ CreateWalletDialog::CreateWalletDialog(QWidget* parent) : }); connect(ui->encrypt_wallet_checkbox, &QCheckBox::toggled, [this](bool checked) { - // Disable disable_privkeys_checkbox when encrypt is set to true, enable it when encrypt is false + // Disable the disable_privkeys_checkbox when isEncryptWalletChecked is + // set to true, enable it when isEncryptWalletChecked is false. ui->disable_privkeys_checkbox->setEnabled(!checked); // When the disable_privkeys_checkbox is disabled, uncheck it. @@ -45,17 +46,17 @@ QString CreateWalletDialog::walletName() const return ui->wallet_name_line_edit->text(); } -bool CreateWalletDialog::encrypt() const +bool CreateWalletDialog::isEncryptWalletChecked() const { return ui->encrypt_wallet_checkbox->isChecked(); } -bool CreateWalletDialog::disablePrivateKeys() const +bool CreateWalletDialog::isDisablePrivateKeysChecked() const { return ui->disable_privkeys_checkbox->isChecked(); } -bool CreateWalletDialog::blank() const +bool CreateWalletDialog::isMakeBlankWalletChecked() const { return ui->blank_wallet_checkbox->isChecked(); } diff --git a/src/qt/createwalletdialog.h b/src/qt/createwalletdialog.h index a1365b5969..30766107b9 100644 --- a/src/qt/createwalletdialog.h +++ b/src/qt/createwalletdialog.h @@ -24,9 +24,9 @@ public: virtual ~CreateWalletDialog(); QString walletName() const; - bool encrypt() const; - bool disablePrivateKeys() const; - bool blank() const; + bool isEncryptWalletChecked() const; + bool isDisablePrivateKeysChecked() const; + bool isMakeBlankWalletChecked() const; private: Ui::CreateWalletDialog *ui; diff --git a/src/qt/forms/askpassphrasedialog.ui b/src/qt/forms/askpassphrasedialog.ui index 69803989cd..e74d183818 100644 --- a/src/qt/forms/askpassphrasedialog.ui +++ b/src/qt/forms/askpassphrasedialog.ui @@ -95,7 +95,7 @@ <item row="3" column="1"> <widget class="QCheckBox" name="toggleShowPasswordButton"> <property name="text"> - <string>Show password</string> + <string>Show passphrase</string> </property> </widget> </item> diff --git a/src/qt/forms/createwalletdialog.ui b/src/qt/forms/createwalletdialog.ui index 1fbaeeaaab..e49bab8f3b 100644 --- a/src/qt/forms/createwalletdialog.ui +++ b/src/qt/forms/createwalletdialog.ui @@ -62,7 +62,7 @@ </rect> </property> <property name="toolTip"> - <string>Encrypt the wallet. The wallet will be encrypted with a password of your choice.</string> + <string>Encrypt the wallet. The wallet will be encrypted with a passphrase of your choice.</string> </property> <property name="text"> <string>Encrypt Wallet</string> diff --git a/src/qt/locale/bitcoin_en.ts b/src/qt/locale/bitcoin_en.ts index 7864f97f31..7d9e7eab4e 100644 --- a/src/qt/locale/bitcoin_en.ts +++ b/src/qt/locale/bitcoin_en.ts @@ -171,16 +171,11 @@ </message> <message> <location line="+14"/> - <source>Show password</source> + <source>Show passphrase</source> <translation type="unfinished"></translation> </message> <message> - <location filename="../askpassphrasedialog.cpp" line="+46"/> - <source>Enter the new passphrase to the wallet.<br/>Please use a passphrase of <b>ten or more random characters</b>, or <b>eight or more words</b>.</source> - <translation type="unfinished"></translation> - </message> - <message> - <location line="+3"/> + <location filename="../askpassphrasedialog.cpp" line="+50"/> <source>Encrypt wallet</source> <translation type="unfinished"></translation> </message> @@ -210,12 +205,7 @@ <translation type="unfinished"></translation> </message> <message> - <location line="+1"/> - <source>Enter the old passphrase and new passphrase to the wallet.</source> - <translation type="unfinished"></translation> - </message> - <message> - <location line="+45"/> + <location line="+46"/> <source>Confirm wallet encryption</source> <translation type="unfinished"></translation> </message> @@ -230,36 +220,61 @@ <translation type="unfinished"></translation> </message> <message> - <location line="+9"/> - <location line="+58"/> + <location line="+19"/> + <location line="+57"/> <source>Wallet encrypted</source> <translation type="unfinished"></translation> </message> <message> - <location line="-56"/> - <source>Your wallet is now encrypted. Remember that encrypting your wallet cannot fully protect your bitcoins from being stolen by malware infecting your computer.</source> + <location line="-145"/> + <source>Enter the new passphrase for the wallet.<br/>Please use a passphrase of <b>ten or more random characters</b>, or <b>eight or more words</b>.</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+23"/> + <source>Enter the old passphrase and new passphrase for the wallet.</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+53"/> + <source>Remember that encrypting your wallet cannot fully protect your bitcoins from being stolen by malware infecting your computer.</source> <translation type="unfinished"></translation> </message> <message> <location line="+4"/> + <source>Wallet to be encrypted</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+2"/> + <source>Your wallet is about to be encrypted. </source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+8"/> + <source>Your wallet is now encrypted. </source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+2"/> <source>IMPORTANT: Any previous backups you have made of your wallet file should be replaced with the newly generated, encrypted wallet file. For security reasons, previous backups of the unencrypted wallet file will become useless as soon as you start using the new, encrypted wallet.</source> <translation type="unfinished"></translation> </message> <message> <location line="+8"/> - <location line="+7"/> + <location line="+8"/> <location line="+43"/> <location line="+6"/> <source>Wallet encryption failed</source> <translation type="unfinished"></translation> </message> <message> - <location line="-55"/> + <location line="-56"/> <source>Wallet encryption failed due to an internal error. Your wallet was not encrypted.</source> <translation type="unfinished"></translation> </message> <message> - <location line="+7"/> + <location line="+8"/> <location line="+49"/> <source>The supplied passphrases do not match.</source> <translation type="unfinished"></translation> @@ -310,17 +325,17 @@ <context> <name>BitcoinGUI</name> <message> - <location filename="../bitcoingui.cpp" line="+315"/> + <location filename="../bitcoingui.cpp" line="+316"/> <source>Sign &message...</source> <translation>Sign &message...</translation> </message> <message> - <location line="+637"/> + <location line="+623"/> <source>Synchronizing with network...</source> <translation>Synchronizing with network...</translation> </message> <message> - <location line="-715"/> + <location line="-701"/> <source>&Overview</source> <translation>&Overview</translation> </message> @@ -400,7 +415,17 @@ <translation type="unfinished"></translation> </message> <message> - <location line="+216"/> + <location line="+11"/> + <source>Create Wallet...</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+1"/> + <source>Create a new wallet</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+190"/> <source>Wallet:</source> <translation type="unfinished"></translation> </message> @@ -435,7 +460,7 @@ <translation type="unfinished"></translation> </message> <message> - <location line="-1035"/> + <location line="-1021"/> <source>Send coins to a Bitcoin address</source> <translation>Send coins to a Bitcoin address</translation> </message> @@ -500,17 +525,17 @@ <translation>Verify messages to ensure they were signed with specified Bitcoin addresses</translation> </message> <message> - <location line="+117"/> + <location line="+110"/> <source>&File</source> <translation>&File</translation> </message> <message> - <location line="+14"/> + <location line="+15"/> <source>&Settings</source> <translation>&Settings</translation> </message> <message> - <location line="+66"/> + <location line="+58"/> <source>&Help</source> <translation>&Help</translation> </message> @@ -520,7 +545,7 @@ <translation>Tabs toolbar</translation> </message> <message> - <location line="-270"/> + <location line="-256"/> <source>Request payments (generates QR codes and bitcoin: URIs)</source> <translation type="unfinished"></translation> </message> @@ -540,12 +565,12 @@ <translation type="unfinished"></translation> </message> <message> - <location line="+10"/> + <location line="+13"/> <source>&Command-line options</source> <translation type="unfinished"></translation> </message> <message numerus="yes"> - <location line="+539"/> + <location line="+522"/> <source>%n active connection(s) to Bitcoin network</source> <translation> <numerusform>%n active connection to Bitcoin network</numerusform> @@ -606,7 +631,7 @@ <translation>Up to date</translation> </message> <message> - <location line="-656"/> + <location line="-642"/> <source>&Sending addresses</source> <translation type="unfinished"></translation> </message> @@ -636,7 +661,7 @@ <translation type="unfinished"></translation> </message> <message> - <location line="+4"/> + <location line="+7"/> <source>Show the %1 help message to get a list with possible Bitcoin command-line options</source> <translation type="unfinished"></translation> </message> @@ -646,22 +671,12 @@ <translation type="unfinished"></translation> </message> <message> - <location line="+13"/> - <source>Opening Wallet <b>%1</b>...</source> - <translation type="unfinished"></translation> - </message> - <message> - <location line="+9"/> - <source>Open Wallet Failed</source> - <translation type="unfinished"></translation> - </message> - <message> - <location line="+15"/> + <location line="+21"/> <source>No wallets available</source> <translation type="unfinished"></translation> </message> <message> - <location line="+48"/> + <location line="+55"/> <source>&Window</source> <translation type="unfinished">&Window</translation> </message> @@ -676,12 +691,7 @@ <translation type="unfinished"></translation> </message> <message> - <location line="+14"/> - <source>Restore</source> - <translation type="unfinished"></translation> - </message> - <message> - <location line="+12"/> + <location line="+18"/> <source>Main Window</source> <translation type="unfinished"></translation> </message> @@ -782,7 +792,7 @@ <translation>Wallet is <b>encrypted</b> and currently <b>locked</b></translation> </message> <message> - <location filename="../bitcoin.cpp" line="+382"/> + <location filename="../bitcoin.cpp" line="+386"/> <source>A fatal error occurred. Bitcoin can no longer continue safely and will quit.</source> <translation type="unfinished"></translation> </message> @@ -978,6 +988,72 @@ </message> </context> <context> + <name>CreateWalletActivity</name> + <message> + <location filename="../walletcontroller.cpp" line="+201"/> + <source>Creating Wallet <b>%1</b>...</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+26"/> + <source>Create wallet failed</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+2"/> + <source>Create wallet warning</source> + <translation type="unfinished"></translation> + </message> +</context> +<context> + <name>CreateWalletDialog</name> + <message> + <location filename="../forms/createwalletdialog.ui" line="+14"/> + <source>Create Wallet</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+38"/> + <source>Wallet Name</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+13"/> + <source>Encrypt the wallet. The wallet will be encrypted with a passphrase of your choice.</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+3"/> + <source>Encrypt Wallet</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+19"/> + <source>Disable private keys for this wallet. Wallets with private keys disabled will have no private keys and cannot have an HD seed or imported private keys. This is ideal for watch-only wallets.</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+3"/> + <source>Disable Private Keys</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+13"/> + <source>Make a blank wallet. Blank wallets do not initially have private keys or scripts. Private keys and addresses can be imported, or an HD seed can be set, at a later time.</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+3"/> + <source>Make Blank Wallet</source> + <translation type="unfinished"></translation> + </message> + <message> + <location filename="../createwalletdialog.cpp" line="+19"/> + <source>Create</source> + <translation type="unfinished"></translation> + </message> +</context> +<context> <name>EditAddressDialog</name> <message> <location filename="../forms/editaddressdialog.ui" line="+14"/> @@ -1121,6 +1197,11 @@ </message> <message> <location line="+10"/> + <source>Reverting this setting requires re-downloading the entire blockchain. It is faster to download the full chain first and prune it later. Disables some advanced features.</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+10"/> <source>This initial synchronisation is very demanding, and may expose hardware problems with your computer that had previously gone unnoticed. Each time you run %1, it will continue downloading where it left off.</source> <translation type="unfinished"></translation> </message> @@ -1130,7 +1211,7 @@ <translation type="unfinished"></translation> </message> <message> - <location line="-160"/> + <location line="-170"/> <source>Use the default data directory</source> <translation>Use the default data directory</translation> </message> @@ -1145,7 +1226,12 @@ <translation type="unfinished">Bitcoin</translation> </message> <message> - <location line="+6"/> + <location line="+9"/> + <source>Discard blocks after verification, except most recent %1 GB (prune)</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+2"/> <source>At least %1 GB of data will be stored in this directory, and it will grow over time.</source> <translation type="unfinished"></translation> </message> @@ -1165,12 +1251,12 @@ <translation type="unfinished"></translation> </message> <message> - <location line="+75"/> + <location line="+78"/> <source>Error: Specified data directory "%1" cannot be created.</source> <translation type="unfinished"></translation> </message> <message> - <location line="+27"/> + <location line="+30"/> <source>Error</source> <translation>Error</translation> </message> @@ -1190,6 +1276,14 @@ <numerusform>(of %n GB needed)</numerusform> </translation> </message> + <message numerus="yes"> + <location line="+4"/> + <source>(%n GB needed for full chain)</source> + <translation type="unfinished"> + <numerusform></numerusform> + <numerusform></numerusform> + </translation> + </message> </context> <context> <name>ModalOverlay</name> @@ -1286,6 +1380,29 @@ </message> </context> <context> + <name>OpenWalletActivity</name> + <message> + <location filename="../walletcontroller.cpp" line="+39"/> + <source>Open wallet failed</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+2"/> + <source>Open wallet warning</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+10"/> + <source>default wallet</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+2"/> + <source>Opening Wallet <b>%1</b>...</source> + <translation type="unfinished"></translation> + </message> +</context> +<context> <name>OptionsDialog</name> <message> <location filename="../forms/optionsdialog.ui" line="+14"/> @@ -1734,7 +1851,7 @@ <name>PaymentServer</name> <message> <location filename="../paymentserver.cpp" line="+226"/> - <location line="+346"/> + <location line="+350"/> <location line="+42"/> <location line="+108"/> <location line="+14"/> @@ -1743,7 +1860,7 @@ <translation type="unfinished"></translation> </message> <message> - <location line="-527"/> + <location line="-531"/> <source>Cannot start bitcoin: click-to-pay handler</source> <translation type="unfinished"></translation> </message> @@ -1752,13 +1869,13 @@ <location line="+9"/> <location line="+16"/> <location line="+16"/> - <location line="+5"/> + <location line="+7"/> <location line="+7"/> <source>URI handling</source> <translation type="unfinished"></translation> </message> <message> - <location line="-53"/> + <location line="-55"/> <source>'bitcoin://' is not a valid URI. Use 'bitcoin:' instead.</source> <translation type="unfinished"></translation> </message> @@ -1774,12 +1891,24 @@ </message> <message> <location line="+16"/> - <location line="+36"/> + <location line="+38"/> <source>Cannot process payment request because BIP70 support was not compiled in.</source> <translation type="unfinished"></translation> </message> <message> - <location line="-32"/> + <location line="-37"/> + <location line="+38"/> + <source>Due to widespread security flaws in BIP70 it's strongly recommended that any merchant instructions to switch wallets be ignored.</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="-37"/> + <location line="+38"/> + <source>If you are receiving this error you should request the merchant provide a BIP21 compatible URI.</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="-34"/> <source>Invalid payment address %1</source> <translation type="unfinished"></translation> </message> @@ -1800,7 +1929,7 @@ <translation type="unfinished"></translation> </message> <message> - <location line="+199"/> + <location line="+201"/> <location line="+9"/> <location line="+31"/> <location line="+10"/> @@ -2032,7 +2161,7 @@ <translation type="unfinished"></translation> </message> <message> - <location filename="../bitcoin.cpp" line="+116"/> + <location filename="../bitcoin.cpp" line="+118"/> <source>Error: Specified data directory "%1" does not exist.</source> <translation type="unfinished"></translation> </message> @@ -2047,7 +2176,7 @@ <translation type="unfinished"></translation> </message> <message> - <location line="+59"/> + <location line="+64"/> <source>%1 didn't yet exit safely...</source> <translation type="unfinished"></translation> </message> @@ -3434,14 +3563,6 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satos </message> </context> <context> - <name>SplashScreen</name> - <message> - <location filename="../networkstyle.cpp" line="+19"/> - <source>[testnet]</source> - <translation>[testnet]</translation> - </message> -</context> -<context> <name>TrafficGraphWidget</name> <message> <location filename="../trafficgraphwidget.cpp" line="+81"/> @@ -4036,13 +4157,13 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satos <context> <name>WalletController</name> <message> - <location filename="../walletcontroller.cpp" line="+73"/> + <location filename="../walletcontroller.cpp" line="-205"/> <source>Close wallet</source> <translation type="unfinished"></translation> </message> <message> <location line="+1"/> - <source>Are you sure you wish to close wallet <i>%1</i>?</source> + <source>Are you sure you wish to close the wallet <i>%1</i>?</source> <translation type="unfinished"></translation> </message> <message> @@ -4410,12 +4531,22 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satos <translation type="unfinished"></translation> </message> <message> - <location line="+25"/> + <location line="+22"/> + <source>Unknown address type '%s'</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+1"/> + <source>Unknown change type '%s'</source> + <translation type="unfinished"></translation> + </message> + <message> + <location line="+4"/> <source>Upgrading txindex database</source> <translation type="unfinished"></translation> </message> <message> - <location line="-44"/> + <location line="-46"/> <source>Loading P2P addresses...</source> <translation type="unfinished"></translation> </message> @@ -4475,7 +4606,7 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satos <translation type="unfinished"></translation> </message> <message> - <location line="+4"/> + <location line="+6"/> <source>Unsupported logging category %s=%s.</source> <translation type="unfinished"></translation> </message> @@ -4500,7 +4631,7 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satos <translation type="unfinished"></translation> </message> <message> - <location line="-154"/> + <location line="-156"/> <source>Error: Listening for incoming connections failed (listen returned error %s)</source> <translation type="unfinished"></translation> </message> @@ -4641,7 +4772,7 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satos <translation type="unfinished"></translation> </message> <message> - <location line="+7"/> + <location line="+9"/> <source>Verifying wallet(s)...</source> <translation type="unfinished"></translation> </message> @@ -4656,7 +4787,7 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satos <translation type="unfinished"></translation> </message> <message> - <location line="-177"/> + <location line="-179"/> <source>-maxtxfee is set very high! Fees this large could be paid on a single transaction.</source> <translation type="unfinished"></translation> </message> @@ -4726,12 +4857,12 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satos <translation type="unfinished"></translation> </message> <message> - <location line="+9"/> + <location line="+11"/> <source>Unknown network specified in -onlynet: '%s'</source> <translation>Unknown network specified in -onlynet: '%s'</translation> </message> <message> - <location line="-50"/> + <location line="-52"/> <source>Insufficient funds</source> <translation>Insufficient funds</translation> </message> diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index 0bb87742e9..00d83d23dd 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -328,7 +328,9 @@ void PaymentServer::handleURIOrFile(const QString& s) #ifndef ENABLE_BIP70 if (uri.hasQueryItem("r")) { // payment request Q_EMIT message(tr("URI handling"), - tr("Cannot process payment request because BIP70 support was not compiled in."), + tr("Cannot process payment request because BIP70 support was not compiled in.")+ + tr("Due to widespread security flaws in BIP70 it's strongly recommended that any merchant instructions to switch wallets be ignored.")+ + tr("If you are receiving this error you should request the merchant provide a BIP21 compatible URI."), CClientUIInterface::ICON_WARNING); } #endif @@ -364,7 +366,9 @@ void PaymentServer::handleURIOrFile(const QString& s) return; #else Q_EMIT message(tr("Payment request file handling"), - tr("Cannot process payment request because BIP70 support was not compiled in."), + tr("Cannot process payment request because BIP70 support was not compiled in.")+ + tr("Due to widespread security flaws in BIP70 it's strongly recommended that any merchant instructions to switch wallets be ignored.")+ + tr("If you are receiving this error you should request the merchant provide a BIP21 compatible URI."), CClientUIInterface::ICON_WARNING); #endif } diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 8b8283d3d8..fa6f9f3f16 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -75,7 +75,7 @@ void WalletController::closeWallet(WalletModel* wallet_model, QWidget* parent) { QMessageBox box(parent); box.setWindowTitle(tr("Close wallet")); - box.setText(tr("Are you sure you wish to close wallet <i>%1</i>?").arg(GUIUtil::HtmlEscape(wallet_model->getDisplayName()))); + box.setText(tr("Are you sure you wish to close the wallet <i>%1</i>?").arg(GUIUtil::HtmlEscape(wallet_model->getDisplayName()))); box.setInformativeText(tr("Closing the wallet for too long can result in having to resync the entire chain if pruning is enabled.")); box.setStandardButtons(QMessageBox::Yes|QMessageBox::Cancel); box.setDefaultButton(QMessageBox::Yes); @@ -179,9 +179,10 @@ CreateWalletActivity::~CreateWalletActivity() delete m_passphrase_dialog; } -void CreateWalletActivity::askPasshprase() +void CreateWalletActivity::askPassphrase() { m_passphrase_dialog = new AskPassphraseDialog(AskPassphraseDialog::Encrypt, m_parent_widget, &m_passphrase); + m_passphrase_dialog->setWindowModality(Qt::ApplicationModal); m_passphrase_dialog->show(); connect(m_passphrase_dialog, &QObject::destroyed, [this] { @@ -201,10 +202,10 @@ void CreateWalletActivity::createWallet() std::string name = m_create_wallet_dialog->walletName().toStdString(); uint64_t flags = 0; - if (m_create_wallet_dialog->disablePrivateKeys()) { + if (m_create_wallet_dialog->isDisablePrivateKeysChecked()) { flags |= WALLET_FLAG_DISABLE_PRIVATE_KEYS; } - if (m_create_wallet_dialog->blank()) { + if (m_create_wallet_dialog->isMakeBlankWalletChecked()) { flags |= WALLET_FLAG_BLANK_WALLET; } @@ -246,8 +247,8 @@ void CreateWalletActivity::create() Q_EMIT finished(); }); connect(m_create_wallet_dialog, &QDialog::accepted, [this] { - if (m_create_wallet_dialog->encrypt()) { - askPasshprase(); + if (m_create_wallet_dialog->isEncryptWalletChecked()) { + askPassphrase(); } else { createWallet(); } diff --git a/src/qt/walletcontroller.h b/src/qt/walletcontroller.h index 4e1a772f3a..fb37b7292c 100644 --- a/src/qt/walletcontroller.h +++ b/src/qt/walletcontroller.h @@ -118,7 +118,7 @@ Q_SIGNALS: void created(WalletModel* wallet_model); private: - void askPasshprase(); + void askPassphrase(); void createWallet(); void finish(); diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 93fca5a6de..c2714f9c83 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -85,7 +85,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getblockheader", 1, "verbose" }, { "getchaintxstats", 0, "nblocks" }, { "gettransaction", 1, "include_watchonly" }, - { "gettransaction", 2, "decode" }, + { "gettransaction", 2, "verbose" }, { "getrawtransaction", 1, "verbose" }, { "createrawtransaction", 0, "inputs" }, { "createrawtransaction", 1, "outputs" }, diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index fb8ea8c227..461e8025da 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -14,9 +14,11 @@ #include <node/coin.h> #include <node/psbt.h> #include <node/transaction.h> +#include <policy/policy.h> #include <policy/rbf.h> #include <primitives/transaction.h> #include <psbt.h> +#include <random.h> #include <rpc/rawtransaction_util.h> #include <rpc/server.h> #include <rpc/util.h> @@ -37,11 +39,11 @@ #include <univalue.h> -/** High fee for sendrawtransaction and testmempoolaccept. - * By default, transaction with a fee higher than this will be rejected by the - * RPCs. This can be overridden with the maxfeerate argument. +/** High fee rate for sendrawtransaction and testmempoolaccept. + * By default, transaction with a fee rate higher than this will be rejected by + * the RPCs. This can be overridden with the maxfeerate argument. */ -constexpr static CAmount DEFAULT_MAX_RAW_TX_FEE{COIN / 10}; +static const CFeeRate DEFAULT_MAX_RAW_TX_FEE_RATE{COIN / 10}; static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry) { @@ -774,7 +776,7 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) "\nAlso see createrawtransaction and signrawtransactionwithkey calls.\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, - {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE), + {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK()), "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kB.\nSet to 0 to accept any fee rate.\n"}, }, @@ -804,19 +806,17 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); CTransactionRef tx(MakeTransactionRef(std::move(mtx))); - CAmount max_raw_tx_fee = DEFAULT_MAX_RAW_TX_FEE; + CFeeRate max_raw_tx_fee_rate = DEFAULT_MAX_RAW_TX_FEE_RATE; // TODO: temporary migration code for old clients. Remove in v0.20 if (request.params[1].isBool()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Second argument must be numeric (maxfeerate) and no longer supports a boolean. To allow a transaction with high fees, set maxfeerate to 0."); } else if (!request.params[1].isNull()) { - size_t weight = GetTransactionWeight(*tx); - CFeeRate fr(AmountFromValue(request.params[1])); - // the +3/4 part rounds the value up, and is the same formula used when - // calculating the fee for a transaction - // (see GetVirtualTransactionSize) - max_raw_tx_fee = fr.GetFee((weight+3)/4); + max_raw_tx_fee_rate = CFeeRate(AmountFromValue(request.params[1])); } + int64_t virtual_size = GetVirtualTransactionSize(*tx); + CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); + std::string err_string; AssertLockNotHeld(cs_main); const TransactionError err = BroadcastTransaction(tx, err_string, max_raw_tx_fee, /*relay*/ true, /*wait_callback*/ true); @@ -840,7 +840,7 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""}, }, }, - {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE), "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kB\n"}, + {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK()), "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kB\n"}, }, RPCResult{ "[ (array) The result of the mempool acceptance test for each raw transaction in the input array.\n" @@ -880,19 +880,17 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) CTransactionRef tx(MakeTransactionRef(std::move(mtx))); const uint256& tx_hash = tx->GetHash(); - CAmount max_raw_tx_fee = DEFAULT_MAX_RAW_TX_FEE; + CFeeRate max_raw_tx_fee_rate = DEFAULT_MAX_RAW_TX_FEE_RATE; // TODO: temporary migration code for old clients. Remove in v0.20 if (request.params[1].isBool()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Second argument must be numeric (maxfeerate) and no longer supports a boolean. To allow a transaction with high fees, set maxfeerate to 0."); } else if (!request.params[1].isNull()) { - size_t weight = GetTransactionWeight(*tx); - CFeeRate fr(AmountFromValue(request.params[1])); - // the +3/4 part rounds the value up, and is the same formula used when - // calculating the fee for a transaction - // (see GetVirtualTransactionSize) - max_raw_tx_fee = fr.GetFee((weight+3)/4); + max_raw_tx_fee_rate = CFeeRate(AmountFromValue(request.params[1])); } + int64_t virtual_size = GetVirtualTransactionSize(*tx); + CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); + UniValue result(UniValue::VARR); UniValue result_0(UniValue::VOBJ); result_0.pushKV("txid", tx_hash.GetHex()); @@ -1615,8 +1613,30 @@ UniValue joinpsbts(const JSONRPCRequest& request) merged_psbt.unknown.insert(psbt.unknown.begin(), psbt.unknown.end()); } + // Generate list of shuffled indices for shuffling inputs and outputs of the merged PSBT + std::vector<int> input_indices(merged_psbt.inputs.size()); + std::iota(input_indices.begin(), input_indices.end(), 0); + std::vector<int> output_indices(merged_psbt.outputs.size()); + std::iota(output_indices.begin(), output_indices.end(), 0); + + // Shuffle input and output indicies lists + Shuffle(input_indices.begin(), input_indices.end(), FastRandomContext()); + Shuffle(output_indices.begin(), output_indices.end(), FastRandomContext()); + + PartiallySignedTransaction shuffled_psbt; + shuffled_psbt.tx = CMutableTransaction(); + shuffled_psbt.tx->nVersion = merged_psbt.tx->nVersion; + shuffled_psbt.tx->nLockTime = merged_psbt.tx->nLockTime; + for (int i : input_indices) { + shuffled_psbt.AddInput(merged_psbt.tx->vin[i], merged_psbt.inputs[i]); + } + for (int i : output_indices) { + shuffled_psbt.AddOutput(merged_psbt.tx->vout[i], merged_psbt.outputs[i]); + } + shuffled_psbt.unknown.insert(merged_psbt.unknown.begin(), merged_psbt.unknown.end()); + CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); - ssTx << merged_psbt; + ssTx << shuffled_psbt; return EncodeBase64((unsigned char*)ssTx.data(), ssTx.size()); } diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index f1d176ba4d..fe98fff4bb 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -268,7 +268,7 @@ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keyst } } -UniValue SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, std::map<COutPoint, Coin>& coins, const UniValue& hashType) +UniValue SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, const UniValue& hashType) { int nHashType = ParseSighashString(hashType); diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index b35e6da4ca..5b92650764 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -19,11 +19,11 @@ class SigningProvider; * * @param mtx The transaction to-be-signed * @param keystore Temporary keystore containing signing keys - * @param coins Map of unspent outputs - coins in mempool and current chain UTXO set, may be extended by previous txns outputs after call + * @param coins Map of unspent outputs * @param hashType The signature hash type * @returns JSON object with details of signed transaction */ -UniValue SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, std::map<COutPoint, Coin>& coins, const UniValue& hashType); +UniValue SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, const UniValue& hashType); /** * Parse a prevtxs UniValue array and get the map of coins from it diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index f8701b6d01..20fae2eebf 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -334,7 +334,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& opcode == OP_MOD || opcode == OP_LSHIFT || opcode == OP_RSHIFT) - return set_error(serror, SCRIPT_ERR_DISABLED_OPCODE); // Disabled opcodes. + return set_error(serror, SCRIPT_ERR_DISABLED_OPCODE); // Disabled opcodes (CVE-2010-5137). // With SCRIPT_VERIFY_CONST_SCRIPTCODE, OP_CODESEPARATOR in non-segwit script is rejected even in an unexecuted branch if (opcode == OP_CODESEPARATOR && sigversion == SigVersion::BASE && (flags & SCRIPT_VERIFY_CONST_SCRIPTCODE)) @@ -1483,6 +1483,8 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, const C return set_error(serror, SCRIPT_ERR_SIG_PUSHONLY); } + // scriptSig and scriptPubKey must be evaluated sequentially on the same stack + // rather than being simply concatenated (see CVE-2010-5141) std::vector<std::vector<unsigned char> > stack, stackCopy; if (!EvalScript(stack, scriptSig, flags, checker, SigVersion::BASE, serror)) // serror is set diff --git a/src/test/data/script_tests.json b/src/test/data/script_tests.json index 9b320b6943..3241f32f56 100644 --- a/src/test/data/script_tests.json +++ b/src/test/data/script_tests.json @@ -829,15 +829,16 @@ ["NOP", "2SWAP 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], ["1", "2 3 2SWAP 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], + +["NOP", "SIZE 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], + +["TEST DISABLED OP CODES (CVE-2010-5137)"], ["'a' 'b'", "CAT", "P2SH,STRICTENC", "DISABLED_OPCODE", "CAT disabled"], ["'a' 'b' 0", "IF CAT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "CAT disabled"], ["'abc' 1 1", "SUBSTR", "P2SH,STRICTENC", "DISABLED_OPCODE", "SUBSTR disabled"], ["'abc' 1 1 0", "IF SUBSTR ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "SUBSTR disabled"], ["'abc' 2 0", "IF LEFT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "LEFT disabled"], ["'abc' 2 0", "IF RIGHT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "RIGHT disabled"], - -["NOP", "SIZE 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], - ["'abc'", "IF INVERT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "INVERT disabled"], ["1 2 0 IF AND ELSE 1 ENDIF", "NOP", "P2SH,STRICTENC", "DISABLED_OPCODE", "AND disabled"], ["1 2 0 IF OR ELSE 1 ENDIF", "NOP", "P2SH,STRICTENC", "DISABLED_OPCODE", "OR disabled"], diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp index 1684258c9f..dc38a1a818 100644 --- a/src/test/merkle_tests.cpp +++ b/src/test/merkle_tests.cpp @@ -249,4 +249,104 @@ BOOST_AUTO_TEST_CASE(merkle_test) } } + +BOOST_AUTO_TEST_CASE(merkle_test_empty_block) +{ + bool mutated = false; + CBlock block; + uint256 root = BlockMerkleRoot(block, &mutated); + + BOOST_CHECK_EQUAL(root.IsNull(), true); + BOOST_CHECK_EQUAL(mutated, false); +} + +BOOST_AUTO_TEST_CASE(merkle_test_oneTx_block) +{ + bool mutated = false; + CBlock block; + + block.vtx.resize(1); + CMutableTransaction mtx; + mtx.nLockTime = 0; + block.vtx[0] = MakeTransactionRef(std::move(mtx)); + uint256 root = BlockMerkleRoot(block, &mutated); + BOOST_CHECK_EQUAL(root, block.vtx[0]->GetHash()); + BOOST_CHECK_EQUAL(mutated, false); +} + +BOOST_AUTO_TEST_CASE(merkle_test_OddTxWithRepeatedLastTx_block) +{ + bool mutated; + CBlock block, blockWithRepeatedLastTx; + + block.vtx.resize(3); + + for (std::size_t pos = 0; pos < block.vtx.size(); pos++) { + CMutableTransaction mtx; + mtx.nLockTime = pos; + block.vtx[pos] = MakeTransactionRef(std::move(mtx)); + } + + blockWithRepeatedLastTx = block; + blockWithRepeatedLastTx.vtx.push_back(blockWithRepeatedLastTx.vtx.back()); + + uint256 rootofBlock = BlockMerkleRoot(block, &mutated); + BOOST_CHECK_EQUAL(mutated, false); + + uint256 rootofBlockWithRepeatedLastTx = BlockMerkleRoot(blockWithRepeatedLastTx, &mutated); + BOOST_CHECK_EQUAL(rootofBlock, rootofBlockWithRepeatedLastTx); + BOOST_CHECK_EQUAL(mutated, true); +} + +BOOST_AUTO_TEST_CASE(merkle_test_LeftSubtreeRightSubtree) +{ + CBlock block, leftSubtreeBlock, rightSubtreeBlock; + + block.vtx.resize(4); + std::size_t pos; + for (pos = 0; pos < block.vtx.size(); pos++) { + CMutableTransaction mtx; + mtx.nLockTime = pos; + block.vtx[pos] = MakeTransactionRef(std::move(mtx)); + } + + for (pos = 0; pos < block.vtx.size() / 2; pos++) + leftSubtreeBlock.vtx.push_back(block.vtx[pos]); + + for (pos = block.vtx.size() / 2; pos < block.vtx.size(); pos++) + rightSubtreeBlock.vtx.push_back(block.vtx[pos]); + + uint256 root = BlockMerkleRoot(block); + uint256 rootOfLeftSubtree = BlockMerkleRoot(leftSubtreeBlock); + uint256 rootOfRightSubtree = BlockMerkleRoot(rightSubtreeBlock); + std::vector<uint256> leftRight; + leftRight.push_back(rootOfLeftSubtree); + leftRight.push_back(rootOfRightSubtree); + uint256 rootOfLR = ComputeMerkleRoot(leftRight); + + BOOST_CHECK_EQUAL(root, rootOfLR); +} + +BOOST_AUTO_TEST_CASE(merkle_test_BlockWitness) +{ + CBlock block; + + block.vtx.resize(2); + for (std::size_t pos = 0; pos < block.vtx.size(); pos++) { + CMutableTransaction mtx; + mtx.nLockTime = pos; + block.vtx[pos] = MakeTransactionRef(std::move(mtx)); + } + + uint256 blockWitness = BlockWitnessMerkleRoot(block); + + std::vector<uint256> hashes; + hashes.resize(block.vtx.size()); + hashes[0].SetNull(); + hashes[1] = block.vtx[1]->GetHash(); + + uint256 merkelRootofHashes = ComputeMerkleRoot(hashes); + + BOOST_CHECK_EQUAL(merkelRootofHashes, blockWitness); +} BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validation.cpp b/src/validation.cpp index 49b05f350a..4caee52424 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -428,21 +428,134 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationSt return CheckInputs(tx, state, view, flags, cacheSigStore, true, txdata); } -/** - * @param[out] coins_to_uncache Return any outpoints which were not previously present in the - * coins cache, but were added as a result of validating the tx - * for mempool acceptance. This allows the caller to optionally - * remove the cache additions if the associated transaction ends - * up being rejected by the mempool. - */ -static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx, - bool* pfMissingInputs, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced, - bool bypass_limits, const CAmount& nAbsurdFee, std::vector<COutPoint>& coins_to_uncache, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +namespace { + +class MemPoolAccept { - const CTransaction& tx = *ptx; - const uint256 hash = tx.GetHash(); - AssertLockHeld(cs_main); - LOCK(pool.cs); // mempool "read lock" (held through GetMainSignals().TransactionAddedToMempool()) +public: + MemPoolAccept(CTxMemPool& mempool) : m_pool(mempool), m_view(&m_dummy), m_viewmempool(&::ChainstateActive().CoinsTip(), m_pool), + m_limit_ancestors(gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT)), + m_limit_ancestor_size(gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000), + m_limit_descendants(gArgs.GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT)), + m_limit_descendant_size(gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000) {} + + // We put the arguments we're handed into a struct, so we can pass them + // around easier. + struct ATMPArgs { + const CChainParams& m_chainparams; + CValidationState &m_state; + bool* m_missing_inputs; + const int64_t m_accept_time; + std::list<CTransactionRef>* m_replaced_transactions; + const bool m_bypass_limits; + const CAmount& m_absurd_fee; + /* + * Return any outpoints which were not previously present in the coins + * cache, but were added as a result of validating the tx for mempool + * acceptance. This allows the caller to optionally remove the cache + * additions if the associated transaction ends up being rejected by + * the mempool. + */ + std::vector<COutPoint>& m_coins_to_uncache; + const bool m_test_accept; + }; + + // Single transaction acceptance + bool AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + +private: + // All the intermediate state that gets passed between the various levels + // of checking a given transaction. + struct Workspace { + Workspace(const CTransactionRef& ptx) : m_ptx(ptx), m_hash(ptx->GetHash()) {} + std::set<uint256> m_conflicts; + CTxMemPool::setEntries m_all_conflicting; + CTxMemPool::setEntries m_ancestors; + std::unique_ptr<CTxMemPoolEntry> m_entry; + + bool m_replacement_transaction; + CAmount m_modified_fees; + CAmount m_conflicting_fees; + size_t m_conflicting_size; + + const CTransactionRef& m_ptx; + const uint256& m_hash; + }; + + // Run the policy checks on a given transaction, excluding any script checks. + // Looks up inputs, calculates feerate, considers replacement, evaluates + // package limits, etc. As this function can be invoked for "free" by a peer, + // only tests that are fast should be done here (to avoid CPU DoS). + bool PreChecks(ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); + + // Run the script checks using our policy flags. As this can be slow, we should + // only invoke this on transactions that have otherwise passed policy checks. + bool PolicyScriptChecks(ATMPArgs& args, Workspace& ws, PrecomputedTransactionData& txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + // Re-run the script checks, using consensus flags, and try to cache the + // result in the scriptcache. This should be done after + // PolicyScriptChecks(). This requires that all inputs either be in our + // utxo set or in the mempool. + bool ConsensusScriptChecks(ATMPArgs& args, Workspace& ws, PrecomputedTransactionData &txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + // Try to add the transaction to the mempool, removing any conflicts first. + // Returns true if the transaction is in the mempool after any size + // limiting is performed, false otherwise. + bool Finalize(ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); + + // Compare a package's feerate against minimum allowed. + bool CheckFeeRate(size_t package_size, CAmount package_fee, CValidationState& state) + { + CAmount mempoolRejectFee = m_pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(package_size); + if (mempoolRejectFee > 0 && package_fee < mempoolRejectFee) { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); + } + + if (package_fee < ::minRelayTxFee.GetFee(package_size)) { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "min relay fee not met", strprintf("%d < %d", package_fee, ::minRelayTxFee.GetFee(package_size))); + } + return true; + } + +private: + CTxMemPool& m_pool; + CCoinsViewCache m_view; + CCoinsViewMemPool m_viewmempool; + CCoinsView m_dummy; + + // The package limits in effect at the time of invocation. + const size_t m_limit_ancestors; + const size_t m_limit_ancestor_size; + // These may be modified while evaluating a transaction (eg to account for + // in-mempool conflicts; see below). + size_t m_limit_descendants; + size_t m_limit_descendant_size; +}; + +bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) +{ + const CTransactionRef& ptx = ws.m_ptx; + const CTransaction& tx = *ws.m_ptx; + const uint256& hash = ws.m_hash; + + // Copy/alias what we need out of args + CValidationState &state = args.m_state; + bool* pfMissingInputs = args.m_missing_inputs; + const int64_t nAcceptTime = args.m_accept_time; + const bool bypass_limits = args.m_bypass_limits; + const CAmount& nAbsurdFee = args.m_absurd_fee; + std::vector<COutPoint>& coins_to_uncache = args.m_coins_to_uncache; + + // Alias what we need out of ws + std::set<uint256>& setConflicts = ws.m_conflicts; + CTxMemPool::setEntries& allConflicting = ws.m_all_conflicting; + CTxMemPool::setEntries& setAncestors = ws.m_ancestors; + std::unique_ptr<CTxMemPoolEntry>& entry = ws.m_entry; + bool& fReplacementTransaction = ws.m_replacement_transaction; + CAmount& nModifiedFees = ws.m_modified_fees; + CAmount& nConflictingFees = ws.m_conflicting_fees; + size_t& nConflictingSize = ws.m_conflicting_size; + if (pfMissingInputs) { *pfMissingInputs = false; } @@ -461,7 +574,8 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // Do not work on transactions that are too small. // A transaction with 1 segwit input and 1 P2WPHK output has non-witness size of 82 bytes. - // Transactions smaller than this are not relayed to reduce unnecessary malloc overhead. + // Transactions smaller than this are not relayed to mitigate CVE-2017-12842 by not relaying + // 64-byte transactions. if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) < MIN_STANDARD_TX_NONWITNESS_SIZE) return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "tx-size-small"); @@ -472,15 +586,14 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-final"); // is it already in the memory pool? - if (pool.exists(hash)) { + if (m_pool.exists(hash)) { return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-in-mempool"); } // Check for conflicts with in-memory transactions - std::set<uint256> setConflicts; for (const CTxIn &txin : tx.vin) { - const CTransaction* ptxConflicting = pool.GetConflictTx(txin.prevout); + const CTransaction* ptxConflicting = m_pool.GetConflictTx(txin.prevout); if (ptxConflicting) { if (!setConflicts.count(ptxConflicting->GetHash())) { @@ -514,395 +627,436 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool } } - { - CCoinsView dummy; - CCoinsViewCache view(&dummy); - - LockPoints lp; - CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip(); - CCoinsViewMemPool viewMemPool(&coins_cache, pool); - view.SetBackend(viewMemPool); - - // do all inputs exist? - for (const CTxIn& txin : tx.vin) { - if (!coins_cache.HaveCoinInCache(txin.prevout)) { - coins_to_uncache.push_back(txin.prevout); - } + LockPoints lp; + m_view.SetBackend(m_viewmempool); - // Note: this call may add txin.prevout to the coins cache - // (CoinsTip().cacheCoins) by way of FetchCoin(). It should be removed - // later (via coins_to_uncache) if this tx turns out to be invalid. - if (!view.HaveCoin(txin.prevout)) { - // Are inputs missing because we already have the tx? - for (size_t out = 0; out < tx.vout.size(); out++) { - // Optimistically just do efficient check of cache for outputs - if (coins_cache.HaveCoinInCache(COutPoint(hash, out))) { - return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-known"); - } - } - // Otherwise assume this might be an orphan tx for which we just haven't seen parents yet - if (pfMissingInputs) { - *pfMissingInputs = true; + CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip(); + // do all inputs exist? + for (const CTxIn& txin : tx.vin) { + if (!coins_cache.HaveCoinInCache(txin.prevout)) { + coins_to_uncache.push_back(txin.prevout); + } + + // Note: this call may add txin.prevout to the coins cache + // (coins_cache.cacheCoins) by way of FetchCoin(). It should be removed + // later (via coins_to_uncache) if this tx turns out to be invalid. + if (!m_view.HaveCoin(txin.prevout)) { + // Are inputs missing because we already have the tx? + for (size_t out = 0; out < tx.vout.size(); out++) { + // Optimistically just do efficient check of cache for outputs + if (coins_cache.HaveCoinInCache(COutPoint(hash, out))) { + return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-known"); } - return false; // fMissingInputs and !state.IsInvalid() is used to detect this condition, don't set state.Invalid() } + // Otherwise assume this might be an orphan tx for which we just haven't seen parents yet + if (pfMissingInputs) { + *pfMissingInputs = true; + } + return false; // fMissingInputs and !state.IsInvalid() is used to detect this condition, don't set state.Invalid() } + } - // Bring the best block into scope - view.GetBestBlock(); + // Bring the best block into scope + m_view.GetBestBlock(); - // we have all inputs cached now, so switch back to dummy, so we don't need to keep lock on mempool - view.SetBackend(dummy); + // we have all inputs cached now, so switch back to dummy (to protect + // against bugs where we pull more inputs from disk that miss being added + // to coins_to_uncache) + m_view.SetBackend(m_dummy); - // Only accept BIP68 sequence locked transactions that can be mined in the next - // block; we don't want our mempool filled up with transactions that can't - // be mined yet. - // Must keep pool.cs for this unless we change CheckSequenceLocks to take a - // CoinsViewCache instead of create its own - if (!CheckSequenceLocks(pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) - return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-BIP68-final"); + // Only accept BIP68 sequence locked transactions that can be mined in the next + // block; we don't want our mempool filled up with transactions that can't + // be mined yet. + // Must keep pool.cs for this unless we change CheckSequenceLocks to take a + // CoinsViewCache instead of create its own + if (!CheckSequenceLocks(m_pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) + return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-BIP68-final"); - CAmount nFees = 0; - if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), nFees)) { - return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); - } + CAmount nFees = 0; + if (!Consensus::CheckTxInputs(tx, state, m_view, GetSpendHeight(m_view), nFees)) { + return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); + } - // Check for non-standard pay-to-script-hash in inputs - if (fRequireStandard && !AreInputsStandard(tx, view)) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs"); + // Check for non-standard pay-to-script-hash in inputs + if (fRequireStandard && !AreInputsStandard(tx, m_view)) + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs"); - // Check for non-standard witness in P2WSH - if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, view)) - return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, REJECT_NONSTANDARD, "bad-witness-nonstandard"); + // Check for non-standard witness in P2WSH + if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view)) + return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, REJECT_NONSTANDARD, "bad-witness-nonstandard"); - int64_t nSigOpsCost = GetTransactionSigOpCost(tx, view, STANDARD_SCRIPT_VERIFY_FLAGS); + int64_t nSigOpsCost = GetTransactionSigOpCost(tx, m_view, STANDARD_SCRIPT_VERIFY_FLAGS); - // nModifiedFees includes any fee deltas from PrioritiseTransaction - CAmount nModifiedFees = nFees; - pool.ApplyDelta(hash, nModifiedFees); + // nModifiedFees includes any fee deltas from PrioritiseTransaction + nModifiedFees = nFees; + m_pool.ApplyDelta(hash, nModifiedFees); - // Keep track of transactions that spend a coinbase, which we re-scan - // during reorgs to ensure COINBASE_MATURITY is still met. - bool fSpendsCoinbase = false; - for (const CTxIn &txin : tx.vin) { - const Coin &coin = view.AccessCoin(txin.prevout); - if (coin.IsCoinBase()) { - fSpendsCoinbase = true; - break; - } + // Keep track of transactions that spend a coinbase, which we re-scan + // during reorgs to ensure COINBASE_MATURITY is still met. + bool fSpendsCoinbase = false; + for (const CTxIn &txin : tx.vin) { + const Coin &coin = m_view.AccessCoin(txin.prevout); + if (coin.IsCoinBase()) { + fSpendsCoinbase = true; + break; } + } - CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, ::ChainActive().Height(), - fSpendsCoinbase, nSigOpsCost, lp); - unsigned int nSize = entry.GetTxSize(); + entry.reset(new CTxMemPoolEntry(ptx, nFees, nAcceptTime, ::ChainActive().Height(), + fSpendsCoinbase, nSigOpsCost, lp)); + unsigned int nSize = entry->GetTxSize(); - if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-too-many-sigops", + if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-too-many-sigops", strprintf("%d", nSigOpsCost)); - CAmount mempoolRejectFee = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nSize); - if (!bypass_limits && mempoolRejectFee > 0 && nModifiedFees < mempoolRejectFee) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", strprintf("%d < %d", nModifiedFees, mempoolRejectFee)); - } + // No transactions are allowed below minRelayTxFee except from disconnected + // blocks + if (!bypass_limits && !CheckFeeRate(nSize, nModifiedFees, state)) return false; - // No transactions are allowed below minRelayTxFee except from disconnected blocks - if (!bypass_limits && nModifiedFees < ::minRelayTxFee.GetFee(nSize)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "min relay fee not met", strprintf("%d < %d", nModifiedFees, ::minRelayTxFee.GetFee(nSize))); - } - - if (nAbsurdFee && nFees > nAbsurdFee) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, + if (nAbsurdFee && nFees > nAbsurdFee) + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_HIGHFEE, "absurdly-high-fee", strprintf("%d > %d", nFees, nAbsurdFee)); - const CTxMemPool::setEntries setIterConflicting = pool.GetIterSet(setConflicts); - // Calculate in-mempool ancestors, up to a limit. - CTxMemPool::setEntries setAncestors; - size_t nLimitAncestors = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); - size_t nLimitAncestorSize = gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000; - size_t nLimitDescendants = gArgs.GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT); - size_t nLimitDescendantSize = gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000; - - if (setConflicts.size() == 1) { - // In general, when we receive an RBF transaction with mempool conflicts, we want to know whether we - // would meet the chain limits after the conflicts have been removed. However, there isn't a practical - // way to do this short of calculating the ancestor and descendant sets with an overlay cache of - // changed mempool entries. Due to both implementation and runtime complexity concerns, this isn't - // very realistic, thus we only ensure a limited set of transactions are RBF'able despite mempool - // conflicts here. Importantly, we need to ensure that some transactions which were accepted using - // the below carve-out are able to be RBF'ed, without impacting the security the carve-out provides - // for off-chain contract systems (see link in the comment below). - // - // Specifically, the subset of RBF transactions which we allow despite chain limits are those which - // conflict directly with exactly one other transaction (but may evict children of said transaction), - // and which are not adding any new mempool dependencies. Note that the "no new mempool dependencies" - // check is accomplished later, so we don't bother doing anything about it here, but if BIP 125 is - // amended, we may need to move that check to here instead of removing it wholesale. - // - // Such transactions are clearly not merging any existing packages, so we are only concerned with - // ensuring that (a) no package is growing past the package size (not count) limits and (b) we are - // not allowing something to effectively use the (below) carve-out spot when it shouldn't be allowed - // to. - // - // To check these we first check if we meet the RBF criteria, above, and increment the descendant - // limits by the direct conflict and its descendants (as these are recalculated in - // CalculateMempoolAncestors by assuming the new transaction being added is a new descendant, with no - // removals, of each parent's existing dependant set). The ancestor count limits are unmodified (as - // the ancestor limits should be the same for both our new transaction and any conflicts). - // We don't bother incrementing nLimitDescendants by the full removal count as that limit never comes - // into force here (as we're only adding a single transaction). - assert(setIterConflicting.size() == 1); - CTxMemPool::txiter conflict = *setIterConflicting.begin(); - - nLimitDescendants += 1; - nLimitDescendantSize += conflict->GetSizeWithDescendants(); - } - - std::string errString; - if (!pool.CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize, nLimitDescendants, nLimitDescendantSize, errString)) { - setAncestors.clear(); - // If CalculateMemPoolAncestors fails second time, we want the original error string. - std::string dummy_err_string; - // Contracting/payment channels CPFP carve-out: - // If the new transaction is relatively small (up to 40k weight) - // and has at most one ancestor (ie ancestor limit of 2, including - // the new transaction), allow it if its parent has exactly the - // descendant limit descendants. - // - // This allows protocols which rely on distrusting counterparties - // being able to broadcast descendants of an unconfirmed transaction - // to be secure by simply only having two immediately-spendable - // outputs - one for each counterparty. For more info on the uses for - // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html - if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || - !pool.CalculateMemPoolAncestors(entry, setAncestors, 2, nLimitAncestorSize, nLimitDescendants + 1, nLimitDescendantSize + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too-long-mempool-chain", errString); - } - } - - // A transaction that spends outputs that would be replaced by it is invalid. Now - // that we have the set of all ancestors we can detect this - // pathological case by making sure setConflicts and setAncestors don't - // intersect. - for (CTxMemPool::txiter ancestorIt : setAncestors) + const CTxMemPool::setEntries setIterConflicting = m_pool.GetIterSet(setConflicts); + // Calculate in-mempool ancestors, up to a limit. + if (setConflicts.size() == 1) { + // In general, when we receive an RBF transaction with mempool conflicts, we want to know whether we + // would meet the chain limits after the conflicts have been removed. However, there isn't a practical + // way to do this short of calculating the ancestor and descendant sets with an overlay cache of + // changed mempool entries. Due to both implementation and runtime complexity concerns, this isn't + // very realistic, thus we only ensure a limited set of transactions are RBF'able despite mempool + // conflicts here. Importantly, we need to ensure that some transactions which were accepted using + // the below carve-out are able to be RBF'ed, without impacting the security the carve-out provides + // for off-chain contract systems (see link in the comment below). + // + // Specifically, the subset of RBF transactions which we allow despite chain limits are those which + // conflict directly with exactly one other transaction (but may evict children of said transaction), + // and which are not adding any new mempool dependencies. Note that the "no new mempool dependencies" + // check is accomplished later, so we don't bother doing anything about it here, but if BIP 125 is + // amended, we may need to move that check to here instead of removing it wholesale. + // + // Such transactions are clearly not merging any existing packages, so we are only concerned with + // ensuring that (a) no package is growing past the package size (not count) limits and (b) we are + // not allowing something to effectively use the (below) carve-out spot when it shouldn't be allowed + // to. + // + // To check these we first check if we meet the RBF criteria, above, and increment the descendant + // limits by the direct conflict and its descendants (as these are recalculated in + // CalculateMempoolAncestors by assuming the new transaction being added is a new descendant, with no + // removals, of each parent's existing dependant set). The ancestor count limits are unmodified (as + // the ancestor limits should be the same for both our new transaction and any conflicts). + // We don't bother incrementing m_limit_descendants by the full removal count as that limit never comes + // into force here (as we're only adding a single transaction). + assert(setIterConflicting.size() == 1); + CTxMemPool::txiter conflict = *setIterConflicting.begin(); + + m_limit_descendants += 1; + m_limit_descendant_size += conflict->GetSizeWithDescendants(); + } + + std::string errString; + if (!m_pool.CalculateMemPoolAncestors(*entry, setAncestors, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, m_limit_descendant_size, errString)) { + setAncestors.clear(); + // If CalculateMemPoolAncestors fails second time, we want the original error string. + std::string dummy_err_string; + // Contracting/payment channels CPFP carve-out: + // If the new transaction is relatively small (up to 40k weight) + // and has at most one ancestor (ie ancestor limit of 2, including + // the new transaction), allow it if its parent has exactly the + // descendant limit descendants. + // + // This allows protocols which rely on distrusting counterparties + // being able to broadcast descendants of an unconfirmed transaction + // to be secure by simply only having two immediately-spendable + // outputs - one for each counterparty. For more info on the uses for + // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html + if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || + !m_pool.CalculateMemPoolAncestors(*entry, setAncestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too-long-mempool-chain", errString); + } + } + + // A transaction that spends outputs that would be replaced by it is invalid. Now + // that we have the set of all ancestors we can detect this + // pathological case by making sure setConflicts and setAncestors don't + // intersect. + for (CTxMemPool::txiter ancestorIt : setAncestors) + { + const uint256 &hashAncestor = ancestorIt->GetTx().GetHash(); + if (setConflicts.count(hashAncestor)) { - const uint256 &hashAncestor = ancestorIt->GetTx().GetHash(); - if (setConflicts.count(hashAncestor)) - { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-spends-conflicting-tx", - strprintf("%s spends conflicting transaction %s", - hash.ToString(), - hashAncestor.ToString())); - } + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-spends-conflicting-tx", + strprintf("%s spends conflicting transaction %s", + hash.ToString(), + hashAncestor.ToString())); } + } - // Check if it's economically rational to mine this transaction rather - // than the ones it replaces. - CAmount nConflictingFees = 0; - size_t nConflictingSize = 0; - uint64_t nConflictingCount = 0; - CTxMemPool::setEntries allConflicting; - - // If we don't hold the lock allConflicting might be incomplete; the - // subsequent RemoveStaged() and addUnchecked() calls don't guarantee - // mempool consistency for us. - const bool fReplacementTransaction = setConflicts.size(); - if (fReplacementTransaction) - { - CFeeRate newFeeRate(nModifiedFees, nSize); - std::set<uint256> setConflictsParents; - const int maxDescendantsToVisit = 100; - for (const auto& mi : setIterConflicting) { - // Don't allow the replacement to reduce the feerate of the - // mempool. - // - // We usually don't want to accept replacements with lower - // feerates than what they replaced as that would lower the - // feerate of the next block. Requiring that the feerate always - // be increased is also an easy-to-reason about way to prevent - // DoS attacks via replacements. - // - // We only consider the feerates of transactions being directly - // replaced, not their indirect descendants. While that does - // mean high feerate children are ignored when deciding whether - // or not to replace, we do require the replacement to pay more - // overall fees too, mitigating most cases. - CFeeRate oldFeeRate(mi->GetModifiedFee(), mi->GetTxSize()); - if (newFeeRate <= oldFeeRate) - { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", - strprintf("rejecting replacement %s; new feerate %s <= old feerate %s", - hash.ToString(), - newFeeRate.ToString(), - oldFeeRate.ToString())); - } - - for (const CTxIn &txin : mi->GetTx().vin) - { - setConflictsParents.insert(txin.prevout.hash); - } + // Check if it's economically rational to mine this transaction rather + // than the ones it replaces. + nConflictingFees = 0; + nConflictingSize = 0; + uint64_t nConflictingCount = 0; - nConflictingCount += mi->GetCountWithDescendants(); - } - // This potentially overestimates the number of actual descendants - // but we just want to be conservative to avoid doing too much - // work. - if (nConflictingCount <= maxDescendantsToVisit) { - // If not too many to replace, then calculate the set of - // transactions that would have to be evicted - for (CTxMemPool::txiter it : setIterConflicting) { - pool.CalculateDescendants(it, allConflicting); - } - for (CTxMemPool::txiter it : allConflicting) { - nConflictingFees += it->GetModifiedFee(); - nConflictingSize += it->GetTxSize(); - } - } else { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too many potential replacements", - strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", + // If we don't hold the lock allConflicting might be incomplete; the + // subsequent RemoveStaged() and addUnchecked() calls don't guarantee + // mempool consistency for us. + fReplacementTransaction = setConflicts.size(); + if (fReplacementTransaction) + { + CFeeRate newFeeRate(nModifiedFees, nSize); + std::set<uint256> setConflictsParents; + const int maxDescendantsToVisit = 100; + for (const auto& mi : setIterConflicting) { + // Don't allow the replacement to reduce the feerate of the + // mempool. + // + // We usually don't want to accept replacements with lower + // feerates than what they replaced as that would lower the + // feerate of the next block. Requiring that the feerate always + // be increased is also an easy-to-reason about way to prevent + // DoS attacks via replacements. + // + // We only consider the feerates of transactions being directly + // replaced, not their indirect descendants. While that does + // mean high feerate children are ignored when deciding whether + // or not to replace, we do require the replacement to pay more + // overall fees too, mitigating most cases. + CFeeRate oldFeeRate(mi->GetModifiedFee(), mi->GetTxSize()); + if (newFeeRate <= oldFeeRate) + { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", + strprintf("rejecting replacement %s; new feerate %s <= old feerate %s", hash.ToString(), - nConflictingCount, - maxDescendantsToVisit)); + newFeeRate.ToString(), + oldFeeRate.ToString())); } - for (unsigned int j = 0; j < tx.vin.size(); j++) + for (const CTxIn &txin : mi->GetTx().vin) { - // We don't want to accept replacements that require low - // feerate junk to be mined first. Ideally we'd keep track of - // the ancestor feerates and make the decision based on that, - // but for now requiring all new inputs to be confirmed works. - // - // Note that if you relax this to make RBF a little more useful, - // this may break the CalculateMempoolAncestors RBF relaxation, - // above. See the comment above the first CalculateMempoolAncestors - // call for more info. - if (!setConflictsParents.count(tx.vin[j].prevout.hash)) - { - // Rather than check the UTXO set - potentially expensive - - // it's cheaper to just check if the new input refers to a - // tx that's in the mempool. - if (pool.exists(tx.vin[j].prevout.hash)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "replacement-adds-unconfirmed", - strprintf("replacement %s adds unconfirmed input, idx %d", - hash.ToString(), j)); - } - } + setConflictsParents.insert(txin.prevout.hash); } - // The replacement must pay greater fees than the transactions it - // replaces - if we did the bandwidth used by those conflicting - // transactions would not be paid for. - if (nModifiedFees < nConflictingFees) - { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", - strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s", - hash.ToString(), FormatMoney(nModifiedFees), FormatMoney(nConflictingFees))); + nConflictingCount += mi->GetCountWithDescendants(); + } + // This potentially overestimates the number of actual descendants + // but we just want to be conservative to avoid doing too much + // work. + if (nConflictingCount <= maxDescendantsToVisit) { + // If not too many to replace, then calculate the set of + // transactions that would have to be evicted + for (CTxMemPool::txiter it : setIterConflicting) { + m_pool.CalculateDescendants(it, allConflicting); } - - // Finally in addition to paying more fees than the conflicts the - // new transaction must pay for its own bandwidth. - CAmount nDeltaFees = nModifiedFees - nConflictingFees; - if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize)) - { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", - strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", - hash.ToString(), - FormatMoney(nDeltaFees), - FormatMoney(::incrementalRelayFee.GetFee(nSize)))); + for (CTxMemPool::txiter it : allConflicting) { + nConflictingFees += it->GetModifiedFee(); + nConflictingSize += it->GetTxSize(); } + } else { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too many potential replacements", + strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", + hash.ToString(), + nConflictingCount, + maxDescendantsToVisit)); } - constexpr unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS; - - // Check against previous transactions - // The first loop above does all the inexpensive checks. - // Only if ALL inputs pass do we perform expensive ECDSA signature checks. - // Helps prevent CPU exhaustion denial-of-service attacks. - PrecomputedTransactionData txdata(tx); - if (!CheckInputs(tx, state, view, scriptVerifyFlags, true, false, txdata)) { - // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we - // need to turn both off, and compare against just turning off CLEANSTACK - // to see if the failure is specifically due to witness validation. - CValidationState stateDummy; // Want reported failures to be from first CheckInputs - if (!tx.HasWitness() && CheckInputs(tx, stateDummy, view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) && - !CheckInputs(tx, stateDummy, view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { - // Only the witness is missing, so the transaction itself may be fine. - state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, - state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); + for (unsigned int j = 0; j < tx.vin.size(); j++) + { + // We don't want to accept replacements that require low + // feerate junk to be mined first. Ideally we'd keep track of + // the ancestor feerates and make the decision based on that, + // but for now requiring all new inputs to be confirmed works. + // + // Note that if you relax this to make RBF a little more useful, + // this may break the CalculateMempoolAncestors RBF relaxation, + // above. See the comment above the first CalculateMempoolAncestors + // call for more info. + if (!setConflictsParents.count(tx.vin[j].prevout.hash)) + { + // Rather than check the UTXO set - potentially expensive - + // it's cheaper to just check if the new input refers to a + // tx that's in the mempool. + if (m_pool.exists(tx.vin[j].prevout.hash)) { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "replacement-adds-unconfirmed", + strprintf("replacement %s adds unconfirmed input, idx %d", + hash.ToString(), j)); + } } - assert(IsTransactionReason(state.GetReason())); - return false; // state filled in by CheckInputs } - // Check again against the current block tip's script verification - // flags to cache our script execution flags. This is, of course, - // useless if the next block has different script flags from the - // previous one, but because the cache tracks script flags for us it - // will auto-invalidate and we'll just have a few blocks of extra - // misses on soft-fork activation. - // - // This is also useful in case of bugs in the standard flags that cause - // transactions to pass as valid when they're actually invalid. For - // instance the STRICTENC flag was incorrectly allowing certain - // CHECKSIG NOT scripts to pass, even though they were invalid. - // - // There is a similar check in CreateNewBlock() to prevent creating - // invalid blocks (using TestBlockValidity), however allowing such - // transactions into the mempool can be exploited as a DoS attack. - unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(::ChainActive().Tip(), chainparams.GetConsensus()); - if (!CheckInputsFromMempoolAndCache(tx, state, view, pool, currentBlockScriptVerifyFlags, true, txdata)) { - return error("%s: BUG! PLEASE REPORT THIS! CheckInputs failed against latest-block but not STANDARD flags %s, %s", - __func__, hash.ToString(), FormatStateMessage(state)); - } - - if (test_accept) { - // Tx was accepted, but not added - return true; + // The replacement must pay greater fees than the transactions it + // replaces - if we did the bandwidth used by those conflicting + // transactions would not be paid for. + if (nModifiedFees < nConflictingFees) + { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", + strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s", + hash.ToString(), FormatMoney(nModifiedFees), FormatMoney(nConflictingFees))); } - // Remove conflicting transactions from the mempool - for (CTxMemPool::txiter it : allConflicting) + // Finally in addition to paying more fees than the conflicts the + // new transaction must pay for its own bandwidth. + CAmount nDeltaFees = nModifiedFees - nConflictingFees; + if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize)) { - LogPrint(BCLog::MEMPOOL, "replacing tx %s with %s for %s BTC additional fees, %d delta bytes\n", - it->GetTx().GetHash().ToString(), - hash.ToString(), - FormatMoney(nModifiedFees - nConflictingFees), - (int)nSize - (int)nConflictingSize); - if (plTxnReplaced) - plTxnReplaced->push_back(it->GetSharedTx()); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", + strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", + hash.ToString(), + FormatMoney(nDeltaFees), + FormatMoney(::incrementalRelayFee.GetFee(nSize)))); } - pool.RemoveStaged(allConflicting, false, MemPoolRemovalReason::REPLACED); + } + return true; +} + +bool MemPoolAccept::PolicyScriptChecks(ATMPArgs& args, Workspace& ws, PrecomputedTransactionData& txdata) +{ + const CTransaction& tx = *ws.m_ptx; - // This transaction should only count for fee estimation if: - // - it isn't a BIP 125 replacement transaction (may not be widely supported) - // - it's not being re-added during a reorg which bypasses typical mempool fee limits - // - the node is not behind - // - the transaction is not dependent on any other transactions in the mempool - bool validForFeeEstimation = !fReplacementTransaction && !bypass_limits && IsCurrentForFeeEstimation() && pool.HasNoInputsOf(tx); + CValidationState &state = args.m_state; - // Store transaction in memory - pool.addUnchecked(entry, setAncestors, validForFeeEstimation); + constexpr unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS; - // trim mempool and check if tx was trimmed - if (!bypass_limits) { - LimitMempoolSize(pool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); - if (!pool.exists(hash)) - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool full"); + // Check against previous transactions + // This is done last to help prevent CPU exhaustion denial-of-service attacks. + if (!CheckInputs(tx, state, m_view, scriptVerifyFlags, true, false, txdata)) { + // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we + // need to turn both off, and compare against just turning off CLEANSTACK + // to see if the failure is specifically due to witness validation. + CValidationState stateDummy; // Want reported failures to be from first CheckInputs + if (!tx.HasWitness() && CheckInputs(tx, stateDummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) && + !CheckInputs(tx, stateDummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { + // Only the witness is missing, so the transaction itself may be fine. + state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, + state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); } + assert(IsTransactionReason(state.GetReason())); + return false; // state filled in by CheckInputs } + return true; +} + +bool MemPoolAccept::ConsensusScriptChecks(ATMPArgs& args, Workspace& ws, PrecomputedTransactionData& txdata) +{ + const CTransaction& tx = *ws.m_ptx; + const uint256& hash = ws.m_hash; + + CValidationState &state = args.m_state; + const CChainParams& chainparams = args.m_chainparams; + + // Check again against the current block tip's script verification + // flags to cache our script execution flags. This is, of course, + // useless if the next block has different script flags from the + // previous one, but because the cache tracks script flags for us it + // will auto-invalidate and we'll just have a few blocks of extra + // misses on soft-fork activation. + // + // This is also useful in case of bugs in the standard flags that cause + // transactions to pass as valid when they're actually invalid. For + // instance the STRICTENC flag was incorrectly allowing certain + // CHECKSIG NOT scripts to pass, even though they were invalid. + // + // There is a similar check in CreateNewBlock() to prevent creating + // invalid blocks (using TestBlockValidity), however allowing such + // transactions into the mempool can be exploited as a DoS attack. + unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(::ChainActive().Tip(), chainparams.GetConsensus()); + if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags, true, txdata)) { + return error("%s: BUG! PLEASE REPORT THIS! CheckInputs failed against latest-block but not STANDARD flags %s, %s", + __func__, hash.ToString(), FormatStateMessage(state)); + } + + return true; +} + +bool MemPoolAccept::Finalize(ATMPArgs& args, Workspace& ws) +{ + const CTransaction& tx = *ws.m_ptx; + const uint256& hash = ws.m_hash; + CValidationState &state = args.m_state; + const bool bypass_limits = args.m_bypass_limits; + + CTxMemPool::setEntries& allConflicting = ws.m_all_conflicting; + CTxMemPool::setEntries& setAncestors = ws.m_ancestors; + const CAmount& nModifiedFees = ws.m_modified_fees; + const CAmount& nConflictingFees = ws.m_conflicting_fees; + const size_t& nConflictingSize = ws.m_conflicting_size; + const bool fReplacementTransaction = ws.m_replacement_transaction; + std::unique_ptr<CTxMemPoolEntry>& entry = ws.m_entry; + + // Remove conflicting transactions from the mempool + for (CTxMemPool::txiter it : allConflicting) + { + LogPrint(BCLog::MEMPOOL, "replacing tx %s with %s for %s BTC additional fees, %d delta bytes\n", + it->GetTx().GetHash().ToString(), + hash.ToString(), + FormatMoney(nModifiedFees - nConflictingFees), + (int)entry->GetTxSize() - (int)nConflictingSize); + if (args.m_replaced_transactions) + args.m_replaced_transactions->push_back(it->GetSharedTx()); + } + m_pool.RemoveStaged(allConflicting, false, MemPoolRemovalReason::REPLACED); + + // This transaction should only count for fee estimation if: + // - it isn't a BIP 125 replacement transaction (may not be widely supported) + // - it's not being re-added during a reorg which bypasses typical mempool fee limits + // - the node is not behind + // - the transaction is not dependent on any other transactions in the mempool + bool validForFeeEstimation = !fReplacementTransaction && !bypass_limits && IsCurrentForFeeEstimation() && m_pool.HasNoInputsOf(tx); + + // Store transaction in memory + m_pool.addUnchecked(*entry, setAncestors, validForFeeEstimation); + + // trim mempool and check if tx was trimmed + if (!bypass_limits) { + LimitMempoolSize(m_pool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); + if (!m_pool.exists(hash)) + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool full"); + } + return true; +} + +bool MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs& args) +{ + AssertLockHeld(cs_main); + LOCK(m_pool.cs); // mempool "read lock" (held through GetMainSignals().TransactionAddedToMempool()) + + Workspace workspace(ptx); + + if (!PreChecks(args, workspace)) return false; + + // Only compute the precomputed transaction data if we need to verify + // scripts (ie, other policy checks pass). We perform the inexpensive + // checks first and avoid hashing and signature verification unless those + // checks pass, to mitigate CPU exhaustion denial-of-service attacks. + PrecomputedTransactionData txdata(*ptx); + + if (!PolicyScriptChecks(args, workspace, txdata)) return false; + + if (!ConsensusScriptChecks(args, workspace, txdata)) return false; + + // Tx was accepted, but not added + if (args.m_test_accept) return true; + + if (!Finalize(args, workspace)) return false; + GetMainSignals().TransactionAddedToMempool(ptx); return true; } +} // anon namespace + /** (try to) add transaction to memory pool with a specified acceptance time **/ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool* pfMissingInputs, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced, bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::vector<COutPoint> coins_to_uncache; - bool res = AcceptToMemoryPoolWorker(chainparams, pool, state, tx, pfMissingInputs, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept); + MemPoolAccept::ATMPArgs args { chainparams, state, pfMissingInputs, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept }; + bool res = MemPoolAccept(pool).AcceptSingleTransaction(tx, args); if (!res) { // Remove coins that were not present in the coins cache before calling ATMPW; // this is to prevent memory DoS in case we receive a large number of @@ -1828,7 +1982,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // If such overwrites are allowed, coinbases and transactions depending upon those // can be duplicated to remove the ability to spend the first instance -- even after // being sent to another address. - // See BIP30 and http://r6.ca/blog/20120206T005236Z.html for more information. + // See BIP30, CVE-2012-1909, and http://r6.ca/blog/20120206T005236Z.html for more information. // This logic is not necessary for memory pool transactions, as AcceptToMemoryPool // already refuses previously-known transaction ids entirely. // This rule was originally applied to all blocks with a timestamp after March 15, 2012, 0:00 UTC. @@ -2508,6 +2662,8 @@ void CChainState::PruneBlockIndexCandidates() { /** * Try to make some progress towards making pindexMostWork the active block. * pblock is either nullptr or a pointer to a CBlock corresponding to pindexMostWork. + * + * @returns true unless a system error occurred */ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) { @@ -2627,15 +2783,6 @@ static void LimitValidationInterfaceQueue() LOCKS_EXCLUDED(cs_main) { } } -/** - * Make the best chain active, in multiple steps. The result is either failure - * or an activated best chain. pblock is either nullptr or a pointer to a block - * that is already loaded (to avoid loading it again from disk). - * - * ActivateBestChain is split into steps (see ActivateBestChainStep) so that - * we avoid holding cs_main for an extended period of time; the length of this - * call may be quite long during reindexing or a substantial reorg. - */ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr<const CBlock> pblock) { // Note that while we're often called here from ProcessNewBlock, this is // far from a guarantee. Things in the P2P/RPC will often end up calling @@ -2683,8 +2830,10 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& bool fInvalidFound = false; std::shared_ptr<const CBlock> nullBlockPtr; - if (!ActivateBestChainStep(state, chainparams, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connectTrace)) + if (!ActivateBestChainStep(state, chainparams, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connectTrace)) { + // A system error occurred return false; + } blocks_connected = true; if (fInvalidFound) { @@ -3105,6 +3254,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-multiple", "more than one coinbase"); // Check transactions + // Must check for duplicate inputs (see CVE-2018-17144) for (const auto& tx : block.vtx) if (!CheckTransaction(*tx, state, true)) return state.Invalid(state.GetReason(), false, state.GetRejectCode(), state.GetRejectReason(), @@ -3937,28 +4087,31 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) EXCLUSIVE_LOCKS_RE return true; } -bool LoadChainTip(const CChainParams& chainparams) +bool CChainState::LoadChainTip(const CChainParams& chainparams) { AssertLockHeld(cs_main); - const CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip(); + const CCoinsViewCache& coins_cache = CoinsTip(); assert(!coins_cache.GetBestBlock().IsNull()); // Never called when the coins view is empty + const CBlockIndex* tip = m_chain.Tip(); - if (::ChainActive().Tip() && - ::ChainActive().Tip()->GetBlockHash() == coins_cache.GetBestBlock()) return true; + if (tip && tip->GetBlockHash() == coins_cache.GetBestBlock()) { + return true; + } // Load pointer to end of best chain CBlockIndex* pindex = LookupBlockIndex(coins_cache.GetBestBlock()); if (!pindex) { return false; } - ::ChainActive().SetTip(pindex); - - ::ChainstateActive().PruneBlockIndexCandidates(); + m_chain.SetTip(pindex); + PruneBlockIndexCandidates(); + tip = m_chain.Tip(); LogPrintf("Loaded best chain: hashBestChain=%s height=%d date=%s progress=%f\n", - ::ChainActive().Tip()->GetBlockHash().ToString(), ::ChainActive().Height(), - FormatISO8601DateTime(::ChainActive().Tip()->GetBlockTime()), - GuessVerificationProgress(chainparams.TxData(), ::ChainActive().Tip())); + tip->GetBlockHash().ToString(), + m_chain.Height(), + FormatISO8601DateTime(tip->GetBlockTime()), + GuessVerificationProgress(chainparams.TxData(), tip)); return true; } @@ -4093,13 +4246,14 @@ bool CChainState::RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& i return true; } -bool CChainState::ReplayBlocks(const CChainParams& params, CCoinsView* view) +bool CChainState::ReplayBlocks(const CChainParams& params) { LOCK(cs_main); - CCoinsViewCache cache(view); + CCoinsView& db = this->CoinsDB(); + CCoinsViewCache cache(&db); - std::vector<uint256> hashHeads = view->GetHeadBlocks(); + std::vector<uint256> hashHeads = db.GetHeadBlocks(); if (hashHeads.empty()) return true; // We're already in a consistent state. if (hashHeads.size() != 2) return error("ReplayBlocks(): unknown inconsistent state"); @@ -4159,10 +4313,6 @@ bool CChainState::ReplayBlocks(const CChainParams& params, CCoinsView* view) return true; } -bool ReplayBlocks(const CChainParams& params, CCoinsView* view) { - return ::ChainstateActive().ReplayBlocks(params, view); -} - //! Helper for CChainState::RewindBlockIndex void CChainState::EraseBlockData(CBlockIndex* index) { diff --git a/src/validation.h b/src/validation.h index 99850f71d9..615b83f028 100644 --- a/src/validation.h +++ b/src/validation.h @@ -211,7 +211,7 @@ static const uint64_t MIN_DISK_SPACE_FOR_BLOCK_FILES = 550 * 1024 * 1024; * @param[in] pblock The block we want to process. * @param[in] fForceProcessing Process this block even if unrequested; used for non-network block sources and whitelisted peers. * @param[out] fNewBlock A boolean which is set to indicate if the block was first received via this call - * @return True if state.IsValid() + * @returns If the block was processed, independently of block validity */ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<const CBlock> pblock, bool fForceProcessing, bool* fNewBlock) LOCKS_EXCLUDED(cs_main); @@ -240,8 +240,6 @@ bool LoadGenesisBlock(const CChainParams& chainparams); /** Load the block tree and coins database from disk, * initializing state if we're running with -reindex. */ bool LoadBlockIndex(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main); -/** Update the chain tip based on database information. */ -bool LoadChainTip(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Unload database information */ void UnloadBlockIndex(); /** Run an instance of the script checking thread */ @@ -400,9 +398,6 @@ public: bool VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, int nCheckLevel, int nCheckDepth); }; -/** Replay blocks that aren't fully applied to the database. */ -bool ReplayBlocks(const CChainParams& params, CCoinsView* view); - CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Find the last common block between the parameter chain and a locator. */ @@ -653,6 +648,8 @@ public: * * If FlushStateMode::NONE is used, then FlushStateToDisk(...) won't do anything * besides checking if we need to prune. + * + * @returns true unless a system error occurred */ bool FlushStateToDisk( const CChainParams& chainparams, @@ -667,7 +664,24 @@ public: //! if we pruned. void PruneAndFlush(); - bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr<const CBlock> pblock) LOCKS_EXCLUDED(cs_main); + /** + * Make the best chain active, in multiple steps. The result is either failure + * or an activated best chain. pblock is either nullptr or a pointer to a block + * that is already loaded (to avoid loading it again from disk). + * + * ActivateBestChain is split into steps (see ActivateBestChainStep) so that + * we avoid holding cs_main for an extended period of time; the length of this + * call may be quite long during reindexing or a substantial reorg. + * + * May not be called with cs_main held. May not be called in a + * validationinterface callback. + * + * @returns true unless a system error occurred + */ + bool ActivateBestChain( + CValidationState& state, + const CChainParams& chainparams, + std::shared_ptr<const CBlock> pblock) LOCKS_EXCLUDED(cs_main); bool AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -684,7 +698,8 @@ public: bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); void ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool ReplayBlocks(const CChainParams& params, CCoinsView* view); + /** Replay blocks that aren't fully applied to the database. */ + bool ReplayBlocks(const CChainParams& params); bool RewindBlockIndex(const CChainParams& params) LOCKS_EXCLUDED(cs_main); bool LoadGenesisBlock(const CChainParams& chainparams); @@ -702,6 +717,9 @@ public: */ void CheckBlockIndex(const Consensus::Params& consensusParams); + /** Update the chain tip based on database information, i.e. CoinsTip()'s best block. */ + bool LoadChainTip(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + private: bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b88aabd0fa..216205ed61 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1648,8 +1648,10 @@ static UniValue gettransaction(const JSONRPCRequest& request) "\nGet detailed information about in-wallet transaction <txid>\n", { {"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"}, - {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Whether to include watch-only addresses in balance calculation and details[]"}, - {"decode", RPCArg::Type::BOOL, /* default */ "false", "Whether to add a field with the decoded transaction"}, + {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", + "Whether to include watch-only addresses in balance calculation and details[]"}, + {"verbose", RPCArg::Type::BOOL, /* default */ "false", + "Whether to include a `decoded` field containing the decoded transaction (equivalent to RPC decoderawtransaction)"}, }, RPCResult{ "{\n" @@ -1685,7 +1687,8 @@ static UniValue gettransaction(const JSONRPCRequest& request) " ,...\n" " ],\n" " \"hex\" : \"data\" (string) Raw data for transaction\n" - " \"decoded\" : transaction (json object) Optional, the decoded transaction\n" + " \"decoded\" : transaction (json object) Optional, the decoded transaction (only present when `verbose` is passed), equivalent to the\n" + " RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed.\n" "}\n" }, RPCExamples{ @@ -1711,7 +1714,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) filter |= ISMINE_WATCH_ONLY; } - bool decode_tx = request.params[2].isNull() ? false : request.params[2].get_bool(); + bool verbose = request.params[2].isNull() ? false : request.params[2].get_bool(); UniValue entry(UniValue::VOBJ); auto it = pwallet->mapWallet.find(hash); @@ -1738,7 +1741,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) std::string strHex = EncodeHexTx(*wtx.tx, pwallet->chain().rpcSerializationFlags()); entry.pushKV("hex", strHex); - if (decode_tx) { + if (verbose) { UniValue decoded(UniValue::VOBJ); TxToUniv(*wtx.tx, uint256(), decoded, false); entry.pushKV("decoded", decoded); @@ -4189,7 +4192,7 @@ static const CRPCCommand commands[] = { "wallet", "getrawchangeaddress", &getrawchangeaddress, {"address_type"} }, { "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf"} }, { "wallet", "getreceivedbylabel", &getreceivedbylabel, {"label","minconf"} }, - { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly","decode"} }, + { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly","verbose"} }, { "wallet", "getunconfirmedbalance", &getunconfirmedbalance, {} }, { "wallet", "getbalances", &getbalances, {} }, { "wallet", "getwalletinfo", &getwalletinfo, {} }, diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py index 454eb583f7..fd69bbd2c7 100644 --- a/test/functional/data/invalid_txs.py +++ b/test/functional/data/invalid_txs.py @@ -24,7 +24,24 @@ import abc from test_framework.messages import CTransaction, CTxIn, CTxOut, COutPoint from test_framework import script as sc from test_framework.blocktools import create_tx_with_script, MAX_BLOCK_SIGOPS - +from test_framework.script import ( + CScript, + OP_CAT, + OP_SUBSTR, + OP_LEFT, + OP_RIGHT, + OP_INVERT, + OP_AND, + OP_OR, + OP_XOR, + OP_2MUL, + OP_2DIV, + OP_MUL, + OP_DIV, + OP_MOD, + OP_LSHIFT, + OP_RSHIFT +) basic_p2sh = sc.CScript([sc.OP_HASH160, sc.hash160(sc.CScript([sc.OP_0])), sc.OP_EQUAL]) @@ -82,6 +99,8 @@ class InputMissing(BadTxTemplate): return tx +# The following check prevents exploit of lack of merkle +# tree depth commitment (CVE-2017-12842) class SizeTooSmall(BadTxTemplate): reject_reason = "tx-size-small" expect_disconnect = False @@ -178,7 +197,44 @@ class TooManySigops(BadTxTemplate): script_pub_key=lotsa_checksigs, amount=1) +def getDisabledOpcodeTemplate(opcode): + """ Creates disabled opcode tx template class""" + def get_tx(self): + tx = CTransaction() + vin = self.valid_txin + vin.scriptSig = CScript([opcode]) + tx.vin.append(vin) + tx.vout.append(CTxOut(1, basic_p2sh)) + tx.calc_sha256() + return tx + + return type('DisabledOpcode_' + str(opcode), (BadTxTemplate,), { + 'reject_reason': "disabled opcode", + 'expect_disconnect': True, + 'get_tx': get_tx, + 'valid_in_block' : True + }) + +# Disabled opcode tx templates (CVE-2010-5137) +DisabledOpcodeTemplates = [getDisabledOpcodeTemplate(opcode) for opcode in [ + OP_CAT, + OP_SUBSTR, + OP_LEFT, + OP_RIGHT, + OP_INVERT, + OP_AND, + OP_OR, + OP_XOR, + OP_2MUL, + OP_2DIV, + OP_MUL, + OP_DIV, + OP_MOD, + OP_LSHIFT, + OP_RSHIFT]] + def iter_all_templates(): """Iterate through all bad transaction template types.""" return BadTxTemplate.__subclasses__() + diff --git a/test/functional/data/wallets/high_minversion/GENERATE.md b/test/functional/data/wallets/high_minversion/GENERATE.md new file mode 100644 index 0000000000..e55c4557ca --- /dev/null +++ b/test/functional/data/wallets/high_minversion/GENERATE.md @@ -0,0 +1,8 @@ +The wallet has been created by starting Bitcoin Core with the options +`-regtest -datadir=/tmp -nowallet -walletdir=$(pwd)/test/functional/data/wallets/`. + +In the source code, `WalletFeature::FEATURE_LATEST` has been modified to be large, so that the minversion is too high +for a current build of the wallet. + +The wallet has then been created with the RPC `createwallet high_minversion true true`, so that a blank wallet with +private keys disabled is created. diff --git a/test/functional/feature_assumevalid.py b/test/functional/feature_assumevalid.py index 420a3a7688..1b434c4485 100755 --- a/test/functional/feature_assumevalid.py +++ b/test/functional/feature_assumevalid.py @@ -40,7 +40,7 @@ from test_framework.messages import ( CTxIn, CTxOut, msg_block, - msg_headers + msg_headers, ) from test_framework.mininode import P2PInterface from test_framework.script import (CScript, OP_TRUE) @@ -180,7 +180,7 @@ class AssumeValidTest(BitcoinTestFramework): for i in range(2202): p2p1.send_message(msg_block(self.blocks[i])) # Syncing 2200 blocks can take a while on slow systems. Give it plenty of time to sync. - p2p1.sync_with_ping(200) + p2p1.sync_with_ping(960) assert_equal(self.nodes[1].getblock(self.nodes[1].getbestblockhash())['height'], 2202) # Send blocks to node2. Block 102 will be rejected. diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 12a52935ef..c74270febc 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -806,7 +806,7 @@ class FullBlockTest(BitcoinTestFramework): # # Blocks are not allowed to contain a transaction whose id matches that of an earlier, # not-fully-spent transaction in the same chain. To test, make identical coinbases; - # the second one should be rejected. + # the second one should be rejected. See also CVE-2012-1909. # self.log.info("Reject a block with a transaction with a duplicate hash of a previous transaction (BIP30)") self.move_tip(60) @@ -1261,7 +1261,7 @@ class FullBlockTest(BitcoinTestFramework): self.save_spendable_output() spend = self.get_spendable_output() - self.send_blocks(blocks, True, timeout=480) + self.send_blocks(blocks, True, timeout=960) chain1_tip = i # now create alt chain of same length @@ -1273,14 +1273,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=480) + self.send_blocks([block], True, timeout=960) # ... 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=480) + self.send_blocks([block], True, timeout=960) 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/feature_dbcrash.py b/test/functional/feature_dbcrash.py index b86f6af4ca..6bd6bb5b8c 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -50,7 +50,7 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = False - self.rpc_timeout = 180 + self.rpc_timeout = 480 # Set -maxmempool=0 to turn off mempool memory sharing with dbcache # Set -rpcservertimeout=900 to reduce socket disconnects in this diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index 13cf951550..da00b773ad 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -7,7 +7,11 @@ import os from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, wait_until, connect_nodes_bi +from test_framework.util import ( + assert_equal, + wait_until, + connect_nodes, +) class NotificationsTest(BitcoinTestFramework): @@ -58,7 +62,7 @@ class NotificationsTest(BitcoinTestFramework): self.log.info("test -walletnotify after rescan") # restart node to rescan to force wallet notifications self.start_node(1) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py index 727f4b9589..51523f13e7 100755 --- a/test/functional/feature_pruning.py +++ b/test/functional/feature_pruning.py @@ -92,6 +92,7 @@ class PruneTest(BitcoinTestFramework): ["-maxreceivebuffer=20000"], ["-prune=550"], ] + self.rpc_timeout = 120 def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index b9db618575..d2826dd1b7 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -257,7 +257,7 @@ class SegWitTest(BitcoinTestFramework): tx.vin.append(CTxIn(COutPoint(int(txid2, 16), 0), b"")) tx.vout.append(CTxOut(int(49.95 * COIN), CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE]))) # Huge fee tx.calc_sha256() - txid3 = self.nodes[0].sendrawtransaction(ToHex(tx)) + txid3 = self.nodes[0].sendrawtransaction(ToHex(tx), 0) assert tx.wit.is_null() assert txid3 in self.nodes[0].getrawmempool() diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 209a222004..dee7a04516 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -183,6 +183,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): self.check_mempool_result( result_expected=[{'txid': tx.rehash(), 'allowed': True}], rawtxs=[tx.serialize().hex()], + maxfeerate=0, ) self.log.info('A transaction with no outputs') @@ -211,6 +212,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): rawtxs=[tx.serialize().hex()], ) + # The following two validations prevent overflow of the output amounts (see CVE-2010-5139). self.log.info('A transaction with too large output value') tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference))) tx.vout[0].nValue = 21000000 * COIN + 1 diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index 788aabc192..f9231614ce 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -27,7 +27,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, ) from test_framework.script import CScriptNum @@ -54,7 +54,7 @@ class MiningTest(BitcoinTestFramework): assert_equal(mining_info['currentblocktx'], 0) assert_equal(mining_info['currentblockweight'], 4000) self.restart_node(0) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) def run_test(self): self.mine_chain() diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py index 1b11a2a294..23dea4b729 100755 --- a/test/functional/p2p_disconnect_ban.py +++ b/test/functional/p2p_disconnect_ban.py @@ -9,7 +9,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, wait_until, ) @@ -18,6 +18,10 @@ class DisconnectBanTest(BitcoinTestFramework): self.num_nodes = 2 def run_test(self): + self.log.info("Connect nodes both way") + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 0) + self.log.info("Test setban and listbanned RPCs") self.log.info("setban: successfully ban single IP address") @@ -74,7 +78,9 @@ class DisconnectBanTest(BitcoinTestFramework): # Clear ban lists self.nodes[1].clearbanned() - connect_nodes_bi(self.nodes, 0, 1) + self.log.info("Connect nodes both way") + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 0) self.log.info("Test disconnectnode RPCs") @@ -93,7 +99,7 @@ class DisconnectBanTest(BitcoinTestFramework): assert not [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1] self.log.info("disconnectnode: successfully reconnect node") - connect_nodes_bi(self.nodes, 0, 1) # reconnect the node + connect_nodes(self.nodes[0], 1) # reconnect the node assert_equal(len(self.nodes[0].getpeerinfo()), 2) assert [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1] diff --git a/test/functional/p2p_invalid_block.py b/test/functional/p2p_invalid_block.py index 1e0b876593..905534b862 100755 --- a/test/functional/p2p_invalid_block.py +++ b/test/functional/p2p_invalid_block.py @@ -53,10 +53,11 @@ class InvalidBlockRequestTest(BitcoinTestFramework): block_time = best_block["time"] + 1 # Use merkle-root malleability to generate an invalid block with - # same blockheader. + # same blockheader (CVE-2012-2459). # Manufacture a block with 3 transactions (coinbase, spend of prior # coinbase, spend of that spend). Duplicate the 3rd transaction to # leave merkle root and blockheader unchanged but invalidate the block. + # For more information on merkle-root malleability see src/consensus/merkle.cpp. self.log.info("Test merkle root malleability.") block2 = create_block(tip, create_coinbase(height), block_time) @@ -81,15 +82,16 @@ class InvalidBlockRequestTest(BitcoinTestFramework): node.p2p.send_blocks_and_test([block2], node, success=False, reject_reason='bad-txns-duplicate') - # Check transactions for duplicate inputs + # Check transactions for duplicate inputs (CVE-2018-17144) self.log.info("Test duplicate input block.") - block2_orig.vtx[2].vin.append(block2_orig.vtx[2].vin[0]) - block2_orig.vtx[2].rehash() - block2_orig.hashMerkleRoot = block2_orig.calc_merkle_root() - block2_orig.rehash() - block2_orig.solve() - node.p2p.send_blocks_and_test([block2_orig], node, success=False, reject_reason='bad-txns-inputs-duplicate') + block2_dup = copy.deepcopy(block2_orig) + block2_dup.vtx[2].vin.append(block2_dup.vtx[2].vin[0]) + block2_dup.vtx[2].rehash() + block2_dup.hashMerkleRoot = block2_dup.calc_merkle_root() + block2_dup.rehash() + block2_dup.solve() + node.p2p.send_blocks_and_test([block2_dup], node, success=False, reject_reason='bad-txns-inputs-duplicate') self.log.info("Test very broken block.") @@ -105,5 +107,31 @@ class InvalidBlockRequestTest(BitcoinTestFramework): node.p2p.send_blocks_and_test([block3], node, success=False, reject_reason='bad-cb-amount') + # Complete testing of CVE-2012-2459 by sending the original block. + # It should be accepted even though it has the same hash as the mutated one. + + self.log.info("Test accepting original block after rejecting its mutated version.") + node.p2p.send_blocks_and_test([block2_orig], node, success=True, timeout=5) + + # Update tip info + height += 1 + block_time += 1 + tip = int(block2_orig.hash, 16) + + # Complete testing of CVE-2018-17144, by checking for the inflation bug. + # Create a block that spends the output of a tx in a previous block. + block4 = create_block(tip, create_coinbase(height), block_time) + tx3 = create_tx_with_script(tx2, 0, script_sig=b'\x51', amount=50 * COIN) + + # Duplicates input + tx3.vin.append(tx3.vin[0]) + tx3.rehash() + block4.vtx.append(tx3) + block4.hashMerkleRoot = block4.calc_merkle_root() + block4.rehash() + block4.solve() + self.log.info("Test inflation by duplicating input") + node.p2p.send_blocks_and_test([block4], node, success=False, reject_reason='bad-txns-inputs-duplicate') + if __name__ == '__main__': InvalidBlockRequestTest().main() diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index d5c68e622b..58c72f89d8 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -85,7 +85,7 @@ class InvalidMessagesTest(BitcoinTestFramework): # Peer 1, despite serving up a bunch of nonsense, should still be connected. self.log.info("Waiting for node to drop junk messages.") - node.p2p.sync_with_ping(timeout=120) + node.p2p.sync_with_ping(timeout=320) assert node.p2p.is_connected # diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py index a4650df8ee..e6451d9f18 100755 --- a/test/functional/p2p_node_network_limited.py +++ b/test/functional/p2p_node_network_limited.py @@ -14,7 +14,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, disconnect_nodes, - connect_nodes_bi, + connect_nodes, wait_until, ) @@ -64,7 +64,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): assert_equal(int(self.nodes[0].getnetworkinfo()['localservices'], 16), expected_services) self.log.info("Mine enough blocks to reach the NODE_NETWORK_LIMITED range.") - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) blocks = self.nodes[1].generatetoaddress(292, self.nodes[1].get_deterministic_priv_key().address) self.sync_blocks([self.nodes[0], self.nodes[1]]) @@ -90,7 +90,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): # connect unsynced node 2 with pruned NODE_NETWORK_LIMITED peer # because node 2 is in IBD and node 0 is a NODE_NETWORK_LIMITED peer, sync must not be possible - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 2) try: self.sync_blocks([self.nodes[0], self.nodes[2]], timeout=5) except: @@ -99,7 +99,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): assert_equal(self.nodes[2].getblockheader(self.nodes[2].getbestblockhash())['height'], 0) # now connect also to node 1 (non pruned) - connect_nodes_bi(self.nodes, 1, 2) + connect_nodes(self.nodes[1], 2) # sync must be possible self.sync_blocks() @@ -111,7 +111,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): self.nodes[0].generatetoaddress(10, self.nodes[0].get_deterministic_priv_key().address) # connect node1 (non pruned) with node0 (pruned) and check if the can sync - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) # sync must be possible, node 1 is no longer in IBD and should therefore connect to node 0 (NODE_NETWORK_LIMITED) self.sync_blocks([self.nodes[0], self.nodes[1]]) diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py index 19d78ff303..aada04f66f 100755 --- a/test/functional/p2p_tx_download.py +++ b/test/functional/p2p_tx_download.py @@ -121,6 +121,7 @@ class TxDownloadTest(BitcoinTestFramework): # peer, plus # * the first time it is re-requested from the outbound peer, plus # * 2 seconds to avoid races + assert self.nodes[1].getpeerinfo()[0]['inbound'] == False timeout = 2 + (MAX_GETDATA_RANDOM_DELAY + INBOUND_PEER_TX_DELAY) + ( GETDATA_TX_INTERVAL + MAX_GETDATA_RANDOM_DELAY) self.log.info("Tx should be received at node 1 after {} seconds".format(timeout)) diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index b621081752..c956af1cbe 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -12,7 +12,7 @@ from test_framework.util import ( assert_greater_than, assert_greater_than_or_equal, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, count_bytes, find_vout_for_address, ) @@ -35,10 +35,10 @@ class RawTransactionsTest(BitcoinTestFramework): def setup_network(self): self.setup_nodes() - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 1, 2) - connect_nodes_bi(self.nodes, 0, 2) - connect_nodes_bi(self.nodes, 0, 3) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) + connect_nodes(self.nodes[0], 3) def run_test(self): self.min_relay_tx_fee = self.nodes[0].getnetworkinfo()['relayfee'] @@ -508,10 +508,10 @@ class RawTransactionsTest(BitcoinTestFramework): for node in self.nodes: node.settxfee(self.min_relay_tx_fee) - connect_nodes_bi(self.nodes,0,1) - connect_nodes_bi(self.nodes,1,2) - connect_nodes_bi(self.nodes,0,2) - connect_nodes_bi(self.nodes,0,3) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) + connect_nodes(self.nodes[0], 3) # Again lock the watchonly UTXO or nodes[0] may spend it, because # lockunspent is memory-only and thus lost on restart self.nodes[0].lockunspent(False, [{"txid": self.watchonly_txid, "vout": self.watchonly_vout}]) diff --git a/test/functional/rpc_invalidateblock.py b/test/functional/rpc_invalidateblock.py index 3d3f694fd3..595b40f7cb 100755 --- a/test/functional/rpc_invalidateblock.py +++ b/test/functional/rpc_invalidateblock.py @@ -8,7 +8,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE from test_framework.util import ( assert_equal, - connect_nodes_bi, + connect_nodes, wait_until, ) @@ -33,7 +33,7 @@ class InvalidateTest(BitcoinTestFramework): assert_equal(self.nodes[1].getblockcount(), 6) self.log.info("Connect nodes to force a reorg") - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) self.sync_blocks(self.nodes[0:2]) assert_equal(self.nodes[0].getblockcount(), 6) badhash = self.nodes[1].getblockhash(2) @@ -44,7 +44,7 @@ class InvalidateTest(BitcoinTestFramework): assert_equal(self.nodes[0].getbestblockhash(), besthash_n0) self.log.info("Make sure we won't reorg to a lower work chain:") - connect_nodes_bi(self.nodes, 1, 2) + connect_nodes(self.nodes[1], 2) self.log.info("Sync node 2 to node 1 so both have 6 blocks") self.sync_blocks(self.nodes[1:3]) assert_equal(self.nodes[2].getblockcount(), 6) diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 104c66aaa7..e24bf3111b 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -15,7 +15,7 @@ from test_framework.util import ( assert_greater_than_or_equal, assert_greater_than, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, p2p_port, wait_until, ) @@ -54,6 +54,10 @@ class NetTest(BitcoinTestFramework): self.extra_args = [["-minrelaytxfee=0.00001000"],["-minrelaytxfee=0.00000500"]] def run_test(self): + self.log.info('Connect nodes both way') + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 0) + self._test_connection_count() self._test_getnettotals() self._test_getnetworkinfo() @@ -62,7 +66,7 @@ class NetTest(BitcoinTestFramework): self._test_getnodeaddresses() def _test_connection_count(self): - # connect_nodes_bi connects each node to the other + # connect_nodes connects each node to the other assert_equal(self.nodes[0].getconnectioncount(), 2) def _test_getnettotals(self): @@ -105,7 +109,10 @@ class NetTest(BitcoinTestFramework): wait_until(lambda: self.nodes[0].getnetworkinfo()['connections'] == 0, timeout=3) self.nodes[0].setnetworkactive(state=True) - connect_nodes_bi(self.nodes, 0, 1) + self.log.info('Connect nodes both way') + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 0) + assert_equal(self.nodes[0].getnetworkinfo()['networkactive'], True) assert_equal(self.nodes[0].getnetworkinfo()['connections'], 2) diff --git a/test/functional/rpc_preciousblock.py b/test/functional/rpc_preciousblock.py index 2d5631bb27..0663ffdf5b 100755 --- a/test/functional/rpc_preciousblock.py +++ b/test/functional/rpc_preciousblock.py @@ -7,7 +7,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - connect_nodes_bi, + connect_nodes, ) def unidirectional_node_sync_via_rpc(node_src, node_dest): @@ -60,7 +60,7 @@ class PreciousTest(BitcoinTestFramework): self.log.info("Connect nodes and check no reorg occurs") # Submit competing blocks via RPC so any reorg should occur before we proceed (no way to wait on inaction for p2p sync) node_sync_via_rpc(self.nodes[0:2]) - connect_nodes_bi(self.nodes,0,1) + connect_nodes(self.nodes[0], 1) assert_equal(self.nodes[0].getbestblockhash(), hashC) assert_equal(self.nodes[1].getbestblockhash(), hashG) self.log.info("Make Node0 prefer block G") @@ -97,8 +97,8 @@ class PreciousTest(BitcoinTestFramework): hashL = self.nodes[2].getbestblockhash() self.log.info("Connect nodes and check no reorg occurs") node_sync_via_rpc(self.nodes[1:3]) - connect_nodes_bi(self.nodes,1,2) - connect_nodes_bi(self.nodes,0,2) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) assert_equal(self.nodes[0].getbestblockhash(), hashH) assert_equal(self.nodes[1].getbestblockhash(), hashH) assert_equal(self.nodes[2].getbestblockhash(), hashL) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 5a04e0c8d8..61572654e0 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -11,7 +11,7 @@ from test_framework.util import ( assert_equal, assert_greater_than, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, disconnect_nodes, find_output, ) @@ -72,8 +72,8 @@ class PSBTTest(BitcoinTestFramework): assert_equal(online_node.gettxout(txid,0)["confirmations"], 1) # Reconnect - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[0], 2) def run_test(self): # Create and fund a raw tx for sending 10 BTC @@ -382,6 +382,16 @@ class PSBTTest(BitcoinTestFramework): joined_decoded = self.nodes[0].decodepsbt(joined) assert len(joined_decoded['inputs']) == 4 and len(joined_decoded['outputs']) == 2 and "final_scriptwitness" not in joined_decoded['inputs'][3] and "final_scriptSig" not in joined_decoded['inputs'][3] + # Check that joining shuffles the inputs and outputs + # 10 attempts should be enough to get a shuffled join + shuffled = False + for i in range(0, 10): + shuffled_joined = self.nodes[0].joinpsbts([psbt, psbt2]) + shuffled |= joined != shuffled_joined + if shuffled: + break + assert shuffled + # Newly created PSBT needs UTXOs and updating addr = self.nodes[1].getnewaddress("", "p2sh-segwit") txid = self.nodes[0].sendtoaddress(addr, 7) diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 4338675270..eb98334988 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -17,7 +17,13 @@ from decimal import Decimal from io import BytesIO from test_framework.messages import CTransaction, ToHex from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error, connect_nodes_bi, hex_str_to_bytes +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, + connect_nodes, + hex_str_to_bytes, +) + class multidict(dict): """Dictionary that allows duplicate keys. @@ -53,7 +59,7 @@ class RawTransactionsTest(BitcoinTestFramework): def setup_network(self): super().setup_network() - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 2) def run_test(self): self.log.info('prepare some coins for multiple *rawtransaction commands') @@ -432,17 +438,18 @@ class RawTransactionsTest(BitcoinTestFramework): self.log.info('sendrawtransaction/testmempoolaccept with maxfeerate') + # Test a transaction with small fee txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) rawTx = self.nodes[0].getrawtransaction(txId, True) vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000')) self.sync_all() inputs = [{ "txid" : txId, "vout" : vout['n'] }] - outputs = { self.nodes[0].getnewaddress() : Decimal("0.99999000") } # 1000 sat fee + outputs = { self.nodes[0].getnewaddress() : Decimal("0.999990000") } # 10000 sat fee rawTx = self.nodes[2].createrawtransaction(inputs, outputs) rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx) assert_equal(rawTxSigned['complete'], True) - # 1000 sat fee, ~100 b transaction, fee rate should land around 10 sat/b = 0.00010000 BTC/kB + # 10000 sat fee, ~100 b transaction, fee rate should land around 100 sat/b = 0.00100000 BTC/kB # Thus, testmempoolaccept should reject testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']], 0.00001000)[0] assert_equal(testres['allowed'], False) @@ -450,9 +457,32 @@ class RawTransactionsTest(BitcoinTestFramework): # and sendrawtransaction should throw assert_raises_rpc_error(-26, "absurdly-high-fee", self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000) # And below calls should both succeed - testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.00070000')[0] + testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']])[0] + assert_equal(testres['allowed'], True) + self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex']) + + # Test a transaction with large fee + txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) + rawTx = self.nodes[0].getrawtransaction(txId, True) + vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000')) + + self.sync_all() + inputs = [{ "txid" : txId, "vout" : vout['n'] }] + outputs = { self.nodes[0].getnewaddress() : Decimal("0.98000000") } # 2000000 sat fee + rawTx = self.nodes[2].createrawtransaction(inputs, outputs) + rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx) + assert_equal(rawTxSigned['complete'], True) + # 2000000 sat fee, ~100 b transaction, fee rate should land around 20000 sat/b = 0.20000000 BTC/kB + # Thus, testmempoolaccept should reject + testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']])[0] + assert_equal(testres['allowed'], False) + assert_equal(testres['reject-reason'], '256: absurdly-high-fee') + # and sendrawtransaction should throw + assert_raises_rpc_error(-26, "absurdly-high-fee", self.nodes[2].sendrawtransaction, rawTxSigned['hex']) + # And below calls should both succeed + testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.20000000')[0] assert_equal(testres['allowed'], True) - self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.00070000') + self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.20000000') if __name__ == '__main__': diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 9aff08fdc7..780aa5fe03 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -25,7 +25,7 @@ from .util import ( PortSeed, assert_equal, check_json_precision, - connect_nodes_bi, + connect_nodes, disconnect_nodes, get_datadir_path, initialize_datadir, @@ -281,8 +281,18 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # Connect the nodes as a "chain". This allows us # to split the network between nodes 1 and 2 to get # two halves that can work on competing chains. + # + # Topology looks like this: + # node0 <-- node1 <-- node2 <-- node3 + # + # If all nodes are in IBD (clean chain from genesis), node0 is assumed to be the source of blocks (miner). To + # ensure block propagation, all nodes will establish outgoing connections toward node0. + # See fPreferredDownload in net_processing. + # + # If further outbound connections are needed, they can be added at the beginning of the test with e.g. + # connect_nodes(self.nodes[1], 2) for i in range(self.num_nodes - 1): - connect_nodes_bi(self.nodes, i, i + 1) + connect_nodes(self.nodes[i + 1], i) self.sync_all() def setup_nodes(self): @@ -423,7 +433,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): """ Join the (previously split) network halves together. """ - connect_nodes_bi(self.nodes, 1, 2) + connect_nodes(self.nodes[1], 2) self.sync_all() def sync_blocks(self, nodes=None, **kwargs): diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index f9f5fe553e..1c254b6755 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -25,6 +25,13 @@ logger = logging.getLogger("TestFramework.utils") # Assert functions ################## +def assert_approx(v, vexp, vspan=0.00001): + """Assert that `v` is within `vspan` of `vexp`""" + if v < vexp - vspan: + raise AssertionError("%s < [%s..%s]" % (str(v), str(vexp - vspan), str(vexp + vspan))) + if v > vexp + vspan: + raise AssertionError("%s > [%s..%s]" % (str(v), str(vexp - vspan), str(vexp + vspan))) + def assert_fee_amount(fee, tx_size, fee_per_kB): """Assert the fee was in range""" target_fee = round(tx_size * fee_per_kB / 1000, 8) @@ -377,10 +384,6 @@ def connect_nodes(from_connection, node_num): # with transaction relaying wait_until(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo())) -def connect_nodes_bi(nodes, a, b): - connect_nodes(nodes[a], b) - connect_nodes(nodes[b], a) - def sync_blocks(rpc_connections, *, wait=1, timeout=60): """ Wait until everybody has the same tip. diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 28a65f7823..355cd7af75 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -19,6 +19,7 @@ class ToolWalletTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True + self.rpc_timeout = 120 def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_address_types.py b/test/functional/wallet_address_types.py index 4e4ed8f26b..c41b31e2e1 100755 --- a/test/functional/wallet_address_types.py +++ b/test/functional/wallet_address_types.py @@ -62,9 +62,12 @@ from test_framework.util import ( assert_equal, assert_greater_than, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, +) +from test_framework.segwit_addr import ( + encode, + decode, ) - class AddressTypeTest(BitcoinTestFramework): def set_test_params(self): @@ -87,7 +90,7 @@ class AddressTypeTest(BitcoinTestFramework): # Fully mesh-connect nodes for faster mempool sync for i, j in itertools.product(range(self.num_nodes), repeat=2): if i > j: - connect_nodes_bi(self.nodes, i, j) + connect_nodes(self.nodes[i], j) self.sync_all() def get_balances(self, confirmed=True): @@ -97,6 +100,13 @@ class AddressTypeTest(BitcoinTestFramework): else: return [self.nodes[i].getunconfirmedbalance() for i in range(4)] + # Quick test of python bech32 implementation + def test_python_bech32(self, addr): + hrp = addr[:4] + assert_equal(hrp, "bcrt") + (witver, witprog) = decode(hrp, addr) + assert_equal(encode(hrp, witver, witprog), addr) + def test_address(self, node, address, multisig, typ): """Run sanity checks on an address.""" info = self.nodes[node].getaddressinfo(address) @@ -121,6 +131,7 @@ class AddressTypeTest(BitcoinTestFramework): assert_equal(info['witness_version'], 0) assert_equal(len(info['witness_program']), 40) assert 'pubkey' in info + self.test_python_bech32(info["address"]) elif typ == 'legacy': # P2SH-multisig assert info['isscript'] @@ -146,6 +157,7 @@ class AddressTypeTest(BitcoinTestFramework): assert_equal(info['witness_version'], 0) assert_equal(len(info['witness_program']), 64) assert 'pubkeys' in info + self.test_python_bech32(info["address"]) else: # Unknown type assert False diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index 58ad835d39..3c8064ea2d 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -6,18 +6,12 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( + assert_approx, assert_equal, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, ) -# TODO: Copied from wallet_groups.py -- should perhaps move into util.py -def assert_approx(v, vexp, vspan=0.00001): - if v < vexp - vspan: - raise AssertionError("%s < [%s..%s]" % (str(v), str(vexp - vspan), str(vexp + vspan))) - if v > vexp + vspan: - raise AssertionError("%s > [%s..%s]" % (str(v), str(vexp - vspan), str(vexp + vspan))) - def reset_balance(node, discardaddr): '''Throw away all owned coins by the node so it gets a balance of 0.''' balance = node.getbalance(avoid_reuse=False) @@ -103,7 +97,7 @@ class AvoidReuseTest(BitcoinTestFramework): # Stop and restart node 1 self.stop_node(1) self.start_node(1) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) # Flags should still be node1.avoid_reuse=false, node2.avoid_reuse=true assert_equal(self.nodes[0].getwalletinfo()["avoid_reuse"], False) diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index 55c517e92f..93178c5ab2 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -49,6 +49,7 @@ class WalletBackupTest(BitcoinTestFramework): self.setup_clean_chain = True # nodes 1, 2,3 are spenders, let's give them a keypool=100 self.extra_args = [["-keypool=100"], ["-keypool=100"], ["-keypool=100"], []] + self.rpc_timeout = 120 def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 137c77a51e..c50dcd987a 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -11,7 +11,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, sync_blocks, ) @@ -211,7 +211,7 @@ class WalletTest(BitcoinTestFramework): # Now confirm tx_orig self.restart_node(1, ['-persistmempool=0']) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) sync_blocks(self.nodes) self.nodes[1].sendrawtransaction(tx_orig) self.nodes[1].generatetoaddress(1, ADDRESS_WATCHONLY) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 74350649c7..5b1c672a48 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -12,7 +12,7 @@ from test_framework.util import ( assert_equal, assert_fee_amount, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, wait_until, ) @@ -32,9 +32,9 @@ class WalletTest(BitcoinTestFramework): self.setup_nodes() # Only need nodes 0-2 running at start of test self.stop_node(3) - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 1, 2) - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) self.sync_all(self.nodes[0:3]) def check_fee_amount(self, curr_balance, balance_with_fee, fee_per_byte, tx_size): @@ -218,7 +218,7 @@ class WalletTest(BitcoinTestFramework): node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) self.start_node(3) - connect_nodes_bi(self.nodes, 0, 3) + connect_nodes(self.nodes[0], 3) self.sync_all() # check if we can list zero value tx as available coins @@ -253,9 +253,9 @@ class WalletTest(BitcoinTestFramework): self.start_node(0, ["-walletbroadcast=0"]) self.start_node(1, ["-walletbroadcast=0"]) self.start_node(2, ["-walletbroadcast=0"]) - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 1, 2) - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) self.sync_all(self.nodes[0:3]) txid_not_broadcast = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 2) @@ -280,9 +280,9 @@ class WalletTest(BitcoinTestFramework): self.start_node(0) self.start_node(1) self.start_node(2) - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 1, 2) - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) self.sync_blocks(self.nodes[0:3]) self.nodes[0].generate(1) @@ -433,7 +433,7 @@ class WalletTest(BitcoinTestFramework): # Split into two chains rawtx = self.nodes[0].createrawtransaction([{"txid": singletxid, "vout": 0}], {chain_addrs[0]: node0_balance / 2 - Decimal('0.01'), chain_addrs[1]: node0_balance / 2 - Decimal('0.01')}) signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx) - singletxid = self.nodes[0].sendrawtransaction(signedtx["hex"]) + singletxid = self.nodes[0].sendrawtransaction(signedtx["hex"], 0) self.nodes[0].generate(1) # Make a long chain of unconfirmed payments without hitting mempool limit @@ -499,10 +499,35 @@ class WalletTest(BitcoinTestFramework): self.nodes[0].setlabel(change, 'foobar') assert_equal(self.nodes[0].getaddressinfo(change)['ischange'], False) - # Test "decoded" field value in gettransaction response - self.log.info("Testing gettransaction decoding...") - tx = self.nodes[0].gettransaction(txid=txid, decode=True) - assert_equal(tx["decoded"], self.nodes[0].decoderawtransaction(tx["hex"])) + # Test gettransaction response with different arguments. + self.log.info("Testing gettransaction response with different arguments...") + self.nodes[0].setlabel(change, 'baz') + baz = self.nodes[0].listtransactions(label="baz", count=1)[0] + expected_receive_vout = {"label": "baz", + "address": baz["address"], + "amount": baz["amount"], + "category": baz["category"], + "vout": baz["vout"]} + expected_fields = frozenset({'amount', 'bip125-replaceable', 'confirmations', 'details', 'fee', + 'hex', 'time', 'timereceived', 'trusted', 'txid', 'walletconflicts'}) + verbose_field = "decoded" + expected_verbose_fields = expected_fields | {verbose_field} + + self.log.debug("Testing gettransaction response without verbose") + tx = self.nodes[0].gettransaction(txid=txid) + assert_equal(set([*tx]), expected_fields) + assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout) + + self.log.debug("Testing gettransaction response with verbose set to False") + tx = self.nodes[0].gettransaction(txid=txid, verbose=False) + assert_equal(set([*tx]), expected_fields) + assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout) + + self.log.debug("Testing gettransaction response with verbose set to True") + tx = self.nodes[0].gettransaction(txid=txid, verbose=True) + assert_equal(set([*tx]), expected_verbose_fields) + assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout) + assert_equal(tx[verbose_field], self.nodes[0].decoderawtransaction(tx["hex"])) if __name__ == '__main__': diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index bfc01e3f5e..a7c79ec916 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -23,7 +23,7 @@ from test_framework.util import ( assert_equal, assert_greater_than, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, hex_str_to_bytes, ) @@ -48,7 +48,7 @@ class BumpFeeTest(BitcoinTestFramework): self.nodes[1].encryptwallet(WALLET_PASSPHRASE) self.nodes[1].walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) self.sync_all() peer_node, rbf_node = self.nodes diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py index 5452433acf..d1178611bd 100755 --- a/test/functional/wallet_groups.py +++ b/test/functional/wallet_groups.py @@ -7,15 +7,10 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.messages import CTransaction, FromHex, ToHex from test_framework.util import ( + assert_approx, assert_equal, ) -def assert_approx(v, vexp, vspan=0.00001): - if v < vexp - vspan: - raise AssertionError("%s < [%s..%s]" % (str(v), str(vexp - vspan), str(vexp + vspan))) - if v > vexp + vspan: - raise AssertionError("%s > [%s..%s]" % (str(v), str(vexp - vspan), str(vexp + vspan))) - class WalletGroupTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index 97172d8b82..fa5d5a8878 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -10,7 +10,7 @@ import shutil from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - connect_nodes_bi, + connect_nodes, assert_raises_rpc_error ) @@ -82,7 +82,7 @@ class WalletHDTest(BitcoinTestFramework): assert_equal(hd_info_2["hdkeypath"], "m/0'/0'/"+str(i)+"'") assert_equal(hd_info_2["hdseedid"], masterkeyid) assert_equal(hd_add, hd_add_2) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) self.sync_all() # Needs rescan @@ -96,7 +96,7 @@ class WalletHDTest(BitcoinTestFramework): shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "chainstate")) shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat")) self.start_node(1, extra_args=self.extra_args[1]) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) self.sync_all() # Wallet automatically scans blocks older than key on startup assert_equal(self.nodes[1].getbalance(), NUM_HD_ADDS + 1) diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 4e20892596..79062a4a29 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -150,7 +150,7 @@ class ImportRescanTest(BitcoinTestFramework): self.skip_if_no_wallet() def setup_network(self): - self.extra_args = [[]] * self.num_nodes + self.extra_args = [[] for _ in range(self.num_nodes)] for i, import_node in enumerate(IMPORT_NODES, 2): if import_node.prune: self.extra_args[i] += ["-prune=1"] diff --git a/test/functional/wallet_keypool_topup.py b/test/functional/wallet_keypool_topup.py index 0014555ade..2e70a9e0a5 100755 --- a/test/functional/wallet_keypool_topup.py +++ b/test/functional/wallet_keypool_topup.py @@ -16,7 +16,7 @@ import shutil from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - connect_nodes_bi, + connect_nodes, ) @@ -38,9 +38,9 @@ class KeypoolRestoreTest(BitcoinTestFramework): self.stop_node(1) shutil.copyfile(wallet_path, wallet_backup_path) self.start_node(1, self.extra_args[1]) - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 0, 2) - connect_nodes_bi(self.nodes, 0, 3) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[0], 2) + connect_nodes(self.nodes[0], 3) for i, output_type in enumerate(["legacy", "p2sh-segwit", "bech32"]): @@ -72,7 +72,7 @@ class KeypoolRestoreTest(BitcoinTestFramework): self.stop_node(idx) shutil.copyfile(wallet_backup_path, wallet_path) self.start_node(idx, self.extra_args[idx]) - connect_nodes_bi(self.nodes, 0, idx) + connect_nodes(self.nodes[0], idx) self.sync_all() self.log.info("Verify keypool is restored and balance is correct") diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index 021a29d4ac..4aeb393255 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -5,9 +5,15 @@ """Test the listsincelast RPC.""" from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_array_result, assert_raises_rpc_error +from test_framework.util import ( + assert_array_result, + assert_equal, + assert_raises_rpc_error, + connect_nodes, +) -class ListSinceBlockTest (BitcoinTestFramework): + +class ListSinceBlockTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = True @@ -16,6 +22,9 @@ class ListSinceBlockTest (BitcoinTestFramework): self.skip_if_no_wallet() def run_test(self): + # All nodes are in IBD from genesis, so they'll need the miner (node2) to be an outbound connection, or have + # only one connection. (See fPreferredDownload in net_processing) + connect_nodes(self.nodes[1], 2) self.nodes[2].generate(101) self.sync_all() diff --git a/test/lint/lint-python-dead-code-whitelist b/test/lint/lint-python-dead-code-whitelist index 2522c8fa1c..33bb7b44fa 100644 --- a/test/lint/lint-python-dead-code-whitelist +++ b/test/lint/lint-python-dead-code-whitelist @@ -6,6 +6,7 @@ _.converter # unused attribute (test/functional/test_framework/test_framework.p _.daemon # unused attribute (test/functional/test_framework/socks5.py) data_received # unused function (test/functional/test_framework/mininode.py) DuplicateInput # unused class (test/functional/data/invalid_txs.py) +DisabledOpcodeTemplates # unused class (test/functional/data/invalid_txs.py) _.filename # unused attribute (contrib/macdeploy/custom_dsstore.py) InvalidOPIFConstruction # unused class (test/functional/data/invalid_txs.py) _.is_compressed # unused property (test/functional/test_framework/key.py) diff --git a/test/lint/lint-python-dead-code.sh b/test/lint/lint-python-dead-code.sh index 77bf5990a7..af37d393e8 100755 --- a/test/lint/lint-python-dead-code.sh +++ b/test/lint/lint-python-dead-code.sh @@ -13,7 +13,11 @@ if ! command -v vulture > /dev/null; then exit 0 fi -vulture \ +VULTURE_SUPPRESSIONS=$(dirname "${BASH_SOURCE[0]}")/lint-python-dead-code-whitelist +if ! vulture \ --min-confidence 60 \ $(git rev-parse --show-toplevel) \ - $(dirname "${BASH_SOURCE[0]}")/lint-python-dead-code-whitelist + "${VULTURE_SUPPRESSIONS}"; then + echo "False positives? Suppressions can be added to ${VULTURE_SUPPRESSIONS}" + exit 1 +fi |