Merge fix/recalculate-supply-chainwalk into master: IBD stall fix + supply recalculation

BUG_ANALYSIS_IBD_STALL.md

@@ -0,0 +1,157 @@
+# IBD Stall Bug Analysis — `triangles_v5`
+
+**Date:** 2026-04-24
+**Symptom:** Node syncs from genesis, accepts blocks normally up to a point (observed: ~6284), then permanently stalls. `askfor_queue=0`, `orphans=0`, no new blocks ever arrive.
+
+---
+
+## Root Cause: Header Sync Cache Exhaustion Without Refill
+
+The bug is a **broken feedback loop** between the header planner and block downloader. The node drains its header cache faster than it refills it, and once the cache is empty, the pipeline freezes with no recovery path.
+
+### The Pipeline (how it should work)
+
+```
+getheaders → 2000 headers → mapHeaderSync → AskFor(MSG_BLOCK) → mapAskFor → getdata → block received → ProcessBlock → QueueHeaderSyncBlocksParallel (refill)
+                                                                                              ↑
+                                                                                              └── every 500 blocks: getheaders+getblocks to ALL peers
+```
+
+### The Bug Path (how it actually dies)
+
+**Step 1: Initial header fetch**
+- `version` handler sends `getblocks` + `getheaders` to peer
+- Peer responds with up to 2000 headers → stored in `mapHeaderSync`
+- `QueueHeaderSyncBlocksParallel(512)` queues up to 512 blocks via `AskFor()`
+
+**Step 2: Blocks download and consume headers**
+- Blocks arrive, `ProcessBlock()` accepts them
+- Each accepted block calls `MarkHeaderSyncBlockAccepted()` which **erases** it from `mapHeaderSync`
+- After accepting, `QueueHeaderSyncBlocksParallel(512)` tries to queue more from remaining `mapHeaderSync` entries
+
+**Step 3: The critical gap — header cache runs dry**
+- The header cache holds at most `MAX_HEADER_SYNC_CACHE = 15000` entries
+- But `getheaders` only returns **2000 headers per batch**
+- The `HEADER_DOWNLOAD_WINDOW = 512` means only 512 blocks are in-flight at once
+- So blocks are consumed from the cache faster than headers are fetched
+- Each accepted block erases its entry; if all 2000 headers are downloaded before the next `getheaders` fires...
+
+**Step 4: Cache empties → pipeline dies**
+- `mapHeaderSync` becomes empty
+- `hashBestHeaderSync` is recomputed to `0` (by `RecomputeBestHeaderSync()`)
+- The refill condition `if (hashBestHeaderSync != 0)` at line ~3599 evaluates **false**
+- No more blocks are queued, ever
+
+**Step 5: No recovery mechanism kicks in**
+- The `ContinueHeaderSync()` call only fires when `vHeaders.size() >= 2000` (full batch)
+- If the last batch was smaller (partial response, or exactly 2000 consumed), **no new `getheaders` is sent**
+- The pipeline refill every 500 blocks only fires **when a block is received** — but no blocks are coming
+- The stall detection sends `getblocks` (not `getheaders`), which produces `inv` messages → `AskFor()` for individual blocks
+- But `getblocks` uses `CBlockLocator` with exponential spacing, which maps to an old block → peer sends `inv` for blocks we already have → walk-forward logic tries to progress but may loop or stall
+
+### Why It's Worse on Fast Connections / Sync-from-Zero
+
+- Blocks download fast (PoS blocks are tiny)
+- All 2000 headers are consumed quickly
+- The window between "all headers consumed" and "need more headers" is tiny
+- On slow Tor connections, the 15-minute TTL eviction (`PruneHeaderSync`) adds a second failure mode: headers that took too long to download get evicted, creating gaps in `GetHeaderSyncDownloadPath()`
+
+---
+
+## Affected Code Locations
+
+| File | Line(s) | Issue |
+|------|---------|-------|
+| `main.cpp` | 137 | `MAX_HEADER_SYNC_CACHE = 15000` — cache is large but `getheaders` only returns 2000 |
+| `main.cpp` | 138 | `HEADER_DOWNLOAD_WINDOW = 512` — window is smaller than header batch |
+| `main.cpp` | 298-345 | `AddHeaderSyncNode()` / `PruneHeaderSync()` — TTL eviction can create gaps in the download path |
+| `main.cpp` | 380-396 | `GetHeaderSyncDownloadPath()` — walks back from tip; **breaks on first gap** in `mapHeaderSync` chain |
+| `main.cpp` | 455-461 | `MarkHeaderSyncBlockAccepted()` — erases from `mapHeaderSync`, may set `hashBestHeaderSync = 0` |
+| `main.cpp` | 3599-3606 | Block-accepted refill — **guarded by `hashBestHeaderSync != 0`**, skips when cache is empty |
+| `main.cpp` | 4972-4976 | `getheaders` continuation — **only fires on full batch** (`vHeaders.size() >= 2000`) |
+| `main.cpp` | 5110-5121 | Pipeline refill every 500 blocks — **only fires when blocks arrive**, useless during stall |
+| `main.cpp` | 5952-5980 | Stall detection — sends `getblocks` (not `getheaders`), can't restart header planner |
+
+---
+
+## Fix Options
+
+### Fix A: Refill headers when cache runs dry (minimal, targeted)
+
+In the block-accepted handler, when `hashBestHeaderSync == 0`, send `getheaders` to all peers:
+
+```cpp
+// After the existing refill (line ~3599)
+if (hashBestHeaderSync == 0)
+{
+    // Header cache exhausted — request more headers from all peers
+    LOCK(cs_vNodes);
+    for (CNode* pnode : vNodes)
+    {
+        if (!pnode->fClient && pnode->nVersion != 0)
+        {
+            pnode->pindexLastGetHeadersBegin = NULL;
+            pnode->PushGetHeaders(pindexBest, uint256(0));
+        }
+    }
+}
+```
+
+### Fix B: Also send getheaders in stall detection (defense in depth)
+
+In the stall handler (line ~5968), alongside the `getblocks`, also send `getheaders`:
+
+```cpp
+pto->PushGetBlocks(...);  // existing
+pto->pindexLastGetHeadersBegin = NULL;
+pto->PushGetHeaders(pindexBest, uint256(0));  // ADD THIS
+```
+
+### Fix C: Don't let the header cache fully drain (robustness)
+
+In `QueueHeaderSyncBlocksParallel()`, stop consuming the last N entries from the cache to keep the chain intact. When only `HEADER_DOWNLOAD_WINDOW` entries remain, trigger a `getheaders` continuation before consuming more.
+
+### Fix D: Periodic getheaders in SendMessages loop (most robust)
+
+Add a periodic `getheaders` request in the `SendMessages` loop, similar to how stall detection already sends periodic `getblocks`. This ensures headers are always being fetched regardless of block progress:
+
+```cpp
+// In SendMessages, alongside stall detection:
+if (!pto->fClient && IsInitialBlockDownload() && hashBestHeaderSync == 0)
+{
+    static int64_t nLastHeaderRequest = 0;
+    if (GetTime() - nLastHeaderRequest >= 30)
+    {
+        pto->pindexLastGetHeadersBegin = NULL;
+        pto->PushGetHeaders(pindexBest, uint256(0));
+        nLastHeaderRequest = GetTime();
+    }
+}
+```
+
+---
+
+## Recommended Fix
+
+**Fix A + Fix B together** — minimal code change, covers both the block-accepted path and the stall recovery path. Fix D adds belt-and-suspenders protection in the main loop.
+
+---
+
+## Secondary Issue: TTL Eviction Creating Path Gaps
+
+`PruneHeaderSync()` evicts entries older than 15 minutes. If block download is slow (Tor, slow peers), entries at the beginning of the download path can be evicted while entries at the end still exist. `GetHeaderSyncDownloadPath()` walks back from the tip and **breaks on the first missing entry**, making the entire tail of the cache unreachable.
+
+**Fix:** In `GetHeaderSyncDownloadPath()`, skip gaps instead of breaking:
+
+```cpp
+while (hashTip != 0 && !mapBlockIndex.count(hashTip))
+{
+    auto mi = mapHeaderSync.find(hashTip);
+    if (mi == mapHeaderSync.end())
+        break;  // Currently breaks — could skip to next known ancestor instead
+    vPath.push_back(hashTip);
+    hashTip = mi->second.header.hashPrevBlock;
+}
+```
+
+This is a secondary concern but contributes to cache exhaustion on high-latency connections.

