“txospenderindex_tests/txospenderindex_initial_sync”: critical check tx_spender->has_value() has failed #34637

issue fanquake openend this issue on February 20, 2026
  1. fanquake commented at 2:38 pm on February 20, 2026: member

    https://github.com/bitcoin/bitcoin/actions/runs/22227876457/job/64299801435?pr=34615#step:8:652

    0
    1./test/txospenderindex_tests.cpp(15): Entering test case "txospenderindex_initial_sync"
    2./test/txospenderindex_tests.cpp(60): fatal error: in "txospenderindex_tests/txospenderindex_initial_sync": critical check tx_spender->has_value() has failed
    3Command '['./bin/test_bitcoin.exe', '-l', 'test_suite']' returned non-zero exit status 3221225477.
    
  2. fanquake added this to the milestone 31.0 on Feb 20, 2026
  3. hodlinator commented at 3:38 pm on February 20, 2026: contributor
    Notable that it only happens on Windows UCRT in the linked run, indicating probable insufficient synchronization between txospenderindex and the new block in chainstate. Maybe we should move SyncWithValidationInterfaceQueue() before the failing check?
  4. sedited commented at 4:10 pm on February 20, 2026: contributor

    Maybe we should move SyncWithValidationInterfaceQueue() before the failing check?

    BlockUntilSyncedToCurrentChain() should already be doing that. It’s a bit puzzling, I also couldn’t reproduce a failure after 10'000 test runs.

  5. w0xlt commented at 3:08 am on February 22, 2026: contributor

    BlockUntilSyncedToCurrentChain() wouldn’t catch this. After Sync(), both best_block_index and chain_tip are at block 151, so it takes the early return without draining the queue:

    0  if (best_block_index->GetAncestor(chain_tip->nHeight) == chain_tip) {
    1      return true;  // skips SyncWithValidationInterfaceQueue()
    2  }
    

    The issue looks to be that 51 BlockConnected callbacks (for blocks mined between Init() and Sync()) are still pending in the validation interface queue. When the scheduler delivers the stale callbacks and BlockConnected sees best_block_index=151 but receives a notification for pindex=101, it interprets this as: “I’m at 151, but I need to process block 101 — I must have missed something, let me rewind to block 100 and re-index from there.” That rewind erases the spending entries for blocks 101-151.

    0if (best_block_index != pindex->pprev && !Rewind(best_block_index, pindex->pprev)) {
    

    On Linux the scheduler seems to drain those callbacks before Sync() sets m_synced = true. But the error can be deterministically triggered by firing a stale BlockConnected directly after Sync():

     0diff --git a/src/test/txospenderindex_tests.cpp b/src/test/txospenderindex_tests.cpp
     1index 889ef7ff8a..fef331afe4 100644
     2--- a/src/test/txospenderindex_tests.cpp
     3+++ b/src/test/txospenderindex_tests.cpp
     4@@ -4,7 +4,10 @@
     5 
     6 #include <chainparams.h>
     7 #include <index/txospenderindex.h>
     8+#include <kernel/types.h>
     9+#include <node/blockstorage.h>
    10 #include <test/util/setup_common.h>
    11+#include <test/util/validation.h>
    12 #include <util/time.h>
    13 #include <validation.h>
    14 
    15@@ -54,6 +57,24 @@ BOOST_FIXTURE_TEST_CASE(txospenderindex_initial_sync, TestChain100Setup)
    16     BOOST_CHECK(!txospenderindex.BlockUntilSyncedToCurrentChain());
    17 
    18     txospenderindex.Sync();
    19+
    20+    // Simulate a stale BlockConnected callback: fire BlockConnected for block
    21+    // 101, which Sync() already indexed. Since m_synced is now true, this
    22+    // triggers Rewind(tip, 100), erasing spending entries for blocks 101-tip.
    23+    // This reproduces the race in issue [#34637](/bitcoin-bitcoin/34637/) deterministically.
    24+    {
    25+        const CBlockIndex* stale_pindex;
    26+        {
    27+            LOCK(cs_main);
    28+            stale_pindex = m_node.chainman->ActiveChain()[101];
    29+        }
    30+        CBlock stale_block;
    31+        m_node.chainman->ActiveChainstate().m_blockman.ReadBlock(stale_block, *stale_pindex);
    32+        auto shared_block = std::make_shared<CBlock>(stale_block);
    33+        ValidationInterfaceTest::BlockConnected(
    34+            kernel::ChainstateRole{}, txospenderindex, shared_block, stale_pindex);
    35+    }
    36+
    37     for (size_t i = 0; i < spent.size(); i++) {
    38         const auto tx_spender{txospenderindex.FindSpender(spent[i])};
    39         BOOST_REQUIRE(tx_spender.has_value());
    

    Adding SyncWithValidationInterfaceQueue() after Sync() to unconditionally drain the stale callbacks before querying the index should work, as observed by @hodlinator .

     0diff --git a/src/test/txospenderindex_tests.cpp b/src/test/txospenderindex_tests.cpp
     1index 889ef7ff8a..4aaa5513de 100644
     2--- a/src/test/txospenderindex_tests.cpp
     3+++ b/src/test/txospenderindex_tests.cpp
     4@@ -54,6 +54,12 @@ BOOST_FIXTURE_TEST_CASE(txospenderindex_initial_sync, TestChain100Setup)
     5     BOOST_CHECK(!txospenderindex.BlockUntilSyncedToCurrentChain());
     6 
     7     txospenderindex.Sync();
     8+
     9+    // Drain stale BlockConnected callbacks that were enqueued for blocks created
    10+    // between Init() and Sync(). After Sync() sets m_synced = true, these
    11+    // callbacks can trigger unnecessary rewinds until fully processed.
    12+    m_node.validation_signals->SyncWithValidationInterfaceQueue();
    13+
    14     for (size_t i = 0; i < spent.size(); i++) {
    15         const auto tx_spender{txospenderindex.FindSpender(spent[i])};
    16         BOOST_REQUIRE(tx_spender.has_value());
    
  6. furszy commented at 0:21 am on February 23, 2026: member

    Oh, just realized your comment w0xlt. I have been cooking some deep index changes to completely eradicate the underlying problem with BlockUntilSyncedToCurrentChain(), not only to fix the test. Still in draft and not ready branch.

    But.. I got sidetracked today by the new txospender tests code and ended up cleaning them up. They weren’t easy to read or work with. So I pushed #34653 to make our future selves’ and future devs’ lives easier (hopefully).

    This isn’t blocking the direct test fix you are suggesting. I will happily ACK that too. Just sharing an update that there is more here, and will be pushing a new PR “soon”.

  7. hebasto commented at 2:59 pm on February 23, 2026: member

    Notable that it only happens on Windows UCRT…

    Here is the log for MSVCRT: https://github.com/bitcoin/bitcoin/actions/runs/22237824844/job/64344924186?pr=33974.

  8. achow101 commented at 10:23 pm on February 23, 2026: member
    To clarify, does #34653 fix this issue?
  9. furszy commented at 10:48 pm on February 23, 2026: member

    To clarify, does #34653 fix this issue?

    Yes. #34653 drains the validation queue before constructing the index, so no lagging notification will arrive during the post-Sync() checks. It also introduces a post-Sync() assertion that ensures the index is fully synced and did not abort prematurely before the checks are run.

    Extra fun fact: I did this without thinking about the issue. I was just focused on cleaning up the test first. So.. would say that we should clean up code more often.

    Updated #34653 description saying this is fixed there.

  10. achow101 commented at 11:06 pm on February 23, 2026: member

    Was able to actually reproduce the issue with

     0diff --git a/src/index/base.cpp b/src/index/base.cpp
     1index fba2f4f6c10..571347d42f1 100644
     2--- a/src/index/base.cpp
     3+++ b/src/index/base.cpp
     4@@ -49,6 +49,12 @@ constexpr uint8_t DB_BEST_BLOCK{'B'};
     5 constexpr auto SYNC_LOG_INTERVAL{30s};
     6 constexpr auto SYNC_LOCATOR_WRITE_INTERVAL{30s};
     7 
     8+std::atomic<bool> g_debug_waiting{false};
     9+std::atomic<bool> g_debug_continue{false};
    10+std::mutex g_debug_mutex;
    11+std::condition_variable g_debug_cv;
    12+Mutex g_index_mutex;
    13+
    14 template <typename... Args>
    15 void BaseIndex::FatalErrorf(util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
    16 {
    17@@ -200,6 +206,16 @@ bool BaseIndex::ProcessBlock(const CBlockIndex* pindex, const CBlock* block_data
    18 
    19 void BaseIndex::Sync()
    20 {
    21+    LOCK(g_index_mutex);
    22+
    23+    {
    24+        g_debug_continue = false;
    25+        g_debug_waiting = true;
    26+        std::unique_lock<std::mutex> lock(g_debug_mutex);
    27+        g_debug_cv.wait_for(lock, std::chrono::seconds(2), [] { return g_debug_continue.load(); });
    28+        g_debug_waiting = false;
    29+    }
    30+
    31     const CBlockIndex* pindex = m_best_block_index.load();
    32     if (!m_synced) {
    33         auto last_log_time{NodeClock::now()};
    34@@ -327,6 +343,14 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti
    35 
    36 void BaseIndex::BlockConnected(const ChainstateRole& role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex)
    37 {
    38+    UninterruptibleSleep(100ms);
    39+    if (g_debug_waiting) {
    40+        g_debug_continue = true;
    41+        g_debug_cv.notify_all();
    42+    }
    43+
    44+    LOCK(g_index_mutex);
    45+
    46     // Ignore events from not fully validated chains to avoid out-of-order indexing.
    47     //
    48     // TODO at some point we could parameterize whether a particular index can be
    
  11. hebasto commented at 10:18 am on February 24, 2026: member
  12. achow101 closed this on Feb 24, 2026

  13. pull[bot] referenced this in commit c88c916e72 on Feb 24, 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-02-27 12:13 UTC

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