fix(consensus): fail-closed reorg guard when startup checkpoint pointer is null

Adversarial Codex review of 935d1d5 flagged that Reorganize()'s below-checkpoint
guard short-circuited on 'pindexLastHardenedCheckpoint == nullptr'. That state
arises during early IBD, reindex, and bootstrap before the checkpoint block has
been downloaded into mapBlockIndex — exactly when an attacker peer would feed a
deep fork. Without the pointer, the guard silently fell through.

Fix: add Checkpoints::GetLastCheckpointHeight(), which reads the compiled
mapCheckpoints directly (independent of mapBlockIndex). Reorganize() now
resolves nHardenedCheckpointHeight from the pointer when available, falling
back to the compiled height otherwise. Same floor, just two paths.

Tests: 278/278 pass; added reorg_guard_fails_closed_when_checkpoint_pointer_null
with four invariant checks (helper existence, fallback assignment, guard
pattern, absence of the old short-circuit pattern).

src/checkpoints.cpp

@@ -138,6 +138,19 @@ namespace Checkpoints
         return nullptr;
     }
 
+    // Independent of mapBlockIndex: returns the highest compiled checkpoint
+    // height for the current network. Returns -1 if the compiled map is
+    // empty (an unusual, but not impossible, configuration). Used as the
+    // fail-closed reorg floor before pindexLastHardenedCheckpoint has been
+    // resolved against the local block index (early IBD / reindex /
+    // bootstrap before the checkpoint block has been downloaded).
+    int GetLastCheckpointHeight()
+    {
+        MapCheckpoints& checkpoints = (fTestNet ? mapCheckpointsTestnet : mapCheckpoints);
+        if (checkpoints.empty()) return -1;
+        return checkpoints.rbegin()->first;
+    }
+
     // triangles: synchronized checkpoint (centrally broadcasted)
     uint256 hashSyncCheckpoint = uint256("0x7e7a6e4dd5fe895106fca912dfbacaeaf2a89e76c6a588df8ff96e0e18b96021");
     uint256 hashPendingCheckpoint = uint256("0x7e7a6e4dd5fe895106fca912dfbacaeaf2a89e76c6a588df8ff96e0e18b96021");

src/checkpoints.h

@@ -53,6 +53,14 @@ namespace Checkpoints
     // Returns last CBlockIndex* in mapBlockIndex that is a checkpoint
     CBlockIndex* GetLastCheckpoint(const std::map<uint256, CBlockIndex*>& mapBlockIndex);
 
+    // Returns the highest *compiled* checkpoint height, independent of
+    // whether mapBlockIndex has loaded the corresponding block yet. Every
+    // node built from the same binary sees the same value. Used as the
+    // fail-closed floor for Reorganize() when pindexLastHardenedCheckpoint
+    // has not yet been resolved (early IBD / reindex / bootstrap before
+    // the checkpoint block has been downloaded).
+    int GetLastCheckpointHeight();
+
     extern uint256 hashSyncCheckpoint;
     extern CSyncCheckpoint checkpointMessage;
     extern uint256 hashInvalidCheckpoint;

src/main.cpp

@@ -2658,14 +2658,30 @@ bool static Reorganize(CTxDBBase& txdb, CBlockIndex* pindexNew)
     //
     // Below the hardened checkpoint: reject unconditionally. The
     // checkpoint is sourced from the same compiled map (Checkpoints::
-    // GetLastCheckpoint via init.cpp startup init) on every node, so
+    // mapCheckpoints via GetLastCheckpointHeight) on every node, so
     // it is a globally shared anchor, not locally invented finality.
-    if (pindexLastHardenedCheckpoint && pfork->nHeight <= pindexLastHardenedCheckpoint->nHeight)
+    //
+    // pindexLastHardenedCheckpoint is set at startup from the same map,
+    // keyed by mapBlockIndex lookup of the compiled checkpoint hash. If
+    // that lookup fails (early IBD, reindex, or bootstrap before the
+    // checkpoint block has been downloaded into the local block index)
+    // the pointer is NULL. In that state we still know the *height* of
+    // the checkpoint from the compiled map directly — every node built
+    // from the same binary sees the same value — and we use it as the
+    // fail-closed floor. Without this second path, an IBD-time reorg
+    // attempt below the compiled checkpoint height would silently slip
+    // through the guard.
+    int nHardenedCheckpointHeight = -1;
+    if (pindexLastHardenedCheckpoint)
+        nHardenedCheckpointHeight = pindexLastHardenedCheckpoint->nHeight;
+    else
+        nHardenedCheckpointHeight = Checkpoints::GetLastCheckpointHeight();
+    if (nHardenedCheckpointHeight >= 0 && pfork->nHeight <= nHardenedCheckpointHeight)
     {
-        printf("REORGANIZE: REJECTED — fork point %d is below shared hardened checkpoint %d\n",
-            pfork->nHeight, pindexLastHardenedCheckpoint->nHeight);
+        printf("REORGANIZE: REJECTED — fork point %d is at or below shared hardened checkpoint %d\n",
+            pfork->nHeight, nHardenedCheckpointHeight);
         return error("Reorganize() : fork point %d at or below shared hardened checkpoint %d",
-            pfork->nHeight, pindexLastHardenedCheckpoint->nHeight);
+            pfork->nHeight, nHardenedCheckpointHeight);
     }
 
     // List of what to disconnect