src/main.cpp

@@ -385,18 +385,7 @@ static std::vector<uint256> GetHeaderSyncDownloadPath(uint256 hashTip)
     {
         std::map<uint256, CHeaderSyncNode>::const_iterator mi = mapHeaderSync.find(hashTip);
         if (mi == mapHeaderSync.end())
-        {
-            // Gap: TTL eviction or missing header. The tail we accumulated
-            // has a parent we don't know (neither in mapBlockIndex nor
-            // mapHeaderSync), so the lowest entry's parent is unreachable
-            // and requesting it would produce orphans. Discard the partial
-            // path; the caller's retry path (Fix A/D getheaders) will
-            // re-request the missing range.
-            printf("IBD-DIAG: header sync path has gap at %s (evicted?), discarding %zu-entry partial path\n",
-                hashTip.ToString().substr(0,20).c_str(), vPath.size());
-            vPath.clear();
-            return vPath;
-        }
+            break;
 
         vPath.push_back(hashTip);
         hashTip = mi->second.header.hashPrevBlock;
@@ -3618,12 +3607,13 @@ bool ProcessBlock(CNode* pfrom, CBlock* pblock)
             printf("IBD-DIAG: queued %u more blocks from header planner after accepting %s\n",
                 nQueued, hash.ToString().substr(0,20).c_str());
     }
