fix(round): three failures that each cost a whole round's weights without looking broken - #237
Conversation
…hout looking broken Audit findings F4, F5 and F6. None is exploitable by an outsider; each silently loses a round. **F4 — the compose window was one block wide.** The compose required `block_offset == weight_set_offset` EXACTLY, so a validator whose tick landed on 6601 instead of 6600 never composed that round at all: it kept re-asserting the previous round's weights for a full day while a fresh set of scores sat unused. Anything that shifts a tick — a GC pause, a slow fetch, a restart, a benchmark run that overruns — cost a round's payout. It now composes at the first block AT OR AFTER the offset. Widening is safe because the compose was already idempotent per round through `last_composed_round`: the first qualifying tick composes and every later one falls through to the keep-alive. `is_weight_set_block` stays for callers that genuinely mean "is this THE block", like a dashboard marking the point. **F5 — a backend outage took down the keep-alive.** Benchmarking ran before the weight action and its failure raised out of `step()`, so a backend the validator merely READS from could stop it discharging a chain obligation that has nothing to do with the backend — and the chain zeroes a validator that goes quiet. The module's own comment already said this must not happen; only the control flow disagreed. The failure is now carried on `RuntimeState.last_benchmark_error`, the weight action still runs, and `reported_round` is deliberately not advanced so the next tick retries. The test that asserted the old behaviour asserted the bug, and has been corrected with the reason written down. **F6 — infrastructure failure was reported as real zeros.** A validator that ran out of time abstained; a validator whose Docker daemon was dead scored every miner zero and reported those zeros as EVALUATED, so one broken validator dragged the whole field down for a fault that says nothing about any miner. `BenchmarkUnavailable` now separates "I could not run the differential" from "the PoC did not reproduce", and `evaluate_round` turns the first into an abstention exactly as it already does for the deadline. The seam raises it for a dead daemon, a missing image (one that never finished pulling is OUR failure), a full disk, and a missing docker binary — all of which previously returned False and read exactly like a PoC that did not work. A malformed proof or a broken PoC is still the miner's result and still scores zero. One unrunnable task marks that miner unevaluated even when its other tasks succeeded: a partial score reported as evaluated is a number we know to be too low. 141 v2 round tests pass; ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Self-review before merge (owner-directed; posted for post-merge audit)All seven checks pass on 3.11 and 3.12. What I checked hardest, per finding: F4 — widening the window is only safe because the compose is idempotent per round, so that property now has its own test rather than being assumed. F5 — F6 — the discrimination is the whole fix, so both directions are tested: a dead daemon, a missing image, a full disk and a missing binary all abstain; a PoC that genuinely does not reproduce, and a malformed proof, still score zero as the miner's result. One unrunnable task marks that miner unevaluated even when its others succeeded — a partial score reported as evaluated is a number we know to be too low. Worth flagging: the pre-existing test Not closed here: F7 (restart reopens the duplicate-agent gate), F8 (handle positions do not rotate), F10 (submit accepts any round_id), the fee's 141 v2 round tests pass; ruff clean. |
1 participant
Audit findings F4, F5, F6. None is reachable by an outsider; each silently loses a round's payout while everything looks healthy.
F4 — the compose window was one block wide
The compose required
block_offset == weight_set_offsetexactly. A tick landing on 6601 instead of 6600 meant the round was never composed at all — the validator re-asserted the previous round's weights for a full day while fresh scores sat unused. A GC pause, a slow fetch, a restart, or a benchmark run that overruns all cost a round.Widening is safe because the compose was already idempotent per round via
last_composed_round— the first qualifying tick composes, every later one falls through to the keep-alive. There's a test for that, since it's the property the widening depends on.F5 — a backend outage took down the keep-alive
Benchmarking ran before the weight action and its failure raised out of
step(). So a backend the validator merely reads from could stop it discharging a chain obligation that has nothing to do with the backend — and the chain zeroes a validator that goes quiet.The module's own comment already said "a failed report must not stop the weight obligation". Only the control flow disagreed.
Now the failure is carried on the state, the weights still go out, and
reported_roundis not advanced so the next tick retries. The existing test asserted the old behaviour — it had encoded the bug — and is corrected with the reasoning in place.F6 — infrastructure failure was reported as real zeros
Two failure modes, opposite treatment: a validator out of time abstained, while a validator with a dead Docker daemon scored every miner zero, reported as evaluated. One broken validator dragged the whole field down for a fault that says nothing about any miner.
BenchmarkUnavailablenow separates "I could not run the differential" from "the PoC did not reproduce":A missing image matters especially now that the corpus is pulled per round: an image that never finished downloading is our failure, and used to score the whole field zero on that task.
One unrunnable task marks that miner unevaluated even when its others succeeded — a partial score reported as evaluated is a number we know to be too low.
141 v2 round tests pass; ruff clean.