Merge pull request #208 from frstrtr/bch/m5-ibd-robust-locator

bch(M5): HeaderChain back-off locator for IBD getheaders continuation

Author: frstrtr

src/impl/bch/coin/embedded_daemon.hpp

@@ -123,6 +123,7 @@ class EmbeddedDaemon {
         wire_chain_ingest();          // new_headers --> HeaderChain (height advance)
         pin_cold_start_anchor();      // operator-approved floor-equivalent anchor
         m_node.start_p2p(peer);       // read-only P2P connect to the BCHN peer
+        bind_locator_provider();      // ContinueSync uses HeaderChain back-off locator
     }
 
     /// NEAR-TIP variant of the read-only IBD harness (UID 1375 follow-up). The
@@ -159,6 +160,7 @@ class EmbeddedDaemon {
         wire_chain_ingest();          // new_headers --> HeaderChain (height advance)
         pin_cold_start_anchor();      // ABLA anchor @ the SAME height as the seed
         m_node.start_p2p(peer);       // read-only P2P connect to the BCHN peer
+        bind_locator_provider();      // ContinueSync uses HeaderChain back-off locator
     }
 
     /// True once the BCHN peer handshake (version/verack) has completed, so the
@@ -198,6 +200,19 @@ class EmbeddedDaemon {
         m_coin_node = std::make_unique<CoinNode>(&m_embedded, m_node.rpc());
     }
 
+    /// Wire NodeP2P's IBD getheaders continuation to the authoritative
+    /// HeaderChain locator (exponential back-off from the tip). Without this the
+    /// ContinueSync follow-up falls back to a single-hash locator anchored at
+    /// the last learned header, which a peer cannot anchor if that header is on
+    /// a minority fork -- IBD then stalls silently. m_chain has already ingested
+    /// each batch (wire_chain_ingest) before ContinueSync fires, so get_locator()
+    /// reflects the just-learned tip. Call AFTER start_p2p (the p2p object must
+    /// exist). p2pool-merged-v36 surface: NONE (local SPV wire-sync only).
+    void bind_locator_provider() {
+        if (auto* p2p = m_node.p2p())
+            p2p->set_locator_provider([this]{ return m_chain.get_locator(); });
+    }
+
     /// Drive the authoritative HeaderChain from the live P2P header stream.
     /// The P2P front-end (NodeP2P) parses `headers` messages and fires
     /// new_headers; until this subscription existed NOTHING advanced m_chain

src/impl/bch/coin/header_sync.hpp

@@ -36,6 +36,7 @@
 // ---------------------------------------------------------------------------
 
 #include <cstddef>
+#include <vector>
 
 namespace bch::coin::header_sync {
 
@@ -68,4 +69,23 @@ inline Followup classify_headers_batch(
     return Followup::Idle;
 }
 
+/// Choose the getheaders locator for a ContinueSync IBD follow-up. PURE.
+///
+/// Prefer the robust HeaderChain-backed locator (`chain_locator`): an
+/// exponential back-off set of hashes lets the peer find a common ancestor even
+/// if our latest header sits on a minority fork. Fall back to a single-hash
+/// locator anchored at the last learned header (`last_hash`) ONLY when the
+/// chain-backed locator is unavailable -- i.e. no provider wired yet -- in which
+/// case the peer may stall if it cannot anchor that lone hash (degraded, but
+/// still forward-progressing on the common-chain case). Templated on the hash
+/// type so it stays dependency-light and unit-testable without uint256/coin lib.
+template <class Hash>
+inline std::vector<Hash> choose_continue_locator(
+    std::vector<Hash> chain_locator, const Hash& last_hash)
+{
+    if (!chain_locator.empty())
+        return chain_locator;
+    return {last_hash};
+}
+
 } // namespace bch::coin::header_sync

src/impl/bch/coin/p2p_node.hpp

@@ -148,6 +148,15 @@ class NodeP2P : public core::ICommunicator, public core::INetwork, public core::
     AddrCallback m_addr_callback;
     using PeerHeightCallback = std::function<void(uint32_t)>;
     PeerHeightCallback m_on_peer_height;