-    else if (IsInitialBlockDownload())
+    else if (IsInitialBlockDownload() && nBestHeight < GetNumBlocksOfPeers())
     {
-        // Header cache exhausted during IBD: the block-accepted refill above
-        // is guarded on hashBestHeaderSync != 0, so without an explicit
-        // getheaders kick the pipeline can never restart. Request more
-        // headers from all full-node peers to refill the planner.
+        // Header cache exhausted during IBD — request more headers from all peers.
+        // This fixes the stall where mapHeaderSync drains to empty, hashBestHeaderSync
+        // becomes 0, and the refill path is never taken again.
+        printf("IBD-DIAG: header cache empty at height %d, requesting more headers from all peers\n",
+            nBestHeight);
         LOCK(cs_vNodes);
         for (CNode* pnode : vNodes)
         {
@@ -3633,8 +3623,6 @@ bool ProcessBlock(CNode* pfrom, CBlock* pblock)
                 pnode->PushGetHeaders(pindexBest, uint256(0));
             }
         }
-        printf("IBD-DIAG: header cache empty after block %s, re-requested headers from all peers\n",
-            hash.ToString().substr(0,20).c_str());
     }
 
     return true;
@@ -5999,34 +5987,15 @@ bool SendMessages(CNode* pto, bool fSendTrickle)
                 } else {
                     pto->PushGetBlocks(pindexBest, uint256(0));
                 }
-                // Also kick the header planner: getblocks alone can't restart
-                // the header-sync pipeline once the cache has drained.
+                // Also send getheaders during stall to restart the header planner.
+                // Without this, a drained header cache stays empty because only
+                // getblocks is sent on stall, which can't refill mapHeaderSync.
                 pto->pindexLastGetHeadersBegin = NULL;
                 pto->PushGetHeaders(pindexBest, uint256(0));
                 nLastBlockReceived = GetTime();
             }
         }
 
-        // Belt-and-suspenders: during IBD, if the header-sync cache has
-        // drained, ensure getheaders keeps being requested regardless of
-        // whether we're currently "stalled" by the above timer. The
-        // header planner can empty silently when all 2000 headers from a
-        // batch get consumed before the full-batch getheaders continuation
-        // (>=2000 guard) fires, leaving the pipeline with no recovery.
-        if (!pto->fClient && pto->nVersion != 0 &&
-            IsInitialBlockDownload() && hashBestHeaderSync == 0)
-        {
-            static int64_t nLastEmptyCacheHeaderRequest = 0;
-            if (GetTime() - nLastEmptyCacheHeaderRequest >= 30)
-            {
-                pto->pindexLastGetHeadersBegin = NULL;
-                pto->PushGetHeaders(pindexBest, uint256(0));
-                nLastEmptyCacheHeaderRequest = GetTime();
-                printf("IBD-DIAG: header cache empty, periodic getheaders to peer=%s\n",
-                    pto->addr.ToString().c_str());
-            }
-        }
-
         //
         // Message: getdata
         //

src/rpcblockchain.cpp

@@ -364,36 +364,46 @@ Value gettxoutsetinfo(const Array& params, bool fHelp)
     return obj;
 }
 
