diff options
86 files changed, 1307 insertions, 681 deletions
diff --git a/.gitignore b/.gitignore index 3e5d284aa3..1173edfaa7 100644 --- a/.gitignore +++ b/.gitignore @@ -119,7 +119,9 @@ releases /*.info test_bitcoin.coverage/ total.coverage/ +fuzz.coverage/ coverage_percent.txt +/cov_tool_wrapper.sh #build tests linux-coverage-build diff --git a/Makefile.am b/Makefile.am index 75a164f49e..1d6358b1d5 100644 --- a/Makefile.am +++ b/Makefile.am @@ -65,10 +65,10 @@ OSX_PACKAGING = $(OSX_DEPLOY_SCRIPT) $(OSX_FANCY_PLIST) $(OSX_INSTALLER_ICONS) \ $(top_srcdir)/contrib/macdeploy/detached-sig-apply.sh \ $(top_srcdir)/contrib/macdeploy/detached-sig-create.sh -COVERAGE_INFO = baseline.info \ +COVERAGE_INFO = $(COV_TOOL_WRAPPER) baseline.info \ test_bitcoin_filtered.info total_coverage.info \ baseline_filtered.info functional_test.info functional_test_filtered.info \ - test_bitcoin_coverage.info test_bitcoin.info fuzz.info fuzz_coverage.info + test_bitcoin_coverage.info test_bitcoin.info fuzz.info fuzz_filtered.info fuzz_coverage.info dist-hook: -$(GIT) archive --format=tar HEAD -- src/clientversion.cpp | $(AMTAR) -C $(top_distdir) -xf - @@ -192,7 +192,11 @@ LCOV_FILTER_PATTERN = \ -p "src/secp256k1" \ -p "depends" -baseline.info: +$(COV_TOOL_WRAPPER): + @echo 'exec $(COV_TOOL) "$$@"' > $(COV_TOOL_WRAPPER) + @chmod +x $(COV_TOOL_WRAPPER) + +baseline.info: $(COV_TOOL_WRAPPER) $(LCOV) -c -i -d $(abs_builddir)/src -o $@ baseline_filtered.info: baseline.info diff --git a/ci/test/00_setup_env.sh b/ci/test/00_setup_env.sh index 2413cfca9f..702e881862 100755 --- a/ci/test/00_setup_env.sh +++ b/ci/test/00_setup_env.sh @@ -35,6 +35,10 @@ export USE_BUSY_BOX=${USE_BUSY_BOX:-false} export RUN_UNIT_TESTS=${RUN_UNIT_TESTS:-true} export RUN_FUNCTIONAL_TESTS=${RUN_FUNCTIONAL_TESTS:-true} export RUN_SECURITY_TESTS=${RUN_SECURITY_TESTS:-false} +# By how much to scale the test_runner timeouts (option --timeout-factor). +# This is needed because some ci machines have slow CPU or disk, so sanitizers +# might be slow or a reindex might be waiting on disk IO. +export TEST_RUNNER_TIMEOUT_FACTOR=${TEST_RUNNER_TIMEOUT_FACTOR:-4} export TEST_RUNNER_ENV=${TEST_RUNNER_ENV:-} export RUN_FUZZ_TESTS=${RUN_FUZZ_TESTS:-false} export EXPECTED_TESTS_DURATION_IN_SECONDS=${EXPECTED_TESTS_DURATION_IN_SECONDS:-1000} diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index 5995964f17..251ece7984 100644 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -10,6 +10,5 @@ export CONTAINER_NAME=ci_native_asan export PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" export DOCKER_NAME_TAG=ubuntu:20.04 export NO_DEPENDS=1 -export TEST_RUNNER_EXTRA="--timeout-factor=4" # Increase timeout because sanitizers slow down export GOAL="install" export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' --with-sanitizers=address,integer,undefined CC=clang CXX=clang++ --with-boost-process" diff --git a/ci/test/00_setup_env_native_tsan.sh b/ci/test/00_setup_env_native_tsan.sh index fc18483425..b14a46562c 100644 --- a/ci/test/00_setup_env_native_tsan.sh +++ b/ci/test/00_setup_env_native_tsan.sh @@ -10,6 +10,6 @@ export CONTAINER_NAME=ci_native_tsan export DOCKER_NAME_TAG=ubuntu:20.04 export PACKAGES="clang llvm libc++abi-dev libc++-dev python3-zmq" export DEP_OPTS="CC=clang CXX='clang++ -stdlib=libc++'" -export TEST_RUNNER_EXTRA="--exclude feature_block --timeout-factor=4" # Increase timeout because sanitizers slow down. Low memory on Travis machines, exclude feature_block. +export TEST_RUNNER_EXTRA="--exclude feature_block" # Low memory on Travis machines, exclude feature_block. export GOAL="install" export BITCOIN_CONFIG="--enable-zmq --with-gui=no CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' CXXFLAGS='-g' --with-sanitizers=thread CC=clang CXX='clang++ -stdlib=libc++' --with-boost-process" diff --git a/ci/test/00_setup_env_native_valgrind.sh b/ci/test/00_setup_env_native_valgrind.sh index 0041122f1e..710d9e1011 100644 --- a/ci/test/00_setup_env_native_valgrind.sh +++ b/ci/test/00_setup_env_native_valgrind.sh @@ -10,6 +10,6 @@ export CONTAINER_NAME=ci_native_valgrind export PACKAGES="valgrind clang llvm python3-zmq libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev" export USE_VALGRIND=1 export NO_DEPENDS=1 -export TEST_RUNNER_EXTRA="--exclude rpc_bind --timeout-factor=4" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 +export TEST_RUNNER_EXTRA="--exclude rpc_bind" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 export GOAL="install" export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC=clang CXX=clang++" # TODO enable GUI diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index 96d44328b8..6c14a3dfbe 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -35,7 +35,7 @@ fi if [ "$RUN_FUNCTIONAL_TESTS" = "true" ]; then BEGIN_FOLD functional-tests - DOCKER_EXEC LD_LIBRARY_PATH=$DEPENDS_DIR/$HOST/lib ${TEST_RUNNER_ENV} test/functional/test_runner.py --ci $MAKEJOBS --tmpdirprefix "${BASE_SCRATCH_DIR}/test_runner/" --ansi --combinedlogslen=4000 ${TEST_RUNNER_EXTRA} --quiet --failfast + DOCKER_EXEC LD_LIBRARY_PATH=$DEPENDS_DIR/$HOST/lib ${TEST_RUNNER_ENV} test/functional/test_runner.py --ci $MAKEJOBS --tmpdirprefix "${BASE_SCRATCH_DIR}/test_runner/" --ansi --combinedlogslen=4000 --timeout-factor=${TEST_RUNNER_TIMEOUT_FACTOR} ${TEST_RUNNER_EXTRA} --quiet --failfast END_FOLD fi diff --git a/configure.ac b/configure.ac index 50ebbdbaba..3ab97eb531 100644 --- a/configure.ac +++ b/configure.ac @@ -105,6 +105,7 @@ AC_PATH_TOOL(AR, ar) AC_PATH_TOOL(RANLIB, ranlib) AC_PATH_TOOL(STRIP, strip) AC_PATH_TOOL(GCOV, gcov) +AC_PATH_TOOL(LLVM_COV, llvm-cov) AC_PATH_PROG(LCOV, lcov) dnl Python 3.5 is specified in .python-version and should be used if available, see doc/dependencies.md AC_PATH_PROGS([PYTHON], [python3.5 python3.6 python3.7 python3.8 python3 python]) @@ -376,6 +377,7 @@ if test "x$enable_werror" = "xyes"; then AX_CHECK_COMPILE_FLAG([-Werror=shadow-field],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=shadow-field"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=switch],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=switch"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=thread-safety],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=thread-safety"],,[[$CXXFLAG_WERROR]]) + AX_CHECK_COMPILE_FLAG([-Werror=range-loop-analysis],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=range-loop-analysis"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=unused-variable],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=unused-variable"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=date-time],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=date-time"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=return-type],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=return-type"],,[[$CXXFLAG_WERROR]]) @@ -684,16 +686,37 @@ if test x$use_lcov = xyes; then if test x$LCOV = x; then AC_MSG_ERROR("lcov testing requested but lcov not found") fi - if test x$GCOV = x; then - AC_MSG_ERROR("lcov testing requested but gcov not found") - fi if test x$PYTHON = x; then AC_MSG_ERROR("lcov testing requested but python not found") fi if test x$GENHTML = x; then AC_MSG_ERROR("lcov testing requested but genhtml not found") fi - LCOV="$LCOV --gcov-tool=$GCOV" + + AC_MSG_CHECKING([whether compiler is Clang]) + AC_PREPROC_IFELSE([AC_LANG_SOURCE([[ + #if defined(__clang__) && defined(__llvm__) + // Compiler is Clang + #else + # error Compiler is not Clang + #endif + ]])],[ + AC_MSG_RESULT([yes]) + if test x$LLVM_COV = x; then + AC_MSG_ERROR([lcov testing requested but llvm-cov not found]) + fi + COV_TOOL="$LLVM_COV gcov" + ],[ + AC_MSG_RESULT([no]) + if test x$GCOV = x; then + AC_MSG_ERROR([lcov testing requested but gcov not found]) + fi + COV_TOOL="$GCOV" + ]) + AC_SUBST(COV_TOOL) + AC_SUBST(COV_TOOL_WRAPPER, "cov_tool_wrapper.sh") + LCOV="$LCOV --gcov-tool $(pwd)/$COV_TOOL_WRAPPER" + AX_CHECK_LINK_FLAG([[--coverage]], [LDFLAGS="$LDFLAGS --coverage"], [AC_MSG_ERROR("lcov testing requested but --coverage linker flag does not work")]) AX_CHECK_COMPILE_FLAG([--coverage],[CXXFLAGS="$CXXFLAGS --coverage"], diff --git a/doc/release-notes-15937.md b/doc/release-notes-15937.md new file mode 100644 index 0000000000..ec7d355dfa --- /dev/null +++ b/doc/release-notes-15937.md @@ -0,0 +1,12 @@ +Configuration +------------- + +The `createwallet`, `loadwallet`, and `unloadwallet` RPCs now accept +`load_on_startup` options that modify bitcoin's dynamic configuration in +`\<datadir\>/settings.json`, and can add or remove a wallet from the list of +wallets automatically loaded at startup. Unless these options are explicitly +set to true or false, the load on startup wallet list is not modified, so this +change is backwards compatible. + +In the future, the GUI will start updating the same startup wallet list as the +RPCs to automatically reopen wallets previously opened in the GUI. diff --git a/src/Makefile.am b/src/Makefile.am index cd3cc95707..175501d4a6 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -140,6 +140,7 @@ BITCOIN_CORE_H = \ httpserver.h \ index/base.h \ index/blockfilterindex.h \ + index/disktxpos.h \ index/txindex.h \ indirectmap.h \ init.h \ @@ -676,12 +677,18 @@ CLEANFILES = $(EXTRA_LIBRARIES) CLEANFILES += *.gcda *.gcno CLEANFILES += compat/*.gcda compat/*.gcno CLEANFILES += consensus/*.gcda consensus/*.gcno +CLEANFILES += crc32c/src/*.gcda crc32c/src/*.gcno CLEANFILES += crypto/*.gcda crypto/*.gcno +CLEANFILES += index/*.gcda index/*.gcno +CLEANFILES += interfaces/*.gcda interfaces/*.gcno +CLEANFILES += node/*.gcda node/*.gcno CLEANFILES += policy/*.gcda policy/*.gcno CLEANFILES += primitives/*.gcda primitives/*.gcno +CLEANFILES += rpc/*.gcda rpc/*.gcno CLEANFILES += script/*.gcda script/*.gcno CLEANFILES += support/*.gcda support/*.gcno CLEANFILES += univalue/*.gcda univalue/*.gcno +CLEANFILES += util/*.gcda util/*.gcno CLEANFILES += wallet/*.gcda wallet/*.gcno CLEANFILES += wallet/test/*.gcda wallet/test/*.gcno CLEANFILES += zmq/*.gcda zmq/*.gcno diff --git a/src/Makefile.test.include b/src/Makefile.test.include index c3e46c0def..0068c94070 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -1208,7 +1208,7 @@ nodist_test_test_bitcoin_SOURCES = $(GENERATED_TEST_FILES) $(BITCOIN_TESTS): $(GENERATED_TEST_FILES) -CLEAN_BITCOIN_TEST = test/*.gcda test/*.gcno test/fuzz/*.gcda test/fuzz/*.gcno $(GENERATED_TEST_FILES) $(BITCOIN_TESTS:=.log) +CLEAN_BITCOIN_TEST = test/*.gcda test/*.gcno test/fuzz/*.gcda test/fuzz/*.gcno test/util/*.gcda test/util/*.gcno $(GENERATED_TEST_FILES) $(BITCOIN_TESTS:=.log) CLEANFILES += $(CLEAN_BITCOIN_TEST) diff --git a/src/addrman.cpp b/src/addrman.cpp index 7aba340d9d..7636c6bad2 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -479,11 +479,15 @@ int CAddrMan::Check_() } #endif -void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr) +void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct) { - unsigned int nNodes = ADDRMAN_GETADDR_MAX_PCT * vRandom.size() / 100; - if (nNodes > ADDRMAN_GETADDR_MAX) - nNodes = ADDRMAN_GETADDR_MAX; + size_t nNodes = vRandom.size(); + if (max_pct != 0) { + nNodes = max_pct * nNodes / 100; + } + if (max_addresses != 0) { + nNodes = std::min(nNodes, max_addresses); + } // gather a list of random nodes, skipping those of low quality for (unsigned int n = 0; n < vRandom.size(); n++) { diff --git a/src/addrman.h b/src/addrman.h index 9e742339db..ca045b91cd 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -153,12 +153,6 @@ public: //! how recent a successful connection should be before we allow an address to be evicted from tried #define ADDRMAN_REPLACEMENT_HOURS 4 -//! the maximum percentage of nodes to return in a getaddr call -#define ADDRMAN_GETADDR_MAX_PCT 23 - -//! the maximum number of nodes to return in a getaddr call -#define ADDRMAN_GETADDR_MAX 1000 - //! Convenience #define ADDRMAN_TRIED_BUCKET_COUNT (1 << ADDRMAN_TRIED_BUCKET_COUNT_LOG2) #define ADDRMAN_NEW_BUCKET_COUNT (1 << ADDRMAN_NEW_BUCKET_COUNT_LOG2) @@ -261,7 +255,7 @@ protected: #endif //! Select several addresses at once. - void GetAddr_(std::vector<CAddress> &vAddr) EXCLUSIVE_LOCKS_REQUIRED(cs); + void GetAddr_(std::vector<CAddress> &vAddr, size_t max_addresses, size_t max_pct) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Mark an entry as currently-connected-to. void Connected_(const CService &addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -638,13 +632,13 @@ public: } //! Return a bunch of addresses, selected at random. - std::vector<CAddress> GetAddr() + std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct) { Check(); std::vector<CAddress> vAddr; { LOCK(cs); - GetAddr_(vAddr); + GetAddr_(vAddr, max_addresses, max_pct); } Check(); return vAddr; diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index 26d9340768..ebdad5a4b8 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -98,7 +98,7 @@ static void AddrManGetAddr(benchmark::Bench& bench) FillAddrMan(addrman); bench.run([&] { - const auto& addresses = addrman.GetAddr(); + const auto& addresses = addrman.GetAddr(2500, 23); assert(addresses.size() > 0); }); } diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 56afcb6ded..a9119d5144 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -320,8 +320,8 @@ static void MutateTxAddOutPubKey(CMutableTransaction& tx, const std::string& str if (!pubkey.IsCompressed()) { throw std::runtime_error("Uncompressed pubkeys are not useable for SegWit outputs"); } - // Call GetScriptForWitness() to build a P2WSH scriptPubKey - scriptPubKey = GetScriptForWitness(scriptPubKey); + // Build a P2WPKH script + scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(pubkey)); } if (bScriptHash) { // Get the ID for the script, and then construct a P2SH destination for it. @@ -390,8 +390,8 @@ static void MutateTxAddOutMultiSig(CMutableTransaction& tx, const std::string& s throw std::runtime_error("Uncompressed pubkeys are not useable for SegWit outputs"); } } - // Call GetScriptForWitness() to build a P2WSH scriptPubKey - scriptPubKey = GetScriptForWitness(scriptPubKey); + // Build a P2WSH with the multisig script + scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(scriptPubKey)); } if (bScriptHash) { if (scriptPubKey.size() > MAX_SCRIPT_ELEMENT_SIZE) { @@ -464,7 +464,7 @@ static void MutateTxAddOutScript(CMutableTransaction& tx, const std::string& str } if (bSegWit) { - scriptPubKey = GetScriptForWitness(scriptPubKey); + scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(scriptPubKey)); } if (bScriptHash) { if (scriptPubKey.size() > MAX_SCRIPT_ELEMENT_SIZE) { diff --git a/src/chainparams.cpp b/src/chainparams.cpp index a7c9e33f07..ffd2076c9a 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -110,7 +110,7 @@ public: // Note that of those which support the service bits prefix, most only support a subset of // possible options. - // This is fine at runtime as we'll fall back to using them as a oneshot if they don't support the + // This is fine at runtime as we'll fall back to using them as an addrfetch if they don't support the // service bits we want, but we should get them updated to support all service bits wanted by any // release ASAP to avoid it where possible. vSeeds.emplace_back("seed.bitcoin.sipa.be"); // Pieter Wuille, only supports x1, x5, x9, and xd diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 18dc7a69e2..380d4eb8ac 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -35,6 +35,7 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const "-discardfee=<amt>", "-fallbackfee=<amt>", "-keypool=<n>", + "-maxapsfee=<n>", "-maxtxfee=<amt>", "-mintxfee=<amt>", "-paytxfee=<amt>", diff --git a/src/index/disktxpos.h b/src/index/disktxpos.h new file mode 100644 index 0000000000..8cd2270028 --- /dev/null +++ b/src/index/disktxpos.h @@ -0,0 +1,37 @@ +// Copyright (c) 2019 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_INDEX_DISKTXPOS_H +#define BITCOIN_INDEX_DISKTXPOS_H + +#include <chain.h> +#include <flatfile.h> +#include <primitives/block.h> +#include <primitives/transaction.h> + +struct CDiskTxPos : public FlatFilePos +{ + unsigned int nTxOffset; // after header + + SERIALIZE_METHODS(CDiskTxPos, obj) + { + READWRITEAS(FlatFilePos, obj); + READWRITE(VARINT(obj.nTxOffset)); + } + + CDiskTxPos(const FlatFilePos &blockIn, unsigned int nTxOffsetIn) : FlatFilePos(blockIn.nFile, blockIn.nPos), nTxOffset(nTxOffsetIn) { + } + + CDiskTxPos() { + SetNull(); + } + + void SetNull() { + FlatFilePos::SetNull(); + nTxOffset = 0; + } +}; + + +#endif // BITCOIN_INDEX_DISKTXPOS_H diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp index 64472714cc..104b05d4ba 100644 --- a/src/index/txindex.cpp +++ b/src/index/txindex.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <index/disktxpos.h> #include <index/txindex.h> #include <node/ui_interface.h> #include <shutdown.h> @@ -15,29 +16,6 @@ constexpr char DB_TXINDEX_BLOCK = 'T'; std::unique_ptr<TxIndex> g_txindex; -struct CDiskTxPos : public FlatFilePos -{ - unsigned int nTxOffset; // after header - - SERIALIZE_METHODS(CDiskTxPos, obj) - { - READWRITEAS(FlatFilePos, obj); - READWRITE(VARINT(obj.nTxOffset)); - } - - CDiskTxPos(const FlatFilePos &blockIn, unsigned int nTxOffsetIn) : FlatFilePos(blockIn.nFile, blockIn.nPos), nTxOffset(nTxOffsetIn) { - } - - CDiskTxPos() { - SetNull(); - } - - void SetNull() { - FlatFilePos::SetNull(); - nTxOffset = 0; - } -}; - /** * Access to the txindex database (indexes/txindex/) * diff --git a/src/init.cpp b/src/init.cpp index 08944b79a5..f5aef08211 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -24,6 +24,7 @@ #include <index/blockfilterindex.h> #include <index/txindex.h> #include <interfaces/chain.h> +#include <interfaces/node.h> #include <key.h> #include <miner.h> #include <net.h> @@ -999,11 +1000,13 @@ bool AppInitParameterInteraction() } } - // Basic filters are the only supported filters. The basic filters index must be enabled - // to serve compact filters - if (gArgs.GetBoolArg("-peerblockfilters", DEFAULT_PEERBLOCKFILTERS) && - g_enabled_filter_types.count(BlockFilterType::BASIC) != 1) { - return InitError(_("Cannot set -peerblockfilters without -blockfilterindex.")); + // Signal NODE_COMPACT_FILTERS if peerblockfilters and basic filters index are both enabled. + if (gArgs.GetBoolArg("-peerblockfilters", DEFAULT_PEERBLOCKFILTERS)) { + if (g_enabled_filter_types.count(BlockFilterType::BASIC) != 1) { + return InitError(_("Cannot set -peerblockfilters without -blockfilterindex.")); + } + + nLocalServices = ServiceFlags(nLocalServices | NODE_COMPACT_FILTERS); } // if using block pruning, then disallow txindex @@ -1242,7 +1245,7 @@ bool AppInitLockDataDirectory() return true; } -bool AppInitMain(const util::Ref& context, NodeContext& node) +bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) { const CChainParams& chainparams = Params(); // ********************************************************* Step 4a: application initialization @@ -1877,6 +1880,15 @@ bool AppInitMain(const util::Ref& context, NodeContext& node) LOCK(cs_main); LogPrintf("block tree size = %u\n", chainman.BlockIndex().size()); chain_active_height = chainman.ActiveChain().Height(); + if (tip_info) { + tip_info->block_height = chain_active_height; + tip_info->block_time = chainman.ActiveChain().Tip() ? chainman.ActiveChain().Tip()->GetBlockTime() : Params().GenesisBlock().GetBlockTime(); + tip_info->verification_progress = GuessVerificationProgress(Params().TxData(), chainman.ActiveChain().Tip()); + } + if (tip_info && ::pindexBestHeader) { + tip_info->header_height = ::pindexBestHeader->nHeight; + tip_info->header_time = ::pindexBestHeader->GetBlockTime(); + } } LogPrintf("nBestHeight = %d\n", chain_active_height); diff --git a/src/init.h b/src/init.h index 33fe96e8ea..20008ba5be 100644 --- a/src/init.h +++ b/src/init.h @@ -11,6 +11,9 @@ #include <util/system.h> struct NodeContext; +namespace interfaces { +struct BlockAndHeaderTipInfo; +} namespace boost { class thread_group; } // namespace boost @@ -54,7 +57,7 @@ bool AppInitLockDataDirectory(); * @note This should only be done after daemonization. Call Shutdown() if this function fails. * @pre Parameters should be parsed and config file should be read, AppInitLockDataDirectory should have been called. */ -bool AppInitMain(const util::Ref& context, NodeContext& node); +bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr); /** * Register all arguments with the ArgsManager diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index d49e4454af..313c1265de 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -372,6 +372,27 @@ public: RPCRunLater(name, std::move(fn), seconds); } int rpcSerializationFlags() override { return RPCSerializationFlags(); } + util::SettingsValue getRwSetting(const std::string& name) override + { + util::SettingsValue result; + gArgs.LockSettings([&](const util::Settings& settings) { + if (const util::SettingsValue* value = util::FindKey(settings.rw_settings, name)) { + result = *value; + } + }); + return result; + } + bool updateRwSetting(const std::string& name, const util::SettingsValue& value) override + { + gArgs.LockSettings([&](util::Settings& settings) { + if (value.isNull()) { + settings.rw_settings.erase(name); + } else { + settings.rw_settings[name] = value; + } + }); + return gArgs.WriteSettingsFile(); + } void requestMempoolTransactions(Notifications& notifications) override { LOCK2(::cs_main, ::mempool.cs); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index bbeb0fa801..053d40335f 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -7,6 +7,7 @@ #include <optional.h> // For Optional and nullopt #include <primitives/transaction.h> // For CTransactionRef +#include <util/settings.h> // For util::SettingsValue #include <functional> #include <memory> @@ -269,6 +270,12 @@ public: //! Current RPC serialization flags. virtual int rpcSerializationFlags() = 0; + //! Return <datadir>/settings.json setting value. + virtual util::SettingsValue getRwSetting(const std::string& name) = 0; + + //! Write a setting to <datadir>/settings.json. + virtual bool updateRwSetting(const std::string& name, const util::SettingsValue& value) = 0; + //! Synchronously send transactionAddedToMempool notifications about all //! current mempool transactions to the specified handler and return after //! the last one is sent. These notifications aren't coordinated with async diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 969767b90f..21400d00f8 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -80,10 +80,10 @@ public: return AppInitBasicSetup() && AppInitParameterInteraction() && AppInitSanityChecks() && AppInitLockDataDirectory(); } - bool appInitMain() override + bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override { m_context->chain = MakeChain(*m_context); - return AppInitMain(m_context_ref, *m_context); + return AppInitMain(m_context_ref, *m_context, tip_info); } void appShutdown() override { diff --git a/src/interfaces/node.h b/src/interfaces/node.h index cd3cfe487d..753f3e6b13 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -39,6 +39,16 @@ class Handler; class Wallet; struct BlockTip; +//! Block and header tip information +struct BlockAndHeaderTipInfo +{ + int block_height; + int64_t block_time; + int header_height; + int64_t header_time; + double verification_progress; +}; + //! Top-level interface for a bitcoin node (bitcoind process). class Node { @@ -96,7 +106,7 @@ public: virtual bool baseInitialize() = 0; //! Start node. - virtual bool appInitMain() = 0; + virtual bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info = nullptr) = 0; //! Stop node. virtual void appShutdown() = 0; diff --git a/src/net.cpp b/src/net.cpp index 25fa4709b0..6c1980735c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -105,10 +105,10 @@ std::map<CNetAddr, LocalServiceInfo> mapLocalHost GUARDED_BY(cs_mapLocalHost); static bool vfLimited[NET_MAX] GUARDED_BY(cs_mapLocalHost) = {}; std::string strSubVersion; -void CConnman::AddOneShot(const std::string& strDest) +void CConnman::AddAddrFetch(const std::string& strDest) { - LOCK(cs_vOneShots); - vOneShots.push_back(strDest); + LOCK(m_addr_fetches_mutex); + m_addr_fetches.push_back(strDest); } uint16_t GetListenPort() @@ -346,7 +346,7 @@ bool CConnman::CheckIncomingNonce(uint64_t nonce) { LOCK(cs_vNodes); for (const CNode* pnode : vNodes) { - if (!pnode->fSuccessfullyConnected && !pnode->fInbound && pnode->GetLocalNonce() == nonce) + if (!pnode->fSuccessfullyConnected && !pnode->IsInboundConn() && pnode->GetLocalNonce() == nonce) return false; } return true; @@ -368,8 +368,10 @@ static CAddress GetBindAddress(SOCKET sock) return addr_bind; } -CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, bool manual_connection, bool block_relay_only) +CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type) { + assert(conn_type != ConnectionType::INBOUND); + if (pszDest == nullptr) { if (IsLocal(addrConnect)) return nullptr; @@ -432,7 +434,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo if (hSocket == INVALID_SOCKET) { return nullptr; } - connected = ConnectSocketDirectly(addrConnect, hSocket, nConnectTimeout, manual_connection); + connected = ConnectSocketDirectly(addrConnect, hSocket, nConnectTimeout, conn_type == ConnectionType::MANUAL); } if (!proxyConnectionFailed) { // If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to @@ -459,7 +461,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo NodeId id = GetNewNodeId(); uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); CAddress addr_bind = GetBindAddress(hSocket); - CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", false, block_relay_only); + CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", conn_type); pnode->AddRef(); // We're making a new connection, harvest entropy from the time (and our peer count) @@ -536,8 +538,8 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap) LOCK(cs_SubVer); X(cleanSubVer); } - X(fInbound); - X(m_manual_connection); + stats.fInbound = IsInboundConn(); + stats.m_manual_connection = IsManualConn(); X(nStartingHeight); { LOCK(cs_vSend); @@ -872,7 +874,7 @@ bool CConnman::AttemptToEvictConnection() for (const CNode* node : vNodes) { if (node->HasPermission(PF_NOBAN)) continue; - if (!node->fInbound) + if (!node->IsInboundConn()) continue; if (node->fDisconnect) continue; @@ -983,7 +985,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { { LOCK(cs_vNodes); for (const CNode* pnode : vNodes) { - if (pnode->fInbound) nInbound++; + if (pnode->IsInboundConn()) nInbound++; } } @@ -1048,7 +1050,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { if (NetPermissions::HasFlag(permissionFlags, PF_BLOOMFILTER)) { nodeServices = static_cast<ServiceFlags>(nodeServices | NODE_BLOOM); } - CNode* pnode = new CNode(id, nodeServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", true); + CNode* pnode = new CNode(id, nodeServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", ConnectionType::INBOUND); pnode->AddRef(); pnode->m_permissionFlags = permissionFlags; // If this flag is present, the user probably expect that RPC and QT report it as whitelisted (backward compatibility) @@ -1646,7 +1648,7 @@ void CConnman::ThreadDNSAddressSeed() { LOCK(cs_vNodes); for (const CNode* pnode : vNodes) { - nRelevant += pnode->fSuccessfullyConnected && !pnode->fFeeler && !pnode->fOneShot && !pnode->m_manual_connection && !pnode->fInbound; + if (pnode->fSuccessfullyConnected && pnode->IsOutboundOrBlockRelayConn()) ++nRelevant; } } if (nRelevant >= 2) { @@ -1674,7 +1676,7 @@ void CConnman::ThreadDNSAddressSeed() LogPrintf("Loading addresses from DNS seed %s\n", seed); if (HaveNameProxy()) { - AddOneShot(seed); + AddAddrFetch(seed); } else { std::vector<CNetAddr> vIPs; std::vector<CAddress> vAdd; @@ -1696,8 +1698,8 @@ void CConnman::ThreadDNSAddressSeed() addrman.Add(vAdd, resolveSource); } else { // We now avoid directly using results from DNS Seeds which do not support service bit filtering, - // instead using them as a oneshot to get nodes with our desired service bits. - AddOneShot(seed); + // instead using them as a addrfetch to get nodes with our desired service bits. + AddAddrFetch(seed); } } --seeds_right_now; @@ -1705,17 +1707,6 @@ void CConnman::ThreadDNSAddressSeed() LogPrintf("%d addresses found from DNS seeds\n", found); } - - - - - - - - - - - void CConnman::DumpAddresses() { int64_t nStart = GetTimeMillis(); @@ -1727,20 +1718,20 @@ void CConnman::DumpAddresses() addrman.size(), GetTimeMillis() - nStart); } -void CConnman::ProcessOneShot() +void CConnman::ProcessAddrFetch() { std::string strDest; { - LOCK(cs_vOneShots); - if (vOneShots.empty()) + LOCK(m_addr_fetches_mutex); + if (m_addr_fetches.empty()) return; - strDest = vOneShots.front(); - vOneShots.pop_front(); + strDest = m_addr_fetches.front(); + m_addr_fetches.pop_front(); } CAddress addr; CSemaphoreGrant grant(*semOutbound, true); if (grant) { - OpenNetworkConnection(addr, false, &grant, strDest.c_str(), true); + OpenNetworkConnection(addr, false, &grant, strDest.c_str(), ConnectionType::ADDR_FETCH); } } @@ -1767,7 +1758,7 @@ int CConnman::GetExtraOutboundCount() { LOCK(cs_vNodes); for (const CNode* pnode : vNodes) { - if (!pnode->fInbound && !pnode->m_manual_connection && !pnode->fFeeler && !pnode->fDisconnect && !pnode->fOneShot && pnode->fSuccessfullyConnected) { + if (pnode->fSuccessfullyConnected && !pnode->fDisconnect && pnode->IsOutboundOrBlockRelayConn()) { ++nOutbound; } } @@ -1782,11 +1773,11 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) { for (int64_t nLoop = 0;; nLoop++) { - ProcessOneShot(); + ProcessAddrFetch(); for (const std::string& strAddr : connect) { CAddress addr(CService(), NODE_NONE); - OpenNetworkConnection(addr, false, nullptr, strAddr.c_str(), false, false, true); + OpenNetworkConnection(addr, false, nullptr, strAddr.c_str(), ConnectionType::MANUAL); for (int i = 0; i < 10 && i < nLoop; i++) { if (!interruptNet.sleep_for(std::chrono::milliseconds(500))) @@ -1805,7 +1796,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) int64_t nNextFeeler = PoissonNextSend(nStart*1000*1000, FEELER_INTERVAL); while (!interruptNet) { - ProcessOneShot(); + ProcessAddrFetch(); if (!interruptNet.sleep_for(std::chrono::milliseconds(500))) return; @@ -1838,21 +1829,27 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) int nOutboundFullRelay = 0; int nOutboundBlockRelay = 0; std::set<std::vector<unsigned char> > setConnected; + { LOCK(cs_vNodes); for (const CNode* pnode : vNodes) { - if (!pnode->fInbound && !pnode->m_manual_connection) { - // Netgroups for inbound and addnode peers are not excluded because our goal here - // is to not use multiple of our limited outbound slots on a single netgroup - // but inbound and addnode peers do not use our outbound slots. Inbound peers - // also have the added issue that they're attacker controlled and could be used - // to prevent us from connecting to particular hosts if we used them here. - setConnected.insert(pnode->addr.GetGroup(addrman.m_asmap)); - if (pnode->m_tx_relay == nullptr) { - nOutboundBlockRelay++; - } else if (!pnode->fFeeler) { - nOutboundFullRelay++; - } + if (pnode->IsFullOutboundConn()) nOutboundFullRelay++; + if (pnode->IsBlockOnlyConn()) nOutboundBlockRelay++; + + // Netgroups for inbound and manual peers are not excluded because our goal here + // is to not use multiple of our limited outbound slots on a single netgroup + // but inbound and manual peers do not use our outbound slots. Inbound peers + // also have the added issue that they could be attacker controlled and used + // to prevent us from connecting to particular hosts if we used them here. + switch(pnode->m_conn_type){ + case ConnectionType::INBOUND: + case ConnectionType::MANUAL: + break; + case ConnectionType::OUTBOUND: + case ConnectionType::BLOCK_RELAY: + case ConnectionType::ADDR_FETCH: + case ConnectionType::FEELER: + setConnected.insert(pnode->addr.GetGroup(addrman.m_asmap)); } } } @@ -1945,14 +1942,24 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) LogPrint(BCLog::NET, "Making feeler connection to %s\n", addrConnect.ToString()); } - // Open this connection as block-relay-only if we're already at our - // full-relay capacity, but not yet at our block-relay peer limit. - // (It should not be possible for fFeeler to be set if we're not - // also at our block-relay peer limit, but check against that as - // well for sanity.) - bool block_relay_only = nOutboundBlockRelay < m_max_outbound_block_relay && !fFeeler && nOutboundFullRelay >= m_max_outbound_full_relay; + ConnectionType conn_type; + // Determine what type of connection to open. If fFeeler is not + // set, open OUTBOUND connections until we meet our full-relay + // capacity. Then open BLOCK_RELAY connections until we hit our + // block-relay peer limit. Otherwise, default to opening an + // OUTBOUND connection. + if (fFeeler) { + conn_type = ConnectionType::FEELER; + } else if (nOutboundFullRelay < m_max_outbound_full_relay) { + conn_type = ConnectionType::OUTBOUND; + } else if (nOutboundBlockRelay < m_max_outbound_block_relay) { + conn_type = ConnectionType::BLOCK_RELAY; + } else { + // GetTryNewOutboundPeer() is true + conn_type = ConnectionType::OUTBOUND; + } - OpenNetworkConnection(addrConnect, (int)setConnected.size() >= std::min(nMaxConnections - 1, 2), &grant, nullptr, false, fFeeler, false, block_relay_only); + OpenNetworkConnection(addrConnect, (int)setConnected.size() >= std::min(nMaxConnections - 1, 2), &grant, nullptr, conn_type); } } } @@ -1976,11 +1983,11 @@ std::vector<AddedNodeInfo> CConnman::GetAddedNodeInfo() LOCK(cs_vNodes); for (const CNode* pnode : vNodes) { if (pnode->addr.IsValid()) { - mapConnected[pnode->addr] = pnode->fInbound; + mapConnected[pnode->addr] = pnode->IsInboundConn(); } std::string addrName = pnode->GetAddrName(); if (!addrName.empty()) { - mapConnectedByName[std::move(addrName)] = std::make_pair(pnode->fInbound, static_cast<const CService&>(pnode->addr)); + mapConnectedByName[std::move(addrName)] = std::make_pair(pnode->IsInboundConn(), static_cast<const CService&>(pnode->addr)); } } } @@ -2027,7 +2034,7 @@ void CConnman::ThreadOpenAddedConnections() } tried = true; CAddress addr(CService(), NODE_NONE); - OpenNetworkConnection(addr, false, &grant, info.strAddedNode.c_str(), false, false, true); + OpenNetworkConnection(addr, false, &grant, info.strAddedNode.c_str(), ConnectionType::MANUAL); if (!interruptNet.sleep_for(std::chrono::milliseconds(500))) return; } @@ -2039,8 +2046,10 @@ void CConnman::ThreadOpenAddedConnections() } // if successful, this moves the passed grant to the constructed node -void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, bool fOneShot, bool fFeeler, bool manual_connection, bool block_relay_only) +void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, ConnectionType conn_type) { + assert(conn_type != ConnectionType::INBOUND); + // // Initiate outbound network connection // @@ -2058,18 +2067,12 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai } else if (FindNode(std::string(pszDest))) return; - CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, manual_connection, block_relay_only); + CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, conn_type); if (!pnode) return; if (grantOutbound) grantOutbound->MoveTo(pnode->grantOutbound); - if (fOneShot) - pnode->fOneShot = true; - if (fFeeler) - pnode->fFeeler = true; - if (manual_connection) - pnode->m_manual_connection = true; m_msgproc->InitializeNode(pnode); { @@ -2127,11 +2130,6 @@ void CConnman::ThreadMessageHandler() } } - - - - - bool CConnman::BindListenPort(const CService& addrBind, bilingual_str& strError, NetPermissionFlags permissions) { int nOne = 1; @@ -2337,7 +2335,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) } for (const auto& strDest : connOptions.vSeedNodes) { - AddOneShot(strDest); + AddAddrFetch(strDest); } if (clientInterface) { @@ -2390,7 +2388,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) else threadDNSAddressSeed = std::thread(&TraceThread<std::function<void()> >, "dnsseed", std::function<void()>(std::bind(&CConnman::ThreadDNSAddressSeed, this))); - // Initiate outbound connections from -addnode + // Initiate manual connections threadOpenAddedConnections = std::thread(&TraceThread<std::function<void()> >, "addcon", std::function<void()>(std::bind(&CConnman::ThreadOpenAddedConnections, this))); if (connOptions.m_use_addrman_outgoing && !connOptions.m_specified_outgoing.empty()) { @@ -2523,14 +2521,14 @@ void CConnman::MarkAddressGood(const CAddress& addr) addrman.Good(addr); } -void CConnman::AddNewAddresses(const std::vector<CAddress>& vAddr, const CAddress& addrFrom, int64_t nTimePenalty) +bool CConnman::AddNewAddresses(const std::vector<CAddress>& vAddr, const CAddress& addrFrom, int64_t nTimePenalty) { - addrman.Add(vAddr, addrFrom, nTimePenalty); + return addrman.Add(vAddr, addrFrom, nTimePenalty); } -std::vector<CAddress> CConnman::GetAddresses() +std::vector<CAddress> CConnman::GetAddresses(size_t max_addresses, size_t max_pct) { - std::vector<CAddress> addresses = addrman.GetAddr(); + std::vector<CAddress> addresses = addrman.GetAddr(max_addresses, max_pct); if (m_banman) { addresses.erase(std::remove_if(addresses.begin(), addresses.end(), [this](const CAddress& addr){return m_banman->IsDiscouraged(addr) || m_banman->IsBanned(addr);}), @@ -2539,12 +2537,12 @@ std::vector<CAddress> CConnman::GetAddresses() return addresses; } -std::vector<CAddress> CConnman::GetAddresses(Network requestor_network) +std::vector<CAddress> CConnman::GetAddresses(Network requestor_network, size_t max_addresses, size_t max_pct) { const auto current_time = GetTime<std::chrono::microseconds>(); if (m_addr_response_caches.find(requestor_network) == m_addr_response_caches.end() || m_addr_response_caches[requestor_network].m_update_addr_response < current_time) { - m_addr_response_caches[requestor_network].m_addrs_response_cache = GetAddresses(); + m_addr_response_caches[requestor_network].m_addrs_response_cache = GetAddresses(max_addresses, max_pct); m_addr_response_caches[requestor_network].m_update_addr_response = current_time + std::chrono::hours(21) + GetRandMillis(std::chrono::hours(6)); } return m_addr_response_caches[requestor_network].m_addrs_response_cache; @@ -2581,7 +2579,7 @@ size_t CConnman::GetNodeCount(NumConnections flags) int nNum = 0; for (const auto& pnode : vNodes) { - if (flags & (pnode->fInbound ? CONNECTIONS_IN : CONNECTIONS_OUT)) { + if (flags & (pnode->IsInboundConn() ? CONNECTIONS_IN : CONNECTIONS_OUT)) { nNum++; } } @@ -2765,26 +2763,26 @@ int CConnman::GetBestHeight() const unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; } -CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, bool fInboundIn, bool block_relay_only) +CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in) : nTimeConnected(GetSystemTimeInSeconds()), addr(addrIn), addrBind(addrBindIn), - fInbound(fInboundIn), nKeyedNetGroup(nKeyedNetGroupIn), // Don't relay addr messages to peers that we connect to as block-relay-only // peers (to prevent adversaries from inferring these links from addr // traffic). - m_addr_known{block_relay_only ? nullptr : MakeUnique<CRollingBloomFilter>(5000, 0.001)}, id(idIn), nLocalHostNonce(nLocalHostNonceIn), + m_conn_type(conn_type_in), nLocalServices(nLocalServicesIn), nMyStartingHeight(nMyStartingHeightIn) { hSocket = hSocketIn; addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn; hashContinue = uint256(); - if (!block_relay_only) { + if (conn_type_in != ConnectionType::BLOCK_RELAY) { m_tx_relay = MakeUnique<TxRelay>(); + m_addr_known = MakeUnique<CRollingBloomFilter>(5000, 0.001); } for (const std::string &msg : getAllNetMessageTypes()) @@ -51,11 +51,8 @@ static const bool DEFAULT_WHITELISTFORCERELAY = false; static const int TIMEOUT_INTERVAL = 20 * 60; /** Run the feeler connection loop once every 2 minutes or 120 seconds. **/ static const int FEELER_INTERVAL = 120; -/** The maximum number of new addresses to accumulate before announcing. */ -static const unsigned int MAX_ADDR_TO_SEND = 1000; -// TODO: remove ADDRMAN_GETADDR_MAX and let the caller specify this limit with MAX_ADDR_TO_SEND. -static_assert(MAX_ADDR_TO_SEND == ADDRMAN_GETADDR_MAX, - "Max allowed ADDR message size should be equal to the max number of records returned from AddrMan."); +/** The maximum number of addresses from our addrman to return in response to a getaddr message. */ +static constexpr size_t MAX_ADDR_TO_SEND = 1000; /** Maximum length of incoming protocol messages (no message over 4 MB is currently acceptable). */ static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 4 * 1000 * 1000; /** Maximum length of the user agent string in `version` message */ @@ -117,6 +114,17 @@ struct CSerializedNetMsg std::string m_type; }; +/** Different types of connections to a peer. This enum encapsulates the + * information we have available at the time of opening or accepting the + * connection. Aside from INBOUND, all types are initiated by us. */ +enum class ConnectionType { + INBOUND, /**< peer initiated connections */ + OUTBOUND, /**< full relay connections (blocks, addrs, txns) made automatically. Addresses selected from AddrMan. */ + MANUAL, /**< connections to addresses added via addnode or the connect command line argument */ + FEELER, /**< short lived connections used to test address validity */ + BLOCK_RELAY, /**< only relay blocks to these automatic outbound connections. Addresses selected from AddrMan. */ + ADDR_FETCH, /**< short lived connections used to solicit addrs when starting the node without a populated AddrMan */ +}; class NetEventsInterface; class CConnman @@ -201,7 +209,7 @@ public: bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; void SetNetworkActive(bool active); - void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = nullptr, const char *strDest = nullptr, bool fOneShot = false, bool fFeeler = false, bool manual_connection = false, bool block_relay_only = false); + void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = nullptr, const char *strDest = nullptr, ConnectionType conn_type = ConnectionType::OUTBOUND); bool CheckIncomingNonce(uint64_t nonce); bool ForNode(NodeId id, std::function<bool(CNode* pnode)> func); @@ -253,15 +261,15 @@ public: // Addrman functions void SetServices(const CService &addr, ServiceFlags nServices); void MarkAddressGood(const CAddress& addr); - void AddNewAddresses(const std::vector<CAddress>& vAddr, const CAddress& addrFrom, int64_t nTimePenalty = 0); - std::vector<CAddress> GetAddresses(); + bool AddNewAddresses(const std::vector<CAddress>& vAddr, const CAddress& addrFrom, int64_t nTimePenalty = 0); + std::vector<CAddress> GetAddresses(size_t max_addresses, size_t max_pct); /** * Cache is used to minimize topology leaks, so it should * be used for all non-trusted calls, for example, p2p. * A non-malicious call (from RPC or a peer with addr permission) should * call the function without a parameter to avoid using the cache. */ - std::vector<CAddress> GetAddresses(Network requestor_network); + std::vector<CAddress> GetAddresses(Network requestor_network, size_t max_addresses, size_t max_pct); // This allows temporarily exceeding m_max_outbound_full_relay, with the goal of finding // a peer that is better than all our current peers. @@ -351,8 +359,8 @@ private: bool Bind(const CService& addr, unsigned int flags, NetPermissionFlags permissions); bool InitBinds(const std::vector<CService>& binds, const std::vector<NetWhitebindPermissions>& whiteBinds); void ThreadOpenAddedConnections(); - void AddOneShot(const std::string& strDest); - void ProcessOneShot(); + void AddAddrFetch(const std::string& strDest); + void ProcessAddrFetch(); void ThreadOpenConnections(std::vector<std::string> connect); void ThreadMessageHandler(); void AcceptConnection(const ListenSocket& hListenSocket); @@ -373,7 +381,7 @@ private: CNode* FindNode(const CService& addr); bool AttemptToEvictConnection(); - CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, bool manual_connection, bool block_relay_only); + CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type); void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const; void DeleteNode(CNode* pnode); @@ -416,8 +424,8 @@ private: std::atomic<bool> fNetworkActive{true}; bool fAddressesInitialized{false}; CAddrMan addrman; - std::deque<std::string> vOneShots GUARDED_BY(cs_vOneShots); - RecursiveMutex cs_vOneShots; + std::deque<std::string> m_addr_fetches GUARDED_BY(m_addr_fetches_mutex); + RecursiveMutex m_addr_fetches_mutex; std::vector<std::string> vAddedNodes GUARDED_BY(cs_vAddedNodes); RecursiveMutex cs_vAddedNodes; std::vector<CNode*> vNodes GUARDED_BY(cs_vNodes); @@ -798,12 +806,8 @@ public: } // This boolean is unusued in actual processing, only present for backward compatibility at RPC/QT level bool m_legacyWhitelisted{false}; - bool fFeeler{false}; // If true this node is being used as a short lived feeler. - bool fOneShot{false}; - bool m_manual_connection{false}; bool fClient{false}; // set by version message bool m_limited_node{false}; //after BIP159, set by version message - const bool fInbound; std::atomic_bool fSuccessfullyConnected{false}; // Setting fDisconnect to true will cause the node to be disconnected the // next time DisconnectNodes() runs @@ -816,6 +820,60 @@ public: std::atomic_bool fPauseRecv{false}; std::atomic_bool fPauseSend{false}; + bool IsOutboundOrBlockRelayConn() const { + switch(m_conn_type) { + case ConnectionType::OUTBOUND: + case ConnectionType::BLOCK_RELAY: + return true; + case ConnectionType::INBOUND: + case ConnectionType::MANUAL: + case ConnectionType::ADDR_FETCH: + case ConnectionType::FEELER: + return false; + } + + assert(false); + } + + bool IsFullOutboundConn() const { + return m_conn_type == ConnectionType::OUTBOUND; + } + + bool IsManualConn() const { + return m_conn_type == ConnectionType::MANUAL; + } + + bool IsBlockOnlyConn() const { + return m_conn_type == ConnectionType::BLOCK_RELAY; + } + + bool IsFeelerConn() const { + return m_conn_type == ConnectionType::FEELER; + } + + bool IsAddrFetchConn() const { + return m_conn_type == ConnectionType::ADDR_FETCH; + } + + bool IsInboundConn() const { + return m_conn_type == ConnectionType::INBOUND; + } + + bool ExpectServicesFromConn() const { + switch(m_conn_type) { + case ConnectionType::INBOUND: + case ConnectionType::MANUAL: + case ConnectionType::FEELER: + return false; + case ConnectionType::OUTBOUND: + case ConnectionType::BLOCK_RELAY: + case ConnectionType::ADDR_FETCH: + return true; + } + + assert(false); + } + protected: mapMsgCmdSize mapSendBytesPerMsgCmd; mapMsgCmdSize mapRecvBytesPerMsgCmd GUARDED_BY(cs_vRecv); @@ -826,7 +884,7 @@ public: // flood relay std::vector<CAddress> vAddrToSend; - const std::unique_ptr<CRollingBloomFilter> m_addr_known; + std::unique_ptr<CRollingBloomFilter> m_addr_known = nullptr; bool fGetAddr{false}; std::chrono::microseconds m_next_addr_send GUARDED_BY(cs_sendProcessing){0}; std::chrono::microseconds m_next_local_addr_send GUARDED_BY(cs_sendProcessing){0}; @@ -890,7 +948,7 @@ public: std::set<uint256> orphan_work_set; - CNode(NodeId id, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress &addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress &addrBindIn, const std::string &addrNameIn = "", bool fInboundIn = false, bool block_relay_only = false); + CNode(NodeId id, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress &addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress &addrBindIn, const std::string &addrNameIn, ConnectionType conn_type_in); ~CNode(); CNode(const CNode&) = delete; CNode& operator=(const CNode&) = delete; @@ -898,6 +956,7 @@ public: private: const NodeId id; const uint64_t nLocalHostNonce; + const ConnectionType m_conn_type; //! Services offered to this peer. //! diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 7c2d7335a0..7b83583e41 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -143,6 +143,8 @@ static constexpr unsigned int MAX_FEEFILTER_CHANGE_DELAY = 5 * 60; static constexpr uint32_t MAX_GETCFILTERS_SIZE = 1000; /** Maximum number of cf hashes that may be requested with one getcfheaders. See BIP 157. */ static constexpr uint32_t MAX_GETCFHEADERS_SIZE = 2000; +/** the maximum percentage of addresses from our addrman to return in response to a getaddr message. */ +static constexpr size_t MAX_PCT_ADDR_TO_SEND = 23; struct COrphanTx { // When modifying, adapt the copy of this definition in tests/DoS_tests. @@ -479,7 +481,7 @@ static void UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUS nPreferredDownload -= state->fPreferredDownload; // Whether this node should be marked as a preferred download node. - state->fPreferredDownload = (!node.fInbound || node.HasPermission(PF_NOBAN)) && !node.fOneShot && !node.fClient; + state->fPreferredDownload = (!node.IsInboundConn() || node.HasPermission(PF_NOBAN)) && !node.IsAddrFetchConn() && !node.fClient; nPreferredDownload += state->fPreferredDownload; } @@ -833,22 +835,15 @@ void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) if (state) state->m_last_block_announcement = time_in_seconds; } -// Returns true for outbound peers, excluding manual connections, feelers, and -// one-shots. -static bool IsOutboundDisconnectionCandidate(const CNode& node) -{ - return !(node.fInbound || node.m_manual_connection || node.fFeeler || node.fOneShot); -} - void PeerLogicValidation::InitializeNode(CNode *pnode) { CAddress addr = pnode->addr; std::string addrName = pnode->GetAddrName(); NodeId nodeid = pnode->GetId(); { LOCK(cs_main); - mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, std::move(addrName), pnode->fInbound, pnode->m_manual_connection)); + mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, std::move(addrName), pnode->IsInboundConn(), pnode->IsManualConn())); } - if(!pnode->fInbound) + if(!pnode->IsInboundConn()) PushNodeVersion(*pnode, *connman, GetTime()); } @@ -1982,14 +1977,14 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateMan // until we have a headers chain that has at least // nMinimumChainWork, even if a peer has a chain past our tip, // as an anti-DoS measure. - if (IsOutboundDisconnectionCandidate(pfrom)) { + if (pfrom.IsOutboundOrBlockRelayConn()) { LogPrintf("Disconnecting outbound peer %d -- headers chain has insufficient work\n", pfrom.GetId()); pfrom.fDisconnect = true; } } } - if (!pfrom.fDisconnect && IsOutboundDisconnectionCandidate(pfrom) && nodestate->pindexBestKnownBlock != nullptr && pfrom.m_tx_relay != nullptr) { + if (!pfrom.fDisconnect && pfrom.IsOutboundOrBlockRelayConn() && nodestate->pindexBestKnownBlock != nullptr && pfrom.m_tx_relay != nullptr) { // If this is an outbound full-relay peer, check to see if we should protect // it from the bad/lagging chain logic. // Note that block-relay-only peers are already implicitly protected, so we @@ -2096,7 +2091,7 @@ void static ProcessOrphanTx(CConnman& connman, CTxMemPool& mempool, std::set<uin * * May disconnect from the peer in the case of a bad request. * - * @param[in] pfrom The peer that we received the request from + * @param[in] peer The peer that we received the request from * @param[in] chain_params Chain parameters * @param[in] filter_type The filter type the request is for. Must be basic filters. * @param[in] start_height The start height for the request @@ -2106,7 +2101,7 @@ void static ProcessOrphanTx(CConnman& connman, CTxMemPool& mempool, std::set<uin * @param[out] filter_index The filter index, if the request can be serviced. * @return True if the request can be serviced. */ -static bool PrepareBlockFilterRequest(CNode& pfrom, const CChainParams& chain_params, +static bool PrepareBlockFilterRequest(CNode& peer, const CChainParams& chain_params, BlockFilterType filter_type, uint32_t start_height, const uint256& stop_hash, uint32_t max_height_diff, const CBlockIndex*& stop_index, @@ -2114,11 +2109,11 @@ static bool PrepareBlockFilterRequest(CNode& pfrom, const CChainParams& chain_pa { const bool supported_filter_type = (filter_type == BlockFilterType::BASIC && - gArgs.GetBoolArg("-peerblockfilters", DEFAULT_PEERBLOCKFILTERS)); + (peer.GetLocalServices() & NODE_COMPACT_FILTERS)); if (!supported_filter_type) { LogPrint(BCLog::NET, "peer %d requested unsupported block filter type: %d\n", - pfrom.GetId(), static_cast<uint8_t>(filter_type)); - pfrom.fDisconnect = true; + peer.GetId(), static_cast<uint8_t>(filter_type)); + peer.fDisconnect = true; return false; } @@ -2129,8 +2124,8 @@ static bool PrepareBlockFilterRequest(CNode& pfrom, const CChainParams& chain_pa // Check that the stop block exists and the peer would be allowed to fetch it. if (!stop_index || !BlockRequestAllowed(stop_index, chain_params.GetConsensus())) { LogPrint(BCLog::NET, "peer %d requested invalid block hash: %s\n", - pfrom.GetId(), stop_hash.ToString()); - pfrom.fDisconnect = true; + peer.GetId(), stop_hash.ToString()); + peer.fDisconnect = true; return false; } } @@ -2139,14 +2134,14 @@ static bool PrepareBlockFilterRequest(CNode& pfrom, const CChainParams& chain_pa if (start_height > stop_height) { LogPrint(BCLog::NET, "peer %d sent invalid getcfilters/getcfheaders with " /* Continued */ "start height %d and stop height %d\n", - pfrom.GetId(), start_height, stop_height); - pfrom.fDisconnect = true; + peer.GetId(), start_height, stop_height); + peer.fDisconnect = true; return false; } if (stop_height - start_height >= max_height_diff) { LogPrint(BCLog::NET, "peer %d requested too many cfilters/cfheaders: %d / %d\n", - pfrom.GetId(), stop_height - start_height + 1, max_height_diff); - pfrom.fDisconnect = true; + peer.GetId(), stop_height - start_height + 1, max_height_diff); + peer.fDisconnect = true; return false; } @@ -2164,12 +2159,12 @@ static bool PrepareBlockFilterRequest(CNode& pfrom, const CChainParams& chain_pa * * May disconnect from the peer in the case of a bad request. * - * @param[in] pfrom The peer that we received the request from + * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received * @param[in] chain_params Chain parameters * @param[in] connman Pointer to the connection manager */ -static void ProcessGetCFilters(CNode& pfrom, CDataStream& vRecv, const CChainParams& chain_params, +static void ProcessGetCFilters(CNode& peer, CDataStream& vRecv, const CChainParams& chain_params, CConnman& connman) { uint8_t filter_type_ser; @@ -2182,13 +2177,12 @@ static void ProcessGetCFilters(CNode& pfrom, CDataStream& vRecv, const CChainPar const CBlockIndex* stop_index; BlockFilterIndex* filter_index; - if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, start_height, stop_hash, + if (!PrepareBlockFilterRequest(peer, chain_params, filter_type, start_height, stop_hash, MAX_GETCFILTERS_SIZE, stop_index, filter_index)) { return; } std::vector<BlockFilter> filters; - if (!filter_index->LookupFilterRange(start_height, stop_index, filters)) { LogPrint(BCLog::NET, "Failed to find block filter in index: filter_type=%s, start_height=%d, stop_hash=%s\n", BlockFilterTypeName(filter_type), start_height, stop_hash.ToString()); @@ -2196,9 +2190,9 @@ static void ProcessGetCFilters(CNode& pfrom, CDataStream& vRecv, const CChainPar } for (const auto& filter : filters) { - CSerializedNetMsg msg = CNetMsgMaker(pfrom.GetSendVersion()) + CSerializedNetMsg msg = CNetMsgMaker(peer.GetSendVersion()) .Make(NetMsgType::CFILTER, filter); - connman.PushMessage(&pfrom, std::move(msg)); + connman.PushMessage(&peer, std::move(msg)); } } @@ -2207,12 +2201,12 @@ static void ProcessGetCFilters(CNode& pfrom, CDataStream& vRecv, const CChainPar * * May disconnect from the peer in the case of a bad request. * - * @param[in] pfrom The peer that we received the request from + * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received * @param[in] chain_params Chain parameters * @param[in] connman Pointer to the connection manager */ -static void ProcessGetCFHeaders(CNode& pfrom, CDataStream& vRecv, const CChainParams& chain_params, +static void ProcessGetCFHeaders(CNode& peer, CDataStream& vRecv, const CChainParams& chain_params, CConnman& connman) { uint8_t filter_type_ser; @@ -2225,7 +2219,7 @@ static void ProcessGetCFHeaders(CNode& pfrom, CDataStream& vRecv, const CChainPa const CBlockIndex* stop_index; BlockFilterIndex* filter_index; - if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, start_height, stop_hash, + if (!PrepareBlockFilterRequest(peer, chain_params, filter_type, start_height, stop_hash, MAX_GETCFHEADERS_SIZE, stop_index, filter_index)) { return; } @@ -2248,13 +2242,13 @@ static void ProcessGetCFHeaders(CNode& pfrom, CDataStream& vRecv, const CChainPa return; } - CSerializedNetMsg msg = CNetMsgMaker(pfrom.GetSendVersion()) + CSerializedNetMsg msg = CNetMsgMaker(peer.GetSendVersion()) .Make(NetMsgType::CFHEADERS, filter_type_ser, stop_index->GetBlockHash(), prev_header, filter_hashes); - connman.PushMessage(&pfrom, std::move(msg)); + connman.PushMessage(&peer, std::move(msg)); } /** @@ -2262,12 +2256,12 @@ static void ProcessGetCFHeaders(CNode& pfrom, CDataStream& vRecv, const CChainPa * * May disconnect from the peer in the case of a bad request. * - * @param[in] pfrom The peer that we received the request from + * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received * @param[in] chain_params Chain parameters * @param[in] connman Pointer to the connection manager */ -static void ProcessGetCFCheckPt(CNode& pfrom, CDataStream& vRecv, const CChainParams& chain_params, +static void ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv, const CChainParams& chain_params, CConnman& connman) { uint8_t filter_type_ser; @@ -2279,7 +2273,7 @@ static void ProcessGetCFCheckPt(CNode& pfrom, CDataStream& vRecv, const CChainPa const CBlockIndex* stop_index; BlockFilterIndex* filter_index; - if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, /*start_height=*/0, stop_hash, + if (!PrepareBlockFilterRequest(peer, chain_params, filter_type, /*start_height=*/0, stop_hash, /*max_height_diff=*/std::numeric_limits<uint32_t>::max(), stop_index, filter_index)) { return; @@ -2300,12 +2294,12 @@ static void ProcessGetCFCheckPt(CNode& pfrom, CDataStream& vRecv, const CChainPa } } - CSerializedNetMsg msg = CNetMsgMaker(pfrom.GetSendVersion()) + CSerializedNetMsg msg = CNetMsgMaker(peer.GetSendVersion()) .Make(NetMsgType::CFCHECKPT, filter_type_ser, stop_index->GetBlockHash(), headers); - connman.PushMessage(&pfrom, std::move(msg)); + connman.PushMessage(&peer, std::move(msg)); } void ProcessMessage( @@ -2352,11 +2346,11 @@ void ProcessMessage( vRecv >> nVersion >> nServiceInt >> nTime >> addrMe; nSendVersion = std::min(nVersion, PROTOCOL_VERSION); nServices = ServiceFlags(nServiceInt); - if (!pfrom.fInbound) + if (!pfrom.IsInboundConn()) { connman.SetServices(pfrom.addr, nServices); } - if (!pfrom.fInbound && !pfrom.fFeeler && !pfrom.m_manual_connection && !HasAllDesirableServiceFlags(nServices)) + if (pfrom.ExpectServicesFromConn() && !HasAllDesirableServiceFlags(nServices)) { LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom.GetId(), nServices, GetDesirableServiceFlags(nServices)); pfrom.fDisconnect = true; @@ -2383,20 +2377,20 @@ void ProcessMessage( if (!vRecv.empty()) vRecv >> fRelay; // Disconnect if we connected to ourself - if (pfrom.fInbound && !connman.CheckIncomingNonce(nNonce)) + if (pfrom.IsInboundConn() && !connman.CheckIncomingNonce(nNonce)) { LogPrintf("connected to self at %s, disconnecting\n", pfrom.addr.ToString()); pfrom.fDisconnect = true; return; } - if (pfrom.fInbound && addrMe.IsRoutable()) + if (pfrom.IsInboundConn() && addrMe.IsRoutable()) { SeenLocal(addrMe); } // Be shy and don't send version until we hear - if (pfrom.fInbound) + if (pfrom.IsInboundConn()) PushNodeVersion(pfrom, connman, GetAdjustedTime()); if (nVersion >= WTXID_RELAY_VERSION) { @@ -2440,7 +2434,7 @@ void ProcessMessage( UpdatePreferredDownload(pfrom, State(pfrom.GetId())); } - if (!pfrom.fInbound && pfrom.IsAddrRelayPeer()) + if (!pfrom.IsInboundConn() && pfrom.IsAddrRelayPeer()) { // Advertise our address if (fListen && !::ChainstateActive().IsInitialBlockDownload()) @@ -2484,8 +2478,7 @@ void ProcessMessage( } // Feeler connections exist only to verify if address is online. - if (pfrom.fFeeler) { - assert(pfrom.fInbound == false); + if (pfrom.IsFeelerConn()) { pfrom.fDisconnect = true; } return; @@ -2505,7 +2498,7 @@ void ProcessMessage( { pfrom.SetRecvVersion(std::min(pfrom.nVersion.load(), PROTOCOL_VERSION)); - if (!pfrom.fInbound) { + if (!pfrom.IsInboundConn()) { // Mark this node as currently connected, so we update its timestamp later. LOCK(cs_main); State(pfrom.GetId())->fCurrentlyConnected = true; @@ -2614,7 +2607,7 @@ void ProcessMessage( connman.AddNewAddresses(vAddrOk, pfrom.addr, 2 * 60 * 60); if (vAddr.size() < 1000) pfrom.fGetAddr = false; - if (pfrom.fOneShot) + if (pfrom.IsAddrFetchConn()) pfrom.fDisconnect = true; return; } @@ -3509,7 +3502,7 @@ void ProcessMessage( // to users' AddrMan and later request them by sending getaddr messages. // Making nodes which are behind NAT and can only make outgoing connections ignore // the getaddr message mitigates the attack. - if (!pfrom.fInbound) { + if (!pfrom.IsInboundConn()) { LogPrint(BCLog::NET, "Ignoring \"getaddr\" from outbound connection. peer=%d\n", pfrom.GetId()); return; } @@ -3529,9 +3522,9 @@ void ProcessMessage( pfrom.vAddrToSend.clear(); std::vector<CAddress> vAddr; if (pfrom.HasPermission(PF_ADDR)) { - vAddr = connman.GetAddresses(); + vAddr = connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); } else { - vAddr = connman.GetAddresses(pfrom.addr.GetNetwork()); + vAddr = connman.GetAddresses(pfrom.addr.GetNetwork(), MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); } FastRandomContext insecure_rand; for (const CAddress &addr : vAddr) { @@ -3792,7 +3785,7 @@ bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode) return false; } - if (pnode.m_manual_connection) { + if (pnode.IsManualConn()) { // We never disconnect or discourage manual peers for bad behavior LogPrintf("Warning: not punishing manually connected peer %d!\n", peer_id); return false; @@ -3913,7 +3906,7 @@ void PeerLogicValidation::ConsiderEviction(CNode& pto, int64_t time_in_seconds) CNodeState &state = *State(pto.GetId()); const CNetMsgMaker msgMaker(pto.GetSendVersion()); - if (!state.m_chain_sync.m_protect && IsOutboundDisconnectionCandidate(pto) && state.fSyncStarted) { + if (!state.m_chain_sync.m_protect && pto.IsOutboundOrBlockRelayConn() && state.fSyncStarted) { // This is an outbound peer subject to disconnection if they don't // announce a block with as much work as the current tip within // CHAIN_SYNC_TIMEOUT + HEADERS_RESPONSE_TIME seconds (note: if @@ -3975,7 +3968,7 @@ void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds) AssertLockHeld(cs_main); // Ignore non-outbound peers, or nodes marked for disconnect already - if (!IsOutboundDisconnectionCandidate(*pnode) || pnode->fDisconnect) return; + if (!pnode->IsOutboundOrBlockRelayConn() || pnode->fDisconnect) return; CNodeState *state = State(pnode->GetId()); if (state == nullptr) return; // shouldn't be possible, but just in case // Don't evict our protected peers @@ -4153,7 +4146,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // Start block sync if (pindexBestHeader == nullptr) pindexBestHeader = ::ChainActive().Tip(); - bool fFetch = state.fPreferredDownload || (nPreferredDownload == 0 && !pto->fClient && !pto->fOneShot); // Download if this is a nice peer, or we have no nice peers and this one might do. + bool fFetch = state.fPreferredDownload || (nPreferredDownload == 0 && !pto->fClient && !pto->IsAddrFetchConn()); // Download if this is a nice peer, or we have no nice peers and this one might do. if (!state.fSyncStarted && !pto->fClient && !fImporting && !fReindex) { // Only actively request headers from a single peer, unless we're close to today. if ((nSyncStarted == 0 && fFetch) || pindexBestHeader->GetBlockTime() > GetAdjustedTime() - 24 * 60 * 60) { @@ -4338,7 +4331,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) bool fSendTrickle = pto->HasPermission(PF_NOBAN); if (pto->m_tx_relay->nNextInvSend < current_time) { fSendTrickle = true; - if (pto->fInbound) { + if (pto->IsInboundConn()) { pto->m_tx_relay->nNextInvSend = std::chrono::microseconds{connman->PoissonNextSendInbound(nNow, INVENTORY_BROADCAST_INTERVAL)}; } else { // Use half the delay for outbound peers, as there is less privacy concern for them. diff --git a/src/protocol.cpp b/src/protocol.cpp index 5a91acee0f..c989aa3902 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -217,6 +217,7 @@ static std::string serviceFlagToStr(size_t bit) case NODE_GETUTXO: return "GETUTXO"; case NODE_BLOOM: return "BLOOM"; case NODE_WITNESS: return "WITNESS"; + case NODE_COMPACT_FILTERS: return "COMPACT_FILTERS"; case NODE_NETWORK_LIMITED: return "NETWORK_LIMITED"; // Not using default, so we get warned when a case is missing } diff --git a/src/protocol.h b/src/protocol.h index 1d0adaae6e..2e6c767cdd 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -272,6 +272,9 @@ enum ServiceFlags : uint64_t { // NODE_WITNESS indicates that a node can be asked for blocks and transactions including // witness data. NODE_WITNESS = (1 << 3), + // NODE_COMPACT_FILTERS means the node will service basic block filter requests. + // See BIP157 and BIP158 for details on how this is implemented. + NODE_COMPACT_FILTERS = (1 << 6), // NODE_NETWORK_LIMITED means the same as NODE_NETWORK with the limitation of only // serving the last 288 (2 day) blocks // See BIP159 for details on how this is implemented. @@ -371,9 +374,10 @@ public: READWRITEAS(CService, obj); } - ServiceFlags nServices{NODE_NONE}; // disk and network only uint32_t nTime{TIME_INIT}; + + ServiceFlags nServices{NODE_NONE}; }; /** getdata message type flags */ @@ -394,7 +398,9 @@ enum GetDataMsg : uint32_t { MSG_CMPCT_BLOCK = 4, //!< Defined in BIP152 MSG_WITNESS_BLOCK = MSG_BLOCK | MSG_WITNESS_FLAG, //!< Defined in BIP144 MSG_WITNESS_TX = MSG_TX | MSG_WITNESS_FLAG, //!< Defined in BIP144 - MSG_FILTERED_WITNESS_BLOCK = MSG_FILTERED_BLOCK | MSG_WITNESS_FLAG, + // MSG_FILTERED_WITNESS_BLOCK is defined in BIP144 as reserved for future + // use and remains unused. + // MSG_FILTERED_WITNESS_BLOCK = MSG_FILTERED_BLOCK | MSG_WITNESS_FLAG, }; /** inv message data */ diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 4f1e0056be..f53fcc41f3 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -81,6 +81,7 @@ static void RegisterMetaTypes() qRegisterMetaType<std::function<void()>>("std::function<void()>"); qRegisterMetaType<QMessageBox::Icon>("QMessageBox::Icon"); + qRegisterMetaType<interfaces::BlockAndHeaderTipInfo>("interfaces::BlockAndHeaderTipInfo"); } static QString GetLangTerritory() @@ -164,8 +165,9 @@ void BitcoinCore::initialize() { qDebug() << __func__ << ": Running initialization in thread"; util::ThreadRename("qt-init"); - bool rv = m_node.appInitMain(); - Q_EMIT initializeResult(rv); + interfaces::BlockAndHeaderTipInfo tip_info; + bool rv = m_node.appInitMain(&tip_info); + Q_EMIT initializeResult(rv, tip_info); } catch (const std::exception& e) { handleRunawayException(&e); } catch (...) { @@ -342,7 +344,7 @@ void BitcoinApplication::requestShutdown() Q_EMIT requestedShutdown(); } -void BitcoinApplication::initializeResult(bool success) +void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info) { qDebug() << __func__ << ": Initialization result: " << success; // Set exit result. @@ -352,7 +354,7 @@ void BitcoinApplication::initializeResult(bool success) // Log this only after AppInitMain finishes, as then logging setup is guaranteed complete qInfo() << "Platform customization:" << platformStyle->getName(); clientModel = new ClientModel(m_node, optionsModel); - window->setClientModel(clientModel); + window->setClientModel(clientModel, &tip_info); #ifdef ENABLE_WALLET if (WalletModel::isWalletEnabled()) { m_wallet_controller = new WalletController(*clientModel, platformStyle, this); diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index 077a37fde5..20c6dfc047 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -12,6 +12,8 @@ #include <QApplication> #include <memory> +#include <interfaces/node.h> + class BitcoinGUI; class ClientModel; class NetworkStyle; @@ -21,10 +23,6 @@ class PlatformStyle; class WalletController; class WalletModel; -namespace interfaces { -class Handler; -class Node; -} // namespace interfaces /** Class encapsulating Bitcoin Core startup and shutdown. * Allows running startup and shutdown in a different thread from the UI thread. @@ -40,7 +38,7 @@ public Q_SLOTS: void shutdown(); Q_SIGNALS: - void initializeResult(bool success); + void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info); void shutdownResult(); void runawayException(const QString &message); @@ -91,7 +89,7 @@ public: void setupPlatformStyle(); public Q_SLOTS: - void initializeResult(bool success); + void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info); void shutdownResult(); /// Handle runaway exceptions. Shows a message box with the problem and quits the program. void handleRunawayException(const QString &message); diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index ebcc04a5eb..56adbf249a 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -574,7 +574,7 @@ void BitcoinGUI::createToolBars() } } -void BitcoinGUI::setClientModel(ClientModel *_clientModel) +void BitcoinGUI::setClientModel(ClientModel *_clientModel, interfaces::BlockAndHeaderTipInfo* tip_info) { this->clientModel = _clientModel; if(_clientModel) @@ -588,8 +588,8 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel) connect(_clientModel, &ClientModel::numConnectionsChanged, this, &BitcoinGUI::setNumConnections); connect(_clientModel, &ClientModel::networkActiveChanged, this, &BitcoinGUI::setNetworkActive); - modalOverlay->setKnownBestHeight(_clientModel->getHeaderTipHeight(), QDateTime::fromTime_t(_clientModel->getHeaderTipTime())); - setNumBlocks(m_node.getNumBlocks(), QDateTime::fromTime_t(m_node.getLastBlockTime()), m_node.getVerificationProgress(), false, SynchronizationState::INIT_DOWNLOAD); + modalOverlay->setKnownBestHeight(tip_info->header_height, QDateTime::fromTime_t(tip_info->header_time)); + setNumBlocks(tip_info->block_height, QDateTime::fromTime_t(tip_info->block_time), tip_info->verification_progress, false, SynchronizationState::INIT_DOWNLOAD); connect(_clientModel, &ClientModel::numBlocksChanged, this, &BitcoinGUI::setNumBlocks); // Receive and report messages from client model @@ -600,7 +600,7 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel) // Show progress dialog connect(_clientModel, &ClientModel::showProgress, this, &BitcoinGUI::showProgress); - rpcConsole->setClientModel(_clientModel); + rpcConsole->setClientModel(_clientModel, tip_info->block_height, tip_info->block_time, tip_info->verification_progress); updateProxyIcon(); diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index 697e83e772..4c55f28693 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -43,6 +43,7 @@ enum class SynchronizationState; namespace interfaces { class Handler; class Node; +struct BlockAndHeaderTipInfo; } QT_BEGIN_NAMESPACE @@ -75,7 +76,7 @@ public: /** Set the client model. The client model represents the part of the core that communicates with the P2P network, and is wallet-agnostic. */ - void setClientModel(ClientModel *clientModel); + void setClientModel(ClientModel *clientModel = nullptr, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr); #ifdef ENABLE_WALLET void setWalletController(WalletController* wallet_controller); #endif diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 821a337a62..a14fae6460 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -556,7 +556,7 @@ bool RPCConsole::eventFilter(QObject* obj, QEvent *event) return QWidget::eventFilter(obj, event); } -void RPCConsole::setClientModel(ClientModel *model) +void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_t bestblock_date, double verification_progress) { clientModel = model; @@ -576,13 +576,13 @@ void RPCConsole::setClientModel(ClientModel *model) setNumConnections(model->getNumConnections()); connect(model, &ClientModel::numConnectionsChanged, this, &RPCConsole::setNumConnections); - interfaces::Node& node = clientModel->node(); - setNumBlocks(node.getNumBlocks(), QDateTime::fromTime_t(node.getLastBlockTime()), node.getVerificationProgress(), false); + setNumBlocks(bestblock_height, QDateTime::fromTime_t(bestblock_date), verification_progress, false); connect(model, &ClientModel::numBlocksChanged, this, &RPCConsole::setNumBlocks); updateNetworkState(); connect(model, &ClientModel::networkActiveChanged, this, &RPCConsole::setNetworkActive); + interfaces::Node& node = clientModel->node(); updateTrafficStats(node.getTotalBytesRecv(), node.getTotalBytesSent()); connect(model, &ClientModel::bytesChanged, this, &RPCConsole::updateTrafficStats); diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index de8e37cca2..280c5bd71a 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -46,7 +46,7 @@ public: return RPCParseCommandLine(&node, strResult, strCommand, true, pstrFilteredOut, wallet_model); } - void setClientModel(ClientModel *model); + void setClientModel(ClientModel *model = nullptr, int bestblock_height = 0, int64_t bestblock_date = 0, double verification_progress = 0.0); void addWallet(WalletModel * const walletModel); void removeWallet(WalletModel* const walletModel); diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index b880a99baf..0b5c341548 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -67,6 +67,7 @@ void AppTests::appTests() return GetDataDir() / "blocks"; }()); + qRegisterMetaType<interfaces::BlockAndHeaderTipInfo>("interfaces::BlockAndHeaderTipInfo"); m_app.parameterSetup(); m_app.createOptionsModel(true /* reset settings */); QScopedPointer<const NetworkStyle> style(NetworkStyle::instantiate(Params().NetworkIDString())); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index f27373b57c..868ff88d08 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2407,7 +2407,7 @@ static const CRPCCommand commands[] = { "hidden", "dumptxoutset", &dumptxoutset, {"path"} }, }; // clang-format on - - for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) - t.appendCommand(commands[vcidx].name, &commands[vcidx]); + for (const auto& c : commands) { + t.appendCommand(c.name, &c); + } } diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 66ace7263a..4d08671bd2 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -151,6 +151,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getmempoolancestors", 1, "verbose" }, { "getmempooldescendants", 1, "verbose" }, { "bumpfee", 1, "options" }, + { "psbtbumpfee", 1, "options" }, { "logging", 0, "include" }, { "logging", 1, "exclude" }, { "disconnectnode", 1, "nodeid" }, @@ -172,7 +173,11 @@ static const CRPCConvertParam vRPCConvertParams[] = { "createwallet", 2, "blank"}, { "createwallet", 4, "avoid_reuse"}, { "createwallet", 5, "descriptors"}, + { "createwallet", 6, "load_on_startup"}, + { "loadwallet", 1, "load_on_startup"}, + { "unloadwallet", 1, "load_on_startup"}, { "getnodeaddresses", 0, "count"}, + { "addpeeraddress", 1, "port"}, { "stop", 0, "wait" }, }; // clang-format on diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index fee6a893eb..76aa9dbfc1 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -236,6 +236,17 @@ static UniValue generatetodescriptor(const JSONRPCRequest& request) return generateBlocks(chainman, mempool, coinbase_script, num_blocks, max_tries); } +static UniValue generate(const JSONRPCRequest& request) +{ + const std::string help_str{"generate ( nblocks maxtries ) has been replaced by the -generate cli option. Refer to -help for more information."}; + + if (request.fHelp) { + throw std::runtime_error(help_str); + } else { + throw JSONRPCError(RPC_METHOD_NOT_FOUND, help_str); + } +} + static UniValue generatetoaddress(const JSONRPCRequest& request) { RPCHelpMan{"generatetoaddress", @@ -1019,7 +1030,7 @@ static UniValue estimatesmartfee(const JSONRPCRequest& request) RPCResult::Type::OBJ, "", "", { {RPCResult::Type::NUM, "feerate", /* optional */ true, "estimate fee rate in " + CURRENCY_UNIT + "/kB (only present if no errors were encountered)"}, - {RPCResult::Type::ARR, "errors", "Errors encountered during processing", + {RPCResult::Type::ARR, "errors", /* optional */ true, "Errors encountered during processing (if there are any)", { {RPCResult::Type::STR, "", "error"}, }}, @@ -1098,7 +1109,7 @@ static UniValue estimaterawfee(const JSONRPCRequest& request) { {RPCResult::Type::ELISION, "", ""}, }}, - {RPCResult::Type::ARR, "errors", /* optional */ true, "Errors encountered during processing", + {RPCResult::Type::ARR, "errors", /* optional */ true, "Errors encountered during processing (if there are any)", { {RPCResult::Type::STR, "error", ""}, }}, @@ -1198,9 +1209,10 @@ static const CRPCCommand commands[] = { "util", "estimatesmartfee", &estimatesmartfee, {"conf_target", "estimate_mode"} }, { "hidden", "estimaterawfee", &estimaterawfee, {"conf_target", "threshold"} }, + { "hidden", "generate", &generate, {} }, }; // clang-format on - - for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) - t.appendCommand(commands[vcidx].name, &commands[vcidx]); + for (const auto& c : commands) { + t.appendCommand(c.name, &c); + } } diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 53d38f4e11..ff31bee1e3 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -27,9 +27,9 @@ #include <univalue.h> -static UniValue validateaddress(const JSONRPCRequest& request) +static RPCHelpMan validateaddress() { - RPCHelpMan{"validateaddress", + return RPCHelpMan{"validateaddress", "\nReturn information about the given bitcoin address.\n", { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address to validate"}, @@ -50,8 +50,8 @@ static UniValue validateaddress(const JSONRPCRequest& request) HelpExampleCli("validateaddress", "\"" + EXAMPLE_ADDRESS[0] + "\"") + HelpExampleRpc("validateaddress", "\"" + EXAMPLE_ADDRESS[0] + "\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ CTxDestination dest = DecodeDestination(request.params[0].get_str()); bool isValid = IsValidDestination(dest); @@ -69,11 +69,13 @@ static UniValue validateaddress(const JSONRPCRequest& request) ret.pushKVs(detail); } return ret; +}, + }; } -static UniValue createmultisig(const JSONRPCRequest& request) +static RPCHelpMan createmultisig() { - RPCHelpMan{"createmultisig", + return RPCHelpMan{"createmultisig", "\nCreates a multi-signature address with n signature of m keys required.\n" "It returns a json object with the address and redeemScript.\n", { @@ -98,8 +100,8 @@ static UniValue createmultisig(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("createmultisig", "2, \"[\\\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\\\",\\\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\\\"]\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ int required = request.params[0].get_int(); // Get the public keys @@ -135,11 +137,13 @@ static UniValue createmultisig(const JSONRPCRequest& request) result.pushKV("descriptor", descriptor->ToString()); return result; +}, + }; } -UniValue getdescriptorinfo(const JSONRPCRequest& request) +static RPCHelpMan getdescriptorinfo() { - RPCHelpMan{"getdescriptorinfo", + return RPCHelpMan{"getdescriptorinfo", {"\nAnalyses a descriptor.\n"}, { {"descriptor", RPCArg::Type::STR, RPCArg::Optional::NO, "The descriptor."}, @@ -157,8 +161,9 @@ UniValue getdescriptorinfo(const JSONRPCRequest& request) RPCExamples{ "Analyse a descriptor\n" + HelpExampleCli("getdescriptorinfo", "\"wpkh([d34db33f/84h/0h/0h]0279be667ef9dcbbac55a06295Ce870b07029Bfcdb2dce28d959f2815b16f81798)\"") - }}.Check(request); - + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ RPCTypeCheck(request.params, {UniValue::VSTR}); FlatSigningProvider provider; @@ -175,11 +180,13 @@ UniValue getdescriptorinfo(const JSONRPCRequest& request) result.pushKV("issolvable", desc->IsSolvable()); result.pushKV("hasprivatekeys", provider.keys.size() > 0); return result; +}, + }; } -UniValue deriveaddresses(const JSONRPCRequest& request) +static RPCHelpMan deriveaddresses() { - RPCHelpMan{"deriveaddresses", + return RPCHelpMan{"deriveaddresses", {"\nDerives one or more addresses corresponding to an output descriptor.\n" "Examples of output descriptors are:\n" " pkh(<pubkey>) P2PKH outputs for the given pubkey\n" @@ -202,8 +209,9 @@ UniValue deriveaddresses(const JSONRPCRequest& request) RPCExamples{ "First three native segwit receive addresses\n" + HelpExampleCli("deriveaddresses", "\"wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#cjjspncu\" \"[0,2]\"") - }}.Check(request); - + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ RPCTypeCheck(request.params, {UniValue::VSTR, UniValueType()}); // Range argument is checked later const std::string desc_str = request.params[0].get_str(); @@ -254,11 +262,13 @@ UniValue deriveaddresses(const JSONRPCRequest& request) } return addresses; +}, + }; } -static UniValue verifymessage(const JSONRPCRequest& request) +static RPCHelpMan verifymessage() { - RPCHelpMan{"verifymessage", + return RPCHelpMan{"verifymessage", "\nVerify a signed message\n", { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address to use for the signature."}, @@ -278,8 +288,8 @@ static UniValue verifymessage(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("verifymessage", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XX\", \"signature\", \"my message\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ LOCK(cs_main); std::string strAddress = request.params[0].get_str(); @@ -301,11 +311,13 @@ static UniValue verifymessage(const JSONRPCRequest& request) } return false; +}, + }; } -static UniValue signmessagewithprivkey(const JSONRPCRequest& request) +static RPCHelpMan signmessagewithprivkey() { - RPCHelpMan{"signmessagewithprivkey", + return RPCHelpMan{"signmessagewithprivkey", "\nSign a message with the private key of an address\n", { {"privkey", RPCArg::Type::STR, RPCArg::Optional::NO, "The private key to sign the message with."}, @@ -322,8 +334,8 @@ static UniValue signmessagewithprivkey(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("signmessagewithprivkey", "\"privkey\", \"my message\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::string strPrivkey = request.params[0].get_str(); std::string strMessage = request.params[1].get_str(); @@ -339,11 +351,13 @@ static UniValue signmessagewithprivkey(const JSONRPCRequest& request) } return signature; +}, + }; } -static UniValue setmocktime(const JSONRPCRequest& request) +static RPCHelpMan setmocktime() { - RPCHelpMan{"setmocktime", + return RPCHelpMan{"setmocktime", "\nSet the local time to given timestamp (-regtest only)\n", { {"timestamp", RPCArg::Type::NUM, RPCArg::Optional::NO, UNIX_EPOCH_TIME + "\n" @@ -351,8 +365,8 @@ static UniValue setmocktime(const JSONRPCRequest& request) }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{""}, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ if (!Params().IsMockableChain()) { throw std::runtime_error("setmocktime is for regression testing (-regtest mode) only"); } @@ -374,19 +388,21 @@ static UniValue setmocktime(const JSONRPCRequest& request) } return NullUniValue; +}, + }; } -static UniValue mockscheduler(const JSONRPCRequest& request) +static RPCHelpMan mockscheduler() { - RPCHelpMan{"mockscheduler", + return RPCHelpMan{"mockscheduler", "\nBump the scheduler into the future (-regtest only)\n", { {"delta_time", RPCArg::Type::NUM, RPCArg::Optional::NO, "Number of seconds to forward the scheduler into the future." }, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{""}, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ if (!Params().IsMockableChain()) { throw std::runtime_error("mockscheduler is for regression testing (-regtest mode) only"); } @@ -405,6 +421,8 @@ static UniValue mockscheduler(const JSONRPCRequest& request) node.scheduler->MockForward(std::chrono::seconds(delta_seconds)); return NullUniValue; +}, + }; } static UniValue RPCLockedMemoryInfo() @@ -439,12 +457,12 @@ static std::string RPCMallocInfo() } #endif -static UniValue getmemoryinfo(const JSONRPCRequest& request) +static RPCHelpMan getmemoryinfo() { /* Please, avoid using the word "pool" here in the RPC interface or help, * as users will undoubtedly confuse it with the other "memory pool" */ - RPCHelpMan{"getmemoryinfo", + return RPCHelpMan{"getmemoryinfo", "Returns an object containing information about memory usage.\n", { {"mode", RPCArg::Type::STR, /* default */ "\"stats\"", "determines what kind of information is returned.\n" @@ -474,8 +492,8 @@ static UniValue getmemoryinfo(const JSONRPCRequest& request) HelpExampleCli("getmemoryinfo", "") + HelpExampleRpc("getmemoryinfo", "") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::string mode = request.params[0].isNull() ? "stats" : request.params[0].get_str(); if (mode == "stats") { UniValue obj(UniValue::VOBJ); @@ -490,6 +508,8 @@ static UniValue getmemoryinfo(const JSONRPCRequest& request) } else { throw JSONRPCError(RPC_INVALID_PARAMETER, "unknown mode " + mode); } +}, + }; } static void EnableOrDisableLogCategories(UniValue cats, bool enable) { @@ -510,9 +530,9 @@ static void EnableOrDisableLogCategories(UniValue cats, bool enable) { } } -UniValue logging(const JSONRPCRequest& request) +static RPCHelpMan logging() { - RPCHelpMan{"logging", + return RPCHelpMan{"logging", "Gets and sets the logging configuration.\n" "When called without an argument, returns the list of categories with status that are currently being debug logged or not.\n" "When called with arguments, adds or removes categories from debug logging and return the lists above.\n" @@ -543,8 +563,8 @@ UniValue logging(const JSONRPCRequest& request) HelpExampleCli("logging", "\"[\\\"all\\\"]\" \"[\\\"http\\\"]\"") + HelpExampleRpc("logging", "[\"all\"], [\"libevent\"]") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ uint32_t original_log_categories = LogInstance().GetCategoryMask(); if (request.params[0].isArray()) { EnableOrDisableLogCategories(request.params[0], true); @@ -575,28 +595,47 @@ UniValue logging(const JSONRPCRequest& request) } return result; +}, + }; } -static UniValue echo(const JSONRPCRequest& request) +static RPCHelpMan echo(const std::string& name) { - if (request.fHelp) - throw std::runtime_error( - RPCHelpMan{"echo|echojson ...", + return RPCHelpMan{name, "\nSimply echo back the input arguments. This command is for testing.\n" - "\nIt will return an internal bug report when exactly 100 arguments are passed.\n" + "\nIt will return an internal bug report when arg9='trigger_internal_bug' is passed.\n" "\nThe difference between echo and echojson is that echojson has argument conversion enabled in the client-side table in " "bitcoin-cli and the GUI. There is no server-side difference.", - {}, + { + {"arg0", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, + {"arg1", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, + {"arg2", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, + {"arg3", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, + {"arg4", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, + {"arg5", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, + {"arg6", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, + {"arg7", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, + {"arg8", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, + {"arg9", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, + }, RPCResult{RPCResult::Type::NONE, "", "Returns whatever was passed in"}, RPCExamples{""}, - }.ToString() - ); + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ + if (request.fHelp) throw std::runtime_error(self.ToString()); - CHECK_NONFATAL(request.params.size() != 100); + if (request.params[9].isStr()) { + CHECK_NONFATAL(request.params[9].get_str() != "trigger_internal_bug"); + } return request.params; +}, + }; } +static RPCHelpMan echo() { return echo("echo"); } +static RPCHelpMan echojson() { return echo("echojson"); } + void RegisterMiscRPCCommands(CRPCTable &t) { // clang-format off @@ -616,10 +655,10 @@ static const CRPCCommand commands[] = { "hidden", "setmocktime", &setmocktime, {"timestamp"}}, { "hidden", "mockscheduler", &mockscheduler, {"delta_time"}}, { "hidden", "echo", &echo, {"arg0","arg1","arg2","arg3","arg4","arg5","arg6","arg7","arg8","arg9"}}, - { "hidden", "echojson", &echo, {"arg0","arg1","arg2","arg3","arg4","arg5","arg6","arg7","arg8","arg9"}}, + { "hidden", "echojson", &echojson, {"arg0","arg1","arg2","arg3","arg4","arg5","arg6","arg7","arg8","arg9"}}, }; // clang-format on - - for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) - t.appendCommand(commands[vcidx].name, &commands[vcidx]); + for (const auto& c : commands) { + t.appendCommand(c.name, &c); + } } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 9981ea35df..9bd7c15992 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -264,7 +264,7 @@ static UniValue addnode(const JSONRPCRequest& request) if (strCommand == "onetry") { CAddress addr; - node.connman->OpenNetworkConnection(addr, false, nullptr, strNode.c_str(), false, false, true); + node.connman->OpenNetworkConnection(addr, false, nullptr, strNode.c_str(), ConnectionType::MANUAL); return NullUniValue; } @@ -276,7 +276,7 @@ static UniValue addnode(const JSONRPCRequest& request) else if(strCommand == "remove") { if(!node.connman->RemoveAddedNode(strNode)) - throw JSONRPCError(RPC_CLIENT_NODE_NOT_ADDED, "Error: Node has not been added."); + throw JSONRPCError(RPC_CLIENT_NODE_NOT_ADDED, "Error: Node could not be removed. It has not been added previously."); } return NullUniValue; @@ -727,7 +727,7 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request) RPCHelpMan{"getnodeaddresses", "\nReturn known addresses which can potentially be used to find new nodes in the network\n", { - {"count", RPCArg::Type::NUM, /* default */ "1", "How many addresses to return. Limited to the smaller of " + ToString(ADDRMAN_GETADDR_MAX) + " or " + ToString(ADDRMAN_GETADDR_MAX_PCT) + "% of all known addresses."}, + {"count", RPCArg::Type::NUM, /* default */ "1", "The maximum number of addresses to return. Specify 0 to return all known addresses."}, }, RPCResult{ RPCResult::Type::ARR, "", "", @@ -754,18 +754,16 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request) int count = 1; if (!request.params[0].isNull()) { count = request.params[0].get_int(); - if (count <= 0) { + if (count < 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Address count out of range"); } } // returns a shuffled list of CAddress - std::vector<CAddress> vAddr = node.connman->GetAddresses(); + std::vector<CAddress> vAddr = node.connman->GetAddresses(count, /* max_pct */ 0); UniValue ret(UniValue::VARR); - int address_return_count = std::min<int>(count, vAddr.size()); - for (int i = 0; i < address_return_count; ++i) { + for (const CAddress& addr : vAddr) { UniValue obj(UniValue::VOBJ); - const CAddress& addr = vAddr[i]; obj.pushKV("time", (int)addr.nTime); obj.pushKV("services", (uint64_t)addr.nServices); obj.pushKV("address", addr.ToStringIP()); @@ -775,6 +773,54 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request) return ret; } +static UniValue addpeeraddress(const JSONRPCRequest& request) +{ + RPCHelpMan{"addpeeraddress", + "\nAdd the address of a potential peer to the address manager. This RPC is for testing only.\n", + { + {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address of the peer"}, + {"port", RPCArg::Type::NUM, RPCArg::Optional::NO, "The port of the peer"}, + }, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::BOOL, "success", "whether the peer address was successfully added to the address manager"}, + }, + }, + RPCExamples{ + HelpExampleCli("addpeeraddress", "\"1.2.3.4\" 8333") + + HelpExampleRpc("addpeeraddress", "\"1.2.3.4\", 8333") + }, + }.Check(request); + + NodeContext& node = EnsureNodeContext(request.context); + if (!node.connman) { + throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + } + + UniValue obj(UniValue::VOBJ); + + std::string addr_string = request.params[0].get_str(); + uint16_t port = request.params[1].get_int(); + + CNetAddr net_addr; + if (!LookupHost(addr_string, net_addr, false)) { + obj.pushKV("success", false); + return obj; + } + CAddress address = CAddress({net_addr, port}, ServiceFlags(NODE_NETWORK|NODE_WITNESS)); + address.nTime = GetAdjustedTime(); + // The source address is set equal to the address. This is equivalent to the peer + // announcing itself. + if (!node.connman->AddNewAddresses({address}, address)) { + obj.pushKV("success", false); + return obj; + } + + obj.pushKV("success", true); + return obj; +} + void RegisterNetRPCCommands(CRPCTable &t) { // clang-format off @@ -794,9 +840,10 @@ static const CRPCCommand commands[] = { "network", "clearbanned", &clearbanned, {} }, { "network", "setnetworkactive", &setnetworkactive, {"state"} }, { "network", "getnodeaddresses", &getnodeaddresses, {"count"} }, + { "hidden", "addpeeraddress", &addpeeraddress, {"address", "port"} }, }; // clang-format on - - for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) - t.appendCommand(commands[vcidx].name, &commands[vcidx]); + for (const auto& c : commands) { + t.appendCommand(c.name, &c); + } } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 9b6ef15785..abc8168c55 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -744,7 +744,7 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request) { {RPCResult::Type::STR_HEX, "hex", "The hex-encoded raw transaction with signature(s)"}, {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, - {RPCResult::Type::ARR, "errors", "Script verification errors (if there are any)", + {RPCResult::Type::ARR, "errors", /* optional */ true, "Script verification errors (if there are any)", { {RPCResult::Type::OBJ, "", "", { @@ -1722,7 +1722,7 @@ UniValue analyzepsbt(const JSONRPCRequest& request) {RPCResult::Type::STR_AMOUNT, "estimated_feerate", /* optional */ true, "Estimated feerate of the final signed transaction in " + CURRENCY_UNIT + "/kB. Shown only if all UTXO slots in the PSBT have been filled"}, {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The transaction fee paid. Shown only if all UTXO slots in the PSBT have been filled"}, {RPCResult::Type::STR, "next", "Role of the next person that this psbt needs to go to"}, - {RPCResult::Type::STR, "error", "Error message if there is one"}, + {RPCResult::Type::STR, "error", /* optional */ true, "Error message (if there is one)"}, } }, RPCExamples { @@ -1821,7 +1821,7 @@ static const CRPCCommand commands[] = { "blockchain", "verifytxoutproof", &verifytxoutproof, {"proof"} }, }; // clang-format on - - for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) - t.appendCommand(commands[vcidx].name, &commands[vcidx]); + for (const auto& c : commands) { + t.appendCommand(c.name, &c); + } } diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index e5f6b1b9f1..9c8e7fe04a 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -256,13 +256,8 @@ static const CRPCCommand vRPCCommands[] = CRPCTable::CRPCTable() { - unsigned int vcidx; - for (vcidx = 0; vcidx < (sizeof(vRPCCommands) / sizeof(vRPCCommands[0])); vcidx++) - { - const CRPCCommand *pcmd; - - pcmd = &vRPCCommands[vcidx]; - mapCommands[pcmd->name].push_back(pcmd); + for (const auto& c : vRPCCommands) { + appendCommand(c.name, &c); } } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 073a7688a9..40dfdb587e 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -504,7 +504,7 @@ std::string RPCHelpMan::ToString() const ret += m_name; bool was_optional{false}; for (const auto& arg : m_args) { - if (arg.m_hidden) continue; + if (arg.m_hidden) break; // Any arg that follows is also hidden const bool optional = arg.IsOptional(); ret += " "; if (optional) { @@ -526,7 +526,7 @@ std::string RPCHelpMan::ToString() const Sections sections; for (size_t i{0}; i < m_args.size(); ++i) { const auto& arg = m_args.at(i); - if (arg.m_hidden) continue; + if (arg.m_hidden) break; // Any arg that follows is also hidden if (i == 0) ret += "\nArguments:\n"; diff --git a/src/script/sign.cpp b/src/script/sign.cpp index f425215549..9b3f94f14d 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -186,6 +186,8 @@ static CScript PushAll(const std::vector<valtype>& values) result << OP_0; } else if (v.size() == 1 && v[0] >= 1 && v[0] <= 16) { result << CScript::EncodeOP_N(v[0]); + } else if (v.size() == 1 && v[0] == 0x81) { + result << OP_1NEGATE; } else { result << v; } diff --git a/src/script/standard.cpp b/src/script/standard.cpp index 3a4882f280..96a3d311a6 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -313,18 +313,6 @@ CScript GetScriptForMultisig(int nRequired, const std::vector<CPubKey>& keys) return script; } -CScript GetScriptForWitness(const CScript& redeemscript) -{ - std::vector<std::vector<unsigned char> > vSolutions; - TxoutType typ = Solver(redeemscript, vSolutions); - if (typ == TxoutType::PUBKEY) { - return GetScriptForDestination(WitnessV0KeyHash(Hash160(vSolutions[0]))); - } else if (typ == TxoutType::PUBKEYHASH) { - return GetScriptForDestination(WitnessV0KeyHash(uint160{vSolutions[0]})); - } - return GetScriptForDestination(WitnessV0ScriptHash(redeemscript)); -} - bool IsValidDestination(const CTxDestination& dest) { return dest.which() != 0; } diff --git a/src/script/standard.h b/src/script/standard.h index 992e37675f..6dbcd04968 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -263,14 +263,4 @@ CScript GetScriptForRawPubKey(const CPubKey& pubkey); /** Generate a multisig script. */ CScript GetScriptForMultisig(int nRequired, const std::vector<CPubKey>& keys); -/** - * Generate a pay-to-witness script for the given redeem script. If the redeem - * script is P2PK or P2PKH, this returns a P2WPKH script, otherwise it returns a - * P2WSH script. - * - * TODO: replace calls to GetScriptForWitness with GetScriptForDestination using - * the various witness-specific CTxDestination subtypes. - */ -CScript GetScriptForWitness(const CScript& redeemscript); - #endif // BITCOIN_SCRIPT_STANDARD_H diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index bc6b38c682..25fdd64568 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -392,7 +392,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) // Test: Sanity check, GetAddr should never return anything if addrman // is empty. BOOST_CHECK_EQUAL(addrman.size(), 0U); - std::vector<CAddress> vAddr1 = addrman.GetAddr(); + std::vector<CAddress> vAddr1 = addrman.GetAddr(/* max_addresses */ 0, /* max_pct */0); BOOST_CHECK_EQUAL(vAddr1.size(), 0U); CAddress addr1 = CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE); @@ -415,13 +415,15 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) BOOST_CHECK(addrman.Add(addr4, source2)); BOOST_CHECK(addrman.Add(addr5, source1)); - // GetAddr returns 23% of addresses, 23% of 5 is 1 rounded down. - BOOST_CHECK_EQUAL(addrman.GetAddr().size(), 1U); + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U); + // Net processing asks for 23% of addresses. 23% of 5 is 1 rounded down. + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23).size(), 1U); // Test: Ensure GetAddr works with new and tried addresses. addrman.Good(CAddress(addr1, NODE_NONE)); addrman.Good(CAddress(addr2, NODE_NONE)); - BOOST_CHECK_EQUAL(addrman.GetAddr().size(), 1U); + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U); + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23).size(), 1U); // Test: Ensure GetAddr still returns 23% when addrman has many addrs. for (unsigned int i = 1; i < (8 * 256); i++) { @@ -436,7 +438,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) if (i % 8 == 0) addrman.Good(addr); } - std::vector<CAddress> vAddr = addrman.GetAddr(); + std::vector<CAddress> vAddr = addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23); size_t percent23 = (addrman.size() * 23) / 100; BOOST_CHECK_EQUAL(vAddr.size(), percent23); diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index b1a635d9da..0115803e58 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -84,7 +84,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); - CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", /*fInboundIn=*/ false); + CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", ConnectionType::OUTBOUND); dummyNode1.SetSendVersion(PROTOCOL_VERSION); peerLogic->InitializeNode(&dummyNode1); @@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerLogicValidation &peerLogic, CConnmanTest* connman) { CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE); - vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", /*fInboundIn=*/ false)); + vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", ConnectionType::OUTBOUND)); CNode &node = *vNodes.back(); node.SetSendVersion(PROTOCOL_VERSION); @@ -227,7 +227,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) banman->ClearBanned(); CAddress addr1(ip(0xa0b0c001), NODE_NONE); - CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", true); + CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", ConnectionType::INBOUND); dummyNode1.SetSendVersion(PROTOCOL_VERSION); peerLogic->InitializeNode(&dummyNode1); dummyNode1.nVersion = 1; @@ -244,7 +244,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) BOOST_CHECK(!banman->IsDiscouraged(ip(0xa0b0c001|0x0000ff00))); // Different IP, not discouraged CAddress addr2(ip(0xa0b0c002), NODE_NONE); - CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, CAddress(), "", true); + CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, CAddress(), "", ConnectionType::INBOUND); dummyNode2.SetSendVersion(PROTOCOL_VERSION); peerLogic->InitializeNode(&dummyNode2); dummyNode2.nVersion = 1; @@ -286,7 +286,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) SetMockTime(nStartTime); // Overrides future calls to GetTime() CAddress addr(ip(0xa0b0c001), NODE_NONE); - CNode dummyNode(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr, 4, 4, CAddress(), "", true); + CNode dummyNode(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr, 4, 4, CAddress(), "", ConnectionType::INBOUND); dummyNode.SetSendVersion(PROTOCOL_VERSION); peerLogic->InitializeNode(&dummyNode); dummyNode.nVersion = 1; diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 9e40d5cd55..677b87a47a 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -80,7 +80,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) return; } CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(), SER_NETWORK, PROTOCOL_VERSION}; - CNode& p2p_node = *MakeUnique<CNode>(0, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_BLOOM), 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, false).release(); + CNode& p2p_node = *MakeUnique<CNode>(0, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_BLOOM), 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND).release(); p2p_node.fSuccessfullyConnected = true; p2p_node.nVersion = PROTOCOL_VERSION; p2p_node.SetSendVersion(PROTOCOL_VERSION); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 91ebf9fb1b..ef427442e9 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -44,9 +44,8 @@ void test_one_input(const std::vector<uint8_t>& buffer) const auto num_peers_to_add = fuzzed_data_provider.ConsumeIntegralInRange(1, 3); for (int i = 0; i < num_peers_to_add; ++i) { const ServiceFlags service_flags = ServiceFlags(fuzzed_data_provider.ConsumeIntegral<uint64_t>()); - const bool inbound{fuzzed_data_provider.ConsumeBool()}; - const bool block_relay_only{fuzzed_data_provider.ConsumeBool()}; - peers.push_back(MakeUnique<CNode>(i, service_flags, 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, inbound, block_relay_only).release()); + const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH}); + peers.push_back(MakeUnique<CNode>(i, service_flags, 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, conn_type).release()); CNode& p2p_node = *peers.back(); p2p_node.fSuccessfullyConnected = true; diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp index 85aac6ac7a..4274fa4351 100644 --- a/src/test/fuzz/script.cpp +++ b/src/test/fuzz/script.cpp @@ -63,8 +63,6 @@ void test_one_input(const std::vector<uint8_t>& buffer) int required_ret; (void)ExtractDestinations(script, type_ret, addresses, required_ret); - (void)GetScriptForWitness(script); - const FlatSigningProvider signing_provider; (void)InferDescriptor(script, signing_provider); diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index ab42be21bd..317000c771 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -180,17 +180,12 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) CAddress addr = CAddress(CService(ipv4Addr, 7777), NODE_NETWORK); std::string pszDest; - bool fInboundIn = false; - // Test that fFeeler is false by default. - std::unique_ptr<CNode> pnode1 = MakeUnique<CNode>(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, CAddress(), pszDest, fInboundIn); - BOOST_CHECK(pnode1->fInbound == false); - BOOST_CHECK(pnode1->fFeeler == false); + std::unique_ptr<CNode> pnode1 = MakeUnique<CNode>(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, CAddress(), pszDest, ConnectionType::OUTBOUND); + BOOST_CHECK(pnode1->IsInboundConn() == false); - fInboundIn = true; - std::unique_ptr<CNode> pnode2 = MakeUnique<CNode>(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, CAddress(), pszDest, fInboundIn); - BOOST_CHECK(pnode2->fInbound == true); - BOOST_CHECK(pnode2->fFeeler == false); + std::unique_ptr<CNode> pnode2 = MakeUnique<CNode>(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, CAddress(), pszDest, ConnectionType::INBOUND); + BOOST_CHECK(pnode2->IsInboundConn() == true); } // prior to PR #14728, this test triggers an undefined behavior @@ -214,7 +209,7 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test) in_addr ipv4AddrPeer; ipv4AddrPeer.s_addr = 0xa0b0c001; CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK); - std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, 0, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, false); + std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, 0, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND); pnode->fSuccessfullyConnected.store(true); // the peer claims to be reaching us via IPv6 diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index 87678af4d1..1d6bcadf69 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -349,21 +349,16 @@ BOOST_AUTO_TEST_CASE(script_standard_GetScriptFor_) result = GetScriptForMultisig(2, std::vector<CPubKey>(pubkeys, pubkeys + 3)); BOOST_CHECK(result == expected); - // GetScriptForWitness - CScript witnessScript; - - witnessScript << ToByteVector(pubkeys[0]) << OP_CHECKSIG; + // WitnessV0KeyHash expected.clear(); expected << OP_0 << ToByteVector(pubkeys[0].GetID()); - result = GetScriptForWitness(witnessScript); + result = GetScriptForDestination(WitnessV0KeyHash(Hash160(ToByteVector(pubkeys[0])))); BOOST_CHECK(result == expected); - - witnessScript.clear(); - witnessScript << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG; - result = GetScriptForWitness(witnessScript); + result = GetScriptForDestination(WitnessV0KeyHash(pubkeys[0].GetID())); BOOST_CHECK(result == expected); - witnessScript.clear(); + // WitnessV0ScriptHash (multisig) + CScript witnessScript; witnessScript << OP_1 << ToByteVector(pubkeys[0]) << OP_1 << OP_CHECKMULTISIG; uint256 scriptHash; @@ -372,7 +367,7 @@ BOOST_AUTO_TEST_CASE(script_standard_GetScriptFor_) expected.clear(); expected << OP_0 << ToByteVector(scriptHash); - result = GetScriptForWitness(witnessScript); + result = GetScriptForDestination(WitnessV0ScriptHash(witnessScript)); BOOST_CHECK(result == expected); } diff --git a/src/test/sigopcount_tests.cpp b/src/test/sigopcount_tests.cpp index 6e36bce7a1..7e5274450d 100644 --- a/src/test/sigopcount_tests.cpp +++ b/src/test/sigopcount_tests.cpp @@ -154,8 +154,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) // P2WPKH witness program { - CScript p2pk = CScript() << ToByteVector(pubkey) << OP_CHECKSIG; - CScript scriptPubKey = GetScriptForWitness(p2pk); + CScript scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(pubkey)); CScript scriptSig = CScript(); CScriptWitness scriptWitness; scriptWitness.stack.push_back(std::vector<unsigned char>(0)); @@ -183,8 +182,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) // P2WPKH nested in P2SH { - CScript p2pk = CScript() << ToByteVector(pubkey) << OP_CHECKSIG; - CScript scriptSig = GetScriptForWitness(p2pk); + CScript scriptSig = GetScriptForDestination(WitnessV0KeyHash(pubkey)); CScript scriptPubKey = GetScriptForDestination(ScriptHash(scriptSig)); scriptSig = CScript() << ToByteVector(scriptSig); CScriptWitness scriptWitness; @@ -199,7 +197,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) // P2WSH witness program { CScript witnessScript = CScript() << 1 << ToByteVector(pubkey) << ToByteVector(pubkey) << 2 << OP_CHECKMULTISIGVERIFY; - CScript scriptPubKey = GetScriptForWitness(witnessScript); + CScript scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(witnessScript)); CScript scriptSig = CScript(); CScriptWitness scriptWitness; scriptWitness.stack.push_back(std::vector<unsigned char>(0)); @@ -215,7 +213,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) // P2WSH nested in P2SH { CScript witnessScript = CScript() << 1 << ToByteVector(pubkey) << ToByteVector(pubkey) << 2 << OP_CHECKMULTISIGVERIFY; - CScript redeemScript = GetScriptForWitness(witnessScript); + CScript redeemScript = GetScriptForDestination(WitnessV0ScriptHash(witnessScript)); CScript scriptPubKey = GetScriptForDestination(ScriptHash(redeemScript)); CScript scriptSig = CScript() << ToByteVector(redeemScript); CScriptWitness scriptWitness; diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 4bf6e734ce..94b5dba913 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -361,6 +361,8 @@ static CScript PushAll(const std::vector<valtype>& values) result << OP_0; } else if (v.size() == 1 && v[0] >= 1 && v[0] <= 16) { result << CScript::EncodeOP_N(v[0]); + } else if (v.size() == 1 && v[0] == 0x81) { + result << OP_1NEGATE; } else { result << v; } @@ -499,13 +501,19 @@ BOOST_AUTO_TEST_CASE(test_witness) BOOST_CHECK(keystore.AddCScript(scriptPubkey1L)); BOOST_CHECK(keystore.AddCScript(scriptPubkey2L)); BOOST_CHECK(keystore.AddCScript(scriptMulti)); - BOOST_CHECK(keystore.AddCScript(GetScriptForWitness(scriptPubkey1))); - BOOST_CHECK(keystore.AddCScript(GetScriptForWitness(scriptPubkey2))); - BOOST_CHECK(keystore.AddCScript(GetScriptForWitness(scriptPubkey1L))); - BOOST_CHECK(keystore.AddCScript(GetScriptForWitness(scriptPubkey2L))); - BOOST_CHECK(keystore.AddCScript(GetScriptForWitness(scriptMulti))); + CScript destination_script_1, destination_script_2, destination_script_1L, destination_script_2L, destination_script_multi; + destination_script_1 = GetScriptForDestination(WitnessV0KeyHash(pubkey1)); + destination_script_2 = GetScriptForDestination(WitnessV0KeyHash(pubkey2)); + destination_script_1L = GetScriptForDestination(WitnessV0KeyHash(pubkey1L)); + destination_script_2L = GetScriptForDestination(WitnessV0KeyHash(pubkey2L)); + destination_script_multi = GetScriptForDestination(WitnessV0ScriptHash(scriptMulti)); + BOOST_CHECK(keystore.AddCScript(destination_script_1)); + BOOST_CHECK(keystore.AddCScript(destination_script_2)); + BOOST_CHECK(keystore.AddCScript(destination_script_1L)); + BOOST_CHECK(keystore.AddCScript(destination_script_2L)); + BOOST_CHECK(keystore.AddCScript(destination_script_multi)); BOOST_CHECK(keystore2.AddCScript(scriptMulti)); - BOOST_CHECK(keystore2.AddCScript(GetScriptForWitness(scriptMulti))); + BOOST_CHECK(keystore2.AddCScript(destination_script_multi)); BOOST_CHECK(keystore2.AddKeyPubKey(key3, pubkey3)); CTransactionRef output1, output2; @@ -537,8 +545,8 @@ BOOST_AUTO_TEST_CASE(test_witness) CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false); // Witness pay-to-compressed-pubkey (v0). - CreateCreditAndSpend(keystore, GetScriptForWitness(scriptPubkey1), output1, input1); - CreateCreditAndSpend(keystore, GetScriptForWitness(scriptPubkey2), output2, input2); + CreateCreditAndSpend(keystore, destination_script_1, output1, input1); + CreateCreditAndSpend(keystore, destination_script_2, output2, input2); CheckWithFlag(output1, input1, 0, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, true); @@ -549,9 +557,9 @@ BOOST_AUTO_TEST_CASE(test_witness) CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false); // P2SH witness pay-to-compressed-pubkey (v0). - CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptPubkey1))), output1, input1); - CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptPubkey2))), output2, input2); - ReplaceRedeemScript(input2.vin[0].scriptSig, GetScriptForWitness(scriptPubkey1)); + CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_1)), output1, input1); + CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_2)), output2, input2); + ReplaceRedeemScript(input2.vin[0].scriptSig, destination_script_1); CheckWithFlag(output1, input1, 0, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, true); @@ -587,12 +595,12 @@ BOOST_AUTO_TEST_CASE(test_witness) CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false); // Signing disabled for witness pay-to-uncompressed-pubkey (v1). - CreateCreditAndSpend(keystore, GetScriptForWitness(scriptPubkey1L), output1, input1, false); - CreateCreditAndSpend(keystore, GetScriptForWitness(scriptPubkey2L), output2, input2, false); + CreateCreditAndSpend(keystore, destination_script_1L, output1, input1, false); + CreateCreditAndSpend(keystore, destination_script_2L, output2, input2, false); // Signing disabled for P2SH witness pay-to-uncompressed-pubkey (v1). - CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptPubkey1L))), output1, input1, false); - CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptPubkey2L))), output2, input2, false); + CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_1L)), output1, input1, false); + CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_2L)), output2, input2, false); // Normal 2-of-2 multisig CreateCreditAndSpend(keystore, scriptMulti, output1, input1, false); @@ -616,10 +624,10 @@ BOOST_AUTO_TEST_CASE(test_witness) CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); // Witness 2-of-2 multisig - CreateCreditAndSpend(keystore, GetScriptForWitness(scriptMulti), output1, input1, false); + CreateCreditAndSpend(keystore, destination_script_multi, output1, input1, false); CheckWithFlag(output1, input1, 0, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false); - CreateCreditAndSpend(keystore2, GetScriptForWitness(scriptMulti), output2, input2, false); + CreateCreditAndSpend(keystore2, destination_script_multi, output2, input2, false); CheckWithFlag(output2, input2, 0, true); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false); BOOST_CHECK(*output1 == *output2); @@ -628,10 +636,10 @@ BOOST_AUTO_TEST_CASE(test_witness) CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); // P2SH witness 2-of-2 multisig - CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptMulti))), output1, input1, false); + CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_multi)), output1, input1, false); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false); - CreateCreditAndSpend(keystore2, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptMulti))), output2, input2, false); + CreateCreditAndSpend(keystore2, GetScriptForDestination(ScriptHash(destination_script_multi)), output2, input2, false); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false); BOOST_CHECK(*output1 == *output2); diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index cdef7dcc3c..034577aa2c 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -157,7 +157,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) CScript p2pk_scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG; CScript p2sh_scriptPubKey = GetScriptForDestination(ScriptHash(p2pk_scriptPubKey)); CScript p2pkh_scriptPubKey = GetScriptForDestination(PKHash(coinbaseKey.GetPubKey())); - CScript p2wpkh_scriptPubKey = GetScriptForWitness(p2pkh_scriptPubKey); + CScript p2wpkh_scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(coinbaseKey.GetPubKey())); FillableSigningProvider keystore; BOOST_CHECK(keystore.AddKey(coinbaseKey)); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index b49370c967..bf7c6c3e3e 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -550,57 +550,52 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) BOOST_CHECK(test_args.m_settings.ro_config["sec1"].size() == 3); BOOST_CHECK(test_args.m_settings.ro_config["sec2"].size() == 2); - BOOST_CHECK(test_args.m_settings.ro_config[""].count("a") - && test_args.m_settings.ro_config[""].count("b") - && test_args.m_settings.ro_config[""].count("ccc") - && test_args.m_settings.ro_config[""].count("d") - && test_args.m_settings.ro_config[""].count("fff") - && test_args.m_settings.ro_config[""].count("ggg") - && test_args.m_settings.ro_config[""].count("h") - && test_args.m_settings.ro_config[""].count("i") - ); - BOOST_CHECK(test_args.m_settings.ro_config["sec1"].count("ccc") - && test_args.m_settings.ro_config["sec1"].count("h") - && test_args.m_settings.ro_config["sec2"].count("ccc") - && test_args.m_settings.ro_config["sec2"].count("iii") - ); - - BOOST_CHECK(test_args.IsArgSet("-a") - && test_args.IsArgSet("-b") - && test_args.IsArgSet("-ccc") - && test_args.IsArgSet("-d") - && test_args.IsArgSet("-fff") - && test_args.IsArgSet("-ggg") - && test_args.IsArgSet("-h") - && test_args.IsArgSet("-i") - && !test_args.IsArgSet("-zzz") - && !test_args.IsArgSet("-iii") - ); - - BOOST_CHECK(test_args.GetArg("-a", "xxx") == "" - && test_args.GetArg("-b", "xxx") == "1" - && test_args.GetArg("-ccc", "xxx") == "argument" - && test_args.GetArg("-d", "xxx") == "e" - && test_args.GetArg("-fff", "xxx") == "0" - && test_args.GetArg("-ggg", "xxx") == "1" - && test_args.GetArg("-h", "xxx") == "0" - && test_args.GetArg("-i", "xxx") == "1" - && test_args.GetArg("-zzz", "xxx") == "xxx" - && test_args.GetArg("-iii", "xxx") == "xxx" - ); + BOOST_CHECK(test_args.m_settings.ro_config[""].count("a")); + BOOST_CHECK(test_args.m_settings.ro_config[""].count("b")); + BOOST_CHECK(test_args.m_settings.ro_config[""].count("ccc")); + BOOST_CHECK(test_args.m_settings.ro_config[""].count("d")); + BOOST_CHECK(test_args.m_settings.ro_config[""].count("fff")); + BOOST_CHECK(test_args.m_settings.ro_config[""].count("ggg")); + BOOST_CHECK(test_args.m_settings.ro_config[""].count("h")); + BOOST_CHECK(test_args.m_settings.ro_config[""].count("i")); + BOOST_CHECK(test_args.m_settings.ro_config["sec1"].count("ccc")); + BOOST_CHECK(test_args.m_settings.ro_config["sec1"].count("h")); + BOOST_CHECK(test_args.m_settings.ro_config["sec2"].count("ccc")); + BOOST_CHECK(test_args.m_settings.ro_config["sec2"].count("iii")); + + BOOST_CHECK(test_args.IsArgSet("-a")); + BOOST_CHECK(test_args.IsArgSet("-b")); + BOOST_CHECK(test_args.IsArgSet("-ccc")); + BOOST_CHECK(test_args.IsArgSet("-d")); + BOOST_CHECK(test_args.IsArgSet("-fff")); + BOOST_CHECK(test_args.IsArgSet("-ggg")); + BOOST_CHECK(test_args.IsArgSet("-h")); + BOOST_CHECK(test_args.IsArgSet("-i")); + BOOST_CHECK(!test_args.IsArgSet("-zzz")); + BOOST_CHECK(!test_args.IsArgSet("-iii")); + + BOOST_CHECK_EQUAL(test_args.GetArg("-a", "xxx"), ""); + BOOST_CHECK_EQUAL(test_args.GetArg("-b", "xxx"), "1"); + BOOST_CHECK_EQUAL(test_args.GetArg("-ccc", "xxx"), "argument"); + BOOST_CHECK_EQUAL(test_args.GetArg("-d", "xxx"), "e"); + BOOST_CHECK_EQUAL(test_args.GetArg("-fff", "xxx"), "0"); + BOOST_CHECK_EQUAL(test_args.GetArg("-ggg", "xxx"), "1"); + BOOST_CHECK_EQUAL(test_args.GetArg("-h", "xxx"), "0"); + BOOST_CHECK_EQUAL(test_args.GetArg("-i", "xxx"), "1"); + BOOST_CHECK_EQUAL(test_args.GetArg("-zzz", "xxx"), "xxx"); + BOOST_CHECK_EQUAL(test_args.GetArg("-iii", "xxx"), "xxx"); for (const bool def : {false, true}) { - BOOST_CHECK(test_args.GetBoolArg("-a", def) - && test_args.GetBoolArg("-b", def) - && !test_args.GetBoolArg("-ccc", def) - && !test_args.GetBoolArg("-d", def) - && !test_args.GetBoolArg("-fff", def) - && test_args.GetBoolArg("-ggg", def) - && !test_args.GetBoolArg("-h", def) - && test_args.GetBoolArg("-i", def) - && test_args.GetBoolArg("-zzz", def) == def - && test_args.GetBoolArg("-iii", def) == def - ); + BOOST_CHECK(test_args.GetBoolArg("-a", def)); + BOOST_CHECK(test_args.GetBoolArg("-b", def)); + BOOST_CHECK(!test_args.GetBoolArg("-ccc", def)); + BOOST_CHECK(!test_args.GetBoolArg("-d", def)); + BOOST_CHECK(!test_args.GetBoolArg("-fff", def)); + BOOST_CHECK(test_args.GetBoolArg("-ggg", def)); + BOOST_CHECK(!test_args.GetBoolArg("-h", def)); + BOOST_CHECK(test_args.GetBoolArg("-i", def)); + BOOST_CHECK(test_args.GetBoolArg("-zzz", def) == def); + BOOST_CHECK(test_args.GetBoolArg("-iii", def) == def); } BOOST_CHECK(test_args.GetArgs("-a").size() == 1 @@ -636,13 +631,12 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) test_args.SelectConfigNetwork("sec1"); // same as original - BOOST_CHECK(test_args.GetArg("-a", "xxx") == "" - && test_args.GetArg("-b", "xxx") == "1" - && test_args.GetArg("-fff", "xxx") == "0" - && test_args.GetArg("-ggg", "xxx") == "1" - && test_args.GetArg("-zzz", "xxx") == "xxx" - && test_args.GetArg("-iii", "xxx") == "xxx" - ); + BOOST_CHECK_EQUAL(test_args.GetArg("-a", "xxx"), ""); + BOOST_CHECK_EQUAL(test_args.GetArg("-b", "xxx"), "1"); + BOOST_CHECK_EQUAL(test_args.GetArg("-fff", "xxx"), "0"); + BOOST_CHECK_EQUAL(test_args.GetArg("-ggg", "xxx"), "1"); + BOOST_CHECK_EQUAL(test_args.GetArg("-zzz", "xxx"), "xxx"); + BOOST_CHECK_EQUAL(test_args.GetArg("-iii", "xxx"), "xxx"); // d is overridden BOOST_CHECK(test_args.GetArg("-d", "xxx") == "eee"); // section-specific setting @@ -657,14 +651,13 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) test_args.SelectConfigNetwork("sec2"); // same as original - BOOST_CHECK(test_args.GetArg("-a", "xxx") == "" - && test_args.GetArg("-b", "xxx") == "1" - && test_args.GetArg("-d", "xxx") == "e" - && test_args.GetArg("-fff", "xxx") == "0" - && test_args.GetArg("-ggg", "xxx") == "1" - && test_args.GetArg("-zzz", "xxx") == "xxx" - && test_args.GetArg("-h", "xxx") == "0" - ); + BOOST_CHECK(test_args.GetArg("-a", "xxx") == ""); + BOOST_CHECK(test_args.GetArg("-b", "xxx") == "1"); + BOOST_CHECK(test_args.GetArg("-d", "xxx") == "e"); + BOOST_CHECK(test_args.GetArg("-fff", "xxx") == "0"); + BOOST_CHECK(test_args.GetArg("-ggg", "xxx") == "1"); + BOOST_CHECK(test_args.GetArg("-zzz", "xxx") == "xxx"); + BOOST_CHECK(test_args.GetArg("-h", "xxx") == "0"); // section-specific setting BOOST_CHECK(test_args.GetArg("-iii", "xxx") == "2"); // section takes priority for multiple values diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 079a5d3d53..1a45a2b313 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -5,6 +5,7 @@ #include <wallet/coinselection.h> #include <optional.h> +#include <policy/feerate.h> #include <util/system.h> #include <util/moneystr.h> @@ -302,7 +303,7 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants) { m_outputs.push_back(output); m_from_me &= from_me; - m_value += output.effective_value; + m_value += output.txout.nValue; m_depth = std::min(m_depth, depth); // ancestors here express the number of ancestors the new coin will end up having, which is // the sum, rather than the max; this will overestimate in the cases where multiple inputs @@ -311,15 +312,19 @@ void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size // descendants is the count as seen from the top ancestor, not the descendants as seen from the // coin itself; thus, this value is counted as the max, not the sum m_descendants = std::max(m_descendants, descendants); - effective_value = m_value; + effective_value += output.effective_value; + fee += output.m_fee; + long_term_fee += output.m_long_term_fee; } std::vector<CInputCoin>::iterator OutputGroup::Discard(const CInputCoin& output) { auto it = m_outputs.begin(); while (it != m_outputs.end() && it->outpoint != output.outpoint) ++it; if (it == m_outputs.end()) return it; - m_value -= output.effective_value; + m_value -= output.txout.nValue; effective_value -= output.effective_value; + fee -= output.m_fee; + long_term_fee -= output.m_long_term_fee; return m_outputs.erase(it); } @@ -329,3 +334,35 @@ bool OutputGroup::EligibleForSpending(const CoinEligibilityFilter& eligibility_f && m_ancestors <= eligibility_filter.max_ancestors && m_descendants <= eligibility_filter.max_descendants; } + +void OutputGroup::SetFees(const CFeeRate effective_feerate, const CFeeRate long_term_feerate) +{ + fee = 0; + long_term_fee = 0; + effective_value = 0; + for (CInputCoin& coin : m_outputs) { + coin.m_fee = coin.m_input_bytes < 0 ? 0 : effective_feerate.GetFee(coin.m_input_bytes); + fee += coin.m_fee; + + coin.m_long_term_fee = coin.m_input_bytes < 0 ? 0 : long_term_feerate.GetFee(coin.m_input_bytes); + long_term_fee += coin.m_long_term_fee; + + coin.effective_value = coin.txout.nValue - coin.m_fee; + effective_value += coin.effective_value; + } +} + +OutputGroup OutputGroup::GetPositiveOnlyGroup() +{ + OutputGroup group(*this); + for (auto it = group.m_outputs.begin(); it != group.m_outputs.end(); ) { + const CInputCoin& coin = *it; + // Only include outputs that are positive effective value (i.e. not dust) + if (coin.effective_value <= 0) { + it = group.Discard(coin); + } else { + ++it; + } + } + return group; +} diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 5348401f45..49c1134ec6 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -9,6 +9,8 @@ #include <primitives/transaction.h> #include <random.h> +class CFeeRate; + //! target minimum change amount static constexpr CAmount MIN_CHANGE{COIN / 100}; //! final minimum change amount after paying for fees @@ -36,6 +38,8 @@ public: COutPoint outpoint; CTxOut txout; CAmount effective_value; + CAmount m_fee{0}; + CAmount m_long_term_fee{0}; /** Pre-computed estimated size of this output as a fully-signed input in a transaction. Can be -1 if it could not be calculated */ int m_input_bytes{-1}; @@ -91,6 +95,10 @@ struct OutputGroup void Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants); std::vector<CInputCoin>::iterator Discard(const CInputCoin& output); bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const; + + //! Update the OutputGroup's fee, long_term_fee, and effective_value based on the given feerates + void SetFees(const CFeeRate effective_feerate, const CFeeRate long_term_feerate); + OutputGroup GetPositiveOnlyGroup(); }; bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret, CAmount not_input_fees); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 52162ab521..bf05ef844a 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -9,6 +9,7 @@ #include <node/context.h> #include <node/ui_interface.h> #include <outputtype.h> +#include <univalue.h> #include <util/check.h> #include <util/moneystr.h> #include <util/system.h> @@ -48,6 +49,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const argsman.AddArg("-fallbackfee=<amt>", strprintf("A fee rate (in %s/kB) that will be used when fee estimation has insufficient data. 0 to entirely disable the fallbackfee feature. (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_FALLBACK_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-keypool=<n>", strprintf("Set key pool size to <n> (default: %u). Warning: Smaller sizes may increase the risk of losing funds when restoring from an old backup, if none of the addresses in the original keypool have been used.", DEFAULT_KEYPOOL_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + argsman.AddArg("-maxapsfee=<n>", strprintf("Spend up to this amount in additional (absolute) fees (in %s) if it allows the use of partial spend avoidance (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_MAX_AVOIDPARTIALSPEND_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-maxtxfee=<amt>", strprintf("Maximum total fees (in %s) to use in a single wallet transaction; setting this too low may abort large transactions (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MAXFEE)), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-mintxfee=<amt>", strprintf("Fees (in %s/kB) smaller than this are considered zero fee for transaction creation (default: %s)", @@ -118,6 +120,14 @@ void WalletInit::Construct(NodeContext& node) const LogPrintf("Wallet disabled!\n"); return; } - args.SoftSetArg("-wallet", ""); + // If there's no -wallet setting with a list of wallets to load, set it to + // load the default "" wallet. + if (!args.IsArgSet("wallet")) { + args.LockSettings([&](util::Settings& settings) { + util::SettingsValue wallets(util::SettingsValue::VARR); + wallets.push_back(""); // Default wallet name is "" + settings.rw_settings["wallet"] = wallets; + }); + } node.chain_clients.emplace_back(interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet"))); } diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 2a81d30133..ae14769edb 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -13,6 +13,8 @@ #include <wallet/wallet.h> #include <wallet/walletdb.h> +#include <univalue.h> + bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files) { if (gArgs.IsArgSet("-walletdir")) { @@ -120,3 +122,26 @@ void UnloadWallets() UnloadWallet(std::move(wallet)); } } + +bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) +{ + util::SettingsValue setting_value = chain.getRwSetting("wallet"); + if (!setting_value.isArray()) setting_value.setArray(); + for (const util::SettingsValue& value : setting_value.getValues()) { + if (value.isStr() && value.get_str() == wallet_name) return true; + } + setting_value.push_back(wallet_name); + return chain.updateRwSetting("wallet", setting_value); +} + +bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) +{ + util::SettingsValue setting_value = chain.getRwSetting("wallet"); + if (!setting_value.isArray()) return true; + util::SettingsValue new_value(util::SettingsValue::VARR); + for (const util::SettingsValue& value : setting_value.getValues()) { + if (!value.isStr() || value.get_str() != wallet_name) new_value.push_back(value); + } + if (new_value.size() == setting_value.size()) return true; + return chain.updateRwSetting("wallet", new_value); +} diff --git a/src/wallet/load.h b/src/wallet/load.h index ff4f5b4b23..30f1a4c90d 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -34,4 +34,10 @@ void StopWallets(); //! Close all wallets. void UnloadWallets(); +//! Add wallet name to persistent configuration so it will be loaded on startup. +bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); + +//! Remove wallet name from persistent configuration so it will not be loaded on startup. +bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); + #endif // BITCOIN_WALLET_LOAD_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 58eaf54175..7a03c4f544 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -30,6 +30,7 @@ #include <wallet/coincontrol.h> #include <wallet/context.h> #include <wallet/feebumper.h> +#include <wallet/load.h> #include <wallet/rpcwallet.h> #include <wallet/wallet.h> #include <wallet/walletdb.h> @@ -229,6 +230,18 @@ static void SetFeeEstimateMode(const CWallet* pwallet, CCoinControl& cc, const U } } +static void UpdateWalletSetting(interfaces::Chain& chain, + const std::string& wallet_name, + const UniValue& load_on_startup, + std::vector<bilingual_str>& warnings) +{ + if (load_on_startup.isTrue() && !AddWalletSetting(chain, wallet_name)) { + warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may not be loaded next node startup.")); + } else if (load_on_startup.isFalse() && !RemoveWalletSetting(chain, wallet_name)) { + warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may still be loaded next node startup.")); + } +} + static UniValue getnewaddress(const JSONRPCRequest& request) { RPCHelpMan{"getnewaddress", @@ -1484,7 +1497,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request) {RPCResult::Type::ARR, "removed", "<structure is the same as \"transactions\" above, only present if include_removed=true>\n" "Note: transactions that were re-added in the active chain will appear as-is in this array, and may thus have a positive confirmation count." , {{RPCResult::Type::ELISION, "", ""},}}, - {RPCResult::Type::STR_HEX, "lastblock", "The hash of the block (target_confirmations-1) from the best block on the main chain. This is typically used to feed back into listsinceblock the next time you call it. So you would generally use a target_confirmations of say 6, so you will be continually re-notified of transactions until they've reached 6 confirmations plus any new ones"}, + {RPCResult::Type::STR_HEX, "lastblock", "The hash of the block (target_confirmations-1) from the best block on the main chain, or the genesis hash if the referenced block does not exist yet. This is typically used to feed back into listsinceblock the next time you call it. So you would generally use a target_confirmations of say 6, so you will be continually re-notified of transactions until they've reached 6 confirmations plus any new ones"}, } }, RPCExamples{ @@ -1567,6 +1580,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request) } uint256 lastblock; + target_confirms = std::min(target_confirms, wallet.GetLastBlockHeight() + 1); CHECK_NONFATAL(wallet.chain().findAncestorByHeight(wallet.GetLastBlockHash(), wallet.GetLastBlockHeight() + 1 - target_confirms, FoundBlock().hash(lastblock))); UniValue ret(UniValue::VOBJ); @@ -2483,6 +2497,7 @@ static UniValue loadwallet(const JSONRPCRequest& request) "\napplied to the new wallet (eg -zapwallettxes, rescan, etc).\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."}, + {"load_on_startup", RPCArg::Type::BOOL, /* default */ "null", "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -2515,6 +2530,8 @@ static UniValue loadwallet(const JSONRPCRequest& request) std::shared_ptr<CWallet> const wallet = LoadWallet(*context.chain, location, error, warnings); if (!wallet) throw JSONRPCError(RPC_WALLET_ERROR, error.original); + UpdateWalletSetting(*context.chain, location.GetName(), request.params[1], warnings); + UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); @@ -2599,6 +2616,7 @@ static UniValue createwallet(const JSONRPCRequest& request) {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Encrypt the wallet with this passphrase."}, {"avoid_reuse", RPCArg::Type::BOOL, /* default */ "false", "Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind."}, {"descriptors", RPCArg::Type::BOOL, /* default */ "false", "Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation"}, + {"load_on_startup", RPCArg::Type::BOOL, /* default */ "null", "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -2654,6 +2672,8 @@ static UniValue createwallet(const JSONRPCRequest& request) // no default case, so the compiler can warn about missing cases } + UpdateWalletSetting(*context.chain, request.params[0].get_str(), request.params[6], warnings); + UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); @@ -2668,8 +2688,11 @@ static UniValue unloadwallet(const JSONRPCRequest& request) "Specifying the wallet name on a wallet endpoint is invalid.", { {"wallet_name", RPCArg::Type::STR, /* default */ "the wallet name from the RPC request", "The name of the wallet to unload."}, + {"load_on_startup", RPCArg::Type::BOOL, /* default */ "null", "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, - RPCResult{RPCResult::Type::NONE, "", ""}, + RPCResult{RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::STR, "warning", "Warning message if wallet was not unloaded cleanly."}, + }}, RPCExamples{ HelpExampleCli("unloadwallet", "wallet_name") + HelpExampleRpc("unloadwallet", "wallet_name") @@ -2697,9 +2720,15 @@ static UniValue unloadwallet(const JSONRPCRequest& request) throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded"); } + interfaces::Chain& chain = wallet->chain(); + std::vector<bilingual_str> warnings; + UnloadWallet(std::move(wallet)); + UpdateWalletSetting(chain, wallet_name, request.params[1], warnings); - return NullUniValue; + UniValue result(UniValue::VOBJ); + result.pushKV("warning", Join(warnings, Untranslated("\n")).original); + return result; } static UniValue listunspent(const JSONRPCRequest& request) @@ -3165,7 +3194,7 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) { {RPCResult::Type::STR_HEX, "hex", "The hex-encoded raw transaction with signature(s)"}, {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, - {RPCResult::Type::ARR, "errors", "Script verification errors (if there are any)", + {RPCResult::Type::ARR, "errors", /* optional */ true, "Script verification errors (if there are any)", { {RPCResult::Type::OBJ, "", "", { @@ -3222,59 +3251,73 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) static UniValue bumpfee(const JSONRPCRequest& request) { - RPCHelpMan{"bumpfee", - "\nBumps the fee of an opt-in-RBF transaction T, replacing it with a new transaction B.\n" - "An opt-in RBF transaction with the given txid must be in the wallet.\n" - "The command will pay the additional fee by reducing change outputs or adding inputs when necessary. It may add a new change output if one does not already exist.\n" - "All inputs in the original transaction will be included in the replacement transaction.\n" - "The command will fail if the wallet or mempool contains a transaction that spends one of T's outputs.\n" - "By default, the new fee will be calculated automatically using estimatesmartfee.\n" - "The user can specify a confirmation target for estimatesmartfee.\n" - "Alternatively, the user can specify a fee_rate (" + CURRENCY_UNIT + " per kB) for the new transaction.\n" - "At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee\n" - "returned by getnetworkinfo) to enter the node's mempool.\n", + bool want_psbt = request.strMethod == "psbtbumpfee"; + + RPCHelpMan{request.strMethod, + "\nBumps the fee of an opt-in-RBF transaction T, replacing it with a new transaction B.\n" + + std::string(want_psbt ? "Returns a PSBT instead of creating and signing a new transaction.\n" : "") + + "An opt-in RBF transaction with the given txid must be in the wallet.\n" + "The command will pay the additional fee by reducing change outputs or adding inputs when necessary. It may add a new change output if one does not already exist.\n" + "All inputs in the original transaction will be included in the replacement transaction.\n" + "The command will fail if the wallet or mempool contains a transaction that spends one of T's outputs.\n" + "By default, the new fee will be calculated automatically using estimatesmartfee.\n" + "The user can specify a confirmation target for estimatesmartfee.\n" + "Alternatively, the user can specify a fee_rate (" + CURRENCY_UNIT + " per kB) for the new transaction.\n" + "At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee\n" + "returned by getnetworkinfo) to enter the node's mempool.\n", + { + {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The txid to be bumped"}, + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", { - {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The txid to be bumped"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", - { - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"}, - {"fee_rate", RPCArg::Type::NUM, /* default */ "fall back to 'conf_target'", "fee rate (NOT total fee) to pay, in " + CURRENCY_UNIT + " per kB\n" - " Specify a fee rate instead of relying on the built-in fee estimator.\n" - "Must be at least 0.0001 " + CURRENCY_UNIT + " per kB higher than the current transaction fee rate.\n"}, - {"replaceable", RPCArg::Type::BOOL, /* default */ "true", "Whether the new transaction should still be\n" - " marked bip-125 replaceable. If true, the sequence numbers in the transaction will\n" - " be left unchanged from the original. If false, any input sequence numbers in the\n" - " original transaction that were less than 0xfffffffe will be increased to 0xfffffffe\n" - " so the new transaction will not be explicitly bip-125 replaceable (though it may\n" - " still be replaceable in practice, for example if it has unconfirmed ancestors which\n" - " are replaceable)."}, - {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" - " \"" + FeeModes("\"\n\"") + "\""}, - }, - "options"}, - }, - RPCResult{ - RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR, "psbt", "The base64-encoded unsigned PSBT of the new transaction. Only returned when wallet private keys are disabled."}, - {RPCResult::Type::STR_HEX, "txid", "The id of the new transaction. Only returned when wallet private keys are enabled."}, - {RPCResult::Type::STR_AMOUNT, "origfee", "The fee of the replaced transaction."}, - {RPCResult::Type::STR_AMOUNT, "fee", "The fee of the new transaction."}, - {RPCResult::Type::ARR, "errors", "Errors encountered during processing (may be empty).", - { - {RPCResult::Type::STR, "", ""}, - }}, - } - }, - RPCExamples{ - "\nBump the fee, get the new transaction\'s txid\n" + - HelpExampleCli("bumpfee", "<txid>") + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"}, + {"fee_rate", RPCArg::Type::NUM, /* default */ "fall back to 'conf_target'", "fee rate (NOT total fee) to pay, in " + CURRENCY_UNIT + " per kB\n" + " Specify a fee rate instead of relying on the built-in fee estimator.\n" + "Must be at least 0.0001 " + CURRENCY_UNIT + " per kB higher than the current transaction fee rate.\n"}, + {"replaceable", RPCArg::Type::BOOL, /* default */ "true", "Whether the new transaction should still be\n" + " marked bip-125 replaceable. If true, the sequence numbers in the transaction will\n" + " be left unchanged from the original. If false, any input sequence numbers in the\n" + " original transaction that were less than 0xfffffffe will be increased to 0xfffffffe\n" + " so the new transaction will not be explicitly bip-125 replaceable (though it may\n" + " still be replaceable in practice, for example if it has unconfirmed ancestors which\n" + " are replaceable)."}, + {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" + " \"" + FeeModes("\"\n\"") + "\""}, }, - }.Check(request); + "options"}, + }, + RPCResult{ + RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>( + { + {RPCResult::Type::STR, "psbt", "The base64-encoded unsigned PSBT of the new transaction." + std::string(want_psbt ? "" : " Only returned when wallet private keys are disabled. (DEPRECATED)")}, + }, + want_psbt ? std::vector<RPCResult>{} : std::vector<RPCResult>{{RPCResult::Type::STR_HEX, "txid", "The id of the new transaction. Only returned when wallet private keys are enabled."}} + ), + { + {RPCResult::Type::STR_AMOUNT, "origfee", "The fee of the replaced transaction."}, + {RPCResult::Type::STR_AMOUNT, "fee", "The fee of the new transaction."}, + {RPCResult::Type::ARR, "errors", "Errors encountered during processing (may be empty).", + { + {RPCResult::Type::STR, "", ""}, + }}, + }) + }, + RPCExamples{ + "\nBump the fee, get the new transaction\'s" + std::string(want_psbt ? "psbt" : "txid") + "\n" + + HelpExampleCli(request.strMethod, "<txid>") + }, + }.Check(request); std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !want_psbt) { + if (!pwallet->chain().rpcEnableDeprecated("bumpfee")) { + throw JSONRPCError(RPC_METHOD_DEPRECATED, "Using bumpfee with wallets that have private keys disabled is deprecated. Use psbtbumpfee instead or restart bitcoind with -deprecatedrpc=bumpfee. This functionality will be removed in 0.22"); + } + want_psbt = true; + } + RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VOBJ}); uint256 hash(ParseHashV(request.params[0], "txid")); @@ -3359,7 +3402,7 @@ static UniValue bumpfee(const JSONRPCRequest& request) // If wallet private keys are enabled, return the new transaction id, // otherwise return the base64-encoded unsigned PSBT of the new transaction. - if (!pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + if (!want_psbt) { if (!feebumper::SignTransaction(*pwallet, mtx)) { throw JSONRPCError(RPC_WALLET_ERROR, "Can't sign transaction."); } @@ -3392,6 +3435,11 @@ static UniValue bumpfee(const JSONRPCRequest& request) return result; } +static UniValue psbtbumpfee(const JSONRPCRequest& request) +{ + return bumpfee(request); +} + UniValue rescanblockchain(const JSONRPCRequest& request) { RPCHelpMan{"rescanblockchain", @@ -4137,7 +4185,8 @@ static const CRPCCommand commands[] = { "wallet", "addmultisigaddress", &addmultisigaddress, {"nrequired","keys","label","address_type"} }, { "wallet", "backupwallet", &backupwallet, {"destination"} }, { "wallet", "bumpfee", &bumpfee, {"txid", "options"} }, - { "wallet", "createwallet", &createwallet, {"wallet_name", "disable_private_keys", "blank", "passphrase", "avoid_reuse", "descriptors"} }, + { "wallet", "psbtbumpfee", &psbtbumpfee, {"txid", "options"} }, + { "wallet", "createwallet", &createwallet, {"wallet_name", "disable_private_keys", "blank", "passphrase", "avoid_reuse", "descriptors", "load_on_startup"} }, { "wallet", "dumpprivkey", &dumpprivkey, {"address"} }, { "wallet", "dumpwallet", &dumpwallet, {"filename"} }, { "wallet", "encryptwallet", &encryptwallet, {"passphrase"} }, @@ -4170,7 +4219,7 @@ static const CRPCCommand commands[] = { "wallet", "listunspent", &listunspent, {"minconf","maxconf","addresses","include_unsafe","query_options"} }, { "wallet", "listwalletdir", &listwalletdir, {} }, { "wallet", "listwallets", &listwallets, {} }, - { "wallet", "loadwallet", &loadwallet, {"filename"} }, + { "wallet", "loadwallet", &loadwallet, {"filename", "load_on_startup"} }, { "wallet", "lockunspent", &lockunspent, {"unlock","transactions"} }, { "wallet", "removeprunedfunds", &removeprunedfunds, {"txid"} }, { "wallet", "rescanblockchain", &rescanblockchain, {"start_height", "stop_height"} }, @@ -4182,7 +4231,7 @@ static const CRPCCommand commands[] = { "wallet", "setwalletflag", &setwalletflag, {"flag","value"} }, { "wallet", "signmessage", &signmessage, {"address","message"} }, { "wallet", "signrawtransactionwithwallet", &signrawtransactionwithwallet, {"hexstring","prevtxs","sighashtype"} }, - { "wallet", "unloadwallet", &unloadwallet, {"wallet_name"} }, + { "wallet", "unloadwallet", &unloadwallet, {"wallet_name", "load_on_startup"} }, { "wallet", "upgradewallet", &upgradewallet, {"version"} }, { "wallet", "walletcreatefundedpsbt", &walletcreatefundedpsbt, {"inputs","outputs","locktime","options","bip32derivs"} }, { "wallet", "walletlock", &walletlock, {} }, diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index af57210f01..c0755db751 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -16,14 +16,12 @@ static const char *HEADER_END = "HEADER=END"; static const char *DATA_END = "DATA=END"; typedef std::pair<std::vector<unsigned char>, std::vector<unsigned char> > KeyValPair; -bool RecoverDatabaseFile(const fs::path& file_path) +bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::vector<bilingual_str>& warnings) { std::string filename; std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, filename); - bilingual_str open_err; - if (!env->Open(open_err)) { - tfm::format(std::cerr, "%s\n", open_err.original); + if (!env->Open(error)) { return false; } @@ -39,11 +37,9 @@ bool RecoverDatabaseFile(const fs::path& file_path) int result = env->dbenv->dbrename(nullptr, filename.c_str(), nullptr, newFilename.c_str(), DB_AUTO_COMMIT); - if (result == 0) - LogPrintf("Renamed %s to %s\n", filename, newFilename); - else + if (result != 0) { - LogPrintf("Failed to rename %s to %s\n", filename, newFilename); + error = strprintf(Untranslated("Failed to rename %s to %s"), filename, newFilename); return false; } @@ -60,10 +56,10 @@ bool RecoverDatabaseFile(const fs::path& file_path) Db db(env->dbenv.get(), 0); result = db.verify(newFilename.c_str(), nullptr, &strDump, DB_SALVAGE | DB_AGGRESSIVE); if (result == DB_VERIFY_BAD) { - LogPrintf("Salvage: Database salvage found errors, all data may not be recoverable.\n"); + warnings.push_back(Untranslated("Salvage: Database salvage found errors, all data may not be recoverable.")); } if (result != 0 && result != DB_VERIFY_BAD) { - LogPrintf("Salvage: Database salvage failed with result %d.\n", result); + error = strprintf(Untranslated("Salvage: Database salvage failed with result %d."), result); return false; } @@ -87,7 +83,7 @@ bool RecoverDatabaseFile(const fs::path& file_path) break; getline(strDump, valueHex); if (valueHex == DATA_END) { - LogPrintf("Salvage: WARNING: Number of keys in data does not match number of values.\n"); + warnings.push_back(Untranslated("Salvage: WARNING: Number of keys in data does not match number of values.")); break; } salvagedData.push_back(make_pair(ParseHex(keyHex), ParseHex(valueHex))); @@ -96,7 +92,7 @@ bool RecoverDatabaseFile(const fs::path& file_path) bool fSuccess; if (keyHex != DATA_END) { - LogPrintf("Salvage: WARNING: Unexpected end of file while reading salvage output.\n"); + warnings.push_back(Untranslated("Salvage: WARNING: Unexpected end of file while reading salvage output.")); fSuccess = false; } else { fSuccess = (result == 0); @@ -104,10 +100,9 @@ bool RecoverDatabaseFile(const fs::path& file_path) if (salvagedData.empty()) { - LogPrintf("Salvage(aggressive) found no records in %s.\n", newFilename); + error = strprintf(Untranslated("Salvage(aggressive) found no records in %s."), newFilename); return false; } - LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size()); std::unique_ptr<Db> pdbCopy = MakeUnique<Db>(env->dbenv.get(), 0); int ret = pdbCopy->open(nullptr, // Txn pointer @@ -117,7 +112,7 @@ bool RecoverDatabaseFile(const fs::path& file_path) DB_CREATE, // Flags 0); if (ret > 0) { - LogPrintf("Cannot create database file %s\n", filename); + error = strprintf(Untranslated("Cannot create database file %s"), filename); pdbCopy->close(0); return false; } @@ -141,7 +136,7 @@ bool RecoverDatabaseFile(const fs::path& file_path) } if (!fReadOK) { - LogPrintf("WARNING: WalletBatch::Recover skipping %s: %s\n", strType, strErr); + warnings.push_back(strprintf(Untranslated("WARNING: WalletBatch::Recover skipping %s: %s"), strType, strErr)); continue; } Dbt datKey(&row.first[0], row.first.size()); diff --git a/src/wallet/salvage.h b/src/wallet/salvage.h index e361930f5e..5a8538f942 100644 --- a/src/wallet/salvage.h +++ b/src/wallet/salvage.h @@ -9,6 +9,8 @@ #include <fs.h> #include <streams.h> -bool RecoverDatabaseFile(const fs::path& file_path); +struct bilingual_str; + +bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::vector<bilingual_str>& warnings); #endif // BITCOIN_WALLET_SALVAGE_H diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index cee2f2214c..ac03b3ca9c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2320,27 +2320,15 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil for (OutputGroup& group : groups) { if (!group.EligibleForSpending(eligibility_filter)) continue; - group.fee = 0; - group.long_term_fee = 0; - group.effective_value = 0; - for (auto it = group.m_outputs.begin(); it != group.m_outputs.end(); ) { - const CInputCoin& coin = *it; - CAmount effective_value = coin.txout.nValue - (coin.m_input_bytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(coin.m_input_bytes)); - // Only include outputs that are positive effective value (i.e. not dust) - if (effective_value > 0) { - group.fee += coin.m_input_bytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(coin.m_input_bytes); - group.long_term_fee += coin.m_input_bytes < 0 ? 0 : long_term_feerate.GetFee(coin.m_input_bytes); - if (coin_selection_params.m_subtract_fee_outputs) { - group.effective_value += coin.txout.nValue; - } else { - group.effective_value += effective_value; - } - ++it; - } else { - it = group.Discard(coin); - } + if (coin_selection_params.m_subtract_fee_outputs) { + // Set the effective feerate to 0 as we don't want to use the effective value since the fees will be deducted from the output + group.SetFees(CFeeRate(0) /* effective_feerate */, long_term_feerate); + } else { + group.SetFees(coin_selection_params.effective_fee, long_term_feerate); } - if (group.effective_value > 0) utxo_pool.push_back(group); + + OutputGroup pos_group = group.GetPositiveOnlyGroup(); + if (pos_group.effective_value > 0) utxo_pool.push_back(pos_group); } // Calculate the fees for things that aren't inputs CAmount not_input_fees = coin_selection_params.effective_fee.GetFee(coin_selection_params.tx_noinputs_size); @@ -2486,23 +2474,6 @@ bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, } // At this point, one input was not fully signed otherwise we would have exited already - // Find that input and figure out what went wrong. - for (unsigned int i = 0; i < tx.vin.size(); i++) { - // Get the prevout - CTxIn& txin = tx.vin[i]; - auto coin = coins.find(txin.prevout); - if (coin == coins.end() || coin->second.IsSpent()) { - input_errors[i] = "Input not found or already spent"; - continue; - } - - // Check if this input is complete - SignatureData sigdata = DataFromTransaction(tx, i, coin->second.out); - if (!sigdata.complete) { - input_errors[i] = "Unable to sign input, missing keys"; - continue; - } - } return false; } @@ -2706,7 +2677,14 @@ OutputType CWallet::TransactionChangeType(const Optional<OutputType>& change_typ return m_default_address_type; } -bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, bool sign) +bool CWallet::CreateTransactionInternal( + const std::vector<CRecipient>& vecSend, + CTransactionRef& tx, + CAmount& nFeeRet, + int& nChangePosInOut, + bilingual_str& error, + const CCoinControl& coin_control, + bool sign) { CAmount nValue = 0; const OutputType change_type = TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : m_default_change_type, vecSend); @@ -3061,6 +3039,39 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac return true; } +bool CWallet::CreateTransaction( + const std::vector<CRecipient>& vecSend, + CTransactionRef& tx, + CAmount& nFeeRet, + int& nChangePosInOut, + bilingual_str& error, + const CCoinControl& coin_control, + bool sign) +{ + int nChangePosIn = nChangePosInOut; + CTransactionRef tx2 = tx; + bool res = CreateTransactionInternal(vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, sign); + // try with avoidpartialspends unless it's enabled already + if (res && nFeeRet > 0 /* 0 means non-functional fee rate estimation */ && m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) { + CCoinControl tmp_cc = coin_control; + tmp_cc.m_avoid_partial_spends = true; + CAmount nFeeRet2; + int nChangePosInOut2 = nChangePosIn; + bilingual_str error2; // fired and forgotten; if an error occurs, we discard the results + if (CreateTransactionInternal(vecSend, tx2, nFeeRet2, nChangePosInOut2, error2, tmp_cc, sign)) { + // if fee of this alternative one is within the range of the max fee, we use this one + const bool use_aps = nFeeRet2 <= nFeeRet + m_max_aps_fee; + WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", nFeeRet, nFeeRet2, use_aps ? "grouped" : "non-grouped"); + if (use_aps) { + tx = tx2; + nFeeRet = nFeeRet2; + nChangePosInOut = nChangePosInOut2; + } + } + } + return res; +} + void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm) { LOCK(cs_wallet); @@ -3878,6 +3889,21 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, walletInstance->m_min_fee = CFeeRate(n); } + if (gArgs.IsArgSet("-maxapsfee")) { + CAmount n = 0; + if (gArgs.GetArg("-maxapsfee", "") == "-1") { + n = -1; + } else if (!ParseMoney(gArgs.GetArg("-maxapsfee", ""), n)) { + error = AmountErrMsg("maxapsfee", gArgs.GetArg("-maxapsfee", "")); + return nullptr; + } + if (n > HIGH_APS_FEE) { + warnings.push_back(AmountHighWarn("-maxapsfee") + Untranslated(" ") + + _("This is the maximum transaction fee you pay to prioritize partial spend avoidance over regular coin selection.")); + } + walletInstance->m_max_aps_fee = n; + } + if (gArgs.IsArgSet("-fallbackfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-fallbackfee", ""), nFeePerK)) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a761caf38c..6d9512f59a 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -72,6 +72,16 @@ static const CAmount DEFAULT_FALLBACK_FEE = 0; static const CAmount DEFAULT_DISCARD_FEE = 10000; //! -mintxfee default static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000; +/** + * maximum fee increase allowed to do partial spend avoidance, even for nodes with this feature disabled by default + * + * A value of -1 disables this feature completely. + * A value of 0 (current default) means to attempt to do partial spend avoidance, and use its results if the fees remain *unchanged* + * A value > 0 means to do partial spend avoidance if the fee difference against a regular coin selection instance is in the range [0..value]. + */ +static const CAmount DEFAULT_MAX_AVOIDPARTIALSPEND_FEE = 0; +//! discourage APS fee higher than this amount +constexpr CAmount HIGH_APS_FEE{COIN / 10000}; //! minimum recommended increment for BIP 125 replacement txs static const CAmount WALLET_INCREMENTAL_RELAY_FEE = 5000; //! Default for -spendzeroconfchange @@ -719,6 +729,8 @@ private: // ScriptPubKeyMan::GetID. In many cases it will be the hash of an internal structure std::map<uint256, std::unique_ptr<ScriptPubKeyMan>> m_spk_managers; + bool CreateTransactionInternal(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, bool sign); + public: /* * Main wallet lock. @@ -1008,6 +1020,7 @@ public: */ CFeeRate m_fallback_fee{DEFAULT_FALLBACK_FEE}; CFeeRate m_discard_rate{DEFAULT_DISCARD_FEE}; + CAmount m_max_aps_fee{DEFAULT_MAX_AVOIDPARTIALSPEND_FEE}; //!< note: this is absolute fee, not fee rate OutputType m_default_address_type{DEFAULT_ADDRESS_TYPE}; /** * Default output type for change outputs. When unset, automatically choose type diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 7c5bf7652b..64d60b1f44 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -261,10 +261,6 @@ public: DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut); /* Function to determine if a certain KV/key-type is a key (cryptographical key) type */ static bool IsKeyType(const std::string& strType); - /* verifies the database environment */ - static bool VerifyEnvironment(const fs::path& wallet_path, bilingual_str& errorStr); - /* verifies the database file */ - static bool VerifyDatabaseFile(const fs::path& wallet_path, bilingual_str& errorStr); //! write the hdchain model (external chain child index counter) bool WriteHDChain(const CHDChain& chain); diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 9f25b1ae7d..9b51461843 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -104,27 +104,6 @@ static void WalletShowInfo(CWallet* wallet_instance) tfm::format(std::cout, "Address Book: %zu\n", wallet_instance->m_address_book.size()); } -static bool SalvageWallet(const fs::path& path) -{ - // Create a Database handle to allow for the db to be initialized before recovery - std::unique_ptr<WalletDatabase> database = CreateWalletDatabase(path); - - // Initialize the environment before recovery - bilingual_str error_string; - try { - database->Verify(error_string); - } catch (const fs::filesystem_error& e) { - error_string = Untranslated(strprintf("Error loading wallet. %s", fsbridge::get_filesystem_error_message(e))); - } - if (!error_string.original.empty()) { - tfm::format(std::cerr, "Failed to open wallet for salvage :%s\n", error_string.original); - return false; - } - - // Perform the recovery - return RecoverDatabaseFile(path); -} - bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) { fs::path path = fs::absolute(name, GetWalletDir()); @@ -147,7 +126,18 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) WalletShowInfo(wallet_instance.get()); wallet_instance->Close(); } else if (command == "salvage") { - return SalvageWallet(path); + bilingual_str error; + std::vector<bilingual_str> warnings; + bool ret = RecoverDatabaseFile(path, error, warnings); + if (!ret) { + for (const auto& warning : warnings) { + tfm::format(std::cerr, "%s\n", warning.original); + } + if (!error.empty()) { + tfm::format(std::cerr, "%s\n", error.original); + } + } + return ret; } } else { tfm::format(std::cerr, "Invalid command: %s\n", command); diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py index 6d947ac660..a9e86bd2fc 100755 --- a/test/functional/p2p_blockfilters.py +++ b/test/functional/p2p_blockfilters.py @@ -4,12 +4,13 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Tests NODE_COMPACT_FILTERS (BIP 157/158). -Tests that a node configured with -blockfilterindex and -peerblockfilters can serve -cfheaders and cfcheckpts. +Tests that a node configured with -blockfilterindex and -peerblockfilters signals +NODE_COMPACT_FILTERS and can serve cfilters, cfheaders and cfcheckpts. """ from test_framework.messages import ( FILTER_TYPE_BASIC, + NODE_COMPACT_FILTERS, hash256, msg_getcfcheckpt, msg_getcfheaders, @@ -70,6 +71,14 @@ class CompactFiltersTest(BitcoinTestFramework): self.nodes[1].generate(1001) wait_until(lambda: self.nodes[1].getblockcount() == 2000) + # Check that nodes have signalled NODE_COMPACT_FILTERS correctly. + assert node0.nServices & NODE_COMPACT_FILTERS != 0 + assert node1.nServices & NODE_COMPACT_FILTERS == 0 + + # Check that the localservices is as expected. + assert int(self.nodes[0].getnetworkinfo()['localservices'], 16) & NODE_COMPACT_FILTERS != 0 + assert int(self.nodes[1].getnetworkinfo()['localservices'], 16) & NODE_COMPACT_FILTERS == 0 + self.log.info("get cfcheckpt on chain to be re-orged out.") request = msg_getcfcheckpt( filter_type=FILTER_TYPE_BASIC, diff --git a/test/functional/p2p_feefilter.py b/test/functional/p2p_feefilter.py index 0b51d8f4bb..3a9b8dfbd7 100755 --- a/test/functional/p2p_feefilter.py +++ b/test/functional/p2p_feefilter.py @@ -5,7 +5,6 @@ """Test processing of feefilter messages.""" from decimal import Decimal -import time from test_framework.messages import MSG_TX, MSG_WTX, msg_feefilter from test_framework.mininode import mininode_lock, P2PInterface @@ -17,16 +16,6 @@ def hashToHex(hash): return format(hash, '064x') -# Wait up to 60 secs to see if the testnode has received all the expected invs -def allInvsMatch(invsExpected, testnode): - for _ in range(60): - with mininode_lock: - if (sorted(invsExpected) == sorted(testnode.txinvs)): - return True - time.sleep(1) - return False - - class FeefilterConn(P2PInterface): feefilter_received = False @@ -48,6 +37,10 @@ class TestP2PConn(P2PInterface): if (i.type == MSG_TX) or (i.type == MSG_WTX): self.txinvs.append(hashToHex(i.hash)) + def wait_for_invs_to_match(self, invs_expected): + invs_expected.sort() + self.wait_until(lambda: invs_expected == sorted(self.txinvs), timeout=60) + def clear_invs(self): with mininode_lock: self.txinvs = [] @@ -61,7 +54,12 @@ class FeeFilterTest(BitcoinTestFramework): # mempool and wallet feerate calculation based on GetFee # rounding down 3 places, leading to stranded transactions. # See issue #16499 - self.extra_args = [["-minrelaytxfee=0.00000100", "-mintxfee=0.00000100"]] * self.num_nodes + # grant noban permission to all peers to speed up tx relay / mempool sync + self.extra_args = [[ + "-minrelaytxfee=0.00000100", + "-mintxfee=0.00000100", + "-whitelist=noban@127.0.0.1", + ]] * self.num_nodes def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -88,24 +86,22 @@ class FeeFilterTest(BitcoinTestFramework): conn = self.nodes[0].add_p2p_connection(TestP2PConn()) - # Test that invs are received by test connection for all txs at - # feerate of .2 sat/byte + self.log.info("Test txs paying 0.2 sat/byte are received by test connection") node1.settxfee(Decimal("0.00000200")) txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for _ in range(3)] - assert allInvsMatch(txids, conn) + conn.wait_for_invs_to_match(txids) conn.clear_invs() - # Set a filter of .15 sat/byte on test connection + # Set a fee filter of 0.15 sat/byte on test connection conn.send_and_ping(msg_feefilter(150)) - # Test that txs are still being received by test connection (paying .15 sat/byte) + self.log.info("Test txs paying 0.15 sat/byte are received by test connection") node1.settxfee(Decimal("0.00000150")) txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for _ in range(3)] - assert allInvsMatch(txids, conn) + conn.wait_for_invs_to_match(txids) conn.clear_invs() - # Change tx fee rate to .1 sat/byte and test they are no longer received - # by the test connection + self.log.info("Test txs paying 0.1 sat/byte are no longer received by test connection") node1.settxfee(Decimal("0.00000100")) [node1.sendtoaddress(node1.getnewaddress(), 1) for _ in range(3)] self.sync_mempools() # must be sure node 0 has received all txs @@ -119,13 +115,13 @@ class FeeFilterTest(BitcoinTestFramework): # as well. node0.settxfee(Decimal("0.00020000")) txids = [node0.sendtoaddress(node0.getnewaddress(), 1)] - assert allInvsMatch(txids, conn) + conn.wait_for_invs_to_match(txids) conn.clear_invs() - # Remove fee filter and check that txs are received again + self.log.info("Remove fee filter and check txs are received again") conn.send_and_ping(msg_feefilter(0)) txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for _ in range(3)] - assert allInvsMatch(txids, conn) + conn.wait_for_invs_to_match(txids) conn.clear_invs() diff --git a/test/functional/rpc_deprecated.py b/test/functional/rpc_deprecated.py index 9a21998d11..b71854d234 100755 --- a/test/functional/rpc_deprecated.py +++ b/test/functional/rpc_deprecated.py @@ -4,13 +4,13 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test deprecation of RPC calls.""" from test_framework.test_framework import BitcoinTestFramework -# from test_framework.util import assert_raises_rpc_error +from test_framework.util import assert_raises_rpc_error, find_vout_for_address class DeprecatedRpcTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True - self.extra_args = [[], []] + self.extra_args = [[], ['-deprecatedrpc=bumpfee']] def run_test(self): # This test should be used to verify correct behaviour of deprecated @@ -23,7 +23,38 @@ class DeprecatedRpcTest(BitcoinTestFramework): # self.log.info("Test generate RPC") # assert_raises_rpc_error(-32, 'The wallet generate rpc method is deprecated', self.nodes[0].rpc.generate, 1) # self.nodes[1].generate(1) - self.log.info("No tested deprecated RPC methods") + + if self.is_wallet_compiled(): + self.log.info("Test bumpfee RPC") + self.nodes[0].generate(101) + self.nodes[0].createwallet(wallet_name='nopriv', disable_private_keys=True) + noprivs0 = self.nodes[0].get_wallet_rpc('nopriv') + w0 = self.nodes[0].get_wallet_rpc('') + self.nodes[1].createwallet(wallet_name='nopriv', disable_private_keys=True) + noprivs1 = self.nodes[1].get_wallet_rpc('nopriv') + + address = w0.getnewaddress() + desc = w0.getaddressinfo(address)['desc'] + change_addr = w0.getrawchangeaddress() + change_desc = w0.getaddressinfo(change_addr)['desc'] + txid = w0.sendtoaddress(address=address, amount=10) + vout = find_vout_for_address(w0, txid, address) + self.nodes[0].generate(1) + rawtx = w0.createrawtransaction([{'txid': txid, 'vout': vout}], {w0.getnewaddress(): 5}, 0, True) + rawtx = w0.fundrawtransaction(rawtx, {'changeAddress': change_addr}) + signed_tx = w0.signrawtransactionwithwallet(rawtx['hex'])['hex'] + + noprivs0.importmulti([{'desc': desc, 'timestamp': 0}, {'desc': change_desc, 'timestamp': 0, 'internal': True}]) + noprivs1.importmulti([{'desc': desc, 'timestamp': 0}, {'desc': change_desc, 'timestamp': 0, 'internal': True}]) + + txid = w0.sendrawtransaction(signed_tx) + self.sync_all() + + assert_raises_rpc_error(-32, 'Using bumpfee with wallets that have private keys disabled is deprecated. Use psbtbumpfee instead or restart bitcoind with -deprecatedrpc=bumpfee. This functionality will be removed in 0.22', noprivs0.bumpfee, txid) + bumped_psbt = noprivs1.bumpfee(txid) + assert 'psbt' in bumped_psbt + else: + self.log.info("No tested deprecated RPC methods") if __name__ == '__main__': DeprecatedRpcTest().main() diff --git a/test/functional/rpc_generate.py b/test/functional/rpc_generate.py new file mode 100755 index 0000000000..9404f1e25e --- /dev/null +++ b/test/functional/rpc_generate.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test generate RPC.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, +) + + +class RPCGenerateTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + + def run_test(self): + message = ( + "generate ( nblocks maxtries ) has been replaced by the -generate " + "cli option. Refer to -help for more information." + ) + + self.log.info("Test rpc generate raises with message to use cli option") + assert_raises_rpc_error(-32601, message, self.nodes[0].rpc.generate) + + self.log.info("Test rpc generate help prints message to use cli option") + assert_equal(message, self.nodes[0].help("generate")) + + self.log.info("Test rpc generate is a hidden command not discoverable in general help") + assert message not in self.nodes[0].help() + + +if __name__ == "__main__": + RPCGenerateTest().main() diff --git a/test/functional/rpc_misc.py b/test/functional/rpc_misc.py index c8517d719e..cc5a264adb 100755 --- a/test/functional/rpc_misc.py +++ b/test/functional/rpc_misc.py @@ -27,8 +27,8 @@ class RpcMiscTest(BitcoinTestFramework): self.log.info("test CHECK_NONFATAL") assert_raises_rpc_error( -1, - "Internal bug detected: 'request.params.size() != 100'", - lambda: node.echo(*[0] * 100), + 'Internal bug detected: \'request.params[9].get_str() != "trigger_internal_bug"\'', + lambda: node.echo(arg9='trigger_internal_bug'), ) self.log.info("test getmemoryinfo") diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 3336246c8b..192b60e5d2 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -22,8 +22,6 @@ from test_framework.util import ( from test_framework.mininode import P2PInterface import test_framework.messages from test_framework.messages import ( - CAddress, - msg_addr, NODE_NETWORK, NODE_WITNESS, ) @@ -131,6 +129,13 @@ class NetTest(BitcoinTestFramework): added_nodes = self.nodes[0].getaddednodeinfo(ip_port) assert_equal(len(added_nodes), 1) assert_equal(added_nodes[0]['addednode'], ip_port) + # check that node cannot be added again + assert_raises_rpc_error(-23, "Node already added", self.nodes[0].addnode, node=ip_port, command='add') + # check that node can be removed + self.nodes[0].addnode(node=ip_port, command='remove') + assert_equal(self.nodes[0].getaddednodeinfo(), []) + # check that trying to remove the node again returns an error + assert_raises_rpc_error(-24, "Node could not be removed", self.nodes[0].addnode, node=ip_port, command='remove') # check that a non-existent node returns an error assert_raises_rpc_error(-24, "Node has not been added", self.nodes[0].getaddednodeinfo, '1.1.1.1') @@ -154,30 +159,34 @@ class NetTest(BitcoinTestFramework): def _test_getnodeaddresses(self): self.nodes[0].add_p2p_connection(P2PInterface()) - # send some addresses to the node via the p2p message addr - msg = msg_addr() + # Add some addresses to the Address Manager over RPC. Due to the way + # bucket and bucket position are calculated, some of these addresses + # will collide. imported_addrs = [] - for i in range(256): - a = "123.123.123.{}".format(i) + for i in range(10000): + first_octet = i >> 8 + second_octet = i % 256 + a = "{}.{}.1.1".format(first_octet, second_octet) imported_addrs.append(a) - addr = CAddress() - addr.time = 100000000 - addr.nServices = NODE_NETWORK | NODE_WITNESS - addr.ip = a - addr.port = 8333 - msg.addrs.append(addr) - self.nodes[0].p2p.send_and_ping(msg) - - # obtain addresses via rpc call and check they were ones sent in before - REQUEST_COUNT = 10 - node_addresses = self.nodes[0].getnodeaddresses(REQUEST_COUNT) - assert_equal(len(node_addresses), REQUEST_COUNT) + self.nodes[0].addpeeraddress(a, 8333) + + # Obtain addresses via rpc call and check they were ones sent in before. + # + # Maximum possible addresses in addrman is 10000, although actual + # number will usually be less due to bucket and bucket position + # collisions. + node_addresses = self.nodes[0].getnodeaddresses(0) + assert_greater_than(len(node_addresses), 5000) + assert_greater_than(10000, len(node_addresses)) for a in node_addresses: - assert_greater_than(a["time"], 1527811200) # 1st June 2018 + assert_greater_than(a["time"], 1527811200) # 1st June 2018 assert_equal(a["services"], NODE_NETWORK | NODE_WITNESS) assert a["address"] in imported_addrs assert_equal(a["port"], 8333) + node_addresses = self.nodes[0].getnodeaddresses(1) + assert_equal(len(node_addresses), 1) + assert_raises_rpc_error(-8, "Address count out of range", self.nodes[0].getnodeaddresses, -1) # addrman's size cannot be known reliably after insertion, as hash collisions may occur diff --git a/test/functional/rpc_signrawtransaction.py b/test/functional/rpc_signrawtransaction.py index 3d08202724..704b65c060 100755 --- a/test/functional/rpc_signrawtransaction.py +++ b/test/functional/rpc_signrawtransaction.py @@ -198,10 +198,30 @@ class SignRawTransactionsTest(BitcoinTestFramework): assert_equal(spending_tx_signed['complete'], True) self.nodes[0].sendrawtransaction(spending_tx_signed['hex']) + def OP_1NEGATE_test(self): + self.log.info("Test OP_1NEGATE (0x4f) satisfies BIP62 minimal push standardness rule") + hex_str = ( + "0200000001FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF" + "FFFFFFFF00000000044F024F9CFDFFFFFF01F0B9F5050000000023210277777777" + "77777777777777777777777777777777777777777777777777777777AC66030000" + ) + prev_txs = [ + { + "txid": "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF", + "vout": 0, + "scriptPubKey": "A914AE44AB6E9AA0B71F1CD2B453B69340E9BFBAEF6087", + "redeemScript": "4F9C", + "amount": 1, + } + ] + txn = self.nodes[0].signrawtransactionwithwallet(hex_str, prev_txs) + assert txn["complete"] + def run_test(self): self.successful_signing_test() self.script_verification_error_test() self.witness_script_test() + self.OP_1NEGATE_test() self.test_with_lock_outputs() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index da956a94de..5207b563a1 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -53,6 +53,7 @@ NODE_NETWORK = (1 << 0) NODE_GETUTXO = (1 << 1) NODE_BLOOM = (1 << 2) NODE_WITNESS = (1 << 3) +NODE_COMPACT_FILTERS = (1 << 6) NODE_NETWORK_LIMITED = (1 << 10) MSG_TX = 1 diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 8f0d45c7f9..5eba554a42 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -650,10 +650,10 @@ class RPCOverloadWrapper(): def __getattr__(self, name): return getattr(self.rpc, name) - def createwallet(self, wallet_name, disable_private_keys=None, blank=None, passphrase='', avoid_reuse=None, descriptors=None): + def createwallet(self, wallet_name, disable_private_keys=None, blank=None, passphrase='', avoid_reuse=None, descriptors=None, load_on_startup=None): if descriptors is None: descriptors = self.descriptors - return self.__getattr__('createwallet')(wallet_name, disable_private_keys, blank, passphrase, avoid_reuse, descriptors) + return self.__getattr__('createwallet')(wallet_name, disable_private_keys, blank, passphrase, avoid_reuse, descriptors, load_on_startup) def importprivkey(self, privkey, label=None, rescan=None): wallet_info = self.getwalletinfo() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index b090e93394..01232bda3c 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -194,6 +194,7 @@ BASE_SCRIPTS = [ 'p2p_eviction.py', 'rpc_signmessage.py', 'rpc_generateblock.py', + 'rpc_generate.py', 'wallet_balance.py', 'feature_nulldummy.py', 'mempool_accept.py', @@ -243,6 +244,7 @@ BASE_SCRIPTS = [ 'p2p_node_network_limited.py', 'p2p_permissions.py', 'feature_blocksdir.py', + 'wallet_startup.py', 'feature_config_args.py', 'feature_settings.py', 'rpc_getdescriptorinfo.py', @@ -713,14 +715,16 @@ class RPCCoverage(): Return a set of currently untested RPC commands. """ - # This is shared from `test/functional/test-framework/coverage.py` + # This is shared from `test/functional/test_framework/coverage.py` reference_filename = 'rpc_interface.txt' coverage_file_prefix = 'coverage.' coverage_ref_filename = os.path.join(self.dir, reference_filename) coverage_filenames = set() all_cmds = set() - covered_cmds = set() + # Consider RPC generate covered, because it is overloaded in + # test_framework/test_node.py and not seen by the coverage check. + covered_cmds = set({'generate'}) if not os.path.isfile(coverage_ref_filename): raise RuntimeError("No coverage reference found") diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index d9a8b58a84..71a1a3f4f6 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -12,7 +12,6 @@ from test_framework.util import ( assert_fee_amount, assert_raises_rpc_error, connect_nodes, - wait_until, ) from test_framework.wallet_util import test_address @@ -540,7 +539,7 @@ class WalletTest(BitcoinTestFramework): self.start_node(2, [m, "-limitancestorcount=" + str(chainlimit)]) if m == '-reindex': # reindex will leave rpc warm up "early"; Wait for it to finish - wait_until(lambda: [block_count] * 3 == [self.nodes[i].getblockcount() for i in range(3)]) + self.wait_until(lambda: [block_count] * 3 == [self.nodes[i].getblockcount() for i in range(3)]) assert_equal(balance_nodes, [self.nodes[i].getbalance() for i in range(3)]) # Exercise listsinceblock with the last two blocks @@ -589,7 +588,7 @@ class WalletTest(BitcoinTestFramework): self.start_node(0, extra_args=extra_args) # wait until the wallet has submitted all transactions to the mempool - wait_until(lambda: len(self.nodes[0].getrawmempool()) == chainlimit * 2) + self.wait_until(lambda: len(self.nodes[0].getrawmempool()) == chainlimit * 2) node0_balance = self.nodes[0].getbalance() # With walletrejectlongchains we will not create the tx and store it in our wallet. diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 0ef78b0e1c..53496084ef 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -123,13 +123,19 @@ def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address): self.sync_mempools((rbf_node, peer_node)) assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool() if mode == "fee_rate": + bumped_psbt = rbf_node.psbtbumpfee(rbfid, {"fee_rate": NORMAL}) bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate": NORMAL}) else: + bumped_psbt = rbf_node.psbtbumpfee(rbfid) bumped_tx = rbf_node.bumpfee(rbfid) assert_equal(bumped_tx["errors"], []) assert bumped_tx["fee"] > -rbftx["fee"] assert_equal(bumped_tx["origfee"], -rbftx["fee"]) assert "psbt" not in bumped_tx + assert_equal(bumped_psbt["errors"], []) + assert bumped_psbt["fee"] > -rbftx["fee"] + assert_equal(bumped_psbt["origfee"], -rbftx["fee"]) + assert "psbt" in bumped_psbt # check that bumped_tx propagates, original tx was evicted and has a wallet conflict self.sync_mempools((rbf_node, peer_node)) assert bumped_tx["txid"] in rbf_node.getrawmempool() @@ -391,7 +397,7 @@ def test_watchonly_psbt(self, peer_node, rbf_node, dest_address): assert_equal(len(watcher.decodepsbt(psbt)["tx"]["vin"]), 1) # Bump fee, obnoxiously high to add additional watchonly input - bumped_psbt = watcher.bumpfee(original_txid, {"fee_rate": HIGH}) + bumped_psbt = watcher.psbtbumpfee(original_txid, {"fee_rate": HIGH}) assert_greater_than(len(watcher.decodepsbt(bumped_psbt['psbt'])["tx"]["vin"]), 1) assert "txid" not in bumped_psbt assert_equal(bumped_psbt["origfee"], -watcher.gettransaction(original_txid)["fee"]) diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py index b6fe295127..cfdc2d51e6 100755 --- a/test/functional/wallet_groups.py +++ b/test/functional/wallet_groups.py @@ -15,8 +15,8 @@ from test_framework.util import ( class WalletGroupTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 3 - self.extra_args = [[], [], ['-avoidpartialspends']] + self.num_nodes = 4 + self.extra_args = [[], [], ['-avoidpartialspends'], ["-maxapsfee=0.0001"]] self.rpc_timeout = 480 def skip_test_if_missing_module(self): @@ -64,6 +64,52 @@ class WalletGroupTest(BitcoinTestFramework): assert_approx(v[0], 0.2) assert_approx(v[1], 1.3, 0.0001) + # Test 'avoid partial if warranted, even if disabled' + self.sync_all() + self.nodes[0].generate(1) + # Nodes 1-2 now have confirmed UTXOs (letters denote destinations): + # Node #1: Node #2: + # - A 1.0 - D0 1.0 + # - B0 1.0 - D1 0.5 + # - B1 0.5 - E0 1.0 + # - C0 1.0 - E1 0.5 + # - C1 0.5 - F ~1.3 + # - D ~0.3 + assert_approx(self.nodes[1].getbalance(), 4.3, 0.0001) + assert_approx(self.nodes[2].getbalance(), 4.3, 0.0001) + # Sending 1.4 btc should pick one 1.0 + one more. For node #1, + # this could be (A / B0 / C0) + (B1 / C1 / D). We ensure that it is + # B0 + B1 or C0 + C1, because this avoids partial spends while not being + # detrimental to transaction cost + txid3 = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1.4) + tx3 = self.nodes[1].getrawtransaction(txid3, True) + # tx3 should have 2 inputs and 2 outputs + assert_equal(2, len(tx3["vin"])) + assert_equal(2, len(tx3["vout"])) + # the accumulated value should be 1.5, so the outputs should be + # ~0.1 and 1.4 and should come from the same destination + values = [vout["value"] for vout in tx3["vout"]] + values.sort() + assert_approx(values[0], 0.1, 0.0001) + assert_approx(values[1], 1.4) + + input_txids = [vin["txid"] for vin in tx3["vin"]] + input_addrs = [self.nodes[1].gettransaction(txid)['details'][0]['address'] for txid in input_txids] + assert_equal(input_addrs[0], input_addrs[1]) + # Node 2 enforces avoidpartialspends so needs no checking here + + # Test wallet option maxapsfee with Node 3 + addr_aps = self.nodes[3].getnewaddress() + self.nodes[0].sendtoaddress(addr_aps, 1.0) + self.nodes[0].sendtoaddress(addr_aps, 1.0) + self.nodes[0].generate(1) + txid4 = self.nodes[3].sendtoaddress(self.nodes[0].getnewaddress(), 0.1) + tx4 = self.nodes[3].getrawtransaction(txid4, True) + # tx4 should have 2 inputs and 2 outputs although one output would + # have been enough and the transaction caused higher fees + assert_equal(2, len(tx4["vin"])) + assert_equal(2, len(tx4["vout"])) + # Empty out node2's wallet self.nodes[2].sendtoaddress(address=self.nodes[0].getnewaddress(), amount=self.nodes[2].getbalance(), subtractfeefromamount=True) self.sync_all() diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index 6d51ca6c93..d4131deabf 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -36,6 +36,7 @@ class ListSinceBlockTest(BitcoinTestFramework): self.test_double_spend() self.test_double_send() self.double_spends_filtered() + self.test_targetconfirmations() def test_no_blockhash(self): self.log.info("Test no blockhash") @@ -74,6 +75,27 @@ class ListSinceBlockTest(BitcoinTestFramework): assert_raises_rpc_error(-8, "blockhash must be hexadecimal string (not 'Z000000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].listsinceblock, "Z000000000000000000000000000000000000000000000000000000000000000") + def test_targetconfirmations(self): + ''' + This tests when the value of target_confirmations exceeds the number of + blocks in the main chain. In this case, the genesis block hash should be + given for the `lastblock` property. If target_confirmations is < 1, then + a -8 invalid parameter error is thrown. + ''' + self.log.info("Test target_confirmations") + blockhash, = self.nodes[2].generate(1) + blockheight = self.nodes[2].getblockheader(blockhash)['height'] + self.sync_all() + + assert_equal( + self.nodes[0].getblockhash(0), + self.nodes[0].listsinceblock(blockhash, blockheight + 1)['lastblock']) + assert_equal( + self.nodes[0].getblockhash(0), + self.nodes[0].listsinceblock(blockhash, blockheight + 1000)['lastblock']) + assert_raises_rpc_error(-8, "Invalid parameter", + self.nodes[0].listsinceblock, blockhash, 0) + def test_reorg(self): ''' `listsinceblock` did not behave correctly when handed a block that was diff --git a/test/functional/wallet_startup.py b/test/functional/wallet_startup.py new file mode 100755 index 0000000000..cfc4edb8ee --- /dev/null +++ b/test/functional/wallet_startup.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017-2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test wallet load on startup. + +Verify that a bitcoind node can maintain list of wallets loading on startup +""" +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, +) + + +class WalletStartupTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + self.supports_cli = True + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def setup_nodes(self): + self.add_nodes(self.num_nodes) + self.start_nodes() + + def run_test(self): + self.nodes[0].createwallet(wallet_name='w0', load_on_startup=True) + self.nodes[0].createwallet(wallet_name='w1', load_on_startup=False) + self.nodes[0].createwallet(wallet_name='w2', load_on_startup=True) + self.nodes[0].createwallet(wallet_name='w3', load_on_startup=False) + self.nodes[0].createwallet(wallet_name='w4', load_on_startup=False) + self.nodes[0].unloadwallet(wallet_name='w0', load_on_startup=False) + self.nodes[0].unloadwallet(wallet_name='w4', load_on_startup=False) + self.nodes[0].loadwallet(filename='w4', load_on_startup=True) + assert_equal(set(self.nodes[0].listwallets()), set(('', 'w1', 'w2', 'w3', 'w4'))) + self.restart_node(0) + assert_equal(set(self.nodes[0].listwallets()), set(('', 'w2', 'w4'))) + self.nodes[0].unloadwallet(wallet_name='', load_on_startup=False) + self.nodes[0].unloadwallet(wallet_name='w4', load_on_startup=False) + self.nodes[0].loadwallet(filename='w3', load_on_startup=True) + self.nodes[0].loadwallet(filename='') + self.restart_node(0) + assert_equal(set(self.nodes[0].listwallets()), set(('w2', 'w3'))) + +if __name__ == '__main__': + WalletStartupTest().main() |