diff --git a/src/chain.cpp b/src/chain.cpp index 46a51ac8bd9..b066f0f7c68 100644 --- a/src/chain.cpp +++ b/src/chain.cpp @@ -47,13 +47,12 @@ CBlockLocator GetLocator(const CBlockIndex* index) return CBlockLocator{LocatorEntries(index)}; } -const CBlockIndex *CChain::FindFork(const CBlockIndex *pindex) const { - if (pindex == nullptr) { - return nullptr; - } +const CBlockIndex* CChain::FindFork(const CBlockIndex& index) const +{ + const auto* pindex{&index}; if (pindex->nHeight > Height()) pindex = pindex->GetAncestor(Height()); - while (pindex && !Contains(pindex)) + while (pindex && !Contains(*pindex)) pindex = pindex->pprev; return pindex; } diff --git a/src/chain.h b/src/chain.h index c27829208c4..7701e9262a4 100644 --- a/src/chain.h +++ b/src/chain.h @@ -407,16 +407,16 @@ public: } /** Efficiently check whether a block is present in this chain. */ - bool Contains(const CBlockIndex* pindex) const + bool Contains(const CBlockIndex& index) const { - return (*this)[pindex->nHeight] == pindex; + return (*this)[index.nHeight] == &index; } /** Find the successor of a block in this chain, or nullptr if the given index is not found or is the tip. */ - CBlockIndex* Next(const CBlockIndex* pindex) const + CBlockIndex* Next(const CBlockIndex& index) const { - if (Contains(pindex)) - return (*this)[pindex->nHeight + 1]; + if (Contains(index)) + return (*this)[index.nHeight + 1]; else return nullptr; } @@ -440,7 +440,7 @@ public: void SetTip(CBlockIndex& block); /** Find the last common block between this chain and a block index entry. */ - const CBlockIndex* FindFork(const CBlockIndex* pindex) const; + const CBlockIndex* FindFork(const CBlockIndex& index) const; /** Find the earliest block with timestamp equal or greater than the given time and height equal or greater than the given height. */ CBlockIndex* FindEarliestAtLeast(int64_t nTime, int height) const; diff --git a/src/index/base.cpp b/src/index/base.cpp index 9e75109d5c9..bb41cc2a632 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -30,7 +31,6 @@ #include #include -#include #include #include #include @@ -147,7 +147,7 @@ bool BaseIndex::Init() return true; } -static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain& chain) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static const CBlockIndex* NextSyncBlock(const CBlockIndex* const pindex_prev, CChain& chain) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); @@ -155,7 +155,7 @@ static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain& return chain.Genesis(); } - if (const auto* pindex{chain.Next(pindex_prev)}) { + if (const auto* pindex{chain.Next(*pindex_prev)}) { return pindex; } @@ -166,7 +166,9 @@ static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain& // Since block is not in the chain, return the next block in the chain AFTER the last common ancestor. // Caller will be responsible for rewinding back to the common ancestor. - return chain.Next(chain.FindFork(pindex_prev)); + const auto* fork{chain.FindFork(*pindex_prev)}; + // Common ancestor must exist (genesis). + return chain.Next(*Assert(fork)); } bool BaseIndex::ProcessBlock(const CBlockIndex* pindex, const CBlock* block_data) diff --git a/src/init.cpp b/src/init.cpp index a9445754288..c53e5ed634c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2371,8 +2371,11 @@ bool StartIndexBackgroundSync(NodeContext& node) { LOCK(::cs_main); pindex = chainman.m_blockman.LookupBlockIndex(summary.best_block_hash); - if (!index_chain.Contains(pindex)) { - pindex = index_chain.FindFork(pindex); + if (!pindex) { + LogWarning("Failed to find block manager entry for best block %s from %s, falling back to genesis for index sync", + summary.best_block_hash.ToString(), summary.name); + } else if (!index_chain.Contains(*pindex)) { + pindex = index_chain.FindFork(*pindex); } } if (!pindex) { diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp index 4ba60c1b56c..ffd8ae029b9 100644 --- a/src/kernel/bitcoinkernel.cpp +++ b/src/kernel/bitcoinkernel.cpp @@ -1367,7 +1367,7 @@ const btck_BlockTreeEntry* btck_chain_get_by_height(const btck_Chain* chain, int int btck_chain_contains(const btck_Chain* chain, const btck_BlockTreeEntry* entry) { LOCK(::cs_main); - return btck_Chain::get(chain).Contains(&btck_BlockTreeEntry::get(entry)) ? 1 : 0; + return btck_Chain::get(chain).Contains(btck_BlockTreeEntry::get(entry)) ? 1 : 0; } btck_BlockHeader* btck_block_header_create(const void* raw_block_header, size_t raw_block_header_len) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e4b218d4ecf..6d35db931a5 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1503,7 +1503,7 @@ void PeerManagerImpl::FindNextBlocks(std::vector& vBlocks, c return; } - if (pindex->nStatus & BLOCK_HAVE_DATA || (activeChain && activeChain->Contains(pindex))) { + if (pindex->nStatus & BLOCK_HAVE_DATA || (activeChain && activeChain->Contains(*pindex))) { if (activeChain && pindex->HaveNumChainTxs()) { state->pindexLastCommonBlock = pindex; } @@ -1952,7 +1952,7 @@ void PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex& block_index) { AssertLockHeld(cs_main); - if (m_chainman.ActiveChain().Contains(&block_index)) return true; + if (m_chainman.ActiveChain().Contains(block_index)) return true; return block_index.IsValid(BLOCK_VALID_SCRIPTS) && (m_chainman.m_best_header != nullptr) && (m_chainman.m_best_header->GetBlockTime() - block_index.GetBlockTime() < STALE_RELAY_AGE_LIMIT) && (GetBlockProofEquivalentTime(*m_chainman.m_best_header, block_index, *m_chainman.m_best_header, m_chainparams.GetConsensus()) < STALE_RELAY_AGE_LIMIT); @@ -2815,7 +2815,7 @@ bool PeerManagerImpl::IsAncestorOfBestHeaderOrTip(const CBlockIndex* header) return false; } else if (m_chainman.m_best_header != nullptr && header == m_chainman.m_best_header->GetAncestor(header->nHeight)) { return true; - } else if (m_chainman.ActiveChain().Contains(header)) { + } else if (m_chainman.ActiveChain().Contains(*header)) { return true; } return false; @@ -2849,7 +2849,7 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c std::vector vToFetch; const CBlockIndex* pindexWalk{&last_header}; // Calculate all the blocks we'd need to switch to last_header, up to a limit. - while (pindexWalk && !m_chainman.ActiveChain().Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { + while (pindexWalk && !m_chainman.ActiveChain().Contains(*pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && !IsBlockRequested(pindexWalk->GetBlockHash()) && (!DeploymentActiveAt(*pindexWalk, m_chainman, Consensus::DEPLOYMENT_SEGWIT) || CanServeWitnesses(peer))) { @@ -2862,7 +2862,8 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c // very large reorg at a time we think we're close to caught up to // the main chain -- this shouldn't really happen. Bail out on the // direct fetch and rely on parallel download instead. - if (!m_chainman.ActiveChain().Contains(pindexWalk)) { + // Common ancestor must exist (genesis). + if (!m_chainman.ActiveChain().Contains(*Assert(pindexWalk))) { LogDebug(BCLog::NET, "Large reorg, won't direct fetch to %s (%d)\n", last_header.GetBlockHash().ToString(), last_header.nHeight); @@ -4214,10 +4215,10 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string // Send the rest of the chain if (pindex) - pindex = m_chainman.ActiveChain().Next(pindex); + pindex = m_chainman.ActiveChain().Next(*pindex); int nLimit = 500; LogDebug(BCLog::NET, "getblocks %d to %s limit %d from peer=%d\n", (pindex ? pindex->nHeight : -1), hashStop.IsNull() ? "end" : hashStop.ToString(), nLimit, pfrom.GetId()); - for (; pindex; pindex = m_chainman.ActiveChain().Next(pindex)) + for (; pindex; pindex = m_chainman.ActiveChain().Next(*pindex)) { if (pindex->GetBlockHash() == hashStop) { @@ -4353,14 +4354,14 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string // Find the last block the caller has in the main chain pindex = m_chainman.ActiveChainstate().FindForkInGlobalIndex(locator); if (pindex) - pindex = m_chainman.ActiveChain().Next(pindex); + pindex = m_chainman.ActiveChain().Next(*pindex); } // we must use CBlocks, as CBlockHeaders won't include the 0x00 nTx count at the end std::vector vHeaders; int nLimit = m_opts.max_headers_result; LogDebug(BCLog::NET, "getheaders %d to %s from peer=%d\n", (pindex ? pindex->nHeight : -1), hashStop.IsNull() ? "end" : hashStop.ToString(), pfrom.GetId()); - for (; pindex; pindex = m_chainman.ActiveChain().Next(pindex)) + for (; pindex; pindex = m_chainman.ActiveChain().Next(*pindex)) { vHeaders.emplace_back(pindex->GetBlockHeader()); if (--nLimit <= 0 || pindex->GetBlockHash() == hashStop) diff --git a/src/rest.cpp b/src/rest.cpp index 53dd3d16119..d2c5a9b4899 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -225,12 +225,12 @@ static bool rest_headers(const std::any& context, CChain& active_chain = chainman.ActiveChain(); tip = active_chain.Tip(); const CBlockIndex* pindex{chainman.m_blockman.LookupBlockIndex(*hash)}; - while (pindex != nullptr && active_chain.Contains(pindex)) { + while (pindex != nullptr && active_chain.Contains(*pindex)) { headers.push_back(pindex); if (headers.size() == *parsed_count) { break; } - pindex = active_chain.Next(pindex); + pindex = active_chain.Next(*pindex); } } @@ -552,11 +552,11 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const LOCK(cs_main); CChain& active_chain = chainman.ActiveChain(); const CBlockIndex* pindex{chainman.m_blockman.LookupBlockIndex(*block_hash)}; - while (pindex != nullptr && active_chain.Contains(pindex)) { + while (pindex != nullptr && active_chain.Contains(*pindex)) { headers.push_back(pindex); if (headers.size() == *parsed_count) break; - pindex = active_chain.Next(pindex); + pindex = active_chain.Next(*pindex); } } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index deb2b42e85d..14412c90fcc 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1631,7 +1631,7 @@ static RPCMethod getchaintips() std::set setPrevs; for (const auto& [_, block_index] : chainman.BlockIndex()) { - if (!active_chain.Contains(&block_index)) { + if (!active_chain.Contains(block_index)) { setOrphans.insert(&block_index); setPrevs.insert(block_index.pprev); } @@ -1649,15 +1649,16 @@ static RPCMethod getchaintips() /* Construct the output array. */ UniValue res(UniValue::VARR); for (const CBlockIndex* block : setTips) { + CHECK_NONFATAL(block); UniValue obj(UniValue::VOBJ); obj.pushKV("height", block->nHeight); obj.pushKV("hash", block->phashBlock->GetHex()); - const int branchLen = block->nHeight - active_chain.FindFork(block)->nHeight; + const int branchLen = block->nHeight - active_chain.FindFork(*block)->nHeight; obj.pushKV("branchlen", branchLen); std::string status; - if (active_chain.Contains(block)) { + if (active_chain.Contains(*block)) { // This block is part of the currently active chain. status = "active"; } else if (block->nStatus & BLOCK_FAILED_VALID) { @@ -1866,7 +1867,7 @@ static RPCMethod getchaintxstats() if (!pindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } - if (!chainman.ActiveChain().Contains(pindex)) { + if (!chainman.ActiveChain().Contains(*pindex)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Block is not in main chain"); } } @@ -2804,7 +2805,7 @@ static RPCMethod getdescriptoractivity() if (!pindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } - if (!chainman.ActiveChain().Contains(pindex)) { + if (!chainman.ActiveChain().Contains(*pindex)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Block is not in main chain"); } blockindexes_sorted.insert(pindex); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 74652c17421..a0f93f0c79c 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -73,7 +73,7 @@ static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry.pushKV("blockhash", hashBlock.GetHex()); const CBlockIndex* pindex = active_chainstate.m_blockman.LookupBlockIndex(hashBlock); if (pindex) { - if (active_chainstate.m_chain.Contains(pindex)) { + if (active_chainstate.m_chain.Contains(*pindex)) { entry.pushKV("confirmations", 1 + active_chainstate.m_chain.Height() - pindex->nHeight); entry.pushKV("time", pindex->GetBlockTime()); entry.pushKV("blocktime", pindex->GetBlockTime()); @@ -336,7 +336,7 @@ static RPCMethod getrawtransaction() UniValue result(UniValue::VOBJ); if (blockindex) { LOCK(cs_main); - result.pushKV("in_active_chain", chainman.ActiveChain().Contains(blockindex)); + result.pushKV("in_active_chain", chainman.ActiveChain().Contains(*blockindex)); } // If request is verbosity >= 1 but no blockhash was given, then look up the blockindex if (request.params[2].isNull()) { diff --git a/src/rpc/txoutproof.cpp b/src/rpc/txoutproof.cpp index f68f53f665d..628825fc80e 100644 --- a/src/rpc/txoutproof.cpp +++ b/src/rpc/txoutproof.cpp @@ -158,7 +158,7 @@ static RPCMethod verifytxoutproof() LOCK(cs_main); const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(merkleBlock.header.GetHash()); - if (!pindex || !chainman.ActiveChain().Contains(pindex) || pindex->nTx == 0) { + if (!pindex || !chainman.ActiveChain().Contains(*pindex) || pindex->nTx == 0) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain"); } diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index f5a18fdd773..10e4ffa4b08 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -133,7 +133,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) for (const CBlockIndex* block_index = m_node.chainman->ActiveChain().Genesis(); block_index != nullptr; - block_index = m_node.chainman->ActiveChain().Next(block_index)) { + block_index = m_node.chainman->ActiveChain().Next(*block_index)) { BOOST_CHECK(!filter_index.LookupFilter(block_index, filter)); BOOST_CHECK(!filter_index.LookupFilterHeader(block_index, filter_header)); BOOST_CHECK(!filter_index.LookupFilterRange(block_index->nHeight, block_index, filters)); @@ -153,7 +153,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) const CBlockIndex* block_index; for (block_index = m_node.chainman->ActiveChain().Genesis(); block_index != nullptr; - block_index = m_node.chainman->ActiveChain().Next(block_index)) { + block_index = m_node.chainman->ActiveChain().Next(*block_index)) { CheckFilterLookups(filter_index, block_index, last_header, m_node.chainman->m_blockman); } } diff --git a/src/test/chain_tests.cpp b/src/test/chain_tests.cpp index a782e880f9f..966802b65ad 100644 --- a/src/test/chain_tests.cpp +++ b/src/test/chain_tests.cpp @@ -7,6 +7,7 @@ #include #include +#include #include BOOST_FIXTURE_TEST_SUITE(chain_tests, BasicTestingSetup) @@ -41,6 +42,106 @@ const CBlockIndex* NaiveLastCommonAncestor(const CBlockIndex* a, const CBlockInd } // namespace +BOOST_AUTO_TEST_CASE(basic_tests) +{ + // An empty chain + const CChain chain_0; + // A chain with 2 blocks + CChain chain_2; + + CBlockIndex genesis; + genesis.nHeight = 0; + chain_2.SetTip(genesis); + + CBlockIndex bi1; + bi1.pprev = &genesis; + bi1.nHeight = 1; + chain_2.SetTip(bi1); + + BOOST_CHECK_EQUAL(chain_0.Height(), -1); + BOOST_CHECK_EQUAL(chain_2.Height(), 1); + + BOOST_CHECK_EQUAL(chain_0.Tip(), nullptr); + BOOST_CHECK_EQUAL(chain_2.Tip(), &bi1); + + // Indexer accessor: call with valid and invalid (low & high) values + BOOST_CHECK_EQUAL(chain_2[-1], nullptr); + BOOST_CHECK_EQUAL(chain_2[0], &genesis); + BOOST_CHECK_EQUAL(chain_2[1], &bi1); + BOOST_CHECK_EQUAL(chain_2[2], nullptr); + + // Contains: call with contained & non-contained blocks + BOOST_CHECK(chain_2.Contains(genesis)); + BOOST_CHECK(chain_2.Contains(bi1)); + BOOST_CHECK(!chain_0.Contains(genesis)); + + // Call with non-tip & tip blocks + BOOST_CHECK_EQUAL(chain_2.Next(genesis), &bi1); + BOOST_CHECK_EQUAL(chain_2.Next(bi1), nullptr); + BOOST_CHECK_EQUAL(chain_0.Next(genesis), nullptr); + + BOOST_CHECK_EQUAL(chain_2.Genesis(), &genesis); + BOOST_CHECK_EQUAL(chain_0.Genesis(), nullptr); +} + +BOOST_AUTO_TEST_CASE(findfork_tests) +{ + // Create a forking chain + const auto init_branch{[](auto& blocks, CBlockIndex* parent, int start_height) { + for (size_t i{0}; i < blocks.size(); ++i) { + blocks[i].pprev = i == 0 ? parent : &blocks[i - 1]; + blocks[i].nHeight = start_height + i; + } + }}; + + const auto check_same{[](const CChain& chain, const auto& blocks) { + for (const auto& block : blocks) { + BOOST_CHECK_EQUAL(chain.FindFork(block), &block); + } + }}; + + const auto check_fork_point{[](const CChain& chain, const auto& blocks, const CBlockIndex& fork_point) { + for (const auto& block : blocks) { + BOOST_CHECK_EQUAL(chain.FindFork(block), &fork_point); + } + }}; + + std::array blocks_common; + init_branch(blocks_common, nullptr, 0); + + std::array blocks_long; + init_branch(blocks_long, &blocks_common.back(), blocks_common.size()); + + std::array blocks_short; + init_branch(blocks_short, &blocks_common.back(), blocks_common.size()); + + // Create a chain with the longer fork + CChain chain_long; + chain_long.SetTip(blocks_long.back()); + BOOST_CHECK_EQUAL(chain_long.Height(), 10 + 10 - 1); + // Test the blocks in the common part -> result should be the same + check_same(chain_long, blocks_common); + // Test the blocks on the longer fork -> result should be the same + check_same(chain_long, blocks_long); + // Test the blocks on the other shorter fork -> result should be the fork point + check_fork_point(chain_long, blocks_short, blocks_common.back()); + + // Create a chain with the shorter fork + CChain chain_short; + chain_short.SetTip(blocks_short.back()); + BOOST_CHECK_EQUAL(chain_short.Height(), 10 + 5 - 1); + // Test the blocks in the common part -> result should be the same + check_same(chain_short, blocks_common); + // Test the blocks on the shorter fork -> result should be the same + check_same(chain_short, blocks_short); + // Test the blocks on the other longer fork -> result should be the fork point + check_fork_point(chain_short, blocks_long, blocks_common.back()); + + // Invalid test case. Mixing chains is not supported + CBlockIndex block_on_unrelated_chain; + BOOST_CHECK_EQUAL(chain_long.FindFork(block_on_unrelated_chain), nullptr); +} + BOOST_AUTO_TEST_CASE(chain_test) { FastRandomContext ctx; diff --git a/src/test/fuzz/block_index_tree.cpp b/src/test/fuzz/block_index_tree.cpp index 0e333cd6288..2eac6d6bd26 100644 --- a/src/test/fuzz/block_index_tree.cpp +++ b/src/test/fuzz/block_index_tree.cpp @@ -102,7 +102,7 @@ FUZZ_TARGET(block_index_tree, .init = initialize_block_index_tree) assert(best_tip); // Should at least return current tip if (best_tip == chain.Tip()) break; // Nothing to do // Rewind chain to forking point - const CBlockIndex* fork = chain.FindFork(best_tip); + const CBlockIndex* fork = chain.FindFork(*best_tip); // If we can't go back to the fork point due to pruned data, abort this run. In reality, a pruned node would also currently just crash in this scenario. // This is very unlikely to happen due to the minimum pruning threshold of 550MiB. CBlockIndex* it = chain.Tip(); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 5aefd65679f..a22cd2b199d 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -45,7 +45,7 @@ bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp) if (lp.maxInputBlock) { // Check whether active_chain is an extension of the block at which the LockPoints // calculation was valid. If not LockPoints are no longer valid - if (!active_chain.Contains(lp.maxInputBlock)) { + if (!active_chain.Contains(*lp.maxInputBlock)) { return false; } } diff --git a/src/validation.cpp b/src/validation.cpp index 62753c41ac8..f85a834f2a4 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -127,7 +127,7 @@ const CBlockIndex* Chainstate::FindForkInGlobalIndex(const CBlockLocator& locato for (const uint256& hash : locator.vHave) { const CBlockIndex* pindex{m_blockman.LookupBlockIndex(hash)}; if (pindex) { - if (m_chain.Contains(pindex)) { + if (m_chain.Contains(*pindex)) { return pindex; } if (pindex->GetAncestor(m_chain.Height()) == m_chain.Tip()) { @@ -3125,7 +3125,7 @@ CBlockIndex* Chainstate::FindMostWorkChain() // Check whether all blocks on the path between the currently active chain and the candidate are valid. // Just going until the active chain is an optimization, as we know all blocks in it are valid already. bool fInvalidAncestor = false; - for (CBlockIndex *pindexTest = pindexNew; pindexTest && !m_chain.Contains(pindexTest); pindexTest = pindexTest->pprev) { + for (CBlockIndex *pindexTest = pindexNew; pindexTest && !m_chain.Contains(*pindexTest); pindexTest = pindexTest->pprev) { assert(pindexTest->HaveNumChainTxs() || pindexTest->nHeight == 0); // Pruned nodes may have entries in setBlockIndexCandidates for @@ -3173,18 +3173,18 @@ void Chainstate::PruneBlockIndexCandidates() { } /** - * Try to make some progress towards making pindexMostWork the active block. - * pblock is either nullptr or a pointer to a CBlock corresponding to pindexMostWork. + * Try to make some progress towards making index_most_work the active block. + * pblock is either nullptr or a pointer to a CBlock corresponding to index_most_work. * * @returns true unless a system error occurred */ -bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, std::vector& connected_blocks) +bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex& index_most_work, const std::shared_ptr& pblock, bool& fInvalidFound, std::vector& connected_blocks) { AssertLockHeld(cs_main); if (m_mempool) AssertLockHeld(m_mempool->cs); const CBlockIndex* pindexOldTip = m_chain.Tip(); - const CBlockIndex* pindexFork = m_chain.FindFork(pindexMostWork); + const CBlockIndex* pindexFork = m_chain.FindFork(index_most_work); // Disconnect active blocks which are no longer in the best chain. bool fBlocksDisconnected = false; @@ -3208,13 +3208,13 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* std::vector vpindexToConnect; bool fContinue = true; int nHeight = pindexFork ? pindexFork->nHeight : -1; - while (fContinue && nHeight != pindexMostWork->nHeight) { + while (fContinue && nHeight != index_most_work.nHeight) { // Don't iterate the entire list of potential improvements toward the best tip, as we likely only need // a few blocks along the way. - int nTargetHeight = std::min(nHeight + 32, pindexMostWork->nHeight); + int nTargetHeight = std::min(nHeight + 32, index_most_work.nHeight); vpindexToConnect.clear(); vpindexToConnect.reserve(nTargetHeight - nHeight); - CBlockIndex* pindexIter = pindexMostWork->GetAncestor(nTargetHeight); + CBlockIndex* pindexIter = index_most_work.GetAncestor(nTargetHeight); while (pindexIter && pindexIter->nHeight != nHeight) { vpindexToConnect.push_back(pindexIter); pindexIter = pindexIter->pprev; @@ -3223,7 +3223,7 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* // Connect new blocks. for (CBlockIndex* pindexConnect : vpindexToConnect | std::views::reverse) { - if (!ConnectTip(state, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr(), connected_blocks, disconnectpool)) { + if (!ConnectTip(state, pindexConnect, pindexConnect == &index_most_work ? pblock : std::shared_ptr(), connected_blocks, disconnectpool)) { if (state.IsInvalid()) { // The block violates a consensus rule. if (state.GetResult() != BlockValidationResult::BLOCK_MUTATED) { @@ -3373,7 +3373,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< // in case snapshot validation is completed during ActivateBestChainStep, the // result of GetRole() changes from BACKGROUND to NORMAL. const ChainstateRole chainstate_role{this->GetRole()}; - if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connected_blocks)) { + if (!ActivateBestChainStep(state, *pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connected_blocks)) { // A system error occurred return false; } @@ -3398,7 +3398,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< } while (!m_chain.Tip() || (starting_tip && CBlockIndexWorkComparator()(m_chain.Tip(), starting_tip))); if (!blocks_connected) return true; - const CBlockIndex* pindexFork = m_chain.FindFork(starting_tip); + const CBlockIndex* pindexFork = starting_tip ? m_chain.FindFork(*starting_tip) : nullptr; bool still_in_ibd = m_chainman.IsInitialBlockDownload(); if (was_in_ibd && !still_in_ibd) { @@ -3508,7 +3508,7 @@ bool Chainstate::PreciousBlock(BlockValidationState& state, CBlockIndex* pindex) return ActivateBestChain(state, std::shared_ptr()); } -bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pindex) +bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* const pindex) { AssertLockNotHeld(m_chainstate_mutex); AssertLockNotHeld(::cs_main); @@ -3534,16 +3534,16 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde { LOCK(cs_main); for (auto& entry : m_blockman.m_block_index) { - CBlockIndex* candidate = &entry.second; + CBlockIndex& candidate = entry.second; // We don't need to put anything in our active chain into the // multimap, because those candidates will be found and considered // as we disconnect. // Instead, consider only non-active-chain blocks that score // at least as good with CBlockIndexWorkComparator as the new tip. if (!m_chain.Contains(candidate) && - !CBlockIndexWorkComparator()(candidate, pindex->pprev) && - !(candidate->nStatus & BLOCK_FAILED_VALID)) { - highpow_outofchain_headers.insert({candidate->nChainWork, candidate}); + !CBlockIndexWorkComparator()(&candidate, pindex->pprev) && + !(candidate.nStatus & BLOCK_FAILED_VALID)) { + highpow_outofchain_headers.insert({candidate.nChainWork, &candidate}); } } } @@ -3563,9 +3563,9 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde // Lock for as long as disconnectpool is in scope to make sure MaybeUpdateMempoolForReorg is // called after DisconnectTip without unlocking in between LOCK(MempoolMutex()); - if (!m_chain.Contains(pindex)) break; + if (!m_chain.Contains(*pindex)) break; pindex_was_in_chain = true; - CBlockIndex* disconnected_tip{m_chain.Tip()}; + CBlockIndex* const disconnected_tip{m_chain.Tip()}; // ActivateBestChain considers blocks already in m_chain // unconditionally valid already, so force disconnect away from it. @@ -3635,7 +3635,7 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde { LOCK(cs_main); - if (m_chain.Contains(to_mark_failed)) { + if (m_chain.Contains(*to_mark_failed)) { // If the to-be-marked invalid block is in the active chain, something is interfering and we can't proceed. return false; } @@ -4712,7 +4712,7 @@ VerifyDBResult CVerifyDB::VerifyDB( reportDone = percentageDone / 10; } m_notifications.progress(_("Verifying blocks…"), percentageDone, false); - pindex = chainstate.m_chain.Next(pindex); + pindex = chainstate.m_chain.Next(*pindex); CBlock block; if (!chainstate.m_blockman.ReadBlock(block, *pindex)) { LogError("Verification error: ReadBlock failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); @@ -5149,7 +5149,7 @@ void ChainstateManager::CheckBlockIndex() const std::multimap forward; for (auto& [_, block_index] : m_blockman.m_block_index) { // Only save indexes in forward that are not part of the best header chain. - if (!best_hdr_chain.Contains(&block_index)) { + if (!best_hdr_chain.Contains(block_index)) { // Only genesis, which must be part of the best header chain, can have a nullptr parent. assert(block_index.pprev); forward.emplace(block_index.pprev, &block_index); @@ -5384,7 +5384,7 @@ void ChainstateManager::CheckBlockIndex() const pindex = range.first->second; nHeight++; continue; - } else if (best_hdr_chain.Contains(pindex)) { + } else if (best_hdr_chain.Contains(*pindex)) { // Descend further into best header chain. nHeight++; pindex = best_hdr_chain[nHeight]; diff --git a/src/validation.h b/src/validation.h index e0f5e80c783..4cb5dc631e9 100644 --- a/src/validation.h +++ b/src/validation.h @@ -851,7 +851,7 @@ public: std::pair GetPruneRange(int last_height_can_prune) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); protected: - bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, std::vector& connected_blocks) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs); + bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex& index_most_work, const std::shared_ptr& pblock, bool& fInvalidFound, std::vector& connected_blocks) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs); bool ConnectTip( BlockValidationState& state, CBlockIndex* pindexNew,