test: Add and use ElapseTime helper #32430

maflcko · 2025-05-06T22:39:14Z

Currently mocktime is written to a global, which may leak between sub-tests (albeit some tests try to reset the mocktime on a best-effort basis). Also, when advancing it, one has to keep a counter variable around.

Fix both issues by a new ElapseTime helper, which resets the mocktime once it goes out of scope. Also, it has a method to advance the mocktime by a delta.

DrahtBot · 2025-05-06T22:39:17Z

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32430.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	l0rinc
Stale ACK	w0xlt

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33106 (policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee by glozow)
#32941 (p2p: TxOrphanage revamp cleanups by glozow)
#32414 (validation: periodically flush dbcache during reindex-chainstate by andrewtoth)
#29700 (kernel, refactor: return error status on all fatal errors by ryanofsky)
#28676 (Cluster mempool implementation by sdaftuar)
#17783 (common: Disallow calling IsArgSet() on ALLOW_LIST options by ryanofsky)
#17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
#16545 (refactor: Implement missing error checking for ArgsManager flags by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

/// Helper to initialize the global MockableSteadyClock, elapse it, and reset -> /// Helper to initialize the global MockableSteadyClock, advance it, and reset [“elapse” is intransitive; you cannot “elapse it”. Use “advance” or “advance the clock” for clarity.]
/// Helper to initialize the global NodeClock, elapse it, and reset -> /// Helper to initialize the global NodeClock, advance it, and reset [same issue: “elapse” is intransitive; “advance” is grammatically correct here.]

^2026-01-16

DrahtBot · 2025-05-06T22:54:38Z

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/runs/41759458930}
_{LLM reason (✨ experimental): (empty)}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

l0rinc

