From ed470940cddbeb40425960d51cefeec4948febe4 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 7 Aug 2022 20:56:17 -0400 Subject: Add functions to construct locators without CChain This introduces an insignificant performance penalty, as it means locator construction needs to use the skiplist-based CBlockIndex::GetAncestor() function instead of the lookup-based CChain, but avoids the need for callers to have access to a relevant CChain object. --- src/chain.cpp | 47 +++++++++++++++++++++++---------------------- src/chain.h | 10 ++++++++-- src/net_processing.cpp | 13 +++++++------ src/node/interfaces.cpp | 5 ++--- src/test/skiplist_tests.cpp | 2 +- 5 files changed, 42 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/src/chain.cpp b/src/chain.cpp index 446bb216c2..19c35b5012 100644 --- a/src/chain.cpp +++ b/src/chain.cpp @@ -28,32 +28,33 @@ void CChain::SetTip(CBlockIndex& block) } } -CBlockLocator CChain::GetLocator(const CBlockIndex *pindex) const { - int nStep = 1; - std::vector vHave; - vHave.reserve(32); - - if (!pindex) - pindex = Tip(); - while (pindex) { - vHave.push_back(pindex->GetBlockHash()); - // Stop when we have added the genesis block. - if (pindex->nHeight == 0) - break; +std::vector LocatorEntries(const CBlockIndex* index) +{ + int step = 1; + std::vector have; + if (index == nullptr) return have; + + have.reserve(32); + while (index) { + have.emplace_back(index->GetBlockHash()); + if (index->nHeight == 0) break; // Exponentially larger steps back, plus the genesis block. - int nHeight = std::max(pindex->nHeight - nStep, 0); - if (Contains(pindex)) { - // Use O(1) CChain index if possible. - pindex = (*this)[nHeight]; - } else { - // Otherwise, use O(log n) skiplist. - pindex = pindex->GetAncestor(nHeight); - } - if (vHave.size() > 10) - nStep *= 2; + int height = std::max(index->nHeight - step, 0); + // Use skiplist. + index = index->GetAncestor(height); + if (have.size() > 10) step *= 2; } + return have; +} - return CBlockLocator(vHave); +CBlockLocator GetLocator(const CBlockIndex* index) +{ + return CBlockLocator{std::move(LocatorEntries(index))}; +} + +CBlockLocator CChain::GetLocator() const +{ + return ::GetLocator(Tip()); } const CBlockIndex *CChain::FindFork(const CBlockIndex *pindex) const { diff --git a/src/chain.h b/src/chain.h index cc1d9e2d22..2d3b084b9b 100644 --- a/src/chain.h +++ b/src/chain.h @@ -473,8 +473,8 @@ public: /** Set/initialize a chain with a given tip. */ void SetTip(CBlockIndex& block); - /** Return a CBlockLocator that refers to a block in this chain (by default the tip). */ - CBlockLocator GetLocator(const CBlockIndex* pindex = nullptr) const; + /** Return a CBlockLocator that refers to the tip in of this chain. */ + CBlockLocator GetLocator() const; /** Find the last common block between this chain and a block index entry. */ const CBlockIndex* FindFork(const CBlockIndex* pindex) const; @@ -483,4 +483,10 @@ public: CBlockIndex* FindEarliestAtLeast(int64_t nTime, int height) const; }; +/** Get a locator for a block index entry. */ +CBlockLocator GetLocator(const CBlockIndex* index); + +/** Construct a list of hash entries to put in a locator. */ +std::vector LocatorEntries(const CBlockIndex* index); + #endif // BITCOIN_CHAIN_H diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b83034463d..cc8d695369 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2285,7 +2285,7 @@ void PeerManagerImpl::HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer, nodestate->nUnconnectingHeaders++; // Try to fill in the missing headers. - if (MaybeSendGetHeaders(pfrom, m_chainman.ActiveChain().GetLocator(m_chainman.m_best_header), peer)) { + if (MaybeSendGetHeaders(pfrom, GetLocator(m_chainman.m_best_header), peer)) { LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, nUnconnectingHeaders=%d)\n", headers[0].GetHash().ToString(), headers[0].hashPrevBlock.ToString(), @@ -2506,11 +2506,12 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, return; } } + Assume(pindexLast); // Consider fetching more headers. if (nCount == MAX_HEADERS_RESULTS) { // Headers message had its maximum size; the peer may have more headers. - if (MaybeSendGetHeaders(pfrom, WITH_LOCK(m_chainman.GetMutex(), return m_chainman.ActiveChain().GetLocator(pindexLast)), peer)) { + if (MaybeSendGetHeaders(pfrom, GetLocator(pindexLast), peer)) { LogPrint(BCLog::NET, "more getheaders (%d) to end to peer=%d (startheight:%d)\n", pindexLast->nHeight, pfrom.GetId(), peer.m_starting_height); } @@ -3285,7 +3286,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // use if we turned on sync with all peers). CNodeState& state{*Assert(State(pfrom.GetId()))}; if (state.fSyncStarted || (!peer->m_inv_triggered_getheaders_before_sync && *best_block != m_last_block_inv_triggering_headers_sync)) { - if (MaybeSendGetHeaders(pfrom, m_chainman.ActiveChain().GetLocator(m_chainman.m_best_header), *peer)) { + if (MaybeSendGetHeaders(pfrom, GetLocator(m_chainman.m_best_header), *peer)) { LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n", m_chainman.m_best_header->nHeight, best_block->ToString(), pfrom.GetId()); @@ -3752,7 +3753,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (!m_chainman.m_blockman.LookupBlockIndex(cmpctblock.header.hashPrevBlock)) { // Doesn't connect (or is genesis), instead of DoSing in AcceptBlockHeader, request deeper headers if (!m_chainman.ActiveChainstate().IsInitialBlockDownload()) { - MaybeSendGetHeaders(pfrom, m_chainman.ActiveChain().GetLocator(m_chainman.m_best_header), *peer); + MaybeSendGetHeaders(pfrom, GetLocator(m_chainman.m_best_header), *peer); } return; } @@ -4502,7 +4503,7 @@ void PeerManagerImpl::ConsiderEviction(CNode& pto, Peer& peer, std::chrono::seco // getheaders in-flight already, in which case the peer should // still respond to us with a sufficiently high work chain tip. MaybeSendGetHeaders(pto, - m_chainman.ActiveChain().GetLocator(state.m_chain_sync.m_work_header->pprev), + GetLocator(state.m_chain_sync.m_work_header->pprev), peer); LogPrint(BCLog::NET, "sending getheaders to outbound peer=%d to verify chain work (current best known block:%s, benchmark blockhash: %s)\n", pto.GetId(), state.pindexBestKnownBlock != nullptr ? state.pindexBestKnownBlock->GetBlockHash().ToString() : "", state.m_chain_sync.m_work_header->GetBlockHash().ToString()); state.m_chain_sync.m_sent_getheaders = true; @@ -4924,7 +4925,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) got back an empty response. */ if (pindexStart->pprev) pindexStart = pindexStart->pprev; - if (MaybeSendGetHeaders(*pto, m_chainman.ActiveChain().GetLocator(pindexStart), *peer)) { + if (MaybeSendGetHeaders(*pto, GetLocator(pindexStart), *peer)) { LogPrint(BCLog::NET, "initial getheaders (%d) to peer=%d (startheight:%d)\n", pindexStart->nHeight, pto->GetId(), peer->m_starting_height); state.fSyncStarted = true; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 2c845d0127..25161d2cd3 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -400,7 +400,7 @@ bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLockGetBlockTimeMax(); if (block.m_mtp_time) *block.m_mtp_time = index->GetMedianTimePast(); if (block.m_in_active_chain) *block.m_in_active_chain = active[index->nHeight] == index; - if (block.m_locator) { *block.m_locator = active.GetLocator(index); } + if (block.m_locator) { *block.m_locator = GetLocator(index); } if (block.m_next_block) FillBlock(active[index->nHeight] == index ? active[index->nHeight + 1] : nullptr, *block.m_next_block, lock, active); if (block.m_data) { REVERSE_LOCK(lock); @@ -527,8 +527,7 @@ public: { LOCK(::cs_main); const CBlockIndex* index = chainman().m_blockman.LookupBlockIndex(block_hash); - if (!index) return {}; - return chainman().ActiveChain().GetLocator(index); + return GetLocator(index); } std::optional findLocatorFork(const CBlockLocator& locator) override { diff --git a/src/test/skiplist_tests.cpp b/src/test/skiplist_tests.cpp index 3d3fd5d93d..9f5e3ab7ae 100644 --- a/src/test/skiplist_tests.cpp +++ b/src/test/skiplist_tests.cpp @@ -78,7 +78,7 @@ BOOST_AUTO_TEST_CASE(getlocator_test) for (int n=0; n<100; n++) { int r = InsecureRandRange(150000); CBlockIndex* tip = (r < 100000) ? &vBlocksMain[r] : &vBlocksSide[r - 100000]; - CBlockLocator locator = chain.GetLocator(tip); + CBlockLocator locator = GetLocator(tip); // The first result must be the block itself, the last one must be genesis. BOOST_CHECK(locator.vHave.front() == tip->GetBlockHash()); -- cgit v1.2.3