diff options
Diffstat (limited to 'src')
112 files changed, 1109 insertions, 828 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index 1bb887af20..caeaee511f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -133,6 +133,7 @@ BITCOIN_CORE_H = \ clientversion.h \ coins.h \ common/bloom.h \ + common/run_command.h \ compat/assumptions.h \ compat/byteswap.h \ compat/compat.h \ @@ -439,7 +440,7 @@ libbitcoin_node_a_SOURCES += dummywallet.cpp endif if ENABLE_ZMQ -libbitcoin_zmq_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS) $(ZMQ_CFLAGS) +libbitcoin_zmq_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(ZMQ_CFLAGS) libbitcoin_zmq_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libbitcoin_zmq_a_SOURCES = \ zmq/zmqabstractnotifier.cpp \ @@ -616,7 +617,7 @@ libbitcoin_consensus_a_SOURCES = \ version.h # common: shared between bitcoind, and bitcoin-qt and non-server tools -libbitcoin_common_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +libbitcoin_common_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS) libbitcoin_common_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libbitcoin_common_a_SOURCES = \ base58.cpp \ @@ -624,6 +625,7 @@ libbitcoin_common_a_SOURCES = \ chainparams.cpp \ coins.cpp \ common/bloom.cpp \ + common/run_command.cpp \ compressor.cpp \ core_read.cpp \ core_write.cpp \ @@ -927,7 +929,6 @@ libbitcoinkernel_la_SOURCES = \ txdb.cpp \ txmempool.cpp \ uint256.cpp \ - util/bytevectorhash.cpp \ util/check.cpp \ util/getuniquepath.cpp \ util/hasher.cpp \ diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 3ed643d932..e1e2066877 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -53,6 +53,7 @@ nodist_bench_bench_bitcoin_SOURCES = $(GENERATED_BENCH_FILES) bench_bench_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/ bench_bench_bitcoin_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +bench_bench_bitcoin_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS) bench_bench_bitcoin_LDADD = \ $(LIBTEST_UTIL) \ $(LIBBITCOIN_NODE) \ @@ -66,7 +67,9 @@ bench_bench_bitcoin_LDADD = \ $(LIBSECP256K1) \ $(LIBUNIVALUE) \ $(EVENT_PTHREADS_LIBS) \ - $(EVENT_LIBS) + $(EVENT_LIBS) \ + $(MINIUPNPC_LIBS) \ + $(NATPMP_LIBS) if ENABLE_ZMQ bench_bench_bitcoin_LDADD += $(LIBBITCOIN_ZMQ) $(ZMQ_LIBS) @@ -76,11 +79,9 @@ if ENABLE_WALLET bench_bench_bitcoin_SOURCES += bench/coin_selection.cpp bench_bench_bitcoin_SOURCES += bench/wallet_balance.cpp bench_bench_bitcoin_SOURCES += bench/wallet_loading.cpp +bench_bench_bitcoin_LDADD += $(BDB_LIBS) $(SQLITE_LIBS) endif -bench_bench_bitcoin_LDADD += $(BDB_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(MINIUPNPC_LIBS) $(NATPMP_LIBS) $(SQLITE_LIBS) -bench_bench_bitcoin_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS) - CLEAN_BITCOIN_BENCH = bench/*.gcda bench/*.gcno $(GENERATED_BENCH_FILES) CLEANFILES += $(CLEAN_BITCOIN_BENCH) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 5f2e535e85..253f64d2c3 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -174,6 +174,7 @@ BITCOIN_TESTS += \ wallet/test/availablecoins_tests.cpp \ wallet/test/init_tests.cpp \ wallet/test/ismine_tests.cpp \ + wallet/test/rpc_util_tests.cpp \ wallet/test/scriptpubkeyman_tests.cpp \ wallet/test/walletload_tests.cpp @@ -186,7 +187,8 @@ BITCOIN_TESTS += wallet/test/db_tests.cpp endif FUZZ_WALLET_SRC = \ - wallet/test/fuzz/coinselection.cpp + wallet/test/fuzz/coinselection.cpp \ + wallet/test/fuzz/parse_iso8601.cpp if USE_SQLITE FUZZ_WALLET_SRC += \ @@ -288,7 +290,6 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/node_eviction.cpp \ test/fuzz/p2p_transport_serialization.cpp \ test/fuzz/parse_hd_keypath.cpp \ - test/fuzz/parse_iso8601.cpp \ test/fuzz/parse_numbers.cpp \ test/fuzz/parse_script.cpp \ test/fuzz/parse_univalue.cpp \ diff --git a/src/Makefile.test_fuzz.include b/src/Makefile.test_fuzz.include index 11b5c12062..b35d713d57 100644 --- a/src/Makefile.test_fuzz.include +++ b/src/Makefile.test_fuzz.include @@ -10,12 +10,13 @@ EXTRA_LIBRARIES += \ TEST_FUZZ_H = \ test/fuzz/fuzz.h \ test/fuzz/FuzzedDataProvider.h \ - test/fuzz/mempool_utils.h \ - test/fuzz/util.h + test/fuzz/util.h \ + test/fuzz/util/mempool.h libtest_fuzz_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS) libtest_fuzz_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libtest_fuzz_a_SOURCES = \ test/fuzz/fuzz.cpp \ test/fuzz/util.cpp \ + test/fuzz/util/mempool.cpp \ $(TEST_FUZZ_H) diff --git a/src/bech32.cpp b/src/bech32.cpp index dce9b2e4cc..8e0025b8f4 100644 --- a/src/bech32.cpp +++ b/src/bech32.cpp @@ -241,7 +241,7 @@ constexpr std::array<uint32_t, 25> GenerateSyndromeConstants() { std::array<uint32_t, 25> SYNDROME_CONSTS{}; for (int k = 1; k < 6; ++k) { for (int shift = 0; shift < 5; ++shift) { - int16_t b = GF1024_LOG.at(1 << shift); + int16_t b = GF1024_LOG.at(size_t{1} << shift); int16_t c0 = GF1024_EXP.at((997*k + b) % 1023); int16_t c1 = GF1024_EXP.at((998*k + b) % 1023); int16_t c2 = GF1024_EXP.at((999*k + b) % 1023); diff --git a/src/bench/nanobench.h b/src/bench/nanobench.h index 27df08fb69..916a7d61ef 100644 --- a/src/bench/nanobench.h +++ b/src/bench/nanobench.h @@ -858,7 +858,7 @@ public: * @brief Retrieves all benchmark results collected by the bench object so far. * * Each call to run() generates a Result that is stored within the Bench instance. This is mostly for advanced users who want to - * see all the nitty gritty detials. + * see all the nitty gritty details. * * @return All results collected so far. */ diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index d49dc72abf..010cac5920 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -27,9 +27,9 @@ #include <util/system.h> #include <util/translation.h> +#include <cstdio> #include <functional> #include <memory> -#include <stdio.h> static bool fCreateBlank; static std::map<std::string,UniValue> registers; diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index a7d49452b0..d556300ee2 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -50,15 +50,16 @@ static void SetupWalletToolArgs(ArgsManager& argsman) argsman.AddCommand("createfromdump", "Create new wallet file from dumped records"); } -static bool WalletAppInit(ArgsManager& args, int argc, char* argv[]) +static std::optional<int> WalletAppInit(ArgsManager& args, int argc, char* argv[]) { SetupWalletToolArgs(args); std::string error_message; if (!args.ParseParameters(argc, argv, error_message)) { tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error_message); - return false; + return EXIT_FAILURE; } - if (argc < 2 || HelpRequested(args) || args.IsArgSet("-version")) { + const bool missing_args{argc < 2}; + if (missing_args || HelpRequested(args) || args.IsArgSet("-version")) { std::string strUsage = strprintf("%s bitcoin-wallet version", PACKAGE_NAME) + " " + FormatFullVersion() + "\n"; if (args.IsArgSet("-version")) { @@ -73,7 +74,11 @@ static bool WalletAppInit(ArgsManager& args, int argc, char* argv[]) strUsage += "\n" + args.GetHelpMessage(); } tfm::format(std::cout, "%s", strUsage); - return false; + if (missing_args) { + tfm::format(std::cerr, "Error: too few parameters\n"); + return EXIT_FAILURE; + } + return EXIT_SUCCESS; } // check for printtoconsole, allow -debug @@ -81,12 +86,12 @@ static bool WalletAppInit(ArgsManager& args, int argc, char* argv[]) if (!CheckDataDirOption()) { tfm::format(std::cerr, "Error: Specified data directory \"%s\" does not exist.\n", args.GetArg("-datadir", "")); - return false; + return EXIT_FAILURE; } // Check for chain settings (Params() calls are only valid after this clause) SelectParams(args.GetChainName()); - return true; + return std::nullopt; } MAIN_FUNCTION @@ -106,7 +111,7 @@ MAIN_FUNCTION SetupEnvironment(); RandomInit(); try { - if (!WalletAppInit(args, argc, argv)) return EXIT_FAILURE; + if (const auto maybe_exit{WalletAppInit(args, argc, argv)}) return *maybe_exit; } catch (const std::exception& e) { PrintExceptionContinue(&e, "WalletAppInit()"); return EXIT_FAILURE; diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 85ba88f6ab..9f81640ddb 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -216,7 +216,7 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) if (token) { // Success exit(EXIT_SUCCESS); } else { // fRet = false or token read error (premature exit). - tfm::format(std::cerr, "Error during initializaton - check debug.log for details\n"); + tfm::format(std::cerr, "Error during initialization - check debug.log for details\n"); exit(EXIT_FAILURE); } } diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp new file mode 100644 index 0000000000..6ad9f75b5d --- /dev/null +++ b/src/common/run_command.cpp @@ -0,0 +1,64 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#if defined(HAVE_CONFIG_H) +#include <config/bitcoin-config.h> +#endif + +#include <common/run_command.h> + +#include <tinyformat.h> +#include <univalue.h> + +#ifdef ENABLE_EXTERNAL_SIGNER +#if defined(__GNUC__) +// Boost 1.78 requires the following workaround. +// See: https://github.com/boostorg/process/issues/235 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +#endif +#include <boost/process.hpp> +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif +#endif // ENABLE_EXTERNAL_SIGNER + +UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) +{ +#ifdef ENABLE_EXTERNAL_SIGNER + namespace bp = boost::process; + + UniValue result_json; + bp::opstream stdin_stream; + bp::ipstream stdout_stream; + bp::ipstream stderr_stream; + + if (str_command.empty()) return UniValue::VNULL; + + bp::child c( + str_command, + bp::std_out > stdout_stream, + bp::std_err > stderr_stream, + bp::std_in < stdin_stream + ); + if (!str_std_in.empty()) { + stdin_stream << str_std_in << std::endl; + } + stdin_stream.pipe().close(); + + std::string result; + std::string error; + std::getline(stdout_stream, result); + std::getline(stderr_stream, error); + + c.wait(); + const int n_error = c.exit_code(); + if (n_error) throw std::runtime_error(strprintf("RunCommandParseJSON error: process(%s) returned %d: %s\n", str_command, n_error, error)); + if (!result_json.read(result)) throw std::runtime_error("Unable to parse JSON: " + result); + + return result_json; +#else + throw std::runtime_error("Compiled without external signing support (required for external signing)."); +#endif // ENABLE_EXTERNAL_SIGNER +} diff --git a/src/common/run_command.h b/src/common/run_command.h new file mode 100644 index 0000000000..2fbdc071ee --- /dev/null +++ b/src/common/run_command.h @@ -0,0 +1,21 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_COMMON_RUN_COMMAND_H +#define BITCOIN_COMMON_RUN_COMMAND_H + +#include <string> + +class UniValue; + +/** + * Execute a command which returns JSON, and parse the result. + * + * @param str_command The command to execute, including any arguments + * @param str_std_in string to pass to stdin + * @return parsed JSON + */ +UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in=""); + +#endif // BITCOIN_COMMON_RUN_COMMAND_H diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h index b2a31e3ba4..10bdbf31a8 100644 --- a/src/consensus/consensus.h +++ b/src/consensus/consensus.h @@ -6,7 +6,7 @@ #ifndef BITCOIN_CONSENSUS_CONSENSUS_H #define BITCOIN_CONSENSUS_CONSENSUS_H -#include <stdlib.h> +#include <cstdlib> #include <stdint.h> /** The maximum allowed size for a serialized block, in bytes (only for buffer size limits) */ diff --git a/src/crypto/chacha20.h b/src/crypto/chacha20.h index 69fbbe9fa5..de16a77878 100644 --- a/src/crypto/chacha20.h +++ b/src/crypto/chacha20.h @@ -5,8 +5,8 @@ #ifndef BITCOIN_CRYPTO_CHACHA20_H #define BITCOIN_CRYPTO_CHACHA20_H +#include <cstdlib> #include <stdint.h> -#include <stdlib.h> /** A class for ChaCha20 256-bit stream cipher developed by Daniel J. Bernstein https://cr.yp.to/chacha/chacha-20080128.pdf */ diff --git a/src/crypto/hkdf_sha256_32.h b/src/crypto/hkdf_sha256_32.h index fa1e42aec1..878b03a37f 100644 --- a/src/crypto/hkdf_sha256_32.h +++ b/src/crypto/hkdf_sha256_32.h @@ -7,8 +7,8 @@ #include <crypto/hmac_sha256.h> +#include <cstdlib> #include <stdint.h> -#include <stdlib.h> /** A rfc5869 HKDF implementation with HMAC_SHA256 and fixed key output length of 32 bytes (L=32) */ class CHKDF_HMAC_SHA256_L32 diff --git a/src/crypto/hmac_sha256.h b/src/crypto/hmac_sha256.h index d31fda1dd1..9c25edd7c1 100644 --- a/src/crypto/hmac_sha256.h +++ b/src/crypto/hmac_sha256.h @@ -7,8 +7,8 @@ #include <crypto/sha256.h> +#include <cstdlib> #include <stdint.h> -#include <stdlib.h> /** A hasher class for HMAC-SHA-256. */ class CHMAC_SHA256 diff --git a/src/crypto/hmac_sha512.h b/src/crypto/hmac_sha512.h index 1ea9a3671e..6acce8992e 100644 --- a/src/crypto/hmac_sha512.h +++ b/src/crypto/hmac_sha512.h @@ -7,8 +7,8 @@ #include <crypto/sha512.h> +#include <cstdlib> #include <stdint.h> -#include <stdlib.h> /** A hasher class for HMAC-SHA-512. */ class CHMAC_SHA512 diff --git a/src/crypto/poly1305.h b/src/crypto/poly1305.h index 1598b013b9..c80faada7e 100644 --- a/src/crypto/poly1305.h +++ b/src/crypto/poly1305.h @@ -5,8 +5,8 @@ #ifndef BITCOIN_CRYPTO_POLY1305_H #define BITCOIN_CRYPTO_POLY1305_H +#include <cstdlib> #include <stdint.h> -#include <stdlib.h> #define POLY1305_KEYLEN 32 #define POLY1305_TAGLEN 16 diff --git a/src/crypto/ripemd160.h b/src/crypto/ripemd160.h index 38ea375c1f..f1d89b8407 100644 --- a/src/crypto/ripemd160.h +++ b/src/crypto/ripemd160.h @@ -5,8 +5,8 @@ #ifndef BITCOIN_CRYPTO_RIPEMD160_H #define BITCOIN_CRYPTO_RIPEMD160_H +#include <cstdlib> #include <stdint.h> -#include <stdlib.h> /** A hasher class for RIPEMD-160. */ class CRIPEMD160 diff --git a/src/crypto/sha1.h b/src/crypto/sha1.h index 8b4568ee12..6ef0187efd 100644 --- a/src/crypto/sha1.h +++ b/src/crypto/sha1.h @@ -5,8 +5,8 @@ #ifndef BITCOIN_CRYPTO_SHA1_H #define BITCOIN_CRYPTO_SHA1_H +#include <cstdlib> #include <stdint.h> -#include <stdlib.h> /** A hasher class for SHA1. */ class CSHA1 diff --git a/src/crypto/sha256.h b/src/crypto/sha256.h index 028ee14345..24bd1f2e7e 100644 --- a/src/crypto/sha256.h +++ b/src/crypto/sha256.h @@ -5,8 +5,8 @@ #ifndef BITCOIN_CRYPTO_SHA256_H #define BITCOIN_CRYPTO_SHA256_H +#include <cstdlib> #include <stdint.h> -#include <stdlib.h> #include <string> /** A hasher class for SHA-256. */ diff --git a/src/crypto/sha256_sse4.cpp b/src/crypto/sha256_sse4.cpp index f1a7fefea3..bc69703607 100644 --- a/src/crypto/sha256_sse4.cpp +++ b/src/crypto/sha256_sse4.cpp @@ -5,8 +5,8 @@ // This is a translation to GCC extended asm syntax from YASM code by Intel // (available at the bottom of this file). +#include <cstdlib> #include <stdint.h> -#include <stdlib.h> #if defined(__x86_64__) || defined(__amd64__) diff --git a/src/crypto/sha3.h b/src/crypto/sha3.h index 88d8c1204d..78608eae76 100644 --- a/src/crypto/sha3.h +++ b/src/crypto/sha3.h @@ -7,8 +7,8 @@ #include <span.h> +#include <cstdlib> #include <stdint.h> -#include <stdlib.h> //! The Keccak-f[1600] transform. void KeccakF(uint64_t (&st)[25]); diff --git a/src/crypto/sha512.h b/src/crypto/sha512.h index 21ca930c75..7356dff6d9 100644 --- a/src/crypto/sha512.h +++ b/src/crypto/sha512.h @@ -5,8 +5,8 @@ #ifndef BITCOIN_CRYPTO_SHA512_H #define BITCOIN_CRYPTO_SHA512_H +#include <cstdlib> #include <stdint.h> -#include <stdlib.h> /** A hasher class for SHA-512. */ class CSHA512 diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 094314e878..0e582629f7 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -3,10 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <chainparams.h> +#include <common/run_command.h> #include <core_io.h> #include <psbt.h> #include <util/strencodings.h> -#include <util/system.h> #include <external_signer.h> #include <algorithm> diff --git a/src/httpserver.cpp b/src/httpserver.cpp index e68436cc2c..1a19555f76 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -21,11 +21,11 @@ #include <util/threadnames.h> #include <util/translation.h> +#include <cstdio> +#include <cstdlib> #include <deque> #include <memory> #include <optional> -#include <stdio.h> -#include <stdlib.h> #include <string> #include <sys/types.h> @@ -320,7 +320,7 @@ static bool HTTPBindAddresses(struct evhttp* http) // Bind addresses for (std::vector<std::pair<std::string, uint16_t> >::iterator i = endpoints.begin(); i != endpoints.end(); ++i) { - LogPrint(BCLog::HTTP, "Binding RPC on address %s port %i\n", i->first, i->second); + LogPrintf("Binding RPC on address %s port %i\n", i->first, i->second); evhttp_bound_socket *bind_handle = evhttp_bind_socket_with_handle(http, i->first.empty() ? nullptr : i->first.c_str(), i->second); if (bind_handle) { CNetAddr addr; diff --git a/src/index/base.cpp b/src/index/base.cpp index 88c2ce98fa..3eea09b17d 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -298,6 +298,10 @@ void BaseIndex::BlockConnected(const std::shared_ptr<const CBlock>& block, const } interfaces::BlockInfo block_info = kernel::MakeBlockInfo(pindex, block.get()); if (CustomAppend(block_info)) { + // Setting the best block index is intentionally the last step of this + // function, so BlockUntilSyncedToCurrentChain callers waiting for the + // best block index to be updated can rely on the block being fully + // processed, and the index object being safe to delete. SetBestBlockIndex(pindex); } else { FatalError("%s: Failed to write block %s to index", @@ -414,10 +418,17 @@ IndexSummary BaseIndex::GetSummary() const void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) { assert(!node::fPruneMode || AllowPrune()); - m_best_block_index = block; if (AllowPrune() && block) { node::PruneLockInfo prune_lock; prune_lock.height_first = block->nHeight; WITH_LOCK(::cs_main, m_chainstate->m_blockman.UpdatePruneLock(GetName(), prune_lock)); } + + // Intentionally set m_best_block_index as the last step in this function, + // after updating prune locks above, and after making any other references + // to *this, so the BlockUntilSyncedToCurrentChain function (which checks + // m_best_block_index as an optimization) can be used to wait for the last + // BlockConnected notification and safely assume that prune locks are + // updated and that the index object is safe to delete. + m_best_block_index = block; } diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index 5af4671091..9e69388dc8 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -12,6 +12,8 @@ #include <index/base.h> #include <util/hasher.h> +static const char* const DEFAULT_BLOCKFILTERINDEX = "0"; + /** Interval between compact filter checkpoints. See BIP 157. */ static constexpr int CFCHECKPT_INTERVAL = 1000; diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h index fa59cb1ab1..aa0d7f9fd5 100644 --- a/src/index/coinstatsindex.h +++ b/src/index/coinstatsindex.h @@ -14,6 +14,8 @@ namespace kernel { struct CCoinsStats; } +static constexpr bool DEFAULT_COINSTATSINDEX{false}; + /** * CoinStatsIndex maintains statistics on the UTXO set. */ diff --git a/src/index/txindex.h b/src/index/txindex.h index 8c1aa00033..4cea35045d 100644 --- a/src/index/txindex.h +++ b/src/index/txindex.h @@ -7,6 +7,8 @@ #include <index/base.h> +static constexpr bool DEFAULT_TXINDEX{false}; + /** * TxIndex is used to look up transactions included in the blockchain by hash. * The index is written to a LevelDB database and records the filesystem diff --git a/src/init.cpp b/src/init.cpp index 20cb483a3a..25b40c6c6e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -476,7 +476,6 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-onlynet=<net>", "Make automatic outbound connections only to network <net> (" + Join(GetNetworkNames(), ", ") + "). Inbound and manual connections are not affected by this option. It can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); // TODO: remove the sentence "Nodes not using ... incoming connections." once the changes from // https://github.com/bitcoin/bitcoin/pull/23542 have become widespread. argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port>. Nodes not using the default ports (default: %u, testnet: %u, signet: %u, regtest: %u) are unlikely to get incoming connections. Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); @@ -566,6 +565,8 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-datacarriersize", strprintf("Maximum size of data in data carrier transactions we relay and mine (default: %u)", MAX_OP_RETURN_RELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-mempoolfullrbf", strprintf("Accept transaction replace-by-fee without requiring replaceability signaling (default: %u)", DEFAULT_MEMPOOL_FULL_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); + argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, + OptionsCategory::NODE_RELAY); argsman.AddArg("-minrelaytxfee=<amt>", strprintf("Fees (in %s/kvB) smaller than this are considered zero fee for relaying, mining and transaction creation (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_MIN_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-whitelistforcerelay", strprintf("Add 'forcerelay' permission to whitelisted inbound peers with default permissions. This will relay transactions even if the transactions were already in the mempool. (default: %d)", DEFAULT_WHITELISTFORCERELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); @@ -1284,6 +1285,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } if (!args.IsArgSet("-cjdnsreachable")) { + if (args.IsArgSet("-onlynet") && IsReachable(NET_CJDNS)) { + return InitError( + _("Outbound connections restricted to CJDNS (-onlynet=cjdns) but " + "-cjdnsreachable is not provided")); + } SetReachable(NET_CJDNS, false); } // Now IsReachable(NET_CJDNS) is true if: @@ -1756,6 +1762,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } SetProxy(NET_I2P, Proxy{addr}); } else { + if (args.IsArgSet("-onlynet") && IsReachable(NET_I2P)) { + return InitError( + _("Outbound connections restricted to i2p (-onlynet=i2p) but " + "-i2psam is not provided")); + } SetReachable(NET_I2P, false); } diff --git a/src/ipc/interfaces.cpp b/src/ipc/interfaces.cpp index 580590fde9..ee0d4123ce 100644 --- a/src/ipc/interfaces.cpp +++ b/src/ipc/interfaces.cpp @@ -12,11 +12,11 @@ #include <tinyformat.h> #include <util/system.h> +#include <cstdio> +#include <cstdlib> #include <functional> #include <memory> #include <stdexcept> -#include <stdio.h> -#include <stdlib.h> #include <string.h> #include <string> #include <unistd.h> diff --git a/src/ipc/process.cpp b/src/ipc/process.cpp index 9036b80c45..9474ad2c4a 100644 --- a/src/ipc/process.cpp +++ b/src/ipc/process.cpp @@ -10,10 +10,10 @@ #include <util/strencodings.h> #include <cstdint> +#include <cstdlib> #include <exception> #include <iostream> #include <stdexcept> -#include <stdlib.h> #include <string.h> #include <system_error> #include <unistd.h> diff --git a/src/kernel/mempool_limits.h b/src/kernel/mempool_limits.h index e192e7e6cd..8d4495c3cb 100644 --- a/src/kernel/mempool_limits.h +++ b/src/kernel/mempool_limits.h @@ -24,6 +24,15 @@ struct MemPoolLimits { int64_t descendant_count{DEFAULT_DESCENDANT_LIMIT}; //! The maximum allowed size in virtual bytes of an entry and its descendants within a package. int64_t descendant_size_vbytes{DEFAULT_DESCENDANT_SIZE_LIMIT_KVB * 1'000}; + + /** + * @return MemPoolLimits with all the limits set to the maximum + */ + static constexpr MemPoolLimits NoLimits() + { + int64_t no_limit{std::numeric_limits<int64_t>::max()}; + return {no_limit, no_limit, no_limit, no_limit}; + } }; } // namespace kernel diff --git a/src/leveldb/util/env_windows.cc b/src/leveldb/util/env_windows.cc index 4dcba222a1..aafcdcc3be 100644 --- a/src/leveldb/util/env_windows.cc +++ b/src/leveldb/util/env_windows.cc @@ -194,7 +194,7 @@ class WindowsRandomAccessFile : public RandomAccessFile { Status Read(uint64_t offset, size_t n, Slice* result, char* scratch) const override { DWORD bytes_read = 0; - OVERLAPPED overlapped = {0}; + OVERLAPPED overlapped = {}; overlapped.OffsetHigh = static_cast<DWORD>(offset >> 32); overlapped.Offset = static_cast<DWORD>(offset); diff --git a/src/memusage.h b/src/memusage.h index a6e894129a..fd85a7956c 100644 --- a/src/memusage.h +++ b/src/memusage.h @@ -8,9 +8,8 @@ #include <indirectmap.h> #include <prevector.h> -#include <stdlib.h> - #include <cassert> +#include <cstdlib> #include <map> #include <memory> #include <set> diff --git a/src/net.cpp b/src/net.cpp index 0cc14b1d2a..0736f3ef1b 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -91,13 +91,12 @@ static constexpr auto FEELER_SLEEP_WINDOW{1s}; /** Used to pass flags to the Bind() function */ enum BindFlags { BF_NONE = 0, - BF_EXPLICIT = (1U << 0), - BF_REPORT_ERROR = (1U << 1), + BF_REPORT_ERROR = (1U << 0), /** * Do not call AddLocal() for our special addresses, e.g., for incoming * Tor connections, to prevent gossiping them over the network. */ - BF_DONT_ADVERTISE = (1U << 2), + BF_DONT_ADVERTISE = (1U << 1), }; // The set of sockets cannot be modified while waiting @@ -1991,8 +1990,12 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai } } +Mutex NetEventsInterface::g_msgproc_mutex; + void CConnman::ThreadMessageHandler() { + LOCK(NetEventsInterface::g_msgproc_mutex); + SetSyscallSandboxPolicy(SyscallSandboxPolicy::MESSAGE_HANDLER); while (!flagInterruptMsgProc) { @@ -2014,10 +2017,7 @@ void CConnman::ThreadMessageHandler() if (flagInterruptMsgProc) return; // Send messages - { - LOCK(pnode->cs_sendProcessing); - m_msgproc->SendMessages(pnode); - } + m_msgproc->SendMessages(pnode); if (flagInterruptMsgProc) return; @@ -2230,9 +2230,6 @@ bool CConnman::Bind(const CService& addr_, unsigned int flags, NetPermissionFlag { const CService addr{MaybeFlipIPv6toCJDNS(addr_)}; - if (!(flags & BF_EXPLICIT) && !IsReachable(addr)) { - return false; - } bilingual_str strError; if (!BindListenPort(addr, strError, permissions)) { if ((flags & BF_REPORT_ERROR) && m_client_interface) { @@ -2252,13 +2249,13 @@ bool CConnman::InitBinds(const Options& options) { bool fBound = false; for (const auto& addrBind : options.vBinds) { - fBound |= Bind(addrBind, (BF_EXPLICIT | BF_REPORT_ERROR), NetPermissionFlags::None); + fBound |= Bind(addrBind, BF_REPORT_ERROR, NetPermissionFlags::None); } for (const auto& addrBind : options.vWhiteBinds) { - fBound |= Bind(addrBind.m_service, (BF_EXPLICIT | BF_REPORT_ERROR), addrBind.m_flags); + fBound |= Bind(addrBind.m_service, BF_REPORT_ERROR, addrBind.m_flags); } for (const auto& addr_bind : options.onion_binds) { - fBound |= Bind(addr_bind, BF_EXPLICIT | BF_DONT_ADVERTISE, NetPermissionFlags::None); + fBound |= Bind(addr_bind, BF_DONT_ADVERTISE, NetPermissionFlags::None); } if (options.bind_on_any) { struct in_addr inaddr_any; @@ -164,6 +164,7 @@ bool SeenLocal(const CService& addr); bool IsLocal(const CService& addr); bool GetLocal(CService &addr, const CNetAddr *paddrPeer = nullptr); CService GetLocalAddress(const CNetAddr& addrPeer); +CService MaybeFlipIPv6toCJDNS(const CService& service); extern bool fDiscover; @@ -377,8 +378,6 @@ public: std::list<CNetMessage> vProcessMsg GUARDED_BY(cs_vProcessMsg); size_t nProcessQueueSize GUARDED_BY(cs_vProcessMsg){0}; - RecursiveMutex cs_sendProcessing; - uint64_t nRecvBytes GUARDED_BY(cs_vRecv){0}; std::atomic<std::chrono::seconds> m_last_send{0s}; @@ -629,6 +628,9 @@ private: class NetEventsInterface { public: + /** Mutex for anything that is only accessed via the msg processing thread */ + static Mutex g_msgproc_mutex; + /** Initialize a peer (setup state, queue any initial messages) */ virtual void InitializeNode(CNode& node, ServiceFlags our_services) = 0; @@ -642,7 +644,7 @@ public: * @param[in] interrupt Interrupt condition for processing threads * @return True if there is more work to be done */ - virtual bool ProcessMessages(CNode* pnode, std::atomic<bool>& interrupt) = 0; + virtual bool ProcessMessages(CNode* pnode, std::atomic<bool>& interrupt) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; /** * Send queued protocol messages to a given node. @@ -650,7 +652,7 @@ public: * @param[in] pnode The node which we are sending messages to. * @return True if there is more work to be done */ - virtual bool SendMessages(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(pnode->cs_sendProcessing) = 0; + virtual bool SendMessages(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; protected: diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 74700580ad..eca6263392 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -264,10 +264,10 @@ struct Peer { /** The feerate in the most recent BIP133 `feefilter` message sent to the peer. * It is *not* a p2p protocol violation for the peer to send us * transactions with a lower fee rate than this. See BIP133. */ - CAmount m_fee_filter_sent{0}; + CAmount m_fee_filter_sent GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; /** Timestamp after which we will send the next BIP133 `feefilter` message * to the peer. */ - std::chrono::microseconds m_next_send_feefilter{0}; + std::chrono::microseconds m_next_send_feefilter GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; struct TxRelay { mutable RecursiveMutex m_bloom_filter_mutex; @@ -298,7 +298,7 @@ struct Peer { std::atomic<std::chrono::seconds> m_last_mempool_req{0s}; /** The next time after which we will send an `inv` message containing * transaction announcements to this peer. */ - std::chrono::microseconds m_next_inv_send_time{0}; + std::chrono::microseconds m_next_inv_send_time GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; /** Minimum fee rate with which to filter transaction announcements to this node. See BIP133. */ std::atomic<CAmount> m_fee_filter_received{0}; @@ -319,7 +319,7 @@ struct Peer { }; /** A vector of addresses to send to the peer, limited to MAX_ADDR_TO_SEND. */ - std::vector<CAddress> m_addrs_to_send; + std::vector<CAddress> m_addrs_to_send GUARDED_BY(NetEventsInterface::g_msgproc_mutex); /** Probabilistic filter to track recent addr messages relayed with this * peer. Used to avoid relaying redundant addresses to this peer. * @@ -329,7 +329,7 @@ struct Peer { * * Presence of this filter must correlate with m_addr_relay_enabled. **/ - std::unique_ptr<CRollingBloomFilter> m_addr_known; + std::unique_ptr<CRollingBloomFilter> m_addr_known GUARDED_BY(NetEventsInterface::g_msgproc_mutex); /** Whether we are participating in address relay with this connection. * * We set this bool to true for outbound peers (other than @@ -346,7 +346,7 @@ struct Peer { * initialized.*/ std::atomic_bool m_addr_relay_enabled{false}; /** Whether a getaddr request to this peer is outstanding. */ - bool m_getaddr_sent{false}; + bool m_getaddr_sent GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; /** Guards address sending timers. */ mutable Mutex m_addr_send_times_mutex; /** Time point to send the next ADDR message to this peer. */ @@ -357,12 +357,12 @@ struct Peer { * messages, indicating a preference to receive ADDRv2 instead of ADDR ones. */ std::atomic_bool m_wants_addrv2{false}; /** Whether this peer has already sent us a getaddr message. */ - bool m_getaddr_recvd{false}; + bool m_getaddr_recvd GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; /** Number of addresses that can be processed from this peer. Start at 1 to * permit self-announcement. */ - double m_addr_token_bucket{1.0}; + double m_addr_token_bucket GUARDED_BY(NetEventsInterface::g_msgproc_mutex){1.0}; /** When m_addr_token_bucket was last updated */ - std::chrono::microseconds m_addr_token_timestamp{GetTime<std::chrono::microseconds>()}; + std::chrono::microseconds m_addr_token_timestamp GUARDED_BY(NetEventsInterface::g_msgproc_mutex){GetTime<std::chrono::microseconds>()}; /** Total number of addresses that were dropped due to rate limiting. */ std::atomic<uint64_t> m_addr_rate_limited{0}; /** Total number of addresses that were processed (excludes rate-limited ones). */ @@ -372,7 +372,7 @@ struct Peer { std::set<uint256> m_orphan_work_set GUARDED_BY(g_cs_orphans); /** Whether we've sent this peer a getheaders in response to an inv prior to initial-headers-sync completing */ - bool m_inv_triggered_getheaders_before_sync{false}; + bool m_inv_triggered_getheaders_before_sync GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; /** Protects m_getdata_requests **/ Mutex m_getdata_requests_mutex; @@ -380,7 +380,7 @@ struct Peer { std::deque<CInv> m_getdata_requests GUARDED_BY(m_getdata_requests_mutex); /** Time of the last getheaders message to this peer */ - NodeClock::time_point m_last_getheaders_timestamp{}; + NodeClock::time_point m_last_getheaders_timestamp GUARDED_BY(NetEventsInterface::g_msgproc_mutex){}; /** Protects m_headers_sync **/ Mutex m_headers_sync_mutex; @@ -515,9 +515,9 @@ public: void InitializeNode(CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex); bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex); - bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing) - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex); + bool SendMessages(CNode* pto) override + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, g_msgproc_mutex); /** Implement PeerManager */ void StartScheduledTasks(CScheduler& scheduler) override; @@ -532,12 +532,12 @@ public: void UnitTestMisbehaving(NodeId peer_id, int howmuch) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex) { Misbehaving(*Assert(GetPeerRef(peer_id)), howmuch, ""); }; void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) override - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex); void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override; private: /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */ - void ConsiderEviction(CNode& pto, Peer& peer, std::chrono::seconds time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void ConsiderEviction(CNode& pto, Peer& peer, std::chrono::seconds time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_msgproc_mutex); /** If we have extra outbound peers, try to disconnect the one with the oldest block announcement */ void EvictExtraOutboundPeers(std::chrono::seconds now) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -601,7 +601,7 @@ private: void ProcessHeadersMessage(CNode& pfrom, Peer& peer, std::vector<CBlockHeader>&& headers, bool via_compact_block) - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex, g_msgproc_mutex); /** Various helpers for headers processing, invoked by ProcessHeadersMessage() */ /** Return true if headers are continuous and have valid proof-of-work (DoS points assigned on failure) */ bool CheckHeadersPoW(const std::vector<CBlockHeader>& headers, const Consensus::Params& consensusParams, Peer& peer); @@ -610,7 +610,7 @@ private: /** Deal with state tracking and headers sync for peers that send the * occasional non-connecting header (this can happen due to BIP 130 headers * announcements for blocks interacting with the 2hr (MAX_FUTURE_BLOCK_TIME) rule). */ - void HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer, const std::vector<CBlockHeader>& headers); + void HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer, const std::vector<CBlockHeader>& headers) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); /** Return true if the headers connect to each other, false otherwise */ bool CheckHeadersAreContinuous(const std::vector<CBlockHeader>& headers) const; /** Try to continue a low-work headers sync that has already begun. @@ -633,7 +633,7 @@ private: */ bool IsContinuationOfLowWorkHeadersSync(Peer& peer, CNode& pfrom, std::vector<CBlockHeader>& headers) - EXCLUSIVE_LOCKS_REQUIRED(peer.m_headers_sync_mutex, !m_headers_presync_mutex); + EXCLUSIVE_LOCKS_REQUIRED(peer.m_headers_sync_mutex, !m_headers_presync_mutex, g_msgproc_mutex); /** Check work on a headers chain to be processed, and if insufficient, * initiate our anti-DoS headers sync mechanism. * @@ -649,7 +649,7 @@ private: bool TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlockIndex* chain_start_header, std::vector<CBlockHeader>& headers) - EXCLUSIVE_LOCKS_REQUIRED(!peer.m_headers_sync_mutex, !m_peer_mutex, !m_headers_presync_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!peer.m_headers_sync_mutex, !m_peer_mutex, !m_headers_presync_mutex, g_msgproc_mutex); /** Return true if the given header is an ancestor of * m_chainman.m_best_header or our current tip */ @@ -659,7 +659,7 @@ private: * We don't issue a getheaders message if we have a recent one outstanding. * This returns true if a getheaders is actually sent, and false otherwise. */ - bool MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer); + bool MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); /** Potentially fetch blocks from this peer upon receipt of a new headers tip */ void HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex* pindexLast); /** Update peer state based on received headers message */ @@ -683,10 +683,10 @@ private: void MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::microseconds now); /** Send `addr` messages on a regular schedule. */ - void MaybeSendAddr(CNode& node, Peer& peer, std::chrono::microseconds current_time); + void MaybeSendAddr(CNode& node, Peer& peer, std::chrono::microseconds current_time) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); /** Send a single `sendheaders` message, after we have completed headers sync with a peer. */ - void MaybeSendSendHeaders(CNode& node, Peer& peer); + void MaybeSendSendHeaders(CNode& node, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); /** Relay (gossip) an address to a few randomly chosen nodes. * @@ -695,10 +695,10 @@ private: * @param[in] fReachable Whether the address' network is reachable. We relay unreachable * addresses less. */ - void RelayAddress(NodeId originator, const CAddress& addr, bool fReachable) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void RelayAddress(NodeId originator, const CAddress& addr, bool fReachable) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex); /** Send `feefilter` message. */ - void MaybeSendFeefilter(CNode& node, Peer& peer, std::chrono::microseconds current_time); + void MaybeSendFeefilter(CNode& node, Peer& peer, std::chrono::microseconds current_time) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); const CChainParams& m_chainparams; CConnman& m_connman; @@ -751,7 +751,7 @@ private: int nSyncStarted GUARDED_BY(cs_main) = 0; /** Hash of the last block we received via INV */ - uint256 m_last_block_inv_triggering_headers_sync{}; + uint256 m_last_block_inv_triggering_headers_sync GUARDED_BY(g_msgproc_mutex){}; /** * Sources of received blocks, saved to be able punish them when processing @@ -863,7 +863,7 @@ private: std::atomic_bool m_headers_presync_should_signal{false}; /** Height of the highest block announced using BIP 152 high-bandwidth mode. */ - int m_highest_fast_announce{0}; + int m_highest_fast_announce GUARDED_BY(::cs_main){0}; /** Have we requested this block from a peer */ bool IsBlockRequested(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -1010,7 +1010,10 @@ private: * @return True if address relay is enabled with peer * False if address relay is disallowed */ - bool SetupAddressRelay(const CNode& node, Peer& peer); + bool SetupAddressRelay(const CNode& node, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); + + void AddAddressKnown(Peer& peer, const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); + void PushAddress(Peer& peer, const CAddress& addr, FastRandomContext& insecure_rand) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); }; const CNodeState* PeerManagerImpl::State(NodeId pnode) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) @@ -1036,13 +1039,13 @@ static bool IsAddrCompatible(const Peer& peer, const CAddress& addr) return peer.m_wants_addrv2 || addr.IsAddrV1Compatible(); } -static void AddAddressKnown(Peer& peer, const CAddress& addr) +void PeerManagerImpl::AddAddressKnown(Peer& peer, const CAddress& addr) { assert(peer.m_addr_known); peer.m_addr_known->insert(addr.GetKey()); } -static void PushAddress(Peer& peer, const CAddress& addr, FastRandomContext& insecure_rand) +void PeerManagerImpl::PushAddress(Peer& peer, const CAddress& addr, FastRandomContext& insecure_rand) { // Known checking here is only to save space from duplicates. // Before sending, we'll filter it again for known addresses that were @@ -2843,7 +2846,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, // If we don't have the last header, then this peer will have given us // something new (if these headers are valid). - bool received_new_header{last_received_header != nullptr}; + bool received_new_header{last_received_header == nullptr}; // Now process all the headers. BlockValidationState state; @@ -3135,6 +3138,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) { + AssertLockHeld(g_msgproc_mutex); + LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(msg_type), vRecv.size(), pfrom.GetId()); PeerRef peer = GetPeerRef(pfrom.GetId()); @@ -4748,6 +4753,8 @@ bool PeerManagerImpl::MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer) bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interruptMsgProc) { + AssertLockHeld(g_msgproc_mutex); + bool fMoreWork = false; PeerRef peer = GetPeerRef(pfrom->GetId()); @@ -5099,7 +5106,7 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros // Remove addr records that the peer already knows about, and add new // addrs to the m_addr_known filter on the same pass. - auto addr_already_known = [&peer](const CAddress& addr) { + auto addr_already_known = [&peer](const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) { bool ret = peer.m_addr_known->contains(addr.GetKey()); if (!ret) peer.m_addr_known->insert(addr.GetKey()); return ret; @@ -5240,6 +5247,8 @@ bool PeerManagerImpl::SetupAddressRelay(const CNode& node, Peer& peer) bool PeerManagerImpl::SendMessages(CNode* pto) { + AssertLockHeld(g_msgproc_mutex); + PeerRef peer = GetPeerRef(pto->GetId()); if (!peer) return false; const Consensus::Params& consensusParams = m_chainparams.GetConsensus(); diff --git a/src/net_processing.h b/src/net_processing.h index 0a882b1e53..0d0842fa8e 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -84,7 +84,7 @@ public: /** Process a single message from a peer. Public for fuzz testing */ virtual void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, - const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) = 0; + const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; /** This function is used for testing the stale tip eviction logic, see denialofservice_tests.cpp */ virtual void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) = 0; diff --git a/src/node/caches.cpp b/src/node/caches.cpp index f168332ee6..a39ad7aeb6 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -4,9 +4,9 @@ #include <node/caches.h> +#include <index/txindex.h> #include <txdb.h> #include <util/system.h> -#include <validation.h> namespace node { CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index aa7ddec770..8a0011a629 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -660,8 +660,7 @@ public: std::string unused_error_string; LOCK(m_node.mempool->cs); return m_node.mempool->CalculateMemPoolAncestors( - entry, ancestors, limits.ancestor_count, limits.ancestor_size_vbytes, - limits.descendant_count, limits.descendant_size_vbytes, unused_error_string); + entry, ancestors, limits, unused_error_string); } CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override { diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 04f28630d1..e11ec5b0f1 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -260,13 +260,9 @@ static int UpdatePackagesForAdded(const CTxMemPool& mempool, modtxiter mit = mapModifiedTx.find(desc); if (mit == mapModifiedTx.end()) { CTxMemPoolModifiedEntry modEntry(desc); - modEntry.nSizeWithAncestors -= it->GetTxSize(); - modEntry.nModFeesWithAncestors -= it->GetModifiedFee(); - modEntry.nSigOpCostWithAncestors -= it->GetSigOpCost(); - mapModifiedTx.insert(modEntry); - } else { - mapModifiedTx.modify(mit, update_for_parent_inclusion(it)); + mit = mapModifiedTx.insert(modEntry).first; } + mapModifiedTx.modify(mit, update_for_parent_inclusion(it)); } } return nDescendantsUpdated; @@ -399,9 +395,8 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele } CTxMemPool::setEntries ancestors; - uint64_t nNoLimit = std::numeric_limits<uint64_t>::max(); std::string dummy; - mempool.CalculateMemPoolAncestors(*iter, ancestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false); + mempool.CalculateMemPoolAncestors(*iter, ancestors, CTxMemPool::Limits::NoLimits(), dummy, false); onlyUnconfirmed(ancestors); ancestors.insert(iter); diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 6098caced9..55f47f485b 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -36,10 +36,9 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) // If all the inputs have nSequence >= maxint-1, it still might be // signaled for RBF if any unconfirmed parents have signaled. - uint64_t noLimit = std::numeric_limits<uint64_t>::max(); std::string dummy; CTxMemPoolEntry entry = *pool.mapTx.find(tx.GetHash()); - pool.CalculateMemPoolAncestors(entry, ancestors, noLimit, noLimit, noLimit, noLimit, dummy, false); + pool.CalculateMemPoolAncestors(entry, ancestors, CTxMemPool::Limits::NoLimits(), dummy, false); for (CTxMemPool::txiter it : ancestors) { if (SignalsOptInRBF(it->GetTx())) { diff --git a/src/prevector.h b/src/prevector.h index a52510930a..7df5a067a2 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -6,7 +6,7 @@ #define BITCOIN_PREVECTOR_H #include <assert.h> -#include <stdlib.h> +#include <cstdlib> #include <stdint.h> #include <string.h> diff --git a/src/qt/utilitydialog.cpp b/src/qt/utilitydialog.cpp index 4894cac905..331487b51d 100644 --- a/src/qt/utilitydialog.cpp +++ b/src/qt/utilitydialog.cpp @@ -17,7 +17,7 @@ #include <util/system.h> #include <util/strencodings.h> -#include <stdio.h> +#include <cstdio> #include <QCloseEvent> #include <QLabel> diff --git a/src/random.cpp b/src/random.cpp index f92e679a00..eab54630b1 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -21,7 +21,7 @@ #include <util/time.h> // for GetTimeMicros() #include <cmath> -#include <stdlib.h> +#include <cstdlib> #include <thread> #ifndef WIN32 diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index d1daf06732..93d11c8276 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -856,7 +856,7 @@ static RPCHelpMan gettxoutsetinfo() "Note this call may take some time if you are not using coinstatsindex.\n", { {"hash_type", RPCArg::Type::STR, RPCArg::Default{"hash_serialized_2"}, "Which UTXO set hash should be calculated. Options: 'hash_serialized_2' (the legacy algorithm), 'muhash', 'none'."}, - {"hash_or_height", RPCArg::Type::NUM, RPCArg::DefaultHint{"the current best block"}, "The block hash or height of the target height (only available with coinstatsindex).", "", {"", "string or numeric"}}, + {"hash_or_height", RPCArg::Type::NUM, RPCArg::DefaultHint{"the current best block"}, "The block hash or height of the target height (only available with coinstatsindex).", RPCArgOptions{.type_str={"", "string or numeric"}}}, {"use_index", RPCArg::Type::BOOL, RPCArg::Default{true}, "Use coinstatsindex, if available."}, }, RPCResult{ @@ -1726,13 +1726,13 @@ static RPCHelpMan getblockstats() "\nCompute per block statistics for a given window. All amounts are in satoshis.\n" "It won't work for some heights with pruning.\n", { - {"hash_or_height", RPCArg::Type::NUM, RPCArg::Optional::NO, "The block hash or height of the target block", "", {"", "string or numeric"}}, + {"hash_or_height", RPCArg::Type::NUM, RPCArg::Optional::NO, "The block hash or height of the target block", RPCArgOptions{.type_str={"", "string or numeric"}}}, {"stats", RPCArg::Type::ARR, RPCArg::DefaultHint{"all values"}, "Values to plot (see result below)", { {"height", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Selected statistic"}, {"time", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Selected statistic"}, }, - "stats"}, + RPCArgOptions{.oneline_description="stats"}}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -2052,7 +2052,7 @@ static RPCHelpMan scantxoutset() {"range", RPCArg::Type::RANGE, RPCArg::Default{1000}, "The range of HD chain indexes to explore (either end or [begin,end])"}, }}, }, - "[scanobjects,...]"}, + RPCArgOptions{.oneline_description="[scanobjects,...]"}}, }, { RPCResult{"when action=='start'; only returns after scan completes", RPCResult::Type::OBJ, "", "", { diff --git a/src/rpc/fees.cpp b/src/rpc/fees.cpp index aa047bdea8..e50bf00473 100644 --- a/src/rpc/fees.cpp +++ b/src/rpc/fees.cpp @@ -64,7 +64,6 @@ static RPCHelpMan estimatesmartfee() [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VSTR}); - RPCTypeCheckArgument(request.params[0], UniValue::VNUM); CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context); const NodeContext& node = EnsureAnyNodeContext(request.context); @@ -157,7 +156,6 @@ static RPCHelpMan estimaterawfee() [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VNUM}, true); - RPCTypeCheckArgument(request.params[0], UniValue::VNUM); CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context); diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 5c1770704d..706d783942 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -449,9 +449,8 @@ static RPCHelpMan getmempoolancestors() } CTxMemPool::setEntries setAncestors; - uint64_t noLimit = std::numeric_limits<uint64_t>::max(); std::string dummy; - mempool.CalculateMemPoolAncestors(*it, setAncestors, noLimit, noLimit, noLimit, noLimit, dummy, false); + mempool.CalculateMemPoolAncestors(*it, setAncestors, CTxMemPool::Limits::NoLimits(), dummy, false); if (!fVerbose) { UniValue o(UniValue::VARR); @@ -605,8 +604,7 @@ static RPCHelpMan gettxspendingprevout() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - RPCTypeCheckArgument(request.params[0], UniValue::VARR); - const UniValue& output_params = request.params[0]; + const UniValue& output_params = request.params[0].get_array(); if (output_params.empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, outputs are missing"); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index f3120d4599..98383fdaca 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -524,7 +524,7 @@ static RPCHelpMan getblocktemplate() {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "other client side supported softfork deployment"}, }}, }, - "\"template_request\""}, + RPCArgOptions{.oneline_description="\"template_request\""}}, }, { RPCResult{"If the proposal was accepted with mode=='proposal'", RPCResult::Type::NONE, "", ""}, diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index e221b3462d..d701a180ab 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -947,7 +947,8 @@ static RPCHelpMan addpeeraddress() bool success{false}; if (LookupHost(addr_string, net_addr, false)) { - CAddress address{{net_addr, port}, ServiceFlags{NODE_NETWORK | NODE_WITNESS}}; + CService service{net_addr, port}; + CAddress address{MaybeFlipIPv6toCJDNS(service), ServiceFlags{NODE_NETWORK | NODE_WITNESS}}; address.nTime = Now<NodeSeconds>(); // The source address is set equal to the address. This is equivalent to the peer // announcing itself. diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 83a352dbea..1d7bd2eb94 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -168,7 +168,7 @@ static RPCHelpMan stop() // to the client (intended for testing) "\nRequest a graceful shutdown of " PACKAGE_NAME ".", { - {"wait", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "how long to wait in ms", "", {}, /*hidden=*/true}, + {"wait", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "how long to wait in ms", RPCArgOptions{.hidden=true}}, }, RPCResult{RPCResult::Type::STR, "", "A string with the content '" + RESULT + "'"}, RPCExamples{""}, @@ -466,8 +466,7 @@ UniValue CRPCTable::execute(const JSONRPCRequest &request) const static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler) { - try - { + try { RPCCommandExecution execution(request.strMethod); // Execute, convert arguments to array if necessary if (request.params.isObject()) { @@ -475,9 +474,9 @@ static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& req } else { return command.actor(request, result, last_handler); } - } - catch (const std::exception& e) - { + } catch (const UniValue::type_error& e) { + throw JSONRPCError(RPC_TYPE_ERROR, e.what()); + } catch (const std::exception& e) { throw JSONRPCError(RPC_MISC_ERROR, e.what()); } } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 43935650fa..3e98e89791 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -418,8 +418,8 @@ struct Sections { case RPCArg::Type::BOOL: { if (outer_type == OuterType::NONE) return; // Nothing more to do for non-recursive types on first recursion auto left = indent; - if (arg.m_type_str.size() != 0 && push_name) { - left += "\"" + arg.GetName() + "\": " + arg.m_type_str.at(0); + if (arg.m_opts.type_str.size() != 0 && push_name) { + left += "\"" + arg.GetName() + "\": " + arg.m_opts.type_str.at(0); } else { left += push_name ? arg.ToStringObj(/*oneline=*/false) : arg.ToString(/*oneline=*/false); } @@ -618,7 +618,7 @@ std::string RPCHelpMan::ToString() const ret += m_name; bool was_optional{false}; for (const auto& arg : m_args) { - if (arg.m_hidden) break; // Any arg that follows is also hidden + if (arg.m_opts.hidden) break; // Any arg that follows is also hidden const bool optional = arg.IsOptional(); ret += " "; if (optional) { @@ -639,7 +639,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) break; // Any arg that follows is also hidden + if (arg.m_opts.hidden) break; // Any arg that follows is also hidden if (i == 0) ret += "\nArguments:\n"; @@ -704,8 +704,8 @@ std::string RPCArg::ToDescriptionString() const { std::string ret; ret += "("; - if (m_type_str.size() != 0) { - ret += m_type_str.at(1); + if (m_opts.type_str.size() != 0) { + ret += m_opts.type_str.at(1); } else { switch (m_type) { case Type::STR_HEX: @@ -991,7 +991,7 @@ std::string RPCArg::ToStringObj(const bool oneline) const std::string RPCArg::ToString(const bool oneline) const { - if (oneline && !m_oneline_description.empty()) return m_oneline_description; + if (oneline && !m_opts.oneline_description.empty()) return m_opts.oneline_description; switch (m_type) { case Type::STR_HEX: diff --git a/src/rpc/util.h b/src/rpc/util.h index e883dc008e..9aa5df00b1 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -137,6 +137,12 @@ enum class OuterType { NONE, // Only set on first recursion }; +struct RPCArgOptions { + std::string oneline_description{}; //!< Should be empty unless it is supposed to override the auto-generated summary line + std::vector<std::string> type_str{}; //!< Should be empty unless it is supposed to override the auto-generated type strings. Vector length is either 0 or 2, m_opts.type_str.at(0) will override the type of the value in a key-value pair, m_opts.type_str.at(1) will override the type in the argument description. + bool hidden{false}; //!< For testing only +}; + struct RPCArg { enum class Type { OBJ, @@ -169,30 +175,25 @@ struct RPCArg { using DefaultHint = std::string; using Default = UniValue; using Fallback = std::variant<Optional, /* hint for default value */ DefaultHint, /* default constant value */ Default>; + const std::string m_names; //!< The name of the arg (can be empty for inner args, can contain multiple aliases separated by | for named request arguments) const Type m_type; - const bool m_hidden; const std::vector<RPCArg> m_inner; //!< Only used for arrays or dicts const Fallback m_fallback; const std::string m_description; - const std::string m_oneline_description; //!< Should be empty unless it is supposed to override the auto-generated summary line - const std::vector<std::string> m_type_str; //!< Should be empty unless it is supposed to override the auto-generated type strings. Vector length is either 0 or 2, m_type_str.at(0) will override the type of the value in a key-value pair, m_type_str.at(1) will override the type in the argument description. + const RPCArgOptions m_opts; RPCArg( const std::string name, const Type type, const Fallback fallback, const std::string description, - const std::string oneline_description = "", - const std::vector<std::string> type_str = {}, - const bool hidden = false) + RPCArgOptions opts = {}) : m_names{std::move(name)}, m_type{std::move(type)}, - m_hidden{hidden}, m_fallback{std::move(fallback)}, m_description{std::move(description)}, - m_oneline_description{std::move(oneline_description)}, - m_type_str{std::move(type_str)} + m_opts{std::move(opts)} { CHECK_NONFATAL(type != Type::ARR && type != Type::OBJ && type != Type::OBJ_USER_KEYS); } @@ -203,16 +204,13 @@ struct RPCArg { const Fallback fallback, const std::string description, const std::vector<RPCArg> inner, - const std::string oneline_description = "", - const std::vector<std::string> type_str = {}) + RPCArgOptions opts = {}) : m_names{std::move(name)}, m_type{std::move(type)}, - m_hidden{false}, m_inner{std::move(inner)}, m_fallback{std::move(fallback)}, m_description{std::move(description)}, - m_oneline_description{std::move(oneline_description)}, - m_type_str{std::move(type_str)} + m_opts{std::move(opts)} { CHECK_NONFATAL(type == Type::ARR || type == Type::OBJ || type == Type::OBJ_USER_KEYS); } @@ -227,7 +225,7 @@ struct RPCArg { /** * Return the type string of the argument. - * Set oneline to allow it to be overridden by a custom oneline type string (m_oneline_description). + * Set oneline to allow it to be overridden by a custom oneline type string (m_opts.oneline_description). */ std::string ToString(bool oneline) const; /** diff --git a/src/script/miniscript.h b/src/script/miniscript.h index f783d1dafc..6faf2624fd 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -13,8 +13,8 @@ #include <string> #include <vector> -#include <stdlib.h> #include <assert.h> +#include <cstdlib> #include <policy/policy.h> #include <primitives/transaction.h> @@ -276,6 +276,8 @@ struct StackSize { StackSize(MaxInt<uint32_t> in_sat, MaxInt<uint32_t> in_dsat) : sat(in_sat), dsat(in_dsat) {}; }; +struct NoDupCheck {}; + } // namespace internal //! A node in a miniscript expression. @@ -301,8 +303,13 @@ private: const Type typ; //! Cached script length (computed by CalcScriptLen). const size_t scriptlen; - //! Whether a public key appears more than once in this node. - const bool duplicate_key; + //! Whether a public key appears more than once in this node. This value is initialized + //! by all constructors except the NoDupCheck ones. The NoDupCheck ones skip the + //! computation, requiring it to be done manually by invoking DuplicateKeyCheck(). + //! DuplicateKeyCheck(), or a non-NoDupCheck constructor, will compute has_duplicate_keys + //! for all subnodes as well. + mutable std::optional<bool> has_duplicate_keys; + //! Compute the length of the script for this miniscript (including children). size_t CalcScriptLen() const { @@ -654,6 +661,7 @@ public: return TreeEvalMaybe<std::string>(false, downfn, upfn); } +private: internal::Ops CalcOps() const { switch (fragment) { case Fragment::JUST_1: return {0, 0, {}}; @@ -777,11 +785,14 @@ public: assert(false); } - /** Check whether any key is repeated. +public: + /** Update duplicate key information in this Node. + * * This uses a custom key comparator provided by the context in order to still detect duplicates * for more complicated types. */ - template<typename Ctx> bool ContainsDuplicateKey(const Ctx& ctx) const { + template<typename Ctx> void DuplicateKeyCheck(const Ctx& ctx) const + { // We cannot use a lambda here, as lambdas are non assignable, and the set operations // below require moving the comparators around. struct Comp { @@ -789,31 +800,55 @@ public: Comp(const Ctx& ctx) : ctx_ptr(&ctx) {} bool operator()(const Key& a, const Key& b) const { return ctx_ptr->KeyCompare(a, b); } }; - using set = std::set<Key, Comp>; - auto upfn = [this, &ctx](const Node& node, Span<set> subs) -> std::optional<set> { - if (&node != this && node.duplicate_key) return {}; + // state in the recursive computation: + // - std::nullopt means "this node has duplicates" + // - an std::set means "this node has no duplicate keys, and they are: ...". + using keyset = std::set<Key, Comp>; + using state = std::optional<keyset>; + + auto upfn = [&ctx](const Node& node, Span<state> subs) -> state { + // If this node is already known to have duplicates, nothing left to do. + if (node.has_duplicate_keys.has_value() && *node.has_duplicate_keys) return {}; + + // Check if one of the children is already known to have duplicates. + for (auto& sub : subs) { + if (!sub.has_value()) { + node.has_duplicate_keys = true; + return {}; + } + } + // Start building the set of keys involved in this node and children. + // Start by keys in this node directly. size_t keys_count = node.keys.size(); - set key_set{node.keys.begin(), node.keys.end(), Comp(ctx)}; - if (key_set.size() != keys_count) return {}; + keyset key_set{node.keys.begin(), node.keys.end(), Comp(ctx)}; + if (key_set.size() != keys_count) { + // It already has duplicates; bail out. + node.has_duplicate_keys = true; + return {}; + } - for (auto& sub: subs) { - keys_count += sub.size(); + // Merge the keys from the children into this set. + for (auto& sub : subs) { + keys_count += sub->size(); // Small optimization: std::set::merge is linear in the size of the second arg but // logarithmic in the size of the first. - if (key_set.size() < sub.size()) std::swap(key_set, sub); - key_set.merge(sub); - if (key_set.size() != keys_count) return {}; + if (key_set.size() < sub->size()) std::swap(key_set, *sub); + key_set.merge(*sub); + if (key_set.size() != keys_count) { + node.has_duplicate_keys = true; + return {}; + } } + node.has_duplicate_keys = false; return key_set; }; - return !TreeEvalMaybe<set>(upfn); + TreeEval<state>(upfn); } -public: //! Return the size of the script for this expression (faster than ToScript().size()). size_t ScriptSize() const { return scriptlen; } @@ -858,7 +893,7 @@ public: bool CheckTimeLocksMix() const { return GetType() << "k"_mst; } //! Check whether there is no duplicate key across this fragment and all its sub-fragments. - bool CheckDuplicateKey() const { return !duplicate_key; } + bool CheckDuplicateKey() const { return has_duplicate_keys && !*has_duplicate_keys; } //! Whether successful non-malleable satisfactions are guaranteed to be valid. bool ValidSatisfactions() const { return IsValid() && CheckOpsLimit() && CheckStackSize(); } @@ -872,13 +907,21 @@ public: //! Equality testing. bool operator==(const Node<Key>& arg) const { return Compare(*this, arg) == 0; } - // Constructors with various argument combinations. - template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<unsigned char> arg, uint32_t val = 0) : fragment(nt), k(val), data(std::move(arg)), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()), duplicate_key(ContainsDuplicateKey(ctx)) {} - template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<unsigned char> arg, uint32_t val = 0) : fragment(nt), k(val), data(std::move(arg)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()), duplicate_key(ContainsDuplicateKey(ctx)) {} - template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<Key> key, uint32_t val = 0) : fragment(nt), k(val), keys(std::move(key)), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()), duplicate_key(ContainsDuplicateKey(ctx)) {} - template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<Key> key, uint32_t val = 0) : fragment(nt), k(val), keys(std::move(key)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()), duplicate_key(ContainsDuplicateKey(ctx)) {} - template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>> sub, uint32_t val = 0) : fragment(nt), k(val), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()), duplicate_key(ContainsDuplicateKey(ctx)) {} - template <typename Ctx> Node(const Ctx& ctx, Fragment nt, uint32_t val = 0) : fragment(nt), k(val), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()), duplicate_key(ContainsDuplicateKey(ctx)) {} + // Constructors with various argument combinations, which bypass the duplicate key check. + Node(internal::NoDupCheck, Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<unsigned char> arg, uint32_t val = 0) : fragment(nt), k(val), data(std::move(arg)), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + Node(internal::NoDupCheck, Fragment nt, std::vector<unsigned char> arg, uint32_t val = 0) : fragment(nt), k(val), data(std::move(arg)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + Node(internal::NoDupCheck, Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<Key> key, uint32_t val = 0) : fragment(nt), k(val), keys(std::move(key)), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + Node(internal::NoDupCheck, Fragment nt, std::vector<Key> key, uint32_t val = 0) : fragment(nt), k(val), keys(std::move(key)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + Node(internal::NoDupCheck, Fragment nt, std::vector<NodeRef<Key>> sub, uint32_t val = 0) : fragment(nt), k(val), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + Node(internal::NoDupCheck, Fragment nt, uint32_t val = 0) : fragment(nt), k(val), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + + // Constructors with various argument combinations, which do perform the duplicate key check. + template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<unsigned char> arg, uint32_t val = 0) : Node(internal::NoDupCheck{}, nt, std::move(sub), std::move(arg), val) { DuplicateKeyCheck(ctx); } + template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<unsigned char> arg, uint32_t val = 0) : Node(internal::NoDupCheck{}, nt, std::move(arg), val) { DuplicateKeyCheck(ctx);} + template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<Key> key, uint32_t val = 0) : Node(internal::NoDupCheck{}, nt, std::move(sub), std::move(key), val) { DuplicateKeyCheck(ctx); } + template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<Key> key, uint32_t val = 0) : Node(internal::NoDupCheck{}, nt, std::move(key), val) { DuplicateKeyCheck(ctx); } + template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>> sub, uint32_t val = 0) : Node(internal::NoDupCheck{}, nt, std::move(sub), val) { DuplicateKeyCheck(ctx); } + template <typename Ctx> Node(const Ctx& ctx, Fragment nt, uint32_t val = 0) : Node(internal::NoDupCheck{}, nt, val) { DuplicateKeyCheck(ctx); } }; namespace internal { @@ -965,15 +1008,15 @@ std::optional<std::pair<std::vector<unsigned char>, int>> ParseHexStrEnd(Span<co } /** BuildBack pops the last two elements off `constructed` and wraps them in the specified Fragment */ -template<typename Key, typename Ctx> -void BuildBack(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>>& constructed, const bool reverse = false) +template<typename Key> +void BuildBack(Fragment nt, std::vector<NodeRef<Key>>& constructed, const bool reverse = false) { NodeRef<Key> child = std::move(constructed.back()); constructed.pop_back(); if (reverse) { - constructed.back() = MakeNodeRef<Key>(ctx, nt, Vector(std::move(child), std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, nt, Vector(std::move(child), std::move(constructed.back()))); } else { - constructed.back() = MakeNodeRef<Key>(ctx, nt, Vector(std::move(constructed.back()), std::move(child))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, nt, Vector(std::move(constructed.back()), std::move(child))); } } @@ -987,6 +1030,18 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) { using namespace spanparsing; + // Account for the minimum script size for all parsed fragments so far. It "borrows" 1 + // script byte from all leaf nodes, counting it instead whenever a space for a recursive + // expression is added (through andor, and_*, or_*, thresh). This guarantees that all fragments + // increment the script_size by at least one, except for: + // - "0", "1": these leafs are only a single byte, so their subtracted-from increment is 0. + // This is not an issue however, as "space" for them has to be created by combinators, + // which do increment script_size. + // - "v:": the v wrapper adds nothing as in some cases it results in no opcode being added + // (instead transforming another opcode into its VERIFY form). However, the v: wrapper has + // to be interleaved with other fragments to be valid, so this is not a concern. + size_t script_size{1}; + // The two integers are used to hold state for thresh() std::vector<std::tuple<ParseContext, int64_t, int64_t>> to_parse; std::vector<NodeRef<Key>> constructed; @@ -994,14 +1049,16 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1); while (!to_parse.empty()) { + if (script_size > MAX_STANDARD_P2WSH_SCRIPT_SIZE) return {}; + // Get the current context we are decoding within auto [cur_context, n, k] = to_parse.back(); to_parse.pop_back(); switch (cur_context) { case ParseContext::WRAPPED_EXPR: { - int colon_index = -1; - for (int i = 1; i < (int)in.size(); ++i) { + std::optional<size_t> colon_index{}; + for (size_t i = 1; i < in.size(); ++i) { if (in[i] == ':') { colon_index = i; break; @@ -1009,106 +1066,131 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) if (in[i] < 'a' || in[i] > 'z') break; } // If there is no colon, this loop won't execute - for (int j = 0; j < colon_index; ++j) { + bool last_was_v{false}; + for (size_t j = 0; colon_index && j < *colon_index; ++j) { + if (script_size > MAX_STANDARD_P2WSH_SCRIPT_SIZE) return {}; if (in[j] == 'a') { + script_size += 2; to_parse.emplace_back(ParseContext::ALT, -1, -1); } else if (in[j] == 's') { + script_size += 1; to_parse.emplace_back(ParseContext::SWAP, -1, -1); } else if (in[j] == 'c') { + script_size += 1; to_parse.emplace_back(ParseContext::CHECK, -1, -1); } else if (in[j] == 'd') { + script_size += 3; to_parse.emplace_back(ParseContext::DUP_IF, -1, -1); } else if (in[j] == 'j') { + script_size += 4; to_parse.emplace_back(ParseContext::NON_ZERO, -1, -1); } else if (in[j] == 'n') { + script_size += 1; to_parse.emplace_back(ParseContext::ZERO_NOTEQUAL, -1, -1); } else if (in[j] == 'v') { + // do not permit "...vv...:"; it's not valid, and also doesn't trigger early + // failure as script_size isn't incremented. + if (last_was_v) return {}; to_parse.emplace_back(ParseContext::VERIFY, -1, -1); } else if (in[j] == 'u') { + script_size += 4; to_parse.emplace_back(ParseContext::WRAP_U, -1, -1); } else if (in[j] == 't') { + script_size += 1; to_parse.emplace_back(ParseContext::WRAP_T, -1, -1); } else if (in[j] == 'l') { // The l: wrapper is equivalent to or_i(0,X) - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::JUST_0)); + script_size += 4; + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::JUST_0)); to_parse.emplace_back(ParseContext::OR_I, -1, -1); } else { return {}; } + last_was_v = (in[j] == 'v'); } to_parse.emplace_back(ParseContext::EXPR, -1, -1); - in = in.subspan(colon_index + 1); + if (colon_index) in = in.subspan(*colon_index + 1); break; } case ParseContext::EXPR: { if (Const("0", in)) { - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::JUST_0)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::JUST_0)); } else if (Const("1", in)) { - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::JUST_1)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::JUST_1)); } else if (Const("pk(", in)) { auto res = ParseKeyEnd<Key, Ctx>(in, ctx); if (!res) return {}; auto& [key, key_size] = *res; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::WRAP_C, Vector(MakeNodeRef<Key>(ctx, Fragment::PK_K, Vector(std::move(key)))))); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_C, Vector(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::PK_K, Vector(std::move(key)))))); in = in.subspan(key_size + 1); + script_size += 34; } else if (Const("pkh(", in)) { auto res = ParseKeyEnd<Key>(in, ctx); if (!res) return {}; auto& [key, key_size] = *res; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::WRAP_C, Vector(MakeNodeRef<Key>(ctx, Fragment::PK_H, Vector(std::move(key)))))); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_C, Vector(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::PK_H, Vector(std::move(key)))))); in = in.subspan(key_size + 1); + script_size += 24; } else if (Const("pk_k(", in)) { auto res = ParseKeyEnd<Key>(in, ctx); if (!res) return {}; auto& [key, key_size] = *res; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::PK_K, Vector(std::move(key)))); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::PK_K, Vector(std::move(key)))); in = in.subspan(key_size + 1); + script_size += 33; } else if (Const("pk_h(", in)) { auto res = ParseKeyEnd<Key>(in, ctx); if (!res) return {}; auto& [key, key_size] = *res; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::PK_H, Vector(std::move(key)))); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::PK_H, Vector(std::move(key)))); in = in.subspan(key_size + 1); + script_size += 23; } else if (Const("sha256(", in)) { auto res = ParseHexStrEnd(in, 32, ctx); if (!res) return {}; auto& [hash, hash_size] = *res; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::SHA256, std::move(hash))); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::SHA256, std::move(hash))); in = in.subspan(hash_size + 1); + script_size += 38; } else if (Const("ripemd160(", in)) { auto res = ParseHexStrEnd(in, 20, ctx); if (!res) return {}; auto& [hash, hash_size] = *res; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::RIPEMD160, std::move(hash))); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::RIPEMD160, std::move(hash))); in = in.subspan(hash_size + 1); + script_size += 26; } else if (Const("hash256(", in)) { auto res = ParseHexStrEnd(in, 32, ctx); if (!res) return {}; auto& [hash, hash_size] = *res; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::HASH256, std::move(hash))); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::HASH256, std::move(hash))); in = in.subspan(hash_size + 1); + script_size += 38; } else if (Const("hash160(", in)) { auto res = ParseHexStrEnd(in, 20, ctx); if (!res) return {}; auto& [hash, hash_size] = *res; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::HASH160, std::move(hash))); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::HASH160, std::move(hash))); in = in.subspan(hash_size + 1); + script_size += 26; } else if (Const("after(", in)) { int arg_size = FindNextChar(in, ')'); if (arg_size < 1) return {}; int64_t num; if (!ParseInt64(std::string(in.begin(), in.begin() + arg_size), &num)) return {}; if (num < 1 || num >= 0x80000000L) return {}; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::AFTER, num)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::AFTER, num)); in = in.subspan(arg_size + 1); + script_size += 1 + (num > 16) + (num > 0x7f) + (num > 0x7fff) + (num > 0x7fffff); } else if (Const("older(", in)) { int arg_size = FindNextChar(in, ')'); if (arg_size < 1) return {}; int64_t num; if (!ParseInt64(std::string(in.begin(), in.begin() + arg_size), &num)) return {}; if (num < 1 || num >= 0x80000000L) return {}; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::OLDER, num)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::OLDER, num)); in = in.subspan(arg_size + 1); + script_size += 1 + (num > 16) + (num > 0x7f) + (num > 0x7fff) + (num > 0x7fffff); } else if (Const("multi(", in)) { // Get threshold int next_comma = FindNextChar(in, ','); @@ -1128,7 +1210,8 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) } if (keys.size() < 1 || keys.size() > 20) return {}; if (k < 1 || k > (int64_t)keys.size()) return {}; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::MULTI, std::move(keys), k)); + script_size += 2 + (keys.size() > 16) + (k > 16) + 34 * keys.size(); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::MULTI, std::move(keys), k)); } else if (Const("thresh(", in)) { int next_comma = FindNextChar(in, ','); if (next_comma < 1) return {}; @@ -1138,6 +1221,7 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) // n = 1 here because we read the first WRAPPED_EXPR before reaching THRESH to_parse.emplace_back(ParseContext::THRESH, 1, k); to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1); + script_size += 2 + (k > 16) + (k > 0x7f) + (k > 0x7fff) + (k > 0x7fffff); } else if (Const("andor(", in)) { to_parse.emplace_back(ParseContext::ANDOR, -1, -1); to_parse.emplace_back(ParseContext::CLOSE_BRACKET, -1, -1); @@ -1146,21 +1230,29 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1); to_parse.emplace_back(ParseContext::COMMA, -1, -1); to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1); + script_size += 5; } else { if (Const("and_n(", in)) { to_parse.emplace_back(ParseContext::AND_N, -1, -1); + script_size += 5; } else if (Const("and_b(", in)) { to_parse.emplace_back(ParseContext::AND_B, -1, -1); + script_size += 2; } else if (Const("and_v(", in)) { to_parse.emplace_back(ParseContext::AND_V, -1, -1); + script_size += 1; } else if (Const("or_b(", in)) { to_parse.emplace_back(ParseContext::OR_B, -1, -1); + script_size += 2; } else if (Const("or_c(", in)) { to_parse.emplace_back(ParseContext::OR_C, -1, -1); + script_size += 3; } else if (Const("or_d(", in)) { to_parse.emplace_back(ParseContext::OR_D, -1, -1); + script_size += 4; } else if (Const("or_i(", in)) { to_parse.emplace_back(ParseContext::OR_I, -1, -1); + script_size += 4; } else { return {}; } @@ -1172,69 +1264,70 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) break; } case ParseContext::ALT: { - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_A, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_A, Vector(std::move(constructed.back()))); break; } case ParseContext::SWAP: { - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_S, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_S, Vector(std::move(constructed.back()))); break; } case ParseContext::CHECK: { - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_C, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_C, Vector(std::move(constructed.back()))); break; } case ParseContext::DUP_IF: { - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_D, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_D, Vector(std::move(constructed.back()))); break; } case ParseContext::NON_ZERO: { - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_J, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_J, Vector(std::move(constructed.back()))); break; } case ParseContext::ZERO_NOTEQUAL: { - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_N, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_N, Vector(std::move(constructed.back()))); break; } case ParseContext::VERIFY: { - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_V, Vector(std::move(constructed.back()))); + script_size += (constructed.back()->GetType() << "x"_mst); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_V, Vector(std::move(constructed.back()))); break; } case ParseContext::WRAP_U: { - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::OR_I, Vector(std::move(constructed.back()), MakeNodeRef<Key>(ctx, Fragment::JUST_0))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::OR_I, Vector(std::move(constructed.back()), MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::JUST_0))); break; } case ParseContext::WRAP_T: { - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::AND_V, Vector(std::move(constructed.back()), MakeNodeRef<Key>(ctx, Fragment::JUST_1))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::AND_V, Vector(std::move(constructed.back()), MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::JUST_1))); break; } case ParseContext::AND_B: { - BuildBack(ctx, Fragment::AND_B, constructed); + BuildBack(Fragment::AND_B, constructed); break; } case ParseContext::AND_N: { auto mid = std::move(constructed.back()); constructed.pop_back(); - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), MakeNodeRef<Key>(ctx, Fragment::JUST_0))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), MakeNodeRef<Key>(ctx, Fragment::JUST_0))); break; } case ParseContext::AND_V: { - BuildBack(ctx, Fragment::AND_V, constructed); + BuildBack(Fragment::AND_V, constructed); break; } case ParseContext::OR_B: { - BuildBack(ctx, Fragment::OR_B, constructed); + BuildBack(Fragment::OR_B, constructed); break; } case ParseContext::OR_C: { - BuildBack(ctx, Fragment::OR_C, constructed); + BuildBack(Fragment::OR_C, constructed); break; } case ParseContext::OR_D: { - BuildBack(ctx, Fragment::OR_D, constructed); + BuildBack(Fragment::OR_D, constructed); break; } case ParseContext::OR_I: { - BuildBack(ctx, Fragment::OR_I, constructed); + BuildBack(Fragment::OR_I, constructed); break; } case ParseContext::ANDOR: { @@ -1242,7 +1335,7 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) constructed.pop_back(); auto mid = std::move(constructed.back()); constructed.pop_back(); - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), std::move(right))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), std::move(right))); break; } case ParseContext::THRESH: { @@ -1251,6 +1344,7 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) in = in.subspan(1); to_parse.emplace_back(ParseContext::THRESH, n+1, k); to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1); + script_size += 2; } else if (in[0] == ')') { if (k > n) return {}; in = in.subspan(1); @@ -1261,7 +1355,7 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) constructed.pop_back(); } std::reverse(subs.begin(), subs.end()); - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::THRESH, std::move(subs), k)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::THRESH, std::move(subs), k)); } else { return {}; } @@ -1282,8 +1376,11 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) // Sanity checks on the produced miniscript assert(constructed.size() == 1); + assert(constructed[0]->ScriptSize() == script_size); if (in.size() > 0) return {}; - return std::move(constructed.front()); + const NodeRef<Key> tl_node = std::move(constructed.front()); + tl_node->DuplicateKeyCheck(ctx); + return tl_node; } /** Decode a script into opcode/push pairs. @@ -1394,12 +1491,12 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) // Constants if (in[0].first == OP_1) { ++in; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::JUST_1)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::JUST_1)); break; } if (in[0].first == OP_0) { ++in; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::JUST_0)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::JUST_0)); break; } // Public keys @@ -1407,14 +1504,14 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) auto key = ctx.FromPKBytes(in[0].second.begin(), in[0].second.end()); if (!key) return {}; ++in; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::PK_K, Vector(std::move(*key)))); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::PK_K, Vector(std::move(*key)))); break; } if (last - in >= 5 && in[0].first == OP_VERIFY && in[1].first == OP_EQUAL && in[3].first == OP_HASH160 && in[4].first == OP_DUP && in[2].second.size() == 20) { auto key = ctx.FromPKHBytes(in[2].second.begin(), in[2].second.end()); if (!key) return {}; in += 5; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::PK_H, Vector(std::move(*key)))); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::PK_H, Vector(std::move(*key)))); break; } // Time locks @@ -1422,31 +1519,31 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) if (last - in >= 2 && in[0].first == OP_CHECKSEQUENCEVERIFY && (num = ParseScriptNumber(in[1]))) { in += 2; if (*num < 1 || *num > 0x7FFFFFFFL) return {}; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::OLDER, *num)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::OLDER, *num)); break; } if (last - in >= 2 && in[0].first == OP_CHECKLOCKTIMEVERIFY && (num = ParseScriptNumber(in[1]))) { in += 2; if (num < 1 || num > 0x7FFFFFFFL) return {}; - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::AFTER, *num)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::AFTER, *num)); break; } // Hashes if (last - in >= 7 && in[0].first == OP_EQUAL && in[3].first == OP_VERIFY && in[4].first == OP_EQUAL && (num = ParseScriptNumber(in[5])) && num == 32 && in[6].first == OP_SIZE) { if (in[2].first == OP_SHA256 && in[1].second.size() == 32) { - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::SHA256, in[1].second)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::SHA256, in[1].second)); in += 7; break; } else if (in[2].first == OP_RIPEMD160 && in[1].second.size() == 20) { - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::RIPEMD160, in[1].second)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::RIPEMD160, in[1].second)); in += 7; break; } else if (in[2].first == OP_HASH256 && in[1].second.size() == 32) { - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::HASH256, in[1].second)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::HASH256, in[1].second)); in += 7; break; } else if (in[2].first == OP_HASH160 && in[1].second.size() == 20) { - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::HASH160, in[1].second)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::HASH160, in[1].second)); in += 7; break; } @@ -1467,7 +1564,7 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) if (!k || *k < 1 || *k > *n) return {}; in += 3 + *n; std::reverse(keys.begin(), keys.end()); - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::MULTI, std::move(keys), *k)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::MULTI, std::move(keys), *k)); break; } /** In the following wrappers, we only need to push SINGLE_BKV_EXPR rather @@ -1562,63 +1659,63 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) case DecodeContext::SWAP: { if (in >= last || in[0].first != OP_SWAP || constructed.empty()) return {}; ++in; - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_S, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_S, Vector(std::move(constructed.back()))); break; } case DecodeContext::ALT: { if (in >= last || in[0].first != OP_TOALTSTACK || constructed.empty()) return {}; ++in; - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_A, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_A, Vector(std::move(constructed.back()))); break; } case DecodeContext::CHECK: { if (constructed.empty()) return {}; - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_C, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_C, Vector(std::move(constructed.back()))); break; } case DecodeContext::DUP_IF: { if (constructed.empty()) return {}; - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_D, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_D, Vector(std::move(constructed.back()))); break; } case DecodeContext::VERIFY: { if (constructed.empty()) return {}; - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_V, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_V, Vector(std::move(constructed.back()))); break; } case DecodeContext::NON_ZERO: { if (constructed.empty()) return {}; - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_J, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_J, Vector(std::move(constructed.back()))); break; } case DecodeContext::ZERO_NOTEQUAL: { if (constructed.empty()) return {}; - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::WRAP_N, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::WRAP_N, Vector(std::move(constructed.back()))); break; } case DecodeContext::AND_V: { if (constructed.size() < 2) return {}; - BuildBack(ctx, Fragment::AND_V, constructed, /*reverse=*/true); + BuildBack(Fragment::AND_V, constructed, /*reverse=*/true); break; } case DecodeContext::AND_B: { if (constructed.size() < 2) return {}; - BuildBack(ctx, Fragment::AND_B, constructed, /*reverse=*/true); + BuildBack(Fragment::AND_B, constructed, /*reverse=*/true); break; } case DecodeContext::OR_B: { if (constructed.size() < 2) return {}; - BuildBack(ctx, Fragment::OR_B, constructed, /*reverse=*/true); + BuildBack(Fragment::OR_B, constructed, /*reverse=*/true); break; } case DecodeContext::OR_C: { if (constructed.size() < 2) return {}; - BuildBack(ctx, Fragment::OR_C, constructed, /*reverse=*/true); + BuildBack(Fragment::OR_C, constructed, /*reverse=*/true); break; } case DecodeContext::OR_D: { if (constructed.size() < 2) return {}; - BuildBack(ctx, Fragment::OR_D, constructed, /*reverse=*/true); + BuildBack(Fragment::OR_D, constructed, /*reverse=*/true); break; } case DecodeContext::ANDOR: { @@ -1628,7 +1725,7 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) NodeRef<Key> right = std::move(constructed.back()); constructed.pop_back(); NodeRef<Key> mid = std::move(constructed.back()); - constructed.back() = MakeNodeRef<Key>(ctx, Fragment::ANDOR, Vector(std::move(left), std::move(mid), std::move(right))); + constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::ANDOR, Vector(std::move(left), std::move(mid), std::move(right))); break; } case DecodeContext::THRESH_W: { @@ -1652,7 +1749,7 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) constructed.pop_back(); subs.push_back(std::move(sub)); } - constructed.push_back(MakeNodeRef<Key>(ctx, Fragment::THRESH, std::move(subs), k)); + constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, Fragment::THRESH, std::move(subs), k)); break; } case DecodeContext::ENDIF: { @@ -1702,7 +1799,7 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) if (in >= last) return {}; if (in[0].first == OP_IF) { ++in; - BuildBack(ctx, Fragment::OR_I, constructed, /*reverse=*/true); + BuildBack(Fragment::OR_I, constructed, /*reverse=*/true); } else if (in[0].first == OP_NOTIF) { ++in; to_parse.emplace_back(DecodeContext::ANDOR, -1, -1); @@ -1717,6 +1814,7 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) } if (constructed.size() != 1) return {}; const NodeRef<Key> tl_node = std::move(constructed.front()); + tl_node->DuplicateKeyCheck(ctx); // Note that due to how ComputeType works (only assign the type to the node if the // subs' types are valid) this would fail if any node of tree is badly typed. if (!tl_node->IsValidTopLevel()) return {}; @@ -1733,6 +1831,8 @@ inline NodeRef<typename Ctx::Key> FromString(const std::string& str, const Ctx& template<typename Ctx> inline NodeRef<typename Ctx::Key> FromScript(const CScript& script, const Ctx& ctx) { using namespace internal; + // A too large Script is necessarily invalid, don't bother parsing it. + if (script.size() > MAX_STANDARD_P2WSH_SCRIPT_SIZE) return {}; auto decomposed = DecomposeScript(script); if (!decomposed) return {}; auto it = decomposed->begin(); diff --git a/src/streams.h b/src/streams.h index f14d347380..ce1ba26d45 100644 --- a/src/streams.h +++ b/src/streams.h @@ -13,11 +13,11 @@ #include <algorithm> #include <assert.h> +#include <cstdio> #include <ios> #include <limits> #include <optional> #include <stdint.h> -#include <stdio.h> #include <string.h> #include <string> #include <utility> @@ -269,7 +269,6 @@ public: // Stream subset // bool eof() const { return size() == 0; } - CDataStream* rdbuf() { return this; } int in_avail() const { return size(); } void SetType(int n) { nType = n; } diff --git a/src/support/cleanse.h b/src/support/cleanse.h index 8c1210a114..b1227770c7 100644 --- a/src/support/cleanse.h +++ b/src/support/cleanse.h @@ -6,7 +6,7 @@ #ifndef BITCOIN_SUPPORT_CLEANSE_H #define BITCOIN_SUPPORT_CLEANSE_H -#include <stdlib.h> +#include <cstdlib> /** Secure overwrite a buffer (possibly containing secret data) with zero-bytes. The write * operation will not be optimized out by the compiler. */ diff --git a/src/sync.h b/src/sync.h index c34d969041..a9d0091bd2 100644 --- a/src/sync.h +++ b/src/sync.h @@ -165,11 +165,11 @@ private: bool TryEnter(const char* pszName, const char* pszFile, int nLine) { EnterCritical(pszName, pszFile, nLine, Base::mutex(), true); - Base::try_lock(); - if (!Base::owns_lock()) { - LeaveCritical(); + if (Base::try_lock()) { + return true; } - return Base::owns_lock(); + LeaveCritical(); + return false; } public: diff --git a/src/test/banman_tests.cpp b/src/test/banman_tests.cpp index 27ce9ad638..ecf60834ce 100644 --- a/src/test/banman_tests.cpp +++ b/src/test/banman_tests.cpp @@ -27,7 +27,7 @@ BOOST_AUTO_TEST_CASE(file) " { \"version\": 1, \"ban_created\": 0, \"banned_until\": 778, \"address\": \"1.0.0.0/8\" }" "] }", }; - assert(WriteBinaryFile(banlist_path + ".json", entries_write)); + BOOST_REQUIRE(WriteBinaryFile(banlist_path + ".json", entries_write)); { // The invalid entries will be dropped, but the valid one remains ASSERT_DEBUG_LOG("Dropping entry with unparseable address or subnet (aaaaaaaaa) from ban list"); @@ -35,7 +35,7 @@ BOOST_AUTO_TEST_CASE(file) BanMan banman{banlist_path, /*client_interface=*/nullptr, /*default_ban_time=*/0}; banmap_t entries_read; banman.GetBanned(entries_read); - assert(entries_read.size() == 1); + BOOST_CHECK_EQUAL(entries_read.size(), 1); } } } diff --git a/src/test/blockchain_tests.cpp b/src/test/blockchain_tests.cpp index c8e8cdeeb3..0157e25071 100644 --- a/src/test/blockchain_tests.cpp +++ b/src/test/blockchain_tests.cpp @@ -4,13 +4,13 @@ #include <boost/test/unit_test.hpp> -#include <stdlib.h> - #include <chain.h> #include <rpc/blockchain.h> #include <test/util/setup_common.h> #include <util/string.h> +#include <cstdlib> + /* Equality between doubles is imprecise. Comparison should be done * with a small threshold of tolerance, rather than exact equality. */ diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 7889156d51..7150698e64 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -45,6 +45,8 @@ BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup) // work. BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) { + LOCK(NetEventsInterface::g_msgproc_mutex); + ConnmanTestMsg& connman = static_cast<ConnmanTestMsg&>(*m_node.connman); // Disable inactivity checks for this test to avoid interference connman.SetPeerConnectTimeout(99999s); @@ -80,10 +82,8 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) } // Test starts here - { - LOCK(dummyNode1.cs_sendProcessing); - BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders - } + BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders + { LOCK(dummyNode1.cs_vSend); BOOST_CHECK(dummyNode1.vSendMsg.size() > 0); @@ -93,20 +93,14 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) int64_t nStartTime = GetTime(); // Wait 21 minutes SetMockTime(nStartTime+21*60); - { - LOCK(dummyNode1.cs_sendProcessing); - BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders - } + BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders { LOCK(dummyNode1.cs_vSend); BOOST_CHECK(dummyNode1.vSendMsg.size() > 0); } // Wait 3 more minutes SetMockTime(nStartTime+24*60); - { - LOCK(dummyNode1.cs_sendProcessing); - BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in disconnect - } + BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in disconnect BOOST_CHECK(dummyNode1.fDisconnect == true); peerman.FinalizeNode(dummyNode1); @@ -274,6 +268,8 @@ BOOST_AUTO_TEST_CASE(block_relay_only_eviction) BOOST_AUTO_TEST_CASE(peer_discouragement) { + LOCK(NetEventsInterface::g_msgproc_mutex); + auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), @@ -308,10 +304,8 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) nodes[0]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[0]); peerLogic->UnitTestMisbehaving(nodes[0]->GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged - { - LOCK(nodes[0]->cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(nodes[0])); - } + BOOST_CHECK(peerLogic->SendMessages(nodes[0])); + BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(nodes[0]->fDisconnect); BOOST_CHECK(!banman->IsDiscouraged(other_addr)); // Different address, not discouraged @@ -330,10 +324,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) nodes[1]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[1]); peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), DISCOURAGEMENT_THRESHOLD - 1); - { - LOCK(nodes[1]->cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(nodes[1])); - } + BOOST_CHECK(peerLogic->SendMessages(nodes[1])); // [0] is still discouraged/disconnected. BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(nodes[0]->fDisconnect); @@ -341,10 +332,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) BOOST_CHECK(!banman->IsDiscouraged(addr[1])); BOOST_CHECK(!nodes[1]->fDisconnect); peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), 1); // [1] reaches discouragement threshold - { - LOCK(nodes[1]->cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(nodes[1])); - } + BOOST_CHECK(peerLogic->SendMessages(nodes[1])); // Expect both [0] and [1] to be discouraged/disconnected now. BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(nodes[0]->fDisconnect); @@ -367,10 +355,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) nodes[2]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[2]); peerLogic->UnitTestMisbehaving(nodes[2]->GetId(), DISCOURAGEMENT_THRESHOLD); - { - LOCK(nodes[2]->cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(nodes[2])); - } + BOOST_CHECK(peerLogic->SendMessages(nodes[2])); BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(banman->IsDiscouraged(addr[1])); BOOST_CHECK(banman->IsDiscouraged(addr[2])); @@ -386,6 +371,8 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) BOOST_AUTO_TEST_CASE(DoS_bantime) { + LOCK(NetEventsInterface::g_msgproc_mutex); + auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), @@ -411,10 +398,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) dummyNode.fSuccessfullyConnected = true; peerLogic->UnitTestMisbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); - { - LOCK(dummyNode.cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(&dummyNode)); - } + BOOST_CHECK(peerLogic->SendMessages(&dummyNode)); BOOST_CHECK(banman->IsDiscouraged(addr)); peerLogic->FinalizeNode(dummyNode); diff --git a/src/test/fuzz/bitdeque.cpp b/src/test/fuzz/bitdeque.cpp index 01af8320b5..634a3de346 100644 --- a/src/test/fuzz/bitdeque.cpp +++ b/src/test/fuzz/bitdeque.cpp @@ -2,11 +2,10 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <util/bitdeque.h> - #include <random.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/util.h> +#include <util/bitdeque.h> #include <deque> #include <vector> @@ -54,7 +53,8 @@ FUZZ_TARGET_INIT(bitdeque, InitRandData) --initlen; } - while (provider.remaining_bytes()) { + LIMITED_WHILE(provider.remaining_bytes() > 0, 900) + { { assert(deq.size() == bitdeq.size()); auto it = deq.begin(); @@ -538,5 +538,4 @@ FUZZ_TARGET_INIT(bitdeque, InitRandData) } ); } - } diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index c52fca5fe8..f05248ab47 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -12,6 +12,7 @@ #include <key_io.h> #include <memusage.h> #include <netbase.h> +#include <policy/policy.h> #include <policy/settings.h> #include <pow.h> #include <protocol.h> diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp index 637ba503c6..a3d57dbdd5 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -8,6 +8,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <test/fuzz/util/mempool.h> #include <test/util/setup_common.h> #include <txmempool.h> diff --git a/src/test/fuzz/pow.cpp b/src/test/fuzz/pow.cpp index 507ce57ec0..eba03da773 100644 --- a/src/test/fuzz/pow.cpp +++ b/src/test/fuzz/pow.cpp @@ -114,7 +114,7 @@ FUZZ_TARGET_INIT(pow_transition, initialize_pow) auto current_block{std::make_unique<CBlockIndex>(header)}; current_block->pprev = blocks.empty() ? nullptr : blocks.back().get(); current_block->nHeight = height; - blocks.emplace_back(std::move(current_block)).get(); + blocks.emplace_back(std::move(current_block)); } auto last_block{blocks.back().get()}; unsigned int new_nbits{GetNextWorkRequired(last_block, nullptr, consensus_params)}; diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 272c9e6cdc..f6a35da4fc 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -73,6 +73,8 @@ void fuzz_target(FuzzBufferType buffer, const std::string& LIMIT_TO_MESSAGE_TYPE SetMockTime(1610000000); // any time to successfully reset ibd chainstate.ResetIbd(); + LOCK(NetEventsInterface::g_msgproc_mutex); + const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::COMMAND_SIZE).c_str()}; if (!LIMIT_TO_MESSAGE_TYPE.empty() && random_message_type != LIMIT_TO_MESSAGE_TYPE) { return; @@ -92,10 +94,7 @@ void fuzz_target(FuzzBufferType buffer, const std::string& LIMIT_TO_MESSAGE_TYPE GetTime<std::chrono::microseconds>(), std::atomic<bool>{false}); } catch (const std::ios_base::failure&) { } - { - LOCK(p2p_node.cs_sendProcessing); - g_setup->m_node.peerman->SendMessages(&p2p_node); - } + g_setup->m_node.peerman->SendMessages(&p2p_node); SyncWithValidationInterfaceQueue(); g_setup->m_node.connman->StopNodes(); } diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 12e682416c..1df1717ec3 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -40,6 +40,8 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages) SetMockTime(1610000000); // any time to successfully reset ibd chainstate.ResetIbd(); + LOCK(NetEventsInterface::g_msgproc_mutex); + std::vector<CNode*> peers; const auto num_peers_to_add = fuzzed_data_provider.ConsumeIntegralInRange(1, 3); for (int i = 0; i < num_peers_to_add; ++i) { @@ -70,10 +72,7 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages) connman.ProcessMessagesOnce(random_node); } catch (const std::ios_base::failure&) { } - { - LOCK(random_node.cs_sendProcessing); - g_setup->m_node.peerman->SendMessages(&random_node); - } + g_setup->m_node.peerman->SendMessages(&random_node); } SyncWithValidationInterfaceQueue(); g_setup->m_node.connman->StopNodes(); diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index 1a06ae886e..e06e57d919 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -9,6 +9,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <test/fuzz/util/mempool.h> #include <test/util/setup_common.h> #include <txmempool.h> diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 283a146369..a34e501fcc 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -8,8 +8,8 @@ #include <node/miner.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> -#include <test/fuzz/mempool_utils.h> #include <test/fuzz/util.h> +#include <test/fuzz/util/mempool.h> #include <test/util/mining.h> #include <test/util/script.h> #include <test/util/setup_common.h> diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index 943f3f5fdf..55060f31cf 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -45,7 +45,7 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) // if true, allow duplicate input when constructing tx const bool duplicate_input = fuzzed_data_provider.ConsumeBool(); - LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10 * DEFAULT_MAX_ORPHAN_TRANSACTIONS) + LIMITED_WHILE(outpoints.size() < 200'000 && fuzzed_data_provider.ConsumeBool(), 10 * DEFAULT_MAX_ORPHAN_TRANSACTIONS) { // construct transaction const CTransactionRef tx = [&] { diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index 38626d4bcf..0080b1e56e 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -307,8 +307,8 @@ CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider, const std::option int64_t ConsumeTime(FuzzedDataProvider& fuzzed_data_provider, const std::optional<int64_t>& min, const std::optional<int64_t>& max) noexcept { // Avoid t=0 (1970-01-01T00:00:00Z) since SetMockTime(0) disables mocktime. - static const int64_t time_min{ParseISO8601DateTime("2000-01-01T00:00:01Z")}; - static const int64_t time_max{ParseISO8601DateTime("2100-12-31T23:59:59Z")}; + static const int64_t time_min{946684801}; // 2000-01-01T00:00:01Z + static const int64_t time_max{4133980799}; // 2100-12-31T23:59:59Z return fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(min.value_or(time_min), max.value_or(time_max)); } @@ -478,21 +478,6 @@ CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) no return tx_destination; } -CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept -{ - // Avoid: - // policy/feerate.cpp:28:34: runtime error: signed integer overflow: 34873208148477500 * 1000 cannot be represented in type 'long' - // - // Reproduce using CFeeRate(348732081484775, 10).GetFeePerK() - const CAmount fee = std::min<CAmount>(ConsumeMoney(fuzzed_data_provider), std::numeric_limits<CAmount>::max() / static_cast<CAmount>(100000)); - assert(MoneyRange(fee)); - const int64_t time = fuzzed_data_provider.ConsumeIntegral<int64_t>(); - const unsigned int entry_height = fuzzed_data_provider.ConsumeIntegral<unsigned int>(); - const bool spends_coinbase = fuzzed_data_provider.ConsumeBool(); - const unsigned int sig_op_cost = fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, MAX_BLOCK_SIGOPS_COST); - return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, spends_coinbase, sig_op_cost, {}}; -} - bool ContainsSpentInput(const CTransaction& tx, const CCoinsViewCache& inputs) noexcept { for (const CTxIn& tx_in : tx.vin) { diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 6d652c922b..a354df2bf7 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -23,7 +23,6 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/util/net.h> -#include <txmempool.h> #include <uint256.h> #include <version.h> @@ -213,8 +212,6 @@ template <typename WeakEnumType, size_t size> return UintToArith256(ConsumeUInt256(fuzzed_data_provider)); } -[[nodiscard]] CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept; - [[nodiscard]] CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept; template <typename T> @@ -328,7 +325,7 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N } inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = std::nullopt) { return ConsumeNode<true>(fdp, node_id_in); } -void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, CNode& node) noexcept; +void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, CNode& node) noexcept EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex); class FuzzedFileProvider { diff --git a/src/test/fuzz/util/mempool.cpp b/src/test/fuzz/util/mempool.cpp new file mode 100644 index 0000000000..d0053f77d2 --- /dev/null +++ b/src/test/fuzz/util/mempool.cpp @@ -0,0 +1,27 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <consensus/amount.h> +#include <primitives/transaction.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/util.h> +#include <test/fuzz/util/mempool.h> +#include <txmempool.h> + +#include <limits> + +CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept +{ + // Avoid: + // policy/feerate.cpp:28:34: runtime error: signed integer overflow: 34873208148477500 * 1000 cannot be represented in type 'long' + // + // Reproduce using CFeeRate(348732081484775, 10).GetFeePerK() + const CAmount fee{ConsumeMoney(fuzzed_data_provider, /*max=*/std::numeric_limits<CAmount>::max() / CAmount{100'000})}; + assert(MoneyRange(fee)); + const int64_t time = fuzzed_data_provider.ConsumeIntegral<int64_t>(); + const unsigned int entry_height = fuzzed_data_provider.ConsumeIntegral<unsigned int>(); + const bool spends_coinbase = fuzzed_data_provider.ConsumeBool(); + const unsigned int sig_op_cost = fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, MAX_BLOCK_SIGOPS_COST); + return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, spends_coinbase, sig_op_cost, {}}; +} diff --git a/src/test/fuzz/mempool_utils.h b/src/test/fuzz/util/mempool.h index c172e8c4b7..4304e5294e 100644 --- a/src/test/fuzz/mempool_utils.h +++ b/src/test/fuzz/util/mempool.h @@ -2,9 +2,12 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#ifndef BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H -#define BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H +#ifndef BITCOIN_TEST_FUZZ_UTIL_MEMPOOL_H +#define BITCOIN_TEST_FUZZ_UTIL_MEMPOOL_H +#include <primitives/transaction.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <txmempool.h> #include <validation.h> class DummyChainState final : public Chainstate @@ -16,4 +19,6 @@ public: } }; -#endif // BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H +[[nodiscard]] CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept; + +#endif // BITCOIN_TEST_FUZZ_UTIL_MEMPOOL_H diff --git a/src/test/fuzz/validation_load_mempool.cpp b/src/test/fuzz/validation_load_mempool.cpp index 8241dff189..d96609416b 100644 --- a/src/test/fuzz/validation_load_mempool.cpp +++ b/src/test/fuzz/validation_load_mempool.cpp @@ -9,8 +9,8 @@ #include <node/mempool_persist_args.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> -#include <test/fuzz/mempool_utils.h> #include <test/fuzz/util.h> +#include <test/fuzz/util/mempool.h> #include <test/util/setup_common.h> #include <txmempool.h> #include <util/time.h> diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 8c745b07b9..19f457b8dd 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -203,7 +203,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) CTxMemPool::setEntries setAncestorsCalculated; std::string dummy; - BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), setAncestorsCalculated, 100, 1000000, 1000, 1000000, dummy), true); + BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(2'000'000LL).FromTx(tx7), setAncestorsCalculated, CTxMemPool::Limits::NoLimits(), dummy), true); BOOST_CHECK(setAncestorsCalculated == setAncestors); pool.addUnchecked(entry.FromTx(tx7), setAncestors); @@ -261,7 +261,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx10.vout[0].nValue = 10 * COIN; setAncestorsCalculated.clear(); - BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(200000LL).Time(4).FromTx(tx10), setAncestorsCalculated, 100, 1000000, 1000, 1000000, dummy), true); + BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(200'000LL).Time(4).FromTx(tx10), setAncestorsCalculated, CTxMemPool::Limits::NoLimits(), dummy), true); BOOST_CHECK(setAncestorsCalculated == setAncestors); pool.addUnchecked(entry.FromTx(tx10), setAncestors); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 9f5fb17b60..696a799872 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -2,7 +2,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <chainparams.h> #include <coins.h> #include <consensus/consensus.h> #include <consensus/merkle.h> @@ -30,15 +29,24 @@ using node::CBlockTemplate; namespace miner_tests { struct MinerTestingSetup : public TestingSetup { - void TestPackageSelection(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs); - void TestBasicMining(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst, int baseheight) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs); - void TestPrioritisedMining(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs); - bool TestSequenceLocks(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs) + void TestPackageSelection(const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void TestBasicMining(const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst, int baseheight) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void TestPrioritisedMining(const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool TestSequenceLocks(const CTransaction& tx, CTxMemPool& tx_mempool) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { - CCoinsViewMemPool view_mempool(&m_node.chainman->ActiveChainstate().CoinsTip(), *m_node.mempool); + CCoinsViewMemPool view_mempool{&m_node.chainman->ActiveChainstate().CoinsTip(), tx_mempool}; return CheckSequenceLocksAtTip(m_node.chainman->ActiveChain().Tip(), view_mempool, tx); } - BlockAssembler AssemblerForTest(const CChainParams& params); + CTxMemPool& MakeMempool() + { + // Delete the previous mempool to ensure with valgrind that the old + // pointer is not accessed, when the new one should be accessed + // instead. + m_node.mempool.reset(); + m_node.mempool = std::make_unique<CTxMemPool>(MemPoolOptionsForTest(m_node)); + return *m_node.mempool; + } + BlockAssembler AssemblerForTest(CTxMemPool& tx_mempool); }; } // namespace miner_tests @@ -46,13 +54,13 @@ BOOST_FIXTURE_TEST_SUITE(miner_tests, MinerTestingSetup) static CFeeRate blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE); -BlockAssembler MinerTestingSetup::AssemblerForTest(const CChainParams& params) +BlockAssembler MinerTestingSetup::AssemblerForTest(CTxMemPool& tx_mempool) { BlockAssembler::Options options; options.nBlockMaxWeight = MAX_BLOCK_WEIGHT; options.blockMinFeeRate = blockMinFeeRate; - return BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}; + return BlockAssembler{m_node.chainman->ActiveChainstate(), &tx_mempool, options}; } constexpr static struct { @@ -89,8 +97,10 @@ static CBlockIndex CreateBlockIndex(int nHeight, CBlockIndex* active_chain_tip) // Test suite for ancestor feerate transaction selection. // Implemented as an additional function, rather than a separate test case, // to allow reusing the blockchain created in CreateNewBlock_validity. -void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) +void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); // Test the ancestor feerate transaction selection. TestMemPoolEntryHelper entry; @@ -105,21 +115,21 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co tx.vout[0].nValue = 5000000000LL - 1000; // This tx has a low fee: 1000 satoshis uint256 hashParentTx = tx.GetHash(); // save this txid for later use - m_node.mempool->addUnchecked(entry.Fee(1000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(1000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); // This tx has a medium fee: 10000 satoshis tx.vin[0].prevout.hash = txFirst[1]->GetHash(); tx.vout[0].nValue = 5000000000LL - 10000; uint256 hashMediumFeeTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(10000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(10000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); // This tx has a high fee, but depends on the first transaction tx.vin[0].prevout.hash = hashParentTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 50k satoshi fee uint256 hashHighFeeTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(50000).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(50000).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); - std::unique_ptr<CBlockTemplate> pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + std::unique_ptr<CBlockTemplate> pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 4U); BOOST_CHECK(pblocktemplate->block.vtx[1]->GetHash() == hashParentTx); BOOST_CHECK(pblocktemplate->block.vtx[2]->GetHash() == hashHighFeeTx); @@ -129,7 +139,7 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co tx.vin[0].prevout.hash = hashHighFeeTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 0 fee uint256 hashFreeTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(0).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(0).FromTx(tx)); size_t freeTxSize = ::GetSerializeSize(tx, PROTOCOL_VERSION); // Calculate a fee on child transaction that will put the package just @@ -139,8 +149,8 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co tx.vin[0].prevout.hash = hashFreeTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000 - feeToUse; uint256 hashLowFeeTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(feeToUse).FromTx(tx)); - pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + tx_mempool.addUnchecked(entry.Fee(feeToUse).FromTx(tx)); + pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); // Verify that the free tx and the low fee tx didn't get selected for (size_t i=0; i<pblocktemplate->block.vtx.size(); ++i) { BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashFreeTx); @@ -150,11 +160,11 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co // Test that packages above the min relay fee do get included, even if one // of the transactions is below the min relay fee // Remove the low fee transaction and replace with a higher fee transaction - m_node.mempool->removeRecursive(CTransaction(tx), MemPoolRemovalReason::REPLACED); + tx_mempool.removeRecursive(CTransaction(tx), MemPoolRemovalReason::REPLACED); tx.vout[0].nValue -= 2; // Now we should be just over the min relay fee hashLowFeeTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(feeToUse+2).FromTx(tx)); - pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + tx_mempool.addUnchecked(entry.Fee(feeToUse + 2).FromTx(tx)); + pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 6U); BOOST_CHECK(pblocktemplate->block.vtx[4]->GetHash() == hashFreeTx); BOOST_CHECK(pblocktemplate->block.vtx[5]->GetHash() == hashLowFeeTx); @@ -167,7 +177,7 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co tx.vout[0].nValue = 5000000000LL - 100000000; tx.vout[1].nValue = 100000000; // 1BTC output uint256 hashFreeTx2 = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); // This tx can't be mined by itself tx.vin[0].prevout.hash = hashFreeTx2; @@ -175,8 +185,8 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co feeToUse = blockMinFeeRate.GetFee(freeTxSize); tx.vout[0].nValue = 5000000000LL - 100000000 - feeToUse; uint256 hashLowFeeTx2 = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx)); - pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + tx_mempool.addUnchecked(entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx)); + pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); // Verify that this tx isn't selected. for (size_t i=0; i<pblocktemplate->block.vtx.size(); ++i) { @@ -188,13 +198,13 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co // as well. tx.vin[0].prevout.n = 1; tx.vout[0].nValue = 100000000 - 10000; // 10k satoshi fee - m_node.mempool->addUnchecked(entry.Fee(10000).FromTx(tx)); - pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + tx_mempool.addUnchecked(entry.Fee(10000).FromTx(tx)); + pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 9U); BOOST_CHECK(pblocktemplate->block.vtx[8]->GetHash() == hashLowFeeTx2); } -void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst, int baseheight) +void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst, int baseheight) { uint256 hash; CMutableTransaction tx; @@ -202,172 +212,205 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C entry.nFee = 11; entry.nHeight = 11; - // Just to make sure we can still make simple blocks - auto pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); - BOOST_CHECK(pblocktemplate); - - const CAmount BLOCKSUBSIDY = 50*COIN; + const CAmount BLOCKSUBSIDY = 50 * COIN; const CAmount LOWFEE = CENT; const CAmount HIGHFEE = COIN; - const CAmount HIGHERFEE = 4*COIN; + const CAmount HIGHERFEE = 4 * COIN; - // block sigops > limit: 1000 CHECKMULTISIG + 1 - tx.vin.resize(1); - // NOTE: OP_NOP is used to force 20 SigOps for the CHECKMULTISIG - tx.vin[0].scriptSig = CScript() << OP_0 << OP_0 << OP_0 << OP_NOP << OP_CHECKMULTISIG << OP_1; - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); - tx.vin[0].prevout.n = 0; - tx.vout.resize(1); - tx.vout[0].nValue = BLOCKSUBSIDY; - for (unsigned int i = 0; i < 1001; ++i) { - tx.vout[0].nValue -= LOWFEE; - hash = tx.GetHash(); - bool spendsCoinbase = i == 0; // only first tx spends coinbase - // If we don't set the # of sig ops in the CTxMemPoolEntry, template creation fails - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); - tx.vin[0].prevout.hash = hash; + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + // Just to make sure we can still make simple blocks + auto pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); + BOOST_CHECK(pblocktemplate); + + // block sigops > limit: 1000 CHECKMULTISIG + 1 + tx.vin.resize(1); + // NOTE: OP_NOP is used to force 20 SigOps for the CHECKMULTISIG + tx.vin[0].scriptSig = CScript() << OP_0 << OP_0 << OP_0 << OP_NOP << OP_CHECKMULTISIG << OP_1; + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + tx.vin[0].prevout.n = 0; + tx.vout.resize(1); + tx.vout[0].nValue = BLOCKSUBSIDY; + for (unsigned int i = 0; i < 1001; ++i) { + tx.vout[0].nValue -= LOWFEE; + hash = tx.GetHash(); + bool spendsCoinbase = i == 0; // only first tx spends coinbase + // If we don't set the # of sig ops in the CTxMemPoolEntry, template creation fails + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); + tx.vin[0].prevout.hash = hash; + } + + BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-blk-sigops")); } - BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-blk-sigops")); - m_node.mempool->clear(); + { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + tx.vout[0].nValue = BLOCKSUBSIDY; + for (unsigned int i = 0; i < 1001; ++i) { + tx.vout[0].nValue -= LOWFEE; + hash = tx.GetHash(); + bool spendsCoinbase = i == 0; // only first tx spends coinbase + // If we do set the # of sig ops in the CTxMemPoolEntry, template creation passes + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).SigOpsCost(80).FromTx(tx)); + tx.vin[0].prevout.hash = hash; + } + BOOST_CHECK(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); + } - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); - tx.vout[0].nValue = BLOCKSUBSIDY; - for (unsigned int i = 0; i < 1001; ++i) { - tx.vout[0].nValue -= LOWFEE; - hash = tx.GetHash(); - bool spendsCoinbase = i == 0; // only first tx spends coinbase - // If we do set the # of sig ops in the CTxMemPoolEntry, template creation passes - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).SigOpsCost(80).FromTx(tx)); - tx.vin[0].prevout.hash = hash; + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + // block size > limit + tx.vin[0].scriptSig = CScript(); + // 18 * (520char + DROP) + OP_1 = 9433 bytes + std::vector<unsigned char> vchData(520); + for (unsigned int i = 0; i < 18; ++i) { + tx.vin[0].scriptSig << vchData << OP_DROP; + } + tx.vin[0].scriptSig << OP_1; + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + tx.vout[0].nValue = BLOCKSUBSIDY; + for (unsigned int i = 0; i < 128; ++i) { + tx.vout[0].nValue -= LOWFEE; + hash = tx.GetHash(); + bool spendsCoinbase = i == 0; // only first tx spends coinbase + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); + tx.vin[0].prevout.hash = hash; + } + BOOST_CHECK(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); } - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); - m_node.mempool->clear(); - - // block size > limit - tx.vin[0].scriptSig = CScript(); - // 18 * (520char + DROP) + OP_1 = 9433 bytes - std::vector<unsigned char> vchData(520); - for (unsigned int i = 0; i < 18; ++i) - tx.vin[0].scriptSig << vchData << OP_DROP; - tx.vin[0].scriptSig << OP_1; - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); - tx.vout[0].nValue = BLOCKSUBSIDY; - for (unsigned int i = 0; i < 128; ++i) + { - tx.vout[0].nValue -= LOWFEE; + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + // orphan in tx_mempool, template creation fails hash = tx.GetHash(); - bool spendsCoinbase = i == 0; // only first tx spends coinbase - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); - tx.vin[0].prevout.hash = hash; + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).FromTx(tx)); + BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); } - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); - m_node.mempool->clear(); - - // orphan in *m_node.mempool, template creation fails - hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).FromTx(tx)); - BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); - m_node.mempool->clear(); - // child with higher feerate than parent - tx.vin[0].scriptSig = CScript() << OP_1; - tx.vin[0].prevout.hash = txFirst[1]->GetHash(); - tx.vout[0].nValue = BLOCKSUBSIDY-HIGHFEE; - hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); - tx.vin[0].prevout.hash = hash; - tx.vin.resize(2); - tx.vin[1].scriptSig = CScript() << OP_1; - tx.vin[1].prevout.hash = txFirst[0]->GetHash(); - tx.vin[1].prevout.n = 0; - tx.vout[0].nValue = tx.vout[0].nValue+BLOCKSUBSIDY-HIGHERFEE; //First txn output + fresh coinbase - new txn fee - hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(HIGHERFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); - m_node.mempool->clear(); + { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); - // coinbase in *m_node.mempool, template creation fails - tx.vin.resize(1); - tx.vin[0].prevout.SetNull(); - tx.vin[0].scriptSig = CScript() << OP_0 << OP_1; - tx.vout[0].nValue = 0; - hash = tx.GetHash(); - // give it a fee so it'll get mined - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); - // Should throw bad-cb-multiple - BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-cb-multiple")); - m_node.mempool->clear(); + // child with higher feerate than parent + tx.vin[0].scriptSig = CScript() << OP_1; + tx.vin[0].prevout.hash = txFirst[1]->GetHash(); + tx.vout[0].nValue = BLOCKSUBSIDY - HIGHFEE; + hash = tx.GetHash(); + tx_mempool.addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + tx.vin[0].prevout.hash = hash; + tx.vin.resize(2); + tx.vin[1].scriptSig = CScript() << OP_1; + tx.vin[1].prevout.hash = txFirst[0]->GetHash(); + tx.vin[1].prevout.n = 0; + tx.vout[0].nValue = tx.vout[0].nValue + BLOCKSUBSIDY - HIGHERFEE; // First txn output + fresh coinbase - new txn fee + hash = tx.GetHash(); + tx_mempool.addUnchecked(entry.Fee(HIGHERFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + BOOST_CHECK(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); + } - // double spend txn pair in *m_node.mempool, template creation fails - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); - tx.vin[0].scriptSig = CScript() << OP_1; - tx.vout[0].nValue = BLOCKSUBSIDY-HIGHFEE; - tx.vout[0].scriptPubKey = CScript() << OP_1; - hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); - tx.vout[0].scriptPubKey = CScript() << OP_2; - hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); - BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); - m_node.mempool->clear(); - - // subsidy changing - int nHeight = m_node.chainman->ActiveChain().Height(); - // Create an actual 209999-long block chain (without valid blocks). - while (m_node.chainman->ActiveChain().Tip()->nHeight < 209999) { - CBlockIndex* prev = m_node.chainman->ActiveChain().Tip(); - CBlockIndex* next = new CBlockIndex(); - next->phashBlock = new uint256(InsecureRand256()); - m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(next->GetBlockHash()); - next->pprev = prev; - next->nHeight = prev->nHeight + 1; - next->BuildSkip(); - m_node.chainman->ActiveChain().SetTip(*next); + { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + // coinbase in tx_mempool, template creation fails + tx.vin.resize(1); + tx.vin[0].prevout.SetNull(); + tx.vin[0].scriptSig = CScript() << OP_0 << OP_1; + tx.vout[0].nValue = 0; + hash = tx.GetHash(); + // give it a fee so it'll get mined + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); + // Should throw bad-cb-multiple + BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-cb-multiple")); } - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); - // Extend to a 210000-long block chain. - while (m_node.chainman->ActiveChain().Tip()->nHeight < 210000) { - CBlockIndex* prev = m_node.chainman->ActiveChain().Tip(); - CBlockIndex* next = new CBlockIndex(); - next->phashBlock = new uint256(InsecureRand256()); - m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(next->GetBlockHash()); - next->pprev = prev; - next->nHeight = prev->nHeight + 1; - next->BuildSkip(); - m_node.chainman->ActiveChain().SetTip(*next); + + { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + // double spend txn pair in tx_mempool, template creation fails + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + tx.vin[0].scriptSig = CScript() << OP_1; + tx.vout[0].nValue = BLOCKSUBSIDY - HIGHFEE; + tx.vout[0].scriptPubKey = CScript() << OP_1; + hash = tx.GetHash(); + tx_mempool.addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + tx.vout[0].scriptPubKey = CScript() << OP_2; + hash = tx.GetHash(); + tx_mempool.addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); } - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); - // invalid p2sh txn in *m_node.mempool, template creation fails - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); - tx.vin[0].prevout.n = 0; - tx.vin[0].scriptSig = CScript() << OP_1; - tx.vout[0].nValue = BLOCKSUBSIDY-LOWFEE; - CScript script = CScript() << OP_0; - tx.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(script)); - hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); - tx.vin[0].prevout.hash = hash; - tx.vin[0].scriptSig = CScript() << std::vector<unsigned char>(script.begin(), script.end()); - tx.vout[0].nValue -= LOWFEE; - hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); - // Should throw block-validation-failed - BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("block-validation-failed")); - m_node.mempool->clear(); - - // Delete the dummy blocks again. - while (m_node.chainman->ActiveChain().Tip()->nHeight > nHeight) { - CBlockIndex* del = m_node.chainman->ActiveChain().Tip(); - m_node.chainman->ActiveChain().SetTip(*Assert(del->pprev)); - m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(del->pprev->GetBlockHash()); - delete del->phashBlock; - delete del; + { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + // subsidy changing + int nHeight = m_node.chainman->ActiveChain().Height(); + // Create an actual 209999-long block chain (without valid blocks). + while (m_node.chainman->ActiveChain().Tip()->nHeight < 209999) { + CBlockIndex* prev = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* next = new CBlockIndex(); + next->phashBlock = new uint256(InsecureRand256()); + m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(next->GetBlockHash()); + next->pprev = prev; + next->nHeight = prev->nHeight + 1; + next->BuildSkip(); + m_node.chainman->ActiveChain().SetTip(*next); + } + BOOST_CHECK(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); + // Extend to a 210000-long block chain. + while (m_node.chainman->ActiveChain().Tip()->nHeight < 210000) { + CBlockIndex* prev = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* next = new CBlockIndex(); + next->phashBlock = new uint256(InsecureRand256()); + m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(next->GetBlockHash()); + next->pprev = prev; + next->nHeight = prev->nHeight + 1; + next->BuildSkip(); + m_node.chainman->ActiveChain().SetTip(*next); + } + BOOST_CHECK(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); + + // invalid p2sh txn in tx_mempool, template creation fails + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + tx.vin[0].prevout.n = 0; + tx.vin[0].scriptSig = CScript() << OP_1; + tx.vout[0].nValue = BLOCKSUBSIDY - LOWFEE; + CScript script = CScript() << OP_0; + tx.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(script)); + hash = tx.GetHash(); + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + tx.vin[0].prevout.hash = hash; + tx.vin[0].scriptSig = CScript() << std::vector<unsigned char>(script.begin(), script.end()); + tx.vout[0].nValue -= LOWFEE; + hash = tx.GetHash(); + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); + // Should throw block-validation-failed + BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("block-validation-failed")); + + // Delete the dummy blocks again. + while (m_node.chainman->ActiveChain().Tip()->nHeight > nHeight) { + CBlockIndex* del = m_node.chainman->ActiveChain().Tip(); + m_node.chainman->ActiveChain().SetTip(*Assert(del->pprev)); + m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(del->pprev->GetBlockHash()); + delete del->phashBlock; + delete del; + } } + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + // non-final txs in mempool SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1); const int flags{LOCKTIME_VERIFY_SEQUENCE}; @@ -388,9 +431,9 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C tx.vout[0].scriptPubKey = CScript() << OP_1; tx.nLockTime = 0; hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes - BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail + BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail { CBlockIndex* active_chain_tip = m_node.chainman->ActiveChain().Tip(); @@ -402,9 +445,9 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | (((m_node.chainman->ActiveChain().Tip()->GetMedianTimePast()+1-m_node.chainman->ActiveChain()[1]->GetMedianTimePast()) >> CTxIn::SEQUENCE_LOCKTIME_GRANULARITY) + 1); // txFirst[1] is the 3rd block prevheights[0] = baseheight + 2; hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Time(GetTime()).FromTx(tx)); + tx_mempool.addUnchecked(entry.Time(GetTime()).FromTx(tx)); BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes - BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail + BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail const int SEQUENCE_LOCK_TIME = 512; // Sequence locks pass 512 seconds later for (int i = 0; i < CBlockIndex::nMedianTimeSpan; ++i) @@ -425,9 +468,9 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C prevheights[0] = baseheight + 3; tx.nLockTime = m_node.chainman->ActiveChain().Tip()->nHeight + 1; hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Time(GetTime()).FromTx(tx)); + tx_mempool.addUnchecked(entry.Time(GetTime()).FromTx(tx)); BOOST_CHECK(!CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime fails - BOOST_CHECK(TestSequenceLocks(CTransaction{tx})); // Sequence locks pass + BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks pass BOOST_CHECK(IsFinalTx(CTransaction(tx), m_node.chainman->ActiveChain().Tip()->nHeight + 2, m_node.chainman->ActiveChain().Tip()->GetMedianTimePast())); // Locktime passes on 2nd block // absolute time locked @@ -436,9 +479,9 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C prevheights.resize(1); prevheights[0] = baseheight + 4; hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Time(GetTime()).FromTx(tx)); + tx_mempool.addUnchecked(entry.Time(GetTime()).FromTx(tx)); BOOST_CHECK(!CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime fails - BOOST_CHECK(TestSequenceLocks(CTransaction{tx})); // Sequence locks pass + BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks pass BOOST_CHECK(IsFinalTx(CTransaction(tx), m_node.chainman->ActiveChain().Tip()->nHeight + 2, m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1)); // Locktime passes 1 second later // mempool-dependent transactions (not added) @@ -447,15 +490,16 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C tx.nLockTime = 0; tx.vin[0].nSequence = 0; BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes - BOOST_CHECK(TestSequenceLocks(CTransaction{tx})); // Sequence locks pass + BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks pass tx.vin[0].nSequence = 1; - BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail + BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG; - BOOST_CHECK(TestSequenceLocks(CTransaction{tx})); // Sequence locks pass + BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks pass tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | 1; - BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail + BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); + auto pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); + BOOST_CHECK(pblocktemplate); // None of the of the absolute height/time locked tx should have made // it into the template because we still check IsFinalTx in CreateNewBlock, @@ -470,12 +514,15 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C m_node.chainman->ActiveChain().Tip()->nHeight++; SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1); - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); + BOOST_CHECK(pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 5U); } -void MinerTestingSetup::TestPrioritisedMining(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) +void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + TestMemPoolEntryHelper entry; // Test that a tx below min fee but prioritised is included @@ -487,29 +534,29 @@ void MinerTestingSetup::TestPrioritisedMining(const CChainParams& chainparams, c tx.vout.resize(1); tx.vout[0].nValue = 5000000000LL; // 0 fee uint256 hashFreePrioritisedTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(0).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); - m_node.mempool->PrioritiseTransaction(hashFreePrioritisedTx, 5 * COIN); + tx_mempool.addUnchecked(entry.Fee(0).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.PrioritiseTransaction(hashFreePrioritisedTx, 5 * COIN); tx.vin[0].prevout.hash = txFirst[1]->GetHash(); tx.vin[0].prevout.n = 0; tx.vout[0].nValue = 5000000000LL - 1000; // This tx has a low fee: 1000 satoshis uint256 hashParentTx = tx.GetHash(); // save this txid for later use - m_node.mempool->addUnchecked(entry.Fee(1000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(1000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); // This tx has a medium fee: 10000 satoshis tx.vin[0].prevout.hash = txFirst[2]->GetHash(); tx.vout[0].nValue = 5000000000LL - 10000; uint256 hashMediumFeeTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(10000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); - m_node.mempool->PrioritiseTransaction(hashMediumFeeTx, -5 * COIN); + tx_mempool.addUnchecked(entry.Fee(10000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.PrioritiseTransaction(hashMediumFeeTx, -5 * COIN); // This tx also has a low fee, but is prioritised tx.vin[0].prevout.hash = hashParentTx; tx.vout[0].nValue = 5000000000LL - 1000 - 1000; // 1000 satoshi fee uint256 hashPrioritsedChild = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(1000).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); - m_node.mempool->PrioritiseTransaction(hashPrioritsedChild, 2 * COIN); + tx_mempool.addUnchecked(entry.Fee(1000).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); + tx_mempool.PrioritiseTransaction(hashPrioritsedChild, 2 * COIN); // Test that transaction selection properly updates ancestor fee calculations as prioritised // parents get included in a block. Create a transaction with two prioritised ancestors, each @@ -520,21 +567,21 @@ void MinerTestingSetup::TestPrioritisedMining(const CChainParams& chainparams, c tx.vin[0].prevout.hash = txFirst[3]->GetHash(); tx.vout[0].nValue = 5000000000LL; // 0 fee uint256 hashFreeParent = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); - m_node.mempool->PrioritiseTransaction(hashFreeParent, 10 * COIN); + tx_mempool.addUnchecked(entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.PrioritiseTransaction(hashFreeParent, 10 * COIN); tx.vin[0].prevout.hash = hashFreeParent; tx.vout[0].nValue = 5000000000LL; // 0 fee uint256 hashFreeChild = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); - m_node.mempool->PrioritiseTransaction(hashFreeChild, 1 * COIN); + tx_mempool.addUnchecked(entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); + tx_mempool.PrioritiseTransaction(hashFreeChild, 1 * COIN); tx.vin[0].prevout.hash = hashFreeChild; tx.vout[0].nValue = 5000000000LL; // 0 fee uint256 hashFreeGrandchild = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); - auto pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + auto pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 6U); BOOST_CHECK(pblocktemplate->block.vtx[1]->GetHash() == hashFreeParent); BOOST_CHECK(pblocktemplate->block.vtx[2]->GetHash() == hashFreePrioritisedTx); @@ -553,15 +600,12 @@ void MinerTestingSetup::TestPrioritisedMining(const CChainParams& chainparams, c BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) { // Note that by default, these tests run with size accounting enabled. - const auto chainParams = CreateChainParams(*m_node.args, CBaseChainParams::MAIN); - const CChainParams& chainparams = *chainParams; CScript scriptPubKey = CScript() << ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f") << OP_CHECKSIG; std::unique_ptr<CBlockTemplate> pblocktemplate; - fCheckpointsEnabled = false; - + CTxMemPool& tx_mempool{*m_node.mempool}; // Simple block creation, nothing special yet: - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); + BOOST_CHECK(pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); // We can't make transactions until we have inputs // Therefore, load 110 blocks :) @@ -593,23 +637,18 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) } LOCK(cs_main); - LOCK(m_node.mempool->cs); - TestBasicMining(chainparams, scriptPubKey, txFirst, baseheight); + TestBasicMining(scriptPubKey, txFirst, baseheight); m_node.chainman->ActiveChain().Tip()->nHeight--; SetMockTime(0); - m_node.mempool->clear(); - TestPackageSelection(chainparams, scriptPubKey, txFirst); + TestPackageSelection(scriptPubKey, txFirst); m_node.chainman->ActiveChain().Tip()->nHeight--; SetMockTime(0); - m_node.mempool->clear(); - - TestPrioritisedMining(chainparams, scriptPubKey, txFirst); - fCheckpointsEnabled = true; + TestPrioritisedMining(scriptPubKey, txFirst); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp index 95e8476b77..9387c01e73 100644 --- a/src/test/miniscript_tests.cpp +++ b/src/test/miniscript_tests.cpp @@ -296,6 +296,9 @@ BOOST_AUTO_TEST_CASE(fixed_tests) // Same when the duplicates are on different levels in the tree const auto ms_dup4 = miniscript::FromString("thresh(2,pkh(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),s:pk(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556),a:and_b(dv:older(1),s:pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65)))", CONVERTER); BOOST_CHECK(ms_dup4 && !ms_dup4->IsSane() && !ms_dup4->CheckDuplicateKey()); + // Sanity check the opposite is true, too. An otherwise sane Miniscript with no duplicate keys is sane. + const auto ms_nondup = miniscript::FromString("pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65)", CONVERTER); + BOOST_CHECK(ms_nondup && ms_nondup->CheckDuplicateKey() && ms_nondup->IsSane()); // Test we find the first insane sub closer to be a leaf node. This fragment is insane for two reasons: // 1. It can be spent without a signature // 2. It contains timelock mixes diff --git a/src/test/minisketch_tests.cpp b/src/test/minisketch_tests.cpp index 9c53ace633..81f2aad623 100644 --- a/src/test/minisketch_tests.cpp +++ b/src/test/minisketch_tests.cpp @@ -40,7 +40,7 @@ BOOST_AUTO_TEST_CASE(minisketch_test) Minisketch sketch_c = std::move(sketch_ar); sketch_c.Merge(sketch_br); auto dec = sketch_c.Decode(errors); - BOOST_CHECK(dec.has_value()); + BOOST_REQUIRE(dec.has_value()); auto sols = std::move(*dec); std::sort(sols.begin(), sols.end()); for (uint32_t i = 0; i < a_not_b; ++i) BOOST_CHECK_EQUAL(sols[i], start_a + i); diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 12905f6b70..f24509dd97 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -805,6 +805,8 @@ BOOST_AUTO_TEST_CASE(LocalAddress_BasicLifecycle) BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) { + LOCK(NetEventsInterface::g_msgproc_mutex); + // Tests the following scenario: // * -bind=3.4.5.6:20001 is specified // * we make an outbound connection to a peer @@ -889,10 +891,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) } }; - { - LOCK(peer.cs_sendProcessing); - m_node.peerman->SendMessages(&peer); - } + m_node.peerman->SendMessages(&peer); BOOST_CHECK(sent); diff --git a/src/test/raii_event_tests.cpp b/src/test/raii_event_tests.cpp index c489ac04e9..d4487cd941 100644 --- a/src/test/raii_event_tests.cpp +++ b/src/test/raii_event_tests.cpp @@ -4,8 +4,8 @@ #include <event2/event.h> +#include <cstdlib> #include <map> -#include <stdlib.h> #include <support/events.h> diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index f160bb08a5..11f4be7fef 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -3,7 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. // #include <test/util/setup_common.h> -#include <util/system.h> +#include <common/run_command.h> #include <univalue.h> #ifdef ENABLE_EXTERNAL_SIGNER diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index 21273ac5c1..2e3e16e681 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -44,10 +44,7 @@ void ConnmanTestMsg::Handshake(CNode& node, (void)connman.ReceiveMsgFrom(node, msg_version); node.fPauseSend = false; connman.ProcessMessagesOnce(node); - { - LOCK(node.cs_sendProcessing); - peerman.SendMessages(&node); - } + peerman.SendMessages(&node); if (node.fDisconnect) return; assert(node.nVersion == version); assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION)); @@ -60,10 +57,7 @@ void ConnmanTestMsg::Handshake(CNode& node, (void)connman.ReceiveMsgFrom(node, msg_verack); node.fPauseSend = false; connman.ProcessMessagesOnce(node); - { - LOCK(node.cs_sendProcessing); - peerman.SendMessages(&node); - } + peerman.SendMessages(&node); assert(node.fSuccessfullyConnected == true); } } diff --git a/src/test/util/net.h b/src/test/util/net.h index b339bee32a..73543de4ca 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -44,9 +44,10 @@ struct ConnmanTestMsg : public CConnman { ServiceFlags remote_services, ServiceFlags local_services, int32_t version, - bool relay_txs); + bool relay_txs) + EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex); - void ProcessMessagesOnce(CNode& node) { m_msgproc->ProcessMessages(&node, flagInterruptMsgProc); } + void ProcessMessagesOnce(CNode& node) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex) { m_msgproc->ProcessMessages(&node, flagInterruptMsgProc); } void NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_bytes, bool& complete) const; diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 0f9f332dc6..6e59d2e8e6 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -282,14 +282,10 @@ BOOST_AUTO_TEST_CASE(util_TrimString) BOOST_CHECK_EQUAL(TrimStringView(std::string("\x05\x04\x03\x02\x01\x00", 6), std::string("\x05\x04\x03\x02\x01\x00", 6)), ""); } -BOOST_AUTO_TEST_CASE(util_FormatParseISO8601DateTime) +BOOST_AUTO_TEST_CASE(util_FormatISO8601DateTime) { BOOST_CHECK_EQUAL(FormatISO8601DateTime(1317425777), "2011-09-30T23:36:17Z"); BOOST_CHECK_EQUAL(FormatISO8601DateTime(0), "1970-01-01T00:00:00Z"); - - BOOST_CHECK_EQUAL(ParseISO8601DateTime("1970-01-01T00:00:00Z"), 0); - BOOST_CHECK_EQUAL(ParseISO8601DateTime("1960-01-01T00:00:00Z"), 0); - BOOST_CHECK_EQUAL(ParseISO8601DateTime("2011-09-30T23:36:17Z"), 1317425777); } BOOST_AUTO_TEST_CASE(util_FormatISO8601Date) diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 24ad9458c9..9fcb7d315a 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -314,7 +314,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) //! Test LoadBlockIndex behavior when multiple chainstates are in use. //! -//! - First, verfiy that setBlockIndexCandidates is as expected when using a single, +//! - First, verify that setBlockIndexCandidates is as expected when using a single, //! fully-validating chainstate. //! //! - Then mark a region of the chain BLOCK_ASSUMED_VALID and introduce a second chainstate diff --git a/src/txmempool.cpp b/src/txmempool.cpp index e1288b7346..84ed2e9ef5 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -145,7 +145,7 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256>& vHashes // Iterate in reverse, so that whenever we are looking at a transaction // we are sure that all in-mempool descendants have already been processed. // This maximizes the benefit of the descendant cache and guarantees that - // CTxMemPool::m_children will be updated, an assumption made in + // CTxMemPoolEntry::m_children will be updated, an assumption made in // UpdateForDescendants. for (const uint256 &hash : reverse_iterate(vHashesToUpdate)) { // calculate children from mapNextTx @@ -154,7 +154,7 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256>& vHashes continue; } auto iter = mapNextTx.lower_bound(COutPoint(hash, 0)); - // First calculate the children, and update CTxMemPool::m_children to + // First calculate the children, and update CTxMemPoolEntry::m_children to // include them, and update their CTxMemPoolEntry::m_parents to include this tx. // we cache the in-mempool children to avoid duplicate updates { @@ -187,10 +187,7 @@ bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, size_t entry_count, setEntries& setAncestors, CTxMemPoolEntry::Parents& staged_ancestors, - uint64_t limitAncestorCount, - uint64_t limitAncestorSize, - uint64_t limitDescendantCount, - uint64_t limitDescendantSize, + const Limits& limits, std::string &errString) const { size_t totalSizeWithAncestors = entry_size; @@ -203,14 +200,14 @@ bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, staged_ancestors.erase(stage); totalSizeWithAncestors += stageit->GetTxSize(); - if (stageit->GetSizeWithDescendants() + entry_size > limitDescendantSize) { - errString = strprintf("exceeds descendant size limit for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limitDescendantSize); + if (stageit->GetSizeWithDescendants() + entry_size > static_cast<uint64_t>(limits.descendant_size_vbytes)) { + errString = strprintf("exceeds descendant size limit for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limits.descendant_size_vbytes); return false; - } else if (stageit->GetCountWithDescendants() + entry_count > limitDescendantCount) { - errString = strprintf("too many descendants for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limitDescendantCount); + } else if (stageit->GetCountWithDescendants() + entry_count > static_cast<uint64_t>(limits.descendant_count)) { + errString = strprintf("too many descendants for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limits.descendant_count); return false; - } else if (totalSizeWithAncestors > limitAncestorSize) { - errString = strprintf("exceeds ancestor size limit [limit: %u]", limitAncestorSize); + } else if (totalSizeWithAncestors > static_cast<uint64_t>(limits.ancestor_size_vbytes)) { + errString = strprintf("exceeds ancestor size limit [limit: %u]", limits.ancestor_size_vbytes); return false; } @@ -222,8 +219,8 @@ bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, if (setAncestors.count(parent_it) == 0) { staged_ancestors.insert(parent); } - if (staged_ancestors.size() + setAncestors.size() + entry_count > limitAncestorCount) { - errString = strprintf("too many unconfirmed ancestors [limit: %u]", limitAncestorCount); + if (staged_ancestors.size() + setAncestors.size() + entry_count > static_cast<uint64_t>(limits.ancestor_count)) { + errString = strprintf("too many unconfirmed ancestors [limit: %u]", limits.ancestor_count); return false; } } @@ -233,10 +230,7 @@ bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, } bool CTxMemPool::CheckPackageLimits(const Package& package, - uint64_t limitAncestorCount, - uint64_t limitAncestorSize, - uint64_t limitDescendantCount, - uint64_t limitDescendantSize, + const Limits& limits, std::string &errString) const { CTxMemPoolEntry::Parents staged_ancestors; @@ -247,8 +241,8 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, std::optional<txiter> piter = GetIter(input.prevout.hash); if (piter) { staged_ancestors.insert(**piter); - if (staged_ancestors.size() + package.size() > limitAncestorCount) { - errString = strprintf("too many unconfirmed parents [limit: %u]", limitAncestorCount); + if (staged_ancestors.size() + package.size() > static_cast<uint64_t>(limits.ancestor_count)) { + errString = strprintf("too many unconfirmed parents [limit: %u]", limits.ancestor_count); return false; } } @@ -260,8 +254,7 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, setEntries setAncestors; const auto ret = CalculateAncestorsAndCheckLimits(total_size, package.size(), setAncestors, staged_ancestors, - limitAncestorCount, limitAncestorSize, - limitDescendantCount, limitDescendantSize, errString); + limits, errString); // It's possible to overestimate the ancestor/descendant totals. if (!ret) errString.insert(0, "possibly "); return ret; @@ -269,10 +262,7 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, - uint64_t limitAncestorCount, - uint64_t limitAncestorSize, - uint64_t limitDescendantCount, - uint64_t limitDescendantSize, + const Limits& limits, std::string &errString, bool fSearchForParents /* = true */) const { @@ -287,8 +277,8 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, std::optional<txiter> piter = GetIter(tx.vin[i].prevout.hash); if (piter) { staged_ancestors.insert(**piter); - if (staged_ancestors.size() + 1 > limitAncestorCount) { - errString = strprintf("too many unconfirmed parents [limit: %u]", limitAncestorCount); + if (staged_ancestors.size() + 1 > static_cast<uint64_t>(limits.ancestor_count)) { + errString = strprintf("too many unconfirmed parents [limit: %u]", limits.ancestor_count); return false; } } @@ -302,8 +292,7 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, return CalculateAncestorsAndCheckLimits(entry.GetTxSize(), /*entry_count=*/1, setAncestors, staged_ancestors, - limitAncestorCount, limitAncestorSize, - limitDescendantCount, limitDescendantSize, errString); + limits, errString); } void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors) @@ -347,7 +336,6 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, b { // For each entry, walk back all ancestors and decrement size associated with this // transaction - const uint64_t nNoLimit = std::numeric_limits<uint64_t>::max(); if (updateDescendants) { // updateDescendants should be true whenever we're not recursively // removing a tx and all its descendants, eg when a transaction is @@ -390,7 +378,7 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, b // mempool parents we'd calculate by searching, and it's important that // we use the cached notion of ancestor transactions as the set of // things to update for removal. - CalculateMemPoolAncestors(entry, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false); + CalculateMemPoolAncestors(entry, setAncestors, Limits::NoLimits(), dummy, false); // Note that UpdateAncestorsOf severs the child links that point to // removeIt in the entries for the parents of removeIt. UpdateAncestorsOf(false, removeIt, setAncestors); @@ -744,9 +732,8 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei assert(std::equal(setParentCheck.begin(), setParentCheck.end(), it->GetMemPoolParentsConst().begin(), comp)); // Verify ancestor state is correct. setEntries setAncestors; - uint64_t nNoLimit = std::numeric_limits<uint64_t>::max(); std::string dummy; - CalculateMemPoolAncestors(*it, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy); + CalculateMemPoolAncestors(*it, setAncestors, Limits::NoLimits(), dummy); uint64_t nCountCheck = setAncestors.size() + 1; uint64_t nSizeCheck = it->GetTxSize(); CAmount nFeesCheck = it->GetModifiedFee(); @@ -908,9 +895,8 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD mapTx.modify(it, [&nFeeDelta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(nFeeDelta); }); // Now update all ancestors' modified fees with descendants setEntries setAncestors; - uint64_t nNoLimit = std::numeric_limits<uint64_t>::max(); std::string dummy; - CalculateMemPoolAncestors(*it, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false); + CalculateMemPoolAncestors(*it, setAncestors, Limits::NoLimits(), dummy, false); for (txiter ancestorIt : setAncestors) { mapTx.modify(ancestorIt, [=](CTxMemPoolEntry& e){ e.UpdateDescendantState(0, nFeeDelta, 0);}); } @@ -1049,9 +1035,8 @@ int CTxMemPool::Expire(std::chrono::seconds time) void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, bool validFeeEstimate) { setEntries setAncestors; - uint64_t nNoLimit = std::numeric_limits<uint64_t>::max(); std::string dummy; - CalculateMemPoolAncestors(entry, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy); + CalculateMemPoolAncestors(entry, setAncestors, Limits::NoLimits(), dummy); return addUnchecked(entry, setAncestors, validFeeEstimate); } diff --git a/src/txmempool.h b/src/txmempool.h index cd15d069b1..4afaac0506 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -526,6 +526,8 @@ public: typedef std::set<txiter, CompareIteratorByHash> setEntries; + using Limits = kernel::MemPoolLimits; + uint64_t CalculateDescendantMaximum(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); private: typedef std::map<txiter, setEntries, CompareIteratorByHash> cacheMap; @@ -545,19 +547,22 @@ private: /** * Helper function to calculate all in-mempool ancestors of staged_ancestors and apply ancestor * and descendant limits (including staged_ancestors thsemselves, entry_size and entry_count). - * param@[in] entry_size Virtual size to include in the limits. - * param@[in] entry_count How many entries to include in the limits. - * param@[in] staged_ancestors Should contain entries in the mempool. - * param@[out] setAncestors Will be populated with all mempool ancestors. + * + * @param[in] entry_size Virtual size to include in the limits. + * @param[in] entry_count How many entries to include in the limits. + * @param[out] setAncestors Will be populated with all mempool ancestors. + * @param[in] staged_ancestors Should contain entries in the mempool. + * @param[in] limits Maximum number and size of ancestors and descendants + * @param[out] errString Populated with error reason if any limits are hit + * + * @return true if no limits were hit and all in-mempool ancestors were calculated, false + * otherwise */ bool CalculateAncestorsAndCheckLimits(size_t entry_size, size_t entry_count, setEntries& setAncestors, CTxMemPoolEntry::Parents &staged_ancestors, - uint64_t limitAncestorCount, - uint64_t limitAncestorSize, - uint64_t limitDescendantCount, - uint64_t limitDescendantSize, + const Limits& limits, std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); public: @@ -576,8 +581,6 @@ public: const bool m_require_standard; const bool m_full_rbf; - using Limits = kernel::MemPoolLimits; - const Limits m_limits; /** Create a new CTxMemPool. @@ -668,38 +671,41 @@ public: */ void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main) LOCKS_EXCLUDED(m_epoch); - /** Try to calculate all in-mempool ancestors of entry. - * (these are all calculated including the tx itself) - * limitAncestorCount = max number of ancestors - * limitAncestorSize = max size of ancestors - * limitDescendantCount = max number of descendants any ancestor can have - * limitDescendantSize = max size of descendants any ancestor can have - * errString = populated with error reason if any limits are hit - * fSearchForParents = whether to search a tx's vin for in-mempool parents, or - * look up parents from mapLinks. Must be true for entries not in the mempool + /** + * Try to calculate all in-mempool ancestors of entry. + * (these are all calculated including the tx itself) + * + * @param[in] entry CTxMemPoolEntry of which all in-mempool ancestors are calculated + * @param[out] setAncestors Will be populated with all mempool ancestors. + * @param[in] limits Maximum number and size of ancestors and descendants + * @param[out] errString Populated with error reason if any limits are hit + * @param[in] fSearchForParents Whether to search a tx's vin for in-mempool parents, or look + * up parents from mapLinks. Must be true for entries not in + * the mempool + * + * @return true if no limits were hit and all in-mempool ancestors were calculated, false + * otherwise */ - bool CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, setEntries& setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string& errString, bool fSearchForParents = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); + bool CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, + setEntries& setAncestors, + const Limits& limits, + std::string& errString, + bool fSearchForParents = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Calculate all in-mempool ancestors of a set of transactions not already in the mempool and * check ancestor and descendant limits. Heuristics are used to estimate the ancestor and * descendant count of all entries if the package were to be added to the mempool. The limits * are applied to the union of all package transactions. For example, if the package has 3 - * transactions and limitAncestorCount = 25, the union of all 3 sets of ancestors (including the + * transactions and limits.ancestor_count = 25, the union of all 3 sets of ancestors (including the * transactions themselves) must be <= 22. * @param[in] package Transaction package being evaluated for acceptance * to mempool. The transactions need not be direct * ancestors/descendants of each other. - * @param[in] limitAncestorCount Max number of txns including ancestors. - * @param[in] limitAncestorSize Max virtual size including ancestors. - * @param[in] limitDescendantCount Max number of txns including descendants. - * @param[in] limitDescendantSize Max virtual size including descendants. + * @param[in] limits Maximum number and size of ancestors and descendants * @param[out] errString Populated with error reason if a limit is hit. */ bool CheckPackageLimits(const Package& package, - uint64_t limitAncestorCount, - uint64_t limitAncestorSize, - uint64_t limitDescendantCount, - uint64_t limitDescendantSize, + const Limits& limits, std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Populate setDescendants with all in-mempool descendants of hash. @@ -825,7 +831,7 @@ private: * mempool but may have child transactions in the mempool, eg during a * chain reorg. * - * @pre CTxMemPool::m_children is correct for the given tx and all + * @pre CTxMemPoolEntry::m_children is correct for the given tx and all * descendants. * @pre cachedDescendants is an accurate cache where each entry has all * descendants of the corresponding key, including those that should diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h index ccaf56a271..850d0a1cbc 100644 --- a/src/univalue/include/univalue.h +++ b/src/univalue/include/univalue.h @@ -19,6 +19,11 @@ class UniValue { public: enum VType { VNULL, VOBJ, VARR, VSTR, VNUM, VBOOL, }; + class type_error : public std::runtime_error + { + using std::runtime_error::runtime_error; + }; + UniValue() { typ = VNULL; } UniValue(UniValue::VType initialType, const std::string& initialStr = "") { typ = initialType; diff --git a/src/univalue/lib/univalue.cpp b/src/univalue/lib/univalue.cpp index 12a434dd0e..4448981d3e 100644 --- a/src/univalue/lib/univalue.cpp +++ b/src/univalue/lib/univalue.cpp @@ -210,7 +210,7 @@ const UniValue& UniValue::operator[](size_t index) const void UniValue::checkType(const VType& expected) const { if (typ != expected) { - throw std::runtime_error{"JSON value of type " + std::string{uvTypeName(typ)} + " is not of expected type " + + throw type_error{"JSON value of type " + std::string{uvTypeName(typ)} + " is not of expected type " + std::string{uvTypeName(expected)}}; } } diff --git a/src/util/system.cpp b/src/util/system.cpp index c3c6cbfef6..ce5d846eb9 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -5,19 +5,6 @@ #include <util/system.h> -#ifdef ENABLE_EXTERNAL_SIGNER -#if defined(__GNUC__) -// Boost 1.78 requires the following workaround. -// See: https://github.com/boostorg/process/issues/235 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wnarrowing" -#endif -#include <boost/process.hpp> -#if defined(__GNUC__) -#pragma GCC diagnostic pop -#endif -#endif // ENABLE_EXTERNAL_SIGNER - #include <chainparamsbase.h> #include <fs.h> #include <sync.h> @@ -1332,44 +1319,6 @@ void runCommand(const std::string& strCommand) } #endif -UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) -{ -#ifdef ENABLE_EXTERNAL_SIGNER - namespace bp = boost::process; - - UniValue result_json; - bp::opstream stdin_stream; - bp::ipstream stdout_stream; - bp::ipstream stderr_stream; - - if (str_command.empty()) return UniValue::VNULL; - - bp::child c( - str_command, - bp::std_out > stdout_stream, - bp::std_err > stderr_stream, - bp::std_in < stdin_stream - ); - if (!str_std_in.empty()) { - stdin_stream << str_std_in << std::endl; - } - stdin_stream.pipe().close(); - - std::string result; - std::string error; - std::getline(stdout_stream, result); - std::getline(stderr_stream, error); - - c.wait(); - const int n_error = c.exit_code(); - if (n_error) throw std::runtime_error(strprintf("RunCommandParseJSON error: process(%s) returned %d: %s\n", str_command, n_error, error)); - if (!result_json.read(result)) throw std::runtime_error("Unable to parse JSON: " + result); - - return result_json; -#else - throw std::runtime_error("Compiled without external signing support (required for external signing)."); -#endif // ENABLE_EXTERNAL_SIGNER -} void SetupEnvironment() { diff --git a/src/util/system.h b/src/util/system.h index c8e1de6700..29629e547e 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -107,14 +107,6 @@ std::string ShellEscape(const std::string& arg); #if HAVE_SYSTEM void runCommand(const std::string& strCommand); #endif -/** - * Execute a command which returns JSON, and parse the result. - * - * @param str_command The command to execute, including any arguments - * @param str_std_in string to pass to stdin - * @return parsed JSON - */ -UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in=""); /** * Most paths passed as configuration arguments are treated as relative to diff --git a/src/util/time.cpp b/src/util/time.cpp index f6d37347f8..0b20849079 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -12,8 +12,6 @@ #include <util/time.h> #include <util/check.h> -#include <boost/date_time/posix_time/posix_time.hpp> - #include <atomic> #include <chrono> #include <ctime> @@ -142,20 +140,6 @@ std::string FormatISO8601Date(int64_t nTime) { return strprintf("%04i-%02i-%02i", ts.tm_year + 1900, ts.tm_mon + 1, ts.tm_mday); } -int64_t ParseISO8601DateTime(const std::string& str) -{ - static const boost::posix_time::ptime epoch = boost::posix_time::from_time_t(0); - static const std::locale loc(std::locale::classic(), - new boost::posix_time::time_input_facet("%Y-%m-%dT%H:%M:%SZ")); - std::istringstream iss(str); - iss.imbue(loc); - boost::posix_time::ptime ptime(boost::date_time::not_a_date_time); - iss >> ptime; - if (ptime.is_not_a_date_time() || epoch > ptime) - return 0; - return (ptime - epoch).total_seconds(); -} - struct timeval MillisToTimeval(int64_t nTimeout) { struct timeval timeout; diff --git a/src/util/time.h b/src/util/time.h index fcaae4af3e..10a581a44c 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -110,7 +110,6 @@ T GetTime() */ std::string FormatISO8601DateTime(int64_t nTime); std::string FormatISO8601Date(int64_t nTime); -int64_t ParseISO8601DateTime(const std::string& str); /** * Convert milliseconds to a struct timeval for e.g. select. diff --git a/src/validation.cpp b/src/validation.cpp index 9ebcc4fb98..514a528314 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -421,11 +421,13 @@ namespace { class MemPoolAccept { public: - explicit MemPoolAccept(CTxMemPool& mempool, Chainstate& active_chainstate) : m_pool(mempool), m_view(&m_dummy), m_viewmempool(&active_chainstate.CoinsTip(), m_pool), m_active_chainstate(active_chainstate), - m_limit_ancestors(m_pool.m_limits.ancestor_count), - m_limit_ancestor_size(m_pool.m_limits.ancestor_size_vbytes), - m_limit_descendants(m_pool.m_limits.descendant_count), - m_limit_descendant_size(m_pool.m_limits.descendant_size_vbytes) { + explicit MemPoolAccept(CTxMemPool& mempool, Chainstate& active_chainstate) : + m_pool(mempool), + m_view(&m_dummy), + m_viewmempool(&active_chainstate.CoinsTip(), m_pool), + m_active_chainstate(active_chainstate), + m_limits{m_pool.m_limits} + { } // We put the arguments we're handed into a struct, so we can pass them @@ -657,13 +659,7 @@ private: Chainstate& m_active_chainstate; - // The package limits in effect at the time of invocation. - const size_t m_limit_ancestors; - const size_t m_limit_ancestor_size; - // These may be modified while evaluating a transaction (eg to account for - // in-mempool conflicts; see below). - size_t m_limit_descendants; - size_t m_limit_descendant_size; + CTxMemPool::Limits m_limits; /** Whether the transaction(s) would replace any mempool transactions. If so, RBF rules apply. */ bool m_rbf{false}; @@ -876,12 +872,12 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) assert(ws.m_iters_conflicting.size() == 1); CTxMemPool::txiter conflict = *ws.m_iters_conflicting.begin(); - m_limit_descendants += 1; - m_limit_descendant_size += conflict->GetSizeWithDescendants(); + m_limits.descendant_count += 1; + m_limits.descendant_size_vbytes += conflict->GetSizeWithDescendants(); } std::string errString; - if (!m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, m_limit_descendant_size, errString)) { + if (!m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, m_limits, errString)) { ws.m_ancestors.clear(); // If CalculateMemPoolAncestors fails second time, we want the original error string. std::string dummy_err_string; @@ -896,8 +892,16 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // to be secure by simply only having two immediately-spendable // outputs - one for each counterparty. For more info on the uses for // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html + CTxMemPool::Limits cpfp_carve_out_limits{ + .ancestor_count = 2, + .ancestor_size_vbytes = m_limits.ancestor_size_vbytes, + .descendant_count = m_limits.descendant_count + 1, + .descendant_size_vbytes = m_limits.descendant_size_vbytes + EXTRA_DESCENDANT_TX_SIZE_LIMIT, + }; if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || - !m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { + !m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, + cpfp_carve_out_limits, + dummy_err_string)) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", errString); } } @@ -973,8 +977,7 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn { return !m_pool.exists(GenTxid::Txid(tx->GetHash()));})); std::string err_string; - if (!m_pool.CheckPackageLimits(txns, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, - m_limit_descendant_size, err_string)) { + if (!m_pool.CheckPackageLimits(txns, m_limits, err_string)) { // This is a package-wide error, separate from an individual transaction error. return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", err_string); } @@ -1118,9 +1121,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>& // Re-calculate mempool ancestors to call addUnchecked(). They may have changed since the // last calculation done in PreChecks, since package ancestors have already been submitted. std::string unused_err_string; - if(!m_pool.CalculateMemPoolAncestors(*ws.m_entry, ws.m_ancestors, m_limit_ancestors, - m_limit_ancestor_size, m_limit_descendants, - m_limit_descendant_size, unused_err_string)) { + if(!m_pool.CalculateMemPoolAncestors(*ws.m_entry, ws.m_ancestors, m_limits, unused_err_string)) { results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); // Since PreChecks() and PackageMempoolChecks() both enforce limits, this should never fail. Assume(false); diff --git a/src/validation.h b/src/validation.h index 9ba206855f..c882eac408 100644 --- a/src/validation.h +++ b/src/validation.h @@ -65,9 +65,6 @@ static const int MAX_SCRIPTCHECK_THREADS = 15; static const int DEFAULT_SCRIPTCHECK_THREADS = 0; static const int64_t DEFAULT_MAX_TIP_AGE = 24 * 60 * 60; static const bool DEFAULT_CHECKPOINTS_ENABLED = true; -static const bool DEFAULT_TXINDEX = false; -static constexpr bool DEFAULT_COINSTATSINDEX{false}; -static const char* const DEFAULT_BLOCKFILTERINDEX = "0"; /** Default for -stopatheight */ static const int DEFAULT_STOPATHEIGHT = 0; /** Block files containing a block-height within MIN_BLOCKS_TO_KEEP of ActiveChain().Tip() will not be pruned. */ diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 98925e6b10..9cf2b677e6 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -560,8 +560,12 @@ public: options.create_flags = wallet_creation_flags; options.create_passphrase = passphrase; bilingual_str error; - util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, CreateWallet(m_context, name, /*load_on_start=*/true, options, status, error, warnings))}; - return wallet ? std::move(wallet) : util::Error{error}; + std::unique_ptr<Wallet> wallet{MakeWallet(m_context, CreateWallet(m_context, name, /*load_on_start=*/true, options, status, error, warnings))}; + if (wallet) { + return {std::move(wallet)}; + } else { + return util::Error{error}; + } } util::Result<std::unique_ptr<Wallet>> loadWallet(const std::string& name, std::vector<bilingual_str>& warnings) override { @@ -570,15 +574,23 @@ public: ReadDatabaseArgs(*m_context.args, options); options.require_existing = true; bilingual_str error; - util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, LoadWallet(m_context, name, /*load_on_start=*/true, options, status, error, warnings))}; - return wallet ? std::move(wallet) : util::Error{error}; + std::unique_ptr<Wallet> wallet{MakeWallet(m_context, LoadWallet(m_context, name, /*load_on_start=*/true, options, status, error, warnings))}; + if (wallet) { + return {std::move(wallet)}; + } else { + return util::Error{error}; + } } util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) override { DatabaseStatus status; bilingual_str error; - util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))}; - return wallet ? std::move(wallet) : util::Error{error}; + std::unique_ptr<Wallet> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))}; + if (wallet) { + return {std::move(wallet)}; + } else { + return util::Error{error}; + } } std::string getWalletDir() override { diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 35df151e84..8babbb4298 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1266,15 +1266,15 @@ RPCHelpMan importmulti() { {"desc", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Descriptor to import. If using descriptor, do not also provide address/scriptPubKey, scripts, or pubkeys"}, {"scriptPubKey", RPCArg::Type::STR, RPCArg::Optional::NO, "Type of scriptPubKey (string for script, json for address). Should not be provided if using a descriptor", - /*oneline_description=*/"", {"\"<script>\" | { \"address\":\"<address>\" }", "string / json"} + RPCArgOptions{.type_str={"\"<script>\" | { \"address\":\"<address>\" }", "string / json"}} }, {"timestamp", RPCArg::Type::NUM, RPCArg::Optional::NO, "Creation time of the key expressed in " + UNIX_EPOCH_TIME + ",\n" - " or the string \"now\" to substitute the current synced blockchain time. The timestamp of the oldest\n" - " key will determine how far back blockchain rescans need to begin for missing wallet transactions.\n" - " \"now\" can be specified to bypass scanning, for keys which are known to never have been used, and\n" - " 0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest key\n" - " creation time of all keys being imported by the importmulti call will be scanned.", - /*oneline_description=*/"", {"timestamp | \"now\"", "integer / string"} + "or the string \"now\" to substitute the current synced blockchain time. The timestamp of the oldest\n" + "key will determine how far back blockchain rescans need to begin for missing wallet transactions.\n" + "\"now\" can be specified to bypass scanning, for keys which are known to never have been used, and\n" + "0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest key\n" + "creation time of all keys being imported by the importmulti call will be scanned.", + RPCArgOptions{.type_str={"timestamp | \"now\"", "integer / string"}} }, {"redeemscript", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Allowed only if the scriptPubKey is a P2SH or P2SH-P2WSH address/scriptPubKey"}, {"witnessscript", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Allowed only if the scriptPubKey is a P2SH-P2WSH or P2WSH address/scriptPubKey"}, @@ -1296,12 +1296,12 @@ RPCHelpMan importmulti() }, }, }, - "\"requests\""}, + RPCArgOptions{.oneline_description="\"requests\""}}, {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", { {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions after all imports."}, }, - "\"options\""}, + RPCArgOptions{.oneline_description="\"options\""}}, }, RPCResult{ RPCResult::Type::ARR, "", "Response is an array with the same size as the input that has the execution result", @@ -1363,7 +1363,18 @@ RPCHelpMan importmulti() UniValue response(UniValue::VARR); { LOCK(pwallet->cs_wallet); - EnsureWalletIsUnlocked(*pwallet); + + // Check all requests are watchonly + bool is_watchonly{true}; + for (size_t i = 0; i < requests.size(); ++i) { + const UniValue& request = requests[i]; + if (!request.exists("watchonly") || !request["watchonly"].get_bool()) { + is_watchonly = false; + break; + } + } + // Wallet does not need to be unlocked if all requests are watchonly + if (!is_watchonly) EnsureWalletIsUnlocked(wallet); // Verify all timestamps are present before importing any keys. CHECK_NONFATAL(pwallet->chain().findBlock(pwallet->GetLastBlockHash(), FoundBlock().time(nLowestTimestamp).mtpTime(now))); @@ -1598,18 +1609,18 @@ RPCHelpMan importdescriptors() {"range", RPCArg::Type::RANGE, RPCArg::Optional::OMITTED, "If a ranged descriptor is used, this specifies the end or the range (in the form [begin,end]) to import"}, {"next_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "If a ranged descriptor is set to active, this specifies the next index to generate addresses from"}, {"timestamp", RPCArg::Type::NUM, RPCArg::Optional::NO, "Time from which to start rescanning the blockchain for this descriptor, in " + UNIX_EPOCH_TIME + "\n" - " Use the string \"now\" to substitute the current synced blockchain time.\n" - " \"now\" can be specified to bypass scanning, for outputs which are known to never have been used, and\n" - " 0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest timestamp\n" + "Use the string \"now\" to substitute the current synced blockchain time.\n" + "\"now\" can be specified to bypass scanning, for outputs which are known to never have been used, and\n" + "0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest timestamp\n" "of all descriptors being imported will be scanned as well as the mempool.", - /*oneline_description=*/"", {"timestamp | \"now\"", "integer / string"} + RPCArgOptions{.type_str={"timestamp | \"now\"", "integer / string"}} }, {"internal", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether matching outputs should be treated as not incoming payments (e.g. change)"}, {"label", RPCArg::Type::STR, RPCArg::Default{""}, "Label to assign to the address, only allowed with internal=false. Disabled for ranged descriptors"}, }, }, }, - "\"requests\""}, + RPCArgOptions{.oneline_description="\"requests\""}}, }, RPCResult{ RPCResult::Type::ARR, "", "Response is an array with the same size as the input that has the execution result", @@ -1749,7 +1760,7 @@ RPCHelpMan listdescriptors() }, RPCResult{RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "wallet_name", "Name of wallet this operation was performed on"}, - {RPCResult::Type::ARR, "descriptors", "Array of descriptor objects", + {RPCResult::Type::ARR, "descriptors", "Array of descriptor objects (sorted by descriptor string representation)", { {RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "desc", "Descriptor string representation"}, diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 50766f20eb..9c0c953a7a 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -290,8 +290,6 @@ RPCHelpMan lockunspent() LOCK(pwallet->cs_wallet); - RPCTypeCheckArgument(request.params[0], UniValue::VBOOL); - bool fUnlock = request.params[0].get_bool(); const bool persistent{request.params[2].isNull() ? false : request.params[2].get_bool()}; @@ -304,9 +302,7 @@ RPCHelpMan lockunspent() return true; } - RPCTypeCheckArgument(request.params[1], UniValue::VARR); - - const UniValue& output_params = request.params[1]; + const UniValue& output_params = request.params[1].get_array(); // Create and validate the COutPoints first. @@ -520,7 +516,7 @@ RPCHelpMan listunspent() {"maximumCount", RPCArg::Type::NUM, RPCArg::DefaultHint{"unlimited"}, "Maximum number of UTXOs"}, {"minimumSumAmount", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"unlimited"}, "Minimum sum value of all UTXOs in " + CURRENCY_UNIT + ""}, }, - "query_options"}, + RPCArgOptions{.oneline_description="query_options"}}, }, RPCResult{ RPCResult::Type::ARR, "", "", @@ -566,19 +562,16 @@ RPCHelpMan listunspent() int nMinDepth = 1; if (!request.params[0].isNull()) { - RPCTypeCheckArgument(request.params[0], UniValue::VNUM); nMinDepth = request.params[0].getInt<int>(); } int nMaxDepth = 9999999; if (!request.params[1].isNull()) { - RPCTypeCheckArgument(request.params[1], UniValue::VNUM); nMaxDepth = request.params[1].getInt<int>(); } std::set<CTxDestination> destinations; if (!request.params[2].isNull()) { - RPCTypeCheckArgument(request.params[2], UniValue::VARR); UniValue inputs = request.params[2].get_array(); for (unsigned int idx = 0; idx < inputs.size(); idx++) { const UniValue& input = inputs[idx]; @@ -594,7 +587,6 @@ RPCHelpMan listunspent() bool include_unsafe = true; if (!request.params[3].isNull()) { - RPCTypeCheckArgument(request.params[3], UniValue::VBOOL); include_unsafe = request.params[3].get_bool(); } diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index e38b13624c..ebf694157b 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -317,7 +317,7 @@ RPCHelpMan sendmany() "\nSend multiple times. Amounts are double-precision floating point numbers." + HELP_REQUIRING_PASSPHRASE, { - {"dummy", RPCArg::Type::STR, RPCArg::Optional::NO, "Must be set to \"\" for backwards compatibility.", "\"\""}, + {"dummy", RPCArg::Type::STR, RPCArg::Optional::NO, "Must be set to \"\" for backwards compatibility.", RPCArgOptions{.oneline_description="\"\""}}, {"amounts", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::NO, "The addresses and amounts", { {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The bitcoin address is the key, the numeric amount (can be string) in " + CURRENCY_UNIT + " is the value"}, @@ -338,7 +338,7 @@ RPCHelpMan sendmany() {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" "\"" + FeeModes("\"\n\"") + "\""}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, - {"verbose", RPCArg::Type::BOOL, RPCArg::Default{false}, "If true, return extra infomration about the transaction."}, + {"verbose", RPCArg::Type::BOOL, RPCArg::Default{false}, "If true, return extra information about the transaction."}, }, { RPCResult{"if verbose is not set or set to false", @@ -503,7 +503,6 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, coinControl.fAllowWatchOnly = options.get_bool(); } else { - RPCTypeCheckArgument(options, UniValue::VOBJ); RPCTypeCheckObj(options, { {"add_inputs", UniValueType(UniValue::VBOOL)}, @@ -775,7 +774,7 @@ RPCHelpMan fundrawtransaction() }, }, FundTxDoc()), - "options"}, + RPCArgOptions{.oneline_description="options"}}, {"iswitness", RPCArg::Type::BOOL, RPCArg::DefaultHint{"depends on heuristic tests"}, "Whether the transaction hex is a serialized witness transaction.\n" "If iswitness is not present, heuristic tests will be used in decoding.\n" "If true, only witness deserialization will be tried.\n" @@ -969,7 +968,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" "\"" + FeeModes("\"\n\"") + "\""}, }, - "options"}, + RPCArgOptions{.oneline_description="options"}}, }, RPCResult{ RPCResult::Type::OBJ, "", "", Cat( @@ -1174,7 +1173,7 @@ RPCHelpMan send() }, }, FundTxDoc()), - "options"}, + RPCArgOptions{.oneline_description="options"}}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -1282,7 +1281,7 @@ RPCHelpMan sendall() }, FundTxDoc() ), - "options" + RPCArgOptions{.oneline_description="options"} }, }, RPCResult{ @@ -1612,7 +1611,7 @@ RPCHelpMan walletcreatefundedpsbt() }, }, FundTxDoc()), - "options"}, + RPCArgOptions{.oneline_description="options"}}, {"bip32derivs", RPCArg::Type::BOOL, RPCArg::Default{true}, "Include BIP 32 derivation paths for public keys if we know them"}, }, RPCResult{ @@ -1653,7 +1652,6 @@ RPCHelpMan walletcreatefundedpsbt() bool rbf{wallet.m_signal_rbf}; const UniValue &replaceable_arg = options["replaceable"]; if (!replaceable_arg.isNull()) { - RPCTypeCheckArgument(replaceable_arg, UniValue::VBOOL); rbf = replaceable_arg.isTrue(); } CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index 3c1634324e..1aa2a87e99 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -12,10 +12,26 @@ #include <univalue.h> +#include <boost/date_time/posix_time/posix_time.hpp> + namespace wallet { static const std::string WALLET_ENDPOINT_BASE = "/wallet/"; const std::string HELP_REQUIRING_PASSPHRASE{"\nRequires wallet passphrase to be set with walletpassphrase call if wallet is encrypted.\n"}; +int64_t ParseISO8601DateTime(const std::string& str) +{ + static const boost::posix_time::ptime epoch = boost::posix_time::from_time_t(0); + static const std::locale loc(std::locale::classic(), + new boost::posix_time::time_input_facet("%Y-%m-%dT%H:%M:%SZ")); + std::istringstream iss(str); + iss.imbue(loc); + boost::posix_time::ptime ptime(boost::date_time::not_a_date_time); + iss >> ptime; + if (ptime.is_not_a_date_time() || epoch > ptime) + return 0; + return (ptime - epoch).total_seconds(); +} + bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param) { bool can_avoid_reuse = wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); bool avoid_reuse = param.isNull() ? can_avoid_reuse : param.get_bool(); diff --git a/src/wallet/rpc/util.h b/src/wallet/rpc/util.h index 2ca28914e7..87d34f7c11 100644 --- a/src/wallet/rpc/util.h +++ b/src/wallet/rpc/util.h @@ -45,6 +45,8 @@ std::string LabelFromValue(const UniValue& value); void PushParentDescriptors(const CWallet& wallet, const CScript& script_pubkey, UniValue& entry); void HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& status, bilingual_str& error); + +int64_t ParseISO8601DateTime(const std::string& str); } // namespace wallet #endif // BITCOIN_WALLET_RPC_UTIL_H diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 41654579c6..6da0741d59 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -586,7 +586,7 @@ bool LegacyScriptPubKeyMan::CanProvide(const CScript& script, SignatureData& sig // or solving information, even if not able to sign fully. return true; } else { - // If, given the stuff in sigdata, we could make a valid sigature, then we can provide for this script + // If, given the stuff in sigdata, we could make a valid signature, then we can provide for this script ProduceSignature(*this, DUMMY_SIGNATURE_CREATOR, script, sigdata); if (!sigdata.signatures.empty()) { // If we could make signatures, make sure we have a private key to actually make a signature diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index d84310c323..6833f9a095 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -260,32 +260,24 @@ CoinsResult AvailableCoins(const CWallet& wallet, // Filter by spendable outputs only if (!spendable && only_spendable) continue; - // If the Output is P2SH and spendable, we want to know if it is + // Obtain script type + std::vector<std::vector<uint8_t>> script_solutions; + TxoutType type = Solver(output.scriptPubKey, script_solutions); + + // If the output is P2SH and solvable, we want to know if it is // a P2SH (legacy) or one of P2SH-P2WPKH, P2SH-P2WSH (P2SH-Segwit). We can determine - // this from the redeemScript. If the Output is not spendable, it will be classified + // this from the redeemScript. If the output is not solvable, it will be classified // as a P2SH (legacy), since we have no way of knowing otherwise without the redeemScript - CScript script; bool is_from_p2sh{false}; - if (output.scriptPubKey.IsPayToScriptHash() && solvable) { - CTxDestination destination; - if (!ExtractDestination(output.scriptPubKey, destination)) - continue; - const CScriptID& hash = CScriptID(std::get<ScriptHash>(destination)); - if (!provider->GetCScript(hash, script)) - continue; + if (type == TxoutType::SCRIPTHASH && solvable) { + CScript script; + if (!provider->GetCScript(CScriptID(uint160(script_solutions[0])), script)) continue; + type = Solver(script, script_solutions); is_from_p2sh = true; - } else { - script = output.scriptPubKey; } - COutput coin(outpoint, output, nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate); - - // When parsing a scriptPubKey, Solver returns the parsed pubkeys or hashes (depending on the script) - // We don't need those here, so we are leaving them in return_values_unused - std::vector<std::vector<uint8_t>> return_values_unused; - TxoutType type; - type = Solver(script, return_values_unused); - result.Add(GetOutputType(type, is_from_p2sh), coin); + result.Add(GetOutputType(type, is_from_p2sh), + COutput(outpoint, output, nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate)); // Cache total amount as we go result.total_amount += output.nValue; @@ -590,7 +582,13 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a if (coin_control.HasSelected() && !coin_control.m_allow_other_inputs) { SelectionResult result(nTargetValue, SelectionAlgorithm::MANUAL); result.AddInput(preset_inputs); - if (result.GetSelectedValue() < nTargetValue) return std::nullopt; + + if (!coin_selection_params.m_subtract_fee_outputs && result.GetSelectedEffectiveValue() < nTargetValue) { + return std::nullopt; + } else if (result.GetSelectedValue() < nTargetValue) { + return std::nullopt; + } + result.ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); return result; } diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 30a7fb9eb6..23f024247d 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -922,5 +922,52 @@ BOOST_AUTO_TEST_CASE(effective_value_test) BOOST_CHECK_EQUAL(output5.GetEffectiveValue(), nValue); // The effective value should be equal to the absolute value if input_bytes is -1 } +BOOST_AUTO_TEST_CASE(SelectCoins_effective_value_test) +{ + // Test that the effective value is used to check whether preset inputs provide sufficient funds when subtract_fee_outputs is not used. + // This test creates a coin whose value is higher than the target but whose effective value is lower than the target. + // The coin is selected using coin control, with m_allow_other_inputs = false. SelectCoins should fail due to insufficient funds. + + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); + wallet->LoadWallet(); + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetupDescriptorScriptPubKeyMans(); + + CoinsResult available_coins; + { + std::unique_ptr<CWallet> dummyWallet = std::make_unique<CWallet>(m_node.chain.get(), "dummy", m_args, CreateMockWalletDatabase()); + dummyWallet->LoadWallet(); + LOCK(dummyWallet->cs_wallet); + dummyWallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + dummyWallet->SetupDescriptorScriptPubKeyMans(); + + add_coin(available_coins, *dummyWallet, 100000); // 0.001 BTC + } + + CAmount target{99900}; // 0.000999 BTC + + FastRandomContext rand; + CoinSelectionParams cs_params{ + rand, + /*change_output_size=*/34, + /*change_spend_size=*/148, + /*min_change_target=*/1000, + /*effective_feerate=*/CFeeRate(3000), + /*long_term_feerate=*/CFeeRate(1000), + /*discard_feerate=*/CFeeRate(1000), + /*tx_noinputs_size=*/0, + /*avoid_partial=*/false, + }; + CCoinControl cc; + cc.m_allow_other_inputs = false; + COutput output = available_coins.All().at(0); + cc.SetInputWeight(output.outpoint, 148); + cc.SelectExternal(output.outpoint, output.txout); + + const auto result = SelectCoins(*wallet, available_coins, target, cc, cs_params); + BOOST_CHECK(!result); +} + BOOST_AUTO_TEST_SUITE_END() } // namespace wallet diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp index 90a328179e..1a0708c43a 100644 --- a/src/wallet/test/fuzz/coinselection.cpp +++ b/src/wallet/test/fuzz/coinselection.cpp @@ -30,7 +30,7 @@ static void GroupCoins(FuzzedDataProvider& fuzzed_data_provider, const std::vect bool valid_outputgroup{false}; for (auto& coin : coins) { output_group.Insert(coin, /*ancestors=*/0, /*descendants=*/0, positive_only); - // If positive_only was specified, nothing may have been inserted, leading to an empty outpout group + // If positive_only was specified, nothing may have been inserted, leading to an empty output group // that would be invalid for the BnB algorithm valid_outputgroup = !positive_only || output_group.GetSelectionAmount() > 0; if (valid_outputgroup && fuzzed_data_provider.ConsumeBool()) { diff --git a/src/test/fuzz/parse_iso8601.cpp b/src/wallet/test/fuzz/parse_iso8601.cpp index 0fef9a9a1d..5be248c2fb 100644 --- a/src/test/fuzz/parse_iso8601.cpp +++ b/src/wallet/test/fuzz/parse_iso8601.cpp @@ -5,6 +5,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <util/time.h> +#include <wallet/rpc/util.h> #include <cassert> #include <cstdint> @@ -20,7 +21,7 @@ FUZZ_TARGET(parse_iso8601) const std::string iso8601_datetime = FormatISO8601DateTime(random_time); (void)FormatISO8601Date(random_time); - const int64_t parsed_time_1 = ParseISO8601DateTime(iso8601_datetime); + const int64_t parsed_time_1 = wallet::ParseISO8601DateTime(iso8601_datetime); if (random_time >= 0) { assert(parsed_time_1 >= 0); if (iso8601_datetime.length() == 20) { @@ -28,6 +29,6 @@ FUZZ_TARGET(parse_iso8601) } } - const int64_t parsed_time_2 = ParseISO8601DateTime(random_string); + const int64_t parsed_time_2 = wallet::ParseISO8601DateTime(random_string); assert(parsed_time_2 >= 0); } diff --git a/src/wallet/test/rpc_util_tests.cpp b/src/wallet/test/rpc_util_tests.cpp new file mode 100644 index 0000000000..32f6f5ab46 --- /dev/null +++ b/src/wallet/test/rpc_util_tests.cpp @@ -0,0 +1,24 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <wallet/rpc/util.h> + +#include <boost/test/unit_test.hpp> + +namespace wallet { + +BOOST_AUTO_TEST_SUITE(wallet_util_tests) + +BOOST_AUTO_TEST_CASE(util_ParseISO8601DateTime) +{ + BOOST_CHECK_EQUAL(ParseISO8601DateTime("1970-01-01T00:00:00Z"), 0); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("1960-01-01T00:00:00Z"), 0); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("2000-01-01T00:00:01Z"), 946684801); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("2011-09-30T23:36:17Z"), 1317425777); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("2100-12-31T23:59:59Z"), 4133980799); +} + +BOOST_AUTO_TEST_SUITE_END() + +} // namespace wallet diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c084ef10ec..461e64751a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -124,7 +124,7 @@ bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet interfaces::Chain& chain = wallet->chain(); std::string name = wallet->GetName(); - // Unregister with the validation interface which also drops shared ponters. + // Unregister with the validation interface which also drops shared pointers. wallet->m_chain_notifications_handler.reset(); LOCK(context.wallets_mutex); std::vector<std::shared_ptr<CWallet>>::iterator i = std::find(context.wallets.begin(), context.wallets.end(), wallet); @@ -386,25 +386,31 @@ std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& b ReadDatabaseArgs(*context.args, options); options.require_existing = true; - if (!fs::exists(backup_file)) { - error = Untranslated("Backup file does not exist"); - status = DatabaseStatus::FAILED_INVALID_BACKUP_FILE; - return nullptr; - } - const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), fs::u8path(wallet_name)); + auto wallet_file = wallet_path / "wallet.dat"; + std::shared_ptr<CWallet> wallet; - if (fs::exists(wallet_path) || !TryCreateDirectories(wallet_path)) { - error = Untranslated(strprintf("Failed to create database path '%s'. Database already exists.", fs::PathToString(wallet_path))); - status = DatabaseStatus::FAILED_ALREADY_EXISTS; - return nullptr; - } + try { + if (!fs::exists(backup_file)) { + error = Untranslated("Backup file does not exist"); + status = DatabaseStatus::FAILED_INVALID_BACKUP_FILE; + return nullptr; + } - auto wallet_file = wallet_path / "wallet.dat"; - fs::copy_file(backup_file, wallet_file, fs::copy_options::none); + if (fs::exists(wallet_path) || !TryCreateDirectories(wallet_path)) { + error = Untranslated(strprintf("Failed to create database path '%s'. Database already exists.", fs::PathToString(wallet_path))); + status = DatabaseStatus::FAILED_ALREADY_EXISTS; + return nullptr; + } - auto wallet = LoadWallet(context, wallet_name, load_on_start, options, status, error, warnings); + fs::copy_file(backup_file, wallet_file, fs::copy_options::none); + wallet = LoadWallet(context, wallet_name, load_on_start, options, status, error, warnings); + } catch (const std::exception& e) { + assert(!wallet); + if (!error.empty()) error += Untranslated("\n"); + error += strprintf(Untranslated("Unexpected exception: %s"), e.what()); + } if (!wallet) { fs::remove(wallet_file); fs::remove(wallet_path); @@ -1901,7 +1907,7 @@ std::set<uint256> CWallet::GetTxConflicts(const CWalletTx& wtx) const // mempool to relay them. On startup, we will do this for all unconfirmed // transactions but will not ask the mempool to relay them. We do this on startup // to ensure that our own mempool is aware of our transactions, and to also -// initialize nNextResend so that the actual rebroadcast is scheduled. There +// initialize m_next_resend so that the actual rebroadcast is scheduled. There // is a privacy side effect here as not broadcasting on startup also means that we won't // inform the world of our wallet's state, particularly if the wallet (or node) is not // yet synced. @@ -1935,9 +1941,9 @@ void CWallet::ResubmitWalletTransactions(bool relay, bool force) // Do this infrequently and randomly to avoid giving away // that these are our transactions. - if (!force && GetTime() < nNextResend) return; + if (!force && GetTime() < m_next_resend) return; // resend 12-36 hours from now, ~1 day on average. - nNextResend = GetTime() + (12 * 60 * 60) + GetRand(24 * 60 * 60); + m_next_resend = GetTime() + (12 * 60 * 60) + GetRand(24 * 60 * 60); int submitted_tx_count = 0; @@ -2104,6 +2110,7 @@ SigningResult CWallet::SignMessage(const std::string& message, const PKHash& pkh CScript script_pub_key = GetScriptForDestination(pkhash); for (const auto& spk_man_pair : m_spk_managers) { if (spk_man_pair.second->CanProvide(script_pub_key, sigdata)) { + LOCK(cs_wallet); // DescriptorScriptPubKeyMan calls IsLocked which can lock cs_wallet in a deadlocking order return spk_man_pair.second->SignMessage(message, pkhash, str_sig); } } @@ -3104,12 +3111,14 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf // If a block is pruned after this check, we will load the wallet, // but fail the rescan with a generic error. - error = chain.hasAssumedValidChain() ? - _( - "Assumed-valid: last wallet synchronisation goes beyond " - "available block data. You need to wait for the background " - "validation chain to download more blocks.") : - _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)"); + error = chain.havePruned() ? + _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)") : + strprintf(_( + "Error loading wallet. Wallet requires blocks to be downloaded, " + "and software does not currently support loading wallets while " + "blocks are being downloaded out of order when using assumeutxo " + "snapshots. Wallet should be able to load successfully after " + "node sync reaches height %s"), block_height); return false; } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f4ea5f1920..953d3e5bde 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -195,7 +195,7 @@ public: util::Result<CTxDestination> GetReservedDestination(bool internal); //! Return reserved address void ReturnDestination(); - //! Keep the address. Do not return it's key to the keypool when this object goes out of scope + //! Keep the address. Do not return its key to the keypool when this object goes out of scope void KeepDestination(); }; @@ -250,7 +250,7 @@ private: int nWalletVersion GUARDED_BY(cs_wallet){FEATURE_BASE}; /** The next scheduled rebroadcast of wallet transactions. */ - int64_t nNextResend = 0; + std::atomic<int64_t> m_next_resend{}; /** Whether this wallet will submit newly created transactions to the node's mempool and * prompt rebroadcasts (see ResendWalletTransactions()). */ bool fBroadcastTransactions = false; diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index 26618735f6..b9b7019a3c 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -8,7 +8,7 @@ #include <zmq.h> -#include <validation.h> +#include <primitives/block.h> #include <util/system.h> CZMQNotificationInterface::CZMQNotificationInterface() : pcontext(nullptr) diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index 011336bf72..51c8ad515e 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -11,7 +11,6 @@ #include <rpc/server.h> #include <streams.h> #include <util/system.h> -#include <validation.h> // For cs_main #include <zmq/zmqutil.h> #include <zmq.h> |