headerssync: Keep tests ahead of increasing params #32579

1. 
   hodlinator commented at 1:48 pm on May 21, 2025: contributor

   Motivation

   Each release we increase the values of the Headers Sync constants HEADER_COMMITMENT_PERIOD and REDOWNLOAD_BUFFER_SIZE as per doc/release-process.md. In the next (v30) or following release, it is very likely that REDOWNLOAD_BUFFER_SIZE (14827 as of v29) will exceed the target_blocks value used to test Headers Sync (15000, headers_sync_chainwork_tests.cpp).

   This would result in the HeadersSyncState::m_redownloaded_headers-buffer not reaching the REDOWNLOAD_BUFFER_SIZE-threshold during the headers_sync_chainwork_tests.cpp unit tests. Preempt this from happening by making the test depend upon the constant being updated.

   Commits

   Initial commits refactor and improve the unit tests, we then extract the constants and add the dependency, and finally add tests for the behavior around the threshold.

   (It would have been nice to add a test that failed because of not reaching the REDOWNLOAD_BUFFER_SIZE-threshold, but HeadersSyncState::PopHeadersReadyForAcceptance() will return all headers anyway due to m_process_all_remaining_headers being set when we reach the PoW threshold regardless of whether the buffer threshold is met).

   Calculation

   Date at time of calculation: 2025-05-21 Current block height: 897'662 Avg block time: ~9.8min (https://mempool.space). Added 6 months to TIME constant in contrib/devtools/headerssync-params.py: datetime(2028, 4, 6) Increased block height in contrib/devtools/headerssync-params.py (MINCHAINWORK_HEADERS) to: 920'000 Block height difference: 22'338 Days: 22'338 * 9.8min / (60min * 24h) = ~152 Estimated date for block height 920'000: 2025-10-20 REDOWNLOAD_BUFFER_SIZE value calculated by contrib/devtools/headerssync-params.py: 15002

2. refactor(test): Break up headers_sync_chainwork_tests

   * Makes tests logically separated through extraction into functions. Also enables using function-scope instances of HeadersSyncState instead of reset()-ing unique_ptr.
   * Also adds missing comment for part 4.

   34e1513efe
3. headerssync: headers_sync_chainwork test improvements

   Basically a refactor except we test for more things and require expected state transitions.
   
   * Use BOOST_REQUIRE_EQUAL for HeadersSyncState::State - Nicer failure output and prevents continuing test in nonsensical state.
   * Verify HeadersSyncState::State directly after ProcessNextHeaders().
   * CHECK for more, like Locator and result.pow_validated_headers.
   * Extract and capitalize constants.
   * Extract genesis block variable.
   
   + Make HeadersSyncState operate on spans of headers.
   + Make arith_uint256 constexpr-constructible.

   13aa0b860e
4. 
   DrahtBot commented at 1:48 pm on May 21, 2025: contributor

   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/32579.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

5. headerssync: Extract constants to header usable by test

   This makes it so that TARGET_BLOCKS in the test always keeps ahead of the constants being updated every release.
   
   Also add MAX_HEADERS_RESULTS (2000) and a smaller arbitrary value of 123 to the test-constant in order to allow for broader testing.

   e9f4f6a5d9
6. test(headerssync): Test returning of pow_validated_headers behavior

   Enabled by having the threshold constant from parent commit.

   315736d50f
7. hodlinator force-pushed on May 21, 2025
8. DrahtBot added the label CI failed on May 21, 2025
9. 
   DrahtBot commented at 2:15 pm on May 21, 2025: contributor

   🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/42636718296 LLM reason (✨ experimental): The CI failure is due to a missing include guard in the src/headerssync-params.h file, as reported by the lint checks.

   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.

10. DrahtBot removed the label CI failed on May 21, 2025

Contributors
hodlinator DrahtBot