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

1. 
   kriss39 commented at 9:21 AM on October 23, 2026: contributor

   /rest/spenttxouts/<hash> reads the undo file for every block above genesis. For a block that was stored but never connected (a side branch) or whose undo data was pruned, there is no undo position, so OpenUndoFile() fails and ReadBlockUndo() writes OpenUndoFile failed for FlatFilePos(nFile=-1, nPos=0) to debug.log with LogError. The request itself already returns 404, but each unauthenticated request for such a block adds an [error] line that looks like disk corruption.

   This checks CheckBlockDataAvailability() for BLOCK_HAVE_UNDO before reading, so nothing is logged. Pruning clears BLOCK_HAVE_UNDO, so pruned blocks are covered too. The response is unchanged.

   The new check at the end of interface_rest.py requests a stale block and fails on master because of the logged error.

2. DrahtBot added the label RPC/REST/ZMQ on Oct 23, 2026
3. 
   DrahtBot commented at 9:21 AM on October 23, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   External sites

   • Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36619
   • Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36619.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36619#author-ralph

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

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

   Type	Reviewers
   ACK	maflcko, stickies-v

   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.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

4. in src/rest.cpp:377 in be3c1db50f

    371 | @@ -372,8 +372,12 @@ static bool rest_spent_txouts(const std::any& context, HTTPRequest* req, const s
    372 |      }
    373 |  
    374 |      CBlockUndo block_undo;
    375 | -    if (pblockindex->nHeight > 0 && !chainman->m_blockman.ReadBlockUndo(block_undo, *pblockindex)) {
    376 | -        return RESTERR(req, HTTP_NOT_FOUND, hashStr + " undo not available");
    377 | +    if (pblockindex->nHeight > 0) {
    378 | +        // Blocks that were never connected or have been pruned have no undo data.
    379 | +        const bool have_undo{WITH_LOCK(cs_main, return (pblockindex->nStatus & BLOCK_HAVE_UNDO) && !chainman->m_blockman.IsBlockPruned(*pblockindex))};
   

   ---

   ---

   

   stickies-v commented at 10:13 AM on October 23, 2026:

   I think the && !chainman->m_blockman.IsBlockPruned(*pblockindex) check here is redundant?

   ---

   

   maflcko commented at 10:39 AM on October 23, 2026:

   Can this use CheckBlockDataAvailability to avoid duplicating the impl?

   ---

   

   kriss39 commented at 10:59 AM on October 23, 2026:

   Thanks, both right. PruneOneBlockFile() clears BLOCK_HAVE_UNDO, so the IsBlockPruned() check added nothing. It now uses CheckBlockDataAvailability(*pblockindex, *pblockindex, BlockStatus{BLOCK_HAVE_UNDO}), in e8af2c7488925e35ffeaf64e523cef48b70280eb.

   ---

   

   stickies-v commented at 11:33 AM on October 23, 2026:

   I think a simple pblockindex->nStatus & BLOCK_HAVE_UNDO is more straightforward tbh but either way works.

   ---

   

   maflcko commented at 2:56 PM on October 23, 2026:

   yeah, i was thinking that the rpc CheckBlockDataAvailability would provide better error messages, but if they are not used, a plain nStatus & BLOCK_HAVE_UNDO seems fine.

   ---

   

   maflcko commented at 11:56 AM on October 27, 2026:

   @kriss39 Can you address the nit? I think both reviewers would be happy to re-ack a nStatus & BLOCK_HAVE_UNDO

   ---

   

   kriss39 commented at 12:22 PM on October 27, 2026:

   Done, switched to a plain nStatus & BLOCK_HAVE_UNDO in eebc145847b4f05da61e95fcae1b420a6ce3ea0f.

5. 
   stickies-v commented at 10:17 AM on October 23, 2026: contributor

   Approach ACK

6. kriss39 force-pushed on Oct 23, 2026
7. in src/rest.cpp:377 in e8af2c7488

    371 | @@ -372,8 +372,12 @@ static bool rest_spent_txouts(const std::any& context, HTTPRequest* req, const s
    372 |      }
    373 |  
    374 |      CBlockUndo block_undo;
    375 | -    if (pblockindex->nHeight > 0 && !chainman->m_blockman.ReadBlockUndo(block_undo, *pblockindex)) {
    376 | -        return RESTERR(req, HTTP_NOT_FOUND, hashStr + " undo not available");
    377 | +    if (pblockindex->nHeight > 0) {
    378 | +        // Blocks that were never connected or have been pruned have no undo data.
    379 | +        const bool have_undo{WITH_LOCK(cs_main, return chainman->m_blockman.CheckBlockDataAvailability(*pblockindex, *pblockindex, BlockStatus{BLOCK_HAVE_UNDO}))};
   

   ---

   ---

   

   maflcko commented at 11:13 AM on October 23, 2026:

   heh, that's funny. I was talking about src/rpc/blockchain.h:void CheckBlockDataAvailability(node::BlockManager& blockman, const CBlockIndex& blockindex, bool check_for_undo) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);, but I guess this works as well.

   edit: I was thinking about CheckBlockDataAvailability from the rpc, to have more accurate error messages, but maybe that is not too important for the rest interface.

8. maflcko approved
9. 
   maflcko commented at 11:13 AM on October 23, 2026: member

   lgtm ACK e8af2c7488925e35ffeaf64e523cef48b70280eb

10. 
    stickies-v commented at 11:33 AM on October 23, 2026: contributor

    ACK e8af2c7488925e35ffeaf64e523cef48b70280eb

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

    `/rest/spenttxouts/<hash>` calls `ReadBlockUndo()` for every block above
    genesis. 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`. The
    endpoint then correctly answers 404, but every unauthenticated request
    for such a block adds an `[error]` line that looks like disk corruption.
    
    Check `BLOCK_HAVE_UNDO` first and only read the undo file when it can
    exist. Pruning clears `BLOCK_HAVE_UNDO`, so this covers pruned blocks
    too. The response is unchanged.
    
    The new interface_rest.py check requests a side-branch block and fails
    on master because of the logged error.

    eebc145847
12. kriss39 force-pushed on Oct 27, 2026
13. 
    maflcko commented at 1:54 PM on October 27, 2026: member

    review ACK eebc145847b4f05da61e95fcae1b420a6ce3ea0f 🛠

    <details><summary>Show signature</summary>

    Signature:

    untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    trusted comment: review ACK eebc145847b4f05da61e95fcae1b420a6ce3ea0f 🛠
    yo6CqrBQUYQJcnbpnkRphcLtq07ySn/lKcrM2Wdeyl1eP6uBy0nsbiggPPNrSEGDfyrHwX4GqMgczZx401+sBQ==
    

    </details>

14. DrahtBot requested review from stickies-v on Oct 27, 2026
15. 
    stickies-v commented at 3:00 PM on October 27, 2026: contributor

    re-ACK eebc145847b4f05da61e95fcae1b420a6ce3ea0f

Contributors

kriss39DrahtBotstickies-vmaflcko

Review Requested
stickies-v