Concept ACK - was thinking of something similar when reviewing 41479ed (#32414)

maflcko · 2025-05-07T09:47:14Z

Sorry for the force-pushes, but this uncovered pre-existing issues in the fuzz tests that were not using mocktime during init. So I went ahead and fixed those as well in a separate commit. Should be good now, hopefully 😅

src/test/util/setup_common.h

w0xlt

ACK fae29c5

DrahtBot · 2025-07-08T20:27:38Z

🚧 At least one of the CI tasks failed.
_{Task previous releases, depends DEBUG: https://github.com/bitcoin/bitcoin/runs/45588216821}
_{LLM reason (✨ experimental): The CI failure is caused by a compilation error in src/util/time.cpp due to incorrect usage of std::chrono::duration, which prevents building.}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

maflcko · 2025-07-09T15:04:10Z

Sorry for the force-pushes, but this uncovered pre-existing issues in the fuzz tests that were not using mocktime during init. So I went ahead and fixed those as well in a separate commit. Should be good now, hopefully 😅

Went ahead and cherry-picked out the fuzz-fixes into #32927. Should be easy to (re-)review that one.

l0rinc

Thanks for fixing these, left quite a few comments, hope you'll find them useful

src/test/util/setup_common.h

src/test/fuzz/headerssync.cpp

src/test/util/time.h

src/test/denialofservice_tests.cpp

src/test/mempool_tests.cpp

src/test/fuzz/p2p_handshake.cpp

src/test/util/time.h

src/bench/util_time.cpp

maflcko · 2025-07-31T10:41:23Z

Force pushed with some minor doc-changes and small refactoring in src/test/util/time.h

l0rinc

code review ACK fa80e75

This is less code and also required for the next commit.

This is a bit more type-safe than a raw i64.

This makes it easier to use mock-time in tests. Also, it resets the global mocktime, so that no state is leaked between test cases. Also, it resets the global steady mock-time for the same reason.

This is required in the next commit.

maflcko · 2026-01-16T10:40:21Z

rebased. should be trivial to re-review via git range-diff bitcoin-core/master fa80e750e7 fa053c2e32. The only changes were (1) dropping hunks of deleted code, (2) two in-line conflicts, (3) some adjacent lines modified in unrelated changes.

l0rinc

diff reACK fa053c2

mini_miner_selection and LimitOrphans fuzz targets were removed during rebase and the refactored chainstate_write_interval refactoring was adjusted.

DrahtBot added the Tests label May 6, 2025

maflcko force-pushed the 2505-elapse-time branch from fa13228 to bbbb2af Compare May 6, 2025 22:54

DrahtBot added the CI failed label May 6, 2025

maflcko force-pushed the 2505-elapse-time branch 5 times, most recently from fa123d4 to fae29c5 Compare May 7, 2025 09:27

l0rinc approved these changes May 7, 2025

View reviewed changes

DrahtBot removed the CI failed label May 7, 2025

DrahtBot mentioned this pull request May 7, 2025

validation: periodically flush dbcache during reindex-chainstate #32414

Merged

w0xlt reviewed May 7, 2025

View reviewed changes

src/test/util/setup_common.h Outdated Show resolved Hide resolved

w0xlt reviewed May 7, 2025

View reviewed changes

DrahtBot requested a review from l0rinc May 7, 2025 18:39

This was referenced May 7, 2025

wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict #29680

Closed

refactor: Implement missing error checking for ArgsManager flags #16545

Draft

DrahtBot mentioned this pull request May 16, 2025

fuzz: Delete wallet_notifications #32526

Merged

DrahtBot added the Needs rebase label May 16, 2025

maflcko force-pushed the 2505-elapse-time branch from fae29c5 to fa67233 Compare July 8, 2025 19:44

DrahtBot added the CI failed label Jul 8, 2025

DrahtBot removed the Needs rebase label Jul 8, 2025

This was referenced Jul 8, 2025

fuzz: Make process_message(s) more deterministic #32822

Merged

p2p: improve TxOrphanage denial of service bounds #31829

Merged

This was referenced Jul 9, 2025

fuzz: Add missing calls to SetMockTime for determinism #32927

Merged

p2p: TxOrphanage revamp cleanups #32941

Merged

DrahtBot removed the CI failed label Jul 11, 2025

maflcko requested a review from w0xlt July 18, 2025 12:43

DrahtBot added the Needs rebase label Jul 18, 2025

maflcko force-pushed the 2505-elapse-time branch 2 times, most recently from fa55137 to fafcff1 Compare July 24, 2025 14:19

DrahtBot added Needs rebase and removed Needs rebase labels Jul 24, 2025

maflcko force-pushed the 2505-elapse-time branch from fafcff1 to faaaaac Compare July 25, 2025 11:09

DrahtBot removed the Needs rebase label Jul 25, 2025

l0rinc reviewed Jul 27, 2025

View reviewed changes

DrahtBot mentioned this pull request Jul 29, 2025

refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags #17580

Draft

maflcko force-pushed the 2505-elapse-time branch from faaaaac to fa80e75 Compare July 31, 2025 10:22

l0rinc approved these changes Jul 31, 2025

View reviewed changes

This was referenced Jul 31, 2025

policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee #33106

Merged

kernel, refactor: return error status on all fatal errors #29700

Draft

common: Disallow calling IsArgSet() on ALLOW_LIST options #17783

Draft

DrahtBot added the Needs rebase label Aug 4, 2025

MarcoFalke added 6 commits January 16, 2026 11:00

fuzz: Use min option in ConsumeTime

faa5a9e

This is less code and also required for the next commit.

fuzz: Return chrono duration from ConsumeTime()

2222e05

This is a bit more type-safe than a raw i64.

test: Add ElapseTime context

fa98ecd

This makes it easier to use mock-time in tests. Also, it resets the global mocktime, so that no state is leaked between test cases. Also, it resets the global steady mock-time for the same reason.

fuzz: Use ElapseTime

faa7876

test: Allow time_point in boost checks

fa7704b

This is required in the next commit.

test: Use ElapseTime in more tests

fa053c2

maflcko force-pushed the 2505-elapse-time branch from fa80e75 to fa053c2 Compare January 16, 2026 10:37

DrahtBot removed the Needs rebase label Jan 16, 2026

l0rinc approved these changes Jan 16, 2026

View reviewed changes

This was referenced Jan 17, 2026

refactor: [p2p] Make ProcessMessage private again, Use references when non-null #34181

Open

refactor: disentangle miner startup defaults from runtime options #33966

Draft

test: Add and use ElapseTime helper #32430

Are you sure you want to change the base?

test: Add and use ElapseTime helper #32430

Uh oh!

Conversation

maflcko commented May 6, 2025

Uh oh!

DrahtBot commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

DrahtBot commented May 6, 2025

Uh oh!

l0rinc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented May 7, 2025

Uh oh!

Uh oh!

w0xlt left a comment

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Jul 8, 2025

Uh oh!

maflcko commented Jul 9, 2025

Uh oh!

l0rinc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maflcko commented Jul 31, 2025

Uh oh!

l0rinc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented Jan 16, 2026

Uh oh!

l0rinc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DrahtBot commented May 6, 2025 •

edited

Loading

l0rinc left a comment •

edited

Loading

l0rinc left a comment •

edited

Loading