Merge bitcoin/bitcoin#36059: test: make index crash test check saved state
What the developer wrote
Authored by merge-script
Merge bitcoin/bitcoin#36059: test: make index crash test check saved state
7ea36e985a900b2291ce549e468f6baab5324dc6 test: preserve index crash test state (Lőrinc)
5aa15df60c49aacd3b3dafe13a4ceded9cec07cc test: expose missing index crash checkpoint (Lőrinc)
Pull request description:
**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.
ACKs for top commit:
jeanpablojp:
tACK 7ea36e985a900b2291ce549e468f6baab5324dc6
mzumsande:
ACK 7ea36e985a900b2291ce549e468f6baab5324dc6
Tree-SHA512: 0dca2bdd978c5df4acbb01692bb2058e74efa70da3d7191687628075a680a57848ddda8087629b5d743d9a1648d7dc849fda9ff487252136a0c91cdfda33ba32
What changed, and why it matters
This is a test-only change for Bitcoin Core. It strengthens an existing automated test that simulates a crash to make sure that, after a restart, an index resumes from the correct block height rather than silently starting over from block 0. No production code was changed, and there is no fix for a live security vulnerability in this commit.
No action required. This is a benign test improvement. If reviewing for security, note that any underlying index checkpointing issue would be in pre-existing production code, not addressed by this commit.
Security signals we found
Test-only change; no production code modified
Strengthens crash-recovery assertion for index state
Removes false positive where index reopening at height 0 would pass
No runtime bug fix or vulnerability patch present in diff
Evidence from the diff
The commit modifies src/test/baseindex_tests.cpp. It updates the index_unclean_shutdown test to force a durable chainstate flush at the current tip, drain pending validation notifications before registering any index, and then assert that each index reloads at the pre-crash height (tip_height) instead of merely starting background sync. The first merged commit intentionally recorded the existing false positive (reopening at height 0); the second merged commit fixed the test to verify the real expected behavior. This is purely a test-hardening change.
Changed components
src/test/baseindex_tests.cppInspect captured patch +7 / −2
### src/test/baseindex_tests.cpp
@@ -108,11 +108,15 @@ BOOST_FIXTURE_TEST_CASE(baseindex_no_commit_ahead_of_flush, TestChain100Setup)
}
// Test shutdown between BlockConnected and ChainStateFlushed notifications,
-// make sure index is not corrupted and is able to reload.
+// make sure index is not corrupted and reloads at the last committed height.
BOOST_FIXTURE_TEST_CASE(index_unclean_shutdown, TestChain100Setup)
{
Chainstate& chainstate = Assert(m_node.chainman)->ActiveChainstate();
const CChainParams& params = Params();
+ const int tip_height{WITH_LOCK(cs_main, return chainstate.m_chain.Height())};
+ chainstate.ForceFlushStateToDisk();
+ // Drain the notification before registering any index.
+ m_node.chain->context()->validation_signals->SyncWithValidationInterfaceQueue();
for (const auto& [index_name, make_index] : INDEX_FACTORIES) {
BOOST_TEST_INFO_SCOPE(index_name);
{
@@ -144,7 +148,8 @@ BOOST_FIXTURE_TEST_CASE(index_unclean_shutdown, TestChain100Setup)
{
auto index{make_index(m_node)};
BOOST_REQUIRE(index->Init());
- // Make sure the index can be loaded.
+ // Make sure the index reloads from the pre-crash commit.
+ BOOST_CHECK_EQUAL(index->GetSummary().best_block_height, tip_height);
BOOST_REQUIRE(index->StartBackgroundSync());
index->Stop();
}Why this scored 13/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.