From fa340b87944764ea4e8e04038fe7471fd452bc23 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 3 Apr 2021 17:26:39 +0200 Subject: [PATCH] refactor: Avoid magic value of all-zeros in assumeutxo base_blockhash Just use std::optional --- .../validation_chainstatemanager_tests.cpp | 12 +++++----- src/validation.cpp | 22 +++++++++---------- src/validation.h | 8 +++---- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 272f6163c90c9..0b912acb088a5 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -226,10 +226,8 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) // Snapshot should refuse to load at this height. BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(m_node, m_path_root)); - BOOST_CHECK(chainman.ActiveChainstate().m_from_snapshot_blockhash.IsNull()); - BOOST_CHECK_EQUAL( - chainman.ActiveChainstate().m_from_snapshot_blockhash, - chainman.SnapshotBlockhash().value_or(uint256())); + BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash); + BOOST_CHECK(!chainman.SnapshotBlockhash()); // Mine 10 more blocks, putting at us height 110 where a valid assumeutxo value can // be found. @@ -274,9 +272,9 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(m_node, m_path_root)); // Ensure our active chain is the snapshot chainstate. - BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash.IsNull()); + BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash->IsNull()); BOOST_CHECK_EQUAL( - chainman.ActiveChainstate().m_from_snapshot_blockhash, + *chainman.ActiveChainstate().m_from_snapshot_blockhash, *chainman.SnapshotBlockhash()); const AssumeutxoData& au_data = *ExpectedAssumeutxo(snapshot_height, ::Params()); @@ -352,7 +350,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) // Snapshot blockhash should be unchanged. BOOST_CHECK_EQUAL( - chainman.ActiveChainstate().m_from_snapshot_blockhash, + *chainman.ActiveChainstate().m_from_snapshot_blockhash, loaded_snapshot_blockhash); } diff --git a/src/validation.cpp b/src/validation.cpp index 65bfbbf999bc4..4f9b8687b77df 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1158,7 +1158,7 @@ void CoinsViews::InitCache() m_cacheview = std::make_unique(&m_catcherview); } -CChainState::CChainState(CTxMemPool& mempool, BlockManager& blockman, uint256 from_snapshot_blockhash) +CChainState::CChainState(CTxMemPool& mempool, BlockManager& blockman, std::optional from_snapshot_blockhash) : m_mempool(mempool), m_blockman(blockman), m_from_snapshot_blockhash(from_snapshot_blockhash) {} @@ -1169,8 +1169,8 @@ void CChainState::InitCoinsDB( bool should_wipe, std::string leveldb_name) { - if (!m_from_snapshot_blockhash.IsNull()) { - leveldb_name += "_" + m_from_snapshot_blockhash.ToString(); + if (m_from_snapshot_blockhash) { + leveldb_name += "_" + m_from_snapshot_blockhash->ToString(); } m_coins_views = std::make_unique( @@ -3877,7 +3877,7 @@ bool CVerifyDB::VerifyDB( int reportDone = 0; LogPrintf("[0%%]..."); /* Continued */ - bool is_snapshot_cs = !chainstate.m_from_snapshot_blockhash.IsNull(); + const bool is_snapshot_cs{!chainstate.m_from_snapshot_blockhash}; for (pindex = chainstate.m_chain.Tip(); pindex && pindex->pprev; pindex = pindex->pprev) { const int percentageDone = std::max(1, std::min(99, (int)(((double)(chainstate.m_chain.Height() - pindex->nHeight)) / (double)nCheckDepth * (nCheckLevel >= 4 ? 50 : 100)))); @@ -4458,8 +4458,8 @@ std::string CChainState::ToString() { CBlockIndex* tip = m_chain.Tip(); return strprintf("Chainstate [%s] @ height %d (%s)", - m_from_snapshot_blockhash.IsNull() ? "ibd" : "snapshot", - tip ? tip->nHeight : -1, tip ? tip->GetBlockHash().ToString() : "null"); + m_from_snapshot_blockhash ? "snapshot" : "ibd", + tip ? tip->nHeight : -1, tip ? tip->GetBlockHash().ToString() : "null"); } bool CChainState::ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) @@ -4662,10 +4662,10 @@ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pin return std::min(pindex->nChainTx / fTxTotal, 1.0); } -std::optional ChainstateManager::SnapshotBlockhash() const { +std::optional ChainstateManager::SnapshotBlockhash() const +{ LOCK(::cs_main); - if (m_active_chainstate != nullptr && - !m_active_chainstate->m_from_snapshot_blockhash.IsNull()) { + if (m_active_chainstate && m_active_chainstate->m_from_snapshot_blockhash) { // If a snapshot chainstate exists, it will always be our active. return m_active_chainstate->m_from_snapshot_blockhash; } @@ -4688,9 +4688,9 @@ std::vector ChainstateManager::GetAll() return out; } -CChainState& ChainstateManager::InitializeChainstate(CTxMemPool& mempool, const uint256& snapshot_blockhash) +CChainState& ChainstateManager::InitializeChainstate(CTxMemPool& mempool, const std::optional& snapshot_blockhash) { - bool is_snapshot = !snapshot_blockhash.IsNull(); + bool is_snapshot = snapshot_blockhash.has_value(); std::unique_ptr& to_modify = is_snapshot ? m_snapshot_chainstate : m_ibd_chainstate; diff --git a/src/validation.h b/src/validation.h index 8fa45c3f77da9..1b50644185db5 100644 --- a/src/validation.h +++ b/src/validation.h @@ -553,7 +553,7 @@ class CChainState //! CChainState instances. BlockManager& m_blockman; - explicit CChainState(CTxMemPool& mempool, BlockManager& blockman, uint256 from_snapshot_blockhash = uint256()); + explicit CChainState(CTxMemPool& mempool, BlockManager& blockman, std::optional from_snapshot_blockhash = std::nullopt); /** * Initialize the CoinsViews UTXO set database management data structures. The in-memory @@ -584,9 +584,9 @@ class CChainState /** * The blockhash which is the base of the snapshot this chainstate was created from. * - * IsNull() if this chainstate was not created from a snapshot. + * std::nullopt if this chainstate was not created from a snapshot. */ - const uint256 m_from_snapshot_blockhash{}; + const std::optional m_from_snapshot_blockhash; /** * The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for itself and all ancestors) and @@ -866,7 +866,7 @@ class ChainstateManager // constructor //! @param[in] snapshot_blockhash If given, signify that this chainstate //! is based on a snapshot. - CChainState& InitializeChainstate(CTxMemPool& mempool, const uint256& snapshot_blockhash = uint256()) + CChainState& InitializeChainstate(CTxMemPool& mempool, const std::optional& snapshot_blockhash = std::nullopt) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! Get all chainstates currently being used.