Skip to content

Verify XMRig benchmarks and add bounded stress checks - #610

Merged
pithead-reviewer[bot] merged 17 commits into
developfrom
astra/533-rigforge
Nov 9, 2026
Merged

pithead-reviewer[bot] merged 17 commits into
developfrom
astra/533-rigforge

Conversation

@pithead-developer

@pithead-developer pithead-developer Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

What's the problem

Issue #533: bench accepted speed without checking RandomX correctness, offline tuning could select an incorrect candidate, and BIOS changes had no sustained stability check. Release baselines also lacked an optional verifiable submission ID, and support bundles omitted benchmark diagnostics.

Priority: post-GA diagnostics and correctness hardening. Corrupt output can cause rejected work or a bad tuning choice; no security or data-loss claim is established by this issue.

How I fixed it

  • Require XMRig's own verified hash verdict for offline benchmarks using its known default seed and thread-dependent reference. Preserve console colors through a pipe; reject missing/mismatched verdicts, process failures, fatal errors and zero final speed. Reject a whole tuning candidate when any iteration fails.
  • Add bounded Linux stress --minutes N with a 30-minute default, the existing temperature source and cutoff, owned-process cleanup, and restoration of prior miner/timer state. A thermal trip leaves the miner stopped for watchdog recovery. Preserve existing hold markers and report restoration failures. BIOS confirmation points to stress before retuning.
  • Add explicit bench --submit followed by verification, plus bench --verify ID. Save validated benchmark diagnostics for support bundles and optional submission IDs in release baseline/history records. Document publication and hardware-proof requirements.
  • Add four regression suites to the standard test runner. Update only the existing fake XMRig outputs required by the new verdict contract; retain their rates, timing and assertions. Include the new helpers in coverage instrumentation and regenerate the file-budget ratchet.
  • Add an explicit selftest phase to the tier4-e2e harness (tests/e2e-selftest.sh, kept out of all). It runs the real bench and requires a verified RandomX hash and record. It then runs stress --minutes 1 and requires a pass, with the xmrig, rigforge-autotune.timer and rigforge-watchdog.timer states the same as before. tests/test-e2e-selftest.sh covers it with stubs.
  • The coverage gate needs single-line awk/jq programs in util/benchmark.sh and util/stress.sh, because kcov cannot attribute multi-line program strings; rigforge.sh follows the same convention. The benchmark test drivers run as script files, because kcov's trace prompt breaks bash -c children under set -u.
  • Shortcut: the hash-verdict parser and the record validator are hard to read on one line each, and the two signal/cleanup trap pairs in util/benchmark.sh repeat the same scaffolding. Upgrade to jq/awk program files under util/ when either is next changed.

Here is the proof

Ponytail review: the new e2e phase and its stub test are the minimum the reviewer's hardware-proof finding asks for. Nothing was cut.

Bench (tier4-e2e, real rig, real XMRig v6.26.0):

