diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -124,7 +124,7 @@ return false; } - if (IsBlockFinalized(pindex)) { + if (::ChainstateActive().IsBlockFinalized(pindex)) { // There is no point polling finalized block. return false; } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -219,7 +219,8 @@ .Check(request); LOCK(cs_main); - const CBlockIndex *blockIndexFinalized = GetFinalizedBlock(); + const CBlockIndex *blockIndexFinalized = + ::ChainstateActive().GetFinalizedBlock(); if (blockIndexFinalized) { return blockIndexFinalized->GetBlockHash().GetHex(); } diff --git a/src/test/finalization_tests.cpp b/src/test/finalization_tests.cpp --- a/src/test/finalization_tests.cpp +++ b/src/test/finalization_tests.cpp @@ -23,7 +23,7 @@ LOCK(cs_main); // We should have no finalized block because the 100 blocks generated by // the test setup are too close to "now"; - BOOST_CHECK_MESSAGE(GetFinalizedBlock() == nullptr, + BOOST_CHECK_MESSAGE(::ChainstateActive().GetFinalizedBlock() == nullptr, "No block finalized (tip at height " << ::ChainActive().Tip()->nHeight << ")"); } @@ -35,7 +35,7 @@ block = CreateAndProcessBlock({}, p2pk_scriptPubKey); LOCK(cs_main); // These blocks are too recent. - BOOST_CHECK_MESSAGE(GetFinalizedBlock() == nullptr, + BOOST_CHECK_MESSAGE(::ChainstateActive().GetFinalizedBlock() == nullptr, "No block finalized (tip at height " << ::ChainActive().Tip()->nHeight << ")"); } @@ -52,11 +52,11 @@ blockToFinalize = ::ChainActive().Next(blockToFinalize); block = CreateAndProcessBlock({}, p2pk_scriptPubKey); LOCK(cs_main); - BOOST_CHECK_MESSAGE(GetFinalizedBlock() == blockToFinalize, - "Block finalized at height " - << blockToFinalize->nHeight - << " (tip at height " - << ::ChainActive().Tip()->nHeight << ")"); + BOOST_CHECK_MESSAGE( + ::ChainstateActive().GetFinalizedBlock() == blockToFinalize, + "Block finalized at height " + << blockToFinalize->nHeight << " (tip at height " + << ::ChainActive().Tip()->nHeight << ")"); } // Next blocks won't cause auto-finalization because the delay is not @@ -65,11 +65,11 @@ block = CreateAndProcessBlock({}, p2pk_scriptPubKey); LOCK(cs_main); // These blocks are finalized. - BOOST_CHECK_MESSAGE(GetFinalizedBlock() == blockToFinalize, - "Finalized block remains unchanged at height " - << blockToFinalize->nHeight - << " (tip at height " - << ::ChainActive().Tip()->nHeight << ")"); + BOOST_CHECK_MESSAGE( + ::ChainstateActive().GetFinalizedBlock() == blockToFinalize, + "Finalized block remains unchanged at height " + << blockToFinalize->nHeight << " (tip at height " + << ::ChainActive().Tip()->nHeight << ")"); } // Make the finalization time to expire @@ -85,11 +85,11 @@ blockToFinalize = ::ChainActive().Next(blockToFinalize); block = CreateAndProcessBlock({}, p2pk_scriptPubKey); LOCK(cs_main); - BOOST_CHECK_MESSAGE(GetFinalizedBlock() == blockToFinalize, - "Block finalized at height " - << blockToFinalize->nHeight - << " (tip at height " - << ::ChainActive().Tip()->nHeight << ")"); + BOOST_CHECK_MESSAGE( + ::ChainstateActive().GetFinalizedBlock() == blockToFinalize, + "Block finalized at height " + << blockToFinalize->nHeight << " (tip at height " + << ::ChainActive().Tip()->nHeight << ")"); } } diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -813,13 +813,18 @@ //! easily as opposed to referencing a global. BlockManager &m_blockman; + /** + * The best finalized block. + * This block cannot be reorged in any way except by explicit user action. + */ + const CBlockIndex *m_finalizedBlockIndex GUARDED_BY(cs_main) = nullptr; + public: explicit CChainState(BlockManager &blockman) : m_blockman(blockman) {} //! The current chain of blockheaders we consult and build on. //! @see CChain, CBlockIndex. CChain m_chain; - CBlockIndex const *pindexFinalized = nullptr; /** * The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for * itself and all ancestors) and as good as our current tip or better. @@ -885,6 +890,7 @@ bool ParkBlock(const Config &config, BlockValidationState &state, CBlockIndex *pindex) LOCKS_EXCLUDED(cs_main, m_cs_chainstate); + /** * Finalize a block. * A finalized block can not be reorged in any way. @@ -892,6 +898,15 @@ bool FinalizeBlock(const Config &config, BlockValidationState &state, CBlockIndex *pindex) LOCKS_EXCLUDED(cs_main, m_cs_chainstate); + /** Return the currently finalized block index. */ + const CBlockIndex *GetFinalizedBlock() const + EXCLUSIVE_LOCKS_REQUIRED(cs_main); + /** + * Checks if a block is finalized. + */ + bool IsBlockFinalized(const CBlockIndex *pindex) const + EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void ResetBlockFailureFlags(CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); template @@ -910,7 +925,7 @@ void PruneBlockIndexCandidates(); - void UnloadBlockIndex(); + void UnloadBlockIndex() EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** * Check whether we are doing an initial block download (synchronizing from @@ -979,17 +994,6 @@ /** Remove parked status from a block. */ void UnparkBlock(CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); -/** - * Retrieve the topmost finalized block. - */ -const CBlockIndex *GetFinalizedBlock() EXCLUSIVE_LOCKS_REQUIRED(cs_main); - -/** - * Checks if a block is finalized. - */ -bool IsBlockFinalized(const CBlockIndex *pindex) - EXCLUSIVE_LOCKS_REQUIRED(cs_main); - /** @returns the most-work valid chainstate. */ CChainState &ChainstateActive(); diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -115,11 +115,6 @@ namespace { CBlockIndex *pindexBestInvalid = nullptr; CBlockIndex *pindexBestParked = nullptr; -/** - * The best finalized block. - * This block cannot be reorged in any way except by explicit user action. - */ -CBlockIndex const *&pindexFinalized = ::ChainstateActive().pindexFinalized; RecursiveMutex cs_LastBlockFile; std::vector vinfoBlockFile; @@ -940,7 +935,7 @@ // If the invalid chain found is supposed to be finalized, we need to move // back the finalization point. if (IsBlockFinalized(pindexNew)) { - pindexFinalized = pindexNew->pprev; + m_finalizedBlockIndex = pindexNew->pprev; } LogPrintf("%s: invalid block=%s height=%d log2_work=%.8g date=%s\n", @@ -2157,8 +2152,8 @@ } // If the tip is finalized, then undo it. - if (pindexFinalized == pindexDelete) { - pindexFinalized = pindexDelete->pprev; + if (m_finalizedBlockIndex == pindexDelete) { + m_finalizedBlockIndex = pindexDelete->pprev; } m_chain.SetTip(pindexDelete->pprev); @@ -2259,7 +2254,8 @@ } // Check that the request is consistent with current finalization. - if (pindexFinalized && !AreOnTheSameFork(pindex, pindexFinalized)) { + if (m_finalizedBlockIndex && + !AreOnTheSameFork(pindex, m_finalizedBlockIndex)) { LogPrintf("ERROR: %s: Trying to finalize block %s which conflicts with " "already finalized block\n", __func__, pindex->GetBlockHash().ToString()); @@ -2274,7 +2270,7 @@ } // We have a new block to finalize. - pindexFinalized = pindex; + m_finalizedBlockIndex = pindex; return true; } @@ -2306,7 +2302,7 @@ // While our candidate is not eligible (finalization delay not expired), try // the previous one. - while (pindex && (pindex != pindexFinalized)) { + while (pindex && (pindex != ::ChainstateActive().GetFinalizedBlock())) { // Check that the block to finalize is known for a long enough time. // This test will ensure that an attacker could not cause a block to // finalize by forking the chain with a depth > maxreorgdepth. @@ -2476,11 +2472,12 @@ // If this block will cause a finalized block to be reorged, then we // mark it as invalid. - if (pindexFinalized && !AreOnTheSameFork(pindexNew, pindexFinalized)) { + if (m_finalizedBlockIndex && + !AreOnTheSameFork(pindexNew, m_finalizedBlockIndex)) { LogPrintf("Mark block %s invalid because it forks prior to the " "finalization point %d.\n", pindexNew->GetBlockHash().ToString(), - pindexFinalized->nHeight); + m_finalizedBlockIndex->nHeight); pindexNew->nStatus = pindexNew->nStatus.withFailed(); InvalidChainFound(pindexNew); } @@ -3281,8 +3278,9 @@ // In case we are reconsidering something before the finalization point, // move the finalization point to the last common ancestor. - if (pindexFinalized) { - pindexFinalized = LastCommonAncestor(pindex, pindexFinalized); + if (m_finalizedBlockIndex) { + m_finalizedBlockIndex = + LastCommonAncestor(pindex, m_finalizedBlockIndex); } UpdateFlags( @@ -3327,15 +3325,16 @@ return ::ChainstateActive().UnparkBlockImpl(pindex, false); } -const CBlockIndex *GetFinalizedBlock() { +bool CChainState::IsBlockFinalized(const CBlockIndex *pindex) const { AssertLockHeld(cs_main); - return pindexFinalized; + return m_finalizedBlockIndex && + m_finalizedBlockIndex->GetAncestor(pindex->nHeight) == pindex; } -bool IsBlockFinalized(const CBlockIndex *pindex) { +/** Return the currently finalized block index. */ +const CBlockIndex *CChainState::GetFinalizedBlock() const { AssertLockHeld(cs_main); - return pindexFinalized && - pindexFinalized->GetAncestor(pindex->nHeight) == pindex; + return m_finalizedBlockIndex; } CBlockIndex *BlockManager::AddToBlockIndex(const CBlockHeader &block) { @@ -5011,6 +5010,9 @@ void CChainState::UnloadBlockIndex() { nBlockSequenceId = 1; setBlockIndexCandidates.clear(); + + // Do not point to CBlockIndex that will be free'd + m_finalizedBlockIndex = nullptr; } // May NOT be used after any connections are up as much @@ -5020,7 +5022,6 @@ LOCK(cs_main); ::ChainActive().SetTip(nullptr); g_blockman.Unload(); - pindexFinalized = nullptr; pindexBestInvalid = nullptr; pindexBestParked = nullptr; pindexBestHeader = nullptr;