diff options
79 files changed, 860 insertions, 557 deletions
diff --git a/build-aux/m4/l_atomic.m4 b/build-aux/m4/l_atomic.m4 index 40639dfe61..602b57fe43 100644 --- a/build-aux/m4/l_atomic.m4 +++ b/build-aux/m4/l_atomic.m4 @@ -18,7 +18,7 @@ m4_define([_CHECK_ATOMIC_testbody], [[ int main() { std::atomic<bool> lock{true}; - std::atomic_exchange(&lock, false); + lock.exchange(false); std::atomic<std::chrono::seconds> t{0s}; t.store(2s); @@ -34,6 +34,8 @@ m4_define([_CHECK_ATOMIC_testbody], [[ AC_DEFUN([CHECK_ATOMIC], [ AC_LANG_PUSH(C++) + TEMP_CXXFLAGS="$CXXFLAGS" + CXXFLAGS="$CXXFLAGS $PTHREAD_CFLAGS" AC_MSG_CHECKING([whether std::atomic can be used without link library]) @@ -51,5 +53,6 @@ AC_DEFUN([CHECK_ATOMIC], [ ]) ]) + CXXFLAGS="$TEMP_CXXFLAGS" AC_LANG_POP ]) diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index e1032ba6bd..77358f93d9 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -41,6 +41,7 @@ if [ "${RUN_TIDY}" = "true" ]; then CI_EXEC "python3 ${DIR_IWYU}/include-what-you-use/iwyu_tool.py"\ " src/compat"\ " src/init"\ + " src/kernel/mempool_persist.cpp"\ " src/policy/feerate.cpp"\ " src/policy/packages.cpp"\ " src/policy/settings.cpp"\ diff --git a/ci/test/wrap-qemu.sh b/ci/test/wrap-qemu.sh index fcd56f533e..eb31edbce8 100755 --- a/ci/test/wrap-qemu.sh +++ b/ci/test/wrap-qemu.sh @@ -6,7 +6,7 @@ export LC_ALL=C.UTF-8 -for b_name in {"${BASE_OUTDIR}/bin"/*,src/secp256k1/*tests,src/minisketch/test{,-verify},src/univalue/{no_nul,test_json,unitester,object}}; do +for b_name in {"${BASE_OUTDIR}/bin"/*,src/secp256k1/*tests,src/minisketch/test{,-verify},src/univalue/{test_json,unitester,object}}; do # shellcheck disable=SC2044 for b in $(find "${BASE_ROOT_DIR}" -executable -type f -name "$(basename "$b_name")"); do echo "Wrap $b ..." diff --git a/ci/test/wrap-wine.sh b/ci/test/wrap-wine.sh index 525db9eded..1662f8f6a3 100755 --- a/ci/test/wrap-wine.sh +++ b/ci/test/wrap-wine.sh @@ -6,7 +6,7 @@ export LC_ALL=C.UTF-8 -for b_name in {"${BASE_OUTDIR}/bin"/*,src/secp256k1/*tests,src/minisketch/test{,-verify},src/univalue/{no_nul,test_json,unitester,object}}.exe; do +for b_name in {"${BASE_OUTDIR}/bin"/*,src/secp256k1/*tests,src/minisketch/test{,-verify},src/univalue/{test_json,unitester,object}}.exe; do # shellcheck disable=SC2044 for b in $(find "${BASE_ROOT_DIR}" -executable -type f -name "$(basename "$b_name")"); do if (file "$b" | grep "Windows"); then diff --git a/configure.ac b/configure.ac index 0e1968f5c4..0cd88543c6 100644 --- a/configure.ac +++ b/configure.ac @@ -87,9 +87,6 @@ else AX_CXX_COMPILE_STDCXX([20], [noext], [mandatory]) fi -dnl Check if -latomic is required for <std::atomic> -CHECK_ATOMIC - dnl check if additional link flags are required for std::filesystem CHECK_FILESYSTEM @@ -887,6 +884,9 @@ AC_C_BIGENDIAN dnl Check for pthread compile/link requirements AX_PTHREAD +dnl Check if -latomic is required for <std::atomic> +CHECK_ATOMIC + dnl The following macro will add the necessary defines to bitcoin-config.h, but dnl they also need to be passed down to any subprojects. Pull the results out of dnl the cache and add them to CPPFLAGS. @@ -2049,5 +2049,6 @@ echo " CPPFLAGS = $DEBUG_CPPFLAGS $HARDENED_CPPFLAGS $CORE_CPPFLAGS $CPP echo " CXX = $CXX" echo " CXXFLAGS = $LTO_CXXFLAGS $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $GPROF_CXXFLAGS $CORE_CXXFLAGS $CXXFLAGS" echo " LDFLAGS = $LTO_LDFLAGS $PTHREAD_LIBS $HARDENED_LDFLAGS $GPROF_LDFLAGS $CORE_LDFLAGS $LDFLAGS" +echo " AR = $AR" echo " ARFLAGS = $ARFLAGS" echo diff --git a/contrib/devtools/iwyu/bitcoin.core.imp b/contrib/devtools/iwyu/bitcoin.core.imp index ce7786f58c..919ffab102 100644 --- a/contrib/devtools/iwyu/bitcoin.core.imp +++ b/contrib/devtools/iwyu/bitcoin.core.imp @@ -3,4 +3,5 @@ { include: [ "<bits/termios-c_lflag.h>", private, "<termios.h>", public ] }, { include: [ "<bits/termios-struct.h>", private, "<termios.h>", public ] }, { include: [ "<bits/termios-tcflow.h>", private, "<termios.h>", public ] }, + { include: [ "<bits/chrono.h>", private, "<chrono>", public ] }, ] diff --git a/depends/config.site.in b/depends/config.site.in index 189330c42d..f7e770343c 100644 --- a/depends/config.site.in +++ b/depends/config.site.in @@ -105,7 +105,7 @@ PYTHONPATH="${depends_prefix}/native/lib/python3/dist-packages${PYTHONPATH:+${PA if test -n "@AR@"; then AR="@AR@" - ac_cv_path_ac_pt_AR="${AR}" + ac_cv_path_AR="${AR}" fi if test -n "@RANLIB@"; then @@ -126,17 +126,17 @@ fi if test "@host_os@" = darwin; then if test -n "@OTOOL@"; then OTOOL="@OTOOL@" - ac_cv_path_ac_pt_OTOOL="${OTOOL}" + ac_cv_path_OTOOL="${OTOOL}" fi if test -n "@INSTALL_NAME_TOOL@"; then INSTALL_NAME_TOOL="@INSTALL_NAME_TOOL@" - ac_cv_path_ac_pt_INSTALL_NAME_TOOL="${INSTALL_NAME_TOOL}" + ac_cv_path_INSTALL_NAME_TOOL="${INSTALL_NAME_TOOL}" fi if test -n "@DSYMUTIL@"; then DSYMUTIL="@DSYMUTIL@" - ac_cv_path_ac_pt_DSYMUTIL="${DSYMUTIL}" + ac_cv_path_DSYMUTIL="${DSYMUTIL}" fi fi diff --git a/depends/hosts/linux.mk b/depends/hosts/linux.mk index b101043439..635d3d16da 100644 --- a/depends/hosts/linux.mk +++ b/depends/hosts/linux.mk @@ -5,6 +5,10 @@ ifneq ($(LTO),) linux_CFLAGS += -flto linux_CXXFLAGS += -flto linux_LDFLAGS += -flto + +linux_AR = $(host_toolchain)gcc-ar +linux_NM = $(host_toolchain)gcc-nm +linux_RANLIB = $(host_toolchain)gcc-ranlib endif linux_release_CFLAGS=-O2 diff --git a/depends/hosts/mingw32.mk b/depends/hosts/mingw32.mk index b98f9ab7ac..fc1cc1afbe 100644 --- a/depends/hosts/mingw32.mk +++ b/depends/hosts/mingw32.mk @@ -9,6 +9,10 @@ ifneq ($(LTO),) mingw32_CFLAGS += -flto mingw32_CXXFLAGS += -flto mingw32_LDFLAGS += -flto + +mingw32_AR = $(host_toolchain)gcc-ar +mingw32_NM = $(host_toolchain)gcc-nm +mingw32_RANLIB = $(host_toolchain)gcc-ranlib endif mingw32_release_CFLAGS=-O2 diff --git a/depends/hosts/netbsd.mk b/depends/hosts/netbsd.mk index 8342dcc6ed..14121dca20 100644 --- a/depends/hosts/netbsd.mk +++ b/depends/hosts/netbsd.mk @@ -5,6 +5,10 @@ ifneq ($(LTO),) netbsd_CFLAGS += -flto netbsd_CXXFLAGS += -flto netbsd_LDFLAGS += -flto + +netbsd_AR = $(host_toolchain)gcc-ar +netbsd_NM = $(host_toolchain)gcc-nm +netbsd_RANLIB = $(host_toolchain)gcc-ranlib endif netbsd_CXXFLAGS=$(netbsd_CFLAGS) diff --git a/depends/packages/expat.mk b/depends/packages/expat.mk index 50791ebc6e..349319138e 100644 --- a/depends/packages/expat.mk +++ b/depends/packages/expat.mk @@ -9,6 +9,7 @@ define $(package)_set_vars $(package)_config_opts += --disable-dependency-tracking --enable-option-checking $(package)_config_opts += --without-xmlwf $(package)_config_opts_linux=--with-pic + $(package)_cflags += -fno-lto endef define $(package)_config_cmds diff --git a/depends/packages/native_ds_store.mk b/depends/packages/native_ds_store.mk index 44108925a4..51a95f48ef 100644 --- a/depends/packages/native_ds_store.mk +++ b/depends/packages/native_ds_store.mk @@ -1,6 +1,6 @@ package=native_ds_store $(package)_version=1.3.0 -$(package)_download_path=https://github.com/al45tair/ds_store/archive/ +$(package)_download_path=https://github.com/dmgbuild/ds_store/archive/ $(package)_file_name=v$($(package)_version).tar.gz $(package)_sha256_hash=76b3280cd4e19e5179defa23fb594a9dd32643b0c80d774bd3108361d94fb46d $(package)_install_libdir=$(build_prefix)/lib/python3/dist-packages diff --git a/depends/packages/native_mac_alias.mk b/depends/packages/native_mac_alias.mk index 783f87ca7c..ddd631186e 100644 --- a/depends/packages/native_mac_alias.mk +++ b/depends/packages/native_mac_alias.mk @@ -1,6 +1,6 @@ package=native_mac_alias $(package)_version=2.2.0 -$(package)_download_path=https://github.com/al45tair/mac_alias/archive/ +$(package)_download_path=https://github.com/dmgbuild/mac_alias/archive/ $(package)_file_name=v$($(package)_version).tar.gz $(package)_sha256_hash=421e6d7586d1f155c7db3e7da01ca0dacc9649a509a253ad7077b70174426499 $(package)_install_libdir=$(build_prefix)/lib/python3/dist-packages diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 7e268fe9a5..d46ca9ff9c 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -152,6 +152,9 @@ $(package)_config_opts_linux += -fontconfig $(package)_config_opts_linux += -no-opengl $(package)_config_opts_linux += -no-feature-vulkan $(package)_config_opts_linux += -dbus-runtime +ifneq ($(LTO),) +$(package)_config_opts_linux += -ltcg +endif $(package)_config_opts_arm_linux += -platform linux-g++ -xplatform bitcoin-linux-g++ $(package)_config_opts_i686_linux = -xplatform linux-g++-32 ifneq (,$(findstring -stdlib=libc++,$($(1)_cxx))) diff --git a/doc/release-notes-24148.md b/doc/release-notes-24148.md new file mode 100644 index 0000000000..f7a0fd6fa1 --- /dev/null +++ b/doc/release-notes-24148.md @@ -0,0 +1,23 @@ +Notable changes +=============== + +Wallet +------ + +- The `wsh()` output descriptor was extended with Miniscript support. You can import Miniscript + descriptors for P2WSH in a watchonly wallet to track coins, but you can't spend from them using + the Bitcoin Core wallet yet. + You can find more about Miniscript on the [reference website](https://bitcoin.sipa.be/miniscript/). + + +Low-level changes +================= + +RPC +--- + +- The `deriveaddresses`, `getdescriptorinfo`, `importdescriptors` and `scantxoutset` commands now + accept Miniscript expression within a `wsh()` descriptor. + +- The `getaddressinfo`, `decodescript`, `listdescriptors` and `listunspent` commands may now output + a Miniscript descriptor inside a `wsh()` where a `wsh(raw())` descriptor was previously returned. diff --git a/src/.clang-tidy b/src/.clang-tidy index e9807d4cb7..df2a080075 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -1,12 +1,14 @@ Checks: ' -*, bugprone-argument-comment, +misc-unused-using-decls, modernize-use-default-member-init, modernize-use-nullptr, readability-redundant-declaration, ' WarningsAsErrors: ' bugprone-argument-comment, +misc-unused-using-decls, modernize-use-default-member-init, modernize-use-nullptr, readability-redundant-declaration, diff --git a/src/Makefile.am b/src/Makefile.am index d293775fc3..02ec5c098e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -177,6 +177,7 @@ BITCOIN_CORE_H = \ kernel/context.h \ kernel/mempool_limits.h \ kernel/mempool_options.h \ + kernel/mempool_persist.h \ key.h \ key_io.h \ logging.h \ @@ -198,6 +199,7 @@ BITCOIN_CORE_H = \ node/chainstate.h \ node/coin.h \ node/context.h \ + node/mempool_persist_args.h \ node/miner.h \ node/minisketchwrapper.h \ node/psbt.h \ @@ -366,6 +368,7 @@ libbitcoin_node_a_SOURCES = \ kernel/checks.cpp \ kernel/coinstats.cpp \ kernel/context.cpp \ + kernel/mempool_persist.cpp \ mapport.cpp \ mempool_args.cpp \ net.cpp \ @@ -379,6 +382,7 @@ libbitcoin_node_a_SOURCES = \ node/context.cpp \ node/eviction.cpp \ node/interfaces.cpp \ + node/mempool_persist_args.cpp \ node/miner.cpp \ node/minisketchwrapper.cpp \ node/psbt.cpp \ @@ -881,6 +885,7 @@ libbitcoinkernel_la_SOURCES = \ kernel/checks.cpp \ kernel/coinstats.cpp \ kernel/context.cpp \ + kernel/mempool_persist.cpp \ key.cpp \ logging.cpp \ node/blockstorage.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index b806b62d5b..d9195ad32e 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -373,7 +373,7 @@ endif $(AM_V_at)$(MAKE) $(AM_MAKEFLAGS) -C secp256k1 check if ENABLE_TESTS -UNIVALUE_TESTS = univalue/test/object univalue/test/unitester univalue/test/no_nul +UNIVALUE_TESTS = univalue/test/object univalue/test/unitester noinst_PROGRAMS += $(UNIVALUE_TESTS) TESTS += $(UNIVALUE_TESTS) @@ -382,11 +382,6 @@ univalue_test_unitester_LDADD = $(LIBUNIVALUE) univalue_test_unitester_CPPFLAGS = -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) -DJSON_TEST_SRC=\"$(srcdir)/$(UNIVALUE_TEST_DATA_DIR_INT)\" univalue_test_unitester_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS) -univalue_test_no_nul_SOURCES = $(UNIVALUE_TEST_NO_NUL_INT) -univalue_test_no_nul_LDADD = $(LIBUNIVALUE) -univalue_test_no_nul_CPPFLAGS = -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) -univalue_test_no_nul_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS) - univalue_test_object_SOURCES = $(UNIVALUE_TEST_OBJECT_INT) univalue_test_object_LDADD = $(LIBUNIVALUE) univalue_test_object_CPPFLAGS = -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) diff --git a/src/Makefile.test_fuzz.include b/src/Makefile.test_fuzz.include index 8922dda3ad..b43816636f 100644 --- a/src/Makefile.test_fuzz.include +++ b/src/Makefile.test_fuzz.include @@ -10,6 +10,7 @@ EXTRA_LIBRARIES += \ TEST_FUZZ_H = \ test/fuzz/fuzz.h \ test/fuzz/FuzzedDataProvider.h \ + test/fuzz/mempool_utils.h \ test/fuzz/util.h libtest_fuzz_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(MINIUPNPC_CPPFLAGS) $(NATPMP_CPPFLAGS) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS) diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp index d258d7a29e..a10f7ff7d1 100644 --- a/src/bench/wallet_loading.cpp +++ b/src/bench/wallet_loading.cpp @@ -19,8 +19,6 @@ using wallet::CWallet; using wallet::DatabaseFormat; using wallet::DatabaseOptions; -using wallet::ISMINE_SPENDABLE; -using wallet::MakeWalletDatabase; using wallet::TxStateInactive; using wallet::WALLET_FLAG_DESCRIPTORS; using wallet::WalletContext; @@ -9,6 +9,7 @@ #include <cstdio> #include <filesystem> +#include <functional> #include <iomanip> #include <ios> #include <ostream> @@ -199,6 +200,7 @@ bool create_directories(const std::filesystem::path& p, std::error_code& ec) = d /** Bridge operations to C stdio */ namespace fsbridge { + using FopenFn = std::function<FILE*(const fs::path&, const char*)>; FILE *fopen(const fs::path& p, const char *mode); /** diff --git a/src/init.cpp b/src/init.cpp index 267070a4c9..5ed1def4db 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -10,6 +10,7 @@ #include <init.h> #include <kernel/checks.h> +#include <kernel/mempool_persist.h> #include <addrman.h> #include <banman.h> @@ -41,6 +42,7 @@ #include <node/chainstate.h> #include <node/context.h> #include <node/interface_ui.h> +#include <node/mempool_persist_args.h> #include <node/miner.h> #include <policy/feerate.h> #include <policy/fees.h> @@ -102,14 +104,18 @@ #include <zmq/zmqrpc.h> #endif +using kernel::DumpMempool; + using node::CacheSizes; using node::CalculateCacheSizes; using node::ChainstateLoadVerifyError; using node::ChainstateLoadingError; -using node::CleanupBlockRevFiles; +using node::DEFAULT_PERSIST_MEMPOOL; using node::DEFAULT_PRINTPRIORITY; using node::DEFAULT_STOPAFTERBLOCKIMPORT; using node::LoadChainstate; +using node::MempoolPath; +using node::ShouldPersistMempool; using node::NodeContext; using node::ThreadImport; using node::VerifyLoadedChainstate; @@ -245,8 +251,8 @@ void Shutdown(NodeContext& node) node.addrman.reset(); node.netgroupman.reset(); - if (node.mempool && node.mempool->IsLoaded() && node.args->GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - DumpMempool(*node.mempool); + if (node.mempool && node.mempool->GetLoadTried() && ShouldPersistMempool(*node.args)) { + DumpMempool(*node.mempool, MempoolPath(*node.args)); } // Drop transactions we were still watching, and record fee estimations. @@ -1669,7 +1675,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &chainman, &args] { - ThreadImport(chainman, vImportFiles, args); + ThreadImport(chainman, vImportFiles, args, ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{}); }); // Wait for genesis block to be processed diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index df9e55874f..ff994b8edc 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -287,6 +287,9 @@ public: //! to be prepared to handle this by ignoring notifications about unknown //! removed transactions and already added new transactions. virtual void requestMempoolTransactions(Notifications& notifications) = 0; + + //! Return true if an assumed-valid chain is in use. + virtual bool hasAssumedValidChain() = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp new file mode 100644 index 0000000000..1a1cf2bbdc --- /dev/null +++ b/src/kernel/mempool_persist.cpp @@ -0,0 +1,189 @@ +// Copyright (c) 2022 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 <kernel/mempool_persist.h> + +#include <clientversion.h> +#include <consensus/amount.h> +#include <fs.h> +#include <logging.h> +#include <primitives/transaction.h> +#include <serialize.h> +#include <shutdown.h> +#include <streams.h> +#include <sync.h> +#include <txmempool.h> +#include <uint256.h> +#include <util/system.h> +#include <util/time.h> +#include <validation.h> + +#include <chrono> +#include <cstdint> +#include <cstdio> +#include <exception> +#include <functional> +#include <map> +#include <memory> +#include <set> +#include <stdexcept> +#include <utility> +#include <vector> + +using fsbridge::FopenFn; + +namespace kernel { + +static const uint64_t MEMPOOL_DUMP_VERSION = 1; + +bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, CChainState& active_chainstate, FopenFn mockable_fopen_function) +{ + if (load_path.empty()) return false; + + FILE* filestr{mockable_fopen_function(load_path, "rb")}; + CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); + if (file.IsNull()) { + LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); + return false; + } + + int64_t count = 0; + int64_t expired = 0; + int64_t failed = 0; + int64_t already_there = 0; + int64_t unbroadcast = 0; + auto now = NodeClock::now(); + + try { + uint64_t version; + file >> version; + if (version != MEMPOOL_DUMP_VERSION) { + return false; + } + uint64_t num; + file >> num; + while (num) { + --num; + CTransactionRef tx; + int64_t nTime; + int64_t nFeeDelta; + file >> tx; + file >> nTime; + file >> nFeeDelta; + + CAmount amountdelta = nFeeDelta; + if (amountdelta) { + pool.PrioritiseTransaction(tx->GetHash(), amountdelta); + } + if (nTime > TicksSinceEpoch<std::chrono::seconds>(now - pool.m_expiry)) { + LOCK(cs_main); + const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false); + if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) { + ++count; + } else { + // mempool may contain the transaction already, e.g. from + // wallet(s) having loaded it while we were processing + // mempool transactions; consider these as valid, instead of + // failed, but mark them as 'already there' + if (pool.exists(GenTxid::Txid(tx->GetHash()))) { + ++already_there; + } else { + ++failed; + } + } + } else { + ++expired; + } + if (ShutdownRequested()) + return false; + } + std::map<uint256, CAmount> mapDeltas; + file >> mapDeltas; + + for (const auto& i : mapDeltas) { + pool.PrioritiseTransaction(i.first, i.second); + } + + std::set<uint256> unbroadcast_txids; + file >> unbroadcast_txids; + unbroadcast = unbroadcast_txids.size(); + for (const auto& txid : unbroadcast_txids) { + // Ensure transactions were accepted to mempool then add to + // unbroadcast set. + if (pool.get(txid) != nullptr) pool.AddUnbroadcastTx(txid); + } + } catch (const std::exception& e) { + LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what()); + return false; + } + + LogPrintf("Imported mempool transactions from disk: %i succeeded, %i failed, %i expired, %i already there, %i waiting for initial broadcast\n", count, failed, expired, already_there, unbroadcast); + return true; +} + +bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mockable_fopen_function, bool skip_file_commit) +{ + auto start = SteadyClock::now(); + + std::map<uint256, CAmount> mapDeltas; + std::vector<TxMempoolInfo> vinfo; + std::set<uint256> unbroadcast_txids; + + static Mutex dump_mutex; + LOCK(dump_mutex); + + { + LOCK(pool.cs); + for (const auto &i : pool.mapDeltas) { + mapDeltas[i.first] = i.second; + } + vinfo = pool.infoAll(); + unbroadcast_txids = pool.GetUnbroadcastTxs(); + } + + auto mid = SteadyClock::now(); + + try { + FILE* filestr{mockable_fopen_function(dump_path + ".new", "wb")}; + if (!filestr) { + return false; + } + + CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); + + uint64_t version = MEMPOOL_DUMP_VERSION; + file << version; + + file << (uint64_t)vinfo.size(); + for (const auto& i : vinfo) { + file << *(i.tx); + file << int64_t{count_seconds(i.m_time)}; + file << int64_t{i.nFeeDelta}; + mapDeltas.erase(i.tx->GetHash()); + } + + file << mapDeltas; + + LogPrintf("Writing %d unbroadcast transactions to disk.\n", unbroadcast_txids.size()); + file << unbroadcast_txids; + + if (!skip_file_commit && !FileCommit(file.Get())) + throw std::runtime_error("FileCommit failed"); + file.fclose(); + if (!RenameOver(dump_path + ".new", dump_path)) { + throw std::runtime_error("Rename failed"); + } + auto last = SteadyClock::now(); + + LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", + Ticks<SecondsDouble>(mid - start), + Ticks<SecondsDouble>(last - mid)); + } catch (const std::exception& e) { + LogPrintf("Failed to dump mempool: %s. Continuing anyway.\n", e.what()); + return false; + } + return true; +} + +} // namespace kernel diff --git a/src/kernel/mempool_persist.h b/src/kernel/mempool_persist.h new file mode 100644 index 0000000000..9a15ec6dca --- /dev/null +++ b/src/kernel/mempool_persist.h @@ -0,0 +1,28 @@ +// Copyright (c) 2022 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_KERNEL_MEMPOOL_PERSIST_H +#define BITCOIN_KERNEL_MEMPOOL_PERSIST_H + +#include <fs.h> + +class CChainState; +class CTxMemPool; + +namespace kernel { + +/** Dump the mempool to disk. */ +bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, + fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen, + bool skip_file_commit = false); + +/** Load the mempool from disk. */ +bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, + CChainState& active_chainstate, + fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); + +} // namespace kernel + + +#endif // BITCOIN_KERNEL_MEMPOOL_PERSIST_H diff --git a/src/net.cpp b/src/net.cpp index 90c73d583e..2c4a1e4bae 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -206,15 +206,13 @@ static std::vector<CAddress> ConvertSeeds(const std::vector<uint8_t> &vSeedsIn) // Otherwise, return the unroutable 0.0.0.0 but filled in with // the normal parameters, since the IP may be changed to a useful // one by discovery. -CAddress GetLocalAddress(const CNetAddr *paddrPeer, ServiceFlags nLocalServices) +CService GetLocalAddress(const CNetAddr& addrPeer) { - CAddress ret(CService(CNetAddr(),GetListenPort()), nLocalServices); + CService ret{CNetAddr(), GetListenPort()}; CService addr; - if (GetLocal(addr, paddrPeer)) - { - ret = CAddress(addr, nLocalServices); + if (GetLocal(addr, &addrPeer)) { + ret = CService{addr}; } - ret.nTime = GetAdjustedTime(); return ret; } @@ -233,35 +231,35 @@ bool IsPeerAddrLocalGood(CNode *pnode) IsReachable(addrLocal.GetNetwork()); } -std::optional<CAddress> GetLocalAddrForPeer(CNode *pnode) +std::optional<CService> GetLocalAddrForPeer(CNode& node) { - CAddress addrLocal = GetLocalAddress(&pnode->addr, pnode->GetLocalServices()); + CService addrLocal{GetLocalAddress(node.addr)}; if (gArgs.GetBoolArg("-addrmantest", false)) { // use IPv4 loopback during addrmantest - addrLocal = CAddress(CService(LookupNumeric("127.0.0.1", GetListenPort())), pnode->GetLocalServices()); + addrLocal = CService(LookupNumeric("127.0.0.1", GetListenPort())); } // If discovery is enabled, sometimes give our peer the address it // tells us that it sees us as in case it has a better idea of our // address than we do. FastRandomContext rng; - if (IsPeerAddrLocalGood(pnode) && (!addrLocal.IsRoutable() || + if (IsPeerAddrLocalGood(&node) && (!addrLocal.IsRoutable() || rng.randbits((GetnScore(addrLocal) > LOCAL_MANUAL) ? 3 : 1) == 0)) { - if (pnode->IsInboundConn()) { + if (node.IsInboundConn()) { // For inbound connections, assume both the address and the port // as seen from the peer. - addrLocal = CAddress{pnode->GetAddrLocal(), addrLocal.nServices, addrLocal.nTime}; + addrLocal = CService{node.GetAddrLocal()}; } else { // For outbound connections, assume just the address as seen from // the peer and leave the port in `addrLocal` as returned by // `GetLocalAddress()` above. The peer has no way to observe our // listening port when we have initiated the connection. - addrLocal.SetIP(pnode->GetAddrLocal()); + addrLocal.SetIP(node.GetAddrLocal()); } } if (addrLocal.IsRoutable() || gArgs.GetBoolArg("-addrmantest", false)) { - LogPrint(BCLog::NET, "Advertising address %s to peer=%d\n", addrLocal.ToString(), pnode->GetId()); + LogPrint(BCLog::NET, "Advertising address %s to peer=%d\n", addrLocal.ToString(), node.GetId()); return addrLocal; } // Address is unroutable. Don't advertise. @@ -543,7 +541,6 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo addr_bind = GetBindAddress(*sock); } CNode* pnode = new CNode(id, - nLocalServices, std::move(sock), addrConnect, CalculateKeyedNetGroup(addrConnect), @@ -603,7 +600,6 @@ Network CNode::ConnectedThroughNetwork() const void CNode::CopyStats(CNodeStats& stats) { stats.nodeid = this->GetId(); - X(nServices); X(addr); X(addrBind); stats.m_network = ConnectedThroughNetwork(); @@ -880,7 +876,7 @@ bool CConnman::AttemptToEvictConnection() .m_min_ping_time = node->m_min_ping_time, .m_last_block_time = node->m_last_block_time, .m_last_tx_time = node->m_last_tx_time, - .fRelevantServices = HasAllDesirableServiceFlags(node->nServices), + .fRelevantServices = node->m_has_all_wanted_services, .m_relay_txs = node->m_relays_txs.load(), .fBloomFilter = node->m_bloom_filter_loaded.load(), .nKeyedNetGroup = node->nKeyedNetGroup, @@ -1014,7 +1010,6 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock, const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end(); CNode* pnode = new CNode(id, - nodeServices, std::move(sock), addr, CalculateKeyedNetGroup(addr), @@ -1026,7 +1021,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock, pnode->AddRef(); pnode->m_permissionFlags = permissionFlags; pnode->m_prefer_evict = discouraged; - m_msgproc->InitializeNode(pnode); + m_msgproc->InitializeNode(*pnode, nodeServices); LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString()); @@ -1964,7 +1959,7 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai if (grantOutbound) grantOutbound->MoveTo(pnode->grantOutbound); - m_msgproc->InitializeNode(pnode); + m_msgproc->InitializeNode(*pnode, nLocalServices); { LOCK(m_nodes_mutex); m_nodes.push_back(pnode); @@ -2708,7 +2703,10 @@ ServiceFlags CConnman::GetLocalServices() const unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; } -CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, std::shared_ptr<Sock> sock, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion) +CNode::CNode(NodeId idIn, std::shared_ptr<Sock> sock, const CAddress& addrIn, + uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, + const CAddress& addrBindIn, const std::string& addrNameIn, + ConnectionType conn_type_in, bool inbound_onion) : m_sock{sock}, m_connected{GetTime<std::chrono::seconds>()}, addr(addrIn), @@ -2718,8 +2716,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, std::shared_ptr<Sock> s nKeyedNetGroup(nKeyedNetGroupIn), id(idIn), nLocalHostNonce(nLocalHostNonceIn), - m_conn_type(conn_type_in), - nLocalServices(nLocalServicesIn) + m_conn_type(conn_type_in) { if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND); @@ -144,8 +144,8 @@ enum }; bool IsPeerAddrLocalGood(CNode *pnode); -/** Returns a local address that we should advertise to this peer */ -std::optional<CAddress> GetLocalAddrForPeer(CNode *pnode); +/** Returns a local address that we should advertise to this peer. */ +std::optional<CService> GetLocalAddrForPeer(CNode& node); /** * Mark a network as reachable or unreachable (no automatic connects to it) @@ -163,7 +163,7 @@ void RemoveLocal(const CService& addr); bool SeenLocal(const CService& addr); bool IsLocal(const CService& addr); bool GetLocal(CService &addr, const CNetAddr *paddrPeer = nullptr); -CAddress GetLocalAddress(const CNetAddr *paddrPeer, ServiceFlags nLocalServices); +CService GetLocalAddress(const CNetAddr& addrPeer); extern bool fDiscover; @@ -187,7 +187,6 @@ class CNodeStats { public: NodeId nodeid; - ServiceFlags nServices; std::chrono::seconds m_last_send; std::chrono::seconds m_last_recv; std::chrono::seconds m_last_tx_time; @@ -346,7 +345,6 @@ public: std::unique_ptr<TransportSerializer> m_serializer; NetPermissionFlags m_permissionFlags{NetPermissionFlags::None}; - std::atomic<ServiceFlags> nServices{NODE_NONE}; /** * Socket used for communication with the node. @@ -399,8 +397,6 @@ public: bool HasPermission(NetPermissionFlags permission) const { return NetPermissions::HasFlag(m_permissionFlags, permission); } - bool fClient{false}; // set by version message - bool m_limited_node{false}; //after BIP159, set by version message /** fSuccessfullyConnected is set to true on receiving VERACK from the peer. */ std::atomic_bool fSuccessfullyConnected{false}; // Setting fDisconnect to true will cause the node to be disconnected the @@ -484,6 +480,9 @@ public: // Peer selected us as (compact blocks) high-bandwidth peer (BIP152) std::atomic<bool> m_bip152_highbandwidth_from{false}; + /** Whether this peer provides all services that we want. Used for eviction decisions */ + std::atomic_bool m_has_all_wanted_services{false}; + /** Whether we should relay transactions to this peer (their version * message did not include fRelay=false and this is not a block-relay-only * connection). This only changes from false to true. It will never change @@ -514,7 +513,10 @@ public: * criterium in CConnman::AttemptToEvictConnection. */ std::atomic<std::chrono::microseconds> m_min_ping_time{std::chrono::microseconds::max()}; - CNode(NodeId id, ServiceFlags nLocalServicesIn, std::shared_ptr<Sock> sock, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion); + CNode(NodeId id, std::shared_ptr<Sock> sock, const CAddress& addrIn, + uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, + const CAddress& addrBindIn, const std::string& addrNameIn, + ConnectionType conn_type_in, bool inbound_onion); CNode(const CNode&) = delete; CNode& operator=(const CNode&) = delete; @@ -572,11 +574,6 @@ public: void CopyStats(CNodeStats& stats) EXCLUSIVE_LOCKS_REQUIRED(!m_subver_mutex, !m_addr_local_mutex, !cs_vSend, !cs_vRecv); - ServiceFlags GetLocalServices() const - { - return nLocalServices; - } - std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); } /** A ping-pong round trip has completed successfully. Update latest and minimum ping times. */ @@ -591,23 +588,6 @@ private: const ConnectionType m_conn_type; std::atomic<int> m_greatest_common_version{INIT_PROTO_VERSION}; - //! Services offered to this peer. - //! - //! This is supplied by the parent CConnman during peer connection - //! (CConnman::ConnectNode()) from its attribute of the same name. - //! - //! This is const because there is no protocol defined for renegotiating - //! services initially offered to a peer. The set of local services we - //! offer should not change after initialization. - //! - //! An interesting example of this is NODE_NETWORK and initial block - //! download: a node which starts up from scratch doesn't have any blocks - //! to serve, but still advertises NODE_NETWORK because it will eventually - //! fulfill this role after IBD completes. P2P code is written in such a - //! way that it can gracefully handle peers who don't make good on their - //! service advertisements. - const ServiceFlags nLocalServices; - std::list<CNetMessage> vRecvMsg; // Used only by SocketHandler thread // Our address, as reported by the peer @@ -625,7 +605,7 @@ class NetEventsInterface { public: /** Initialize a peer (setup state, queue any initial messages) */ - virtual void InitializeNode(CNode* pnode) = 0; + virtual void InitializeNode(CNode& node, ServiceFlags our_services) = 0; /** Handle removal of a peer (clear state) */ virtual void FinalizeNode(const CNode& node) = 0; @@ -1035,16 +1015,14 @@ private: std::map<uint64_t, CachedAddrResponse> m_addr_response_caches; /** - * Services this instance offers. + * Services this node offers. * - * This data is replicated in each CNode instance we create during peer - * connection (in ConnectNode()) under a member also called - * nLocalServices. + * This data is replicated in each Peer instance we create. * * This data is not marked const, but after being set it should not - * change. See the note in CNode::nLocalServices documentation. + * change. * - * \sa CNode::nLocalServices + * \sa Peer::our_services */ ServiceFlags nLocalServices; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 77fa23b594..74d1bf44d2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -207,6 +207,23 @@ struct Peer { /** Same id as the CNode object for this peer */ const NodeId m_id{0}; + /** Services we offered to this peer. + * + * This is supplied by CConnman during peer initialization. It's const + * because there is no protocol defined for renegotiating services + * initially offered to a peer. The set of local services we offer should + * not change after initialization. + * + * An interesting example of this is NODE_NETWORK and initial block + * download: a node which starts up from scratch doesn't have any blocks + * to serve, but still advertises NODE_NETWORK because it will eventually + * fulfill this role after IBD completes. P2P code is written in such a + * way that it can gracefully handle peers who don't make good on their + * service advertisements. */ + const ServiceFlags m_our_services; + /** Services this peer offered to us. */ + std::atomic<ServiceFlags> m_their_services{NODE_NONE}; + /** Protects misbehavior data members */ Mutex m_misbehavior_mutex; /** Accumulated misbehavior score for this peer */ @@ -360,8 +377,9 @@ struct Peer { /** Time of the last getheaders message to this peer */ NodeClock::time_point m_last_getheaders_timestamp{}; - Peer(NodeId id) + explicit Peer(NodeId id, ServiceFlags our_services) : m_id{id} + , m_our_services{our_services} {} private: @@ -410,8 +428,6 @@ struct CNodeState { bool m_requested_hb_cmpctblocks{false}; /** Whether this peer will send us cmpctblocks if we request them. */ bool m_provides_cmpctblocks{false}; - //! Whether this peer can give us witnesses - bool fHaveWitness{false}; /** State used to enforce CHAIN_SYNC_TIMEOUT and EXTRA_PEER_CHECK_INTERVAL logic. * @@ -482,7 +498,7 @@ public: EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex); /** Implement NetEventsInterface */ - void InitializeNode(CNode* pnode) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void InitializeNode(CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex); @@ -492,7 +508,8 @@ public: /** Implement PeerManager */ void StartScheduledTasks(CScheduler& scheduler) override; void CheckForStaleTipAndEvictPeers() override; - std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override; + std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool IgnoresIncomingTxs() override { return m_ignore_incoming_txs; } void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); @@ -578,7 +595,7 @@ private: */ bool MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer); /** Potentially fetch blocks from this peer upon receipt of a new headers tip */ - void HeadersDirectFetchBlocks(CNode& pfrom, const CBlockIndex* pindexLast); + void HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex* pindexLast); /** Update peer state based on received headers message */ void UpdatePeerStateForReceivedHeaders(CNode& pfrom, const CBlockIndex *pindexLast, bool received_new_header, bool may_have_more_headers); @@ -657,7 +674,7 @@ private: /** Get a pointer to a mutable CNodeState. */ CNodeState* State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - uint32_t GetFetchFlags(const CNode& pfrom) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + uint32_t GetFetchFlags(const Peer& peer) const; std::atomic<std::chrono::microseconds> m_next_inv_to_inbounds{0us}; @@ -778,7 +795,7 @@ private: /** Update pindexLastCommonBlock and add not-in-flight missing successors to vBlocks, until it has * at most count entries. */ - void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void FindNextBlocksToDownload(const Peer& peer, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller) EXCLUSIVE_LOCKS_REQUIRED(cs_main); std::map<uint256, std::pair<NodeId, std::list<QueuedBlock>::iterator> > mapBlocksInFlight GUARDED_BY(cs_main); @@ -848,6 +865,7 @@ private: * * May disconnect from the peer in the case of a bad request. * + * @param[in] node The node that we received the request from * @param[in] peer The peer that we received the request from * @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 @@ -857,7 +875,7 @@ private: * @param[out] filter_index The filter index, if the request can be serviced. * @return True if the request can be serviced. */ - bool PrepareBlockFilterRequest(CNode& peer, + bool PrepareBlockFilterRequest(CNode& node, Peer& peer, BlockFilterType filter_type, uint32_t start_height, const uint256& stop_hash, uint32_t max_height_diff, const CBlockIndex*& stop_index, @@ -868,30 +886,33 @@ private: * * May disconnect from the peer in the case of a bad request. * + * @param[in] node The node that we received the request from * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received */ - void ProcessGetCFilters(CNode& peer, CDataStream& vRecv); + void ProcessGetCFilters(CNode& node, Peer& peer, CDataStream& vRecv); /** * Handle a cfheaders request. * * May disconnect from the peer in the case of a bad request. * + * @param[in] node The node that we received the request from * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received */ - void ProcessGetCFHeaders(CNode& peer, CDataStream& vRecv); + void ProcessGetCFHeaders(CNode& node, Peer& peer, CDataStream& vRecv); /** * Handle a getcfcheckpt request. * * May disconnect from the peer in the case of a bad request. * + * @param[in] node The node that we received the request from * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received */ - void ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv); + void ProcessGetCFCheckPt(CNode& node, Peer& peer, CDataStream& vRecv); /** Checks if address relay is permitted with peer. If needed, initializes * the m_addr_known bloom filter and sets m_addr_relay_enabled to true. @@ -955,6 +976,26 @@ static void AddKnownTx(Peer& peer, const uint256& hash) tx_relay->m_tx_inventory_known_filter.insert(hash); } +/** Whether this peer can serve us blocks. */ +static bool CanServeBlocks(const Peer& peer) +{ + return peer.m_their_services & (NODE_NETWORK|NODE_NETWORK_LIMITED); +} + +/** Whether this peer can only serve limited recent blocks (e.g. because + * it prunes old blocks) */ +static bool IsLimitedPeer(const Peer& peer) +{ + return (!(peer.m_their_services & NODE_NETWORK) && + (peer.m_their_services & NODE_NETWORK_LIMITED)); +} + +/** Whether this peer can serve us witness data */ +static bool CanServeWitnesses(const Peer& peer) +{ + return peer.m_their_services & NODE_WITNESS; +} + std::chrono::microseconds PeerManagerImpl::NextInvToInbounds(std::chrono::microseconds now, std::chrono::seconds average_interval) { @@ -1148,17 +1189,17 @@ void PeerManagerImpl::UpdateBlockAvailability(NodeId nodeid, const uint256 &hash } } -void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller) +void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller) { if (count == 0) return; vBlocks.reserve(vBlocks.size() + count); - CNodeState *state = State(nodeid); + CNodeState *state = State(peer.m_id); assert(state != nullptr); // Make sure pindexBestKnownBlock is up to date, we'll need it. - ProcessBlockAvailability(nodeid); + ProcessBlockAvailability(peer.m_id); if (state->pindexBestKnownBlock == nullptr || state->pindexBestKnownBlock->nChainWork < m_chainman.ActiveChain().Tip()->nChainWork || state->pindexBestKnownBlock->nChainWork < nMinimumChainWork) { // This peer has nothing interesting. @@ -1206,7 +1247,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count // We consider the chain that this peer is on invalid. return; } - if (!State(nodeid)->fHaveWitness && DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) { + if (!CanServeWitnesses(peer) && DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) { // We wouldn't download this block or its descendants from this peer. return; } @@ -1217,7 +1258,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count // The block is not already downloaded, and not yet in flight. if (pindex->nHeight > nWindowEnd) { // We reached the end of the window. - if (vBlocks.size() == 0 && waitingfor != nodeid) { + if (vBlocks.size() == 0 && waitingfor != peer.m_id) { // We aren't able to fetch anything, but we would be if the download window was one larger. nodeStaller = waitingfor; } @@ -1239,10 +1280,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer) { - // Note that pnode->GetLocalServices() is a reflection of the local - // services we were offering when the CNode object was created for this - // peer. - uint64_t my_services{pnode.GetLocalServices()}; + uint64_t my_services{peer.m_our_services}; const int64_t nTime{count_seconds(GetTime<std::chrono::seconds>())}; uint64_t nonce = pnode.GetLocalNonce(); const int nNodeStartingHeight{m_best_height}; @@ -1299,21 +1337,21 @@ void PeerManagerImpl::UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_s if (state) state->m_last_block_announcement = time_in_seconds; } -void PeerManagerImpl::InitializeNode(CNode *pnode) +void PeerManagerImpl::InitializeNode(CNode& node, ServiceFlags our_services) { - NodeId nodeid = pnode->GetId(); + NodeId nodeid = node.GetId(); { LOCK(cs_main); - m_node_states.emplace_hint(m_node_states.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(pnode->IsInboundConn())); + m_node_states.emplace_hint(m_node_states.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(node.IsInboundConn())); assert(m_txrequest.Count(nodeid) == 0); } - PeerRef peer = std::make_shared<Peer>(nodeid); + PeerRef peer = std::make_shared<Peer>(nodeid, our_services); { LOCK(m_peer_mutex); m_peer_map.emplace_hint(m_peer_map.end(), nodeid, peer); } - if (!pnode->IsInboundConn()) { - PushNodeVersion(*pnode, *peer); + if (!node.IsInboundConn()) { + PushNodeVersion(node, *peer); } } @@ -1431,6 +1469,7 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c PeerRef peer = GetPeerRef(nodeid); if (peer == nullptr) return false; + stats.their_services = peer->m_their_services; stats.m_starting_height = peer->m_starting_height; // It is common for nodes with good ping times to suddenly become lagged, // due to a new block arriving or other large transfer. @@ -1585,12 +1624,14 @@ std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, const CBl if (fImporting) return "Importing..."; if (fReindex) return "Reindexing..."; - LOCK(cs_main); // Ensure this peer exists and hasn't been disconnected - CNodeState* state = State(peer_id); - if (state == nullptr) return "Peer does not exist"; + PeerRef peer = GetPeerRef(peer_id); + if (peer == nullptr) return "Peer does not exist"; + // Ignore pre-segwit peers - if (!state->fHaveWitness) return "Pre-SegWit peer"; + if (!CanServeWitnesses(*peer)) return "Pre-SegWit peer"; + + LOCK(cs_main); // Mark block as in-flight unless it already is (for this peer). // If a block was already in-flight for a different peer, its BLOCKTXN @@ -1974,7 +2015,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } // Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold if (!pfrom.HasPermission(NetPermissionFlags::NoBan) && ( - (((pfrom.GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom.GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (m_chainman.ActiveChain().Tip()->nHeight - pindex->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) ) + (((peer.m_our_services & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((peer.m_our_services & NODE_NETWORK) != NODE_NETWORK) && (m_chainman.ActiveChain().Tip()->nHeight - pindex->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) ) )) { LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold, disconnect peer=%d\n", pfrom.GetId()); //disconnect node and prevent it from stalling (would otherwise wait for the missing block) @@ -2191,10 +2232,10 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic } } -uint32_t PeerManagerImpl::GetFetchFlags(const CNode& pfrom) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) +uint32_t PeerManagerImpl::GetFetchFlags(const Peer& peer) const { uint32_t nFetchFlags = 0; - if (State(pfrom.GetId())->fHaveWitness) { + if (CanServeWitnesses(peer)) { nFetchFlags |= MSG_WITNESS_FLAG; } return nFetchFlags; @@ -2289,7 +2330,7 @@ bool PeerManagerImpl::MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& loc * We require that the given tip have at least as much work as our tip, and for * our current tip to be "close to synced" (see CanDirectFetch()). */ -void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const CBlockIndex* pindexLast) +void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex* pindexLast) { const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); @@ -2304,7 +2345,7 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const CBlockIndex* while (pindexWalk && !m_chainman.ActiveChain().Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && !IsBlockRequested(pindexWalk->GetBlockHash()) && - (!DeploymentActiveAt(*pindexWalk, m_chainman, Consensus::DEPLOYMENT_SEGWIT) || State(pfrom.GetId())->fHaveWitness)) { + (!DeploymentActiveAt(*pindexWalk, m_chainman, Consensus::DEPLOYMENT_SEGWIT) || CanServeWitnesses(peer))) { // We don't have this block, and it's not yet in flight. vToFetch.push_back(pindexWalk); } @@ -2326,7 +2367,7 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const CBlockIndex* // Can't download any more from this peer break; } - uint32_t nFetchFlags = GetFetchFlags(pfrom); + uint32_t nFetchFlags = GetFetchFlags(peer); vGetData.push_back(CInv(MSG_BLOCK | nFetchFlags, pindex->GetBlockHash())); BlockRequested(pfrom.GetId(), *pindex); LogPrint(BCLog::NET, "Requesting block %s from peer=%d\n", @@ -2471,7 +2512,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, UpdatePeerStateForReceivedHeaders(pfrom, pindexLast, received_new_header, nCount == MAX_HEADERS_RESULTS); // Consider immediately downloading blocks. - HeadersDirectFetchBlocks(pfrom, pindexLast); + HeadersDirectFetchBlocks(pfrom, peer, pindexLast); return; } @@ -2555,7 +2596,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set) } } -bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer, +bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, BlockFilterType filter_type, uint32_t start_height, const uint256& stop_hash, uint32_t max_height_diff, const CBlockIndex*& stop_index, @@ -2563,11 +2604,11 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer, { const bool supported_filter_type = (filter_type == BlockFilterType::BASIC && - (peer.GetLocalServices() & NODE_COMPACT_FILTERS)); + (peer.m_our_services & NODE_COMPACT_FILTERS)); if (!supported_filter_type) { LogPrint(BCLog::NET, "peer %d requested unsupported block filter type: %d\n", - peer.GetId(), static_cast<uint8_t>(filter_type)); - peer.fDisconnect = true; + node.GetId(), static_cast<uint8_t>(filter_type)); + node.fDisconnect = true; return false; } @@ -2578,8 +2619,8 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer, // Check that the stop block exists and the peer would be allowed to fetch it. if (!stop_index || !BlockRequestAllowed(stop_index)) { LogPrint(BCLog::NET, "peer %d requested invalid block hash: %s\n", - peer.GetId(), stop_hash.ToString()); - peer.fDisconnect = true; + node.GetId(), stop_hash.ToString()); + node.fDisconnect = true; return false; } } @@ -2588,14 +2629,14 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer, if (start_height > stop_height) { LogPrint(BCLog::NET, "peer %d sent invalid getcfilters/getcfheaders with " /* Continued */ "start height %d and stop height %d\n", - peer.GetId(), start_height, stop_height); - peer.fDisconnect = true; + node.GetId(), start_height, stop_height); + node.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", - peer.GetId(), stop_height - start_height + 1, max_height_diff); - peer.fDisconnect = true; + node.GetId(), stop_height - start_height + 1, max_height_diff); + node.fDisconnect = true; return false; } @@ -2608,7 +2649,7 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer, return true; } -void PeerManagerImpl::ProcessGetCFilters(CNode& peer, CDataStream& vRecv) +void PeerManagerImpl::ProcessGetCFilters(CNode& node,Peer& peer, CDataStream& vRecv) { uint8_t filter_type_ser; uint32_t start_height; @@ -2620,7 +2661,7 @@ void PeerManagerImpl::ProcessGetCFilters(CNode& peer, CDataStream& vRecv) const CBlockIndex* stop_index; BlockFilterIndex* filter_index; - if (!PrepareBlockFilterRequest(peer, filter_type, start_height, stop_hash, + if (!PrepareBlockFilterRequest(node, peer, filter_type, start_height, stop_hash, MAX_GETCFILTERS_SIZE, stop_index, filter_index)) { return; } @@ -2633,13 +2674,13 @@ void PeerManagerImpl::ProcessGetCFilters(CNode& peer, CDataStream& vRecv) } for (const auto& filter : filters) { - CSerializedNetMsg msg = CNetMsgMaker(peer.GetCommonVersion()) + CSerializedNetMsg msg = CNetMsgMaker(node.GetCommonVersion()) .Make(NetMsgType::CFILTER, filter); - m_connman.PushMessage(&peer, std::move(msg)); + m_connman.PushMessage(&node, std::move(msg)); } } -void PeerManagerImpl::ProcessGetCFHeaders(CNode& peer, CDataStream& vRecv) +void PeerManagerImpl::ProcessGetCFHeaders(CNode& node, Peer& peer, CDataStream& vRecv) { uint8_t filter_type_ser; uint32_t start_height; @@ -2651,7 +2692,7 @@ void PeerManagerImpl::ProcessGetCFHeaders(CNode& peer, CDataStream& vRecv) const CBlockIndex* stop_index; BlockFilterIndex* filter_index; - if (!PrepareBlockFilterRequest(peer, filter_type, start_height, stop_hash, + if (!PrepareBlockFilterRequest(node, peer, filter_type, start_height, stop_hash, MAX_GETCFHEADERS_SIZE, stop_index, filter_index)) { return; } @@ -2674,16 +2715,16 @@ void PeerManagerImpl::ProcessGetCFHeaders(CNode& peer, CDataStream& vRecv) return; } - CSerializedNetMsg msg = CNetMsgMaker(peer.GetCommonVersion()) + CSerializedNetMsg msg = CNetMsgMaker(node.GetCommonVersion()) .Make(NetMsgType::CFHEADERS, filter_type_ser, stop_index->GetBlockHash(), prev_header, filter_hashes); - m_connman.PushMessage(&peer, std::move(msg)); + m_connman.PushMessage(&node, std::move(msg)); } -void PeerManagerImpl::ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv) +void PeerManagerImpl::ProcessGetCFCheckPt(CNode& node, Peer& peer, CDataStream& vRecv) { uint8_t filter_type_ser; uint256 stop_hash; @@ -2694,7 +2735,7 @@ void PeerManagerImpl::ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv) const CBlockIndex* stop_index; BlockFilterIndex* filter_index; - if (!PrepareBlockFilterRequest(peer, filter_type, /*start_height=*/0, stop_hash, + if (!PrepareBlockFilterRequest(node, peer, filter_type, /*start_height=*/0, stop_hash, /*max_height_diff=*/std::numeric_limits<uint32_t>::max(), stop_index, filter_index)) { return; @@ -2715,12 +2756,12 @@ void PeerManagerImpl::ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv) } } - CSerializedNetMsg msg = CNetMsgMaker(peer.GetCommonVersion()) + CSerializedNetMsg msg = CNetMsgMaker(node.GetCommonVersion()) .Make(NetMsgType::CFCHECKPT, filter_type_ser, stop_index->GetBlockHash(), headers); - m_connman.PushMessage(&peer, std::move(msg)); + m_connman.PushMessage(&node, std::move(msg)); } void PeerManagerImpl::ProcessBlock(CNode& node, const std::shared_ptr<const CBlock>& block, bool force_processing) @@ -2842,7 +2883,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK)); - pfrom.nServices = nServices; + pfrom.m_has_all_wanted_services = HasAllDesirableServiceFlags(nServices); + peer->m_their_services = nServices; pfrom.SetAddrLocal(addrMe); { LOCK(pfrom.m_subver_mutex); @@ -2850,18 +2892,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } peer->m_starting_height = starting_height; - // set nodes not relaying blocks and tx and not serving (parts) of the historical blockchain as "clients" - pfrom.fClient = (!(nServices & NODE_NETWORK) && !(nServices & NODE_NETWORK_LIMITED)); - - // set nodes not capable of serving the complete blockchain history as "limited nodes" - pfrom.m_limited_node = (!(nServices & NODE_NETWORK) && (nServices & NODE_NETWORK_LIMITED)); - // We only initialize the m_tx_relay data structure if: // - this isn't an outbound block-relay-only connection; and // - fRelay=true or we're offering NODE_BLOOM to this peer // (NODE_BLOOM means that the peer may turn on tx relay later) if (!pfrom.IsBlockOnlyConn() && - (fRelay || (pfrom.GetLocalServices() & NODE_BLOOM))) { + (fRelay || (peer->m_our_services & NODE_BLOOM))) { auto* const tx_relay = peer->SetTxRelay(); { LOCK(tx_relay->m_bloom_filter_mutex); @@ -2870,17 +2906,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (fRelay) pfrom.m_relays_txs = true; } - if((nServices & NODE_WITNESS)) - { - LOCK(cs_main); - State(pfrom.GetId())->fHaveWitness = true; - } - // Potentially mark this peer as a preferred download peer. { LOCK(cs_main); CNodeState* state = State(pfrom.GetId()); - state->fPreferredDownload = (!pfrom.IsInboundConn() || pfrom.HasPermission(NetPermissionFlags::NoBan)) && !pfrom.IsAddrFetchConn() && !pfrom.fClient; + state->fPreferredDownload = (!pfrom.IsInboundConn() || pfrom.HasPermission(NetPermissionFlags::NoBan)) && !pfrom.IsAddrFetchConn() && CanServeBlocks(*peer); m_num_preferred_download_peers += state->fPreferredDownload; } @@ -2899,7 +2929,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // indicate to the peer that we will participate in addr relay. if (fListen && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) { - CAddress addr = GetLocalAddress(&pfrom.addr, pfrom.GetLocalServices()); + CAddress addr{GetLocalAddress(pfrom.addr), peer->m_our_services, (uint32_t)GetAdjustedTime()}; FastRandomContext insecure_rand; if (addr.IsRoutable()) { @@ -3758,7 +3788,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // We requested this block for some reason, but our mempool will probably be useless // so we just grab the block via normal getdata std::vector<CInv> vInv(1); - vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); + vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), cmpctblock.header.GetHash()); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); } return; @@ -3794,7 +3824,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } else if (status == READ_STATUS_FAILED) { // Duplicate txindexes, the block is now in-flight, so just request it std::vector<CInv> vInv(1); - vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); + vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), cmpctblock.header.GetHash()); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); return; } @@ -3837,7 +3867,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // We requested this block, but its far into the future, so our // mempool will probably be useless - request the block normally std::vector<CInv> vInv(1); - vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); + vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), cmpctblock.header.GetHash()); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); return; } else { @@ -3921,7 +3951,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :( std::vector<CInv> invs; - invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(pfrom), resp.blockhash)); + invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(*peer), resp.blockhash)); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs)); } else { // Block is either okay, or possibly we received @@ -4061,7 +4091,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::MEMPOOL) { - if (!(pfrom.GetLocalServices() & NODE_BLOOM) && !pfrom.HasPermission(NetPermissionFlags::Mempool)) + if (!(peer->m_our_services & NODE_BLOOM) && !pfrom.HasPermission(NetPermissionFlags::Mempool)) { if (!pfrom.HasPermission(NetPermissionFlags::NoBan)) { @@ -4164,7 +4194,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::FILTERLOAD) { - if (!(pfrom.GetLocalServices() & NODE_BLOOM)) { + if (!(peer->m_our_services & NODE_BLOOM)) { LogPrint(BCLog::NET, "filterload received despite not offering bloom services from peer=%d; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; @@ -4189,7 +4219,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::FILTERADD) { - if (!(pfrom.GetLocalServices() & NODE_BLOOM)) { + if (!(peer->m_our_services & NODE_BLOOM)) { LogPrint(BCLog::NET, "filteradd received despite not offering bloom services from peer=%d; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; @@ -4217,7 +4247,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::FILTERCLEAR) { - if (!(pfrom.GetLocalServices() & NODE_BLOOM)) { + if (!(peer->m_our_services & NODE_BLOOM)) { LogPrint(BCLog::NET, "filterclear received despite not offering bloom services from peer=%d; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; @@ -4248,17 +4278,17 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::GETCFILTERS) { - ProcessGetCFilters(pfrom, vRecv); + ProcessGetCFilters(pfrom, *peer, vRecv); return; } if (msg_type == NetMsgType::GETCFHEADERS) { - ProcessGetCFHeaders(pfrom, vRecv); + ProcessGetCFHeaders(pfrom, *peer, vRecv); return; } if (msg_type == NetMsgType::GETCFCHECKPT) { - ProcessGetCFCheckPt(pfrom, vRecv); + ProcessGetCFCheckPt(pfrom, *peer, vRecv); return; } @@ -4654,9 +4684,10 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros if (peer.m_next_local_addr_send != 0us) { peer.m_addr_known->reset(); } - if (std::optional<CAddress> local_addr = GetLocalAddrForPeer(&node)) { + if (std::optional<CService> local_service = GetLocalAddrForPeer(node)) { + CAddress local_addr{*local_service, peer.m_our_services, (uint32_t)GetAdjustedTime()}; FastRandomContext insecure_rand; - PushAddress(peer, *local_addr, insecure_rand); + PushAddress(peer, local_addr, insecure_rand); } peer.m_next_local_addr_send = GetExponentialRand(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); } @@ -4840,7 +4871,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) bool sync_blocks_and_headers_from_peer = false; if (state.fPreferredDownload) { sync_blocks_and_headers_from_peer = true; - } else if (!pto->fClient && !pto->IsAddrFetchConn()) { + } else if (CanServeBlocks(*peer) && !pto->IsAddrFetchConn()) { // Typically this is an inbound peer. If we don't have any outbound // peers, or if we aren't downloading any blocks from such peers, // then allow block downloads from this peer, too. @@ -4855,7 +4886,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } } - if (!state.fSyncStarted && !pto->fClient && !fImporting && !fReindex) { + if (!state.fSyncStarted && CanServeBlocks(*peer) && !fImporting && !fReindex) { // Only actively request headers from a single peer, unless we're close to today. if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->GetBlockTime() > GetAdjustedTime() - 24 * 60 * 60) { const CBlockIndex* pindexStart = m_chainman.m_best_header; @@ -5232,12 +5263,12 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Message: getdata (blocks) // std::vector<CInv> vGetData; - if (!pto->fClient && ((sync_blocks_and_headers_from_peer && !pto->m_limited_node) || !m_chainman.ActiveChainstate().IsInitialBlockDownload()) && state.nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER) { + if (CanServeBlocks(*peer) && ((sync_blocks_and_headers_from_peer && !IsLimitedPeer(*peer)) || !m_chainman.ActiveChainstate().IsInitialBlockDownload()) && state.nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER) { std::vector<const CBlockIndex*> vToDownload; NodeId staller = -1; - FindNextBlocksToDownload(pto->GetId(), MAX_BLOCKS_IN_TRANSIT_PER_PEER - state.nBlocksInFlight, vToDownload, staller); + FindNextBlocksToDownload(*peer, MAX_BLOCKS_IN_TRANSIT_PER_PEER - state.nBlocksInFlight, vToDownload, staller); for (const CBlockIndex *pindex : vToDownload) { - uint32_t nFetchFlags = GetFetchFlags(*pto); + uint32_t nFetchFlags = GetFetchFlags(*peer); vGetData.push_back(CInv(MSG_BLOCK | nFetchFlags, pindex->GetBlockHash())); BlockRequested(pto->GetId(), *pindex); LogPrint(BCLog::NET, "Requesting block %s (%d) peer=%d\n", pindex->GetBlockHash().ToString(), @@ -5264,7 +5295,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (!AlreadyHaveTx(gtxid)) { LogPrint(BCLog::NET, "Requesting %s %s peer=%d\n", gtxid.IsWtxid() ? "wtx" : "tx", gtxid.GetHash().ToString(), pto->GetId()); - vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*pto)), gtxid.GetHash()); + vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.GetHash()); if (vGetData.size() >= MAX_GETDATA_SZ) { m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::GETDATA, vGetData)); vGetData.clear(); diff --git a/src/net_processing.h b/src/net_processing.h index 5fbae98c27..bcda9614d4 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -34,6 +34,7 @@ struct CNodeStateStats { uint64_t m_addr_processed = 0; uint64_t m_addr_rate_limited = 0; bool m_addr_relay_enabled{false}; + ServiceFlags their_services; }; class PeerManager : public CValidationInterface, public NetEventsInterface diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index cadafcaa8d..103f4f0d7f 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -823,7 +823,7 @@ struct CImportingNow { } }; -void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args) +void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args, const fs::path& mempool_path) { SetSyscallSandboxPolicy(SyscallSandboxPolicy::INITIALIZATION_LOAD_BLOCKS); ScheduleBatchPriority(); @@ -893,6 +893,6 @@ void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFile return; } } // End scope of CImportingNow - chainman.ActiveChainstate().LoadMempool(args); + chainman.ActiveChainstate().LoadMempool(mempool_path); } } // namespace node diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index e017f3f427..9b76371aae 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -211,7 +211,7 @@ bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, c bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex); -void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args); +void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args, const fs::path& mempool_path); } // namespace node #endif // BITCOIN_NODE_BLOCKSTORAGE_H diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 93aa9e526b..46e0efb9b6 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -775,6 +775,11 @@ public: notifications.transactionAddedToMempool(entry.GetSharedTx(), 0 /* mempool_sequence */); } } + bool hasAssumedValidChain() override + { + return Assert(m_node.chainman)->IsSnapshotActive(); + } + NodeContext& m_node; }; } // namespace diff --git a/src/node/mempool_persist_args.cpp b/src/node/mempool_persist_args.cpp new file mode 100644 index 0000000000..4e775869c6 --- /dev/null +++ b/src/node/mempool_persist_args.cpp @@ -0,0 +1,23 @@ +// Copyright (c) 2022 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 <node/mempool_persist_args.h> + +#include <fs.h> +#include <util/system.h> +#include <validation.h> + +namespace node { + +bool ShouldPersistMempool(const ArgsManager& argsman) +{ + return argsman.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL); +} + +fs::path MempoolPath(const ArgsManager& argsman) +{ + return argsman.GetDataDirNet() / "mempool.dat"; +} + +} // namespace node diff --git a/src/node/mempool_persist_args.h b/src/node/mempool_persist_args.h new file mode 100644 index 0000000000..f719ec62ab --- /dev/null +++ b/src/node/mempool_persist_args.h @@ -0,0 +1,25 @@ +// Copyright (c) 2022 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_NODE_MEMPOOL_PERSIST_ARGS_H +#define BITCOIN_NODE_MEMPOOL_PERSIST_ARGS_H + +#include <fs.h> + +class ArgsManager; + +namespace node { + +/** + * Default for -persistmempool, indicating whether the node should attempt to + * automatically load the mempool on start and save to disk on shutdown + */ +static constexpr bool DEFAULT_PERSIST_MEMPOOL{true}; + +bool ShouldPersistMempool(const ArgsManager& argsman); +fs::path MempoolPath(const ArgsManager& argsman); + +} // namespace node + +#endif // BITCOIN_NODE_MEMPOOL_PERSIST_ARGS_H diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 27d3a1b9e2..33c60deafb 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -77,8 +77,6 @@ Q_DECLARE_METATYPE(CAmount) Q_DECLARE_METATYPE(SynchronizationState) Q_DECLARE_METATYPE(uint256) -using node::NodeContext; - static void RegisterMetaTypes() { // Register meta types used for QMetaObject::invokeMethod and Qt::QueuedConnection diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index b791fd30c4..70fccdef1c 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1162,7 +1162,6 @@ void RPCConsole::updateDetailWidget() if (!stats->nodeStats.addrLocal.empty()) peerAddrDetails += "<br />" + tr("via %1").arg(QString::fromStdString(stats->nodeStats.addrLocal)); ui->peerHeading->setText(peerAddrDetails); - ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStats.nServices)); QString bip152_hb_settings; if (stats->nodeStats.m_bip152_highbandwidth_to) bip152_hb_settings = ts.to; if (stats->nodeStats.m_bip152_highbandwidth_from) bip152_hb_settings += (bip152_hb_settings.isEmpty() ? ts.from : QLatin1Char('/') + ts.from); @@ -1197,6 +1196,7 @@ void RPCConsole::updateDetailWidget() // This check fails for example if the lock was busy and // nodeStateStats couldn't be fetched. if (stats->fNodeStateStatsAvailable) { + ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStateStats.their_services)); // Sync height is init to -1 if (stats->nodeStateStats.nSyncHeight > -1) { ui->peerSyncHeight->setText(QString("%1").arg(stats->nodeStateStats.nSyncHeight)); diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index de23f51c92..846fa519ee 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -43,8 +43,6 @@ Q_IMPORT_PLUGIN(QAndroidPlatformIntegrationPlugin) #endif #endif -using node::NodeContext; - const std::function<void(const std::string&)> G_TEST_LOG_FUN{}; const std::function<std::vector<const char*>()> G_TEST_COMMAND_LINE_ARGUMENTS{}; diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index 6b38e207d3..8dc97e66a2 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -213,6 +213,14 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard) } std::ifstream in{filename.toLocal8Bit().data(), std::ios::binary}; data.assign(std::istream_iterator<unsigned char>{in}, {}); + + // Some psbt files may be base64 strings in the file rather than binary data + std::string b64_str{data.begin(), data.end()}; + b64_str.erase(b64_str.find_last_not_of(" \t\n\r\f\v") + 1); // Trim trailing whitespace + auto b64_dec = DecodeBase64(b64_str); + if (b64_dec.has_value()) { + data = b64_dec.value(); + } } std::string error; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index b80cdce3f0..52d5eaaa50 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -617,7 +617,7 @@ const RPCResult getblock_vin{ {RPCResult::Type::OBJ, "scriptPubKey", "", { {RPCResult::Type::STR, "asm", "The asm"}, - {RPCResult::Type::STR, "hex", "The hex"}, + {RPCResult::Type::STR_HEX, "hex", "The hex"}, {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, {RPCResult::Type::STR, "type", "The type (one of: " + GetAllOutputTypes() + ")"}, }}, @@ -1014,9 +1014,9 @@ static RPCHelpMan gettxout() {RPCResult::Type::NUM, "confirmations", "The number of confirmations"}, {RPCResult::Type::STR_AMOUNT, "value", "The transaction value in " + CURRENCY_UNIT}, {RPCResult::Type::OBJ, "scriptPubKey", "", { - {RPCResult::Type::STR, "asm", ""}, + {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, - {RPCResult::Type::STR_HEX, "hex", ""}, + {RPCResult::Type::STR_HEX, "hex", "The hex"}, {RPCResult::Type::STR, "type", "The type, eg pubkeyhash"}, {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, }}, diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 84d43e7818..d59ff3f75c 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -5,9 +5,12 @@ #include <rpc/blockchain.h> +#include <kernel/mempool_persist.h> + #include <chainparams.h> #include <core_io.h> #include <fs.h> +#include <node/mempool_persist_args.h> #include <policy/rbf.h> #include <policy/settings.h> #include <primitives/transaction.h> @@ -18,7 +21,10 @@ #include <univalue.h> #include <util/moneystr.h> +using kernel::DumpMempool; + using node::DEFAULT_MAX_RAW_TX_FEE_RATE; +using node::MempoolPath; using node::NodeContext; static RPCHelpMan sendrawtransaction() @@ -653,7 +659,7 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool) // Make sure this call is atomic in the pool. LOCK(pool.cs); UniValue ret(UniValue::VOBJ); - ret.pushKV("loaded", pool.IsLoaded()); + ret.pushKV("loaded", pool.GetLoadTried()); ret.pushKV("size", (int64_t)pool.size()); ret.pushKV("bytes", (int64_t)pool.GetTotalTxSize()); ret.pushKV("usage", (int64_t)pool.DynamicMemoryUsage()); @@ -717,16 +723,18 @@ static RPCHelpMan savemempool() const ArgsManager& args{EnsureAnyArgsman(request.context)}; const CTxMemPool& mempool = EnsureAnyMemPool(request.context); - if (!mempool.IsLoaded()) { + if (!mempool.GetLoadTried()) { throw JSONRPCError(RPC_MISC_ERROR, "The mempool was not loaded yet"); } - if (!DumpMempool(mempool)) { + const fs::path& dump_path = MempoolPath(args); + + if (!DumpMempool(mempool, dump_path)) { throw JSONRPCError(RPC_MISC_ERROR, "Unable to dump mempool to disk"); } UniValue ret(UniValue::VOBJ); - ret.pushKV("filename", fs::path((args.GetDataDirNet() / "mempool.dat")).u8string()); + ret.pushKV("filename", dump_path.u8string()); return ret; }, diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index fad92629c5..27eea824bc 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -195,8 +195,9 @@ static RPCHelpMan getpeerinfo() if (stats.m_mapped_as != 0) { obj.pushKV("mapped_as", uint64_t(stats.m_mapped_as)); } - obj.pushKV("services", strprintf("%016x", stats.nServices)); - obj.pushKV("servicesnames", GetServicesNames(stats.nServices)); + ServiceFlags services{fStateStats ? statestats.their_services : ServiceFlags::NODE_NONE}; + obj.pushKV("services", strprintf("%016x", services)); + obj.pushKV("servicesnames", GetServicesNames(services)); obj.pushKV("lastsend", count_seconds(stats.m_last_send)); obj.pushKV("lastrecv", count_seconds(stats.m_last_recv)); obj.pushKV("last_transaction", count_seconds(stats.m_last_tx_time)); diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index f4bb76f50f..744f809814 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -26,11 +26,6 @@ #include <tuple> #include <vector> -namespace node { -struct NodeContext; -} -using node::NodeContext; - static RPCHelpMan validateaddress() { return RPCHelpMan{ diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 5ca921ae2f..16105a85d5 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -46,12 +46,10 @@ #include <univalue.h> using node::AnalyzePSBT; -using node::BroadcastTransaction; using node::FindCoins; using node::GetTransaction; using node::NodeContext; using node::PSBTAnalysis; -using node::ReadBlockFromDisk; static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry, CChainState& active_chainstate) { @@ -730,7 +728,7 @@ const RPCResult decodepsbt_inputs{ {RPCResult::Type::OBJ, "final_scriptSig", /*optional=*/true, "", { {RPCResult::Type::STR, "asm", "The asm"}, - {RPCResult::Type::STR, "hex", "The hex"}, + {RPCResult::Type::STR_HEX, "hex", "The hex"}, }}, {RPCResult::Type::ARR, "final_scriptwitness", /*optional=*/true, "", { diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp index 50eb479035..1d9a037a66 100644 --- a/src/test/coinstatsindex_tests.cpp +++ b/src/test/coinstatsindex_tests.cpp @@ -13,9 +13,6 @@ #include <chrono> -using kernel::CCoinsStats; -using kernel::CoinStatsHashType; - BOOST_AUTO_TEST_SUITE(coinstatsindex_tests) static void IndexWaitSynced(BaseIndex& index) diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index c87ed82c88..66c4605afa 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -15,6 +15,7 @@ #include <serialize.h> #include <test/util/net.h> #include <test/util/setup_common.h> +#include <timedata.h> #include <util/string.h> #include <util/system.h> #include <util/time.h> @@ -44,17 +45,15 @@ BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup) // work. BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) { - auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); + ConnmanTestMsg& connman = static_cast<ConnmanTestMsg&>(*m_node.connman); // Disable inactivity checks for this test to avoid interference - static_cast<ConnmanTestMsg*>(connman.get())->SetPeerConnectTimeout(99999s); - auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, - *m_node.chainman, *m_node.mempool, false); + connman.SetPeerConnectTimeout(99999s); + PeerManager& peerman = *m_node.peerman; // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); NodeId id{0}; CNode dummyNode1{id++, - ServiceFlags(NODE_NETWORK | NODE_WITNESS), /*sock=*/nullptr, addr1, /*nKeyedNetGroupIn=*/0, @@ -63,10 +62,16 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) /*addrNameIn=*/"", ConnectionType::OUTBOUND_FULL_RELAY, /*inbound_onion=*/false}; - dummyNode1.SetCommonVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(&dummyNode1); - dummyNode1.fSuccessfullyConnected = true; + connman.Handshake( + /*node=*/dummyNode1, + /*successfully_connected=*/true, + /*remote_services=*/ServiceFlags(NODE_NETWORK | NODE_WITNESS), + /*local_services=*/ServiceFlags(NODE_NETWORK | NODE_WITNESS), + /*permission_flags=*/NetPermissionFlags::None, + /*version=*/PROTOCOL_VERSION, + /*relay_txs=*/true); + TestOnlyResetTimeData(); // This test requires that we have a chain with non-zero work. { @@ -78,7 +83,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) // Test starts here { LOCK(dummyNode1.cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); // should result in getheaders + BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders } { LOCK(dummyNode1.cs_vSend); @@ -91,7 +96,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) SetMockTime(nStartTime+21*60); { LOCK(dummyNode1.cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); // should result in getheaders + BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders } { LOCK(dummyNode1.cs_vSend); @@ -101,18 +106,17 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) SetMockTime(nStartTime+24*60); { LOCK(dummyNode1.cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); // should result in disconnect + BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in disconnect } BOOST_CHECK(dummyNode1.fDisconnect == true); - peerLogic->FinalizeNode(dummyNode1); + peerman.FinalizeNode(dummyNode1); } static void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerManager& peerLogic, ConnmanTestMsg& connman, ConnectionType connType) { CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE); vNodes.emplace_back(new CNode{id++, - ServiceFlags(NODE_NETWORK | NODE_WITNESS), /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/0, @@ -124,7 +128,7 @@ static void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerM CNode &node = *vNodes.back(); node.SetCommonVersion(PROTOCOL_VERSION); - peerLogic.InitializeNode(&node); + peerLogic.InitializeNode(node, ServiceFlags(NODE_NETWORK | NODE_WITNESS)); node.fSuccessfullyConnected = true; connman.AddTestNode(node); @@ -292,7 +296,6 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) banman->ClearBanned(); NodeId id{0}; nodes[0] = new CNode{id++, - NODE_NETWORK, /*sock=*/nullptr, addr[0], /*nKeyedNetGroupIn=*/0, @@ -302,7 +305,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) ConnectionType::INBOUND, /*inbound_onion=*/false}; nodes[0]->SetCommonVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(nodes[0]); + peerLogic->InitializeNode(*nodes[0], NODE_NETWORK); nodes[0]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[0]); peerLogic->UnitTestMisbehaving(nodes[0]->GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged @@ -315,7 +318,6 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) BOOST_CHECK(!banman->IsDiscouraged(other_addr)); // Different address, not discouraged nodes[1] = new CNode{id++, - NODE_NETWORK, /*sock=*/nullptr, addr[1], /*nKeyedNetGroupIn=*/1, @@ -325,7 +327,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) ConnectionType::INBOUND, /*inbound_onion=*/false}; nodes[1]->SetCommonVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(nodes[1]); + peerLogic->InitializeNode(*nodes[1], NODE_NETWORK); nodes[1]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[1]); peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), DISCOURAGEMENT_THRESHOLD - 1); @@ -353,7 +355,6 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) // Make sure non-IP peers are discouraged and disconnected properly. nodes[2] = new CNode{id++, - NODE_NETWORK, /*sock=*/nullptr, addr[2], /*nKeyedNetGroupIn=*/1, @@ -363,7 +364,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) ConnectionType::OUTBOUND_FULL_RELAY, /*inbound_onion=*/false}; nodes[2]->SetCommonVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(nodes[2]); + peerLogic->InitializeNode(*nodes[2], NODE_NETWORK); nodes[2]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[2]); peerLogic->UnitTestMisbehaving(nodes[2]->GetId(), DISCOURAGEMENT_THRESHOLD); @@ -398,7 +399,6 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) CAddress addr(ip(0xa0b0c001), NODE_NONE); NodeId id{0}; CNode dummyNode{id++, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/4, @@ -408,7 +408,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) ConnectionType::INBOUND, /*inbound_onion=*/false}; dummyNode.SetCommonVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(&dummyNode); + peerLogic->InitializeNode(dummyNode, NODE_NETWORK); dummyNode.fSuccessfullyConnected = true; peerLogic->UnitTestMisbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); diff --git a/src/test/fuzz/mempool_utils.h b/src/test/fuzz/mempool_utils.h new file mode 100644 index 0000000000..bfe12e30ba --- /dev/null +++ b/src/test/fuzz/mempool_utils.h @@ -0,0 +1,19 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H +#define BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H + +#include <validation.h> + +class DummyChainState final : public CChainState +{ +public: + void SetMempool(CTxMemPool* mempool) + { + m_mempool = mempool; + } +}; + +#endif // BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp index 4981287152..741810f6a2 100644 --- a/src/test/fuzz/net.cpp +++ b/src/test/fuzz/net.cpp @@ -68,7 +68,6 @@ FUZZ_TARGET_INIT(net, initialize_net) (void)node.GetAddrLocal(); (void)node.GetId(); (void)node.GetLocalNonce(); - (void)node.GetLocalServices(); const int ref_count = node.GetRefCount(); assert(ref_count >= 0); (void)node.GetCommonVersion(); diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 2d88ee295b..63fbf0516a 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -8,6 +8,7 @@ #include <node/miner.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> +#include <test/fuzz/mempool_utils.h> #include <test/fuzz/util.h> #include <test/util/mining.h> #include <test/util/script.h> @@ -34,15 +35,6 @@ struct MockedTxPool : public CTxMemPool { } }; -class DummyChainState final : public CChainState -{ -public: - void SetMempool(CTxMemPool* mempool) - { - m_mempool = mempool; - } -}; - void initialize_tx_pool() { static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(); diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index d318baa6a2..b18d783259 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -3,8 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <consensus/amount.h> -#include <net.h> +#include <consensus/validation.h> #include <net_processing.h> +#include <node/eviction.h> +#include <policy/policy.h> #include <primitives/transaction.h> #include <script/script.h> #include <sync.h> @@ -99,16 +101,20 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) [&] { bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); // AddTx should return false if tx is too big or already have it + // tx weight is unknown, we only check when tx is already in orphanage { LOCK(g_cs_orphans); - Assert(have_tx != orphanage.AddTx(tx, peer_id)); + bool add_tx = orphanage.AddTx(tx, peer_id); + // have_tx == true -> add_tx == false + Assert(!have_tx || !add_tx); } have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); - // tx should already be added since it will not be too big in the test - // have_tx should be true and AddTx should fail { LOCK(g_cs_orphans); - Assert(have_tx && !orphanage.AddTx(tx, peer_id)); + bool add_tx = orphanage.AddTx(tx, peer_id); + // if have_tx is still false, it must be too big + Assert(!have_tx == GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT); + Assert(!have_tx || !add_tx); } }, [&] { diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index f0cff74f94..fabcea22c3 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -294,6 +294,7 @@ void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, connman.Handshake(node, /*successfully_connected=*/fuzzed_data_provider.ConsumeBool(), /*remote_services=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), + /*local_services=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), /*permission_flags=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS), /*version=*/fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()), /*relay_txs=*/fuzzed_data_provider.ConsumeBool()); diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index d189a50a51..406e11c573 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -296,7 +296,6 @@ template <bool ReturnUniquePtr = false> auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<NodeId>& node_id_in = std::nullopt) noexcept { const NodeId node_id = node_id_in.value_or(fuzzed_data_provider.ConsumeIntegralInRange<NodeId>(0, std::numeric_limits<NodeId>::max())); - const ServiceFlags local_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); const auto sock = std::make_shared<FuzzedSock>(fuzzed_data_provider); const CAddress address = ConsumeAddress(fuzzed_data_provider); const uint64_t keyed_net_group = fuzzed_data_provider.ConsumeIntegral<uint64_t>(); @@ -307,7 +306,6 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N const bool inbound_onion{conn_type == ConnectionType::INBOUND ? fuzzed_data_provider.ConsumeBool() : false}; if constexpr (ReturnUniquePtr) { return std::make_unique<CNode>(node_id, - local_services, sock, address, keyed_net_group, @@ -318,7 +316,6 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N inbound_onion); } else { return CNode{node_id, - local_services, sock, address, keyed_net_group, diff --git a/src/test/fuzz/validation_load_mempool.cpp b/src/test/fuzz/validation_load_mempool.cpp index 9532610f8d..90c1a71d9f 100644 --- a/src/test/fuzz/validation_load_mempool.cpp +++ b/src/test/fuzz/validation_load_mempool.cpp @@ -2,10 +2,14 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <kernel/mempool_persist.h> + #include <chainparamsbase.h> #include <mempool_args.h> +#include <node/mempool_persist_args.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> +#include <test/fuzz/mempool_utils.h> #include <test/fuzz/util.h> #include <test/util/setup_common.h> #include <txmempool.h> @@ -15,6 +19,10 @@ #include <cstdint> #include <vector> +using kernel::DumpMempool; + +using node::MempoolPath; + namespace { const TestingSetup* g_setup; } // namespace @@ -33,9 +41,12 @@ FUZZ_TARGET_INIT(validation_load_mempool, initialize_validation_load_mempool) CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node)}; + auto& chainstate{static_cast<DummyChainState&>(g_setup->m_node.chainman->ActiveChainstate())}; + chainstate.SetMempool(&pool); + auto fuzzed_fopen = [&](const fs::path&, const char*) { return fuzzed_file_provider.open(); }; - (void)LoadMempool(pool, g_setup->m_node.chainman->ActiveChainstate(), fuzzed_fopen); - (void)DumpMempool(pool, fuzzed_fopen, true); + (void)chainstate.LoadMempool(MempoolPath(g_setup->m_args), fuzzed_fopen); + (void)DumpMempool(pool, MempoolPath(g_setup->m_args), fuzzed_fopen, true); } diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp index bc5c49ef63..95e8476b77 100644 --- a/src/test/miniscript_tests.cpp +++ b/src/test/miniscript_tests.cpp @@ -120,6 +120,8 @@ struct KeyConverter { //! Singleton instance of KeyConverter. const KeyConverter CONVERTER{}; +// https://github.com/llvm/llvm-project/issues/53444 +// NOLINTNEXTLINE(misc-unused-using-decls) using miniscript::operator"" _mst; enum TestMode : int { diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 115c4b9b24..f2eaa0179f 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -58,7 +58,6 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) std::string pszDest; std::unique_ptr<CNode> pnode1 = std::make_unique<CNode>(id++, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/0, @@ -77,7 +76,6 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) BOOST_CHECK_EQUAL(pnode1->ConnectedThroughNetwork(), Network::NET_IPV4); std::unique_ptr<CNode> pnode2 = std::make_unique<CNode>(id++, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/1, @@ -96,7 +94,6 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) BOOST_CHECK_EQUAL(pnode2->ConnectedThroughNetwork(), Network::NET_IPV4); std::unique_ptr<CNode> pnode3 = std::make_unique<CNode>(id++, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/0, @@ -115,7 +112,6 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) BOOST_CHECK_EQUAL(pnode3->ConnectedThroughNetwork(), Network::NET_IPV4); std::unique_ptr<CNode> pnode4 = std::make_unique<CNode>(id++, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/1, @@ -629,7 +625,6 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test) ipv4AddrPeer.s_addr = 0xa0b0c001; CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK); std::unique_ptr<CNode> pnode = std::make_unique<CNode>(/*id=*/0, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/0, @@ -648,7 +643,7 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test) pnode->SetAddrLocal(addrLocal); // before patch, this causes undefined behavior detectable with clang's -fsanitize=memory - GetLocalAddrForPeer(&*pnode); + GetLocalAddrForPeer(*pnode); // suppress no-checks-run warning; if this test fails, it's by triggering a sanitizer BOOST_CHECK(1); @@ -675,19 +670,15 @@ BOOST_AUTO_TEST_CASE(get_local_addr_for_peer_port) const uint16_t bind_port = 20001; m_node.args->ForceSetArg("-bind", strprintf("3.4.5.6:%u", bind_port)); - const uint32_t current_time = static_cast<uint32_t>(GetAdjustedTime()); - SetMockTime(current_time); - // Our address:port as seen from the peer, completely different from the above. in_addr peer_us_addr; peer_us_addr.s_addr = htonl(0x02030405); - const CAddress peer_us{CService{peer_us_addr, 20002}, NODE_NETWORK, current_time}; + const CService peer_us{peer_us_addr, 20002}; // Create a peer with a routable IPv4 address (outbound). in_addr peer_out_in_addr; peer_out_in_addr.s_addr = htonl(0x01020304); CNode peer_out{/*id=*/0, - /*nLocalServicesIn=*/NODE_NETWORK, /*sock=*/nullptr, /*addrIn=*/CAddress{CService{peer_out_in_addr, 8333}, NODE_NETWORK}, /*nKeyedNetGroupIn=*/0, @@ -700,16 +691,15 @@ BOOST_AUTO_TEST_CASE(get_local_addr_for_peer_port) peer_out.SetAddrLocal(peer_us); // Without the fix peer_us:8333 is chosen instead of the proper peer_us:bind_port. - auto chosen_local_addr = GetLocalAddrForPeer(&peer_out); + auto chosen_local_addr = GetLocalAddrForPeer(peer_out); BOOST_REQUIRE(chosen_local_addr); - const CAddress expected{CService{peer_us_addr, bind_port}, NODE_NETWORK, current_time}; + const CService expected{peer_us_addr, bind_port}; BOOST_CHECK(*chosen_local_addr == expected); // Create a peer with a routable IPv4 address (inbound). in_addr peer_in_in_addr; peer_in_in_addr.s_addr = htonl(0x05060708); CNode peer_in{/*id=*/0, - /*nLocalServicesIn=*/NODE_NETWORK, /*sock=*/nullptr, /*addrIn=*/CAddress{CService{peer_in_in_addr, 8333}, NODE_NETWORK}, /*nKeyedNetGroupIn=*/0, @@ -722,7 +712,7 @@ BOOST_AUTO_TEST_CASE(get_local_addr_for_peer_port) peer_in.SetAddrLocal(peer_us); // Without the fix peer_us:8333 is chosen instead of the proper peer_us:peer_us.GetPort(). - chosen_local_addr = GetLocalAddrForPeer(&peer_in); + chosen_local_addr = GetLocalAddrForPeer(peer_in); BOOST_REQUIRE(chosen_local_addr); BOOST_CHECK(*chosen_local_addr == peer_us); @@ -837,7 +827,6 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) in_addr peer_in_addr; peer_in_addr.s_addr = htonl(0x01020304); CNode peer{/*id=*/0, - /*nLocalServicesIn=*/NODE_NETWORK, /*sock=*/nullptr, /*addrIn=*/CAddress{CService{peer_in_addr, 8333}, NODE_NETWORK}, /*nKeyedNetGroupIn=*/0, @@ -857,7 +846,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) *static_cast<TestChainState*>(&m_node.chainman->ActiveChainstate()); chainstate.JumpOutOfIbd(); - m_node.peerman->InitializeNode(&peer); + m_node.peerman->InitializeNode(peer, NODE_NETWORK); std::atomic<bool> interrupt_dummy{false}; std::chrono::microseconds time_received_dummy{0}; diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index e70ad036bd..223db16c6c 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -16,6 +16,7 @@ void ConnmanTestMsg::Handshake(CNode& node, bool successfully_connected, ServiceFlags remote_services, + ServiceFlags local_services, NetPermissionFlags permission_flags, int32_t version, bool relay_txs) @@ -24,7 +25,7 @@ void ConnmanTestMsg::Handshake(CNode& node, auto& connman{*this}; const CNetMsgMaker mm{0}; - peerman.InitializeNode(&node); + peerman.InitializeNode(node, local_services); CSerializedNetMsg msg_version{ mm.Make(NetMsgType::VERSION, @@ -51,10 +52,10 @@ void ConnmanTestMsg::Handshake(CNode& node, if (node.fDisconnect) return; assert(node.nVersion == version); assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION)); - assert(node.nServices == remote_services); CNodeStateStats statestats; assert(peerman.GetNodeStateStats(node.GetId(), statestats)); assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn())); + assert(statestats.their_services == remote_services); node.m_permissionFlags = permission_flags; if (successfully_connected) { CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)}; diff --git a/src/test/util/net.h b/src/test/util/net.h index 0cf55f8a22..7f61a03d27 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -42,6 +42,7 @@ struct ConnmanTestMsg : public CConnman { void Handshake(CNode& node, bool successfully_connected, ServiceFlags remote_services, + ServiceFlags local_services, NetPermissionFlags permission_flags, int32_t version, bool relay_txs); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index aeaa10034e..7eff6bdbe3 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1210,14 +1210,14 @@ void CTxMemPool::GetTransactionAncestry(const uint256& txid, size_t& ancestors, } } -bool CTxMemPool::IsLoaded() const +bool CTxMemPool::GetLoadTried() const { LOCK(cs); - return m_is_loaded; + return m_load_tried; } -void CTxMemPool::SetIsLoaded(bool loaded) +void CTxMemPool::SetLoadTried(bool load_tried) { LOCK(cs); - m_is_loaded = loaded; + m_load_tried = load_tried; } diff --git a/src/txmempool.h b/src/txmempool.h index 6e37f59f2e..d7d308038c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -451,7 +451,7 @@ protected: void trackPackageRemoved(const CFeeRate& rate) EXCLUSIVE_LOCKS_REQUIRED(cs); - bool m_is_loaded GUARDED_BY(cs){false}; + bool m_load_tried GUARDED_BY(cs){false}; CFeeRate GetMinFee(size_t sizelimit) const; @@ -728,11 +728,17 @@ public: */ void GetTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) const; - /** @returns true if the mempool is fully loaded */ - bool IsLoaded() const; + /** + * @returns true if we've made an attempt to load the mempool regardless of + * whether the attempt was successful or not + */ + bool GetLoadTried() const; - /** Sets the current loaded state */ - void SetIsLoaded(bool loaded); + /** + * Set whether or not we've made an attempt to load the mempool (regardless + * of whether the attempt was successful or not) + */ + void SetLoadTried(bool load_tried); unsigned long size() const { diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h index 7f9a6aaffd..5cb8268472 100644 --- a/src/univalue/include/univalue.h +++ b/src/univalue/include/univalue.h @@ -82,14 +82,14 @@ public: bool isArray() const { return (typ == VARR); } bool isObject() const { return (typ == VOBJ); } - bool push_back(const UniValue& val); - bool push_backV(const std::vector<UniValue>& vec); + void push_back(const UniValue& val); + void push_backV(const std::vector<UniValue>& vec); template <class It> - bool push_backV(It first, It last); + void push_backV(It first, It last); void __pushKV(const std::string& key, const UniValue& val); - bool pushKV(const std::string& key, const UniValue& val); - bool pushKVs(const UniValue& obj); + void pushKV(const std::string& key, const UniValue& val); + void pushKVs(const UniValue& obj); std::string write(unsigned int prettyIndent = 0, unsigned int indentLevel = 0) const; @@ -140,11 +140,10 @@ public: }; template <class It> -bool UniValue::push_backV(It first, It last) +void UniValue::push_backV(It first, It last) { - if (typ != VARR) return false; + if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"}; values.insert(values.end(), first, last); - return true; } enum jtokentype { diff --git a/src/univalue/lib/univalue.cpp b/src/univalue/lib/univalue.cpp index 3553995c28..ac6f31a5a9 100644 --- a/src/univalue/lib/univalue.cpp +++ b/src/univalue/lib/univalue.cpp @@ -108,53 +108,45 @@ bool UniValue::setObject() return true; } -bool UniValue::push_back(const UniValue& val_) +void UniValue::push_back(const UniValue& val_) { - if (typ != VARR) - return false; + if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"}; values.push_back(val_); - return true; } -bool UniValue::push_backV(const std::vector<UniValue>& vec) +void UniValue::push_backV(const std::vector<UniValue>& vec) { - if (typ != VARR) - return false; + if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"}; values.insert(values.end(), vec.begin(), vec.end()); - - return true; } void UniValue::__pushKV(const std::string& key, const UniValue& val_) { + if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"}; + keys.push_back(key); values.push_back(val_); } -bool UniValue::pushKV(const std::string& key, const UniValue& val_) +void UniValue::pushKV(const std::string& key, const UniValue& val_) { - if (typ != VOBJ) - return false; + if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"}; size_t idx; if (findKey(key, idx)) values[idx] = val_; else __pushKV(key, val_); - return true; } -bool UniValue::pushKVs(const UniValue& obj) +void UniValue::pushKVs(const UniValue& obj) { - if (typ != VOBJ || obj.typ != VOBJ) - return false; + if (typ != VOBJ || obj.typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"}; for (size_t i = 0; i < obj.keys.size(); i++) __pushKV(obj.keys[i], obj.values.at(i)); - - return true; } void UniValue::getObjMap(std::map<std::string,UniValue>& kv) const diff --git a/src/univalue/sources.mk b/src/univalue/sources.mk index e156216378..5e4d9c3831 100644 --- a/src/univalue/sources.mk +++ b/src/univalue/sources.mk @@ -1,12 +1,8 @@ -# - All variables are namespaced with UNIVALUE_ to avoid colliding with -# downstream makefiles. # - All Variables ending in _HEADERS or _SOURCES confuse automake, so the # _INT postfix is applied. # - Convenience variables, for example a UNIVALUE_TEST_DIR should not be used # as they interfere with automatic dependency generation -# - The %reldir% is the relative path from the Makefile.am. This allows -# downstreams to use these variables without having to manually account for -# the path change. +# - The %reldir% is the relative path from the Makefile.am. UNIVALUE_INCLUDE_DIR_INT = %reldir%/include @@ -29,9 +25,6 @@ UNIVALUE_TEST_UNITESTER_INT += %reldir%/test/unitester.cpp UNIVALUE_TEST_JSON_INT = UNIVALUE_TEST_JSON_INT += %reldir%/test/test_json.cpp -UNIVALUE_TEST_NO_NUL_INT = -UNIVALUE_TEST_NO_NUL_INT += %reldir%/test/no_nul.cpp - UNIVALUE_TEST_OBJECT_INT = UNIVALUE_TEST_OBJECT_INT += %reldir%/test/object.cpp diff --git a/src/univalue/test/.gitignore b/src/univalue/test/.gitignore index 7b27cf0da2..5812c96b14 100644 --- a/src/univalue/test/.gitignore +++ b/src/univalue/test/.gitignore @@ -2,7 +2,6 @@ object unitester test_json -no_nul *.trs *.log diff --git a/src/univalue/test/no_nul.cpp b/src/univalue/test/no_nul.cpp deleted file mode 100644 index 3a7a727abb..0000000000 --- a/src/univalue/test/no_nul.cpp +++ /dev/null @@ -1,8 +0,0 @@ -#include <univalue.h> - -int main (int argc, char *argv[]) -{ - char buf[] = "___[1,2,3]___"; - UniValue val; - return val.read(buf + 3, 7) ? 0 : 1; -} diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp index 8a35bf914d..cf8c29ec67 100644 --- a/src/univalue/test/object.cpp +++ b/src/univalue/test/object.cpp @@ -13,9 +13,6 @@ #include <string> #include <vector> -#define BOOST_FIXTURE_TEST_SUITE(a, b) -#define BOOST_AUTO_TEST_CASE(funcName) void funcName() -#define BOOST_AUTO_TEST_SUITE_END() #define BOOST_CHECK(expr) assert(expr) #define BOOST_CHECK_EQUAL(v1, v2) assert((v1) == (v2)) #define BOOST_CHECK_THROW(stmt, excMatch) { \ @@ -35,9 +32,7 @@ } \ } -BOOST_FIXTURE_TEST_SUITE(univalue_tests, BasicTestingSetup) - -BOOST_AUTO_TEST_CASE(univalue_constructor) +void univalue_constructor() { UniValue v1; BOOST_CHECK(v1.isNull()); @@ -85,7 +80,17 @@ BOOST_AUTO_TEST_CASE(univalue_constructor) BOOST_CHECK_EQUAL(v9.getValStr(), "zappa"); } -BOOST_AUTO_TEST_CASE(univalue_typecheck) +void univalue_push_throw() +{ + UniValue j; + BOOST_CHECK_THROW(j.push_back(1), std::runtime_error); + BOOST_CHECK_THROW(j.push_backV({1}), std::runtime_error); + BOOST_CHECK_THROW(j.__pushKV("k", 1), std::runtime_error); + BOOST_CHECK_THROW(j.pushKV("k", 1), std::runtime_error); + BOOST_CHECK_THROW(j.pushKVs({}), std::runtime_error); +} + +void univalue_typecheck() { UniValue v1; BOOST_CHECK(v1.setNumStr("1")); @@ -134,7 +139,7 @@ BOOST_AUTO_TEST_CASE(univalue_typecheck) BOOST_CHECK_THROW(vals[1].get_bool(), std::runtime_error); } -BOOST_AUTO_TEST_CASE(univalue_set) +void univalue_set() { UniValue v(UniValue::VSTR, "foo"); v.clear(); @@ -193,18 +198,18 @@ BOOST_AUTO_TEST_CASE(univalue_set) BOOST_CHECK(v.isNull()); } -BOOST_AUTO_TEST_CASE(univalue_array) +void univalue_array() { UniValue arr(UniValue::VARR); UniValue v((int64_t)1023LL); - BOOST_CHECK(arr.push_back(v)); + arr.push_back(v); std::string vStr("zippy"); - BOOST_CHECK(arr.push_back(vStr)); + arr.push_back(vStr); const char *s = "pippy"; - BOOST_CHECK(arr.push_back(s)); + arr.push_back(s); std::vector<UniValue> vec; v.setStr("boing"); @@ -213,13 +218,13 @@ BOOST_AUTO_TEST_CASE(univalue_array) v.setStr("going"); vec.push_back(v); - BOOST_CHECK(arr.push_backV(vec)); + arr.push_backV(vec); - BOOST_CHECK(arr.push_back((uint64_t) 400ULL)); - BOOST_CHECK(arr.push_back((int64_t) -400LL)); - BOOST_CHECK(arr.push_back((int) -401)); - BOOST_CHECK(arr.push_back(-40.1)); - BOOST_CHECK(arr.push_back(true)); + arr.push_back(uint64_t{400ULL}); + arr.push_back(int64_t{-400LL}); + arr.push_back(int{-401}); + arr.push_back(-40.1); + arr.push_back(true); BOOST_CHECK_EQUAL(arr.empty(), false); BOOST_CHECK_EQUAL(arr.size(), 10); @@ -252,7 +257,7 @@ BOOST_AUTO_TEST_CASE(univalue_array) BOOST_CHECK_EQUAL(arr.size(), 0); } -BOOST_AUTO_TEST_CASE(univalue_object) +void univalue_object() { UniValue obj(UniValue::VOBJ); std::string strKey, strVal; @@ -260,39 +265,39 @@ BOOST_AUTO_TEST_CASE(univalue_object) strKey = "age"; v.setInt(100); - BOOST_CHECK(obj.pushKV(strKey, v)); + obj.pushKV(strKey, v); strKey = "first"; strVal = "John"; - BOOST_CHECK(obj.pushKV(strKey, strVal)); + obj.pushKV(strKey, strVal); strKey = "last"; - const char *cVal = "Smith"; - BOOST_CHECK(obj.pushKV(strKey, cVal)); + const char* cVal = "Smith"; + obj.pushKV(strKey, cVal); strKey = "distance"; - BOOST_CHECK(obj.pushKV(strKey, (int64_t) 25)); + obj.pushKV(strKey, int64_t{25}); strKey = "time"; - BOOST_CHECK(obj.pushKV(strKey, (uint64_t) 3600)); + obj.pushKV(strKey, uint64_t{3600}); strKey = "calories"; - BOOST_CHECK(obj.pushKV(strKey, (int) 12)); + obj.pushKV(strKey, int{12}); strKey = "temperature"; - BOOST_CHECK(obj.pushKV(strKey, (double) 90.012)); + obj.pushKV(strKey, double{90.012}); strKey = "moon"; - BOOST_CHECK(obj.pushKV(strKey, true)); + obj.pushKV(strKey, true); strKey = "spoon"; - BOOST_CHECK(obj.pushKV(strKey, false)); + obj.pushKV(strKey, false); UniValue obj2(UniValue::VOBJ); - BOOST_CHECK(obj2.pushKV("cat1", 9000)); - BOOST_CHECK(obj2.pushKV("cat2", 12345)); + obj2.pushKV("cat1", 9000); + obj2.pushKV("cat2", 12345); - BOOST_CHECK(obj.pushKVs(obj2)); + obj.pushKVs(obj2); BOOST_CHECK_EQUAL(obj.empty(), false); BOOST_CHECK_EQUAL(obj.size(), 11); @@ -371,7 +376,7 @@ BOOST_AUTO_TEST_CASE(univalue_object) static const char *json1 = "[1.10000000,{\"key1\":\"str\\u0000\",\"key2\":800,\"key3\":{\"name\":\"martian http://test.com\"}}]"; -BOOST_AUTO_TEST_CASE(univalue_readwrite) +void univalue_readwrite() { UniValue v; BOOST_CHECK(v.read(json1)); @@ -414,11 +419,10 @@ BOOST_AUTO_TEST_CASE(univalue_readwrite) BOOST_CHECK(!v.read("{} 42")); } -BOOST_AUTO_TEST_SUITE_END() - int main (int argc, char *argv[]) { univalue_constructor(); + univalue_push_throw(); univalue_typecheck(); univalue_set(); univalue_array(); diff --git a/src/univalue/test/unitester.cpp b/src/univalue/test/unitester.cpp index 94c149b39f..6344a5a0ab 100644 --- a/src/univalue/test/unitester.cpp +++ b/src/univalue/test/unitester.cpp @@ -149,6 +149,13 @@ void unescape_unicode_test() assert(val[0].get_str() == "\xf0\x9d\x85\xa1"); } +void no_nul_test() +{ + char buf[] = "___[1,2,3]___"; + UniValue val; + assert(val.read(buf + 3, 7)); +} + int main (int argc, char *argv[]) { for (const auto& f: filenames) { @@ -156,6 +163,7 @@ int main (int argc, char *argv[]) } unescape_unicode_test(); + no_nul_test(); return 0; } diff --git a/src/util/time.h b/src/util/time.h index 9df69a953c..fc49f23ce3 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -24,6 +24,7 @@ struct NodeClock : public std::chrono::system_clock { }; using NodeSeconds = std::chrono::time_point<NodeClock, std::chrono::seconds>; +using SteadyClock = std::chrono::steady_clock; using SteadySeconds = std::chrono::time_point<std::chrono::steady_clock, std::chrono::seconds>; using SteadyMilliseconds = std::chrono::time_point<std::chrono::steady_clock, std::chrono::milliseconds>; using SteadyMicroseconds = std::chrono::time_point<std::chrono::steady_clock, std::chrono::microseconds>; diff --git a/src/validation.cpp b/src/validation.cpp index 4c694a2c21..c46eb36e5c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5,6 +5,9 @@ #include <validation.h> +#include <kernel/coinstats.h> +#include <kernel/mempool_persist.h> + #include <arith_uint256.h> #include <chain.h> #include <chainparams.h> @@ -17,8 +20,8 @@ #include <consensus/validation.h> #include <cuckoocache.h> #include <flatfile.h> +#include <fs.h> #include <hash.h> -#include <kernel/coinstats.h> #include <logging.h> #include <logging/timer.h> #include <node/blockstorage.h> @@ -47,12 +50,14 @@ #include <util/rbf.h> #include <util/strencodings.h> #include <util/system.h> +#include <util/time.h> #include <util/trace.h> #include <util/translation.h> #include <validationinterface.h> #include <warnings.h> #include <algorithm> +#include <chrono> #include <deque> #include <numeric> #include <optional> @@ -61,8 +66,9 @@ using kernel::CCoinsStats; using kernel::CoinStatsHashType; using kernel::ComputeUTXOStats; +using kernel::LoadMempool; -using node::BLOCKFILE_CHUNK_SIZE; +using fsbridge::FopenFn; using node::BlockManager; using node::BlockMap; using node::CBlockIndexHeightOnlyComparator; @@ -70,11 +76,8 @@ using node::CBlockIndexWorkComparator; using node::fImporting; using node::fPruneMode; using node::fReindex; -using node::nPruneTarget; -using node::OpenBlockFile; using node::ReadBlockFromDisk; using node::SnapshotMetadata; -using node::UNDOFILE_CHUNK_SIZE; using node::UndoReadFromDisk; using node::UnlinkPrunedFiles; @@ -3861,13 +3864,11 @@ void PruneBlockFilesManual(CChainState& active_chainstate, int nManualPruneHeigh } } -void CChainState::LoadMempool(const ArgsManager& args) +void CChainState::LoadMempool(const fs::path& load_path, FopenFn mockable_fopen_function) { if (!m_mempool) return; - if (args.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - ::LoadMempool(*m_mempool, *this); - } - m_mempool->SetIsLoaded(!ShutdownRequested()); + ::LoadMempool(*m_mempool, load_path, *this, mockable_fopen_function); + m_mempool->SetLoadTried(!ShutdownRequested()); } bool CChainState::LoadChainTip() @@ -4638,153 +4639,6 @@ bool CChainState::ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) return ret; } -static const uint64_t MEMPOOL_DUMP_VERSION = 1; - -bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function) -{ - int64_t nExpiryTimeout = std::chrono::seconds{pool.m_expiry}.count(); - FILE* filestr{mockable_fopen_function(gArgs.GetDataDirNet() / "mempool.dat", "rb")}; - CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); - if (file.IsNull()) { - LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); - return false; - } - - int64_t count = 0; - int64_t expired = 0; - int64_t failed = 0; - int64_t already_there = 0; - int64_t unbroadcast = 0; - int64_t nNow = GetTime(); - - try { - uint64_t version; - file >> version; - if (version != MEMPOOL_DUMP_VERSION) { - return false; - } - uint64_t num; - file >> num; - while (num) { - --num; - CTransactionRef tx; - int64_t nTime; - int64_t nFeeDelta; - file >> tx; - file >> nTime; - file >> nFeeDelta; - - CAmount amountdelta = nFeeDelta; - if (amountdelta) { - pool.PrioritiseTransaction(tx->GetHash(), amountdelta); - } - if (nTime > nNow - nExpiryTimeout) { - LOCK(cs_main); - const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false); - if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) { - ++count; - } else { - // mempool may contain the transaction already, e.g. from - // wallet(s) having loaded it while we were processing - // mempool transactions; consider these as valid, instead of - // failed, but mark them as 'already there' - if (pool.exists(GenTxid::Txid(tx->GetHash()))) { - ++already_there; - } else { - ++failed; - } - } - } else { - ++expired; - } - if (ShutdownRequested()) - return false; - } - std::map<uint256, CAmount> mapDeltas; - file >> mapDeltas; - - for (const auto& i : mapDeltas) { - pool.PrioritiseTransaction(i.first, i.second); - } - - std::set<uint256> unbroadcast_txids; - file >> unbroadcast_txids; - unbroadcast = unbroadcast_txids.size(); - for (const auto& txid : unbroadcast_txids) { - // Ensure transactions were accepted to mempool then add to - // unbroadcast set. - if (pool.get(txid) != nullptr) pool.AddUnbroadcastTx(txid); - } - } catch (const std::exception& e) { - LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what()); - return false; - } - - LogPrintf("Imported mempool transactions from disk: %i succeeded, %i failed, %i expired, %i already there, %i waiting for initial broadcast\n", count, failed, expired, already_there, unbroadcast); - return true; -} - -bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function, bool skip_file_commit) -{ - int64_t start = GetTimeMicros(); - - std::map<uint256, CAmount> mapDeltas; - std::vector<TxMempoolInfo> vinfo; - std::set<uint256> unbroadcast_txids; - - static Mutex dump_mutex; - LOCK(dump_mutex); - - { - LOCK(pool.cs); - for (const auto &i : pool.mapDeltas) { - mapDeltas[i.first] = i.second; - } - vinfo = pool.infoAll(); - unbroadcast_txids = pool.GetUnbroadcastTxs(); - } - - int64_t mid = GetTimeMicros(); - - try { - FILE* filestr{mockable_fopen_function(gArgs.GetDataDirNet() / "mempool.dat.new", "wb")}; - if (!filestr) { - return false; - } - - CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); - - uint64_t version = MEMPOOL_DUMP_VERSION; - file << version; - - file << (uint64_t)vinfo.size(); - for (const auto& i : vinfo) { - file << *(i.tx); - file << int64_t{count_seconds(i.m_time)}; - file << int64_t{i.nFeeDelta}; - mapDeltas.erase(i.tx->GetHash()); - } - - file << mapDeltas; - - LogPrintf("Writing %d unbroadcast transactions to disk.\n", unbroadcast_txids.size()); - file << unbroadcast_txids; - - if (!skip_file_commit && !FileCommit(file.Get())) - throw std::runtime_error("FileCommit failed"); - file.fclose(); - if (!RenameOver(gArgs.GetDataDirNet() / "mempool.dat.new", gArgs.GetDataDirNet() / "mempool.dat")) { - throw std::runtime_error("Rename failed"); - } - int64_t last = GetTimeMicros(); - LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", (mid-start)*MICRO, (last-mid)*MICRO); - } catch (const std::exception& e) { - LogPrintf("Failed to dump mempool: %s. Continuing anyway.\n", e.what()); - return false; - } - return true; -} - //! Guess how far we are in the verification process at the given block index //! require cs_main if pindex has not been validated yet (because nChainTx might be unset) double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pindex) { diff --git a/src/validation.h b/src/validation.h index aa9fcf9f2d..a21d9e3a28 100644 --- a/src/validation.h +++ b/src/validation.h @@ -68,8 +68,6 @@ static const bool DEFAULT_CHECKPOINTS_ENABLED = true; static const bool DEFAULT_TXINDEX = false; static constexpr bool DEFAULT_COINSTATSINDEX{false}; static const char* const DEFAULT_BLOCKFILTERINDEX = "0"; -/** Default for -persistmempool */ -static const bool DEFAULT_PERSIST_MEMPOOL = true; /** Default for -stopatheight */ static const int DEFAULT_STOPATHEIGHT = 0; /** Block files containing a block-height within MIN_BLOCKS_TO_KEEP of ActiveChain().Tip() will not be pruned. */ @@ -679,7 +677,7 @@ public: void CheckBlockIndex(); /** Load the persisted mempool from disk */ - void LoadMempool(const ArgsManager& args); + void LoadMempool(const fs::path& load_path, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); /** Update the chain tip based on database information, i.e. CoinsTip()'s best block. */ bool LoadChainTip() EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -1014,14 +1012,6 @@ bool DeploymentEnabled(const ChainstateManager& chainman, DEP dep) return DeploymentEnabled(chainman.GetConsensus(), dep); } -using FopenFn = std::function<FILE*(const fs::path&, const char*)>; - -/** Dump the mempool to disk. */ -bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function = fsbridge::fopen, bool skip_file_commit = false); - -/** Load the mempool from disk. */ -bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function = fsbridge::fopen); - /** * Return the expected assumeutxo value for a given height, if one exists. * diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 8de4017371..d3303c0b1f 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -130,12 +130,10 @@ CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const ism return 0; CAmount credit = 0; - if (filter & ISMINE_SPENDABLE) { + const isminefilter get_amount_filter{filter & ISMINE_ALL}; + if (get_amount_filter) { // GetBalance can assume transactions in mapWallet won't change - credit += GetCachableAmount(wallet, wtx, CWalletTx::CREDIT, ISMINE_SPENDABLE); - } - if (filter & ISMINE_WATCH_ONLY) { - credit += GetCachableAmount(wallet, wtx, CWalletTx::CREDIT, ISMINE_WATCH_ONLY); + credit += GetCachableAmount(wallet, wtx, CWalletTx::CREDIT, get_amount_filter); } return credit; } @@ -146,11 +144,9 @@ CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const ismi return 0; CAmount debit = 0; - if (filter & ISMINE_SPENDABLE) { - debit += GetCachableAmount(wallet, wtx, CWalletTx::DEBIT, ISMINE_SPENDABLE); - } - if (filter & ISMINE_WATCH_ONLY) { - debit += GetCachableAmount(wallet, wtx, CWalletTx::DEBIT, ISMINE_WATCH_ONLY); + const isminefilter get_amount_filter{filter & ISMINE_ALL}; + if (get_amount_filter) { + debit += GetCachableAmount(wallet, wtx, CWalletTx::DEBIT, get_amount_filter); } return debit; } diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 5f1673fb12..62aba4f3f2 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -100,11 +100,13 @@ RPCHelpMan importprivkey() "Hint: use importmulti to import more than one private key.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported key exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"privkey", RPCArg::Type::STR, RPCArg::Optional::NO, "The private key (see dumpprivkey)"}, {"label", RPCArg::Type::STR, RPCArg::DefaultHint{"current label if address exists, otherwise \"\""}, "An optional label"}, - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Rescan the wallet for transactions"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions."}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{ @@ -201,6 +203,8 @@ RPCHelpMan importaddress() "\nAdds an address or script (in hex) that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported address exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "If you have the full public key, you should call importpubkey instead of this.\n" "Hint: use importmulti to import more than one address.\n" "\nNote: If you import a non-standard raw script in hex form, outputs sending to it will be treated\n" @@ -210,7 +214,7 @@ RPCHelpMan importaddress() { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The Bitcoin address (or hex-encoded script)"}, {"label", RPCArg::Type::STR, RPCArg::Default{""}, "An optional label"}, - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Rescan the wallet for transactions"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions."}, {"p2sh", RPCArg::Type::BOOL, RPCArg::Default{false}, "Add the P2SH version of the script as well"}, }, RPCResult{RPCResult::Type::NONE, "", ""}, @@ -401,11 +405,13 @@ RPCHelpMan importpubkey() "Hint: use importmulti to import more than one public key.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported pubkey exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"pubkey", RPCArg::Type::STR, RPCArg::Optional::NO, "The hex-encoded public key"}, {"label", RPCArg::Type::STR, RPCArg::Default{""}, "An optional label"}, - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Rescan the wallet for transactions"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions."}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{ @@ -484,7 +490,7 @@ RPCHelpMan importwallet() { return RPCHelpMan{"importwallet", "\nImports keys from a wallet dump file (see dumpwallet). Requires a new wallet backup to include imported keys.\n" - "Note: Use \"getwalletinfo\" to query the scanning progress.\n", + "Note: Blockchain and Mempool will be rescanned after a successful import. Use \"getwalletinfo\" to query the scanning progress.\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet file"}, }, @@ -1250,6 +1256,8 @@ RPCHelpMan importmulti() "Conversely, if all the private keys are provided and the address/script is spendable, the watchonly option must be set to false, or a warning will be returned.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported keys, addresses or scripts exist but related transactions are still missing.\n" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"requests", RPCArg::Type::ARR, RPCArg::Optional::NO, "Data to be imported", @@ -1291,7 +1299,7 @@ RPCHelpMan importmulti() "\"requests\""}, {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", { - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Stating if should rescan the blockchain after all imports"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions after all imports."}, }, "\"options\""}, }, @@ -1593,7 +1601,7 @@ RPCHelpMan importdescriptors() " Use the string \"now\" to substitute the current synced blockchain time.\n" " \"now\" can be specified to bypass scanning, for outputs which are known to never have been used, and\n" " 0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest timestamp\n" - " of all descriptors being imported will be scanned.", + "of all descriptors being imported will be scanned as well as the mempool.", /*oneline_description=*/"", {"timestamp | \"now\"", "integer / string"} }, {"internal", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether matching outputs should be treated as not incoming payments (e.g. change)"}, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 22c1f2bdc2..83e23497cb 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1633,7 +1633,7 @@ RPCHelpMan walletcreatefundedpsbt() }, true ); - UniValue options = request.params[3]; + UniValue options{request.params[3].isNull() ? UniValue::VOBJ : request.params[3]}; CAmount fee; int change_position; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 81c01e5023..7b0906c0a8 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -53,9 +53,6 @@ static const std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context) auto database = MakeWalletDatabase("", options, status, error); auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags, error, warnings); NotifyWalletLoaded(context, wallet); - if (context.chain) { - wallet->postInitProcess(); - } return wallet; } @@ -768,6 +765,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // being blocked wallet = TestLoadWallet(context); BOOST_CHECK(rescan_completed); + // AddToWallet events for block_tx and mempool_tx BOOST_CHECK_EQUAL(addtx_count, 2); { LOCK(wallet->cs_wallet); @@ -780,6 +778,8 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // transactionAddedToMempool events are processed promise.set_value(); SyncWithValidationInterfaceQueue(); + // AddToWallet events for block_tx and mempool_tx events are counted a + // second time as the notificaiton queue is processed BOOST_CHECK_EQUAL(addtx_count, 4); @@ -803,7 +803,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) SyncWithValidationInterfaceQueue(); }); wallet = TestLoadWallet(context); - BOOST_CHECK_EQUAL(addtx_count, 4); + BOOST_CHECK_EQUAL(addtx_count, 2); { LOCK(wallet->cs_wallet); BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_tx.GetHash()), 1U); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 475627c76c..9af68000aa 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1678,7 +1678,8 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r /** * Scan the block chain (starting in start_block) for transactions * from or to us. If fUpdate is true, found transactions that already - * exist in the wallet will be updated. + * exist in the wallet will be updated. If max_height is not set, the + * mempool will be scanned as well. * * @param[in] start_block Scan starting block. If block is not on the active * chain, the scan will return SUCCESS immediately. @@ -1799,6 +1800,10 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc } } } + if (!max_height) { + WalletLogPrintf("Scanning current mempool transactions.\n"); + WITH_LOCK(cs_wallet, chain().requestMempoolTransactions(*this)); + } ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), 100); // hide progress dialog in GUI if (block_height && fAbortRescan) { WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", block_height, progress_current); @@ -1979,7 +1984,6 @@ TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& comp if (n_signed) { *n_signed = 0; } - const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx); LOCK(cs_wallet); // Get all of the previous transactions for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { @@ -2003,6 +2007,8 @@ TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& comp } } + const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx); + // Fill in information from ScriptPubKeyMans for (ScriptPubKeyMan* spk_man : GetAllScriptPubKeyMans()) { int n_signed_this_spkm = 0; @@ -3044,20 +3050,31 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf if (tip_height && *tip_height != rescan_height) { - if (chain.havePruned()) { + // Technically we could execute the code below in any case, but performing the + // `while` loop below can make startup very slow, so only check blocks on disk + // if necessary. + if (chain.havePruned() || chain.hasAssumedValidChain()) { int block_height = *tip_height; while (block_height > 0 && chain.haveBlockOnDisk(block_height - 1) && rescan_height != block_height) { --block_height; } if (rescan_height != block_height) { - // We can't rescan beyond non-pruned blocks, stop and throw an error. + // We can't rescan beyond blocks we don't have data for, stop and throw an error. // This might happen if a user uses an old wallet within a pruned node // or if they ran -disablewallet for a longer time, then decided to re-enable // Exit early and print an error. + // It also may happen if an assumed-valid chain is in use and therefore not + // all block data is available. // If a block is pruned after this check, we will load the wallet, // but fail the rescan with a generic error. - error = _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)"); + + error = chain.hasAssumedValidChain() ? + _( + "Assumed-valid: last wallet synchronisation goes beyond " + "available block data. You need to wait for the background " + "validation chain to download more blocks.") : + _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)"); return false; } } diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index 58f4c91343..b6fa7fbd91 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -141,6 +141,16 @@ class MempoolPersistTest(BitcoinTestFramework): self.nodes[2].syncwithvalidationinterfacequeue() # Flush mempool to wallet assert_equal(node2_balance, wallet_watch.getbalance()) + mempooldat0 = os.path.join(self.nodes[0].datadir, self.chain, 'mempool.dat') + mempooldat1 = os.path.join(self.nodes[1].datadir, self.chain, 'mempool.dat') + + self.log.debug("Force -persistmempool=0 node1 to savemempool to disk via RPC") + assert not os.path.exists(mempooldat1) + result1 = self.nodes[1].savemempool() + assert os.path.isfile(mempooldat1) + assert_equal(result1['filename'], mempooldat1) + os.remove(mempooldat1) + self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.") self.stop_nodes() self.start_node(0, extra_args=["-persistmempool=0"]) @@ -153,8 +163,6 @@ class MempoolPersistTest(BitcoinTestFramework): assert self.nodes[0].getmempoolinfo()["loaded"] assert_equal(len(self.nodes[0].getrawmempool()), 7) - mempooldat0 = os.path.join(self.nodes[0].datadir, self.chain, 'mempool.dat') - mempooldat1 = os.path.join(self.nodes[1].datadir, self.chain, 'mempool.dat') self.log.debug("Remove the mempool.dat file. Verify that savemempool to disk via RPC re-creates it") os.remove(mempooldat0) result0 = self.nodes[0].savemempool() diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 264b2ac32d..93b8d81959 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -22,6 +22,7 @@ from test_framework.util import ( assert_greater_than, assert_raises_rpc_error, find_output, + find_vout_for_address, ) from test_framework.wallet_util import bytes_to_wif @@ -764,5 +765,15 @@ class PSBTTest(BitcoinTestFramework): psbt = self.nodes[0].walletprocesspsbt(psbt)["psbt"] self.nodes[0].sendrawtransaction(self.nodes[0].finalizepsbt(psbt)["hex"]) + self.log.info("Test that walletprocesspsbt both updates and signs a non-updated psbt containing Taproot inputs") + addr = self.nodes[0].getnewaddress("", "bech32m") + txid = self.nodes[0].sendtoaddress(addr, 1) + vout = find_vout_for_address(self.nodes[0], txid, addr) + psbt = self.nodes[0].createpsbt([{"txid": txid, "vout": vout}], [{self.nodes[0].getnewaddress(): 0.9999}]) + signed = self.nodes[0].walletprocesspsbt(psbt) + rawtx = self.nodes[0].finalizepsbt(signed["psbt"])["hex"] + self.nodes[0].sendrawtransaction(rawtx) + self.generate(self.nodes[0], 1) + if __name__ == '__main__': PSBTTest().main() diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 0c93821e7f..d49bca6855 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -273,6 +273,26 @@ class WalletTest(BitcoinTestFramework): self.generatetoaddress(self.nodes[1], 1, ADDRESS_WATCHONLY) assert_equal(self.nodes[0].getbalance(minconf=0), total_amount + 1) # The reorg recovered our fee of 1 coin + if not self.options.descriptors: + self.log.info('Check if mempool is taken into account after import*') + address = self.nodes[0].getnewaddress() + privkey = self.nodes[0].dumpprivkey(address) + self.nodes[0].sendtoaddress(address, 0.1) + self.nodes[0].unloadwallet('') + # check importaddress on fresh wallet + self.nodes[0].createwallet('w1', False, True) + self.nodes[0].importaddress(address) + assert_equal(self.nodes[0].getbalances()['mine']['untrusted_pending'], 0) + assert_equal(self.nodes[0].getbalances()['watchonly']['untrusted_pending'], Decimal('0.1')) + self.nodes[0].importprivkey(privkey) + assert_equal(self.nodes[0].getbalances()['mine']['untrusted_pending'], Decimal('0.1')) + assert_equal(self.nodes[0].getbalances()['watchonly']['untrusted_pending'], 0) + self.nodes[0].unloadwallet('w1') + # check importprivkey on fresh wallet + self.nodes[0].createwallet('w2', False, True) + self.nodes[0].importprivkey(privkey) + assert_equal(self.nodes[0].getbalances()['mine']['untrusted_pending'], Decimal('0.1')) + if __name__ == '__main__': WalletTest().main() diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index d9acc8cea5..085ad51c79 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -87,6 +87,7 @@ class Variant(collections.namedtuple("Variant", "call data address_type rescan p assert_equal(len(txs), self.expected_txs) addresses = self.node.listreceivedbyaddress(minconf=0, include_watchonly=True, address_filter=self.address['address']) + if self.expected_txs: assert_equal(len(addresses[0]["txids"]), self.expected_txs) @@ -98,13 +99,18 @@ class Variant(collections.namedtuple("Variant", "call data address_type rescan p assert_equal(tx["category"], "receive") assert_equal(tx["label"], self.label) assert_equal(tx["txid"], txid) - assert_equal(tx["confirmations"], 1 + current_height - confirmation_height) - assert "trusted" not in tx + + # If no confirmation height is given, the tx is still in the + # mempool. + confirmations = (1 + current_height - confirmation_height) if confirmation_height else 0 + assert_equal(tx["confirmations"], confirmations) + if confirmations: + assert "trusted" not in tx address, = [ad for ad in addresses if txid in ad["txids"]] assert_equal(address["address"], self.address["address"]) assert_equal(address["amount"], self.expected_balance) - assert_equal(address["confirmations"], 1 + current_height - confirmation_height) + assert_equal(address["confirmations"], confirmations) # Verify the transaction is correctly marked watchonly depending on # whether the transaction pays to an imported public key or # imported private key. The test setup ensures that transaction @@ -162,11 +168,12 @@ class ImportRescanTest(BitcoinTestFramework): self.import_deterministic_coinbase_privkeys() self.stop_nodes() - self.start_nodes() + self.start_nodes(extra_args=[["-whitelist=noban@127.0.0.1"]] * self.num_nodes) for i in range(1, self.num_nodes): self.connect_nodes(i, 0) def run_test(self): + # Create one transaction on node 0 with a unique amount for # each possible type of wallet import RPC. for i, variant in enumerate(IMPORT_VARIANTS): @@ -207,7 +214,7 @@ class ImportRescanTest(BitcoinTestFramework): variant.check() # Create new transactions sending to each address. - for i, variant in enumerate(IMPORT_VARIANTS): + for variant in IMPORT_VARIANTS: variant.sent_amount = get_rand_amount() variant.sent_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.sent_amount) self.generate(self.nodes[0], 1) # Generate one block for each send @@ -223,6 +230,46 @@ class ImportRescanTest(BitcoinTestFramework): variant.expected_txs += 1 variant.check(variant.sent_txid, variant.sent_amount, variant.confirmation_height) + self.log.info('Test that the mempool is rescanned as well if the rescan parameter is set to true') + + # The late timestamp and pruned variants are not necessary when testing mempool rescan + mempool_variants = [variant for variant in IMPORT_VARIANTS if variant.rescan != Rescan.late_timestamp and not variant.prune] + # No further blocks are mined so the timestamp will stay the same + timestamp = self.nodes[0].getblockheader(self.nodes[0].getbestblockhash())["time"] + + # Create one transaction on node 0 with a unique amount for + # each possible type of wallet import RPC. + for i, variant in enumerate(mempool_variants): + variant.label = "mempool label {} {}".format(i, variant) + variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress( + label=variant.label, + address_type=variant.address_type.value, + )) + variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) + variant.initial_amount = get_rand_amount() + variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) + variant.confirmation_height = 0 + variant.timestamp = timestamp + + assert_equal(len(self.nodes[0].getrawmempool()), len(mempool_variants)) + self.sync_mempools() + + # For each variation of wallet key import, invoke the import RPC and + # check the results from getbalance and listtransactions. + for variant in mempool_variants: + self.log.info('Run import for mempool variant {}'.format(variant)) + expect_rescan = variant.rescan == Rescan.yes + variant.node = self.nodes[2 + IMPORT_NODES.index(ImportNode(variant.prune, expect_rescan))] + variant.do_import(variant.timestamp) + if expect_rescan: + variant.expected_balance = variant.initial_amount + variant.expected_txs = 1 + variant.check(variant.initial_txid, variant.initial_amount) + else: + variant.expected_balance = 0 + variant.expected_txs = 0 + variant.check() + if __name__ == "__main__": ImportRescanTest().main() diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index ff11f421a1..525b91a6e0 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -480,7 +480,9 @@ class ImportDescriptorsTest(BitcoinTestFramework): addr = wmulti_pub.getnewaddress('', 'bech32') assert_equal(addr, 'bcrt1qp8s25ckjl7gr6x2q3dx3tn2pytwp05upkjztk6ey857tt50r5aeqn6mvr9') # Derived at m/84'/0'/0'/1 change_addr = wmulti_pub.getrawchangeaddress('bech32') - assert_equal(change_addr, 'bcrt1qt9uhe3a9hnq7vajl7a094z4s3crm9ttf8zw3f5v9gr2nyd7e3lnsy44n8e') + assert_equal(change_addr, 'bcrt1qzxl0qz2t88kljdnkzg4n4gapr6kte26390gttrg79x66nt4p04fssj53nl') + assert(send_txid in self.nodes[0].getrawmempool(True)) + assert(send_txid in (x['txid'] for x in wmulti_pub.listunspent(0))) assert_equal(wmulti_pub.getwalletinfo()['keypoolsize'], 999) # generate some utxos for next tests diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index 5d157eb4b1..f44ed8f7c7 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -22,6 +22,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES = ( "wallet/fees -> wallet/wallet -> wallet/fees", "wallet/wallet -> wallet/walletdb -> wallet/wallet", "kernel/coinstats -> validation -> kernel/coinstats", + "kernel/mempool_persist -> validation -> kernel/mempool_persist", ) CODE_DIR = "src" |