#36619 rest: don't log an error for /spenttxouts on a block without undo data

ReviewabilityReviewsAgreementSize
#36619 kriss39 rest: don't log an error for /spenttxouts on a block without undo dataReady2Positive6 + 8 tests
  • Prevent spurious error messages in debug.log when spent outputs are requested for blocks without undo data
  • Stops unauthenticated REST queries on pruned or side-branch blocks from spamming false disk corruption errors
  • Ready to review
  • Concept approval without stated reasons (stickies-v, maflcko)
  • Both reviewers re-ACKed after the author simplified the block status check (maflcko, stickies-v)
  • 6 lines added or modified outside tests
  • 8 lines added or modified in tests
  • 2 lines removed in total
  • 2 files, 1 commits
RPC / REST / ZMQ#38 of 55
P3 · bug fix
  • P3 because it fixes spurious error logs that mimic disk corruption on normal 404 REST queries
  • Affects node operators querying spent transaction outputs on pruned or side-chain blocks
  • Ranking pass: Moved up 9: unauthenticated REST requests emitting fake disk-corruption errors misleads operators, slightly above pure interface hygiene.
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

Check block undo data availability before attempting to read undo data in the `/rest/spenttxouts/<hash>` REST endpoint. If `BLOCK_HAVE_UNDO` is not set on the block index, the endpoint returns an HTTP 404 immediately without calling `ReadBlockUndo()`. A functional test verifies that querying a side-branch block returns a 404 without writing undo failure errors to `debug.log`.

Problem: When `/rest/spenttxouts/<hash>` is requested for a block that lacks undo data—such as pruned blocks or disconnected side branches—`ReadBlockUndo()` fails and logs an `[error] OpenUndoFile failed for FlatFilePos(nFile=-1, nPos=0)` to `debug.log`. Although the REST endpoint correctly returns a 404 response, unauthenticated requests can spam the log with false error messages that mimic disk corruption.

Evidence of importance:

  • Commit and PR description: "For a block that was stored but never connected (a side branch) or whose undo data was pruned, GetUndoPos() is null, so OpenUndoFile() fails and ReadBlockUndo() writes 'OpenUndoFile failed for FlatFilePos(nFile=-1, nPos=0)' to debug.log with LogError."
  • PR description notes the test fails on master: "The new check at the end of interface_rest.py requests a stale block and fails on master because of the logged error."

Risk of a mistake: medium (rpc_api). A mistake in inspecting block status flags or locking cs_main could return false 404s for valid undo data or trigger lock contention on REST requests.

Changed since the previous assessment: The implementation directly checks `nStatus & BLOCK_HAVE_UNDO` under `cs_main` instead of using a helper function.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • nothing: all comments addressed and both reviewers re-ACKed the latest push

Agreement

Positive

support without stated reasons, no open objection (stickies-v, stickies-v, maflcko, maflcko, stickies-v)

