consensus: keep live PoS checks during stale-tip IBD

notes/audit-progress.md

@@ -565,3 +565,33 @@ Verification:
 - Normal build/test: `ctest --output-on-failure` => 4/4 passed in build.
 
 This work intentionally does not touch production datadir `/root/.triangles/`, wallet files, consensus constants, or live daemon state.
+
+
+## 2026-07-07 -- Krystie (stake modifier / PoS validation audit)
+
+### Action taken: checked stake modifier and fixed stale-tip PoS validation bypass
+
+Sami asked to check the stake modifier. Findings:
+
+1. **Stake modifier interval deviation (documented, not changed):**
+   - Upstream Peercoin v0.3/v0.4 uses the full `GetStakeModifierSelectionInterval()` in `GetKernelStakeModifier()`.
+   - Triangles has a 2014 consensus override: `nStakeModifierSelectionInterval = 2 * nModifierInterval`.
+   - Mainnet numbers: `nModifierInterval = 300s`; full 64-section interval = `10554s` (~2h56m); Triangles lookup delay = `600s` (~10m).
+   - Because minting and validation both use this, it is live consensus. Removing it without an activation gate would hard-fork historical/live behavior. Treat restoring the upstream interval as a future coordinated protocol upgrade, not a silent patch.
+
+2. **Critical stale-tip IBD validation bug (fixed on branch `audit/stake-modifier-review`):**
+   - `IsInitialBlockDownload()` also returns true when a synced node's tip is stale for >24h.
+   - `AcceptBlock()` used that operational IBD state to skip `CheckProofOfStake()` for any PoS block.
+   - `ConnectBlock()` used the same state to skip coinstake reward limit enforcement.
+   - Result: a stale-but-above-checkpoint node could accept live PoS blocks without kernel-target validation and without reward-limit validation.
+   - Fix: introduced `IsConsensusAssumeValidHeight(int nHeight)` so only the height-based historical fast path (hardcoded checkpoint / rolling assume-valid) skips PoS kernel/reward checks. Stale-tip IBD no longer disables live PoS checks.
+
+Cross-check: Z.Ai agreed the interval finding is correctly framed as a consensus/security weakening requiring activation, and agreed the stale-tip IBD validation bypass is a real critical bug with the height-based fix direction.
+
+Verification:
+- Watched new regression test fail before implementation (missing helper / compile red).
+- Targeted test: `./bin/test_triangles --run_test=consensus_safety_tests/pos_validation_skip_is_only_historical_fast_path --catch_system_errors=no --log_level=test_suite` => pass.
+- Normal build: `ctest --output-on-failure` in `build` => 4/4 passed.
+- Sanitizer build with CI flags: `ctest --output-on-failure` in `build-san-local` => 4/4 passed.
+
+No wallet files, production datadir, or live daemon state touched.

src/main.cpp

@@ -1670,6 +1670,12 @@ bool IsInitialBlockDownload()
     return false;
 }
 