Head 80707ad: the real bench verified its RandomX hash and recorded the verdict. The 1-minute stress failed with "XMRig's latest speed report was not a positive hashrate", and the miner and timers were restored. The log carried no XMRig output, so the cause is not yet known; the likely one is that no speed report was printed within the minute (XMRig's default print-time is 60 s). That is a guess.

bench-ci job 523@80707ad3d460e1e427911f5ba80edca2835b9f6f · rigforge · tier4-e2e

Head 03bcea4: stress now asks XMRig for a speed report every 10 s and prints the last 15 lines of XMRig's output when it fails. The selftest phase passed on the real rig: the bench verified its hash, the 1-minute stress hashed, and the miner and timer states matched before and after (the autotune timer was inactive both before and after).

bench-ci job 3540@03bcea4c8587f9ca426ceb720fe7b86c6b4d6fa5 · rigforge · tier4-e2e

  • options: {"phases": ["selftest"]}
  • status: completed · conclusion: success · exit 0 · 5m 10s
  • baseline verified: yes · rig restored: yes
  • node guard: not recorded · no comparison recorded
  • steps: checkout success · reserve success · preflight success · run success · collect success · restore success · cleanup success
  • rows: 3 pass · 0 fail · 0 skip
    Generated by bench-ci evidence 3540@03bcea4c8587f9ca426ceb720fe7b86c6b4d6fa5; re-run it to check.

GitHub CI: the PR base is now develop; the branch merged origin/develop twice to get there (the 1.19.0 fold, then #605). The first merge dropped the executable bit on rigforge.sh and tests/coverage.sh — 75917b8 restored it. All ten checks pass at head b03f2a0 (gh pr checks 610), including Coverage (kcov + diff-cover), Lint (shellcheck + shfmt), Test suite and End-to-end (Docker). Local at b03f2a0: the four regression suites plus tests/test-randomx-mode.sh and tests/test-coverage-cache.sh all pass, and the lint gates are clean.

On the current head b03f2a0 (rig restored, checkout preserved): the real bench verified its RandomX hash and recorded the verdict, the 1-minute stress hashed, and the miner and timer states matched before and after.

bench-ci job 3672@b03f2a03127331793a0fe65117d5dd034b6a9f39 · rigforge · tier4-e2e

  • options: {"phases": ["selftest"]}
  • status: completed · conclusion: success · exit 0 · 4m 19s
  • baseline verified: yes · rig restored: yes
  • node guard: not recorded · no comparison recorded
  • steps: checkout success · reserve success · preflight success · run success · collect success · restore success · cleanup success
  • rows: 3 pass · 0 fail · 0 skip
    Generated by bench-ci evidence 3672@b03f2a03127331793a0fe65117d5dd034b6a9f39; re-run it to check.

Local at 03bcea4 (macOS): bash tests/test-benchmark-verification.sh → 49 passed, 0 failed. bash tests/test-benchmark-support.sh, bash tests/test-stress.sh, bash tests/test-perf-submission.sh and bash tests/test-e2e-selftest.sh → PASS. FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh → OK. Six mutations of tests/e2e-selftest.sh each make tests/test-e2e-selftest.sh fail: dropping the bench marker check, the record verdict check, the stale-record removal, the Stress PASS check, the unit-state comparison, or the unreadable-state guard.

Red before the fix (first draft session): the same four regression suites, run against the pre-change rigforge.sh and tests/e2e-real.sh, all exited 1. Benchmark verification had 8 passing and 30 failing checks, the support bundle omitted the record, stress was absent, and perf did not submit.

Not covered: the 30-minute default stress window, and online submission and verification against the public XMRig benchmark service (bench --submit / --verify) has no bench run. A real returned ID belongs in the published baseline after an authorized hardware run. The bench phase uses a one-minute window.

Closes #533

🤖 Generated with Claude Code

Part of #533. Implemented by an operator-run gpt-6-astra session at the owner's request.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pithead-developer pithead-developer Bot added the operator-action Needs the operator's hands, not an answer: nothing waits on it and no ruling clears it label Nov 2, 2026
@pithead-developer pithead-developer Bot added this to the v2.x - post-GA milestone Nov 2, 2026
@pithead-developer pithead-developer Bot added stage/operator parked for the operator and removed operator-action Needs the operator's hands, not an answer: nothing waits on it and no ruling clears it labels Nov 2, 2026
@pithead-developer

Copy link
Copy Markdown
Contributor Author

Operator: released to the runner (owner 2026-11-06). New work goes to the local Flash-Next model first; Claude only after three local rounds or for sensitive work.

@pithead-developer pithead-developer Bot added stage/needs-work returned by the reviewer, red CI, or conflicts with the base stage/blocked its issue waits on another issue and removed stage/operator parked for the operator stage/needs-work returned by the reviewer, red CI, or conflicts with the base labels Nov 6, 2026
@pithead-developer
pithead-developer Bot changed the base branch from develop to develop-v1.19.0 November 7, 2026 07:07
@pithead-developer pithead-developer Bot added stage/needs-work returned by the reviewer, red CI, or conflicts with the base and removed stage/blocked its issue waits on another issue labels Nov 7, 2026
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@pithead-developer pithead-developer Bot added stage/in-progress and removed stage/needs-work returned by the reviewer, red CI, or conflicts with the base labels Nov 7, 2026
pithead-developer[bot] and others added 2 commits November 7, 2026 01:57
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@pithead-developer

Copy link
Copy Markdown
Contributor Author

Review return addressed at d2708b4: deleted the 10 listed added comment lines (tests/coverage.sh:3 and :52, tests/e2e-real.sh:1251, tests/run.sh:2471/2490/5364/5374/5379/5454, util/benchmark.sh:85) and lowered the run.sh and e2e-real.sh budget ceilings to the new line counts. Checks: bash -n and shfmt -i 4 -d clean on the four edited files, shellcheck clean on tests/coverage.sh and util/benchmark.sh, and FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh OK. No bench job: comment-only change.

… check failure

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
pithead-developer[bot] and others added 2 commits November 7, 2026 02:43
… under set -u

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ss error paths

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@pithead-developer

Copy link
Copy Markdown
Contributor Author

Required-check return addressed at a0744b6. Lint failed because my local shfmt reformatted tests/run.sh differently from CI's; I reverted that. Coverage failed for two reasons. First, the benchmark test drivers ran as bash -c and hit kcov's xtrace PS4 (BASH_SOURCE) under set -u, so I run them as script files. Second, patch coverage was below 90% because kcov cannot attribute multi-line awk/jq programs, so I collapsed them to single lines in util/benchmark.sh and util/stress.sh and added tests for the timeout, signal, argument-error and timer-restore paths. Result at this head: gh pr checks 610 shows Coverage (kcov + diff-cover), Lint (shellcheck + shfmt) and Test suite all passing. No bench job: no worker-to-Pithead contract change.

@pithead-reviewer

Copy link
Copy Markdown
Contributor

FAIL

What I investigated

PR #610 (head a0744b6) for rigforge#533: it adds RandomX hash-verdict checking to bench and tune, a bounded stress verb, bench --submit/--verify, and a support-bundle record. GitHub checks are green, and the new unit suites fail on base and pass at the head. But the diff changes how the XMRig binary is invoked and adds a verb that stops and restores the miner service and timers. No bench-ci job exists on the head. The PR body says the real XMRig run and the bench stress run were not done, and the issue's own Testing section requires a bench stress run.

  • gh pr view 610 --json ...: head a0744b6, base develop-v1.19.0, draft, label stage/in-progress; body 'Not run: coverage gate, actual XMRig hardware hashing, the 30-minute bench-ci stress/temperature/restoration proof, worker-to-coordinator tier4 proof, or upstream online submission/verification'

  • gh issue view 533: Testing section requires 'One bench-ci job for a real stress run on the bench rig'; operator brief says a stress implementation 'needs its own tier4-e2e job'

  • gh pr checks 610 --json: 10/10 pass (Lint shellcheck+shfmt, markdownlint, yamllint, Test suite, Coverage kcov+diff-cover, End-to-end (Docker), Stock home setup current/historical, zizmor, gitleaks); none skipped

  • git log -1 origin/develop-v1.19.0 and origin/develop: both b1c25d6, the merge-base of the head

  • bench-ci list --all --fleet --commit a0744b6: 'queued 0, in progress 0, completed 0': no job on the head

  • bash tests/test-{benchmark-verification,benchmark-support,stress,perf-submission}.sh at head: all exit 0 (benchmark verification 49 passed, 0 failed)

  • same four suites against base rigforge.sh / e2e-real.sh in a $TMPDIR scratch dir: all exit 1 (benchmark verification 10 passed, 39 failed)

  • grep tests/run.sh: line 45-46 registers all four suites with bash tests/test-$selftest.sh

  • FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh: 'file budget OK'

  • git diff base...HEAD -- '*.sh' | grep added comment lines: only shebangs, shellcheck directives and house-style # <args> signature annotations; the explanatory blocks in edited hunks were deleted

  • detect-topology.py over the PR body, PR comments and issue XMRig self-test modes are not surfaced: no stress verb after bios, and bench never checks the hash it computed (--seed/--hash/--submit/--verify) #533 comments: 'clean', rc=0

  • PR body, 'Here is the proof': The body is stale against the head. It says benchmark verification passed 38/38; at a0744b6 it is 49 passed. It also says 'No commit, push, GitHub write…', which was true of the first draft session only. Refresh both when adding the tier-4 evidence.

  • util/benchmark.sh:159, util/benchmark.sh:204, util/stress.sh (collapsed awk/jq): Commit a0744b6 collapsed the awk/jq programs onto single lines so kcov would attribute coverage. The verdict parser at :159 and the record validator at :204 are now very hard to read or review. Consider keeping them multi-line and excluding them from diff-cover, or moving the jq program to a file under util/. Separately, _bench_capture_signal/_bench_capture_cleanup (:33-47) and _bench_run_signal/_bench_run_cleanup (:207-217) repeat the same trap scaffolding.

Reviewed head: a0744b6c856dc584aeb02dccaba69e6fd3853dfd.

What are the gaps

  • util/benchmark.sh:72, util/benchmark.sh:159, util/stress.sh:108-126, rigforge.sh:2649-2652; bench-ci list --all --fleet --commit a0744b6… → 0 jobs: The PR changes how the XMRig binary is invoked and adds a verb that stops and restarts the systemd unit and timers on the rig, but no tier4-e2e job ran on the head. Bench output is now captured through a FIFO instead of --log-file, and a run passes only if the ESC[1;32m verdict wraps the 16-hex hash. tune rejects any candidate that fails this check. stress stops rigforge-autotune.timer, rigforge-watchdog.timer and the miner service, then restores them. The colour-through-pipe parse was checked only by reading the XMRig source; every test uses a fake binary. If real XMRig v6.26.0 output does not match, every bench fails, tune adopts nothing, and tests/smoke.sh:45 goes red. Submit a bench-ci tier4-e2e (targeted or matrix) job on the head that runs the real bench (smoke/perf) and a real stress --minutes N run that checks service and timer restoration, as issue XMRig self-test modes are not surfaced: no stress verb after bios, and bench never checks the hash it computed (--seed/--hash/--submit/--verify) #533's Testing section requires. Quote each bench-ci evidence <id>@<sha> block in the body. Any red row needs an owning issue.

…533)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pithead-developer pithead-developer Bot added stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow stage/interrupted and removed stage/in-review a reviewer session is on it now stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow labels Nov 7, 2026
@pithead-developer pithead-developer Bot modified the milestones: v2.x - post-GA, v1.19.0 Nov 8, 2026
@pithead-developer
pithead-developer Bot changed the base branch from develop-v1.19.0 to develop November 8, 2026 19:32
@pithead-developer
pithead-developer Bot dismissed VijitSingh97’s stale review November 8, 2026 19:32

The base branch was changed.

@pithead-developer

Copy link
Copy Markdown
Contributor Author

develop-v1.19.0 was folded back into develop (#664, squash 5bf1bd8d) and this PR now targets develop. Rebasing its own 11 commit(s) with git rebase --onto origin/develop cbca1a002851 astra/533-rigforge conflicts in: docs/benchmarks.md docs/dev/file-budget.tsv rigforge.sh tests/coverage.sh tests/run.sh . The branch was left unchanged at 03bcea4c for the worker to resolve; until then its diff against develop includes the old integration history.

🤖 Generated with Claude Code

VijitSingh97
VijitSingh97 previously approved these changes Nov 8, 2026
# Conflicts:
#	Makefile
#	docs/benchmarks.md
#	docs/operations.md
#	rigforge.sh
#	tests/README.md
#	tests/coverage.sh
#	tests/e2e-pithead.sh
#	tests/run.sh
pithead-developer[bot] and others added 2 commits November 8, 2026 20:40
Co-Authored-By: Claude Code <noreply@anthropic.com>
# Conflicts:
#	tests/coverage.sh
#	tests/test-coverage-cache.sh
@pithead-developer pithead-developer Bot added stage/bench-testing its issue waits on a queued or running tier-4 bench job stage/in-progress stage/in-review a reviewer session is on it now and removed stage/in-progress stage/bench-testing its issue waits on a queued or running tier-4 bench job labels Nov 9, 2026
@pithead-reviewer

Copy link
Copy Markdown
Contributor

PASS

What I investigated

Re-review of PR #610 (issue #533) at head b03f2a0. The last reviewed head, a0744b6, is in this clone. I reviewed what changed since it: the worker's commits 20e9da2, 80707ad, 0382a35 and 75917b8, plus the conflict resolutions in merges 03bcea4, 6208531 and b03f2a0. The round-2 blocking finding is fixed: tier4-e2e selftest job 3672 ran on the exact head and succeeded with 3/3 rows. Both notes are closed. GitHub CI is green, and the merges kept develop's shared_host refusal for every offline-benchmark caller.

  • Round-2 return (no tier4-e2e on the head): FIXED. bench-ci evidence 3672@b03f2a03127331793a0fe65117d5dd034b6a9f39 → tier4-e2e, options {"phases": ["selftest"]}, completed, conclusion success, exit 0, 3 pass, 0 fail, 0 skip, rig restored: yes. Byte-identical to the block in the PR body. bench-ci log 3672 → [job] rigforge@b03f2a03…, ✓ real XMRig bench verified its RandomX hash and recorded the verdict (#533), ✓ real XMRig stress hashed for the bounded window (#533), ✓ stress restored the miner and timer states: xmrig=active rigforge-autotune.timer=inactive rigforge-watchdog.timer=active (#533), phase selftest: passed (3 assertions, 0 skipped). Real XMRig output therefore passes the coloured-verdict parse that was the risk.
  • Round-2 note (stale body: 38/38, 'No commit, push'): FIXED. The body now quotes 49 passed scoped to 03bcea4, and the no-write sentence is gone.
  • Round-2 note (one-line awk/jq, duplicated trap scaffolding): ANSWERED. The PR body's 'How I fixed it' now lists both as a known shortcut with an upgrade trigger. No code comment was added.
  • git cat-file -e a0744b6… → present. git log --no-merges a0744b6..b03f2a0 → worker commits 20e9da2, 80707ad, 0382a35, 75917b8; the rest came in through develop merges.
  • git show --remerge-diff 03bcea4/6208531/b03f2a0. The resolutions keep util/benchmark.sh and util/stress.sh and drop develop's inline _xmrig_bench. Develop's shared_host guard moved to util/benchmark.sh:317 (bench), with a new one at util/stress.sh:85. tune keeps its own guard at rigforge.sh:3072. grep _xmrig_bench → the only callers are bench() and _bench_once (tune), so both remain guarded. The coverage include-pattern now carries both util/randomx.sh and util/benchmark.sh,util/stress.sh.
  • gh pr checks 610 --json → 10/10 SUCCESS: zizmor, yamllint, gitleaks, markdownlint, shellcheck+shfmt, both stock-home setups, End-to-end (Docker), Coverage (kcov+diff-cover), Test suite.
  • git merge-base --is-ancestor origin/develop(154fe29) b03f2a0 → yes, up to date. The PR base is develop.
  • Local at b03f2a0, run directly: test-benchmark-verification → 50 passed, 0 failed. test-benchmark-support, test-stress, test-perf-submission, test-e2e-selftest, test-randomx-mode and test-coverage-cache → PASS (rc=0 each).
  • FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh → 'file budget OK … monotonic'.
  • Revert check in a git-archive scratch copy under $TMPDIR. Removing the bench+stress shared_host guards and the print-time/last-output hunks gives test-benchmark-verification rc=1 (49 passed, 1 failed) and test-stress rc=1 ('FAIL: failed stress did not print the last XMRig output'). Removing only the stress shared_host guard gives test-stress rc=1 ('FAIL: shared-host refusal was not reported').
  • git ls-tree b03f2a0 → rigforge.sh and tests/coverage.sh are 100755, matching develop. 75917b8 restored the bits 6208531 had dropped.
  • git diff origin/develop...b03f2a0 (non-doc) → no added explanatory comments. rigforge.sh only loses the moved _xmrig_bench and gains the source lines and tune candidate rejection.
  • detect-topology.py over the PR body+comments+reviews and the issue XMRig self-test modes are not surfaced: no stress verb after bios, and bench never checks the hash it computed (--seed/--hash/--submit/--verify) #533 body+comments → rc=0 for both, no hits.
  • Ponytail pass over the delta since a0744b6: the e2e-selftest phase (36 lines) and its stub test are the minimum hardware proof. Nothing new to cut beyond the shortcut already recorded.

Reviewed head: b03f2a03127331793a0fe65117d5dd034b6a9f39.

@pithead-reviewer

Copy link
Copy Markdown
Contributor

Owner brief for b03f2a031273

What it does. bench now only passes if XMRig's own RandomX hash check says the result is correct, not just fast. Offline tune throws out any candidate that fails the check. A new Linux stress --minutes N command (30 by default) runs a bounded offline load test after BIOS changes. It reports pass/fail and peak temperature, then restores the miner and timers. bench --submit and bench --verify ID are opt-in online checks. Bundles now carry benchmark diagnostics, and release baselines can carry a verifiable ID.

Why. Closes #533 (milestone v1.19.0). Before this, bench read only the H/s figure. Bad RAM or an aggressive undervolt could corrupt hashes without a crash, and tuning could pick such a candidate. Nothing checked stability after BIOS changes, and the perf baselines recorded numbers nobody could verify.

Risk. Moderate and contained to rigs. bench changes how XMRig is invoked and parsed, so a parse mismatch on some XMRig output would make a good bench fail, and tune would reject every candidate. stress stops the miner and both timers and restores them. A thermal trip deliberately leaves the miner stopped for the watchdog to resume. --submit publishes hardware and performance data, but only when the operator asks. Nothing irreversible found.

Proof. CI: 11 success. Reviewer: PASS at b03f2a0 by pithead-reviewer, round 2 (recorded 2026-11-09T10:50:40Z). Bench: job 523: not recorded on 80707ad, not the head; job 3540: success on 03bcea4, not the head; job 3672: success on the head.

What to check.

  • util/benchmark.sh, the hash-verdict parser (about line 159): it requires the green-colored verdict around the 16-hex hash. It is a one-line awk program, so read it against real XMRig output.
  • util/stress.sh: the stop/run/restore path, and that the thermal-trip case leaves the hold marker in place. The reviewer also noted tail at about line 169 prints temp-dir paths in failure output (harmless; tail -q would drop them).
  • rigforge.sh:2653-2657 in _measure: one failed iteration zeroes the candidate and breaks out, so confirm the tune search still finishes with a winner.
  • docs/pithead-integration.md:113-120: the diff duplicates the rigforge.appliance paragraph, which looks like a merge artifact.

Verdict. merge

@pithead-developer pithead-developer Bot added stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow and removed stage/in-review a reviewer session is on it now labels Nov 9, 2026
@pithead-reviewer
pithead-reviewer Bot merged commit 655f0af into develop Nov 9, 2026
11 checks passed
@pithead-reviewer
pithead-reviewer Bot deleted the astra/533-rigforge branch November 9, 2026 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XMRig self-test modes are not surfaced: no stress verb after bios, and bench never checks the hash it computed (--seed/--hash/--submit/--verify)

1 participant