consensus: keep live PoS checks during stale-tip IBD

ci: fix sanitizer failures

.github/workflows/build-all.yml

@@ -88,11 +88,9 @@ jobs:
         run: cd build && ctest --output-on-failure || true
 
   test-linux-sanitizers:
-    # ASan + UBSan build of the daemon + unit tests. Allowed to fail until
-    # findings are triaged — see .github/workflows/lint.yml comment block.
-    # Once the test suite is clean under sanitizers, drop continue-on-error.
+    # ASan + UBSan build of the daemon + unit tests. This is a blocking
+    # signal: sanitizer regressions should fail the PR.
     runs-on: ubuntu-22.04
-    continue-on-error: true
     env:
       # ASan: leak detection off by default (BDB and OpenSSL produce noise on shutdown).
       # Re-enable once we've quieted the legitimate suspects.

notes/audit-progress.md

@@ -544,3 +544,54 @@ Full suite: 262/262 cases, 21713/21713 assertions. ctest: 4/4 green. Branch push
 
 ### PR #14 CI: ALL REAL JOBS GREEN
 Final CI run (run 28845879030 on f9a11fc) — every required job passes except the pre-existing simd.c sanitizer failure. PR #14 is merge-ready.
+
+## 2026-07-07 -- Krystie
+
+### Action taken: sanitizer lane fixed (branch fix/simd-ubsan-shift)
+
+Sami asked to fix the sanitizer failure after the release-infrastructure merge made all open PRs green except the known sanitizer issue.
+
+Root failures fixed:
+- `src/simd.c`: SPHlib SIMD FFT macros performed signed left shifts on values that can be negative (`simd.c:265` in CI). Replaced the signed arithmetic shifts with equivalent bounded multiplications by powers of two. This preserves intended arithmetic while removing C undefined behavior.
+- `src/util.cpp`: `DecodeBase32(std::string)` and `DecodeBase64(std::string)` took `&vchRet[0]` on empty decoded vectors. Added empty-return guards.
+- `src/base58.h` + `src/test/base58_tests.cpp`: `EncodeBase58(vector)` and its test harness took `&vch[0]` for empty vectors. Added an empty-vector guard and routed the test through the vector overload.
+- `src/util.h`: `Hash160(vector)` took `&vch[0]` for empty vectors. Switched to the existing pblank/length-0 pattern used by `Hash()` helpers.
+- `src/script.cpp`: OP_RIPEMD160 / OP_SHA1 / OP_SHA256 used `&vch[0]` for empty stack data. Added pblank/length-0 handling; OP_HASH160 already routes through `Hash160`.
+- `src/test/DoS_tests.cpp`: sanitizer instrumentation made the signature microbenchmark threshold false-fire. Kept all signature correctness checks, but skips the perf threshold under ASan builds.
+- `.github/workflows/build-all.yml`: removed `continue-on-error: true` from `test-linux-sanitizers`; sanitizer regressions are blocking again.
+
+Verification:
+- Local sanitizer build with CI flags: `ctest --output-on-failure` => 4/4 passed in build-san-local.
+- 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/base58.h