+bool IsConsensusAssumeValidHeight(int nHeight)
+{
+    return (nHeight <= Checkpoints::GetTotalBlocksEstimate())
+        || (nHeight <= nAssumeValidThreshold);
+}
+
 void static InvalidChainFound(CBlockIndex* pindexNew)
 {
     if (pindexNew->nChainTrust > nBestInvalidTrust)
@@ -2192,8 +2198,7 @@ bool CBlock::ConnectBlock(CTxDBBase& txdb, CBlockIndex* pindex, bool fJustCheck)
     // are fully validated every time. Everything older takes the fast
     // path because we've already connected it successfully. A reorg that
     // tries to rewrite within the buffer is caught by full validation.
-    bool fAssumeValid = (pindex->nHeight <= Checkpoints::GetTotalBlocksEstimate())
-                     || (pindex->nHeight <= nAssumeValidThreshold);
+    bool fAssumeValid = IsConsensusAssumeValidHeight(pindex->nHeight);
     bool fIsInitialDownload = IsInitialBlockDownload();
 
     //// issue here: it doesn't know the version
@@ -2371,9 +2376,11 @@ bool CBlock::ConnectBlock(CTxDBBase& txdb, CBlockIndex* pindex, bool fJustCheck)
 
             int64_t nCalculatedStakeReward = GetProofOfStakeReward(nCoinAge, nFees);
 
-            // Enforce coinstake reward check only after IBD completes.
-            // During IBD the UTXO set is incomplete, causing nCalculatedStakeReward=0.
-            if (!IsInitialBlockDownload())
+            // Enforce coinstake reward for every fully validated block.
+            // Historical checkpoint / rolling-assume-valid blocks take the
+            // fAssumeValid fast path above; stale-tip IBD must not disable
+            // live reward validation for blocks above that fast path.
+            if (!fAssumeValid)
             {
                 if (nStakeReward > nCalculatedStakeReward)
                     return DoS(100, error("ConnectBlock() : coinstake pays too much(actual=%" PRId64 " vs calculated=%" PRId64 ")", nStakeReward, nCalculatedStakeReward));
@@ -3345,17 +3352,22 @@ bool CBlock::AcceptBlock()
     uint256 hashProofOfStake = 0, targetProofOfStake = 0;
     if (IsProofOfStake())
     {
-        if (IsInitialBlockDownload())
+        if (IsConsensusAssumeValidHeight(nHeight))
         {
-            // During IBD the UTXO set isn't fully loaded; CheckProofOfStake()
-            // would fail reading txPrev. Skip with a throttled log.
+            // Historical fast path: blocks at/below hardcoded checkpoint or
+            // rolling assume-valid have already been accepted by chain-level
+            // trust, so skip expensive PoS kernel verification there only.
+            // Do not key this off IsInitialBlockDownload(): stale-tip IBD is
+            // operational state, not permission to accept unchecked live PoS.
             if (nHeight % 10000 == 0)
-                printf("SKIP: PoS kernel check skipped for block %d during IBD\n", nHeight);
+                printf("SKIP: PoS kernel check skipped for historical fast-path block %d\n", nHeight);
             hashProofOfStake = 0; targetProofOfStake = 0;
         }
         else
         {
-            // Post-IBD: verify the PoS kernel signature normally.
+            // Verify the PoS kernel signature normally for every live block
+            // above the historical fast path, even if the tip is stale enough
+            // for IsInitialBlockDownload() to be true.
             if (!CheckProofOfStake(vtx[1], nBits, hashProofOfStake, targetProofOfStake))
                 return DoS(100, error("AcceptBlock() : check proof-of-stake failed for block %d", nHeight));
         }

src/main.h

@@ -147,6 +147,10 @@ unsigned int ComputeMinWork(unsigned int nBase, int64_t nTime);
 unsigned int ComputeMinStake(unsigned int nBase, int64_t nTime, unsigned int nBlockTime);
 int GetNumBlocksOfPeers();
 [[nodiscard]] bool IsInitialBlockDownload();
+// Height-based consensus fast path for historical checkpoint / rolling
+// assume-valid validation. This intentionally excludes operational IBD states
+// such as a stale tip; stale-tip IBD must not disable live PoS checks.
+[[nodiscard]] bool IsConsensusAssumeValidHeight(int nHeight);
 std::string GetWarnings(std::string strFor);
 bool GetTransaction(const uint256 &hash, CTransaction &tx, uint256 &hashBlock);
 uint256 WantedByOrphan(const CBlock* pblockOrphan);

src/test/consensus_safety_tests.cpp

@@ -18,6 +18,7 @@
 #include "../main.h"
 #include "../kernel.h"
 #include "../script.h"
+#include "../checkpoints.h"
 
 extern CBlockIndex* pindexBest;
 extern unsigned int nTargetSpacing;
@@ -317,6 +318,27 @@ BOOST_AUTO_TEST_CASE(stake_age_soft_cap_does_not_apply_pre_v5)
     BOOST_CHECK_EQUAL(GetWeight(justPastMax, now), (int64_t)nStakeMaxAge);
 }
 
+// ─── PoS validation fast path must be height-based (P0) ───────────────────
+// IsInitialBlockDownload() can also mean "tip is stale". That operational
+// state must never disable proof-of-stake kernel/reward validation for new
+// blocks above the hardened-checkpoint / rolling-assume-valid fast path.
+BOOST_AUTO_TEST_CASE(pos_validation_skip_is_only_historical_fast_path)
+{
+    int oldAssumeValid = nAssumeValidThreshold;
+    nAssumeValidThreshold = 0;
+
+    const int checkpointHeight = Checkpoints::GetTotalBlocksEstimate();
+
+    BOOST_CHECK(IsConsensusAssumeValidHeight(checkpointHeight));
+    BOOST_CHECK(!IsConsensusAssumeValidHeight(checkpointHeight + 1));
+
+    nAssumeValidThreshold = checkpointHeight + 25;
+    BOOST_CHECK(IsConsensusAssumeValidHeight(checkpointHeight + 25));
+    BOOST_CHECK(!IsConsensusAssumeValidHeight(checkpointHeight + 26));
+
+    nAssumeValidThreshold = oldAssumeValid;
+}
+
 // ─── Orphan block cap (P1 — DoS) ──────────────────────────────────────────
 // The cap on stored orphan blocks prevents an attacker from filling
 // memory with garbage. If too low, legitimate orphans are dropped. If