diff options
151 files changed, 1930 insertions, 1197 deletions
diff --git a/build_msvc/README.md b/build_msvc/README.md index b9bebd369c..7520700a34 100644 --- a/build_msvc/README.md +++ b/build_msvc/README.md @@ -7,7 +7,9 @@ Visual Studio 2022 is minimum required to build Bitcoin Core. Solution and project files to build with `msbuild` or Visual Studio can be found in the `build_msvc` directory. -To build Bitcoin Core from the command-line, it is sufficient to only install the Visual Studio Build Tools component. +To build Bitcoin Core from the command-line, it is sufficient to only install the [Visual Studio Build Tools](https://visualstudio.microsoft.com/downloads/) component. + +The "Desktop development with C++" workload must be installed as well. Building with Visual Studio is an alternative to the Linux based [cross-compiler build](../doc/build-windows.md). diff --git a/ci/lint/06_script.sh b/ci/lint/06_script.sh index 4506848740..826888b6cc 100755 --- a/ci/lint/06_script.sh +++ b/ci/lint/06_script.sh @@ -9,7 +9,12 @@ export LC_ALL=C GIT_HEAD=$(git rev-parse HEAD) if [ -n "$CIRRUS_PR" ]; then COMMIT_RANGE="${CIRRUS_BASE_SHA}..$GIT_HEAD" + echo + git log --no-merges --oneline "$COMMIT_RANGE" + echo test/lint/commit-script-check.sh "$COMMIT_RANGE" +else + COMMIT_RANGE="SKIP_EMPTY_NOT_A_PR" fi export COMMIT_RANGE @@ -36,8 +41,3 @@ if [ "$CIRRUS_REPO_FULL_NAME" = "bitcoin/bitcoin" ] && [ "$CIRRUS_PR" = "" ] ; t ${CI_RETRY_EXE} gpg --keyserver hkps://keys.openpgp.org --recv-keys "${KEYS[@]}" && ./contrib/verify-commits/verify-commits.py; fi - -if [ -n "$COMMIT_RANGE" ]; then - echo - git log --no-merges --oneline "$COMMIT_RANGE" -fi diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index 4f1792a9f0..d8ebbbcdc9 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -6,14 +6,21 @@ export LC_ALL=C.UTF-8 -# We install an up-to-date 'bpfcc-tools' package from an untrusted PPA. -# This can be dropped with the next Ubuntu or Debian release that includes up-to-date packages. -# See the if-then in ci/test/04_install.sh too. -export ADD_UNTRUSTED_BPFCC_PPA=true +# Only install BCC tracing packages in Cirrus CI. +if [[ "${CIRRUS_CI}" == "true" ]]; then + # We install an up-to-date 'bpfcc-tools' package from an untrusted PPA. + # This can be dropped with the next Ubuntu or Debian release that includes up-to-date packages. + # See the if-then in ci/test/04_install.sh too. + export ADD_UNTRUSTED_BPFCC_PPA=true + export BPFCC_PACKAGE="bpfcc-tools" +else + export ADD_UNTRUSTED_BPFCC_PPA=false + export BPFCC_PACKAGE="" +fi export CONTAINER_NAME=ci_native_asan -export PACKAGES="systemtap-sdt-dev bpfcc-tools clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libqrencode-dev libsqlite3-dev" -export DOCKER_NAME_TAG=ubuntu:22.04 # May not run in docker unless --enable-usdt is dropped +export PACKAGES="systemtap-sdt-dev clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libqrencode-dev libsqlite3-dev ${BPFCC_PACKAGE}" +export DOCKER_NAME_TAG=ubuntu:22.04 export NO_DEPENDS=1 export GOAL="install" export BITCOIN_CONFIG="--enable-usdt --enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' --with-sanitizers=address,integer,undefined CC=clang CXX=clang++" diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index c25abb99ee..4915797ca4 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -130,8 +130,8 @@ fi CI_EXEC mkdir -p "${BASE_SCRATCH_DIR}/sanitizer-output/" if [[ ${USE_MEMORY_SANITIZER} == "true" ]]; then - CI_EXEC "update-alternatives --install /usr/bin/clang++ clang++ \$(which clang++-12) 100" - CI_EXEC "update-alternatives --install /usr/bin/clang clang \$(which clang-12) 100" + CI_EXEC_ROOT "update-alternatives --install /usr/bin/clang++ clang++ \$(which clang++-12) 100" + CI_EXEC_ROOT "update-alternatives --install /usr/bin/clang clang \$(which clang-12) 100" CI_EXEC "mkdir -p ${BASE_SCRATCH_DIR}/msan/build/" CI_EXEC "git clone --depth=1 https://github.com/llvm/llvm-project -b llvmorg-12.0.0 ${BASE_SCRATCH_DIR}/msan/llvm-project" CI_EXEC "cd ${BASE_SCRATCH_DIR}/msan/build/ && cmake -DLLVM_ENABLE_PROJECTS='libcxx;libcxxabi' -DCMAKE_BUILD_TYPE=Release -DLLVM_USE_SANITIZER=MemoryWithOrigins -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_TARGETS_TO_BUILD=X86 ../llvm-project/llvm/" diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index 14a5c08bb4..46312b50eb 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -56,6 +56,7 @@ if [ "${RUN_TIDY}" = "true" ]; then " src/rpc/fees.cpp"\ " src/rpc/signmessage.cpp"\ " src/test/fuzz/txorphan.cpp"\ + " src/test/fuzz/util/"\ " src/util/bip32.cpp"\ " src/util/bytevectorhash.cpp"\ " src/util/check.cpp"\ diff --git a/configure.ac b/configure.ac index 685c840f2b..541f31e3f0 100644 --- a/configure.ac +++ b/configure.ac @@ -1984,10 +1984,6 @@ AC_SUBST(HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR) AC_CONFIG_FILES([Makefile src/Makefile doc/man/Makefile share/setup.nsi share/qt/Info.plist test/config.ini]) AC_CONFIG_FILES([contrib/devtools/split-debug.sh],[chmod +x contrib/devtools/split-debug.sh]) AM_COND_IF([HAVE_DOXYGEN], [AC_CONFIG_FILES([doc/Doxyfile])]) -AC_CONFIG_LINKS([contrib/devtools/security-check.py:contrib/devtools/security-check.py]) -AC_CONFIG_LINKS([contrib/devtools/symbol-check.py:contrib/devtools/symbol-check.py]) -AC_CONFIG_LINKS([contrib/devtools/test-security-check.py:contrib/devtools/test-security-check.py]) -AC_CONFIG_LINKS([contrib/devtools/test-symbol-check.py:contrib/devtools/test-symbol-check.py]) AC_CONFIG_LINKS([contrib/devtools/iwyu/bitcoin.core.imp:contrib/devtools/iwyu/bitcoin.core.imp]) AC_CONFIG_LINKS([contrib/filter-lcov.py:contrib/filter-lcov.py]) AC_CONFIG_LINKS([contrib/macdeploy/background.tiff:contrib/macdeploy/background.tiff]) diff --git a/contrib/builder-keys/keys.txt b/contrib/builder-keys/keys.txt index 2d96b863ef..b2b5b031f8 100644 --- a/contrib/builder-keys/keys.txt +++ b/contrib/builder-keys/keys.txt @@ -22,7 +22,6 @@ D35176BE9264832E4ACA8986BF0792FBE95DC863 fivepiece (fivepiece) 01CDF4627A3B88AAE4A571C87588242FBE38D3A8 Gavin Andresen (gavinandresen) 6B002C6EA3F91B1B0DF0C9BC8F617F1200A6D25C Gloria Zhao (glozow) D1DBF2C4B96F2DEBF4C16654410108112E7EA81F Hennadii Stepanov (hebasto) -A2FD494D0021AA9B4FA58F759102B7AE654A4A5A Ilyas Ridhuan (IlyasRidhuan) 2688F5A9A4BE0F295E921E8A25F27A38A47AD566 James O'Beirne (jamesob) D3F22A3A4C366C2DCB66D3722DA9C5A7FA81EA35 Jarol Rodriguez (jarolrod) 7480909378D544EA6B6DCEB7535B12980BB8A4D3 Jeffri H Frontz (jhfrontz) diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index d3d225f3ab..84283e3522 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -39,7 +39,7 @@ def call_security_check(cc, source, executable, options): env_flags += filter(None, os.environ.get(var, '').split(' ')) subprocess.run([*cc,source,'-o',executable] + env_flags + options, check=True) - p = subprocess.run(['./contrib/devtools/security-check.py',executable], stdout=subprocess.PIPE, universal_newlines=True) + p = subprocess.run([os.path.join(os.path.dirname(__file__), 'security-check.py'), executable], stdout=subprocess.PIPE, universal_newlines=True) return (p.returncode, p.stdout.rstrip()) def get_arch(cc, source, executable): diff --git a/contrib/devtools/test-symbol-check.py b/contrib/devtools/test-symbol-check.py index 2881e3efac..85d3a6d07b 100755 --- a/contrib/devtools/test-symbol-check.py +++ b/contrib/devtools/test-symbol-check.py @@ -23,7 +23,7 @@ def call_symbol_check(cc: List[str], source, executable, options): env_flags += filter(None, os.environ.get(var, '').split(' ')) subprocess.run([*cc,source,'-o',executable] + env_flags + options, check=True) - p = subprocess.run(['./contrib/devtools/symbol-check.py',executable], stdout=subprocess.PIPE, universal_newlines=True) + p = subprocess.run([os.path.join(os.path.dirname(__file__), 'symbol-check.py'), executable], stdout=subprocess.PIPE, universal_newlines=True) os.remove(source) os.remove(executable) return (p.returncode, p.stdout.rstrip()) diff --git a/contrib/guix/INSTALL.md b/contrib/guix/INSTALL.md index a9a41ddff6..bbd88e58f3 100644 --- a/contrib/guix/INSTALL.md +++ b/contrib/guix/INSTALL.md @@ -167,6 +167,10 @@ For reference, the graphic below outlines Guix v1.3.0's dependency graph:  +#### Consider /tmp on tmpfs + +If you use an NVME (SSD) drive, you may encounter [cryptic build errors](#coreutils-fail-teststail-2inotify-dir-recreate). Mounting a [tmpfs at /tmp](https://ubuntu.com/blog/data-driven-analysis-tmp-on-tmpfs) should prevent this and may improve performance as a bonus. + #### Guile ##### Choosing a Guile version and sticking to it @@ -334,6 +338,8 @@ packages in Debian at the time of writing. |-----------------------|---------------------| | guile-gcrypt | libgcrypt-dev | | guile-git | libgit2-dev | +| guile-gnutls | (none) | +| guile-json | (none) | | guile-lzlib | liblz-dev | | guile-ssh | libssh-dev | | guile-sqlite3 | libsqlite3-dev | @@ -384,8 +390,9 @@ cd guix ``` You will likely want to build the latest release, however, if the latest release -when you're reading this is still 1.2.0 then you may want to use 95aca29 instead -to avoid a problem in the GnuTLS test suite. +when you're reading this is still 1.3.0 then you may want to use 998eda30 instead +to avoid the issues described in [#25099]( +https://github.com/bitcoin/bitcoin/pull/25099). ``` git branch -a -l 'origin/version-*' # check for the latest release @@ -609,6 +616,8 @@ systemctl enable guix-daemon systemctl start guix-daemon ``` +Remember to set `--no-substitute` in `$libdir/systemd/system/guix-daemon.service` and other customizations if you used them for `guix-daemon-original.service`. + ##### If you installed Guix via the Debian/Ubuntu distribution packages You will need to create a `guix-daemon-latest` service which points to the new @@ -717,6 +726,19 @@ $ bzcat /var/log/guix/drvs/../...-foo-3.6.12.drv.bz2 | less times, it may be `/tmp/...drv-1` or `/tmp/...drv-2`. Always consult the build failure output for the most accurate, up-to-date information. +### openssl-1.1.1l and openssl-1.1.1n + +OpenSSL includes tests that will fail once some certificate has expired. A workaround +is to change your system clock: + +```sh +sudo timedatectl set-ntp no +sudo date --set "28 may 2022 15:00:00" +sudo --login guix build --cores=1 /gnu/store/g9alz81w4q03ncm542487xd001s6akd4-openssl-1.1.1l.drv +sudo --login guix build --cores=1 /gnu/store/mw6ax0gk33gh082anrdrxp2flrbskxv6-openssl-1.1.1n.drv +sudo timedatectl set-ntp yes +``` + ### python(-minimal): [Errno 84] Invalid or incomplete multibyte or wide character This error occurs when your `$TMPDIR` (default: /tmp) exists on a filesystem @@ -774,7 +796,7 @@ The inotify-dir-create test fails on "remote" filesystems such as overlayfs as non-remote. A relatively easy workaround to this is to make sure that a somewhat traditional -filesystem is mounted at `/tmp` (where `guix-daemon` performs its builds). For +filesystem is mounted at `/tmp` (where `guix-daemon` performs its builds), see [/tmp on tmpfs](#consider-tmp-on-tmpfs). For Docker users, this might mean [using a volume][docker/volumes], [binding mounting][docker/bind-mnt] from host, or (for those with enough RAM and swap) [mounting a tmpfs][docker/tmpfs] using the `--tmpfs` flag. @@ -782,7 +804,7 @@ mounting][docker/bind-mnt] from host, or (for those with enough RAM and swap) Please see the following links for more details: - An upstream coreutils bug has been filed: [debbugs#47940](https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47940) -- A Guix bug detailing the underlying problem has been filed: [guix-issues#47935](https://issues.guix.gnu.org/47935) +- A Guix bug detailing the underlying problem has been filed: [guix-issues#47935](https://issues.guix.gnu.org/47935), [guix-issues#49985](https://issues.guix.gnu.org/49985#5) - A commit to skip this test in Guix has been merged into the core-updates branch: [savannah/guix@6ba1058](https://git.savannah.gnu.org/cgit/guix.git/commit/?id=6ba1058df0c4ce5611c2367531ae5c3cdc729ab4) @@ -799,3 +821,39 @@ Please see the following links for more details: [docker/volumes]: https://docs.docker.com/storage/volumes/ [docker/bind-mnt]: https://docs.docker.com/storage/bind-mounts/ [docker/tmpfs]: https://docs.docker.com/storage/tmpfs/ + +# Purging/Uninstalling Guix + +In the extraordinarily rare case where you messed up your Guix installation in +an irreversible way, you may want to completely purge Guix from your system and +start over. + +1. Uninstall Guix itself according to the way you installed it (e.g. `sudo apt + purge guix` for Ubuntu packaging, `sudo make uninstall` for a build from source). +2. Remove all build users and groups + + You may check for relevant users and groups using: + + ``` + getent passwd | grep guix + getent group | grep guix + ``` + + Then, you may remove users and groups using: + + ``` + sudo userdel <user> + sudo groupdel <group> + ``` + +3. Remove all possible Guix-related directories + - `/var/guix/` + - `/var/log/guix/` + - `/gnu/` + - `/etc/guix/` + - `/home/*/.config/guix/` + - `/home/*/.cache/guix/` + - `/home/*/.guix-profile/` + - `/root/.config/guix/` + - `/root/.cache/guix/` + - `/root/.guix-profile/` diff --git a/contrib/guix/README.md b/contrib/guix/README.md index ed6ac8d589..c0feb486ff 100644 --- a/contrib/guix/README.md +++ b/contrib/guix/README.md @@ -430,55 +430,6 @@ used. If you start `guix-daemon` using an init script, you can edit said script to supply this flag. - -# Purging/Uninstalling Guix - -In the extraordinarily rare case where you messed up your Guix installation in -an irreversible way, you may want to completely purge Guix from your system and -start over. - -1. Uninstall Guix itself according to the way you installed it (e.g. `sudo apt - purge guix` for Ubuntu packaging, `sudo make uninstall` for a build from source). -2. Remove all build users and groups - - You may check for relevant users and groups using: - - ``` - getent passwd | grep guix - getent group | grep guix - ``` - - Then, you may remove users and groups using: - - ``` - sudo userdel <user> - sudo groupdel <group> - ``` - -3. Remove all possible Guix-related directories - - `/var/guix/` - - `/var/log/guix/` - - `/gnu/` - - `/etc/guix/` - - `/home/*/.config/guix/` - - `/home/*/.cache/guix/` - - `/home/*/.guix-profile/` - - `/root/.config/guix/` - - `/root/.cache/guix/` - - `/root/.guix-profile/` - [b17e]: https://bootstrappable.org/ [r12e/source-date-epoch]: https://reproducible-builds.org/docs/source-date-epoch/ - -[guix/install.sh]: https://git.savannah.gnu.org/cgit/guix.git/plain/etc/guix-install.sh -[guix/bin-install]: https://www.gnu.org/software/guix/manual/en/html_node/Binary-Installation.html -[guix/env-setup]: https://www.gnu.org/software/guix/manual/en/html_node/Build-Environment-Setup.html -[guix/substitutes]: https://www.gnu.org/software/guix/manual/en/html_node/Substitutes.html -[guix/substitute-server-auth]: https://www.gnu.org/software/guix/manual/en/html_node/Substitute-Server-Authorization.html -[guix/time-machine]: https://guix.gnu.org/manual/en/html_node/Invoking-guix-time_002dmachine.html - -[debian/guix-bullseye]: https://packages.debian.org/bullseye/guix -[ubuntu/guix-hirsute]: https://packages.ubuntu.com/hirsute/guix -[fanquake/guix-docker]: https://github.com/fanquake/core-review/tree/master/guix - [env-vars-list]: #recognized-environment-variables diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index f39f83d443..f26b6795d1 100755 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -69,16 +69,12 @@ unset CPLUS_INCLUDE_PATH unset OBJC_INCLUDE_PATH unset OBJCPLUS_INCLUDE_PATH -export LIBRARY_PATH="${NATIVE_GCC}/lib:${NATIVE_GCC}/lib64:${NATIVE_GCC_STATIC}/lib:${NATIVE_GCC_STATIC}/lib64" +export LIBRARY_PATH="${NATIVE_GCC}/lib:${NATIVE_GCC_STATIC}/lib" export C_INCLUDE_PATH="${NATIVE_GCC}/include" export CPLUS_INCLUDE_PATH="${NATIVE_GCC}/include/c++:${NATIVE_GCC}/include" export OBJC_INCLUDE_PATH="${NATIVE_GCC}/include" export OBJCPLUS_INCLUDE_PATH="${NATIVE_GCC}/include/c++:${NATIVE_GCC}/include" -prepend_to_search_env_var() { - export "${1}=${2}${!1:+:}${!1}" -} - # Set environment variables to point the CROSS toolchain to the right # includes/libs for $HOST case "$HOST" in diff --git a/depends/README.md b/depends/README.md index 66e1ddc4eb..a8831eb0fc 100644 --- a/depends/README.md +++ b/depends/README.md @@ -62,7 +62,7 @@ For more information, see [SDK Extraction](../contrib/macdeploy/README.md#sdk-ex Common linux dependencies: - sudo apt-get install make automake cmake curl g++-multilib libtool binutils-gold bsdmainutils pkg-config python3 patch bison + sudo apt-get install make automake cmake curl g++-multilib libtool binutils bsdmainutils pkg-config python3 patch bison For linux ARM cross compilation: diff --git a/depends/packages/bdb.mk b/depends/packages/bdb.mk index 2370c5b759..262587690c 100644 --- a/depends/packages/bdb.mk +++ b/depends/packages/bdb.mk @@ -15,6 +15,9 @@ $(package)_config_opts_netbsd=--with-pic $(package)_config_opts_openbsd=--with-pic $(package)_config_opts_android=--with-pic $(package)_cflags+=-Wno-error=implicit-function-declaration -Wno-error=format-security +$(package)_cppflags_freebsd=-D_XOPEN_SOURCE=600 +$(package)_cppflags_netbsd=-D_XOPEN_SOURCE=600 +$(package)_cppflags_openbsd=-D_XOPEN_SOURCE=600 $(package)_cppflags_mingw32=-DUNICODE -D_UNICODE endef diff --git a/doc/JSON-RPC-interface.md b/doc/JSON-RPC-interface.md index 12807bfb86..ab5db58cdd 100644 --- a/doc/JSON-RPC-interface.md +++ b/doc/JSON-RPC-interface.md @@ -5,6 +5,28 @@ The headless daemon `bitcoind` has the JSON-RPC API enabled by default, the GUI option. In the GUI it is possible to execute RPC methods in the Debug Console Dialog. +## Parameter passing + +The JSON-RPC server supports both _by-position_ and _by-name_ [parameter +structures](https://www.jsonrpc.org/specification#parameter_structures) +described in the JSON-RPC specification. For extra convenience, to avoid the +need to name every parameter value, all RPC methods accept a named parameter +called `args`, which can be set to an array of initial positional values that +are combined with named values. + +Examples: + +```sh +# "params": ["mywallet", false, false, "", false, false, true] +bitcoin-cli createwallet mywallet false false "" false false true + +# "params": {"wallet_name": "mywallet", "load_on_startup": true} +bitcoin-cli -named createwallet wallet_name=mywallet load_on_startup=true + +# "params": {"args": ["mywallet"], "load_on_startup": true} +bitcoin-cli -named createwallet mywallet load_on_startup=true +``` + ## Versioning The RPC interface might change from one major version of Bitcoin Core to the diff --git a/doc/release-notes-19762.md b/doc/release-notes-19762.md new file mode 100644 index 0000000000..4dc45fb2c8 --- /dev/null +++ b/doc/release-notes-19762.md @@ -0,0 +1,19 @@ +JSON-RPC +--- + +All JSON-RPC methods accept a new [named +parameter](JSON-RPC-interface.md#parameter-passing) called `args` that can +contain positional parameter values. This is a convenience to allow some +parameter values to be passed by name without having to name every value. The +python test framework and `bitcoin-cli` tool both take advantage of this, so +for example: + +```sh +bitcoin-cli -named createwallet wallet_name=mywallet load_on_startup=1 +``` + +Can now be shortened to: + +```sh +bitcoin-cli -named createwallet mywallet load_on_startup=1 +``` diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 9a9424e84c..74c30f1caf 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -199,8 +199,6 @@ FUZZ_WALLET_SRC += \ endif # USE_SQLITE BITCOIN_TEST_SUITE += \ - wallet/test/util.cpp \ - wallet/test/util.h \ wallet/test/wallet_test_fixture.cpp \ wallet/test/wallet_test_fixture.h \ wallet/test/init_test_fixture.cpp \ diff --git a/src/Makefile.test_util.include b/src/Makefile.test_util.include index d142572b27..a4e8b3f842 100644 --- a/src/Makefile.test_util.include +++ b/src/Makefile.test_util.include @@ -18,8 +18,11 @@ TEST_UTIL_H = \ test/util/str.h \ test/util/transaction_utils.h \ test/util/txmempool.h \ - test/util/validation.h \ - test/util/wallet.h + test/util/validation.h + +if ENABLE_WALLET +TEST_UTIL_H += wallet/test/util.h +endif # ENABLE_WALLET libtest_util_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS) libtest_util_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) @@ -33,6 +36,10 @@ libtest_util_a_SOURCES = \ test/util/str.cpp \ test/util/transaction_utils.cpp \ test/util/txmempool.cpp \ - test/util/validation.cpp \ - test/util/wallet.cpp \ - $(TEST_UTIL_H) + test/util/validation.cpp + +if ENABLE_WALLET +libtest_util_a_SOURCES += wallet/test/util.cpp +endif # ENABLE_WALLET + +libtest_util_a_SOURCES += $(TEST_UTIL_H) diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 09be011fda..69258377d5 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -8,7 +8,6 @@ #include <test/util/mining.h> #include <test/util/script.h> #include <test/util/setup_common.h> -#include <test/util/wallet.h> #include <txmempool.h> #include <validation.h> diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index 22d99c0e29..5a52774a8e 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -7,7 +7,7 @@ #include <node/context.h> #include <test/util/mining.h> #include <test/util/setup_common.h> -#include <test/util/wallet.h> +#include <wallet/test/util.h> #include <validationinterface.h> #include <wallet/receive.h> #include <wallet/wallet.h> @@ -20,6 +20,8 @@ using wallet::DBErrors; using wallet::GetBalance; using wallet::WALLET_FLAG_DESCRIPTORS; +const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj"; + static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const bool add_mine) { const auto test_setup = MakeNoLogFileContext<const TestingSetup>(); diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp index 8f5c50872b..3cbf2c9008 100644 --- a/src/bench/wallet_create_tx.cpp +++ b/src/bench/wallet_create_tx.cpp @@ -9,9 +9,9 @@ #include <kernel/chain.h> #include <node/context.h> #include <test/util/setup_common.h> -#include <test/util/wallet.h> #include <validation.h> #include <wallet/spend.h> +#include <wallet/test/util.h> #include <wallet/wallet.h> using wallet::CWallet; diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp index 8bfaf3044b..2f7dc53b0c 100644 --- a/src/bench/wallet_loading.cpp +++ b/src/bench/wallet_loading.cpp @@ -7,7 +7,7 @@ #include <node/context.h> #include <test/util/mining.h> #include <test/util/setup_common.h> -#include <test/util/wallet.h> +#include <wallet/test/util.h> #include <util/translation.h> #include <validationinterface.h> #include <wallet/context.h> @@ -52,30 +52,6 @@ static void AddTx(CWallet& wallet) wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}); } -static std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options) -{ - auto new_database = CreateMockWalletDatabase(options); - - // Get a cursor to the original database - auto batch = database.MakeBatch(); - batch->StartCursor(); - - // Get a batch for the new database - auto new_batch = new_database->MakeBatch(); - - // Read all records from the original database and write them to the new one - while (true) { - CDataStream key(SER_DISK, CLIENT_VERSION); - CDataStream value(SER_DISK, CLIENT_VERSION); - bool complete; - batch->ReadAtCursor(key, value, complete); - if (complete) break; - new_batch->Write(key, value); - } - - return new_database; -} - static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet) { const auto test_setup = MakeNoLogFileContext<TestingSetup>(); diff --git a/src/init.cpp b/src/init.cpp index 6f44ed6c3d..2fdc2717c9 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -67,7 +67,6 @@ #include <torcontrol.h> #include <txdb.h> #include <txmempool.h> -#include <txorphanage.h> #include <util/asmap.h> #include <util/check.h> #include <util/moneystr.h> @@ -478,6 +477,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-onlynet=<net>", "Make automatic outbound connections only to network <net> (" + Join(GetNetworkNames(), ", ") + "). Inbound and manual connections are not affected by this option. It can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); // TODO: remove the sentence "Nodes not using ... incoming connections." once the changes from // https://github.com/bitcoin/bitcoin/pull/23542 have become widespread. argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port>. Nodes not using the default ports (default: %u, testnet: %u, signet: %u, regtest: %u) are unlikely to get incoming connections. Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); @@ -486,7 +486,6 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-seednode=<ip>", "Connect to a node to retrieve peer addresses, and disconnect. This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-networkactive", "Enable all P2P network activity (default: 1). Can be changed by the setnetworkactive RPC command", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-timeout=<n>", strprintf("Specify socket connection timeout in milliseconds. If an initial attempt to connect is unsuccessful after this amount of time, drop it (minimum: 1, default: %d)", DEFAULT_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-peertimeout=<n>", strprintf("Specify a p2p connection timeout delay in seconds. After connecting to a peer, wait this amount of time before considering disconnection based on inactivity (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-torpassword=<pass>", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::CONNECTION); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6d5eb3a449..b09fd4b0c2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -365,9 +365,6 @@ struct Peer { /** Total number of addresses that were processed (excludes rate-limited ones). */ std::atomic<uint64_t> m_addr_processed{0}; - /** Set of txids to reconsider once their parent transactions have been accepted **/ - std::set<uint256> m_orphan_work_set GUARDED_BY(g_cs_orphans); - /** Whether we've sent this peer a getheaders in response to an inv prior to initial-headers-sync completing */ bool m_inv_triggered_getheaders_before_sync GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; @@ -584,8 +581,17 @@ private: */ bool MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer); - void ProcessOrphanTx(std::set<uint256>& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + /** + * Reconsider orphan transactions after a parent has been accepted to the mempool. + * + * @peer[in] peer The peer whose orphan transactions we will reconsider. Generally only one + * orphan will be reconsidered on each call of this function. This set + * may be added to if accepting an orphan causes its children to be + * reconsidered. + * @return True if there are still orphans in this peer's work set. + */ + bool ProcessOrphanTx(Peer& peer) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main); /** Process a single headers message from a peer. * * @param[in] pfrom CNode of the peer @@ -919,14 +925,14 @@ private: /** Storage for orphan information */ TxOrphanage m_orphanage; - void AddToCompactExtraTransactions(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + void AddToCompactExtraTransactions(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); /** Orphan/conflicted/etc transactions that are kept for compact block reconstruction. * The last -blockreconstructionextratxn/DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN of * these are kept in a ring buffer */ - std::vector<std::pair<uint256, CTransactionRef>> vExtraTxnForCompact GUARDED_BY(g_cs_orphans); + std::vector<std::pair<uint256, CTransactionRef>> vExtraTxnForCompact GUARDED_BY(g_msgproc_mutex); /** Offset into vExtraTxnForCompact to insert the next tx */ - size_t vExtraTxnForCompactIt GUARDED_BY(g_cs_orphans) = 0; + size_t vExtraTxnForCompactIt GUARDED_BY(g_msgproc_mutex) = 0; /** Check whether the last unknown block a peer advertised is not yet known. */ void ProcessBlockAvailability(NodeId nodeid) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -1490,7 +1496,7 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) for (const QueuedBlock& entry : state->vBlocksInFlight) { mapBlocksInFlight.erase(entry.pindex->GetBlockHash()); } - WITH_LOCK(g_cs_orphans, m_orphanage.EraseForPeer(nodeid)); + m_orphanage.EraseForPeer(nodeid); m_txrequest.DisconnectedPeer(nodeid); if (m_txreconciliation) m_txreconciliation->ForgetPeer(nodeid); m_num_preferred_download_peers -= state->fPreferredDownload; @@ -2887,33 +2893,24 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, return; } -/** - * Reconsider orphan transactions after a parent has been accepted to the mempool. - * - * @param[in,out] orphan_work_set The set of orphan transactions to reconsider. Generally only one - * orphan will be reconsidered on each call of this function. This set - * may be added to if accepting an orphan causes its children to be - * reconsidered. - */ -void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set) +bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) { + AssertLockHeld(g_msgproc_mutex); AssertLockHeld(cs_main); - AssertLockHeld(g_cs_orphans); - - while (!orphan_work_set.empty()) { - const uint256 orphanHash = *orphan_work_set.begin(); - orphan_work_set.erase(orphan_work_set.begin()); - const auto [porphanTx, from_peer] = m_orphanage.GetTx(orphanHash); - if (porphanTx == nullptr) continue; + CTransactionRef porphanTx = nullptr; + NodeId from_peer = -1; + bool more = false; + while (CTransactionRef porphanTx = m_orphanage.GetTxToReconsider(peer.m_id, from_peer, more)) { const MempoolAcceptResult result = m_chainman.ProcessTransaction(porphanTx); const TxValidationState& state = result.m_state; + const uint256& orphanHash = porphanTx->GetHash(); if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString()); RelayTransaction(orphanHash, porphanTx->GetWitnessHash()); - m_orphanage.AddChildrenToWorkSet(*porphanTx, orphan_work_set); + m_orphanage.AddChildrenToWorkSet(*porphanTx, peer.m_id); m_orphanage.EraseTx(orphanHash); for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) { AddToCompactExtraTransactions(removedTx); @@ -2964,6 +2961,8 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set) break; } } + + return more; } bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, @@ -3281,17 +3280,14 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (greatest_common_version >= WTXID_RELAY_VERSION && m_txreconciliation) { // Per BIP-330, we announce txreconciliation support if: - // - protocol version per the VERSION message supports WTXID_RELAY; - // - we intended to exchange transactions over this connection while establishing it - // and the peer indicated support for transaction relay in the VERSION message; + // - protocol version per the peer's VERSION message supports WTXID_RELAY; + // - transaction relay is supported per the peer's VERSION message (see m_relays_txs); + // - this is not a block-relay-only connection and not a feeler (see m_relays_txs); + // - this is not an addr fetch connection; // - we are not in -blocksonly mode. - if (pfrom.m_relays_txs && !m_ignore_incoming_txs) { + if (pfrom.m_relays_txs && !pfrom.IsAddrFetchConn() && !m_ignore_incoming_txs) { const uint64_t recon_salt = m_txreconciliation->PreRegisterPeer(pfrom.GetId()); - // We suggest our txreconciliation role (initiator/responder) based on - // the connection direction. m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDTXRCNCL, - !pfrom.IsInboundConn(), - pfrom.IsInboundConn(), TXRECONCILIATION_VERSION, recon_salt)); } } @@ -3522,41 +3518,44 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (pfrom.fSuccessfullyConnected) { - // Disconnect peers that send a SENDTXRCNCL message after VERACK. LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received after verack from peer=%d; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; } - if (!peer->GetTxRelay()) { - // Disconnect peers that send a SENDTXRCNCL message even though we indicated we don't - // support transaction relay. + // Peer must not offer us reconciliations if we specified no tx relay support in VERSION. + if (RejectIncomingTxs(pfrom)) { LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received from peer=%d to which we indicated no tx relay; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; } - bool is_peer_initiator, is_peer_responder; + // Peer must not offer us reconciliations if they specified no tx relay support in VERSION. + // This flag might also be false in other cases, but the RejectIncomingTxs check above + // eliminates them, so that this flag fully represents what we are looking for. + if (!pfrom.m_relays_txs) { + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received from peer=%d which indicated no tx relay to us; disconnecting\n", pfrom.GetId()); + pfrom.fDisconnect = true; + return; + } + uint32_t peer_txreconcl_version; uint64_t remote_salt; - vRecv >> is_peer_initiator >> is_peer_responder >> peer_txreconcl_version >> remote_salt; + vRecv >> peer_txreconcl_version >> remote_salt; - if (m_txreconciliation->IsPeerRegistered(pfrom.GetId())) { - // A peer is already registered, meaning we already received SENDTXRCNCL from them. + const ReconciliationRegisterResult result = m_txreconciliation->RegisterPeer(pfrom.GetId(), pfrom.IsInboundConn(), + peer_txreconcl_version, remote_salt); + switch (result) { + case ReconciliationRegisterResult::NOT_FOUND: + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "Ignore unexpected txreconciliation signal from peer=%d\n", pfrom.GetId()); + break; + case ReconciliationRegisterResult::SUCCESS: + break; + case ReconciliationRegisterResult::ALREADY_REGISTERED: LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "txreconciliation protocol violation from peer=%d (sendtxrcncl received from already registered peer); disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; - } - - const ReconciliationRegisterResult result = m_txreconciliation->RegisterPeer(pfrom.GetId(), pfrom.IsInboundConn(), - is_peer_initiator, is_peer_responder, - peer_txreconcl_version, - remote_salt); - - // If it's a protocol violation, disconnect. - // If the peer was not found (but something unexpected happened) or it was registered, - // nothing to be done. - if (result == ReconciliationRegisterResult::PROTOCOL_VIOLATION) { + case ReconciliationRegisterResult::PROTOCOL_VIOLATION: LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "txreconciliation protocol violation from peer=%d; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; @@ -4011,7 +4010,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, AddKnownTx(*peer, txid); } - LOCK2(cs_main, g_cs_orphans); + LOCK(cs_main); m_txrequest.ReceivedResponse(pfrom.GetId(), txid); if (tx.HasWitness()) m_txrequest.ReceivedResponse(pfrom.GetId(), wtxid); @@ -4052,7 +4051,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, m_txrequest.ForgetTxHash(tx.GetHash()); m_txrequest.ForgetTxHash(tx.GetWitnessHash()); RelayTransaction(tx.GetHash(), tx.GetWitnessHash()); - m_orphanage.AddChildrenToWorkSet(tx, peer->m_orphan_work_set); + m_orphanage.AddChildrenToWorkSet(tx, peer->m_id); pfrom.m_last_tx_time = GetTime<std::chrono::seconds>(); @@ -4066,7 +4065,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } // Recursively process any orphan transactions that depended on this one - ProcessOrphanTx(peer->m_orphan_work_set); + ProcessOrphanTx(*peer); } else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { @@ -4247,7 +4246,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, bool fBlockReconstructed = false; { - LOCK2(cs_main, g_cs_orphans); + LOCK(cs_main); // If AcceptBlockHeader returned true, it set pindex assert(pindex); UpdateBlockAvailability(pfrom.GetId(), pindex->GetBlockHash()); @@ -4875,16 +4874,17 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt } } + bool has_more_orphans; { - LOCK2(cs_main, g_cs_orphans); - if (!peer->m_orphan_work_set.empty()) { - ProcessOrphanTx(peer->m_orphan_work_set); - } + LOCK(cs_main); + has_more_orphans = ProcessOrphanTx(*peer); } if (pfrom->fDisconnect) return false; + if (has_more_orphans) return true; + // this maintains the order of responses // and prevents m_getdata_requests to grow unbounded { @@ -4892,11 +4892,6 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt if (!peer->m_getdata_requests.empty()) return true; } - { - LOCK(g_cs_orphans); - if (!peer->m_orphan_work_set.empty()) return true; - } - // Don't bother if send buffer is too full to respond anyway if (pfrom->fPauseSend) return false; diff --git a/src/node/txreconciliation.cpp b/src/node/txreconciliation.cpp index 974358fcda..ed04a78cec 100644 --- a/src/node/txreconciliation.cpp +++ b/src/node/txreconciliation.cpp @@ -39,7 +39,8 @@ public: * the following commits. * * Reconciliation protocol assumes using one role consistently: either a reconciliation - * initiator (requesting sketches), or responder (sending sketches). This defines our role. + * initiator (requesting sketches), or responder (sending sketches). This defines our role, + * based on the direction of the p2p connection. * */ bool m_we_initiate; @@ -81,31 +82,30 @@ public: { AssertLockNotHeld(m_txreconciliation_mutex); LOCK(m_txreconciliation_mutex); - // We do not support txreconciliation salt/version updates. - assert(m_states.find(peer_id) == m_states.end()); LogPrintLevel(BCLog::TXRECONCILIATION, BCLog::Level::Debug, "Pre-register peer=%d\n", peer_id); const uint64_t local_salt{GetRand(UINT64_MAX)}; // We do this exactly once per peer (which are unique by NodeId, see GetNewNodeId) so it's // safe to assume we don't have this record yet. - Assert(m_states.emplace(peer_id, local_salt).second); + Assume(m_states.emplace(peer_id, local_salt).second); return local_salt; } - ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, bool is_peer_recon_initiator, - bool is_peer_recon_responder, uint32_t peer_recon_version, - uint64_t remote_salt) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) + ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, uint32_t peer_recon_version, + uint64_t remote_salt) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) { AssertLockNotHeld(m_txreconciliation_mutex); LOCK(m_txreconciliation_mutex); auto recon_state = m_states.find(peer_id); - // A peer should be in the pre-registered state to proceed here. - if (recon_state == m_states.end()) return NOT_FOUND; - uint64_t* local_salt = std::get_if<uint64_t>(&recon_state->second); - // A peer is already registered. This should be checked by the caller. - Assume(local_salt); + if (recon_state == m_states.end()) return ReconciliationRegisterResult::NOT_FOUND; + + if (std::holds_alternative<TxReconciliationState>(recon_state->second)) { + return ReconciliationRegisterResult::ALREADY_REGISTERED; + } + + uint64_t local_salt = *std::get_if<uint64_t>(&recon_state->second); // If the peer supports the version which is lower than ours, we downgrade to the version // it supports. For now, this only guarantees that nodes with future reconciliation @@ -114,27 +114,14 @@ public: // satisfactory (e.g. too low). const uint32_t recon_version{std::min(peer_recon_version, m_recon_version)}; // v1 is the lowest version, so suggesting something below must be a protocol violation. - if (recon_version < 1) return PROTOCOL_VIOLATION; - - // Must match SENDTXRCNCL logic. - const bool they_initiate = is_peer_recon_initiator && is_peer_inbound; - const bool we_initiate = !is_peer_inbound && is_peer_recon_responder; - - // If we ever announce support for both requesting and responding, this will need - // tie-breaking. For now, this is mutually exclusive because both are based on the - // inbound flag. - assert(!(they_initiate && we_initiate)); - - // The peer set both flags to false, we treat it as a protocol violation. - if (!(they_initiate || we_initiate)) return PROTOCOL_VIOLATION; + if (recon_version < 1) return ReconciliationRegisterResult::PROTOCOL_VIOLATION; - LogPrintLevel(BCLog::TXRECONCILIATION, BCLog::Level::Debug, "Register peer=%d with the following params: " /* Continued */ - "we_initiate=%i, they_initiate=%i.\n", - peer_id, we_initiate, they_initiate); + LogPrintLevel(BCLog::TXRECONCILIATION, BCLog::Level::Debug, "Register peer=%d (inbound=%i)\n", + peer_id, is_peer_inbound); - const uint256 full_salt{ComputeSalt(*local_salt, remote_salt)}; - recon_state->second = TxReconciliationState(we_initiate, full_salt.GetUint64(0), full_salt.GetUint64(1)); - return SUCCESS; + const uint256 full_salt{ComputeSalt(local_salt, remote_salt)}; + recon_state->second = TxReconciliationState(!is_peer_inbound, full_salt.GetUint64(0), full_salt.GetUint64(1)); + return ReconciliationRegisterResult::SUCCESS; } void ForgetPeer(NodeId peer_id) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) @@ -166,11 +153,9 @@ uint64_t TxReconciliationTracker::PreRegisterPeer(NodeId peer_id) } ReconciliationRegisterResult TxReconciliationTracker::RegisterPeer(NodeId peer_id, bool is_peer_inbound, - bool is_peer_recon_initiator, bool is_peer_recon_responder, uint32_t peer_recon_version, uint64_t remote_salt) { - return m_impl->RegisterPeer(peer_id, is_peer_inbound, is_peer_recon_initiator, is_peer_recon_responder, - peer_recon_version, remote_salt); + return m_impl->RegisterPeer(peer_id, is_peer_inbound, peer_recon_version, remote_salt); } void TxReconciliationTracker::ForgetPeer(NodeId peer_id) diff --git a/src/node/txreconciliation.h b/src/node/txreconciliation.h index a4f0870914..4591dd5df7 100644 --- a/src/node/txreconciliation.h +++ b/src/node/txreconciliation.h @@ -16,10 +16,11 @@ static constexpr bool DEFAULT_TXRECONCILIATION_ENABLE{false}; /** Supported transaction reconciliation protocol version */ static constexpr uint32_t TXRECONCILIATION_VERSION{1}; -enum ReconciliationRegisterResult { - NOT_FOUND = 0, - SUCCESS = 1, - PROTOCOL_VIOLATION = 2, +enum class ReconciliationRegisterResult { + NOT_FOUND, + SUCCESS, + ALREADY_REGISTERED, + PROTOCOL_VIOLATION, }; /** @@ -72,8 +73,8 @@ public: * Step 0. Once the peer agreed to reconcile txs with us, generate the state required to track * ongoing reconciliations. Must be called only after pre-registering the peer and only once. */ - ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, bool is_peer_recon_initiator, - bool is_peer_recon_responder, uint32_t peer_recon_version, uint64_t remote_salt); + ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, + uint32_t peer_recon_version, uint64_t remote_salt); /** * Attempts to forget txreconciliation-related state of the peer (if we previously stored any). diff --git a/src/protocol.h b/src/protocol.h index 17a363b1d3..51fabf8da0 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -259,9 +259,7 @@ extern const char* CFCHECKPT; */ extern const char* WTXIDRELAY; /** - * Contains 2 1-byte bools, a 4-byte version number and an 8-byte salt. - * The 2 booleans indicate that a node is willing to participate in transaction - * reconciliation, respectively as an initiator or as a receiver. + * Contains a 4-byte version number and an 8-byte salt. * The salt is used to compute short txids needed for efficient * txreconciliation, as described by BIP 330. */ diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 8688263ef5..b3434b80c7 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -277,11 +277,13 @@ UniValue RPCConvertValues(const std::string &strMethod, const std::vector<std::s UniValue RPCConvertNamedValues(const std::string &strMethod, const std::vector<std::string> &strParams) { UniValue params(UniValue::VOBJ); + UniValue positional_args{UniValue::VARR}; for (const std::string &s: strParams) { size_t pos = s.find('='); if (pos == std::string::npos) { - throw(std::runtime_error("No '=' in named argument '"+s+"', this needs to be present for every argument (even if it is empty)")); + positional_args.push_back(rpcCvtTable.convert(strMethod, positional_args.size()) ? ParseNonRFCJSONValue(s) : s); + continue; } std::string name = s.substr(0, pos); @@ -296,5 +298,9 @@ UniValue RPCConvertNamedValues(const std::string &strMethod, const std::vector<s } } + if (!positional_args.empty()) { + params.pushKV("args", positional_args); + } + return params; } diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index a980c609e8..2ac6d6d76f 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -222,10 +222,11 @@ static RPCHelpMan deriveaddresses() return RPCHelpMan{"deriveaddresses", {"\nDerives one or more addresses corresponding to an output descriptor.\n" "Examples of output descriptors are:\n" - " pkh(<pubkey>) P2PKH outputs for the given pubkey\n" - " wpkh(<pubkey>) Native segwit P2PKH outputs for the given pubkey\n" - " sh(multi(<n>,<pubkey>,<pubkey>,...)) P2SH-multisig outputs for the given threshold and pubkeys\n" - " raw(<hex script>) Outputs whose scriptPubKey equals the specified hex scripts\n" + " pkh(<pubkey>) P2PKH outputs for the given pubkey\n" + " wpkh(<pubkey>) Native segwit P2PKH outputs for the given pubkey\n" + " sh(multi(<n>,<pubkey>,<pubkey>,...)) P2SH-multisig outputs for the given threshold and pubkeys\n" + " raw(<hex script>) Outputs whose scriptPubKey equals the specified hex scripts\n" + " tr(<pubkey>,multi_a(<n>,<pubkey>,<pubkey>,...)) P2TR-multisig outputs for the given threshold and pubkeys\n" "\nIn the above, <pubkey> either refers to a fixed public key in hexadecimal notation, or to an xpub/xprv optionally followed by one\n" "or more path elements separated by \"/\", where \"h\" represents a hardened child key.\n" "For more information on output descriptors, see the documentation in the doc/descriptors.md file.\n"}, diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 1d7bd2eb94..f57133f75b 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -401,8 +401,16 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c for (size_t i=0; i<keys.size(); ++i) { argsIn[keys[i]] = &values[i]; } - // Process expected parameters. + // Process expected parameters. If any parameters were left unspecified in + // the request before a parameter that was specified, null values need to be + // inserted at the unspecifed parameter positions, and the "hole" variable + // below tracks the number of null values that need to be inserted. + // The "initial_hole_size" variable stores the size of the initial hole, + // i.e. how many initial positional arguments were left unspecified. This is + // used after the for-loop to add initial positional arguments from the + // "args" parameter, if present. int hole = 0; + int initial_hole_size = 0; for (const std::string &argNamePattern: argNames) { std::vector<std::string> vargNames = SplitString(argNamePattern, '|'); auto fr = argsIn.end(); @@ -424,6 +432,24 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c argsIn.erase(fr); } else { hole += 1; + if (out.params.empty()) initial_hole_size = hole; + } + } + // If leftover "args" param was found, use it as a source of positional + // arguments and add named arguments after. This is a convenience for + // clients that want to pass a combination of named and positional + // arguments as described in doc/JSON-RPC-interface.md#parameter-passing + auto positional_args{argsIn.extract("args")}; + if (positional_args && positional_args.mapped()->isArray()) { + const bool has_named_arguments{initial_hole_size < (int)argNames.size()}; + if (initial_hole_size < (int)positional_args.mapped()->size() && has_named_arguments) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + argNames[initial_hole_size] + " specified twice both as positional and named argument"); + } + // Assign positional_args to out.params and append named_args after. + UniValue named_args{std::move(out.params)}; + out.params = *positional_args.mapped(); + for (size_t i{out.params.size()}; i < named_args.size(); ++i) { + out.params.push_back(named_args[i]); } } // If there are still arguments in the argsIn map, this is an error. diff --git a/src/test/fuzz/i2p.cpp b/src/test/fuzz/i2p.cpp index b6e3ca07e2..72b7f9e334 100644 --- a/src/test/fuzz/i2p.cpp +++ b/src/test/fuzz/i2p.cpp @@ -8,6 +8,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <test/fuzz/util/net.h> #include <test/util/setup_common.h> #include <util/system.h> #include <util/threadinterrupt.h> diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp index 741810f6a2..9f7c87dcd5 100644 --- a/src/test/fuzz/net.cpp +++ b/src/test/fuzz/net.cpp @@ -12,6 +12,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <test/fuzz/util/net.h> #include <test/util/net.h> #include <test/util/setup_common.h> #include <util/asmap.h> diff --git a/src/test/fuzz/net_permissions.cpp b/src/test/fuzz/net_permissions.cpp index e62fe0328e..21a6640ef4 100644 --- a/src/test/fuzz/net_permissions.cpp +++ b/src/test/fuzz/net_permissions.cpp @@ -6,6 +6,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <test/fuzz/util/net.h> #include <util/translation.h> #include <cassert> diff --git a/src/test/fuzz/node_eviction.cpp b/src/test/fuzz/node_eviction.cpp index e27b254580..0f204babfa 100644 --- a/src/test/fuzz/node_eviction.cpp +++ b/src/test/fuzz/node_eviction.cpp @@ -7,6 +7,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <test/fuzz/util/net.h> #include <algorithm> #include <cassert> diff --git a/src/test/fuzz/pow.cpp b/src/test/fuzz/pow.cpp index eba03da773..82fac8b9ee 100644 --- a/src/test/fuzz/pow.cpp +++ b/src/test/fuzz/pow.cpp @@ -9,6 +9,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <util/check.h> #include <util/overflow.h> #include <cstdint> diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 5a4df735da..f6000535b3 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -14,11 +14,11 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <test/fuzz/util/net.h> #include <test/util/mining.h> #include <test/util/net.h> #include <test/util/setup_common.h> #include <test/util/validation.h> -#include <txorphanage.h> #include <validationinterface.h> #include <version.h> diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 1df1717ec3..41831fd176 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -9,11 +9,11 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <test/fuzz/util/net.h> #include <test/util/mining.h> #include <test/util/net.h> #include <test/util/setup_common.h> #include <test/util/validation.h> -#include <txorphanage.h> #include <validation.h> #include <validationinterface.h> diff --git a/src/test/fuzz/socks5.cpp b/src/test/fuzz/socks5.cpp index c3a6eed089..15f479b009 100644 --- a/src/test/fuzz/socks5.cpp +++ b/src/test/fuzz/socks5.cpp @@ -7,6 +7,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <test/fuzz/util/net.h> #include <test/util/setup_common.h> #include <cstdint> diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index 55060f31cf..02743051e8 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -36,7 +36,6 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) SetMockTime(ConsumeTime(fuzzed_data_provider)); TxOrphanage orphanage; - std::set<uint256> orphan_work_set; std::vector<COutPoint> outpoints; // initial outpoints used to construct transactions later for (uint8_t i = 0; i < 4; i++) { @@ -86,15 +85,19 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) CallOneOf( fuzzed_data_provider, [&] { - LOCK(g_cs_orphans); - orphanage.AddChildrenToWorkSet(*tx, orphan_work_set); + orphanage.AddChildrenToWorkSet(*tx, peer_id); }, [&] { - bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); { - LOCK(g_cs_orphans); - bool get_tx = orphanage.GetTx(tx->GetHash()).first != nullptr; - Assert(have_tx == get_tx); + NodeId originator; + bool more = true; + CTransactionRef ref = orphanage.GetTxToReconsider(peer_id, originator, more); + if (!ref) { + Assert(!more); + } else { + bool have_tx = orphanage.HaveTx(GenTxid::Txid(ref->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(ref->GetHash())); + Assert(have_tx); + } } }, [&] { @@ -102,14 +105,12 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) // AddTx should return false if tx is too big or already have it // tx weight is unknown, we only check when tx is already in orphanage { - LOCK(g_cs_orphans); bool add_tx = orphanage.AddTx(tx, peer_id); // have_tx == true -> add_tx == false Assert(!have_tx || !add_tx); } have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); { - LOCK(g_cs_orphans); bool add_tx = orphanage.AddTx(tx, peer_id); // if have_tx is still false, it must be too big Assert(!have_tx == (GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT)); @@ -120,25 +121,22 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); // EraseTx should return 0 if m_orphans doesn't have the tx { - LOCK(g_cs_orphans); Assert(have_tx == orphanage.EraseTx(tx->GetHash())); } have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); // have_tx should be false and EraseTx should fail { - LOCK(g_cs_orphans); Assert(!have_tx && !orphanage.EraseTx(tx->GetHash())); } }, [&] { - LOCK(g_cs_orphans); orphanage.EraseForPeer(peer_id); }, [&] { // test mocktime and expiry SetMockTime(ConsumeTime(fuzzed_data_provider)); auto limit = fuzzed_data_provider.ConsumeIntegral<unsigned int>(); - WITH_LOCK(g_cs_orphans, orphanage.LimitOrphans(limit)); + orphanage.LimitOrphans(limit); Assert(orphanage.Size() <= limit); }); } diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index 73ceb94b14..8babfadf4f 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -3,12 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <consensus/amount.h> -#include <net_processing.h> -#include <netaddress.h> -#include <netmessagemaker.h> #include <pubkey.h> #include <test/fuzz/util.h> #include <test/util/script.h> +#include <util/check.h> #include <util/overflow.h> #include <util/rbf.h> #include <util/time.h> @@ -16,308 +14,6 @@ #include <memory> -FuzzedSock::FuzzedSock(FuzzedDataProvider& fuzzed_data_provider) - : m_fuzzed_data_provider{fuzzed_data_provider}, m_selectable{fuzzed_data_provider.ConsumeBool()} -{ - m_socket = fuzzed_data_provider.ConsumeIntegralInRange<SOCKET>(INVALID_SOCKET - 1, INVALID_SOCKET); -} - -FuzzedSock::~FuzzedSock() -{ - // Sock::~Sock() will be called after FuzzedSock::~FuzzedSock() and it will call - // close(m_socket) if m_socket is not INVALID_SOCKET. - // Avoid closing an arbitrary file descriptor (m_socket is just a random very high number which - // theoretically may concide with a real opened file descriptor). - m_socket = INVALID_SOCKET; -} - -FuzzedSock& FuzzedSock::operator=(Sock&& other) -{ - assert(false && "Move of Sock into FuzzedSock not allowed."); - return *this; -} - -ssize_t FuzzedSock::Send(const void* data, size_t len, int flags) const -{ - constexpr std::array send_errnos{ - EACCES, - EAGAIN, - EALREADY, - EBADF, - ECONNRESET, - EDESTADDRREQ, - EFAULT, - EINTR, - EINVAL, - EISCONN, - EMSGSIZE, - ENOBUFS, - ENOMEM, - ENOTCONN, - ENOTSOCK, - EOPNOTSUPP, - EPIPE, - EWOULDBLOCK, - }; - if (m_fuzzed_data_provider.ConsumeBool()) { - return len; - } - const ssize_t r = m_fuzzed_data_provider.ConsumeIntegralInRange<ssize_t>(-1, len); - if (r == -1) { - SetFuzzedErrNo(m_fuzzed_data_provider, send_errnos); - } - return r; -} - -ssize_t FuzzedSock::Recv(void* buf, size_t len, int flags) const -{ - // Have a permanent error at recv_errnos[0] because when the fuzzed data is exhausted - // SetFuzzedErrNo() will always return the first element and we want to avoid Recv() - // returning -1 and setting errno to EAGAIN repeatedly. - constexpr std::array recv_errnos{ - ECONNREFUSED, - EAGAIN, - EBADF, - EFAULT, - EINTR, - EINVAL, - ENOMEM, - ENOTCONN, - ENOTSOCK, - EWOULDBLOCK, - }; - assert(buf != nullptr || len == 0); - if (len == 0 || m_fuzzed_data_provider.ConsumeBool()) { - const ssize_t r = m_fuzzed_data_provider.ConsumeBool() ? 0 : -1; - if (r == -1) { - SetFuzzedErrNo(m_fuzzed_data_provider, recv_errnos); - } - return r; - } - std::vector<uint8_t> random_bytes; - bool pad_to_len_bytes{m_fuzzed_data_provider.ConsumeBool()}; - if (m_peek_data.has_value()) { - // `MSG_PEEK` was used in the preceding `Recv()` call, return `m_peek_data`. - random_bytes.assign({m_peek_data.value()}); - if ((flags & MSG_PEEK) == 0) { - m_peek_data.reset(); - } - pad_to_len_bytes = false; - } else if ((flags & MSG_PEEK) != 0) { - // New call with `MSG_PEEK`. - random_bytes = m_fuzzed_data_provider.ConsumeBytes<uint8_t>(1); - if (!random_bytes.empty()) { - m_peek_data = random_bytes[0]; - pad_to_len_bytes = false; - } - } else { - random_bytes = m_fuzzed_data_provider.ConsumeBytes<uint8_t>( - m_fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, len)); - } - if (random_bytes.empty()) { - const ssize_t r = m_fuzzed_data_provider.ConsumeBool() ? 0 : -1; - if (r == -1) { - SetFuzzedErrNo(m_fuzzed_data_provider, recv_errnos); - } - return r; - } - std::memcpy(buf, random_bytes.data(), random_bytes.size()); - if (pad_to_len_bytes) { - if (len > random_bytes.size()) { - std::memset((char*)buf + random_bytes.size(), 0, len - random_bytes.size()); - } - return len; - } - if (m_fuzzed_data_provider.ConsumeBool() && std::getenv("FUZZED_SOCKET_FAKE_LATENCY") != nullptr) { - std::this_thread::sleep_for(std::chrono::milliseconds{2}); - } - return random_bytes.size(); -} - -int FuzzedSock::Connect(const sockaddr*, socklen_t) const -{ - // Have a permanent error at connect_errnos[0] because when the fuzzed data is exhausted - // SetFuzzedErrNo() will always return the first element and we want to avoid Connect() - // returning -1 and setting errno to EAGAIN repeatedly. - constexpr std::array connect_errnos{ - ECONNREFUSED, - EAGAIN, - ECONNRESET, - EHOSTUNREACH, - EINPROGRESS, - EINTR, - ENETUNREACH, - ETIMEDOUT, - }; - if (m_fuzzed_data_provider.ConsumeBool()) { - SetFuzzedErrNo(m_fuzzed_data_provider, connect_errnos); - return -1; - } - return 0; -} - -int FuzzedSock::Bind(const sockaddr*, socklen_t) const -{ - // Have a permanent error at bind_errnos[0] because when the fuzzed data is exhausted - // SetFuzzedErrNo() will always set the global errno to bind_errnos[0]. We want to - // avoid this method returning -1 and setting errno to a temporary error (like EAGAIN) - // repeatedly because proper code should retry on temporary errors, leading to an - // infinite loop. - constexpr std::array bind_errnos{ - EACCES, - EADDRINUSE, - EADDRNOTAVAIL, - EAGAIN, - }; - if (m_fuzzed_data_provider.ConsumeBool()) { - SetFuzzedErrNo(m_fuzzed_data_provider, bind_errnos); - return -1; - } - return 0; -} - -int FuzzedSock::Listen(int) const -{ - // Have a permanent error at listen_errnos[0] because when the fuzzed data is exhausted - // SetFuzzedErrNo() will always set the global errno to listen_errnos[0]. We want to - // avoid this method returning -1 and setting errno to a temporary error (like EAGAIN) - // repeatedly because proper code should retry on temporary errors, leading to an - // infinite loop. - constexpr std::array listen_errnos{ - EADDRINUSE, - EINVAL, - EOPNOTSUPP, - }; - if (m_fuzzed_data_provider.ConsumeBool()) { - SetFuzzedErrNo(m_fuzzed_data_provider, listen_errnos); - return -1; - } - return 0; -} - -std::unique_ptr<Sock> FuzzedSock::Accept(sockaddr* addr, socklen_t* addr_len) const -{ - constexpr std::array accept_errnos{ - ECONNABORTED, - EINTR, - ENOMEM, - }; - if (m_fuzzed_data_provider.ConsumeBool()) { - SetFuzzedErrNo(m_fuzzed_data_provider, accept_errnos); - return std::unique_ptr<FuzzedSock>(); - } - return std::make_unique<FuzzedSock>(m_fuzzed_data_provider); -} - -int FuzzedSock::GetSockOpt(int level, int opt_name, void* opt_val, socklen_t* opt_len) const -{ - constexpr std::array getsockopt_errnos{ - ENOMEM, - ENOBUFS, - }; - if (m_fuzzed_data_provider.ConsumeBool()) { - SetFuzzedErrNo(m_fuzzed_data_provider, getsockopt_errnos); - return -1; - } - if (opt_val == nullptr) { - return 0; - } - std::memcpy(opt_val, - ConsumeFixedLengthByteVector(m_fuzzed_data_provider, *opt_len).data(), - *opt_len); - return 0; -} - -int FuzzedSock::SetSockOpt(int, int, const void*, socklen_t) const -{ - constexpr std::array setsockopt_errnos{ - ENOMEM, - ENOBUFS, - }; - if (m_fuzzed_data_provider.ConsumeBool()) { - SetFuzzedErrNo(m_fuzzed_data_provider, setsockopt_errnos); - return -1; - } - return 0; -} - -int FuzzedSock::GetSockName(sockaddr* name, socklen_t* name_len) const -{ - constexpr std::array getsockname_errnos{ - ECONNRESET, - ENOBUFS, - }; - if (m_fuzzed_data_provider.ConsumeBool()) { - SetFuzzedErrNo(m_fuzzed_data_provider, getsockname_errnos); - return -1; - } - *name_len = m_fuzzed_data_provider.ConsumeData(name, *name_len); - return 0; -} - -bool FuzzedSock::SetNonBlocking() const -{ - constexpr std::array setnonblocking_errnos{ - EBADF, - EPERM, - }; - if (m_fuzzed_data_provider.ConsumeBool()) { - SetFuzzedErrNo(m_fuzzed_data_provider, setnonblocking_errnos); - return false; - } - return true; -} - -bool FuzzedSock::IsSelectable() const -{ - return m_selectable; -} - -bool FuzzedSock::Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred) const -{ - constexpr std::array wait_errnos{ - EBADF, - EINTR, - EINVAL, - }; - if (m_fuzzed_data_provider.ConsumeBool()) { - SetFuzzedErrNo(m_fuzzed_data_provider, wait_errnos); - return false; - } - if (occurred != nullptr) { - *occurred = m_fuzzed_data_provider.ConsumeBool() ? requested : 0; - } - return true; -} - -bool FuzzedSock::WaitMany(std::chrono::milliseconds timeout, EventsPerSock& events_per_sock) const -{ - for (auto& [sock, events] : events_per_sock) { - (void)sock; - events.occurred = m_fuzzed_data_provider.ConsumeBool() ? events.requested : 0; - } - return true; -} - -bool FuzzedSock::IsConnected(std::string& errmsg) const -{ - if (m_fuzzed_data_provider.ConsumeBool()) { - return true; - } - errmsg = "disconnected at random by the fuzzer"; - return false; -} - -void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, CNode& node) noexcept -{ - connman.Handshake(node, - /*successfully_connected=*/fuzzed_data_provider.ConsumeBool(), - /*remote_services=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), - /*local_services=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), - /*version=*/fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()), - /*relay_txs=*/fuzzed_data_provider.ConsumeBool()); -} - CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider, const std::optional<CAmount>& max) noexcept { return fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(0, max.value_or(MAX_MONEY)); @@ -508,11 +204,6 @@ bool ContainsSpentInput(const CTransaction& tx, const CCoinsViewCache& inputs) n return false; } -CAddress ConsumeAddress(FuzzedDataProvider& fuzzed_data_provider) noexcept -{ - return {ConsumeService(fuzzed_data_provider), ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), NodeSeconds{std::chrono::seconds{fuzzed_data_provider.ConsumeIntegral<uint32_t>()}}}; -} - FILE* FuzzedFileProvider::open() { SetFuzzedErrNo(m_fuzzed_data_provider); diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index ecd6eead3f..09c57c7be3 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -12,9 +12,6 @@ #include <consensus/amount.h> #include <consensus/consensus.h> #include <merkleblock.h> -#include <net.h> -#include <netaddress.h> -#include <netbase.h> #include <primitives/transaction.h> #include <script/script.h> #include <script/standard.h> @@ -22,8 +19,6 @@ #include <streams.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> -#include <test/fuzz/util/net.h> -#include <test/util/net.h> #include <uint256.h> #include <version.h> @@ -37,65 +32,6 @@ class PeerManager; -class FuzzedSock : public Sock -{ - FuzzedDataProvider& m_fuzzed_data_provider; - - /** - * Data to return when `MSG_PEEK` is used as a `Recv()` flag. - * If `MSG_PEEK` is used, then our `Recv()` returns some random data as usual, but on the next - * `Recv()` call we must return the same data, thus we remember it here. - */ - mutable std::optional<uint8_t> m_peek_data; - - /** - * Whether to pretend that the socket is select(2)-able. This is randomly set in the - * constructor. It should remain constant so that repeated calls to `IsSelectable()` - * return the same value. - */ - const bool m_selectable; - -public: - explicit FuzzedSock(FuzzedDataProvider& fuzzed_data_provider); - - ~FuzzedSock() override; - - FuzzedSock& operator=(Sock&& other) override; - - ssize_t Send(const void* data, size_t len, int flags) const override; - - ssize_t Recv(void* buf, size_t len, int flags) const override; - - int Connect(const sockaddr*, socklen_t) const override; - - int Bind(const sockaddr*, socklen_t) const override; - - int Listen(int backlog) const override; - - std::unique_ptr<Sock> Accept(sockaddr* addr, socklen_t* addr_len) const override; - - int GetSockOpt(int level, int opt_name, void* opt_val, socklen_t* opt_len) const override; - - int SetSockOpt(int level, int opt_name, const void* opt_val, socklen_t opt_len) const override; - - int GetSockName(sockaddr* name, socklen_t* name_len) const override; - - bool SetNonBlocking() const override; - - bool IsSelectable() const override; - - bool Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred = nullptr) const override; - - bool WaitMany(std::chrono::milliseconds timeout, EventsPerSock& events_per_sock) const override; - - bool IsConnected(std::string& errmsg) const override; -}; - -[[nodiscard]] inline FuzzedSock ConsumeSock(FuzzedDataProvider& fuzzed_data_provider) -{ - return FuzzedSock{fuzzed_data_provider}; -} - template <typename... Callables> size_t CallOneOf(FuzzedDataProvider& fuzzed_data_provider, Callables... callables) { @@ -284,59 +220,6 @@ inline void SetFuzzedErrNo(FuzzedDataProvider& fuzzed_data_provider) noexcept return result; } -inline CSubNet ConsumeSubNet(FuzzedDataProvider& fuzzed_data_provider) noexcept -{ - return {ConsumeNetAddr(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<uint8_t>()}; -} - -inline CService ConsumeService(FuzzedDataProvider& fuzzed_data_provider) noexcept -{ - return {ConsumeNetAddr(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<uint16_t>()}; -} - -CAddress ConsumeAddress(FuzzedDataProvider& fuzzed_data_provider) noexcept; - -template <bool ReturnUniquePtr = false> -auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<NodeId>& node_id_in = std::nullopt) noexcept -{ - const NodeId node_id = node_id_in.value_or(fuzzed_data_provider.ConsumeIntegralInRange<NodeId>(0, std::numeric_limits<NodeId>::max())); - const auto sock = std::make_shared<FuzzedSock>(fuzzed_data_provider); - const CAddress address = ConsumeAddress(fuzzed_data_provider); - const uint64_t keyed_net_group = fuzzed_data_provider.ConsumeIntegral<uint64_t>(); - const uint64_t local_host_nonce = fuzzed_data_provider.ConsumeIntegral<uint64_t>(); - const CAddress addr_bind = ConsumeAddress(fuzzed_data_provider); - const std::string addr_name = fuzzed_data_provider.ConsumeRandomLengthString(64); - const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray(ALL_CONNECTION_TYPES); - const bool inbound_onion{conn_type == ConnectionType::INBOUND ? fuzzed_data_provider.ConsumeBool() : false}; - NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS); - if constexpr (ReturnUniquePtr) { - return std::make_unique<CNode>(node_id, - sock, - address, - keyed_net_group, - local_host_nonce, - addr_bind, - addr_name, - conn_type, - inbound_onion, - CNodeOptions{ .permission_flags = permission_flags }); - } else { - return CNode{node_id, - sock, - address, - keyed_net_group, - local_host_nonce, - addr_bind, - addr_name, - conn_type, - inbound_onion, - CNodeOptions{ .permission_flags = permission_flags }}; - } -} -inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = std::nullopt) { return ConsumeNode<true>(fdp, node_id_in); } - -void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, CNode& node) noexcept EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex); - class FuzzedFileProvider { FuzzedDataProvider& m_fuzzed_data_provider; diff --git a/src/test/fuzz/util/mempool.cpp b/src/test/fuzz/util/mempool.cpp index ac83f6ca21..c6a6943603 100644 --- a/src/test/fuzz/util/mempool.cpp +++ b/src/test/fuzz/util/mempool.cpp @@ -3,12 +3,15 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <consensus/amount.h> +#include <consensus/consensus.h> #include <primitives/transaction.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/util.h> #include <test/fuzz/util/mempool.h> #include <txmempool_entry.h> +#include <cassert> +#include <cstdint> #include <limits> CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept diff --git a/src/test/fuzz/util/mempool.h b/src/test/fuzz/util/mempool.h index 4304e5294e..ada657d970 100644 --- a/src/test/fuzz/util/mempool.h +++ b/src/test/fuzz/util/mempool.h @@ -5,11 +5,13 @@ #ifndef BITCOIN_TEST_FUZZ_UTIL_MEMPOOL_H #define BITCOIN_TEST_FUZZ_UTIL_MEMPOOL_H -#include <primitives/transaction.h> -#include <test/fuzz/FuzzedDataProvider.h> -#include <txmempool.h> +#include <txmempool_entry.h> #include <validation.h> +class CTransaction; +class CTxMemPool; +class FuzzedDataProvider; + class DummyChainState final : public Chainstate { public: diff --git a/src/test/fuzz/util/net.cpp b/src/test/fuzz/util/net.cpp index f8e996cfa5..c6c6e3ad16 100644 --- a/src/test/fuzz/util/net.cpp +++ b/src/test/fuzz/util/net.cpp @@ -2,14 +2,29 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <test/fuzz/util/net.h> + #include <compat/compat.h> #include <netaddress.h> +#include <protocol.h> #include <test/fuzz/FuzzedDataProvider.h> -#include <util/strencodings.h> +#include <test/fuzz/util.h> +#include <test/util/net.h> +#include <util/sock.h> +#include <util/time.h> +#include <version.h> +#include <array> +#include <cassert> +#include <cerrno> #include <cstdint> +#include <cstdlib> +#include <cstring> +#include <thread> #include <vector> +class CNode; + CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept { const Network network = fuzzed_data_provider.PickValueInArray({Network::NET_IPV4, Network::NET_IPV6, Network::NET_INTERNAL, Network::NET_ONION}); @@ -34,3 +49,310 @@ CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept } return net_addr; } + +CAddress ConsumeAddress(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + return {ConsumeService(fuzzed_data_provider), ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), NodeSeconds{std::chrono::seconds{fuzzed_data_provider.ConsumeIntegral<uint32_t>()}}}; +} + +FuzzedSock::FuzzedSock(FuzzedDataProvider& fuzzed_data_provider) + : m_fuzzed_data_provider{fuzzed_data_provider}, m_selectable{fuzzed_data_provider.ConsumeBool()} +{ + m_socket = fuzzed_data_provider.ConsumeIntegralInRange<SOCKET>(INVALID_SOCKET - 1, INVALID_SOCKET); +} + +FuzzedSock::~FuzzedSock() +{ + // Sock::~Sock() will be called after FuzzedSock::~FuzzedSock() and it will call + // close(m_socket) if m_socket is not INVALID_SOCKET. + // Avoid closing an arbitrary file descriptor (m_socket is just a random very high number which + // theoretically may concide with a real opened file descriptor). + m_socket = INVALID_SOCKET; +} + +FuzzedSock& FuzzedSock::operator=(Sock&& other) +{ + assert(false && "Move of Sock into FuzzedSock not allowed."); + return *this; +} + +ssize_t FuzzedSock::Send(const void* data, size_t len, int flags) const +{ + constexpr std::array send_errnos{ + EACCES, + EAGAIN, + EALREADY, + EBADF, + ECONNRESET, + EDESTADDRREQ, + EFAULT, + EINTR, + EINVAL, + EISCONN, + EMSGSIZE, + ENOBUFS, + ENOMEM, + ENOTCONN, + ENOTSOCK, + EOPNOTSUPP, + EPIPE, + EWOULDBLOCK, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + return len; + } + const ssize_t r = m_fuzzed_data_provider.ConsumeIntegralInRange<ssize_t>(-1, len); + if (r == -1) { + SetFuzzedErrNo(m_fuzzed_data_provider, send_errnos); + } + return r; +} + +ssize_t FuzzedSock::Recv(void* buf, size_t len, int flags) const +{ + // Have a permanent error at recv_errnos[0] because when the fuzzed data is exhausted + // SetFuzzedErrNo() will always return the first element and we want to avoid Recv() + // returning -1 and setting errno to EAGAIN repeatedly. + constexpr std::array recv_errnos{ + ECONNREFUSED, + EAGAIN, + EBADF, + EFAULT, + EINTR, + EINVAL, + ENOMEM, + ENOTCONN, + ENOTSOCK, + EWOULDBLOCK, + }; + assert(buf != nullptr || len == 0); + if (len == 0 || m_fuzzed_data_provider.ConsumeBool()) { + const ssize_t r = m_fuzzed_data_provider.ConsumeBool() ? 0 : -1; + if (r == -1) { + SetFuzzedErrNo(m_fuzzed_data_provider, recv_errnos); + } + return r; + } + std::vector<uint8_t> random_bytes; + bool pad_to_len_bytes{m_fuzzed_data_provider.ConsumeBool()}; + if (m_peek_data.has_value()) { + // `MSG_PEEK` was used in the preceding `Recv()` call, return `m_peek_data`. + random_bytes.assign({m_peek_data.value()}); + if ((flags & MSG_PEEK) == 0) { + m_peek_data.reset(); + } + pad_to_len_bytes = false; + } else if ((flags & MSG_PEEK) != 0) { + // New call with `MSG_PEEK`. + random_bytes = m_fuzzed_data_provider.ConsumeBytes<uint8_t>(1); + if (!random_bytes.empty()) { + m_peek_data = random_bytes[0]; + pad_to_len_bytes = false; + } + } else { + random_bytes = m_fuzzed_data_provider.ConsumeBytes<uint8_t>( + m_fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, len)); + } + if (random_bytes.empty()) { + const ssize_t r = m_fuzzed_data_provider.ConsumeBool() ? 0 : -1; + if (r == -1) { + SetFuzzedErrNo(m_fuzzed_data_provider, recv_errnos); + } + return r; + } + std::memcpy(buf, random_bytes.data(), random_bytes.size()); + if (pad_to_len_bytes) { + if (len > random_bytes.size()) { + std::memset((char*)buf + random_bytes.size(), 0, len - random_bytes.size()); + } + return len; + } + if (m_fuzzed_data_provider.ConsumeBool() && std::getenv("FUZZED_SOCKET_FAKE_LATENCY") != nullptr) { + std::this_thread::sleep_for(std::chrono::milliseconds{2}); + } + return random_bytes.size(); +} + +int FuzzedSock::Connect(const sockaddr*, socklen_t) const +{ + // Have a permanent error at connect_errnos[0] because when the fuzzed data is exhausted + // SetFuzzedErrNo() will always return the first element and we want to avoid Connect() + // returning -1 and setting errno to EAGAIN repeatedly. + constexpr std::array connect_errnos{ + ECONNREFUSED, + EAGAIN, + ECONNRESET, + EHOSTUNREACH, + EINPROGRESS, + EINTR, + ENETUNREACH, + ETIMEDOUT, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, connect_errnos); + return -1; + } + return 0; +} + +int FuzzedSock::Bind(const sockaddr*, socklen_t) const +{ + // Have a permanent error at bind_errnos[0] because when the fuzzed data is exhausted + // SetFuzzedErrNo() will always set the global errno to bind_errnos[0]. We want to + // avoid this method returning -1 and setting errno to a temporary error (like EAGAIN) + // repeatedly because proper code should retry on temporary errors, leading to an + // infinite loop. + constexpr std::array bind_errnos{ + EACCES, + EADDRINUSE, + EADDRNOTAVAIL, + EAGAIN, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, bind_errnos); + return -1; + } + return 0; +} + +int FuzzedSock::Listen(int) const +{ + // Have a permanent error at listen_errnos[0] because when the fuzzed data is exhausted + // SetFuzzedErrNo() will always set the global errno to listen_errnos[0]. We want to + // avoid this method returning -1 and setting errno to a temporary error (like EAGAIN) + // repeatedly because proper code should retry on temporary errors, leading to an + // infinite loop. + constexpr std::array listen_errnos{ + EADDRINUSE, + EINVAL, + EOPNOTSUPP, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, listen_errnos); + return -1; + } + return 0; +} + +std::unique_ptr<Sock> FuzzedSock::Accept(sockaddr* addr, socklen_t* addr_len) const +{ + constexpr std::array accept_errnos{ + ECONNABORTED, + EINTR, + ENOMEM, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, accept_errnos); + return std::unique_ptr<FuzzedSock>(); + } + return std::make_unique<FuzzedSock>(m_fuzzed_data_provider); +} + +int FuzzedSock::GetSockOpt(int level, int opt_name, void* opt_val, socklen_t* opt_len) const +{ + constexpr std::array getsockopt_errnos{ + ENOMEM, + ENOBUFS, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, getsockopt_errnos); + return -1; + } + if (opt_val == nullptr) { + return 0; + } + std::memcpy(opt_val, + ConsumeFixedLengthByteVector(m_fuzzed_data_provider, *opt_len).data(), + *opt_len); + return 0; +} + +int FuzzedSock::SetSockOpt(int, int, const void*, socklen_t) const +{ + constexpr std::array setsockopt_errnos{ + ENOMEM, + ENOBUFS, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, setsockopt_errnos); + return -1; + } + return 0; +} + +int FuzzedSock::GetSockName(sockaddr* name, socklen_t* name_len) const +{ + constexpr std::array getsockname_errnos{ + ECONNRESET, + ENOBUFS, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, getsockname_errnos); + return -1; + } + *name_len = m_fuzzed_data_provider.ConsumeData(name, *name_len); + return 0; +} + +bool FuzzedSock::SetNonBlocking() const +{ + constexpr std::array setnonblocking_errnos{ + EBADF, + EPERM, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, setnonblocking_errnos); + return false; + } + return true; +} + +bool FuzzedSock::IsSelectable() const +{ + return m_selectable; +} + +bool FuzzedSock::Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred) const +{ + constexpr std::array wait_errnos{ + EBADF, + EINTR, + EINVAL, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, wait_errnos); + return false; + } + if (occurred != nullptr) { + *occurred = m_fuzzed_data_provider.ConsumeBool() ? requested : 0; + } + return true; +} + +bool FuzzedSock::WaitMany(std::chrono::milliseconds timeout, EventsPerSock& events_per_sock) const +{ + for (auto& [sock, events] : events_per_sock) { + (void)sock; + events.occurred = m_fuzzed_data_provider.ConsumeBool() ? events.requested : 0; + } + return true; +} + +bool FuzzedSock::IsConnected(std::string& errmsg) const +{ + if (m_fuzzed_data_provider.ConsumeBool()) { + return true; + } + errmsg = "disconnected at random by the fuzzer"; + return false; +} + +void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, CNode& node) noexcept +{ + connman.Handshake(node, + /*successfully_connected=*/fuzzed_data_provider.ConsumeBool(), + /*remote_services=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), + /*local_services=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), + /*version=*/fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()), + /*relay_txs=*/fuzzed_data_provider.ConsumeBool()); +} diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h index d81adab650..74afbe1cd9 100644 --- a/src/test/fuzz/util/net.h +++ b/src/test/fuzz/util/net.h @@ -5,10 +5,137 @@ #ifndef BITCOIN_TEST_FUZZ_UTIL_NET_H #define BITCOIN_TEST_FUZZ_UTIL_NET_H +#include <net.h> +#include <net_permissions.h> #include <netaddress.h> +#include <node/connection_types.h> +#include <node/eviction.h> +#include <protocol.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/util.h> +#include <test/util/net.h> +#include <threadsafety.h> +#include <util/sock.h> -class FuzzedDataProvider; +#include <chrono> +#include <cstdint> +#include <limits> +#include <memory> +#include <optional> +#include <string> CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept; +class FuzzedSock : public Sock +{ + FuzzedDataProvider& m_fuzzed_data_provider; + + /** + * Data to return when `MSG_PEEK` is used as a `Recv()` flag. + * If `MSG_PEEK` is used, then our `Recv()` returns some random data as usual, but on the next + * `Recv()` call we must return the same data, thus we remember it here. + */ + mutable std::optional<uint8_t> m_peek_data; + + /** + * Whether to pretend that the socket is select(2)-able. This is randomly set in the + * constructor. It should remain constant so that repeated calls to `IsSelectable()` + * return the same value. + */ + const bool m_selectable; + +public: + explicit FuzzedSock(FuzzedDataProvider& fuzzed_data_provider); + + ~FuzzedSock() override; + + FuzzedSock& operator=(Sock&& other) override; + + ssize_t Send(const void* data, size_t len, int flags) const override; + + ssize_t Recv(void* buf, size_t len, int flags) const override; + + int Connect(const sockaddr*, socklen_t) const override; + + int Bind(const sockaddr*, socklen_t) const override; + + int Listen(int backlog) const override; + + std::unique_ptr<Sock> Accept(sockaddr* addr, socklen_t* addr_len) const override; + + int GetSockOpt(int level, int opt_name, void* opt_val, socklen_t* opt_len) const override; + + int SetSockOpt(int level, int opt_name, const void* opt_val, socklen_t opt_len) const override; + + int GetSockName(sockaddr* name, socklen_t* name_len) const override; + + bool SetNonBlocking() const override; + + bool IsSelectable() const override; + + bool Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred = nullptr) const override; + + bool WaitMany(std::chrono::milliseconds timeout, EventsPerSock& events_per_sock) const override; + + bool IsConnected(std::string& errmsg) const override; +}; + +[[nodiscard]] inline FuzzedSock ConsumeSock(FuzzedDataProvider& fuzzed_data_provider) +{ + return FuzzedSock{fuzzed_data_provider}; +} + +inline CSubNet ConsumeSubNet(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + return {ConsumeNetAddr(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<uint8_t>()}; +} + +inline CService ConsumeService(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + return {ConsumeNetAddr(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<uint16_t>()}; +} + +CAddress ConsumeAddress(FuzzedDataProvider& fuzzed_data_provider) noexcept; + +template <bool ReturnUniquePtr = false> +auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<NodeId>& node_id_in = std::nullopt) noexcept +{ + const NodeId node_id = node_id_in.value_or(fuzzed_data_provider.ConsumeIntegralInRange<NodeId>(0, std::numeric_limits<NodeId>::max())); + const auto sock = std::make_shared<FuzzedSock>(fuzzed_data_provider); + const CAddress address = ConsumeAddress(fuzzed_data_provider); + const uint64_t keyed_net_group = fuzzed_data_provider.ConsumeIntegral<uint64_t>(); + const uint64_t local_host_nonce = fuzzed_data_provider.ConsumeIntegral<uint64_t>(); + const CAddress addr_bind = ConsumeAddress(fuzzed_data_provider); + const std::string addr_name = fuzzed_data_provider.ConsumeRandomLengthString(64); + const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray(ALL_CONNECTION_TYPES); + const bool inbound_onion{conn_type == ConnectionType::INBOUND ? fuzzed_data_provider.ConsumeBool() : false}; + NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS); + if constexpr (ReturnUniquePtr) { + return std::make_unique<CNode>(node_id, + sock, + address, + keyed_net_group, + local_host_nonce, + addr_bind, + addr_name, + conn_type, + inbound_onion, + CNodeOptions{ .permission_flags = permission_flags }); + } else { + return CNode{node_id, + sock, + address, + keyed_net_group, + local_host_nonce, + addr_bind, + addr_name, + conn_type, + inbound_onion, + CNodeOptions{ .permission_flags = permission_flags }}; + } +} +inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = std::nullopt) { return ConsumeNode<true>(fdp, node_id_in); } + +void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, CNode& node) noexcept EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex); + #endif // BITCOIN_TEST_FUZZ_UTIL_NET_H diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp index 842daa8bd4..a55b0bbcd0 100644 --- a/src/test/orphanage_tests.cpp +++ b/src/test/orphanage_tests.cpp @@ -20,13 +20,15 @@ BOOST_FIXTURE_TEST_SUITE(orphanage_tests, TestingSetup) class TxOrphanageTest : public TxOrphanage { public: - inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) + inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { + LOCK(m_mutex); return m_orphans.size(); } - CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) + CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { + LOCK(m_mutex); std::map<uint256, OrphanTx>::iterator it; it = m_orphans.lower_bound(InsecureRand256()); if (it == m_orphans.end()) @@ -59,8 +61,6 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) FillableSigningProvider keystore; BOOST_CHECK(keystore.AddKey(key)); - LOCK(g_cs_orphans); - // 50 orphan transactions: for (int i = 0; i < 50; i++) { diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index a52530e179..21ccbe9648 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -17,12 +17,49 @@ #include <boost/test/unit_test.hpp> +static UniValue JSON(std::string_view json) +{ + UniValue value; + BOOST_CHECK(value.read(json.data(), json.size())); + return value; +} + +class HasJSON +{ +public: + explicit HasJSON(std::string json) : m_json(std::move(json)) {} + bool operator()(const UniValue& value) const + { + std::string json{value.write()}; + BOOST_CHECK_EQUAL(json, m_json); + return json == m_json; + }; + +private: + const std::string m_json; +}; + class RPCTestingSetup : public TestingSetup { public: + UniValue TransformParams(const UniValue& params, std::vector<std::string> arg_names) const; UniValue CallRPC(std::string args); }; +UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector<std::string> arg_names) const +{ + UniValue transformed_params; + CRPCTable table; + CRPCCommand command{"category", "method", [&](const JSONRPCRequest& request, UniValue&, bool) -> bool { transformed_params = request.params; return true; }, arg_names, /*unique_id=*/0}; + table.appendCommand("method", &command); + JSONRPCRequest request; + request.strMethod = "method"; + request.params = params; + if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); + table.execute(request); + return transformed_params; +} + UniValue RPCTestingSetup::CallRPC(std::string args) { std::vector<std::string> vArgs{SplitString(args, ' ')}; @@ -45,6 +82,29 @@ UniValue RPCTestingSetup::CallRPC(std::string args) BOOST_FIXTURE_TEST_SUITE(rpc_tests, RPCTestingSetup) +BOOST_AUTO_TEST_CASE(rpc_namedparams) +{ + const std::vector<std::string> arg_names{{"arg1", "arg2", "arg3", "arg4", "arg5"}}; + + // Make sure named arguments are transformed into positional arguments in correct places separated by nulls + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg2": 2, "arg4": 4})"), arg_names).write(), "[null,2,null,4]"); + + // Make sure named and positional arguments can be combined. + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg5": 5, "args": [1, 2], "arg4": 4})"), arg_names).write(), "[1,2,null,4,5]"); + + // Make sure a unknown named argument raises an exception + BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"arg2": 2, "unknown": 6})"), arg_names), UniValue, + HasJSON(R"({"code":-8,"message":"Unknown named parameter unknown"})")); + + // Make sure an overlap between a named argument and positional argument raises an exception + BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"args": [1,2,3], "arg4": 4, "arg2": 2})"), arg_names), UniValue, + HasJSON(R"({"code":-8,"message":"Parameter arg2 specified twice both as positional and named argument"})")); + + // Make sure extra positional arguments can be passed through to the method implemenation, as long as they don't overlap with named arguments. + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"args": [1,2,3,4,5,6,7,8,9,10]})"), arg_names).write(), "[1,2,3,4,5,6,7,8,9,10]"); + BOOST_CHECK_EQUAL(TransformParams(JSON(R"([1,2,3,4,5,6,7,8,9,10])"), arg_names).write(), "[1,2,3,4,5,6,7,8,9,10]"); +} + BOOST_AUTO_TEST_CASE(rpc_rawparams) { // Test raw transaction API argument handling diff --git a/src/test/txreconciliation_tests.cpp b/src/test/txreconciliation_tests.cpp index bd74998002..b018629e76 100644 --- a/src/test/txreconciliation_tests.cpp +++ b/src/test/txreconciliation_tests.cpp @@ -12,56 +12,54 @@ BOOST_FIXTURE_TEST_SUITE(txreconciliation_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(RegisterPeerTest) { - TxReconciliationTracker tracker(1); + TxReconciliationTracker tracker(TXRECONCILIATION_VERSION); const uint64_t salt = 0; // Prepare a peer for reconciliation. tracker.PreRegisterPeer(0); - // Both roles are false, don't register. - BOOST_CHECK(tracker.RegisterPeer(/*peer_id=*/0, /*is_peer_inbound=*/true, - /*is_peer_recon_initiator=*/false, - /*is_peer_recon_responder=*/false, - /*peer_recon_version=*/1, salt) == - ReconciliationRegisterResult::PROTOCOL_VIOLATION); - - // Invalid roles for the given connection direction. - BOOST_CHECK(tracker.RegisterPeer(0, true, false, true, 1, salt) == ReconciliationRegisterResult::PROTOCOL_VIOLATION); - BOOST_CHECK(tracker.RegisterPeer(0, false, true, false, 1, salt) == ReconciliationRegisterResult::PROTOCOL_VIOLATION); - // Invalid version. - BOOST_CHECK(tracker.RegisterPeer(0, true, true, false, 0, salt) == ReconciliationRegisterResult::PROTOCOL_VIOLATION); + BOOST_CHECK_EQUAL(tracker.RegisterPeer(/*peer_id=*/0, /*is_peer_inbound=*/true, + /*peer_recon_version=*/0, salt), + ReconciliationRegisterResult::PROTOCOL_VIOLATION); - // Valid registration. + // Valid registration (inbound and outbound peers). BOOST_REQUIRE(!tracker.IsPeerRegistered(0)); - BOOST_REQUIRE(tracker.RegisterPeer(0, true, true, false, 1, salt) == ReconciliationRegisterResult::SUCCESS); + BOOST_REQUIRE_EQUAL(tracker.RegisterPeer(0, true, 1, salt), ReconciliationRegisterResult::SUCCESS); BOOST_CHECK(tracker.IsPeerRegistered(0)); - - // Reconciliation version is higher than ours, should be able to register. BOOST_REQUIRE(!tracker.IsPeerRegistered(1)); tracker.PreRegisterPeer(1); - BOOST_REQUIRE(tracker.RegisterPeer(1, true, true, false, 2, salt) == ReconciliationRegisterResult::SUCCESS); + BOOST_REQUIRE(tracker.RegisterPeer(1, false, 1, salt) == ReconciliationRegisterResult::SUCCESS); BOOST_CHECK(tracker.IsPeerRegistered(1)); + // Reconciliation version is higher than ours, should be able to register. + BOOST_REQUIRE(!tracker.IsPeerRegistered(2)); + tracker.PreRegisterPeer(2); + BOOST_REQUIRE(tracker.RegisterPeer(2, true, 2, salt) == ReconciliationRegisterResult::SUCCESS); + BOOST_CHECK(tracker.IsPeerRegistered(2)); + + // Try registering for the second time. + BOOST_REQUIRE(tracker.RegisterPeer(1, false, 1, salt) == ReconciliationRegisterResult::ALREADY_REGISTERED); + // Do not register if there were no pre-registration for the peer. - BOOST_REQUIRE(tracker.RegisterPeer(100, true, true, false, 1, salt) == ReconciliationRegisterResult::NOT_FOUND); + BOOST_REQUIRE_EQUAL(tracker.RegisterPeer(100, true, 1, salt), ReconciliationRegisterResult::NOT_FOUND); BOOST_CHECK(!tracker.IsPeerRegistered(100)); } BOOST_AUTO_TEST_CASE(ForgetPeerTest) { - TxReconciliationTracker tracker(1); + TxReconciliationTracker tracker(TXRECONCILIATION_VERSION); NodeId peer_id0 = 0; // Removing peer after pre-registring works and does not let to register the peer. tracker.PreRegisterPeer(peer_id0); tracker.ForgetPeer(peer_id0); - BOOST_CHECK(tracker.RegisterPeer(peer_id0, true, true, false, 1, 1) == ReconciliationRegisterResult::NOT_FOUND); + BOOST_CHECK_EQUAL(tracker.RegisterPeer(peer_id0, true, 1, 1), ReconciliationRegisterResult::NOT_FOUND); // Removing peer after it is registered works. tracker.PreRegisterPeer(peer_id0); BOOST_REQUIRE(!tracker.IsPeerRegistered(peer_id0)); - BOOST_REQUIRE(tracker.RegisterPeer(peer_id0, true, true, false, 1, 1) == ReconciliationRegisterResult::SUCCESS); + BOOST_REQUIRE_EQUAL(tracker.RegisterPeer(peer_id0, true, 1, 1), ReconciliationRegisterResult::SUCCESS); BOOST_CHECK(tracker.IsPeerRegistered(peer_id0)); tracker.ForgetPeer(peer_id0); BOOST_CHECK(!tracker.IsPeerRegistered(peer_id0)); @@ -69,14 +67,14 @@ BOOST_AUTO_TEST_CASE(ForgetPeerTest) BOOST_AUTO_TEST_CASE(IsPeerRegisteredTest) { - TxReconciliationTracker tracker(1); + TxReconciliationTracker tracker(TXRECONCILIATION_VERSION); NodeId peer_id0 = 0; BOOST_REQUIRE(!tracker.IsPeerRegistered(peer_id0)); tracker.PreRegisterPeer(peer_id0); BOOST_REQUIRE(!tracker.IsPeerRegistered(peer_id0)); - BOOST_REQUIRE(tracker.RegisterPeer(peer_id0, true, true, false, 1, 1) == ReconciliationRegisterResult::SUCCESS); + BOOST_REQUIRE_EQUAL(tracker.RegisterPeer(peer_id0, true, 1, 1), ReconciliationRegisterResult::SUCCESS); BOOST_CHECK(tracker.IsPeerRegistered(peer_id0)); tracker.ForgetPeer(peer_id0); diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp deleted file mode 100644 index 2dadffafb4..0000000000 --- a/src/test/util/wallet.cpp +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) 2019-2021 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include <test/util/wallet.h> - -#include <key_io.h> -#include <outputtype.h> -#include <script/standard.h> -#ifdef ENABLE_WALLET -#include <util/check.h> -#include <util/translation.h> -#include <wallet/wallet.h> -#endif - -using wallet::CWallet; - -const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj"; - -#ifdef ENABLE_WALLET -std::string getnewaddress(CWallet& w) -{ - constexpr auto output_type = OutputType::BECH32; - return EncodeDestination(getNewDestination(w, output_type)); -} - -CTxDestination getNewDestination(CWallet& w, OutputType output_type) -{ - return *Assert(w.GetNewDestination(output_type, "")); -} - -#endif // ENABLE_WALLET diff --git a/src/test/util/wallet.h b/src/test/util/wallet.h deleted file mode 100644 index d8f1db3fd7..0000000000 --- a/src/test/util/wallet.h +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) 2019 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#ifndef BITCOIN_TEST_UTIL_WALLET_H -#define BITCOIN_TEST_UTIL_WALLET_H - -#include <outputtype.h> -#include <string> - -namespace wallet { -class CWallet; -} // namespace wallet - -// Constants // - -extern const std::string ADDRESS_BCRT1_UNSPENDABLE; - -// RPC-like // - -/** Import the address to the wallet */ -void importaddress(wallet::CWallet& wallet, const std::string& address); -/** Returns a new encoded destination from the wallet (hardcoded to BECH32) */ -std::string getnewaddress(wallet::CWallet& w); -/** Returns a new destination, of an specific type, from the wallet */ -CTxDestination getNewDestination(wallet::CWallet& w, OutputType output_type); - - -#endif // BITCOIN_TEST_UTIL_WALLET_H diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 69ae8ea582..b0b71e135c 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -15,11 +15,10 @@ static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; /** Minimum time between orphan transactions expire time checks in seconds */ static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; -RecursiveMutex g_cs_orphans; bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) { - AssertLockHeld(g_cs_orphans); + LOCK(m_mutex); const uint256& hash = tx->GetHash(); if (m_orphans.count(hash)) @@ -55,7 +54,13 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) int TxOrphanage::EraseTx(const uint256& txid) { - AssertLockHeld(g_cs_orphans); + LOCK(m_mutex); + return _EraseTx(txid); +} + +int TxOrphanage::_EraseTx(const uint256& txid) +{ + AssertLockHeld(m_mutex); std::map<uint256, OrphanTx>::iterator it = m_orphans.find(txid); if (it == m_orphans.end()) return 0; @@ -87,7 +92,9 @@ int TxOrphanage::EraseTx(const uint256& txid) void TxOrphanage::EraseForPeer(NodeId peer) { - AssertLockHeld(g_cs_orphans); + LOCK(m_mutex); + + m_peer_work_set.erase(peer); int nErased = 0; std::map<uint256, OrphanTx>::iterator iter = m_orphans.begin(); @@ -96,7 +103,7 @@ void TxOrphanage::EraseForPeer(NodeId peer) std::map<uint256, OrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid if (maybeErase->second.fromPeer == peer) { - nErased += EraseTx(maybeErase->second.tx->GetHash()); + nErased += _EraseTx(maybeErase->second.tx->GetHash()); } } if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx from peer=%d\n", nErased, peer); @@ -104,7 +111,7 @@ void TxOrphanage::EraseForPeer(NodeId peer) void TxOrphanage::LimitOrphans(unsigned int max_orphans) { - AssertLockHeld(g_cs_orphans); + LOCK(m_mutex); unsigned int nEvicted = 0; static int64_t nNextSweep; @@ -118,7 +125,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans) { std::map<uint256, OrphanTx>::iterator maybeErase = iter++; if (maybeErase->second.nTimeExpire <= nNow) { - nErased += EraseTx(maybeErase->second.tx->GetHash()); + nErased += _EraseTx(maybeErase->second.tx->GetHash()); } else { nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime); } @@ -132,15 +139,19 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans) { // Evict a random orphan: size_t randompos = rng.randrange(m_orphan_list.size()); - EraseTx(m_orphan_list[randompos]->first); + _EraseTx(m_orphan_list[randompos]->first); ++nEvicted; } if (nEvicted > 0) LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted); } -void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) const +void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) { - AssertLockHeld(g_cs_orphans); + LOCK(m_mutex); + + // Get this peer's work set, emplacing an empty set it didn't exist + std::set<uint256>& orphan_work_set = m_peer_work_set.try_emplace(peer).first->second; + for (unsigned int i = 0; i < tx.vout.size(); i++) { const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(tx.GetHash(), i)); if (it_by_prev != m_outpoint_to_orphan_it.end()) { @@ -153,7 +164,7 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256> bool TxOrphanage::HaveTx(const GenTxid& gtxid) const { - LOCK(g_cs_orphans); + LOCK(m_mutex); if (gtxid.IsWtxid()) { return m_wtxid_to_orphan_it.count(gtxid.GetHash()); } else { @@ -161,18 +172,32 @@ bool TxOrphanage::HaveTx(const GenTxid& gtxid) const } } -std::pair<CTransactionRef, NodeId> TxOrphanage::GetTx(const uint256& txid) const +CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer, NodeId& originator, bool& more) { - AssertLockHeld(g_cs_orphans); - - const auto it = m_orphans.find(txid); - if (it == m_orphans.end()) return {nullptr, -1}; - return {it->second.tx, it->second.fromPeer}; + LOCK(m_mutex); + + auto work_set_it = m_peer_work_set.find(peer); + if (work_set_it != m_peer_work_set.end()) { + auto& work_set = work_set_it->second; + while (!work_set.empty()) { + uint256 txid = *work_set.begin(); + work_set.erase(work_set.begin()); + + const auto orphan_it = m_orphans.find(txid); + if (orphan_it != m_orphans.end()) { + more = !work_set.empty(); + originator = orphan_it->second.fromPeer; + return orphan_it->second.tx; + } + } + } + more = false; + return nullptr; } void TxOrphanage::EraseForBlock(const CBlock& block) { - LOCK(g_cs_orphans); + LOCK(m_mutex); std::vector<uint256> vOrphanErase; @@ -195,7 +220,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block) if (vOrphanErase.size()) { int nErased = 0; for (const uint256& orphanHash : vOrphanErase) { - nErased += EraseTx(orphanHash); + nErased += _EraseTx(orphanHash); } LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx included or conflicted by block\n", nErased); } diff --git a/src/txorphanage.h b/src/txorphanage.h index 9363e6f733..551502d325 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -10,8 +10,8 @@ #include <primitives/transaction.h> #include <sync.h> -/** Guards orphan transactions and extra txs for compact blocks */ -extern RecursiveMutex g_cs_orphans; +#include <map> +#include <set> /** A class to track orphan transactions (failed on TX_MISSING_INPUTS) * Since we cannot distinguish orphans from bad transactions with @@ -21,40 +21,46 @@ extern RecursiveMutex g_cs_orphans; class TxOrphanage { public: /** Add a new orphan transaction */ - bool AddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + bool AddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Check if we already have an orphan transaction (by txid or wtxid) */ - bool HaveTx(const GenTxid& gtxid) const LOCKS_EXCLUDED(::g_cs_orphans); - - /** Get an orphan transaction and its originating peer - * (Transaction ref will be nullptr if not found) + bool HaveTx(const GenTxid& gtxid) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + + /** Extract a transaction from a peer's work set + * Returns nullptr and sets more to false if there are no transactions + * to work on. Otherwise returns the transaction reference, removes + * the transaction from the work set, and populates its arguments with + * the originating peer, and whether there are more orphans for this peer + * to work on after this tx. */ - std::pair<CTransactionRef, NodeId> GetTx(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + CTransactionRef GetTxToReconsider(NodeId peer, NodeId& originator, bool& more) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Erase an orphan by txid */ - int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Erase all orphans announced by a peer (eg, after that peer disconnects) */ - void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Erase all orphans included in or invalidated by a new block */ - void EraseForBlock(const CBlock& block) LOCKS_EXCLUDED(::g_cs_orphans); + void EraseForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Limit the orphanage to the given maximum */ - void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); - /** Add any orphans that list a particular tx as a parent into a peer's work set - * (ie orphans that may have found their final missing parent, and so should be reconsidered for the mempool) */ - void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + /** Add any orphans that list a particular tx as a parent into a peer's work set */ + void AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Return how many entries exist in the orphange */ - size_t Size() LOCKS_EXCLUDED(::g_cs_orphans) + size_t Size() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { - LOCK(::g_cs_orphans); + LOCK(m_mutex); return m_orphans.size(); } protected: + /** Guards orphan transactions */ + mutable Mutex m_mutex; + struct OrphanTx { CTransactionRef tx; NodeId fromPeer; @@ -64,7 +70,10 @@ protected: /** Map from txid to orphan transaction record. Limited by * -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */ - std::map<uint256, OrphanTx> m_orphans GUARDED_BY(g_cs_orphans); + std::map<uint256, OrphanTx> m_orphans GUARDED_BY(m_mutex); + + /** Which peer provided a parent tx of orphans that need to be reconsidered */ + std::map<NodeId, std::set<uint256>> m_peer_work_set GUARDED_BY(m_mutex); using OrphanMap = decltype(m_orphans); @@ -79,14 +88,17 @@ protected: /** Index from the parents' COutPoint into the m_orphans. Used * to remove orphan transactions from the m_orphans */ - std::map<COutPoint, std::set<OrphanMap::iterator, IteratorComparator>> m_outpoint_to_orphan_it GUARDED_BY(g_cs_orphans); + std::map<COutPoint, std::set<OrphanMap::iterator, IteratorComparator>> m_outpoint_to_orphan_it GUARDED_BY(m_mutex); /** Orphan transactions in vector for quick random eviction */ - std::vector<OrphanMap::iterator> m_orphan_list GUARDED_BY(g_cs_orphans); + std::vector<OrphanMap::iterator> m_orphan_list GUARDED_BY(m_mutex); /** Index from wtxid into the m_orphans to lookup orphan * transactions using their witness ids. */ - std::map<uint256, OrphanMap::iterator> m_wtxid_to_orphan_it GUARDED_BY(g_cs_orphans); + std::map<uint256, OrphanMap::iterator> m_wtxid_to_orphan_it GUARDED_BY(m_mutex); + + /** Erase an orphan by txid */ + int _EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex); }; #endif // BITCOIN_TXORPHANAGE_H diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 675c4a759d..971814e9cd 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -730,7 +730,9 @@ static RPCHelpMan migratewallet() std::shared_ptr<CWallet> wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; - EnsureWalletIsUnlocked(*wallet); + if (wallet->IsCrypted()) { + throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: migratewallet on encrypted wallets is currently unsupported."); + } WalletContext& context = EnsureWalletContext(request.context); diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp index 68146eb079..2c83d25c20 100644 --- a/src/wallet/test/ismine_tests.cpp +++ b/src/wallet/test/ismine_tests.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <key.h> +#include <key_io.h> #include <node/context.h> #include <script/script.h> #include <script/standard.h> @@ -16,6 +17,25 @@ namespace wallet { BOOST_FIXTURE_TEST_SUITE(ismine_tests, BasicTestingSetup) +wallet::ScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, const bool success) +{ + keystore.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + + FlatSigningProvider keys; + std::string error; + std::unique_ptr<Descriptor> parsed_desc = Parse(desc_str, keys, error, false); + BOOST_CHECK(success == (parsed_desc != nullptr)); + if (!success) return nullptr; + + const int64_t range_start = 0, range_end = 1, next_index = 0, timestamp = 1; + + WalletDescriptor w_desc(std::move(parsed_desc), timestamp, range_start, range_end, next_index); + + LOCK(keystore.cs_wallet); + + return Assert(keystore.AddWalletDescriptor(w_desc, keys,/*label=*/"", /*internal=*/false)); +}; + BOOST_AUTO_TEST_CASE(ismine_standard) { CKey keys[2]; @@ -33,7 +53,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) CScript scriptPubKey; isminetype result; - // P2PK compressed + // P2PK compressed - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -52,7 +72,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } - // P2PK uncompressed + // P2PK compressed - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "pk(" + EncodeSecret(keys[0]) + ")"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + scriptPubKey = GetScriptForRawPubKey(pubkeys[0]); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + } + + // P2PK uncompressed - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -71,7 +103,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } - // P2PKH compressed + // P2PK uncompressed - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "pk(" + EncodeSecret(uncompressedKey) + ")"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + } + + // P2PKH compressed - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -90,7 +134,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } - // P2PKH uncompressed + // P2PKH compressed - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "pkh(" + EncodeSecret(keys[0]) + ")"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + scriptPubKey = GetScriptForDestination(PKHash(pubkeys[0])); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + } + + // P2PKH uncompressed - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -109,7 +165,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } - // P2SH + // P2PKH uncompressed - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "pkh(" + EncodeSecret(uncompressedKey) + ")"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + scriptPubKey = GetScriptForDestination(PKHash(uncompressedPubkey)); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + } + + // P2SH - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -136,7 +204,20 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } - // (P2PKH inside) P2SH inside P2SH (invalid) + // P2SH - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "sh(pkh(" + EncodeSecret(keys[0]) + "))"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + CScript redeemScript = GetScriptForDestination(PKHash(pubkeys[0])); + scriptPubKey = GetScriptForDestination(ScriptHash(redeemScript)); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + } + + // (P2PKH inside) P2SH inside P2SH (invalid) - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -155,7 +236,16 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } - // (P2PKH inside) P2SH inside P2WSH (invalid) + // (P2PKH inside) P2SH inside P2SH (invalid) - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "sh(sh(" + EncodeSecret(keys[0]) + "))"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, false); + BOOST_CHECK_EQUAL(spk_manager, nullptr); + } + + // (P2PKH inside) P2SH inside P2WSH (invalid) - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -174,7 +264,16 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } - // P2WPKH inside P2WSH (invalid) + // (P2PKH inside) P2SH inside P2WSH (invalid) - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "wsh(sh(" + EncodeSecret(keys[0]) + "))"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, false); + BOOST_CHECK_EQUAL(spk_manager, nullptr); + } + + // P2WPKH inside P2WSH (invalid) - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -191,7 +290,16 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } - // (P2PKH inside) P2WSH inside P2WSH (invalid) + // P2WPKH inside P2WSH (invalid) - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "wsh(wpkh(" + EncodeSecret(keys[0]) + "))"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, false); + BOOST_CHECK_EQUAL(spk_manager, nullptr); + } + + // (P2PKH inside) P2WSH inside P2WSH (invalid) - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -210,7 +318,16 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } - // P2WPKH compressed + // (P2PKH inside) P2WSH inside P2WSH (invalid) - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "wsh(wsh(" + EncodeSecret(keys[0]) + "))"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, false); + BOOST_CHECK_EQUAL(spk_manager, nullptr); + } + + // P2WPKH compressed - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -226,7 +343,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } - // P2WPKH uncompressed + // P2WPKH compressed - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "wpkh(" + EncodeSecret(keys[0]) + ")"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(pubkeys[0])); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + } + + // P2WPKH uncompressed - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -247,7 +376,16 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } - // scriptPubKey multisig + // P2WPKH uncompressed (invalid) - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "wpkh(" + EncodeSecret(uncompressedKey) + ")"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, false); + BOOST_CHECK_EQUAL(spk_manager, nullptr); + } + + // scriptPubKey multisig - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -282,7 +420,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } - // P2SH multisig + // scriptPubKey multisig - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + std::string desc_str = "multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + ")"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + scriptPubKey = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]}); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + } + + // P2SH multisig - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -305,7 +455,21 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } - // P2WSH multisig with compressed keys + // P2SH multisig - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + + std::string desc_str = "sh(multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + "))"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + CScript redeemScript = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]}); + scriptPubKey = GetScriptForDestination(ScriptHash(redeemScript)); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + } + + // P2WSH multisig with compressed keys - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -334,7 +498,21 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } - // P2WSH multisig with uncompressed key + // P2WSH multisig with compressed keys - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + + std::string desc_str = "wsh(multi(2, " + EncodeSecret(keys[0]) + ", " + EncodeSecret(keys[1]) + "))"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + CScript redeemScript = GetScriptForMultisig(2, {pubkeys[0], pubkeys[1]}); + scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(redeemScript)); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + } + + // P2WSH multisig with uncompressed key - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -363,7 +541,17 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } - // P2WSH multisig wrapped in P2SH + // P2WSH multisig with uncompressed key (invalid) - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + + std::string desc_str = "wsh(multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + "))"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, false); + BOOST_CHECK_EQUAL(spk_manager, nullptr); + } + + // P2WSH multisig wrapped in P2SH - Legacy { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); @@ -393,6 +581,83 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } + // P2WSH multisig wrapped in P2SH - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + + std::string desc_str = "sh(wsh(multi(2, " + EncodeSecret(keys[0]) + ", " + EncodeSecret(keys[1]) + ")))"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + CScript witnessScript = GetScriptForMultisig(2, {pubkeys[0], pubkeys[1]}); + CScript redeemScript = GetScriptForDestination(WitnessV0ScriptHash(witnessScript)); + scriptPubKey = GetScriptForDestination(ScriptHash(redeemScript)); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + } + + // Combo - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + + std::string desc_str = "combo(" + EncodeSecret(keys[0]) + ")"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + // Test P2PK + result = spk_manager->IsMine(GetScriptForRawPubKey(pubkeys[0])); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + + // Test P2PKH + result = spk_manager->IsMine(GetScriptForDestination(PKHash(pubkeys[0]))); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + + // Test P2SH (combo descriptor does not describe P2SH) + CScript redeemScript = GetScriptForDestination(PKHash(pubkeys[0])); + scriptPubKey = GetScriptForDestination(ScriptHash(redeemScript)); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_NO); + + // Test P2WPKH + scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(pubkeys[0])); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + + // P2SH-P2WPKH output + redeemScript = GetScriptForDestination(WitnessV0KeyHash(pubkeys[0])); + scriptPubKey = GetScriptForDestination(ScriptHash(redeemScript)); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + + // Test P2TR (combo descriptor does not describe P2TR) + XOnlyPubKey xpk(pubkeys[0]); + Assert(xpk.IsFullyValid()); + TaprootBuilder builder; + builder.Finalize(xpk); + WitnessV1Taproot output = builder.GetOutput(); + scriptPubKey = GetScriptForDestination(output); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_NO); + } + + // Taproot - Descriptor + { + CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + + std::string desc_str = "tr(" + EncodeSecret(keys[0]) + ")"; + + auto spk_manager = CreateDescriptor(keystore, desc_str, true); + + XOnlyPubKey xpk(pubkeys[0]); + Assert(xpk.IsFullyValid()); + TaprootBuilder builder; + builder.Finalize(xpk); + WitnessV1Taproot output = builder.GetOutput(); + scriptPubKey = GetScriptForDestination(output); + result = spk_manager->IsMine(scriptPubKey); + BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + } + // OP_RETURN { CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index ab72721f9d..f6c7ecb598 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -11,8 +11,6 @@ #include <wallet/wallet.h> #include <wallet/walletdb.h> -#include <boost/test/unit_test.hpp> - #include <memory> namespace wallet { @@ -39,10 +37,46 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cc WalletRescanReserver reserver(*wallet); reserver.reserve(); CWallet::ScanResult result = wallet->ScanForWalletTransactions(cchain.Genesis()->GetBlockHash(), /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); - BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); - BOOST_CHECK_EQUAL(result.last_scanned_block, cchain.Tip()->GetBlockHash()); - BOOST_CHECK_EQUAL(*result.last_scanned_height, cchain.Height()); - BOOST_CHECK(result.last_failed_block.IsNull()); + assert(result.status == CWallet::ScanResult::SUCCESS); + assert(result.last_scanned_block == cchain.Tip()->GetBlockHash()); + assert(*result.last_scanned_height == cchain.Height()); + assert(result.last_failed_block.IsNull()); return wallet; } + +std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options) +{ + auto new_database = CreateMockWalletDatabase(options); + + // Get a cursor to the original database + auto batch = database.MakeBatch(); + batch->StartCursor(); + + // Get a batch for the new database + auto new_batch = new_database->MakeBatch(); + + // Read all records from the original database and write them to the new one + while (true) { + CDataStream key(SER_DISK, CLIENT_VERSION); + CDataStream value(SER_DISK, CLIENT_VERSION); + bool complete; + batch->ReadAtCursor(key, value, complete); + if (complete) break; + new_batch->Write(key, value); + } + + return new_database; +} + +std::string getnewaddress(CWallet& w) +{ + constexpr auto output_type = OutputType::BECH32; + return EncodeDestination(getNewDestination(w, output_type)); +} + +CTxDestination getNewDestination(CWallet& w, OutputType output_type) +{ + return *Assert(w.GetNewDestination(output_type, "")); +} + } // namespace wallet diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index 712d0251cd..a2e361ddca 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -5,19 +5,32 @@ #ifndef BITCOIN_WALLET_TEST_UTIL_H #define BITCOIN_WALLET_TEST_UTIL_H +#include <script/standard.h> #include <memory> class ArgsManager; class CChain; class CKey; +enum class OutputType; namespace interfaces { class Chain; } // namespace interfaces namespace wallet { class CWallet; +struct DatabaseOptions; +class WalletDatabase; std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, ArgsManager& args, const CKey& key); + +// Creates a copy of the provided database +std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options); + +/** Returns a new encoded destination from the wallet (hardcoded to BECH32) */ +std::string getnewaddress(CWallet& w); +/** Returns a new destination, of an specific type, from the wallet */ +CTxDestination getNewDestination(CWallet& w, OutputType output_type); + } // namespace wallet #endif // BITCOIN_WALLET_TEST_UTIL_H diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index f45b69a418..24d21c2f22 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://www.opensource.org/licenses/mit-license.php. +#include <wallet/test/util.h> #include <wallet/wallet.h> #include <test/util/setup_common.h> @@ -50,5 +51,139 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_unknown_descriptor, TestingSetup) } } +bool HasAnyRecordOfType(WalletDatabase& db, const std::string& key) +{ + std::unique_ptr<DatabaseBatch> batch = db.MakeBatch(false); + BOOST_CHECK(batch->StartCursor()); + while (true) { + CDataStream ssKey(SER_DISK, CLIENT_VERSION); + CDataStream ssValue(SER_DISK, CLIENT_VERSION); + bool complete; + BOOST_CHECK(batch->ReadAtCursor(ssKey, ssValue, complete)); + if (complete) break; + std::string type; + ssKey >> type; + if (type == key) return true; + } + return false; +} + +BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup) +{ + // The test duplicates the db so each case has its own db instance. + int NUMBER_OF_TESTS = 4; + std::vector<std::unique_ptr<WalletDatabase>> dbs; + CKey first_key; + auto get_db = [](std::vector<std::unique_ptr<WalletDatabase>>& dbs) { + std::unique_ptr<WalletDatabase> db = std::move(dbs.back()); + dbs.pop_back(); + return db; + }; + + { // Context setup. + // Create and encrypt legacy wallet + std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, CreateMockWalletDatabase())); + LOCK(wallet->cs_wallet); + auto legacy_spkm = wallet->GetOrCreateLegacyScriptPubKeyMan(); + BOOST_CHECK(legacy_spkm->SetupGeneration(true)); + + // Get the first key in the wallet + CTxDestination dest = *Assert(legacy_spkm->GetNewDestination(OutputType::LEGACY)); + CKeyID key_id = GetKeyForDestination(*legacy_spkm, dest); + BOOST_CHECK(legacy_spkm->GetKey(key_id, first_key)); + + // Encrypt the wallet and duplicate database + BOOST_CHECK(wallet->EncryptWallet("encrypt")); + wallet->Flush(); + + DatabaseOptions options; + for (int i=0; i < NUMBER_OF_TESTS; i++) { + dbs.emplace_back(DuplicateMockDatabase(wallet->GetDatabase(), options)); + } + } + + { + // First test case: + // Erase all the crypted keys from db and unlock the wallet. + // The wallet will only re-write the crypted keys to db if any checksum is missing at load time. + // So, if any 'ckey' record re-appears on db, then the checksums were not properly calculated, and we are re-writing + // the records every time that 'CWallet::Unlock' gets called, which is not good. + + // Load the wallet and check that is encrypted + std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, get_db(dbs))); + BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::LOAD_OK); + BOOST_CHECK(wallet->IsCrypted()); + BOOST_CHECK(HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY)); + + // Now delete all records and check that the 'Unlock' function doesn't re-write them + BOOST_CHECK(wallet->GetLegacyScriptPubKeyMan()->DeleteRecords()); + BOOST_CHECK(!HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY)); + BOOST_CHECK(wallet->Unlock("encrypt")); + BOOST_CHECK(!HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY)); + } + + { + // Second test case: + // Verify that loading up a 'ckey' with no checksum triggers a complete re-write of the crypted keys. + std::unique_ptr<WalletDatabase> db = get_db(dbs); + { + std::unique_ptr<DatabaseBatch> batch = db->MakeBatch(false); + std::pair<std::vector<unsigned char>, uint256> value; + BOOST_CHECK(batch->Read(std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey()), value)); + + const auto key = std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey()); + BOOST_CHECK(batch->Write(key, value.first, /*fOverwrite=*/true)); + } + + // Load the wallet and check that is encrypted + std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, std::move(db))); + BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::LOAD_OK); + BOOST_CHECK(wallet->IsCrypted()); + BOOST_CHECK(HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY)); + + // Now delete all ckey records and check that the 'Unlock' function re-writes them + // (this is because the wallet, at load time, found a ckey record with no checksum) + BOOST_CHECK(wallet->GetLegacyScriptPubKeyMan()->DeleteRecords()); + BOOST_CHECK(!HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY)); + BOOST_CHECK(wallet->Unlock("encrypt")); + BOOST_CHECK(HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY)); + } + + { + // Third test case: + // Verify that loading up a 'ckey' with an invalid checksum throws an error. + std::unique_ptr<WalletDatabase> db = get_db(dbs); + { + std::unique_ptr<DatabaseBatch> batch = db->MakeBatch(false); + std::vector<unsigned char> crypted_data; + BOOST_CHECK(batch->Read(std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey()), crypted_data)); + + // Write an invalid checksum + std::pair<std::vector<unsigned char>, uint256> value = std::make_pair(crypted_data, uint256::ONE); + const auto key = std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey()); + BOOST_CHECK(batch->Write(key, value, /*fOverwrite=*/true)); + } + + std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, std::move(db))); + BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::CORRUPT); + } + + { + // Fourth test case: + // Verify that loading up a 'ckey' with an invalid pubkey throws an error + std::unique_ptr<WalletDatabase> db = get_db(dbs); + { + CPubKey invalid_key; + BOOST_ASSERT(!invalid_key.IsValid()); + const auto key = std::make_pair(DBKeys::CRYPTED_KEY, invalid_key); + std::pair<std::vector<unsigned char>, uint256> value; + BOOST_CHECK(db->MakeBatch(false)->Write(key, value, /*fOverwrite=*/true)); + } + + std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, std::move(db))); + BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::CORRUPT); + } +} + BOOST_AUTO_TEST_SUITE_END() } // namespace wallet diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 431e970edc..0f297b44ab 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4182,8 +4182,8 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet> // Make list of wallets to cleanup std::vector<std::shared_ptr<CWallet>> created_wallets; - created_wallets.push_back(std::move(res.watchonly_wallet)); - created_wallets.push_back(std::move(res.solvables_wallet)); + if (res.watchonly_wallet) created_wallets.push_back(std::move(res.watchonly_wallet)); + if (res.solvables_wallet) created_wallets.push_back(std::move(res.solvables_wallet)); // Get the directories to remove after unloading for (std::shared_ptr<CWallet>& w : created_wallets) { diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 6a8f0d2481..9d72323f46 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -482,7 +482,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, if (!ssValue.eof()) { uint256 checksum; ssValue >> checksum; - if ((checksum_valid = Hash(vchPrivKey) != checksum)) { + if (!(checksum_valid = Hash(vchPrivKey) == checksum)) { strErr = "Error reading wallet database: Encrypted key corrupt"; return false; } diff --git a/test/functional/example_test.py b/test/functional/example_test.py index 9cf756060e..3ea7614661 100755 --- a/test/functional/example_test.py +++ b/test/functional/example_test.py @@ -79,6 +79,9 @@ class ExampleTest(BitcoinTestFramework): # Override the set_test_params(), skip_test_if_missing_module(), add_options(), setup_chain(), setup_network() # and setup_nodes() methods to customize the test setup as required. + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): """Override test parameters for your individual test. diff --git a/test/functional/feature_backwards_compatibility.py b/test/functional/feature_backwards_compatibility.py index 59a12193fd..5fe4a95f6f 100755 --- a/test/functional/feature_backwards_compatibility.py +++ b/test/functional/feature_backwards_compatibility.py @@ -32,6 +32,9 @@ from test_framework.util import ( class BackwardsCompatibilityTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 10 diff --git a/test/functional/feature_bip68_sequence.py b/test/functional/feature_bip68_sequence.py index 5b43fe4f8e..f8854d0e8d 100755 --- a/test/functional/feature_bip68_sequence.py +++ b/test/functional/feature_bip68_sequence.py @@ -44,6 +44,9 @@ SEQUENCE_LOCKTIME_MASK = 0x0000ffff NOT_FINAL_ERROR = "non-BIP68-final" class BIP68Test(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 self.extra_args = [ diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index 9d32749a08..44725ad85e 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -8,6 +8,7 @@ Test that the CHECKLOCKTIMEVERIFY soft-fork activates. """ from test_framework.blocktools import ( + TIME_GENESIS_BLOCK, create_block, create_coinbase, ) @@ -61,7 +62,7 @@ def cltv_invalidate(tx, failure_reason): # +-------------------------------------------------+------------+--------------+ [[OP_CHECKLOCKTIMEVERIFY], None, None], [[OP_1NEGATE, OP_CHECKLOCKTIMEVERIFY, OP_DROP], None, None], - [[CScriptNum(100), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0, 1296688602], # timestamp of genesis block + [[CScriptNum(100), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0, TIME_GENESIS_BLOCK], [[CScriptNum(100), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0, 50], [[CScriptNum(50), OP_CHECKLOCKTIMEVERIFY, OP_DROP], SEQUENCE_FINAL, 50], ][failure_reason] diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 112dbb9e6a..fbaf93bac4 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -12,6 +12,9 @@ from test_framework import util class ConfArgsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py index 945ece6a33..025fe38d30 100755 --- a/test/functional/feature_filelock.py +++ b/test/functional/feature_filelock.py @@ -11,6 +11,9 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.test_node import ErrorMatch class FilelockTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index 56d093c396..cf626bc7c6 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -17,6 +17,9 @@ class InitStressTest(BitcoinTestFramework): subsequent starts. """ + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = False self.num_nodes = 1 diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index e038afa1ad..8e821295b8 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -24,6 +24,9 @@ def notify_outputname(walletname, txid): class NotificationsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py index 7dbeccbc09..58bc6ca67c 100755 --- a/test/functional/feature_pruning.py +++ b/test/functional/feature_pruning.py @@ -76,6 +76,9 @@ def calc_usage(blockdir): return sum(os.path.getsize(blockdir + f) for f in os.listdir(blockdir) if os.path.isfile(os.path.join(blockdir, f))) / (1024. * 1024.) class PruneTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 6 diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index 7603248ae5..1d46959a4d 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -21,6 +21,9 @@ from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE MAX_REPLACEMENT_LIMIT = 100 class ReplaceByFeeTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 self.extra_args = [ diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index 7f2a615be1..34eca32c11 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -78,6 +78,9 @@ txs_mined = {} # txindex from txid to blockhash class SegWitTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 diff --git a/test/functional/feature_taproot.py b/test/functional/feature_taproot.py index 31a6b31225..dbe51f8f54 100755 --- a/test/functional/feature_taproot.py +++ b/test/functional/feature_taproot.py @@ -1274,6 +1274,7 @@ UTXOData = namedtuple('UTXOData', 'outpoint,output,spender') class TaprootTest(BitcoinTestFramework): def add_options(self, parser): + self.add_wallet_options(parser) parser.add_argument("--dumptests", dest="dump_tests", default=False, action="store_true", help="Dump generated test cases to directory set by TEST_DUMP_DIR environment variable") diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index db5564ac50..b1369c2615 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -66,11 +66,12 @@ def cli_get_info_string_to_dict(cli_get_info_string): class TestBitcoinCli(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - if self.is_specified_wallet_compiled(): - self.requires_wallet = True def skip_test_if_missing_module(self): self.skip_if_no_cli() @@ -84,6 +85,11 @@ class TestBitcoinCli(BitcoinTestFramework): rpc_response = self.nodes[0].getblockchaininfo() assert_equal(cli_response, rpc_response) + self.log.info("Test named arguments") + assert_equal(self.nodes[0].cli.echo(0, 1, arg3=3, arg5=5), ['0', '1', None, '3', None, '5']) + assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", self.nodes[0].cli.echo, 0, 1, arg1=1) + assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", self.nodes[0].cli.echo, 0, None, 2, arg1=1) + user, password = get_auth_cookie(self.nodes[0].datadir, self.chain) self.log.info("Test -stdinrpcpass option") @@ -114,6 +120,7 @@ class TestBitcoinCli(BitcoinTestFramework): self.log.info("Test -getinfo returns expected network and blockchain info") if self.is_specified_wallet_compiled(): + self.import_deterministic_coinbase_privkeys() self.nodes[0].encryptwallet(password) cli_get_info_string = self.nodes[0].cli('-getinfo').send_cli() cli_get_info = cli_get_info_string_to_dict(cli_get_info_string) diff --git a/test/functional/interface_usdt_coinselection.py b/test/functional/interface_usdt_coinselection.py index ef32feda99..a3c830bb51 100755 --- a/test/functional/interface_usdt_coinselection.py +++ b/test/functional/interface_usdt_coinselection.py @@ -97,6 +97,9 @@ int trace_aps_create_tx(struct pt_regs *ctx) { class CoinSelectionTracepointTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True diff --git a/test/functional/mempool_compatibility.py b/test/functional/mempool_compatibility.py index c545a7f68d..7f03a215b2 100755 --- a/test/functional/mempool_compatibility.py +++ b/test/functional/mempool_compatibility.py @@ -21,6 +21,9 @@ from test_framework.wallet import ( class MempoolCompatibilityTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 self.wallet_names = [None] diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index def0b1fce4..a8ebcd875b 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -28,6 +28,9 @@ assert CUSTOM_DESCENDANT_LIMIT >= CUSTOM_ANCESTOR_LIMIT class MempoolPackagesTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 self.extra_args = [ diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index b6fa7fbd91..1c06426256 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -50,6 +50,9 @@ from test_framework.wallet import MiniWallet class MempoolPersistTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + def set_test_params(self): self.num_nodes = 3 self.extra_args = [[], ["-persistmempool=0"], []] diff --git a/test/functional/mempool_unbroadcast.py b/test/functional/mempool_unbroadcast.py index 7587adc257..5487ca40c1 100755 --- a/test/functional/mempool_unbroadcast.py +++ b/test/functional/mempool_unbroadcast.py @@ -15,10 +15,11 @@ from test_framework.wallet import MiniWallet MAX_INITIAL_BROADCAST_DELAY = 15 * 60 # 15 minutes in seconds class MempoolUnbroadcastTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 - if self.is_wallet_compiled(): - self.requires_wallet = True def run_test(self): self.wallet = MiniWallet(self.nodes[0]) @@ -35,6 +36,7 @@ class MempoolUnbroadcastTest(BitcoinTestFramework): self.log.info("Generate transactions that only node 0 knows about") if self.is_wallet_compiled(): + self.import_deterministic_coinbase_privkeys() # generate a wallet txn addr = node.getnewaddress() wallet_tx_hsh = node.sendtoaddress(addr, 0.0001) diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py index fed9832a7d..0e349ef70c 100755 --- a/test/functional/p2p_sendtxrcncl.py +++ b/test/functional/p2p_sendtxrcncl.py @@ -10,6 +10,7 @@ from test_framework.messages import ( msg_verack, msg_version, msg_wtxidrelay, + NODE_BLOOM, ) from test_framework.p2p import ( P2PInterface, @@ -54,10 +55,8 @@ class PeerTrackMsgOrder(P2PInterface): super().on_message(message) self.messages.append(message) -def create_sendtxrcncl_msg(initiator=True): +def create_sendtxrcncl_msg(): sendtxrcncl_msg = msg_sendtxrcncl() - sendtxrcncl_msg.initiator = initiator - sendtxrcncl_msg.responder = not initiator sendtxrcncl_msg.version = 1 sendtxrcncl_msg.salt = 2 return sendtxrcncl_msg @@ -68,11 +67,11 @@ class SendTxRcnclTest(BitcoinTestFramework): self.extra_args = [['-txreconciliation']] def run_test(self): + # Check everything concerning *sending* SENDTXRCNCL + # First, *sending* to *inbound*. self.log.info('SENDTXRCNCL sent to an inbound') peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) assert peer.sendtxrcncl_msg_received - assert not peer.sendtxrcncl_msg_received.initiator - assert peer.sendtxrcncl_msg_received.responder assert_equal(peer.sendtxrcncl_msg_received.version, 1) self.nodes[0].disconnect_p2ps() @@ -81,7 +80,7 @@ class SendTxRcnclTest(BitcoinTestFramework): peer.wait_for_verack() verack_index = [i for i, msg in enumerate(peer.messages) if msg.msgtype == b'verack'][0] sendtxrcncl_index = [i for i, msg in enumerate(peer.messages) if msg.msgtype == b'sendtxrcncl'][0] - assert(sendtxrcncl_index < verack_index) + assert sendtxrcncl_index < verack_index self.nodes[0].disconnect_p2ps() self.log.info('SENDTXRCNCL on pre-WTXID version should not be sent') @@ -108,30 +107,77 @@ class SendTxRcnclTest(BitcoinTestFramework): assert not peer.sendtxrcncl_msg_received self.nodes[0].disconnect_p2ps() + self.log.info('SENDTXRCNCL for fRelay=false should not be sent (with NODE_BLOOM offered)') + self.restart_node(0, ["-peerbloomfilters", "-txreconciliation"]) + peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=False, wait_for_verack=False) + no_txrelay_version_msg = msg_version() + no_txrelay_version_msg.nVersion = P2P_VERSION + no_txrelay_version_msg.strSubVer = P2P_SUBVERSION + no_txrelay_version_msg.nServices = P2P_SERVICES + no_txrelay_version_msg.relay = 0 + peer.send_message(no_txrelay_version_msg) + peer.wait_for_verack() + assert peer.nServices & NODE_BLOOM != 0 + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + # Now, *sending* to *outbound*. + self.log.info('SENDTXRCNCL sent to an outbound') + peer = self.nodes[0].add_outbound_p2p_connection( + SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="outbound-full-relay") + assert peer.sendtxrcncl_msg_received + assert_equal(peer.sendtxrcncl_msg_received.version, 1) + self.nodes[0].disconnect_p2ps() + + self.log.info('SENDTXRCNCL should not be sent if block-relay-only') + peer = self.nodes[0].add_outbound_p2p_connection( + SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="block-relay-only") + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + self.log.info("SENDTXRCNCL should not be sent if feeler") + peer = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=0, connection_type="feeler") + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + self.log.info("SENDTXRCNCL should not be sent if addrfetch") + peer = self.nodes[0].add_outbound_p2p_connection( + SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="addr-fetch") + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + self.log.info('SENDTXRCNCL not sent if -txreconciliation flag is not set') + self.restart_node(0, []) + peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + self.log.info('SENDTXRCNCL not sent if blocksonly is set') + self.restart_node(0, ["-txreconciliation", "-blocksonly"]) + peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + # Check everything concerning *receiving* SENDTXRCNCL + # First, receiving from *inbound*. + self.restart_node(0, ["-txreconciliation"]) self.log.info('valid SENDTXRCNCL received') peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) - peer.send_message(create_sendtxrcncl_msg()) - self.wait_until(lambda : "sendtxrcncl" in self.nodes[0].getpeerinfo()[-1]["bytesrecv_per_msg"]) + with self.nodes[0].assert_debug_log(["received: sendtxrcncl"]): + peer.send_message(create_sendtxrcncl_msg()) self.log.info('second SENDTXRCNCL triggers a disconnect') with self.nodes[0].assert_debug_log(["(sendtxrcncl received from already registered peer); disconnecting"]): peer.send_message(create_sendtxrcncl_msg()) peer.wait_for_disconnect() - self.log.info('SENDTXRCNCL with initiator=responder=0 triggers a disconnect') - sendtxrcncl_no_role = create_sendtxrcncl_msg() - sendtxrcncl_no_role.initiator = False - sendtxrcncl_no_role.responder = False + self.restart_node(0, []) + self.log.info('SENDTXRCNCL if no txreconciliation supported is ignored') peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) - with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]): - peer.send_message(sendtxrcncl_no_role) - peer.wait_for_disconnect() + with self.nodes[0].assert_debug_log(['ignored, as our node does not have txreconciliation enabled']): + peer.send_message(create_sendtxrcncl_msg()) + self.nodes[0].disconnect_p2ps() - self.log.info('SENDTXRCNCL with initiator=0 and responder=1 from inbound triggers a disconnect') - sendtxrcncl_wrong_role = create_sendtxrcncl_msg(initiator=False) - peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) - with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]): - peer.send_message(sendtxrcncl_wrong_role) - peer.wait_for_disconnect() + self.restart_node(0, ["-txreconciliation"]) self.log.info('SENDTXRCNCL with version=0 triggers a disconnect') sendtxrcncl_low_version = create_sendtxrcncl_msg() @@ -141,6 +187,26 @@ class SendTxRcnclTest(BitcoinTestFramework): peer.send_message(sendtxrcncl_low_version) peer.wait_for_disconnect() + self.log.info('SENDTXRCNCL with version=2 is valid') + sendtxrcncl_higher_version = create_sendtxrcncl_msg() + sendtxrcncl_higher_version.version = 2 + peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) + with self.nodes[0].assert_debug_log(['Register peer=1']): + peer.send_message(sendtxrcncl_higher_version) + self.nodes[0].disconnect_p2ps() + + self.log.info('unexpected SENDTXRCNCL is ignored') + peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=False, wait_for_verack=False) + old_version_msg = msg_version() + old_version_msg.nVersion = 70015 + old_version_msg.strSubVer = P2P_SUBVERSION + old_version_msg.nServices = P2P_SERVICES + old_version_msg.relay = 1 + peer.send_message(old_version_msg) + with self.nodes[0].assert_debug_log(['Ignore unexpected txreconciliation signal']): + peer.send_message(create_sendtxrcncl_msg()) + self.nodes[0].disconnect_p2ps() + self.log.info('sending SENDTXRCNCL after sending VERACK triggers a disconnect') peer = self.nodes[0].add_p2p_connection(P2PInterface()) with self.nodes[0].assert_debug_log(["sendtxrcncl received after verack"]): @@ -154,53 +220,14 @@ class SendTxRcnclTest(BitcoinTestFramework): peer.send_message(msg_verack()) self.nodes[0].disconnect_p2ps() - self.log.info('SENDTXRCNCL sent to an outbound') - peer = self.nodes[0].add_outbound_p2p_connection( - SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="outbound-full-relay") - assert peer.sendtxrcncl_msg_received - assert peer.sendtxrcncl_msg_received.initiator - assert not peer.sendtxrcncl_msg_received.responder - assert_equal(peer.sendtxrcncl_msg_received.version, 1) - self.nodes[0].disconnect_p2ps() - - self.log.info('SENDTXRCNCL should not be sent if block-relay-only') - peer = self.nodes[0].add_outbound_p2p_connection( - SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="block-relay-only") - assert not peer.sendtxrcncl_msg_received - self.nodes[0].disconnect_p2ps() - - self.log.info("SENDTXRCNCL should not be sent if feeler") - peer = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=0, connection_type="feeler") - assert not peer.sendtxrcncl_msg_received - self.nodes[0].disconnect_p2ps() - + # Now, *receiving* from *outbound*. self.log.info('SENDTXRCNCL if block-relay-only triggers a disconnect') peer = self.nodes[0].add_outbound_p2p_connection( PeerNoVerack(), wait_for_verack=False, p2p_idx=0, connection_type="block-relay-only") with self.nodes[0].assert_debug_log(["we indicated no tx relay; disconnecting"]): - peer.send_message(create_sendtxrcncl_msg(initiator=False)) - peer.wait_for_disconnect() - - self.log.info('SENDTXRCNCL with initiator=1 and responder=0 from outbound triggers a disconnect') - sendtxrcncl_wrong_role = create_sendtxrcncl_msg(initiator=True) - peer = self.nodes[0].add_outbound_p2p_connection( - PeerNoVerack(), wait_for_verack=False, p2p_idx=0, connection_type="outbound-full-relay") - with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]): - peer.send_message(sendtxrcncl_wrong_role) + peer.send_message(create_sendtxrcncl_msg()) peer.wait_for_disconnect() - self.log.info('SENDTXRCNCL not sent if -txreconciliation flag is not set') - self.restart_node(0, []) - peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) - assert not peer.sendtxrcncl_msg_received - self.nodes[0].disconnect_p2ps() - - self.log.info('SENDTXRCNCL not sent if blocksonly is set') - self.restart_node(0, ["-txreconciliation", "-blocksonly"]) - peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) - assert not peer.sendtxrcncl_msg_received - self.nodes[0].disconnect_p2ps() - if __name__ == '__main__': SendTxRcnclTest().main() diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py index 1ad3fc24c9..5b6eb2d22c 100755 --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -24,12 +24,13 @@ from test_framework.wallet import ( ) class RpcCreateMultiSigTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 self.supports_cli = False - if self.is_bdb_compiled(): - self.requires_wallet = True def get_keys(self): self.pub = [] @@ -50,6 +51,7 @@ class RpcCreateMultiSigTest(BitcoinTestFramework): self.wallet = MiniWallet(test_node=node0) if self.is_bdb_compiled(): + self.import_deterministic_coinbase_privkeys() self.check_addmultisigaddress_errors() self.log.info('Generating blocks ...') diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 1152995ac9..0f77d6ad30 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -35,6 +35,9 @@ def get_unspent(listunspent, amount): raise AssertionError('Could not find unspent with amount={}'.format(amount)) class RawTransactionsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = True diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py index f683577c47..a2de8e3ef5 100755 --- a/test/functional/rpc_help.py +++ b/test/functional/rpc_help.py @@ -43,6 +43,9 @@ def process_mapping(fname): class HelpRpcTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 1 self.supports_cli = False diff --git a/test/functional/rpc_invalid_address_message.py b/test/functional/rpc_invalid_address_message.py index fcc49d0a75..1694020663 100755 --- a/test/functional/rpc_invalid_address_message.py +++ b/test/functional/rpc_invalid_address_message.py @@ -39,6 +39,9 @@ INVALID_ADDRESS = 'asfah14i8fajz0123f' INVALID_ADDRESS_2 = '1q049ldschfnwystcqnsvyfpj23mpsg3jcedq9xv' class InvalidAddressErrorMessageTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 diff --git a/test/functional/rpc_named_arguments.py b/test/functional/rpc_named_arguments.py index 41b9312969..cc3ee9efd5 100755 --- a/test/functional/rpc_named_arguments.py +++ b/test/functional/rpc_named_arguments.py @@ -30,6 +30,9 @@ class NamedArgumentTest(BitcoinTestFramework): assert_equal(node.echo(arg1=1), [None, 1]) assert_equal(node.echo(arg9=None), [None]*10) assert_equal(node.echo(arg0=0,arg3=3,arg9=9), [0] + [None]*2 + [3] + [None]*5 + [9]) + assert_equal(node.echo(0, 1, arg3=3, arg5=5), [0, 1, None, 3, None, 5]) + assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", node.echo, 0, 1, arg1=1) + assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", node.echo, 0, None, 2, arg1=1) if __name__ == '__main__': NamedArgumentTest().main() diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index 9a563cbf5f..a512a6b675 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -7,31 +7,24 @@ from decimal import Decimal import random -from test_framework.address import ADDRESS_BCRT1_P2WSH_OP_TRUE -from test_framework.test_framework import BitcoinTestFramework +from test_framework.blocktools import COINBASE_MATURITY from test_framework.messages import ( MAX_BIP125_RBF_SEQUENCE, - COIN, - CTxInWitness, tx_from_hex, ) from test_framework.p2p import P2PTxInvStore -from test_framework.script import ( - CScript, - OP_TRUE, -) +from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_fee_amount, assert_raises_rpc_error, ) from test_framework.wallet import ( - create_child_with_parents, - create_raw_chain, DEFAULT_FEE, - make_chain, + MiniWallet, ) + class RPCPackagesTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 @@ -49,38 +42,38 @@ class RPCPackagesTest(BitcoinTestFramework): assert_equal(shuffled_testres, self.nodes[0].testmempoolaccept(shuffled_package)) def run_test(self): - self.log.info("Generate blocks to create UTXOs") node = self.nodes[0] - self.privkeys = [node.get_deterministic_priv_key().key] - self.address = node.get_deterministic_priv_key().address - self.coins = [] - # The last 100 coinbase transactions are premature - for b in self.generatetoaddress(node, 220, self.address)[:-100]: - coinbase = node.getblock(blockhash=b, verbosity=2)["tx"][0] - self.coins.append({ + + # get an UTXO that requires signature to be spent + deterministic_address = node.get_deterministic_priv_key().address + blockhash = self.generatetoaddress(node, 1, deterministic_address)[0] + coinbase = node.getblock(blockhash=blockhash, verbosity=2)["tx"][0] + coin = { "txid": coinbase["txid"], "amount": coinbase["vout"][0]["value"], "scriptPubKey": coinbase["vout"][0]["scriptPubKey"], - }) + "vout": 0, + "height": 0 + } + self.wallet = MiniWallet(self.nodes[0]) + self.generate(self.wallet, COINBASE_MATURITY + 100) # blocks generated for inputs + + self.log.info("Create some transactions") # Create some transactions that can be reused throughout the test. Never submit these to mempool. self.independent_txns_hex = [] self.independent_txns_testres = [] for _ in range(3): - coin = self.coins.pop() - rawtx = node.createrawtransaction([{"txid": coin["txid"], "vout": 0}], - {self.address : coin["amount"] - Decimal("0.0001")}) - signedtx = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=self.privkeys) - assert signedtx["complete"] - testres = node.testmempoolaccept([signedtx["hex"]]) + tx_hex = self.wallet.create_self_transfer(fee_rate=Decimal("0.0001"))["hex"] + testres = self.nodes[0].testmempoolaccept([tx_hex]) assert testres[0]["allowed"] - self.independent_txns_hex.append(signedtx["hex"]) + self.independent_txns_hex.append(tx_hex) # testmempoolaccept returns a list of length one, avoid creating a 2D list self.independent_txns_testres.append(testres[0]) self.independent_txns_testres_blank = [{ "txid": res["txid"], "wtxid": res["wtxid"]} for res in self.independent_txns_testres] - self.test_independent() + self.test_independent(coin) self.test_chain() self.test_multiple_children() self.test_multiple_parents() @@ -88,14 +81,15 @@ class RPCPackagesTest(BitcoinTestFramework): self.test_rbf() self.test_submitpackage() - def test_independent(self): + def test_independent(self, coin): self.log.info("Test multiple independent transactions in a package") node = self.nodes[0] # For independent transactions, order doesn't matter. self.assert_testres_equal(self.independent_txns_hex, self.independent_txns_testres) self.log.info("Test an otherwise valid package with an extra garbage tx appended") - garbage_tx = node.createrawtransaction([{"txid": "00" * 32, "vout": 5}], {self.address: 1}) + address = node.get_deterministic_priv_key().address + garbage_tx = node.createrawtransaction([{"txid": "00" * 32, "vout": 5}], {address: 1}) tx = tx_from_hex(garbage_tx) # Only the txid and wtxids are returned because validation is incomplete for the independent txns. # Package validation is atomic: if the node cannot find a UTXO for any single tx in the package, @@ -105,9 +99,8 @@ class RPCPackagesTest(BitcoinTestFramework): self.assert_testres_equal(package_bad, testres_bad) self.log.info("Check testmempoolaccept tells us when some transactions completed validation successfully") - coin = self.coins.pop() tx_bad_sig_hex = node.createrawtransaction([{"txid": coin["txid"], "vout": 0}], - {self.address : coin["amount"] - Decimal("0.0001")}) + {address : coin["amount"] - Decimal("0.0001")}) tx_bad_sig = tx_from_hex(tx_bad_sig_hex) testres_bad_sig = node.testmempoolaccept(self.independent_txns_hex + [tx_bad_sig_hex]) # By the time the signature for the last transaction is checked, all the other transactions @@ -120,23 +113,22 @@ class RPCPackagesTest(BitcoinTestFramework): }]) self.log.info("Check testmempoolaccept reports txns in packages that exceed max feerate") - coin = self.coins.pop() - tx_high_fee_raw = node.createrawtransaction([{"txid": coin["txid"], "vout": 0}], - {self.address : coin["amount"] - Decimal("0.999")}) - tx_high_fee_signed = node.signrawtransactionwithkey(hexstring=tx_high_fee_raw, privkeys=self.privkeys) - assert tx_high_fee_signed["complete"] - tx_high_fee = tx_from_hex(tx_high_fee_signed["hex"]) - testres_high_fee = node.testmempoolaccept([tx_high_fee_signed["hex"]]) + tx_high_fee = self.wallet.create_self_transfer(fee=Decimal("0.999")) + testres_high_fee = node.testmempoolaccept([tx_high_fee["hex"]]) assert_equal(testres_high_fee, [ - {"txid": tx_high_fee.rehash(), "wtxid": tx_high_fee.getwtxid(), "allowed": False, "reject-reason": "max-fee-exceeded"} + {"txid": tx_high_fee["txid"], "wtxid": tx_high_fee["wtxid"], "allowed": False, "reject-reason": "max-fee-exceeded"} ]) - package_high_fee = [tx_high_fee_signed["hex"]] + self.independent_txns_hex + package_high_fee = [tx_high_fee["hex"]] + self.independent_txns_hex testres_package_high_fee = node.testmempoolaccept(package_high_fee) assert_equal(testres_package_high_fee, testres_high_fee + self.independent_txns_testres_blank) def test_chain(self): node = self.nodes[0] - (chain_hex, chain_txns) = create_raw_chain(node, self.coins.pop(), self.address, self.privkeys) + + chain = self.wallet.create_self_transfer_chain(chain_length=25) + chain_hex = chain["chain_hex"] + chain_txns = chain["chain_txns"] + self.log.info("Check that testmempoolaccept requires packages to be sorted by dependency") assert_equal(node.testmempoolaccept(rawtxs=chain_hex[::-1]), [{"txid": tx.rehash(), "wtxid": tx.getwtxid(), "package-error": "package-not-sorted"} for tx in chain_txns[::-1]]) @@ -158,78 +150,57 @@ class RPCPackagesTest(BitcoinTestFramework): def test_multiple_children(self): node = self.nodes[0] - self.log.info("Testmempoolaccept a package in which a transaction has two children within the package") - first_coin = self.coins.pop() - value = (first_coin["amount"] - Decimal("0.0002")) / 2 # Deduct reasonable fee and make 2 outputs - inputs = [{"txid": first_coin["txid"], "vout": 0}] - outputs = [{self.address : value}, {ADDRESS_BCRT1_P2WSH_OP_TRUE : value}] - rawtx = node.createrawtransaction(inputs, outputs) - - parent_signed = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=self.privkeys) - assert parent_signed["complete"] - parent_tx = tx_from_hex(parent_signed["hex"]) - parent_txid = parent_tx.rehash() - assert node.testmempoolaccept([parent_signed["hex"]])[0]["allowed"] - parent_locking_script_a = parent_tx.vout[0].scriptPubKey.hex() - child_value = value - Decimal("0.0001") + parent_tx = self.wallet.create_self_transfer_multi(num_outputs=2) + assert node.testmempoolaccept([parent_tx["hex"]])[0]["allowed"] # Child A - (_, tx_child_a_hex, _, _) = make_chain(node, self.address, self.privkeys, parent_txid, child_value, 0, parent_locking_script_a) - assert not node.testmempoolaccept([tx_child_a_hex])[0]["allowed"] + child_a_tx = self.wallet.create_self_transfer(utxo_to_spend=parent_tx["new_utxos"][0]) + assert not node.testmempoolaccept([child_a_tx["hex"]])[0]["allowed"] # Child B - rawtx_b = node.createrawtransaction([{"txid": parent_txid, "vout": 1}], {self.address : child_value}) - tx_child_b = tx_from_hex(rawtx_b) - tx_child_b.wit.vtxinwit = [CTxInWitness()] - tx_child_b.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE])] - tx_child_b_hex = tx_child_b.serialize().hex() - assert not node.testmempoolaccept([tx_child_b_hex])[0]["allowed"] + child_b_tx = self.wallet.create_self_transfer(utxo_to_spend=parent_tx["new_utxos"][1]) + assert not node.testmempoolaccept([child_b_tx["hex"]])[0]["allowed"] self.log.info("Testmempoolaccept with entire package, should work with children in either order") - testres_multiple_ab = node.testmempoolaccept(rawtxs=[parent_signed["hex"], tx_child_a_hex, tx_child_b_hex]) - testres_multiple_ba = node.testmempoolaccept(rawtxs=[parent_signed["hex"], tx_child_b_hex, tx_child_a_hex]) + testres_multiple_ab = node.testmempoolaccept(rawtxs=[parent_tx["hex"], child_a_tx["hex"], child_b_tx["hex"]]) + testres_multiple_ba = node.testmempoolaccept(rawtxs=[parent_tx["hex"], child_b_tx["hex"], child_a_tx["hex"]]) assert all([testres["allowed"] for testres in testres_multiple_ab + testres_multiple_ba]) testres_single = [] # Test accept and then submit each one individually, which should be identical to package testaccept - for rawtx in [parent_signed["hex"], tx_child_a_hex, tx_child_b_hex]: + for rawtx in [parent_tx["hex"], child_a_tx["hex"], child_b_tx["hex"]]: testres = node.testmempoolaccept([rawtx]) testres_single.append(testres[0]) # Submit the transaction now so its child should have no problem validating node.sendrawtransaction(rawtx) assert_equal(testres_single, testres_multiple_ab) - def test_multiple_parents(self): node = self.nodes[0] - self.log.info("Testmempoolaccept a package in which a transaction has multiple parents within the package") + for num_parents in [2, 10, 24]: # Test a package with num_parents parents and 1 child transaction. + parent_coins = [] package_hex = [] - parents_tx = [] - values = [] - parent_locking_scripts = [] + for _ in range(num_parents): - parent_coin = self.coins.pop() - value = parent_coin["amount"] - (tx, txhex, value, parent_locking_script) = make_chain(node, self.address, self.privkeys, parent_coin["txid"], value) - package_hex.append(txhex) - parents_tx.append(tx) - values.append(value) - parent_locking_scripts.append(parent_locking_script) - child_hex = create_child_with_parents(node, self.address, self.privkeys, parents_tx, values, parent_locking_scripts) - # Package accept should work with the parents in any order (as long as parents come before child) + # Package accept should work with the parents in any order (as long as parents come before child) + parent_tx = self.wallet.create_self_transfer() + parent_coins.append(parent_tx["new_utxo"]) + package_hex.append(parent_tx["hex"]) + + child_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=parent_coins, fee_per_output=2000) for _ in range(10): random.shuffle(package_hex) - testres_multiple = node.testmempoolaccept(rawtxs=package_hex + [child_hex]) + testres_multiple = node.testmempoolaccept(rawtxs=package_hex + [child_tx['hex']]) assert all([testres["allowed"] for testres in testres_multiple]) testres_single = [] # Test accept and then submit each one individually, which should be identical to package testaccept - for rawtx in package_hex + [child_hex]: + for rawtx in package_hex + [child_tx["hex"]]: testres_single.append(node.testmempoolaccept([rawtx])[0]) # Submit the transaction now so its child should have no problem validating node.sendrawtransaction(rawtx) @@ -237,77 +208,60 @@ class RPCPackagesTest(BitcoinTestFramework): def test_conflicting(self): node = self.nodes[0] - prevtx = self.coins.pop() - inputs = [{"txid": prevtx["txid"], "vout": 0}] - output1 = {node.get_deterministic_priv_key().address: 50 - 0.00125} - output2 = {ADDRESS_BCRT1_P2WSH_OP_TRUE: 50 - 0.00125} + coin = self.wallet.get_utxo() # tx1 and tx2 share the same inputs - rawtx1 = node.createrawtransaction(inputs, output1) - rawtx2 = node.createrawtransaction(inputs, output2) - signedtx1 = node.signrawtransactionwithkey(hexstring=rawtx1, privkeys=self.privkeys) - signedtx2 = node.signrawtransactionwithkey(hexstring=rawtx2, privkeys=self.privkeys) - tx1 = tx_from_hex(signedtx1["hex"]) - tx2 = tx_from_hex(signedtx2["hex"]) - assert signedtx1["complete"] - assert signedtx2["complete"] + tx1 = self.wallet.create_self_transfer(utxo_to_spend=coin) + tx2 = self.wallet.create_self_transfer(utxo_to_spend=coin) # Ensure tx1 and tx2 are valid by themselves - assert node.testmempoolaccept([signedtx1["hex"]])[0]["allowed"] - assert node.testmempoolaccept([signedtx2["hex"]])[0]["allowed"] + assert node.testmempoolaccept([tx1["hex"]])[0]["allowed"] + assert node.testmempoolaccept([tx2["hex"]])[0]["allowed"] self.log.info("Test duplicate transactions in the same package") - testres = node.testmempoolaccept([signedtx1["hex"], signedtx1["hex"]]) + testres = node.testmempoolaccept([tx1["hex"], tx1["hex"]]) assert_equal(testres, [ - {"txid": tx1.rehash(), "wtxid": tx1.getwtxid(), "package-error": "conflict-in-package"}, - {"txid": tx1.rehash(), "wtxid": tx1.getwtxid(), "package-error": "conflict-in-package"} + {"txid": tx1["txid"], "wtxid": tx1["wtxid"], "package-error": "conflict-in-package"}, + {"txid": tx1["txid"], "wtxid": tx1["wtxid"], "package-error": "conflict-in-package"} ]) self.log.info("Test conflicting transactions in the same package") - testres = node.testmempoolaccept([signedtx1["hex"], signedtx2["hex"]]) + testres = node.testmempoolaccept([tx1["hex"], tx2["hex"]]) assert_equal(testres, [ - {"txid": tx1.rehash(), "wtxid": tx1.getwtxid(), "package-error": "conflict-in-package"}, - {"txid": tx2.rehash(), "wtxid": tx2.getwtxid(), "package-error": "conflict-in-package"} + {"txid": tx1["txid"], "wtxid": tx1["wtxid"], "package-error": "conflict-in-package"}, + {"txid": tx2["txid"], "wtxid": tx2["wtxid"], "package-error": "conflict-in-package"} ]) def test_rbf(self): node = self.nodes[0] - coin = self.coins.pop() - inputs = [{"txid": coin["txid"], "vout": 0, "sequence": MAX_BIP125_RBF_SEQUENCE}] - fee = Decimal('0.00125000') - output = {node.get_deterministic_priv_key().address: 50 - fee} - raw_replaceable_tx = node.createrawtransaction(inputs, output) - signed_replaceable_tx = node.signrawtransactionwithkey(hexstring=raw_replaceable_tx, privkeys=self.privkeys) - testres_replaceable = node.testmempoolaccept([signed_replaceable_tx["hex"]]) - replaceable_tx = tx_from_hex(signed_replaceable_tx["hex"]) + + coin = self.wallet.get_utxo() + fee = Decimal("0.00125000") + replaceable_tx = self.wallet.create_self_transfer(utxo_to_spend=coin, sequence=MAX_BIP125_RBF_SEQUENCE, fee = fee) + testres_replaceable = node.testmempoolaccept([replaceable_tx["hex"]]) assert_equal(testres_replaceable, [ - {"txid": replaceable_tx.rehash(), "wtxid": replaceable_tx.getwtxid(), - "allowed": True, "vsize": replaceable_tx.get_vsize(), "fees": { "base": fee }} + {"txid": replaceable_tx["txid"], "wtxid": replaceable_tx["wtxid"], + "allowed": True, "vsize": replaceable_tx["tx"].get_vsize(), "fees": { "base": fee }} ]) # Replacement transaction is identical except has double the fee - replacement_tx = tx_from_hex(signed_replaceable_tx["hex"]) - replacement_tx.vout[0].nValue -= int(fee * COIN) # Doubled fee - signed_replacement_tx = node.signrawtransactionwithkey(replacement_tx.serialize().hex(), self.privkeys) - replacement_tx = tx_from_hex(signed_replacement_tx["hex"]) - - self.log.info("Test that transactions within a package cannot replace each other") - testres_rbf_conflicting = node.testmempoolaccept([signed_replaceable_tx["hex"], signed_replacement_tx["hex"]]) + replacement_tx = self.wallet.create_self_transfer(utxo_to_spend=coin, sequence=MAX_BIP125_RBF_SEQUENCE, fee = 2 * fee) + testres_rbf_conflicting = node.testmempoolaccept([replaceable_tx["hex"], replacement_tx["hex"]]) assert_equal(testres_rbf_conflicting, [ - {"txid": replaceable_tx.rehash(), "wtxid": replaceable_tx.getwtxid(), "package-error": "conflict-in-package"}, - {"txid": replacement_tx.rehash(), "wtxid": replacement_tx.getwtxid(), "package-error": "conflict-in-package"} + {"txid": replaceable_tx["txid"], "wtxid": replaceable_tx["wtxid"], "package-error": "conflict-in-package"}, + {"txid": replacement_tx["txid"], "wtxid": replacement_tx["wtxid"], "package-error": "conflict-in-package"} ]) self.log.info("Test that packages cannot conflict with mempool transactions, even if a valid BIP125 RBF") - node.sendrawtransaction(signed_replaceable_tx["hex"]) - testres_rbf_single = node.testmempoolaccept([signed_replacement_tx["hex"]]) # This transaction is a valid BIP125 replace-by-fee + self.wallet.sendrawtransaction(from_node=node, tx_hex=replaceable_tx["hex"]) + testres_rbf_single = node.testmempoolaccept([replacement_tx["hex"]]) assert testres_rbf_single[0]["allowed"] testres_rbf_package = self.independent_txns_testres_blank + [{ - "txid": replacement_tx.rehash(), "wtxid": replacement_tx.getwtxid(), "allowed": False, + "txid": replacement_tx["txid"], "wtxid": replacement_tx["wtxid"], "allowed": False, "reject-reason": "bip125-replacement-disallowed" }] - self.assert_testres_equal(self.independent_txns_hex + [signed_replacement_tx["hex"]], testres_rbf_package) + self.assert_testres_equal(self.independent_txns_hex + [replacement_tx["hex"]], testres_rbf_package) def assert_equal_package_results(self, node, testmempoolaccept_result, submitpackage_result): """Assert that a successful submitpackage result is consistent with testmempoolaccept @@ -330,36 +284,26 @@ class RPCPackagesTest(BitcoinTestFramework): def test_submit_child_with_parents(self, num_parents, partial_submit): node = self.nodes[0] peer = node.add_p2p_connection(P2PTxInvStore()) - # Test a package with num_parents parents and 1 child transaction. - package_hex = [] + package_txns = [] - values = [] - scripts = [] for _ in range(num_parents): - parent_coin = self.coins.pop() - value = parent_coin["amount"] - (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, parent_coin["txid"], value) - package_hex.append(txhex) - package_txns.append(tx) - values.append(value) - scripts.append(spk) + parent_tx = self.wallet.create_self_transfer(fee=DEFAULT_FEE) + package_txns.append(parent_tx) if partial_submit and random.choice([True, False]): - node.sendrawtransaction(txhex) - child_hex = create_child_with_parents(node, self.address, self.privkeys, package_txns, values, scripts) - package_hex.append(child_hex) - package_txns.append(tx_from_hex(child_hex)) + node.sendrawtransaction(parent_tx["hex"]) + child_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=[tx["new_utxo"] for tx in package_txns], fee_per_output=10000) #DEFAULT_FEE + package_txns.append(child_tx) - testmempoolaccept_result = node.testmempoolaccept(rawtxs=package_hex) - submitpackage_result = node.submitpackage(package=package_hex) + testmempoolaccept_result = node.testmempoolaccept(rawtxs=[tx["hex"] for tx in package_txns]) + submitpackage_result = node.submitpackage(package=[tx["hex"] for tx in package_txns]) # Check that each result is present, with the correct size and fees - for i in range(num_parents + 1): - tx = package_txns[i] - wtxid = tx.getwtxid() - assert wtxid in submitpackage_result["tx-results"] - tx_result = submitpackage_result["tx-results"][wtxid] + for package_txn in package_txns: + tx = package_txn["tx"] + assert tx.getwtxid() in submitpackage_result["tx-results"] + tx_result = submitpackage_result["tx-results"][tx.getwtxid()] assert_equal(tx_result, { - "txid": tx.rehash(), + "txid": package_txn["txid"], "vsize": tx.get_vsize(), "fees": { "base": DEFAULT_FEE, @@ -376,30 +320,25 @@ class RPCPackagesTest(BitcoinTestFramework): assert "package-feerate" not in submitpackage_result # The node should announce each transaction. No guarantees for propagation. - peer.wait_for_broadcast([tx.getwtxid() for tx in package_txns]) + peer.wait_for_broadcast([tx["tx"].getwtxid() for tx in package_txns]) self.generate(node, 1) - def test_submit_cpfp(self): node = self.nodes[0] peer = node.add_p2p_connection(P2PTxInvStore()) - # 2 parent 1 child CPFP. First parent pays high fees, second parent pays 0 fees and is - # fee-bumped by the child. - coin_rich = self.coins.pop() - coin_poor = self.coins.pop() - tx_rich, hex_rich, value_rich, spk_rich = make_chain(node, self.address, self.privkeys, coin_rich["txid"], coin_rich["amount"]) - tx_poor, hex_poor, value_poor, spk_poor = make_chain(node, self.address, self.privkeys, coin_poor["txid"], coin_poor["amount"], fee=0) + tx_poor = self.wallet.create_self_transfer(fee=0, fee_rate=0) + tx_rich = self.wallet.create_self_transfer(fee=DEFAULT_FEE) package_txns = [tx_rich, tx_poor] - hex_child = create_child_with_parents(node, self.address, self.privkeys, package_txns, [value_rich, value_poor], [spk_rich, spk_poor]) - tx_child = tx_from_hex(hex_child) + coins = [tx["new_utxo"] for tx in package_txns] + tx_child = self.wallet.create_self_transfer_multi(utxos_to_spend=coins, fee_per_output=10000) #DEFAULT_FEE package_txns.append(tx_child) - submitpackage_result = node.submitpackage([hex_rich, hex_poor, hex_child]) + submitpackage_result = node.submitpackage([tx["hex"] for tx in package_txns]) - rich_parent_result = submitpackage_result["tx-results"][tx_rich.getwtxid()] - poor_parent_result = submitpackage_result["tx-results"][tx_poor.getwtxid()] - child_result = submitpackage_result["tx-results"][tx_child.getwtxid()] + rich_parent_result = submitpackage_result["tx-results"][tx_rich["wtxid"]] + poor_parent_result = submitpackage_result["tx-results"][tx_poor["wtxid"]] + child_result = submitpackage_result["tx-results"][tx_child["tx"].getwtxid()] assert_equal(rich_parent_result["fees"]["base"], DEFAULT_FEE) assert_equal(poor_parent_result["fees"]["base"], 0) assert_equal(child_result["fees"]["base"], DEFAULT_FEE) @@ -410,10 +349,9 @@ class RPCPackagesTest(BitcoinTestFramework): assert_fee_amount(DEFAULT_FEE, rich_parent_result["vsize"] + child_result["vsize"], submitpackage_result["package-feerate"]) # The node will broadcast each transaction, still abiding by its peer's fee filter - peer.wait_for_broadcast([tx.getwtxid() for tx in package_txns]) + peer.wait_for_broadcast([tx["tx"].getwtxid() for tx in package_txns]) self.generate(node, 1) - def test_submitpackage(self): node = self.nodes[0] @@ -427,7 +365,7 @@ class RPCPackagesTest(BitcoinTestFramework): self.log.info("Submitpackage only allows packages of 1 child with its parents") # Chain of 3 transactions has too many generations - chain_hex, _ = create_raw_chain(node, self.coins.pop(), self.address, self.privkeys, 3) + chain_hex = self.wallet.create_self_transfer_chain(chain_length=25)["chain_hex"] assert_raises_rpc_error(-25, "not-child-with-parents", node.submitpackage, chain_hex) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 8243f43736..f6a697438a 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -47,8 +47,9 @@ import json import os -# Create one-input, one-output, no-fee transaction: class PSBTTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) def set_test_params(self): self.num_nodes = 3 diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 930aaaa897..15fc947eef 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -54,6 +54,9 @@ class multidict(dict): class RawTransactionsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, descriptors=False) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 @@ -65,8 +68,6 @@ class RawTransactionsTest(BitcoinTestFramework): # whitelist all peers to speed up tx relay / mempool sync for args in self.extra_args: args.append("-whitelist=noban@127.0.0.1") - self.requires_wallet = self.is_specified_wallet_compiled() - self.supports_cli = False def setup_network(self): @@ -85,7 +86,8 @@ class RawTransactionsTest(BitcoinTestFramework): self.sendrawtransaction_testmempoolaccept_tests() self.decoderawtransaction_tests() self.transaction_version_number_tests() - if self.requires_wallet and not self.options.descriptors: + if self.is_specified_wallet_compiled() and not self.options.descriptors: + self.import_deterministic_coinbase_privkeys() self.raw_multisig_transaction_legacy_tests() def getrawtransaction_tests(self): diff --git a/test/functional/test_framework/authproxy.py b/test/functional/test_framework/authproxy.py index c4ffd1fbf6..dd20b28550 100644 --- a/test/functional/test_framework/authproxy.py +++ b/test/functional/test_framework/authproxy.py @@ -131,10 +131,12 @@ class AuthServiceProxy(): json.dumps(args or argsn, default=EncodeDecimal, ensure_ascii=self.ensure_ascii), )) if args and argsn: - raise ValueError('Cannot handle both named and positional arguments') + params = dict(args=args, **argsn) + else: + params = args or argsn return {'version': '1.1', 'method': self._service_name, - 'params': args or argsn, + 'params': params, 'id': AuthServiceProxy.__id_count} def __call__(self, *args, **argsn): diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 252b49cc6d..7389b1624e 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1840,29 +1840,23 @@ class msg_cfcheckpt: self.filter_type, self.stop_hash) class msg_sendtxrcncl: - __slots__ = ("initiator", "responder", "version", "salt") + __slots__ = ("version", "salt") msgtype = b"sendtxrcncl" def __init__(self): - self.initiator = False - self.responder = False self.version = 0 self.salt = 0 def deserialize(self, f): - self.initiator = struct.unpack("<?", f.read(1))[0] - self.responder = struct.unpack("<?", f.read(1))[0] self.version = struct.unpack("<I", f.read(4))[0] self.salt = struct.unpack("<Q", f.read(8))[0] def serialize(self): r = b"" - r += struct.pack("<?", self.initiator) - r += struct.pack("<?", self.responder) r += struct.pack("<I", self.version) r += struct.pack("<Q", self.salt) return r def __repr__(self): - return "msg_sendtxrcncl(initiator=%i, responder=%i, version=%lu, salt=%lu)" %\ - (self.initiator, self.responder, self.version, self.salt) + return "msg_sendtxrcncl(version=%lu, salt=%lu)" %\ + (self.version, self.salt) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index b1164b98fd..7cfeae3ff6 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -97,6 +97,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): self.chain: str = 'regtest' self.setup_clean_chain: bool = False self.nodes: List[TestNode] = [] + self.extra_args = None self.network_thread = None self.rpc_timeout = 60 # Wait for up to 60 seconds for the RPC server to respond self.supports_cli = True @@ -113,7 +114,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): self.wallet_names = None # By default the wallet is not required. Set to true by skip_if_no_wallet(). # When False, we ignore wallet_names regardless of what it is. - self.requires_wallet = False + self._requires_wallet = False # Disable ThreadOpenConnections by default, so that adding entries to # addrman will not result in automatic connections to them. self.disable_autoconnect = True @@ -194,12 +195,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): help="set a random seed for deterministically reproducing a previous test run") parser.add_argument('--timeout-factor', dest="timeout_factor", type=float, default=1.0, help='adjust test timeouts by a factor. Setting it to 0 disables all timeouts') - group = parser.add_mutually_exclusive_group() - group.add_argument("--descriptors", action='store_const', const=True, - help="Run test using a descriptor wallet", dest='descriptors') - group.add_argument("--legacy-wallet", action='store_const', const=False, - help="Run test using legacy wallets", dest='descriptors') - self.add_options(parser) # Running TestShell in a Jupyter notebook causes an additional -f argument # To keep TestShell from failing with an "unrecognized argument" error, we add a dummy "-f" argument @@ -212,7 +207,13 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): config.read_file(open(self.options.configfile)) self.config = config - if self.options.descriptors is None: + if "descriptors" not in self.options: + # Wallet is not required by the test at all and the value of self.options.descriptors won't matter. + # It still needs to exist and be None in order for tests to work however. + # So set it to None to force -disablewallet, because the wallet is not needed. + self.options.descriptors = None + elif self.options.descriptors is None: + # Some wallet is either required or optionally used by the test. # Prefer BDB unless it isn't available if self.is_bdb_compiled(): self.options.descriptors = False @@ -221,6 +222,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): else: # If neither are compiled, tests requiring a wallet will be skipped and the value of self.options.descriptors won't matter # It still needs to exist and be None in order for tests to work however. + # So set it to None, which will also set -disablewallet. self.options.descriptors = None PortSeed.n = self.options.port_seed @@ -407,12 +409,9 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): def setup_nodes(self): """Override this method to customize test node setup""" - extra_args = [[]] * self.num_nodes - if hasattr(self, "extra_args"): - extra_args = self.extra_args - self.add_nodes(self.num_nodes, extra_args) + self.add_nodes(self.num_nodes, self.extra_args) self.start_nodes() - if self.requires_wallet: + if self._requires_wallet: self.import_deterministic_coinbase_privkeys() if not self.setup_clean_chain: for n in self.nodes: @@ -446,6 +445,21 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # Public helper methods. These can be accessed by the subclass test scripts. + def add_wallet_options(self, parser, *, descriptors=True, legacy=True): + kwargs = {} + if descriptors + legacy == 1: + # If only one type can be chosen, set it as default + kwargs["default"] = descriptors + group = parser.add_mutually_exclusive_group( + # If only one type is allowed, require it to be set in test_runner.py + required=os.getenv("REQUIRE_WALLET_TYPE_SET") == "1" and "default" in kwargs) + if descriptors: + group.add_argument("--descriptors", action='store_const', const=True, **kwargs, + help="Run test using a descriptor wallet", dest='descriptors') + if legacy: + group.add_argument("--legacy-wallet", action='store_const', const=False, **kwargs, + help="Run test using legacy wallets", dest='descriptors') + def add_nodes(self, num_nodes: int, extra_args=None, *, rpchost=None, binary=None, binary_cli=None, versions=None): """Instantiate TestNode objects. @@ -866,7 +880,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): def skip_if_no_wallet(self): """Skip the running test if wallet has not been compiled.""" - self.requires_wallet = True + self._requires_wallet = True if not self.is_wallet_compiled(): raise SkipTest("wallet has not been compiled.") if self.options.descriptors: diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 64edff15ae..fc93940b63 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -105,6 +105,9 @@ class TestNode(): "-debugexclude=rand", "-uacomment=testnode%d" % i, ] + if self.descriptors is None: + self.args.append("-disablewallet") + if use_valgrind: default_suppressions_file = os.path.join( os.path.dirname(os.path.realpath(__file__)), @@ -720,7 +723,6 @@ class TestNodeCLI(): """Run bitcoin-cli command. Deserializes returned string as python object.""" pos_args = [arg_to_cli(arg) for arg in args] named_args = [str(key) + "=" + arg_to_cli(value) for (key, value) in kwargs.items()] - assert not (pos_args and named_args), "Cannot use positional arguments and named arguments in the same bitcoin-cli call" p_args = [self.binary, "-datadir=" + self.datadir] + self.options if named_args: p_args += ["-named"] diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index 374fda5c23..2b31fe7c87 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -32,7 +32,6 @@ from test_framework.messages import ( CTxIn, CTxInWitness, CTxOut, - tx_from_hex, ) from test_framework.script import ( CScript, @@ -338,6 +337,28 @@ class MiniWallet: self.scan_tx(from_node.decoderawtransaction(tx_hex)) return txid + def create_self_transfer_chain(self, *, chain_length): + """ + Create a "chain" of chain_length transactions. The nth transaction in + the chain is a child of the n-1th transaction and parent of the n+1th transaction. + + Returns a dic {"chain_hex": chain_hex, "chain_txns" : chain_txns} + + "chain_hex" is a list representing the chain's transactions in hexadecimal. + "chain_txns" is a list representing the chain's transactions in the CTransaction object. + """ + chaintip_utxo = self.get_utxo() + chain_hex = [] + chain_txns = [] + + for _ in range(chain_length): + tx = self.create_self_transfer(utxo_to_spend=chaintip_utxo) + chaintip_utxo = tx["new_utxo"] + chain_hex.append(tx["hex"]) + chain_txns.append(tx["tx"]) + + return {"chain_hex": chain_hex, "chain_txns" : chain_txns} + def send_self_transfer_chain(self, *, from_node, chain_length, utxo_to_spend=None): """Create and send a "chain" of chain_length transactions. The nth transaction in the chain is a child of the n-1th transaction and parent of the n+1th transaction. @@ -388,56 +409,3 @@ def address_to_scriptpubkey(address): # TODO: also support other address formats else: assert False - - -def make_chain(node, address, privkeys, parent_txid, parent_value, n=0, parent_locking_script=None, fee=DEFAULT_FEE): - """Build a transaction that spends parent_txid.vout[n] and produces one output with - amount = parent_value with a fee deducted. - Return tuple (CTransaction object, raw hex, nValue, scriptPubKey of the output created). - """ - inputs = [{"txid": parent_txid, "vout": n}] - my_value = parent_value - fee - outputs = {address : my_value} - rawtx = node.createrawtransaction(inputs, outputs) - prevtxs = [{ - "txid": parent_txid, - "vout": n, - "scriptPubKey": parent_locking_script, - "amount": parent_value, - }] if parent_locking_script else None - signedtx = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=privkeys, prevtxs=prevtxs) - assert signedtx["complete"] - tx = tx_from_hex(signedtx["hex"]) - return (tx, signedtx["hex"], my_value, tx.vout[0].scriptPubKey.hex()) - -def create_child_with_parents(node, address, privkeys, parents_tx, values, locking_scripts, fee=DEFAULT_FEE): - """Creates a transaction that spends the first output of each parent in parents_tx.""" - num_parents = len(parents_tx) - total_value = sum(values) - inputs = [{"txid": tx.rehash(), "vout": 0} for tx in parents_tx] - outputs = {address : total_value - fee} - rawtx_child = node.createrawtransaction(inputs, outputs) - prevtxs = [] - for i in range(num_parents): - prevtxs.append({"txid": parents_tx[i].rehash(), "vout": 0, "scriptPubKey": locking_scripts[i], "amount": values[i]}) - signedtx_child = node.signrawtransactionwithkey(hexstring=rawtx_child, privkeys=privkeys, prevtxs=prevtxs) - assert signedtx_child["complete"] - return signedtx_child["hex"] - -def create_raw_chain(node, first_coin, address, privkeys, chain_length=25): - """Helper function: create a "chain" of chain_length transactions. The nth transaction in the - chain is a child of the n-1th transaction and parent of the n+1th transaction. - """ - parent_locking_script = None - txid = first_coin["txid"] - chain_hex = [] - chain_txns = [] - value = first_coin["amount"] - - for _ in range(chain_length): - (tx, txhex, value, parent_locking_script) = make_chain(node, address, privkeys, txid, value, 0, parent_locking_script) - txid = tx.rehash() - chain_hex.append(txhex) - chain_txns.append(tx) - - return (chain_hex, chain_txns) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 785a97a48b..b9adb5dcb5 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -27,6 +27,8 @@ import re import logging import unittest +os.environ["REQUIRE_WALLET_TYPE_SET"] = "1" + # Formatting. Default colors to empty strings. DEFAULT, BOLD, GREEN, RED = ("", ""), ("", ""), ("", ""), ("", "") try: @@ -159,7 +161,7 @@ BASE_SCRIPTS = [ 'wallet_avoidreuse.py --descriptors', 'wallet_avoid_mixing_output_types.py --descriptors', 'mempool_reorg.py', - 'mempool_persist.py', + 'mempool_persist.py --descriptors', 'p2p_block_sync.py', 'wallet_multiwallet.py --legacy-wallet', 'wallet_multiwallet.py --descriptors', @@ -208,7 +210,7 @@ BASE_SCRIPTS = [ 'wallet_keypool.py --legacy-wallet', 'wallet_keypool.py --descriptors', 'wallet_descriptor.py --descriptors', - 'wallet_miniscript.py', + 'wallet_miniscript.py --descriptors', 'feature_maxtipage.py', 'p2p_nobloomfilter_messages.py', 'p2p_filter.py', @@ -222,7 +224,7 @@ BASE_SCRIPTS = [ 'feature_assumevalid.py', 'example_test.py', 'wallet_txn_doublespend.py --legacy-wallet', - 'wallet_multisig_descriptor_psbt.py', + 'wallet_multisig_descriptor_psbt.py --descriptors', 'wallet_txn_doublespend.py --descriptors', 'feature_backwards_compatibility.py --legacy-wallet', 'feature_backwards_compatibility.py --descriptors', @@ -294,8 +296,8 @@ BASE_SCRIPTS = [ 'wallet_sendall.py --legacy-wallet', 'wallet_sendall.py --descriptors', 'wallet_create_tx.py --descriptors', - 'wallet_taproot.py', - 'wallet_inactive_hdchains.py', + 'wallet_taproot.py --descriptors', + 'wallet_inactive_hdchains.py --legacy-wallet', 'p2p_fingerprint.py', 'feature_uacomment.py', 'feature_init.py', diff --git a/test/functional/tool_signet_miner.py b/test/functional/tool_signet_miner.py index e6fc9072ab..1ad2a579bf 100755 --- a/test/functional/tool_signet_miner.py +++ b/test/functional/tool_signet_miner.py @@ -20,6 +20,9 @@ CHALLENGE_PRIVATE_KEY = (42).to_bytes(32, 'big') class SignetMinerTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.chain = "signet" self.setup_clean_chain = True diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 1e5ce513cb..076288293c 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -19,6 +19,9 @@ BUFFER_SIZE = 16 * 1024 class ToolWalletTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index d7850b41ac..d7bfe08437 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -21,6 +21,9 @@ from test_framework.util import ( class AbandonConflictTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 self.extra_args = [["-minrelaytxfee=0.00001"], []] diff --git a/test/functional/wallet_address_types.py b/test/functional/wallet_address_types.py index 5b836f693f..497795409e 100755 --- a/test/functional/wallet_address_types.py +++ b/test/functional/wallet_address_types.py @@ -66,6 +66,9 @@ from test_framework.util import ( ) class AddressTypeTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 6 self.extra_args = [ diff --git a/test/functional/wallet_avoid_mixing_output_types.py b/test/functional/wallet_avoid_mixing_output_types.py index cad9d02808..861765f452 100755 --- a/test/functional/wallet_avoid_mixing_output_types.py +++ b/test/functional/wallet_avoid_mixing_output_types.py @@ -106,6 +106,9 @@ def generate_payment_values(n, m): class AddressInputTypeGrouping(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index f663666f57..474270cf80 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -63,6 +63,8 @@ def assert_balances(node, mine, margin=0.001): assert_approx(got[k], v, margin) class AvoidReuseTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) def set_test_params(self): self.num_nodes = 2 diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index 292fe3a310..eb36673b8a 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -44,6 +44,9 @@ from test_framework.util import ( class WalletBackupTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = True diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 60da22ca26..e8c09e4c1a 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -46,6 +46,9 @@ def create_transactions(node, address, amt, fees): return txs class WalletTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 20c577ceb3..bbda771e18 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -23,6 +23,9 @@ OUT_OF_RANGE = "Amount out of range" class WalletTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 4 self.extra_args = [[ diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 2ee3e00a7b..07ec55f8f3 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -17,10 +17,6 @@ from decimal import Decimal from test_framework.blocktools import ( COINBASE_MATURITY, - add_witness_commitment, - create_block, - create_coinbase, - send_to_witness, ) from test_framework.messages import ( MAX_BIP125_RBF_SEQUENCE, @@ -46,6 +42,9 @@ TOO_HIGH = 100000 class BumpFeeTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True @@ -200,16 +199,8 @@ def test_segwit_bumpfee_succeeds(self, rbf_node, dest_address): # Create a transaction with segwit output, then create an RBF transaction # which spends it, and make sure bumpfee can be called on it. - segwit_in = next(u for u in rbf_node.listunspent() if u["amount"] == Decimal("0.001")) - segwit_out = rbf_node.getaddressinfo(rbf_node.getnewaddress(address_type='bech32')) - segwitid = send_to_witness( - use_p2wsh=False, - node=rbf_node, - utxo=segwit_in, - pubkey=segwit_out["pubkey"], - encode_p2sh=False, - amount=Decimal("0.0009"), - sign=True) + segwit_out = rbf_node.getnewaddress(address_type='bech32') + segwitid = rbf_node.send({segwit_out: "0.0009"}, options={"change_position": 1})["txid"] rbfraw = rbf_node.createrawtransaction([{ 'txid': segwitid, @@ -541,10 +532,10 @@ def test_unconfirmed_not_spendable(self, rbf_node, rbf_node_address): # then invalidate the block so the rbf tx will be put back in the mempool. # This makes it possible to check whether the rbf tx outputs are # spendable before the rbf tx is confirmed. - block = submit_block_with_tx(rbf_node, rbftx) + block = self.generateblock(rbf_node, output="raw(51)", transactions=[rbftx]) # Can not abandon conflicted tx assert_raises_rpc_error(-5, 'Transaction not eligible for abandonment', lambda: rbf_node.abandontransaction(txid=bumpid)) - rbf_node.invalidateblock(block.hash) + rbf_node.invalidateblock(block["hash"]) # Call abandon to make sure the wallet doesn't attempt to resubmit # the bump tx and hope the wallet does not rebroadcast before we call. rbf_node.abandontransaction(bumpid) @@ -619,17 +610,6 @@ def spend_one_input(node, dest_address, change_size=Decimal("0.00049000")): return txid -def submit_block_with_tx(node, tx): - tip = node.getbestblockhash() - height = node.getblockcount() + 1 - block_time = node.getblockheader(tip)["mediantime"] + 1 - block = create_block(int(tip, 16), create_coinbase(height), block_time, txlist=[tx]) - add_witness_commitment(block) - block.solve() - node.submitblock(block.serialize().hex()) - return block - - def test_no_more_inputs_fails(self, rbf_node, dest_address): self.log.info('Test that bumpfee fails when there are no available confirmed outputs') # feerate rbf requires confirmed outputs when change output doesn't exist or is insufficient diff --git a/test/functional/wallet_coinbase_category.py b/test/functional/wallet_coinbase_category.py index c2a8e612cf..06cafd62f9 100755 --- a/test/functional/wallet_coinbase_category.py +++ b/test/functional/wallet_coinbase_category.py @@ -13,6 +13,9 @@ from test_framework.util import ( ) class CoinbaseCategoryTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True diff --git a/test/functional/wallet_create_tx.py b/test/functional/wallet_create_tx.py index a213a261ef..2415172c74 100755 --- a/test/functional/wallet_create_tx.py +++ b/test/functional/wallet_create_tx.py @@ -14,6 +14,9 @@ from test_framework.blocktools import ( class CreateTxWalletTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py index 12480d4d1e..95aa40720d 100755 --- a/test/functional/wallet_createwallet.py +++ b/test/functional/wallet_createwallet.py @@ -16,6 +16,9 @@ from test_framework.util import ( from test_framework.wallet_util import bytes_to_wif, generate_wif_key class CreateWalletTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 1 diff --git a/test/functional/wallet_crosschain.py b/test/functional/wallet_crosschain.py index c0047542ed..549eec34c3 100755 --- a/test/functional/wallet_crosschain.py +++ b/test/functional/wallet_crosschain.py @@ -9,6 +9,9 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_raises_rpc_error class WalletCrossChain(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True diff --git a/test/functional/wallet_descriptor.py b/test/functional/wallet_descriptor.py index e7cfa56c46..4b305106f1 100755 --- a/test/functional/wallet_descriptor.py +++ b/test/functional/wallet_descriptor.py @@ -13,6 +13,9 @@ from test_framework.util import ( class WalletDescriptorTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py index 9f0d666270..20b91fc523 100755 --- a/test/functional/wallet_dump.py +++ b/test/functional/wallet_dump.py @@ -93,6 +93,9 @@ def read_dump(file_name, addrs, script_addrs, hd_master_addr_old): class WalletDumpTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, descriptors=False) + def set_test_params(self): self.num_nodes = 1 self.extra_args = [["-keypool=90", "-addresstype=legacy"]] diff --git a/test/functional/wallet_encryption.py b/test/functional/wallet_encryption.py index 37c1c4bff3..abf6bc6393 100755 --- a/test/functional/wallet_encryption.py +++ b/test/functional/wallet_encryption.py @@ -14,6 +14,9 @@ from test_framework.util import ( class WalletEncryptionTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 diff --git a/test/functional/wallet_fallbackfee.py b/test/functional/wallet_fallbackfee.py index acd92097ff..4aa8eb466e 100755 --- a/test/functional/wallet_fallbackfee.py +++ b/test/functional/wallet_fallbackfee.py @@ -9,6 +9,9 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_raises_rpc_error class WalletRBFTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True diff --git a/test/functional/wallet_fast_rescan.py b/test/functional/wallet_fast_rescan.py index 3b8ae8eb92..52e33acb24 100755 --- a/test/functional/wallet_fast_rescan.py +++ b/test/functional/wallet_fast_rescan.py @@ -21,6 +21,9 @@ NUM_BLOCKS = 6 # number of blocks to mine class WalletFastRescanTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + def set_test_params(self): self.num_nodes = 1 self.extra_args = [[f'-keypool={KEYPOOL_SIZE}', '-blockfilterindex=1']] diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py index e5e4cf03bf..5da4c1e462 100755 --- a/test/functional/wallet_groups.py +++ b/test/functional/wallet_groups.py @@ -16,6 +16,9 @@ from test_framework.util import ( class WalletGroupTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 5 diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index 220c856498..05d1ac132d 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -16,6 +16,9 @@ from test_framework.util import ( class WalletHDTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 diff --git a/test/functional/wallet_implicitsegwit.py b/test/functional/wallet_implicitsegwit.py index a8583e2879..59f11ae9d7 100755 --- a/test/functional/wallet_implicitsegwit.py +++ b/test/functional/wallet_implicitsegwit.py @@ -39,6 +39,9 @@ def check_implicit_transactions(implicit_keys, implicit_node): assert(('receive', b_address) in tuple((tx['category'], tx['address']) for tx in txs)) class ImplicitSegwitTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, descriptors=False) + def set_test_params(self): self.num_nodes = 2 self.supports_cli = False diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 085ad51c79..64f8f2eb6e 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -147,6 +147,9 @@ def get_rand_amount(): class ImportRescanTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, descriptors=False) + def set_test_params(self): self.num_nodes = 2 + len(IMPORT_NODES) self.supports_cli = False diff --git a/test/functional/wallet_import_with_label.py b/test/functional/wallet_import_with_label.py index 6a9d2e8290..0c18448473 100755 --- a/test/functional/wallet_import_with_label.py +++ b/test/functional/wallet_import_with_label.py @@ -15,6 +15,9 @@ from test_framework.wallet_util import test_address class ImportWithLabel(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, descriptors=False) + def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index 9744009af8..f70b83cc5b 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -30,6 +30,9 @@ from test_framework.wallet_util import ( ) class ImportDescriptorsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + def set_test_params(self): self.num_nodes = 2 self.extra_args = [["-addresstype=legacy"], diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py index 62a1a3341d..78f33c0ac4 100755 --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -35,6 +35,9 @@ from test_framework.wallet_util import ( class ImportMultiTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, descriptors=False) + def set_test_params(self): self.num_nodes = 2 self.extra_args = [["-addresstype=legacy"], ["-addresstype=legacy"]] diff --git a/test/functional/wallet_importprunedfunds.py b/test/functional/wallet_importprunedfunds.py index 2a4d0981c7..a912856198 100755 --- a/test/functional/wallet_importprunedfunds.py +++ b/test/functional/wallet_importprunedfunds.py @@ -21,6 +21,9 @@ from test_framework.wallet_util import bytes_to_wif class ImportPrunedFundsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 diff --git a/test/functional/wallet_inactive_hdchains.py b/test/functional/wallet_inactive_hdchains.py index e1dad00876..43a0fa7c55 100755 --- a/test/functional/wallet_inactive_hdchains.py +++ b/test/functional/wallet_inactive_hdchains.py @@ -17,6 +17,9 @@ from test_framework.wallet_util import ( class InactiveHDChainsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, descriptors=False) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 diff --git a/test/functional/wallet_keypool.py b/test/functional/wallet_keypool.py index 54c47511a9..aa31757f35 100755 --- a/test/functional/wallet_keypool.py +++ b/test/functional/wallet_keypool.py @@ -11,6 +11,9 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_raises_rpc_error class KeyPoolTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 1 diff --git a/test/functional/wallet_keypool_topup.py b/test/functional/wallet_keypool_topup.py index 4c965b7160..d57e1b5aff 100755 --- a/test/functional/wallet_keypool_topup.py +++ b/test/functional/wallet_keypool_topup.py @@ -21,6 +21,9 @@ from test_framework.util import ( class KeypoolRestoreTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 4 diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py index c29b02e661..c92b6f2c2a 100755 --- a/test/functional/wallet_labels.py +++ b/test/functional/wallet_labels.py @@ -18,6 +18,9 @@ from test_framework.wallet_util import test_address class WalletLabelsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 diff --git a/test/functional/wallet_listdescriptors.py b/test/functional/wallet_listdescriptors.py index d5372f5aee..39217dd0f1 100755 --- a/test/functional/wallet_listdescriptors.py +++ b/test/functional/wallet_listdescriptors.py @@ -4,8 +4,11 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the listdescriptors RPC.""" +from test_framework.blocktools import ( + TIME_GENESIS_BLOCK, +) from test_framework.descriptors import ( - descsum_create + descsum_create, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -15,6 +18,9 @@ from test_framework.util import ( class ListDescriptorsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + def set_test_params(self): self.num_nodes = 1 @@ -63,13 +69,13 @@ class ListDescriptorsTest(BitcoinTestFramework): wallet = node.get_wallet_rpc('w2') wallet.importdescriptors([{ 'desc': descsum_create('wpkh(' + xprv + hardened_path + '/0/*)'), - 'timestamp': 1296688602, + 'timestamp': TIME_GENESIS_BLOCK, }]) expected = { 'wallet_name': 'w2', 'descriptors': [ {'desc': descsum_create('wpkh([80002067' + hardened_path + ']' + xpub_acc + '/0/*)'), - 'timestamp': 1296688602, + 'timestamp': TIME_GENESIS_BLOCK, 'active': False, 'range': [0, 0], 'next': 0}, @@ -83,7 +89,7 @@ class ListDescriptorsTest(BitcoinTestFramework): 'wallet_name': 'w2', 'descriptors': [ {'desc': descsum_create('wpkh(' + xprv + hardened_path + '/0/*)'), - 'timestamp': 1296688602, + 'timestamp': TIME_GENESIS_BLOCK, 'active': False, 'range': [0, 0], 'next': 0}, @@ -105,7 +111,7 @@ class ListDescriptorsTest(BitcoinTestFramework): watch_only_wallet = node.get_wallet_rpc('watch-only') watch_only_wallet.importdescriptors([{ 'desc': descsum_create('wpkh(' + xpub_acc + ')'), - 'timestamp': 1296688602, + 'timestamp': TIME_GENESIS_BLOCK, }]) assert_raises_rpc_error(-4, 'Can\'t get descriptor string', watch_only_wallet.listdescriptors, True) @@ -114,14 +120,14 @@ class ListDescriptorsTest(BitcoinTestFramework): wallet = node.get_wallet_rpc('w4') wallet.importdescriptors([{ 'desc': descsum_create('combo(' + node.get_deterministic_priv_key().key + ')'), - 'timestamp': 1296688602, + 'timestamp': TIME_GENESIS_BLOCK, }]) expected = { 'wallet_name': 'w4', 'descriptors': [ {'active': False, 'desc': 'combo(0227d85ba011276cf25b51df6a188b75e604b38770a462b2d0e9fb2fc839ef5d3f)#np574htj', - 'timestamp': 1296688602}, + 'timestamp': TIME_GENESIS_BLOCK}, ] } assert_equal(expected, wallet.listdescriptors()) diff --git a/test/functional/wallet_listreceivedby.py b/test/functional/wallet_listreceivedby.py index f1d7de2f27..04f1e87fe1 100755 --- a/test/functional/wallet_listreceivedby.py +++ b/test/functional/wallet_listreceivedby.py @@ -16,6 +16,9 @@ from test_framework.wallet_util import test_address class ReceivedByTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 # whitelist peers to speed up tx relay / mempool sync diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index aff408ceb1..ecdfb7d0e3 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -20,6 +20,9 @@ from test_framework.wallet_util import bytes_to_wif from decimal import Decimal class ListSinceBlockTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = True diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index 9bb06774a5..27246e3902 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -21,6 +21,9 @@ from test_framework.util import ( class ListTransactionsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 3 # This test isn't testing txn relay/timing, so set whitelist on the diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 3c1cb6ac32..37625e50d8 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -19,6 +19,9 @@ from test_framework.wallet_util import ( class WalletMigrationTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 @@ -393,6 +396,15 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(bals, wallet.getbalances()) + def test_encrypted(self): + self.log.info("Test migration of an encrypted wallet") + wallet = self.create_legacy_wallet("encrypted") + + wallet.encryptwallet("pass") + + assert_raises_rpc_error(-15, "Error: migratewallet on encrypted wallets is currently unsupported.", wallet.migratewallet) + # TODO: Fix migratewallet so that we can actually migrate encrypted wallets + def run_test(self): self.generate(self.nodes[0], 101) @@ -402,6 +414,7 @@ class WalletMigrationTest(BitcoinTestFramework): self.test_other_watchonly() self.test_no_privkeys() self.test_pk_coinbases() + self.test_encrypted() if __name__ == '__main__': WalletMigrationTest().main() diff --git a/test/functional/wallet_miniscript.py b/test/functional/wallet_miniscript.py index 2252f1e424..cefcaf4dc7 100755 --- a/test/functional/wallet_miniscript.py +++ b/test/functional/wallet_miniscript.py @@ -22,6 +22,9 @@ MINISCRIPTS = [ class WalletMiniscriptTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + def set_test_params(self): self.num_nodes = 1 diff --git a/test/functional/wallet_multisig_descriptor_psbt.py b/test/functional/wallet_multisig_descriptor_psbt.py index 2b565db137..f741eac9f3 100755 --- a/test/functional/wallet_multisig_descriptor_psbt.py +++ b/test/functional/wallet_multisig_descriptor_psbt.py @@ -16,6 +16,9 @@ from test_framework.util import ( class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + def set_test_params(self): self.num_nodes = 3 self.setup_clean_chain = True diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 1c890d7207..1d0bb5a9b3 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -52,6 +52,7 @@ class MultiWalletTest(BitcoinTestFramework): self.skip_if_no_wallet() def add_options(self, parser): + self.add_wallet_options(parser) parser.add_argument( '--data_wallets_dir', default=os.path.join(os.path.dirname(os.path.realpath(__file__)), 'data/wallets/'), diff --git a/test/functional/wallet_orphanedreward.py b/test/functional/wallet_orphanedreward.py index 7295db4653..06a96754cf 100755 --- a/test/functional/wallet_orphanedreward.py +++ b/test/functional/wallet_orphanedreward.py @@ -8,6 +8,9 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal class OrphanedBlockRewardTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 diff --git a/test/functional/wallet_reorgsrestore.py b/test/functional/wallet_reorgsrestore.py index f2bdb114b7..5350c73abb 100755 --- a/test/functional/wallet_reorgsrestore.py +++ b/test/functional/wallet_reorgsrestore.py @@ -23,6 +23,9 @@ from test_framework.util import ( ) class ReorgsRestoreTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 3 diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py index b3d02fbfc9..fb8d1215f8 100755 --- a/test/functional/wallet_resendwallettransactions.py +++ b/test/functional/wallet_resendwallettransactions.py @@ -18,6 +18,9 @@ from test_framework.util import ( ) class ResendWalletTransactionsTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 1 diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 83b8332c4d..eb7d9616a1 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -25,6 +25,9 @@ from test_framework.util import ( from test_framework.wallet_util import bytes_to_wif class WalletSendTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 2 # whitelist all peers to speed up tx relay / mempool sync diff --git a/test/functional/wallet_sendall.py b/test/functional/wallet_sendall.py index 4fe11455b1..042b5dbf79 100755 --- a/test/functional/wallet_sendall.py +++ b/test/functional/wallet_sendall.py @@ -26,6 +26,9 @@ def cleanup(func): class SendallTest(BitcoinTestFramework): # Setup and helpers + def add_options(self, parser): + self.add_wallet_options(parser) + def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index db3a8a2efa..9ab7154424 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -18,6 +18,9 @@ from test_framework.util import ( class WalletSignerTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + def mock_signer_path(self): path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'signer.py') if platform.system() == "Windows": diff --git a/test/functional/wallet_signmessagewithaddress.py b/test/functional/wallet_signmessagewithaddress.py index 74a8f2eef2..be43bab501 100755 --- a/test/functional/wallet_signmessagewithaddress.py +++ b/test/functional/wallet_signmessagewithaddress.py @@ -10,6 +10,9 @@ from test_framework.util import ( ) class SignMessagesWithAddressTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 diff --git a/test/functional/wallet_signrawtransactionwithwallet.py b/test/functional/wallet_signrawtransactionwithwallet.py index 6b30386b7e..247269ce2a 100755 --- a/test/functional/wallet_signrawtransactionwithwallet.py +++ b/test/functional/wallet_signrawtransactionwithwallet.py @@ -34,6 +34,9 @@ from decimal import ( ) class SignRawTransactionWithWalletTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 diff --git a/test/functional/wallet_simulaterawtx.py b/test/functional/wallet_simulaterawtx.py index a408b99515..b7c64f3a93 100755 --- a/test/functional/wallet_simulaterawtx.py +++ b/test/functional/wallet_simulaterawtx.py @@ -15,6 +15,9 @@ from test_framework.util import ( ) class SimulateTxTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 diff --git a/test/functional/wallet_startup.py b/test/functional/wallet_startup.py index d96c2da686..fefd5798f7 100755 --- a/test/functional/wallet_startup.py +++ b/test/functional/wallet_startup.py @@ -13,6 +13,9 @@ from test_framework.util import ( class WalletStartupTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py index 4c28958982..dde83269fa 100755 --- a/test/functional/wallet_taproot.py +++ b/test/functional/wallet_taproot.py @@ -187,6 +187,9 @@ def compute_raw_taproot_address(pubkey): class WalletTaprootTest(BitcoinTestFramework): """Test generation and spending of P2TR address outputs.""" + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True diff --git a/test/functional/wallet_timelock.py b/test/functional/wallet_timelock.py index a71cec6607..57a7c3907a 100755 --- a/test/functional/wallet_timelock.py +++ b/test/functional/wallet_timelock.py @@ -8,6 +8,9 @@ from test_framework.util import assert_equal class WalletLocktimeTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 1 diff --git a/test/functional/wallet_transactiontime_rescan.py b/test/functional/wallet_transactiontime_rescan.py index 9caa1fa3d0..c8f4e260da 100755 --- a/test/functional/wallet_transactiontime_rescan.py +++ b/test/functional/wallet_transactiontime_rescan.py @@ -17,6 +17,9 @@ from test_framework.util import ( class TransactionTimeRescanTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.setup_clean_chain = False self.num_nodes = 3 diff --git a/test/functional/wallet_txn_clone.py b/test/functional/wallet_txn_clone.py index 5bdde13aa4..a06f094610 100755 --- a/test/functional/wallet_txn_clone.py +++ b/test/functional/wallet_txn_clone.py @@ -24,6 +24,7 @@ class TxnMallTest(BitcoinTestFramework): self.skip_if_no_wallet() def add_options(self, parser): + self.add_wallet_options(parser) parser.add_argument("--mineblock", dest="mine_block", default=False, action="store_true", help="Test double-spend of 1-confirmed transaction") parser.add_argument("--segwit", dest="segwit", default=False, action="store_true", diff --git a/test/functional/wallet_txn_doublespend.py b/test/functional/wallet_txn_doublespend.py index 206187fb61..bfb29ae773 100755 --- a/test/functional/wallet_txn_doublespend.py +++ b/test/functional/wallet_txn_doublespend.py @@ -22,6 +22,7 @@ class TxnMallTest(BitcoinTestFramework): self.skip_if_no_wallet() def add_options(self, parser): + self.add_wallet_options(parser) parser.add_argument("--mineblock", dest="mine_block", default=False, action="store_true", help="Test double-spend of 1-confirmed transaction") diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py index c452e1eafd..97df320464 100755 --- a/test/functional/wallet_upgradewallet.py +++ b/test/functional/wallet_upgradewallet.py @@ -45,6 +45,9 @@ def deser_keymeta(f): return ver, create_time, kp_str, seed_id, fpr, path_len, path, has_key_orig class UpgradeWalletTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, descriptors=False) + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 diff --git a/test/functional/wallet_watchonly.py b/test/functional/wallet_watchonly.py index 69c32ba54c..f5bccb14c0 100755 --- a/test/functional/wallet_watchonly.py +++ b/test/functional/wallet_watchonly.py @@ -14,6 +14,9 @@ from test_framework.util import ( class CreateWalletWatchonlyTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 1 diff --git a/test/get_previous_releases.py b/test/get_previous_releases.py index c983dce619..ca06d7b8f8 100755 --- a/test/get_previous_releases.py +++ b/test/get_previous_releases.py @@ -136,7 +136,7 @@ def download_binary(tag, args) -> int: tarballHash = hasher.hexdigest() if tarballHash not in SHA256_SUMS or SHA256_SUMS[tarballHash]['tarball'] != tarball: - if tarball in SHA256_SUMS.values(): + if tarball in [v['tarball'] for v in SHA256_SUMS.values()]: print("Checksum did not match") return 1 @@ -260,11 +260,10 @@ if __name__ == '__main__': help='download release binary.') parser.add_argument('-t', '--target-dir', action='store', help='target directory.', default='releases') - parser.add_argument('tags', nargs='*', default=set( - [v['tag'] for v in SHA256_SUMS.values()] - ), + all_tags = sorted([*set([v['tag'] for v in SHA256_SUMS.values()])]) + parser.add_argument('tags', nargs='*', default=all_tags, help='release tags. e.g.: v0.18.1 v0.20.0rc2 ' - '(if not specified, the full list needed for' + '(if not specified, the full list needed for ' 'backwards compatibility tests will be used)' ) args = parser.parse_args() diff --git a/test/lint/lint-git-commit-check.py b/test/lint/lint-git-commit-check.py index a1d03370e8..049104398a 100755 --- a/test/lint/lint-git-commit-check.py +++ b/test/lint/lint-git-commit-check.py @@ -46,6 +46,8 @@ def main(): commit_range = merge_base + "..HEAD" else: commit_range = os.getenv("COMMIT_RANGE") + if commit_range == "SKIP_EMPTY_NOT_A_PR": + sys.exit(0) commit_hashes = check_output(["git", "log", commit_range, "--format=%H"], universal_newlines=True, encoding="utf8").splitlines() diff --git a/test/lint/lint-whitespace.py b/test/lint/lint-whitespace.py index 3fb5b80013..72b7ebc394 100755 --- a/test/lint/lint-whitespace.py +++ b/test/lint/lint-whitespace.py @@ -97,6 +97,8 @@ def main(): commit_range = merge_base + "..HEAD" else: commit_range = os.getenv("COMMIT_RANGE") + if commit_range == "SKIP_EMPTY_NOT_A_PR": + sys.exit(0) whitespace_selection = [] tab_selection = [] |