diff options
Diffstat (limited to 'src')
119 files changed, 1077 insertions, 1001 deletions
diff --git a/src/.clang-tidy b/src/.clang-tidy index b1f543f610..e9807d4cb7 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -1,11 +1,13 @@ Checks: ' -*, bugprone-argument-comment, +modernize-use-default-member-init, modernize-use-nullptr, readability-redundant-declaration, ' WarningsAsErrors: ' bugprone-argument-comment, +modernize-use-default-member-init, modernize-use-nullptr, readability-redundant-declaration, ' diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 02a3f9ae7d..77ff683974 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -364,8 +364,8 @@ endif if TARGET_WINDOWS else if ENABLE_BENCH - @echo "Running bench/bench_bitcoin ..." - $(BENCH_BINARY) > /dev/null + @echo "Running bench/bench_bitcoin (one iteration sanity check)..." + $(BENCH_BINARY) --sanity-check > /dev/null endif endif $(AM_V_at)$(MAKE) $(AM_MAKEFLAGS) -C secp256k1 check diff --git a/src/addrdb.cpp b/src/addrdb.cpp index b9eae292d7..31f8eadf98 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -49,8 +49,7 @@ template <typename Data> bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data& data, int version) { // Generate random temporary filename - uint16_t randv = 0; - GetRandBytes({(unsigned char*)&randv, sizeof(randv)}); + const uint16_t randv{GetRand<uint16_t>()}; std::string tmpfn = strprintf("%s.%04x", prefix, randv); // open temp output file, and associate with CAutoFile diff --git a/src/bench/bench.cpp b/src/bench/bench.cpp index 033d319750..d26b52410c 100644 --- a/src/bench/bench.cpp +++ b/src/bench/bench.cpp @@ -57,6 +57,10 @@ void benchmark::BenchRunner::RunAll(const Args& args) std::regex reFilter(args.regex_filter); std::smatch baseMatch; + if (args.sanity_check) { + std::cout << "Running with --sanity check option, benchmark results will be useless." << std::endl; + } + std::vector<ankerl::nanobench::Result> benchmarkResults; for (const auto& p : benchmarks()) { if (!std::regex_match(p.first, baseMatch, reFilter)) { @@ -69,6 +73,9 @@ void benchmark::BenchRunner::RunAll(const Args& args) } Bench bench; + if (args.sanity_check) { + bench.epochs(1).epochIterations(1); + } bench.name(p.first); if (args.min_time > 0ms) { // convert to nanos before dividing to reduce rounding errors diff --git a/src/bench/bench.h b/src/bench/bench.h index 6634138beb..17535e4e81 100644 --- a/src/bench/bench.h +++ b/src/bench/bench.h @@ -43,6 +43,7 @@ typedef std::function<void(Bench&)> BenchFunction; struct Args { bool is_list_only; + bool sanity_check; std::chrono::milliseconds min_time; std::vector<double> asymptote; fs::path output_csv; diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp index d6f9c0f8b5..1bb4d34db9 100644 --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -26,9 +26,10 @@ static void SetupBenchArgs(ArgsManager& argsman) argsman.AddArg("-asymptote=<n1,n2,n3,...>", "Test asymptotic growth of the runtime of an algorithm, if supported by the benchmark", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-filter=<regex>", strprintf("Regular expression filter to select benchmark by name (default: %s)", DEFAULT_BENCH_FILTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-list", "List benchmarks without executing them", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-min_time=<milliseconds>", strprintf("Minimum runtime per benchmark, in milliseconds (default: %d)", DEFAULT_MIN_TIME_MS), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); - argsman.AddArg("-output_csv=<output.csv>", "Generate CSV file with the most important benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-output_json=<output.json>", "Generate JSON file with all benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-min-time=<milliseconds>", strprintf("Minimum runtime per benchmark, in milliseconds (default: %d)", DEFAULT_MIN_TIME_MS), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); + argsman.AddArg("-output-csv=<output.csv>", "Generate CSV file with the most important benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-output-json=<output.json>", "Generate JSON file with all benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-sanity-check", "Run benchmarks for only one iteration", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); } // parses a comma separated list like "10,20,30,50" @@ -73,7 +74,7 @@ int main(int argc, char** argv) " sure each run has exactly the same preconditions.\n" "\n" " * If results are still not reliable, increase runtime with e.g.\n" - " -min_time=5000 to let a benchmark run for at least 5 seconds.\n" + " -min-time=5000 to let a benchmark run for at least 5 seconds.\n" "\n" " * bench_bitcoin uses nanobench [3] for which there is extensive\n" " documentation available online.\n" @@ -108,10 +109,11 @@ int main(int argc, char** argv) benchmark::Args args; args.asymptote = parseAsymptote(argsman.GetArg("-asymptote", "")); args.is_list_only = argsman.GetBoolArg("-list", false); - args.min_time = std::chrono::milliseconds(argsman.GetIntArg("-min_time", DEFAULT_MIN_TIME_MS)); - args.output_csv = argsman.GetPathArg("-output_csv"); - args.output_json = argsman.GetPathArg("-output_json"); + args.min_time = std::chrono::milliseconds(argsman.GetIntArg("-min-time", DEFAULT_MIN_TIME_MS)); + args.output_csv = argsman.GetPathArg("-output-csv"); + args.output_json = argsman.GetPathArg("-output-json"); args.regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER); + args.sanity_check = argsman.GetBoolArg("-sanity-check", false); benchmark::BenchRunner::RunAll(args); diff --git a/src/bench/checkqueue.cpp b/src/bench/checkqueue.cpp index 53591f8905..602081fb9b 100644 --- a/src/bench/checkqueue.cpp +++ b/src/bench/checkqueue.cpp @@ -30,8 +30,7 @@ static void CCheckQueueSpeedPrevectorJob(benchmark::Bench& bench) struct PrevectorJob { prevector<PREVECTOR_SIZE, uint8_t> p; - PrevectorJob(){ - } + PrevectorJob() = default; explicit PrevectorJob(FastRandomContext& insecure_rand){ p.resize(insecure_rand.randrange(PREVECTOR_SIZE*2)); } diff --git a/src/bench/prevector.cpp b/src/bench/prevector.cpp index 6343ed7848..b3688bab1b 100644 --- a/src/bench/prevector.cpp +++ b/src/bench/prevector.cpp @@ -10,8 +10,8 @@ #include <bench/bench.h> struct nontrivial_t { - int x; - nontrivial_t() :x(-1) {} + int x{-1}; + nontrivial_t() = default; SERIALIZE_METHODS(nontrivial_t, obj) { READWRITE(obj.x); } }; static_assert(!std::is_trivially_default_constructible<nontrivial_t>::value, diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index e5b6875a36..ab805ac1ec 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -70,7 +70,7 @@ int main(int argc, char* argv[]) // SETUP: Chainstate - ChainstateManager chainman; + ChainstateManager chainman{chainparams}; auto rv = node::LoadChainstate(false, std::ref(chainman), @@ -163,7 +163,7 @@ int main(int argc, char* argv[]) LOCK(cs_main); const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock); if (pindex) { - UpdateUncommittedBlockStructures(block, pindex, chainparams.GetConsensus()); + chainman.UpdateUncommittedBlockStructures(block, pindex); } } @@ -190,7 +190,7 @@ int main(int argc, char* argv[]) bool new_block; auto sc = std::make_shared<submitblock_StateCatcher>(block.GetHash()); RegisterSharedValidationInterface(sc); - bool accepted = chainman.ProcessNewBlock(chainparams, blockptr, /*force_processing=*/true, /*new_block=*/&new_block); + bool accepted = chainman.ProcessNewBlock(blockptr, /*force_processing=*/true, /*new_block=*/&new_block); UnregisterSharedValidationInterface(sc); if (!new_block && accepted) { std::cerr << "duplicate" << std::endl; diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index dea46693bc..c58167e033 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -9,23 +9,26 @@ #include <chainparamsbase.h> #include <clientversion.h> +#include <compat/stdin.h> #include <policy/feerate.h> #include <rpc/client.h> #include <rpc/mining.h> #include <rpc/protocol.h> #include <rpc/request.h> #include <tinyformat.h> +#include <univalue.h> #include <util/strencodings.h> #include <util/system.h> #include <util/translation.h> #include <util/url.h> #include <algorithm> +#include <chrono> #include <cmath> +#include <cstdio> #include <functional> #include <memory> #include <optional> -#include <stdio.h> #include <string> #include <tuple> @@ -37,8 +40,11 @@ #include <event2/keyvalq_struct.h> #include <support/events.h> -#include <univalue.h> -#include <compat/stdin.h> +// The server returns time values from a mockable system clock, but it is not +// trivial to get the mocked time from the server, nor is it needed for now, so +// just use a plain system_clock. +using CliClock = std::chrono::system_clock; +using CliSeconds = std::chrono::time_point<CliClock, std::chrono::seconds>; const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr; UrlDecodeFn* const URL_DECODE = urlDecode; @@ -174,10 +180,10 @@ static int AppInitRPC(int argc, char* argv[]) /** Reply structure for request_done to fill in */ struct HTTPReply { - HTTPReply(): status(0), error(-1) {} + HTTPReply() = default; - int status; - int error; + int status{0}; + int error{-1}; std::string body; }; @@ -238,7 +244,7 @@ static void http_error_cb(enum evhttp_request_error err, void *ctx) class BaseRequestHandler { public: - virtual ~BaseRequestHandler() {} + virtual ~BaseRequestHandler() = default; virtual UniValue PrepareRequest(const std::string& method, const std::vector<std::string>& args) = 0; virtual UniValue ProcessReply(const UniValue &batch_in) = 0; }; @@ -433,7 +439,6 @@ private: if (conn_type == "addr-fetch") return "addr"; return ""; } - const int64_t m_time_now{GetTimeSeconds()}; public: static constexpr int ID_PEERINFO = 0; @@ -465,6 +470,7 @@ public: if (networkinfo["version"].get_int() < 209900) { throw std::runtime_error("-netinfo requires bitcoind server to be running v0.21.0 and up"); } + const int64_t time_now{count_seconds(Now<CliSeconds>())}; // Count peer connection totals, and if DetailsRequested(), store peer data in a vector of structs. for (const UniValue& peer : batch[ID_PEERINFO]["result"].getValues()) { @@ -495,7 +501,7 @@ public: const double min_ping{peer["minping"].isNull() ? -1 : peer["minping"].get_real()}; const double ping{peer["pingtime"].isNull() ? -1 : peer["pingtime"].get_real()}; const std::string addr{peer["addr"].get_str()}; - const std::string age{conn_time == 0 ? "" : ToString((m_time_now - conn_time) / 60)}; + const std::string age{conn_time == 0 ? "" : ToString((time_now - conn_time) / 60)}; const std::string sub_version{peer["subver"].get_str()}; const bool is_addr_relay_enabled{peer["addr_relay_enabled"].isNull() ? false : peer["addr_relay_enabled"].get_bool()}; const bool is_bip152_hb_from{peer["bip152_hb_from"].get_bool()}; @@ -531,10 +537,10 @@ public: peer.network, PingTimeToString(peer.min_ping), PingTimeToString(peer.ping), - peer.last_send ? ToString(m_time_now - peer.last_send) : "", - peer.last_recv ? ToString(m_time_now - peer.last_recv) : "", - peer.last_trxn ? ToString((m_time_now - peer.last_trxn) / 60) : peer.is_block_relay ? "*" : "", - peer.last_blck ? ToString((m_time_now - peer.last_blck) / 60) : "", + peer.last_send ? ToString(time_now - peer.last_send) : "", + peer.last_recv ? ToString(time_now - peer.last_recv) : "", + peer.last_trxn ? ToString((time_now - peer.last_trxn) / 60) : peer.is_block_relay ? "*" : "", + peer.last_blck ? ToString((time_now - peer.last_blck) / 60) : "", strprintf("%s%s", peer.is_bip152_hb_to ? "." : " ", peer.is_bip152_hb_from ? "*" : " "), m_max_addr_processed_length, // variable spacing peer.addr_processed ? ToString(peer.addr_processed) : peer.is_addr_relay_enabled ? "" : ".", @@ -838,7 +844,7 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str // Execute and handle connection failures with -rpcwait. const bool fWait = gArgs.GetBoolArg("-rpcwait", false); const int timeout = gArgs.GetIntArg("-rpcwaittimeout", DEFAULT_WAIT_CLIENT_TIMEOUT); - const auto deadline{GetTime<std::chrono::microseconds>() + 1s * timeout}; + const auto deadline{std::chrono::steady_clock::now() + 1s * timeout}; do { try { @@ -851,8 +857,7 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str } break; // Connection succeeded, no need to retry. } catch (const CConnectionFailed& e) { - const auto now{GetTime<std::chrono::microseconds>()}; - if (fWait && (timeout <= 0 || now < deadline)) { + if (fWait && (timeout <= 0 || std::chrono::steady_clock::now() < deadline)) { UninterruptibleSleep(1s); } else { throw CConnectionFailed(strprintf("timeout on transient error: %s", e.what())); diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index aa111b5939..f96353510f 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -16,15 +16,15 @@ #include <unordered_map> -CBlockHeaderAndShortTxIDs::CBlockHeaderAndShortTxIDs(const CBlock& block, bool fUseWTXID) : - nonce(GetRand(std::numeric_limits<uint64_t>::max())), +CBlockHeaderAndShortTxIDs::CBlockHeaderAndShortTxIDs(const CBlock& block) : + nonce(GetRand<uint64_t>()), shorttxids(block.vtx.size() - 1), prefilledtxn(1), header(block) { FillShortTxIDSelector(); //TODO: Use our mempool prior to block acceptance to predictively fill more than just the coinbase prefilledtxn[0] = {0, block.vtx[0]}; for (size_t i = 1; i < block.vtx.size(); i++) { const CTransaction& tx = *block.vtx[i]; - shorttxids[i - 1] = GetShortID(fUseWTXID ? tx.GetWitnessHash() : tx.GetHash()); + shorttxids[i - 1] = GetShortID(tx.GetWitnessHash()); } } diff --git a/src/blockencodings.h b/src/blockencodings.h index 326db1b4a7..67c4e57156 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -104,7 +104,7 @@ public: // Dummy for deserialization CBlockHeaderAndShortTxIDs() {} - CBlockHeaderAndShortTxIDs(const CBlock& block, bool fUseWTXID); + CBlockHeaderAndShortTxIDs(const CBlock& block); uint64_t GetShortID(const uint256& txhash) const; diff --git a/src/checkqueue.h b/src/checkqueue.h index d0e88a3410..bead6f0c6f 100644 --- a/src/checkqueue.h +++ b/src/checkqueue.h @@ -66,7 +66,7 @@ private: bool m_request_stop GUARDED_BY(m_mutex){false}; /** Internal function that does bulk of the verification work. */ - bool Loop(bool fMaster) + bool Loop(bool fMaster) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { std::condition_variable& cond = fMaster ? m_master_cv : m_worker_cv; std::vector<T> vChecks; @@ -140,7 +140,7 @@ public: } //! Create a pool of new worker threads. - void StartWorkerThreads(const int threads_num) + void StartWorkerThreads(const int threads_num) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { { LOCK(m_mutex); @@ -159,13 +159,13 @@ public: } //! Wait until execution finishes, and return whether all evaluations were successful. - bool Wait() + bool Wait() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { return Loop(true /* master thread */); } //! Add a batch of checks to the queue - void Add(std::vector<T>& vChecks) + void Add(std::vector<T>& vChecks) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { if (vChecks.empty()) { return; @@ -188,7 +188,7 @@ public: } //! Stop all of the worker threads. - void StopWorkerThreads() + void StopWorkerThreads() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { WITH_LOCK(m_mutex, m_request_stop = true); m_worker_cv.notify_all(); diff --git a/src/common/bloom.cpp b/src/common/bloom.cpp index 8b32a6c94a..aa3fcf1ce2 100644 --- a/src/common/bloom.cpp +++ b/src/common/bloom.cpp @@ -239,7 +239,7 @@ bool CRollingBloomFilter::contains(Span<const unsigned char> vKey) const void CRollingBloomFilter::reset() { - nTweak = GetRand(std::numeric_limits<unsigned int>::max()); + nTweak = GetRand<unsigned int>(); nEntriesThisGeneration = 0; nGeneration = 1; std::fill(data.begin(), data.end(), 0); diff --git a/src/crypto/chacha20.cpp b/src/crypto/chacha20.cpp index f3ff4268ee..c7e12b0612 100644 --- a/src/crypto/chacha20.cpp +++ b/src/crypto/chacha20.cpp @@ -18,6 +18,8 @@ constexpr static inline uint32_t rotl32(uint32_t v, int c) { return (v << c) | ( a += b; d = rotl32(d ^ a, 8); \ c += d; b = rotl32(b ^ c, 7); +#define REPEAT10(a) do { {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; } while(0) + static const unsigned char sigma[] = "expand 32-byte k"; static const unsigned char tau[] = "expand 16-byte k"; @@ -119,16 +121,19 @@ void ChaCha20::Keystream(unsigned char* c, size_t bytes) x13 = j13; x14 = j14; x15 = j15; - for (i = 20;i > 0;i -= 2) { - QUARTERROUND( x0, x4, x8,x12) - QUARTERROUND( x1, x5, x9,x13) - QUARTERROUND( x2, x6,x10,x14) - QUARTERROUND( x3, x7,x11,x15) - QUARTERROUND( x0, x5,x10,x15) - QUARTERROUND( x1, x6,x11,x12) - QUARTERROUND( x2, x7, x8,x13) - QUARTERROUND( x3, x4, x9,x14) - } + + // The 20 inner ChaCha20 rounds are unrolled here for performance. + REPEAT10( + QUARTERROUND( x0, x4, x8,x12); + QUARTERROUND( x1, x5, x9,x13); + QUARTERROUND( x2, x6,x10,x14); + QUARTERROUND( x3, x7,x11,x15); + QUARTERROUND( x0, x5,x10,x15); + QUARTERROUND( x1, x6,x11,x12); + QUARTERROUND( x2, x7, x8,x13); + QUARTERROUND( x3, x4, x9,x14); + ); + x0 += j0; x1 += j1; x2 += j2; @@ -231,16 +236,19 @@ void ChaCha20::Crypt(const unsigned char* m, unsigned char* c, size_t bytes) x13 = j13; x14 = j14; x15 = j15; - for (i = 20;i > 0;i -= 2) { - QUARTERROUND( x0, x4, x8,x12) - QUARTERROUND( x1, x5, x9,x13) - QUARTERROUND( x2, x6,x10,x14) - QUARTERROUND( x3, x7,x11,x15) - QUARTERROUND( x0, x5,x10,x15) - QUARTERROUND( x1, x6,x11,x12) - QUARTERROUND( x2, x7, x8,x13) - QUARTERROUND( x3, x4, x9,x14) - } + + // The 20 inner ChaCha20 rounds are unrolled here for performance. + REPEAT10( + QUARTERROUND( x0, x4, x8,x12); + QUARTERROUND( x1, x5, x9,x13); + QUARTERROUND( x2, x6,x10,x14); + QUARTERROUND( x3, x7,x11,x15); + QUARTERROUND( x0, x5,x10,x15); + QUARTERROUND( x1, x6,x11,x12); + QUARTERROUND( x2, x7, x8,x13); + QUARTERROUND( x3, x4, x9,x14); + ); + x0 += j0; x1 += j1; x2 += j2; diff --git a/src/deploymentstatus.cpp b/src/deploymentstatus.cpp index ae19a6e40d..3a524af29e 100644 --- a/src/deploymentstatus.cpp +++ b/src/deploymentstatus.cpp @@ -9,8 +9,6 @@ #include <type_traits> -VersionBitsCache g_versionbitscache; - /* Basic sanity checking for BuriedDeployment/DeploymentPos enums and * ValidDeployment check */ diff --git a/src/deploymentstatus.h b/src/deploymentstatus.h index ba5103de74..9f5919f71b 100644 --- a/src/deploymentstatus.h +++ b/src/deploymentstatus.h @@ -10,33 +10,30 @@ #include <limits> -/** Global cache for versionbits deployment status */ -extern VersionBitsCache g_versionbitscache; - /** Determine if a deployment is active for the next block */ -inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::BuriedDeployment dep) +inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::BuriedDeployment dep, [[maybe_unused]] VersionBitsCache& versionbitscache) { assert(Consensus::ValidDeployment(dep)); return (pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1) >= params.DeploymentHeight(dep); } -inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos dep) +inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos dep, VersionBitsCache& versionbitscache) { assert(Consensus::ValidDeployment(dep)); - return ThresholdState::ACTIVE == g_versionbitscache.State(pindexPrev, params, dep); + return ThresholdState::ACTIVE == versionbitscache.State(pindexPrev, params, dep); } /** Determine if a deployment is active for this block */ -inline bool DeploymentActiveAt(const CBlockIndex& index, const Consensus::Params& params, Consensus::BuriedDeployment dep) +inline bool DeploymentActiveAt(const CBlockIndex& index, const Consensus::Params& params, Consensus::BuriedDeployment dep, [[maybe_unused]] VersionBitsCache& versionbitscache) { assert(Consensus::ValidDeployment(dep)); return index.nHeight >= params.DeploymentHeight(dep); } -inline bool DeploymentActiveAt(const CBlockIndex& index, const Consensus::Params& params, Consensus::DeploymentPos dep) +inline bool DeploymentActiveAt(const CBlockIndex& index, const Consensus::Params& params, Consensus::DeploymentPos dep, VersionBitsCache& versionbitscache) { assert(Consensus::ValidDeployment(dep)); - return DeploymentActiveAfter(index.pprev, params, dep); + return DeploymentActiveAfter(index.pprev, params, dep, versionbitscache); } /** Determine if a deployment is enabled (can ever be active) */ diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 75070899c6..d125fe479b 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -74,11 +74,12 @@ bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::str // Serialize the PSBT CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); ssTx << psbtx; - + // parse ExternalSigner master fingerprint + std::vector<unsigned char> parsed_m_fingerprint = ParseHex(m_fingerprint); // Check if signer fingerprint matches any input master key fingerprint auto matches_signer_fingerprint = [&](const PSBTInput& input) { for (const auto& entry : input.hd_keypaths) { - if (m_fingerprint == strprintf("%08x", ReadBE32(entry.second.fingerprint))) return true; + if (parsed_m_fingerprint == MakeUCharSpan(entry.second.fingerprint)) return true; } return false; }; diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 96bee8640d..b9a1fc672a 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -73,21 +73,18 @@ private: Mutex cs; std::condition_variable cond GUARDED_BY(cs); std::deque<std::unique_ptr<WorkItem>> queue GUARDED_BY(cs); - bool running GUARDED_BY(cs); + bool running GUARDED_BY(cs){true}; const size_t maxDepth; public: - explicit WorkQueue(size_t _maxDepth) : running(true), - maxDepth(_maxDepth) + explicit WorkQueue(size_t _maxDepth) : maxDepth(_maxDepth) { } /** Precondition: worker threads have all stopped (they have been joined). */ - ~WorkQueue() - { - } + ~WorkQueue() = default; /** Enqueue a work item */ - bool Enqueue(WorkItem* item) + bool Enqueue(WorkItem* item) EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); if (!running || queue.size() >= maxDepth) { @@ -98,7 +95,7 @@ public: return true; } /** Thread function */ - void Run() + void Run() EXCLUSIVE_LOCKS_REQUIRED(!cs) { while (true) { std::unique_ptr<WorkItem> i; @@ -115,7 +112,7 @@ public: } } /** Interrupt and exit loops */ - void Interrupt() + void Interrupt() EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); running = false; @@ -84,7 +84,7 @@ public: * to the listening socket and address. * @return true on success */ - bool Listen(Connection& conn); + bool Listen(Connection& conn) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** * Wait for and accept a new incoming connection. @@ -103,7 +103,7 @@ public: * it is set to `false`. Only set if `false` is returned. * @return true on success */ - bool Connect(const CService& to, Connection& conn, bool& proxy_error); + bool Connect(const CService& to, Connection& conn, bool& proxy_error) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); private: /** @@ -172,7 +172,7 @@ private: /** * Check the control socket for errors and possibly disconnect. */ - void CheckControlSock(); + void CheckControlSock() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** * Generate a new destination with the SAM proxy and set `m_private_key` to it. diff --git a/src/index/base.cpp b/src/index/base.cpp index 09f76adad9..9f0c1dea24 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -18,8 +18,8 @@ using node::ReadBlockFromDisk; constexpr uint8_t DB_BEST_BLOCK{'B'}; -constexpr int64_t SYNC_LOG_INTERVAL = 30; // seconds -constexpr int64_t SYNC_LOCATOR_WRITE_INTERVAL = 30; // seconds +constexpr auto SYNC_LOG_INTERVAL{30s}; +constexpr auto SYNC_LOCATOR_WRITE_INTERVAL{30s}; template <typename... Args> static void FatalError(const char* fmt, const Args&... args) @@ -130,8 +130,8 @@ void BaseIndex::ThreadSync() if (!m_synced) { auto& consensus_params = Params().GetConsensus(); - int64_t last_log_time = 0; - int64_t last_locator_write_time = 0; + std::chrono::steady_clock::time_point last_log_time{0s}; + std::chrono::steady_clock::time_point last_locator_write_time{0s}; while (true) { if (m_interrupt) { SetBestBlockIndex(pindex); @@ -160,7 +160,7 @@ void BaseIndex::ThreadSync() pindex = pindex_next; } - int64_t current_time = GetTime(); + auto current_time{std::chrono::steady_clock::now()}; if (last_log_time + SYNC_LOG_INTERVAL < current_time) { LogPrintf("Syncing %s with block chain from height %d\n", GetName(), pindex->nHeight); diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index b1836fe12f..6deff59000 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -64,7 +64,7 @@ public: bool LookupFilter(const CBlockIndex* block_index, BlockFilter& filter_out) const; /** Get a single filter header by block. */ - bool LookupFilterHeader(const CBlockIndex* block_index, uint256& header_out); + bool LookupFilterHeader(const CBlockIndex* block_index, uint256& header_out) EXCLUSIVE_LOCKS_REQUIRED(!m_cs_headers_cache); /** Get a range of filters between two heights on a chain. */ bool LookupFilterRange(int start_height, const CBlockIndex* stop_index, diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp index e1d807f39a..97c11c4383 100644 --- a/src/index/txindex.cpp +++ b/src/index/txindex.cpp @@ -52,7 +52,7 @@ TxIndex::TxIndex(size_t n_cache_size, bool f_memory, bool f_wipe) : m_db(std::make_unique<TxIndex::DB>(n_cache_size, f_memory, f_wipe)) {} -TxIndex::~TxIndex() {} +TxIndex::~TxIndex() = default; bool TxIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex) { diff --git a/src/init.cpp b/src/init.cpp index 713598f411..f06b5bd0d0 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -570,6 +570,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcauth=<userpw>", "Username and HMAC-SHA-256 hashed password for JSON-RPC connections. The field <userpw> comes in the format: <USERNAME>:<SALT>$<HASH>. A canonical python script is included in share/rpcauth. The client then connects normally using the rpcuser=<USERNAME>/rpcpassword=<PASSWORD> pair of arguments. This option can be specified multiple times", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC); argsman.AddArg("-rpcbind=<addr>[:port]", "Bind to given address to listen for JSON-RPC connections. Do not expose the RPC server to untrusted networks such as the public internet! This option is ignored unless -rpcallowip is also passed. Port is optional and overrides -rpcport. Use [host]:port notation for IPv6. This option can be specified multiple times (default: 127.0.0.1 and ::1 i.e., localhost)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY | ArgsManager::SENSITIVE, OptionsCategory::RPC); + argsman.AddArg("-rpcdoccheck", strprintf("Throw a non-fatal error at runtime if the documentation for an RPC is incorrect (default: %u)", DEFAULT_RPC_DOC_CHECK), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC); argsman.AddArg("-rpccookiefile=<loc>", "Location of the auth cookie. Relative paths will be prefixed by a net-specific datadir location. (default: data dir)", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcpassword=<pw>", "Password for JSON-RPC connections", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC); argsman.AddArg("-rpcport=<port>", strprintf("Listen for JSON-RPC connections on <port> (default: %u, testnet: %u, signet: %u, regtest: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort(), signetBaseParams->RPCPort(), regtestBaseParams->RPCPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::RPC); @@ -1280,8 +1281,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.banman); node.banman = std::make_unique<BanMan>(gArgs.GetDataDirNet() / "banlist", &uiInterface, args.GetIntArg("-bantime", DEFAULT_MISBEHAVING_BANTIME)); assert(!node.connman); - node.connman = std::make_unique<CConnman>(GetRand(std::numeric_limits<uint64_t>::max()), - GetRand(std::numeric_limits<uint64_t>::max()), + node.connman = std::make_unique<CConnman>(GetRand<uint64_t>(), + GetRand<uint64_t>(), *node.addrman, *node.netgroupman, args.GetBoolArg("-networkactive", true)); assert(!node.fee_estimator); @@ -1424,7 +1425,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) for (bool fLoaded = false; !fLoaded && !ShutdownRequested();) { node.mempool = std::make_unique<CTxMemPool>(node.fee_estimator.get(), mempool_check_ratio); - node.chainman = std::make_unique<ChainstateManager>(); + node.chainman = std::make_unique<ChainstateManager>(chainparams); ChainstateManager& chainman = *node.chainman; const bool fReset = fReindex; diff --git a/src/net.cpp b/src/net.cpp index 46d7020c5e..676037a357 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1867,6 +1867,12 @@ void CConnman::SetTryNewOutboundPeer(bool flag) LogPrint(BCLog::NET, "net: setting try another outbound peer=%s\n", flag ? "true" : "false"); } +void CConnman::StartExtraBlockRelayPeers() +{ + LogPrint(BCLog::NET, "net: enabling extra block-relay-only peers\n"); + m_start_extra_block_relay_peers = true; +} + // Return the number of peers we have over our outbound connection limit // Exclude peers that are marked for disconnect, or are going to be // disconnected soon (eg ADDR_FETCH and FEELER) @@ -2160,7 +2166,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) if (fFeeler) { // Add small amount of random noise before connection to avoid synchronization. - int randsleep = GetRandInt(FEELER_SLEEP_WINDOW * 1000); + int randsleep = GetRand<int>(FEELER_SLEEP_WINDOW * 1000); if (!interruptNet.sleep_for(std::chrono::milliseconds(randsleep))) return; LogPrint(BCLog::NET, "Making feeler connection to %s\n", addrConnect.ToString()); @@ -2685,7 +2691,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) class CNetCleanup { public: - CNetCleanup() {} + CNetCleanup() = default; ~CNetCleanup() { @@ -13,7 +13,6 @@ #include <crypto/siphash.h> #include <hash.h> #include <i2p.h> -#include <logging.h> #include <net_permissions.h> #include <netaddress.h> #include <netbase.h> @@ -613,7 +612,7 @@ public: * @return True if the peer should stay connected, * False if the peer should be disconnected from. */ - bool ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete); + bool ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete) EXCLUSIVE_LOCKS_REQUIRED(!cs_vRecv); void SetCommonVersion(int greatest_common_version) { @@ -625,9 +624,9 @@ public: return m_greatest_common_version; } - CService GetAddrLocal() const LOCKS_EXCLUDED(m_addr_local_mutex); + CService GetAddrLocal() const EXCLUSIVE_LOCKS_REQUIRED(!m_addr_local_mutex); //! May not be called more than once - void SetAddrLocal(const CService& addrLocalIn) LOCKS_EXCLUDED(m_addr_local_mutex); + void SetAddrLocal(const CService& addrLocalIn) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_local_mutex); CNode* AddRef() { @@ -640,9 +639,9 @@ public: nRefCount--; } - void CloseSocketDisconnect(); + void CloseSocketDisconnect() EXCLUSIVE_LOCKS_REQUIRED(!m_sock_mutex); - void CopyStats(CNodeStats& stats); + void CopyStats(CNodeStats& stats) EXCLUSIVE_LOCKS_REQUIRED(!m_subver_mutex, !m_addr_local_mutex, !cs_vSend, !cs_vRecv); ServiceFlags GetLocalServices() const { @@ -761,7 +760,7 @@ public: bool m_i2p_accept_incoming; }; - void Init(const Options& connOptions) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex) + void Init(const Options& connOptions) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_total_bytes_sent_mutex) { AssertLockNotHeld(m_total_bytes_sent_mutex); @@ -795,7 +794,8 @@ public: bool network_active = true); ~CConnman(); - bool Start(CScheduler& scheduler, const Options& options) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); + + bool Start(CScheduler& scheduler, const Options& options) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !m_added_nodes_mutex, !m_addr_fetches_mutex, !mutexMsgProc); void StopThreads(); void StopNodes(); @@ -805,7 +805,7 @@ public: StopNodes(); }; - void Interrupt(); + void Interrupt() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; void SetNetworkActive(bool active); @@ -857,10 +857,7 @@ public: void SetTryNewOutboundPeer(bool flag); bool GetTryNewOutboundPeer() const; - void StartExtraBlockRelayPeers() { - LogPrint(BCLog::NET, "net: enabling extra block-relay-only peers\n"); - m_start_extra_block_relay_peers = true; - } + void StartExtraBlockRelayPeers(); // Return the number of outbound peers we have in excess of our target (eg, // if we previously called SetTryNewOutboundPeer(true), and have since set @@ -872,9 +869,9 @@ public: // Count the number of block-relay-only peers we have over our limit. int GetExtraBlockRelayCount() const; - bool AddNode(const std::string& node); - bool RemoveAddedNode(const std::string& node); - std::vector<AddedNodeInfo> GetAddedNodeInfo() const; + bool AddNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); + bool RemoveAddedNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); + std::vector<AddedNodeInfo> GetAddedNodeInfo() const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); /** * Attempts to open a connection. Currently only used from tests. @@ -927,7 +924,7 @@ public: unsigned int GetReceiveFloodSize() const; - void WakeMessageHandler(); + void WakeMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); /** Return true if we should disconnect the peer for failing an inactivity check. */ bool ShouldRunInactivityChecks(const CNode& node, std::chrono::seconds now) const; @@ -954,11 +951,11 @@ private: bool Bind(const CService& addr, unsigned int flags, NetPermissionFlags permissions); bool InitBinds(const Options& options); - void ThreadOpenAddedConnections(); - void AddAddrFetch(const std::string& strDest); - void ProcessAddrFetch(); - void ThreadOpenConnections(std::vector<std::string> connect); - void ThreadMessageHandler(); + void ThreadOpenAddedConnections() EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); + void AddAddrFetch(const std::string& strDest) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex); + void ProcessAddrFetch() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex); + void ThreadOpenConnections(std::vector<std::string> connect) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex); + void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc); void ThreadI2PAcceptIncoming(); void AcceptConnection(const ListenSocket& hListenSocket); @@ -1009,7 +1006,7 @@ private: /** * Check connected and listening sockets for IO readiness and process them accordingly. */ - void SocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); + void SocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc); /** * Do the read/write for connected sockets that are ready for IO. @@ -1023,7 +1020,7 @@ private: const std::set<SOCKET>& recv_set, const std::set<SOCKET>& send_set, const std::set<SOCKET>& error_set) - EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc); /** * Accept incoming connections, one from each read-ready listening socket. @@ -1031,8 +1028,8 @@ private: */ void SocketHandlerListening(const std::set<SOCKET>& recv_set); - void ThreadSocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); - void ThreadDNSAddressSeed(); + void ThreadSocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc); + void ThreadDNSAddressSeed() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex); uint64_t CalculateKeyedNetGroup(const CAddress& ad) const; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 478368b673..f9178baf25 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -175,6 +175,8 @@ static constexpr double MAX_ADDR_RATE_PER_SECOND{0.1}; * based increments won't go above this, but the MAX_ADDR_TO_SEND increment following GETADDR * is exempt from this limit). */ static constexpr size_t MAX_ADDR_PROCESSING_TOKEN_BUCKET{MAX_ADDR_TO_SEND}; +/** The compactblocks version we support. See BIP 152. */ +static constexpr uint64_t CMPCTBLOCKS_VERSION{2}; // Internal stuff namespace { @@ -237,36 +239,62 @@ struct Peer { /** Whether this peer relays txs via wtxid */ std::atomic<bool> m_wtxid_relay{false}; + /** 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}; + /** Timestamp after which we will send the next BIP133 `feefilter` message + * to the peer. */ + std::chrono::microseconds m_next_send_feefilter{0}; struct TxRelay { mutable RecursiveMutex m_bloom_filter_mutex; - // We use m_relay_txs for two purposes - - // a) it allows us to not relay tx invs before receiving the peer's version message - // b) the peer may tell us in its version message that we should not relay tx invs - // unless it loads a bloom filter. + /** Whether the peer wishes to receive transaction announcements. + * + * This is initially set based on the fRelay flag in the received + * `version` message. If initially set to false, it can only be flipped + * to true if we have offered the peer NODE_BLOOM services and it sends + * us a `filterload` or `filterclear` message. See BIP37. */ bool m_relay_txs GUARDED_BY(m_bloom_filter_mutex){false}; + /** A bloom filter for which transactions to announce to the peer. See BIP37. */ std::unique_ptr<CBloomFilter> m_bloom_filter PT_GUARDED_BY(m_bloom_filter_mutex) GUARDED_BY(m_bloom_filter_mutex){nullptr}; mutable RecursiveMutex m_tx_inventory_mutex; + /** A filter of all the txids and wtxids that the peer has announced to + * us or we have announced to the peer. We use this to avoid announcing + * the same txid/wtxid to a peer that already has the transaction. */ CRollingBloomFilter m_tx_inventory_known_filter GUARDED_BY(m_tx_inventory_mutex){50000, 0.000001}; - // Set of transaction ids we still have to announce. - // They are sorted by the mempool before relay, so the order is not important. + /** Set of transaction ids we still have to announce (txid for + * non-wtxid-relay peers, wtxid for wtxid-relay peers). We use the + * mempool to sort transactions in dependency order before relay, so + * this does not have to be sorted. */ std::set<uint256> m_tx_inventory_to_send; - // Used for BIP35 mempool sending + /** Whether the peer has requested us to send our complete mempool. Only + * permitted if the peer has NetPermissionFlags::Mempool. See BIP35. */ bool m_send_mempool GUARDED_BY(m_tx_inventory_mutex){false}; - // Last time a "MEMPOOL" request was serviced. + /** The last time a BIP35 `mempool` request was serviced. */ 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}; - /** Minimum fee rate with which to filter inv's to this node */ + /** Minimum fee rate with which to filter transaction announcements to this node. See BIP133. */ std::atomic<CAmount> m_fee_filter_received{0}; - CAmount m_fee_filter_sent{0}; - std::chrono::microseconds m_next_send_feefilter{0}; }; - /** Transaction relay data. Will be a nullptr if we're not relaying - * transactions with this peer (e.g. if it's a block-relay-only peer) */ - std::unique_ptr<TxRelay> m_tx_relay; + /* Initializes a TxRelay struct for this peer. Can be called at most once for a peer. */ + TxRelay* SetTxRelay() + { + LOCK(m_tx_relay_mutex); + Assume(!m_tx_relay); + m_tx_relay = std::make_unique<Peer::TxRelay>(); + return m_tx_relay.get(); + }; + + TxRelay* GetTxRelay() + { + return WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get()); + }; /** A vector of addresses to send to the peer, limited to MAX_ADDR_TO_SEND. */ std::vector<CAddress> m_addrs_to_send; @@ -326,10 +354,17 @@ struct Peer { /** Work queue of items requested by this peer **/ std::deque<CInv> m_getdata_requests GUARDED_BY(m_getdata_requests_mutex); - explicit Peer(NodeId id, bool tx_relay) - : m_id(id) - , m_tx_relay(tx_relay ? std::make_unique<TxRelay>() : nullptr) + Peer(NodeId id) + : m_id{id} {} + +private: + Mutex m_tx_relay_mutex; + + /** Transaction relay data. Will be a nullptr if we're not relaying + * transactions with this peer (e.g. if it's a block-relay-only peer or + * the peer has sent us fRelay=false with bloom filters disabled). */ + std::unique_ptr<TxRelay> m_tx_relay GUARDED_BY(m_tx_relay_mutex); }; using PeerRef = std::shared_ptr<Peer>; @@ -365,23 +400,12 @@ struct CNodeState { bool fPreferredDownload{false}; //! Whether this peer wants invs or headers (when possible) for block announcements. bool fPreferHeaders{false}; - //! Whether this peer wants invs or cmpctblocks (when possible) for block announcements. - bool fPreferHeaderAndIDs{false}; - /** - * Whether this peer will send us cmpctblocks if we request them. - * This is not used to gate request logic, as we really only care about fSupportsDesiredCmpctVersion, - * but is used as a flag to "lock in" the version of compact blocks (fWantsCmpctWitness) we send. - */ - bool fProvidesHeaderAndIDs{false}; + /** Whether this peer wants invs or cmpctblocks (when possible) for block announcements. */ + bool m_requested_hb_cmpctblocks{false}; + /** Whether this peer will send us cmpctblocks if we request them. */ + bool m_provides_cmpctblocks{false}; //! Whether this peer can give us witnesses bool fHaveWitness{false}; - //! Whether this peer wants witnesses in cmpctblocks/blocktxns - bool fWantsCmpctWitness{false}; - /** - * If we've announced NODE_WITNESS to this peer: whether the peer sends witnesses in cmpctblocks/blocktxns, - * otherwise: whether this peer sends non-witnesses in cmpctblocks/blocktxns. - */ - bool fSupportsDesiredCmpctVersion{false}; /** State used to enforce CHAIN_SYNC_TIMEOUT and EXTRA_PEER_CHECK_INTERVAL logic. * @@ -440,30 +464,37 @@ public: CTxMemPool& pool, bool ignore_incoming_txs); /** Overridden from CValidationInterface. */ - void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override; - void BlockDisconnected(const std::shared_ptr<const CBlock> &block, const CBlockIndex* pindex) override; - void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; - void BlockChecked(const CBlock& block, const BlockValidationState& state) override; + void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override + EXCLUSIVE_LOCKS_REQUIRED(!m_recent_confirmed_transactions_mutex); + void BlockDisconnected(const std::shared_ptr<const CBlock> &block, const CBlockIndex* pindex) override + EXCLUSIVE_LOCKS_REQUIRED(!m_recent_confirmed_transactions_mutex); + void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void BlockChecked(const CBlock& block, const BlockValidationState& state) override + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& pblock) override; /** Implement NetEventsInterface */ - void InitializeNode(CNode* pnode) override; - void FinalizeNode(const CNode& node) override; - bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override; - bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing); + void InitializeNode(CNode* pnode) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex); + bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex); /** Implement PeerManager */ void StartScheduledTasks(CScheduler& scheduler) override; void CheckForStaleTipAndEvictPeers() override; std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override; - bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override; + bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool IgnoresIncomingTxs() override { return m_ignore_incoming_txs; } - void SendPings() override; - void RelayTransaction(const uint256& txid, const uint256& wtxid) override; + void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void SetBestHeight(int height) override { m_best_height = height; }; - void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message) override; + void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, - const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) override; + const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) override + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex); void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override; private: @@ -474,15 +505,15 @@ private: void EvictExtraOutboundPeers(std::chrono::seconds now) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Retrieve unbroadcast transactions from the mempool and reattempt sending to peers */ - void ReattemptInitialBroadcast(CScheduler& scheduler); + void ReattemptInitialBroadcast(CScheduler& scheduler) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Get a shared pointer to the Peer object. * May return an empty shared_ptr if the Peer object can't be found. */ - PeerRef GetPeerRef(NodeId id) const; + PeerRef GetPeerRef(NodeId id) const EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Get a shared pointer to the Peer object and remove it from m_peer_map. * May return an empty shared_ptr if the Peer object can't be found. */ - PeerRef RemovePeer(NodeId id); + PeerRef RemovePeer(NodeId id) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** * Potentially mark a node discouraged based on the contents of a BlockValidationState object @@ -495,14 +526,16 @@ private: * @return Returns true if the peer was punished (probably disconnected) */ bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& state, - bool via_compact_block, const std::string& message = ""); + bool via_compact_block, const std::string& message = "") + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** * Potentially disconnect and discourage a node based on the contents of a TxValidationState object * * @return Returns true if the peer was punished (probably disconnected) */ - bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, const std::string& message = ""); + bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, const std::string& message = "") + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Maybe disconnect a peer and discourage future connections from its address. * @@ -512,13 +545,16 @@ private: */ bool MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer); - void ProcessOrphanTx(std::set<uint256>& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans); + void ProcessOrphanTx(std::set<uint256>& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Process a single headers message from a peer. */ void ProcessHeadersMessage(CNode& pfrom, const Peer& peer, const std::vector<CBlockHeader>& headers, - bool via_compact_block); + bool via_compact_block) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); - void SendBlockTransactions(CNode& pfrom, const CBlock& block, const BlockTransactionsRequest& req); + void SendBlockTransactions(CNode& pfrom, const CBlock& block, const BlockTransactionsRequest& req) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Register with TxRequestTracker that an INV has been received from a * peer. The announcement parameters are decided in PeerManager and then @@ -545,7 +581,7 @@ private: * @param[in] fReachable Whether the address' network is reachable. We relay unreachable * addresses less. */ - void RelayAddress(NodeId originator, const CAddress& addr, bool fReachable); + void RelayAddress(NodeId originator, const CAddress& addr, bool fReachable) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Send `feefilter` message. */ void MaybeSendFeefilter(CNode& node, Peer& peer, std::chrono::microseconds current_time); @@ -615,7 +651,8 @@ private: /** Number of preferable block download peers. */ int m_num_preferred_download_peers GUARDED_BY(cs_main){0}; - bool AlreadyHaveTx(const GenTxid& gtxid) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool AlreadyHaveTx(const GenTxid& gtxid) + EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_recent_confirmed_transactions_mutex); /** * Filter for transactions that were recently rejected by the mempool. @@ -683,11 +720,10 @@ private: // All of the following cache a recent block, and are protected by m_most_recent_block_mutex - RecursiveMutex m_most_recent_block_mutex; + Mutex m_most_recent_block_mutex; std::shared_ptr<const CBlock> m_most_recent_block GUARDED_BY(m_most_recent_block_mutex); std::shared_ptr<const CBlockHeaderAndShortTxIDs> m_most_recent_compact_block GUARDED_BY(m_most_recent_block_mutex); uint256 m_most_recent_block_hash GUARDED_BY(m_most_recent_block_mutex); - bool m_most_recent_compact_block_has_witnesses GUARDED_BY(m_most_recent_block_mutex){false}; /** Height of the highest block announced using BIP 152 high-bandwidth mode. */ int m_highest_fast_announce{0}; @@ -880,10 +916,11 @@ static void PushAddress(Peer& peer, const CAddress& addr, FastRandomContext& ins static void AddKnownTx(Peer& peer, const uint256& hash) { - if (peer.m_tx_relay != nullptr) { - LOCK(peer.m_tx_relay->m_tx_inventory_mutex); - peer.m_tx_relay->m_tx_inventory_known_filter.insert(hash); - } + auto tx_relay = peer.GetTxRelay(); + if (!tx_relay) return; + + LOCK(tx_relay->m_tx_inventory_mutex); + tx_relay->m_tx_inventory_known_filter.insert(hash); } std::chrono::microseconds PeerManagerImpl::NextInvToInbounds(std::chrono::microseconds now, @@ -974,54 +1011,52 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) if (m_ignore_incoming_txs) return; CNodeState* nodestate = State(nodeid); - if (!nodestate || !nodestate->fSupportsDesiredCmpctVersion) { - // Never ask from peers who can't provide witnesses. + if (!nodestate || !nodestate->m_provides_cmpctblocks) { + // Don't request compact blocks if the peer has not signalled support return; } - if (nodestate->fProvidesHeaderAndIDs) { - int num_outbound_hb_peers = 0; - for (std::list<NodeId>::iterator it = lNodesAnnouncingHeaderAndIDs.begin(); it != lNodesAnnouncingHeaderAndIDs.end(); it++) { - if (*it == nodeid) { - lNodesAnnouncingHeaderAndIDs.erase(it); - lNodesAnnouncingHeaderAndIDs.push_back(nodeid); - return; - } - CNodeState *state = State(*it); - if (state != nullptr && !state->m_is_inbound) ++num_outbound_hb_peers; - } - if (nodestate->m_is_inbound) { - // If we're adding an inbound HB peer, make sure we're not removing - // our last outbound HB peer in the process. - if (lNodesAnnouncingHeaderAndIDs.size() >= 3 && num_outbound_hb_peers == 1) { - CNodeState *remove_node = State(lNodesAnnouncingHeaderAndIDs.front()); - if (remove_node != nullptr && !remove_node->m_is_inbound) { - // Put the HB outbound peer in the second slot, so that it - // doesn't get removed. - std::swap(lNodesAnnouncingHeaderAndIDs.front(), *std::next(lNodesAnnouncingHeaderAndIDs.begin())); - } - } + + int num_outbound_hb_peers = 0; + for (std::list<NodeId>::iterator it = lNodesAnnouncingHeaderAndIDs.begin(); it != lNodesAnnouncingHeaderAndIDs.end(); it++) { + if (*it == nodeid) { + lNodesAnnouncingHeaderAndIDs.erase(it); + lNodesAnnouncingHeaderAndIDs.push_back(nodeid); + return; } - m_connman.ForNode(nodeid, [this](CNode* pfrom) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { - AssertLockHeld(::cs_main); - uint64_t nCMPCTBLOCKVersion = 2; - if (lNodesAnnouncingHeaderAndIDs.size() >= 3) { - // As per BIP152, we only get 3 of our peers to announce - // blocks using compact encodings. - m_connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [this, nCMPCTBLOCKVersion](CNode* pnodeStop){ - m_connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/false, nCMPCTBLOCKVersion)); - // save BIP152 bandwidth state: we select peer to be low-bandwidth - pnodeStop->m_bip152_highbandwidth_to = false; - return true; - }); - lNodesAnnouncingHeaderAndIDs.pop_front(); + CNodeState *state = State(*it); + if (state != nullptr && !state->m_is_inbound) ++num_outbound_hb_peers; + } + if (nodestate->m_is_inbound) { + // If we're adding an inbound HB peer, make sure we're not removing + // our last outbound HB peer in the process. + if (lNodesAnnouncingHeaderAndIDs.size() >= 3 && num_outbound_hb_peers == 1) { + CNodeState *remove_node = State(lNodesAnnouncingHeaderAndIDs.front()); + if (remove_node != nullptr && !remove_node->m_is_inbound) { + // Put the HB outbound peer in the second slot, so that it + // doesn't get removed. + std::swap(lNodesAnnouncingHeaderAndIDs.front(), *std::next(lNodesAnnouncingHeaderAndIDs.begin())); } - m_connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/true, nCMPCTBLOCKVersion)); - // save BIP152 bandwidth state: we select peer to be high-bandwidth - pfrom->m_bip152_highbandwidth_to = true; - lNodesAnnouncingHeaderAndIDs.push_back(pfrom->GetId()); - return true; - }); + } } + m_connman.ForNode(nodeid, [this](CNode* pfrom) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + AssertLockHeld(::cs_main); + if (lNodesAnnouncingHeaderAndIDs.size() >= 3) { + // As per BIP152, we only get 3 of our peers to announce + // blocks using compact encodings. + m_connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [this](CNode* pnodeStop){ + m_connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); + // save BIP152 bandwidth state: we select peer to be low-bandwidth + pnodeStop->m_bip152_highbandwidth_to = false; + return true; + }); + lNodesAnnouncingHeaderAndIDs.pop_front(); + } + m_connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/true, /*version=*/CMPCTBLOCKS_VERSION)); + // save BIP152 bandwidth state: we select peer to be high-bandwidth + pfrom->m_bip152_highbandwidth_to = true; + lNodesAnnouncingHeaderAndIDs.push_back(pfrom->GetId()); + return true; + }); } bool PeerManagerImpl::TipMayBeStale() @@ -1110,7 +1145,6 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count if (state->pindexLastCommonBlock == state->pindexBestKnownBlock) return; - const Consensus::Params& consensusParams = m_chainparams.GetConsensus(); std::vector<const CBlockIndex*> vToFetch; const CBlockIndex *pindexWalk = state->pindexLastCommonBlock; // Never fetch further than the best block we know the peer has, or more than BLOCK_DOWNLOAD_WINDOW + 1 beyond the last @@ -1140,7 +1174,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count // We consider the chain that this peer is on invalid. return; } - if (!State(nodeid)->fHaveWitness && DeploymentActiveAt(*pindex, consensusParams, Consensus::DEPLOYMENT_SEGWIT)) { + if (!State(nodeid)->fHaveWitness && DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) { // We wouldn't download this block or its descendants from this peer. return; } @@ -1186,7 +1220,7 @@ void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer) CService addr_you = addr.IsRoutable() && !IsProxy(addr) && addr.IsAddrV1Compatible() ? addr : CService(); uint64_t your_services{addr.nServices}; - const bool tx_relay = !m_ignore_incoming_txs && peer.m_tx_relay != nullptr && !pnode.IsFeelerConn(); + const bool tx_relay = !m_ignore_incoming_txs && !pnode.IsBlockOnlyConn() && !pnode.IsFeelerConn(); m_connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, my_services, nTime, your_services, addr_you, // Together the pre-version-31402 serialization of CAddress "addrYou" (without nTime) my_services, CService(), // Together the pre-version-31402 serialization of CAddress "addrMe" (without nTime) @@ -1241,7 +1275,7 @@ void PeerManagerImpl::InitializeNode(CNode *pnode) m_node_states.emplace_hint(m_node_states.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(pnode->IsInboundConn())); assert(m_txrequest.Count(nodeid) == 0); } - PeerRef peer = std::make_shared<Peer>(nodeid, /*tx_relay=*/ !pnode->IsBlockOnlyConn()); + PeerRef peer = std::make_shared<Peer>(nodeid); { LOCK(m_peer_mutex); m_peer_map.emplace_hint(m_peer_map.end(), nodeid, peer); @@ -1377,9 +1411,9 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c ping_wait = GetTime<std::chrono::microseconds>() - peer->m_ping_start.load(); } - if (peer->m_tx_relay != nullptr) { - stats.m_relay_txs = WITH_LOCK(peer->m_tx_relay->m_bloom_filter_mutex, return peer->m_tx_relay->m_relay_txs); - stats.m_fee_filter_received = peer->m_tx_relay->m_fee_filter_received.load(); + if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { + stats.m_relay_txs = WITH_LOCK(tx_relay->m_bloom_filter_mutex, return tx_relay->m_relay_txs); + stats.m_fee_filter_received = tx_relay->m_fee_filter_received.load(); } else { stats.m_relay_txs = false; stats.m_fee_filter_received = 0; @@ -1632,7 +1666,7 @@ void PeerManagerImpl::BlockDisconnected(const std::shared_ptr<const CBlock> &blo */ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& pblock) { - std::shared_ptr<const CBlockHeaderAndShortTxIDs> pcmpctblock = std::make_shared<const CBlockHeaderAndShortTxIDs> (*pblock, true); + auto pcmpctblock = std::make_shared<const CBlockHeaderAndShortTxIDs>(*pblock); const CNetMsgMaker msgMaker(PROTOCOL_VERSION); LOCK(cs_main); @@ -1641,7 +1675,8 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha return; m_highest_fast_announce = pindex->nHeight; - bool fWitnessEnabled = DeploymentActiveAt(*pindex, m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT); + if (!DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) return; + uint256 hashBlock(pblock->GetHash()); const std::shared_future<CSerializedNetMsg> lazy_ser{ std::async(std::launch::deferred, [&] { return msgMaker.Make(NetMsgType::CMPCTBLOCK, *pcmpctblock); })}; @@ -1651,10 +1686,9 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha m_most_recent_block_hash = hashBlock; m_most_recent_block = pblock; m_most_recent_compact_block = pcmpctblock; - m_most_recent_compact_block_has_witnesses = fWitnessEnabled; } - m_connman.ForEachNode([this, pindex, fWitnessEnabled, &lazy_ser, &hashBlock](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + m_connman.ForEachNode([this, pindex, &lazy_ser, &hashBlock](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); if (pnode->GetCommonVersion() < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect) @@ -1663,8 +1697,7 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha CNodeState &state = *State(pnode->GetId()); // If the peer has, or we announced to them the previous block already, // but we don't think they have this one, go ahead and announce it - if (state.fPreferHeaderAndIDs && (!fWitnessEnabled || state.fWantsCmpctWitness) && - !PeerHasHeader(&state, pindex) && PeerHasHeader(&state, pindex->pprev)) { + if (state.m_requested_hb_cmpctblocks && !PeerHasHeader(&state, pindex) && PeerHasHeader(&state, pindex->pprev)) { LogPrint(BCLog::NET, "%s sending header-and-ids %s to peer=%d\n", "PeerManager::NewPoWValidBlock", hashBlock.ToString(), pnode->GetId()); @@ -1795,12 +1828,13 @@ void PeerManagerImpl::RelayTransaction(const uint256& txid, const uint256& wtxid LOCK(m_peer_mutex); for(auto& it : m_peer_map) { Peer& peer = *it.second; - if (!peer.m_tx_relay) continue; + auto tx_relay = peer.GetTxRelay(); + if (!tx_relay) continue; const uint256& hash{peer.m_wtxid_relay ? wtxid : txid}; - LOCK(peer.m_tx_relay->m_tx_inventory_mutex); - if (!peer.m_tx_relay->m_tx_inventory_known_filter.contains(hash)) { - peer.m_tx_relay->m_tx_inventory_to_send.insert(hash); + LOCK(tx_relay->m_tx_inventory_mutex); + if (!tx_relay->m_tx_inventory_known_filter.contains(hash)) { + tx_relay->m_tx_inventory_to_send.insert(hash); } }; } @@ -1859,12 +1893,10 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& { std::shared_ptr<const CBlock> a_recent_block; std::shared_ptr<const CBlockHeaderAndShortTxIDs> a_recent_compact_block; - bool fWitnessesPresentInARecentCompactBlock; { LOCK(m_most_recent_block_mutex); a_recent_block = m_most_recent_block; a_recent_compact_block = m_most_recent_compact_block; - fWitnessesPresentInARecentCompactBlock = m_most_recent_compact_block_has_witnesses; } bool need_activate_chain = false; @@ -1951,11 +1983,11 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } else if (inv.IsMsgFilteredBlk()) { bool sendMerkleBlock = false; CMerkleBlock merkleBlock; - if (peer.m_tx_relay != nullptr) { - LOCK(peer.m_tx_relay->m_bloom_filter_mutex); - if (peer.m_tx_relay->m_bloom_filter) { + if (auto tx_relay = peer.GetTxRelay(); tx_relay != nullptr) { + LOCK(tx_relay->m_bloom_filter_mutex); + if (tx_relay->m_bloom_filter) { sendMerkleBlock = true; - merkleBlock = CMerkleBlock(*pblock, *peer.m_tx_relay->m_bloom_filter); + merkleBlock = CMerkleBlock(*pblock, *tx_relay->m_bloom_filter); } } if (sendMerkleBlock) { @@ -1977,17 +2009,15 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // they won't have a useful mempool to match against a compact block, // and we don't feel like constructing the object for them, so // instead we respond with the full, non-compact block. - bool fPeerWantsWitness = State(pfrom.GetId())->fWantsCmpctWitness; - int nSendFlags = fPeerWantsWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; if (CanDirectFetch() && pindex->nHeight >= m_chainman.ActiveChain().Height() - MAX_CMPCTBLOCK_DEPTH) { - if ((fPeerWantsWitness || !fWitnessesPresentInARecentCompactBlock) && a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { - m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); + if (a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); } else { - CBlockHeaderAndShortTxIDs cmpctblock(*pblock, fPeerWantsWitness); - m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); + CBlockHeaderAndShortTxIDs cmpctblock{*pblock}; + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); } } else { - m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCK, *pblock)); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); } } } @@ -2038,13 +2068,15 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic { AssertLockNotHeld(cs_main); + auto tx_relay = peer.GetTxRelay(); + std::deque<CInv>::iterator it = peer.m_getdata_requests.begin(); std::vector<CInv> vNotFound; const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); const auto now{GetTime<std::chrono::seconds>()}; // Get last mempool request time - const auto mempool_req = peer.m_tx_relay != nullptr ? peer.m_tx_relay->m_last_mempool_req.load() : std::chrono::seconds::min(); + const auto mempool_req = tx_relay != nullptr ? tx_relay->m_last_mempool_req.load() : std::chrono::seconds::min(); // Process as many TX items from the front of the getdata queue as // possible, since they're common and it's efficient to batch process @@ -2057,8 +2089,9 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic const CInv &inv = *it++; - if (peer.m_tx_relay == nullptr) { - // Ignore GETDATA requests for transactions from blocks-only peers. + if (tx_relay == nullptr) { + // Ignore GETDATA requests for transactions from block-relay-only + // peers and peers that asked us not to announce transactions. continue; } @@ -2085,7 +2118,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic } for (const uint256& parent_txid : parent_ids_to_add) { // Relaying a transaction with a recent but unconfirmed parent. - if (WITH_LOCK(peer.m_tx_relay->m_tx_inventory_mutex, return !peer.m_tx_relay->m_tx_inventory_known_filter.contains(parent_txid))) { + if (WITH_LOCK(tx_relay->m_tx_inventory_mutex, return !tx_relay->m_tx_inventory_known_filter.contains(parent_txid))) { LOCK(cs_main); State(pfrom.GetId())->m_recently_announced_invs.insert(parent_txid); } @@ -2146,10 +2179,9 @@ void PeerManagerImpl::SendBlockTransactions(CNode& pfrom, const CBlock& block, c } resp.txn[i] = block.vtx[req.indexes[i]]; } - LOCK(cs_main); + const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); - int nSendFlags = State(pfrom.GetId())->fWantsCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; - m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCKTXN, resp)); } void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, @@ -2214,7 +2246,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, } BlockValidationState state; - if (!m_chainman.ProcessNewBlockHeaders(headers, state, m_chainparams, &pindexLast)) { + if (!m_chainman.ProcessNewBlockHeaders(headers, state, &pindexLast)) { if (state.IsInvalid()) { MaybePunishNodeForBlock(pfrom.GetId(), state, via_compact_block, "invalid header received"); return; @@ -2258,7 +2290,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, while (pindexWalk && !m_chainman.ActiveChain().Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && !IsBlockRequested(pindexWalk->GetBlockHash()) && - (!DeploymentActiveAt(*pindexWalk, m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT) || State(pfrom.GetId())->fHaveWitness)) { + (!DeploymentActiveAt(*pindexWalk, m_chainman, Consensus::DEPLOYMENT_SEGWIT) || State(pfrom.GetId())->fHaveWitness)) { // We don't have this block, and it's not yet in flight. vToFetch.push_back(pindexWalk); } @@ -2292,7 +2324,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, } if (vGetData.size() > 0) { if (!m_ignore_incoming_txs && - nodestate->fSupportsDesiredCmpctVersion && + nodestate->m_provides_cmpctblocks && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { @@ -2591,7 +2623,7 @@ void PeerManagerImpl::ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv) void PeerManagerImpl::ProcessBlock(CNode& node, const std::shared_ptr<const CBlock>& block, bool force_processing) { bool new_block{false}; - m_chainman.ProcessNewBlock(m_chainparams, block, force_processing, &new_block); + m_chainman.ProcessNewBlock(block, force_processing, &new_block); if (new_block) { node.m_last_block_time = GetTime<std::chrono::seconds>(); } else { @@ -2721,10 +2753,16 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // set nodes not capable of serving the complete blockchain history as "limited nodes" pfrom.m_limited_node = (!(nServices & NODE_NETWORK) && (nServices & NODE_NETWORK_LIMITED)); - if (peer->m_tx_relay != nullptr) { + // We only initialize the m_tx_relay data structure if: + // - this isn't an outbound block-relay-only connection; and + // - fRelay=true or we're offering NODE_BLOOM to this peer + // (NODE_BLOOM means that the peer may turn on tx relay later) + if (!pfrom.IsBlockOnlyConn() && + (fRelay || (pfrom.GetLocalServices() & NODE_BLOOM))) { + auto* const tx_relay = peer->SetTxRelay(); { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - peer->m_tx_relay->m_relay_txs = fRelay; // set to true after we get the first filter* message + LOCK(tx_relay->m_bloom_filter_mutex); + tx_relay->m_relay_txs = fRelay; // set to true after we get the first filter* message } if (fRelay) pfrom.m_relays_txs = true; } @@ -2862,16 +2900,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDHEADERS)); } if (pfrom.GetCommonVersion() >= SHORT_IDS_BLOCKS_VERSION) { - // Tell our peer we are willing to provide version 1 or 2 cmpctblocks + // Tell our peer we are willing to provide version 2 cmpctblocks. // However, we do not request new block announcements using // cmpctblock messages. // We send this to non-NODE NETWORK peers as well, because // they may wish to request compact blocks from us - bool fAnnounceUsingCMPCTBLOCK = false; - uint64_t nCMPCTBLOCKVersion = 2; - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); - nCMPCTBLOCKVersion = 1; - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); } pfrom.fSuccessfullyConnected = true; return; @@ -2884,26 +2918,20 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::SENDCMPCT) { - bool fAnnounceUsingCMPCTBLOCK = false; - uint64_t nCMPCTBLOCKVersion = 0; - vRecv >> fAnnounceUsingCMPCTBLOCK >> nCMPCTBLOCKVersion; - if (nCMPCTBLOCKVersion == 1 || nCMPCTBLOCKVersion == 2) { - LOCK(cs_main); - // fProvidesHeaderAndIDs is used to "lock in" version of compact blocks we send (fWantsCmpctWitness) - if (!State(pfrom.GetId())->fProvidesHeaderAndIDs) { - State(pfrom.GetId())->fProvidesHeaderAndIDs = true; - State(pfrom.GetId())->fWantsCmpctWitness = nCMPCTBLOCKVersion == 2; - } - if (State(pfrom.GetId())->fWantsCmpctWitness == (nCMPCTBLOCKVersion == 2)) { // ignore later version announces - State(pfrom.GetId())->fPreferHeaderAndIDs = fAnnounceUsingCMPCTBLOCK; - // save whether peer selects us as BIP152 high-bandwidth peer - // (receiving sendcmpct(1) signals high-bandwidth, sendcmpct(0) low-bandwidth) - pfrom.m_bip152_highbandwidth_from = fAnnounceUsingCMPCTBLOCK; - } - if (!State(pfrom.GetId())->fSupportsDesiredCmpctVersion) { - State(pfrom.GetId())->fSupportsDesiredCmpctVersion = (nCMPCTBLOCKVersion == 2); - } - } + bool sendcmpct_hb{false}; + uint64_t sendcmpct_version{0}; + vRecv >> sendcmpct_hb >> sendcmpct_version; + + // Only support compact block relay with witnesses + if (sendcmpct_version != CMPCTBLOCKS_VERSION) return; + + LOCK(cs_main); + CNodeState* nodestate = State(pfrom.GetId()); + nodestate->m_provides_cmpctblocks = true; + nodestate->m_requested_hb_cmpctblocks = sendcmpct_hb; + // save whether peer selects us as BIP152 high-bandwidth peer + // (receiving sendcmpct(1) signals high-bandwidth, sendcmpct(0) low-bandwidth) + pfrom.m_bip152_highbandwidth_from = sendcmpct_hb; return; } @@ -3054,7 +3082,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Reject tx INVs when the -blocksonly setting is enabled, or this is a // block-relay-only peer - bool reject_tx_invs{m_ignore_incoming_txs || (peer->m_tx_relay == nullptr)}; + bool reject_tx_invs{m_ignore_incoming_txs || pfrom.IsBlockOnlyConn()}; // Allow peers with relay permission to send data other than blocks in blocks only mode if (pfrom.HasPermission(NetPermissionFlags::Relay)) { @@ -3252,9 +3280,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // expensive disk reads, because it will require the peer to // actually receive all the data read from disk over the network. LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block > %i deep\n", pfrom.GetId(), MAX_BLOCKTXN_DEPTH); - CInv inv; - WITH_LOCK(cs_main, inv.type = State(pfrom.GetId())->fWantsCmpctWitness ? MSG_WITNESS_BLOCK : MSG_BLOCK); - inv.hash = req.blockhash; + CInv inv{MSG_WITNESS_BLOCK, req.blockhash}; WITH_LOCK(peer->m_getdata_requests_mutex, peer->m_getdata_requests.push_back(inv)); // The message processing loop will go around again (without pausing) and we'll respond then return; @@ -3329,9 +3355,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (msg_type == NetMsgType::TX) { // Stop processing the transaction early if - // 1) We are in blocks only mode and peer has no relay permission + // 1) We are in blocks only mode and peer has no relay permission; OR // 2) This peer is a block-relay-only peer - if ((m_ignore_incoming_txs && !pfrom.HasPermission(NetPermissionFlags::Relay)) || (peer->m_tx_relay == nullptr)) { + if ((m_ignore_incoming_txs && !pfrom.HasPermission(NetPermissionFlags::Relay)) || pfrom.IsBlockOnlyConn()) { LogPrint(BCLog::NET, "transaction sent in violation of protocol peer=%d\n", pfrom.GetId()); pfrom.fDisconnect = true; return; @@ -3569,7 +3595,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const CBlockIndex *pindex = nullptr; BlockValidationState state; - if (!m_chainman.ProcessNewBlockHeaders({cmpctblock.header}, state, m_chainparams, &pindex)) { + if (!m_chainman.ProcessNewBlockHeaders({cmpctblock.header}, state, &pindex)) { if (state.IsInvalid()) { MaybePunishNodeForBlock(pfrom.GetId(), state, /*via_compact_block=*/true, "invalid header via cmpctblock"); return; @@ -3629,12 +3655,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } - if (DeploymentActiveAt(*pindex, m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT) && !nodestate->fSupportsDesiredCmpctVersion) { - // Don't bother trying to process compact blocks from v1 peers - // after segwit activates. - return; - } - // We want to be a bit conservative just to be extra careful about DoS // possibilities in compact block processing... if (pindex->nHeight <= m_chainman.ActiveChain().Height() + 2) { @@ -3943,9 +3963,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } - if (peer->m_tx_relay != nullptr) { - LOCK(peer->m_tx_relay->m_tx_inventory_mutex); - peer->m_tx_relay->m_send_mempool = true; + if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { + LOCK(tx_relay->m_tx_inventory_mutex); + tx_relay->m_send_mempool = true; } return; } @@ -4038,16 +4058,13 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, { // There is no excuse for sending a too-large filter Misbehaving(pfrom.GetId(), 100, "too-large bloom filter"); - } - else if (peer->m_tx_relay != nullptr) - { + } else if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - peer->m_tx_relay->m_bloom_filter.reset(new CBloomFilter(filter)); - peer->m_tx_relay->m_relay_txs = true; + LOCK(tx_relay->m_bloom_filter_mutex); + tx_relay->m_bloom_filter.reset(new CBloomFilter(filter)); + tx_relay->m_relay_txs = true; } pfrom.m_bloom_filter_loaded = true; - pfrom.m_relays_txs = true; } return; } @@ -4066,10 +4083,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, bool bad = false; if (vData.size() > MAX_SCRIPT_ELEMENT_SIZE) { bad = true; - } else if (peer->m_tx_relay != nullptr) { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - if (peer->m_tx_relay->m_bloom_filter) { - peer->m_tx_relay->m_bloom_filter->insert(vData); + } else if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { + LOCK(tx_relay->m_bloom_filter_mutex); + if (tx_relay->m_bloom_filter) { + tx_relay->m_bloom_filter->insert(vData); } else { bad = true; } @@ -4086,14 +4103,13 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, pfrom.fDisconnect = true; return; } - if (peer->m_tx_relay == nullptr) { - return; - } + auto tx_relay = peer->GetTxRelay(); + if (!tx_relay) return; { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - peer->m_tx_relay->m_bloom_filter = nullptr; - peer->m_tx_relay->m_relay_txs = true; + LOCK(tx_relay->m_bloom_filter_mutex); + tx_relay->m_bloom_filter = nullptr; + tx_relay->m_relay_txs = true; } pfrom.m_bloom_filter_loaded = false; pfrom.m_relays_txs = true; @@ -4104,8 +4120,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, CAmount newFeeFilter = 0; vRecv >> newFeeFilter; if (MoneyRange(newFeeFilter)) { - if (peer->m_tx_relay != nullptr) { - peer->m_tx_relay->m_fee_filter_received = newFeeFilter; + if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { + tx_relay->m_fee_filter_received = newFeeFilter; } LogPrint(BCLog::NET, "received: feefilter of %s from peer=%d\n", CFeeRate(newFeeFilter).ToString(), pfrom.GetId()); } @@ -4479,10 +4495,10 @@ void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::mic } if (pingSend) { - uint64_t nonce = 0; - while (nonce == 0) { - GetRandBytes({(unsigned char*)&nonce, sizeof(nonce)}); - } + uint64_t nonce; + do { + nonce = GetRand<uint64_t>(); + } while (nonce == 0); peer.m_ping_queued = false; peer.m_ping_start = now; if (node_to.GetCommonVersion() > BIP0031_VERSION) { @@ -4566,10 +4582,12 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, Peer& peer, std::chrono::microseconds current_time) { if (m_ignore_incoming_txs) return; - if (!peer.m_tx_relay) return; if (pto.GetCommonVersion() < FEEFILTER_VERSION) return; // peers with the forcerelay permission should not filter txs to us if (pto.HasPermission(NetPermissionFlags::ForceRelay)) return; + // Don't send feefilter messages to outbound block-relay-only peers since they should never announce + // transactions to us, regardless of feefilter state. + if (pto.IsBlockOnlyConn()) return; CAmount currentFilter = m_mempool.GetMinFee(gArgs.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); static FeeFilterRounder g_filter_rounder{CFeeRate{DEFAULT_MIN_RELAY_TX_FEE}}; @@ -4580,27 +4598,27 @@ void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, Peer& peer, std::chrono::mi currentFilter = MAX_MONEY; } else { static const CAmount MAX_FILTER{g_filter_rounder.round(MAX_MONEY)}; - if (peer.m_tx_relay->m_fee_filter_sent == MAX_FILTER) { + if (peer.m_fee_filter_sent == MAX_FILTER) { // Send the current filter if we sent MAX_FILTER previously // and made it out of IBD. - peer.m_tx_relay->m_next_send_feefilter = 0us; + peer.m_next_send_feefilter = 0us; } } - if (current_time > peer.m_tx_relay->m_next_send_feefilter) { + if (current_time > peer.m_next_send_feefilter) { CAmount filterToSend = g_filter_rounder.round(currentFilter); // We always have a fee filter of at least minRelayTxFee filterToSend = std::max(filterToSend, ::minRelayTxFee.GetFeePerK()); - if (filterToSend != peer.m_tx_relay->m_fee_filter_sent) { + if (filterToSend != peer.m_fee_filter_sent) { m_connman.PushMessage(&pto, CNetMsgMaker(pto.GetCommonVersion()).Make(NetMsgType::FEEFILTER, filterToSend)); - peer.m_tx_relay->m_fee_filter_sent = filterToSend; + peer.m_fee_filter_sent = filterToSend; } - peer.m_tx_relay->m_next_send_feefilter = GetExponentialRand(current_time, AVG_FEEFILTER_BROADCAST_INTERVAL); + peer.m_next_send_feefilter = GetExponentialRand(current_time, AVG_FEEFILTER_BROADCAST_INTERVAL); } // If the fee filter has changed substantially and it's still more than MAX_FEEFILTER_CHANGE_DELAY // until scheduled broadcast, then move the broadcast to within MAX_FEEFILTER_CHANGE_DELAY. - else if (current_time + MAX_FEEFILTER_CHANGE_DELAY < peer.m_tx_relay->m_next_send_feefilter && - (currentFilter < 3 * peer.m_tx_relay->m_fee_filter_sent / 4 || currentFilter > 4 * peer.m_tx_relay->m_fee_filter_sent / 3)) { - peer.m_tx_relay->m_next_send_feefilter = current_time + GetRandomDuration<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY); + else if (current_time + MAX_FEEFILTER_CHANGE_DELAY < peer.m_next_send_feefilter && + (currentFilter < 3 * peer.m_fee_filter_sent / 4 || currentFilter > 4 * peer.m_fee_filter_sent / 3)) { + peer.m_next_send_feefilter = current_time + GetRandomDuration<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY); } } @@ -4724,7 +4742,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) LOCK(peer->m_block_inv_mutex); std::vector<CBlock> vHeaders; bool fRevertToInv = ((!state.fPreferHeaders && - (!state.fPreferHeaderAndIDs || peer->m_blocks_for_headers_relay.size() > 1)) || + (!state.m_requested_hb_cmpctblocks || peer->m_blocks_for_headers_relay.size() > 1)) || peer->m_blocks_for_headers_relay.size() > MAX_BLOCKS_TO_ANNOUNCE); const CBlockIndex *pBestIndex = nullptr; // last header queued for delivery ProcessBlockAvailability(pto->GetId()); // ensure pindexBestKnownBlock is up-to-date @@ -4777,33 +4795,27 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } } if (!fRevertToInv && !vHeaders.empty()) { - if (vHeaders.size() == 1 && state.fPreferHeaderAndIDs) { + if (vHeaders.size() == 1 && state.m_requested_hb_cmpctblocks) { // We only send up to 1 block as header-and-ids, as otherwise // probably means we're doing an initial-ish-sync or they're slow LogPrint(BCLog::NET, "%s sending header-and-ids %s to peer=%d\n", __func__, vHeaders.front().GetHash().ToString(), pto->GetId()); - int nSendFlags = state.fWantsCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; - - bool fGotBlockFromCache = false; + std::optional<CSerializedNetMsg> cached_cmpctblock_msg; { LOCK(m_most_recent_block_mutex); if (m_most_recent_block_hash == pBestIndex->GetBlockHash()) { - if (state.fWantsCmpctWitness || !m_most_recent_compact_block_has_witnesses) - m_connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *m_most_recent_compact_block)); - else { - CBlockHeaderAndShortTxIDs cmpctblock(*m_most_recent_block, state.fWantsCmpctWitness); - m_connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); - } - fGotBlockFromCache = true; + cached_cmpctblock_msg = msgMaker.Make(NetMsgType::CMPCTBLOCK, *m_most_recent_compact_block); } } - if (!fGotBlockFromCache) { + if (cached_cmpctblock_msg.has_value()) { + m_connman.PushMessage(pto, std::move(cached_cmpctblock_msg.value())); + } else { CBlock block; bool ret = ReadBlockFromDisk(block, pBestIndex, consensusParams); assert(ret); - CBlockHeaderAndShortTxIDs cmpctblock(block, state.fWantsCmpctWitness); - m_connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); + CBlockHeaderAndShortTxIDs cmpctblock{block}; + m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); } state.pindexBestHeaderSent = pBestIndex; } else if (state.fPreferHeaders) { @@ -4868,45 +4880,45 @@ bool PeerManagerImpl::SendMessages(CNode* pto) peer->m_blocks_for_inv_relay.clear(); } - if (peer->m_tx_relay != nullptr) { - LOCK(peer->m_tx_relay->m_tx_inventory_mutex); + if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { + LOCK(tx_relay->m_tx_inventory_mutex); // Check whether periodic sends should happen bool fSendTrickle = pto->HasPermission(NetPermissionFlags::NoBan); - if (peer->m_tx_relay->m_next_inv_send_time < current_time) { + if (tx_relay->m_next_inv_send_time < current_time) { fSendTrickle = true; if (pto->IsInboundConn()) { - peer->m_tx_relay->m_next_inv_send_time = NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL); + tx_relay->m_next_inv_send_time = NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL); } else { - peer->m_tx_relay->m_next_inv_send_time = GetExponentialRand(current_time, OUTBOUND_INVENTORY_BROADCAST_INTERVAL); + tx_relay->m_next_inv_send_time = GetExponentialRand(current_time, OUTBOUND_INVENTORY_BROADCAST_INTERVAL); } } // Time to send but the peer has requested we not relay transactions. if (fSendTrickle) { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - if (!peer->m_tx_relay->m_relay_txs) peer->m_tx_relay->m_tx_inventory_to_send.clear(); + LOCK(tx_relay->m_bloom_filter_mutex); + if (!tx_relay->m_relay_txs) tx_relay->m_tx_inventory_to_send.clear(); } // Respond to BIP35 mempool requests - if (fSendTrickle && peer->m_tx_relay->m_send_mempool) { + if (fSendTrickle && tx_relay->m_send_mempool) { auto vtxinfo = m_mempool.infoAll(); - peer->m_tx_relay->m_send_mempool = false; - const CFeeRate filterrate{peer->m_tx_relay->m_fee_filter_received.load()}; + tx_relay->m_send_mempool = false; + const CFeeRate filterrate{tx_relay->m_fee_filter_received.load()}; - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); + LOCK(tx_relay->m_bloom_filter_mutex); for (const auto& txinfo : vtxinfo) { const uint256& hash = peer->m_wtxid_relay ? txinfo.tx->GetWitnessHash() : txinfo.tx->GetHash(); CInv inv(peer->m_wtxid_relay ? MSG_WTX : MSG_TX, hash); - peer->m_tx_relay->m_tx_inventory_to_send.erase(hash); + tx_relay->m_tx_inventory_to_send.erase(hash); // Don't send transactions that peers will not put into their mempool if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { continue; } - if (peer->m_tx_relay->m_bloom_filter) { - if (!peer->m_tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; + if (tx_relay->m_bloom_filter) { + if (!tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; } - peer->m_tx_relay->m_tx_inventory_known_filter.insert(hash); + tx_relay->m_tx_inventory_known_filter.insert(hash); // Responses to MEMPOOL requests bypass the m_recently_announced_invs filter. vInv.push_back(inv); if (vInv.size() == MAX_INV_SZ) { @@ -4914,18 +4926,18 @@ bool PeerManagerImpl::SendMessages(CNode* pto) vInv.clear(); } } - peer->m_tx_relay->m_last_mempool_req = std::chrono::duration_cast<std::chrono::seconds>(current_time); + tx_relay->m_last_mempool_req = std::chrono::duration_cast<std::chrono::seconds>(current_time); } // Determine transactions to relay if (fSendTrickle) { // Produce a vector with all candidates for sending std::vector<std::set<uint256>::iterator> vInvTx; - vInvTx.reserve(peer->m_tx_relay->m_tx_inventory_to_send.size()); - for (std::set<uint256>::iterator it = peer->m_tx_relay->m_tx_inventory_to_send.begin(); it != peer->m_tx_relay->m_tx_inventory_to_send.end(); it++) { + vInvTx.reserve(tx_relay->m_tx_inventory_to_send.size()); + for (std::set<uint256>::iterator it = tx_relay->m_tx_inventory_to_send.begin(); it != tx_relay->m_tx_inventory_to_send.end(); it++) { vInvTx.push_back(it); } - const CFeeRate filterrate{peer->m_tx_relay->m_fee_filter_received.load()}; + const CFeeRate filterrate{tx_relay->m_fee_filter_received.load()}; // Topologically and fee-rate sort the inventory we send for privacy and priority reasons. // A heap is used so that not all items need sorting if only a few are being sent. CompareInvMempoolOrder compareInvMempoolOrder(&m_mempool, peer->m_wtxid_relay); @@ -4933,7 +4945,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // No reason to drain out at many times the network's capacity, // especially since we have many peers and some will draw much shorter delays. unsigned int nRelayedTransactions = 0; - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); + LOCK(tx_relay->m_bloom_filter_mutex); while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) { // Fetch the top element from the heap std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); @@ -4942,9 +4954,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto) uint256 hash = *it; CInv inv(peer->m_wtxid_relay ? MSG_WTX : MSG_TX, hash); // Remove it from the to-be-sent set - peer->m_tx_relay->m_tx_inventory_to_send.erase(it); + tx_relay->m_tx_inventory_to_send.erase(it); // Check if not in the filter already - if (peer->m_tx_relay->m_tx_inventory_known_filter.contains(hash)) { + if (tx_relay->m_tx_inventory_known_filter.contains(hash)) { continue; } // Not in the mempool anymore? don't bother sending it. @@ -4958,7 +4970,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { continue; } - if (peer->m_tx_relay->m_bloom_filter && !peer->m_tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; + if (tx_relay->m_bloom_filter && !tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; // Send State(pto->GetId())->m_recently_announced_invs.insert(hash); vInv.push_back(inv); @@ -4985,14 +4997,14 @@ bool PeerManagerImpl::SendMessages(CNode* pto) m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); } - peer->m_tx_relay->m_tx_inventory_known_filter.insert(hash); + tx_relay->m_tx_inventory_known_filter.insert(hash); if (hash != txid) { // Insert txid into m_tx_inventory_known_filter, even for // wtxidrelay peers. This prevents re-adding of // unconfirmed parents to the recently_announced // filter, when a child tx is requested. See // ProcessGetData(). - peer->m_tx_relay->m_tx_inventory_known_filter.insert(txid); + tx_relay->m_tx_inventory_known_filter.insert(txid); } } } diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 9717f7abce..ca148bfa51 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -98,7 +98,7 @@ bool CNetAddr::SetNetFromBIP155Network(uint8_t possible_bip155_net, size_t addre * * @note This address is considered invalid by CNetAddr::IsValid() */ -CNetAddr::CNetAddr() {} +CNetAddr::CNetAddr() = default; void CNetAddr::SetIP(const CNetAddr& ipIn) { diff --git a/src/netaddress.h b/src/netaddress.h index b9a8dc589a..77e6171054 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -556,8 +556,8 @@ class CServiceHash { public: CServiceHash() - : m_salt_k0{GetRand(std::numeric_limits<uint64_t>::max())}, - m_salt_k1{GetRand(std::numeric_limits<uint64_t>::max())} + : m_salt_k0{GetRand<uint64_t>()}, + m_salt_k1{GetRand<uint64_t>()} { } diff --git a/src/node/context.cpp b/src/node/context.cpp index 0b31c10f44..4787efa1de 100644 --- a/src/node/context.cpp +++ b/src/node/context.cpp @@ -16,6 +16,6 @@ #include <validation.h> namespace node { -NodeContext::NodeContext() {} -NodeContext::~NodeContext() {} +NodeContext::NodeContext() = default; +NodeContext::~NodeContext() = default; } // namespace node diff --git a/src/node/miner.cpp b/src/node/miner.cpp index be5d58527b..770ccdbe1a 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -51,7 +51,7 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman) block.vtx.at(0) = MakeTransactionRef(tx); const CBlockIndex* prev_block = WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock)); - GenerateCoinbaseCommitment(block, prev_block, Params().GetConsensus()); + chainman.GenerateCoinbaseCommitment(block, prev_block); block.hashMerkleRoot = BlockMerkleRoot(block); } @@ -126,7 +126,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc assert(pindexPrev != nullptr); nHeight = pindexPrev->nHeight + 1; - pblock->nVersion = g_versionbitscache.ComputeBlockVersion(pindexPrev, chainparams.GetConsensus()); + pblock->nVersion = m_chainstate.m_chainman.m_versionbitscache.ComputeBlockVersion(pindexPrev, chainparams.GetConsensus()); // -regtest only: allow overriding block.nVersion with // -blockversion=N to test forking scenarios if (chainparams.MineBlocksOnDemand()) { @@ -154,7 +154,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc coinbaseTx.vout[0].nValue = nFees + GetBlockSubsidy(nHeight, chainparams.GetConsensus()); coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0; pblock->vtx[0] = MakeTransactionRef(std::move(coinbaseTx)); - pblocktemplate->vchCoinbaseCommitment = GenerateCoinbaseCommitment(*pblock, pindexPrev, chainparams.GetConsensus()); + pblocktemplate->vchCoinbaseCommitment = m_chainstate.m_chainman.GenerateCoinbaseCommitment(*pblock, pindexPrev); pblocktemplate->vTxFees[0] = -nFees; LogPrintf("CreateNewBlock(): block weight: %u txs: %u fees: %ld sigops %d\n", GetBlockWeight(*pblock), nBlockTx, nFees, nBlockSigOpsCost); diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 6499dbd97f..d2deaf69d0 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -537,9 +537,7 @@ CBlockPolicyEstimator::CBlockPolicyEstimator() } } -CBlockPolicyEstimator::~CBlockPolicyEstimator() -{ -} +CBlockPolicyEstimator::~CBlockPolicyEstimator() = default; void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, bool validFeeEstimate) { diff --git a/src/prevector.h b/src/prevector.h index 830b31e315..a52510930a 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -35,6 +35,8 @@ */ template<unsigned int N, typename T, typename Size = uint32_t, typename Diff = int32_t> class prevector { + static_assert(std::is_trivially_copyable_v<T>); + public: typedef Size size_type; typedef Diff difference_type; @@ -411,15 +413,7 @@ public: // representation (with capacity N and size <= N). iterator p = first; char* endp = (char*)&(*end()); - if (!std::is_trivially_destructible<T>::value) { - while (p != last) { - (*p).~T(); - _size--; - ++p; - } - } else { - _size -= last - p; - } + _size -= last - p; memmove(&(*first), &(*last), endp - ((char*)(&(*last)))); return first; } @@ -465,9 +459,6 @@ public: } ~prevector() { - if (!std::is_trivially_destructible<T>::value) { - clear(); - } if (!is_direct()) { free(_union.indirect_contents.indirect); _union.indirect_contents.indirect = nullptr; diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index dcab631d98..27ee9509e6 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -30,7 +30,7 @@ struct AddressTableEntry QString label; QString address; - AddressTableEntry() {} + AddressTableEntry() = default; AddressTableEntry(Type _type, const QString &_label, const QString &_address): type(_type), label(_label), address(_address) {} }; diff --git a/src/qt/bantablemodel.cpp b/src/qt/bantablemodel.cpp index e004fba308..3d0be69302 100644 --- a/src/qt/bantablemodel.cpp +++ b/src/qt/bantablemodel.cpp @@ -89,10 +89,7 @@ BanTableModel::BanTableModel(interfaces::Node& node, QObject* parent) : refresh(); } -BanTableModel::~BanTableModel() -{ - // Intentionally left empty -} +BanTableModel::~BanTableModel() = default; int BanTableModel::rowCount(const QModelIndex &parent) const { diff --git a/src/qt/clientmodel.h b/src/qt/clientmodel.h index 846691c0c0..1c8116738d 100644 --- a/src/qt/clientmodel.h +++ b/src/qt/clientmodel.h @@ -61,7 +61,7 @@ public: //! Return number of connections, default is in- and outbound (total) int getNumConnections(unsigned int flags = CONNECTIONS_ALL) const; int getNumBlocks() const; - uint256 getBestBlockHash(); + uint256 getBestBlockHash() EXCLUSIVE_LOCKS_REQUIRED(!m_cached_tip_mutex); int getHeaderTipHeight() const; int64_t getHeaderTipTime() const; diff --git a/src/qt/notificator.cpp b/src/qt/notificator.cpp index 51151b0be8..483db2892b 100644 --- a/src/qt/notificator.cpp +++ b/src/qt/notificator.cpp @@ -71,7 +71,7 @@ Notificator::~Notificator() class FreedesktopImage { public: - FreedesktopImage() {} + FreedesktopImage() = default; explicit FreedesktopImage(const QImage &img); // Image to variant that can be marshalled over DBus diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 820bcbf3cd..a8133f481e 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -35,8 +35,7 @@ class TxViewDelegate : public QAbstractItemDelegate Q_OBJECT public: explicit TxViewDelegate(const PlatformStyle* _platformStyle, QObject* parent = nullptr) - : QAbstractItemDelegate(parent), unit(BitcoinUnit::BTC), - platformStyle(_platformStyle) + : QAbstractItemDelegate(parent), platformStyle(_platformStyle) { connect(this, &TxViewDelegate::width_changed, this, &TxViewDelegate::sizeHintChanged); } @@ -125,7 +124,7 @@ public: return {DECORATION_SIZE + 8 + minimum_text_width, DECORATION_SIZE}; } - BitcoinUnit unit; + BitcoinUnit unit{BitcoinUnit::BTC}; Q_SIGNALS: //! An intermediate signal for emitting from the `paint() const` member function. diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index c82f0683fc..be6f604932 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -158,9 +158,7 @@ PaymentServer::PaymentServer(QObject* parent, bool startLocalServer) : } } -PaymentServer::~PaymentServer() -{ -} +PaymentServer::~PaymentServer() = default; // // OSX-specific way of handling bitcoin: URIs diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index 41c389d9cc..b7de88225e 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -28,10 +28,7 @@ PeerTableModel::PeerTableModel(interfaces::Node& node, QObject* parent) : refresh(); } -PeerTableModel::~PeerTableModel() -{ - // Intentionally left empty -} +PeerTableModel::~PeerTableModel() = default; void PeerTableModel::startAutoRefresh() { diff --git a/src/qt/recentrequeststablemodel.cpp b/src/qt/recentrequeststablemodel.cpp index 03ca9ad7dc..061513b58f 100644 --- a/src/qt/recentrequeststablemodel.cpp +++ b/src/qt/recentrequeststablemodel.cpp @@ -34,10 +34,7 @@ RecentRequestsTableModel::RecentRequestsTableModel(WalletModel *parent) : connect(walletModel->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &RecentRequestsTableModel::updateDisplayUnit); } -RecentRequestsTableModel::~RecentRequestsTableModel() -{ - /* Intentionally left empty */ -} +RecentRequestsTableModel::~RecentRequestsTableModel() = default; int RecentRequestsTableModel::rowCount(const QModelIndex &parent) const { diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 4a51990f88..13455b3bec 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -120,7 +120,7 @@ public: connect(&timer, &QTimer::timeout, [this]{ func(); }); timer.start(millis); } - ~QtRPCTimerBase() {} + ~QtRPCTimerBase() = default; private: QTimer timer; std::function<void()> func; @@ -129,7 +129,7 @@ private: class QtRPCTimerInterface: public RPCTimerInterface { public: - ~QtRPCTimerInterface() {} + ~QtRPCTimerInterface() = default; const char *Name() override { return "Qt"; } RPCTimerBase* NewTimer(std::function<void()>& func, int64_t millis) override { diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index 66637a5dcf..ba0e4c686f 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -8,6 +8,7 @@ #include <interfaces/chain.h> #include <interfaces/node.h> +#include <qt/addressbookpage.h> #include <qt/clientmodel.h> #include <qt/editaddressdialog.h> #include <qt/optionsmodel.h> @@ -23,8 +24,9 @@ #include <chrono> #include <QApplication> -#include <QTimer> #include <QMessageBox> +#include <QTableView> +#include <QTimer> using wallet::AddWallet; using wallet::CWallet; @@ -131,14 +133,19 @@ void TestAddAddressesToSendBook(interfaces::Node& node) EditAddressDialog editAddressDialog(EditAddressDialog::NewSendingAddress); editAddressDialog.setModel(walletModel.getAddressTableModel()); + AddressBookPage address_book{platformStyle.get(), AddressBookPage::ForEditing, AddressBookPage::SendingTab}; + address_book.setModel(walletModel.getAddressTableModel()); + auto table_view = address_book.findChild<QTableView*>("tableView"); + QCOMPARE(table_view->model()->rowCount(), 1); + EditAddressAndSubmit( &editAddressDialog, QString("uhoh"), preexisting_r_address, QString( "Address \"%1\" already exists as a receiving address with label " "\"%2\" and so cannot be added as a sending address." ).arg(preexisting_r_address).arg(r_label)); - check_addbook_size(2); + QCOMPARE(table_view->model()->rowCount(), 1); EditAddressAndSubmit( &editAddressDialog, QString("uhoh, different"), preexisting_s_address, @@ -146,15 +153,15 @@ void TestAddAddressesToSendBook(interfaces::Node& node) "The entered address \"%1\" is already in the address book with " "label \"%2\"." ).arg(preexisting_s_address).arg(s_label)); - check_addbook_size(2); + QCOMPARE(table_view->model()->rowCount(), 1); // Submit a new address which should add successfully - we expect the // warning message to be blank. EditAddressAndSubmit( &editAddressDialog, QString("new"), new_address, QString("")); - check_addbook_size(3); + QCOMPARE(table_view->model()->rowCount(), 2); } } // namespace diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 7b932890cf..4312b3cd24 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -62,7 +62,7 @@ struct TxLessThan struct TransactionNotification { public: - TransactionNotification() {} + TransactionNotification() = default; TransactionNotification(uint256 _hash, ChangeType _status, bool _showTransaction): hash(_hash), status(_status), showTransaction(_showTransaction) {} diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index dc4e25a02b..11bea85b21 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -55,9 +55,7 @@ WalletFrame::WalletFrame(const PlatformStyle* _platformStyle, QWidget* parent) walletStack->addWidget(no_wallet_group); } -WalletFrame::~WalletFrame() -{ -} +WalletFrame::~WalletFrame() = default; void WalletFrame::setClientModel(ClientModel *_clientModel) { diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 5ee32e79d5..1f6c90af4a 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -145,7 +145,7 @@ void WalletModel::updateWatchOnlyFlag(bool fHaveWatchonly) Q_EMIT notifyWatchonlyChanged(fHaveWatchonly); } -bool WalletModel::validateAddress(const QString &address) +bool WalletModel::validateAddress(const QString& address) const { return IsValidDestinationString(address.toStdString()); } @@ -284,22 +284,22 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran return SendCoinsReturn(OK); } -OptionsModel *WalletModel::getOptionsModel() +OptionsModel* WalletModel::getOptionsModel() const { return optionsModel; } -AddressTableModel *WalletModel::getAddressTableModel() +AddressTableModel* WalletModel::getAddressTableModel() const { return addressTableModel; } -TransactionTableModel *WalletModel::getTransactionTableModel() +TransactionTableModel* WalletModel::getTransactionTableModel() const { return transactionTableModel; } -RecentRequestsTableModel *WalletModel::getRecentRequestsTableModel() +RecentRequestsTableModel* WalletModel::getRecentRequestsTableModel() const { return recentRequestsTableModel; } @@ -369,7 +369,7 @@ static void NotifyAddressBookChanged(WalletModel *walletmodel, QString strPurpose = QString::fromStdString(purpose); qDebug() << "NotifyAddressBookChanged: " + strAddress + " " + strLabel + " isMine=" + QString::number(isMine) + " purpose=" + strPurpose + " status=" + QString::number(status); - bool invoked = QMetaObject::invokeMethod(walletmodel, "updateAddressBook", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(walletmodel, "updateAddressBook", Q_ARG(QString, strAddress), Q_ARG(QString, strLabel), Q_ARG(bool, isMine), @@ -557,7 +557,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) return true; } -bool WalletModel::displayAddress(std::string sAddress) +bool WalletModel::displayAddress(std::string sAddress) const { CTxDestination dest = DecodeDestination(sAddress); bool res = false; @@ -585,7 +585,7 @@ QString WalletModel::getDisplayName() const return name.isEmpty() ? "["+tr("default wallet")+"]" : name; } -bool WalletModel::isMultiwallet() +bool WalletModel::isMultiwallet() const { return m_node.walletLoader().getWallets().size() > 1; } diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index ad1239ccdc..540fdaafe3 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -77,15 +77,15 @@ public: Unlocked // wallet->IsCrypted() && !wallet->IsLocked() }; - OptionsModel *getOptionsModel(); - AddressTableModel *getAddressTableModel(); - TransactionTableModel *getTransactionTableModel(); - RecentRequestsTableModel *getRecentRequestsTableModel(); + OptionsModel* getOptionsModel() const; + AddressTableModel* getAddressTableModel() const; + TransactionTableModel* getTransactionTableModel() const; + RecentRequestsTableModel* getRecentRequestsTableModel() const; EncryptionStatus getEncryptionStatus() const; // Check address for validity - bool validateAddress(const QString &address); + bool validateAddress(const QString& address) const; // Return status record for SendCoins, contains error id + information struct SendCoinsReturn @@ -137,7 +137,7 @@ public: UnlockContext requestUnlock(); bool bumpFee(uint256 hash, uint256& new_hash); - bool displayAddress(std::string sAddress); + bool displayAddress(std::string sAddress) const; static bool isWalletEnabled(); @@ -149,9 +149,7 @@ public: QString getWalletName() const; QString getDisplayName() const; - bool isMultiwallet(); - - AddressTableModel* getAddressTableModel() const { return addressTableModel; } + bool isMultiwallet() const; void refresh(bool pk_hash_only = false); diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index 344bf628bb..2f92c57607 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -111,9 +111,7 @@ WalletView::WalletView(WalletModel* wallet_model, const PlatformStyle* _platform connect(walletModel, &WalletModel::showProgress, this, &WalletView::showProgress); } -WalletView::~WalletView() -{ -} +WalletView::~WalletView() = default; void WalletView::setClientModel(ClientModel *_clientModel) { diff --git a/src/random.cpp b/src/random.cpp index 6ae08103b1..74ceb3d2a3 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -370,11 +370,9 @@ public: InitHardwareRand(); } - ~RNGState() - { - } + ~RNGState() = default; - void AddEvent(uint32_t event_info) noexcept + void AddEvent(uint32_t event_info) noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_events_mutex) { LOCK(m_events_mutex); @@ -388,7 +386,7 @@ public: /** * Feed (the hash of) all events added through AddEvent() to hasher. */ - void SeedEvents(CSHA512& hasher) noexcept + void SeedEvents(CSHA512& hasher) noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_events_mutex) { // We use only SHA256 for the events hashing to get the ASM speedups we have for SHA256, // since we want it to be fast as network peers may be able to trigger it repeatedly. @@ -407,7 +405,7 @@ public: * * If this function has never been called with strong_seed = true, false is returned. */ - bool MixExtract(unsigned char* out, size_t num, CSHA512&& hasher, bool strong_seed) noexcept + bool MixExtract(unsigned char* out, size_t num, CSHA512&& hasher, bool strong_seed) noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { assert(num <= 32); unsigned char buf[64]; @@ -586,16 +584,11 @@ void RandAddEvent(const uint32_t event_info) noexcept { GetRNGState().AddEvent(e bool g_mock_deterministic_tests{false}; -uint64_t GetRand(uint64_t nMax) noexcept +uint64_t GetRandInternal(uint64_t nMax) noexcept { return FastRandomContext(g_mock_deterministic_tests).randrange(nMax); } -int GetRandInt(int nMax) noexcept -{ - return GetRand(nMax); -} - uint256 GetRandHash() noexcept { uint256 hash; diff --git a/src/random.h b/src/random.h index 285158b1c3..b92c29f0be 100644 --- a/src/random.h +++ b/src/random.h @@ -69,7 +69,17 @@ */ void GetRandBytes(Span<unsigned char> bytes) noexcept; /** Generate a uniform random integer in the range [0..range). Precondition: range > 0 */ -uint64_t GetRand(uint64_t nMax) noexcept; +uint64_t GetRandInternal(uint64_t nMax) noexcept; +/** Generate a uniform random integer of type T in the range [0..nMax) + * nMax defaults to std::numeric_limits<T>::max() + * Precondition: nMax > 0, T is an integral type, no larger than uint64_t + */ +template<typename T> +T GetRand(T nMax=std::numeric_limits<T>::max()) noexcept { + static_assert(std::is_integral<T>(), "T must be integral"); + static_assert(std::numeric_limits<T>::max() <= std::numeric_limits<uint64_t>::max(), "GetRand only supports up to uint64_t"); + return T(GetRandInternal(nMax)); +} /** Generate a uniform random duration in the range [0..max). Precondition: max.count() > 0 */ template <typename D> D GetRandomDuration(typename std::common_type<D>::type max) noexcept @@ -95,7 +105,6 @@ constexpr auto GetRandMillis = GetRandomDuration<std::chrono::milliseconds>; * */ std::chrono::microseconds GetExponentialRand(std::chrono::microseconds now, std::chrono::seconds average_interval); -int GetRandInt(int nMax) noexcept; uint256 GetRandHash() noexcept; /** @@ -223,6 +232,17 @@ public: /** Generate a random boolean. */ bool randbool() noexcept { return randbits(1); } + /** Return the time point advanced by a uniform random duration. */ + template <typename Tp> + Tp rand_uniform_delay(const Tp& time, typename Tp::duration range) + { + using Dur = typename Tp::duration; + Dur dur{range.count() > 0 ? /* interval [0..range) */ Dur{randrange(range.count())} : + range.count() < 0 ? /* interval (range..0] */ -Dur{randrange(-range.count())} : + /* interval [0..0] */ Dur{0}}; + return time + dur; + } + // Compatibility with the C++11 UniformRandomBitGenerator concept typedef uint64_t result_type; static constexpr uint64_t min() { return 0; } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 57b5178d78..0715ceae97 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -993,8 +993,7 @@ static RPCHelpMan gettxout() UniValue ret(UniValue::VOBJ); uint256 hash(ParseHashV(request.params[0], "txid")); - int n = request.params[1].get_int(); - COutPoint out(hash, n); + COutPoint out{hash, request.params[1].getInt<uint32_t>()}; bool fMempool = true; if (!request.params[2].isNull()) fMempool = request.params[2].get_bool(); @@ -1064,26 +1063,26 @@ static RPCHelpMan verifychain() }; } -static void SoftForkDescPushBack(const CBlockIndex* blockindex, UniValue& softforks, const Consensus::Params& params, Consensus::BuriedDeployment dep) +static void SoftForkDescPushBack(const CBlockIndex* blockindex, UniValue& softforks, const ChainstateManager& chainman, Consensus::BuriedDeployment dep) { // For buried deployments. - if (!DeploymentEnabled(params, dep)) return; + if (!DeploymentEnabled(chainman, dep)) return; UniValue rv(UniValue::VOBJ); rv.pushKV("type", "buried"); // getdeploymentinfo reports the softfork as active from when the chain height is // one below the activation height - rv.pushKV("active", DeploymentActiveAfter(blockindex, params, dep)); - rv.pushKV("height", params.DeploymentHeight(dep)); + rv.pushKV("active", DeploymentActiveAfter(blockindex, chainman, dep)); + rv.pushKV("height", chainman.GetConsensus().DeploymentHeight(dep)); softforks.pushKV(DeploymentName(dep), rv); } -static void SoftForkDescPushBack(const CBlockIndex* blockindex, UniValue& softforks, const Consensus::Params& consensusParams, Consensus::DeploymentPos id) +static void SoftForkDescPushBack(const CBlockIndex* blockindex, UniValue& softforks, const ChainstateManager& chainman, Consensus::DeploymentPos id) { // For BIP9 deployments. - if (!DeploymentEnabled(consensusParams, id)) return; + if (!DeploymentEnabled(chainman, id)) return; if (blockindex == nullptr) return; auto get_state_name = [](const ThresholdState state) -> std::string { @@ -1099,29 +1098,29 @@ static void SoftForkDescPushBack(const CBlockIndex* blockindex, UniValue& softfo UniValue bip9(UniValue::VOBJ); - const ThresholdState next_state = g_versionbitscache.State(blockindex, consensusParams, id); - const ThresholdState current_state = g_versionbitscache.State(blockindex->pprev, consensusParams, id); + const ThresholdState next_state = chainman.m_versionbitscache.State(blockindex, chainman.GetConsensus(), id); + const ThresholdState current_state = chainman.m_versionbitscache.State(blockindex->pprev, chainman.GetConsensus(), id); const bool has_signal = (ThresholdState::STARTED == current_state || ThresholdState::LOCKED_IN == current_state); // BIP9 parameters if (has_signal) { - bip9.pushKV("bit", consensusParams.vDeployments[id].bit); + bip9.pushKV("bit", chainman.GetConsensus().vDeployments[id].bit); } - bip9.pushKV("start_time", consensusParams.vDeployments[id].nStartTime); - bip9.pushKV("timeout", consensusParams.vDeployments[id].nTimeout); - bip9.pushKV("min_activation_height", consensusParams.vDeployments[id].min_activation_height); + bip9.pushKV("start_time", chainman.GetConsensus().vDeployments[id].nStartTime); + bip9.pushKV("timeout", chainman.GetConsensus().vDeployments[id].nTimeout); + bip9.pushKV("min_activation_height", chainman.GetConsensus().vDeployments[id].min_activation_height); // BIP9 status bip9.pushKV("status", get_state_name(current_state)); - bip9.pushKV("since", g_versionbitscache.StateSinceHeight(blockindex->pprev, consensusParams, id)); + bip9.pushKV("since", chainman.m_versionbitscache.StateSinceHeight(blockindex->pprev, chainman.GetConsensus(), id)); bip9.pushKV("status_next", get_state_name(next_state)); // BIP9 signalling status, if applicable if (has_signal) { UniValue statsUV(UniValue::VOBJ); std::vector<bool> signals; - BIP9Stats statsStruct = g_versionbitscache.Statistics(blockindex, consensusParams, id, &signals); + BIP9Stats statsStruct = chainman.m_versionbitscache.Statistics(blockindex, chainman.GetConsensus(), id, &signals); statsUV.pushKV("period", statsStruct.period); statsUV.pushKV("elapsed", statsStruct.elapsed); statsUV.pushKV("count", statsStruct.count); @@ -1142,7 +1141,7 @@ static void SoftForkDescPushBack(const CBlockIndex* blockindex, UniValue& softfo UniValue rv(UniValue::VOBJ); rv.pushKV("type", "bip9"); if (ThresholdState::ACTIVE == next_state) { - rv.pushKV("height", g_versionbitscache.StateSinceHeight(blockindex, consensusParams, id)); + rv.pushKV("height", chainman.m_versionbitscache.StateSinceHeight(blockindex, chainman.GetConsensus(), id)); } rv.pushKV("active", ThresholdState::ACTIVE == next_state); rv.pushKV("bip9", bip9); @@ -1150,16 +1149,9 @@ static void SoftForkDescPushBack(const CBlockIndex* blockindex, UniValue& softfo softforks.pushKV(DeploymentName(id), rv); } -namespace { -/* TODO: when -deprecatedrpc=softforks is removed, drop these */ -UniValue DeploymentInfo(const CBlockIndex* tip, const Consensus::Params& consensusParams); -extern const std::vector<RPCResult> RPCHelpForDeployment; -} - // used by rest.cpp:rest_chaininfo, so cannot be static RPCHelpMan getblockchaininfo() { - /* TODO: from v24, remove -deprecatedrpc=softforks */ return RPCHelpMan{"getblockchaininfo", "Returns an object containing various state info regarding blockchain processing.\n", {}, @@ -1178,15 +1170,9 @@ RPCHelpMan getblockchaininfo() {RPCResult::Type::STR_HEX, "chainwork", "total amount of work in active chain, in hexadecimal"}, {RPCResult::Type::NUM, "size_on_disk", "the estimated size of the block and undo files on disk"}, {RPCResult::Type::BOOL, "pruned", "if the blocks are subject to pruning"}, - {RPCResult::Type::NUM, "pruneheight", /*optional=*/true, "lowest-height complete block stored (only present if pruning is enabled)"}, + {RPCResult::Type::NUM, "pruneheight", /*optional=*/true, "height of the last block pruned, plus one (only present if pruning is enabled)"}, {RPCResult::Type::BOOL, "automatic_pruning", /*optional=*/true, "whether automatic pruning is enabled (only present if pruning is enabled)"}, {RPCResult::Type::NUM, "prune_target_size", /*optional=*/true, "the target size used by pruning (only present if automatic pruning is enabled)"}, - {RPCResult::Type::OBJ_DYN, "softforks", /*optional=*/true, "(DEPRECATED, returned only if config option -deprecatedrpc=softforks is passed) status of softforks", - { - {RPCResult::Type::OBJ, "xxxx", "name of the softfork", - RPCHelpForDeployment - }, - }}, {RPCResult::Type::STR, "warnings", "any network and blockchain warnings"}, }}, RPCExamples{ @@ -1226,11 +1212,6 @@ RPCHelpMan getblockchaininfo() } } - if (IsDeprecatedRPCEnabled("softforks")) { - const Consensus::Params& consensusParams = Params().GetConsensus(); - obj.pushKV("softforks", DeploymentInfo(&tip, consensusParams)); - } - obj.pushKV("warnings", GetWarnings(false).original); return obj; }, @@ -1263,16 +1244,16 @@ const std::vector<RPCResult> RPCHelpForDeployment{ }}, }; -UniValue DeploymentInfo(const CBlockIndex* blockindex, const Consensus::Params& consensusParams) +UniValue DeploymentInfo(const CBlockIndex* blockindex, const ChainstateManager& chainman) { UniValue softforks(UniValue::VOBJ); - SoftForkDescPushBack(blockindex, softforks, consensusParams, Consensus::DEPLOYMENT_HEIGHTINCB); - SoftForkDescPushBack(blockindex, softforks, consensusParams, Consensus::DEPLOYMENT_DERSIG); - SoftForkDescPushBack(blockindex, softforks, consensusParams, Consensus::DEPLOYMENT_CLTV); - SoftForkDescPushBack(blockindex, softforks, consensusParams, Consensus::DEPLOYMENT_CSV); - SoftForkDescPushBack(blockindex, softforks, consensusParams, Consensus::DEPLOYMENT_SEGWIT); - SoftForkDescPushBack(blockindex, softforks, consensusParams, Consensus::DEPLOYMENT_TESTDUMMY); - SoftForkDescPushBack(blockindex, softforks, consensusParams, Consensus::DEPLOYMENT_TAPROOT); + SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_HEIGHTINCB); + SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_DERSIG); + SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_CLTV); + SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_CSV); + SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_SEGWIT); + SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_TESTDUMMY); + SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_TAPROOT); return softforks; } } // anon namespace @@ -1311,12 +1292,10 @@ static RPCHelpMan getdeploymentinfo() } } - const Consensus::Params& consensusParams = Params().GetConsensus(); - UniValue deploymentinfo(UniValue::VOBJ); deploymentinfo.pushKV("hash", blockindex->GetBlockHash().ToString()); deploymentinfo.pushKV("height", blockindex->nHeight); - deploymentinfo.pushKV("deployments", DeploymentInfo(blockindex, consensusParams)); + deploymentinfo.pushKV("deployments", DeploymentInfo(blockindex, chainman)); return deploymentinfo; }, }; @@ -1969,9 +1948,9 @@ static std::atomic<bool> g_should_abort_scan; class CoinsViewScanReserver { private: - bool m_could_reserve; + bool m_could_reserve{false}; public: - explicit CoinsViewScanReserver() : m_could_reserve(false) {} + explicit CoinsViewScanReserver() = default; bool reserve() { CHECK_NONFATAL(!m_could_reserve); @@ -2281,6 +2260,12 @@ static RPCHelpMan dumptxoutset() FILE* file{fsbridge::fopen(temppath, "wb")}; CAutoFile afile{file, SER_DISK, CLIENT_VERSION}; + if (afile.IsNull()) { + throw JSONRPCError( + RPC_INVALID_PARAMETER, + "Couldn't open file " + temppath.u8string() + " for writing."); + } + NodeContext& node = EnsureAnyNodeContext(request.context); UniValue result = CreateUTXOSnapshot( node, node.chainman->ActiveChainstate(), afile, path, temppath); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index b552528951..2b8bd01c07 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -133,7 +133,7 @@ static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t& } std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(block); - if (!chainman.ProcessNewBlock(chainparams, shared_pblock, true, nullptr)) { + if (!chainman.ProcessNewBlock(shared_pblock, true, nullptr)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted"); } @@ -770,7 +770,7 @@ static RPCHelpMan getblocktemplate() pblock->nNonce = 0; // NOTE: If at some point we support pre-segwit miners post-segwit-activation, this needs to take segwit support into consideration - const bool fPreSegWit = !DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_SEGWIT); + const bool fPreSegWit = !DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_SEGWIT); UniValue aCaps(UniValue::VARR); aCaps.push_back("proposal"); @@ -836,7 +836,7 @@ static RPCHelpMan getblocktemplate() UniValue vbavailable(UniValue::VOBJ); for (int j = 0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) { Consensus::DeploymentPos pos = Consensus::DeploymentPos(j); - ThresholdState state = g_versionbitscache.State(pindexPrev, consensusParams, pos); + ThresholdState state = chainman.m_versionbitscache.State(pindexPrev, consensusParams, pos); switch (state) { case ThresholdState::DEFINED: case ThresholdState::FAILED: @@ -844,7 +844,7 @@ static RPCHelpMan getblocktemplate() break; case ThresholdState::LOCKED_IN: // Ensure bit is set in block version - pblock->nVersion |= g_versionbitscache.Mask(consensusParams, pos); + pblock->nVersion |= chainman.m_versionbitscache.Mask(consensusParams, pos); [[fallthrough]]; case ThresholdState::STARTED: { @@ -853,7 +853,7 @@ static RPCHelpMan getblocktemplate() if (setClientRules.find(vbinfo.name) == setClientRules.end()) { if (!vbinfo.gbt_force) { // If the client doesn't support this, don't indicate it in the [default] version - pblock->nVersion &= ~g_versionbitscache.Mask(consensusParams, pos); + pblock->nVersion &= ~chainman.m_versionbitscache.Mask(consensusParams, pos); } } break; @@ -930,10 +930,10 @@ class submitblock_StateCatcher final : public CValidationInterface { public: uint256 hash; - bool found; + bool found{false}; BlockValidationState state; - explicit submitblock_StateCatcher(const uint256 &hashIn) : hash(hashIn), found(false), state() {} + explicit submitblock_StateCatcher(const uint256 &hashIn) : hash(hashIn), state() {} protected: void BlockChecked(const CBlock& block, const BlockValidationState& stateIn) override { @@ -993,14 +993,14 @@ static RPCHelpMan submitblock() LOCK(cs_main); const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock); if (pindex) { - UpdateUncommittedBlockStructures(block, pindex, Params().GetConsensus()); + chainman.UpdateUncommittedBlockStructures(block, pindex); } } bool new_block; auto sc = std::make_shared<submitblock_StateCatcher>(block.GetHash()); RegisterSharedValidationInterface(sc); - bool accepted = chainman.ProcessNewBlock(Params(), blockptr, /*force_processing=*/true, /*new_block=*/&new_block); + bool accepted = chainman.ProcessNewBlock(blockptr, /*force_processing=*/true, /*new_block=*/&new_block); UnregisterSharedValidationInterface(sc); if (!new_block && accepted) { return "duplicate"; @@ -1042,7 +1042,7 @@ static RPCHelpMan submitheader() } BlockValidationState state; - chainman.ProcessNewBlockHeaders({h}, state, Params()); + chainman.ProcessNewBlockHeaders({h}, state); if (state.IsValid()) return NullUniValue; if (state.IsError()) { throw JSONRPCError(RPC_VERIFY_ERROR, state.ToString()); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 09dc8eb3eb..ff714eaf36 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -932,7 +932,7 @@ static RPCHelpMan addpeeraddress() } const std::string& addr_string{request.params[0].get_str()}; - const uint16_t port{static_cast<uint16_t>(request.params[1].get_int())}; + const auto port{request.params[1].getInt<uint16_t>()}; const bool tried{request.params[2].isTrue()}; UniValue obj(UniValue::VOBJ); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index ef3e58494e..932c93b48c 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -12,6 +12,7 @@ #include <util/check.h> #include <util/strencodings.h> #include <util/string.h> +#include <util/system.h> #include <util/translation.h> #include <tuple> @@ -267,7 +268,7 @@ CTxDestination AddAndGetMultisigDestination(const int required, const std::vecto class DescribeAddressVisitor { public: - explicit DescribeAddressVisitor() {} + explicit DescribeAddressVisitor() = default; UniValue operator()(const CNoDestination& dest) const { @@ -581,7 +582,9 @@ UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) const throw std::runtime_error(ToString()); } const UniValue ret = m_fun(*this, request); - CHECK_NONFATAL(std::any_of(m_results.m_results.begin(), m_results.m_results.end(), [ret](const RPCResult& res) { return res.MatchesType(ret); })); + if (gArgs.GetBoolArg("-rpcdoccheck", DEFAULT_RPC_DOC_CHECK)) { + CHECK_NONFATAL(std::any_of(m_results.m_results.begin(), m_results.m_results.end(), [ret](const RPCResult& res) { return res.MatchesType(ret); })); + } return ret; } diff --git a/src/rpc/util.h b/src/rpc/util.h index e16fed75bc..6e23caff6c 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -22,6 +22,8 @@ #include <variant> #include <vector> +static constexpr bool DEFAULT_RPC_DOC_CHECK{false}; + /** * String used to describe UNIX epoch time in documentation, factored out to a * constant for consistency. diff --git a/src/scheduler.cpp b/src/scheduler.cpp index 3e7ee7d370..3df1d48b3c 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -4,17 +4,15 @@ #include <scheduler.h> -#include <random.h> +#include <sync.h> #include <util/syscall_sandbox.h> #include <util/time.h> -#include <assert.h> +#include <cassert> #include <functional> #include <utility> -CScheduler::CScheduler() -{ -} +CScheduler::CScheduler() = default; CScheduler::~CScheduler() { @@ -43,7 +41,7 @@ void CScheduler::serviceQueue() // the time of the first item on the queue: while (!shouldStop() && !taskQueue.empty()) { - std::chrono::system_clock::time_point timeToWaitFor = taskQueue.begin()->first; + std::chrono::steady_clock::time_point timeToWaitFor = taskQueue.begin()->first; if (newTaskScheduled.wait_until(lock, timeToWaitFor) == std::cv_status::timeout) { break; // Exit loop after timeout, it means we reached the time of the event } @@ -72,7 +70,7 @@ void CScheduler::serviceQueue() newTaskScheduled.notify_one(); } -void CScheduler::schedule(CScheduler::Function f, std::chrono::system_clock::time_point t) +void CScheduler::schedule(CScheduler::Function f, std::chrono::steady_clock::time_point t) { { LOCK(newTaskMutex); @@ -89,7 +87,7 @@ void CScheduler::MockForward(std::chrono::seconds delta_seconds) LOCK(newTaskMutex); // use temp_queue to maintain updated schedule - std::multimap<std::chrono::system_clock::time_point, Function> temp_queue; + std::multimap<std::chrono::steady_clock::time_point, Function> temp_queue; for (const auto& element : taskQueue) { temp_queue.emplace_hint(temp_queue.cend(), element.first - delta_seconds, element.second); @@ -114,8 +112,8 @@ void CScheduler::scheduleEvery(CScheduler::Function f, std::chrono::milliseconds scheduleFromNow([this, f, delta] { Repeat(*this, f, delta); }, delta); } -size_t CScheduler::getQueueInfo(std::chrono::system_clock::time_point& first, - std::chrono::system_clock::time_point& last) const +size_t CScheduler::getQueueInfo(std::chrono::steady_clock::time_point& first, + std::chrono::steady_clock::time_point& last) const { LOCK(newTaskMutex); size_t result = taskQueue.size(); @@ -143,7 +141,7 @@ void SingleThreadedSchedulerClient::MaybeScheduleProcessQueue() if (m_are_callbacks_running) return; if (m_callbacks_pending.empty()) return; } - m_scheduler.schedule([this] { this->ProcessQueue(); }, std::chrono::system_clock::now()); + m_scheduler.schedule([this] { this->ProcessQueue(); }, std::chrono::steady_clock::now()); } void SingleThreadedSchedulerClient::ProcessQueue() diff --git a/src/scheduler.h b/src/scheduler.h index eb350e4bc3..749e5442b0 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -46,12 +46,12 @@ public: typedef std::function<void()> Function; /** Call func at/after time t */ - void schedule(Function f, std::chrono::system_clock::time_point t); + void schedule(Function f, std::chrono::steady_clock::time_point t) EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); /** Call f once after the delta has passed */ - void scheduleFromNow(Function f, std::chrono::milliseconds delta) + void scheduleFromNow(Function f, std::chrono::milliseconds delta) EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex) { - schedule(std::move(f), std::chrono::system_clock::now() + delta); + schedule(std::move(f), std::chrono::steady_clock::now() + delta); } /** @@ -60,29 +60,29 @@ public: * The timing is not exact: Every time f is finished, it is rescheduled to run again after delta. If you need more * accurate scheduling, don't use this method. */ - void scheduleEvery(Function f, std::chrono::milliseconds delta); + void scheduleEvery(Function f, std::chrono::milliseconds delta) EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); /** * Mock the scheduler to fast forward in time. * Iterates through items on taskQueue and reschedules them * to be delta_seconds sooner. */ - void MockForward(std::chrono::seconds delta_seconds); + void MockForward(std::chrono::seconds delta_seconds) EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); /** * Services the queue 'forever'. Should be run in a thread. */ - void serviceQueue(); + void serviceQueue() EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); /** Tell any threads running serviceQueue to stop as soon as the current task is done */ - void stop() + void stop() EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex) { WITH_LOCK(newTaskMutex, stopRequested = true); newTaskScheduled.notify_all(); if (m_service_thread.joinable()) m_service_thread.join(); } /** Tell any threads running serviceQueue to stop when there is no work left to be done */ - void StopWhenDrained() + void StopWhenDrained() EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex) { WITH_LOCK(newTaskMutex, stopWhenEmpty = true); newTaskScheduled.notify_all(); @@ -93,16 +93,17 @@ public: * Returns number of tasks waiting to be serviced, * and first and last task times */ - size_t getQueueInfo(std::chrono::system_clock::time_point& first, - std::chrono::system_clock::time_point& last) const; + size_t getQueueInfo(std::chrono::steady_clock::time_point& first, + std::chrono::steady_clock::time_point& last) const + EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); /** Returns true if there are threads actively running in serviceQueue() */ - bool AreThreadsServicingQueue() const; + bool AreThreadsServicingQueue() const EXCLUSIVE_LOCKS_REQUIRED(!newTaskMutex); private: mutable Mutex newTaskMutex; std::condition_variable newTaskScheduled; - std::multimap<std::chrono::system_clock::time_point, Function> taskQueue GUARDED_BY(newTaskMutex); + std::multimap<std::chrono::steady_clock::time_point, Function> taskQueue GUARDED_BY(newTaskMutex); int nThreadsServicingQueue GUARDED_BY(newTaskMutex){0}; bool stopRequested GUARDED_BY(newTaskMutex){false}; bool stopWhenEmpty GUARDED_BY(newTaskMutex){false}; @@ -128,8 +129,8 @@ private: std::list<std::function<void()>> m_callbacks_pending GUARDED_BY(m_callbacks_mutex); bool m_are_callbacks_running GUARDED_BY(m_callbacks_mutex) = false; - void MaybeScheduleProcessQueue(); - void ProcessQueue(); + void MaybeScheduleProcessQueue() EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex); + void ProcessQueue() EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex); public: explicit SingleThreadedSchedulerClient(CScheduler& scheduler LIFETIMEBOUND) : m_scheduler{scheduler} {} @@ -140,15 +141,15 @@ public: * Practically, this means that callbacks can behave as if they are executed * in order by a single thread. */ - void AddToProcessQueue(std::function<void()> func); + void AddToProcessQueue(std::function<void()> func) EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex); /** * Processes all remaining queue members on the calling thread, blocking until queue is empty * Must be called after the CScheduler has no remaining processing threads! */ - void EmptyQueue(); + void EmptyQueue() EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex); - size_t CallbacksPending(); + size_t CallbacksPending() EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex); }; #endif // BITCOIN_SCHEDULER_H diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 7328e8d1ad..2d569d674a 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -563,7 +563,7 @@ namespace { class DummySignatureChecker final : public BaseSignatureChecker { public: - DummySignatureChecker() {} + DummySignatureChecker() = default; bool CheckECDSASignature(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override { return true; } bool CheckSchnorrSignature(Span<const unsigned char> sig, Span<const unsigned char> pubkey, SigVersion sigversion, ScriptExecutionData& execdata, ScriptError* serror) const override { return true; } }; diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index ea1a27c6f6..6907749c6d 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -282,9 +282,8 @@ LockedPool::LockedPool(std::unique_ptr<LockedPageAllocator> allocator_in, Lockin { } -LockedPool::~LockedPool() -{ -} +LockedPool::~LockedPool() = default; + void* LockedPool::alloc(size_t size) { std::lock_guard<std::mutex> lock(mutex); diff --git a/src/sync.h b/src/sync.h index c69b58741b..a175926113 100644 --- a/src/sync.h +++ b/src/sync.h @@ -83,8 +83,6 @@ void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLi inline void DeleteLock(void* cs) {} inline bool LockStackEmpty() { return true; } #endif -#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) -#define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs) /** * Template mixin that adds -Wthread-safety locking annotations and lock order @@ -129,7 +127,13 @@ public: using RecursiveMutex = AnnotatedMixin<std::recursive_mutex>; /** Wrapped mutex: supports waiting but not recursive locking */ -typedef AnnotatedMixin<std::mutex> Mutex; +using Mutex = AnnotatedMixin<std::mutex>; + +#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) + +inline void AssertLockNotHeldInline(const char* name, const char* file, int line, Mutex* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) { AssertLockNotHeldInternal(name, file, line, cs); } +inline void AssertLockNotHeldInline(const char* name, const char* file, int line, RecursiveMutex* cs) LOCKS_EXCLUDED(cs) { AssertLockNotHeldInternal(name, file, line, cs); } +#define AssertLockNotHeld(cs) AssertLockNotHeldInline(#cs, __FILE__, __LINE__, &cs) /** Wrapper around std::unique_lock style lock for Mutex. */ template <typename Mutex, typename Base = typename Mutex::UniqueLock> diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 9dfbd7ba7c..875241094d 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) // Do a simple ShortTxIDs RT { - CBlockHeaderAndShortTxIDs shortIDs(block, true); + CBlockHeaderAndShortTxIDs shortIDs{block}; CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << shortIDs; @@ -122,7 +122,7 @@ public: stream >> *this; } explicit TestHeaderAndShortIDs(const CBlock& block) : - TestHeaderAndShortIDs(CBlockHeaderAndShortTxIDs(block, true)) {} + TestHeaderAndShortIDs(CBlockHeaderAndShortTxIDs{block}) {} uint64_t GetShortID(const uint256& txhash) const { CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); @@ -279,7 +279,7 @@ BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest) // Test simple header round-trip with only coinbase { - CBlockHeaderAndShortTxIDs shortIDs(block, false); + CBlockHeaderAndShortTxIDs shortIDs{block}; CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << shortIDs; diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 82b9617384..ffb9fb55d9 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -101,7 +101,7 @@ bool BuildChainTestingSetup::BuildChain(const CBlockIndex* pindex, CBlockHeader header = block->GetBlockHeader(); BlockValidationState state; - if (!Assert(m_node.chainman)->ProcessNewBlockHeaders({header}, state, Params(), &pindex)) { + if (!Assert(m_node.chainman)->ProcessNewBlockHeaders({header}, state, &pindex)) { return false; } } @@ -178,7 +178,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) uint256 chainA_last_header = last_header; for (size_t i = 0; i < 2; i++) { const auto& block = chainA[i]; - BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, nullptr)); + BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(block, true, nullptr)); } for (size_t i = 0; i < 2; i++) { const auto& block = chainA[i]; @@ -196,7 +196,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) uint256 chainB_last_header = last_header; for (size_t i = 0; i < 3; i++) { const auto& block = chainB[i]; - BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, nullptr)); + BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(block, true, nullptr)); } for (size_t i = 0; i < 3; i++) { const auto& block = chainB[i]; @@ -227,7 +227,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) // Reorg back to chain A. for (size_t i = 2; i < 4; i++) { const auto& block = chainA[i]; - BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, nullptr)); + BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(block, true, nullptr)); } // Check that chain A and B blocks can be retrieved. diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 79d6b94dff..875522d744 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -95,7 +95,7 @@ struct MemoryCheck { { return true; } - MemoryCheck(){}; + MemoryCheck() = default; MemoryCheck(const MemoryCheck& x) { // We have to do this to make sure that destructor calls are paired @@ -129,7 +129,7 @@ struct FrozenCleanupCheck { { return true; } - FrozenCleanupCheck() {} + FrozenCleanupCheck() = default; ~FrozenCleanupCheck() { if (should_freeze) { diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index fc89fe1450..ab4c587c46 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -321,7 +321,7 @@ struct StringContentsSerializer { // Used to make two serialized objects the same while letting them have different lengths // This is a terrible idea std::string str; - StringContentsSerializer() {} + StringContentsSerializer() = default; explicit StringContentsSerializer(const std::string& inp) : str(inp) {} StringContentsSerializer& operator+=(const std::string& s) { diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp index 59adec075e..24ae34bd9e 100644 --- a/src/test/fuzz/fuzz.cpp +++ b/src/test/fuzz/fuzz.cpp @@ -194,7 +194,7 @@ int main(int argc, char** argv) return 0; } std::signal(SIGABRT, signal_handler); - int64_t start_time = GetTimeSeconds(); + const auto start_time{Now<SteadySeconds>()}; int tested = 0; for (int i = 1; i < argc; ++i) { fs::path input_path(*(argv + i)); @@ -215,8 +215,8 @@ int main(int argc, char** argv) buffer.clear(); } } - int64_t end_time = GetTimeSeconds(); - std::cout << g_fuzz_target << ": succeeded against " << tested << " files in " << (end_time - start_time) << "s." << std::endl; + const auto end_time{Now<SteadySeconds>()}; + std::cout << g_fuzz_target << ": succeeded against " << tested << " files in " << count_seconds(end_time - start_time) << "s." << std::endl; #endif return 0; } diff --git a/src/test/fuzz/signature_checker.cpp b/src/test/fuzz/signature_checker.cpp index f6c591aca4..a585680de1 100644 --- a/src/test/fuzz/signature_checker.cpp +++ b/src/test/fuzz/signature_checker.cpp @@ -49,7 +49,7 @@ public: return m_fuzzed_data_provider.ConsumeBool(); } - virtual ~FuzzedSignatureChecker() {} + virtual ~FuzzedSignatureChecker() = default; }; } // namespace diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index e513f1883c..33496a457e 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -58,7 +58,7 @@ FUZZ_TARGET_INIT(utxo_snapshot, initialize_chain) if (fuzzed_data_provider.ConsumeBool()) { for (const auto& block : *g_chain) { BlockValidationState dummy; - bool processed{chainman.ProcessNewBlockHeaders({*block}, dummy, ::Params())}; + bool processed{chainman.ProcessNewBlockHeaders({*block}, dummy)}; Assert(processed); const auto* index{WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block->GetHash()))}; Assert(index); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index cafa5710fa..0c3a13c5f8 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -587,7 +587,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) pblock->nNonce = bi.nonce; } std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(*pblock); - BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(chainparams, shared_pblock, true, nullptr)); + BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(shared_pblock, true, nullptr)); pblock->hashPrevBlock = pblock->GetHash(); } diff --git a/src/test/random_tests.cpp b/src/test/random_tests.cpp index 978a7bee4d..9b2760fd1c 100644 --- a/src/test/random_tests.cpp +++ b/src/test/random_tests.cpp @@ -26,11 +26,21 @@ BOOST_AUTO_TEST_CASE(fastrandom_tests) FastRandomContext ctx2(true); for (int i = 10; i > 0; --i) { - BOOST_CHECK_EQUAL(GetRand(std::numeric_limits<uint64_t>::max()), uint64_t{10393729187455219830U}); - BOOST_CHECK_EQUAL(GetRandInt(std::numeric_limits<int>::max()), int{769702006}); + BOOST_CHECK_EQUAL(GetRand<uint64_t>(), uint64_t{10393729187455219830U}); + BOOST_CHECK_EQUAL(GetRand<int>(), int{769702006}); BOOST_CHECK_EQUAL(GetRandMicros(std::chrono::hours{1}).count(), 2917185654); BOOST_CHECK_EQUAL(GetRandMillis(std::chrono::hours{1}).count(), 2144374); } + { + constexpr SteadySeconds time_point{1s}; + FastRandomContext ctx{true}; + BOOST_CHECK_EQUAL(7, ctx.rand_uniform_delay(time_point, 9s).time_since_epoch().count()); + BOOST_CHECK_EQUAL(-6, ctx.rand_uniform_delay(time_point, -9s).time_since_epoch().count()); + BOOST_CHECK_EQUAL(1, ctx.rand_uniform_delay(time_point, 0s).time_since_epoch().count()); + BOOST_CHECK_EQUAL(1467825113502396065, ctx.rand_uniform_delay(time_point, 9223372036854775807s).time_since_epoch().count()); + BOOST_CHECK_EQUAL(-970181367944767837, ctx.rand_uniform_delay(time_point, -9223372036854775807s).time_since_epoch().count()); + BOOST_CHECK_EQUAL(24761, ctx.rand_uniform_delay(time_point, 9h).time_since_epoch().count()); + } BOOST_CHECK_EQUAL(ctx1.rand32(), ctx2.rand32()); BOOST_CHECK_EQUAL(ctx1.rand32(), ctx2.rand32()); BOOST_CHECK_EQUAL(ctx1.rand64(), ctx2.rand64()); @@ -47,8 +57,8 @@ BOOST_AUTO_TEST_CASE(fastrandom_tests) // Check that a nondeterministic ones are not g_mock_deterministic_tests = false; for (int i = 10; i > 0; --i) { - BOOST_CHECK(GetRand(std::numeric_limits<uint64_t>::max()) != uint64_t{10393729187455219830U}); - BOOST_CHECK(GetRandInt(std::numeric_limits<int>::max()) != int{769702006}); + BOOST_CHECK(GetRand<uint64_t>() != uint64_t{10393729187455219830U}); + BOOST_CHECK(GetRand<int>() != int{769702006}); BOOST_CHECK(GetRandMicros(std::chrono::hours{1}) != std::chrono::microseconds{2917185654}); BOOST_CHECK(GetRandMillis(std::chrono::hours{1}) != std::chrono::milliseconds{2144374}); } diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp index 6e089de0c1..7b5dda8114 100644 --- a/src/test/scheduler_tests.cpp +++ b/src/test/scheduler_tests.cpp @@ -15,13 +15,13 @@ BOOST_AUTO_TEST_SUITE(scheduler_tests) -static void microTask(CScheduler& s, std::mutex& mutex, int& counter, int delta, std::chrono::system_clock::time_point rescheduleTime) +static void microTask(CScheduler& s, std::mutex& mutex, int& counter, int delta, std::chrono::steady_clock::time_point rescheduleTime) { { std::lock_guard<std::mutex> lock(mutex); counter += delta; } - std::chrono::system_clock::time_point noTime = std::chrono::system_clock::time_point::min(); + auto noTime = std::chrono::steady_clock::time_point::min(); if (rescheduleTime != noTime) { CScheduler::Function f = std::bind(µTask, std::ref(s), std::ref(mutex), std::ref(counter), -delta + 1, noTime); s.schedule(f, rescheduleTime); @@ -49,15 +49,15 @@ BOOST_AUTO_TEST_CASE(manythreads) auto randomMsec = [](FastRandomContext& rc) -> int { return -11 + (int)rc.randrange(1012); }; // [-11, 1000] auto randomDelta = [](FastRandomContext& rc) -> int { return -1000 + (int)rc.randrange(2001); }; // [-1000, 1000] - std::chrono::system_clock::time_point start = std::chrono::system_clock::now(); - std::chrono::system_clock::time_point now = start; - std::chrono::system_clock::time_point first, last; + auto start = std::chrono::steady_clock::now(); + auto now = start; + std::chrono::steady_clock::time_point first, last; size_t nTasks = microTasks.getQueueInfo(first, last); BOOST_CHECK(nTasks == 0); for (int i = 0; i < 100; ++i) { - std::chrono::system_clock::time_point t = now + std::chrono::microseconds(randomMsec(rng)); - std::chrono::system_clock::time_point tReschedule = now + std::chrono::microseconds(500 + randomMsec(rng)); + auto t = now + std::chrono::microseconds(randomMsec(rng)); + auto tReschedule = now + std::chrono::microseconds(500 + randomMsec(rng)); int whichCounter = zeroToNine(rng); CScheduler::Function f = std::bind(µTask, std::ref(microTasks), std::ref(counterMutex[whichCounter]), std::ref(counter[whichCounter]), @@ -75,14 +75,14 @@ BOOST_AUTO_TEST_CASE(manythreads) microThreads.emplace_back(std::bind(&CScheduler::serviceQueue, µTasks)); UninterruptibleSleep(std::chrono::microseconds{600}); - now = std::chrono::system_clock::now(); + now = std::chrono::steady_clock::now(); // More threads and more tasks: for (int i = 0; i < 5; i++) microThreads.emplace_back(std::bind(&CScheduler::serviceQueue, µTasks)); for (int i = 0; i < 100; i++) { - std::chrono::system_clock::time_point t = now + std::chrono::microseconds(randomMsec(rng)); - std::chrono::system_clock::time_point tReschedule = now + std::chrono::microseconds(500 + randomMsec(rng)); + auto t = now + std::chrono::microseconds(randomMsec(rng)); + auto tReschedule = now + std::chrono::microseconds(500 + randomMsec(rng)); int whichCounter = zeroToNine(rng); CScheduler::Function f = std::bind(µTask, std::ref(microTasks), std::ref(counterMutex[whichCounter]), std::ref(counter[whichCounter]), @@ -111,8 +111,8 @@ BOOST_AUTO_TEST_CASE(wait_until_past) Mutex mtx; WAIT_LOCK(mtx, lock); - const auto no_wait= [&](const std::chrono::seconds& d) { - return condvar.wait_until(lock, std::chrono::system_clock::now() - d); + const auto no_wait = [&](const std::chrono::seconds& d) { + return condvar.wait_until(lock, std::chrono::steady_clock::now() - d); }; BOOST_CHECK(std::cv_status::timeout == no_wait(std::chrono::seconds{1})); @@ -183,7 +183,7 @@ BOOST_AUTO_TEST_CASE(mockforward) scheduler.scheduleFromNow(dummy, std::chrono::minutes{8}); // check taskQueue - std::chrono::system_clock::time_point first, last; + std::chrono::steady_clock::time_point first, last; size_t num_tasks = scheduler.getQueueInfo(first, last); BOOST_CHECK_EQUAL(num_tasks, 3ul); @@ -204,7 +204,7 @@ BOOST_AUTO_TEST_CASE(mockforward) BOOST_CHECK_EQUAL(counter, 2); // check that the time of the remaining job has been updated - std::chrono::system_clock::time_point now = std::chrono::system_clock::now(); + auto now = std::chrono::steady_clock::now(); int delta = std::chrono::duration_cast<std::chrono::seconds>(first - now).count(); // should be between 2 & 3 minutes from now BOOST_CHECK(delta > 2*60 && delta < 3*60); diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 19e32d0c36..e44acc4932 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -257,11 +257,11 @@ private: CScriptWitness scriptWitness; CTransactionRef creditTx; CMutableTransaction spendTx; - bool havePush; + bool havePush{false}; std::vector<unsigned char> push; std::string comment; uint32_t flags; - int scriptError; + int scriptError{SCRIPT_ERR_OK}; CAmount nValue; void DoPush() @@ -280,7 +280,7 @@ private: } public: - TestBuilder(const CScript& script_, const std::string& comment_, uint32_t flags_, bool P2SH = false, WitnessMode wm = WitnessMode::NONE, int witnessversion = 0, CAmount nValue_ = 0) : script(script_), havePush(false), comment(comment_), flags(flags_), scriptError(SCRIPT_ERR_OK), nValue(nValue_) + TestBuilder(const CScript& script_, const std::string& comment_, uint32_t flags_, bool P2SH = false, WitnessMode wm = WitnessMode::NONE, int witnessversion = 0, CAmount nValue_ = 0) : script(script_), comment(comment_), flags(flags_), nValue(nValue_) { CScript scriptPubKey = script; if (wm == WitnessMode::PKH) { diff --git a/src/test/util/mining.cpp b/src/test/util/mining.cpp index 5ed8598e8e..52e02c4fa4 100644 --- a/src/test/util/mining.cpp +++ b/src/test/util/mining.cpp @@ -68,7 +68,7 @@ CTxIn MineBlock(const NodeContext& node, const CScript& coinbase_scriptPubKey) assert(block->nNonce); } - bool processed{Assert(node.chainman)->ProcessNewBlock(Params(), block, true, nullptr)}; + bool processed{Assert(node.chainman)->ProcessNewBlock(block, true, nullptr)}; assert(processed); return CTxIn{block->vtx[0]->GetHash(), 0}; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2fc71c2a6e..6891629f8e 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -151,6 +151,8 @@ BasicTestingSetup::~BasicTestingSetup() ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::vector<const char*>& extra_args) : BasicTestingSetup(chainName, extra_args) { + const CChainParams& chainparams = Params(); + // We have to run a scheduler thread to prevent ActivateBestChain // from blocking due to queue overrun. m_node.scheduler = std::make_unique<CScheduler>(); @@ -162,7 +164,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve m_cache_sizes = CalculateCacheSizes(m_args); - m_node.chainman = std::make_unique<ChainstateManager>(); + m_node.chainman = std::make_unique<ChainstateManager>(chainparams); m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<CBlockTreeDB>(m_cache_sizes.block_tree_db, true); // Start script-checking threads. Set g_parallel_script_checks to true so they are used. @@ -298,10 +300,9 @@ CBlock TestChain100Setup::CreateAndProcessBlock( chainstate = &Assert(m_node.chainman)->ActiveChainstate(); } - const CChainParams& chainparams = Params(); const CBlock block = this->CreateBlock(txns, scriptPubKey, *chainstate); std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(block); - Assert(m_node.chainman)->ProcessNewBlock(chainparams, shared_pblock, true, nullptr); + Assert(m_node.chainman)->ProcessNewBlock(shared_pblock, true, nullptr); return block; } diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 3b2aca5887..c110cd44aa 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -265,9 +265,6 @@ BOOST_AUTO_TEST_CASE(util_FormatParseISO8601DateTime) 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); - - auto time = GetTimeSeconds(); - BOOST_CHECK_EQUAL(ParseISO8601DateTime(FormatISO8601DateTime(time)), time); } BOOST_AUTO_TEST_CASE(util_FormatISO8601Date) @@ -1488,8 +1485,8 @@ BOOST_AUTO_TEST_CASE(util_time_GetTime) { SetMockTime(111); // Check that mock time does not change after a sleep - for (const auto& num_sleep : {0, 1}) { - UninterruptibleSleep(std::chrono::milliseconds{num_sleep}); + for (const auto& num_sleep : {0ms, 1ms}) { + UninterruptibleSleep(num_sleep); BOOST_CHECK_EQUAL(111, GetTime()); // Deprecated time getter BOOST_CHECK_EQUAL(111, GetTime<std::chrono::seconds>().count()); BOOST_CHECK_EQUAL(111000, GetTime<std::chrono::milliseconds>().count()); @@ -1497,10 +1494,14 @@ BOOST_AUTO_TEST_CASE(util_time_GetTime) } SetMockTime(0); - // Check that system time changes after a sleep + // Check that steady time and system time changes after a sleep + const auto steady_ms_0 = Now<SteadyMilliseconds>(); + const auto steady_0 = std::chrono::steady_clock::now(); const auto ms_0 = GetTime<std::chrono::milliseconds>(); const auto us_0 = GetTime<std::chrono::microseconds>(); - UninterruptibleSleep(std::chrono::milliseconds{1}); + UninterruptibleSleep(1ms); + BOOST_CHECK(steady_ms_0 < Now<SteadyMilliseconds>()); + BOOST_CHECK(steady_0 + 1ms <= std::chrono::steady_clock::now()); BOOST_CHECK(ms_0 < GetTime<std::chrono::milliseconds>()); BOOST_CHECK(us_0 < GetTime<std::chrono::microseconds>()); } @@ -2449,9 +2450,9 @@ struct Tracker //! Points to the original object (possibly itself) we moved/copied from const Tracker* origin; //! How many copies where involved between the original object and this one (moves are not counted) - int copies; + int copies{0}; - Tracker() noexcept : origin(this), copies(0) {} + Tracker() noexcept : origin(this) {} Tracker(const Tracker& t) noexcept : origin(t.origin), copies(t.copies + 1) {} Tracker(Tracker&& t) noexcept : origin(t.origin), copies(t.copies) {} Tracker& operator=(const Tracker& t) noexcept diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index c5b1dabcb7..18882929a8 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -89,7 +89,7 @@ std::shared_ptr<CBlock> MinerTestingSetup::Block(const uint256& prev_hash) std::shared_ptr<CBlock> MinerTestingSetup::FinalizeBlock(std::shared_ptr<CBlock> pblock) { const CBlockIndex* prev_block{WITH_LOCK(::cs_main, return m_node.chainman->m_blockman.LookupBlockIndex(pblock->hashPrevBlock))}; - GenerateCoinbaseCommitment(*pblock, prev_block, Params().GetConsensus()); + m_node.chainman->GenerateCoinbaseCommitment(*pblock, prev_block); pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); @@ -100,7 +100,7 @@ std::shared_ptr<CBlock> MinerTestingSetup::FinalizeBlock(std::shared_ptr<CBlock> // submit block header, so that miner can get the block height from the // global state and the node has the topology of the chain BlockValidationState ignored; - BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlockHeaders({pblock->GetBlockHeader()}, ignored, Params())); + BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlockHeaders({pblock->GetBlockHeader()}, ignored)); return pblock; } @@ -157,7 +157,7 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) bool ignored; // Connect the genesis block and drain any outstanding events - BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(Params(), std::make_shared<CBlock>(Params().GenesisBlock()), true, &ignored)); + BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(std::make_shared<CBlock>(Params().GenesisBlock()), true, &ignored)); SyncWithValidationInterfaceQueue(); // subscribe to events (this subscriber will validate event ordering) @@ -179,13 +179,13 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) FastRandomContext insecure; for (int i = 0; i < 1000; i++) { auto block = blocks[insecure.randrange(blocks.size() - 1)]; - Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, &ignored); + Assert(m_node.chainman)->ProcessNewBlock(block, true, &ignored); } // to make sure that eventually we process the full chain - do it here for (auto block : blocks) { if (block->vtx.size() == 1) { - bool processed = Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, &ignored); + bool processed = Assert(m_node.chainman)->ProcessNewBlock(block, true, &ignored); assert(processed); } } @@ -224,7 +224,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) { bool ignored; auto ProcessBlock = [&](std::shared_ptr<const CBlock> block) -> bool { - return Assert(m_node.chainman)->ProcessNewBlock(Params(), block, /*force_processing=*/true, /*new_block=*/&ignored); + return Assert(m_node.chainman)->ProcessNewBlock(block, /*force_processing=*/true, /*new_block=*/&ignored); }; // Process all mined blocks diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 2a3990bb7c..e7c7584f1c 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -22,7 +22,8 @@ BOOST_FIXTURE_TEST_SUITE(validation_chainstate_tests, TestingSetup) //! BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) { - ChainstateManager manager; + const CChainParams& chainparams = Params(); + ChainstateManager manager(chainparams); WITH_LOCK(::cs_main, manager.m_blockman.m_block_tree_db = std::make_unique<CBlockTreeDB>(1 << 20, true)); CTxMemPool mempool; diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp index bf87812a8a..129976ec15 100644 --- a/src/test/versionbits_tests.cpp +++ b/src/test/versionbits_tests.cpp @@ -5,9 +5,7 @@ #include <chain.h> #include <chainparams.h> #include <consensus/params.h> -#include <deploymentstatus.h> #include <test/util/setup_common.h> -#include <validation.h> #include <versionbits.h> #include <boost/test/unit_test.hpp> @@ -184,7 +182,7 @@ public: CBlockIndex* Tip() { return vpblock.empty() ? nullptr : vpblock.back(); } }; -BOOST_FIXTURE_TEST_SUITE(versionbits_tests, TestingSetup) +BOOST_FIXTURE_TEST_SUITE(versionbits_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(versionbits_test) { @@ -257,10 +255,10 @@ BOOST_AUTO_TEST_CASE(versionbits_test) } /** Check that ComputeBlockVersion will set the appropriate bit correctly */ -static void check_computeblockversion(const Consensus::Params& params, Consensus::DeploymentPos dep) +static void check_computeblockversion(VersionBitsCache& versionbitscache, const Consensus::Params& params, Consensus::DeploymentPos dep) { - // This implicitly uses g_versionbitscache, so clear it every time - g_versionbitscache.Clear(); + // Clear the cache everytime + versionbitscache.Clear(); int64_t bit = params.vDeployments[dep].bit; int64_t nStartTime = params.vDeployments[dep].nStartTime; @@ -268,7 +266,7 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus int min_activation_height = params.vDeployments[dep].min_activation_height; // should not be any signalling for first block - BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(nullptr, params), VERSIONBITS_TOP_BITS); + BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(nullptr, params), VERSIONBITS_TOP_BITS); // always/never active deployments shouldn't need to be tested further if (nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE || @@ -288,7 +286,7 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus // Check min_activation_height is on a retarget boundary BOOST_REQUIRE_EQUAL(min_activation_height % params.nMinerConfirmationWindow, 0U); - const uint32_t bitmask{g_versionbitscache.Mask(params, dep)}; + const uint32_t bitmask{versionbitscache.Mask(params, dep)}; BOOST_CHECK_EQUAL(bitmask, uint32_t{1} << bit); // In the first chain, test that the bit is set by CBV until it has failed. @@ -307,9 +305,9 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus // earlier time, so will transition from DEFINED to STARTED at the // end of the first period by mining blocks at nTime == 0 lastBlock = firstChain.Mine(params.nMinerConfirmationWindow - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); + BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); lastBlock = firstChain.Mine(params.nMinerConfirmationWindow, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); + BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); // then we'll keep mining at nStartTime... } else { // use a time 1s earlier than start time to check we stay DEFINED @@ -317,28 +315,28 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus // Start generating blocks before nStartTime lastBlock = firstChain.Mine(params.nMinerConfirmationWindow, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); + BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); // Mine more blocks (4 less than the adjustment period) at the old time, and check that CBV isn't setting the bit yet. for (uint32_t i = 1; i < params.nMinerConfirmationWindow - 4; i++) { lastBlock = firstChain.Mine(params.nMinerConfirmationWindow + i, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); + BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); } // Now mine 5 more blocks at the start time -- MTP should not have passed yet, so // CBV should still not yet set the bit. nTime = nStartTime; for (uint32_t i = params.nMinerConfirmationWindow - 4; i <= params.nMinerConfirmationWindow; i++) { lastBlock = firstChain.Mine(params.nMinerConfirmationWindow + i, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); + BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); } // Next we will advance to the next period and transition to STARTED, } lastBlock = firstChain.Mine(params.nMinerConfirmationWindow * 3, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); // so ComputeBlockVersion should now set the bit, - BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); + BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); // and should also be using the VERSIONBITS_TOP_BITS. - BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(lastBlock, params) & VERSIONBITS_TOP_MASK, VERSIONBITS_TOP_BITS); + BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & VERSIONBITS_TOP_MASK, VERSIONBITS_TOP_BITS); // Check that ComputeBlockVersion will set the bit until nTimeout nTime += 600; @@ -347,8 +345,8 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus // These blocks are all before nTimeout is reached. while (nTime < nTimeout && blocksToMine > 0) { lastBlock = firstChain.Mine(nHeight+1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); - BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(lastBlock, params) & VERSIONBITS_TOP_MASK, VERSIONBITS_TOP_BITS); + BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); + BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & VERSIONBITS_TOP_MASK, VERSIONBITS_TOP_BITS); blocksToMine--; nTime += 600; nHeight += 1; @@ -362,7 +360,7 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus // finish the last period before we start timing out while (nHeight % params.nMinerConfirmationWindow != 0) { lastBlock = firstChain.Mine(nHeight+1, nTime - 1, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); + BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); nHeight += 1; } @@ -370,12 +368,12 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus // the bit until the period transition. for (uint32_t i = 0; i < params.nMinerConfirmationWindow - 1; i++) { lastBlock = firstChain.Mine(nHeight+1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); + BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); nHeight += 1; } // The next block should trigger no longer setting the bit. lastBlock = firstChain.Mine(nHeight+1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); + BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); } // On a new chain: @@ -386,34 +384,36 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus // Mine one period worth of blocks, and check that the bit will be on for the // next period. lastBlock = secondChain.Mine(params.nMinerConfirmationWindow, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); + BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); // Mine another period worth of blocks, signaling the new bit. lastBlock = secondChain.Mine(params.nMinerConfirmationWindow * 2, nTime, VERSIONBITS_TOP_BITS | (1<<bit)).Tip(); // After one period of setting the bit on each block, it should have locked in. // We keep setting the bit for one more period though, until activation. - BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); + BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); // Now check that we keep mining the block until the end of this period, and // then stop at the beginning of the next period. lastBlock = secondChain.Mine((params.nMinerConfirmationWindow * 3) - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); + BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); lastBlock = secondChain.Mine(params.nMinerConfirmationWindow * 3, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); if (lastBlock->nHeight + 1 < min_activation_height) { // check signalling continues while min_activation_height is not reached lastBlock = secondChain.Mine(min_activation_height - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); + BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); // then reach min_activation_height, which was already REQUIRE'd to start a new period lastBlock = secondChain.Mine(min_activation_height, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); } // Check that we don't signal after activation - BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); + BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0); } BOOST_AUTO_TEST_CASE(versionbits_computeblockversion) { + VersionBitsCache vbcache; + // check that any deployment on any chain can conceivably reach both // ACTIVE and FAILED states in roughly the way we expect for (const auto& chain_name : {CBaseChainParams::MAIN, CBaseChainParams::TESTNET, CBaseChainParams::SIGNET, CBaseChainParams::REGTEST}) { @@ -426,10 +426,10 @@ BOOST_AUTO_TEST_CASE(versionbits_computeblockversion) // not take precedence over STARTED/LOCKED_IN. So all softforks on // the same bit might overlap, even when non-overlapping start-end // times are picked. - const uint32_t dep_mask{g_versionbitscache.Mask(chainParams->GetConsensus(), dep)}; + const uint32_t dep_mask{vbcache.Mask(chainParams->GetConsensus(), dep)}; BOOST_CHECK(!(chain_all_vbits & dep_mask)); chain_all_vbits |= dep_mask; - check_computeblockversion(chainParams->GetConsensus(), dep); + check_computeblockversion(vbcache, chainParams->GetConsensus(), dep); } } @@ -439,7 +439,7 @@ BOOST_AUTO_TEST_CASE(versionbits_computeblockversion) ArgsManager args; args.ForceSetArg("-vbparams", "testdummy:1199145601:1230767999"); // January 1, 2008 - December 31, 2008 const auto chainParams = CreateChainParams(args, CBaseChainParams::REGTEST); - check_computeblockversion(chainParams->GetConsensus(), Consensus::DEPLOYMENT_TESTDUMMY); + check_computeblockversion(vbcache, chainParams->GetConsensus(), Consensus::DEPLOYMENT_TESTDUMMY); } { @@ -449,7 +449,7 @@ BOOST_AUTO_TEST_CASE(versionbits_computeblockversion) ArgsManager args; args.ForceSetArg("-vbparams", "testdummy:1199145601:1230767999:403200"); // January 1, 2008 - December 31, 2008, min act height 403200 const auto chainParams = CreateChainParams(args, CBaseChainParams::REGTEST); - check_computeblockversion(chainParams->GetConsensus(), Consensus::DEPLOYMENT_TESTDUMMY); + check_computeblockversion(vbcache, chainParams->GetConsensus(), Consensus::DEPLOYMENT_TESTDUMMY); } } diff --git a/src/threadinterrupt.h b/src/threadinterrupt.h index cb9a5fbf8b..992016b4f6 100644 --- a/src/threadinterrupt.h +++ b/src/threadinterrupt.h @@ -21,11 +21,11 @@ class CThreadInterrupt public: CThreadInterrupt(); explicit operator bool() const; - void operator()(); + void operator()() EXCLUSIVE_LOCKS_REQUIRED(!mut); void reset(); - bool sleep_for(std::chrono::milliseconds rel_time); - bool sleep_for(std::chrono::seconds rel_time); - bool sleep_for(std::chrono::minutes rel_time); + bool sleep_for(std::chrono::milliseconds rel_time) EXCLUSIVE_LOCKS_REQUIRED(!mut); + bool sleep_for(std::chrono::seconds rel_time) EXCLUSIVE_LOCKS_REQUIRED(!mut); + bool sleep_for(std::chrono::minutes rel_time) EXCLUSIVE_LOCKS_REQUIRED(!mut); private: std::condition_variable cond; diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 05dbc6057f..2064f0fb8a 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -304,8 +304,7 @@ std::map<std::string,std::string> ParseTorReplyMapping(const std::string &s) TorController::TorController(struct event_base* _base, const std::string& tor_control_center, const CService& target): base(_base), - m_tor_control_center(tor_control_center), conn(base), reconnect(true), reconnect_ev(nullptr), - reconnect_timeout(RECONNECT_TIMEOUT_START), + m_tor_control_center(tor_control_center), conn(base), reconnect(true), reconnect_timeout(RECONNECT_TIMEOUT_START), m_target(target) { reconnect_ev = event_new(base, -1, 0, reconnect_cb, this); diff --git a/src/txdb.cpp b/src/txdb.cpp index afcd1985f5..a0939873ad 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -207,7 +207,7 @@ public: // cache warmup on instantiation. CCoinsViewDBCursor(CDBIterator* pcursorIn, const uint256&hashBlockIn): CCoinsViewCursor(hashBlockIn), pcursor(pcursorIn) {} - ~CCoinsViewDBCursor() {} + ~CCoinsViewDBCursor() = default; bool GetKey(COutPoint &key) const override; bool GetValue(Coin &coin) const override; diff --git a/src/univalue/README.md b/src/univalue/README.md index 7c62c33970..d622f5b1e0 100644 --- a/src/univalue/README.md +++ b/src/univalue/README.md @@ -15,7 +15,7 @@ This class is aligned with the JSON standard, [RFC ## Library usage This is a fork of univalue used by Bitcoin Core. It is not maintained for usage -by other projects. Notably, the API may break in non-backward-compatible ways. +by other projects. Notably, the API is broken in non-backward-compatible ways. Other projects looking for a maintained library should use the upstream univalue at https://github.com/jgarzik/univalue. diff --git a/src/univalue/configure.ac b/src/univalue/configure.ac index 495b25a53d..ed9c5f0c5c 100644 --- a/src/univalue/configure.ac +++ b/src/univalue/configure.ac @@ -45,8 +45,8 @@ AC_SUBST(LIBUNIVALUE_AGE) LT_INIT LT_LANG([C++]) -dnl Require C++11 compiler (no GNU extensions) -AX_CXX_COMPILE_STDCXX([11], [noext], [mandatory], [nodefault]) +dnl Require C++17 compiler (no GNU extensions) +AX_CXX_COMPILE_STDCXX([17], [noext], [mandatory], [nodefault]) case $host in *mingw*) diff --git a/src/univalue/gen/gen.cpp b/src/univalue/gen/gen.cpp index b8a6c73f4e..ca5b470ddc 100644 --- a/src/univalue/gen/gen.cpp +++ b/src/univalue/gen/gen.cpp @@ -8,9 +8,11 @@ // $ ./gen > univalue_escapes.h // -#include <stdio.h> -#include <string.h> -#include "univalue.h" +#include <univalue.h> + +#include <cstdio> +#include <cstring> +#include <string> static bool initEscapes; static std::string escapes[256]; diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h index fc5cf402be..35eaa2dd0d 100644 --- a/src/univalue/include/univalue.h +++ b/src/univalue/include/univalue.h @@ -6,13 +6,14 @@ #ifndef __UNIVALUE_H__ #define __UNIVALUE_H__ -#include <stdint.h> -#include <string.h> - +#include <charconv> +#include <cstdint> +#include <cstring> +#include <map> +#include <stdexcept> #include <string> +#include <type_traits> #include <vector> -#include <map> -#include <cassert> class UniValue { public: @@ -168,10 +169,24 @@ public: // value is of unexpected type const std::vector<std::string>& getKeys() const; const std::vector<UniValue>& getValues() const; + template <typename Int> + auto getInt() const + { + static_assert(std::is_integral<Int>::value); + if (typ != VNUM) { + throw std::runtime_error("JSON value is not an integer as expected"); + } + Int result; + const auto [first_nonmatching, error_condition] = std::from_chars(val.data(), val.data() + val.size(), result); + if (first_nonmatching != val.data() + val.size() || error_condition != std::errc{}) { + throw std::runtime_error("JSON integer out of range"); + } + return result; + } bool get_bool() const; const std::string& get_str() const; - int get_int() const; - int64_t get_int64() const; + auto get_int() const { return getInt<int>(); }; + auto get_int64() const { return getInt<int64_t>(); }; double get_real() const; const UniValue& get_obj() const; const UniValue& get_array() const; diff --git a/src/univalue/lib/univalue.cpp b/src/univalue/lib/univalue.cpp index c4e59fae74..3553995c28 100644 --- a/src/univalue/lib/univalue.cpp +++ b/src/univalue/lib/univalue.cpp @@ -3,12 +3,15 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://opensource.org/licenses/mit-license.php. -#include <stdint.h> +#include <univalue.h> + #include <iomanip> +#include <map> +#include <memory> #include <sstream> -#include <stdlib.h> - -#include "univalue.h" +#include <string> +#include <utility> +#include <vector> const UniValue NullUniValue; diff --git a/src/univalue/lib/univalue_get.cpp b/src/univalue/lib/univalue_get.cpp index 5af89a3561..9bbdb1fe69 100644 --- a/src/univalue/lib/univalue_get.cpp +++ b/src/univalue/lib/univalue_get.cpp @@ -3,17 +3,18 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://opensource.org/licenses/mit-license.php. -#include <stdint.h> -#include <errno.h> -#include <string.h> -#include <stdlib.h> -#include <stdexcept> -#include <vector> +#include <univalue.h> + +#include <cerrno> +#include <cstdint> +#include <cstdlib> +#include <cstring> #include <limits> -#include <string> +#include <locale> #include <sstream> - -#include "univalue.h" +#include <stdexcept> +#include <string> +#include <vector> namespace { @@ -28,37 +29,6 @@ static bool ParsePrechecks(const std::string& str) return true; } -bool ParseInt32(const std::string& str, int32_t *out) -{ - if (!ParsePrechecks(str)) - return false; - char *endp = nullptr; - errno = 0; // strtol will not set errno if valid - long int n = strtol(str.c_str(), &endp, 10); - if(out) *out = (int32_t)n; - // Note that strtol returns a *long int*, so even if strtol doesn't report an over/underflow - // we still have to check that the returned value is within the range of an *int32_t*. On 64-bit - // platforms the size of these types may be different. - return endp && *endp == 0 && !errno && - n >= std::numeric_limits<int32_t>::min() && - n <= std::numeric_limits<int32_t>::max(); -} - -bool ParseInt64(const std::string& str, int64_t *out) -{ - if (!ParsePrechecks(str)) - return false; - char *endp = nullptr; - errno = 0; // strtoll will not set errno if valid - long long int n = strtoll(str.c_str(), &endp, 10); - if(out) *out = (int64_t)n; - // Note that strtoll returns a *long long int*, so even if strtol doesn't report a over/underflow - // we still have to check that the returned value is within the range of an *int64_t*. - return endp && *endp == 0 && !errno && - n >= std::numeric_limits<int64_t>::min() && - n <= std::numeric_limits<int64_t>::max(); -} - bool ParseDouble(const std::string& str, double *out) { if (!ParsePrechecks(str)) @@ -102,26 +72,6 @@ const std::string& UniValue::get_str() const return getValStr(); } -int UniValue::get_int() const -{ - if (typ != VNUM) - throw std::runtime_error("JSON value is not an integer as expected"); - int32_t retval; - if (!ParseInt32(getValStr(), &retval)) - throw std::runtime_error("JSON integer out of range"); - return retval; -} - -int64_t UniValue::get_int64() const -{ - if (typ != VNUM) - throw std::runtime_error("JSON value is not an integer as expected"); - int64_t retval; - if (!ParseInt64(getValStr(), &retval)) - throw std::runtime_error("JSON integer out of range"); - return retval; -} - double UniValue::get_real() const { if (typ != VNUM) diff --git a/src/univalue/lib/univalue_read.cpp b/src/univalue/lib/univalue_read.cpp index be39bfe57a..a6ed75e57a 100644 --- a/src/univalue/lib/univalue_read.cpp +++ b/src/univalue/lib/univalue_read.cpp @@ -2,19 +2,22 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://opensource.org/licenses/mit-license.php. -#include <string.h> -#include <vector> -#include <stdio.h> -#include "univalue.h" +#include <univalue.h> #include "univalue_utffilter.h" +#include <cstdio> +#include <cstdint> +#include <cstring> +#include <string> +#include <vector> + /* * According to stackexchange, the original json test suite wanted * to limit depth to 22. Widely-deployed PHP bails at depth 512, * so we will follow PHP's lead, which should be more than sufficient * (further stackexchange comments indicate depth > 32 rarely occurs). */ -static const size_t MAX_JSON_DEPTH = 512; +static constexpr size_t MAX_JSON_DEPTH = 512; static bool json_isdigit(int ch) { diff --git a/src/univalue/lib/univalue_write.cpp b/src/univalue/lib/univalue_write.cpp index 3a2c580c7f..18833077b7 100644 --- a/src/univalue/lib/univalue_write.cpp +++ b/src/univalue/lib/univalue_write.cpp @@ -2,11 +2,13 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://opensource.org/licenses/mit-license.php. -#include <iomanip> -#include <stdio.h> -#include "univalue.h" +#include <univalue.h> #include "univalue_escapes.h" +#include <memory> +#include <string> +#include <vector> + static std::string json_escape(const std::string& inS) { std::string outS; diff --git a/src/univalue/test/no_nul.cpp b/src/univalue/test/no_nul.cpp index 83d292200b..3a7a727abb 100644 --- a/src/univalue/test/no_nul.cpp +++ b/src/univalue/test/no_nul.cpp @@ -1,4 +1,4 @@ -#include "univalue.h" +#include <univalue.h> int main (int argc, char *argv[]) { diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp index c2f52f83ac..b9697a8cb7 100644 --- a/src/univalue/test/object.cpp +++ b/src/univalue/test/object.cpp @@ -3,13 +3,15 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://opensource.org/licenses/mit-license.php. -#include <stdint.h> -#include <vector> -#include <string> -#include <map> +#include <univalue.h> + #include <cassert> +#include <cstdint> +#include <map> +#include <memory> #include <stdexcept> -#include <univalue.h> +#include <string> +#include <vector> #define BOOST_FIXTURE_TEST_SUITE(a, b) #define BOOST_AUTO_TEST_CASE(funcName) void funcName() diff --git a/src/univalue/test/test_json.cpp b/src/univalue/test/test_json.cpp index 2943bae2b1..f8c10238d4 100644 --- a/src/univalue/test/test_json.cpp +++ b/src/univalue/test/test_json.cpp @@ -4,9 +4,11 @@ // It reads JSON input from stdin and exits with code 0 if it can be parsed // successfully. It also pretty prints the parsed JSON value to stdout. +#include <univalue.h> + #include <iostream> +#include <iterator> #include <string> -#include "univalue.h" using namespace std; diff --git a/src/univalue/test/unitester.cpp b/src/univalue/test/unitester.cpp index 02e1a83c6d..81b1c5d3b1 100644 --- a/src/univalue/test/unitester.cpp +++ b/src/univalue/test/unitester.cpp @@ -2,12 +2,11 @@ // Distributed under the MIT/X11 software license, see the accompanying // file COPYING or https://opensource.org/licenses/mit-license.php. -#include <stdlib.h> -#include <stdio.h> -#include <string.h> +#include <univalue.h> + #include <cassert> +#include <cstdio> #include <string> -#include "univalue.h" #ifndef JSON_TEST_SRC #error JSON_TEST_SRC must point to test source directory diff --git a/src/util/bytevectorhash.cpp b/src/util/bytevectorhash.cpp index bc060a44c9..9054db4759 100644 --- a/src/util/bytevectorhash.cpp +++ b/src/util/bytevectorhash.cpp @@ -6,10 +6,10 @@ #include <random.h> #include <util/bytevectorhash.h> -ByteVectorHash::ByteVectorHash() +ByteVectorHash::ByteVectorHash() : + m_k0(GetRand<uint64_t>()), + m_k1(GetRand<uint64_t>()) { - GetRandBytes({reinterpret_cast<unsigned char*>(&m_k0), sizeof(m_k0)}); - GetRandBytes({reinterpret_cast<unsigned char*>(&m_k1), sizeof(m_k1)}); } size_t ByteVectorHash::operator()(const std::vector<unsigned char>& input) const diff --git a/src/util/hasher.cpp b/src/util/hasher.cpp index 5900daf050..c21941eb88 100644 --- a/src/util/hasher.cpp +++ b/src/util/hasher.cpp @@ -7,11 +7,11 @@ #include <limits> -SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits<uint64_t>::max())), k1(GetRand(std::numeric_limits<uint64_t>::max())) {} +SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand<uint64_t>()), k1(GetRand<uint64_t>()) {} -SaltedOutpointHasher::SaltedOutpointHasher() : k0(GetRand(std::numeric_limits<uint64_t>::max())), k1(GetRand(std::numeric_limits<uint64_t>::max())) {} +SaltedOutpointHasher::SaltedOutpointHasher() : k0(GetRand<uint64_t>()), k1(GetRand<uint64_t>()) {} -SaltedSipHasher::SaltedSipHasher() : m_k0(GetRand(std::numeric_limits<uint64_t>::max())), m_k1(GetRand(std::numeric_limits<uint64_t>::max())) {} +SaltedSipHasher::SaltedSipHasher() : m_k0(GetRand<uint64_t>()), m_k1(GetRand<uint64_t>()) {} size_t SaltedSipHasher::operator()(const Span<const unsigned char>& script) const { diff --git a/src/util/system.cpp b/src/util/system.cpp index 44ebf5cb3e..7697991c4b 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -258,8 +258,8 @@ static std::optional<util::SettingsValue> InterpretValue(const KeyInfo& key, con // Define default constructor and destructor that are not inline, so code instantiating this class doesn't need to // #include class definitions for all members. // For example, m_settings has an internal dependency on univalue. -ArgsManager::ArgsManager() {} -ArgsManager::~ArgsManager() {} +ArgsManager::ArgsManager() = default; +ArgsManager::~ArgsManager() = default; const std::set<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const { diff --git a/src/util/time.cpp b/src/util/time.cpp index e428430bac..4ec44509ab 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -115,11 +115,6 @@ int64_t GetTimeMicros() return int64_t{GetSystemTime<std::chrono::microseconds>().count()}; } -int64_t GetTimeSeconds() -{ - return int64_t{GetSystemTime<std::chrono::seconds>().count()}; -} - int64_t GetTime() { return GetTime<std::chrono::seconds>().count(); } std::string FormatISO8601DateTime(int64_t nTime) { diff --git a/src/util/time.h b/src/util/time.h index 9d92b23725..72956ea0d7 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -14,18 +14,27 @@ using namespace std::chrono_literals; +using SteadySeconds = std::chrono::time_point<std::chrono::steady_clock, std::chrono::seconds>; +using SteadyMilliseconds = std::chrono::time_point<std::chrono::steady_clock, std::chrono::milliseconds>; +using SteadyMicroseconds = std::chrono::time_point<std::chrono::steady_clock, std::chrono::microseconds>; + void UninterruptibleSleep(const std::chrono::microseconds& n); /** - * Helper to count the seconds of a duration. + * Helper to count the seconds of a duration/time_point. * - * All durations should be using std::chrono and calling this should generally + * All durations/time_points should be using std::chrono and calling this should generally * be avoided in code. Though, it is still preferred to an inline t.count() to * protect against a reliance on the exact type of t. * - * This helper is used to convert durations before passing them over an + * This helper is used to convert durations/time_points before passing them over an * interface that doesn't support std::chrono (e.g. RPC, debug log, or the GUI) */ +template <typename Clock> +constexpr int64_t count_seconds(std::chrono::time_point<Clock, std::chrono::seconds> t) +{ + return t.time_since_epoch().count(); +} constexpr int64_t count_seconds(std::chrono::seconds t) { return t.count(); } constexpr int64_t count_milliseconds(std::chrono::milliseconds t) { return t.count(); } constexpr int64_t count_microseconds(std::chrono::microseconds t) { return t.count(); } @@ -47,8 +56,6 @@ int64_t GetTime(); int64_t GetTimeMillis(); /** Returns the system time (not mockable) */ int64_t GetTimeMicros(); -/** Returns the system time (not mockable) */ -int64_t GetTimeSeconds(); // Like GetTime(), but not mockable /** * DEPRECATED @@ -67,6 +74,15 @@ std::chrono::seconds GetMockTime(); /** Return system time (or mocked time, if set) */ template <typename T> T GetTime(); +/** + * Return the current time point cast to the given precicion. Only use this + * when an exact precicion is needed, otherwise use T::clock::now() directly. + */ +template <typename T> +T Now() +{ + return std::chrono::time_point_cast<typename T::duration>(T::clock::now()); +} /** * ISO 8601 formatting is preferred. Use the FormatISO8601{DateTime,Date} diff --git a/src/validation.cpp b/src/validation.cpp index b5d6a66088..a7cdf63a2a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -16,7 +16,6 @@ #include <consensus/tx_verify.h> #include <consensus/validation.h> #include <cuckoocache.h> -#include <deploymentstatus.h> #include <flatfile.h> #include <hash.h> #include <logging.h> @@ -262,7 +261,7 @@ bool CheckSequenceLocksAtTip(CBlockIndex* tip, } // Returns the script flags which should be checked for a given block -static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const Consensus::Params& chainparams); +static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const ChainstateManager& chainman); static void LimitMempoolSize(CTxMemPool& pool, CCoinsViewCache& coins_cache, size_t limit, std::chrono::seconds age) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs) @@ -1027,7 +1026,6 @@ bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws) const CTransaction& tx = *ws.m_ptx; const uint256& hash = ws.m_hash; TxValidationState& state = ws.m_state; - const CChainParams& chainparams = args.m_chainparams; // Check again against the current block tip's script verification // flags to cache our script execution flags. This is, of course, @@ -1044,7 +1042,7 @@ bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws) // There is a similar check in CreateNewBlock() to prevent creating // invalid blocks (using TestBlockValidity), however allowing such // transactions into the mempool can be exploited as a DoS attack. - unsigned int currentBlockScriptVerifyFlags{GetBlockScriptFlags(*m_active_chainstate.m_chain.Tip(), chainparams.GetConsensus())}; + unsigned int currentBlockScriptVerifyFlags{GetBlockScriptFlags(*m_active_chainstate.m_chain.Tip(), m_active_chainstate.m_chainman)}; if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags, ws.m_precomputed_txdata, m_active_chainstate.CoinsTip())) { LogPrintf("BUG! PLEASE REPORT THIS! CheckInputScripts failed against latest-block but not STANDARD flags %s, %s\n", hash.ToString(), state.ToString()); @@ -1457,7 +1455,7 @@ PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTx assert(std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx != nullptr;})); std::vector<COutPoint> coins_to_uncache; - const CChainParams& chainparams = Params(); + const CChainParams& chainparams = active_chainstate.m_params; const auto result = [&]() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); if (test_accept) { @@ -1515,7 +1513,7 @@ CChainState::CChainState( std::optional<uint256> from_snapshot_blockhash) : m_mempool(mempool), m_blockman(blockman), - m_params(::Params()), + m_params(chainman.GetParams()), m_chainman(chainman), m_from_snapshot_blockhash(from_snapshot_blockhash) {} @@ -1909,10 +1907,11 @@ void StopScriptCheckWorkerThreads() class WarningBitsConditionChecker : public AbstractThresholdConditionChecker { private: - int bit; + const ChainstateManager& m_chainman; + int m_bit; public: - explicit WarningBitsConditionChecker(int bitIn) : bit(bitIn) {} + explicit WarningBitsConditionChecker(const ChainstateManager& chainman, int bit) : m_chainman{chainman}, m_bit(bit) {} int64_t BeginTime(const Consensus::Params& params) const override { return 0; } int64_t EndTime(const Consensus::Params& params) const override { return std::numeric_limits<int64_t>::max(); } @@ -1923,15 +1922,17 @@ public: { return pindex->nHeight >= params.MinBIP9WarningHeight && ((pindex->nVersion & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS) && - ((pindex->nVersion >> bit) & 1) != 0 && - ((g_versionbitscache.ComputeBlockVersion(pindex->pprev, params) >> bit) & 1) == 0; + ((pindex->nVersion >> m_bit) & 1) != 0 && + ((m_chainman.m_versionbitscache.ComputeBlockVersion(pindex->pprev, params) >> m_bit) & 1) == 0; } }; static std::array<ThresholdConditionCache, VERSIONBITS_NUM_BITS> warningcache GUARDED_BY(cs_main); -static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const Consensus::Params& consensusparams) +static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const ChainstateManager& chainman) { + const Consensus::Params& consensusparams = chainman.GetConsensus(); + // BIP16 didn't become active until Apr 1 2012 (on mainnet, and // retroactively applied to testnet) // However, only one historical block violated the P2SH rules (on both @@ -1947,22 +1948,22 @@ static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const Co } // Enforce the DERSIG (BIP66) rule - if (DeploymentActiveAt(block_index, consensusparams, Consensus::DEPLOYMENT_DERSIG)) { + if (DeploymentActiveAt(block_index, chainman, Consensus::DEPLOYMENT_DERSIG)) { flags |= SCRIPT_VERIFY_DERSIG; } // Enforce CHECKLOCKTIMEVERIFY (BIP65) - if (DeploymentActiveAt(block_index, consensusparams, Consensus::DEPLOYMENT_CLTV)) { + if (DeploymentActiveAt(block_index, chainman, Consensus::DEPLOYMENT_CLTV)) { flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY; } // Enforce CHECKSEQUENCEVERIFY (BIP112) - if (DeploymentActiveAt(block_index, consensusparams, Consensus::DEPLOYMENT_CSV)) { + if (DeploymentActiveAt(block_index, chainman, Consensus::DEPLOYMENT_CSV)) { flags |= SCRIPT_VERIFY_CHECKSEQUENCEVERIFY; } // Enforce BIP147 NULLDUMMY (activated simultaneously with segwit) - if (DeploymentActiveAt(block_index, consensusparams, Consensus::DEPLOYMENT_SEGWIT)) { + if (DeploymentActiveAt(block_index, chainman, Consensus::DEPLOYMENT_SEGWIT)) { flags |= SCRIPT_VERIFY_NULLDUMMY; } @@ -2153,12 +2154,12 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, // Enforce BIP68 (sequence locks) int nLockTimeFlags = 0; - if (DeploymentActiveAt(*pindex, m_params.GetConsensus(), Consensus::DEPLOYMENT_CSV)) { + if (DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_CSV)) { nLockTimeFlags |= LOCKTIME_VERIFY_SEQUENCE; } // Get the script flags for this block - unsigned int flags{GetBlockScriptFlags(*pindex, m_params.GetConsensus())}; + unsigned int flags{GetBlockScriptFlags(*pindex, m_chainman)}; int64_t nTime2 = GetTimeMicros(); nTimeForks += nTime2 - nTime1; LogPrint(BCLog::BENCH, " - Fork checks: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime2 - nTime1), nTimeForks * MICRO, nTimeForks * MILLI / nBlocksTotal); @@ -2454,9 +2455,9 @@ bool CChainState::FlushStateToDisk( full_flush_completed = true; TRACE5(utxocache, flush, (int64_t)(GetTimeMicros() - nNow.count()), // in microseconds (µs) - (u_int32_t)mode, - (u_int64_t)coins_count, - (u_int64_t)coins_mem_usage, + (uint32_t)mode, + (uint64_t)coins_count, + (uint64_t)coins_mem_usage, (bool)fFlushForPrune); } } @@ -2556,7 +2557,7 @@ void CChainState::UpdateTip(const CBlockIndex* pindexNew) if (!this->IsInitialBlockDownload()) { const CBlockIndex* pindex = pindexNew; for (int bit = 0; bit < VERSIONBITS_NUM_BITS; bit++) { - WarningBitsConditionChecker checker(bit); + WarningBitsConditionChecker checker(m_chainman, bit); ThresholdState state = checker.GetStateFor(pindex, m_params.GetConsensus(), warningcache.at(bit)); if (state == ThresholdState::ACTIVE || state == ThresholdState::LOCKED_IN) { const bilingual_str warning = strprintf(_("Unknown new rules activated (versionbit %i)"), bit); @@ -2653,7 +2654,7 @@ static int64_t nTimePostConnect = 0; struct PerBlockConnectTrace { CBlockIndex* pindex = nullptr; std::shared_ptr<const CBlock> pblock; - PerBlockConnectTrace() {} + PerBlockConnectTrace() = default; }; /** * Used to track blocks whose transactions were applied to the UTXO state as a @@ -3280,7 +3281,7 @@ void CChainState::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pi pindexNew->nDataPos = pos.nPos; pindexNew->nUndoPos = 0; pindexNew->nStatus |= BLOCK_HAVE_DATA; - if (DeploymentActiveAt(*pindexNew, m_params.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) { + if (DeploymentActiveAt(*pindexNew, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) { pindexNew->nStatus |= BLOCK_OPT_WITNESS; } pindexNew->RaiseValidity(BLOCK_VALID_TRANSACTIONS); @@ -3398,11 +3399,11 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu return true; } -void UpdateUncommittedBlockStructures(CBlock& block, const CBlockIndex* pindexPrev, const Consensus::Params& consensusParams) +void ChainstateManager::UpdateUncommittedBlockStructures(CBlock& block, const CBlockIndex* pindexPrev) const { int commitpos = GetWitnessCommitmentIndex(block); static const std::vector<unsigned char> nonce(32, 0x00); - if (commitpos != NO_WITNESS_COMMITMENT && DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_SEGWIT) && !block.vtx[0]->HasWitness()) { + if (commitpos != NO_WITNESS_COMMITMENT && DeploymentActiveAfter(pindexPrev, *this, Consensus::DEPLOYMENT_SEGWIT) && !block.vtx[0]->HasWitness()) { CMutableTransaction tx(*block.vtx[0]); tx.vin[0].scriptWitness.stack.resize(1); tx.vin[0].scriptWitness.stack[0] = nonce; @@ -3410,7 +3411,7 @@ void UpdateUncommittedBlockStructures(CBlock& block, const CBlockIndex* pindexPr } } -std::vector<unsigned char> GenerateCoinbaseCommitment(CBlock& block, const CBlockIndex* pindexPrev, const Consensus::Params& consensusParams) +std::vector<unsigned char> ChainstateManager::GenerateCoinbaseCommitment(CBlock& block, const CBlockIndex* pindexPrev) const { std::vector<unsigned char> commitment; int commitpos = GetWitnessCommitmentIndex(block); @@ -3433,7 +3434,7 @@ std::vector<unsigned char> GenerateCoinbaseCommitment(CBlock& block, const CBloc tx.vout.push_back(out); block.vtx[0] = MakeTransactionRef(std::move(tx)); } - UpdateUncommittedBlockStructures(block, pindexPrev, consensusParams); + UpdateUncommittedBlockStructures(block, pindexPrev); return commitment; } @@ -3446,14 +3447,14 @@ std::vector<unsigned char> GenerateCoinbaseCommitment(CBlock& block, const CBloc * in ConnectBlock(). * Note that -reindex-chainstate skips the validation that happens here! */ -static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) +static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const ChainstateManager& chainman, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); assert(pindexPrev != nullptr); const int nHeight = pindexPrev->nHeight + 1; // Check proof of work - const Consensus::Params& consensusParams = params.GetConsensus(); + const Consensus::Params& consensusParams = chainman.GetConsensus(); if (block.nBits != GetNextWorkRequired(pindexPrev, &block, consensusParams)) return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "bad-diffbits", "incorrect proof of work"); @@ -3462,7 +3463,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio // Don't accept any forks from the main chain prior to last checkpoint. // GetLastCheckpoint finds the last checkpoint in MapCheckpoints that's in our // BlockIndex(). - const CBlockIndex* pcheckpoint = blockman.GetLastCheckpoint(params.Checkpoints()); + const CBlockIndex* pcheckpoint = blockman.GetLastCheckpoint(chainman.GetParams().Checkpoints()); if (pcheckpoint && nHeight < pcheckpoint->nHeight) { LogPrintf("ERROR: %s: forked chain older than last checkpoint (height %d)\n", __func__, nHeight); return state.Invalid(BlockValidationResult::BLOCK_CHECKPOINT, "bad-fork-prior-to-checkpoint"); @@ -3478,9 +3479,9 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio return state.Invalid(BlockValidationResult::BLOCK_TIME_FUTURE, "time-too-new", "block timestamp too far in the future"); // Reject blocks with outdated version - if ((block.nVersion < 2 && DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_HEIGHTINCB)) || - (block.nVersion < 3 && DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_DERSIG)) || - (block.nVersion < 4 && DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_CLTV))) { + if ((block.nVersion < 2 && DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_HEIGHTINCB)) || + (block.nVersion < 3 && DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_DERSIG)) || + (block.nVersion < 4 && DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_CLTV))) { return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, strprintf("bad-version(0x%08x)", block.nVersion), strprintf("rejected nVersion=0x%08x block", block.nVersion)); } @@ -3494,13 +3495,13 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio * in ConnectBlock(). * Note that -reindex-chainstate skips the validation that happens here! */ -static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev) +static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& state, const ChainstateManager& chainman, const CBlockIndex* pindexPrev) { const int nHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1; // Enforce BIP113 (Median Time Past). int nLockTimeFlags = 0; - if (DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_CSV)) { + if (DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_CSV)) { assert(pindexPrev != nullptr); nLockTimeFlags |= LOCKTIME_MEDIAN_TIME_PAST; } @@ -3517,7 +3518,7 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat } // Enforce rule that the coinbase starts with serialized block height - if (DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_HEIGHTINCB)) + if (DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_HEIGHTINCB)) { CScript expect = CScript() << nHeight; if (block.vtx[0]->vin[0].scriptSig.size() < expect.size() || @@ -3535,7 +3536,7 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat // {0xaa, 0x21, 0xa9, 0xed}, and the following 32 bytes are SHA256^2(witness root, witness reserved value). In case there are // multiple, the last one is used. bool fHaveWitness = false; - if (DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_SEGWIT)) { + if (DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_SEGWIT)) { int commitpos = GetWitnessCommitmentIndex(block); if (commitpos != NO_WITNESS_COMMITMENT) { bool malleated = false; @@ -3576,13 +3577,13 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat return true; } -bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) +bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationState& state, CBlockIndex** ppindex) { AssertLockHeld(cs_main); // Check for duplicate uint256 hash = block.GetHash(); BlockMap::iterator miSelf{m_blockman.m_block_index.find(hash)}; - if (hash != chainparams.GetConsensus().hashGenesisBlock) { + if (hash != GetConsensus().hashGenesisBlock) { if (miSelf != m_blockman.m_block_index.end()) { // Block header is already known. CBlockIndex* pindex = &(miSelf->second); @@ -3595,7 +3596,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida return true; } - if (!CheckBlockHeader(block, state, chainparams.GetConsensus())) { + if (!CheckBlockHeader(block, state, GetConsensus())) { LogPrint(BCLog::VALIDATION, "%s: Consensus::CheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString()); return false; } @@ -3612,7 +3613,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida LogPrint(BCLog::VALIDATION, "%s: %s prev block invalid\n", __func__, hash.ToString()); return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk"); } - if (!ContextualCheckBlockHeader(block, state, m_blockman, chainparams, pindexPrev, GetAdjustedTime())) { + if (!ContextualCheckBlockHeader(block, state, m_blockman, *this, pindexPrev, GetAdjustedTime())) { LogPrint(BCLog::VALIDATION, "%s: Consensus::ContextualCheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString()); return false; } @@ -3665,14 +3666,14 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida } // Exposed wrapper for AcceptBlockHeader -bool ChainstateManager::ProcessNewBlockHeaders(const std::vector<CBlockHeader>& headers, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex) +bool ChainstateManager::ProcessNewBlockHeaders(const std::vector<CBlockHeader>& headers, BlockValidationState& state, const CBlockIndex** ppindex) { AssertLockNotHeld(cs_main); { LOCK(cs_main); for (const CBlockHeader& header : headers) { CBlockIndex *pindex = nullptr; // Use a temp pindex instead of ppindex to avoid a const_cast - bool accepted{AcceptBlockHeader(header, state, chainparams, &pindex)}; + bool accepted{AcceptBlockHeader(header, state, &pindex)}; ActiveChainstate().CheckBlockIndex(); if (!accepted) { @@ -3686,7 +3687,7 @@ bool ChainstateManager::ProcessNewBlockHeaders(const std::vector<CBlockHeader>& if (NotifyHeaderTip(ActiveChainstate())) { if (ActiveChainstate().IsInitialBlockDownload() && ppindex && *ppindex) { const CBlockIndex& last_accepted{**ppindex}; - const int64_t blocks_left{(GetTime() - last_accepted.GetBlockTime()) / chainparams.GetConsensus().nPowTargetSpacing}; + const int64_t blocks_left{(GetTime() - last_accepted.GetBlockTime()) / GetConsensus().nPowTargetSpacing}; const double progress{100.0 * last_accepted.nHeight / (last_accepted.nHeight + blocks_left)}; LogPrintf("Synchronizing blockheaders, height: %d (~%.2f%%)\n", last_accepted.nHeight, progress); } @@ -3705,7 +3706,7 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, Block CBlockIndex *pindexDummy = nullptr; CBlockIndex *&pindex = ppindex ? *ppindex : pindexDummy; - bool accepted_header{m_chainman.AcceptBlockHeader(block, state, m_params, &pindex)}; + bool accepted_header{m_chainman.AcceptBlockHeader(block, state, &pindex)}; CheckBlockIndex(); if (!accepted_header) @@ -3745,7 +3746,7 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, Block } if (!CheckBlock(block, state, m_params.GetConsensus()) || - !ContextualCheckBlock(block, state, m_params.GetConsensus(), pindex->pprev)) { + !ContextualCheckBlock(block, state, m_chainman, pindex->pprev)) { if (state.IsInvalid() && state.GetResult() != BlockValidationResult::BLOCK_MUTATED) { pindex->nStatus |= BLOCK_FAILED_VALID; m_blockman.m_dirty_blockindex.insert(pindex); @@ -3778,7 +3779,7 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, Block return true; } -bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<const CBlock>& block, bool force_processing, bool* new_block) +bool ChainstateManager::ProcessNewBlock(const std::shared_ptr<const CBlock>& block, bool force_processing, bool* new_block) { AssertLockNotHeld(cs_main); @@ -3796,7 +3797,7 @@ bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const s // malleability that cause CheckBlock() to fail; see e.g. CVE-2012-2459 and // https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-February/016697.html. Because CheckBlock() is // not very expensive, the anti-DoS benefits of caching failure (of a definitely-invalid block) are not substantial. - bool ret = CheckBlock(*block, state, chainparams.GetConsensus()); + bool ret = CheckBlock(*block, state, GetConsensus()); if (ret) { // Store to disk ret = ActiveChainstate().AcceptBlock(block, state, &pindex, force_processing, nullptr, new_block); @@ -3849,11 +3850,11 @@ bool TestBlockValidity(BlockValidationState& state, indexDummy.phashBlock = &block_hash; // NOTE: CheckBlockHeader is called by CheckBlock - if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainparams, pindexPrev, GetAdjustedTime())) + if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, state.ToString()); if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot)) return error("%s: Consensus::CheckBlock: %s", __func__, state.ToString()); - if (!ContextualCheckBlock(block, state, chainparams.GetConsensus(), pindexPrev)) + if (!ContextualCheckBlock(block, state, chainstate.m_chainman, pindexPrev)) return error("%s: Consensus::ContextualCheckBlock: %s", __func__, state.ToString()); if (!chainstate.ConnectBlock(block, state, &indexDummy, viewNew, true)) { return false; @@ -4133,7 +4134,7 @@ bool CChainState::NeedsRedownload() const // At and above m_params.SegwitHeight, segwit consensus rules must be validated CBlockIndex* block{m_chain.Tip()}; - while (block != nullptr && DeploymentActiveAt(*block, m_params.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) { + while (block != nullptr && DeploymentActiveAt(*block, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) { if (!(block->nStatus & BLOCK_OPT_WITNESS)) { // block is insufficiently validated for a segwit client return true; @@ -4993,7 +4994,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( } int base_height = snapshot_start_block->nHeight; - auto maybe_au_data = ExpectedAssumeutxo(base_height, ::Params()); + auto maybe_au_data = ExpectedAssumeutxo(base_height, GetParams()); if (!maybe_au_data) { LogPrintf("[snapshot] assumeutxo height in snapshot metadata not recognized " /* Continued */ @@ -5147,7 +5148,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( // Fake BLOCK_OPT_WITNESS so that CChainState::NeedsRedownload() // won't ask to rewind the entire assumed-valid chain on startup. - if (DeploymentActiveAt(*index, ::Params().GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) { + if (DeploymentActiveAt(*index, *this, Consensus::DEPLOYMENT_SEGWIT)) { index->nStatus |= BLOCK_OPT_WITNESS; } @@ -5216,9 +5217,9 @@ ChainstateManager::~ChainstateManager() { LOCK(::cs_main); - // TODO: The version bits cache and warning cache should probably become - // non-globals - g_versionbitscache.Clear(); + m_versionbitscache.Clear(); + + // TODO: The warning cache should probably become non-global for (auto& i : warningcache) { i.clear(); } diff --git a/src/validation.h b/src/validation.h index 42e41502f9..04745a6e36 100644 --- a/src/validation.h +++ b/src/validation.h @@ -13,7 +13,9 @@ #include <arith_uint256.h> #include <attributes.h> #include <chain.h> +#include <chainparams.h> #include <consensus/amount.h> +#include <deploymentstatus.h> #include <fs.h> #include <node/blockstorage.h> #include <policy/feerate.h> @@ -26,6 +28,7 @@ #include <util/check.h> #include <util/hasher.h> #include <util/translation.h> +#include <versionbits.h> #include <atomic> #include <map> @@ -40,7 +43,6 @@ class CChainState; class CBlockTreeDB; -class CChainParams; class CTxMemPool; class ChainstateManager; struct ChainTxData; @@ -51,6 +53,9 @@ struct AssumeutxoData; namespace node { class SnapshotMetadata; } // namespace node +namespace Consensus { +struct Params; +} // namespace Consensus /** Default for -minrelaytxfee, minimum relay fee for transactions */ static const unsigned int DEFAULT_MIN_RELAY_TX_FEE = 1000; @@ -359,12 +364,6 @@ bool TestBlockValidity(BlockValidationState& state, bool fCheckPOW = true, bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main); -/** Update uncommitted block structures (currently: only the witness reserved value). This is safe for submitted blocks. */ -void UpdateUncommittedBlockStructures(CBlock& block, const CBlockIndex* pindexPrev, const Consensus::Params& consensusParams); - -/** Produce the necessary coinbase commitment for a block (modifies the hash, don't call for mined blocks). */ -std::vector<unsigned char> GenerateCoinbaseCommitment(CBlock& block, const CBlockIndex* pindexPrev, const Consensus::Params& consensusParams); - /** RAII wrapper for VerifyDB: Verify consistency of the block and coin databases */ class CVerifyDB { public: @@ -495,6 +494,7 @@ public: node::BlockManager& m_blockman; /** Chain parameters for this chainstate */ + /* TODO: replace with m_chainman.GetParams() */ const CChainParams& m_params; //! The chainstate manager that owns this chainstate. The reference is @@ -834,6 +834,8 @@ private: CBlockIndex* m_best_invalid GUARDED_BY(::cs_main){nullptr}; + const CChainParams& m_chainparams; + //! Internal helper for ActivateSnapshot(). [[nodiscard]] bool PopulateAndValidateSnapshot( CChainState& snapshot_chainstate, @@ -847,11 +849,15 @@ private: bool AcceptBlockHeader( const CBlockHeader& block, BlockValidationState& state, - const CChainParams& chainparams, CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); friend CChainState; public: + explicit ChainstateManager(const CChainParams& chainparams) : m_chainparams{chainparams} { } + + const CChainParams& GetParams() const { return m_chainparams; } + const Consensus::Params& GetConsensus() const { return m_chainparams.GetConsensus(); } + std::thread m_load_block; //! A single BlockManager instance is shared across each constructed //! chainstate to avoid duplicating block metadata. @@ -932,6 +938,11 @@ public: return m_blockman.m_block_index; } + /** + * Track versionbit status + */ + mutable VersionBitsCache m_versionbitscache; + //! @returns true if a snapshot-based chainstate is in use. Also implies //! that a background validation chainstate is also in use. bool IsSnapshotActive() const; @@ -960,7 +971,7 @@ public: * @param[out] new_block A boolean which is set to indicate if the block was first received via this call * @returns If the block was processed, independently of block validity */ - bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<const CBlock>& block, bool force_processing, bool* new_block) LOCKS_EXCLUDED(cs_main); + bool ProcessNewBlock(const std::shared_ptr<const CBlock>& block, bool force_processing, bool* new_block) LOCKS_EXCLUDED(cs_main); /** * Process incoming block headers. @@ -970,10 +981,9 @@ public: * * @param[in] block The block headers themselves * @param[out] state This may be set to an Error state if any error occurred processing them - * @param[in] chainparams The params for the chain we want to connect to * @param[out] ppindex If set, the pointer will be set to point to the last new block index object for the given headers */ - bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& block, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main); + bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& block, BlockValidationState& state, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main); /** * Try to add a transaction to the memory pool. @@ -991,9 +1001,34 @@ public: //! ResizeCoinsCaches() as needed. void MaybeRebalanceCaches() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + /** Update uncommitted block structures (currently: only the witness reserved value). This is safe for submitted blocks. */ + void UpdateUncommittedBlockStructures(CBlock& block, const CBlockIndex* pindexPrev) const; + + /** Produce the necessary coinbase commitment for a block (modifies the hash, don't call for mined blocks). */ + std::vector<unsigned char> GenerateCoinbaseCommitment(CBlock& block, const CBlockIndex* pindexPrev) const; + ~ChainstateManager(); }; +/** Deployment* info via ChainstateManager */ +template<typename DEP> +bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const ChainstateManager& chainman, DEP dep) +{ + return DeploymentActiveAfter(pindexPrev, chainman.GetConsensus(), dep, chainman.m_versionbitscache); +} + +template<typename DEP> +bool DeploymentActiveAt(const CBlockIndex& index, const ChainstateManager& chainman, DEP dep) +{ + return DeploymentActiveAt(index, chainman.GetConsensus(), dep, chainman.m_versionbitscache); +} + +template<typename DEP> +bool DeploymentEnabled(const ChainstateManager& chainman, DEP dep) +{ + return DeploymentEnabled(chainman.GetConsensus(), dep); +} + using FopenFn = std::function<FILE*(const fs::path&, const char*)>; /** Dump the mempool to disk. */ diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index edc4633c01..fffab39cc1 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -16,14 +16,16 @@ #include <unordered_map> #include <utility> -//! The MainSignalsInstance manages a list of shared_ptr<CValidationInterface> -//! callbacks. -//! -//! A std::unordered_map is used to track what callbacks are currently -//! registered, and a std::list is to used to store the callbacks that are -//! currently registered as well as any callbacks that are just unregistered -//! and about to be deleted when they are done executing. -struct MainSignalsInstance { +/** + * MainSignalsImpl manages a list of shared_ptr<CValidationInterface> callbacks. + * + * A std::unordered_map is used to track what callbacks are currently + * registered, and a std::list is used to store the callbacks that are + * currently registered as well as any callbacks that are just unregistered + * and about to be deleted when they are done executing. + */ +class MainSignalsImpl +{ private: Mutex m_mutex; //! List entries consist of a callback pointer and reference count. The @@ -40,9 +42,9 @@ public: // our own queue here :( SingleThreadedSchedulerClient m_schedulerClient; - explicit MainSignalsInstance(CScheduler& scheduler LIFETIMEBOUND) : m_schedulerClient(scheduler) {} + explicit MainSignalsImpl(CScheduler& scheduler LIFETIMEBOUND) : m_schedulerClient(scheduler) {} - void Register(std::shared_ptr<CValidationInterface> callbacks) + void Register(std::shared_ptr<CValidationInterface> callbacks) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { LOCK(m_mutex); auto inserted = m_map.emplace(callbacks.get(), m_list.end()); @@ -50,7 +52,7 @@ public: inserted.first->second->callbacks = std::move(callbacks); } - void Unregister(CValidationInterface* callbacks) + void Unregister(CValidationInterface* callbacks) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { LOCK(m_mutex); auto it = m_map.find(callbacks); @@ -64,7 +66,7 @@ public: //! map entry. After this call, the list may still contain callbacks that //! are currently executing, but it will be cleared when they are done //! executing. - void Clear() + void Clear() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { LOCK(m_mutex); for (const auto& entry : m_map) { @@ -73,7 +75,7 @@ public: m_map.clear(); } - template<typename F> void Iterate(F&& f) + template<typename F> void Iterate(F&& f) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { WAIT_LOCK(m_mutex, lock); for (auto it = m_list.begin(); it != m_list.end();) { @@ -92,7 +94,7 @@ static CMainSignals g_signals; void CMainSignals::RegisterBackgroundSignalScheduler(CScheduler& scheduler) { assert(!m_internals); - m_internals = std::make_unique<MainSignalsInstance>(scheduler); + m_internals = std::make_unique<MainSignalsImpl>(scheduler); } void CMainSignals::UnregisterBackgroundSignalScheduler() diff --git a/src/validationinterface.h b/src/validationinterface.h index ac62f8b467..a929a3d56b 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -17,9 +17,7 @@ class BlockValidationState; class CBlock; class CBlockIndex; struct CBlockLocator; -class CConnman; class CValidationInterface; -class uint256; class CScheduler; enum class MemPoolRemovalReason; @@ -177,10 +175,10 @@ protected: friend class ValidationInterfaceTest; }; -struct MainSignalsInstance; +class MainSignalsImpl; class CMainSignals { private: - std::unique_ptr<MainSignalsInstance> m_internals; + std::unique_ptr<MainSignalsImpl> m_internals; friend void ::RegisterSharedValidationInterface(std::shared_ptr<CValidationInterface>); friend void ::UnregisterValidationInterface(CValidationInterface*); diff --git a/src/versionbits.h b/src/versionbits.h index 1b3fa11e61..9f7ee1b48e 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -92,16 +92,16 @@ public: static uint32_t Mask(const Consensus::Params& params, Consensus::DeploymentPos pos); /** Get the BIP9 state for a given deployment for the block after pindexPrev. */ - ThresholdState State(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); + ThresholdState State(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Get the block height at which the BIP9 deployment switched into the state for the block after pindexPrev. */ - int StateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); + int StateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Determine what nVersion a new block should use */ - int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params); + int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); - void Clear(); + void Clear() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); }; #endif // BITCOIN_VERSIONBITS_H diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 1aa0339445..f8230f7a1d 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -99,7 +99,7 @@ void BerkeleyEnvironment::Close() if (ret != 0) LogPrintf("BerkeleyEnvironment::Close: Error %d closing database environment: %s\n", ret, DbEnv::strerror(ret)); if (!fMockDb) - DbEnv((u_int32_t)0).remove(strPath.c_str(), 0); + DbEnv((uint32_t)0).remove(strPath.c_str(), 0); if (error_file) fclose(error_file); @@ -248,7 +248,7 @@ const void* BerkeleyBatch::SafeDbt::get_data() const return m_dbt.get_data(); } -u_int32_t BerkeleyBatch::SafeDbt::get_size() const +uint32_t BerkeleyBatch::SafeDbt::get_size() const { return m_dbt.get_size(); } diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 1c99e1f9af..ddab85521b 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -34,7 +34,7 @@ struct bilingual_str; namespace wallet { struct WalletDatabaseFileId { - u_int8_t value[DB_FILE_ID_LEN]; + uint8_t value[DB_FILE_ID_LEN]; bool operator==(const WalletDatabaseFileId& rhs) const; }; @@ -182,7 +182,7 @@ class BerkeleyBatch : public DatabaseBatch // delegate to Dbt const void* get_data() const; - u_int32_t get_size() const; + uint32_t get_size() const; // conversion operator to access the underlying Dbt operator Dbt*(); diff --git a/src/wallet/context.cpp b/src/wallet/context.cpp index 800aa5bf9c..3d4bf9d703 100644 --- a/src/wallet/context.cpp +++ b/src/wallet/context.cpp @@ -5,6 +5,6 @@ #include <wallet/context.h> namespace wallet { -WalletContext::WalletContext() {} -WalletContext::~WalletContext() {} +WalletContext::WalletContext() = default; +WalletContext::~WalletContext() = default; } // namespace wallet diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 73042424ad..afd2b83971 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -218,21 +218,20 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo // We cannot source new unconfirmed inputs(bip125 rule 2) new_coin_control.m_min_depth = 1; - CTransactionRef tx_new; - CAmount fee_ret; - int change_pos_in_out = -1; // No requested location for change + constexpr int RANDOM_CHANGE_POSITION = -1; bilingual_str fail_reason; FeeCalculation fee_calc_out; - if (!CreateTransaction(wallet, recipients, tx_new, fee_ret, change_pos_in_out, fail_reason, new_coin_control, fee_calc_out, false)) { + std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, fail_reason, new_coin_control, fee_calc_out, false); + if (!txr) { errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + fail_reason); return Result::WALLET_ERROR; } // Write back new fee if successful - new_fee = fee_ret; + new_fee = txr->fee; // Write back transaction - mtx = CMutableTransaction(*tx_new); + mtx = CMutableTransaction(*txr->tx); return Result::OK; } diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 98e843385c..e1203817e0 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -80,7 +80,10 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) //! Construct wallet tx status struct. WalletTxStatus MakeWalletTxStatus(const CWallet& wallet, const CWalletTx& wtx) + EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { + AssertLockHeld(wallet.cs_wallet); + WalletTxStatus result; result.block_height = wtx.state<TxStateConfirmed>() ? wtx.state<TxStateConfirmed>()->confirmed_block_height : @@ -257,13 +260,14 @@ public: bilingual_str& fail_reason) override { LOCK(m_wallet->cs_wallet); - CTransactionRef tx; FeeCalculation fee_calc_out; - if (!CreateTransaction(*m_wallet, recipients, tx, fee, change_pos, - fail_reason, coin_control, fee_calc_out, sign)) { - return {}; - } - return tx; + std::optional<CreatedTransactionResult> txr = CreateTransaction(*m_wallet, recipients, change_pos, + fail_reason, coin_control, fee_calc_out, sign); + if (!txr) return {}; + fee = txr->fee; + change_pos = txr->change_pos; + + return txr->tx; } void commitTransaction(CTransactionRef tx, WalletValueMap value_map, diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index cddf94aab2..8cce07b921 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -123,6 +123,8 @@ static CAmount GetCachableAmount(const CWallet& wallet, const CWalletTx& wtx, CW CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter) { + AssertLockHeld(wallet.cs_wallet); + // Must wait until coinbase is safely deep enough in the chain before valuing it if (wallet.IsTxImmatureCoinBase(wtx)) return 0; @@ -164,6 +166,8 @@ CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx) CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache) { + AssertLockHeld(wallet.cs_wallet); + if (wallet.IsTxImmatureCoinBase(wtx) && wallet.IsTxInMainChain(wtx)) { return GetCachableAmount(wallet, wtx, CWalletTx::IMMATURE_CREDIT, ISMINE_SPENDABLE, !fUseCache); } @@ -173,6 +177,8 @@ CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, b CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletTx& wtx, const bool fUseCache) { + AssertLockHeld(wallet.cs_wallet); + if (wallet.IsTxImmatureCoinBase(wtx) && wallet.IsTxInMainChain(wtx)) { return GetCachableAmount(wallet, wtx, CWalletTx::IMMATURE_CREDIT, ISMINE_WATCH_ONLY, !fUseCache); } @@ -182,6 +188,8 @@ CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletT CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache, const isminefilter& filter) { + AssertLockHeld(wallet.cs_wallet); + // Avoid caching ismine for NO or ALL cases (could remove this check and simplify in the future). bool allow_cache = (filter & ISMINE_ALL) && (filter & ISMINE_ALL) != ISMINE_ALL; diff --git a/src/wallet/receive.h b/src/wallet/receive.h index d7705b5262..1caef293f2 100644 --- a/src/wallet/receive.h +++ b/src/wallet/receive.h @@ -24,17 +24,17 @@ bool OutputIsChange(const CWallet& wallet, const CTxOut& txout) EXCLUSIVE_LOCKS_ CAmount OutputGetChange(const CWallet& wallet, const CTxOut& txout) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); CAmount TxGetChange(const CWallet& wallet, const CTransaction& tx); -CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter); +CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter) + EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); //! filter decides which addresses will count towards the debit CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter); CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx); -CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true); -CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletTx& wtx, const bool fUseCache = true); -// TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct -// annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The -// annotation "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid -// having to resolve the issue of member access into incomplete type CWallet. -CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true, const isminefilter& filter = ISMINE_SPENDABLE) NO_THREAD_SAFETY_ANALYSIS; +CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true) + EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); +CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletTx& wtx, const bool fUseCache = true) + EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); +CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true, const isminefilter& filter = ISMINE_SPENDABLE) + EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); struct COutputEntry { CTxDestination destination; diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 4eccff3969..bd61c9c62f 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -18,12 +18,17 @@ namespace wallet { static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { - std::set<CTxDestination> address_set; + std::set<CScript> output_scripts; if (by_label) { // Get the set of addresses assigned to label std::string label = LabelFromValue(params[0]); - address_set = wallet.GetLabelAddresses(label); + for (const auto& address : wallet.GetLabelAddresses(label)) { + auto output_script{GetScriptForDestination(address)}; + if (wallet.IsMine(output_script)) { + output_scripts.insert(output_script); + } + } } else { // Get the address CTxDestination dest = DecodeDestination(params[0].get_str()); @@ -34,7 +39,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b if (!wallet.IsMine(script_pub_key)) { throw JSONRPCError(RPC_WALLET_ERROR, "Address not found in wallet"); } - address_set.insert(dest); + output_scripts.insert(script_pub_key); } // Minimum confirmations @@ -66,8 +71,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b } for (const CTxOut& txout : wtx.tx->vout) { - CTxDestination address; - if (ExtractDestination(txout.scriptPubKey, address) && wallet.IsMine(address) && address_set.count(address)) { + if (output_scripts.count(txout.scriptPubKey) > 0) { amount += txout.nValue; } } diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 07119133b7..3d975b5402 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -155,15 +155,14 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto std::shuffle(recipients.begin(), recipients.end(), FastRandomContext()); // Send - CAmount nFeeRequired = 0; - int nChangePosRet = -1; + constexpr int RANDOM_CHANGE_POSITION = -1; bilingual_str error; - CTransactionRef tx; FeeCalculation fee_calc_out; - const bool fCreated = CreateTransaction(wallet, recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, true); - if (!fCreated) { + std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, error, coin_control, fee_calc_out, true); + if (!txr) { throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); } + CTransactionRef tx = txr->tx; wallet.CommitTransaction(tx, std::move(map_value), {} /* orderForm */); if (verbose) { UniValue entry(UniValue::VOBJ); diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index c87af2ea30..7dddc4e4bb 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -15,6 +15,7 @@ using interfaces::FoundBlock; namespace wallet { static void WalletTxToJSON(const CWallet& wallet, const CWalletTx& wtx, UniValue& entry) + EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { interfaces::Chain& chain = wallet.chain(); int confirms = wallet.GetTxDepthInMainChain(wtx); @@ -63,9 +64,7 @@ struct tallyitem int nConf{std::numeric_limits<int>::max()}; std::vector<uint256> txids; bool fIsWatchonly{false}; - tallyitem() - { - } + tallyitem() = default; }; static UniValue ListReceived(const CWallet& wallet, const UniValue& params, const bool by_label, const bool include_immature_coinbase) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 55c0a2cb7f..e5fd7b0eb4 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -656,12 +656,10 @@ static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng } } -static bool CreateTransactionInternal( +static std::optional<CreatedTransactionResult> CreateTransactionInternal( CWallet& wallet, const std::vector<CRecipient>& vecSend, - CTransactionRef& tx, - CAmount& nFeeRet, - int& nChangePosInOut, + int change_pos, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, @@ -669,6 +667,11 @@ static bool CreateTransactionInternal( { AssertLockHeld(wallet.cs_wallet); + // out variables, to be packed into returned result structure + CTransactionRef tx; + CAmount nFeeRet; + int nChangePosInOut = change_pos; + FastRandomContext rng_fast; CMutableTransaction txNew; // The resulting transaction that we make @@ -742,12 +745,12 @@ static bool CreateTransactionInternal( // provided one if (coin_control.m_feerate && coin_selection_params.m_effective_feerate > *coin_control.m_feerate) { error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB)); - return false; + return std::nullopt; } if (feeCalc.reason == FeeReason::FALLBACK && !wallet.m_allow_fallback_fee) { // eventually allow a fallback fee error = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); - return false; + return std::nullopt; } // Calculate the cost of change @@ -774,7 +777,7 @@ static bool CreateTransactionInternal( if (IsDust(txout, wallet.chain().relayDustFee())) { error = _("Transaction amount too small"); - return false; + return std::nullopt; } txNew.vout.push_back(txout); } @@ -791,7 +794,7 @@ static bool CreateTransactionInternal( std::optional<SelectionResult> result = SelectCoins(wallet, vAvailableCoins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); if (!result) { error = _("Insufficient funds"); - return false; + return std::nullopt; } TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->m_algo).c_str(), result->m_target, result->GetWaste(), result->GetSelectedValue()); @@ -808,7 +811,7 @@ static bool CreateTransactionInternal( else if ((unsigned int)nChangePosInOut > txNew.vout.size()) { error = _("Transaction change output index out of range"); - return false; + return std::nullopt; } assert(nChangePosInOut != -1); @@ -836,7 +839,7 @@ static bool CreateTransactionInternal( int nBytes = tx_sizes.vsize; if (nBytes == -1) { error = _("Missing solving data for estimating transaction size"); - return false; + return std::nullopt; } nFeeRet = coin_selection_params.m_effective_feerate.GetFee(nBytes); @@ -900,7 +903,7 @@ static bool CreateTransactionInternal( } else { error = _("The transaction amount is too small to send after the fee has been deducted"); } - return false; + return std::nullopt; } } ++i; @@ -910,12 +913,12 @@ static bool CreateTransactionInternal( // Give up if change keypool ran out and change is required if (scriptChange.empty() && nChangePosInOut != -1) { - return false; + return std::nullopt; } if (sign && !wallet.SignTransaction(txNew)) { error = _("Signing transaction failed"); - return false; + return std::nullopt; } // Return the constructed transaction data. @@ -926,19 +929,19 @@ static bool CreateTransactionInternal( (!sign && tx_sizes.weight > MAX_STANDARD_TX_WEIGHT)) { error = _("Transaction too large"); - return false; + return std::nullopt; } if (nFeeRet > wallet.m_default_max_tx_fee) { error = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED); - return false; + return std::nullopt; } if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits if (!wallet.chain().checkChainLimits(tx)) { error = _("Transaction has too long of a mempool chain"); - return false; + return std::nullopt; } } @@ -955,15 +958,13 @@ static bool CreateTransactionInternal( feeCalc.est.fail.start, feeCalc.est.fail.end, (feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) > 0.0 ? 100 * feeCalc.est.fail.withinTarget / (feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) : 0.0, feeCalc.est.fail.withinTarget, feeCalc.est.fail.totalConfirmed, feeCalc.est.fail.inMempool, feeCalc.est.fail.leftMempool); - return true; + return CreatedTransactionResult(tx, nFeeRet, nChangePosInOut); } -bool CreateTransaction( +std::optional<CreatedTransactionResult> CreateTransaction( CWallet& wallet, const std::vector<CRecipient>& vecSend, - CTransactionRef& tx, - CAmount& nFeeRet, - int& nChangePosInOut, + int change_pos, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, @@ -971,42 +972,37 @@ bool CreateTransaction( { if (vecSend.empty()) { error = _("Transaction must have at least one recipient"); - return false; + return std::nullopt; } if (std::any_of(vecSend.cbegin(), vecSend.cend(), [](const auto& recipient){ return recipient.nAmount < 0; })) { error = _("Transaction amounts must not be negative"); - return false; + return std::nullopt; } LOCK(wallet.cs_wallet); - int nChangePosIn = nChangePosInOut; - Assert(!tx); // tx is an out-param. TODO change the return type from bool to tx (or nullptr) - bool res = CreateTransactionInternal(wallet, vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, fee_calc_out, sign); - TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), res, nFeeRet, nChangePosInOut); + std::optional<CreatedTransactionResult> txr_ungrouped = CreateTransactionInternal(wallet, vecSend, change_pos, error, coin_control, fee_calc_out, sign); + TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), txr_ungrouped.has_value(), + txr_ungrouped.has_value() ? txr_ungrouped->fee : 0, txr_ungrouped.has_value() ? txr_ungrouped->change_pos : 0); + if (!txr_ungrouped) return std::nullopt; // try with avoidpartialspends unless it's enabled already - if (res && nFeeRet > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) { + if (txr_ungrouped->fee > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) { TRACE1(coin_selection, attempting_aps_create_tx, wallet.GetName().c_str()); CCoinControl tmp_cc = coin_control; tmp_cc.m_avoid_partial_spends = true; - CAmount nFeeRet2; - CTransactionRef tx2; - int nChangePosInOut2 = nChangePosIn; bilingual_str error2; // fired and forgotten; if an error occurs, we discard the results - if (CreateTransactionInternal(wallet, vecSend, tx2, nFeeRet2, nChangePosInOut2, error2, tmp_cc, fee_calc_out, sign)) { + std::optional<CreatedTransactionResult> txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, error2, tmp_cc, fee_calc_out, sign); + if (txr_grouped) { // if fee of this alternative one is within the range of the max fee, we use this one - const bool use_aps = nFeeRet2 <= nFeeRet + wallet.m_max_aps_fee; - wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", nFeeRet, nFeeRet2, use_aps ? "grouped" : "non-grouped"); - TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, res, nFeeRet2, nChangePosInOut2); - if (use_aps) { - tx = tx2; - nFeeRet = nFeeRet2; - nChangePosInOut = nChangePosInOut2; - } + const bool use_aps = txr_grouped->fee <= txr_ungrouped->fee + wallet.m_max_aps_fee; + wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", + txr_ungrouped->fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped"); + TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, true, txr_grouped->fee, txr_grouped->change_pos); + if (use_aps) return txr_grouped; } } - return res; + return txr_ungrouped; } bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl coinControl) @@ -1030,11 +1026,12 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, // CreateTransaction call and LockCoin calls (when lockUnspents is true). LOCK(wallet.cs_wallet); - CTransactionRef tx_new; FeeCalculation fee_calc_out; - if (!CreateTransaction(wallet, vecSend, tx_new, nFeeRet, nChangePosInOut, error, coinControl, fee_calc_out, false)) { - return false; - } + std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, vecSend, nChangePosInOut, error, coinControl, fee_calc_out, false); + if (!txr) return false; + CTransactionRef tx_new = txr->tx; + nFeeRet = txr->fee; + nChangePosInOut = txr->change_pos; if (nChangePosInOut != -1) { tx.vout.insert(tx.vout.begin() + nChangePosInOut, tx_new->vout[nChangePosInOut]); diff --git a/src/wallet/spend.h b/src/wallet/spend.h index e43aac5273..8af712110d 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -10,6 +10,8 @@ #include <wallet/transaction.h> #include <wallet/wallet.h> +#include <optional> + namespace wallet { /** Get the marginal bytes if spending the specified output from this transaction. * use_max_sig indicates whether to use the maximum sized, 72 byte signature when calculating the @@ -82,12 +84,22 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); +struct CreatedTransactionResult +{ + CTransactionRef tx; + CAmount fee; + int change_pos; + + CreatedTransactionResult(CTransactionRef tx, CAmount fee, int change_pos) + : tx(tx), fee(fee), change_pos(change_pos) {} +}; + /** * Create a new transaction paying the recipients with a set of coins * selected by SelectCoins(); Also create the change output, when needed - * @note passing nChangePosInOut as -1 will result in setting a random position + * @note passing change_pos as -1 will result in setting a random position */ -bool CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign = true); +std::optional<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign = true); /** * Insert additional inputs into the transaction by diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index 334bd5b8bc..bdc148afb4 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -27,9 +27,7 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) // instead of the miner. auto check_tx = [&wallet](CAmount leftover_input_amount) { CRecipient recipient{GetScriptForRawPubKey({}), 50 * COIN - leftover_input_amount, true /* subtract fee */}; - CTransactionRef tx; - CAmount fee; - int change_pos = -1; + constexpr int RANDOM_CHANGE_POSITION = -1; bilingual_str error; CCoinControl coin_control; coin_control.m_feerate.emplace(10000); @@ -37,11 +35,12 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) // We need to use a change type with high cost of change so that the leftover amount will be dropped to fee instead of added as a change output coin_control.m_change_type = OutputType::LEGACY; FeeCalculation fee_calc; - BOOST_CHECK(CreateTransaction(*wallet, {recipient}, tx, fee, change_pos, error, coin_control, fee_calc)); - BOOST_CHECK_EQUAL(tx->vout.size(), 1); - BOOST_CHECK_EQUAL(tx->vout[0].nValue, recipient.nAmount + leftover_input_amount - fee); - BOOST_CHECK_GT(fee, 0); - return fee; + std::optional<CreatedTransactionResult> txr = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, error, coin_control, fee_calc); + BOOST_CHECK(txr.has_value()); + BOOST_CHECK_EQUAL(txr->tx->vout.size(), 1); + BOOST_CHECK_EQUAL(txr->tx->vout[0].nValue, recipient.nAmount + leftover_input_amount - txr->fee); + BOOST_CHECK_GT(txr->fee, 0); + return txr->fee; }; // Send full input amount to recipient, check that only nonzero fee is diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 683f0eb327..d4406fd5dd 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -521,13 +521,14 @@ public: CWalletTx& AddTx(CRecipient recipient) { CTransactionRef tx; - CAmount fee; - int changePos = -1; bilingual_str error; CCoinControl dummy; FeeCalculation fee_calc_out; { - BOOST_CHECK(CreateTransaction(*wallet, {recipient}, tx, fee, changePos, error, dummy, fee_calc_out)); + constexpr int RANDOM_CHANGE_POSITION = -1; + std::optional<CreatedTransactionResult> txr = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, error, dummy, fee_calc_out); + BOOST_CHECK(txr.has_value()); + tx = txr->tx; } wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 356d0ce5a0..6c333c709b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1707,8 +1707,10 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r */ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate) { - int64_t nNow = GetTime(); - int64_t start_time = GetTimeMillis(); + using Clock = std::chrono::steady_clock; + constexpr auto LOG_INTERVAL{60s}; + auto current_time{Clock::now()}; + auto start_time{Clock::now()}; assert(reserver.isReserved()); @@ -1735,8 +1737,8 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc if (block_height % 100 == 0 && progress_end - progress_begin > 0.0) { ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), std::max(1, std::min(99, (int)(m_scanning_progress * 100)))); } - if (GetTime() >= nNow + 60) { - nNow = GetTime(); + if (Clock::now() >= current_time + LOG_INTERVAL) { + current_time = Clock::now(); WalletLogPrintf("Still rescanning. At block %d. Progress=%f\n", block_height, progress_current); } @@ -1803,7 +1805,8 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc WalletLogPrintf("Rescan interrupted by shutdown request at block %d. Progress=%f\n", block_height, progress_current); result.status = ScanResult::USER_ABORT; } else { - WalletLogPrintf("Rescan completed in %15dms\n", GetTimeMillis() - start_time); + auto duration_milliseconds = std::chrono::duration_cast<std::chrono::milliseconds>(Clock::now() - start_time); + WalletLogPrintf("Rescan completed in %15dms\n", duration_milliseconds.count()); } return result; } @@ -1838,6 +1841,8 @@ void CWallet::ReacceptWalletTransactions() bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const { + AssertLockHeld(cs_wallet); + // Can't relay if wallet is not broadcasting if (!GetBroadcastTransactions()) return false; // Don't relay abandoned transactions @@ -1866,12 +1871,11 @@ bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string std::set<uint256> CWallet::GetTxConflicts(const CWalletTx& wtx) const { - std::set<uint256> result; - { - uint256 myHash = wtx.GetHash(); - result = GetConflicts(myHash); - result.erase(myHash); - } + AssertLockHeld(cs_wallet); + + const uint256 myHash{wtx.GetHash()}; + std::set<uint256> result{GetConflicts(myHash)}; + result.erase(myHash); return result; } @@ -2961,6 +2965,7 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf // so that in case of a shutdown event, the rescan will be repeated at the next start. // This is temporary until rescan and notifications delivery are unified under same // interface. + walletInstance->m_attaching_chain = true; //ignores chainStateFlushed notifications walletInstance->m_chain_notifications_handler = walletInstance->chain().handleNotifications(walletInstance); // If rescan_required = true, rescan_height remains equal to 0 @@ -2987,7 +2992,6 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf if (tip_height && *tip_height != rescan_height) { - walletInstance->m_attaching_chain = true; //ignores chainStateFlushed notifications if (chain.havePruned()) { int block_height = *tip_height; while (block_height > 0 && chain.haveBlockOnDisk(block_height - 1) && rescan_height != block_height) { @@ -3031,6 +3035,7 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf walletInstance->chainStateFlushed(chain.getTipLocator()); walletInstance->GetDatabase().IncrementUpdateCounter(); } + walletInstance->m_attaching_chain = false; return true; } @@ -3124,8 +3129,11 @@ int CWallet::GetTxDepthInMainChain(const CWalletTx& wtx) const int CWallet::GetTxBlocksToMaturity(const CWalletTx& wtx) const { - if (!wtx.IsCoinBase()) + AssertLockHeld(cs_wallet); + + if (!wtx.IsCoinBase()) { return 0; + } int chain_depth = GetTxDepthInMainChain(wtx); assert(chain_depth >= 0); // coinbase tx should not be conflicted return std::max(0, (COINBASE_MATURITY+1) - chain_depth); @@ -3133,6 +3141,8 @@ int CWallet::GetTxBlocksToMaturity(const CWalletTx& wtx) const bool CWallet::IsTxImmatureCoinBase(const CWalletTx& wtx) const { + AssertLockHeld(cs_wallet); + // note GetBlocksToMaturity is 0 for non-coinbase tx return GetTxBlocksToMaturity(wtx) > 0; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4e81a2b957..7da601c3b7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -415,13 +415,7 @@ public: const CWalletTx* GetWalletTx(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct - // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation - // "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid having to - // resolve the issue of member access into incomplete type CWallet. Note - // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)" - // in place. - std::set<uint256> GetTxConflicts(const CWalletTx& wtx) const NO_THREAD_SAFETY_ANALYSIS; + std::set<uint256> GetTxConflicts(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Return depth of transaction in blockchain: @@ -429,22 +423,20 @@ public: * 0 : in memory pool, waiting to be included in a block * >=1 : this many blocks deep in the main chain */ - // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct - // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation - // "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid having to - // resolve the issue of member access into incomplete type CWallet. Note - // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)" - // in place. - int GetTxDepthInMainChain(const CWalletTx& wtx) const NO_THREAD_SAFETY_ANALYSIS; - bool IsTxInMainChain(const CWalletTx& wtx) const { return GetTxDepthInMainChain(wtx) > 0; } + int GetTxDepthInMainChain(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool IsTxInMainChain(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) + { + AssertLockHeld(cs_wallet); + return GetTxDepthInMainChain(wtx) > 0; + } /** * @return number of blocks to maturity for this transaction: * 0 : is not a coinbase transaction, or is a mature coinbase transaction * >0 : is a coinbase transaction which matures in this many blocks */ - int GetTxBlocksToMaturity(const CWalletTx& wtx) const; - bool IsTxImmatureCoinBase(const CWalletTx& wtx) const; + int GetTxBlocksToMaturity(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool IsTxImmatureCoinBase(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! check whether we support the named feature bool CanSupportFeature(enum WalletFeature wf) const override EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return IsFeatureSupported(nWalletVersion, wf); } @@ -584,7 +576,8 @@ public: void CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm); /** Pass this transaction to node for mempool insertion and relay to peers if flag set to true */ - bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const; + bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const + EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, const CCoinControl* coin_control = nullptr) const { diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 7bbed7973f..79e0a330b7 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -314,8 +314,7 @@ public: std::map<uint160, CHDChain> m_hd_chains; bool tx_corrupt{false}; - CWalletScanState() { - } + CWalletScanState() = default; }; static bool |