+    // Robust getheaders locator provider (exponential back-off over the
+    // authoritative HeaderChain). When set, IBD continuation re-issues
+    // getheaders with this multi-hash locator so the peer can always find a
+    // common ancestor even if our latest header sits on a minority fork. When
+    // unset we fall back to a single-hash locator anchored at the last header
+    // (degraded -- may stall if the peer cannot anchor that lone hash). Pure
+    // SPV wire-sync; no PoW/share/coinbase/PPLNS surface.
+    using LocatorProvider = std::function<std::vector<uint256>()>;
+    LocatorProvider m_locator_provider;
 
 public:
     NodeP2P(io::io_context* context, bch::interfaces::Node* coin, config_t* config,
@@ -321,6 +330,10 @@ class NodeP2P : public core::ICommunicator, public core::INetwork, public core::
     void set_addr_callback(AddrCallback cb) { m_addr_callback = std::move(cb); }
     /// Set callback for peer's reported chain height (from version message).
     void set_on_peer_height(PeerHeightCallback cb) { m_on_peer_height = std::move(cb); }
+    /// Set provider for the robust IBD getheaders locator (HeaderChain
+    /// exponential back-off). Without it, ContinueSync falls back to a
+    /// single-hash locator anchored at the last learned header.
+    void set_locator_provider(LocatorProvider cb) { m_locator_provider = std::move(cb); }
 
     /// Send getaddr to request peer addresses.
     void send_getaddr()
@@ -829,15 +842,25 @@ class NodeP2P : public core::ICommunicator, public core::INetwork, public core::
             switch (header_sync::classify_headers_batch(vheaders.size())) {
             case Followup::ContinueSync:
                 if (m_peer) {
-                    // Anchor the next locator at the last header we just learned
-                    // so the peer streams the next batch forward to its tip.
+                    // The header chain already ingested this batch (new_headers
+                    // fired above), so a HeaderChain-backed locator is anchored
+                    // at the just-learned tip AND carries exponential back-off
+                    // references -- the peer can find a common ancestor even if
+                    // our tip is on a minority fork. Fall back to a single-hash
+                    // anchor only when no provider is wired (degraded).
                     auto last_packed = pack(vheaders.back());
                     auto last_hash = Hash(last_packed.get_span());
+                    std::vector<uint256> chain_locator;
+                    if (m_locator_provider)
+                        chain_locator = m_locator_provider();
+                    auto locator = header_sync::choose_continue_locator(
+                        std::move(chain_locator), last_hash);
                     send_getheaders(m_peer_version ? m_peer_version : 1,
-                                    {last_hash}, uint256::ZERO);
+                                    locator, uint256::ZERO);
                     LOG_INFO << "[" << m_chain_label << "] IBD: got "
                              << vheaders.size() << " headers, continuing from "
-                             << last_hash.GetHex().substr(0, 16) << "...";
+                             << last_hash.GetHex().substr(0, 16) << "... ("
+                             << locator.size() << "-hash locator)";
                 }
                 break;
             case Followup::RequestBlocks:

src/impl/bch/test/header_sync_progress_test.cpp

@@ -33,9 +33,11 @@
 // PPLNS math). per-coin isolation: src/impl/bch/coin/ only.
 // ---------------------------------------------------------------------------
 
+#include <algorithm>
 #include <cstddef>
 #include <cstdint>
 #include <iostream>
+#include <vector>
 
 #include "../coin/header_sync.hpp"
 
@@ -83,6 +85,105 @@ SyncRun drive_ibd(std::uint64_t gap, std::size_t cap = MAX_HEADERS_RESULTS,
     return r;
 }
 
+// --- Fork-convergence soak: drive the IBD loop through choose_continue_locator
+// against a peer whose tip sits past a FORK from our latest header. The
+// COMPOSITIONAL counterpart to header_sync_test's pure choose_continue_locator
+// cases: proves the back-off locator makes the *loop* CONVERGE where the
+// degraded single-hash locator STALLS -- the silent-stall failure PR #208 fixed.
+// Header-layer "false_evict" analog = headers the peer re-serves that we already
+// hold; anchoring at the exact common ancestor keeps that at ZERO.
+using bch::coin::header_sync::choose_continue_locator;
+
+// Hash stand-in: main-chain blocks hash to their height; our fork-only blocks
+// hash to height + FORK_TAG so the peer (which holds only the main chain) can
+// never anchor a fork-only locator entry.
+constexpr std::uint64_t FORK_TAG = 1'000'000'000ull;
+
+inline std::uint64_t our_hash(std::uint64_t height, std::uint64_t fork_point) {
+    return (height <= fork_point) ? height : height + FORK_TAG;
+}
+inline bool peer_can_anchor(std::uint64_t hash, std::uint64_t peer_tip) {
+    return hash < FORK_TAG && hash <= peer_tip;   // a main-chain hash the peer holds
+}
+
+// Mirror of HeaderChain::get_locator_internal (BIP31 back-off): dense head of 11
+// consecutive heights from the tip, then the step doubles each entry down to 0.
+std::vector<std::uint64_t> make_chain_locator(std::uint64_t tip_height,
+                                              std::uint64_t fork_point) {
+    std::vector<std::uint64_t> loc;
+    std::int64_t step = 1;
+    std::int64_t h = static_cast<std::int64_t>(tip_height);
+    while (h >= 0) {
+        loc.push_back(our_hash(static_cast<std::uint64_t>(h), fork_point));
+        if (h == 0) break;
+        h -= step;
+        if (h < 0) h = 0;
+        if (loc.size() > 10) step *= 2;
+    }
+    return loc;
+}
+
+struct ForkRun {
+    std::uint64_t synced    = 0;  // highest MAIN-chain height we hold at halt
+    std::size_t   reissues  = 0;  // ContinueSync getheaders re-issues
+    std::size_t   redundant = 0;  // already-held headers re-served (false_evict analog)
+    bool          converged = false;
+    bool          stalled   = false;
+};
+
+// Drive the headers-first IBD continuation against a forked peer.
+//  fork_tip    : height of OUR latest header (on a minority fork above fork_point)
+//  fork_point  : last height common to us and the peer (the true common ancestor)
+//  peer_tip    : the peer's main-chain tip we must converge to
+//  use_provider: true  -> robust HeaderChain back-off locator wired (THE FIX)
+//                false -> degraded single-hash fallback (no provider) -> empty chain_locator
+ForkRun drive_forked_ibd(std::uint64_t fork_tip, std::uint64_t fork_point,
+                         std::uint64_t peer_tip, bool use_provider,
+                         std::size_t cap = MAX_HEADERS_RESULTS) {
+    ForkRun r;
+    std::uint64_t our_tip = fork_tip;          // height of our latest header
+    std::uint64_t main_progress = fork_point;  // highest main height we already hold
+    const std::size_t MAX_ROUNDS = 100000;     // runaway guard: a real stall trips it
+    std::size_t rounds = 0;
+    while (rounds < MAX_ROUNDS) {
+        ++rounds;
+        // Build the continuation locator exactly as NodeP2P now does.
+        std::vector<std::uint64_t> chain_loc =
+            use_provider ? make_chain_locator(our_tip, fork_point)
+                         : std::vector<std::uint64_t>{};      // no provider -> degraded
+        std::vector<std::uint64_t> loc =
+            choose_continue_locator(chain_loc, our_hash(our_tip, fork_point));
+
+        // Peer anchors at the FIRST (highest) locator entry it holds on its main
+        // chain; if none, it returns nothing (the silent stall #208 addressed).
+        std::int64_t anchor = -1;
+        for (std::uint64_t hh : loc) {
+            if (peer_can_anchor(hh, peer_tip)) { anchor = static_cast<std::int64_t>(hh); break; }
+        }
+        if (anchor < 0) { r.stalled = true; break; }   // cannot anchor -> stall
+
+        std::uint64_t from = static_cast<std::uint64_t>(anchor);
+        if (from >= peer_tip) break;                    // nothing new to serve -> caught up
+        std::uint64_t want = peer_tip - from;
+        std::size_t batch = (want >= cap) ? cap : static_cast<std::size_t>(want);
+        std::uint64_t served_to = from + batch;
+
+        // Headers in (from, main_progress] are ones we already hold -> re-served.
+        if (from < main_progress)
+            r.redundant += static_cast<std::size_t>(std::min(served_to, main_progress) - from);
+
+        our_tip = served_to;                            // now on the peer's main chain
+        if (served_to > main_progress) main_progress = served_to;
+        r.synced = main_progress;
+
+        Followup f = classify_headers_batch(batch, DEFAULT_ANNOUNCE_THRESHOLD, cap);
+        if (f == Followup::ContinueSync) { ++r.reissues; continue; }
+        break;                                          // converged / caught up
+    }
+    r.converged = (r.synced == peer_tip) && !r.stalled;
+    return r;
+}
+
 } // namespace
 
 int main()
