diff --git a/CHANGELOG.md b/CHANGELOG.md index 03fd15e..7ec48a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,33 @@ All notable changes to RigForge are documented here. The format is based on ## [Unreleased] +## [1.16.0] - 2026-08-23 + +### Added + +- **Control API: a restart-free fast path for `watchdog_interval_min` and `max_temp_c` (#381, from + #344 item 1).** `control-apply` used to re-run the entire `apply` pipeline for every accepted + change — regenerate XMRig's config, re-render its unit, restart the service, then poll for a live + pool connection — even for a change that never touches XMRig at all. A change whose keys are + *only* `watchdog_interval_min` and/or `max_temp_c` now reconciles just the watchdog timer and + leaves XMRig running, closing the ~62s gap the original walkthrough measured for a single-key + change. The allowlist is closed and checked as a subset match (a key not on it, including any + future addition to the control-writable set, still takes the full path), and a fast-path failure + falls back to the same full-pipeline rollback a failed restart already uses. + +### Fixed + +- **A new `hugepages_pool_ceiling_mb` bounds the grow-only HugePages write to a declared ceiling + (#398).** `hugepages_reserve_extra_mb` adds declared co-resident headroom into the computed + requirement; the grow-only write's availability check gives no credit back for pages a *different* + consumer (a co-hosted Pithead stack) already holds from that same headroom, so the two combine + into a double count no value of `hugepages_reserve_extra_mb` can correct — traced to an ~12 GiB + pool request on an 8 GiB box (pithead#1103). `hugepages_pool_ceiling_mb` (default `0`, no + ceiling) caps the write itself instead: when declared, `vm.nr_hugepages` is never grown past the + ceiling regardless of how the requirement/availability arithmetic comes out — an odd declared MB + value floors to the 2MB page below rather than rounding up past it. Inert unless a caller sets + it — every existing config computes exactly as before. + ## [1.15.2] - 2026-08-21 ### Added diff --git a/VERSION b/VERSION index 42cf067..15b989e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.15.2 +1.16.0 diff --git a/config.reference.json b/config.reference.json index 818fa9f..3e96b08 100644 --- a/config.reference.json +++ b/config.reference.json @@ -1,5 +1,5 @@ { - "_docs": "Reference for every config.json key, each shown with its default value. Copy ONLY the keys you actually want to change into config.json; any key you omit keeps the default shown here. The pool target is the native XMRig 'pools' array: each pool needs a 'url' (host:port); every other field falls back to a Pithead-friendly default. A pool's 'user' is the rig's label on the dashboard (defaults to the machine hostname); list multiple entries for failover. An empty ACCESS_TOKEN (the default) leaves the rig's read-only HTTP API open (no token) — which matches Pithead's default no-auth stats probe; set a value only to require a Bearer token (then match it dashboard-side). 'autotune' is one of \"disabled\" (default — no scheduled tuning), \"performance\" (monthly tune for raw hashrate), or \"efficiency\" (monthly tune for hashrate-per-watt). 'watchdog' set to \"enabled\" installs a timer that health-checks the miner every 'watchdog_interval_min' minutes (default 5) and restarts it after two consecutive checks see 0 H/s or a dead API; 'max_temp_c' (empty = off) additionally stops the miner above that °C and starts it again 5°C below — check what your board's thermal_zone0 reports before setting it. 'miner_user' (empty = run as root, the default) runs the miner as that dedicated non-root system user — RigForge applies the MSR preset root-side; on CPU families without a known preset the ~10-15% MSR boost is skipped. 'api_allow_from' (empty = off) scopes the API port(s) — including the writable control port when 'control' is enabled — to one IPv4 or IPv6 address/CIDR source + loopback via an own nftables table (Linux, needs nft; for an IPv6 source the served port must bind a v6 address, so set 'api_bind'/'control_bind' to \"::\"). Set 'api' to enabled for the sister API: a second read-only port (default 8081) serving XMRig's summary enriched with RigForge tune/power/health/provenance data, gated by the same ACCESS_TOKEN; 'api_port'/'api_bind' place it. Set 'control' to \"enabled\" for the WRITABLE control path (#236): a SEPARATE authenticated port (default 8082, 'control_port'/'control_bind' place it) that lets a Pithead stack apply config changes through RigForge so config.json stays authoritative. Fail-closed: enabling it REQUIRES both ACCESS_TOKEN and api_allow_from (a writable API with no token or no pinned source is refused). Only pools, DONATION, autotune, watchdog(+interval), and max_temp_c are writable through it (and the REMOTE path refuses to disable watchdog or to unset/out-of-band max_temp_c — a rig's thermal protection can only be removed by a local rigforge.sh apply, #257); every change is validated, the old config is snapshotted to config-backups/, and a change that doesn't come back live is rolled back. Set 'control_upgrade' to \"enabled\" (a SECOND opt-in, only valid when 'control' is also enabled) to let the stack trigger this rig to upgrade its own RigForge to the latest release (#308, ADR 0002) — i.e. fetch and run new root code on a remote trigger. Default off, and never on just because 'control' is: the rig refuses any target that isn't a real release newer than the one running, verifies the release commit is reachable on main (the release branch), throttles repeat runs, and health-gates the swap with rollback. It rides the control path's own Bearer token + api_allow_from pin. Releases are checksummed, not signed — GitHub is the trust root (see SECURITY.md). For co-locating a miner on a busy box (e.g. a Pithead stack host), 'hugepages_reserve_extra_mb' (default 0) adds that many MB of HugePages to RigForge's computed mining reservation so the pool covers stack + miner — RigForge stays the sole writer of the reservation and, when the box's existing reservation already covers both, changes nothing and needs no reboot; 'threads' (empty/null = auto) caps the RandomX thread count (a ceiling — min(auto, threads), e.g. nproc-2) so the miner leaves spare cores, and it sizes the reservation to match. A TLS pool can pin the server certificate with 'tls-fingerprint' (its SHA-256 as 64 hex chars; null = no pin) — XMRig does no CA verification for stratum, so the pin is the only server authentication TLS stratum has. Full descriptions: https://github.com/p2pool-starter-stack/rigforge/blob/main/docs/configuration.md#configuration-reference", + "_docs": "Reference for every config.json key, each shown with its default value. Copy ONLY the keys you actually want to change into config.json; any key you omit keeps the default shown here. The pool target is the native XMRig 'pools' array: each pool needs a 'url' (host:port); every other field falls back to a Pithead-friendly default. A pool's 'user' is the rig's label on the dashboard (defaults to the machine hostname); list multiple entries for failover. An empty ACCESS_TOKEN (the default) leaves the rig's read-only HTTP API open (no token) — which matches Pithead's default no-auth stats probe; set a value only to require a Bearer token (then match it dashboard-side). 'autotune' is one of \"disabled\" (default — no scheduled tuning), \"performance\" (monthly tune for raw hashrate), or \"efficiency\" (monthly tune for hashrate-per-watt). 'watchdog' set to \"enabled\" installs a timer that health-checks the miner every 'watchdog_interval_min' minutes (default 5) and restarts it after two consecutive checks see 0 H/s or a dead API; 'max_temp_c' (empty = off) additionally stops the miner above that °C and starts it again 5°C below — check what your board's thermal_zone0 reports before setting it. 'miner_user' (empty = run as root, the default) runs the miner as that dedicated non-root system user — RigForge applies the MSR preset root-side; on CPU families without a known preset the ~10-15% MSR boost is skipped. 'api_allow_from' (empty = off) scopes the API port(s) — including the writable control port when 'control' is enabled — to one IPv4 or IPv6 address/CIDR source + loopback via an own nftables table (Linux, needs nft; for an IPv6 source the served port must bind a v6 address, so set 'api_bind'/'control_bind' to \"::\"). Set 'api' to enabled for the sister API: a second read-only port (default 8081) serving XMRig's summary enriched with RigForge tune/power/health/provenance data, gated by the same ACCESS_TOKEN; 'api_port'/'api_bind' place it. Set 'control' to \"enabled\" for the WRITABLE control path (#236): a SEPARATE authenticated port (default 8082, 'control_port'/'control_bind' place it) that lets a Pithead stack apply config changes through RigForge so config.json stays authoritative. Fail-closed: enabling it REQUIRES both ACCESS_TOKEN and api_allow_from (a writable API with no token or no pinned source is refused). Only pools, DONATION, autotune, watchdog(+interval), and max_temp_c are writable through it (and the REMOTE path refuses to disable watchdog or to unset/out-of-band max_temp_c — a rig's thermal protection can only be removed by a local rigforge.sh apply, #257); every change is validated, the old config is snapshotted to config-backups/, and a change that doesn't come back live is rolled back. Set 'control_upgrade' to \"enabled\" (a SECOND opt-in, only valid when 'control' is also enabled) to let the stack trigger this rig to upgrade its own RigForge to the latest release (#308, ADR 0002) — i.e. fetch and run new root code on a remote trigger. Default off, and never on just because 'control' is: the rig refuses any target that isn't a real release newer than the one running, verifies the release commit is reachable on main (the release branch), throttles repeat runs, and health-gates the swap with rollback. It rides the control path's own Bearer token + api_allow_from pin. Releases are checksummed, not signed — GitHub is the trust root (see SECURITY.md). For co-locating a miner on a busy box (e.g. a Pithead stack host), 'hugepages_reserve_extra_mb' (default 0) adds that many MB of HugePages to RigForge's computed mining reservation so the pool covers stack + miner — RigForge stays the sole writer of the reservation and, when the box's existing reservation already covers both, changes nothing and needs no reboot; 'hugepages_pool_ceiling_mb' (default 0 = no ceiling) is a HARD CAP on the runtime pool RigForge will grow to, in MB, distinct from hugepages_reserve_extra_mb (which adds declared headroom into the computed requirement rather than bounding the write) — set it on a RAM-constrained co-resident box so the grow-only write can never exceed the box's honest capacity no matter what the requirement/availability math computes (rigforge#398; an odd MB value floors to the 2MB page below, e.g. 5121 -> 5120MB effective, never rounds up past the declared ceiling); 'threads' (empty/null = auto) caps the RandomX thread count (a ceiling — min(auto, threads), e.g. nproc-2) so the miner leaves spare cores, and it sizes the reservation to match. A TLS pool can pin the server certificate with 'tls-fingerprint' (its SHA-256 as 64 hex chars; null = no pin) — XMRig does no CA verification for stratum, so the pin is the only server authentication TLS stratum has. Full descriptions: https://github.com/p2pool-starter-stack/rigforge/blob/main/docs/configuration.md#configuration-reference", "pools": [ { "url": ":3333", "user": "", "pass": "x", "keepalive": true, "tls": false, "tls-fingerprint": null, "enabled": true } @@ -26,6 +26,7 @@ "add_to_path": false, "hugepages_reserve_extra_mb": 0, + "hugepages_pool_ceiling_mb": 0, "threads": null, "HOME_DIR": "DYNAMIC_HOME" diff --git a/docs/adr/0001-writable-worker-config-control-path.md b/docs/adr/0001-writable-worker-config-control-path.md index 25d8296..5331174 100644 --- a/docs/adr/0001-writable-worker-config-control-path.md +++ b/docs/adr/0001-writable-worker-config-control-path.md @@ -96,6 +96,12 @@ D7 stamps `source: "control"` on a control apply. #254 extends the provenance to The accepted→poll contract (D2) has a race: a concurrent change between a caller's `POST` and its poll makes the no-arg `GET /status` report the *newer* change, so the caller can't confirm its own. #255 adds `GET /status?change_id=<16hex>` returning that change's recorded outcome (or `404`) — the applier already writes each outcome, so it additionally indexes them under `changes/.json` (last ~20; the id is server-generated 16-hex and re-validated before it becomes a path component). The no-arg form stays most-recent for compatibility; auth is unchanged. Chosen over a `/changes` ring-buffer endpoint (issue Option B) as the smaller, direct change. +### D12. A restart-free fast path for a closed subset of the allowlist (#381, from #344 item 1) + +D6's `apply` re-run is correct as a baseline but expensive: a live walkthrough measured a single `watchdog_interval_min` change taking ~62s round-trip through `POST /apply`, because the applier re-runs the *entire* `apply` pipeline — regenerate XMRig's config, re-render its unit, restart the service, then poll for a live pool connection — for every change, even one that never touches XMRig at all. + +The #344 discussion set one constraint before this could land: the fast path must not fork a second apply implementation that can drift from the real one. Decision: a closed allowlist (`watchdog_interval_min`, `max_temp_c`), checked as a **subset** match — every changed key must be on it, never a "not otherwise restart-requiring" complement — so a future addition to the D3 allowlist that nobody has re-proven restart-free here takes the full path by construction. Both current members are proven restart-free from the applier's own code, not asserted: `watchdog_interval_min` bakes into *only* `rigforge-watchdog.timer`'s cadence, and `max_temp_c` is never rendered into a unit at all — the watchdog verb re-reads `config.json` on every scheduled run. Neither reaches XMRig's generated config or its unit template. The fast path reuses `install_watchdog` — the same call the full `apply` pipeline already makes on every run — rather than re-implementing unit rendering, so the two paths cannot drift on what "restart-free" renders; it still stamps `config_meta` provenance (D10) the same way `apply` does, so a consumer cannot tell which path served a change from the feed alone. Success is a *run-state* comparison, not a bare is-active snapshot: the applier records whether the miner service was active *before* the fast apply and again *after*, and only a transition from active to inactive counts as failure. A rig can be legitimately stopped when a restart-free change lands — a watchdog thermal hold (D8/#257), or an operator's manual stop — and since the fast path never touches the XMRig unit or service, a stopped rig staying stopped (or even coming back on its own) is not this change's doing and must not be read as a fast-path failure; the new value still takes effect on the watchdog's next scheduled tick. An adversarial review of the first version of this decision found it gated on is-active alone, which would have discarded the operator's change and force-restarted a rig that was deliberately offline — exactly the thermal-hold-plus-`max_temp_c`-edit case this fast path exists for. A failure (active before, inactive after) falls through to the *same* full-pipeline rollback D6 already defines, never a bespoke recovery path. `pools`, `DONATION`, `autotune`, and the `watchdog` enable/disable flag stay on the full path: the first two are XMRig's own served config, and the latter two touch `install_*` behaviour this issue did not audit for restart-freedom. + ## Alternatives considered - **Write verbs on the sister API.** Rejected: violates the read-only invariant, and the `DynamicUser` read process cannot persist or apply. diff --git a/docs/configuration.md b/docs/configuration.md index b021d3a..0770f2b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -55,7 +55,7 @@ not read by `parse_config` today, so it isn't in the table below.) | `api` | `"disabled"` | `"enabled"` serves the sister API: a second **read-only** port with XMRig's `/1/summary`+`/2/summary` passed through verbatim plus a namespaced `rigforge` object (tune state, RAPL watts, firmware/health probes, pinned versions), and `/health` + `/tune` endpoints. One tiny persistent stdlib server; a systemd timer refreshes its data every 15 s, so requests never touch the miner (see [operations › sister API](operations.md)). Gated by the same `ACCESS_TOKEN`; Linux-only. | | `api_port` | `8081` | Sister API port (8080 is rejected — that's XMRig's own API). | | `api_bind` | `"0.0.0.0"` | Sister API listen address. | -| `control` | `"disabled"` | `"enabled"` serves the **writable** control path (#236): a *separate* authenticated port that lets a Pithead stack apply config changes through RigForge, so `config.json` stays the source of truth (the producer for pithead Worker Inspect). **Fail-closed:** enabling it requires *both* `ACCESS_TOKEN` and `api_allow_from` — a writable API with no token or no pinned source is refused with a hard error. Only `pools`, `DONATION`, `autotune`, `watchdog`(+`watchdog_interval_min`), and `max_temp_c` are writable through it; anything else is rejected. The **remote** path additionally refuses to disable `watchdog` or to unset / out-of-band `max_temp_c` — a rig's thermal protection can only be *removed* by a local `rigforge.sh apply` on the box (#257). Each change is validated, the old config is snapshotted to `config-backups/` first, and a change that doesn't come back live is rolled back. The receiver holds no privilege and stages off the request path, so writes never touch mining. Linux-only. See [Operations › Control path](operations.md#writable-control-path-opt-in). | +| `control` | `"disabled"` | `"enabled"` serves the **writable** control path (#236): a *separate* authenticated port that lets a Pithead stack apply config changes through RigForge, so `config.json` stays the source of truth (the producer for pithead Worker Inspect). **Fail-closed:** enabling it requires *both* `ACCESS_TOKEN` and `api_allow_from` — a writable API with no token or no pinned source is refused with a hard error. Only `pools`, `DONATION`, `autotune`, `watchdog`(+`watchdog_interval_min`), and `max_temp_c` are writable through it; anything else is rejected. A change touching only `watchdog_interval_min` and/or `max_temp_c` applies without restarting XMRig (#381) — every other key restarts it. The **remote** path additionally refuses to disable `watchdog` or to unset / out-of-band `max_temp_c` — a rig's thermal protection can only be *removed* by a local `rigforge.sh apply` on the box (#257). Each change is validated, the old config is snapshotted to `config-backups/` first, and a change that doesn't come back live is rolled back. The receiver holds no privilege and stages off the request path, so writes never touch mining. Linux-only. See [Operations › Control path](operations.md#writable-control-path-opt-in). | | `control_port` | `8082` | Control path port (rejects 8080 and the `api_port`). | | `control_bind` | `"0.0.0.0"` | Control path listen address. Pair with `api_allow_from` (required) to pin who may write. | | `miner_user` | `""` *(root)* | Run the miner as this dedicated non-root system user (created at setup, nologin). RigForge applies the CPU's MSR preset root-side before start; on families without a known preset the ~10-15% MSR boost is skipped — which is why this ships opt-in. Lowering privilege changes nothing else: HugePages come from the boot reservation, tune/doctor/apply all keep working. | diff --git a/docs/operations.md b/docs/operations.md index dec6e4c..734156c 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -590,10 +590,16 @@ How a change flows: 2. A path-triggered root oneshot picks up the staged change, snapshots the current `config.json` to `config-backups/config-.json`, merges only the allowlisted keys, and re-validates the result with the same rules `apply` uses. An invalid change is rejected and **nothing is written**. -3. A valid change is written durably (temp file, `fsync`, atomic rename) and applied through the - normal `apply` path. If the miner does not come back to a live hashrate, the snapshot is restored - and re-applied, and the outcome is recorded as `rolled_back` (or `failed`, if the rollback snapshot - itself could not be read back). +3. A valid change is written durably (temp file, `fsync`, atomic rename) and applied. Changing only + `watchdog_interval_min` and/or `max_temp_c` takes a **restart-free fast path** (#381): neither key + ever reaches XMRig's generated config or its unit, so only the watchdog timer is reconciled and + XMRig itself is left running — round-trip in about a second instead of the ~60s a full restart + costs on a big-page host. Any other key (alone or mixed with those two) applies through the normal + `apply` path, which restarts XMRig. Either way, if the miner does not come back to a live hashrate + (full path) or the miner service is not found still running (fast path — a signal something else + was already wrong, since neither key can cause that), the snapshot is restored and re-applied + through the full path, and the outcome is recorded as `rolled_back` (or `failed`, if the rollback + snapshot itself could not be read back). 4. `GET :8082/status` returns the last change's outcome (`applied` / `rejected` / `rolled_back` / `failed`, with `source: "control"`, the changed keys, the backup path, and a `warnings[]` for any change that touched thermal protection). Every response also carries a derived `age_seconds` next to its own diff --git a/docs/pithead-integration.md b/docs/pithead-integration.md index b893a12..c7258c4 100644 --- a/docs/pithead-integration.md +++ b/docs/pithead-integration.md @@ -191,7 +191,11 @@ apply config changes through RigForge — the RigForge-side producer for pithead (pithead #185). It is deliberately independent of the read API: a `POST :8082/apply` of an allowlisted change (`pools`, `DONATION`, `autotune`, `watchdog`, `watchdog_interval_min`, `max_temp_c`) returns `202 Accepted`; RigForge validates, snapshots the old config, applies it, and -rolls back anything that doesn't come back live. The stack reads the new effective config back from +rolls back anything that doesn't come back live. Changing only `watchdog_interval_min` and/or +`max_temp_c` never restarts XMRig (#381) — those two are proven not to reach XMRig's config or unit, +so RigForge reconciles just the watchdog timer instead of running the full apply pipeline, landing in +about a second instead of the ~60s a restart costs. Any other key, alone or mixed with those two, +takes the full, XMRig-restarting path. The stack reads the new effective config back from `:8081/2/summary` and polls `:8082/status` for the outcome. The write path is pinned to the stack host by `api_allow_from` (mandatory) — the miner never accepts a config from anywhere else. Full mechanics and the security model: [Operations › Writable control path](operations.md#writable-control-path-opt-in) diff --git a/rigforge.sh b/rigforge.sh index bbb3b43..e6de5cd 100755 --- a/rigforge.sh +++ b/rigforge.sh @@ -575,6 +575,21 @@ parse_config() { if ! [[ "$HUGEPAGES_RESERVE_EXTRA_MB" =~ ^[0-9]+$ ]] || [ "$HUGEPAGES_RESERVE_EXTRA_MB" -gt 65536 ]; then error "hugepages_reserve_extra_mb must be a whole number of MB, 0-65536 (got: $HUGEPAGES_RESERVE_EXTRA_MB)." fi + # Total-pool ceiling (#398): a HARD CAP on the runtime pool RigForge will grow to, expressed in MB — + # unlike hugepages_reserve_extra_mb (which ADDS declared headroom into the computed requirement), + # this bounds the WRITE itself. It exists because no value of hugepages_reserve_extra_mb can fix a + # double-count between that requirement and the grow-only write's availability check (rigforge#398): + # a co-resident consumer's already-held pages land inside the requirement (as declared headroom) and + # are simultaneously excluded from "available," so the shortfall the grow-only write computes counts + # the same pages twice. A ceiling stops the WRITE from ever exceeding the box's declared honest + # capacity, regardless of how that upstream arithmetic comes out. Default 0 = no ceiling (today's + # behavior, unchanged) — only a caller that declares this key opts into the cap. An odd MB value + # FLOORS to the 2MB page below (e.g. 5121 -> 2560 pages, 5120MB effective) — a cap must round + # toward less memory, never more. + HUGEPAGES_POOL_CEILING_MB=$(jq -r '.hugepages_pool_ceiling_mb // 0' "$CONFIG_JSON") + if ! [[ "$HUGEPAGES_POOL_CEILING_MB" =~ ^[0-9]+$ ]] || [ "$HUGEPAGES_POOL_CEILING_MB" -gt 65536 ]; then + error "hugepages_pool_ceiling_mb must be a whole number of MB, 0-65536 (got: $HUGEPAGES_POOL_CEILING_MB)." + fi # First-class thread cap (#305): a CEILING on the RandomX thread count — min(auto-detected, threads). # Empty (default) = let XMRig/L3 sizing pick. A co-located miner sets this (e.g. nproc-2) to leave # the stack cores free. Sizing and the generated cpu.rx both honour it; it never raises the count. @@ -673,7 +688,7 @@ parse_config() { # `_` are the comment convention (config.reference.json's own _docs); RIG_NAME is reserved for the # #1 image seed. Warn NAMES only, never values — a fat-fingered token must not land in a log. _warn_unknown_config_keys() { - local known="pools ACCESS_TOKEN DONATION autotune add_to_path HOME_DIR api api_port api_bind api_allow_from miner_user RIG_NAME watchdog watchdog_interval_min max_temp_c control control_port control_bind control_upgrade hugepages_reserve_extra_mb threads" + local known="pools ACCESS_TOKEN DONATION autotune add_to_path HOME_DIR api api_port api_bind api_allow_from miner_user RIG_NAME watchdog watchdog_interval_min max_temp_c control control_port control_bind control_upgrade hugepages_reserve_extra_mb hugepages_pool_ceiling_mb threads" local known_pool="url user pass keepalive tls enabled tls-fingerprint" local k lk m lm hit hint unknown_seen=0 while IFS= read -r k; do @@ -1482,8 +1497,10 @@ _miner_held_hugepages() { # now, or already held by the running miner) and grow the pool by the shortfall only; never shrink # a reservation someone else made. Idempotent: a re-run that finds enough available writes nothing. # ponytail: with headroom configured AND the co-resident already running, its live pages and the -# headroom both count toward the target — a bounded over-reserve in the safe direction; tighten to -# max(headroom, live use) if a real box ever needs the difference. +# headroom both count toward the target — a bounded over-reserve in the safe direction, UNLESS the +# box is too small to absorb it (rigforge#398 / pithead#1103: ~12 GiB requested on an 8 GiB box). +# hugepages_pool_ceiling_mb (see _ensure_hugepages) is the bound for a box that needs one; tighten +# this function itself to max(headroom, live use) only if a real box needs the difference instead. _hugepages_avail() { # -> 2MB pages the miner could draw on right now (free + already held) local free held free=$(awk '/^HugePages_Free:/ { print $2; exit }' "$MEMINFO" 2>/dev/null) || free="" @@ -1492,14 +1509,35 @@ _hugepages_avail() { # -> 2MB pages the miner could draw on right now (free + al } _ensure_hugepages() { # - local required=$1 current avail + local required=$1 current avail target ceiling_pages current=$(cat "$NR_HUGEPAGES_FILE" 2>/dev/null) || current=0 avail=$(_hugepages_avail) - if [ "$avail" -lt "$required" ]; then - sudo sysctl -w vm.nr_hugepages=$((current + required - avail)) - else + if [ "$avail" -ge "$required" ]; then log "HugePages pool already covers the miner ($avail of $required pages available; pool: $current) — leaving it as-is (#328)." + return + fi + target=$((current + required - avail)) + # Total-pool ceiling (#398): hugepages_pool_ceiling_mb caps the WRITE itself rather than feeding + # more headroom into the requirement above — the requirement/availability arithmetic can double- + # count a co-resident consumer's already-held pages (rigforge#398), and no declared value of + # hugepages_reserve_extra_mb can fix that from the requirement side. Absent (0, the default) this + # whole block is skipped and the write below is byte-for-byte the pre-#398 arithmetic (#328). + # FLOORED to whole 2MB pages (not rounded up like EXTRA_2MB_PAGES elsewhere): a cap must round + # toward less memory, never more — an odd declared MB value (e.g. 5121) floors to the page below + # (2560, 5120MB) rather than overshooting the declared ceiling by a page (security review finding + # on the first version of this fix: ceil(MB/2) let an odd ceiling write one page past itself). + if [ "${HUGEPAGES_POOL_CEILING_MB:-0}" -gt 0 ] 2>/dev/null; then + ceiling_pages=$((HUGEPAGES_POOL_CEILING_MB / 2)) + if [ "$target" -gt "$ceiling_pages" ]; then + if [ "$ceiling_pages" -le "$current" ]; then + warn "HugePages pool is already at its declared ceiling ($current pages, ${HUGEPAGES_POOL_CEILING_MB}MB) — the computed requirement ($target pages) exceeds it, so the pool is left as-is; the miner may run without a full HugePages share (#398)." + return + fi + log "HugePages requirement ($target pages) exceeds the declared pool ceiling — capping the write at $ceiling_pages pages (${HUGEPAGES_POOL_CEILING_MB}MB) instead of growing further (#398)." + target=$ceiling_pages + fi fi + sudo sysctl -w vm.nr_hugepages=$target } tune_kernel() { @@ -4134,6 +4172,89 @@ _control_do_apply() { _wait_miner_live "${CONTROL_LIVE_TRIES:-20}" } +# #381 (from #344 item 1): a live walkthrough measured a single restart-free key +# (watchdog_interval_min) taking ~62s round-trip through POST /apply, because _control_do_apply +# above re-runs the ENTIRE apply() pipeline — regenerate xmrig's config, re-render its unit, restart +# the service, then wait out _wait_miner_live's pool-liveness retries — for every change, even one +# that never touches xmrig at all. +# +# CONTROL_FAST_PATH_KEYS is a CLOSED allowlist, checked as a SUBSET match (never a "not on the slow +# list" complement) by _control_fast_path_eligible below: a changed-keys set takes the fast path only +# when EVERY key in it is in this list, so an unrecognised key — including any future addition to +# CONTROL_WRITABLE_KEYS that nobody has re-proven restart-free here — falls through to the full, +# restart-safe path by construction. Evidence per key (grep rigforge.sh for both to re-check): +# +# watchdog_interval_min — bakes into ONLY rigforge-watchdog.timer's OnUnitActiveSec, rendered by +# install_watchdog. It never reaches generate_xmrig_config or the xmrig unit template, so xmrig +# itself has nothing to reload; install_watchdog (the SAME function the full path already calls +# unconditionally on every apply, see apply() above) is sufficient — it re-renders the timer and +# daemon-reloads it. +# max_temp_c — never rendered into ANY unit. install_watchdog's own comment records why: "only the +# cadence is baked into the units — the verb re-reads config.json every run, so an apply after a +# max_temp_c ... edit needs no unit rewrite". The watchdog verb picks it up on its next scheduled +# run; nothing beyond the config.json write _control_commit already did is needed here. +# +# pools/DONATION change xmrig's OWN generated config — restarting it is the only way xmrig serves the +# new values — and autotune/watchdog (the enable/disable flag, not the interval) govern install_* +# paths this issue has not audited for restart-freedom. All four stay on the full path. Never widen +# this list without the same kind of evidence trail (a grep proving the key never reaches +# generate_xmrig_config or a unit template); the closed-set-subset design means an unaudited key's +# safe default is already "restart", not "skip". +CONTROL_FAST_PATH_KEYS="watchdog_interval_min max_temp_c" + +# True (rc 0) iff is non-empty and every key in it is in CONTROL_FAST_PATH_KEYS. A single +# key outside the list — or an empty/malformed keys-csv (e.g. control_apply's "?" sentinel when jq +# couldn't read the staged file's keys) — fails CLOSED to "not eligible", so the caller takes the +# full apply() path. Subset match, never complement: this can only be talked into "eligible" by a key +# that is actually named on the allowlist above, so the allowlist growing is the only way this +# function's answer changes for a given input. +_control_fast_path_eligible() { # + local keys="$1" k found=0 + [ -n "$keys" ] || return 1 + local IFS=',' + for k in $keys; do + [ -n "$k" ] || continue + found=1 + case " $CONTROL_FAST_PATH_KEYS " in + *" $k "*) ;; + *) return 1 ;; + esac + done + [ "$found" -eq 1 ] +} + +# The restart-free counterpart to _control_do_apply (#381). Only reached when +# _control_fast_path_eligible says every changed key is provably restart-free (see +# CONTROL_FAST_PATH_KEYS above), so this DELIBERATELY skips apply()/_apply_runtime entirely: no +# generate_xmrig_config, no xmrig unit re-render, no `systemctl restart` of the miner, no +# _wait_miner_live retry loop, no _apply_pool_check. It reuses install_watchdog verbatim — the same +# call the full apply() makes unconditionally on every run — rather than re-implementing unit +# rendering, so the fast and full paths cannot drift on what "restart-free" actually renders. It +# still stamps provenance exactly like apply() does, via the same RIGFORGE_CONFIG_SOURCE / +# RIGFORGE_CONFIG_CHANGE_ID dynamic-scope contract, so config_meta on the read feed does not depend on +# which path served the change. +# +# Success is "the run-state did not DEGRADE", not "is currently active" (security review finding on +# the original version of this function, which gated on is-active alone): a rig can be LEGITIMATELY +# stopped before this change lands — a watchdog thermal hold, or an operator's manual stop — and by +# construction the fast path never touches the xmrig unit or service, so a stopped rig staying +# stopped (or even coming back up on its own) is correct, not a failure; the new value takes effect +# on its own schedule (the watchdog's next tick). Only a transition from active to inactive is a real +# regression worth the caller's rollback. No thermal-hold-marker special-casing needed: the +# before/after comparison covers every "was already down" case with less machinery. +_control_do_apply_fast() { + local was_active is_active + was_active=1 + systemctl is-active --quiet "$SERVICE_NAME" 2>/dev/null && was_active=0 + parse_config + install_watchdog >/dev/null 2>&1 || true + _stamp_config_meta "${RIGFORGE_CONFIG_SOURCE:-local}" "${RIGFORGE_CONFIG_CHANGE_ID:-}" + is_active=1 + systemctl is-active --quiet "$SERVICE_NAME" 2>/dev/null && is_active=0 + [ "$was_active" -eq 1 ] && return 0 + [ "$is_active" -eq 0 ] +} + # Record a status record for the receiver's GET /status (mode 644 so the DynamicUser server reads it # back) — a terminal outcome, or control_upgrade's non-terminal `started` marker (#320). _control_status() { # @@ -4198,13 +4319,29 @@ control_apply() { fi backup="${result#committed }" _reown_config_backups "$backups" - log "control-apply: committed change $cid (keys: $change_keys); applying..." # #254: attribute this (and the rollback re-apply) to the control path with its change_id — the # nested apply()'s _stamp_config_meta reads these via dynamic scope. local RIGFORGE_CONFIG_SOURCE=control RIGFORGE_CONFIG_CHANGE_ID="$cid" - if _control_do_apply; then + # #381: dispatch on the closed fast-path allowlist. Only the CLASSIFICATION differs between the + # two branches below — a success writes the same "applied" status either way, and a failure of + # EITHER path falls through to the same full-pipeline rollback, so a wrong "eligible" verdict (or + # the fast path failing for an unrelated reason) still ends up restart-safe, never silently stuck. + local fast=0 apply_ok=0 + if _control_fast_path_eligible "$change_keys"; then + fast=1 + log "control-apply: committed change $cid (keys: $change_keys); applying (fast path — xmrig untouched)..." + _control_do_apply_fast && apply_ok=1 + else + log "control-apply: committed change $cid (keys: $change_keys); applying..." + _control_do_apply && apply_ok=1 + fi + if [ "$apply_ok" -eq 1 ]; then _control_status "$status" applied "$cid" "$change_keys" "" "$backup" - log "control-apply: change $cid applied." + if [ "$fast" -eq 1 ]; then + log "control-apply: change $cid applied (fast path)." + else + log "control-apply: change $cid applied." + fi else warn "control-apply: change $cid did not come back live — rolling back to $backup." # #276: the backup must be readable to restore it — guard the cp explicitly (not just -e/ERR) diff --git a/tests/perf-baselines/miner-3.history.jsonl b/tests/perf-baselines/miner-3.history.jsonl index d1a8661..7ecf76d 100644 --- a/tests/perf-baselines/miner-3.history.jsonl +++ b/tests/perf-baselines/miner-3.history.jsonl @@ -8,3 +8,4 @@ {"tag":"v1.10.0","recorded":"2026-07-17","bench_1m_hs":36760.7} {"tag":"v1.11.1","recorded":"2026-07-18","bench_1m_hs":36847.3} {"tag":"v1.15.1","recorded":"2026-08-15","bench_1m_hs":36850.1} +{"tag":"v1.15.2","recorded":"2026-08-21","bench_1m_hs":36828.3} diff --git a/tests/perf-baselines/miner-3.json b/tests/perf-baselines/miner-3.json index c10d7bb..8afe72e 100644 --- a/tests/perf-baselines/miner-3.json +++ b/tests/perf-baselines/miner-3.json @@ -1,5 +1,5 @@ { - "bench_1m_hs": 36850.1, + "bench_1m_hs": 36828.3, "cpu": "AMD EPYC 7642 48-Core Processor", - "recorded": "2026-08-15" + "recorded": "2026-08-21" } diff --git a/tests/run.sh b/tests/run.sh index 22fb1f4..4d0de99 100644 --- a/tests/run.sh +++ b/tests/run.sh @@ -1034,6 +1034,31 @@ out="$( )" assert_contains "threads 0 rejected (min 1)" "$out" "threads must be" +# parse_config: hugepages_pool_ceiling_mb (#398) — parses, defaults to 0 (no ceiling), rejects bad +# values the same way hugepages_reserve_extra_mb does. +echo "== unit: parse_config hugepages_pool_ceiling_mb (#398) ==" +c398="$SANDBOX/c398.json" +printf '{"pools":[{"url":"h:3333"}],"hugepages_pool_ceiling_mb":5120}\n' >"$c398" +assert_eq "hugepages_pool_ceiling_mb parsed" "$(parse_and_print "$c398" "$ROOT" HUGEPAGES_POOL_CEILING_MB)" "5120" +printf '{"pools":[{"url":"h:3333"}]}\n' >"$c398" +assert_eq "hugepages_pool_ceiling_mb defaults to 0 (no ceiling, #398)" "$(parse_and_print "$c398" "$ROOT" HUGEPAGES_POOL_CEILING_MB)" "0" +printf '{"pools":[{"url":"h:3333"}],"hugepages_pool_ceiling_mb":-5}\n' >"$c398" +out="$( + source "$SCRIPT" + CONFIG_JSON="$c398" + SCRIPT_DIR="$ROOT" + PATH="$STUBS:$PATH" parse_config 2>&1 +)" +assert_contains "negative ceiling rejected (#398)" "$out" "hugepages_pool_ceiling_mb must be" +printf '{"pools":[{"url":"h:3333"}],"hugepages_pool_ceiling_mb":99999999}\n' >"$c398" +out="$( + source "$SCRIPT" + CONFIG_JSON="$c398" + SCRIPT_DIR="$ROOT" + PATH="$STUBS:$PATH" parse_config 2>&1 +)" +assert_contains "oversized ceiling rejected (#398)" "$out" "hugepages_pool_ceiling_mb must be" + # generate_xmrig_config: the thread cap clamps cpu.rx AFTER any tune overlay (a stale tuned count can't # exceed the operator's headroom); a valid count at/under the cap is left alone. echo "== config-gen: threads cap clamps cpu.rx (#305) ==" @@ -5621,6 +5646,96 @@ SC="$HP/calls6" out="$(run_tk328_nopg "$SC")" assert_contains "fallback (no proposed-grub.sh) is grow-only too (#328)" "$(cat "$SC")" "vm.nr_hugepages=3072" +# --------------------------------------------------------------------------- +# #398: hugepages_pool_ceiling_mb bounds the grow-only write to a declared ceiling instead of +# letting a co-resident stack's declared headroom double-count into the write (rigforge#398). The +# fixture below is the worked example from rigforge#398 / pithead#1103: an 8 GB reduced-tier +# appliance box with NUMA_NODES=1, THREADS=4, hugepages_reserve_extra_mb=5120 (the tier's own 5 GiB +# reservation, declared as the co-located miner's headroom), a pool already at 2560 pages (5 GiB) +# of which the stack holds ~2336 (HugePages_Free=224), and no miner running yet (held=0). +# util/proposed-grub.sh's real fallback formula (1168*NUMA + THREADS + 50 + ceil(extra_mb/2)) gives +# required=3782 2MB pages for these inputs (re-derived independently in rigforge#398 — NOT the 3870 +# both pithead#1103 and pithead#1306 quote, which conflates proposed-grub.sh's two formula branches +# into one call that never actually happens). `_ensure_hugepages` is exercised directly (not +# through tune_kernel) so the ceiling logic is pinned in isolation from proposed-grub.sh's own +# math, already covered by the #65/#305 suites above. +echo "== unit: hugepages pool ceiling bounds the grow-only write (#398) ==" +HPC="$(mktemp -d "$SANDBOX/hpc398.XXXXXX")" +mkdir -p "$HPC/bin" +cat >"$HPC/bin/sysctl" <<'EOF' +#!/usr/bin/env bash +echo "$*" >>"$SYSCTL_CALLS" +EOF +chmod +x "$HPC/bin/sysctl" +run_ensure_hp398() { # [ceiling_mb] + ( + source "$SCRIPT" + printf 'HugePages_Free: %s\n' "$3" >"$HPC/meminfo" + printf '%s\n' "$4" >"$HPC/nr_hugepages" + MEMINFO="$HPC/meminfo" + NR_HUGEPAGES_FILE="$HPC/nr_hugepages" + _miner_held_hugepages() { echo 0; } # out of scope here — #328 above covers the held-pages credit + HUGEPAGES_POOL_CEILING_MB="${5:-0}" + export SYSCTL_CALLS="$1" + set +e + PATH="$HPC/bin:$STUBS:$PATH" _ensure_hugepages "$2" 2>&1 + ) +} + +# No ceiling declared: the 8 GB fixture's required/avail math is BYTE-FOR-BYTE the pre-#398 +# arithmetic — the conservative "no declaration, no behavior change" pin. (This unavoidably also +# reproduces the double count itself: fixing that is what declaring the ceiling below does, not a +# change to this arithmetic.) +SC="$HPC/calls1" +: >"$SC" +out="$(run_ensure_hp398 "$SC" 3782 224 2560)" +assert_contains "no ceiling declared -> unchanged pre-#398 arithmetic (8 GB fixture, #398)" "$(cat "$SC")" "vm.nr_hugepages=6118" + +# Ceiling declared, above current but below the uncapped target: the write is CAPPED at the +# ceiling instead of the double-counted 6118. Mutation kill: respelling _ensure_hugepages back to +# its pre-#398 body (`sudo sysctl -w vm.nr_hugepages=$((current + required - avail))`, no ceiling +# clamp) turns both assertions below red — the write goes back to 6118 regardless of the declared +# ceiling. Confirmed by hand against that exact reverted body before this test was written. +SC="$HPC/calls2" +: >"$SC" +out="$(run_ensure_hp398 "$SC" 3782 224 2560 6400)" +assert_contains "ceiling above current caps the write, not the double count (#398)" "$(cat "$SC")" "vm.nr_hugepages=3200" +assert_absent "capped write is no longer the double-counted 6118 (#398 mutation kill)" "$(cat "$SC")" "6118" +assert_contains "capping is logged with the ceiling reason (#398)" "$out" "capping the write" + +# Ceiling declared AT the tier's own already-committed reservation (2560 pages / 5120 MB, the real +# reduced-tier number from pithead#1103): the pool is already at the ceiling, so the write is +# skipped entirely rather than growing it — the miner gets zero extra pages, but the box is never +# pushed past its declared honest capacity. +SC="$HPC/calls3" +: >"$SC" +out="$(run_ensure_hp398 "$SC" 3782 224 2560 5120)" +assert_absent "ceiling already met -> no write at all (#398)" "$(cat "$SC")" "vm.nr_hugepages" +assert_contains "ceiling-already-met is a WARN naming the ceiling (#398)" "$out" "already at its declared ceiling" + +# An ODD declared ceiling must FLOOR to the 2MB page below, never round up past itself (security +# review finding on the first version of this fix: `(HUGEPAGES_POOL_CEILING_MB + 1) / 2` rounds an +# odd MB value UP, so 5121MB became 2561 pages = 5122MB — one page past the declared ceiling, +# violating "never grown past the ceiling"). 5121 floors to the SAME 2560 pages as the even 5120MB +# case above, so with current already at 2560 the pool is already at-or-past the (floored) ceiling +# and the write is skipped, exactly like calls3. Mutation kill: restoring the `+ 1` rounds 5121MB +# up to 2561 pages instead, which is > current(2560) — the code takes the CAP branch instead of the +# already-met branch and WRITES `vm.nr_hugepages=2561` (5122MB, over the declared 5121MB), flipping +# both assertions below red. +SC="$HPC/calls_odd" +: >"$SC" +out="$(run_ensure_hp398 "$SC" 3782 224 2560 5121)" +assert_absent "odd ceiling (5121MB) floors to 2560 pages -> no write, not 2561 (#398 mutation kill)" "$(cat "$SC")" "vm.nr_hugepages" +assert_contains "odd-ceiling floor is a WARN naming the ceiling (#398)" "$out" "already at its declared ceiling" + +# Regression: no headroom and no ceiling at all still behaves like plain #328 grow-only sizing, +# through the SAME direct-call path used above — proves #398 didn't reshape the ceiling-absent +# code path. +SC="$HPC/calls4" +: >"$SC" +out="$(run_ensure_hp398 "$SC" 200 0 0)" +assert_contains "no headroom, no ceiling -> plain requirement, unchanged (#328 x #398)" "$(cat "$SC")" "vm.nr_hugepages=200" + # --------------------------------------------------------------------------- # Appliance mode (pithead#797 R1): RIGFORGE_APPLIANCE=1 runs setup on the Pithead appliance image — # read-only root, volatile /etc overlay, a boot leg re-runs setup every boot. Under the flag setup @@ -7044,6 +7159,160 @@ bkf_out="$( ( assert_contains "commit: unwritable backup dir -> rejected" "$bkf_out" "rejected backup-failed" assert_eq "commit: backup failure leaves config.json untouched (donation 1)" "$(jq -r .DONATION "$bkfail/config.json")" "1" +echo "== unit: _control_fast_path_eligible — closed allowlist classification (#381) ==" +fpe() { # -> eligible|not + ( + source "$SCRIPT" + set +e + if _control_fast_path_eligible "$1"; then echo eligible; else echo not; fi + ) +} +assert_eq "single restart-free key: watchdog_interval_min -> eligible" "$(fpe "watchdog_interval_min")" "eligible" +assert_eq "single restart-free key: max_temp_c -> eligible" "$(fpe "max_temp_c")" "eligible" +assert_eq "both restart-free keys together -> eligible" "$(fpe "max_temp_c,watchdog_interval_min")" "eligible" +assert_eq "pools alone (own xmrig config) -> not eligible" "$(fpe "pools")" "not" +assert_eq "DONATION alone (own xmrig config) -> not eligible" "$(fpe "DONATION")" "not" +assert_eq "autotune alone (unaudited install_* path) -> not eligible" "$(fpe "autotune")" "not" +assert_eq "watchdog alone (unaudited install_* path) -> not eligible" "$(fpe "watchdog")" "not" +# Mutation this catches: the ALL-keys-must-qualify subset check loosened to an ANY-key check — a +# restart-free key riding alongside DONATION would wrongly clear the whole change for the fast path. +assert_eq "mixed restart-free + non-restart-free -> not eligible" "$(fpe "DONATION,max_temp_c")" "not" +# Mutation this catches: the closed-allowlist subset check inverted into a "not on the slow list" +# complement — an unrecognised/future CONTROL_WRITABLE_KEYS addition would then wrongly pass. +assert_eq "unrecognised/future key alone -> not eligible (closed set, not a complement)" "$(fpe "some_future_key")" "not" +assert_eq "empty keys-csv -> not eligible (fail closed)" "$(fpe "")" "not" +assert_eq "control_apply's jq-failure sentinel '?' -> not eligible (fail closed)" "$(fpe "?")" "not" + +# Pin the allowlist's exact membership (mirrors the CONTROL_WRITABLE_KEYS drift guard further below). +# Mutation this catches: the allowlist silently growing (or shrinking) without a matching +# evidence-trail/test update. +fp_keys="$(grep -oE 'CONTROL_FAST_PATH_KEYS="[^"]*"' "$SCRIPT" | head -1 | sed 's/.*="//; s/"//' | tr ' ' '\n' | sort | tr '\n' ' ')" +assert_eq "fast-path allowlist is EXACTLY {max_temp_c, watchdog_interval_min} (#381)" "$fp_keys" "max_temp_c watchdog_interval_min " +# Drift guard: the fast-path allowlist must stay a SUBSET of the control-writable allowlist — a +# fast-path key control_apply couldn't even accept as writable in the first place would be dead code. +ck_keys="$(grep -oE 'CONTROL_WRITABLE_KEYS="[^"]*"' "$SCRIPT" | head -1 | sed 's/.*="//; s/"//' | tr ' ' '\n' | sort | tr '\n' ' ')" +fp_subset_ok=y +for k in $fp_keys; do + case " $ck_keys " in *" $k "*) ;; *) fp_subset_ok=n ;; esac +done +assert_eq "fast-path allowlist is a subset of the control-writable allowlist (#381)" "$fp_subset_ok" "y" + +echo "== unit: _control_do_apply_fast — reuses install_watchdog, skips apply()/xmrig restart (#381) ==" +FPA="$(mktemp -d "$SANDBOX/fpa.XXXXXX")" +mkdir -p "$FPA/systemd" +cp "$ROOT/systemd/rigforge-watchdog.service.template" "$ROOT/systemd/rigforge-watchdog.timer.template" "$FPA/systemd/" +fpa_run() { # -> "rc="; side effects: $FPA/apply-called, $FPA/meta.json + rm -f "$FPA/apply-called" "$FPA/meta.json" + ( + source "$SCRIPT" + OS_TYPE=Linux + SCRIPT_DIR="$FPA" + SYSTEMD_DIR="$FPA/systemd" + REAL_USER=rfop + SERVICE_NAME=xmrig + CONFIG_JSON="$FPA/config.json" + CONFIG_META_FILE="$FPA/meta.json" + WATCHDOG_MODE=enabled + WATCHDOG_INTERVAL_MIN="$1" + # Isolate: parse_config would normally derive the globals above from CONFIG_JSON; stub it so + # this test pins _control_do_apply_fast's OWN behaviour, not parse_config's (covered elsewhere). + parse_config() { :; } + apply() { + echo called >"$FPA/apply-called" 2>/dev/null + return 0 + } + RIGFORGE_CONFIG_SOURCE=control + RIGFORGE_CONFIG_CHANGE_ID=fedcba9876543210 + set +e + PATH="$STUBS:$PATH" _control_do_apply_fast + echo "rc=$?" + ) +} +printf '{"pools":[{"url":"h:3333"}],"watchdog":"enabled","watchdog_interval_min":9}\n' >"$FPA/config.json" +out="$(fpa_run 9)" +assert_contains "_control_do_apply_fast returns 0 when the miner service is active" "$out" "rc=0" +assert_eq "_control_do_apply_fast NEVER calls apply() — xmrig is not restarted (#381)" "$([ -f "$FPA/apply-called" ] && echo called || echo not-called)" "not-called" +assert_contains "install_watchdog re-renders the timer with the NEW interval (#381)" "$(cat "$FPA/systemd/rigforge-watchdog.timer")" "OnUnitActiveSec=9min" +assert_eq "config_meta stamped source=control, parity with apply()'s own _stamp_config_meta call (#381)" "$(jq -r .source "$FPA/meta.json" 2>/dev/null)" "control" +assert_eq "config_meta records the change_id, same parity (#381)" "$(jq -r .last_change_id "$FPA/meta.json" 2>/dev/null)" "fedcba9876543210" + +echo "== unit: control_apply + REAL _control_do_apply_fast — run-state criterion, not is-active alone (#381 security review) ==" +# The generic systemctl stub always exits 0, so the fpa_run tests above only ever exercise the +# active-before/active-after case. A rig can be LEGITIMATELY stopped when a restart-free change +# lands — a watchdog thermal hold, or an operator's manual stop — and the fast path must not read +# that pre-existing stop as its own failure and roll the change back. This exercises control_apply +# end to end with the REAL _control_do_apply_fast (unlike the marker-stubbed one in ca_exec below) and +# a STATEFUL systemctl stub that answers `is-active` differently across the two calls the function +# makes (before the watchdog reconcile, and after), so the run-state comparison is genuinely tested. +CAF="$(mktemp -d "$SANDBOX/caf.XXXXXX")" +caf_systemctl_stub() { # -> writes $CAF/bin/systemctl + mkdir -p "$CAF/bin" + cat >"$CAF/bin/systemctl" <> "\${CALL_LOG:-/dev/null}" +case "\$*" in +*"is-active"*) + n=\$(( \$(cat "$CAF/systemctl-calls" 2>/dev/null || echo 0) + 1 )) + echo "\$n" >"$CAF/systemctl-calls" + if [ "\$n" -eq 1 ]; then exit $1; else exit $2; fi + ;; +*) exit 0 ;; +esac +EOF + chmod +x "$CAF/bin/systemctl" +} +caf_exec() { + ( + source "$SCRIPT" + parse_config() { :; } # the live config is already valid; don't re-validate it (matches ca_exec) + # apply()/_wait_miner_live only matter for the ROLLBACK leg here — already covered in depth by + # the #236/#276 tests below — so they stay simple stubs; the marker proves whether a rollback + # (i.e. a restart attempt) was ever reached, which is exactly what a wrongly-tripped fast-path + # failure would cause. + apply() { + echo called >"$CAF/full-apply-called" 2>/dev/null || true + return 0 + } + _wait_miner_live() { return 0; } + OS_TYPE=Linux + SCRIPT_DIR="$CAF" + SYSTEMD_DIR="$CAF/systemd" + CONFIG_JSON="$CAF/config.json" + REAL_USER=rfop + SERVICE_NAME=xmrig + WATCHDOG_MODE=enabled + RIGFORGE_CONTROL_STATE="$CAF/state" + set +e + PATH="$CAF/bin:$STUBS:$PATH" control_apply >/dev/null 2>&1 + ) +} +caf_run() { # + rm -rf "$CAF" + mkdir -p "$CAF/systemd" "$CAF/state/spool" + cp "$ROOT/systemd/rigforge-watchdog.service.template" "$ROOT/systemd/rigforge-watchdog.timer.template" "$CAF/systemd/" + printf '%s\n' "$CFG_236" >"$CAF/config.json" + printf '%s' "$1" >"$CAF/state/spool/pending-abc123.json" + caf_systemctl_stub "$2" "$3" + caf_exec +} +cfst() { jq -r ".$1" "$CAF/state/status.json" 2>/dev/null; } + +# (a) inactive-before, inactive-after (rc 1, 1): a rig thermally held or manually stopped before the +# change. Mutation this catches: reverting the run-state comparison to a naive "is it active NOW" +# check — that mutant reports rc=1 here (not active) and would wrongly roll the change back. +caf_run '{"max_temp_c":90}' 1 1 +assert_eq "inactive-before/inactive-after -> status applied, not rolled back (#381)" "$(cfst status)" "applied" +assert_eq "inactive-before/inactive-after -> the new value lands in config.json (#381)" "$(jq -r .max_temp_c "$CAF/config.json")" "90" +assert_eq "inactive-before/inactive-after -> no restart/rollback ever attempted (#381)" "$([ -f "$CAF/full-apply-called" ] && echo called || echo not-called)" "not-called" + +# (b) active-before, inactive-after (rc 0, 1): the miner really did go down across this change. +# Guards against the run-state criterion being dropped entirely (e.g. _control_do_apply_fast reverted +# to always returning 0) — a real regression here must still trip the existing rollback. +caf_run '{"max_temp_c":90}' 0 1 +assert_eq "active-before/inactive-after -> status rolled_back (#381)" "$(cfst status)" "rolled_back" +assert_eq "active-before/inactive-after -> config restored, max_temp_c unset again (#381)" "$(jq -r .max_temp_c "$CAF/config.json")" "null" +assert_eq "active-before/inactive-after -> rollback re-apply invoked (#381)" "$([ -f "$CAF/full-apply-called" ] && echo called || echo not-called)" "called" + echo "== unit: control_apply orchestration + rollback (#236) ==" CA="$(mktemp -d "$SANDBOX/ca.XXXXXX")" ca_exec() { @@ -7051,8 +7320,21 @@ ca_exec() { source "$SCRIPT" parse_config() { :; } # the live config is already valid; don't re-validate it # Stub apply + liveness (not _control_do_apply itself) so its real body runs: apply is a - # no-op, CA_APPLY_OK drives whether the miner "comes back" (0 -> the rollback path). - apply() { return 0; } + # no-op, CA_APPLY_OK drives whether the miner "comes back" (0 -> the rollback path). The + # marker file lets an assertion OUTSIDE this subshell prove apply() — the pipeline that + # regenerates xmrig's config and restarts it — was (or, #381, was deliberately NOT) reached. + apply() { + echo called >"$CA/full-apply-called" 2>/dev/null || true + return 0 + } + # #381: the fast-path counterpart, stubbed the same way — not _control_fast_path_eligible + # (its real body runs, since the routing decision IS what these tests exercise). + # CA_FAST_APPLY_OK drives whether it "succeeds" (default 1); on failure control_apply must + # fall through to the SAME full-pipeline rollback a failed full apply already takes. + _control_do_apply_fast() { + echo called >"$CA/fast-apply-called" 2>/dev/null || true + [ "${CA_FAST_APPLY_OK:-1}" = 1 ] + } # _wait_miner_live is called once for the initial apply and (on the rollback path) again for the # rollback re-apply. CA_APPLY_OK drives the 1st call; CA_ROLLBACK_OK drives the 2nd, defaulting to # CA_APPLY_OK so every pre-#276 test (which only ever sets CA_APPLY_OK) is unaffected — #276 pins @@ -7144,6 +7426,45 @@ printf '%s' '{"status":"pending","change_id":"'"$CID344"'","accepted_at":"2020-0 CA_APPLY_OK=1 ca_exec assert_eq "control_apply writes the terminal changes/.json (#344)" "$([ -f "$CA/state/changes/$CID344.json" ] && echo y || echo n)" "y" assert_eq "control_apply clears the now-superseded pending/.json (#344)" "$([ -f "$CA/state/pending/$CID344.json" ] && echo y || echo n)" "n" + +# #381 (from #344 item 1): control_apply must route a change through _control_do_apply_fast instead +# of the full, xmrig-restarting apply() IFF every changed key is on the closed CONTROL_FAST_PATH_KEYS +# allowlist — proven here via the full-apply-called/fast-apply-called markers ca_exec's stubs write, +# not just by the reported status (which is "applied" either way and so can't tell the paths apart). +ca_run "$CFG_236" '{"watchdog_interval_min":9}' 1 +assert_eq "watchdog_interval_min-only change -> status applied (#381)" "$(cst status)" "applied" +assert_eq "watchdog_interval_min-only change -> config committed" "$(jq -r .watchdog_interval_min "$CA/config.json")" "9" +assert_eq "watchdog_interval_min-only change takes the fast path" "$([ -f "$CA/fast-apply-called" ] && echo called || echo not-called)" "called" +assert_eq "watchdog_interval_min-only change NEVER calls apply() (xmrig untouched, #381)" "$([ -f "$CA/full-apply-called" ] && echo called || echo not-called)" "not-called" +ca_run "$CFG_236" '{"max_temp_c":90}' 1 +assert_eq "max_temp_c-only change also takes the fast path (#381)" "$([ -f "$CA/fast-apply-called" ] && echo called || echo not-called)" "called" +assert_eq "max_temp_c-only change never calls apply() (#381)" "$([ -f "$CA/full-apply-called" ] && echo called || echo not-called)" "not-called" +ca_run "$CFG_236" '{"watchdog_interval_min":9,"max_temp_c":90}' 1 +assert_eq "both restart-free keys together still take the fast path (#381)" "$([ -f "$CA/fast-apply-called" ] && echo called || echo not-called)" "called" +assert_eq "both-keys change reports applied (#381)" "$(cst status)" "applied" + +# Mutation this catches: if the ALL-keys-must-qualify subset check were loosened to an ANY-key (or a +# "not explicitly on the slow list") check, this mixed change would wrongly take the fast path and +# skip the restart DONATION needs to actually reach xmrig. +ca_run "$CFG_236" '{"DONATION":5,"max_temp_c":90}' 1 +assert_eq "mixed restart-free + full-path key -> takes the FULL path (#381)" "$([ -f "$CA/full-apply-called" ] && echo called || echo not-called)" "called" +assert_eq "mixed change never takes the fast path (#381)" "$([ -f "$CA/fast-apply-called" ] && echo called || echo not-called)" "not-called" +assert_eq "mixed change still reports applied" "$(cst status)" "applied" +# A pure DONATION change (already covered under #236 above) stays on the full path too — restated +# here under #381 naming so the fast/full boundary is asserted with the marker files in one place. +ca_run "$CFG_236" '{"DONATION":6}' 1 +assert_eq "DONATION-only change takes the full path, not fast (#381)" "$([ -f "$CA/full-apply-called" ] && echo called || echo not-called)" "called" +assert_eq "DONATION-only change never takes the fast path (#381)" "$([ -f "$CA/fast-apply-called" ] && echo called || echo not-called)" "not-called" + +# A failed fast-path apply must fall back to the SAME full, restart-safe rollback a failed full apply +# already takes — never report "applied" on a fast-path failure, and never leave the change stuck. +# Mutation this catches: the fast branch skipping the rollback on failure, or reporting "applied" +# regardless of _control_do_apply_fast's return code. +CA_FAST_APPLY_OK=0 ca_run "$CFG_236" '{"max_temp_c":90}' 1 +assert_eq "failed fast-path apply -> status rolled_back, not applied (#381)" "$(cst status)" "rolled_back" +assert_eq "failed fast-path apply -> config restored to pre-change (max_temp_c unset again)" "$(jq -r .max_temp_c "$CA/config.json")" "null" +assert_eq "the rollback re-apply after a fast-path failure uses the FULL path (#381)" "$([ -f "$CA/full-apply-called" ] && echo called || echo not-called)" "called" + # prune: KEEP_CONFIG_BACKUPS caps the history PB="$(mktemp -d "$SANDBOX/pb.XXXXXX")" mkdir -p "$PB/bk"