diff options
author | Ryan Ofsky <ryan@ofsky.org> | 2024-07-16 09:35:51 -0400 |
---|---|---|
committer | Ryan Ofsky <ryan@ofsky.org> | 2024-07-16 10:14:23 -0400 |
commit | 46878326808f643f08ec3f69dcec5c8fafeb7b59 (patch) | |
tree | d3cf683245b414d63dbd41cc1d9fbc5172f16d4d /src | |
parent | 1db0be83535fc5cab6d2b42db27a38c155e15b4e (diff) | |
parent | 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d (diff) | |
download | bitcoin-46878326808f643f08ec3f69dcec5c8fafeb7b59.tar.xz |
Merge bitcoin/bitcoin#30425: kernel: De-globalize static validation variables
51fa26239af9bbfd44029aaf595cb4c6a8d4a75d refactor: Mark some static global vars as const (TheCharlatan)
39f9b80fba85d9818222c4d76e99ea1a804f5dda refactor: De-globalize last notified header index (TheCharlatan)
3443943f86678eb27d9895f3871fcf3945eb1c4f refactor: De-globalize validation benchmark timekeeping (TheCharlatan)
Pull request description:
In future, users of the kernel library might run multiple chainstates in parallel, or create and destroy multiple chainstates over the lifetime of a process. Having static, mutable variables could lead to state inconsistencies in these scenarios.
---
This pull request is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587).
ACKs for top commit:
dergoegge:
Code review ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d
maflcko:
ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d 🍚
tdb3:
code review ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d
Tree-SHA512: da91aa7ffa343325cabb8764ef03c8358845662cf0ba8a6cc1dd38e40e5462d88734f2b459c2de8e7a041551eda9143d92487842609f7f30636f61a0cd3c57ee
Diffstat (limited to 'src')
-rw-r--r-- | src/core_read.cpp | 2 | ||||
-rw-r--r-- | src/script/descriptor.cpp | 4 | ||||
-rw-r--r-- | src/validation.cpp | 89 | ||||
-rw-r--r-- | src/validation.h | 18 |
4 files changed, 58 insertions, 55 deletions
diff --git a/src/core_read.cpp b/src/core_read.cpp index 114f92fc45..0ba271a8d2 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -245,7 +245,7 @@ bool ParseHashStr(const std::string& strHex, uint256& result) util::Result<int> SighashFromStr(const std::string& sighash) { - static std::map<std::string, int> map_sighash_values = { + static const std::map<std::string, int> map_sighash_values = { {std::string("DEFAULT"), int(SIGHASH_DEFAULT)}, {std::string("ALL"), int(SIGHASH_ALL)}, {std::string("ALL|ANYONECANPAY"), int(SIGHASH_ALL|SIGHASH_ANYONECANPAY)}, diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 0987db194c..ae9dba6a50 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -116,13 +116,13 @@ std::string DescriptorChecksum(const Span<const char>& span) * As a result, within-group-of-32 errors count as 1 symbol, as do cross-group errors that don't affect * the position within the groups. */ - static std::string INPUT_CHARSET = + static const std::string INPUT_CHARSET = "0123456789()[],'/*abcdefgh@:$%{}" "IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~" "ijklmnopqrstuvwxyzABCDEFGH`#\"\\ "; /** The character set for the checksum itself (same as bech32). */ - static std::string CHECKSUM_CHARSET = "qpzry9x8gf2tvdw0s3jn54khce6mua7l"; + static const std::string CHECKSUM_CHARSET = "qpzry9x8gf2tvdw0s3jn54khce6mua7l"; uint64_t c = 1; int cls = 0; diff --git a/src/validation.cpp b/src/validation.cpp index 988df3802a..72ff24d584 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2399,15 +2399,6 @@ static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const Ch } -static SteadyClock::duration time_check{}; -static SteadyClock::duration time_forks{}; -static SteadyClock::duration time_connect{}; -static SteadyClock::duration time_verify{}; -static SteadyClock::duration time_undo{}; -static SteadyClock::duration time_index{}; -static SteadyClock::duration time_total{}; -static int64_t num_blocks_total = 0; - /** Apply the effects of this block (with given index) on the UTXO set represented by coins. * Validity checks that depend on the UTXO set are also done; ConnectBlock() * can fail if those validity checks fail (among other reasons). */ @@ -2452,7 +2443,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, uint256 hashPrevBlock = pindex->pprev == nullptr ? uint256() : pindex->pprev->GetBlockHash(); assert(hashPrevBlock == view.GetBestBlock()); - num_blocks_total++; + m_chainman.num_blocks_total++; // Special case for the genesis block, skipping connection of its transactions // (its coinbase is unspendable) @@ -2494,11 +2485,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, } const auto time_1{SteadyClock::now()}; - time_check += time_1 - time_start; + m_chainman.time_check += time_1 - time_start; LogPrint(BCLog::BENCH, " - Sanity checks: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_1 - time_start), - Ticks<SecondsDouble>(time_check), - Ticks<MillisecondsDouble>(time_check) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_check), + Ticks<MillisecondsDouble>(m_chainman.time_check) / m_chainman.num_blocks_total); // Do not allow blocks that contain transactions which 'overwrite' older transactions, // unless those are already completely spent. @@ -2596,11 +2587,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, unsigned int flags{GetBlockScriptFlags(*pindex, m_chainman)}; const auto time_2{SteadyClock::now()}; - time_forks += time_2 - time_1; + m_chainman.time_forks += time_2 - time_1; LogPrint(BCLog::BENCH, " - Fork checks: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_2 - time_1), - Ticks<SecondsDouble>(time_forks), - Ticks<MillisecondsDouble>(time_forks) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_forks), + Ticks<MillisecondsDouble>(m_chainman.time_forks) / m_chainman.num_blocks_total); CBlockUndo blockundo; @@ -2687,12 +2678,12 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, UpdateCoins(tx, view, i == 0 ? undoDummy : blockundo.vtxundo.back(), pindex->nHeight); } const auto time_3{SteadyClock::now()}; - time_connect += time_3 - time_2; + m_chainman.time_connect += time_3 - time_2; LogPrint(BCLog::BENCH, " - Connect %u transactions: %.2fms (%.3fms/tx, %.3fms/txin) [%.2fs (%.2fms/blk)]\n", (unsigned)block.vtx.size(), Ticks<MillisecondsDouble>(time_3 - time_2), Ticks<MillisecondsDouble>(time_3 - time_2) / block.vtx.size(), nInputs <= 1 ? 0 : Ticks<MillisecondsDouble>(time_3 - time_2) / (nInputs - 1), - Ticks<SecondsDouble>(time_connect), - Ticks<MillisecondsDouble>(time_connect) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_connect), + Ticks<MillisecondsDouble>(m_chainman.time_connect) / m_chainman.num_blocks_total); CAmount blockReward = nFees + GetBlockSubsidy(pindex->nHeight, params.GetConsensus()); if (block.vtx[0]->GetValueOut() > blockReward) { @@ -2705,12 +2696,12 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "block-validation-failed"); } const auto time_4{SteadyClock::now()}; - time_verify += time_4 - time_2; + m_chainman.time_verify += time_4 - time_2; LogPrint(BCLog::BENCH, " - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs (%.2fms/blk)]\n", nInputs - 1, Ticks<MillisecondsDouble>(time_4 - time_2), nInputs <= 1 ? 0 : Ticks<MillisecondsDouble>(time_4 - time_2) / (nInputs - 1), - Ticks<SecondsDouble>(time_verify), - Ticks<MillisecondsDouble>(time_verify) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_verify), + Ticks<MillisecondsDouble>(m_chainman.time_verify) / m_chainman.num_blocks_total); if (fJustCheck) return true; @@ -2720,11 +2711,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, } const auto time_5{SteadyClock::now()}; - time_undo += time_5 - time_4; + m_chainman.time_undo += time_5 - time_4; LogPrint(BCLog::BENCH, " - Write undo data: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_5 - time_4), - Ticks<SecondsDouble>(time_undo), - Ticks<MillisecondsDouble>(time_undo) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_undo), + Ticks<MillisecondsDouble>(m_chainman.time_undo) / m_chainman.num_blocks_total); if (!pindex->IsValid(BLOCK_VALID_SCRIPTS)) { pindex->RaiseValidity(BLOCK_VALID_SCRIPTS); @@ -2735,11 +2726,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, view.SetBestBlock(pindex->GetBlockHash()); const auto time_6{SteadyClock::now()}; - time_index += time_6 - time_5; + m_chainman.time_index += time_6 - time_5; LogPrint(BCLog::BENCH, " - Index writing: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_6 - time_5), - Ticks<SecondsDouble>(time_index), - Ticks<MillisecondsDouble>(time_index) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_index), + Ticks<MillisecondsDouble>(m_chainman.time_index) / m_chainman.num_blocks_total); TRACE6(validation, block_connected, block_hash.data(), @@ -3097,11 +3088,6 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra return true; } -static SteadyClock::duration time_connect_total{}; -static SteadyClock::duration time_flush{}; -static SteadyClock::duration time_chainstate{}; -static SteadyClock::duration time_post_connect{}; - struct PerBlockConnectTrace { CBlockIndex* pindex = nullptr; std::shared_ptr<const CBlock> pblock; @@ -3188,31 +3174,31 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, return false; } time_3 = SteadyClock::now(); - time_connect_total += time_3 - time_2; - assert(num_blocks_total > 0); + m_chainman.time_connect_total += time_3 - time_2; + assert(m_chainman.num_blocks_total > 0); LogPrint(BCLog::BENCH, " - Connect total: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_3 - time_2), - Ticks<SecondsDouble>(time_connect_total), - Ticks<MillisecondsDouble>(time_connect_total) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_connect_total), + Ticks<MillisecondsDouble>(m_chainman.time_connect_total) / m_chainman.num_blocks_total); bool flushed = view.Flush(); assert(flushed); } const auto time_4{SteadyClock::now()}; - time_flush += time_4 - time_3; + m_chainman.time_flush += time_4 - time_3; LogPrint(BCLog::BENCH, " - Flush: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_4 - time_3), - Ticks<SecondsDouble>(time_flush), - Ticks<MillisecondsDouble>(time_flush) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_flush), + Ticks<MillisecondsDouble>(m_chainman.time_flush) / m_chainman.num_blocks_total); // Write the chain state to disk, if necessary. if (!FlushStateToDisk(state, FlushStateMode::IF_NEEDED)) { return false; } const auto time_5{SteadyClock::now()}; - time_chainstate += time_5 - time_4; + m_chainman.time_chainstate += time_5 - time_4; LogPrint(BCLog::BENCH, " - Writing chainstate: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_5 - time_4), - Ticks<SecondsDouble>(time_chainstate), - Ticks<MillisecondsDouble>(time_chainstate) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_chainstate), + Ticks<MillisecondsDouble>(m_chainman.time_chainstate) / m_chainman.num_blocks_total); // Remove conflicting transactions from the mempool.; if (m_mempool) { m_mempool->removeForBlock(blockConnecting.vtx, pindexNew->nHeight); @@ -3223,16 +3209,16 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, UpdateTip(pindexNew); const auto time_6{SteadyClock::now()}; - time_post_connect += time_6 - time_5; - time_total += time_6 - time_1; + m_chainman.time_post_connect += time_6 - time_5; + m_chainman.time_total += time_6 - time_1; LogPrint(BCLog::BENCH, " - Connect postprocess: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_6 - time_5), - Ticks<SecondsDouble>(time_post_connect), - Ticks<MillisecondsDouble>(time_post_connect) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_post_connect), + Ticks<MillisecondsDouble>(m_chainman.time_post_connect) / m_chainman.num_blocks_total); LogPrint(BCLog::BENCH, "- Connect block: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_6 - time_1), - Ticks<SecondsDouble>(time_total), - Ticks<MillisecondsDouble>(time_total) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_total), + Ticks<MillisecondsDouble>(m_chainman.time_total) / m_chainman.num_blocks_total); // If we are the background validation chainstate, check to see if we are done // validating the snapshot (i.e. our tip has reached the snapshot's base block). @@ -3423,16 +3409,15 @@ static bool NotifyHeaderTip(ChainstateManager& chainman) LOCKS_EXCLUDED(cs_main) { bool fNotify = false; bool fInitialBlockDownload = false; - static CBlockIndex* pindexHeaderOld = nullptr; CBlockIndex* pindexHeader = nullptr; { LOCK(cs_main); pindexHeader = chainman.m_best_header; - if (pindexHeader != pindexHeaderOld) { + if (pindexHeader != chainman.m_last_notified_header) { fNotify = true; fInitialBlockDownload = chainman.IsInitialBlockDownload(); - pindexHeaderOld = pindexHeader; + chainman.m_last_notified_header = pindexHeader; } } // Send block tip changed notifications without cs_main diff --git a/src/validation.h b/src/validation.h index 9f99ba796b..ea4ec84242 100644 --- a/src/validation.h +++ b/src/validation.h @@ -949,6 +949,21 @@ private: //! A queue for script verifications that have to be performed by worker threads. CCheckQueue<CScriptCheck> m_script_check_queue; + //! Timers and counters used for benchmarking validation in both background + //! and active chainstates. + SteadyClock::duration GUARDED_BY(::cs_main) time_check{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_forks{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_connect{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_verify{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_undo{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_index{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_total{}; + int64_t GUARDED_BY(::cs_main) num_blocks_total{0}; + SteadyClock::duration GUARDED_BY(::cs_main) time_connect_total{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_flush{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_chainstate{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_post_connect{}; + public: using Options = kernel::ChainstateManagerOpts; @@ -1048,6 +1063,9 @@ public: /** Best header we've seen so far (used for getheaders queries' starting points). */ CBlockIndex* m_best_header GUARDED_BY(::cs_main){nullptr}; + /** The last header for which a headerTip notification was issued. */ + CBlockIndex* m_last_notified_header GUARDED_BY(::cs_main){nullptr}; + //! The total number of bytes available for us to use across all in-memory //! coins caches. This will be split somehow across chainstates. int64_t m_total_coinstip_cache{0}; |