Skip to content

Commit

Permalink
Merge bitcoin#25786: refactor: Make adjusted time type safe
Browse files Browse the repository at this point in the history
eeee5ad Make adjusted time type safe (MacroFake)
fa3be79 Add time helpers (MacroFake)

Pull request description:

  This makes follow-ups easier to review. Also, it makes sense by itself.

ACKs for top commit:
  ryanofsky:
    Code review ACK eeee5ad. Confirmed type changes and equivalent code changes only.

Tree-SHA512: 51bf1ae5428552177286113babdd49e82459d6c710a07b6e80a0a045d373cf51045ee010461aba98e0151d8d71b9b3b5f8f73e302d46ba4558e0b55201f99e9f
  • Loading branch information
fanquake authored and vijaydasmp committed Mar 8, 2025
1 parent 26ea618 commit 288f6e2
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 12 deletions.
6 changes: 6 additions & 0 deletions src/chain.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <primitives/block.h>
#include <sync.h>
#include <uint256.h>
#include <util/time.h>

#include <vector>

Expand Down Expand Up @@ -259,6 +260,11 @@ class CBlockIndex
*/
bool HaveTxsDownloaded() const { return nChainTx != 0; }

NodeSeconds Time() const
{
return NodeSeconds{std::chrono::seconds{nTime}};
}

