Skip to content

Preserve optional XMRig pool fields - #607

Open
pithead-developer[bot] wants to merge 4 commits into
develop-v1.19.0from
astra/527-rigforge
Open

pithead-developer[bot] wants to merge 4 commits into
develop-v1.19.0from
astra/527-rigforge

Conversation

@pithead-developer

Copy link
Copy Markdown
Contributor

What's the problem

RigForge drops XMRig's rig-id, nicehash, algo, coin, and sni pool fields when generating the miner configuration. Operators cannot select these pool behaviors, and status/doctor do not identify a non-default live algorithm. Fixes #527.

How I fixed it

Preserve non-null values for all five fields, including explicit false, and validate them before generating or remotely applying configuration. Keep the existing user default and legacy output unchanged. Accept the pinned XMRig release's canonical RandomX algorithms and RandomX coin names/codes; reject unknown values and non-RandomX coins to maintain the issue's scope.

Show non-default live algorithms in status and doctor, document every supported pool field, and add an isolated regression suite through tests/run.sh. The control receiver already stages complete pool objects, so its existing shared validation path needs no separate schema change. Existing assertions and file-budget ceilings are unchanged.

Here is the proof

  • Before changing production code: bash tests/test-pool-fields.sh rigforge.sh tests/contract/v1/feed.json exited 1. All five fields disappeared from both generated pools and explicit false values disappeared; null/absent controls passed.
  • After the fix, the same command exited 0: pool schema fields: PASS. Coverage includes pool indexing, strict types and diagnostics, all accepted algorithms/coins, legacy null/absent behavior, live diagnostic output, receiver staging, remote application, and rejection without changing the stored configuration.
  • PATH="/private/tmp/rigforge-527-tools:$PATH" make test exited 0: 2,431 passed, 0 failed, followed by passing contract-credential, access-token-readiness, documentation clone-pin, and tool-pin-watch suites. This includes the control writable-key drift guard and wire-shape fixtures. The tool directory contains the repository's pinned shfmt 3.14.1. The first unpinned make test stopped on existing formatting differences under local shfmt 3.13.1. An intermediate pinned run recorded 2,430 passes and one failure in the draft compatibility-test harness; that harness was corrected before the clean passing run.
  • FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh passed.
  • shellcheck --severity=warning tests/test-pool-fields.sh and /private/tmp/rigforge-527-tools/shfmt -i 4 -d rigforge.sh tests/test-pool-fields.sh tests/run.sh passed.
  • jq empty config.reference.json and git diff --check passed.
  • npm_config_cache=/private/tmp/rigforge-527-npm-cache make lint-md failed only on three pre-existing formatting errors in the supplied, untracked ISSUE.md, which is unchanged. npm_config_cache=/private/tmp/rigforge-527-npm-cache npx --yes markdownlint-cli2@0.23.3 --no-globs docs/configuration.md docs/operations.md passed with zero issues.

The following exact command compared the entire generated legacy configuration against the original checkout's implementation. It exited 0 and printed default generated config baseline/current: PASS:

bash <<'BASH'
set -euo pipefail
repo=$PWD
d=$(mktemp -d)
trap 'rm -rf "$d"' EXIT
pool=$(jq -c '.rigforge.config.pools[0] | del(.pass)' tests/contract/v1/feed.json)
id=$(jq -r '.rigforge.control.change_id' tests/contract/v1/feed.json)
jq -n --argjson pool "$pool" '{pools:[$pool]}' >"$d/input.json"
mkdir -p "$d/logrotate"
render() {
    local code="$1" output="$2"
    rm -rf "$d/worker"
    (
        export RIGFORGE_HOME="$repo" LOGROTATE_DIR="$d/logrotate"
        eval "$code"
        CONFIG_JSON="$d/input.json" SCRIPT_DIR="$repo" OS_TYPE=Linux LOGROTATE_DIR="$d/logrotate"
        parse_config >/dev/null
        WORKER_ROOT="$d/worker" CPUINFO="$d/no-cpuinfo"
        mkdir -p "$WORKER_ROOT/xmrig/build"
        lscpu() { printf 'Model name: fixture cpu\n'; }
        hostname() { printf '%s\n' "$id"; }
        sudo() { "$@"; }
        cd "$WORKER_ROOT/xmrig/build"
        generate_xmrig_config >/dev/null
    )
    cp "$d/worker/xmrig/build/config.json" "$output"
}
baseline=$(git show HEAD:rigforge.sh | sed '/^if \[ "\$_RIGFORGE_SOURCED" = "0" \]; then/,$d')
current=$(sed '/^if \[ "\$_RIGFORGE_SOURCED" = "0" \]; then/,$d' rigforge.sh)
render "$baseline" "$d/baseline.json"
render "$current" "$d/current.json"
cmp "$d/baseline.json" "$d/current.json"
printf 'default generated config baseline/current: PASS\n'
BASH

