diff options
45 files changed, 934 insertions, 456 deletions
diff --git a/build-aux/m4/ax_boost_chrono.m4 b/build-aux/m4/ax_boost_chrono.m4 deleted file mode 100644 index 4cd3b86041..0000000000 --- a/build-aux/m4/ax_boost_chrono.m4 +++ /dev/null @@ -1,118 +0,0 @@ -# =========================================================================== -# https://www.gnu.org/software/autoconf-archive/ax_boost_chrono.html -# =========================================================================== -# -# SYNOPSIS -# -# AX_BOOST_CHRONO -# -# DESCRIPTION -# -# Test for Chrono library from the Boost C++ libraries. The macro requires -# a preceding call to AX_BOOST_BASE. Further documentation is available at -# <http://randspringer.de/boost/index.html>. -# -# This macro calls: -# -# AC_SUBST(BOOST_CHRONO_LIB) -# -# And sets: -# -# HAVE_BOOST_CHRONO -# -# LICENSE -# -# Copyright (c) 2012 Xiyue Deng <manphiz@gmail.com> -# -# Copying and distribution of this file, with or without modification, are -# permitted in any medium without royalty provided the copyright notice -# and this notice are preserved. This file is offered as-is, without any -# warranty. - -#serial 5 - -AC_DEFUN([AX_BOOST_CHRONO], -[ - AC_ARG_WITH([boost-chrono], - AS_HELP_STRING([--with-boost-chrono@<:@=special-lib@:>@], - [use the Chrono library from boost - it is possible to specify a certain library for the linker - e.g. --with-boost-chrono=boost_chrono-gcc-mt ]), - [ - if test "$withval" = "no"; then - want_boost="no" - elif test "$withval" = "yes"; then - want_boost="yes" - ax_boost_user_chrono_lib="" - else - want_boost="yes" - ax_boost_user_chrono_lib="$withval" - fi - ], - [want_boost="yes"] - ) - - if test "x$want_boost" = "xyes"; then - AC_REQUIRE([AC_PROG_CC]) - AC_REQUIRE([AC_CANONICAL_BUILD]) - CPPFLAGS_SAVED="$CPPFLAGS" - CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS" - export CPPFLAGS - - LDFLAGS_SAVED="$LDFLAGS" - LDFLAGS="$LDFLAGS $BOOST_LDFLAGS" - export LDFLAGS - - AC_CACHE_CHECK(whether the Boost::Chrono library is available, - ax_cv_boost_chrono, - [AC_LANG_PUSH([C++]) - CXXFLAGS_SAVE=$CXXFLAGS - - AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[@%:@include <boost/chrono.hpp>]], - [[boost::chrono::system_clock::time_point* time = new boost::chrono::system_clock::time_point; delete time;]])], - ax_cv_boost_chrono=yes, ax_cv_boost_chrono=no) - CXXFLAGS=$CXXFLAGS_SAVE - AC_LANG_POP([C++]) - ]) - if test "x$ax_cv_boost_chrono" = "xyes"; then - AC_SUBST(BOOST_CPPFLAGS) - - AC_DEFINE(HAVE_BOOST_CHRONO,,[define if the Boost::Chrono library is available]) - BOOSTLIBDIR=`echo $BOOST_LDFLAGS | sed -e 's/@<:@^\/@:>@*//'` - - LDFLAGS_SAVE=$LDFLAGS - if test "x$ax_boost_user_chrono_lib" = "x"; then - for libextension in `ls $BOOSTLIBDIR/libboost_chrono*.so* $BOOSTLIBDIR/libboost_chrono*.dylib* $BOOSTLIBDIR/libboost_chrono*.a* 2>/dev/null | sed 's,.*/,,' | sed -e 's;^lib\(boost_chrono.*\)\.so.*$;\1;' -e 's;^lib\(boost_chrono.*\)\.dylib.*$;\1;' -e 's;^lib\(boost_chrono.*\)\.a.*$;\1;'` ; do - ax_lib=${libextension} - AC_CHECK_LIB($ax_lib, exit, - [BOOST_CHRONO_LIB="-l$ax_lib"; AC_SUBST(BOOST_CHRONO_LIB) link_chrono="yes"; break], - [link_chrono="no"]) - done - if test "x$link_chrono" != "xyes"; then - for libextension in `ls $BOOSTLIBDIR/boost_chrono*.dll* $BOOSTLIBDIR/boost_chrono*.a* 2>/dev/null | sed 's,.*/,,' | sed -e 's;^\(boost_chrono.*\)\.dll.*$;\1;' -e 's;^\(boost_chrono.*\)\.a.*$;\1;'` ; do - ax_lib=${libextension} - AC_CHECK_LIB($ax_lib, exit, - [BOOST_CHRONO_LIB="-l$ax_lib"; AC_SUBST(BOOST_CHRONO_LIB) link_chrono="yes"; break], - [link_chrono="no"]) - done - fi - - else - for ax_lib in $ax_boost_user_chrono_lib boost_chrono-$ax_boost_user_chrono_lib; do - AC_CHECK_LIB($ax_lib, exit, - [BOOST_CHRONO_LIB="-l$ax_lib"; AC_SUBST(BOOST_CHRONO_LIB) link_chrono="yes"; break], - [link_chrono="no"]) - done - - fi - if test "x$ax_lib" = "x"; then - AC_MSG_ERROR(Could not find a version of the Boost::Chrono library!) - fi - if test "x$link_chrono" = "xno"; then - AC_MSG_ERROR(Could not link against $ax_lib !) - fi - fi - - CPPFLAGS="$CPPFLAGS_SAVED" - LDFLAGS="$LDFLAGS_SAVED" - fi -]) diff --git a/build_msvc/bitcoin_config.h b/build_msvc/bitcoin_config.h index 47c0a5f407..ea304dc9fc 100644 --- a/build_msvc/bitcoin_config.h +++ b/build_msvc/bitcoin_config.h @@ -47,9 +47,6 @@ /* define if the Boost library is available */ #define HAVE_BOOST /**/ -/* define if the Boost::Chrono library is available */ -#define HAVE_BOOST_CHRONO /**/ - /* define if the Boost::Filesystem library is available */ #define HAVE_BOOST_FILESYSTEM /**/ diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index 24c043cb52..d5f39daaf5 100644 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -7,7 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_asan -export PACKAGES="clang-8 llvm-8 python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" +export PACKAGES="clang-8 llvm-8 python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" # Use clang-8 instead of default clang (which is clang-6 on Bionic) to avoid spurious segfaults when running on ppc64le export NO_DEPENDS=1 export GOAL="install" diff --git a/ci/test/00_setup_env_native_fuzz.sh b/ci/test/00_setup_env_native_fuzz.sh index af93dd1d68..a739ad50d2 100644 --- a/ci/test/00_setup_env_native_fuzz.sh +++ b/ci/test/00_setup_env_native_fuzz.sh @@ -7,7 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_fuzz -export PACKAGES="clang-8 llvm-8 python3 libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev" +export PACKAGES="clang-8 llvm-8 python3 libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev" export NO_DEPENDS=1 export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false diff --git a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh index 45b13a669d..fabb3affa4 100644 --- a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh +++ b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh @@ -7,7 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_fuzz_valgrind -export PACKAGES="clang-8 llvm-8 python3 libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev valgrind" +export PACKAGES="clang-8 llvm-8 python3 libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev valgrind" export NO_DEPENDS=1 export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false diff --git a/ci/test/00_setup_env_native_tsan.sh b/ci/test/00_setup_env_native_tsan.sh index 37884128fe..4d3f345ca6 100644 --- a/ci/test/00_setup_env_native_tsan.sh +++ b/ci/test/00_setup_env_native_tsan.sh @@ -8,7 +8,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_tsan export DOCKER_NAME_TAG=ubuntu:16.04 -export PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" +export PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" export NO_DEPENDS=1 export GOAL="install" export BITCOIN_CONFIG="--enable-zmq --disable-wallet --with-gui=qt5 CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' --with-sanitizers=thread --disable-hardening --disable-asm CC=clang CXX=clang++" diff --git a/ci/test/00_setup_env_native_valgrind.sh b/ci/test/00_setup_env_native_valgrind.sh index 156a83590c..f112aa87f9 100644 --- a/ci/test/00_setup_env_native_valgrind.sh +++ b/ci/test/00_setup_env_native_valgrind.sh @@ -7,10 +7,10 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_valgrind -export PACKAGES="valgrind clang llvm python3-zmq libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev" +export PACKAGES="valgrind clang llvm python3-zmq libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev" export USE_VALGRIND=1 export NO_DEPENDS=1 -export TEST_RUNNER_EXTRA="p2p_segwit.py" # Only run one test for now. TODO enable all and bump timeouts +export TEST_RUNNER_EXTRA="--exclude feature_abortnode,feature_block,rpc_bind" # Excluded for now export RUN_FUNCTIONAL_TESTS=true export GOAL="install" export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC=clang CXX=clang++" # TODO enable GUI diff --git a/configure.ac b/configure.ac index 8433615bb2..1c0f39d7f7 100644 --- a/configure.ac +++ b/configure.ac @@ -1139,7 +1139,6 @@ fi AX_BOOST_SYSTEM AX_BOOST_FILESYSTEM AX_BOOST_THREAD -AX_BOOST_CHRONO dnl Boost 1.56 through 1.62 allow using std::atomic instead of its own atomic dnl counter implementations. In 1.63 and later the std::atomic approach is default. @@ -1206,7 +1205,7 @@ fi if test x$use_boost = xyes; then -BOOST_LIBS="$BOOST_LDFLAGS $BOOST_SYSTEM_LIB $BOOST_FILESYSTEM_LIB $BOOST_THREAD_LIB $BOOST_CHRONO_LIB" +BOOST_LIBS="$BOOST_LDFLAGS $BOOST_SYSTEM_LIB $BOOST_FILESYSTEM_LIB $BOOST_THREAD_LIB" dnl If boost (prior to 1.57) was built without c++11, it emulated scoped enums diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index cd0e70fb1c..cbe4fe4d97 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -22,7 +22,7 @@ $(package)_config_opts_armv7a_android=address-model=32 $(package)_toolset_$(host_os)=gcc $(package)_archiver_$(host_os)=$($(package)_ar) $(package)_toolset_darwin=clang-darwin -$(package)_config_libraries=chrono,filesystem,system,thread,test +$(package)_config_libraries=filesystem,system,thread,test $(package)_cxxflags=-std=c++11 -fvisibility=hidden $(package)_cxxflags_linux=-fPIC $(package)_cxxflags_android=-fPIC diff --git a/doc/build-unix.md b/doc/build-unix.md index e799e709fa..6b51db5f55 100644 --- a/doc/build-unix.md +++ b/doc/build-unix.md @@ -80,7 +80,7 @@ Build requirements: Now, you can either build from self-compiled [depends](/depends/README.md) or install the required dependencies: - sudo apt-get install libevent-dev libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev + sudo apt-get install libevent-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev BerkeleyDB is required for the wallet. diff --git a/doc/release-notes/release-notes-0.19.1.md b/doc/release-notes/release-notes-0.19.1.md new file mode 100644 index 0000000000..5746bebb0d --- /dev/null +++ b/doc/release-notes/release-notes-0.19.1.md @@ -0,0 +1,115 @@ +0.19.1 Release Notes +=============================== + +Bitcoin Core version 0.19.1 is now available from: + + <https://bitcoincore.org/bin/bitcoin-core-0.19.1/> + +This minor release includes various bug fixes and performance +improvements, as well as updated translations. + +Please report bugs using the issue tracker at GitHub: + + <https://github.com/bitcoin/bitcoin/issues> + +To receive security and update notifications, please subscribe to: + + <https://bitcoincore.org/en/list/announcements/join/> + +How to Upgrade +============== + +If you are running an older version, shut it down. Wait until it has completely +shut down (which might take a few minutes for older versions), then run the +installer (on Windows) or just copy over `/Applications/Bitcoin-Qt` (on Mac) +or `bitcoind`/`bitcoin-qt` (on Linux). + +Upgrading directly from a version of Bitcoin Core that has reached its EOL is +possible, but it might take some time if the datadir needs to be migrated. Old +wallet versions of Bitcoin Core are generally supported. + +Compatibility +============== + +Bitcoin Core is supported and extensively tested on operating systems using +the Linux kernel, macOS 10.10+, and Windows 7 and newer. It is not recommended +to use Bitcoin Core on unsupported systems. + +Bitcoin Core should also work on most other Unix-like systems but is not +as frequently tested on them. + +From Bitcoin Core 0.17.0 onwards, macOS versions earlier than 10.10 are no +longer supported, as Bitcoin Core is now built using Qt 5.9.x which requires +macOS 10.10+. Additionally, Bitcoin Core does not yet change appearance when +macOS "dark mode" is activated. + +In addition to previously supported CPU platforms, this release's pre-compiled +distribution provides binaries for the RISC-V platform. + +0.19.1 change log +================= + +### Wallet +- #17643 Fix origfee return for bumpfee with feerate arg (instagibbs) +- #16963 Fix `unique_ptr` usage in boost::signals2 (promag) +- #17258 Fix issue with conflicted mempool tx in listsinceblock (adamjonas, mchrostowski) +- #17924 Bug: IsUsedDestination shouldn't use key id as script id for ScriptHash (instagibbs) +- #17621 IsUsedDestination should count any known single-key address (instagibbs) +- #17843 Reset reused transactions cache (fjahr) + +### RPC and other APIs +- #17687 cli: Fix fatal leveldb error when specifying -blockfilterindex=basic twice (brakmic) +- #17728 require second argument only for scantxoutset start action (achow101) +- #17445 zmq: Fix due to invalid argument and multiple notifiers (promag) +- #17524 psbt: handle unspendable psbts (achow101) +- #17156 psbt: check that various indexes and amounts are within bounds (achow101) + +### GUI +- #17427 Fix missing qRegisterMetaType for `size_t` (hebasto) +- #17695 disable File-\>CreateWallet during startup (fanquake) +- #17634 Fix comparison function signature (hebasto) +- #18062 Fix unintialized WalletView::progressDialog (promag) + +### Tests and QA +- #17416 Appveyor improvement - text file for vcpkg package list (sipsorcery) +- #17488 fix "bitcoind already running" warnings on macOS (fanquake) +- #17980 add missing #include to fix compiler errors (kallewoof) + +### Platform support +- #17736 Update msvc build for Visual Studio 2019 v16.4 (sipsorcery) +- #17364 Updates to appveyor config for VS2019 and Qt5.9.8 + msvc project fixes (sipsorcery) +- #17887 bug-fix macos: give free bytes to `F_PREALLOCATE` (kallewoof) + +### Miscellaneous +- #17897 init: Stop indexes on shutdown after ChainStateFlushed callback (jimpo) +- #17450 util: Add missing headers to util/fees.cpp (hebasto) +- #17654 Unbreak build with Boost 1.72.0 (jbeich) +- #17857 scripts: Fix symbol-check & security-check argument passing (fanquake) +- #17762 Log to net category for exceptions in ProcessMessages (laanwj) +- #18100 Update univalue subtree (MarcoFalke) + +Credits +======= + +Thanks to everyone who directly contributed to this release: + +- Aaron Clauson +- Adam Jonas +- Andrew Chow +- Fabian Jahr +- fanquake +- Gregory Sanders +- Harris +- Hennadii Stepanov +- Jan Beich +- Jim Posen +- João Barbosa +- Karl-Johan Alm +- Luke Dashjr +- MarcoFalke +- Michael Chrostowski +- Russell Yanofsky +- Wladimir J. van der Laan + +As well as to everyone that helped with translations on +[Transifex](https://www.transifex.com/bitcoin/bitcoin/). diff --git a/src/Makefile.am b/src/Makefile.am index abd3bb881a..6fc6d5b5a3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -241,7 +241,6 @@ BITCOIN_CORE_H = \ wallet/fees.h \ wallet/ismine.h \ wallet/load.h \ - wallet/psbtwallet.h \ wallet/rpcwallet.h \ wallet/scriptpubkeyman.h \ wallet/wallet.h \ @@ -349,7 +348,6 @@ libbitcoin_wallet_a_SOURCES = \ wallet/feebumper.cpp \ wallet/fees.cpp \ wallet/load.cpp \ - wallet/psbtwallet.cpp \ wallet/rpcdump.cpp \ wallet/rpcwallet.cpp \ wallet/scriptpubkeyman.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index e1ca4d6cdc..5a2ecff651 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -85,6 +85,8 @@ FUZZ_TARGETS = \ test/fuzz/script \ test/fuzz/script_deserialize \ test/fuzz/script_flags \ + test/fuzz/script_ops \ + test/fuzz/scriptnum_ops \ test/fuzz/service_deserialize \ test/fuzz/spanparsing \ test/fuzz/strprintf \ @@ -764,6 +766,18 @@ test_fuzz_script_flags_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_script_flags_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_script_flags_SOURCES = $(FUZZ_SUITE) test/fuzz/script_flags.cpp +test_fuzz_script_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_script_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_script_ops_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_script_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_script_ops_SOURCES = $(FUZZ_SUITE) test/fuzz/script_ops.cpp + +test_fuzz_scriptnum_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_scriptnum_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_scriptnum_ops_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_scriptnum_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_scriptnum_ops_SOURCES = $(FUZZ_SUITE) test/fuzz/scriptnum_ops.cpp + test_fuzz_service_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DSERVICE_DESERIALIZE=1 test_fuzz_service_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_service_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 7ddf3b22e8..01ade56b2a 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -19,7 +19,6 @@ #include <wallet/fees.h> #include <wallet/ismine.h> #include <wallet/load.h> -#include <wallet/psbtwallet.h> #include <wallet/rpcwallet.h> #include <wallet/wallet.h> @@ -119,19 +118,15 @@ public: } bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) override { - std::unique_ptr<SigningProvider> provider = m_wallet->GetSigningProvider(script); + std::unique_ptr<SigningProvider> provider = m_wallet->GetSolvingProvider(script); if (provider) { return provider->GetPubKey(address, pub_key); } return false; } - bool getPrivKey(const CScript& script, const CKeyID& address, CKey& key) override + SigningResult signMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) override { - std::unique_ptr<SigningProvider> provider = m_wallet->GetSigningProvider(script); - if (provider) { - return provider->GetKey(address, key); - } - return false; + return m_wallet->SignMessage(message, pkhash, str_sig); } bool isSpendable(const CTxDestination& dest) override { return m_wallet->IsMine(dest) & ISMINE_SPENDABLE; } bool haveWatchOnly() override @@ -361,9 +356,9 @@ public: bool& complete, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, - bool bip32derivs = false) override + bool bip32derivs = false) const override { - return FillPSBT(m_wallet.get(), psbtx, complete, sighash_type, sign, bip32derivs); + return m_wallet->FillPSBT(psbtx, complete, sighash_type, sign, bip32derivs); } WalletBalances getBalances() override { diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 4351253412..9476c9f77f 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -10,6 +10,7 @@ #include <script/standard.h> // For CTxDestination #include <support/allocators/secure.h> // For SecureString #include <ui_interface.h> // For ChangeType +#include <util/message.h> #include <functional> #include <map> @@ -84,8 +85,8 @@ public: //! Get public key. virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0; - //! Get private key. - virtual bool getPrivKey(const CScript& script, const CKeyID& address, CKey& key) = 0; + //! Sign message + virtual SigningResult signMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) = 0; //! Return whether wallet has private key. virtual bool isSpendable(const CTxDestination& dest) = 0; @@ -196,7 +197,7 @@ public: bool& complete, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, - bool bip32derivs = false) = 0; + bool bip32derivs = false) const = 0; //! Get balances. virtual WalletBalances getBalances() = 0; diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index cc01aafb23..4ddee513a1 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -26,7 +26,6 @@ #include <ui_interface.h> #include <wallet/coincontrol.h> #include <wallet/fees.h> -#include <wallet/psbtwallet.h> #include <wallet/wallet.h> #include <QFontMetrics> diff --git a/src/qt/signverifymessagedialog.cpp b/src/qt/signverifymessagedialog.cpp index 883dcecf9a..4552753bf6 100644 --- a/src/qt/signverifymessagedialog.cpp +++ b/src/qt/signverifymessagedialog.cpp @@ -133,20 +133,27 @@ void SignVerifyMessageDialog::on_signMessageButton_SM_clicked() return; } - CKey key; - if (!model->wallet().getPrivKey(GetScriptForDestination(destination), CKeyID(*pkhash), key)) - { - ui->statusLabel_SM->setStyleSheet("QLabel { color: red; }"); - ui->statusLabel_SM->setText(tr("Private key for the entered address is not available.")); - return; - } - const std::string& message = ui->messageIn_SM->document()->toPlainText().toStdString(); std::string signature; + SigningResult res = model->wallet().signMessage(message, *pkhash, signature); + + QString error; + switch (res) { + case SigningResult::OK: + error = tr("No error"); + break; + case SigningResult::PRIVATE_KEY_NOT_AVAILABLE: + error = tr("Private key for the entered address is not available."); + break; + case SigningResult::SIGNING_FAILED: + error = tr("Message signing failed."); + break; + // no default case, so the compiler can warn about missing cases + } - if (!MessageSign(key, message, signature)) { + if (res != SigningResult::OK) { ui->statusLabel_SM->setStyleSheet("QLabel { color: red; }"); - ui->statusLabel_SM->setText(QString("<nobr>") + tr("Message signing failed.") + QString("</nobr>")); + ui->statusLabel_SM->setText(QString("<nobr>") + error + QString("</nobr>")); return; } diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 40334883c5..54baec6c6f 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -272,55 +272,27 @@ void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, { int nHashType = ParseSighashString(hashType); - bool fHashSingle = ((nHashType & ~SIGHASH_ANYONECANPAY) == SIGHASH_SINGLE); - // Script verification errors - UniValue vErrors(UniValue::VARR); - - // Use CTransaction for the constant parts of the - // transaction to avoid rehashing. - const CTransaction txConst(mtx); - // Sign what we can: - for (unsigned int i = 0; i < mtx.vin.size(); i++) { - CTxIn& txin = mtx.vin[i]; - auto coin = coins.find(txin.prevout); - if (coin == coins.end() || coin->second.IsSpent()) { - TxInErrorToJSON(txin, vErrors, "Input not found or already spent"); - continue; - } - const CScript& prevPubKey = coin->second.out.scriptPubKey; - const CAmount& amount = coin->second.out.nValue; - - SignatureData sigdata = DataFromTransaction(mtx, i, coin->second.out); - // Only sign SIGHASH_SINGLE if there's a corresponding output: - if (!fHashSingle || (i < mtx.vout.size())) { - ProduceSignature(*keystore, MutableTransactionSignatureCreator(&mtx, i, amount, nHashType), prevPubKey, sigdata); - } - - UpdateInput(txin, sigdata); + std::map<int, std::string> input_errors; - // amount must be specified for valid segwit signature - if (amount == MAX_MONEY && !txin.scriptWitness.IsNull()) { - throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Missing amount for %s", coin->second.out.ToString())); - } + bool complete = SignTransaction(mtx, keystore, coins, nHashType, input_errors); + SignTransactionResultToJSON(mtx, complete, coins, input_errors, result); +} - ScriptError serror = SCRIPT_ERR_OK; - if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) { - if (serror == SCRIPT_ERR_INVALID_STACK_OPERATION) { - // Unable to sign input and verification failed (possible attempt to partially sign). - TxInErrorToJSON(txin, vErrors, "Unable to sign input, invalid stack size (possibly missing key)"); - } else if (serror == SCRIPT_ERR_SIG_NULLFAIL) { - // Verification failed (possibly due to insufficient signatures). - TxInErrorToJSON(txin, vErrors, "CHECK(MULTI)SIG failing with non-zero signature (possibly need more signatures)"); - } else { - TxInErrorToJSON(txin, vErrors, ScriptErrorString(serror)); - } +void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map<COutPoint, Coin>& coins, std::map<int, std::string>& input_errors, UniValue& result) +{ + // Make errors UniValue + UniValue vErrors(UniValue::VARR); + for (const auto& err_pair : input_errors) { + if (err_pair.second == "Missing amount") { + // This particular error needs to be an exception for some reason + throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Missing amount for %s", coins.at(mtx.vin.at(err_pair.first).prevout).out.ToString())); } + TxInErrorToJSON(mtx.vin.at(err_pair.first), vErrors, err_pair.second); } - bool fComplete = vErrors.empty(); result.pushKV("hex", EncodeHexTx(CTransaction(mtx))); - result.pushKV("complete", fComplete); + result.pushKV("complete", complete); if (!vErrors.empty()) { if (result.exists("errors")) { vErrors.push_backV(result["errors"].getValues()); diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index 4750fd64ed..436db5dc60 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -6,6 +6,7 @@ #define BITCOIN_RPC_RAWTRANSACTION_UTIL_H #include <map> +#include <string> class FillableSigningProvider; class UniValue; @@ -24,6 +25,7 @@ class SigningProvider; * @param result JSON object where signed transaction results accumulate */ void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, const UniValue& hashType, UniValue& result); +void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map<COutPoint, Coin>& coins, std::map<int, std::string>& input_errors, UniValue& result); /** * Parse a prevtxs UniValue array and get the map of coins from it diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 58eae3ce96..fe8292fe57 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -465,3 +465,54 @@ bool IsSegWitOutput(const SigningProvider& provider, const CScript& script) } return false; } + +bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, int nHashType, std::map<int, std::string>& input_errors) +{ + bool fHashSingle = ((nHashType & ~SIGHASH_ANYONECANPAY) == SIGHASH_SINGLE); + + // Use CTransaction for the constant parts of the + // transaction to avoid rehashing. + const CTransaction txConst(mtx); + // Sign what we can: + for (unsigned int i = 0; i < mtx.vin.size(); i++) { + CTxIn& txin = mtx.vin[i]; + auto coin = coins.find(txin.prevout); + if (coin == coins.end() || coin->second.IsSpent()) { + input_errors[i] = "Input not found or already spent"; + continue; + } + const CScript& prevPubKey = coin->second.out.scriptPubKey; + const CAmount& amount = coin->second.out.nValue; + + SignatureData sigdata = DataFromTransaction(mtx, i, coin->second.out); + // Only sign SIGHASH_SINGLE if there's a corresponding output: + if (!fHashSingle || (i < mtx.vout.size())) { + ProduceSignature(*keystore, MutableTransactionSignatureCreator(&mtx, i, amount, nHashType), prevPubKey, sigdata); + } + + UpdateInput(txin, sigdata); + + // amount must be specified for valid segwit signature + if (amount == MAX_MONEY && !txin.scriptWitness.IsNull()) { + input_errors[i] = "Missing amount"; + continue; + } + + ScriptError serror = SCRIPT_ERR_OK; + if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) { + if (serror == SCRIPT_ERR_INVALID_STACK_OPERATION) { + // Unable to sign input and verification failed (possible attempt to partially sign). + input_errors[i] = "Unable to sign input, invalid stack size (possibly missing key)"; + } else if (serror == SCRIPT_ERR_SIG_NULLFAIL) { + // Verification failed (possibly due to insufficient signatures). + input_errors[i] = "CHECK(MULTI)SIG failing with non-zero signature (possibly need more signatures)"; + } else { + input_errors[i] = ScriptErrorString(serror); + } + } else { + // If this input succeeds, make sure there is no error set for it + input_errors.erase(i); + } + } + return input_errors.empty(); +} diff --git a/src/script/sign.h b/src/script/sign.h index 033c9ba19e..f03af0713f 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -6,6 +6,7 @@ #ifndef BITCOIN_SCRIPT_SIGN_H #define BITCOIN_SCRIPT_SIGN_H +#include <coins.h> #include <hash.h> #include <pubkey.h> #include <script/interpreter.h> @@ -168,4 +169,7 @@ bool IsSolvable(const SigningProvider& provider, const CScript& script); /** Check whether a scriptPubKey is known to be segwit. */ bool IsSegWitOutput(const SigningProvider& provider, const CScript& script); +/** Sign the CMutableTransaction */ +bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* provider, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors); + #endif // BITCOIN_SCRIPT_SIGN_H diff --git a/src/test/fuzz/bloom_filter.cpp b/src/test/fuzz/bloom_filter.cpp index b78744d9df..d1112f8e62 100644 --- a/src/test/fuzz/bloom_filter.cpp +++ b/src/test/fuzz/bloom_filter.cpp @@ -27,7 +27,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) while (fuzzed_data_provider.remaining_bytes() > 0) { switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 6)) { case 0: { - const std::vector<unsigned char>& b = ConsumeRandomLengthByteVector(fuzzed_data_provider); + const std::vector<unsigned char> b = ConsumeRandomLengthByteVector(fuzzed_data_provider); (void)bloom_filter.contains(b); bloom_filter.insert(b); const bool present = bloom_filter.contains(b); diff --git a/src/test/fuzz/rolling_bloom_filter.cpp b/src/test/fuzz/rolling_bloom_filter.cpp index ce69c4e8da..3b37321977 100644 --- a/src/test/fuzz/rolling_bloom_filter.cpp +++ b/src/test/fuzz/rolling_bloom_filter.cpp @@ -24,7 +24,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) while (fuzzed_data_provider.remaining_bytes() > 0) { switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 2)) { case 0: { - const std::vector<unsigned char>& b = ConsumeRandomLengthByteVector(fuzzed_data_provider); + const std::vector<unsigned char> b = ConsumeRandomLengthByteVector(fuzzed_data_provider); (void)rolling_bloom_filter.contains(b); rolling_bloom_filter.insert(b); const bool present = rolling_bloom_filter.contains(b); diff --git a/src/test/fuzz/script_ops.cpp b/src/test/fuzz/script_ops.cpp new file mode 100644 index 0000000000..0cd129ba7a --- /dev/null +++ b/src/test/fuzz/script_ops.cpp @@ -0,0 +1,67 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <script/script.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <string> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + CScript script = ConsumeScript(fuzzed_data_provider); + while (fuzzed_data_provider.remaining_bytes() > 0) { + switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 7)) { + case 0: + script += ConsumeScript(fuzzed_data_provider); + break; + case 1: + script = script + ConsumeScript(fuzzed_data_provider); + break; + case 2: + script << fuzzed_data_provider.ConsumeIntegral<int64_t>(); + break; + case 3: + script << ConsumeOpcodeType(fuzzed_data_provider); + break; + case 4: + script << ConsumeScriptNum(fuzzed_data_provider); + break; + case 5: + script << ConsumeRandomLengthByteVector(fuzzed_data_provider); + break; + case 6: + script.clear(); + break; + case 7: { + (void)script.GetSigOpCount(false); + (void)script.GetSigOpCount(true); + (void)script.GetSigOpCount(script); + (void)script.HasValidOps(); + (void)script.IsPayToScriptHash(); + (void)script.IsPayToWitnessScriptHash(); + (void)script.IsPushOnly(); + (void)script.IsUnspendable(); + { + CScript::const_iterator pc = script.begin(); + opcodetype opcode; + (void)script.GetOp(pc, opcode); + std::vector<uint8_t> data; + (void)script.GetOp(pc, opcode, data); + (void)script.IsPushOnly(pc); + } + { + int version; + std::vector<uint8_t> program; + (void)script.IsWitnessProgram(version, program); + } + break; + } + } + } +} diff --git a/src/test/fuzz/scriptnum_ops.cpp b/src/test/fuzz/scriptnum_ops.cpp new file mode 100644 index 0000000000..db44bb9e19 --- /dev/null +++ b/src/test/fuzz/scriptnum_ops.cpp @@ -0,0 +1,137 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <script/script.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cassert> +#include <cstdint> +#include <limits> +#include <vector> + +namespace { +bool IsValidAddition(const CScriptNum& lhs, const CScriptNum& rhs) +{ + return rhs == 0 || (rhs > 0 && lhs <= CScriptNum{std::numeric_limits<int64_t>::max()} - rhs) || (rhs < 0 && lhs >= CScriptNum{std::numeric_limits<int64_t>::min()} - rhs); +} + +bool IsValidSubtraction(const CScriptNum& lhs, const CScriptNum& rhs) +{ + return rhs == 0 || (rhs > 0 && lhs >= CScriptNum{std::numeric_limits<int64_t>::min()} + rhs) || (rhs < 0 && lhs <= CScriptNum{std::numeric_limits<int64_t>::max()} + rhs); +} +} // namespace + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + CScriptNum script_num = ConsumeScriptNum(fuzzed_data_provider); + while (fuzzed_data_provider.remaining_bytes() > 0) { + switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 11)) { + case 0: { + const int64_t i = fuzzed_data_provider.ConsumeIntegral<int64_t>(); + assert((script_num == i) != (script_num != i)); + assert((script_num <= i) != script_num > i); + assert((script_num >= i) != (script_num < i)); + // Avoid signed integer overflow: + // script/script.h:264:93: runtime error: signed integer overflow: -2261405121394637306 + -9223372036854775802 cannot be represented in type 'long' + if (IsValidAddition(script_num, CScriptNum{i})) { + assert((script_num + i) - i == script_num); + } + // Avoid signed integer overflow: + // script/script.h:265:93: runtime error: signed integer overflow: 9223371895120855039 - -9223372036854710486 cannot be represented in type 'long' + if (IsValidSubtraction(script_num, CScriptNum{i})) { + assert((script_num - i) + i == script_num); + } + break; + } + case 1: { + const CScriptNum random_script_num = ConsumeScriptNum(fuzzed_data_provider); + assert((script_num == random_script_num) != (script_num != random_script_num)); + assert((script_num <= random_script_num) != (script_num > random_script_num)); + assert((script_num >= random_script_num) != (script_num < random_script_num)); + // Avoid signed integer overflow: + // script/script.h:264:93: runtime error: signed integer overflow: -9223126527765971126 + -9223372036854756825 cannot be represented in type 'long' + if (IsValidAddition(script_num, random_script_num)) { + assert((script_num + random_script_num) - random_script_num == script_num); + } + // Avoid signed integer overflow: + // script/script.h:265:93: runtime error: signed integer overflow: 6052837899185946624 - -9223372036854775808 cannot be represented in type 'long' + if (IsValidSubtraction(script_num, random_script_num)) { + assert((script_num - random_script_num) + random_script_num == script_num); + } + break; + } + case 2: { + const CScriptNum random_script_num = ConsumeScriptNum(fuzzed_data_provider); + if (!IsValidAddition(script_num, random_script_num)) { + // Avoid assertion failure: + // ./script/script.h:292: CScriptNum &CScriptNum::operator+=(const int64_t &): Assertion `rhs == 0 || (rhs > 0 && m_value <= std::numeric_limits<int64_t>::max() - rhs) || (rhs < 0 && m_value >= std::numeric_limits<int64_t>::min() - rhs)' failed. + break; + } + script_num += random_script_num; + break; + } + case 3: { + const CScriptNum random_script_num = ConsumeScriptNum(fuzzed_data_provider); + if (!IsValidSubtraction(script_num, random_script_num)) { + // Avoid assertion failure: + // ./script/script.h:300: CScriptNum &CScriptNum::operator-=(const int64_t &): Assertion `rhs == 0 || (rhs > 0 && m_value >= std::numeric_limits<int64_t>::min() + rhs) || (rhs < 0 && m_value <= std::numeric_limits<int64_t>::max() + rhs)' failed. + break; + } + script_num -= random_script_num; + break; + } + case 4: + script_num = script_num & fuzzed_data_provider.ConsumeIntegral<int64_t>(); + break; + case 5: + script_num = script_num & ConsumeScriptNum(fuzzed_data_provider); + break; + case 6: + script_num &= ConsumeScriptNum(fuzzed_data_provider); + break; + case 7: + if (script_num == CScriptNum{std::numeric_limits<int64_t>::min()}) { + // Avoid assertion failure: + // ./script/script.h:279: CScriptNum CScriptNum::operator-() const: Assertion `m_value != std::numeric_limits<int64_t>::min()' failed. + break; + } + script_num = -script_num; + break; + case 8: + script_num = fuzzed_data_provider.ConsumeIntegral<int64_t>(); + break; + case 9: { + const int64_t random_integer = fuzzed_data_provider.ConsumeIntegral<int64_t>(); + if (!IsValidAddition(script_num, CScriptNum{random_integer})) { + // Avoid assertion failure: + // ./script/script.h:292: CScriptNum &CScriptNum::operator+=(const int64_t &): Assertion `rhs == 0 || (rhs > 0 && m_value <= std::numeric_limits<int64_t>::max() - rhs) || (rhs < 0 && m_value >= std::numeric_limits<int64_t>::min() - rhs)' failed. + break; + } + script_num += random_integer; + break; + } + case 10: { + const int64_t random_integer = fuzzed_data_provider.ConsumeIntegral<int64_t>(); + if (!IsValidSubtraction(script_num, CScriptNum{random_integer})) { + // Avoid assertion failure: + // ./script/script.h:300: CScriptNum &CScriptNum::operator-=(const int64_t &): Assertion `rhs == 0 || (rhs > 0 && m_value >= std::numeric_limits<int64_t>::min() + rhs) || (rhs < 0 && m_value <= std::numeric_limits<int64_t>::max() + rhs)' failed. + break; + } + script_num -= random_integer; + break; + } + case 11: + script_num &= fuzzed_data_provider.ConsumeIntegral<int64_t>(); + break; + } + // Avoid negation failure: + // script/script.h:332:35: runtime error: negation of -9223372036854775808 cannot be represented in type 'int64_t' (aka 'long'); cast to an unsigned type to negate this value to itself + if (script_num != CScriptNum{std::numeric_limits<int64_t>::min()}) { + (void)script_num.getvch(); + } + } +} diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 62907c7e0b..2f4aa9ad2b 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -7,9 +7,11 @@ #include <attributes.h> #include <optional.h> +#include <script/script.h> #include <serialize.h> #include <streams.h> #include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> #include <version.h> #include <cstdint> @@ -25,7 +27,7 @@ NODISCARD inline std::vector<uint8_t> ConsumeRandomLengthByteVector(FuzzedDataPr template <typename T> NODISCARD inline Optional<T> ConsumeDeserializable(FuzzedDataProvider& fuzzed_data_provider, size_t max_length = 4096) noexcept { - const std::vector<uint8_t>& buffer = ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length); + const std::vector<uint8_t> buffer = ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length); CDataStream ds{buffer, SER_NETWORK, INIT_PROTO_VERSION}; T obj; try { @@ -36,4 +38,20 @@ NODISCARD inline Optional<T> ConsumeDeserializable(FuzzedDataProvider& fuzzed_da return obj; } +NODISCARD inline opcodetype ConsumeOpcodeType(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + return static_cast<opcodetype>(fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(0, MAX_OPCODE)); +} + +NODISCARD inline CScript ConsumeScript(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + const std::vector<uint8_t> b = ConsumeRandomLengthByteVector(fuzzed_data_provider); + return {b.begin(), b.end()}; +} + +NODISCARD inline CScriptNum ConsumeScriptNum(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + return CScriptNum{fuzzed_data_provider.ConsumeIntegral<int64_t>()}; +} + #endif // BITCOIN_TEST_FUZZ_UTIL_H diff --git a/src/util/message.cpp b/src/util/message.cpp index 17603a43d2..1e7128d225 100644 --- a/src/util/message.cpp +++ b/src/util/message.cpp @@ -76,3 +76,17 @@ uint256 MessageHash(const std::string& message) return hasher.GetHash(); } + +std::string SigningResultString(const SigningResult res) +{ + switch (res) { + case SigningResult::OK: + return "No error"; + case SigningResult::PRIVATE_KEY_NOT_AVAILABLE: + return "Private key not available"; + case SigningResult::SIGNING_FAILED: + return "Sign failed"; + // no default case, so the compiler can warn about missing cases + } + assert(false); +} diff --git a/src/util/message.h b/src/util/message.h index 01fd14ce2d..b31c5f5761 100644 --- a/src/util/message.h +++ b/src/util/message.h @@ -39,6 +39,12 @@ enum class MessageVerificationResult { OK }; +enum class SigningResult { + OK, //!< No error + PRIVATE_KEY_NOT_AVAILABLE, + SIGNING_FAILED, +}; + /** Verify a signed message. * @param[in] address Signer's bitcoin address, it must refer to a public key. * @param[in] signature The signature in base64 format. @@ -65,4 +71,6 @@ bool MessageSign( */ uint256 MessageHash(const std::string& message); +std::string SigningResultString(const SigningResult res); + #endif // BITCOIN_UTIL_MESSAGE_H diff --git a/src/wallet/psbtwallet.cpp b/src/wallet/psbtwallet.cpp deleted file mode 100644 index d995fb06d4..0000000000 --- a/src/wallet/psbtwallet.cpp +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright (c) 2009-2019 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include <wallet/psbtwallet.h> - -TransactionError FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, bool& complete, int sighash_type, bool sign, bool bip32derivs) -{ - LOCK(pwallet->cs_wallet); - // Get all of the previous transactions - complete = true; - for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { - const CTxIn& txin = psbtx.tx->vin[i]; - PSBTInput& input = psbtx.inputs.at(i); - - if (PSBTInputSigned(input)) { - continue; - } - - // Verify input looks sane. This will check that we have at most one uxto, witness or non-witness. - if (!input.IsSane()) { - return TransactionError::INVALID_PSBT; - } - - // If we have no utxo, grab it from the wallet. - if (!input.non_witness_utxo && input.witness_utxo.IsNull()) { - const uint256& txhash = txin.prevout.hash; - const auto it = pwallet->mapWallet.find(txhash); - if (it != pwallet->mapWallet.end()) { - const CWalletTx& wtx = it->second; - // We only need the non_witness_utxo, which is a superset of the witness_utxo. - // The signing code will switch to the smaller witness_utxo if this is ok. - input.non_witness_utxo = wtx.tx; - } - } - - // Get the Sighash type - if (sign && input.sighash_type > 0 && input.sighash_type != sighash_type) { - return TransactionError::SIGHASH_MISMATCH; - } - - // Get the scriptPubKey to know which SigningProvider to use - CScript script; - if (!input.witness_utxo.IsNull()) { - script = input.witness_utxo.scriptPubKey; - } else if (input.non_witness_utxo) { - if (txin.prevout.n >= input.non_witness_utxo->vout.size()) { - return TransactionError::MISSING_INPUTS; - } - script = input.non_witness_utxo->vout[txin.prevout.n].scriptPubKey; - } else { - // There's no UTXO so we can just skip this now - complete = false; - continue; - } - SignatureData sigdata; - input.FillSignatureData(sigdata); - std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(script, sigdata); - if (!provider) { - complete = false; - continue; - } - - complete &= SignPSBTInput(HidingSigningProvider(provider.get(), !sign, !bip32derivs), psbtx, i, sighash_type); - } - - // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change - for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { - const CTxOut& out = psbtx.tx->vout.at(i); - std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(out.scriptPubKey); - if (provider) { - UpdatePSBTOutput(HidingSigningProvider(provider.get(), true, !bip32derivs), psbtx, i); - } - } - - return TransactionError::OK; -} diff --git a/src/wallet/psbtwallet.h b/src/wallet/psbtwallet.h deleted file mode 100644 index 8285bb901c..0000000000 --- a/src/wallet/psbtwallet.h +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) 2009-2019 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#ifndef BITCOIN_WALLET_PSBTWALLET_H -#define BITCOIN_WALLET_PSBTWALLET_H - -#include <psbt.h> -#include <wallet/wallet.h> - -/** - * Fills out a PSBT with information from the wallet. Fills in UTXOs if we have - * them. Tries to sign if sign=true. Sets `complete` if the PSBT is now complete - * (i.e. has all required signatures or signature-parts, and is ready to - * finalize.) Sets `error` and returns false if something goes wrong. - * - * @param[in] pwallet pointer to a wallet - * @param[in] psbtx PartiallySignedTransaction to fill in - * @param[out] complete indicates whether the PSBT is now complete - * @param[in] sighash_type the sighash type to use when signing (if PSBT does not specify) - * @param[in] sign whether to sign or not - * @param[in] bip32derivs whether to fill in bip32 derivation information if available - * return error - */ -NODISCARD TransactionError FillPSBT(const CWallet* pwallet, - PartiallySignedTransaction& psbtx, - bool& complete, - int sighash_type = 1 /* SIGHASH_ALL */, - bool sign = true, - bool bip32derivs = true); - -#endif // BITCOIN_WALLET_PSBTWALLET_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index bbbaea88d1..44e580a52a 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -27,7 +27,6 @@ #include <util/vector.h> #include <wallet/coincontrol.h> #include <wallet/feebumper.h> -#include <wallet/psbtwallet.h> #include <wallet/rpcwallet.h> #include <wallet/wallet.h> #include <wallet/walletdb.h> @@ -566,22 +565,12 @@ static UniValue signmessage(const JSONRPCRequest& request) throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key"); } - CScript script_pub_key = GetScriptForDestination(*pkhash); - std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(script_pub_key); - if (!provider) { - throw JSONRPCError(RPC_WALLET_ERROR, "Private key not available"); - } - - CKey key; - CKeyID keyID(*pkhash); - if (!provider->GetKey(keyID, key)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Private key not available"); - } - std::string signature; - - if (!MessageSign(key, strMessage, signature)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Sign failed"); + SigningResult err = pwallet->SignMessage(strMessage, *pkhash, signature); + if (err == SigningResult::SIGNING_FAILED) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, SigningResultString(err)); + } else if (err != SigningResult::OK){ + throw JSONRPCError(RPC_WALLET_ERROR, SigningResultString(err)); } return signature; @@ -2973,7 +2962,7 @@ static UniValue listunspent(const JSONRPCRequest& request) entry.pushKV("label", i->second.name); } - std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(scriptPubKey); + std::unique_ptr<SigningProvider> provider = pwallet->GetSolvingProvider(scriptPubKey); if (provider) { if (scriptPubKey.IsPayToScriptHash()) { const CScriptID& hash = CScriptID(boost::get<ScriptHash>(address)); @@ -3013,7 +3002,7 @@ static UniValue listunspent(const JSONRPCRequest& request) entry.pushKV("spendable", out.fSpendable); entry.pushKV("solvable", out.fSolvable); if (out.fSolvable) { - std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(scriptPubKey); + std::unique_ptr<SigningProvider> provider = pwallet->GetSolvingProvider(scriptPubKey); if (provider) { auto descriptor = InferDescriptor(scriptPubKey, *provider); entry.pushKV("desc", descriptor->ToString()); @@ -3329,23 +3318,15 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) // Parse the prevtxs array ParsePrevouts(request.params[1], nullptr, coins); - std::set<std::shared_ptr<SigningProvider>> providers; - for (const std::pair<COutPoint, Coin> coin_pair : coins) { - std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(coin_pair.second.out.scriptPubKey); - if (provider) { - providers.insert(std::move(provider)); - } - } - if (providers.size() == 0) { - // When there are no available providers, use a dummy SigningProvider so we can check if the tx is complete - providers.insert(std::make_shared<SigningProvider>()); - } + int nHashType = ParseSighashString(request.params[2]); + // Script verification errors + std::map<int, std::string> input_errors; + + bool complete = pwallet->SignTransaction(mtx, coins, nHashType, input_errors); UniValue result(UniValue::VOBJ); - for (std::shared_ptr<SigningProvider> provider : providers) { - SignTransaction(mtx, provider.get(), coins, request.params[2], result); - } - return result; + SignTransactionResultToJSON(mtx, complete, coins, input_errors, result); + return result; } static UniValue bumpfee(const JSONRPCRequest& request) @@ -3524,7 +3505,7 @@ static UniValue bumpfee(const JSONRPCRequest& request) } else { PartiallySignedTransaction psbtx(mtx); bool complete = false; - const TransactionError err = FillPSBT(pwallet, psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */); + const TransactionError err = pwallet->FillPSBT(psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */); CHECK_NONFATAL(err == TransactionError::OK); CHECK_NONFATAL(!complete); CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); @@ -3735,7 +3716,7 @@ static UniValue DescribeWalletAddress(const CWallet* const pwallet, const CTxDes CScript script = GetScriptForDestination(dest); std::unique_ptr<SigningProvider> provider = nullptr; if (pwallet) { - provider = pwallet->GetSigningProvider(script); + provider = pwallet->GetSolvingProvider(script); } ret.pushKVs(detail); ret.pushKVs(boost::apply_visitor(DescribeWalletAddressVisitor(provider.get()), dest)); @@ -3837,7 +3818,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) CScript scriptPubKey = GetScriptForDestination(dest); ret.pushKV("scriptPubKey", HexStr(scriptPubKey.begin(), scriptPubKey.end())); - std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(scriptPubKey); + std::unique_ptr<SigningProvider> provider = pwallet->GetSolvingProvider(scriptPubKey); isminetype mine = pwallet->IsMine(dest); ret.pushKV("ismine", bool(mine & ISMINE_SPENDABLE)); @@ -4141,7 +4122,7 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request) bool sign = request.params[1].isNull() ? true : request.params[1].get_bool(); bool bip32derivs = request.params[3].isNull() ? true : request.params[3].get_bool(); bool complete = true; - const TransactionError err = FillPSBT(pwallet, psbtx, complete, nHashType, sign, bip32derivs); + const TransactionError err = pwallet->FillPSBT(psbtx, complete, nHashType, sign, bip32derivs); if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } @@ -4264,7 +4245,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) // Fill transaction with out data but don't sign bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool(); bool complete = true; - const TransactionError err = FillPSBT(pwallet, psbtx, complete, 1, false, bip32derivs); + const TransactionError err = pwallet->FillPSBT(psbtx, complete, 1, false, bip32derivs); if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 8851d0af31..6fe1d84d64 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -5,6 +5,7 @@ #include <key_io.h> #include <outputtype.h> #include <script/descriptor.h> +#include <script/sign.h> #include <util/bip32.h> #include <util/strencodings.h> #include <util/translation.h> @@ -477,7 +478,7 @@ int64_t LegacyScriptPubKeyMan::GetTimeFirstKey() const return nTimeFirstKey; } -std::unique_ptr<SigningProvider> LegacyScriptPubKeyMan::GetSigningProvider(const CScript& script) const +std::unique_ptr<SigningProvider> LegacyScriptPubKeyMan::GetSolvingProvider(const CScript& script) const { return MakeUnique<LegacySigningProvider>(*this); } @@ -505,6 +506,67 @@ bool LegacyScriptPubKeyMan::CanProvide(const CScript& script, SignatureData& sig } } +bool LegacyScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const +{ + return ::SignTransaction(tx, this, coins, sighash, input_errors); +} + +SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const +{ + CKeyID key_id(pkhash); + CKey key; + if (!GetKey(key_id, key)) { + return SigningResult::PRIVATE_KEY_NOT_AVAILABLE; + } + + if (MessageSign(key, message, str_sig)) { + return SigningResult::OK; + } + return SigningResult::SIGNING_FAILED; +} + +TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs) const +{ + for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + const CTxIn& txin = psbtx.tx->vin[i]; + PSBTInput& input = psbtx.inputs.at(i); + + if (PSBTInputSigned(input)) { + continue; + } + + // Verify input looks sane. This will check that we have at most one uxto, witness or non-witness. + if (!input.IsSane()) { + return TransactionError::INVALID_PSBT; + } + + // Get the Sighash type + if (sign && input.sighash_type > 0 && input.sighash_type != sighash_type) { + return TransactionError::SIGHASH_MISMATCH; + } + + // Check non_witness_utxo has specified prevout + if (input.non_witness_utxo) { + if (txin.prevout.n >= input.non_witness_utxo->vout.size()) { + return TransactionError::MISSING_INPUTS; + } + } else if (input.witness_utxo.IsNull()) { + // There's no UTXO so we can just skip this now + continue; + } + SignatureData sigdata; + input.FillSignatureData(sigdata); + SignPSBTInput(HidingSigningProvider(this, !sign, !bip32derivs), psbtx, i, sighash_type); + } + + // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change + for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { + UpdatePSBTOutput(HidingSigningProvider(this, true, !bip32derivs), psbtx, i); + } + + return TransactionError::OK; +} + const CKeyMetadata* LegacyScriptPubKeyMan::GetMetadata(const CTxDestination& dest) const { LOCK(cs_KeyStore); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 1a0d34a445..8512eadf31 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -5,8 +5,11 @@ #ifndef BITCOIN_WALLET_SCRIPTPUBKEYMAN_H #define BITCOIN_WALLET_SCRIPTPUBKEYMAN_H +#include <psbt.h> #include <script/signingprovider.h> #include <script/standard.h> +#include <util/error.h> +#include <util/message.h> #include <wallet/crypter.h> #include <wallet/ismine.h> #include <wallet/walletdb.h> @@ -203,13 +206,20 @@ public: virtual const CKeyMetadata* GetMetadata(const CTxDestination& dest) const { return nullptr; } - virtual std::unique_ptr<SigningProvider> GetSigningProvider(const CScript& script) const { return nullptr; } + virtual std::unique_ptr<SigningProvider> GetSolvingProvider(const CScript& script) const { return nullptr; } - /** Whether this ScriptPubKeyMan can provide a SigningProvider (via GetSigningProvider) that, combined with - * sigdata, can produce a valid signature. + /** Whether this ScriptPubKeyMan can provide a SigningProvider (via GetSolvingProvider) that, combined with + * sigdata, can produce solving data. */ virtual bool CanProvide(const CScript& script, SignatureData& sigdata) { return false; } + /** Creates new signatures and adds them to the transaction. Returns whether all inputs were signed */ + virtual bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const { return false; } + /** Sign a message with the given script */ + virtual SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const { return SigningResult::SIGNING_FAILED; }; + /** Adds script and derivation path information to a PSBT, and optionally signs it. */ + virtual TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false) const { return TransactionError::INVALID_PSBT; } + virtual uint256 GetID() const { return uint256(); } /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */ @@ -346,10 +356,14 @@ public: bool CanGetAddresses(bool internal = false) const override; - std::unique_ptr<SigningProvider> GetSigningProvider(const CScript& script) const override; + std::unique_ptr<SigningProvider> GetSolvingProvider(const CScript& script) const override; bool CanProvide(const CScript& script, SignatureData& sigdata) override; + bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const override; + SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override; + TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false) const override; + uint256 GetID() const override; // Map from Key ID to key metadata. @@ -447,7 +461,7 @@ public: std::set<CKeyID> GetKeys() const override; }; -/** Wraps a LegacyScriptPubKeyMan so that it can be returned in a new unique_ptr */ +/** Wraps a LegacyScriptPubKeyMan so that it can be returned in a new unique_ptr. Does not provide privkeys */ class LegacySigningProvider : public SigningProvider { private: @@ -458,8 +472,8 @@ public: bool GetCScript(const CScriptID &scriptid, CScript& script) const override { return m_spk_man.GetCScript(scriptid, script); } bool HaveCScript(const CScriptID &scriptid) const override { return m_spk_man.HaveCScript(scriptid); } bool GetPubKey(const CKeyID &address, CPubKey& pubkey) const override { return m_spk_man.GetPubKey(address, pubkey); } - bool GetKey(const CKeyID &address, CKey& key) const override { return m_spk_man.GetKey(address, key); } - bool HaveKey(const CKeyID &address) const override { return m_spk_man.HaveKey(address); } + bool GetKey(const CKeyID &address, CKey& key) const override { return false; } + bool HaveKey(const CKeyID &address) const override { return false; } bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const override { return m_spk_man.GetKeyOrigin(keyid, info); } }; diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index f923de6178..8b7b7af21d 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -5,7 +5,6 @@ #include <key_io.h> #include <util/bip32.h> #include <util/strencodings.h> -#include <wallet/psbtwallet.h> #include <wallet/wallet.h> #include <boost/test/unit_test.hpp> @@ -61,7 +60,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) // Fill transaction with our data bool complete = true; - BOOST_REQUIRE_EQUAL(TransactionError::OK, FillPSBT(&m_wallet, psbtx, complete, SIGHASH_ALL, false, true)); + BOOST_REQUIRE_EQUAL(TransactionError::OK, m_wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, false, true)); // Get the final tx CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); @@ -74,9 +73,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) // Try to sign the mutated input SignatureData sigdata; - psbtx.inputs[0].FillSignatureData(sigdata); - const std::unique_ptr<SigningProvider> provider = m_wallet.GetSigningProvider(ws1, sigdata); - BOOST_CHECK(!SignPSBTInput(*provider, psbtx, 0, SIGHASH_ALL)); + BOOST_CHECK(spk_man->FillPSBT(psbtx, SIGHASH_ALL, true, true) != TransactionError::OK); } BOOST_AUTO_TEST_CASE(parse_hd_keypath) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9d2e5598e8..79e29d050f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1407,7 +1407,7 @@ bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig const CScript& scriptPubKey = txout.scriptPubKey; SignatureData sigdata; - std::unique_ptr<SigningProvider> provider = GetSigningProvider(scriptPubKey); + std::unique_ptr<SigningProvider> provider = GetSolvingProvider(scriptPubKey); if (!provider) { // We don't know about this scriptpbuKey; return false; @@ -2171,7 +2171,7 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector< continue; } - std::unique_ptr<SigningProvider> provider = GetSigningProvider(wtx.tx->vout[i].scriptPubKey); + std::unique_ptr<SigningProvider> provider = GetSolvingProvider(wtx.tx->vout[i].scriptPubKey); bool solvable = provider ? IsSolvable(*provider, wtx.tx->vout[i].scriptPubKey) : false; bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable)); @@ -2410,34 +2410,172 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm return res; } -bool CWallet::SignTransaction(CMutableTransaction& tx) +bool CWallet::SignTransaction(CMutableTransaction& tx) const { AssertLockHeld(cs_wallet); - // sign the new tx - int nIn = 0; + // Build coins map + std::map<COutPoint, Coin> coins; for (auto& input : tx.vin) { std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(input.prevout.hash); if(mi == mapWallet.end() || input.prevout.n >= mi->second.tx->vout.size()) { return false; } - const CScript& scriptPubKey = mi->second.tx->vout[input.prevout.n].scriptPubKey; - const CAmount& amount = mi->second.tx->vout[input.prevout.n].nValue; + const CWalletTx& wtx = mi->second; + coins[input.prevout] = Coin(wtx.tx->vout[input.prevout.n], wtx.m_confirm.block_height, wtx.IsCoinBase()); + } + std::map<int, std::string> input_errors; + return SignTransaction(tx, coins, SIGHASH_ALL, input_errors); +} + +bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const +{ + // Sign the tx with ScriptPubKeyMans + // Because each ScriptPubKeyMan can sign more than one input, we need to keep track of each ScriptPubKeyMan that has signed this transaction. + // Each iteration, we may sign more txins than the txin that is specified in that iteration. + // We assume that each input is signed by only one ScriptPubKeyMan. + std::set<uint256> visited_spk_mans; + for (unsigned int i = 0; i < tx.vin.size(); i++) { + // Get the prevout + CTxIn& txin = tx.vin[i]; + auto coin = coins.find(txin.prevout); + if (coin == coins.end() || coin->second.IsSpent()) { + input_errors[i] = "Input not found or already spent"; + continue; + } + + // Check if this input is complete + SignatureData sigdata = DataFromTransaction(tx, i, coin->second.out); + if (sigdata.complete) { + continue; + } + + // Input needs to be signed, find the right ScriptPubKeyMan + std::set<ScriptPubKeyMan*> spk_mans = GetScriptPubKeyMans(coin->second.out.scriptPubKey, sigdata); + if (spk_mans.size() == 0) { + input_errors[i] = "Unable to sign input, missing keys"; + continue; + } + + for (auto& spk_man : spk_mans) { + // If we've already been signed by this spk_man, skip it + if (visited_spk_mans.count(spk_man->GetID()) > 0) { + continue; + } + + // Sign the tx. + // spk_man->SignTransaction will return true if the transaction is complete, + // so we can exit early and return true if that happens. + if (spk_man->SignTransaction(tx, coins, sighash, input_errors)) { + return true; + } + + // Add this spk_man to visited_spk_mans so we can skip it later + visited_spk_mans.insert(spk_man->GetID()); + } + } + return false; +} + +TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, int sighash_type, bool sign, bool bip32derivs) const +{ + LOCK(cs_wallet); + // Get all of the previous transactions + for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + const CTxIn& txin = psbtx.tx->vin[i]; + PSBTInput& input = psbtx.inputs.at(i); + + if (PSBTInputSigned(input)) { + continue; + } + + // Verify input looks sane. This will check that we have at most one uxto, witness or non-witness. + if (!input.IsSane()) { + return TransactionError::INVALID_PSBT; + } + + // If we have no utxo, grab it from the wallet. + if (!input.non_witness_utxo && input.witness_utxo.IsNull()) { + const uint256& txhash = txin.prevout.hash; + const auto it = mapWallet.find(txhash); + if (it != mapWallet.end()) { + const CWalletTx& wtx = it->second; + // We only need the non_witness_utxo, which is a superset of the witness_utxo. + // The signing code will switch to the smaller witness_utxo if this is ok. + input.non_witness_utxo = wtx.tx; + } + } + } + + // Fill in information from ScriptPubKeyMans + // Because each ScriptPubKeyMan may be able to fill more than one input, we need to keep track of each ScriptPubKeyMan that has filled this psbt. + // Each iteration, we may fill more inputs than the input that is specified in that iteration. + // We assume that each input is filled by only one ScriptPubKeyMan + std::set<uint256> visited_spk_mans; + for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + const CTxIn& txin = psbtx.tx->vin[i]; + PSBTInput& input = psbtx.inputs.at(i); + + if (PSBTInputSigned(input)) { + continue; + } + + // Get the scriptPubKey to know which ScriptPubKeyMan to use + CScript script; + if (!input.witness_utxo.IsNull()) { + script = input.witness_utxo.scriptPubKey; + } else if (input.non_witness_utxo) { + if (txin.prevout.n >= input.non_witness_utxo->vout.size()) { + return TransactionError::MISSING_INPUTS; + } + script = input.non_witness_utxo->vout[txin.prevout.n].scriptPubKey; + } else { + // There's no UTXO so we can just skip this now + continue; + } SignatureData sigdata; + input.FillSignatureData(sigdata); + std::set<ScriptPubKeyMan*> spk_mans = GetScriptPubKeyMans(script, sigdata); + if (spk_mans.size() == 0) { + continue; + } - std::unique_ptr<SigningProvider> provider = GetSigningProvider(scriptPubKey); - if (!provider) { - // We don't know about this scriptpbuKey; - return false; + for (auto& spk_man : spk_mans) { + // If we've already been signed by this spk_man, skip it + if (visited_spk_mans.count(spk_man->GetID()) > 0) { + continue; + } + + // Fill in the information from the spk_man + TransactionError res = spk_man->FillPSBT(psbtx, sighash_type, sign, bip32derivs); + if (res != TransactionError::OK) { + return res; + } + + // Add this spk_man to visited_spk_mans so we can skip it later + visited_spk_mans.insert(spk_man->GetID()); } + } - if (!ProduceSignature(*provider, MutableTransactionSignatureCreator(&tx, nIn, amount, SIGHASH_ALL), scriptPubKey, sigdata)) { - return false; + // Complete if every input is now signed + complete = true; + for (const auto& input : psbtx.inputs) { + complete &= PSBTInputSigned(input); + } + + return TransactionError::OK; +} + +SigningResult CWallet::SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const +{ + SignatureData sigdata; + CScript script_pub_key = GetScriptForDestination(pkhash); + for (const auto& spk_man_pair : m_spk_managers) { + if (spk_man_pair.second->CanProvide(script_pub_key, sigdata)) { + return spk_man_pair.second->SignMessage(message, pkhash, str_sig); } - UpdateInput(input, sigdata); - nIn++; } - return true; + return SigningResult::PRIVATE_KEY_NOT_AVAILABLE; } bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl coinControl) @@ -2886,25 +3024,9 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std txNew.vin.push_back(CTxIn(coin.outpoint, CScript(), nSequence)); } - if (sign) - { - int nIn = 0; - for (const auto& coin : selected_coins) - { - const CScript& scriptPubKey = coin.txout.scriptPubKey; - SignatureData sigdata; - - std::unique_ptr<SigningProvider> provider = GetSigningProvider(scriptPubKey); - if (!provider || !ProduceSignature(*provider, MutableTransactionSignatureCreator(&txNew, nIn, coin.txout.nValue, SIGHASH_ALL), scriptPubKey, sigdata)) - { - strFailReason = _("Signing transaction failed").translated; - return false; - } else { - UpdateInput(txNew.vin.at(nIn), sigdata); - } - - nIn++; - } + if (sign && !SignTransaction(txNew)) { + strFailReason = _("Signing transaction failed").translated; + return false; } // Return the constructed transaction data. @@ -4155,6 +4277,17 @@ ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(const OutputType& type, bool intern return it->second; } +std::set<ScriptPubKeyMan*> CWallet::GetScriptPubKeyMans(const CScript& script, SignatureData& sigdata) const +{ + std::set<ScriptPubKeyMan*> spk_mans; + for (const auto& spk_man_pair : m_spk_managers) { + if (spk_man_pair.second->CanProvide(script, sigdata)) { + spk_mans.insert(spk_man_pair.second.get()); + } + } + return spk_mans; +} + ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(const CScript& script) const { SignatureData sigdata; @@ -4174,17 +4307,17 @@ ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(const uint256& id) const return nullptr; } -std::unique_ptr<SigningProvider> CWallet::GetSigningProvider(const CScript& script) const +std::unique_ptr<SigningProvider> CWallet::GetSolvingProvider(const CScript& script) const { SignatureData sigdata; - return GetSigningProvider(script, sigdata); + return GetSolvingProvider(script, sigdata); } -std::unique_ptr<SigningProvider> CWallet::GetSigningProvider(const CScript& script, SignatureData& sigdata) const +std::unique_ptr<SigningProvider> CWallet::GetSolvingProvider(const CScript& script, SignatureData& sigdata) const { for (const auto& spk_man_pair : m_spk_managers) { if (spk_man_pair.second->CanProvide(script, sigdata)) { - return spk_man_pair.second->GetSigningProvider(script); + return spk_man_pair.second->GetSolvingProvider(script); } } return nullptr; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3664a6787c..0c86a0c1e8 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -11,8 +11,10 @@ #include <interfaces/handler.h> #include <outputtype.h> #include <policy/feerate.h> +#include <psbt.h> #include <tinyformat.h> #include <ui_interface.h> +#include <util/message.h> #include <util/strencodings.h> #include <util/system.h> #include <validationinterface.h> @@ -916,7 +918,30 @@ public: * calling CreateTransaction(); */ bool FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl); - bool SignTransaction(CMutableTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + // Fetch the inputs and sign with SIGHASH_ALL. + bool SignTransaction(CMutableTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + // Sign the tx given the input coins and sighash. + bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const; + SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const; + + /** + * Fills out a PSBT with information from the wallet. Fills in UTXOs if we have + * them. Tries to sign if sign=true. Sets `complete` if the PSBT is now complete + * (i.e. has all required signatures or signature-parts, and is ready to + * finalize.) Sets `error` and returns false if something goes wrong. + * + * @param[in] psbtx PartiallySignedTransaction to fill in + * @param[out] complete indicates whether the PSBT is now complete + * @param[in] sighash_type the sighash type to use when signing (if PSBT does not specify) + * @param[in] sign whether to sign or not + * @param[in] bip32derivs whether to fill in bip32 derivation information if available + * return error + */ + TransactionError FillPSBT(PartiallySignedTransaction& psbtx, + bool& complete, + int sighash_type = 1 /* SIGHASH_ALL */, + bool sign = true, + bool bip32derivs = true) const; /** * Create a new transaction paying the recipients with a set of coins @@ -1153,9 +1178,12 @@ public: //! Get the ScriptPubKeyMan by id ScriptPubKeyMan* GetScriptPubKeyMan(const uint256& id) const; + //! Get all of the ScriptPubKeyMans for a script given additional information in sigdata (populated by e.g. a psbt) + std::set<ScriptPubKeyMan*> GetScriptPubKeyMans(const CScript& script, SignatureData& sigdata) const; + //! Get the SigningProvider for a script - std::unique_ptr<SigningProvider> GetSigningProvider(const CScript& script) const; - std::unique_ptr<SigningProvider> GetSigningProvider(const CScript& script, SignatureData& sigdata) const; + std::unique_ptr<SigningProvider> GetSolvingProvider(const CScript& script) const; + std::unique_ptr<SigningProvider> GetSolvingProvider(const CScript& script, SignatureData& sigdata) const; //! Get the LegacyScriptPubKeyMan which is used for all types, internal, and external. LegacyScriptPubKeyMan* GetLegacyScriptPubKeyMan() const; diff --git a/test/README.md b/test/README.md index c3e4ae9ad2..e1dab92a06 100644 --- a/test/README.md +++ b/test/README.md @@ -145,7 +145,7 @@ levels using the logger included in the test_framework, e.g. `test_framework.log` and no logs are output to the console. - when run directly, *all* logs are written to `test_framework.log` and INFO level and above are output to the console. -- when run on Travis, no logs are output to the console. However, if a test +- when run by [our CI (Continuous Integration)](/ci/README.md), no logs are output to the console. However, if a test fails, the `test_framework.log` and bitcoind `debug.log`s will all be dumped to the console to help troubleshooting. diff --git a/test/functional/README.md b/test/functional/README.md index 77a9ce9acb..6582c1cbcd 100644 --- a/test/functional/README.md +++ b/test/functional/README.md @@ -51,10 +51,13 @@ don't have test cases for. #### General test-writing advice +- Instead of inline comments or no test documentation at all, log the comments to the test log, e.g. + `self.log.info('Create enough transactions to fill a block')`. Logs make the test code easier to read and the test + logic easier [to debug](/test/README.md#test-logging). - Set `self.num_nodes` to the minimum number of nodes necessary for the test. Having additional unrequired nodes adds to the execution time of the test as well as memory/CPU/disk requirements (which is important when running tests in - parallel or on Travis). + parallel). - Avoid stop-starting the nodes multiple times during the test if possible. A stop-start takes several seconds, so doing it several times blows up the runtime of the test. diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index e2b347f925..073ed8d7c7 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -60,7 +60,7 @@ class BIP65Test(BitcoinTestFramework): '-acceptnonstdtxn=1', # cltv_invalidate is nonstandard ]] self.setup_clean_chain = True - self.rpc_timeout = 120 + self.rpc_timeout = 480 def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index 82c7e55245..909a43c8d9 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -72,6 +72,7 @@ class SegWitTest(BitcoinTestFramework): "-addresstype=legacy", ], ] + self.rpc_timeout = 120 def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/rpc_getdescriptorinfo.py b/test/functional/rpc_getdescriptorinfo.py new file mode 100755 index 0000000000..977dc805ef --- /dev/null +++ b/test/functional/rpc_getdescriptorinfo.py @@ -0,0 +1,65 @@ +#!/usr/bin/env python3 +# Copyright (c) 2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test getdescriptorinfo RPC. +""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.descriptors import descsum_create +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, +) + + +class DescriptorTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.extra_args = [["-disablewallet"]] + + def test_desc(self, desc, isrange, issolvable, hasprivatekeys): + info = self.nodes[0].getdescriptorinfo(desc) + assert_equal(info, self.nodes[0].getdescriptorinfo(descsum_create(desc))) + assert_equal(info['descriptor'], descsum_create(desc)) + assert_equal(info['isrange'], isrange) + assert_equal(info['issolvable'], issolvable) + assert_equal(info['hasprivatekeys'], hasprivatekeys) + + def run_test(self): + assert_raises_rpc_error(-1, 'getdescriptorinfo', self.nodes[0].getdescriptorinfo) + assert_raises_rpc_error(-3, 'Expected type string', self.nodes[0].getdescriptorinfo, 1) + assert_raises_rpc_error(-5, 'is not a valid descriptor function', self.nodes[0].getdescriptorinfo, '') + + # P2PK output with the specified public key. + self.test_desc('pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)', isrange=False, issolvable=True, hasprivatekeys=False) + # P2PKH output with the specified public key. + self.test_desc('pkh(02c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5)', isrange=False, issolvable=True, hasprivatekeys=False) + # P2WPKH output with the specified public key. + self.test_desc('wpkh(02f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9)', isrange=False, issolvable=True, hasprivatekeys=False) + # P2SH-P2WPKH output with the specified public key. + self.test_desc('sh(wpkh(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556))', isrange=False, issolvable=True, hasprivatekeys=False) + # Any P2PK, P2PKH, P2WPKH, or P2SH-P2WPKH output with the specified public key. + self.test_desc('combo(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)', isrange=False, issolvable=True, hasprivatekeys=False) + # An (overly complicated) P2SH-P2WSH-P2PKH output with the specified public key. + self.test_desc('sh(wsh(pkh(02e493dbf1c10d80f3581e4904930b1404cc6c13900ee0758474fa94abe8c4cd13)))', isrange=False, issolvable=True, hasprivatekeys=False) + # A bare *1-of-2* multisig output with keys in the specified order. + self.test_desc('multi(1,022f8bde4d1a07209355b4a7250a5c5128e88b84bddc619ab7cba8d569b240efe4,025cbdf0646e5db4eaa398f365f2ea7a0e3d419b7e0330e39ce92bddedcac4f9bc)', isrange=False, issolvable=True, hasprivatekeys=False) + # A P2SH *2-of-2* multisig output with keys in the specified order. + self.test_desc('sh(multi(2,022f01e5e15cca351daff3843fb70f3c2f0a1bdd05e5af888a67784ef3e10a2a01,03acd484e2f0c7f65309ad178a9f559abde09796974c57e714c35f110dfc27ccbe))', isrange=False, issolvable=True, hasprivatekeys=False) + # A P2WSH *2-of-3* multisig output with keys in the specified order. + self.test_desc('wsh(multi(2,03a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7,03774ae7f858a9411e5ef4246b70c65aac5649980be5c17891bbec17895da008cb,03d01115d548e7561b15c38f004d734633687cf4419620095bc5b0f47070afe85a))', isrange=False, issolvable=True, hasprivatekeys=False) + # A P2SH-P2WSH *1-of-3* multisig output with keys in the specified order. + self.test_desc('sh(wsh(multi(1,03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8,03499fdf9e895e719cfd64e67f07d38e3226aa7b63678949e6e49b241a60e823e4,02d7924d4f7d43ea965a465ae3095ff41131e5946f3c85f79e44adbcf8e27e080e)))', isrange=False, issolvable=True, hasprivatekeys=False) + # A P2PK output with the public key of the specified xpub. + self.test_desc('pk(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B)', isrange=False, issolvable=True, hasprivatekeys=False) + # A P2PKH output with child key *1'/2* of the specified xpub. + self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1'/2)", isrange=False, issolvable=True, hasprivatekeys=False) + # A set of P2PKH outputs, but additionally specifies that the specified xpub is a child of a master with fingerprint `d34db33f`, and derived using path `44'/0'/0'`. + self.test_desc("pkh([d34db33f/44'/0'/0']tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/*)", isrange=True, issolvable=True, hasprivatekeys=False) + # A set of *1-of-2* P2WSH multisig outputs where the first multisig key is the *1/0/`i`* child of the first specified xpub and the second multisig key is the *0/0/`i`* child of the second specified xpub, and `i` is any number in a configurable range (`0-1000` by default). + self.test_desc("wsh(multi(1,tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/0/*,tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/0/0/*))", isrange=True, issolvable=True, hasprivatekeys=False) + + +if __name__ == '__main__': + DescriptorTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2036d20852..7edb36c151 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -217,6 +217,7 @@ BASE_SCRIPTS = [ 'feature_config_args.py', 'rpc_getaddressinfo_labels_purpose_deprecation.py', 'rpc_getaddressinfo_label_deprecation.py', + 'rpc_getdescriptorinfo.py', 'rpc_help.py', 'feature_help.py', 'feature_shutdown.py', diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 4eb0d19a4f..336e246e33 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -71,28 +71,29 @@ class BumpFeeTest(BitcoinTestFramework): test_simple_bumpfee_succeeds(self, "default", rbf_node, peer_node, dest_address) test_simple_bumpfee_succeeds(self, "fee_rate", rbf_node, peer_node, dest_address) test_feerate_args(self, rbf_node, peer_node, dest_address) - test_segwit_bumpfee_succeeds(rbf_node, dest_address) - test_nonrbf_bumpfee_fails(peer_node, dest_address) - test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address) - test_bumpfee_with_descendant_fails(rbf_node, rbf_node_address, dest_address) - test_small_output_fails(rbf_node, dest_address) - test_dust_to_fee(rbf_node, dest_address) - test_settxfee(rbf_node, dest_address) + test_segwit_bumpfee_succeeds(self, rbf_node, dest_address) + test_nonrbf_bumpfee_fails(self, peer_node, dest_address) + test_notmine_bumpfee_fails(self, rbf_node, peer_node, dest_address) + test_bumpfee_with_descendant_fails(self, rbf_node, rbf_node_address, dest_address) + test_small_output_fails(self, rbf_node, dest_address) + test_dust_to_fee(self, rbf_node, dest_address) + test_settxfee(self, rbf_node, dest_address) test_watchonly_psbt(self, peer_node, rbf_node, dest_address) - test_rebumping(rbf_node, dest_address) - test_rebumping_not_replaceable(rbf_node, dest_address) - test_unconfirmed_not_spendable(rbf_node, rbf_node_address) - test_bumpfee_metadata(rbf_node, dest_address) - test_locked_wallet_fails(rbf_node, dest_address) - test_change_script_match(rbf_node, dest_address) + test_rebumping(self, rbf_node, dest_address) + test_rebumping_not_replaceable(self, rbf_node, dest_address) + test_unconfirmed_not_spendable(self, rbf_node, rbf_node_address) + test_bumpfee_metadata(self, rbf_node, dest_address) + test_locked_wallet_fails(self, rbf_node, dest_address) + test_change_script_match(self, rbf_node, dest_address) test_maxtxfee_fails(self, rbf_node, dest_address) # These tests wipe out a number of utxos that are expected in other tests - test_small_output_with_feerate_succeeds(rbf_node, dest_address) - test_no_more_inputs_fails(rbf_node, dest_address) + test_small_output_with_feerate_succeeds(self, rbf_node, dest_address) + test_no_more_inputs_fails(self, rbf_node, dest_address) self.log.info("Success") def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address): + self.log.info('Test simple bumpfee') rbfid = spend_one_input(rbf_node, dest_address) rbftx = rbf_node.gettransaction(rbfid) self.sync_mempools((rbf_node, peer_node)) @@ -119,6 +120,7 @@ def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address): assert_equal(bumpedwtx["replaces_txid"], rbfid) def test_feerate_args(self, rbf_node, peer_node, dest_address): + self.log.info('Test feerate args') rbfid = spend_one_input(rbf_node, dest_address) self.sync_mempools((rbf_node, peer_node)) assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool() @@ -135,7 +137,8 @@ def test_feerate_args(self, rbf_node, peer_node, dest_address): assert_raises_rpc_error(-4, "is too high (cannot be higher than", rbf_node.bumpfee, rbfid, {"fee_rate":1}) -def test_segwit_bumpfee_succeeds(rbf_node, dest_address): +def test_segwit_bumpfee_succeeds(self, rbf_node, dest_address): + self.log.info('Test that segwit-sourcing bumpfee works') # Create a transaction with segwit output, then create an RBF transaction # which spends it, and make sure bumpfee can be called on it. @@ -165,14 +168,14 @@ def test_segwit_bumpfee_succeeds(rbf_node, dest_address): assert rbfid not in rbf_node.getrawmempool() -def test_nonrbf_bumpfee_fails(peer_node, dest_address): - # cannot replace a non RBF transaction (from node which did not enable RBF) +def test_nonrbf_bumpfee_fails(self, peer_node, dest_address): + self.log.info('Test that we cannot replace a non RBF transaction') not_rbfid = peer_node.sendtoaddress(dest_address, Decimal("0.00090000")) assert_raises_rpc_error(-4, "not BIP 125 replaceable", peer_node.bumpfee, not_rbfid) -def test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address): - # cannot bump fee unless the tx has only inputs that we own. +def test_notmine_bumpfee_fails(self, rbf_node, peer_node, dest_address): + self.log.info('Test that it cannot bump fee if non-owned inputs are included') # here, the rbftx has a peer_node coin and then adds a rbf_node input # Note that this test depends upon the RPC code checking input ownership prior to change outputs # (since it can't use fundrawtransaction, it lacks a proper change output) @@ -192,8 +195,8 @@ def test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address): rbf_node.bumpfee, rbfid) -def test_bumpfee_with_descendant_fails(rbf_node, rbf_node_address, dest_address): - # cannot bump fee if the transaction has a descendant +def test_bumpfee_with_descendant_fails(self, rbf_node, rbf_node_address, dest_address): + self.log.info('Test that fee cannot be bumped when it has descendant') # parent is send-to-self, so we don't have to check which output is change when creating the child tx parent_id = spend_one_input(rbf_node, rbf_node_address) tx = rbf_node.createrawtransaction([{"txid": parent_id, "vout": 0}], {dest_address: 0.00020000}) @@ -201,7 +204,8 @@ def test_bumpfee_with_descendant_fails(rbf_node, rbf_node_address, dest_address) rbf_node.sendrawtransaction(tx["hex"]) assert_raises_rpc_error(-8, "Transaction has descendants in the wallet", rbf_node.bumpfee, parent_id) -def test_small_output_fails(rbf_node, dest_address): +def test_small_output_fails(self, rbf_node, dest_address): + self.log.info('Test totalFee bump with small output fails') # cannot bump fee with a too-small output rbfid = spend_one_input(rbf_node, dest_address) rbf_node.bumpfee(rbfid, {"totalFee": 50000}) @@ -209,7 +213,8 @@ def test_small_output_fails(rbf_node, dest_address): rbfid = spend_one_input(rbf_node, dest_address) assert_raises_rpc_error(-4, "Change output is too small", rbf_node.bumpfee, rbfid, {"totalFee": 50001}) -def test_small_output_with_feerate_succeeds(rbf_node, dest_address): +def test_small_output_with_feerate_succeeds(self, rbf_node, dest_address): + self.log.info('Testing small output with feerate bump succeeds') # Make sure additional inputs exist rbf_node.generatetoaddress(101, rbf_node.getnewaddress()) @@ -217,9 +222,9 @@ def test_small_output_with_feerate_succeeds(rbf_node, dest_address): input_list = rbf_node.getrawtransaction(rbfid, 1)["vin"] assert_equal(len(input_list), 1) original_txin = input_list[0] - # Keep bumping until we out-spend change output + self.log.info('Keep bumping until transaction fee out-spends non-destination value') tx_fee = 0 - while tx_fee < Decimal("0.0005"): + while True: input_list = rbf_node.getrawtransaction(rbfid, 1)["vin"] new_item = list(input_list)[0] assert_equal(len(input_list), 1) @@ -231,7 +236,11 @@ def test_small_output_with_feerate_succeeds(rbf_node, dest_address): assert rbfid not in raw_pool assert rbfid_new in raw_pool rbfid = rbfid_new - tx_fee = rbfid_new_details["origfee"] + tx_fee = rbfid_new_details["fee"] + + # Total value from input not going to destination + if tx_fee > Decimal('0.00050000'): + break # input(s) have been added final_input_list = rbf_node.getrawtransaction(rbfid, 1)["vin"] @@ -244,8 +253,8 @@ def test_small_output_with_feerate_succeeds(rbf_node, dest_address): rbf_node.generatetoaddress(1, rbf_node.getnewaddress()) assert_equal(rbf_node.gettransaction(rbfid)["confirmations"], 1) -def test_dust_to_fee(rbf_node, dest_address): - # check that if output is reduced to dust, it will be converted to fee +def test_dust_to_fee(self, rbf_node, dest_address): + self.log.info('Test that bumped output that is dust is dropped to fee') # the bumped tx sets fee=49,900, but it converts to 50,000 rbfid = spend_one_input(rbf_node, dest_address) fulltx = rbf_node.getrawtransaction(rbfid, 1) @@ -257,7 +266,8 @@ def test_dust_to_fee(rbf_node, dest_address): assert_equal(len(full_bumped_tx["vout"]), 1) # change output is eliminated -def test_settxfee(rbf_node, dest_address): +def test_settxfee(self, rbf_node, dest_address): + self.log.info('Test settxfee') assert_raises_rpc_error(-8, "txfee cannot be less than min relay tx fee", rbf_node.settxfee, Decimal('0.000005')) assert_raises_rpc_error(-8, "txfee cannot be less than wallet min fee", rbf_node.settxfee, Decimal('0.000015')) # check that bumpfee reacts correctly to the use of settxfee (paytxfee) @@ -272,17 +282,19 @@ def test_settxfee(rbf_node, dest_address): rbf_node.settxfee(Decimal("0.00000000")) # unset paytxfee -def test_maxtxfee_fails(test, rbf_node, dest_address): +def test_maxtxfee_fails(self, rbf_node, dest_address): + self.log.info('Test that bumpfee fails when it hits -matxfee') # size of bumped transaction (p2wpkh, 1 input, 2 outputs): 141 vbytes # expected bumping feerate of 20 sats/vbyte => 141*20 sats = 0.00002820 btc - test.restart_node(1, ['-maxtxfee=0.000025'] + test.extra_args[1]) + self.restart_node(1, ['-maxtxfee=0.000025'] + self.extra_args[1]) rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT) rbfid = spend_one_input(rbf_node, dest_address) assert_raises_rpc_error(-4, "Unable to create transaction: Fee exceeds maximum configured by -maxtxfee", rbf_node.bumpfee, rbfid) - test.restart_node(1, test.extra_args[1]) + self.restart_node(1, self.extra_args[1]) rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT) -def test_watchonly_psbt(test, peer_node, rbf_node, dest_address): +def test_watchonly_psbt(self, peer_node, rbf_node, dest_address): + self.log.info('Test that PSBT is returned for bumpfee in watchonly wallets') priv_rec_desc = "wpkh([00000001/84'/1'/0']tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/0/*)#rweraev0" pub_rec_desc = rbf_node.getdescriptorinfo(priv_rec_desc)["descriptor"] priv_change_desc = "wpkh([00000001/84'/1'/0']tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/*)#j6uzqvuh" @@ -334,7 +346,7 @@ def test_watchonly_psbt(test, peer_node, rbf_node, dest_address): funding_address2 = watcher.getnewaddress(address_type='bech32') peer_node.sendmany("", {funding_address1: 0.001, funding_address2: 0.001}) peer_node.generate(1) - test.sync_all() + self.sync_all() # Create single-input PSBT for transaction to be bumped psbt = watcher.walletcreatefundedpsbt([], {dest_address:0.0005}, 0, {"feeRate": 0.00001}, True)['psbt'] @@ -363,24 +375,24 @@ def test_watchonly_psbt(test, peer_node, rbf_node, dest_address): rbf_node.unloadwallet("watcher") rbf_node.unloadwallet("signer") -def test_rebumping(rbf_node, dest_address): - # check that re-bumping the original tx fails, but bumping the bumper succeeds +def test_rebumping(self, rbf_node, dest_address): + self.log.info('Test that re-bumping the original tx fails, but bumping successor works') rbfid = spend_one_input(rbf_node, dest_address) bumped = rbf_node.bumpfee(rbfid, {"totalFee": 2000}) assert_raises_rpc_error(-4, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 3000}) rbf_node.bumpfee(bumped["txid"], {"totalFee": 3000}) -def test_rebumping_not_replaceable(rbf_node, dest_address): - # check that re-bumping a non-replaceable bump tx fails +def test_rebumping_not_replaceable(self, rbf_node, dest_address): + self.log.info('Test that re-bumping non-replaceable fails') rbfid = spend_one_input(rbf_node, dest_address) bumped = rbf_node.bumpfee(rbfid, {"totalFee": 10000, "replaceable": False}) assert_raises_rpc_error(-4, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"], {"totalFee": 20000}) -def test_unconfirmed_not_spendable(rbf_node, rbf_node_address): - # check that unconfirmed outputs from bumped transactions are not spendable +def test_unconfirmed_not_spendable(self, rbf_node, rbf_node_address): + self.log.info('Test that unconfirmed outputs from bumped txns are not spendable') rbfid = spend_one_input(rbf_node, rbf_node_address) rbftx = rbf_node.gettransaction(rbfid)["hex"] assert rbfid in rbf_node.getrawmempool() @@ -418,7 +430,8 @@ def test_unconfirmed_not_spendable(rbf_node, rbf_node_address): if t["txid"] == rbfid and t["address"] == rbf_node_address and t["spendable"]), 1) -def test_bumpfee_metadata(rbf_node, dest_address): +def test_bumpfee_metadata(self, rbf_node, dest_address): + self.log.info('Test that bumped txn metadata persists to new txn record') assert(rbf_node.getbalance() < 49) rbf_node.generatetoaddress(101, rbf_node.getnewaddress()) rbfid = rbf_node.sendtoaddress(dest_address, 49, "comment value", "to value") @@ -428,15 +441,17 @@ def test_bumpfee_metadata(rbf_node, dest_address): assert_equal(bumped_wtx["to"], "to value") -def test_locked_wallet_fails(rbf_node, dest_address): +def test_locked_wallet_fails(self, rbf_node, dest_address): + self.log.info('Test that locked wallet cannot bump txn') rbfid = spend_one_input(rbf_node, dest_address) rbf_node.walletlock() assert_raises_rpc_error(-13, "Please enter the wallet passphrase with walletpassphrase first.", rbf_node.bumpfee, rbfid) rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT) -def test_change_script_match(rbf_node, dest_address): - """Test that the same change addresses is used for the replacement transaction when possible.""" +def test_change_script_match(self, rbf_node, dest_address): + self.log.info('Test that the same change addresses is used for the replacement transaction when possible.') + def get_change_address(tx): tx_details = rbf_node.getrawtransaction(tx, 1) txout_addresses = [txout['scriptPubKey']['addresses'][0] for txout in tx_details["vout"]] @@ -480,7 +495,8 @@ def submit_block_with_tx(node, tx): node.submitblock(block.serialize().hex()) return block -def test_no_more_inputs_fails(rbf_node, dest_address): +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 rbf_node.generatetoaddress(1, dest_address) # spend all funds, no change output diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py index 048b3127ff..dabd78f66c 100755 --- a/test/functional/wallet_createwallet.py +++ b/test/functional/wallet_createwallet.py @@ -79,7 +79,7 @@ class CreateWalletTest(BitcoinTestFramework): assert_raises_rpc_error(-4, "Error: This wallet has no available keys", w4.getnewaddress) assert_raises_rpc_error(-4, "Error: This wallet has no available keys", w4.getrawchangeaddress) # Now set a seed and it should work. Wallet should also be encrypted - w4.walletpassphrase('pass', 2) + w4.walletpassphrase('pass', 60) w4.sethdseed() w4.getnewaddress() w4.getrawchangeaddress() diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index 6f248c9bd3..229eda9806 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -2,7 +2,7 @@ # Copyright (c) 2017-2019 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. -"""Test the listsincelast RPC.""" +"""Test the listsinceblock RPC.""" from test_framework.test_framework import BitcoinTestFramework from test_framework.messages import BIP125_SEQUENCE_NUMBER @@ -38,6 +38,7 @@ class ListSinceBlockTest(BitcoinTestFramework): self.double_spends_filtered() def test_no_blockhash(self): + self.log.info("Test no blockhash") txid = self.nodes[2].sendtoaddress(self.nodes[0].getnewaddress(), 1) blockhash, = self.nodes[2].generate(1) blockheight = self.nodes[2].getblockheader(blockhash)['height'] @@ -63,6 +64,7 @@ class ListSinceBlockTest(BitcoinTestFramework): "transactions": txs}) def test_invalid_blockhash(self): + self.log.info("Test invalid blockhash") assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock, "42759cde25462784395a337460bde75f58e73d3f08bd31fdc3507cbac856a2c4") assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock, @@ -100,6 +102,7 @@ class ListSinceBlockTest(BitcoinTestFramework): This test only checks that [tx0] is present. ''' + self.log.info("Test reorg") # Split network into two self.split_network() @@ -110,7 +113,7 @@ class ListSinceBlockTest(BitcoinTestFramework): # generate on both sides lastblockhash = self.nodes[1].generate(6)[5] self.nodes[2].generate(7) - self.log.info('lastblockhash=%s' % (lastblockhash)) + self.log.debug('lastblockhash={}'.format(lastblockhash)) self.sync_all(self.nodes[:2]) self.sync_all(self.nodes[2:]) @@ -155,6 +158,7 @@ class ListSinceBlockTest(BitcoinTestFramework): until the fork point, and to include all transactions that relate to the node wallet. ''' + self.log.info("Test double spend") self.sync_all() @@ -234,6 +238,7 @@ class ListSinceBlockTest(BitcoinTestFramework): 3. It is listed with a confirmation count of 2 (bb3, bb4), not 3 (aa1, aa2, aa3). ''' + self.log.info("Test double send") self.sync_all() @@ -302,6 +307,7 @@ class ListSinceBlockTest(BitcoinTestFramework): `listsinceblock` was returning conflicted transactions even if they occurred before the specified cutoff blockhash ''' + self.log.info("Test spends filtered") spending_node = self.nodes[2] dest_address = spending_node.getnewaddress() |