diff options
65 files changed, 856 insertions, 690 deletions
diff --git a/.appveyor.yml b/.appveyor.yml index 0a644992dd..ac17e2eeb6 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -33,7 +33,7 @@ before_build: - ps: Start-Process clcache-server - ps: fsutil behavior set disablelastaccess 0 # Enable Access time feature on Windows (for clcache) build_script: -- cmd: msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nologo +- cmd: msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:n /nologo after_build: - ps: fsutil behavior set disablelastaccess 1 # Disable Access time feature on Windows (better performance) - ps: clcache -z diff --git a/.gitignore b/.gitignore index 5cef80f92f..cec1fb3dec 100644 --- a/.gitignore +++ b/.gitignore @@ -86,6 +86,7 @@ src/qt/bitcoin-qt.includes # Compilation and Qt preprocessor part *.qm Makefile +!depends/Makefile bitcoin-qt Bitcoin-Qt.app background.tiff* diff --git a/.travis.yml b/.travis.yml index b893f35699..422e9149f4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -130,21 +130,21 @@ jobs: CONFIG_SHELL="/bin/dash" - stage: test - name: 'x86_64 Linux [GOAL: install] [bionic] [uses qt5 dev package instead of depends Qt to speed up build and avoid timeout]' + name: 'x86_64 Linux [GOAL: install] [bionic] [uses qt5 dev package instead of depends Qt to speed up build and avoid timeout] [unsigned char]' env: >- HOST=x86_64-unknown-linux-gnu PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools protobuf-compiler libdbus-1-dev libharfbuzz-dev libprotobuf-dev" DEP_OPTS="NO_QT=1 NO_UPNP=1 DEBUG=1 ALLOW_HOST_PACKAGES=1" TEST_RUNNER_EXTRA="--coverage --extended --exclude feature_dbcrash" # Run extended tests so that coverage does not fail, but exclude the very slow dbcrash GOAL="install" - BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-glibc-back-compat --enable-reduce-exports --enable-debug CXXFLAGS=\"-g0 -O2\"" + BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-glibc-back-compat --enable-reduce-exports --enable-debug CFLAGS=\"-g0 -O2 -funsigned-char\" CXXFLAGS=\"-g0 -O2 -funsigned-char\"" - stage: test name: 'x86_64 Linux [GOAL: install] [trusty] [no functional tests, no depends, only system libs]' env: >- HOST=x86_64-unknown-linux-gnu DOCKER_NAME_TAG=ubuntu:14.04 - PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libicu-dev libpng-dev libssl-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libdb5.1++-dev libminiupnpc-dev libzmq3-dev libprotobuf-dev protobuf-compiler libqrencode-dev" + PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libicu-dev libpng-dev libssl-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libdb5.1++-dev libzmq3-dev libprotobuf-dev protobuf-compiler libqrencode-dev" NO_DEPENDS=1 RUN_FUNCTIONAL_TESTS=false GOAL="install" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a2456f5b8c..268259c82d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -80,21 +80,15 @@ The title of the pull request should be prefixed by the component or area that the pull request affects. Valid areas as: - *Consensus* for changes to consensus critical code - - *Docs* for changes to the documentation + - *Doc* for changes to the documentation - *Qt* for changes to bitcoin-qt + - *Log* Changes to log messages - *Mining* for changes to the mining code - *Net* or *P2P* for changes to the peer-to-peer network code + - *Refactor* for structural changes that do not change behavior - *RPC/REST/ZMQ* for changes to the RPC, REST or ZMQ APIs - *Scripts and tools* for changes to the scripts and tools - - *Tests* for changes to the bitcoin unit tests or QA tests - - *Trivial* should **only** be used for PRs that do not change generated - executable code. Notably, refactors (change of function arguments and code - reorganization) and changes in behavior should **not** be marked as trivial. - Examples of trivial PRs are changes to: - - comments - - whitespace - - variable names - - logging and messages + - *Test* for changes to the bitcoin unit tests or QA tests - *Utils and libraries* for changes to the utils and libraries - *Wallet* for changes to the wallet code @@ -103,10 +97,10 @@ Examples: Consensus: Add new opcode for BIP-XXXX OP_CHECKAWESOMESIG Net: Automatically create hidden service, listen on Tor Qt: Add feed bump button - Trivial: Fix typo in init.cpp + Log: Fix typo in log message Note that translations should not be submitted as pull requests, please see -[Translation Process](https://github.com/bitcoin/bitcoin/blob/master/doc/translation_process.md) +[Translation Process](https://github.com/bitcoin/bitcoin/blob/master/doc/translation_process.md) for more information on helping with translations. If a pull request is not to be considered for merging (yet), please @@ -322,7 +316,7 @@ The project leader is the release manager for each Bitcoin Core release. Copyright --------- -By contributing to this repository, you agree to license your work under the -MIT license unless specified otherwise in `contrib/debian/copyright` or at -the top of the file itself. Any work contributed where you are not the original +By contributing to this repository, you agree to license your work under the +MIT license unless specified otherwise in `contrib/debian/copyright` or at +the top of the file itself. Any work contributed where you are not the original author must contain its license header with the original author(s) and source. diff --git a/build-aux/m4/bitcoin_qt.m4 b/build-aux/m4/bitcoin_qt.m4 index 8ccbb19300..675fb6d3fb 100644 --- a/build-aux/m4/bitcoin_qt.m4 +++ b/build-aux/m4/bitcoin_qt.m4 @@ -268,7 +268,7 @@ AC_DEFUN([_BITCOIN_QT_CHECK_QT5],[ dnl Internal. Check if the included version of Qt is greater than Qt58. dnl Requires: INCLUDES must be populated as necessary. -dnl Output: bitcoin_cv_qt5=yes|no +dnl Output: bitcoin_cv_qt58=yes|no AC_DEFUN([_BITCOIN_QT_CHECK_QT58],[ AC_CACHE_CHECK(for > Qt 5.7, bitcoin_cv_qt58,[ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ @@ -468,7 +468,6 @@ AC_DEFUN([_BITCOIN_QT_FIND_LIBS_WITHOUT_PKGCONFIG],[ ]) BITCOIN_QT_CHECK(AC_CHECK_LIB([z] ,[main],,AC_MSG_WARN([zlib not found. Assuming qt has it built-in]))) - BITCOIN_QT_CHECK(AC_SEARCH_LIBS([jpeg_create_decompress] ,[qtjpeg jpeg],,AC_MSG_WARN([libjpeg not found. Assuming qt has it built-in]))) if test x$bitcoin_cv_qt58 = xno; then BITCOIN_QT_CHECK(AC_SEARCH_LIBS([png_error] ,[qtpng png],,AC_MSG_WARN([libpng not found. Assuming qt has it built-in]))) BITCOIN_QT_CHECK(AC_SEARCH_LIBS([pcre16_exec], [qtpcre pcre16],,AC_MSG_WARN([libpcre16 not found. Assuming qt has it built-in]))) diff --git a/build_msvc/README.md b/build_msvc/README.md index d6543e6d67..2e93979aca 100644 --- a/build_msvc/README.md +++ b/build_msvc/README.md @@ -44,7 +44,7 @@ The instructions below use `vcpkg` to install the dependencies. - Use Python to generate *.vcxproj from Makefile ``` - PS >python msvc-autogen.py + PS >py -3 msvc-autogen.py ``` - Build in Visual Studio. diff --git a/configure.ac b/configure.ac index 08a850cfa2..118d52d423 100644 --- a/configure.ac +++ b/configure.ac @@ -323,6 +323,7 @@ if test "x$enable_werror" = "xyes"; then AC_MSG_ERROR("enable-werror set but -Werror is not usable") fi AX_CHECK_COMPILE_FLAG([-Werror=vla],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=vla"],,[[$CXXFLAG_WERROR]]) + AX_CHECK_COMPILE_FLAG([-Werror=switch],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=switch"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=thread-safety-analysis],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=thread-safety-analysis"],,[[$CXXFLAG_WERROR]]) fi @@ -331,6 +332,7 @@ if test "x$CXXFLAGS_overridden" = "xno"; then AX_CHECK_COMPILE_FLAG([-Wextra],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wextra"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wformat],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wformat"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wvla],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wvla"],,[[$CXXFLAG_WERROR]]) + AX_CHECK_COMPILE_FLAG([-Wswitch],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wswitch"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wformat-security],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wformat-security"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wthread-safety-analysis],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wthread-safety-analysis"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wrange-loop-analysis],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wrange-loop-analysis"],,[[$CXXFLAG_WERROR]]) @@ -992,6 +994,26 @@ if test x$use_upnp != xno; then [AC_CHECK_LIB([miniupnpc], [upnpDiscover], [MINIUPNPC_LIBS=-lminiupnpc], [have_miniupnpc=no])], [have_miniupnpc=no] ) +dnl The minimum supported miniUPnPc API version is set to 10. This keeps compatibility +dnl with Ubuntu 16.04 LTS and Debian 8 libminiupnpc-dev packages. +if test x$have_miniupnpc != xno; then + AC_MSG_CHECKING([whether miniUPnPc API version is supported]) + AC_PREPROC_IFELSE([AC_LANG_PROGRAM([[ + @%:@include <miniupnpc/miniupnpc.h> + ]], [[ + #if MINIUPNPC_API_VERSION >= 10 + // Everything is okay + #else + # error miniUPnPc API version is too old + #endif + ]])],[ + AC_MSG_RESULT(yes) + ],[ + AC_MSG_RESULT(no) + AC_MSG_WARN([miniUPnPc API version < 10 is unsupported, disabling UPnP support.]) + have_miniupnpc=no + ]) +fi fi if test x$build_bitcoin_wallet$build_bitcoin_cli$build_bitcoin_tx$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench = xnonononononono; then @@ -1385,9 +1407,10 @@ dnl enable upnp support AC_MSG_CHECKING([whether to build with support for UPnP]) if test x$have_miniupnpc = xno; then if test x$use_upnp = xyes; then - AC_MSG_ERROR("UPnP requested but cannot be built. use --without-miniupnpc") + AC_MSG_ERROR("UPnP requested but cannot be built. Use --without-miniupnpc.") fi AC_MSG_RESULT(no) + use_upnp=no else if test x$use_upnp != xno; then AC_MSG_RESULT(yes) diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 204feb605b..2610c1e748 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -39,6 +39,7 @@ $(package)_config_opts += -no-ico $(package)_config_opts += -no-iconv $(package)_config_opts += -no-kms $(package)_config_opts += -no-linuxfb +$(package)_config_opts += -no-libjpeg $(package)_config_opts += -no-libudev $(package)_config_opts += -no-mtdev $(package)_config_opts += -no-openvg @@ -64,7 +65,6 @@ $(package)_config_opts += -pch $(package)_config_opts += -pkg-config $(package)_config_opts += -prefix $(host_prefix) $(package)_config_opts += -qt-libpng -$(package)_config_opts += -qt-libjpeg $(package)_config_opts += -qt-pcre $(package)_config_opts += -qt-harfbuzz $(package)_config_opts += -system-zlib diff --git a/doc/bitcoin-conf.md b/doc/bitcoin-conf.md index 88ecb8fe65..f8146b5d75 100644 --- a/doc/bitcoin-conf.md +++ b/doc/bitcoin-conf.md @@ -30,6 +30,21 @@ Network specific options can be: - placed into sections with headers `[main]` (not `[mainnet]`), `[test]` (not `[testnet]`) or `[regtest]`; - prefixed with a chain name; e.g., `regtest.maxmempool=100`. +Network specific options take precedence over non-network specific options. +If multiple values for the same option are found with the same precedence, the +first one is generally chosen. + +This means that given the following configuration, `regtest.rpcport` is set to `3000`: + +``` +regtest=1 +rpcport=2000 +regtest.rpcport=3000 + +[regtest] +rpcport=4000 +``` + ## Configuration File Path The configuration file is not automatically created; you can create it using your favorite text editor. By default, the configuration file name is `bitcoin.conf` and it is located in the Bitcoin data directory, but both the Bitcoin data directory and the configuration file path may be changed using the `-datadir` and `-conf` command-line options. diff --git a/doc/dependencies.md b/doc/dependencies.md index c9c6a93c01..0b23ca0a2d 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -7,25 +7,24 @@ These are the dependencies currently used by Bitcoin Core. You can find instruct | --- | --- | --- | --- | --- | --- | | Berkeley DB | [4.8.30](https://www.oracle.com/technetwork/database/database-technologies/berkeleydb/downloads/index.html) | 4.8.x | No | | | | Boost | [1.70.0](https://www.boost.org/users/download/) | [1.47.0](https://github.com/bitcoin/bitcoin/pull/8920) | No | | | -| Clang | | [3.3+](https://llvm.org/releases/download.html) (C++11 support) | | | | +| Clang | | [3.3+](https://releases.llvm.org/download.html) (C++11 support) | | | | | Expat | [2.2.7](https://libexpat.github.io/) | | No | Yes | | | fontconfig | [2.12.1](https://www.freedesktop.org/software/fontconfig/release/) | | No | Yes | | | FreeType | [2.7.1](https://download.savannah.gnu.org/releases/freetype) | | No | | | | GCC | | [4.8+](https://gcc.gnu.org/) (C++11 support) | | | | | HarfBuzz-NG | | | | | | | libevent | [2.1.8-stable](https://github.com/libevent/libevent/releases) | 2.0.22 | No | | | -| libjpeg | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk#L65) | -| libpng | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk#L64) | +| libpng | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk) | | librsvg | | | | | | | MiniUPnPc | [2.0.20180203](http://miniupnp.free.fr/files) | | No | | | | OpenSSL | [1.0.1k](https://www.openssl.org/source) | | Yes | | | -| PCRE | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk#L66) | +| PCRE | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk) | | protobuf | [2.6.1](https://github.com/google/protobuf/releases) | | No | | | | Python (tests) | | [3.5](https://www.python.org/downloads) | | | | | qrencode | [3.4.4](https://fukuchi.org/works/qrencode) | | No | | | | Qt | [5.9.7](https://download.qt.io/official_releases/qt/) | [5.5.1](https://github.com/bitcoin/bitcoin/issues/13478) | No | | | -| XCB | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk#L87) (Linux only) | -| xkbcommon | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk#L86) (Linux only) | +| XCB | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk) (Linux only) | +| xkbcommon | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk) (Linux only) | | ZeroMQ | [4.3.1](https://github.com/zeromq/libzmq/releases) | 4.0.0 | No | | | | zlib | [1.2.11](https://zlib.net/) | | | | No | diff --git a/doc/release-notes-15993.md b/doc/release-notes-15993.md new file mode 100644 index 0000000000..493c7126ee --- /dev/null +++ b/doc/release-notes-15993.md @@ -0,0 +1,3 @@ +Build system changes +-------------------- +The minimum supported miniUPnPc API version is set to 10. This keeps compatibility with Ubuntu 16.04 LTS and Debian 8 `libminiupnpc-dev` packages. Please note, on Debian this package is still vulnerable to [CVE-2017-8798](https://security-tracker.debian.org/tracker/CVE-2017-8798) (in jessie only) and [CVE-2017-1000494](https://security-tracker.debian.org/tracker/CVE-2017-1000494) (both in jessie and in stretch). diff --git a/doc/release-notes-16394.md b/doc/release-notes-16394.md new file mode 100644 index 0000000000..f09cba4b6d --- /dev/null +++ b/doc/release-notes-16394.md @@ -0,0 +1,4 @@ +RPC changes +----------- +`createwallet` now returns a warning if an empty string is used as an encryption password, and does not encrypt the wallet, instead of raising an error. +This makes it easier to disable encryption but also specify other options when using the `bitcoin-cli` tool. diff --git a/doc/release-notes.md b/doc/release-notes.md index afb0469291..e0673ae7ee 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -88,6 +88,12 @@ Low-level Changes section below. `-limitancestorcount`, `-limitdescendantcount` and `-walletrejectlongchains` command line arguments. +Deprecated or removed RPCs +-------------------------- + +- The `totalFee` option of the `bumpfee` RPC has been deprecated and will be + removed in 0.20. To continue using this option start with + `-deprecatedrpc=totalFee`. See the `bumpfee` RPC help text for more details. Low-level changes ================= diff --git a/src/Makefile.am b/src/Makefile.am index 0f05439227..ef5b1900d9 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -310,7 +310,7 @@ libbitcoin_server_a_SOURCES += dummywallet.cpp endif if ENABLE_ZMQ -libbitcoin_zmq_a_CPPFLAGS = $(BITCOIN_INCLUDES) $(ZMQ_CFLAGS) +libbitcoin_zmq_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(ZMQ_CFLAGS) libbitcoin_zmq_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libbitcoin_zmq_a_SOURCES = \ zmq/zmqabstractnotifier.cpp \ diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index ba021a5163..8e31f6e32b 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -143,7 +143,7 @@ static bool AppInit(int argc, char* argv[]) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" #endif - tfm::format(std::cout, PACKAGE_NAME "daemon starting\n"); + tfm::format(std::cout, PACKAGE_NAME " daemon starting\n"); // Daemonize if (daemon(1, 0)) { // don't chdir (1), do close FDs (0) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index f937e2754b..c24234aeb7 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -167,9 +167,6 @@ public: /* nTxCount */ 383732546, /* dTxRate */ 3.685496590998308 }; - - /* disable fallback fee on mainnet */ - m_fallback_fee_enabled = false; } }; @@ -262,9 +259,6 @@ public: /* nTxCount */ 19438708, /* dTxRate */ 0.626 }; - - /* enable fallback fee on testnet */ - m_fallback_fee_enabled = true; } }; @@ -346,9 +340,6 @@ public: base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x35, 0x83, 0x94}; bech32_hrp = "bcrt"; - - /* enable fallback fee on regtest */ - m_fallback_fee_enabled = true; } /** diff --git a/src/chainparams.h b/src/chainparams.h index b3fcd77cea..8f1d27e03c 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -66,7 +66,7 @@ public: bool DefaultConsistencyChecks() const { return fDefaultConsistencyChecks; } /** Policy: Filter transactions that do not match well-defined patterns */ bool RequireStandard() const { return fRequireStandard; } - /** If this is a test chain */ + /** If this chain is exclusively used for testing */ bool IsTestChain() const { return m_is_test_chain; } uint64_t PruneAfterHeight() const { return nPruneAfterHeight; } /** Minimum free space (in GB) needed for data directory */ @@ -77,8 +77,6 @@ public: bool MineBlocksOnDemand() const { return consensus.fPowNoRetargeting; } /** Return the BIP70 network string (main, test or regtest) */ std::string NetworkIDString() const { return strNetworkID; } - /** Return true if the fallback fee is by default enabled for this network */ - bool IsFallbackFeeEnabled() const { return m_fallback_fee_enabled; } /** Return the list of hostnames to look up for DNS seeds */ const std::vector<std::string>& DNSSeeds() const { return vSeeds; } const std::vector<unsigned char>& Base58Prefix(Base58Type type) const { return base58Prefixes[type]; } @@ -106,7 +104,6 @@ protected: bool m_is_test_chain; CCheckpointData checkpointData; ChainTxData chainTxData; - bool m_fallback_fee_enabled; }; /** diff --git a/src/chainparamsbase.h b/src/chainparamsbase.h index 355df043d3..f34646f7ac 100644 --- a/src/chainparamsbase.h +++ b/src/chainparamsbase.h @@ -7,7 +7,6 @@ #include <memory> #include <string> -#include <vector> /** * CBaseChainParams defines the base parameters (shared between bitcoin-cli and bitcoind) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 02f39cef8e..1ad4308f29 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -9,7 +9,9 @@ #include <interfaces/handler.h> #include <interfaces/wallet.h> #include <net.h> +#include <net_processing.h> #include <node/coin.h> +#include <node/transaction.h> #include <policy/fees.h> #include <policy/policy.h> #include <policy/rbf.h> @@ -149,12 +151,6 @@ class LockImpl : public Chain::Lock, public UniqueLock<CCriticalSection> LockAssertion lock(::cs_main); return CheckFinalTx(tx); } - bool submitToMemoryPool(const CTransactionRef& tx, CAmount absurd_fee, CValidationState& state) override - { - LockAssertion lock(::cs_main); - return AcceptToMemoryPool(::mempool, state, tx, nullptr /* missing inputs */, nullptr /* txn replaced */, - false /* bypass limits */, absurd_fee); - } using UniqueLock::UniqueLock; }; @@ -290,10 +286,13 @@ public: auto it = ::mempool.GetIter(txid); return it && (*it)->GetCountWithDescendants() > 1; } - void relayTransaction(const uint256& txid) override + bool broadcastTransaction(const CTransactionRef& tx, std::string& err_string, const CAmount& max_tx_fee, bool relay) override { - CInv inv(MSG_TX, txid); - g_connman->ForEachNode([&inv](CNode* node) { node->PushInventory(inv); }); + const TransactionError err = BroadcastTransaction(tx, err_string, max_tx_fee, relay, /*wait_callback*/ false); + // Chain clients only care about failures to accept the tx to the mempool. Disregard non-mempool related failures. + // Note: this will need to be updated if BroadcastTransactions() is updated to return other non-mempool failures + // that Chain clients do not need to know about. + return TransactionError::OK == err; } void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) override { diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index e675defd47..1d6ed05522 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -43,10 +43,6 @@ class Wallet; //! asynchronously //! (https://github.com/bitcoin/bitcoin/pull/10973#issuecomment-380101269). //! -//! * The relayTransactions() and submitToMemoryPool() methods could be replaced -//! with a higher-level broadcastTransaction method -//! (https://github.com/bitcoin/bitcoin/pull/14978#issuecomment-459373984). -//! //! * The initMessages() and loadWallet() methods which the wallet uses to send //! notifications to the GUI should go away when GUI and wallet can directly //! communicate with each other without going through the node @@ -127,11 +123,6 @@ public: //! Check if transaction will be final given chain height current time. virtual bool checkFinalTx(const CTransaction& tx) = 0; - - //! Add transaction to memory pool if the transaction fee is below the - //! amount specified by absurd_fee. Returns false if the transaction - //! could not be added due to the fee or for another reason. - virtual bool submitToMemoryPool(const CTransactionRef& tx, CAmount absurd_fee, CValidationState& state) = 0; }; //! Return Lock interface. Chain is locked when this is called, and @@ -164,8 +155,10 @@ public: //! Check if transaction has descendants in mempool. virtual bool hasDescendantsInMempool(const uint256& txid) = 0; - //! Relay transaction. - virtual void relayTransaction(const uint256& txid) = 0; + //! Transaction is added to memory pool, if the transaction fee is below the + //! amount specified by max_tx_fee, and broadcast to all peers if relay is set to true. + //! Return false if the transaction could not be added due to the fee or for another reason. + virtual bool broadcastTransaction(const CTransactionRef& tx, std::string& err_string, const CAmount& max_tx_fee, bool relay) = 0; //! Calculate mempool ancestor and descendant counts for the given transaction. virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0; diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index deb1618ceb..077dc1ab4d 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -33,31 +33,6 @@ namespace interfaces { namespace { -class PendingWalletTxImpl : public PendingWalletTx -{ -public: - explicit PendingWalletTxImpl(CWallet& wallet) : m_wallet(wallet) {} - - const CTransaction& get() override { return *m_tx; } - - bool commit(WalletValueMap value_map, - WalletOrderForm order_form, - std::string& reject_reason) override - { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - CValidationState state; - if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), state)) { - reject_reason = state.GetRejectReason(); - return false; - } - return true; - } - - CTransactionRef m_tx; - CWallet& m_wallet; -}; - //! Construct wallet tx struct. WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, const CWalletTx& wtx) { @@ -227,7 +202,7 @@ public: LOCK(m_wallet->cs_wallet); return m_wallet->ListLockedCoins(outputs); } - std::unique_ptr<PendingWalletTx> createTransaction(const std::vector<CRecipient>& recipients, + CTransactionRef createTransaction(const std::vector<CRecipient>& recipients, const CCoinControl& coin_control, bool sign, int& change_pos, @@ -236,12 +211,26 @@ public: { auto locked_chain = m_wallet->chain().lock(); LOCK(m_wallet->cs_wallet); - auto pending = MakeUnique<PendingWalletTxImpl>(*m_wallet); - if (!m_wallet->CreateTransaction(*locked_chain, recipients, pending->m_tx, fee, change_pos, + CTransactionRef tx; + if (!m_wallet->CreateTransaction(*locked_chain, recipients, tx, fee, change_pos, fail_reason, coin_control, sign)) { return {}; } - return std::move(pending); + return tx; + } + bool commitTransaction(CTransactionRef tx, + WalletValueMap value_map, + WalletOrderForm order_form, + std::string& reject_reason) override + { + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + CValidationState state; + if (!m_wallet->CommitTransaction(std::move(tx), std::move(value_map), std::move(order_form), state)) { + reject_reason = state.GetRejectReason(); + return false; + } + return true; } bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); } bool abandonTransaction(const uint256& txid) override diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index db47dbafaf..89e056b18b 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -34,7 +34,6 @@ struct CRecipient; namespace interfaces { class Handler; -class PendingWalletTx; struct WalletAddress; struct WalletBalances; struct WalletTx; @@ -134,13 +133,19 @@ public: virtual void listLockedCoins(std::vector<COutPoint>& outputs) = 0; //! Create transaction. - virtual std::unique_ptr<PendingWalletTx> createTransaction(const std::vector<CRecipient>& recipients, + virtual CTransactionRef createTransaction(const std::vector<CRecipient>& recipients, const CCoinControl& coin_control, bool sign, int& change_pos, CAmount& fee, std::string& fail_reason) = 0; + //! Commit transaction. + virtual bool commitTransaction(CTransactionRef tx, + WalletValueMap value_map, + WalletOrderForm order_form, + std::string& reject_reason) = 0; + //! Return whether transaction can be abandoned. virtual bool transactionCanBeAbandoned(const uint256& txid) = 0; @@ -288,21 +293,6 @@ public: virtual std::unique_ptr<Handler> handleCanGetAddressesChanged(CanGetAddressesChangedFn fn) = 0; }; -//! Tracking object returned by CreateTransaction and passed to CommitTransaction. -class PendingWalletTx -{ -public: - virtual ~PendingWalletTx() {} - - //! Get transaction data. - virtual const CTransaction& get() = 0; - - //! Send pending transaction and commit to wallet. - virtual bool commit(WalletValueMap value_map, - WalletOrderForm order_form, - std::string& reject_reason) = 0; -}; - //! Information about one wallet address. struct WalletAddress { diff --git a/src/net.cpp b/src/net.cpp index 8e263b7953..7d6eb31a7c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -37,6 +37,9 @@ #include <miniupnpc/miniwget.h> #include <miniupnpc/upnpcommands.h> #include <miniupnpc/upnperrors.h> +// The minimum supported miniUPnPc API version is set to 10. This keeps compatibility +// with Ubuntu 16.04 LTS and Debian 8 libminiupnpc-dev packages. +static_assert(MINIUPNPC_API_VERSION >= 10, "miniUPnPc API version >= 10 assumed"); #endif #include <unordered_map> @@ -1404,16 +1407,10 @@ static void ThreadMapPort() struct UPNPDev * devlist = nullptr; char lanaddr[64]; -#ifndef UPNPDISCOVER_SUCCESS - /* miniupnpc 1.5 */ - devlist = upnpDiscover(2000, multicastif, minissdpdpath, 0); -#elif MINIUPNPC_API_VERSION < 14 - /* miniupnpc 1.6 */ int error = 0; +#if MINIUPNPC_API_VERSION < 14 devlist = upnpDiscover(2000, multicastif, minissdpdpath, 0, 0, &error); #else - /* miniupnpc 1.9.20150730 */ - int error = 0; devlist = upnpDiscover(2000, multicastif, minissdpdpath, 0, 0, 2, &error); #endif @@ -1427,43 +1424,32 @@ static void ThreadMapPort() if (fDiscover) { char externalIPAddress[40]; r = UPNP_GetExternalIPAddress(urls.controlURL, data.first.servicetype, externalIPAddress); - if(r != UPNPCOMMAND_SUCCESS) + if (r != UPNPCOMMAND_SUCCESS) { LogPrintf("UPnP: GetExternalIPAddress() returned %d\n", r); - else - { - if(externalIPAddress[0]) - { + } else { + if (externalIPAddress[0]) { CNetAddr resolved; - if(LookupHost(externalIPAddress, resolved, false)) { + if (LookupHost(externalIPAddress, resolved, false)) { LogPrintf("UPnP: ExternalIPAddress = %s\n", resolved.ToString().c_str()); AddLocal(resolved, LOCAL_UPNP); } - } - else + } else { LogPrintf("UPnP: GetExternalIPAddress failed.\n"); + } } } - std::string strDesc = "Bitcoin " + FormatFullVersion(); + std::string strDesc = PACKAGE_NAME " " + FormatFullVersion(); do { -#ifndef UPNPDISCOVER_SUCCESS - /* miniupnpc 1.5 */ - r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype, - port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", 0); -#else - /* miniupnpc 1.6 */ - r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype, - port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", 0, "0"); -#endif + r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype, port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", 0, "0"); - if(r!=UPNPCOMMAND_SUCCESS) - LogPrintf("AddPortMapping(%s, %s, %s) failed with code %d (%s)\n", - port, port, lanaddr, r, strupnperror(r)); - else + if (r != UPNPCOMMAND_SUCCESS) { + LogPrintf("AddPortMapping(%s, %s, %s) failed with code %d (%s)\n", port, port, lanaddr, r, strupnperror(r)); + } else { LogPrintf("UPnP Port Mapping successful.\n"); - } - while(g_upnp_interrupt.sleep_for(std::chrono::minutes(20))); + } + } while (g_upnp_interrupt.sleep_for(std::chrono::minutes(20))); r = UPNP_DeletePortMapping(urls.controlURL, data.first.servicetype, port.c_str(), "TCP", 0); LogPrintf("UPNP_DeletePortMapping() returned: %d\n", r); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 4b43b2cdf2..4e631fd00e 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1305,10 +1305,10 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) return true; } -static void RelayTransaction(const CTransaction& tx, CConnman* connman) +void RelayTransaction(const uint256& txid, const CConnman& connman) { - CInv inv(MSG_TX, tx.GetHash()); - connman->ForEachNode([&inv](CNode* pnode) + CInv inv(MSG_TX, txid); + connman.ForEachNode([&inv](CNode* pnode) { pnode->PushInventory(inv); }); @@ -1811,7 +1811,7 @@ void static ProcessOrphanTx(CConnman* connman, std::set<uint256>& orphan_work_se if (setMisbehaving.count(fromPeer)) continue; if (AcceptToMemoryPool(mempool, orphan_state, porphanTx, &fMissingInputs2, &removed_txn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString()); - RelayTransaction(orphanTx, connman); + RelayTransaction(orphanHash, *connman); for (unsigned int i = 0; i < orphanTx.vout.size(); i++) { auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(orphanHash, i)); if (it_by_prev != mapOrphanTransactionsByPrev.end()) { @@ -2498,7 +2498,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (!AlreadyHave(inv) && AcceptToMemoryPool(mempool, state, ptx, &fMissingInputs, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { mempool.check(pcoinsTip.get()); - RelayTransaction(tx, connman); + RelayTransaction(tx.GetHash(), *connman); for (unsigned int i = 0; i < tx.vout.size(); i++) { auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(inv.hash, i)); if (it_by_prev != mapOrphanTransactionsByPrev.end()) { @@ -2577,7 +2577,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LogPrintf("Not relaying invalid transaction %s from whitelisted peer=%d (%s)\n", tx.GetHash().ToString(), pfrom->GetId(), FormatStateMessage(state)); } else { LogPrintf("Force relaying tx %s from whitelisted peer=%d\n", tx.GetHash().ToString(), pfrom->GetId()); - RelayTransaction(tx, connman); + RelayTransaction(tx.GetHash(), *connman); } } } diff --git a/src/net_processing.h b/src/net_processing.h index dffc3f273f..1d26164b18 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -90,4 +90,7 @@ struct CNodeStateStats { /** Get statistics from node state */ bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats); +/** Relay transaction to every node */ +void RelayTransaction(const uint256&, const CConnman& connman); + #endif // BITCOIN_NET_PROCESSING_H diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 5ffb15ed3c..8e56496358 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -5,6 +5,7 @@ #include <consensus/validation.h> #include <net.h> +#include <net_processing.h> #include <txmempool.h> #include <util/validation.h> #include <validation.h> @@ -13,26 +14,30 @@ #include <future> -TransactionError BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, std::string& err_string, const CAmount& highfee) +TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback) { + assert(g_connman); std::promise<void> promise; - hashTx = tx->GetHash(); + uint256 hashTx = tx->GetHash(); + bool callback_set = false; { // cs_main scope LOCK(cs_main); + // If the transaction is already confirmed in the chain, don't do anything + // and return early. CCoinsViewCache &view = *pcoinsTip; - bool fHaveChain = false; - for (size_t o = 0; !fHaveChain && o < tx->vout.size(); o++) { + for (size_t o = 0; o < tx->vout.size(); o++) { const Coin& existingCoin = view.AccessCoin(COutPoint(hashTx, o)); - fHaveChain = !existingCoin.IsSpent(); + // IsSpent doesnt mean the coin is spent, it means the output doesnt' exist. + // So if the output does exist, then this transaction exists in the chain. + if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_CHAIN; } - bool fHaveMempool = mempool.exists(hashTx); - if (!fHaveMempool && !fHaveChain) { - // push to local node and sync with wallets + if (!mempool.exists(hashTx)) { + // Transaction is not already in the mempool. Submit it. CValidationState state; bool fMissingInputs; if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs, - nullptr /* plTxnReplaced */, false /* bypass_limits */, highfee)) { + nullptr /* plTxnReplaced */, false /* bypass_limits */, max_tx_fee)) { if (state.IsInvalid()) { err_string = FormatStateMessage(state); return TransactionError::MEMPOOL_REJECTED; @@ -43,36 +48,37 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, err_string = FormatStateMessage(state); return TransactionError::MEMPOOL_ERROR; } - } else { - // If wallet is enabled, ensure that the wallet has been made aware - // of the new transaction prior to returning. This prevents a race - // where a user might call sendrawtransaction with a transaction - // to/from their wallet, immediately call some wallet RPC, and get - // a stale result because callbacks have not yet been processed. + } + + // Transaction was accepted to the mempool. + + if (wait_callback) { + // For transactions broadcast from outside the wallet, make sure + // that the wallet has been notified of the transaction before + // continuing. + // + // This prevents a race where a user might call sendrawtransaction + // with a transaction to/from their wallet, immediately call some + // wallet RPC, and get a stale result because callbacks have not + // yet been processed. CallFunctionInValidationInterfaceQueue([&promise] { promise.set_value(); }); + callback_set = true; } - } else if (fHaveChain) { - return TransactionError::ALREADY_IN_CHAIN; - } else { - // Make sure we don't block forever if re-sending - // a transaction already in mempool. - promise.set_value(); } } // cs_main - promise.get_future().wait(); - - if (!g_connman) { - return TransactionError::P2P_DISABLED; + if (callback_set) { + // Wait until Validation Interface clients have been notified of the + // transaction entering the mempool. + promise.get_future().wait(); } - CInv inv(MSG_TX, hashTx); - g_connman->ForEachNode([&inv](CNode* pnode) { - pnode->PushInventory(inv); - }); + if (relay) { + RelayTransaction(hashTx, *g_connman); + } return TransactionError::OK; } diff --git a/src/node/transaction.h b/src/node/transaction.h index 51033f94e5..cf64fc28d9 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -11,14 +11,21 @@ #include <util/error.h> /** - * Broadcast a transaction + * Submit a transaction to the mempool and (optionally) relay it to all P2P peers. + * + * Mempool submission can be synchronous (will await mempool entry notification + * over the CValidationInterface) or asynchronous (will submit and not wait for + * notification), depending on the value of wait_callback. wait_callback MUST + * NOT be set while cs_main, cs_mempool or cs_wallet are held to avoid + * deadlock. * * @param[in] tx the transaction to broadcast - * @param[out] &txid the txid of the transaction, if successfully broadcast * @param[out] &err_string reference to std::string to fill with error string if available - * @param[in] highfee Reject txs with fees higher than this (if 0, accept any fee) + * @param[in] max_tx_fee reject txs with fees higher than this (if 0, accept any fee) + * @param[in] relay flag if both mempool insertion and p2p relay are requested + * @param[in] wait_callback, wait until callbacks have been processed to avoid stale result due to a sequentially RPC. * return error */ -NODISCARD TransactionError BroadcastTransaction(CTransactionRef tx, uint256& txid, std::string& err_string, const CAmount& highfee); +NODISCARD TransactionError BroadcastTransaction(CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback); #endif // BITCOIN_NODE_TRANSACTION_H diff --git a/src/noui.cpp b/src/noui.cpp index caab9f326e..c07939cc79 100644 --- a/src/noui.cpp +++ b/src/noui.cpp @@ -13,6 +13,12 @@ #include <string> #include <boost/signals2/connection.hpp> +#include <boost/signals2/signal.hpp> + +/** Store connections so we can disconnect them when suppressing output */ +boost::signals2::connection noui_ThreadSafeMessageBoxConn; +boost::signals2::connection noui_ThreadSafeQuestionConn; +boost::signals2::connection noui_InitMessageConn; bool noui_ThreadSafeMessageBox(const std::string& message, const std::string& caption, unsigned int style) { @@ -57,7 +63,39 @@ void noui_InitMessage(const std::string& message) void noui_connect() { - uiInterface.ThreadSafeMessageBox_connect(noui_ThreadSafeMessageBox); - uiInterface.ThreadSafeQuestion_connect(noui_ThreadSafeQuestion); - uiInterface.InitMessage_connect(noui_InitMessage); + noui_ThreadSafeMessageBoxConn = uiInterface.ThreadSafeMessageBox_connect(noui_ThreadSafeMessageBox); + noui_ThreadSafeQuestionConn = uiInterface.ThreadSafeQuestion_connect(noui_ThreadSafeQuestion); + noui_InitMessageConn = uiInterface.InitMessage_connect(noui_InitMessage); +} + +bool noui_ThreadSafeMessageBoxSuppressed(const std::string& message, const std::string& caption, unsigned int style) +{ + return false; +} + +bool noui_ThreadSafeQuestionSuppressed(const std::string& /* ignored interactive message */, const std::string& message, const std::string& caption, unsigned int style) +{ + return false; } + +void noui_InitMessageSuppressed(const std::string& message) +{ +} + +void noui_suppress() +{ + noui_ThreadSafeMessageBoxConn.disconnect(); + noui_ThreadSafeQuestionConn.disconnect(); + noui_InitMessageConn.disconnect(); + noui_ThreadSafeMessageBoxConn = uiInterface.ThreadSafeMessageBox_connect(noui_ThreadSafeMessageBoxSuppressed); + noui_ThreadSafeQuestionConn = uiInterface.ThreadSafeQuestion_connect(noui_ThreadSafeQuestionSuppressed); + noui_InitMessageConn = uiInterface.InitMessage_connect(noui_InitMessageSuppressed); +} + +void noui_reconnect() +{ + noui_ThreadSafeMessageBoxConn.disconnect(); + noui_ThreadSafeQuestionConn.disconnect(); + noui_InitMessageConn.disconnect(); + noui_connect(); +}
\ No newline at end of file diff --git a/src/noui.h b/src/noui.h index 79a79a9af2..854aeeacca 100644 --- a/src/noui.h +++ b/src/noui.h @@ -17,4 +17,10 @@ void noui_InitMessage(const std::string& message); /** Connect all bitcoind signal handlers */ void noui_connect(); +/** Suppress all bitcoind signal handlers. Used to suppress output during test runs that produce expected errors */ +void noui_suppress(); + +/** Reconnects the regular Non-GUI handlers after having used noui_suppress */ +void noui_reconnect(); + #endif // BITCOIN_NOUI_H diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index e8b776b405..482bf0543d 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -25,7 +25,8 @@ #ifdef ENABLE_WALLET #include <qt/paymentserver.h> #include <qt/walletcontroller.h> -#endif +#include <qt/walletmodel.h> +#endif // ENABLE_WALLET #include <interfaces/handler.h> #include <interfaces/node.h> @@ -207,12 +208,6 @@ BitcoinApplication::~BitcoinApplication() delete window; window = nullptr; -#ifdef ENABLE_WALLET - delete paymentServer; - paymentServer = nullptr; - delete m_wallet_controller; - m_wallet_controller = nullptr; -#endif delete optionsModel; optionsModel = nullptr; delete platformStyle; @@ -328,24 +323,21 @@ void BitcoinApplication::initializeResult(bool success) { // Log this only after AppInitMain finishes, as then logging setup is guaranteed complete qInfo() << "Platform customization:" << platformStyle->getName(); -#ifdef ENABLE_WALLET - m_wallet_controller = new WalletController(m_node, platformStyle, optionsModel, this); -#ifdef ENABLE_BIP70 - PaymentServer::LoadRootCAs(); -#endif - if (paymentServer) { - paymentServer->setOptionsModel(optionsModel); -#ifdef ENABLE_BIP70 - connect(m_wallet_controller, &WalletController::coinsSent, paymentServer, &PaymentServer::fetchPaymentACK); -#endif - } -#endif - clientModel = new ClientModel(m_node, optionsModel); window->setClientModel(clientModel); #ifdef ENABLE_WALLET - window->setWalletController(m_wallet_controller); + if (WalletModel::isWalletEnabled()) { + m_wallet_controller = new WalletController(m_node, platformStyle, optionsModel, this); + window->setWalletController(m_wallet_controller); + if (paymentServer) { + paymentServer->setOptionsModel(optionsModel); +#ifdef ENABLE_BIP70 + PaymentServer::LoadRootCAs(); + connect(m_wallet_controller, &WalletController::coinsSent, paymentServer, &PaymentServer::fetchPaymentACK); #endif + } + } +#endif // ENABLE_WALLET // If -min option passed, start window minimized (iconified) or minimized to tray if (!gArgs.GetBoolArg("-min", false)) { @@ -549,8 +541,10 @@ int GuiMain(int argc, char* argv[]) // Start up the payment server early, too, so impatient users that click on // bitcoin: links repeatedly have their payment requests routed to this process: - app.createPaymentServer(); -#endif + if (WalletModel::isWalletEnabled()) { + app.createPaymentServer(); + } +#endif // ENABLE_WALLET /// 9. Main GUI initialization // Install global event filter that makes sure that long tooltips can be word-wrapped diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 84b4a2d0d8..cdf84eae9a 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1265,11 +1265,6 @@ void RPCConsole::showOrHideBanTableIfRequired() ui->banHeading->setVisible(visible); } -RPCConsole::TabTypes RPCConsole::tabFocus() const -{ - return (TabTypes) ui->tabWidget->currentIndex(); -} - void RPCConsole::setTabFocus(enum TabTypes tabType) { ui->tabWidget->setCurrentIndex(tabType); diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index 79b0f3b19c..38015e38fd 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -67,7 +67,6 @@ public: std::vector<TabTypes> tabs() const { return {TAB_INFO, TAB_CONSOLE, TAB_GRAPH, TAB_PEERS}; } - TabTypes tabFocus() const; QString tabTitle(TabTypes tab_type) const; protected: diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index cb9efe9319..193fba78b1 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -392,7 +392,7 @@ void SendCoinsDialog::on_sendButton_clicked() accept(); CoinControlDialog::coinControl()->UnSelectAll(); coinControlUpdateLabels(); - Q_EMIT coinsSent(currentTransaction.getWtx()->get().GetHash()); + Q_EMIT coinsSent(currentTransaction.getWtx()->GetHash()); } fNewRecipientAllowed = true; } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 57406179f7..49a13330ec 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -261,11 +261,11 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran auto& newTx = transaction.getWtx(); std::string rejectReason; - if (!newTx->commit({} /* mapValue */, std::move(vOrderForm), rejectReason)) + if (!wallet().commitTransaction(newTx, {} /* mapValue */, std::move(vOrderForm), rejectReason)) return SendCoinsReturn(TransactionCommitFailed, QString::fromStdString(rejectReason)); CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); - ssTx << newTx->get(); + ssTx << *newTx; transaction_array.append(&(ssTx[0]), ssTx.size()); } diff --git a/src/qt/walletmodeltransaction.cpp b/src/qt/walletmodeltransaction.cpp index 8c0dc276b0..d00ccf70d9 100644 --- a/src/qt/walletmodeltransaction.cpp +++ b/src/qt/walletmodeltransaction.cpp @@ -21,14 +21,14 @@ QList<SendCoinsRecipient> WalletModelTransaction::getRecipients() const return recipients; } -std::unique_ptr<interfaces::PendingWalletTx>& WalletModelTransaction::getWtx() +CTransactionRef& WalletModelTransaction::getWtx() { return wtx; } unsigned int WalletModelTransaction::getTransactionSize() { - return wtx ? GetVirtualTransactionSize(wtx->get()) : 0; + return wtx ? GetVirtualTransactionSize(*wtx) : 0; } CAmount WalletModelTransaction::getTransactionFee() const @@ -43,7 +43,7 @@ void WalletModelTransaction::setTransactionFee(const CAmount& newFee) void WalletModelTransaction::reassignAmounts(int nChangePosRet) { - const CTransaction* walletTransaction = &wtx->get(); + const CTransaction* walletTransaction = wtx.get(); int i = 0; for (QList<SendCoinsRecipient>::iterator it = recipients.begin(); it != recipients.end(); ++it) { diff --git a/src/qt/walletmodeltransaction.h b/src/qt/walletmodeltransaction.h index 289aee847b..a41d8f2457 100644 --- a/src/qt/walletmodeltransaction.h +++ b/src/qt/walletmodeltransaction.h @@ -16,7 +16,6 @@ class SendCoinsRecipient; namespace interfaces { class Node; -class PendingWalletTx; } /** Data model for a walletmodel transaction. */ @@ -27,7 +26,7 @@ public: QList<SendCoinsRecipient> getRecipients() const; - std::unique_ptr<interfaces::PendingWalletTx>& getWtx(); + CTransactionRef& getWtx(); unsigned int getTransactionSize(); void setTransactionFee(const CAmount& newFee); @@ -39,7 +38,7 @@ public: private: QList<SendCoinsRecipient> recipients; - std::unique_ptr<interfaces::PendingWalletTx> wtx; + CTransactionRef wtx; CAmount fee; }; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 532765b3d8..966c159f0f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -406,7 +406,11 @@ static UniValue createrawtransaction(const JSONRPCRequest& request) }, true ); - CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], request.params[3]); + bool rbf = false; + if (!request.params[3].isNull()) { + rbf = request.params[3].isTrue(); + } + CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); return EncodeHexTx(CTransaction(rawTx)); } @@ -760,7 +764,10 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request) static UniValue sendrawtransaction(const JSONRPCRequest& request) { RPCHelpMan{"sendrawtransaction", - "\nSubmits raw transaction (serialized, hex-encoded) to local node and network.\n" + "\nSubmit a raw transaction (serialized, hex-encoded) to local node and network.\n" + "\nNote that the transaction will be sent unconditionally to all peers, so using this\n" + "for manual rebroadcast may degrade privacy by leaking the transaction's origin, as\n" + "nodes will normally not rebroadcast non-wallet transactions already in their mempool.\n" "\nAlso see createrawtransaction and signrawtransactionwithkey calls.\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, @@ -807,14 +814,14 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) max_raw_tx_fee = fr.GetFee((weight+3)/4); } - uint256 txid; std::string err_string; - const TransactionError err = BroadcastTransaction(tx, txid, err_string, max_raw_tx_fee); + AssertLockNotHeld(cs_main); + const TransactionError err = BroadcastTransaction(tx, err_string, max_raw_tx_fee, /*relay*/ true, /*wait_callback*/ true); if (TransactionError::OK != err) { throw JSONRPCTransactionError(err, err_string); } - return txid.GetHex(); + return tx->GetHash().GetHex(); } static UniValue testmempoolaccept(const JSONRPCRequest& request) @@ -1362,7 +1369,11 @@ UniValue createpsbt(const JSONRPCRequest& request) }, true ); - CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], request.params[3]); + bool rbf = false; + if (!request.params[3].isNull()) { + rbf = request.params[3].isTrue(); + } + CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); // Make a blank psbt PartiallySignedTransaction psbtx; diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 1c96d01232..55425cca35 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -19,7 +19,7 @@ #include <util/rbf.h> #include <util/strencodings.h> -CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, const UniValue& rbf) +CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, bool rbf) { if (inputs_in.isNull() || outputs_in.isNull()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, arguments 1 and 2 must be non-null"); @@ -37,8 +37,6 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal rawTx.nLockTime = nLockTime; } - bool rbfOptIn = rbf.isTrue(); - for (unsigned int idx = 0; idx < inputs.size(); idx++) { const UniValue& input = inputs[idx]; const UniValue& o = input.get_obj(); @@ -53,7 +51,7 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout must be positive"); uint32_t nSequence; - if (rbfOptIn) { + if (rbf) { nSequence = MAX_BIP125_RBF_SEQUENCE; /* CTxIn::SEQUENCE_FINAL - 2 */ } else if (rawTx.nLockTime) { nSequence = CTxIn::SEQUENCE_FINAL - 1; @@ -125,7 +123,7 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal } } - if (!rbf.isNull() && rawTx.vin.size() > 0 && rbfOptIn != SignalsOptInRBF(CTransaction(rawTx))) { + if (rbf && rawTx.vin.size() > 0 && !SignalsOptInRBF(CTransaction(rawTx))) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter combination: Sequence number(s) contradict replaceable option"); } diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index d198887b93..c85593e71e 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -27,6 +27,6 @@ class COutPoint; UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxs, FillableSigningProvider* keystore, std::map<COutPoint, Coin>& coins, bool tempKeystore, const UniValue& hashType); /** Create a transaction from univalue parameters */ -CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, const UniValue& rbf); +CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, bool rbf); #endif // BITCOIN_RPC_RAWTRANSACTION_UTIL_H diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index dac201a35f..5ce8e6feb0 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -85,7 +85,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); size_t poolSize = pool.size(); - pool.removeRecursive(*block.vtx[2]); + pool.removeRecursive(*block.vtx[2], MemPoolRemovalReason::REPLACED); BOOST_CHECK_EQUAL(pool.size(), poolSize - 1); CBlock block2; diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index c6a3de2285..fe5d31b7d3 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -14,6 +14,8 @@ BOOST_FIXTURE_TEST_SUITE(mempool_tests, TestingSetup) +static constexpr auto REMOVAL_REASON_DUMMY = MemPoolRemovalReason::REPLACED; + BOOST_AUTO_TEST_CASE(MempoolRemoveTest) { // Test CTxMemPool::remove functionality @@ -59,13 +61,13 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) // Nothing in pool, remove should do nothing: unsigned int poolSize = testPool.size(); - testPool.removeRecursive(CTransaction(txParent)); + testPool.removeRecursive(CTransaction(txParent), REMOVAL_REASON_DUMMY); BOOST_CHECK_EQUAL(testPool.size(), poolSize); // Just the parent: testPool.addUnchecked(entry.FromTx(txParent)); poolSize = testPool.size(); - testPool.removeRecursive(CTransaction(txParent)); + testPool.removeRecursive(CTransaction(txParent), REMOVAL_REASON_DUMMY); BOOST_CHECK_EQUAL(testPool.size(), poolSize - 1); // Parent, children, grandchildren: @@ -77,18 +79,18 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) } // Remove Child[0], GrandChild[0] should be removed: poolSize = testPool.size(); - testPool.removeRecursive(CTransaction(txChild[0])); + testPool.removeRecursive(CTransaction(txChild[0]), REMOVAL_REASON_DUMMY); BOOST_CHECK_EQUAL(testPool.size(), poolSize - 2); // ... make sure grandchild and child are gone: poolSize = testPool.size(); - testPool.removeRecursive(CTransaction(txGrandChild[0])); + testPool.removeRecursive(CTransaction(txGrandChild[0]), REMOVAL_REASON_DUMMY); BOOST_CHECK_EQUAL(testPool.size(), poolSize); poolSize = testPool.size(); - testPool.removeRecursive(CTransaction(txChild[0])); + testPool.removeRecursive(CTransaction(txChild[0]), REMOVAL_REASON_DUMMY); BOOST_CHECK_EQUAL(testPool.size(), poolSize); // Remove parent, all children/grandchildren should go: poolSize = testPool.size(); - testPool.removeRecursive(CTransaction(txParent)); + testPool.removeRecursive(CTransaction(txParent), REMOVAL_REASON_DUMMY); BOOST_CHECK_EQUAL(testPool.size(), poolSize - 5); BOOST_CHECK_EQUAL(testPool.size(), 0U); @@ -101,7 +103,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) // Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be // put into the mempool (maybe because it is non-standard): poolSize = testPool.size(); - testPool.removeRecursive(CTransaction(txParent)); + testPool.removeRecursive(CTransaction(txParent), REMOVAL_REASON_DUMMY); BOOST_CHECK_EQUAL(testPool.size(), poolSize - 6); BOOST_CHECK_EQUAL(testPool.size(), 0U); } @@ -283,11 +285,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) BOOST_CHECK_EQUAL(pool.size(), 10U); // Now try removing tx10 and verify the sort order returns to normal - pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx()); + pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); CheckSort<descendant_score>(pool, snapshotOrder); - pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx()); - pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx()); + pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); + pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); } BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 4bd40687a6..05d7f76983 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -158,7 +158,7 @@ static void TestPackageSelection(const CChainParams& chainparams, const CScript& // Test that packages above the min relay fee do get included, even if one // of the transactions is below the min relay fee // Remove the low fee transaction and replace with a higher fee transaction - mempool.removeRecursive(CTransaction(tx)); + mempool.removeRecursive(CTransaction(tx), MemPoolRemovalReason::REPLACED); tx.vout[0].nValue -= 2; // Now we should be just over the min relay fee hashLowFeeTx = tx.GetHash(); mempool.addUnchecked(entry.Fee(feeToUse+2).FromTx(tx)); diff --git a/src/txmempool.h b/src/txmempool.h index 565dd61f0f..7169e80da2 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -345,7 +345,6 @@ struct TxMempoolInfo * this is passed to the notification signal. */ enum class MemPoolRemovalReason { - UNKNOWN = 0, //!< Manually removed or unknown reason EXPIRY, //!< Expired from mempool SIZELIMIT, //!< Removed in size limiting REORG, //!< Removed for reorganization @@ -574,7 +573,7 @@ public: void addUnchecked(const CTxMemPoolEntry& entry, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); - void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -613,7 +612,7 @@ public: * Set updateDescendants to true when removing a tx that was in a block, so * that any in-mempool descendants have their ancestor state updated. */ - void RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN) EXCLUSIVE_LOCKS_REQUIRED(cs); + void RemoveStaged(setEntries& stage, bool updateDescendants, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); /** When adding transactions from a disconnected block back to the mempool, * new mempool entries may have children in the mempool (which is generally @@ -735,7 +734,7 @@ private: * transactions in a chain before we've updated all the state for the * removal. */ - void removeUnchecked(txiter entry, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeUnchecked(txiter entry, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); }; /** diff --git a/src/validation.cpp b/src/validation.cpp index 19f4f098d7..b4677df62f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -618,6 +618,8 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool std::string errString; if (!pool.CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize, nLimitDescendants, nLimitDescendantSize, errString)) { setAncestors.clear(); + // If CalculateMemPoolAncestors fails second time, we want the original error string. + std::string dummy_err_string; // If the new transaction is relatively small (up to 40k weight) // and has at most one ancestor (ie ancestor limit of 2, including // the new transaction), allow it if its parent has exactly the @@ -629,7 +631,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // outputs - one for each counterparty. For more info on the uses for // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || - !pool.CalculateMemPoolAncestors(entry, setAncestors, 2, nLimitAncestorSize, nLimitDescendants + 1, nLimitDescendantSize + EXTRA_DESCENDANT_TX_SIZE_LIMIT, errString)) { + !pool.CalculateMemPoolAncestors(entry, setAncestors, 2, nLimitAncestorSize, nLimitDescendants + 1, nLimitDescendantSize + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too-long-mempool-chain", errString); } } diff --git a/src/wallet/coincontrol.cpp b/src/wallet/coincontrol.cpp index 60bce66839..14513bc9e9 100644 --- a/src/wallet/coincontrol.cpp +++ b/src/wallet/coincontrol.cpp @@ -20,5 +20,7 @@ void CCoinControl::SetNull() m_confirm_target.reset(); m_signal_bip125_rbf.reset(); m_fee_mode = FeeEstimateMode::UNSET; + m_min_depth = DEFAULT_MIN_DEPTH; + m_max_depth = DEFAULT_MAX_DEPTH; } diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 249c402e4d..92a290530c 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -12,6 +12,9 @@ #include <boost/optional.hpp> +const int DEFAULT_MIN_DEPTH = 0; +const int DEFAULT_MAX_DEPTH = 9999999; + /** Coin Control Features. */ class CCoinControl { @@ -39,7 +42,9 @@ public: //! Fee estimation mode to control arguments to estimateSmartFee FeeEstimateMode m_fee_mode; //! Minimum chain depth value for coin availability - int m_min_depth{0}; + int m_min_depth = DEFAULT_MIN_DEPTH; + //! Maximum chain depth value for coin availability + int m_max_depth = DEFAULT_MAX_DEPTH; CCoinControl() { diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 852b194386..26aeb754ad 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -585,7 +585,7 @@ BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode, bo if (fCreate && !Exists(std::string("version"))) { bool fTmp = fReadOnly; fReadOnly = false; - WriteVersion(CLIENT_VERSION); + Write(std::string("version"), CLIENT_VERSION); fReadOnly = fTmp; } } diff --git a/src/wallet/db.h b/src/wallet/db.h index b3856fbaf9..94f41eaf16 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -399,17 +399,6 @@ public: return (ret == 0); } - bool ReadVersion(int& nVersion) - { - nVersion = 0; - return Read(std::string("version"), nVersion); - } - - bool WriteVersion(int nVersion) - { - return Write(std::string("version"), nVersion); - } - bool static Rewrite(BerkeleyDatabase& database, const char* pszSkip = nullptr); }; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index ab732dc0d8..f94214b6ee 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -359,8 +359,8 @@ static UniValue sendtoaddress(const JSONRPCRequest& request) " transaction, just kept in your wallet."}, {"subtractfeefromamount", RPCArg::Type::BOOL, /* default */ "false", "The fee will be deducted from the amount being sent.\n" " The recipient will receive less bitcoins than you enter in the amount field."}, - {"replaceable", RPCArg::Type::BOOL, /* default */ "fallback to wallet's default", "Allow this transaction to be replaced by a transaction with higher fees via BIP 125"}, - {"conf_target", RPCArg::Type::NUM, /* default */ "fallback to wallet's default", "Confirmation target (in blocks)"}, + {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Allow this transaction to be replaced by a transaction with higher fees via BIP 125"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n" " \"UNSET\"\n" " \"ECONOMICAL\"\n" @@ -815,8 +815,8 @@ static UniValue sendmany(const JSONRPCRequest& request) {"address", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Subtract fee from this address"}, }, }, - {"replaceable", RPCArg::Type::BOOL, /* default */ "fallback to wallet's default", "Allow this transaction to be replaced by a transaction with higher fees via BIP 125"}, - {"conf_target", RPCArg::Type::NUM, /* default */ "fallback to wallet's default", "Confirmation target (in blocks)"}, + {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Allow this transaction to be replaced by a transaction with higher fees via BIP 125"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n" " \"UNSET\"\n" " \"ECONOMICAL\"\n" @@ -2676,11 +2676,12 @@ static UniValue createwallet(const JSONRPCRequest& request) } SecureString passphrase; passphrase.reserve(100); + std::string warning; if (!request.params[3].isNull()) { passphrase = request.params[3].get_str().c_str(); if (passphrase.empty()) { - // Empty string is invalid - throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Cannot encrypt a wallet with a blank password"); + // Empty string means unencrypted + warning = "Empty string given as passphrase, wallet will not be encrypted."; } } @@ -2689,15 +2690,23 @@ static UniValue createwallet(const JSONRPCRequest& request) } std::string error; - std::string warning; - WalletCreationStatus status; - std::shared_ptr<CWallet> wallet = CreateWallet(*g_rpc_interfaces->chain, request.params[0].get_str(), error, warning, status, passphrase, flags); - if (status == WalletCreationStatus::CREATION_FAILED) { - throw JSONRPCError(RPC_WALLET_ERROR, error); - } else if (status == WalletCreationStatus::ENCRYPTION_FAILED) { - throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, error); - } else if (status != WalletCreationStatus::SUCCESS) { - throw JSONRPCError(RPC_WALLET_ERROR, "Wallet creation failed"); + std::string create_warning; + std::shared_ptr<CWallet> wallet; + WalletCreationStatus status = CreateWallet(*g_rpc_interfaces->chain, passphrase, flags, request.params[0].get_str(), error, create_warning, wallet); + switch (status) { + case WalletCreationStatus::CREATION_FAILED: + throw JSONRPCError(RPC_WALLET_ERROR, error); + case WalletCreationStatus::ENCRYPTION_FAILED: + throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, error); + case WalletCreationStatus::SUCCESS: + break; + // no default case, so the compiler can warn about missing cases + } + + if (warning.empty()) { + warning = create_warning; + } else if (!warning.empty() && !create_warning.empty()){ + warning += "; " + create_warning; } UniValue obj(UniValue::VOBJ); @@ -2877,9 +2886,11 @@ static UniValue listunspent(const JSONRPCRequest& request) { CCoinControl cctl; cctl.m_avoid_address_reuse = false; + cctl.m_min_depth = nMinDepth; + cctl.m_max_depth = nMaxDepth; auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); - pwallet->AvailableCoins(*locked_chain, vecOutputs, !include_unsafe, &cctl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount, nMinDepth, nMaxDepth); + pwallet->AvailableCoins(*locked_chain, vecOutputs, !include_unsafe, &cctl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount); } LOCK(pwallet->cs_wallet); @@ -3110,9 +3121,9 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, }, - {"replaceable", RPCArg::Type::BOOL, /* default */ "fallback to wallet's default", "Marks this transaction as BIP125 replaceable.\n" + {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Marks this transaction as BIP125 replaceable.\n" " Allows this transaction to be replaced by a transaction with higher fees"}, - {"conf_target", RPCArg::Type::NUM, /* default */ "fallback to wallet's default", "Confirmation target (in blocks)"}, + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n" " \"UNSET\"\n" " \"ECONOMICAL\"\n" @@ -3263,20 +3274,20 @@ static UniValue bumpfee(const JSONRPCRequest& request) "\nBumps the fee of an opt-in-RBF transaction T, replacing it with a new transaction B.\n" "An opt-in RBF transaction with the given txid must be in the wallet.\n" "The command will pay the additional fee by reducing change outputs or adding inputs when necessary. It may add a new change output if one does not already exist.\n" - "If `totalFee` is given, adding inputs is not supported, so there must be a single change output that is big enough or it will fail.\n" + "If `totalFee` (DEPRECATED) is given, adding inputs is not supported, so there must be a single change output that is big enough or it will fail.\n" "All inputs in the original transaction will be included in the replacement transaction.\n" "The command will fail if the wallet or mempool contains a transaction that spends one of T's outputs.\n" "By default, the new fee will be calculated automatically using estimatesmartfee.\n" "The user can specify a confirmation target for estimatesmartfee.\n" - "Alternatively, the user can specify totalFee, or use RPC settxfee to set a higher fee rate.\n" + "Alternatively, the user can specify totalFee (DEPRECATED), or use RPC settxfee to set a higher fee rate.\n" "At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee\n" "returned by getnetworkinfo) to enter the node's mempool.\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The txid to be bumped"}, {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", { - {"confTarget", RPCArg::Type::NUM, /* default */ "fallback to wallet's default", "Confirmation target (in blocks)"}, - {"totalFee", RPCArg::Type::NUM, /* default */ "fallback to 'confTarget'", "Total fee (NOT feerate) to pay, in satoshis.\n" + {"confTarget", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"}, + {"totalFee", RPCArg::Type::NUM, /* default */ "fallback to 'confTarget'", "Total fee (NOT feerate) to pay, in satoshis. (DEPRECATED)\n" " In rare cases, the actual fee paid might be slightly higher than the specified\n" " totalFee if the tx change output has to be removed because it is too close to\n" " the dust threshold."}, @@ -3331,6 +3342,9 @@ static UniValue bumpfee(const JSONRPCRequest& request) } else if (options.exists("confTarget")) { // TODO: alias this to conf_target coin_control.m_confirm_target = ParseConfirmTarget(options["confTarget"], pwallet->chain().estimateMaxBlocks()); } else if (options.exists("totalFee")) { + if (!pwallet->chain().rpcEnableDeprecated("totalFee")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "totalFee argument has been deprecated and will be removed in 0.20. Please use -deprecatedrpc=totalFee to continue using this argument until removal."); + } totalFee = options["totalFee"].get_int64(); if (totalFee <= 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid totalFee %s (must be greater than 0)", FormatMoney(totalFee))); @@ -4052,7 +4066,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, }, - {"replaceable", RPCArg::Type::BOOL, /* default */ "false", "Marks this transaction as BIP125 replaceable.\n" + {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Marks this transaction as BIP125 replaceable.\n" " Allows this transaction to be replaced by a transaction with higher fees"}, {"conf_target", RPCArg::Type::NUM, /* default */ "Fallback to wallet's confirmation target", "Confirmation target (in blocks)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n" @@ -4087,7 +4101,13 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) CAmount fee; int change_position; - CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], request.params[3]["replaceable"]); + bool rbf = pwallet->m_signal_rbf; + const UniValue &replaceable_arg = request.params[3]["replaceable"]; + if (!replaceable_arg.isNull()) { + RPCTypeCheckArgument(replaceable_arg, UniValue::VBOOL); + rbf = replaceable_arg.isTrue(); + } + CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); FundTransaction(pwallet, rawTx, fee, change_position, request.params[3]); // Make a blank psbt diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp index 1816fca257..279542ffad 100644 --- a/src/wallet/test/init_tests.cpp +++ b/src/wallet/test/init_tests.cpp @@ -4,6 +4,7 @@ #include <boost/test/unit_test.hpp> +#include <noui.h> #include <test/setup_common.h> #include <util/system.h> #include <wallet/test/init_test_fixture.h> @@ -33,21 +34,27 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_custom) BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_does_not_exist) { SetWalletDir(m_walletdir_path_cases["nonexistent"]); + noui_suppress(); bool result = m_chain_client->verify(); + noui_reconnect(); BOOST_CHECK(result == false); } BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_directory) { SetWalletDir(m_walletdir_path_cases["file"]); + noui_suppress(); bool result = m_chain_client->verify(); + noui_reconnect(); BOOST_CHECK(result == false); } BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_relative) { SetWalletDir(m_walletdir_path_cases["relative"]); + noui_suppress(); bool result = m_chain_client->verify(); + noui_reconnect(); BOOST_CHECK(result == false); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index eddce2850d..b3269083ec 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -161,7 +161,7 @@ std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& return LoadWallet(chain, WalletLocation(name), error, warning); } -std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::string& warning, WalletCreationStatus& status, const SecureString& passphrase, uint64_t wallet_creation_flags) +WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::string& warning, std::shared_ptr<CWallet>& result) { // Indicate that the wallet is actually supposed to be blank and not just blank to make it encrypted bool create_blank = (wallet_creation_flags & WALLET_FLAG_BLANK_WALLET); @@ -175,39 +175,40 @@ std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::strin WalletLocation location(name); if (location.Exists()) { error = "Wallet " + location.GetName() + " already exists."; - status = WalletCreationStatus::CREATION_FAILED; - return nullptr; + return WalletCreationStatus::CREATION_FAILED; } // Wallet::Verify will check if we're trying to create a wallet with a duplicate name. std::string wallet_error; if (!CWallet::Verify(chain, location, false, wallet_error, warning)) { error = "Wallet file verification failed: " + wallet_error; - status = WalletCreationStatus::CREATION_FAILED; - return nullptr; + return WalletCreationStatus::CREATION_FAILED; + } + + // Do not allow a passphrase when private keys are disabled + if (!passphrase.empty() && (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + error = "Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled."; + return WalletCreationStatus::CREATION_FAILED; } // Make the wallet std::shared_ptr<CWallet> wallet = CWallet::CreateWalletFromFile(chain, location, wallet_creation_flags); if (!wallet) { error = "Wallet creation failed"; - status = WalletCreationStatus::CREATION_FAILED; - return nullptr; + return WalletCreationStatus::CREATION_FAILED; } // Encrypt the wallet if (!passphrase.empty() && !(wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { if (!wallet->EncryptWallet(passphrase)) { error = "Error: Wallet created but failed to encrypt."; - status = WalletCreationStatus::ENCRYPTION_FAILED; - return nullptr; + return WalletCreationStatus::ENCRYPTION_FAILED; } if (!create_blank) { // Unlock the wallet if (!wallet->Unlock(passphrase)) { error = "Error: Wallet was encrypted but could not be unlocked"; - status = WalletCreationStatus::ENCRYPTION_FAILED; - return nullptr; + return WalletCreationStatus::ENCRYPTION_FAILED; } // Set a seed for the wallet @@ -221,13 +222,13 @@ std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::strin } AddWallet(wallet); wallet->postInitProcess(); - status = WalletCreationStatus::SUCCESS; - return wallet; + result = wallet; + return WalletCreationStatus::SUCCESS; } const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; -const uint256 CMerkleTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); +const uint256 CWalletTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); /** @defgroup mapWallet * @@ -2133,8 +2134,7 @@ void CWallet::ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain) std::map<int64_t, CWalletTx*> mapSorted; // Sort pending wallet transactions based on their initial wallet insertion order - for (std::pair<const uint256, CWalletTx>& item : mapWallet) - { + for (std::pair<const uint256, CWalletTx>& item : mapWallet) { const uint256& wtxid = item.first; CWalletTx& wtx = item.second; assert(wtx.GetHash() == wtxid); @@ -2149,32 +2149,32 @@ void CWallet::ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain) // Try to add wallet transactions to memory pool for (const std::pair<const int64_t, CWalletTx*>& item : mapSorted) { CWalletTx& wtx = *(item.second); - CValidationState state; - wtx.AcceptToMemoryPool(locked_chain, state); + std::string unused_err_string; + wtx.SubmitMemoryPoolAndRelay(unused_err_string, false); } } -bool CWalletTx::RelayWalletTransaction(interfaces::Chain::Lock& locked_chain) +bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay) { // Can't relay if wallet is not broadcasting if (!pwallet->GetBroadcastTransactions()) return false; - // Don't relay coinbase transactions outside blocks - if (IsCoinBase()) return false; // Don't relay abandoned transactions if (isAbandoned()) return false; - // Don't relay conflicted or already confirmed transactions - if (GetDepthInMainChain(locked_chain) != 0) return false; - // Don't relay transactions that aren't accepted to the mempool - CValidationState unused_state; - if (!InMempool() && !AcceptToMemoryPool(locked_chain, unused_state)) return false; - // Don't try to relay if the node is not connected to the p2p network - if (!pwallet->chain().p2pEnabled()) return false; - - // Try to relay the transaction - pwallet->WalletLogPrintf("Relaying wtx %s\n", GetHash().ToString()); - pwallet->chain().relayTransaction(GetHash()); - return true; + // Submit transaction to mempool for relay + pwallet->WalletLogPrintf("Submitting wtx %s to mempool for relay\n", GetHash().ToString()); + // We must set fInMempool here - while it will be re-set to true by the + // entered-mempool callback, if we did not there would be a race where a + // user could call sendmoney in a loop and hit spurious out of funds errors + // because we think that this newly generated transaction's change is + // unavailable as we're not yet aware that it is in the mempool. + // + // Irrespective of the failure reason, un-marking fInMempool + // out-of-order is incorrect - it should be unmarked when + // TransactionRemovedFromMempool fires. + bool ret = pwallet->chain().broadcastTransaction(tx, err_string, pwallet->m_default_max_tx_fee, relay); + fInMempool |= ret; + return ret; } std::set<uint256> CWalletTx::GetConflicts() const @@ -2365,7 +2365,7 @@ void CWallet::ResendWalletTransactions() if (m_best_block_time < nLastResend) return; nLastResend = GetTime(); - int relayed_tx_count = 0; + int submitted_tx_count = 0; { // locked_chain and cs_wallet scope auto locked_chain = chain().lock(); @@ -2377,12 +2377,13 @@ void CWallet::ResendWalletTransactions() // only rebroadcast unconfirmed txes older than 5 minutes before the // last block was found if (wtx.nTimeReceived > m_best_block_time - 5 * 60) continue; - if (wtx.RelayWalletTransaction(*locked_chain)) ++relayed_tx_count; + std::string unused_err_string; + if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true)) ++submitted_tx_count; } } // locked_chain and cs_wallet - if (relayed_tx_count > 0) { - WalletLogPrintf("%s: rebroadcast %u unconfirmed transactions\n", __func__, relayed_tx_count); + if (submitted_tx_count > 0) { + WalletLogPrintf("%s: resubmit %u unconfirmed transactions\n", __func__, submitted_tx_count); } } @@ -2447,7 +2448,7 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const return balance; } -void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<COutput> &vCoins, bool fOnlySafe, const CCoinControl *coinControl, const CAmount &nMinimumAmount, const CAmount &nMaximumAmount, const CAmount &nMinimumSumAmount, const uint64_t nMaximumCount, const int nMinDepth, const int nMaxDepth) const +void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<COutput>& vCoins, bool fOnlySafe, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount& nMinimumSumAmount, const uint64_t nMaximumCount) const { AssertLockHeld(cs_wallet); @@ -2456,6 +2457,8 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector< // Either the WALLET_FLAG_AVOID_REUSE flag is not set (in which case we always allow), or we default to avoiding, and only in the case where // a coin control object is provided, and has the avoid address reuse flag set to false, do we allow already used addresses bool allow_used_addresses = !IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) || (coinControl && !coinControl->m_avoid_address_reuse); + const int min_depth = {coinControl ? coinControl->m_min_depth : DEFAULT_MIN_DEPTH}; + const int max_depth = {coinControl ? coinControl->m_max_depth : DEFAULT_MAX_DEPTH}; for (const auto& entry : mapWallet) { @@ -2515,8 +2518,9 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector< continue; } - if (nDepth < nMinDepth || nDepth > nMaxDepth) + if (nDepth < min_depth || nDepth > max_depth) { continue; + } for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) { if (wtx.tx->vout[i].nValue < nMinimumAmount || wtx.tx->vout[i].nValue > nMaximumAmount) @@ -2958,7 +2962,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std LOCK(cs_wallet); { std::vector<COutput> vAvailableCoins; - AvailableCoins(*locked_chain, vAvailableCoins, true, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0, coin_control.m_min_depth); + AvailableCoins(*locked_chain, vAvailableCoins, true, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0); CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy // Create change script that will be used if we need change @@ -3318,12 +3322,10 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve if (fBroadcastTransactions) { - // Broadcast - if (!wtx.AcceptToMemoryPool(*locked_chain, state)) { - WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", FormatStateMessage(state)); + std::string err_string; + if (!wtx.SubmitMemoryPoolAndRelay(err_string, true)) { + WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string); // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure. - } else { - wtx.RelayWalletTransaction(*locked_chain); } } } @@ -4409,7 +4411,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, walletInstance->m_min_fee = CFeeRate(n); } - walletInstance->m_allow_fallback_fee = Params().IsFallbackFeeEnabled(); + walletInstance->m_allow_fallback_fee = Params().IsTestChain(); if (gArgs.IsArgSet("-fallbackfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-fallbackfee", ""), nFeePerK)) { @@ -4626,13 +4628,7 @@ CKeyPool::CKeyPool(const CPubKey& vchPubKeyIn, bool internalIn) m_pre_split = false; } -CWalletKey::CWalletKey(int64_t nExpires) -{ - nTimeCreated = (nExpires ? GetTime() : 0); - nTimeExpires = nExpires; -} - -void CMerkleTx::SetMerkleBranch(const uint256& block_hash, int posInBlock) +void CWalletTx::SetMerkleBranch(const uint256& block_hash, int posInBlock) { // Update the tx's hashBlock hashBlock = block_hash; @@ -4641,7 +4637,7 @@ void CMerkleTx::SetMerkleBranch(const uint256& block_hash, int posInBlock) nIndex = posInBlock; } -int CMerkleTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const +int CWalletTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const { if (hashUnset()) return 0; @@ -4649,7 +4645,7 @@ int CMerkleTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const return locked_chain.getBlockDepth(hashBlock) * (nIndex == -1 ? -1 : 1); } -int CMerkleTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const +int CWalletTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const { if (!IsCoinBase()) return 0; @@ -4658,24 +4654,12 @@ int CMerkleTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const return std::max(0, (COINBASE_MATURITY+1) - chain_depth); } -bool CMerkleTx::IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const +bool CWalletTx::IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const { // note GetBlocksToMaturity is 0 for non-coinbase tx return GetBlocksToMaturity(locked_chain) > 0; } -bool CWalletTx::AcceptToMemoryPool(interfaces::Chain::Lock& locked_chain, CValidationState& state) -{ - // We must set fInMempool here - while it will be re-set to true by the - // entered-mempool callback, if we did not there would be a race where a - // user could call sendmoney in a loop and hit spurious out of funds errors - // because we think that this newly generated transaction's change is - // unavailable as we're not yet aware that it is in the mempool. - bool ret = locked_chain.submitToMemoryPool(tx, pwallet->m_default_max_tx_fee, state); - fInMempool |= ret; - return ret; -} - void CWallet::LearnRelatedScripts(const CPubKey& key, OutputType type) { if (key.IsCompressed() && (type == OutputType::P2SH_SEGWIT || type == OutputType::BECH32)) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 69b8acc8c8..3a45c1ccc5 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -12,7 +12,6 @@ #include <outputtype.h> #include <policy/feerate.h> #include <script/sign.h> -#include <streams.h> #include <tinyformat.h> #include <ui_interface.h> #include <util/strencodings.h> @@ -51,13 +50,13 @@ std::vector<std::shared_ptr<CWallet>> GetWallets(); std::shared_ptr<CWallet> GetWallet(const std::string& name); std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::string& warning); -enum WalletCreationStatus { +enum class WalletCreationStatus { SUCCESS, CREATION_FAILED, ENCRYPTION_FAILED }; -std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::string& warning, WalletCreationStatus& status, const SecureString& passphrase, uint64_t wallet_creation_flags); +WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::string& warning, std::shared_ptr<CWallet>& result); //! Default for -keypool static const unsigned int DEFAULT_KEYPOOL_SIZE = 1000; @@ -365,82 +364,24 @@ struct COutputEntry int vout; }; -/** A transaction with a merkle branch linking it to the block chain. */ +/** Legacy class used for deserializing vtxPrev for backwards compatibility. + * vtxPrev was removed in commit 93a18a3650292afbb441a47d1fa1b94aeb0164e3, + * but old wallet.dat files may still contain vtxPrev vectors of CMerkleTxs. + * These need to get deserialized for field alignment when deserializing + * a CWalletTx, but the deserialized values are discarded.**/ class CMerkleTx { -private: - /** Constant used in hashBlock to indicate tx has been abandoned */ - static const uint256 ABANDON_HASH; - public: - CTransactionRef tx; - uint256 hashBlock; - - /* An nIndex == -1 means that hashBlock (in nonzero) refers to the earliest - * block in the chain we know this or any in-wallet dependency conflicts - * with. Older clients interpret nIndex == -1 as unconfirmed for backward - * compatibility. - */ - int nIndex; - - CMerkleTx() - { - SetTx(MakeTransactionRef()); - Init(); - } - - explicit CMerkleTx(CTransactionRef arg) - { - SetTx(std::move(arg)); - Init(); - } - - void Init() + template<typename Stream> + void Unserialize(Stream& s) { - hashBlock = uint256(); - nIndex = -1; - } + CTransactionRef tx; + uint256 hashBlock; + std::vector<uint256> vMerkleBranch; + int nIndex; - void SetTx(CTransactionRef arg) - { - tx = std::move(arg); + s >> tx >> hashBlock >> vMerkleBranch >> nIndex; } - - ADD_SERIALIZE_METHODS; - - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) { - std::vector<uint256> vMerkleBranch; // For compatibility with older versions. - READWRITE(tx); - READWRITE(hashBlock); - READWRITE(vMerkleBranch); - READWRITE(nIndex); - } - - void SetMerkleBranch(const uint256& block_hash, int posInBlock); - - /** - * Return depth of transaction in blockchain: - * <0 : conflicts with a transaction this deep in the blockchain - * 0 : in memory pool, waiting to be included in a block - * >=1 : this many blocks deep in the main chain - */ - int GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const; - bool IsInMainChain(interfaces::Chain::Lock& locked_chain) const { return GetDepthInMainChain(locked_chain) > 0; } - - /** - * @return number of blocks to maturity for this transaction: - * 0 : is not a coinbase transaction, or is a mature coinbase transaction - * >0 : is a coinbase transaction which matures in this many blocks - */ - int GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const; - bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } - bool isAbandoned() const { return (hashBlock == ABANDON_HASH); } - void setAbandoned() { hashBlock = ABANDON_HASH; } - - const uint256& GetHash() const { return tx->GetHash(); } - bool IsCoinBase() const { return tx->IsCoinBase(); } - bool IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const; }; //Get the marginal bytes of spending the specified output @@ -450,11 +391,14 @@ int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, * A transaction with a bunch of additional info that only the owner cares about. * It includes any unrecorded transactions needed to link it back to the block chain. */ -class CWalletTx : public CMerkleTx +class CWalletTx { private: const CWallet* pwallet; + /** Constant used in hashBlock to indicate tx has been abandoned */ + static const uint256 ABANDON_HASH; + public: /** * Key/value map with information about the transaction. @@ -512,7 +456,10 @@ public: mutable bool fInMempool; mutable CAmount nChangeCached; - CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) : CMerkleTx(std::move(arg)) + CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) + : tx(std::move(arg)), + hashBlock(uint256()), + nIndex(-1) { Init(pwalletIn); } @@ -532,10 +479,18 @@ public: nOrderPos = -1; } + CTransactionRef tx; + uint256 hashBlock; + /* An nIndex == -1 means that hashBlock (in nonzero) refers to the earliest + * block in the chain we know this or any in-wallet dependency conflicts + * with. Older clients interpret nIndex == -1 as unconfirmed for backward + * compatibility. + */ + int nIndex; + template<typename Stream> void Serialize(Stream& s) const { - char fSpent = false; mapValue_t mapValueCopy = mapValue; mapValueCopy["fromaccount"] = ""; @@ -544,20 +499,21 @@ public: mapValueCopy["timesmart"] = strprintf("%u", nTimeSmart); } - s << static_cast<const CMerkleTx&>(*this); - std::vector<CMerkleTx> vUnused; //!< Used to be vtxPrev - s << vUnused << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << fSpent; + std::vector<char> dummy_vector1; //!< Used to be vMerkleBranch + std::vector<char> dummy_vector2; //!< Used to be vtxPrev + char dummy_char = false; //!< Used to be fSpent + s << tx << hashBlock << dummy_vector1 << nIndex << dummy_vector2 << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << dummy_char; } template<typename Stream> void Unserialize(Stream& s) { Init(nullptr); - char fSpent; - s >> static_cast<CMerkleTx&>(*this); - std::vector<CMerkleTx> vUnused; //!< Used to be vtxPrev - s >> vUnused >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> fSpent; + std::vector<uint256> dummy_vector1; //!< Used to be vMerkleBranch + std::vector<CMerkleTx> dummy_vector2; //!< Used to be vtxPrev + char dummy_char; //! Used to be fSpent + s >> tx >> hashBlock >> dummy_vector1 >> nIndex >> dummy_vector2 >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> dummy_char; ReadOrderPos(nOrderPos, mapValue); nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(mapValue["timesmart"]) : 0; @@ -568,6 +524,11 @@ public: mapValue.erase("timesmart"); } + void SetTx(CTransactionRef arg) + { + tx = std::move(arg); + } + //! make sure balances are recalculated void MarkDirty() { @@ -618,11 +579,8 @@ public: int64_t GetTxTime() const; - // Pass this transaction to the node to relay to its peers - bool RelayWalletTransaction(interfaces::Chain::Lock& locked_chain); - - /** Pass this transaction to the mempool. Fails if absolute fee exceeds absurd fee. */ - bool AcceptToMemoryPool(interfaces::Chain::Lock& locked_chain, CValidationState& state); + // Pass this transaction to node for mempool insertion and relay to peers if flag set to true + bool SubmitMemoryPoolAndRelay(std::string& err_string, bool relay); // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation @@ -631,6 +589,31 @@ public: // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)" // in place. std::set<uint256> GetConflicts() const NO_THREAD_SAFETY_ANALYSIS; + + void SetMerkleBranch(const uint256& block_hash, int posInBlock); + + /** + * Return depth of transaction in blockchain: + * <0 : conflicts with a transaction this deep in the blockchain + * 0 : in memory pool, waiting to be included in a block + * >=1 : this many blocks deep in the main chain + */ + int GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const; + bool IsInMainChain(interfaces::Chain::Lock& locked_chain) const { return GetDepthInMainChain(locked_chain) > 0; } + + /** + * @return number of blocks to maturity for this transaction: + * 0 : is not a coinbase transaction, or is a mature coinbase transaction + * >0 : is a coinbase transaction which matures in this many blocks + */ + int GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const; + bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } + bool isAbandoned() const { return (hashBlock == ABANDON_HASH); } + void setAbandoned() { hashBlock = ABANDON_HASH; } + + const uint256& GetHash() const { return tx->GetHash(); } + bool IsCoinBase() const { return tx->IsCoinBase(); } + bool IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const; }; class COutput @@ -677,33 +660,6 @@ public: } }; -/** Private key that includes an expiration date in case it never gets used. */ -class CWalletKey -{ -public: - CPrivKey vchPrivKey; - int64_t nTimeCreated; - int64_t nTimeExpires; - std::string strComment; - // todo: add something to note what created it (user, getnewaddress, change) - // maybe should have a map<string, string> property map - - explicit CWalletKey(int64_t nExpires=0); - - ADD_SERIALIZE_METHODS; - - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) { - int nVersion = s.GetVersion(); - if (!(s.GetType() & SER_GETHASH)) - READWRITE(nVersion); - READWRITE(vchPrivKey); - READWRITE(nTimeCreated); - READWRITE(nTimeExpires); - READWRITE(LIMITED_STRING(strComment, 65536)); - } -}; - struct CoinSelectionParams { bool use_bnb = true; @@ -970,7 +926,7 @@ public: /** * populate vCoins with vector of available COutputs. */ - void AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<COutput>& vCoins, bool fOnlySafe=true, const CCoinControl *coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0, const int nMinDepth = 0, const int nMaxDepth = 9999999) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<COutput>& vCoins, bool fOnlySafe = true, const CCoinControl* coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Return list of available coins and locked coins grouped by non-change output address. diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index ece97e2a75..635997afc9 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -21,45 +21,71 @@ #include <boost/thread.hpp> +namespace DBKeys { +const std::string ACENTRY{"acentry"}; +const std::string BESTBLOCK_NOMERKLE{"bestblock_nomerkle"}; +const std::string BESTBLOCK{"bestblock"}; +const std::string CRYPTED_KEY{"ckey"}; +const std::string CSCRIPT{"cscript"}; +const std::string DEFAULTKEY{"defaultkey"}; +const std::string DESTDATA{"destdata"}; +const std::string FLAGS{"flags"}; +const std::string HDCHAIN{"hdchain"}; +const std::string KEYMETA{"keymeta"}; +const std::string KEY{"key"}; +const std::string MASTER_KEY{"mkey"}; +const std::string MINVERSION{"minversion"}; +const std::string NAME{"name"}; +const std::string OLD_KEY{"wkey"}; +const std::string ORDERPOSNEXT{"orderposnext"}; +const std::string POOL{"pool"}; +const std::string PURPOSE{"purpose"}; +const std::string SETTINGS{"settings"}; +const std::string TX{"tx"}; +const std::string VERSION{"version"}; +const std::string WATCHMETA{"watchmeta"}; +const std::string WATCHS{"watchs"}; +} // namespace DBKeys + // // WalletBatch // bool WalletBatch::WriteName(const std::string& strAddress, const std::string& strName) { - return WriteIC(std::make_pair(std::string("name"), strAddress), strName); + return WriteIC(std::make_pair(DBKeys::NAME, strAddress), strName); } bool WalletBatch::EraseName(const std::string& strAddress) { // This should only be used for sending addresses, never for receiving addresses, // receiving addresses must always have an address book entry if they're not change return. - return EraseIC(std::make_pair(std::string("name"), strAddress)); + return EraseIC(std::make_pair(DBKeys::NAME, strAddress)); } bool WalletBatch::WritePurpose(const std::string& strAddress, const std::string& strPurpose) { - return WriteIC(std::make_pair(std::string("purpose"), strAddress), strPurpose); + return WriteIC(std::make_pair(DBKeys::PURPOSE, strAddress), strPurpose); } bool WalletBatch::ErasePurpose(const std::string& strAddress) { - return EraseIC(std::make_pair(std::string("purpose"), strAddress)); + return EraseIC(std::make_pair(DBKeys::PURPOSE, strAddress)); } bool WalletBatch::WriteTx(const CWalletTx& wtx) { - return WriteIC(std::make_pair(std::string("tx"), wtx.GetHash()), wtx); + return WriteIC(std::make_pair(DBKeys::TX, wtx.GetHash()), wtx); } bool WalletBatch::EraseTx(uint256 hash) { - return EraseIC(std::make_pair(std::string("tx"), hash)); + return EraseIC(std::make_pair(DBKeys::TX, hash)); } bool WalletBatch::WriteKeyMetadata(const CKeyMetadata& meta, const CPubKey& pubkey, const bool overwrite) { - return WriteIC(std::make_pair(std::string("keymeta"), pubkey), meta, overwrite); + return WriteIC(std::make_pair(DBKeys::KEYMETA, pubkey), meta, overwrite); } bool WalletBatch::WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, const CKeyMetadata& keyMeta) @@ -74,7 +100,7 @@ bool WalletBatch::WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, vchKey.insert(vchKey.end(), vchPubKey.begin(), vchPubKey.end()); vchKey.insert(vchKey.end(), vchPrivKey.begin(), vchPrivKey.end()); - return WriteIC(std::make_pair(std::string("key"), vchPubKey), std::make_pair(vchPrivKey, Hash(vchKey.begin(), vchKey.end())), false); + return WriteIC(std::make_pair(DBKeys::KEY, vchPubKey), std::make_pair(vchPrivKey, Hash(vchKey.begin(), vchKey.end())), false); } bool WalletBatch::WriteCryptedKey(const CPubKey& vchPubKey, @@ -85,75 +111,74 @@ bool WalletBatch::WriteCryptedKey(const CPubKey& vchPubKey, return false; } - if (!WriteIC(std::make_pair(std::string("ckey"), vchPubKey), vchCryptedSecret, false)) { + if (!WriteIC(std::make_pair(DBKeys::CRYPTED_KEY, vchPubKey), vchCryptedSecret, false)) { return false; } - EraseIC(std::make_pair(std::string("key"), vchPubKey)); - EraseIC(std::make_pair(std::string("wkey"), vchPubKey)); + EraseIC(std::make_pair(DBKeys::KEY, vchPubKey)); return true; } bool WalletBatch::WriteMasterKey(unsigned int nID, const CMasterKey& kMasterKey) { - return WriteIC(std::make_pair(std::string("mkey"), nID), kMasterKey, true); + return WriteIC(std::make_pair(DBKeys::MASTER_KEY, nID), kMasterKey, true); } bool WalletBatch::WriteCScript(const uint160& hash, const CScript& redeemScript) { - return WriteIC(std::make_pair(std::string("cscript"), hash), redeemScript, false); + return WriteIC(std::make_pair(DBKeys::CSCRIPT, hash), redeemScript, false); } bool WalletBatch::WriteWatchOnly(const CScript &dest, const CKeyMetadata& keyMeta) { - if (!WriteIC(std::make_pair(std::string("watchmeta"), dest), keyMeta)) { + if (!WriteIC(std::make_pair(DBKeys::WATCHMETA, dest), keyMeta)) { return false; } - return WriteIC(std::make_pair(std::string("watchs"), dest), '1'); + return WriteIC(std::make_pair(DBKeys::WATCHS, dest), '1'); } bool WalletBatch::EraseWatchOnly(const CScript &dest) { - if (!EraseIC(std::make_pair(std::string("watchmeta"), dest))) { + if (!EraseIC(std::make_pair(DBKeys::WATCHMETA, dest))) { return false; } - return EraseIC(std::make_pair(std::string("watchs"), dest)); + return EraseIC(std::make_pair(DBKeys::WATCHS, dest)); } bool WalletBatch::WriteBestBlock(const CBlockLocator& locator) { - WriteIC(std::string("bestblock"), CBlockLocator()); // Write empty block locator so versions that require a merkle branch automatically rescan - return WriteIC(std::string("bestblock_nomerkle"), locator); + WriteIC(DBKeys::BESTBLOCK, CBlockLocator()); // Write empty block locator so versions that require a merkle branch automatically rescan + return WriteIC(DBKeys::BESTBLOCK_NOMERKLE, locator); } bool WalletBatch::ReadBestBlock(CBlockLocator& locator) { - if (m_batch.Read(std::string("bestblock"), locator) && !locator.vHave.empty()) return true; - return m_batch.Read(std::string("bestblock_nomerkle"), locator); + if (m_batch.Read(DBKeys::BESTBLOCK, locator) && !locator.vHave.empty()) return true; + return m_batch.Read(DBKeys::BESTBLOCK_NOMERKLE, locator); } bool WalletBatch::WriteOrderPosNext(int64_t nOrderPosNext) { - return WriteIC(std::string("orderposnext"), nOrderPosNext); + return WriteIC(DBKeys::ORDERPOSNEXT, nOrderPosNext); } bool WalletBatch::ReadPool(int64_t nPool, CKeyPool& keypool) { - return m_batch.Read(std::make_pair(std::string("pool"), nPool), keypool); + return m_batch.Read(std::make_pair(DBKeys::POOL, nPool), keypool); } bool WalletBatch::WritePool(int64_t nPool, const CKeyPool& keypool) { - return WriteIC(std::make_pair(std::string("pool"), nPool), keypool); + return WriteIC(std::make_pair(DBKeys::POOL, nPool), keypool); } bool WalletBatch::ErasePool(int64_t nPool) { - return EraseIC(std::make_pair(std::string("pool"), nPool)); + return EraseIC(std::make_pair(DBKeys::POOL, nPool)); } bool WalletBatch::WriteMinVersion(int nVersion) { - return WriteIC(std::string("minversion"), nVersion); + return WriteIC(DBKeys::MINVERSION, nVersion); } class CWalletScanState { @@ -165,7 +190,6 @@ public: unsigned int m_unknown_records{0}; bool fIsEncrypted{false}; bool fAnyUnordered{false}; - int nFileVersion{0}; std::vector<uint256> vWalletUpgrade; CWalletScanState() { @@ -181,20 +205,15 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, // Taking advantage of the fact that pair serialization // is just the two items serialized one after the other ssKey >> strType; - if (strType == "name") - { + if (strType == DBKeys::NAME) { std::string strAddress; ssKey >> strAddress; ssValue >> pwallet->mapAddressBook[DecodeDestination(strAddress)].name; - } - else if (strType == "purpose") - { + } else if (strType == DBKeys::PURPOSE) { std::string strAddress; ssKey >> strAddress; ssValue >> pwallet->mapAddressBook[DecodeDestination(strAddress)].purpose; - } - else if (strType == "tx") - { + } else if (strType == DBKeys::TX) { uint256 hash; ssKey >> hash; CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); @@ -228,9 +247,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, wss.fAnyUnordered = true; pwallet->LoadToWallet(wtx); - } - else if (strType == "watchs") - { + } else if (strType == DBKeys::WATCHS) { wss.nWatchKeys++; CScript script; ssKey >> script; @@ -238,9 +255,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, ssValue >> fYes; if (fYes == '1') pwallet->LoadWatchOnly(script); - } - else if (strType == "key" || strType == "wkey") - { + } else if (strType == DBKeys::KEY) { CPubKey vchPubKey; ssKey >> vchPubKey; if (!vchPubKey.IsValid()) @@ -252,20 +267,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, CPrivKey pkey; uint256 hash; - if (strType == "key") - { - wss.nKeys++; - ssValue >> pkey; - } else { - CWalletKey wkey; - ssValue >> wkey; - pkey = wkey.vchPrivKey; - } + wss.nKeys++; + ssValue >> pkey; - // Old wallets store keys as "key" [pubkey] => [privkey] + // Old wallets store keys as DBKeys::KEY [pubkey] => [privkey] // ... which was slow for wallets with lots of keys, because the public key is re-derived from the private key // using EC operations as a checksum. - // Newer wallets store keys as "key"[pubkey] => [privkey][hash(pubkey,privkey)], which is much faster while + // Newer wallets store keys as DBKeys::KEY [pubkey] => [privkey][hash(pubkey,privkey)], which is much faster while // remaining backwards-compatible. try { @@ -302,9 +310,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, strErr = "Error reading wallet database: LoadKey failed"; return false; } - } - else if (strType == "mkey") - { + } else if (strType == DBKeys::MASTER_KEY) { unsigned int nID; ssKey >> nID; CMasterKey kMasterKey; @@ -317,9 +323,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, pwallet->mapMasterKeys[nID] = kMasterKey; if (pwallet->nMasterKeyMaxID < nID) pwallet->nMasterKeyMaxID = nID; - } - else if (strType == "ckey") - { + } else if (strType == DBKeys::CRYPTED_KEY) { CPubKey vchPubKey; ssKey >> vchPubKey; if (!vchPubKey.IsValid()) @@ -337,27 +341,21 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, return false; } wss.fIsEncrypted = true; - } - else if (strType == "keymeta") - { + } else if (strType == DBKeys::KEYMETA) { CPubKey vchPubKey; ssKey >> vchPubKey; CKeyMetadata keyMeta; ssValue >> keyMeta; wss.nKeyMeta++; pwallet->LoadKeyMetadata(vchPubKey.GetID(), keyMeta); - } - else if (strType == "watchmeta") - { + } else if (strType == DBKeys::WATCHMETA) { CScript script; ssKey >> script; CKeyMetadata keyMeta; ssValue >> keyMeta; wss.nKeyMeta++; pwallet->LoadScriptMetadata(CScriptID(script), keyMeta); - } - else if (strType == "defaultkey") - { + } else if (strType == DBKeys::DEFAULTKEY) { // We don't want or need the default key, but if there is one set, // we want to make sure that it is valid so that we can detect corruption CPubKey vchPubKey; @@ -366,24 +364,14 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, strErr = "Error reading wallet database: Default Key corrupt"; return false; } - } - else if (strType == "pool") - { + } else if (strType == DBKeys::POOL) { int64_t nIndex; ssKey >> nIndex; CKeyPool keypool; ssValue >> keypool; pwallet->LoadKeyPool(nIndex, keypool); - } - else if (strType == "version") - { - ssValue >> wss.nFileVersion; - if (wss.nFileVersion == 10300) - wss.nFileVersion = 300; - } - else if (strType == "cscript") - { + } else if (strType == DBKeys::CSCRIPT) { uint160 hash; ssKey >> hash; CScript script; @@ -393,33 +381,31 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, strErr = "Error reading wallet database: LoadCScript failed"; return false; } - } - else if (strType == "orderposnext") - { + } else if (strType == DBKeys::ORDERPOSNEXT) { ssValue >> pwallet->nOrderPosNext; - } - else if (strType == "destdata") - { + } else if (strType == DBKeys::DESTDATA) { std::string strAddress, strKey, strValue; ssKey >> strAddress; ssKey >> strKey; ssValue >> strValue; pwallet->LoadDestData(DecodeDestination(strAddress), strKey, strValue); - } - else if (strType == "hdchain") - { + } else if (strType == DBKeys::HDCHAIN) { CHDChain chain; ssValue >> chain; pwallet->SetHDChain(chain, true); - } else if (strType == "flags") { + } else if (strType == DBKeys::FLAGS) { uint64_t flags; ssValue >> flags; if (!pwallet->SetWalletFlags(flags, true)) { strErr = "Error reading wallet database: Unknown non-tolerable wallet flags found"; return false; } - } else if (strType != "bestblock" && strType != "bestblock_nomerkle" && - strType != "minversion" && strType != "acentry") { + } else if (strType == DBKeys::OLD_KEY) { + strErr = "Found unsupported 'wkey' record, try loading with version 0.18"; + return false; + } else if (strType != DBKeys::BESTBLOCK && strType != DBKeys::BESTBLOCK_NOMERKLE && + strType != DBKeys::MINVERSION && strType != DBKeys::ACENTRY && + strType != DBKeys::VERSION && strType != DBKeys::SETTINGS) { wss.m_unknown_records++; } } catch (const std::exception& e) { @@ -438,8 +424,8 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, bool WalletBatch::IsKeyType(const std::string& strType) { - return (strType== "key" || strType == "wkey" || - strType == "mkey" || strType == "ckey"); + return (strType == DBKeys::KEY || + strType == DBKeys::MASTER_KEY || strType == DBKeys::CRYPTED_KEY); } DBErrors WalletBatch::LoadWallet(CWallet* pwallet) @@ -451,8 +437,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) LOCK(pwallet->cs_wallet); try { int nMinVersion = 0; - if (m_batch.Read((std::string)"minversion", nMinVersion)) - { + if (m_batch.Read(DBKeys::MINVERSION, nMinVersion)) { if (nMinVersion > FEATURE_LATEST) return DBErrors::TOO_NEW; pwallet->LoadMinVersion(nMinVersion); @@ -486,15 +471,15 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { // losing keys is considered a catastrophic error, anything else // we assume the user can live with: - if (IsKeyType(strType) || strType == "defaultkey") { + if (IsKeyType(strType) || strType == DBKeys::DEFAULTKEY) { result = DBErrors::CORRUPT; - } else if(strType == "flags") { + } else if (strType == DBKeys::FLAGS) { // reading the wallet flags can only fail if unknown flags are present result = DBErrors::TOO_NEW; } else { // Leave other errors alone, if we try to fix them we might make things worse. fNoncriticalErrors = true; // ... but do warn the user there is something wrong. - if (strType == "tx") + if (strType == DBKeys::TX) // Rescan if there is a bad transaction record: gArgs.SoftSetBoolArg("-rescan", true); } @@ -519,7 +504,12 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) if (result != DBErrors::LOAD_OK) return result; - pwallet->WalletLogPrintf("nFileVersion = %d\n", wss.nFileVersion); + // Last client version to open this wallet, was previously the file version number + int last_client = CLIENT_VERSION; + m_batch.Read(DBKeys::VERSION, last_client); + + int wallet_version = pwallet->GetVersion(); + pwallet->WalletLogPrintf("Wallet File Version = %d\n", wallet_version > 0 ? wallet_version : last_client); pwallet->WalletLogPrintf("Keys: %u plaintext, %u encrypted, %u w/ metadata, %u total. Unknown wallet records: %u\n", wss.nKeys, wss.nCKeys, wss.nKeyMeta, wss.nKeys + wss.nCKeys, wss.m_unknown_records); @@ -532,11 +522,11 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) WriteTx(pwallet->mapWallet.at(hash)); // Rewrite encrypted wallets of versions 0.4.0 and 0.5.0rc: - if (wss.fIsEncrypted && (wss.nFileVersion == 40000 || wss.nFileVersion == 50000)) + if (wss.fIsEncrypted && (last_client == 40000 || last_client == 50000)) return DBErrors::NEED_REWRITE; - if (wss.nFileVersion < CLIENT_VERSION) // Update - WriteVersion(CLIENT_VERSION); + if (last_client < CLIENT_VERSION) // Update + m_batch.Write(DBKeys::VERSION, CLIENT_VERSION); if (wss.fAnyUnordered) result = pwallet->ReorderTransactions(); @@ -558,8 +548,7 @@ DBErrors WalletBatch::FindWalletTx(std::vector<uint256>& vTxHash, std::vector<CW try { int nMinVersion = 0; - if (m_batch.Read((std::string)"minversion", nMinVersion)) - { + if (m_batch.Read(DBKeys::MINVERSION, nMinVersion)) { if (nMinVersion > FEATURE_LATEST) return DBErrors::TOO_NEW; } @@ -588,7 +577,7 @@ DBErrors WalletBatch::FindWalletTx(std::vector<uint256>& vTxHash, std::vector<CW std::string strType; ssKey >> strType; - if (strType == "tx") { + if (strType == DBKeys::TX) { uint256 hash; ssKey >> hash; @@ -723,8 +712,9 @@ bool WalletBatch::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, C fReadOK = ReadKeyValue(dummyWallet, ssKey, ssValue, dummyWss, strType, strErr); } - if (!IsKeyType(strType) && strType != "hdchain") + if (!IsKeyType(strType) && strType != DBKeys::HDCHAIN) { return false; + } if (!fReadOK) { LogPrintf("WARNING: WalletBatch::Recover skipping %s: %s\n", strType, strErr); @@ -746,23 +736,23 @@ bool WalletBatch::VerifyDatabaseFile(const fs::path& wallet_path, std::string& w bool WalletBatch::WriteDestData(const std::string &address, const std::string &key, const std::string &value) { - return WriteIC(std::make_pair(std::string("destdata"), std::make_pair(address, key)), value); + return WriteIC(std::make_pair(DBKeys::DESTDATA, std::make_pair(address, key)), value); } bool WalletBatch::EraseDestData(const std::string &address, const std::string &key) { - return EraseIC(std::make_pair(std::string("destdata"), std::make_pair(address, key))); + return EraseIC(std::make_pair(DBKeys::DESTDATA, std::make_pair(address, key))); } bool WalletBatch::WriteHDChain(const CHDChain& chain) { - return WriteIC(std::string("hdchain"), chain); + return WriteIC(DBKeys::HDCHAIN, chain); } bool WalletBatch::WriteWalletFlags(const uint64_t flags) { - return WriteIC(std::string("flags"), flags); + return WriteIC(DBKeys::FLAGS, flags); } bool WalletBatch::TxnBegin() @@ -779,13 +769,3 @@ bool WalletBatch::TxnAbort() { return m_batch.TxnAbort(); } - -bool WalletBatch::ReadVersion(int& nVersion) -{ - return m_batch.ReadVersion(nVersion); -} - -bool WalletBatch::WriteVersion(int nVersion) -{ - return m_batch.WriteVersion(nVersion); -} diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index d4a3bba97a..0fee35934d 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -55,6 +55,32 @@ enum class DBErrors NEED_REWRITE }; +namespace DBKeys { +extern const std::string ACENTRY; +extern const std::string BESTBLOCK; +extern const std::string BESTBLOCK_NOMERKLE; +extern const std::string CRYPTED_KEY; +extern const std::string CSCRIPT; +extern const std::string DEFAULTKEY; +extern const std::string DESTDATA; +extern const std::string FLAGS; +extern const std::string HDCHAIN; +extern const std::string KEY; +extern const std::string KEYMETA; +extern const std::string MASTER_KEY; +extern const std::string MINVERSION; +extern const std::string NAME; +extern const std::string OLD_KEY; +extern const std::string ORDERPOSNEXT; +extern const std::string POOL; +extern const std::string PURPOSE; +extern const std::string SETTINGS; +extern const std::string TX; +extern const std::string VERSION; +extern const std::string WATCHMETA; +extern const std::string WATCHS; +} // namespace DBKeys + /* simple HD chain data model */ class CHDChain { @@ -249,10 +275,6 @@ public: bool TxnCommit(); //! Abort current transaction bool TxnAbort(); - //! Read wallet version - bool ReadVersion(int& nVersion); - //! Write wallet version - bool WriteVersion(int nVersion); private: BerkeleyBatch m_batch; WalletDatabase& m_database; diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index b5eac88ba7..fdb608d457 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -486,6 +486,14 @@ class FullBlockTest(BitcoinTestFramework): tx_last = tx_new b39_outputs += 1 + # The accounting in the loop above can be off, because it misses the + # compact size encoding of the number of transactions in the block. + # Make sure we didn't accidentally make too big a block. Note that the + # size of the block has non-determinism due to the ECDSA signature in + # the first transaction. + while (len(b39.serialize()) >= MAX_BLOCK_BASE_SIZE): + del b39.vtx[-1] + b39 = self.update_block(39, []) self.send_blocks([b39], True) self.save_spendable_output() diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py index 62062926a6..ff014de0e0 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -30,17 +30,27 @@ import http.client import random import time -from test_framework.messages import COIN, COutPoint, CTransaction, CTxIn, CTxOut, ToHex +from test_framework.messages import ( + COIN, + COutPoint, + CTransaction, + CTxIn, + CTxOut, + ToHex, +) from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, create_confirmed_utxos, hex_str_to_bytes +from test_framework.util import ( + assert_equal, + create_confirmed_utxos, + hex_str_to_bytes, +) class ChainstateWriteCrashTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = False - # Need a bit of extra time for the nodes to start up for this test - self.rpc_timeout = 90 + self.rpc_timeout = 180 # Set -maxmempool=0 to turn off mempool memory sharing with dbcache # Set -rpcservertimeout=900 to reduce socket disconnects in this @@ -54,7 +64,8 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): self.node2_args = ["-dbcrashratio=24", "-dbcache=16"] + self.base_args # Node3 is a normal node with default args, except will mine full blocks - self.node3_args = ["-blockmaxweight=4000000"] + # and non-standard txs (e.g. txs with "dust" outputs) + self.node3_args = ["-blockmaxweight=4000000", "-acceptnonstdtxn"] self.extra_args = [self.node0_args, self.node1_args, self.node2_args, self.node3_args] def skip_test_if_missing_module(self): @@ -267,7 +278,7 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): # Warn if any of the nodes escaped restart. for i in range(3): if self.restart_counts[i] == 0: - self.log.warn("Node %d never crashed during utxo flush!", i) + self.log.warning("Node %d never crashed during utxo flush!", i) if __name__ == "__main__": ChainstateWriteCrashTest().main() diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index a4b9f213a1..071f7d5cca 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -120,9 +120,16 @@ def check_estimates(node, fees_seen): else: assert_greater_than_or_equal(i + 1, e["blocks"]) + class EstimateFeeTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 3 + # mine non-standard txs (e.g. txs with "dust" outputs) + self.extra_args = [ + ["-acceptnonstdtxn", "-maxorphantx=1000", "-whitelist=127.0.0.1"], + ["-acceptnonstdtxn", "-blockmaxweight=68000", "-maxorphantx=1000"], + ["-acceptnonstdtxn", "-blockmaxweight=32000", "-maxorphantx=1000"], + ] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -133,9 +140,7 @@ class EstimateFeeTest(BitcoinTestFramework): But first we need to use one node to create a lot of outputs which we will use to generate our transactions. """ - self.add_nodes(3, extra_args=[["-maxorphantx=1000", "-whitelist=127.0.0.1"], - ["-blockmaxweight=68000", "-maxorphantx=1000"], - ["-blockmaxweight=32000", "-maxorphantx=1000"]]) + self.add_nodes(3, extra_args=self.extra_args) # Use node0 to mine blocks for input splitting # Node1 mines small blocks but that are bigger than the expected transaction rate. # NOTE: the CreateNewBlock code starts counting block weight at 4,000 weight, diff --git a/test/functional/mempool_package_onemore.py b/test/functional/mempool_package_onemore.py index f955c1a77f..30f851fb8e 100755 --- a/test/functional/mempool_package_onemore.py +++ b/test/functional/mempool_package_onemore.py @@ -66,14 +66,14 @@ class MempoolPackagesTest(BitcoinTestFramework): assert_equal(len(self.nodes[0].getrawmempool(True)), MAX_ANCESTORS + 1) # Adding one more transaction on to the chain should fail. - assert_raises_rpc_error(-26, "too-long-mempool-chain", self.chain_transaction, self.nodes[0], [txid], [0], value, fee, 1) + assert_raises_rpc_error(-26, "too-long-mempool-chain, too many unconfirmed ancestors [limit: 25]", self.chain_transaction, self.nodes[0], [txid], [0], value, fee, 1) # ...even if it chains on from some point in the middle of the chain. - assert_raises_rpc_error(-26, "too-long-mempool-chain", self.chain_transaction, self.nodes[0], [chain[2][0]], [1], chain[2][1], fee, 1) - assert_raises_rpc_error(-26, "too-long-mempool-chain", self.chain_transaction, self.nodes[0], [chain[1][0]], [1], chain[1][1], fee, 1) + assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[2][0]], [1], chain[2][1], fee, 1) + assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[1][0]], [1], chain[1][1], fee, 1) # ...even if it chains on to two parent transactions with one in the chain. - assert_raises_rpc_error(-26, "too-long-mempool-chain", self.chain_transaction, self.nodes[0], [chain[0][0], second_chain], [1, 0], chain[0][1] + second_chain_value, fee, 1) + assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[0][0], second_chain], [1, 0], chain[0][1] + second_chain_value, fee, 1) # ...especially if its > 40k weight - assert_raises_rpc_error(-26, "too-long-mempool-chain", self.chain_transaction, self.nodes[0], [chain[0][0]], [1], chain[0][1], fee, 350) + assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_transaction, self.nodes[0], [chain[0][0]], [1], chain[0][1], fee, 350) # But not if it chains directly off the first transaction self.chain_transaction(self.nodes[0], [chain[0][0]], [1], chain[0][1], fee, 1) # and the second chain should work just fine diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index cdf636e200..b621081752 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -41,11 +41,11 @@ class RawTransactionsTest(BitcoinTestFramework): connect_nodes_bi(self.nodes, 0, 3) def run_test(self): - min_relay_tx_fee = self.nodes[0].getnetworkinfo()['relayfee'] + self.min_relay_tx_fee = self.nodes[0].getnetworkinfo()['relayfee'] # This test is not meant to test fee estimation and we'd like # to be sure all txs are sent at a consistent desired feerate for node in self.nodes: - node.settxfee(min_relay_tx_fee) + node.settxfee(self.min_relay_tx_fee) # if the fee's positive delta is higher than this value tests will fail, # neg. delta always fail the tests. @@ -53,13 +53,43 @@ class RawTransactionsTest(BitcoinTestFramework): # than a minimum sized signature. # = 2 bytes * minRelayTxFeePerByte - feeTolerance = 2 * min_relay_tx_fee/1000 + self.fee_tolerance = 2 * self.min_relay_tx_fee / 1000 self.nodes[2].generate(1) self.sync_all() self.nodes[0].generate(121) self.sync_all() + self.test_change_position() + self.test_simple() + self.test_simple_two_coins() + self.test_simple_two_outputs() + self.test_change() + self.test_no_change() + self.test_invalid_option() + self.test_invalid_change_address() + self.test_valid_change_address() + self.test_change_type() + self.test_coin_selection() + self.test_two_vin() + self.test_two_vin_two_vout() + self.test_invalid_input() + self.test_fee_p2pkh() + self.test_fee_p2pkh_multi_out() + self.test_fee_p2sh() + self.test_fee_4of5() + self.test_spend_2of2() + self.test_locked_wallet() + self.test_many_inputs_fee() + self.test_many_inputs_send() + self.test_op_return() + self.test_watchonly() + self.test_all_watched_funds() + self.test_option_feerate() + self.test_address_reuse() + self.test_option_subtract_fee_from_outputs() + + def test_change_position(self): # ensure that setting changePosition in fundraw with an exact match is handled properly rawmatch = self.nodes[2].createrawtransaction([], {self.nodes[2].getnewaddress():50}) rawmatch = self.nodes[2].fundrawtransaction(rawmatch, {"changePosition":1, "subtractFeeFromOutputs":[0]}) @@ -67,15 +97,15 @@ class RawTransactionsTest(BitcoinTestFramework): watchonly_address = self.nodes[0].getnewaddress() watchonly_pubkey = self.nodes[0].getaddressinfo(watchonly_address)["pubkey"] - watchonly_amount = Decimal(200) + self.watchonly_amount = Decimal(200) self.nodes[3].importpubkey(watchonly_pubkey, "", True) - watchonly_txid = self.nodes[0].sendtoaddress(watchonly_address, watchonly_amount) + self.watchonly_txid = self.nodes[0].sendtoaddress(watchonly_address, self.watchonly_amount) # Lock UTXO so nodes[0] doesn't accidentally spend it - watchonly_vout = find_vout_for_address(self.nodes[0], watchonly_txid, watchonly_address) - self.nodes[0].lockunspent(False, [{"txid": watchonly_txid, "vout": watchonly_vout}]) + self.watchonly_vout = find_vout_for_address(self.nodes[0], self.watchonly_txid, watchonly_address) + self.nodes[0].lockunspent(False, [{"txid": self.watchonly_txid, "vout": self.watchonly_vout}]) - self.nodes[0].sendtoaddress(self.nodes[3].getnewaddress(), watchonly_amount / 10) + self.nodes[0].sendtoaddress(self.nodes[3].getnewaddress(), self.watchonly_amount / 10) self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.5) self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) @@ -84,6 +114,7 @@ class RawTransactionsTest(BitcoinTestFramework): self.nodes[0].generate(1) self.sync_all() + def test_simple(self): ############### # simple test # ############### @@ -92,10 +123,10 @@ class RawTransactionsTest(BitcoinTestFramework): rawtx = self.nodes[2].createrawtransaction(inputs, outputs) dec_tx = self.nodes[2].decoderawtransaction(rawtx) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) assert len(dec_tx['vin']) > 0 #test that we have enough inputs + def test_simple_two_coins(self): ############################## # simple test with two coins # ############################## @@ -105,25 +136,11 @@ class RawTransactionsTest(BitcoinTestFramework): dec_tx = self.nodes[2].decoderawtransaction(rawtx) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) assert len(dec_tx['vin']) > 0 #test if we have enough inputs - - ############################## - # simple test with two coins # - ############################## - inputs = [ ] - outputs = { self.nodes[0].getnewaddress() : 2.6 } - rawtx = self.nodes[2].createrawtransaction(inputs, outputs) - dec_tx = self.nodes[2].decoderawtransaction(rawtx) - - rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] - dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) - assert len(dec_tx['vin']) > 0 assert_equal(dec_tx['vin'][0]['scriptSig']['hex'], '') - + def test_simple_two_outputs(self): ################################ # simple test with two outputs # ################################ @@ -133,7 +150,6 @@ class RawTransactionsTest(BitcoinTestFramework): dec_tx = self.nodes[2].decoderawtransaction(rawtx) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 for out in dec_tx['vout']: @@ -142,7 +158,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert len(dec_tx['vin']) > 0 assert_equal(dec_tx['vin'][0]['scriptSig']['hex'], '') - + def test_change(self): ######################################################################### # test a fundrawtransaction with a VIN greater than the required amount # ######################################################################### @@ -156,6 +172,7 @@ class RawTransactionsTest(BitcoinTestFramework): rawtxfund = self.nodes[2].fundrawtransaction(rawtx) fee = rawtxfund['fee'] + self.test_no_change_fee = fee # Use the same fee for the next tx dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 for out in dec_tx['vout']: @@ -163,14 +180,14 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(fee + totalOut, utx['amount']) #compare vin total and totalout+fee - + def test_no_change(self): ##################################################################### # test a fundrawtransaction with which will not get a change output # ##################################################################### utx = get_unspent(self.nodes[2].listunspent(), 5) inputs = [ {'txid' : utx['txid'], 'vout' : utx['vout']}] - outputs = { self.nodes[0].getnewaddress() : Decimal(5.0) - fee - feeTolerance } + outputs = {self.nodes[0].getnewaddress(): Decimal(5.0) - self.test_no_change_fee - self.fee_tolerance} rawtx = self.nodes[2].createrawtransaction(inputs, outputs) dec_tx = self.nodes[2].decoderawtransaction(rawtx) assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) @@ -185,7 +202,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(rawtxfund['changepos'], -1) assert_equal(fee + totalOut, utx['amount']) #compare vin total and totalout+fee - + def test_invalid_option(self): #################################################### # test a fundrawtransaction with an invalid option # #################################################### @@ -202,6 +219,7 @@ class RawTransactionsTest(BitcoinTestFramework): # reserveChangeKey was deprecated and is now removed assert_raises_rpc_error(-3, "Unexpected key reserveChangeKey", lambda: self.nodes[2].fundrawtransaction(hexstring=rawtx, options={'reserveChangeKey': True})) + def test_invalid_change_address(self): ############################################################ # test a fundrawtransaction with an invalid change address # ############################################################ @@ -215,6 +233,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert_raises_rpc_error(-5, "changeAddress must be a valid bitcoin address", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':'foobar'}) + def test_valid_change_address(self): ############################################################ # test a fundrawtransaction with a provided change address # ############################################################ @@ -233,6 +252,7 @@ class RawTransactionsTest(BitcoinTestFramework): out = dec_tx['vout'][0] assert_equal(change, out['scriptPubKey']['addresses'][0]) + def test_change_type(self): ######################################################### # test a fundrawtransaction with a provided change type # ######################################################### @@ -247,6 +267,7 @@ class RawTransactionsTest(BitcoinTestFramework): dec_tx = self.nodes[2].decoderawtransaction(rawtx['hex']) assert_equal('witness_v0_keyhash', dec_tx['vout'][rawtx['changepos']]['scriptPubKey']['type']) + def test_coin_selection(self): ######################################################################### # test a fundrawtransaction with a VIN smaller than the required amount # ######################################################################### @@ -264,7 +285,6 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal("00", dec_tx['vin'][0]['scriptSig']['hex']) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 matchingOuts = 0 @@ -281,7 +301,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(matchingOuts, 1) assert_equal(len(dec_tx['vout']), 2) - + def test_two_vin(self): ########################################### # test a fundrawtransaction with two VINs # ########################################### @@ -295,7 +315,6 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 matchingOuts = 0 @@ -315,6 +334,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(matchingIns, 2) #we now must see two vins identical to vins given as params + def test_two_vin_two_vout(self): ######################################################### # test a fundrawtransaction with two VINs and two vOUTs # ######################################################### @@ -328,7 +348,6 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 matchingOuts = 0 @@ -340,16 +359,16 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(matchingOuts, 2) assert_equal(len(dec_tx['vout']), 3) + def test_invalid_input(self): ############################################## # test a fundrawtransaction with invalid vin # ############################################## inputs = [ {'txid' : "1c7f966dab21119bac53213a2bc7532bff1fa844c124fd750a7d0b1332440bd1", 'vout' : 0} ] #invalid vin! outputs = { self.nodes[0].getnewaddress() : 1.0} rawtx = self.nodes[2].createrawtransaction(inputs, outputs) - dec_tx = self.nodes[2].decoderawtransaction(rawtx) - assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx) + def test_fee_p2pkh(self): ############################################################ #compare fee of a standard pubkeyhash transaction inputs = [] @@ -363,9 +382,10 @@ class RawTransactionsTest(BitcoinTestFramework): #compare fee feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) - assert feeDelta >= 0 and feeDelta <= feeTolerance + assert feeDelta >= 0 and feeDelta <= self.fee_tolerance ############################################################ + def test_fee_p2pkh_multi_out(self): ############################################################ #compare fee of a standard pubkeyhash transaction with multiple outputs inputs = [] @@ -378,10 +398,10 @@ class RawTransactionsTest(BitcoinTestFramework): #compare fee feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) - assert feeDelta >= 0 and feeDelta <= feeTolerance + assert feeDelta >= 0 and feeDelta <= self.fee_tolerance ############################################################ - + def test_fee_p2sh(self): ############################################################ #compare fee of a 2of2 multisig p2sh transaction @@ -405,10 +425,10 @@ class RawTransactionsTest(BitcoinTestFramework): #compare fee feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) - assert feeDelta >= 0 and feeDelta <= feeTolerance + assert feeDelta >= 0 and feeDelta <= self.fee_tolerance ############################################################ - + def test_fee_4of5(self): ############################################################ #compare fee of a standard pubkeyhash transaction @@ -438,10 +458,10 @@ class RawTransactionsTest(BitcoinTestFramework): #compare fee feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) - assert feeDelta >= 0 and feeDelta <= feeTolerance + assert feeDelta >= 0 and feeDelta <= self.fee_tolerance ############################################################ - + def test_spend_2of2(self): ############################################################ # spend a 2of2 multisig transaction over fundraw @@ -456,7 +476,7 @@ class RawTransactionsTest(BitcoinTestFramework): # send 1.2 BTC to msig addr - txId = self.nodes[0].sendtoaddress(mSigObj, 1.2) + self.nodes[0].sendtoaddress(mSigObj, 1.2) self.sync_all() self.nodes[1].generate(1) self.sync_all() @@ -468,7 +488,7 @@ class RawTransactionsTest(BitcoinTestFramework): fundedTx = self.nodes[2].fundrawtransaction(rawtx) signedTx = self.nodes[2].signrawtransactionwithwallet(fundedTx['hex']) - txId = self.nodes[2].sendrawtransaction(signedTx['hex']) + self.nodes[2].sendrawtransaction(signedTx['hex']) self.sync_all() self.nodes[1].generate(1) self.sync_all() @@ -476,6 +496,7 @@ class RawTransactionsTest(BitcoinTestFramework): # make sure funds are received at node1 assert_equal(oldBalance+Decimal('1.10000000'), self.nodes[1].getbalance()) + def test_locked_wallet(self): ############################################################ # locked wallet test self.nodes[1].encryptwallet("test") @@ -485,7 +506,7 @@ class RawTransactionsTest(BitcoinTestFramework): # This test is not meant to test fee estimation and we'd like # to be sure all txs are sent at a consistent desired feerate for node in self.nodes: - node.settxfee(min_relay_tx_fee) + node.settxfee(self.min_relay_tx_fee) connect_nodes_bi(self.nodes,0,1) connect_nodes_bi(self.nodes,1,2) @@ -493,7 +514,7 @@ class RawTransactionsTest(BitcoinTestFramework): connect_nodes_bi(self.nodes,0,3) # Again lock the watchonly UTXO or nodes[0] may spend it, because # lockunspent is memory-only and thus lost on restart - self.nodes[0].lockunspent(False, [{"txid": watchonly_txid, "vout": watchonly_vout}]) + self.nodes[0].lockunspent(False, [{"txid": self.watchonly_txid, "vout": self.watchonly_vout}]) self.sync_all() # drain the keypool @@ -523,14 +544,14 @@ class RawTransactionsTest(BitcoinTestFramework): #now we need to unlock self.nodes[1].walletpassphrase("test", 600) signedTx = self.nodes[1].signrawtransactionwithwallet(fundedTx['hex']) - txId = self.nodes[1].sendrawtransaction(signedTx['hex']) + self.nodes[1].sendrawtransaction(signedTx['hex']) self.nodes[1].generate(1) self.sync_all() # make sure funds are received at node1 assert_equal(oldBalance+Decimal('51.10000000'), self.nodes[0].getbalance()) - + def test_many_inputs_fee(self): ############################################### # multiple (~19) inputs tx test | Compare fee # ############################################### @@ -558,9 +579,9 @@ class RawTransactionsTest(BitcoinTestFramework): #compare fee feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) - assert feeDelta >= 0 and feeDelta <= feeTolerance*19 #~19 inputs - + assert feeDelta >= 0 and feeDelta <= self.fee_tolerance * 19 #~19 inputs + def test_many_inputs_send(self): ############################################# # multiple (~19) inputs tx test | sign/send # ############################################# @@ -584,12 +605,13 @@ class RawTransactionsTest(BitcoinTestFramework): rawtx = self.nodes[1].createrawtransaction(inputs, outputs) fundedTx = self.nodes[1].fundrawtransaction(rawtx) fundedAndSignedTx = self.nodes[1].signrawtransactionwithwallet(fundedTx['hex']) - txId = self.nodes[1].sendrawtransaction(fundedAndSignedTx['hex']) + self.nodes[1].sendrawtransaction(fundedAndSignedTx['hex']) self.sync_all() self.nodes[0].generate(1) self.sync_all() assert_equal(oldBalance+Decimal('50.19000000'), self.nodes[0].getbalance()) #0.19+block reward + def test_op_return(self): ##################################################### # test fundrawtransaction with OP_RETURN and no vin # ##################################################### @@ -606,40 +628,41 @@ class RawTransactionsTest(BitcoinTestFramework): assert_greater_than(len(dec_tx['vin']), 0) # at least one vin assert_equal(len(dec_tx['vout']), 2) # one change output added - + def test_watchonly(self): ################################################## # test a fundrawtransaction using only watchonly # ################################################## inputs = [] - outputs = {self.nodes[2].getnewaddress() : watchonly_amount / 2} + outputs = {self.nodes[2].getnewaddress(): self.watchonly_amount / 2} rawtx = self.nodes[3].createrawtransaction(inputs, outputs) result = self.nodes[3].fundrawtransaction(rawtx, {'includeWatching': True }) res_dec = self.nodes[0].decoderawtransaction(result["hex"]) assert_equal(len(res_dec["vin"]), 1) - assert_equal(res_dec["vin"][0]["txid"], watchonly_txid) + assert_equal(res_dec["vin"][0]["txid"], self.watchonly_txid) assert "fee" in result.keys() assert_greater_than(result["changepos"], -1) + def test_all_watched_funds(self): ############################################################### # test fundrawtransaction using the entirety of watched funds # ############################################################### inputs = [] - outputs = {self.nodes[2].getnewaddress() : watchonly_amount} + outputs = {self.nodes[2].getnewaddress(): self.watchonly_amount} rawtx = self.nodes[3].createrawtransaction(inputs, outputs) # Backward compatibility test (2nd param is includeWatching) result = self.nodes[3].fundrawtransaction(rawtx, True) res_dec = self.nodes[0].decoderawtransaction(result["hex"]) assert_equal(len(res_dec["vin"]), 2) - assert res_dec["vin"][0]["txid"] == watchonly_txid or res_dec["vin"][1]["txid"] == watchonly_txid + assert res_dec["vin"][0]["txid"] == self.watchonly_txid or res_dec["vin"][1]["txid"] == self.watchonly_txid assert_greater_than(result["fee"], 0) assert_greater_than(result["changepos"], -1) - assert_equal(result["fee"] + res_dec["vout"][result["changepos"]]["value"], watchonly_amount / 10) + assert_equal(result["fee"] + res_dec["vout"][result["changepos"]]["value"], self.watchonly_amount / 10) signedtx = self.nodes[3].signrawtransactionwithwallet(result["hex"]) assert not signedtx["complete"] @@ -649,6 +672,7 @@ class RawTransactionsTest(BitcoinTestFramework): self.nodes[0].generate(1) self.sync_all() + def test_option_feerate(self): ####################### # Test feeRate option # ####################### @@ -659,18 +683,20 @@ class RawTransactionsTest(BitcoinTestFramework): inputs = [] outputs = {self.nodes[3].getnewaddress() : 1} rawtx = self.nodes[3].createrawtransaction(inputs, outputs) - result = self.nodes[3].fundrawtransaction(rawtx) # uses min_relay_tx_fee (set by settxfee) - result2 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee}) - result3 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 10*min_relay_tx_fee}) + result = self.nodes[3].fundrawtransaction(rawtx) # uses self.min_relay_tx_fee (set by settxfee) + result2 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee}) + result3 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 10 * self.min_relay_tx_fee}) assert_raises_rpc_error(-4, "Fee exceeds maximum configured by -maxtxfee", self.nodes[3].fundrawtransaction, rawtx, {"feeRate": 1}) result_fee_rate = result['fee'] * 1000 / count_bytes(result['hex']) assert_fee_amount(result2['fee'], count_bytes(result2['hex']), 2 * result_fee_rate) assert_fee_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate) + def test_address_reuse(self): ################################ # Test no address reuse occurs # ################################ + rawtx = self.nodes[3].createrawtransaction(inputs=[], outputs={self.nodes[3].getnewaddress(): 1}) result3 = self.nodes[3].fundrawtransaction(rawtx) res_dec = self.nodes[0].decoderawtransaction(result3["hex"]) changeaddress = "" @@ -682,6 +708,7 @@ class RawTransactionsTest(BitcoinTestFramework): # Now the change address key should be removed from the keypool assert changeaddress != nextaddr + def test_option_subtract_fee_from_outputs(self): ###################################### # Test subtractFeeFromOutputs option # ###################################### @@ -693,11 +720,11 @@ class RawTransactionsTest(BitcoinTestFramework): outputs = {self.nodes[2].getnewaddress(): 1} rawtx = self.nodes[3].createrawtransaction(inputs, outputs) - result = [self.nodes[3].fundrawtransaction(rawtx), # uses min_relay_tx_fee (set by settxfee) - self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": []}), # empty subtraction list - self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": [0]}), # uses min_relay_tx_fee (set by settxfee) - self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee}), - self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee, "subtractFeeFromOutputs": [0]})] + result = [self.nodes[3].fundrawtransaction(rawtx), # uses self.min_relay_tx_fee (set by settxfee) + self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": []}), # empty subtraction list + self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": [0]}), # uses self.min_relay_tx_fee (set by settxfee) + self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee}), + self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee, "subtractFeeFromOutputs": [0]}),] dec_tx = [self.nodes[3].decoderawtransaction(tx_['hex']) for tx_ in result] output = [d['vout'][1 - r['changepos']]['value'] for d, r in zip(dec_tx, result)] diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index b3d8696208..5a04e0c8d8 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -27,6 +27,11 @@ class PSBTTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = False self.num_nodes = 3 + self.extra_args = [ + ["-walletrbf=1"], + ["-walletrbf=0"], + [] + ] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -207,18 +212,18 @@ class PSBTTest(BitcoinTestFramework): # replaceable arg block_height = self.nodes[0].getblockcount() unspent = self.nodes[0].listunspent()[0] - psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable":True}, False) + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable": False}, False) decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) for tx_in, psbt_in in zip(decoded_psbt["tx"]["vin"], decoded_psbt["inputs"]): - assert_equal(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) + assert_greater_than(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) assert "bip32_derivs" not in psbt_in assert_equal(decoded_psbt["tx"]["locktime"], block_height+2) - # Same construction with only locktime set - psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height, {}, True) + # Same construction with only locktime set and RBF explicitly enabled + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height, {"replaceable": True}, True) decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) for tx_in, psbt_in in zip(decoded_psbt["tx"]["vin"], decoded_psbt["inputs"]): - assert tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE + assert_equal(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) assert "bip32_derivs" in psbt_in assert_equal(decoded_psbt["tx"]["locktime"], block_height) @@ -226,9 +231,16 @@ class PSBTTest(BitcoinTestFramework): psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}]) decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) for tx_in in decoded_psbt["tx"]["vin"]: - assert tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE + assert_equal(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) assert_equal(decoded_psbt["tx"]["locktime"], 0) + # Same construction without optional arguments, for a node with -walletrbf=0 + unspent1 = self.nodes[1].listunspent()[0] + psbtx_info = self.nodes[1].walletcreatefundedpsbt([{"txid":unspent1["txid"], "vout":unspent1["vout"]}], [{self.nodes[2].getnewaddress():unspent1["amount"]+1}], block_height) + decoded_psbt = self.nodes[1].decodepsbt(psbtx_info["psbt"]) + for tx_in in decoded_psbt["tx"]["vin"]: + assert_greater_than(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) + # Make sure change address wallet does not have P2SH innerscript access to results in success # when attempting BnB coin selection self.nodes[0].walletcreatefundedpsbt([], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"changeAddress":self.nodes[1].getnewaddress()}, False) diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index cc3a4cc72a..779863df79 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -363,6 +363,7 @@ class P2PInterface(P2PConnection): def wait_for_tx(self, txid, timeout=60): def test_function(): + assert self.is_connected if not self.last_message.get('tx'): return False return self.last_message['tx'].tx.rehash() == txid @@ -370,11 +371,15 @@ class P2PInterface(P2PConnection): wait_until(test_function, timeout=timeout, lock=mininode_lock) def wait_for_block(self, blockhash, timeout=60): - test_function = lambda: self.last_message.get("block") and self.last_message["block"].block.rehash() == blockhash + def test_function(): + assert self.is_connected + return self.last_message.get("block") and self.last_message["block"].block.rehash() == blockhash + wait_until(test_function, timeout=timeout, lock=mininode_lock) def wait_for_header(self, blockhash, timeout=60): def test_function(): + assert self.is_connected last_headers = self.last_message.get('headers') if not last_headers: return False @@ -389,7 +394,11 @@ class P2PInterface(P2PConnection): value must be explicitly cleared before calling this method, or this will return immediately with success. TODO: change this method to take a hash value and only return true if the correct block/tx has been requested.""" - test_function = lambda: self.last_message.get("getdata") + + def test_function(): + assert self.is_connected + return self.last_message.get("getdata") + wait_until(test_function, timeout=timeout, lock=mininode_lock) def wait_for_getheaders(self, timeout=60): @@ -399,20 +408,30 @@ class P2PInterface(P2PConnection): value must be explicitly cleared before calling this method, or this will return immediately with success. TODO: change this method to take a hash value and only return true if the correct block header has been requested.""" - test_function = lambda: self.last_message.get("getheaders") + + def test_function(): + assert self.is_connected + return self.last_message.get("getheaders") + wait_until(test_function, timeout=timeout, lock=mininode_lock) def wait_for_inv(self, expected_inv, timeout=60): """Waits for an INV message and checks that the first inv object in the message was as expected.""" if len(expected_inv) > 1: raise NotImplementedError("wait_for_inv() will only verify the first inv object") - test_function = lambda: self.last_message.get("inv") and \ + + def test_function(): + assert self.is_connected + return self.last_message.get("inv") and \ self.last_message["inv"].inv[0].type == expected_inv[0].type and \ self.last_message["inv"].inv[0].hash == expected_inv[0].hash + wait_until(test_function, timeout=timeout, lock=mininode_lock) def wait_for_verack(self, timeout=60): - test_function = lambda: self.message_count["verack"] + def test_function(): + return self.message_count["verack"] + wait_until(test_function, timeout=timeout, lock=mininode_lock) # Message sending helper functions @@ -424,7 +443,11 @@ class P2PInterface(P2PConnection): # Sync up with the node def sync_with_ping(self, timeout=60): self.send_message(msg_ping(nonce=self.ping_counter)) - test_function = lambda: self.last_message.get("pong") and self.last_message["pong"].nonce == self.ping_counter + + def test_function(): + assert self.is_connected + return self.last_message.get("pong") and self.last_message["pong"].nonce == self.ping_counter + wait_until(test_function, timeout=timeout, lock=mininode_lock) self.ping_counter += 1 diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 0db8382733..7ad2b78d4e 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -175,6 +175,7 @@ BASE_SCRIPTS = [ 'rpc_bind.py --nonloopback', 'mining_basic.py', 'wallet_bumpfee.py', + 'wallet_bumpfee_totalfee_deprecation.py', 'rpc_named_arguments.py', 'wallet_listsinceblock.py', 'p2p_leak.py', diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 030eb50791..bfc01e3f5e 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -37,6 +37,7 @@ class BumpFeeTest(BitcoinTestFramework): self.extra_args = [[ "-walletrbf={}".format(i), "-mintxfee=0.00002", + "-deprecatedrpc=totalFee", ] for i in range(self.num_nodes)] def skip_test_if_missing_module(self): diff --git a/test/functional/wallet_bumpfee_totalfee_deprecation.py b/test/functional/wallet_bumpfee_totalfee_deprecation.py new file mode 100755 index 0000000000..b8e097c32e --- /dev/null +++ b/test/functional/wallet_bumpfee_totalfee_deprecation.py @@ -0,0 +1,54 @@ +#!/usr/bin/env python3 +# Copyright (c) 2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test deprecation of passing `totalFee` to the bumpfee RPC.""" +from decimal import Decimal + +from test_framework.messages import BIP125_SEQUENCE_NUMBER +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_raises_rpc_error + +class BumpFeeWithTotalFeeArgumentDeprecationTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.extra_args = [[ + "-walletrbf={}".format(i), + "-mintxfee=0.00002", + ] for i in range(self.num_nodes)] + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + peer_node, rbf_node = self.nodes + peer_node.generate(110) + self.sync_all() + peer_node.sendtoaddress(rbf_node.getnewaddress(), 0.001) + self.sync_all() + peer_node.generate(1) + self.sync_all() + rbfid = spend_one_input(rbf_node, peer_node.getnewaddress()) + + self.log.info("Testing bumpfee with totalFee argument raises RPC error with deprecation message") + assert_raises_rpc_error( + -8, + "totalFee argument has been deprecated and will be removed in 0.20. " + + "Please use -deprecatedrpc=totalFee to continue using this argument until removal.", + rbf_node.bumpfee, rbfid, {"totalFee": 2000}) + + self.log.info("Testing bumpfee without totalFee argument does not raise") + rbf_node.bumpfee(rbfid) + +def spend_one_input(node, dest_address, change_size=Decimal("0.00049000")): + tx_input = dict(sequence=BIP125_SEQUENCE_NUMBER, + **next(u for u in node.listunspent() if u["amount"] == Decimal("0.00100000"))) + destinations = {dest_address: Decimal("0.00050000")} + destinations[node.getrawchangeaddress()] = change_size + rawtx = node.createrawtransaction([tx_input], destinations) + signedtx = node.signrawtransactionwithwallet(rawtx) + txid = node.sendrawtransaction(signedtx["hex"]) + return txid + +if __name__ == "__main__": + BumpFeeWithTotalFeeArgumentDeprecationTest().main() diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py index c17949a2f6..e302e499f4 100755 --- a/test/functional/wallet_createwallet.py +++ b/test/functional/wallet_createwallet.py @@ -116,8 +116,20 @@ class CreateWalletTest(BitcoinTestFramework): walletinfo = w6.getwalletinfo() assert_equal(walletinfo['keypoolsize'], 1) assert_equal(walletinfo['keypoolsize_hd_internal'], 1) - # Empty passphrase, error - assert_raises_rpc_error(-16, 'Cannot encrypt a wallet with a blank password', self.nodes[0].createwallet, 'w7', False, False, '') + # Allow empty passphrase, but there should be a warning + resp = self.nodes[0].createwallet(wallet_name='w7', disable_private_keys=False, blank=False, passphrase='') + assert_equal(resp['warning'], 'Empty string given as passphrase, wallet will not be encrypted.') + w7 = node.get_wallet_rpc('w7') + assert_raises_rpc_error(-15, 'Error: running with an unencrypted wallet, but walletpassphrase was called.', w7.walletpassphrase, '', 10) + + self.log.info('Test making a wallet with avoid reuse flag') + self.nodes[0].createwallet('w8', False, False, '', True) # Use positional arguments to check for bug where avoid_reuse could not be set for wallets without needing them to be encrypted + w8 = node.get_wallet_rpc('w8') + assert_raises_rpc_error(-15, 'Error: running with an unencrypted wallet, but walletpassphrase was called.', w7.walletpassphrase, '', 10) + assert_equal(w8.getwalletinfo()["avoid_reuse"], True) + + self.log.info('Using a passphrase with private keys disabled returns error') + assert_raises_rpc_error(-4, 'Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.', self.nodes[0].createwallet, wallet_name='w9', disable_private_keys=True, passphrase='thisisapassphrase') if __name__ == '__main__': CreateWalletTest().main() |