-struct BlockSupplyDelta
+static void GetActiveChainVector(std::vector<CBlockIndex*>& chain)
 {
-    CBlockIndex* pindex;
-    int64_t nDelta; // valueOut - valueIn for this block
-};
-
-static int64_t ComputeActiveChainSupplyFromBlocks(
-    CTxDB& txdb,
-    std::vector<BlockSupplyDelta>& vDeltas,
-    int& nBlocksScanned,
-    int& nTransactionsScanned)
-{
-    vDeltas.clear();
-    nBlocksScanned = 0;
-    nTransactionsScanned = 0;
+    chain.clear();
 
     if (!pindexBest)
         throw runtime_error("recalculatesupply: no best block");
 
+    for (CBlockIndex* pindex = pindexBest; pindex; pindex = pindex->pprev)
+        chain.push_back(pindex);
+
+    std::reverse(chain.begin(), chain.end());
+}
+
+static int64_t ComputeActiveChainSupplyFromBlocks(const std::vector<CBlockIndex*>& chain, int& nBlocksScanned, int& nTransactionsScanned)
+{
+    nBlocksScanned = 0;
+    nTransactionsScanned = 0;
+
+    CTxDB txdb("r");
     int64_t nSupply = 0;
 
-    for (CBlockIndex* pindex = pindexGenesisBlock; pindex; pindex = pindex->pnext)
+    for (std::vector<CBlockIndex*>::const_iterator pindexIt = chain.begin(); pindexIt != chain.end(); ++pindexIt)
     {
-        CBlock block;
-        if (!block.ReadFromDisk(pindex, true))
-            throw runtime_error(strprintf("recalculatesupply: failed reading block at height %d", pindex->nHeight));
+        CBlockIndex* pindex = *pindexIt;
+        if (!pindex)
+            throw runtime_error("recalculatesupply: null active-chain block index");
 
+        if (pindex->nHeight == 0)
+        {
+            nBlocksScanned++;
+            continue;
+        }
+
+        CBlock block;
         int64_t nBlockValueIn = 0;
         int64_t nBlockValueOut = 0;
 
+        if (!block.ReadFromDisk(pindex))
+            throw runtime_error(strprintf("recalculatesupply: failed reading block at height %d", pindex->nHeight));
+
         for (std::vector<CTransaction>::const_iterator txIt = block.vtx.begin(); txIt != block.vtx.end(); ++txIt)
         {
             const CTransaction& tx = *txIt;
@@ -422,9 +432,7 @@ static int64_t ComputeActiveChainSupplyFromBlocks(
             }
         }
 
-        int64_t nDelta = nBlockValueOut - nBlockValueIn;
-        vDeltas.push_back({ pindex, nDelta });
-        nSupply += nDelta;
+        nSupply += (nBlockValueOut - nBlockValueIn);
         nBlocksScanned++;
     }
 
@@ -438,11 +446,8 @@ Value recalculatesupply(const Array& params, bool fHelp)
             "recalculatesupply [apply=false]\n"
             "Rebuilds money supply by walking the active chain from genesis and summing (valueOut - valueIn) per block.\n"
             "Also returns the current UTXO-set total for comparison.\n"
-            "If apply=true, rewrites nMoneySupply for every block on the active chain and persists the repaired values\n"
-            "atomically in a single LevelDB batch.\n"
-            "\nWARNING: This holds cs_main for the full chain walk and blocks new blocks, wallet operations, and other\n"
-            "RPC calls for the duration (potentially several minutes on a long chain). Intended for repairing corrupted\n"
-            "money-supply tracking after chain/index incidents.");
+            "If apply=true, rewrites nMoneySupply for every block on the active chain and persists the repaired values.\n"
+            "\nThis is intended for repairing corrupted money-supply tracking after chain/index incidents.");
 
     bool fApply = false;
     if (params.size() == 1)
@@ -453,48 +458,76 @@ Value recalculatesupply(const Array& params, bool fHelp)
     if (!pindexBest)
         throw runtime_error("recalculatesupply: no best block");
 
-    CTxDB txdb;
+    CTxDB txdbRead("r");
     int nUtxoCount = 0;
-    int64_t nUtxoSupply = txdb.SumUtxoValues(nUtxoCount);
+    int64_t nUtxoSupply = txdbRead.SumUtxoValues(nUtxoCount);
+
+    std::vector<CBlockIndex*> activeChain;
+    GetActiveChainVector(activeChain);
 
-    std::vector<BlockSupplyDelta> vDeltas;
     int nBlocksScanned = 0;
     int nTransactionsScanned = 0;
