Skip to content

Commit

Permalink
refactor: Avoid magic value of all-zeros in assumeutxo base_blockhash
Browse files Browse the repository at this point in the history
Just use std::optional
  • Loading branch information
MarcoFalke committed May 11, 2021
1 parent fae33f9 commit fa340b8
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 22 deletions.
12 changes: 5 additions & 7 deletions src/test/validation_chainstatemanager_tests.cpp
Expand Up @@ -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.
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
}

Expand Down
22 changes: 11 additions & 11 deletions src/validation.cpp
Expand Up @@ -1158,7 +1158,7 @@ void CoinsViews::InitCache()
m_cacheview = std::make_unique<CCoinsViewCache>(&m_catcherview);
}

CChainState::CChainState(CTxMemPool& mempool, BlockManager& blockman, uint256 from_snapshot_blockhash)
CChainState::CChainState(CTxMemPool& mempool, BlockManager& blockman, std::optional<uint256> from_snapshot_blockhash)
: m_mempool(mempool),
m_blockman(blockman),
m_from_snapshot_blockhash(from_snapshot_blockhash) {}
Expand All @@ -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<CoinsViews>(
Expand Down Expand Up @@ -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))));
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -4662,10 +4662,10 @@ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pin
return std::min<double>(pindex->nChainTx / fTxTotal, 1.0);
}

std::optional<uint256> ChainstateManager::SnapshotBlockhash() const {
std::optional<uint256> 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;
}
Expand All @@ -4688,9 +4688,9 @@ std::vector<CChainState*> ChainstateManager::GetAll()
return out;
}

CChainState& ChainstateManager::InitializeChainstate(CTxMemPool& mempool, const uint256& snapshot_blockhash)
CChainState& ChainstateManager::InitializeChainstate(CTxMemPool& mempool, const std::optional<uint256>& snapshot_blockhash)
{
bool is_snapshot = !snapshot_blockhash.IsNull();
bool is_snapshot = snapshot_blockhash.has_value();
std::unique_ptr<CChainState>& to_modify =
is_snapshot ? m_snapshot_chainstate : m_ibd_chainstate;

Expand Down
8 changes: 4 additions & 4 deletions src/validation.h
Expand Up @@ -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<uint256> from_snapshot_blockhash = std::nullopt);

/**
* Initialize the CoinsViews UTXO set database management data structures. The in-memory
Expand Down Expand Up @@ -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<uint256> m_from_snapshot_blockhash;

/**
* The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for itself and all ancestors) and
Expand Down Expand Up @@ -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<uint256>& snapshot_blockhash = std::nullopt)
EXCLUSIVE_LOCKS_REQUIRED(::cs_main);

//! Get all chainstates currently being used.
Expand Down

0 comments on commit fa340b8

Please sign in to comment.