diff options
132 files changed, 2817 insertions, 1082 deletions
diff --git a/Makefile.am b/Makefile.am index 43790f1c23..45dab3930d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -41,7 +41,7 @@ OSX_DEPLOY_SCRIPT=$(top_srcdir)/contrib/macdeploy/macdeployqtplus OSX_FANCY_PLIST=$(top_srcdir)/contrib/macdeploy/fancy.plist OSX_INSTALLER_ICONS=$(top_srcdir)/src/qt/res/icons/bitcoin.icns OSX_PLIST=$(top_builddir)/share/qt/Info.plist #not installed -OSX_QT_TRANSLATIONS = da,de,es,hu,ru,uk,zh_CN,zh_TW +OSX_QT_TRANSLATIONS = ar,bg,ca,cs,da,de,es,fa,fi,fr,gd,gl,he,hu,it,ja,ko,lt,lv,pl,pt,ru,sk,sl,sv,uk,zh_CN,zh_TW DIST_CONTRIB = \ $(top_srcdir)/contrib/linearize/linearize-data.py \ diff --git a/configure.ac b/configure.ac index f3b603e48a..3d8291bf58 100644 --- a/configure.ac +++ b/configure.ac @@ -381,7 +381,7 @@ if test "x$enable_werror" = "xyes"; then AX_CHECK_COMPILE_FLAG([-Werror=gnu],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=gnu"],,[[$CXXFLAG_WERROR]]) 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]]) + AX_CHECK_COMPILE_FLAG([-Werror=thread-safety],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=thread-safety"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=unused-variable],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=unused-variable"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=date-time],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=date-time"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=return-type],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=return-type"],,[[$CXXFLAG_WERROR]]) @@ -401,7 +401,7 @@ if test "x$CXXFLAGS_overridden" = "xno"; then 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([-Wthread-safety],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wthread-safety"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wrange-loop-analysis],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wrange-loop-analysis"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wredundant-decls],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wredundant-decls"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wunused-variable],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunused-variable"],,[[$CXXFLAG_WERROR]]) @@ -422,6 +422,9 @@ if test "x$CXXFLAGS_overridden" = "xno"; then AX_CHECK_COMPILE_FLAG([-Wdeprecated-copy],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-deprecated-copy"],,[[$CXXFLAG_WERROR]]) fi +dnl Don't allow extended (non-ASCII) symbols in identifiers. This is easier for code review. +AX_CHECK_COMPILE_FLAG([-fno-extended-identifiers],[[CXXFLAGS="$CXXFLAGS -fno-extended-identifiers"]],,[[$CXXFLAG_WERROR]]) + enable_sse42=no enable_sse41=no enable_avx2=no diff --git a/depends/Makefile b/depends/Makefile index 5ad82bb56a..3d0784cb6b 100644 --- a/depends/Makefile +++ b/depends/Makefile @@ -4,6 +4,30 @@ print-%: @echo $* = $($*) +# When invoking a sub-make, keep only the command line variable definitions +# matching the pattern in the filter function. +# +# e.g. invoking: +# $ make A=1 C=1 print-MAKEOVERRIDES print-MAKEFLAGS +# +# with the following in the Makefile: +# MAKEOVERRIDES := $(filter A=% B=%,$(MAKEOVERRIDES)) +# +# will print: +# MAKEOVERRIDES = A=1 +# MAKEFLAGS = -- A=1 +# +# this is because as the GNU make manual says: +# The command line variable definitions really appear in the variable +# MAKEOVERRIDES, and MAKEFLAGS contains a reference to this variable. +# +# and since the GNU make manual also says: +# variables defined on the command line are passed to the sub-make through +# MAKEFLAGS +# +# this means that sub-makes will be invoked as if: +# $(MAKE) A=1 blah blah +MAKEOVERRIDES := $(filter V=%,$(MAKEOVERRIDES)) SOURCES_PATH ?= $(BASEDIR)/sources WORK_PATH = $(BASEDIR)/work BASE_CACHE ?= $(BASEDIR)/built diff --git a/depends/hosts/default.mk b/depends/hosts/default.mk index 144e5f88b7..258619a9d0 100644 --- a/depends/hosts/default.mk +++ b/depends/hosts/default.mk @@ -13,9 +13,18 @@ default_host_OTOOL = $(host_toolchain)otool default_host_NM = $(host_toolchain)nm define add_host_tool_func +ifneq ($(filter $(origin $1),undefined default),) +# Do not consider the well-known var $1 if it is undefined or is taking a value +# that is predefined by "make" (e.g. the make variable "CC" has a predefined +# value of "cc") $(host_os)_$1?=$$(default_host_$1) $(host_arch)_$(host_os)_$1?=$$($(host_os)_$1) $(host_arch)_$(host_os)_$(release_type)_$1?=$$($(host_os)_$1) +else +$(host_os)_$1=$(or $($1),$($(host_os)_$1),$(default_host_$1)) +$(host_arch)_$(host_os)_$1=$(or $($1),$($(host_arch)_$(host_os)_$1),$$($(host_os)_$1)) +$(host_arch)_$(host_os)_$(release_type)_$1=$(or $($1),$($(host_arch)_$(host_os)_$(release_type)_$1),$$($(host_os)_$1)) +endif host_$1=$$($(host_arch)_$(host_os)_$1) endef diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index 970c81041e..3a7e605b4f 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -9,9 +9,9 @@ $(package)_config_opts_release=variant=release $(package)_config_opts_debug=variant=debug $(package)_config_opts=--layout=tagged --build-type=complete --user-config=user-config.jam $(package)_config_opts+=threading=multi link=static -sNO_BZIP2=1 -sNO_ZLIB=1 -$(package)_config_opts_linux=threadapi=pthread runtime-link=shared -$(package)_config_opts_darwin=--toolset=clang-darwin runtime-link=shared -$(package)_config_opts_mingw32=binary-format=pe target-os=windows threadapi=win32 runtime-link=static +$(package)_config_opts_linux=target-os=linux threadapi=pthread runtime-link=shared +$(package)_config_opts_darwin=target-os=darwin runtime-link=shared +$(package)_config_opts_mingw32=target-os=windows binary-format=pe threadapi=win32 runtime-link=static $(package)_config_opts_x86_64_mingw32=address-model=64 $(package)_config_opts_i686_mingw32=address-model=32 $(package)_config_opts_i686_linux=address-model=32 architecture=x86 @@ -20,8 +20,11 @@ $(package)_config_opts_aarch64_android=address-model=64 $(package)_config_opts_x86_64_android=address-model=64 $(package)_config_opts_armv7a_android=address-model=32 $(package)_toolset_$(host_os)=gcc +$(package)_toolset_darwin=clang +ifneq (,$(findstring clang,$($(package)_cxx))) + $(package)_toolset_$(host_os)=clang +endif $(package)_archiver_$(host_os)=$($(package)_ar) -$(package)_toolset_darwin=clang-darwin $(package)_config_libraries=filesystem,system,thread,test $(package)_cxxflags=-std=c++11 -fvisibility=hidden $(package)_cxxflags_linux=-fPIC @@ -33,13 +36,13 @@ define $(package)_preprocess_cmds endef define $(package)_config_cmds - ./bootstrap.sh --without-icu --with-libraries=$($(package)_config_libraries) + ./bootstrap.sh --without-icu --with-libraries=$($(package)_config_libraries) --with-toolset=$($(package)_toolset_$(host_os)) endef define $(package)_build_cmds - ./b2 -d2 -j2 -d1 --prefix=$($(package)_staging_prefix_dir) $($(package)_config_opts) stage + ./b2 -d2 -j2 -d1 --prefix=$($(package)_staging_prefix_dir) $($(package)_config_opts) toolset=$($(package)_toolset_$(host_os)) stage endef define $(package)_stage_cmds - ./b2 -d0 -j4 --prefix=$($(package)_staging_prefix_dir) $($(package)_config_opts) install + ./b2 -d0 -j4 --prefix=$($(package)_staging_prefix_dir) $($(package)_config_opts) toolset=$($(package)_toolset_$(host_os)) install endef diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 366b1d0c42..631851855a 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -10,6 +10,7 @@ $(package)_build_subdir=qtbase $(package)_qt_libs=corelib network widgets gui plugins testlib $(package)_patches=fix_qt_pkgconfig.patch mac-qmake.conf fix_configure_mac.patch fix_no_printer.patch fix_rcc_determinism.patch fix_riscv64_arch.patch xkb-default.patch no-xlib.patch fix_android_qmake_conf.patch fix_android_jni_static.patch +# Update OSX_QT_TRANSLATIONS when this is updated $(package)_qttranslations_file_name=qttranslations-$($(package)_suffix) $(package)_qttranslations_sha256_hash=fb5a47799754af73d3bf501fe513342cfe2fc37f64e80df5533f6110e804220c diff --git a/doc/build-windows.md b/doc/build-windows.md index bbff638b90..d3dc467f19 100644 --- a/doc/build-windows.md +++ b/doc/build-windows.md @@ -9,7 +9,7 @@ The options known to work for building Bitcoin Core on Windows are: and is the platform used to build the Bitcoin Core Windows release binaries. * On Windows, using [Windows Subsystem for Linux (WSL)](https://docs.microsoft.com/windows/wsl/about) and the Mingw-w64 cross compiler tool chain. -* On Windows, using a native compiler tool chain such as [Visual Studio](https://www.visualstudio.com). +* On Windows, using a native compiler tool chain such as [Visual Studio](https://www.visualstudio.com). See [README.md](/build_msvc/README.md). Other options which may work, but which have not been extensively tested are (please contribute instructions): diff --git a/doc/release-notes-18918.md b/doc/release-notes-18918.md new file mode 100644 index 0000000000..f57a62eeb7 --- /dev/null +++ b/doc/release-notes-18918.md @@ -0,0 +1,3 @@ +# Wallet + +The `-salvagewallet` startup option has been removed. A new `salvage` command has been added to the `bitcoin-wallet` tool which performs the salvage operations that `-salvagewallet` did. diff --git a/doc/release-notes.md b/doc/release-notes.md index 0d668a6302..0a2d0fbfc3 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -65,9 +65,28 @@ Notable changes P2P and network changes ----------------------- +- The mempool now tracks whether transactions submitted via the wallet or RPCs + have been successfully broadcast. Every 10-15 minutes, the node will try to + announce unbroadcast transactions until a peer requests it via a `getdata` + message or the transaction is removed from the mempool for other reasons. + The node will not track the broadcast status of transactions submitted to the + node using P2P relay. This version reduces the initial broadcast guarantees + for wallet transactions submitted via P2P to a node running the wallet. (#18038) + Updated RPCs ------------ +- `getmempoolinfo` now returns an additional `unbroadcastcount` field. The + mempool tracks locally submitted transactions until their initial broadcast + is acknowledged by a peer. This field returns the count of transactions + waiting for acknowledgement. + +- Mempool RPCs such as `getmempoolentry` and `getrawmempool` with + `verbose=true` now return an additional `unbroadcast` field. This indicates + whether initial broadcast of the transaction has been acknowledged by a + peer. `getmempoolancestors` and `getmempooldescendants` are also updated. + + Changes to Wallet or GUI related RPCs can be found in the GUI or Wallet section below. New RPCs @@ -87,6 +106,13 @@ New settings Wallet ------ +- To improve wallet privacy, the frequency of wallet rebroadcast attempts is + reduced from approximately once every 15 minutes to once every 12-36 hours. + To maintain a similar level of guarantee for initial broadcast of wallet + transactions, the mempool tracks these transactions as a part of the newly + introduced unbroadcast set. See the "P2P and network changes" section for + more information on the unbroadcast set. (#18038) + #### Wallet RPC changes - The `upgradewallet` RPC replaces the `-upgradewallet` command line option. diff --git a/src/Makefile.am b/src/Makefile.am index 882d83e0b8..2b004691fd 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -248,6 +248,7 @@ BITCOIN_CORE_H = \ wallet/ismine.h \ wallet/load.h \ wallet/rpcwallet.h \ + wallet/salvage.h \ wallet/scriptpubkeyman.h \ wallet/wallet.h \ wallet/walletdb.h \ @@ -356,6 +357,7 @@ libbitcoin_wallet_a_SOURCES = \ wallet/load.cpp \ wallet/rpcdump.cpp \ wallet/rpcwallet.cpp \ + wallet/salvage.cpp \ wallet/scriptpubkeyman.cpp \ wallet/wallet.cpp \ wallet/walletdb.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 6ba0e9f270..7909cb4a0f 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -31,6 +31,7 @@ FUZZ_TARGETS = \ test/fuzz/chain \ test/fuzz/checkqueue \ test/fuzz/coins_deserialize \ + test/fuzz/coins_view \ test/fuzz/cuckoocache \ test/fuzz/decode_tx \ test/fuzz/descriptor_parse \ @@ -109,9 +110,14 @@ FUZZ_TARGETS = \ test/fuzz/rbf \ test/fuzz/rolling_bloom_filter \ test/fuzz/script \ + test/fuzz/script_bitcoin_consensus \ + test/fuzz/script_descriptor_cache \ test/fuzz/script_deserialize \ test/fuzz/script_flags \ + test/fuzz/script_interpreter \ test/fuzz/script_ops \ + test/fuzz/script_sigcache \ + test/fuzz/script_sign \ test/fuzz/scriptnum_ops \ test/fuzz/service_deserialize \ test/fuzz/signature_checker \ @@ -466,6 +472,12 @@ test_fuzz_coins_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_coins_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_coins_deserialize_SOURCES = test/fuzz/deserialize.cpp +test_fuzz_coins_view_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_coins_view_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_coins_view_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_coins_view_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_coins_view_SOURCES = test/fuzz/coins_view.cpp + test_fuzz_cuckoocache_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_cuckoocache_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_cuckoocache_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -934,6 +946,18 @@ test_fuzz_script_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_script_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_script_SOURCES = test/fuzz/script.cpp +test_fuzz_script_bitcoin_consensus_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_script_bitcoin_consensus_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_script_bitcoin_consensus_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_script_bitcoin_consensus_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_script_bitcoin_consensus_SOURCES = test/fuzz/script_bitcoin_consensus.cpp + +test_fuzz_script_descriptor_cache_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_script_descriptor_cache_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_script_descriptor_cache_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_script_descriptor_cache_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_script_descriptor_cache_SOURCES = test/fuzz/script_descriptor_cache.cpp + test_fuzz_script_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DSCRIPT_DESERIALIZE=1 test_fuzz_script_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_script_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -946,12 +970,30 @@ test_fuzz_script_flags_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_script_flags_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_script_flags_SOURCES = test/fuzz/script_flags.cpp +test_fuzz_script_interpreter_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_script_interpreter_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_script_interpreter_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_script_interpreter_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_script_interpreter_SOURCES = test/fuzz/script_interpreter.cpp + test_fuzz_script_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_script_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_script_ops_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_script_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_script_ops_SOURCES = test/fuzz/script_ops.cpp +test_fuzz_script_sigcache_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_script_sigcache_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_script_sigcache_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_script_sigcache_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_script_sigcache_SOURCES = test/fuzz/script_sigcache.cpp + +test_fuzz_script_sign_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_script_sign_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_script_sign_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_script_sign_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_script_sign_SOURCES = test/fuzz/script_sign.cpp + test_fuzz_scriptnum_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_scriptnum_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_scriptnum_ops_LDADD = $(FUZZ_SUITE_LD_COMMON) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index cdaabd6fab..8d85789b4e 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -9,6 +9,7 @@ #include <chainparamsbase.h> #include <clientversion.h> +#include <optional.h> #include <rpc/client.h> #include <rpc/protocol.h> #include <rpc/request.h> @@ -20,6 +21,7 @@ #include <functional> #include <memory> #include <stdio.h> +#include <string> #include <tuple> #include <event2/buffer.h> @@ -157,7 +159,7 @@ struct HTTPReply std::string body; }; -static const char *http_errorstring(int code) +static std::string http_errorstring(int code) { switch(code) { #if LIBEVENT_VERSION_NUMBER >= 0x02010300 @@ -250,7 +252,7 @@ public: UniValue ProcessReply(const UniValue &batch_in) override { UniValue result(UniValue::VOBJ); - std::vector<UniValue> batch = JSONRPCProcessBatchReply(batch_in, batch_in.size()); + const std::vector<UniValue> batch = JSONRPCProcessBatchReply(batch_in); // Errors in getnetworkinfo() and getblockchaininfo() are fatal, pass them on; // getwalletinfo() and getbalances() are allowed to fail if there is no wallet. if (!batch[ID_NETWORKINFO]["error"].isNull()) { @@ -304,7 +306,7 @@ public: } }; -static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, const std::vector<std::string>& args) +static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, const std::vector<std::string>& args, const Optional<std::string>& rpcwallet = {}) { std::string host; // In preference order, we choose the following for the port: @@ -369,14 +371,12 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co // check if we should use a special wallet endpoint std::string endpoint = "/"; - if (!gArgs.GetArgs("-rpcwallet").empty()) { - std::string walletName = gArgs.GetArg("-rpcwallet", ""); - char *encodedURI = evhttp_uriencode(walletName.data(), walletName.size(), false); + if (rpcwallet) { + char* encodedURI = evhttp_uriencode(rpcwallet->data(), rpcwallet->size(), false); if (encodedURI) { - endpoint = "/wallet/"+ std::string(encodedURI); + endpoint = "/wallet/" + std::string(encodedURI); free(encodedURI); - } - else { + } else { throw CConnectionFailed("uri-encode failed"); } } @@ -418,6 +418,65 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co return reply; } +/** + * ConnectAndCallRPC wraps CallRPC with -rpcwait and an exception handler. + * + * @param[in] rh Pointer to RequestHandler. + * @param[in] strMethod Reference to const string method to forward to CallRPC. + * @param[in] rpcwallet Reference to const optional string wallet name to forward to CallRPC. + * @returns the RPC response as a UniValue object. + * @throws a CConnectionFailed std::runtime_error if connection failed or RPC server still in warmup. + */ +static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& strMethod, const std::vector<std::string>& args, const Optional<std::string>& rpcwallet = {}) +{ + UniValue response(UniValue::VOBJ); + // Execute and handle connection failures with -rpcwait. + const bool fWait = gArgs.GetBoolArg("-rpcwait", false); + do { + try { + response = CallRPC(rh, strMethod, args, rpcwallet); + if (fWait) { + const UniValue& error = find_value(response, "error"); + if (!error.isNull() && error["code"].get_int() == RPC_IN_WARMUP) { + throw CConnectionFailed("server in warmup"); + } + } + break; // Connection succeeded, no need to retry. + } catch (const CConnectionFailed&) { + if (fWait) { + UninterruptibleSleep(std::chrono::milliseconds{1000}); + } else { + throw; + } + } + } while (fWait); + return response; +} + +/** + * GetWalletBalances calls listwallets; if more than one wallet is loaded, it then + * fetches mine.trusted balances for each loaded wallet and pushes them to `result`. + * + * @param result Reference to UniValue object the wallet names and balances are pushed to. + */ +static void GetWalletBalances(UniValue& result) +{ + std::unique_ptr<BaseRequestHandler> rh{MakeUnique<DefaultRequestHandler>()}; + const UniValue listwallets = ConnectAndCallRPC(rh.get(), "listwallets", /* args=*/{}); + if (!find_value(listwallets, "error").isNull()) return; + const UniValue& wallets = find_value(listwallets, "result"); + if (wallets.size() <= 1) return; + + UniValue balances(UniValue::VOBJ); + for (const UniValue& wallet : wallets.getValues()) { + const std::string wallet_name = wallet.get_str(); + const UniValue getbalances = ConnectAndCallRPC(rh.get(), "getbalances", /* args=*/{}, wallet_name); + const UniValue& balance = find_value(getbalances, "result")["mine"]["trusted"]; + balances.pushKV(wallet_name, balance); + } + result.pushKV("balances", balances); +} + static int CommandLineRPC(int argc, char *argv[]) { std::string strPrint; @@ -474,9 +533,8 @@ static int CommandLineRPC(int argc, char *argv[]) } std::unique_ptr<BaseRequestHandler> rh; std::string method; - if (gArgs.GetBoolArg("-getinfo", false)) { + if (gArgs.IsArgSet("-getinfo")) { rh.reset(new GetinfoRequestHandler()); - method = ""; } else { rh.reset(new DefaultRequestHandler()); if (args.size() < 1) { @@ -485,62 +543,46 @@ static int CommandLineRPC(int argc, char *argv[]) method = args[0]; args.erase(args.begin()); // Remove trailing method name from arguments vector } - - // Execute and handle connection failures with -rpcwait - const bool fWait = gArgs.GetBoolArg("-rpcwait", false); - do { - try { - const UniValue reply = CallRPC(rh.get(), method, args); - - // Parse reply - const UniValue& result = find_value(reply, "result"); - const UniValue& error = find_value(reply, "error"); - - if (!error.isNull()) { - // Error - int code = error["code"].get_int(); - if (fWait && code == RPC_IN_WARMUP) - throw CConnectionFailed("server in warmup"); - strPrint = "error: " + error.write(); - nRet = abs(code); - if (error.isObject()) - { - UniValue errCode = find_value(error, "code"); - UniValue errMsg = find_value(error, "message"); - strPrint = errCode.isNull() ? "" : "error code: "+errCode.getValStr()+"\n"; - - if (errMsg.isStr()) - strPrint += "error message:\n"+errMsg.get_str(); - - if (errCode.isNum() && errCode.get_int() == RPC_WALLET_NOT_SPECIFIED) { - strPrint += "\nTry adding \"-rpcwallet=<filename>\" option to bitcoin-cli command line."; - } - } - } else { - // Result - if (result.isNull()) - strPrint = ""; - else if (result.isStr()) - strPrint = result.get_str(); - else - strPrint = result.write(2); + Optional<std::string> wallet_name{}; + if (gArgs.IsArgSet("-rpcwallet")) wallet_name = gArgs.GetArg("-rpcwallet", ""); + const UniValue reply = ConnectAndCallRPC(rh.get(), method, args, wallet_name); + + // Parse reply + UniValue result = find_value(reply, "result"); + const UniValue& error = find_value(reply, "error"); + if (!error.isNull()) { + // Error + strPrint = "error: " + error.write(); + nRet = abs(error["code"].get_int()); + if (error.isObject()) { + const UniValue& errCode = find_value(error, "code"); + const UniValue& errMsg = find_value(error, "message"); + strPrint = errCode.isNull() ? "" : ("error code: " + errCode.getValStr() + "\n"); + + if (errMsg.isStr()) { + strPrint += ("error message:\n" + errMsg.get_str()); + } + if (errCode.isNum() && errCode.get_int() == RPC_WALLET_NOT_SPECIFIED) { + strPrint += "\nTry adding \"-rpcwallet=<filename>\" option to bitcoin-cli command line."; } - // Connection succeeded, no need to retry. - break; } - catch (const CConnectionFailed&) { - if (fWait) - UninterruptibleSleep(std::chrono::milliseconds{1000}); - else - throw; + } else { + if (gArgs.IsArgSet("-getinfo") && !gArgs.IsArgSet("-rpcwallet")) { + GetWalletBalances(result); // fetch multiwallet balances and append to result } - } while (fWait); - } - catch (const std::exception& e) { + // Result + if (result.isNull()) { + strPrint = ""; + } else if (result.isStr()) { + strPrint = result.get_str(); + } else { + strPrint = result.write(2); + } + } + } catch (const std::exception& e) { strPrint = std::string("error: ") + e.what(); nRet = EXIT_FAILURE; - } - catch (...) { + } catch (...) { PrintExceptionContinue(nullptr, "CommandLineRPC()"); throw; } diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index 7f9439788a..b420463c00 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -31,6 +31,7 @@ static void SetupWalletToolArgs() gArgs.AddArg("info", "Get wallet info", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); gArgs.AddArg("create", "Create new wallet file", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); + gArgs.AddArg("salvage", "Attempt to recover private keys from a corrupt wallet", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); } static bool WalletAppInit(int argc, char* argv[]) diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 263d863cfa..a47709cd82 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -105,13 +105,12 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c std::vector<bool> have_txn(txn_available.size()); { LOCK(pool->cs); - const std::vector<std::pair<uint256, CTxMemPool::txiter> >& vTxHashes = pool->vTxHashes; - for (size_t i = 0; i < vTxHashes.size(); i++) { - uint64_t shortid = cmpctblock.GetShortID(vTxHashes[i].first); + for (size_t i = 0; i < pool->vTxHashes.size(); i++) { + uint64_t shortid = cmpctblock.GetShortID(pool->vTxHashes[i].first); std::unordered_map<uint64_t, uint16_t>::iterator idit = shorttxids.find(shortid); if (idit != shorttxids.end()) { if (!have_txn[idit->second]) { - txn_available[idit->second] = vTxHashes[i].second->GetSharedTx(); + txn_available[idit->second] = pool->vTxHashes[i].second->GetSharedTx(); have_txn[idit->second] = true; mempool_count++; } else { diff --git a/src/blockencodings.h b/src/blockencodings.h index 9ec1beeaf7..326db1b4a7 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -126,7 +126,7 @@ class PartiallyDownloadedBlock { protected: std::vector<CTransactionRef> txn_available; size_t prefilled_count = 0, mempool_count = 0, extra_count = 0; - CTxMemPool* pool; + const CTxMemPool* pool; public: CBlockHeader header; explicit PartiallyDownloadedBlock(CTxMemPool* poolIn) : pool(poolIn) {} diff --git a/src/blockfilter.h b/src/blockfilter.h index ff8744b217..96cefbf3b2 100644 --- a/src/blockfilter.h +++ b/src/blockfilter.h @@ -144,8 +144,8 @@ public: template <typename Stream> void Serialize(Stream& s) const { - s << m_block_hash - << static_cast<uint8_t>(m_filter_type) + s << static_cast<uint8_t>(m_filter_type) + << m_block_hash << m_filter.GetEncoded(); } @@ -154,8 +154,8 @@ public: std::vector<unsigned char> encoded_filter; uint8_t filter_type; - s >> m_block_hash - >> filter_type + s >> filter_type + >> m_block_hash >> encoded_filter; m_filter_type = static_cast<BlockFilterType>(filter_type); diff --git a/src/core_read.cpp b/src/core_read.cpp index df78c319ee..1c0a8a096d 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -19,6 +19,7 @@ #include <boost/algorithm/string/split.hpp> #include <algorithm> +#include <string> CScript ParseScript(const std::string& s) { @@ -34,10 +35,9 @@ CScript ParseScript(const std::string& s) if (op < OP_NOP && op != OP_RESERVED) continue; - const char* name = GetOpName(static_cast<opcodetype>(op)); - if (strcmp(name, "OP_UNKNOWN") == 0) + std::string strName = GetOpName(static_cast<opcodetype>(op)); + if (strName == "OP_UNKNOWN") continue; - std::string strName(name); mapOpNames[strName] = static_cast<opcodetype>(op); // Convenience: OP_ADD and just ADD are both recognized: boost::algorithm::replace_first(strName, "OP_", ""); diff --git a/src/fs.cpp b/src/fs.cpp index 066c6c10d3..e68c97b3ca 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -6,6 +6,9 @@ #ifndef WIN32 #include <fcntl.h> +#include <string> +#include <sys/file.h> +#include <sys/utsname.h> #else #ifndef NOMINMAX #define NOMINMAX @@ -47,20 +50,38 @@ FileLock::~FileLock() } } +static bool IsWSL() +{ + struct utsname uname_data; + return uname(&uname_data) == 0 && std::string(uname_data.version).find("Microsoft") != std::string::npos; +} + bool FileLock::TryLock() { if (fd == -1) { return false; } - struct flock lock; - lock.l_type = F_WRLCK; - lock.l_whence = SEEK_SET; - lock.l_start = 0; - lock.l_len = 0; - if (fcntl(fd, F_SETLK, &lock) == -1) { - reason = GetErrorReason(); - return false; + + // Exclusive file locking is broken on WSL using fcntl (issue #18622) + // This workaround can be removed once the bug on WSL is fixed + static const bool is_wsl = IsWSL(); + if (is_wsl) { + if (flock(fd, LOCK_EX | LOCK_NB) == -1) { + reason = GetErrorReason(); + return false; + } + } else { + struct flock lock; + lock.l_type = F_WRLCK; + lock.l_whence = SEEK_SET; + lock.l_start = 0; + lock.l_len = 0; + if (fcntl(fd, F_SETLK, &lock) == -1) { + reason = GetErrorReason(); + return false; + } } + return true; } #else diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index 7ca43540c7..317f8c0e40 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -39,6 +39,7 @@ private: size_t WriteFilterToDisk(FlatFilePos& pos, const BlockFilter& filter); Mutex m_cs_headers_cache; + /** cache of block hash to filter header, to avoid disk access when responding to getcfcheckpt. */ std::unordered_map<uint256, uint256, FilterHeaderHasher> m_headers_cache GUARDED_BY(m_cs_headers_cache); protected: diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 3c94e44b53..bd1b36fea5 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -88,7 +88,15 @@ public: Interrupt(m_context); Shutdown(m_context); } - void startShutdown() override { StartShutdown(); } + void startShutdown() override + { + StartShutdown(); + // Stop RPC for clean shutdown if any of waitfor* commands is executed. + if (gArgs.GetBoolArg("-server", false)) { + InterruptRPC(); + StopRPC(); + } + } bool shutdownRequested() override { return ShutdownRequested(); } void mapPort(bool use_upnp) override { @@ -187,6 +195,11 @@ public: LOCK(::cs_main); return ::ChainActive().Height(); } + uint256 getBestBlockHash() override + { + const CBlockIndex* tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip()); + return tip ? tip->GetBlockHash() : Params().GenesisBlock().GetHash(); + } int64_t getLastBlockTime() override { LOCK(::cs_main); @@ -310,7 +323,7 @@ public: std::unique_ptr<Handler> handleNotifyBlockTip(NotifyBlockTipFn fn) override { return MakeHandler(::uiInterface.NotifyBlockTip_connect([fn](SynchronizationState sync_state, const CBlockIndex* block) { - fn(sync_state, block->nHeight, block->GetBlockTime(), + fn(sync_state, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()}, GuessVerificationProgress(Params().TxData(), block)); })); } @@ -318,7 +331,7 @@ public: { return MakeHandler( ::uiInterface.NotifyHeaderTip_connect([fn](SynchronizationState sync_state, const CBlockIndex* block) { - fn(sync_state, block->nHeight, block->GetBlockTime(), + fn(sync_state, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()}, /* verification progress is unused when a header was received */ 0); })); } diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 45b0e18fae..0b7fb6736a 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -36,6 +36,7 @@ struct bilingual_str; namespace interfaces { class Handler; class Wallet; +struct BlockTip; //! Top-level interface for a bitcoin node (bitcoind process). class Node @@ -149,6 +150,9 @@ public: //! Get num blocks. virtual int getNumBlocks() = 0; + //! Get best block hash. + virtual uint256 getBestBlockHash() = 0; + //! Get last block time. virtual int64_t getLastBlockTime() = 0; @@ -250,12 +254,12 @@ public: //! Register handler for block tip messages. using NotifyBlockTipFn = - std::function<void(SynchronizationState, int height, int64_t block_time, double verification_progress)>; + std::function<void(SynchronizationState, interfaces::BlockTip tip, double verification_progress)>; virtual std::unique_ptr<Handler> handleNotifyBlockTip(NotifyBlockTipFn fn) = 0; //! Register handler for header tip messages. using NotifyHeaderTipFn = - std::function<void(SynchronizationState, int height, int64_t block_time, double verification_progress)>; + std::function<void(SynchronizationState, interfaces::BlockTip tip, double verification_progress)>; virtual std::unique_ptr<Handler> handleNotifyHeaderTip(NotifyHeaderTipFn fn) = 0; //! Return pointer to internal chain interface, useful for testing. @@ -265,6 +269,13 @@ public: //! Return implementation of Node interface. std::unique_ptr<Node> MakeNode(); +//! Block tip (could be a header or not, depends on the subscribed signal). +struct BlockTip { + int block_height; + int64_t block_time; + uint256 block_hash; +}; + } // namespace interfaces #endif // BITCOIN_INTERFACES_NODE_H diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 349dce0247..cec75030ad 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -351,13 +351,13 @@ public: } return result; } - bool tryGetBalances(WalletBalances& balances, int& num_blocks) override + bool tryGetBalances(WalletBalances& balances, uint256& block_hash) override { TRY_LOCK(m_wallet->cs_wallet, locked_wallet); if (!locked_wallet) { return false; } - num_blocks = m_wallet->GetLastBlockHeight(); + block_hash = m_wallet->GetLastBlockHash(); balances = getBalances(); return true; } diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 421d35af15..67569a3e55 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -203,7 +203,7 @@ public: virtual WalletBalances getBalances() = 0; //! Get balances if possible without blocking. - virtual bool tryGetBalances(WalletBalances& balances, int& num_blocks) = 0; + virtual bool tryGetBalances(WalletBalances& balances, uint256& block_hash) = 0; //! Get balance. virtual CAmount getBalance() = 0; diff --git a/src/logging.cpp b/src/logging.cpp index eb9da06d9b..fe58ae9e73 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -22,8 +22,8 @@ BCLog::Logger& LogInstance() * access the logger. When the shutdown sequence is fully audited and tested, * explicit destruction of these objects can be implemented by changing this * from a raw pointer to a std::unique_ptr. - * Since the destructor is never called, the logger and all its members must - * have a trivial destructor. + * Since the ~Logger() destructor is never called, the Logger class and all + * its subclasses must have implicitly-defined destructors. * * This method of initialization was originally introduced in * ee3374234c60aba2cc4c5cd5cac1c0aefc2d817c. @@ -41,7 +41,7 @@ static int FileWriteStr(const std::string &str, FILE *fp) bool BCLog::Logger::StartLogging() { - std::lock_guard<std::mutex> scoped_lock(m_cs); + StdLockGuard scoped_lock(m_cs); assert(m_buffering); assert(m_fileout == nullptr); @@ -80,7 +80,7 @@ bool BCLog::Logger::StartLogging() void BCLog::Logger::DisconnectTestLogger() { - std::lock_guard<std::mutex> scoped_lock(m_cs); + StdLockGuard scoped_lock(m_cs); m_buffering = true; if (m_fileout != nullptr) fclose(m_fileout); m_fileout = nullptr; @@ -246,7 +246,7 @@ namespace BCLog { void BCLog::Logger::LogPrintStr(const std::string& str) { - std::lock_guard<std::mutex> scoped_lock(m_cs); + StdLockGuard scoped_lock(m_cs); std::string str_prefixed = LogEscapeMessage(str); if (m_log_threadnames && m_started_new_line) { diff --git a/src/logging.h b/src/logging.h index ab07010316..7e646ef67a 100644 --- a/src/logging.h +++ b/src/logging.h @@ -8,6 +8,7 @@ #include <fs.h> #include <tinyformat.h> +#include <threadsafety.h> #include <util/string.h> #include <atomic> @@ -61,10 +62,11 @@ namespace BCLog { class Logger { private: - mutable std::mutex m_cs; // Can not use Mutex from sync.h because in debug mode it would cause a deadlock when a potential deadlock was detected - FILE* m_fileout = nullptr; // GUARDED_BY(m_cs) - std::list<std::string> m_msgs_before_open; // GUARDED_BY(m_cs) - bool m_buffering{true}; //!< Buffer messages before logging can be started. GUARDED_BY(m_cs) + mutable StdMutex m_cs; // Can not use Mutex from sync.h because in debug mode it would cause a deadlock when a potential deadlock was detected + + FILE* m_fileout GUARDED_BY(m_cs) = nullptr; + std::list<std::string> m_msgs_before_open GUARDED_BY(m_cs); + bool m_buffering GUARDED_BY(m_cs) = true; //!< Buffer messages before logging can be started. /** * m_started_new_line is a state variable that will suppress printing of @@ -79,7 +81,7 @@ namespace BCLog { std::string LogTimestampStr(const std::string& str); /** Slots that connect to the print signal */ - std::list<std::function<void(const std::string&)>> m_print_callbacks /* GUARDED_BY(m_cs) */ {}; + std::list<std::function<void(const std::string&)>> m_print_callbacks GUARDED_BY(m_cs) {}; public: bool m_print_to_console = false; @@ -98,14 +100,14 @@ namespace BCLog { /** Returns whether logs will be written to any output */ bool Enabled() const { - std::lock_guard<std::mutex> scoped_lock(m_cs); + StdLockGuard scoped_lock(m_cs); return m_buffering || m_print_to_console || m_print_to_file || !m_print_callbacks.empty(); } /** Connect a slot to the print signal and return the connection */ std::list<std::function<void(const std::string&)>>::iterator PushBackCallback(std::function<void(const std::string&)> fun) { - std::lock_guard<std::mutex> scoped_lock(m_cs); + StdLockGuard scoped_lock(m_cs); m_print_callbacks.push_back(std::move(fun)); return --m_print_callbacks.end(); } @@ -113,7 +115,7 @@ namespace BCLog { /** Delete a connection */ void DeleteCallback(std::list<std::function<void(const std::string&)>>::iterator it) { - std::lock_guard<std::mutex> scoped_lock(m_cs); + StdLockGuard scoped_lock(m_cs); m_print_callbacks.erase(it); } diff --git a/src/net.cpp b/src/net.cpp index 9950b9aea4..707412bb32 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -52,6 +52,12 @@ static constexpr std::chrono::minutes DUMP_PEERS_INTERVAL{15}; /** Number of DNS seeds to query when the number of connections is low. */ static constexpr int DNSSEEDS_TO_QUERY_AT_ONCE = 3; +/** How long to delay before querying DNS seeds + */ +static constexpr std::chrono::seconds DNSSEEDS_DELAY_FEW_PEERS{11}; // 11sec +static constexpr std::chrono::seconds DNSSEEDS_DELAY_MANY_PEERS{300}; // 5min +static constexpr int DNSSEEDS_DELAY_PEER_THRESHOLD = 1000; // "many" vs "few" peers -- you should only get this many if you've been on the live network + // We add a random period time (0 to 1 seconds) to feeler connections to prevent synchronization. #define FEELER_SLEEP_WINDOW 1 @@ -1455,7 +1461,7 @@ void CConnman::ThreadSocketHandler() void CConnman::WakeMessageHandler() { { - std::lock_guard<std::mutex> lock(mutexMsgProc); + LOCK(mutexMsgProc); fMsgProcWake = true; } condMsgProc.notify_one(); @@ -1587,31 +1593,67 @@ void CConnman::ThreadDNSAddressSeed() if (gArgs.GetBoolArg("-forcednsseed", DEFAULT_FORCEDNSSEED)) { // When -forcednsseed is provided, query all. seeds_right_now = seeds.size(); + } else if (addrman.size() == 0) { + // If we have no known peers, query all. + seeds_right_now = seeds.size(); } + // goal: only query DNS seed if address need is acute + // * If we have a reasonable number of peers in addrman, spend + // some time trying them first. This improves user privacy by + // creating fewer identifying DNS requests, reduces trust by + // giving seeds less influence on the network topology, and + // reduces traffic to the seeds. + // * When querying DNS seeds query a few at once, this ensures + // that we don't give DNS seeds the ability to eclipse nodes + // that query them. + // * If we continue having problems, eventually query all the + // DNS seeds, and if that fails too, also try the fixed seeds. + // (done in ThreadOpenConnections) + const std::chrono::seconds seeds_wait_time = (addrman.size() >= DNSSEEDS_DELAY_PEER_THRESHOLD ? DNSSEEDS_DELAY_MANY_PEERS : DNSSEEDS_DELAY_FEW_PEERS); + for (const std::string& seed : seeds) { - // goal: only query DNS seed if address need is acute - // Avoiding DNS seeds when we don't need them improves user privacy by - // creating fewer identifying DNS requests, reduces trust by giving seeds - // less influence on the network topology, and reduces traffic to the seeds. - if (addrman.size() > 0 && seeds_right_now == 0) { - if (!interruptNet.sleep_for(std::chrono::seconds(11))) return; + if (seeds_right_now == 0) { + seeds_right_now += DNSSEEDS_TO_QUERY_AT_ONCE; - LOCK(cs_vNodes); - int nRelevant = 0; - for (const CNode* pnode : vNodes) { - nRelevant += pnode->fSuccessfullyConnected && !pnode->fFeeler && !pnode->fOneShot && !pnode->m_manual_connection && !pnode->fInbound; - } - if (nRelevant >= 2) { - LogPrintf("P2P peers available. Skipped DNS seeding.\n"); - return; + if (addrman.size() > 0) { + LogPrintf("Waiting %d seconds before querying DNS seeds.\n", seeds_wait_time.count()); + std::chrono::seconds to_wait = seeds_wait_time; + while (to_wait.count() > 0) { + std::chrono::seconds w = std::min(DNSSEEDS_DELAY_FEW_PEERS, to_wait); + if (!interruptNet.sleep_for(w)) return; + to_wait -= w; + + int nRelevant = 0; + { + LOCK(cs_vNodes); + for (const CNode* pnode : vNodes) { + nRelevant += pnode->fSuccessfullyConnected && !pnode->fFeeler && !pnode->fOneShot && !pnode->m_manual_connection && !pnode->fInbound; + } + } + if (nRelevant >= 2) { + if (found > 0) { + LogPrintf("%d addresses found from DNS seeds\n", found); + LogPrintf("P2P peers available. Finished DNS seeding.\n"); + } else { + LogPrintf("P2P peers available. Skipped DNS seeding.\n"); + } + return; + } + } } - seeds_right_now += DNSSEEDS_TO_QUERY_AT_ONCE; } - if (interruptNet) { - return; + if (interruptNet) return; + + // hold off on querying seeds if p2p network deactivated + if (!fNetworkActive) { + LogPrintf("Waiting for network to be reactivated before querying DNS seeds.\n"); + do { + if (!interruptNet.sleep_for(std::chrono::seconds{1})) return; + } while (!fNetworkActive); } + LogPrintf("Loading addresses from DNS seed %s\n", seed); if (HaveNameProxy()) { AddOneShot(seed); @@ -2058,7 +2100,7 @@ void CConnman::ThreadMessageHandler() WAIT_LOCK(mutexMsgProc, lock); if (!fMoreWork) { - condMsgProc.wait_until(lock, std::chrono::steady_clock::now() + std::chrono::milliseconds(100), [this] { return fMsgProcWake; }); + condMsgProc.wait_until(lock, std::chrono::steady_clock::now() + std::chrono::milliseconds(100), [this]() EXCLUSIVE_LOCKS_REQUIRED(mutexMsgProc) { return fMsgProcWake; }); } fMsgProcWake = false; } @@ -2366,7 +2408,7 @@ static CNetCleanup instance_of_cnetcleanup; void CConnman::Interrupt() { { - std::lock_guard<std::mutex> lock(mutexMsgProc); + LOCK(mutexMsgProc); flagInterruptMsgProc = true; } condMsgProc.notify_all(); @@ -454,7 +454,7 @@ private: const uint64_t nSeed0, nSeed1; /** flag for waking the message processor. */ - bool fMsgProcWake; + bool fMsgProcWake GUARDED_BY(mutexMsgProc); std::condition_variable condMsgProc; Mutex mutexMsgProc; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d0e345c30e..404b33a977 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -129,6 +129,10 @@ static constexpr unsigned int INVENTORY_BROADCAST_MAX = 7 * INVENTORY_BROADCAST_ static constexpr unsigned int AVG_FEEFILTER_BROADCAST_INTERVAL = 10 * 60; /** Maximum feefilter broadcast delay after significant change. */ static constexpr unsigned int MAX_FEEFILTER_CHANGE_DELAY = 5 * 60; +/** Maximum number of compact filters that may be requested with one getcfilters. See BIP 157. */ +static constexpr uint32_t MAX_GETCFILTERS_SIZE = 1000; +/** Maximum number of cf hashes that may be requested with one getcfheaders. See BIP 157. */ +static constexpr uint32_t MAX_GETCFHEADERS_SIZE = 2000; struct COrphanTx { // When modifying, adapt the copy of this definition in tests/DoS_tests. @@ -825,7 +829,8 @@ void PeerLogicValidation::ReattemptInitialBroadcast(CScheduler& scheduler) const } } - // schedule next run for 10-15 minutes in the future + // Schedule next run for 10-15 minutes in the future. + // We add randomness on every cycle to avoid the possibility of P2P fingerprinting. const std::chrono::milliseconds delta = std::chrono::minutes{10} + GetRandMillis(std::chrono::minutes{5}); scheduler.scheduleFromNow([&] { ReattemptInitialBroadcast(scheduler); }, delta); } @@ -1989,14 +1994,16 @@ void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::set<uin * @param[in] pfrom The peer that we received the request from * @param[in] chain_params Chain parameters * @param[in] filter_type The filter type the request is for. Must be basic filters. + * @param[in] start_height The start height for the request * @param[in] stop_hash The stop_hash for the request + * @param[in] max_height_diff The maximum number of items permitted to request, as specified in BIP 157 * @param[out] stop_index The CBlockIndex for the stop_hash block, if the request can be serviced. * @param[out] filter_index The filter index, if the request can be serviced. * @return True if the request can be serviced. */ -static bool PrepareBlockFilterRequest(CNode* pfrom, const CChainParams& chain_params, - BlockFilterType filter_type, - const uint256& stop_hash, +static bool PrepareBlockFilterRequest(CNode& pfrom, const CChainParams& chain_params, + BlockFilterType filter_type, uint32_t start_height, + const uint256& stop_hash, uint32_t max_height_diff, const CBlockIndex*& stop_index, BlockFilterIndex*& filter_index) { @@ -2005,8 +2012,8 @@ static bool PrepareBlockFilterRequest(CNode* pfrom, const CChainParams& chain_pa gArgs.GetBoolArg("-peerblockfilters", DEFAULT_PEERBLOCKFILTERS)); if (!supported_filter_type) { LogPrint(BCLog::NET, "peer %d requested unsupported block filter type: %d\n", - pfrom->GetId(), static_cast<uint8_t>(filter_type)); - pfrom->fDisconnect = true; + pfrom.GetId(), static_cast<uint8_t>(filter_type)); + pfrom.fDisconnect = true; return false; } @@ -2017,12 +2024,27 @@ static bool PrepareBlockFilterRequest(CNode* pfrom, const CChainParams& chain_pa // Check that the stop block exists and the peer would be allowed to fetch it. if (!stop_index || !BlockRequestAllowed(stop_index, chain_params.GetConsensus())) { LogPrint(BCLog::NET, "peer %d requested invalid block hash: %s\n", - pfrom->GetId(), stop_hash.ToString()); - pfrom->fDisconnect = true; + pfrom.GetId(), stop_hash.ToString()); + pfrom.fDisconnect = true; return false; } } + uint32_t stop_height = stop_index->nHeight; + if (start_height > stop_height) { + LogPrint(BCLog::NET, "peer %d sent invalid getcfilters/getcfheaders with " /* Continued */ + "start height %d and stop height %d\n", + pfrom.GetId(), start_height, stop_height); + pfrom.fDisconnect = true; + return false; + } + if (stop_height - start_height >= max_height_diff) { + LogPrint(BCLog::NET, "peer %d requested too many cfilters/cfheaders: %d / %d\n", + pfrom.GetId(), stop_height - start_height + 1, max_height_diff); + pfrom.fDisconnect = true; + return false; + } + filter_index = GetBlockFilterIndex(filter_type); if (!filter_index) { LogPrint(BCLog::NET, "Filter index for supported type %s not found\n", BlockFilterTypeName(filter_type)); @@ -2033,6 +2055,104 @@ static bool PrepareBlockFilterRequest(CNode* pfrom, const CChainParams& chain_pa } /** + * Handle a cfilters request. + * + * May disconnect from the peer in the case of a bad request. + * + * @param[in] pfrom The peer that we received the request from + * @param[in] vRecv The raw message received + * @param[in] chain_params Chain parameters + * @param[in] connman Pointer to the connection manager + */ +static void ProcessGetCFilters(CNode& pfrom, CDataStream& vRecv, const CChainParams& chain_params, + CConnman& connman) +{ + uint8_t filter_type_ser; + uint32_t start_height; + uint256 stop_hash; + + vRecv >> filter_type_ser >> start_height >> stop_hash; + + const BlockFilterType filter_type = static_cast<BlockFilterType>(filter_type_ser); + + const CBlockIndex* stop_index; + BlockFilterIndex* filter_index; + if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, start_height, stop_hash, + MAX_GETCFILTERS_SIZE, stop_index, filter_index)) { + return; + } + + std::vector<BlockFilter> filters; + + if (!filter_index->LookupFilterRange(start_height, stop_index, filters)) { + LogPrint(BCLog::NET, "Failed to find block filter in index: filter_type=%s, start_height=%d, stop_hash=%s\n", + BlockFilterTypeName(filter_type), start_height, stop_hash.ToString()); + return; + } + + for (const auto& filter : filters) { + CSerializedNetMsg msg = CNetMsgMaker(pfrom.GetSendVersion()) + .Make(NetMsgType::CFILTER, filter); + connman.PushMessage(&pfrom, std::move(msg)); + } +} + +/** + * Handle a cfheaders request. + * + * May disconnect from the peer in the case of a bad request. + * + * @param[in] pfrom The peer that we received the request from + * @param[in] vRecv The raw message received + * @param[in] chain_params Chain parameters + * @param[in] connman Pointer to the connection manager + */ +static void ProcessGetCFHeaders(CNode& pfrom, CDataStream& vRecv, const CChainParams& chain_params, + CConnman& connman) +{ + uint8_t filter_type_ser; + uint32_t start_height; + uint256 stop_hash; + + vRecv >> filter_type_ser >> start_height >> stop_hash; + + const BlockFilterType filter_type = static_cast<BlockFilterType>(filter_type_ser); + + const CBlockIndex* stop_index; + BlockFilterIndex* filter_index; + if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, start_height, stop_hash, + MAX_GETCFHEADERS_SIZE, stop_index, filter_index)) { + return; + } + + uint256 prev_header; + if (start_height > 0) { + const CBlockIndex* const prev_block = + stop_index->GetAncestor(static_cast<int>(start_height - 1)); + if (!filter_index->LookupFilterHeader(prev_block, prev_header)) { + LogPrint(BCLog::NET, "Failed to find block filter header in index: filter_type=%s, block_hash=%s\n", + BlockFilterTypeName(filter_type), prev_block->GetBlockHash().ToString()); + return; + } + } + + std::vector<uint256> filter_hashes; + if (!filter_index->LookupFilterHashRange(start_height, stop_index, filter_hashes)) { + LogPrint(BCLog::NET, "Failed to find block filter hashes in index: filter_type=%s, start_height=%d, stop_hash=%s\n", + BlockFilterTypeName(filter_type), start_height, stop_hash.ToString()); + return; + } + + CSerializedNetMsg msg = CNetMsgMaker(pfrom.GetSendVersion()) + .Make(NetMsgType::CFHEADERS, + filter_type_ser, + stop_index->GetBlockHash(), + prev_header, + filter_hashes); + connman.PushMessage(&pfrom, std::move(msg)); +} + +/** * Handle a getcfcheckpt request. * * May disconnect from the peer in the case of a bad request. @@ -2042,8 +2162,8 @@ static bool PrepareBlockFilterRequest(CNode* pfrom, const CChainParams& chain_pa * @param[in] chain_params Chain parameters * @param[in] connman Pointer to the connection manager */ -static void ProcessGetCFCheckPt(CNode* pfrom, CDataStream& vRecv, const CChainParams& chain_params, - CConnman* connman) +static void ProcessGetCFCheckPt(CNode& pfrom, CDataStream& vRecv, const CChainParams& chain_params, + CConnman& connman) { uint8_t filter_type_ser; uint256 stop_hash; @@ -2054,7 +2174,8 @@ static void ProcessGetCFCheckPt(CNode* pfrom, CDataStream& vRecv, const CChainPa const CBlockIndex* stop_index; BlockFilterIndex* filter_index; - if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, stop_hash, + if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, /*start_height=*/0, stop_hash, + /*max_height_diff=*/std::numeric_limits<uint32_t>::max(), stop_index, filter_index)) { return; } @@ -2074,12 +2195,12 @@ static void ProcessGetCFCheckPt(CNode* pfrom, CDataStream& vRecv, const CChainPa } } - CSerializedNetMsg msg = CNetMsgMaker(pfrom->GetSendVersion()) + CSerializedNetMsg msg = CNetMsgMaker(pfrom.GetSendVersion()) .Make(NetMsgType::CFCHECKPT, filter_type_ser, stop_index->GetBlockHash(), headers); - connman->PushMessage(pfrom, std::move(msg)); + connman.PushMessage(&pfrom, std::move(msg)); } bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, CConnman* connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc) @@ -3391,8 +3512,18 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec return true; } + if (msg_type == NetMsgType::GETCFILTERS) { + ProcessGetCFilters(*pfrom, vRecv, chainparams, *connman); + return true; + } + + if (msg_type == NetMsgType::GETCFHEADERS) { + ProcessGetCFHeaders(*pfrom, vRecv, chainparams, *connman); + return true; + } + if (msg_type == NetMsgType::GETCFCHECKPT) { - ProcessGetCFCheckPt(pfrom, vRecv, chainparams, connman); + ProcessGetCFCheckPt(*pfrom, vRecv, chainparams, *connman); return true; } diff --git a/src/node/coinstats.cpp b/src/node/coinstats.cpp index ec52a08ace..e3c4c828b6 100644 --- a/src/node/coinstats.cpp +++ b/src/node/coinstats.cpp @@ -33,7 +33,7 @@ static void ApplyStats(CCoinsStats &stats, CHashWriter& ss, const uint256& hash, } //! Calculate statistics about the unspent transaction output set -bool GetUTXOStats(CCoinsView *view, CCoinsStats &stats) +bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const std::function<void()>& interruption_point) { stats = CCoinsStats(); std::unique_ptr<CCoinsViewCursor> pcursor(view->Cursor()); @@ -49,6 +49,7 @@ bool GetUTXOStats(CCoinsView *view, CCoinsStats &stats) uint256 prevkey; std::map<uint32_t, Coin> outputs; while (pcursor->Valid()) { + interruption_point(); COutPoint key; Coin coin; if (pcursor->GetKey(key) && pcursor->GetValue(coin)) { diff --git a/src/node/coinstats.h b/src/node/coinstats.h index a19af0fd1b..d9cdaa3036 100644 --- a/src/node/coinstats.h +++ b/src/node/coinstats.h @@ -10,6 +10,7 @@ #include <uint256.h> #include <cstdint> +#include <functional> class CCoinsView; @@ -29,6 +30,6 @@ struct CCoinsStats }; //! Calculate statistics about the unspent transaction output set -bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats); +bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const std::function<void()>& interruption_point = {}); #endif // BITCOIN_NODE_COINSTATS_H diff --git a/src/protocol.cpp b/src/protocol.cpp index e929cff110..83a24b9d95 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -40,6 +40,10 @@ const char *SENDCMPCT="sendcmpct"; const char *CMPCTBLOCK="cmpctblock"; const char *GETBLOCKTXN="getblocktxn"; const char *BLOCKTXN="blocktxn"; +const char *GETCFILTERS="getcfilters"; +const char *CFILTER="cfilter"; +const char *GETCFHEADERS="getcfheaders"; +const char *CFHEADERS="cfheaders"; const char *GETCFCHECKPT="getcfcheckpt"; const char *CFCHECKPT="cfcheckpt"; } // namespace NetMsgType @@ -73,6 +77,10 @@ const static std::string allNetMessageTypes[] = { NetMsgType::CMPCTBLOCK, NetMsgType::GETBLOCKTXN, NetMsgType::BLOCKTXN, + NetMsgType::GETCFILTERS, + NetMsgType::CFILTER, + NetMsgType::GETCFHEADERS, + NetMsgType::CFHEADERS, NetMsgType::GETCFCHECKPT, NetMsgType::CFCHECKPT, }; @@ -190,3 +198,46 @@ const std::vector<std::string> &getAllNetMessageTypes() { return allNetMessageTypesVec; } + +/** + * Convert a service flag (NODE_*) to a human readable string. + * It supports unknown service flags which will be returned as "UNKNOWN[...]". + * @param[in] bit the service flag is calculated as (1 << bit) + */ +static std::string serviceFlagToStr(size_t bit) +{ + const uint64_t service_flag = 1ULL << bit; + switch ((ServiceFlags)service_flag) { + case NODE_NONE: abort(); // impossible + case NODE_NETWORK: return "NETWORK"; + case NODE_GETUTXO: return "GETUTXO"; + case NODE_BLOOM: return "BLOOM"; + case NODE_WITNESS: return "WITNESS"; + case NODE_NETWORK_LIMITED: return "NETWORK_LIMITED"; + // Not using default, so we get warned when a case is missing + } + + std::ostringstream stream; + stream.imbue(std::locale::classic()); + stream << "UNKNOWN["; + if (bit < 8) { + stream << service_flag; + } else { + stream << "2^" << bit; + } + stream << "]"; + return stream.str(); +} + +std::vector<std::string> serviceFlagsToStr(uint64_t flags) +{ + std::vector<std::string> str_flags; + + for (size_t i = 0; i < sizeof(flags) * 8; ++i) { + if (flags & (1ULL << i)) { + str_flags.emplace_back(serviceFlagToStr(i)); + } + } + + return str_flags; +} diff --git a/src/protocol.h b/src/protocol.h index 8092ad7c13..985f44640b 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -226,6 +226,30 @@ extern const char* GETBLOCKTXN; */ extern const char* BLOCKTXN; /** + * getcfilters requests compact filters for a range of blocks. + * Only available with service bit NODE_COMPACT_FILTERS as described by + * BIP 157 & 158. + */ +extern const char* GETCFILTERS; +/** + * cfilter is a response to a getcfilters request containing a single compact + * filter. + */ +extern const char* CFILTER; +/** + * getcfheaders requests a compact filter header and the filter hashes for a + * range of blocks, which can then be used to reconstruct the filter headers + * for those blocks. + * Only available with service bit NODE_COMPACT_FILTERS as described by + * BIP 157 & 158. + */ +extern const char* GETCFHEADERS; +/** + * cfheaders is a response to a getcfheaders request containing a filter header + * and a vector of filter hashes for each subsequent block in the requested range. + */ +extern const char* CFHEADERS; +/** * getcfcheckpt requests evenly spaced compact filter headers, enabling * parallelized download and validation of the headers between them. * Only available with service bit NODE_COMPACT_FILTERS as described by @@ -235,8 +259,6 @@ extern const char* GETCFCHECKPT; /** * cfcheckpt is a response to a getcfcheckpt request containing a vector of * evenly spaced filter headers for blocks on the requested chain. - * Only available with service bit NODE_COMPACT_FILTERS as described by - * BIP 157 & 158. */ extern const char* CFCHECKPT; }; // namespace NetMsgType @@ -246,7 +268,7 @@ const std::vector<std::string>& getAllNetMessageTypes(); /** nServices flags */ enum ServiceFlags : uint64_t { - // NOTE: When adding here, be sure to update qt/guiutil.cpp's formatServicesStr too + // NOTE: When adding here, be sure to update serviceFlagToStr too // Nothing NODE_NONE = 0, // NODE_NETWORK means that the node is capable of serving the complete block chain. It is currently @@ -278,6 +300,13 @@ enum ServiceFlags : uint64_t { }; /** + * Convert service flags (a bitmask of NODE_*) to human readable strings. + * It supports unknown service flags which will be returned as "UNKNOWN[...]". + * @param[in] flags multiple NODE_* bitwise-OR-ed together + */ +std::vector<std::string> serviceFlagsToStr(uint64_t flags); + +/** * Gets the set of service flags which are "desirable" for a given peer. * * These are the flags which are required for a peer to support for them diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp index 0d3b08fe7d..aa4ec04497 100644 --- a/src/qt/addressbookpage.cpp +++ b/src/qt/addressbookpage.cpp @@ -106,7 +106,7 @@ AddressBookPage::AddressBookPage(const PlatformStyle *platformStyle, Mode _mode, ui->newAddress->setVisible(true); break; case ReceivingTab: - ui->labelExplanation->setText(tr("These are your Bitcoin addresses for receiving payments. Use the 'Create new receiving address' button in the receive tab to create new addresses.")); + ui->labelExplanation->setText(tr("These are your Bitcoin addresses for receiving payments. Use the 'Create new receiving address' button in the receive tab to create new addresses.\nSigning is only possible with addresses of the type 'legacy'.")); ui->deleteAddress->setVisible(false); ui->newAddress->setVisible(false); break; diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index 8110f4e895..665c8e6053 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -11,6 +11,7 @@ #include <wallet/wallet.h> #include <algorithm> +#include <typeinfo> #include <QFont> #include <QDebug> @@ -75,12 +76,14 @@ public: explicit AddressTablePriv(AddressTableModel *_parent): parent(_parent) {} - void refreshAddressTable(interfaces::Wallet& wallet) + void refreshAddressTable(interfaces::Wallet& wallet, bool pk_hash_only = false) { cachedAddressTable.clear(); { for (const auto& address : wallet.getAddresses()) { + if (pk_hash_only && address.dest.type() != typeid(PKHash)) + continue; AddressTableEntry::Type addressType = translateTransactionType( QString::fromStdString(address.purpose), address.is_mine); cachedAddressTable.append(AddressTableEntry(addressType, @@ -159,12 +162,12 @@ public: } }; -AddressTableModel::AddressTableModel(WalletModel *parent) : +AddressTableModel::AddressTableModel(WalletModel *parent, bool pk_hash_only) : QAbstractTableModel(parent), walletModel(parent) { columns << tr("Label") << tr("Address"); priv = new AddressTablePriv(this); - priv->refreshAddressTable(parent->wallet()); + priv->refreshAddressTable(parent->wallet(), pk_hash_only); } AddressTableModel::~AddressTableModel() diff --git a/src/qt/addresstablemodel.h b/src/qt/addresstablemodel.h index 97f673caf1..73316cadc4 100644 --- a/src/qt/addresstablemodel.h +++ b/src/qt/addresstablemodel.h @@ -25,7 +25,7 @@ class AddressTableModel : public QAbstractTableModel Q_OBJECT public: - explicit AddressTableModel(WalletModel *parent = nullptr); + explicit AddressTableModel(WalletModel *parent = nullptr, bool pk_hash_only = false); ~AddressTableModel(); enum ColumnIndex { diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 6fdf6322ff..ad74ca3a02 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -212,8 +212,6 @@ BitcoinApplication::~BitcoinApplication() delete window; window = nullptr; - delete optionsModel; - optionsModel = nullptr; delete platformStyle; platformStyle = nullptr; } @@ -227,7 +225,7 @@ void BitcoinApplication::createPaymentServer() void BitcoinApplication::createOptionsModel(bool resetSettings) { - optionsModel = new OptionsModel(m_node, nullptr, resetSettings); + optionsModel = new OptionsModel(m_node, this, resetSettings); } void BitcoinApplication::createWindow(const NetworkStyle *networkStyle) diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 2090c233ac..6192013e5f 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -354,6 +354,11 @@ void BitcoinGUI::createActions() showHelpMessageAction->setMenuRole(QAction::NoRole); showHelpMessageAction->setStatusTip(tr("Show the %1 help message to get a list with possible Bitcoin command-line options").arg(PACKAGE_NAME)); + m_mask_values_action = new QAction(tr("&Mask values"), this); + m_mask_values_action->setShortcut(QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_M)); + m_mask_values_action->setStatusTip(tr("Mask the values in the Overview tab")); + m_mask_values_action->setCheckable(true); + connect(quitAction, &QAction::triggered, qApp, QApplication::quit); connect(aboutAction, &QAction::triggered, this, &BitcoinGUI::aboutClicked); connect(aboutQtAction, &QAction::triggered, qApp, QApplication::aboutQt); @@ -416,6 +421,8 @@ void BitcoinGUI::createActions() connect(activity, &CreateWalletActivity::finished, activity, &QObject::deleteLater); activity->create(); }); + + connect(m_mask_values_action, &QAction::toggled, this, &BitcoinGUI::setPrivacy); } #endif // ENABLE_WALLET @@ -456,6 +463,8 @@ void BitcoinGUI::createMenuBar() settings->addAction(encryptWalletAction); settings->addAction(changePassphraseAction); settings->addSeparator(); + settings->addAction(m_mask_values_action); + settings->addSeparator(); } settings->addAction(optionsAction); @@ -1414,6 +1423,12 @@ void BitcoinGUI::unsubscribeFromCoreSignals() m_handler_question->disconnect(); } +bool BitcoinGUI::isPrivacyModeActivated() const +{ + assert(m_mask_values_action); + return m_mask_values_action->isChecked(); +} + UnitDisplayStatusBarControl::UnitDisplayStatusBarControl(const PlatformStyle *platformStyle) : optionsModel(nullptr), menu(nullptr) diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index 82a2db9ba2..c0198dd168 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -99,6 +99,8 @@ public: /** Disconnect core signals from GUI client */ void unsubscribeFromCoreSignals(); + bool isPrivacyModeActivated() const; + protected: void changeEvent(QEvent *e) override; void closeEvent(QCloseEvent *event) override; @@ -155,6 +157,7 @@ private: QAction* m_close_wallet_action{nullptr}; QAction* m_wallet_selector_label_action = nullptr; QAction* m_wallet_selector_action = nullptr; + QAction* m_mask_values_action{nullptr}; QLabel *m_wallet_selector_label = nullptr; QComboBox* m_wallet_selector = nullptr; @@ -207,6 +210,7 @@ Q_SIGNALS: void receivedURI(const QString &uri); /** Signal raised when RPC console shown */ void consoleShown(RPCConsole* console); + void setPrivacy(bool privacy); public Q_SLOTS: /** Set number of connections shown in the UI */ diff --git a/src/qt/bitcoinunits.cpp b/src/qt/bitcoinunits.cpp index d9711af123..318a6dcbfd 100644 --- a/src/qt/bitcoinunits.cpp +++ b/src/qt/bitcoinunits.cpp @@ -6,6 +6,8 @@ #include <QStringList> +#include <cassert> + BitcoinUnits::BitcoinUnits(QObject *parent): QAbstractListModel(parent), unitlist(availableUnits()) @@ -94,7 +96,7 @@ int BitcoinUnits::decimals(int unit) } } -QString BitcoinUnits::format(int unit, const CAmount& nIn, bool fPlus, SeparatorStyle separators) +QString BitcoinUnits::format(int unit, const CAmount& nIn, bool fPlus, SeparatorStyle separators, bool justify) { // Note: not using straight sprintf here because we do NOT want // localized number formatting. @@ -106,6 +108,7 @@ QString BitcoinUnits::format(int unit, const CAmount& nIn, bool fPlus, Separator qint64 n_abs = (n > 0 ? n : -n); qint64 quotient = n_abs / coin; QString quotient_str = QString::number(quotient); + if (justify) quotient_str = quotient_str.rightJustified(16 - num_decimals, ' '); // Use SI-style thin space separators as these are locale independent and can't be // confused with the decimal marker. @@ -150,6 +153,17 @@ QString BitcoinUnits::formatHtmlWithUnit(int unit, const CAmount& amount, bool p return QString("<span style='white-space: nowrap;'>%1</span>").arg(str); } +QString BitcoinUnits::formatWithPrivacy(int unit, const CAmount& amount, SeparatorStyle separators, bool privacy) +{ + assert(amount >= 0); + QString value; + if (privacy) { + value = format(unit, 0, false, separators, true).replace('0', '#'); + } else { + value = format(unit, amount, false, separators, true); + } + return value + QString(" ") + shortName(unit); +} bool BitcoinUnits::parse(int unit, const QString &value, CAmount *val_out) { diff --git a/src/qt/bitcoinunits.h b/src/qt/bitcoinunits.h index 1ff4702117..dac5484393 100644 --- a/src/qt/bitcoinunits.h +++ b/src/qt/bitcoinunits.h @@ -72,11 +72,13 @@ public: //! Number of decimals left static int decimals(int unit); //! Format as string - static QString format(int unit, const CAmount& amount, bool plussign=false, SeparatorStyle separators=separatorStandard); + static QString format(int unit, const CAmount& amount, bool plussign = false, SeparatorStyle separators = separatorStandard, bool justify = false); //! Format as string (with unit) static QString formatWithUnit(int unit, const CAmount& amount, bool plussign=false, SeparatorStyle separators=separatorStandard); //! Format as HTML string (with unit) static QString formatHtmlWithUnit(int unit, const CAmount& amount, bool plussign=false, SeparatorStyle separators=separatorStandard); + //! Format as string (with unit) of fixed length to preserve privacy, if it is set. + static QString formatWithPrivacy(int unit, const CAmount& amount, SeparatorStyle separators, bool privacy); //! Parse string to coin amount static bool parse(int unit, const QString &value, CAmount *val_out); //! Gets title for amount column including current display unit if optionsModel reference available */ diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index 159b0d3df3..f15921c5bc 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -114,6 +114,15 @@ int ClientModel::getNumBlocks() const return m_cached_num_blocks; } +uint256 ClientModel::getBestBlockHash() +{ + LOCK(m_cached_tip_mutex); + if (m_cached_tip_blocks.IsNull()) { + m_cached_tip_blocks = m_node.getBestBlockHash(); + } + return m_cached_tip_blocks; +} + void ClientModel::updateNumConnections(int numConnections) { Q_EMIT numConnectionsChanged(numConnections); @@ -235,14 +244,15 @@ static void BannedListChanged(ClientModel *clientmodel) assert(invoked); } -static void BlockTipChanged(ClientModel* clientmodel, SynchronizationState sync_state, int height, int64_t blockTime, double verificationProgress, bool fHeader) +static void BlockTipChanged(ClientModel* clientmodel, SynchronizationState sync_state, interfaces::BlockTip tip, double verificationProgress, bool fHeader) { if (fHeader) { // cache best headers time and height to reduce future cs_main locks - clientmodel->cachedBestHeaderHeight = height; - clientmodel->cachedBestHeaderTime = blockTime; + clientmodel->cachedBestHeaderHeight = tip.block_height; + clientmodel->cachedBestHeaderTime = tip.block_time; } else { - clientmodel->m_cached_num_blocks = height; + clientmodel->m_cached_num_blocks = tip.block_height; + WITH_LOCK(clientmodel->m_cached_tip_mutex, clientmodel->m_cached_tip_blocks = tip.block_hash;); } // Throttle GUI notifications about (a) blocks during initial sync, and (b) both blocks and headers during reindex. @@ -254,8 +264,8 @@ static void BlockTipChanged(ClientModel* clientmodel, SynchronizationState sync_ } bool invoked = QMetaObject::invokeMethod(clientmodel, "numBlocksChanged", Qt::QueuedConnection, - Q_ARG(int, height), - Q_ARG(QDateTime, QDateTime::fromTime_t(blockTime)), + Q_ARG(int, tip.block_height), + Q_ARG(QDateTime, QDateTime::fromTime_t(tip.block_time)), Q_ARG(double, verificationProgress), Q_ARG(bool, fHeader), Q_ARG(SynchronizationState, sync_state)); @@ -271,8 +281,8 @@ void ClientModel::subscribeToCoreSignals() m_handler_notify_network_active_changed = m_node.handleNotifyNetworkActiveChanged(std::bind(NotifyNetworkActiveChanged, this, std::placeholders::_1)); m_handler_notify_alert_changed = m_node.handleNotifyAlertChanged(std::bind(NotifyAlertChanged, this)); m_handler_banned_list_changed = m_node.handleBannedListChanged(std::bind(BannedListChanged, this)); - m_handler_notify_block_tip = m_node.handleNotifyBlockTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4, false)); - m_handler_notify_header_tip = m_node.handleNotifyHeaderTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4, true)); + m_handler_notify_block_tip = m_node.handleNotifyBlockTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, false)); + m_handler_notify_header_tip = m_node.handleNotifyHeaderTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, true)); } void ClientModel::unsubscribeFromCoreSignals() diff --git a/src/qt/clientmodel.h b/src/qt/clientmodel.h index ace77f5972..7f12cce1d9 100644 --- a/src/qt/clientmodel.h +++ b/src/qt/clientmodel.h @@ -10,6 +10,8 @@ #include <atomic> #include <memory> +#include <sync.h> +#include <uint256.h> class BanTableModel; class CBlockIndex; @@ -57,6 +59,7 @@ public: //! Return number of connections, default is in- and outbound (total) int getNumConnections(unsigned int flags = CONNECTIONS_ALL) const; int getNumBlocks() const; + uint256 getBestBlockHash(); int getHeaderTipHeight() const; int64_t getHeaderTipTime() const; @@ -74,11 +77,14 @@ public: bool getProxyInfo(std::string& ip_port) const; - // caches for the best header, number of blocks + // caches for the best header: hash, number of blocks and block time mutable std::atomic<int> cachedBestHeaderHeight; mutable std::atomic<int64_t> cachedBestHeaderTime; mutable std::atomic<int> m_cached_num_blocks{-1}; + Mutex m_cached_tip_mutex; + uint256 m_cached_tip_blocks GUARDED_BY(m_cached_tip_mutex){}; + private: interfaces::Node& m_node; std::unique_ptr<interfaces::Handler> m_handler_show_progress; diff --git a/src/qt/forms/overviewpage.ui b/src/qt/forms/overviewpage.ui index 710801ee96..4d3f90c484 100644 --- a/src/qt/forms/overviewpage.ui +++ b/src/qt/forms/overviewpage.ui @@ -6,8 +6,8 @@ <rect> <x>0</x> <y>0</y> - <width>596</width> - <height>342</height> + <width>798</width> + <height>318</height> </rect> </property> <property name="windowTitle"> @@ -118,6 +118,7 @@ <widget class="QLabel" name="labelWatchPending"> <property name="font"> <font> + <family>Monospace</family> <weight>75</weight> <bold>true</bold> </font> @@ -129,7 +130,7 @@ <string>Unconfirmed transactions to watch-only addresses</string> </property> <property name="text"> - <string notr="true">0.000 000 00 BTC</string> + <string notr="true">0.00000000 BTC</string> </property> <property name="alignment"> <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set> @@ -143,6 +144,7 @@ <widget class="QLabel" name="labelUnconfirmed"> <property name="font"> <font> + <family>Monospace</family> <weight>75</weight> <bold>true</bold> </font> @@ -154,7 +156,7 @@ <string>Total of transactions that have yet to be confirmed, and do not yet count toward the spendable balance</string> </property> <property name="text"> - <string notr="true">0.000 000 00 BTC</string> + <string notr="true">0.00000000 BTC</string> </property> <property name="alignment"> <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set> @@ -168,6 +170,7 @@ <widget class="QLabel" name="labelWatchImmature"> <property name="font"> <font> + <family>Monospace</family> <weight>75</weight> <bold>true</bold> </font> @@ -179,7 +182,7 @@ <string>Mined balance in watch-only addresses that has not yet matured</string> </property> <property name="text"> - <string notr="true">0.000 000 00 BTC</string> + <string notr="true">0.00000000 BTC</string> </property> <property name="alignment"> <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set> @@ -226,6 +229,7 @@ <widget class="QLabel" name="labelImmature"> <property name="font"> <font> + <family>Monospace</family> <weight>75</weight> <bold>true</bold> </font> @@ -237,7 +241,7 @@ <string>Mined balance that has not yet matured</string> </property> <property name="text"> - <string notr="true">0.000 000 00 BTC</string> + <string notr="true">0.00000000 BTC</string> </property> <property name="alignment"> <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set> @@ -271,6 +275,7 @@ <widget class="QLabel" name="labelTotal"> <property name="font"> <font> + <family>Monospace</family> <weight>75</weight> <bold>true</bold> </font> @@ -282,7 +287,7 @@ <string>Your current total balance</string> </property> <property name="text"> - <string notr="true">0.000 000 00 BTC</string> + <string notr="true">21 000 000.00000000 BTC</string> </property> <property name="alignment"> <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set> @@ -296,6 +301,7 @@ <widget class="QLabel" name="labelWatchTotal"> <property name="font"> <font> + <family>Monospace</family> <weight>75</weight> <bold>true</bold> </font> @@ -307,7 +313,7 @@ <string>Current total balance in watch-only addresses</string> </property> <property name="text"> - <string notr="true">0.000 000 00 BTC</string> + <string notr="true">21 000 000.00000000 BTC</string> </property> <property name="alignment"> <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set> @@ -338,6 +344,7 @@ <widget class="QLabel" name="labelBalance"> <property name="font"> <font> + <family>Monospace</family> <weight>75</weight> <bold>true</bold> </font> @@ -349,7 +356,7 @@ <string>Your current spendable balance</string> </property> <property name="text"> - <string notr="true">0.000 000 00 BTC</string> + <string notr="true">21 000 000.00000000 BTC</string> </property> <property name="alignment"> <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set> @@ -363,6 +370,7 @@ <widget class="QLabel" name="labelWatchAvailable"> <property name="font"> <font> + <family>Monospace</family> <weight>75</weight> <bold>true</bold> </font> @@ -374,7 +382,7 @@ <string>Your current balance in watch-only addresses</string> </property> <property name="text"> - <string notr="true">0.000 000 00 BTC</string> + <string notr="true">21 000 000.00000000 BTC</string> </property> <property name="alignment"> <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set> diff --git a/src/qt/forms/receiverequestdialog.ui b/src/qt/forms/receiverequestdialog.ui index 9f896ee3b1..f6d4723465 100644 --- a/src/qt/forms/receiverequestdialog.ui +++ b/src/qt/forms/receiverequestdialog.ui @@ -6,68 +6,233 @@ <rect> <x>0</x> <y>0</y> - <width>487</width> - <height>597</height> + <width>413</width> + <height>229</height> </rect> </property> - <layout class="QVBoxLayout" name="verticalLayout_3"> - <item> - <widget class="QRImageWidget" name="lblQRCode"> - <property name="sizePolicy"> - <sizepolicy hsizetype="Expanding" vsizetype="Fixed"> - <horstretch>0</horstretch> - <verstretch>0</verstretch> - </sizepolicy> - </property> - <property name="minimumSize"> - <size> - <width>300</width> - <height>320</height> - </size> - </property> - <property name="toolTip"> - <string>QR Code</string> + <property name="windowTitle"> + <string>Request payment to ...</string> + </property> + <layout class="QGridLayout" name="gridLayout" columnstretch="0,1"> + <property name="sizeConstraint"> + <enum>QLayout::SetFixedSize</enum> + </property> + <item row="0" column="0" colspan="2" alignment="Qt::AlignHCenter"> + <widget class="QRImageWidget" name="qr_code"> + <property name="text"> + <string notr="true">QR image</string> + </property> + <property name="textInteractionFlags"> + <set>Qt::NoTextInteraction</set> + </property> + </widget> + </item> + <item row="1" column="0" colspan="2"> + <widget class="QLabel" name="payment_header"> + <property name="font"> + <font> + <weight>75</weight> + <bold>true</bold> + </font> + </property> + <property name="text"> + <string>Payment information</string> + </property> + <property name="textInteractionFlags"> + <set>Qt::NoTextInteraction</set> + </property> + </widget> + </item> + <item row="2" column="0" alignment="Qt::AlignRight|Qt::AlignTop"> + <widget class="QLabel" name="uri_tag"> + <property name="font"> + <font> + <weight>75</weight> + <bold>true</bold> + </font> + </property> + <property name="text"> + <string notr="true">URI:</string> </property> <property name="textFormat"> <enum>Qt::PlainText</enum> </property> - <property name="alignment"> - <set>Qt::AlignCenter</set> + <property name="textInteractionFlags"> + <set>Qt::NoTextInteraction</set> + </property> + </widget> + </item> + <item row="2" column="1" alignment="Qt::AlignTop"> + <widget class="QLabel" name="uri_content"> + <property name="text"> + <string notr="true">bitcoin:BC1...</string> + </property> + <property name="textFormat"> + <enum>Qt::RichText</enum> </property> <property name="wordWrap"> <bool>true</bool> </property> + <property name="textInteractionFlags"> + <set>Qt::TextSelectableByMouse</set> + </property> + </widget> + </item> + <item row="3" column="0" alignment="Qt::AlignRight|Qt::AlignTop"> + <widget class="QLabel" name="address_tag"> + <property name="font"> + <font> + <weight>75</weight> + <bold>true</bold> + </font> + </property> + <property name="text"> + <string>Address:</string> + </property> + <property name="textInteractionFlags"> + <set>Qt::NoTextInteraction</set> + </property> </widget> </item> - <item> - <widget class="QTextEdit" name="outUri"> - <property name="sizePolicy"> - <sizepolicy hsizetype="Expanding" vsizetype="Expanding"> - <horstretch>0</horstretch> - <verstretch>0</verstretch> - </sizepolicy> - </property> - <property name="minimumSize"> - <size> - <width>0</width> - <height>50</height> - </size> - </property> - <property name="frameShape"> - <enum>QFrame::NoFrame</enum> - </property> - <property name="frameShadow"> - <enum>QFrame::Plain</enum> - </property> - <property name="tabChangesFocus"> + <item row="3" column="1" alignment="Qt::AlignTop"> + <widget class="QLabel" name="address_content"> + <property name="text"> + <string notr="true">bc1...</string> + </property> + <property name="textFormat"> + <enum>Qt::PlainText</enum> + </property> + <property name="textInteractionFlags"> + <set>Qt::TextSelectableByMouse</set> + </property> + </widget> + </item> + <item row="4" column="0" alignment="Qt::AlignRight|Qt::AlignTop"> + <widget class="QLabel" name="amount_tag"> + <property name="font"> + <font> + <weight>75</weight> + <bold>true</bold> + </font> + </property> + <property name="text"> + <string>Amount:</string> + </property> + <property name="textInteractionFlags"> + <set>Qt::NoTextInteraction</set> + </property> + </widget> + </item> + <item row="4" column="1" alignment="Qt::AlignTop"> + <widget class="QLabel" name="amount_content"> + <property name="text"> + <string notr="true">0.00000000 BTC</string> + </property> + <property name="textFormat"> + <enum>Qt::PlainText</enum> + </property> + <property name="textInteractionFlags"> + <set>Qt::TextSelectableByMouse</set> + </property> + </widget> + </item> + <item row="5" column="0" alignment="Qt::AlignRight|Qt::AlignTop"> + <widget class="QLabel" name="label_tag"> + <property name="font"> + <font> + <weight>75</weight> + <bold>true</bold> + </font> + </property> + <property name="text"> + <string>Label:</string> + </property> + <property name="textInteractionFlags"> + <set>Qt::NoTextInteraction</set> + </property> + </widget> + </item> + <item row="5" column="1" alignment="Qt::AlignTop"> + <widget class="QLabel" name="label_content"> + <property name="text"> + <string notr="true">label content</string> + </property> + <property name="textFormat"> + <enum>Qt::PlainText</enum> + </property> + <property name="wordWrap"> + <bool>true</bool> + </property> + <property name="textInteractionFlags"> + <set>Qt::TextSelectableByMouse</set> + </property> + </widget> + </item> + <item row="6" column="0" alignment="Qt::AlignRight|Qt::AlignTop"> + <widget class="QLabel" name="message_tag"> + <property name="font"> + <font> + <weight>75</weight> + <bold>true</bold> + </font> + </property> + <property name="text"> + <string>Message:</string> + </property> + <property name="textInteractionFlags"> + <set>Qt::NoTextInteraction</set> + </property> + </widget> + </item> + <item row="6" column="1" alignment="Qt::AlignTop"> + <widget class="QLabel" name="message_content"> + <property name="text"> + <string notr="true">message content</string> + </property> + <property name="textFormat"> + <enum>Qt::PlainText</enum> + </property> + <property name="wordWrap"> + <bool>true</bool> + </property> + <property name="textInteractionFlags"> + <set>Qt::TextSelectableByMouse</set> + </property> + </widget> + </item> + <item row="7" column="0" alignment="Qt::AlignRight|Qt::AlignTop"> + <widget class="QLabel" name="wallet_tag"> + <property name="font"> + <font> + <weight>75</weight> + <bold>true</bold> + </font> + </property> + <property name="text"> + <string>Wallet:</string> + </property> + <property name="textInteractionFlags"> + <set>Qt::NoTextInteraction</set> + </property> + </widget> + </item> + <item row="7" column="1" alignment="Qt::AlignTop"> + <widget class="QLabel" name="wallet_content"> + <property name="text"> + <string notr="true">wallet name</string> + </property> + <property name="textFormat"> + <enum>Qt::PlainText</enum> + </property> + <property name="wordWrap"> <bool>true</bool> </property> <property name="textInteractionFlags"> - <set>Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse</set> + <set>Qt::TextSelectableByMouse</set> </property> </widget> </item> - <item> + <item row="8" column="0" colspan="2"> <layout class="QHBoxLayout" name="horizontalLayout"> <item> <widget class="QPushButton" name="btnCopyURI"> @@ -114,8 +279,11 @@ </item> <item> <widget class="QDialogButtonBox" name="buttonBox"> + <property name="focusPolicy"> + <enum>Qt::StrongFocus</enum> + </property> <property name="standardButtons"> - <set>QDialogButtonBox::Close</set> + <set>QDialogButtonBox::Ok</set> </property> </widget> </item> @@ -130,37 +298,27 @@ <header>qt/qrimagewidget.h</header> </customwidget> </customwidgets> + <tabstops> + <tabstop>buttonBox</tabstop> + <tabstop>btnCopyURI</tabstop> + <tabstop>btnCopyAddress</tabstop> + <tabstop>btnSaveAs</tabstop> + </tabstops> <resources/> <connections> <connection> <sender>buttonBox</sender> - <signal>rejected()</signal> - <receiver>ReceiveRequestDialog</receiver> - <slot>reject()</slot> - <hints> - <hint type="sourcelabel"> - <x>452</x> - <y>573</y> - </hint> - <hint type="destinationlabel"> - <x>243</x> - <y>298</y> - </hint> - </hints> - </connection> - <connection> - <sender>buttonBox</sender> <signal>accepted()</signal> <receiver>ReceiveRequestDialog</receiver> <slot>accept()</slot> <hints> <hint type="sourcelabel"> - <x>452</x> - <y>573</y> + <x>135</x> + <y>230</y> </hint> <hint type="destinationlabel"> - <x>243</x> - <y>298</y> + <x>135</x> + <y>126</y> </hint> </hints> </connection> diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index af86fe5d27..ce44d4f3a5 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -48,13 +48,14 @@ #include <QKeyEvent> #include <QLineEdit> #include <QList> +#include <QMenu> #include <QMouseEvent> #include <QProgressDialog> #include <QScreen> #include <QSettings> +#include <QShortcut> #include <QSize> #include <QString> -#include <QShortcut> #include <QTextDocument> // for Qt::mightBeRichText #include <QThread> #include <QUrlQuery> @@ -231,7 +232,7 @@ QString HtmlEscape(const std::string& str, bool fMultiLine) return HtmlEscape(QString::fromStdString(str), fMultiLine); } -void copyEntryData(QAbstractItemView *view, int column, int role) +void copyEntryData(const QAbstractItemView *view, int column, int role) { if(!view || !view->selectionModel()) return; @@ -244,13 +245,20 @@ void copyEntryData(QAbstractItemView *view, int column, int role) } } -QList<QModelIndex> getEntryData(QAbstractItemView *view, int column) +QList<QModelIndex> getEntryData(const QAbstractItemView *view, int column) { if(!view || !view->selectionModel()) return QList<QModelIndex>(); return view->selectionModel()->selectedRows(column); } +bool hasEntryData(const QAbstractItemView *view, int column, int role) +{ + QModelIndexList selection = getEntryData(view, column); + if (selection.isEmpty()) return false; + return !selection.at(0).data(role).toString().isEmpty(); +} + QString getDefaultDataDirectory() { return boostPathToQString(GetDefaultDataDir()); @@ -743,34 +751,12 @@ QString formatDurationStr(int secs) return strList.join(" "); } -QString serviceFlagToStr(const quint64 mask, const int bit) -{ - switch (ServiceFlags(mask)) { - case NODE_NONE: abort(); // impossible - case NODE_NETWORK: return "NETWORK"; - case NODE_GETUTXO: return "GETUTXO"; - case NODE_BLOOM: return "BLOOM"; - case NODE_WITNESS: return "WITNESS"; - case NODE_NETWORK_LIMITED: return "NETWORK_LIMITED"; - // Not using default, so we get warned when a case is missing - } - if (bit < 8) { - return QString("%1[%2]").arg("UNKNOWN").arg(mask); - } else { - return QString("%1[2^%2]").arg("UNKNOWN").arg(bit); - } -} - QString formatServicesStr(quint64 mask) { QStringList strList; - for (int i = 0; i < 64; i++) { - uint64_t check = 1LL << i; - if (mask & check) - { - strList.append(serviceFlagToStr(check, i)); - } + for (const auto& flag : serviceFlagsToStr(mask)) { + strList.append(QString::fromStdString(flag)); } if (strList.size()) @@ -910,4 +896,11 @@ void LogQtInfo() } } +void PopupMenu(QMenu* menu, const QPoint& point, QAction* at_action) +{ + // The qminimal plugin does not provide window system integration. + if (QApplication::platformName() == "minimal") return; + menu->popup(point, at_action); +} + } // namespace GUIUtil diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index 8b9fca4fb1..8741d90102 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -28,9 +28,12 @@ namespace interfaces QT_BEGIN_NAMESPACE class QAbstractItemView; +class QAction; class QDateTime; class QFont; class QLineEdit; +class QMenu; +class QPoint; class QProgressDialog; class QUrl; class QWidget; @@ -68,14 +71,21 @@ namespace GUIUtil @param[in] role Data role to extract from the model @see TransactionView::copyLabel, TransactionView::copyAmount, TransactionView::copyAddress */ - void copyEntryData(QAbstractItemView *view, int column, int role=Qt::EditRole); + void copyEntryData(const QAbstractItemView *view, int column, int role=Qt::EditRole); /** Return a field of the currently selected entry as a QString. Does nothing if nothing is selected. @param[in] column Data column to extract from the model @see TransactionView::copyLabel, TransactionView::copyAmount, TransactionView::copyAddress */ - QList<QModelIndex> getEntryData(QAbstractItemView *view, int column); + QList<QModelIndex> getEntryData(const QAbstractItemView *view, int column); + + /** Returns true if the specified field of the currently selected view entry is not empty. + @param[in] column Data column to extract from the model + @param[in] role Data role to extract from the model + @see TransactionView::contextualMenu + */ + bool hasEntryData(const QAbstractItemView *view, int column, int role); void setClipboard(const QString& str); @@ -273,6 +283,11 @@ namespace GUIUtil * Writes to debug.log short info about the used Qt and the host system. */ void LogQtInfo(); + + /** + * Call QMenu::popup() only on supported QT_QPA_PLATFORM. + */ + void PopupMenu(QMenu* menu, const QPoint& point, QAction* at_action = nullptr); } // namespace GUIUtil #endif // BITCOIN_QT_GUIUTIL_H diff --git a/src/qt/modaloverlay.cpp b/src/qt/modaloverlay.cpp index 49a1992803..0ba1beaf3e 100644 --- a/src/qt/modaloverlay.cpp +++ b/src/qt/modaloverlay.cpp @@ -5,12 +5,12 @@ #include <qt/modaloverlay.h> #include <qt/forms/ui_modaloverlay.h> -#include <qt/guiutil.h> - #include <chainparams.h> +#include <qt/guiutil.h> -#include <QResizeEvent> +#include <QEasingCurve> #include <QPropertyAnimation> +#include <QResizeEvent> ModalOverlay::ModalOverlay(bool enable_wallet, QWidget *parent) : QWidget(parent), @@ -33,6 +33,11 @@ userClosed(false) ui->infoText->setVisible(false); ui->infoTextStrong->setText(tr("%1 is currently syncing. It will download headers and blocks from peers and validate them until reaching the tip of the block chain.").arg(PACKAGE_NAME)); } + + m_animation.setTargetObject(this); + m_animation.setPropertyName("pos"); + m_animation.setDuration(300 /* ms */); + m_animation.setEasingCurve(QEasingCurve::OutQuad); } ModalOverlay::~ModalOverlay() @@ -48,6 +53,9 @@ bool ModalOverlay::eventFilter(QObject * obj, QEvent * ev) { if (!layerIsVisible) setGeometry(0, height(), width(), height()); + if (m_animation.endValue().toPoint().y() > 0) { + m_animation.setEndValue(QPoint(0, height())); + } } else if (ev->type() == QEvent::ChildAdded) { raise(); @@ -166,14 +174,10 @@ void ModalOverlay::showHide(bool hide, bool userRequested) if (!isVisible() && !hide) setVisible(true); - setGeometry(0, hide ? 0 : height(), width(), height()); - - QPropertyAnimation* animation = new QPropertyAnimation(this, "pos"); - animation->setDuration(300); - animation->setStartValue(QPoint(0, hide ? 0 : this->height())); - animation->setEndValue(QPoint(0, hide ? this->height() : 0)); - animation->setEasingCurve(QEasingCurve::OutQuad); - animation->start(QAbstractAnimation::DeleteWhenStopped); + m_animation.setStartValue(QPoint(0, hide ? 0 : height())); + // The eventFilter() updates the endValue if it is required for QEvent::Resize. + m_animation.setEndValue(QPoint(0, hide ? height() : 0)); + m_animation.start(QAbstractAnimation::KeepWhenStopped); layerIsVisible = !hide; } diff --git a/src/qt/modaloverlay.h b/src/qt/modaloverlay.h index a565d7d8f3..1d84046d3d 100644 --- a/src/qt/modaloverlay.h +++ b/src/qt/modaloverlay.h @@ -6,6 +6,7 @@ #define BITCOIN_QT_MODALOVERLAY_H #include <QDateTime> +#include <QPropertyAnimation> #include <QWidget> //! The required delta of headers to the estimated number of available headers until we show the IBD progress @@ -45,6 +46,7 @@ private: QVector<QPair<qint64, double> > blockProcessTime; bool layerIsVisible; bool userClosed; + QPropertyAnimation m_animation; void UpdateHeaderSyncLabel(); }; diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index e20ec229fc..0af70f2735 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -16,7 +16,9 @@ #include <qt/walletmodel.h> #include <QAbstractItemDelegate> +#include <QApplication> #include <QPainter> +#include <QStatusTipEvent> #define DECORATION_SIZE 54 #define NUM_ITEMS 5 @@ -152,6 +154,21 @@ void OverviewPage::handleOutOfSyncWarningClicks() Q_EMIT outOfSyncWarningClicked(); } +void OverviewPage::setPrivacy(bool privacy) +{ + m_privacy = privacy; + if (m_balances.balance != -1) { + setBalance(m_balances); + } + + ui->listTransactions->setVisible(!m_privacy); + + const QString status_tip = m_privacy ? tr("Privacy mode activated for the Overview tab. To unmask the values, uncheck Settings->Mask values.") : ""; + setStatusTip(status_tip); + QStatusTipEvent event(status_tip); + QApplication::sendEvent(this, &event); +} + OverviewPage::~OverviewPage() { delete ui; @@ -163,25 +180,25 @@ void OverviewPage::setBalance(const interfaces::WalletBalances& balances) m_balances = balances; if (walletModel->wallet().isLegacy()) { if (walletModel->wallet().privateKeysDisabled()) { - ui->labelBalance->setText(BitcoinUnits::formatWithUnit(unit, balances.watch_only_balance, false, BitcoinUnits::separatorAlways)); - ui->labelUnconfirmed->setText(BitcoinUnits::formatWithUnit(unit, balances.unconfirmed_watch_only_balance, false, BitcoinUnits::separatorAlways)); - ui->labelImmature->setText(BitcoinUnits::formatWithUnit(unit, balances.immature_watch_only_balance, false, BitcoinUnits::separatorAlways)); - ui->labelTotal->setText(BitcoinUnits::formatWithUnit(unit, balances.watch_only_balance + balances.unconfirmed_watch_only_balance + balances.immature_watch_only_balance, false, BitcoinUnits::separatorAlways)); + ui->labelBalance->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelUnconfirmed->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance + balances.unconfirmed_watch_only_balance + balances.immature_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); } else { - ui->labelBalance->setText(BitcoinUnits::formatWithUnit(unit, balances.balance, false, BitcoinUnits::separatorAlways)); - ui->labelUnconfirmed->setText(BitcoinUnits::formatWithUnit(unit, balances.unconfirmed_balance, false, BitcoinUnits::separatorAlways)); - ui->labelImmature->setText(BitcoinUnits::formatWithUnit(unit, balances.immature_balance, false, BitcoinUnits::separatorAlways)); - ui->labelTotal->setText(BitcoinUnits::formatWithUnit(unit, balances.balance + balances.unconfirmed_balance + balances.immature_balance, false, BitcoinUnits::separatorAlways)); - ui->labelWatchAvailable->setText(BitcoinUnits::formatWithUnit(unit, balances.watch_only_balance, false, BitcoinUnits::separatorAlways)); - ui->labelWatchPending->setText(BitcoinUnits::formatWithUnit(unit, balances.unconfirmed_watch_only_balance, false, BitcoinUnits::separatorAlways)); - ui->labelWatchImmature->setText(BitcoinUnits::formatWithUnit(unit, balances.immature_watch_only_balance, false, BitcoinUnits::separatorAlways)); - ui->labelWatchTotal->setText(BitcoinUnits::formatWithUnit(unit, balances.watch_only_balance + balances.unconfirmed_watch_only_balance + balances.immature_watch_only_balance, false, BitcoinUnits::separatorAlways)); + ui->labelBalance->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelUnconfirmed->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance + balances.unconfirmed_balance + balances.immature_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelWatchAvailable->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelWatchPending->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelWatchImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelWatchTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance + balances.unconfirmed_watch_only_balance + balances.immature_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); } } else { - ui->labelBalance->setText(BitcoinUnits::formatWithUnit(unit, balances.balance, false, BitcoinUnits::separatorAlways)); - ui->labelUnconfirmed->setText(BitcoinUnits::formatWithUnit(unit, balances.unconfirmed_balance, false, BitcoinUnits::separatorAlways)); - ui->labelImmature->setText(BitcoinUnits::formatWithUnit(unit, balances.immature_balance, false, BitcoinUnits::separatorAlways)); - ui->labelTotal->setText(BitcoinUnits::formatWithUnit(unit, balances.balance + balances.unconfirmed_balance + balances.immature_balance, false, BitcoinUnits::separatorAlways)); + ui->labelBalance->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelUnconfirmed->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance + balances.unconfirmed_balance + balances.immature_balance, BitcoinUnits::separatorAlways, m_privacy)); } // only show immature (newly mined) balance if it's non-zero, so as not to complicate things // for the non-mining users diff --git a/src/qt/overviewpage.h b/src/qt/overviewpage.h index 00ba7ad4ce..4cf673b6a6 100644 --- a/src/qt/overviewpage.h +++ b/src/qt/overviewpage.h @@ -39,6 +39,7 @@ public: public Q_SLOTS: void setBalance(const interfaces::WalletBalances& balances); + void setPrivacy(bool privacy); Q_SIGNALS: void transactionClicked(const QModelIndex &index); @@ -49,6 +50,7 @@ private: ClientModel *clientModel; WalletModel *walletModel; interfaces::WalletBalances m_balances; + bool m_privacy{false}; TxViewDelegate *txdelegate; std::unique_ptr<TransactionFilterProxy> filter; diff --git a/src/qt/receiverequestdialog.cpp b/src/qt/receiverequestdialog.cpp index 30bd5c6a5a..d385c42821 100644 --- a/src/qt/receiverequestdialog.cpp +++ b/src/qt/receiverequestdialog.cpp @@ -8,10 +8,11 @@ #include <qt/bitcoinunits.h> #include <qt/guiutil.h> #include <qt/optionsmodel.h> +#include <qt/qrimagewidget.h> #include <qt/walletmodel.h> -#include <QClipboard> -#include <QPixmap> +#include <QDialog> +#include <QString> #if defined(HAVE_CONFIG_H) #include <config/bitcoin-config.h> /* for USE_QRCODE */ @@ -23,14 +24,6 @@ ReceiveRequestDialog::ReceiveRequestDialog(QWidget *parent) : model(nullptr) { ui->setupUi(this); - -#ifndef USE_QRCODE - ui->btnSaveAs->setVisible(false); - ui->lblQRCode->setVisible(false); -#endif - - connect(ui->btnSaveAs, &QPushButton::clicked, ui->lblQRCode, &QRImageWidget::saveImage); - GUIUtil::handleCloseWindowShortcut(this); } @@ -44,7 +37,7 @@ void ReceiveRequestDialog::setModel(WalletModel *_model) this->model = _model; if (_model) - connect(_model->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &ReceiveRequestDialog::update); + connect(_model->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &ReceiveRequestDialog::updateDisplayUnit); // update the display unit if necessary update(); @@ -53,40 +46,55 @@ void ReceiveRequestDialog::setModel(WalletModel *_model) void ReceiveRequestDialog::setInfo(const SendCoinsRecipient &_info) { this->info = _info; - update(); -} + setWindowTitle(tr("Request payment to %1").arg(info.label.isEmpty() ? info.address : info.label)); + QString uri = GUIUtil::formatBitcoinURI(info); -void ReceiveRequestDialog::update() -{ - if(!model) - return; - QString target = info.label; - if(target.isEmpty()) - target = info.address; - setWindowTitle(tr("Request payment to %1").arg(target)); +#ifdef USE_QRCODE + if (ui->qr_code->setQR(uri, info.address)) { + connect(ui->btnSaveAs, &QPushButton::clicked, ui->qr_code, &QRImageWidget::saveImage); + } else { + ui->btnSaveAs->setEnabled(false); + } +#else + ui->btnSaveAs->hide(); + ui->qr_code->hide(); +#endif - QString uri = GUIUtil::formatBitcoinURI(info); - ui->btnSaveAs->setEnabled(false); - QString html; - html += "<html><font face='verdana, arial, helvetica, sans-serif'>"; - html += "<b>"+tr("Payment information")+"</b><br>"; - html += "<b>"+tr("URI")+"</b>: "; - html += "<a href=\""+uri+"\">" + GUIUtil::HtmlEscape(uri) + "</a><br>"; - html += "<b>"+tr("Address")+"</b>: " + GUIUtil::HtmlEscape(info.address) + "<br>"; - if(info.amount) - html += "<b>"+tr("Amount")+"</b>: " + BitcoinUnits::formatHtmlWithUnit(model->getOptionsModel()->getDisplayUnit(), info.amount) + "<br>"; - if(!info.label.isEmpty()) - html += "<b>"+tr("Label")+"</b>: " + GUIUtil::HtmlEscape(info.label) + "<br>"; - if(!info.message.isEmpty()) - html += "<b>"+tr("Message")+"</b>: " + GUIUtil::HtmlEscape(info.message) + "<br>"; - if(model->isMultiwallet()) { - html += "<b>"+tr("Wallet")+"</b>: " + GUIUtil::HtmlEscape(model->getWalletName()) + "<br>"; + ui->uri_content->setText("<a href=\"" + uri + "\">" + GUIUtil::HtmlEscape(uri) + "</a>"); + ui->address_content->setText(info.address); + + if (!info.amount) { + ui->amount_tag->hide(); + ui->amount_content->hide(); + } // Amount is set in updateDisplayUnit() slot. + updateDisplayUnit(); + + if (!info.label.isEmpty()) { + ui->label_content->setText(info.label); + } else { + ui->label_tag->hide(); + ui->label_content->hide(); } - ui->outUri->setText(html); - if (ui->lblQRCode->setQR(uri, info.address)) { - ui->btnSaveAs->setEnabled(true); + if (!info.message.isEmpty()) { + ui->message_content->setText(info.message); + } else { + ui->message_tag->hide(); + ui->message_content->hide(); } + + if (!model->getWalletName().isEmpty()) { + ui->wallet_content->setText(model->getWalletName()); + } else { + ui->wallet_tag->hide(); + ui->wallet_content->hide(); + } +} + +void ReceiveRequestDialog::updateDisplayUnit() +{ + if (!model) return; + ui->amount_content->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), info.amount)); } void ReceiveRequestDialog::on_btnCopyURI_clicked() diff --git a/src/qt/receiverequestdialog.h b/src/qt/receiverequestdialog.h index 40e3d5ffa8..846478643d 100644 --- a/src/qt/receiverequestdialog.h +++ b/src/qt/receiverequestdialog.h @@ -29,8 +29,7 @@ public: private Q_SLOTS: void on_btnCopyURI_clicked(); void on_btnCopyAddress_clicked(); - - void update(); + void updateDisplayUnit(); private: Ui::ReceiveRequestDialog *ui; diff --git a/src/qt/recentrequeststablemodel.h b/src/qt/recentrequeststablemodel.h index addf5ad0ae..c0bd3461bb 100644 --- a/src/qt/recentrequeststablemodel.h +++ b/src/qt/recentrequeststablemodel.h @@ -24,19 +24,11 @@ public: QDateTime date; SendCoinsRecipient recipient; - ADD_SERIALIZE_METHODS; - - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) { - unsigned int nDate = date.toTime_t(); - - READWRITE(this->nVersion); - READWRITE(id); - READWRITE(nDate); - READWRITE(recipient); - - if (ser_action.ForRead()) - date = QDateTime::fromTime_t(nDate); + SERIALIZE_METHODS(RecentRequestEntry, obj) { + unsigned int date_timet; + SER_WRITE(obj, date_timet = obj.date.toTime_t()); + READWRITE(obj.nVersion, obj.id, date_timet, obj.recipient); + SER_READ(obj, obj.date = QDateTime::fromTime_t(date_timet)); } }; diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 66f1c8fd9c..0f89d4e6fe 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -28,6 +28,7 @@ #include <wallet/wallet.h> #endif +#include <QFont> #include <QKeyEvent> #include <QMenu> #include <QMessageBox> @@ -494,7 +495,7 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty ui->detailWidget->hide(); ui->peerHeading->setText(tr("Select a peer to view detailed information.")); - consoleFontSize = settings.value(fontSizeSettingsKey, QFontInfo(QFont()).pointSize()).toInt(); + consoleFontSize = settings.value(fontSizeSettingsKey, QFont().pointSize()).toInt(); clear(); GUIUtil::handleCloseWindowShortcut(this); diff --git a/src/qt/sendcoinsrecipient.h b/src/qt/sendcoinsrecipient.h index 12279fab64..6619faf417 100644 --- a/src/qt/sendcoinsrecipient.h +++ b/src/qt/sendcoinsrecipient.h @@ -44,30 +44,21 @@ public: static const int CURRENT_VERSION = 1; int nVersion; - ADD_SERIALIZE_METHODS; + SERIALIZE_METHODS(SendCoinsRecipient, obj) + { + std::string address_str, label_str, message_str, auth_merchant_str; - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) { - std::string sAddress = address.toStdString(); - std::string sLabel = label.toStdString(); - std::string sMessage = message.toStdString(); - std::string sAuthenticatedMerchant = authenticatedMerchant.toStdString(); + SER_WRITE(obj, address_str = obj.address.toStdString()); + SER_WRITE(obj, label_str = obj.label.toStdString()); + SER_WRITE(obj, message_str = obj.message.toStdString()); + SER_WRITE(obj, auth_merchant_str = obj.authenticatedMerchant.toStdString()); - READWRITE(this->nVersion); - READWRITE(sAddress); - READWRITE(sLabel); - READWRITE(amount); - READWRITE(sMessage); - READWRITE(sPaymentRequest); - READWRITE(sAuthenticatedMerchant); + READWRITE(obj.nVersion, address_str, label_str, obj.amount, message_str, obj.sPaymentRequest, auth_merchant_str); - if (ser_action.ForRead()) - { - address = QString::fromStdString(sAddress); - label = QString::fromStdString(sLabel); - message = QString::fromStdString(sMessage); - authenticatedMerchant = QString::fromStdString(sAuthenticatedMerchant); - } + SER_READ(obj, obj.address = QString::fromStdString(address_str)); + SER_READ(obj, obj.label = QString::fromStdString(label_str)); + SER_READ(obj, obj.message = QString::fromStdString(message_str)); + SER_READ(obj, obj.authenticatedMerchant = QString::fromStdString(auth_merchant_str)); } }; diff --git a/src/qt/signverifymessagedialog.cpp b/src/qt/signverifymessagedialog.cpp index 5cfbb67449..4835dd7954 100644 --- a/src/qt/signverifymessagedialog.cpp +++ b/src/qt/signverifymessagedialog.cpp @@ -91,6 +91,7 @@ void SignVerifyMessageDialog::on_addressBookButton_SM_clicked() { if (model && model->getAddressTableModel()) { + model->refresh(/* pk_hash_only */ true); AddressBookPage dlg(platformStyle, AddressBookPage::ForSelection, AddressBookPage::ReceivingTab, this); dlg.setModel(model->getAddressTableModel()); if (dlg.exec()) diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 2ee9ae0d86..8da0250e57 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -201,7 +201,7 @@ void TestGUI(interfaces::Node& node) OverviewPage overviewPage(platformStyle.get()); overviewPage.setWalletModel(&walletModel); QLabel* balanceLabel = overviewPage.findChild<QLabel*>("labelBalance"); - QString balanceText = balanceLabel->text(); + QString balanceText = balanceLabel->text().trimmed(); int unit = walletModel.getOptionsModel()->getDisplayUnit(); CAmount balance = walletModel.wallet().getBalance(); QString balanceComparison = BitcoinUnits::formatWithUnit(unit, balance, false, BitcoinUnits::separatorAlways); @@ -229,15 +229,23 @@ void TestGUI(interfaces::Node& node) for (QWidget* widget : QApplication::topLevelWidgets()) { if (widget->inherits("ReceiveRequestDialog")) { ReceiveRequestDialog* receiveRequestDialog = qobject_cast<ReceiveRequestDialog*>(widget); - QTextEdit* rlist = receiveRequestDialog->QObject::findChild<QTextEdit*>("outUri"); - QString paymentText = rlist->toPlainText(); - QStringList paymentTextList = paymentText.split('\n'); - QCOMPARE(paymentTextList.at(0), QString("Payment information")); - QVERIFY(paymentTextList.at(1).indexOf(QString("URI: bitcoin:")) != -1); - QVERIFY(paymentTextList.at(2).indexOf(QString("Address:")) != -1); - QCOMPARE(paymentTextList.at(3), QString("Amount: 0.00000001 ") + QString::fromStdString(CURRENCY_UNIT)); - QCOMPARE(paymentTextList.at(4), QString("Label: TEST_LABEL_1")); - QCOMPARE(paymentTextList.at(5), QString("Message: TEST_MESSAGE_1")); + QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("payment_header")->text(), QString("Payment information")); + QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("uri_tag")->text(), QString("URI:")); + QString uri = receiveRequestDialog->QObject::findChild<QLabel*>("uri_content")->text(); + QCOMPARE(uri.count("bitcoin:"), 2); + QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("address_tag")->text(), QString("Address:")); + + QCOMPARE(uri.count("amount=0.00000001"), 2); + QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("amount_tag")->text(), QString("Amount:")); + QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("amount_content")->text(), QString("0.00000001 ") + QString::fromStdString(CURRENCY_UNIT)); + + QCOMPARE(uri.count("label=TEST_LABEL_1"), 2); + QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("label_tag")->text(), QString("Label:")); + QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("label_content")->text(), QString("TEST_LABEL_1")); + + QCOMPARE(uri.count("message=TEST_MESSAGE_1"), 2); + QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("message_tag")->text(), QString("Message:")); + QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("message_content")->text(), QString("TEST_MESSAGE_1")); } } diff --git a/src/qt/trafficgraphwidget.cpp b/src/qt/trafficgraphwidget.cpp index 757648f485..6428fc4daf 100644 --- a/src/qt/trafficgraphwidget.cpp +++ b/src/qt/trafficgraphwidget.cpp @@ -7,6 +7,7 @@ #include <qt/clientmodel.h> #include <QPainter> +#include <QPainterPath> #include <QColor> #include <QTimer> diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index a32d218fc9..01dff8069c 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -162,7 +162,7 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const interface return parts; } -void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, int numBlocks, int64_t block_time) +void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, const uint256& block_hash, int numBlocks, int64_t block_time) { // Determine transaction status @@ -174,7 +174,7 @@ void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, int idx); status.countsForBalance = wtx.is_trusted && !(wtx.blocks_to_maturity > 0); status.depth = wtx.depth_in_main_chain; - status.cur_num_blocks = numBlocks; + status.m_cur_block_hash = block_hash; const bool up_to_date = ((int64_t)QDateTime::currentMSecsSinceEpoch() / 1000 - block_time < MAX_BLOCK_TIME_GAP); if (up_to_date && !wtx.is_final) { @@ -233,9 +233,9 @@ void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, int status.needsUpdate = false; } -bool TransactionRecord::statusUpdateNeeded(int numBlocks) const +bool TransactionRecord::statusUpdateNeeded(const uint256& block_hash) const { - return status.cur_num_blocks != numBlocks || status.needsUpdate; + return status.m_cur_block_hash != block_hash || status.needsUpdate; } QString TransactionRecord::getTxHash() const diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index 3f64cefd09..c983c527c0 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -23,9 +23,8 @@ struct WalletTxStatus; class TransactionStatus { public: - TransactionStatus(): - countsForBalance(false), sortKey(""), - matures_in(0), status(Unconfirmed), depth(0), open_for(0), cur_num_blocks(-1) + TransactionStatus() : countsForBalance(false), sortKey(""), + matures_in(0), status(Unconfirmed), depth(0), open_for(0) { } enum Status { @@ -61,8 +60,8 @@ public: finalization */ /**@}*/ - /** Current number of blocks (to know whether cached status is still valid) */ - int cur_num_blocks; + /** Current block hash (to know whether cached status is still valid) */ + uint256 m_cur_block_hash{}; bool needsUpdate; }; @@ -138,11 +137,11 @@ public: /** Update status from core wallet tx. */ - void updateStatus(const interfaces::WalletTxStatus& wtx, int numBlocks, int64_t block_time); + void updateStatus(const interfaces::WalletTxStatus& wtx, const uint256& block_hash, int numBlocks, int64_t block_time); /** Return whether a status update is needed. */ - bool statusUpdateNeeded(int numBlocks) const; + bool statusUpdateNeeded(const uint256& block_hash) const; }; #endif // BITCOIN_QT_TRANSACTIONRECORD_H diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 7a15503228..04eb1ae706 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -176,7 +176,7 @@ public: return cachedWallet.size(); } - TransactionRecord *index(interfaces::Wallet& wallet, const int cur_num_blocks, const int idx) + TransactionRecord* index(interfaces::Wallet& wallet, const uint256& cur_block_hash, const int idx) { if(idx >= 0 && idx < cachedWallet.size()) { @@ -192,8 +192,8 @@ public: interfaces::WalletTxStatus wtx; int numBlocks; int64_t block_time; - if (rec->statusUpdateNeeded(cur_num_blocks) && wallet.tryGetTxStatus(rec->hash, wtx, numBlocks, block_time)) { - rec->updateStatus(wtx, numBlocks, block_time); + if (rec->statusUpdateNeeded(cur_block_hash) && wallet.tryGetTxStatus(rec->hash, wtx, numBlocks, block_time)) { + rec->updateStatus(wtx, cur_block_hash, numBlocks, block_time); } return rec; } @@ -618,7 +618,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const return details; } case ConfirmedRole: - return rec->status.countsForBalance; + return rec->status.status == TransactionStatus::Status::Confirming || rec->status.status == TransactionStatus::Status::Confirmed; case FormattedAmountRole: // Used for copy/export, so don't include separators return formatTxAmount(rec, false, BitcoinUnits::separatorNever); @@ -664,7 +664,7 @@ QVariant TransactionTableModel::headerData(int section, Qt::Orientation orientat QModelIndex TransactionTableModel::index(int row, int column, const QModelIndex &parent) const { Q_UNUSED(parent); - TransactionRecord *data = priv->index(walletModel->wallet(), walletModel->getNumBlocks(), row); + TransactionRecord* data = priv->index(walletModel->wallet(), walletModel->clientModel().getBestBlockHash(), row); if(data) { return createIndex(row, column, data); diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 3c638fb358..3df81807f0 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -8,6 +8,7 @@ #include <qt/bitcoinunits.h> #include <qt/csvmodelwriter.h> #include <qt/editaddressdialog.h> +#include <qt/guiutil.h> #include <qt/optionsmodel.h> #include <qt/platformstyle.h> #include <qt/transactiondescdialog.h> @@ -36,8 +37,7 @@ #include <QVBoxLayout> TransactionView::TransactionView(const PlatformStyle *platformStyle, QWidget *parent) : - QWidget(parent), model(nullptr), transactionProxyModel(nullptr), - transactionView(nullptr), abandonAction(nullptr), bumpFeeAction(nullptr), columnResizingFixer(nullptr) + QWidget(parent) { // Build filter row setContentsMargins(0,0,0,0); @@ -152,8 +152,8 @@ TransactionView::TransactionView(const PlatformStyle *platformStyle, QWidget *pa abandonAction = new QAction(tr("Abandon transaction"), this); bumpFeeAction = new QAction(tr("Increase transaction fee"), this); bumpFeeAction->setObjectName("bumpFeeAction"); - QAction *copyAddressAction = new QAction(tr("Copy address"), this); - QAction *copyLabelAction = new QAction(tr("Copy label"), this); + copyAddressAction = new QAction(tr("Copy address"), this); + copyLabelAction = new QAction(tr("Copy label"), this); QAction *copyAmountAction = new QAction(tr("Copy amount"), this); QAction *copyTxIDAction = new QAction(tr("Copy transaction ID"), this); QAction *copyTxHexAction = new QAction(tr("Copy raw transaction"), this); @@ -395,10 +395,11 @@ void TransactionView::contextualMenu(const QPoint &point) hash.SetHex(selection.at(0).data(TransactionTableModel::TxHashRole).toString().toStdString()); abandonAction->setEnabled(model->wallet().transactionCanBeAbandoned(hash)); bumpFeeAction->setEnabled(model->wallet().transactionCanBeBumped(hash)); + copyAddressAction->setEnabled(GUIUtil::hasEntryData(transactionView, 0, TransactionTableModel::AddressRole)); + copyLabelAction->setEnabled(GUIUtil::hasEntryData(transactionView, 0, TransactionTableModel::LabelRole)); - if(index.isValid()) - { - contextMenu->popup(transactionView->viewport()->mapToGlobal(point)); + if (index.isValid()) { + GUIUtil::PopupMenu(contextMenu, transactionView->viewport()->mapToGlobal(point)); } } diff --git a/src/qt/transactionview.h b/src/qt/transactionview.h index 268e3751b3..9ce7f4ad97 100644 --- a/src/qt/transactionview.h +++ b/src/qt/transactionview.h @@ -60,9 +60,9 @@ public: }; private: - WalletModel *model; - TransactionFilterProxy *transactionProxyModel; - QTableView *transactionView; + WalletModel *model{nullptr}; + TransactionFilterProxy *transactionProxyModel{nullptr}; + QTableView *transactionView{nullptr}; QComboBox *dateWidget; QComboBox *typeWidget; @@ -75,12 +75,14 @@ private: QFrame *dateRangeWidget; QDateTimeEdit *dateFrom; QDateTimeEdit *dateTo; - QAction *abandonAction; - QAction *bumpFeeAction; + QAction *abandonAction{nullptr}; + QAction *bumpFeeAction{nullptr}; + QAction *copyAddressAction{nullptr}; + QAction *copyLabelAction{nullptr}; QWidget *createDateRangeWidget(); - GUIUtil::TableViewLastColumnResizingFixer *columnResizingFixer; + GUIUtil::TableViewLastColumnResizingFixer *columnResizingFixer{nullptr}; virtual void resizeEvent(QResizeEvent* event) override; diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index 02a9583ae9..5e68ee4f93 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -53,6 +53,7 @@ bool WalletFrame::addWallet(WalletModel *walletModel) walletView->setClientModel(clientModel); walletView->setWalletModel(walletModel); walletView->showOutOfSyncWarning(bOutOfSync); + walletView->setPrivacy(gui->isPrivacyModeActivated()); WalletView* current_wallet_view = currentWalletView(); if (current_wallet_view) { @@ -73,6 +74,7 @@ bool WalletFrame::addWallet(WalletModel *walletModel) connect(walletView, &WalletView::encryptionStatusChanged, gui, &BitcoinGUI::updateWalletStatus); connect(walletView, &WalletView::incomingTransaction, gui, &BitcoinGUI::incomingTransaction); connect(walletView, &WalletView::hdEnabledStatusChanged, gui, &BitcoinGUI::updateWalletStatus); + connect(gui, &BitcoinGUI::setPrivacy, walletView, &WalletView::setPrivacy); return true; } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index b1e61e03b3..671b5e1ce6 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -46,7 +46,6 @@ WalletModel::WalletModel(std::unique_ptr<interfaces::Wallet> wallet, ClientModel transactionTableModel(nullptr), recentRequestsTableModel(nullptr), cachedEncryptionStatus(Unencrypted), - cachedNumBlocks(0), timer(new QTimer(this)) { fHaveWatchOnly = m_wallet->haveWatchOnly(); @@ -88,24 +87,23 @@ void WalletModel::pollBalanceChanged() { // Avoid recomputing wallet balances unless a TransactionChanged or // BlockTip notification was received. - if (!fForceCheckBalanceChanged && cachedNumBlocks == m_client_model->getNumBlocks()) return; + if (!fForceCheckBalanceChanged && m_cached_last_update_tip == m_client_model->getBestBlockHash()) return; // Try to get balances and return early if locks can't be acquired. This // avoids the GUI from getting stuck on periodical polls if the core is // holding the locks for a longer time - for example, during a wallet // rescan. interfaces::WalletBalances new_balances; - int numBlocks = -1; - if (!m_wallet->tryGetBalances(new_balances, numBlocks)) { + uint256 block_hash; + if (!m_wallet->tryGetBalances(new_balances, block_hash)) { return; } - if(fForceCheckBalanceChanged || numBlocks != cachedNumBlocks) - { + if (fForceCheckBalanceChanged || block_hash != m_cached_last_update_tip) { fForceCheckBalanceChanged = false; // Balance and number of transactions might have changed - cachedNumBlocks = numBlocks; + m_cached_last_update_tip = block_hash; checkBalanceChanged(new_balances); if(transactionTableModel) @@ -585,3 +583,8 @@ bool WalletModel::isMultiwallet() { return m_node.getWallets().size() > 1; } + +void WalletModel::refresh(bool pk_hash_only) +{ + addressTableModel = new AddressTableModel(this, pk_hash_only); +} diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 23232ec66b..38e8a14556 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -144,8 +144,8 @@ public: interfaces::Node& node() const { return m_node; } interfaces::Wallet& wallet() const { return *m_wallet; } + ClientModel& clientModel() const { return *m_client_model; } void setClientModel(ClientModel* client_model); - int getNumBlocks() const { return cachedNumBlocks; } QString getWalletName() const; QString getDisplayName() const; @@ -153,6 +153,8 @@ public: bool isMultiwallet(); AddressTableModel* getAddressTableModel() const { return addressTableModel; } + + void refresh(bool pk_hash_only = false); private: std::unique_ptr<interfaces::Wallet> m_wallet; std::unique_ptr<interfaces::Handler> m_handler_unload; @@ -179,9 +181,11 @@ private: // Cache some values to be able to detect changes interfaces::WalletBalances m_cached_balances; EncryptionStatus cachedEncryptionStatus; - int cachedNumBlocks; QTimer* timer; + // Block hash denoting when the last balance update was done. + uint256 m_cached_last_update_tip{}; + void subscribeToCoreSignals(); void unsubscribeFromCoreSignals(); void checkBalanceChanged(const interfaces::WalletBalances& new_balances); diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index 09c63fb948..861d1c5f4a 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -85,6 +85,8 @@ WalletView::WalletView(const PlatformStyle *_platformStyle, QWidget *parent): connect(sendCoinsPage, &SendCoinsDialog::message, this, &WalletView::message); // Pass through messages from transactionView connect(transactionView, &TransactionView::message, this, &WalletView::message); + + connect(this, &WalletView::setPrivacy, overviewPage, &OverviewPage::setPrivacy); } WalletView::~WalletView() diff --git a/src/qt/walletview.h b/src/qt/walletview.h index 11f894e7f8..fd09456baa 100644 --- a/src/qt/walletview.h +++ b/src/qt/walletview.h @@ -115,6 +115,7 @@ public Q_SLOTS: void requestedSyncWarningInfo(); Q_SIGNALS: + void setPrivacy(bool privacy); void transactionClicked(); void coinsSent(); /** Fired when a message should be reported to the user */ diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 717ca308ac..b0936cef5a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -52,7 +52,7 @@ struct CUpdatedBlock static Mutex cs_blockchange; static std::condition_variable cond_blockchange; -static CUpdatedBlock latestblock; +static CUpdatedBlock latestblock GUARDED_BY(cs_blockchange); NodeContext& EnsureNodeContext(const util::Ref& context) { @@ -223,7 +223,7 @@ static UniValue getbestblockhash(const JSONRPCRequest& request) void RPCNotifyBlockChange(const CBlockIndex* pindex) { if(pindex) { - std::lock_guard<std::mutex> lock(cs_blockchange); + LOCK(cs_blockchange); latestblock.hash = pindex->GetBlockHash(); latestblock.height = pindex->nHeight; } @@ -258,9 +258,9 @@ static UniValue waitfornewblock(const JSONRPCRequest& request) WAIT_LOCK(cs_blockchange, lock); block = latestblock; if(timeout) - cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&block]{return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); }); + cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&block]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); }); else - cond_blockchange.wait(lock, [&block]{return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); }); + cond_blockchange.wait(lock, [&block]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); }); block = latestblock; } UniValue ret(UniValue::VOBJ); @@ -300,9 +300,9 @@ static UniValue waitforblock(const JSONRPCRequest& request) { WAIT_LOCK(cs_blockchange, lock); if(timeout) - cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&hash]{return latestblock.hash == hash || !IsRPCRunning();}); + cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&hash]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.hash == hash || !IsRPCRunning();}); else - cond_blockchange.wait(lock, [&hash]{return latestblock.hash == hash || !IsRPCRunning(); }); + cond_blockchange.wait(lock, [&hash]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.hash == hash || !IsRPCRunning(); }); block = latestblock; } @@ -344,9 +344,9 @@ static UniValue waitforblockheight(const JSONRPCRequest& request) { WAIT_LOCK(cs_blockchange, lock); if(timeout) - cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&height]{return latestblock.height >= height || !IsRPCRunning();}); + cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&height]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height >= height || !IsRPCRunning();}); else - cond_blockchange.wait(lock, [&height]{return latestblock.height >= height || !IsRPCRunning(); }); + cond_blockchange.wait(lock, [&height]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height >= height || !IsRPCRunning(); }); block = latestblock; } UniValue ret(UniValue::VOBJ); @@ -414,7 +414,7 @@ static std::vector<RPCResult> MempoolEntryDescription() { return { RPCResult{RPCResult::Type::ARR, "spentby", "unconfirmed transactions spending outputs from this transaction", {RPCResult{RPCResult::Type::STR_HEX, "transactionid", "child transaction id"}}}, RPCResult{RPCResult::Type::BOOL, "bip125-replaceable", "Whether this transaction could be replaced due to BIP125 (replace-by-fee)"}, - RPCResult{RPCResult::Type::BOOL, "unbroadcast", "Whether this transaction is currently unbroadcast (initial broadcast not yet confirmed)"}, + RPCResult{RPCResult::Type::BOOL, "unbroadcast", "Whether this transaction is currently unbroadcast (initial broadcast not yet acknowledged by any peers)"}, };} static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPoolEntry& e) EXCLUSIVE_LOCKS_REQUIRED(pool.cs) @@ -996,7 +996,7 @@ static UniValue gettxoutsetinfo(const JSONRPCRequest& request) ::ChainstateActive().ForceFlushStateToDisk(); CCoinsView* coins_view = WITH_LOCK(cs_main, return &ChainstateActive().CoinsDB()); - if (GetUTXOStats(coins_view, stats)) { + if (GetUTXOStats(coins_view, stats, RpcInterruptionPoint)) { ret.pushKV("height", (int64_t)stats.nHeight); ret.pushKV("bestblock", stats.hashBlock.GetHex()); ret.pushKV("transactions", (int64_t)stats.nTransactions); @@ -1974,6 +1974,7 @@ bool FindScriptPubKey(std::atomic<int>& scan_progress, const std::atomic<bool>& Coin coin; if (!cursor->GetKey(key) || !cursor->GetValue(coin)) return false; if (++count % 8192 == 0) { + RpcInterruptionPoint(); if (should_abort) { // allow to abort the scan via the abort reference return false; @@ -1994,7 +1995,6 @@ bool FindScriptPubKey(std::atomic<int>& scan_progress, const std::atomic<bool>& } /** RAII object to prevent concurrency issue when scanning the txout set */ -static std::mutex g_utxosetscan; static std::atomic<int> g_scan_progress; static std::atomic<bool> g_scan_in_progress; static std::atomic<bool> g_should_abort_scan; @@ -2007,18 +2007,15 @@ public: bool reserve() { CHECK_NONFATAL(!m_could_reserve); - std::lock_guard<std::mutex> lock(g_utxosetscan); - if (g_scan_in_progress) { + if (g_scan_in_progress.exchange(true)) { return false; } - g_scan_in_progress = true; m_could_reserve = true; return true; } ~CoinsViewScanReserver() { if (m_could_reserve) { - std::lock_guard<std::mutex> lock(g_utxosetscan); g_scan_in_progress = false; } } @@ -2317,7 +2314,7 @@ UniValue dumptxoutset(const JSONRPCRequest& request) ::ChainstateActive().ForceFlushStateToDisk(); - if (!GetUTXOStats(&::ChainstateActive().CoinsDB(), stats)) { + if (!GetUTXOStats(&::ChainstateActive().CoinsDB(), stats, RpcInterruptionPoint)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to read UTXO set"); } @@ -2335,9 +2332,7 @@ UniValue dumptxoutset(const JSONRPCRequest& request) unsigned int iter{0}; while (pcursor->Valid()) { - if (iter % 5000 == 0 && !IsRPCRunning()) { - throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, "Shutting down"); - } + if (iter % 5000 == 0) RpcInterruptionPoint(); ++iter; if (pcursor->GetKey(key) && pcursor->GetValue(coin)) { afile << key; diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index 56cac6661e..7fef45f50e 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -130,20 +130,20 @@ void DeleteAuthCookie() } } -std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue &in, size_t num) +std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue& in) { if (!in.isArray()) { throw std::runtime_error("Batch must be an array"); } + const size_t num {in.size()}; std::vector<UniValue> batch(num); - for (size_t i=0; i<in.size(); ++i) { - const UniValue &rec = in[i]; + for (const UniValue& rec : in.getValues()) { if (!rec.isObject()) { - throw std::runtime_error("Batch member must be object"); + throw std::runtime_error("Batch member must be an object"); } size_t id = rec["id"].get_int(); if (id >= num) { - throw std::runtime_error("Batch member id larger than size"); + throw std::runtime_error("Batch member id is larger than batch size"); } batch[id] = rec; } diff --git a/src/rpc/request.h b/src/rpc/request.h index 0a15b48de4..02ec5393a7 100644 --- a/src/rpc/request.h +++ b/src/rpc/request.h @@ -26,7 +26,7 @@ bool GetAuthCookie(std::string *cookie_out); /** Delete RPC authentication cookie from disk */ void DeleteAuthCookie(); /** Parse JSON-RPC batch reply into a vector */ -std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue &in, size_t num); +std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue& in); class JSONRPCRequest { diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 219979f095..2a0079ac39 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -11,15 +11,19 @@ #include <util/strencodings.h> #include <util/system.h> -#include <boost/signals2/signal.hpp> #include <boost/algorithm/string/classification.hpp> #include <boost/algorithm/string/split.hpp> +#include <boost/signals2/signal.hpp> +#include <cassert> #include <memory> // for unique_ptr +#include <mutex> #include <unordered_map> static RecursiveMutex cs_rpcWarmup; static std::atomic<bool> g_rpc_running{false}; +static std::once_flag g_rpc_interrupt_flag; +static std::once_flag g_rpc_stop_flag; static bool fRPCInWarmup GUARDED_BY(cs_rpcWarmup) = true; static std::string rpcWarmupStatus GUARDED_BY(cs_rpcWarmup) = "RPC server started"; /* Timer-creating functions */ @@ -291,17 +295,24 @@ void StartRPC() void InterruptRPC() { - LogPrint(BCLog::RPC, "Interrupting RPC\n"); - // Interrupt e.g. running longpolls - g_rpc_running = false; + // This function could be called twice if the GUI has been started with -server=1. + std::call_once(g_rpc_interrupt_flag, []() { + LogPrint(BCLog::RPC, "Interrupting RPC\n"); + // Interrupt e.g. running longpolls + g_rpc_running = false; + }); } void StopRPC() { - LogPrint(BCLog::RPC, "Stopping RPC\n"); - WITH_LOCK(g_deadline_timers_mutex, deadlineTimers.clear()); - DeleteAuthCookie(); - g_rpcSignals.Stopped(); + // This function could be called twice if the GUI has been started with -server=1. + assert(!g_rpc_running); + std::call_once(g_rpc_stop_flag, []() { + LogPrint(BCLog::RPC, "Stopping RPC\n"); + WITH_LOCK(g_deadline_timers_mutex, deadlineTimers.clear()); + DeleteAuthCookie(); + g_rpcSignals.Stopped(); + }); } bool IsRPCRunning() @@ -309,6 +320,11 @@ bool IsRPCRunning() return g_rpc_running; } +void RpcInterruptionPoint() +{ + if (!IsRPCRunning()) throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, "Shutting down"); +} + void SetRPCWarmupStatus(const std::string& newStatus) { LOCK(cs_rpcWarmup); diff --git a/src/rpc/server.h b/src/rpc/server.h index c91bf1f613..d7a04ff6e8 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -9,10 +9,10 @@ #include <amount.h> #include <rpc/request.h> +#include <functional> #include <map> #include <stdint.h> #include <string> -#include <functional> #include <univalue.h> @@ -29,6 +29,9 @@ namespace RPCServer /** Query whether RPC is running */ bool IsRPCRunning(); +/** Throw JSONRPCError if RPC is not running */ +void RpcInterruptionPoint(); + /** * Set the RPC warmup status. When this is done, all RPC calls will error out * immediately with RPC_IN_WARMUP. diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 860fa198d5..39bf05fbbd 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -843,16 +843,9 @@ UniValue GetServicesNames(ServiceFlags services) { UniValue servicesNames(UniValue::VARR); - if (services & NODE_NETWORK) - servicesNames.push_back("NETWORK"); - if (services & NODE_GETUTXO) - servicesNames.push_back("GETUTXO"); - if (services & NODE_BLOOM) - servicesNames.push_back("BLOOM"); - if (services & NODE_WITNESS) - servicesNames.push_back("WITNESS"); - if (services & NODE_NETWORK_LIMITED) - servicesNames.push_back("NETWORK_LIMITED"); + for (const auto& flag : serviceFlagsToStr(services)) { + servicesNames.push_back(flag); + } return servicesNames; } diff --git a/src/script/script.cpp b/src/script/script.cpp index ae0de1d24e..92c6fe7785 100644 --- a/src/script/script.cpp +++ b/src/script/script.cpp @@ -7,7 +7,9 @@ #include <util/strencodings.h> -const char* GetOpName(opcodetype opcode) +#include <string> + +std::string GetOpName(opcodetype opcode) { switch (opcode) { diff --git a/src/script/script.h b/src/script/script.h index b61581767f..c1f2b66921 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -193,7 +193,7 @@ enum opcodetype // Maximum value that an opcode can be static const unsigned int MAX_OPCODE = OP_NOP10; -const char* GetOpName(opcodetype opcode); +std::string GetOpName(opcodetype opcode); class scriptnum_error : public std::runtime_error { diff --git a/src/script/script_error.cpp b/src/script/script_error.cpp index 57e8fee539..69e14803f1 100644 --- a/src/script/script_error.cpp +++ b/src/script/script_error.cpp @@ -5,7 +5,9 @@ #include <script/script_error.h> -const char* ScriptErrorString(const ScriptError serror) +#include <string> + +std::string ScriptErrorString(const ScriptError serror) { switch (serror) { diff --git a/src/script/script_error.h b/src/script/script_error.h index 400f63ff0f..2978c147e1 100644 --- a/src/script/script_error.h +++ b/src/script/script_error.h @@ -6,6 +6,8 @@ #ifndef BITCOIN_SCRIPT_SCRIPT_ERROR_H #define BITCOIN_SCRIPT_SCRIPT_ERROR_H +#include <string> + typedef enum ScriptError_t { SCRIPT_ERR_OK = 0, @@ -73,6 +75,6 @@ typedef enum ScriptError_t #define SCRIPT_ERR_LAST SCRIPT_ERR_ERROR_COUNT -const char* ScriptErrorString(const ScriptError error); +std::string ScriptErrorString(const ScriptError error); #endif // BITCOIN_SCRIPT_SCRIPT_ERROR_H diff --git a/src/script/standard.cpp b/src/script/standard.cpp index 7d89a336fb..c90c2c24a0 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -9,6 +9,8 @@ #include <pubkey.h> #include <script/script.h> +#include <string> + typedef std::vector<unsigned char> valtype; bool fAcceptDatacarrier = DEFAULT_ACCEPT_DATACARRIER; @@ -25,7 +27,7 @@ WitnessV0ScriptHash::WitnessV0ScriptHash(const CScript& in) CSHA256().Write(in.data(), in.size()).Finalize(begin()); } -const char* GetTxnOutputType(txnouttype t) +std::string GetTxnOutputType(txnouttype t) { switch (t) { @@ -39,7 +41,7 @@ const char* GetTxnOutputType(txnouttype t) case TX_WITNESS_V0_SCRIPTHASH: return "witness_v0_scripthash"; case TX_WITNESS_UNKNOWN: return "witness_unknown"; } - return nullptr; + assert(false); } static bool MatchPayToPubkey(const CScript& script, valtype& pubkey) diff --git a/src/script/standard.h b/src/script/standard.h index 49a45f3eba..2929425670 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -11,6 +11,8 @@ #include <boost/variant.hpp> +#include <string> + static const bool DEFAULT_ACCEPT_DATACARRIER = true; @@ -44,8 +46,7 @@ extern unsigned nMaxDatacarrierBytes; /** * Mandatory script verification flags that all new blocks must comply with for * them to be valid. (but old blocks may not comply with) Currently just P2SH, - * but in the future other flags may be added, such as a soft-fork to enforce - * strict DER encoding. + * but in the future other flags may be added. * * Failing one of these tests may trigger a DoS ban - see CheckInputScripts() for * details. @@ -146,7 +147,7 @@ typedef boost::variant<CNoDestination, PKHash, ScriptHash, WitnessV0ScriptHash, bool IsValidDestination(const CTxDestination& dest); /** Get the name of a txnouttype as a C string, or nullptr if unknown. */ -const char* GetTxnOutputType(txnouttype t); +std::string GetTxnOutputType(txnouttype t); /** * Parse a scriptPubKey and identify script type for standard scripts. If diff --git a/src/serialize.h b/src/serialize.h index af75c50ff9..71c2cfa164 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -43,26 +43,6 @@ static const unsigned int MAX_VECTOR_ALLOCATE = 5000000; struct deserialize_type {}; constexpr deserialize_type deserialize {}; -/** - * Used to bypass the rule against non-const reference to temporary - * where it makes sense with wrappers. - */ -template<typename T> -inline T& REF(const T& val) -{ - return const_cast<T&>(val); -} - -/** - * Used to acquire a non-const pointer "this" to generate bodies - * of const serialization operations from a template - */ -template<typename T> -inline T* NCONST_PTR(const T* val) -{ - return const_cast<T*>(val); -} - //! Safely convert odd char pointer types to standard ones. inline char* CharCast(char* c) { return c; } inline char* CharCast(unsigned char* c) { return (char*)c; } @@ -194,22 +174,6 @@ template<typename X> const X& ReadWriteAsHelper(const X& x) { return x; } #define SER_WRITE(obj, code) ::SerWrite(s, ser_action, obj, [&](Stream& s, const Type& obj) { code; }) /** - * Implement three methods for serializable objects. These are actually wrappers over - * "SerializationOp" template, which implements the body of each class' serialization - * code. Adding "ADD_SERIALIZE_METHODS" in the body of the class causes these wrappers to be - * added as members. - */ -#define ADD_SERIALIZE_METHODS \ - template<typename Stream> \ - void Serialize(Stream& s) const { \ - NCONST_PTR(this)->SerializationOp(s, CSerActionSerialize()); \ - } \ - template<typename Stream> \ - void Unserialize(Stream& s) { \ - SerializationOp(s, CSerActionUnserialize()); \ - } - -/** * Implement the Ser and Unser methods needed for implementing a formatter (see Using below). * * Both Ser and Unser are delegated to a single static method SerializationOps, which is polymorphic @@ -503,7 +467,7 @@ static inline Wrapper<Formatter, T&> Using(T&& t) { return Wrapper<Formatter, T& #define VARINT_MODE(obj, mode) Using<VarIntFormatter<mode>>(obj) #define VARINT(obj) Using<VarIntFormatter<VarIntMode::DEFAULT>>(obj) #define COMPACTSIZE(obj) Using<CompactSizeFormatter>(obj) -#define LIMITED_STRING(obj,n) LimitedString< n >(REF(obj)) +#define LIMITED_STRING(obj,n) Using<LimitedStringFormatter<n>>(obj) /** Serialization wrapper class for integers in VarInt format. */ template<VarIntMode Mode> @@ -588,31 +552,23 @@ struct CompactSizeFormatter }; template<size_t Limit> -class LimitedString +struct LimitedStringFormatter { -protected: - std::string& string; -public: - explicit LimitedString(std::string& _string) : string(_string) {} - template<typename Stream> - void Unserialize(Stream& s) + void Unser(Stream& s, std::string& v) { size_t size = ReadCompactSize(s); if (size > Limit) { throw std::ios_base::failure("String length limit exceeded"); } - string.resize(size); - if (size != 0) - s.read((char*)string.data(), size); + v.resize(size); + if (size != 0) s.read((char*)v.data(), size); } template<typename Stream> - void Serialize(Stream& s) const + void Ser(Stream& s, const std::string& v) { - WriteCompactSize(s, string.size()); - if (!string.empty()) - s.write((char*)string.data(), string.size()); + s << v; } }; @@ -1012,7 +968,7 @@ void Unserialize(Stream& is, std::shared_ptr<const T>& p) /** - * Support for ADD_SERIALIZE_METHODS and READWRITE macro + * Support for SERIALIZE_METHODS and READWRITE macro. */ struct CSerActionSerialize { diff --git a/src/sync.cpp b/src/sync.cpp index b86c57e498..9abdedbed4 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -7,15 +7,19 @@ #endif #include <sync.h> -#include <tinyformat.h> #include <logging.h> +#include <tinyformat.h> #include <util/strencodings.h> #include <util/threadnames.h> #include <map> #include <set> #include <system_error> +#include <thread> +#include <unordered_map> +#include <utility> +#include <vector> #ifdef DEBUG_LOCKCONTENTION #if !defined(HAVE_THREAD_LOCAL) @@ -73,35 +77,35 @@ private: int sourceLine; }; -typedef std::vector<std::pair<void*, CLockLocation> > LockStack; -typedef std::map<std::pair<void*, void*>, LockStack> LockOrders; -typedef std::set<std::pair<void*, void*> > InvLockOrders; +using LockStackItem = std::pair<void*, CLockLocation>; +using LockStack = std::vector<LockStackItem>; +using LockStacks = std::unordered_map<std::thread::id, LockStack>; -struct LockData { - // Very ugly hack: as the global constructs and destructors run single - // threaded, we use this boolean to know whether LockData still exists, - // as DeleteLock can get called by global RecursiveMutex destructors - // after LockData disappears. - bool available; - LockData() : available(true) {} - ~LockData() { available = false; } +using LockPair = std::pair<void*, void*>; +using LockOrders = std::map<LockPair, LockStack>; +using InvLockOrders = std::set<LockPair>; +struct LockData { + LockStacks m_lock_stacks; LockOrders lockorders; InvLockOrders invlockorders; std::mutex dd_mutex; }; + LockData& GetLockData() { - static LockData lockdata; - return lockdata; + // This approach guarantees that the object is not destroyed until after its last use. + // The operating system automatically reclaims all the memory in a program's heap when that program exits. + // Since the ~LockData() destructor is never called, the LockData class and all + // its subclasses must have implicitly-defined destructors. + static LockData& lock_data = *new LockData(); + return lock_data; } -static thread_local LockStack g_lockstack; - -static void potential_deadlock_detected(const std::pair<void*, void*>& mismatch, const LockStack& s1, const LockStack& s2) +static void potential_deadlock_detected(const LockPair& mismatch, const LockStack& s1, const LockStack& s2) { LogPrintf("POTENTIAL DEADLOCK DETECTED\n"); LogPrintf("Previous lock order was:\n"); - for (const std::pair<void*, CLockLocation> & i : s2) { + for (const LockStackItem& i : s2) { if (i.first == mismatch.first) { LogPrintf(" (1)"); /* Continued */ } @@ -111,7 +115,7 @@ static void potential_deadlock_detected(const std::pair<void*, void*>& mismatch, LogPrintf(" %s\n", i.second.ToString()); } LogPrintf("Current lock order is:\n"); - for (const std::pair<void*, CLockLocation> & i : s1) { + for (const LockStackItem& i : s1) { if (i.first == mismatch.first) { LogPrintf(" (1)"); /* Continued */ } @@ -132,18 +136,18 @@ static void push_lock(void* c, const CLockLocation& locklocation) LockData& lockdata = GetLockData(); std::lock_guard<std::mutex> lock(lockdata.dd_mutex); - g_lockstack.push_back(std::make_pair(c, locklocation)); - - for (const std::pair<void*, CLockLocation>& i : g_lockstack) { + LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; + lock_stack.emplace_back(c, locklocation); + for (const LockStackItem& i : lock_stack) { if (i.first == c) break; - std::pair<void*, void*> p1 = std::make_pair(i.first, c); + const LockPair p1 = std::make_pair(i.first, c); if (lockdata.lockorders.count(p1)) continue; - lockdata.lockorders.emplace(p1, g_lockstack); + lockdata.lockorders.emplace(p1, lock_stack); - std::pair<void*, void*> p2 = std::make_pair(c, i.first); + const LockPair p2 = std::make_pair(c, i.first); lockdata.invlockorders.insert(p2); if (lockdata.lockorders.count(p2)) potential_deadlock_detected(p1, lockdata.lockorders[p2], lockdata.lockorders[p1]); @@ -152,7 +156,14 @@ static void push_lock(void* c, const CLockLocation& locklocation) static void pop_lock() { - g_lockstack.pop_back(); + LockData& lockdata = GetLockData(); + std::lock_guard<std::mutex> lock(lockdata.dd_mutex); + + LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; + lock_stack.pop_back(); + if (lock_stack.empty()) { + lockdata.m_lock_stacks.erase(std::this_thread::get_id()); + } } void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry) @@ -162,11 +173,17 @@ void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) { - if (!g_lockstack.empty()) { - const auto& lastlock = g_lockstack.back(); - if (lastlock.first == cs) { - lockname = lastlock.second.Name(); - return; + { + LockData& lockdata = GetLockData(); + std::lock_guard<std::mutex> lock(lockdata.dd_mutex); + + const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; + if (!lock_stack.empty()) { + const auto& lastlock = lock_stack.back(); + if (lastlock.first == cs) { + lockname = lastlock.second.Name(); + return; + } } } throw std::system_error(EPERM, std::generic_category(), strprintf("%s:%s %s was not most recent critical section locked", file, line, guardname)); @@ -179,49 +196,60 @@ void LeaveCritical() std::string LocksHeld() { + LockData& lockdata = GetLockData(); + std::lock_guard<std::mutex> lock(lockdata.dd_mutex); + + const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; std::string result; - for (const std::pair<void*, CLockLocation>& i : g_lockstack) + for (const LockStackItem& i : lock_stack) result += i.second.ToString() + std::string("\n"); return result; } -void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) +static bool LockHeld(void* mutex) +{ + LockData& lockdata = GetLockData(); + std::lock_guard<std::mutex> lock(lockdata.dd_mutex); + + const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; + for (const LockStackItem& i : lock_stack) { + if (i.first == mutex) return true; + } + + return false; +} + +template <typename MutexType> +void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) { - for (const std::pair<void*, CLockLocation>& i : g_lockstack) - if (i.first == cs) - return; + if (LockHeld(cs)) return; tfm::format(std::cerr, "Assertion failed: lock %s not held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld()); abort(); } +template void AssertLockHeldInternal(const char*, const char*, int, Mutex*); +template void AssertLockHeldInternal(const char*, const char*, int, RecursiveMutex*); void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) { - for (const std::pair<void*, CLockLocation>& i : g_lockstack) { - if (i.first == cs) { - tfm::format(std::cerr, "Assertion failed: lock %s held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld()); - abort(); - } - } + if (!LockHeld(cs)) return; + tfm::format(std::cerr, "Assertion failed: lock %s held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld()); + abort(); } void DeleteLock(void* cs) { LockData& lockdata = GetLockData(); - if (!lockdata.available) { - // We're already shutting down. - return; - } std::lock_guard<std::mutex> lock(lockdata.dd_mutex); - std::pair<void*, void*> item = std::make_pair(cs, nullptr); + const LockPair item = std::make_pair(cs, nullptr); LockOrders::iterator it = lockdata.lockorders.lower_bound(item); while (it != lockdata.lockorders.end() && it->first.first == cs) { - std::pair<void*, void*> invitem = std::make_pair(it->first.second, it->first.first); + const LockPair invitem = std::make_pair(it->first.second, it->first.first); lockdata.invlockorders.erase(invitem); lockdata.lockorders.erase(it++); } InvLockOrders::iterator invit = lockdata.invlockorders.lower_bound(item); while (invit != lockdata.invlockorders.end() && invit->first == cs) { - std::pair<void*, void*> invinvitem = std::make_pair(invit->second, invit->first); + const LockPair invinvitem = std::make_pair(invit->second, invit->first); lockdata.lockorders.erase(invinvitem); lockdata.invlockorders.erase(invit++); } diff --git a/src/sync.h b/src/sync.h index 0c6f0ef0a7..60e5a87aec 100644 --- a/src/sync.h +++ b/src/sync.h @@ -52,7 +52,8 @@ void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs void LeaveCritical(); void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line); std::string LocksHeld(); -void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs); +template <typename MutexType> +void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs); void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); void DeleteLock(void* cs); @@ -66,7 +67,8 @@ extern bool g_debug_lockorder_abort; void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} void static inline LeaveCritical() {} void static inline CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {} -void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} +template <typename MutexType> +void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} void static inline DeleteLock(void* cs) {} #endif diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 0565982215..35750b2ebc 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <checkqueue.h> +#include <sync.h> #include <test/util/setup_common.h> #include <util/memory.h> #include <util/system.h> @@ -57,14 +58,14 @@ struct FailingCheck { }; struct UniqueCheck { - static std::mutex m; - static std::unordered_multiset<size_t> results; + static Mutex m; + static std::unordered_multiset<size_t> results GUARDED_BY(m); size_t check_id; UniqueCheck(size_t check_id_in) : check_id(check_id_in){}; UniqueCheck() : check_id(0){}; bool operator()() { - std::lock_guard<std::mutex> l(m); + LOCK(m); results.insert(check_id); return true; } @@ -127,7 +128,7 @@ struct FrozenCleanupCheck { std::mutex FrozenCleanupCheck::m{}; std::atomic<uint64_t> FrozenCleanupCheck::nFrozen{0}; std::condition_variable FrozenCleanupCheck::cv{}; -std::mutex UniqueCheck::m; +Mutex UniqueCheck::m; std::unordered_multiset<size_t> UniqueCheck::results; std::atomic<size_t> FakeCheckCheckCompletion::n_calls{0}; std::atomic<size_t> MemoryCheck::fake_allocated_memory{0}; @@ -290,11 +291,15 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck) control.Add(vChecks); } } - bool r = true; - BOOST_REQUIRE_EQUAL(UniqueCheck::results.size(), COUNT); - for (size_t i = 0; i < COUNT; ++i) - r = r && UniqueCheck::results.count(i) == 1; - BOOST_REQUIRE(r); + { + LOCK(UniqueCheck::m); + bool r = true; + BOOST_REQUIRE_EQUAL(UniqueCheck::results.size(), COUNT); + for (size_t i = 0; i < COUNT; ++i) { + r = r && UniqueCheck::results.count(i) == 1; + } + BOOST_REQUIRE(r); + } tg.interrupt_all(); tg.join_all(); } diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp new file mode 100644 index 0000000000..52dd62a145 --- /dev/null +++ b/src/test/fuzz/coins_view.cpp @@ -0,0 +1,294 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <amount.h> +#include <chainparams.h> +#include <chainparamsbase.h> +#include <coins.h> +#include <consensus/tx_verify.h> +#include <consensus/validation.h> +#include <key.h> +#include <node/coinstats.h> +#include <policy/policy.h> +#include <primitives/transaction.h> +#include <pubkey.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <validation.h> + +#include <cstdint> +#include <limits> +#include <optional> +#include <string> +#include <vector> + +namespace { +const Coin EMPTY_COIN{}; + +bool operator==(const Coin& a, const Coin& b) +{ + if (a.IsSpent() && b.IsSpent()) return true; + return a.fCoinBase == b.fCoinBase && a.nHeight == b.nHeight && a.out == b.out; +} +} // namespace + +void initialize() +{ + static const ECCVerifyHandle ecc_verify_handle; + ECC_Start(); + SelectParams(CBaseChainParams::REGTEST); +} + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + CCoinsView backend_coins_view; + CCoinsViewCache coins_view_cache{&backend_coins_view}; + COutPoint random_out_point; + Coin random_coin; + CMutableTransaction random_mutable_transaction; + while (fuzzed_data_provider.ConsumeBool()) { + switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 9)) { + case 0: { + if (random_coin.IsSpent()) { + break; + } + Coin coin = random_coin; + bool expected_code_path = false; + const bool possible_overwrite = fuzzed_data_provider.ConsumeBool(); + try { + coins_view_cache.AddCoin(random_out_point, std::move(coin), possible_overwrite); + expected_code_path = true; + } catch (const std::logic_error& e) { + if (e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}) { + assert(!possible_overwrite); + expected_code_path = true; + } + } + assert(expected_code_path); + break; + } + case 1: { + (void)coins_view_cache.Flush(); + break; + } + case 2: { + coins_view_cache.SetBestBlock(ConsumeUInt256(fuzzed_data_provider)); + break; + } + case 3: { + Coin move_to; + (void)coins_view_cache.SpendCoin(random_out_point, fuzzed_data_provider.ConsumeBool() ? &move_to : nullptr); + break; + } + case 4: { + coins_view_cache.Uncache(random_out_point); + break; + } + case 5: { + if (fuzzed_data_provider.ConsumeBool()) { + backend_coins_view = CCoinsView{}; + } + coins_view_cache.SetBackend(backend_coins_view); + break; + } + case 6: { + const std::optional<COutPoint> opt_out_point = ConsumeDeserializable<COutPoint>(fuzzed_data_provider); + if (!opt_out_point) { + break; + } + random_out_point = *opt_out_point; + break; + } + case 7: { + const std::optional<Coin> opt_coin = ConsumeDeserializable<Coin>(fuzzed_data_provider); + if (!opt_coin) { + break; + } + random_coin = *opt_coin; + break; + } + case 8: { + const std::optional<CMutableTransaction> opt_mutable_transaction = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider); + if (!opt_mutable_transaction) { + break; + } + random_mutable_transaction = *opt_mutable_transaction; + break; + } + case 9: { + CCoinsMap coins_map; + while (fuzzed_data_provider.ConsumeBool()) { + CCoinsCacheEntry coins_cache_entry; + coins_cache_entry.flags = fuzzed_data_provider.ConsumeIntegral<unsigned char>(); + if (fuzzed_data_provider.ConsumeBool()) { + coins_cache_entry.coin = random_coin; + } else { + const std::optional<Coin> opt_coin = ConsumeDeserializable<Coin>(fuzzed_data_provider); + if (!opt_coin) { + break; + } + coins_cache_entry.coin = *opt_coin; + } + coins_map.emplace(random_out_point, std::move(coins_cache_entry)); + } + bool expected_code_path = false; + try { + coins_view_cache.BatchWrite(coins_map, fuzzed_data_provider.ConsumeBool() ? ConsumeUInt256(fuzzed_data_provider) : coins_view_cache.GetBestBlock()); + expected_code_path = true; + } catch (const std::logic_error& e) { + if (e.what() == std::string{"FRESH flag misapplied to coin that exists in parent cache"}) { + expected_code_path = true; + } + } + assert(expected_code_path); + break; + } + } + } + + { + const Coin& coin_using_access_coin = coins_view_cache.AccessCoin(random_out_point); + const bool exists_using_access_coin = !(coin_using_access_coin == EMPTY_COIN); + const bool exists_using_have_coin = coins_view_cache.HaveCoin(random_out_point); + const bool exists_using_have_coin_in_cache = coins_view_cache.HaveCoinInCache(random_out_point); + Coin coin_using_get_coin; + const bool exists_using_get_coin = coins_view_cache.GetCoin(random_out_point, coin_using_get_coin); + if (exists_using_get_coin) { + assert(coin_using_get_coin == coin_using_access_coin); + } + assert((exists_using_access_coin && exists_using_have_coin_in_cache && exists_using_have_coin && exists_using_get_coin) || + (!exists_using_access_coin && !exists_using_have_coin_in_cache && !exists_using_have_coin && !exists_using_get_coin)); + const bool exists_using_have_coin_in_backend = backend_coins_view.HaveCoin(random_out_point); + if (exists_using_have_coin_in_backend) { + assert(exists_using_have_coin); + } + Coin coin_using_backend_get_coin; + if (backend_coins_view.GetCoin(random_out_point, coin_using_backend_get_coin)) { + assert(exists_using_have_coin_in_backend); + assert(coin_using_get_coin == coin_using_backend_get_coin); + } else { + assert(!exists_using_have_coin_in_backend); + } + } + + { + bool expected_code_path = false; + try { + (void)coins_view_cache.Cursor(); + } catch (const std::logic_error&) { + expected_code_path = true; + } + assert(expected_code_path); + (void)coins_view_cache.DynamicMemoryUsage(); + (void)coins_view_cache.EstimateSize(); + (void)coins_view_cache.GetBestBlock(); + (void)coins_view_cache.GetCacheSize(); + (void)coins_view_cache.GetHeadBlocks(); + (void)coins_view_cache.HaveInputs(CTransaction{random_mutable_transaction}); + } + + { + const CCoinsViewCursor* coins_view_cursor = backend_coins_view.Cursor(); + assert(coins_view_cursor == nullptr); + (void)backend_coins_view.EstimateSize(); + (void)backend_coins_view.GetBestBlock(); + (void)backend_coins_view.GetHeadBlocks(); + } + + if (fuzzed_data_provider.ConsumeBool()) { + switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 6)) { + case 0: { + const CTransaction transaction{random_mutable_transaction}; + bool is_spent = false; + for (const CTxOut& tx_out : transaction.vout) { + if (Coin{tx_out, 0, transaction.IsCoinBase()}.IsSpent()) { + is_spent = true; + } + } + if (is_spent) { + // Avoid: + // coins.cpp:69: void CCoinsViewCache::AddCoin(const COutPoint &, Coin &&, bool): Assertion `!coin.IsSpent()' failed. + break; + } + bool expected_code_path = false; + const int height = fuzzed_data_provider.ConsumeIntegral<int>(); + const bool possible_overwrite = fuzzed_data_provider.ConsumeBool(); + try { + AddCoins(coins_view_cache, transaction, height, possible_overwrite); + expected_code_path = true; + } catch (const std::logic_error& e) { + if (e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}) { + assert(!possible_overwrite); + expected_code_path = true; + } + } + assert(expected_code_path); + break; + } + case 1: { + (void)AreInputsStandard(CTransaction{random_mutable_transaction}, coins_view_cache); + break; + } + case 2: { + TxValidationState state; + CAmount tx_fee_out; + const CTransaction transaction{random_mutable_transaction}; + if (ContainsSpentInput(transaction, coins_view_cache)) { + // Avoid: + // consensus/tx_verify.cpp:171: bool Consensus::CheckTxInputs(const CTransaction &, TxValidationState &, const CCoinsViewCache &, int, CAmount &): Assertion `!coin.IsSpent()' failed. + break; + } + try { + (void)Consensus::CheckTxInputs(transaction, state, coins_view_cache, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()), tx_fee_out); + assert(MoneyRange(tx_fee_out)); + } catch (const std::runtime_error&) { + } + break; + } + case 3: { + const CTransaction transaction{random_mutable_transaction}; + if (ContainsSpentInput(transaction, coins_view_cache)) { + // Avoid: + // consensus/tx_verify.cpp:130: unsigned int GetP2SHSigOpCount(const CTransaction &, const CCoinsViewCache &): Assertion `!coin.IsSpent()' failed. + break; + } + (void)GetP2SHSigOpCount(transaction, coins_view_cache); + break; + } + case 4: { + const CTransaction transaction{random_mutable_transaction}; + if (ContainsSpentInput(transaction, coins_view_cache)) { + // Avoid: + // consensus/tx_verify.cpp:130: unsigned int GetP2SHSigOpCount(const CTransaction &, const CCoinsViewCache &): Assertion `!coin.IsSpent()' failed. + break; + } + const int flags = fuzzed_data_provider.ConsumeIntegral<int>(); + if (!transaction.vin.empty() && (flags & SCRIPT_VERIFY_WITNESS) != 0 && (flags & SCRIPT_VERIFY_P2SH) == 0) { + // Avoid: + // script/interpreter.cpp:1705: size_t CountWitnessSigOps(const CScript &, const CScript &, const CScriptWitness *, unsigned int): Assertion `(flags & SCRIPT_VERIFY_P2SH) != 0' failed. + break; + } + (void)GetTransactionSigOpCost(transaction, coins_view_cache, flags); + break; + } + case 5: { + CCoinsStats stats; + bool expected_code_path = false; + try { + (void)GetUTXOStats(&coins_view_cache, stats); + } catch (const std::logic_error&) { + expected_code_path = true; + } + assert(expected_code_path); + break; + } + case 6: { + (void)IsWitnessStandard(CTransaction{random_mutable_transaction}, coins_view_cache); + break; + } + } + } +} diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp index e0c4ad7eb7..933cf9049d 100644 --- a/src/test/fuzz/script.cpp +++ b/src/test/fuzz/script.cpp @@ -11,6 +11,7 @@ #include <script/descriptor.h> #include <script/interpreter.h> #include <script/script.h> +#include <script/script_error.h> #include <script/sign.h> #include <script/signingprovider.h> #include <script/standard.h> @@ -21,6 +22,8 @@ #include <univalue.h> #include <util/memory.h> +#include <algorithm> +#include <cassert> #include <cstdint> #include <optional> #include <string> @@ -124,4 +127,40 @@ void test_one_input(const std::vector<uint8_t>& buffer) wit.SetNull(); } } + + (void)GetOpName(ConsumeOpcodeType(fuzzed_data_provider)); + (void)ScriptErrorString(static_cast<ScriptError>(fuzzed_data_provider.ConsumeIntegralInRange<int>(0, SCRIPT_ERR_ERROR_COUNT))); + + { + const std::vector<uint8_t> bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider); + CScript append_script{bytes.begin(), bytes.end()}; + append_script << fuzzed_data_provider.ConsumeIntegral<int64_t>(); + append_script << ConsumeOpcodeType(fuzzed_data_provider); + append_script << CScriptNum{fuzzed_data_provider.ConsumeIntegral<int64_t>()}; + append_script << ConsumeRandomLengthByteVector(fuzzed_data_provider); + } + + { + WitnessUnknown witness_unknown_1{}; + witness_unknown_1.version = fuzzed_data_provider.ConsumeIntegral<int>(); + const std::vector<uint8_t> witness_unknown_program_1 = fuzzed_data_provider.ConsumeBytes<uint8_t>(40); + witness_unknown_1.length = witness_unknown_program_1.size(); + std::copy(witness_unknown_program_1.begin(), witness_unknown_program_1.end(), witness_unknown_1.program); + + WitnessUnknown witness_unknown_2{}; + witness_unknown_2.version = fuzzed_data_provider.ConsumeIntegral<int>(); + const std::vector<uint8_t> witness_unknown_program_2 = fuzzed_data_provider.ConsumeBytes<uint8_t>(40); + witness_unknown_2.length = witness_unknown_program_2.size(); + std::copy(witness_unknown_program_2.begin(), witness_unknown_program_2.end(), witness_unknown_2.program); + + (void)(witness_unknown_1 == witness_unknown_2); + (void)(witness_unknown_1 < witness_unknown_2); + } + + { + const CTxDestination tx_destination_1 = ConsumeTxDestination(fuzzed_data_provider); + const CTxDestination tx_destination_2 = ConsumeTxDestination(fuzzed_data_provider); + (void)(tx_destination_1 == tx_destination_2); + (void)(tx_destination_1 < tx_destination_2); + } } diff --git a/src/test/fuzz/script_bitcoin_consensus.cpp b/src/test/fuzz/script_bitcoin_consensus.cpp new file mode 100644 index 0000000000..22f4b4f44a --- /dev/null +++ b/src/test/fuzz/script_bitcoin_consensus.cpp @@ -0,0 +1,31 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <script/bitcoinconsensus.h> +#include <script/interpreter.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <string> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + const std::vector<uint8_t> random_bytes_1 = ConsumeRandomLengthByteVector(fuzzed_data_provider); + const std::vector<uint8_t> random_bytes_2 = ConsumeRandomLengthByteVector(fuzzed_data_provider); + const CAmount money = ConsumeMoney(fuzzed_data_provider); + bitcoinconsensus_error err; + bitcoinconsensus_error* err_p = fuzzed_data_provider.ConsumeBool() ? &err : nullptr; + const unsigned int n_in = fuzzed_data_provider.ConsumeIntegral<unsigned int>(); + const unsigned int flags = fuzzed_data_provider.ConsumeIntegral<unsigned int>(); + assert(bitcoinconsensus_version() == BITCOINCONSENSUS_API_VER); + if ((flags & SCRIPT_VERIFY_WITNESS) != 0 && (flags & SCRIPT_VERIFY_P2SH) == 0) { + return; + } + (void)bitcoinconsensus_verify_script(random_bytes_1.data(), random_bytes_1.size(), random_bytes_2.data(), random_bytes_2.size(), n_in, flags, err_p); + (void)bitcoinconsensus_verify_script_with_amount(random_bytes_1.data(), random_bytes_1.size(), money, random_bytes_2.data(), random_bytes_2.size(), n_in, flags, err_p); +} diff --git a/src/test/fuzz/script_descriptor_cache.cpp b/src/test/fuzz/script_descriptor_cache.cpp new file mode 100644 index 0000000000..4bfe61cec7 --- /dev/null +++ b/src/test/fuzz/script_descriptor_cache.cpp @@ -0,0 +1,42 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <optional.h> +#include <pubkey.h> +#include <script/descriptor.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <string> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + DescriptorCache descriptor_cache; + while (fuzzed_data_provider.ConsumeBool()) { + const std::vector<uint8_t> code = fuzzed_data_provider.ConsumeBytes<uint8_t>(BIP32_EXTKEY_SIZE); + if (code.size() == BIP32_EXTKEY_SIZE) { + CExtPubKey xpub; + xpub.Decode(code.data()); + const uint32_t key_exp_pos = fuzzed_data_provider.ConsumeIntegral<uint32_t>(); + CExtPubKey xpub_fetched; + if (fuzzed_data_provider.ConsumeBool()) { + (void)descriptor_cache.GetCachedParentExtPubKey(key_exp_pos, xpub_fetched); + descriptor_cache.CacheParentExtPubKey(key_exp_pos, xpub); + assert(descriptor_cache.GetCachedParentExtPubKey(key_exp_pos, xpub_fetched)); + } else { + const uint32_t der_index = fuzzed_data_provider.ConsumeIntegral<uint32_t>(); + (void)descriptor_cache.GetCachedDerivedExtPubKey(key_exp_pos, der_index, xpub_fetched); + descriptor_cache.CacheDerivedExtPubKey(key_exp_pos, der_index, xpub); + assert(descriptor_cache.GetCachedDerivedExtPubKey(key_exp_pos, der_index, xpub_fetched)); + } + assert(xpub == xpub_fetched); + } + (void)descriptor_cache.GetCachedParentExtPubKeys(); + (void)descriptor_cache.GetCachedDerivedExtPubKeys(); + } +} diff --git a/src/test/fuzz/script_interpreter.cpp b/src/test/fuzz/script_interpreter.cpp new file mode 100644 index 0000000000..26d5732f24 --- /dev/null +++ b/src/test/fuzz/script_interpreter.cpp @@ -0,0 +1,41 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <primitives/transaction.h> +#include <script/interpreter.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <optional> +#include <string> +#include <vector> + +bool CastToBool(const std::vector<unsigned char>& vch); + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + { + const CScript script_code = ConsumeScript(fuzzed_data_provider); + const std::optional<CMutableTransaction> mtx = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider); + if (mtx) { + const CTransaction tx_to{*mtx}; + const unsigned int in = fuzzed_data_provider.ConsumeIntegral<unsigned int>(); + if (in < tx_to.vin.size()) { + (void)SignatureHash(script_code, tx_to, in, fuzzed_data_provider.ConsumeIntegral<int>(), ConsumeMoney(fuzzed_data_provider), fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0}), nullptr); + const std::optional<CMutableTransaction> mtx_precomputed = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider); + if (mtx_precomputed) { + const CTransaction tx_precomputed{*mtx_precomputed}; + const PrecomputedTransactionData precomputed_transaction_data{tx_precomputed}; + (void)SignatureHash(script_code, tx_to, in, fuzzed_data_provider.ConsumeIntegral<int>(), ConsumeMoney(fuzzed_data_provider), fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0}), &precomputed_transaction_data); + } + } + } + } + { + (void)CastToBool(ConsumeRandomLengthByteVector(fuzzed_data_provider)); + } +} diff --git a/src/test/fuzz/script_sigcache.cpp b/src/test/fuzz/script_sigcache.cpp new file mode 100644 index 0000000000..434a47b702 --- /dev/null +++ b/src/test/fuzz/script_sigcache.cpp @@ -0,0 +1,45 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <chainparams.h> +#include <chainparamsbase.h> +#include <key.h> +#include <pubkey.h> +#include <script/sigcache.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <optional> +#include <string> +#include <vector> + +void initialize() +{ + static const ECCVerifyHandle ecc_verify_handle; + ECC_Start(); + SelectParams(CBaseChainParams::REGTEST); + InitSignatureCache(); +} + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + + const std::optional<CMutableTransaction> mutable_transaction = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider); + const CTransaction tx = mutable_transaction ? CTransaction{*mutable_transaction} : CTransaction{}; + const unsigned int n_in = fuzzed_data_provider.ConsumeIntegral<unsigned int>(); + const CAmount amount = ConsumeMoney(fuzzed_data_provider); + const bool store = fuzzed_data_provider.ConsumeBool(); + PrecomputedTransactionData tx_data; + CachingTransactionSignatureChecker caching_transaction_signature_checker{mutable_transaction ? &tx : nullptr, n_in, amount, store, tx_data}; + const std::optional<CPubKey> pub_key = ConsumeDeserializable<CPubKey>(fuzzed_data_provider); + if (pub_key) { + const std::vector<uint8_t> random_bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider); + if (!random_bytes.empty()) { + (void)caching_transaction_signature_checker.VerifySignature(random_bytes, *pub_key, ConsumeUInt256(fuzzed_data_provider)); + } + } +} diff --git a/src/test/fuzz/script_sign.cpp b/src/test/fuzz/script_sign.cpp new file mode 100644 index 0000000000..c626f950e7 --- /dev/null +++ b/src/test/fuzz/script_sign.cpp @@ -0,0 +1,149 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <chainparams.h> +#include <chainparamsbase.h> +#include <key.h> +#include <pubkey.h> +#include <script/keyorigin.h> +#include <script/sign.h> +#include <script/signingprovider.h> +#include <streams.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cassert> +#include <cstdint> +#include <iostream> +#include <map> +#include <optional> +#include <string> +#include <vector> + +void initialize() +{ + static const ECCVerifyHandle ecc_verify_handle; + ECC_Start(); + SelectParams(CBaseChainParams::REGTEST); +} + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + const std::vector<uint8_t> key = ConsumeRandomLengthByteVector(fuzzed_data_provider, 128); + + { + CDataStream random_data_stream = ConsumeDataStream(fuzzed_data_provider); + std::map<CPubKey, KeyOriginInfo> hd_keypaths; + try { + DeserializeHDKeypaths(random_data_stream, key, hd_keypaths); + } catch (const std::ios_base::failure&) { + } + CDataStream serialized{SER_NETWORK, PROTOCOL_VERSION}; + SerializeHDKeypaths(serialized, hd_keypaths, fuzzed_data_provider.ConsumeIntegral<uint8_t>()); + } + + { + std::map<CPubKey, KeyOriginInfo> hd_keypaths; + while (fuzzed_data_provider.ConsumeBool()) { + const std::optional<CPubKey> pub_key = ConsumeDeserializable<CPubKey>(fuzzed_data_provider); + if (!pub_key) { + break; + } + const std::optional<KeyOriginInfo> key_origin_info = ConsumeDeserializable<KeyOriginInfo>(fuzzed_data_provider); + if (!key_origin_info) { + break; + } + hd_keypaths[*pub_key] = *key_origin_info; + } + CDataStream serialized{SER_NETWORK, PROTOCOL_VERSION}; + try { + SerializeHDKeypaths(serialized, hd_keypaths, fuzzed_data_provider.ConsumeIntegral<uint8_t>()); + } catch (const std::ios_base::failure&) { + } + std::map<CPubKey, KeyOriginInfo> deserialized_hd_keypaths; + try { + DeserializeHDKeypaths(serialized, key, hd_keypaths); + } catch (const std::ios_base::failure&) { + } + assert(hd_keypaths.size() >= deserialized_hd_keypaths.size()); + } + + { + SignatureData signature_data_1{ConsumeScript(fuzzed_data_provider)}; + SignatureData signature_data_2{ConsumeScript(fuzzed_data_provider)}; + signature_data_1.MergeSignatureData(signature_data_2); + } + + FillableSigningProvider provider; + CKey k; + const std::vector<uint8_t> key_data = ConsumeRandomLengthByteVector(fuzzed_data_provider); + k.Set(key_data.begin(), key_data.end(), fuzzed_data_provider.ConsumeBool()); + if (k.IsValid()) { + provider.AddKey(k); + } + + { + const std::optional<CMutableTransaction> mutable_transaction = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider); + const std::optional<CTxOut> tx_out = ConsumeDeserializable<CTxOut>(fuzzed_data_provider); + const unsigned int n_in = fuzzed_data_provider.ConsumeIntegral<unsigned int>(); + if (mutable_transaction && tx_out && mutable_transaction->vin.size() > n_in) { + SignatureData signature_data_1 = DataFromTransaction(*mutable_transaction, n_in, *tx_out); + CTxIn input; + UpdateInput(input, signature_data_1); + const CScript script = ConsumeScript(fuzzed_data_provider); + SignatureData signature_data_2{script}; + signature_data_1.MergeSignatureData(signature_data_2); + } + if (mutable_transaction) { + CTransaction tx_from{*mutable_transaction}; + CMutableTransaction tx_to; + const std::optional<CMutableTransaction> opt_tx_to = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider); + if (opt_tx_to) { + tx_to = *opt_tx_to; + } + CMutableTransaction script_tx_to = tx_to; + CMutableTransaction sign_transaction_tx_to = tx_to; + if (n_in < tx_to.vin.size() && tx_to.vin[n_in].prevout.n < tx_from.vout.size()) { + (void)SignSignature(provider, tx_from, tx_to, n_in, fuzzed_data_provider.ConsumeIntegral<int>()); + } + if (n_in < script_tx_to.vin.size()) { + (void)SignSignature(provider, ConsumeScript(fuzzed_data_provider), script_tx_to, n_in, ConsumeMoney(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<int>()); + MutableTransactionSignatureCreator signature_creator{&tx_to, n_in, ConsumeMoney(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<int>()}; + std::vector<unsigned char> vch_sig; + CKeyID address; + if (fuzzed_data_provider.ConsumeBool()) { + if (k.IsValid()) { + address = k.GetPubKey().GetID(); + } + } else { + address = CKeyID{ConsumeUInt160(fuzzed_data_provider)}; + } + (void)signature_creator.CreateSig(provider, vch_sig, address, ConsumeScript(fuzzed_data_provider), fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0})); + } + std::map<COutPoint, Coin> coins; + while (fuzzed_data_provider.ConsumeBool()) { + const std::optional<COutPoint> outpoint = ConsumeDeserializable<COutPoint>(fuzzed_data_provider); + if (!outpoint) { + break; + } + const std::optional<Coin> coin = ConsumeDeserializable<Coin>(fuzzed_data_provider); + if (!coin) { + break; + } + coins[*outpoint] = *coin; + } + std::map<int, std::string> input_errors; + (void)SignTransaction(sign_transaction_tx_to, &provider, coins, fuzzed_data_provider.ConsumeIntegral<int>(), input_errors); + } + } + + { + SignatureData signature_data_1; + (void)ProduceSignature(provider, DUMMY_SIGNATURE_CREATOR, ConsumeScript(fuzzed_data_provider), signature_data_1); + SignatureData signature_data_2; + (void)ProduceSignature(provider, DUMMY_MAXIMUM_SIGNATURE_CREATOR, ConsumeScript(fuzzed_data_provider), signature_data_2); + } +} diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index 3c1f911f7e..271062dc95 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -12,6 +12,7 @@ #include <rpc/server.h> #include <rpc/util.h> #include <script/descriptor.h> +#include <script/script.h> #include <serialize.h> #include <streams.h> #include <test/fuzz/FuzzedDataProvider.h> @@ -89,11 +90,15 @@ void test_one_input(const std::vector<uint8_t>& buffer) (void)urlDecode(random_string_1); (void)ValidAsCString(random_string_1); (void)_(random_string_1.c_str()); + try { + throw scriptnum_error{random_string_1}; + } catch (const std::runtime_error&) { + } { CDataStream data_stream{SER_NETWORK, INIT_PROTO_VERSION}; std::string s; - LimitedString<10> limited_string = LIMITED_STRING(s, 10); + auto limited_string = LIMITED_STRING(s, 10); data_stream << random_string_1; try { data_stream >> limited_string; @@ -108,7 +113,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) } { CDataStream data_stream{SER_NETWORK, INIT_PROTO_VERSION}; - const LimitedString<10> limited_string = LIMITED_STRING(random_string_1, 10); + const auto limited_string = LIMITED_STRING(random_string_1, 10); data_stream << limited_string; std::string deserialized_string; data_stream >> deserialized_string; diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index f72d9380eb..f26878a704 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -8,9 +8,11 @@ #include <amount.h> #include <arith_uint256.h> #include <attributes.h> +#include <coins.h> #include <consensus/consensus.h> #include <primitives/transaction.h> #include <script/script.h> +#include <script/standard.h> #include <serialize.h> #include <streams.h> #include <test/fuzz/FuzzedDataProvider.h> @@ -19,6 +21,7 @@ #include <uint256.h> #include <version.h> +#include <algorithm> #include <cstdint> #include <optional> #include <string> @@ -30,6 +33,11 @@ NODISCARD inline std::vector<uint8_t> ConsumeRandomLengthByteVector(FuzzedDataPr return {s.begin(), s.end()}; } +NODISCARD inline CDataStream ConsumeDataStream(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length = 4096) noexcept +{ + return {ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length), SER_NETWORK, INIT_PROTO_VERSION}; +} + NODISCARD inline std::vector<std::string> ConsumeRandomLengthStringVector(FuzzedDataProvider& fuzzed_data_provider, const size_t max_vector_size = 16, const size_t max_string_length = 16) noexcept { const size_t n_elements = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, max_vector_size); @@ -86,10 +94,19 @@ NODISCARD inline CScriptNum ConsumeScriptNum(FuzzedDataProvider& fuzzed_data_pro return CScriptNum{fuzzed_data_provider.ConsumeIntegral<int64_t>()}; } +NODISCARD inline uint160 ConsumeUInt160(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + const std::vector<uint8_t> v160 = fuzzed_data_provider.ConsumeBytes<uint8_t>(160 / 8); + if (v160.size() != 160 / 8) { + return {}; + } + return uint160{v160}; +} + NODISCARD inline uint256 ConsumeUInt256(FuzzedDataProvider& fuzzed_data_provider) noexcept { - const std::vector<unsigned char> v256 = fuzzed_data_provider.ConsumeBytes<unsigned char>(sizeof(uint256)); - if (v256.size() != sizeof(uint256)) { + const std::vector<uint8_t> v256 = fuzzed_data_provider.ConsumeBytes<uint8_t>(256 / 8); + if (v256.size() != 256 / 8) { return {}; } return uint256{v256}; @@ -115,6 +132,43 @@ NODISCARD inline CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzze return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, spends_coinbase, sig_op_cost, {}}; } +NODISCARD inline CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + CTxDestination tx_destination; + switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 5)) { + case 0: { + tx_destination = CNoDestination{}; + break; + } + case 1: { + tx_destination = PKHash{ConsumeUInt160(fuzzed_data_provider)}; + break; + } + case 2: { + tx_destination = ScriptHash{ConsumeUInt160(fuzzed_data_provider)}; + break; + } + case 3: { + tx_destination = WitnessV0ScriptHash{ConsumeUInt256(fuzzed_data_provider)}; + break; + } + case 4: { + tx_destination = WitnessV0KeyHash{ConsumeUInt160(fuzzed_data_provider)}; + break; + } + case 5: { + WitnessUnknown witness_unknown{}; + witness_unknown.version = fuzzed_data_provider.ConsumeIntegral<int>(); + const std::vector<uint8_t> witness_unknown_program_1 = fuzzed_data_provider.ConsumeBytes<uint8_t>(40); + witness_unknown.length = witness_unknown_program_1.size(); + std::copy(witness_unknown_program_1.begin(), witness_unknown_program_1.end(), witness_unknown.program); + tx_destination = witness_unknown; + break; + } + } + return tx_destination; +} + template <typename T> NODISCARD bool MultiplicationOverflow(const T i, const T j) noexcept { @@ -149,4 +203,15 @@ NODISCARD bool AdditionOverflow(const T i, const T j) noexcept return std::numeric_limits<T>::max() - i < j; } +NODISCARD inline bool ContainsSpentInput(const CTransaction& tx, const CCoinsViewCache& inputs) noexcept +{ + for (const CTxIn& tx_in : tx.vin) { + const Coin& coin = inputs.AccessCoin(tx_in.prevout); + if (coin.IsSpent()) { + return true; + } + } + return false; +} + #endif // BITCOIN_TEST_FUZZ_UTIL_H diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp index 512e48f8e5..45c9b90ee9 100644 --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -13,9 +13,18 @@ #include <boost/algorithm/string.hpp> #include <boost/test/unit_test.hpp> -BOOST_FIXTURE_TEST_SUITE(getarg_tests, BasicTestingSetup) +namespace getarg_tests{ + class LocalTestingSetup : BasicTestingSetup { + protected: + void SetupArgs(const std::vector<std::pair<std::string, unsigned int>>& args); + void ResetArgs(const std::string& strArg); + ArgsManager m_args; + }; +} + +BOOST_FIXTURE_TEST_SUITE(getarg_tests, LocalTestingSetup) -static void ResetArgs(const std::string& strArg) +void LocalTestingSetup :: ResetArgs(const std::string& strArg) { std::vector<std::string> vecArg; if (strArg.size()) @@ -30,14 +39,14 @@ static void ResetArgs(const std::string& strArg) vecChar.push_back(s.c_str()); std::string error; - BOOST_CHECK(gArgs.ParseParameters(vecChar.size(), vecChar.data(), error)); + BOOST_CHECK(m_args.ParseParameters(vecChar.size(), vecChar.data(), error)); } -static void SetupArgs(const std::vector<std::pair<std::string, unsigned int>>& args) +void LocalTestingSetup :: SetupArgs(const std::vector<std::pair<std::string, unsigned int>>& args) { - gArgs.ClearArgs(); + m_args.ClearArgs(); for (const auto& arg : args) { - gArgs.AddArg(arg.first, "", arg.second, OptionsCategory::OPTIONS); + m_args.AddArg(arg.first, "", arg.second, OptionsCategory::OPTIONS); } } @@ -46,52 +55,52 @@ BOOST_AUTO_TEST_CASE(boolarg) const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY); SetupArgs({foo}); ResetArgs("-foo"); - BOOST_CHECK(gArgs.GetBoolArg("-foo", false)); - BOOST_CHECK(gArgs.GetBoolArg("-foo", true)); + BOOST_CHECK(m_args.GetBoolArg("-foo", false)); + BOOST_CHECK(m_args.GetBoolArg("-foo", true)); - BOOST_CHECK(!gArgs.GetBoolArg("-fo", false)); - BOOST_CHECK(gArgs.GetBoolArg("-fo", true)); + BOOST_CHECK(!m_args.GetBoolArg("-fo", false)); + BOOST_CHECK(m_args.GetBoolArg("-fo", true)); - BOOST_CHECK(!gArgs.GetBoolArg("-fooo", false)); - BOOST_CHECK(gArgs.GetBoolArg("-fooo", true)); + BOOST_CHECK(!m_args.GetBoolArg("-fooo", false)); + BOOST_CHECK(m_args.GetBoolArg("-fooo", true)); ResetArgs("-foo=0"); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", false)); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", true)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", false)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", true)); ResetArgs("-foo=1"); - BOOST_CHECK(gArgs.GetBoolArg("-foo", false)); - BOOST_CHECK(gArgs.GetBoolArg("-foo", true)); + BOOST_CHECK(m_args.GetBoolArg("-foo", false)); + BOOST_CHECK(m_args.GetBoolArg("-foo", true)); // New 0.6 feature: auto-map -nosomething to !-something: ResetArgs("-nofoo"); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", false)); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", true)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", false)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", true)); ResetArgs("-nofoo=1"); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", false)); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", true)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", false)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", true)); ResetArgs("-foo -nofoo"); // -nofoo should win - BOOST_CHECK(!gArgs.GetBoolArg("-foo", false)); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", true)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", false)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", true)); ResetArgs("-foo=1 -nofoo=1"); // -nofoo should win - BOOST_CHECK(!gArgs.GetBoolArg("-foo", false)); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", true)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", false)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", true)); ResetArgs("-foo=0 -nofoo=0"); // -nofoo=0 should win - BOOST_CHECK(gArgs.GetBoolArg("-foo", false)); - BOOST_CHECK(gArgs.GetBoolArg("-foo", true)); + BOOST_CHECK(m_args.GetBoolArg("-foo", false)); + BOOST_CHECK(m_args.GetBoolArg("-foo", true)); // New 0.6 feature: treat -- same as -: ResetArgs("--foo=1"); - BOOST_CHECK(gArgs.GetBoolArg("-foo", false)); - BOOST_CHECK(gArgs.GetBoolArg("-foo", true)); + BOOST_CHECK(m_args.GetBoolArg("-foo", false)); + BOOST_CHECK(m_args.GetBoolArg("-foo", true)); ResetArgs("--nofoo=1"); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", false)); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", true)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", false)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", true)); } @@ -101,24 +110,24 @@ BOOST_AUTO_TEST_CASE(stringarg) const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY); SetupArgs({foo, bar}); ResetArgs(""); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), ""); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", "eleven"), "eleven"); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", ""), ""); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", "eleven"), "eleven"); ResetArgs("-foo -bar"); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), ""); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", "eleven"), ""); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", ""), ""); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", "eleven"), ""); ResetArgs("-foo="); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), ""); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", "eleven"), ""); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", ""), ""); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", "eleven"), ""); ResetArgs("-foo=11"); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), "11"); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", "eleven"), "11"); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", ""), "11"); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", "eleven"), "11"); ResetArgs("-foo=eleven"); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), "eleven"); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", "eleven"), "eleven"); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", ""), "eleven"); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", "eleven"), "eleven"); } @@ -128,20 +137,20 @@ BOOST_AUTO_TEST_CASE(intarg) const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY); SetupArgs({foo, bar}); ResetArgs(""); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 11), 11); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 0), 0); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", 11), 11); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", 0), 0); ResetArgs("-foo -bar"); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 11), 0); - BOOST_CHECK_EQUAL(gArgs.GetArg("-bar", 11), 0); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", 11), 0); + BOOST_CHECK_EQUAL(m_args.GetArg("-bar", 11), 0); ResetArgs("-foo=11 -bar=12"); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 0), 11); - BOOST_CHECK_EQUAL(gArgs.GetArg("-bar", 11), 12); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", 0), 11); + BOOST_CHECK_EQUAL(m_args.GetArg("-bar", 11), 12); ResetArgs("-foo=NaN -bar=NotANumber"); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 1), 0); - BOOST_CHECK_EQUAL(gArgs.GetArg("-bar", 11), 0); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", 1), 0); + BOOST_CHECK_EQUAL(m_args.GetArg("-bar", 11), 0); } BOOST_AUTO_TEST_CASE(doubledash) @@ -150,11 +159,11 @@ BOOST_AUTO_TEST_CASE(doubledash) const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY); SetupArgs({foo, bar}); ResetArgs("--foo"); - BOOST_CHECK_EQUAL(gArgs.GetBoolArg("-foo", false), true); + BOOST_CHECK_EQUAL(m_args.GetBoolArg("-foo", false), true); ResetArgs("--foo=verbose --bar=1"); - BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), "verbose"); - BOOST_CHECK_EQUAL(gArgs.GetArg("-bar", 0), 1); + BOOST_CHECK_EQUAL(m_args.GetArg("-foo", ""), "verbose"); + BOOST_CHECK_EQUAL(m_args.GetArg("-bar", 0), 1); } BOOST_AUTO_TEST_CASE(boolargno) @@ -163,24 +172,24 @@ BOOST_AUTO_TEST_CASE(boolargno) const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY); SetupArgs({foo, bar}); ResetArgs("-nofoo"); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", true)); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", false)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", true)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", false)); ResetArgs("-nofoo=1"); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", true)); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", false)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", true)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", false)); ResetArgs("-nofoo=0"); - BOOST_CHECK(gArgs.GetBoolArg("-foo", true)); - BOOST_CHECK(gArgs.GetBoolArg("-foo", false)); + BOOST_CHECK(m_args.GetBoolArg("-foo", true)); + BOOST_CHECK(m_args.GetBoolArg("-foo", false)); ResetArgs("-foo --nofoo"); // --nofoo should win - BOOST_CHECK(!gArgs.GetBoolArg("-foo", true)); - BOOST_CHECK(!gArgs.GetBoolArg("-foo", false)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", true)); + BOOST_CHECK(!m_args.GetBoolArg("-foo", false)); ResetArgs("-nofoo -foo"); // foo always wins: - BOOST_CHECK(gArgs.GetBoolArg("-foo", true)); - BOOST_CHECK(gArgs.GetBoolArg("-foo", false)); + BOOST_CHECK(m_args.GetBoolArg("-foo", true)); + BOOST_CHECK(m_args.GetBoolArg("-foo", false)); } BOOST_AUTO_TEST_CASE(logargs) @@ -200,7 +209,7 @@ BOOST_AUTO_TEST_CASE(logargs) }); // Log the arguments - gArgs.LogArgs(); + m_args.LogArgs(); LogInstance().DeleteCallback(print_connection); // Check that what should appear does, and what shouldn't doesn't. diff --git a/src/test/raii_event_tests.cpp b/src/test/raii_event_tests.cpp index 04bf7c20c1..8c2712f764 100644 --- a/src/test/raii_event_tests.cpp +++ b/src/test/raii_event_tests.cpp @@ -4,9 +4,6 @@ #include <event2/event.h> -#ifdef EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED -// It would probably be ideal to define dummy test(s) that report skipped, but boost::test doesn't seem to make that practical (at least not in versions available with common distros) - #include <map> #include <stdlib.h> @@ -16,6 +13,10 @@ #include <boost/test/unit_test.hpp> +BOOST_FIXTURE_TEST_SUITE(raii_event_tests, BasicTestingSetup) + +#ifdef EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED + static std::map<void*, short> tags; static std::map<void*, uint16_t> orders; static uint16_t tagSequence = 0; @@ -34,8 +35,6 @@ static void tag_free(void* mem) { free(mem); } -BOOST_FIXTURE_TEST_SUITE(raii_event_tests, BasicTestingSetup) - BOOST_AUTO_TEST_CASE(raii_event_creation) { event_set_mem_functions(tag_malloc, realloc, tag_free); @@ -87,6 +86,14 @@ BOOST_AUTO_TEST_CASE(raii_event_order) event_set_mem_functions(malloc, realloc, free); } -BOOST_AUTO_TEST_SUITE_END() +#else + +BOOST_AUTO_TEST_CASE(raii_event_tests_SKIPPED) +{ + // It would probably be ideal to report skipped, but boost::test doesn't seem to make that practical (at least not in versions available with common distros) + BOOST_TEST_MESSAGE("Skipping raii_event_tess: libevent doesn't support event_set_mem_functions"); +} #endif // EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 56454f61f3..cb3ae290d1 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -102,7 +102,7 @@ static ScriptErrorDesc script_errors[]={ {SCRIPT_ERR_SIG_FINDANDDELETE, "SIG_FINDANDDELETE"}, }; -static const char *FormatScriptError(ScriptError_t err) +static std::string FormatScriptError(ScriptError_t err) { for (unsigned int i=0; i<ARRAYLEN(script_errors); ++i) if (script_errors[i].err == err) @@ -134,7 +134,7 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, const CScript CMutableTransaction tx = BuildSpendingTransaction(scriptSig, scriptWitness, txCredit); CMutableTransaction tx2 = tx; BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, &scriptWitness, flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue), &err) == expect, message); - BOOST_CHECK_MESSAGE(err == scriptError, std::string(FormatScriptError(err)) + " where " + std::string(FormatScriptError((ScriptError_t)scriptError)) + " expected: " + message); + BOOST_CHECK_MESSAGE(err == scriptError, FormatScriptError(err) + " where " + FormatScriptError((ScriptError_t)scriptError) + " expected: " + message); // Verify that removing flags from a passing test or adding flags to a failing test does not change the result. for (int i = 0; i < 16; ++i) { diff --git a/src/threadsafety.h b/src/threadsafety.h index bb988dfdfd..942aa3fdcd 100644 --- a/src/threadsafety.h +++ b/src/threadsafety.h @@ -6,6 +6,8 @@ #ifndef BITCOIN_THREADSAFETY_H #define BITCOIN_THREADSAFETY_H +#include <mutex> + #ifdef __clang__ // TL;DR Add GUARDED_BY(mutex) to member variables. The others are // rarely necessary. Ex: int nFoo GUARDED_BY(cs_foo); @@ -54,4 +56,19 @@ #define ASSERT_EXCLUSIVE_LOCK(...) #endif // __GNUC__ +// StdMutex provides an annotated version of std::mutex for us, +// and should only be used when sync.h Mutex/LOCK/etc are not usable. +class LOCKABLE StdMutex : public std::mutex +{ +}; + +// StdLockGuard provides an annotated version of std::lock_guard for us, +// and should only be used when sync.h Mutex/LOCK/etc are not usable. +class SCOPED_LOCKABLE StdLockGuard : public std::lock_guard<StdMutex> +{ +public: + explicit StdLockGuard(StdMutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard<StdMutex>(cs) {} + ~StdLockGuard() UNLOCK_FUNCTION() {} +}; + #endif // BITCOIN_THREADSAFETY_H diff --git a/src/txmempool.h b/src/txmempool.h index 4568eb928d..583f7614b7 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -704,7 +704,7 @@ public: /** Adds a transaction to the unbroadcast set */ void AddUnbroadcastTx(const uint256& txid) { LOCK(cs); - /** Sanity Check: the transaction should also be in the mempool */ + // Sanity Check: the transaction should also be in the mempool if (exists(txid)) { m_unbroadcast_txids.insert(txid); } @@ -714,12 +714,12 @@ public: void RemoveUnbroadcastTx(const uint256& txid, const bool unchecked = false); /** Returns transactions in unbroadcast set */ - const std::set<uint256> GetUnbroadcastTxs() const { + std::set<uint256> GetUnbroadcastTxs() const { LOCK(cs); return m_unbroadcast_txids; } - // Returns if a txid is in the unbroadcast set + /** Returns whether a txid is in the unbroadcast set */ bool IsUnbroadcastTx(const uint256& txid) const { LOCK(cs); return (m_unbroadcast_txids.count(txid) != 0); diff --git a/src/util/system.cpp b/src/util/system.cpp index 2013b416db..bde0f097be 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -3,6 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <sync.h> #include <util/system.h> #include <chainparamsbase.h> @@ -75,18 +76,18 @@ const char * const BITCOIN_CONF_FILENAME = "bitcoin.conf"; ArgsManager gArgs; +/** Mutex to protect dir_locks. */ +static Mutex cs_dir_locks; /** A map that contains all the currently held directory locks. After * successful locking, these will be held here until the global destructor * cleans them up and thus automatically unlocks them, or ReleaseDirectoryLocks * is called. */ -static std::map<std::string, std::unique_ptr<fsbridge::FileLock>> dir_locks; -/** Mutex to protect dir_locks. */ -static std::mutex cs_dir_locks; +static std::map<std::string, std::unique_ptr<fsbridge::FileLock>> dir_locks GUARDED_BY(cs_dir_locks); bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only) { - std::lock_guard<std::mutex> ulock(cs_dir_locks); + LOCK(cs_dir_locks); fs::path pathLockFile = directory / lockfile_name; // If a lock for this directory already exists in the map, don't try to re-lock it @@ -110,13 +111,13 @@ bool LockDirectory(const fs::path& directory, const std::string lockfile_name, b void UnlockDirectory(const fs::path& directory, const std::string& lockfile_name) { - std::lock_guard<std::mutex> lock(cs_dir_locks); + LOCK(cs_dir_locks); dir_locks.erase((directory / lockfile_name).string()); } void ReleaseDirectoryLocks() { - std::lock_guard<std::mutex> ulock(cs_dir_locks); + LOCK(cs_dir_locks); dir_locks.clear(); } diff --git a/src/validation.cpp b/src/validation.cpp index dbdf5028fd..396fc0a1b5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5067,12 +5067,18 @@ bool LoadMempool(CTxMemPool& pool) pool.PrioritiseTransaction(i.first, i.second); } - std::set<uint256> unbroadcast_txids; - file >> unbroadcast_txids; - unbroadcast = unbroadcast_txids.size(); + // TODO: remove this try except in v0.22 + try { + std::set<uint256> unbroadcast_txids; + file >> unbroadcast_txids; + unbroadcast = unbroadcast_txids.size(); - for (const auto& txid : unbroadcast_txids) { + for (const auto& txid : unbroadcast_txids) { pool.AddUnbroadcastTx(txid); + } + } catch (const std::exception&) { + // mempool.dat files created prior to v0.21 will not have an + // unbroadcast set. No need to log a failure if parsing fails here. } } catch (const std::exception& e) { diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h index f59c63260e..f2df786e2e 100644 --- a/src/wallet/crypter.h +++ b/src/wallet/crypter.h @@ -43,15 +43,9 @@ public: //! such as the various parameters to scrypt std::vector<unsigned char> vchOtherDerivationParameters; - ADD_SERIALIZE_METHODS; - - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(vchCryptedKey); - READWRITE(vchSalt); - READWRITE(nDerivationMethod); - READWRITE(nDeriveIterations); - READWRITE(vchOtherDerivationParameters); + SERIALIZE_METHODS(CMasterKey, obj) + { + READWRITE(obj.vchCryptedKey, obj.vchSalt, obj.nDerivationMethod, obj.nDeriveIterations, obj.vchOtherDerivationParameters); } CMasterKey() diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 1b2bd83a4c..4ed28b0623 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -268,21 +268,14 @@ BerkeleyEnvironment::BerkeleyEnvironment() fMockDb = true; } -BerkeleyEnvironment::VerifyResult BerkeleyEnvironment::Verify(const std::string& strFile, recoverFunc_type recoverFunc, std::string& out_backup_filename) +bool BerkeleyEnvironment::Verify(const std::string& strFile) { LOCK(cs_db); assert(mapFileUseCount.count(strFile) == 0); Db db(dbenv.get(), 0); int result = db.verify(strFile.c_str(), nullptr, nullptr, 0); - if (result == 0) - return VerifyResult::VERIFY_OK; - else if (recoverFunc == nullptr) - return VerifyResult::RECOVER_FAIL; - - // Try to recover: - bool fRecovered = (*recoverFunc)(fs::path(strPath) / strFile, out_backup_filename); - return (fRecovered ? VerifyResult::RECOVER_OK : VerifyResult::RECOVER_FAIL); + return result == 0; } BerkeleyBatch::SafeDbt::SafeDbt() @@ -324,75 +317,6 @@ BerkeleyBatch::SafeDbt::operator Dbt*() return &m_dbt; } -bool BerkeleyBatch::Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) -{ - std::string filename; - std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, filename); - - // Recovery procedure: - // move wallet file to walletfilename.timestamp.bak - // Call Salvage with fAggressive=true to - // get as much data as possible. - // Rewrite salvaged data to fresh wallet file - // Set -rescan so any missing transactions will be - // found. - int64_t now = GetTime(); - newFilename = strprintf("%s.%d.bak", filename, now); - - int result = env->dbenv->dbrename(nullptr, filename.c_str(), nullptr, - newFilename.c_str(), DB_AUTO_COMMIT); - if (result == 0) - LogPrintf("Renamed %s to %s\n", filename, newFilename); - else - { - LogPrintf("Failed to rename %s to %s\n", filename, newFilename); - return false; - } - - std::vector<BerkeleyEnvironment::KeyValPair> salvagedData; - bool fSuccess = env->Salvage(newFilename, true, salvagedData); - if (salvagedData.empty()) - { - LogPrintf("Salvage(aggressive) found no records in %s.\n", newFilename); - return false; - } - LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size()); - - std::unique_ptr<Db> pdbCopy = MakeUnique<Db>(env->dbenv.get(), 0); - int ret = pdbCopy->open(nullptr, // Txn pointer - filename.c_str(), // Filename - "main", // Logical db name - DB_BTREE, // Database type - DB_CREATE, // Flags - 0); - if (ret > 0) { - LogPrintf("Cannot create database file %s\n", filename); - pdbCopy->close(0); - return false; - } - - DbTxn* ptxn = env->TxnBegin(); - for (BerkeleyEnvironment::KeyValPair& row : salvagedData) - { - if (recoverKVcallback) - { - CDataStream ssKey(row.first, SER_DISK, CLIENT_VERSION); - CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION); - if (!(*recoverKVcallback)(callbackDataIn, ssKey, ssValue)) - continue; - } - Dbt datKey(&row.first[0], row.first.size()); - Dbt datValue(&row.second[0], row.second.size()); - int ret2 = pdbCopy->put(ptxn, &datKey, &datValue, DB_NOOVERWRITE); - if (ret2 > 0) - fSuccess = false; - } - ptxn->commit(0); - pdbCopy->close(0); - - return fSuccess; -} - bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, bilingual_str& errorStr) { std::string walletFile; @@ -410,7 +334,7 @@ bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, bilingual_str& return true; } -bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::vector<bilingual_str>& warnings, bilingual_str& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc) +bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, bilingual_str& errorStr) { std::string walletFile; std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, walletFile); @@ -418,19 +342,8 @@ bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::vector<bi if (fs::exists(walletDir / walletFile)) { - std::string backup_filename; - BerkeleyEnvironment::VerifyResult r = env->Verify(walletFile, recoverFunc, backup_filename); - if (r == BerkeleyEnvironment::VerifyResult::RECOVER_OK) - { - warnings.push_back(strprintf(_("Warning: Wallet file corrupt, data salvaged!" - " Original %s saved as %s in %s; if" - " your balance or transactions are incorrect you should" - " restore from a backup."), - walletFile, backup_filename, walletDir)); - } - if (r == BerkeleyEnvironment::VerifyResult::RECOVER_FAIL) - { - errorStr = strprintf(_("%s corrupt, salvage failed"), walletFile); + if (!env->Verify(walletFile)) { + errorStr = strprintf(_("%s corrupt. Try using the wallet tool bitcoin-wallet to salvage or restoring a backup."), walletFile); return false; } } @@ -438,72 +351,6 @@ bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::vector<bi return true; } -/* End of headers, beginning of key/value data */ -static const char *HEADER_END = "HEADER=END"; -/* End of key/value data */ -static const char *DATA_END = "DATA=END"; - -bool BerkeleyEnvironment::Salvage(const std::string& strFile, bool fAggressive, std::vector<BerkeleyEnvironment::KeyValPair>& vResult) -{ - LOCK(cs_db); - assert(mapFileUseCount.count(strFile) == 0); - - u_int32_t flags = DB_SALVAGE; - if (fAggressive) - flags |= DB_AGGRESSIVE; - - std::stringstream strDump; - - Db db(dbenv.get(), 0); - int result = db.verify(strFile.c_str(), nullptr, &strDump, flags); - if (result == DB_VERIFY_BAD) { - LogPrintf("BerkeleyEnvironment::Salvage: Database salvage found errors, all data may not be recoverable.\n"); - if (!fAggressive) { - LogPrintf("BerkeleyEnvironment::Salvage: Rerun with aggressive mode to ignore errors and continue.\n"); - return false; - } - } - if (result != 0 && result != DB_VERIFY_BAD) { - LogPrintf("BerkeleyEnvironment::Salvage: Database salvage failed with result %d.\n", result); - return false; - } - - // Format of bdb dump is ascii lines: - // header lines... - // HEADER=END - // hexadecimal key - // hexadecimal value - // ... repeated - // DATA=END - - std::string strLine; - while (!strDump.eof() && strLine != HEADER_END) - getline(strDump, strLine); // Skip past header - - std::string keyHex, valueHex; - while (!strDump.eof() && keyHex != DATA_END) { - getline(strDump, keyHex); - if (keyHex != DATA_END) { - if (strDump.eof()) - break; - getline(strDump, valueHex); - if (valueHex == DATA_END) { - LogPrintf("BerkeleyEnvironment::Salvage: WARNING: Number of keys in data does not match number of values.\n"); - break; - } - vResult.push_back(make_pair(ParseHex(keyHex), ParseHex(valueHex))); - } - } - - if (keyHex != DATA_END) { - LogPrintf("BerkeleyEnvironment::Salvage: WARNING: Unexpected end of file while reading salvage output.\n"); - return false; - } - - return (result == 0); -} - - void BerkeleyEnvironment::CheckpointLSN(const std::string& strFile) { dbenv->txn_checkpoint(0, 0, 0); diff --git a/src/wallet/db.h b/src/wallet/db.h index 37f96a1a96..54ce144ffc 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -66,26 +66,7 @@ public: bool IsDatabaseLoaded(const std::string& db_filename) const { return m_databases.find(db_filename) != m_databases.end(); } fs::path Directory() const { return strPath; } - /** - * Verify that database file strFile is OK. If it is not, - * call the callback to try to recover. - * This must be called BEFORE strFile is opened. - * Returns true if strFile is OK. - */ - enum class VerifyResult { VERIFY_OK, - RECOVER_OK, - RECOVER_FAIL }; - typedef bool (*recoverFunc_type)(const fs::path& file_path, std::string& out_backup_filename); - VerifyResult Verify(const std::string& strFile, recoverFunc_type recoverFunc, std::string& out_backup_filename); - /** - * Salvage data from a file that Verify says is bad. - * fAggressive sets the DB_AGGRESSIVE flag (see berkeley DB->verify() method documentation). - * Appends binary key/value pairs to vResult, returns true if successful. - * NOTE: reads the entire database into memory, so cannot be used - * for huge databases. - */ - typedef std::pair<std::vector<unsigned char>, std::vector<unsigned char> > KeyValPair; - bool Salvage(const std::string& strFile, bool fAggressive, std::vector<KeyValPair>& vResult); + bool Verify(const std::string& strFile); bool Open(bool retry); void Close(); @@ -245,7 +226,6 @@ public: void Flush(); void Close(); - static bool Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); /* flush the wallet passively (TRY_LOCK) ideal to be called periodically */ @@ -253,7 +233,7 @@ public: /* verifies the database environment */ static bool VerifyEnvironment(const fs::path& file_path, bilingual_str& errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const fs::path& file_path, std::vector<bilingual_str>& warnings, bilingual_str& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc); + static bool VerifyDatabaseFile(const fs::path& file_path, bilingual_str& errorStr); template <typename K, typename T> bool Read(const K& key, T& value) diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 6f973aab1c..3885eb6185 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -54,7 +54,6 @@ void WalletInit::AddWalletOptions() const gArgs.AddArg("-paytxfee=<amt>", strprintf("Fee (in %s/kB) to add to transactions you send (default: %s)", CURRENCY_UNIT, FormatMoney(CFeeRate{DEFAULT_PAY_TX_FEE}.GetFeePerK())), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); gArgs.AddArg("-rescan", "Rescan the block chain for missing wallet transactions on startup", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); - gArgs.AddArg("-salvagewallet", "Attempt to recover private keys from a corrupt wallet on startup", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); gArgs.AddArg("-spendzeroconfchange", strprintf("Spend unconfirmed change when sending transactions (default: %u)", DEFAULT_SPEND_ZEROCONF_CHANGE), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); gArgs.AddArg("-txconfirmtarget=<n>", strprintf("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)", DEFAULT_TX_CONFIRM_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); gArgs.AddArg("-wallet=<path>", "Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to <walletdir> if it is not absolute, and will be created if it does not exist (as a directory containing a wallet.dat file and log files). For backwards compatibility this will also accept names of existing data files in <walletdir>.)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET); @@ -89,16 +88,6 @@ bool WalletInit::ParameterInteraction() const LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__); } - if (gArgs.GetBoolArg("-salvagewallet", false)) { - if (is_multiwallet) { - return InitError(strprintf(Untranslated("%s is only allowed with a single wallet file"), "-salvagewallet")); - } - // Rewrite just private keys: rescan to find transactions - if (gArgs.SoftSetBoolArg("-rescan", true)) { - LogPrintf("%s: parameter interaction: -salvagewallet=1 -> setting -rescan=1\n", __func__); - } - } - bool zapwallettxes = gArgs.GetBoolArg("-zapwallettxes", false); // -zapwallettxes implies dropping the mempool on startup if (zapwallettxes && gArgs.SoftSetBoolArg("-persistmempool", false)) { diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 16f3699d37..8df3e78215 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -37,11 +37,6 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal chain.initMessage(_("Verifying wallet(s)...").translated); - // Parameter interaction code should have thrown an error if -salvagewallet - // was enabled with more than wallet file, so the wallet_files size check - // here should have no effect. - bool salvage_wallet = gArgs.GetBoolArg("-salvagewallet", false) && wallet_files.size() <= 1; - // Keep track of each wallet absolute path to detect duplicates. std::set<fs::path> wallet_paths; @@ -55,7 +50,7 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal bilingual_str error_string; std::vector<bilingual_str> warnings; - bool verify_success = CWallet::Verify(chain, location, salvage_wallet, error_string, warnings); + bool verify_success = CWallet::Verify(chain, location, error_string, warnings); if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n"))); if (!verify_success) { chain.initError(error_string); diff --git a/src/wallet/load.h b/src/wallet/load.h index 5a62e29303..e24b1f2e69 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -16,8 +16,6 @@ class Chain; } // namespace interfaces //! Responsible for reading and validating the -wallet arguments and verifying the wallet database. -//! This function will perform salvage on the wallet if requested, as long as only one wallet is -//! being loaded (WalletInit::ParameterInteraction() forbids -salvagewallet, -zapwallettxes or -upgradewallet with multiwallet). bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files); //! Load wallet databases. diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 7bf3d169c3..d5f6d63a46 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -746,7 +746,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now wallet.BlockUntilSyncedToCurrentChain(); - LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); + LOCK2(wallet.cs_wallet, spk_man.cs_KeyStore); EnsureWalletIsUnlocked(&wallet); @@ -769,7 +769,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) std::map<CKeyID, int64_t> mapKeyBirth; const std::map<CKeyID, int64_t>& mapKeyPool = spk_man.GetAllReserveKeys(); - pwallet->GetKeyBirthTimes(mapKeyBirth); + wallet.GetKeyBirthTimes(mapKeyBirth); std::set<CScriptID> scripts = spk_man.GetCScripts(); diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp new file mode 100644 index 0000000000..70067ebef0 --- /dev/null +++ b/src/wallet/salvage.cpp @@ -0,0 +1,150 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <fs.h> +#include <streams.h> +#include <wallet/salvage.h> +#include <wallet/wallet.h> +#include <wallet/walletdb.h> + +/* End of headers, beginning of key/value data */ +static const char *HEADER_END = "HEADER=END"; +/* End of key/value data */ +static const char *DATA_END = "DATA=END"; +typedef std::pair<std::vector<unsigned char>, std::vector<unsigned char> > KeyValPair; + +bool RecoverDatabaseFile(const fs::path& file_path) +{ + std::string filename; + std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, filename); + + // Recovery procedure: + // move wallet file to walletfilename.timestamp.bak + // Call Salvage with fAggressive=true to + // get as much data as possible. + // Rewrite salvaged data to fresh wallet file + // Set -rescan so any missing transactions will be + // found. + int64_t now = GetTime(); + std::string newFilename = strprintf("%s.%d.bak", filename, now); + + int result = env->dbenv->dbrename(nullptr, filename.c_str(), nullptr, + newFilename.c_str(), DB_AUTO_COMMIT); + if (result == 0) + LogPrintf("Renamed %s to %s\n", filename, newFilename); + else + { + LogPrintf("Failed to rename %s to %s\n", filename, newFilename); + return false; + } + + /** + * Salvage data from a file. The DB_AGGRESSIVE flag is being used (see berkeley DB->verify() method documentation). + * key/value pairs are appended to salvagedData which are then written out to a new wallet file. + * NOTE: reads the entire database into memory, so cannot be used + * for huge databases. + */ + std::vector<KeyValPair> salvagedData; + + std::stringstream strDump; + + Db db(env->dbenv.get(), 0); + result = db.verify(newFilename.c_str(), nullptr, &strDump, DB_SALVAGE | DB_AGGRESSIVE); + if (result == DB_VERIFY_BAD) { + LogPrintf("Salvage: Database salvage found errors, all data may not be recoverable.\n"); + } + if (result != 0 && result != DB_VERIFY_BAD) { + LogPrintf("Salvage: Database salvage failed with result %d.\n", result); + return false; + } + + // Format of bdb dump is ascii lines: + // header lines... + // HEADER=END + // hexadecimal key + // hexadecimal value + // ... repeated + // DATA=END + + std::string strLine; + while (!strDump.eof() && strLine != HEADER_END) + getline(strDump, strLine); // Skip past header + + std::string keyHex, valueHex; + while (!strDump.eof() && keyHex != DATA_END) { + getline(strDump, keyHex); + if (keyHex != DATA_END) { + if (strDump.eof()) + break; + getline(strDump, valueHex); + if (valueHex == DATA_END) { + LogPrintf("Salvage: WARNING: Number of keys in data does not match number of values.\n"); + break; + } + salvagedData.push_back(make_pair(ParseHex(keyHex), ParseHex(valueHex))); + } + } + + bool fSuccess; + if (keyHex != DATA_END) { + LogPrintf("Salvage: WARNING: Unexpected end of file while reading salvage output.\n"); + fSuccess = false; + } else { + fSuccess = (result == 0); + } + + if (salvagedData.empty()) + { + LogPrintf("Salvage(aggressive) found no records in %s.\n", newFilename); + return false; + } + LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size()); + + std::unique_ptr<Db> pdbCopy = MakeUnique<Db>(env->dbenv.get(), 0); + int ret = pdbCopy->open(nullptr, // Txn pointer + filename.c_str(), // Filename + "main", // Logical db name + DB_BTREE, // Database type + DB_CREATE, // Flags + 0); + if (ret > 0) { + LogPrintf("Cannot create database file %s\n", filename); + pdbCopy->close(0); + return false; + } + + DbTxn* ptxn = env->TxnBegin(); + CWallet dummyWallet(nullptr, WalletLocation(), WalletDatabase::CreateDummy()); + for (KeyValPair& row : salvagedData) + { + /* Filter for only private key type KV pairs to be added to the salvaged wallet */ + CDataStream ssKey(row.first, SER_DISK, CLIENT_VERSION); + CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION); + std::string strType, strErr; + bool fReadOK; + { + // Required in LoadKeyMetadata(): + LOCK(dummyWallet.cs_wallet); + fReadOK = ReadKeyValue(&dummyWallet, ssKey, ssValue, strType, strErr); + } + if (!WalletBatch::IsKeyType(strType) && strType != DBKeys::HDCHAIN) { + continue; + } + if (!fReadOK) + { + LogPrintf("WARNING: WalletBatch::Recover skipping %s: %s\n", strType, strErr); + continue; + } + Dbt datKey(&row.first[0], row.first.size()); + Dbt datValue(&row.second[0], row.second.size()); + int ret2 = pdbCopy->put(ptxn, &datKey, &datValue, DB_NOOVERWRITE); + if (ret2 > 0) + fSuccess = false; + } + ptxn->commit(0); + pdbCopy->close(0); + + return fSuccess; +} diff --git a/src/wallet/salvage.h b/src/wallet/salvage.h new file mode 100644 index 0000000000..e361930f5e --- /dev/null +++ b/src/wallet/salvage.h @@ -0,0 +1,14 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_WALLET_SALVAGE_H +#define BITCOIN_WALLET_SALVAGE_H + +#include <fs.h> +#include <streams.h> + +bool RecoverDatabaseFile(const fs::path& file_path); + +#endif // BITCOIN_WALLET_SALVAGE_H diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 2f6245bbe2..d62d30f339 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -112,36 +112,37 @@ public: CKeyPool(); CKeyPool(const CPubKey& vchPubKeyIn, bool internalIn); - ADD_SERIALIZE_METHODS; + template<typename Stream> + void Serialize(Stream& s) const + { + int nVersion = s.GetVersion(); + if (!(s.GetType() & SER_GETHASH)) { + s << nVersion; + } + s << nTime << vchPubKey << fInternal << m_pre_split; + } - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) { + template<typename Stream> + void Unserialize(Stream& s) + { int nVersion = s.GetVersion(); - if (!(s.GetType() & SER_GETHASH)) - READWRITE(nVersion); - READWRITE(nTime); - READWRITE(vchPubKey); - if (ser_action.ForRead()) { - try { - READWRITE(fInternal); - } - catch (std::ios_base::failure&) { - /* flag as external address if we can't read the internal boolean - (this will be the case for any wallet before the HD chain split version) */ - fInternal = false; - } - try { - READWRITE(m_pre_split); - } - catch (std::ios_base::failure&) { - /* flag as postsplit address if we can't read the m_pre_split boolean - (this will be the case for any wallet that upgrades to HD chain split)*/ - m_pre_split = false; - } + if (!(s.GetType() & SER_GETHASH)) { + s >> nVersion; + } + s >> nTime >> vchPubKey; + try { + s >> fInternal; + } catch (std::ios_base::failure&) { + /* flag as external address if we can't read the internal boolean + (this will be the case for any wallet before the HD chain split version) */ + fInternal = false; } - else { - READWRITE(fInternal); - READWRITE(m_pre_split); + try { + s >> m_pre_split; + } catch (std::ios_base::failure&) { + /* flag as postsplit address if we can't read the m_pre_split boolean + (this will be the case for any wallet that upgrades to HD chain split) */ + m_pre_split = false; } } }; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 255770552f..7824563254 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -153,7 +153,7 @@ void UnloadWallet(std::shared_ptr<CWallet>&& wallet) std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& warnings) { try { - if (!CWallet::Verify(chain, location, false, error, warnings)) { + if (!CWallet::Verify(chain, location, error, warnings)) { error = Untranslated("Wallet file verification failed.") + Untranslated(" ") + error; return nullptr; } @@ -195,7 +195,7 @@ WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& } // Wallet::Verify will check if we're trying to create a wallet with a duplicate name. - if (!CWallet::Verify(chain, location, false, error, warnings)) { + if (!CWallet::Verify(chain, location, error, warnings)) { error = Untranslated("Wallet file verification failed.") + Untranslated(" ") + error; return WalletCreationStatus::CREATION_FAILED; } @@ -3650,7 +3650,7 @@ std::vector<std::string> CWallet::GetDestValues(const std::string& prefix) const return values; } -bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, bool salvage_wallet, bilingual_str& error_string, std::vector<bilingual_str>& warnings) +bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error_string, std::vector<bilingual_str>& warnings) { // Do some checking on wallet path. It should be either a: // @@ -3690,16 +3690,7 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b return false; } - if (salvage_wallet) { - // Recover readable keypairs: - CWallet dummyWallet(&chain, WalletLocation(), WalletDatabase::CreateDummy()); - std::string backup_filename; - if (!WalletBatch::Recover(wallet_path, (void *)&dummyWallet, WalletBatch::RecoverKeysOnlyFilter, backup_filename)) { - return false; - } - } - - return WalletBatch::VerifyDatabaseFile(wallet_path, warnings, error_string); + return WalletBatch::VerifyDatabaseFile(wallet_path, error_string); } std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& warnings, uint64_t wallet_creation_flags) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 9d312e8ee5..e3141baef0 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -631,7 +631,6 @@ private: std::atomic<bool> fScanningWallet{false}; // controlled by WalletRescanReserver std::atomic<int64_t> m_scanning_start{0}; std::atomic<double> m_scanning_progress{0}; - std::mutex mutexScanning; friend class WalletRescanReserver; //! the current wallet version: clients below this version are not able to load the wallet @@ -1136,7 +1135,7 @@ public: bool MarkReplaced(const uint256& originalHash, const uint256& newHash); //! Verify wallet naming and perform salvage on the wallet if required - static bool Verify(interfaces::Chain& chain, const WalletLocation& location, bool salvage_wallet, bilingual_str& error_string, std::vector<bilingual_str>& warnings); + static bool Verify(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error_string, std::vector<bilingual_str>& warnings); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ static std::shared_ptr<CWallet> CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& warnings, uint64_t wallet_creation_flags = 0); @@ -1287,13 +1286,11 @@ public: bool reserve() { assert(!m_could_reserve); - std::lock_guard<std::mutex> lock(m_wallet.mutexScanning); - if (m_wallet.fScanningWallet) { + if (m_wallet.fScanningWallet.exchange(true)) { return false; } m_wallet.m_scanning_start = GetTimeMillis(); m_wallet.m_scanning_progress = 0; - m_wallet.fScanningWallet = true; m_could_reserve = true; return true; } @@ -1305,7 +1302,6 @@ public: ~WalletRescanReserver() { - std::lock_guard<std::mutex> lock(m_wallet.mutexScanning); if (m_could_reserve) { m_wallet.fScanningWallet = false; } diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 331408ef48..e7adbfea77 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -672,6 +672,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, return true; } +bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr) +{ + CWalletScanState dummy_wss; + LOCK(pwallet->cs_wallet); + return ReadKeyValue(pwallet, ssKey, ssValue, dummy_wss, strType, strErr); +} + bool WalletBatch::IsKeyType(const std::string& strType) { return (strType == DBKeys::KEY || @@ -976,53 +983,14 @@ void MaybeCompactWalletDB() fOneThread = false; } -// -// Try to (very carefully!) recover wallet file if there is a problem. -// -bool WalletBatch::Recover(const fs::path& wallet_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename) -{ - return BerkeleyBatch::Recover(wallet_path, callbackDataIn, recoverKVcallback, out_backup_filename); -} - -bool WalletBatch::Recover(const fs::path& wallet_path, std::string& out_backup_filename) -{ - // recover without a key filter callback - // results in recovering all record types - return WalletBatch::Recover(wallet_path, nullptr, nullptr, out_backup_filename); -} - -bool WalletBatch::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue) -{ - CWallet *dummyWallet = reinterpret_cast<CWallet*>(callbackData); - CWalletScanState dummyWss; - std::string strType, strErr; - bool fReadOK; - { - // Required in LoadKeyMetadata(): - LOCK(dummyWallet->cs_wallet); - fReadOK = ReadKeyValue(dummyWallet, ssKey, ssValue, - dummyWss, strType, strErr); - } - if (!IsKeyType(strType) && strType != DBKeys::HDCHAIN) { - return false; - } - if (!fReadOK) - { - LogPrintf("WARNING: WalletBatch::Recover skipping %s: %s\n", strType, strErr); - return false; - } - - return true; -} - bool WalletBatch::VerifyEnvironment(const fs::path& wallet_path, bilingual_str& errorStr) { return BerkeleyBatch::VerifyEnvironment(wallet_path, errorStr); } -bool WalletBatch::VerifyDatabaseFile(const fs::path& wallet_path, std::vector<bilingual_str>& warnings, bilingual_str& errorStr) +bool WalletBatch::VerifyDatabaseFile(const fs::path& wallet_path, bilingual_str& errorStr) { - return BerkeleyBatch::VerifyDatabaseFile(wallet_path, warnings, errorStr, WalletBatch::Recover); + return BerkeleyBatch::VerifyDatabaseFile(wallet_path, errorStr); } bool WalletBatch::WriteDestData(const std::string &address, const std::string &key, const std::string &value) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index ee1a8cd5b2..b95ed24d12 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -98,15 +98,13 @@ public: int nVersion; CHDChain() { SetNull(); } - ADD_SERIALIZE_METHODS; - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) + + SERIALIZE_METHODS(CHDChain, obj) { - READWRITE(this->nVersion); - READWRITE(nExternalChainCounter); - READWRITE(seed_id); - if (this->nVersion >= VERSION_HD_CHAIN_SPLIT) - READWRITE(nInternalChainCounter); + READWRITE(obj.nVersion, obj.nExternalChainCounter, obj.seed_id); + if (obj.nVersion >= VERSION_HD_CHAIN_SPLIT) { + READWRITE(obj.nInternalChainCounter); + } } void SetNull() @@ -147,21 +145,16 @@ public: nCreateTime = nCreateTime_; } - ADD_SERIALIZE_METHODS; - - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(this->nVersion); - READWRITE(nCreateTime); - if (this->nVersion >= VERSION_WITH_HDDATA) - { - READWRITE(hdKeypath); - READWRITE(hd_seed_id); + SERIALIZE_METHODS(CKeyMetadata, obj) + { + READWRITE(obj.nVersion, obj.nCreateTime); + if (obj.nVersion >= VERSION_WITH_HDDATA) { + READWRITE(obj.hdKeypath, obj.hd_seed_id); } - if (this->nVersion >= VERSION_WITH_KEY_ORIGIN) + if (obj.nVersion >= VERSION_WITH_KEY_ORIGIN) { - READWRITE(key_origin); - READWRITE(has_key_origin); + READWRITE(obj.key_origin); + READWRITE(obj.has_key_origin); } } @@ -268,18 +261,12 @@ public: DBErrors FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWalletTx>& vWtx); DBErrors ZapWalletTx(std::list<CWalletTx>& vWtx); DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut); - /* Try to (very carefully!) recover wallet database (with a possible key type filter) */ - static bool Recover(const fs::path& wallet_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); - /* Recover convenience-function to bypass the key filter callback, called when verify fails, recovers everything */ - static bool Recover(const fs::path& wallet_path, std::string& out_backup_filename); - /* Recover filter (used as callback), will only let keys (cryptographical keys) as KV/key-type pass through */ - static bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue); /* Function to determine if a certain KV/key-type is a key (cryptographical key) type */ static bool IsKeyType(const std::string& strType); /* verifies the database environment */ static bool VerifyEnvironment(const fs::path& wallet_path, bilingual_str& errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const fs::path& wallet_path, std::vector<bilingual_str>& warnings, bilingual_str& errorStr); + static bool VerifyDatabaseFile(const fs::path& wallet_path, bilingual_str& errorStr); //! write the hdchain model (external chain child index counter) bool WriteHDChain(const CHDChain& chain); @@ -299,4 +286,7 @@ private: //! Compacts BDB state so that wallet.dat is self-contained (if there are changes) void MaybeCompactWalletDB(); +//! Unserialize a given Key-Value pair and load it into the wallet +bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr); + #endif // BITCOIN_WALLET_WALLETDB_H diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 522efaa884..be07c28503 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -5,6 +5,7 @@ #include <fs.h> #include <util/system.h> #include <util/translation.h> +#include <wallet/salvage.h> #include <wallet/wallet.h> #include <wallet/walletutil.h> @@ -103,6 +104,27 @@ static void WalletShowInfo(CWallet* wallet_instance) tfm::format(std::cout, "Address Book: %zu\n", wallet_instance->m_address_book.size()); } +static bool SalvageWallet(const fs::path& path) +{ + // Create a Database handle to allow for the db to be initialized before recovery + std::unique_ptr<WalletDatabase> database = WalletDatabase::Create(path); + + // Initialize the environment before recovery + bilingual_str error_string; + try { + WalletBatch::VerifyEnvironment(path, error_string); + } catch (const fs::filesystem_error& e) { + error_string = Untranslated(strprintf("Error loading wallet. %s", fsbridge::get_filesystem_error_message(e))); + } + if (!error_string.original.empty()) { + tfm::format(std::cerr, "Failed to open wallet for salvage :%s\n", error_string.original); + return false; + } + + // Perform the recovery + return RecoverDatabaseFile(path); +} + bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) { fs::path path = fs::absolute(name, GetWalletDir()); @@ -113,7 +135,7 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) WalletShowInfo(wallet_instance.get()); wallet_instance->Flush(true); } - } else if (command == "info") { + } else if (command == "info" || command == "salvage") { if (!fs::exists(path)) { tfm::format(std::cerr, "Error: no wallet file at %s\n", name); return false; @@ -123,10 +145,15 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) tfm::format(std::cerr, "%s\nError loading %s. Is wallet being used by other process?\n", error.original, name); return false; } - std::shared_ptr<CWallet> wallet_instance = LoadWallet(name, path); - if (!wallet_instance) return false; - WalletShowInfo(wallet_instance.get()); - wallet_instance->Flush(true); + + if (command == "info") { + std::shared_ptr<CWallet> wallet_instance = LoadWallet(name, path); + if (!wallet_instance) return false; + WalletShowInfo(wallet_instance.get()); + wallet_instance->Flush(true); + } else if (command == "salvage") { + return SalvageWallet(path); + } } else { tfm::format(std::cerr, "Invalid command: %s\n", command); return false; diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h index 599b1a9f5a..a4e4fda8a1 100644 --- a/src/wallet/walletutil.h +++ b/src/wallet/walletutil.h @@ -98,26 +98,22 @@ public: int32_t next_index = 0; // Position of the next item to generate DescriptorCache cache; - ADD_SERIALIZE_METHODS; - - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) { - if (ser_action.ForRead()) { - std::string desc; - std::string error; - READWRITE(desc); - FlatSigningProvider keys; - descriptor = Parse(desc, keys, error, true); - if (!descriptor) { - throw std::ios_base::failure("Invalid descriptor: " + error); - } - } else { - READWRITE(descriptor->ToString()); + void DeserializeDescriptor(const std::string& str) + { + std::string error; + FlatSigningProvider keys; + descriptor = Parse(str, keys, error, true); + if (!descriptor) { + throw std::ios_base::failure("Invalid descriptor: " + error); } - READWRITE(creation_time); - READWRITE(next_index); - READWRITE(range_start); - READWRITE(range_end); + } + + SERIALIZE_METHODS(WalletDescriptor, obj) + { + std::string descriptor_str; + SER_WRITE(obj, descriptor_str = obj.descriptor->ToString()); + READWRITE(descriptor_str, obj.creation_time, obj.next_index, obj.range_start, obj.range_end); + SER_READ(obj, obj.DeserializeDescriptor(descriptor_str)); } WalletDescriptor() {} diff --git a/test/config.ini.in b/test/config.ini.in index 9687206ee1..be1bfe8752 100644 --- a/test/config.ini.in +++ b/test/config.ini.in @@ -7,6 +7,7 @@ [environment] PACKAGE_NAME=@PACKAGE_NAME@ +PACKAGE_BUGREPORT=@PACKAGE_BUGREPORT@ SRCDIR=@abs_top_srcdir@ BUILDDIR=@abs_top_builddir@ EXEEXT=@EXEEXT@ diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py index 5bbdb8cda1..7b38e09bf9 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -256,7 +256,11 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): self.log.debug("Mining longer tip") block_hashes = [] while current_height + 1 > self.nodes[3].getblockcount(): - block_hashes.extend(self.nodes[3].generate(min(10, current_height + 1 - self.nodes[3].getblockcount()))) + block_hashes.extend(self.nodes[3].generatetoaddress( + nblocks=min(10, current_height + 1 - self.nodes[3].getblockcount()), + # new address to avoid mining a block that has just been invalidated + address=self.nodes[3].getnewaddress(), + )) self.log.debug("Syncing %d new blocks...", len(block_hashes)) self.sync_node3blocks(block_hashes) utxo_list = self.nodes[3].listunspent() @@ -281,5 +285,6 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): if self.restart_counts[i] == 0: self.log.warning("Node %d never crashed during utxo flush!", i) + if __name__ == "__main__": ChainstateWriteCrashTest().main() diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index 1c94305220..7530e7daf6 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -67,6 +67,7 @@ class TestBitcoinCli(BitcoinTestFramework): if self.is_wallet_compiled(): self.log.info("Test -getinfo and bitcoin-cli getwalletinfo return expected wallet info") assert_equal(cli_get_info['balance'], BALANCE) + assert 'balances' not in cli_get_info.keys() wallet_info = self.nodes[0].getwalletinfo() assert_equal(cli_get_info['keypoolsize'], wallet_info['keypoolsize']) assert_equal(cli_get_info['unlocked_until'], wallet_info['unlocked_until']) @@ -76,42 +77,60 @@ class TestBitcoinCli(BitcoinTestFramework): # Setup to test -getinfo and -rpcwallet= with multiple wallets. wallets = ['', 'Encrypted', 'secret'] - amounts = [Decimal('59.999928'), Decimal(9), Decimal(31)] + amounts = [BALANCE + Decimal('9.999928'), Decimal(9), Decimal(31)] self.nodes[0].createwallet(wallet_name=wallets[1]) self.nodes[0].createwallet(wallet_name=wallets[2]) w1 = self.nodes[0].get_wallet_rpc(wallets[0]) w2 = self.nodes[0].get_wallet_rpc(wallets[1]) w3 = self.nodes[0].get_wallet_rpc(wallets[2]) w1.walletpassphrase(password, self.rpc_timeout) + w2.encryptwallet(password) w1.sendtoaddress(w2.getnewaddress(), amounts[1]) w1.sendtoaddress(w3.getnewaddress(), amounts[2]) # Mine a block to confirm; adds a block reward (50 BTC) to the default wallet. self.nodes[0].generate(1) - self.log.info("Test -getinfo with multiple wallets loaded returns no balance") - assert_equal(set(self.nodes[0].listwallets()), set(wallets)) - assert 'balance' not in self.nodes[0].cli('-getinfo').send_cli().keys() - self.log.info("Test -getinfo with multiple wallets and -rpcwallet returns specified wallet balance") for i in range(len(wallets)): - cli_get_info = self.nodes[0].cli('-getinfo').send_cli('-rpcwallet={}'.format(wallets[i])) + cli_get_info = self.nodes[0].cli('-getinfo', '-rpcwallet={}'.format(wallets[i])).send_cli() + assert 'balances' not in cli_get_info.keys() assert_equal(cli_get_info['balance'], amounts[i]) - self.log.info("Test -getinfo with multiple wallets and -rpcwallet=non-existing-wallet returns no balance") - assert 'balance' not in self.nodes[0].cli('-getinfo').send_cli('-rpcwallet=does-not-exist').keys() + self.log.info("Test -getinfo with multiple wallets and -rpcwallet=non-existing-wallet returns no balances") + cli_get_info_keys = self.nodes[0].cli('-getinfo', '-rpcwallet=does-not-exist').send_cli().keys() + assert 'balance' not in cli_get_info_keys + assert 'balances' not in cli_get_info_keys - self.log.info("Test -getinfo after unloading all wallets except a non-default one returns its balance") + self.log.info("Test -getinfo with multiple wallets returns all loaded wallet names and balances") + assert_equal(set(self.nodes[0].listwallets()), set(wallets)) + cli_get_info = self.nodes[0].cli('-getinfo').send_cli() + assert 'balance' not in cli_get_info.keys() + assert_equal(cli_get_info['balances'], {k: v for k, v in zip(wallets, amounts)}) + + # Unload the default wallet and re-verify. self.nodes[0].unloadwallet(wallets[0]) + assert wallets[0] not in self.nodes[0].listwallets() + cli_get_info = self.nodes[0].cli('-getinfo').send_cli() + assert 'balance' not in cli_get_info.keys() + assert_equal(cli_get_info['balances'], {k: v for k, v in zip(wallets[1:], amounts[1:])}) + + self.log.info("Test -getinfo after unloading all wallets except a non-default one returns its balance") self.nodes[0].unloadwallet(wallets[2]) assert_equal(self.nodes[0].listwallets(), [wallets[1]]) - assert_equal(self.nodes[0].cli('-getinfo').send_cli()['balance'], amounts[1]) - - self.log.info("Test -getinfo -rpcwallet=remaining-non-default-wallet returns its balance") - assert_equal(self.nodes[0].cli('-getinfo').send_cli('-rpcwallet={}'.format(wallets[1]))['balance'], amounts[1]) - - self.log.info("Test -getinfo with -rpcwallet=unloaded wallet returns no balance") - assert 'balance' not in self.nodes[0].cli('-getinfo').send_cli('-rpcwallet={}'.format(wallets[2])).keys() + cli_get_info = self.nodes[0].cli('-getinfo').send_cli() + assert 'balances' not in cli_get_info.keys() + assert_equal(cli_get_info['balance'], amounts[1]) + + self.log.info("Test -getinfo with -rpcwallet=remaining-non-default-wallet returns only its balance") + cli_get_info = self.nodes[0].cli('-getinfo', '-rpcwallet={}'.format(wallets[1])).send_cli() + assert 'balances' not in cli_get_info.keys() + assert_equal(cli_get_info['balance'], amounts[1]) + + self.log.info("Test -getinfo with -rpcwallet=unloaded wallet returns no balances") + cli_get_info = self.nodes[0].cli('-getinfo', '-rpcwallet={}'.format(wallets[2])).send_cli() + assert 'balance' not in cli_get_info_keys + assert 'balances' not in cli_get_info_keys else: self.log.info("*** Wallet not compiled; cli getwalletinfo and -getinfo wallet tests skipped") self.nodes[0].generate(1) # maintain block parity with the wallet_compiled conditional branch diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index 3969da2eb0..5d00648aed 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -84,7 +84,9 @@ class MempoolPersistTest(BitcoinTestFramework): assert_greater_than_or_equal(tx_creation_time_higher, tx_creation_time) # disconnect nodes & make a txn that remains in the unbroadcast set. - disconnect_nodes(self.nodes[0], 2) + disconnect_nodes(self.nodes[0], 1) + assert(len(self.nodes[0].getpeerinfo()) == 0) + assert(len(self.nodes[0].p2ps) == 0) self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), Decimal("12")) connect_nodes(self.nodes[0], 2) @@ -157,8 +159,10 @@ class MempoolPersistTest(BitcoinTestFramework): # clear out mempool node0.generate(1) - # disconnect nodes to make a txn that remains in the unbroadcast set. - disconnect_nodes(node0, 1) + # ensure node0 doesn't have any connections + # make a transaction that will remain in the unbroadcast set + assert(len(node0.getpeerinfo()) == 0) + assert(len(node0.p2ps) == 0) node0.sendtoaddress(self.nodes[1].getnewaddress(), Decimal("12")) # shutdown, then startup with wallet disabled diff --git a/test/functional/mempool_unbroadcast.py b/test/functional/mempool_unbroadcast.py index dedf5b8a47..365d011157 100755 --- a/test/functional/mempool_unbroadcast.py +++ b/test/functional/mempool_unbroadcast.py @@ -16,6 +16,7 @@ from test_framework.util import ( disconnect_nodes, ) +MAX_INITIAL_BROADCAST_DELAY = 15 * 60 # 15 minutes in seconds class MempoolUnbroadcastTest(BitcoinTestFramework): def set_test_params(self): @@ -72,7 +73,7 @@ class MempoolUnbroadcastTest(BitcoinTestFramework): connect_nodes(node, 1) # fast forward into the future & ensure that the second node has the txns - node.mockscheduler(15 * 60) # 15 min in seconds + node.mockscheduler(MAX_INITIAL_BROADCAST_DELAY) self.sync_mempools(timeout=30) mempool = self.nodes[1].getrawmempool() assert rpc_tx_hsh in mempool @@ -86,15 +87,16 @@ class MempoolUnbroadcastTest(BitcoinTestFramework): self.log.info("Add another connection & ensure transactions aren't broadcast again") conn = node.add_p2p_connection(P2PTxInvStore()) - node.mockscheduler(15 * 60) - time.sleep(5) + node.mockscheduler(MAX_INITIAL_BROADCAST_DELAY) + time.sleep(2) # allow sufficient time for possibility of broadcast assert_equal(len(conn.get_invs()), 0) + disconnect_nodes(node, 1) + node.disconnect_p2ps() + def test_txn_removal(self): self.log.info("Test that transactions removed from mempool are removed from unbroadcast set") node = self.nodes[0] - disconnect_nodes(node, 1) - node.disconnect_p2ps # since the node doesn't have any connections, it will not receive # any GETDATAs & thus the transaction will remain in the unbroadcast set. diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py index 4d00a6dc07..6d947ac660 100755 --- a/test/functional/p2p_blockfilters.py +++ b/test/functional/p2p_blockfilters.py @@ -5,12 +5,17 @@ """Tests NODE_COMPACT_FILTERS (BIP 157/158). Tests that a node configured with -blockfilterindex and -peerblockfilters can serve -cfcheckpts. +cfheaders and cfcheckpts. """ from test_framework.messages import ( FILTER_TYPE_BASIC, + hash256, msg_getcfcheckpt, + msg_getcfheaders, + msg_getcfilters, + ser_uint256, + uint256_from_str, ) from test_framework.mininode import P2PInterface from test_framework.test_framework import BitcoinTestFramework @@ -21,6 +26,21 @@ from test_framework.util import ( wait_until, ) +class CFiltersClient(P2PInterface): + def __init__(self): + super().__init__() + # Store the cfilters received. + self.cfilters = [] + + def pop_cfilters(self): + cfilters = self.cfilters + self.cfilters = [] + return cfilters + + def on_cfilter(self, message): + """Store cfilters received in a list.""" + self.cfilters.append(message) + class CompactFiltersTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True @@ -33,8 +53,8 @@ class CompactFiltersTest(BitcoinTestFramework): def run_test(self): # Node 0 supports COMPACT_FILTERS, node 1 does not. - node0 = self.nodes[0].add_p2p_connection(P2PInterface()) - node1 = self.nodes[1].add_p2p_connection(P2PInterface()) + node0 = self.nodes[0].add_p2p_connection(CFiltersClient()) + node1 = self.nodes[1].add_p2p_connection(CFiltersClient()) # Nodes 0 & 1 share the same first 999 blocks in the chain. self.nodes[0].generate(999) @@ -100,12 +120,89 @@ class CompactFiltersTest(BitcoinTestFramework): [int(header, 16) for header in (stale_cfcheckpt,)] ) + self.log.info("Check that peers can fetch cfheaders on active chain.") + request = msg_getcfheaders( + filter_type=FILTER_TYPE_BASIC, + start_height=1, + stop_hash=int(main_block_hash, 16) + ) + node0.send_and_ping(request) + response = node0.last_message['cfheaders'] + main_cfhashes = response.hashes + assert_equal(len(main_cfhashes), 1000) + assert_equal( + compute_last_header(response.prev_header, response.hashes), + int(main_cfcheckpt, 16) + ) + + self.log.info("Check that peers can fetch cfheaders on stale chain.") + request = msg_getcfheaders( + filter_type=FILTER_TYPE_BASIC, + start_height=1, + stop_hash=int(stale_block_hash, 16) + ) + node0.send_and_ping(request) + response = node0.last_message['cfheaders'] + stale_cfhashes = response.hashes + assert_equal(len(stale_cfhashes), 1000) + assert_equal( + compute_last_header(response.prev_header, response.hashes), + int(stale_cfcheckpt, 16) + ) + + self.log.info("Check that peers can fetch cfilters.") + stop_hash = self.nodes[0].getblockhash(10) + request = msg_getcfilters( + filter_type=FILTER_TYPE_BASIC, + start_height=1, + stop_hash=int(stop_hash, 16) + ) + node0.send_message(request) + node0.sync_with_ping() + response = node0.pop_cfilters() + assert_equal(len(response), 10) + + self.log.info("Check that cfilter responses are correct.") + for cfilter, cfhash, height in zip(response, main_cfhashes, range(1, 11)): + block_hash = self.nodes[0].getblockhash(height) + assert_equal(cfilter.filter_type, FILTER_TYPE_BASIC) + assert_equal(cfilter.block_hash, int(block_hash, 16)) + computed_cfhash = uint256_from_str(hash256(cfilter.filter_data)) + assert_equal(computed_cfhash, cfhash) + + self.log.info("Check that peers can fetch cfilters for stale blocks.") + request = msg_getcfilters( + filter_type=FILTER_TYPE_BASIC, + start_height=1000, + stop_hash=int(stale_block_hash, 16) + ) + node0.send_message(request) + node0.sync_with_ping() + response = node0.pop_cfilters() + assert_equal(len(response), 1) + + cfilter = response[0] + assert_equal(cfilter.filter_type, FILTER_TYPE_BASIC) + assert_equal(cfilter.block_hash, int(stale_block_hash, 16)) + computed_cfhash = uint256_from_str(hash256(cfilter.filter_data)) + assert_equal(computed_cfhash, stale_cfhashes[999]) + self.log.info("Requests to node 1 without NODE_COMPACT_FILTERS results in disconnection.") requests = [ msg_getcfcheckpt( filter_type=FILTER_TYPE_BASIC, stop_hash=int(main_block_hash, 16) ), + msg_getcfheaders( + filter_type=FILTER_TYPE_BASIC, + start_height=1000, + stop_hash=int(main_block_hash, 16) + ), + msg_getcfilters( + filter_type=FILTER_TYPE_BASIC, + start_height=1000, + stop_hash=int(main_block_hash, 16) + ), ] for request in requests: node1 = self.nodes[1].add_p2p_connection(P2PInterface()) @@ -114,6 +211,18 @@ class CompactFiltersTest(BitcoinTestFramework): self.log.info("Check that invalid requests result in disconnection.") requests = [ + # Requesting too many filters results in disconnection. + msg_getcfilters( + filter_type=FILTER_TYPE_BASIC, + start_height=0, + stop_hash=int(main_block_hash, 16) + ), + # Requesting too many filter headers results in disconnection. + msg_getcfheaders( + filter_type=FILTER_TYPE_BASIC, + start_height=0, + stop_hash=int(tip_hash, 16) + ), # Requesting unknown filter type results in disconnection. msg_getcfcheckpt( filter_type=255, @@ -130,5 +239,12 @@ class CompactFiltersTest(BitcoinTestFramework): node0.send_message(request) node0.wait_for_disconnect() +def compute_last_header(prev_header, hashes): + """Compute the last filter header from a starting header and a sequence of filter hashes.""" + header = ser_uint256(prev_header) + for filter_hash in hashes: + header = hash256(ser_uint256(filter_hash) + header) + return uint256_from_str(header) + if __name__ == '__main__': CompactFiltersTest().main() diff --git a/test/functional/p2p_getdata.py b/test/functional/p2p_getdata.py index fd94a09d80..d1b11c2c61 100755 --- a/test/functional/p2p_getdata.py +++ b/test/functional/p2p_getdata.py @@ -9,15 +9,11 @@ from test_framework.messages import ( CInv, msg_getdata, ) -from test_framework.mininode import ( - mininode_lock, - P2PInterface, -) +from test_framework.mininode import P2PInterface from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import wait_until -class P2PStoreBlock(P2PInterface): +class P2PStoreBlock(P2PInterface): def __init__(self): super().__init__() self.blocks = defaultdict(int) @@ -26,26 +22,28 @@ class P2PStoreBlock(P2PInterface): message.block.calc_sha256() self.blocks[message.block.sha256] += 1 + class GetdataTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 def run_test(self): - self.nodes[0].add_p2p_connection(P2PStoreBlock()) + p2p_block_store = self.nodes[0].add_p2p_connection(P2PStoreBlock()) self.log.info("test that an invalid GETDATA doesn't prevent processing of future messages") # Send invalid message and verify that node responds to later ping invalid_getdata = msg_getdata() invalid_getdata.inv.append(CInv(t=0, h=0)) # INV type 0 is invalid. - self.nodes[0].p2ps[0].send_and_ping(invalid_getdata) + p2p_block_store.send_and_ping(invalid_getdata) # Check getdata still works by fetching tip block best_block = int(self.nodes[0].getbestblockhash(), 16) good_getdata = msg_getdata() good_getdata.inv.append(CInv(t=2, h=best_block)) - self.nodes[0].p2ps[0].send_and_ping(good_getdata) - wait_until(lambda: self.nodes[0].p2ps[0].blocks[best_block] == 1, timeout=30, lock=mininode_lock) + p2p_block_store.send_and_ping(good_getdata) + p2p_block_store.wait_until(lambda: self.nodes[0].p2ps[0].blocks[best_block] == 1) + if __name__ == '__main__': GetdataTest().main() diff --git a/test/functional/test_framework/address.py b/test/functional/test_framework/address.py index 8f410f233e..9506b63f82 100644 --- a/test/functional/test_framework/address.py +++ b/test/functional/test_framework/address.py @@ -5,12 +5,15 @@ """Encode and decode BASE58, P2PKH and P2SH addresses.""" import enum +import unittest from .script import hash256, hash160, sha256, CScript, OP_0 from .util import hex_str_to_bytes from . import segwit_addr +from test_framework.util import assert_equal + ADDRESS_BCRT1_UNSPENDABLE = 'bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj' ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR = 'addr(bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj)#juyq9d97' # Coins sent to this address can be spent with a witness stack of just OP_TRUE @@ -41,7 +44,32 @@ def byte_to_base58(b, version): str = str[2:] return result -# TODO: def base58_decode + +def base58_to_byte(s, verify_checksum=True): + if not s: + return b'' + n = 0 + for c in s: + n *= 58 + assert c in chars + digit = chars.index(c) + n += digit + h = '%x' % n + if len(h) % 2: + h = '0' + h + res = n.to_bytes((n.bit_length() + 7) // 8, 'big') + pad = 0 + for c in s: + if c == chars[0]: + pad += 1 + else: + break + res = b'\x00' * pad + res + if verify_checksum: + assert_equal(hash256(res[:-4])[:4], res[-4:]) + + return res[1:-4], int(res[0]) + def keyhash_to_p2pkh(hash, main = False): assert len(hash) == 20 @@ -100,3 +128,22 @@ def check_script(script): if (type(script) is bytes or type(script) is CScript): return script assert False + + +class TestFrameworkScript(unittest.TestCase): + def test_base58encodedecode(self): + def check_base58(data, version): + self.assertEqual(base58_to_byte(byte_to_base58(data, version)), (data, version)) + + check_base58(b'\x1f\x8e\xa1p*{\xd4\x94\x1b\xca\tA\xb8R\xc4\xbb\xfe\xdb.\x05', 111) + check_base58(b':\x0b\x05\xf4\xd7\xf6l;\xa7\x00\x9fE50)l\x84\\\xc9\xcf', 111) + check_base58(b'A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 111) + check_base58(b'\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 111) + check_base58(b'\0\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 111) + check_base58(b'\0\0\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 111) + check_base58(b'\x1f\x8e\xa1p*{\xd4\x94\x1b\xca\tA\xb8R\xc4\xbb\xfe\xdb.\x05', 0) + check_base58(b':\x0b\x05\xf4\xd7\xf6l;\xa7\x00\x9fE50)l\x84\\\xc9\xcf', 0) + check_base58(b'A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 0) + check_base58(b'\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 0) + check_base58(b'\0\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 0) + check_base58(b'\0\0\0A\xc1\xea\xf1\x11\x80%Y\xba\xd6\x1b`\xd6+\x1f\x89|c\x92\x8a', 0) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 6c9c8a7397..4d1dd4422e 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1516,6 +1516,110 @@ class msg_no_witness_blocktxn(msg_blocktxn): def serialize(self): return self.block_transactions.serialize(with_witness=False) + +class msg_getcfilters: + __slots__ = ("filter_type", "start_height", "stop_hash") + msgtype = b"getcfilters" + + def __init__(self, filter_type, start_height, stop_hash): + self.filter_type = filter_type + self.start_height = start_height + self.stop_hash = stop_hash + + def deserialize(self, f): + self.filter_type = struct.unpack("<B", f.read(1))[0] + self.start_height = struct.unpack("<I", f.read(4))[0] + self.stop_hash = deser_uint256(f) + + def serialize(self): + r = b"" + r += struct.pack("<B", self.filter_type) + r += struct.pack("<I", self.start_height) + r += ser_uint256(self.stop_hash) + return r + + def __repr__(self): + return "msg_getcfilters(filter_type={:#x}, start_height={}, stop_hash={:x})".format( + self.filter_type, self.start_height, self.stop_hash) + +class msg_cfilter: + __slots__ = ("filter_type", "block_hash", "filter_data") + msgtype = b"cfilter" + + def __init__(self, filter_type=None, block_hash=None, filter_data=None): + self.filter_type = filter_type + self.block_hash = block_hash + self.filter_data = filter_data + + def deserialize(self, f): + self.filter_type = struct.unpack("<B", f.read(1))[0] + self.block_hash = deser_uint256(f) + self.filter_data = deser_string(f) + + def serialize(self): + r = b"" + r += struct.pack("<B", self.filter_type) + r += ser_uint256(self.block_hash) + r += ser_string(self.filter_data) + return r + + def __repr__(self): + return "msg_cfilter(filter_type={:#x}, block_hash={:x})".format( + self.filter_type, self.block_hash) + +class msg_getcfheaders: + __slots__ = ("filter_type", "start_height", "stop_hash") + msgtype = b"getcfheaders" + + def __init__(self, filter_type, start_height, stop_hash): + self.filter_type = filter_type + self.start_height = start_height + self.stop_hash = stop_hash + + def deserialize(self, f): + self.filter_type = struct.unpack("<B", f.read(1))[0] + self.start_height = struct.unpack("<I", f.read(4))[0] + self.stop_hash = deser_uint256(f) + + def serialize(self): + r = b"" + r += struct.pack("<B", self.filter_type) + r += struct.pack("<I", self.start_height) + r += ser_uint256(self.stop_hash) + return r + + def __repr__(self): + return "msg_getcfheaders(filter_type={:#x}, start_height={}, stop_hash={:x})".format( + self.filter_type, self.start_height, self.stop_hash) + +class msg_cfheaders: + __slots__ = ("filter_type", "stop_hash", "prev_header", "hashes") + msgtype = b"cfheaders" + + def __init__(self, filter_type=None, stop_hash=None, prev_header=None, hashes=None): + self.filter_type = filter_type + self.stop_hash = stop_hash + self.prev_header = prev_header + self.hashes = hashes + + def deserialize(self, f): + self.filter_type = struct.unpack("<B", f.read(1))[0] + self.stop_hash = deser_uint256(f) + self.prev_header = deser_uint256(f) + self.hashes = deser_uint256_vector(f) + + def serialize(self): + r = b"" + r += struct.pack("<B", self.filter_type) + r += ser_uint256(self.stop_hash) + r += ser_uint256(self.prev_header) + r += ser_uint256_vector(self.hashes) + return r + + def __repr__(self): + return "msg_cfheaders(filter_type={:#x}, stop_hash={:x})".format( + self.filter_type, self.stop_hash) + class msg_getcfcheckpt: __slots__ = ("filter_type", "stop_hash") msgtype = b"getcfcheckpt" diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index 95a63717d6..45063aaff2 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -32,6 +32,8 @@ from test_framework.messages import ( MSG_BLOCK, msg_blocktxn, msg_cfcheckpt, + msg_cfheaders, + msg_cfilter, msg_cmpctblock, msg_feefilter, msg_filteradd, @@ -69,6 +71,8 @@ MESSAGEMAP = { b"block": msg_block, b"blocktxn": msg_blocktxn, b"cfcheckpt": msg_cfcheckpt, + b"cfheaders": msg_cfheaders, + b"cfilter": msg_cfilter, b"cmpctblock": msg_cmpctblock, b"feefilter": msg_feefilter, b"filteradd": msg_filteradd, @@ -331,6 +335,8 @@ class P2PInterface(P2PConnection): def on_block(self, message): pass def on_blocktxn(self, message): pass def on_cfcheckpt(self, message): pass + def on_cfheaders(self, message): pass + def on_cfilter(self, message): pass def on_cmpctblock(self, message): pass def on_feefilter(self, message): pass def on_filteradd(self, message): pass @@ -371,7 +377,7 @@ class P2PInterface(P2PConnection): # Connection helper methods - def wait_until(self, test_function, timeout): + def wait_until(self, test_function, timeout=60): wait_until(test_function, timeout=timeout, lock=mininode_lock, timeout_factor=self.timeout_factor) def wait_for_disconnect(self, timeout=60): @@ -652,6 +658,8 @@ class P2PTxInvStore(P2PInterface): # save txid self.tx_invs_received[i.hash] += 1 + super().on_inv(message) + def get_invs(self): with mininode_lock: return list(self.tx_invs_received.keys()) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 716fa1d845..5469f808d1 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -290,7 +290,12 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): exit_code = TEST_EXIT_SKIPPED else: self.log.error("Test failed. Test logging available at %s/test_framework.log", self.options.tmpdir) + self.log.error("") self.log.error("Hint: Call {} '{}' to consolidate all logs".format(os.path.normpath(os.path.dirname(os.path.realpath(__file__)) + "/../combine_logs.py"), self.options.tmpdir)) + self.log.error("") + self.log.error("If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.") + self.log.error(self.config['environment']['PACKAGE_BUGREPORT']) + self.log.error("") exit_code = TEST_EXIT_FAILED # Logging.shutdown will not remove stream- and filehandlers, so we must # do it explicitly. Handlers are removed so the next test run can apply diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 7821355e29..0812470b0c 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -67,6 +67,7 @@ TEST_EXIT_PASSED = 0 TEST_EXIT_SKIPPED = 77 TEST_FRAMEWORK_MODULES = [ + "address", "script", ] diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 039ce7daee..524e1593ba 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -203,6 +203,14 @@ class ToolWalletTest(BitcoinTestFramework): assert_equal(shasum_after, shasum_before) self.log.debug('Wallet file shasum unchanged\n') + def test_salvage(self): + # TODO: Check salvage actually salvages and doesn't break things. https://github.com/bitcoin/bitcoin/issues/7463 + self.log.info('Check salvage') + self.start_node(0, ['-wallet=salvage']) + self.stop_node(0) + + self.assert_tool_output('', '-wallet=salvage', 'salvage') + def run_test(self): self.wallet_path = os.path.join(self.nodes[0].datadir, self.chain, 'wallets', 'wallet.dat') self.test_invalid_tool_commands_and_args() @@ -211,7 +219,7 @@ class ToolWalletTest(BitcoinTestFramework): self.test_tool_wallet_info_after_transaction() self.test_tool_wallet_create_on_existing_wallet() self.test_getwalletinfo_on_different_wallet() - + self.test_salvage() if __name__ == '__main__': ToolWalletTest().main() diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 9e295af330..797c903dd3 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -404,8 +404,6 @@ class WalletTest(BitcoinTestFramework): '-reindex', '-zapwallettxes=1', '-zapwallettxes=2', - # disabled until issue is fixed: https://github.com/bitcoin/bitcoin/issues/7463 - # '-salvagewallet', ] chainlimit = 6 for m in maintenance: diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 580a61f9f3..ff9ff34185 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -122,10 +122,6 @@ class MultiWalletTest(BitcoinTestFramework): self.nodes[0].assert_start_raises_init_error(['-zapwallettxes=1', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") self.nodes[0].assert_start_raises_init_error(['-zapwallettxes=2', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") - self.log.info("Do not allow -salvagewallet with multiwallet") - self.nodes[0].assert_start_raises_init_error(['-salvagewallet', '-wallet=w1', '-wallet=w2'], "Error: -salvagewallet is only allowed with a single wallet file") - self.nodes[0].assert_start_raises_init_error(['-salvagewallet=1', '-wallet=w1', '-wallet=w2'], "Error: -salvagewallet is only allowed with a single wallet file") - # if wallets/ doesn't exist, datadir should be the default wallet dir wallet_dir2 = data_dir('walletdir') os.rename(wallet_dir(), wallet_dir2) diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py index b384998d56..3417616d77 100755 --- a/test/functional/wallet_resendwallettransactions.py +++ b/test/functional/wallet_resendwallettransactions.py @@ -49,16 +49,21 @@ class ResendWalletTransactionsTest(BitcoinTestFramework): block.solve() node.submitblock(ToHex(block)) - # Transaction should not be rebroadcast node.syncwithvalidationinterfacequeue() - node.p2ps[1].sync_with_ping() - assert_equal(node.p2ps[1].tx_invs_received[txid], 0) + now = int(time.time()) + + # Transaction should not be rebroadcast within first 12 hours + # Leave 2 mins for buffer + twelve_hrs = 12 * 60 * 60 + two_min = 2 * 60 + node.setmocktime(now + twelve_hrs - two_min) + time.sleep(2) # ensure enough time has passed for rebroadcast attempt to occur + assert_equal(txid in node.p2ps[1].get_invs(), False) self.log.info("Bump time & check that transaction is rebroadcast") # Transaction should be rebroadcast approximately 24 hours in the future, # but can range from 12-36. So bump 36 hours to be sure. - rebroadcast_time = int(time.time()) + 36 * 60 * 60 - node.setmocktime(rebroadcast_time) + node.setmocktime(now + 36 * 60 * 60) wait_until(lambda: node.p2ps[1].tx_invs_received[txid] >= 1, lock=mininode_lock) |