cryptogera/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-03-17 08:52:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: Use reference for chain_start in HeadersSyncState

Browse Source 
chain_start can never be null, so it's better to pass it as a reference
rather than a raw pointer

Also slightly reformat HeaderSyncState constructor to make clang-format
happy

Lastly, remove `const` from `chain_start` declaration in
headers_sync_chainwork_tests, to work aroud a false-positive
dangling-reference warning in gcc 13.0

Co-Authored-By: maflcko <6399679+maflcko@users.noreply.github.com>
This commit is contained in:
Daniela Brozzoni 2025-06-11 18:33:30 +02:00
parent e37555e540
commit de4242f474
No known key found for this signature in database
GPG Key ID: 7DE4F1FDCED0AB87
5 changed files with 34 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
src/headerssync.cpp
View File

@ -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) {

6
src/headerssync.h
View File

@ -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;

12
src/net_processing.cpp
View File

@ -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());

4
src/test/fuzz/headerssync.cpp
View File

@ -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.

2
src/test/headers_sync_chainwork_tests.cpp
View File

@ -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).