#36618 test: Wait for index sync in all tests

ReviewabilityReviewsAgreementSize
#36618 maflcko test: Wait for index sync in all testsReady1Strong0 + 41 tests
  • Prevent intermittent test flakes caused by races between background indices and test operations
  • Automatically wait for enabled indices on node startup and sync instead of requiring manual waits
  • Ready to review
  • Substantive support with no open objections
  • Concept approval because it cleans up tests with no performance impact (ViniciusCestarii)
  • Coverage objection withdrawn after author clarified test behavior (ViniciusCestarii)
  • Confirmed the test failure reproducer and asked whether the first commit is needed (hebasto)
  • 0 lines added or modified outside tests
  • 41 lines added or modified in tests
  • 71 lines removed in total
  • 18 files, 2 commits
Test infrastructure#2 of 54
P2 · bug fix
  • P2 because it fixes intermittent test failures caused by races with unsynced background indices
  • Centralizing index synchronization improves functional test reliability and eliminates boilerplate
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

Updates the functional test framework to wait for all enabled background indices (such as txindex and blockfilterindex) to sync by default during node startup and when calling `sync_all()`. It cleans up redundant manual index-synchronization calls across multiple test files and removes the standalone `sync_txindex` helper.

Problem: Functional tests relying on background indices can race against wallet or RPC operations if an index has not caught up to the chain tip, causing intermittent test flakes such as missing block filters during fast wallet rescans (#36564). Previously, tests had to remember to call manual `wait_until` index checks or helpers.

Evidence of importance:

  • PR description links to failure report in issue #36564 and provides a reproduction snippet using a sleep in `TraceThread` that triggers `(WARNING: block filter not found!)`.
  • Commit fa74dc72d9 notes: 'The tests already wait for the mempool and blocks after start and after mining, so they should wait for indexes as well to avoid intermittent test failures.'

Scope: Touches 18 test files, primarily replacing manual `wait_until` index checks with the centralized default wait in `BitcoinTestFramework` and `TestNode`.

Risk of a mistake: low (tests). A mistake would only cause functional test timeouts or CI failures without impacting release binaries or consensus.

Changed since the previous assessment: None. The diff confirms the previous description.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • reviewer: hebasto to evaluate author's explanation regarding keeping the first commit 2026-11-01

Agreement

Strong

substantive support, no open objection (ViniciusCestarii)

Thread notes: Claim c:6545513372 moved to answered after author maflcko explained the purpose of the first commit in c:6561772062.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
ViniciusCestarii (contributor)objection, correctnessfeature_index_prune.py loses test coverage because it previously checked for absolute height and missing indexaccepted · author explained behavior does not change and reviewer acceptedchangedyes2026-10-29: 'This changes test behavior. It used to check for absolute height and missing index.'
Status from: 2026-10-30: ViniciusCestarii: 'thanks for explaining! Makes sense.'
hebasto (member)question, correctnesswhether the reproducer is correct as all tests pass on Ubuntu 26.04accepted · hebasto confirmed reproducing the issue with a longer delay on Fedora 44answeredyes2026-10-30: 'I’m not sure this is the right reproducer, as all the tests pass on my Ubuntu 26.04 machine.'
Status from: 2026-10-31: hebasto: 'I was able to reproduce the issue using a longer delay on another machine of mine, running Fedora 44.'
hebasto (member)suggestion, scopedrop the first commit if the second commit already fixes all failuresanswered · author explained the first commit was meant as a hardening refactor and also fixes other testsansweredyes2026-10-31: 'The second commit fixes all of them. So is the first commit needed at all?'
Status from: 2026-11-01: maflcko: 'it was meant as a hardening refactor'

Support:

  • ViniciusCestarii (contributor) (Concept ACK, 2026-10-29) [both]: I like how cleaner it is. I also measured performance but saw no significant change.
  • ViniciusCestarii (contributor) (ACK, 2026-10-30): (no reason given) [not substantive]

Participants:

  • ViniciusCestarii (contributor, support, 4 statements 2026-10-29 to 2026-10-30): Concept ACK and full ACK after author clarified feature_index_prune test behavior
  • hebasto (member, question, 2 statements 2026-10-30 to 2026-10-31): confirmed reproducer works and asked whether the first commit is needed

Files

112 lines under test/bench/ci.

  • test/functional/test_framework/test_framework.py +15/-7
  • test/functional/test_framework/test_node.py +18/-3
  • test/functional/feature_index_prune.py +1/-19
  • test/functional/feature_coinstatsindex.py +1/-6
  • test/functional/feature_txindex_compatibility.py +1/-5
  • test/functional/interface_rest.py +0/-6
  • test/functional/test_framework/util.py +0/-6
  • test/functional/feature_reindex.py +2/-2
  • test/functional/feature_coinstatsindex_compatibility.py +0/-3
  • test/functional/rpc_misc.py +1/-2
  • test/functional/feature_assumeutxo.py +1/-1
  • test/functional/mempool_accept.py +0/-2
  • test/functional/rpc_gettxspendingprevout.py +0/-2
  • test/functional/rpc_rawtransaction.py +0/-2
  • test/functional/rpc_txoutproof.py +0/-2
  • test/functional/rpc_whitelist.py +1/-1
  • test/functional/feature_init.py +0/-1
  • test/functional/rpc_scanblocks.py +0/-1

Card

The short text the ranking pass compares across PRs.

This pull request configures functional test node startup and sync helpers to automatically wait for background index synchronization, resolving intermittent test flakes caused by race conditions. It addresses issue #36564 where wallet rescans failed due to block filters lagging behind the chain tip. Redundant manual index sync logic across 18 test files is removed, with no changes to production node code.

Data

dossier JSON · extract JSON · model ledger, generated 2026-11-02T01:11, confidence high, input hash ea7b910235f40c94
thread read through 2026-11-01 (11 statements), head fa74dc72d9
labels: Tests

Categories

Test infrastructure

P2 · bug fix

Addresses intermittent test flakiness caused by races between background indices and test RPC/wallet operations (#36564). Centralizing index synchronization into `sync_all()` and node startup eliminates redundant boilerplate while directly targeting test harness reliability.

Membership: Labeled 'Tests' and updates `test_framework.py` and `test_node.py` to wait for enabled background indices by default.

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