GitHub Actions CI (workflow 'Build All Platforms', run #29559727754)
flagged test-linux-unit and test-linux-sanitizers as failing. The CI
runs the test binary via 'ctest --output-on-failure' from build/, but
the consensus_safety_tests static-source grep tests called
readEntireFile('src/main.cpp') with paths resolved relative to CWD.
With CWD = build/, those paths did not exist; the tests failed with
'critical check !src.empty() has failed'. Same root cause for the
staking_tests::is_staking_safe_is_continuous_not_one_shot test which
opened 'src/miner.cpp' by raw __FILE__ slicing.
Specifically:
consensus_safety_tests.cpp: 9 failures across
convergence_rejects_below_hardened_checkpoint, hardened_checkpoint_init_is_startup_only,
above_checkpoint_greatest_trust_wins, getheaders_recovers_via_genesis_when_locator_disjoint,
getheaders_recovers_via_checkpoint_when_locator_has_it, reorg_guard_fails_closed_when_checkpoint_pointer_null,
reorg_guard_offbyone_hardening, hardened_checkpoint_no_rogue_guard_in_other_files
staking_tests.cpp: 1 failure in is_staking_safe_is_continuous_not_one_shot
Note: my pre-merge '280/280 tests pass' claim was based on running the
test binary directly from the repo root, where 'src/' resolves
trivially. ctest is the canonical CI invocation. This CI run was the
first time we exercised it.
Fix:
- Add findProjectRootFromHere(__FILE__) helper that:
(1) prefers an absolute path in __FILE__ (/foo/bar/src/test/...),
(2) falls back to a build-dir-relative anchor (./src/test/... or
bare src/test/...) when cmake+ninja produces those,
(3) defends against ctest's CWD=build/ by walking up from CWD
looking for the canonical src/checkpoints.cpp sentinel.
- Apply uniformly in consensus_safety_tests.cpp (helper + 1 rogue-guard
walker that builds project-root-relative paths before comparing
against allowed_files) and staking_tests.cpp (mirrored helper).
- Strict-mode BOOST_REQUIRE_MESSAGE failure paths now include the
resolved path so the next person debugging this hits the issue
immediately.
Verified: 'cd build && ctest --output-on-failure' now reports 0
failures across all 4 ctest projects (triangles_unit_tests,
chaindb_equivalence_tests, snapshotnet_tests, chaindb_runtime_tests).
Direct 'test_triangles' invocation still works for ad-hoc checks.
fix/consensus-convergence — the rules around reorg finality and the
getheaders fork-peer handler previously used locally advanced state
that prevented two honest nodes from converging after extended
disconnection. This commit removes the local-finality rules and
restores convergence above the last globally shared hardened
checkpoint.
Reorganize() now:
- Rejects reorgs whose fork point is at or below the compiled
hardened checkpoint (sourced from Checkpoints::GetLastCheckpoint
at startup, never advanced at runtime).
- Above the checkpoint: greatest cumulative chain trust wins. No
depth cap, no local finality, no 10% trust hysteresis.
pindexFinalized is renamed to pindexLastHardenedCheckpoint to make
clear that the variable now refers to the compiled checkpoint anchor,
not a locally advanced finality depth. Its initialization in init.cpp
runs once at startup; no runtime advancement.
The getheaders handler now serves canonical history based on what the
peer actually knows:
- If the peer's locator contains the hardened checkpoint, serve
headers from the checkpoint forward.
- Otherwise, serve from the last common ancestor (falling back to
genesis if no overlap exists). This lets a forked peer recover
instead of being handed a header whose parent it doesn't have.
CBlockLocator gains two small public accessors (Has, FindCommonAncestorInMainChain)
so the recovery code doesn't have to reach into protected state.
Staking safety gate is now continuous in StakeMiner (main.cpp's
IsStakingSafe runs every iteration). Removed the once-only fTryToSync
flag whose reset-after-first-use made the strong peer-count / IBD
check ineffective after a network outage mid-staking. The gate refuses
to stake when IBD is active, fewer than 2 handshaken peers exist, our
height is behind the peer median, or a peer reports a tip >=2 blocks
ahead of ours (possible competing fork signal).
Tests:
- consensus_safety_tests.cpp: 6 new tests pinning the convergence
rule's structure against src/main.cpp and src/init.cpp. Replaces the
old max_reorg_depth_enforced test (which pinned the now-removed
local-finality constant).
- staking_tests.cpp: 3 new tests pinning the continuous gate's
behavior and the absence of fTryToSync from runtime code.
All 277 unit-test cases (21,752 assertions) pass locally. The Qt GUI
was not rebuilt; the daemon (trianglesd), CLI (triangles-cli), and
test binary (test_triangles) all link and execute.
Reviewed-against: pre-commit HEAD
No push to master performed per standing rule.
Two coverage gaps closed in one commit because they were both
identified during the same test audit pass.
--- keystore_tests.cpp (NEW, 472 lines) ---
The keystore layer guards every spendable key in the wallet: a bug
here loses keys, accepts wrong keys, or breaks encryption round-trips.
The audit flagged it as security-critical with zero coverage.
27 cases:
- CBasicKeyStore: add/have/get roundtrips, missing-key negative cases,
pubkey derivation paths, secret compressed-flag preservation, GetKeys
enumeration + input-set 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 (not Unlock on a
plaintext store, which SetCrypted refuses), wrong-master rejection,
AddKey-when-locked refusal, AddKey-when-crypted-and-unlocked actually
encrypts, crypted-mode HaveKey/GetKeys/GetPubKey paths, edge cases
(empty store Unlock, double Unlock).
Uses TestableCryptoKeyStore (a unit-test-only subclass that widens the
protected Unlock/EncryptKeys access via using-declarations) so the test
can drive the protected paths without modifying production code.
--- staking_tests.cpp: GetWeight V5 soft-cap (8 cases) ---
The 2026-04-20 deploy added a 7-day soft cap to GetWeight that activates
ONLY when BOTH height >= FORK_HEIGHT_V5 (17651) AND nIntervalEnd >=
STAKE_AGE_SOFT_CAP_ACTIVATION (1776000000 = 2026-04-12 ~13:20 UTC). This
is the production code path for every stake on the live chain since the
deploy.
The existing staking_tests only covered the pre-V5 (nStakeMaxAge hard
cap) path, plus one negative test that confirmed the soft cap does NOT
apply pre-V5. The two production regimes -- V5+post-activation and
V5+pre-activation -- had no direct test coverage.
Adds 8 cases:
- V5+post-activation: cap at 7 days for stakes past the cap
- V5+post-activation: linear below the cap
- V5+post-activation: exactly at the cap (boundary)
- V5+post-activation: 1 second past the cap (boundary)
- V5+pre-activation: UNcapped (historical stakes preserve original rules)
- V5+activation-exact: >= semantics include the activation timestamp
- V5+high height (2.5M, like DNS2 live): cap unchanged by distance from fork
- V5+min-age floor: nStakeMinAge still returns 0 below floor
Uses RAII (BestChainGuard) to scope pindexBest swaps so a failed
assertion can't leave a stack pointer dangling in the global -- an
improvement over the manual save/restore pattern used in
consensus_safety_tests.
Verified: full test_triangles suite green (0 errors). Keystore 27/27,
staking 11/11 (3 original + 8 new), 21713+ assertions, ctest 4/4.
Co-authored-by: Sami Ahmed <sami@sami-ahmed.net>
After reverting the consensus-affecting PoS reward rework, the original
truncating formula (nCoinAge * rate / 365 / COIN) is restored. It is not
exactly proportional at every boundary (r2 can be 2*r1 +/- 1 due to integer
truncation). That rounding is the on-chain behavior and must not be changed
in consensus code, so relax pos_reward_proportional_to_coinage to allow a
1-unit difference rather than demanding exact doubling. Test-only change.
DO NOT MERGE without explicit sign-off. This changes GetProofOfStakeReward
rounding (round-half-up vs truncation, and whole-coin truncation of coin
age first). New formula can pay 1 unit more than the old one for some
inputs; un-upgraded nodes would reject such coinstakes — hard-fork risk.
The test-suite proportionality failures it addresses could instead be
fixed by relaxing the test. staking_tests expectations updated to match.
(From prior audit session; isolated here for review.)
Sami Ahmed
sami7777