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

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

Start a node with -blockfilterindex=1 and let it mine/receive some blocks.
Stop the node, delete <datadir>/<chain>/indexes/blockfilter, restart with -blockfilterindex=1.
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.

DrahtBot added the label RPC/REST/ZMQ on Jul 29, 2026

DrahtBot commented at 6:18 PM on July 29, 2026: contributor

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

External sites

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

Reviews

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

Type	Reviewers
Concept ACK	maflcko, fjahr

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36257 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36257.svg"></sub> (qa: assert_equals -> assert_true/assert_false by hodlinator)
#35229 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35229.svg"></sub> (refactor: Use CBlockIndex parameters as reference by optout21)

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.

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>

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:

This is incorrect, getblockfilter does not do this: https://github.com/bitcoin/bitcoin/blob/67efced1fc83a0b7215cc1513e7c4754fee0f12f/src/rpc/blockchain.cpp#L3008

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.

DrahtBot added the label CI failed on Jul 30, 2026

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

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

MicSm force-pushed on Jul 31, 2026

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.

DrahtBot added the label Needs rebase on Aug 5, 2026

MicSm force-pushed on Aug 5, 2026

MicSm commented at 3:56 PM on August 5, 2026: none

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

done

DrahtBot removed the label Needs rebase on Aug 5, 2026

DrahtBot removed the label CI failed on Aug 5, 2026

sedited requested review from fjahr on Sep 17, 2026