The native macOS suite skipped Linux /etc idempotency and tuning benchmark timing cases. No Docker e2e, kcov coverage, KVM, bench, or hardware run was performed. Actual hashing for the selected variants and live NiceHash/TLS-SNI interoperability still need a rig and pool. The change does not alter kernel provisioning or KVM behavior. No commits, pushes, GitHub writes, SSH connections, or bench jobs were made.

Closes #527

Draft from an operator-run gpt-6-astra session (owner request, 2026-11-01). The operator reviews and gates it before it leaves draft.

🤖 Generated with Claude Code

Part of #527. 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

Copy link
Copy Markdown
Contributor Author

Operator: this v2.x enhancement stays a held draft until RigForge 1.18.1 is tagged (due Fri 2026-11-06, Sat 11-07 at the latest). 1.18.1 is cut from develop, so merging this first would widen that release. After the tag: rebase on develop, run the agent review, and request the owner's approval.

@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:06
pithead-developer[bot] and others added 2 commits November 7, 2026 01:07
@pithead-developer pithead-developer Bot added stage/in-progress stage/needs-check stage/in-review a reviewer session is on it now and removed stage/blocked its issue waits on another issue stage/in-progress stage/needs-check labels Nov 7, 2026
@pithead-reviewer

Copy link
Copy Markdown
Contributor

PASS

What I investigated

Reviewed PR #607 at head e2ea9bb against issue #527. The new pool-fields test is green on the head and red with rigforge.sh reverted, and the head's GitHub checks are all green. Both XMRig validators match v6.26.0's source. I did not run make test or make check locally, and no bench-ci job is quoted or required.

  • gh pr checks 607 --json: all 10 checks SUCCESS at the head (Test suite, Lint shellcheck+shfmt, markdownlint, yamllint, End-to-end (Docker), Coverage kcov+diff-cover, Stock home setup current and historical, zizmor, gitleaks). gh pr view headRefOid = e2ea9bb, same as the worktree HEAD.

  • git merge-base HEAD origin/develop-v1.19.0 = b1c25d6 = origin/develop-v1.19.0, so the branch is not behind its base. The PR is a draft, which is not a finding.

  • bash tests/test-pool-fields.sh rigforge.sh tests/contract/v1/feed.json on the head: exit 0, 63 'ok:' lines, 0 FAIL, final line 'pool schema fields: PASS'.

  • Same command in a scratch worktree with only rigforge.sh checked out from origin/develop-v1.19.0: rev_rc=1, 29 FAIL lines, 34 ok lines (the null, absent and legacy controls). The first 15 FAIL lines show the new fields rendering as [null,null,null,null,null], explicit false lost for nicehash and sni, the 'unknown pool field' warning firing, and invalid rig-id, nicehash and sni values accepted.

  • XMRig v6.26.0 Algorithm.cpp (fetched with gh api): kRX_0, kRX_V2 (rx/2), kRX_WOW, kRX_ARQ, kRX_GRAFT, kRX_SFX, kRX_YADA match the seven algo values the PR accepts. Coin.cpp: the RandomX coins monero/XMR, arqma/ARQ, graft/GRFT, wownero/WOW, zephyr/ZEPH, townforge, yadacoin/YDA match the PR's list. Coin.cpp parse uses strcasecmp, so the PR's case-insensitive coin check is right. The docs' pinned commit b2ca7248… equals XMRIG_COMMIT at rigforge.sh:81.

  • rigforge.sh:446 re-attaches the five fields only when non-null, so explicit false survives and null or absent leaves legacy output unchanged. The validators sit at rigforge.sh:476-491. known_pool at rigforge.sh:728 is extended.

  • rigforge.sh:5260 masks only pass and tls-fingerprint in the feed, so the new fields round-trip through the feed. util/control-server.py:21 still stages 'pools' opaquely, and the new test drives stage_change and _control_commit for both a valid and a rejected change.

  • git diff -U0 origin/develop-v1.19.0...HEAD: added comment lines are only the shebang and the shellcheck pragma in tests/test-pool-fields.sh. The explanatory comments inside the edited rigforge.sh hunks were removed, as the policy requires. VERSION and CHANGELOG.md are untouched.

  • The new test is registered in tests/run.sh and runs under the Test suite check, not sourced. rigforge.sh is 6296 lines against a ceiling of 6303 in docs/dev/file-budget.tsv. The Docker/kcov/e2e legs are read from the head's green checks, since this Mac runs no Docker.

  • detect-topology.py over the PR body plus comments and over the issue comments: 'clean' for both, exit 0.

  • PR body, 'Here is the proof': The body quotes local results (2,431 passed, the file-budget run) and a local-only tools path. The head's own checks cover the same ground and are green. Tidy the body to cite those checks.

  • rigforge.sh:5460 (doctor): The live-algorithm line is printed unmarked, without the ✓ or ! prefix that doctor's other findings carry. This is cosmetic and covered by the test, not a defect.