Thread notes: maflcko asked the author to adopt the direct bitwise check suggested earlier by stickies-v. The author updated the PR to use nStatus & BLOCK_HAVE_UNDO, and both reviewers re-ACKed.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
stickies-v (member)suggestion, maintenanceRemove the redundant IsBlockPruned checkfixed · fix pushed · author confirmed IsBlockPruned is redundant because BLOCK_HAVE_UNDO is cleared on prune, and removed it in pushansweredyes2026-10-23: 'I think the `&& !chainman->m_blockman.IsBlockPruned(*pblockindex)` check here is redundant?'
Status from: 2026-10-23: kriss39: 'PruneOneBlockFile() clears BLOCK_HAVE_UNDO, so the IsBlockPruned() check added nothing.'
maflcko (member)suggestion, maintenanceUse CheckBlockDataAvailability to avoid duplicating implementationfixed · fix pushed · switched to CheckBlockDataAvailability in e8af2c7488925e35ffeaf64e523cef48b70280eb and acceptedansweredyes2026-10-23: 'Can this use `CheckBlockDataAvailability` to avoid duplicating the impl?'
Status from: 2026-10-23: maflcko: 'lgtm ACK e8af2c7488925e35ffeaf64e523cef48b70280eb'
stickies-v (member)suggestion, maintenanceConsider a direct bitwise check on nStatus instead of CheckBlockDataAvailabilityfixed · fix pushed · author implemented the plain nStatus check in eebc145847b4f05da61e95fcae1b420a6ce3ea0f and reviewer re-ACKedansweredyes2026-10-23: 'I think a simple `pblockindex->nStatus & BLOCK_HAVE_UNDO` is more straightforward tbh but either way works.'
Status from: 2026-10-27: stickies-v: 're-ACK eebc145847b4f05da61e95fcae1b420a6ce3ea0f'
maflcko (member)suggestion, maintenanceAddress the nit to switch to a direct nStatus & BLOCK_HAVE_UNDO checkfixed · fix pushed · author switched to plain nStatus & BLOCK_HAVE_UNDO in eebc145847b4f05da61e95fcae1b420a6ce3ea0fansweredyes2026-10-27: 'Can you address the nit? I think both reviewers would be happy to re-ack a `nStatus & BLOCK_HAVE_UNDO`'
Status from: 2026-10-27: maflcko: 'review ACK eebc145847b4f05da61e95fcae1b420a6ce3ea0f 🛠'

Support:

  • stickies-v (member) (ACK, 2026-10-23): ACK e8af2c7488925e35ffeaf64e523cef48b70280eb [not substantive]
  • stickies-v (member) (Approach ACK, 2026-10-23): (no reason given) [not substantive]
  • maflcko (member) (ACK, 2026-10-23): lgtm ACK e8af2c7488925e35ffeaf64e523cef48b70280eb [not substantive]
  • maflcko (member) (ACK, 2026-10-27): review ACK eebc145847b4f05da61e95fcae1b420a6ce3ea0f 🛠 [not substantive]
  • stickies-v (member) (ACK, 2026-10-27): re-ACK eebc145847b4f05da61e95fcae1b420a6ce3ea0f [not substantive]

Participants:

  • maflcko (member, support, 6 statements 2026-10-23 to 2026-10-27): suggested CheckBlockDataAvailability then asked to switch to nStatus check, re-ACKed update
  • stickies-v (member, support, 5 statements 2026-10-23 to 2026-10-27): reviewed redundancy of prune check, suggested direct bitwise check, Approach ACKed and re-ACKed updates

Files

8 lines under test/bench/ci.

  • src/rest.cpp +6/-2
  • test/functional/interface_rest.py +8/-0

Card

The short text the ranking pass compares across PRs.

Suppresses spurious disk-corruption error logs when querying `/rest/spenttxouts/<hash>` for blocks without undo data. Today, unauthenticated requests for pruned blocks or disconnected side branches cause OpenUndoFile to fail and log an error to debug.log before returning an HTTP 404. This PR verifies that BLOCK_HAVE_UNDO is set before attempting to read undo files, keeping the HTTP 404 response unchanged while preventing debug.log spam.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-29T01:22, confidence high, input hash 3d0332bc0f424ac2
thread read through 2026-10-27 (13 statements), head eebc145847
labels: RPC/REST/ZMQ

Categories

RPC / REST / ZMQ

P3 · bug fix (ranking pass: Moved up 9: unauthenticated REST requests emitting fake disk-corruption errors misleads operators, slightly above pure interface hygiene.)

Prevents spurious `[error]` log entries that mimic disk corruption when requesting spent TXOs for pruned or side-branch blocks. An unauthenticated endpoint was invoking `ReadBlockUndo()` on blocks without undo data, spamming `debug.log` despite properly returning a 404.

Membership: Matches RPC/REST/ZMQ label and modifies REST endpoint handler in src/rest.cpp.

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