rpc: fail scanblocks when block filter range is unavailable #35837

pull MicSm wants to merge 1 commits into bitcoin:master from MicSm:rpc/scanblocks-require-synced-index changing 2 files +81 −13
  1. MicSm commented at 6:17 PM on July 29, 2026: none

    The issue

    scanblocks reads block filters over a range via LookupFilterRange. When the block filter index is behind the active chain (e.g. right after startup, while it is still syncing in the background), the lookup for the not yet indexed range fails. The old code ignored that failure, advanced start_index to the end of the chunk, and kept going - so the scan skipped every unindexed block and still returned "completed": true.

    The result is a silent gap: a caller gets a relevant_blocks list that looks fine but is missing any match in the unindexed range, with no way to distinguish "no matches" from "range was never scanned".

    Note that "completed" only means the scan was not aborted - it does not mean every filter in the requested range was actually read.

    Steps to reproduce

    1. Start a node with -blockfilterindex=1 and let it mine/receive some blocks.
    2. Stop the node, delete <datadir>/<chain>/indexes/blockfilter, restart with -blockfilterindex=1.
    3. Immediately (before the index finishes rebuilding) call scanblocks start '["addr(<addr>)"]'.

    Before this change the call returns "completed": true with an empty/partial relevant_blocks, silently skipping the range the index had not rebuilt yet.

    How it is fixed and why

    Mirror getblockfilter's error classification:

    • Call BlockUntilSyncedToCurrentChain() only to obtain an index_ready flag. Do not fail unconditionally when the index is still behind the tip - the requested range may already be available.
    • If LookupFilterRange fails and the index is not ready, throw the same "still in the process of being indexed" RPC_MISC_ERROR that getblockfilter uses.
    • If LookupFilterRange fails after the index is ready, throw RPC_INTERNAL_ERROR (unexpected / corruption), instead of skipping the chunk.

    This closes the silent gap without rejecting scans whose range is already covered while the index catches up to the tip.

    The diff also reindents the match loop: inverting if (LookupFilterRange(...)) into an early throw removes one nesting level from the existing body. That reindentation is a consequence of the bug fix, not a standalone style change.

    Tests

    • test/functional/rpc_scanblocks.py - new test_scanblocks_unindexed_range covers the modified code: it pads the chain, wipes the filter index, restarts, and while the index is behind the tip asserts that a scan to the tip returns the indexing error, that a scan over an already-written prefix (genesis) still succeeds, and that after sync the match is found.
    • The existing run_test cases (which wait_until(... synced ...) before scanning) continue to exercise the normal synced path.

    AI / tooling note

    I found this regression while reviewing the RPC with an AI-assisted tool. I reproduced the unsynced-index behavior myself, chose to mirror getblockfilter's existing guard, and verified the fix with the functional test above. This PR text and any review replies are my own.

  2. DrahtBot added the label RPC/REST/ZMQ on Jul 29, 2026
  3. DrahtBot commented at 6:18 PM on July 29, 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/35837.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

    Type Reviewers
    Concept ACK maflcko

    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:

    • #35229 (refactor: Use CBlockIndex parameters as reference by optout21)
    • #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-->

    LLM Linter (✨ experimental)

    Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

    • node.scanblocks("start", [f"addr({addr})"], 0, tip) in test/functional/rpc_scanblocks.py
    • node.scanblocks("start", [f"addr({addr})"], 0, 0) in test/functional/rpc_scanblocks.py

    <sup>2026-08-05 15:56:06</sup>

  4. maflcko commented at 6:52 AM on July 30, 2026: member

    I found this regression while reviewing the RPC with an AI-assisted tool I maintain, Boffin 0.3.2.

    Thx for following the projects AI policy, and Concept ACK. However, I think the link to the tool can be removed and this can just say "I found this regression while reviewing the RPC with an AI-assisted tool". The tool is "a layer for agents", but I am sure any vanilla agent will find the same issue with the same prompt. E.g:

    <details><summary>LLM output</summary>

    cdx -c model=gpt-5.4-mini exec 'Review the scanblocks RPC for correctness' 2>/dev/null 
    Findings:
    
    1. [`src/rpc/blockchain.cpp`](src\/rpc\/blockchain.cpp#L2683) silently ignores `LookupFilterRange()` failures. If the block filter DB read fails or the range is otherwise unreadable, the RPC just skips that chunk, keeps going, and can still return `completed: true` with missing `relevant_blocks`. That is a correctness bug, not just an error-path detail. It should surface the failure to the caller or at least mark the scan incomplete.
    
    2. [`src/rpc/blockchain.cpp`](src\/rpc\/blockchain.cpp#L2664) has an abort race during startup. `g_scanfilter_should_abort_scan` is cleared only after the scan has already reserved the global slot and finished descriptor/index setup. If another thread issues `scanblocks("abort")` in that window, it can set the flag and then have it overwritten by the startup path, causing the scan to ignore a valid abort request. The reset needs to happen before any long setup, or the state transition needs to be synchronized.
    
    No other correctness issue stood out in the range logic itself.
    

    </details>

  5. in src/rpc/blockchain.cpp:2649 in 9467446a9f
    2645 | @@ -2646,6 +2646,13 @@ static RPCMethod scanblocks()
    2646 |          CHECK_NONFATAL(start_index);
    2647 |          CHECK_NONFATAL(stop_block);
    2648 |  
    2649 | +        // Bail out if the filter index is behind the active chain, as getblockfilter
    


    fjahr commented at 8:47 PM on July 30, 2026:
  6. fjahr commented at 8:47 PM on July 30, 2026: contributor

    getblockfilter already guards against this state and returns an error;

    What this is doing here is different from what getblockfilter does. There we don't fail unconditionally if the index isn't synced. We only fail if we the range isn't available. This makes sense because we may have the range available even if the index isn't synced to the tip yet. I don't see why we shouldn't be able to do it the same way here.

  7. DrahtBot added the label CI failed on Jul 30, 2026
  8. in test/functional/rpc_scanblocks.py:180 in 9467446a9f
     175 | +            if behind_tip:
     176 | +                assert (not out["completed"]) or (match_hash in out["relevant_blocks"]), \
     177 | +                    "scanblocks skipped an unindexed range but still reported completed=true"
     178 | +            # Index had already caught up; retry to catch it mid-sync.
     179 | +        else:
     180 | +            raise AssertionError("filter index synced too quickly to exercise the unsynced path")
    


    DrahtBot commented at 8:55 PM on July 30, 2026:

    the ci fails here:

                                   AssertionError: filter index synced too quickly to exercise the unsynced path
  9. MicSm renamed this:
    rpc: fail scanblocks when block filter index is not caught up
    rpc: fail scanblocks when block filter range is unavailable
    on Jul 31, 2026
  10. MicSm force-pushed on Jul 31, 2026
  11. MicSm commented at 4:29 PM on August 4, 2026: none

    getblockfilter already guards against this state and returns an error;

    What this is doing here is different from what getblockfilter does. There we don't fail unconditionally if the index isn't synced. We only fail if we the range isn't available. This makes sense because we may have the range available even if the index isn't synced to the tip yet. I don't see why we shouldn't be able to do it the same way here.

    You're right - thanks. getblockfilter does not fail merely because index is behind the tip, so it fails when the requested filter isn't available (and then classifies it still-indexing vs unexpected/corruption).

    So, I updated to match this: BlockUntilSyncedToCurrentChain() is only used as an index_ready flag and a failed LookupFilterRange is classified the same way. A scan whose requested range is already available while the tip is ahead, now succeeds.

  12. DrahtBot added the label Needs rebase on Aug 5, 2026
  13. rpc: fail scanblocks when block filter range is unavailable
    scanblocks reads block filters over a range with LookupFilterRange. A
    failed read was ignored, so the scan skipped the affected blocks and
    still returned "completed": true, hiding the gap from the caller.
    
    Report the failure instead. As getblockfilter does, it is returned as
    "still being indexed" while the index is behind the tip and as an
    internal error once synced, so a scan whose range is already available
    keeps working.
    
    Add functional test coverage for the unindexed-tip error and for a
    successful scan over an already-indexed range.
    55abc8b820
  14. MicSm force-pushed on Aug 5, 2026
  15. MicSm commented at 3:56 PM on August 5, 2026: none

    🐙 This pull request conflicts with the target branch and needs rebase.

    done

  16. DrahtBot removed the label Needs rebase on Aug 5, 2026
  17. DrahtBot removed the label CI failed on Aug 5, 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-08-25 15:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me