diff --git a/src/headerssync.cpp b/src/headerssync.cpp index fc73d4031de..039e7f68e8a 100644 --- a/src/headerssync.cpp +++ b/src/headerssync.cpp @@ -14,18 +14,21 @@ // CompressedHeader (we should re-calculate parameters if we compress further). static_assert(sizeof(CompressedHeader) == 48); -HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus_params, - const HeadersSyncParams& params, const CBlockIndex* chain_start, - const arith_uint256& minimum_required_work) : - m_commit_offset((assert(params.commitment_period > 0), // HeadersSyncParams field must be initialized to non-zero. - FastRandomContext().randrange(params.commitment_period))), - m_id(id), m_consensus_params(consensus_params), - m_params(params), - m_chain_start(chain_start), - m_minimum_required_work(minimum_required_work), - m_current_chain_work(chain_start->nChainWork), - m_last_header_received(m_chain_start->GetBlockHeader()), - m_current_height(chain_start->nHeight) +HeadersSyncState::HeadersSyncState(NodeId id, + const Consensus::Params& consensus_params, + const HeadersSyncParams& params, + const CBlockIndex& chain_start, + const arith_uint256& minimum_required_work) + : m_commit_offset((assert(params.commitment_period > 0), // HeadersSyncParams field must be initialized to non-zero. + FastRandomContext().randrange(params.commitment_period))), + m_id(id), + m_consensus_params(consensus_params), + m_params(params), + m_chain_start(chain_start), + m_minimum_required_work(minimum_required_work), + m_current_chain_work(chain_start.nChainWork), + m_last_header_received(m_chain_start.GetBlockHeader()), + m_current_height(chain_start.nHeight) { // Estimate the number of blocks that could possibly exist on the peer's // chain *right now* using 6 blocks/second (fastest blockrate given the MTP @@ -35,7 +38,7 @@ HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus // exceeds this bound, because it's not possible for a consensus-valid // chain to be longer than this (at the current time -- in the future we // could try again, if necessary, to sync a longer chain). - const auto max_seconds_since_start{(Ticks(NodeClock::now() - NodeSeconds{std::chrono::seconds{chain_start->GetMedianTimePast()}})) + const auto max_seconds_since_start{(Ticks(NodeClock::now() - NodeSeconds{std::chrono::seconds{chain_start.GetMedianTimePast()}})) + MAX_FUTURE_BLOCK_TIME}; m_max_commitments = 6 * max_seconds_since_start / m_params.commitment_period; @@ -161,10 +164,10 @@ bool HeadersSyncState::ValidateAndStoreHeadersCommitments(std::span= m_minimum_required_work) { m_redownloaded_headers.clear(); - m_redownload_buffer_last_height = m_chain_start->nHeight; - m_redownload_buffer_first_prev_hash = m_chain_start->GetBlockHash(); - m_redownload_buffer_last_hash = m_chain_start->GetBlockHash(); - m_redownload_chain_work = m_chain_start->nChainWork; + m_redownload_buffer_last_height = m_chain_start.nHeight; + m_redownload_buffer_first_prev_hash = m_chain_start.GetBlockHash(); + m_redownload_buffer_last_hash = m_chain_start.GetBlockHash(); + m_redownload_chain_work = m_chain_start.nChainWork; m_download_state = State::REDOWNLOAD; LogDebug(BCLog::NET, "Initial headers sync transition with peer=%d: reached sufficient work at height=%i, redownloading from height=%i\n", m_id, m_current_height, m_redownload_buffer_last_height); } @@ -228,7 +231,7 @@ bool HeadersSyncState::ValidateAndStoreRedownloadedHeader(const CBlockHeader& he if (!m_redownloaded_headers.empty()) { previous_nBits = m_redownloaded_headers.back().nBits; } else { - previous_nBits = m_chain_start->nBits; + previous_nBits = m_chain_start.nBits; } if (!PermittedDifficultyTransition(m_consensus_params, next_height, @@ -295,7 +298,7 @@ CBlockLocator HeadersSyncState::NextHeadersRequestLocator() const Assume(m_download_state != State::FINAL); if (m_download_state == State::FINAL) return {}; - auto chain_start_locator = LocatorEntries(m_chain_start); + auto chain_start_locator = LocatorEntries(&m_chain_start); std::vector locator; if (m_download_state == State::PRESYNC) { diff --git a/src/headerssync.h b/src/headerssync.h index a29726fb345..6d720874411 100644 --- a/src/headerssync.h +++ b/src/headerssync.h @@ -137,8 +137,8 @@ public: * minimum_required_work: amount of chain work required to accept the chain */ HeadersSyncState(NodeId id, const Consensus::Params& consensus_params, - const HeadersSyncParams& params, const CBlockIndex* chain_start, - const arith_uint256& minimum_required_work); + const HeadersSyncParams& params, const CBlockIndex& chain_start, + const arith_uint256& minimum_required_work); /** Result data structure for ProcessNextHeaders. */ struct ProcessingResult { @@ -220,7 +220,7 @@ private: const HeadersSyncParams m_params; /** Store the last block in our block index that the peer's chain builds from */ - const CBlockIndex* m_chain_start{nullptr}; + const CBlockIndex& m_chain_start; /** Minimum work that we're looking for on this chain. */ const arith_uint256 m_minimum_required_work; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 578a7d715c9..5d97a464f7c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -686,8 +686,8 @@ private: * calling); false otherwise. */ bool TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, - const CBlockIndex* chain_start_header, - std::vector& headers) + const CBlockIndex& chain_start_header, + std::vector& headers) EXCLUSIVE_LOCKS_REQUIRED(!peer.m_headers_sync_mutex, !m_peer_mutex, !m_headers_presync_mutex, g_msgproc_mutex); /** Return true if the given header is an ancestor of @@ -2633,10 +2633,10 @@ bool PeerManagerImpl::IsContinuationOfLowWorkHeadersSync(Peer& peer, CNode& pfro return false; } -bool PeerManagerImpl::TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlockIndex* chain_start_header, std::vector& headers) +bool PeerManagerImpl::TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlockIndex& chain_start_header, std::vector& headers) { // Calculate the claimed total work on this chain. - arith_uint256 total_work = chain_start_header->nChainWork + CalculateClaimedHeadersWork(headers); + arith_uint256 total_work = chain_start_header.nChainWork + CalculateClaimedHeadersWork(headers); // Our dynamic anti-DoS threshold (minimum work required on a headers chain // before we'll store it) @@ -2667,7 +2667,7 @@ bool PeerManagerImpl::TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlo // handled inside of IsContinuationOfLowWorkHeadersSync. (void)IsContinuationOfLowWorkHeadersSync(peer, pfrom, headers); } else { - LogDebug(BCLog::NET, "Ignoring low-work chain (height=%u) from peer=%d\n", chain_start_header->nHeight + headers.size(), pfrom.GetId()); + LogDebug(BCLog::NET, "Ignoring low-work chain (height=%u) from peer=%d\n", chain_start_header.nHeight + headers.size(), pfrom.GetId()); } // The peer has not yet given us a chain that meets our work threshold, @@ -2933,7 +2933,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, // Do anti-DoS checks to determine if we should process or store for later // processing. if (!already_validated_work && TryLowWorkHeadersSync(peer, pfrom, - chain_start_header, headers)) { + *chain_start_header, headers)) { // If we successfully started a low-work headers sync, then there // should be no headers to process any further. Assume(headers.empty()); diff --git a/src/test/fuzz/headerssync.cpp b/src/test/fuzz/headerssync.cpp index 5bfbed85375..b33f4dc728f 100644 --- a/src/test/fuzz/headerssync.cpp +++ b/src/test/fuzz/headerssync.cpp @@ -44,7 +44,7 @@ class FuzzedHeadersSyncState : public HeadersSyncState { public: FuzzedHeadersSyncState(const HeadersSyncParams& sync_params, const size_t commit_offset, - const CBlockIndex* chain_start, const arith_uint256& minimum_required_work) + const CBlockIndex& chain_start, const arith_uint256& minimum_required_work) : HeadersSyncState(/*id=*/0, Params().GetConsensus(), sync_params, chain_start, minimum_required_work) { const_cast(m_commit_offset) = commit_offset; @@ -74,7 +74,7 @@ FUZZ_TARGET(headers_sync_state, .init = initialize_headers_sync_state_fuzz) FuzzedHeadersSyncState headers_sync( params, /*commit_offset=*/fuzzed_data_provider.ConsumeIntegralInRange(0, params.commitment_period - 1), - /*chain_start=*/&start_index, + /*chain_start=*/start_index, /*minimum_required_work=*/min_work); // Store headers for potential redownload phase. diff --git a/src/test/headers_sync_chainwork_tests.cpp b/src/test/headers_sync_chainwork_tests.cpp index c91420978ff..b6a5e153e51 100644 --- a/src/test/headers_sync_chainwork_tests.cpp +++ b/src/test/headers_sync_chainwork_tests.cpp @@ -52,7 +52,7 @@ constexpr size_t COMMITMENT_PERIOD{600}; // Somewhat close to mainnet. struct HeadersGeneratorSetup : public RegTestingSetup { const CBlock& genesis{Params().GenesisBlock()}; - const CBlockIndex* chain_start{WITH_LOCK(::cs_main, return m_node.chainman->m_blockman.LookupBlockIndex(genesis.GetHash()))}; + CBlockIndex& chain_start{WITH_LOCK(::cs_main, return *Assert(m_node.chainman->m_blockman.LookupBlockIndex(genesis.GetHash())))}; // Generate headers for two different chains (using differing merkle roots // to ensure the headers are different).