src/test/consensus_safety_tests.cpp

@@ -446,10 +446,18 @@ BOOST_AUTO_TEST_CASE(convergence_rejects_below_hardened_checkpoint)
     BOOST_CHECK(src.find("pindexLastHardenedCheckpoint") != std::string::npos);
     BOOST_CHECK(src.find("pindexFinalized") == std::string::npos);
 
-    // (2) The reorg-rejection block compares fork height to the
-    //     checkpoint height — not to MAX_REORG_DEPTH or any tip-based
-    //     value. We look for the rejection guard pattern.
-    BOOST_CHECK(src.find("pfork->nHeight <= pindexLastHardenedCheckpoint->nHeight")
+    // (2) The reorg-rejection block compares fork height against a
+    //     resolved checkpoint height (`nHardenedCheckpointHeight`), not
+    //     against MAX_REORG_DEPTH or any tip-based value. The literal
+    //     source pattern we look for is the new guard variable being
+    //     assigned from `Checkpoints::GetLastCheckpointHeight()` —
+    //     which is the bootstrap-time path that covers the
+    //     `pindexLastHardenedCheckpoint == nullptr` case (otherwise an
+    //     IBD-time reorg below the compiled checkpoint could slip
+    //     through the guard).
+    BOOST_CHECK(src.find("Checkpoints::GetLastCheckpointHeight()")
+                != std::string::npos);
+    BOOST_CHECK(src.find("nHardenedCheckpointHeight = pindexLastHardenedCheckpoint->nHeight")
                 != std::string::npos);
 
     // (3) No absolute depth cap in Reorganize(). The old code had
@@ -553,4 +561,54 @@ BOOST_AUTO_TEST_CASE(getheaders_recovers_via_checkpoint_when_locator_has_it)
                 != std::string::npos);
 }
 
+// ─── Bootstrap-state reorg guard: hardened_checkpoint_height is fail-closed ─
+// Adversarial review (Codex, SHA 935d1d5) flagged that the original guard
+// short-circuited on `pindexLastHardenedCheckpoint == nullptr`. That happens
+// during early IBD, reindex, and bootstrap before the checkpoint block has
+// been downloaded — exactly when an attacker peer would most want to feed a
+// deep fork. The fix consults `Checkpoints::GetLastCheckpointHeight()`
+// (compiled map, independent of mapBlockIndex) as the second-layer floor.
+BOOST_AUTO_TEST_CASE(reorg_guard_fails_closed_when_checkpoint_pointer_null)
+{
+    std::string src = readEntireFile("src/main.cpp");
+    BOOST_REQUIRE(!src.empty());
+
+    // (a) The compiled-map helper is declared in checkpoints.h and
+    //     defined in checkpoints.cpp. The signature is
+    //     `int GetLastCheckpointHeight()` (declared inside the
+    //     Checkpoints namespace; namespace-qualified at call sites).
+    std::string cp_h = readEntireFile("src/checkpoints.h");
+    std::string cp_cpp = readEntireFile("src/checkpoints.cpp");
+    BOOST_REQUIRE(!cp_h.empty());
+    BOOST_REQUIRE(!cp_cpp.empty());
+    BOOST_CHECK(cp_h.find("int GetLastCheckpointHeight();") != std::string::npos);
+    BOOST_CHECK(cp_cpp.find("int GetLastCheckpointHeight()") != std::string::npos);
+    // The implementation is independent of mapBlockIndex — it returns
+    // checkpoints.rbegin()->first directly. This is what makes it usable
+    // before the checkpoint hash has been resolved in our local index.
+    BOOST_CHECK(cp_cpp.find("checkpoints.rbegin()->first") != std::string::npos);
+
+    // (b) The Reorganize() guard uses the compiled-height fallback when
+    //     the local pointer is NULL. The pattern is the local variable
+    //     `nHardenedCheckpointHeight` being assigned from
+    //     `Checkpoints::GetLastCheckpointHeight()` in the else branch.
+    BOOST_CHECK(src.find("nHardenedCheckpointHeight = Checkpoints::GetLastCheckpointHeight()")
+                != std::string::npos);
+
+    // (c) The guard fires for any fork point at or below the resolved
+    //     checkpoint height — independent of whether the resolution came
+    //     from the pointer or the compiled map. The literal pattern that
+    //     matters is `pfork->nHeight <= nHardenedCheckpointHeight`.
+    BOOST_CHECK(src.find("pfork->nHeight <= nHardenedCheckpointHeight")
+                != std::string::npos);
+
+    // (d) The old guard pattern that short-circuited on the null pointer
+    //     is gone. The exact prior pattern was:
+    //         if (pindexLastHardenedCheckpoint && pfork->nHeight <= pindexLastHardenedCheckpoint->nHeight)
+    //     That single `if` is no longer a guard by itself — it has been
+    //     replaced by the `nHardenedCheckpointHeight` two-layer check.
+    BOOST_CHECK(src.find("if (pindexLastHardenedCheckpoint && pfork->nHeight <= pindexLastHardenedCheckpoint->nHeight)")
+                == std::string::npos);
+}
+
 BOOST_AUTO_TEST_SUITE_END()