Skip to content

Commit fad2214

Browse files
author
MarcoFalke
committed
refactor: Use MockableSteadyClock in ReportHeadersPresync
This allows the clock to be mockable in tests. Also, replace cs_main with GetMutex() while touching this function. Also, use the ElapseSteady test helper in the p2p_headers_presync fuzz target to make it more deterministic. The m_last_presync_update variable is a global that is not reset in ResetAndInitialize. However, it is only used for logging, so completely disable it for now. Without this patch, the tool would report a diff: cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../qa-assets/fuzz_corpora/ p2p_headers_presync 32 ... 4468| 81| auto now = std::chrono::steady_clock::now(); 4469| 81| if (now < m_last_presync_update + std::chrono::milliseconds{250}) return; - ^80 + ^79 ...
1 parent fa9c387 commit fad2214

File tree

3 files changed

+9
-5
lines changed

3 files changed

+9
-5
lines changed

src/test/fuzz/p2p_headers_presync.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <test/util/net.h>
1616
#include <test/util/script.h>
1717
#include <test/util/setup_common.h>
18+
#include <test/util/time.h>
1819
#include <uint256.h>
1920
#include <validation.h>
2021

@@ -158,6 +159,9 @@ FUZZ_TARGET(p2p_headers_presync, .init = initialize)
158159
{
159160
SeedRandomStateForTest(SeedRand::ZEROS);
160161
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
162+
// The steady clock is currently only used for logging, so a constant
163+
// time-point seems acceptable for now.
164+
ElapseSteady elapse_steady{};
161165
SetMockTime(ConsumeTime(fuzzed_data_provider));
162166

163167
ChainstateManager& chainman = *g_testing_setup->m_node.chainman;

src/validation.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4456,16 +4456,16 @@ bool ChainstateManager::ProcessNewBlockHeaders(std::span<const CBlockHeader> hea
44564456

44574457
void ChainstateManager::ReportHeadersPresync(const arith_uint256& work, int64_t height, int64_t timestamp)
44584458
{
4459-
AssertLockNotHeld(cs_main);
4459+
AssertLockNotHeld(GetMutex());
44604460
{
4461-
LOCK(cs_main);
4461+
LOCK(GetMutex());
44624462
// Don't report headers presync progress if we already have a post-minchainwork header chain.
44634463
// This means we lose reporting for potentially legitimate, but unlikely, deep reorgs, but
44644464
// prevent attackers that spam low-work headers from filling our logs.
44654465
if (m_best_header->nChainWork >= UintToArith256(GetConsensus().nMinimumChainWork)) return;
44664466
// Rate limit headers presync updates to 4 per second, as these are not subject to DoS
44674467
// protection.
4468-
auto now = std::chrono::steady_clock::now();
4468+
auto now = MockableSteadyClock::now();
44694469
if (now < m_last_presync_update + std::chrono::milliseconds{250}) return;
44704470
m_last_presync_update = now;
44714471
}

src/validation.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// Copyright (c) 2009-2010 Satoshi Nakamoto
2-
// Copyright (c) 2009-2022 The Bitcoin Core developers
2+
// Copyright (c) 2009-present The Bitcoin Core developers
33
// Distributed under the MIT software license, see the accompanying
44
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
55

@@ -933,7 +933,7 @@ class ChainstateManager
933933
friend Chainstate;
934934

935935
/** Most recent headers presync progress update, for rate-limiting. */
936-
std::chrono::time_point<std::chrono::steady_clock> m_last_presync_update GUARDED_BY(::cs_main) {};
936+
MockableSteadyClock::time_point m_last_presync_update GUARDED_BY(GetMutex()){};
937937

938938
std::array<ThresholdConditionCache, VERSIONBITS_NUM_BITS> m_warningcache GUARDED_BY(::cs_main);
939939

0 commit comments

Comments
 (0)