int64_t GetBlockTime() const
{
return (int64_t)nTime;
Expand Down
5 changes: 5 additions & 0 deletions src/consensus/params.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <uint256.h>
#include <llmq/params.h>

#include <chrono>
#include <limits>
#include <vector>

Expand Down Expand Up @@ -168,6 +169,10 @@ struct Params {
int64_t nPowTargetTimespan;
int nPowKGWHeight;
int nPowDGWHeight;
std::chrono::seconds PowTargetSpacing() const
{
return std::chrono::seconds{nPowTargetSpacing};
}
int64_t DifficultyAdjustmentInterval() const { return nPowTargetTimespan / nPowTargetSpacing; }
uint256 nMinimumChainWork;
uint256 defaultAssumeValid;
Expand Down
8 changes: 4 additions & 4 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1302,7 +1302,7 @@ bool PeerManagerImpl::TipMayBeStale()

bool PeerManagerImpl::CanDirectFetch()
{
return m_chainman.ActiveChain().Tip()->GetBlockTime() > GetAdjustedTime() - m_chainparams.GetConsensus().nPowTargetSpacing * 20;
return m_chainman.ActiveChain().Tip()->Time() > GetAdjustedTime() - m_chainparams.GetConsensus().PowTargetSpacing() * 20;
}

static bool PeerHasHeader(CNodeState *state, const CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
Expand Down Expand Up @@ -5833,7 +5833,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)

if (!state.fSyncStarted && CanServeBlocks(*peer) && !fImporting && !fReindex && pto->CanRelay()) {
// Only actively request headers from a single peer, unless we're close to end of initial download.
if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->GetBlockTime() > GetAdjustedTime() - nMaxTipAge) {
if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->Time() > GetAdjustedTime() - 24h) {
const CBlockIndex* pindexStart = m_chainman.m_best_header;
/* If possible, start at the block preceding the currently
best known header. This ensures that we always get a
Expand All @@ -5854,7 +5854,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
// Convert HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER to microseconds before scaling
// to maintain precision
std::chrono::microseconds{HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER} *
(GetAdjustedTime() - m_chainman.m_best_header->GetBlockTime()) / consensusParams.nPowTargetSpacing
Ticks<std::chrono::seconds>(GetAdjustedTime() - m_chainman.m_best_header->Time()) / consensusParams.nPowTargetSpacing
);
nSyncStarted++;
}
Expand Down Expand Up @@ -6233,7 +6233,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
// Check for headers sync timeouts
if (state.fSyncStarted && state.m_headers_sync_timeout < std::chrono::microseconds::max()) {
// Detect whether this is a stalling initial-headers-sync peer
if (m_chainman.m_best_header->GetBlockTime() <= GetAdjustedTime() - nMaxTipAge) {
if (m_chainman.m_best_header->Time() <= GetAdjustedTime() - 24h) {
if (current_time > state.m_headers_sync_timeout && nSyncStarted == 1 && (m_num_preferred_download_peers - state.fPreferredDownload >= 1)) {
// Disconnect a peer (without NetPermissionFlags::NoBan permission) if it is our only sync peer,
// and we have others we could be using instead.
Expand Down
4 changes: 2 additions & 2 deletions src/node/miner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev)
{
int64_t nOldTime = pblock->nTime;
int64_t nNewTime = std::max(pindexPrev->GetMedianTimePast() + 1, GetAdjustedTime());
int64_t nNewTime{std::max<int64_t>(pindexPrev->GetMedianTimePast() + 1, TicksSinceEpoch<std::chrono::seconds>(GetAdjustedTime()))};

if (nOldTime < nNewTime) {
pblock->nTime = nNewTime;
Expand Down Expand Up @@ -158,7 +158,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
pblock->nVersion = gArgs.GetIntArg("-blockversion", pblock->nVersion);
}

pblock->nTime = GetAdjustedTime();
pblock->nTime = TicksSinceEpoch<std::chrono::seconds>(GetAdjustedTime());
m_lock_time_cutoff = pindexPrev->GetMedianTimePast();

if (fDIP0003Active_context) {
Expand Down
6 changes: 6 additions & 0 deletions src/primitives/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <uint256.h>
#include <cstddef>
#include <type_traits>
#include <util/time.h>

/** Nodes collect new transactions into a block, hash them into a hash tree,
* and scan through nonce values to make the block's hash satisfy proof-of-work
Expand Down Expand Up @@ -55,6 +56,11 @@ class CBlockHeader

uint256 GetHash() const;

NodeSeconds Time() const
{
return NodeSeconds{std::chrono::seconds{nTime}};
}

int64_t GetBlockTime() const
{
return (int64_t)nTime;
Expand Down
4 changes: 2 additions & 2 deletions src/timedata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ int64_t GetTimeOffset()
return nTimeOffset;
}

int64_t GetAdjustedTime()
NodeClock::time_point GetAdjustedTime()
{
return GetTime() + GetTimeOffset();
return NodeClock::now() + std::chrono::seconds{GetTimeOffset()};
}

#define BITCOIN_TIMEDATA_MAX_SAMPLES 200
Expand Down
2 changes: 1 addition & 1 deletion src/timedata.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class CMedianFilter

/** Functions to keep track of adjusted P2P time */
int64_t GetTimeOffset();
int64_t GetAdjustedTime();
NodeClock::time_point GetAdjustedTime();
void AddTimeData(const CNetAddr& ip, int64_t nTime);

/**
Expand Down
6 changes: 3 additions & 3 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3901,7 +3901,7 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu
* in ConnectBlock().
* Note that -reindex-chainstate skips the validation that happens here!
*/
static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const CChainParams& params, const CBlockIndex* pindexPrev, NodeClock::time_point now) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);
assert(pindexPrev != nullptr);
Expand Down Expand Up @@ -3941,9 +3941,9 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio
return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "time-too-old", strprintf("block's timestamp is too early %d %d", block.GetBlockTime(), pindexPrev->GetMedianTimePast()));

// Check timestamp
if (block.GetBlockTime() > nAdjustedTime + MAX_FUTURE_BLOCK_TIME)
if (block.Time() > now + std::chrono::seconds{MAX_FUTURE_BLOCK_TIME}) {
return state.Invalid(BlockValidationResult::BLOCK_TIME_FUTURE, "time-too-new", strprintf("block timestamp too far in the future %d %d", block.GetBlockTime(), nAdjustedTime + 2 * 60 * 60));

}
// Reject blocks with outdated version
if ((block.nVersion < 2 && DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_HEIGHTINCB)) ||
(block.nVersion < 3 && DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_DERSIG)) ||
Expand Down

0 comments on commit 288f6e2

Please sign in to comment.