#35731 Indexes: Harden the flush-error notification invariant

ReviewabilityReviewsAgreementSize
#35731 arejula27 Indexes: Harden the flush-error notification invariantAuthor silent 109d0Crickets18 + 103 tests
  • Ensure failed disk flushes reliably trigger node shutdown to protect chainstate integrity
  • Prevent future file flush error paths from returning failure without aborting the node
  • Review #35714 first
  • CI is failing and author has been silent for over two months
  • No review verdicts recorded
  • No substantive review comments or feedback yet
  • No participants have weighed in on the proposed refactoring
  • 18 lines added or modified outside tests
  • 103 lines added or modified in tests
  • 10 lines removed in total
  • 5 files, 4 commits
Validation#42 of 53
P3 · cleanup
  • P3 because it is a defensive refactoring that hardens error handling rather than fixing an active bug
  • Ensures all failed block or undo disk flushes reliably trigger node shutdown
  • Protects maintainers from future flush code paths failing silently without aborting
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Couples disk flush operations with flush-error notification handling by introducing a private `BlockManager::FlushFile()` helper in `blockstorage.cpp`. Replaces individual flush and notification calls across block and undo file flushes with this helper to guarantee that any failed file flush emits a notification. Adds unit tests exercising separate block and undo file flush failures to verify notification behavior.

Problem: When `FlushChainstateBlockFile()` fails, callers such as `FlushStateToDisk()` rely on the invariant that a fatal flush-error notification (which initiates node shutdown) has already been fired. Today, this invariant depends on each individual branch in `FlushBlockFile` and `FlushUndoFile` remembering to invoke the notification before returning failure, creating a risk that future flush code paths return an error without triggering node shutdown.

Evidence of importance:

  • PR description: 'A future flush source (a new file type, a new error branch) that sets success = false without notifying would make FlushStateToDisk() return an error without AbortNode().'

Scope: Includes commits from PR #35714 in its branch history.

Reviewability

Stale · Author silent 109d

No author activity for 109 days; a review may wait a long time for a response.

Agreement

Crickets

no substantive comment either way

Objections, suggestions, and questions: none recorded.

Dependencies

Depends on: #35714

Files

103 lines under test/bench/ci.

  • src/test/blockmanager_tests.cpp +79/-0
  • src/test/chainstate_write_tests.cpp +24/-0
  • src/node/blockstorage.cpp +11/-7
  • src/node/blockstorage.h +5/-0
  • src/validation.cpp +2/-3

Card

The short text the ranking pass compares across PRs.

This PR refactors block and undo file flushing in BlockManager to encapsulate file-level flushing and error notification into a single FlushFile helper, ensuring any flush failure invariably triggers the flush-error notification. It solves the risk of future code paths reporting a flush failure to FlushStateToDisk without properly initiating node shutdown via AbortNode. Unit tests are added to verify that both block and undo flush failures trigger notifications. It is a defensive internal refactor that builds on top of PR 35714.

Data

dossier JSON · extract JSON · model ledger, generated 2026-09-18T03:14, confidence high, input hash cb5b27b455caeb7a
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: UTXO Db and Indexes, Needs rebase, CI failed · draft

Categories

Validation

P3 · cleanup

Hardens the flush-error notification invariant to ensure failed block or undo file flushes reliably trigger node shutdown. As noted in the description, callers rely on the invariant that a fatal flush error notification has fired, but it previously depended on individual branches remembering to invoke it; this centralizes the check to prevent future flush branches from silently failing without an AbortNode.

Membership: Changes block and undo file flush handling in src/node/blockstorage.cpp and src/validation.cpp, covered under block and undo file storage and chainstate integrity.

Factors: security/stability 1, bug 0, performance 0, user value 0, leverage 0