in test/functional/rpc_scanblocks.py:154 in 55abc8b820

 149 | +        requested range is not yet available.
 150 | +        """
 151 | +        node = self.nodes[0]
 152 | +        wallet = MiniWallet(node)
 153 | +
 154 | +        def bfi():

fjahr commented at 9:50 PM on September 17, 2026:

This is a bit unusual but ok for me. This is always used for checking "synced" so I would put this into the function. And I would prefer a bit more expessive name like index_synced or so.

MicSm commented at 5:17 PM on November 3, 2026:

That's already done. The helper is gone in the new version, see the test. The test does not poll for "synced" anymore: since #36618, start_node() and sync_all() already wait for the index to reach the tip. The only check of the index state that is left is a direct assert_equal on getindexinfo.

in test/functional/rpc_scanblocks.py:143 in 55abc8b820

 137 | @@ -137,6 +138,64 @@ def run_test(self):
 138 |          # test that null scanobjects is rejected for start
 139 |          assert_raises_rpc_error(-1, "scanobjects argument is required for the start action", node.scanblocks, "start", None)
 140 |  
 141 | +        self.test_scanblocks_unindexed_range()
 142 | +
 143 | +    def test_scanblocks_unindexed_range(self):

fjahr commented at 9:54 PM on September 17, 2026:

We usually add the implementation of new test cases above run_test, not below.

MicSm commented at 5:18 PM on November 3, 2026:

Sure, both new test cases (test_unindexed_range and test_unreadable_range) are now above run_test, check it please.

in test/functional/rpc_scanblocks.py:166 in 55abc8b820

 161 | +        # leaves the node briefly behind the tip. Missing that window is harmless.
 162 | +        self.generate(node, 3000, sync_fun=self.no_op)
 163 | +        self.wait_until(lambda: bfi()["synced"])
 164 | +
 165 | +        self.stop_node(0)
 166 | +        self.cleanup_folder(node.chain_path / "indexes" / "blockfilter")

fjahr commented at 9:58 PM on September 17, 2026:

This doesn't make sense, you are waiting for the index to be synced just to delete it without doing anything else.

MicSm commented at 5:19 PM on November 3, 2026:

Agreed, that was pointless, so the new version does not delete the index at all. For the "not indexed yet" case, node2 loads an assumeutxo snapshot, so the index cannot get past the cached chain until the node has a peer. For the "cannot read" case, only the filter file is deleted and the index database is kept, so the node starts with a synced index and the lookup fails every time.

in test/functional/rpc_scanblocks.py:162 in 55abc8b820

 157 | +        _, spk, addr = getnewdestination()
 158 | +        wallet.send_to(from_node=node, scriptPubKey=spk, amount=1 * COIN)
 159 | +        match_hash = self.generate(node, 1, sync_fun=self.no_op)[0]
 160 | +        # A large enough chain that rebuilding the wiped index after a restart
 161 | +        # leaves the node briefly behind the tip. Missing that window is harmless.
 162 | +        self.generate(node, 3000, sync_fun=self.no_op)

fjahr commented at 10:00 PM on September 17, 2026:

No normal functional test should require mining this many blocks. And we don't leave the actual testing to chance. The test either needs to work deterministically or it is of no use for us, with maybe some very rare exceptions.

MicSm commented at 5:20 PM on November 3, 2026:

I agree, the new test does not depend on timing anymore, it mines 100 blocks, which is the minimum needed to build the regtest snapshot chain (its hash at height 299 is hardcoded in the chainparams). It is the same recipe as in tool_bitcoin_chainstate.py. The second case does not mine any blocks. Both cases give the same result on every run.

in src/rpc/blockchain.cpp:2693 in 55abc8b820

2699 | -
2700 | -                            if (!CheckBlockFilterMatches(chainman.m_blockman, blockindex, needle_set)) {
2701 | -                                continue;
2702 | -                            }
2703 | +            if (!index->LookupFilterRange(start_block, end_range, filters)) {
2704 | +                // Report the failure like getblockfilter: still indexing vs. corruption.

fjahr commented at 10:08 PM on September 17, 2026:

This reference to the other RPC is unnecessary for readers to understand the code and it could get stale too easily, so it should be removed

MicSm commented at 5:21 PM on November 3, 2026:

Yes, I removed it and the comment now only describes the condition in the line below it and does not mention the other RPC.

in test/functional/rpc_scanblocks.py:167 in 55abc8b820

 162 | +        self.generate(node, 3000, sync_fun=self.no_op)
 163 | +        self.wait_until(lambda: bfi()["synced"])
 164 | +
 165 | +        self.stop_node(0)
 166 | +        self.cleanup_folder(node.chain_path / "indexes" / "blockfilter")
 167 | +        self.start_node(0, extra_args=["-blockfilterindex=1"])

fjahr commented at 10:29 PM on September 17, 2026:

No need to re-apply the args here when they are unchanged from the test params.

in test/functional/rpc_scanblocks.py:183 in 55abc8b820

 178 | +                assert_equal(out["completed"], True)
 179 | +                assert match_hash in out["relevant_blocks"]
 180 | +
 181 | +            self.log.info("scanning an already-indexed prefix succeeds while still behind the tip")
 182 | +            # Genesis is indexed first, so it is available while the tip is not.
 183 | +            while not bfi()["synced"]:

fjahr commented at 10:38 PM on September 17, 2026:

Bruteforcing this in a loop is really not necessary, we have wait_until etc. for this.

in test/functional/rpc_scanblocks.py:170 in 55abc8b820

 165 | +        self.stop_node(0)
 166 | +        self.cleanup_folder(node.chain_path / "indexes" / "blockfilter")
 167 | +        self.start_node(0, extra_args=["-blockfilterindex=1"])
 168 | +        tip = node.getblockcount()
 169 | +
 170 | +        if not bfi()["synced"]:

fjahr commented at 10:43 PM on September 17, 2026:

As mentioned above, skipping the actual testing under some race condition makes the test basically useless for our purposes. You should look for a more robust way of testing this or consider removing the test. It seems like the getblockfilter RPC doesn't have coverage for this. Maybe it's possible with a partial sync like we are using in feature_assumeutxo.py.

MicSm commented at 5:27 PM on November 3, 2026:

Thanks, this was the right hint and the new test does exactly that: node2 loads the regtest snapshot while it has no peers, so its filter index stays at height 199 while the active tip is at 299. A scan to the tip fails with "still being indexed", and a scan up to height 199 succeeds. No timing is involved.

This also showed a problem in my previous version: in this state the index reports "synced", so the old check returned "index corruption" instead of "still being indexed". That is why the error is now chosen by checking if the best block of the index covers the requested range.

Side note: while testing this, I found an pre-existing bug in getblockfilter. In the same state it also returns the wrong error ("index corruption" for a block above the snapshot). It is not related to this change, so I think it belongs in a separate PR. Anyone who wants to pick it up is welcome.

in src/rpc/blockchain.cpp:2676 in 55abc8b820

2670 | @@ -2671,6 +2671,10 @@ static RPCMethod scanblocks()
2671 |          g_scanfilter_progress_height = start_block_height;
2672 |          bool completed = true;
2673 |  
2674 | +        // Only used to classify a failed range lookup below; the requested range
2675 | +        // may be available even while the index is behind the tip.
2676 | +        const bool index_ready = index->BlockUntilSyncedToCurrentChain();

fjahr commented at 10:45 PM on September 17, 2026:

This status may be outdated at the end of a long scan.

MicSm commented at 5:29 PM on November 3, 2026:

Agreed. The flag is gone. The state of the index is now read at the moment the lookup fails, with GetSummary(), and the error is chosen by checking if the best block of the index covers the chunk that failed. So the check always uses the current state, also at the end of a long scan.

The call to BlockUntilSyncedToCurrentChain() before the loop is still there, but only so that a synced index finishes processing the blocks that were just connected. Its return value is not used anymore.

in src/rpc/blockchain.cpp:2695 in 55abc8b820 outdated

2701 | -                                continue;
2702 | -                            }
2703 | +            if (!index->LookupFilterRange(start_block, end_range, filters)) {
2704 | +                // Report the failure like getblockfilter: still indexing vs. corruption.
2705 | +                if (!index_ready) {
2706 | +                    throw JSONRPCError(RPC_MISC_ERROR, "Block filters are still in the process of being indexed.");

fjahr commented at 10:47 PM on September 17, 2026:

Why throw here instead of returning a partial result?

MicSm commented at 5:30 PM on November 3, 2026:

I thought about it and chose the error for these reasons:

"completed" is documented as "true if the scan was not aborted". A caller that never calls abort has no reason to check it. That's exactly why the silent gap was not noticed before. A partial result with completed=false would put the same callers in the same trap.
A partial result cannot say why it stopped. We would need a new field for that, and that is an API change with release notes.
The result would often be empty anyway. The lookup works in chunks of 10000 blocks, and in the assumeutxo case the first chunk already fails.
A caller that only wants the part that is already indexed can scan up to best_block_height from getindexinfo.

An error also matches what getblockfilter does for a missing filter. If you still prefer a partial result, I can change it, but then I think it should be a separate change with its own field.

in test/functional/rpc_scanblocks.py:197 in 55abc8b820

 192 | +
 193 | +        self.log.info("once the index is synced the same scan completes and finds the match")
 194 | +        self.wait_until(lambda: bfi()["synced"])
 195 | +        out = node.scanblocks("start", [f"addr({addr})"])
 196 | +        assert_equal(out["completed"], True)
 197 | +        assert match_hash in out["relevant_blocks"]

fjahr commented at 10:57 PM on September 17, 2026:

I don't think the success case test needs to be repeated here at the end. Also you already do the same check in racy part above if the index is already synced.

MicSm commented at 5:31 PM on November 3, 2026:

I removed it, the success case after the sync is already covered by the existing checks in run_test.

in test/functional/rpc_scanblocks.py:173 in 55abc8b820

 168 | +        tip = node.getblockcount()
 169 | +
 170 | +        if not bfi()["synced"]:
 171 | +            self.log.info("scanning to the not-yet-indexed tip errors instead of skipping silently")
 172 | +            try:
 173 | +                out = node.scanblocks("start", [f"addr({addr})"], 0, tip)

fjahr commented at 11:01 PM on September 17, 2026:

tip will be used by default so the variable can be removed and the call here simplified.

MicSm commented at 5:32 PM on November 3, 2026:

Done. The variable is gone. The scan to the tip is called without heights, and the only explicit limit left is passed as stop_height=START_HEIGHT.

fjahr commented at 11:07 PM on September 17, 2026: contributor

Concept ACK on handling this but the test needs a lot of work

sedited commented at 4:19 PM on October 30, 2026: contributor

@MicSm are you still working on this? There has been pending review here for a while.

MicSm commented at 5:52 PM on November 1, 2026: none

Concept ACK on handling this but the test needs a lot of work

yes, I see, working on it

MicSm commented at 5:53 PM on November 1, 2026: none

@MicSm are you still working on this? There has been pending review here for a while.

Sure

MicSm force-pushed on Nov 3, 2026

rpc: fail scanblocks when block filter range is unavailable

scanblocks reads block filters over a range with LookupFilterRange. A
failed lookup was ignored, so the scan skipped the affected blocks and
still returned "completed": true, hiding the gap from the caller.

Report the failure instead. It is "still being indexed" unless the
synced index already covers the range, in which case it indicates index
corruption. A scan over a range that is already indexed keeps working
while the index is behind the tip.

Add functional test coverage for both errors: a scan past the height
the index can reach during assumeutxo background validation, and a scan
over filters whose file has been deleted.

64eca3dd8c

MicSm force-pushed on Nov 3, 2026