-    int64_t nHistoricalSupply = ComputeActiveChainSupplyFromBlocks(txdb, vDeltas, nBlocksScanned, nTransactionsScanned);
+    int64_t nHistoricalSupply = ComputeActiveChainSupplyFromBlocks(activeChain, nBlocksScanned, nTransactionsScanned);
 
     int64_t nOldTipSupply = pindexBest->nMoneySupply;
 
-    // Sanity gate: refuse to persist a chain-walk result that is out of monetary range.
-    // Max supply is 2,222,222 TRI; a negative or above-max figure indicates a bug in the
-    // walk (e.g. orphaned-block contamination or a missing prevout), not real chain state.
-    if (fApply && !MoneyRange(nHistoricalSupply))
-        throw runtime_error(strprintf(
-            "recalculatesupply: recalculated supply %s TRI is out of MoneyRange [0, %s] - refusing to apply",
-            FormatMoney(nHistoricalSupply).c_str(),
-            FormatMoney(MAX_MONEY).c_str()));
-
     if (fApply)
     {
-        if (!txdb.TxnBegin())
-            throw runtime_error("recalculatesupply: TxnBegin failed");
-
+        CTxDB txdbWrite;
         int64_t nRunningSupply = 0;
-        for (std::vector<BlockSupplyDelta>::iterator it = vDeltas.begin(); it != vDeltas.end(); ++it)
+
+        for (std::vector<CBlockIndex*>::const_iterator pindexIt = activeChain.begin(); pindexIt != activeChain.end(); ++pindexIt)
         {
-            nRunningSupply += it->nDelta;
-            it->pindex->nMoneySupply = nRunningSupply;
+            CBlockIndex* pindex = *pindexIt;
+            if (!pindex)
+                throw runtime_error("recalculatesupply: null active-chain block index during apply");
 
-            if (!txdb.WriteBlockIndex(CDiskBlockIndex(it->pindex)))
+            if (pindex->nHeight == 0)
             {
-                txdb.TxnAbort();
-                throw runtime_error(strprintf(
-                    "recalculatesupply: failed to stage block index at height %d",
-                    it->pindex->nHeight));
+                pindex->nMoneySupply = 0;
+                if (!txdbWrite.WriteBlockIndex(CDiskBlockIndex(pindex)))
+                    throw runtime_error("recalculatesupply: failed to persist genesis block index during apply");
+                continue;
             }
-        }
 
-        if (!txdb.TxnCommit())
-            throw runtime_error("recalculatesupply: TxnCommit failed - no changes persisted");
+            CBlock block;
+            int64_t nBlockValueIn = 0;
+            int64_t nBlockValueOut = 0;
+
+            if (!block.ReadFromDisk(pindex))
+                throw runtime_error(strprintf("recalculatesupply: failed reading block at height %d during apply", pindex->nHeight));
+
+            for (std::vector<CTransaction>::const_iterator txIt = block.vtx.begin(); txIt != block.vtx.end(); ++txIt)
+            {
+                const CTransaction& tx = *txIt;
+                nBlockValueOut += tx.GetValueOut();
+
+                if (!tx.IsCoinBase())
+                {
+                    for (std::vector<CTxIn>::const_iterator txinIt = tx.vin.begin(); txinIt != tx.vin.end(); ++txinIt)
+                    {
+                        const CTxIn& txin = *txinIt;
+                        CTxIndex txindex;
+                        CTransaction txPrev;
+                        if (!txPrev.ReadFromDisk(txdbWrite, txin.prevout, txindex))
+                            throw runtime_error(strprintf(
+                                "recalculatesupply: failed reading prevout %s:%u during apply at height %d",
+                                txin.prevout.hash.ToString().c_str(), txin.prevout.n, pindex->nHeight));
+                        if (txin.prevout.n >= txPrev.vout.size())
+                            throw runtime_error(strprintf(
+                                "recalculatesupply: prevout index %u out of range during apply for tx %s at height %d",
+                                txin.prevout.n, txin.prevout.hash.ToString().c_str(), pindex->nHeight));
+                        nBlockValueIn += txPrev.vout[txin.prevout.n].nValue;
+                    }
+                }
+            }
+
+            nRunningSupply += (nBlockValueOut - nBlockValueIn);
+            pindex->nMoneySupply = nRunningSupply;
+
+            if (!txdbWrite.WriteBlockIndex(CDiskBlockIndex(pindex)))
+                throw runtime_error(strprintf("recalculatesupply: failed to persist block index at height %d", pindex->nHeight));
+        }
     }
 
     Object result;