Skip to content

Commit ccb8bce

Browse files
MacroFakePastaPastaPasta
authored andcommitted
Merge bitcoin#25786: refactor: Make adjusted time type safe
Upstream commits: fa3be79 Add time helpers eeee5ad Make adjusted time type safe
1 parent 2406ebc commit ccb8bce

11 files changed

Lines changed: 37 additions & 17 deletions

File tree

src/bitcoin-chainstate.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ int main(int argc, char* argv[])
8989
// SETUP: Chainstate
9090
const ChainstateManager::Options chainman_opts{
9191
.chainparams = chainparams,
92-
.adjusted_time_callback = static_cast<int64_t (*)()>(GetTime),
92+
.adjusted_time_callback = NodeClock::now,
9393
};
9494
ChainstateManager chainman{chainman_opts};
9595

src/chain.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <primitives/block.h>
1313
#include <sync.h>
1414
#include <uint256.h>
15+
#include <util/time.h>
1516

1617
#include <vector>
1718

@@ -255,6 +256,11 @@ class CBlockIndex
255256
*/
256257
bool HaveTxsDownloaded() const { return nChainTx != 0; }
257258

259+
NodeSeconds Time() const
260+
{
261+
return NodeSeconds{std::chrono::seconds{nTime}};
262+
}
263+
258264
int64_t GetBlockTime() const
259265
{
260266
return (int64_t)nTime;

src/consensus/params.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <uint256.h>
1010
#include <llmq/params.h>
1111

12+
#include <chrono>
1213
#include <limits>
1314
#include <vector>
1415

@@ -176,6 +177,10 @@ struct Params {
176177
int64_t nPowTargetTimespan;
177178
int nPowKGWHeight;
178179
int nPowDGWHeight;
180+
std::chrono::seconds PowTargetSpacing() const
181+
{
182+
return std::chrono::seconds{nPowTargetSpacing};
183+
}
179184
int64_t DifficultyAdjustmentInterval() const { return nPowTargetTimespan / nPowTargetSpacing; }
180185
/** The best chain should have at least this much work */
181186
uint256 nMinimumChainWork;

src/kernel/chainstatemanager_opts.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#ifndef BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
66
#define BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
77

8+
#include <util/time.h>
9+
810
#include <cstdint>
911
#include <functional>
1012

@@ -19,7 +21,7 @@ namespace kernel {
1921
*/
2022
struct ChainstateManagerOpts {
2123
const CChainParams& chainparams;
22-
const std::function<int64_t()> adjusted_time_callback{nullptr};
24+
const std::function<NodeClock::time_point()> adjusted_time_callback{nullptr};
2325
};
2426

2527
} // namespace kernel

src/net_processing.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1408,7 +1408,7 @@ bool PeerManagerImpl::TipMayBeStale()
14081408

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

14141414
static bool PeerHasHeader(CNodeState *state, const CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
@@ -6218,7 +6218,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
62186218

62196219
if (!state.fSyncStarted && CanServeBlocks(*peer) && !fImporting && !fReindex && pto->CanRelay()) {
62206220
// Only actively request headers from a single peer, unless we're close to end of initial download.
6221-
if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->GetBlockTime() > GetAdjustedTime() - nMaxTipAge) {
6221+
if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->Time() > GetAdjustedTime() - std::chrono::seconds{nMaxTipAge}) {
62226222
const CBlockIndex* pindexStart = m_chainman.m_best_header;
62236223
/* If possible, start at the block preceding the currently
62246224
best known header. This ensures that we always get a
@@ -6239,7 +6239,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
62396239
// Convert HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER to microseconds before scaling
62406240
// to maintain precision
62416241
std::chrono::microseconds{HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER} *
6242-
(GetAdjustedTime() - m_chainman.m_best_header->GetBlockTime()) / consensusParams.nPowTargetSpacing
6242+
Ticks<std::chrono::seconds>(GetAdjustedTime() - m_chainman.m_best_header->Time()) / consensusParams.nPowTargetSpacing
62436243
);
62446244
nSyncStarted++;
62456245
}
@@ -6620,7 +6620,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
66206620
// Check for headers sync timeouts
66216621
if (state.fSyncStarted && state.m_headers_sync_timeout < std::chrono::microseconds::max()) {
66226622
// Detect whether this is a stalling initial-headers-sync peer
6623-
if (m_chainman.m_best_header->GetBlockTime() <= GetAdjustedTime() - nMaxTipAge) {
6623+
if (m_chainman.m_best_header->Time() <= GetAdjustedTime() - std::chrono::seconds{nMaxTipAge}) {
66246624
if (current_time > state.m_headers_sync_timeout && nSyncStarted == 1 && (m_num_preferred_download_peers - state.fPreferredDownload >= 1)) {
66256625
// Disconnect a peer (without NetPermissionFlags::NoBan permission) if it is our only sync peer,
66266626
// and we have others we could be using instead.

src/node/miner.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ namespace node {
4949
int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev)
5050
{
5151
int64_t nOldTime = pblock->nTime;
52-
int64_t nNewTime = std::max(pindexPrev->GetMedianTimePast() + 1, GetAdjustedTime());
52+
int64_t nNewTime{std::max<int64_t>(pindexPrev->GetMedianTimePast() + 1, TicksSinceEpoch<std::chrono::seconds>(GetAdjustedTime()))};
5353

5454
if (nOldTime < nNewTime) {
5555
pblock->nTime = nNewTime;
@@ -218,7 +218,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
218218
pblock->nVersion = gArgs.GetIntArg("-blockversion", pblock->nVersion);
219219
}
220220

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

224224
if (fDIP0003Active_context) {

src/primitives/block.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <uint256.h>
1313
#include <cstddef>
1414
#include <type_traits>
15+
#include <util/time.h>
1516

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

5657
uint256 GetHash() const;
5758

59+
NodeSeconds Time() const
60+
{
61+
return NodeSeconds{std::chrono::seconds{nTime}};
62+
}
63+
5864
int64_t GetBlockTime() const
5965
{
6066
return (int64_t)nTime;

src/timedata.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ int64_t GetTimeOffset()
3232
return nTimeOffset;
3333
}
3434

35-
int64_t GetAdjustedTime()
35+
NodeClock::time_point GetAdjustedTime()
3636
{
37-
return GetTime() + GetTimeOffset();
37+
return NodeClock::now() + std::chrono::seconds{GetTimeOffset()};
3838
}
3939

4040
#define BITCOIN_TIMEDATA_MAX_SAMPLES 200

src/timedata.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ class CMedianFilter
7575

7676
/** Functions to keep track of adjusted P2P time */
7777
int64_t GetTimeOffset();
78-
int64_t GetAdjustedTime();
78+
NodeClock::time_point GetAdjustedTime();
7979
void AddTimeData(const CNetAddr& ip, int64_t nTime);
8080

8181
/**

src/validation.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3916,7 +3916,7 @@ bool IsBlockMutated(const CBlock& block)
39163916
* in ConnectBlock().
39173917
* Note that -reindex-chainstate skips the validation that happens here!
39183918
*/
3919-
static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const ChainstateManager& chainman, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
3919+
static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const ChainstateManager& chainman, const CBlockIndex* pindexPrev, NodeClock::time_point now) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
39203920
{
39213921
AssertLockHeld(::cs_main);
39223922
assert(pindexPrev != nullptr);
@@ -3956,8 +3956,9 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio
39563956
return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "time-too-old", strprintf("block's timestamp is too early %d %d", block.GetBlockTime(), pindexPrev->GetMedianTimePast()));
39573957

39583958
// Check timestamp
3959-
if (block.GetBlockTime() > nAdjustedTime + MAX_FUTURE_BLOCK_TIME)
3960-
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));
3959+
if (block.Time() > now + std::chrono::seconds{MAX_FUTURE_BLOCK_TIME}) {
3960+
return state.Invalid(BlockValidationResult::BLOCK_TIME_FUTURE, "time-too-new", strprintf("block timestamp too far in the future %d %d", block.GetBlockTime(), TicksSinceEpoch<std::chrono::seconds>(now) + 2 * 60 * 60));
3961+
}
39613962

39623963
// Reject blocks with outdated version
39633964
if ((block.nVersion < 2 && DeploymentActiveAfter(pindexPrev, chainman.GetConsensus(), Consensus::DEPLOYMENT_HEIGHTINCB)) ||
@@ -4361,7 +4362,7 @@ bool TestBlockValidity(BlockValidationState& state,
43614362
Chainstate& chainstate,
43624363
const CBlock& block,
43634364
CBlockIndex* pindexPrev,
4364-
const std::function<int64_t()>& adjusted_time_callback,
4365+
const std::function<NodeClock::time_point()>& adjusted_time_callback,
43654366
bool fCheckPOW,
43664367
bool fCheckMerkleRoot)
43674368
{

0 commit comments

Comments
 (0)