@@ -67,6 +67,8 @@ inline std::string EncodeBase58(const unsigned char* pbegin, const unsigned char
 // Encode a byte vector as a base58-encoded string
 inline std::string EncodeBase58(const std::vector<unsigned char>& vch)
 {
+    if (vch.empty())
+        return std::string();
     return EncodeBase58(&vch[0], &vch[0] + vch.size());
 }
 

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/script.cpp

@@ -971,17 +971,19 @@ bool EvalScript(vector<vector<unsigned char> >& stack, const CScript& script, co
                     if (stack.size() < 1)
                         return false;
                     valtype& vch = stacktop(-1);
+                    static unsigned char pblank[1];
+                    const unsigned char* pch = vch.empty() ? pblank : &vch[0];
                     valtype vchHash((opcode == OP_RIPEMD160 || opcode == OP_SHA1 || opcode == OP_HASH160) ? 20 : 32);
                     if (opcode == OP_RIPEMD160)
                     {
                         TRI_OPENSSL_SUPPRESS_DEPRECATED_BEGIN
-                        RIPEMD160(&vch[0], vch.size(), &vchHash[0]);
+                        RIPEMD160(pch, vch.size(), &vchHash[0]);
                         TRI_OPENSSL_SUPPRESS_DEPRECATED_END
                     }
                     else if (opcode == OP_SHA1)
-                        SHA1(&vch[0], vch.size(), &vchHash[0]);
+                        SHA1(pch, vch.size(), &vchHash[0]);
                     else if (opcode == OP_SHA256)
-                        SHA256(&vch[0], vch.size(), &vchHash[0]);
+                        SHA256(pch, vch.size(), &vchHash[0]);
                     else if (opcode == OP_HASH160)
                     {
                         uint160 hash160 = Hash160(vch);

src/simd.c

@@ -143,6 +143,11 @@ static const s32 alpha_tab[] = {
  *   d5:   min= -252   max= 4402
  *   d6:   min=-4335   max= 4335
  *   d7:   min=-4332   max=  322
+ *
+ * Use multiplication, not signed left shift, for powers of two below. FFT
+ * values can be negative; left-shifting a negative signed integer is
+ * undefined in C, while these bounded multiplications are defined and
+ * preserve the intended arithmetic.
  */
 #define FFT8(xb, xs, d)   do { \
 		s32 x0 = x[(xb)]; \
@@ -150,13 +155,13 @@ static const s32 alpha_tab[] = {
 		s32 x2 = x[(xb) + 2 * (xs)]; \
 		s32 x3 = x[(xb) + 3 * (xs)]; \
 		s32 a0 = x0 + x2; \
-		s32 a1 = x0 + (x2 << 4); \
+		s32 a1 = x0 + (x2 * 16); \
 		s32 a2 = x0 - x2; \
-		s32 a3 = x0 - (x2 << 4); \
+		s32 a3 = x0 - (x2 * 16); \
 		s32 b0 = x1 + x3; \
-		s32 b1 = REDS1((x1 << 2) + (x3 << 6)); \
-		s32 b2 = (x1 << 4) - (x3 << 4); \
-		s32 b3 = REDS1((x1 << 6) + (x3 << 2)); \
+		s32 b1 = REDS1((x1 * 4) + (x3 * 64)); \
+		s32 b2 = (x1 * 16) - (x3 * 16); \
+		s32 b3 = REDS1((x1 * 64) + (x3 * 4)); \
 		d ## 0 = a0 + b0; \
 		d ## 1 = a1 + b1; \
 		d ## 2 = a2 + b2; \
@@ -179,21 +184,21 @@ static const s32 alpha_tab[] = {
 		FFT8(xb, (xs) << 1, d1_); \
 		FFT8((xb) + (xs), (xs) << 1, d2_); \
 		q[(rb) +  0] = d1_0 + d2_0; \
-		q[(rb) +  1] = d1_1 + (d2_1 << 1); \
-		q[(rb) +  2] = d1_2 + (d2_2 << 2); \
-		q[(rb) +  3] = d1_3 + (d2_3 << 3); \
-		q[(rb) +  4] = d1_4 + (d2_4 << 4); \
-		q[(rb) +  5] = d1_5 + (d2_5 << 5); \
-		q[(rb) +  6] = d1_6 + (d2_6 << 6); \
-		q[(rb) +  7] = d1_7 + (d2_7 << 7); \
+		q[(rb) +  1] = d1_1 + (d2_1 * 2); \
+		q[(rb) +  2] = d1_2 + (d2_2 * 4); \
+		q[(rb) +  3] = d1_3 + (d2_3 * 8); \
+		q[(rb) +  4] = d1_4 + (d2_4 * 16); \
+		q[(rb) +  5] = d1_5 + (d2_5 * 32); \
+		q[(rb) +  6] = d1_6 + (d2_6 * 64); \
+		q[(rb) +  7] = d1_7 + (d2_7 * 128); \
 		q[(rb) +  8] = d1_0 - d2_0; \
-		q[(rb) +  9] = d1_1 - (d2_1 << 1); \
-		q[(rb) + 10] = d1_2 - (d2_2 << 2); \
-		q[(rb) + 11] = d1_3 - (d2_3 << 3); \
-		q[(rb) + 12] = d1_4 - (d2_4 << 4); \
-		q[(rb) + 13] = d1_5 - (d2_5 << 5); \
-		q[(rb) + 14] = d1_6 - (d2_6 << 6); \
-		q[(rb) + 15] = d1_7 - (d2_7 << 7); \
+		q[(rb) +  9] = d1_1 - (d2_1 * 2); \
+		q[(rb) + 10] = d1_2 - (d2_2 * 4); \
+		q[(rb) + 11] = d1_3 - (d2_3 * 8); \
+		q[(rb) + 12] = d1_4 - (d2_4 * 16); \
+		q[(rb) + 13] = d1_5 - (d2_5 * 32); \
+		q[(rb) + 14] = d1_6 - (d2_6 * 64); \
+		q[(rb) + 15] = d1_7 - (d2_7 * 128); \
 	} while (0)
 
 /*

src/test/DoS_tests.cpp

@@ -304,11 +304,18 @@ BOOST_AUTO_TEST_CASE(DoS_checkSig)
     // OpenSSL instead of libsecp256k1). Adjust if this false-fires on a
     // materially slower CI runner — the per-trial prints above make the
     // threshold-defining evidence reproducible.
+#if defined(__SANITIZE_ADDRESS__)
+    // ASan/UBSan builds intentionally instrument every memory access and are
+    // not meaningful microbenchmark environments. Keep the correctness checks
+    // above and below, but do not enforce the perf threshold under sanitizers.
+    if (fDebug) printf("DoS_Checksig sanitizer build: skipping perf threshold (%ld ms)\n", nPerVerifyMs);
+#else
     BOOST_CHECK_MESSAGE(nPerVerifyMs < 600,
         "Signature verify regression: " << nPerVerifyMs
         << "ms for 500 verifies (expected <600ms). "
         << "Cache is a no-op by design (see script.cpp CheckSig); "
         << "if this fires, an actual verify-path change has slowed it down.");
+#endif
 
     // Empty a signature, validation should fail:
     CScript save = tx.vin[0].scriptSig;

src/test/base58_tests.cpp

@@ -26,7 +26,7 @@ BOOST_AUTO_TEST_CASE(base58_EncodeBase58)
         std::vector<unsigned char> sourcedata = ParseHex(test[0].get_str());
         std::string base58string = test[1].get_str();
         BOOST_CHECK_MESSAGE(
-                    EncodeBase58(&sourcedata[0], &sourcedata[sourcedata.size()]) == base58string,
+                    EncodeBase58(sourcedata) == base58string,
                     strTest);
     }
 }

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

src/util.cpp

@@ -757,6 +757,8 @@ vector<unsigned char> DecodeBase64(const char* p, bool* pfInvalid)
 string DecodeBase64(const string& str)
 {
     vector<unsigned char> vchRet = DecodeBase64(str.c_str());
+    if (vchRet.empty())
+        return string();
     return string((const char*)&vchRet[0], vchRet.size());
 }
 
@@ -944,6 +946,8 @@ vector<unsigned char> DecodeBase32(const char* p, bool* pfInvalid)
 string DecodeBase32(const string& str)
 {
     vector<unsigned char> vchRet = DecodeBase32(str.c_str());
+    if (vchRet.empty())
+        return string();
     return string((const char*)&vchRet[0], vchRet.size());
 }
 

src/util.h

@@ -603,8 +603,9 @@ uint256 SerializeHash(const T& obj, int nType=SER_GETHASH, int nVersion=PROTOCOL
 
 inline uint160 Hash160(const std::vector<unsigned char>& vch)
 {
+    static unsigned char pblank[1];
     uint256 hash1;
-    SHA256(&vch[0], vch.size(), (unsigned char*)&hash1);
+    SHA256(vch.empty() ? pblank : &vch[0], vch.size(), (unsigned char*)&hash1);
     uint160 hash2;
     TRI_OPENSSL_SUPPRESS_DEPRECATED_BEGIN
     RIPEMD160((unsigned char*)&hash1, sizeof(hash1), (unsigned char*)&hash2);