fuzz: Use time helpers in node_eviction #34913

1. 
   maflcko commented at 4:32 pm on March 24, 2026: member

   The node_eviction fuzz test has many issues:

   • It uses the full int64_t range (in seconds) as input, which is absurdly large (millions of years) and also violates https://en.cppreference.com/w/cpp/chrono/duration.html:

   Each of the predefined duration types up to hours covers a range of at least ±292 years.

   • It does not use the existing ConsumeDuration and ConsumeTime helpers, which makes specifying a proper range difficult.

   So fix all issues by using ConsumeTime for time points with default arguments, and ConsumeDuration with the same precision, as well as possibly negative values.

2. DrahtBot added the label Fuzzing on Mar 24, 2026
3. 
   DrahtBot commented at 4:32 pm on March 24, 2026: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	marcofleon, brunoerg, w0xlt
   Stale ACK	sedited

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

4. sedited approved
5. 
   sedited commented at 9:02 pm on March 24, 2026: contributor

   ACK fa12c7999f98630accdd8ada8418a8f5ac9fdbc1
6. sedited requested review from marcofleon on Mar 24, 2026
7. in src/test/fuzz/node_eviction.cpp:27 in fa12c7999f

   27-            /*m_connected=*/std::chrono::seconds{fuzzed_data_provider.ConsumeIntegral<int64_t>()},
   28-            /*m_min_ping_time=*/std::chrono::microseconds{fuzzed_data_provider.ConsumeIntegral<int64_t>()},
   29-            /*m_last_block_time=*/std::chrono::seconds{fuzzed_data_provider.ConsumeIntegral<int64_t>()},
   30-            /*m_last_tx_time=*/std::chrono::seconds{fuzzed_data_provider.ConsumeIntegral<int64_t>()},
   31+            /*m_connected=*/ConsumeTime(fuzzed_data_provider).time_since_epoch(),
   32+            /*m_min_ping_time=*/ConsumeDuration<std::chrono::microseconds>(fuzzed_data_provider, /*min=*/std::chrono::years{-1}, /*max=*/std::chrono::years{99}),
   

   ---

   ---

   

   brunoerg commented at 2:02 pm on March 25, 2026:

   fa12c7999f98630accdd8ada8418a8f5ac9fdbc1: Perhaps the max value in m_min_ping_time could be ~292 years to represent a case where the peer has not been pinged yet (std::chrono::microseconds::max())?

   ---

   

   maflcko commented at 3:58 pm on March 25, 2026:

   Thanks, good point. To avoid having to touch this again in the future, and to have self-updating code, I used decltype: ConsumeDuration<decltype(NodeEvictionCandidate::m_min_ping_time)>(fuzzed_data_provider, /*min=*/std::chrono::years{-1}, /*max=*/decltype(CNode::m_min_ping_time.load())::max())

   Also, I pushed the common_type_t stuff from https://github.com/bitcoin/bitcoin/blob/2fe76ed8324af44c985b96455a05c3e8bec0a03e/src/random.h#L347-L350

8. brunoerg approved
9. 
   brunoerg commented at 2:03 pm on March 25, 2026: contributor

   code review ACK fa12c7999f98630accdd8ada8418a8f5ac9fdbc1

   just left a comment about the m_min_ping_time but I’m fine as is.

10. fuzz: Use time helpers in node_eviction fa1ebde1ad
11. maflcko force-pushed on Mar 25, 2026
12. in src/test/fuzz/node_eviction.cpp:28 in fa1ebde1ad

    28-            /*m_min_ping_time=*/std::chrono::microseconds{fuzzed_data_provider.ConsumeIntegral<int64_t>()},
    29-            /*m_last_block_time=*/std::chrono::seconds{fuzzed_data_provider.ConsumeIntegral<int64_t>()},
    30-            /*m_last_tx_time=*/std::chrono::seconds{fuzzed_data_provider.ConsumeIntegral<int64_t>()},
    31+            /*m_connected=*/ConsumeTime(fuzzed_data_provider).time_since_epoch(),
    32+            /*m_min_ping_time=*/ConsumeDuration<decltype(NodeEvictionCandidate::m_min_ping_time)>(fuzzed_data_provider, /*min=*/std::chrono::years{-1}, /*max=*/decltype(CNode::m_min_ping_time.load())::max()),
    33+            /*m_last_block_time=*/ConsumeTime(fuzzed_data_provider).time_since_epoch(),
    

    ---

    ---

    

    marcofleon commented at 5:22 pm on March 25, 2026:

    nit: Why do we go through CNode::m_min_ping_time.load() instead of just reusing NodeEvictionCandidate::m_min_ping_time? Aren’t they both std::chrono::microseconds?

    edit: Meant to comment on the line above. The ConsumeDuration().

    ---

    

    marcofleon commented at 5:49 pm on March 25, 2026:

    I just saw the explanation in #34913 (review) (long day it seems), but I think NodeEvictionCandidate would self-update as well.

    ---

    

    maflcko commented at 9:03 am on March 26, 2026:

    I just think it is a bit cleaner, because it describes where the max really comes from.

    In theory, (not sure if it makes sense), one could increase decltype(NodeEvictionCandidate::m_min_ping_time) to nanoseconds, while leaving CNode as-is.

    I guess in that case, there will be a runtime ubsan error, which is fine, but could be minimally more confusing.

    Happy to push any diff, if someone bothers to write one. I just minimally prefer the current version.

    ---

    

    marcofleon commented at 11:35 am on March 26, 2026:

    Got it, thanks for explaining. Current version is fine for me as well.
13. 
    marcofleon commented at 5:23 pm on March 25, 2026: contributor

    crACK fa1ebde1adf976a3d1ab7bc5e801d84e58d1385b
14. DrahtBot requested review from sedited on Mar 25, 2026
15. DrahtBot requested review from brunoerg on Mar 25, 2026
16. brunoerg approved
17. 
    brunoerg commented at 11:59 pm on March 25, 2026: contributor

    reACK fa1ebde1adf976a3d1ab7bc5e801d84e58d1385b
18. 
    w0xlt commented at 11:20 pm on March 26, 2026: contributor

    ACK fa1ebde1adf976a3d1ab7bc5e801d84e58d1385b
19. fanquake merged this on Mar 27, 2026
20. fanquake closed this on Mar 27, 2026

    ---

21. maflcko deleted the branch on Mar 27, 2026

Contributors
maflcko DrahtBot sedited brunoerg marcofleon w0xlt

Review Requested
sedited

Labels
Fuzzing