Reviewed head: e2ea9bb55bc52ab7918f487ebb821801fe688504.

@pithead-reviewer
pithead-reviewer Bot marked this pull request as ready for review November 7, 2026 07:49
@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 7, 2026
@pithead-reviewer

Copy link
Copy Markdown
Contributor

Owner brief for e2ea9bb55bc5

What it does. RigForge now passes five optional XMRig pool settings through to the generated miner config: rig-id, nicehash, algo, coin and sni. An explicit false is kept, and null or absent values are left out. Bad values are rejected with a clear message. status and doctor print the live algorithm when it isn't rx/0, and the docs and config.reference.json list every pool field.

Why. Closes #527, which is in the v2.x post-GA milestone and not a must-fix. The PR targets the develop-v1.19.0 branch. Before this, the pool mapper whitelisted only a few keys and dropped these five with an unknown-field warning. Operators couldn't use NiceHash pools, TLS name-based pools, pool-side rig IDs or RandomX variants.

Risk. Low. Configs that don't use the new fields should generate the same output as before; the PR body says it compared the full generated legacy config against the old code. The one behaviour change is that an algo, coin, nicehash, sni or rig-id value that used to be silently dropped now stops config parsing if it's invalid. That includes remote applies, which are rejected without touching the stored config. A rig with a stray non-RandomX algo in config.json would fail on its next apply. No real-rig hashing, NiceHash or TLS-SNI run was done.

Proof. CI: 11 success. Reviewer: PASS at e2ea9bb by pithead-reviewer, round 1 (recorded 2026-11-07T07:49:16Z). Bench: no job cited in the PR body.

What to check.

Verdict. merge

@pithead-developer pithead-developer Bot added stage/interrupted and removed stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow labels Nov 7, 2026
@pithead-reviewer

Copy link
Copy Markdown
Contributor

PASS

What I investigated

