test: make index crash tests check the saved state #36059

pull l0rinc wants to merge 2 commits into bitcoin:master from l0rinc:l0rinc/test-preserve-index-crash-state changing 1 files +7 −2
  1. l0rinc commented at 3:48 AM on August 23, 2026: contributor

    Problem: #35847 moved the unclean-shutdown test into the shared base index tests, but it checked only that each index could reopen and start background sync. Both checks also pass when the index reopens at height 0, so they do not verify that a height-100 checkpoint was saved before the simulated crash and reloaded afterward.

    Fix: The first commit records the existing false positive by asserting that each index reopens at height 0 before background sync. The second commit establishes a durable checkpoint at height 100, drains its setup notification, and changes the same assertion to the pre-crash height.

  2. test: expose missing index crash checkpoint
    `index_unclean_shutdown` previously checked only that each index could reopen and start background sync after the simulated crash.
    An empty index at height 0 satisfies both checks, so the test could pass without preserving any pre-crash checkpoint.
    
    Assert the current reopened height before background sync to make the false positive explicit.
    5aa15df60c
  3. test: preserve index crash test state
    Flush the chainstate at the current tip and drain its notification before registering any index.
    This lets each end-of-sync `Commit()` persist the pre-crash height and keeps the setup callback out of the simulated crash window.
    
    Replace the TODO-marked `0` expectation with the captured tip height.
    The check runs before background sync, so rebuilding cannot hide a missing checkpoint.
    7ea36e985a
  4. DrahtBot added the label Tests on Aug 23, 2026
  5. DrahtBot commented at 3:48 AM on August 23, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process.

    Type Reviewers
    ACK jeanpablojp, mzumsande

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #24230 (indexes: Stop using node internal types and locking cs_main, improve sync logic 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.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  6. jeanpablojp commented at 8:34 PM on August 23, 2026: contributor

    tACK 7ea36e985a900b2291ce549e468f6baab5324dc6

    Good catch. Built and ran the tests, nothing else came up.

  7. mzumsande commented at 1:12 PM on August 24, 2026: contributor

    ACK 7ea36e985a900b2291ce549e468f6baab5324dc6

    good find! I think the test coverage regression was already caused by #34897, which prevented the index from committing after Sync (because the chainstate was never flushed in the test then). #35847 seems unrelated, since it just moved the test to another file and ran it for more indexes.

  8. fanquake merged this on Aug 24, 2026
  9. fanquake closed this on Aug 24, 2026

  10. l0rinc renamed this:
    test: make index crash test check saved state
    test: make index crash tests check the saved state
    on Aug 25, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-09-09 07:56 UTC