@@ -141,6 +242,46 @@ int main()
         CHECK(empty.final_action == Followup::Idle);  // nothing arrived -> Idle
     }
 
+    // ---- inv. 6: shallow fork -> back-off locator anchors at the EXACT common
+    // ancestor, converges to peer tip, re-walks ZERO already-held headers
+    // (false_evict analog == 0). Loop-level proof of PR #208 over a live-shaped
+    // cursor (anchor 955700 -> peer tip 955822, the VM300 bchn-bch range).
+    {
+        const std::uint64_t peer_tip = 955822, fork_point = 955700;
+        const std::uint64_t fork_tip = fork_point + 3;   // 3-block minority fork
+        ForkRun r = drive_forked_ibd(fork_tip, fork_point, peer_tip, /*use_provider=*/true);
+        CHECK(r.converged);                              // reached peer tip (no stall)
+        CHECK(r.synced == peer_tip);
+        CHECK(r.redundant == 0);                         // anchored at exact ancestor: 0 re-walk
+        CHECK(r.reissues == (peer_tip - fork_point - 1) / CAP); // bounded re-issues
+        CHECK(!r.stalled);
+    }
+
+    // ---- inv. 7: SAME fork, degraded single-hash locator (no provider) STALLS.
+    // The peer cannot anchor our lone minority-fork tip -> serves nothing -> IBD
+    // never converges. Proves the back-off provider is LOAD-BEARING, not cosmetic:
+    // remove it and cold-start IBD across a fork silently hangs (the pre-#208 bug).
+    {
+        const std::uint64_t peer_tip = 955822, fork_point = 955700;
+        const std::uint64_t fork_tip = fork_point + 3;
+        ForkRun r = drive_forked_ibd(fork_tip, fork_point, peer_tip, /*use_provider=*/false);
+        CHECK(r.stalled);                                // degraded path hangs on the fork
+        CHECK(!r.converged);
+        CHECK(r.synced < peer_tip);
+    }
+
+    // ---- inv. 8: DEEP fork (50 blocks) still converges via back-off, never
+    // stalls; the re-walk stays bounded by one batch, not unbounded.
+    {
+        const std::uint64_t peer_tip = 955822, fork_point = 955700;
+        const std::uint64_t fork_tip = fork_point + 50;  // deep minority fork
+        ForkRun r = drive_forked_ibd(fork_tip, fork_point, peer_tip, /*use_provider=*/true);
+        CHECK(r.converged);
+        CHECK(r.synced == peer_tip);
+        CHECK(!r.stalled);
+        CHECK(r.redundant < CAP);                        // re-walk bounded, finite
+    }
+
     if (failures == 0)
         std::cout << "bch header_sync sync-to-peer-tip soak: ALL PASS\n";
     else

