diff options
| -rw-r--r-- | src/node/chainstate.cpp | 2 | ||||
| -rw-r--r-- | src/test/validation_chainstatemanager_tests.cpp | 6 | ||||
| -rw-r--r-- | src/validation.cpp | 17 | ||||
| -rw-r--r-- | src/validation.h | 34 |
4 files changed, 36 insertions, 23 deletions
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 255d8be0ec..0828f64856 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -221,7 +221,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize // A reload of the block index is required to recompute setBlockIndexCandidates // for the fully validated chainstate. - chainman.ActiveChainstate().UnloadBlockIndex(); + chainman.ActiveChainstate().ClearBlockIndexCandidates(); auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options); if (init_status != ChainstateLoadStatus::SUCCESS) { diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 99860961a2..442c7036e6 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -433,9 +433,13 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup) CBlockIndex* assumed_tip{WITH_LOCK(chainman.GetMutex(), return chainman.ActiveChain().Tip())}; auto reload_all_block_indexes = [&]() { + // For completeness, we also reset the block sequence counters to + // ensure that no state which affects the ranking of tip-candidates is + // retained (even though this isn't strictly necessary). + WITH_LOCK(::cs_main, return chainman.ResetBlockSequenceCounters()); for (Chainstate* cs : chainman.GetAll()) { LOCK(::cs_main); - cs->UnloadBlockIndex(); + cs->ClearBlockIndexCandidates(); BOOST_CHECK(cs->setBlockIndexCandidates.empty()); } diff --git a/src/validation.cpp b/src/validation.cpp index a3b9c53d8d..cdf19e72a1 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3213,17 +3213,17 @@ bool Chainstate::PreciousBlock(BlockValidationState& state, CBlockIndex* pindex) // Nothing to do, this block is not at the tip. return true; } - if (m_chain.Tip()->nChainWork > nLastPreciousChainwork) { + if (m_chain.Tip()->nChainWork > m_chainman.nLastPreciousChainwork) { // The chain has been extended since the last call, reset the counter. - nBlockReverseSequenceId = -1; + m_chainman.nBlockReverseSequenceId = -1; } - nLastPreciousChainwork = m_chain.Tip()->nChainWork; + m_chainman.nLastPreciousChainwork = m_chain.Tip()->nChainWork; setBlockIndexCandidates.erase(pindex); - pindex->nSequenceId = nBlockReverseSequenceId; - if (nBlockReverseSequenceId > std::numeric_limits<int32_t>::min()) { + pindex->nSequenceId = m_chainman.nBlockReverseSequenceId; + if (m_chainman.nBlockReverseSequenceId > std::numeric_limits<int32_t>::min()) { // We can't keep reducing the counter if somebody really wants to // call preciousblock 2**31-1 times on the same set of tips... - nBlockReverseSequenceId--; + m_chainman.nBlockReverseSequenceId--; } if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && pindex->HaveTxsDownloaded()) { setBlockIndexCandidates.insert(pindex); @@ -3442,7 +3442,7 @@ void Chainstate::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pin CBlockIndex *pindex = queue.front(); queue.pop_front(); pindex->nChainTx = (pindex->pprev ? pindex->pprev->nChainTx : 0) + pindex->nTx; - pindex->nSequenceId = nBlockSequenceId++; + pindex->nSequenceId = m_chainman.nBlockSequenceId++; if (m_chain.Tip() == nullptr || !setBlockIndexCandidates.value_comp()(pindex, m_chain.Tip())) { setBlockIndexCandidates.insert(pindex); } @@ -4379,10 +4379,9 @@ bool Chainstate::NeedsRedownload() const return false; } -void Chainstate::UnloadBlockIndex() +void Chainstate::ClearBlockIndexCandidates() { AssertLockHeld(::cs_main); - nBlockSequenceId = 1; setBlockIndexCandidates.clear(); } diff --git a/src/validation.h b/src/validation.h index af8ceb5dfa..d34e31d1de 100644 --- a/src/validation.h +++ b/src/validation.h @@ -466,17 +466,6 @@ class Chainstate { protected: /** - * Every received block is assigned a unique and increasing identifier, so we - * know which one to give priority in case of a fork. - */ - /** Blocks loaded from disk are assigned id 0, so start the counter at 1. */ - int32_t nBlockSequenceId GUARDED_BY(::cs_main) = 1; - /** Decreasing counter (used by subsequent preciousblock calls). */ - int32_t nBlockReverseSequenceId = -1; - /** chainwork for the last block that preciousblock has been applied to. */ - arith_uint256 nLastPreciousChainwork = 0; - - /** * The ChainState Mutex * A lock that must be held when modifying this ChainState - held in ActivateBestChain() and * InvalidateBlock() @@ -740,7 +729,7 @@ public: void PruneBlockIndexCandidates(); - void UnloadBlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void ClearBlockIndexCandidates() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Check whether we are doing an initial block download (synchronizing from disk or network) */ bool IsInitialBlockDownload() const; @@ -991,6 +980,27 @@ public: node::BlockManager m_blockman; /** + * Every received block is assigned a unique and increasing identifier, so we + * know which one to give priority in case of a fork. + */ + /** Blocks loaded from disk are assigned id 0, so start the counter at 1. */ + int32_t nBlockSequenceId GUARDED_BY(::cs_main) = 1; + /** Decreasing counter (used by subsequent preciousblock calls). */ + int32_t nBlockReverseSequenceId = -1; + /** chainwork for the last block that preciousblock has been applied to. */ + arith_uint256 nLastPreciousChainwork = 0; + + // Reset the memory-only sequence counters we use to track block arrival + // (used by tests to reset state) + void ResetBlockSequenceCounters() EXCLUSIVE_LOCKS_REQUIRED(::cs_main) + { + AssertLockHeld(::cs_main); + nBlockSequenceId = 1; + nBlockReverseSequenceId = -1; + } + + + /** * In order to efficiently track invalidity of headers, we keep the set of * blocks which we tried to connect and found to be invalid here (ie which * were set to BLOCK_FAILED_VALID since the last restart). We can then |