test: Wait for index sync in all tests #36618

Fixes #36564 by adding the missing wait on the index sync.

Diff to reproduce:

diff --git a/src/util/thread.cpp b/src/util/thread.cpp
index 5b20e807b4..2e988c9eea 100644
--- a/src/util/thread.cpp
+++ b/src/util/thread.cpp
@@ -8,2 +8,3 @@
 #include <util/log.h>
+#include <util/time.h>
 #include <util/threadnames.h>
@@ -17,2 +18,3 @@ void util::TraceThread(std::string_view thread_name, std::function<void()> threa
     util::ThreadRename(std::string{thread_name});
+    UninterruptibleSleep(999ms);
     try {

Command to reproduce (run twice to create the cache the first time, and reproduce on the second):

./bld-cmake/test/functional/wallet_fast_rescan.py | grep WARNING

On failure, the log will say something like:

 - 2026-10-18T05:06:44.451973Z [http.01] [wallet/scan.cpp:142] [ShouldFetchBlock] [scan] Fast rescan: inspect block 200 [4c063a93914ec0cb1604297f2f601dab9b9342bf073e6b80e1c11efd71714e2e] (WARNING: block filter not found!)

<!--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/36618
• Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36618.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36618#author-ralph

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

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

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:

• #36629 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36629.svg">} (Add output script index and corresponding script history RPC by andrewtoth)
• #36292 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36292.svg">} (index: Delete old coinstats index directory on startup by fjahr)
• #36257 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36257.svg">} (qa: assert_equals -> assert_true/assert_false by hodlinator)
• #36002 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36002.svg">} (txindex: allow running in pruned mode by andrewtoth)
• #33904 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/33904.svg">} (rest: add interface for gettxspendingprevout rpc by kevkevinpal)

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-->

It seems a bit tedious to find and fix all of those issues by hand. Also, the tests will become overly verbose with all the manual syncs.

The framework should do this by itself, so I pushed that refactor and removed all of the manual syncs in tests.

Concept ACK, I like how cleaner it is. I also measured performance but saw no significant change.

Commented a test that loses coverage with these changes.

ACK fa74dc72d942e1628cea3c143bb56eed1457bbce

Diff to reproduce:...

I’m not sure this is the right reproducer, as all the tests pass on my Ubuntu 26.04 machine.

UPDATE. A bit unrelated to this PR, but that reproducer with a longer sleep triggered the following tests on my machine:

feature_anchors.py                                     | ✖ Failed  | 12 s
feature_config_args.py                                 | ✖ Failed  | 15 s
tool_utxo_to_sqlite.py                                 | ✖ Failed  | 3 s

UPDATE 2. Only tool_utxo_to_sqlite.py seems to have an issue, as others pass with the proper --timeout-factor.

UPDATE 3. A clanker's fix:

--- a/test/functional/tool_utxo_to_sqlite.py
+++ b/test/functional/tool_utxo_to_sqlite.py
@@ -119,6 +119,7 @@ class UtxoToSqliteTest(BitcoinTestFramework):
 
         self.log.info('Dump UTXO set via `dumptxoutset` RPC')
         input_filename = os.path.join(self.options.tmpdir, "utxos.dat")
+        self.wait_until(lambda: node.getindexinfo()['coinstatsindex']['synced'])
         node.dumptxoutset(input_filename, "latest")
 
         for i, (txid_format, spk_format) in enumerate(product(["hex", "raw", "rawle"], ["hex", "raw"])):

I’m not sure this is the right reproducer, as all the tests pass on my Ubuntu 26.04 machine.

Sorry, I forgot to clarify this only reproduces when running the test stand-alone and on the second run, because it needs the cache.

UPDATE 3. A clanker's fix:

I don't understand. The pull request here is supposed to fix the all index sync test issue (and I've confirmed this locally for tool_utxo_to_sqlite.py as well). Can you try this again and confirm you tested this pull?

I’m not sure this is the right reproducer, as all the tests pass on my Ubuntu 26.04 machine.

Sorry, I forgot to clarify this only reproduces when running the test stand-alone and on the second run, because it needs the cache.

I was able to reproduce the issue using a longer delay on another machine of mine, running Fedora 44.

UPDATE 3. A clanker's fix:

I don't understand. The pull request here is supposed to fix the all index sync test issue (and I've confirmed this locally for tool_utxo_to_sqlite.py as well). Can you try this again and confirm you tested this pull?

Yes, I can confirm that I have been testing this PR yesterday and today :)

It indeed fixes tool_utxo_to_sqlite.py as well. But the fix comes in the second commit. Its commit message reads "This refactor does not change any test behavior", but there is evidence that it is a fix rather than a pure refactor.

To summarise, the extra delay in src/util/thread.cpp reliably triggers failures in both wallet_fast_rescan.py and tool_utxo_to_sqlite.py. The second commit fixes all of them. So is the first commit needed at all?

It indeed fixes tool_utxo_to_sqlite.py as well. But the fix comes in the second commit. Its commit message reads "This refactor does not change any test behavior", but there is evidence that it is a fix rather than a pure refactor.

I wasn't aware that other tests also need fixing and when I wrote this, it was meant as a hardening refactor. I don't think there is much value in spending too much time enumerating all tests that are fixed. This also fixes #35728 (review) and I am sure it fixes a bunch of others more.

Happy to squash or reword the commit msg, but the overall diff won't change, so I am not sure if this is worth the review churn.

ACK fa74dc72d942e1628cea3c143bb56eed1457bbce, the changes are correct.

Happy to squash or reword the commit msg, but the overall diff won't change, so I am not sure if this is worth the review churn.

You are right about the overall diff. However, I'd be happy to re-ACK if you change your mind about the imprecise commit message.