refactor: Use reference for chain_start in HeadersSyncState
What changed, and why it matters
This is a code cleanup (refactor) that changes how one internal data structure receives a known-valid block index. It replaces a raw pointer with a C++ reference because the value is never null. There is no functional change to how Bitcoin validates blocks, syncs headers, or handles network data, and no security vulnerability is fixed or introduced.
No security action required; treat as ordinary code-quality refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors HeadersSyncState to take chain_start as const CBlockIndex& instead of const CBlockIndex*. All call sites and member accesses are updated accordingly (pointer dereferences -> direct member access, &m_chain_start for LocatorEntries). The constructor initializer list is reformatted. A test variable’s const qualifier is removed to silence a gcc 13.0 false-positive dangling-reference warning. No logic changes.
Changed components
src/headerssync.cppsrc/headerssync.hsrc/net_processing.cppsrc/test/fuzz/headerssync.cppsrc/test/headers_sync_chainwork_tests.cppInspect captured patch +34 / −31
diff --git a/src/headerssync.cpp b/src/headerssync.cpp
index fc73d403..039e7f68 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<std::chrono::seconds>(NodeClock::now() - NodeSeconds{std::chrono::seconds{chain_start->GetMedianTimePast()}}))
+ const auto max_seconds_since_start{(Ticks<std::chrono::seconds>(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<const CBlock
if (m_current_chain_work >= 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<uint256> locator;
if (m_download_state == State::PRESYNC) {
diff --git a/src/headerssync.h b/src/headerssync.h
index a29726fb..6d720874 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 578a7d71..5d97a464 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<CBlockHeader>& headers)
+ const CBlockIndex& chain_start_header,
+ std::vector<CBlockHeader>& 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<CBlockHeader>& headers)
+bool PeerManagerImpl::TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlockIndex& chain_start_header, std::vector<CBlockHeader>& 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 5bfbed85..b33f4dc7 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<size_t&>(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<size_t>(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 c9142097..b6a5e153 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).
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.