src/impl/bch/test/header_sync_test.cpp

@@ -15,13 +15,16 @@
 
 #include <cassert>
 #include <iostream>
+#include <string>
+#include <vector>
 
 #include "../coin/header_sync.hpp"
 
 using bch::coin::header_sync::Followup;
 using bch::coin::header_sync::classify_headers_batch;
 using bch::coin::header_sync::MAX_HEADERS_RESULTS;
 using bch::coin::header_sync::DEFAULT_ANNOUNCE_THRESHOLD;
+using bch::coin::header_sync::choose_continue_locator;
 
 int main()
 {
@@ -53,6 +56,40 @@ int main()
     // Custom small cap: at-cap batch is ContinueSync.
     assert(classify_headers_batch(10, /*announce=*/3, /*cap=*/10) == Followup::ContinueSync);
 
-    std::cout << "bch header_sync classify: ALL PASS\n";
+    // -- choose_continue_locator: ContinueSync locator selection (fork safety) --
+    // Use std::string as the hash stand-in so this stays pure (no uint256/coin lib).
+    const std::string last = "last_header";
+
+    // No provider / empty chain locator -> degraded single-hash fallback anchored
+    // at the last learned header (forward-progress on the common-chain case).
+    {
+        std::vector<std::string> empty_chain;
+        auto loc = choose_continue_locator(empty_chain, last);
+        assert(loc.size() == 1);
+        assert(loc[0] == last);
+    }
+
+    // Provider returns a robust HeaderChain back-off locator -> it is used
+    // VERBATIM (multi-hash, fork-resilient), NOT the single-hash fallback. THE FIX.
+    {
+        std::vector<std::string> chain = {"tip", "tip-1", "tip-3", "tip-7", "genesis"};
+        auto loc = choose_continue_locator(chain, last);
+        assert(loc.size() == 5);
+        assert(loc.front() == "tip");          // anchored at the just-learned tip
+        assert(loc.back() == "genesis");       // back-off reaches a deep ancestor
+        // The lone last_hash is NOT substituted when a real locator exists.
+        assert(loc != std::vector<std::string>{last});
+    }
+
+    // A single-element chain locator is still preferred over the raw fallback
+    // (the provider, not the handler, decided that one hash suffices).
+    {
+        std::vector<std::string> chain = {"only_tip"};
+        auto loc = choose_continue_locator(chain, last);
+        assert(loc.size() == 1);
+        assert(loc[0] == "only_tip");
+    }
+
+    std::cout << "bch header_sync classify + continue-locator: ALL PASS\n";
     return 0;
 }