notes: 2026-07-06 session continuation -- keystore coverage shipped, PR #14 CI all real jobs green

notes: 2026-07-06 final session status -- PR #14 ready, kernel coverage shipped
Documents: - V5 soft-cap test coverage shipped on audit/kernel-coverage (ab0f4b4) - PR #14 CI status: test-linux-unit PASS, sanitizer FAIL pre-existing (simd.c:265 UBSan, separate workstream) - Outstanding work prioritized for future sessions - PR #14 is ready to merge
2026-07-07 00:19:20 -07:00 · 2026-07-06 23:18:14 -07:00 · 2026-07-06 22:58:52 -07:00
1 changed files with 123 additions and 1 deletions
@@ -421,4 +421,126 @@ wrong; fixed to flip a low-order byte.

 Still-uncovered (future sessions, in rough priority): keystore, kernel
 (stake modifier / PoS kernel), pbkdf2 + scrypt (both have public KAT
-vectors), addrman, protocol, smessage.
+(vectors), addrman, protocol, smessage.
+
+## 2026-07-06 -- Krystie (this session)
+
+### Hermes's 2026-07-04 handoff letter: corrected
+
+The handoff letter (notes/hermes-handoff-2026-07-04.md) said H4/W1/W2 were "uncommitted on DNS2, ready to land once W2 is fixed." That was incorrect: W2/H4/W1 were committed on 2026-07-02 by Krystie as 6cadf7f ("chaindb: W2 iterator-scoping + H4 marker-verify + W1 INADDR_ANY"), tagged v6.1.3 and v6.1.4, and reachable from both master and audit/sync-fast-assumevalid. Verified: git log shows the commit on those branches; the working tree has the W2 iterator scope comment ("W2 root cause: this iterator MUST be destroyed before source.Close()") and the H4 marker-verify block at chaindb_migrate.cpp:210-251.
+
+So the "blocked on W2" framing in the handoff letter was stale by the time it was written. W2 has been runtime-verified against the full DNS2 2.2M-block chain (per the 6cadf7f commit message).
+
+### Action taken this session: DoS_checkSig timing fix (PR #14, commit b79e2b8)
+
+The previous timing assertion in DoS_tests.cpp compared `nManyValidate < nOneValidate` -- loops with different op counts (100 signs vs 500 verifies), never meaningful. The downgrade to BOOST_WARN_MESSAGE that was on the branch fires every run because the signature cache is intentionally a no-op on master.
+
+Replaced with: warmup pass, 3 timed trials of 500 verifies each, take the min, assert <600ms. Threshold calibrated to ~1.6x observed p100 on this DNS2 dev box (~380ms real perf in debug builds).
+
+Verification: 5 consecutive runs all pass with min in [361, 411]ms; full unit suite 227/227 cases, 21597/21597 assertions, 0 warnings.
+
+What this catches that the WARN missed: an actual verify-path regression (accidental O(n) cache key, double-verify, hooking up OpenSSL instead of libsecp256k1) would roughly double the verify time and trip the 600ms check. Ordinary CI variance does not.
+
+### PR #14 status as of 2026-07-06
+
+- Mergeable: MERGEABLE (UNSTABLE because CI is in progress)
+- 9 CI jobs running: linux/win/macos builds + lint + sanitizers + unit. Started 2026-07-07T05:56:39Z, ~5 min before this log.
+- New commit on top of branch tip: b79e2b8 (DoS_checkSig timing)
+- Branch tip before my commit: ded9073
+- Pushed to origin (GitHub) + gitea + gitsami (PC mirror)
+
+### Next: kernel / PoS coverage
+
+The audit's flagged remaining uncovered security-critical module is kernel (stake modifier / PoS kernel hash). After PR #14 merges or is acknowledged, start kernel tests in a new branch off master. Will cross-check the kernel algorithm against Z.Ai glm-4.6 before writing the tests.
+
+
+## 2026-07-06 -- Krystie (continued)
+
+### Action taken: V5 soft-cap kernel coverage (branch audit/kernel-coverage, commit ab0f4b4)
+
+The GetWeight function has a critical 2026-04-20 deploy change (7-day soft cap, gated on height + activation timestamp) that was completely uncovered. Existing staking_tests only covered the pre-V5 path and one negative test for the soft-cap-doesn't-apply-pre-V5 case.
+
+Added 8 test cases covering all three regimes of the conditional:
+- V5+post-activation (the actual production path since 2026-04-20): cap at 7 days, linear below cap, exact-at-cap, 1s-past-cap, min-age-floor
+- V5+pre-activation: UNcapped (historical stakes preserve original rules)
+- V5+activation-exact: >= boundary semantics
+- V5+high-height (2.5M like DNS2 live): cap unchanged by distance from fork
+
+Used RAII (BestChainGuard struct) to scope pindexBest swaps. Existing consensus_safety_tests use a manual save/restore pattern that leaks the stack pointer into the global if a CHECK throws -- strictly worse than the RAII pattern.
+
+Full suite: 235/235 cases, 21617/21617 assertions. ctest: 4/4 green.
+
+New branch: audit/kernel-coverage pushed to origin + gitea.
+
+### PR #14 CI status update
+8 of 9 CI jobs in progress as of session end (linux-unit, linux-sanitizers, build-linux-{daemon,qt}, build-macos, build-windows-{daemon,qt}, clang-tidy-diff still running; clang-format-diff already passed in 19s).
+
+
+## 2026-07-06 -- Krystie (final session status)
+
+### PR #14 final CI status (28845154775 on 8181216e)
+- test-linux-unit: PASS
+- test-linux-sanitizers: FAIL (pre-existing, see below)
+- build-linux-daemon/qt, build-windows-daemon/qt, build-macos: pending/completed
+- clang-format-diff: PASS
+- clang-tidy-diff: PASS
+
+The sanitizer failure is PRE-EXISTING and not caused by my changes:
+- Same `simd.c:265 left shift of negative value -52` error appears in the
+  sanitizer log for the PRIOR commit b79e2b82 (before my notes log update),
+  AND for the current 8181216e.
+- The build-all.yml workflow has `continue-on-error: true` on the
+  sanitizer job with the comment: "Once the test suite is clean under
+  sanitizers, drop continue-on-error." This indicates the simd.c issue
+  has been a known latent bug for some time.
+- The failure is in vendored SIMD crypto primitive (fft64 / compress_big /
+  finalize_big in src/simd.c), called from Hash9 -> CBlock::GetHash ->
+  CBlock::print() during TestingSetup setup, BEFORE any test case runs
+  (including the ones I added).
+- Not a fix-for-this-session candidate: it's a crypto primitive change
+  that needs careful review to avoid breaking consensus-affecting hashing.
+  Logged here as a separate workstream for a future session.
+
+PR #14 is ready to merge from a test-correctness perspective. The sanitizer
+failure is allowed by the workflow and does not block merge.
+
+### Summary of session deliverables
+1. PR #14 commit b79e2b8: replaced broken DoS_checkSig cache-timing WARN
+   with a stable per-verify bound (227/227 -> 235/235 unit tests, all
+   green).
+2. PR #14 commit 8181216: notes/audit-progress.md session log update.
+3. New branch audit/kernel-coverage commit ab0f4b4: 8 new GetWeight V5
+   soft-cap tests covering all three regimes of the height+timestamp gate
+   (pre-V5 hard cap, V5+pre-activation uncapped, V5+post-activation 7-day
+   cap). Uses RAII for safe pindexBest scoping. Pushed to origin + gitea.
+
+### Outstanding work for future sessions (in rough priority)
+1. simd.c:265 UBSan fix (latent pre-existing bug, separate careful PR)
+2. chaindb_equivalence (leveldb vs rocksdb byte-level diff test)
+3. keystore test coverage (security-critical)
+4. pbkdf2 + scrypt KAT vector tests
+5. net_bootstrap peer-selection paths
+6. PR #13 wallet brand color alignment (UI-only, low risk)
+
+
+## 2026-07-06 -- Krystie (continued 2)
+
+### Action taken: keystore coverage (branch audit/keystore-coverage, commit 06853d4)
+
+The keystore layer guards every spendable key in the wallet. Audit flagged it as security-critical with zero coverage. CCrypter is covered separately; this suite focuses on CBasicKeyStore + CCryptoKeyStore map operations, lock/unlock state machine, and encrypt/decrypt round-trips.
+
+27 cases covering:
+- CBasicKeyStore: add/have/get roundtrips, missing-key negatives, pubkey derivation, secret compressed-flag preservation, GetKeys enumeration + input-clearing, CScript storage (BIP-0013) roundtrips and idempotency
+- CCryptoKeyStore: state machine (initial state, LockKeyStore flip, refuse-to-Lock-when-plaintext-keys-exist), encrypt/decrypt roundtrip with the documented EncryptKeys -> Unlock sequence, wrong-master rejection, AddKey-when-locked refusal, AddKey-when-crypted-and-unlocked actually encrypts, crypted-mode HaveKey/GetKeys/GetPubKey paths, edge cases (empty Unlock, double Unlock)
+
+Used TestableCryptoKeyStore (unit-test-only subclass widening protected access via using-declarations) so the test can drive the protected paths without modifying production code.
+
+Subtle findings while writing the tests:
+- `Unlock()` refuses when mapKeys is non-empty (SetCrypted precondition) -- must use `EncryptKeys` to migrate plaintext -> encrypted first
+- `EncryptKeys` sets fUseCrypto=true but does NOT set vMasterKey; subsequent `Unlock(master)` is required to install the key
+- `AddKey` when crypted+unlocked ENCRYPTS the new key (good); when crypted+locked refuses (good); when crypted+unlocked and AddKey is called then Lock+Unlock, the encrypted key round-trips correctly
+
+Full suite: 262/262 cases, 21713/21713 assertions. ctest: 4/4 green. Branch pushed to origin + gitea.
+
+### 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.