Round 2 at head 192d18e, a merge of develop-v1.19.0 into the PR branch. The PR's own diff against its base is unchanged since the round-1 PASS at e2ea9bb. The new pool-fields test is green at the head and red with rigforge.sh reverted, and my local full tests/run.sh run finished at the head with exit 0 and 2447 passed, 0 failed. The head's CI is green.

  • gh pr view 607 --json headRefOid: 192d18e; base develop-v1.19.0 = a985c6e = merge-base, so the branch is not behind its base. The PR is no longer a draft. Neither is a finding.

  • gh pr checks 607 --json: all 10 checks SUCCESS, none skipped (Test suite, Lint shellcheck+shfmt, markdownlint, yamllint, End-to-end (Docker), Coverage kcov+diff-cover, Stock home setup current and historical, zizmor, gitleaks). gh api actions/runs?head_sha=192d18e: 'CI' and 'Security' both completed/success on 192d18e.

  • git diff b1c25d6 e2ea9bb vs git diff a985c6e HEAD, with index lines stripped: the only differences are hunk line offsets and the base-owned 'shared_host' entry in the known-keys line. Same 6 files, +271/-43. config.reference.json, docs/configuration.md, docs/operations.md, rigforge.sh, tests/run.sh and tests/test-pool-fields.sh carry the same PR changes as at the round-1 head.

  • bash tests/test-pool-fields.sh rigforge.sh tests/contract/v1/feed.json at the head: exit 0, 63 ' ok:' lines, final line 'pool schema fields: PASS'.

  • Same command in a scratch worktree with rigforge.sh checked out from origin/develop-v1.19.0: exit 1, 29 FAIL lines. The scratch worktree is removed (git worktree list shows no revert-wt). The round-1 scratch worktree in the earlier session dir no longer exists, so there was nothing to remove.

  • bash tests/run.sh at the head, run locally to completion: run_rc=0, 0 failing checks, final line 'rigforge tests: 2447 passed, 0 failed'. The only 'FAIL' text in its output is in passing test names.

  • The PR body's legacy-config script, with git show HEAD:rigforge.sh replaced by git show a985c6e:rigforge.sh (the base): prints 'default generated config baseline/current: PASS', exit 0. A config with none of the new fields produces a byte-identical generated xmrig config.

  • gh api xmrig/xmrig contents at b2ca72480c58d197e18c885d9fc1a0c8d517e60a, the commit pinned at rigforge.sh:81: Algorithm.cpp has kRX_0, rx/2, rx/wow, rx/arq, rx/graft, rx/sfx and rx/yada, matching the seven algos the PR accepts. Coin.cpp lists XMR, ARQ, GRFT, WOW, ZEPH, Townforge and YDA as RandomX coins, matching the PR's coin list. SUMO and RVN are non-RandomX and are rejected. Coin.cpp parse uses strcasecmp, so the PR's case-insensitive coin check is right.

  • I read the rigforge.sh diff in full. The five fields are re-attached only when non-null, so an explicit false survives. rig-id, nicehash and sni are type-checked strictly, and algo and coin use allowlists. _read_api_summary always returns 0 and _print_pool_algorithm ends in || true, so the new doctor pipe is safe under set -Eeuo pipefail (rigforge.sh:15). util/control-server.py:21 stages 'pools' opaquely, with no per-pool key whitelist, and the test drives stage_change and _control_commit.

  • Comment discipline: the only comment lines the PR adds are the shebang and the shellcheck pragma in tests/test-pool-fields.sh:1-2. The explanatory comments in the edited rigforge.sh hunks are removed. Docs and config.reference.json keep their documentation text. VERSION and CHANGELOG.md are untouched.

  • python3 /Users/vijit/code/pithead-developer/private/detect-topology.py over the PR body plus comments: 'clean', exit 0. The same over issue 527's comments: 'clean', exit 0. Sanity check: the detector flags a planted private IP and path in a test file.

  • FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh: exit 0, 'file budget OK'. rigforge.sh is 6284 lines against a ceiling of 6303. make lint-topology: 'topology classes OK'.

  • No bench-ci job is quoted in the PR body, and none was run by me. This Mac runs no Docker, so the Docker e2e and kcov results come from the head's green GitHub checks. The PR body discloses that no real-rig hashing, NiceHash or TLS-SNI run was done.

  • PR body, 'Here is the proof': The body quotes local results (2,431 passed, a local-only tools path) rather than the head's own CI checks, and it predates the base merge. Tidy it to cite the green checks at 192d18e.

  • rigforge.sh:481-491: The algo allowlist is case-sensitive and accepts only the canonical names, so an XMRig alias like 'randomx/0' or 'RX/0' is rejected. The coin check is case-insensitive. This matches XMRig's canonical names, as documented in docs/configuration.md, and is stricter than XMRig. Not a defect.

  • rigforge.sh:5472 (doctor): The live-algorithm line prints without doctor's usual ✓ or ! marker. docs/operations.md files it as an advisory finding, and it is cosmetic and covered by tests.

  • PR base: The PR targets develop-v1.19.0, not develop. This is runner-managed and not a finding.

Reviewed head: 192d18ecb210c3f0ebbc159bda8cb60134e9ccac.

@pithead-reviewer

Copy link
Copy Markdown
Contributor

Owner brief for 192d18ecb210

What it does. RigForge now passes five optional XMRig pool settings through to the generated miner config: rig-id, nicehash, algo, coin and sni. Before, it silently dropped them. Each is checked before use, and an explicit false is kept. Status and doctor now print the live algorithm when it isn't rx/0. Docs and the reference config list every pool field.

Why. Closes #527, filed under the v2.x post-GA milestone. The PR targets the v1.19.0 branch. Before, operators couldn't use NiceHash pools, name-based TLS endpoints, a pinned RandomX variant or a pool-side rig label, and nicehash pools rejected every share.

Risk. Low. Configs that don't use the new fields generate the same output as before; the PR shows a full-file comparison against the old code. New validation could reject a config that previously loaded with a warning only if it already used these keys with odd values. algo and coin accept only RandomX values, so a pool needing a non-RandomX algorithm is refused. Mining with the chosen variants and NiceHash/SNI against real pools was not tested on hardware. Nothing irreversible.

Proof. CI: 11 success. Reviewer: PASS at 192d18e by pithead-reviewer, round 1 (recorded 2026-11-07T23:04:05Z). Bench: no job cited in the PR body.

What to check.

  • rigforge.sh:445-495: the new re-attach pass (nulls dropped, false kept) and the validators for rig-id, nicehash/sni, algo and coin.
  • rigforge.sh:481-494: the algo and coin allowlists match the documented XMRig v6.26.0 names. algo is case-sensitive and stricter than XMRig.
  • rigforge.sh:3961 and :5472: the new Algorithm line in status and doctor. It prints without doctor's usual marker, which is cosmetic.
  • The PR removes many explanatory comments in rigforge.sh (the project forbids added comments, but this deletes existing ones too). Confirm that was intended and no needed context was lost.

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 7, 2026
@pithead-developer pithead-developer Bot added stage/interrupted and removed stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow labels Nov 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pool mapper drops every XMRig pool key it does not know: rig-id, nicehash, algo/coin, sni

1 participant